From 0b063d57d43540bda9c455c4cc0c48bd36550db3 Mon Sep 17 00:00:00 2001 From: Felix Reichenbach <felix.reichenbach@mgm-tp.com> Date: Wed, 26 Feb 2025 08:47:48 +0100 Subject: [PATCH] OZG-3936 apply code review comments --- .../alfa/common/LinkedResourceProcessor.java | 24 +++++---- .../alfa/RootViewLinkHandlerTest.java | 37 ------------- .../CollaborationVorgangProcessorTest.java | 2 +- ...LinkedResourceCollectionProcessorTest.java | 2 +- .../common/LinkedResourceProcessorTest.java | 52 +++++++++---------- ...edUserProfileResourceSerializerITCase.java | 5 +- 6 files changed, 44 insertions(+), 78 deletions(-) diff --git a/alfa-service/src/main/java/de/ozgcloud/alfa/common/LinkedResourceProcessor.java b/alfa-service/src/main/java/de/ozgcloud/alfa/common/LinkedResourceProcessor.java index dbde196bf7..a39073c1c3 100644 --- a/alfa-service/src/main/java/de/ozgcloud/alfa/common/LinkedResourceProcessor.java +++ b/alfa-service/src/main/java/de/ozgcloud/alfa/common/LinkedResourceProcessor.java @@ -51,13 +51,13 @@ public class LinkedResourceProcessor<T> implements RepresentationModelProcessor< @Override public EntityModel<T> process(EntityModel<T> model) { - addLinkByLinkedResourceAnnotationIfMissing(model); - addLinkByLinkedUserProfileResourceAnnotationIfMissing(model); + addResourceLinksIfMissing(model); + addUserProfileLinksIfMissing(model); return model; } - void addLinkByLinkedResourceAnnotationIfMissing(EntityModel<T> model) { - getFields(LinkedResource.class, model.getContent()) + void addResourceLinksIfMissing(EntityModel<T> model) { + getAnnotatedFields(LinkedResource.class, model.getContent()) .filter(field -> shouldAddLink(model, field)) .forEach(field -> addLinkForLinkedResourceField(model, field)); } @@ -66,16 +66,16 @@ public class LinkedResourceProcessor<T> implements RepresentationModelProcessor< getEntityFieldValue(model.getContent(), field).map(Object::toString).filter(StringUtils::isNotBlank) .ifPresent(val -> model .add(WebMvcLinkBuilder.linkTo(field.getAnnotation(LinkedResource.class).controllerClass()).slash(val) - .withRel(trimIdSuffix(field.getName())))); + .withRel(getResourceName(field)))); } - void addLinkByLinkedUserProfileResourceAnnotationIfMissing(EntityModel<T> resource) { - getFields(LinkedUserProfileResource.class, resource.getContent()) + void addUserProfileLinksIfMissing(EntityModel<T> resource) { + getAnnotatedFields(LinkedUserProfileResource.class, resource.getContent()) .filter(field -> shouldAddLink(resource, field)) .forEach(field -> addLinkForLinkedUserProfileResourceField(resource, field)); } - Stream<Field> getFields(Class<? extends Annotation> annotationClass, T content) { + Stream<Field> getAnnotatedFields(Class<? extends Annotation> annotationClass, T content) { if (Objects.isNull(content)) { return Stream.empty(); } @@ -84,7 +84,8 @@ public class LinkedResourceProcessor<T> implements RepresentationModelProcessor< } boolean shouldAddLink(EntityModel<T> resource, Field field) { - return !(field.getType().isArray() || Collection.class.isAssignableFrom(field.getType()) || resource.hasLink(trimIdSuffix(field.getName()))); + return !(field.getType().isArray() || Collection.class.isAssignableFrom(field.getType()) + || resource.hasLink(getResourceName(field))); } void addLinkForLinkedUserProfileResourceField(EntityModel<T> model, Field field) { @@ -94,7 +95,7 @@ public class LinkedResourceProcessor<T> implements RepresentationModelProcessor< private void addUserProfileLink(EntityModel<T> model, Field field, String value) { Optional.ofNullable(userManagerUrlProvider.getUserProfileTemplate()).filter(StringUtils::isNotBlank) - .ifPresent(template -> model.add(Link.of(template.formatted(value)).withRel(trimIdSuffix(field.getName())))); + .ifPresent(template -> model.add(Link.of(template.formatted(value)).withRel(getResourceName(field)))); } private Optional<Object> getEntityFieldValue(T content, Field field) { @@ -109,7 +110,8 @@ public class LinkedResourceProcessor<T> implements RepresentationModelProcessor< return Optional.empty(); } - private String trimIdSuffix(String fieldName) { + private String getResourceName(Field field) { + var fieldName = field.getName(); if (fieldName.endsWith("Id")) { return fieldName.substring(0, fieldName.indexOf("Id")); } diff --git a/alfa-service/src/test/java/de/ozgcloud/alfa/RootViewLinkHandlerTest.java b/alfa-service/src/test/java/de/ozgcloud/alfa/RootViewLinkHandlerTest.java index fb0c2b9228..6f0e2b6558 100644 --- a/alfa-service/src/test/java/de/ozgcloud/alfa/RootViewLinkHandlerTest.java +++ b/alfa-service/src/test/java/de/ozgcloud/alfa/RootViewLinkHandlerTest.java @@ -706,41 +706,4 @@ class RootViewLinkHandlerTest { } } - - // @DisplayName("Test user assistance documentation link") - // @Nested - // class TestDocumentationLink { - // - // private static final String DOCUMENTATION_URL = "http://alfa-docs"; - // - // @Test - // void shouldNotHaveLinkForUndefinedUrl() { - // var link = viewLinkHandler.toModel(root); - // - // assertThat(link.getLink(RootViewLinkHandler.REL_DOCUMENTATIONS)).isEmpty(); - // } - // - // @Test - // void shouldHaveLinkForDefinedUrl() { - // ReflectionTestUtils.setField(viewLinkHandler, "documentationUrl", - // DOCUMENTATION_URL); - // - // var link = viewLinkHandler.toModel(root); - // - // assertThat(link.getLink(RootViewLinkHandler.REL_DOCUMENTATIONS)) - // .isPresent() - // .map(Link::getHref) - // .hasValue(DOCUMENTATION_URL); - // } - // - // @Test - // void shouldNotHaveLinkForEmptyStringUrl() { - // ReflectionTestUtils.setField(viewLinkHandler, "documentationUrl", ""); - // - // var link = viewLinkHandler.toModel(root); - // - // assertThat(link.getLink(RootViewLinkHandler.REL_DOCUMENTATIONS)).isEmpty(); - // } - // - // } } diff --git a/alfa-service/src/test/java/de/ozgcloud/alfa/collaboration/CollaborationVorgangProcessorTest.java b/alfa-service/src/test/java/de/ozgcloud/alfa/collaboration/CollaborationVorgangProcessorTest.java index 271ce3f758..97079a883a 100644 --- a/alfa-service/src/test/java/de/ozgcloud/alfa/collaboration/CollaborationVorgangProcessorTest.java +++ b/alfa-service/src/test/java/de/ozgcloud/alfa/collaboration/CollaborationVorgangProcessorTest.java @@ -136,7 +136,7 @@ class CollaborationVorgangProcessorTest { } @Test - void shouldHaveFourLinks() { + void shouldHaveThreeLinks() { var model = callProcessor(); assertThat(model.getLinks()).hasSize(3); diff --git a/alfa-service/src/test/java/de/ozgcloud/alfa/common/LinkedResourceCollectionProcessorTest.java b/alfa-service/src/test/java/de/ozgcloud/alfa/common/LinkedResourceCollectionProcessorTest.java index 9759da29d1..d08e2a160e 100644 --- a/alfa-service/src/test/java/de/ozgcloud/alfa/common/LinkedResourceCollectionProcessorTest.java +++ b/alfa-service/src/test/java/de/ozgcloud/alfa/common/LinkedResourceCollectionProcessorTest.java @@ -55,7 +55,7 @@ class LinkedResourceCollectionProcessorTest { } @Test - void shouldCallLinkedResourceprocessor() { + void shouldCallLinkedResourceProcessor() { var entityModel = EntityModel.of(TestEntityTestFactory.create()); var collectionModel = CollectionModel.of(List.of(entityModel)); diff --git a/alfa-service/src/test/java/de/ozgcloud/alfa/common/LinkedResourceProcessorTest.java b/alfa-service/src/test/java/de/ozgcloud/alfa/common/LinkedResourceProcessorTest.java index 9fda7bde37..b0a5df5337 100644 --- a/alfa-service/src/test/java/de/ozgcloud/alfa/common/LinkedResourceProcessorTest.java +++ b/alfa-service/src/test/java/de/ozgcloud/alfa/common/LinkedResourceProcessorTest.java @@ -66,8 +66,8 @@ class LinkedResourceProcessorTest { @BeforeEach void mock() { - doNothing().when(processor).addLinkByLinkedResourceAnnotationIfMissing(any()); - doNothing().when(processor).addLinkByLinkedUserProfileResourceAnnotationIfMissing(any()); + doNothing().when(processor).addResourceLinksIfMissing(any()); + doNothing().when(processor).addUserProfileLinksIfMissing(any()); } @Test @@ -78,22 +78,22 @@ class LinkedResourceProcessorTest { } @Test - void shouldCallAddLinkByLinkedResourceAnnotationIfMissing() { + void shouldCallAddResourceLinksIfMissing() { processor.process(model); - verify(processor).addLinkByLinkedResourceAnnotationIfMissing(model); + verify(processor).addResourceLinksIfMissing(model); } @Test - void shouldCallAddLinkByLinkedUserProfileResourceAnnotationIfMissing() { + void shouldCallAddUserProfileLinksIfMissing() { processor.process(model); - verify(processor).addLinkByLinkedUserProfileResourceAnnotationIfMissing(model); + verify(processor).addUserProfileLinksIfMissing(model); } } @Nested - class TestAddLinkByLinkedResourceAnnotationIfMissing { + class TestAddResourceLinksIfMissing { private final TestEntity entity = TestEntityTestFactory.create(); private final EntityModel<TestEntity> model = EntityModel.of(entity); @@ -103,23 +103,23 @@ class LinkedResourceProcessorTest { @BeforeEach void mock() { - doReturn(Stream.of(field)).when(processor).getFields(any(), any()); + doReturn(Stream.of(field)).when(processor).getAnnotatedFields(any(), any()); } @Test - void shouldGetFieldsWithLinkedResourceAnnotation() { + void shouldGetAnnotatedFieldsWithLinkedResourceAnnotation() { doReturn(false).when(processor).shouldAddLink(any(), any()); - processor.addLinkByLinkedResourceAnnotationIfMissing(model); + processor.addResourceLinksIfMissing(model); - verify(processor).getFields(LinkedResource.class, entity); + verify(processor).getAnnotatedFields(LinkedResource.class, entity); } @Test void shouldCallShouldAddLink() { doReturn(false).when(processor).shouldAddLink(any(), any()); - processor.addLinkByLinkedResourceAnnotationIfMissing(model); + processor.addResourceLinksIfMissing(model); verify(processor).shouldAddLink(model, field); } @@ -128,7 +128,7 @@ class LinkedResourceProcessorTest { void shouldCallAddLinkForLinkedResourceField() { doReturn(true).when(processor).shouldAddLink(any(), any()); - processor.addLinkByLinkedResourceAnnotationIfMissing(model); + processor.addResourceLinksIfMissing(model); verify(processor).addLinkForLinkedResourceField(model, field); } @@ -137,14 +137,14 @@ class LinkedResourceProcessorTest { void shouldNotCallAddLinkForLinkedResourceField() { doReturn(false).when(processor).shouldAddLink(any(), any()); - processor.addLinkByLinkedResourceAnnotationIfMissing(model); + processor.addResourceLinksIfMissing(model); verify(processor, never()).addLinkForLinkedResourceField(any(), any()); } } @Nested - class TestAddLinkByLinkedUserProfileResourceAnnotationIfMissing { + class TestAddUserProfileLinksIfMissing { private final TestEntity entity = TestEntityTestFactory.create(); private final EntityModel<TestEntity> model = EntityModel.of(entity); @@ -154,23 +154,23 @@ class LinkedResourceProcessorTest { @BeforeEach void mock() { - doReturn(Stream.of(field)).when(processor).getFields(any(), any()); + doReturn(Stream.of(field)).when(processor).getAnnotatedFields(any(), any()); } @Test - void shouldGetFieldsWithLinkedUserProfileResourceAnnotation() { + void shouldGetAnnotatedFieldsWithLinkedUserProfileResourceAnnotation() { doReturn(false).when(processor).shouldAddLink(any(), any()); - processor.addLinkByLinkedUserProfileResourceAnnotationIfMissing(model); + processor.addUserProfileLinksIfMissing(model); - verify(processor).getFields(LinkedUserProfileResource.class, entity); + verify(processor).getAnnotatedFields(LinkedUserProfileResource.class, entity); } @Test void shouldCallShouldAddLink() { doReturn(false).when(processor).shouldAddLink(any(), any()); - processor.addLinkByLinkedUserProfileResourceAnnotationIfMissing(model); + processor.addUserProfileLinksIfMissing(model); verify(processor).shouldAddLink(model, field); } @@ -179,7 +179,7 @@ class LinkedResourceProcessorTest { void shouldCallAddLinkForLinkedUserProfileResourceField() { doReturn(true).when(processor).shouldAddLink(any(), any()); - processor.addLinkByLinkedUserProfileResourceAnnotationIfMissing(model); + processor.addUserProfileLinksIfMissing(model); verify(processor).addLinkForLinkedUserProfileResourceField(model, field); } @@ -188,18 +188,18 @@ class LinkedResourceProcessorTest { void shouldNotCallAddLinkForLinkedUserProfileResourceField() { doReturn(false).when(processor).shouldAddLink(any(), any()); - processor.addLinkByLinkedUserProfileResourceAnnotationIfMissing(model); + processor.addUserProfileLinksIfMissing(model); verify(processor, never()).addLinkForLinkedUserProfileResourceField(any(), any()); } } @Nested - class TestGetFields { + class TestGetAnnotatedFields { @Test void shouldReturnEmptyStreamIfContentIsNull() { - var result = processor.getFields(LinkedResource.class, null); + var result = processor.getAnnotatedFields(LinkedResource.class, null); assertThat(result).isEmpty(); } @@ -209,7 +209,7 @@ class LinkedResourceProcessorTest { var entity = TestEntityTestFactory.create(); var expectedFields = List.of(getField("linkedResource"), getField("testId")); - var result = processor.getFields(LinkedResource.class, entity); + var result = processor.getAnnotatedFields(LinkedResource.class, entity); assertThat(result).containsExactlyInAnyOrderElementsOf(expectedFields); } @@ -219,7 +219,7 @@ class LinkedResourceProcessorTest { var entity = TestEntityTestFactory.create(); var expectedFields = List.of(getField("user"), getField("differentUserId")); - var result = processor.getFields(LinkedUserProfileResource.class, entity); + var result = processor.getAnnotatedFields(LinkedUserProfileResource.class, entity); assertThat(result).containsExactlyInAnyOrderElementsOf(expectedFields); } diff --git a/alfa-service/src/test/java/de/ozgcloud/alfa/common/LinkedUserProfileResourceSerializerITCase.java b/alfa-service/src/test/java/de/ozgcloud/alfa/common/LinkedUserProfileResourceSerializerITCase.java index 9da33f6544..ca3058226b 100644 --- a/alfa-service/src/test/java/de/ozgcloud/alfa/common/LinkedUserProfileResourceSerializerITCase.java +++ b/alfa-service/src/test/java/de/ozgcloud/alfa/common/LinkedUserProfileResourceSerializerITCase.java @@ -35,6 +35,7 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import de.ozgcloud.alfa.common.user.UserManagerUrlProvider; +import de.ozgcloud.alfa.common.user.UserProfileTestFactory; import de.ozgcloud.common.test.ITCase; @ITCase @@ -58,10 +59,10 @@ class LinkedUserProfileResourceSerializerITCase { @Test void shouldSerialize() throws JsonProcessingException { - var testObj = TestEntityTestFactory.create(); + var testObj = new LinkedUserProfileResourceTestObject(UserProfileTestFactory.ID); var serialized = objectMapper.writeValueAsString(testObj); - assertThat(serialized).contains("\"user\":\"" + HTTP_LOCALHOST + API_PATH + TestEntityTestFactory.USER + "\""); + assertThat(serialized).isEqualTo("{\"id\":\"" + HTTP_LOCALHOST + API_PATH + UserProfileTestFactory.ID.toString() + "\"}"); } } -- GitLab