Extract Diff interface and implement polymorphic serialization

nikita-tkachenko-datadog · nikita-tkachenko-datadog · commit 7d5881954f01 · 2025-01-20T17:07:58.000+01:00
diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/config/EarlyFlakeDetectionSettingsSerializer.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/config/EarlyFlakeDetectionSettingsSerializer.java
@@ -1,6 +1,6 @@
 package datadog.trace.civisibility.config;
 
-import datadog.trace.civisibility.ipc.Serializer;
+import datadog.trace.civisibility.ipc.serialization.Serializer;
 import java.nio.ByteBuffer;
 import java.util.List;
 
diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/config/ExecutionSettings.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/config/ExecutionSettings.java
@@ -2,8 +2,8 @@
 
 import datadog.trace.api.civisibility.config.TestIdentifier;
 import datadog.trace.api.civisibility.config.TestMetadata;
-import datadog.trace.civisibility.git.Diff;
-import datadog.trace.civisibility.ipc.Serializer;
+import datadog.trace.civisibility.diff.Diff;
+import datadog.trace.civisibility.ipc.serialization.Serializer;
 import java.nio.ByteBuffer;
 import java.util.BitSet;
 import java.util.Collection;
@@ -205,7 +205,7 @@ public static ByteBuffer serialize(ExecutionSettings settings) {
       s.write(settings.flakyTests, TestIdentifierSerializer::serialize);
       s.write(settings.knownTests, TestIdentifierSerializer::serialize);
 
-      settings.pullRequestDiff.serialize(s);
+      Diff.SERIALIZER.serialize(settings.pullRequestDiff, s);
 
       return s.flush();
     }
@@ -234,7 +234,7 @@ public static ExecutionSettings deserialize(ByteBuffer buffer) {
       Collection<TestIdentifier> knownTests =
           Serializer.readSet(buffer, TestIdentifierSerializer::deserialize);
 
-      Diff diff = Diff.deserialize(buffer);
+      Diff diff = Diff.SERIALIZER.deserialize(buffer);
 
       return new ExecutionSettings(
           itrEnabled,
diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/config/ExecutionSettingsFactoryImpl.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/config/ExecutionSettingsFactoryImpl.java
@@ -8,7 +8,7 @@
 import datadog.trace.api.git.GitInfo;
 import datadog.trace.api.git.GitInfoProvider;
 import datadog.trace.civisibility.ci.PullRequestInfo;
-import datadog.trace.civisibility.git.Diff;
+import datadog.trace.civisibility.diff.Diff;
 import datadog.trace.civisibility.git.tree.GitClient;
 import datadog.trace.civisibility.git.tree.GitDataUploader;
 import java.nio.file.Path;
@@ -304,8 +304,7 @@ private Diff getPullRequestDiff(boolean impactedTestsDetectionEnabled) {
     if (repositoryRoot == null || !impactedTestsDetectionEnabled) {
       return Diff.EMPTY;
     }
-    // FIXME nikita: add file-based granularity fallback if Git executable is not available (add
-    // telemetry for backend)?
+    // FIXME nikita: add file-based granularity fallback (+ telemetry)
     // FIXME nikita: add integration/smoke tests
     try {
       GitClient gitClient = gitClientFactory.create(repositoryRoot);
diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/config/JvmInfo.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/config/JvmInfo.java
@@ -2,7 +2,7 @@
 
 import datadog.trace.api.Config;
 import datadog.trace.api.civisibility.CiVisibilityWellKnownTags;
-import datadog.trace.civisibility.ipc.Serializer;
+import datadog.trace.civisibility.ipc.serialization.Serializer;
 import java.nio.ByteBuffer;
 import java.util.Objects;
 
diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/config/TestIdentifierSerializer.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/config/TestIdentifierSerializer.java
@@ -1,7 +1,7 @@
 package datadog.trace.civisibility.config;
 
 import datadog.trace.api.civisibility.config.TestIdentifier;
-import datadog.trace.civisibility.ipc.Serializer;
+import datadog.trace.civisibility.ipc.serialization.Serializer;
 import java.nio.ByteBuffer;
 
 public abstract class TestIdentifierSerializer {
diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/config/TestMetadataSerializer.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/config/TestMetadataSerializer.java
@@ -1,7 +1,7 @@
 package datadog.trace.civisibility.config;
 
 import datadog.trace.api.civisibility.config.TestMetadata;
-import datadog.trace.civisibility.ipc.Serializer;
+import datadog.trace.civisibility.ipc.serialization.Serializer;
 import java.nio.ByteBuffer;
 
 public abstract class TestMetadataSerializer {
diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/diff/Diff.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/diff/Diff.java
@@ -0,0 +1,13 @@
+package datadog.trace.civisibility.diff;
+
+import datadog.trace.civisibility.ipc.serialization.PolymorphicSerializer;
+import datadog.trace.civisibility.ipc.serialization.SerializableType;
+
+public interface Diff extends SerializableType {
+
+  Diff EMPTY = LineDiff.EMPTY;
+
+  PolymorphicSerializer<Diff> SERIALIZER = new PolymorphicSerializer<>(LineDiff.class);
+
+  boolean contains(String relativePath, int startLine, int endLine);
+}
diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/diff/LineDiff.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/diff/LineDiff.java
@@ -1,26 +1,28 @@
-package datadog.trace.civisibility.git;
+package datadog.trace.civisibility.diff;
 
-import datadog.trace.civisibility.ipc.Serializer;
+import datadog.trace.civisibility.ipc.serialization.Serializer;
 import java.nio.ByteBuffer;
 import java.util.BitSet;
 import java.util.Collections;
 import java.util.Map;
 import java.util.Objects;
 
-public class Diff {
+/** Diff data with per-line granularity. */
+public class LineDiff implements Diff {
 
-  public static final Diff EMPTY = new Diff(Collections.emptyMap());
+  public static final LineDiff EMPTY = new LineDiff(Collections.emptyMap());
 
   private final Map<String, BitSet> linesByRelativePath;
 
-  public Diff(Map<String, BitSet> linesByRelativePath) {
+  public LineDiff(Map<String, BitSet> linesByRelativePath) {
     this.linesByRelativePath = linesByRelativePath;
   }
 
   public Map<String, BitSet> getLinesByRelativePath() {
     return Collections.unmodifiableMap(linesByRelativePath);
   }
 
+  @Override
   public boolean contains(String relativePath, int startLine, int endLine) {
     BitSet lines = linesByRelativePath.get(relativePath);
     if (lines == null) {
@@ -31,14 +33,15 @@ public boolean contains(String relativePath, int startLine, int endLine) {
     return changedLine != -1 && changedLine <= endLine;
   }
 
+  @Override
   public void serialize(Serializer s) {
     s.write(linesByRelativePath, Serializer::write, Serializer::write);
   }
 
-  public static Diff deserialize(ByteBuffer buffer) {
+  public static LineDiff deserialize(ByteBuffer buffer) {
     Map<String, BitSet> linesByRelativePath =
         Serializer.readMap(buffer, Serializer::readString, Serializer::readBitSet);
-    return new Diff(linesByRelativePath);
+    return new LineDiff(linesByRelativePath);
   }
 
   @Override
@@ -49,7 +52,7 @@ public boolean equals(Object o) {
     if (o == null || getClass() != o.getClass()) {
       return false;
     }
-    Diff diff = (Diff) o;
+    LineDiff diff = (LineDiff) o;
     return Objects.equals(linesByRelativePath, diff.linesByRelativePath);
   }
 
@@ -60,6 +63,6 @@ public int hashCode() {
 
   @Override
   public String toString() {
-    return "Diff{" + "linesByRelativePath=" + linesByRelativePath + '}';
+    return "LineDiff{linesByRelativePath=" + linesByRelativePath + '}';
   }
 }
diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitClient.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitClient.java
@@ -7,7 +7,7 @@
 import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector;
 import datadog.trace.api.civisibility.telemetry.tag.Command;
 import datadog.trace.api.civisibility.telemetry.tag.ExitCode;
-import datadog.trace.civisibility.git.Diff;
+import datadog.trace.civisibility.diff.LineDiff;
 import datadog.trace.civisibility.utils.ShellCommandExecutor;
 import datadog.trace.util.Strings;
 import edu.umd.cs.findbugs.annotations.NonNull;
@@ -554,7 +554,7 @@ private Path createTempDirectory() throws IOException {
    * @throws InterruptedException If current thread was interrupted while waiting for Git command to
    *     finish
    */
-  public @NonNull Diff getGitDiff(String baseCommit, String targetCommit)
+  public @NonNull LineDiff getGitDiff(String baseCommit, String targetCommit)
       throws IOException, TimeoutException, InterruptedException {
     if (Strings.isNotBlank(baseCommit) && Strings.isNotBlank(targetCommit)) {
       return executeCommand(
@@ -573,7 +573,7 @@ private Path createTempDirectory() throws IOException {
           "Base commit and/or target commit info is not available, returning empty git diff: {}/{}",
           baseCommit,
           targetCommit);
-      return Diff.EMPTY;
+      return LineDiff.EMPTY;
     }
   }
 
diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitDiffParser.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitDiffParser.java
@@ -1,6 +1,6 @@
 package datadog.trace.civisibility.git.tree;
 
-import datadog.trace.civisibility.git.Diff;
+import datadog.trace.civisibility.diff.LineDiff;
 import edu.umd.cs.findbugs.annotations.NonNull;
 import java.io.BufferedReader;
 import java.io.IOException;
@@ -20,7 +20,7 @@ public class GitDiffParser {
   private static final Pattern CHANGED_LINES_PATTERN =
       Pattern.compile("^@@ -\\d+(,\\d+)? \\+(?<startline>\\d+)(,(?<count>\\d+))? @@");
 
-  public static @NonNull Diff parse(InputStream input) throws IOException {
+  public static @NonNull LineDiff parse(InputStream input) throws IOException {
     Map<String, BitSet> linesByRelativePath = new HashMap<>();
 
     BufferedReader bufferedReader =
@@ -59,6 +59,6 @@ public class GitDiffParser {
       linesByRelativePath.put(changedFile, changedLines);
     }
 
-    return new Diff(linesByRelativePath);
+    return new LineDiff(linesByRelativePath);
   }
 }
diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/ipc/ErrorResponse.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/ipc/ErrorResponse.java
@@ -1,5 +1,6 @@
 package datadog.trace.civisibility.ipc;
 
+import datadog.trace.civisibility.ipc.serialization.Serializer;
 import java.nio.ByteBuffer;
 
 public class ErrorResponse implements SignalResponse {
diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/ipc/ExecutionSettingsRequest.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/ipc/ExecutionSettingsRequest.java
@@ -1,6 +1,7 @@
 package datadog.trace.civisibility.ipc;
 
 import datadog.trace.civisibility.config.JvmInfo;
+import datadog.trace.civisibility.ipc.serialization.Serializer;
 import java.nio.ByteBuffer;
 import java.util.Objects;
 
diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/ipc/ModuleCoverageDataJacoco.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/ipc/ModuleCoverageDataJacoco.java
@@ -1,6 +1,7 @@
 package datadog.trace.civisibility.ipc;
 
 import datadog.trace.api.DDTraceId;
+import datadog.trace.civisibility.ipc.serialization.Serializer;
 import java.nio.ByteBuffer;
 import java.util.Arrays;
 import java.util.Objects;
diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/ipc/ModuleExecutionResult.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/ipc/ModuleExecutionResult.java
@@ -1,6 +1,7 @@
 package datadog.trace.civisibility.ipc;
 
 import datadog.trace.api.DDTraceId;
+import datadog.trace.civisibility.ipc.serialization.Serializer;
 import java.nio.ByteBuffer;
 import java.util.Collection;
 import java.util.Objects;
diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/ipc/TestFramework.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/ipc/TestFramework.java
@@ -1,5 +1,6 @@
 package datadog.trace.civisibility.ipc;
 
+import datadog.trace.civisibility.ipc.serialization.Serializer;
 import java.nio.ByteBuffer;
 import java.util.Objects;
 
diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/ipc/serialization/PolymorphicSerializer.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/ipc/serialization/PolymorphicSerializer.java
@@ -0,0 +1,82 @@
+package datadog.trace.civisibility.ipc.serialization;
+
+import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
+import java.nio.ByteBuffer;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.function.Function;
+
+public class PolymorphicSerializer<ParentType extends SerializableType> {
+  private final Map<Byte, Function<ByteBuffer, ? extends ParentType>> deserializers =
+      new HashMap<>();
+  private final Map<Class<? extends ParentType>, Byte> ids = new HashMap<>();
+
+  @SafeVarargs
+  public PolymorphicSerializer(Class<? extends ParentType>... types) {
+    for (Class<? extends ParentType> type : types) {
+      register(type);
+    }
+  }
+
+  private <T extends ParentType> void register(Class<T> type) {
+    byte id = (byte) (ids.size() + 1);
+    Function<ByteBuffer, T> deserializer = findDeserializer(type);
+    deserializers.put(id, deserializer);
+    ids.put(type, id);
+  }
+
+  private <T extends ParentType> Function<ByteBuffer, T> findDeserializer(Class<T> type) {
+    for (Method method : type.getDeclaredMethods()) {
+      boolean isStatic = (method.getModifiers() & Modifier.STATIC) != 0;
+      Class<?>[] parameterTypes = method.getParameterTypes();
+      if (isStatic
+          && method.getReturnType() == type
+          && parameterTypes.length == 1
+          && parameterTypes[0] == ByteBuffer.class) {
+        return toDeserializer(method);
+      }
+    }
+    throw new IllegalArgumentException(
+        "Could not find a static method that accepts ByteBuffer and returns "
+            + type.getName()
+            + "in "
+            + type.getName());
+  }
+
+  @SuppressWarnings("unchecked")
+  private <T extends ParentType> Function<ByteBuffer, T> toDeserializer(Method method) {
+    return bb -> {
+      try {
+        return (T) method.invoke(null, bb);
+      } catch (Exception e) {
+        throw new RuntimeException(e);
+      }
+    };
+  }
+
+  public void serialize(ParentType o, Serializer s) {
+    if (o == null) {
+      s.write(0);
+      return;
+    }
+    Byte id = ids.get(o.getClass());
+    if (id == null) {
+      throw new IllegalArgumentException("Unknown type: " + o.getClass().getName());
+    }
+    s.write(id);
+    o.serialize(s);
+  }
+
+  public ParentType deserialize(ByteBuffer buffer) {
+    byte id = Serializer.readByte(buffer);
+    if (id == 0) {
+      return null;
+    }
+    Function<ByteBuffer, ? extends ParentType> deserializer = deserializers.get(id);
+    if (deserializer == null) {
+      throw new IllegalArgumentException("Unknown type ID: " + id);
+    }
+    return deserializer.apply(buffer);
+  }
+}
diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/ipc/serialization/SerializableType.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/ipc/serialization/SerializableType.java
@@ -0,0 +1,6 @@
+package datadog.trace.civisibility.ipc.serialization;
+
+public interface SerializableType {
+
+  void serialize(Serializer s);
+}
diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/ipc/serialization/Serializer.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/ipc/serialization/Serializer.java
@@ -1,4 +1,4 @@
-package datadog.trace.civisibility.ipc;
+package datadog.trace.civisibility.ipc.serialization;
 
 import java.io.ByteArrayOutputStream;
 import java.nio.ByteBuffer;
diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/source/index/RepoIndex.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/source/index/RepoIndex.java
@@ -2,7 +2,7 @@
 
 import datadog.trace.api.Config;
 import datadog.trace.api.civisibility.domain.Language;
-import datadog.trace.civisibility.ipc.Serializer;
+import datadog.trace.civisibility.ipc.serialization.Serializer;
 import datadog.trace.civisibility.source.SourceResolutionException;
 import datadog.trace.civisibility.source.Utils;
 import datadog.trace.util.ClassNameTrie;
diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/config/EarlyFlakeDetectionSettingsSerializerTest.groovy b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/config/EarlyFlakeDetectionSettingsSerializerTest.groovy
@@ -1,7 +1,7 @@
 package datadog.trace.civisibility.config
 
 
-import datadog.trace.civisibility.ipc.Serializer
+import datadog.trace.civisibility.ipc.serialization.Serializer
 import spock.lang.Specification
 
 class EarlyFlakeDetectionSettingsSerializerTest extends Specification {
diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/config/ExecutionSettingsTest.groovy b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/config/ExecutionSettingsTest.groovy
diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/config/TestIdentifierSerializerTest.groovy b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/config/TestIdentifierSerializerTest.groovy
diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/diff/LineDiffTest.groovy b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/diff/LineDiffTest.groovy
diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/ipc/serialization/PolymorphicSerializerTest.groovy b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/ipc/serialization/PolymorphicSerializerTest.groovy
diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/ipc/serialization/SerializerTest.groovy b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/ipc/serialization/SerializerTest.groovy
diff --git a/dd-java-agent/agent-ci-visibility/src/testFixtures/groovy/datadog/trace/civisibility/CiVisibilityInstrumentationTest.groovy b/dd-java-agent/agent-ci-visibility/src/testFixtures/groovy/datadog/trace/civisibility/CiVisibilityInstrumentationTest.groovy

Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`package datadog.trace.civisibility.git.tree;`
`2`	`2`
`3`		`-import datadog.trace.civisibility.git.Diff;`
	`3`	`+import datadog.trace.civisibility.diff.LineDiff;`
`4`	`4`	`import edu.umd.cs.findbugs.annotations.NonNull;`
`5`	`5`	`import java.io.BufferedReader;`
`6`	`6`	`import java.io.IOException;`
`@@ -20,7 +20,7 @@ public class GitDiffParser {`
`20`	`20`	`private static final Pattern CHANGED_LINES_PATTERN =`
`21`	`21`	`Pattern.compile("^@@ -\\d+(,\\d+)? \\+(?<startline>\\d+)(,(?<count>\\d+))? @@");`
`22`	`22`
`23`		`- public static @NonNull Diff parse(InputStream input) throws IOException {`
	`23`	`+ public static @NonNull LineDiff parse(InputStream input) throws IOException {`
`24`	`24`	`Map<String, BitSet> linesByRelativePath = new HashMap<>();`
`25`	`25`
`26`	`26`	`BufferedReader bufferedReader =`
`@@ -59,6 +59,6 @@ public class GitDiffParser {`
`59`	`59`	`linesByRelativePath.put(changedFile, changedLines);`
`60`	`60`	`}`
`61`	`61`
`62`		`- return new Diff(linesByRelativePath);`
	`62`	`+ return new LineDiff(linesByRelativePath);`
`63`	`63`	`}`
`64`	`64`	`}`