From b05863c6ddfc2de1bb9527f8fab72568fceed944 Mon Sep 17 00:00:00 2001 From: potato <65760583+juntae6942@users.noreply.github.com> Date: Fri, 19 Sep 2025 00:00:17 +0900 Subject: [PATCH 1/3] Fix(core): Prevent redundant schema resolution by fixing AnnotatedType equality Signed-off-by: potato <65760583+juntae6942@users.noreply.github.com> --- .../v3/core/converter/AnnotatedType.java | 53 +++++--------- .../converting/AnnotatedTypeCachingTest.java | 72 +++++++++++++++++++ 2 files changed, 89 insertions(+), 36 deletions(-) create mode 100644 modules/swagger-core/src/test/java/io/swagger/v3/core/converting/AnnotatedTypeCachingTest.java diff --git a/modules/swagger-core/src/main/java/io/swagger/v3/core/converter/AnnotatedType.java b/modules/swagger-core/src/main/java/io/swagger/v3/core/converter/AnnotatedType.java index 49bbce7b48..1d6734ebd1 100644 --- a/modules/swagger-core/src/main/java/io/swagger/v3/core/converter/AnnotatedType.java +++ b/modules/swagger-core/src/main/java/io/swagger/v3/core/converter/AnnotatedType.java @@ -243,47 +243,28 @@ public AnnotatedType propertyName(String propertyName) { @Override public boolean equals(Object o) { - if (this == o) { - return true; - } - if (!(o instanceof AnnotatedType)) { - return false; - } + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; AnnotatedType that = (AnnotatedType) o; - - if ((type == null && that.type != null) || (type != null && that.type == null)) { - return false; - } - - if (type != null && that.type != null && !type.equals(that.type)) { - return false; - } - return Arrays.equals(this.ctxAnnotations, that.ctxAnnotations); + return skipOverride == that.skipOverride && + schemaProperty == that.schemaProperty && + resolveAsRef == that.resolveAsRef && + resolveEnumAsRef == that.resolveEnumAsRef && + includePropertiesWithoutJSONView == that.includePropertiesWithoutJSONView && + skipSchemaName == that.skipSchemaName && + skipJsonIdentity == that.skipJsonIdentity && + Objects.equals(type, that.type) && + Objects.equals(name, that.name) && + Arrays.equals(ctxAnnotations, that.ctxAnnotations) && + Objects.equals(jsonViewAnnotation, that.jsonViewAnnotation); } - @Override public int hashCode() { - if (ctxAnnotations == null || ctxAnnotations.length == 0) { - return Objects.hash(type, "fixed"); - } - List meaningfulAnnotations = new ArrayList<>(); - - boolean hasDifference = false; - for (Annotation a: ctxAnnotations) { - if(!a.annotationType().getName().startsWith("sun") && !a.annotationType().getName().startsWith("jdk")) { - meaningfulAnnotations.add(a); - } else { - hasDifference = true; - } - } - int result = 1; - result = 31 * result + (type == null ? 0 : Objects.hash(type, "fixed")); - if (hasDifference) { - result = 31 * result + meaningfulAnnotations.hashCode(); - } else { - result = 31 * result + Arrays.hashCode(ctxAnnotations); - } + int result = Objects.hash(type, name, skipOverride, schemaProperty, resolveAsRef, + resolveEnumAsRef, jsonViewAnnotation, includePropertiesWithoutJSONView, skipSchemaName, + skipJsonIdentity); + result = 31 * result + Arrays.hashCode(ctxAnnotations); return result; } } diff --git a/modules/swagger-core/src/test/java/io/swagger/v3/core/converting/AnnotatedTypeCachingTest.java b/modules/swagger-core/src/test/java/io/swagger/v3/core/converting/AnnotatedTypeCachingTest.java new file mode 100644 index 0000000000..c79340ca80 --- /dev/null +++ b/modules/swagger-core/src/test/java/io/swagger/v3/core/converting/AnnotatedTypeCachingTest.java @@ -0,0 +1,72 @@ +package io.swagger.v3.core.converting; + +import io.swagger.v3.core.converter.AnnotatedType; +import io.swagger.v3.core.converter.ModelConverter; +import io.swagger.v3.core.converter.ModelConverterContext; +import io.swagger.v3.core.converter.ModelConverterContextImpl; +import io.swagger.v3.oas.models.media.Schema; +import org.testng.annotations.Test; + +import java.lang.reflect.Field; +import java.util.Iterator; +import java.util.Set; + +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertNotNull; + +public class AnnotatedTypeCachingTest { + + @Test + public void testAnnotatedTypeEqualityIgnoresContextualFields() { + AnnotatedType type1 = new AnnotatedType(String.class) + .propertyName("userStatus"); + AnnotatedType type2 = new AnnotatedType(String.class) + .propertyName("city"); + assertEquals(type1, type2, "AnnotatedType objects with different contextual fields (e.g., propertyName) should be equal."); + assertEquals(type1.hashCode(), type2.hashCode(), "The hash codes of equal AnnotatedType objects must be the same."); + } + + static class User { + public String username; + public String email; + public Address address; + } + + static class Address { + public String street; + public String city; + } + + private static class DummyModelConverter implements ModelConverter { + @Override + public Schema resolve(AnnotatedType type, ModelConverterContext context, Iterator chain) { + if (type.getType().equals(User.class)) { + context.resolve(new AnnotatedType(String.class).propertyName("username")); + context.resolve(new AnnotatedType(String.class).propertyName("email")); + context.resolve(new AnnotatedType(Address.class).propertyName("address")); + return new Schema(); + } + if (type.getType().equals(Address.class)) { + context.resolve(new AnnotatedType(String.class).propertyName("street")); + context.resolve(new AnnotatedType(String.class).propertyName("city")); + return new Schema(); + } + return new Schema(); + } + } + + @Test + @SuppressWarnings("unchecked") + public void testCacheHitsForRepeatedStringTypeWithCorrectedEquals() throws Exception { + ModelConverterContextImpl context = new ModelConverterContextImpl(new DummyModelConverter()); + Schema userSchema = context.resolve(new AnnotatedType(User.class)); + assertNotNull(userSchema); + Field processedTypesField = ModelConverterContextImpl.class.getDeclaredField("processedTypes"); + processedTypesField.setAccessible(true); + Set processedTypes = (Set) processedTypesField.get(context); + long stringTypeCount = processedTypes.stream() + .filter(annotatedType -> annotatedType.getType().equals(String.class)) + .count(); + assertEquals(stringTypeCount, 1, "With the correct equals/hashCode, String type should be added to the cache only once."); + } +} \ No newline at end of file From aa2cbb25d24f38ae8b009d7fa6546f1e099a2bfb Mon Sep 17 00:00:00 2001 From: potato <65760583+juntae6942@users.noreply.github.com> Date: Thu, 2 Oct 2025 16:32:10 +0900 Subject: [PATCH 2/3] fix(converter): Refactor AnnotatedType equals/hashCode for correctness and caching --- .../v3/core/converter/AnnotatedType.java | 34 ++++-- .../v3/core/converting/AnnotatedTypeTest.java | 105 ++++++++++++++++++ 2 files changed, 131 insertions(+), 8 deletions(-) create mode 100644 modules/swagger-core/src/test/java/io/swagger/v3/core/converting/AnnotatedTypeTest.java diff --git a/modules/swagger-core/src/main/java/io/swagger/v3/core/converter/AnnotatedType.java b/modules/swagger-core/src/main/java/io/swagger/v3/core/converter/AnnotatedType.java index 1d6734ebd1..473b3bb92a 100644 --- a/modules/swagger-core/src/main/java/io/swagger/v3/core/converter/AnnotatedType.java +++ b/modules/swagger-core/src/main/java/io/swagger/v3/core/converter/AnnotatedType.java @@ -8,9 +8,11 @@ import java.lang.reflect.Type; import java.util.ArrayList; import java.util.Arrays; +import java.util.Comparator; import java.util.List; import java.util.Objects; import java.util.function.Function; +import java.util.stream.Collectors; public class AnnotatedType { private Type type; @@ -155,11 +157,12 @@ public AnnotatedType name(String name) { } public Annotation[] getCtxAnnotations() { - return ctxAnnotations; +// return ctxAnnotations; + return ctxAnnotations == null ? null : Arrays.copyOf(ctxAnnotations, ctxAnnotations.length); } public void setCtxAnnotations(Annotation[] ctxAnnotations) { - this.ctxAnnotations = ctxAnnotations; + this.ctxAnnotations = ctxAnnotations == null ? null : Arrays.copyOf(ctxAnnotations, ctxAnnotations.length); } public AnnotatedType ctxAnnotations(Annotation[] ctxAnnotations) { @@ -241,11 +244,26 @@ public AnnotatedType propertyName(String propertyName) { return this; } + private List getProcessedAnnotations(Annotation[] annotations) { + if (annotations == null || annotations.length == 0) { + return new ArrayList<>(); + } + return Arrays.stream(annotations) + .filter(a -> { + String pkg = a.annotationType().getPackage().getName(); + return !pkg.startsWith("java.") && !pkg.startsWith("jdk.") && !pkg.startsWith("sun."); + }) + .sorted(Comparator.comparing(a -> a.annotationType().getName())) + .collect(Collectors.toList()); + } + @Override public boolean equals(Object o) { if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; + if (!(o instanceof AnnotatedType)) return false; AnnotatedType that = (AnnotatedType) o; + List thisAnnotatinons = getProcessedAnnotations(this.ctxAnnotations); + List thatAnnotatinons = getProcessedAnnotations(that.ctxAnnotations); return skipOverride == that.skipOverride && schemaProperty == that.schemaProperty && resolveAsRef == that.resolveAsRef && @@ -255,16 +273,16 @@ public boolean equals(Object o) { skipJsonIdentity == that.skipJsonIdentity && Objects.equals(type, that.type) && Objects.equals(name, that.name) && - Arrays.equals(ctxAnnotations, that.ctxAnnotations) && + Objects.equals(thisAnnotatinons, thatAnnotatinons) && Objects.equals(jsonViewAnnotation, that.jsonViewAnnotation); } @Override public int hashCode() { - int result = Objects.hash(type, name, skipOverride, schemaProperty, resolveAsRef, + List processedAnnotations = getProcessedAnnotations(this.ctxAnnotations); + + return Objects.hash(type, name, skipOverride, schemaProperty, resolveAsRef, resolveEnumAsRef, jsonViewAnnotation, includePropertiesWithoutJSONView, skipSchemaName, - skipJsonIdentity); - result = 31 * result + Arrays.hashCode(ctxAnnotations); - return result; + skipJsonIdentity, processedAnnotations); } } diff --git a/modules/swagger-core/src/test/java/io/swagger/v3/core/converting/AnnotatedTypeTest.java b/modules/swagger-core/src/test/java/io/swagger/v3/core/converting/AnnotatedTypeTest.java new file mode 100644 index 0000000000..855890273c --- /dev/null +++ b/modules/swagger-core/src/test/java/io/swagger/v3/core/converting/AnnotatedTypeTest.java @@ -0,0 +1,105 @@ +package io.swagger.v3.core.converting; + +import io.swagger.v3.core.converter.AnnotatedType; +import org.testng.annotations.Test; + +import java.lang.annotation.Annotation; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; +import java.lang.reflect.Type; +import java.util.HashSet; +import java.util.Set; + +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertNotEquals; +import static org.testng.Assert.assertTrue; + +public class AnnotatedTypeTest { + + @Retention(RetentionPolicy.RUNTIME) + @Target(ElementType.TYPE) + @interface TestAnnA {} + + @Retention(RetentionPolicy.RUNTIME) + @Target(ElementType.TYPE) + @interface TestAnnB {} + + @TestAnnA + @TestAnnB + @Deprecated + private static class AnnotationHolder {} + + private Annotation getAnnotationInstance(Class clazz) { + return AnnotationHolder.class.getAnnotation(clazz); + } + + /** + * Tests that equals() and hashCode() are order-insensitive for context annotations. + */ + @Test + public void testEqualsAndHashCode_shouldBeOrderInsensitiveForAnnotations() { + Annotation annA = getAnnotationInstance(TestAnnA.class); + Annotation annB = getAnnotationInstance(TestAnnB.class); + AnnotatedType type1 = new AnnotatedType(String.class).ctxAnnotations(new Annotation[]{annA, annB}); + AnnotatedType type2 = new AnnotatedType(String.class).ctxAnnotations(new Annotation[]{annB, annA}); + assertEquals(type1, type2, "Objects should be equal even if annotation order is different."); + assertEquals(type1.hashCode(), type2.hashCode(), "Hash codes should be equal even if annotation order is different."); + } + + /** + * Tests that JDK/internal annotations are filtered out for equals() and hashCode() comparison. + */ + @Test + public void testEqualsAndHashCode_shouldIgnoreJdkInternalAnnotations() { + Annotation annA = getAnnotationInstance(TestAnnA.class); + Annotation deprecated = getAnnotationInstance(Deprecated.class); + AnnotatedType typeWithUserAnn = new AnnotatedType(String.class).ctxAnnotations(new Annotation[]{annA}); + AnnotatedType typeWithJdkAnn = new AnnotatedType(String.class).ctxAnnotations(new Annotation[]{annA, deprecated}); + AnnotatedType typeWithOnlyJdkAnn = new AnnotatedType(String.class).ctxAnnotations(new Annotation[]{deprecated}); + AnnotatedType typeWithNoAnn = new AnnotatedType(String.class); + assertEquals(typeWithUserAnn, typeWithJdkAnn, "JDK annotations should be ignored in equality comparison."); + assertEquals(typeWithUserAnn.hashCode(), typeWithJdkAnn.hashCode(), "JDK annotations should be ignored in hashCode calculation."); + assertEquals(typeWithOnlyJdkAnn, typeWithNoAnn, "An object with only JDK annotations should be equal to one with no annotations."); + assertEquals(typeWithOnlyJdkAnn.hashCode(), typeWithNoAnn.hashCode(), "The hash code of an object with only JDK annotations should be the same as one with no annotations."); + } + + /** + * Tests that defensive copying prevents Set corruption from external array mutation. + */ + @Test + public void testImmutability_shouldPreventCorruptionInHashSet() { + Annotation annA = getAnnotationInstance(TestAnnA.class); + Annotation annB = getAnnotationInstance(TestAnnB.class); + Annotation[] originalAnnotations = new Annotation[]{annA}; + AnnotatedType type = new AnnotatedType(String.class).ctxAnnotations(originalAnnotations); + Set typeSet = new HashSet<>(); + typeSet.add(type); + int initialHashCode = type.hashCode(); + originalAnnotations[0] = annB; + assertEquals(initialHashCode, type.hashCode(), "Hash code must remain the same after mutating the external array."); + assertTrue(typeSet.contains(type), "The Set must still contain the object after mutating the external array."); + } + + /** + * Tests that an instance of a subclass can be equal to an instance of the parent class. + */ + @Test + public void testEqualsAndHashCode_shouldAllowSubclassEquality() { + class SubAnnotatedType extends AnnotatedType { + public SubAnnotatedType(Type type) { super(type); } + } + Annotation annA = getAnnotationInstance(TestAnnA.class); + Annotation[] annotations = {annA}; + AnnotatedType parent = new AnnotatedType(Integer.class).ctxAnnotations(annotations).name("number"); + SubAnnotatedType child = new SubAnnotatedType(Integer.class); + child.ctxAnnotations(annotations); + child.name("number"); + AnnotatedType differentParent = new AnnotatedType(Long.class).name("number"); + assertEquals(parent, child, "Parent and child objects should be equal if their properties are the same."); + assertEquals(child, parent, "Equality comparison should be symmetric."); + assertEquals(parent.hashCode(), child.hashCode(), "Parent and child hash codes should be equal if their properties are the same."); + assertNotEquals(parent, differentParent, "Objects with different properties should not be equal."); + } +} \ No newline at end of file From dfb90769253009fc8ecf32ec1a584d7d1943247c Mon Sep 17 00:00:00 2001 From: potato <65760583+juntae6942@users.noreply.github.com> Date: Thu, 2 Oct 2025 16:36:58 +0900 Subject: [PATCH 3/3] style: Remove comments --- .../main/java/io/swagger/v3/core/converter/AnnotatedType.java | 1 - 1 file changed, 1 deletion(-) diff --git a/modules/swagger-core/src/main/java/io/swagger/v3/core/converter/AnnotatedType.java b/modules/swagger-core/src/main/java/io/swagger/v3/core/converter/AnnotatedType.java index 473b3bb92a..a808b99bf5 100644 --- a/modules/swagger-core/src/main/java/io/swagger/v3/core/converter/AnnotatedType.java +++ b/modules/swagger-core/src/main/java/io/swagger/v3/core/converter/AnnotatedType.java @@ -157,7 +157,6 @@ public AnnotatedType name(String name) { } public Annotation[] getCtxAnnotations() { -// return ctxAnnotations; return ctxAnnotations == null ? null : Arrays.copyOf(ctxAnnotations, ctxAnnotations.length); }