From 80f13bc6705a29b7cdf29e9639b8401c314788aa Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Thu, 27 Feb 2020 16:26:53 -0800 Subject: [PATCH] Add proper hashcode/equals methods to our matchers I copied over the methods that would have been generated had the `HashCodeAndEqualsPlugin` actually been working. Also updated the matcher tests to use the TypePool for more realistic values. --- .../matcher/HasSuperMethodMatcher.java | 20 +++++++++++-- .../matcher/LoggingFailSafeMatcher.java | 24 +++++++++++++-- .../bytebuddy/matcher/SafeErasureMatcher.java | 30 ++++++++++++++----- .../matcher/SafeExtendsClassMatcher.java | 19 ++++++++++++ .../matcher/SafeHasSuperTypeMatcher.java | 20 +++++++++++-- .../matcher/ExtendsClassMatcherTest.groovy | 8 ++++- .../matcher/HasInterfaceMatcherTest.groovy | 8 ++++- .../ImplementsInterfaceMatcherTest.groovy | 8 ++++- .../SafeHasSuperTypeMatcherTest.groovy | 8 ++++- .../ServerErrorHandlerInstrumentation.java | 4 +-- 10 files changed, 128 insertions(+), 21 deletions(-) diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/HasSuperMethodMatcher.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/HasSuperMethodMatcher.java index e0dd0ae66a..cacef3ed17 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/HasSuperMethodMatcher.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/HasSuperMethodMatcher.java @@ -5,14 +5,12 @@ import static net.bytebuddy.matcher.ElementMatchers.hasSignature; import java.util.HashSet; import java.util.Set; -import net.bytebuddy.build.HashCodeAndEqualsPlugin; import net.bytebuddy.description.method.MethodDescription; import net.bytebuddy.description.type.TypeDefinition; import net.bytebuddy.description.type.TypeList; import net.bytebuddy.matcher.ElementMatcher; // TODO: add javadoc -@HashCodeAndEqualsPlugin.Enhance class HasSuperMethodMatcher extends ElementMatcher.Junction.AbstractBase { @@ -69,4 +67,22 @@ class HasSuperMethodMatcher public String toString() { return "hasSuperMethodMatcher(" + matcher + ")"; } + + @Override + public boolean equals(final Object other) { + if (this == other) { + return true; + } else if (other == null) { + return false; + } else if (getClass() != other.getClass()) { + return false; + } else { + return matcher.equals(((HasSuperMethodMatcher) other).matcher); + } + } + + @Override + public int hashCode() { + return 17 * 31 + matcher.hashCode(); + } } diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/LoggingFailSafeMatcher.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/LoggingFailSafeMatcher.java index 3d7552dff3..e7dd5f97ab 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/LoggingFailSafeMatcher.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/LoggingFailSafeMatcher.java @@ -1,7 +1,6 @@ package datadog.trace.agent.tooling.bytebuddy.matcher; import lombok.extern.slf4j.Slf4j; -import net.bytebuddy.build.HashCodeAndEqualsPlugin; import net.bytebuddy.matcher.ElementMatcher; /** @@ -14,7 +13,6 @@ import net.bytebuddy.matcher.ElementMatcher; * @see net.bytebuddy.matcher.FailSafeMatcher */ @Slf4j -@HashCodeAndEqualsPlugin.Enhance class LoggingFailSafeMatcher extends ElementMatcher.Junction.AbstractBase { /** The delegate matcher that might throw an exception. */ @@ -52,6 +50,26 @@ class LoggingFailSafeMatcher extends ElementMatcher.Junction.AbstractBase @Override public String toString() { - return "safeMatcher(try(" + matcher + ") or " + fallback + ")"; + return "failSafe(try(" + matcher + ") or " + fallback + ")"; + } + + @Override + public boolean equals(final Object var1) { + if (this == var1) { + return true; + } else if (var1 == null) { + return false; + } else if (getClass() != var1.getClass()) { + return false; + } else if (fallback != ((LoggingFailSafeMatcher) var1).fallback) { + return false; + } else { + return matcher.equals(((LoggingFailSafeMatcher) var1).matcher); + } + } + + @Override + public int hashCode() { + return (17 * 31 + matcher.hashCode()) * 31 + (fallback ? 1231 : 1237); } } diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/SafeErasureMatcher.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/SafeErasureMatcher.java index 29430da3ed..464417ea4e 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/SafeErasureMatcher.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/SafeErasureMatcher.java @@ -3,7 +3,6 @@ package datadog.trace.agent.tooling.bytebuddy.matcher; import static datadog.trace.agent.tooling.bytebuddy.matcher.DDElementMatchers.safeTypeDefinitionName; import lombok.extern.slf4j.Slf4j; -import net.bytebuddy.build.HashCodeAndEqualsPlugin; import net.bytebuddy.description.type.TypeDefinition; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; @@ -19,7 +18,6 @@ import net.bytebuddy.matcher.ElementMatcher; * @see net.bytebuddy.matcher.ErasureMatcher */ @Slf4j -@HashCodeAndEqualsPlugin.Enhance class SafeErasureMatcher extends ElementMatcher.Junction.AbstractBase { /** The matcher to apply to the raw type of the matched element. */ @@ -45,11 +43,6 @@ class SafeErasureMatcher extends ElementMatcher.Juncti } } - @Override - public String toString() { - return "safeErasure(" + matcher + ")"; - } - static TypeDescription safeAsErasure(final TypeDefinition typeDefinition) { try { return typeDefinition.asErasure(); @@ -62,4 +55,27 @@ class SafeErasureMatcher extends ElementMatcher.Juncti return null; } } + + @Override + public String toString() { + return "safeErasure(" + matcher + ")"; + } + + @Override + public boolean equals(final Object other) { + if (this == other) { + return true; + } else if (other == null) { + return false; + } else if (getClass() != other.getClass()) { + return false; + } else { + return matcher.equals(((SafeErasureMatcher) other).matcher); + } + } + + @Override + public int hashCode() { + return 17 * 31 + matcher.hashCode(); + } } diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/SafeExtendsClassMatcher.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/SafeExtendsClassMatcher.java index ddce190fd6..bacff3b5e6 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/SafeExtendsClassMatcher.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/SafeExtendsClassMatcher.java @@ -6,6 +6,7 @@ import net.bytebuddy.description.type.TypeDefinition; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; +// TODO: add javadoc class SafeExtendsClassMatcher extends ElementMatcher.Junction.AbstractBase { @@ -33,4 +34,22 @@ class SafeExtendsClassMatcher public String toString() { return "safeExtendsClass(" + matcher + ")"; } + + @Override + public boolean equals(final Object other) { + if (this == other) { + return true; + } else if (other == null) { + return false; + } else if (getClass() != other.getClass()) { + return false; + } else { + return matcher.equals(((SafeExtendsClassMatcher) other).matcher); + } + } + + @Override + public int hashCode() { + return 17 * 31 + matcher.hashCode(); + } } diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/SafeHasSuperTypeMatcher.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/SafeHasSuperTypeMatcher.java index c3a0d74f66..be34ef9a44 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/SafeHasSuperTypeMatcher.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/SafeHasSuperTypeMatcher.java @@ -9,7 +9,6 @@ import java.util.Iterator; import java.util.List; import java.util.Set; import lombok.extern.slf4j.Slf4j; -import net.bytebuddy.build.HashCodeAndEqualsPlugin; import net.bytebuddy.description.type.TypeDefinition; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; @@ -33,7 +32,6 @@ import net.bytebuddy.matcher.ElementMatcher; * @see net.bytebuddy.matcher.HasSuperTypeMatcher */ @Slf4j -@HashCodeAndEqualsPlugin.Enhance class SafeHasSuperTypeMatcher extends ElementMatcher.Junction.AbstractBase { @@ -132,4 +130,22 @@ class SafeHasSuperTypeMatcher public String toString() { return "safeHasSuperType(" + matcher + ")"; } + + @Override + public boolean equals(final Object other) { + if (this == other) { + return true; + } else if (other == null) { + return false; + } else if (getClass() != other.getClass()) { + return false; + } else { + return matcher.equals(((SafeHasSuperTypeMatcher) other).matcher); + } + } + + @Override + public int hashCode() { + return 17 * 31 + matcher.hashCode(); + } } diff --git a/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/bytebuddy/matcher/ExtendsClassMatcherTest.groovy b/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/bytebuddy/matcher/ExtendsClassMatcherTest.groovy index fff3572044..0e9f37f556 100644 --- a/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/bytebuddy/matcher/ExtendsClassMatcherTest.groovy +++ b/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/bytebuddy/matcher/ExtendsClassMatcherTest.groovy @@ -1,5 +1,6 @@ package datadog.trace.agent.tooling.bytebuddy.matcher +import datadog.trace.agent.tooling.AgentTooling import datadog.trace.agent.tooling.bytebuddy.matcher.testclasses.A import datadog.trace.agent.tooling.bytebuddy.matcher.testclasses.B import datadog.trace.agent.tooling.bytebuddy.matcher.testclasses.F @@ -7,11 +8,16 @@ import datadog.trace.agent.tooling.bytebuddy.matcher.testclasses.G import datadog.trace.util.test.DDSpecification import net.bytebuddy.description.type.TypeDescription import net.bytebuddy.jar.asm.Opcodes +import spock.lang.Shared import static datadog.trace.agent.tooling.bytebuddy.matcher.DDElementMatchers.extendsClass import static net.bytebuddy.matcher.ElementMatchers.named class ExtendsClassMatcherTest extends DDSpecification { + @Shared + def typePool = + AgentTooling.poolStrategy() + .typePool(AgentTooling.locationStrategy().classFileLocator(this.class.classLoader, null), this.class.classLoader) def "test matcher #matcherClass.simpleName -> #type.simpleName"() { expect: @@ -26,7 +32,7 @@ class ExtendsClassMatcherTest extends DDSpecification { F | G | true matcher = named(matcherClass.name) - argument = TypeDescription.ForLoadedType.of(type) + argument = typePool.describe(type.name).resolve() } def "test traversal exceptions"() { diff --git a/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/bytebuddy/matcher/HasInterfaceMatcherTest.groovy b/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/bytebuddy/matcher/HasInterfaceMatcherTest.groovy index 6825085adc..5ef308ed22 100644 --- a/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/bytebuddy/matcher/HasInterfaceMatcherTest.groovy +++ b/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/bytebuddy/matcher/HasInterfaceMatcherTest.groovy @@ -1,5 +1,6 @@ package datadog.trace.agent.tooling.bytebuddy.matcher +import datadog.trace.agent.tooling.AgentTooling import datadog.trace.agent.tooling.bytebuddy.matcher.testclasses.A import datadog.trace.agent.tooling.bytebuddy.matcher.testclasses.B import datadog.trace.agent.tooling.bytebuddy.matcher.testclasses.E @@ -8,12 +9,17 @@ import datadog.trace.agent.tooling.bytebuddy.matcher.testclasses.G import datadog.trace.util.test.DDSpecification import net.bytebuddy.description.type.TypeDescription import net.bytebuddy.jar.asm.Opcodes +import spock.lang.Shared import static datadog.trace.agent.tooling.bytebuddy.matcher.DDElementMatchers.hasInterface import static datadog.trace.agent.tooling.bytebuddy.matcher.DDElementMatchers.implementsInterface import static net.bytebuddy.matcher.ElementMatchers.named class HasInterfaceMatcherTest extends DDSpecification { + @Shared + def typePool = + AgentTooling.poolStrategy() + .typePool(AgentTooling.locationStrategy().classFileLocator(this.class.classLoader, null), this.class.classLoader) def "test matcher #matcherClass.simpleName -> #type.simpleName"() { expect: @@ -32,7 +38,7 @@ class HasInterfaceMatcherTest extends DDSpecification { F | G | false matcher = named(matcherClass.name) - argument = TypeDescription.ForLoadedType.of(type) + argument = typePool.describe(type.name).resolve() } def "test traversal exceptions"() { diff --git a/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/bytebuddy/matcher/ImplementsInterfaceMatcherTest.groovy b/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/bytebuddy/matcher/ImplementsInterfaceMatcherTest.groovy index 2decb1114e..efd65ed0b6 100644 --- a/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/bytebuddy/matcher/ImplementsInterfaceMatcherTest.groovy +++ b/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/bytebuddy/matcher/ImplementsInterfaceMatcherTest.groovy @@ -1,5 +1,6 @@ package datadog.trace.agent.tooling.bytebuddy.matcher +import datadog.trace.agent.tooling.AgentTooling import datadog.trace.agent.tooling.bytebuddy.matcher.testclasses.A import datadog.trace.agent.tooling.bytebuddy.matcher.testclasses.B import datadog.trace.agent.tooling.bytebuddy.matcher.testclasses.E @@ -8,11 +9,16 @@ import datadog.trace.agent.tooling.bytebuddy.matcher.testclasses.G import datadog.trace.util.test.DDSpecification import net.bytebuddy.description.type.TypeDescription import net.bytebuddy.jar.asm.Opcodes +import spock.lang.Shared import static datadog.trace.agent.tooling.bytebuddy.matcher.DDElementMatchers.implementsInterface import static net.bytebuddy.matcher.ElementMatchers.named class ImplementsInterfaceMatcherTest extends DDSpecification { + @Shared + def typePool = + AgentTooling.poolStrategy() + .typePool(AgentTooling.locationStrategy().classFileLocator(this.class.classLoader, null), this.class.classLoader) def "test matcher #matcherClass.simpleName -> #type.simpleName"() { expect: @@ -31,7 +37,7 @@ class ImplementsInterfaceMatcherTest extends DDSpecification { F | G | false matcher = named(matcherClass.name) - argument = TypeDescription.ForLoadedType.of(type) + argument = typePool.describe(type.name).resolve() } def "test traversal exceptions"() { diff --git a/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/bytebuddy/matcher/SafeHasSuperTypeMatcherTest.groovy b/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/bytebuddy/matcher/SafeHasSuperTypeMatcherTest.groovy index d64af5b951..9271e256bd 100644 --- a/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/bytebuddy/matcher/SafeHasSuperTypeMatcherTest.groovy +++ b/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/bytebuddy/matcher/SafeHasSuperTypeMatcherTest.groovy @@ -1,5 +1,6 @@ package datadog.trace.agent.tooling.bytebuddy.matcher +import datadog.trace.agent.tooling.AgentTooling import datadog.trace.agent.tooling.bytebuddy.matcher.testclasses.A import datadog.trace.agent.tooling.bytebuddy.matcher.testclasses.B import datadog.trace.agent.tooling.bytebuddy.matcher.testclasses.E @@ -8,11 +9,16 @@ import datadog.trace.agent.tooling.bytebuddy.matcher.testclasses.G import datadog.trace.util.test.DDSpecification import net.bytebuddy.description.type.TypeDescription import net.bytebuddy.jar.asm.Opcodes +import spock.lang.Shared import static datadog.trace.agent.tooling.bytebuddy.matcher.DDElementMatchers.safeHasSuperType import static net.bytebuddy.matcher.ElementMatchers.named class SafeHasSuperTypeMatcherTest extends DDSpecification { + @Shared + def typePool = + AgentTooling.poolStrategy() + .typePool(AgentTooling.locationStrategy().classFileLocator(this.class.classLoader, null), this.class.classLoader) def "test matcher #matcherClass.simpleName -> #type.simpleName"() { expect: @@ -31,7 +37,7 @@ class SafeHasSuperTypeMatcherTest extends DDSpecification { F | G | true matcher = named(matcherClass.name) - argument = TypeDescription.ForLoadedType.of(type) + argument = typePool.describe(type.name).resolve() } def "test traversal exceptions"() { diff --git a/dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/ServerErrorHandlerInstrumentation.java b/dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/ServerErrorHandlerInstrumentation.java index 0ddf2b0feb..57a18979ed 100644 --- a/dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/ServerErrorHandlerInstrumentation.java +++ b/dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/ServerErrorHandlerInstrumentation.java @@ -3,7 +3,6 @@ package datadog.trace.instrumentation.ratpack; import static datadog.trace.agent.tooling.bytebuddy.matcher.DDElementMatchers.implementsInterface; import static java.util.Collections.singletonMap; import static net.bytebuddy.matcher.ElementMatchers.isAbstract; -import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.not; import static net.bytebuddy.matcher.ElementMatchers.takesArgument; @@ -24,8 +23,7 @@ public class ServerErrorHandlerInstrumentation extends Instrumenter.Default { @Override public ElementMatcher typeMatcher() { - return not(isInterface().or(isAbstract())) - .and(implementsInterface(named("ratpack.error.ServerErrorHandler"))); + return not(isAbstract()).and(implementsInterface(named("ratpack.error.ServerErrorHandler"))); } @Override