Merge branch 'master' into sezen.leblay/APPSEC-57259-extract-schema-spring

Author: sezen-datadog

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/ci/GithubActionsInfo.java

@@ -94,7 +94,7 @@ public CIInfo buildCIInfo() {
   @Override
   public PullRequestInfo buildPullRequestInfo() {
     String baseRef = environment.get(GITHUB_BASE_REF);
-    if (!Strings.isNotBlank(baseRef)) {
+    if (Strings.isBlank(baseRef)) {
       return PullRequestInfo.EMPTY;
     }
 

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/config/ChangedFiles.java

@@ -4,6 +4,7 @@
 import java.nio.file.Path;
 import java.util.Map;
 import java.util.Objects;
+import javax.annotation.Nullable;
 
 public class CiVisibilitySettings {
 
@@ -17,7 +18,8 @@ public class CiVisibilitySettings {
           false,
           false,
           EarlyFlakeDetectionSettings.DEFAULT,
-          TestManagementSettings.DEFAULT);
+          TestManagementSettings.DEFAULT,
+          null);
 
   private final boolean itrEnabled;
   private final boolean codeCoverage;
@@ -28,6 +30,7 @@ public class CiVisibilitySettings {
   private final boolean knownTestsEnabled;
   private final EarlyFlakeDetectionSettings earlyFlakeDetectionSettings;
   private final TestManagementSettings testManagementSettings;
+  @Nullable private final String defaultBranch;
 
   CiVisibilitySettings(
       boolean itrEnabled,
@@ -38,7 +41,8 @@ public class CiVisibilitySettings {
       boolean impactedTestsDetectionEnabled,
       boolean knownTestsEnabled,
       EarlyFlakeDetectionSettings earlyFlakeDetectionSettings,
-      TestManagementSettings testManagementSettings) {
+      TestManagementSettings testManagementSettings,
+      @Nullable String defaultBranch) {
     this.itrEnabled = itrEnabled;
     this.codeCoverage = codeCoverage;
     this.testsSkipping = testsSkipping;
@@ -48,6 +52,7 @@ public class CiVisibilitySettings {
     this.knownTestsEnabled = knownTestsEnabled;
     this.earlyFlakeDetectionSettings = earlyFlakeDetectionSettings;
     this.testManagementSettings = testManagementSettings;
+    this.defaultBranch = defaultBranch;
   }
 
   public boolean isItrEnabled() {
@@ -86,6 +91,11 @@ public TestManagementSettings getTestManagementSettings() {
     return testManagementSettings;
   }
 
+  @Nullable
+  public String getDefaultBranch() {
+    return defaultBranch;
+  }
+
   @Override
   public boolean equals(Object o) {
     if (this == o) {
@@ -103,7 +113,8 @@ public boolean equals(Object o) {
         && impactedTestsDetectionEnabled == that.impactedTestsDetectionEnabled
         && knownTestsEnabled == that.knownTestsEnabled
         && Objects.equals(earlyFlakeDetectionSettings, that.earlyFlakeDetectionSettings)
-        && Objects.equals(testManagementSettings, that.testManagementSettings);
+        && Objects.equals(testManagementSettings, that.testManagementSettings)
+        && Objects.equals(defaultBranch, that.defaultBranch);
   }
 
   @Override
@@ -117,7 +128,8 @@ public int hashCode() {
         impactedTestsDetectionEnabled,
         knownTestsEnabled,
         earlyFlakeDetectionSettings,
-        testManagementSettings);
+        testManagementSettings,
+        defaultBranch);
   }
 
   public interface Factory {
@@ -145,13 +157,20 @@ public CiVisibilitySettings fromJson(Map<String, Object> json) {
           EarlyFlakeDetectionSettings.JsonAdapter.INSTANCE.fromJson(
               (Map<String, Object>) json.get("early_flake_detection")),
           TestManagementSettings.JsonAdapter.INSTANCE.fromJson(
-              (Map<String, Object>) json.get("test_management")));
+              (Map<String, Object>) json.get("test_management")),
+          getString(json, "default_branch", null));
     }
 
     private static boolean getBoolean(
         Map<String, Object> json, String fieldName, boolean defaultValue) {
       Object value = json.get(fieldName);
       return value instanceof Boolean ? (Boolean) value : defaultValue;
     }
+
+    private static String getString(
+        Map<String, Object> json, String fieldName, String defaultValue) {
+      Object value = json.get(fieldName);
+      return value instanceof String ? (String) value : defaultValue;
+    }
   }
 }

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/config/CiVisibilitySettings.java

@@ -38,11 +38,6 @@ public Map<TestSetting, Map<String, Collection<TestFQN>>> getTestManagementTests
             TracerEnvironment tracerEnvironment) {
           return Collections.emptyMap();
         }
-
-        @Override
-        public ChangedFiles getChangedFiles(TracerEnvironment tracerEnvironment) {
-          return ChangedFiles.EMPTY;
-        }
       };
 
   CiVisibilitySettings getSettings(TracerEnvironment tracerEnvironment) throws IOException;
@@ -58,6 +53,4 @@ Map<String, Collection<TestFQN>> getKnownTestsByModule(TracerEnvironment tracerE
 
   Map<TestSetting, Map<String, Collection<TestFQN>>> getTestManagementTestsByModule(
       TracerEnvironment tracerEnvironment) throws IOException;
-
-  ChangedFiles getChangedFiles(TracerEnvironment tracerEnvironment) throws IOException;
 }

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/config/ConfigurationApi.java

@@ -53,7 +53,6 @@ public class ConfigurationApiImpl implements ConfigurationApi {
 
   private static final String SETTINGS_URI = "libraries/tests/services/setting";
   private static final String SKIPPABLE_TESTS_URI = "ci/tests/skippable";
-  private static final String CHANGED_FILES_URI = "ci/tests/diffs";
   private static final String FLAKY_TESTS_URI = "ci/libraries/tests/flaky";
   private static final String KNOWN_TESTS_URI = "ci/libraries/tests";
   private static final String TEST_MANAGEMENT_TESTS_URI = "test/libraries/test-management/tests";
@@ -68,7 +67,6 @@ public class ConfigurationApiImpl implements ConfigurationApi {
   private final JsonAdapter<EnvelopeDto<KnownTestsDto>> testFullNamesResponseAdapter;
   private final JsonAdapter<EnvelopeDto<TestManagementDto>> testManagementRequestAdapter;
   private final JsonAdapter<EnvelopeDto<TestManagementTestsDto>> testManagementTestsResponseAdapter;
-  private final JsonAdapter<EnvelopeDto<ChangedFiles>> changedFilesResponseAdapter;
 
   public ConfigurationApiImpl(BackendApi backendApi, CiVisibilityMetricCollector metricCollector) {
     this(backendApi, metricCollector, () -> RandomUtils.randomUUID().toString());
@@ -119,11 +117,6 @@ public ConfigurationApiImpl(BackendApi backendApi, CiVisibilityMetricCollector m
         Types.newParameterizedTypeWithOwner(
             ConfigurationApiImpl.class, EnvelopeDto.class, TestManagementTestsDto.class);
     testManagementTestsResponseAdapter = moshi.adapter(testManagementTestsResponseType);
-
-    ParameterizedType changedFilesResponseAdapterType =
-        Types.newParameterizedTypeWithOwner(
-            ConfigurationApiImpl.class, EnvelopeDto.class, ChangedFiles.class);
-    changedFilesResponseAdapter = moshi.adapter(changedFilesResponseAdapterType);
   }
 
   @Override
@@ -416,37 +409,6 @@ private Map<TestSetting, Map<String, Collection<TestFQN>>> parseTestManagementTe
     return testsByTypeByModule;
   }
 
-  @Override
-  public ChangedFiles getChangedFiles(TracerEnvironment tracerEnvironment) throws IOException {
-    OkHttpUtils.CustomListener telemetryListener =
-        new TelemetryListener.Builder(metricCollector)
-            .requestCount(CiVisibilityCountMetric.IMPACTED_TESTS_DETECTION_REQUEST)
-            .requestErrors(CiVisibilityCountMetric.IMPACTED_TESTS_DETECTION_REQUEST_ERRORS)
-            .requestDuration(CiVisibilityDistributionMetric.IMPACTED_TESTS_DETECTION_REQUEST_MS)
-            .responseBytes(CiVisibilityDistributionMetric.IMPACTED_TESTS_DETECTION_RESPONSE_BYTES)
-            .build();
-
-    String uuid = uuidGenerator.get();
-    EnvelopeDto<TracerEnvironment> request =
-        new EnvelopeDto<>(new DataDto<>(uuid, "ci_app_tests_diffs_request", tracerEnvironment));
-    String json = requestAdapter.toJson(request);
-    RequestBody requestBody = RequestBody.create(JSON, json);
-    ChangedFiles changedFiles =
-        backendApi.post(
-            CHANGED_FILES_URI,
-            requestBody,
-            is ->
-                changedFilesResponseAdapter.fromJson(Okio.buffer(Okio.source(is))).data.attributes,
-            telemetryListener,
-            false);
-
-    int filesCount = changedFiles.getFiles().size();
-    LOGGER.debug("Received {} changed files", filesCount);
-    metricCollector.add(
-        CiVisibilityDistributionMetric.IMPACTED_TESTS_DETECTION_RESPONSE_FILES, filesCount);
-    return changedFiles;
-  }
-
   private static final class EnvelopeDto<T> {
     private final DataDto<T> data;
 

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/config/ConfigurationApiImpl.java

@@ -8,7 +8,6 @@
 import datadog.trace.api.git.GitInfoProvider;
 import datadog.trace.civisibility.ci.PullRequestInfo;
 import datadog.trace.civisibility.diff.Diff;
-import datadog.trace.civisibility.diff.FileDiff;
 import datadog.trace.civisibility.diff.LineDiff;
 import datadog.trace.civisibility.git.tree.GitClient;
 import datadog.trace.civisibility.git.tree.GitDataUploader;
@@ -215,7 +214,8 @@ private Map<String, ExecutionSettings> doCreate(
                 getTestManagementTestsByModule(
                     tracerEnvironment, testManagementSettings.isEnabled()));
     Future<Diff> pullRequestDiffFuture =
-        executor.submit(() -> getPullRequestDiff(tracerEnvironment, impactedTestsEnabled));
+        executor.submit(
+            () -> getPullRequestDiff(impactedTestsEnabled, settings.getDefaultBranch()));
 
     SkippableTests skippableTests = skippableTestsFuture.get();
     Map<String, Collection<TestFQN>> flakyTestsByModule = flakyTestsFuture.get();
@@ -404,8 +404,7 @@ private Map<TestSetting, Map<String, Collection<TestFQN>>> getTestManagementTest
   }
 
   @Nonnull
-  private Diff getPullRequestDiff(
-      TracerEnvironment tracerEnvironment, boolean impactedTestsDetectionEnabled) {
+  private Diff getPullRequestDiff(boolean impactedTestsDetectionEnabled, String defaultBranch) {
     if (!impactedTestsDetectionEnabled) {
       return LineDiff.EMPTY;
     }
@@ -414,49 +413,25 @@ private Diff getPullRequestDiff(
       if (repositoryRoot != null) {
         // ensure repo is not shallow before attempting to get git diff
         gitRepoUnshallow.unshallow();
-        Diff diff =
-            gitClient.getGitDiff(
-                pullRequestInfo.getPullRequestBaseBranchSha(),
-                pullRequestInfo.getGitCommitHeadSha());
+
+        String baseCommitSha = pullRequestInfo.getPullRequestBaseBranchSha();
+        if (baseCommitSha == null) {
+          baseCommitSha =
+              gitClient.getBaseCommitSha(pullRequestInfo.getPullRequestBaseBranch(), defaultBranch);
+        }
+
+        Diff diff = gitClient.getGitDiff(baseCommitSha, pullRequestInfo.getGitCommitHeadSha());
         if (diff != null) {
           return diff;
         }
       }
-
     } catch (InterruptedException e) {
       LOGGER.error("Interrupted while getting git diff for PR: {}", pullRequestInfo, e);
       Thread.currentThread().interrupt();
-
     } catch (Exception e) {
       LOGGER.error("Could not get git diff for PR: {}", pullRequestInfo, e);
     }
 
-    if (config.isCiVisibilityImpactedTestsBackendRequestEnabled()) {
-      try {
-        ChangedFiles changedFiles = configurationApi.getChangedFiles(tracerEnvironment);
-
-        // attempting to use base SHA returned by the backend to calculate git diff
-        if (repositoryRoot != null) {
-          // ensure repo is not shallow before attempting to get git diff
-          gitRepoUnshallow.unshallow();
-          Diff diff = gitClient.getGitDiff(changedFiles.getBaseSha(), tracerEnvironment.getSha());
-          if (diff != null) {
-            return diff;
-          }
-        }
-
-        // falling back to file-level granularity
-        return new FileDiff(changedFiles.getFiles());
-
-      } catch (InterruptedException e) {
-        LOGGER.error("Interrupted while getting git diff for: {}", tracerEnvironment, e);
-        Thread.currentThread().interrupt();
-
-      } catch (Exception e) {
-        LOGGER.error("Could not get git diff for: {}", tracerEnvironment, e);
-      }
-    }
-
     return LineDiff.EMPTY;
   }
 

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/config/ExecutionSettingsFactoryImpl.java

@@ -5,8 +5,7 @@
 
 public interface Diff extends SerializableType {
 
-  PolymorphicSerializer<Diff> SERIALIZER =
-      new PolymorphicSerializer<>(LineDiff.class, FileDiff.class);
+  PolymorphicSerializer<Diff> SERIALIZER = new PolymorphicSerializer<>(LineDiff.class);
 
   boolean contains(String relativePath, int startLine, int endLine);
 }

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/diff/Diff.java

@@ -72,6 +72,10 @@ List<String> getObjects(Collection<String> commitsToSkip, Collection<String> com
   Path createPackFiles(List<String> objectHashes)
       throws IOException, TimeoutException, InterruptedException;
 
+  @Nullable
+  String getBaseCommitSha(@Nullable String baseBranch, @Nullable String defaultBranch)
+      throws IOException, TimeoutException, InterruptedException;
+
   @Nullable
   LineDiff getGitDiff(String baseCommit, String targetCommit)
       throws IOException, TimeoutException, InterruptedException;

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/diff/FileDiff.java

@@ -19,7 +19,8 @@ public GitRepoUnshallow(Config config, GitClient gitClient) {
     this.gitClient = gitClient;
   }
 
-  public boolean unshallow() throws IOException, InterruptedException, TimeoutException {
+  public synchronized boolean unshallow()
+      throws IOException, InterruptedException, TimeoutException {
     if (!config.isCiVisibilityGitUnshallowEnabled() || !gitClient.isShallow()) {
       return false;
     }