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 49287679d07..7af94627b0c 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 @@ static ContextClassLoaderFactory getContextFactory() { } // 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 85e03ea1c29..b49acae2481 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 @@ private PropUtil() {} */ public static void setProperties(final ServerContext context, final PropStoreKey propStoreKey, final Map properties) throws IllegalArgumentException { - PropUtil.validateProperties(propStoreKey, properties); + PropUtil.validateProperties(context, propStoreKey, properties); context.getPropStore().putAll(propStoreKey, properties); } @@ -51,12 +51,13 @@ public static void removeProperties(final ServerContext context, public static void replaceProperties(final ServerContext context, final PropStoreKey propStoreKey, final long version, final Map 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 properties) throws IllegalArgumentException { + protected static void validateProperties(final ServerContext context, + final PropStoreKey propStoreKey, final Map properties) + throws IllegalArgumentException { for (Map.Entry prop : properties.entrySet()) { if (!Property.isValidProperty(prop.getKey(), prop.getValue())) { String exceptionMessage = "Invalid property for : "; @@ -66,10 +67,12 @@ protected static void validateProperties(final PropStoreKey propStoreKey, 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 0bb2e8a8e3b..b8589ab21ab 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 static void modifyProperties(ServerContext context, long version, final Map 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 static void removePropWithoutDeprecationWarning(ServerContext context, St 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 @@ private static String validateSystemProperty(SystemPropKey key, String property, 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 00000000000..e16fc75e450 --- /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); + } + +}