TSK-1414: Fixed most of the new SonaCloud Code Smells since 4.0.2-SNAPSHOT

common/taskana-common-test/src/test/java/pro/taskana/common/test/security/JaasExtensionTest.java

@@ -35,7 +35,7 @@ class JaasExtensionTest {
       dynamicTest("dynamic test", () -> assertThat(CURRENT_USER_CONTEXT.getUserid()).isNotNull());
   private static final DynamicTest NULL_DYNAMIC_TEST =
       dynamicTest(
-          "dynamic test", () -> assertThat(CURRENT_USER_CONTEXT.getUserid()).isEqualTo(null));
+          "dynamic test", () -> assertThat(CURRENT_USER_CONTEXT.getUserid()).isNull());
   private static final DynamicTest DYNAMIC_TEST_USER_DYNAMIC_TEST =
       dynamicTest(
           "dynamic test",
@@ -45,7 +45,7 @@ class JaasExtensionTest {
 
   @BeforeAll
   static void should_NotSetJaasSubject_When_AnnotationIsMissing_On_BeforeAll() {
-    assertThat(CURRENT_USER_CONTEXT.getUserid()).isEqualTo(null);
+    assertThat(CURRENT_USER_CONTEXT.getUserid()).isNull();
   }
 
   @WithAccessId(user = "beforeall")
@@ -58,7 +58,7 @@ class JaasExtensionTest {
   @WithAccessId(user = "beforeall2")
   @BeforeAll
   static void should_NotSetJaasSubject_When_MultipleAnnotationsExist_On_BeforeAll() {
-    assertThat(CURRENT_USER_CONTEXT.getUserid()).isEqualTo(null);
+    assertThat(CURRENT_USER_CONTEXT.getUserid()).isNull();
   }
 
   // endregion
@@ -67,7 +67,7 @@ class JaasExtensionTest {
 
   @BeforeEach
   void should_NotSetJaasSubject_When_AnnotationIsMissing_On_BeforeEach() {
-    assertThat(CURRENT_USER_CONTEXT.getUserid()).isEqualTo(null);
+    assertThat(CURRENT_USER_CONTEXT.getUserid()).isNull();
   }
 
   @WithAccessId(user = "beforeeach")
@@ -80,7 +80,7 @@ class JaasExtensionTest {
   @WithAccessId(user = "beforeeach2")
   @BeforeEach
   void should_NotSetJaasSubject_When_MultipleAnnotationsExist_On_BeforeEach() {
-    assertThat(CURRENT_USER_CONTEXT.getUserid()).isEqualTo(null);
+    assertThat(CURRENT_USER_CONTEXT.getUserid()).isNull();
   }
 
   // endregion
@@ -89,7 +89,7 @@ class JaasExtensionTest {
 
   @AfterEach
   void should_NotSetJaasSubject_When_AnnotationIsMissing_On_AfterEach() {
-    assertThat(CURRENT_USER_CONTEXT.getUserid()).isEqualTo(null);
+    assertThat(CURRENT_USER_CONTEXT.getUserid()).isNull();
   }
 
   @WithAccessId(user = "aftereach")
@@ -102,7 +102,7 @@ class JaasExtensionTest {
   @WithAccessId(user = "afterach2")
   @AfterEach
   void should_NotSetJaasSubject_When_MultipleAnnotationsExist_On_AfterEach() {
-    assertThat(CURRENT_USER_CONTEXT.getUserid()).isEqualTo(null);
+    assertThat(CURRENT_USER_CONTEXT.getUserid()).isNull();
   }
 
   // endregion
@@ -111,7 +111,7 @@ class JaasExtensionTest {
 
   @AfterAll
   static void should_NotSetJaasSubject_When_AnnotationIsMissing_On_AfterAll() {
-    assertThat(CURRENT_USER_CONTEXT.getUserid()).isEqualTo(null);
+    assertThat(CURRENT_USER_CONTEXT.getUserid()).isNull();
   }
 
   @WithAccessId(user = "afterall")
@@ -124,7 +124,7 @@ class JaasExtensionTest {
   @WithAccessId(user = "afterall2")
   @AfterAll
   static void should_NotSetJaasSubject_When_MultipleAnnotationsExist_On_AfterAll() {
-    assertThat(CURRENT_USER_CONTEXT.getUserid()).isEqualTo(null);
+    assertThat(CURRENT_USER_CONTEXT.getUserid()).isNull();
   }
 
   // endregion
@@ -133,7 +133,7 @@ class JaasExtensionTest {
 
   @Test
   void should_NotSetJaasSubject_When_AnnotationIsMissing_On_Test() {
-    assertThat(CURRENT_USER_CONTEXT.getUserid()).isEqualTo(null);
+    assertThat(CURRENT_USER_CONTEXT.getUserid()).isNull();
   }
 
   @WithAccessId(user = "user")
@@ -164,7 +164,7 @@ class JaasExtensionTest {
   @Test
   @Disabled("this can be tested with a org.junit.platform.launcher.TestExecutionListener")
   void should_ThrowException_When_MultipleAnnotationsExist_On_Test() {
-    assertThat(CURRENT_USER_CONTEXT.getUserid()).isEqualTo(null);
+    assertThat(CURRENT_USER_CONTEXT.getUserid()).isNull();
   }
 
   // endregion
@@ -173,7 +173,7 @@ class JaasExtensionTest {
 
   @TestFactory
   List<DynamicTest> should_NotSetJaasSubject_When_AnnotationIsMissing_On_TestFactory() {
-    assertThat(CURRENT_USER_CONTEXT.getUserid()).isEqualTo(null);
+    assertThat(CURRENT_USER_CONTEXT.getUserid()).isNull();
     return Collections.emptyList();
   }
 
@@ -708,7 +708,7 @@ class JaasExtensionTest {
   @Nested
   class ConstructorWithoutAccessId {
     ConstructorWithoutAccessId() {
-      assertThat(CURRENT_USER_CONTEXT.getUserid()).isEqualTo(null);
+      assertThat(CURRENT_USER_CONTEXT.getUserid()).isNull();
     }
 
     @Test

history/taskana-simplehistory-provider/src/main/java/pro/taskana/simplehistory/impl/ClassificationHistoryQueryImpl.java

@@ -32,12 +32,11 @@ public class ClassificationHistoryQueryImpl implements ClassificationHistoryQuer
   private static final String SQL_EXCEPTION_MESSAGE =
       "Method openConnection() could not open a connection to the database.";
 
-  private TaskanaHistoryEngineImpl taskanaHistoryEngine;
+  private final TaskanaHistoryEngineImpl taskanaHistoryEngine;
 
+  private final List<String> orderBy = new ArrayList<>();
+  private final List<String> orderColumns = new ArrayList<>();
   private ClassificationHistoryQueryColumnName columnName;
-  private List<String> orderBy;
-  private List<String> orderColumns;
-
   private String[] idIn;
   private String[] eventTypeIn;
   private TimeInterval[] createdIn;
@@ -61,7 +60,6 @@ public class ClassificationHistoryQueryImpl implements ClassificationHistoryQuer
   private String[] custom6In;
   private String[] custom7In;
   private String[] custom8In;
-
   private String[] eventTypeLike;
   private String[] userIdLike;
   private String[] classificationIdLike;
@@ -85,8 +83,6 @@ public class ClassificationHistoryQueryImpl implements ClassificationHistoryQuer
 
   public ClassificationHistoryQueryImpl(TaskanaHistoryEngineImpl internalTaskanaHistoryEngine) {
     this.taskanaHistoryEngine = internalTaskanaHistoryEngine;
-    this.orderBy = new ArrayList<>();
-    this.orderColumns = new ArrayList<>();
   }
 
   @Override
@@ -469,7 +465,7 @@ public class ClassificationHistoryQueryImpl implements ClassificationHistoryQuer
         this);
     List<String> result = new ArrayList<>();
     this.columnName = dbColumnName;
-    List<String> cacheOrderBy = this.orderBy;
+    List<String> cacheOrderBy = new ArrayList<>(this.orderBy);
     this.orderBy.clear();
     this.addOrderCriteria(columnName.toString(), sortDirection);
 
@@ -481,7 +477,7 @@ public class ClassificationHistoryQueryImpl implements ClassificationHistoryQuer
       LOGGER.error(SQL_EXCEPTION_MESSAGE, e.getCause());
       return result;
     } finally {
-      this.orderBy = cacheOrderBy;
+      this.orderBy.addAll(cacheOrderBy);
       this.columnName = null;
       this.orderColumns.remove(orderColumns.size() - 1);
       taskanaHistoryEngine.returnConnection();
@@ -532,15 +528,6 @@ public class ClassificationHistoryQueryImpl implements ClassificationHistoryQuer
     }
   }
 
-  private ClassificationHistoryQueryImpl addOrderCriteria(
-      String columnName, SortDirection sortDirection) {
-    String orderByDirection =
-        " " + (sortDirection == null ? SortDirection.ASCENDING : sortDirection);
-    orderBy.add(columnName + orderByDirection);
-    orderColumns.add(columnName);
-    return this;
-  }
-
   public String[] getIdIn() {
     return idIn;
   }
@@ -712,4 +699,25 @@ public class ClassificationHistoryQueryImpl implements ClassificationHistoryQuer
   public String[] getCustom8Like() {
     return custom8Like;
   }
+
+  public ClassificationHistoryQueryColumnName getColumnName() {
+    return columnName;
+  }
+
+  public List<String> getOrderBy() {
+    return orderBy;
+  }
+
+  public List<String> getOrderColumns() {
+    return orderColumns;
+  }
+
+  private ClassificationHistoryQueryImpl addOrderCriteria(
+      String columnName, SortDirection sortDirection) {
+    String orderByDirection =
+        " " + (sortDirection == null ? SortDirection.ASCENDING : sortDirection);
+    orderBy.add(columnName + orderByDirection);
+    orderColumns.add(columnName);
+    return this;
+  }
 }

history/taskana-simplehistory-rest-spring/src/test/java/pro/taskana/doc/api/TaskHistoryEventControllerRestDocumentation.java

@@ -15,7 +15,7 @@ import org.springframework.test.web.servlet.result.MockMvcResultMatchers;
 import pro.taskana.common.test.doc.api.BaseRestDocumentation;
 
 /** Generate documentation for the history event controller. */
-public class TaskHistoryEventControllerRestDocumentation extends BaseRestDocumentation {
+class TaskHistoryEventControllerRestDocumentation extends BaseRestDocumentation {
 
 
   private final HashMap<String, String> taskHistoryEventFieldDescriptionsMap = new HashMap<>();
@@ -24,7 +24,7 @@ public class TaskHistoryEventControllerRestDocumentation extends BaseRestDocumen
   private FieldDescriptor[] taskHistoryEventFieldDescriptors;
 
   @BeforeEach
-  public void setUp() {
+  void setUp() {
     taskHistoryEventFieldDescriptionsMap.put("taskHistoryId", "Unique ID");
     taskHistoryEventFieldDescriptionsMap.put("businessProcessId", "The id of the business process");
     taskHistoryEventFieldDescriptionsMap.put(

rest/taskana-rest-spring/src/test/java/pro/taskana/workbasket/rest/WorkbasketAccessItemControllerIntTest.java

@@ -2,12 +2,18 @@ package pro.taskana.workbasket.rest;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static pro.taskana.common.test.rest.RestHelper.TEMPLATE;
 
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Stream;
 import org.assertj.core.api.ThrowableAssert.ThrowingCallable;
-import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.DynamicTest;
 import org.junit.jupiter.api.MethodOrderer;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestFactory;
 import org.junit.jupiter.api.TestMethodOrder;
+import org.junit.jupiter.api.function.ThrowingConsumer;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.core.ParameterizedTypeReference;
 import org.springframework.hateoas.IanaLinkRelations;
@@ -15,7 +21,6 @@ import org.springframework.http.HttpMethod;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.ResponseEntity;
 import org.springframework.web.client.HttpClientErrorException;
-import org.springframework.web.client.RestTemplate;
 
 import pro.taskana.common.rest.Mapping;
 import pro.taskana.common.rest.models.TaskanaPagedModel;
@@ -33,18 +38,18 @@ class WorkbasketAccessItemControllerIntTest {
       WORKBASKET_ACCESS_ITEM_PAGE_MODEL_TYPE =
           new ParameterizedTypeReference<
               TaskanaPagedModel<WorkbasketAccessItemRepresentationModel>>() {};
-  private static RestTemplate template;
-  @Autowired RestHelper restHelper;
 
-  @BeforeAll
+  private final RestHelper restHelper;
-  static void init() {
+
-    template = RestHelper.TEMPLATE;
+  @Autowired
+  WorkbasketAccessItemControllerIntTest(RestHelper restHelper) {
+    this.restHelper = restHelper;
   }
 
   @Test
   void testGetAllWorkbasketAccessItems() {
     ResponseEntity<TaskanaPagedModel<WorkbasketAccessItemRepresentationModel>> response =
-        template.exchange(
+        TEMPLATE.exchange(
             restHelper.toUrl(Mapping.URL_WORKBASKET_ACCESS_ITEMS),
             HttpMethod.GET,
             restHelper.defaultRequest(),
@@ -57,7 +62,7 @@ class WorkbasketAccessItemControllerIntTest {
   void testGetWorkbasketAccessItemsKeepingFilters() {
     String parameters = "?sort-by=workbasket-key&order=asc&page-size=9&access-ids=user-1-1&page=1";
     ResponseEntity<TaskanaPagedModel<WorkbasketAccessItemRepresentationModel>> response =
-        template.exchange(
+        TEMPLATE.exchange(
             restHelper.toUrl(Mapping.URL_WORKBASKET_ACCESS_ITEMS) + parameters,
             HttpMethod.GET,
             restHelper.defaultRequest(),
@@ -77,7 +82,7 @@ class WorkbasketAccessItemControllerIntTest {
   void testThrowsExceptionIfInvalidFilterIsUsed() {
     ThrowingCallable httpCall =
         () ->
-            template.exchange(
+            TEMPLATE.exchange(
                 restHelper.toUrl(Mapping.URL_WORKBASKET_ACCESS_ITEMS)
                     + "?sort-by=workbasket-key&order=asc&page=1&page-size=9&invalid=user-1-1",
                 HttpMethod.GET,
@@ -94,7 +99,7 @@ class WorkbasketAccessItemControllerIntTest {
   void testGetSecondPageSortedByWorkbasketKey() {
     String parameters = "?sort-by=workbasket-key&order=asc&page=2&page-size=9&access-ids=user-1-1";
     ResponseEntity<TaskanaPagedModel<WorkbasketAccessItemRepresentationModel>> response =
-        template.exchange(
+        TEMPLATE.exchange(
             restHelper.toUrl(Mapping.URL_WORKBASKET_ACCESS_ITEMS) + parameters,
             HttpMethod.GET,
             restHelper.defaultRequest(),
@@ -124,7 +129,7 @@ class WorkbasketAccessItemControllerIntTest {
 
     String parameters = "?access-id=teamlead-2";
     ResponseEntity<Void> response =
-        template.exchange(
+        TEMPLATE.exchange(
             restHelper.toUrl(Mapping.URL_WORKBASKET_ACCESS_ITEMS) + parameters,
             HttpMethod.DELETE,
             restHelper.defaultRequest(),
@@ -133,51 +138,29 @@ class WorkbasketAccessItemControllerIntTest {
     assertThat(response.getStatusCode()).isEqualTo(HttpStatus.NO_CONTENT);
   }
 
-  @Test
+  @TestFactory
-  void should_ReturnBadRequest_ifAccessIdIsSubStringOfUser() {
+  Stream<DynamicTest> should_ReturnBadRequest_When_AccessIdIsInvalid() {
-    String parameters = "?access-id=user-1";
+    List<String> accessIds =
-    ThrowingCallable httpCall =
+        Arrays.asList(
-        () ->
+            "cn=organisationseinheit ksc,cn=organisation,ou=test,o=taskana",
-            template.exchange(
+            "cn=monitor-users,cn=groups,ou=test,o=taskana",
-                restHelper.toUrl(Mapping.URL_WORKBASKET_ACCESS_ITEMS) + parameters,
+            "user-1");
-                HttpMethod.DELETE,
-                restHelper.defaultRequest(),
-                ParameterizedTypeReference.forType(Void.class));
-    assertThatThrownBy(httpCall)
-        .isInstanceOf(HttpClientErrorException.class)
-        .extracting(ex -> ((HttpClientErrorException) ex).getStatusCode())
-        .isEqualTo(HttpStatus.BAD_REQUEST);
-  }
 
-  @Test
+    ThrowingConsumer<String> test =
-  void should_ReturnBadRequest_ifAccessIdIsGroup() {
+        accessId -> {
-    String parameters = "?access-id=cn=monitor-users,cn=groups,ou=test,o=taskana";
+          String parameters = "?access-id=" + accessId;
-    ThrowingCallable httpCall =
+          ThrowingCallable httpCall =
-        () ->
+              () ->
-            template.exchange(
+                  TEMPLATE.exchange(
-                restHelper.toUrl(Mapping.URL_WORKBASKET_ACCESS_ITEMS) + parameters,
+                      restHelper.toUrl(Mapping.URL_WORKBASKET_ACCESS_ITEMS) + parameters,
-                HttpMethod.DELETE,
+                      HttpMethod.DELETE,
-                restHelper.defaultRequest(),
+                      restHelper.defaultRequest(),
-                ParameterizedTypeReference.forType(Void.class));
+                      ParameterizedTypeReference.forType(Void.class));
-    assertThatThrownBy(httpCall)
+          assertThatThrownBy(httpCall)
-        .isInstanceOf(HttpClientErrorException.class)
+              .isInstanceOf(HttpClientErrorException.class)
-        .extracting(ex -> ((HttpClientErrorException) ex).getStatusCode())
+              .extracting(ex -> ((HttpClientErrorException) ex).getStatusCode())
-        .isEqualTo(HttpStatus.BAD_REQUEST);
+              .isEqualTo(HttpStatus.BAD_REQUEST);
-  }
+        };
-
+    return DynamicTest.stream(accessIds.iterator(), s -> String.format("for user '%s'", s), test);
-  @Test
-  void should_ReturnBadRequest_ifAccessIdIsOrganizationalGroup() {
-    String parameters = "?access-id=cn=organisationseinheit ksc,cn=organisation,ou=test,o=taskana";
-    ThrowingCallable httpCall =
-        () ->
-            template.exchange(
-                restHelper.toUrl(Mapping.URL_WORKBASKET_ACCESS_ITEMS) + parameters,
-                HttpMethod.DELETE,
-                restHelper.defaultRequest(),
-                ParameterizedTypeReference.forType(Void.class));
-    assertThatThrownBy(httpCall)
-        .isInstanceOf(HttpClientErrorException.class)
-        .extracting(ex -> ((HttpClientErrorException) ex).getStatusCode())
-        .isEqualTo(HttpStatus.BAD_REQUEST);
   }
 }