diff --git a/server/src/main/java/au/org/aodn/ogcapi/server/core/configuration/Config.java b/server/src/main/java/au/org/aodn/ogcapi/server/core/configuration/Config.java index ba484234..97363250 100644 --- a/server/src/main/java/au/org/aodn/ogcapi/server/core/configuration/Config.java +++ b/server/src/main/java/au/org/aodn/ogcapi/server/core/configuration/Config.java @@ -42,6 +42,7 @@ public class Config { public static final String DAS_REST_TEMPLATE = "dasRestTemplate"; + public static final String DOWNLOAD_REST_TEMPLATE = "downloadRestTemplate"; public static final String DAS_SSE_WEB_CLIENT = "dasSseWebClient"; @Autowired @@ -65,8 +66,20 @@ public void init() { ); } + /** + * The default template, short timeouts so a down service cannot hang the caller. + */ @Bean public RestTemplate createRestTemplate() { + SimpleClientHttpRequestFactory factory = new SimpleClientHttpRequestFactory(); + factory.setConnectTimeout(10000); // 10 seconds connection timeout + factory.setReadTimeout(60000); // 1 minute read timeout + + return new RestTemplate(factory); + } + + @Bean(name = DOWNLOAD_REST_TEMPLATE, defaultCandidate = false) + public RestTemplate createDownloadRestTemplate() { SimpleClientHttpRequestFactory factory = new SimpleClientHttpRequestFactory(); factory.setConnectTimeout(1200000); // 20 minutes connection timeout factory.setReadTimeout(1200000); // 20 minutes read timeout for large downloads diff --git a/server/src/main/java/au/org/aodn/ogcapi/server/core/configuration/GeoServerConfig.java b/server/src/main/java/au/org/aodn/ogcapi/server/core/configuration/GeoServerConfig.java index 36d3501e..9a47b1b3 100644 --- a/server/src/main/java/au/org/aodn/ogcapi/server/core/configuration/GeoServerConfig.java +++ b/server/src/main/java/au/org/aodn/ogcapi/server/core/configuration/GeoServerConfig.java @@ -100,7 +100,7 @@ public WmsServer createWmsServer(Search search, @Lazy WfsServer wfsServer, @Qual @Bean @ConditionalOnMissingBean(DownloadWfsDataService.class) public DownloadWfsDataService createDownloadWfsDataService(WfsServer wfsServer, - RestTemplate restTemplate, + @Qualifier(Config.DOWNLOAD_REST_TEMPLATE) RestTemplate restTemplate, @Qualifier("pretendUserEntity") HttpEntity pretendUserEntity, @Value("${app.sse.chunkSize:16384}") int chunkSize, ObjectMapper objectMapper) { diff --git a/server/src/main/java/au/org/aodn/ogcapi/server/core/service/ApplicationInfo.java b/server/src/main/java/au/org/aodn/ogcapi/server/core/service/ApplicationInfo.java index 5bbb8c8a..c9894b54 100644 --- a/server/src/main/java/au/org/aodn/ogcapi/server/core/service/ApplicationInfo.java +++ b/server/src/main/java/au/org/aodn/ogcapi/server/core/service/ApplicationInfo.java @@ -8,12 +8,38 @@ import java.util.Collections; import java.util.Map; +import java.util.function.Supplier; public interface ApplicationInfo { @Slf4j class LogHolder { // Lombok injects the logger here inside the static inner class } + + /** + * Load the info on first use, not at startup, so a down service cannot block startup. + * Only a good result is kept, an empty result is retried on the next call. + */ + class InfoCache { + protected final Supplier>> loader; + protected volatile Map> info; + + public InfoCache(Supplier>> loader) { + this.loader = loader; + } + + public Map> get() { + if (info == null) { + Map> result = loader.get(); + if (result == null || result.isEmpty()) { + return Collections.emptyMap(); + } + info = result; + } + return info; + } + } + /** * Query this is repeat code to query the info path * @param restTemplate - The template of the service @@ -35,18 +61,37 @@ default Map> queryInfo(RestTemplate restTemplate, String host, } ); - if (response.getStatusCode().is2xxSuccessful()) { + if (response != null && response.getStatusCode().is2xxSuccessful() && response.getBody() != null) { return response.getBody(); } } catch (Exception e) { - // Do not throw exception that impacts start, UI will use health endpoint to decide level of + // Do not throw exception that impacts service, UI will use health endpoint to decide level of // service, for example if geonetwork is not work, we can still provide service + LogHolder.log.warn("Fail to query service info for {}, {}, cause: {}", host, path, e.getMessage()); } } return Collections.emptyMap(); } - String getName(); - String getVersion(); - String getDescription(); + /** + * @return - The info of the service, empty map if the service cannot be reached + */ + Map> getAppInfo(); + + default String getName() { + return getApplicationValue("name"); + } + + default String getVersion() { + return getApplicationValue("version"); + } + + default String getDescription() { + return getApplicationValue("description"); + } + + private String getApplicationValue(String key) { + Object value = getAppInfo().getOrDefault("application", Collections.emptyMap()).getOrDefault(key, null); + return value != null ? value.toString() : null; + } } diff --git a/server/src/main/java/au/org/aodn/ogcapi/server/core/service/das/DasService.java b/server/src/main/java/au/org/aodn/ogcapi/server/core/service/das/DasService.java index 085efba3..fb6f3f28 100644 --- a/server/src/main/java/au/org/aodn/ogcapi/server/core/service/das/DasService.java +++ b/server/src/main/java/au/org/aodn/ogcapi/server/core/service/das/DasService.java @@ -20,7 +20,6 @@ import java.io.IOException; import java.io.UncheckedIOException; -import java.util.Collections; import java.util.HashMap; import java.util.Iterator; import java.util.Map; @@ -39,7 +38,7 @@ public class DasService implements ApplicationInfo { protected final RestTemplate httpClient; protected final WebClient sseHttpClient; protected final ObjectMapper objectMapper; - protected final Map> appInfo; + protected final InfoCache appInfo; public DasService( DasProperties dasProperties, @@ -50,7 +49,7 @@ public DasService( this.httpClient = httpClient; this.sseHttpClient = sseHttpClient; this.objectMapper = objectMapper; - this.appInfo = queryInfo(httpClient, dasProperties.host(), dasProperties.infoPath()); + this.appInfo = new InfoCache(() -> queryInfo(httpClient, dasProperties.host(), dasProperties.infoPath())); } /** @@ -217,20 +216,7 @@ public ResponseEntity getDatasetMetadata(String datasetId) { } @Override - public String getName() { - Object name = this.appInfo.getOrDefault("application", Collections.emptyMap()).getOrDefault("name", null); - return name != null ? name.toString() : null; - } - - @Override - public String getVersion() { - Object version = this.appInfo.getOrDefault("application", Collections.emptyMap()).getOrDefault("version", null); - return version != null ? version.toString() : null; - } - - @Override - public String getDescription() { - Object description = this.appInfo.getOrDefault("application", Collections.emptyMap()).getOrDefault("description", null); - return description != null ? description.toString() : null; + public Map> getAppInfo() { + return appInfo.get(); } } diff --git a/server/src/main/java/au/org/aodn/ogcapi/server/core/service/dda/DdaService.java b/server/src/main/java/au/org/aodn/ogcapi/server/core/service/dda/DdaService.java index 7fec8cbe..38b2c9a0 100644 --- a/server/src/main/java/au/org/aodn/ogcapi/server/core/service/dda/DdaService.java +++ b/server/src/main/java/au/org/aodn/ogcapi/server/core/service/dda/DdaService.java @@ -2,31 +2,17 @@ import au.org.aodn.ogcapi.server.core.service.ApplicationInfo; import org.springframework.web.client.RestTemplate; -import java.util.Collections; import java.util.Map; public class DdaService implements ApplicationInfo { - protected final Map> appInfo; + protected final InfoCache appInfo; public DdaService(DdaProperties properties, RestTemplate template) { - this.appInfo = queryInfo(template, properties.host(), properties.infoPath()); + this.appInfo = new InfoCache(() -> queryInfo(template, properties.host(), properties.infoPath())); } @Override - public String getName() { - Object name = this.appInfo.getOrDefault("application", Collections.emptyMap()).getOrDefault("name", null); - return name != null ? name.toString() : null; - } - - @Override - public String getVersion() { - Object version = this.appInfo.getOrDefault("application", Collections.emptyMap()).getOrDefault("version", null); - return version != null ? version.toString() : null; - } - - @Override - public String getDescription() { - Object description = this.appInfo.getOrDefault("application", Collections.emptyMap()).getOrDefault("description", null); - return description != null ? description.toString() : null; + public Map> getAppInfo() { + return appInfo.get(); } } diff --git a/server/src/main/java/au/org/aodn/ogcapi/server/core/service/geonetwork/Geonetwork.java b/server/src/main/java/au/org/aodn/ogcapi/server/core/service/geonetwork/Geonetwork.java index 6f624b75..06ea1c32 100644 --- a/server/src/main/java/au/org/aodn/ogcapi/server/core/service/geonetwork/Geonetwork.java +++ b/server/src/main/java/au/org/aodn/ogcapi/server/core/service/geonetwork/Geonetwork.java @@ -3,7 +3,6 @@ import au.org.aodn.ogcapi.server.core.service.ApplicationInfo; import org.springframework.web.client.RestTemplate; -import java.util.Collections; import java.util.Map; /** @@ -11,27 +10,14 @@ */ public class Geonetwork implements ApplicationInfo { - protected final Map> appInfo; + protected final InfoCache appInfo; public Geonetwork(GNProperties properties, RestTemplate template) { - this.appInfo = queryInfo(template, properties.host(), properties.infoPath()); + this.appInfo = new InfoCache(() -> queryInfo(template, properties.host(), properties.infoPath())); } @Override - public String getName() { - Object name = this.appInfo.getOrDefault("application", Collections.emptyMap()).getOrDefault("name", null); - return name != null ? name.toString() : null; - } - - @Override - public String getVersion() { - Object version = this.appInfo.getOrDefault("application", Collections.emptyMap()).getOrDefault("version", null); - return version != null ? version.toString() : null; - } - - @Override - public String getDescription() { - Object description = this.appInfo.getOrDefault("application", Collections.emptyMap()).getOrDefault("description", null); - return description != null ? description.toString() : null; + public Map> getAppInfo() { + return appInfo.get(); } } diff --git a/server/src/main/java/au/org/aodn/ogcapi/server/core/service/indexer/EsIndexer.java b/server/src/main/java/au/org/aodn/ogcapi/server/core/service/indexer/EsIndexer.java index c9723963..ddee345b 100644 --- a/server/src/main/java/au/org/aodn/ogcapi/server/core/service/indexer/EsIndexer.java +++ b/server/src/main/java/au/org/aodn/ogcapi/server/core/service/indexer/EsIndexer.java @@ -3,32 +3,18 @@ import au.org.aodn.ogcapi.server.core.service.ApplicationInfo; import org.springframework.web.client.RestTemplate; -import java.util.Collections; import java.util.Map; public class EsIndexer implements ApplicationInfo { - protected final Map> appInfo; + protected final InfoCache appInfo; public EsIndexer(IndexerProperties properties, RestTemplate template) { - this.appInfo = queryInfo(template, properties.host(), properties.infoPath()); + this.appInfo = new InfoCache(() -> queryInfo(template, properties.host(), properties.infoPath())); } @Override - public String getName() { - Object name = this.appInfo.getOrDefault("application", Collections.emptyMap()).getOrDefault("name", null); - return name != null ? name.toString() : null; - } - - @Override - public String getVersion() { - Object version = this.appInfo.getOrDefault("application", Collections.emptyMap()).getOrDefault("version", null); - return version != null ? version.toString() : null; - } - - @Override - public String getDescription() { - Object description = this.appInfo.getOrDefault("application", Collections.emptyMap()).getOrDefault("description", null); - return description != null ? description.toString() : null; + public Map> getAppInfo() { + return appInfo.get(); } } diff --git a/server/src/test/java/au/org/aodn/ogcapi/server/core/configuration/ConfigTest.java b/server/src/test/java/au/org/aodn/ogcapi/server/core/configuration/ConfigTest.java index 6b384e4b..4c6d8eec 100644 --- a/server/src/test/java/au/org/aodn/ogcapi/server/core/configuration/ConfigTest.java +++ b/server/src/test/java/au/org/aodn/ogcapi/server/core/configuration/ConfigTest.java @@ -8,8 +8,10 @@ import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; import org.springframework.http.client.ClientHttpRequestInterceptor; +import org.springframework.http.client.SimpleClientHttpRequestFactory; import org.springframework.mock.http.client.MockClientHttpRequest; import org.springframework.mock.http.client.MockClientHttpResponse; +import org.springframework.test.util.ReflectionTestUtils; import org.springframework.web.client.RestTemplate; import java.io.IOException; @@ -100,4 +102,20 @@ public void testInternalSecretOmittedWhenNotConfigured() throws IOException { assertEquals("test-secret", headers.getFirst("X-API-KEY")); assertNull(headers.getFirst("x-internal-das-header-secret")); } + + @Test + public void testDefaultTemplateHasShortTimeout() { + SimpleClientHttpRequestFactory factory = (SimpleClientHttpRequestFactory) config.createRestTemplate().getRequestFactory(); + + assertEquals(10000, ReflectionTestUtils.getField(factory, "connectTimeout")); + assertEquals(60000, ReflectionTestUtils.getField(factory, "readTimeout")); + } + + @Test + public void testDownloadTemplateHasLongTimeout() { + SimpleClientHttpRequestFactory factory = (SimpleClientHttpRequestFactory) config.createDownloadRestTemplate().getRequestFactory(); + + assertEquals(1200000, ReflectionTestUtils.getField(factory, "connectTimeout")); + assertEquals(1200000, ReflectionTestUtils.getField(factory, "readTimeout")); + } } diff --git a/server/src/test/java/au/org/aodn/ogcapi/server/core/service/ApplicationInfoTest.java b/server/src/test/java/au/org/aodn/ogcapi/server/core/service/ApplicationInfoTest.java new file mode 100644 index 00000000..1ac6f5bf --- /dev/null +++ b/server/src/test/java/au/org/aodn/ogcapi/server/core/service/ApplicationInfoTest.java @@ -0,0 +1,79 @@ +package au.org.aodn.ogcapi.server.core.service; + +import au.org.aodn.ogcapi.server.core.service.dda.DdaProperties; +import au.org.aodn.ogcapi.server.core.service.dda.DdaService; +import org.junit.jupiter.api.Test; +import org.mockito.stubbing.OngoingStubbing; +import org.springframework.core.ParameterizedTypeReference; +import org.springframework.http.HttpMethod; +import org.springframework.http.ResponseEntity; +import org.springframework.web.client.ResourceAccessException; +import org.springframework.web.client.RestTemplate; + +import java.util.Map; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; + +public class ApplicationInfoTest { + + private static final DdaProperties PROPERTIES = new DdaProperties("http://dda", "api/v1/ml/manage/info"); + + private static final Map> INFO = Map.of( + "application", Map.of("name", "dda", "version", "1.0", "description", "desc")); + + @SuppressWarnings("unchecked") + private static OngoingStubbing> whenExchange(RestTemplate template) { + return when(template.exchange(anyString(), eq(HttpMethod.GET), any(), any(ParameterizedTypeReference.class))); + } + + @Test + public void testNoCallAtStartup() { + RestTemplate template = mock(RestTemplate.class); + + new DdaService(PROPERTIES, template); + + verifyNoInteractions(template); + } + + @Test + public void testServiceDownReturnNull() { + RestTemplate template = mock(RestTemplate.class); + whenExchange(template).thenThrow(new ResourceAccessException("Connect timed out")); + + DdaService service = new DdaService(PROPERTIES, template); + + assertNull(service.getName()); + assertNull(service.getVersion()); + assertNull(service.getDescription()); + } + + @Test + @SuppressWarnings("unchecked") + public void testRetryAfterFailAndCacheAfterSuccess() { + RestTemplate template = mock(RestTemplate.class); + whenExchange(template) + .thenThrow(new ResourceAccessException("Connect timed out")) + .thenReturn(ResponseEntity.ok(INFO)); + + DdaService service = new DdaService(PROPERTIES, template); + + // 1. Service down, nothing cached + assertNull(service.getName()); + // 2. Service up, info loaded + assertEquals("dda", service.getName()); + // 3. Info from cache, no more call + assertEquals("1.0", service.getVersion()); + assertEquals("desc", service.getDescription()); + + verify(template, times(2)).exchange(anyString(), eq(HttpMethod.GET), any(), any(ParameterizedTypeReference.class)); + } +}