Repository export read-only lock (#1519)

* Lock repository for read-only access only while exporting
* Create read-only check api

Co-authored-by: René Pfeuffer <rene.pfeuffer@cloudogu.com>
This commit is contained in:
Eduard Heimbuch
2021-02-04 15:29:49 +01:00
committed by GitHub
parent 04c6243f64
commit ac5d145266
54 changed files with 1104 additions and 182 deletions

View File

@@ -33,6 +33,7 @@ import sonia.scm.repository.InternalRepositoryException;
import sonia.scm.repository.NamespaceAndName;
import sonia.scm.repository.Repository;
import sonia.scm.repository.RepositoryDAO;
import sonia.scm.repository.RepositoryExportingCheck;
import sonia.scm.repository.RepositoryLocationResolver;
import sonia.scm.store.StoreReadOnlyException;
@@ -54,14 +55,16 @@ public class XmlRepositoryDAO implements RepositoryDAO {
private final PathBasedRepositoryLocationResolver repositoryLocationResolver;
private final FileSystem fileSystem;
private final RepositoryExportingCheck repositoryExportingCheck;
private final Map<String, Repository> byId;
private final Map<NamespaceAndName, Repository> byNamespaceAndName;
@Inject
public XmlRepositoryDAO(PathBasedRepositoryLocationResolver repositoryLocationResolver, FileSystem fileSystem) {
public XmlRepositoryDAO(PathBasedRepositoryLocationResolver repositoryLocationResolver, FileSystem fileSystem, RepositoryExportingCheck repositoryExportingCheck) {
this.repositoryLocationResolver = repositoryLocationResolver;
this.fileSystem = fileSystem;
this.repositoryExportingCheck = repositoryExportingCheck;
this.byId = new ConcurrentHashMap<>();
this.byNamespaceAndName = new ConcurrentHashMap<>();
@@ -140,7 +143,7 @@ public class XmlRepositoryDAO implements RepositoryDAO {
@Override
public void modify(Repository repository) {
Repository clone = repository.clone();
if (clone.isArchived() && byId.get(clone.getId()).isArchived()) {
if (mustNotModifyRepository(clone)) {
throw new StoreReadOnlyException(repository);
}
@@ -160,9 +163,14 @@ public class XmlRepositoryDAO implements RepositoryDAO {
metadataStore.write(repositoryPath, clone);
}
private boolean mustNotModifyRepository(Repository clone) {
return clone.isArchived() && byId.get(clone.getId()).isArchived()
|| repositoryExportingCheck.isExporting(clone);
}
@Override
public void delete(Repository repository) {
if (repository.isArchived()) {
if (repository.isArchived() || repositoryExportingCheck.isExporting(repository)) {
throw new StoreReadOnlyException(repository);
}
Path path;

View File

@@ -29,8 +29,8 @@ package sonia.scm.store;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import sonia.scm.SCMContextProvider;
import sonia.scm.repository.RepositoryArchivedCheck;
import sonia.scm.repository.RepositoryLocationResolver;
import sonia.scm.repository.RepositoryReadOnlyChecker;
import sonia.scm.util.IOUtil;
import java.io.File;
@@ -52,13 +52,13 @@ public abstract class FileBasedStoreFactory {
private final SCMContextProvider contextProvider;
private final RepositoryLocationResolver repositoryLocationResolver;
private final Store store;
private final RepositoryArchivedCheck archivedCheck;
private final RepositoryReadOnlyChecker readOnlyChecker;
protected FileBasedStoreFactory(SCMContextProvider contextProvider, RepositoryLocationResolver repositoryLocationResolver, Store store, RepositoryArchivedCheck archivedCheck) {
protected FileBasedStoreFactory(SCMContextProvider contextProvider, RepositoryLocationResolver repositoryLocationResolver, Store store, RepositoryReadOnlyChecker readOnlyChecker) {
this.contextProvider = contextProvider;
this.repositoryLocationResolver = repositoryLocationResolver;
this.store = store;
this.archivedCheck = archivedCheck;
this.readOnlyChecker = readOnlyChecker;
}
protected File getStoreLocation(StoreParameters storeParameters) {
@@ -83,12 +83,13 @@ public abstract class FileBasedStoreFactory {
}
protected boolean mustBeReadOnly(StoreParameters storeParameters) {
return storeParameters.getRepositoryId() != null && archivedCheck.isArchived(storeParameters.getRepositoryId());
return storeParameters.getRepositoryId() != null && readOnlyChecker.isReadOnly(storeParameters.getRepositoryId());
}
/**
* Get the store directory of a specific repository
* @param store the type of the store
*
* @param store the type of the store
* @param repositoryId the id of the repossitory
* @return the store directory of a specific repository
*/
@@ -98,6 +99,7 @@ public abstract class FileBasedStoreFactory {
/**
* Get the global store directory
*
* @param store the type of the store
* @return the global store directory
*/

View File

@@ -28,11 +28,9 @@ package sonia.scm.store;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import sonia.scm.SCMContextProvider;
import sonia.scm.repository.RepositoryArchivedCheck;
import sonia.scm.repository.RepositoryLocationResolver;
import sonia.scm.repository.RepositoryReadOnlyChecker;
import sonia.scm.security.KeyGenerator;
import sonia.scm.util.IOUtil;
@@ -46,11 +44,6 @@ import java.io.File;
@Singleton
public class FileBlobStoreFactory extends FileBasedStoreFactory implements BlobStoreFactory {
/**
* the logger for FileBlobStoreFactory
*/
private static final Logger LOG = LoggerFactory.getLogger(FileBlobStoreFactory.class);
private final KeyGenerator keyGenerator;
/**
@@ -60,8 +53,8 @@ public class FileBlobStoreFactory extends FileBasedStoreFactory implements BlobS
* @param keyGenerator key generator
*/
@Inject
public FileBlobStoreFactory(SCMContextProvider contextProvider , RepositoryLocationResolver repositoryLocationResolver, KeyGenerator keyGenerator, RepositoryArchivedCheck archivedCheck) {
super(contextProvider, repositoryLocationResolver, Store.BLOB, archivedCheck);
public FileBlobStoreFactory(SCMContextProvider contextProvider , RepositoryLocationResolver repositoryLocationResolver, KeyGenerator keyGenerator, RepositoryReadOnlyChecker readOnlyChecker) {
super(contextProvider, repositoryLocationResolver, Store.BLOB, readOnlyChecker);
this.keyGenerator = keyGenerator;
}

View File

@@ -29,8 +29,8 @@ package sonia.scm.store;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import sonia.scm.SCMContextProvider;
import sonia.scm.repository.RepositoryArchivedCheck;
import sonia.scm.repository.RepositoryLocationResolver;
import sonia.scm.repository.RepositoryReadOnlyChecker;
import sonia.scm.security.KeyGenerator;
//~--- JDK imports ------------------------------------------------------------
@@ -46,8 +46,8 @@ public class JAXBConfigurationEntryStoreFactory extends FileBasedStoreFactory
private KeyGenerator keyGenerator;
@Inject
public JAXBConfigurationEntryStoreFactory(SCMContextProvider contextProvider, RepositoryLocationResolver repositoryLocationResolver, KeyGenerator keyGenerator, RepositoryArchivedCheck archivedCheck) {
super(contextProvider, repositoryLocationResolver, Store.CONFIG, archivedCheck);
public JAXBConfigurationEntryStoreFactory(SCMContextProvider contextProvider, RepositoryLocationResolver repositoryLocationResolver, KeyGenerator keyGenerator, RepositoryReadOnlyChecker readOnlyChecker) {
super(contextProvider, repositoryLocationResolver, Store.CONFIG, readOnlyChecker);
this.keyGenerator = keyGenerator;
}

View File

@@ -27,8 +27,8 @@ package sonia.scm.store;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import sonia.scm.SCMContextProvider;
import sonia.scm.repository.RepositoryArchivedCheck;
import sonia.scm.repository.RepositoryLocationResolver;
import sonia.scm.repository.RepositoryReadOnlyChecker;
/**
* JAXB implementation of {@link ConfigurationStoreFactory}.
@@ -44,8 +44,8 @@ public class JAXBConfigurationStoreFactory extends FileBasedStoreFactory impleme
* @param repositoryLocationResolver Resolver to get the repository Directory
*/
@Inject
public JAXBConfigurationStoreFactory(SCMContextProvider contextProvider, RepositoryLocationResolver repositoryLocationResolver, RepositoryArchivedCheck archivedCheck) {
super(contextProvider, repositoryLocationResolver, Store.CONFIG, archivedCheck);
public JAXBConfigurationStoreFactory(SCMContextProvider contextProvider, RepositoryLocationResolver repositoryLocationResolver, RepositoryReadOnlyChecker readOnlyChecker) {
super(contextProvider, repositoryLocationResolver, Store.CONFIG, readOnlyChecker);
}
@Override

View File

@@ -29,8 +29,8 @@ package sonia.scm.store;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import sonia.scm.SCMContextProvider;
import sonia.scm.repository.RepositoryArchivedCheck;
import sonia.scm.repository.RepositoryLocationResolver;
import sonia.scm.repository.RepositoryReadOnlyChecker;
import sonia.scm.security.KeyGenerator;
import sonia.scm.util.IOUtil;
@@ -47,8 +47,8 @@ public class JAXBDataStoreFactory extends FileBasedStoreFactory
private final KeyGenerator keyGenerator;
@Inject
public JAXBDataStoreFactory(SCMContextProvider contextProvider , RepositoryLocationResolver repositoryLocationResolver, KeyGenerator keyGenerator, RepositoryArchivedCheck archivedCheck) {
super(contextProvider, repositoryLocationResolver, Store.DATA, archivedCheck);
public JAXBDataStoreFactory(SCMContextProvider contextProvider , RepositoryLocationResolver repositoryLocationResolver, KeyGenerator keyGenerator, RepositoryReadOnlyChecker readOnlyChecker) {
super(contextProvider, repositoryLocationResolver, Store.DATA, readOnlyChecker);
this.keyGenerator = keyGenerator;
}

View File

@@ -36,6 +36,7 @@ import sonia.scm.io.DefaultFileSystem;
import sonia.scm.io.FileSystem;
import sonia.scm.repository.InitialRepositoryLocationResolver;
import sonia.scm.repository.Repository;
import sonia.scm.repository.RepositoryExportingCheck;
import java.nio.file.Path;
import java.util.concurrent.ExecutorService;
@@ -54,6 +55,8 @@ class XmlRepositoryDAOSynchronizationTest {
@Mock
private SCMContextProvider provider;
@Mock
private RepositoryExportingCheck repositoryExportingCheck;
private FileSystem fileSystem;
private PathBasedRepositoryLocationResolver resolver;
@@ -75,7 +78,7 @@ class XmlRepositoryDAOSynchronizationTest {
provider, new InitialRepositoryLocationResolver(), fileSystem
);
repositoryDAO = new XmlRepositoryDAO(resolver, fileSystem);
repositoryDAO = new XmlRepositoryDAO(resolver, fileSystem, repositoryExportingCheck);
}
@Test
@@ -88,7 +91,7 @@ class XmlRepositoryDAOSynchronizationTest {
}
private void assertCreated() {
XmlRepositoryDAO assertionDao = new XmlRepositoryDAO(resolver, fileSystem);
XmlRepositoryDAO assertionDao = new XmlRepositoryDAO(resolver, fileSystem, repositoryExportingCheck);
assertThat(assertionDao.getAll()).hasSize(CREATION_COUNT);
}
@@ -97,7 +100,7 @@ class XmlRepositoryDAOSynchronizationTest {
void shouldCreateALotOfRepositoriesInParallel() throws InterruptedException {
ExecutorService executors = Executors.newFixedThreadPool(Runtime.getRuntime().availableProcessors());
final XmlRepositoryDAO repositoryDAO = new XmlRepositoryDAO(resolver, fileSystem);
final XmlRepositoryDAO repositoryDAO = new XmlRepositoryDAO(resolver, fileSystem, repositoryExportingCheck);
for (int i=0; i<CREATION_COUNT; i++) {
executors.submit(create(repositoryDAO, i));
}

View File

@@ -41,6 +41,7 @@ import sonia.scm.io.DefaultFileSystem;
import sonia.scm.io.FileSystem;
import sonia.scm.repository.NamespaceAndName;
import sonia.scm.repository.Repository;
import sonia.scm.repository.RepositoryExportingCheck;
import sonia.scm.repository.RepositoryLocationResolver;
import sonia.scm.repository.RepositoryPermission;
import sonia.scm.store.StoreReadOnlyException;
@@ -72,8 +73,10 @@ class XmlRepositoryDAOTest {
@Mock
private PathBasedRepositoryLocationResolver locationResolver;
private Consumer<BiConsumer<String, Path>> triggeredOnForAllLocations = none -> {};
@Mock
private RepositoryExportingCheck repositoryExportingCheck;
private FileSystem fileSystem = new DefaultFileSystem();
private final FileSystem fileSystem = new DefaultFileSystem();
private XmlRepositoryDAO dao;
@@ -120,7 +123,7 @@ class XmlRepositoryDAOTest {
@BeforeEach
void createDAO() {
dao = new XmlRepositoryDAO(locationResolver, fileSystem);
dao = new XmlRepositoryDAO(locationResolver, fileSystem, repositoryExportingCheck);
}
@Test
@@ -245,6 +248,15 @@ class XmlRepositoryDAOTest {
assertThrows(StoreReadOnlyException.class, () -> dao.modify(heartOfGold));
}
@Test
void shouldNotModifyExportingRepository() {
when(repositoryExportingCheck.isExporting(REPOSITORY)).thenReturn(true);
dao.add(REPOSITORY);
Repository heartOfGold = createRepository("42");
assertThrows(StoreReadOnlyException.class, () -> dao.modify(heartOfGold));
}
@Test
void shouldRemoveRepository() {
dao.add(REPOSITORY);
@@ -268,6 +280,15 @@ class XmlRepositoryDAOTest {
assertThrows(StoreReadOnlyException.class, () -> dao.delete(REPOSITORY));
}
@Test
void shouldNotRemoveExportingRepository() {
when(repositoryExportingCheck.isExporting(REPOSITORY)).thenReturn(true);
dao.add(REPOSITORY);
assertThat(dao.contains("42")).isTrue();
assertThrows(StoreReadOnlyException.class, () -> dao.delete(REPOSITORY));
}
@Test
void shouldRenameTheRepository() {
dao.add(REPOSITORY);
@@ -317,8 +338,9 @@ class XmlRepositoryDAOTest {
dao.add(REPOSITORY);
String content = getXmlFileContent(REPOSITORY.getId());
assertThat(content).containsSubsequence("trillian", "<verb>read</verb>", "<verb>write</verb>");
assertThat(content).containsSubsequence("vogons", "<verb>delete</verb>");
assertThat(content)
.containsSubsequence("trillian", "<verb>read</verb>", "<verb>write</verb>")
.containsSubsequence("vogons", "<verb>delete</verb>");
}
@Test
@@ -372,7 +394,7 @@ class XmlRepositoryDAOTest {
mockExistingPath();
// when
XmlRepositoryDAO dao = new XmlRepositoryDAO(locationResolver, fileSystem);
XmlRepositoryDAO dao = new XmlRepositoryDAO(locationResolver, fileSystem, repositoryExportingCheck);
// then
assertThat(dao.contains(new NamespaceAndName("space", "existing"))).isTrue();
@@ -383,7 +405,7 @@ class XmlRepositoryDAOTest {
// given
mockExistingPath();
XmlRepositoryDAO dao = new XmlRepositoryDAO(locationResolver, fileSystem);
XmlRepositoryDAO dao = new XmlRepositoryDAO(locationResolver, fileSystem, repositoryExportingCheck);
// when
dao.refresh();

View File

@@ -30,7 +30,7 @@ import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import sonia.scm.AbstractTestBase;
import sonia.scm.repository.Repository;
import sonia.scm.repository.RepositoryArchivedCheck;
import sonia.scm.repository.RepositoryReadOnlyChecker;
import sonia.scm.repository.RepositoryTestData;
import sonia.scm.security.UUIDKeyGenerator;
@@ -51,8 +51,8 @@ import static org.mockito.Mockito.when;
class FileBlobStoreTest extends AbstractTestBase
{
private Repository repository = RepositoryTestData.createHeartOfGold();
private RepositoryArchivedCheck archivedCheck = mock(RepositoryArchivedCheck.class);
private final Repository repository = RepositoryTestData.createHeartOfGold();
private final RepositoryReadOnlyChecker readOnlyChecker = mock(RepositoryReadOnlyChecker.class);
private BlobStore store;
@BeforeEach
@@ -191,7 +191,7 @@ class FileBlobStoreTest extends AbstractTestBase
@BeforeEach
void setRepositoryArchived() {
store.create("1"); // store for test must not be empty
when(archivedCheck.isArchived(repository.getId())).thenReturn(true);
when(readOnlyChecker.isReadOnly(repository.getId())).thenReturn(true);
createBlobStore();
}
@@ -227,6 +227,6 @@ class FileBlobStoreTest extends AbstractTestBase
protected BlobStoreFactory createBlobStoreFactory()
{
return new FileBlobStoreFactory(contextProvider, repositoryLocationResolver, new UUIDKeyGenerator(), archivedCheck);
return new FileBlobStoreFactory(contextProvider, repositoryLocationResolver, new UUIDKeyGenerator(), readOnlyChecker);
}
}

View File

@@ -26,7 +26,7 @@ package sonia.scm.store;
import org.junit.Test;
import sonia.scm.repository.Repository;
import sonia.scm.repository.RepositoryArchivedCheck;
import sonia.scm.repository.RepositoryReadOnlyChecker;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
@@ -41,17 +41,16 @@ import static org.mockito.Mockito.when;
*/
public class JAXBConfigurationStoreTest extends StoreTestBase {
private final RepositoryArchivedCheck archivedCheck = mock(RepositoryArchivedCheck.class);
private final RepositoryReadOnlyChecker readOnlyChecker = mock(RepositoryReadOnlyChecker.class);
@Override
protected ConfigurationStoreFactory createStoreFactory()
{
return new JAXBConfigurationStoreFactory(contextProvider, repositoryLocationResolver, archivedCheck);
return new JAXBConfigurationStoreFactory(contextProvider, repositoryLocationResolver, readOnlyChecker);
}
@Test
@SuppressWarnings("unchecked")
public void shouldStoreAndLoadInRepository()
{
Repository repository = new Repository("id", "git", "ns", "n");
@@ -70,17 +69,17 @@ public class JAXBConfigurationStoreTest extends StoreTestBase {
@Test
@SuppressWarnings("unchecked")
public void shouldNotWriteArchivedRepository()
{
Repository repository = new Repository("id", "git", "ns", "n");
when(archivedCheck.isArchived("id")).thenReturn(true);
when(readOnlyChecker.isReadOnly("id")).thenReturn(true);
ConfigurationStore<StoreObject> store = createStoreFactory()
.withType(StoreObject.class)
.withName("test")
.forRepository(repository)
.build();
assertThrows(RuntimeException.class, () -> store.set(new StoreObject("value")));
StoreObject storeObject = new StoreObject("value");
assertThrows(RuntimeException.class, () -> store.set(storeObject));
}
}

View File

@@ -28,7 +28,7 @@ package sonia.scm.store;
import org.junit.Test;
import sonia.scm.repository.Repository;
import sonia.scm.repository.RepositoryArchivedCheck;
import sonia.scm.repository.RepositoryReadOnlyChecker;
import sonia.scm.security.UUIDKeyGenerator;
import static org.junit.Assert.assertEquals;
@@ -42,12 +42,12 @@ import static org.mockito.Mockito.when;
*/
public class JAXBDataStoreTest extends DataStoreTestBase {
private final RepositoryArchivedCheck archivedCheck = mock(RepositoryArchivedCheck.class);
private final RepositoryReadOnlyChecker readOnlyChecker = mock(RepositoryReadOnlyChecker.class);
@Override
protected DataStoreFactory createDataStoreFactory()
{
return new JAXBDataStoreFactory(contextProvider, repositoryLocationResolver, new UUIDKeyGenerator(), archivedCheck);
return new JAXBDataStoreFactory(contextProvider, repositoryLocationResolver, new UUIDKeyGenerator(), readOnlyChecker);
}
@Override
@@ -80,7 +80,7 @@ public class JAXBDataStoreTest extends DataStoreTestBase {
@Test(expected = StoreReadOnlyException.class)
public void shouldNotStoreForReadOnlyRepository()
{
when(archivedCheck.isArchived(repository.getId())).thenReturn(true);
when(readOnlyChecker.isReadOnly(repository.getId())).thenReturn(true);
getDataStore(StoreObject.class, repository).put("abc", new StoreObject("abc_value"));
}
}

View File

@@ -29,7 +29,7 @@ import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
import sonia.scm.SCMContextProvider;
import sonia.scm.Stage;
import sonia.scm.repository.RepositoryArchivedCheck;
import sonia.scm.repository.RepositoryReadOnlyChecker;
import sonia.scm.security.KeyGenerator;
import sonia.scm.store.JAXBConfigurationEntryStoreFactory;
import sonia.scm.update.RepositoryV1PropertyReader;
@@ -111,8 +111,8 @@ class XmlV1PropertyDAOTest {
Files.createDirectories(configPath);
Path propFile = configPath.resolve("repository-properties-v1.xml");
Files.write(propFile, PROPERTIES.getBytes());
RepositoryArchivedCheck archivedCheck = mock(RepositoryArchivedCheck.class);
XmlV1PropertyDAO dao = new XmlV1PropertyDAO(new JAXBConfigurationEntryStoreFactory(new SimpleContextProvider(temp), null, new SimpleKeyGenerator(), archivedCheck));
RepositoryReadOnlyChecker readOnlyChecker = mock(RepositoryReadOnlyChecker.class);
XmlV1PropertyDAO dao = new XmlV1PropertyDAO(new JAXBConfigurationEntryStoreFactory(new SimpleContextProvider(temp), null, new SimpleKeyGenerator(), readOnlyChecker));
dao.getProperties(new RepositoryV1PropertyReader())
.forEachEntry((key, prop) -> {