From 1505e7bd66b17e95f90ae9db5c054d8dad6e1860 Mon Sep 17 00:00:00 2001
From: Michael Hoennig <michael@hoennig.de>
Date: Tue, 23 Apr 2019 09:22:23 +0200
Subject: [PATCH] JSonDeserializerWithAccessFilter:
 shouldDetectMultipleSelfIdFields

---
 .../JSonDeserializerWithAccessFilter.java     | 54 ++++++++++---------
 .../service/accessfilter/SelfId.java          |  2 +-
 ...nDeserializerWithAccessFilterUnitTest.java |  7 ++-
 3 files changed, 34 insertions(+), 29 deletions(-)

diff --git a/src/main/java/org/hostsharing/hsadminng/service/accessfilter/JSonDeserializerWithAccessFilter.java b/src/main/java/org/hostsharing/hsadminng/service/accessfilter/JSonDeserializerWithAccessFilter.java
index 13e3fbdd..dc9f2f62 100644
--- a/src/main/java/org/hostsharing/hsadminng/service/accessfilter/JSonDeserializerWithAccessFilter.java
+++ b/src/main/java/org/hostsharing/hsadminng/service/accessfilter/JSonDeserializerWithAccessFilter.java
@@ -31,6 +31,24 @@ public class JSonDeserializerWithAccessFilter<T> {
 
     // Jackson deserializes from the JsonParser, thus no input parameter needed.
     public T deserialize() {
+        determineSelfIdField();
+        deserializeValues();
+        checkAccessToModifiedFields();
+        return dto;
+    }
+
+     private void determineSelfIdField() {
+        for (Field field : dto.getClass().getDeclaredFields()) {
+            if (field.isAnnotationPresent(SelfId.class)) {
+                if (selfIdField != null) {
+                    throw new AssertionError("multiple @" + SelfId.class.getSimpleName() + " detected in " + field.getDeclaringClass().getSimpleName());
+                }
+                selfIdField = field;
+            }
+        }
+    }
+
+    private void deserializeValues() {
         treeNode.fieldNames().forEachRemaining(fieldName -> {
             try {
                 final Field field = dto.getClass().getDeclaredField(fieldName);
@@ -41,9 +59,6 @@ public class JSonDeserializerWithAccessFilter<T> {
                 throw new RuntimeException("setting field " + fieldName + " failed", e);
             }
         });
-
-        modifiedFields.forEach(this::checkAccess);
-        return dto;
     }
 
     private Object readValue(final TreeNode treeNode, final Field field) {
@@ -82,29 +97,20 @@ public class JSonDeserializerWithAccessFilter<T> {
         return ReflectionUtil.getValue(dto, selfIdField);
     }
 
-    private void checkAccess(final Field field) {
-        if ( !rememberSelfIdField(field) ) {
-            if (getId() == null) {
-                if (!getLoginUserRole().isAllowedToInit(field)) {
-                    throw new BadRequestAlertException("Initialization of field prohibited for current user", toDisplay(field), "initializationProhibited");
-                }
-            } else if (getId() != null) {
-                if (!getLoginUserRole().isAllowedToUpdate(field)) {
-                    throw new BadRequestAlertException("Update of field prohibited for current user", toDisplay(field), "updateProhibited");
+    private void checkAccessToModifiedFields() {
+        modifiedFields.forEach(field -> {
+            if ( !field.equals(selfIdField) ) {
+                if (getId() == null) {
+                    if (!getLoginUserRole().isAllowedToInit(field)) {
+                        throw new BadRequestAlertException("Initialization of field prohibited for current user", toDisplay(field), "initializationProhibited");
+                    }
+                } else if (getId() != null) {
+                    if (!getLoginUserRole().isAllowedToUpdate(field)) {
+                        throw new BadRequestAlertException("Update of field prohibited for current user", toDisplay(field), "updateProhibited");
+                    }
                 }
             }
-        }
-    }
-
-    private boolean rememberSelfIdField(final Field field) {
-        if ( field.isAnnotationPresent(SelfId.class) ) {
-            if ( selfIdField != null ) {
-                throw new AssertionError("multiple " + SelfId.class + " detected in " + field.getDeclaringClass().getSimpleName() );
-            }
-            selfIdField = field;
-            return true;
-        }
-        return false;
+        });
     }
 
     private String toDisplay(final Field field) {
diff --git a/src/main/java/org/hostsharing/hsadminng/service/accessfilter/SelfId.java b/src/main/java/org/hostsharing/hsadminng/service/accessfilter/SelfId.java
index 8d3a352d..baf95906 100644
--- a/src/main/java/org/hostsharing/hsadminng/service/accessfilter/SelfId.java
+++ b/src/main/java/org/hostsharing/hsadminng/service/accessfilter/SelfId.java
@@ -3,7 +3,7 @@ package org.hostsharing.hsadminng.service.accessfilter;
 import java.lang.annotation.*;
 
 /**
- * Used to mark a field within a DTO as containing the id of a field,
+ * Used to mark a field within a DTO as containing the own id,
  * it's needed to identify an existing entity for update functions.
  * Initialization and update rights have no meaning for such fields,
  * its initialized automatically and never updated.
diff --git a/src/test/java/org/hostsharing/hsadminng/service/accessfilter/JSonDeserializerWithAccessFilterUnitTest.java b/src/test/java/org/hostsharing/hsadminng/service/accessfilter/JSonDeserializerWithAccessFilterUnitTest.java
index 2b131490..9ddafa09 100644
--- a/src/test/java/org/hostsharing/hsadminng/service/accessfilter/JSonDeserializerWithAccessFilterUnitTest.java
+++ b/src/test/java/org/hostsharing/hsadminng/service/accessfilter/JSonDeserializerWithAccessFilterUnitTest.java
@@ -127,15 +127,15 @@ public class JSonDeserializerWithAccessFilterUnitTest {
     }
 
     @Test
-    public void should() throws IOException {
+    public void shouldDetectMultipleSelfIdFields() throws IOException {
         // given
-        givenJSonTree(asJSon(ImmutablePair.of("restrictedField", "Restricted String Value")));
+        givenJSonTree(asJSon(ImmutablePair.of("id", 1111L)));
 
         // when
         Throwable exception = catchThrowable(() -> new JSonDeserializerWithAccessFilter<>(jsonParser, null, GivenDtoWithMultipleSelfId.class).deserialize());
 
         // then
-        assertThat(exception).isInstanceOf(AssertionError.class).hasMessageContaining("xx");
+        assertThat(exception).isInstanceOf(AssertionError.class).hasMessage("multiple @SelfId detected in GivenDtoWithMultipleSelfId");
     }
 
     // --- only fixture code below ---
@@ -183,7 +183,6 @@ public class JSonDeserializerWithAccessFilterUnitTest {
         Long openLongField;
     }
 
-
     public static class GivenDtoWithMultipleSelfId {
 
         @SelfId