From 3d671caadaa80f7d98cb9ad41d299f3a0111ff1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Fri, 23 Nov 2018 15:04:59 +0100 Subject: [PATCH] Let dao create repository base directory before native creation --- .../AbstractSimpleRepositoryHandler.java | 50 +++---------------- .../InitialRepositoryLocationResolver.java | 4 +- .../RepositoryLocationResolver.java | 4 -- .../scm/repository/xml/XmlRepositoryDAO.java | 27 ++++++++-- .../repository/xml/XmlRepositoryDAOTest.java | 19 ++++--- .../scm/repository/GitRepositoryHandler.java | 13 +---- .../repository/GitRepositoryHandlerTest.java | 10 ++-- .../scm/repository/HgRepositoryHandler.java | 15 +----- .../repository/HgRepositoryHandlerTest.java | 10 ++-- .../java/sonia/scm/repository/HgTestUtil.java | 20 +++----- .../scm/repository/SvnRepositoryHandler.java | 5 +- .../repository/SvnRepositoryHandlerTest.java | 9 ++-- .../repository/DummyRepositoryHandler.java | 5 +- .../SimpleRepositoryHandlerTestBase.java | 12 ----- .../repository/DefaultRepositoryManager.java | 10 ++-- .../DefaultRepositoryManagerTest.java | 8 +-- 16 files changed, 75 insertions(+), 146 deletions(-) diff --git a/scm-core/src/main/java/sonia/scm/repository/AbstractSimpleRepositoryHandler.java b/scm-core/src/main/java/sonia/scm/repository/AbstractSimpleRepositoryHandler.java index c08af3cb8b..12f0715c8d 100644 --- a/scm-core/src/main/java/sonia/scm/repository/AbstractSimpleRepositoryHandler.java +++ b/scm-core/src/main/java/sonia/scm/repository/AbstractSimpleRepositoryHandler.java @@ -34,16 +34,12 @@ package sonia.scm.repository; //~--- non-JDK imports -------------------------------------------------------- import com.google.common.base.Charsets; -import com.google.common.base.Throwables; import com.google.common.io.Resources; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import sonia.scm.AlreadyExistsException; import sonia.scm.ConfigurationException; -import sonia.scm.ContextEntry; import sonia.scm.io.CommandResult; import sonia.scm.io.ExtendedCommand; -import sonia.scm.io.FileSystem; import sonia.scm.store.ConfigurationStoreFactory; import java.io.File; @@ -69,47 +65,25 @@ public abstract class AbstractSimpleRepositoryHandler hgContextProvider, - RepositoryLocationResolver repositoryLocationResolver, - InitialRepositoryLocationResolver initialRepositoryLocationResolver) + RepositoryLocationResolver repositoryLocationResolver) { - super(storeFactory, fileSystem, repositoryLocationResolver, initialRepositoryLocationResolver); + super(storeFactory, repositoryLocationResolver); this.hgContextProvider = hgContextProvider; try diff --git a/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/HgRepositoryHandlerTest.java b/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/HgRepositoryHandlerTest.java index 4bf9e6ce77..f3fb914459 100644 --- a/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/HgRepositoryHandlerTest.java +++ b/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/HgRepositoryHandlerTest.java @@ -38,7 +38,6 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; -import sonia.scm.io.DefaultFileSystem; import sonia.scm.store.ConfigurationStoreFactory; import java.io.File; @@ -61,7 +60,6 @@ public class HgRepositoryHandlerTest extends SimpleRepositoryHandlerTestBase { private com.google.inject.Provider provider; private RepositoryLocationResolver repositoryLocationResolver; - private InitialRepositoryLocationResolver initialRepositoryLocationResolver; @Override protected void checkDirectory(File directory) { @@ -74,11 +72,9 @@ public class HgRepositoryHandlerTest extends SimpleRepositoryHandlerTestBase { @Override protected RepositoryHandler createRepositoryHandler(ConfigurationStoreFactory factory, File directory) { - initialRepositoryLocationResolver = new InitialRepositoryLocationResolver(contextProvider); - repositoryLocationResolver = new RepositoryLocationResolver(repoDao, initialRepositoryLocationResolver); + repositoryLocationResolver = new RepositoryLocationResolver(repoDao, new InitialRepositoryLocationResolver(contextProvider)); HgRepositoryHandler handler = new HgRepositoryHandler(factory, - new DefaultFileSystem(), - new HgContextProvider(), repositoryLocationResolver, initialRepositoryLocationResolver); + new HgContextProvider(), repositoryLocationResolver); handler.init(contextProvider); HgTestUtil.checkForSkip(handler); @@ -89,7 +85,7 @@ public class HgRepositoryHandlerTest extends SimpleRepositoryHandlerTestBase { @Test public void getDirectory() { HgRepositoryHandler repositoryHandler = new HgRepositoryHandler(factory, - new DefaultFileSystem(), provider, repositoryLocationResolver, initialRepositoryLocationResolver); + provider, repositoryLocationResolver); HgConfig hgConfig = new HgConfig(); hgConfig.setHgBinary("hg"); diff --git a/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/HgTestUtil.java b/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/HgTestUtil.java index 4933df527c..d2344816ef 100644 --- a/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/HgTestUtil.java +++ b/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/HgTestUtil.java @@ -36,19 +36,18 @@ package sonia.scm.repository; //~--- non-JDK imports -------------------------------------------------------- import org.junit.Assume; - import sonia.scm.SCMContext; -import sonia.scm.io.FileSystem; import sonia.scm.store.InMemoryConfigurationStoreFactory; -import static org.mockito.Mockito.*; - -//~--- JDK imports ------------------------------------------------------------ - +import javax.servlet.http.HttpServletRequest; import java.io.File; import java.nio.file.Path; -import javax.servlet.http.HttpServletRequest; +import static org.mockito.Mockito.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +//~--- JDK imports ------------------------------------------------------------ /** * @@ -102,14 +101,11 @@ public final class HgTestUtil context.setBaseDirectory(directory); - FileSystem fileSystem = mock(FileSystem.class); PathBasedRepositoryDAO repoDao = mock(PathBasedRepositoryDAO.class); - InitialRepositoryLocationResolver initialRepositoryLocationResolver = new InitialRepositoryLocationResolver(context); - RepositoryLocationResolver repositoryLocationResolver = new RepositoryLocationResolver(repoDao, initialRepositoryLocationResolver); + RepositoryLocationResolver repositoryLocationResolver = new RepositoryLocationResolver(repoDao, new InitialRepositoryLocationResolver(context)); HgRepositoryHandler handler = - new HgRepositoryHandler(new InMemoryConfigurationStoreFactory(), fileSystem, - new HgContextProvider(), repositoryLocationResolver, initialRepositoryLocationResolver); + new HgRepositoryHandler(new InMemoryConfigurationStoreFactory(), new HgContextProvider(), repositoryLocationResolver); Path repoDir = directory.toPath(); when(repoDao.getPath(any())).thenReturn(repoDir); handler.init(context); diff --git a/scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/repository/SvnRepositoryHandler.java b/scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/repository/SvnRepositoryHandler.java index 2e89fb221f..88bbb55321 100644 --- a/scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/repository/SvnRepositoryHandler.java +++ b/scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/repository/SvnRepositoryHandler.java @@ -46,7 +46,6 @@ import org.tmatesoft.svn.core.internal.io.fs.FSRepositoryFactory; import org.tmatesoft.svn.core.io.SVNRepository; import org.tmatesoft.svn.core.io.SVNRepositoryFactory; import org.tmatesoft.svn.util.SVNDebugLog; -import sonia.scm.io.FileSystem; import sonia.scm.logging.SVNKitLogger; import sonia.scm.plugin.Extension; import sonia.scm.repository.spi.HookEventFacade; @@ -87,13 +86,11 @@ public class SvnRepositoryHandler @Inject public SvnRepositoryHandler(ConfigurationStoreFactory storeFactory, - FileSystem fileSystem, HookEventFacade eventFacade, RepositoryLocationResolver repositoryLocationResolver, - InitialRepositoryLocationResolver initialRepositoryLocationResolver, RepositoryDAO repositoryDAO) { - super(storeFactory, fileSystem, repositoryLocationResolver, initialRepositoryLocationResolver); + super(storeFactory, repositoryLocationResolver); // register logger SVNDebugLog.setDefaultLog(new SVNKitLogger()); diff --git a/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/SvnRepositoryHandlerTest.java b/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/SvnRepositoryHandlerTest.java index 1920cefdb1..3025b41154 100644 --- a/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/SvnRepositoryHandlerTest.java +++ b/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/SvnRepositoryHandlerTest.java @@ -36,7 +36,6 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; -import sonia.scm.io.DefaultFileSystem; import sonia.scm.repository.api.HookContextFactory; import sonia.scm.repository.spi.HookEventFacade; import sonia.scm.store.ConfigurationStore; @@ -76,7 +75,6 @@ public class SvnRepositoryHandlerTest extends SimpleRepositoryHandlerTestBase { private HookEventFacade facade = new HookEventFacade(repositoryManagerProvider, hookContextFactory); private RepositoryLocationResolver repositoryLocationResolver; - private InitialRepositoryLocationResolver initialRepositoryLocationResolver; @Override protected void checkDirectory(File directory) { @@ -94,9 +92,8 @@ public class SvnRepositoryHandlerTest extends SimpleRepositoryHandlerTestBase { @Override protected RepositoryHandler createRepositoryHandler(ConfigurationStoreFactory factory, File directory) { - initialRepositoryLocationResolver = new InitialRepositoryLocationResolver(contextProvider); - repositoryLocationResolver = new RepositoryLocationResolver(repoDao, initialRepositoryLocationResolver); - SvnRepositoryHandler handler = new SvnRepositoryHandler(factory, new DefaultFileSystem(), null, repositoryLocationResolver, initialRepositoryLocationResolver, repositoryDAO); + repositoryLocationResolver = new RepositoryLocationResolver(repoDao, new InitialRepositoryLocationResolver(contextProvider)); + SvnRepositoryHandler handler = new SvnRepositoryHandler(factory, null, repositoryLocationResolver, repositoryDAO); handler.init(contextProvider); @@ -112,7 +109,7 @@ public class SvnRepositoryHandlerTest extends SimpleRepositoryHandlerTestBase { public void getDirectory() { when(factory.getStore(any(), any())).thenReturn(store); SvnRepositoryHandler repositoryHandler = new SvnRepositoryHandler(factory, - new DefaultFileSystem(), facade, repositoryLocationResolver, initialRepositoryLocationResolver, repositoryDAO); + facade, repositoryLocationResolver, repositoryDAO); SvnConfig svnConfig = new SvnConfig(); repositoryHandler.setConfig(svnConfig); diff --git a/scm-test/src/main/java/sonia/scm/repository/DummyRepositoryHandler.java b/scm-test/src/main/java/sonia/scm/repository/DummyRepositoryHandler.java index f1549f0aa3..3efe78c820 100644 --- a/scm-test/src/main/java/sonia/scm/repository/DummyRepositoryHandler.java +++ b/scm-test/src/main/java/sonia/scm/repository/DummyRepositoryHandler.java @@ -35,7 +35,6 @@ package sonia.scm.repository; import com.google.common.collect.Sets; import sonia.scm.AlreadyExistsException; -import sonia.scm.io.DefaultFileSystem; import sonia.scm.store.ConfigurationStoreFactory; import javax.xml.bind.annotation.XmlRootElement; @@ -59,8 +58,8 @@ public class DummyRepositoryHandler private final Set existingRepoNames = new HashSet<>(); - public DummyRepositoryHandler(ConfigurationStoreFactory storeFactory, RepositoryLocationResolver repositoryLocationResolver, InitialRepositoryLocationResolver initialRepositoryLocationResolver) { - super(storeFactory, new DefaultFileSystem(), repositoryLocationResolver, initialRepositoryLocationResolver); + public DummyRepositoryHandler(ConfigurationStoreFactory storeFactory, RepositoryLocationResolver repositoryLocationResolver) { + super(storeFactory, repositoryLocationResolver); } @Override diff --git a/scm-test/src/main/java/sonia/scm/repository/SimpleRepositoryHandlerTestBase.java b/scm-test/src/main/java/sonia/scm/repository/SimpleRepositoryHandlerTestBase.java index 1061d0f5c5..c3a372ddd5 100644 --- a/scm-test/src/main/java/sonia/scm/repository/SimpleRepositoryHandlerTestBase.java +++ b/scm-test/src/main/java/sonia/scm/repository/SimpleRepositoryHandlerTestBase.java @@ -43,7 +43,6 @@ import java.io.File; import java.io.IOException; import java.nio.file.Path; -import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.mock; @@ -82,17 +81,6 @@ public abstract class SimpleRepositoryHandlerTestBase extends AbstractTestBase { assertTrue(path.contains(repository.getId())); } - @Test - public void testDelete() { - createRepository(); - - handler.delete(repository); - - File directory = new File(baseDirectory, repository.getId()); - - assertFalse(directory.exists()); - } - @Override protected void postSetUp() throws IOException, RepositoryPathNotFoundException { InMemoryConfigurationStoreFactory storeFactory = new InMemoryConfigurationStoreFactory(); 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 d906873d80..4fd4682456 100644 --- a/scm-webapp/src/main/java/sonia/scm/repository/DefaultRepositoryManager.java +++ b/scm-webapp/src/main/java/sonia/scm/repository/DefaultRepositoryManager.java @@ -39,7 +39,6 @@ import com.google.inject.Singleton; import org.apache.shiro.concurrent.SubjectAwareExecutorService; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import sonia.scm.AlreadyExistsException; import sonia.scm.ConfigurationException; import sonia.scm.HandlerEventType; import sonia.scm.ManagerDaoAdapter; @@ -138,17 +137,18 @@ public class DefaultRepositoryManager extends AbstractRepositoryManager { return managerDaoAdapter.create( repository, RepositoryPermissions::create, + newRepository -> fireEvent(HandlerEventType.BEFORE_CREATE, newRepository), newRepository -> { + fireEvent(HandlerEventType.CREATE, newRepository); if (initRepository) { try { getHandler(newRepository).create(newRepository); - } catch (AlreadyExistsException e) { - throw new InternalRepositoryException(repository, "directory for repository does already exist", e); + } catch (InternalRepositoryException e) { + delete(repository); + throw e; } } - fireEvent(HandlerEventType.BEFORE_CREATE, newRepository); }, - newRepository -> fireEvent(HandlerEventType.CREATE, newRepository), newRepository -> repositoryDAO.contains(newRepository.getNamespaceAndName()) ); } 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 093c41f215..bb7c861d33 100644 --- a/scm-webapp/src/test/java/sonia/scm/repository/DefaultRepositoryManagerTest.java +++ b/scm-webapp/src/test/java/sonia/scm/repository/DefaultRepositoryManagerTest.java @@ -435,16 +435,16 @@ public class DefaultRepositoryManagerTest extends ManagerTestBase { Set handlerSet = new HashSet<>(); ConfigurationStoreFactory factory = new JAXBConfigurationStoreFactory(contextProvider); InitialRepositoryLocationResolver initialRepositoryLocationResolver = new InitialRepositoryLocationResolver(contextProvider); - XmlRepositoryDAO repositoryDAO = new XmlRepositoryDAO(factory, initialRepositoryLocationResolver, contextProvider); + XmlRepositoryDAO repositoryDAO = new XmlRepositoryDAO(factory, initialRepositoryLocationResolver, fileSystem, contextProvider); RepositoryLocationResolver repositoryLocationResolver = new RepositoryLocationResolver(repositoryDAO, initialRepositoryLocationResolver); - handlerSet.add(new DummyRepositoryHandler(factory, repositoryLocationResolver, initialRepositoryLocationResolver)); - handlerSet.add(new DummyRepositoryHandler(factory, repositoryLocationResolver, initialRepositoryLocationResolver) { + handlerSet.add(new DummyRepositoryHandler(factory, repositoryLocationResolver)); + handlerSet.add(new DummyRepositoryHandler(factory, repositoryLocationResolver) { @Override public RepositoryType getType() { return new RepositoryType("hg", "Mercurial", Sets.newHashSet()); } }); - handlerSet.add(new DummyRepositoryHandler(factory, repositoryLocationResolver, initialRepositoryLocationResolver) { + handlerSet.add(new DummyRepositoryHandler(factory, repositoryLocationResolver) { @Override public RepositoryType getType() { return new RepositoryType("git", "Git", Sets.newHashSet());