RemoteCache: replace getCacheCapabilities with getServerCapabilities

Follow up of bazelbuild#23582 and bazelbuild#18270
sluongng · Sep 12, 2024 · cef46be · cef46be
1 parent 657e34c
commit cef46be
Show file tree

Hide file tree

Showing 7 changed files with 33 additions and 43 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/remote/GrpcCacheClient.java b/src/main/java/com/google/devtools/build/lib/remote/GrpcCacheClient.java
@@ -20,7 +20,6 @@
 import build.bazel.remote.execution.v2.ActionCacheGrpc;
 import build.bazel.remote.execution.v2.ActionCacheGrpc.ActionCacheFutureStub;
 import build.bazel.remote.execution.v2.ActionResult;
-import build.bazel.remote.execution.v2.CacheCapabilities;
 import build.bazel.remote.execution.v2.ContentAddressableStorageGrpc;
 import build.bazel.remote.execution.v2.ContentAddressableStorageGrpc.ContentAddressableStorageFutureStub;
 import build.bazel.remote.execution.v2.Digest;
@@ -266,11 +265,6 @@ private ListenableFuture<CachedActionResult> handleStatus(
         MoreExecutors.directExecutor());
   }
 
-  @Override
-  public CacheCapabilities getCacheCapabilities() throws IOException {
-    return channel.getServerCapabilities().getCacheCapabilities();
-  }
-
   @Override
   public ServerCapabilities getServerCapabilities() throws IOException {
     return channel.getServerCapabilities();

diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java
@@ -108,10 +108,7 @@ public RemoteCache(
   }
 
   public CacheCapabilities getRemoteCacheCapabilities() throws IOException {
-    if (remoteCacheClient == null) {
-      return CacheCapabilities.getDefaultInstance();
-    }
-    return remoteCacheClient.getCacheCapabilities();
+    return getServerCapabilities().getCacheCapabilities();
   }
 
   public ListenableFuture<String> getRemoteAuthority() {

diff --git a/src/main/java/com/google/devtools/build/lib/remote/common/RemoteCacheClient.java b/src/main/java/com/google/devtools/build/lib/remote/common/RemoteCacheClient.java
@@ -16,7 +16,6 @@
 
 import build.bazel.remote.execution.v2.Action;
 import build.bazel.remote.execution.v2.ActionResult;
-import build.bazel.remote.execution.v2.CacheCapabilities;
 import build.bazel.remote.execution.v2.Digest;
 import build.bazel.remote.execution.v2.ServerCapabilities;
 import com.google.auto.value.AutoValue;
@@ -34,8 +33,6 @@
  * <p>Implementations must be thread-safe.
  */
 public interface RemoteCacheClient extends MissingDigestsFinder {
-  CacheCapabilities getCacheCapabilities() throws IOException;
-
   ServerCapabilities getServerCapabilities() throws IOException;
 
   ListenableFuture<String> getAuthority();

diff --git a/src/main/java/com/google/devtools/build/lib/remote/disk/DiskCacheClient.java b/src/main/java/com/google/devtools/build/lib/remote/disk/DiskCacheClient.java
@@ -245,18 +245,15 @@ private void checkActionResult(ActionResult actionResult) throws IOException {
     }
   }
 
-  @Override
-  public CacheCapabilities getCacheCapabilities() {
-    return CacheCapabilities.newBuilder()
-        .setActionCacheUpdateCapabilities(
-            ActionCacheUpdateCapabilities.newBuilder().setUpdateEnabled(true).build())
-        .setSymlinkAbsolutePathStrategy(SymlinkAbsolutePathStrategy.Value.ALLOWED)
-        .build();
-  }
-
   @Override
   public ServerCapabilities getServerCapabilities() {
-    return ServerCapabilities.newBuilder().setCacheCapabilities(getCacheCapabilities()).build();
+    var cacheCapabilities =
+        CacheCapabilities.newBuilder()
+            .setActionCacheUpdateCapabilities(
+                ActionCacheUpdateCapabilities.newBuilder().setUpdateEnabled(true).build())
+            .setSymlinkAbsolutePathStrategy(SymlinkAbsolutePathStrategy.Value.ALLOWED)
+            .build();
+    return ServerCapabilities.newBuilder().setCacheCapabilities(cacheCapabilities).build();
   }
 
   @Override

diff --git a/src/main/java/com/google/devtools/build/lib/remote/http/HttpCacheClient.java b/src/main/java/com/google/devtools/build/lib/remote/http/HttpCacheClient.java
@@ -606,18 +606,15 @@ private void getAfterCredentialRefresh(DownloadCommand cmd, SettableFuture<Void>
             });
   }
 
-  @Override
-  public CacheCapabilities getCacheCapabilities() {
-    return CacheCapabilities.newBuilder()
-        .setActionCacheUpdateCapabilities(
-            ActionCacheUpdateCapabilities.newBuilder().setUpdateEnabled(true).build())
-        .setSymlinkAbsolutePathStrategy(SymlinkAbsolutePathStrategy.Value.ALLOWED)
-        .build();
-  }
-
   @Override
   public ServerCapabilities getServerCapabilities() {
-    return ServerCapabilities.newBuilder().setCacheCapabilities(getCacheCapabilities()).build();
+    var cacheCapabilities =
+        CacheCapabilities.newBuilder()
+            .setActionCacheUpdateCapabilities(
+                ActionCacheUpdateCapabilities.newBuilder().setUpdateEnabled(true).build())
+            .setSymlinkAbsolutePathStrategy(SymlinkAbsolutePathStrategy.Value.ALLOWED)
+            .build();
+    return ServerCapabilities.newBuilder().setCacheCapabilities(cacheCapabilities).build();
   }
 
   @Override

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
@@ -34,7 +34,9 @@
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
+import build.bazel.remote.execution.v2.ActionCacheUpdateCapabilities;
 import build.bazel.remote.execution.v2.ActionResult;
+import build.bazel.remote.execution.v2.CacheCapabilities;
 import build.bazel.remote.execution.v2.Digest;
 import build.bazel.remote.execution.v2.Directory;
 import build.bazel.remote.execution.v2.DirectoryNode;
@@ -48,6 +50,7 @@
 import build.bazel.remote.execution.v2.Platform;
 import build.bazel.remote.execution.v2.RequestMetadata;
 import build.bazel.remote.execution.v2.ServerCapabilities;
+import build.bazel.remote.execution.v2.SymlinkAbsolutePathStrategy;
 import build.bazel.remote.execution.v2.SymlinkNode;
 import build.bazel.remote.execution.v2.Tree;
 import com.google.common.base.Throwables;
@@ -155,17 +158,25 @@ public class RemoteExecutionServiceTest {
   private final Reporter reporter = new Reporter(new EventBus());
   private final StoredEventHandler eventHandler = new StoredEventHandler();
 
+  private final CacheCapabilities cacheCapabilities =
+      CacheCapabilities.newBuilder()
+          .setActionCacheUpdateCapabilities(
+              ActionCacheUpdateCapabilities.newBuilder().setUpdateEnabled(true).build())
+          .setSymlinkAbsolutePathStrategy(SymlinkAbsolutePathStrategy.Value.ALLOWED)
+          .build();
   // In the past, Bazel only supports RemoteApi version 2.0.
   // Use this to ensure we are backward compatible with Servers that only support 2.0.
   private final ServerCapabilities legacyRemoteExecutorCapabilities =
       ServerCapabilities.newBuilder()
+          .setCacheCapabilities(cacheCapabilities)
           .setLowApiVersion(ApiVersion.twoPointZero.toSemVer())
           .setHighApiVersion(ApiVersion.twoPointZero.toSemVer())
           .setExecutionCapabilities(ExecutionCapabilities.newBuilder().setExecEnabled(true).build())
           .build();
 
   private final ServerCapabilities remoteExecutorCapabilities =
       ServerCapabilities.newBuilder()
+          .setCacheCapabilities(cacheCapabilities)
           .setLowApiVersion(ApiVersion.low.toSemVer())
           .setHighApiVersion(ApiVersion.high.toSemVer())
           .setExecutionCapabilities(ExecutionCapabilities.newBuilder().setExecEnabled(true).build())

diff --git a/src/test/java/com/google/devtools/build/lib/remote/util/InMemoryCacheClient.java b/src/test/java/com/google/devtools/build/lib/remote/util/InMemoryCacheClient.java
@@ -109,18 +109,15 @@ public ListenableFuture<Void> downloadBlob(
     return Futures.immediateFuture(null);
   }
 
-  @Override
-  public CacheCapabilities getCacheCapabilities() {
-    return CacheCapabilities.newBuilder()
-        .setActionCacheUpdateCapabilities(
-            ActionCacheUpdateCapabilities.newBuilder().setUpdateEnabled(true).build())
-        .setSymlinkAbsolutePathStrategy(SymlinkAbsolutePathStrategy.Value.ALLOWED)
-        .build();
-  }
-
   @Override
   public ServerCapabilities getServerCapabilities() {
-    return ServerCapabilities.getDefaultInstance();
+    var cacheCapabilities =
+        CacheCapabilities.newBuilder()
+            .setActionCacheUpdateCapabilities(
+                ActionCacheUpdateCapabilities.newBuilder().setUpdateEnabled(true).build())
+            .setSymlinkAbsolutePathStrategy(SymlinkAbsolutePathStrategy.Value.ALLOWED)
+            .build();
+    return ServerCapabilities.newBuilder().setCacheCapabilities(cacheCapabilities).build();
   }
 
   @Override