diff --git a/README.md b/README.md index e698a00..d07d7e2 100644 --- a/README.md +++ b/README.md @@ -15,7 +15,7 @@ Add the dependency: ai.singlr scim-sql - 1.0.0 + 1.2.0 ``` @@ -26,16 +26,22 @@ var engine = new ScimEngine(); var filter = engine.parseFilter("userName eq \"john\"", "p", null); filter.toClause(); -// → "user_name = :p_userName1" +// → "p.user_name = :userName1" filter.context().indexedParams(); -// → {p_userName1=john} +// → {userName1=john} ``` -The `prefix` parameter namespaces all generated parameter keys, making it safe to combine multiple parsed filters in a single query. +The `prefix` is the table alias put in front of every attribute that does not name its own. Pass an empty string for no alias: `user_name = :userName1`. + +Parameter keys are the attribute name plus a counter (`userName1`, `userName2`). They are unique within one parsed filter and are not prefixed, so two separately parsed filters can produce the same key. To use two filters in one query, combine them into one expression first (see [Scoping a Client Filter](#scoping-a-client-filter)) and parse that. + +Call `toClause()` before reading `indexedParams()`. The parameters are collected while the clause is rendered. ## Supported Operators +The examples use an empty prefix. + | SCIM Operator | SQL Output | Example | |--------------|------------|---------| | `eq` | `=` | `name eq "John"` → `name = :name1` | @@ -50,19 +56,21 @@ The `prefix` parameter namespaces all generated parameter keys, making it safe t | `pr` | `IS NOT NULL` | `name pr` → `name IS NOT NULL` | | `in` | `IN (…)` | `status in ["active", "pending"]` → `status IN (:status1, :status2)` | +Values are strings in double quotes, whole numbers, decimals, `true`, `false` and `null`. + ## Logical Operators Combine filters with `and`, `or`, `not`, and parentheses: ```java engine.parseFilter("name eq \"John\" and age gt 21", "p", null).toClause(); -// → "name = :p_name1 AND age > :p_age1" +// → "p.name = :name1 AND p.age > :age1" engine.parseFilter("not (active eq true)", "p", null).toClause(); -// → "NOT (active = :p_active1)" +// → "NOT (p.active = :active1)" engine.parseFilter("(a eq 1 or b eq 2) and c eq 3", "p", null).toClause(); -// → "(a = :p_a1 OR b = :p_b1) AND c = :p_c1" +// → "(p.a = :a1 OR p.b = :b1) AND p.c = :c1" ``` `and` binds tighter than `or`, as in SQL. Use parentheses to group. @@ -87,7 +95,6 @@ A filter is also rejected with `FilterSyntaxException` when it exceeds one of th |-------|-------| | Levels of nested parentheses | 50 | | Logical operators (`and`, `or`) | 500 | -| Segments in one attribute path | 10 | A list of values (`in [...]`) is not limited. A whole number that does not fit in a `long` is rejected the same way. @@ -112,6 +119,8 @@ The result is itself a filter expression, so it fits wherever a single filter is Do not join filter text by hand. `scope + " and " + clientFilter` renders as `owner AND status OR status`, and a client filter containing `or` then matches rows outside the scope. +The allowlist sees the scope's attributes too. Either check the client filter against its allowlist before scoping it, or include the scope's attributes when checking the scoped filter. A client that names a scope attribute can only narrow the result within the scope. + ## Typed Value Prefixes Values can carry type hints that produce SQL `CAST` expressions. Prefix the value inside the quotes: @@ -120,16 +129,21 @@ Values can carry type hints that produce SQL `CAST` expressions. Prefix the valu |--------|------|----------|---------------| | `#` | UUID | `CAST(… AS UUID)` | `"#550e8400-e29b-41d4-a716-446655440000"` | | `@` | Timestamp | `CAST(… AS timestamptz)` | `"@2026-01-15T10:30:00Z"` | -| `$` | JSON | `CAST(… AS jsonb)` | `"${"key":"value"}"` | +| `$` | JSON | `CAST(… AS jsonb)` | `"${\"key\":\"value\"}"` | ```java engine.parseFilter("id eq \"#550e8400-e29b-41d4-a716-446655440000\"", "p", null).toClause(); -// → "id = CAST(:p_id1 AS UUID)" +// → "p.id = CAST(:id1 AS UUID)" -engine.parseFilter("metadata eq \"${\"role\":\"admin\"}\"", "p", null).toClause(); -// → "metadata @> CAST(:p_metadata1 AS jsonb)" +engine.parseFilter("createdAt gt \"@2026-01-15T10:30:00Z\"", "p", null).toClause(); +// → "p.created_at > CAST(:createdAt1 AS timestamptz)" + +engine.parseFilter("metadata eq \"${\\\"role\\\":\\\"admin\\\"}\"", "p", null).toClause(); +// → "p.metadata @> CAST(:metadata1 AS jsonb)" ``` +Quotes inside a JSON value are escaped with a backslash in the filter text: `metadata eq "${\"role\":\"admin\"}"`. + JSON equality uses PostgreSQL's `@>` (contains) operator instead of `=`. ## Attribute Name Conversion @@ -139,11 +153,18 @@ CamelCase attribute names are automatically converted to snake_case column names - `userName` → `user_name` - `createdAtUtc` → `created_at_utc` -Dotted paths (e.g. `name.familyName`) are preserved as-is in the clause and flattened to underscores in parameter keys. +An attribute can name its own table alias, which replaces the prefix for that attribute: + +```java +engine.parseFilter("u.userName eq \"john\" and active eq true", "p", null).toClause(); +// → "u.user_name = :u_userName1 AND p.active = :active1" +``` + +The dot becomes an underscore in the parameter key. Only `alias.attribute` is supported. A deeper path such as `emails.work.value` is rejected with `FilterSyntaxException`. ## Parameter Binding -`Context` collects all parameter bindings as the filter tree is evaluated: +`Context` collects the parameter bindings while the clause is rendered: ```java var filter = engine.parseFilter("name eq \"John\" and age gt 21", "p", null); @@ -151,11 +172,16 @@ var clause = filter.toClause(); var params = filter.context().indexedParams(); // Use with JDBC named parameters, JOOQ, or any query builder: -// clause = "name = :p_name1 AND age > :p_age1" -// params = {p_name1=John, p_age1=21} +// clause = "p.name = :name1 AND p.age > :age1" +// params = {name1=John, age1=21} ``` -Use `context().isValid(Set.of("name", "age"))` to allowlist which attributes callers are permitted to filter on. +Use `context().isValid(Set.of("name", "age"))` to allowlist which attributes callers are permitted to filter on. Every attribute the filter uses must be listed, whether it appears in a comparison, a list check or a presence check (`pr`). An attribute with its own alias is listed by its full path (`u.userName`). The answer is the same before and after `toClause()`. + +```java +engine.parseFilter("name eq \"John\" and secret pr", "p", null).context().isValid(Set.of("name")); +// → false +``` ## Custom Filter Builders @@ -165,11 +191,15 @@ Override SQL generation for specific comparisons by passing a `compareFilterBuil var filter = engine.parseFilter( "tags eq \"admin\"", "p", - cf -> new ComparisonFilter.ListFilter(cf) // wraps as ListFilter + cf -> new ComparisonFilter.ListFilter(cf) ); ``` -This lets you intercept `ComparisonFilter` instances and return a subclass with custom `toClause()` or `paramKey()` behavior. +This lets you intercept `ComparisonFilter` instances and return a subclass with custom `toClause()` or `paramKey()` behavior. `ComparisonFilter.ListFilter` is a ready-made subclass that renders like the default and marks the comparison as coming from a list query. + +## Known Limitations + +- **`eq null` and `ne null`.** They render as `= :param` and `!= :param` with a null value, which SQL never matches. Use `pr` and `not (… pr)` to test for a value. ## PostgreSQL Query Analysis @@ -182,13 +212,15 @@ var analysis = PostgresQueryAnalyzer.analyze( analysis.statementKind(); // SELECT analysis.statementCount(); // 1 analysis.relations(); // [RelationReference[schema=null, name=users, alias=u, kind=PHYSICAL]] -analysis.columns(); // [u.id, u.created_at, u.id] -analysis.functions(); // [count at 1:17] +analysis.columns(); // [ColumnReference[qualifier=u, name=id], ColumnReference[qualifier=u, name=created_at], …] +analysis.functions(); // [FunctionReference[schema=null, name=count, line=1, column=13]] analysis.parameters(); // [start_at] analysis.features(); // [] -analysis.normalizedSql(); // deterministic form for hashing/audit +analysis.normalizedSql(); // "select u . id , count ( * ) from users u where u . created_at >= :start_at group by u . id" ``` +`normalizedSql()` is a deterministic form for hashing and audit, not SQL meant to be read. + Key properties: - **Named parameters** (`:start_at`, `:user_id`) are first-class expression values via a deliberate, documented grammar extension. Names are reported exactly; values are never bound or substituted. diff --git a/RELEASE.md b/RELEASE.md index 06aca29..b1883ac 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -99,10 +99,12 @@ git checkout -b release/1.0.0 ### 3. Set the release version +Set the same version in the dependency snippet in `README.md` first, so the README never names an older release. + ```bash mvn versions:set -DnewVersion=1.0.0 mvn versions:commit -git add pom.xml +git add pom.xml README.md git commit -m "Release 1.0.0" ``` diff --git a/src/main/java/ai/singlr/scimsql/BoundedScimLexer.java b/src/main/java/ai/singlr/scimsql/BoundedScimLexer.java index 8389bee..1b24709 100644 --- a/src/main/java/ai/singlr/scimsql/BoundedScimLexer.java +++ b/src/main/java/ai/singlr/scimsql/BoundedScimLexer.java @@ -9,19 +9,21 @@ import org.antlr.v4.runtime.Token; /** - * A lexer that rejects a filter before it grows past what the parser and the filter tree can - * handle. + * A lexer that rejects a filter the parser and the evaluator cannot handle faithfully. * - *

Parsing and evaluating a filter recurses once per level of parentheses, once per logical - * operator and once per attribute path segment. Without a bound, a filter built for the purpose - * exhausts the thread's stack. The limits are far above any filter written for a real query. A list - * of values ({@code in [...]}) does not recurse and is not limited. + *

Parsing and evaluating a filter recurses once per level of parentheses and once per logical + * operator. Without a bound, a filter built for the purpose exhausts the thread's stack. The limits + * are far above any filter written for a real query. A list of values ({@code in [...]}) does not + * recurse and is not limited. + * + *

An attribute path is {@code alias.attribute} at most. That is all the evaluator uses, and a + * deeper path would otherwise be cut short without notice. */ final class BoundedScimLexer extends ScimLexer { static final int MAX_NESTING_DEPTH = 50; static final int MAX_LOGICAL_OPERATORS = 500; - static final int MAX_PATH_SEGMENTS = 10; + static final int MAX_PATH_SEGMENTS = 2; private int nestingDepth; private int logicalOperators; @@ -48,7 +50,7 @@ public Token nextToken() { } case DOT -> { if (++pathSegments > MAX_PATH_SEGMENTS) { - reject(token, "attribute path longer than " + MAX_PATH_SEGMENTS + " segments"); + reject(token, "attribute path deeper than alias.attribute"); } } case ATTRNAME -> {} diff --git a/src/main/java/ai/singlr/scimsql/Context.java b/src/main/java/ai/singlr/scimsql/Context.java index efeceed..2d117d8 100644 --- a/src/main/java/ai/singlr/scimsql/Context.java +++ b/src/main/java/ai/singlr/scimsql/Context.java @@ -7,6 +7,7 @@ import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -15,13 +16,26 @@ public class Context { private final Map> params; private final Map indexedParams; + private final Set attributes; + private final Map keyCounts; public Context() { this.params = new HashMap<>(); this.indexedParams = new HashMap<>(); + this.attributes = new HashSet<>(); + this.keyCounts = new HashMap<>(); + } + + /** + * Records that the filter uses an attribute, so the allowlist sees it before the clause is + * rendered and whether or not it binds a parameter. + */ + void reference(Filter attribute) { + attributes.add(attribute.toString()); } public String process(Filter attribute, Filter value, Function keyMapper) { + reference(attribute); var rawValue = value instanceof ValueFilter ? ((ValueFilter) value).value() : null; var key = attribute.toString(); @@ -29,7 +43,7 @@ public String process(Filter attribute, Filter value, Function k List values = params.computeIfAbsent(key, k -> new ArrayList<>()); values.add(rawValue); - var indexedKey = key.replace(".", "_") + values.size(); + var indexedKey = nextIndexedKey(key); indexedParams.put(indexedKey, rawValue); return keyMapper.apply(indexedKey); @@ -37,6 +51,7 @@ public String process(Filter attribute, Filter value, Function k public String processArray( Filter attribute, List valueFilters, Function keyMapper) { + reference(attribute); var key = attribute.toString(); List values = params.computeIfAbsent(key, k -> new ArrayList<>()); var builder = new StringBuilder(); @@ -53,7 +68,7 @@ public String processArray( values.add(rawValue); - var indexedKey = key.replace(".", "_") + values.size(); + var indexedKey = nextIndexedKey(key); indexedParams.put(indexedKey, rawValue); var paramKey = keyMapper.apply(indexedKey); @@ -70,13 +85,32 @@ public String processArray( return builder.toString(); } + /** + * A parameter key is the attribute path with its dot flattened to an underscore, plus a counter. + * The counter runs per flattened name, so {@code u.name} and {@code u_name} never share a key. + */ + private String nextIndexedKey(String attributeKey) { + var flattened = attributeKey.replace(".", "_"); + return flattened + keyCounts.merge(flattened, 1, Integer::sum); + } + + /** + * Checks the filter against an allowlist of attributes. + * + *

Every attribute the filter uses must be listed: those in comparisons, in list checks and in + * presence checks ({@code pr}). An attribute with its own alias is listed by its full path + * ({@code u.userName}). For a filter parsed by {@link ScimEngine} the answer does not depend on + * whether the clause has been rendered. + * + * @param validParamKeys the attributes callers may filter on + * @return {@code true} if the filter uses only listed attributes + */ public boolean isValid(Set validParamKeys) { - for (String key : params.keySet()) { - if (!validParamKeys.contains(key)) { + for (String attribute : attributes) { + if (!validParamKeys.contains(attribute)) { return false; } } - return true; } diff --git a/src/main/java/ai/singlr/scimsql/ScimEngine.java b/src/main/java/ai/singlr/scimsql/ScimEngine.java index a476d91..091ed0c 100644 --- a/src/main/java/ai/singlr/scimsql/ScimEngine.java +++ b/src/main/java/ai/singlr/scimsql/ScimEngine.java @@ -32,8 +32,8 @@ public class ScimEngine { * @param compareFilterBuilder customises how comparisons render, or {@code null} for the default * @return the parsed filter * @throws FilterSyntaxException if the expression is not one complete filter, holds a whole - * number that does not fit, or exceeds a limit: 50 levels of parentheses, 500 logical - * operators, or 10 segments in an attribute path + * number that does not fit, names an attribute path deeper than {@code alias.attribute}, or + * exceeds a limit: 50 levels of parentheses or 500 logical operators * @throws IllegalArgumentException if the expression or the prefix is missing; that is a mistake * in the calling code, never in a client's filter */ diff --git a/src/main/java/ai/singlr/scimsql/ScimEvaluator.java b/src/main/java/ai/singlr/scimsql/ScimEvaluator.java index 33bf748..503c658 100644 --- a/src/main/java/ai/singlr/scimsql/ScimEvaluator.java +++ b/src/main/java/ai/singlr/scimsql/ScimEvaluator.java @@ -58,6 +58,7 @@ public Filter visitLogicalExp(ScimParser.LogicalExpContext ctx) { @Override public Filter visitPresentExp(ScimParser.PresentExpContext ctx) { Filter attributePath = visitAttrPath(ctx.attrPath()); + context.reference(attributePath); return new PresentFilter(attributePath); } @@ -65,6 +66,7 @@ public Filter visitPresentExp(ScimParser.PresentExpContext ctx) { public Filter visitInExp(ScimParser.InExpContext ctx) { Filter attributePath = visitAttrPath(ctx.attrPath()); Filter values = visitArrayValue(ctx.arrayValue()); + context.reference(attributePath); return new InFilter(attributePath, (ArrayValueFilter) values, context); } @@ -74,8 +76,10 @@ public Filter visitCompareExp(ScimParser.CompareExpContext ctx) { String operator = ctx.op.getText(); Filter value = visit(ctx.value()); - return compareFilterBuilder.apply( - new ComparisonFilter(attributePath, operator, value, context)); + var comparison = + compareFilterBuilder.apply(new ComparisonFilter(attributePath, operator, value, context)); + context.reference(comparison.attribute()); + return comparison; } @Override diff --git a/src/test/java/ai/singlr/scimsql/ContextTest.java b/src/test/java/ai/singlr/scimsql/ContextTest.java index a7bc9e6..73d5178 100644 --- a/src/test/java/ai/singlr/scimsql/ContextTest.java +++ b/src/test/java/ai/singlr/scimsql/ContextTest.java @@ -144,4 +144,46 @@ void shouldReturnTrueForEmptyParams() { var context = new Context(); assertTrue(context.isValid(Set.of())); } + + @Test + @DisplayName("isValid sees an attribute processed as a list") + void shouldSeeAnAttributeProcessedAsAList() { + var context = new Context(); + var attr = new AttributeFilter("id", null, "t", context); + context.processArray(attr, List.of(new ValueFilter(1L, context)), k -> ":" + k); + + assertFalse(context.isValid(Set.of("name"))); + assertTrue(context.isValid(Set.of("id"))); + } + + @Test + @DisplayName("isValid sees a recorded attribute that binds no parameter") + void shouldSeeARecordedAttributeThatBindsNoParameter() { + var context = new Context(); + var sub = new AttributeFilter("userName", null, "", context); + context.reference(new AttributeFilter("u", sub, "t", context)); + + assertFalse(context.isValid(Set.of("userName"))); + assertTrue(context.isValid(Set.of("u.userName"))); + assertTrue(context.indexedParams().isEmpty()); + } + + @Test + @DisplayName("Building a filter by hand records nothing") + void shouldRecordNothingWhenAFilterIsOnlyBuilt() { + var context = new Context(); + var attr = new AttributeFilter("scratch", null, "t", context); + new PresentFilter(attr); + new ComparisonFilter(attr, "eq", new ValueFilter("x", context), context); + + assertTrue(context.isValid(Set.of())); + } + + @Test + @DisplayName("A presence check needs no context to render") + void shouldRenderAPresenceCheckWithoutAContext() { + var filter = new PresentFilter(new AttributeFilter("a", null, "t", null)); + + assertEquals("t.a IS NOT NULL", filter.toClause()); + } } diff --git a/src/test/java/ai/singlr/scimsql/ScimEngineTest.java b/src/test/java/ai/singlr/scimsql/ScimEngineTest.java index 16b00e4..2ad636c 100644 --- a/src/test/java/ai/singlr/scimsql/ScimEngineTest.java +++ b/src/test/java/ai/singlr/scimsql/ScimEngineTest.java @@ -6,13 +6,16 @@ package ai.singlr.scimsql; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import java.util.List; +import java.util.Map; import java.util.Random; +import java.util.Set; import org.antlr.v4.runtime.CharStreams; import org.antlr.v4.runtime.CommonTokenStream; import org.junit.jupiter.api.BeforeEach; @@ -157,7 +160,7 @@ void shouldHandleInOperator() { void shouldHandleComplexNestedOperators() { Filter result = engine.parseFilter( - "emails.work.value co \"@example.com\" and (age gt 25 or active eq true)", "t", null); + "emails.value co \"@example.com\" and (age gt 25 or active eq true)", "t", null); result.toClause(); var map = result.context().params(); assertEquals(3, map.size()); @@ -832,27 +835,47 @@ void shouldRejectLogicalOperatorsPastTheLimit() { assertTrue(ex.getMessage().endsWith("more than 500 logical operators"), ex.getMessage()); } - @Test - @DisplayName("Attribute path up to the limit") - void shouldAcceptAnAttributePathUpToTheLimit() { - var filter = "a" + ".b".repeat(9) + " pr"; - assertNotNull(engine.parseFilter(filter, "t", null).toClause()); + @ParameterizedTest(name = "rejects [{0}]") + @ValueSource( + strings = { + "emails.work.value co \"x\"", + "a.b.c pr", + "a.b.c.d eq 1", + "u.name.x in [1, 2]", + "name pr and a.b.c pr", + "a..b pr", + "a. pr", + "a.b. pr" + }) + @DisplayName("An attribute path deeper than alias.attribute") + void shouldRejectAnAttributePathDeeperThanAliasAndAttribute(String filter) { + assertThrows(FilterSyntaxException.class, () -> engine.parseFilter(filter, "t", null)); } @Test - @DisplayName("Attribute path past the limit") - void shouldRejectAnAttributePathPastTheLimit() { - var filter = "a" + ".b".repeat(10) + " pr"; + @DisplayName("The error points at the segment that is not supported") + void shouldPointAtTheUnsupportedPathSegment() { + var filter = "emails.work.value co \"x\""; var ex = assertThrows(FilterSyntaxException.class, () -> engine.parseFilter(filter, "t", null)); - assertTrue( - ex.getMessage().endsWith("attribute path longer than 10 segments"), ex.getMessage()); + assertEquals( + "Failed to parse filter: Invalid filter syntax at position " + + "emails.work".length() + + ": attribute path deeper than alias.attribute", + ex.getMessage()); + } + + @Test + @DisplayName("An aliased attribute is kept whole") + void shouldKeepAnAliasedAttributeWhole() { + Filter result = engine.parseFilter("u.userName eq \"john\" and active pr", "t", null); + assertEquals("u.user_name = :u_userName1 AND t.active IS NOT NULL", result.toClause()); } @Test - @DisplayName("Attribute paths do not add up across the filter") - void shouldCountPathSegmentsPerAttribute() { - var filter = "a.b.c pr" + " and a.b.c pr".repeat(50); + @DisplayName("Aliased attributes do not add up across the filter") + void shouldAcceptManyAliasedAttributes() { + var filter = "a.b pr" + " and a.b pr".repeat(50); assertNotNull(engine.parseFilter(filter, "t", null).toClause()); } @@ -979,6 +1002,173 @@ void shouldRenderMixedOperatorsWithSqlPrecedence() { } } + @Nested + @DisplayName("Parameter Keys") + class ParameterKeyTests { + + @Test + @DisplayName("An aliased attribute and an underscore attribute keep separate parameters") + void shouldKeepSeparateParametersForNamesThatFlattenAlike() { + Filter result = engine.parseFilter("u.name eq \"alice\" and u_name eq \"bob\"", "t", null); + + assertEquals("u.name = :u_name1 AND t.u_name = :u_name2", result.toClause()); + assertEquals(Map.of("u_name1", "alice", "u_name2", "bob"), result.context().indexedParams()); + } + + @Test + @DisplayName("List checks on names that flatten alike keep separate parameters") + void shouldKeepSeparateListParametersForNamesThatFlattenAlike() { + Filter result = engine.parseFilter("u.id in [1, 2] and u_id in [3]", "t", null); + + assertEquals("u.id IN (:u_id1, :u_id2) AND t.u_id IN (:u_id3)", result.toClause()); + assertEquals(Map.of("u_id1", 1L, "u_id2", 2L, "u_id3", 3L), result.context().indexedParams()); + } + + @Test + @DisplayName( + "A comparison and a list check on names that flatten alike keep separate parameters") + void shouldKeepSeparateParametersAcrossComparisonsAndListChecks() { + Filter result = engine.parseFilter("u_id eq 9 and u.id in [1, 2]", "t", null); + + assertEquals("t.u_id = :u_id1 AND u.id IN (:u_id2, :u_id3)", result.toClause()); + assertEquals(Map.of("u_id1", 9L, "u_id2", 1L, "u_id3", 2L), result.context().indexedParams()); + } + + @Test + @DisplayName("Repeated use of one attribute still counts from one") + void shouldCountRepeatedUseOfOneAttributeFromOne() { + Filter result = engine.parseFilter("name eq \"a\" or name eq \"b\" or age gt 1", "t", null); + + assertEquals("t.name = :name1 OR t.name = :name2 OR t.age > :age1", result.toClause()); + assertEquals( + Map.of("name", List.of("a", "b"), "age", List.of(1L)), result.context().params()); + } + } + + @Nested + @DisplayName("Allowlist") + class AllowlistTests { + + @Test + @DisplayName("A presence check on an attribute outside the allowlist is invalid") + void shouldRejectAPresenceCheckOutsideTheAllowlist() { + Filter result = engine.parseFilter("secret pr", "t", null); + assertFalse(result.context().isValid(Set.of("name"))); + + result.toClause(); + assertFalse(result.context().isValid(Set.of("name"))); + assertTrue(result.context().isValid(Set.of("secret"))); + } + + @Test + @DisplayName("A presence check hidden among allowed comparisons is invalid") + void shouldRejectAPresenceCheckAmongAllowedComparisons() { + Filter result = engine.parseFilter("name eq \"John\" and secret pr and age gt 21", "t", null); + result.toClause(); + + assertFalse(result.context().isValid(Set.of("name", "age"))); + assertTrue(result.context().isValid(Set.of("name", "age", "secret"))); + } + + @ParameterizedTest(name = "is invalid [{0}]") + @ValueSource( + strings = { + "not (secret pr)", + "name eq \"John\" or (age gt 21 and (secret pr))", + "not (name eq \"John\" and not (secret pr))" + }) + @DisplayName("A presence check is seen wherever it is nested") + void shouldSeeAPresenceCheckWhereverItIsNested(String filter) { + Filter result = engine.parseFilter(filter, "t", null); + assertFalse(result.context().isValid(Set.of("name", "age"))); + } + + @Test + @DisplayName("The allowlist applies before the clause is rendered") + void shouldApplyTheAllowlistBeforeRendering() { + Filter result = engine.parseFilter("name eq \"John\" and age gt 21", "t", null); + + assertFalse(result.context().isValid(Set.of("name"))); + assertTrue(result.context().isValid(Set.of("name", "age"))); + } + + @Test + @DisplayName("The allowlist applies to a list check before the clause is rendered") + void shouldApplyTheAllowlistToAListCheckBeforeRendering() { + Filter result = engine.parseFilter("id in [1, 2]", "t", null); + + assertFalse(result.context().isValid(Set.of("name"))); + assertTrue(result.context().isValid(Set.of("id"))); + } + + @Test + @DisplayName("An aliased attribute is checked by its full path") + void shouldCheckAnAliasedAttributeByItsFullPath() { + Filter result = engine.parseFilter("u.userName pr and u.email eq \"a@b.c\"", "t", null); + + assertFalse(result.context().isValid(Set.of("userName", "email"))); + assertTrue(result.context().isValid(Set.of("u.userName", "u.email"))); + } + + @Test + @DisplayName("A custom comparison builder does not hide the attribute") + void shouldSeeTheAttributeThroughACustomComparisonBuilder() { + Filter result = + engine.parseFilter("tags eq \"admin\"", "t", ComparisonFilter.ListFilter::new); + + assertFalse(result.context().isValid(Set.of("name"))); + assertTrue(result.context().isValid(Set.of("tags"))); + } + + @Test + @DisplayName("Rendering does not change the verdict") + void shouldGiveTheSameVerdictBeforeAndAfterRendering() { + Filter result = + engine.parseFilter("name eq \"John\" and age gt 21 and id in [1, 2]", "t", null); + var before = result.context().isValid(Set.of("name", "age", "id")); + result.toClause(); + result.toClause(); + + assertTrue(before); + assertTrue(result.context().isValid(Set.of("name", "age", "id"))); + assertFalse(result.context().isValid(Set.of("name", "age"))); + } + + @Test + @DisplayName("A builder that renames the attribute is checked by the new name only") + void shouldCheckARenamedAttributeByItsNewName() { + Filter result = + engine.parseFilter( + "email eq \"a@b.c\"", + "t", + cf -> + new ComparisonFilter( + new AttributeFilter("emailAddress", null, "u", cf.context()), + cf.operator(), + cf.value(), + cf.context())); + + assertTrue(result.context().isValid(Set.of("emailAddress"))); + assertFalse(result.context().isValid(Set.of("email"))); + assertEquals("u.email_address = :emailAddress1", result.toClause()); + assertTrue(result.context().isValid(Set.of("emailAddress"))); + } + + @Test + @DisplayName("A client filter can be checked on its own before it is scoped") + void shouldCheckAClientFilterBeforeScoping() { + var clientFilter = "name eq \"John\""; + var clientIsValid = + engine.parseFilter(clientFilter, "t", null).context().isValid(Set.of("name")); + var scoped = engine.scopeFilter("not (deletedAt pr)", clientFilter); + + assertTrue(clientIsValid); + assertFalse(engine.parseFilter(scoped, "t", null).context().isValid(Set.of("name"))); + assertTrue( + engine.parseFilter(scoped, "t", null).context().isValid(Set.of("name", "deletedAt"))); + } + } + @Nested @DisplayName("Scope Filter") class ScopeFilterTests {