This is an automated email from the ASF dual-hosted git repository.
Cole-Greer pushed a commit to branch 3.7-dev
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
The following commit(s) were added to refs/heads/3.7-dev by this push:
new 51a7f29c78 CTR: Verify io() class names before loading and invoking
them
51a7f29c78 is described below
commit 51a7f29c7899f74757ae47bf4c09ddfb60755a0a
Author: Cole Greer <[email protected]>
AuthorDate: Mon Aug 17 15:25:31 2026 -0700
CTR: Verify io() class names before loading and invoking them
A class name supplied to IO.reader, IO.writer or IO.registry was passed to
Class.forName() and had build() or instance() invoked before anything
checked
that it was a GraphReader, GraphWriter or IoRegistry. Naming any class on
the
server classpath therefore ran its static initializer and any static no-arg
factory method it declared. Such names are now loaded without being
initialized, checked, and only then invoked. IoRegistryHelper is hardened on
the same terms, since it also falls back to a no-arg constructor.
io().write() now resolves its GraphWriter before opening the output file,
which
previously truncated the target when the writer could not be constructed.
Assisted-by: Kiro:claude-opus-5
---
CHANGELOG.asciidoc | 2 +
.../process/traversal/step/sideEffect/IoStep.java | 86 ++++++---
.../structure/io/util/IoRegistryHelper.java | 20 +-
.../traversal/step/sideEffect/IoStepTest.java | 202 +++++++++++++++++++++
.../structure/io/util/IoRegistryHelperTest.java | 146 +++++++++++++++
5 files changed, 425 insertions(+), 31 deletions(-)
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index 7303ca3939..1505c29d7f 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -27,6 +27,8 @@
image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
* Fixed GraphBinary deserialization to reject invalid length and count values
with an `IOException`.
* Disabled unsafe Java deserialization on the Gryo IO paths and added
`GryoMapper.Builder.javaSerializationAllowed(boolean)` to control it.
+* Added verification to `io()` ensuring a class named by `IO.reader`,
`IO.writer` or `IO.registry` implements the expected interface before loading
and initializing it.
+* Fixed `io().write()` to resolve its `GraphWriter` before opening the output
file, so that a writer which cannot be constructed no longer truncates the
target.
* Fixed `subgraph()` to throw a descriptive error identifying the required
`Edge` input instead of an internal `ClassCastException` when the traversal
produces a non-edge value.
* Fixed `where(P)` to throw a descriptive error identifying the required
String scope key (and suggesting `is(P)` for value comparisons) instead of an
internal `ClassCastException` when given a non-String predicate value.
* Fixed `PeerPressure.property_name` in `gremlin-python` incorrectly mapping
to the `pageRank` property name token.
diff --git
a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/IoStep.java
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/IoStep.java
index 839dd5eb0c..ba2aebadd6 100644
---
a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/IoStep.java
+++
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/IoStep.java
@@ -47,7 +47,7 @@ import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.lang.reflect.Method;
-import java.util.Collections;
+import java.lang.reflect.Modifier;
import java.util.List;
import java.util.stream.Collectors;
@@ -116,9 +116,13 @@ public class IoStep<S> extends AbstractStep<S,S>
implements ReadWriting {
}
protected Traverser.Admin<S> write(final File file) {
+ // the writer is resolved before the stream is opened, since opening
it creates or truncates the file. a writer
+ // that cannot be constructed would otherwise leave the destination
destroyed on the way to the error
+ final GraphWriter writer = constructWriter();
+
try (final OutputStream stream = new FileOutputStream(file)) {
final Graph graph = (Graph) this.traversal.getGraph().get();
- constructWriter().writeGraph(stream, graph);
+ writer.writeGraph(stream, graph);
return EmptyTraverser.instance();
} catch (IOException ioe) {
@@ -127,9 +131,11 @@ public class IoStep<S> extends AbstractStep<S,S>
implements ReadWriting {
}
protected Traverser.Admin<S> read(final File file) {
+ final GraphReader reader = constructReader();
+
try (final InputStream stream = new FileInputStream(file)) {
final Graph graph = (Graph) this.traversal.getGraph().get();
- constructReader().readGraph(stream, graph);
+ reader.readGraph(stream, graph);
return EmptyTraverser.instance();
} catch (IOException ioe) {
@@ -158,14 +164,8 @@ public class IoStep<S> extends AbstractStep<S,S>
implements ReadWriting {
} else if (objectOrClass.equals(IO.graphml))
return GraphMLReader.build().create();
else {
- try {
- final Class<?> graphReaderClazz = Class.forName((String)
objectOrClass);
- final Method build = graphReaderClazz.getMethod("build");
- final GraphReader.ReaderBuilder builder =
(GraphReader.ReaderBuilder) build.invoke(null);
- return builder.create();
- } catch (Exception ex) {
- throw new IllegalStateException(String.format("Could not
construct the specified GraphReader of %s", objectOrClass), ex);
- }
+ return invokeIoFactory((String) objectOrClass,
GraphReader.class,
+ "build", GraphReader.ReaderBuilder.class).create();
}
} else {
throw new IllegalStateException("GraphReader could not be
determined");
@@ -193,14 +193,8 @@ public class IoStep<S> extends AbstractStep<S,S>
implements ReadWriting {
} else if (objectOrClass.equals(IO.graphml))
return GraphMLWriter.build().create();
else {
- try {
- final Class<?> graphWriterClazz = Class.forName((String)
objectOrClass);
- final Method build = graphWriterClazz.getMethod("build");
- final GraphWriter.WriterBuilder builder =
(GraphWriter.WriterBuilder) build.invoke(null);
- return builder.create();
- } catch (Exception ex) {
- throw new IllegalStateException(String.format("Could not
construct the specified GraphWriter of %s", objectOrClass), ex);
- }
+ return invokeIoFactory((String) objectOrClass,
GraphWriter.class,
+ "build", GraphWriter.WriterBuilder.class).create();
}
} else {
throw new IllegalStateException("GraphWriter could not be
determined");
@@ -221,19 +215,53 @@ public class IoStep<S> extends AbstractStep<S,S>
implements ReadWriting {
protected List<IoRegistry> detectRegistries() {
final List<Object> k = parameters.get(IO.registry, null);
return k.stream().map(cn -> {
- try {
- if (cn instanceof IoRegistry)
- return (IoRegistry) cn;
- else {
- final Class<?> clazz = Class.forName(cn.toString());
- return (IoRegistry)
clazz.getMethod("instance").invoke(null);
- }
- } catch (Exception ex) {
- throw new IllegalStateException(ex);
- }
+ if (cn instanceof IoRegistry)
+ return (IoRegistry) cn;
+ else
+ return invokeIoFactory(cn.toString(), IoRegistry.class,
"instance", IoRegistry.class);
}).collect(Collectors.toList());
}
+ /**
+ * Resolves a class name supplied to {@link IO#reader}, {@link IO#writer}
or {@link IO#registry} to the object its
+ * static factory method returns. The name comes from user input in the
traversal, so the class is loaded without
+ * being initialized and the return types are validated before the factory
is invoked.
+ */
+ private static <R> R invokeIoFactory(final String className, final
Class<?> ioType,
+ final String factoryMethodName, final
Class<R> returnType) {
+ // one message for every rejection, so that a traversal cannot use the
error to tell which classes the server
+ // has on its classpath
+ final String cannotConstruct = String.format("Could not construct the
specified %s of %s",
+ ioType.getSimpleName(), className);
+
+ final Class<?> clazz;
+ try {
+ // initialize is false to avoid running initialization blocks
prior to class verification.
+ clazz = Class.forName(className, false,
IoStep.class.getClassLoader());
+ } catch (ClassNotFoundException | LinkageError ex) {
+ throw new IllegalStateException(cannotConstruct, ex);
+ }
+
+ if (!ioType.isAssignableFrom(clazz))
+ throw new IllegalStateException(cannotConstruct);
+
+ final Method factory;
+ try {
+ factory = clazz.getMethod(factoryMethodName);
+ } catch (NoSuchMethodException ex) {
+ throw new IllegalStateException(cannotConstruct, ex);
+ }
+
+ if (!Modifier.isStatic(factory.getModifiers()) ||
!returnType.isAssignableFrom(factory.getReturnType()))
+ throw new IllegalStateException(cannotConstruct);
+
+ try {
+ return returnType.cast(factory.invoke(null));
+ } catch (Exception | ExceptionInInitializerError ex) {
+ throw new IllegalStateException(cannotConstruct, ex);
+ }
+ }
+
@Override
public int hashCode() {
final int hash = super.hashCode() ^ this.parameters.hashCode();
diff --git
a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/util/IoRegistryHelper.java
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/util/IoRegistryHelper.java
index 75501a27b2..72c8eccd87 100644
---
a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/util/IoRegistryHelper.java
+++
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/util/IoRegistryHelper.java
@@ -23,6 +23,7 @@ import org.apache.commons.configuration2.Configuration;
import org.apache.tinkerpop.gremlin.structure.io.IoRegistry;
import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
@@ -46,7 +47,19 @@ public final class IoRegistryHelper {
registries.add((IoRegistry) object);
else if (object instanceof String || object instanceof Class) {
try {
- final Class<?> clazz = object instanceof String ?
Class.forName((String) object) : (Class) object;
+ // a String naming the class is loaded without being
initialized, since this value can originate
+ // from a traversal (io() writes its with() options into
the graph configuration on the OLAP path)
+ // and the name must be proven to be an IoRegistry before
anything the class declares runs. a Class
+ // was resolved by whoever supplied it, so there is
nothing left to defer there
+ final Class<?> clazz = object instanceof String
+ ? Class.forName((String) object, false,
IoRegistryHelper.class.getClassLoader())
+ : (Class) object;
+
+ // checked ahead of the instance() lookup and the
constructor below, so that neither a static
+ // method nor a constructor on some other class is invoked
for its side effects
+ if (!IoRegistry.class.isAssignableFrom(clazz))
+ throw new IllegalStateException("The provided registry
object can not be resolved to an instance: " + object);
+
Method instanceMethod = null;
try {
instanceMethod = clazz.getDeclaredMethod("instance");
// try for getInstance() ??
@@ -60,10 +73,13 @@ public final class IoRegistryHelper {
// no instance() or getInstance() methods
}
}
- if (null != instanceMethod &&
IoRegistry.class.isAssignableFrom(instanceMethod.getReturnType()))
+ if (null != instanceMethod &&
Modifier.isStatic(instanceMethod.getModifiers())
+ &&
IoRegistry.class.isAssignableFrom(instanceMethod.getReturnType()))
registries.add((IoRegistry)
instanceMethod.invoke(null));
else
registries.add((IoRegistry) clazz.newInstance()); //
no instance() or getInstance() methods, try instantiate class
+ } catch (final IllegalStateException ise) {
+ throw ise;
} catch (final Exception e) {
throw new IllegalStateException(e.getMessage(), e);
}
diff --git
a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/IoStepTest.java
b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/IoStepTest.java
index b4987c1726..5d71333ec5 100644
---
a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/IoStepTest.java
+++
b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/IoStepTest.java
@@ -18,26 +18,34 @@
*/
package org.apache.tinkerpop.gremlin.process.traversal.step.sideEffect;
+import org.apache.tinkerpop.gremlin.process.traversal.IO;
import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__;
import
org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.OptionsStrategy;
+import org.apache.tinkerpop.gremlin.structure.io.AbstractIoRegistry;
import org.apache.tinkerpop.gremlin.structure.io.GraphReader;
import org.apache.tinkerpop.gremlin.structure.io.GraphWriter;
+import org.apache.tinkerpop.gremlin.structure.io.IoRegistry;
import org.apache.tinkerpop.gremlin.structure.io.gryo.GryoMapper;
import org.apache.tinkerpop.gremlin.structure.io.gryo.GryoReader;
+import org.apache.tinkerpop.gremlin.structure.io.gryo.GryoWriter;
import org.apache.tinkerpop.shaded.kryo.io.Output;
import org.junit.Test;
import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
+import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
import java.io.OutputStream;
import java.io.Serializable;
+import java.nio.file.Files;
+import java.util.List;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.core.IsInstanceOf.instanceOf;
+import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
@@ -134,6 +142,147 @@ public class IoStepTest {
}
}
+ /**
+ * {@code io()} takes a class name for its reader, writer and registry,
and the traversal that supplies it may have
+ * arrived as a remote request, so a name that is not the type the
parameter asks for has to be refused before the
+ * class it names is initialized. The canary classes below record any of
their code running in {@link IoCanary},
+ * which is a separate class so that reading the flag does not initialize
the canary being watched.
+ */
+ @Test
+ public void shouldNotInitializeANamedReaderThatIsNotAGraphReader() {
+ IoCanary.FIRED = false;
+
+ final IoStep<?> step = new IoStep<>(__.start().asAdmin(),
"graph.kryo");
+ // a class literal does not initialize the class it names, so naming
the canary this way does not fire it
+ step.configure(IO.reader, NotAnIoType.class.getName());
+
+ try {
+ step.constructReader();
+ fail("a class that is not a GraphReader must not be accepted as
one");
+ } catch (IllegalStateException expected) {
+ // the name is refused, and the assertion below is what makes the
refusal meaningful
+ }
+
+ assertFalse("a class named by IO.reader must not be initialized before
it is known to be a GraphReader",
+ IoCanary.FIRED);
+ }
+
+ @Test
+ public void shouldNotInitializeANamedWriterThatIsNotAGraphWriter() {
+ IoCanary.FIRED = false;
+
+ final IoStep<?> step = new IoStep<>(__.start().asAdmin(),
"graph.kryo");
+ step.configure(IO.writer, NotAnIoType.class.getName());
+
+ try {
+ step.constructWriter();
+ fail("a class that is not a GraphWriter must not be accepted as
one");
+ } catch (IllegalStateException expected) {
+ // as above
+ }
+
+ assertFalse("a class named by IO.writer must not be initialized before
it is known to be a GraphWriter",
+ IoCanary.FIRED);
+ }
+
+ @Test
+ public void shouldNotInitializeANamedRegistryThatIsNotAnIoRegistry() {
+ IoCanary.FIRED = false;
+
+ final IoStep<?> step = new IoStep<>(__.start().asAdmin(),
"graph.kryo");
+ step.configure(IO.registry, NotAnIoType.class.getName());
+
+ try {
+ step.detectRegistries();
+ fail("a class that is not an IoRegistry must not be accepted as
one");
+ } catch (IllegalStateException expected) {
+ // as above
+ }
+
+ assertFalse("a class named by IO.registry must not be initialized
before it is known to be an IoRegistry",
+ IoCanary.FIRED);
+ }
+
+ /**
+ * The named class is an {@link IoRegistry} here, so it passes the type
check and the factory method is what has to
+ * be rejected. {@code instance()} is called with a {@code null} receiver,
so a non-static one was never going to
+ * work, but checking the signature rather than discovering it through the
call is what keeps the class from being
+ * initialized on the way to the error.
+ */
+ @Test
+ public void
shouldNotInitializeANamedRegistryWhoseInstanceMethodIsNotStatic() {
+ IoCanary.FIRED = false;
+
+ final IoStep<?> step = new IoStep<>(__.start().asAdmin(),
"graph.kryo");
+ step.configure(IO.registry, NonStaticInstanceRegistry.class.getName());
+
+ try {
+ step.detectRegistries();
+ fail("a non-static instance() must not be accepted as the factory
for an IoRegistry");
+ } catch (IllegalStateException expected) {
+ // as above
+ }
+
+ assertFalse("a class whose instance() is not static must not be
initialized on the way to that error",
+ IoCanary.FIRED);
+ }
+
+ /**
+ * The documented form of these parameters, which GLVs have no alternative
to since they cannot send an instance
+ * over the wire. The hardening above narrows what a name may resolve to
and must not withdraw the feature.
+ */
+ @Test
+ public void shouldConstructAGraphReaderNamedByClassName() {
+ final IoStep<?> step = new IoStep<>(__.start().asAdmin(),
"graph.kryo");
+ step.configure(IO.reader, GryoReader.class.getName());
+
+ assertThat(step.constructReader(), instanceOf(GryoReader.class));
+ }
+
+ @Test
+ public void shouldConstructAGraphWriterNamedByClassName() {
+ final IoStep<?> step = new IoStep<>(__.start().asAdmin(),
"graph.kryo");
+ step.configure(IO.writer, GryoWriter.class.getName());
+
+ assertThat(step.constructWriter(), instanceOf(GryoWriter.class));
+ }
+
+ @Test
+ public void shouldConstructAnIoRegistryNamedByClassName() {
+ final IoStep<?> step = new IoStep<>(__.start().asAdmin(),
"graph.kryo");
+ step.configure(IO.registry, StaticInstanceRegistry.class.getName());
+
+ final List<IoRegistry> registries = step.detectRegistries();
+ assertEquals(1, registries.size());
+ assertThat(registries.get(0),
instanceOf(StaticInstanceRegistry.class));
+ }
+
+ /**
+ * Opening the output stream creates or truncates the destination, so it
must not be opened until the writer is in
+ * hand. Otherwise naming a writer that cannot be constructed destroys the
contents of whatever file the traversal
+ * pointed at, and the traversal chooses that path.
+ */
+ @Test
+ public void
shouldNotTruncateTheTargetFileWhenTheWriterCannotBeConstructed() throws
Exception {
+ final File target = File.createTempFile("io-step-target", ".kryo");
+ target.deleteOnExit();
+ final byte[] contents = "do not truncate me".getBytes();
+ Files.write(target.toPath(), contents);
+
+ final IoStep<?> step = new IoStep<>(__.start().asAdmin(),
target.getAbsolutePath());
+ step.configure(IO.writer, NotAnIoType.class.getName());
+
+ try {
+ step.write(target);
+ fail("a writer that cannot be constructed must fail the write");
+ } catch (IllegalStateException expected) {
+ // the writer is resolved first, so the failure arrives before the
file is opened
+ }
+
+ assertArrayEquals("a writer that cannot be constructed must leave the
target file untouched",
+ contents, Files.readAllBytes(target.toPath()));
+ }
+
/**
* A Gryo stream that presents {@code OptionsStrategy}'s type id and then
a raw Java-serialized payload. Crafting
* it needs no cooperation from the Gryo writer, which is why the sink was
reachable from untrusted bytes.
@@ -166,4 +315,57 @@ public class IoStepTest {
FIRED = true;
}
}
+
+ /**
+ * Holds the flag the canary classes below set. It is deliberately not a
field on those classes, since reading a
+ * static field initializes the class that declares it, which is the very
thing the tests assert did not happen.
+ */
+ public static class IoCanary {
+ static volatile boolean FIRED = false;
+ }
+
+ /**
+ * Not a {@link GraphReader}, a {@link GraphWriter} or an {@link
IoRegistry}, and it reports every route by which
+ * {@code io()} might run its code: its static initializer, and the two
factory method names the step looks for.
+ * The bodies are inert, since firing the flag is all that has to be
observable.
+ */
+ public static class NotAnIoType {
+ static {
+ IoCanary.FIRED = true;
+ }
+
+ public static Object build() {
+ IoCanary.FIRED = true;
+ return null;
+ }
+
+ public static Object instance() {
+ IoCanary.FIRED = true;
+ return null;
+ }
+ }
+
+ /**
+ * An {@link IoRegistry}, so it clears the type check, whose {@code
instance()} is not static.
+ */
+ public static class NonStaticInstanceRegistry extends AbstractIoRegistry {
+ static {
+ IoCanary.FIRED = true;
+ }
+
+ public IoRegistry instance() {
+ return this;
+ }
+ }
+
+ /**
+ * A well-formed registry, used to hold the documented class-name form
open.
+ */
+ public static class StaticInstanceRegistry extends AbstractIoRegistry {
+ private static final StaticInstanceRegistry INSTANCE = new
StaticInstanceRegistry();
+
+ public static StaticInstanceRegistry instance() {
+ return INSTANCE;
+ }
+ }
}
diff --git
a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/structure/io/util/IoRegistryHelperTest.java
b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/structure/io/util/IoRegistryHelperTest.java
new file mode 100644
index 0000000000..8fd7265632
--- /dev/null
+++
b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/structure/io/util/IoRegistryHelperTest.java
@@ -0,0 +1,146 @@
+/*
+ * 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
+ *
+ * http://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.tinkerpop.gremlin.structure.io.util;
+
+import org.apache.commons.configuration2.BaseConfiguration;
+import org.apache.commons.configuration2.Configuration;
+import org.apache.tinkerpop.gremlin.structure.io.AbstractIoRegistry;
+import org.apache.tinkerpop.gremlin.structure.io.IoRegistry;
+import org.junit.Test;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.IsInstanceOf.instanceOf;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.fail;
+
+/**
+ * A registry name reaches this helper from the graph configuration, and on
the OLAP path {@code io()} copies its
+ * {@code with()} options into that configuration, so a name here is not
necessarily operator-supplied. A name must
+ * therefore be proven to be an {@link IoRegistry} before the class it names
runs anything: this helper both invokes a
+ * static factory method and, failing that, calls a no-arg constructor.
+ */
+public class IoRegistryHelperTest {
+
+ @Test
+ public void shouldNotInitializeANamedClassThatIsNotAnIoRegistry() {
+ RegistryCanary.FIRED = false;
+
+ try {
+ // a class literal does not initialize the class it names, so
naming the canary this way does not fire it
+
IoRegistryHelper.createRegistries(Collections.singletonList(NotARegistry.class.getName()));
+ fail("a class that is not an IoRegistry must not be accepted as
one");
+ } catch (IllegalStateException expected) {
+ // the name is refused, and the assertion below is what makes the
refusal meaningful
+ }
+
+ assertFalse("a class named as a registry must not be initialized,
constructed, or have a method invoked on " +
+ "it before it is known to be an IoRegistry",
RegistryCanary.FIRED);
+ }
+
+ @Test
+ public void
shouldNotInitializeANamedClassThatIsNotAnIoRegistryFromConfiguration() {
+ RegistryCanary.FIRED = false;
+
+ final Configuration conf = new BaseConfiguration();
+ conf.setProperty(IoRegistry.IO_REGISTRY, NotARegistry.class.getName());
+
+ try {
+ IoRegistryHelper.createRegistries(conf);
+ fail("a class that is not an IoRegistry must not be accepted as
one");
+ } catch (IllegalStateException expected) {
+ // as above
+ }
+
+ assertFalse("the configuration form must refuse the name on the same
terms", RegistryCanary.FIRED);
+ }
+
+ /**
+ * The forms the helper is documented to accept have to keep working: an
instance, a {@link Class}, a class name
+ * with a static {@code instance()}, and a class name with only a no-arg
constructor.
+ */
+ @Test
+ public void shouldCreateRegistriesFromTheAcceptedForms() {
+ final List<IoRegistry> registries =
IoRegistryHelper.createRegistries(Arrays.asList(
+ new ConstructorOnlyRegistry(),
+ ConstructorOnlyRegistry.class,
+ StaticInstanceRegistry.class.getName(),
+ ConstructorOnlyRegistry.class.getName()));
+
+ assertEquals(4, registries.size());
+ assertThat(registries.get(0),
instanceOf(ConstructorOnlyRegistry.class));
+ assertThat(registries.get(1),
instanceOf(ConstructorOnlyRegistry.class));
+ assertThat(registries.get(2),
instanceOf(StaticInstanceRegistry.class));
+ assertThat(registries.get(3),
instanceOf(ConstructorOnlyRegistry.class));
+ }
+
+ @Test
+ public void shouldReturnEmptyForAConfigurationWithoutARegistry() {
+ assertEquals(Collections.emptyList(),
IoRegistryHelper.createRegistries(new BaseConfiguration()));
+ }
+
+ /**
+ * Holds the flag {@link NotARegistry} sets. Kept out of that class so
that reading the flag does not initialize
+ * the class the test asserts was never initialized.
+ */
+ public static class RegistryCanary {
+ static volatile boolean FIRED = false;
+ }
+
+ /**
+ * Not an {@link IoRegistry}. Its static initializer, its constructor and
both factory method names the helper
+ * looks for all report, since the helper can reach a named class through
any of them.
+ */
+ public static class NotARegistry {
+ static {
+ RegistryCanary.FIRED = true;
+ }
+
+ public NotARegistry() {
+ RegistryCanary.FIRED = true;
+ }
+
+ public static Object instance() {
+ RegistryCanary.FIRED = true;
+ return null;
+ }
+
+ public static Object getInstance() {
+ RegistryCanary.FIRED = true;
+ return null;
+ }
+ }
+
+ public static class StaticInstanceRegistry extends AbstractIoRegistry {
+ private static final StaticInstanceRegistry INSTANCE = new
StaticInstanceRegistry();
+
+ public static StaticInstanceRegistry instance() {
+ return INSTANCE;
+ }
+ }
+
+ public static class ConstructorOnlyRegistry extends AbstractIoRegistry {
+ public ConstructorOnlyRegistry() {
+ }
+ }
+}
