Skip to content

Commit 9521ea1

Browse files
U117293U117293
U117293
authored and
U117293
committed
fix: corrected build + tentative of caching glue for #2902 
1 parent ef7be7f commit 9521ea1

File tree

2 files changed

+123
-40
lines changed

2 files changed

+123
-40
lines changed

Diff for: ‎cucumber-core/src/main/java/io/cucumber/core/runner/CachingGlue.java

+104-21
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,15 @@
2020
import io.cucumber.core.stepexpression.StepExpressionFactory;
2121
import io.cucumber.core.stepexpression.StepTypeRegistry;
2222
import io.cucumber.cucumberexpressions.CucumberExpression;
23+
import io.cucumber.cucumberexpressions.DuplicateTypeNameException;
2324
import io.cucumber.cucumberexpressions.Expression;
2425
import io.cucumber.cucumberexpressions.ParameterByTypeTransformer;
2526
import io.cucumber.cucumberexpressions.ParameterType;
2627
import io.cucumber.cucumberexpressions.RegularExpression;
28+
import io.cucumber.datatable.DuplicateTypeException;
2729
import io.cucumber.datatable.TableCellByTypeTransformer;
2830
import io.cucumber.datatable.TableEntryByTypeTransformer;
31+
import io.cucumber.docstring.CucumberDocStringException;
2932
import io.cucumber.messages.types.Envelope;
3033
import io.cucumber.messages.types.Hook;
3134
import io.cucumber.messages.types.JavaMethod;
@@ -83,6 +86,7 @@ final class CachingGlue implements Glue {
8386

8487
private final EventBus bus;
8588
private StepTypeRegistry stepTypeRegistry;
89+
private Locale locale = null;
8690

8791
CachingGlue(EventBus bus) {
8892
this.bus = bus;
@@ -231,21 +235,98 @@ List<DocStringTypeDefinition> getDocStringTypeDefinitions() {
231235
}
232236

233237
StepTypeRegistry getStepTypeRegistry() {
234-
return null;
238+
return stepTypeRegistry;
235239
}
240+
int parameterTypeDefinitionsHashCode = 0;
241+
int stepDefinitionsHashCode = 0;
242+
int dataTableTypeDefinitionsHashCode = 0;
243+
int docStringTypeDefinitionsHashCode = 0;
236244

245+
StepExpressionFactory stepExpressionFactory = null;
237246
void prepareGlue(Locale locale) throws DuplicateStepDefinitionException {
238-
stepTypeRegistry = new StepTypeRegistry(locale);
239-
StepExpressionFactory stepExpressionFactory = new StepExpressionFactory(stepTypeRegistry, bus);
247+
int parameterTypeDefinitionsHashCodeNew = parameterTypeDefinitions.hashCode();
248+
int dataTableTypeDefinitionsHashCodeNew = dataTableTypeDefinitions.hashCode();
249+
int docStringTypeDefinitionsHashCodeNew = docStringTypeDefinitions.hashCode();
250+
int stepDefinitionsHashCodeNew = stepDefinitions.hashCode();
251+
boolean firstTime = stepTypeRegistry == null;
252+
boolean languageChanged = firstTime || !locale.equals(this.locale);
253+
boolean parameterTypesDefinitionsChanged = parameterTypeDefinitionsHashCode != parameterTypeDefinitionsHashCodeNew
254+
|| parameterTypeDefinitionsHashCode == 0;
255+
boolean docStringTypeDefinitionsChanged = docStringTypeDefinitionsHashCode != docStringTypeDefinitionsHashCodeNew
256+
||
257+
docStringTypeDefinitionsHashCode == 0;
258+
boolean dataTableTypeDefinitionsChanged = dataTableTypeDefinitionsHashCode != dataTableTypeDefinitionsHashCodeNew
259+
||
260+
dataTableTypeDefinitionsHashCode == 0;
261+
boolean stepDefinitionsChanged = stepDefinitionsHashCodeNew != stepDefinitionsHashCode
262+
|| stepDefinitionsHashCode == 0;
263+
if (firstTime || languageChanged ||
264+
parameterTypesDefinitionsChanged || stepDefinitionsChanged ||
265+
dataTableTypeDefinitionsChanged || docStringTypeDefinitionsChanged) {
266+
// conditions changed => invalidate the glue cache
267+
this.locale = locale;
268+
stepTypeRegistry = new StepTypeRegistry(locale);
269+
stepExpressionFactory = new StepExpressionFactory(stepTypeRegistry, bus);
270+
parameterTypesDefinitionsChanged = true;
271+
stepDefinitionsChanged = true;
272+
dataTableTypeDefinitionsChanged = true;
273+
docStringTypeDefinitionsChanged = true;
274+
parameterTypeDefinitionsHashCode = parameterTypeDefinitionsHashCodeNew;
275+
dataTableTypeDefinitionsHashCode = dataTableTypeDefinitionsHashCodeNew;
276+
docStringTypeDefinitionsHashCode = docStringTypeDefinitionsHashCodeNew;
277+
stepDefinitionsHashCode = stepDefinitionsHashCodeNew;
278+
}
279+
280+
stepDefinitionsChanged = true; // TODO comment this line to make build
281+
// fail, e.g. due to
282+
// io.cucumber.core.plugin.PrettyFormatterTest.should_handle_scenario_outline
240283

241284
// TODO: separate prepared and unprepared glue into different classes
242-
parameterTypeDefinitions.forEach(ptd -> {
243-
ParameterType<?> parameterType = ptd.parameterType();
244-
stepTypeRegistry.defineParameterType(parameterType);
245-
emitParameterTypeDefined(ptd);
246-
});
247-
dataTableTypeDefinitions.forEach(dtd -> stepTypeRegistry.defineDataTableType(dtd.dataTableType()));
248-
docStringTypeDefinitions.forEach(dtd -> stepTypeRegistry.defineDocStringType(dtd.docStringType()));
285+
if (parameterTypesDefinitionsChanged) {
286+
// parameters changed from the previous scenario => re-register them
287+
parameterTypeDefinitions.forEach(ptd -> {
288+
try {
289+
ParameterType<?> parameterType = ptd.parameterType();
290+
stepTypeRegistry.defineParameterType(parameterType);
291+
emitParameterTypeDefined(ptd);
292+
} catch (DuplicateTypeNameException e) {
293+
// do nothing (intentionally)
294+
// FIXME catching an exception is a dirty hack, so this
295+
// should
296+
// be replaced by another mechanism (this would probably
297+
// require
298+
// to change StepTypeRegistry API)
299+
}
300+
});
301+
}
302+
if (dataTableTypeDefinitionsChanged) {
303+
dataTableTypeDefinitions.forEach(dtd -> {
304+
try {
305+
stepTypeRegistry.defineDataTableType(dtd.dataTableType());
306+
} catch (DuplicateTypeException e) {
307+
// do nothing (intentionally)
308+
// FIXME catching an exception is a dirty hack, so this
309+
// should
310+
// be replaced by another mechanism (this would probably
311+
// require
312+
// to change StepTypeRegistry API)
313+
}
314+
});
315+
}
316+
if (docStringTypeDefinitionsChanged) {
317+
docStringTypeDefinitions.forEach(dtd -> {
318+
try {
319+
stepTypeRegistry.defineDocStringType(dtd.docStringType());
320+
} catch (CucumberDocStringException e) {
321+
// do nothing (intentionally)
322+
// FIXME catching an exception is a dirty hack, so this
323+
// should
324+
// be replaced by another mechanism (this would probably
325+
// require
326+
// to change StepTypeRegistry API)
327+
}
328+
});
329+
}
249330

250331
if (defaultParameterTransformers.size() == 1) {
251332
DefaultParameterTransformerDefinition definition = defaultParameterTransformers.get(0);
@@ -276,17 +357,19 @@ void prepareGlue(Locale locale) throws DuplicateStepDefinitionException {
276357
beforeHooks.forEach(this::emitHook);
277358
beforeStepHooks.forEach(this::emitHook);
278359

279-
stepDefinitions.forEach(stepDefinition -> {
280-
StepExpression expression = stepExpressionFactory.createExpression(stepDefinition);
281-
CoreStepDefinition coreStepDefinition = new CoreStepDefinition(bus.generateId(), stepDefinition,
282-
expression);
283-
CoreStepDefinition previous = stepDefinitionsByPattern.get(stepDefinition.getPattern());
284-
if (previous != null) {
285-
throw new DuplicateStepDefinitionException(previous, stepDefinition);
286-
}
287-
stepDefinitionsByPattern.put(coreStepDefinition.getExpression().getSource(), coreStepDefinition);
288-
emitStepDefined(coreStepDefinition);
289-
});
360+
if (stepDefinitionsChanged) {
361+
stepDefinitions.forEach(stepDefinition -> {
362+
StepExpression expression = stepExpressionFactory.createExpression(stepDefinition);
363+
CoreStepDefinition coreStepDefinition = new CoreStepDefinition(bus.generateId(), stepDefinition,
364+
expression);
365+
CoreStepDefinition previous = stepDefinitionsByPattern.get(stepDefinition.getPattern());
366+
if (previous != null) {
367+
throw new DuplicateStepDefinitionException(previous, stepDefinition);
368+
}
369+
stepDefinitionsByPattern.put(coreStepDefinition.getExpression().getSource(), coreStepDefinition);
370+
emitStepDefined(coreStepDefinition);
371+
});
372+
}
290373

291374
afterStepHooks.forEach(this::emitHook);
292375
afterHooks.forEach(this::emitHook);

Diff for: ‎cucumber-core/src/test/java/io/cucumber/core/runner/CachingGlueTest.java

+19-19
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
import io.cucumber.core.gherkin.Feature;
1717
import io.cucumber.core.gherkin.Step;
1818
import io.cucumber.core.runtime.TimeServiceEventBus;
19-
import io.cucumber.core.stepexpression.StepTypeRegistry;
2019
import io.cucumber.cucumberexpressions.ParameterByTypeTransformer;
2120
import io.cucumber.cucumberexpressions.ParameterType;
2221
import io.cucumber.datatable.DataTable;
@@ -33,6 +32,7 @@
3332
import java.time.Clock;
3433
import java.util.ArrayList;
3534
import java.util.List;
35+
import java.util.Locale;
3636
import java.util.Optional;
3737
import java.util.UUID;
3838
import java.util.stream.Collectors;
@@ -53,7 +53,7 @@
5353

5454
class CachingGlueTest {
5555

56-
private final StepTypeRegistry stepTypeRegistry = new StepTypeRegistry(ENGLISH);
56+
public static final Locale LANGUAGE = ENGLISH;
5757
private final CachingGlue glue = new CachingGlue(new TimeServiceEventBus(Clock.systemUTC(), UUID::randomUUID));
5858

5959
@Test
@@ -70,7 +70,7 @@ void throws_duplicate_error_on_dupe_stepdefs() {
7070

7171
DuplicateStepDefinitionException exception = assertThrows(
7272
DuplicateStepDefinitionException.class,
73-
() -> glue.prepareGlue(stepTypeRegistry));
73+
() -> glue.prepareGlue(LANGUAGE));
7474
assertThat(exception.getMessage(), equalTo("Duplicate step definitions in foo.bf:10 and bar.bf:90"));
7575
}
7676

@@ -81,7 +81,7 @@ void throws_on_duplicate_default_parameter_transformer() {
8181

8282
DuplicateDefaultParameterTransformers exception = assertThrows(
8383
DuplicateDefaultParameterTransformers.class,
84-
() -> glue.prepareGlue(stepTypeRegistry));
84+
() -> glue.prepareGlue(LANGUAGE));
8585
assertThat(exception.getMessage(), equalTo("" +
8686
"There may not be more then one default parameter transformer. Found:\n" +
8787
" - mocked default parameter transformer\n" +
@@ -95,7 +95,7 @@ void throws_on_duplicate_default_table_entry_transformer() {
9595

9696
DuplicateDefaultDataTableEntryTransformers exception = assertThrows(
9797
DuplicateDefaultDataTableEntryTransformers.class,
98-
() -> glue.prepareGlue(stepTypeRegistry));
98+
() -> glue.prepareGlue(LANGUAGE));
9999
assertThat(exception.getMessage(), equalTo("" +
100100
"There may not be more then one default data table entry. Found:\n" +
101101
" - mocked default data table entry transformer\n" +
@@ -109,7 +109,7 @@ void throws_on_duplicate_default_table_cell_transformer() {
109109

110110
DuplicateDefaultDataTableCellTransformers exception = assertThrows(
111111
DuplicateDefaultDataTableCellTransformers.class,
112-
() -> glue.prepareGlue(stepTypeRegistry));
112+
() -> glue.prepareGlue(LANGUAGE));
113113
assertThat(exception.getMessage(), equalTo("" +
114114
"There may not be more then one default table cell transformers. Found:\n" +
115115
" - mocked default data table cell transformer\n" +
@@ -135,7 +135,7 @@ void removes_glue_that_is_scenario_scoped() {
135135
glue.addDefaultDataTableCellTransformer(new MockedDefaultDataTableCellTransformer());
136136
glue.addDefaultDataTableEntryTransformer(new MockedDefaultDataTableEntryTransformer());
137137

138-
glue.prepareGlue(stepTypeRegistry);
138+
glue.prepareGlue(LANGUAGE);
139139

140140
assertAll(
141141
() -> assertThat(glue.getStepDefinitions().size(), is(equalTo(1))),
@@ -191,7 +191,7 @@ void returns_match_from_cache_if_single_found() throws AmbiguousStepDefinitionsE
191191
StepDefinition stepDefinition2 = new MockedStepDefinition("^pattern2");
192192
glue.addStepDefinition(stepDefinition1);
193193
glue.addStepDefinition(stepDefinition2);
194-
glue.prepareGlue(stepTypeRegistry);
194+
glue.prepareGlue(LANGUAGE);
195195

196196
URI uri = URI.create("file:path/to.feature");
197197
String stepText = "pattern1";
@@ -219,7 +219,7 @@ void returns_match_from_cache_for_step_with_table() throws AmbiguousStepDefiniti
219219
StepDefinition stepDefinition2 = new MockedStepDefinition("^pattern2", DataTable.class);
220220
glue.addStepDefinition(stepDefinition1);
221221
glue.addStepDefinition(stepDefinition2);
222-
glue.prepareGlue(stepTypeRegistry);
222+
glue.prepareGlue(LANGUAGE);
223223

224224
URI uri = URI.create("file:path/to.feature");
225225
String stepText = "pattern1";
@@ -260,7 +260,7 @@ void returns_match_from_cache_for_ste_with_doc_string() throws AmbiguousStepDefi
260260
StepDefinition stepDefinition2 = new MockedStepDefinition("^pattern2", String.class);
261261
glue.addStepDefinition(stepDefinition1);
262262
glue.addStepDefinition(stepDefinition2);
263-
glue.prepareGlue(stepTypeRegistry);
263+
glue.prepareGlue(LANGUAGE);
264264

265265
URI uri = URI.create("file:path/to.feature");
266266
String stepText = "pattern1";
@@ -304,7 +304,7 @@ void returns_fresh_match_from_cache_after_evicting_scenario_scoped() throws Ambi
304304

305305
StepDefinition stepDefinition1 = new MockedScenarioScopedStepDefinition("^pattern1");
306306
glue.addStepDefinition(stepDefinition1);
307-
glue.prepareGlue(stepTypeRegistry);
307+
glue.prepareGlue(LANGUAGE);
308308

309309
PickleStepDefinitionMatch pickleStepDefinitionMatch = glue.stepDefinitionMatch(uri, pickleStep1);
310310
assertThat(((CoreStepDefinition) pickleStepDefinitionMatch.getStepDefinition()).getStepDefinition(),
@@ -314,7 +314,7 @@ void returns_fresh_match_from_cache_after_evicting_scenario_scoped() throws Ambi
314314

315315
StepDefinition stepDefinition2 = new MockedScenarioScopedStepDefinition("^pattern1");
316316
glue.addStepDefinition(stepDefinition2);
317-
glue.prepareGlue(stepTypeRegistry);
317+
glue.prepareGlue(LANGUAGE);
318318

319319
PickleStepDefinitionMatch pickleStepDefinitionMatch2 = glue.stepDefinitionMatch(uri, pickleStep1);
320320
assertThat(((CoreStepDefinition) pickleStepDefinitionMatch2.getStepDefinition()).getStepDefinition(),
@@ -347,7 +347,7 @@ void disposes_of_scenario_scoped_beans() {
347347
MockedDefaultParameterTransformer defaultParameterTransformer = new MockedDefaultParameterTransformer();
348348
glue.addDefaultParameterTransformer(defaultParameterTransformer);
349349

350-
glue.prepareGlue(stepTypeRegistry);
350+
glue.prepareGlue(LANGUAGE);
351351
glue.removeScenarioScopedGlue();
352352

353353
assertThat(stepDefinition.isDisposed(), is(true));
@@ -371,15 +371,15 @@ void returns_no_match_after_evicting_scenario_scoped() throws AmbiguousStepDefin
371371

372372
StepDefinition stepDefinition1 = new MockedScenarioScopedStepDefinition("^pattern1");
373373
glue.addStepDefinition(stepDefinition1);
374-
glue.prepareGlue(stepTypeRegistry);
374+
glue.prepareGlue(LANGUAGE);
375375

376376
PickleStepDefinitionMatch pickleStepDefinitionMatch = glue.stepDefinitionMatch(uri, pickleStep1);
377377
assertThat(((CoreStepDefinition) pickleStepDefinitionMatch.getStepDefinition()).getStepDefinition(),
378378
is(equalTo(stepDefinition1)));
379379

380380
glue.removeScenarioScopedGlue();
381381

382-
glue.prepareGlue(stepTypeRegistry);
382+
glue.prepareGlue(LANGUAGE);
383383

384384
PickleStepDefinitionMatch pickleStepDefinitionMatch2 = glue.stepDefinitionMatch(uri, pickleStep1);
385385
assertThat(pickleStepDefinitionMatch2, nullValue());
@@ -393,7 +393,7 @@ void throws_ambiguous_steps_def_exception_when_many_patterns_match() {
393393
glue.addStepDefinition(stepDefinition1);
394394
glue.addStepDefinition(stepDefinition2);
395395
glue.addStepDefinition(stepDefinition3);
396-
glue.prepareGlue(stepTypeRegistry);
396+
glue.prepareGlue(LANGUAGE);
397397

398398
URI uri = URI.create("file:path/to.feature");
399399

@@ -480,7 +480,7 @@ void emits_hook_messages_to_bus() {
480480
glue.addBeforeStepHook(new MockedScenarioScopedHookDefinition());
481481
glue.addAfterStepHook(new MockedScenarioScopedHookDefinition());
482482

483-
glue.prepareGlue(stepTypeRegistry);
483+
glue.prepareGlue(LANGUAGE);
484484
assertThat(events.size(), is(4));
485485
}
486486

@@ -497,7 +497,7 @@ void parameterTypeDefinition_without_source_reference_emits_parameterType_with_e
497497
glue.addParameterType(new MockedParameterTypeDefinition());
498498

499499
// When
500-
glue.prepareGlue(stepTypeRegistry);
500+
glue.prepareGlue(LANGUAGE);
501501

502502
// Then
503503
assertThat(events.size(), is(1));
@@ -519,7 +519,7 @@ void parameterTypeDefinition_with_source_reference_emits_parameterType_with_non_
519519
glue.addParameterType(new MockedParameterTypeDefinitionWithSourceReference());
520520

521521
// When
522-
glue.prepareGlue(stepTypeRegistry);
522+
glue.prepareGlue(LANGUAGE);
523523

524524
// Then
525525
assertThat(events.size(), is(1));

0 commit comments

Comments
 (0)