diff --git a/cucumber-core/src/main/java/io/cucumber/core/runner/CachingGlue.java b/cucumber-core/src/main/java/io/cucumber/core/runner/CachingGlue.java index b442a2c417..9f0aae8690 100644 --- a/cucumber-core/src/main/java/io/cucumber/core/runner/CachingGlue.java +++ b/cucumber-core/src/main/java/io/cucumber/core/runner/CachingGlue.java @@ -20,12 +20,15 @@ import io.cucumber.core.stepexpression.StepExpressionFactory; import io.cucumber.core.stepexpression.StepTypeRegistry; import io.cucumber.cucumberexpressions.CucumberExpression; +import io.cucumber.cucumberexpressions.DuplicateTypeNameException; import io.cucumber.cucumberexpressions.Expression; import io.cucumber.cucumberexpressions.ParameterByTypeTransformer; import io.cucumber.cucumberexpressions.ParameterType; import io.cucumber.cucumberexpressions.RegularExpression; +import io.cucumber.datatable.DuplicateTypeException; import io.cucumber.datatable.TableCellByTypeTransformer; import io.cucumber.datatable.TableEntryByTypeTransformer; +import io.cucumber.docstring.CucumberDocStringException; import io.cucumber.messages.types.Envelope; import io.cucumber.messages.types.Hook; import io.cucumber.messages.types.JavaMethod; @@ -83,6 +86,7 @@ final class CachingGlue implements Glue { private final EventBus bus; private StepTypeRegistry stepTypeRegistry; + private Locale locale = null; CachingGlue(EventBus bus) { this.bus = bus; @@ -231,21 +235,98 @@ List getDocStringTypeDefinitions() { } StepTypeRegistry getStepTypeRegistry() { - return null; + return stepTypeRegistry; } + int parameterTypeDefinitionsHashCode = 0; + int stepDefinitionsHashCode = 0; + int dataTableTypeDefinitionsHashCode = 0; + int docStringTypeDefinitionsHashCode = 0; + StepExpressionFactory stepExpressionFactory = null; void prepareGlue(Locale locale) throws DuplicateStepDefinitionException { - stepTypeRegistry = new StepTypeRegistry(locale); - StepExpressionFactory stepExpressionFactory = new StepExpressionFactory(stepTypeRegistry, bus); + int parameterTypeDefinitionsHashCodeNew = parameterTypeDefinitions.hashCode(); + int dataTableTypeDefinitionsHashCodeNew = dataTableTypeDefinitions.hashCode(); + int docStringTypeDefinitionsHashCodeNew = docStringTypeDefinitions.hashCode(); + int stepDefinitionsHashCodeNew = stepDefinitions.hashCode(); + boolean firstTime = stepTypeRegistry == null; + boolean languageChanged = firstTime || !locale.equals(this.locale); + boolean parameterTypesDefinitionsChanged = parameterTypeDefinitionsHashCode != parameterTypeDefinitionsHashCodeNew + || parameterTypeDefinitionsHashCode == 0; + boolean docStringTypeDefinitionsChanged = docStringTypeDefinitionsHashCode != docStringTypeDefinitionsHashCodeNew + || + docStringTypeDefinitionsHashCode == 0; + boolean dataTableTypeDefinitionsChanged = dataTableTypeDefinitionsHashCode != dataTableTypeDefinitionsHashCodeNew + || + dataTableTypeDefinitionsHashCode == 0; + boolean stepDefinitionsChanged = stepDefinitionsHashCodeNew != stepDefinitionsHashCode + || stepDefinitionsHashCode == 0; + if (firstTime || languageChanged || + parameterTypesDefinitionsChanged || stepDefinitionsChanged || + dataTableTypeDefinitionsChanged || docStringTypeDefinitionsChanged) { + // conditions changed => invalidate the glue cache + this.locale = locale; + stepTypeRegistry = new StepTypeRegistry(locale); + stepExpressionFactory = new StepExpressionFactory(stepTypeRegistry, bus); + parameterTypesDefinitionsChanged = true; + stepDefinitionsChanged = true; + dataTableTypeDefinitionsChanged = true; + docStringTypeDefinitionsChanged = true; + parameterTypeDefinitionsHashCode = parameterTypeDefinitionsHashCodeNew; + dataTableTypeDefinitionsHashCode = dataTableTypeDefinitionsHashCodeNew; + docStringTypeDefinitionsHashCode = docStringTypeDefinitionsHashCodeNew; + stepDefinitionsHashCode = stepDefinitionsHashCodeNew; + } + + stepDefinitionsChanged = true; // TODO comment this line to make build + // fail, e.g. due to + // io.cucumber.core.plugin.PrettyFormatterTest.should_handle_scenario_outline // TODO: separate prepared and unprepared glue into different classes - parameterTypeDefinitions.forEach(ptd -> { - ParameterType parameterType = ptd.parameterType(); - stepTypeRegistry.defineParameterType(parameterType); - emitParameterTypeDefined(ptd); - }); - dataTableTypeDefinitions.forEach(dtd -> stepTypeRegistry.defineDataTableType(dtd.dataTableType())); - docStringTypeDefinitions.forEach(dtd -> stepTypeRegistry.defineDocStringType(dtd.docStringType())); + if (parameterTypesDefinitionsChanged) { + // parameters changed from the previous scenario => re-register them + parameterTypeDefinitions.forEach(ptd -> { + try { + ParameterType parameterType = ptd.parameterType(); + stepTypeRegistry.defineParameterType(parameterType); + emitParameterTypeDefined(ptd); + } catch (DuplicateTypeNameException e) { + // do nothing (intentionally) + // FIXME catching an exception is a dirty hack, so this + // should + // be replaced by another mechanism (this would probably + // require + // to change StepTypeRegistry API) + } + }); + } + if (dataTableTypeDefinitionsChanged) { + dataTableTypeDefinitions.forEach(dtd -> { + try { + stepTypeRegistry.defineDataTableType(dtd.dataTableType()); + } catch (DuplicateTypeException e) { + // do nothing (intentionally) + // FIXME catching an exception is a dirty hack, so this + // should + // be replaced by another mechanism (this would probably + // require + // to change StepTypeRegistry API) + } + }); + } + if (docStringTypeDefinitionsChanged) { + docStringTypeDefinitions.forEach(dtd -> { + try { + stepTypeRegistry.defineDocStringType(dtd.docStringType()); + } catch (CucumberDocStringException e) { + // do nothing (intentionally) + // FIXME catching an exception is a dirty hack, so this + // should + // be replaced by another mechanism (this would probably + // require + // to change StepTypeRegistry API) + } + }); + } if (defaultParameterTransformers.size() == 1) { DefaultParameterTransformerDefinition definition = defaultParameterTransformers.get(0); @@ -276,17 +357,19 @@ void prepareGlue(Locale locale) throws DuplicateStepDefinitionException { beforeHooks.forEach(this::emitHook); beforeStepHooks.forEach(this::emitHook); - stepDefinitions.forEach(stepDefinition -> { - StepExpression expression = stepExpressionFactory.createExpression(stepDefinition); - CoreStepDefinition coreStepDefinition = new CoreStepDefinition(bus.generateId(), stepDefinition, - expression); - CoreStepDefinition previous = stepDefinitionsByPattern.get(stepDefinition.getPattern()); - if (previous != null) { - throw new DuplicateStepDefinitionException(previous, stepDefinition); - } - stepDefinitionsByPattern.put(coreStepDefinition.getExpression().getSource(), coreStepDefinition); - emitStepDefined(coreStepDefinition); - }); + if (stepDefinitionsChanged) { + stepDefinitions.forEach(stepDefinition -> { + StepExpression expression = stepExpressionFactory.createExpression(stepDefinition); + CoreStepDefinition coreStepDefinition = new CoreStepDefinition(bus.generateId(), stepDefinition, + expression); + CoreStepDefinition previous = stepDefinitionsByPattern.get(stepDefinition.getPattern()); + if (previous != null) { + throw new DuplicateStepDefinitionException(previous, stepDefinition); + } + stepDefinitionsByPattern.put(coreStepDefinition.getExpression().getSource(), coreStepDefinition); + emitStepDefined(coreStepDefinition); + }); + } afterStepHooks.forEach(this::emitHook); afterHooks.forEach(this::emitHook); diff --git a/cucumber-core/src/test/java/io/cucumber/core/runner/CachingGlueTest.java b/cucumber-core/src/test/java/io/cucumber/core/runner/CachingGlueTest.java index c67d33d9e3..53a63d04cc 100644 --- a/cucumber-core/src/test/java/io/cucumber/core/runner/CachingGlueTest.java +++ b/cucumber-core/src/test/java/io/cucumber/core/runner/CachingGlueTest.java @@ -16,7 +16,6 @@ import io.cucumber.core.gherkin.Feature; import io.cucumber.core.gherkin.Step; import io.cucumber.core.runtime.TimeServiceEventBus; -import io.cucumber.core.stepexpression.StepTypeRegistry; import io.cucumber.cucumberexpressions.ParameterByTypeTransformer; import io.cucumber.cucumberexpressions.ParameterType; import io.cucumber.datatable.DataTable; @@ -33,6 +32,7 @@ import java.time.Clock; import java.util.ArrayList; import java.util.List; +import java.util.Locale; import java.util.Optional; import java.util.UUID; import java.util.stream.Collectors; @@ -53,7 +53,7 @@ class CachingGlueTest { - private final StepTypeRegistry stepTypeRegistry = new StepTypeRegistry(ENGLISH); + public static final Locale LANGUAGE = ENGLISH; private final CachingGlue glue = new CachingGlue(new TimeServiceEventBus(Clock.systemUTC(), UUID::randomUUID)); @Test @@ -70,7 +70,7 @@ void throws_duplicate_error_on_dupe_stepdefs() { DuplicateStepDefinitionException exception = assertThrows( DuplicateStepDefinitionException.class, - () -> glue.prepareGlue(stepTypeRegistry)); + () -> glue.prepareGlue(LANGUAGE)); assertThat(exception.getMessage(), equalTo("Duplicate step definitions in foo.bf:10 and bar.bf:90")); } @@ -81,7 +81,7 @@ void throws_on_duplicate_default_parameter_transformer() { DuplicateDefaultParameterTransformers exception = assertThrows( DuplicateDefaultParameterTransformers.class, - () -> glue.prepareGlue(stepTypeRegistry)); + () -> glue.prepareGlue(LANGUAGE)); assertThat(exception.getMessage(), equalTo("" + "There may not be more then one default parameter transformer. Found:\n" + " - mocked default parameter transformer\n" + @@ -95,7 +95,7 @@ void throws_on_duplicate_default_table_entry_transformer() { DuplicateDefaultDataTableEntryTransformers exception = assertThrows( DuplicateDefaultDataTableEntryTransformers.class, - () -> glue.prepareGlue(stepTypeRegistry)); + () -> glue.prepareGlue(LANGUAGE)); assertThat(exception.getMessage(), equalTo("" + "There may not be more then one default data table entry. Found:\n" + " - mocked default data table entry transformer\n" + @@ -109,7 +109,7 @@ void throws_on_duplicate_default_table_cell_transformer() { DuplicateDefaultDataTableCellTransformers exception = assertThrows( DuplicateDefaultDataTableCellTransformers.class, - () -> glue.prepareGlue(stepTypeRegistry)); + () -> glue.prepareGlue(LANGUAGE)); assertThat(exception.getMessage(), equalTo("" + "There may not be more then one default table cell transformers. Found:\n" + " - mocked default data table cell transformer\n" + @@ -135,7 +135,7 @@ void removes_glue_that_is_scenario_scoped() { glue.addDefaultDataTableCellTransformer(new MockedDefaultDataTableCellTransformer()); glue.addDefaultDataTableEntryTransformer(new MockedDefaultDataTableEntryTransformer()); - glue.prepareGlue(stepTypeRegistry); + glue.prepareGlue(LANGUAGE); assertAll( () -> assertThat(glue.getStepDefinitions().size(), is(equalTo(1))), @@ -191,7 +191,7 @@ void returns_match_from_cache_if_single_found() throws AmbiguousStepDefinitionsE StepDefinition stepDefinition2 = new MockedStepDefinition("^pattern2"); glue.addStepDefinition(stepDefinition1); glue.addStepDefinition(stepDefinition2); - glue.prepareGlue(stepTypeRegistry); + glue.prepareGlue(LANGUAGE); URI uri = URI.create("file:path/to.feature"); String stepText = "pattern1"; @@ -219,7 +219,7 @@ void returns_match_from_cache_for_step_with_table() throws AmbiguousStepDefiniti StepDefinition stepDefinition2 = new MockedStepDefinition("^pattern2", DataTable.class); glue.addStepDefinition(stepDefinition1); glue.addStepDefinition(stepDefinition2); - glue.prepareGlue(stepTypeRegistry); + glue.prepareGlue(LANGUAGE); URI uri = URI.create("file:path/to.feature"); String stepText = "pattern1"; @@ -260,7 +260,7 @@ void returns_match_from_cache_for_ste_with_doc_string() throws AmbiguousStepDefi StepDefinition stepDefinition2 = new MockedStepDefinition("^pattern2", String.class); glue.addStepDefinition(stepDefinition1); glue.addStepDefinition(stepDefinition2); - glue.prepareGlue(stepTypeRegistry); + glue.prepareGlue(LANGUAGE); URI uri = URI.create("file:path/to.feature"); String stepText = "pattern1"; @@ -304,7 +304,7 @@ void returns_fresh_match_from_cache_after_evicting_scenario_scoped() throws Ambi StepDefinition stepDefinition1 = new MockedScenarioScopedStepDefinition("^pattern1"); glue.addStepDefinition(stepDefinition1); - glue.prepareGlue(stepTypeRegistry); + glue.prepareGlue(LANGUAGE); PickleStepDefinitionMatch pickleStepDefinitionMatch = glue.stepDefinitionMatch(uri, pickleStep1); assertThat(((CoreStepDefinition) pickleStepDefinitionMatch.getStepDefinition()).getStepDefinition(), @@ -314,7 +314,7 @@ void returns_fresh_match_from_cache_after_evicting_scenario_scoped() throws Ambi StepDefinition stepDefinition2 = new MockedScenarioScopedStepDefinition("^pattern1"); glue.addStepDefinition(stepDefinition2); - glue.prepareGlue(stepTypeRegistry); + glue.prepareGlue(LANGUAGE); PickleStepDefinitionMatch pickleStepDefinitionMatch2 = glue.stepDefinitionMatch(uri, pickleStep1); assertThat(((CoreStepDefinition) pickleStepDefinitionMatch2.getStepDefinition()).getStepDefinition(), @@ -347,7 +347,7 @@ void disposes_of_scenario_scoped_beans() { MockedDefaultParameterTransformer defaultParameterTransformer = new MockedDefaultParameterTransformer(); glue.addDefaultParameterTransformer(defaultParameterTransformer); - glue.prepareGlue(stepTypeRegistry); + glue.prepareGlue(LANGUAGE); glue.removeScenarioScopedGlue(); assertThat(stepDefinition.isDisposed(), is(true)); @@ -371,7 +371,7 @@ void returns_no_match_after_evicting_scenario_scoped() throws AmbiguousStepDefin StepDefinition stepDefinition1 = new MockedScenarioScopedStepDefinition("^pattern1"); glue.addStepDefinition(stepDefinition1); - glue.prepareGlue(stepTypeRegistry); + glue.prepareGlue(LANGUAGE); PickleStepDefinitionMatch pickleStepDefinitionMatch = glue.stepDefinitionMatch(uri, pickleStep1); assertThat(((CoreStepDefinition) pickleStepDefinitionMatch.getStepDefinition()).getStepDefinition(), @@ -379,7 +379,7 @@ void returns_no_match_after_evicting_scenario_scoped() throws AmbiguousStepDefin glue.removeScenarioScopedGlue(); - glue.prepareGlue(stepTypeRegistry); + glue.prepareGlue(LANGUAGE); PickleStepDefinitionMatch pickleStepDefinitionMatch2 = glue.stepDefinitionMatch(uri, pickleStep1); assertThat(pickleStepDefinitionMatch2, nullValue()); @@ -393,7 +393,7 @@ void throws_ambiguous_steps_def_exception_when_many_patterns_match() { glue.addStepDefinition(stepDefinition1); glue.addStepDefinition(stepDefinition2); glue.addStepDefinition(stepDefinition3); - glue.prepareGlue(stepTypeRegistry); + glue.prepareGlue(LANGUAGE); URI uri = URI.create("file:path/to.feature"); @@ -480,7 +480,7 @@ void emits_hook_messages_to_bus() { glue.addBeforeStepHook(new MockedScenarioScopedHookDefinition()); glue.addAfterStepHook(new MockedScenarioScopedHookDefinition()); - glue.prepareGlue(stepTypeRegistry); + glue.prepareGlue(LANGUAGE); assertThat(events.size(), is(4)); } @@ -497,7 +497,7 @@ void parameterTypeDefinition_without_source_reference_emits_parameterType_with_e glue.addParameterType(new MockedParameterTypeDefinition()); // When - glue.prepareGlue(stepTypeRegistry); + glue.prepareGlue(LANGUAGE); // Then assertThat(events.size(), is(1)); @@ -519,7 +519,7 @@ void parameterTypeDefinition_with_source_reference_emits_parameterType_with_non_ glue.addParameterType(new MockedParameterTypeDefinitionWithSourceReference()); // When - glue.prepareGlue(stepTypeRegistry); + glue.prepareGlue(LANGUAGE); // Then assertThat(events.size(), is(1));