Skip to content

Commit 0e9dc7a

Browse files
committed
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
1 parent b21b2e8 commit 0e9dc7a

13 files changed

+54
-58
lines changed

src/main/java/org/springframework/data/auditing/AnnotationAuditingMetadata.java

+1-5
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,7 @@
2121
import java.util.Collections;
2222
import java.util.Date;
2323
import java.util.List;
24-
import java.util.Map;
2524
import java.util.Optional;
26-
import java.util.concurrent.ConcurrentHashMap;
2725

2826
import org.springframework.data.annotation.CreatedBy;
2927
import org.springframework.data.annotation.CreatedDate;
@@ -53,8 +51,6 @@ final class AnnotationAuditingMetadata {
5351
private static final AnnotationFieldFilter LAST_MODIFIED_DATE_FILTER = new AnnotationFieldFilter(
5452
LastModifiedDate.class);
5553

56-
private static final Map<Class<?>, AnnotationAuditingMetadata> metadataCache = new ConcurrentHashMap<>();
57-
5854
static final List<String> SUPPORTED_DATE_TYPES;
5955

6056
static {
@@ -126,7 +122,7 @@ private void assertValidDateFieldType(Optional<Field> field) {
126122
* @param type the type to inspect, must not be {@literal null}.
127123
*/
128124
public static AnnotationAuditingMetadata getMetadata(Class<?> type) {
129-
return metadataCache.computeIfAbsent(type, AnnotationAuditingMetadata::new);
125+
return new AnnotationAuditingMetadata(type);
130126
}
131127

132128
/**

src/main/java/org/springframework/data/auditing/DefaultAuditableBeanWrapperFactory.java

+7-2
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@
1919
import java.time.Instant;
2020
import java.time.temporal.TemporalAccessor;
2121
import java.util.Date;
22+
import java.util.Map;
2223
import java.util.Optional;
24+
import java.util.concurrent.ConcurrentHashMap;
2325
import java.util.stream.Stream;
2426

2527
import org.springframework.core.ResolvableType;
@@ -44,6 +46,7 @@
4446
class DefaultAuditableBeanWrapperFactory implements AuditableBeanWrapperFactory {
4547

4648
private final ConversionService conversionService;
49+
private final Map<Class<?>, AnnotationAuditingMetadata> metadataCache;
4750

4851
public DefaultAuditableBeanWrapperFactory() {
4952

@@ -52,6 +55,7 @@ public DefaultAuditableBeanWrapperFactory() {
5255
Jsr310Converters.getConvertersToRegister().forEach(conversionService::addConverter);
5356

5457
this.conversionService = conversionService;
58+
this.metadataCache = new ConcurrentHashMap<>();
5559
}
5660

5761
ConversionService getConversionService() {
@@ -77,10 +81,11 @@ public <T> Optional<AuditableBeanWrapper<T>> getBeanWrapperFor(T source) {
7781
(Auditable<Object, ?, TemporalAccessor>) it);
7882
}
7983

80-
AnnotationAuditingMetadata metadata = AnnotationAuditingMetadata.getMetadata(it.getClass());
84+
AnnotationAuditingMetadata metadata = metadataCache.computeIfAbsent(it.getClass(),
85+
AnnotationAuditingMetadata::getMetadata);
8186

8287
if (metadata.isAuditable()) {
83-
return new ReflectionAuditingBeanWrapper<T>(conversionService, it);
88+
return new ReflectionAuditingBeanWrapper<>(conversionService, it);
8489
}
8590

8691
return null;

src/main/java/org/springframework/data/geo/format/DistanceFormatter.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ public enum DistanceFormatter implements Converter<String, Distance>, Formatter<
4747

4848
static {
4949

50-
Map<String, Metric> metrics = new LinkedHashMap<>(Metrics.values().length);
50+
Map<String, Metric> metrics = new LinkedHashMap<>(Metrics.values().length, 1.0f);
5151

5252
for (Metric metric : Metrics.values()) {
5353
metrics.put(metric.getAbbreviation(), metric);

src/main/java/org/springframework/data/mapping/model/BasicPersistentEntity.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ public BasicPersistentEntity(TypeInformation<T> information, @Nullable Comparato
115115
this.creator = InstanceCreatorMetadataDiscoverer.discover(this);
116116
this.associations = comparator == null ? new HashSet<>() : new TreeSet<>(new AssociationComparator<>(comparator));
117117

118-
this.propertyCache = new HashMap<>(16, 1f);
118+
this.propertyCache = new HashMap<>(16, 1.0f);
119119
this.annotationCache = new ConcurrentHashMap<>(16);
120120
this.propertyAnnotationCache = CollectionUtils
121121
.toMultiValueMap(new ConcurrentHashMap<>(16));

src/main/java/org/springframework/data/projection/SpelEvaluatingMethodInterceptor.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ private static Map<Integer, Expression> potentiallyCreateExpressionsForMethodsOn
108108
ExpressionParser parser, Class<?> targetInterface) {
109109

110110
Method[] methods = targetInterface.getMethods();
111-
Map<Integer, Expression> expressions = new HashMap<>(methods.length);
111+
Map<Integer, Expression> expressions = new HashMap<>(methods.length, 1.0f);
112112

113113
for (Method method : methods) {
114114

src/main/java/org/springframework/data/repository/core/support/QueryExecutorMethodInterceptor.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ private Map<Method, RepositoryQuery> mapMethodsToQuery(RepositoryInformation rep
9393
QueryLookupStrategy lookupStrategy, ProjectionFactory projectionFactory) {
9494

9595
List<Method> queryMethods = repositoryInformation.getQueryMethods().toList();
96-
Map<Method, RepositoryQuery> result = new HashMap<>(queryMethods.size());
96+
Map<Method, RepositoryQuery> result = new HashMap<>(queryMethods.size(), 1.0f);
9797

9898
for (Method method : queryMethods) {
9999

src/main/java/org/springframework/data/repository/core/support/RepositoryFactorySupport.java

+14-14
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,13 @@
2323
import java.util.List;
2424
import java.util.Map;
2525
import java.util.Optional;
26-
import java.util.concurrent.ConcurrentHashMap;
2726
import java.util.stream.Collectors;
2827

2928
import org.aopalliance.intercept.MethodInterceptor;
3029
import org.aopalliance.intercept.MethodInvocation;
3130
import org.apache.commons.logging.Log;
3231
import org.apache.commons.logging.LogFactory;
32+
3333
import org.springframework.aop.framework.ProxyFactory;
3434
import org.springframework.aop.interceptor.ExposeInvocationInterceptor;
3535
import org.springframework.beans.BeanUtils;
@@ -108,7 +108,7 @@ public abstract class RepositoryFactorySupport implements BeanClassLoaderAware,
108108
@SuppressWarnings("null")
109109
public RepositoryFactorySupport() {
110110

111-
this.repositoryInformationCache = new ConcurrentHashMap<>(16);
111+
this.repositoryInformationCache = new HashMap<>(16);
112112
this.postProcessors = new ArrayList<>();
113113

114114
this.repositoryBaseClass = Optional.empty();
@@ -364,8 +364,7 @@ public <T> T getRepository(Class<T> repositoryInterface, RepositoryFragments fra
364364

365365
if (logger.isDebugEnabled()) {
366366
logger
367-
.debug(LogMessage.format("Finished creation of repository instance for %s.",
368-
repositoryInterface.getName()));
367+
.debug(LogMessage.format("Finished creation of repository instance for %s.", repositoryInterface.getName()));
369368
}
370369

371370
return repository;
@@ -440,12 +439,15 @@ private RepositoryInformation getRepositoryInformation(RepositoryMetadata metada
440439

441440
RepositoryInformationCacheKey cacheKey = new RepositoryInformationCacheKey(metadata, composition);
442441

443-
return repositoryInformationCache.computeIfAbsent(cacheKey, key -> {
442+
synchronized (repositoryInformationCache) {
444443

445-
Class<?> baseClass = repositoryBaseClass.orElse(getRepositoryBaseClass(metadata));
444+
return repositoryInformationCache.computeIfAbsent(cacheKey, key -> {
446445

447-
return new DefaultRepositoryInformation(metadata, baseClass, composition);
448-
});
446+
Class<?> baseClass = repositoryBaseClass.orElse(getRepositoryBaseClass(metadata));
447+
448+
return new DefaultRepositoryInformation(metadata, baseClass, composition);
449+
});
450+
}
449451
}
450452

451453
protected List<QueryMethod> getQueryMethods() {
@@ -725,7 +727,7 @@ public String toString() {
725727
*/
726728
static class RepositoryValidator {
727729

728-
static Map<Class<?>, String> WELL_KNOWN_EXECUTORS = new HashMap<>(4, 1f);
730+
static Map<Class<?>, String> WELL_KNOWN_EXECUTORS = new HashMap<>(4, 1.0f);
729731

730732
static {
731733

@@ -786,11 +788,9 @@ public static void validate(RepositoryComposition composition, Class<?> source,
786788
}
787789

788790
if (!containsFragmentImplementation(composition, executorInterface)) {
789-
throw new UnsupportedFragmentException(
790-
String.format("Repository %s implements %s but %s does not support %s",
791-
ClassUtils.getQualifiedName(repositoryInterface), ClassUtils.getQualifiedName(executorInterface),
792-
ClassUtils.getShortName(source), entry.getValue()),
793-
repositoryInterface, executorInterface);
791+
throw new UnsupportedFragmentException(String.format("Repository %s implements %s but %s does not support %s",
792+
ClassUtils.getQualifiedName(repositoryInterface), ClassUtils.getQualifiedName(executorInterface),
793+
ClassUtils.getShortName(source), entry.getValue()), repositoryInterface, executorInterface);
794794
}
795795
}
796796
}

src/main/java/org/springframework/data/repository/query/ExtensionAwareQueryMethodEvaluationContextProvider.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ public ExtensionAwareQueryMethodEvaluationContextProvider(List<? extends Evaluat
100100
*/
101101
static Map<String, Object> collectVariables(Parameters<?, ?> parameters, Object[] arguments) {
102102

103-
Map<String, Object> variables = new HashMap<>(parameters.getNumberOfParameters());
103+
Map<String, Object> variables = new HashMap<>(parameters.getNumberOfParameters(), 1.0f);
104104

105105
parameters.stream()//
106106
.filter(Parameter::isSpecialParameter)//

src/main/java/org/springframework/data/repository/util/QueryExecutionConverters.java

+4-4
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@
6363
* <li>{@code javaslang.collection.Seq}, {@code javaslang.collection.Map}, {@code javaslang.collection.Set} - as of
6464
* 1.13</li>
6565
* <li>{@code io.vavr.collection.Seq}, {@code io.vavr.collection.Map}, {@code io.vavr.collection.Set} - as of 2.0</li>
66-
* <li>Reactive wrappers supported by {@link ReactiveWrappers} - as of 2.0</li>
66+
* <li>Reactive wrappers supported by {@link org.springframework.data.util.ReactiveWrappers} - as of 2.0</li>
6767
* </ul>
6868
*
6969
* @author Oliver Gierke
@@ -79,11 +79,11 @@ public abstract class QueryExecutionConverters {
7979
private static final boolean VAVR_PRESENT = ClassUtils.isPresent("io.vavr.control.Try",
8080
QueryExecutionConverters.class.getClassLoader());
8181

82-
private static final Set<WrapperType> WRAPPER_TYPES = new HashSet<>(10, 1f);
83-
private static final Set<WrapperType> UNWRAPPER_TYPES = new HashSet<WrapperType>(10, 1f);
82+
private static final Set<WrapperType> WRAPPER_TYPES = new HashSet<>(10, 1.0f);
83+
private static final Set<WrapperType> UNWRAPPER_TYPES = new HashSet<WrapperType>(10, 1.0f);
8484
private static final Set<Function<Object, Object>> UNWRAPPERS = new HashSet<>();
8585
private static final Set<Class<?>> ALLOWED_PAGEABLE_TYPES = new HashSet<>();
86-
private static final Map<Class<?>, ExecutionAdapter> EXECUTION_ADAPTER = new HashMap<>(3, 1f);
86+
private static final Map<Class<?>, ExecutionAdapter> EXECUTION_ADAPTER = new HashMap<>(3, 1.0f);
8787
private static final Map<Class<?>, Boolean> supportsCache = new ConcurrentReferenceHashMap<>();
8888
private static final TypeInformation<Void> VOID_INFORMATION = TypeInformation.of(Void.class);
8989

src/main/java/org/springframework/data/util/NullableWrapperConverters.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
import java.util.HashSet;
2424
import java.util.Map;
2525
import java.util.Set;
26-
import java.util.concurrent.ConcurrentHashMap;
2726
import java.util.stream.Stream;
2827

2928
import org.springframework.core.convert.TypeDescriptor;
@@ -33,6 +32,7 @@
3332
import org.springframework.lang.Nullable;
3433
import org.springframework.util.Assert;
3534
import org.springframework.util.ClassUtils;
35+
import org.springframework.util.ConcurrentReferenceHashMap;
3636
import org.springframework.util.ObjectUtils;
3737

3838
import com.google.common.base.Optional;
@@ -67,7 +67,7 @@ public abstract class NullableWrapperConverters {
6767
private static final Set<WrapperType> WRAPPER_TYPES = new HashSet<WrapperType>();
6868
private static final Set<WrapperType> UNWRAPPER_TYPES = new HashSet<WrapperType>();
6969
private static final Set<Converter<Object, Object>> UNWRAPPERS = new HashSet<Converter<Object, Object>>();
70-
private static final Map<Class<?>, Boolean> supportsCache = new ConcurrentHashMap<>();
70+
private static final Map<Class<?>, Boolean> supportsCache = new ConcurrentReferenceHashMap<>();
7171

7272
static {
7373

src/main/java/org/springframework/data/util/Optionals.java

+17-12
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,16 @@ public interface Optionals {
4141
* @param optionals must not be {@literal null}.
4242
* @return
4343
*/
44-
public static boolean isAnyPresent(Optional<?>... optionals) {
44+
static boolean isAnyPresent(Optional<?>... optionals) {
4545

4646
Assert.notNull(optionals, "Optionals must not be null");
4747

48-
return Arrays.stream(optionals).anyMatch(Optional::isPresent);
48+
for (Optional<?> optional : optionals) {
49+
if (optional.isPresent()) {
50+
return true;
51+
}
52+
}
53+
return false;
4954
}
5055

5156
/**
@@ -55,7 +60,7 @@ public static boolean isAnyPresent(Optional<?>... optionals) {
5560
* @return
5661
*/
5762
@SafeVarargs
58-
public static <T> Stream<T> toStream(Optional<? extends T>... optionals) {
63+
static <T> Stream<T> toStream(Optional<? extends T>... optionals) {
5964

6065
Assert.notNull(optionals, "Optional must not be null");
6166

@@ -69,7 +74,7 @@ public static <T> Stream<T> toStream(Optional<? extends T>... optionals) {
6974
* @param function must not be {@literal null}.
7075
* @return
7176
*/
72-
public static <S, T> Optional<T> firstNonEmpty(Iterable<S> source, Function<S, Optional<T>> function) {
77+
static <S, T> Optional<T> firstNonEmpty(Iterable<S> source, Function<S, Optional<T>> function) {
7378

7479
Assert.notNull(source, "Source must not be null");
7580
Assert.notNull(function, "Function must not be null");
@@ -87,7 +92,7 @@ public static <S, T> Optional<T> firstNonEmpty(Iterable<S> source, Function<S, O
8792
* @param function must not be {@literal null}.
8893
* @return
8994
*/
90-
public static <S, T> T firstNonEmpty(Iterable<S> source, Function<S, T> function, T defaultValue) {
95+
static <S, T> T firstNonEmpty(Iterable<S> source, Function<S, T> function, T defaultValue) {
9196

9297
Assert.notNull(source, "Source must not be null");
9398
Assert.notNull(function, "Function must not be null");
@@ -105,7 +110,7 @@ public static <S, T> T firstNonEmpty(Iterable<S> source, Function<S, T> function
105110
* @return
106111
*/
107112
@SafeVarargs
108-
public static <T> Optional<T> firstNonEmpty(Supplier<Optional<T>>... suppliers) {
113+
static <T> Optional<T> firstNonEmpty(Supplier<Optional<T>>... suppliers) {
109114

110115
Assert.notNull(suppliers, "Suppliers must not be null");
111116

@@ -118,7 +123,7 @@ public static <T> Optional<T> firstNonEmpty(Supplier<Optional<T>>... suppliers)
118123
* @param suppliers must not be {@literal null}.
119124
* @return
120125
*/
121-
public static <T> Optional<T> firstNonEmpty(Iterable<Supplier<Optional<T>>> suppliers) {
126+
static <T> Optional<T> firstNonEmpty(Iterable<Supplier<Optional<T>>> suppliers) {
122127

123128
Assert.notNull(suppliers, "Suppliers must not be null");
124129

@@ -135,7 +140,7 @@ public static <T> Optional<T> firstNonEmpty(Iterable<Supplier<Optional<T>>> supp
135140
* @param iterator must not be {@literal null}.
136141
* @return
137142
*/
138-
public static <T> Optional<T> next(Iterator<T> iterator) {
143+
static <T> Optional<T> next(Iterator<T> iterator) {
139144

140145
Assert.notNull(iterator, "Iterator must not be null");
141146

@@ -150,7 +155,7 @@ public static <T> Optional<T> next(Iterator<T> iterator) {
150155
* @param right
151156
* @return
152157
*/
153-
public static <T, S> Optional<Pair<T, S>> withBoth(Optional<T> left, Optional<S> right) {
158+
static <T, S> Optional<Pair<T, S>> withBoth(Optional<T> left, Optional<S> right) {
154159
return left.flatMap(l -> right.map(r -> Pair.of(l, r)));
155160
}
156161

@@ -161,7 +166,7 @@ public static <T, S> Optional<Pair<T, S>> withBoth(Optional<T> left, Optional<S>
161166
* @param right must not be {@literal null}.
162167
* @param consumer must not be {@literal null}.
163168
*/
164-
public static <T, S> void ifAllPresent(Optional<T> left, Optional<S> right, BiConsumer<T, S> consumer) {
169+
static <T, S> void ifAllPresent(Optional<T> left, Optional<S> right, BiConsumer<T, S> consumer) {
165170

166171
Assert.notNull(left, "Optional must not be null");
167172
Assert.notNull(right, "Optional must not be null");
@@ -181,7 +186,7 @@ public static <T, S> void ifAllPresent(Optional<T> left, Optional<S> right, BiCo
181186
* @param function must not be {@literal null}.
182187
* @return
183188
*/
184-
public static <T, S, R> Optional<R> mapIfAllPresent(Optional<T> left, Optional<S> right,
189+
static <T, S, R> Optional<R> mapIfAllPresent(Optional<T> left, Optional<S> right,
185190
BiFunction<T, S, R> function) {
186191

187192
Assert.notNull(left, "Optional must not be null");
@@ -198,7 +203,7 @@ public static <T, S, R> Optional<R> mapIfAllPresent(Optional<T> left, Optional<S
198203
* @param consumer must not be {@literal null}.
199204
* @param runnable must not be {@literal null}.
200205
*/
201-
public static <T> void ifPresentOrElse(Optional<T> optional, Consumer<? super T> consumer, Runnable runnable) {
206+
static <T> void ifPresentOrElse(Optional<T> optional, Consumer<? super T> consumer, Runnable runnable) {
202207

203208
Assert.notNull(optional, "Optional must not be null");
204209
Assert.notNull(consumer, "Consumer must not be null");

src/test/java/org/springframework/data/auditing/AnnotationAuditingMetadataUnitTests.java

-10
Original file line numberDiff line numberDiff line change
@@ -49,16 +49,6 @@ void checkAnnotationDiscovery() {
4949
assertThat(metadata.getLastModifiedDateField()).hasValue(lastModifiedDateField);
5050
}
5151

52-
@Test
53-
void checkCaching() {
54-
55-
var firstCall = AnnotationAuditingMetadata.getMetadata(AnnotatedUser.class);
56-
assertThat(firstCall).isNotNull();
57-
58-
var secondCall = AnnotationAuditingMetadata.getMetadata(AnnotatedUser.class);
59-
assertThat(firstCall).isEqualTo(secondCall);
60-
}
61-
6252
@Test
6353
void checkIsAuditable() {
6454

src/test/java/org/springframework/data/mapping/model/ClassGeneratingPropertyAccessorFactoryDatatypeTests.java

+4-4
Original file line numberDiff line numberDiff line change
@@ -68,10 +68,10 @@ static List<Object[]> parameters() throws Exception {
6868
parameters.addAll(create(types, "primitiveBooleanArray", new boolean[] { true, false }));
6969
parameters.addAll(create(types, "boxedBoolean", Boolean.valueOf(true)));
7070
parameters.addAll(create(types, "boxedBooleanArray", new Boolean[] { Boolean.valueOf(true) }));
71-
parameters.addAll(create(types, "primitiveFloat", Float.valueOf(1f)));
72-
parameters.addAll(create(types, "primitiveFloatArray", new float[] { 1f, 2f }));
73-
parameters.addAll(create(types, "boxedFloat", Float.valueOf(1f)));
74-
parameters.addAll(create(types, "boxedFloatArray", new Float[] { Float.valueOf(1f) }));
71+
parameters.addAll(create(types, "primitiveFloat", Float.valueOf(1.0f)));
72+
parameters.addAll(create(types, "primitiveFloatArray", new float[] { 1.0f, 2f }));
73+
parameters.addAll(create(types, "boxedFloat", Float.valueOf(1.0f)));
74+
parameters.addAll(create(types, "boxedFloatArray", new Float[] { Float.valueOf(1.0f) }));
7575
parameters.addAll(create(types, "primitiveDouble", Double.valueOf(1d)));
7676
parameters.addAll(create(types, "primitiveDoubleArray", new double[] { 1d, 2d }));
7777
parameters.addAll(create(types, "boxedDouble", Double.valueOf(1d)));

0 commit comments

Comments
 (0)