From 2ca4dfab22490374d099923ceaf7a0ca6d5a7882 Mon Sep 17 00:00:00 2001 From: Clemens Portele Date: Tue, 25 Aug 2026 12:48:44 +0200 Subject: [PATCH] fix filters on a feature reference not matching any feature A queryable for a feature reference is filtered against the id property of the reference. That only worked as long as none of the properties of the reference was mapped through a sub-decoder. MappingRulesDeriver synthesizes a rule for the container of a value whose parent has no rule yet, and derived the target of that rule from the target of the value. For a sub-decoder column that is wrong: the column is not a property of the target schema, it is the container of the values that the following rules address, so its target has to be the root target. At the root that was the case by accident, because the target of a top-level value has no parent. Inside an object the container inherited the target of the object, the object was registered as a value, and it shadowed the id of the reference. The filter was then resolved against a member of the sub-decoder document named after the reference, which never exists. - MappingRule.endsWithConnector tells a sub-decoder column from a join, MappingRulesDeriver uses it for the target of the container. - SqlMappingDeriver registers only values as value targets, so an object can no longer shadow a value with the same target. - The properties of a sub-decoder column are resolved from the schema the table is mapped from instead of from the root schema, so values in the sub-decoder document of a joined table are mapped, too. Their path in the document is now always recorded, it cannot be derived from the property name, e.g. the title of a feature reference may be mapped to any member. --- .../features/sql/app/SqlMappingDeriver.java | 49 +++-- .../sql/app/FeatureRefQueryableSpec.groovy | 195 ++++++++++++++++++ .../features/domain/MappingRule.java | 13 ++ .../features/domain/MappingRulesDeriver.java | 12 +- 4 files changed, 254 insertions(+), 15 deletions(-) create mode 100644 xtraplatform-features-sql/src/test/groovy/de/ii/xtraplatform/features/sql/app/FeatureRefQueryableSpec.groovy diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlMappingDeriver.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlMappingDeriver.java index fcb0825f5..b24bf6db9 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlMappingDeriver.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlMappingDeriver.java @@ -163,6 +163,10 @@ private void addToMapping( schemas.add(querySchema); mapping.addTables(querySchema); + // the schema node that this table is mapped from; a sub-decoder column of the table holds the + // values of the properties of that node, which is the root schema only for the main table + FeatureSchema tableOwner = includeSchema ? getTableOwner(schema, tableRule) : null; + if (!columnRules.isEmpty() || !writableColumnRules.isEmpty()) { previous.add(pathParser.parseTablePath(tableRule.getSource()).getFullPath()); } @@ -189,6 +193,7 @@ private void addToMapping( addToMapping( schema, + tableOwner, mapping, seenProperties, seenWritableProperties, @@ -208,6 +213,7 @@ private void addToMapping( addToMapping( schema, + tableOwner, mapping, seenProperties, seenWritableProperties, @@ -231,6 +237,7 @@ private void addToMapping( addToMapping( schema, + tableOwner, mapping, seenProperties, seenWritableProperties, @@ -244,6 +251,7 @@ private void addToMapping( private void addToMapping( FeatureSchema schema, + FeatureSchema tableOwner, ImmutableSqlQueryMapping.Builder mapping, List seenProperties, List seenWritableProperties, @@ -255,12 +263,12 @@ private void addToMapping( if ("$".equals(column.getTarget())) { if (column1.hasOperation(SqlQueryColumn.Operation.CONNECTOR)) { List connectedSchemas = - includeSchema - ? getConnectedSchemas(schema, column1.getPathSegment(), "", false) + includeSchema && Objects.nonNull(tableOwner) + ? getConnectedSchemas(tableOwner, column1.getPathSegment(), "", false) : List.of(); for (FeatureSchema p : connectedSchemas) { - if (isWritable) { + if (isWritable && !seenWritableProperties.contains(p.getFullPathAsString())) { mapping.putWritableTables(p.getFullPathAsString(), querySchema); mapping.putWritableColumns(p.getFullPathAsString(), column1); seenWritableProperties.add(p.getFullPathAsString()); @@ -289,7 +297,7 @@ private void addToMapping( target = applyRename(column.getTarget(), propertySchema); - if (!seenProperties.contains(target)) { + if (!seenProperties.contains(target) && propertySchema.isValue()) { mapping.putValueSchemas(target, propertySchema); } @@ -309,7 +317,11 @@ private void addToMapping( mapping.putWritableColumns(target, column1); seenWritableProperties.add(target); } - if (!seenProperties.contains(target)) { + // only values can be filtered or sorted on; an object (e.g. a feature reference) must never + // claim a value target, otherwise it shadows the value that legitimately owns that target - + // the queryable of a feature reference, which is filtered against the id of the reference + if (!seenProperties.contains(target) + && (Objects.isNull(propertySchema) || propertySchema.isValue())) { mapping.putValueTables(target, querySchema); mapping.putValueColumns(target, column1); seenProperties.add(target); @@ -317,6 +329,17 @@ private void addToMapping( } } + private static FeatureSchema getTableOwner(FeatureSchema schema, MappingRule tableRule) { + if (Objects.equals(tableRule.getTarget(), ROOT_TARGET)) { + return schema; + } + + return schema.getAllNestedProperties().stream() + .filter(property -> Objects.equals(property.getFullPathAsString(), tableRule.getTarget())) + .findFirst() + .orElse(schema); + } + private static String applyRename(String target, FeatureSchema schema) { if (Objects.nonNull(schema)) { Optional rename = @@ -361,14 +384,16 @@ private List getConnectedSchemas( ? path.replace(connector + "/", "").replace(connector, "") : pathInConnector + "." + path; + // always record the path, never rely on the name fallback in + // SqlQueryMapping.getPathInConnector: the property name is only the same as the path + // in the connector by convention (e.g. a feature reference title mapped to + // "[JSON]properties/name" is named "title") FeatureSchema schema = - !inArray && pathInConnector.isEmpty() - ? p - : new ImmutableFeatureSchema.Builder() - .from(p) - .putAdditionalInfo(IN_CONNECTED_ARRAY, String.valueOf(inArray)) - .putAdditionalInfo(PATH_IN_CONNECTOR, newPathInConnector) - .build(); + new ImmutableFeatureSchema.Builder() + .from(p) + .putAdditionalInfo(IN_CONNECTED_ARRAY, String.valueOf(inArray)) + .putAdditionalInfo(PATH_IN_CONNECTOR, newPathInConnector) + .build(); if (p.isValue()) { return Stream.of(schema); diff --git a/xtraplatform-features-sql/src/test/groovy/de/ii/xtraplatform/features/sql/app/FeatureRefQueryableSpec.groovy b/xtraplatform-features-sql/src/test/groovy/de/ii/xtraplatform/features/sql/app/FeatureRefQueryableSpec.groovy new file mode 100644 index 000000000..8bbf44a37 --- /dev/null +++ b/xtraplatform-features-sql/src/test/groovy/de/ii/xtraplatform/features/sql/app/FeatureRefQueryableSpec.groovy @@ -0,0 +1,195 @@ +/* + * Copyright 2026 interactive instruments GmbH + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ +package de.ii.xtraplatform.features.sql.app + +import de.ii.xtraplatform.cql.app.CqlImpl +import de.ii.xtraplatform.cql.domain.Eq +import de.ii.xtraplatform.cql.domain.Property +import de.ii.xtraplatform.cql.domain.ScalarLiteral +import de.ii.xtraplatform.crs.domain.OgcCrs +import de.ii.xtraplatform.features.domain.ConstantsResolver +import de.ii.xtraplatform.features.domain.FeatureSchema +import de.ii.xtraplatform.features.domain.ImmutableFeatureSchema +import de.ii.xtraplatform.features.domain.MappingOperationResolver +import de.ii.xtraplatform.features.domain.MappingRulesDeriver +import de.ii.xtraplatform.features.domain.SchemaBase +import de.ii.xtraplatform.features.domain.TypesResolver +import de.ii.xtraplatform.features.domain.transform.DefaultRolesResolver +import de.ii.xtraplatform.features.domain.transform.FeatureRefResolver +import de.ii.xtraplatform.features.domain.transform.ImplicitMappingResolver +import de.ii.xtraplatform.features.domain.transform.LabelTemplateResolver +import de.ii.xtraplatform.features.json.app.DecoderFactoryJson +import de.ii.xtraplatform.features.sql.domain.ImmutableQueryGeneratorSettings +import de.ii.xtraplatform.features.sql.domain.ImmutableSqlPathDefaults +import de.ii.xtraplatform.features.sql.domain.SqlDialectPgis +import de.ii.xtraplatform.features.sql.domain.SqlPathParser +import de.ii.xtraplatform.features.sql.domain.SqlQueryMapping +import spock.lang.Shared +import spock.lang.Specification + +/** + * A queryable for a feature reference must always be filtered against the id property of the + * reference, no matter how the other properties of the reference are mapped. + */ +class FeatureRefQueryableSpec extends Specification { + + static final String REF_PATH = "[id=related_id]refs{filter=rel_inv='gehoertZuPlan'}/[base_id=id]coretable" + + static final String JOINED = "A.pk IN (SELECT AA.pk FROM coretable AA" + + " JOIN refs AB ON (AA.id=AB.related_id AND (AB.rel_inv = 'gehoertZuPlan'))" + + " JOIN coretable AC ON (AB.base_id=AC.id)" + + @Shared + SqlMappingDeriver mappingDeriver + + @Shared + FilterEncoderSql filterEncoder + + def setupSpec() { + def defaults = new ImmutableSqlPathDefaults.Builder().primaryKey("pk").sortKey("pk").build() + def cql = new CqlImpl() + def pathParser = new SqlPathParser(defaults, cql, + Map.of("JSON", new DecoderFactoryJson(), "EXPRESSION", new DecoderFactorySqlExpression())) + + mappingDeriver = new SqlMappingDeriver(pathParser, new ImmutableQueryGeneratorSettings.Builder().build()) + filterEncoder = new FilterEncoderSql(OgcCrs.CRS84, new SqlDialectPgis(), null, null, cql, null) + } + + /** + * a feature type on a JSON document column, with a feature reference over a junction table; + * the title of the reference is optionally mapped to a value inside the JSON document of the + * referenced feature + */ + static FeatureSchema bereich(Optional titleSourcePath) { + def ref = new ImmutableFeatureSchema.Builder() + .name("gehoertZuPlan") + .type(SchemaBase.Type.FEATURE_REF) + .sourcePath(REF_PATH) + .refType("WP_Plan") + .putProperties2("id", new ImmutableFeatureSchema.Builder() + .type(SchemaBase.Type.STRING) + .sourcePath("id")) + + titleSourcePath.ifPresent(path -> ref.putProperties2("title", + new ImmutableFeatureSchema.Builder().type(SchemaBase.Type.STRING).sourcePath(path))) + + return new ImmutableFeatureSchema.Builder() + .name("WP_Bereich") + .type(SchemaBase.Type.OBJECT) + .sourcePath("/coretable{filter=featuretype='WP_Bereich'}") + .putProperties2("oid", new ImmutableFeatureSchema.Builder() + .type(SchemaBase.Type.STRING) + .sourcePath("id") + .role(SchemaBase.Role.ID)) + .putProperties2("name", new ImmutableFeatureSchema.Builder() + .type(SchemaBase.Type.STRING) + .sourcePath("[JSON]properties/name")) + .putProperties2("gehoertZuPlan", ref) + .build() + } + + static FeatureSchema resolve(FeatureSchema type) { + Map types = Map.of("WP_Bereich", type) + List resolvers = List.of( + new MappingOperationResolver(true), + new FeatureRefResolver(Set.of("JSON", "EXPRESSION")), + new ImplicitMappingResolver(), + new ConstantsResolver(), + new LabelTemplateResolver(Optional.empty()), + new DefaultRolesResolver(), + new MappingOperationResolver()) + + for (TypesResolver resolver : resolvers) { + int rounds = 0 + while (resolver.needsResolving(types) && rounds < resolver.maxRounds()) { + types = resolver.resolve(types) + rounds++ + } + } + + return types.get("WP_Bereich") + } + + SqlQueryMapping mapping(Optional titleSourcePath) { + def resolved = resolve(bereich(titleSourcePath)) + + return mappingDeriver.derive(resolved.accept(new MappingRulesDeriver()), resolved).get(0) + } + + String encode(SqlQueryMapping mapping, String property) { + return filterEncoder.encode(Eq.of(Property.of(property), ScalarLiteral.of("X")), mapping) + } + + def 'feature reference queryable filters against the id column: title in the JSON document'() { + + given: "a reference whose title is mapped into the JSON document of the referenced feature" + def mapping = mapping(Optional.of("[JSON]properties/name")) + + when: "the reference is used as a queryable" + def actual = encode(mapping, "gehoertZuPlan") + + then: "the id column of the referenced table is filtered, not a value in the JSON document" + actual == JOINED + " WHERE AC.id = 'X')" + } + + def 'feature reference queryable filters against the id column: no title'() { + + given: "a reference without a title" + def mapping = mapping(Optional.empty()) + + when: "the reference is used as a queryable" + def actual = encode(mapping, "gehoertZuPlan") + + then: "the same filter is generated as with a title" + actual == JOINED + " WHERE AC.id = 'X')" + } + + def 'feature reference queryable filters against the id column: title in a column'() { + + given: "a reference whose title is mapped to a column of the referenced table" + def mapping = mapping(Optional.of("label")) + + when: "the reference is used as a queryable" + def actual = encode(mapping, "gehoertZuPlan") + + then: "the id column of the referenced table is filtered" + actual == JOINED + " WHERE AC.id = 'X')" + } + + def 'no object ever claims a value target: title #titleSourcePath'() { + + given: "a feature type with a feature reference" + def mapping = mapping(titleSourcePath) + + when: "the value targets are resolved" + def objectValues = mapping.getValueSchemas().findAll { target, schema -> !schema.isValue() } + + then: "none of them is an object, only values can be filtered or sorted on" + objectValues.isEmpty() + + and: "the reference itself resolves to the id column of the referenced table" + mapping.getValueColumns().get("gehoertZuPlan").getName() == "id" + + where: + titleSourcePath << [Optional.of("[JSON]properties/name"), Optional.of("label"), Optional.empty()] + } + + def 'a value in the JSON document of a referenced feature keeps its path in the document'() { + + given: "a reference whose title is mapped to properties/name of the referenced feature" + def mapping = mapping(Optional.of("[JSON]properties/name")) + + when: "the title is resolved" + def schema = mapping.getSchemaForValue("gehoertZuPlan.title") + + then: "the path in the connector is the path in the document, not the property name" + schema.isPresent() + mapping.getPathInConnector(schema.get()) == "name" + mapping.getValueColumns().get("gehoertZuPlan.title").getName() == "properties" + } +} diff --git a/xtraplatform-features/src/main/java/de/ii/xtraplatform/features/domain/MappingRule.java b/xtraplatform-features/src/main/java/de/ii/xtraplatform/features/domain/MappingRule.java index 5a0a47bf3..45246b5c4 100644 --- a/xtraplatform-features/src/main/java/de/ii/xtraplatform/features/domain/MappingRule.java +++ b/xtraplatform-features/src/main/java/de/ii/xtraplatform/features/domain/MappingRule.java @@ -109,4 +109,17 @@ static String maskPathAttributes(String path) { .matcher(path) .replaceAll(m -> "{" + "_".repeat(m.group(1).length()) + "}"); } + + /** + * Whether the last segment of the given source path is a sub-decoder column, that is {@code + * [CONNECTOR]column} (a join is {@code [sourceField=targetField]table} and has an {@code =} + * inside the brackets). + */ + static boolean endsWithConnector(String path) { + String masked = maskPathAttributes(path); + String segment = path.substring(masked.lastIndexOf('/') + 1); + int end = segment.indexOf(']'); + + return segment.startsWith("[") && end > 1 && !segment.substring(1, end).contains("="); + } } diff --git a/xtraplatform-features/src/main/java/de/ii/xtraplatform/features/domain/MappingRulesDeriver.java b/xtraplatform-features/src/main/java/de/ii/xtraplatform/features/domain/MappingRulesDeriver.java index 6ad046a2d..86b286311 100644 --- a/xtraplatform-features/src/main/java/de/ii/xtraplatform/features/domain/MappingRulesDeriver.java +++ b/xtraplatform-features/src/main/java/de/ii/xtraplatform/features/domain/MappingRulesDeriver.java @@ -106,10 +106,16 @@ public List finalize(FeatureSchema featureSchema, List MappingRule tableRule = new ImmutableMappingRule.Builder() .source(rule.getSourceParent()) + // a sub-decoder column (e.g. [JSON]properties) is not a property of the target + // schema, it is the container whose values the following rules address. It must + // always be ROOT_TARGET, otherwise it is taken for the enclosing object itself + // and that object gets mapped to the container column. .target( - rule.getTarget().contains(".") - ? rule.getTarget().substring(0, rule.getTarget().lastIndexOf(".")) - : ROOT_TARGET) + MappingRule.endsWithConnector(rule.getSourceParent()) + ? ROOT_TARGET + : rule.getTarget().contains(".") + ? rule.getTarget().substring(0, rule.getTarget().lastIndexOf(".")) + : ROOT_TARGET) .type( rule.getTarget().endsWith(VALUE_ARRAY_VALUE_SUFFIX) ? Type.VALUE_ARRAY