jmeter-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From fschumac...@apache.org
Subject [jmeter] branch master updated: Minor refactorings to make sonar happy
Date Thu, 29 Oct 2020 17:48:37 GMT
This is an automated email from the ASF dual-hosted git repository.

fschumacher pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/jmeter.git


The following commit(s) were added to refs/heads/master by this push:
     new 45ba5df  Minor refactorings to make sonar happy
45ba5df is described below

commit 45ba5df1428b296c8f8ad2ee323849fdf04c9c4a
Author: Felix Schumacher <felix.schumacher@internetallee.de>
AuthorDate: Thu Oct 29 18:42:36 2020 +0100

    Minor refactorings to make sonar happy
    
    * Use a private constructor to make it clear, that the loader is a utility class
    * Use generics when possible (an exception might be thrown, but will be catched as before)
    * Extract inner try block to a private method to reduce complexity
    * Use Class#getDeclaredConstructor() to get the default constructor, instead of using
      the deprecated (as of Java 9) Class#newInstance() directly
    * Add two tests (an empty "classname" and a valid classname, that is not a Provider)
    * Add serialVersionUIDs to the dummy classes
---
 .../apache/jmeter/util/SecurityProviderLoader.java | 35 +++++++++++++++-------
 .../jmeter/util/SecurityProviderLoaderTest.java    | 14 +++++----
 2 files changed, 33 insertions(+), 16 deletions(-)

diff --git a/src/core/src/main/java/org/apache/jmeter/util/SecurityProviderLoader.java b/src/core/src/main/java/org/apache/jmeter/util/SecurityProviderLoader.java
index 656ee35..db817ff 100644
--- a/src/core/src/main/java/org/apache/jmeter/util/SecurityProviderLoader.java
+++ b/src/core/src/main/java/org/apache/jmeter/util/SecurityProviderLoader.java
@@ -18,6 +18,7 @@
 package org.apache.jmeter.util;
 
 import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
 import java.security.Provider;
 import java.security.Security;
 import java.util.Comparator;
@@ -33,6 +34,10 @@ public class SecurityProviderLoader {
     private static final Logger log = LoggerFactory.getLogger(SecurityProviderLoader.class);
     private static final Pattern CONFIGURATION_REGEX = Pattern.compile("^(?<classname>[^:]+)(:(?<position>\\d+)(:(?<config>.+))?)?$");
 
+    private SecurityProviderLoader() {
+        throw new IllegalStateException("Utility class");
+    }
+
     public static void addSecurityProvider(Properties properties) {
         properties.keySet().stream()
                 .filter(key -> key.toString().matches("security\\.provider(\\.\\d+)?"))
@@ -44,29 +49,26 @@ public class SecurityProviderLoader {
 
         if (matcher.matches()) {
             final String classname = matcher.group("classname");
-            final Integer position = Integer.parseInt(StringUtils.defaultString(matcher.group("position"),
"0"));
+            final int position = Integer.parseInt(StringUtils.defaultString(matcher.group("position"),
"0"));
             final String config = matcher.group("config");
 
             try {
-                Class providerClass = Class.forName(classname);
+                @SuppressWarnings("unchecked")
+                Class<Provider> providerClass = (Class<Provider>) Class.forName(classname);
 
                 Provider provider = null;
 
                 if (config != null) {
-                    try {
-                        Constructor constructor = providerClass.getConstructor(String.class);
-                        provider = (Provider) constructor.newInstance(config);
-                    } catch (NoSuchMethodException e) {
-                        log.warn("Security Provider {} has no constructor with a single String
argument - try to use default constructor.", providerClass);
-                    }
+                    provider = tryConstructorWithString(providerClass, config);
                 }
 
                 if (provider == null) {
-                    provider = (Provider) providerClass.newInstance();
+                    provider = providerClass.getDeclaredConstructor().newInstance();
                 }
                 int installedPosition = Security.insertProviderAt(provider, position);
 
-                log.info("Security Provider {} ({}) is installed at position {}", provider.getClass().getSimpleName(),
provider.getName(), installedPosition);
+                log.info("Security Provider {} ({}) is installed at position {}", provider.getClass().getSimpleName(),
+                        provider.getName(), Integer.valueOf(installedPosition));
             } catch (Exception exception) {
                 String message = String.format("Security Provider '%s' could not be installed.",
classname);
                 log.error(message, exception);
@@ -75,4 +77,17 @@ public class SecurityProviderLoader {
             }
         }
     }
+
+    private static Provider tryConstructorWithString(Class<Provider> providerClass,
final String config)
+            throws InstantiationException, IllegalAccessException, InvocationTargetException
{
+        try {
+            Constructor<Provider> constructor = providerClass.getConstructor(String.class);
+            return constructor.newInstance(config);
+        } catch (NoSuchMethodException e) {
+            log.warn(
+                    "Security Provider {} has no constructor with a single String argument
- try to use default constructor.",
+                    providerClass);
+        }
+        return null;
+    }
 }
diff --git a/src/core/src/test/java/org/apache/jmeter/util/SecurityProviderLoaderTest.java
b/src/core/src/test/java/org/apache/jmeter/util/SecurityProviderLoaderTest.java
index 8b9f110..2571c0f 100644
--- a/src/core/src/test/java/org/apache/jmeter/util/SecurityProviderLoaderTest.java
+++ b/src/core/src/test/java/org/apache/jmeter/util/SecurityProviderLoaderTest.java
@@ -76,19 +76,19 @@ public class SecurityProviderLoaderTest {
         Assert.assertEquals(provider, providers_after[providers_after.length - 1]);
     }
 
-    @Test
-    public void addUnknownSecurityProviderTest() {
+    @ParameterizedTest
+    @ValueSource(strings = {"", "java.lang.Object", "org.apache.jmeter.util.SecurityProviderLoaderTest.UnknownProvider"})
+    public void addInvalidProviderClassTest(String invalidClassname) {
         removeAllDummyProviders();
         int providersCountBefore = Security.getProviders().length;
 
-        SecurityProviderLoader.addSecurityProvider("org.apache.jmeter.util.SecurityProviderLoaderTest.UnknownProvider");
+        SecurityProviderLoader.addSecurityProvider(invalidClassname);
 
-        Provider[] providers_after = Security.getProviders();
+        int providersCountAfter = Security.getProviders().length;
 
-        Assert.assertEquals(providersCountBefore, providers_after.length);
+        Assert.assertEquals(providersCountBefore, providersCountAfter);
     }
 
-
     @ParameterizedTest
     @ValueSource(ints = {0, 1, 2, 3})
     public void addSecurityProviderWithPositionTest(int position) {
@@ -154,6 +154,7 @@ public class SecurityProviderLoaderTest {
     }
 
     public static class DummyProvider extends Provider {
+        private static final long serialVersionUID = 1L;
         public static final String PROVIDER_NAME = "DUMMY";
 
         public DummyProvider() {
@@ -163,6 +164,7 @@ public class SecurityProviderLoaderTest {
     }
 
     public static class DummyProviderWithConfig extends Provider {
+        private static final long serialVersionUID = 1L;
         public static final String PROVIDER_NAME = "DUMMY_CONFIG";
 
         private String config = null;


Mime
View raw message