tiagomlalves commented on code in PR #3931:
URL: https://github.com/apache/cassandra/pull/3931#discussion_r1983146979
##########
src/java/org/apache/cassandra/config/ParameterizedClass.java:
##########
@@ -74,32 +77,53 @@ static public <K> K newInstance(ParameterizedClass
parameterizedClass, List<Stri
if (!searchPackage.isEmpty() && !searchPackage.endsWith("."))
searchPackage = searchPackage + '.';
String name = searchPackage + parameterizedClass.class_name;
- Class<?> providerClass = Class.forName(name);
- try
- {
- Constructor<?> constructor =
providerClass.getConstructor(Map.class);
- K instance = (K)
constructor.newInstance(parameterizedClass.parameters);
- return instance;
- }
- catch (Exception constructorEx)
- {
- //no-op
- }
- // fallback to no arg constructor if no params present
- if (parameterizedClass.parameters == null ||
parameterizedClass.parameters.isEmpty())
- {
- Constructor<?> constructor =
providerClass.getConstructor();
- K instance = (K) constructor.newInstance();
- return instance;
- }
+ providerClass = Class.forName(name);
}
- // there are about 5 checked exceptions that could be thrown here.
- catch (Exception e)
+ catch (ClassNotFoundException e)
{
- last = e;
+ //no-op
}
}
- throw new ConfigurationException("Unable to create parameterized class
" + parameterizedClass.class_name, last);
+
+ if (providerClass == null)
+ {
+ String error = "Unable to find class " +
parameterizedClass.class_name + " in packages [" +
+ searchPackages.stream().map(p -> '"' + p +
'"').collect(Collectors.joining(",")) + ']';
+ throw new ConfigurationException(error);
+ }
+
+ try
+ {
+ Constructor<?>[] declaredConstructors =
providerClass.getDeclaredConstructors();
+
+ Constructor mapConstructor = Arrays.stream(declaredConstructors)
+ .filter(c -> c.getParameterTypes().length == 1 &&
c.getParameterTypes()[0].equals(Map.class))
Review Comment:
Currently we support:
* a constructor with no arguments, most common case (e.g.
`AllowAllAuthenticator`) which is handled first
* a constructor that receives a `Map` as argument, required for
implementations that receive parameters.
Original logic did the same, but relied on trying to instantiate the class
and handling the exception while my code explicitly checks for the constructor.
There are a few corner cases that were not handled in the original code (nor
in my code). For instance, we expect a `Map<String, String>` but this is not
validated (but is caught as exception).
If we add a new auth class, we need to respect any of the two supported
constructors.
Does this address your concern?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org
For additional commands, e-mail: pr-h...@cassandra.apache.org
