From d4e9f46aac5ea04a0c0c51433b147f0e57693edd Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Mon, 14 Sep 2020 15:48:45 +0200 Subject: [PATCH 1/2] Fix missing synchronization during repository creation --- CHANGELOG.md | 4 + .../scm/repository/xml/XmlRepositoryDAO.java | 28 ++--- .../XmlRepositoryDAOSynchronizationTest.java | 114 ++++++++++++++++++ 3 files changed, 131 insertions(+), 15 deletions(-) create mode 100644 scm-dao-xml/src/test/java/sonia/scm/repository/xml/XmlRepositoryDAOSynchronizationTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ac3fca88b..73f45ea43a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## Unreleased +### Fixed +- Missing synchronization during repository creation ([#1328](https://github.com/scm-manager/scm-manager/pull/1328)) + ## [2.5.0] - 2020-09-10 ### Added - Tags now have date information attached ([#1305](https://github.com/scm-manager/scm-manager/pull/1305)) 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 6873bd06ce..7ec23ce880 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 @@ -21,7 +21,7 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - + package sonia.scm.repository.xml; //~--- non-JDK imports -------------------------------------------------------- @@ -83,29 +83,27 @@ public class XmlRepositoryDAO implements RepositoryDAO { } @Override - public void add(Repository repository) { + public synchronized void add(Repository repository) { add(repository, repositoryLocationResolver.create(repository.getId())); } - public void add(Repository repository, Object location) { + public synchronized void add(Repository repository, Object location) { if (!(location instanceof Path)) { throw new IllegalArgumentException("can only handle locations of type " + Path.class.getName() + ", not of type " + location.getClass().getName()); } + Path repositoryPath = (Path) location; + Repository clone = repository.clone(); - synchronized (this) { - Path repositoryPath = (Path) location; - - try { - metadataStore.write(repositoryPath, repository); - } catch (Exception e) { - repositoryLocationResolver.remove(repository.getId()); - throw new InternalRepositoryException(repository, "failed to create filesystem", e); - } - - byId.put(repository.getId(), clone); - byNamespaceAndName.put(repository.getNamespaceAndName(), clone); + try { + metadataStore.write(repositoryPath, repository); + } catch (Exception e) { + repositoryLocationResolver.remove(repository.getId()); + throw new InternalRepositoryException(repository, "failed to create filesystem", e); } + + byId.put(repository.getId(), clone); + byNamespaceAndName.put(repository.getNamespaceAndName(), clone); } @Override diff --git a/scm-dao-xml/src/test/java/sonia/scm/repository/xml/XmlRepositoryDAOSynchronizationTest.java b/scm-dao-xml/src/test/java/sonia/scm/repository/xml/XmlRepositoryDAOSynchronizationTest.java new file mode 100644 index 0000000000..91db706a76 --- /dev/null +++ b/scm-dao-xml/src/test/java/sonia/scm/repository/xml/XmlRepositoryDAOSynchronizationTest.java @@ -0,0 +1,114 @@ +/* + * MIT License + * + * Copyright (c) 2020-present Cloudogu GmbH and Contributors + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +package sonia.scm.repository.xml; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.io.TempDir; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import sonia.scm.SCMContextProvider; +import sonia.scm.io.DefaultFileSystem; +import sonia.scm.io.FileSystem; +import sonia.scm.repository.InitialRepositoryLocationResolver; +import sonia.scm.repository.Repository; + +import java.nio.file.Path; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class XmlRepositoryDAOSynchronizationTest { + + private static final int CREATION_COUNT = 100; + private static final long TIMEOUT = 10L; + + @Mock + private SCMContextProvider provider; + + private FileSystem fileSystem; + private PathBasedRepositoryLocationResolver resolver; + + private XmlRepositoryDAO repositoryDAO; + + @BeforeEach + void setUpObjectUnderTest(@TempDir Path path) { + when(provider.getBaseDirectory()).thenReturn(path.toFile()); + + when(provider.resolve(any())).then(ic -> { + Path args = ic.getArgument(0); + return path.resolve(args); + }); + + fileSystem = new DefaultFileSystem(); + + resolver = new PathBasedRepositoryLocationResolver( + provider, new InitialRepositoryLocationResolver(), fileSystem + ); + + repositoryDAO = new XmlRepositoryDAO(resolver, fileSystem); + } + + @Test + @Timeout(TIMEOUT) + void shouldCreateALotOfRepositoriesInSerial() { + for (int i=0; i repositoryDAO.add(new Repository("repo_" + index, "git", "sync_it", "repo_" + index)); + } + +} From 32d500cc44a9ab56ffd316286273edb418f77fe4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Tue, 15 Sep 2020 15:15:12 +0200 Subject: [PATCH 2/2] Add root exception to new exception This is needed to get an idea why the context could not be created. --- .../src/main/java/sonia/scm/store/TypedStoreContext.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scm-dao-xml/src/main/java/sonia/scm/store/TypedStoreContext.java b/scm-dao-xml/src/main/java/sonia/scm/store/TypedStoreContext.java index 58e3e1efdc..224946040d 100644 --- a/scm-dao-xml/src/main/java/sonia/scm/store/TypedStoreContext.java +++ b/scm-dao-xml/src/main/java/sonia/scm/store/TypedStoreContext.java @@ -48,7 +48,7 @@ final class TypedStoreContext { JAXBContext jaxbContext = JAXBContext.newInstance(parameters.getType()); return new TypedStoreContext<>(jaxbContext, parameters); } catch (JAXBException e) { - throw new StoreException("failed to create context for store"); + throw new StoreException("failed to create context for store", e); } }