[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

Author: mpkorstanje

spring/README.md

@@ -85,19 +85,18 @@ The `cucumber-glue` scope starts prior to a scenario and ends after a scenario.
 All beans in this scope will be created before a scenario execution and
 disposed at the end of it.
 
-By using the `CucumberTestContext.SCOPE_CUCUMBER_GLUE` additional components
-can be added to the glue scope. These components can be used to safely share
-state between scenarios. 
+By using the `@ScenarioScope` annotation additional components can be added to
+the glue scope. These components can be used to safely share state between
+scenarios. 
 
 ```java
 package com.example.app;
 
 import org.springframework.stereotype.Component;
-import org.springframework.context.annotation.Scope;
-import static io.cucumber.spring.CucumberTestContext;
+import io.cucumber.spring.ScenarioScope;
 
 @Component
-@Scope(CucumberTestContext.SCOPE_CUCUMBER_GLUE)
+@ScenarioScope
 public class TestUserInformation {
 
     private User testUser;

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

@@ -3,11 +3,11 @@
 import org.springframework.beans.factory.ObjectFactory;
 import org.springframework.beans.factory.config.Scope;
 
-class GlueCodeScope implements Scope {
+class CucumberScenarioScope implements Scope {
 
     @Override
     public Object get(String name, ObjectFactory<?> objectFactory) {
-        GlueCodeContext context = GlueCodeContext.getInstance();
+        CucumberTestContext context = CucumberTestContext.getInstance();
         Object obj = context.get(name);
         if (obj == null) {
             obj = objectFactory.getObject();
@@ -19,13 +19,13 @@ public Object get(String name, ObjectFactory<?> objectFactory) {
 
     @Override
     public Object remove(String name) {
-        GlueCodeContext context = GlueCodeContext.getInstance();
+        CucumberTestContext context = CucumberTestContext.getInstance();
         return context.remove(name);
     }
 
     @Override
     public void registerDestructionCallback(String name, Runnable callback) {
-        GlueCodeContext context = GlueCodeContext.getInstance();
+        CucumberTestContext context = CucumberTestContext.getInstance();
         context.registerDestructionCallback(name, callback);
     }
 
@@ -36,7 +36,7 @@ public Object resolveContextualObject(String key) {
 
     @Override
     public String getConversationId() {
-        GlueCodeContext context = GlueCodeContext.getInstance();
+        CucumberTestContext context = CucumberTestContext.getInstance();
         return context.getId();
     }
 

spring/src/main/java/io/cucumber/spring/CucumberTestContext.java

@@ -2,12 +2,75 @@
 
 import org.apiguardian.api.API;
 
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.atomic.AtomicInteger;
+
 @API(status = API.Status.STABLE)
 public final class CucumberTestContext {
 
     public static final String SCOPE_CUCUMBER_GLUE = "cucumber-glue";
 
+    private static final ThreadLocal<CucumberTestContext> localContext = ThreadLocal
+            .withInitial(CucumberTestContext::new);
+    private static final AtomicInteger globalCounter = new AtomicInteger(0);
+
+    private final Map<String, Object> objects = new HashMap<>();
+    private final Map<String, Runnable> callbacks = new HashMap<>();
+
+    private int counter;
+    private boolean active = false;
+
     private CucumberTestContext() {
     }
 
+    static CucumberTestContext getInstance() {
+        return localContext.get();
+    }
+
+    void start() {
+        counter = globalCounter.incrementAndGet();
+        active = true;
+    }
+
+    String getId() {
+        return "cucumber_test_context_" + counter;
+    }
+
+    void stop() {
+        for (Runnable callback : callbacks.values()) {
+            callback.run();
+        }
+        localContext.remove();
+        active = false;
+    }
+
+    Object get(String name) {
+        requireActiveScenario();
+        return objects.get(name);
+    }
+
+    void put(String name, Object object) {
+        requireActiveScenario();
+        objects.put(name, object);
+    }
+
+    Object remove(String name) {
+        requireActiveScenario();
+        callbacks.remove(name);
+        return objects.remove(name);
+    }
+
+    void registerDestructionCallback(String name, Runnable callback) {
+        requireActiveScenario();
+        callbacks.put(name, callback);
+    }
+
+    void requireActiveScenario() {
+        if (!active) {
+            throw new IllegalStateException(
+                "Scenario scoped beans can only be created while Cucumber is executing a scenario");
+        }
+    }
+
 }

spring/src/main/java/io/cucumber/spring/GlueCodeContext.java

@@ -0,0 +1,27 @@
+package io.cucumber.spring;
+
+import org.apiguardian.api.API;
+import org.springframework.context.annotation.Scope;
+import org.springframework.context.annotation.ScopedProxyMode;
+import org.springframework.core.annotation.AliasFor;
+
+import java.lang.annotation.Documented;
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+/**
+ * Marks a bean as scoped to the execution of a cucumber scenario.
+ */
+@Target({ ElementType.TYPE, ElementType.METHOD })
+@Retention(RetentionPolicy.RUNTIME)
+@Documented
+@Scope(CucumberTestContext.SCOPE_CUCUMBER_GLUE)
+@API(status = API.Status.STABLE)
+public @interface ScenarioScope {
+    @AliasFor(
+            annotation = Scope.class)
+    ScopedProxyMode proxyMode() default ScopedProxyMode.TARGET_CLASS;
+
+}

spring/src/main/java/io/cucumber/spring/ScenarioScope.java

@@ -52,12 +52,12 @@ public final void start() {
             notifyContextManagerAboutTestClassStarted();
             registerStepClassBeanDefinitions(applicationContext.getBeanFactory());
         }
-        GlueCodeContext.getInstance().start();
+        CucumberTestContext.getInstance().start();
     }
 
     final void registerGlueCodeScope(ConfigurableApplicationContext context) {
         while (context != null) {
-            context.getBeanFactory().registerScope(SCOPE_CUCUMBER_GLUE, new GlueCodeScope());
+            context.getBeanFactory().registerScope(SCOPE_CUCUMBER_GLUE, new CucumberScenarioScope());
             context = (ConfigurableApplicationContext) context.getParent();
         }
     }
@@ -91,7 +91,7 @@ private void registerStepClassBeanDefinition(BeanDefinitionRegistry registry, Cl
     }
 
     public final void stop() {
-        GlueCodeContext.getInstance().stop();
+        CucumberTestContext.getInstance().stop();
         try {
             delegate.afterTestClass();
         } catch (Exception e) {

spring/src/main/java/io/cucumber/spring/TestContextAdaptor.java

@@ -0,0 +1,88 @@
+package io.cucumber.spring;
+
+import io.cucumber.core.backend.ObjectFactory;
+import org.junit.jupiter.api.Test;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.beans.factory.config.BeanFactoryPostProcessor;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.test.context.ContextConfiguration;
+
+import java.util.concurrent.atomic.AtomicInteger;
+
+import static io.cucumber.spring.CucumberTestContext.SCOPE_CUCUMBER_GLUE;
+import static org.junit.Assert.assertNotEquals;
+
+class Issue1970 {
+
+    @Test
+    public void issue1970() {
+        ObjectFactory factory = new SpringFactory();
+        factory.addClass(GlueClass.class); // Add glue with Spring configuration
+        factory.start();
+        GlueClass instance = factory.getInstance(GlueClass.class);
+        String response = instance.service.get();
+        factory.stop();
+        factory.start();
+        GlueClass instance2 = factory.getInstance(GlueClass.class);
+        String response2 = instance2.service.get();
+        factory.stop();
+
+        assertNotEquals(response, response2);
+    }
+
+    @CucumberContextConfiguration
+    @ContextConfiguration(classes = TestApplicationConfiguration.class)
+    public static class GlueClass {
+
+        @Autowired
+        ExampleService service;
+
+    }
+
+    @Configuration
+    public static class TestApplicationConfiguration {
+
+        @Bean
+        public BeanFactoryPostProcessor beanFactoryPostProcessor() {
+            return factory -> factory.registerScope(SCOPE_CUCUMBER_GLUE, new CucumberScenarioScope());
+        }
+
+        @Bean
+        public ExampleService service(ScenarioScopedApi api) {
+            return new ExampleService(api);
+        }
+
+        @Bean
+        @ScenarioScope
+        public ScenarioScopedApi api() {
+            return new ScenarioScopedApi();
+        }
+
+    }
+
+    public static class ExampleService {
+
+        final ScenarioScopedApi api;
+
+        public ExampleService(ScenarioScopedApi api) {
+            this.api = api;
+        }
+
+        String get() {
+            return "Api response: " + api.get();
+        }
+    }
+
+    public static class ScenarioScopedApi {
+
+        private static final AtomicInteger globalCounter = new AtomicInteger(0);
+        private final int instanceId = globalCounter.getAndIncrement();
+
+        public String get() {
+            return "instance " + instanceId;
+        }
+
+    }
+
+}

spring/src/test/java/io/cucumber/spring/Issue1970.java

@@ -18,6 +18,7 @@
 import io.cucumber.spring.metaconfig.general.BellyMetaStepDefinitions;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.function.Executable;
+import org.springframework.beans.factory.BeanCreationException;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.test.context.ContextConfiguration;
@@ -262,26 +263,35 @@ void shouldGlueScopedSpringBeanBehaveLikeGlueLifecycle() {
 
         // Scenario 1
         factory.start();
-        final Belly belly1 = factory.getInstance(Belly.class);
-        final GlueScopedComponent glue1 = factory.getInstance(GlueScopedComponent.class);
-
-        assertAll(
-            () -> assertThat(belly1, is(notNullValue())),
-            () -> assertThat(glue1, is(notNullValue())));
-
+        long bellyInstance1 = factory.getInstance(Belly.class).getInstanceId();
+        long glueInstance1 = factory.getInstance(GlueScopedComponent.class).getInstanceId();
         factory.stop();
 
         // Scenario 2
-        final Belly belly2 = factory.getInstance(Belly.class);
-        final GlueScopedComponent glue2 = factory.getInstance(GlueScopedComponent.class);
+        factory.start();
+        long bellyInstance2 = factory.getInstance(Belly.class).getInstanceId();
+        long glueInstance2 = factory.getInstance(GlueScopedComponent.class).getInstanceId();
+        factory.stop();
 
         assertAll(
-            () -> assertThat(belly2, is(notNullValue())),
-            () -> assertThat(glue2, is(notNullValue())),
-            () -> assertThat(glue1, is(not(equalTo(glue2)))),
-            () -> assertThat(glue2, is(not(equalTo(glue1)))),
-            () -> assertThat(belly1, is(equalTo(belly2))),
-            () -> assertThat(belly2, is(equalTo(belly1))));
+            () -> assertThat(glueInstance1, is(not(glueInstance2))),
+            () -> assertThat(glueInstance2, is(not(glueInstance1))),
+            () -> assertThat(bellyInstance1, is(bellyInstance2)),
+            () -> assertThat(bellyInstance2, is(bellyInstance1)));
+    }
+
+    @Test
+    void shouldThrowWhenGlueScopedSpringBeanAreUsedOutsideLifecycle() {
+        final ObjectFactory factory = new SpringFactory();
+        factory.addClass(WithSpringAnnotations.class);
+
+        factory.start();
+        final Belly belly = factory.getInstance(Belly.class);
+        final GlueScopedComponent glue = factory.getInstance(GlueScopedComponent.class);
+        factory.stop();
+
+        assertDoesNotThrow(belly::getInstanceId);
+        assertThrows(BeanCreationException.class, glue::getInstanceId);
     }
 
     @Test