This is an automated email from the ASF dual-hosted git repository. Cole-Greer pushed a commit to branch docs-3.7 in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit a6b6534cc14f0d636380b9afd74f3bd8365df423 Author: Cole Greer <[email protected]> AuthorDate: Fri May 29 12:33:56 2026 -0700 Fix graph state leak between blocks and add sugar plugin support Two fixes for no-result blocks: 1. initGraphIfNeeded now re-initializes graph + g for EVERY non-existing block, matching the old preprocessor behavior. Previously it skipped re-init when the graph name matched the prior block, so a block that reassigned g (e.g. 'g = traversal().withEmbedded(graph).withComputer()') leaked the OLAP/mutated source into later blocks that expected a fresh OLTP 'g' (e.g. path().by() blocks returned nothing under GraphComputer). 2. Detect sugar syntax (g.V, g.V[0..2], etc.) and call SugarLoader.load() on a fresh console for those blocks, restarting afterward so the permanent Groovy metaclass mutation doesn't leak into other blocks. --- .../gremlin/docs/GremlinTreeprocessor.java | 55 +++++++++++++++++----- .../gremlin/docs/GremlinTreeprocessorTest.java | 22 ++++++++- 2 files changed, 62 insertions(+), 15 deletions(-) diff --git a/tools/tinkerpop-docs/src/main/java/org/apache/tinkerpop/gremlin/docs/GremlinTreeprocessor.java b/tools/tinkerpop-docs/src/main/java/org/apache/tinkerpop/gremlin/docs/GremlinTreeprocessor.java index 26531e47ca..41e5c238b0 100644 --- a/tools/tinkerpop-docs/src/main/java/org/apache/tinkerpop/gremlin/docs/GremlinTreeprocessor.java +++ b/tools/tinkerpop-docs/src/main/java/org/apache/tinkerpop/gremlin/docs/GremlinTreeprocessor.java @@ -116,6 +116,7 @@ public class GremlinTreeprocessor extends Treeprocessor { private GremlinConsole lazyConsole; private Path consoleHomePath; private boolean dryRun; + private boolean sugarLoaded; @Override public Document process(final Document document) { @@ -463,20 +464,38 @@ public class GremlinTreeprocessor extends Treeprocessor { if (!dryRun) { ensureConsoleStarted(); } - if (!dryRun && getActiveExecutor() != null) { - final String graphName = extractGraphName(block); - initGraphIfNeeded(graphName); - } final String source = block.getSource(); if (source == null || source.isEmpty()) { return ""; } - final StringBuilder output = new StringBuilder(); final String[] lines = source.split("\\r?\\n"); final List<String> displayStatements = buildDisplayStatements(lines); final List<String> execStatements = buildStatements(lines); + + // Sugar syntax permanently mutates the Groovy metaclass, so run sugar + // blocks on a fresh console and mark it dirty to force a restart after. + final boolean needsSugar = !dryRun && execStatements.stream().anyMatch(GremlinTreeprocessor::usesSugarSyntax); + if (needsSugar && getActiveExecutor() != null) { + if (sugarLoaded) { + restartConsole(); + } + final String graphName = extractGraphName(block); + initGraphIfNeeded(graphName); + executeSafely("org.apache.tinkerpop.gremlin.groovy.loaders.SugarLoader.load()"); + sugarLoaded = true; + } else if (!dryRun && getActiveExecutor() != null) { + if (sugarLoaded) { + // Previous block loaded sugar; restart to get a clean metaclass. + restartConsole(); + sugarLoaded = false; + } + final String graphName = extractGraphName(block); + initGraphIfNeeded(graphName); + } + + final StringBuilder output = new StringBuilder(); for (int s = 0; s < displayStatements.size(); s++) { // Show original lines with callouts for display final String[] displayLines = displayStatements.get(s).split("\\r?\\n"); @@ -572,6 +591,20 @@ public class GremlinTreeprocessor extends Treeprocessor { return line.replaceAll("\\s*((<\\d+>\\s*)*<\\d+>)\\s*$", ""); } + /** + * Detects sugar-syntax usage that requires SugarLoader: bare step properties + * (e.g. {@code g.V}, {@code g.V.name}) or range operators (e.g. {@code g.V[0..2]}). + */ + static boolean usesSugarSyntax(final String statement) { + if (statement == null) return false; + // Range access: g.V[...] or g.E[...] + if (statement.matches(".*\\bg\\.[VE]\\s*\\[.*")) return true; + // Bare step property: g.V or g.E not immediately followed by '(' + final java.util.regex.Matcher m = java.util.regex.Pattern + .compile("\\bg\\.[VE]([^(\\w]|$)").matcher(statement); + return m.find(); + } + /** * Extracts the graph name from the second positional attribute of the block. */ @@ -585,20 +618,16 @@ public class GremlinTreeprocessor extends Treeprocessor { } /** - * Initializes the graph in the console if the graph name has changed. + * Initializes the graph and traversal source for the block. Re-initializes for + * every non-{@code existing} block because prior blocks may have mutated + * {@code graph} or reassigned {@code g} (e.g. {@code g = ...withComputer()}), + * which would otherwise leak into subsequent blocks that expect a fresh source. */ private void initGraphIfNeeded(final String graphName) { if (EXISTING.equals(graphName)) { return; } - if (graphName != null && graphName.equals(currentGraph)) { - return; - } - if (graphName == null && currentGraph == null && gremlinBlockCount > 1) { - return; - } - final String initStatement; if (graphName == null) { initStatement = "graph = TinkerGraph.open()"; diff --git a/tools/tinkerpop-docs/src/test/java/org/apache/tinkerpop/gremlin/docs/GremlinTreeprocessorTest.java b/tools/tinkerpop-docs/src/test/java/org/apache/tinkerpop/gremlin/docs/GremlinTreeprocessorTest.java index 40cd41655e..55c93a7e62 100644 --- a/tools/tinkerpop-docs/src/test/java/org/apache/tinkerpop/gremlin/docs/GremlinTreeprocessorTest.java +++ b/tools/tinkerpop-docs/src/test/java/org/apache/tinkerpop/gremlin/docs/GremlinTreeprocessorTest.java @@ -38,6 +38,22 @@ import static org.hamcrest.MatcherAssert.assertThat; */ public class GremlinTreeprocessorTest { + @Test + public void shouldDetectSugarSyntax() { + assertThat(GremlinTreeprocessor.usesSugarSyntax("g.V"), is(true)); + assertThat(GremlinTreeprocessor.usesSugarSyntax("g.V.name"), is(true)); + assertThat(GremlinTreeprocessor.usesSugarSyntax("g.V.outE.weight"), is(true)); + assertThat(GremlinTreeprocessor.usesSugarSyntax("g.V[0..2]"), is(true)); + assertThat(GremlinTreeprocessor.usesSugarSyntax("g.V[0..<2]"), is(true)); + assertThat(GremlinTreeprocessor.usesSugarSyntax("g.E"), is(true)); + // Normal (non-sugar) statements + assertThat(GremlinTreeprocessor.usesSugarSyntax("g.V()"), is(false)); + assertThat(GremlinTreeprocessor.usesSugarSyntax("g.V().values('name')"), is(false)); + assertThat(GremlinTreeprocessor.usesSugarSyntax("g.V(1).out()"), is(false)); + assertThat(GremlinTreeprocessor.usesSugarSyntax("g.E().count()"), is(false)); + assertThat(GremlinTreeprocessor.usesSugarSyntax("graph = TinkerFactory.createModern()"), is(false)); + } + @Test public void shouldExecuteModernGraphBlock() { final RecordingExecutor executor = new RecordingExecutor("==>v[1]"); @@ -94,7 +110,9 @@ public class GremlinTreeprocessorTest { } @Test - public void shouldSkipGraphInitForConsecutiveSameGraph() { + public void shouldReInitGraphForEachBlock() { + // Each non-'existing' block must re-init graph and g, because a prior + // block may have mutated graph or reassigned g (e.g. withComputer()). final RecordingExecutor executor = new RecordingExecutor("==>result"); final GremlinTreeprocessor processor = new GremlinTreeprocessor(executor); @@ -109,7 +127,7 @@ public class GremlinTreeprocessorTest { assertThat(processor.getGremlinBlockCount(), is(2)); final long modernInitCount = executor.statements.stream() .filter(s -> s.equals("graph = TinkerFactory.createModern()")).count(); - assertThat(modernInitCount, is(1L)); + assertThat(modernInitCount, is(2L)); } }
