Fix export of read-only repositories

- Fix startup issue with unfinished export of deleted repository
- Add withIgnoreReadOnly setting
- Build store with ignore read only flag
- Add logging

gradle/changelog/export-issues.yaml

@@ -0,0 +1,4 @@
+- type: fixed
+  description: Startup issues, if an unfinished export of a deleted repository exists
+- type: fixed
+  description: Archived repositories can now be exported

scm-core/src/main/java/sonia/scm/store/StoreParameters.java

@@ -28,9 +28,17 @@ public interface StoreParameters {
   String getRepositoryId();
 
   /**
-   * Returns optional namespace to which the store is related
+   * Returns optional namespace to which the store is related.
    * @return namespace
    * @since 2.44.0
    */
   String getNamespace();
+
+  /**
+   * Whether the store that is supposed to be built, should ignore that the repository is read-only or not.
+   * @return true if it should ignore the read-only property of a repository
+   */
+  default boolean isIgnoreReadOnly() {
+    return false;
+  }
 }

scm-core/src/main/java/sonia/scm/store/StoreParametersBuilder.java

@@ -46,6 +46,7 @@ public final class StoreParametersBuilder<S> {
     private final String name;
     private String repositoryId;
     private String namespace;
+    private boolean ignoreReadOnly;
   }
 
 
@@ -73,6 +74,20 @@ public final class StoreParametersBuilder<S> {
     return this;
   }
 
+  /**
+   * Use this to create or get a store, that can write data into a repository store,
+   * even if the repository itself is read-only.
+   * One use-case example is the 'ExportService' within scm-webapp.
+   * Only use this feature, if you are sure that your service needs to write data into a repository,
+   * even if it is read-only.
+   *
+   * @return Floating API to finish the call.
+   */
+  public StoreParametersBuilder<S> withReadOnlyIgnore() {
+    parameters.ignoreReadOnly = true;
+    return this;
+  }
+
   /**
    * Creates or gets the store with the given name and (if specified) the given repository. If no
    * repository is given, the store will be global.

scm-persistence/src/main/java/sonia/scm/store/file/FileBasedStoreFactory.java

@@ -16,7 +16,6 @@
 
 package sonia.scm.store.file;
 
-
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import sonia.scm.SCMContextProvider;
@@ -75,7 +74,23 @@ abstract class FileBasedStoreFactory {
   }
 
   protected boolean mustBeReadOnly(StoreParameters storeParameters) {
-    return storeParameters.getRepositoryId() != null && readOnlyChecker.isReadOnly(storeParameters.getRepositoryId());
+    if (storeParameters.isIgnoreReadOnly()) {
+      LOG.debug("ignoring if store should be readonly");
+      return false;
+    }
+
+    if (storeParameters.getRepositoryId() == null) {
+      LOG.debug("store cannot be readonly, because it is not built for a repository");
+      return false;
+    }
+
+    if (!readOnlyChecker.isReadOnly(storeParameters.getRepositoryId())) {
+      LOG.debug("created writeable store for repository {}", storeParameters.getRepositoryId());
+      return false;
+    }
+
+    LOG.debug("created readonly store for repository {}", storeParameters.getRepositoryId());
+    return true;
   }
 
   /**
@@ -92,7 +107,7 @@ abstract class FileBasedStoreFactory {
   /**
    * Get the store directory of a specific namespace
    *
-   * @param store        the type of the store
+   * @param store     the type of the store
    * @param namespace the name of the namespace
    * @return the store directory of a specific namespace
    */

scm-webapp/src/main/java/sonia/scm/importexport/ExportService.java

@@ -17,9 +17,11 @@
 package sonia.scm.importexport;
 
 import jakarta.inject.Inject;
+import lombok.extern.slf4j.Slf4j;
 import org.apache.shiro.SecurityUtils;
 import sonia.scm.NotFoundException;
 import sonia.scm.repository.Repository;
+import sonia.scm.repository.RepositoryManager;
 import sonia.scm.repository.RepositoryPermissions;
 import sonia.scm.repository.api.ExportFailedException;
 import sonia.scm.store.Blob;
@@ -37,10 +39,10 @@ import java.time.temporal.ChronoUnit;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
-import java.util.stream.Collectors;
 
 import static sonia.scm.ContextEntry.ContextBuilder.entity;
 
+@Slf4j
 public class ExportService {
 
   static final String STORE_NAME = "repository-export";
@@ -48,16 +50,23 @@ public class ExportService {
   private final DataStoreFactory dataStoreFactory;
   private final ExportFileExtensionResolver fileExtensionResolver;
   private final ExportNotificationHandler notificationHandler;
+  private final RepositoryManager repositoryManager;
 
   @Inject
-  public ExportService(BlobStoreFactory blobStoreFactory, DataStoreFactory dataStoreFactory, ExportFileExtensionResolver fileExtensionResolver, ExportNotificationHandler notificationHandler) {
+  public ExportService(BlobStoreFactory blobStoreFactory,
+                       DataStoreFactory dataStoreFactory,
+                       ExportFileExtensionResolver fileExtensionResolver,
+                       ExportNotificationHandler notificationHandler,
+                       RepositoryManager repositoryManager) {
     this.blobStoreFactory = blobStoreFactory;
     this.dataStoreFactory = dataStoreFactory;
     this.fileExtensionResolver = fileExtensionResolver;
     this.notificationHandler = notificationHandler;
+    this.repositoryManager = repositoryManager;
   }
 
   public OutputStream store(Repository repository, boolean withMetadata, boolean compressed, boolean encrypted) {
+    log.debug("Start storing export for repository {}",  repository);
     RepositoryPermissions.export(repository).check();
     storeExportInformation(repository.getId(), withMetadata, compressed, encrypted);
     try {
@@ -104,12 +113,14 @@ public class ExportService {
   }
 
   public void clear(String repositoryId) {
+    log.debug("Clearing export for repository {}",  repositoryId);
     RepositoryPermissions.export(repositoryId).check();
     createDataStore().remove(repositoryId);
     createBlobStore(repositoryId).clear();
   }
 
   public void setExportFinished(Repository repository) {
+    log.debug("Setting export as finished for repository {}",  repository);
     RepositoryPermissions.export(repository).check();
     DataStore<RepositoryExportInformation> dataStore = createDataStore();
     RepositoryExportInformation info = dataStore.get(repository.getId());
@@ -121,31 +132,48 @@ public class ExportService {
   public boolean isExporting(Repository repository) {
     RepositoryPermissions.export(repository).check();
     RepositoryExportInformation info = createDataStore().get(repository.getId());
-    return info != null && info.getStatus() == ExportStatus.EXPORTING;
+    boolean isExporting = info != null && info.getStatus() == ExportStatus.EXPORTING;
+
+    if (isExporting) {
+      log.debug("Repository {} is still exporting", repository);
+    }
+
+    return isExporting;
   }
 
   public void cleanupUnfinishedExports() {
     DataStore<RepositoryExportInformation> dataStore = createDataStore();
     List<Map.Entry<String, RepositoryExportInformation>> unfinishedExports = dataStore.getAll().entrySet().stream()
       .filter(e -> e.getValue().getStatus() == ExportStatus.EXPORTING)
-      .collect(Collectors.toList());
+      .toList();
 
     for (Map.Entry<String, RepositoryExportInformation> export : unfinishedExports) {
-      createBlobStore(export.getKey()).clear();
+      log.debug("Cleaning up export for repository {}",  export.getKey());
-      RepositoryExportInformation info = dataStore.get(export.getKey());
+      if (isRepositoryExisting(export.getKey())) {
-      info.setStatus(ExportStatus.INTERRUPTED);
+        createBlobStore(export.getKey()).clear();
-      dataStore.put(export.getKey(), info);
+        RepositoryExportInformation info = dataStore.get(export.getKey());
+        info.setStatus(ExportStatus.INTERRUPTED);
+        dataStore.put(export.getKey(), info);
+        log.debug("Export for repository {} has been cleaned up",  export.getKey());
+      } else {
+        dataStore.remove(export.getKey());
+        log.debug("Repository {} has already been deleted. Deleting dangling export.",  export.getKey());
+      }
     }
   }
 
   void cleanupOutdatedExports() {
+    log.debug("Cleaning up outdated exports");
     DataStore<RepositoryExportInformation> dataStore = createDataStore();
     List<String> outdatedExportIds = collectOutdatedExportIds(dataStore);
 
     for (String id : outdatedExportIds) {
       createBlobStore(id).clear();
+      log.debug("Cleaned up blob of outdated export for repository {}",  id);
     }
+
     outdatedExportIds.forEach(dataStore::remove);
+    log.debug("Cleaned up outdated exports");
   }
 
   private List<String> collectOutdatedExportIds(DataStore<RepositoryExportInformation> dataStore) {
@@ -190,6 +218,10 @@ public class ExportService {
   }
 
   private BlobStore createBlobStore(String repositoryId) {
-    return blobStoreFactory.withName(STORE_NAME).forRepository(repositoryId).build();
+    return blobStoreFactory.withName(STORE_NAME).forRepository(repositoryId).withReadOnlyIgnore().build();
+  }
+
+  private boolean isRepositoryExisting(String repositoryId) {
+    return this.repositoryManager.get(repositoryId) != null;
   }
 }

scm-webapp/src/test/java/sonia/scm/importexport/ExportServiceTest.java

@@ -28,8 +28,8 @@ import org.mockito.InjectMocks;
 import org.mockito.Mock;
 import org.mockito.junit.jupiter.MockitoExtension;
 import sonia.scm.NotFoundException;
-import sonia.scm.notifications.Type;
 import sonia.scm.repository.Repository;
+import sonia.scm.repository.RepositoryManager;
 import sonia.scm.repository.RepositoryTestData;
 import sonia.scm.store.Blob;
 import sonia.scm.store.BlobStore;
@@ -48,7 +48,6 @@ import java.util.List;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertThrows;
-import static org.mockito.ArgumentMatchers.argThat;
 import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.lenient;
 import static org.mockito.Mockito.mock;
@@ -73,6 +72,9 @@ class ExportServiceTest {
   @Mock
   private ExportNotificationHandler notificationHandler;
 
+  @Mock
+  private RepositoryManager repositoryManager;
+
   private BlobStore blobStore;
   private DataStore<RepositoryExportInformation> dataStore;
 
@@ -92,7 +94,7 @@ class ExportServiceTest {
     );
 
     blobStore = new InMemoryBlobStore();
-    when(blobStoreFactory.withName(STORE_NAME).forRepository(REPOSITORY.getId()).build())
+    when(blobStoreFactory.withName(STORE_NAME).forRepository(REPOSITORY.getId()).withReadOnlyIgnore().build())
       .thenReturn(blobStore);
 
     dataStore = new InMemoryDataStore<>();
@@ -205,6 +207,7 @@ class ExportServiceTest {
     );
     when(blobStoreFactory.withName(STORE_NAME).forRepository(finishedExport.getId()).build())
       .thenReturn(finishedExportBlobStore);
+    when(repositoryManager.get(REPOSITORY.getId())).thenReturn(REPOSITORY);
 
     exportService.cleanupUnfinishedExports();
 
@@ -214,6 +217,21 @@ class ExportServiceTest {
     assertThat(dataStore.get(finishedExport.getId()).getStatus()).isEqualTo(ExportStatus.FINISHED);
   }
 
+  @Test
+  void shouldDeleteUnfinishedExportsOfDeletedRepository() {
+    RepositoryExportInformation info = new RepositoryExportInformation();
+    info.setStatus(ExportStatus.EXPORTING);
+    dataStore.put(
+      REPOSITORY.getId(),
+      info
+    );
+    when(repositoryManager.get(REPOSITORY.getId())).thenReturn(null);
+
+    exportService.cleanupUnfinishedExports();
+
+    assertThat(dataStore.getOptional(REPOSITORY.getId())).isEmpty();
+  }
+
   @Test
   void shouldOnlyCleanupOutdatedExports() {
     blobStore.create(REPOSITORY.getId());
@@ -229,7 +247,7 @@ class ExportServiceTest {
     Instant old = Instant.now().minus(11, ChronoUnit.DAYS);
     oldExportInfo.setCreated(old);
     dataStore.put(oldExportRepo.getId(), oldExportInfo);
-    when(blobStoreFactory.withName(STORE_NAME).forRepository(oldExportRepo.getId()).build())
+    when(blobStoreFactory.withName(STORE_NAME).forRepository(oldExportRepo.getId()).withReadOnlyIgnore().build())
       .thenReturn(oldExportBlobStore);
 
     exportService.cleanupOutdatedExports();