Skip to content

Commit e2f79c8

Browse files
tiagomlalvestiagomlalves
committed
HCD-122 Backports CASSANDRA-20368 and CASSANDRA-20450
Work done in HCD-82 was further refined in CASSANDRA-20368 and CASSANDRA-20450. This backports C* changes in order to minimize the diff in future backports.
1 parent 362af8d commit e2f79c8

File tree

5 files changed

+72
-67
lines changed

5 files changed

+72
-67
lines changed

‎CHANGES.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ Future version (tbd)
33
Merged from 5.1:
44
* Expose current compaction throughput in nodetool (CASSANDRA-13890)
55
Merged from 5.0:
6+
* Improve error messages when initializing auth classes (CASSANDRA-20368 and CASSANDRA-20450)
67
* Use ParameterizedClass for all auth-related implementations (CASSANDRA-19946 and partially CASSANDRA-18554)
78
* Enables IAuthenticator's to return own AuthenticateMessage (CASSANDRA-19984)
89
* Disable chronicle analytics (CASSANDRA-19656)

‎src/java/org/apache/cassandra/auth/AuthConfig.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ public static void applyAuth()
8585
IRoleManager roleManager = authInstantiate(conf.role_manager, CassandraRoleManager.class);
8686

8787
if (authenticator instanceof PasswordAuthenticator && !(roleManager instanceof CassandraRoleManager))
88-
throw new ConfigurationException(authenticator.getClass().getName() + " requires CassandraRoleManager", false);
88+
throw new ConfigurationException(authenticator.getClass().getName() + " requires " + CassandraRoleManager.class.getName(), false);
8989

9090
DatabaseDescriptor.setRoleManager(roleManager);
9191

@@ -122,6 +122,8 @@ private static <T> T authInstantiate(ParameterizedClass authCls, Class<T> defaul
122122
return ParameterizedClass.newInstance(authCls, List.of("", authPackage));
123123
}
124124

125+
// for now, this has to stay and can not be replaced by ParameterizedClass.newInstance as above
126+
// due to that failing for simulator dtests. See CASSANDRA-20450 for more information.
125127
try
126128
{
127129
return defaultCls.newInstance();

‎src/java/org/apache/cassandra/config/ParameterizedClass.java

Lines changed: 24 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -19,16 +19,15 @@
1919

2020
import java.lang.reflect.Constructor;
2121
import java.lang.reflect.InvocationTargetException;
22-
import java.util.Arrays;
2322
import java.util.Collections;
2423
import java.util.List;
2524
import java.util.Map;
25+
import java.util.function.Predicate;
2626
import java.util.stream.Collectors;
2727

2828
import com.google.common.base.Objects;
2929

3030
import org.apache.cassandra.exceptions.ConfigurationException;
31-
import org.apache.cassandra.utils.IntegerInterval;
3231

3332
public class ParameterizedClass
3433
{
@@ -45,8 +44,7 @@ public ParameterizedClass()
4544

4645
public ParameterizedClass(String class_name)
4746
{
48-
this.class_name = class_name;
49-
this.parameters = Collections.emptyMap();
47+
this(class_name, Collections.emptyMap());
5048
}
5149

5250
public ParameterizedClass(String class_name, Map<String, String> parameters)
@@ -84,45 +82,48 @@ static public <K> K newInstance(ParameterizedClass parameterizedClass, List<Stri
8482

8583
if (providerClass == null)
8684
{
87-
String error = "Unable to find class " + parameterizedClass.class_name + " in packages [" +
88-
searchPackages.stream().map(p -> '"' + p + '"').collect(Collectors.joining(",")) + ']';
85+
String pkgList = '[' + searchPackages.stream().map(p -> '"' + p + '"').collect(Collectors.joining(",")) + ']';
86+
String error = "Unable to find class " + parameterizedClass.class_name + " in packages " + pkgList;
8987
throw new ConfigurationException(error);
9088
}
9189

9290
try
9391
{
94-
Constructor<?>[] declaredConstructors = providerClass.getDeclaredConstructors();
95-
96-
Constructor mapConstructor = Arrays.stream(declaredConstructors)
97-
.filter(c -> c.getParameterTypes().length == 1 && c.getParameterTypes()[0].equals(Map.class))
98-
.findFirst().orElse(null);
92+
Constructor<?> mapConstructor = filterConstructor(providerClass, c -> c.getParameterTypes().length == 1 && c.getParameterTypes()[0].equals(Map.class));
9993
if (mapConstructor != null)
100-
return (K) mapConstructor.newInstance(parameterizedClass.parameters);
94+
return (K) mapConstructor.newInstance(parameterizedClass.parameters == null ? Collections.emptyMap() : parameterizedClass.parameters);
10195

102-
// Falls-back to no-arg constructor if no parameters are present
103-
if (parameterizedClass.parameters == null || parameterizedClass.parameters.isEmpty())
104-
{
105-
Constructor emptyConstructor = Arrays.stream(declaredConstructors)
106-
.filter(c -> c.getParameterTypes().length == 0)
107-
.findFirst().orElse(null);
108-
if (emptyConstructor != null)
109-
return (K) emptyConstructor.newInstance();
110-
}
96+
// Falls-back to no-arg constructor
97+
Constructor<?> noArgsConstructor = filterConstructor(providerClass, c -> c.getParameterTypes().length == 0);
98+
if (noArgsConstructor != null)
99+
return (K) noArgsConstructor.newInstance();
111100

112101
throw new ConfigurationException("No valid constructor found for class " + parameterizedClass.class_name);
113102
}
114-
catch (IllegalAccessException|InstantiationException|ExceptionInInitializerError e)
103+
catch (IllegalAccessException | InstantiationException | ExceptionInInitializerError e)
115104
{
116105
throw new ConfigurationException("Unable to instantiate parameterized class " + parameterizedClass.class_name, e);
117106
}
118107
catch (InvocationTargetException e)
119108
{
120109
Throwable cause = e.getCause();
121-
String error = "Failed to instantiate class " + parameterizedClass.class_name + ": " + cause.getMessage();
110+
String error = "Failed to instantiate class " + parameterizedClass.class_name +
111+
(cause.getMessage() != null ? ": " + cause.getMessage() : "");
122112
throw new ConfigurationException(error, cause);
123113
}
124114
}
125115

116+
private static Constructor<?> filterConstructor(Class<?> providerClass, Predicate<Constructor<?>> filter)
117+
{
118+
for (Constructor<?> constructor : providerClass.getDeclaredConstructors())
119+
{
120+
if (filter.test(constructor))
121+
return constructor;
122+
}
123+
124+
return null;
125+
}
126+
126127
@Override
127128
public boolean equals(Object that)
128129
{

‎test/unit/org/apache/cassandra/config/ParameterizedClassExample.java

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,19 +24,15 @@
2424

2525
public class ParameterizedClassExample
2626
{
27-
boolean calledMapConstructor = false;
28-
2927
public ParameterizedClassExample()
3028
{
3129
Assert.fail("This constructor should not be called");
3230
}
3331

3432
public ParameterizedClassExample(Map<String, String> parameters)
3533
{
36-
calledMapConstructor = true;
37-
3834
if (parameters == null)
39-
return;
35+
throw new IllegalArgumentException("Parameters must not be null");
4036

4137
boolean simulateFailure = Boolean.parseBoolean(parameters.getOrDefault("fail", "false"));
4238
if (simulateFailure)

‎test/unit/org/apache/cassandra/config/ParameterizedClassTest.java

Lines changed: 43 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -23,74 +23,79 @@
2323

2424
import org.junit.Test;
2525

26-
import static org.hamcrest.CoreMatchers.containsString;
27-
import static org.hamcrest.CoreMatchers.startsWith;
28-
import static org.junit.Assert.assertEquals;
29-
import static org.junit.Assert.assertNotNull;
30-
import static org.junit.Assert.assertThrows;
31-
import static org.hamcrest.MatcherAssert.assertThat;
32-
import static org.junit.Assert.assertTrue;
33-
3426
import org.apache.cassandra.auth.AllowAllAuthorizer;
3527
import org.apache.cassandra.auth.IAuthorizer;
3628
import org.apache.cassandra.exceptions.ConfigurationException;
3729

30+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
31+
import static org.junit.Assert.assertNotNull;
32+
import static org.junit.Assert.assertNull;
33+
3834
public class ParameterizedClassTest
3935
{
4036
@Test
41-
public void newInstance_NonExistentClass_FailsWithConfigurationException()
37+
public void testParameterizedClassEmptyConstructorHasNullParameters()
38+
{
39+
ParameterizedClass parameterizedClass = new ParameterizedClass();
40+
assertNull(parameterizedClass.parameters);
41+
}
42+
43+
@Test
44+
public void testParameterizedClassConstructorWithClassNameHasNonNullParameters()
4245
{
43-
ParameterizedClass nonExistentClass = new ParameterizedClass("NonExistentClass");
46+
ParameterizedClass parameterizedClass = new ParameterizedClass("TestClass");
47+
assertNotNull(parameterizedClass.parameters);
48+
}
4449

45-
ConfigurationException exception = assertThrows(ConfigurationException.class, () -> {
46-
ParameterizedClass.newInstance(nonExistentClass, List.of("org.apache.cassandra.config"));
47-
});
50+
@Test
51+
public void testParameterizedClassConstructorWithClassNameAndParametersHasNullParamters()
52+
{
53+
ParameterizedClass parameterizedClass = new ParameterizedClass("TestClass", null);
54+
assertNull(parameterizedClass.parameters);
55+
}
4856

49-
String expectedError = "Unable to find class NonExistentClass in packages [\"org.apache.cassandra.config\"]";
50-
assertEquals(expectedError, exception.getMessage());
57+
@Test
58+
public void testNewInstanceWithNonExistentClassFailsWithConfigurationException()
59+
{
60+
assertThatThrownBy(() -> ParameterizedClass.newInstance(new ParameterizedClass("NonExistentClass"),
61+
List.of("org.apache.cassandra.config")))
62+
.hasMessage("Unable to find class NonExistentClass in packages [\"org.apache.cassandra.config\"]")
63+
.isInstanceOf(ConfigurationException.class);
5164
}
5265

5366
@Test
54-
public void newInstance_WithSingleEmptyConstructor_UsesEmptyConstructor()
67+
public void testNewInstanceWithSingleEmptyConstructorUsesEmptyConstructor()
5568
{
5669
ParameterizedClass parameterizedClass = new ParameterizedClass(AllowAllAuthorizer.class.getName());
5770
IAuthorizer instance = ParameterizedClass.newInstance(parameterizedClass, null);
5871
assertNotNull(instance);
5972
}
6073

6174
@Test
62-
public void newInstance_SingleEmptyConstructorWithParameters_FailsWithConfigurationException()
75+
public void testNewInstanceWithValidConstructorsFavorsMapConstructor()
6376
{
64-
Map<String, String> parameters = Map.of("key", "value");
65-
ParameterizedClass parameterizedClass = new ParameterizedClass(AllowAllAuthorizer.class.getName(), parameters);
66-
67-
ConfigurationException exception = assertThrows(ConfigurationException.class, () -> {
68-
ParameterizedClass.newInstance(parameterizedClass, null);
69-
});
70-
71-
assertThat(exception.getMessage(), startsWith("No valid constructor found for class"));
77+
ParameterizedClass parameterizedClass = new ParameterizedClass(ParameterizedClassExample.class.getName());
78+
ParameterizedClassExample instance = ParameterizedClass.newInstance(parameterizedClass, null);
79+
assertNotNull(instance);
7280
}
7381

7482
@Test
75-
public void newInstance_WithValidConstructors_FavorsMapConstructor()
83+
public void testNewInstanceWithValidConstructorsUsingNullParamtersFavorsMapConstructor()
7684
{
7785
ParameterizedClass parameterizedClass = new ParameterizedClass(ParameterizedClassExample.class.getName());
78-
ParameterizedClassExample instance = ParameterizedClass.newInstance(parameterizedClass, null);
86+
parameterizedClass.parameters = null;
7987

80-
assertTrue(instance.calledMapConstructor);
88+
ParameterizedClassExample instance = ParameterizedClass.newInstance(parameterizedClass, null);
89+
assertNotNull(instance);
8190
}
8291

8392
@Test
84-
public void newInstance_WithConstructorException_PreservesOriginalFailure()
93+
public void testNewInstanceWithConstructorExceptionPreservesOriginalFailure()
8594
{
86-
Map <String, String> parameters = Map.of("fail", "true");
87-
ParameterizedClass parameterizedClass = new ParameterizedClass(ParameterizedClassExample.class.getName(), parameters);
88-
89-
ConfigurationException exception = assertThrows(ConfigurationException.class, () -> {
90-
ParameterizedClass.newInstance(parameterizedClass, null);
91-
});
92-
93-
assertThat(exception.getMessage(), startsWith("Failed to instantiate class"));
94-
assertThat(exception.getMessage(), containsString("Simulated failure"));
95+
assertThatThrownBy(() -> ParameterizedClass.newInstance(new ParameterizedClass(ParameterizedClassExample.class.getName(),
96+
Map.of("fail", "true")), null))
97+
.hasMessageStartingWith("Failed to instantiate class")
98+
.hasMessageContaining("Simulated failure")
99+
.isInstanceOf(ConfigurationException.class);
95100
}
96101
}

0 commit comments

Comments
 (0)