Deserializer: improved test code coverage for IGNORE field access

src/main/java/org/hostsharing/hsadminng/service/accessfilter/JSonAccessFilter.java

@@ -82,13 +82,16 @@ abstract class JSonAccessFilter<T extends AccessMappings> {
 
         final Class<?> parentDtoClass = ReflectionUtil.<T> determineGenericInterfaceParameter(parentDtoLoader, rawType, 0);
         final Long parentId = ReflectionUtil.getValue(dto, parentIdField);
+        if (parentId == null) {
+            return emptySet();
+        }
         final Set<Role> rolesOnParent = getLoginUserDirectRolesFor(parentDtoClass, parentId);
 
         final Object parentEntity = loadDto(parentDtoLoader, parentId);
         return union(rolesOnParent, getLoginUserRoleOnAncestorIfHigher(parentEntity));
     }
 
-    private Set<Role> getLoginUserDirectRolesFor(final Class<?> dtoClass, final Long id) {
+    private Set<Role> getLoginUserDirectRolesFor(final Class<?> dtoClass, final long id) {
         if (!SecurityUtils.isAuthenticated()) {
             return emptySet();
         }

src/main/java/org/hostsharing/hsadminng/service/accessfilter/JsonDeserializerWithAccessFilter.java

@@ -199,38 +199,47 @@ public abstract class JsonDeserializerWithAccessFilter<T extends AccessMappings>
         private void checkAccessToWrittenFields(final T currentDto) {
             updatingFields.forEach(
                     field -> {
-                        // TODO this ugly code needs cleanup
                         if (!field.equals(selfIdField)) {
                             final Set<Role> roles = getLoginUserRoles();
                             if (isInitAccess()) {
-                                if (!isAllowedToInit(roles, field)) {
+                                validateInitAccess(field, roles);
-                                    if (!field.equals(parentIdField)) {
+                            } else {
-                                        throw new BadRequestAlertException(
+                                validateUpdateAccess(field, roles);
-                                                "Initialization of field " + toDisplay(field)
-                                                        + " prohibited for current user role(s): "
-                                                        + Joiner.on("+").join(roles),
-                                                toDisplay(field),
-                                                "initializationProhibited");
-                                    } else {
-                                        throw new BadRequestAlertException(
-                                                "Referencing field " + toDisplay(field)
-                                                        + " prohibited for current user role(s): "
-                                                        + Joiner.on("+").join(roles),
-                                                toDisplay(field),
-                                                "referencingProhibited");
-                                    }
-                                }
-                            } else if (!Role.toBeIgnoredForUpdates(field) && !isAllowedToUpdate(getLoginUserRoles(), field)) {
-                                throw new BadRequestAlertException(
-                                        "Update of field " + toDisplay(field) + " prohibited for current user role(s): "
-                                                + Joiner.on("+").join(roles),
-                                        toDisplay(field),
-                                        "updateProhibited");
                             }
                         }
                     });
         }
 
+        private void validateInitAccess(Field field, Set<Role> roles) {
+            if (!Role.toBeIgnoredForUpdates(field) && !isAllowedToInit(roles, field)) {
+                if (!field.equals(parentIdField)) {
+                    throw new BadRequestAlertException(
+                            "Initialization of field " + toDisplay(field)
+                                    + " prohibited for current user role(s): "
+                                    + Joiner.on("+").join(roles),
+                            toDisplay(field),
+                            "initializationProhibited");
+                } else {
+                    throw new BadRequestAlertException(
+                            "Referencing field " + toDisplay(field)
+                                    + " prohibited for current user role(s): "
+                                    + Joiner.on("+").join(roles),
+                            toDisplay(field),
+                            "referencingProhibited");
+                }
+            }
+        }
+
+        private void validateUpdateAccess(Field field, Set<Role> roles) {
+            if (!Role.toBeIgnoredForUpdates(field) && !isAllowedToUpdate(getLoginUserRoles(), field)) {
+                throw new BadRequestAlertException(
+                        "Update of field " + toDisplay(field) + " prohibited for current user role(s): "
+                                + Joiner.on("+").join(roles),
+                        toDisplay(field),
+                        "updateProhibited");
+            }
+        }
+
         private boolean isAllowedToInit(final Set<Role> roles, final Field field) {
             for (Role role : roles) {
                 if (role.isAllowedToInit(field)) {

src/test/java/org/hostsharing/hsadminng/service/accessfilter/JSonAccessFilterTestFixture.java

@@ -106,6 +106,9 @@ public class JSonAccessFilterTestFixture {
 
         @AccessFor(init = ANYBODY, update = ANYBODY, read = ANYBODY)
         int[] openArrayField;
+
+        @AccessFor(init = IGNORED, update = IGNORED, read = ANYBODY)
+        String displayLabel;
     }
 
     static abstract class GivenService implements IdToDtoResolver<GivenDto> {

src/test/java/org/hostsharing/hsadminng/service/accessfilter/JSonDeserializationWithAccessFilterUnitTest.java

@@ -119,7 +119,7 @@ public class JSonDeserializationWithAccessFilterUnitTest {
                         ImmutablePair.of("openStringField", null)));
 
         // when
-        final GivenDto actualDto = deserializerForGivenDto().deserialize(jsonParser, null);
+        final GivenDto actualDto = deserializerFor(GivenDto.class).deserialize(jsonParser, null);
 
         // then
         assertThat(actualDto.openStringField).isNull();
@@ -135,7 +135,7 @@ public class JSonDeserializationWithAccessFilterUnitTest {
                         ImmutablePair.of("openStringField", "String Value")));
 
         // when
-        final GivenDto actualDto = deserializerForGivenDto().deserialize(jsonParser, null);
+        final GivenDto actualDto = deserializerFor(GivenDto.class).deserialize(jsonParser, null);
 
         // then
         assertThat(actualDto.openStringField).isEqualTo("String Value");
@@ -152,7 +152,7 @@ public class JSonDeserializationWithAccessFilterUnitTest {
 
         // when
         // @formatter:off
-        final GivenDto actualDto = deserializerForGivenDto().deserialize(jsonParser, null);;
+        final GivenDto actualDto = deserializerFor(GivenDto.class).deserialize(jsonParser, null);;
         // @formatter:on
 
         // then
@@ -170,7 +170,7 @@ public class JSonDeserializationWithAccessFilterUnitTest {
                         ImmutablePair.of("restrictedBigDecimalField", SOME_BIG_DECIMAL_WITH_ANOTHER_SCALE)));
 
         // when
-        final GivenDto actualDto = deserializerForGivenDto().deserialize(jsonParser, null);
+        final GivenDto actualDto = deserializerFor(GivenDto.class).deserialize(jsonParser, null);
         ;
 
         // then
@@ -201,7 +201,7 @@ public class JSonDeserializationWithAccessFilterUnitTest {
                         ImmutablePair.of("openEnumField", TestEnum.GREEN)));
 
         // when
-        final GivenDto actualDto = deserializerForGivenDto().deserialize(jsonParser, null);
+        final GivenDto actualDto = deserializerFor(GivenDto.class).deserialize(jsonParser, null);
         ;
 
         // then
@@ -227,7 +227,7 @@ public class JSonDeserializationWithAccessFilterUnitTest {
                         ImmutablePair.of("openArrayField", Arrays.asList(11, 22, 33))));
 
         // when
-        Throwable exception = catchThrowable(() -> deserializerForGivenDto().deserialize(jsonParser, null));
+        Throwable exception = catchThrowable(() -> deserializerFor(GivenDto.class).deserialize(jsonParser, null));
 
         // then
         assertThat(exception).isInstanceOf(NotImplementedException.class);
@@ -245,7 +245,7 @@ public class JSonDeserializationWithAccessFilterUnitTest {
                         ImmutablePair.of("restrictedField", "update value of restricted field")));
 
         // when
-        final GivenDto actualDto = deserializerForGivenDto().deserialize(jsonParser, null);
+        final GivenDto actualDto = deserializerFor(GivenDto.class).deserialize(jsonParser, null);
 
         // then
         assertThat(actualDto.restrictedField).isEqualTo("update value of restricted field");
@@ -263,7 +263,7 @@ public class JSonDeserializationWithAccessFilterUnitTest {
                         ImmutablePair.of("restrictedField", "initial value of restricted field")));
 
         // when
-        final GivenDto actualDto = deserializerForGivenDto().deserialize(jsonParser, null);
+        final GivenDto actualDto = deserializerFor(GivenDto.class).deserialize(jsonParser, null);
 
         // then
         assertThat(actualDto.restrictedField).isEqualTo("initial value of restricted field");
@@ -280,7 +280,7 @@ public class JSonDeserializationWithAccessFilterUnitTest {
                         ImmutablePair.of("restrictedField", "updated value of restricted field")));
 
         // when
-        final Throwable exception = catchThrowable(() -> deserializerForGivenDto().deserialize(jsonParser, null));
+        final Throwable exception = catchThrowable(() -> deserializerFor(GivenDto.class).deserialize(jsonParser, null));
 
         // then
         assertThat(exception).isInstanceOfSatisfying(BadRequestAlertException.class, badRequestAlertException -> {
@@ -300,7 +300,7 @@ public class JSonDeserializationWithAccessFilterUnitTest {
                         ImmutablePair.of("restrictedField", "another value of restricted field")));
 
         // when
-        final Throwable exception = catchThrowable(() -> deserializerForGivenDto().deserialize(jsonParser, null));
+        final Throwable exception = catchThrowable(() -> deserializerFor(GivenDto.class).deserialize(jsonParser, null));
 
         // then
         assertThat(exception).isInstanceOfSatisfying(BadRequestAlertException.class, badRequestAlertException -> {
@@ -320,7 +320,7 @@ public class JSonDeserializationWithAccessFilterUnitTest {
 
         // when
         Throwable exception = catchThrowable(
-                () -> deserializerForGivenChildDto().deserialize(jsonParser, null));
+                () -> deserializerFor(GivenChildDto.class).deserialize(jsonParser, null));
 
         // then
         assertThat(exception).isInstanceOfSatisfying(BadRequestAlertException.class, badRequestAlertException -> {
@@ -339,7 +339,7 @@ public class JSonDeserializationWithAccessFilterUnitTest {
                         ImmutablePair.of("parentId", 1234L)));
 
         // when
-        final GivenChildDto actualDto = deserializerForGivenChildDto().deserialize(jsonParser, null);
+        final GivenChildDto actualDto = deserializerFor(GivenChildDto.class).deserialize(jsonParser, null);
         ;
 
         // then
@@ -359,7 +359,7 @@ public class JSonDeserializationWithAccessFilterUnitTest {
 
         // when
         final Throwable exception = catchThrowable(
-                () -> deserializerForGivenDto().deserialize(jsonParser, null));
+                () -> deserializerFor(GivenDto.class).deserialize(jsonParser, null));
 
         // then
         assertThat(exception).isInstanceOfSatisfying(BadRequestAlertException.class, badRequestAlertException -> {
@@ -375,7 +375,7 @@ public class JSonDeserializationWithAccessFilterUnitTest {
 
         // when
         final Throwable exception = catchThrowable(
-                () -> deserializerForGivenDtoWithMultipleSelfId().deserialize(jsonParser, null));
+                () -> deserializerFor(GivenDtoWithMultipleSelfId.class).deserialize(jsonParser, null));
 
         // then
         assertThat(exception).isInstanceOf(AssertionError.class)
@@ -390,7 +390,7 @@ public class JSonDeserializationWithAccessFilterUnitTest {
 
         // when
         final Throwable exception = catchThrowable(
-                () -> deserializerForGivenDtoWithUnknownFieldType().deserialize(jsonParser, null));
+                () -> deserializerFor(GivenDtoWithUnknownFieldType.class).deserialize(jsonParser, null));
 
         // then
         assertThat(exception).isInstanceOf(NotImplementedException.class)
@@ -407,7 +407,7 @@ public class JSonDeserializationWithAccessFilterUnitTest {
 
         // when
         final Throwable exception = catchThrowable(
-                () -> deserializerForGivenDto().deserialize(jsonParser, null));
+                () -> deserializerFor(GivenDto.class).deserialize(jsonParser, null));
 
         // then
         assertThat(exception).isInstanceOfSatisfying(BadRequestAlertException.class, (exc) -> {
@@ -417,6 +417,38 @@ public class JSonDeserializationWithAccessFilterUnitTest {
         });
     }
 
+    @Test
+    public void shouldIgnorePropertyToIgnoreForInit() throws IOException {
+        // given
+        securityContext.havingAuthenticatedUser().withAuthority(AuthoritiesConstants.ADMIN);
+        givenJSonTree(
+                asJSon(
+                        ImmutablePair.of("displayLabel", "Some Value")));
+
+        // when
+        deserializerFor(GivenDto.class).deserialize(jsonParser, null);
+        final Throwable exception = catchThrowable(() -> deserializerFor(GivenDto.class).deserialize(jsonParser, null));
+
+        // then
+        assertThat(exception).isNull();
+    }
+
+    @Test
+    public void shouldIgnorePropertyToIgnoreForUpdate() throws IOException {
+        // given
+        securityContext.havingAuthenticatedUser().withAuthority(AuthoritiesConstants.ADMIN);
+        givenJSonTree(
+                asJSon(
+                        ImmutablePair.of("id", 1234L),
+                        ImmutablePair.of("displayLabel", "Some Value")));
+
+        // when
+        final Throwable exception = catchThrowable(() -> deserializerFor(GivenDto.class).deserialize(jsonParser, null));
+
+        // then
+        assertThat(exception).isNull();
+    }
+
     // --- only fixture code below ---
 
     private void givenJSonTree(String givenJSon) throws IOException {
@@ -425,28 +457,41 @@ public class JSonDeserializationWithAccessFilterUnitTest {
         given(codec.readTree(jsonParser)).willReturn(new ObjectMapper().readTree(givenJSon));
     }
 
-    // We need specialied factories for the deserializer subclasses so that the generic type can be accessed via reflection.
+    // We need specialied factories for the deserializer subclasses
+    // so that the generic type can be accessed via reflection.
     // And it's down here to keep the ugly formatting out of the test cases.
+    // The trick with the unused ...-parameterr might look strange but the reason is
+    // that I wanted the concrete class to be navigable in the tests and
+    // multiple deserializer(Class<...Dto>) methods would have the same erasure
+    // and adding the type the method name, is redundant for the reader.
 
-    public JsonDeserializerWithAccessFilter<GivenDto> deserializerForGivenDto() throws IOException {
+    public JsonDeserializerWithAccessFilter<GivenDto> deserializerFor(
+            final Class<GivenDto> clazz,
+            final GivenDto... qualifier) {
         return new JsonDeserializerWithAccessFilter<GivenDto>(ctx, userRoleAssignmentService) {
             // no need to overload any method here
         };
     }
 
-    public JsonDeserializerWithAccessFilter<GivenChildDto> deserializerForGivenChildDto() throws IOException {
+    public JsonDeserializerWithAccessFilter<GivenChildDto> deserializerFor(
+            final Class<GivenChildDto> clazz,
+            final GivenChildDto... qualifier) {
         return new JsonDeserializerWithAccessFilter<GivenChildDto>(ctx, userRoleAssignmentService) {
             // no need to overload any method here
         };
     }
 
-    private JsonDeserializer<GivenDtoWithMultipleSelfId> deserializerForGivenDtoWithMultipleSelfId() {
+    private JsonDeserializer<GivenDtoWithMultipleSelfId> deserializerFor(
+            final Class<GivenDtoWithMultipleSelfId> clazz,
+            final GivenDtoWithMultipleSelfId... qualifier) {
         return new JsonDeserializerWithAccessFilter<GivenDtoWithMultipleSelfId>(ctx, userRoleAssignmentService) {
             // no need to overload any method here
         };
     }
 
-    private JsonDeserializer<GivenDtoWithUnknownFieldType> deserializerForGivenDtoWithUnknownFieldType() {
+    private JsonDeserializer<GivenDtoWithUnknownFieldType> deserializerFor(
+            final Class<GivenDtoWithUnknownFieldType> clazz,
+            final GivenDtoWithUnknownFieldType... qualifier) {
         return new JsonDeserializerWithAccessFilter<GivenDtoWithUnknownFieldType>(ctx, userRoleAssignmentService) {
             // no need to overload any method here
         };