Skip to content

Commit 5883df8

Browse files
committed
fix(converter): Refactor AnnotatedType equals/hashCode for correctness and caching
1 parent 7f65d7b commit 5883df8

File tree

2 files changed

+131
-8
lines changed

2 files changed

+131
-8
lines changed

modules/swagger-core/src/main/java/io/swagger/v3/core/converter/AnnotatedType.java

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,11 @@
88
import java.lang.reflect.Type;
99
import java.util.ArrayList;
1010
import java.util.Arrays;
11+
import java.util.Comparator;
1112
import java.util.List;
1213
import java.util.Objects;
1314
import java.util.function.Function;
15+
import java.util.stream.Collectors;
1416

1517
public class AnnotatedType {
1618
private Type type;
@@ -155,11 +157,12 @@ public AnnotatedType name(String name) {
155157
}
156158

157159
public Annotation[] getCtxAnnotations() {
158-
return ctxAnnotations;
160+
// return ctxAnnotations;
161+
return ctxAnnotations == null ? null : Arrays.copyOf(ctxAnnotations, ctxAnnotations.length);
159162
}
160163

161164
public void setCtxAnnotations(Annotation[] ctxAnnotations) {
162-
this.ctxAnnotations = ctxAnnotations;
165+
this.ctxAnnotations = ctxAnnotations == null ? null : Arrays.copyOf(ctxAnnotations, ctxAnnotations.length);
163166
}
164167

165168
public AnnotatedType ctxAnnotations(Annotation[] ctxAnnotations) {
@@ -241,11 +244,26 @@ public AnnotatedType propertyName(String propertyName) {
241244
return this;
242245
}
243246

247+
private List<Annotation> getProcessedAnnotations(Annotation[] annotations) {
248+
if (annotations == null || annotations.length == 0) {
249+
return new ArrayList<>();
250+
}
251+
return Arrays.stream(annotations)
252+
.filter(a -> {
253+
String pkg = a.annotationType().getPackage().getName();
254+
return !pkg.startsWith("java.") && !pkg.startsWith("jdk.") && !pkg.startsWith("sun.");
255+
})
256+
.sorted(Comparator.comparing(a -> a.annotationType().getName()))
257+
.collect(Collectors.toList());
258+
}
259+
244260
@Override
245261
public boolean equals(Object o) {
246262
if (this == o) return true;
247-
if (o == null || getClass() != o.getClass()) return false;
263+
if (!(o instanceof AnnotatedType)) return false;
248264
AnnotatedType that = (AnnotatedType) o;
265+
List<Annotation> thisAnnotatinons = getProcessedAnnotations(this.ctxAnnotations);
266+
List<Annotation> thatAnnotatinons = getProcessedAnnotations(that.ctxAnnotations);
249267
return skipOverride == that.skipOverride &&
250268
schemaProperty == that.schemaProperty &&
251269
resolveAsRef == that.resolveAsRef &&
@@ -255,16 +273,16 @@ public boolean equals(Object o) {
255273
skipJsonIdentity == that.skipJsonIdentity &&
256274
Objects.equals(type, that.type) &&
257275
Objects.equals(name, that.name) &&
258-
Arrays.equals(ctxAnnotations, that.ctxAnnotations) &&
276+
Objects.equals(thisAnnotatinons, thatAnnotatinons) &&
259277
Objects.equals(jsonViewAnnotation, that.jsonViewAnnotation);
260278
}
261279

262280
@Override
263281
public int hashCode() {
264-
int result = Objects.hash(type, name, skipOverride, schemaProperty, resolveAsRef,
282+
List<Annotation> processedAnnotations = getProcessedAnnotations(this.ctxAnnotations);
283+
284+
return Objects.hash(type, name, skipOverride, schemaProperty, resolveAsRef,
265285
resolveEnumAsRef, jsonViewAnnotation, includePropertiesWithoutJSONView, skipSchemaName,
266-
skipJsonIdentity);
267-
result = 31 * result + Arrays.hashCode(ctxAnnotations);
268-
return result;
286+
skipJsonIdentity, processedAnnotations);
269287
}
270288
}
Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
package io.swagger.v3.core.converting;
2+
3+
import io.swagger.v3.core.converter.AnnotatedType;
4+
import org.testng.annotations.Test;
5+
6+
import java.lang.annotation.Annotation;
7+
import java.lang.annotation.ElementType;
8+
import java.lang.annotation.Retention;
9+
import java.lang.annotation.RetentionPolicy;
10+
import java.lang.annotation.Target;
11+
import java.lang.reflect.Type;
12+
import java.util.HashSet;
13+
import java.util.Set;
14+
15+
import static org.testng.Assert.assertEquals;
16+
import static org.testng.Assert.assertNotEquals;
17+
import static org.testng.Assert.assertTrue;
18+
19+
public class AnnotatedTypeTest {
20+
21+
@Retention(RetentionPolicy.RUNTIME)
22+
@Target(ElementType.TYPE)
23+
@interface TestAnnA {}
24+
25+
@Retention(RetentionPolicy.RUNTIME)
26+
@Target(ElementType.TYPE)
27+
@interface TestAnnB {}
28+
29+
@TestAnnA
30+
@TestAnnB
31+
@Deprecated
32+
private static class AnnotationHolder {}
33+
34+
private Annotation getAnnotationInstance(Class<? extends Annotation> clazz) {
35+
return AnnotationHolder.class.getAnnotation(clazz);
36+
}
37+
38+
/**
39+
* Tests that equals() and hashCode() are order-insensitive for context annotations.
40+
*/
41+
@Test
42+
public void testEqualsAndHashCode_shouldBeOrderInsensitiveForAnnotations() {
43+
Annotation annA = getAnnotationInstance(TestAnnA.class);
44+
Annotation annB = getAnnotationInstance(TestAnnB.class);
45+
AnnotatedType type1 = new AnnotatedType(String.class).ctxAnnotations(new Annotation[]{annA, annB});
46+
AnnotatedType type2 = new AnnotatedType(String.class).ctxAnnotations(new Annotation[]{annB, annA});
47+
assertEquals(type1, type2, "Objects should be equal even if annotation order is different.");
48+
assertEquals(type1.hashCode(), type2.hashCode(), "Hash codes should be equal even if annotation order is different.");
49+
}
50+
51+
/**
52+
* Tests that JDK/internal annotations are filtered out for equals() and hashCode() comparison.
53+
*/
54+
@Test
55+
public void testEqualsAndHashCode_shouldIgnoreJdkInternalAnnotations() {
56+
Annotation annA = getAnnotationInstance(TestAnnA.class);
57+
Annotation deprecated = getAnnotationInstance(Deprecated.class);
58+
AnnotatedType typeWithUserAnn = new AnnotatedType(String.class).ctxAnnotations(new Annotation[]{annA});
59+
AnnotatedType typeWithJdkAnn = new AnnotatedType(String.class).ctxAnnotations(new Annotation[]{annA, deprecated});
60+
AnnotatedType typeWithOnlyJdkAnn = new AnnotatedType(String.class).ctxAnnotations(new Annotation[]{deprecated});
61+
AnnotatedType typeWithNoAnn = new AnnotatedType(String.class);
62+
assertEquals(typeWithUserAnn, typeWithJdkAnn, "JDK annotations should be ignored in equality comparison.");
63+
assertEquals(typeWithUserAnn.hashCode(), typeWithJdkAnn.hashCode(), "JDK annotations should be ignored in hashCode calculation.");
64+
assertEquals(typeWithOnlyJdkAnn, typeWithNoAnn, "An object with only JDK annotations should be equal to one with no annotations.");
65+
assertEquals(typeWithOnlyJdkAnn.hashCode(), typeWithNoAnn.hashCode(), "The hash code of an object with only JDK annotations should be the same as one with no annotations.");
66+
}
67+
68+
/**
69+
* Tests that defensive copying prevents Set corruption from external array mutation.
70+
*/
71+
@Test
72+
public void testImmutability_shouldPreventCorruptionInHashSet() {
73+
Annotation annA = getAnnotationInstance(TestAnnA.class);
74+
Annotation annB = getAnnotationInstance(TestAnnB.class);
75+
Annotation[] originalAnnotations = new Annotation[]{annA};
76+
AnnotatedType type = new AnnotatedType(String.class).ctxAnnotations(originalAnnotations);
77+
Set<AnnotatedType> typeSet = new HashSet<>();
78+
typeSet.add(type);
79+
int initialHashCode = type.hashCode();
80+
originalAnnotations[0] = annB;
81+
assertEquals(initialHashCode, type.hashCode(), "Hash code must remain the same after mutating the external array.");
82+
assertTrue(typeSet.contains(type), "The Set must still contain the object after mutating the external array.");
83+
}
84+
85+
/**
86+
* Tests that an instance of a subclass can be equal to an instance of the parent class.
87+
*/
88+
@Test
89+
public void testEqualsAndHashCode_shouldAllowSubclassEquality() {
90+
class SubAnnotatedType extends AnnotatedType {
91+
public SubAnnotatedType(Type type) { super(type); }
92+
}
93+
Annotation annA = getAnnotationInstance(TestAnnA.class);
94+
Annotation[] annotations = {annA};
95+
AnnotatedType parent = new AnnotatedType(Integer.class).ctxAnnotations(annotations).name("number");
96+
SubAnnotatedType child = new SubAnnotatedType(Integer.class);
97+
child.ctxAnnotations(annotations);
98+
child.name("number");
99+
AnnotatedType differentParent = new AnnotatedType(Long.class).name("number");
100+
assertEquals(parent, child, "Parent and child objects should be equal if their properties are the same.");
101+
assertEquals(child, parent, "Equality comparison should be symmetric.");
102+
assertEquals(parent.hashCode(), child.hashCode(), "Parent and child hash codes should be equal if their properties are the same.");
103+
assertNotEquals(parent, differentParent, "Objects with different properties should not be equal.");
104+
}
105+
}

0 commit comments

Comments
 (0)