Deserializer: improved test code coverage for entity associations

src/main/java/org/hostsharing/hsadminng/domain/UserRoleAssignment.java

@@ -197,7 +197,7 @@ public class UserRoleAssignment implements AccessMappings {
         protected JSonFieldReader<UserRoleAssignment> jsonFieldReader(final TreeNode treeNode, final Field field) {
             if ("user".equals(field.getName())) {
                 return (final UserRoleAssignment target) -> {
-                    target.setUser(userRepository.getOne(getSubNode(treeNode, "id")));
+                    target.setUser(userRepository.getOne(getSubNode(treeNode, "id").asLong()));
                 };
             }
 

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

@@ -8,4 +8,6 @@ import java.io.Serializable;
  * {@link JsonDeserializerWithAccessFilter}.
  */
 public interface AccessMappings extends Serializable {
+
+    Long getId();
 }

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

@@ -81,10 +81,11 @@ abstract class JSonAccessFilter<T extends AccessMappings> {
         final Class<IdToDtoResolver> rawType = IdToDtoResolver.class;
 
         final Class<?> parentDtoClass = ReflectionUtil.<T> determineGenericInterfaceParameter(parentDtoLoader, rawType, 0);
-        final Long parentId = ReflectionUtil.getValue(dto, parentIdField);
+        final Object parent = ReflectionUtil.getValue(dto, parentIdField);
-        if (parentId == null) {
+        if (parent == null) {
             return emptySet();
         }
+        final Long parentId = parent instanceof AccessMappings ? (((AccessMappings) parent).getId()) : (Long) parent;
         final Set<Role> rolesOnParent = getLoginUserDirectRolesFor(parentDtoClass, parentId);
 
         final Object parentEntity = loadDto(parentDtoLoader, parentId);

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

@@ -1,6 +1,7 @@
 // Licensed under Apache-2.0
 package org.hostsharing.hsadminng.service.accessfilter;
 
+import static com.google.common.base.Verify.verify;
 import static org.hostsharing.hsadminng.service.util.ReflectionUtil.unchecked;
 
 import org.hostsharing.hsadminng.service.UserRoleAssignmentService;
@@ -58,16 +59,24 @@ public abstract class JsonDeserializerWithAccessFilter<T extends AccessMappings>
         };
     }
 
-    protected final Long getSubNode(final TreeNode node, final String name) {
+    /**
-        if (!node.isObject()) {
+     * Returns the named subnode of the given node.
-            throw new IllegalArgumentException(node + " is not a JSON object");
+     * <p>
-        }
+     * If entities are used instead of DTOs, JHipster will generate code which sends
+     * complete entity trees to the REST endpoint. In most cases, we only need the "id",
+     * though.
+     * </p>
+     *
+     * @param node the JSON node of which a subnode is to be returned
+     * @param name the name of the subnode within 'node'
+     * @return the subnode of 'node' with the given 'name'
+     */
+    protected final JsonNode getSubNode(final TreeNode node, final String name) {
+        verify(node.isObject(), node + " is not a JSON object");
         final ObjectNode objectNode = (ObjectNode) node;
         final JsonNode subNode = objectNode.get(name);
-        if (!subNode.isNumber()) {
+        verify(subNode.isNumber(), node + "." + name + " is not a number");
-            throw new IllegalArgumentException(node + "." + name + " is not a number");
+        return subNode;
-        }
-        return subNode.asLong();
     }
 
     private Object readValueFromJSon(final TreeNode treeNode, final Field field) {
@@ -199,13 +208,11 @@ public abstract class JsonDeserializerWithAccessFilter<T extends AccessMappings>
         private void checkAccessToWrittenFields(final T currentDto) {
             updatingFields.forEach(
                     field -> {
-                        if (!field.equals(selfIdField)) {
+                        final Set<Role> roles = getLoginUserRoles();
-                            final Set<Role> roles = getLoginUserRoles();
+                        if (isInitAccess()) {
-                            if (isInitAccess()) {
+                            validateInitAccess(field, roles);
-                                validateInitAccess(field, roles);
+                        } else {
-                            } else {
+                            validateUpdateAccess(field, roles);
-                                validateUpdateAccess(field, roles);
-                            }
                         }
                     });
         }
@@ -265,11 +272,14 @@ public abstract class JsonDeserializerWithAccessFilter<T extends AccessMappings>
         private <F> boolean isActuallyUpdated(final Field field, final T dto, T currentDto) {
             final Object o1 = ReflectionUtil.getValue(dto, field);
             final Object o2 = ReflectionUtil.getValue(currentDto, field);
-            if (o1 != null && o2 != null && o1 instanceof Comparable && o2 instanceof Comparable) {
+
+            if (o1 instanceof Comparable && o2 instanceof Comparable) {
+                verify(
+                        o2 instanceof Comparable,
+                        "either neither or both objects must implement Comparable"); // $COVERAGE-IGNORE$
                 return 0 != ((Comparable) o1).compareTo(o2);
             }
             return ObjectUtils.notEqual(o1, o2);
         }
     }
-
 }

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

@@ -109,6 +109,11 @@ public class JSonAccessFilterTestFixture {
 
         @AccessFor(init = IGNORED, update = IGNORED, read = ANYBODY)
         String displayLabel;
+
+        @Override
+        public Long getId() {
+            return id;
+        }
     }
 
     static abstract class GivenService implements IdToDtoResolver<GivenDto> {
@@ -134,6 +139,11 @@ public class JSonAccessFilterTestFixture {
 
         @AccessFor(init = { TECHNICAL_CONTACT, FINANCIAL_CONTACT }, update = { TECHNICAL_CONTACT, FINANCIAL_CONTACT })
         String restrictedField;
+
+        @Override
+        public Long getId() {
+            return id;
+        }
     }
 
     public static class GivenDtoWithMultipleSelfId implements AccessMappings {
@@ -146,6 +156,10 @@ public class JSonAccessFilterTestFixture {
         @AccessFor(read = Role.ANY_CUSTOMER_USER)
         Long id2;
 
+        @Override
+        public Long getId() {
+            return id;
+        }
     }
 
     public static class GivenDtoWithUnknownFieldType implements AccessMappings {
@@ -157,8 +171,53 @@ public class JSonAccessFilterTestFixture {
         @AccessFor(init = Role.ANYBODY, read = Role.ANYBODY)
         Arbitrary unknown;
 
+        @Override
+        public Long getId() {
+            return id;
+        }
     }
 
     public static class Arbitrary {
     }
+
+    @EntityTypeId("givenParent")
+    public static class GivenParent implements AccessMappings, FluentBuilder<GivenParent> {
+
+        @SelfId(resolver = GivenParentService.class)
+        @AccessFor(read = Role.ANY_CUSTOMER_USER)
+        Long id;
+
+        @Override
+        public Long getId() {
+            return id;
+        }
+
+        public GivenParent id(final long id) {
+            this.id = id;
+            return this;
+        }
+    }
+
+    public static class GivenChild implements AccessMappings, FluentBuilder<GivenChild> {
+
+        @SelfId(resolver = GivenChildService.class)
+        @AccessFor(read = Role.ANY_CUSTOMER_USER)
+        Long id;
+
+        @AccessFor(init = Role.CONTRACTUAL_CONTACT, update = Role.CONTRACTUAL_CONTACT, read = ACTUAL_CUSTOMER_USER)
+        @ParentId(resolver = GivenParentService.class)
+        GivenParent parent;
+
+        @AccessFor(init = { TECHNICAL_CONTACT, FINANCIAL_CONTACT }, update = { TECHNICAL_CONTACT, FINANCIAL_CONTACT })
+        String restrictedField;
+
+        @Override
+        public Long getId() {
+            return id;
+        }
+    }
+
+    static abstract class GivenParentService implements IdToDtoResolver<GivenParent> {
+    }
+
 }

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

@@ -31,6 +31,7 @@ import org.springframework.beans.factory.config.AutowireCapableBeanFactory;
 import org.springframework.context.ApplicationContext;
 
 import java.io.IOException;
+import java.lang.reflect.Field;
 import java.math.BigDecimal;
 import java.time.LocalDate;
 import java.util.Arrays;
@@ -71,6 +72,9 @@ public class JSonDeserializationWithAccessFilterUnitTest {
     @Mock
     private GivenChildService givenChildService;
 
+    @Mock
+    private GivenParentService givenParentService;
+
     @Mock
     private GivenCustomerService givenCustomerService;
     private SecurityContextMock securityContext;
@@ -106,6 +110,9 @@ public class JSonDeserializationWithAccessFilterUnitTest {
                         new GivenCustomerDto()
                                 .with(dto -> dto.id = 888L)));
 
+        given(autowireCapableBeanFactory.createBean(GivenChildService.class)).willReturn(givenChildService);
+        given(autowireCapableBeanFactory.createBean(GivenParentService.class)).willReturn(givenParentService);
+
         given(jsonParser.getCodec()).willReturn(codec);
     }
 
@@ -346,6 +353,26 @@ public class JSonDeserializationWithAccessFilterUnitTest {
         assertThat(actualDto.parentId).isEqualTo(1234L);
     }
 
+    @Test
+    public void shouldResolveParentIdFromIdOfSerializedSubEntity() throws IOException {
+        // given
+        securityContext.havingAuthenticatedUser()
+                .withRole(GivenParent.class, 1234L, Role.CONTRACTUAL_CONTACT);
+        givenJSonTree(
+                asJSon(
+                        ImmutablePair.of(
+                                "parent",
+                                asJSon(
+                                        ImmutablePair.of("id", 1234L)))));
+        given(givenParentService.findOne(1234L)).willReturn(Optional.of(new GivenParent().id(1234)));
+
+        // when
+        final GivenChild actualDto = deserializerFor(GivenChild.class).deserialize(jsonParser, null);
+
+        // then
+        assertThat(actualDto.parent.id).isEqualTo(1234L);
+    }
+
     @Test
     public void shouldNotUpdateFieldIfRequiredRoleIsNotCoveredByUser() throws IOException {
         // given
@@ -417,6 +444,34 @@ public class JSonDeserializationWithAccessFilterUnitTest {
         });
     }
 
+    @Test
+    public void shouldNotDeserializeArrayValue() throws IOException {
+        // given
+        securityContext.havingAuthenticatedUser().withAuthority(AuthoritiesConstants.ADMIN);
+        givenJSonTree(asJSon(ImmutablePair.of("openStringField", Arrays.asList(1, 2))));
+
+        // when
+        final Throwable exception = catchThrowable(
+                () -> deserializerFor(GivenDto.class).deserialize(jsonParser, null));
+
+        // then
+        assertThat(exception).isInstanceOf(NotImplementedException.class);
+    }
+
+    @Test
+    public void shouldNotDeserializeObjectValue() throws IOException {
+        // given
+        securityContext.havingAuthenticatedUser().withAuthority(AuthoritiesConstants.ADMIN);
+        givenJSonTree("{ \"openStringField\": {\"a\": 1, \"b\": 2 } }");
+
+        // when
+        final Throwable exception = catchThrowable(
+                () -> deserializerFor(GivenDto.class).deserialize(jsonParser, null));
+
+        // then
+        assertThat(exception).isInstanceOf(NotImplementedException.class);
+    }
+
     @Test
     public void shouldIgnorePropertyToIgnoreForInit() throws IOException {
         // given
@@ -496,4 +551,28 @@ public class JSonDeserializationWithAccessFilterUnitTest {
             // no need to overload any method here
         };
     }
+
+    public JsonDeserializerWithAccessFilter<GivenChild> deserializerFor(
+            final Class<GivenChild> clazz,
+            final GivenChild... qualifier) {
+        return new JsonDeserializerWithAccessFilter<GivenChild>(ctx, userRoleAssignmentService) {
+
+            @Override
+            protected JSonFieldReader<GivenChild> jsonFieldReader(final TreeNode treeNode, final Field field) {
+                if ("parent".equals(field.getName())) {
+                    return (final GivenChild target) -> {
+                        final long id = getSubNode(treeNode, "id").asLong();
+                        target.parent = givenParentService.findOne(id)
+                                .orElseThrow(
+                                        () -> new BadRequestAlertException(
+                                                GivenParent.class.getSimpleName() + "#" + id + " not found",
+                                                String.valueOf(id),
+                                                "idNotFound"));
+                    };
+                }
+                return super.jsonFieldReader(treeNode, field);
+            }
+        };
+    }
+
 }

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

@@ -190,6 +190,11 @@ public class JSonSerializationWithAccessFilterUnitTest {
 
             @AccessFor(read = Role.ANYBODY)
             Arbitrary fieldWithUnimplementedType = new Arbitrary();
+
+            @Override
+            public Long getId() {
+                return null;
+            }
         }
         final GivenDtoWithUnimplementedFieldType givenDtoWithUnimplementedFieldType = new GivenDtoWithUnimplementedFieldType();
         SecurityContextFake.havingAuthenticatedUser();