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 <[email protected]>
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;

Reply via email to