From f7348d8882d30e34ea395e00d76db98daa5f4058 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= <rene.pfeuffer@cloudogu.com>
Date: Wed, 7 Nov 2018 16:10:32 +0100
Subject: [PATCH] Incorporate comments from peer review

---
 .../AbstractSimpleRepositoryHandler.java      |  6 ++--
 .../InternalRepositoryException.java          |  4 +++
 .../api/RepositoryServiceFactory.java         | 32 +++++++++++++++++++
 .../resources/ChangesetRootResourceTest.java  |  6 +---
 4 files changed, 40 insertions(+), 8 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 6cfa08975e..d190935fae 100644
--- a/scm-core/src/main/java/sonia/scm/repository/AbstractSimpleRepositoryHandler.java
+++ b/scm-core/src/main/java/sonia/scm/repository/AbstractSimpleRepositoryHandler.java
@@ -89,7 +89,7 @@ public abstract class AbstractSimpleRepositoryHandler<C extends RepositoryConfig
       throw new AlreadyExistsException(repository);
     }
 
-    checkPath(directory);
+    checkPath(directory, repository);
 
     try {
       fileSystem.create(directory);
@@ -259,7 +259,7 @@ public abstract class AbstractSimpleRepositoryHandler<C extends RepositoryConfig
    * @param directory repository target directory
    * @throws RuntimeException when the parent directory already is a repository
    */
-  private void checkPath(File directory) {
+  private void checkPath(File directory, Repository repository) {
     File repositoryDirectory = config.getRepositoryDirectory();
     File parent = directory.getParentFile();
 
@@ -267,7 +267,7 @@ public abstract class AbstractSimpleRepositoryHandler<C extends RepositoryConfig
       logger.trace("check {} for existing repository", parent);
 
       if (isRepository(parent)) {
-        throw new RuntimeException("parent path " + parent + " is a repository");
+        throw new InternalRepositoryException(repository, "parent path " + parent + " is a repository");
       }
 
       parent = parent.getParentFile();
diff --git a/scm-core/src/main/java/sonia/scm/repository/InternalRepositoryException.java b/scm-core/src/main/java/sonia/scm/repository/InternalRepositoryException.java
index 76485daf56..c53ae32750 100644
--- a/scm-core/src/main/java/sonia/scm/repository/InternalRepositoryException.java
+++ b/scm-core/src/main/java/sonia/scm/repository/InternalRepositoryException.java
@@ -15,6 +15,10 @@ public class InternalRepositoryException extends ExceptionWithContext {
     this(context.build(), message, cause);
   }
 
+  public InternalRepositoryException(Repository repository, String message) {
+    this(ContextEntry.ContextBuilder.entity(repository), message, null);
+  }
+
   public InternalRepositoryException(Repository repository, String message, Exception cause) {
     this(ContextEntry.ContextBuilder.entity(repository), message, cause);
   }
diff --git a/scm-core/src/main/java/sonia/scm/repository/api/RepositoryServiceFactory.java b/scm-core/src/main/java/sonia/scm/repository/api/RepositoryServiceFactory.java
index d57a965a38..8db3ab7546 100644
--- a/scm-core/src/main/java/sonia/scm/repository/api/RepositoryServiceFactory.java
+++ b/scm-core/src/main/java/sonia/scm/repository/api/RepositoryServiceFactory.java
@@ -38,6 +38,7 @@ package sonia.scm.repository.api;
 import com.github.legman.ReferenceType;
 import com.github.legman.Subscribe;
 import com.google.common.base.Preconditions;
+import com.google.common.base.Strings;
 import com.google.common.collect.Sets;
 import com.google.inject.Inject;
 import com.google.inject.Singleton;
@@ -154,6 +155,37 @@ public final class RepositoryServiceFactory
 
   //~--- methods --------------------------------------------------------------
 
+  /**
+   * Creates a new RepositoryService for the given repository.
+   *
+   *
+   * @param repositoryId id of the repository
+   *
+   * @return a implementation of RepositoryService
+   *         for the given type of repository
+   *
+   * @throws NotFoundException if no repository
+   *         with the given id is available
+   * @throws RepositoryServiceNotFoundException if no repository service
+   *         implementation for this kind of repository is available
+   * @throws IllegalArgumentException if the repository id is null or empty
+   * @throws ScmSecurityException if current user has not read permissions
+   *         for that repository
+   */
+  public RepositoryService create(String repositoryId) {
+    Preconditions.checkArgument(!Strings.isNullOrEmpty(repositoryId),
+      "a non empty repositoryId is required");
+
+    Repository repository = repositoryManager.get(repositoryId);
+
+    if (repository == null)
+    {
+      throw new NotFoundException(Repository.class, repositoryId);
+    }
+
+    return create(repository);
+  }
+
   /**
    * Creates a new RepositoryService for the given repository.
    *
diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ChangesetRootResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ChangesetRootResourceTest.java
index 998e697d15..d5c0f91f81 100644
--- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ChangesetRootResourceTest.java
+++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ChangesetRootResourceTest.java
@@ -9,7 +9,6 @@ import org.apache.shiro.util.ThreadContext;
 import org.apache.shiro.util.ThreadState;
 import org.assertj.core.util.Lists;
 import org.jboss.resteasy.core.Dispatcher;
-import org.jboss.resteasy.mock.MockDispatcherFactory;
 import org.jboss.resteasy.mock.MockHttpRequest;
 import org.jboss.resteasy.mock.MockHttpResponse;
 import org.junit.After;
@@ -19,8 +18,6 @@ import org.junit.runner.RunWith;
 import org.mockito.InjectMocks;
 import org.mockito.Mock;
 import org.mockito.junit.MockitoJUnitRunner;
-import sonia.scm.api.rest.AuthorizationExceptionMapper;
-import sonia.scm.api.v2.NotFoundExceptionMapper;
 import sonia.scm.repository.Changeset;
 import sonia.scm.repository.ChangesetPagingResult;
 import sonia.scm.repository.NamespaceAndName;
@@ -39,7 +36,6 @@ import java.util.List;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
@@ -79,7 +75,7 @@ public class ChangesetRootResourceTest extends RepositoryTestBase {
 
 
   @Before
-  public void prepareEnvironment() throws Exception {
+  public void prepareEnvironment() {
     changesetCollectionToDtoMapper = new ChangesetCollectionToDtoMapper(changesetToChangesetDtoMapper, resourceLinks);
     changesetRootResource = new ChangesetRootResource(serviceFactory, changesetCollectionToDtoMapper, changesetToChangesetDtoMapper);
     super.changesetRootResource = Providers.of(changesetRootResource);