remote: moved SimpleBlobStore creation logic into the Factory

Part of #7205

Closes #7852.

PiperOrigin-RevId: 240972987

src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java

diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java
index 535eb0a..16ffb84 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java
@@ -248,7 +248,7 @@
                 SimpleBlobStoreFactory.create(
                     remoteOptions,
                     GoogleAuthUtils.newCredentials(authAndTlsOptions),
-                    env.getWorkingDirectory()),
+                    Preconditions.checkNotNull(env.getWorkingDirectory(), "workingDirectory")),
                 digestUtil);
       }
 

src/main/java/com/google/devtools/build/lib/remote/SimpleBlobStoreFactory.java

diff --git a/src/main/java/com/google/devtools/build/lib/remote/SimpleBlobStoreFactory.java b/src/main/java/com/google/devtools/build/lib/remote/SimpleBlobStoreFactory.java
index a362f74..c592691 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/SimpleBlobStoreFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/SimpleBlobStoreFactory.java
@@ -14,10 +14,10 @@
 
 package com.google.devtools.build.lib.remote;
 
-import static com.google.common.base.Preconditions.checkNotNull;
-
 import com.google.auth.Credentials;
+import com.google.common.base.Preconditions;
 import com.google.devtools.build.lib.remote.blobstore.CombinedDiskHttpBlobStore;
+import com.google.devtools.build.lib.remote.blobstore.ConcurrentMapBlobStore;
 import com.google.devtools.build.lib.remote.blobstore.OnDiskBlobStore;
 import com.google.devtools.build.lib.remote.blobstore.SimpleBlobStore;
 import com.google.devtools.build.lib.remote.blobstore.http.HttpBlobStore;
@@ -27,6 +27,7 @@
 import io.netty.channel.unix.DomainSocketAddress;
 import java.io.IOException;
 import java.net.URI;
+import java.util.concurrent.ConcurrentHashMap;
 import javax.annotation.Nullable;
 
 /**
@@ -37,19 +38,28 @@
 
   private SimpleBlobStoreFactory() {}
 
-  // TODO(ishikhman): make workingDirectory not nullable
+  public static SimpleBlobStore create(RemoteOptions remoteOptions, @Nullable Path casPath) {
+    if (isHttpUrlOptions(remoteOptions)) {
+      return createHttp(remoteOptions, /* creds= */ null);
+    } else if (casPath != null) {
+      return new OnDiskBlobStore(casPath);
+    } else {
+      return new ConcurrentMapBlobStore(new ConcurrentHashMap<>());
+    }
+  }
+
   public static SimpleBlobStore create(
-      RemoteOptions options, @Nullable Credentials creds, @Nullable Path workingDirectory)
+      RemoteOptions options, @Nullable Credentials creds, Path workingDirectory)
       throws IOException {
 
+    Preconditions.checkNotNull(workingDirectory, "workingDirectory");
     if (isHttpUrlOptions(options) && isDiskCache(options)) {
       return createCombinedCache(workingDirectory, options.diskCache, options, creds);
     }
     if (isHttpUrlOptions(options)) {
       return createHttp(options, creds);
     }
-    // TODO(ishikhman): check this condition when making workingDirectory not nullable
-    if (workingDirectory != null && isDiskCache(options)) {
+    if (isDiskCache(options)) {
       return createDiskCache(workingDirectory, options.diskCache);
     }
     throw new IllegalArgumentException(
@@ -87,7 +97,8 @@
 
   private static SimpleBlobStore createDiskCache(Path workingDirectory, PathFragment diskCachePath)
       throws IOException {
-    Path cacheDir = workingDirectory.getRelative(checkNotNull(diskCachePath));
+    Path cacheDir =
+        workingDirectory.getRelative(Preconditions.checkNotNull(diskCachePath, "diskCachePath"));
     if (!cacheDir.exists()) {
       cacheDir.createDirectoryAndParents();
     }
@@ -97,7 +108,8 @@
   private static SimpleBlobStore createCombinedCache(
       Path workingDirectory, PathFragment diskCachePath, RemoteOptions options, Credentials cred)
       throws IOException {
-    Path cacheDir = workingDirectory.getRelative(checkNotNull(diskCachePath));
+    Path cacheDir =
+        workingDirectory.getRelative(Preconditions.checkNotNull(diskCachePath, "diskCachePath"));
     if (!cacheDir.exists()) {
       cacheDir.createDirectoryAndParents();
     }

src/test/java/com/google/devtools/build/lib/remote/SimpleBlobStoreFactoryTest.java

diff --git a/src/test/java/com/google/devtools/build/lib/remote/SimpleBlobStoreFactoryTest.java b/src/test/java/com/google/devtools/build/lib/remote/SimpleBlobStoreFactoryTest.java
index faf5629..c4d3c6c 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/SimpleBlobStoreFactoryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/SimpleBlobStoreFactoryTest.java
@@ -19,6 +19,7 @@
 
 import com.google.devtools.build.lib.clock.JavaClock;
 import com.google.devtools.build.lib.remote.blobstore.CombinedDiskHttpBlobStore;
+import com.google.devtools.build.lib.remote.blobstore.ConcurrentMapBlobStore;
 import com.google.devtools.build.lib.remote.blobstore.OnDiskBlobStore;
 import com.google.devtools.build.lib.remote.blobstore.SimpleBlobStore;
 import com.google.devtools.build.lib.remote.blobstore.http.HttpBlobStore;
@@ -169,4 +170,70 @@
   public void isRemoteCacheOptions_defaultOptions() {
     assertThat(SimpleBlobStoreFactory.isRemoteCacheOptions(remoteOptions)).isFalse();
   }
+
+  @Test
+  public void create_httpCacheWhenHttpAndDiskCacheEnabled() {
+    remoteOptions.remoteHttpCache = "http://doesnotexist.com";
+    remoteOptions.diskCache = PathFragment.create("/etc/something/cache/here");
+
+    SimpleBlobStore blobStore = SimpleBlobStoreFactory.create(remoteOptions, /* casPath= */ null);
+
+    assertThat(blobStore).isInstanceOf(HttpBlobStore.class);
+  }
+
+  @Test
+  public void create_httpCacheWithProxy() {
+    remoteOptions.remoteHttpCache = "http://doesnotexist.com";
+    remoteOptions.remoteCacheProxy = "unix://some-proxy";
+
+    SimpleBlobStore blobStore = SimpleBlobStoreFactory.create(remoteOptions, /* casPath= */ null);
+
+    assertThat(blobStore).isInstanceOf(HttpBlobStore.class);
+  }
+
+  @Test
+  public void create_httpCacheFailsWithUnsupportedProxyProtocol() {
+    remoteOptions.remoteHttpCache = "http://doesnotexist.com";
+    remoteOptions.remoteCacheProxy = "bad-proxy";
+
+    assertThat(
+            assertThrows(
+                Exception.class,
+                () -> SimpleBlobStoreFactory.create(remoteOptions, /* casPath= */ null)))
+        .hasMessageThat()
+        .contains("Remote cache proxy unsupported: bad-proxy");
+  }
+
+  @Test
+  public void create_httpCacheWithoutProxy() {
+    remoteOptions.remoteHttpCache = "http://doesnotexist.com";
+
+    SimpleBlobStore blobStore = SimpleBlobStoreFactory.create(remoteOptions, /* casPath= */ null);
+
+    assertThat(blobStore).isInstanceOf(HttpBlobStore.class);
+  }
+
+  @Test
+  public void create_diskCacheWithCasPath() {
+    SimpleBlobStore blobStore =
+        SimpleBlobStoreFactory.create(remoteOptions, fs.getPath("/cas/path/is/here"));
+
+    assertThat(blobStore).isInstanceOf(OnDiskBlobStore.class);
+  }
+
+  @Test
+  public void create_defaultCacheWhenDiskCacheEnabled() {
+    remoteOptions.diskCache = PathFragment.create("/etc/something/cache/here");
+
+    SimpleBlobStore blobStore = SimpleBlobStoreFactory.create(remoteOptions, /* casPath= */ null);
+
+    assertThat(blobStore).isInstanceOf(ConcurrentMapBlobStore.class);
+  }
+
+  @Test
+  public void create_defaultCache() {
+    SimpleBlobStore blobStore = SimpleBlobStoreFactory.create(remoteOptions, null);
+
+    assertThat(blobStore).isInstanceOf(ConcurrentMapBlobStore.class);
+  }
 }

src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/RemoteWorker.java

diff --git a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/RemoteWorker.java b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/RemoteWorker.java
index 7490200..7a5a7b1 100644
--- a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/RemoteWorker.java
+++ b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/RemoteWorker.java
@@ -33,8 +33,6 @@
 import com.google.common.util.concurrent.MoreExecutors;
 import com.google.devtools.build.lib.remote.SimpleBlobStoreActionCache;
 import com.google.devtools.build.lib.remote.SimpleBlobStoreFactory;
-import com.google.devtools.build.lib.remote.blobstore.ConcurrentMapBlobStore;
-import com.google.devtools.build.lib.remote.blobstore.OnDiskBlobStore;
 import com.google.devtools.build.lib.remote.blobstore.SimpleBlobStore;
 import com.google.devtools.build.lib.remote.options.RemoteOptions;
 import com.google.devtools.build.lib.remote.util.DigestUtil;
@@ -243,17 +241,9 @@
       return;
     }
 
-    // The instance of SimpleBlobStore used is based on these criteria in order:
-    // 1. If remote cache or local disk cache is specified then use it first.
-    // 2. Finally use a ConcurrentMap to back the blob store.
-    final SimpleBlobStore blobStore;
-    if (usingRemoteCache) {
-      blobStore = SimpleBlobStoreFactory.create(remoteOptions, null, null);
-    } else if (remoteWorkerOptions.casPath != null) {
-      blobStore = new OnDiskBlobStore(fs.getPath(remoteWorkerOptions.casPath));
-    } else {
-      blobStore = new ConcurrentMapBlobStore(new ConcurrentHashMap<>());
-    }
+    Path casPath =
+        remoteWorkerOptions.casPath != null ? fs.getPath(remoteWorkerOptions.casPath) : null;
+    final SimpleBlobStore blobStore = SimpleBlobStoreFactory.create(remoteOptions, casPath);
 
     ListeningScheduledExecutorService retryService =
         MoreExecutors.listeningDecorator(Executors.newScheduledThreadPool(1));