Hide the RemoteFileArtifactValue constructor behind a factory method.

This makes it easier to later return different specializations depending on which fields are present, which will be required by bazelbuild#16283. PiperOrigin-RevId: 477674504 Change-Id: Ieec385e923ab4ce0546f8853811df18316a8d884
aiuto · Oct 12, 2022 · 4cfe794 · 4cfe794
1 parent d8fc16b
commit 4cfe794
Show file tree

Hide file tree

Showing 14 changed files with 30 additions and 25 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java b/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java
@@ -540,15 +540,20 @@ public static class RemoteFileArtifactValue extends FileArtifactValue {
     private final int locationIndex;
     private final String actionId;
 
-    public RemoteFileArtifactValue(byte[] digest, long size, int locationIndex, String actionId) {
+    private RemoteFileArtifactValue(byte[] digest, long size, int locationIndex, String actionId) {
       this.digest = Preconditions.checkNotNull(digest, actionId);
       this.size = size;
       this.locationIndex = locationIndex;
       this.actionId = actionId;
     }
 
-    public RemoteFileArtifactValue(byte[] digest, long size, int locationIndex) {
-      this(digest, size, locationIndex, /* actionId= */ "");
+    public static RemoteFileArtifactValue create(
+        byte[] digest, long size, int locationIndex, String actionId) {
+      return new RemoteFileArtifactValue(digest, size, locationIndex, actionId);
+    }
+
+    public static RemoteFileArtifactValue create(byte[] digest, long size, int locationIndex) {
+      return new RemoteFileArtifactValue(digest, size, locationIndex, /* actionId= */ "");
     }
 
     @Override

diff --git a/src/main/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCache.java b/src/main/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCache.java
@@ -484,7 +484,7 @@ private static RemoteFileArtifactValue decodeRemoteMetadata(
 
     String actionId = getStringForIndex(indexer, VarInt.getVarInt(source));
 
-    return new RemoteFileArtifactValue(digest, size, locationIndex, actionId);
+    return RemoteFileArtifactValue.create(digest, size, locationIndex, actionId);
   }
 
   /** @return action data encoded as a byte[] array. */

diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java
@@ -785,7 +785,7 @@ private void injectRemoteArtifact(
         TreeFileArtifact child =
             TreeFileArtifact.createTreeOutput(parent, file.path().relativeTo(parent.getPath()));
         RemoteFileArtifactValue value =
-            new RemoteFileArtifactValue(
+            RemoteFileArtifactValue.create(
                 DigestUtil.toBinaryDigest(file.digest()),
                 file.digest().getSizeBytes(),
                 /*locationIndex=*/ 1,
@@ -802,7 +802,7 @@ private void injectRemoteArtifact(
       }
       remoteActionFileSystem.injectFile(
           output,
-          new RemoteFileArtifactValue(
+          RemoteFileArtifactValue.create(
               DigestUtil.toBinaryDigest(outputMetadata.digest()),
               outputMetadata.digest().getSizeBytes(),
               /*locationIndex=*/ 1,

diff --git a/src/test/java/com/google/devtools/build/lib/actions/ActionCacheCheckerTest.java b/src/test/java/com/google/devtools/build/lib/actions/ActionCacheCheckerTest.java
@@ -448,7 +448,7 @@ public void testDeletedConstantMetadataOutputCausesReexecution() throws Exceptio
 
   private RemoteFileArtifactValue createRemoteFileMetadata(String content) {
     byte[] bytes = content.getBytes(UTF_8);
-    return new RemoteFileArtifactValue(digest(bytes), bytes.length, 1, "action-id");
+    return RemoteFileArtifactValue.create(digest(bytes), bytes.length, 1, "action-id");
   }
 
   private static TreeArtifactValue createTreeMetadata(

diff --git a/src/test/java/com/google/devtools/build/lib/actions/CompletionContextTest.java b/src/test/java/com/google/devtools/build/lib/actions/CompletionContextTest.java
@@ -40,7 +40,7 @@
 @RunWith(JUnit4.class)
 public class CompletionContextTest {
   private static final FileArtifactValue DUMMY_METADATA =
-      new RemoteFileArtifactValue(/*digest=*/ new byte[0], /*size=*/ 0, /*locationIndex=*/ 0);
+      RemoteFileArtifactValue.create(/*digest=*/ new byte[0], /*size=*/ 0, /*locationIndex=*/ 0);
 
   private Path execRoot;
   private ArtifactRoot outputRoot;

diff --git a/...st/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCacheTest.java b/...st/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCacheTest.java
@@ -204,7 +204,7 @@ private RemoteFileArtifactValue createRemoteMetadata(Artifact artifact, String c
             .getHashFunction()
             .hashBytes(bytes)
             .asBytes();
-    return new RemoteFileArtifactValue(digest, bytes.length, 1, "action-id");
+    return RemoteFileArtifactValue.create(digest, bytes.length, 1, "action-id");
   }
 
   private TreeArtifactValue createTreeMetadata(

diff --git a/src/test/java/com/google/devtools/build/lib/remote/ActionInputPrefetcherTestBase.java b/src/test/java/com/google/devtools/build/lib/remote/ActionInputPrefetcherTestBase.java
@@ -87,7 +87,7 @@ protected Artifact createRemoteArtifact(
     byte[] contentsBytes = contents.getBytes(UTF_8);
     HashCode hashCode = HASH_FUNCTION.getHashFunction().hashBytes(contentsBytes);
     FileArtifactValue f =
-        new RemoteFileArtifactValue(
+        RemoteFileArtifactValue.create(
             hashCode.asBytes(), contentsBytes.length, /* locationIndex= */ 1, "action-id");
     metadata.put(a, f);
     cas.put(hashCode, contentsBytes);

diff --git a/...t/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderTest.java b/...t/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderTest.java
@@ -439,7 +439,7 @@ private Artifact createRemoteArtifact(
     byte[] b = contents.getBytes(StandardCharsets.UTF_8);
     HashCode h = HashCode.fromString(DIGEST_UTIL.compute(b).getHash());
     FileArtifactValue f =
-        new RemoteFileArtifactValue(h.asBytes(), b.length, /* locationIndex= */ 1, "action-id");
+        RemoteFileArtifactValue.create(h.asBytes(), b.length, /* locationIndex= */ 1, "action-id");
     inputs.putWithNoDepOwner(a, f);
     return a;
   }

diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTest.java
@@ -169,7 +169,7 @@ private Artifact createRemoteArtifact(
     byte[] b = contents.getBytes(StandardCharsets.UTF_8);
     HashCode h = HASH_FUNCTION.getHashFunction().hashBytes(b);
     FileArtifactValue f =
-        new RemoteFileArtifactValue(h.asBytes(), b.length, /* locationIndex= */ 1, "action-id");
+        RemoteFileArtifactValue.create(h.asBytes(), b.length, /* locationIndex= */ 1, "action-id");
     inputs.putWithNoDepOwner(a, f);
     return a;
   }

diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java
@@ -993,11 +993,11 @@ public void downloadOutputs_outputDirectoriesWithMinimal_injectingMetadata() thr
         TreeArtifactValue.newBuilder(dir)
             .putChild(
                 TreeFileArtifact.createTreeOutput(dir, "file1"),
-                new RemoteFileArtifactValue(
+                RemoteFileArtifactValue.create(
                     toBinaryDigest(d1), d1.getSizeBytes(), 1, action.getActionId()))
             .putChild(
                 TreeFileArtifact.createTreeOutput(dir, "a/file2"),
-                new RemoteFileArtifactValue(
+                RemoteFileArtifactValue.create(
                     toBinaryDigest(d2), d2.getSizeBytes(), 1, action.getActionId()))
             .build();
     verify(actionFileSystem).injectTree(dir, tree);

diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandlerTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandlerTest.java
@@ -296,7 +296,7 @@ public void resettingOutputs() throws Exception {
 
     // Inject a remote file of size 42.
     handler.injectFile(
-        artifact, new RemoteFileArtifactValue(new byte[] {1, 2, 3}, 42, 0, "ultimate-answer"));
+        artifact, RemoteFileArtifactValue.create(new byte[] {1, 2, 3}, 42, 0, "ultimate-answer"));
     assertThat(handler.getMetadata(artifact).getSize()).isEqualTo(42);
 
     // Reset this output, which will make the handler stat the file again.
@@ -320,7 +320,7 @@ public void injectRemoteArtifactMetadata() throws Exception {
     byte[] digest = new byte[] {1, 2, 3};
     int size = 10;
     handler.injectFile(
-        artifact, new RemoteFileArtifactValue(digest, size, /*locationIndex=*/ 1, "action-id"));
+        artifact, RemoteFileArtifactValue.create(digest, size, /*locationIndex=*/ 1, "action-id"));
 
     FileArtifactValue v = handler.getMetadata(artifact);
     assertThat(v).isNotNull();
@@ -343,7 +343,7 @@ public void cannotInjectTreeArtifactChildIndividually() {
             /*outputs=*/ ImmutableSet.of(treeArtifact));
     handler.prepareForActionExecution();
 
-    RemoteFileArtifactValue childValue = new RemoteFileArtifactValue(new byte[] {1, 2, 3}, 5, 1);
+    RemoteFileArtifactValue childValue = RemoteFileArtifactValue.create(new byte[] {1, 2, 3}, 5, 1);
 
     assertThrows(IllegalArgumentException.class, () -> handler.injectFile(child, childValue));
     assertThat(handler.getOutputStore().getAllArtifactData()).isEmpty();
@@ -367,7 +367,7 @@ public void canInjectTemplateExpansionOutput() {
             /*outputs=*/ ImmutableSet.of(treeArtifact));
     handler.prepareForActionExecution();
 
-    RemoteFileArtifactValue value = new RemoteFileArtifactValue(new byte[] {1, 2, 3}, 5, 1);
+    RemoteFileArtifactValue value = RemoteFileArtifactValue.create(new byte[] {1, 2, 3}, 5, 1);
     handler.injectFile(output, value);
 
     assertThat(handler.getOutputStore().getAllArtifactData()).containsExactly(output, value);
@@ -391,10 +391,10 @@ public void injectRemoteTreeArtifactMetadata() throws Exception {
         TreeArtifactValue.newBuilder(treeArtifact)
             .putChild(
                 TreeFileArtifact.createTreeOutput(treeArtifact, "foo"),
-                new RemoteFileArtifactValue(new byte[] {1, 2, 3}, 5, 1, "foo"))
+                RemoteFileArtifactValue.create(new byte[] {1, 2, 3}, 5, 1, "foo"))
             .putChild(
                 TreeFileArtifact.createTreeOutput(treeArtifact, "bar"),
-                new RemoteFileArtifactValue(new byte[] {4, 5, 6}, 10, 1, "bar"))
+                RemoteFileArtifactValue.create(new byte[] {4, 5, 6}, 10, 1, "bar"))
             .build();
 
     handler.injectTree(treeArtifact, tree);
@@ -589,7 +589,7 @@ public void transformAfterInputDiscovery() throws Exception {
     assertThat(handler.getMetadata(unknown)).isNull();
 
     OutputStore newStore = new OutputStore();
-    FileArtifactValue knownMetadata = new RemoteFileArtifactValue(new byte[] {1, 2, 3}, 5, 1);
+    FileArtifactValue knownMetadata = RemoteFileArtifactValue.create(new byte[] {1, 2, 3}, 5, 1);
     newStore.putArtifactData(known, knownMetadata);
     ActionMetadataHandler newHandler = handler.transformAfterInputDiscovery(newStore);
     assertThat(newHandler.getOutputStore()).isNotEqualTo(handler.getOutputStore());

diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java
@@ -1327,7 +1327,7 @@ private RemoteFileArtifactValue createRemoteFileArtifactValue(String contents) {
     byte[] data = contents.getBytes();
     DigestHashFunction hashFn = fs.getDigestFunction();
     HashCode hash = hashFn.getHashFunction().hashBytes(data);
-    return new RemoteFileArtifactValue(hash.asBytes(), data.length, -1, "action-id");
+    return RemoteFileArtifactValue.create(hash.asBytes(), data.length, -1, "action-id");
   }
 
   @Test

diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java
@@ -615,10 +615,10 @@ void run(ActionExecutionContext actionExecutionContext) throws IOException {
   public void remoteDirectoryInjection() throws Exception {
     SpecialArtifact out = createTreeArtifact("output");
     RemoteFileArtifactValue remoteFile1 =
-        new RemoteFileArtifactValue(
+        RemoteFileArtifactValue.create(
             Hashing.sha256().hashString("one", UTF_8).asBytes(), /*size=*/ 3, /*locationIndex=*/ 1);
     RemoteFileArtifactValue remoteFile2 =
-        new RemoteFileArtifactValue(
+        RemoteFileArtifactValue.create(
             Hashing.sha256().hashString("two", UTF_8).asBytes(), /*size=*/ 3, /*locationIndex=*/ 2);
 
     Action action =

diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactValueTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactValueTest.java
@@ -699,7 +699,7 @@ private static SpecialArtifact createTreeArtifact(String execPath, ArtifactRoot
   }
 
   private static FileArtifactValue metadataWithId(int id) {
-    return new RemoteFileArtifactValue(new byte[] {(byte) id}, id, id);
+    return RemoteFileArtifactValue.create(new byte[] {(byte) id}, id, id);
   }
 
   private static FileArtifactValue metadataWithIdNoDigest(int id) {