Improving getAllDigests to remove duplicates.

--
MOS_MIGRATED_REVID=133580990

2 files changed, 20 insertions, 6 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/remote/TreeNodeRepository.java b/src/main/java/com/google/devtools/build/lib/remote/TreeNodeRepository.java
index 6cac205109..911097f1fe 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/TreeNodeRepository.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/TreeNodeRepository.java
@@ -16,7 +16,9 @@ package com.google.devtools.build.lib.remote;
 
 import com.google.common.base.Function;
 import com.google.common.base.Predicate;
+import com.google.common.collect.ImmutableCollection;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Interner;
 import com.google.common.collect.Interners;
 import com.google.common.collect.Iterables;
@@ -335,8 +337,8 @@ public final class TreeNodeRepository extends TreeTraverser<TreeNodeRepository.T
    * Returns the precomputed digests for both data and metadata. Should only be used after
    * computeMerkleDigests has been called on one of the node ancestors.
    */
-  public ImmutableList<ContentDigest> getAllDigests(TreeNode root) {
-    ImmutableList.Builder<ContentDigest> digests = ImmutableList.builder();
+  public ImmutableCollection<ContentDigest> getAllDigests(TreeNode root) {
+    ImmutableSet.Builder<ContentDigest> digests = ImmutableSet.builder();
     for (TreeNode node : descendants(root)) {
       digests.add(Preconditions.checkNotNull(treeNodeDigestCache.get(node)));
       if (node.isLeaf()) {
diff --git a/src/test/java/com/google/devtools/build/lib/remote/TreeNodeRepositoryTest.java b/src/test/java/com/google/devtools/build/lib/remote/TreeNodeRepositoryTest.java
index 3217f70c4e..3366c480ac 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/TreeNodeRepositoryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/TreeNodeRepositoryTest.java
@@ -15,6 +15,7 @@ package com.google.devtools.build.lib.remote;
 
 import static com.google.common.truth.Truth.assertThat;
 
+import com.google.common.collect.ImmutableCollection;
 import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.actions.ActionInput;
 import com.google.devtools.build.lib.actions.Artifact;
@@ -68,7 +69,7 @@ public class TreeNodeRepositoryTest {
     TreeNode barNode = aNode.getChildEntries().get(0).getChild();
 
     repo.computeMerkleDigests(root);
-    ImmutableList<ContentDigest> digests = repo.getAllDigests(root);
+    ImmutableCollection<ContentDigest> digests = repo.getAllDigests(root);
     ContentDigest rootDigest = repo.getMerkleDigest(root);
     ContentDigest aDigest = repo.getMerkleDigest(aNode);
     ContentDigest fooDigest = repo.getMerkleDigest(fooNode);
@@ -77,13 +78,12 @@ public class TreeNodeRepositoryTest {
     ContentDigest barContentsDigest = ContentDigests.computeDigest(bar.getPath());
     assertThat(digests)
         .containsExactly(
-            rootDigest, aDigest, barDigest, barContentsDigest, fooDigest, fooContentsDigest)
-        .inOrder();
+            rootDigest, aDigest, barDigest, barContentsDigest, fooDigest, fooContentsDigest);
 
     ArrayList<FileNode> fileNodes = new ArrayList<>();
     ArrayList<ActionInput> actionInputs = new ArrayList<>();
     repo.getDataFromDigests(digests, actionInputs, fileNodes);
-    assertThat(actionInputs).containsExactly(bar, foo).inOrder();
+    assertThat(actionInputs).containsExactly(bar, foo);
     assertThat(fileNodes).hasSize(4);
     FileNode rootFileNode = fileNodes.get(0);
     assertThat(rootFileNode.getChild(0).getPath()).isEqualTo("a");
@@ -98,4 +98,16 @@ public class TreeNodeRepositoryTest {
     FileNode fooFileNode = fileNodes.get(3);
     assertThat(fooFileNode.getFileMetadata().getDigest()).isEqualTo(fooContentsDigest);
   }
+
+  @Test
+  public void testGetAllDigests() throws Exception {
+    Artifact foo1 = new Artifact(scratch.file("/exec/root/a/foo", "1"), rootDir);
+    Artifact foo2 = new Artifact(scratch.file("/exec/root/b/foo", "1"), rootDir);
+    Artifact foo3 = new Artifact(scratch.file("/exec/root/c/foo", "1"), rootDir);
+    TreeNodeRepository repo = new TreeNodeRepository(rootDir.getPath());
+    TreeNode root = repo.buildFromActionInputs(ImmutableList.<ActionInput>of(foo1, foo2, foo3));
+    repo.computeMerkleDigests(root);
+    // Reusing same node for the "foo" subtree: only need the root, root child, foo, and contents:
+    assertThat(repo.getAllDigests(root)).hasSize(4);
+  }
 }