refactor: separate fetching and unshallowing functionality

Author: daniel-mohedano

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

@@ -130,7 +130,9 @@ static PullRequestInfo buildPullRequestInfo(
     if (Strings.isNotBlank(headSha)) {
       // if head sha present try to populate author, committer and message info through git client
       try {
-        gitRepoUnshallow.unshallow(headSha);
+        if (!gitClient.isCommitPresent(headSha)) {
+          gitClient.fetchCommit(headSha);
+        }
         CommitInfo commitInfo = gitClient.getCommitInfo(headSha);
         return PullRequestInfo.merge(ciInfo, new PullRequestInfo(null, null, commitInfo, null));
       } catch (Exception ignored) {

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

@@ -421,7 +421,7 @@ private Diff getPullRequestDiff(boolean impactedTestsDetectionEnabled, String de
     try {
       if (repositoryRoot != null) {
         // ensure repo is not shallow before attempting to get git diff
-        gitRepoUnshallow.unshallow(null);
+        gitRepoUnshallow.unshallow();
 
         String baseCommitSha = pullRequestInfo.getPullRequestBaseBranchSha();
         if (baseCommitSha == null) {

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitClient.java

@@ -20,7 +20,10 @@ public interface GitClient {
   boolean isCommitPresent(String commitReference)
       throws IOException, TimeoutException, InterruptedException;
 
-  void unshallow(@Nullable String remoteCommitReference, boolean unshallowUntilCommit)
+  void unshallow(@Nullable String remoteCommitReference)
+      throws IOException, TimeoutException, InterruptedException;
+
+  void fetchCommit(String remoteCommitReference)
       throws IOException, TimeoutException, InterruptedException;
 
   @Nullable

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitDataUploaderImpl.java

@@ -91,7 +91,7 @@ private void uploadGitData() {
       LOGGER.debug("Starting git data upload, {}", gitClient);
 
       if (!config.isCiVisibilityGitUnshallowDefer()) {
-        gitRepoUnshallow.unshallow(null);
+        gitRepoUnshallow.unshallow();
       }
 
       GitInfo gitInfo = gitInfoProvider.getGitInfo(repoRoot);
@@ -112,7 +112,7 @@ private void uploadGitData() {
         return;
       }
 
-      if (config.isCiVisibilityGitUnshallowDefer() && gitRepoUnshallow.unshallow(null)) {
+      if (config.isCiVisibilityGitUnshallowDefer() && gitRepoUnshallow.unshallow()) {
         latestCommits = gitClient.getLatestCommits();
         commitsToSkip = gitDataApi.searchCommits(remoteUrl, latestCommits);
       }

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitRepoUnshallow.java

@@ -4,7 +4,6 @@
 import datadog.trace.civisibility.utils.ShellCommandExecutor;
 import java.io.IOException;
 import java.util.concurrent.TimeoutException;
-import javax.annotation.Nullable;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -20,47 +19,29 @@ public GitRepoUnshallow(Config config, GitClient gitClient) {
     this.gitClient = gitClient;
   }
 
-  /**
-   * Unshallows git repo up to a specific boundary commit, if provided, or up to the time limit
-   * configured in the git client if not. Won't perform an unshallow action when a boundary is
-   * provided and the object is already present, or if the repository is already unshallowed.
-   *
-   * @param boundaryCommitSha used as boundary for the unshallowing if provided.
-   * @return false if unshallowing is not enabled or unnecessary, true otherwise
-   */
-  public synchronized boolean unshallow(@Nullable String boundaryCommitSha)
+  public synchronized boolean unshallow()
       throws IOException, InterruptedException, TimeoutException {
-    if (!config.isCiVisibilityGitUnshallowEnabled()
-        || (boundaryCommitSha != null && gitClient.isCommitPresent(boundaryCommitSha))
-        || !gitClient.isShallow()) {
+    if (!config.isCiVisibilityGitUnshallowEnabled() || !gitClient.isShallow()) {
       return false;
     }
 
     long unshallowStart = System.currentTimeMillis();
-    if (boundaryCommitSha != null) {
-      try {
-        gitClient.unshallow(boundaryCommitSha, true);
-      } catch (ShellCommandExecutor.ShellCommandFailedException e) {
-        LOGGER.debug("Could not unshallow to specific boundary {}", boundaryCommitSha, e);
-      }
-    } else {
-      try {
-        gitClient.unshallow(GitClient.HEAD, false);
-      } catch (ShellCommandExecutor.ShellCommandFailedException e) {
-        LOGGER.debug(
-            "Could not unshallow using HEAD - assuming HEAD points to a local commit that does not exist in the remote repo",
-            e);
-      }
+    try {
+      gitClient.unshallow(GitClient.HEAD);
+    } catch (ShellCommandExecutor.ShellCommandFailedException e) {
+      LOGGER.debug(
+          "Could not unshallow using HEAD - assuming HEAD points to a local commit that does not exist in the remote repo",
+          e);
+    }
 
-      try {
-        String upstreamBranch = gitClient.getUpstreamBranchSha();
-        gitClient.unshallow(upstreamBranch, false);
-      } catch (ShellCommandExecutor.ShellCommandFailedException e) {
-        LOGGER.debug(
-            "Could not unshallow using upstream branch - assuming currently checked out local branch does not track any remote branch",
-            e);
-        gitClient.unshallow(null, false);
-      }
+    try {
+      String upstreamBranch = gitClient.getUpstreamBranchSha();
+      gitClient.unshallow(upstreamBranch);
+    } catch (ShellCommandExecutor.ShellCommandFailedException e) {
+      LOGGER.debug(
+          "Could not unshallow using upstream branch - assuming currently checked out local branch does not track any remote branch",
+          e);
+      gitClient.unshallow(null);
     }
     LOGGER.debug("Repository unshallowing took {} ms", System.currentTimeMillis() - unshallowStart);
     return true;

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/NoOpGitClient.java

@@ -26,7 +26,7 @@ public boolean isCommitPresent(String commitReference) {
   }
 
   @Override
-  public void unshallow(@Nullable String remoteCommitReference, boolean unshallowUntilCommit) {
+  public void unshallow(@Nullable String remoteCommitReference) {
     // no op
   }
 

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/ShellGitClient.java

@@ -161,30 +161,21 @@ public String getUpstreamBranchSha() throws IOException, TimeoutException, Inter
    *
    * @param remoteCommitReference The commit to fetch from the remote repository, so local repo will
    *     be updated with this commit and its ancestors. If {@code null}, everything will be fetched.
-   * @param unshallowUntilCommit Only unshallow up until the commit provided, or use the time limit
-   *     configured if not. Ignored if no reference is provided.
    * @throws IOException If an error was encountered while writing command input or reading output
    * @throws TimeoutException If timeout was reached while waiting for Git command to finish
    * @throws InterruptedException If current thread was interrupted while waiting for Git command to
    *     finish
    */
   @Override
-  public void unshallow(@Nullable String remoteCommitReference, boolean unshallowUntilCommit)
+  public void unshallow(@Nullable String remoteCommitReference)
       throws IOException, TimeoutException, InterruptedException {
     executeCommand(
         Command.UNSHALLOW,
         () -> {
           String remote = getRemoteName();
 
           // refetch data from the server for the given period of time
-          if (remoteCommitReference != null) {
-            if (!GitUtils.isValidRef(remoteCommitReference)) {
-              return (Void) null;
-            }
-            String boundaryArg =
-                unshallowUntilCommit
-                    ? "--no-write-fetch-head"
-                    : String.format("--shallow-since='%s'", latestCommitsSince);
+          if (remoteCommitReference != null && GitUtils.isValidRef(remoteCommitReference)) {
             String commitSha = getSha(remoteCommitReference);
             commandExecutor.executeCommand(
                 ShellCommandExecutor.OutputParser.IGNORE,
@@ -193,7 +184,7 @@ public void unshallow(@Nullable String remoteCommitReference, boolean unshallowU
                 "--update-shallow",
                 "--filter=blob:none",
                 "--recurse-submodules=no",
-                boundaryArg,
+                String.format("--shallow-since='%s'", latestCommitsSince),
                 remote,
                 commitSha);
           } else {
@@ -212,6 +203,40 @@ public void unshallow(@Nullable String remoteCommitReference, boolean unshallowU
         });
   }
 
+  /**
+   * Fetches provided commit object from the server.
+   *
+   * @param commitReference Commit to fetch from the remote repository
+   * @throws IOException If an error was encountered while writing command input or reading output
+   * @throws TimeoutException If timeout was reached while waiting for Git command to finish
+   * @throws InterruptedException If current thread was interrupted while waiting for Git command to
+   */
+  @Override
+  public void fetchCommit(String commitReference)
+      throws IOException, TimeoutException, InterruptedException {
+    if (!GitUtils.isValidRef(commitReference)) {
+      return;
+    }
+    executeCommand(
+        Command.OTHER,
+        () -> {
+          String remote = getRemoteName();
+
+          commandExecutor.executeCommand(
+              ShellCommandExecutor.OutputParser.IGNORE,
+              "git",
+              "fetch",
+              "--update-shallow",
+              "--filter=blob:none",
+              "--recurse-submodules=no",
+              "--no-write-fetch-head",
+              remote,
+              commitReference);
+
+          return (Void) null;
+        });
+  }
+
   /**
    * Returns the absolute path of the .git directory.
    *

dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/git/tree/GitClientTest.groovy

@@ -84,41 +84,35 @@ class GitClientTest extends Specification {
     commits.size() == 1
 
     when:
-    gitClient.unshallow(remoteSha, false)
+    gitClient.unshallow(remoteSha)
     shallow = gitClient.isShallow()
     commits = gitClient.getLatestCommits()
 
     then:
-    shallow == isShallow
-    commits.size() == numCommits
+    !shallow
+    commits.size() == 10
 
     where:
-    remoteSha      | isShallow | numCommits
-    GitClient.HEAD | false     | 10
-    null           | false     | 10
+    remoteSha << [GitClient.HEAD, null]
   }
 
-  def "test unshallow using commit as boundary"() {
+  def "test commit fetch"() {
     given:
     givenGitRepo("ci/git/shallow/git")
 
     when:
     def commit = "f4377e97f10c2d58696192b170b2fef2a8464b04"
     def gitClient = givenGitClient()
-    def shallow = gitClient.isShallow()
     def isPresent = gitClient.isCommitPresent(commit)
 
     then:
-    shallow
     !isPresent
 
     when:
-    gitClient.unshallow(commit, true)
-    shallow = gitClient.isShallow()
+    gitClient.fetchCommit(commit)
     isPresent = gitClient.isCommitPresent(commit)
 
     then:
-    shallow
     isPresent
   }