Skip to content
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
62 changes: 62 additions & 0 deletions docs/next-engine-ast-boundary.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
# Engine / AST dependency boundary

The target is an execution engine independent of parser and SPARQL AST types.
The current engine still exposes three dependencies. `NextModuleBoundaryTest`
inventories them as exact source-path / referenced-type pairs, rather than
exempting whole files or packages.

## Existing exceptions

Paths below are relative to `next/query/impl/engine`.

| Source | AST type | Current use and callers |
| --- | --- | --- |
| `model/Filter.java` | `TermAst` | Return types of `getFilterExpression()` and `coreseNextSource()`. `NextFilterFromAst` implements both in the bridge. `Exp` delegates the first; `AstBackedExprTest` checks its result. No production caller of `coreseNextSource()` was found in `next`. |
| `pattern/Exp.java` | `TermAst` | Return type of `getFilterExpression()`, delegating to `Filter`. No production caller of this accessor was found in `next`. |
| `pattern/Query.java` | `QueryAst` | Field `ast` and accessors `getAST()`, `getGlobalAST()`, `setAST()`. `CoreseAstQueryBuilder` sets the source AST for each query form; `QueryTest` tests the accessors. No external production reader of these accessors was found in `next`. |

This is a source-reference inventory, not a statement that these APIs can be
removed without checking downstream clients. Bridge implementations may still
own AST objects internally after the engine contracts stop exposing their types.

## Guard behavior

- Parser dependencies have no exceptions.
- Both the actual `query.impl.sparql.ast` package and the historical
`query.impl.ast` package are checked.
- Imports, wildcard imports, static references and fully qualified references
are detected by the existing source-reference matching approach.
- Adding a type to an already listed file is rejected.
- Adding an existing AST type to another file is rejected.
- Removing a dependency requires removing its exception in the same change.

The source scanner also sees fully qualified names in comments and strings. It
does not perform bytecode or transitive dependency analysis. Its regression test
uses synthetic sources to check new files, new types, wildcard imports and fully
qualified references outside imports.

## Follow-up: remove the exceptions

1. Check repository-wide and downstream uses of the source-AST accessors before
removing them; assess compatibility of these implementation classes.
2. Remove the unused `Exp` forwarding accessor and the AST accessors from the
engine `Filter` contract. Keep source-AST access inside the bridge where needed;
adapt `AstBackedExprTest` to test bridge behavior rather than an engine AST API.
3. Remove AST retention from `Query` if no required consumer exists. If diagnostics
need source information, keep it in a bridge-owned structure with an explicit
lifecycle, rather than replacing typed accessors with `Object` or unsafe casts.
4. Remove the corresponding builder writes and accessor-only tests. Preserve
behavioral tests for query forms, expressions and subqueries.
5. Delete each exception as its dependency disappears, then restore the strict
no-AST-dependencies invariant in the engine package documentation.

Do not introduce new runtime behavior as part of this follow-up. Validate with
the architecture tests, the full core test suite and a comparison against the
existing W3C baseline.

## Validation

```bash
./gradlew test --tests '*NextModuleBoundaryTest'
./gradlew check -x test
```
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@
*
* <h2>Architectural Invariants &amp; Development Guardrails</h2>
* <ul>
* <li><b>Zero AST Leakage</b>: No direct dependencies exist from execution engine
* components ({@code eval}, {@code pattern}, {@code solution}, {@code memory})
* to ANTLR or parser AST classes. The engine executes compiled logical operators only.</li>
* <li><b>AST Boundary</b>: New dependencies on parser or AST classes are forbidden.
* Three existing source/type dependencies in {@code Filter}, {@code Exp}, and
* {@code Query} remain explicitly inventoried by {@code NextModuleBoundaryTest}.
* Removing these exceptions is tracked in {@code docs/next-engine-ast-boundary.md};
* the target architecture executes compiled logical operators only.</li>
* <li><b>Storage Isolation</b>: Query evaluation and triple access must strictly
* transit through the {@link fr.inria.corese.core.next.query.impl.engine.spi.Producer}
* and Storage SPI abstractions.</li>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package fr.inria.corese.core.next.architecture;

import static org.junit.jupiter.api.Assertions.fail;
import static org.junit.jupiter.api.Assertions.assertEquals;

import java.io.IOException;
import java.nio.file.Files;
Expand All @@ -15,6 +16,7 @@
import java.util.stream.Stream;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;

/** Protects the dependency direction between the top-level {@code next} modules. */
class NextModuleBoundaryTest {
Expand All @@ -25,6 +27,13 @@ class NextModuleBoundaryTest {
"\\bfr\\.inria\\.corese\\.core(?:\\.[A-Za-z_$][A-Za-z0-9_$]*)+");
private static final Pattern STATIC_IMPORT_PREFIX = Pattern.compile("^static\\s+");

// Exact source/type pairs only: see docs/next-engine-ast-boundary.md.
// Equality below also requires removing exceptions when their dependencies disappear.
private static final Set<String> EXISTING_ENGINE_AST_DEPENDENCIES = Set.of(
"model/Filter.java -> fr.inria.corese.core.next.query.impl.sparql.ast.TermAst",
"pattern/Exp.java -> fr.inria.corese.core.next.query.impl.sparql.ast.TermAst",
"pattern/Query.java -> fr.inria.corese.core.next.query.impl.sparql.ast.QueryAst");

@Test
void sharedCodeMustNotDependOnDomainModules() throws IOException {
assertNoImports(
Expand Down Expand Up @@ -88,12 +97,75 @@ void queryRuntimeMustNotDependOnTheLegacyPipeline() throws IOException {
}

@Test
void queryEngineMustNotDependOnParserOrAst() throws IOException {
void queryEngineMustNotAddParserOrAstDependencies() throws IOException {
Path engineSources = NEXT_SOURCES.resolve("query/impl/engine");
assertNoReferences(
engineSources,
reference -> reference.startsWith("fr.inria.corese.core.next.query.impl.sparql.parser")
|| reference.startsWith("fr.inria.corese.core.next.query.impl.ast"));
assertEquals(EXISTING_ENGINE_AST_DEPENDENCIES, engineSyntaxDependencies(engineSources),
"Unexpected engine syntax dependency, or obsolete exception: "
+ "remove dependencies rather than expanding the exception list.");
}

@Test
void engineBoundaryFindsSyntaxReferencesBeyondExistingExceptions(@TempDir Path sources)
throws IOException {
Path filter = sources.resolve("model/Filter.java");
Files.createDirectories(filter.getParent());
Files.writeString(filter, """
import fr.inria.corese.core.next.query.impl.sparql.ast.TermAst;
import fr.inria.corese.core.next.query.impl.sparql.ast.QueryAst;
import fr.inria.corese.core.next.query.impl.sparql.parser.SparqlParser;
import fr.inria.corese.core.next.query.impl.sparql.ast.*;
class Filter {
fr.inria.corese.core.next.query.impl.sparql.ast.VarAst variable;
}
""");
Files.writeString(sources.resolve("NewOperator.java"),
"import fr.inria.corese.core.next.query.impl.sparql.ast.TermAst;");
Set<String> unexpected = engineSyntaxDependencies(sources);
unexpected.removeAll(EXISTING_ENGINE_AST_DEPENDENCIES);
assertEquals(Set.of(
"model/Filter.java -> fr.inria.corese.core.next.query.impl.sparql.ast.QueryAst",
"model/Filter.java -> fr.inria.corese.core.next.query.impl.sparql.parser.SparqlParser",
"model/Filter.java -> fr.inria.corese.core.next.query.impl.sparql.ast.VarAst",
"model/Filter.java -> fr.inria.corese.core.next.query.impl.sparql.ast",
"NewOperator.java -> fr.inria.corese.core.next.query.impl.sparql.ast.TermAst"), unexpected);
}

/**
* Inventories syntax dependencies by source path and fully qualified reference.
*
* @param sources engine source root
* @return mutable set of dependency pairs, including references outside imports
* @throws IOException if sources cannot be read
*/
private static Set<String> engineSyntaxDependencies(Path sources) throws IOException {
Set<String> dependencies = new LinkedHashSet<>();
try (Stream<Path> paths = Files.walk(sources)) {
for (Path source : paths.filter(path -> path.toString().endsWith(".java")).toList()) {
Matcher matcher = CORESE_TYPE_REFERENCE.matcher(Files.readString(source));
while (matcher.find()) {
String reference = matcher.group();
if (isQuerySyntaxReference(reference)) {
dependencies.add(sources.relativize(source).toString().replace('\\', '/')
+ " -> " + reference);
}
}
}
}
return dependencies;
}

/**
* Recognizes parser and AST packages, including the historical AST location.
*
* @param reference fully qualified source reference
* @return whether the reference crosses the engine/syntax boundary
*/
private static boolean isQuerySyntaxReference(String reference) {
return List.of(
"fr.inria.corese.core.next.query.impl.sparql.parser",
"fr.inria.corese.core.next.query.impl.sparql.ast",
"fr.inria.corese.core.next.query.impl.ast").stream()
.anyMatch(prefix -> reference.equals(prefix) || reference.startsWith(prefix + "."));
}

@Test
Expand Down
Loading