From 38d0325b4c22d715d1c4211caf4b1e7589cc8e4e Mon Sep 17 00:00:00 2001 From: Laura Gorzitze Date: Mon, 24 Jun 2024 10:12:53 +0200 Subject: [PATCH] Check on duplicate name for rename Add check whether namespace and name already exist when renaming a repository. --- gradle/changelog/rename_check.yaml | 2 + .../scm/repository/xml/XmlRepositoryDAO.java | 9 ++++ .../repository/xml/XmlRepositoryDAOTest.java | 46 +++++++++++++++++-- .../repository/DefaultRepositoryManager.java | 9 +++- .../DefaultRepositoryManagerTest.java | 10 ++++ 5 files changed, 70 insertions(+), 6 deletions(-) create mode 100644 gradle/changelog/rename_check.yaml diff --git a/gradle/changelog/rename_check.yaml b/gradle/changelog/rename_check.yaml new file mode 100644 index 0000000000..178e148b82 --- /dev/null +++ b/gradle/changelog/rename_check.yaml @@ -0,0 +1,2 @@ +- type: fixed + description: Check for already existing Namespace and Name when renaming a repository diff --git a/scm-dao-xml/src/main/java/sonia/scm/repository/xml/XmlRepositoryDAO.java b/scm-dao-xml/src/main/java/sonia/scm/repository/xml/XmlRepositoryDAO.java index b9d69c7910..7a4c36826a 100644 --- a/scm-dao-xml/src/main/java/sonia/scm/repository/xml/XmlRepositoryDAO.java +++ b/scm-dao-xml/src/main/java/sonia/scm/repository/xml/XmlRepositoryDAO.java @@ -28,6 +28,8 @@ package sonia.scm.repository.xml; import com.google.common.collect.ImmutableList; import com.google.inject.Singleton; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import sonia.scm.io.FileSystem; import sonia.scm.repository.InternalRepositoryException; import sonia.scm.repository.NamespaceAndName; @@ -67,6 +69,9 @@ public class XmlRepositoryDAO implements RepositoryDAO { private final Map byNamespaceAndName; private final ReadWriteLock byNamespaceLock = new ReentrantReadWriteLock(); + private static final Logger LOG = LoggerFactory.getLogger(XmlRepositoryDAO.class); + + @Inject public XmlRepositoryDAO(PathBasedRepositoryLocationResolver repositoryLocationResolver, FileSystem fileSystem, RepositoryExportingCheck repositoryExportingCheck) { this.repositoryLocationResolver = repositoryLocationResolver; @@ -84,6 +89,10 @@ public class XmlRepositoryDAO implements RepositoryDAO { RepositoryLocationResolver.RepositoryLocationResolverInstance pathRepositoryLocationResolverInstance = repositoryLocationResolver.create(Path.class); pathRepositoryLocationResolverInstance.forAllLocations((repositoryId, repositoryPath) -> { Repository repository = metadataStore.read(repositoryPath); + if (byNamespaceAndName.containsKey(repository.getNamespaceAndName())) { + LOG.warn("Duplicate repository found. Adding suffix DUPLICATE to repository {}", repository); + repository.setName(repository.getName() + "-" + repositoryId + "-DUPLICATE"); + } byNamespaceAndName.put(repository.getNamespaceAndName(), repository); byId.put(repositoryId, repository); }); diff --git a/scm-dao-xml/src/test/java/sonia/scm/repository/xml/XmlRepositoryDAOTest.java b/scm-dao-xml/src/test/java/sonia/scm/repository/xml/XmlRepositoryDAOTest.java index a388621e31..05be72e3bb 100644 --- a/scm-dao-xml/src/test/java/sonia/scm/repository/xml/XmlRepositoryDAOTest.java +++ b/scm-dao-xml/src/test/java/sonia/scm/repository/xml/XmlRepositoryDAOTest.java @@ -83,7 +83,7 @@ class XmlRepositoryDAOTest { @BeforeEach void createDAO(@TempDir Path basePath) { when(locationResolver.create(Path.class)).thenReturn( - new RepositoryLocationResolver.RepositoryLocationResolverInstance() { + new RepositoryLocationResolver.RepositoryLocationResolverInstance<>() { @Override public Path getLocation(String repositoryId) { return locationResolver.create(repositoryId); @@ -367,7 +367,7 @@ class XmlRepositoryDAOTest { private String content(Path storePath) { try { - return new String(Files.readAllBytes(storePath), Charsets.UTF_8); + return Files.readString(storePath, Charsets.UTF_8); } catch (IOException e) { throw new RuntimeException(e); } @@ -383,9 +383,7 @@ class XmlRepositoryDAOTest { void createMetadataFileForRepository(@TempDir Path basePath) throws IOException { repositoryPath = basePath.resolve("existing"); - Files.createDirectories(repositoryPath); - URL metadataUrl = Resources.getResource("sonia/scm/store/repositoryDaoMetadata.xml"); - Files.copy(metadataUrl.openStream(), repositoryPath.resolve("metadata.xml")); + prepareRepositoryPath(repositoryPath); } @Test @@ -420,6 +418,44 @@ class XmlRepositoryDAOTest { } } + @Nested + class WithDuplicateRepositories { + private Path repositoryPath; + private Path duplicateRepositoryPath; + + @BeforeEach + void createMetadataFileForRepository(@TempDir Path basePath) throws IOException { + repositoryPath = basePath.resolve("existing"); + duplicateRepositoryPath = basePath.resolve("duplicate"); + + prepareRepositoryPath(repositoryPath); + prepareRepositoryPath(duplicateRepositoryPath); + } + + @Test + void shouldRenameDuplicateRepositories() { + mockExistingPath(); + + XmlRepositoryDAO dao = new XmlRepositoryDAO(locationResolver, fileSystem, repositoryExportingCheck); + + assertThat(dao.contains(new NamespaceAndName("space", "existing"))).isTrue(); + assertThat(dao.contains(new NamespaceAndName("space", "existing-existing2-DUPLICATE"))).isTrue(); + } + + private void mockExistingPath() { + triggeredOnForAllLocations = consumer -> { + consumer.accept("existing", repositoryPath); + consumer.accept("existing2", duplicateRepositoryPath); + }; + } + } + + private void prepareRepositoryPath(Path repositoryPath) throws IOException { + Files.createDirectories(repositoryPath); + URL metadataUrl = Resources.getResource("sonia/scm/store/repositoryDaoMetadata.xml"); + Files.copy(metadataUrl.openStream(), repositoryPath.resolve("metadata.xml")); + } + private Repository createRepository(String id) { return new Repository(id, "xml", "space", id); } diff --git a/scm-webapp/src/main/java/sonia/scm/repository/DefaultRepositoryManager.java b/scm-webapp/src/main/java/sonia/scm/repository/DefaultRepositoryManager.java index 65c6da4115..253ee43f91 100644 --- a/scm-webapp/src/main/java/sonia/scm/repository/DefaultRepositoryManager.java +++ b/scm-webapp/src/main/java/sonia/scm/repository/DefaultRepositoryManager.java @@ -30,6 +30,7 @@ import com.google.inject.Inject; import com.google.inject.Singleton; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import sonia.scm.AlreadyExistsException; import sonia.scm.ConfigurationException; import sonia.scm.HandlerEventType; import sonia.scm.ManagerDaoAdapter; @@ -183,7 +184,7 @@ public class DefaultRepositoryManager extends AbstractRepositoryManager { }, newRepository -> { if (repositoryDAO.contains(newRepository.getNamespaceAndName())) { - throw alreadyExists(entity(newRepository.getClass(), newRepository.getNamespaceAndName().logString())); + throw alreadyExists(entity(newRepository.getNamespaceAndName())); } } ); @@ -292,10 +293,16 @@ public class DefaultRepositoryManager extends AbstractRepositoryManager { public Repository rename(Repository repository, String newNamespace, String newName) { + NamespaceAndName newNamespaceAndName = new NamespaceAndName(newNamespace, newName); + if (hasNamespaceOrNameNotChanged(repository, newNamespace, newName)) { throw new NoChangesMadeException(repository); } + if (this.get(newNamespaceAndName) != null){ + throw AlreadyExistsException.alreadyExists(entity(NamespaceAndName.class, newNamespaceAndName.logString())); + } + Repository changedRepository = repository.clone(); if (!Strings.isNullOrEmpty(newName)) { changedRepository.setName(newName); diff --git a/scm-webapp/src/test/java/sonia/scm/repository/DefaultRepositoryManagerTest.java b/scm-webapp/src/test/java/sonia/scm/repository/DefaultRepositoryManagerTest.java index dff97c98cd..1c479640c7 100644 --- a/scm-webapp/src/test/java/sonia/scm/repository/DefaultRepositoryManagerTest.java +++ b/scm-webapp/src/test/java/sonia/scm/repository/DefaultRepositoryManagerTest.java @@ -457,6 +457,16 @@ public class DefaultRepositoryManagerTest extends ManagerTestBase { assertEquals("hitchhiker", changedRepo.getNamespace()); } + @Test + public void shouldNotRenameRepositoryIfNameOrNamespaceAlreadyInUse() { + Repository repository1 = createTestRepository(); + Repository repository2 = createSecondTestRepository(); + RepositoryManager repoManager = (RepositoryManager) manager; + + assertThrows(AlreadyExistsException.class, () -> repoManager.rename(repository2, repository1.getNamespace(), repository1.getName())); + + } + @Test public void shouldReturnDistinctNamespaces() { createTestRepository();