Skip to content

Commit cbc512f

Browse files
jhoellerjhoeller
committed
Efficient type plus annotation comparisons during converter retrieval
Issue: SPR-14926 Issue: SPR-12926 (cherry picked from commit f6b8b84)
1 parent 7ac9f92 commit cbc512f

File tree

3 files changed

+80
-52
lines changed

3 files changed

+80
-52
lines changed

‎spring-context/src/main/java/org/springframework/format/support/FormattingConversionService.java‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,7 @@ private class AnnotationPrinterConverter implements ConditionalGenericConverter
231231

232232
public AnnotationPrinterConverter(Class<? extends Annotation> annotationType,
233233
AnnotationFormatterFactory<?> annotationFormatterFactory, Class<?> fieldType) {
234+
234235
this.annotationType = annotationType;
235236
this.annotationFormatterFactory = annotationFormatterFactory;
236237
this.fieldType = fieldType;
@@ -284,6 +285,7 @@ private class AnnotationParserConverter implements ConditionalGenericConverter {
284285

285286
public AnnotationParserConverter(Class<? extends Annotation> annotationType,
286287
AnnotationFormatterFactory<?> annotationFormatterFactory, Class<?> fieldType) {
288+
287289
this.annotationType = annotationType;
288290
this.annotationFormatterFactory = annotationFormatterFactory;
289291
this.fieldType = fieldType;
@@ -350,16 +352,13 @@ public boolean equals(Object other) {
350352
if (this == other) {
351353
return true;
352354
}
353-
if (!(other instanceof AnnotationConverterKey)) {
354-
return false;
355-
}
356355
AnnotationConverterKey otherKey = (AnnotationConverterKey) other;
357-
return (this.annotation.equals(otherKey.annotation) && this.fieldType.equals(otherKey.fieldType));
356+
return (this.fieldType == otherKey.fieldType && this.annotation.equals(otherKey.annotation));
358357
}
359358

360359
@Override
361360
public int hashCode() {
362-
return (this.annotation.hashCode() + 29 * this.fieldType.hashCode());
361+
return (this.fieldType.hashCode() * 29 + this.annotation.hashCode());
363362
}
364363
}
365364

‎spring-core/src/main/java/org/springframework/core/convert/TypeDescriptor.java‎

Lines changed: 60 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ public class TypeDescriptor implements Serializable {
7272

7373
private final ResolvableType resolvableType;
7474

75-
private final AnnotatedElement annotatedElement;
75+
private final AnnotatedElementAdapter annotatedElement;
7676

7777

7878
/**
@@ -82,7 +82,6 @@ public class TypeDescriptor implements Serializable {
8282
* @param methodParameter the method parameter
8383
*/
8484
public TypeDescriptor(MethodParameter methodParameter) {
85-
Assert.notNull(methodParameter, "MethodParameter must not be null");
8685
this.resolvableType = ResolvableType.forMethodParameter(methodParameter);
8786
this.type = this.resolvableType.resolve(methodParameter.getParameterType());
8887
this.annotatedElement = new AnnotatedElementAdapter(methodParameter.getParameterIndex() == -1 ?
@@ -95,7 +94,6 @@ public TypeDescriptor(MethodParameter methodParameter) {
9594
* @param field the field
9695
*/
9796
public TypeDescriptor(Field field) {
98-
Assert.notNull(field, "Field must not be null");
9997
this.resolvableType = ResolvableType.forField(field);
10098
this.type = this.resolvableType.resolve(field.getType());
10199
this.annotatedElement = new AnnotatedElementAdapter(field.getAnnotations());
@@ -121,6 +119,7 @@ public TypeDescriptor(Property property) {
121119
* @param resolvableType the resolvable type
122120
* @param type the backing type (or {@code null} if it should get resolved)
123121
* @param annotations the type annotations
122+
* @since 4.0
124123
*/
125124
protected TypeDescriptor(ResolvableType resolvableType, Class<?> type, Annotation[] annotations) {
126125
this.resolvableType = resolvableType;
@@ -211,7 +210,7 @@ public TypeDescriptor upcast(Class<?> superType) {
211210
}
212211

213212
/**
214-
* Returns the name of this type: the fully qualified class name.
213+
* Return the name of this type: the fully qualified class name.
215214
*/
216215
public String getName() {
217216
return ClassUtils.getQualifiedName(getType());
@@ -225,7 +224,7 @@ public boolean isPrimitive() {
225224
}
226225

227226
/**
228-
* The annotations associated with this type descriptor, if any.
227+
* Return the annotations associated with this type descriptor, if any.
229228
* @return the annotations, or an empty array if none
230229
*/
231230
public Annotation[] getAnnotations() {
@@ -240,6 +239,11 @@ public Annotation[] getAnnotations() {
240239
* @return <tt>true</tt> if the annotation is present
241240
*/
242241
public boolean hasAnnotation(Class<? extends Annotation> annotationType) {
242+
if (this.annotatedElement.isEmpty()) {
243+
// Shortcut: AnnotatedElementUtils would have to expect AnnotatedElement.getAnnotations()
244+
// to return a copy of the array, whereas we can do it more efficiently here.
245+
return false;
246+
}
243247
return AnnotatedElementUtils.isAnnotated(this.annotatedElement, annotationType);
244248
}
245249

@@ -251,6 +255,11 @@ public boolean hasAnnotation(Class<? extends Annotation> annotationType) {
251255
*/
252256
@SuppressWarnings("unchecked")
253257
public <T extends Annotation> T getAnnotation(Class<T> annotationType) {
258+
if (this.annotatedElement.isEmpty()) {
259+
// Shortcut: AnnotatedElementUtils would have to expect AnnotatedElement.getAnnotations()
260+
// to return a copy of the array, whereas we can do it more efficiently here.
261+
return null;
262+
}
254263
return AnnotatedElementUtils.getMergedAnnotation(this.annotatedElement, annotationType);
255264
}
256265

@@ -434,37 +443,51 @@ private TypeDescriptor narrow(Object value, TypeDescriptor typeDescriptor) {
434443
}
435444

436445
@Override
437-
public boolean equals(Object obj) {
438-
if (this == obj) {
446+
public boolean equals(Object other) {
447+
if (this == other) {
439448
return true;
440449
}
441-
if (!(obj instanceof TypeDescriptor)) {
450+
if (!(other instanceof TypeDescriptor)) {
442451
return false;
443452
}
444-
TypeDescriptor other = (TypeDescriptor) obj;
445-
if (!ObjectUtils.nullSafeEquals(this.type, other.type)) {
453+
TypeDescriptor otherDesc = (TypeDescriptor) other;
454+
if (getType() != otherDesc.getType()) {
446455
return false;
447456
}
448-
if (getAnnotations().length != other.getAnnotations().length) {
457+
if (!annotationsMatch(otherDesc)) {
449458
return false;
450459
}
451-
for (Annotation ann : getAnnotations()) {
452-
if (!ann.equals(other.getAnnotation(ann.annotationType()))) {
453-
return false;
454-
}
455-
}
456460
if (isCollection() || isArray()) {
457-
return ObjectUtils.nullSafeEquals(getElementTypeDescriptor(), other.getElementTypeDescriptor());
461+
return ObjectUtils.nullSafeEquals(getElementTypeDescriptor(), otherDesc.getElementTypeDescriptor());
458462
}
459463
else if (isMap()) {
460-
return ObjectUtils.nullSafeEquals(getMapKeyTypeDescriptor(), other.getMapKeyTypeDescriptor()) &&
461-
ObjectUtils.nullSafeEquals(getMapValueTypeDescriptor(), other.getMapValueTypeDescriptor());
464+
return (ObjectUtils.nullSafeEquals(getMapKeyTypeDescriptor(), otherDesc.getMapKeyTypeDescriptor()) &&
465+
ObjectUtils.nullSafeEquals(getMapValueTypeDescriptor(), otherDesc.getMapValueTypeDescriptor()));
462466
}
463467
else {
464468
return true;
465469
}
466470
}
467471

472+
private boolean annotationsMatch(TypeDescriptor otherDesc) {
473+
Annotation[] anns = getAnnotations();
474+
Annotation[] otherAnns = otherDesc.getAnnotations();
475+
if (anns == otherAnns) {
476+
return true;
477+
}
478+
if (anns.length != otherAnns.length) {
479+
return false;
480+
}
481+
if (anns.length > 0) {
482+
for (int i = 0; i < anns.length; i++) {
483+
if (anns[i] != otherAnns[i]) {
484+
return false;
485+
}
486+
}
487+
}
488+
return true;
489+
}
490+
468491
@Override
469492
public int hashCode() {
470493
return getType().hashCode();
@@ -480,6 +503,20 @@ public String toString() {
480503
return builder.toString();
481504
}
482505

506+
507+
/**
508+
* Create a new type descriptor for an object.
509+
* <p>Use this factory method to introspect a source object before asking the
510+
* conversion system to convert it to some another type.
511+
* <p>If the provided object is {@code null}, returns {@code null}, else calls
512+
* {@link #valueOf(Class)} to build a TypeDescriptor from the object's class.
513+
* @param source the source object
514+
* @return the type descriptor
515+
*/
516+
public static TypeDescriptor forObject(Object source) {
517+
return (source != null ? valueOf(source.getClass()) : null);
518+
}
519+
483520
/**
484521
* Create a new type descriptor from the given type.
485522
* <p>Use this to instruct the conversion system to convert an object to a
@@ -640,19 +677,6 @@ public static TypeDescriptor nested(Property property, int nestingLevel) {
640677
return nested(new TypeDescriptor(property), nestingLevel);
641678
}
642679

643-
/**
644-
* Create a new type descriptor for an object.
645-
* <p>Use this factory method to introspect a source object before asking the
646-
* conversion system to convert it to some another type.
647-
* <p>If the provided object is {@code null}, returns {@code null}, else calls
648-
* {@link #valueOf(Class)} to build a TypeDescriptor from the object's class.
649-
* @param source the source object
650-
* @return the type descriptor
651-
*/
652-
public static TypeDescriptor forObject(Object source) {
653-
return (source != null ? valueOf(source.getClass()) : null);
654-
}
655-
656680
private static TypeDescriptor nested(TypeDescriptor typeDescriptor, int nestingLevel) {
657681
ResolvableType nested = typeDescriptor.resolvableType;
658682
for (int i = 0; i < nestingLevel; i++) {
@@ -723,6 +747,10 @@ public Annotation[] getDeclaredAnnotations() {
723747
return getAnnotations();
724748
}
725749

750+
public boolean isEmpty() {
751+
return ObjectUtils.isEmpty(this.annotations);
752+
}
753+
726754
@Override
727755
public boolean equals(Object other) {
728756
return (this == other || (other instanceof AnnotatedElementAdapter &&

‎spring-core/src/test/java/org/springframework/core/convert/TypeDescriptorTests.java‎

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2015 the original author or authors.
2+
* Copyright 2002-2016 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -533,7 +533,7 @@ public void elementType() {
533533
public void elementTypePreserveContext() throws Exception {
534534
TypeDescriptor desc = new TypeDescriptor(getClass().getField("listPreserveContext"));
535535
assertEquals(Integer.class, desc.getElementTypeDescriptor().getElementTypeDescriptor().getType());
536-
List<Integer> value = new ArrayList<Integer>(3);
536+
List<Integer> value = new ArrayList<>(3);
537537
desc = desc.elementTypeDescriptor(value);
538538
assertEquals(Integer.class, desc.getElementTypeDescriptor().getType());
539539
assertNotNull(desc.getAnnotation(FieldAnnotation.class));
@@ -551,7 +551,7 @@ public void mapKeyType() {
551551
public void mapKeyTypePreserveContext() throws Exception {
552552
TypeDescriptor desc = new TypeDescriptor(getClass().getField("mapPreserveContext"));
553553
assertEquals(Integer.class, desc.getMapKeyTypeDescriptor().getElementTypeDescriptor().getType());
554-
List<Integer> value = new ArrayList<Integer>(3);
554+
List<Integer> value = new ArrayList<>(3);
555555
desc = desc.getMapKeyTypeDescriptor(value);
556556
assertEquals(Integer.class, desc.getElementTypeDescriptor().getType());
557557
assertNotNull(desc.getAnnotation(FieldAnnotation.class));
@@ -569,7 +569,7 @@ public void mapValueType() {
569569
public void mapValueTypePreserveContext() throws Exception {
570570
TypeDescriptor desc = new TypeDescriptor(getClass().getField("mapPreserveContext"));
571571
assertEquals(Integer.class, desc.getMapValueTypeDescriptor().getElementTypeDescriptor().getType());
572-
List<Integer> value = new ArrayList<Integer>(3);
572+
List<Integer> value = new ArrayList<>(3);
573573
desc = desc.getMapValueTypeDescriptor(value);
574574
assertEquals(Integer.class, desc.getElementTypeDescriptor().getType());
575575
assertNotNull(desc.getAnnotation(FieldAnnotation.class));
@@ -598,15 +598,16 @@ public void equality() throws Exception {
598598
TypeDescriptor t12 = new TypeDescriptor(getClass().getField("mapField"));
599599
assertEquals(t11, t12);
600600

601-
TypeDescriptor t13 = new TypeDescriptor(new MethodParameter(getClass().getMethod("testAnnotatedMethod", String.class), 0));
602-
TypeDescriptor t14 = new TypeDescriptor(new MethodParameter(getClass().getMethod("testAnnotatedMethod", String.class), 0));
601+
MethodParameter testAnnotatedMethod = new MethodParameter(getClass().getMethod("testAnnotatedMethod", String.class), 0);
602+
TypeDescriptor t13 = new TypeDescriptor(testAnnotatedMethod);
603+
TypeDescriptor t14 = new TypeDescriptor(testAnnotatedMethod);
603604
assertEquals(t13, t14);
604605

605-
TypeDescriptor t15 = new TypeDescriptor(new MethodParameter(getClass().getMethod("testAnnotatedMethod", String.class), 0));
606+
TypeDescriptor t15 = new TypeDescriptor(testAnnotatedMethod);
606607
TypeDescriptor t16 = new TypeDescriptor(new MethodParameter(getClass().getMethod("testAnnotatedMethodDifferentAnnotationValue", String.class), 0));
607608
assertNotEquals(t15, t16);
608609

609-
TypeDescriptor t17 = new TypeDescriptor(new MethodParameter(getClass().getMethod("testAnnotatedMethod", String.class), 0));
610+
TypeDescriptor t17 = new TypeDescriptor(testAnnotatedMethod);
610611
TypeDescriptor t18 = new TypeDescriptor(new MethodParameter(getClass().getMethod("test5", String.class), 0));
611612
assertNotEquals(t17, t18);
612613
}
@@ -841,19 +842,19 @@ public void testAnnotatedMethodDifferentAnnotationValue(@ParameterAnnotation(567
841842

842843
public List<String> listOfString;
843844

844-
public List<List<String>> listOfListOfString = new ArrayList<List<String>>();
845+
public List<List<String>> listOfListOfString = new ArrayList<>();
845846

846-
public List<List> listOfListOfUnknown = new ArrayList<List>();
847+
public List<List> listOfListOfUnknown = new ArrayList<>();
847848

848849
public int[] intArray;
849850

850851
public List<String>[] arrayOfListOfString;
851852

852-
public List<Integer> listField = new ArrayList<Integer>();
853+
public List<Integer> listField = new ArrayList<>();
853854

854-
public Map<String, Integer> mapField = new HashMap<String, Integer>();
855+
public Map<String, Integer> mapField = new HashMap<>();
855856

856-
public Map<String, List<Integer>> nestedMapField = new HashMap<String, List<Integer>>();
857+
public Map<String, List<Integer>> nestedMapField = new HashMap<>();
857858

858859
public Map<List<Integer>, List<Long>> fieldMap;
859860

@@ -879,9 +880,9 @@ public void testAnnotatedMethodDifferentAnnotationValue(@ParameterAnnotation(567
879880

880881
public Map<CharSequence, Number> isAssignableMapKeyValueTypes;
881882

882-
public MultiValueMap<String, Integer> multiValueMap = new LinkedMultiValueMap<String, Integer>();
883+
public MultiValueMap<String, Integer> multiValueMap = new LinkedMultiValueMap<>();
883884

884-
public PassDownGeneric<Integer> passDownGeneric = new PassDownGeneric<Integer>();
885+
public PassDownGeneric<Integer> passDownGeneric = new PassDownGeneric<>();
885886

886887

887888
// Classes designed for test introspection

0 commit comments

Comments
 (0)