Skip to content

Fix bug on count report is off #99

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
vietnguyengit merged 1 commit into from
Dec 4, 2024
Merged

Fix bug on count report is off #99

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions server/src/main/java/au/org/aodn/ogcapi/server/core/service/ElasticSearchBase.java
View file
Open in desktop
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
Open in desktop
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
Loading