Use namespace and name for exceptions instead of technical id

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)) {
+        throw new AlreadyExistsException(newObject);
+      }
+    });
   }
 
-  public T create(T newObject, Supplier<PermissionCheck> permissionCheck, AroundHandler<T> beforeCreate, AroundHandler<T> afterCreate, Predicate<T> existsCheck) {
+  public T create(T newObject, Supplier<PermissionCheck> permissionCheck, AroundHandler<T> beforeCreate, AroundHandler<T> afterCreate, Consumer<T> existsCheck) {
     permissionCheck.get().check();
     AssertUtil.assertIsValid(newObject);
-    if (existsCheck.test(newObject)) {
+    existsCheck.accept(newObject);
-      throw new AlreadyExistsException(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/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