From b992ab414f196fdba80f45a0c1d7bb9508d0e1da Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= <rene.pfeuffer@cloudogu.com>
Date: Thu, 30 Aug 2018 10:34:43 +0200
Subject: [PATCH] Fix concurrent modification check for missing modification
 date

---
 .../SingleResourceManagerAdapter.java         |  4 ++--
 .../v2/resources/GroupRootResourceTest.java   | 23 ++++++++++++++++---
 ...ate-concurrent-modification_null_date.json | 12 ++++++++++
 .../sonia/scm/api/v2/group-test-update.json   |  3 ++-
 4 files changed, 36 insertions(+), 6 deletions(-)
 create mode 100644 scm-webapp/src/test/resources/sonia/scm/api/v2/group-test-update-concurrent-modification_null_date.json

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 c8d8dd7e65..fa50cdcc87 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
@@ -71,8 +71,8 @@ class SingleResourceManagerAdapter<MODEL_OBJECT extends ModelObject,
   }
 
   private boolean modelObjectWasModifiedConcurrently(MODEL_OBJECT existing, MODEL_OBJECT updated) {
-    return (existing.getLastModified() != null && updated.getLastModified() != null)
-      && (existing.getLastModified() > updated.getLastModified());
+    return existing.getLastModified() != null
+      && (updated.getLastModified() == null || existing.getLastModified() > updated.getLastModified());
   }
 
   public Response delete(Supplier<Optional<MODEL_OBJECT>> reader) {
diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/GroupRootResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/GroupRootResourceTest.java
index f8df23e3d2..1e42016ace 100644
--- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/GroupRootResourceTest.java
+++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/GroupRootResourceTest.java
@@ -122,10 +122,10 @@ public class GroupRootResourceTest {
     MockHttpResponse response = new MockHttpResponse();
 
     dispatcher.invoke(request, response);
-    Group capturedGroup = groupCaptor.getValue();
 
     assertEquals(HttpServletResponse.SC_NO_CONTENT, response.getStatus());
 
+    Group capturedGroup = groupCaptor.getValue();
     assertEquals("Updated description", capturedGroup.getDescription());
   }
 
@@ -147,7 +147,7 @@ public class GroupRootResourceTest {
   }
 
   @Test
-  public void updateShouldFailOnConcurrentModification() throws URISyntaxException, IOException {
+  public void updateShouldFailOnConcurrentModification_oldModificationDate() throws URISyntaxException, IOException {
     URL url = Resources.getResource("sonia/scm/api/v2/group-test-update-concurrent-modification.json");
     byte[] groupJson = Resources.toByteArray(url);
 
@@ -163,6 +163,23 @@ public class GroupRootResourceTest {
     assertEquals(HttpServletResponse.SC_CONFLICT, response.getStatus());
   }
 
+  @Test
+  public void updateShouldFailOnConcurrentModification_unsetModificationDate() throws URISyntaxException, IOException {
+    URL url = Resources.getResource("sonia/scm/api/v2/group-test-update-concurrent-modification_null_date.json");
+    byte[] groupJson = Resources.toByteArray(url);
+
+    MockHttpRequest request = MockHttpRequest
+      .put("/" + GroupRootResource.GROUPS_PATH_V2 + "admin")
+      .contentType(VndMediaType.GROUP)
+      .content(groupJson);
+
+    MockHttpResponse response = new MockHttpResponse();
+
+    dispatcher.invoke(request, response);
+
+    assertEquals(HttpServletResponse.SC_CONFLICT, response.getStatus());
+  }
+
   @Test
   public void shouldDeleteGroup() throws URISyntaxException {
     Group group = createDummyGroup();
@@ -236,7 +253,7 @@ public class GroupRootResourceTest {
     group.setName("admin");
     group.setCreationDate(0L);
     group.setMembers(Collections.singletonList("user"));
-    group.setLastModified(1234L);
+    group.setLastModified(3600000L);
     return group;
   }
 }
diff --git a/scm-webapp/src/test/resources/sonia/scm/api/v2/group-test-update-concurrent-modification_null_date.json b/scm-webapp/src/test/resources/sonia/scm/api/v2/group-test-update-concurrent-modification_null_date.json
new file mode 100644
index 0000000000..324a2ef3c0
--- /dev/null
+++ b/scm-webapp/src/test/resources/sonia/scm/api/v2/group-test-update-concurrent-modification_null_date.json
@@ -0,0 +1,12 @@
+{
+  "description": "Updated description",
+  "name": "admin",
+  "type": "xml",
+  "_embedded": {
+    "members": [
+      {
+        "name": "user"
+      }
+    ]
+  }
+}
diff --git a/scm-webapp/src/test/resources/sonia/scm/api/v2/group-test-update.json b/scm-webapp/src/test/resources/sonia/scm/api/v2/group-test-update.json
index 324a2ef3c0..13d6d5f574 100644
--- a/scm-webapp/src/test/resources/sonia/scm/api/v2/group-test-update.json
+++ b/scm-webapp/src/test/resources/sonia/scm/api/v2/group-test-update.json
@@ -8,5 +8,6 @@
         "name": "user"
       }
     ]
-  }
+  },
+  "lastModified": "1970-01-01T01:00:00.00Z"
 }