diff --git a/gradle/changelog/export-issues.yaml b/gradle/changelog/export-issues.yaml new file mode 100644 index 0000000000..5c117c0a3c --- /dev/null +++ b/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 diff --git a/scm-core/src/main/java/sonia/scm/store/StoreParameters.java b/scm-core/src/main/java/sonia/scm/store/StoreParameters.java index c629ad51cd..8e6e6b9cd0 100644 --- a/scm-core/src/main/java/sonia/scm/store/StoreParameters.java +++ b/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; + } } diff --git a/scm-core/src/main/java/sonia/scm/store/StoreParametersBuilder.java b/scm-core/src/main/java/sonia/scm/store/StoreParametersBuilder.java index 3f1b18b667..982c476372 100644 --- a/scm-core/src/main/java/sonia/scm/store/StoreParametersBuilder.java +++ b/scm-core/src/main/java/sonia/scm/store/StoreParametersBuilder.java @@ -46,6 +46,7 @@ public final class StoreParametersBuilder { private final String name; private String repositoryId; private String namespace; + private boolean ignoreReadOnly; } @@ -73,6 +74,20 @@ public final class StoreParametersBuilder { 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 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. diff --git a/scm-persistence/src/main/java/sonia/scm/store/file/FileBasedStoreFactory.java b/scm-persistence/src/main/java/sonia/scm/store/file/FileBasedStoreFactory.java index 5ce74732bf..4c35f80cae 100644 --- a/scm-persistence/src/main/java/sonia/scm/store/file/FileBasedStoreFactory.java +++ b/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 */ diff --git a/scm-webapp/src/main/java/sonia/scm/importexport/ExportService.java b/scm-webapp/src/main/java/sonia/scm/importexport/ExportService.java index 8d8faf7493..c2e8b9db1a 100644 --- a/scm-webapp/src/main/java/sonia/scm/importexport/ExportService.java +++ b/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 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 dataStore = createDataStore(); List> unfinishedExports = dataStore.getAll().entrySet().stream() .filter(e -> e.getValue().getStatus() == ExportStatus.EXPORTING) - .collect(Collectors.toList()); + .toList(); for (Map.Entry export : unfinishedExports) { - createBlobStore(export.getKey()).clear(); - RepositoryExportInformation info = dataStore.get(export.getKey()); - info.setStatus(ExportStatus.INTERRUPTED); - dataStore.put(export.getKey(), info); + log.debug("Cleaning up export for repository {}", export.getKey()); + if (isRepositoryExisting(export.getKey())) { + createBlobStore(export.getKey()).clear(); + 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 dataStore = createDataStore(); List 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 collectOutdatedExportIds(DataStore 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; } } diff --git a/scm-webapp/src/test/java/sonia/scm/importexport/ExportServiceTest.java b/scm-webapp/src/test/java/sonia/scm/importexport/ExportServiceTest.java index 378a358c83..733d13cdb4 100644 --- a/scm-webapp/src/test/java/sonia/scm/importexport/ExportServiceTest.java +++ b/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 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();