Incorporate comments from peer review

This commit is contained in:
René Pfeuffer
2018-11-07 16:10:32 +01:00
parent 176cda36dc
commit f7348d8882
4 changed files with 40 additions and 8 deletions

View File

@@ -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();

View File

@@ -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);
}

View File

@@ -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.
*

View File

@@ -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);