Skip to content

Commit 8fdb224

Browse files
committed
[Spring] Require an active scenario before creating beans
The glue code context does not require an active scenario. As a result it was possible to create scenario scoped beans that would be shared (leak) between scenarios. An example of this is illustrated below. Because this does require access to the `GlueCodeScope` it is not possible to do this in practice. ```java @configuration public static class TestApplicationConfiguration { @bean public BeanFactoryPostProcessor beanFactoryPostProcessor(){ return factory -> factory.registerScope(SCOPE_CUCUMBER_GLUE, new GlueCodeScope()); } @bean public ExampleService service(ScenarioScopedApi api) { return new ExampleService(api); } @bean @scope(value = SCOPE_CUCUMBER_GLUE) public ScenarioScopedApi api() { return new ScenarioScopedApi(); } } ``` However without registering the the `GlueCodeScope` at start up Spring will complain about missing the cucumber-glue scope rather then complain about scope not having started or the missing proxy mode. So to avoid further confusion: 1. GlueCodeContext will check if the context has been started. 2. Add a `@ScenarioScope` annotation that sets the correct proxy mode. 3. Do some renaming of internal classes 4. Use a global counter for conversation ids Related - #1970 - #1667
1 parent b40c4f4 commit 8fdb224

File tree

10 files changed

+235
-90
lines changed

10 files changed

+235
-90
lines changed

Diff for: spring/README.md

+5-6
Original file line numberDiff line numberDiff line change
@@ -85,19 +85,18 @@ The `cucumber-glue` scope starts prior to a scenario and ends after a scenario.
8585
All beans in this scope will be created before a scenario execution and
8686
disposed at the end of it.
8787

88-
By using the `CucumberTestContext.SCOPE_CUCUMBER_GLUE` additional components
89-
can be added to the glue scope. These components can be used to safely share
90-
state between scenarios.
88+
By using the `@ScenarioScope` annotation additional components can be added to
89+
the glue scope. These components can be used to safely share state between
90+
scenarios.
9191

9292
```java
9393
package com.example.app;
9494

9595
import org.springframework.stereotype.Component;
96-
import org.springframework.context.annotation.Scope;
97-
import static io.cucumber.spring.CucumberTestContext;
96+
import io.cucumber.spring.ScenarioScope;
9897

9998
@Component
100-
@Scope(CucumberTestContext.SCOPE_CUCUMBER_GLUE)
99+
@ScenarioScope
101100
public class TestUserInformation {
102101

103102
private User testUser;

Diff for: spring/src/main/java/io/cucumber/spring/GlueCodeScope.java renamed to spring/src/main/java/io/cucumber/spring/CucumberScenarioScope.java

+5-5
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,11 @@
33
import org.springframework.beans.factory.ObjectFactory;
44
import org.springframework.beans.factory.config.Scope;
55

6-
class GlueCodeScope implements Scope {
6+
class CucumberScenarioScope implements Scope {
77

88
@Override
99
public Object get(String name, ObjectFactory<?> objectFactory) {
10-
GlueCodeContext context = GlueCodeContext.getInstance();
10+
CucumberTestContext context = CucumberTestContext.getInstance();
1111
Object obj = context.get(name);
1212
if (obj == null) {
1313
obj = objectFactory.getObject();
@@ -19,13 +19,13 @@ public Object get(String name, ObjectFactory<?> objectFactory) {
1919

2020
@Override
2121
public Object remove(String name) {
22-
GlueCodeContext context = GlueCodeContext.getInstance();
22+
CucumberTestContext context = CucumberTestContext.getInstance();
2323
return context.remove(name);
2424
}
2525

2626
@Override
2727
public void registerDestructionCallback(String name, Runnable callback) {
28-
GlueCodeContext context = GlueCodeContext.getInstance();
28+
CucumberTestContext context = CucumberTestContext.getInstance();
2929
context.registerDestructionCallback(name, callback);
3030
}
3131

@@ -36,7 +36,7 @@ public Object resolveContextualObject(String key) {
3636

3737
@Override
3838
public String getConversationId() {
39-
GlueCodeContext context = GlueCodeContext.getInstance();
39+
CucumberTestContext context = CucumberTestContext.getInstance();
4040
return context.getId();
4141
}
4242

Diff for: spring/src/main/java/io/cucumber/spring/CucumberTestContext.java

+61
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,73 @@
22

33
import org.apiguardian.api.API;
44

5+
import java.util.HashMap;
6+
import java.util.Map;
7+
import java.util.concurrent.atomic.AtomicInteger;
8+
59
@API(status = API.Status.STABLE)
610
public final class CucumberTestContext {
711

812
public static final String SCOPE_CUCUMBER_GLUE = "cucumber-glue";
913

14+
private static final ThreadLocal<CucumberTestContext> localContext = ThreadLocal
15+
.withInitial(CucumberTestContext::new);
16+
private static final AtomicInteger sessionCounter = new AtomicInteger(0);
17+
18+
private final Map<String, Object> objects = new HashMap<>();
19+
private final Map<String, Runnable> callbacks = new HashMap<>();
20+
21+
private Integer sessionId;
22+
1023
private CucumberTestContext() {
1124
}
1225

26+
static CucumberTestContext getInstance() {
27+
return localContext.get();
28+
}
29+
30+
void start() {
31+
sessionId = sessionCounter.incrementAndGet();
32+
}
33+
34+
String getId() {
35+
return "cucumber_test_context_" + sessionId;
36+
}
37+
38+
void stop() {
39+
for (Runnable callback : callbacks.values()) {
40+
callback.run();
41+
}
42+
localContext.remove();
43+
sessionId = null;
44+
}
45+
46+
Object get(String name) {
47+
requireActiveScenario();
48+
return objects.get(name);
49+
}
50+
51+
void put(String name, Object object) {
52+
requireActiveScenario();
53+
objects.put(name, object);
54+
}
55+
56+
Object remove(String name) {
57+
requireActiveScenario();
58+
callbacks.remove(name);
59+
return objects.remove(name);
60+
}
61+
62+
void registerDestructionCallback(String name, Runnable callback) {
63+
requireActiveScenario();
64+
callbacks.put(name, callback);
65+
}
66+
67+
void requireActiveScenario() {
68+
if (sessionId == null) {
69+
throw new IllegalStateException(
70+
"Scenario scoped beans can only be created while Cucumber is executing a scenario");
71+
}
72+
}
73+
1374
}

Diff for: spring/src/main/java/io/cucumber/spring/GlueCodeContext.java

-58
This file was deleted.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
package io.cucumber.spring;
2+
3+
import org.apiguardian.api.API;
4+
import org.springframework.context.annotation.Scope;
5+
import org.springframework.context.annotation.ScopedProxyMode;
6+
import org.springframework.core.annotation.AliasFor;
7+
8+
import java.lang.annotation.Documented;
9+
import java.lang.annotation.ElementType;
10+
import java.lang.annotation.Retention;
11+
import java.lang.annotation.RetentionPolicy;
12+
import java.lang.annotation.Target;
13+
14+
/**
15+
* Marks a bean as scoped to the execution of a cucumber scenario.
16+
*/
17+
@Target({ ElementType.TYPE, ElementType.METHOD })
18+
@Retention(RetentionPolicy.RUNTIME)
19+
@Documented
20+
@Scope(CucumberTestContext.SCOPE_CUCUMBER_GLUE)
21+
@API(status = API.Status.STABLE)
22+
public @interface ScenarioScope {
23+
@AliasFor(
24+
annotation = Scope.class)
25+
ScopedProxyMode proxyMode() default ScopedProxyMode.TARGET_CLASS;
26+
27+
}

Diff for: spring/src/main/java/io/cucumber/spring/TestContextAdaptor.java

+3-3
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,12 @@ public final void start() {
5252
notifyContextManagerAboutTestClassStarted();
5353
registerStepClassBeanDefinitions(applicationContext.getBeanFactory());
5454
}
55-
GlueCodeContext.getInstance().start();
55+
CucumberTestContext.getInstance().start();
5656
}
5757

5858
final void registerGlueCodeScope(ConfigurableApplicationContext context) {
5959
while (context != null) {
60-
context.getBeanFactory().registerScope(SCOPE_CUCUMBER_GLUE, new GlueCodeScope());
60+
context.getBeanFactory().registerScope(SCOPE_CUCUMBER_GLUE, new CucumberScenarioScope());
6161
context = (ConfigurableApplicationContext) context.getParent();
6262
}
6363
}
@@ -91,7 +91,7 @@ private void registerStepClassBeanDefinition(BeanDefinitionRegistry registry, Cl
9191
}
9292

9393
public final void stop() {
94-
GlueCodeContext.getInstance().stop();
94+
CucumberTestContext.getInstance().stop();
9595
try {
9696
delegate.afterTestClass();
9797
} catch (Exception e) {
+88
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
package io.cucumber.spring;
2+
3+
import io.cucumber.core.backend.ObjectFactory;
4+
import org.junit.jupiter.api.Test;
5+
import org.springframework.beans.factory.annotation.Autowired;
6+
import org.springframework.beans.factory.config.BeanFactoryPostProcessor;
7+
import org.springframework.context.annotation.Bean;
8+
import org.springframework.context.annotation.Configuration;
9+
import org.springframework.test.context.ContextConfiguration;
10+
11+
import java.util.concurrent.atomic.AtomicInteger;
12+
13+
import static io.cucumber.spring.CucumberTestContext.SCOPE_CUCUMBER_GLUE;
14+
import static org.junit.Assert.assertNotEquals;
15+
16+
class Issue1970 {
17+
18+
@Test
19+
public void issue1970() {
20+
ObjectFactory factory = new SpringFactory();
21+
factory.addClass(GlueClass.class); // Add glue with Spring configuration
22+
factory.start();
23+
GlueClass instance = factory.getInstance(GlueClass.class);
24+
String response = instance.service.get();
25+
factory.stop();
26+
factory.start();
27+
GlueClass instance2 = factory.getInstance(GlueClass.class);
28+
String response2 = instance2.service.get();
29+
factory.stop();
30+
31+
assertNotEquals(response, response2);
32+
}
33+
34+
@CucumberContextConfiguration
35+
@ContextConfiguration(classes = TestApplicationConfiguration.class)
36+
public static class GlueClass {
37+
38+
@Autowired
39+
ExampleService service;
40+
41+
}
42+
43+
@Configuration
44+
public static class TestApplicationConfiguration {
45+
46+
@Bean
47+
public BeanFactoryPostProcessor beanFactoryPostProcessor() {
48+
return factory -> factory.registerScope(SCOPE_CUCUMBER_GLUE, new CucumberScenarioScope());
49+
}
50+
51+
@Bean
52+
public ExampleService service(ScenarioScopedApi api) {
53+
return new ExampleService(api);
54+
}
55+
56+
@Bean
57+
@ScenarioScope
58+
public ScenarioScopedApi api() {
59+
return new ScenarioScopedApi();
60+
}
61+
62+
}
63+
64+
public static class ExampleService {
65+
66+
final ScenarioScopedApi api;
67+
68+
public ExampleService(ScenarioScopedApi api) {
69+
this.api = api;
70+
}
71+
72+
String get() {
73+
return "Api response: " + api.get();
74+
}
75+
}
76+
77+
public static class ScenarioScopedApi {
78+
79+
private static final AtomicInteger globalCounter = new AtomicInteger(0);
80+
private final int instanceId = globalCounter.getAndIncrement();
81+
82+
public String get() {
83+
return "instance " + instanceId;
84+
}
85+
86+
}
87+
88+
}

Diff for: spring/src/test/java/io/cucumber/spring/SpringFactoryTest.java

+25-15
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import io.cucumber.spring.metaconfig.general.BellyMetaStepDefinitions;
1919
import org.junit.jupiter.api.Test;
2020
import org.junit.jupiter.api.function.Executable;
21+
import org.springframework.beans.factory.BeanCreationException;
2122
import org.springframework.beans.factory.annotation.Autowired;
2223
import org.springframework.beans.factory.annotation.Value;
2324
import org.springframework.test.context.ContextConfiguration;
@@ -262,26 +263,35 @@ void shouldGlueScopedSpringBeanBehaveLikeGlueLifecycle() {
262263

263264
// Scenario 1
264265
factory.start();
265-
final Belly belly1 = factory.getInstance(Belly.class);
266-
final GlueScopedComponent glue1 = factory.getInstance(GlueScopedComponent.class);
267-
268-
assertAll(
269-
() -> assertThat(belly1, is(notNullValue())),
270-
() -> assertThat(glue1, is(notNullValue())));
271-
266+
long bellyInstance1 = factory.getInstance(Belly.class).getInstanceId();
267+
long glueInstance1 = factory.getInstance(GlueScopedComponent.class).getInstanceId();
272268
factory.stop();
273269

274270
// Scenario 2
275-
final Belly belly2 = factory.getInstance(Belly.class);
276-
final GlueScopedComponent glue2 = factory.getInstance(GlueScopedComponent.class);
271+
factory.start();
272+
long bellyInstance2 = factory.getInstance(Belly.class).getInstanceId();
273+
long glueInstance2 = factory.getInstance(GlueScopedComponent.class).getInstanceId();
274+
factory.stop();
277275

278276
assertAll(
279-
() -> assertThat(belly2, is(notNullValue())),
280-
() -> assertThat(glue2, is(notNullValue())),
281-
() -> assertThat(glue1, is(not(equalTo(glue2)))),
282-
() -> assertThat(glue2, is(not(equalTo(glue1)))),
283-
() -> assertThat(belly1, is(equalTo(belly2))),
284-
() -> assertThat(belly2, is(equalTo(belly1))));
277+
() -> assertThat(glueInstance1, is(not(glueInstance2))),
278+
() -> assertThat(glueInstance2, is(not(glueInstance1))),
279+
() -> assertThat(bellyInstance1, is(bellyInstance2)),
280+
() -> assertThat(bellyInstance2, is(bellyInstance1)));
281+
}
282+
283+
@Test
284+
void shouldThrowWhenGlueScopedSpringBeanAreUsedOutsideLifecycle() {
285+
final ObjectFactory factory = new SpringFactory();
286+
factory.addClass(WithSpringAnnotations.class);
287+
288+
factory.start();
289+
final Belly belly = factory.getInstance(Belly.class);
290+
final GlueScopedComponent glue = factory.getInstance(GlueScopedComponent.class);
291+
factory.stop();
292+
293+
assertDoesNotThrow(belly::getInstanceId);
294+
assertThrows(BeanCreationException.class, glue::getInstanceId);
285295
}
286296

287297
@Test

0 commit comments

Comments
 (0)