From 0b5aa92a489971e10dc2525d4bd9ce3863acf260 Mon Sep 17 00:00:00 2001 From: amber Date: Wed, 16 Sep 2026 14:30:08 +1000 Subject: [PATCH 1/2] fix(collections): honour limit, load batches up to cap, survive 1 cpu --- Dockerfile | 3 + .../aodn/ogcapi/server/common/RestApi.java | 3 + .../core/service/ElasticSearchBase.java | 21 ++-- .../server/core/service/OGCApiService.java | 22 +++++ .../server/core/util/GeometryUtils.java | 4 +- .../core/service/OGCApiServiceTest.java | 26 +++++ .../ogcapi/server/features/RestApiTest.java | 97 +++++++++++++++++++ 7 files changed, 167 insertions(+), 9 deletions(-) diff --git a/Dockerfile b/Dockerfile index 80374059..4eb5fdad 100644 --- a/Dockerfile +++ b/Dockerfile @@ -6,6 +6,9 @@ ENV MAX_HEAP_PERCENTAGE=70 COPY ./server/target/ogcapi-java-server-*-exec.jar app.jar ENTRYPOINT ["/bin/sh", "-c", "java \ -XX:MaxRAMPercentage=${MAX_HEAP_PERCENTAGE} \ + -XX:+ExitOnOutOfMemoryError \ + -XX:+HeapDumpOnOutOfMemoryError \ + -XX:HeapDumpPath=/tmp \ -Duser.timezone=UTC \ -Delasticsearch.index.name=${INDEX_NAME} \ -Delasticsearch.cloud_optimized_index.name=${CO_INDEX_NAME} \ diff --git a/server/src/main/java/au/org/aodn/ogcapi/server/common/RestApi.java b/server/src/main/java/au/org/aodn/ogcapi/server/common/RestApi.java index 316c7276..0726f123 100644 --- a/server/src/main/java/au/org/aodn/ogcapi/server/common/RestApi.java +++ b/server/src/main/java/au/org/aodn/ogcapi/server/common/RestApi.java @@ -149,6 +149,8 @@ public ResponseEntity getCollections( @RequestParam(value = "crs", required = false, defaultValue = "https://epsg.io/4326") String crs, @Parameter(in = ParameterIn.QUERY, description = "Filter expression") @RequestParam(value = "filter", required = false) String filter, + @Parameter(in = ParameterIn.QUERY, description = "Max number of collections in the response, 1..10000, default 10") + @RequestParam(value = "limit", required = false) Integer limit, @Size(min=1) @Parameter(in = ParameterIn.QUERY, description = "Sort by, property needs to valid in the CQL" ,schema=@Schema()) @Valid @RequestParam(value = "sortby", required = false, defaultValue = "-score,-rank") String sortBy) { @@ -162,6 +164,7 @@ public ResponseEntity getCollections( // the same, we append the bbox parameter to the filter in case user use this parameter filter = OGCApiService.processBBoxParameter(CQLFields.geometry.name(), bbox, filter); } + filter = OGCApiService.processLimitParameter(limit, filter); return commonService.getCollectionList( q, filter, diff --git a/server/src/main/java/au/org/aodn/ogcapi/server/core/service/ElasticSearchBase.java b/server/src/main/java/au/org/aodn/ogcapi/server/core/service/ElasticSearchBase.java index c7373599..25acc712 100644 --- a/server/src/main/java/au/org/aodn/ogcapi/server/core/service/ElasticSearchBase.java +++ b/server/src/main/java/au/org/aodn/ogcapi/server/core/service/ElasticSearchBase.java @@ -354,12 +354,12 @@ protected SearchResult searchCollectionBy(final List final Double score, final Long maxSize) { Supplier builderSupplier = buildCollectionSearchRequestSupplier( - queries, should, filters, properties, searchAfter, sortOptions, score, maxSize + queries, should, filters, properties, null, sortOptions, score, maxSize ); try { log.info("Start search {} {}", ZonedDateTime.now(), Thread.currentThread().getName()); - Iterable> response = pageableSearch(builderSupplier, ObjectNode.class, maxSize); + Iterable> response = pageableSearch(builderSupplier, ObjectNode.class, maxSize, searchAfter); SearchResult result = new SearchResult<>(); result.collections = new ArrayList<>(); @@ -515,12 +515,19 @@ protected Long countRecordsHit(Supplier requestBuilder) { * * @param requestBuilder, assume it is sorted with order, what order isn't important, as long as it is sorted * @param clazz - The type + * @param maxSize - Stop after this many hits, null means every hit + * @param searchAfter - Cursor for the first batch, later batches use the last hit's sort values. The supplier + * must not set it because the builder appends search_after values instead of replacing them * @return - The items that matches the query mentioned in the requestBuilder * @param A generic type for Elastic query */ - protected Iterable> pageableSearch(Supplier requestBuilder, Class clazz, Long maxSize) { + protected Iterable> pageableSearch(Supplier requestBuilder, Class clazz, Long maxSize, List searchAfter) { try { - SearchRequest sr = requestBuilder.get().build(); + SearchRequest.Builder first = requestBuilder.get(); + if(searchAfter != null) { + first.searchAfter(searchAfter); + } + SearchRequest sr = first.build(); log.debug("Final elastic search payload {}", sr); final AtomicLong count = new AtomicLong(0); @@ -533,9 +540,9 @@ protected Iterable> pageableSearch(Supplier re @Override public boolean hasNext() { - // No need continue if we already hit the end - if(maxSize != null) { - return count.get() < maxSize; + // Stop once the caller's cap is reached, otherwise keep loading batches below + if(maxSize != null && count.get() >= maxSize) { + return false; } // If we hit the end, that means we have iterated to end of page. if (index < response.get().hits().hits().size()) { diff --git a/server/src/main/java/au/org/aodn/ogcapi/server/core/service/OGCApiService.java b/server/src/main/java/au/org/aodn/ogcapi/server/core/service/OGCApiService.java index b96b7d5b..80c6e1d0 100644 --- a/server/src/main/java/au/org/aodn/ogcapi/server/core/service/OGCApiService.java +++ b/server/src/main/java/au/org/aodn/ogcapi/server/core/service/OGCApiService.java @@ -2,6 +2,7 @@ import au.org.aodn.ogcapi.features.model.FeatureCollectionGeoJSON; import au.org.aodn.ogcapi.server.core.exception.CustomException; +import au.org.aodn.ogcapi.server.core.exception.InvalidParameterException; import au.org.aodn.stac.model.StacCollectionModel; import au.org.aodn.ogcapi.server.core.model.enumeration.CQLCrsType; import au.org.aodn.ogcapi.server.core.model.enumeration.FeatureId; @@ -20,6 +21,7 @@ import java.math.BigDecimal; import java.util.List; import java.util.function.BiFunction; +import java.util.regex.Pattern; /** * @@ -28,6 +30,11 @@ public abstract class OGCApiService { protected Logger logger = LoggerFactory.getLogger(RestApi.class); + // OGC API Records limit parameter: default and maximum from the bundled spec + public static final int DEFAULT_LIMIT = 10; + public static final int MAX_LIMIT = 10000; + protected static final Pattern PAGE_SIZE_IN_FILTER = Pattern.compile("\\bpage_size\\s*=", Pattern.CASE_INSENSITIVE); + @Autowired protected Search search; @@ -174,4 +181,19 @@ else if(bbox.size() == 6) { return String.join(" AND ", filter, f); } } + + /** + * Rewrite limit as CQL page_size. limit wins over page_size in the filter, without either use the spec default. + */ + public static String processLimitParameter(Integer limit, String filter) { + if (limit != null && (limit < 1 || limit > MAX_LIMIT)) { + throw new InvalidParameterException("limit must be between 1 and " + MAX_LIMIT); + } + boolean filterHasPageSize = filter != null && PAGE_SIZE_IN_FILTER.matcher(filter).find(); + if (limit == null && filterHasPageSize) { + return filter; + } + String f = "page_size=" + (limit != null ? limit : DEFAULT_LIMIT); + return (filter == null || filter.isBlank()) ? f : String.join(" AND ", filter, f); + } } diff --git a/server/src/main/java/au/org/aodn/ogcapi/server/core/util/GeometryUtils.java b/server/src/main/java/au/org/aodn/ogcapi/server/core/util/GeometryUtils.java index 2d2a930a..b57f576a 100644 --- a/server/src/main/java/au/org/aodn/ogcapi/server/core/util/GeometryUtils.java +++ b/server/src/main/java/au/org/aodn/ogcapi/server/core/util/GeometryUtils.java @@ -54,10 +54,10 @@ public class GeometryUtils { @Setter protected static int centroidScale = 5; - // Create an ExecutorService with a fixed thread pool size + // Create an ExecutorService with a fixed thread pool size, at least 1 so a 1 cpu container can still start @Getter @Setter - protected static ExecutorService executorService = Executors.newFixedThreadPool(Runtime.getRuntime().availableProcessors() - 1); + protected static ExecutorService executorService = Executors.newFixedThreadPool(Math.max(1, Runtime.getRuntime().availableProcessors() - 1)); protected static Logger logger = LoggerFactory.getLogger(GeometryUtils.class); diff --git a/server/src/test/java/au/org/aodn/ogcapi/server/core/service/OGCApiServiceTest.java b/server/src/test/java/au/org/aodn/ogcapi/server/core/service/OGCApiServiceTest.java index 162e6584..9bf7fe0f 100644 --- a/server/src/test/java/au/org/aodn/ogcapi/server/core/service/OGCApiServiceTest.java +++ b/server/src/test/java/au/org/aodn/ogcapi/server/core/service/OGCApiServiceTest.java @@ -1,8 +1,10 @@ package au.org.aodn.ogcapi.server.core.service; +import au.org.aodn.ogcapi.server.core.exception.InvalidParameterException; import au.org.aodn.ogcapi.server.core.model.enumeration.CQLFields; import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; public class OGCApiServiceTest { @@ -31,4 +33,28 @@ public void verifyProcessDatetimeParameter() { o = OGCApiService.processDatetimeParameter(CQLFields.temporal.name(),"/2021-10-10", "geometry is null"); assertEquals( "geometry is null AND temporal before 2021-10-10", o, "Before plus filter incorrect1"); } + + /** + * limit becomes a CQL page_size appended last, so it wins over any page_size already in the filter. + */ + @Test + public void verifyProcessLimitParameter() { + String o = OGCApiService.processLimitParameter(null, null); + assertEquals("page_size=10", o, "Spec default when nothing given"); + + o = OGCApiService.processLimitParameter(null, "page_size=3"); + assertEquals("page_size=3", o, "Filter page_size kept when no limit"); + + o = OGCApiService.processLimitParameter(null, "temporal after 2021-10-10"); + assertEquals("temporal after 2021-10-10 AND page_size=10", o, "Default appended to filter"); + + o = OGCApiService.processLimitParameter(5, null); + assertEquals("page_size=5", o, "Limit alone"); + + o = OGCApiService.processLimitParameter(2, "page_size=3"); + assertEquals("page_size=3 AND page_size=2", o, "Limit appended last so it overrides"); + + assertThrows(InvalidParameterException.class, () -> OGCApiService.processLimitParameter(0, null), "Below 1 rejected"); + assertThrows(InvalidParameterException.class, () -> OGCApiService.processLimitParameter(10001, null), "Above max rejected"); + } } diff --git a/server/src/test/java/au/org/aodn/ogcapi/server/features/RestApiTest.java b/server/src/test/java/au/org/aodn/ogcapi/server/features/RestApiTest.java index 5ea97cf7..97a64f60 100644 --- a/server/src/test/java/au/org/aodn/ogcapi/server/features/RestApiTest.java +++ b/server/src/test/java/au/org/aodn/ogcapi/server/features/RestApiTest.java @@ -498,4 +498,101 @@ public void verifyBBoxCorrect() throws IOException { assertEquals(154.0, bbox.get(0).get(2).doubleValue(), "Overall bounding box coor 3"); assertEquals(-9.0, bbox.get(0).get(3).doubleValue(), "Overall bounding box coor 4"); } + + /** + * limit caps the collections in one response the same way filter=page_size does + */ + @Test + public void verifyLimitCapsCollections() throws IOException { + super.insertJsonToElasticRecordIndex( + "5c418118-2581-4936-b6fd-d6bedfe74f62.json", + "19da2ce7-138f-4427-89de-a50c724f5f54.json", + "516811d7-cd1e-207a-e0440003ba8c79dd.json", + "7709f541-fc0c-4318-b5b9-9053aa474e0e.json", + "bc55eff4-7596-3565-e044-00144fdd4fa6.json", + "bf287dfe-9ce4-4969-9c59-51c39ea4d011.json"); + + ResponseEntity collections = testRestTemplate.exchange( + getBasePath() + "/collections?limit=1", + HttpMethod.GET, + null, + new ParameterizedTypeReference<>() { + }); + + assertEquals(HttpStatus.OK, collections.getStatusCode(), "Get status OK"); + assertEquals(1, Objects.requireNonNull(collections.getBody()).getCollections().size(), "limit=1 returns one record"); + assertEquals(6, collections.getBody().getTotal(), "Total still counts every record"); + assertEquals(3, collections.getBody().getSearchAfter().size(), "search_after given for the next page"); + } + + /** + * limit larger than one elastic batch keeps loading batches until the limit, test batch size is 4 + */ + @Test + public void verifyLimitSpansElasticBatches() throws IOException { + assertEquals(4, pageSize, "This test only works with small page"); + + super.insertJsonToElasticRecordIndex( + "5c418118-2581-4936-b6fd-d6bedfe74f62.json", + "19da2ce7-138f-4427-89de-a50c724f5f54.json", + "516811d7-cd1e-207a-e0440003ba8c79dd.json", + "7709f541-fc0c-4318-b5b9-9053aa474e0e.json", + "bc55eff4-7596-3565-e044-00144fdd4fa6.json", + "bf287dfe-9ce4-4969-9c59-51c39ea4d011.json"); + + ResponseEntity collections = testRestTemplate.exchange( + getBasePath() + "/collections?limit=5", + HttpMethod.GET, + null, + new ParameterizedTypeReference<>() { + }); + + assertEquals(HttpStatus.OK, collections.getStatusCode(), "Get status OK"); + assertEquals(5, Objects.requireNonNull(collections.getBody()).getCollections().size(), "limit=5 spans two batches of 4"); + assertEquals(6, collections.getBody().getTotal(), "Total still counts every record"); + } + + /** + * An explicit limit wins over page_size in the filter + */ + @Test + public void verifyLimitOverridesFilterPageSize() throws IOException { + super.insertJsonToElasticRecordIndex( + "5c418118-2581-4936-b6fd-d6bedfe74f62.json", + "19da2ce7-138f-4427-89de-a50c724f5f54.json", + "516811d7-cd1e-207a-e0440003ba8c79dd.json", + "7709f541-fc0c-4318-b5b9-9053aa474e0e.json", + "bc55eff4-7596-3565-e044-00144fdd4fa6.json", + "bf287dfe-9ce4-4969-9c59-51c39ea4d011.json"); + + ResponseEntity collections = testRestTemplate.exchange( + getBasePath() + "/collections?limit=2&filter=page_size=3", + HttpMethod.GET, + null, + new ParameterizedTypeReference<>() { + }); + + assertEquals(HttpStatus.OK, collections.getStatusCode(), "Get status OK"); + assertEquals(2, Objects.requireNonNull(collections.getBody()).getCollections().size(), "limit wins over page_size"); + } + + /** + * limit outside 1..10000 is rejected + */ + @Test + public void verifyLimitOutOfRangeRejected() { + ResponseEntity response = testRestTemplate.exchange( + getBasePath() + "/collections?limit=0", + HttpMethod.GET, + null, + String.class); + assertEquals(HttpStatus.BAD_REQUEST, response.getStatusCode(), "limit=0 rejected"); + + response = testRestTemplate.exchange( + getBasePath() + "/collections?limit=10001", + HttpMethod.GET, + null, + String.class); + assertEquals(HttpStatus.BAD_REQUEST, response.getStatusCode(), "limit above max rejected"); + } } From e26d13acbb1b3d7a4372affddc211f9c7eea807c Mon Sep 17 00:00:00 2001 From: amber Date: Wed, 16 Sep 2026 15:51:20 +1000 Subject: [PATCH 2/2] Default page_size via config instead of a limit param, batch size 800 --- .../aodn/ogcapi/server/common/RestApi.java | 8 ++- .../configuration/ElasticSearchConfig.java | 2 +- .../server/core/service/OGCApiService.java | 16 ++--- server/src/main/resources/application.yaml | 7 +- .../org/aodn/ogcapi/server/BaseTestClass.java | 1 + .../core/service/OGCApiServiceTest.java | 25 ++----- .../ogcapi/server/features/RestApiTest.java | 71 +++++-------------- .../src/test/resources/application-test.yaml | 2 + 8 files changed, 42 insertions(+), 90 deletions(-) diff --git a/server/src/main/java/au/org/aodn/ogcapi/server/common/RestApi.java b/server/src/main/java/au/org/aodn/ogcapi/server/common/RestApi.java index 0726f123..ee6d5802 100644 --- a/server/src/main/java/au/org/aodn/ogcapi/server/common/RestApi.java +++ b/server/src/main/java/au/org/aodn/ogcapi/server/common/RestApi.java @@ -26,6 +26,7 @@ import org.apache.commons.lang3.NotImplementedException; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; +import org.springframework.beans.factory.annotation.Value; import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; @@ -52,6 +53,9 @@ public class RestApi implements ApiApi, DefaultApi, ConformanceApi { @Qualifier("FeaturesRestService") protected OGCApiService featuresService; + @Value("${ogcapi.collections.default-page-size:800}") + protected int defaultPageSize; + @Autowired protected StacToCollections stacToCollection; @@ -149,8 +153,6 @@ public ResponseEntity getCollections( @RequestParam(value = "crs", required = false, defaultValue = "https://epsg.io/4326") String crs, @Parameter(in = ParameterIn.QUERY, description = "Filter expression") @RequestParam(value = "filter", required = false) String filter, - @Parameter(in = ParameterIn.QUERY, description = "Max number of collections in the response, 1..10000, default 10") - @RequestParam(value = "limit", required = false) Integer limit, @Size(min=1) @Parameter(in = ParameterIn.QUERY, description = "Sort by, property needs to valid in the CQL" ,schema=@Schema()) @Valid @RequestParam(value = "sortby", required = false, defaultValue = "-score,-rank") String sortBy) { @@ -164,7 +166,7 @@ public ResponseEntity getCollections( // the same, we append the bbox parameter to the filter in case user use this parameter filter = OGCApiService.processBBoxParameter(CQLFields.geometry.name(), bbox, filter); } - filter = OGCApiService.processLimitParameter(limit, filter); + filter = OGCApiService.addDefaultPageSize(filter, defaultPageSize); return commonService.getCollectionList( q, filter, diff --git a/server/src/main/java/au/org/aodn/ogcapi/server/core/configuration/ElasticSearchConfig.java b/server/src/main/java/au/org/aodn/ogcapi/server/core/configuration/ElasticSearchConfig.java index 10934756..e151f80b 100644 --- a/server/src/main/java/au/org/aodn/ogcapi/server/core/configuration/ElasticSearchConfig.java +++ b/server/src/main/java/au/org/aodn/ogcapi/server/core/configuration/ElasticSearchConfig.java @@ -64,7 +64,7 @@ public Search createElasticSearch(ElasticsearchClient client, CacheNoLandGeometry cacheNoLandGeometry, ObjectMapper mapper, @Value("${elasticsearch.index.name}") String indexName, - @Value("${elasticsearch.index.pageSize:2200}") Integer pageSize, + @Value("${elasticsearch.index.pageSize:800}") Integer pageSize, @Value("${elasticsearch.index.lightweightPageSize:10000}") Integer lightweightPageSize, @Value("${elasticsearch.search_as_you_type.size:10}") Integer searchAsYouTypeSize) { diff --git a/server/src/main/java/au/org/aodn/ogcapi/server/core/service/OGCApiService.java b/server/src/main/java/au/org/aodn/ogcapi/server/core/service/OGCApiService.java index 80c6e1d0..a4371f6a 100644 --- a/server/src/main/java/au/org/aodn/ogcapi/server/core/service/OGCApiService.java +++ b/server/src/main/java/au/org/aodn/ogcapi/server/core/service/OGCApiService.java @@ -2,7 +2,6 @@ import au.org.aodn.ogcapi.features.model.FeatureCollectionGeoJSON; import au.org.aodn.ogcapi.server.core.exception.CustomException; -import au.org.aodn.ogcapi.server.core.exception.InvalidParameterException; import au.org.aodn.stac.model.StacCollectionModel; import au.org.aodn.ogcapi.server.core.model.enumeration.CQLCrsType; import au.org.aodn.ogcapi.server.core.model.enumeration.FeatureId; @@ -30,9 +29,6 @@ public abstract class OGCApiService { protected Logger logger = LoggerFactory.getLogger(RestApi.class); - // OGC API Records limit parameter: default and maximum from the bundled spec - public static final int DEFAULT_LIMIT = 10; - public static final int MAX_LIMIT = 10000; protected static final Pattern PAGE_SIZE_IN_FILTER = Pattern.compile("\\bpage_size\\s*=", Pattern.CASE_INSENSITIVE); @Autowired @@ -183,17 +179,13 @@ else if(bbox.size() == 6) { } /** - * Rewrite limit as CQL page_size. limit wins over page_size in the filter, without either use the spec default. + * Append the default page_size when the CQL filter has none, so a bare /collections never loads the whole index. */ - public static String processLimitParameter(Integer limit, String filter) { - if (limit != null && (limit < 1 || limit > MAX_LIMIT)) { - throw new InvalidParameterException("limit must be between 1 and " + MAX_LIMIT); - } - boolean filterHasPageSize = filter != null && PAGE_SIZE_IN_FILTER.matcher(filter).find(); - if (limit == null && filterHasPageSize) { + public static String addDefaultPageSize(String filter, int defaultPageSize) { + if (filter != null && PAGE_SIZE_IN_FILTER.matcher(filter).find()) { return filter; } - String f = "page_size=" + (limit != null ? limit : DEFAULT_LIMIT); + String f = "page_size=" + defaultPageSize; return (filter == null || filter.isBlank()) ? f : String.join(" AND ", filter, f); } } diff --git a/server/src/main/resources/application.yaml b/server/src/main/resources/application.yaml index 3c3e0ffc..e748936d 100644 --- a/server/src/main/resources/application.yaml +++ b/server/src/main/resources/application.yaml @@ -6,6 +6,9 @@ server: min-response-size: 1024 # Minimum response size in bytes to compress ogcapi: + collections: + # page_size applied when the CQL filter has none, the portal's largest page + default-page-size: 800 debug: elasticsearch-explain-enabled: false http-cache: @@ -21,8 +24,8 @@ ogcapi: elasticsearch: index: name: dev_portal_records - # search_after batch for full-document / geometry / links queries - pageSize: 2200 + # search_after batch for full-document / geometry / links queries, the portal's largest page + pageSize: 800 # search_after batch when properties is a small field list (id, temporal, title, ...) lightweightPageSize: 7000 vocabs_index: diff --git a/server/src/test/java/au/org/aodn/ogcapi/server/BaseTestClass.java b/server/src/test/java/au/org/aodn/ogcapi/server/BaseTestClass.java index b7b0ea60..8be004eb 100644 --- a/server/src/test/java/au/org/aodn/ogcapi/server/BaseTestClass.java +++ b/server/src/test/java/au/org/aodn/ogcapi/server/BaseTestClass.java @@ -232,6 +232,7 @@ protected void insertJsonToElasticIndex(String index, String[] filenames) throws // Check the number of doc store inside the ES instance is correct SearchRequest.Builder b = new SearchRequest.Builder() .index(index) + .size(filenames.length) // elastic returns 10 hits by default, tests may insert more files .query(QueryBuilders.matchAll().build()._toQuery()); SearchRequest request = b.build(); diff --git a/server/src/test/java/au/org/aodn/ogcapi/server/core/service/OGCApiServiceTest.java b/server/src/test/java/au/org/aodn/ogcapi/server/core/service/OGCApiServiceTest.java index 9bf7fe0f..f7b26190 100644 --- a/server/src/test/java/au/org/aodn/ogcapi/server/core/service/OGCApiServiceTest.java +++ b/server/src/test/java/au/org/aodn/ogcapi/server/core/service/OGCApiServiceTest.java @@ -1,10 +1,8 @@ package au.org.aodn.ogcapi.server.core.service; -import au.org.aodn.ogcapi.server.core.exception.InvalidParameterException; import au.org.aodn.ogcapi.server.core.model.enumeration.CQLFields; import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertThrows; public class OGCApiServiceTest { @@ -35,26 +33,17 @@ public void verifyProcessDatetimeParameter() { } /** - * limit becomes a CQL page_size appended last, so it wins over any page_size already in the filter. + * A default page_size is appended only when the CQL filter has none */ @Test - public void verifyProcessLimitParameter() { - String o = OGCApiService.processLimitParameter(null, null); - assertEquals("page_size=10", o, "Spec default when nothing given"); + public void verifyAddDefaultPageSize() { + String o = OGCApiService.addDefaultPageSize(null, 10); + assertEquals("page_size=10", o, "Default when no filter"); - o = OGCApiService.processLimitParameter(null, "page_size=3"); - assertEquals("page_size=3", o, "Filter page_size kept when no limit"); + o = OGCApiService.addDefaultPageSize("page_size=3", 10); + assertEquals("page_size=3", o, "Filter page_size kept"); - o = OGCApiService.processLimitParameter(null, "temporal after 2021-10-10"); + o = OGCApiService.addDefaultPageSize("temporal after 2021-10-10", 10); assertEquals("temporal after 2021-10-10 AND page_size=10", o, "Default appended to filter"); - - o = OGCApiService.processLimitParameter(5, null); - assertEquals("page_size=5", o, "Limit alone"); - - o = OGCApiService.processLimitParameter(2, "page_size=3"); - assertEquals("page_size=3 AND page_size=2", o, "Limit appended last so it overrides"); - - assertThrows(InvalidParameterException.class, () -> OGCApiService.processLimitParameter(0, null), "Below 1 rejected"); - assertThrows(InvalidParameterException.class, () -> OGCApiService.processLimitParameter(10001, null), "Above max rejected"); } } diff --git a/server/src/test/java/au/org/aodn/ogcapi/server/features/RestApiTest.java b/server/src/test/java/au/org/aodn/ogcapi/server/features/RestApiTest.java index 97a64f60..a4ef5e8d 100644 --- a/server/src/test/java/au/org/aodn/ogcapi/server/features/RestApiTest.java +++ b/server/src/test/java/au/org/aodn/ogcapi/server/features/RestApiTest.java @@ -500,36 +500,43 @@ public void verifyBBoxCorrect() throws IOException { } /** - * limit caps the collections in one response the same way filter=page_size does + * Without page_size in the filter the server applies the configured default, 10 in the test profile, + * so a bare call never loads the whole index */ @Test - public void verifyLimitCapsCollections() throws IOException { + public void verifyDefaultPageSizeCapsCollections() throws IOException { super.insertJsonToElasticRecordIndex( "5c418118-2581-4936-b6fd-d6bedfe74f62.json", "19da2ce7-138f-4427-89de-a50c724f5f54.json", "516811d7-cd1e-207a-e0440003ba8c79dd.json", "7709f541-fc0c-4318-b5b9-9053aa474e0e.json", "bc55eff4-7596-3565-e044-00144fdd4fa6.json", - "bf287dfe-9ce4-4969-9c59-51c39ea4d011.json"); + "bf287dfe-9ce4-4969-9c59-51c39ea4d011.json", + "073fde5a-bff3-1c1f-e053-08114f8c5588.json", + "ae86e2f5-eaaf-459e-a405-e654d85adb9c.json", + "caf7220a-19e0-4a7f-9af6-eade6c79a47a.json", + "b299cdcd-3dee-48aa-abdd-e0fcdbb9cadc.json", + "35234913-aa3c-48ec-b9a4-77f822f66ef8.json", + "e26d0a56-5603-4413-911d-7b359a533a75.json"); ResponseEntity collections = testRestTemplate.exchange( - getBasePath() + "/collections?limit=1", + getBasePath() + "/collections", HttpMethod.GET, null, new ParameterizedTypeReference<>() { }); assertEquals(HttpStatus.OK, collections.getStatusCode(), "Get status OK"); - assertEquals(1, Objects.requireNonNull(collections.getBody()).getCollections().size(), "limit=1 returns one record"); - assertEquals(6, collections.getBody().getTotal(), "Total still counts every record"); + assertEquals(10, Objects.requireNonNull(collections.getBody()).getCollections().size(), "Test profile default page_size is 10"); + assertEquals(12, collections.getBody().getTotal(), "Total still counts every record"); assertEquals(3, collections.getBody().getSearchAfter().size(), "search_after given for the next page"); } /** - * limit larger than one elastic batch keeps loading batches until the limit, test batch size is 4 + * page_size larger than one elastic batch keeps loading batches until page_size, test batch size is 4 */ @Test - public void verifyLimitSpansElasticBatches() throws IOException { + public void verifyPageSizeSpansElasticBatches() throws IOException { assertEquals(4, pageSize, "This test only works with small page"); super.insertJsonToElasticRecordIndex( @@ -541,58 +548,14 @@ public void verifyLimitSpansElasticBatches() throws IOException { "bf287dfe-9ce4-4969-9c59-51c39ea4d011.json"); ResponseEntity collections = testRestTemplate.exchange( - getBasePath() + "/collections?limit=5", + getBasePath() + "/collections?filter=page_size=5", HttpMethod.GET, null, new ParameterizedTypeReference<>() { }); assertEquals(HttpStatus.OK, collections.getStatusCode(), "Get status OK"); - assertEquals(5, Objects.requireNonNull(collections.getBody()).getCollections().size(), "limit=5 spans two batches of 4"); + assertEquals(5, Objects.requireNonNull(collections.getBody()).getCollections().size(), "page_size=5 spans two batches of 4"); assertEquals(6, collections.getBody().getTotal(), "Total still counts every record"); } - - /** - * An explicit limit wins over page_size in the filter - */ - @Test - public void verifyLimitOverridesFilterPageSize() throws IOException { - super.insertJsonToElasticRecordIndex( - "5c418118-2581-4936-b6fd-d6bedfe74f62.json", - "19da2ce7-138f-4427-89de-a50c724f5f54.json", - "516811d7-cd1e-207a-e0440003ba8c79dd.json", - "7709f541-fc0c-4318-b5b9-9053aa474e0e.json", - "bc55eff4-7596-3565-e044-00144fdd4fa6.json", - "bf287dfe-9ce4-4969-9c59-51c39ea4d011.json"); - - ResponseEntity collections = testRestTemplate.exchange( - getBasePath() + "/collections?limit=2&filter=page_size=3", - HttpMethod.GET, - null, - new ParameterizedTypeReference<>() { - }); - - assertEquals(HttpStatus.OK, collections.getStatusCode(), "Get status OK"); - assertEquals(2, Objects.requireNonNull(collections.getBody()).getCollections().size(), "limit wins over page_size"); - } - - /** - * limit outside 1..10000 is rejected - */ - @Test - public void verifyLimitOutOfRangeRejected() { - ResponseEntity response = testRestTemplate.exchange( - getBasePath() + "/collections?limit=0", - HttpMethod.GET, - null, - String.class); - assertEquals(HttpStatus.BAD_REQUEST, response.getStatusCode(), "limit=0 rejected"); - - response = testRestTemplate.exchange( - getBasePath() + "/collections?limit=10001", - HttpMethod.GET, - null, - String.class); - assertEquals(HttpStatus.BAD_REQUEST, response.getStatusCode(), "limit above max rejected"); - } } diff --git a/server/src/test/resources/application-test.yaml b/server/src/test/resources/application-test.yaml index 6b2db68d..927af7c8 100644 --- a/server/src/test/resources/application-test.yaml +++ b/server/src/test/resources/application-test.yaml @@ -1,4 +1,6 @@ ogcapi: + collections: + default-page-size: 10 docker: elasticVersion: "8.19.10" debug: