Merge pull request #12882 from overleaf/em-git-bridge-swap-job-config

Disable swap job when swap store is unsafe

GitOrigin-RevId: 104b03e378e7802f9ba6ff96b5a626a2fa7960fb

services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/swap/job/SwapJob.java

@@ -4,6 +4,7 @@ import uk.ac.ic.wlgitbridge.bridge.db.DBStore;
 import uk.ac.ic.wlgitbridge.bridge.lock.ProjectLock;
 import uk.ac.ic.wlgitbridge.bridge.repo.RepoStore;
 import uk.ac.ic.wlgitbridge.bridge.swap.store.SwapStore;
+import uk.ac.ic.wlgitbridge.util.Log;
 
 import java.io.IOException;
 import java.util.Optional;
@@ -60,16 +61,20 @@ public interface SwapJob {
             DBStore dbStore,
             SwapStore swapStore
     ) {
-        if (cfg.isPresent()) {
-            return new SwapJobImpl(
-                    cfg.get(),
-                    lock,
-                    repoStore,
-                    dbStore,
-                    swapStore
-            );
+        if (!cfg.isPresent()) {
+            return new NoopSwapJob();
         }
-        return new NoopSwapJob();
+        if (!swapStore.isSafe()) {
+            Log.warn("Swap store '{}' is not safe; disabling swap job", swapStore.getClass().getSimpleName());
+            return new NoopSwapJob();
+        }
+        return new SwapJobImpl(
+                cfg.get(),
+                lock,
+                repoStore,
+                dbStore,
+                swapStore
+        );
     }
 
     /**

services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/swap/store/InMemorySwapStore.java

@@ -51,4 +51,8 @@ public class InMemorySwapStore implements SwapStore {
         store.remove(projectName);
     }
 
+    @Override
+    public boolean isSafe() {
+      return false;
+    }
 }

services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/swap/store/NoopSwapStore.java

@@ -25,4 +25,8 @@ public class NoopSwapStore implements SwapStore {
     @Override
     public void remove(String projectName) {}
 
+    @Override
+    public boolean isSafe() {
+      return false;
+    }
 }

services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/swap/store/S3SwapStore.java

@@ -83,4 +83,8 @@ public class S3SwapStore implements SwapStore {
         s3.deleteObject(del);
     }
 
+    @Override
+    public boolean isSafe() {
+      return true;
+    }
 }

services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/swap/store/SwapStore.java

@@ -41,4 +41,11 @@ public interface SwapStore {
 
     void remove(String projectName);
 
+    /**
+     * Returns true if the swap store safely persists swapped projects.
+     *
+     * Fake swap stores should return false.
+     */
+    boolean isSafe();
+
 }

services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java

@@ -21,6 +21,7 @@ import org.asynchttpclient.*;
 import org.eclipse.jgit.api.errors.GitAPIException;
 import org.junit.After;
 import org.junit.Before;
+import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
@@ -657,6 +658,11 @@ public class WLGitBridgeIntegrationTest {
 
         }
 
+    // We skip this test because it now hangs. It relies on the in-memory swap
+    // job, but we disable the swap job when the noop or in-memory swap store
+    // are configured. We do that for safety. Such a configuration in a
+    // production environment would lead to data loss.
+    @Ignore
     @Test
     public void wlgbCanSwapProjects(
     ) throws IOException, GitAPIException, InterruptedException {