Skip to content

Commit

Permalink
Merge branch '3.1'
Browse files Browse the repository at this point in the history
  • Loading branch information
dlmarion committed Jan 8, 2025
2 parents 8f123f2 + 7d1a816 commit 3f438b1
Show file tree
Hide file tree
Showing 6 changed files with 121 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ static ContextClassLoaderFactory getContextFactory() {
}

// for testing
static synchronized void resetContextFactoryForTests() {
public static synchronized void resetContextFactoryForTests() {
FACTORY = null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,6 @@ public void execute(String[] args) throws Exception {
log.info("print properties: {}", opts.printProps);
log.info("print instances: {}", opts.printInstanceIds);

var conf = opts.getSiteConfiguration();

try (ServerContext context = getContext(opts)) {
generateReport(context, opts);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ private 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);
}

Expand All @@ -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<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 : ";
Expand All @@ -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());
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -46,7 +47,7 @@ public static void modifyProperties(ServerContext context, long version,
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);
}
Expand All @@ -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;
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,6 @@ public void instanceIdOutputTest() throws Exception {

String testFileName = "./target/zoo-info-viewer-" + System.currentTimeMillis() + ".txt";

expect(context.getZooReader()).andReturn(zooReader).once();
context.close();

replay(context, zooReader);
Expand Down Expand Up @@ -295,15 +294,13 @@ public void propTest() throws Exception {
expect(zooReader.getData(tBasePath + "/t" + ZTABLE_NAMESPACE))
.andReturn("+default".getBytes(UTF_8)).anyTimes();

replay(context, zooReader);

NamespacePropKey nsKey = NamespacePropKey.of(iid, nsId);
log.trace("namespace base path: {}", nsKey.getPath());

String testFileName = "./target/zoo-info-viewer-" + System.currentTimeMillis() + ".txt";
context.close();

replay(context);
replay(context, zooReader);

class ZooInfoViewerTestClazz extends ZooInfoViewer {
@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}

}

0 comments on commit 3f438b1

Please sign in to comment.