feat: unify coalesce behavior

Author: daniel-mohedano

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

@@ -125,13 +125,13 @@ static PullRequestInfo buildPullRequestInfo(
     }
 
     // complete with CI vars if user didn't provide all information
-    PullRequestInfo ciInfo = PullRequestInfo.merge(userInfo, ciProviderInfo.buildPullRequestInfo());
+    PullRequestInfo ciInfo = PullRequestInfo.coalesce(userInfo, ciProviderInfo.buildPullRequestInfo());
     String headSha = ciInfo.getHeadCommit().getSha();
     if (Strings.isNotBlank(headSha)) {
       // if head sha present try to populate author, committer and message info through git client
       try {
         CommitInfo commitInfo = gitClient.getCommitInfo(headSha, true);
-        return PullRequestInfo.merge(
+        return PullRequestInfo.coalesce(
             ciInfo, new PullRequestInfo(null, null, null, commitInfo, null));
       } catch (Exception ignored) {
       }
@@ -170,7 +170,7 @@ private static PullRequestInfo buildUserPullRequestInfo(
             new CommitInfo(environment.get(Constants.DDCI_PULL_REQUEST_SOURCE_SHA)),
             null);
 
-    return PullRequestInfo.merge(userInfo, ddCiInfo);
+    return PullRequestInfo.coalesce(userInfo, ddCiInfo);
   }
 
   private static String getRepoRoot(CIInfo ciInfo, GitClient.Factory gitClientFactory) {

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

@@ -67,23 +67,19 @@ public boolean isComplete() {
   }
 
   /**
-   * Merges info by completing the empty information fields with the fallback's
+   * Combine infos by completing the empty information fields in {@code first} with {@code second}'s
    *
-   * @param info Base PR info
-   * @param fallback Fallback PR info
-   * @return Completed PR info
+   * @param first Base PR info
+   * @param second Fallback PR info
+   * @return Combined PR info
    */
-  public static PullRequestInfo merge(PullRequestInfo info, PullRequestInfo fallback) {
+  public static PullRequestInfo coalesce(final PullRequestInfo first, final PullRequestInfo second) {
     return new PullRequestInfo(
-        Strings.isNotBlank(info.baseBranch) ? info.baseBranch : fallback.baseBranch,
-        Strings.isNotBlank(info.baseBranchSha) ? info.baseBranchSha : fallback.baseBranchSha,
-        Strings.isNotBlank(info.baseBranchHeadSha)
-            ? info.baseBranchHeadSha
-            : fallback.baseBranchHeadSha,
-        CommitInfo.merge(info.headCommit, fallback.headCommit),
-        Strings.isNotBlank(info.pullRequestNumber)
-            ? info.pullRequestNumber
-            : fallback.pullRequestNumber);
+        Strings.coalesce(first.baseBranch, second.baseBranch),
+        Strings.coalesce(first.baseBranchSha, second.baseBranchSha),
+        Strings.coalesce(first.baseBranchHeadSha, second.baseBranchHeadSha),
+        CommitInfo.coalesce(first.headCommit, second.headCommit),
+        Strings.coalesce(first.pullRequestNumber, second.pullRequestNumber));
   }
 
   @Override

dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/ci/PullRequestInfoTest.groovy

@@ -38,9 +38,9 @@ class PullRequestInfoTest extends Specification {
     new PullRequestInfo("branch", "baseSha", SHA_A, COMMIT_A, "42") | true
   }
 
-  def "test info merge"() {
+  def "test info coalesce"() {
     expect:
-    PullRequestInfo.merge(infoA, infoB) == result
+    PullRequestInfo.coalesce(infoA, infoB) == result
 
     where:
     infoA                                                             | infoB                                                             | result

internal-api/src/main/java/datadog/trace/api/git/CommitInfo.java

@@ -59,18 +59,18 @@ public boolean isComplete() {
   }
 
   /**
-   * Merges info by completing the empty information fields with the fallback's
+   * Combine infos by completing the empty information fields in {@code first} with {@code second}'s
    *
-   * @param info Base commit info
-   * @param fallback Fallback commit info
-   * @return Completed commit info
+   * @param first Base commit info
+   * @param second Fallback commit info
+   * @return Combined commit info
    */
-  public static CommitInfo merge(CommitInfo info, CommitInfo fallback) {
+  public static CommitInfo coalesce(final CommitInfo first, final CommitInfo second) {
     return new CommitInfo(
-        Strings.isNotBlank(info.sha) ? info.sha : fallback.sha,
-        PersonInfo.merge(info.author, fallback.author),
-        PersonInfo.merge(info.committer, fallback.committer),
-        Strings.isNotBlank(info.fullMessage) ? info.fullMessage : fallback.fullMessage);
+        Strings.coalesce(first.sha, second.sha),
+        PersonInfo.coalesce(first.author, second.author),
+        PersonInfo.coalesce(first.committer, second.committer),
+        Strings.coalesce(first.fullMessage, second.fullMessage));
   }
 
   @Override

internal-api/src/main/java/datadog/trace/api/git/PersonInfo.java

@@ -57,17 +57,17 @@ public boolean isComplete() {
   }
 
   /**
-   * Merges info by completing the empty information fields with the fallback's
+   * Combine infos by completing the empty information fields in {@code first} with {@code second}'s
    *
-   * @param info Base person info
-   * @param fallback Fallback person info
-   * @return Completed person info
+   * @param first Base person info
+   * @param second Fallback person info
+   * @return Combined person info
    */
-  public static PersonInfo merge(final PersonInfo info, final PersonInfo fallback) {
+  public static PersonInfo coalesce(final PersonInfo first, final PersonInfo second) {
     return new PersonInfo(
-        Strings.isNotBlank(info.name) ? info.name : fallback.name,
-        Strings.isNotBlank(info.email) ? info.email : fallback.email,
-        Strings.isNotBlank(info.iso8601Date) ? info.iso8601Date : fallback.iso8601Date);
+        Strings.coalesce(first.name, second.name),
+        Strings.coalesce(first.email, second.email),
+        Strings.coalesce(first.iso8601Date, second.iso8601Date));
   }
 
   @Override

internal-api/src/main/java/datadog/trace/util/Strings.java

@@ -7,6 +7,7 @@
 import java.security.NoSuchAlgorithmException;
 import java.util.concurrent.ThreadLocalRandom;
 import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
 
 public final class Strings {
 
@@ -240,4 +241,18 @@ public static String[] concat(String[] arr, String... extra) {
     System.arraycopy(extra, 0, result, arr.length, extra.length);
     return result;
   }
+
+  /**
+   * @return first non-blank string out of the two, {@code null} if both are blank
+   */
+  @Nullable
+  public static String coalesce(@Nullable final String first, @Nullable final String second) {
+    if (isNotBlank(first)) {
+      return first;
+    } else if (isNotBlank(second)) {
+      return second;
+    } else {
+      return null;
+    }
+  }
 }

internal-api/src/test/groovy/datadog/trace/api/civisibility/git/CommitInfoTest.groovy

@@ -33,9 +33,9 @@ class CommitInfoTest extends Specification {
     new CommitInfo("sha", PERSON_A, PERSON_A, "msg")       | true
   }
 
-  def "test info merge"() {
+  def 'test info coalesce'() {
     expect:
-    CommitInfo.merge(infoA, infoB) == result
+    CommitInfo.coalesce(infoA, infoB) == result
     where:
     infoA                                                      | infoB                                                  | result
     new CommitInfo("shaA", PERSON_A, PERSON_A, "msgA")         | new CommitInfo("shaB", PERSON_B, PERSON_B, "msgB")     | new CommitInfo("shaA", PERSON_A, PERSON_A, "msgA")

internal-api/src/test/groovy/datadog/trace/api/civisibility/git/PersonInfoTest.groovy

@@ -28,9 +28,9 @@ class PersonInfoTest extends Specification {
     new PersonInfo("name", "email", "date") | true
   }
 
-  def "test info merge"() {
+  def 'test info coalesce'() {
     expect:
-    PersonInfo.merge(infoA, infoB) == result
+    PersonInfo.coalesce(infoA, infoB) == result
     where:
     infoA                                      | infoB                                      | result
     new PersonInfo("nameA", "emailA", "dateA") | new PersonInfo("nameB", "emailB", "dateB") | new PersonInfo("nameA", "emailA", "dateA")

internal-api/src/test/groovy/datadog/trace/util/StringsTest.groovy

@@ -158,4 +158,21 @@ class StringsTest extends DDSpecification {
     ''                      | ''
     '[email protected]' | '7A6F757A6F754073616E73676C7574656E2E636F6D'
   }
+
+  void 'test coalesce: #first - #second'() {
+    when:
+    def combined = Strings.coalesce(first, second)
+
+    then:
+    expected == combined
+
+    where:
+    first | second | expected
+    "a"   | "b"    | "a"
+    "a"   | null   | "a"
+    null  | "b"    | "b"
+    ""    | "b"    | "b"
+    null  | null   | null
+    ""    | ""     | null
+  }
 }