diff --git a/src/main/java/org/springframework/data/keyvalue/core/AbstractKeyValueAdapter.java b/src/main/java/org/springframework/data/keyvalue/core/AbstractKeyValueAdapter.java index e4a8531..615e4d1 100644 --- a/src/main/java/org/springframework/data/keyvalue/core/AbstractKeyValueAdapter.java +++ b/src/main/java/org/springframework/data/keyvalue/core/AbstractKeyValueAdapter.java @@ -16,6 +16,7 @@ package org.springframework.data.keyvalue.core; import java.util.Collection; +import java.util.Comparator; import org.springframework.data.keyvalue.core.query.KeyValueQuery; import org.springframework.lang.Nullable; @@ -35,7 +36,17 @@ public abstract class AbstractKeyValueAdapter implements KeyValueAdapter { * Creates new {@link AbstractKeyValueAdapter} with using the default query engine. */ protected AbstractKeyValueAdapter() { - this(null); + this((QueryEngine) null); + } + + /** + * Creates new {@link AbstractKeyValueAdapter} with using the default query engine and provided comparator for sorting. + * + * @param sortAccessor must not be {@literal null}. + * @since 3.1.10 + */ + protected AbstractKeyValueAdapter(SortAccessor> sortAccessor) { + this(new SpelQueryEngine(sortAccessor)); } /** diff --git a/src/main/java/org/springframework/data/keyvalue/core/PathSortAccessor.java b/src/main/java/org/springframework/data/keyvalue/core/PathSortAccessor.java new file mode 100644 index 0000000..1f02ca7 --- /dev/null +++ b/src/main/java/org/springframework/data/keyvalue/core/PathSortAccessor.java @@ -0,0 +1,69 @@ +/* + * Copyright 2024 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.keyvalue.core; + +import java.util.Comparator; +import java.util.Optional; + +import org.springframework.data.domain.Sort.Direction; +import org.springframework.data.domain.Sort.NullHandling; +import org.springframework.data.domain.Sort.Order; +import org.springframework.data.keyvalue.core.query.KeyValueQuery; +import org.springframework.lang.Nullable; + +/** + * @author Christoph Strobl + * @since 3.1.10 + */ +public class PathSortAccessor implements SortAccessor> { + + @Nullable + @Override + public Comparator resolve(KeyValueQuery query) { + + if (query.getSort().isUnsorted()) { + return null; + } + + Optional> comparator = Optional.empty(); + for (Order order : query.getSort()) { + + PropertyPathComparator pathSort = new PropertyPathComparator<>(order.getProperty()); + + if (Direction.DESC.equals(order.getDirection())) { + + pathSort.desc(); + + if (!NullHandling.NATIVE.equals(order.getNullHandling())) { + pathSort = NullHandling.NULLS_FIRST.equals(order.getNullHandling()) ? pathSort.nullsFirst() + : pathSort.nullsLast(); + } + } + + if (!comparator.isPresent()) { + comparator = Optional.of(pathSort); + } else { + + PropertyPathComparator pathSortToUse = pathSort; + comparator = comparator.map(it -> it.thenComparing(pathSortToUse)); + } + } + + return comparator.orElseThrow( + () -> new IllegalStateException("No sort definitions have been added to this CompoundComparator to compare")); + + } +} diff --git a/src/main/java/org/springframework/data/keyvalue/core/PropertyPathComparator.java b/src/main/java/org/springframework/data/keyvalue/core/PropertyPathComparator.java new file mode 100644 index 0000000..9ad5152 --- /dev/null +++ b/src/main/java/org/springframework/data/keyvalue/core/PropertyPathComparator.java @@ -0,0 +1,104 @@ +/* + * Copyright 2024 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.keyvalue.core; + +import java.util.Comparator; +import java.util.HashMap; +import java.util.Map; + +import org.springframework.data.mapping.PropertyPath; +import org.springframework.data.util.Lazy; +import org.springframework.util.comparator.NullSafeComparator; + +/** + * @author Christoph Strobl + * @since 3.1.10 + */ +public class PropertyPathComparator implements Comparator { + + private final String path; + + private boolean asc = true; + private boolean nullsFirst = true; + + private final Map, PropertyPath> pathCache = new HashMap<>(2); + private Lazy> comparator = Lazy + .of(() -> new NullSafeComparator(Comparator.naturalOrder(), this.nullsFirst)); + + public PropertyPathComparator(String path) { + this.path = path; + } + + @Override + public int compare(T o1, T o2) { + + if (o1 == null && o2 == null) { + return 0; + } + if (o1 == null) { + return nullsFirst ? 1 : -1; + } + if (o2 == null) { + return nullsFirst ? 1 : -1; + } + + PropertyPath propertyPath = pathCache.computeIfAbsent(o1.getClass(), it -> PropertyPath.from(path, it)); + Object value1 = new SimplePropertyPathAccessor<>(o1).getValue(propertyPath); + Object value2 = new SimplePropertyPathAccessor<>(o2).getValue(propertyPath); + + return comparator.get().compare(value1, value2) * (asc ? 1 : -1); + } + + /** + * Sort {@literal ascending}. + * + * @return + */ + public PropertyPathComparator asc() { + this.asc = true; + return this; + } + + /** + * Sort {@literal descending}. + * + * @return + */ + public PropertyPathComparator desc() { + this.asc = false; + return this; + } + + /** + * Sort {@literal null} values first. + * + * @return + */ + public PropertyPathComparator nullsFirst() { + this.nullsFirst = true; + return this; + } + + /** + * Sort {@literal null} values last. + * + * @return + */ + public PropertyPathComparator nullsLast() { + this.nullsFirst = false; + return this; + } +} diff --git a/src/main/java/org/springframework/data/keyvalue/core/SimplePropertyPathAccessor.java b/src/main/java/org/springframework/data/keyvalue/core/SimplePropertyPathAccessor.java new file mode 100644 index 0000000..ac45429 --- /dev/null +++ b/src/main/java/org/springframework/data/keyvalue/core/SimplePropertyPathAccessor.java @@ -0,0 +1,49 @@ +/* + * Copyright 2024 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.keyvalue.core; + +import org.springframework.beans.BeanWrapper; +import org.springframework.data.mapping.PropertyPath; +import org.springframework.data.util.DirectFieldAccessFallbackBeanWrapper; + +/** + * @author Christoph Strobl + * @since 3.1.10 + */ +class SimplePropertyPathAccessor { + + private final Object root; + + public SimplePropertyPathAccessor(Object source) { + this.root = source; + } + + Object getValue(PropertyPath path) { + + Object currentValue = root; + for (PropertyPath current : path) { + currentValue = wrap(currentValue).getPropertyValue(current.getSegment()); + if (currentValue == null) { + break; + } + } + return currentValue; + } + + BeanWrapper wrap(Object o) { + return new DirectFieldAccessFallbackBeanWrapper(o); + } +} diff --git a/src/main/java/org/springframework/data/keyvalue/core/SpelQueryEngine.java b/src/main/java/org/springframework/data/keyvalue/core/SpelQueryEngine.java index 78bb266..7655289 100644 --- a/src/main/java/org/springframework/data/keyvalue/core/SpelQueryEngine.java +++ b/src/main/java/org/springframework/data/keyvalue/core/SpelQueryEngine.java @@ -44,7 +44,16 @@ class SpelQueryEngine extends QueryEngine> sortAccessor) { + super(new SpelCriteriaAccessor(PARSER), sortAccessor); } @Override diff --git a/src/main/java/org/springframework/data/map/MapKeyValueAdapter.java b/src/main/java/org/springframework/data/map/MapKeyValueAdapter.java index 2eb18db..9c204e0 100644 --- a/src/main/java/org/springframework/data/map/MapKeyValueAdapter.java +++ b/src/main/java/org/springframework/data/map/MapKeyValueAdapter.java @@ -16,6 +16,7 @@ package org.springframework.data.map; import java.util.Collection; +import java.util.Comparator; import java.util.Map; import java.util.Map.Entry; import java.util.concurrent.ConcurrentHashMap; @@ -25,6 +26,7 @@ import org.springframework.data.keyvalue.core.AbstractKeyValueAdapter; import org.springframework.data.keyvalue.core.ForwardingCloseableIterator; import org.springframework.data.keyvalue.core.KeyValueAdapter; import org.springframework.data.keyvalue.core.QueryEngine; +import org.springframework.data.keyvalue.core.SortAccessor; import org.springframework.data.util.CloseableIterator; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; @@ -69,6 +71,23 @@ public class MapKeyValueAdapter extends AbstractKeyValueAdapter { this(CollectionFactory.createMap(mapType, 100), mapType, null); } + /** + * Creates a new {@link MapKeyValueAdapter} using the given {@link Map} as backing store. + * + * @param mapType must not be {@literal null}. + * @param sortAccessor accessor granting access to sorting implementation + * @since 3.1.10 + */ + public MapKeyValueAdapter(Class mapType, SortAccessor> sortAccessor) { + + super(sortAccessor); + + Assert.notNull(mapType, "Store must not be null"); + + this.store = CollectionFactory.createMap(mapType, 100); + this.keySpaceMapType = (Class) ClassUtils.getUserClass(store); + } + /** * Creates a new {@link MapKeyValueAdapter} using the given {@link Map} as backing store and query engine. * diff --git a/src/main/java/org/springframework/data/map/repository/config/EnableMapRepositories.java b/src/main/java/org/springframework/data/map/repository/config/EnableMapRepositories.java index a70a940..74f3230 100644 --- a/src/main/java/org/springframework/data/map/repository/config/EnableMapRepositories.java +++ b/src/main/java/org/springframework/data/map/repository/config/EnableMapRepositories.java @@ -30,6 +30,7 @@ import org.springframework.context.annotation.ComponentScan.Filter; import org.springframework.context.annotation.Import; import org.springframework.data.keyvalue.core.KeyValueOperations; import org.springframework.data.keyvalue.core.KeyValueTemplate; +import org.springframework.data.keyvalue.core.SortAccessor; import org.springframework.data.keyvalue.repository.config.QueryCreatorType; import org.springframework.data.keyvalue.repository.query.CachingKeyValuePartTreeQuery; import org.springframework.data.keyvalue.repository.query.SpelQueryCreator; @@ -143,4 +144,12 @@ public @interface EnableMapRepositories { */ @SuppressWarnings("rawtypes") Class mapType() default ConcurrentHashMap.class; + + /** + * Configures the {@link SortAccessor accessor} for sorting results. + * + * @return {@link SortAccessor} to indicate usage of default implementation. + * @since 3.1.10 + */ + Class sortAccessor() default SortAccessor.class; } diff --git a/src/main/java/org/springframework/data/map/repository/config/MapRepositoryConfigurationExtension.java b/src/main/java/org/springframework/data/map/repository/config/MapRepositoryConfigurationExtension.java index dfa9762..c8a6316 100644 --- a/src/main/java/org/springframework/data/map/repository/config/MapRepositoryConfigurationExtension.java +++ b/src/main/java/org/springframework/data/map/repository/config/MapRepositoryConfigurationExtension.java @@ -17,15 +17,18 @@ package org.springframework.data.map.repository.config; import java.util.Map; +import org.springframework.beans.BeanUtils; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.support.AbstractBeanDefinition; import org.springframework.beans.factory.support.BeanDefinitionBuilder; import org.springframework.core.type.AnnotationMetadata; import org.springframework.data.config.ParsingUtils; import org.springframework.data.keyvalue.core.KeyValueTemplate; +import org.springframework.data.keyvalue.core.SortAccessor; import org.springframework.data.keyvalue.repository.config.KeyValueRepositoryConfigurationExtension; import org.springframework.data.map.MapKeyValueAdapter; import org.springframework.data.repository.config.RepositoryConfigurationSource; +import org.springframework.lang.Nullable; /** * @author Christoph Strobl @@ -54,6 +57,11 @@ public class MapRepositoryConfigurationExtension extends KeyValueRepositoryConfi BeanDefinitionBuilder adapterBuilder = BeanDefinitionBuilder.rootBeanDefinition(MapKeyValueAdapter.class); adapterBuilder.addConstructorArgValue(getMapTypeToUse(configurationSource)); + SortAccessor sortAccessor = getSortAccessor(configurationSource); + if(sortAccessor != null) { + adapterBuilder.addConstructorArgValue(sortAccessor); + } + BeanDefinitionBuilder builder = BeanDefinitionBuilder.rootBeanDefinition(KeyValueTemplate.class); builder .addConstructorArgValue(ParsingUtils.getSourceBeanDefinition(adapterBuilder, configurationSource.getSource())); @@ -68,4 +76,17 @@ public class MapRepositoryConfigurationExtension extends KeyValueRepositoryConfi return (Class) ((AnnotationMetadata) source.getSource()).getAnnotationAttributes( EnableMapRepositories.class.getName()).get("mapType"); } + + @Nullable + private static SortAccessor getSortAccessor(RepositoryConfigurationSource source) { + + Class> sortAccessorType = (Class>) ((AnnotationMetadata) source.getSource()).getAnnotationAttributes( + EnableMapRepositories.class.getName()).get("sortAccessor"); + + if(sortAccessorType != null && !sortAccessorType.isInterface()) { + return BeanUtils.instantiateClass(sortAccessorType); + } + + return null; + } } diff --git a/src/test/java/org/springframework/data/keyvalue/core/PropertyPathComparatorUnitTests.java b/src/test/java/org/springframework/data/keyvalue/core/PropertyPathComparatorUnitTests.java new file mode 100644 index 0000000..d5df69a --- /dev/null +++ b/src/test/java/org/springframework/data/keyvalue/core/PropertyPathComparatorUnitTests.java @@ -0,0 +1,186 @@ +/* + * Copyright 2024 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.keyvalue.core; + +import static org.assertj.core.api.Assertions.*; + +import java.util.Comparator; + +import org.junit.jupiter.api.Test; + +/** + * Unit tests for {@link PropertyPathComparator}. + * + * @author Christoph Strobl + */ +class PropertyPathComparatorUnitTests { + + private static final SomeType ONE = new SomeType("one", 1, 1); + private static final SomeType TWO = new SomeType("two", 2, 2); + private static final WrapperType WRAPPER_ONE = new WrapperType("w-one", ONE); + private static final WrapperType WRAPPER_TWO = new WrapperType("w-two", TWO); + + @Test // DATACMNS-525 + void shouldCompareStringAscCorrectly() { + + Comparator comparator = new PropertyPathComparator<>("stringProperty"); + assertThat(comparator.compare(ONE, TWO)).isEqualTo(ONE.getStringProperty().compareTo(TWO.getStringProperty())); + } + + @Test // DATACMNS-525 + void shouldCompareStringDescCorrectly() { + + Comparator comparator = new PropertyPathComparator("stringProperty").desc(); + assertThat(comparator.compare(ONE, TWO)).isEqualTo(TWO.getStringProperty().compareTo(ONE.getStringProperty())); + } + + @Test // DATACMNS-525 + void shouldCompareIntegerAscCorrectly() { + + Comparator comparator = new PropertyPathComparator<>("integerProperty"); + assertThat(comparator.compare(ONE, TWO)).isEqualTo(ONE.getIntegerProperty().compareTo(TWO.getIntegerProperty())); + } + + @Test // DATACMNS-525 + void shouldCompareIntegerDescCorrectly() { + + Comparator comparator = new PropertyPathComparator("integerProperty").desc(); + assertThat(comparator.compare(ONE, TWO)).isEqualTo(TWO.getIntegerProperty().compareTo(ONE.getIntegerProperty())); + } + + @Test // DATACMNS-525 + void shouldComparePrimitiveIntegerAscCorrectly() { + + Comparator comparator = new PropertyPathComparator<>("primitiveProperty"); + assertThat(comparator.compare(ONE, TWO)) + .isEqualTo(Integer.compare(ONE.getPrimitiveProperty(), TWO.getPrimitiveProperty())); + } + + @Test // DATACMNS-525 + void shouldNotFailOnNullValues() { + + Comparator comparator = new PropertyPathComparator<>("stringProperty"); + assertThat(comparator.compare(ONE, new SomeType(null, null, 2))).isEqualTo(1); + } + + @Test // DATACMNS-525 + void shouldComparePrimitiveIntegerDescCorrectly() { + + Comparator comparator = new PropertyPathComparator("primitiveProperty").desc(); + assertThat(comparator.compare(ONE, TWO)) + .isEqualTo(Integer.compare(TWO.getPrimitiveProperty(), ONE.getPrimitiveProperty())); + } + + @Test // DATACMNS-525 + void shouldSortNullsFirstCorrectly() { + Comparator comparator = new PropertyPathComparator("stringProperty").nullsFirst(); + assertThat(comparator.compare(ONE, new SomeType(null, null, 2))).isEqualTo(1); + } + + @Test // DATACMNS-525 + void shouldSortNullsLastCorrectly() { + + Comparator comparator = new PropertyPathComparator("stringProperty").nullsLast(); + assertThat(comparator.compare(ONE, new SomeType(null, null, 2))).isEqualTo(-1); + } + + @Test // DATACMNS-525 + void shouldCompareNestedTypesCorrectly() { + + Comparator comparator = new PropertyPathComparator<>("nestedType.stringProperty"); + assertThat(comparator.compare(WRAPPER_ONE, WRAPPER_TWO)).isEqualTo( + WRAPPER_ONE.getNestedType().getStringProperty().compareTo(WRAPPER_TWO.getNestedType().getStringProperty())); + } + + @Test // DATACMNS-525 + void shouldCompareNestedTypesCorrectlyWhenOneOfThemHasNullValue() { + + PropertyPathComparator comparator = new PropertyPathComparator<>("nestedType.stringProperty"); + assertThat(comparator.compare(WRAPPER_ONE, new WrapperType("two", null))).isGreaterThanOrEqualTo(1); + } + + public static class WrapperType { + + private String stringPropertyWrapper; + private SomeType nestedType; + + WrapperType(String stringPropertyWrapper, SomeType nestedType) { + this.stringPropertyWrapper = stringPropertyWrapper; + this.nestedType = nestedType; + } + + public String getStringPropertyWrapper() { + return stringPropertyWrapper; + } + + public void setStringPropertyWrapper(String stringPropertyWrapper) { + this.stringPropertyWrapper = stringPropertyWrapper; + } + + public SomeType getNestedType() { + return nestedType; + } + + public void setNestedType(SomeType nestedType) { + this.nestedType = nestedType; + } + + } + + @SuppressWarnings("WeakerAccess") + public static class SomeType { + + public SomeType() { + + } + + SomeType(String stringProperty, Integer integerProperty, int primitiveProperty) { + this.stringProperty = stringProperty; + this.integerProperty = integerProperty; + this.primitiveProperty = primitiveProperty; + } + + String stringProperty; + Integer integerProperty; + int primitiveProperty; + + public String getStringProperty() { + return stringProperty; + } + + public void setStringProperty(String stringProperty) { + this.stringProperty = stringProperty; + } + + public Integer getIntegerProperty() { + return integerProperty; + } + + public void setIntegerProperty(Integer integerProperty) { + this.integerProperty = integerProperty; + } + + public int getPrimitiveProperty() { + return primitiveProperty; + } + + public void setPrimitiveProperty(int primitiveProperty) { + this.primitiveProperty = primitiveProperty; + } + + } + +} diff --git a/src/test/java/org/springframework/data/map/repository/config/MapRepositoriesConfigurationExtensionIntegrationTests.java b/src/test/java/org/springframework/data/map/repository/config/MapRepositoriesConfigurationExtensionIntegrationTests.java index ec24239..b6acc7b 100644 --- a/src/test/java/org/springframework/data/map/repository/config/MapRepositoriesConfigurationExtensionIntegrationTests.java +++ b/src/test/java/org/springframework/data/map/repository/config/MapRepositoriesConfigurationExtensionIntegrationTests.java @@ -22,6 +22,7 @@ import java.util.Arrays; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentSkipListMap; +import org.assertj.core.api.InstanceOfAssertFactories; import org.junit.jupiter.api.Test; import org.springframework.context.ApplicationContext; import org.springframework.context.ConfigurableApplicationContext; @@ -34,6 +35,7 @@ import org.springframework.data.annotation.Id; import org.springframework.data.keyvalue.core.KeyValueAdapter; import org.springframework.data.keyvalue.core.KeyValueOperations; import org.springframework.data.keyvalue.core.KeyValueTemplate; +import org.springframework.data.keyvalue.core.PathSortAccessor; import org.springframework.data.keyvalue.repository.KeyValueRepository; import org.springframework.data.map.MapKeyValueAdapter; import org.springframework.test.util.ReflectionTestUtils; @@ -93,6 +95,12 @@ class MapRepositoriesConfigurationExtensionIntegrationTests { ConfigWithCustomizedMapTypeAndExplicitDefinitionOfKeyValueTemplate.class)); } + @Test // GH-565 + void considersSortAccessorConfiguredOnAnnotation() { + assertKeyValueTemplateWithSortAccessorFor(PathSortAccessor.class, + new AnnotationConfigApplicationContext(ConfigWithCustomizedSortAccessor.class)); + } + private static void assertKeyValueTemplateWithAdapterFor(Class mapType, ApplicationContext context) { KeyValueTemplate template = context.getBean(KeyValueTemplate.class); @@ -102,6 +110,19 @@ class MapRepositoriesConfigurationExtensionIntegrationTests { assertThat(ReflectionTestUtils.getField(adapter, "store")).isInstanceOf(mapType); } + private static void assertKeyValueTemplateWithSortAccessorFor(Class sortAccessorType, ApplicationContext context) { + + KeyValueTemplate template = context.getBean(KeyValueTemplate.class); + Object adapter = ReflectionTestUtils.getField(template, "adapter"); + + assertThat(adapter).isInstanceOf(MapKeyValueAdapter.class); + + Object engine = ReflectionTestUtils.getField(adapter, "engine"); + Object sortAccessor = ReflectionTestUtils.getField(engine, "sortAccessor"); + + assertThat(sortAccessor).asInstanceOf(InstanceOfAssertFactories.OPTIONAL).containsInstanceOf(sortAccessorType); + } + @Configuration @EnableMapRepositories static class Config {} @@ -146,6 +167,9 @@ class MapRepositoriesConfigurationExtensionIntegrationTests { } } + @EnableMapRepositories(sortAccessor = PathSortAccessor.class) + static class ConfigWithCustomizedSortAccessor {} + interface PersonRepository extends KeyValueRepository { }