From 77a1ad50fea1758b8a00ab15c79752f6ef9b5fa1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Thu, 6 Jun 2019 10:45:56 +0200 Subject: [PATCH] Refresh repository dao after repository.xml file was renamed Without this, the XmlRepositoryDAO will be initialized at a time where there is no repository-paths.xml file. Therefore the dao cannot initialize with the existing repositories whose paths are kept in repositories.xml at that time. In this commit we trigger a refresh after the file was renamed, so that the PathBasedRepositoryLocationResolver can read the moved repository-paths.xml file and all repositories will be found. --- .../PathBasedRepositoryLocationResolver.java | 8 +- .../scm/repository/xml/XmlRepositoryDAO.java | 7 ++ .../repository/xml/XmlRepositoryDAOTest.java | 104 ++++++++++++------ .../XmlRepositoryFileNameUpdateStep.java | 6 +- .../XmlRepositoryFileNameUpdateStepTest.java | 9 +- 5 files changed, 94 insertions(+), 40 deletions(-) diff --git a/scm-dao-xml/src/main/java/sonia/scm/repository/xml/PathBasedRepositoryLocationResolver.java b/scm-dao-xml/src/main/java/sonia/scm/repository/xml/PathBasedRepositoryLocationResolver.java index a1ce516069..bfb59f9f35 100644 --- a/scm-dao-xml/src/main/java/sonia/scm/repository/xml/PathBasedRepositoryLocationResolver.java +++ b/scm-dao-xml/src/main/java/sonia/scm/repository/xml/PathBasedRepositoryLocationResolver.java @@ -7,6 +7,7 @@ import sonia.scm.repository.InternalRepositoryException; import sonia.scm.store.StoreConstants; import javax.inject.Inject; +import javax.inject.Singleton; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; @@ -28,6 +29,7 @@ import static sonia.scm.ContextEntry.ContextBuilder.entity; * * @since 2.0.0 */ +@Singleton public class PathBasedRepositoryLocationResolver extends BasicRepositoryLocationResolver { public static final String STORE_NAME = "repository-paths"; @@ -48,7 +50,7 @@ public class PathBasedRepositoryLocationResolver extends BasicRepositoryLocation this(contextProvider, initialRepositoryLocationResolver, Clock.systemUTC()); } - public PathBasedRepositoryLocationResolver(SCMContextProvider contextProvider, InitialRepositoryLocationResolver initialRepositoryLocationResolver, Clock clock) { + PathBasedRepositoryLocationResolver(SCMContextProvider contextProvider, InitialRepositoryLocationResolver initialRepositoryLocationResolver, Clock clock) { super(Path.class); this.contextProvider = contextProvider; this.initialRepositoryLocationResolver = initialRepositoryLocationResolver; @@ -138,4 +140,8 @@ public class PathBasedRepositoryLocationResolver extends BasicRepositoryLocation .resolve(StoreConstants.CONFIG_DIRECTORY_NAME) .resolve(STORE_NAME.concat(StoreConstants.FILE_EXTENSION)); } + + public void refresh() { + this.read(); + } } 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 f506793d1a..151e8f1281 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 @@ -198,4 +198,11 @@ public class XmlRepositoryDAO implements RepositoryDAO { public Long getLastModified() { return repositoryLocationResolver.getLastModified(); } + + public void refresh() { + repositoryLocationResolver.refresh(); + byNamespaceAndName.clear(); + byId.clear(); + init(); + } } 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 5b9a00aec8..d77bfb3c63 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 @@ -8,8 +8,6 @@ import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.junitpioneer.jupiter.TempDirectory; -import org.mockito.ArgumentCaptor; -import org.mockito.Captor; import org.mockito.Mock; import org.mockito.invocation.InvocationOnMock; import org.mockito.junit.jupiter.MockitoExtension; @@ -32,7 +30,9 @@ import static java.util.Arrays.asList; import static java.util.Collections.singletonList; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.fail; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; @@ -47,9 +47,6 @@ class XmlRepositoryDAOTest { @Mock private PathBasedRepositoryLocationResolver locationResolver; - @Captor - private ArgumentCaptor> forAllCaptor; - private FileSystem fileSystem = new DefaultFileSystem(); private XmlRepositoryDAO dao; @@ -268,43 +265,80 @@ class XmlRepositoryDAOTest { verify(locationResolver).updateModificationDate(); } - } - @Test - void shouldReadExistingRepositoriesFromPathDatabase(@TempDirectory.TempDir Path basePath) throws IOException { - doNothing().when(locationResolver).forAllPaths(forAllCaptor.capture()); - XmlRepositoryDAO dao = new XmlRepositoryDAO(locationResolver, fileSystem); + private String getXmlFileContent(String id) { + Path storePath = metadataFile(id); - Path repositoryPath = basePath.resolve("existing"); - Files.createDirectories(repositoryPath); - URL metadataUrl = Resources.getResource("sonia/scm/store/repositoryDaoMetadata.xml"); - Files.copy(metadataUrl.openStream(), repositoryPath.resolve("metadata.xml")); + assertThat(storePath).isRegularFile(); + return content(storePath); + } - forAllCaptor.getValue().accept("existing", repositoryPath); + private Path metadataFile(String id) { + return locationResolver.create(id).resolve("metadata.xml"); + } - assertThat(dao.contains(new NamespaceAndName("space", "existing"))).isTrue(); - } - - private String getXmlFileContent(String id) { - Path storePath = metadataFile(id); - - assertThat(storePath).isRegularFile(); - return content(storePath); - } - - private Path metadataFile(String id) { - return locationResolver.create(id).resolve("metadata.xml"); - } - - private String content(Path storePath) { - try { - return new String(Files.readAllBytes(storePath), Charsets.UTF_8); - } catch (IOException e) { - throw new RuntimeException(e); + private String content(Path storePath) { + try { + return new String(Files.readAllBytes(storePath), Charsets.UTF_8); + } catch (IOException e) { + throw new RuntimeException(e); + } } } - private static Repository createRepository(String id) { + @Nested + class WithExistingRepositories { + + private Path repositoryPath; + + @BeforeEach + void createMetadataFileForRepository(@TempDirectory.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")); + } + + @Test + void shouldReadExistingRepositoriesFromPathDatabase() { + // given + mockExistingPath(); + + // when + XmlRepositoryDAO dao = new XmlRepositoryDAO(locationResolver, fileSystem); + + // then + assertThat(dao.contains(new NamespaceAndName("space", "existing"))).isTrue(); + } + + @Test + void shouldRefreshWithExistingRepositoriesFromPathDatabase() { + // given + doNothing().when(locationResolver).forAllPaths(any()); + XmlRepositoryDAO dao = new XmlRepositoryDAO(locationResolver, fileSystem); + + mockExistingPath(); + + // when + dao.refresh(); + + // then + verify(locationResolver).refresh(); + assertThat(dao.contains(new NamespaceAndName("space", "existing"))).isTrue(); + } + + private void mockExistingPath() { + doAnswer( + invocation -> { + ((BiConsumer) invocation.getArgument(0)).accept("existing", repositoryPath); + return null; + } + ).when(locationResolver).forAllPaths(any()); + } + } + + private Repository createRepository(String id) { return new Repository(id, "xml", "space", id); } } diff --git a/scm-webapp/src/main/java/sonia/scm/update/repository/XmlRepositoryFileNameUpdateStep.java b/scm-webapp/src/main/java/sonia/scm/update/repository/XmlRepositoryFileNameUpdateStep.java index 62902713f0..9df6f81440 100644 --- a/scm-webapp/src/main/java/sonia/scm/update/repository/XmlRepositoryFileNameUpdateStep.java +++ b/scm-webapp/src/main/java/sonia/scm/update/repository/XmlRepositoryFileNameUpdateStep.java @@ -6,6 +6,7 @@ import sonia.scm.SCMContextProvider; import sonia.scm.migration.UpdateStep; import sonia.scm.plugin.Extension; import sonia.scm.repository.xml.PathBasedRepositoryLocationResolver; +import sonia.scm.repository.xml.XmlRepositoryDAO; import sonia.scm.store.StoreConstants; import sonia.scm.version.Version; @@ -27,10 +28,12 @@ public class XmlRepositoryFileNameUpdateStep implements UpdateStep { private static final Logger LOG = LoggerFactory.getLogger(XmlRepositoryFileNameUpdateStep.class); private final SCMContextProvider contextProvider; + private final XmlRepositoryDAO repositoryDAO; @Inject - public XmlRepositoryFileNameUpdateStep(SCMContextProvider contextProvider) { + public XmlRepositoryFileNameUpdateStep(SCMContextProvider contextProvider, XmlRepositoryDAO repositoryDAO) { this.contextProvider = contextProvider; + this.repositoryDAO = repositoryDAO; } @Override @@ -41,6 +44,7 @@ public class XmlRepositoryFileNameUpdateStep implements UpdateStep { if (Files.exists(oldRepositoriesFile)) { LOG.info("moving old repositories database files to repository-paths file"); Files.move(oldRepositoriesFile, newRepositoryPathsFile); + repositoryDAO.refresh(); } } diff --git a/scm-webapp/src/test/java/sonia/scm/update/repository/XmlRepositoryFileNameUpdateStepTest.java b/scm-webapp/src/test/java/sonia/scm/update/repository/XmlRepositoryFileNameUpdateStepTest.java index 51be47fc82..658330956d 100644 --- a/scm-webapp/src/test/java/sonia/scm/update/repository/XmlRepositoryFileNameUpdateStepTest.java +++ b/scm-webapp/src/test/java/sonia/scm/update/repository/XmlRepositoryFileNameUpdateStepTest.java @@ -7,8 +7,8 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.junitpioneer.jupiter.TempDirectory; import sonia.scm.SCMContextProvider; import sonia.scm.repository.xml.PathBasedRepositoryLocationResolver; +import sonia.scm.repository.xml.XmlRepositoryDAO; -import javax.xml.bind.JAXBException; import java.io.IOException; import java.net.URL; import java.nio.file.Files; @@ -16,12 +16,14 @@ import java.nio.file.Path; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @ExtendWith(TempDirectory.class) class XmlRepositoryFileNameUpdateStepTest { SCMContextProvider contextProvider = mock(SCMContextProvider.class); + XmlRepositoryDAO repositoryDAO = mock(XmlRepositoryDAO.class); @BeforeEach void mockScmHome(@TempDirectory.TempDir Path tempDir) { @@ -29,8 +31,8 @@ class XmlRepositoryFileNameUpdateStepTest { } @Test - void shouldCopyRepositoriesFileToRepositoryPathsFile(@TempDirectory.TempDir Path tempDir) throws JAXBException, IOException { - XmlRepositoryFileNameUpdateStep updateStep = new XmlRepositoryFileNameUpdateStep(contextProvider); + void shouldCopyRepositoriesFileToRepositoryPathsFile(@TempDirectory.TempDir Path tempDir) throws IOException { + XmlRepositoryFileNameUpdateStep updateStep = new XmlRepositoryFileNameUpdateStep(contextProvider, repositoryDAO); URL url = Resources.getResource("sonia/scm/update/repository/formerV2RepositoryFile.xml"); Path configDir = tempDir.resolve("config"); Files.createDirectories(configDir); @@ -40,5 +42,6 @@ class XmlRepositoryFileNameUpdateStepTest { assertThat(configDir.resolve(PathBasedRepositoryLocationResolver.STORE_NAME + ".xml")).exists(); assertThat(configDir.resolve("repositories.xml")).doesNotExist(); + verify(repositoryDAO).refresh(); } }