From e997ae066a705d96d6d10706b7b6d02dd4fe0ccf Mon Sep 17 00:00:00 2001
From: gcornut <guillaume.cornut@inra.fr>
Date: Mon, 13 May 2019 19:55:52 +0200
Subject: [PATCH 1/3] feat: Remove max result window limitation on BrAPI calls.
- Implement cached, deep paginated search using ES search_after
- Keep max result window on data discovery calls
- Simplify ESRequestFactory
---
.../api/brapi/v1/BrapiExceptionHandler.java | 8 +
.../faidare/v1/DataDiscoveryController.java | 3 +-
.../faidare/config/ElasticSearchConfig.java | 12 +
.../criteria/base/PaginationCriteriaImpl.java | 1 -
.../base/PaginationMaxResultValidator.java | 26 +-
.../domain/criteria/base/ValidPagination.java | 28 ---
.../criteria/DataDiscoveryCriteriaImpl.java | 2 +
.../elasticsearch/ESRequestFactory.java | 118 +++++----
.../elasticsearch/ESResponseParser.java | 17 ++
.../elasticsearch/ESScrollIterator.java | 14 +-
.../document/DocumentMetadata.java | 6 +
.../repository/impl/BaseESRepository.java | 2 +-
.../impl/ESGenericFindRepository.java | 225 +++++++++++++-----
.../impl/ESGenericGetByIdRepository.java | 36 +--
.../impl/ESGenericSuggestRepository.java | 10 +-
.../es/DataDiscoveryRepositoryImpl.java | 13 +-
.../es/GermplasmAttributeRepositoryImpl.java | 3 +-
.../es/GermplasmRepositoryImpl.java | 10 +-
.../base/MaxResultWindowPagination.java | 38 +++
.../brapi/v1/BrapiExceptionHandlerTest.java | 3 -
.../base/PaginationValidatorTest.java | 24 +-
.../elasticsearch/ESRequestFactoryTest.java | 2 +-
.../gnpis/v1/DataDiscoveryControllerTest.java | 53 +++++
23 files changed, 451 insertions(+), 203 deletions(-)
create mode 100644 backend/src/main/java/fr/inra/urgi/gpds/domain/criteria/base/MaxResultWindowPagination.java
create mode 100644 backend/src/test/java/fr/inra/urgi/gpds/api/gnpis/v1/DataDiscoveryControllerTest.java
diff --git a/backend/src/main/java/fr/inra/urgi/faidare/api/brapi/v1/BrapiExceptionHandler.java b/backend/src/main/java/fr/inra/urgi/faidare/api/brapi/v1/BrapiExceptionHandler.java
index 3d4f78b1..d30b90c7 100644
--- a/backend/src/main/java/fr/inra/urgi/faidare/api/brapi/v1/BrapiExceptionHandler.java
+++ b/backend/src/main/java/fr/inra/urgi/faidare/api/brapi/v1/BrapiExceptionHandler.java
@@ -6,6 +6,13 @@ import fr.inra.urgi.faidare.api.brapi.v1.exception.BrapiPaginationException;
import fr.inra.urgi.faidare.domain.brapi.v1.response.BrapiPagination;
import fr.inra.urgi.faidare.domain.brapi.v1.response.BrapiStatus;
import fr.inra.urgi.faidare.domain.response.ApiResponseFactory;
+import fr.inra.urgi.gpds.api.BadRequestException;
+import fr.inra.urgi.gpds.api.NotFoundException;
+import fr.inra.urgi.gpds.api.brapi.v1.exception.BrapiPaginationException;
+import fr.inra.urgi.gpds.domain.brapi.v1.response.BrapiPagination;
+import fr.inra.urgi.gpds.domain.brapi.v1.response.BrapiStatus;
+import fr.inra.urgi.gpds.domain.response.ApiResponseFactory;
+import fr.inra.urgi.gpds.elasticsearch.repository.impl.ESGenericFindRepository;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.validation.BindException;
@@ -103,6 +110,7 @@ public class BrapiExceptionHandler {
*/
@ExceptionHandler({
BadRequestException.class,
+ ESGenericFindRepository.PageOutOfBoundException.class,
BindException.class,
MethodArgumentNotValidException.class,
HttpMediaTypeNotSupportedException.class,
diff --git a/backend/src/main/java/fr/inra/urgi/faidare/api/faidare/v1/DataDiscoveryController.java b/backend/src/main/java/fr/inra/urgi/faidare/api/faidare/v1/DataDiscoveryController.java
index b4bef53d..c1424ed2 100644
--- a/backend/src/main/java/fr/inra/urgi/faidare/api/faidare/v1/DataDiscoveryController.java
+++ b/backend/src/main/java/fr/inra/urgi/faidare/api/faidare/v1/DataDiscoveryController.java
@@ -15,7 +15,7 @@ import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.web.bind.annotation.*;
import javax.validation.Valid;
-import java.io.UnsupportedEncodingException;
+import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
@@ -46,6 +46,7 @@ public class DataDiscoveryController {
if (fetchSize == null) {
fetchSize = Integer.MAX_VALUE;
}
+ ) {
return dataDiscoveryRepository.suggest(field, StringFunctions.asUTF8(text), fetchSize, criteria);
}
diff --git a/backend/src/main/java/fr/inra/urgi/faidare/config/ElasticSearchConfig.java b/backend/src/main/java/fr/inra/urgi/faidare/config/ElasticSearchConfig.java
index 8d4901f9..33f42fab 100644
--- a/backend/src/main/java/fr/inra/urgi/faidare/config/ElasticSearchConfig.java
+++ b/backend/src/main/java/fr/inra/urgi/faidare/config/ElasticSearchConfig.java
@@ -2,6 +2,7 @@ package fr.inra.urgi.faidare.config;
import org.apache.http.HttpHost;
import org.apache.http.impl.nio.reactor.IOReactorConfig;
+import org.elasticsearch.action.support.IndicesOptions;
import org.elasticsearch.client.NodeSelector;
import org.elasticsearch.client.RestClient;
import org.elasticsearch.client.RestClientBuilder;
@@ -37,4 +38,15 @@ public class ElasticSearchConfig {
);
}
+ @Bean
+ public IndicesOptions indicesOptions() {
+ boolean ignoreUnavailable = true;
+ boolean allowNoIndices = true;
+ boolean expandToOpenIndices = true;
+ boolean expandToClosedIndices = false;
+ return IndicesOptions.fromOptions(
+ ignoreUnavailable, allowNoIndices, expandToOpenIndices, expandToClosedIndices
+ );
+ }
+
}
diff --git a/backend/src/main/java/fr/inra/urgi/faidare/domain/criteria/base/PaginationCriteriaImpl.java b/backend/src/main/java/fr/inra/urgi/faidare/domain/criteria/base/PaginationCriteriaImpl.java
index 939ddbd4..148e0252 100644
--- a/backend/src/main/java/fr/inra/urgi/faidare/domain/criteria/base/PaginationCriteriaImpl.java
+++ b/backend/src/main/java/fr/inra/urgi/faidare/domain/criteria/base/PaginationCriteriaImpl.java
@@ -9,7 +9,6 @@ import javax.validation.constraints.NotNull;
/**
* @author gcornut
*/
-@ValidPagination // global validation rule
public class PaginationCriteriaImpl implements PaginationCriteria {
public static final long MIN_PAGE = 0;
public static final long MIN_PAGE_SIZE = 1;
diff --git a/backend/src/main/java/fr/inra/urgi/faidare/domain/criteria/base/PaginationMaxResultValidator.java b/backend/src/main/java/fr/inra/urgi/faidare/domain/criteria/base/PaginationMaxResultValidator.java
index 1f7f0096..4b2e8b75 100644
--- a/backend/src/main/java/fr/inra/urgi/faidare/domain/criteria/base/PaginationMaxResultValidator.java
+++ b/backend/src/main/java/fr/inra/urgi/faidare/domain/criteria/base/PaginationMaxResultValidator.java
@@ -8,21 +8,13 @@ import javax.validation.ConstraintValidatorContext;
*
* @author gcornut
*/
-public class PaginationMaxResultValidator implements ConstraintValidator<ValidPagination, PaginationCriteriaImpl> {
+public class PaginationMaxResultValidator implements ConstraintValidator<MaxResultWindowPagination, PaginationCriteriaImpl> {
- /**
- * Default constrain on elasticsearch search result window
- *
- * @link https://www.elastic.co/guide/en/elasticsearch/reference/2.4/index-modules.html
- */
- public static final long MAX_RESULT_WINDOW = 10000;
-
- public static final String ERROR_MAX_RESULT_WINDOW =
- "The result window (page x pageSize) cannot be over " + MAX_RESULT_WINDOW + ". " +
- "Please use an export API to download all the requested data.";
+ private long maxResultWindow;
@Override
- public void initialize(ValidPagination annotation) {
+ public void initialize(MaxResultWindowPagination annotation) {
+ maxResultWindow = annotation.value();
}
@Override
@@ -31,12 +23,18 @@ public class PaginationMaxResultValidator implements ConstraintValidator<ValidPa
Long page = paginationCriteria.getPage();
if (page != null && pageSize != null) {
- if (page * pageSize >= (MAX_RESULT_WINDOW - pageSize)) {
+ if (page * pageSize >= (maxResultWindow)) {
context.disableDefaultConstraintViolation();
- context.buildConstraintViolationWithTemplate(ERROR_MAX_RESULT_WINDOW).addConstraintViolation();
+ context.buildConstraintViolationWithTemplate(getErrorMessage())
+ .addConstraintViolation();
return false;
}
}
return true;
}
+
+ private String getErrorMessage() {
+ return "The result window (page x pageSize) cannot be over " + maxResultWindow + ". " +
+ "Please use an export API to download all the requested data.";
+ }
}
diff --git a/backend/src/main/java/fr/inra/urgi/faidare/domain/criteria/base/ValidPagination.java b/backend/src/main/java/fr/inra/urgi/faidare/domain/criteria/base/ValidPagination.java
index 23097613..e69de29b 100644
--- a/backend/src/main/java/fr/inra/urgi/faidare/domain/criteria/base/ValidPagination.java
+++ b/backend/src/main/java/fr/inra/urgi/faidare/domain/criteria/base/ValidPagination.java
@@ -1,28 +0,0 @@
-package fr.inra.urgi.faidare.domain.criteria.base;
-
-import javax.validation.Constraint;
-import javax.validation.Payload;
-import java.lang.annotation.Documented;
-import java.lang.annotation.Retention;
-import java.lang.annotation.Target;
-
-import static java.lang.annotation.ElementType.ANNOTATION_TYPE;
-import static java.lang.annotation.ElementType.TYPE;
-import static java.lang.annotation.RetentionPolicy.RUNTIME;
-
-/**
- * @author gcornut
- */
-
-@Target({TYPE, ANNOTATION_TYPE})
-@Retention(RUNTIME)
-@Constraint(validatedBy = PaginationMaxResultValidator.class)
-@Documented
-public @interface ValidPagination {
-
- String message() default "";
-
- Class<?>[] groups() default {};
-
- Class<? extends Payload>[] payload() default {};
-}
diff --git a/backend/src/main/java/fr/inra/urgi/faidare/domain/datadiscovery/criteria/DataDiscoveryCriteriaImpl.java b/backend/src/main/java/fr/inra/urgi/faidare/domain/datadiscovery/criteria/DataDiscoveryCriteriaImpl.java
index 892f95d0..37a729da 100644
--- a/backend/src/main/java/fr/inra/urgi/faidare/domain/datadiscovery/criteria/DataDiscoveryCriteriaImpl.java
+++ b/backend/src/main/java/fr/inra/urgi/faidare/domain/datadiscovery/criteria/DataDiscoveryCriteriaImpl.java
@@ -1,5 +1,6 @@
package fr.inra.urgi.faidare.domain.datadiscovery.criteria;
+import fr.inra.urgi.faidare.domain.criteria.base.MaxResultWindowPagination;
import fr.inra.urgi.faidare.domain.criteria.base.PaginationCriteriaImpl;
import fr.inra.urgi.faidare.domain.datadiscovery.data.DataDiscoveryDocument;
import fr.inra.urgi.faidare.elasticsearch.criteria.annotation.CriteriaForDocument;
@@ -13,6 +14,7 @@ import java.util.List;
/**
* @author gcornut
*/
+@MaxResultWindowPagination // Restrict this criteria to the ES max result window
@CriteriaForDocument(DataDiscoveryDocument.class)
public class DataDiscoveryCriteriaImpl extends PaginationCriteriaImpl implements DataDiscoveryCriteria {
diff --git a/backend/src/main/java/fr/inra/urgi/faidare/elasticsearch/ESRequestFactory.java b/backend/src/main/java/fr/inra/urgi/faidare/elasticsearch/ESRequestFactory.java
index 21102c83..e6b159e2 100644
--- a/backend/src/main/java/fr/inra/urgi/faidare/elasticsearch/ESRequestFactory.java
+++ b/backend/src/main/java/fr/inra/urgi/faidare/elasticsearch/ESRequestFactory.java
@@ -2,15 +2,17 @@ package fr.inra.urgi.faidare.elasticsearch;
import com.google.common.base.Joiner;
import fr.inra.urgi.faidare.config.FaidareProperties;
+import fr.inra.urgi.faidare.domain.criteria.base.SortCriteria;
+import fr.inra.urgi.faidare.elasticsearch.criteria.AnnotatedCriteriaMapper;
+import fr.inra.urgi.faidare.elasticsearch.document.DocumentMetadata;
import fr.inra.urgi.faidare.repository.http.UserGroupsResourceClient;
import org.elasticsearch.action.search.SearchRequest;
-import org.elasticsearch.action.search.SearchRequestBuilder;
-import org.elasticsearch.action.search.SearchScrollRequest;
import org.elasticsearch.action.support.IndicesOptions;
import org.elasticsearch.index.query.QueryBuilder;
import org.elasticsearch.search.builder.SearchSourceBuilder;
+import org.elasticsearch.search.sort.FieldSortBuilder;
+import org.elasticsearch.search.sort.SortOrder;
import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
import org.springframework.stereotype.Component;
import java.util.ArrayList;
@@ -19,42 +21,32 @@ import java.util.List;
@Component
public class ESRequestFactory {
- private static final Logger logger = LoggerFactory.getLogger(ESRequestFactory.class);
-
public static final int MAX_TERM_AGG_SIZE = Integer.MAX_VALUE;
- private UserGroupsResourceClient userGroupsResourceClient;
- private FaidareProperties properties;
-
+ private final UserGroupsResourceClient userGroupsResourceClient;
+ private final FaidareProperties properties;
private final IndicesOptions indicesOptions;
public ESRequestFactory(
UserGroupsResourceClient userGroupsResourceClient,
- FaidareProperties properties
+ FaidareProperties properties,
+ IndicesOptions indicesOptions
) {
this.userGroupsResourceClient = userGroupsResourceClient;
this.properties = properties;
- boolean ignoreUnavailable = true;
- boolean allowNoIndices = true;
- boolean expandToOpenIndices = true;
- boolean expandToClosedIndices = false;
- indicesOptions = IndicesOptions.fromOptions(
- ignoreUnavailable, allowNoIndices, expandToOpenIndices, expandToClosedIndices
- );
+ this.indicesOptions = indicesOptions;
}
/**
- * Utility method to retrieve a new {@link SearchRequestBuilder} already
- * configured with the index. This method allows to not forget the index and
+ * Utility method to retrieve a new {@link SearchRequest} already
+ * configured with the groupId filtered aliases. This method allows to not forget the index and
* so to not query the whole cluster!
*
- * @return the {@link SearchRequestBuilder} configured with index
+ * @return the {@link SearchRequest} configured with index
*/
- public SearchRequest prepareSearch(String documentType, QueryBuilder query) {
- String[] aliases = getAliases(documentType, null);
-
+ public SearchRequest prepareSearch(String documentType) {
SearchRequest request = new SearchRequest();
- request.indices(aliases);
+ request.indices(getAliases(documentType, null));
request.source(new SearchSourceBuilder());
request.indicesOptions(indicesOptions);
@@ -62,37 +54,77 @@ public class ESRequestFactory {
request.types(documentType);
}
- if (query != null) {
- request.source().query(query);
+ return request;
+ }
+
+ /**
+ * Prepare a search request with query and criteria.
+ *
+ * Same as {@link #prepareSearch(String)} but with added query, sort and field exclusion
+ */
+ public <C> SearchRequest prepareSearch(
+ QueryBuilder query, C criteria
+ ) {
+ // Get document metadata from criteria annotations
+ DocumentMetadata<?> documentMetadata = AnnotatedCriteriaMapper.getMapping(criteria.getClass()).getDocumentMetadata();
+
+ // Build request
+ String documentType = documentMetadata.getDocumentType();
+ SearchRequest request = prepareSearch(documentType);
+ request.source().query(query);
+
+ // Add sort
+ FieldSortBuilder idSort = new FieldSortBuilder("_id");
+ if (criteria instanceof SortCriteria) {
+ SortCriteria sortCriteria = (SortCriteria) criteria;
+ String field = sortCriteria.getSortBy();
+ if (field == null) {
+ // Default ES sort
+ field = idSort.getFieldName();
+ }
+
+ String strOrder = sortCriteria.getSortOrder();
+ SortOrder order = SortOrder.ASC;
+ if (strOrder != null) {
+ order = SortOrder.valueOf(strOrder.toUpperCase());
+ }
+ request.source().sort(field, order);
+ }
+ request.source().sort(idSort);
+
+ // Add excluded fields if requested
+ String[] excludedFields = documentMetadata.getExcludedFields();
+ if (excludedFields != null && excludedFields.length >= 1) {
+ request.source().fetchSource(null, excludedFields);
}
+ return request;
+ }
+
+
+ /**
+ * Utility function to log a SearchRequest in DEBUG
+ */
+ public static void logRequest(Logger logger, SearchRequest request) {
// Debug
if (logger.isDebugEnabled()) {
- String aliasesString = Joiner.on(",").join(aliases);
+
StringBuilder message = new StringBuilder();
message.append("Search ES:\n");
- message.append("POST ").append(aliasesString);
- if (documentType != null) {
- message.append("/").append(documentType);
+
+ message.append("POST ").append(Joiner.on(",").join(request.indices()));
+ if (request.types() != null && request.types().length > 0) {
+ message.append("/").append(Joiner.on(",").join(request.types()));
}
message.append("/_search");
- if (query != null) {
+ if (request.source() != null) {
message.append("\n");
message.append(request.source().toString());
}
logger.debug(message.toString());
}
-
- return request;
}
- /**
- * List ElasticSearch index group id filtered aliases to a document type
- */
- public String[] getAliases(String documentType) {
- List<String> aliasList = getAliasList(documentType, null);
- return aliasList.toArray(new String[0]);
- }
/**
* List ElasticSearch index group id filtered aliases to a document type
@@ -118,12 +150,4 @@ public class ESRequestFactory {
return aliases;
}
- public SearchRequest prepareSearch(String documentType) {
- return prepareSearch(documentType, null);
- }
-
- public SearchScrollRequest getSearchScroll(String scrollId) {
- return new SearchScrollRequest(scrollId);
- }
-
}
diff --git a/backend/src/main/java/fr/inra/urgi/faidare/elasticsearch/ESResponseParser.java b/backend/src/main/java/fr/inra/urgi/faidare/elasticsearch/ESResponseParser.java
index 09289239..f8a4649a 100644
--- a/backend/src/main/java/fr/inra/urgi/faidare/elasticsearch/ESResponseParser.java
+++ b/backend/src/main/java/fr/inra/urgi/faidare/elasticsearch/ESResponseParser.java
@@ -14,6 +14,7 @@ import org.elasticsearch.search.aggregations.Aggregation;
import org.elasticsearch.search.aggregations.Aggregations;
import org.elasticsearch.search.aggregations.bucket.SingleBucketAggregation;
import org.elasticsearch.search.aggregations.bucket.terms.Terms;
+import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Component;
import java.io.IOException;
@@ -31,6 +32,7 @@ public class ESResponseParser {
private final ObjectMapper objectMapper;
+ @Autowired
public ESResponseParser(ObjectMapper objectMapper) {
this.objectMapper = objectMapper;
}
@@ -171,4 +173,19 @@ public class ESResponseParser {
public List<FacetTermImpl> parseFacetTerms(SearchResponse response, String... aggregationPath) {
return parseFacetTerms(response, Arrays.asList(aggregationPath));
}
+
+ /**
+ * Parse the last hit sort values
+ * (used for next page "searchAfter")
+ */
+ public Object[] parseLastSortValues(SearchResponse response) {
+ SearchHits hits = response.getHits();
+ if (hits == null) return null;
+
+ SearchHit[] searchHits = hits.getHits();
+ if (searchHits == null || searchHits.length == 0) return null;
+
+ SearchHit lastHit = searchHits[searchHits.length - 1];
+ return lastHit.getSortValues();
+ }
}
diff --git a/backend/src/main/java/fr/inra/urgi/faidare/elasticsearch/ESScrollIterator.java b/backend/src/main/java/fr/inra/urgi/faidare/elasticsearch/ESScrollIterator.java
index f106e9b7..bd8d656e 100644
--- a/backend/src/main/java/fr/inra/urgi/faidare/elasticsearch/ESScrollIterator.java
+++ b/backend/src/main/java/fr/inra/urgi/faidare/elasticsearch/ESScrollIterator.java
@@ -38,7 +38,6 @@ public class ESScrollIterator<T> implements Iterator<T> {
private final Class<T> documentClass;
private final RestHighLevelClient client;
- private final ESRequestFactory requestFactory;
private final ESResponseParser parser;
private final long totalHits;
@@ -54,21 +53,21 @@ public class ESScrollIterator<T> implements Iterator<T> {
int fetchSize
) {
this.client = client;
- this.requestFactory = requestFactory;
this.parser = parser;
this.documentClass = documentClass;
DocumentMetadata<T> documentMetadata = DocumentAnnotationUtil.getDocumentObjectMetadata(documentClass);
SearchRequest request = requestFactory
- .prepareSearch(documentMetadata.getDocumentType(), query)
+ .prepareSearch(documentMetadata.getDocumentType())
.scroll(DEFAULT_KEEP_ALIVE_TIME);
request.source()
.size(fetchSize)
- .sort(FieldSortBuilder.DOC_FIELD_NAME, SortOrder.ASC);
+ .sort(FieldSortBuilder.DOC_FIELD_NAME, SortOrder.ASC)
+ .query(query);
- SearchResponse response = null;
+ SearchResponse response;
try {
response = client.search(request, RequestOptions.DEFAULT);
} catch (IOException e) {
@@ -99,13 +98,12 @@ public class ESScrollIterator<T> implements Iterator<T> {
return true;
} else if (totalHits > hitIndex) {
- SearchScrollRequest request = requestFactory
- .getSearchScroll(scrollId)
+ SearchScrollRequest request = new SearchScrollRequest(scrollId)
.scroll(DEFAULT_KEEP_ALIVE_TIME);
LOGGER.debug("Scroll new page: " + scrollId);
- SearchResponse response = null;
+ SearchResponse response;
try {
response = client.scroll(request, RequestOptions.DEFAULT);
} catch (IOException e) {
diff --git a/backend/src/main/java/fr/inra/urgi/faidare/elasticsearch/document/DocumentMetadata.java b/backend/src/main/java/fr/inra/urgi/faidare/elasticsearch/document/DocumentMetadata.java
index 6bf7389b..994d5b94 100644
--- a/backend/src/main/java/fr/inra/urgi/faidare/elasticsearch/document/DocumentMetadata.java
+++ b/backend/src/main/java/fr/inra/urgi/faidare/elasticsearch/document/DocumentMetadata.java
@@ -57,6 +57,12 @@ public class DocumentMetadata<VO> {
return idField;
}
+ public void checkIdField() {
+ if (idField == null) {
+ throw new DocumentAnnotationUtil.ValueObjectMetadataException("Could not identify an identifier field on document " + documentClass.getSimpleName());
+ }
+ }
+
public String[] getExcludedFields() {
return excludedFields;
}
diff --git a/backend/src/main/java/fr/inra/urgi/faidare/elasticsearch/repository/impl/BaseESRepository.java b/backend/src/main/java/fr/inra/urgi/faidare/elasticsearch/repository/impl/BaseESRepository.java
index bce6728b..81d7d317 100644
--- a/backend/src/main/java/fr/inra/urgi/faidare/elasticsearch/repository/impl/BaseESRepository.java
+++ b/backend/src/main/java/fr/inra/urgi/faidare/elasticsearch/repository/impl/BaseESRepository.java
@@ -26,7 +26,7 @@ public class BaseESRepository<C extends PaginationCriteria, VO>
ESResponseParser parser
) {
getByIdRepository = new ESGenericGetByIdRepository<>(client, requestFactory, voClass, parser);
- findRepository = new ESGenericFindRepository<>(client, requestFactory, voClass, parser);
+ findRepository = new ESGenericFindRepository<>(client, requestFactory, voClass, new ESGenericQueryFactory<>(), parser);
}
@Override
diff --git a/backend/src/main/java/fr/inra/urgi/faidare/elasticsearch/repository/impl/ESGenericFindRepository.java b/backend/src/main/java/fr/inra/urgi/faidare/elasticsearch/repository/impl/ESGenericFindRepository.java
index 54e7b287..a3ee3408 100644
--- a/backend/src/main/java/fr/inra/urgi/faidare/elasticsearch/repository/impl/ESGenericFindRepository.java
+++ b/backend/src/main/java/fr/inra/urgi/faidare/elasticsearch/repository/impl/ESGenericFindRepository.java
@@ -1,27 +1,36 @@
package fr.inra.urgi.faidare.elasticsearch.repository.impl;
-import fr.inra.urgi.faidare.domain.criteria.base.PaginationCriteria;
+import com.google.common.cache.Cache;
+import com.google.common.cache.CacheBuilder;
+import fr.inra.urgi.gpds.domain.criteria.base.PaginationCriteria;
+import fr.inra.urgi.gpds.domain.response.PaginatedList;
+import fr.inra.urgi.gpds.domain.response.Pagination;
+import fr.inra.urgi.gpds.domain.response.PaginationImpl;
+import fr.inra.urgi.gpds.elasticsearch.ESRequestFactory;
+import fr.inra.urgi.gpds.elasticsearch.ESResponseParser;
+import fr.inra.urgi.gpds.elasticsearch.query.ESQueryFactory;
+import fr.inra.urgi.gpds.elasticsearch.repository.ESFindRepository;
import fr.inra.urgi.faidare.domain.criteria.base.SortCriteria;
-import fr.inra.urgi.faidare.domain.response.PaginatedList;
-import fr.inra.urgi.faidare.domain.response.Pagination;
-import fr.inra.urgi.faidare.domain.response.PaginationImpl;
-import fr.inra.urgi.faidare.elasticsearch.ESRequestFactory;
-import fr.inra.urgi.faidare.elasticsearch.ESResponseParser;
import fr.inra.urgi.faidare.elasticsearch.document.DocumentAnnotationUtil;
import fr.inra.urgi.faidare.elasticsearch.document.DocumentMetadata;
-import fr.inra.urgi.faidare.elasticsearch.query.ESQueryFactory;
import fr.inra.urgi.faidare.elasticsearch.query.impl.ESGenericQueryFactory;
-import fr.inra.urgi.faidare.elasticsearch.repository.ESFindRepository;
import org.elasticsearch.action.search.SearchRequest;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.client.RequestOptions;
import org.elasticsearch.client.RestHighLevelClient;
import org.elasticsearch.index.query.QueryBuilder;
-import org.elasticsearch.search.sort.SortOrder;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import java.io.IOException;
+import java.util.HashMap;
import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.TimeUnit;
+
+import static fr.inra.urgi.gpds.domain.criteria.base.MaxResultWindowPagination.MAX_RESULT_WINDOW;
/**
* Generic implementation of an ElasticSearch document repository which can find documents by criteria.
@@ -30,10 +39,15 @@ import java.util.List;
*/
public class ESGenericFindRepository<C extends PaginationCriteria, VO> implements ESFindRepository<C, VO> {
+ private static final Logger LOGGER = LoggerFactory.getLogger(ESGenericFindRepository.class);
+ private static final Cache<String, Map<PageCoordinate, Object[]>> SORT_VALUES_CACHE = CacheBuilder.newBuilder()
+ .expireAfterWrite(1, TimeUnit.HOURS)
+ .build();
+
private final RestHighLevelClient client;
private final ESRequestFactory requestFactory;
+ private final Class<VO> voClass;
private final ESQueryFactory<C> queryFactory;
- private final DocumentMetadata<VO> documentMetadata;
private final ESResponseParser parser;
public ESGenericFindRepository(
@@ -45,62 +59,63 @@ public class ESGenericFindRepository<C extends PaginationCriteria, VO> implement
) {
this.client = client;
this.requestFactory = requestFactory;
+ this.voClass = voClass;
this.queryFactory = queryFactory;
- this.documentMetadata = DocumentAnnotationUtil.getDocumentObjectMetadata(voClass);
this.parser = parser;
}
- public ESGenericFindRepository(
- RestHighLevelClient client,
- ESRequestFactory requestFactory,
- Class<VO> voClass,
- ESResponseParser parser
- ) {
- this(client, requestFactory, voClass, new ESGenericQueryFactory<>(), parser);
- }
-
- public static <C extends PaginationCriteria, D> SearchRequest prepareSearchRequest(
- QueryBuilder query, C criteria, DocumentMetadata<D> documentMetadata, ESRequestFactory requestFactory
- ) {
- // Build query
- int size = criteria.getPageSize().intValue();
- int from = criteria.getPage().intValue() * size;
-
- // Build request
- String documentType = documentMetadata.getDocumentType();
- SearchRequest request = requestFactory.prepareSearch(documentType, query);
- request.source()
- .from(from)
- .size(size);
-
- // Add sort if requested
- if (criteria instanceof SortCriteria) {
- SortCriteria sortCriteria = (SortCriteria) criteria;
- String field = sortCriteria.getSortBy();
- if (field == null) {
- // Default ES sort
- field = "_doc";
- }
-
- String strOrder = sortCriteria.getSortOrder();
- SortOrder order = SortOrder.ASC;
- if (strOrder != null) {
- order = SortOrder.valueOf(strOrder.toUpperCase());
+ /**
+ * Get search after value for query
+ */
+ private Object[] getSearchAfter(
+ SearchRequest request,
+ PageCoordinate page
+ ) throws IOException, ExecutionException {
+ String searchSource = request.source().toString();
+ Map<PageCoordinate, Object[]> pageToLastSorted = SORT_VALUES_CACHE.get(searchSource, HashMap::new);
+
+ Object[] lastSortValues = pageToLastSorted.get(page.previousPage());
+
+ if (lastSortValues == null) {
+ // Find last sort value for previous page
+
+ // Start at first page under the max result window
+ PageCoordinate currentPage = page;
+ while (currentPage.from >= MAX_RESULT_WINDOW) {
+ currentPage = currentPage.previousPage();
}
- request.source().sort(field, order);
- }
-
- // Add excluded fields if requested
- String[] excludedFields = documentMetadata.getExcludedFields();
- if (excludedFields != null && excludedFields.length >= 1) {
- request.source().fetchSource(null, excludedFields);
+ request.source().size(currentPage.size);
+
+ Object[] previousSortValues = null;
+ do {
+ Object[] currentSortValues = pageToLastSorted.get(currentPage);
+ if (currentSortValues == null) {
+ // Page sort value not yet fetched
+
+ if (previousSortValues == null) {
+ request.source().from(currentPage.from);
+ } else {
+ request.source().from(0);
+ request.source().searchAfter(previousSortValues);
+ }
+
+ ESRequestFactory.logRequest(LOGGER, request);
+ SearchResponse response = client.search(request, RequestOptions.DEFAULT);
+ checkPaginationBound(page, response);
+
+ currentSortValues = parser.parseLastSortValues(response);
+
+ // Cache page sort value
+ pageToLastSorted.put(currentPage, currentSortValues);
+ }
+
+ // Next page
+ previousSortValues = currentSortValues;
+ currentPage = currentPage.nextPage();
+ } while (!currentPage.equals(page));
+ return previousSortValues;
}
-
- Logger logger = LoggerFactory.getLogger(ESGenericFindRepository.class);
- if (logger.isDebugEnabled()) {
- logger.debug(request.toString());
- }
- return request;
+ return lastSortValues;
}
@Override
@@ -108,18 +123,46 @@ public class ESGenericFindRepository<C extends PaginationCriteria, VO> implement
try {
QueryBuilder query = queryFactory.createQuery(criteria);
- SearchRequest request = prepareSearchRequest(
- query, criteria, documentMetadata, requestFactory
+ SearchRequest request = requestFactory.prepareSearch(
+ query, criteria
);
+ String sourceQueryWithoutPagination = request.source().toString();
+
+ // Set pagination
+ int size = criteria.getPageSize().intValue();
+ int from = criteria.getPage().intValue() * size;
+ PageCoordinate page = new PageCoordinate(size, from);
+
+ if (from < MAX_RESULT_WINDOW) {
+ // Classic search from / size
+ request.source().from(from);
+ request.source().size(size);
+ } else {
+ // More complex search using search after
+ Object[] searchAfterSortValues = getSearchAfter(
+ request,
+ page
+ );
+ request.source().size(size);
+ request.source().from(0);
+ request.source().searchAfter(searchAfterSortValues);
+ }
// Execute request
- SearchResponse result = client.search(request, RequestOptions.DEFAULT);
+ ESRequestFactory.logRequest(LOGGER, request);
+ SearchResponse response = client.search(request, RequestOptions.DEFAULT);
+ checkPaginationBound(page, response);
+
+ // Cache last sort value for later
+ SORT_VALUES_CACHE
+ .get(sourceQueryWithoutPagination, HashMap::new)
+ .put(page, parser.parseLastSortValues(response));
// Prepare pagination info
- Pagination pagination = PaginationImpl.create(criteria, parser.parseTotalHits(result));
+ Pagination pagination = PaginationImpl.create(criteria, parser.parseTotalHits(response));
// Parse result list
- List<VO> resultList = parser.parseHits(result, documentMetadata.getDocumentClass());
+ List<VO> resultList = parser.parseHits(response, voClass);
// Return paginated list
return new PaginatedList<>(pagination, resultList);
@@ -128,4 +171,56 @@ public class ESGenericFindRepository<C extends PaginationCriteria, VO> implement
}
}
+
+ private void checkPaginationBound(PageCoordinate page, SearchResponse response) {
+ Long totalHits = parser.parseTotalHits(response);
+ if (page.from > totalHits) {
+ throw new PageOutOfBoundException();
+ }
+ }
+
+ /**
+ * RuntimeException occurring when requesting a page above the maximum number of page of a result set.
+ */
+ static public class PageOutOfBoundException extends RuntimeException {
+ PageOutOfBoundException() {
+ super("The current page number should be strictly less than the total number of pages.");
+ }
+ }
+
+ /**
+ * Data class used to represent from/size Elasticsearch page coordinates
+ */
+ static private class PageCoordinate {
+ int size;
+ int from;
+
+ PageCoordinate(int size, int from) {
+ this.size = size;
+ this.from = from;
+ }
+
+ PageCoordinate previousPage() {
+ int nextFrom = this.from - size;
+ if (nextFrom < 0) {
+ nextFrom = 0;
+ }
+ return new PageCoordinate(size, nextFrom);
+ }
+
+ PageCoordinate nextPage() {
+ return new PageCoordinate(size, from + size);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(size, from);
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ return o instanceof PageCoordinate && hashCode() == o.hashCode();
+ }
+ }
+
}
diff --git a/backend/src/main/java/fr/inra/urgi/faidare/elasticsearch/repository/impl/ESGenericGetByIdRepository.java b/backend/src/main/java/fr/inra/urgi/faidare/elasticsearch/repository/impl/ESGenericGetByIdRepository.java
index 967fc51f..7ee3086d 100644
--- a/backend/src/main/java/fr/inra/urgi/faidare/elasticsearch/repository/impl/ESGenericGetByIdRepository.java
+++ b/backend/src/main/java/fr/inra/urgi/faidare/elasticsearch/repository/impl/ESGenericGetByIdRepository.java
@@ -9,7 +9,6 @@ import org.elasticsearch.action.search.SearchRequest;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.client.RequestOptions;
import org.elasticsearch.client.RestHighLevelClient;
-import org.elasticsearch.index.query.QueryBuilder;
import org.elasticsearch.index.query.QueryBuilders;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -17,20 +16,22 @@ import org.slf4j.LoggerFactory;
import java.util.List;
/**
- * Generic implementation of an ElasticSearch query for document by its identifier
+ * Generic implementation of an ElasticSearch query for document by its identifier.
+ *
+ * Requires the document VO to have the @{@link fr.inra.urgi.gpds.elasticsearch.document.annotation.Document}
+ * and @{@link fr.inra.urgi.gpds.elasticsearch.document.annotation.Id} annotations
*
* @author gcornut
*/
public class ESGenericGetByIdRepository<VO> implements ESGetByIdRepository<VO> {
+ private static final Logger LOGGER = LoggerFactory.getLogger(ESGenericGetByIdRepository.class);
+
private final RestHighLevelClient client;
private final ESRequestFactory requestFactory;
private final Class<VO> voClass;
private final ESResponseParser parser;
-
- private DocumentMetadata<VO> documentMetadata;
-
- private Logger logger = LoggerFactory.getLogger(getClass());
+ private final DocumentMetadata<VO> documentMetadata;
public ESGenericGetByIdRepository(
RestHighLevelClient client,
@@ -42,24 +43,31 @@ public class ESGenericGetByIdRepository<VO> implements ESGetByIdRepository<VO> {
this.requestFactory = requestFactory;
this.voClass = voClass;
this.parser = parser;
- documentMetadata = DocumentAnnotationUtil.getDocumentObjectMetadata(voClass);
- if (documentMetadata.getIdField() == null) {
- throw new RuntimeException("Could not identify an identifier field on document " + voClass.getSimpleName());
- }
+ this.documentMetadata = DocumentAnnotationUtil.getDocumentObjectMetadata(voClass);
+ this.documentMetadata.checkIdField();
}
+ /**
+ * Get document by ID.
+ *
+ * The implementation uses a search request rather than a get document
+ * request because we want to get this document for in a list of index
+ * aliases with cannot be done with the get document request.
+ */
@Override
public VO getById(String id) {
try {
String documentType = documentMetadata.getDocumentType();
+ String idField = documentMetadata.getIdField();
- // Build term id query
- QueryBuilder query = QueryBuilders.termQuery(documentMetadata.getIdField(), id);
+ // Prepare request
+ SearchRequest request = requestFactory.prepareSearch(documentType);
- // Build request
- SearchRequest request = requestFactory.prepareSearch(documentType, query);
+ // Build term id query
+ request.source().query(QueryBuilders.termQuery(idField, id));
// Execute request
+ ESRequestFactory.logRequest(LOGGER, request);
SearchResponse result = client.search(request, RequestOptions.DEFAULT);
// Parse result list
diff --git a/backend/src/main/java/fr/inra/urgi/faidare/elasticsearch/repository/impl/ESGenericSuggestRepository.java b/backend/src/main/java/fr/inra/urgi/faidare/elasticsearch/repository/impl/ESGenericSuggestRepository.java
index 9be7e0ce..ed414bbd 100644
--- a/backend/src/main/java/fr/inra/urgi/faidare/elasticsearch/repository/impl/ESGenericSuggestRepository.java
+++ b/backend/src/main/java/fr/inra/urgi/faidare/elasticsearch/repository/impl/ESGenericSuggestRepository.java
@@ -60,14 +60,6 @@ public class ESGenericSuggestRepository<C extends PaginationCriteria, VO> implem
this.parser = parser;
}
- public ESGenericSuggestRepository(
- RestHighLevelClient client,
- ESRequestFactory requestFactory,
- Class<VO> voClass,
- ESResponseParser parser) {
- this(client, requestFactory, voClass, new ESGenericQueryFactory<C>(), parser);
- }
-
@Override
public LinkedHashSet<String> suggest(String field, String searchText, Integer fetchSize, C criteria) {
int size = fetchSize != null ? fetchSize : 10;
@@ -104,10 +96,10 @@ public class ESGenericSuggestRepository<C extends PaginationCriteria, VO> implem
request.source().size(0);
request.source().aggregation(agg);
- LOGGER.debug(request.toString());
SearchResponse searchResponse = null;
try {
+ ESRequestFactory.logRequest(LOGGER, request);
searchResponse = client.search(request, RequestOptions.DEFAULT);
} catch (IOException e) {
throw new RuntimeException(e);
diff --git a/backend/src/main/java/fr/inra/urgi/faidare/repository/es/DataDiscoveryRepositoryImpl.java b/backend/src/main/java/fr/inra/urgi/faidare/repository/es/DataDiscoveryRepositoryImpl.java
index 73662c5d..e13faece 100644
--- a/backend/src/main/java/fr/inra/urgi/faidare/repository/es/DataDiscoveryRepositoryImpl.java
+++ b/backend/src/main/java/fr/inra/urgi/faidare/repository/es/DataDiscoveryRepositoryImpl.java
@@ -91,7 +91,7 @@ public class DataDiscoveryRepositoryImpl implements DataDiscoveryRepository {
SearchRequest request = prepareSearchRequest(criteria);
// Execute request
- LOGGER.debug(request.toString());
+ ESRequestFactory.logRequest(LOGGER, request);
SearchResponse response = client.search(request, RequestOptions.DEFAULT);
// Parse request result
@@ -112,8 +112,15 @@ public class DataDiscoveryRepositoryImpl implements DataDiscoveryRepository {
QueryBuilder query = queryFactory.createQueryExcludingFields(criteria, documentFieldsInFacets);
// Prepare search request with query
- SearchRequest request = ESGenericFindRepository.prepareSearchRequest(
- query, criteria, documentMetadata, requestFactory);
+ SearchRequest request = requestFactory.prepareSearch(query, criteria);
+
+ // Set pagination
+ int size = criteria.getPageSize().intValue();
+ int from = criteria.getPage().intValue() * size;
+
+ request.source()
+ .from(from)
+ .size(size);
// Build facet aggregations
if (facetFields != null) {
diff --git a/backend/src/main/java/fr/inra/urgi/faidare/repository/es/GermplasmAttributeRepositoryImpl.java b/backend/src/main/java/fr/inra/urgi/faidare/repository/es/GermplasmAttributeRepositoryImpl.java
index 2fee54cd..5417cd9a 100644
--- a/backend/src/main/java/fr/inra/urgi/faidare/repository/es/GermplasmAttributeRepositoryImpl.java
+++ b/backend/src/main/java/fr/inra/urgi/faidare/repository/es/GermplasmAttributeRepositoryImpl.java
@@ -7,6 +7,7 @@ import fr.inra.urgi.faidare.elasticsearch.ESRequestFactory;
import fr.inra.urgi.faidare.elasticsearch.ESResponseParser;
import fr.inra.urgi.faidare.elasticsearch.repository.ESFindRepository;
import fr.inra.urgi.faidare.elasticsearch.repository.impl.ESGenericFindRepository;
+import fr.inra.urgi.faidare.elasticsearch.query.impl.ESGenericQueryFactory;
import org.elasticsearch.client.RestHighLevelClient;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Repository;
@@ -27,7 +28,7 @@ public class GermplasmAttributeRepositoryImpl
ESRequestFactory requestFactory
) {
Class<GermplasmAttributeValueListVO> voClass = GermplasmAttributeValueListVO.class;
- findAttributeRepository = new ESGenericFindRepository<>(client, requestFactory, voClass, parser);
+ findAttributeRepository = new ESGenericFindRepository<>(client, requestFactory, voClass, new ESGenericQueryFactory<>(), parser);
}
@Override
diff --git a/backend/src/main/java/fr/inra/urgi/faidare/repository/es/GermplasmRepositoryImpl.java b/backend/src/main/java/fr/inra/urgi/faidare/repository/es/GermplasmRepositoryImpl.java
index e89383e1..c1476788 100644
--- a/backend/src/main/java/fr/inra/urgi/faidare/repository/es/GermplasmRepositoryImpl.java
+++ b/backend/src/main/java/fr/inra/urgi/faidare/repository/es/GermplasmRepositoryImpl.java
@@ -57,7 +57,7 @@ public class GermplasmRepositoryImpl implements GermplasmRepository {
this.parser = parser;
Class<GermplasmVO> voClass = GermplasmVO.class;
this.queryFactory = new ESGenericQueryFactory<>();
- this.findRepository = new ESGenericFindRepository<>(client, requestFactory, voClass, this.parser);
+ this.findRepository = new ESGenericFindRepository<>(client, requestFactory, voClass, queryFactory, this.parser);
this.getByIdRepository = new ESGenericGetByIdRepository<>(client, requestFactory, voClass, this.parser);
}
@@ -78,10 +78,12 @@ public class GermplasmRepositoryImpl implements GermplasmRepository {
return findRepository.find(criteria);
}
+ // TODO: Replace this using a ESGenericGetByIdRepository<PedigreeVO>
@Override
public PedigreeVO findPedigree(String germplasmDbId) {
QueryBuilder termQuery = QueryBuilders.termQuery("germplasmDbId", germplasmDbId);
- SearchRequest request = requestFactory.prepareSearch("germplasmPedigree", termQuery);
+ SearchRequest request = requestFactory.prepareSearch("germplasmPedigree");
+ request.source().query(termQuery);
SearchResponse response = null;
try {
response = client.search(request, RequestOptions.DEFAULT);
@@ -108,10 +110,12 @@ public class GermplasmRepositoryImpl implements GermplasmRepository {
return pedigreeVO;
}
+ // TODO: Replace this using a ESGenericGetByIdRepository<ProgenyVO>
@Override
public ProgenyVO findProgeny(String germplasmDbId) {
QueryBuilder termQuery = QueryBuilders.termQuery("germplasmDbId", germplasmDbId);
- SearchRequest request = requestFactory.prepareSearch("germplasmProgeny", termQuery);
+ SearchRequest request = requestFactory.prepareSearch("germplasmProgeny");
+ request.source().query(termQuery);
SearchResponse response = null;
try {
response = client.search(request, RequestOptions.DEFAULT);
diff --git a/backend/src/main/java/fr/inra/urgi/gpds/domain/criteria/base/MaxResultWindowPagination.java b/backend/src/main/java/fr/inra/urgi/gpds/domain/criteria/base/MaxResultWindowPagination.java
new file mode 100644
index 00000000..c55fb0f7
--- /dev/null
+++ b/backend/src/main/java/fr/inra/urgi/gpds/domain/criteria/base/MaxResultWindowPagination.java
@@ -0,0 +1,38 @@
+package fr.inra.urgi.gpds.domain.criteria.base;
+
+import javax.validation.Constraint;
+import javax.validation.Payload;
+import java.lang.annotation.Documented;
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+
+import static java.lang.annotation.ElementType.ANNOTATION_TYPE;
+import static java.lang.annotation.ElementType.TYPE;
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+
+/**
+ * Constraint on pagination criteria to not get over the max result window
+ *
+ * @author gcornut
+ */
+@Target({TYPE, ANNOTATION_TYPE})
+@Retention(RUNTIME)
+@Constraint(validatedBy = PaginationMaxResultValidator.class)
+@Documented
+public @interface MaxResultWindowPagination {
+
+ /**
+ * Default constrain on elasticsearch search result window
+ *
+ * @link https://www.elastic.co/guide/en/elasticsearch/reference/2.4/index-modules.html
+ */
+ long MAX_RESULT_WINDOW = 10000L;
+
+ long value() default MAX_RESULT_WINDOW;
+
+ String message() default "";
+
+ Class<?>[] groups() default {};
+
+ Class<? extends Payload>[] payload() default {};
+}
diff --git a/backend/src/test/java/fr/inra/urgi/faidare/api/brapi/v1/BrapiExceptionHandlerTest.java b/backend/src/test/java/fr/inra/urgi/faidare/api/brapi/v1/BrapiExceptionHandlerTest.java
index d395c9d0..4f4895a3 100644
--- a/backend/src/test/java/fr/inra/urgi/faidare/api/brapi/v1/BrapiExceptionHandlerTest.java
+++ b/backend/src/test/java/fr/inra/urgi/faidare/api/brapi/v1/BrapiExceptionHandlerTest.java
@@ -48,9 +48,6 @@ class BrapiExceptionHandlerTest {
" {" +
" \"name\":\"Bad Request: Page size cannot be above 1000\"," +
" \"code\":\"400\"" +
- " },{" +
- " \"name\":\"Bad Request: The result window (page x pageSize) cannot be over 10000. Please use an export API to download all the requested data.\"," +
- " \"code\":\"400\"" +
" }" +
" ]," +
" \"datafiles\": []\n" +
diff --git a/backend/src/test/java/fr/inra/urgi/faidare/domain/criteria/base/PaginationValidatorTest.java b/backend/src/test/java/fr/inra/urgi/faidare/domain/criteria/base/PaginationValidatorTest.java
index 368e64ce..2fed235a 100644
--- a/backend/src/test/java/fr/inra/urgi/faidare/domain/criteria/base/PaginationValidatorTest.java
+++ b/backend/src/test/java/fr/inra/urgi/faidare/domain/criteria/base/PaginationValidatorTest.java
@@ -1,5 +1,6 @@
package fr.inra.urgi.faidare.domain.criteria.base;
+import fr.inra.urgi.gpds.domain.datadiscovery.criteria.DataDiscoveryCriteriaImpl;
import org.assertj.core.api.Assertions;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
@@ -67,18 +68,33 @@ public class PaginationValidatorTest {
}
@Test
- public void should_Control_Max_Result_Window() {
+ public void should_Control_No_Max_Result_Window() {
+ // The default pagination criteria does not have a max result window
PaginationCriteriaImpl criteria = new PaginationCriteriaImpl();
criteria.setPageSize(101L);
criteria.setPage(100L);
+ Set<ConstraintViolation<PaginationCriteria>> violations;
+ violations = validator.validate(((PaginationCriteria) criteria));
+ Assertions.assertThat(violations).isNotNull().hasSize(0);
+ }
+
+ @Test
+ public void should_Control_Data_Discovery_Criteria_Max_Result_Window() {
+ // The data discovery criteria does have a max result window
+ DataDiscoveryCriteriaImpl criteria = new DataDiscoveryCriteriaImpl();
+
+ criteria.setPageSize(101L);
+ criteria.setPage(100L);
+
Set<ConstraintViolation<PaginationCriteria>> violations;
violations = validator.validate(((PaginationCriteria) criteria));
Assertions.assertThat(violations).isNotNull().hasSize(1);
- Assertions.assertThat(violations)
- .extracting("message")
- .containsOnly(PaginationMaxResultValidator.ERROR_MAX_RESULT_WINDOW);
+ String message = violations.iterator().next().getMessage();
+ Assertions.assertThat(message)
+ .startsWith("The result window (page x pageSize) cannot be over "
+ + MaxResultWindowPagination.MAX_RESULT_WINDOW);
}
@Test
diff --git a/backend/src/test/java/fr/inra/urgi/faidare/elasticsearch/ESRequestFactoryTest.java b/backend/src/test/java/fr/inra/urgi/faidare/elasticsearch/ESRequestFactoryTest.java
index 5ff7d1ca..d43f1bd5 100644
--- a/backend/src/test/java/fr/inra/urgi/faidare/elasticsearch/ESRequestFactoryTest.java
+++ b/backend/src/test/java/fr/inra/urgi/faidare/elasticsearch/ESRequestFactoryTest.java
@@ -41,7 +41,7 @@ class ESRequestFactoryTest {
when(userGroupsResourceClient.getUserGroups()).thenReturn(groups);
- String[] aliases = requestFactory.getAliases(documentType);
+ String[] aliases = requestFactory.getAliases(documentType, null);
assertThat(aliases).containsExactly(index0);
}
diff --git a/backend/src/test/java/fr/inra/urgi/gpds/api/gnpis/v1/DataDiscoveryControllerTest.java b/backend/src/test/java/fr/inra/urgi/gpds/api/gnpis/v1/DataDiscoveryControllerTest.java
new file mode 100644
index 00000000..f6a29afd
--- /dev/null
+++ b/backend/src/test/java/fr/inra/urgi/gpds/api/gnpis/v1/DataDiscoveryControllerTest.java
@@ -0,0 +1,53 @@
+package fr.inra.urgi.gpds.api.gnpis.v1;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import fr.inra.urgi.gpds.domain.datadiscovery.criteria.DataDiscoveryCriteriaImpl;
+import fr.inra.urgi.gpds.repository.es.DataDiscoveryRepository;
+import fr.inra.urgi.gpds.repository.file.DataSourceRepository;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest;
+import org.springframework.boot.test.mock.mockito.MockBean;
+import org.springframework.http.MediaType;
+import org.springframework.test.context.junit.jupiter.SpringExtension;
+import org.springframework.test.web.servlet.MockMvc;
+import org.springframework.test.web.servlet.MvcResult;
+
+import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
+import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
+import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
+
+/**
+ * @author gcornut
+ */
+@ExtendWith(SpringExtension.class)
+@WebMvcTest(controllers = DataDiscoveryController.class)
+public class DataDiscoveryControllerTest {
+
+ @Autowired
+ private MockMvc mockMvc;
+
+ @MockBean
+ private DataDiscoveryRepository repository;
+
+ @MockBean
+ private DataSourceRepository dataSourceRepository;
+
+ @Test
+ void should_Throw_Pagination_Max_Size_Exception() throws Exception {
+ DataDiscoveryCriteriaImpl criteria = new DataDiscoveryCriteriaImpl();
+ criteria.setPage(10L);
+ criteria.setPageSize(1000L);
+
+ MvcResult res = mockMvc.perform(
+ post("/gnpis/v1/datadiscovery/search")
+ .contentType(MediaType.APPLICATION_JSON_UTF8)
+ .content(new ObjectMapper().writeValueAsString(criteria))
+ ).andExpect(status().isBadRequest()).andReturn();
+
+ assertThat(res.getResponse().getContentAsString()).contains(
+ "The result window (page x pageSize) cannot be over 10000"
+ );
+ }
+}
--
GitLab
From f112c2b37f3d9c73a38e51b1d2a1f89205dbc9a2 Mon Sep 17 00:00:00 2001
From: jdestin <jeremy.destin@inra.fr>
Date: Fri, 21 Feb 2020 11:44:32 +0100
Subject: [PATCH 2/3] fix: Rebase on master.GNP-5623
---
.../api/brapi/v1/BrapiExceptionHandler.java | 8 +------
.../faidare/v1/DataDiscoveryController.java | 12 ++++------
.../base/MaxResultWindowPagination.java | 2 +-
.../repository/impl/BaseESRepository.java | 1 +
.../impl/ESGenericFindRepository.java | 22 ++++++++-----------
backend/src/main/resources/application.yml | 5 -----
6 files changed, 16 insertions(+), 34 deletions(-)
rename backend/src/main/java/fr/inra/urgi/{gpds => faidare}/domain/criteria/base/MaxResultWindowPagination.java (95%)
diff --git a/backend/src/main/java/fr/inra/urgi/faidare/api/brapi/v1/BrapiExceptionHandler.java b/backend/src/main/java/fr/inra/urgi/faidare/api/brapi/v1/BrapiExceptionHandler.java
index d30b90c7..580cf146 100644
--- a/backend/src/main/java/fr/inra/urgi/faidare/api/brapi/v1/BrapiExceptionHandler.java
+++ b/backend/src/main/java/fr/inra/urgi/faidare/api/brapi/v1/BrapiExceptionHandler.java
@@ -6,13 +6,7 @@ import fr.inra.urgi.faidare.api.brapi.v1.exception.BrapiPaginationException;
import fr.inra.urgi.faidare.domain.brapi.v1.response.BrapiPagination;
import fr.inra.urgi.faidare.domain.brapi.v1.response.BrapiStatus;
import fr.inra.urgi.faidare.domain.response.ApiResponseFactory;
-import fr.inra.urgi.gpds.api.BadRequestException;
-import fr.inra.urgi.gpds.api.NotFoundException;
-import fr.inra.urgi.gpds.api.brapi.v1.exception.BrapiPaginationException;
-import fr.inra.urgi.gpds.domain.brapi.v1.response.BrapiPagination;
-import fr.inra.urgi.gpds.domain.brapi.v1.response.BrapiStatus;
-import fr.inra.urgi.gpds.domain.response.ApiResponseFactory;
-import fr.inra.urgi.gpds.elasticsearch.repository.impl.ESGenericFindRepository;
+import fr.inra.urgi.faidare.elasticsearch.repository.impl.ESGenericFindRepository;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.validation.BindException;
diff --git a/backend/src/main/java/fr/inra/urgi/faidare/api/faidare/v1/DataDiscoveryController.java b/backend/src/main/java/fr/inra/urgi/faidare/api/faidare/v1/DataDiscoveryController.java
index c1424ed2..0463bddd 100644
--- a/backend/src/main/java/fr/inra/urgi/faidare/api/faidare/v1/DataDiscoveryController.java
+++ b/backend/src/main/java/fr/inra/urgi/faidare/api/faidare/v1/DataDiscoveryController.java
@@ -15,7 +15,6 @@ import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.web.bind.annotation.*;
import javax.validation.Valid;
-import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
@@ -42,15 +41,12 @@ public class DataDiscoveryController {
@RequestParam(required = false) String text,
@RequestParam(required = false) Integer fetchSize,
@RequestBody(required = false) @Valid DataDiscoveryCriteriaImpl criteria
- ) throws UnsupportedEncodingException {
- if (fetchSize == null) {
- fetchSize = Integer.MAX_VALUE;
- }
) {
- return dataDiscoveryRepository.suggest(field, StringFunctions.asUTF8(text), fetchSize, criteria);
- }
+ return dataDiscoveryRepository.suggest(field, StringFunctions.asUTF8(text), fetchSize, criteria);
+ }
+
- @ApiOperation("Search for data discovery documents")
+ @ApiOperation("Search for data discovery documents")
@PostMapping(value = "/search", consumes = APPLICATION_JSON_VALUE)
public DataDiscoveryResponse search(
@RequestBody @Valid DataDiscoveryCriteriaImpl criteria
diff --git a/backend/src/main/java/fr/inra/urgi/gpds/domain/criteria/base/MaxResultWindowPagination.java b/backend/src/main/java/fr/inra/urgi/faidare/domain/criteria/base/MaxResultWindowPagination.java
similarity index 95%
rename from backend/src/main/java/fr/inra/urgi/gpds/domain/criteria/base/MaxResultWindowPagination.java
rename to backend/src/main/java/fr/inra/urgi/faidare/domain/criteria/base/MaxResultWindowPagination.java
index c55fb0f7..2fc880f6 100644
--- a/backend/src/main/java/fr/inra/urgi/gpds/domain/criteria/base/MaxResultWindowPagination.java
+++ b/backend/src/main/java/fr/inra/urgi/faidare/domain/criteria/base/MaxResultWindowPagination.java
@@ -1,4 +1,4 @@
-package fr.inra.urgi.gpds.domain.criteria.base;
+package fr.inra.urgi.faidare.domain.criteria.base;
import javax.validation.Constraint;
import javax.validation.Payload;
diff --git a/backend/src/main/java/fr/inra/urgi/faidare/elasticsearch/repository/impl/BaseESRepository.java b/backend/src/main/java/fr/inra/urgi/faidare/elasticsearch/repository/impl/BaseESRepository.java
index 81d7d317..aa3b6796 100644
--- a/backend/src/main/java/fr/inra/urgi/faidare/elasticsearch/repository/impl/BaseESRepository.java
+++ b/backend/src/main/java/fr/inra/urgi/faidare/elasticsearch/repository/impl/BaseESRepository.java
@@ -4,6 +4,7 @@ import fr.inra.urgi.faidare.domain.criteria.base.PaginationCriteria;
import fr.inra.urgi.faidare.domain.response.PaginatedList;
import fr.inra.urgi.faidare.elasticsearch.ESRequestFactory;
import fr.inra.urgi.faidare.elasticsearch.ESResponseParser;
+import fr.inra.urgi.faidare.elasticsearch.query.impl.ESGenericQueryFactory;
import fr.inra.urgi.faidare.elasticsearch.repository.ESFindRepository;
import fr.inra.urgi.faidare.elasticsearch.repository.ESGetByIdRepository;
import org.elasticsearch.client.RestHighLevelClient;
diff --git a/backend/src/main/java/fr/inra/urgi/faidare/elasticsearch/repository/impl/ESGenericFindRepository.java b/backend/src/main/java/fr/inra/urgi/faidare/elasticsearch/repository/impl/ESGenericFindRepository.java
index a3ee3408..f55cdf1d 100644
--- a/backend/src/main/java/fr/inra/urgi/faidare/elasticsearch/repository/impl/ESGenericFindRepository.java
+++ b/backend/src/main/java/fr/inra/urgi/faidare/elasticsearch/repository/impl/ESGenericFindRepository.java
@@ -2,18 +2,14 @@ package fr.inra.urgi.faidare.elasticsearch.repository.impl;
import com.google.common.cache.Cache;
import com.google.common.cache.CacheBuilder;
-import fr.inra.urgi.gpds.domain.criteria.base.PaginationCriteria;
-import fr.inra.urgi.gpds.domain.response.PaginatedList;
-import fr.inra.urgi.gpds.domain.response.Pagination;
-import fr.inra.urgi.gpds.domain.response.PaginationImpl;
-import fr.inra.urgi.gpds.elasticsearch.ESRequestFactory;
-import fr.inra.urgi.gpds.elasticsearch.ESResponseParser;
-import fr.inra.urgi.gpds.elasticsearch.query.ESQueryFactory;
-import fr.inra.urgi.gpds.elasticsearch.repository.ESFindRepository;
-import fr.inra.urgi.faidare.domain.criteria.base.SortCriteria;
-import fr.inra.urgi.faidare.elasticsearch.document.DocumentAnnotationUtil;
-import fr.inra.urgi.faidare.elasticsearch.document.DocumentMetadata;
-import fr.inra.urgi.faidare.elasticsearch.query.impl.ESGenericQueryFactory;
+import fr.inra.urgi.faidare.domain.criteria.base.PaginationCriteria;
+import fr.inra.urgi.faidare.domain.response.PaginatedList;
+import fr.inra.urgi.faidare.domain.response.Pagination;
+import fr.inra.urgi.faidare.domain.response.PaginationImpl;
+import fr.inra.urgi.faidare.elasticsearch.ESRequestFactory;
+import fr.inra.urgi.faidare.elasticsearch.ESResponseParser;
+import fr.inra.urgi.faidare.elasticsearch.query.ESQueryFactory;
+import fr.inra.urgi.faidare.elasticsearch.repository.ESFindRepository;
import org.elasticsearch.action.search.SearchRequest;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.client.RequestOptions;
@@ -30,7 +26,7 @@ import java.util.Objects;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
-import static fr.inra.urgi.gpds.domain.criteria.base.MaxResultWindowPagination.MAX_RESULT_WINDOW;
+import static fr.inra.urgi.faidare.domain.criteria.base.MaxResultWindowPagination.MAX_RESULT_WINDOW;
/**
* Generic implementation of an ElasticSearch document repository which can find documents by criteria.
diff --git a/backend/src/main/resources/application.yml b/backend/src/main/resources/application.yml
index 8f989fcc..e0f33da1 100644
--- a/backend/src/main/resources/application.yml
+++ b/backend/src/main/resources/application.yml
@@ -66,11 +66,6 @@ faidare:
url: http://tropgenedb.cirad.fr
name: CIRAD TropGENE
image: https://urgi.versailles.inra.fr/files/faidare/logo/CIRAD.jpg
- # EVA
- - uri: https://www.ebi.ac.uk/eva
- url: https://www.ebi.ac.uk/eva
- name: EVA
- image: https://urgi.versailles.inra.fr/files/faidare/logo/EVA.png
# ENA
- uri: https://www.ebi.ac.uk/eva
url: https://www.ebi.ac.uk/eva
--
GitLab
From 44be382e211ee95ebbeb1346b019cf43b2b32aad Mon Sep 17 00:00:00 2001
From: jdestin <jeremy.destin@inra.fr>
Date: Fri, 21 Feb 2020 16:50:15 +0100
Subject: [PATCH 3/3] fix: Fix backend tests.GNP-5623
---
.../api/faidare}/v1/DataDiscoveryControllerTest.java | 12 ++++--------
.../criteria/base/PaginationValidatorTest.java | 2 +-
2 files changed, 5 insertions(+), 9 deletions(-)
rename backend/src/test/java/fr/inra/urgi/{gpds/api/gnpis => faidare/api/faidare}/v1/DataDiscoveryControllerTest.java (82%)
diff --git a/backend/src/test/java/fr/inra/urgi/gpds/api/gnpis/v1/DataDiscoveryControllerTest.java b/backend/src/test/java/fr/inra/urgi/faidare/api/faidare/v1/DataDiscoveryControllerTest.java
similarity index 82%
rename from backend/src/test/java/fr/inra/urgi/gpds/api/gnpis/v1/DataDiscoveryControllerTest.java
rename to backend/src/test/java/fr/inra/urgi/faidare/api/faidare/v1/DataDiscoveryControllerTest.java
index f6a29afd..6ec3b6e1 100644
--- a/backend/src/test/java/fr/inra/urgi/gpds/api/gnpis/v1/DataDiscoveryControllerTest.java
+++ b/backend/src/test/java/fr/inra/urgi/faidare/api/faidare/v1/DataDiscoveryControllerTest.java
@@ -1,9 +1,8 @@
-package fr.inra.urgi.gpds.api.gnpis.v1;
+package fr.inra.urgi.faidare.api.faidare.v1;
import com.fasterxml.jackson.databind.ObjectMapper;
-import fr.inra.urgi.gpds.domain.datadiscovery.criteria.DataDiscoveryCriteriaImpl;
-import fr.inra.urgi.gpds.repository.es.DataDiscoveryRepository;
-import fr.inra.urgi.gpds.repository.file.DataSourceRepository;
+import fr.inra.urgi.faidare.domain.datadiscovery.criteria.DataDiscoveryCriteriaImpl;
+import fr.inra.urgi.faidare.repository.es.DataDiscoveryRepository;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.springframework.beans.factory.annotation.Autowired;
@@ -31,9 +30,6 @@ public class DataDiscoveryControllerTest {
@MockBean
private DataDiscoveryRepository repository;
- @MockBean
- private DataSourceRepository dataSourceRepository;
-
@Test
void should_Throw_Pagination_Max_Size_Exception() throws Exception {
DataDiscoveryCriteriaImpl criteria = new DataDiscoveryCriteriaImpl();
@@ -41,7 +37,7 @@ public class DataDiscoveryControllerTest {
criteria.setPageSize(1000L);
MvcResult res = mockMvc.perform(
- post("/gnpis/v1/datadiscovery/search")
+ post("/faidare/v1/datadiscovery/search")
.contentType(MediaType.APPLICATION_JSON_UTF8)
.content(new ObjectMapper().writeValueAsString(criteria))
).andExpect(status().isBadRequest()).andReturn();
diff --git a/backend/src/test/java/fr/inra/urgi/faidare/domain/criteria/base/PaginationValidatorTest.java b/backend/src/test/java/fr/inra/urgi/faidare/domain/criteria/base/PaginationValidatorTest.java
index 2fed235a..4cc9bd4b 100644
--- a/backend/src/test/java/fr/inra/urgi/faidare/domain/criteria/base/PaginationValidatorTest.java
+++ b/backend/src/test/java/fr/inra/urgi/faidare/domain/criteria/base/PaginationValidatorTest.java
@@ -1,6 +1,6 @@
package fr.inra.urgi.faidare.domain.criteria.base;
-import fr.inra.urgi.gpds.domain.datadiscovery.criteria.DataDiscoveryCriteriaImpl;
+import fr.inra.urgi.faidare.domain.datadiscovery.criteria.DataDiscoveryCriteriaImpl;
import org.assertj.core.api.Assertions;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
--
GitLab