Skip to content

Use ServiceLoader for ObjectFactory #1463

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
339 changes: 339 additions & 0 deletions core/src/test/java/cucumber/runtime/model/CucumberFeatureTest.java

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
io.cucumber.guice.impl.GuiceFactory
52 changes: 31 additions & 21 deletions java/src/main/java/io/cucumber/java/ObjectFactoryLoader.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,45 +2,54 @@

import cucumber.api.java.ObjectFactory;
import io.cucumber.core.io.ClassFinder;
import io.cucumber.core.exception.CucumberException;
import io.cucumber.core.reflection.NoInstancesException;
import io.cucumber.core.reflection.Reflections;
import io.cucumber.core.reflection.TooManyInstancesException;

import static java.util.Arrays.asList;
import java.util.Iterator;
import java.util.ServiceLoader;

public class ObjectFactoryLoader {

private static final ServiceLoader<ObjectFactory> LOADER = ServiceLoader.load(ObjectFactory.class);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The service loader is not safe for use by multiple concurrent threads. Runners are executed in parallel and each runner will create it's own backends with object factory. You can either make all fields and methods non-static or move this field into the method.


private ObjectFactoryLoader() {
}

/**
* Loads an instance of {@link ObjectFactory}. The class name can be explicit, or it can be null.
* When it's null, the implementation is searched for in the <pre>cucumber.runtime</pre> packahe.
* When it's null, the implementation is searched for in the <pre>cucumber.runtime</pre> package.
*
* @param classFinder where to load classes from
* @param objectFactoryClassName specific class name of {@link ObjectFactory} implementation. May be null.
* @return an instance of {@link ObjectFactory}
*/
// TODO remove once confirmed classFinder and/or objectFactoryClassName are not need for serviceLoader
public static ObjectFactory loadObjectFactory(ClassFinder classFinder, String objectFactoryClassName) {
return loadObjectFactory();
}

/**
* Loads an instance of {@link ObjectFactory} using the {@link ServiceLoader}.
* The class name can be explicit, or it can be null.
* When it's null, the implementation is searched for in the <pre>cucumber.runtime</pre> package.
*
* @return an instance of {@link ObjectFactory}
*/
public static ObjectFactory loadObjectFactory() {

final Iterator<ObjectFactory> objectFactories = LOADER.iterator();

ObjectFactory objectFactory;
try {
Reflections reflections = new Reflections(classFinder);

if(objectFactoryClassName != null) {
Class<ObjectFactory> objectFactoryClass = (Class<ObjectFactory>) classFinder.loadClass(objectFactoryClassName);
objectFactory = reflections.newInstance(new Class[0], new Object[0], objectFactoryClass);
} else {
objectFactory = reflections.instantiateExactlyOneSubclass(ObjectFactory.class, asList("io.cucumber"), new Class[0], new Object[0], null);
}
} catch (TooManyInstancesException e) {
System.out.println(e.getMessage());
System.out.println(getMultipleObjectFactoryLogMessage());
if (objectFactories.hasNext()) {
objectFactory = objectFactories.next();

} else {
objectFactory = new DefaultJavaObjectFactory();
} catch (NoInstancesException e) {
}

if (objectFactories.hasNext()) {
System.out.println(getMultipleObjectFactoryLogMessage());
objectFactory = new DefaultJavaObjectFactory();
} catch (ClassNotFoundException e) {
throw new CucumberException("Couldn't instantiate custom ObjectFactory", e);
}

return objectFactory;
}

Expand All @@ -53,4 +62,5 @@ private static String getMultipleObjectFactoryLogMessage() {
sb.append("In order to enjoy IoC features, please remove the unnecessary dependencies from your classpath.\n");
return sb.toString();
}

}
4 changes: 3 additions & 1 deletion java/src/test/java/io/cucumber/java/JavaSnippetTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,7 @@ public String transform(String... strings) {

@Test
@Ignore
// TODO issue tracked to within io.cucumber.cucumberexpressions.CucumberExpressionGenerator
public void recognisesWordWithNumbers() {
String expected = "" +
"@Given(\"Then it responds ([\\\"]*)\")\n" +
Expand Down Expand Up @@ -304,7 +305,8 @@ public void generateSnippetWithOutlineParam() {

private String snippetFor(String name) {
PickleStep step = new PickleStep(name, Collections.<Argument>emptyList(), Collections.<PickleLocation>emptyList());
List<String> snippet = new SnippetGenerator(new JavaSnippet(), new ParameterTypeRegistry(Locale.ENGLISH)).getSnippet(step, GIVEN_KEYWORD, functionNameGenerator);
List<String> snippet = new SnippetGenerator(new JavaSnippet(), new ParameterTypeRegistry(Locale.ENGLISH))
.getSnippet(step, GIVEN_KEYWORD, functionNameGenerator);
return StringJoiner.join("\n", snippet);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
io.cucumber.needle.NeedleFactory
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
io.cucumber.openejb.OpenEJBObjectFactory
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
io.cucumber.picocontainer.PicoFactory
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
io.cucumber.spring.SpringFactory
13 changes: 13 additions & 0 deletions weld/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -37,5 +37,18 @@
<artifactId>junit</artifactId>
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.hamcrest</groupId>
<artifactId>hamcrest-core</artifactId>
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
<scope>test</scope>
</dependency>

</dependencies>
</project>
54 changes: 34 additions & 20 deletions weld/src/main/java/io/cucumber/weld/WeldFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,26 +5,42 @@
import org.jboss.weld.environment.se.Weld;
import org.jboss.weld.environment.se.WeldContainer;

public class WeldFactory implements ObjectFactory {
public class WeldFactory
implements ObjectFactory {

protected static final String START_EXCEPTION_MESSAGE = "" +
"\n" +
"It looks like you're running on a single-core machine, and Weld doesn't like that. See:\n" +
"* http://in.relation.to/Bloggers/Weld200Alpha2Released\n" +
"* https://issues.jboss.org/browse/WELD-1119\n" +
"\n" +
"The workaround is to add enabled=false to a org.jboss.weld.executor.properties file on\n" +
"your CLASSPATH. Beware that this will trigger another Weld bug - startup will now work,\n" +
"but shutdown will fail with a NullPointerException. This exception will be printed and\n" +
"not rethrown. It's the best Cucumber-JVM can do until this bug is fixed in Weld.\n" +
"\n";

protected static final String STOP_EXCEPTION_MESSAGE = "" +
"\nIf you have set enabled=false in org.jboss.weld.executor.properties and you are seeing\n" +
"this message, it means your weld container didn't shut down properly. It's a Weld bug\n" +
"and we can't do much to fix it in Cucumber-JVM.\n" +
"";

private WeldContainer containerInstance;

@Override
public void start() {
start(null);
}

protected void start(Weld weld) {
try {
containerInstance = new Weld().initialize();
if (weld == null) {
weld = new Weld();
}
containerInstance = weld.initialize();
} catch (IllegalArgumentException e) {
throw new CucumberException("" +
"\n" +
"It looks like you're running on a single-core machine, and Weld doesn't like that. See:\n" +
"* http://in.relation.to/Bloggers/Weld200Alpha2Released\n" +
"* https://issues.jboss.org/browse/WELD-1119\n" +
"\n" +
"The workaround is to add enabled=false to a org.jboss.weld.executor.properties file on\n" +
"your CLASSPATH. Beware that this will trigger another Weld bug - startup will now work,\n" +
"but shutdown will fail with a NullPointerException. This exception will be printed and\n" +
"not rethrown. It's the best Cucumber-JVM can do until this bug is fixed in Weld.\n" +
"\n", e);
throw new CucumberException(START_EXCEPTION_MESSAGE, e);
}
}

Expand All @@ -35,12 +51,8 @@ public void stop() {
containerInstance.close();
}
} catch (NullPointerException npe) {
System.err.println("" +
"\nIf you have set enabled=false in org.jboss.weld.executor.properties and you are seeing\n" +
"this message, it means your weld container didn't shut down properly. It's a Weld bug\n" +
"and we can't do much to fix it in Cucumber-JVM.\n" +
"");
npe.printStackTrace();
System.err.println(STOP_EXCEPTION_MESSAGE);
npe.printStackTrace(System.err);
}
}

Expand All @@ -51,6 +63,8 @@ public boolean addClass(Class<?> clazz) {

@Override
public <T> T getInstance(Class<T> type) {
return containerInstance.select(type).get();
return containerInstance.select(type)
.get();
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
io.cucumber.weld.WeldFactory
78 changes: 75 additions & 3 deletions weld/src/test/java/io/cucumber/weld/WeldFactoryTest.java
Original file line number Diff line number Diff line change
@@ -1,29 +1,101 @@
package io.cucumber.weld;

import cucumber.api.java.ObjectFactory;
import cucumber.api.junit.Cucumber;
import cucumber.runtime.CucumberException;
import org.jboss.weld.environment.se.Weld;
import org.junit.After;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;

import java.io.ByteArrayOutputStream;
import java.io.PrintStream;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.core.Is.is;
import static org.hamcrest.core.IsEqual.equalTo;
import static org.hamcrest.core.StringStartsWith.startsWith;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNotSame;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

public class WeldFactoryTest {

@Rule
public ExpectedException expectedException = ExpectedException.none();

private static final PrintStream ORIGINAL_OUT = System.out;
private static final PrintStream ORIGINAL_ERR = System.err;

private final ByteArrayOutputStream outContent = new ByteArrayOutputStream();
private final ByteArrayOutputStream errContent = new ByteArrayOutputStream();

@Before
public void setUpStreams() {
System.setOut(new PrintStream(outContent));
System.setErr(new PrintStream(errContent));
}

@After
public void restoreStreams() {
System.setOut(ORIGINAL_OUT);
System.setErr(ORIGINAL_ERR);
}

@Test
public void shouldGiveUsNewInstancesForEachScenario() {
ObjectFactory factory = new WeldFactory();

final ObjectFactory factory = new WeldFactory();
factory.addClass(BellyStepdefs.class);

// Scenario 1
factory.start();
BellyStepdefs o1 = factory.getInstance(BellyStepdefs.class);
final BellyStepdefs o1 = factory.getInstance(BellyStepdefs.class);
factory.stop();

// Scenario 2
factory.start();
BellyStepdefs o2 = factory.getInstance(BellyStepdefs.class);
final BellyStepdefs o2 = factory.getInstance(BellyStepdefs.class);
factory.stop();

assertNotNull(o1);
assertNotSame(o1, o2);
}

@Test
public void startstopCalledWithoutStart() {

final Weld weld = mock(Weld.class);
when(weld.initialize())
.thenThrow(new IllegalArgumentException());

final WeldFactory factory = new WeldFactory();

this.expectedException.expect(CucumberException.class);
this.expectedException.expectMessage(is(equalTo(WeldFactory.START_EXCEPTION_MESSAGE)));

factory.start(weld);
}

@Test
public void stopCalledWithoutStart() {

final ObjectFactory factory = new WeldFactory();

factory.stop();

final String expectedErrOutput = "\n" +
"If you have set enabled=false in org.jboss.weld.executor.properties and you are seeing\n" +
"this message, it means your weld container didn't shut down properly. It's a Weld bug\n" +
"and we can't do much to fix it in Cucumber-JVM.\n" +
"\n" +
"java.lang.NullPointerException\n" +
"\tat cucumber.runtime.java.weld.WeldFactory.stop";

assertThat(this.errContent.toString(), is(startsWith(expectedErrOutput)));
}

}