From b3add569ff9dd5a190b8fc1a10fbe0578c12c4c6 Mon Sep 17 00:00:00 2001 From: Mustapha Zorgati <15628173+mustaphazorgati@users.noreply.github.com> Date: Sun, 28 Jun 2020 21:55:22 +0200 Subject: [PATCH] fixed new sonarqube code smells and bugs --- ...TaskHistoryEventResourceAssemblerTest.java | 3 +- .../UpdateClassificationAccTest.java | 5 +- .../jobs/WorkbasketCleanupJobAccTest.java | 19 +++---- .../acceptance/task/CompleteTaskAccTest.java | 5 +- .../QueryWorkbasketAccessItemsAccTest.java | 2 - .../rest/ClassificationControllerIntTest.java | 21 ++++---- .../rest/AccessIdControllerIntTest.java | 51 +++++++++---------- .../pro/taskana/common/rest/RestHelper.java | 7 ++- 8 files changed, 54 insertions(+), 59 deletions(-) diff --git a/history/taskana-simplehistory-rest-spring/src/test/java/pro/taskana/TaskHistoryEventResourceAssemblerTest.java b/history/taskana-simplehistory-rest-spring/src/test/java/pro/taskana/TaskHistoryEventResourceAssemblerTest.java index fd2b4fdcf..b5087c813 100644 --- a/history/taskana-simplehistory-rest-spring/src/test/java/pro/taskana/TaskHistoryEventResourceAssemblerTest.java +++ b/history/taskana-simplehistory-rest-spring/src/test/java/pro/taskana/TaskHistoryEventResourceAssemblerTest.java @@ -77,7 +77,8 @@ public class TaskHistoryEventResourceAssemblerTest { .isEqualTo(taskHistoryEventResource.getWorkbasketKey()); assertThat(historyEvent.getAttachmentClassificationKey()) .isEqualTo(taskHistoryEventResource.getAttachmentClassificationKey()); - assertThat(historyEvent.getCreated()).isEqualTo(taskHistoryEventResource.getCreated()); + assertThat(historyEvent.getCreated()) + .isEqualTo(Instant.parse(taskHistoryEventResource.getCreated())); assertThat(historyEvent.getOldValue()).isEqualTo(taskHistoryEventResource.getOldValue()); assertThat(historyEvent.getNewValue()).isEqualTo(taskHistoryEventResource.getNewValue()); assertThat(historyEvent.getPorCompany()).isEqualTo(taskHistoryEventResource.getPorCompany()); diff --git a/lib/taskana-core/src/test/java/acceptance/classification/UpdateClassificationAccTest.java b/lib/taskana-core/src/test/java/acceptance/classification/UpdateClassificationAccTest.java index 754bf36e3..e0c644ff5 100644 --- a/lib/taskana-core/src/test/java/acceptance/classification/UpdateClassificationAccTest.java +++ b/lib/taskana-core/src/test/java/acceptance/classification/UpdateClassificationAccTest.java @@ -152,10 +152,7 @@ class UpdateClassificationAccTest extends AbstractAccTest { classification.setName("NOW IT´S MY TURN"); classification.setDescription("IT SHOULD BE TO LATE..."); ThrowingCallable call = () -> classificationService.updateClassification(classification); - assertThatThrownBy(call) - .isInstanceOf(ConcurrencyException.class) - .describedAs( - "The Classification should not be updated, because it was modified while editing."); + assertThatThrownBy(call).isInstanceOf(ConcurrencyException.class); } @WithAccessId(user = "businessadmin") diff --git a/lib/taskana-core/src/test/java/acceptance/jobs/WorkbasketCleanupJobAccTest.java b/lib/taskana-core/src/test/java/acceptance/jobs/WorkbasketCleanupJobAccTest.java index 908b7a25a..de8938c8c 100644 --- a/lib/taskana-core/src/test/java/acceptance/jobs/WorkbasketCleanupJobAccTest.java +++ b/lib/taskana-core/src/test/java/acceptance/jobs/WorkbasketCleanupJobAccTest.java @@ -5,7 +5,6 @@ import static org.assertj.core.api.Assertions.assertThat; import acceptance.AbstractAccTest; import java.util.List; import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -24,14 +23,8 @@ import pro.taskana.workbasket.internal.jobs.WorkbasketCleanupJob; @ExtendWith(JaasExtension.class) class WorkbasketCleanupJobAccTest extends AbstractAccTest { - WorkbasketService workbasketService; - TaskService taskService; - - @BeforeEach - void before() { - workbasketService = taskanaEngine.getWorkbasketService(); - taskService = taskanaEngine.getTaskService(); - } + WorkbasketService workbasketService = taskanaEngine.getWorkbasketService(); + TaskService taskService = taskanaEngine.getTaskService(); @AfterEach void after() throws Exception { @@ -50,8 +43,8 @@ class WorkbasketCleanupJobAccTest extends AbstractAccTest { .orderByKey(BaseQuery.SortDirection.ASCENDING) .list(); - assertThat(0).isEqualTo(getNumberTaskNotCompleted(workbaskets.get(0).getId())); - assertThat(1).isEqualTo(getNumberTaskCompleted(workbaskets.get(0).getId())); + assertThat(getNumberTaskNotCompleted(workbaskets.get(0).getId())).isZero(); + assertThat(getNumberTaskCompleted(workbaskets.get(0).getId())).isOne(); // Workbasket with completed task will be marked for deletion. workbasketService.deleteWorkbasket(workbaskets.get(0).getId()); @@ -60,7 +53,7 @@ class WorkbasketCleanupJobAccTest extends AbstractAccTest { TaskCleanupJob taskCleanupJob = new TaskCleanupJob(taskanaEngine, null, null); taskCleanupJob.run(); - assertThat(0).isEqualTo(getNumberTaskCompleted(workbaskets.get(0).getId())); + assertThat(getNumberTaskCompleted(workbaskets.get(0).getId())).isZero(); WorkbasketCleanupJob workbasketCleanupJob = new WorkbasketCleanupJob(taskanaEngine, null, null); workbasketCleanupJob.run(); @@ -81,7 +74,7 @@ class WorkbasketCleanupJobAccTest extends AbstractAccTest { .orderByKey(BaseQuery.SortDirection.ASCENDING) .list(); - assertThat(0).isNotEqualTo(getNumberTaskCompleted(workbaskets.get(0).getId())); + assertThat(getNumberTaskCompleted(workbaskets.get(0).getId())).isPositive(); // Workbasket with completed task will be marked for deletion. workbasketService.deleteWorkbasket(workbaskets.get(0).getId()); diff --git a/lib/taskana-core/src/test/java/acceptance/task/CompleteTaskAccTest.java b/lib/taskana-core/src/test/java/acceptance/task/CompleteTaskAccTest.java index 29aa16d8e..febb04156 100644 --- a/lib/taskana-core/src/test/java/acceptance/task/CompleteTaskAccTest.java +++ b/lib/taskana-core/src/test/java/acceptance/task/CompleteTaskAccTest.java @@ -217,8 +217,9 @@ class CompleteTaskAccTest extends AbstractAccTest { Task taskAfterClaim = TASK_SERVICE.forceClaim(createdTask.getId()); assertThat(taskAfterClaim.getOwner()).isEqualTo(CurrentUserContext.getUserid()); - assertThat(beforeForceClaim).isBeforeOrEqualTo(taskAfterClaim.getModified()); - assertThat(beforeForceClaim).isBeforeOrEqualTo(taskAfterClaim.getClaimed()); + assertThat(beforeForceClaim) + .isBeforeOrEqualTo(taskAfterClaim.getModified()) + .isBeforeOrEqualTo(taskAfterClaim.getClaimed()); assertThat(taskAfterClaim.getCreated()).isBeforeOrEqualTo(taskAfterClaim.getModified()); assertThat(taskAfterClaim.getState()).isEqualTo(TaskState.CLAIMED); diff --git a/lib/taskana-core/src/test/java/acceptance/workbasket/QueryWorkbasketAccessItemsAccTest.java b/lib/taskana-core/src/test/java/acceptance/workbasket/QueryWorkbasketAccessItemsAccTest.java index 5c96737ed..e8215a2c1 100644 --- a/lib/taskana-core/src/test/java/acceptance/workbasket/QueryWorkbasketAccessItemsAccTest.java +++ b/lib/taskana-core/src/test/java/acceptance/workbasket/QueryWorkbasketAccessItemsAccTest.java @@ -40,12 +40,10 @@ class QueryWorkbasketAccessItemsAccTest extends AbstractAccTest { columnValueList = workbasketService.createWorkbasketAccessItemQuery().listValues(ACCESS_ID, null); - assertThat(columnValueList).isNotNull(); assertThat(columnValueList).hasSize(10); columnValueList = workbasketService.createWorkbasketAccessItemQuery().listValues(WORKBASKET_KEY, null); - assertThat(columnValueList).isNotNull(); assertThat(columnValueList).hasSize(24); long countEntries = workbasketService.createWorkbasketAccessItemQuery().count(); diff --git a/rest/taskana-rest-spring/src/test/java/pro/taskana/classification/rest/ClassificationControllerIntTest.java b/rest/taskana-rest-spring/src/test/java/pro/taskana/classification/rest/ClassificationControllerIntTest.java index 1f6114363..80722f04c 100644 --- a/rest/taskana-rest-spring/src/test/java/pro/taskana/classification/rest/ClassificationControllerIntTest.java +++ b/rest/taskana-rest-spring/src/test/java/pro/taskana/classification/rest/ClassificationControllerIntTest.java @@ -49,9 +49,9 @@ class ClassificationControllerIntTest { HttpMethod.GET, restHelper.defaultRequest(), ParameterizedTypeReference.forType(ClassificationRepresentationModel.class)); + assertThat(response.getBody()).isNotNull(); assertThat(response.getBody().getLink(IanaLinkRelations.SELF)).isNotNull(); - assertThat(response.getHeaders().getContentType().toString()) - .isEqualTo(MediaTypes.HAL_JSON_VALUE); + assertThat(response.getHeaders().getContentType()).isEqualTo(MediaTypes.HAL_JSON); } @Test @@ -62,6 +62,7 @@ class ClassificationControllerIntTest { HttpMethod.GET, restHelper.defaultRequest(), CLASSIFICATION_SUMMARY_PAGE_MODEL_TYPE); + assertThat(response.getBody()).isNotNull(); assertThat(response.getBody().getLink(IanaLinkRelations.SELF)).isNotNull(); } @@ -73,6 +74,8 @@ class ClassificationControllerIntTest { HttpMethod.GET, restHelper.defaultRequest(), CLASSIFICATION_SUMMARY_PAGE_MODEL_TYPE); + + assertThat(response.getBody()).isNotNull(); assertThat(response.getBody().getLink(IanaLinkRelations.SELF)).isNotNull(); assertThat(response.getBody().getContent()).hasSize(13); } @@ -86,14 +89,10 @@ class ClassificationControllerIntTest { HttpMethod.GET, restHelper.defaultRequest(), CLASSIFICATION_SUMMARY_PAGE_MODEL_TYPE); + assertThat(response.getBody()).isNotNull(); assertThat(response.getBody().getLink(IanaLinkRelations.SELF)).isNotNull(); - assertThat( - response - .getBody() - .getRequiredLink(IanaLinkRelations.SELF) - .getHref() - .endsWith("/api/v1/classifications?domain=DOMAIN_A&sort-by=key&order=asc")) - .isTrue(); + assertThat(response.getBody().getRequiredLink(IanaLinkRelations.SELF).getHref()) + .endsWith("/api/v1/classifications?domain=DOMAIN_A&sort-by=key&order=asc"); assertThat(response.getBody().getContent()).hasSize(17); assertThat(response.getBody().getContent().iterator().next().getKey()).isEqualTo("A12"); } @@ -107,6 +106,7 @@ class ClassificationControllerIntTest { HttpMethod.GET, restHelper.defaultRequest(), CLASSIFICATION_SUMMARY_PAGE_MODEL_TYPE); + assertThat(response.getBody()).isNotNull(); assertThat(response.getBody().getContent()).hasSize(5); assertThat(response.getBody().getContent().iterator().next().getKey()).isEqualTo("L1050"); assertThat(response.getBody().getLink(IanaLinkRelations.SELF)).isNotNull(); @@ -219,6 +219,8 @@ class ClassificationControllerIntTest { HttpMethod.GET, restHelper.defaultRequest(), CLASSIFICATION_SUMMARY_PAGE_MODEL_TYPE); + + assertThat(response.getBody()).isNotNull(); assertThat(response.getBody().getLink(IanaLinkRelations.SELF)).isNotNull(); boolean foundClassificationCreated = false; for (ClassificationSummaryRepresentationModel classification : @@ -290,6 +292,7 @@ class ClassificationControllerIntTest { HttpMethod.GET, request, ParameterizedTypeReference.forType(ClassificationSummaryRepresentationModel.class)); + assertThat(response.getBody()).isNotNull(); assertThat(response.getBody().getName()).isEqualTo("Zustimmungserklärung"); } diff --git a/rest/taskana-rest-spring/src/test/java/pro/taskana/common/rest/AccessIdControllerIntTest.java b/rest/taskana-rest-spring/src/test/java/pro/taskana/common/rest/AccessIdControllerIntTest.java index eb95b6a1d..e8eb424bc 100644 --- a/rest/taskana-rest-spring/src/test/java/pro/taskana/common/rest/AccessIdControllerIntTest.java +++ b/rest/taskana-rest-spring/src/test/java/pro/taskana/common/rest/AccessIdControllerIntTest.java @@ -2,11 +2,11 @@ package pro.taskana.common.rest; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static pro.taskana.common.rest.RestHelper.TEMPLATE; import java.util.ArrayList; import java.util.List; import org.assertj.core.api.ThrowableAssert.ThrowingCallable; -import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.core.ParameterizedTypeReference; @@ -15,7 +15,6 @@ import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.test.context.ActiveProfiles; import org.springframework.web.client.HttpClientErrorException; -import org.springframework.web.client.RestTemplate; import pro.taskana.common.rest.models.AccessIdRepresentationModel; @@ -23,55 +22,55 @@ import pro.taskana.common.rest.models.AccessIdRepresentationModel; @ActiveProfiles({"test"}) class AccessIdControllerIntTest { - private static RestTemplate template; + private final RestHelper restHelper; - @Autowired RestHelper restHelper; - - @BeforeAll - static void init() { - template = RestHelper.TEMPLATE; + @Autowired + AccessIdControllerIntTest(RestHelper restHelper) { + this.restHelper = restHelper; } @Test void testQueryGroupsByDn() { ResponseEntity response = - template.exchange( + TEMPLATE.exchange( restHelper.toUrl(Mapping.URL_ACCESSID) + "?search-for=cn=ksc-users,cn=groups,OU=Test,O=TASKANA", HttpMethod.GET, restHelper.defaultRequest(), ParameterizedTypeReference.forType(AccessIdListResource.class)); - assertThat(response.getBody()).hasSize(1); - assertThat(response.getBody().get(0).getAccessId()) - .isEqualToIgnoringCase("cn=ksc-users,cn=groups,OU=Test,O=TASKANA"); + assertThat(response.getBody()) + .isNotNull() + .extracting(AccessIdRepresentationModel::getAccessId) + .usingElementComparator(String.CASE_INSENSITIVE_ORDER) + .containsExactly("cn=ksc-users,cn=groups,OU=Test,O=TASKANA"); } @Test void testQueryGroupsByCn() { ResponseEntity response = - template.exchange( + TEMPLATE.exchange( restHelper.toUrl(Mapping.URL_ACCESSID) + "?search-for=ksc-use", HttpMethod.GET, restHelper.defaultRequest(), ParameterizedTypeReference.forType(AccessIdListResource.class)); - assertThat(response.getBody()).hasSize(1); - assertThat(response.getBody().get(0).getAccessId()) - .isEqualToIgnoringCase("cn=ksc-users,cn=groups,OU=Test,O=TASKANA"); + assertThat(response.getBody()) + .isNotNull() + .extracting(AccessIdRepresentationModel::getAccessId) + .usingElementComparator(String.CASE_INSENSITIVE_ORDER) + .containsExactly("cn=ksc-users,cn=groups,OU=Test,O=TASKANA"); } @Test void testGetMatches() { ResponseEntity> response = - template.exchange( + TEMPLATE.exchange( restHelper.toUrl(Mapping.URL_ACCESSID) + "?search-for=rig", HttpMethod.GET, restHelper.defaultRequest(), ParameterizedTypeReference.forType(AccessIdListResource.class)); - List body = response.getBody(); - assertThat(body).isNotNull(); - assertThat(body).hasSize(2); - assertThat(body) + assertThat(response.getBody()) + .isNotNull() .extracting(AccessIdRepresentationModel::getName) .containsExactlyInAnyOrder("Schläfrig, Tim", "Eifrig, Elena"); } @@ -79,16 +78,14 @@ class AccessIdControllerIntTest { @Test void should_returnAccessIdWithUmlauten_ifBased64EncodedUserIsLookedUp() { ResponseEntity> response = - template.exchange( + TEMPLATE.exchange( restHelper.toUrl(Mapping.URL_ACCESSID) + "?search-for=läf", HttpMethod.GET, restHelper.defaultRequest(), ParameterizedTypeReference.forType(AccessIdListResource.class)); - List body = response.getBody(); - assertThat(body).isNotNull(); - assertThat(body).hasSize(1); - assertThat(body) + assertThat(response.getBody()) + .isNotNull() .extracting(AccessIdRepresentationModel::getName) .containsExactlyInAnyOrder("Schläfrig, Tim"); } @@ -97,7 +94,7 @@ class AccessIdControllerIntTest { void testBadRequestWhenSearchForIsTooShort() { ThrowingCallable httpCall = () -> { - template.exchange( + TEMPLATE.exchange( restHelper.toUrl(Mapping.URL_ACCESSID) + "?search-for=al", HttpMethod.GET, restHelper.defaultRequest(), diff --git a/rest/taskana-rest-spring/src/test/java/pro/taskana/common/rest/RestHelper.java b/rest/taskana-rest-spring/src/test/java/pro/taskana/common/rest/RestHelper.java index 73776335b..c95cd9a93 100644 --- a/rest/taskana-rest-spring/src/test/java/pro/taskana/common/rest/RestHelper.java +++ b/rest/taskana-rest-spring/src/test/java/pro/taskana/common/rest/RestHelper.java @@ -31,7 +31,12 @@ public class RestHelper { public static final RestTemplate TEMPLATE = getRestTemplate(); - @Autowired Environment environment; + private final Environment environment; + + @Autowired + public RestHelper(Environment environment) { + this.environment = environment; + } public String toUrl(String relativeUrl, Object... uriVariables) { return UriComponentsBuilder.fromPath(relativeUrl)