From 0e9dc7aec4d030c4f3d4e82e3d264181560e6876 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Thu, 13 Jun 2024 13:54:34 +0200 Subject: [PATCH] Polishing. Provide load-factor to sets with well-known sizing, move annotation auditing metadata cache to DefaultAuditableBeanWrapperFactory to avoid strong static references. See #3067 Original pull request: #3073 --- .../auditing/AnnotationAuditingMetadata.java | 6 +--- .../DefaultAuditableBeanWrapperFactory.java | 9 ++++-- .../data/geo/format/DistanceFormatter.java | 2 +- .../mapping/model/BasicPersistentEntity.java | 2 +- .../SpelEvaluatingMethodInterceptor.java | 2 +- .../QueryExecutorMethodInterceptor.java | 2 +- .../support/RepositoryFactorySupport.java | 28 +++++++++--------- ...eQueryMethodEvaluationContextProvider.java | 2 +- .../util/QueryExecutionConverters.java | 8 ++--- .../data/util/NullableWrapperConverters.java | 4 +-- .../springframework/data/util/Optionals.java | 29 +++++++++++-------- .../AnnotationAuditingMetadataUnitTests.java | 10 ------- ...gPropertyAccessorFactoryDatatypeTests.java | 8 ++--- 13 files changed, 54 insertions(+), 58 deletions(-) diff --git a/src/main/java/org/springframework/data/auditing/AnnotationAuditingMetadata.java b/src/main/java/org/springframework/data/auditing/AnnotationAuditingMetadata.java index ac580b1d8..16a894e27 100644 --- a/src/main/java/org/springframework/data/auditing/AnnotationAuditingMetadata.java +++ b/src/main/java/org/springframework/data/auditing/AnnotationAuditingMetadata.java @@ -21,9 +21,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.Date; import java.util.List; -import java.util.Map; import java.util.Optional; -import java.util.concurrent.ConcurrentHashMap; import org.springframework.data.annotation.CreatedBy; import org.springframework.data.annotation.CreatedDate; @@ -53,8 +51,6 @@ final class AnnotationAuditingMetadata { private static final AnnotationFieldFilter LAST_MODIFIED_DATE_FILTER = new AnnotationFieldFilter( LastModifiedDate.class); - private static final Map, AnnotationAuditingMetadata> metadataCache = new ConcurrentHashMap<>(); - static final List SUPPORTED_DATE_TYPES; static { @@ -126,7 +122,7 @@ final class AnnotationAuditingMetadata { * @param type the type to inspect, must not be {@literal null}. */ public static AnnotationAuditingMetadata getMetadata(Class type) { - return metadataCache.computeIfAbsent(type, AnnotationAuditingMetadata::new); + return new AnnotationAuditingMetadata(type); } /** diff --git a/src/main/java/org/springframework/data/auditing/DefaultAuditableBeanWrapperFactory.java b/src/main/java/org/springframework/data/auditing/DefaultAuditableBeanWrapperFactory.java index d838d6ba6..c7e260e78 100644 --- a/src/main/java/org/springframework/data/auditing/DefaultAuditableBeanWrapperFactory.java +++ b/src/main/java/org/springframework/data/auditing/DefaultAuditableBeanWrapperFactory.java @@ -19,7 +19,9 @@ import java.lang.reflect.Field; import java.time.Instant; import java.time.temporal.TemporalAccessor; import java.util.Date; +import java.util.Map; import java.util.Optional; +import java.util.concurrent.ConcurrentHashMap; import java.util.stream.Stream; import org.springframework.core.ResolvableType; @@ -44,6 +46,7 @@ import org.springframework.util.Assert; class DefaultAuditableBeanWrapperFactory implements AuditableBeanWrapperFactory { private final ConversionService conversionService; + private final Map, AnnotationAuditingMetadata> metadataCache; public DefaultAuditableBeanWrapperFactory() { @@ -52,6 +55,7 @@ class DefaultAuditableBeanWrapperFactory implements AuditableBeanWrapperFactory Jsr310Converters.getConvertersToRegister().forEach(conversionService::addConverter); this.conversionService = conversionService; + this.metadataCache = new ConcurrentHashMap<>(); } ConversionService getConversionService() { @@ -77,10 +81,11 @@ class DefaultAuditableBeanWrapperFactory implements AuditableBeanWrapperFactory (Auditable) it); } - AnnotationAuditingMetadata metadata = AnnotationAuditingMetadata.getMetadata(it.getClass()); + AnnotationAuditingMetadata metadata = metadataCache.computeIfAbsent(it.getClass(), + AnnotationAuditingMetadata::getMetadata); if (metadata.isAuditable()) { - return new ReflectionAuditingBeanWrapper(conversionService, it); + return new ReflectionAuditingBeanWrapper<>(conversionService, it); } return null; diff --git a/src/main/java/org/springframework/data/geo/format/DistanceFormatter.java b/src/main/java/org/springframework/data/geo/format/DistanceFormatter.java index a28626bd5..90a4648ea 100644 --- a/src/main/java/org/springframework/data/geo/format/DistanceFormatter.java +++ b/src/main/java/org/springframework/data/geo/format/DistanceFormatter.java @@ -47,7 +47,7 @@ public enum DistanceFormatter implements Converter, Formatter< static { - Map metrics = new LinkedHashMap<>(Metrics.values().length); + Map metrics = new LinkedHashMap<>(Metrics.values().length, 1.0f); for (Metric metric : Metrics.values()) { metrics.put(metric.getAbbreviation(), metric); diff --git a/src/main/java/org/springframework/data/mapping/model/BasicPersistentEntity.java b/src/main/java/org/springframework/data/mapping/model/BasicPersistentEntity.java index 9bf1b7676..ff612ed6f 100644 --- a/src/main/java/org/springframework/data/mapping/model/BasicPersistentEntity.java +++ b/src/main/java/org/springframework/data/mapping/model/BasicPersistentEntity.java @@ -115,7 +115,7 @@ public class BasicPersistentEntity> implement this.creator = InstanceCreatorMetadataDiscoverer.discover(this); this.associations = comparator == null ? new HashSet<>() : new TreeSet<>(new AssociationComparator<>(comparator)); - this.propertyCache = new HashMap<>(16, 1f); + this.propertyCache = new HashMap<>(16, 1.0f); this.annotationCache = new ConcurrentHashMap<>(16); this.propertyAnnotationCache = CollectionUtils .toMultiValueMap(new ConcurrentHashMap<>(16)); diff --git a/src/main/java/org/springframework/data/projection/SpelEvaluatingMethodInterceptor.java b/src/main/java/org/springframework/data/projection/SpelEvaluatingMethodInterceptor.java index 52be16cc7..21c6e24fa 100644 --- a/src/main/java/org/springframework/data/projection/SpelEvaluatingMethodInterceptor.java +++ b/src/main/java/org/springframework/data/projection/SpelEvaluatingMethodInterceptor.java @@ -108,7 +108,7 @@ class SpelEvaluatingMethodInterceptor implements MethodInterceptor { ExpressionParser parser, Class targetInterface) { Method[] methods = targetInterface.getMethods(); - Map expressions = new HashMap<>(methods.length); + Map expressions = new HashMap<>(methods.length, 1.0f); for (Method method : methods) { diff --git a/src/main/java/org/springframework/data/repository/core/support/QueryExecutorMethodInterceptor.java b/src/main/java/org/springframework/data/repository/core/support/QueryExecutorMethodInterceptor.java index 4b3f609da..0348cc4e4 100644 --- a/src/main/java/org/springframework/data/repository/core/support/QueryExecutorMethodInterceptor.java +++ b/src/main/java/org/springframework/data/repository/core/support/QueryExecutorMethodInterceptor.java @@ -93,7 +93,7 @@ class QueryExecutorMethodInterceptor implements MethodInterceptor { QueryLookupStrategy lookupStrategy, ProjectionFactory projectionFactory) { List queryMethods = repositoryInformation.getQueryMethods().toList(); - Map result = new HashMap<>(queryMethods.size()); + Map result = new HashMap<>(queryMethods.size(), 1.0f); for (Method method : queryMethods) { diff --git a/src/main/java/org/springframework/data/repository/core/support/RepositoryFactorySupport.java b/src/main/java/org/springframework/data/repository/core/support/RepositoryFactorySupport.java index 13274805a..b3babf465 100644 --- a/src/main/java/org/springframework/data/repository/core/support/RepositoryFactorySupport.java +++ b/src/main/java/org/springframework/data/repository/core/support/RepositoryFactorySupport.java @@ -23,13 +23,13 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; -import java.util.concurrent.ConcurrentHashMap; import java.util.stream.Collectors; import org.aopalliance.intercept.MethodInterceptor; import org.aopalliance.intercept.MethodInvocation; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; + import org.springframework.aop.framework.ProxyFactory; import org.springframework.aop.interceptor.ExposeInvocationInterceptor; import org.springframework.beans.BeanUtils; @@ -108,7 +108,7 @@ public abstract class RepositoryFactorySupport implements BeanClassLoaderAware, @SuppressWarnings("null") public RepositoryFactorySupport() { - this.repositoryInformationCache = new ConcurrentHashMap<>(16); + this.repositoryInformationCache = new HashMap<>(16); this.postProcessors = new ArrayList<>(); this.repositoryBaseClass = Optional.empty(); @@ -364,8 +364,7 @@ public abstract class RepositoryFactorySupport implements BeanClassLoaderAware, if (logger.isDebugEnabled()) { logger - .debug(LogMessage.format("Finished creation of repository instance for %s.", - repositoryInterface.getName())); + .debug(LogMessage.format("Finished creation of repository instance for %s.", repositoryInterface.getName())); } return repository; @@ -440,12 +439,15 @@ public abstract class RepositoryFactorySupport implements BeanClassLoaderAware, RepositoryInformationCacheKey cacheKey = new RepositoryInformationCacheKey(metadata, composition); - return repositoryInformationCache.computeIfAbsent(cacheKey, key -> { + synchronized (repositoryInformationCache) { - Class baseClass = repositoryBaseClass.orElse(getRepositoryBaseClass(metadata)); + return repositoryInformationCache.computeIfAbsent(cacheKey, key -> { - return new DefaultRepositoryInformation(metadata, baseClass, composition); - }); + Class baseClass = repositoryBaseClass.orElse(getRepositoryBaseClass(metadata)); + + return new DefaultRepositoryInformation(metadata, baseClass, composition); + }); + } } protected List getQueryMethods() { @@ -725,7 +727,7 @@ public abstract class RepositoryFactorySupport implements BeanClassLoaderAware, */ static class RepositoryValidator { - static Map, String> WELL_KNOWN_EXECUTORS = new HashMap<>(4, 1f); + static Map, String> WELL_KNOWN_EXECUTORS = new HashMap<>(4, 1.0f); static { @@ -786,11 +788,9 @@ public abstract class RepositoryFactorySupport implements BeanClassLoaderAware, } if (!containsFragmentImplementation(composition, executorInterface)) { - throw new UnsupportedFragmentException( - String.format("Repository %s implements %s but %s does not support %s", - ClassUtils.getQualifiedName(repositoryInterface), ClassUtils.getQualifiedName(executorInterface), - ClassUtils.getShortName(source), entry.getValue()), - repositoryInterface, executorInterface); + throw new UnsupportedFragmentException(String.format("Repository %s implements %s but %s does not support %s", + ClassUtils.getQualifiedName(repositoryInterface), ClassUtils.getQualifiedName(executorInterface), + ClassUtils.getShortName(source), entry.getValue()), repositoryInterface, executorInterface); } } } diff --git a/src/main/java/org/springframework/data/repository/query/ExtensionAwareQueryMethodEvaluationContextProvider.java b/src/main/java/org/springframework/data/repository/query/ExtensionAwareQueryMethodEvaluationContextProvider.java index 44cd3f655..2897872fe 100644 --- a/src/main/java/org/springframework/data/repository/query/ExtensionAwareQueryMethodEvaluationContextProvider.java +++ b/src/main/java/org/springframework/data/repository/query/ExtensionAwareQueryMethodEvaluationContextProvider.java @@ -100,7 +100,7 @@ public class ExtensionAwareQueryMethodEvaluationContextProvider implements Query */ static Map collectVariables(Parameters parameters, Object[] arguments) { - Map variables = new HashMap<>(parameters.getNumberOfParameters()); + Map variables = new HashMap<>(parameters.getNumberOfParameters(), 1.0f); parameters.stream()// .filter(Parameter::isSpecialParameter)// diff --git a/src/main/java/org/springframework/data/repository/util/QueryExecutionConverters.java b/src/main/java/org/springframework/data/repository/util/QueryExecutionConverters.java index 17aaf5647..f9f37fac0 100644 --- a/src/main/java/org/springframework/data/repository/util/QueryExecutionConverters.java +++ b/src/main/java/org/springframework/data/repository/util/QueryExecutionConverters.java @@ -63,7 +63,7 @@ import org.springframework.util.concurrent.ListenableFuture; *
  • {@code javaslang.collection.Seq}, {@code javaslang.collection.Map}, {@code javaslang.collection.Set} - as of * 1.13
    • *
  • {@code io.vavr.collection.Seq}, {@code io.vavr.collection.Map}, {@code io.vavr.collection.Set} - as of 2.0
    • - *
  • Reactive wrappers supported by {@link ReactiveWrappers} - as of 2.0
    • + *
  • Reactive wrappers supported by {@link org.springframework.data.util.ReactiveWrappers} - as of 2.0
    • * * * @author Oliver Gierke @@ -79,11 +79,11 @@ public abstract class QueryExecutionConverters { private static final boolean VAVR_PRESENT = ClassUtils.isPresent("io.vavr.control.Try", QueryExecutionConverters.class.getClassLoader()); - private static final Set WRAPPER_TYPES = new HashSet<>(10, 1f); - private static final Set UNWRAPPER_TYPES = new HashSet(10, 1f); + private static final Set WRAPPER_TYPES = new HashSet<>(10, 1.0f); + private static final Set UNWRAPPER_TYPES = new HashSet(10, 1.0f); private static final Set> UNWRAPPERS = new HashSet<>(); private static final Set> ALLOWED_PAGEABLE_TYPES = new HashSet<>(); - private static final Map, ExecutionAdapter> EXECUTION_ADAPTER = new HashMap<>(3, 1f); + private static final Map, ExecutionAdapter> EXECUTION_ADAPTER = new HashMap<>(3, 1.0f); private static final Map, Boolean> supportsCache = new ConcurrentReferenceHashMap<>(); private static final TypeInformation VOID_INFORMATION = TypeInformation.of(Void.class); diff --git a/src/main/java/org/springframework/data/util/NullableWrapperConverters.java b/src/main/java/org/springframework/data/util/NullableWrapperConverters.java index f83b877e4..044198c90 100644 --- a/src/main/java/org/springframework/data/util/NullableWrapperConverters.java +++ b/src/main/java/org/springframework/data/util/NullableWrapperConverters.java @@ -23,7 +23,6 @@ import java.util.Collections; import java.util.HashSet; import java.util.Map; import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; import java.util.stream.Stream; import org.springframework.core.convert.TypeDescriptor; @@ -33,6 +32,7 @@ import org.springframework.core.convert.converter.GenericConverter; import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; +import org.springframework.util.ConcurrentReferenceHashMap; import org.springframework.util.ObjectUtils; import com.google.common.base.Optional; @@ -67,7 +67,7 @@ public abstract class NullableWrapperConverters { private static final Set WRAPPER_TYPES = new HashSet(); private static final Set UNWRAPPER_TYPES = new HashSet(); private static final Set> UNWRAPPERS = new HashSet>(); - private static final Map, Boolean> supportsCache = new ConcurrentHashMap<>(); + private static final Map, Boolean> supportsCache = new ConcurrentReferenceHashMap<>(); static { diff --git a/src/main/java/org/springframework/data/util/Optionals.java b/src/main/java/org/springframework/data/util/Optionals.java index 71fefbde0..93aec782c 100644 --- a/src/main/java/org/springframework/data/util/Optionals.java +++ b/src/main/java/org/springframework/data/util/Optionals.java @@ -41,11 +41,16 @@ public interface Optionals { * @param optionals must not be {@literal null}. * @return */ - public static boolean isAnyPresent(Optional... optionals) { + static boolean isAnyPresent(Optional... optionals) { Assert.notNull(optionals, "Optionals must not be null"); - return Arrays.stream(optionals).anyMatch(Optional::isPresent); + for (Optional optional : optionals) { + if (optional.isPresent()) { + return true; + } + } + return false; } /** @@ -55,7 +60,7 @@ public interface Optionals { * @return */ @SafeVarargs - public static Stream toStream(Optional... optionals) { + static Stream toStream(Optional... optionals) { Assert.notNull(optionals, "Optional must not be null"); @@ -69,7 +74,7 @@ public interface Optionals { * @param function must not be {@literal null}. * @return */ - public static Optional firstNonEmpty(Iterable source, Function> function) { + static Optional firstNonEmpty(Iterable source, Function> function) { Assert.notNull(source, "Source must not be null"); Assert.notNull(function, "Function must not be null"); @@ -87,7 +92,7 @@ public interface Optionals { * @param function must not be {@literal null}. * @return */ - public static T firstNonEmpty(Iterable source, Function function, T defaultValue) { + static T firstNonEmpty(Iterable source, Function function, T defaultValue) { Assert.notNull(source, "Source must not be null"); Assert.notNull(function, "Function must not be null"); @@ -105,7 +110,7 @@ public interface Optionals { * @return */ @SafeVarargs - public static Optional firstNonEmpty(Supplier>... suppliers) { + static Optional firstNonEmpty(Supplier>... suppliers) { Assert.notNull(suppliers, "Suppliers must not be null"); @@ -118,7 +123,7 @@ public interface Optionals { * @param suppliers must not be {@literal null}. * @return */ - public static Optional firstNonEmpty(Iterable>> suppliers) { + static Optional firstNonEmpty(Iterable>> suppliers) { Assert.notNull(suppliers, "Suppliers must not be null"); @@ -135,7 +140,7 @@ public interface Optionals { * @param iterator must not be {@literal null}. * @return */ - public static Optional next(Iterator iterator) { + static Optional next(Iterator iterator) { Assert.notNull(iterator, "Iterator must not be null"); @@ -150,7 +155,7 @@ public interface Optionals { * @param right * @return */ - public static Optional> withBoth(Optional left, Optional right) { + static Optional> withBoth(Optional left, Optional right) { return left.flatMap(l -> right.map(r -> Pair.of(l, r))); } @@ -161,7 +166,7 @@ public interface Optionals { * @param right must not be {@literal null}. * @param consumer must not be {@literal null}. */ - public static void ifAllPresent(Optional left, Optional right, BiConsumer consumer) { + static void ifAllPresent(Optional left, Optional right, BiConsumer consumer) { Assert.notNull(left, "Optional must not be null"); Assert.notNull(right, "Optional must not be null"); @@ -181,7 +186,7 @@ public interface Optionals { * @param function must not be {@literal null}. * @return */ - public static Optional mapIfAllPresent(Optional left, Optional right, + static Optional mapIfAllPresent(Optional left, Optional right, BiFunction function) { Assert.notNull(left, "Optional must not be null"); @@ -198,7 +203,7 @@ public interface Optionals { * @param consumer must not be {@literal null}. * @param runnable must not be {@literal null}. */ - public static void ifPresentOrElse(Optional optional, Consumer consumer, Runnable runnable) { + static void ifPresentOrElse(Optional optional, Consumer consumer, Runnable runnable) { Assert.notNull(optional, "Optional must not be null"); Assert.notNull(consumer, "Consumer must not be null"); diff --git a/src/test/java/org/springframework/data/auditing/AnnotationAuditingMetadataUnitTests.java b/src/test/java/org/springframework/data/auditing/AnnotationAuditingMetadataUnitTests.java index b6ceb7099..ed02cdf6d 100755 --- a/src/test/java/org/springframework/data/auditing/AnnotationAuditingMetadataUnitTests.java +++ b/src/test/java/org/springframework/data/auditing/AnnotationAuditingMetadataUnitTests.java @@ -49,16 +49,6 @@ class AnnotationAuditingMetadataUnitTests { assertThat(metadata.getLastModifiedDateField()).hasValue(lastModifiedDateField); } - @Test - void checkCaching() { - - var firstCall = AnnotationAuditingMetadata.getMetadata(AnnotatedUser.class); - assertThat(firstCall).isNotNull(); - - var secondCall = AnnotationAuditingMetadata.getMetadata(AnnotatedUser.class); - assertThat(firstCall).isEqualTo(secondCall); - } - @Test void checkIsAuditable() { diff --git a/src/test/java/org/springframework/data/mapping/model/ClassGeneratingPropertyAccessorFactoryDatatypeTests.java b/src/test/java/org/springframework/data/mapping/model/ClassGeneratingPropertyAccessorFactoryDatatypeTests.java index 654fda0b0..f440d8402 100755 --- a/src/test/java/org/springframework/data/mapping/model/ClassGeneratingPropertyAccessorFactoryDatatypeTests.java +++ b/src/test/java/org/springframework/data/mapping/model/ClassGeneratingPropertyAccessorFactoryDatatypeTests.java @@ -68,10 +68,10 @@ public class ClassGeneratingPropertyAccessorFactoryDatatypeTests { parameters.addAll(create(types, "primitiveBooleanArray", new boolean[] { true, false })); parameters.addAll(create(types, "boxedBoolean", Boolean.valueOf(true))); parameters.addAll(create(types, "boxedBooleanArray", new Boolean[] { Boolean.valueOf(true) })); - parameters.addAll(create(types, "primitiveFloat", Float.valueOf(1f))); - parameters.addAll(create(types, "primitiveFloatArray", new float[] { 1f, 2f })); - parameters.addAll(create(types, "boxedFloat", Float.valueOf(1f))); - parameters.addAll(create(types, "boxedFloatArray", new Float[] { Float.valueOf(1f) })); + parameters.addAll(create(types, "primitiveFloat", Float.valueOf(1.0f))); + parameters.addAll(create(types, "primitiveFloatArray", new float[] { 1.0f, 2f })); + parameters.addAll(create(types, "boxedFloat", Float.valueOf(1.0f))); + parameters.addAll(create(types, "boxedFloatArray", new Float[] { Float.valueOf(1.0f) })); parameters.addAll(create(types, "primitiveDouble", Double.valueOf(1d))); parameters.addAll(create(types, "primitiveDoubleArray", new double[] { 1d, 2d })); parameters.addAll(create(types, "boxedDouble", Double.valueOf(1d)));