From cbb5532394029a3738d764c57345e7b61c0961a0 Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Wed, 22 May 2019 21:53:45 +0200 Subject: [PATCH] Deserializer: improved test code coverage for entity associations --- .../hsadminng/domain/UserRoleAssignment.java | 2 +- .../service/accessfilter/AccessMappings.java | 2 + .../accessfilter/JSonAccessFilter.java | 5 +- .../JsonDeserializerWithAccessFilter.java | 44 +++++++---- .../JSonAccessFilterTestFixture.java | 59 ++++++++++++++ ...serializationWithAccessFilterUnitTest.java | 79 +++++++++++++++++++ ...SerializationWithAccessFilterUnitTest.java | 5 ++ 7 files changed, 176 insertions(+), 20 deletions(-) diff --git a/src/main/java/org/hostsharing/hsadminng/domain/UserRoleAssignment.java b/src/main/java/org/hostsharing/hsadminng/domain/UserRoleAssignment.java index cc8453e8..a6174126 100644 --- a/src/main/java/org/hostsharing/hsadminng/domain/UserRoleAssignment.java +++ b/src/main/java/org/hostsharing/hsadminng/domain/UserRoleAssignment.java @@ -197,7 +197,7 @@ public class UserRoleAssignment implements AccessMappings { protected JSonFieldReader 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())); }; } diff --git a/src/main/java/org/hostsharing/hsadminng/service/accessfilter/AccessMappings.java b/src/main/java/org/hostsharing/hsadminng/service/accessfilter/AccessMappings.java index e3b9252e..af8f2c74 100644 --- a/src/main/java/org/hostsharing/hsadminng/service/accessfilter/AccessMappings.java +++ b/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(); } diff --git a/src/main/java/org/hostsharing/hsadminng/service/accessfilter/JSonAccessFilter.java b/src/main/java/org/hostsharing/hsadminng/service/accessfilter/JSonAccessFilter.java index db364afe..b4564d3c 100644 --- a/src/main/java/org/hostsharing/hsadminng/service/accessfilter/JSonAccessFilter.java +++ b/src/main/java/org/hostsharing/hsadminng/service/accessfilter/JSonAccessFilter.java @@ -81,10 +81,11 @@ abstract class JSonAccessFilter { final Class rawType = IdToDtoResolver.class; final Class parentDtoClass = ReflectionUtil. determineGenericInterfaceParameter(parentDtoLoader, rawType, 0); - final Long parentId = ReflectionUtil.getValue(dto, parentIdField); - if (parentId == null) { + final Object parent = ReflectionUtil.getValue(dto, parentIdField); + if (parent == null) { return emptySet(); } + final Long parentId = parent instanceof AccessMappings ? (((AccessMappings) parent).getId()) : (Long) parent; final Set rolesOnParent = getLoginUserDirectRolesFor(parentDtoClass, parentId); final Object parentEntity = loadDto(parentDtoLoader, parentId); 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 8398f01a..f4a05c52 100644 --- a/src/main/java/org/hostsharing/hsadminng/service/accessfilter/JsonDeserializerWithAccessFilter.java +++ b/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 }; } - protected final Long getSubNode(final TreeNode node, final String name) { - if (!node.isObject()) { - throw new IllegalArgumentException(node + " is not a JSON object"); - } + /** + * Returns the named subnode of the given node. + *

+ * 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. + *

+ * + * @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()) { - throw new IllegalArgumentException(node + "." + name + " is not a number"); - } - return subNode.asLong(); + verify(subNode.isNumber(), node + "." + name + " is not a number"); + return subNode; } private Object readValueFromJSon(final TreeNode treeNode, final Field field) { @@ -199,13 +208,11 @@ public abstract class JsonDeserializerWithAccessFilter private void checkAccessToWrittenFields(final T currentDto) { updatingFields.forEach( field -> { - if (!field.equals(selfIdField)) { - final Set roles = getLoginUserRoles(); - if (isInitAccess()) { - validateInitAccess(field, roles); - } else { - validateUpdateAccess(field, roles); - } + final Set roles = getLoginUserRoles(); + if (isInitAccess()) { + validateInitAccess(field, roles); + } else { + validateUpdateAccess(field, roles); } }); } @@ -265,11 +272,14 @@ public abstract class JsonDeserializerWithAccessFilter private 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); } } - } diff --git a/src/test/java/org/hostsharing/hsadminng/service/accessfilter/JSonAccessFilterTestFixture.java b/src/test/java/org/hostsharing/hsadminng/service/accessfilter/JSonAccessFilterTestFixture.java index dac62ce0..0f49d61e 100644 --- a/src/test/java/org/hostsharing/hsadminng/service/accessfilter/JSonAccessFilterTestFixture.java +++ b/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 { @@ -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 { + + @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 { + + @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 { + } + } diff --git a/src/test/java/org/hostsharing/hsadminng/service/accessfilter/JSonDeserializationWithAccessFilterUnitTest.java b/src/test/java/org/hostsharing/hsadminng/service/accessfilter/JSonDeserializationWithAccessFilterUnitTest.java index 23d3d0d2..f835e05f 100644 --- a/src/test/java/org/hostsharing/hsadminng/service/accessfilter/JSonDeserializationWithAccessFilterUnitTest.java +++ b/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 deserializerFor( + final Class clazz, + final GivenChild... qualifier) { + return new JsonDeserializerWithAccessFilter(ctx, userRoleAssignmentService) { + + @Override + protected JSonFieldReader 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); + } + }; + } + } diff --git a/src/test/java/org/hostsharing/hsadminng/service/accessfilter/JSonSerializationWithAccessFilterUnitTest.java b/src/test/java/org/hostsharing/hsadminng/service/accessfilter/JSonSerializationWithAccessFilterUnitTest.java index 42d9c97f..8e98ea52 100644 --- a/src/test/java/org/hostsharing/hsadminng/service/accessfilter/JSonSerializationWithAccessFilterUnitTest.java +++ b/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();