From db6f723441b3c88bb2daef84aa471449aa11c847 Mon Sep 17 00:00:00 2001 From: Junghoon Ban Date: Thu, 30 Nov 2023 10:37:04 +0900 Subject: [PATCH 1/3] Use pattern matching instead of type casting. --- .../client/elc/CriteriaFilterProcessor.java | 11 ++++--- .../elc/ElasticsearchExceptionTranslator.java | 3 +- .../elc/ReactiveElasticsearchTemplate.java | 3 +- .../MappingElasticsearchConverter.java | 13 ++++---- .../parser/ElasticsearchQueryCreator.java | 33 ++++++++++--------- 5 files changed, 34 insertions(+), 29 deletions(-) diff --git a/src/main/java/org/springframework/data/elasticsearch/client/elc/CriteriaFilterProcessor.java b/src/main/java/org/springframework/data/elasticsearch/client/elc/CriteriaFilterProcessor.java index 9b52c375e0..ed388c4710 100644 --- a/src/main/java/org/springframework/data/elasticsearch/client/elc/CriteriaFilterProcessor.java +++ b/src/main/java/org/springframework/data/elasticsearch/client/elc/CriteriaFilterProcessor.java @@ -50,6 +50,7 @@ * filter. * * @author Peter-Josef Meisch + * @author Junghoon Ban * @since 4.4 */ class CriteriaFilterProcessor { @@ -169,7 +170,7 @@ private static ObjectBuilder withinQuery(String fieldName, Obj Assert.isTrue(values[1] instanceof String || values[1] instanceof Distance, "Second element of a geo distance filter must be a text or a Distance"); - String dist = (values[1] instanceof Distance) ? extractDistanceString((Distance) values[1]) : (String) values[1]; + String dist = (values[1] instanceof Distance distance) ? extractDistanceString(distance) : (String) values[1]; return QueryBuilders.geoDistance() // .field(fieldName) // @@ -178,8 +179,8 @@ private static ObjectBuilder withinQuery(String fieldName, Obj .location(location -> { if (values[0]instanceof GeoPoint loc) { location.latlon(latlon -> latlon.lat(loc.getLat()).lon(loc.getLon())); - } else if (values[0] instanceof Point) { - GeoPoint loc = GeoPoint.fromPoint((Point) values[0]); + } else if (values[0] instanceof Point point) { + GeoPoint loc = GeoPoint.fromPoint(point); location.latlon(latlon -> latlon.lat(loc.getLat()).lon(loc.getLon())); } else { String loc = (String) values[0]; @@ -220,8 +221,8 @@ private static void oneParameterBBox(GeoBoundingBoxQuery.Builder queryBuilder, O "single-element of boundedBy filter must be type of GeoBox or Box"); GeoBox geoBBox; - if (value instanceof Box) { - geoBBox = GeoBox.fromBox((Box) value); + if (value instanceof Box box) { + geoBBox = GeoBox.fromBox(box); } else { geoBBox = (GeoBox) value; } diff --git a/src/main/java/org/springframework/data/elasticsearch/client/elc/ElasticsearchExceptionTranslator.java b/src/main/java/org/springframework/data/elasticsearch/client/elc/ElasticsearchExceptionTranslator.java index 2851190d93..b773506d71 100644 --- a/src/main/java/org/springframework/data/elasticsearch/client/elc/ElasticsearchExceptionTranslator.java +++ b/src/main/java/org/springframework/data/elasticsearch/client/elc/ElasticsearchExceptionTranslator.java @@ -40,6 +40,7 @@ * appropriate: any other exception may have resulted from user code, and should not be translated. * * @author Peter-Josef Meisch + * @author Junghoon Ban * @since 4.4 */ public class ElasticsearchExceptionTranslator implements PersistenceExceptionTranslator { @@ -59,7 +60,7 @@ public ElasticsearchExceptionTranslator(JsonpMapper jsonpMapper) { */ public RuntimeException translateException(Throwable throwable) { - RuntimeException runtimeException = throwable instanceof RuntimeException ? (RuntimeException) throwable + RuntimeException runtimeException = throwable instanceof RuntimeException ex ? ex : new RuntimeException(throwable.getMessage(), throwable); RuntimeException potentiallyTranslatedException = translateExceptionIfPossible(runtimeException); diff --git a/src/main/java/org/springframework/data/elasticsearch/client/elc/ReactiveElasticsearchTemplate.java b/src/main/java/org/springframework/data/elasticsearch/client/elc/ReactiveElasticsearchTemplate.java index 06b177b51a..1d60ab94a6 100644 --- a/src/main/java/org/springframework/data/elasticsearch/client/elc/ReactiveElasticsearchTemplate.java +++ b/src/main/java/org/springframework/data/elasticsearch/client/elc/ReactiveElasticsearchTemplate.java @@ -79,6 +79,7 @@ * * @author Peter-Josef Meisch * @author Illia Ulianov + * @author Junghoon Ban * @since 4.4 */ public class ReactiveElasticsearchTemplate extends AbstractReactiveElasticsearchTemplate { @@ -645,7 +646,7 @@ public Publisher execute(ReactiveElasticsearchTemplate.ClientCallback getCollectionComponentType(TypeInformation type) { private Object propertyConverterRead(ElasticsearchPersistentProperty property, Object source) { PropertyValueConverter propertyValueConverter = Objects.requireNonNull(property.getPropertyValueConverter()); - if (source instanceof String[]) { + if (source instanceof String[] strings) { // convert to a List - source = Arrays.asList((String[]) source); + source = Arrays.asList(strings); } - if (source instanceof List) { - source = ((List) source).stream().map(it -> convertOnRead(propertyValueConverter, it)) + if (source instanceof List list) { + source = list.stream().map(it -> convertOnRead(propertyValueConverter, it)) .collect(Collectors.toList()); - } else if (source instanceof Set) { - source = ((Set) source).stream().map(it -> convertOnRead(propertyValueConverter, it)) + } else if (source instanceof Set set) { + source = set.stream().map(it -> convertOnRead(propertyValueConverter, it)) .collect(Collectors.toSet()); } else { source = convertOnRead(propertyValueConverter, source); diff --git a/src/main/java/org/springframework/data/elasticsearch/repository/query/parser/ElasticsearchQueryCreator.java b/src/main/java/org/springframework/data/elasticsearch/repository/query/parser/ElasticsearchQueryCreator.java index c8079922b6..217c9aaf49 100644 --- a/src/main/java/org/springframework/data/elasticsearch/repository/query/parser/ElasticsearchQueryCreator.java +++ b/src/main/java/org/springframework/data/elasticsearch/repository/query/parser/ElasticsearchQueryCreator.java @@ -44,6 +44,7 @@ * @author Franck Marchand * @author Artur Konczak * @author Peter-Josef Meisch + * @author Junghoon Ban */ public class ElasticsearchQueryCreator extends AbstractQueryCreator { @@ -154,37 +155,37 @@ private Criteria from(Part part, Criteria criteria, Iterator parameters) { secondParameter = parameters.next(); } - if (firstParameter instanceof GeoPoint && secondParameter instanceof String) - return criteria.within((GeoPoint) firstParameter, (String) secondParameter); + if (firstParameter instanceof GeoPoint geoPoint && secondParameter instanceof String string) + return criteria.within(geoPoint, string); - if (firstParameter instanceof Point && secondParameter instanceof Distance) - return criteria.within((Point) firstParameter, (Distance) secondParameter); + if (firstParameter instanceof Point point && secondParameter instanceof Distance distance) + return criteria.within(point, distance); - if (firstParameter instanceof String && secondParameter instanceof String) - return criteria.within((String) firstParameter, (String) secondParameter); + if (firstParameter instanceof String firstString && secondParameter instanceof String secondString) + return criteria.within(firstString, secondString); } case NEAR: { Object firstParameter = parameters.next(); - if (firstParameter instanceof GeoBox) { - return criteria.boundedBy((GeoBox) firstParameter); + if (firstParameter instanceof GeoBox geoBox) { + return criteria.boundedBy(geoBox); } - if (firstParameter instanceof Box) { - return criteria.boundedBy(GeoBox.fromBox((Box) firstParameter)); + if (firstParameter instanceof Box box) { + return criteria.boundedBy(GeoBox.fromBox(box)); } Object secondParameter = parameters.next(); // "near" query can be the same query as the "within" query - if (firstParameter instanceof GeoPoint && secondParameter instanceof String) - return criteria.within((GeoPoint) firstParameter, (String) secondParameter); + if (firstParameter instanceof GeoPoint geoPoint && secondParameter instanceof String string) + return criteria.within(geoPoint, string); - if (firstParameter instanceof Point && secondParameter instanceof Distance) - return criteria.within((Point) firstParameter, (Distance) secondParameter); + if (firstParameter instanceof Point point && secondParameter instanceof Distance distance) + return criteria.within(point, distance); - if (firstParameter instanceof String && secondParameter instanceof String) - return criteria.within((String) firstParameter, (String) secondParameter); + if (firstParameter instanceof String firstString && secondParameter instanceof String secondString) + return criteria.within(firstString, secondString); } case EXISTS: case IS_NOT_NULL: From b8493e51bab4b63047493ca74357ec3cef0627c0 Mon Sep 17 00:00:00 2001 From: Junghoon Ban Date: Thu, 30 Nov 2023 10:55:46 +0900 Subject: [PATCH 2/3] Create doWithinIfPossible method to remove duplicated logic. --- .../parser/ElasticsearchQueryCreator.java | 54 +++++++++++-------- 1 file changed, 32 insertions(+), 22 deletions(-) diff --git a/src/main/java/org/springframework/data/elasticsearch/repository/query/parser/ElasticsearchQueryCreator.java b/src/main/java/org/springframework/data/elasticsearch/repository/query/parser/ElasticsearchQueryCreator.java index 217c9aaf49..8aac05e271 100644 --- a/src/main/java/org/springframework/data/elasticsearch/repository/query/parser/ElasticsearchQueryCreator.java +++ b/src/main/java/org/springframework/data/elasticsearch/repository/query/parser/ElasticsearchQueryCreator.java @@ -17,7 +17,6 @@ import java.util.Collection; import java.util.Iterator; - import org.springframework.dao.InvalidDataAccessApiUsageException; import org.springframework.data.domain.Sort; import org.springframework.data.elasticsearch.core.geo.GeoBox; @@ -63,8 +62,8 @@ public ElasticsearchQueryCreator(PartTree tree, MappingContext iterator) { - PersistentPropertyPath path = context - .getPersistentPropertyPath(part.getProperty()); + PersistentPropertyPath path = context.getPersistentPropertyPath( + part.getProperty()); return new CriteriaQuery(from(part, new Criteria(path.toDotPath(ElasticsearchPersistentProperty.QueryPropertyToFieldNameConverter.INSTANCE)), iterator)); @@ -75,8 +74,8 @@ protected CriteriaQuery and(Part part, CriteriaQuery base, Iterator iter if (base == null) { return create(part, iterator); } - PersistentPropertyPath path = context - .getPersistentPropertyPath(part.getProperty()); + PersistentPropertyPath path = context.getPersistentPropertyPath( + part.getProperty()); return base.addCriteria(from(part, new Criteria(path.toDotPath(ElasticsearchPersistentProperty.QueryPropertyToFieldNameConverter.INSTANCE)), iterator)); @@ -155,14 +154,7 @@ private Criteria from(Part part, Criteria criteria, Iterator parameters) { secondParameter = parameters.next(); } - if (firstParameter instanceof GeoPoint geoPoint && secondParameter instanceof String string) - return criteria.within(geoPoint, string); - - if (firstParameter instanceof Point point && secondParameter instanceof Distance distance) - return criteria.within(point, distance); - - if (firstParameter instanceof String firstString && secondParameter instanceof String secondString) - return criteria.within(firstString, secondString); + return doWithinIfPossible(criteria, firstParameter, secondParameter); } case NEAR: { Object firstParameter = parameters.next(); @@ -177,15 +169,7 @@ private Criteria from(Part part, Criteria criteria, Iterator parameters) { Object secondParameter = parameters.next(); - // "near" query can be the same query as the "within" query - if (firstParameter instanceof GeoPoint geoPoint && secondParameter instanceof String string) - return criteria.within(geoPoint, string); - - if (firstParameter instanceof Point point && secondParameter instanceof Distance distance) - return criteria.within(point, distance); - - if (firstParameter instanceof String firstString && secondParameter instanceof String secondString) - return criteria.within(firstString, secondString); + return doWithinIfPossible(criteria, firstParameter, secondParameter); } case EXISTS: case IS_NOT_NULL: @@ -201,6 +185,32 @@ private Criteria from(Part part, Criteria criteria, Iterator parameters) { } } + /** + * Do a within query if possible, otherwise return the criteria unchanged. + * + * @param criteria must not be {@literal null} + * @param firstParameter must not be {@literal null} + * @param secondParameter must not be {@literal null} + * @return the criteria with the within query applied if possible. + * @author Junghoon Ban + */ + private Criteria doWithinIfPossible(Criteria criteria, Object firstParameter, Object secondParameter) { + + if (firstParameter instanceof GeoPoint geoPoint && secondParameter instanceof String string) { + return criteria.within(geoPoint, string); + } + + if (firstParameter instanceof Point point && secondParameter instanceof Distance distance) { + return criteria.within(point, distance); + } + + if (firstParameter instanceof String firstString && secondParameter instanceof String secondString) { + return criteria.within(firstString, secondString); + } + + return criteria; + } + private Object[] asArray(Object o) { if (o instanceof Collection) { return ((Collection) o).toArray(); From 1278a601ce51c8dd28bd036199d256c3944ac9d1 Mon Sep 17 00:00:00 2001 From: Junghoon Ban Date: Thu, 30 Nov 2023 10:58:29 +0900 Subject: [PATCH 3/3] Merge switch branches in ElasticsearchQueryCreator. --- .../query/parser/ElasticsearchQueryCreator.java | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/springframework/data/elasticsearch/repository/query/parser/ElasticsearchQueryCreator.java b/src/main/java/org/springframework/data/elasticsearch/repository/query/parser/ElasticsearchQueryCreator.java index 8aac05e271..600e02a8a2 100644 --- a/src/main/java/org/springframework/data/elasticsearch/repository/query/parser/ElasticsearchQueryCreator.java +++ b/src/main/java/org/springframework/data/elasticsearch/repository/query/parser/ElasticsearchQueryCreator.java @@ -109,8 +109,7 @@ private Criteria from(Part part, Criteria criteria, Iterator parameters) { return criteria.is(parameters.next()).not(); case REGEX: return criteria.expression(parameters.next().toString()); - case LIKE: - case STARTING_WITH: + case LIKE, STARTING_WITH: return criteria.startsWith(parameters.next().toString()); case ENDING_WITH: return criteria.endsWith(parameters.next().toString()); @@ -118,13 +117,11 @@ private Criteria from(Part part, Criteria criteria, Iterator parameters) { return criteria.contains(parameters.next().toString()); case GREATER_THAN: return criteria.greaterThan(parameters.next()); - case AFTER: - case GREATER_THAN_EQUAL: + case AFTER, GREATER_THAN_EQUAL: return criteria.greaterThanEqual(parameters.next()); case LESS_THAN: return criteria.lessThan(parameters.next()); - case BEFORE: - case LESS_THAN_EQUAL: + case BEFORE, LESS_THAN_EQUAL: return criteria.lessThanEqual(parameters.next()); case BETWEEN: return criteria.between(parameters.next(), parameters.next()); @@ -132,8 +129,7 @@ private Criteria from(Part part, Criteria criteria, Iterator parameters) { return criteria.in(asArray(parameters.next())); case NOT_IN: return criteria.notIn(asArray(parameters.next())); - case SIMPLE_PROPERTY: - case WITHIN: { + case SIMPLE_PROPERTY, WITHIN: { Object firstParameter = parameters.next(); Object secondParameter = null; if (type == Part.Type.SIMPLE_PROPERTY) { @@ -171,8 +167,7 @@ private Criteria from(Part part, Criteria criteria, Iterator parameters) { return doWithinIfPossible(criteria, firstParameter, secondParameter); } - case EXISTS: - case IS_NOT_NULL: + case EXISTS, IS_NOT_NULL: return criteria.exists(); case IS_NULL: return criteria.not().exists();