From bac2643b35422e498ee11509d6f19759bb5ec787 Mon Sep 17 00:00:00 2001 From: Gregor Zeitlinger Date: Tue, 7 Oct 2025 13:14:59 +0200 Subject: [PATCH 01/10] convert test --- .../muzzle/ReferenceMatcherTest.groovy | 313 ++++++------- .../tooling/muzzle/ReferenceMatcherTest.java | 439 ++++++++++++++++++ 2 files changed, 596 insertions(+), 156 deletions(-) create mode 100644 muzzle/src/test/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcherTest.java diff --git a/muzzle/src/test/groovy/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcherTest.groovy b/muzzle/src/test/groovy/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcherTest.groovy index 51e270b863c1..4f6102397be0 100644 --- a/muzzle/src/test/groovy/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcherTest.groovy +++ b/muzzle/src/test/groovy/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcherTest.groovy @@ -5,6 +5,8 @@ package io.opentelemetry.javaagent.tooling.muzzle +import static org.assertj.core.api.Assertions.assertThat + import external.LibraryBaseClass import io.opentelemetry.instrumentation.TestHelperClasses import io.opentelemetry.instrumentation.test.utils.ClasspathUtils @@ -16,10 +18,12 @@ import io.opentelemetry.test.TestAbstractSuperClass import io.opentelemetry.test.TestInterface import muzzle.TestClasses import muzzle.TestClasses.Nested +import org.junit.jupiter.api.BeforeAll +import org.junit.jupiter.api.Test +import org.junit.jupiter.params.ParameterizedTest +import org.junit.jupiter.params.provider.CsvSource +import org.junit.jupiter.params.provider.ValueSource import org.objectweb.asm.Type -import spock.lang.Shared -import spock.lang.Specification -import spock.lang.Unroll import static io.opentelemetry.javaagent.tooling.muzzle.references.Flag.ManifestationFlag.ABSTRACT import static io.opentelemetry.javaagent.tooling.muzzle.references.Flag.ManifestationFlag.INTERFACE @@ -30,37 +34,40 @@ import static io.opentelemetry.javaagent.tooling.muzzle.references.Flag.MinimumV import static io.opentelemetry.javaagent.tooling.muzzle.references.Flag.OwnershipFlag.NON_STATIC import static io.opentelemetry.javaagent.tooling.muzzle.references.Flag.OwnershipFlag.STATIC -@Unroll -class ReferenceMatcherTest extends Specification { +class ReferenceMatcherTest { static final TEST_EXTERNAL_INSTRUMENTATION_PACKAGE = "com.external.otel.instrumentation" - @Shared - ClassLoader safeClasspath = new URLClassLoader([ClasspathUtils.createJarWithClasses(Nested.A, - Nested.B, - Nested.SomeInterface, - Nested.SomeImplementation)] as URL[], - (ClassLoader) null) - - @Shared - ClassLoader unsafeClasspath = new URLClassLoader([ClasspathUtils.createJarWithClasses(Nested.A, - Nested.SomeInterface, - Nested.SomeImplementation)] as URL[], - (ClassLoader) null) - - def "match safe classpaths"() { - setup: + static ClassLoader safeClasspath + static ClassLoader unsafeClasspath + + @BeforeAll + static void setup() { + safeClasspath = new URLClassLoader([ClasspathUtils.createJarWithClasses(Nested.A, + Nested.B, + Nested.SomeInterface, + Nested.SomeImplementation)] as URL[], + (ClassLoader) null) + + unsafeClasspath = new URLClassLoader([ClasspathUtils.createJarWithClasses(Nested.A, + Nested.SomeInterface, + Nested.SomeImplementation)] as URL[], + (ClassLoader) null) + } + + @Test + void matchSafeClasspaths() { def collector = new ReferenceCollector({ false }) collector.collectReferencesFromAdvice(TestClasses.MethodBodyAdvice.name) def refMatcher = createMatcher(collector.getReferences()) - expect: - getMismatchClassSet(refMatcher.getMismatchedReferenceSources(safeClasspath)).empty - getMismatchClassSet(refMatcher.getMismatchedReferenceSources(unsafeClasspath)) == [Mismatch.MissingClass] as Set + assertThat(getMismatchClassSet(refMatcher.getMismatchedReferenceSources(safeClasspath))).isEmpty() + assertThat(getMismatchClassSet(refMatcher.getMismatchedReferenceSources(unsafeClasspath))) + .containsExactly(Mismatch.MissingClass) } - def "matching does not hold a strong reference to classloaders"() { - expect: - MuzzleWeakReferenceTestUtil.classLoaderRefIsGarbageCollected() + @Test + void matchingDoesNotHoldStrongReferenceToClassloaders() { + assertThat(MuzzleWeakReferenceTestUtil.classLoaderRefIsGarbageCollected()).isTrue() } private static class CountingClassLoader extends URLClassLoader { @@ -77,8 +84,8 @@ class ReferenceMatcherTest extends Specification { } } - def "muzzle type pool caches"() { - setup: + @Test + void muzzleTypePoolCaches() { def cl = new CountingClassLoader( [ClasspathUtils.createJarWithClasses(Nested.A, Nested.B, @@ -91,146 +98,140 @@ class ReferenceMatcherTest extends Specification { def refMatcher1 = createMatcher(collector.getReferences()) def refMatcher2 = createMatcher(collector.getReferences()) - assert getMismatchClassSet(refMatcher1.getMismatchedReferenceSources(cl)).empty + assertThat(getMismatchClassSet(refMatcher1.getMismatchedReferenceSources(cl))).isEmpty() int countAfterFirstMatch = cl.count // the second matcher should be able to used cached type descriptions from the first - assert getMismatchClassSet(refMatcher2.getMismatchedReferenceSources(cl)).empty + assertThat(getMismatchClassSet(refMatcher2.getMismatchedReferenceSources(cl))).isEmpty() - expect: - cl.count == countAfterFirstMatch + assertThat(cl.count).isEqualTo(countAfterFirstMatch) } - def "matching ref #referenceName #referenceFlag against #classToCheck produces #expectedMismatches"() { - setup: + @ParameterizedTest + @CsvSource([ + "muzzle.TestClasses\$Nested\$B, NON_INTERFACE, ''", + "muzzle.TestClasses\$Nested\$B, INTERFACE, MissingFlag" + ]) + void matchingRef(String referenceName, String referenceFlag, String expectedMismatch) { + def flag = referenceFlag == "NON_INTERFACE" ? NON_INTERFACE : INTERFACE def ref = ClassRef.builder(referenceName) - .addFlag(referenceFlag) + .addFlag(flag) .build() - when: def mismatches = createMatcher([(ref.className): ref]).getMismatchedReferenceSources(this.class.classLoader) - then: - getMismatchClassSet(mismatches) == expectedMismatches as Set - - where: - referenceName | referenceFlag | classToCheck | expectedMismatches - Nested.B.name | NON_INTERFACE | Nested.B | [] - Nested.B.name | INTERFACE | Nested.B | [Mismatch.MissingFlag] + if (expectedMismatch.isEmpty()) { + assertThat(getMismatchClassSet(mismatches)).isEmpty() + } else { + assertThat(getMismatchClassSet(mismatches)).containsExactly(Mismatch.MissingFlag) + } } - def "method match #methodTestDesc"() { - setup: + @ParameterizedTest + @CsvSource([ + "method, (Ljava/lang/String;)Ljava/lang/String;, '', muzzle.TestClasses\$Nested\$B, ''", + "hashCode, ()I, '', muzzle.TestClasses\$Nested\$B, ''", + "someMethod, ()V, '', muzzle.TestClasses\$Nested\$SomeInterface, ''", + "privateStuff, ()V, PRIVATE_OR_HIGHER, muzzle.TestClasses\$Nested\$B, ''", + "privateStuff, ()V, PROTECTED_OR_HIGHER, muzzle.TestClasses\$Nested\$B2, MissingFlag", + "staticMethod, ()V, NON_STATIC, muzzle.TestClasses\$Nested\$B, MissingFlag", + "missingMethod, ()V, '', muzzle.TestClasses\$Nested\$B, MissingMethod" + ]) + void methodMatch(String methodName, String methodDesc, String methodFlagsStr, String classToCheckName, String expectedMismatch) { def methodType = Type.getMethodType(methodDesc) + def methodFlags = parseMethodFlags(methodFlagsStr) + def classToCheck = Class.forName(classToCheckName) + def reference = ClassRef.builder(classToCheck.name) .addMethod(new Source[0], methodFlags as Flag[], methodName, methodType.returnType, methodType.argumentTypes) .build() - when: def mismatches = createMatcher([(reference.className): reference]) .getMismatchedReferenceSources(this.class.classLoader) - then: - getMismatchClassSet(mismatches) == expectedMismatches as Set - - where: - methodName | methodDesc | methodFlags | classToCheck | expectedMismatches | methodTestDesc - "method" | "(Ljava/lang/String;)Ljava/lang/String;" | [] | Nested.B | [] | "match method declared in class" - "hashCode" | "()I" | [] | Nested.B | [] | "match method declared in superclass" - "someMethod" | "()V" | [] | Nested.SomeInterface | [] | "match method declared in interface" - "privateStuff" | "()V" | [PRIVATE_OR_HIGHER] | Nested.B | [] | "match private method" - "privateStuff" | "()V" | [PROTECTED_OR_HIGHER] | Nested.B2 | [Mismatch.MissingFlag] | "fail match private in supertype" - "staticMethod" | "()V" | [NON_STATIC] | Nested.B | [Mismatch.MissingFlag] | "static method mismatch" - "missingMethod" | "()V" | [] | Nested.B | [Mismatch.MissingMethod] | "missing method mismatch" + if (expectedMismatch.isEmpty()) { + assertThat(getMismatchClassSet(mismatches)).isEmpty() + } else { + def expectedMismatchClass = getMismatchClass(expectedMismatch) + assertThat(getMismatchClassSet(mismatches)).containsExactly(expectedMismatchClass) + } } - def "field match #fieldTestDesc"() { - setup: + @ParameterizedTest + @CsvSource([ + "missingField, Ljava/lang/String;, '', muzzle.TestClasses\$Nested\$A, MissingField", + "privateField, Ljava/lang/String;, '', muzzle.TestClasses\$Nested\$A, MissingField", + "privateField, Ljava/lang/Object;, PRIVATE_OR_HIGHER, muzzle.TestClasses\$Nested\$A, ''", + "privateField, Ljava/lang/Object;, PROTECTED_OR_HIGHER, muzzle.TestClasses\$Nested\$A2, MissingFlag", + "protectedField, Ljava/lang/Object;, STATIC, muzzle.TestClasses\$Nested\$A, MissingFlag", + "staticB, Lmuzzle/TestClasses\$Nested\$B;, STATIC|PROTECTED_OR_HIGHER, muzzle.TestClasses\$Nested\$A, ''", + "number, I, PACKAGE_OR_HIGHER, muzzle.TestClasses\$Nested\$Primitives, ''", + "flag, Z, PACKAGE_OR_HIGHER, muzzle.TestClasses\$Nested\$Primitives, ''" + ]) + void fieldMatch(String fieldName, String fieldType, String fieldFlagsStr, String classToCheckName, String expectedMismatch) { + def fieldFlags = parseFieldFlags(fieldFlagsStr) + def classToCheck = Class.forName(classToCheckName) + def reference = ClassRef.builder(classToCheck.name) .addField(new Source[0], fieldFlags as Flag[], fieldName, Type.getType(fieldType), false) .build() - when: def mismatches = createMatcher([(reference.className): reference]) .getMismatchedReferenceSources(this.class.classLoader) - then: - getMismatchClassSet(mismatches) == expectedMismatches as Set - - where: - fieldName | fieldType | fieldFlags | classToCheck | expectedMismatches | fieldTestDesc - "missingField" | "Ljava/lang/String;" | [] | Nested.A | [Mismatch.MissingField] | "mismatch missing field" - "privateField" | "Ljava/lang/String;" | [] | Nested.A | [Mismatch.MissingField] | "mismatch field type signature" - "privateField" | "Ljava/lang/Object;" | [PRIVATE_OR_HIGHER] | Nested.A | [] | "match private field" - "privateField" | "Ljava/lang/Object;" | [PROTECTED_OR_HIGHER] | Nested.A2 | [Mismatch.MissingFlag] | "mismatch private field in supertype" - "protectedField" | "Ljava/lang/Object;" | [STATIC] | Nested.A | [Mismatch.MissingFlag] | "mismatch static field" - "staticB" | Type.getType(Nested.B).getDescriptor() | [STATIC, PROTECTED_OR_HIGHER] | Nested.A | [] | "match static field" - "number" | "I" | [PACKAGE_OR_HIGHER] | Nested.Primitives | [] | "match primitive int" - "flag" | "Z" | [PACKAGE_OR_HIGHER] | Nested.Primitives | [] | "match primitive boolean" + if (expectedMismatch.isEmpty()) { + assertThat(getMismatchClassSet(mismatches)).isEmpty() + } else { + def expectedMismatchClass = getMismatchClass(expectedMismatch) + assertThat(getMismatchClassSet(mismatches)).containsExactly(expectedMismatchClass) + } } - def "should not check abstract #desc helper classes"() { - given: + @ParameterizedTest + @ValueSource(strings = ["io.opentelemetry.instrumentation.Helper", "com.external.otel.instrumentation.Helper"]) + void shouldNotCheckAbstractHelperClasses(String className) { def reference = ClassRef.builder(className) .setSuperClassName(TestHelperClasses.HelperSuperClass.name) .addFlag(ABSTRACT) .addMethod(new Source[0], [ABSTRACT] as Flag[], "unimplemented", Type.VOID_TYPE) .build() - when: def mismatches = createMatcher([(reference.className): reference], [reference.className]) .getMismatchedReferenceSources(this.class.classLoader) - then: - mismatches.empty - - where: - desc | className - "internal" | "io.opentelemetry.instrumentation.Helper" - "external" | "${TEST_EXTERNAL_INSTRUMENTATION_PACKAGE}.Helper" + assertThat(mismatches).isEmpty() } - def "should not check #desc helper classes with no supertypes"() { - given: + @ParameterizedTest + @ValueSource(strings = ["io.opentelemetry.instrumentation.Helper", "com.external.otel.instrumentation.Helper"]) + void shouldNotCheckHelperClassesWithNoSupertypes(String className) { def reference = ClassRef.builder(className) .setSuperClassName(Object.name) .addMethod(new Source[0], [] as Flag[], "someMethod", Type.VOID_TYPE) .build() - when: def mismatches = createMatcher([(reference.className): reference], [reference.className]) .getMismatchedReferenceSources(this.class.classLoader) - then: - mismatches.empty - - where: - desc | className - "internal" | "io.opentelemetry.instrumentation.Helper" - "external" | "${TEST_EXTERNAL_INSTRUMENTATION_PACKAGE}.Helper" + assertThat(mismatches).isEmpty() } - def "should fail #desc helper classes that does not implement all abstract methods"() { - given: + @ParameterizedTest + @ValueSource(strings = ["io.opentelemetry.instrumentation.Helper", "com.external.otel.instrumentation.Helper"]) + void shouldFailHelperClassesThatDoesNotImplementAllAbstractMethods(String className) { def reference = ClassRef.builder(className) .setSuperClassName(TestAbstractSuperClass.name) .addMethod(new Source[0], [] as Flag[], "someMethod", Type.VOID_TYPE) .build() - when: def mismatches = createMatcher([(reference.className): reference], [reference.className]) .getMismatchedReferenceSources(this.class.classLoader) - then: - getMismatchClassSet(mismatches) == [Mismatch.MissingMethod] as Set - - where: - desc | className - "internal" | "io.opentelemetry.instrumentation.Helper" - "external" | "${TEST_EXTERNAL_INSTRUMENTATION_PACKAGE}.Helper" + assertThat(getMismatchClassSet(mismatches)).containsExactly(Mismatch.MissingMethod) } - def "should fail #desc helper classes that do not implement all abstract methods - even if empty abstract class reference exists"() { - given: + @ParameterizedTest + @ValueSource(strings = ["io.opentelemetry.instrumentation.Helper", "com.external.otel.instrumentation.Helper"]) + void shouldFailHelperClassesThatDoNotImplementAllAbstractMethodsEvenIfEmptyAbstractClassReferenceExists(String className) { def emptySuperClassRef = ClassRef.builder(TestAbstractSuperClass.name) .build() def reference = ClassRef.builder(className) @@ -238,23 +239,17 @@ class ReferenceMatcherTest extends Specification { .addMethod(new Source[0], [] as Flag[], "someMethod", Type.VOID_TYPE) .build() - when: def mismatches = createMatcher( [(reference.className): reference, (emptySuperClassRef.className): emptySuperClassRef], [reference.className, emptySuperClassRef.className]) .getMismatchedReferenceSources(this.class.classLoader) - then: - getMismatchClassSet(mismatches) == [Mismatch.MissingMethod] as Set - - where: - desc | className - "internal" | "io.opentelemetry.instrumentation.Helper" - "external" | "${TEST_EXTERNAL_INSTRUMENTATION_PACKAGE}.Helper" + assertThat(getMismatchClassSet(mismatches)).containsExactly(Mismatch.MissingMethod) } - def "should check #desc helper class whether interface methods are implemented in the super class"() { - given: + @ParameterizedTest + @ValueSource(strings = ["io.opentelemetry.instrumentation.Helper", "com.external.otel.instrumentation.Helper"]) + void shouldCheckHelperClassWhetherInterfaceMethodsAreImplementedInTheSuperClass(String className) { def baseHelper = ClassRef.builder("io.opentelemetry.instrumentation.BaseHelper") .setSuperClassName(Object.name) .addInterfaceName(TestInterface.name) @@ -267,65 +262,50 @@ class ReferenceMatcherTest extends Specification { .addMethod(new Source[0], [] as Flag[], "bar", Type.VOID_TYPE) .build() - when: def mismatches = createMatcher( [(helper.className): helper, (baseHelper.className): baseHelper], [helper.className, baseHelper.className]) .getMismatchedReferenceSources(this.class.classLoader) - then: - mismatches.empty - - where: - desc | className - "internal" | "io.opentelemetry.instrumentation.Helper" - "external" | "${TEST_EXTERNAL_INSTRUMENTATION_PACKAGE}.Helper" + assertThat(mismatches).isEmpty() } - def "should check #desc helper class whether used fields are declared in the super class"() { - given: + @ParameterizedTest + @ValueSource(strings = ["io.opentelemetry.instrumentation.Helper", "com.external.otel.instrumentation.Helper"]) + void shouldCheckHelperClassWhetherUsedFieldsAreDeclaredInTheSuperClass(String className) { def helper = ClassRef.builder(className) .setSuperClassName(LibraryBaseClass.name) .addField(new Source[0], new Flag[0], "field", Type.getType("Ljava/lang/Integer;"), false) .build() - when: def mismatches = createMatcher([(helper.className): helper], [helper.className]) .getMismatchedReferenceSources(this.class.classLoader) - then: - mismatches.empty - - where: - desc | className - "internal" | "io.opentelemetry.instrumentation.Helper" - "external" | "${TEST_EXTERNAL_INSTRUMENTATION_PACKAGE}.Helper" + assertThat(mismatches).isEmpty() } - def "should fail helper class when it uses fields undeclared in the super class: #desc"() { - given: + @ParameterizedTest + @CsvSource([ + "io.opentelemetry.instrumentation.Helper, differentField, Ljava/lang/Integer;", + "io.opentelemetry.instrumentation.Helper, field, Lcom/external/DifferentType;", + "com.external.otel.instrumentation.Helper, differentField, Ljava/lang/Integer;", + "com.external.otel.instrumentation.Helper, field, Lcom/external/DifferentType;" + ]) + void shouldFailHelperClassWhenItUsesFieldsUndeclaredInTheSuperClass(String className, String fieldName, String fieldType) { def helper = ClassRef.builder(className) .setSuperClassName(DeclaredFieldTestClass.LibraryBaseClass.name) .addField(new Source[0], new Flag[0], fieldName, Type.getType(fieldType), false) .build() - when: def mismatches = createMatcher([(helper.className): helper], [helper.className]) .getMismatchedReferenceSources(this.class.classLoader) - then: - getMismatchClassSet(mismatches) == [Mismatch.MissingField] as Set - - where: - desc | className | fieldName | fieldType - "internal helper, different field name" | "io.opentelemetry.instrumentation.Helper" | "differentField" | "Ljava/lang/Integer;" - "internal helper, different field type" | "io.opentelemetry.instrumentation.Helper" | "field" | "Lcom/external/DifferentType;" - "external helper, different field name" | "${TEST_EXTERNAL_INSTRUMENTATION_PACKAGE}.Helper" | "differentField" | "Ljava/lang/Integer;" - "external helper, different field type" | "${TEST_EXTERNAL_INSTRUMENTATION_PACKAGE}.Helper" | "field" | "Lcom/external/DifferentType;" + assertThat(getMismatchClassSet(mismatches)).containsExactly(Mismatch.MissingField) } - def "should fail #desc helper class when the library parent class has different constructor"() { - given: + @ParameterizedTest + @ValueSource(strings = ["io.opentelemetry.instrumentation.Helper", "com.external.otel.instrumentation.Helper"]) + void shouldFailHelperClassWhenTheLibraryParentClassHasDifferentConstructor(String className) { def helper = ClassRef.builder(className) .setSuperClassName(TestClasses.BaseClassWithConstructor.name) .build() @@ -335,17 +315,10 @@ class ReferenceMatcherTest extends Specification { .addMethod(new Source[0], new Flag[0], "", Type.VOID_TYPE) .build() - when: def mismatches = createMatcher([(helper.className): helper, (baseClassRef.className): baseClassRef], [helper.className]) .getMismatchedReferenceSources(this.class.classLoader) - then: - getMismatchClassSet(mismatches) == [Mismatch.MissingMethod] as Set - - where: - desc | className - "internal" | "io.opentelemetry.instrumentation.Helper" - "external" | "${TEST_EXTERNAL_INSTRUMENTATION_PACKAGE}.Helper" + assertThat(getMismatchClassSet(mismatches)).containsExactly(Mismatch.MissingMethod) } private static ReferenceMatcher createMatcher(Map references = [:], @@ -360,4 +333,32 @@ class ReferenceMatcherTest extends Specification { } return mismatchClasses } + + private static List parseMethodFlags(String flagsStr) { + if (flagsStr.isEmpty()) return [] + def flags = [] + if (flagsStr.contains("PRIVATE_OR_HIGHER")) flags.add(PRIVATE_OR_HIGHER) + if (flagsStr.contains("PROTECTED_OR_HIGHER")) flags.add(PROTECTED_OR_HIGHER) + if (flagsStr.contains("NON_STATIC")) flags.add(NON_STATIC) + return flags + } + + private static List parseFieldFlags(String flagsStr) { + if (flagsStr.isEmpty()) return [] + def flags = [] + if (flagsStr.contains("PRIVATE_OR_HIGHER")) flags.add(PRIVATE_OR_HIGHER) + if (flagsStr.contains("PROTECTED_OR_HIGHER")) flags.add(PROTECTED_OR_HIGHER) + if (flagsStr.contains("PACKAGE_OR_HIGHER")) flags.add(PACKAGE_OR_HIGHER) + if (flagsStr.contains("STATIC")) flags.add(STATIC) + return flags + } + + private static Class getMismatchClass(String mismatchName) { + switch (mismatchName) { + case "MissingFlag": return Mismatch.MissingFlag + case "MissingMethod": return Mismatch.MissingMethod + case "MissingField": return Mismatch.MissingField + default: throw new IllegalArgumentException("Unknown mismatch: " + mismatchName) + } + } } diff --git a/muzzle/src/test/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcherTest.java b/muzzle/src/test/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcherTest.java new file mode 100644 index 000000000000..8182441bd954 --- /dev/null +++ b/muzzle/src/test/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcherTest.java @@ -0,0 +1,439 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.tooling.muzzle; + +import static io.opentelemetry.javaagent.tooling.muzzle.references.Flag.ManifestationFlag.ABSTRACT; +import static io.opentelemetry.javaagent.tooling.muzzle.references.Flag.ManifestationFlag.INTERFACE; +import static io.opentelemetry.javaagent.tooling.muzzle.references.Flag.ManifestationFlag.NON_INTERFACE; +import static io.opentelemetry.javaagent.tooling.muzzle.references.Flag.MinimumVisibilityFlag.PACKAGE_OR_HIGHER; +import static io.opentelemetry.javaagent.tooling.muzzle.references.Flag.MinimumVisibilityFlag.PRIVATE_OR_HIGHER; +import static io.opentelemetry.javaagent.tooling.muzzle.references.Flag.MinimumVisibilityFlag.PROTECTED_OR_HIGHER; +import static io.opentelemetry.javaagent.tooling.muzzle.references.Flag.OwnershipFlag.NON_STATIC; +import static io.opentelemetry.javaagent.tooling.muzzle.references.Flag.OwnershipFlag.STATIC; +import static org.assertj.core.api.Assertions.assertThat; + +import external.LibraryBaseClass; +import io.opentelemetry.instrumentation.TestHelperClasses; +import io.opentelemetry.instrumentation.test.utils.ClasspathUtils; +import io.opentelemetry.javaagent.tooling.muzzle.references.ClassRef; +import io.opentelemetry.javaagent.tooling.muzzle.references.Flag; +import io.opentelemetry.javaagent.tooling.muzzle.references.Source; +import io.opentelemetry.test.AnotherTestInterface; +import io.opentelemetry.test.TestAbstractSuperClass; +import io.opentelemetry.test.TestInterface; +import java.net.URL; +import java.net.URLClassLoader; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.TimeoutException; +import muzzle.TestClasses; +import muzzle.TestClasses.Nested; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; +import org.junit.jupiter.params.provider.ValueSource; +import org.objectweb.asm.Type; + +class ReferenceMatcherTest { + private static final String TEST_EXTERNAL_INSTRUMENTATION_PACKAGE = "com.external.otel.instrumentation"; + + private static ClassLoader safeClasspath; + private static ClassLoader unsafeClasspath; + + @BeforeAll + static void setup() throws Exception { + safeClasspath = new URLClassLoader( + new URL[] { + ClasspathUtils.createJarWithClasses( + Nested.A.class, + Nested.B.class, + Nested.SomeInterface.class, + Nested.SomeImplementation.class) + }, + null); + + unsafeClasspath = new URLClassLoader( + new URL[] { + ClasspathUtils.createJarWithClasses( + Nested.A.class, + Nested.SomeInterface.class, + Nested.SomeImplementation.class) + }, + null); + } + + @Test + void matchSafeClasspaths() { + ReferenceCollector collector = new ReferenceCollector(className -> false); + collector.collectReferencesFromAdvice(TestClasses.MethodBodyAdvice.class.getName()); + ReferenceMatcher refMatcher = createMatcher(collector.getReferences()); + + assertThat(getMismatchClassSet(refMatcher.getMismatchedReferenceSources(safeClasspath))).isEmpty(); + assertThat(getMismatchClassSet(refMatcher.getMismatchedReferenceSources(unsafeClasspath))) + .containsExactly(Mismatch.MissingClass.class); + } + + @Test + void matchingDoesNotHoldStrongReferenceToClassloaders() { + try { + assertThat(MuzzleWeakReferenceTestUtil.classLoaderRefIsGarbageCollected()).isTrue(); + } catch (InterruptedException | TimeoutException e) { + throw new RuntimeException(e); + } + } + + private static class CountingClassLoader extends URLClassLoader { + int count = 0; + + CountingClassLoader(URL[] urls, ClassLoader parent) { + super(urls, parent); + } + + @Override + public URL getResource(String name) { + count++; + return super.getResource(name); + } + } + + @Test + void muzzleTypePoolCaches() throws Exception { + CountingClassLoader cl = new CountingClassLoader( + new URL[] { + ClasspathUtils.createJarWithClasses( + Nested.A.class, + Nested.B.class, + Nested.SomeInterface.class, + Nested.SomeImplementation.class) + }, + null); + + ReferenceCollector collector = new ReferenceCollector(className -> false); + collector.collectReferencesFromAdvice(Nested.class.getName()); + + ReferenceMatcher refMatcher1 = createMatcher(collector.getReferences()); + ReferenceMatcher refMatcher2 = createMatcher(collector.getReferences()); + assertThat(getMismatchClassSet(refMatcher1.getMismatchedReferenceSources(cl))).isEmpty(); + int countAfterFirstMatch = cl.count; + // the second matcher should be able to used cached type descriptions from the first + assertThat(getMismatchClassSet(refMatcher2.getMismatchedReferenceSources(cl))).isEmpty(); + + assertThat(cl.count).isEqualTo(countAfterFirstMatch); + } + + @ParameterizedTest + @CsvSource({ + "muzzle.TestClasses$Nested$B, NON_INTERFACE, ''", + "muzzle.TestClasses$Nested$B, INTERFACE, MissingFlag" + }) + void matchingRef(String referenceName, String referenceFlag, String expectedMismatch) { + Flag flag = "NON_INTERFACE".equals(referenceFlag) ? NON_INTERFACE : INTERFACE; + ClassRef ref = ClassRef.builder(referenceName) + .addFlag(flag) + .build(); + + List mismatches = createMatcher(Collections.singletonMap(ref.getClassName(), ref)) + .getMismatchedReferenceSources(this.getClass().getClassLoader()); + + if (expectedMismatch.isEmpty()) { + assertThat(getMismatchClassSet(mismatches)).isEmpty(); + } else { + assertThat(getMismatchClassSet(mismatches)).containsExactly(Mismatch.MissingFlag.class); + } + } + + @ParameterizedTest + @CsvSource({ + "method, (Ljava/lang/String;)Ljava/lang/String;, '', muzzle.TestClasses$Nested$B, ''", + "hashCode, ()I, '', muzzle.TestClasses$Nested$B, ''", + "someMethod, ()V, '', muzzle.TestClasses$Nested$SomeInterface, ''", + "privateStuff, ()V, PRIVATE_OR_HIGHER, muzzle.TestClasses$Nested$B, ''", + "privateStuff, ()V, PROTECTED_OR_HIGHER, muzzle.TestClasses$Nested$B2, MissingFlag", + "staticMethod, ()V, NON_STATIC, muzzle.TestClasses$Nested$B, MissingFlag", + "missingMethod, ()V, '', muzzle.TestClasses$Nested$B, MissingMethod" + }) + void methodMatch(String methodName, String methodDesc, String methodFlagsStr, String classToCheckName, String expectedMismatch) + throws ClassNotFoundException { + Type methodType = Type.getMethodType(methodDesc); + List methodFlags = parseMethodFlags(methodFlagsStr); + Class classToCheck = Class.forName(classToCheckName); + + ClassRef reference = ClassRef.builder(classToCheck.getName()) + .addMethod(new Source[0], methodFlags.toArray(new Flag[0]), methodName, methodType.getReturnType(), methodType.getArgumentTypes()) + .build(); + + List mismatches = createMatcher(Collections.singletonMap(reference.getClassName(), reference)) + .getMismatchedReferenceSources(this.getClass().getClassLoader()); + + if (expectedMismatch.isEmpty()) { + assertThat(getMismatchClassSet(mismatches)).isEmpty(); + } else { + Class expectedMismatchClass = getMismatchClass(expectedMismatch); + assertThat(getMismatchClassSet(mismatches)).containsExactly(expectedMismatchClass); + } + } + + @ParameterizedTest + @CsvSource({ + "missingField, Ljava/lang/String;, '', muzzle.TestClasses$Nested$A, MissingField", + "privateField, Ljava/lang/String;, '', muzzle.TestClasses$Nested$A, MissingField", + "privateField, Ljava/lang/Object;, PRIVATE_OR_HIGHER, muzzle.TestClasses$Nested$A, ''", + "privateField, Ljava/lang/Object;, PROTECTED_OR_HIGHER, muzzle.TestClasses$Nested$A2, MissingFlag", + "protectedField, Ljava/lang/Object;, STATIC, muzzle.TestClasses$Nested$A, MissingFlag", + "staticB, Lmuzzle/TestClasses$Nested$B;, STATIC|PROTECTED_OR_HIGHER, muzzle.TestClasses$Nested$A, ''", + "number, I, PACKAGE_OR_HIGHER, muzzle.TestClasses$Nested$Primitives, ''", + "flag, Z, PACKAGE_OR_HIGHER, muzzle.TestClasses$Nested$Primitives, ''" + }) + void fieldMatch(String fieldName, String fieldType, String fieldFlagsStr, String classToCheckName, String expectedMismatch) + throws ClassNotFoundException { + List fieldFlags = parseFieldFlags(fieldFlagsStr); + Class classToCheck = Class.forName(classToCheckName); + + ClassRef reference = ClassRef.builder(classToCheck.getName()) + .addField(new Source[0], fieldFlags.toArray(new Flag[0]), fieldName, Type.getType(fieldType), false) + .build(); + + List mismatches = createMatcher(Collections.singletonMap(reference.getClassName(), reference)) + .getMismatchedReferenceSources(this.getClass().getClassLoader()); + + if (expectedMismatch.isEmpty()) { + assertThat(getMismatchClassSet(mismatches)).isEmpty(); + } else { + Class expectedMismatchClass = getMismatchClass(expectedMismatch); + assertThat(getMismatchClassSet(mismatches)).containsExactly(expectedMismatchClass); + } + } + + @ParameterizedTest + @ValueSource(strings = {"io.opentelemetry.instrumentation.Helper", "com.external.otel.instrumentation.Helper"}) + void shouldNotCheckAbstractHelperClasses(String className) { + ClassRef reference = ClassRef.builder(className) + .setSuperClassName(TestHelperClasses.HelperSuperClass.class.getName()) + .addFlag(ABSTRACT) + .addMethod(new Source[0], new Flag[] {ABSTRACT}, "unimplemented", Type.VOID_TYPE) + .build(); + + List mismatches = createMatcher( + Collections.singletonMap(reference.getClassName(), reference), + Collections.singletonList(reference.getClassName())) + .getMismatchedReferenceSources(this.getClass().getClassLoader()); + + assertThat(mismatches).isEmpty(); + } + + @ParameterizedTest + @ValueSource(strings = {"io.opentelemetry.instrumentation.Helper", "com.external.otel.instrumentation.Helper"}) + void shouldNotCheckHelperClassesWithNoSupertypes(String className) { + ClassRef reference = ClassRef.builder(className) + .setSuperClassName(Object.class.getName()) + .addMethod(new Source[0], new Flag[0], "someMethod", Type.VOID_TYPE) + .build(); + + List mismatches = createMatcher( + Collections.singletonMap(reference.getClassName(), reference), + Collections.singletonList(reference.getClassName())) + .getMismatchedReferenceSources(this.getClass().getClassLoader()); + + assertThat(mismatches).isEmpty(); + } + + @ParameterizedTest + @ValueSource(strings = {"io.opentelemetry.instrumentation.Helper", "com.external.otel.instrumentation.Helper"}) + void shouldFailHelperClassesThatDoesNotImplementAllAbstractMethods(String className) { + ClassRef reference = ClassRef.builder(className) + .setSuperClassName(TestAbstractSuperClass.class.getName()) + .addMethod(new Source[0], new Flag[0], "someMethod", Type.VOID_TYPE) + .build(); + + List mismatches = createMatcher( + Collections.singletonMap(reference.getClassName(), reference), + Collections.singletonList(reference.getClassName())) + .getMismatchedReferenceSources(this.getClass().getClassLoader()); + + assertThat(getMismatchClassSet(mismatches)).containsExactly(Mismatch.MissingMethod.class); + } + + @ParameterizedTest + @ValueSource(strings = {"io.opentelemetry.instrumentation.Helper", "com.external.otel.instrumentation.Helper"}) + void shouldFailHelperClassesThatDoNotImplementAllAbstractMethodsEvenIfEmptyAbstractClassReferenceExists(String className) { + ClassRef emptySuperClassRef = ClassRef.builder(TestAbstractSuperClass.class.getName()) + .build(); + ClassRef reference = ClassRef.builder(className) + .setSuperClassName(TestAbstractSuperClass.class.getName()) + .addMethod(new Source[0], new Flag[0], "someMethod", Type.VOID_TYPE) + .build(); + + Map references = new HashMap<>(); + references.put(reference.getClassName(), reference); + references.put(emptySuperClassRef.getClassName(), emptySuperClassRef); + + List helperClasses = Arrays.asList(reference.getClassName(), emptySuperClassRef.getClassName()); + + List mismatches = createMatcher(references, helperClasses) + .getMismatchedReferenceSources(this.getClass().getClassLoader()); + + assertThat(getMismatchClassSet(mismatches)).containsExactly(Mismatch.MissingMethod.class); + } + + @ParameterizedTest + @ValueSource(strings = {"io.opentelemetry.instrumentation.Helper", "com.external.otel.instrumentation.Helper"}) + void shouldCheckHelperClassWhetherInterfaceMethodsAreImplementedInTheSuperClass(String className) { + ClassRef baseHelper = ClassRef.builder("io.opentelemetry.instrumentation.BaseHelper") + .setSuperClassName(Object.class.getName()) + .addInterfaceName(TestInterface.class.getName()) + .addMethod(new Source[0], new Flag[0], "foo", Type.VOID_TYPE) + .build(); + // abstract HelperInterface#foo() is implemented by BaseHelper + ClassRef helper = ClassRef.builder(className) + .setSuperClassName(baseHelper.getClassName()) + .addInterfaceName(AnotherTestInterface.class.getName()) + .addMethod(new Source[0], new Flag[0], "bar", Type.VOID_TYPE) + .build(); + + Map references = new HashMap<>(); + references.put(helper.getClassName(), helper); + references.put(baseHelper.getClassName(), baseHelper); + + List helperClasses = Arrays.asList(helper.getClassName(), baseHelper.getClassName()); + + List mismatches = createMatcher(references, helperClasses) + .getMismatchedReferenceSources(this.getClass().getClassLoader()); + + assertThat(mismatches).isEmpty(); + } + + @ParameterizedTest + @ValueSource(strings = {"io.opentelemetry.instrumentation.Helper", "com.external.otel.instrumentation.Helper"}) + void shouldCheckHelperClassWhetherUsedFieldsAreDeclaredInTheSuperClass(String className) { + ClassRef helper = ClassRef.builder(className) + .setSuperClassName(LibraryBaseClass.class.getName()) + .addField(new Source[0], new Flag[0], "field", Type.getType("Ljava/lang/Integer;"), false) + .build(); + + List mismatches = createMatcher( + Collections.singletonMap(helper.getClassName(), helper), + Collections.singletonList(helper.getClassName())) + .getMismatchedReferenceSources(this.getClass().getClassLoader()); + + assertThat(mismatches).isEmpty(); + } + + @ParameterizedTest + @CsvSource({ + "io.opentelemetry.instrumentation.Helper, differentField, Ljava/lang/Integer;", + "io.opentelemetry.instrumentation.Helper, field, Lcom/external/DifferentType;", + "com.external.otel.instrumentation.Helper, differentField, Ljava/lang/Integer;", + "com.external.otel.instrumentation.Helper, field, Lcom/external/DifferentType;" + }) + void shouldFailHelperClassWhenItUsesFieldsUndeclaredInTheSuperClass(String className, String fieldName, String fieldType) { + ClassRef helper = ClassRef.builder(className) + .setSuperClassName(io.opentelemetry.javaagent.tooling.muzzle.DeclaredFieldTestClass.LibraryBaseClass.class.getName()) + .addField(new Source[0], new Flag[0], fieldName, Type.getType(fieldType), false) + .build(); + + List mismatches = createMatcher( + Collections.singletonMap(helper.getClassName(), helper), + Collections.singletonList(helper.getClassName())) + .getMismatchedReferenceSources(this.getClass().getClassLoader()); + + assertThat(getMismatchClassSet(mismatches)).containsExactly(Mismatch.MissingField.class); + } + + @ParameterizedTest + @ValueSource(strings = {"io.opentelemetry.instrumentation.Helper", "com.external.otel.instrumentation.Helper"}) + void shouldFailHelperClassWhenTheLibraryParentClassHasDifferentConstructor(String className) { + ClassRef helper = ClassRef.builder(className) + .setSuperClassName(TestClasses.BaseClassWithConstructor.class.getName()) + .build(); + // muzzle codegen plugin has captured a no-arg constructor reference; + // the actual constructor of the base class on the classpath requires a long + ClassRef baseClassRef = ClassRef.builder(TestClasses.BaseClassWithConstructor.class.getName()) + .addMethod(new Source[0], new Flag[0], "", Type.VOID_TYPE) + .build(); + + Map references = new HashMap<>(); + references.put(helper.getClassName(), helper); + references.put(baseClassRef.getClassName(), baseClassRef); + + List mismatches = createMatcher(references, Collections.singletonList(helper.getClassName())) + .getMismatchedReferenceSources(this.getClass().getClassLoader()); + + assertThat(getMismatchClassSet(mismatches)).containsExactly(Mismatch.MissingMethod.class); + } + + private static ReferenceMatcher createMatcher(Map references) { + return createMatcher(references, Collections.emptyList()); + } + + private static ReferenceMatcher createMatcher(Map references, List helperClasses) { + return new ReferenceMatcher(helperClasses, references, className -> className.startsWith(TEST_EXTERNAL_INSTRUMENTATION_PACKAGE)); + } + + private static Set> getMismatchClassSet(List mismatches) { + Set> mismatchClasses = new HashSet<>(mismatches.size()); + for (Mismatch mismatch : mismatches) { + mismatchClasses.add(mismatch.getClass()); + } + return mismatchClasses; + } + + private static List parseMethodFlags(String flagsStr) { + if (flagsStr.isEmpty()) { + return Collections.emptyList(); + } + List flags = new ArrayList<>(); + if (flagsStr.contains("PRIVATE_OR_HIGHER")) { + flags.add(PRIVATE_OR_HIGHER); + } + if (flagsStr.contains("PROTECTED_OR_HIGHER")) { + flags.add(PROTECTED_OR_HIGHER); + } + if (flagsStr.contains("NON_STATIC")) { + flags.add(NON_STATIC); + } + return flags; + } + + private static List parseFieldFlags(String flagsStr) { + if (flagsStr.isEmpty()) { + return Collections.emptyList(); + } + List flags = new ArrayList<>(); + if (flagsStr.contains("PRIVATE_OR_HIGHER")) { + flags.add(PRIVATE_OR_HIGHER); + } + if (flagsStr.contains("PROTECTED_OR_HIGHER")) { + flags.add(PROTECTED_OR_HIGHER); + } + if (flagsStr.contains("PACKAGE_OR_HIGHER")) { + flags.add(PACKAGE_OR_HIGHER); + } + if (flagsStr.contains("STATIC")) { + flags.add(STATIC); + } + return flags; + } + + private static Class getMismatchClass(String mismatchName) { + switch (mismatchName) { + case "MissingFlag": + return Mismatch.MissingFlag.class; + case "MissingMethod": + return Mismatch.MissingMethod.class; + case "MissingField": + return Mismatch.MissingField.class; + default: + throw new IllegalArgumentException("Unknown mismatch: " + mismatchName); + } + } +} From 439a0570ba875852f1f232383eeb38b1b5fbc9df Mon Sep 17 00:00:00 2001 From: Gregor Zeitlinger Date: Tue, 7 Oct 2025 13:15:18 +0200 Subject: [PATCH 02/10] convert test --- .../muzzle/ReferenceMatcherTest.groovy | 364 ------------------ 1 file changed, 364 deletions(-) delete mode 100644 muzzle/src/test/groovy/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcherTest.groovy diff --git a/muzzle/src/test/groovy/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcherTest.groovy b/muzzle/src/test/groovy/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcherTest.groovy deleted file mode 100644 index 4f6102397be0..000000000000 --- a/muzzle/src/test/groovy/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcherTest.groovy +++ /dev/null @@ -1,364 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.tooling.muzzle - -import static org.assertj.core.api.Assertions.assertThat - -import external.LibraryBaseClass -import io.opentelemetry.instrumentation.TestHelperClasses -import io.opentelemetry.instrumentation.test.utils.ClasspathUtils -import io.opentelemetry.javaagent.tooling.muzzle.references.ClassRef -import io.opentelemetry.javaagent.tooling.muzzle.references.Flag -import io.opentelemetry.javaagent.tooling.muzzle.references.Source -import io.opentelemetry.test.AnotherTestInterface -import io.opentelemetry.test.TestAbstractSuperClass -import io.opentelemetry.test.TestInterface -import muzzle.TestClasses -import muzzle.TestClasses.Nested -import org.junit.jupiter.api.BeforeAll -import org.junit.jupiter.api.Test -import org.junit.jupiter.params.ParameterizedTest -import org.junit.jupiter.params.provider.CsvSource -import org.junit.jupiter.params.provider.ValueSource -import org.objectweb.asm.Type - -import static io.opentelemetry.javaagent.tooling.muzzle.references.Flag.ManifestationFlag.ABSTRACT -import static io.opentelemetry.javaagent.tooling.muzzle.references.Flag.ManifestationFlag.INTERFACE -import static io.opentelemetry.javaagent.tooling.muzzle.references.Flag.ManifestationFlag.NON_INTERFACE -import static io.opentelemetry.javaagent.tooling.muzzle.references.Flag.MinimumVisibilityFlag.PACKAGE_OR_HIGHER -import static io.opentelemetry.javaagent.tooling.muzzle.references.Flag.MinimumVisibilityFlag.PRIVATE_OR_HIGHER -import static io.opentelemetry.javaagent.tooling.muzzle.references.Flag.MinimumVisibilityFlag.PROTECTED_OR_HIGHER -import static io.opentelemetry.javaagent.tooling.muzzle.references.Flag.OwnershipFlag.NON_STATIC -import static io.opentelemetry.javaagent.tooling.muzzle.references.Flag.OwnershipFlag.STATIC - -class ReferenceMatcherTest { - static final TEST_EXTERNAL_INSTRUMENTATION_PACKAGE = "com.external.otel.instrumentation" - - static ClassLoader safeClasspath - static ClassLoader unsafeClasspath - - @BeforeAll - static void setup() { - safeClasspath = new URLClassLoader([ClasspathUtils.createJarWithClasses(Nested.A, - Nested.B, - Nested.SomeInterface, - Nested.SomeImplementation)] as URL[], - (ClassLoader) null) - - unsafeClasspath = new URLClassLoader([ClasspathUtils.createJarWithClasses(Nested.A, - Nested.SomeInterface, - Nested.SomeImplementation)] as URL[], - (ClassLoader) null) - } - - @Test - void matchSafeClasspaths() { - def collector = new ReferenceCollector({ false }) - collector.collectReferencesFromAdvice(TestClasses.MethodBodyAdvice.name) - def refMatcher = createMatcher(collector.getReferences()) - - assertThat(getMismatchClassSet(refMatcher.getMismatchedReferenceSources(safeClasspath))).isEmpty() - assertThat(getMismatchClassSet(refMatcher.getMismatchedReferenceSources(unsafeClasspath))) - .containsExactly(Mismatch.MissingClass) - } - - @Test - void matchingDoesNotHoldStrongReferenceToClassloaders() { - assertThat(MuzzleWeakReferenceTestUtil.classLoaderRefIsGarbageCollected()).isTrue() - } - - private static class CountingClassLoader extends URLClassLoader { - int count = 0 - - CountingClassLoader(URL[] urls, ClassLoader parent) { - super(urls, (ClassLoader) parent) - } - - @Override - URL getResource(String name) { - count++ - return super.getResource(name) - } - } - - @Test - void muzzleTypePoolCaches() { - def cl = new CountingClassLoader( - [ClasspathUtils.createJarWithClasses(Nested.A, - Nested.B, - Nested.SomeInterface, - Nested.SomeImplementation)] as URL[], - (ClassLoader) null) - - def collector = new ReferenceCollector({ false }) - collector.collectReferencesFromAdvice(Nested.name) - - def refMatcher1 = createMatcher(collector.getReferences()) - def refMatcher2 = createMatcher(collector.getReferences()) - assertThat(getMismatchClassSet(refMatcher1.getMismatchedReferenceSources(cl))).isEmpty() - int countAfterFirstMatch = cl.count - // the second matcher should be able to used cached type descriptions from the first - assertThat(getMismatchClassSet(refMatcher2.getMismatchedReferenceSources(cl))).isEmpty() - - assertThat(cl.count).isEqualTo(countAfterFirstMatch) - } - - @ParameterizedTest - @CsvSource([ - "muzzle.TestClasses\$Nested\$B, NON_INTERFACE, ''", - "muzzle.TestClasses\$Nested\$B, INTERFACE, MissingFlag" - ]) - void matchingRef(String referenceName, String referenceFlag, String expectedMismatch) { - def flag = referenceFlag == "NON_INTERFACE" ? NON_INTERFACE : INTERFACE - def ref = ClassRef.builder(referenceName) - .addFlag(flag) - .build() - - def mismatches = createMatcher([(ref.className): ref]).getMismatchedReferenceSources(this.class.classLoader) - - if (expectedMismatch.isEmpty()) { - assertThat(getMismatchClassSet(mismatches)).isEmpty() - } else { - assertThat(getMismatchClassSet(mismatches)).containsExactly(Mismatch.MissingFlag) - } - } - - @ParameterizedTest - @CsvSource([ - "method, (Ljava/lang/String;)Ljava/lang/String;, '', muzzle.TestClasses\$Nested\$B, ''", - "hashCode, ()I, '', muzzle.TestClasses\$Nested\$B, ''", - "someMethod, ()V, '', muzzle.TestClasses\$Nested\$SomeInterface, ''", - "privateStuff, ()V, PRIVATE_OR_HIGHER, muzzle.TestClasses\$Nested\$B, ''", - "privateStuff, ()V, PROTECTED_OR_HIGHER, muzzle.TestClasses\$Nested\$B2, MissingFlag", - "staticMethod, ()V, NON_STATIC, muzzle.TestClasses\$Nested\$B, MissingFlag", - "missingMethod, ()V, '', muzzle.TestClasses\$Nested\$B, MissingMethod" - ]) - void methodMatch(String methodName, String methodDesc, String methodFlagsStr, String classToCheckName, String expectedMismatch) { - def methodType = Type.getMethodType(methodDesc) - def methodFlags = parseMethodFlags(methodFlagsStr) - def classToCheck = Class.forName(classToCheckName) - - def reference = ClassRef.builder(classToCheck.name) - .addMethod(new Source[0], methodFlags as Flag[], methodName, methodType.returnType, methodType.argumentTypes) - .build() - - def mismatches = createMatcher([(reference.className): reference]) - .getMismatchedReferenceSources(this.class.classLoader) - - if (expectedMismatch.isEmpty()) { - assertThat(getMismatchClassSet(mismatches)).isEmpty() - } else { - def expectedMismatchClass = getMismatchClass(expectedMismatch) - assertThat(getMismatchClassSet(mismatches)).containsExactly(expectedMismatchClass) - } - } - - @ParameterizedTest - @CsvSource([ - "missingField, Ljava/lang/String;, '', muzzle.TestClasses\$Nested\$A, MissingField", - "privateField, Ljava/lang/String;, '', muzzle.TestClasses\$Nested\$A, MissingField", - "privateField, Ljava/lang/Object;, PRIVATE_OR_HIGHER, muzzle.TestClasses\$Nested\$A, ''", - "privateField, Ljava/lang/Object;, PROTECTED_OR_HIGHER, muzzle.TestClasses\$Nested\$A2, MissingFlag", - "protectedField, Ljava/lang/Object;, STATIC, muzzle.TestClasses\$Nested\$A, MissingFlag", - "staticB, Lmuzzle/TestClasses\$Nested\$B;, STATIC|PROTECTED_OR_HIGHER, muzzle.TestClasses\$Nested\$A, ''", - "number, I, PACKAGE_OR_HIGHER, muzzle.TestClasses\$Nested\$Primitives, ''", - "flag, Z, PACKAGE_OR_HIGHER, muzzle.TestClasses\$Nested\$Primitives, ''" - ]) - void fieldMatch(String fieldName, String fieldType, String fieldFlagsStr, String classToCheckName, String expectedMismatch) { - def fieldFlags = parseFieldFlags(fieldFlagsStr) - def classToCheck = Class.forName(classToCheckName) - - def reference = ClassRef.builder(classToCheck.name) - .addField(new Source[0], fieldFlags as Flag[], fieldName, Type.getType(fieldType), false) - .build() - - def mismatches = createMatcher([(reference.className): reference]) - .getMismatchedReferenceSources(this.class.classLoader) - - if (expectedMismatch.isEmpty()) { - assertThat(getMismatchClassSet(mismatches)).isEmpty() - } else { - def expectedMismatchClass = getMismatchClass(expectedMismatch) - assertThat(getMismatchClassSet(mismatches)).containsExactly(expectedMismatchClass) - } - } - - @ParameterizedTest - @ValueSource(strings = ["io.opentelemetry.instrumentation.Helper", "com.external.otel.instrumentation.Helper"]) - void shouldNotCheckAbstractHelperClasses(String className) { - def reference = ClassRef.builder(className) - .setSuperClassName(TestHelperClasses.HelperSuperClass.name) - .addFlag(ABSTRACT) - .addMethod(new Source[0], [ABSTRACT] as Flag[], "unimplemented", Type.VOID_TYPE) - .build() - - def mismatches = createMatcher([(reference.className): reference], [reference.className]) - .getMismatchedReferenceSources(this.class.classLoader) - - assertThat(mismatches).isEmpty() - } - - @ParameterizedTest - @ValueSource(strings = ["io.opentelemetry.instrumentation.Helper", "com.external.otel.instrumentation.Helper"]) - void shouldNotCheckHelperClassesWithNoSupertypes(String className) { - def reference = ClassRef.builder(className) - .setSuperClassName(Object.name) - .addMethod(new Source[0], [] as Flag[], "someMethod", Type.VOID_TYPE) - .build() - - def mismatches = createMatcher([(reference.className): reference], [reference.className]) - .getMismatchedReferenceSources(this.class.classLoader) - - assertThat(mismatches).isEmpty() - } - - @ParameterizedTest - @ValueSource(strings = ["io.opentelemetry.instrumentation.Helper", "com.external.otel.instrumentation.Helper"]) - void shouldFailHelperClassesThatDoesNotImplementAllAbstractMethods(String className) { - def reference = ClassRef.builder(className) - .setSuperClassName(TestAbstractSuperClass.name) - .addMethod(new Source[0], [] as Flag[], "someMethod", Type.VOID_TYPE) - .build() - - def mismatches = createMatcher([(reference.className): reference], [reference.className]) - .getMismatchedReferenceSources(this.class.classLoader) - - assertThat(getMismatchClassSet(mismatches)).containsExactly(Mismatch.MissingMethod) - } - - @ParameterizedTest - @ValueSource(strings = ["io.opentelemetry.instrumentation.Helper", "com.external.otel.instrumentation.Helper"]) - void shouldFailHelperClassesThatDoNotImplementAllAbstractMethodsEvenIfEmptyAbstractClassReferenceExists(String className) { - def emptySuperClassRef = ClassRef.builder(TestAbstractSuperClass.name) - .build() - def reference = ClassRef.builder(className) - .setSuperClassName(TestAbstractSuperClass.name) - .addMethod(new Source[0], [] as Flag[], "someMethod", Type.VOID_TYPE) - .build() - - def mismatches = createMatcher( - [(reference.className): reference, (emptySuperClassRef.className): emptySuperClassRef], - [reference.className, emptySuperClassRef.className]) - .getMismatchedReferenceSources(this.class.classLoader) - - assertThat(getMismatchClassSet(mismatches)).containsExactly(Mismatch.MissingMethod) - } - - @ParameterizedTest - @ValueSource(strings = ["io.opentelemetry.instrumentation.Helper", "com.external.otel.instrumentation.Helper"]) - void shouldCheckHelperClassWhetherInterfaceMethodsAreImplementedInTheSuperClass(String className) { - def baseHelper = ClassRef.builder("io.opentelemetry.instrumentation.BaseHelper") - .setSuperClassName(Object.name) - .addInterfaceName(TestInterface.name) - .addMethod(new Source[0], [] as Flag[], "foo", Type.VOID_TYPE) - .build() - // abstract HelperInterface#foo() is implemented by BaseHelper - def helper = ClassRef.builder(className) - .setSuperClassName(baseHelper.className) - .addInterfaceName(AnotherTestInterface.name) - .addMethod(new Source[0], [] as Flag[], "bar", Type.VOID_TYPE) - .build() - - def mismatches = createMatcher( - [(helper.className): helper, (baseHelper.className): baseHelper], - [helper.className, baseHelper.className]) - .getMismatchedReferenceSources(this.class.classLoader) - - assertThat(mismatches).isEmpty() - } - - @ParameterizedTest - @ValueSource(strings = ["io.opentelemetry.instrumentation.Helper", "com.external.otel.instrumentation.Helper"]) - void shouldCheckHelperClassWhetherUsedFieldsAreDeclaredInTheSuperClass(String className) { - def helper = ClassRef.builder(className) - .setSuperClassName(LibraryBaseClass.name) - .addField(new Source[0], new Flag[0], "field", Type.getType("Ljava/lang/Integer;"), false) - .build() - - def mismatches = createMatcher([(helper.className): helper], [helper.className]) - .getMismatchedReferenceSources(this.class.classLoader) - - assertThat(mismatches).isEmpty() - } - - @ParameterizedTest - @CsvSource([ - "io.opentelemetry.instrumentation.Helper, differentField, Ljava/lang/Integer;", - "io.opentelemetry.instrumentation.Helper, field, Lcom/external/DifferentType;", - "com.external.otel.instrumentation.Helper, differentField, Ljava/lang/Integer;", - "com.external.otel.instrumentation.Helper, field, Lcom/external/DifferentType;" - ]) - void shouldFailHelperClassWhenItUsesFieldsUndeclaredInTheSuperClass(String className, String fieldName, String fieldType) { - def helper = ClassRef.builder(className) - .setSuperClassName(DeclaredFieldTestClass.LibraryBaseClass.name) - .addField(new Source[0], new Flag[0], fieldName, Type.getType(fieldType), false) - .build() - - def mismatches = createMatcher([(helper.className): helper], [helper.className]) - .getMismatchedReferenceSources(this.class.classLoader) - - assertThat(getMismatchClassSet(mismatches)).containsExactly(Mismatch.MissingField) - } - - @ParameterizedTest - @ValueSource(strings = ["io.opentelemetry.instrumentation.Helper", "com.external.otel.instrumentation.Helper"]) - void shouldFailHelperClassWhenTheLibraryParentClassHasDifferentConstructor(String className) { - def helper = ClassRef.builder(className) - .setSuperClassName(TestClasses.BaseClassWithConstructor.name) - .build() - // muzzle codegen plugin has captured a no-arg constructor reference; - // the actual constructor of the base class on the classpath requires a long - def baseClassRef = ClassRef.builder(TestClasses.BaseClassWithConstructor.name) - .addMethod(new Source[0], new Flag[0], "", Type.VOID_TYPE) - .build() - - def mismatches = createMatcher([(helper.className): helper, (baseClassRef.className): baseClassRef], [helper.className]) - .getMismatchedReferenceSources(this.class.classLoader) - - assertThat(getMismatchClassSet(mismatches)).containsExactly(Mismatch.MissingMethod) - } - - private static ReferenceMatcher createMatcher(Map references = [:], - List helperClasses = []) { - new ReferenceMatcher(helperClasses, references, { it.startsWith(TEST_EXTERNAL_INSTRUMENTATION_PACKAGE) }) - } - - private static Set getMismatchClassSet(List mismatches) { - Set mismatchClasses = new HashSet<>(mismatches.size()) - for (Mismatch mismatch : mismatches) { - mismatchClasses.add(mismatch.class) - } - return mismatchClasses - } - - private static List parseMethodFlags(String flagsStr) { - if (flagsStr.isEmpty()) return [] - def flags = [] - if (flagsStr.contains("PRIVATE_OR_HIGHER")) flags.add(PRIVATE_OR_HIGHER) - if (flagsStr.contains("PROTECTED_OR_HIGHER")) flags.add(PROTECTED_OR_HIGHER) - if (flagsStr.contains("NON_STATIC")) flags.add(NON_STATIC) - return flags - } - - private static List parseFieldFlags(String flagsStr) { - if (flagsStr.isEmpty()) return [] - def flags = [] - if (flagsStr.contains("PRIVATE_OR_HIGHER")) flags.add(PRIVATE_OR_HIGHER) - if (flagsStr.contains("PROTECTED_OR_HIGHER")) flags.add(PROTECTED_OR_HIGHER) - if (flagsStr.contains("PACKAGE_OR_HIGHER")) flags.add(PACKAGE_OR_HIGHER) - if (flagsStr.contains("STATIC")) flags.add(STATIC) - return flags - } - - private static Class getMismatchClass(String mismatchName) { - switch (mismatchName) { - case "MissingFlag": return Mismatch.MissingFlag - case "MissingMethod": return Mismatch.MissingMethod - case "MissingField": return Mismatch.MissingField - default: throw new IllegalArgumentException("Unknown mismatch: " + mismatchName) - } - } -} From 7388b7d4fb96cf3b3495da2e763edae15991d248 Mon Sep 17 00:00:00 2001 From: Gregor Zeitlinger Date: Tue, 7 Oct 2025 13:20:31 +0200 Subject: [PATCH 03/10] convert test --- .../tooling/muzzle/ReferenceMatcherTest.java | 350 +++++++++++------- 1 file changed, 217 insertions(+), 133 deletions(-) diff --git a/muzzle/src/test/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcherTest.java b/muzzle/src/test/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcherTest.java index 8182441bd954..e6d92643d051 100644 --- a/muzzle/src/test/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcherTest.java +++ b/muzzle/src/test/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcherTest.java @@ -45,31 +45,32 @@ import org.objectweb.asm.Type; class ReferenceMatcherTest { - private static final String TEST_EXTERNAL_INSTRUMENTATION_PACKAGE = "com.external.otel.instrumentation"; + private static final String TEST_EXTERNAL_INSTRUMENTATION_PACKAGE = + "com.external.otel.instrumentation"; private static ClassLoader safeClasspath; private static ClassLoader unsafeClasspath; @BeforeAll static void setup() throws Exception { - safeClasspath = new URLClassLoader( - new URL[] { - ClasspathUtils.createJarWithClasses( - Nested.A.class, - Nested.B.class, - Nested.SomeInterface.class, - Nested.SomeImplementation.class) - }, - null); - - unsafeClasspath = new URLClassLoader( - new URL[] { - ClasspathUtils.createJarWithClasses( - Nested.A.class, - Nested.SomeInterface.class, - Nested.SomeImplementation.class) - }, - null); + safeClasspath = + new URLClassLoader( + new URL[] { + ClasspathUtils.createJarWithClasses( + Nested.A.class, + Nested.B.class, + Nested.SomeInterface.class, + Nested.SomeImplementation.class) + }, + null); + + unsafeClasspath = + new URLClassLoader( + new URL[] { + ClasspathUtils.createJarWithClasses( + Nested.A.class, Nested.SomeInterface.class, Nested.SomeImplementation.class) + }, + null); } @Test @@ -78,7 +79,8 @@ void matchSafeClasspaths() { collector.collectReferencesFromAdvice(TestClasses.MethodBodyAdvice.class.getName()); ReferenceMatcher refMatcher = createMatcher(collector.getReferences()); - assertThat(getMismatchClassSet(refMatcher.getMismatchedReferenceSources(safeClasspath))).isEmpty(); + assertThat(getMismatchClassSet(refMatcher.getMismatchedReferenceSources(safeClasspath))) + .isEmpty(); assertThat(getMismatchClassSet(refMatcher.getMismatchedReferenceSources(unsafeClasspath))) .containsExactly(Mismatch.MissingClass.class); } @@ -108,15 +110,16 @@ public URL getResource(String name) { @Test void muzzleTypePoolCaches() throws Exception { - CountingClassLoader cl = new CountingClassLoader( - new URL[] { - ClasspathUtils.createJarWithClasses( - Nested.A.class, - Nested.B.class, - Nested.SomeInterface.class, - Nested.SomeImplementation.class) - }, - null); + CountingClassLoader cl = + new CountingClassLoader( + new URL[] { + ClasspathUtils.createJarWithClasses( + Nested.A.class, + Nested.B.class, + Nested.SomeInterface.class, + Nested.SomeImplementation.class) + }, + null); ReferenceCollector collector = new ReferenceCollector(className -> false); collector.collectReferencesFromAdvice(Nested.class.getName()); @@ -138,12 +141,11 @@ void muzzleTypePoolCaches() throws Exception { }) void matchingRef(String referenceName, String referenceFlag, String expectedMismatch) { Flag flag = "NON_INTERFACE".equals(referenceFlag) ? NON_INTERFACE : INTERFACE; - ClassRef ref = ClassRef.builder(referenceName) - .addFlag(flag) - .build(); + ClassRef ref = ClassRef.builder(referenceName).addFlag(flag).build(); - List mismatches = createMatcher(Collections.singletonMap(ref.getClassName(), ref)) - .getMismatchedReferenceSources(this.getClass().getClassLoader()); + List mismatches = + createMatcher(Collections.singletonMap(ref.getClassName(), ref)) + .getMismatchedReferenceSources(this.getClass().getClassLoader()); if (expectedMismatch.isEmpty()) { assertThat(getMismatchClassSet(mismatches)).isEmpty(); @@ -162,18 +164,30 @@ void matchingRef(String referenceName, String referenceFlag, String expectedMism "staticMethod, ()V, NON_STATIC, muzzle.TestClasses$Nested$B, MissingFlag", "missingMethod, ()V, '', muzzle.TestClasses$Nested$B, MissingMethod" }) - void methodMatch(String methodName, String methodDesc, String methodFlagsStr, String classToCheckName, String expectedMismatch) + void methodMatch( + String methodName, + String methodDesc, + String methodFlagsStr, + String classToCheckName, + String expectedMismatch) throws ClassNotFoundException { Type methodType = Type.getMethodType(methodDesc); List methodFlags = parseMethodFlags(methodFlagsStr); Class classToCheck = Class.forName(classToCheckName); - ClassRef reference = ClassRef.builder(classToCheck.getName()) - .addMethod(new Source[0], methodFlags.toArray(new Flag[0]), methodName, methodType.getReturnType(), methodType.getArgumentTypes()) - .build(); + ClassRef reference = + ClassRef.builder(classToCheck.getName()) + .addMethod( + new Source[0], + methodFlags.toArray(new Flag[0]), + methodName, + methodType.getReturnType(), + methodType.getArgumentTypes()) + .build(); - List mismatches = createMatcher(Collections.singletonMap(reference.getClassName(), reference)) - .getMismatchedReferenceSources(this.getClass().getClassLoader()); + List mismatches = + createMatcher(Collections.singletonMap(reference.getClassName(), reference)) + .getMismatchedReferenceSources(this.getClass().getClassLoader()); if (expectedMismatch.isEmpty()) { assertThat(getMismatchClassSet(mismatches)).isEmpty(); @@ -194,17 +208,29 @@ void methodMatch(String methodName, String methodDesc, String methodFlagsStr, St "number, I, PACKAGE_OR_HIGHER, muzzle.TestClasses$Nested$Primitives, ''", "flag, Z, PACKAGE_OR_HIGHER, muzzle.TestClasses$Nested$Primitives, ''" }) - void fieldMatch(String fieldName, String fieldType, String fieldFlagsStr, String classToCheckName, String expectedMismatch) + void fieldMatch( + String fieldName, + String fieldType, + String fieldFlagsStr, + String classToCheckName, + String expectedMismatch) throws ClassNotFoundException { List fieldFlags = parseFieldFlags(fieldFlagsStr); Class classToCheck = Class.forName(classToCheckName); - ClassRef reference = ClassRef.builder(classToCheck.getName()) - .addField(new Source[0], fieldFlags.toArray(new Flag[0]), fieldName, Type.getType(fieldType), false) - .build(); + ClassRef reference = + ClassRef.builder(classToCheck.getName()) + .addField( + new Source[0], + fieldFlags.toArray(new Flag[0]), + fieldName, + Type.getType(fieldType), + false) + .build(); - List mismatches = createMatcher(Collections.singletonMap(reference.getClassName(), reference)) - .getMismatchedReferenceSources(this.getClass().getClassLoader()); + List mismatches = + createMatcher(Collections.singletonMap(reference.getClassName(), reference)) + .getMismatchedReferenceSources(this.getClass().getClassLoader()); if (expectedMismatch.isEmpty()) { assertThat(getMismatchClassSet(mismatches)).isEmpty(); @@ -215,90 +241,123 @@ void fieldMatch(String fieldName, String fieldType, String fieldFlagsStr, String } @ParameterizedTest - @ValueSource(strings = {"io.opentelemetry.instrumentation.Helper", "com.external.otel.instrumentation.Helper"}) + @ValueSource( + strings = { + "io.opentelemetry.instrumentation.Helper", + "com.external.otel.instrumentation.Helper" + }) void shouldNotCheckAbstractHelperClasses(String className) { - ClassRef reference = ClassRef.builder(className) - .setSuperClassName(TestHelperClasses.HelperSuperClass.class.getName()) - .addFlag(ABSTRACT) - .addMethod(new Source[0], new Flag[] {ABSTRACT}, "unimplemented", Type.VOID_TYPE) - .build(); - - List mismatches = createMatcher( - Collections.singletonMap(reference.getClassName(), reference), - Collections.singletonList(reference.getClassName())) - .getMismatchedReferenceSources(this.getClass().getClassLoader()); + ClassRef reference = + ClassRef.builder(className) + .setSuperClassName(TestHelperClasses.HelperSuperClass.class.getName()) + .addFlag(ABSTRACT) + .addMethod(new Source[0], new Flag[] {ABSTRACT}, "unimplemented", Type.VOID_TYPE) + .build(); + + List mismatches = + createMatcher( + Collections.singletonMap(reference.getClassName(), reference), + Collections.singletonList(reference.getClassName())) + .getMismatchedReferenceSources(this.getClass().getClassLoader()); assertThat(mismatches).isEmpty(); } @ParameterizedTest - @ValueSource(strings = {"io.opentelemetry.instrumentation.Helper", "com.external.otel.instrumentation.Helper"}) + @ValueSource( + strings = { + "io.opentelemetry.instrumentation.Helper", + "com.external.otel.instrumentation.Helper" + }) void shouldNotCheckHelperClassesWithNoSupertypes(String className) { - ClassRef reference = ClassRef.builder(className) - .setSuperClassName(Object.class.getName()) - .addMethod(new Source[0], new Flag[0], "someMethod", Type.VOID_TYPE) - .build(); - - List mismatches = createMatcher( - Collections.singletonMap(reference.getClassName(), reference), - Collections.singletonList(reference.getClassName())) - .getMismatchedReferenceSources(this.getClass().getClassLoader()); + ClassRef reference = + ClassRef.builder(className) + .setSuperClassName(Object.class.getName()) + .addMethod(new Source[0], new Flag[0], "someMethod", Type.VOID_TYPE) + .build(); + + List mismatches = + createMatcher( + Collections.singletonMap(reference.getClassName(), reference), + Collections.singletonList(reference.getClassName())) + .getMismatchedReferenceSources(this.getClass().getClassLoader()); assertThat(mismatches).isEmpty(); } @ParameterizedTest - @ValueSource(strings = {"io.opentelemetry.instrumentation.Helper", "com.external.otel.instrumentation.Helper"}) + @ValueSource( + strings = { + "io.opentelemetry.instrumentation.Helper", + "com.external.otel.instrumentation.Helper" + }) void shouldFailHelperClassesThatDoesNotImplementAllAbstractMethods(String className) { - ClassRef reference = ClassRef.builder(className) - .setSuperClassName(TestAbstractSuperClass.class.getName()) - .addMethod(new Source[0], new Flag[0], "someMethod", Type.VOID_TYPE) - .build(); - - List mismatches = createMatcher( - Collections.singletonMap(reference.getClassName(), reference), - Collections.singletonList(reference.getClassName())) - .getMismatchedReferenceSources(this.getClass().getClassLoader()); + ClassRef reference = + ClassRef.builder(className) + .setSuperClassName(TestAbstractSuperClass.class.getName()) + .addMethod(new Source[0], new Flag[0], "someMethod", Type.VOID_TYPE) + .build(); + + List mismatches = + createMatcher( + Collections.singletonMap(reference.getClassName(), reference), + Collections.singletonList(reference.getClassName())) + .getMismatchedReferenceSources(this.getClass().getClassLoader()); assertThat(getMismatchClassSet(mismatches)).containsExactly(Mismatch.MissingMethod.class); } @ParameterizedTest - @ValueSource(strings = {"io.opentelemetry.instrumentation.Helper", "com.external.otel.instrumentation.Helper"}) - void shouldFailHelperClassesThatDoNotImplementAllAbstractMethodsEvenIfEmptyAbstractClassReferenceExists(String className) { - ClassRef emptySuperClassRef = ClassRef.builder(TestAbstractSuperClass.class.getName()) - .build(); - ClassRef reference = ClassRef.builder(className) - .setSuperClassName(TestAbstractSuperClass.class.getName()) - .addMethod(new Source[0], new Flag[0], "someMethod", Type.VOID_TYPE) - .build(); + @ValueSource( + strings = { + "io.opentelemetry.instrumentation.Helper", + "com.external.otel.instrumentation.Helper" + }) + void + shouldFailHelperClassesThatDoNotImplementAllAbstractMethodsEvenIfEmptyAbstractClassReferenceExists( + String className) { + ClassRef emptySuperClassRef = ClassRef.builder(TestAbstractSuperClass.class.getName()).build(); + ClassRef reference = + ClassRef.builder(className) + .setSuperClassName(TestAbstractSuperClass.class.getName()) + .addMethod(new Source[0], new Flag[0], "someMethod", Type.VOID_TYPE) + .build(); Map references = new HashMap<>(); references.put(reference.getClassName(), reference); references.put(emptySuperClassRef.getClassName(), emptySuperClassRef); - List helperClasses = Arrays.asList(reference.getClassName(), emptySuperClassRef.getClassName()); + List helperClasses = + Arrays.asList(reference.getClassName(), emptySuperClassRef.getClassName()); - List mismatches = createMatcher(references, helperClasses) - .getMismatchedReferenceSources(this.getClass().getClassLoader()); + List mismatches = + createMatcher(references, helperClasses) + .getMismatchedReferenceSources(this.getClass().getClassLoader()); assertThat(getMismatchClassSet(mismatches)).containsExactly(Mismatch.MissingMethod.class); } @ParameterizedTest - @ValueSource(strings = {"io.opentelemetry.instrumentation.Helper", "com.external.otel.instrumentation.Helper"}) - void shouldCheckHelperClassWhetherInterfaceMethodsAreImplementedInTheSuperClass(String className) { - ClassRef baseHelper = ClassRef.builder("io.opentelemetry.instrumentation.BaseHelper") - .setSuperClassName(Object.class.getName()) - .addInterfaceName(TestInterface.class.getName()) - .addMethod(new Source[0], new Flag[0], "foo", Type.VOID_TYPE) - .build(); + @ValueSource( + strings = { + "io.opentelemetry.instrumentation.Helper", + "com.external.otel.instrumentation.Helper" + }) + void shouldCheckHelperClassWhetherInterfaceMethodsAreImplementedInTheSuperClass( + String className) { + ClassRef baseHelper = + ClassRef.builder("io.opentelemetry.instrumentation.BaseHelper") + .setSuperClassName(Object.class.getName()) + .addInterfaceName(TestInterface.class.getName()) + .addMethod(new Source[0], new Flag[0], "foo", Type.VOID_TYPE) + .build(); // abstract HelperInterface#foo() is implemented by BaseHelper - ClassRef helper = ClassRef.builder(className) - .setSuperClassName(baseHelper.getClassName()) - .addInterfaceName(AnotherTestInterface.class.getName()) - .addMethod(new Source[0], new Flag[0], "bar", Type.VOID_TYPE) - .build(); + ClassRef helper = + ClassRef.builder(className) + .setSuperClassName(baseHelper.getClassName()) + .addInterfaceName(AnotherTestInterface.class.getName()) + .addMethod(new Source[0], new Flag[0], "bar", Type.VOID_TYPE) + .build(); Map references = new HashMap<>(); references.put(helper.getClassName(), helper); @@ -306,24 +365,32 @@ void shouldCheckHelperClassWhetherInterfaceMethodsAreImplementedInTheSuperClass( List helperClasses = Arrays.asList(helper.getClassName(), baseHelper.getClassName()); - List mismatches = createMatcher(references, helperClasses) - .getMismatchedReferenceSources(this.getClass().getClassLoader()); + List mismatches = + createMatcher(references, helperClasses) + .getMismatchedReferenceSources(this.getClass().getClassLoader()); assertThat(mismatches).isEmpty(); } @ParameterizedTest - @ValueSource(strings = {"io.opentelemetry.instrumentation.Helper", "com.external.otel.instrumentation.Helper"}) + @ValueSource( + strings = { + "io.opentelemetry.instrumentation.Helper", + "com.external.otel.instrumentation.Helper" + }) void shouldCheckHelperClassWhetherUsedFieldsAreDeclaredInTheSuperClass(String className) { - ClassRef helper = ClassRef.builder(className) - .setSuperClassName(LibraryBaseClass.class.getName()) - .addField(new Source[0], new Flag[0], "field", Type.getType("Ljava/lang/Integer;"), false) - .build(); - - List mismatches = createMatcher( - Collections.singletonMap(helper.getClassName(), helper), - Collections.singletonList(helper.getClassName())) - .getMismatchedReferenceSources(this.getClass().getClassLoader()); + ClassRef helper = + ClassRef.builder(className) + .setSuperClassName(LibraryBaseClass.class.getName()) + .addField( + new Source[0], new Flag[0], "field", Type.getType("Ljava/lang/Integer;"), false) + .build(); + + List mismatches = + createMatcher( + Collections.singletonMap(helper.getClassName(), helper), + Collections.singletonList(helper.getClassName())) + .getMismatchedReferenceSources(this.getClass().getClassLoader()); assertThat(mismatches).isEmpty(); } @@ -335,38 +402,51 @@ void shouldCheckHelperClassWhetherUsedFieldsAreDeclaredInTheSuperClass(String cl "com.external.otel.instrumentation.Helper, differentField, Ljava/lang/Integer;", "com.external.otel.instrumentation.Helper, field, Lcom/external/DifferentType;" }) - void shouldFailHelperClassWhenItUsesFieldsUndeclaredInTheSuperClass(String className, String fieldName, String fieldType) { - ClassRef helper = ClassRef.builder(className) - .setSuperClassName(io.opentelemetry.javaagent.tooling.muzzle.DeclaredFieldTestClass.LibraryBaseClass.class.getName()) - .addField(new Source[0], new Flag[0], fieldName, Type.getType(fieldType), false) - .build(); - - List mismatches = createMatcher( - Collections.singletonMap(helper.getClassName(), helper), - Collections.singletonList(helper.getClassName())) - .getMismatchedReferenceSources(this.getClass().getClassLoader()); + void shouldFailHelperClassWhenItUsesFieldsUndeclaredInTheSuperClass( + String className, String fieldName, String fieldType) { + ClassRef helper = + ClassRef.builder(className) + .setSuperClassName( + io.opentelemetry.javaagent.tooling.muzzle.DeclaredFieldTestClass.LibraryBaseClass + .class + .getName()) + .addField(new Source[0], new Flag[0], fieldName, Type.getType(fieldType), false) + .build(); + + List mismatches = + createMatcher( + Collections.singletonMap(helper.getClassName(), helper), + Collections.singletonList(helper.getClassName())) + .getMismatchedReferenceSources(this.getClass().getClassLoader()); assertThat(getMismatchClassSet(mismatches)).containsExactly(Mismatch.MissingField.class); } @ParameterizedTest - @ValueSource(strings = {"io.opentelemetry.instrumentation.Helper", "com.external.otel.instrumentation.Helper"}) + @ValueSource( + strings = { + "io.opentelemetry.instrumentation.Helper", + "com.external.otel.instrumentation.Helper" + }) void shouldFailHelperClassWhenTheLibraryParentClassHasDifferentConstructor(String className) { - ClassRef helper = ClassRef.builder(className) - .setSuperClassName(TestClasses.BaseClassWithConstructor.class.getName()) - .build(); + ClassRef helper = + ClassRef.builder(className) + .setSuperClassName(TestClasses.BaseClassWithConstructor.class.getName()) + .build(); // muzzle codegen plugin has captured a no-arg constructor reference; // the actual constructor of the base class on the classpath requires a long - ClassRef baseClassRef = ClassRef.builder(TestClasses.BaseClassWithConstructor.class.getName()) - .addMethod(new Source[0], new Flag[0], "", Type.VOID_TYPE) - .build(); + ClassRef baseClassRef = + ClassRef.builder(TestClasses.BaseClassWithConstructor.class.getName()) + .addMethod(new Source[0], new Flag[0], "", Type.VOID_TYPE) + .build(); Map references = new HashMap<>(); references.put(helper.getClassName(), helper); references.put(baseClassRef.getClassName(), baseClassRef); - List mismatches = createMatcher(references, Collections.singletonList(helper.getClassName())) - .getMismatchedReferenceSources(this.getClass().getClassLoader()); + List mismatches = + createMatcher(references, Collections.singletonList(helper.getClassName())) + .getMismatchedReferenceSources(this.getClass().getClassLoader()); assertThat(getMismatchClassSet(mismatches)).containsExactly(Mismatch.MissingMethod.class); } @@ -375,8 +455,12 @@ private static ReferenceMatcher createMatcher(Map references) return createMatcher(references, Collections.emptyList()); } - private static ReferenceMatcher createMatcher(Map references, List helperClasses) { - return new ReferenceMatcher(helperClasses, references, className -> className.startsWith(TEST_EXTERNAL_INSTRUMENTATION_PACKAGE)); + private static ReferenceMatcher createMatcher( + Map references, List helperClasses) { + return new ReferenceMatcher( + helperClasses, + references, + className -> className.startsWith(TEST_EXTERNAL_INSTRUMENTATION_PACKAGE)); } private static Set> getMismatchClassSet(List mismatches) { From df62180fbf9727b95869e8db3277d4d61a2f937f Mon Sep 17 00:00:00 2001 From: Gregor Zeitlinger Date: Fri, 10 Oct 2025 09:06:34 +0200 Subject: [PATCH 04/10] Update muzzle/src/test/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcherTest.java Co-authored-by: Lauri Tulmin --- .../javaagent/tooling/muzzle/ReferenceMatcherTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/muzzle/src/test/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcherTest.java b/muzzle/src/test/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcherTest.java index e6d92643d051..4bfa0fc85f71 100644 --- a/muzzle/src/test/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcherTest.java +++ b/muzzle/src/test/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcherTest.java @@ -291,7 +291,7 @@ void shouldNotCheckHelperClassesWithNoSupertypes(String className) { "io.opentelemetry.instrumentation.Helper", "com.external.otel.instrumentation.Helper" }) - void shouldFailHelperClassesThatDoesNotImplementAllAbstractMethods(String className) { + void shouldFailHelperClassesThatDoNotImplementAllAbstractMethods(String className) { ClassRef reference = ClassRef.builder(className) .setSuperClassName(TestAbstractSuperClass.class.getName()) From afefdf739a82963fe6f40d3de9f1ab930ee1a284 Mon Sep 17 00:00:00 2001 From: Gregor Zeitlinger Date: Fri, 10 Oct 2025 09:06:01 +0200 Subject: [PATCH 05/10] pr review --- .../javaagent/tooling/muzzle/ReferenceMatcherTest.java | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/muzzle/src/test/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcherTest.java b/muzzle/src/test/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcherTest.java index 4bfa0fc85f71..767818c35d7e 100644 --- a/muzzle/src/test/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcherTest.java +++ b/muzzle/src/test/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcherTest.java @@ -86,12 +86,9 @@ void matchSafeClasspaths() { } @Test - void matchingDoesNotHoldStrongReferenceToClassloaders() { - try { - assertThat(MuzzleWeakReferenceTestUtil.classLoaderRefIsGarbageCollected()).isTrue(); - } catch (InterruptedException | TimeoutException e) { - throw new RuntimeException(e); - } + void matchingDoesNotHoldStrongReferenceToClassloaders() + throws InterruptedException, TimeoutException { + assertThat(MuzzleWeakReferenceTestUtil.classLoaderRefIsGarbageCollected()).isTrue(); } private static class CountingClassLoader extends URLClassLoader { From 48268d7e165dd7dfdbd250226a418f5a8d8498f3 Mon Sep 17 00:00:00 2001 From: Gregor Zeitlinger Date: Fri, 10 Oct 2025 09:19:50 +0200 Subject: [PATCH 06/10] pr review --- .../tooling/muzzle/ReferenceMatcherTest.java | 118 +++++++++--------- 1 file changed, 56 insertions(+), 62 deletions(-) diff --git a/muzzle/src/test/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcherTest.java b/muzzle/src/test/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcherTest.java index 767818c35d7e..fe31d41f9b53 100644 --- a/muzzle/src/test/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcherTest.java +++ b/muzzle/src/test/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcherTest.java @@ -35,12 +35,14 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.TimeoutException; +import java.util.stream.Stream; import muzzle.TestClasses; import muzzle.TestClasses.Nested; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.CsvSource; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import org.junit.jupiter.params.provider.ValueSource; import org.objectweb.asm.Type; @@ -131,46 +133,49 @@ void muzzleTypePoolCaches() throws Exception { assertThat(cl.count).isEqualTo(countAfterFirstMatch); } + private static Stream matchingRefProvider() { + return Stream.of( + Arguments.of(Nested.B.class, NON_INTERFACE, null), + Arguments.of(Nested.B.class, INTERFACE, Mismatch.MissingFlag.class)); + } + @ParameterizedTest - @CsvSource({ - "muzzle.TestClasses$Nested$B, NON_INTERFACE, ''", - "muzzle.TestClasses$Nested$B, INTERFACE, MissingFlag" - }) - void matchingRef(String referenceName, String referenceFlag, String expectedMismatch) { - Flag flag = "NON_INTERFACE".equals(referenceFlag) ? NON_INTERFACE : INTERFACE; - ClassRef ref = ClassRef.builder(referenceName).addFlag(flag).build(); + @MethodSource("matchingRefProvider") + void matchingRef(Class referenceClass, Flag referenceFlag, Class expectedMismatch) { + ClassRef ref = ClassRef.builder(referenceClass.getName()).addFlag(referenceFlag).build(); List mismatches = createMatcher(Collections.singletonMap(ref.getClassName(), ref)) .getMismatchedReferenceSources(this.getClass().getClassLoader()); - if (expectedMismatch.isEmpty()) { + if (expectedMismatch == null) { assertThat(getMismatchClassSet(mismatches)).isEmpty(); } else { - assertThat(getMismatchClassSet(mismatches)).containsExactly(Mismatch.MissingFlag.class); + assertThat(getMismatchClassSet(mismatches)).containsExactly(expectedMismatch); } } + private static Stream methodMatchProvider() { + return Stream.of( + Arguments.of("method", "(Ljava/lang/String;)Ljava/lang/String;", "", Nested.B.class, null), + Arguments.of("hashCode", "()I", "", Nested.B.class, null), + Arguments.of("someMethod", "()V", "", Nested.SomeInterface.class, null), + Arguments.of("privateStuff", "()V", "PRIVATE_OR_HIGHER", Nested.B.class, null), + Arguments.of("privateStuff", "()V", "PROTECTED_OR_HIGHER", Nested.B2.class, Mismatch.MissingFlag.class), + Arguments.of("staticMethod", "()V", "NON_STATIC", Nested.B.class, Mismatch.MissingFlag.class), + Arguments.of("missingMethod", "()V", "", Nested.B.class, Mismatch.MissingMethod.class)); + } + @ParameterizedTest - @CsvSource({ - "method, (Ljava/lang/String;)Ljava/lang/String;, '', muzzle.TestClasses$Nested$B, ''", - "hashCode, ()I, '', muzzle.TestClasses$Nested$B, ''", - "someMethod, ()V, '', muzzle.TestClasses$Nested$SomeInterface, ''", - "privateStuff, ()V, PRIVATE_OR_HIGHER, muzzle.TestClasses$Nested$B, ''", - "privateStuff, ()V, PROTECTED_OR_HIGHER, muzzle.TestClasses$Nested$B2, MissingFlag", - "staticMethod, ()V, NON_STATIC, muzzle.TestClasses$Nested$B, MissingFlag", - "missingMethod, ()V, '', muzzle.TestClasses$Nested$B, MissingMethod" - }) + @MethodSource("methodMatchProvider") void methodMatch( String methodName, String methodDesc, String methodFlagsStr, - String classToCheckName, - String expectedMismatch) - throws ClassNotFoundException { + Class classToCheck, + Class expectedMismatch) { Type methodType = Type.getMethodType(methodDesc); List methodFlags = parseMethodFlags(methodFlagsStr); - Class classToCheck = Class.forName(classToCheckName); ClassRef reference = ClassRef.builder(classToCheck.getName()) @@ -186,34 +191,34 @@ void methodMatch( createMatcher(Collections.singletonMap(reference.getClassName(), reference)) .getMismatchedReferenceSources(this.getClass().getClassLoader()); - if (expectedMismatch.isEmpty()) { + if (expectedMismatch == null) { assertThat(getMismatchClassSet(mismatches)).isEmpty(); } else { - Class expectedMismatchClass = getMismatchClass(expectedMismatch); - assertThat(getMismatchClassSet(mismatches)).containsExactly(expectedMismatchClass); + assertThat(getMismatchClassSet(mismatches)).containsExactly(expectedMismatch); } } + private static Stream fieldMatchProvider() { + return Stream.of( + Arguments.of("missingField", "Ljava/lang/String;", "", Nested.A.class, Mismatch.MissingField.class), + Arguments.of("privateField", "Ljava/lang/String;", "", Nested.A.class, Mismatch.MissingField.class), + Arguments.of("privateField", "Ljava/lang/Object;", "PRIVATE_OR_HIGHER", Nested.A.class, null), + Arguments.of("privateField", "Ljava/lang/Object;", "PROTECTED_OR_HIGHER", Nested.A2.class, Mismatch.MissingFlag.class), + Arguments.of("protectedField", "Ljava/lang/Object;", "STATIC", Nested.A.class, Mismatch.MissingFlag.class), + Arguments.of("staticB", "Lmuzzle/TestClasses$Nested$B;", "STATIC|PROTECTED_OR_HIGHER", Nested.A.class, null), + Arguments.of("number", "I", "PACKAGE_OR_HIGHER", Nested.Primitives.class, null), + Arguments.of("flag", "Z", "PACKAGE_OR_HIGHER", Nested.Primitives.class, null)); + } + @ParameterizedTest - @CsvSource({ - "missingField, Ljava/lang/String;, '', muzzle.TestClasses$Nested$A, MissingField", - "privateField, Ljava/lang/String;, '', muzzle.TestClasses$Nested$A, MissingField", - "privateField, Ljava/lang/Object;, PRIVATE_OR_HIGHER, muzzle.TestClasses$Nested$A, ''", - "privateField, Ljava/lang/Object;, PROTECTED_OR_HIGHER, muzzle.TestClasses$Nested$A2, MissingFlag", - "protectedField, Ljava/lang/Object;, STATIC, muzzle.TestClasses$Nested$A, MissingFlag", - "staticB, Lmuzzle/TestClasses$Nested$B;, STATIC|PROTECTED_OR_HIGHER, muzzle.TestClasses$Nested$A, ''", - "number, I, PACKAGE_OR_HIGHER, muzzle.TestClasses$Nested$Primitives, ''", - "flag, Z, PACKAGE_OR_HIGHER, muzzle.TestClasses$Nested$Primitives, ''" - }) + @MethodSource("fieldMatchProvider") void fieldMatch( String fieldName, String fieldType, String fieldFlagsStr, - String classToCheckName, - String expectedMismatch) - throws ClassNotFoundException { + Class classToCheck, + Class expectedMismatch) { List fieldFlags = parseFieldFlags(fieldFlagsStr); - Class classToCheck = Class.forName(classToCheckName); ClassRef reference = ClassRef.builder(classToCheck.getName()) @@ -229,11 +234,10 @@ void fieldMatch( createMatcher(Collections.singletonMap(reference.getClassName(), reference)) .getMismatchedReferenceSources(this.getClass().getClassLoader()); - if (expectedMismatch.isEmpty()) { + if (expectedMismatch == null) { assertThat(getMismatchClassSet(mismatches)).isEmpty(); } else { - Class expectedMismatchClass = getMismatchClass(expectedMismatch); - assertThat(getMismatchClassSet(mismatches)).containsExactly(expectedMismatchClass); + assertThat(getMismatchClassSet(mismatches)).containsExactly(expectedMismatch); } } @@ -392,13 +396,16 @@ void shouldCheckHelperClassWhetherUsedFieldsAreDeclaredInTheSuperClass(String cl assertThat(mismatches).isEmpty(); } + private static Stream shouldFailHelperClassWhenItUsesFieldsUndeclaredInTheSuperClassProvider() { + return Stream.of( + Arguments.of("io.opentelemetry.instrumentation.Helper", "differentField", "Ljava/lang/Integer;"), + Arguments.of("io.opentelemetry.instrumentation.Helper", "field", "Lcom/external/DifferentType;"), + Arguments.of("com.external.otel.instrumentation.Helper", "differentField", "Ljava/lang/Integer;"), + Arguments.of("com.external.otel.instrumentation.Helper", "field", "Lcom/external/DifferentType;")); + } + @ParameterizedTest - @CsvSource({ - "io.opentelemetry.instrumentation.Helper, differentField, Ljava/lang/Integer;", - "io.opentelemetry.instrumentation.Helper, field, Lcom/external/DifferentType;", - "com.external.otel.instrumentation.Helper, differentField, Ljava/lang/Integer;", - "com.external.otel.instrumentation.Helper, field, Lcom/external/DifferentType;" - }) + @MethodSource("shouldFailHelperClassWhenItUsesFieldsUndeclaredInTheSuperClassProvider") void shouldFailHelperClassWhenItUsesFieldsUndeclaredInTheSuperClass( String className, String fieldName, String fieldType) { ClassRef helper = @@ -504,17 +511,4 @@ private static List parseFieldFlags(String flagsStr) { } return flags; } - - private static Class getMismatchClass(String mismatchName) { - switch (mismatchName) { - case "MissingFlag": - return Mismatch.MissingFlag.class; - case "MissingMethod": - return Mismatch.MissingMethod.class; - case "MissingField": - return Mismatch.MissingField.class; - default: - throw new IllegalArgumentException("Unknown mismatch: " + mismatchName); - } - } } From 72f53a7ee2ae08bb2ab81a728beac58ccb515884 Mon Sep 17 00:00:00 2001 From: Gregor Zeitlinger Date: Fri, 10 Oct 2025 09:29:09 +0200 Subject: [PATCH 07/10] pr review --- .../tooling/muzzle/ReferenceMatcherTest.java | 77 +++++-------------- 1 file changed, 19 insertions(+), 58 deletions(-) diff --git a/muzzle/src/test/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcherTest.java b/muzzle/src/test/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcherTest.java index fe31d41f9b53..7eaf4cc438a0 100644 --- a/muzzle/src/test/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcherTest.java +++ b/muzzle/src/test/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcherTest.java @@ -13,6 +13,8 @@ import static io.opentelemetry.javaagent.tooling.muzzle.references.Flag.MinimumVisibilityFlag.PROTECTED_OR_HIGHER; import static io.opentelemetry.javaagent.tooling.muzzle.references.Flag.OwnershipFlag.NON_STATIC; import static io.opentelemetry.javaagent.tooling.muzzle.references.Flag.OwnershipFlag.STATIC; +import static java.util.Collections.emptySet; +import static java.util.Collections.singleton; import static org.assertj.core.api.Assertions.assertThat; import external.LibraryBaseClass; @@ -26,7 +28,6 @@ import io.opentelemetry.test.TestInterface; import java.net.URL; import java.net.URLClassLoader; -import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; @@ -157,13 +158,13 @@ void matchingRef(Class referenceClass, Flag referenceFlag, Class methodMatchProvider() { return Stream.of( - Arguments.of("method", "(Ljava/lang/String;)Ljava/lang/String;", "", Nested.B.class, null), - Arguments.of("hashCode", "()I", "", Nested.B.class, null), - Arguments.of("someMethod", "()V", "", Nested.SomeInterface.class, null), - Arguments.of("privateStuff", "()V", "PRIVATE_OR_HIGHER", Nested.B.class, null), - Arguments.of("privateStuff", "()V", "PROTECTED_OR_HIGHER", Nested.B2.class, Mismatch.MissingFlag.class), - Arguments.of("staticMethod", "()V", "NON_STATIC", Nested.B.class, Mismatch.MissingFlag.class), - Arguments.of("missingMethod", "()V", "", Nested.B.class, Mismatch.MissingMethod.class)); + Arguments.of("method", "(Ljava/lang/String;)Ljava/lang/String;", emptySet(), Nested.B.class, null), + Arguments.of("hashCode", "()I", emptySet(), Nested.B.class, null), + Arguments.of("someMethod", "()V", emptySet(), Nested.SomeInterface.class, null), + Arguments.of("privateStuff", "()V", singleton(PRIVATE_OR_HIGHER), Nested.B.class, null), + Arguments.of("privateStuff", "()V", singleton(PROTECTED_OR_HIGHER), Nested.B2.class, Mismatch.MissingFlag.class), + Arguments.of("staticMethod", "()V", singleton(NON_STATIC), Nested.B.class, Mismatch.MissingFlag.class), + Arguments.of("missingMethod", "()V", emptySet(), Nested.B.class, Mismatch.MissingMethod.class)); } @ParameterizedTest @@ -171,11 +172,10 @@ private static Stream methodMatchProvider() { void methodMatch( String methodName, String methodDesc, - String methodFlagsStr, + Set methodFlags, Class classToCheck, Class expectedMismatch) { Type methodType = Type.getMethodType(methodDesc); - List methodFlags = parseMethodFlags(methodFlagsStr); ClassRef reference = ClassRef.builder(classToCheck.getName()) @@ -200,14 +200,14 @@ void methodMatch( private static Stream fieldMatchProvider() { return Stream.of( - Arguments.of("missingField", "Ljava/lang/String;", "", Nested.A.class, Mismatch.MissingField.class), - Arguments.of("privateField", "Ljava/lang/String;", "", Nested.A.class, Mismatch.MissingField.class), - Arguments.of("privateField", "Ljava/lang/Object;", "PRIVATE_OR_HIGHER", Nested.A.class, null), - Arguments.of("privateField", "Ljava/lang/Object;", "PROTECTED_OR_HIGHER", Nested.A2.class, Mismatch.MissingFlag.class), - Arguments.of("protectedField", "Ljava/lang/Object;", "STATIC", Nested.A.class, Mismatch.MissingFlag.class), - Arguments.of("staticB", "Lmuzzle/TestClasses$Nested$B;", "STATIC|PROTECTED_OR_HIGHER", Nested.A.class, null), - Arguments.of("number", "I", "PACKAGE_OR_HIGHER", Nested.Primitives.class, null), - Arguments.of("flag", "Z", "PACKAGE_OR_HIGHER", Nested.Primitives.class, null)); + Arguments.of("missingField", "Ljava/lang/String;", emptySet(), Nested.A.class, Mismatch.MissingField.class), + Arguments.of("privateField", "Ljava/lang/String;", emptySet(), Nested.A.class, Mismatch.MissingField.class), + Arguments.of("privateField", "Ljava/lang/Object;", singleton(PRIVATE_OR_HIGHER), Nested.A.class, null), + Arguments.of("privateField", "Ljava/lang/Object;", singleton(PROTECTED_OR_HIGHER), Nested.A2.class, Mismatch.MissingFlag.class), + Arguments.of("protectedField", "Ljava/lang/Object;", singleton(STATIC), Nested.A.class, Mismatch.MissingFlag.class), + Arguments.of("staticB", "Lmuzzle/TestClasses$Nested$B;", new HashSet<>(Arrays.asList(STATIC, PROTECTED_OR_HIGHER)), Nested.A.class, null), + Arguments.of("number", "I", singleton(PACKAGE_OR_HIGHER), Nested.Primitives.class, null), + Arguments.of("flag", "Z", singleton(PACKAGE_OR_HIGHER), Nested.Primitives.class, null)); } @ParameterizedTest @@ -215,11 +215,9 @@ private static Stream fieldMatchProvider() { void fieldMatch( String fieldName, String fieldType, - String fieldFlagsStr, + Set fieldFlags, Class classToCheck, Class expectedMismatch) { - List fieldFlags = parseFieldFlags(fieldFlagsStr); - ClassRef reference = ClassRef.builder(classToCheck.getName()) .addField( @@ -474,41 +472,4 @@ private static Set> getMismatchClassSet(List } return mismatchClasses; } - - private static List parseMethodFlags(String flagsStr) { - if (flagsStr.isEmpty()) { - return Collections.emptyList(); - } - List flags = new ArrayList<>(); - if (flagsStr.contains("PRIVATE_OR_HIGHER")) { - flags.add(PRIVATE_OR_HIGHER); - } - if (flagsStr.contains("PROTECTED_OR_HIGHER")) { - flags.add(PROTECTED_OR_HIGHER); - } - if (flagsStr.contains("NON_STATIC")) { - flags.add(NON_STATIC); - } - return flags; - } - - private static List parseFieldFlags(String flagsStr) { - if (flagsStr.isEmpty()) { - return Collections.emptyList(); - } - List flags = new ArrayList<>(); - if (flagsStr.contains("PRIVATE_OR_HIGHER")) { - flags.add(PRIVATE_OR_HIGHER); - } - if (flagsStr.contains("PROTECTED_OR_HIGHER")) { - flags.add(PROTECTED_OR_HIGHER); - } - if (flagsStr.contains("PACKAGE_OR_HIGHER")) { - flags.add(PACKAGE_OR_HIGHER); - } - if (flagsStr.contains("STATIC")) { - flags.add(STATIC); - } - return flags; - } } From 2e9225a5b1db72f807104ec591889f3feed89e82 Mon Sep 17 00:00:00 2001 From: Gregor Zeitlinger Date: Fri, 10 Oct 2025 09:37:01 +0200 Subject: [PATCH 08/10] pr review --- .../tooling/muzzle/ReferenceMatcherTest.java | 131 ++++++++++-------- 1 file changed, 71 insertions(+), 60 deletions(-) diff --git a/muzzle/src/test/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcherTest.java b/muzzle/src/test/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcherTest.java index 7eaf4cc438a0..58aa25f7f5a0 100644 --- a/muzzle/src/test/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcherTest.java +++ b/muzzle/src/test/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcherTest.java @@ -44,7 +44,6 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; -import org.junit.jupiter.params.provider.ValueSource; import org.objectweb.asm.Type; class ReferenceMatcherTest { @@ -158,25 +157,23 @@ void matchingRef(Class referenceClass, Flag referenceFlag, Class methodMatchProvider() { return Stream.of( - Arguments.of("method", "(Ljava/lang/String;)Ljava/lang/String;", emptySet(), Nested.B.class, null), - Arguments.of("hashCode", "()I", emptySet(), Nested.B.class, null), - Arguments.of("someMethod", "()V", emptySet(), Nested.SomeInterface.class, null), - Arguments.of("privateStuff", "()V", singleton(PRIVATE_OR_HIGHER), Nested.B.class, null), - Arguments.of("privateStuff", "()V", singleton(PROTECTED_OR_HIGHER), Nested.B2.class, Mismatch.MissingFlag.class), - Arguments.of("staticMethod", "()V", singleton(NON_STATIC), Nested.B.class, Mismatch.MissingFlag.class), - Arguments.of("missingMethod", "()V", emptySet(), Nested.B.class, Mismatch.MissingMethod.class)); + Arguments.of("method", Type.getMethodType(Type.getType(String.class), Type.getType(String.class)), emptySet(), Nested.B.class, null), + Arguments.of("hashCode", Type.getMethodType(Type.INT_TYPE), emptySet(), Nested.B.class, null), + Arguments.of("someMethod", Type.getMethodType(Type.VOID_TYPE), emptySet(), Nested.SomeInterface.class, null), + Arguments.of("privateStuff", Type.getMethodType(Type.VOID_TYPE), singleton(PRIVATE_OR_HIGHER), Nested.B.class, null), + Arguments.of("privateStuff", Type.getMethodType(Type.VOID_TYPE), singleton(PROTECTED_OR_HIGHER), Nested.B2.class, Mismatch.MissingFlag.class), + Arguments.of("staticMethod", Type.getMethodType(Type.VOID_TYPE), singleton(NON_STATIC), Nested.B.class, Mismatch.MissingFlag.class), + Arguments.of("missingMethod", Type.getMethodType(Type.VOID_TYPE), emptySet(), Nested.B.class, Mismatch.MissingMethod.class)); } @ParameterizedTest @MethodSource("methodMatchProvider") void methodMatch( String methodName, - String methodDesc, + Type methodType, Set methodFlags, Class classToCheck, Class expectedMismatch) { - Type methodType = Type.getMethodType(methodDesc); - ClassRef reference = ClassRef.builder(classToCheck.getName()) .addMethod( @@ -200,21 +197,21 @@ void methodMatch( private static Stream fieldMatchProvider() { return Stream.of( - Arguments.of("missingField", "Ljava/lang/String;", emptySet(), Nested.A.class, Mismatch.MissingField.class), - Arguments.of("privateField", "Ljava/lang/String;", emptySet(), Nested.A.class, Mismatch.MissingField.class), - Arguments.of("privateField", "Ljava/lang/Object;", singleton(PRIVATE_OR_HIGHER), Nested.A.class, null), - Arguments.of("privateField", "Ljava/lang/Object;", singleton(PROTECTED_OR_HIGHER), Nested.A2.class, Mismatch.MissingFlag.class), - Arguments.of("protectedField", "Ljava/lang/Object;", singleton(STATIC), Nested.A.class, Mismatch.MissingFlag.class), - Arguments.of("staticB", "Lmuzzle/TestClasses$Nested$B;", new HashSet<>(Arrays.asList(STATIC, PROTECTED_OR_HIGHER)), Nested.A.class, null), - Arguments.of("number", "I", singleton(PACKAGE_OR_HIGHER), Nested.Primitives.class, null), - Arguments.of("flag", "Z", singleton(PACKAGE_OR_HIGHER), Nested.Primitives.class, null)); + Arguments.of("missingField", Type.getType(String.class), emptySet(), Nested.A.class, Mismatch.MissingField.class), + Arguments.of("privateField", Type.getType(String.class), emptySet(), Nested.A.class, Mismatch.MissingField.class), + Arguments.of("privateField", Type.getType(Object.class), singleton(PRIVATE_OR_HIGHER), Nested.A.class, null), + Arguments.of("privateField", Type.getType(Object.class), singleton(PROTECTED_OR_HIGHER), Nested.A2.class, Mismatch.MissingFlag.class), + Arguments.of("protectedField", Type.getType(Object.class), singleton(STATIC), Nested.A.class, Mismatch.MissingFlag.class), + Arguments.of("staticB", Type.getType(Nested.B.class), new HashSet<>(Arrays.asList(STATIC, PROTECTED_OR_HIGHER)), Nested.A.class, null), + Arguments.of("number", Type.INT_TYPE, singleton(PACKAGE_OR_HIGHER), Nested.Primitives.class, null), + Arguments.of("flag", Type.BOOLEAN_TYPE, singleton(PACKAGE_OR_HIGHER), Nested.Primitives.class, null)); } @ParameterizedTest @MethodSource("fieldMatchProvider") void fieldMatch( String fieldName, - String fieldType, + Type fieldType, Set fieldFlags, Class classToCheck, Class expectedMismatch) { @@ -224,7 +221,7 @@ void fieldMatch( new Source[0], fieldFlags.toArray(new Flag[0]), fieldName, - Type.getType(fieldType), + fieldType, false) .build(); @@ -239,12 +236,14 @@ void fieldMatch( } } + private static Stream shouldNotCheckAbstractHelperClassesProvider() { + return Stream.of( + Arguments.of("io.opentelemetry.instrumentation.Helper"), + Arguments.of("com.external.otel.instrumentation.Helper")); + } + @ParameterizedTest - @ValueSource( - strings = { - "io.opentelemetry.instrumentation.Helper", - "com.external.otel.instrumentation.Helper" - }) + @MethodSource("shouldNotCheckAbstractHelperClassesProvider") void shouldNotCheckAbstractHelperClasses(String className) { ClassRef reference = ClassRef.builder(className) @@ -262,17 +261,19 @@ void shouldNotCheckAbstractHelperClasses(String className) { assertThat(mismatches).isEmpty(); } + private static Stream shouldNotCheckHelperClassesWithNoSupertypesProvider() { + return Stream.of( + Arguments.of("io.opentelemetry.instrumentation.Helper", "someMethod", Type.VOID_TYPE), + Arguments.of("com.external.otel.instrumentation.Helper", "someMethod", Type.VOID_TYPE)); + } + @ParameterizedTest - @ValueSource( - strings = { - "io.opentelemetry.instrumentation.Helper", - "com.external.otel.instrumentation.Helper" - }) - void shouldNotCheckHelperClassesWithNoSupertypes(String className) { + @MethodSource("shouldNotCheckHelperClassesWithNoSupertypesProvider") + void shouldNotCheckHelperClassesWithNoSupertypes(String className, String methodName, Type returnType) { ClassRef reference = ClassRef.builder(className) .setSuperClassName(Object.class.getName()) - .addMethod(new Source[0], new Flag[0], "someMethod", Type.VOID_TYPE) + .addMethod(new Source[0], new Flag[0], methodName, returnType) .build(); List mismatches = @@ -284,12 +285,14 @@ void shouldNotCheckHelperClassesWithNoSupertypes(String className) { assertThat(mismatches).isEmpty(); } + private static Stream shouldFailHelperClassesThatDoNotImplementAllAbstractMethodsProvider() { + return Stream.of( + Arguments.of("io.opentelemetry.instrumentation.Helper"), + Arguments.of("com.external.otel.instrumentation.Helper")); + } + @ParameterizedTest - @ValueSource( - strings = { - "io.opentelemetry.instrumentation.Helper", - "com.external.otel.instrumentation.Helper" - }) + @MethodSource("shouldFailHelperClassesThatDoNotImplementAllAbstractMethodsProvider") void shouldFailHelperClassesThatDoNotImplementAllAbstractMethods(String className) { ClassRef reference = ClassRef.builder(className) @@ -306,12 +309,14 @@ void shouldFailHelperClassesThatDoNotImplementAllAbstractMethods(String classNam assertThat(getMismatchClassSet(mismatches)).containsExactly(Mismatch.MissingMethod.class); } + private static Stream shouldFailHelperClassesThatDoNotImplementAllAbstractMethodsEvenIfEmptyAbstractClassReferenceExistsProvider() { + return Stream.of( + Arguments.of("io.opentelemetry.instrumentation.Helper"), + Arguments.of("com.external.otel.instrumentation.Helper")); + } + @ParameterizedTest - @ValueSource( - strings = { - "io.opentelemetry.instrumentation.Helper", - "com.external.otel.instrumentation.Helper" - }) + @MethodSource("shouldFailHelperClassesThatDoNotImplementAllAbstractMethodsEvenIfEmptyAbstractClassReferenceExistsProvider") void shouldFailHelperClassesThatDoNotImplementAllAbstractMethodsEvenIfEmptyAbstractClassReferenceExists( String className) { @@ -336,12 +341,14 @@ void shouldFailHelperClassesThatDoNotImplementAllAbstractMethods(String classNam assertThat(getMismatchClassSet(mismatches)).containsExactly(Mismatch.MissingMethod.class); } + private static Stream shouldCheckHelperClassWhetherInterfaceMethodsAreImplementedInTheSuperClassProvider() { + return Stream.of( + Arguments.of("io.opentelemetry.instrumentation.Helper"), + Arguments.of("com.external.otel.instrumentation.Helper")); + } + @ParameterizedTest - @ValueSource( - strings = { - "io.opentelemetry.instrumentation.Helper", - "com.external.otel.instrumentation.Helper" - }) + @MethodSource("shouldCheckHelperClassWhetherInterfaceMethodsAreImplementedInTheSuperClassProvider") void shouldCheckHelperClassWhetherInterfaceMethodsAreImplementedInTheSuperClass( String className) { ClassRef baseHelper = @@ -371,12 +378,14 @@ void shouldCheckHelperClassWhetherInterfaceMethodsAreImplementedInTheSuperClass( assertThat(mismatches).isEmpty(); } + private static Stream shouldCheckHelperClassWhetherUsedFieldsAreDeclaredInTheSuperClassProvider() { + return Stream.of( + Arguments.of("io.opentelemetry.instrumentation.Helper"), + Arguments.of("com.external.otel.instrumentation.Helper")); + } + @ParameterizedTest - @ValueSource( - strings = { - "io.opentelemetry.instrumentation.Helper", - "com.external.otel.instrumentation.Helper" - }) + @MethodSource("shouldCheckHelperClassWhetherUsedFieldsAreDeclaredInTheSuperClassProvider") void shouldCheckHelperClassWhetherUsedFieldsAreDeclaredInTheSuperClass(String className) { ClassRef helper = ClassRef.builder(className) @@ -388,7 +397,7 @@ void shouldCheckHelperClassWhetherUsedFieldsAreDeclaredInTheSuperClass(String cl List mismatches = createMatcher( Collections.singletonMap(helper.getClassName(), helper), - Collections.singletonList(helper.getClassName())) + Collections.singletonList(helper.getClass().getClassLoader())) .getMismatchedReferenceSources(this.getClass().getClassLoader()); assertThat(mismatches).isEmpty(); @@ -418,18 +427,20 @@ void shouldFailHelperClassWhenItUsesFieldsUndeclaredInTheSuperClass( List mismatches = createMatcher( Collections.singletonMap(helper.getClassName(), helper), - Collections.singletonList(helper.getClassName())) + Collections.singletonList(helper.getClass().getClassLoader())) .getMismatchedReferenceSources(this.getClass().getClassLoader()); assertThat(getMismatchClassSet(mismatches)).containsExactly(Mismatch.MissingField.class); } + private static Stream shouldFailHelperClassWhenTheLibraryParentClassHasDifferentConstructorProvider() { + return Stream.of( + Arguments.of("io.opentelemetry.instrumentation.Helper"), + Arguments.of("com.external.otel.instrumentation.Helper")); + } + @ParameterizedTest - @ValueSource( - strings = { - "io.opentelemetry.instrumentation.Helper", - "com.external.otel.instrumentation.Helper" - }) + @MethodSource("shouldFailHelperClassWhenTheLibraryParentClassHasDifferentConstructorProvider") void shouldFailHelperClassWhenTheLibraryParentClassHasDifferentConstructor(String className) { ClassRef helper = ClassRef.builder(className) From a946937a7288f1112997c874630f5e2f73070eb9 Mon Sep 17 00:00:00 2001 From: Gregor Zeitlinger Date: Fri, 10 Oct 2025 13:00:34 +0200 Subject: [PATCH 09/10] pr review --- .../tooling/muzzle/ReferenceMatcherTest.java | 77 +++++-------------- 1 file changed, 20 insertions(+), 57 deletions(-) diff --git a/muzzle/src/test/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcherTest.java b/muzzle/src/test/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcherTest.java index 58aa25f7f5a0..9d02ec59c6fb 100644 --- a/muzzle/src/test/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcherTest.java +++ b/muzzle/src/test/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcherTest.java @@ -43,6 +43,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.CsvSource; import org.junit.jupiter.params.provider.MethodSource; import org.objectweb.asm.Type; @@ -166,9 +167,10 @@ private static Stream methodMatchProvider() { Arguments.of("missingMethod", Type.getMethodType(Type.VOID_TYPE), emptySet(), Nested.B.class, Mismatch.MissingMethod.class)); } - @ParameterizedTest + @ParameterizedTest(name = "{0}") @MethodSource("methodMatchProvider") void methodMatch( + String name, String methodName, Type methodType, Set methodFlags, @@ -236,14 +238,14 @@ void fieldMatch( } } - private static Stream shouldNotCheckAbstractHelperClassesProvider() { + private static Stream helperClassNames() { return Stream.of( Arguments.of("io.opentelemetry.instrumentation.Helper"), Arguments.of("com.external.otel.instrumentation.Helper")); } @ParameterizedTest - @MethodSource("shouldNotCheckAbstractHelperClassesProvider") + @MethodSource("helperClassNames") void shouldNotCheckAbstractHelperClasses(String className) { ClassRef reference = ClassRef.builder(className) @@ -261,19 +263,13 @@ void shouldNotCheckAbstractHelperClasses(String className) { assertThat(mismatches).isEmpty(); } - private static Stream shouldNotCheckHelperClassesWithNoSupertypesProvider() { - return Stream.of( - Arguments.of("io.opentelemetry.instrumentation.Helper", "someMethod", Type.VOID_TYPE), - Arguments.of("com.external.otel.instrumentation.Helper", "someMethod", Type.VOID_TYPE)); - } - @ParameterizedTest - @MethodSource("shouldNotCheckHelperClassesWithNoSupertypesProvider") - void shouldNotCheckHelperClassesWithNoSupertypes(String className, String methodName, Type returnType) { + @MethodSource("helperClassNames") + void shouldNotCheckHelperClassesWithNoSupertypes(String className) { ClassRef reference = ClassRef.builder(className) .setSuperClassName(Object.class.getName()) - .addMethod(new Source[0], new Flag[0], methodName, returnType) + .addMethod(new Source[0], new Flag[0], "someMethod", Type.VOID_TYPE) .build(); List mismatches = @@ -285,14 +281,8 @@ void shouldNotCheckHelperClassesWithNoSupertypes(String className, String method assertThat(mismatches).isEmpty(); } - private static Stream shouldFailHelperClassesThatDoNotImplementAllAbstractMethodsProvider() { - return Stream.of( - Arguments.of("io.opentelemetry.instrumentation.Helper"), - Arguments.of("com.external.otel.instrumentation.Helper")); - } - @ParameterizedTest - @MethodSource("shouldFailHelperClassesThatDoNotImplementAllAbstractMethodsProvider") + @MethodSource("helperClassNames") void shouldFailHelperClassesThatDoNotImplementAllAbstractMethods(String className) { ClassRef reference = ClassRef.builder(className) @@ -309,14 +299,8 @@ void shouldFailHelperClassesThatDoNotImplementAllAbstractMethods(String classNam assertThat(getMismatchClassSet(mismatches)).containsExactly(Mismatch.MissingMethod.class); } - private static Stream shouldFailHelperClassesThatDoNotImplementAllAbstractMethodsEvenIfEmptyAbstractClassReferenceExistsProvider() { - return Stream.of( - Arguments.of("io.opentelemetry.instrumentation.Helper"), - Arguments.of("com.external.otel.instrumentation.Helper")); - } - @ParameterizedTest - @MethodSource("shouldFailHelperClassesThatDoNotImplementAllAbstractMethodsEvenIfEmptyAbstractClassReferenceExistsProvider") + @MethodSource("helperClassNames") void shouldFailHelperClassesThatDoNotImplementAllAbstractMethodsEvenIfEmptyAbstractClassReferenceExists( String className) { @@ -341,14 +325,8 @@ private static Stream shouldFailHelperClassesThatDoNotImplementAllAbs assertThat(getMismatchClassSet(mismatches)).containsExactly(Mismatch.MissingMethod.class); } - private static Stream shouldCheckHelperClassWhetherInterfaceMethodsAreImplementedInTheSuperClassProvider() { - return Stream.of( - Arguments.of("io.opentelemetry.instrumentation.Helper"), - Arguments.of("com.external.otel.instrumentation.Helper")); - } - @ParameterizedTest - @MethodSource("shouldCheckHelperClassWhetherInterfaceMethodsAreImplementedInTheSuperClassProvider") + @MethodSource("helperClassNames") void shouldCheckHelperClassWhetherInterfaceMethodsAreImplementedInTheSuperClass( String className) { ClassRef baseHelper = @@ -378,14 +356,8 @@ void shouldCheckHelperClassWhetherInterfaceMethodsAreImplementedInTheSuperClass( assertThat(mismatches).isEmpty(); } - private static Stream shouldCheckHelperClassWhetherUsedFieldsAreDeclaredInTheSuperClassProvider() { - return Stream.of( - Arguments.of("io.opentelemetry.instrumentation.Helper"), - Arguments.of("com.external.otel.instrumentation.Helper")); - } - @ParameterizedTest - @MethodSource("shouldCheckHelperClassWhetherUsedFieldsAreDeclaredInTheSuperClassProvider") + @MethodSource("helperClassNames") void shouldCheckHelperClassWhetherUsedFieldsAreDeclaredInTheSuperClass(String className) { ClassRef helper = ClassRef.builder(className) @@ -397,22 +369,19 @@ void shouldCheckHelperClassWhetherUsedFieldsAreDeclaredInTheSuperClass(String cl List mismatches = createMatcher( Collections.singletonMap(helper.getClassName(), helper), - Collections.singletonList(helper.getClass().getClassLoader())) + singletonList(helper.getClass().getClassLoader())) .getMismatchedReferenceSources(this.getClass().getClassLoader()); assertThat(mismatches).isEmpty(); } - private static Stream shouldFailHelperClassWhenItUsesFieldsUndeclaredInTheSuperClassProvider() { - return Stream.of( - Arguments.of("io.opentelemetry.instrumentation.Helper", "differentField", "Ljava/lang/Integer;"), - Arguments.of("io.opentelemetry.instrumentation.Helper", "field", "Lcom/external/DifferentType;"), - Arguments.of("com.external.otel.instrumentation.Helper", "differentField", "Ljava/lang/Integer;"), - Arguments.of("com.external.otel.instrumentation.Helper", "field", "Lcom/external/DifferentType;")); - } - @ParameterizedTest - @MethodSource("shouldFailHelperClassWhenItUsesFieldsUndeclaredInTheSuperClassProvider") + @CsvSource({ + "io.opentelemetry.instrumentation.Helper, differentField, Ljava/lang/Integer;", + "io.opentelemetry.instrumentation.Helper, field, Lcom/external/DifferentType;", + "com.external.otel.instrumentation.Helper, differentField, Ljava/lang/Integer;", + "com.external.otel.instrumentation.Helper, field, Lcom/external/DifferentType;" + }) void shouldFailHelperClassWhenItUsesFieldsUndeclaredInTheSuperClass( String className, String fieldName, String fieldType) { ClassRef helper = @@ -433,14 +402,8 @@ void shouldFailHelperClassWhenItUsesFieldsUndeclaredInTheSuperClass( assertThat(getMismatchClassSet(mismatches)).containsExactly(Mismatch.MissingField.class); } - private static Stream shouldFailHelperClassWhenTheLibraryParentClassHasDifferentConstructorProvider() { - return Stream.of( - Arguments.of("io.opentelemetry.instrumentation.Helper"), - Arguments.of("com.external.otel.instrumentation.Helper")); - } - @ParameterizedTest - @MethodSource("shouldFailHelperClassWhenTheLibraryParentClassHasDifferentConstructorProvider") + @MethodSource("helperClassNames") void shouldFailHelperClassWhenTheLibraryParentClassHasDifferentConstructor(String className) { ClassRef helper = ClassRef.builder(className) From f125132274d7a89659a4c908129c6008c375fb13 Mon Sep 17 00:00:00 2001 From: Gregor Zeitlinger Date: Fri, 10 Oct 2025 13:11:46 +0200 Subject: [PATCH 10/10] pr review --- .../tooling/muzzle/ReferenceMatcherTest.java | 181 +++++++++++++----- 1 file changed, 135 insertions(+), 46 deletions(-) diff --git a/muzzle/src/test/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcherTest.java b/muzzle/src/test/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcherTest.java index 9d02ec59c6fb..cbaa17664bd0 100644 --- a/muzzle/src/test/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcherTest.java +++ b/muzzle/src/test/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcherTest.java @@ -15,6 +15,8 @@ import static io.opentelemetry.javaagent.tooling.muzzle.references.Flag.OwnershipFlag.STATIC; import static java.util.Collections.emptySet; import static java.util.Collections.singleton; +import static java.util.Collections.singletonList; +import static java.util.Collections.singletonMap; import static org.assertj.core.api.Assertions.assertThat; import external.LibraryBaseClass; @@ -142,11 +144,12 @@ private static Stream matchingRefProvider() { @ParameterizedTest @MethodSource("matchingRefProvider") - void matchingRef(Class referenceClass, Flag referenceFlag, Class expectedMismatch) { + void matchingRef( + Class referenceClass, Flag referenceFlag, Class expectedMismatch) { ClassRef ref = ClassRef.builder(referenceClass.getName()).addFlag(referenceFlag).build(); List mismatches = - createMatcher(Collections.singletonMap(ref.getClassName(), ref)) + createMatcher(singletonMap(ref.getClassName(), ref)) .getMismatchedReferenceSources(this.getClass().getClassLoader()); if (expectedMismatch == null) { @@ -158,19 +161,61 @@ void matchingRef(Class referenceClass, Flag referenceFlag, Class methodMatchProvider() { return Stream.of( - Arguments.of("method", Type.getMethodType(Type.getType(String.class), Type.getType(String.class)), emptySet(), Nested.B.class, null), - Arguments.of("hashCode", Type.getMethodType(Type.INT_TYPE), emptySet(), Nested.B.class, null), - Arguments.of("someMethod", Type.getMethodType(Type.VOID_TYPE), emptySet(), Nested.SomeInterface.class, null), - Arguments.of("privateStuff", Type.getMethodType(Type.VOID_TYPE), singleton(PRIVATE_OR_HIGHER), Nested.B.class, null), - Arguments.of("privateStuff", Type.getMethodType(Type.VOID_TYPE), singleton(PROTECTED_OR_HIGHER), Nested.B2.class, Mismatch.MissingFlag.class), - Arguments.of("staticMethod", Type.getMethodType(Type.VOID_TYPE), singleton(NON_STATIC), Nested.B.class, Mismatch.MissingFlag.class), - Arguments.of("missingMethod", Type.getMethodType(Type.VOID_TYPE), emptySet(), Nested.B.class, Mismatch.MissingMethod.class)); + Arguments.of( + "match method declared in class", + "method", + Type.getMethodType(Type.getType(String.class), Type.getType(String.class)), + emptySet(), + Nested.B.class, + null), + Arguments.of( + "match method declared in superclass", + "hashCode", + Type.getMethodType(Type.INT_TYPE), + emptySet(), + Nested.B.class, + null), + Arguments.of( + "match method declared in interface", + "someMethod", + Type.getMethodType(Type.VOID_TYPE), + emptySet(), + Nested.SomeInterface.class, + null), + Arguments.of( + "match private method", + "privateStuff", + Type.getMethodType(Type.VOID_TYPE), + singleton(PRIVATE_OR_HIGHER), + Nested.B.class, + null), + Arguments.of( + "fail match private in supertype", + "privateStuff", + Type.getMethodType(Type.VOID_TYPE), + singleton(PROTECTED_OR_HIGHER), + Nested.B2.class, + Mismatch.MissingFlag.class), + Arguments.of( + "static method mismatch", + "staticMethod", + Type.getMethodType(Type.VOID_TYPE), + singleton(NON_STATIC), + Nested.B.class, + Mismatch.MissingFlag.class), + Arguments.of( + "missing method mismatch", + "missingMethod", + Type.getMethodType(Type.VOID_TYPE), + emptySet(), + Nested.B.class, + Mismatch.MissingMethod.class)); } @ParameterizedTest(name = "{0}") @MethodSource("methodMatchProvider") void methodMatch( - String name, + String testName, String methodName, Type methodType, Set methodFlags, @@ -187,7 +232,7 @@ void methodMatch( .build(); List mismatches = - createMatcher(Collections.singletonMap(reference.getClassName(), reference)) + createMatcher(singletonMap(reference.getClassName(), reference)) .getMismatchedReferenceSources(this.getClass().getClassLoader()); if (expectedMismatch == null) { @@ -199,19 +244,68 @@ void methodMatch( private static Stream fieldMatchProvider() { return Stream.of( - Arguments.of("missingField", Type.getType(String.class), emptySet(), Nested.A.class, Mismatch.MissingField.class), - Arguments.of("privateField", Type.getType(String.class), emptySet(), Nested.A.class, Mismatch.MissingField.class), - Arguments.of("privateField", Type.getType(Object.class), singleton(PRIVATE_OR_HIGHER), Nested.A.class, null), - Arguments.of("privateField", Type.getType(Object.class), singleton(PROTECTED_OR_HIGHER), Nested.A2.class, Mismatch.MissingFlag.class), - Arguments.of("protectedField", Type.getType(Object.class), singleton(STATIC), Nested.A.class, Mismatch.MissingFlag.class), - Arguments.of("staticB", Type.getType(Nested.B.class), new HashSet<>(Arrays.asList(STATIC, PROTECTED_OR_HIGHER)), Nested.A.class, null), - Arguments.of("number", Type.INT_TYPE, singleton(PACKAGE_OR_HIGHER), Nested.Primitives.class, null), - Arguments.of("flag", Type.BOOLEAN_TYPE, singleton(PACKAGE_OR_HIGHER), Nested.Primitives.class, null)); + Arguments.of( + "mismatch missing field", + "missingField", + Type.getType(String.class), + emptySet(), + Nested.A.class, + Mismatch.MissingField.class), + Arguments.of( + "mismatch field type signature", + "privateField", + Type.getType(String.class), + emptySet(), + Nested.A.class, + Mismatch.MissingField.class), + Arguments.of( + "match private field", + "privateField", + Type.getType(Object.class), + singleton(PRIVATE_OR_HIGHER), + Nested.A.class, + null), + Arguments.of( + "mismatch private field in supertype", + "privateField", + Type.getType(Object.class), + singleton(PROTECTED_OR_HIGHER), + Nested.A2.class, + Mismatch.MissingFlag.class), + Arguments.of( + "mismatch static field", + "protectedField", + Type.getType(Object.class), + singleton(STATIC), + Nested.A.class, + Mismatch.MissingFlag.class), + Arguments.of( + "match static field", + "staticB", + Type.getType(Nested.B.class), + new HashSet<>(Arrays.asList(STATIC, PROTECTED_OR_HIGHER)), + Nested.A.class, + null), + Arguments.of( + "match primitive int", + "number", + Type.INT_TYPE, + singleton(PACKAGE_OR_HIGHER), + Nested.Primitives.class, + null), + Arguments.of( + "match primitive boolean", + "flag", + Type.BOOLEAN_TYPE, + singleton(PACKAGE_OR_HIGHER), + Nested.Primitives.class, + null)); } - @ParameterizedTest + @ParameterizedTest(name = "{0}") @MethodSource("fieldMatchProvider") void fieldMatch( + String testName, String fieldName, Type fieldType, Set fieldFlags, @@ -219,16 +313,11 @@ void fieldMatch( Class expectedMismatch) { ClassRef reference = ClassRef.builder(classToCheck.getName()) - .addField( - new Source[0], - fieldFlags.toArray(new Flag[0]), - fieldName, - fieldType, - false) + .addField(new Source[0], fieldFlags.toArray(new Flag[0]), fieldName, fieldType, false) .build(); List mismatches = - createMatcher(Collections.singletonMap(reference.getClassName(), reference)) + createMatcher(singletonMap(reference.getClassName(), reference)) .getMismatchedReferenceSources(this.getClass().getClassLoader()); if (expectedMismatch == null) { @@ -256,8 +345,8 @@ void shouldNotCheckAbstractHelperClasses(String className) { List mismatches = createMatcher( - Collections.singletonMap(reference.getClassName(), reference), - Collections.singletonList(reference.getClassName())) + singletonMap(reference.getClassName(), reference), + singletonList(reference.getClassName())) .getMismatchedReferenceSources(this.getClass().getClassLoader()); assertThat(mismatches).isEmpty(); @@ -274,8 +363,8 @@ void shouldNotCheckHelperClassesWithNoSupertypes(String className) { List mismatches = createMatcher( - Collections.singletonMap(reference.getClassName(), reference), - Collections.singletonList(reference.getClassName())) + singletonMap(reference.getClassName(), reference), + singletonList(reference.getClassName())) .getMismatchedReferenceSources(this.getClass().getClassLoader()); assertThat(mismatches).isEmpty(); @@ -292,8 +381,8 @@ void shouldFailHelperClassesThatDoNotImplementAllAbstractMethods(String classNam List mismatches = createMatcher( - Collections.singletonMap(reference.getClassName(), reference), - Collections.singletonList(reference.getClassName())) + singletonMap(reference.getClassName(), reference), + singletonList(reference.getClassName())) .getMismatchedReferenceSources(this.getClass().getClassLoader()); assertThat(getMismatchClassSet(mismatches)).containsExactly(Mismatch.MissingMethod.class); @@ -368,22 +457,23 @@ void shouldCheckHelperClassWhetherUsedFieldsAreDeclaredInTheSuperClass(String cl List mismatches = createMatcher( - Collections.singletonMap(helper.getClassName(), helper), - singletonList(helper.getClass().getClassLoader())) + singletonMap(helper.getClassName(), helper), singletonList(helper.getClassName())) .getMismatchedReferenceSources(this.getClass().getClassLoader()); assertThat(mismatches).isEmpty(); } - @ParameterizedTest - @CsvSource({ - "io.opentelemetry.instrumentation.Helper, differentField, Ljava/lang/Integer;", - "io.opentelemetry.instrumentation.Helper, field, Lcom/external/DifferentType;", - "com.external.otel.instrumentation.Helper, differentField, Ljava/lang/Integer;", - "com.external.otel.instrumentation.Helper, field, Lcom/external/DifferentType;" - }) + @ParameterizedTest(name = "{0}") + @CsvSource( + delimiter = '|', + value = { + "internal helper, different field name | io.opentelemetry.instrumentation.Helper | differentField | Ljava/lang/Integer;", + "internal helper, different field type | io.opentelemetry.instrumentation.Helper | field | Lcom/external/DifferentType;", + "external helper, different field name | com.external.otel.instrumentation.Helper | differentField | Ljava/lang/Integer;", + "external helper, different field type | com.external.otel.instrumentation.Helper | field | Lcom/external/DifferentType;" + }) void shouldFailHelperClassWhenItUsesFieldsUndeclaredInTheSuperClass( - String className, String fieldName, String fieldType) { + String testName, String className, String fieldName, String fieldType) { ClassRef helper = ClassRef.builder(className) .setSuperClassName( @@ -395,8 +485,7 @@ void shouldFailHelperClassWhenItUsesFieldsUndeclaredInTheSuperClass( List mismatches = createMatcher( - Collections.singletonMap(helper.getClassName(), helper), - Collections.singletonList(helper.getClass().getClassLoader())) + singletonMap(helper.getClassName(), helper), singletonList(helper.getClassName())) .getMismatchedReferenceSources(this.getClass().getClassLoader()); assertThat(getMismatchClassSet(mismatches)).containsExactly(Mismatch.MissingField.class); @@ -421,7 +510,7 @@ void shouldFailHelperClassWhenTheLibraryParentClassHasDifferentConstructor(Strin references.put(baseClassRef.getClassName(), baseClassRef); List mismatches = - createMatcher(references, Collections.singletonList(helper.getClassName())) + createMatcher(references, singletonList(helper.getClassName())) .getMismatchedReferenceSources(this.getClass().getClassLoader()); assertThat(getMismatchClassSet(mismatches)).containsExactly(Mismatch.MissingMethod.class);