This is an automated email from the ASF dual-hosted git repository.
dlmarion pushed a commit to branch 2.1
in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/2.1 by this push:
new 25dcfbfe14 Fix issue with ZooPropEditor setting classloader context
property (#5238)
25dcfbfe14 is described below
commit 25dcfbfe14d9b71e8c9b2e4b079287bbcd6ae476
Author: Dave Marion <dlmar...@apache.org>
AuthorDate: Wed Jan 8 14:30:54 2025 -0500
Fix issue with ZooPropEditor setting classloader context property (#5238)
The root cause of the issue is that ClassLoaderUtil.initContextFactory
was not being called which caused an NPE to be thrown which returned
false from ClassLoaderUtil.isValidContext.
Closes #5198
---
.../accumulo/core/classloader/ClassLoaderUtil.java | 2 +-
.../org/apache/accumulo/server/util/PropUtil.java | 19 ++--
.../accumulo/server/util/SystemPropUtil.java | 11 ++-
.../apache/accumulo/server/util/PropUtilTest.java | 102 +++++++++++++++++++++
4 files changed, 120 insertions(+), 14 deletions(-)
diff --git
a/core/src/main/java/org/apache/accumulo/core/classloader/ClassLoaderUtil.java
b/core/src/main/java/org/apache/accumulo/core/classloader/ClassLoaderUtil.java
index 49287679d0..7af94627b0 100644
---
a/core/src/main/java/org/apache/accumulo/core/classloader/ClassLoaderUtil.java
+++
b/core/src/main/java/org/apache/accumulo/core/classloader/ClassLoaderUtil.java
@@ -69,7 +69,7 @@ public class ClassLoaderUtil {
}
// for testing
- static synchronized void resetContextFactoryForTests() {
+ public static synchronized void resetContextFactoryForTests() {
FACTORY = null;
}
diff --git
a/server/base/src/main/java/org/apache/accumulo/server/util/PropUtil.java
b/server/base/src/main/java/org/apache/accumulo/server/util/PropUtil.java
index 85e03ea1c2..b49acae248 100644
--- a/server/base/src/main/java/org/apache/accumulo/server/util/PropUtil.java
+++ b/server/base/src/main/java/org/apache/accumulo/server/util/PropUtil.java
@@ -39,7 +39,7 @@ public final class PropUtil {
*/
public static void setProperties(final ServerContext context, final
PropStoreKey<?> propStoreKey,
final Map<String,String> properties) throws IllegalArgumentException {
- PropUtil.validateProperties(propStoreKey, properties);
+ PropUtil.validateProperties(context, propStoreKey, properties);
context.getPropStore().putAll(propStoreKey, properties);
}
@@ -51,12 +51,13 @@ public final class PropUtil {
public static void replaceProperties(final ServerContext context,
final PropStoreKey<?> propStoreKey, final long version, final
Map<String,String> properties)
throws IllegalArgumentException {
- PropUtil.validateProperties(propStoreKey, properties);
+ PropUtil.validateProperties(context, propStoreKey, properties);
context.getPropStore().replaceAll(propStoreKey, version, properties);
}
- protected static void validateProperties(final PropStoreKey<?> propStoreKey,
- final Map<String,String> properties) throws IllegalArgumentException {
+ protected static void validateProperties(final ServerContext context,
+ final PropStoreKey<?> propStoreKey, final Map<String,String> properties)
+ throws IllegalArgumentException {
for (Map.Entry<String,String> prop : properties.entrySet()) {
if (!Property.isValidProperty(prop.getKey(), prop.getValue())) {
String exceptionMessage = "Invalid property for : ";
@@ -66,10 +67,12 @@ public final class PropUtil {
throw new IllegalArgumentException(exceptionMessage + propStoreKey + "
name: "
+ prop.getKey() + ", value: " + prop.getValue());
} else if
(prop.getKey().equals(Property.TABLE_CLASSLOADER_CONTEXT.getKey())
- &&
!Property.TABLE_CLASSLOADER_CONTEXT.getDefaultValue().equals(prop.getValue())
- && !ClassLoaderUtil.isValidContext(prop.getValue())) {
- throw new IllegalArgumentException(
- "Unable to resolve classloader for context: " + prop.getValue());
+ &&
!Property.TABLE_CLASSLOADER_CONTEXT.getDefaultValue().equals(prop.getValue())) {
+ ClassLoaderUtil.initContextFactory(context.getConfiguration());
+ if (!ClassLoaderUtil.isValidContext(prop.getValue())) {
+ throw new IllegalArgumentException(
+ "Unable to resolve classloader for context: " + prop.getValue());
+ }
}
}
}
diff --git
a/server/base/src/main/java/org/apache/accumulo/server/util/SystemPropUtil.java
b/server/base/src/main/java/org/apache/accumulo/server/util/SystemPropUtil.java
index 0bb2e8a8e3..b8589ab21a 100644
---
a/server/base/src/main/java/org/apache/accumulo/server/util/SystemPropUtil.java
+++
b/server/base/src/main/java/org/apache/accumulo/server/util/SystemPropUtil.java
@@ -37,7 +37,8 @@ public class SystemPropUtil {
public static void setSystemProperty(ServerContext context, String property,
String value)
throws IllegalArgumentException {
final SystemPropKey key = SystemPropKey.of(context);
- context.getPropStore().putAll(key, Map.of(validateSystemProperty(key,
property, value), value));
+ context.getPropStore().putAll(key,
+ Map.of(validateSystemProperty(context, key, property, value), value));
}
public static void modifyProperties(ServerContext context, long version,
@@ -46,7 +47,7 @@ public class SystemPropUtil {
final Map<String,
String> checkedProperties = properties.entrySet().stream()
.collect(Collectors.toMap(
- entry -> validateSystemProperty(key, entry.getKey(),
entry.getValue()),
+ entry -> validateSystemProperty(context, key, entry.getKey(),
entry.getValue()),
Map.Entry::getValue));
context.getPropStore().replaceAll(key, version, checkedProperties);
}
@@ -66,8 +67,8 @@ public class SystemPropUtil {
context.getPropStore().removeProperties(SystemPropKey.of(context),
List.of(property));
}
- private static String validateSystemProperty(SystemPropKey key, String
property,
- final String value) throws IllegalArgumentException {
+ private static String validateSystemProperty(ServerContext context,
SystemPropKey key,
+ String property, final String value) throws IllegalArgumentException {
// Retrieve the replacement name for this property, if there is one.
// Do this before we check if the name is a valid zookeeper name.
final var original = property;
@@ -88,7 +89,7 @@ public class SystemPropUtil {
throw iae;
}
if (Property.isValidTablePropertyKey(property)) {
- PropUtil.validateProperties(key, Map.of(property, value));
+ PropUtil.validateProperties(context, key, Map.of(property, value));
}
// Find the property taking prefix into account
diff --git
a/server/base/src/test/java/org/apache/accumulo/server/util/PropUtilTest.java
b/server/base/src/test/java/org/apache/accumulo/server/util/PropUtilTest.java
new file mode 100644
index 0000000000..e16fc75e45
--- /dev/null
+++
b/server/base/src/test/java/org/apache/accumulo/server/util/PropUtilTest.java
@@ -0,0 +1,102 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.server.util;
+
+import static org.easymock.EasyMock.createMock;
+import static org.easymock.EasyMock.expect;
+import static org.easymock.EasyMock.replay;
+import static org.easymock.EasyMock.verify;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import java.util.Map;
+
+import org.apache.accumulo.core.classloader.ClassLoaderUtil;
+import org.apache.accumulo.core.conf.AccumuloConfiguration;
+import org.apache.accumulo.core.conf.Property;
+import org.apache.accumulo.core.data.InstanceId;
+import org.apache.accumulo.core.data.TableId;
+import org.apache.accumulo.core.spi.common.ContextClassLoaderFactory;
+import org.apache.accumulo.server.ServerContext;
+import org.apache.accumulo.server.conf.store.TablePropKey;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+public class PropUtilTest {
+
+ private static final String TEST_CONTEXT_NAME = "TestContext";
+ private static final String INVALID_TEST_CONTEXT_NAME = "InvalidTestContext";
+
+ public static class TestContextClassLoaderFactory implements
ContextClassLoaderFactory {
+
+ @Override
+ public ClassLoader getClassLoader(String contextName) {
+ if (contextName.equals(TEST_CONTEXT_NAME)) {
+ return this.getClass().getClassLoader();
+ }
+ return null;
+ }
+
+ }
+
+ @BeforeEach
+ public void before() {
+ ClassLoaderUtil.resetContextFactoryForTests();
+ }
+
+ @Test
+ public void testSetClasspathContextFails() {
+ ServerContext ctx = createMock(ServerContext.class);
+ AccumuloConfiguration conf = createMock(AccumuloConfiguration.class);
+ InstanceId iid = createMock(InstanceId.class);
+ TableId tid = createMock(TableId.class);
+ TablePropKey tkey = TablePropKey.of(iid, tid);
+
+ expect(ctx.getConfiguration()).andReturn(conf).once();
+ expect(conf.get(Property.GENERAL_CONTEXT_CLASSLOADER_FACTORY))
+ .andReturn(TestContextClassLoaderFactory.class.getName());
+
+ replay(ctx, conf, iid, tid);
+ IllegalArgumentException e =
+ assertThrows(IllegalArgumentException.class, () ->
PropUtil.validateProperties(ctx, tkey,
+ Map.of(Property.TABLE_CLASSLOADER_CONTEXT.getKey(),
INVALID_TEST_CONTEXT_NAME)));
+ assertEquals("Unable to resolve classloader for context: " +
INVALID_TEST_CONTEXT_NAME,
+ e.getMessage());
+ verify(ctx, conf, iid, tid);
+ }
+
+ @Test
+ public void testSetClasspathContext() {
+ ServerContext ctx = createMock(ServerContext.class);
+ AccumuloConfiguration conf = createMock(AccumuloConfiguration.class);
+ InstanceId iid = createMock(InstanceId.class);
+ TableId tid = createMock(TableId.class);
+ TablePropKey tkey = TablePropKey.of(iid, tid);
+
+ expect(ctx.getConfiguration()).andReturn(conf).once();
+ expect(conf.get(Property.GENERAL_CONTEXT_CLASSLOADER_FACTORY))
+ .andReturn(TestContextClassLoaderFactory.class.getName());
+
+ replay(ctx, conf, iid, tid);
+ PropUtil.validateProperties(ctx, tkey,
+ Map.of(Property.TABLE_CLASSLOADER_CONTEXT.getKey(),
TEST_CONTEXT_NAME));
+ verify(ctx, conf, iid, tid);
+ }
+
+}
