Skip to content

Commit

Permalink
Merge pull request #99 from aodn/bugs/5953-report-count-not-correct
Browse files Browse the repository at this point in the history 
Fix bug on count report is off
  • Loading branch information
@vietnguyengit
vietnguyengit authored Dec 4, 2024
2 parents 683d368 + 84ff276 commit 26e8500
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 17 deletions.
16 changes: 12 additions & 4 deletions server/src/main/java/au/org/aodn/ogcapi/server/core/service/ElasticSearchBase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
import co.elastic.clients.elasticsearch._types.query_dsl.BoolQuery;
import co.elastic.clients.elasticsearch._types.query_dsl.Query;
import co.elastic.clients.elasticsearch._types.query_dsl.QueryBuilders;
import co.elastic.clients.elasticsearch.core.CountRequest;
import co.elastic.clients.elasticsearch.core.CountResponse;
import co.elastic.clients.elasticsearch.core.SearchRequest;
import co.elastic.clients.elasticsearch.core.SearchResponse;
import co.elastic.clients.elasticsearch.core.search.Hit;
Expand Down Expand Up @@ -224,16 +226,22 @@ protected SearchResult searchCollectionBy(final List<Query> queries,
}
/**
* Count the total number hit. There are two ways to get the total, one is use search but set the size to 0,
* then it will fill the size with total.
* then it will fill the size with total, but due to paging it will only return the max of search return say 10000
* will be the max, even you have more then 1000 records, hence we need to use count request
*
* @param requestBuilder - A query builder, this make the function general count given a query
* @return - The number of record that matches the requestBuilder
*/
protected Long countRecordsHit(Supplier<SearchRequest.Builder> requestBuilder) {
try {
SearchRequest sr = requestBuilder.get().size(0).build();
SearchResponse<ObjectNode> response = esClient.search(sr, ObjectNode.class);
return (response.hits().total() != null) ? response.hits().total().value() : null;
// The query need to be the same as the content, otherwise the total count report will be off.
CountRequest cr = CountRequest.of(f -> f
.query(requestBuilder.get().build().query())
.index(indexName)
);
log.debug("Elastic search payload for count {}", cr.toString());
CountResponse response = esClient.count(cr);
return (response != null) ? response.count() : null;
}
catch (IOException e) {
return null;
Expand Down
28 changes: 15 additions & 13 deletions server/src/test/java/au/org/aodn/ogcapi/server/common/RestApiTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ public void verifyClusterIsHealthy() throws IOException {
super.assertClusterHealthResponse();
}
/**
* The search is a fuzzy search based on title and description and few param field. So you expect 1 hit only
* The search is a fuzzy search based on title and description and few param field. So you expect 1 hit only,
* we check via getTotal() which is value from another elastic search.
* @throws IOException - IO Exception
*/
@Test
Expand All @@ -70,23 +71,23 @@ public void verifyApiCollectionsQueryOnText1() throws IOException {
);

// Call rest api directly and get query result
ResponseEntity<Collections> collections = testRestTemplate.getForEntity(getBasePath() + "/collections?q=reproduction", Collections.class);
assertEquals(1, Objects.requireNonNull(collections.getBody()).getCollections().size(), "Only 1 hit");
ResponseEntity<ExtendedCollections> collections = testRestTemplate.getForEntity(getBasePath() + "/collections?q=reproduction", ExtendedCollections.class);
assertEquals(1, Objects.requireNonNull(collections.getBody()).getTotal(), "Only 1 hit");
assertEquals(
"516811d7-cd1e-207a-e0440003ba8c79dd",
collections.getBody().getCollections().get(0).getId(),
"Correct UUID - 516811d7-cd1e-207a-e0440003ba8c79dd");

// This time we make a typo but we should still get the result back as Fuzzy search
collections = testRestTemplate.getForEntity(getBasePath() + "/collections?q=temperatura", Collections.class);
assertEquals(1, Objects.requireNonNull(collections.getBody()).getCollections().size(), "Only 1 hit");
collections = testRestTemplate.getForEntity(getBasePath() + "/collections?q=temperatura", ExtendedCollections.class);
assertEquals(1, Objects.requireNonNull(collections.getBody()).getTotal(), "Only 1 hit");
assertEquals(
"7709f541-fc0c-4318-b5b9-9053aa474e0e",
collections.getBody().getCollections().get(0).getId(),
"Correct UUID - 7709f541-fc0c-4318-b5b9-9053aa474e0e");

collections = testRestTemplate.getForEntity(getBasePath() + "/collections?q=temperatura,reproduction", Collections.class);
assertEquals(2, Objects.requireNonNull(collections.getBody()).getCollections().size(), "hit 2");
collections = testRestTemplate.getForEntity(getBasePath() + "/collections?q=temperatura,reproduction", ExtendedCollections.class);
assertEquals(2, Objects.requireNonNull(collections.getBody()).getTotal(), "hit 2");
assertEquals(
"516811d7-cd1e-207a-e0440003ba8c79dd",
collections.getBody().getCollections().get(0).getId(),
Expand All @@ -98,7 +99,8 @@ public void verifyApiCollectionsQueryOnText1() throws IOException {
}
/**
* The search is a fuzzy search based on title and description and few param field. This test add one more text
* which should hit the organization, paramter or vocab field
* which should hit the organization, paramter or vocab field. We use getTotal() instead of count the collection list
* therefore we can verify this function call too. The getTotal() value comes from elastic count CountRequest.
* 516811d7-cd1e-207a-e0440003ba8c79dd - Have repoduction
* 073fde5a-bff3-1c1f-e053-08114f8c5588 - Nothing match (although the word 'and' will match, but we use AND operator in fuzzy match so it will not count)
* 9fdb1eee-bc28-43a9-88c5-972324784837 - Contains 'precipitation and evaporation' in parameter_vocabs
Expand All @@ -114,8 +116,8 @@ public void verifyApiCollectionsQueryOnText2() throws IOException {
);

// Call rest api directly and get query result
ResponseEntity<Collections> collections = testRestTemplate.getForEntity(getBasePath() + "/collections?q=reproduction,precipitation and evaporation", Collections.class);
assertEquals(2, Objects.requireNonNull(collections.getBody()).getCollections().size(), "Only 2 hit");
ResponseEntity<ExtendedCollections> collections = testRestTemplate.getForEntity(getBasePath() + "/collections?q=reproduction,precipitation and evaporation", ExtendedCollections.class);
assertEquals(2, Objects.requireNonNull(collections.getBody()).getTotal(), "Only 2 hit");
assertEquals(
"516811d7-cd1e-207a-e0440003ba8c79dd",
collections.getBody().getCollections().get(0).getId(),
Expand All @@ -137,23 +139,23 @@ public void verifyDateTimeAfterBounds() throws IOException {
"5c418118-2581-4936-b6fd-d6bedfe74f62.json"
);

ResponseEntity<Collections> collections = testRestTemplate.getForEntity(getBasePath() + "/collections?datetime=1994-02-16T13:00:00Z/..", Collections.class);
ResponseEntity<ExtendedCollections> collections = testRestTemplate.getForEntity(getBasePath() + "/collections?datetime=1994-02-16T13:00:00Z/..", ExtendedCollections.class);
assertEquals(1, Objects.requireNonNull(collections.getBody()).getCollections().size(), "hit 1 because 1 document do not have start date");
assertEquals(
"5c418118-2581-4936-b6fd-d6bedfe74f62",
collections.getBody().getCollections().get(0).getId(),
"Correct UUID - 5c418118-2581-4936-b6fd-d6bedfe74f62");

// The syntax slightly diff but they are the same / = /.. the time is slightly off with one of the record so still get 1
collections = testRestTemplate.getForEntity(getBasePath() + "/collections?datetime=1870-07-16T15:10:44Z/", Collections.class);
collections = testRestTemplate.getForEntity(getBasePath() + "/collections?datetime=1870-07-16T15:10:44Z/", ExtendedCollections.class);
assertEquals(1, Objects.requireNonNull(collections.getBody()).getCollections().size(), "hit 1 because 1 document do not have start date");
assertEquals(
"5c418118-2581-4936-b6fd-d6bedfe74f62",
collections.getBody().getCollections().get(0).getId(),
"Correct UUID - 5c418118-2581-4936-b6fd-d6bedfe74f62");

// The syntax slightly diff but they are the same / = /.. the time is slightly off with one of the record so still get 1
collections = testRestTemplate.getForEntity(getBasePath() + "/collections?datetime=1870-07-16T14:10:44Z/", Collections.class);
collections = testRestTemplate.getForEntity(getBasePath() + "/collections?datetime=1870-07-16T14:10:44Z/", ExtendedCollections.class);
assertEquals(2, Objects.requireNonNull(collections.getBody()).getCollections().size(), "hit 2");
assertEquals(
"5c418118-2581-4936-b6fd-d6bedfe74f62",
Expand Down

0 comments on commit 26e8500

Please # to comment.