Fix code style & improve test coverage

pom.xml

@@ -63,7 +63,7 @@
 		<asciidoctor-maven-plugin.version>1.5.2</asciidoctor-maven-plugin.version>
 		<maven-release-plugin.version>2.5.3</maven-release-plugin.version>
 		<license-maven-plugin.version>3.0</license-maven-plugin.version>
-		<jacoco-maven-plugin.version>0.8.0</jacoco-maven-plugin.version>
+		<jacoco-maven-plugin.version>0.8.2</jacoco-maven-plugin.version>
 		<checkstyle-plugin.version>3.0.0</checkstyle-plugin.version>
 		<puppycrawl-tools-checkstyle.version>8.10</puppycrawl-tools-checkstyle.version>
 

src/main/java/it/reply/orchestrator/config/DatasourceConfig.java

@@ -28,6 +28,7 @@
 import net.ttddyy.dsproxy.listener.logging.SLF4JLogLevel;
 import net.ttddyy.dsproxy.support.ProxyDataSourceBuilder;
 
+import org.checkerframework.checker.nullness.qual.NonNull;
 import org.springframework.beans.BeanUtils;
 import org.springframework.beans.BeansException;
 import org.springframework.beans.MutablePropertyValues;
@@ -118,8 +119,8 @@ public SpringLiquibase workflowLiquibase(XADataSourceWrapper wrapper) throws Exc
     return springLiquibase(workflowDataSource(wrapper), workflowLiquibaseProperties());
   }
 
-  private static SpringLiquibase springLiquibase(DataSource dataSource,
-      LiquibaseProperties properties) {
+  private static SpringLiquibase springLiquibase(@NonNull DataSource dataSource,
+      @NonNull LiquibaseProperties properties) {
     SpringLiquibase liquibase = new SpringLiquibase();
     liquibase.setDataSource(dataSource);
     liquibase.setChangeLog(properties.getChangeLog());

src/main/java/it/reply/orchestrator/config/IgniteConfig.java

@@ -16,12 +16,20 @@
 
 package it.reply.orchestrator.config;
 
+import java.util.Objects;
+
+import javax.cache.configuration.Factory;
+import javax.transaction.TransactionManager;
+
+import lombok.AllArgsConstructor;
+
 import org.apache.ignite.Ignite;
 import org.apache.ignite.IgniteCheckedException;
 import org.apache.ignite.IgniteSpring;
 import org.apache.ignite.configuration.IgniteConfiguration;
 import org.apache.ignite.configuration.TransactionConfiguration;
 import org.apache.ignite.logger.slf4j.Slf4jLogger;
+import org.checkerframework.checker.nullness.qual.NonNull;
 import org.springframework.context.ApplicationContext;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
@@ -30,6 +38,18 @@
 @Configuration
 public class IgniteConfig {
 
+  @AllArgsConstructor
+  public static class TransactionManagerFactory implements Factory<TransactionManager> {
+
+    @NonNull
+    private transient TransactionManager transactionManager;
+
+    @Override
+    public TransactionManager create() {
+      return Objects.requireNonNull(transactionManager);
+    }
+  }
+
   /**
    * Generates a new IgniteConfiguration.
    * 
@@ -38,14 +58,15 @@ public class IgniteConfig {
   @Bean
   public IgniteConfiguration igniteConfiguration(JtaTransactionManager transactionManager) {
 
-    TransactionConfiguration txCfg = new TransactionConfiguration()
-        .setTxManagerFactory(() -> transactionManager.getTransactionManager());
+    Factory<TransactionManager> txManagerFactory = new TransactionManagerFactory(
+        transactionManager.getTransactionManager());
 
     return new IgniteConfiguration()
         .setGridLogger(new Slf4jLogger())
         .setClientMode(false)
         .setActiveOnStart(true)
-        .setTransactionConfiguration(txCfg)
+        .setTransactionConfiguration(
+            new TransactionConfiguration().setTxManagerFactory(txManagerFactory))
         .setMetricsLogFrequency(0);
   }
 

src/main/java/it/reply/orchestrator/config/filters/CustomRequestLoggingFilter.java

@@ -20,16 +20,17 @@
 import com.fasterxml.jackson.annotation.JsonProperty;
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.google.common.base.Strings;
+import com.google.common.collect.Lists;
 
 import it.reply.orchestrator.utils.JsonUtils;
 import it.reply.orchestrator.utils.MdcUtils;
 
 import java.io.IOException;
 import java.io.UnsupportedEncodingException;
-import java.util.ArrayList;
-import java.util.List;
+import java.util.HashSet;
 import java.util.Map;
 import java.util.Optional;
+import java.util.Set;
 import java.util.UUID;
 
 import javax.servlet.FilterChain;
@@ -47,6 +48,7 @@
 
 import org.checkerframework.checker.nullness.qual.NonNull;
 import org.checkerframework.checker.nullness.qual.Nullable;
+import org.springframework.http.HttpHeaders;
 import org.springframework.http.server.ServletServerHttpRequest;
 import org.springframework.util.StopWatch;
 import org.springframework.web.filter.OncePerRequestFilter;
@@ -72,8 +74,9 @@ public static class RequestWrapper extends AbstractWrapper {
 
     private String type = "request";
 
-    public RequestWrapper(HttpServletRequest request, int maxPayloadLength) {
-      super(request, maxPayloadLength);
+    public RequestWrapper(HttpServletRequest request, int maxPayloadLength,
+        Set<String> headersToOmit) {
+      super(request, maxPayloadLength, headersToOmit);
     }
 
     @Override
@@ -110,8 +113,8 @@ public static class ResponseWrapper extends AbstractWrapper {
      *          the response time
      */
     public ResponseWrapper(HttpServletRequest request, HttpServletResponse response,
-        int maxPayloadLength, double responseTime) {
-      super(request, maxPayloadLength);
+        int maxPayloadLength, Set<String> headersToOmit, double responseTime) {
+      super(request, maxPayloadLength, headersToOmit);
       this.responseStatus = response.getStatus();
       this.responseTime = responseTime;
     }
@@ -197,13 +200,16 @@ public void setUserFromRequest(HttpServletRequest request) {
      * @param request
      *          the request
      */
-    public void setHeadersFromRequest(HttpServletRequest request) {
-      setHeaders(new ServletServerHttpRequest(request).getHeaders().toSingleValueMap());
-      // for (String headerKey : getHeadersToOmitt()) {
-      // if (headers.containsKey(headerKey)) {
-      // headers.put(headerKey, "<omitted>");
-      // }
-      // }
+    public void setHeadersFromRequest(HttpServletRequest request,
+        @NonNull Set<String> headersToOmit) {
+      HttpHeaders httpHeaders = new ServletServerHttpRequest(request)
+          .getHeaders();
+      if (!headersToOmit.isEmpty()) {
+        httpHeaders.replaceAll((key, value) -> {
+          return headersToOmit.contains(key) ? Lists.newArrayList("<omitted>") : value;
+        });
+      }
+      setHeaders(httpHeaders.toSingleValueMap());
     }
 
     /**
@@ -243,14 +249,15 @@ public void setPayloadFromRequest(HttpServletRequest request, int maxPayloadLeng
      * @param maxPayloadLength
      *          the max payload request to parse
      */
-    public AbstractWrapper(HttpServletRequest request, int maxPayloadLength) {
+    public AbstractWrapper(HttpServletRequest request, int maxPayloadLength,
+        Set<String> headersToOmit) {
       setHttpMethod(request.getMethod());
 
       setUriFromRequest(request.getRequestURI(), request.getQueryString());
       setClientIpFromRequest(request);
       setSessionFromRequest(request);
       setUserFromRequest(request);
-      setHeadersFromRequest(request);
+      setHeadersFromRequest(request, headersToOmit);
       setPayloadFromRequest(request, maxPayloadLength);
     }
   }
@@ -262,7 +269,7 @@ public AbstractWrapper(HttpServletRequest request, int maxPayloadLength) {
   @Getter
   @Setter
   @NonNull
-  private List<String> headersToOmitt = new ArrayList<>();
+  private Set<String> headersToOmit = new HashSet<>();
 
   public boolean isIncludePayload() {
     return isIncludePayload(getMaxPayloadLength());
@@ -288,15 +295,15 @@ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse
         response.addHeader(X_REQUEST_ID, requestId);
       }
 
-      if (isIncludePayload() && isFirstRequest
-          && !(request instanceof ContentCachingRequestWrapper)) {
-        requestToUse = new ContentCachingRequestWrapper(request);
-      }
-
       boolean shouldLog = shouldLog(requestToUse);
+
       if (shouldLog && isFirstRequest) {
+        if (isIncludePayload() && !(request instanceof ContentCachingRequestWrapper)) {
+          requestToUse = new ContentCachingRequestWrapper(request);
+        }
         beforeRequest(requestToUse);
       }
+
       try {
         filterChain.doFilter(requestToUse, response);
       } finally {
@@ -364,7 +371,8 @@ public boolean shouldLog(HttpServletRequest request) {
    */
   protected void beforeRequest(HttpServletRequest request) {
     try {
-      LOG.debug(JsonUtils.serialize(new RequestWrapper(request, getMaxPayloadLength())));
+      LOG.debug(JsonUtils
+          .serialize(new RequestWrapper(request, getMaxPayloadLength(), getHeadersToOmit())));
     } catch (JsonProcessingException ex) {
       LOG.error("Error logging request {}", request, ex);
     }
@@ -377,7 +385,8 @@ protected void afterRequest(HttpServletRequest request, HttpServletResponse resp
       double responseTime) {
     try {
       LOG.debug(JsonUtils.serialize(
-          new ResponseWrapper(request, response, getMaxPayloadLength(), responseTime)));
+          new ResponseWrapper(request, response, getMaxPayloadLength(), getHeadersToOmit(),
+              responseTime)));
     } catch (JsonProcessingException ex) {
       LOG.error("Error logging response {} for request {}", response, request, ex);
     }

src/main/java/it/reply/orchestrator/config/security/AnonimousWebSecurityConfig.java

@@ -18,6 +18,7 @@
 
 import it.reply.orchestrator.config.properties.OidcProperties;
 
+import org.checkerframework.checker.nullness.qual.NonNull;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
@@ -30,7 +31,7 @@
 public class AnonimousWebSecurityConfig extends BaseWebSecurityConfig {
 
   @Override
-  public void configure(HttpSecurity http) throws Exception {
+  public void configure(@NonNull HttpSecurity http) throws Exception {
     super.configure(http);
     http
         .authorizeRequests()

src/main/java/it/reply/orchestrator/config/security/AuthenticatedWebSecurityConfig.java

@@ -29,6 +29,7 @@
 import java.util.HashSet;
 import java.util.Map;
 
+import org.checkerframework.checker.nullness.qual.NonNull;
 import org.mitre.jwt.signer.service.impl.JWKSetCacheService;
 import org.mitre.oauth2.introspectingfilter.service.IntrospectionConfigurationService;
 import org.mitre.oauth2.introspectingfilter.service.impl.JWTParsingIntrospectionConfigurationService;
@@ -149,7 +150,7 @@ public ResourceServerTokenServices introspectingTokenService() {
   }
 
   @Override
-  public void configure(HttpSecurity http) throws Exception {
+  public void configure(@NonNull HttpSecurity http) throws Exception {
     super.configure(http);
     http
         .authorizeRequests()

src/main/java/it/reply/orchestrator/config/security/BaseWebSecurityConfig.java

@@ -18,6 +18,7 @@
 
 import it.reply.orchestrator.config.properties.OidcProperties;
 
+import org.checkerframework.checker.nullness.qual.NonNull;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.autoconfigure.security.SecurityProperties;
 import org.springframework.core.annotation.Order;
@@ -40,7 +41,7 @@ protected void configure(AuthenticationManagerBuilder auth) throws Exception {
   }
 
   @Override
-  public void configure(HttpSecurity http) throws Exception {
+  public void configure(@NonNull HttpSecurity http) throws Exception {
     http
         .csrf()
         .disable()

src/main/java/it/reply/orchestrator/dto/cmdb/ChronosServiceData.java

@@ -17,7 +17,9 @@
 package it.reply.orchestrator.dto.cmdb;
 
 import it.reply.orchestrator.dto.cmdb.ChronosServiceData.ChronosServiceProperties;
+import it.reply.orchestrator.utils.CommonUtils;
 
+import java.util.ArrayList;
 import java.util.List;
 
 import lombok.AccessLevel;
@@ -34,8 +36,17 @@
 @NoArgsConstructor(access = AccessLevel.PROTECTED)
 public class ChronosServiceData extends MesosFrameworkServiceData<ChronosServiceProperties> {
 
-  public ChronosServiceData(@NonNull ChronosServiceProperties properties) {
-    super(properties);
+  @Builder(builderMethodName = "chronosBuilder")
+  public ChronosServiceData(
+      @NonNull String serviceType,
+      @NonNull String endpoint,
+      @NonNull String providerId,
+      @NonNull Type type,
+      boolean publicService,
+      @Nullable String region,
+      @NonNull String hostname,
+      @NonNull ChronosServiceProperties properties) {
+    super(serviceType, endpoint, providerId, type, publicService, region, hostname, properties);
   }
 
   @Data
@@ -48,7 +59,8 @@ public ChronosServiceProperties(
         @Nullable String localVolumesHostBasePath,
         boolean gpuSupport,
         @NonNull List<String> persistentStorageDrivers) {
-      super(localVolumesHostBasePath, gpuSupport, persistentStorageDrivers);
+      super(localVolumesHostBasePath, gpuSupport, CommonUtils
+          .notNullOrDefaultValue(persistentStorageDrivers, ArrayList::new));
     }
   }
 }

src/main/java/it/reply/orchestrator/dto/cmdb/MarathonServiceData.java

@@ -40,9 +40,17 @@
 @NoArgsConstructor(access = AccessLevel.PROTECTED)
 public class MarathonServiceData extends MesosFrameworkServiceData<MarathonServiceProperties> {
 
-  protected MarathonServiceData(
+  @Builder(builderMethodName = "marathonBuilder")
+  public MarathonServiceData(
+      @NonNull String serviceType,
+      @NonNull String endpoint,
+      @NonNull String providerId,
+      @NonNull Type type,
+      boolean publicService,
+      @Nullable String region,
+      @NonNull String hostname,
       @NonNull MarathonServiceProperties properties) {
-    super(properties);
+    super(serviceType, endpoint, providerId, type, publicService, region, hostname, properties);
   }
 
   @Data
@@ -61,7 +69,8 @@ protected MarathonServiceProperties(
         boolean gpuSupport,
         @NonNull List<String> persistentStorageDrivers,
         @NonNull List<String> loadBalancerIps) {
-      super(localVolumesHostBasePath, gpuSupport, persistentStorageDrivers);
+      super(localVolumesHostBasePath, gpuSupport,
+          CommonUtils.notNullOrDefaultValue(persistentStorageDrivers, ArrayList::new));
       this.loadBalancerIps = CommonUtils.notNullOrDefaultValue(loadBalancerIps, ArrayList::new);
     }
 

src/main/java/it/reply/orchestrator/dto/cmdb/MesosFrameworkServiceData.java

@@ -25,6 +25,7 @@
 import lombok.NoArgsConstructor;
 
 import org.checkerframework.checker.nullness.qual.NonNull;
+import org.checkerframework.checker.nullness.qual.Nullable;
 
 @Data
 @EqualsAndHashCode(callSuper = true)
@@ -37,4 +38,16 @@ public class MesosFrameworkServiceData<T extends MesosFrameworkServiceProperties
   @NotNull
   private T properties;
 
+  public MesosFrameworkServiceData(
+      @NonNull String serviceType,
+      @NonNull String endpoint,
+      @NonNull String providerId,
+      @NonNull Type type,
+      boolean publicService,
+      @Nullable String region,
+      @NonNull String hostname,
+      @NonNull T properties) {
+    super(serviceType, endpoint, providerId, type, publicService, region, hostname);
+    this.properties = properties;
+  }
 }

src/main/java/it/reply/orchestrator/dto/cmdb/MesosFrameworkServiceProperties.java

@@ -56,11 +56,18 @@ protected MesosFrameworkServiceProperties() {
     persistentStorageDrivers = new ArrayList<>();
   }
 
-  public String generateLocalVolumesHostPath(String groupId) {
+  /**
+   * Generate the local volume path.
+   *
+   * @param deploymentId
+   *     the deployment ID
+   * @return the path
+   */
+  public String generateLocalVolumesHostPath(String deploymentId) {
     if (StringUtils.isBlank(localVolumesHostBasePath)) {
       throw new ToscaException("Error generating host path: no base host path has been provided");
     } else {
-      return localVolumesHostBasePath.concat(groupId);
+      return localVolumesHostBasePath.concat(deploymentId);
     }
   }
 }