From 9e2a99d93383fd4b08b48c6cd8415dc4391d0aad Mon Sep 17 00:00:00 2001
From: Yurii Kondakov <ykondakov@slb.com>
Date: Tue, 20 Jun 2023 15:18:46 +0300
Subject: [PATCH] add request timeout to crs conversion service requests
---
.../conversion/CrsConversionService.java | 104 +++++++++++++-----
.../osdu/storage/di/CrsConversionConfig.java | 34 ++++++
.../conversion/CrsConversionServiceTest.java | 13 ++-
3 files changed, 122 insertions(+), 29 deletions(-)
create mode 100644 storage-core/src/main/java/org/opengroup/osdu/storage/di/CrsConversionConfig.java
diff --git a/storage-core/src/main/java/org/opengroup/osdu/storage/conversion/CrsConversionService.java b/storage-core/src/main/java/org/opengroup/osdu/storage/conversion/CrsConversionService.java
index ba79800e6..3647fe320 100644
--- a/storage-core/src/main/java/org/opengroup/osdu/storage/conversion/CrsConversionService.java
+++ b/storage-core/src/main/java/org/opengroup/osdu/storage/conversion/CrsConversionService.java
@@ -1,4 +1,4 @@
-// Copyright 2017-2019, Schlumberger
+// Copyright 2017-2023, Schlumberger
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
@@ -14,28 +14,60 @@
package org.opengroup.osdu.storage.conversion;
-import java.util.*;
-import com.google.gson.*;
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.common.base.Strings;
+import com.google.gson.Gson;
+import com.google.gson.JsonArray;
+import com.google.gson.JsonElement;
+import com.google.gson.JsonNull;
+import com.google.gson.JsonObject;
+import com.google.gson.JsonParser;
+import com.google.gson.JsonSyntaxException;
import org.apache.http.HttpStatus;
-import com.fasterxml.jackson.databind.ObjectMapper;
-import com.fasterxml.jackson.core.JsonProcessingException;
+import org.apache.http.client.config.RequestConfig;
import org.opengroup.osdu.core.common.Constants;
-import org.opengroup.osdu.core.common.model.crs.*;
-import org.opengroup.osdu.core.common.logging.JaxRsDpsLog;
-import org.opengroup.osdu.core.common.model.crs.GeoJson.*;
-import org.opengroup.osdu.core.common.model.http.AppException;
+import org.opengroup.osdu.core.common.crs.CrsConversionServiceErrorMessages;
import org.opengroup.osdu.core.common.crs.ICrsConverterFactory;
import org.opengroup.osdu.core.common.crs.ICrsConverterService;
-import org.opengroup.osdu.core.common.crs.CrsConversionServiceErrorMessages;
+import org.opengroup.osdu.core.common.logging.JaxRsDpsLog;
+import org.opengroup.osdu.core.common.model.crs.ConvertGeoJsonRequest;
+import org.opengroup.osdu.core.common.model.crs.ConvertGeoJsonResponse;
+import org.opengroup.osdu.core.common.model.crs.ConvertPointsRequest;
+import org.opengroup.osdu.core.common.model.crs.ConvertPointsResponse;
+import org.opengroup.osdu.core.common.model.crs.ConvertStatus;
+import org.opengroup.osdu.core.common.model.crs.CrsConverterException;
+import org.opengroup.osdu.core.common.model.crs.CrsPropertySet;
+import org.opengroup.osdu.core.common.model.crs.GeoJson.GeoJsonBase;
+import org.opengroup.osdu.core.common.model.crs.GeoJson.GeoJsonFeature;
+import org.opengroup.osdu.core.common.model.crs.GeoJson.GeoJsonFeatureCollection;
+import org.opengroup.osdu.core.common.model.crs.GeoJson.GeoJsonGeometryCollection;
+import org.opengroup.osdu.core.common.model.crs.GeoJson.GeoJsonLineString;
+import org.opengroup.osdu.core.common.model.crs.GeoJson.GeoJsonMultiLineString;
+import org.opengroup.osdu.core.common.model.crs.GeoJson.GeoJsonMultiPoint;
+import org.opengroup.osdu.core.common.model.crs.GeoJson.GeoJsonMultiPolygon;
+import org.opengroup.osdu.core.common.model.crs.GeoJson.GeoJsonPoint;
+import org.opengroup.osdu.core.common.model.crs.GeoJson.GeoJsonPolygon;
+import org.opengroup.osdu.core.common.model.crs.Point;
+import org.opengroup.osdu.core.common.model.crs.PointConversionInfo;
+import org.opengroup.osdu.core.common.model.crs.RecordsAndStatuses;
+import org.opengroup.osdu.core.common.model.http.AppException;
+import org.opengroup.osdu.core.common.model.http.DpsHeaders;
+import org.opengroup.osdu.core.common.model.http.RequestStatus;
+import org.opengroup.osdu.core.common.model.storage.ConversionStatus;
import org.opengroup.osdu.core.common.util.IServiceAccountJwtClient;
+import org.opengroup.osdu.storage.di.CrsConversionConfig;
+import org.opengroup.osdu.storage.di.SpringConfig;
import org.opengroup.osdu.storage.util.ConversionJsonUtils;
+import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.annotation.Lazy;
import org.springframework.stereotype.Service;
-import org.springframework.beans.factory.annotation.Autowired;
-import org.opengroup.osdu.core.common.model.http.DpsHeaders;
-import org.opengroup.osdu.core.common.model.storage.ConversionStatus;
-import org.opengroup.osdu.storage.di.SpringConfig;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
import static org.opengroup.osdu.core.common.Constants.*;
import static org.opengroup.osdu.core.common.crs.CrsConversionServiceErrorMessages.MISSING_GEOMETRIES;
@@ -72,9 +104,13 @@ public class CrsConversionService {
@Autowired
private SpringConfig springConfig;
+
@Autowired
private ConversionJsonUtils conversionJsonUtils;
+ @Autowired
+ private CrsConversionConfig crsConversionConfig;
+
public RecordsAndStatuses doCrsConversion(List<JsonObject> originalRecords, List<ConversionStatus.ConversionStatusBuilder> conversionStatuses) {
RecordsAndStatuses crsConversionResult = new RecordsAndStatuses();
Map<String, List<PointConversionInfo>> pointConversionInfoList = this.gatherCrsConversionData(originalRecords, conversionStatuses);
@@ -141,8 +177,7 @@ public class CrsConversionService {
}
private void gatherCrsGeoJsonConversionData(List<JsonObject> originalRecords, List<ConversionStatus.ConversionStatusBuilder> conversionStatuses) {
- for (int i = 0; i < originalRecords.size(); i++) {
- JsonObject recordJsonObject = originalRecords.get(i);
+ for (JsonObject recordJsonObject : originalRecords) {
String recordId = this.getRecordId(recordJsonObject);
ConversionStatus.ConversionStatusBuilder statusBuilder = this.getConversionStatusBuilderFromList(recordId, conversionStatuses);
if(statusBuilder == null){
@@ -150,9 +185,7 @@ public class CrsConversionService {
}
List<String> validationErrors = new ArrayList<>();
JsonObject filteredObjects = this.dpsConversionService.filterDataFields(recordJsonObject, validationErrors);
- Iterator<String> keys = filteredObjects.keySet().iterator();
- while(keys.hasNext()) {
- String attributeName = keys.next();
+ for (String attributeName : filteredObjects.keySet()) {
JsonObject asIngestedCoordinates = filteredObjects.getAsJsonObject(attributeName).getAsJsonObject(AS_INGESTED_COORDINATES);
if (asIngestedCoordinates != null) {
GeoJsonFeatureCollection fc = new GeoJsonFeatureCollection();
@@ -176,7 +209,7 @@ public class CrsConversionService {
}
fc.setFeatures(featureArray);
- ICrsConverterService crsConverterService = this.crsConverterFactory.create(this.customizeHeaderBeforeCallingCrsConversion(this.dpsHeaders));
+ ICrsConverterService crsConverterService = this.crsConverterFactory.create(this.customizeHeaderBeforeCallingCrsConversion(this.dpsHeaders), getRequestConfig());
ConvertGeoJsonRequest request = new ConvertGeoJsonRequest(fc, TO_CRS_GEO_JSON, TO_UNIT_Z);
try {
if (statusBuilder.getErrors().isEmpty()) {
@@ -192,6 +225,10 @@ public class CrsConversionService {
} else {
this.logger.error(String.format(CrsConversionServiceErrorMessages.CRS_OTHER_ERROR, crsEx.getHttpResponse().toString()));
}
+ } catch (AppException ex) {
+ if (ex.getError().getCode() == RequestStatus.SOCKET_TIMEOUT) {
+ statusBuilder.addError(String.format(CrsConversionServiceErrorMessages.CRS_OTHER_ERROR, ex.getError().getMessage()));
+ }
}
} else {
statusBuilder.addError(CrsConversionServiceErrorMessages.MISSING_AS_INGESTED_COORDINATES);
@@ -382,7 +419,7 @@ public class CrsConversionService {
originalPoints.add(point);
}
- ICrsConverterService crsConverterService = this.crsConverterFactory.create(this.customizeHeaderBeforeCallingCrsConversion(this.dpsHeaders));
+ ICrsConverterService crsConverterService = this.crsConverterFactory.create(this.customizeHeaderBeforeCallingCrsConversion(this.dpsHeaders), getRequestConfig());
ConvertPointsRequest request = new ConvertPointsRequest(persistableReference, TO_CRS, originalPoints);
ConvertPointsResponse response = crsConverterService.convertPoints(request);
@@ -424,10 +461,12 @@ public class CrsConversionService {
this.logger.error(String.format(CrsConversionServiceErrorMessages.CRS_OTHER_ERROR, cvEx.getHttpResponse().toString()));
throw new AppException(HttpStatus.SC_INTERNAL_SERVER_ERROR, UNKNOWN_ERROR, "crs conversion service error.");
}
- } catch (ClassCastException ccEx) {
+ } catch (ClassCastException | IllegalStateException ccEx) {
statusBuilder.addError(String.format(CrsConversionServiceErrorMessages.ILLEGAL_DATA_IN_NESTED_PROPERTY, nestedFieldName, ccEx.getMessage()));
- } catch (IllegalStateException isEx) {
- statusBuilder.addError(String.format(CrsConversionServiceErrorMessages.ILLEGAL_DATA_IN_NESTED_PROPERTY, nestedFieldName, isEx.getMessage()));
+ } catch (AppException ex) {
+ if (ex.getError().getCode() == RequestStatus.SOCKET_TIMEOUT) {
+ statusBuilder.addError(String.format(CrsConversionServiceErrorMessages.CRS_OTHER_ERROR, ex.getError().getMessage()));
+ }
} catch (Exception e) {
statusBuilder.addError(e.getMessage());
}
@@ -449,8 +488,8 @@ public class CrsConversionService {
}
List<PointConversionInfo> callClientLibraryDoConversion(Map<String, List<PointConversionInfo>> originalPointsMap, List<ConversionStatus.ConversionStatusBuilder> conversionStatuses) {
- ICrsConverterService crsConverterService = this.crsConverterFactory.create(this.customizeHeaderBeforeCallingCrsConversion(this.dpsHeaders));
- List<PointConversionInfo> convertedPointInfo = new ArrayList<>();
+ ICrsConverterService crsConverterService = this.crsConverterFactory.create(this.customizeHeaderBeforeCallingCrsConversion(this.dpsHeaders), getRequestConfig());
+ List<PointConversionInfo> convertedPointInfo = new ArrayList<>();
for (Map.Entry<String, List<PointConversionInfo>> entry : originalPointsMap.entrySet()) {
List<Point> pointsToBeConverted = new ArrayList<>();
@@ -470,11 +509,14 @@ public class CrsConversionService {
} catch (CrsConverterException e) {
if (e.getHttpResponse().IsBadRequestCode()) {
convertedPointInfo.addAll(this.putDataErrorFromCrsIntoPointsInfo(pointsList, e.getMessage()));
- continue;
} else {
this.logger.error(String.format(CrsConversionServiceErrorMessages.CRS_OTHER_ERROR, e.getHttpResponse().toString()));
throw new AppException(HttpStatus.SC_INTERNAL_SERVER_ERROR, UNKNOWN_ERROR, "crs conversion service error.");
}
+ } catch (AppException ex) {
+ if (ex.getError().getCode() == RequestStatus.SOCKET_TIMEOUT) {
+ convertedPointInfo.addAll(this.putDataErrorFromCrsIntoPointsInfo(pointsList, ex.getMessage()));
+ }
}
}
return convertedPointInfo;
@@ -760,4 +802,12 @@ public class CrsConversionService {
}
return bbox;
}
+
+ private RequestConfig getRequestConfig(){
+ return RequestConfig.custom()
+ .setSocketTimeout(crsConversionConfig.getSocketTimeout())
+ .setConnectTimeout(crsConversionConfig.getConnectTimeout())
+ .setConnectionRequestTimeout(crsConversionConfig.getConnectionRequestTimeout())
+ .build();
+ }
}
diff --git a/storage-core/src/main/java/org/opengroup/osdu/storage/di/CrsConversionConfig.java b/storage-core/src/main/java/org/opengroup/osdu/storage/di/CrsConversionConfig.java
new file mode 100644
index 000000000..6fd28cd09
--- /dev/null
+++ b/storage-core/src/main/java/org/opengroup/osdu/storage/di/CrsConversionConfig.java
@@ -0,0 +1,34 @@
+// Copyright 2017-2023, Schlumberger
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package org.opengroup.osdu.storage.di;
+
+import lombok.Getter;
+import org.springframework.beans.factory.annotation.Value;
+import org.springframework.context.annotation.Configuration;
+
+@Getter
+@Configuration
+public class CrsConversionConfig {
+
+ @Value("${crs.conversion.connect.timeout:5000}")
+ private int connectTimeout;
+
+ @Value("${crs.conversion.socket.timeout:60000}")
+ private int socketTimeout;
+
+ @Value("${crs.conversion.connection.request.timeout:60000}")
+ private int connectionRequestTimeout;
+
+}
diff --git a/storage-core/src/test/java/org/opengroup/osdu/storage/conversion/CrsConversionServiceTest.java b/storage-core/src/test/java/org/opengroup/osdu/storage/conversion/CrsConversionServiceTest.java
index 242c531d1..4b81e2b6f 100644
--- a/storage-core/src/test/java/org/opengroup/osdu/storage/conversion/CrsConversionServiceTest.java
+++ b/storage-core/src/test/java/org/opengroup/osdu/storage/conversion/CrsConversionServiceTest.java
@@ -1,4 +1,4 @@
-// Copyright 2017-2019, Schlumberger
+// Copyright 2017-2023, Schlumberger
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
@@ -16,6 +16,7 @@ package org.opengroup.osdu.storage.conversion;
import com.google.gson.JsonObject;
import com.google.gson.JsonParser;
+import org.apache.http.client.config.RequestConfig;
import org.opengroup.osdu.core.common.crs.CrsConversionServiceErrorMessages;
import org.opengroup.osdu.core.common.model.crs.*;
import org.opengroup.osdu.core.common.model.storage.ConversionStatus;
@@ -23,6 +24,7 @@ import org.opengroup.osdu.core.common.model.crs.CrsPropertySet;
import org.opengroup.osdu.core.common.model.crs.RecordsAndStatuses;
import org.opengroup.osdu.core.common.logging.JaxRsDpsLog;
import org.opengroup.osdu.core.common.util.IServiceAccountJwtClient;
+import org.opengroup.osdu.storage.di.CrsConversionConfig;
import org.opengroup.osdu.storage.di.SpringConfig;
import org.apache.http.HttpStatus;
import org.junit.Assert;
@@ -69,6 +71,9 @@ public class CrsConversionServiceTest {
@Mock
private SpringConfig springConfig;
+ @Mock
+ private CrsConversionConfig crsConversionConfig;
+
private List<JsonObject> originalRecords = new ArrayList<>();
private List<ConversionStatus.ConversionStatusBuilder> conversionStatuses = new ArrayList<>();
private List<Point> convertedPoints = new ArrayList<>();
@@ -110,6 +115,7 @@ public class CrsConversionServiceTest {
private static final String CONVERTED_RECORD_7 = "{\"id\":\"unit-test-6\",\"kind\":\"unit:test:1.0.0\",\"acl\":{\"viewers\":[\"viewers@unittest.com\"],\"owners\":[\"owners@unittest.com\"]},\"legal\":{\"legaltags\":[\"unit-test-legal\"],\"otherRelevantDataCountries\":[\"AA\"]},\"data\":{\"msg\":\"testing record 1\",\"X\":15788.036,\"Y\":9567.4,\"Z\":0.0},\"meta\":[{\"path\":\"\",\"kind\":\"CRS\",\"propertyNames\":[\"X\",\"Y\",\"Z\",\"T\"],\"name\":\"GCS_WGS_1984\",\"persistableReference\":\"%s\"}]}";
private static final String CONVERTED_RECORD_8 = "{\"id\":\"unit-test-20\",\"kind\":\"unit:test:1.0.0\",\"acl\":{\"viewers\":[\"viewers@unittest.com\"],\"owners\":[\"owners@unittest.com\"]},\"legal\":{\"legaltags\":[\"unit-test-legal\"],\"otherRelevantDataCountries\":[\"AA\"]},\"data\":{\"msg\":\"testing record 1\",\"Nested\":{\"X\":15788.036,\"Y\":9567.4},\"Z\":0.0},\"meta\":[{\"path\":\"\",\"kind\":\"CRS\",\"propertyNames\":[\"Nested.X\",\"Nested.Y\"],\"name\":\"GCS_WGS_1984\",\"persistableReference\":\"%s\"}]}";
private static final String CONVERTED_RECORD_9 = "{\"id\":\"unit-test-21\",\"kind\":\"unit:test:1.0.0\",\"acl\":{\"viewers\":[\"viewers@unittest.com\"],\"owners\":[\"owners@unittest.com\"]},\"legal\":{\"legaltags\":[\"unit-test-legal\"],\"otherRelevantDataCountries\":[\"AA\"]},\"data\":{\"msg\":\"testing record 1\",\"Nested\":{\"X\":15788.036,\"Y\":9567.4},\"Z\":0.0},\"meta\":[{\"path\":\"\",\"kind\":\"CRS\",\"propertyNames\":[\"Nested.X\",\"Nested.Y\"],\"name\":\"GCS_WGS_1984\",\"persistableReference\":\"%s\"}]}";
+ private static final int TIMEOUT = 60000;
@Before
public void setup() throws Exception{
@@ -132,7 +138,10 @@ public class CrsConversionServiceTest {
when(this.crsPropertySet.getPropertyPairing()).thenReturn(this.pairProperty);
when(this.crsPropertySet.getNestedPropertyNames()).thenReturn(this.nestedPropertyNames);
- when(this.crsConverterFactory.create(any())).thenReturn(this.crsConverterService);
+ when(this.crsConversionConfig.getSocketTimeout()).thenReturn(TIMEOUT);
+ when(this.crsConversionConfig.getConnectTimeout()).thenReturn(TIMEOUT);
+ when(this.crsConversionConfig.getConnectionRequestTimeout()).thenReturn(TIMEOUT);
+ when(this.crsConverterFactory.create(any(), any(RequestConfig.class))).thenReturn(this.crsConverterService);
when(this.crsConverterService.convertPoints(any())).thenReturn(this.convertPointsResponse);
when(this.jwtClient.getIdToken(any())).thenReturn("auth-token-unit-test");
--
GitLab