KNOX-2122 - Code cleanup from static code analysis (apache#191)

* Diamond operator * Static method calls instead of on instances * Remove null check before instanceof * Use computeIfAbsent where possible * Remove simple nested if statements * Remove redundant cast * Use entrySet for iteration of maps Signed-off-by: Kevin Risden <[email protected]>
vedantlodha · Nov 12, 2019 · 2b6cee2 · 2b6cee2
1 parent 6f7d633
commit 2b6cee2
Show file tree

Hide file tree

Showing 56 changed files with 211 additions and 303 deletions.
diff --git a/build-tools/src/main/resources/build-tools/pmd/pmd-ruleset.xml b/build-tools/src/main/resources/build-tools/pmd/pmd-ruleset.xml
@@ -57,6 +57,7 @@ limitations under the License.
     <rule ref="category/java/codestyle.xml/PackageCase" />
     <rule ref="category/java/codestyle.xml/UnnecessaryModifier" />
     <rule ref="category/java/codestyle.xml/UnnecessaryReturn" />
+    <rule ref="category/java/codestyle.xml/UseDiamondOperator" />
 
     <!--<rule ref="category/java/design.xml" />-->
     <!--<rule ref="category/java/documentation.xml" />-->

diff --git a/...o-ldap/src/main/java/org/apache/knox/gateway/security/ldap/SimpleLdapDirectoryServer.java b/...o-ldap/src/main/java/org/apache/knox/gateway/security/ldap/SimpleLdapDirectoryServer.java
@@ -82,8 +82,9 @@ public SimpleLdapDirectoryServer( String rootDn, File usersLdif, Transport... tr
         partitionFactory = new JdbmPartitionFactory();
       }
     } catch ( Exception e ) {
-      LOG.error( "Error instantiating custom partiton factory", e );
-      throw new RuntimeException( e );
+      String msg = "Error instantiating custom partition factory";
+      LOG.error(msg, e );
+      throw new RuntimeException(msg, e);
     }
 
     DirectoryServiceFactory factory = new DefaultDirectoryServiceFactory(directoryService, partitionFactory);

diff --git a/...ava/org/apache/knox/gateway/topology/discovery/ambari/AmbariDynamicServiceURLCreator.java b/...ava/org/apache/knox/gateway/topology/discovery/ambari/AmbariDynamicServiceURLCreator.java
@@ -150,10 +150,8 @@ public List<String> create(String serviceName, Map<String, String> serviceParams
                         log.handlingDerivedProperty(serviceName, configProperty.getType(), configProperty.getName());
                         ServiceURLPropertyConfig.Property p = config.getConfigProperty(serviceName, configProperty.getName());
                         propertyValue = p.getValue();
-                        if (propertyValue == null) {
-                            if (p.getConditionHandler() != null) {
-                                propertyValue = p.getConditionHandler().evaluate(config, cluster);
-                            }
+                        if (propertyValue == null && p.getConditionHandler() != null) {
+                            propertyValue = p.getConditionHandler().evaluate(config, cluster);
                         }
                     }
 

diff --git a/...c/main/java/org/apache/knox/gateway/topology/discovery/ambari/AmbariServiceDiscovery.java b/...c/main/java/org/apache/knox/gateway/topology/discovery/ambari/AmbariServiceDiscovery.java
@@ -171,11 +171,9 @@ private AmbariConfigurationMonitor getConfigurationChangeMonitor() {
                         ClusterConfigurationMonitorService clusterMonitorService =
                               ((GatewayServices) obj).getService(ServiceType.CLUSTER_CONFIGURATION_MONITOR_SERVICE);
                         ClusterConfigurationMonitor monitor =
-                                                 clusterMonitorService.getMonitor(AmbariConfigurationMonitor.getType());
-                        if (monitor != null) {
-                            if (AmbariConfigurationMonitor.class.isAssignableFrom(monitor.getClass())) {
-                                ambariMonitor = (AmbariConfigurationMonitor) monitor;
-                            }
+                            clusterMonitorService.getMonitor(AmbariConfigurationMonitor.getType());
+                        if (monitor != null && AmbariConfigurationMonitor.class.isAssignableFrom(monitor.getClass())) {
+                            ambariMonitor = (AmbariConfigurationMonitor) monitor;
                         }
                     }
                 }
@@ -323,8 +321,9 @@ public Cluster discover(GatewayConfig gatewayConfig, ServiceDiscoveryConfig conf
             }
 
             // Construct the AmbariCluster model
-            for (String componentName : serviceComponents.keySet()) {
-                String serviceName = serviceComponents.get(componentName);
+            for (Entry<String, String> entry : serviceComponents.entrySet()) {
+                String componentName = entry.getKey();
+                String serviceName = entry.getValue();
                 List<String> hostNames = componentHostNames.get(componentName);
 
                 Map<String, AmbariCluster.ServiceConfiguration> configs = serviceConfigurations.get(serviceName);

diff --git a/...main/java/org/apache/knox/gateway/topology/discovery/ambari/ServiceURLPropertyConfig.java b/...main/java/org/apache/knox/gateway/topology/discovery/ambari/ServiceURLPropertyConfig.java
@@ -164,20 +164,16 @@ void setAll(ServiceURLPropertyConfig overrides) {
                     }
 
                     // Add the override properties
-                    for (String propertyName : serviceProperties.keySet()) {
-                        setConfigProperty(service, propertyName, serviceProperties.get(propertyName));
+                    for (Map.Entry<String, Property> entry : serviceProperties.entrySet()) {
+                        setConfigProperty(service, entry.getKey(), entry.getValue());
                     }
                 }
             }
         }
     }
 
     void setConfigProperty(String service, String name, Property value) {
-        Map<String, Property> serviceProperties = properties.get(service);
-        if (serviceProperties == null) {
-            serviceProperties = new HashMap<>();
-            properties.put(service, serviceProperties);
-        }
+        Map<String, Property> serviceProperties = properties.computeIfAbsent(service, k -> new HashMap<>());
         serviceProperties.put(name, value);
     }
 

diff --git a/.../java/org/apache/knox/gateway/topology/discovery/ambari/SparkCommonServiceURLCreator.java b/.../java/org/apache/knox/gateway/topology/discovery/ambari/SparkCommonServiceURLCreator.java
@@ -49,7 +49,6 @@ String getPort(AmbariComponent comp) {
     return comp.getConfigProperty(portConfigProperty);
   }
 
-
   @Override
   public List<String> create(String service, Map<String, String> serviceParams) {
     List<String> urls = new ArrayList<>();
@@ -71,7 +70,4 @@ public List<String> create(String service, Map<String, String> serviceParams) {
 
     return urls;
   }
-
-
-
 }
diff --git a/...n/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscovery.java b/...n/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscovery.java
@@ -191,12 +191,11 @@ private static Cluster discoverCluster(DiscoveryApiClient client, String cluster
             List<ServiceModelGenerator> smgList = serviceModelGenerators.get(service.getType());
             if (smgList != null) {
               for (ServiceModelGenerator serviceModelGenerator : smgList) {
-                if (serviceModelGenerator != null) {
-                  if (serviceModelGenerator.handles(service, serviceConfig, role, roleConfig)) {
-                    serviceModelGenerator.setApiClient(client);
-                    ServiceModel serviceModel = serviceModelGenerator.generateService(service, serviceConfig, role, roleConfig);
-                    serviceModels.add(serviceModel);
-                  }
+                if (serviceModelGenerator != null &&
+                    serviceModelGenerator.handles(service, serviceConfig, role, roleConfig)) {
+                  serviceModelGenerator.setApiClient(client);
+                  ServiceModel serviceModel = serviceModelGenerator.generateService(service, serviceConfig, role, roleConfig);
+                  serviceModels.add(serviceModel);
                 }
               }
             }

diff --git a/...ry-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/DiscoveryApiClient.java b/...ry-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/DiscoveryApiClient.java
@@ -138,10 +138,8 @@ public String buildUrl(String path, List<Pair> queryParams) {
   private String getUsername() {
     String username = null;
     Authentication basicAuth = getAuthentication("basic");
-    if (basicAuth != null) {
-      if (basicAuth instanceof HttpBasicAuth) {
-        username = ((HttpBasicAuth) basicAuth).getUsername();
-      }
+    if (basicAuth instanceof HttpBasicAuth) {
+      username = ((HttpBasicAuth) basicAuth).getUsername();
     }
     return username;
   }

diff --git a/gateway-i18n/src/main/java/org/apache/knox/gateway/i18n/messages/MessagesInvoker.java b/gateway-i18n/src/main/java/org/apache/knox/gateway/i18n/messages/MessagesInvoker.java
@@ -89,11 +89,9 @@ private static Throwable findLoggableThrowable( final MessageLogger logger, fina
         Object arg = args[i];
         if( arg instanceof Throwable ) {
           StackTrace anno = getStackTraceAnno( method, i );
-          if( anno != null ) {
-            if( logger.isLoggable( anno.level() ) ) {
-              throwable = (Throwable)arg;
-              break;
-            }
+          if( anno != null && logger.isLoggable( anno.level() ) ) {
+            throwable = (Throwable)arg;
+            break;
           }
         }
       }

diff --git a/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/dispatch/AtlasHaDispatch.java b/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/dispatch/AtlasHaDispatch.java
@@ -15,7 +15,6 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-
 package org.apache.knox.gateway.ha.dispatch;
 
 import org.apache.http.Header;
@@ -26,25 +25,15 @@
 import javax.servlet.http.HttpServletResponse;
 import java.io.IOException;
 import java.util.Collections;
-import java.util.HashSet;
 import java.util.Set;
 
 public class AtlasHaDispatch extends DefaultHaDispatch {
-    private static Set<String> REQUEST_EXCLUDE_HEADERS = new HashSet<>();
-
-    static {
-        REQUEST_EXCLUDE_HEADERS.add("Content-Length");
-    }
+    private static final Set<String> REQUEST_EXCLUDE_HEADERS = Collections.singleton("Content-Length");
 
     public AtlasHaDispatch() {
         setServiceRole("ATLAS");
     }
 
-    @Override
-    public void init() {
-        super.init();
-    }
-
     @Override
     public Set<String> getOutboundResponseExcludeHeaders() {
         return Collections.emptySet();

diff --git a/...org/apache/knox/gateway/identityasserter/common/filter/CommonIdentityAssertionFilter.java b/...org/apache/knox/gateway/identityasserter/common/filter/CommonIdentityAssertionFilter.java
@@ -96,7 +96,7 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
 
   private String[] combineGroupMappings(String[] mappedGroups, String[] groups) {
     if (mappedGroups != null && groups != null) {
-      return (String[])ArrayUtils.addAll(mappedGroups, groups);
+      return ArrayUtils.addAll(mappedGroups, groups);
     }
     else {
       return groups != null ? groups : mappedGroups;

diff --git a/.../java/org/apache/knox/gateway/filter/rewrite/ext/UrlRewriteActionRewriteProcessorExt.java b/.../java/org/apache/knox/gateway/filter/rewrite/ext/UrlRewriteActionRewriteProcessorExt.java
@@ -29,7 +29,6 @@ public class UrlRewriteActionRewriteProcessorExt
     implements UrlRewriteStepProcessor<UrlRewriteActionRewriteDescriptorExt> {
 
   private Template template;
-  private Expander expander;
 
   @Override
   public String getType() {
@@ -38,7 +37,6 @@ public String getType() {
 
   @Override
   public void initialize( UrlRewriteEnvironment environment, UrlRewriteActionRewriteDescriptorExt descriptor ) throws Exception {
-    this.expander = new Expander();
     if ( descriptor.parameter() != null ) {
       this.template = Parser.parseTemplate( descriptor.parameter() );
     } else {
@@ -48,7 +46,7 @@ public void initialize( UrlRewriteEnvironment environment, UrlRewriteActionRewri
 
   @Override
   public UrlRewriteStepStatus process( UrlRewriteContext context ) throws Exception {
-    Template rewritten = expander.expandToTemplate( template, context.getParameters(), context.getEvaluator() );
+    Template rewritten = Expander.expandToTemplate( template, context.getParameters(), context.getEvaluator() );
     context.setCurrentUrl( rewritten );
     return UrlRewriteStepStatus.SUCCESS;
   }

diff --git a/.../java/org/apache/knox/gateway/filter/rewrite/impl/UrlRewriteFunctionProcessorFactory.java b/.../java/org/apache/knox/gateway/filter/rewrite/impl/UrlRewriteFunctionProcessorFactory.java
@@ -66,11 +66,8 @@ private static Map<Class<? extends UrlRewriteFunctionDescriptor>,Map<String,Clas
     ServiceLoader<UrlRewriteFunctionProcessor> processors = ServiceLoader.load( UrlRewriteFunctionProcessor.class );
     for( UrlRewriteFunctionProcessor processor : processors ) {
       Class<? extends UrlRewriteFunctionDescriptor> descriptorInterface = getDescriptorInterface( processor );
-      Map<String,Class<? extends UrlRewriteFunctionProcessor>> typeMap = descriptorMap.get( descriptorInterface );
-      if( typeMap == null ) {
-        typeMap = new HashMap<>();
-        descriptorMap.put( descriptorInterface, typeMap );
-      }
+      Map<String, Class<? extends UrlRewriteFunctionProcessor>> typeMap = descriptorMap
+          .computeIfAbsent(descriptorInterface, k -> new HashMap<>());
       String functionName = processor.name();
       typeMap.put( functionName, processor.getClass() );
     }

diff --git a/...main/java/org/apache/knox/gateway/filter/rewrite/impl/UrlRewriteStepProcessorFactory.java b/...main/java/org/apache/knox/gateway/filter/rewrite/impl/UrlRewriteStepProcessorFactory.java
@@ -62,11 +62,8 @@ private static Map<Class<? extends UrlRewriteStepDescriptor>,Map<String,Class<?
     ServiceLoader<UrlRewriteStepProcessor> processors = ServiceLoader.load( UrlRewriteStepProcessor.class );
     for( UrlRewriteStepProcessor processor : processors ) {
       Class<? extends UrlRewriteStepDescriptor> descriptorInterface = getDescriptorInterface( processor );
-      Map<String,Class<? extends UrlRewriteStepProcessor>> typeMap = descriptorMap.get( descriptorInterface );
-      if( typeMap == null ) {
-        typeMap = new HashMap<>();
-        descriptorMap.put( descriptorInterface, typeMap );
-      }
+      Map<String, Class<? extends UrlRewriteStepProcessor>> typeMap = descriptorMap
+          .computeIfAbsent(descriptorInterface, k -> new HashMap<>());
       String processorType = processor.getType();
       typeMap.put( processorType, processor.getClass() );
     }

diff --git a/...rite/src/main/java/org/apache/knox/gateway/filter/rewrite/impl/json/JsonFilterReader.java b/...rite/src/main/java/org/apache/knox/gateway/filter/rewrite/impl/json/JsonFilterReader.java
@@ -189,10 +189,8 @@ private void processStartObject() throws IOException {
         throw new IllegalStateException();
       }
     }
-    if( bufferingLevel == null ) {
-      if( !startBuffering( child ) ) {
-        generator.writeStartObject();
-      }
+    if( bufferingLevel == null && !startBuffering( child ) ) {
+      generator.writeStartObject();
     }
   }
 
@@ -253,10 +251,8 @@ private void processStartArray() throws IOException {
         throw new IllegalStateException();
       }
     }
-    if( bufferingLevel == null ) {
-      if( !startBuffering( child ) ) {
-        generator.writeStartArray();
-      }
+    if( bufferingLevel == null && !startBuffering( child ) ) {
+      generator.writeStartArray();
     }
   }
 
@@ -525,12 +521,10 @@ protected String filterStreamValue( Level node ) {
         List<JsonPath.Match> matches = path.evaluate( node.scopeNode );
         if( matches != null && !matches.isEmpty() ) {
           JsonPath.Match match = matches.get( 0 );
-          if( match.getNode().isTextual() ) {
-            if( selector instanceof UrlRewriteFilterApplyDescriptor ) {
-              UrlRewriteFilterApplyDescriptor apply = (UrlRewriteFilterApplyDescriptor)selector;
-              rule = apply.rule();
-              break;
-            }
+          if( match.getNode().isTextual() && selector instanceof UrlRewriteFilterApplyDescriptor ) {
+            UrlRewriteFilterApplyDescriptor apply = (UrlRewriteFilterApplyDescriptor)selector;
+            rule = apply.rule();
+            break;
           }
         }
       }
@@ -580,10 +574,8 @@ private void filterBufferedValues( Level node, List<UrlRewriteFilterPathDescript
       JsonPath.Expression path = (JsonPath.Expression)selector.compiledPath( JPATH_COMPILER );
       List<JsonPath.Match> matches = path.evaluate( node.node );
       for( JsonPath.Match match : matches ) {
-        if( match.getNode().isTextual() ) {
-          if( selector instanceof UrlRewriteFilterApplyDescriptor ) {
-            filterBufferedValue( match, (UrlRewriteFilterApplyDescriptor)selector );
-          }
+        if( match.getNode().isTextual() && selector instanceof UrlRewriteFilterApplyDescriptor ) {
+          filterBufferedValue( match, (UrlRewriteFilterApplyDescriptor)selector );
         }
       }
     }

diff --git a/...rc/main/java/org/apache/knox/gateway/deploy/impl/CompositeAuthzDeploymentContributor.java b/...rc/main/java/org/apache/knox/gateway/deploy/impl/CompositeAuthzDeploymentContributor.java
@@ -42,11 +42,6 @@ public String getName() {
     return "CompositeAuthz";
   }
 
-  @Override
-  public void initializeContribution(DeploymentContext context) {
-    super.initializeContribution(context);
-  }
-
   @Override
   public void contributeProvider( DeploymentContext context, Provider provider ) {
   }
@@ -71,8 +66,7 @@ public void contributeFilter( DeploymentContext context, Provider provider, Serv
   }
 
   String[] parseProviderNames(String providerNames) {
-    String[] names = providerNames.split(",\\s*");
-    return names;
+    return providerNames.split(",\\s*");
   }
 
   void getProviderSpecificParams(ResourceDescriptor resource, List<FilterParamDescriptor> params,

diff --git a/...rovider-security-shiro/src/main/java/org/apache/knox/gateway/deploy/impl/ShiroConfig.java b/...rovider-security-shiro/src/main/java/org/apache/knox/gateway/deploy/impl/ShiroConfig.java
@@ -50,11 +50,7 @@ public ShiroConfig(Provider provider, String clusterName) {
   }
 
   private void addNameValueToSection(String name, String value, String sectionName) {
-    Map<String, String> section = sections.get(sectionName);
-    if (section == null) {
-      section = new LinkedHashMap<>();
-      sections.put(sectionName, section);
-    }
+    Map<String, String> section = sections.computeIfAbsent(sectionName, k -> new LinkedHashMap<>());
     section.put(name, value);
   }
 

diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/GatewayServer.java b/gateway-server/src/main/java/org/apache/knox/gateway/GatewayServer.java
@@ -126,6 +126,8 @@ public class GatewayServer {
   private static final Auditor auditor = AuditServiceFactory.getAuditService().getAuditor(AuditConstants.DEFAULT_AUDITOR_NAME,
       AuditConstants.KNOX_SERVICE_NAME, AuditConstants.KNOX_COMPONENT_NAME);
 
+  private static final String TOPOLOGY_EXTENSION = ".topo.";
+
   static final String KNOXSESSIONCOOKIENAME = "KNOXSESSIONID";
 
   private static GatewayServer server;
@@ -1032,7 +1034,7 @@ private File calculateDeploymentDir( Topology topology ) {
   }
 
   private String calculateDeploymentExtension() {
-    return ".topo.";
+    return TOPOLOGY_EXTENSION;
   }
 
   private String calculateDeploymentName( Topology topology ) {