diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitLogCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitLogCommand.java index a6f74d24eb..bdbb87f2aa 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitLogCommand.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitLogCommand.java @@ -49,6 +49,7 @@ import org.eclipse.jgit.treewalk.filter.PathFilter; import org.eclipse.jgit.treewalk.filter.TreeFilter; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import sonia.scm.NotFoundException; import sonia.scm.repository.Changeset; import sonia.scm.repository.ChangesetPagingResult; import sonia.scm.repository.GitChangesetConverter; @@ -206,6 +207,9 @@ public class GitLogCommand extends AbstractGitCommand implements LogCommand if (!Strings.isNullOrEmpty(request.getAncestorChangeset())) { ancestorId = repository.resolve(request.getAncestorChangeset()); + if (ancestorId == null) { + throw notFound(entity("Revision", request.getAncestorChangeset()).in(this.repository)); + } } revWalk = new RevWalk(repository); @@ -245,6 +249,8 @@ public class GitLogCommand extends AbstractGitCommand implements LogCommand break; } } + } else if (ancestorId != null) { + throw notFound(entity("Revision", request.getBranch()).in(this.repository)); } if (branch != null) { @@ -263,6 +269,10 @@ public class GitLogCommand extends AbstractGitCommand implements LogCommand { throw notFound(entity("Revision", e.getObjectId().getName()).in(repository)); } + catch (NotFoundException e) + { + throw e; + } catch (Exception ex) { throw new InternalRepositoryException(repository, "could not create change log", ex); diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMergeCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMergeCommand.java index be91d06361..64fbadd5a2 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMergeCommand.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMergeCommand.java @@ -62,12 +62,24 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand try { Repository repository = context.open(); ResolveMerger merger = (ResolveMerger) MergeStrategy.RECURSIVE.newMerger(repository, true); - return new MergeDryRunCommandResult(merger.merge(repository.resolve(request.getBranchToMerge()), repository.resolve(request.getTargetBranch()))); + return new MergeDryRunCommandResult( + merger.merge( + resolveRevisionOrThrowNotFound(repository, request.getBranchToMerge()), + resolveRevisionOrThrowNotFound(repository, request.getTargetBranch()))); } catch (IOException e) { throw new InternalRepositoryException(context.getRepository(), "could not clone repository for merge", e); } } + private ObjectId resolveRevisionOrThrowNotFound(Repository repository, String revision) throws IOException { + ObjectId resolved = repository.resolve(revision); + if (resolved == null) { + throw notFound(entity("revision", revision).in(context.getRepository())); + } else { + return resolved; + } + } + private class MergeWorker { private final String target; @@ -110,9 +122,6 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand private void checkOutTargetAsNewLocalBranch() throws IOException { try { ObjectId targetRevision = resolveRevision(target); - if (targetRevision == null) { - throw notFound(entity("revision", target).in(context.getRepository())); - } clone.checkout().setStartPoint(targetRevision.getName()).setName(target).setCreateBranch(true).call(); } catch (RefNotFoundException e) { logger.debug("could not checkout target branch {} for merge as local branch", target, e); @@ -126,9 +135,6 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand MergeResult result; try { ObjectId sourceRevision = resolveRevision(toMerge); - if (sourceRevision == null) { - throw notFound(entity("revision", toMerge).in(context.getRepository())); - } result = clone.merge() .setFastForward(FastForwardMode.NO_FF) .setCommit(false) // we want to set the author manually @@ -190,10 +196,10 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand return MergeCommandResult.failure(result.getConflicts().keySet()); } - private ObjectId resolveRevision(String branchToMerge) throws IOException { - ObjectId resolved = clone.getRepository().resolve(branchToMerge); + private ObjectId resolveRevision(String revision) throws IOException { + ObjectId resolved = clone.getRepository().resolve(revision); if (resolved == null) { - return clone.getRepository().resolve("origin/" + branchToMerge); + return resolveRevisionOrThrowNotFound(clone.getRepository(), "origin/" + revision); } else { return resolved; } diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitLogCommandAncestorTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitLogCommandAncestorTest.java index d36922f941..a2bfa1bd36 100644 --- a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitLogCommandAncestorTest.java +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitLogCommandAncestorTest.java @@ -35,6 +35,7 @@ package sonia.scm.repository.spi; import org.junit.Test; +import sonia.scm.NotFoundException; import sonia.scm.repository.ChangesetPagingResult; import static org.junit.Assert.assertEquals; @@ -95,6 +96,28 @@ public class GitLogCommandAncestorTest extends AbstractGitCommandTestBase assertEquals("201ecc1131e6b99fb0a0fe9dcbc8c044383e1a07", result.getChangesets().get(6).getId()); } + @Test(expected = NotFoundException.class) + public void testAncestorWithDeletedSourceBranch() + { + LogCommandRequest request = new LogCommandRequest(); + + request.setBranch("no_such_branch"); + request.setAncestorChangeset("master"); + + createCommand().getChangesets(request); + } + + @Test(expected = NotFoundException.class) + public void testAncestorWithDeletedAncestorBranch() + { + LogCommandRequest request = new LogCommandRequest(); + + request.setBranch("b"); + request.setAncestorChangeset("no_such_branch"); + + createCommand().getChangesets(request); + } + private GitLogCommand createCommand() { return new GitLogCommand(createContext(), repository); diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitMergeCommandTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitMergeCommandTest.java index 7e50b48b9a..c926935496 100644 --- a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitMergeCommandTest.java +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitMergeCommandTest.java @@ -16,12 +16,14 @@ import org.junit.After; import org.junit.Before; import org.junit.Rule; import org.junit.Test; +import sonia.scm.NotFoundException; import sonia.scm.repository.GitRepositoryHandler; import sonia.scm.repository.Person; import sonia.scm.repository.PreProcessorUtil; import sonia.scm.repository.RepositoryManager; import sonia.scm.repository.api.HookContextFactory; import sonia.scm.repository.api.MergeCommandResult; +import sonia.scm.repository.api.MergeDryRunCommandResult; import sonia.scm.user.User; import java.io.IOException; @@ -220,6 +222,46 @@ public class GitMergeCommandTest extends AbstractGitCommandTestBase { assertThat(new String(contentOfFileB)).isEqualTo("b\ncontent from branch\n"); } + @Test(expected = NotFoundException.class) + public void shouldHandleNotExistingSourceBranchInMerge() { + GitMergeCommand command = createCommand(); + MergeCommandRequest request = new MergeCommandRequest(); + request.setTargetBranch("mergeable"); + request.setBranchToMerge("not_existing"); + + command.merge(request); + } + + @Test(expected = NotFoundException.class) + public void shouldHandleNotExistingTargetBranchInMerge() { + GitMergeCommand command = createCommand(); + MergeCommandRequest request = new MergeCommandRequest(); + request.setTargetBranch("not_existing"); + request.setBranchToMerge("master"); + + command.merge(request); + } + + @Test(expected = NotFoundException.class) + public void shouldHandleNotExistingSourceBranchInDryRun() { + GitMergeCommand command = createCommand(); + MergeCommandRequest request = new MergeCommandRequest(); + request.setTargetBranch("mergeable"); + request.setBranchToMerge("not_existing"); + + command.dryRun(request); + } + + @Test(expected = NotFoundException.class) + public void shouldHandleNotExistingTargetBranchInDryRun() { + GitMergeCommand command = createCommand(); + MergeCommandRequest request = new MergeCommandRequest(); + request.setTargetBranch("not_existing"); + request.setBranchToMerge("master"); + + command.dryRun(request); + } + private GitMergeCommand createCommand() { return new GitMergeCommand(createContext(), repository, new SimpleGitWorkdirFactory()); } diff --git a/scm-webapp/src/main/java/sonia/scm/ManagerDaoAdapter.java b/scm-webapp/src/main/java/sonia/scm/ManagerDaoAdapter.java index fb5cd9f5cc..e89cedb750 100644 --- a/scm-webapp/src/main/java/sonia/scm/ManagerDaoAdapter.java +++ b/scm-webapp/src/main/java/sonia/scm/ManagerDaoAdapter.java @@ -3,8 +3,8 @@ package sonia.scm; import com.github.sdorra.ssp.PermissionCheck; import sonia.scm.util.AssertUtil; +import java.util.function.Consumer; import java.util.function.Function; -import java.util.function.Predicate; import java.util.function.Supplier; public class ManagerDaoAdapter { @@ -35,15 +35,17 @@ public class ManagerDaoAdapter { } public T create(T newObject, Supplier permissionCheck, AroundHandler beforeCreate, AroundHandler afterCreate) { - return create(newObject, permissionCheck, beforeCreate, afterCreate, dao::contains); + return create(newObject, permissionCheck, beforeCreate, afterCreate, o -> { + if (dao.contains(o)) { + throw new AlreadyExistsException(newObject); + } + }); } - public T create(T newObject, Supplier permissionCheck, AroundHandler beforeCreate, AroundHandler afterCreate, Predicate existsCheck) { + public T create(T newObject, Supplier permissionCheck, AroundHandler beforeCreate, AroundHandler afterCreate, Consumer existsCheck) { permissionCheck.get().check(); AssertUtil.assertIsValid(newObject); - if (existsCheck.test(newObject)) { - throw new AlreadyExistsException(newObject); - } + existsCheck.accept(newObject); newObject.setCreationDate(System.currentTimeMillis()); beforeCreate.handle(newObject); dao.add(newObject); diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryResource.java index 5cc9df38f8..5bca6a6e16 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryResource.java @@ -148,7 +148,8 @@ public class RepositoryResource { return adapter.update( loadBy(namespace, name), existing -> processUpdate(repository, existing), - nameAndNamespaceStaysTheSame(namespace, name) + nameAndNamespaceStaysTheSame(namespace, name), + r -> r.getNamespaceAndName().logString() ); } diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/SingleResourceManagerAdapter.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/SingleResourceManagerAdapter.java index e61a9f3455..72507562dd 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/SingleResourceManagerAdapter.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/SingleResourceManagerAdapter.java @@ -56,17 +56,21 @@ class SingleResourceManagerAdapter reader, Function applyChanges, Predicate hasSameKey) { + return update(reader, applyChanges, hasSameKey, ModelObject::getId); + } + + Response update(Supplier reader, Function applyChanges, Predicate hasSameKey, Function keyExtractor) { MODEL_OBJECT existingModelObject = reader.get(); MODEL_OBJECT changedModelObject = applyChanges.apply(existingModelObject); if (!hasSameKey.test(changedModelObject)) { return Response.status(BAD_REQUEST).entity("illegal change of id").build(); } else if (modelObjectWasModifiedConcurrently(existingModelObject, changedModelObject)) { - throw new ConcurrentModificationException(type, existingModelObject.getId()); + throw new ConcurrentModificationException(type, keyExtractor.apply(existingModelObject)); } return update(getId(existingModelObject), changedModelObject); } 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 4fd4682456..91ea79795b 100644 --- a/scm-webapp/src/main/java/sonia/scm/repository/DefaultRepositoryManager.java +++ b/scm-webapp/src/main/java/sonia/scm/repository/DefaultRepositoryManager.java @@ -64,6 +64,7 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.ThreadFactory; +import static sonia.scm.AlreadyExistsException.alreadyExists; import static sonia.scm.ContextEntry.ContextBuilder.entity; import static sonia.scm.NotFoundException.notFound; @@ -149,7 +150,11 @@ public class DefaultRepositoryManager extends AbstractRepositoryManager { } } }, - newRepository -> repositoryDAO.contains(newRepository.getNamespaceAndName()) + newRepository -> { + if (repositoryDAO.contains(newRepository.getNamespaceAndName())) { + throw alreadyExists(entity(newRepository.getClass(), newRepository.getNamespaceAndName().logString())); + } + } ); } diff --git a/scm-webapp/src/main/resources/META-INF/scm/permissions.xml b/scm-webapp/src/main/resources/META-INF/scm/permissions.xml index 6b7f7bd99f..27f343bc30 100644 --- a/scm-webapp/src/main/resources/META-INF/scm/permissions.xml +++ b/scm-webapp/src/main/resources/META-INF/scm/permissions.xml @@ -32,7 +32,10 @@ --> - + + + * + repository:read,pull:* diff --git a/scm-webapp/src/main/resources/locales/de/plugins.json b/scm-webapp/src/main/resources/locales/de/plugins.json index 0822be9cdd..0a0719ec18 100644 --- a/scm-webapp/src/main/resources/locales/de/plugins.json +++ b/scm-webapp/src/main/resources/locales/de/plugins.json @@ -1,5 +1,9 @@ { "permissions": { + "*": { + "displayName": "Globaler Administrator", + "description": "Darf die komplette Instanz administrieren." + }, "repository": { "read,pull": { "*": { @@ -47,11 +51,11 @@ }, "read,write": { "global": { - "displayName": "zentrale Konfiguration", + "displayName": "Zentrale Konfiguration", "description": "Darf die Konfiguration des SCM-Manager anpassen" }, "*": { - "displayName": "zentrale + Plugin Konfiguration", + "displayName": "Zentrale + Plugin Konfiguration", "description": "Darf die Konfiguration des SCM-Manager und aller Plugins anpassen" } } diff --git a/scm-webapp/src/main/resources/locales/en/plugins.json b/scm-webapp/src/main/resources/locales/en/plugins.json index cc9902565b..86e8a40df1 100644 --- a/scm-webapp/src/main/resources/locales/en/plugins.json +++ b/scm-webapp/src/main/resources/locales/en/plugins.json @@ -1,5 +1,9 @@ { "permissions": { + "*": { + "displayName": "Global administrator", + "description": "May administer the entire instance" + }, "repository": { "read,pull": { "*": { diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/RepositoryRootResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/RepositoryRootResourceTest.java index 1f6ed6b3a7..d00d45bb24 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/RepositoryRootResourceTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/RepositoryRootResourceTest.java @@ -35,6 +35,7 @@ import java.net.URL; import static java.util.Collections.singletonList; import static java.util.stream.Stream.of; import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST; +import static javax.servlet.http.HttpServletResponse.SC_CONFLICT; import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND; import static javax.servlet.http.HttpServletResponse.SC_NO_CONTENT; import static javax.servlet.http.HttpServletResponse.SC_OK; @@ -196,6 +197,26 @@ public class RepositoryRootResourceTest extends RepositoryTestBase { verify(repositoryManager).modify(anyObject()); } + @Test + public void shouldHandleUpdateForConcurrentlyChangedRepository() throws Exception { + mockRepository("space", "repo", 1337); + + URL url = Resources.getResource("sonia/scm/api/v2/repository-test-update.json"); + byte[] repository = Resources.toByteArray(url); + + MockHttpRequest request = MockHttpRequest + .put("/" + RepositoryRootResource.REPOSITORIES_PATH_V2 + "space/repo") + .contentType(VndMediaType.REPOSITORY) + .content(repository); + MockHttpResponse response = new MockHttpResponse(); + + dispatcher.invoke(request, response); + + assertEquals(SC_CONFLICT, response.getStatus()); + assertThat(response.getContentAsString()).contains("space/repo"); + verify(repositoryManager, never()).modify(anyObject()); + } + @Test public void shouldHandleUpdateForExistingRepositoryForChangedNamespace() throws Exception { mockRepository("wrong", "repo"); @@ -313,9 +334,16 @@ public class RepositoryRootResourceTest extends RepositoryTestBase { } private Repository mockRepository(String namespace, String name) { + return mockRepository(namespace, name, 0); + } + + private Repository mockRepository(String namespace, String name, long lastModified) { Repository repository = new Repository(); repository.setNamespace(namespace); repository.setName(name); + if (lastModified > 0) { + repository.setLastModified(lastModified); + } String id = namespace + "-" + name; repository.setId(id); when(repositoryManager.get(new NamespaceAndName(namespace, name))).thenReturn(repository); 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 416babfab0..5630ad02fe 100644 --- a/scm-webapp/src/test/java/sonia/scm/repository/DefaultRepositoryManagerTest.java +++ b/scm-webapp/src/test/java/sonia/scm/repository/DefaultRepositoryManagerTest.java @@ -123,9 +123,12 @@ public class DefaultRepositoryManagerTest extends ManagerTestBase { createTestRepository(); } - @Test(expected = AlreadyExistsException.class) + @Test public void testCreateExisting() { - createTestRepository(); + Repository testRepository = createTestRepository(); + String expectedNamespaceAndName = testRepository.getNamespaceAndName().logString(); + thrown.expect(AlreadyExistsException.class); + thrown.expectMessage(expectedNamespaceAndName); createTestRepository(); } @@ -392,18 +395,6 @@ public class DefaultRepositoryManagerTest extends ManagerTestBase { assertNotNull(repository.getNamespace()); } - private void createUriTestRepositories(RepositoryManager m) { - mockedNamespace = "namespace"; - createRepository(m, new Repository("1", "hg", "namespace", "scm")); - createRepository(m, new Repository("2", "hg", "namespace", "scm-test")); - createRepository(m, new Repository("3", "git", "namespace", "test-1")); - createRepository(m, new Repository("4", "git", "namespace", "test-2")); - - mockedNamespace = "other"; - createRepository(m, new Repository("1", "hg", "other", "scm")); - createRepository(m, new Repository("2", "hg", "other", "scm-test")); - } - //~--- methods -------------------------------------------------------------- @Override