Merge with 2.0.0-m3

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

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 {
+    private ObjectId resolveRevision(String revision) throws IOException {
-      ObjectId resolved = clone.getRepository().resolve(branchToMerge);
+      ObjectId resolved = clone.getRepository().resolve(revision);
       if (resolved == null) {
-        return clone.getRepository().resolve("origin/" + branchToMerge);
+        return resolveRevisionOrThrowNotFound(clone.getRepository(), "origin/" + revision);
       } else {
         return resolved;
       }

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

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

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<T extends ModelObject> {
@@ -35,15 +35,17 @@ public class ManagerDaoAdapter<T extends ModelObject> {
   }
 
   public T create(T newObject, Supplier<PermissionCheck> permissionCheck, AroundHandler<T> beforeCreate, AroundHandler<T> afterCreate) {
-    return create(newObject, permissionCheck, beforeCreate, afterCreate, dao::contains);
+    return create(newObject, permissionCheck, beforeCreate, afterCreate, o -> {
-  }
+      if (dao.contains(o)) {
-
-  public T create(T newObject, Supplier<PermissionCheck> permissionCheck, AroundHandler<T> beforeCreate, AroundHandler<T> afterCreate, Predicate<T> existsCheck) {
-    permissionCheck.get().check();
-    AssertUtil.assertIsValid(newObject);
-    if (existsCheck.test(newObject)) {
         throw new AlreadyExistsException(newObject);
       }
+    });
+  }
+
+  public T create(T newObject, Supplier<PermissionCheck> permissionCheck, AroundHandler<T> beforeCreate, AroundHandler<T> afterCreate, Consumer<T> existsCheck) {
+    permissionCheck.get().check();
+    AssertUtil.assertIsValid(newObject);
+    existsCheck.accept(newObject);
     newObject.setCreationDate(System.currentTimeMillis());
     beforeCreate.handle(newObject);
     dao.add(newObject);

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

scm-webapp/src/main/java/sonia/scm/api/v2/resources/SingleResourceManagerAdapter.java

@@ -56,17 +56,21 @@ class SingleResourceManagerAdapter<MODEL_OBJECT extends ModelObject,
   }
 
   /**
-   * Update the model object for the given id according to the given function and returns a corresponding http response.
+   * Updates the model object provided by the reader according to the given function and returns a corresponding http
-   * This handles all corner cases, eg. no matching object for the id or missing privileges.
+   * response. This handles all corner cases, eg. no matching object for the id or missing privileges.
    */
   Response update(Supplier<MODEL_OBJECT> reader, Function<MODEL_OBJECT, MODEL_OBJECT> applyChanges, Predicate<MODEL_OBJECT> hasSameKey) {
+    return update(reader, applyChanges, hasSameKey, ModelObject::getId);
+  }
+
+  Response update(Supplier<MODEL_OBJECT> reader, Function<MODEL_OBJECT, MODEL_OBJECT> applyChanges, Predicate<MODEL_OBJECT> hasSameKey, Function<MODEL_OBJECT, String> 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);
   }

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()));
+        }
+      }
     );
   }
 

scm-webapp/src/main/resources/META-INF/scm/permissions.xml

@@ -33,6 +33,9 @@
 -->
 <permissions>
 
+  <permission>
+    <value>*</value>
+  </permission>
   <permission>
     <value>repository:read,pull:*</value>
   </permission>

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"
         }
       }

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": {
         "*": {

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

scm-webapp/src/test/java/sonia/scm/repository/DefaultRepositoryManagerTest.java

@@ -123,9 +123,12 @@ public class DefaultRepositoryManagerTest extends ManagerTestBase<Repository> {
     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<Repository> {
     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