TSK-1281: reduced open bugs and recent code smells

lib/taskana-core/src/main/java/pro/taskana/classification/api/models/Classification.java

@@ -3,7 +3,6 @@ package pro.taskana.classification.api.models;
 import java.time.Instant;
 
 /** Interface used to specify the Classification-Model. */
-// @JsonDeserialize(as = ClassificationImpl.class)
 public interface Classification extends ClassificationSummary {
 
   /**

lib/taskana-core/src/main/java/pro/taskana/classification/internal/ClassificationServiceImpl.java

@@ -283,8 +283,7 @@ public class ClassificationServiceImpl implements ClassificationService {
     }
     // check that the duration is based on format PnD, i.e. it must start with a P, end with a D
     String serviceLevelLower = serviceLevel.toLowerCase();
-    if (!('p' == serviceLevelLower.charAt(0))
+    if (!serviceLevelLower.startsWith("p") || !serviceLevelLower.endsWith("d")) {
-        || !('d' == serviceLevelLower.charAt(serviceLevel.length() - 1))) {
 
       throw new InvalidArgumentException(
           String.format(

lib/taskana-core/src/main/java/pro/taskana/classification/internal/models/ClassificationImpl.java

@@ -35,6 +35,74 @@ public class ClassificationImpl extends ClassificationSummaryImpl implements Cla
     this.applicationEntryPoint = applicationEntryPoint;
   }
 
+  @Override
+  public ClassificationImpl copy(String key) {
+    return new ClassificationImpl(this, key);
+  }
+
+  @Override
+  public Boolean getIsValidInDomain() {
+    return isValidInDomain;
+  }
+
+  @Override
+  public void setIsValidInDomain(Boolean isValidInDomain) {
+    this.isValidInDomain = isValidInDomain;
+  }
+
+  @Override
+  public Instant getCreated() {
+    return created;
+  }
+
+  public void setCreated(Instant created) {
+    this.created = created;
+  }
+
+  @Override
+  public Instant getModified() {
+    return modified;
+  }
+
+  public void setModified(Instant modified) {
+    this.modified = modified;
+  }
+
+  @Override
+  public String getDescription() {
+    return description;
+  }
+
+  @Override
+  public void setDescription(String description) {
+    this.description = description;
+  }
+
+  @Override
+  public ClassificationSummary asSummary() {
+    ClassificationSummaryImpl summary = new ClassificationSummaryImpl();
+    summary.setCategory(this.category);
+    summary.setDomain(this.domain);
+    summary.setId(this.id);
+    summary.setKey(this.key);
+    summary.setName(this.name);
+    summary.setType(this.type);
+    summary.setParentId(this.parentId);
+    summary.setParentKey(this.parentKey);
+    summary.setPriority(this.priority);
+    summary.setServiceLevel(this.serviceLevel);
+    summary.setApplicationEntryPoint(this.applicationEntryPoint);
+    summary.setCustom1(custom1);
+    summary.setCustom2(custom2);
+    summary.setCustom3(custom3);
+    summary.setCustom4(custom4);
+    summary.setCustom5(custom5);
+    summary.setCustom6(custom6);
+    summary.setCustom7(custom7);
+    summary.setCustom8(custom8);
+    return summary;
+  }
+
   protected boolean canEqual(Object other) {
     return (other instanceof ClassificationImpl);
   }
@@ -112,72 +180,4 @@ public class ClassificationImpl extends ClassificationSummaryImpl implements Cla
         + custom8
         + "]";
   }
-
-  @Override
-  public ClassificationImpl copy(String key) {
-    return new ClassificationImpl(this, key);
-  }
-
-  @Override
-  public Boolean getIsValidInDomain() {
-    return isValidInDomain;
-  }
-
-  @Override
-  public void setIsValidInDomain(Boolean isValidInDomain) {
-    this.isValidInDomain = isValidInDomain;
-  }
-
-  @Override
-  public Instant getCreated() {
-    return created;
-  }
-
-  public void setCreated(Instant created) {
-    this.created = created;
-  }
-
-  @Override
-  public Instant getModified() {
-    return modified;
-  }
-
-  public void setModified(Instant modified) {
-    this.modified = modified;
-  }
-
-  @Override
-  public String getDescription() {
-    return description;
-  }
-
-  @Override
-  public void setDescription(String description) {
-    this.description = description;
-  }
-
-  @Override
-  public ClassificationSummary asSummary() {
-    ClassificationSummaryImpl summary = new ClassificationSummaryImpl();
-    summary.setCategory(this.category);
-    summary.setDomain(this.domain);
-    summary.setId(this.id);
-    summary.setKey(this.key);
-    summary.setName(this.name);
-    summary.setType(this.type);
-    summary.setParentId(this.parentId);
-    summary.setParentKey(this.parentKey);
-    summary.setPriority(this.priority);
-    summary.setServiceLevel(this.serviceLevel);
-    summary.setApplicationEntryPoint(this.applicationEntryPoint);
-    summary.setCustom1(custom1);
-    summary.setCustom2(custom2);
-    summary.setCustom3(custom3);
-    summary.setCustom4(custom4);
-    summary.setCustom5(custom5);
-    summary.setCustom6(custom6);
-    summary.setCustom7(custom7);
-    summary.setCustom8(custom8);
-    return summary;
-  }
 }

lib/taskana-core/src/main/java/pro/taskana/common/internal/TaskanaEngineImpl.java

@@ -69,15 +69,15 @@ public class TaskanaEngineImpl implements TaskanaEngine {
 
   private static final String DEFAULT = "default";
   private static final Logger LOGGER = LoggerFactory.getLogger(TaskanaEngineImpl.class);
-  private static SessionStack sessionStack = new SessionStack();
+  private static final SessionStack SESSION_STACK = new SessionStack();
   protected TaskanaEngineConfiguration taskanaEngineConfiguration;
   protected TransactionFactory transactionFactory;
   protected SqlSessionManager sessionManager;
   protected ConnectionManagementMode mode = ConnectionManagementMode.PARTICIPATE;
   protected java.sql.Connection connection = null;
-  private HistoryEventProducer historyEventProducer;
+  private final HistoryEventProducer historyEventProducer;
-  private TaskRoutingManager taskRoutingManager;
+  private final TaskRoutingManager taskRoutingManager;
-  private InternalTaskanaEngineImpl internalTaskanaEngineImpl;
+  private final InternalTaskanaEngineImpl internalTaskanaEngineImpl;
 
   protected TaskanaEngineImpl(TaskanaEngineConfiguration taskanaEngineConfiguration) {
     this.taskanaEngineConfiguration = taskanaEngineConfiguration;
@@ -295,7 +295,7 @@ public class TaskanaEngineImpl implements TaskanaEngine {
    */
   private static class SessionStack {
 
-    private ThreadLocal<Deque<SqlSessionManager>> sessionStack = new ThreadLocal<>();
+    private final ThreadLocal<Deque<SqlSessionManager>> sessionStack = new ThreadLocal<>();
 
     /**
      * Get latest SqlSession from session stack.
@@ -338,15 +338,15 @@ public class TaskanaEngineImpl implements TaskanaEngine {
             e.getCause());
       }
       if (mode != ConnectionManagementMode.EXPLICIT) {
-        sessionStack.pushSessionToStack(sessionManager);
+        SESSION_STACK.pushSessionToStack(sessionManager);
       }
     }
 
     @Override
     public void returnConnection() {
       if (mode != ConnectionManagementMode.EXPLICIT) {
-        sessionStack.popSessionFromStack();
+        SESSION_STACK.popSessionFromStack();
-        if (sessionStack.getSessionStack().isEmpty()
+        if (SESSION_STACK.getSessionStack().isEmpty()
             && sessionManager != null
             && sessionManager.isManagedSessionStarted()) {
           if (mode == ConnectionManagementMode.AUTOCOMMIT) {

lib/taskana-core/src/main/java/pro/taskana/common/internal/security/CurrentUserContext.java

@@ -1,8 +1,8 @@
 package pro.taskana.common.internal.security;
 
 import static pro.taskana.TaskanaEngineConfiguration.shouldUseLowerCaseForAccessIds;
+import static pro.taskana.common.internal.util.CheckedFunction.wrap;
 
-import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.security.AccessController;
 import java.security.Principal;
@@ -42,9 +42,9 @@ public final class CurrentUserContext {
    */
   public static String getUserid() {
     if (runningOnWebSphere()) {
-      return getUseridFromWsSubject();
+      return getUserIdFromWsSubject();
     } else {
-      return getUseridFromJaasSubject();
+      return getUserIdFromJaasSubject();
     }
   }
 
@@ -76,7 +76,7 @@ public final class CurrentUserContext {
    *
    * @return the userid of the caller. If the userid could not be obtained, null is returned.
    */
-  private static String getUseridFromWsSubject() {
+  private static String getUserIdFromWsSubject() {
     try {
       Class<?> wsSubjectClass = Class.forName(WSSUBJECT_CLASSNAME);
       Method getCallerSubjectMethod =
@@ -86,27 +86,24 @@ public final class CurrentUserContext {
       if (callerSubject != null) {
         Set<Object> publicCredentials = callerSubject.getPublicCredentials();
         LOGGER.debug("Public credentials of caller: {}", publicCredentials);
-        for (Object credential : publicCredentials) {
+        return publicCredentials.stream()
-          Object o =
+            .map(
-              credential
+                wrap(
-                  .getClass()
+                    credential ->
-                  .getMethod(GET_UNIQUE_SECURITY_NAME_METHOD, (Class<?>[]) null)
+                        credential
-                  .invoke(credential, (Object[]) null);
+                            .getClass()
-          LOGGER.debug("Returning the unique security name of first public credential: {}", o);
+                            .getMethod(GET_UNIQUE_SECURITY_NAME_METHOD, (Class<?>[]) null)
-          String userIdFound = o.toString();
+                            .invoke(credential, (Object[]) null)))
-          String userIdUsed = userIdFound;
+            .peek(
-          if (shouldUseLowerCaseForAccessIds() && userIdFound != null) {
+                o ->
-            userIdUsed = userIdFound.toLowerCase();
+                    LOGGER.debug(
-          }
+                        "Returning the unique security name of first public credential: {}", o))
-          LOGGER.trace("Found User id {}. Returning User id {} ", userIdFound, userIdUsed);
+            .map(Object::toString)
-          return userIdUsed;
+            .map(CurrentUserContext::convertAccessId)
-        }
+            .findFirst()
+            .orElse(null);
       }
-    } catch (RuntimeException
+    } catch (Exception e) {
-        | ClassNotFoundException
-        | IllegalAccessException
-        | InvocationTargetException
-        | NoSuchMethodException e) {
       LOGGER.warn("Could not get user from WSSubject. Going ahead unauthorized.");
     }
     return null;
@@ -131,7 +128,7 @@ public final class CurrentUserContext {
     return runningOnWebSphere;
   }
 
-  private static String getUseridFromJaasSubject() {
+  private static String getUserIdFromJaasSubject() {
     Subject subject = Subject.getSubject(AccessController.getContext());
     LOGGER.trace("Subject of caller: {}", subject);
     if (subject != null) {

lib/taskana-core/src/main/java/pro/taskana/workbasket/api/models/Workbasket.java

@@ -5,7 +5,6 @@ import java.time.Instant;
 import pro.taskana.workbasket.api.WorkbasketType;
 
 /** Workbasket entity interface. */
-// @JsonDeserialize(as = WorkbasketImpl.class)
 public interface Workbasket extends WorkbasketSummary {
 
   /**

lib/taskana-core/src/main/java/pro/taskana/workbasket/internal/WorkbasketServiceImpl.java

@@ -237,7 +237,7 @@ public class WorkbasketServiceImpl implements WorkbasketService {
             "Method createWorkbasketAccessItem() created workbaskteAccessItem {}", accessItem);
       } catch (PersistenceException e) {
         LOGGER.debug(
-            "when trying to insert WorkbasketAccessItem {} caught exception {}", accessItem, e);
+            "when trying to insert WorkbasketAccessItem {} caught exception", accessItem, e);
         Stream<String> accessItemExistsIdentifier =
             Stream.of(
                 "SQLCODE=-803", // DB2

lib/taskana-core/src/main/java/pro/taskana/workbasket/internal/jobs/WorkbasketCleanupJob.java

@@ -116,7 +116,7 @@ public class WorkbasketCleanupJob extends AbstractTaskanaJob {
         "{} workbasket deleted.", workbasketsToBeDeleted.size() - results.getFailedIds().size());
     for (String failedId : results.getFailedIds()) {
       LOGGER.warn(
-          "Workbasket with id {} could not be deleted. Reason: {}",
+          "Workbasket with id {} could not be deleted. Reason:",
           failedId,
           results.getErrorForId(failedId));
     }

lib/taskana-core/src/test/java/acceptance/classification/UpdateClassificationAccTest.java

@@ -1,6 +1,7 @@
 package acceptance.classification;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatCode;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
 import acceptance.AbstractAccTest;
@@ -16,11 +17,9 @@ import org.junit.jupiter.api.TestTemplate;
 import org.junit.jupiter.api.extension.ExtendWith;
 
 import pro.taskana.classification.api.ClassificationService;
-import pro.taskana.classification.api.exceptions.ClassificationAlreadyExistException;
 import pro.taskana.classification.api.exceptions.ClassificationNotFoundException;
 import pro.taskana.classification.api.models.Classification;
 import pro.taskana.common.api.exceptions.ConcurrencyException;
-import pro.taskana.common.api.exceptions.DomainNotFoundException;
 import pro.taskana.common.api.exceptions.InvalidArgumentException;
 import pro.taskana.common.api.exceptions.NotAuthorizedException;
 import pro.taskana.common.internal.jobs.JobRunner;
@@ -303,15 +302,15 @@ class UpdateClassificationAccTest extends AbstractAccTest {
 
   @WithAccessId(user = "dummy", groups = "businessadmin")
   @Test
-  void testUpdateClassificationWithEmptyServiceLevel()
+  void testUpdateClassificationWithEmptyServiceLevel() throws Exception {
-      throws DomainNotFoundException, ClassificationAlreadyExistException, NotAuthorizedException,
-          InvalidArgumentException, ClassificationNotFoundException, ConcurrencyException {
 
     Classification classification =
         classificationService.newClassification("Key=0818", "DOMAIN_A", "TASK");
     Classification created = classificationService.createClassification(classification);
     created.setServiceLevel("");
-    classificationService.updateClassification(created);
+
+    assertThatCode(() -> classificationService.updateClassification(created))
+        .doesNotThrowAnyException();
   }
 
   @WithAccessId(user = "dummy", groups = "admin")

lib/taskana-core/src/test/resources/log4j2-test.xml

@@ -8,7 +8,7 @@
         </Console>
     </Appenders>
     <Loggers>
-        <Root level="INFO">
+        <Root level="OFF">
             <AppenderRef ref="Console" />
         </Root>
     </Loggers>

rest/taskana-rest-spring-example-common/src/main/java/pro/taskana/rest/security/WebSecurityConfig.java

@@ -70,6 +70,50 @@ public class WebSecurityConfig extends WebSecurityConfigurerAdapter {
     return bean;
   }
 
+  @Bean
+  public DefaultSpringSecurityContextSource defaultSpringSecurityContextSource() {
+    return new DefaultSpringSecurityContextSource(ldapServerUrl + "/" + ldapBaseDn);
+  }
+
+  @Bean
+  public LdapAuthoritiesPopulator authoritiesPopulator() {
+    Function<Map<String, List<String>>, GrantedAuthority> authorityMapper =
+        record -> {
+          String role = record.get("spring.security.ldap.dn").get(0);
+          return new SimpleGrantedAuthority(role);
+        };
+
+    DefaultLdapAuthoritiesPopulator populator =
+        new DefaultLdapAuthoritiesPopulator(
+            defaultSpringSecurityContextSource(), ldapGroupSearchBase);
+    populator.setGroupSearchFilter(ldapGroupSearchFilter);
+    populator.setSearchSubtree(true);
+    populator.setRolePrefix("");
+    populator.setAuthorityMapper(authorityMapper);
+    return populator;
+  }
+
+  @Bean
+  public GrantedAuthoritiesMapper grantedAuthoritiesMapper() {
+    SimpleAuthorityMapper grantedAuthoritiesMapper = new SimpleAuthorityMapper();
+    grantedAuthoritiesMapper.setPrefix("");
+    return grantedAuthoritiesMapper;
+  }
+
+  @Override
+  public void configure(AuthenticationManagerBuilder auth) throws Exception {
+    auth.ldapAuthentication()
+        .userDnPatterns(ldapUserDnPatterns)
+        .groupSearchBase(ldapGroupSearchBase)
+        .ldapAuthoritiesPopulator(authoritiesPopulator())
+        .authoritiesMapper(grantedAuthoritiesMapper())
+        .contextSource()
+        .url(ldapServerUrl + "/" + ldapBaseDn)
+        .and()
+        .passwordCompare()
+        .passwordAttribute("userPassword");
+  }
+
   @Override
   protected void configure(HttpSecurity http) throws Exception {
 
@@ -101,51 +145,7 @@ public class WebSecurityConfig extends WebSecurityConfigurerAdapter {
     }
   }
 
-  @Override
+  protected void addLoginPageConfiguration(HttpSecurity http) throws Exception {
-  public void configure(AuthenticationManagerBuilder auth) throws Exception {
-    auth.ldapAuthentication()
-        .userDnPatterns(ldapUserDnPatterns)
-        .groupSearchBase(ldapGroupSearchBase)
-        .ldapAuthoritiesPopulator(authoritiesPopulator())
-        .authoritiesMapper(grantedAuthoritiesMapper())
-        .contextSource()
-        .url(ldapServerUrl + "/" + ldapBaseDn)
-        .and()
-        .passwordCompare()
-        .passwordAttribute("userPassword");
-  }
-
-  @Bean
-  public DefaultSpringSecurityContextSource defaultSpringSecurityContextSource() {
-    return new DefaultSpringSecurityContextSource(ldapServerUrl + "/" + ldapBaseDn);
-  }
-
-  @Bean
-  public LdapAuthoritiesPopulator authoritiesPopulator() {
-    Function<Map<String, List<String>>, GrantedAuthority> authorityMapper =
-        record -> {
-          String role = record.get("spring.security.ldap.dn").get(0);
-          return new SimpleGrantedAuthority(role);
-        };
-
-    DefaultLdapAuthoritiesPopulator populator =
-        new DefaultLdapAuthoritiesPopulator(
-            defaultSpringSecurityContextSource(), ldapGroupSearchBase);
-    populator.setGroupSearchFilter(ldapGroupSearchFilter);
-    populator.setSearchSubtree(true);
-    populator.setRolePrefix("");
-    populator.setAuthorityMapper(authorityMapper);
-    return populator;
-  }
-
-  @Bean
-  public GrantedAuthoritiesMapper grantedAuthoritiesMapper() {
-    SimpleAuthorityMapper grantedAuthoritiesMapper = new SimpleAuthorityMapper();
-    grantedAuthoritiesMapper.setPrefix("");
-    return grantedAuthoritiesMapper;
-  }
-
-  private void addLoginPageConfiguration(HttpSecurity http) throws Exception {
     http.authorizeRequests()
         .anyRequest()
         .fullyAuthenticated()
@@ -165,6 +165,12 @@ public class WebSecurityConfig extends WebSecurityConfigurerAdapter {
         .permitAll();
   }
 
+  protected JaasApiIntegrationFilter jaasApiIntegrationFilter() {
+    JaasApiIntegrationFilter filter = new JaasApiIntegrationFilter();
+    filter.setCreateEmptySubject(true);
+    return filter;
+  }
+
   private static class CorsWebMvcConfigurer implements WebMvcConfigurer {
 
     @Override
@@ -172,10 +178,4 @@ public class WebSecurityConfig extends WebSecurityConfigurerAdapter {
       registry.addMapping("/**").allowedOrigins("*");
     }
   }
-
-  private JaasApiIntegrationFilter jaasApiIntegrationFilter() {
-    JaasApiIntegrationFilter filter = new JaasApiIntegrationFilter();
-    filter.setCreateEmptySubject(true);
-    return filter;
-  }
 }

rest/taskana-rest-spring-example-wildfly/src/main/java/pro/taskana/wildfly/security/WildflyWebSecurityConfig.java

@@ -7,9 +7,7 @@ import org.springframework.beans.factory.annotation.Value;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.core.annotation.Order;
-import org.springframework.http.HttpMethod;
 import org.springframework.security.authentication.AuthenticationManager;
-import org.springframework.security.config.annotation.web.builders.HttpSecurity;
 import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.AuthenticationException;
@@ -21,8 +19,6 @@ import org.springframework.security.core.userdetails.UsernameNotFoundException;
 import org.springframework.security.web.authentication.preauth.PreAuthenticatedAuthenticationProvider;
 import org.springframework.security.web.authentication.preauth.PreAuthenticatedAuthenticationToken;
 import org.springframework.security.web.authentication.preauth.j2ee.J2eePreAuthenticatedProcessingFilter;
-import org.springframework.security.web.jaasapi.JaasApiIntegrationFilter;
-import org.springframework.security.web.util.matcher.AntPathRequestMatcher;
 import org.wildfly.security.auth.server.SecurityDomain;
 import org.wildfly.security.auth.server.SecurityIdentity;
 import org.wildfly.security.authz.Roles;
@@ -74,64 +70,6 @@ public class WildflyWebSecurityConfig extends WebSecurityConfig {
     return new PreAuthenticatedAuthenticationTokenAuthenticationUserDetailsService();
   }
 
-  @Override
-  protected void configure(HttpSecurity http) throws Exception {
-    http.authorizeRequests()
-        .antMatchers("/css/**", "/img/**")
-        .permitAll()
-        .and()
-        .csrf()
-        .disable()
-        .httpBasic()
-        .and()
-        .authenticationProvider(preauthAuthProvider())
-        .authorizeRequests()
-        .antMatchers(HttpMethod.GET, "/docs/**")
-        .permitAll()
-        .and()
-        .addFilter(preAuthFilter())
-        .addFilterAfter(new ElytronToJaasFilter(), JaasApiIntegrationFilter.class)
-        .addFilter(jaasApiIntegrationFilter());
-
-    if (devMode) {
-      http.headers()
-          .frameOptions()
-          .sameOrigin()
-          .and()
-          .authorizeRequests()
-          .antMatchers("/h2-console/**")
-          .permitAll();
-    } else {
-      addLoginPageConfiguration(http);
-    }
-  }
-
-  private JaasApiIntegrationFilter jaasApiIntegrationFilter() {
-    JaasApiIntegrationFilter filter = new JaasApiIntegrationFilter();
-    filter.setCreateEmptySubject(true);
-    return filter;
-  }
-
-  private void addLoginPageConfiguration(HttpSecurity http) throws Exception {
-    http.authorizeRequests()
-        .anyRequest()
-        .fullyAuthenticated()
-        .and()
-        .formLogin()
-        .loginPage("/login")
-        .failureUrl("/login?error")
-        .defaultSuccessUrl("/")
-        .permitAll()
-        .and()
-        .logout()
-        .invalidateHttpSession(true)
-        .clearAuthentication(true)
-        .logoutRequestMatcher(new AntPathRequestMatcher("/logout"))
-        .logoutSuccessUrl("/login?logout")
-        .deleteCookies("JSESSIONID")
-        .permitAll();
-  }
-
   private static class PreAuthenticatedAuthenticationTokenAuthenticationUserDetailsService
       implements AuthenticationUserDetailsService<PreAuthenticatedAuthenticationToken> {
 

rest/taskana-rest-spring/src/test/java/pro/taskana/common/rest/WebSecurityConfig.java

@@ -117,10 +117,7 @@ public class WebSecurityConfig extends WebSecurityConfigurerAdapter {
 
   @Bean
   public DefaultSpringSecurityContextSource defaultSpringSecurityContextSource() {
-
+    return new DefaultSpringSecurityContextSource(ldapServerUrl + "/" + ldapBaseDn);
-    DefaultSpringSecurityContextSource contextSource =
-        new DefaultSpringSecurityContextSource(ldapServerUrl + "/" + ldapBaseDn);
-    return contextSource;
   }
 
   @Bean