From a9f037dae1845ba7b39603848f98ae2372ee3abe Mon Sep 17 00:00:00 2001 From: Kusal Kithul-Godage Date: Wed, 2 Oct 2024 19:29:46 +1000 Subject: [PATCH] Refactor HashGenerator to support default hashing implementation --- .../javaagent/collecting/MethodRegistry.java | 4 +- .../scavenger/javaagent/model/Method.java | 4 +- .../scavenger/javaagent/model/MethodTest.java | 6 +-- .../navercorp/scavenger/model/Publication.kt | 6 +-- .../scavenger/model/PublicationTest.kt | 20 +++---- .../scavenger/util/HashGenerator.java | 53 +++++++++++-------- 6 files changed, 50 insertions(+), 43 deletions(-) diff --git a/scavenger-agent-java/src/main/java/com/navercorp/scavenger/javaagent/collecting/MethodRegistry.java b/scavenger-agent-java/src/main/java/com/navercorp/scavenger/javaagent/collecting/MethodRegistry.java index 64d9aec9..0be8e8de 100644 --- a/scavenger-agent-java/src/main/java/com/navercorp/scavenger/javaagent/collecting/MethodRegistry.java +++ b/scavenger-agent-java/src/main/java/com/navercorp/scavenger/javaagent/collecting/MethodRegistry.java @@ -6,7 +6,7 @@ import lombok.Getter; import lombok.extern.java.Log; -import com.navercorp.scavenger.util.HashGenerator; +import com.navercorp.scavenger.util.HashGenerator.DefaultHash; @Log public class MethodRegistry { @@ -28,7 +28,7 @@ private String generateHash(String byteBuddySignature) { if (isLegacyCompatibilityMode) { signature = signature.replace('$', '.').replace(",", ", "); } - return HashGenerator.Md5.from(signature); + return DefaultHash.from(signature); } static String extractSignature(String byteBuddySignature) { diff --git a/scavenger-agent-java/src/main/java/com/navercorp/scavenger/javaagent/model/Method.java b/scavenger-agent-java/src/main/java/com/navercorp/scavenger/javaagent/model/Method.java index 086d5608..54960664 100644 --- a/scavenger-agent-java/src/main/java/com/navercorp/scavenger/javaagent/model/Method.java +++ b/scavenger-agent-java/src/main/java/com/navercorp/scavenger/javaagent/model/Method.java @@ -14,7 +14,7 @@ import lombok.Value; import com.navercorp.scavenger.model.CodeBasePublication; -import com.navercorp.scavenger.util.HashGenerator; +import com.navercorp.scavenger.util.HashGenerator.DefaultHash; @Value public class Method { @@ -111,7 +111,7 @@ public CodeBasePublication.CodeBaseEntry toCodeBaseEntry() { .setModifiers(defaultIfNull(this.modifiers, "")) .setPackageName(defaultIfNull(packageName, "")) .setParameterTypes(defaultIfNull(this.parameterTypes, "")) - .setSignatureHash(HashGenerator.Md5.from(signature)) + .setSignatureHash(DefaultHash.from(signature)) .build(); } diff --git a/scavenger-agent-java/src/test/java/com/navercorp/scavenger/javaagent/model/MethodTest.java b/scavenger-agent-java/src/test/java/com/navercorp/scavenger/javaagent/model/MethodTest.java index 01e1711a..989820ab 100644 --- a/scavenger-agent-java/src/test/java/com/navercorp/scavenger/javaagent/model/MethodTest.java +++ b/scavenger-agent-java/src/test/java/com/navercorp/scavenger/javaagent/model/MethodTest.java @@ -4,7 +4,7 @@ import org.junit.jupiter.api.Test; -import com.navercorp.scavenger.util.HashGenerator; +import com.navercorp.scavenger.util.HashGenerator.DefaultHash; class MethodTest { @Test @@ -19,7 +19,7 @@ void testMethodCreation() { assertThat(it.getModifiers()).isEqualTo("static"); assertThat(it.getPackageName()).isEqualTo("packageName"); assertThat(it.getSignature()).isEqualTo("name(int a, int wow)"); - assertThat(it.getSignatureHash()).isEqualTo(HashGenerator.Md5.from("name(int a, int wow)")); + assertThat(it.getSignatureHash()).isEqualTo(DefaultHash.from("name(int a, int wow)")); }); Method method2 = new Method(null, Visibility.PRIVATE, null, false, false, "", "int a,int wow", null); @@ -30,7 +30,7 @@ void testMethodCreation() { assertThat(it.getModifiers()).isEqualTo(""); assertThat(it.getPackageName()).isEqualTo(""); assertThat(it.getSignature()).isEqualTo(""); - assertThat(it.getSignatureHash()).isEqualTo(HashGenerator.Md5.from("")); + assertThat(it.getSignatureHash()).isEqualTo(DefaultHash.from("")); }); } diff --git a/scavenger-collector/src/main/kotlin/com/navercorp/scavenger/model/Publication.kt b/scavenger-collector/src/main/kotlin/com/navercorp/scavenger/model/Publication.kt index 87e5969f..3e724151 100644 --- a/scavenger-collector/src/main/kotlin/com/navercorp/scavenger/model/Publication.kt +++ b/scavenger-collector/src/main/kotlin/com/navercorp/scavenger/model/Publication.kt @@ -5,7 +5,7 @@ import com.navercorp.scavenger.dto.CommonImportDto import com.navercorp.scavenger.dto.CommonImportResultDto import com.navercorp.scavenger.dto.InvocationImportDto import com.navercorp.scavenger.exception.UnknownPublicationException -import com.navercorp.scavenger.util.HashGenerator +import com.navercorp.scavenger.util.HashGenerator.DefaultHash import io.codekvast.javaagent.model.v4.CodeBasePublication4 import io.codekvast.javaagent.model.v4.CommonPublicationData4 import io.codekvast.javaagent.model.v4.InvocationDataPublication4 @@ -117,7 +117,7 @@ sealed class LegacyPublication private constructor(val commonData: CommonPublica modifiers = it.methodSignature.modifiers, packageName = it.methodSignature.packageName, parameterTypes = it.methodSignature.parameterTypes, - signatureHash = HashGenerator.Md5.from(it.signature) + signatureHash = DefaultHash.from(it.signature) ) }.sortedBy { it.signatureHash } @@ -141,7 +141,7 @@ sealed class LegacyPublication private constructor(val commonData: CommonPublica environmentId = environmentId, invocations = pub.invocations .filterNot { syntheticSignaturePattern.matcher(it).matches() } - .map { HashGenerator.Md5.from(it) } + .map { DefaultHash.from(it) } .sorted(), invokedAtMillis = pub.recordingIntervalStartedAtMillis, ) diff --git a/scavenger-collector/src/test/kotlin/com/navercorp/scavenger/model/PublicationTest.kt b/scavenger-collector/src/test/kotlin/com/navercorp/scavenger/model/PublicationTest.kt index 8e9cd21a..1c9c75d5 100644 --- a/scavenger-collector/src/test/kotlin/com/navercorp/scavenger/model/PublicationTest.kt +++ b/scavenger-collector/src/test/kotlin/com/navercorp/scavenger/model/PublicationTest.kt @@ -2,7 +2,7 @@ package com.navercorp.scavenger.model import com.navercorp.scavenger.dto.CommonImportResultDto import com.navercorp.scavenger.model.CodeBasePublication.CodeBaseEntry -import com.navercorp.scavenger.util.HashGenerator.Md5 +import com.navercorp.scavenger.util.HashGenerator.DefaultHash import com.navercorp.scavenger.util.SamplePublications import io.codekvast.javaagent.model.v4.CodeBaseEntry4 import io.codekvast.javaagent.model.v4.CodeBasePublication4 @@ -72,14 +72,14 @@ class PublicationTest { ).getCodeBaseImportDto(commonImportResultDto).entries ).hasSize(2) .extracting("signatureHash") - .isEqualTo(listOf(Md5.from(targetSignature), Md5.from(CodeBaseEntry4.sampleCodeBaseEntry().signature)).sorted()) + .isEqualTo(listOf(DefaultHash.from(targetSignature), DefaultHash.from(CodeBaseEntry4.sampleCodeBaseEntry().signature)).sorted()) } } @Nested @DisplayName("if invocation data contains SpringCGLIB generated method") inner class Invocation { - private val cglibHash = Md5.from(cglibSignature) + private val cglibHash = DefaultHash.from(cglibSignature) @Test @DisplayName("it ignores it") @@ -117,7 +117,7 @@ class PublicationTest { ) .build() ).getInvocationImportDto(commonImportResultDto).invocations - ).isEqualTo(listOf(Md5.from(targetSignature), Md5.from("signature()")).sorted()) + ).isEqualTo(listOf(DefaultHash.from(targetSignature), DefaultHash.from("signature()")).sorted()) } } } @@ -129,10 +129,10 @@ class PublicationTest { private val codeBaseEntries = listOf( CodeBaseEntry.newBuilder() - .setSignatureHash((Md5.from("TestClass.method()"))) + .setSignatureHash(DefaultHash.from("TestClass.method()")) .build(), CodeBaseEntry.newBuilder() - .setSignatureHash((Md5.from("signature()"))) + .setSignatureHash(DefaultHash.from("signature()")) .build() ) @@ -149,7 +149,7 @@ class PublicationTest { .getCodeBaseImportDto(commonImportResultDto).entries ) .extracting("signatureHash") - .isEqualTo(listOf(Md5.from("TestClass.method()"), Md5.from("signature()")).sorted()) + .isEqualTo(listOf(DefaultHash.from("TestClass.method()"), DefaultHash.from("signature()")).sorted()) } @Test @@ -161,17 +161,17 @@ class PublicationTest { .setCommonData(SamplePublications.commonPublicationData) .addEntry( InvocationDataPublication.InvocationDataEntry.newBuilder() - .setHash(Md5.from("signature()")) + .setHash(DefaultHash.from("signature()")) ) .addEntry( InvocationDataPublication.InvocationDataEntry.newBuilder() - .setHash(Md5.from("TestClass.method()")) + .setHash(DefaultHash.from("TestClass.method()")) ) .setRecordingIntervalStartedAtMillis(0) .build() ).getInvocationImportDto(commonImportResultDto).invocations ) - .isEqualTo(listOf(Md5.from("TestClass.method()"), Md5.from("signature()")).sorted()) + .isEqualTo(listOf(DefaultHash.from("TestClass.method()"), DefaultHash.from("signature()")).sorted()) } } } diff --git a/scavenger-model/src/main/java/com/navercorp/scavenger/util/HashGenerator.java b/scavenger-model/src/main/java/com/navercorp/scavenger/util/HashGenerator.java index f98e4f38..5c01d239 100644 --- a/scavenger-model/src/main/java/com/navercorp/scavenger/util/HashGenerator.java +++ b/scavenger-model/src/main/java/com/navercorp/scavenger/util/HashGenerator.java @@ -3,35 +3,42 @@ import java.math.BigInteger; import java.nio.charset.StandardCharsets; import java.security.MessageDigest; -import java.security.NoSuchAlgorithmException; +import java.util.concurrent.Callable; public class HashGenerator { - public static class Sha256 { + + public static class DefaultHash { public static String from(String signature) { - try { - MessageDigest md = MessageDigest.getInstance("SHA-256"); - md.update(signature.getBytes(StandardCharsets.UTF_8)); - return String.format("%x", new BigInteger(1, md.digest())); - } catch (NoSuchAlgorithmException ignore) { - // ignore - return null; - } + return Md5.from(signature); } } - public static class Md5 { - public static String from(String signature) { - try { - if (signature == null) { - return null; - } - MessageDigest md = MessageDigest.getInstance("MD5"); - md.update(signature.getBytes(StandardCharsets.UTF_8)); - return String.format("%x", new BigInteger(1, md.digest())); - } catch (NoSuchAlgorithmException ignore) { - // ignore - return null; - } + private static class Sha256 { + private static String from(String signature) { + MessageDigest md = callWithCheckedExceptionWrapping(() -> MessageDigest.getInstance("SHA-256")); + md.update(signature.getBytes(StandardCharsets.UTF_8)); + return String.format("%x", new BigInteger(1, md.digest())); + } + } + + private static class Md5 { + private static String from(String signature) { + MessageDigest md = callWithCheckedExceptionWrapping(() -> MessageDigest.getInstance("MD5")); + md.update(signature.getBytes(StandardCharsets.UTF_8)); + return String.format("%x", new BigInteger(1, md.digest())); + } + } + + /** + * Utility method to call a {@link Callable} and rethrow any exceptions as unchecked. + */ + private static T callWithCheckedExceptionWrapping(Callable callable) { + try { + return callable.call(); + } catch (RuntimeException e) { + throw e; + } catch (Exception e) { + throw new RuntimeException(e); } } }