From 4cfe794698dabe626e40812542378d7564b8f405 Mon Sep 17 00:00:00 2001 From: Googler Date: Thu, 29 Sep 2022 02:47:24 -0700 Subject: [PATCH] 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 https://github.com/bazelbuild/bazel/pull/16283. PiperOrigin-RevId: 477674504 Change-Id: Ieec385e923ab4ce0546f8853811df18316a8d884 --- .../build/lib/actions/FileArtifactValue.java | 11 ++++++++--- .../cache/CompactPersistentActionCache.java | 2 +- .../build/lib/remote/RemoteExecutionService.java | 4 ++-- .../build/lib/actions/ActionCacheCheckerTest.java | 2 +- .../build/lib/actions/CompletionContextTest.java | 2 +- .../cache/CompactPersistentActionCacheTest.java | 2 +- .../lib/remote/ActionInputPrefetcherTestBase.java | 2 +- .../ByteStreamBuildEventArtifactUploaderTest.java | 2 +- .../lib/remote/RemoteActionFileSystemTest.java | 2 +- .../lib/remote/RemoteExecutionServiceTest.java | 4 ++-- .../lib/skyframe/ActionMetadataHandlerTest.java | 14 +++++++------- .../lib/skyframe/FilesystemValueCheckerTest.java | 2 +- .../build/lib/skyframe/TreeArtifactBuildTest.java | 4 ++-- .../build/lib/skyframe/TreeArtifactValueTest.java | 2 +- 14 files changed, 30 insertions(+), 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 index b8dd83f748e348..f08bdb949f834a 100644 --- 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 index 36c4a8ef99ccf1..6fc3d4956d65ca 100644 --- 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 index 4eaf15b5ae6615..2c61a0038feee9 100644 --- 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 index 536d0866e32373..2244795ea915aa 100644 --- 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 index fb4cbc0201ef5d..764adbf976a4b2 100644 --- 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/src/test/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCacheTest.java b/src/test/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCacheTest.java index 7e8be67a8ee5b6..6bb87ee6331d28 100644 --- a/src/test/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCacheTest.java +++ b/src/test/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 index 96a3f87d61f33f..623f493b04ac77 100644 --- 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/src/test/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderTest.java b/src/test/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderTest.java index f2af95407b5dbf..5cc44a9323e2af 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderTest.java +++ b/src/test/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 index 061ab11524b7f7..e5dcbfa208cb7e 100644 --- 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 index 47610ff10b1b51..39b6cc35c7986d 100644 --- 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 index 62b476c10af70e..bdf714b4fc0abd 100644 --- 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 index 13f838eb5e23c9..728beb2dbca88b 100644 --- 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 index 89418d0ba8e405..62a8c848aa7a6f 100644 --- 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 index 6223c04c4f00cf..d3bd5e25628f50 100644 --- 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) {