Skip to content
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

[Java] Improve expression creation performance #187

Merged
merged 6 commits into from
Dec 8, 2022
Merged
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/)
and this project adheres to [Semantic Versioning](http://semver.org/).

## [Unreleased]
### Fixed
- [Java] Improve expression creation performance ([#187](https://github.com/cucumber/cucumber-expressions/pull/187), [#189](https://github.com/cucumber/cucumber-expressions/pull/189))

## [16.1.0] - 2022-11-28
### Added
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@

@API(status = API.Status.STABLE)
public final class CucumberExpression implements Expression {
private static final Pattern ESCAPE_PATTERN = Pattern.compile("([\\\\^\\[({$.|?*+})\\]])");
private static final Pattern ESCAPE_PATTERN = Pattern.compile("[\\\\^\\[({$.|?*+})\\]]");

private final List<ParameterType<?>> parameterTypes = new ArrayList<>();
private final String source;
Expand Down Expand Up @@ -62,7 +62,7 @@ private String rewriteToRegex(Node node) {
}

private static String escapeRegex(String text) {
return ESCAPE_PATTERN.matcher(text).replaceAll("\\\\$1");
return ESCAPE_PATTERN.matcher(text).replaceAll("\\\\$0");
}

private String rewriteOptional(Node node) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,13 @@
* using heuristics. This is particularly useful for languages that don't have a
* literal syntax for regular expressions. In Java, a regular expression has to be represented as a String.
*
* A string that starts with `^` and/or ends with `$` is considered a regular expression.
* A string that starts with `^` and/or ends with `$` (or written in script style, i.e. starting with `/`
* and ending with `/`) is considered a regular expression.
* Everything else is considered a Cucumber expression.
*/
@API(status = API.Status.STABLE)
public final class ExpressionFactory {

private static final Pattern BEGIN_ANCHOR = Pattern.compile("^\\^.*");
private static final Pattern END_ANCHOR = Pattern.compile(".*\\$$");
private static final Pattern SCRIPT_STYLE_REGEXP = Pattern.compile("^/(.*)/$");
private static final Pattern PARAMETER_PATTERN = Pattern.compile("((?:\\\\){0,2})\\{([^}]*)\\}");

private final ParameterTypeRegistry parameterTypeRegistry;
Expand All @@ -29,14 +27,30 @@ public ExpressionFactory(ParameterTypeRegistry parameterTypeRegistry) {
}

public Expression createExpression(String expressionString) {
if (BEGIN_ANCHOR.matcher(expressionString).find() || END_ANCHOR.matcher(expressionString).find()) {
return createRegularExpressionWithAnchors(expressionString);
/* This method is called often (typically about number_of_steps x
* nbr_test_scenarios), thus performance is more important than
* readability here.
* Consequently, we check the first and last expressionString
* characters to determine whether we need to create a
* RegularExpression or a CucumberExpression (because character
* matching is faster than startsWith/endsWith and regexp matching).
*/
int length = expressionString.length();
if (length == 0) {
return new CucumberExpression(expressionString, this.parameterTypeRegistry);
}
Matcher m = SCRIPT_STYLE_REGEXP.matcher(expressionString);
if (m.find()) {
return new RegularExpression(Pattern.compile(m.group(1)), parameterTypeRegistry);

int lastCharIndex = length - 1;
char firstChar = expressionString.charAt(0);
char lastChar = expressionString.charAt(lastCharIndex);

if (firstChar == '^' || lastChar == '$') {
return this.createRegularExpressionWithAnchors(expressionString);
} else if (firstChar == '/' && lastChar == '/') {
jkronegg marked this conversation as resolved.
Show resolved Hide resolved
return new RegularExpression(Pattern.compile(expressionString.substring(1, lastCharIndex)), this.parameterTypeRegistry);
}
return new CucumberExpression(expressionString, parameterTypeRegistry);

return new CucumberExpression(expressionString, this.parameterTypeRegistry);
}

private RegularExpression createRegularExpressionWithAnchors(String expressionString) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,12 @@
import static org.junit.jupiter.api.Assertions.assertThrows;

public class ExpressionFactoryTest {


@Test
public void creates_cucumber_expression_for_empty() {
assertCucumberExpression("");
}

@Test
public void creates_cucumber_expression_by_default() {
assertCucumberExpression("strings are cukexp by default");
Expand Down