diff --git a/xtraplatform-features-gml/src/main/java/de/ii/xtraplatform/features/gml/domain/Circles.java b/xtraplatform-features-gml/src/main/java/de/ii/xtraplatform/features/gml/domain/Circles.java new file mode 100644 index 000000000..1fcddfb48 --- /dev/null +++ b/xtraplatform-features-gml/src/main/java/de/ii/xtraplatform/features/gml/domain/Circles.java @@ -0,0 +1,105 @@ +/* + * Copyright 2025 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.gml.domain; + +/** + * Helpers for converting between {@code } (3 control points on a circle) and the + * 5-position closed CIRCULARSTRING representation used internally. The internal form satisfies + * {@code isClosed()} (first position equals last) so it passes ring-closure validation and is + * accepted as a full circle by PostGIS as {@code CIRCULARSTRING(P1, P2, P3, antipode(P2), P1)}. + * + *

Only 2D (XY) circles are handled; 3D circles would require resolving the circle's plane in + * 3-space and are not produced by current data. + */ +final class Circles { + + private Circles() {} + + /** Coordinate-unit tolerance for the {@link #isFullCircleClosed(double[])} check. */ + private static final double EPS = 1.0e-6; + + /** Determinant tolerance below which three points are treated as colinear. */ + private static final double COLINEAR_EPS = 1.0e-12; + + /** + * Expand the 3 control points of a {@code } into a 5-position closed CIRCULARSTRING: + * {@code (P1, P2, P3, antipode(P2), P1)}. + * + * @param xyP1P2P3 6 doubles: {@code x1, y1, x2, y2, x3, y3} + * @return 10 doubles: the 5 expanded positions + * @throws IllegalArgumentException if the three points are colinear (no circle is defined) + */ + static double[] expandCircleToClosed(double[] xyP1P2P3) { + if (xyP1P2P3.length != 6) { + throw new IllegalArgumentException( + "expected exactly 3 XY positions (6 doubles), got " + xyP1P2P3.length); + } + double x1 = xyP1P2P3[0]; + double y1 = xyP1P2P3[1]; + double x2 = xyP1P2P3[2]; + double y2 = xyP1P2P3[3]; + double x3 = xyP1P2P3[4]; + double y3 = xyP1P2P3[5]; + double[] c = circumcenter(x1, y1, x2, y2, x3, y3); + double x4 = 2.0 * c[0] - x2; + double y4 = 2.0 * c[1] - y2; + return new double[] {x1, y1, x2, y2, x3, y3, x4, y4, x1, y1}; + } + + /** + * Test whether 5 closed positions form a full circle: positions 1, 2, 3 define a circle, the + * first and fifth positions coincide, and the fourth position is the antipode of the second + * position on that circle (all within {@link #EPS}). + * + * @param xy5positions 10 doubles: {@code x1, y1, x2, y2, x3, y3, x4, y4, x5, y5} + */ + static boolean isFullCircleClosed(double[] xy5positions) { + if (xy5positions.length != 10) { + return false; + } + double x1 = xy5positions[0]; + double y1 = xy5positions[1]; + double x5 = xy5positions[8]; + double y5 = xy5positions[9]; + if (Math.abs(x1 - x5) > EPS || Math.abs(y1 - y5) > EPS) { + return false; + } + double x2 = xy5positions[2]; + double y2 = xy5positions[3]; + double x3 = xy5positions[4]; + double y3 = xy5positions[5]; + double x4 = xy5positions[6]; + double y4 = xy5positions[7]; + try { + double[] c = circumcenter(x1, y1, x2, y2, x3, y3); + double expectedX4 = 2.0 * c[0] - x2; + double expectedY4 = 2.0 * c[1] - y2; + return Math.abs(x4 - expectedX4) <= EPS && Math.abs(y4 - expectedY4) <= EPS; + } catch (IllegalArgumentException colinear) { + return false; + } + } + + /** + * Circumcenter of three non-colinear 2D points. + * + * @throws IllegalArgumentException if the three points are colinear + */ + static double[] circumcenter(double x1, double y1, double x2, double y2, double x3, double y3) { + double d = 2.0 * (x1 * (y2 - y3) + x2 * (y3 - y1) + x3 * (y1 - y2)); + if (Math.abs(d) < COLINEAR_EPS) { + throw new IllegalArgumentException("three points are colinear; no circle is defined"); + } + double s1 = x1 * x1 + y1 * y1; + double s2 = x2 * x2 + y2 * y2; + double s3 = x3 * x3 + y3 * y3; + double cx = (s1 * (y2 - y3) + s2 * (y3 - y1) + s3 * (y1 - y2)) / d; + double cy = (s1 * (x3 - x2) + s2 * (x1 - x3) + s3 * (x2 - x1)) / d; + return new double[] {cx, cy}; + } +} diff --git a/xtraplatform-features-gml/src/main/java/de/ii/xtraplatform/features/gml/domain/FeatureTokenDecoderGml.java b/xtraplatform-features-gml/src/main/java/de/ii/xtraplatform/features/gml/domain/FeatureTokenDecoderGml.java index 4220bb126..e95fb1c39 100644 --- a/xtraplatform-features-gml/src/main/java/de/ii/xtraplatform/features/gml/domain/FeatureTokenDecoderGml.java +++ b/xtraplatform-features-gml/src/main/java/de/ii/xtraplatform/features/gml/domain/FeatureTokenDecoderGml.java @@ -240,6 +240,16 @@ private static final class Frame { boolean nilOnCurrent; String pendingXlinkHrefValue; + /** + * Temporary fallback for a STRING-typed VALUE / VALUE_ARRAY property whose element carries an + * {@code xlink:href} attribute but no text content. Used only when the property does not route + * hrefs via {@link #pendingXlinkHrefValue} (i.e. it is not a feature-ref or codelist property), + * and the element body produced no buffered text. Supports the workaround where a concat'd + * {@code FEATURE_REF_ARRAY} is modelled as a plain STRING {@code VALUE_ARRAY}, with the target + * id still arriving on the wire as {@code xlink:href}. + */ + String pendingXlinkHrefFallback; + /** * For VALUE_PROPERTY: set to {@code true} when the property's {@code fullPathAsString} is * listed in {@link FeatureTokenDecoderGmlInputProfile#getValueWrap()}. Children that appear @@ -653,6 +663,14 @@ && resolveVariableNameDiscriminator( Frame frame = Frame.valueProperty(prop, segment, segmentPathDepth); frame.nilOnCurrent = readXsiNil(); frame.pendingXlinkHrefValue = readXlinkHrefAsValue(prop); + if (frame.pendingXlinkHrefValue == null + && prop.getValueType().orElse(prop.getType()) == Type.STRING) { + String raw = readRawXlinkHref(); + frame.pendingXlinkHrefFallback = + raw == null + ? null + : applyReverseTemplate(inputProfile.getFeatureRefTemplate(), raw).orElse(raw); + } frame.valueWrapped = isValueWrapped(prop); validateUom(prop); frames.push(frame); @@ -699,8 +717,20 @@ && resolveVariableNameDiscriminator( private void onEndElement() throws XMLStreamException, java.io.IOException { if (geometryDecoder.isWaitingForInput()) { + // The geometry decoder paused on EVENT_INCOMPLETE while reading a coordinate element's text + // (pos/posList/coordinates). Any CHARACTERS that arrived after the pause landed in this + // decoder's `buffer` via the main loop. Hand them over so continueDecoding can append them + // to the geometry decoder's coord frame before finalising — otherwise the trailing chunk of + // the coordinate text is silently dropped (observed: a posList split mid-number produced a + // truncated odd coord count, which the dimension heuristic then promoted to XYZ). + String pending = isBuffering ? buffer.toString() : ""; + if (isBuffering) { + isBuffering = false; + buffer.setLength(0); + } Optional> optGeometry = - geometryDecoder.continueDecoding(parser, crs, srsDimension, parser.getLocalName(), ""); + geometryDecoder.continueDecoding( + parser, crs, srsDimension, parser.getLocalName(), pending); if (optGeometry.isPresent()) { emitGeometry(optGeometry.get()); } @@ -745,6 +775,10 @@ private void onEndElement() throws XMLStreamException, java.io.IOException { context.setValue(bufferedText); context.setValueType(Type.STRING); downstream.onValue(context); + } else if (frame.pendingXlinkHrefFallback != null) { + context.setValue(frame.pendingXlinkHrefFallback); + context.setValueType(Type.STRING); + downstream.onValue(context); } } else if (frame != null && frame.kind == FrameKind.OBJECT_PROPERTY) { // Re-track the OBJECT_PROPERTY's own path before emitting onObjectEnd — nested child @@ -1074,35 +1108,54 @@ private String readXlinkHrefAsValue(FeatureSchema prop) { if (!shouldRouteXlinkHrefAsValue(prop)) { return null; } - String href = null; + String href = readRawXlinkHref(); + if (href == null) { + return null; + } + return reverseXlinkHrefTemplate(href, prop); + } + + /** Returns the raw {@code xlink:href} attribute on the current START_ELEMENT, or {@code null}. */ + private String readRawXlinkHref() { for (int i = 0; i < parser.getAttributeCount(); i++) { if (XLINK_NS.equals(parser.getAttributeNamespace(i)) && "href".equals(parser.getAttributeLocalName(i))) { - href = parser.getAttributeValue(i); - break; + return parser.getAttributeValue(i); } } - if (href == null) { - return null; - } - return reverseXlinkHrefTemplate(href, prop); + return null; } /** * Reduces an {@code xlink:href} to its bare value segment via the appropriate reverse template * from the input profile. For codelist properties the schema's codelist id is substituted into - * {@code {{codelistId}}} first. If no template is configured, or the href does not match, the - * href is returned unchanged. + * {@code {{codelistId}}} first. If no template is configured, the href is returned unchanged + * silently. If a template is configured but the href does not match, the href is returned + * unchanged and a warning is logged — the raw URI will almost always overflow the storage column + * or be wrong as a value, so the operator needs visibility to fix the config mismatch. */ private String reverseXlinkHrefTemplate(String href, FeatureSchema prop) { + String template; if (prop.isFeatureRef()) { - return applyReverseTemplate(inputProfile.getFeatureRefTemplate(), href, null); + template = inputProfile.getFeatureRefTemplate(); + } else { + Optional codelistId = prop.getConstraints().flatMap(SchemaConstraints::getCodelist); + if (codelistId.isEmpty()) { + return href; + } + String raw = inputProfile.getCodelistUriTemplate(); + template = raw == null ? null : raw.replace("{{codelistId}}", codelistId.get()); } - Optional codelistId = prop.getConstraints().flatMap(SchemaConstraints::getCodelist); - if (codelistId.isPresent()) { - return applyReverseTemplate(inputProfile.getCodelistUriTemplate(), href, codelistId.get()); + Optional reduced = applyReverseTemplate(template, href); + if (reduced.isEmpty() && template != null && !template.isEmpty() && LOGGER.isWarnEnabled()) { + LOGGER.warn( + "xlink:href '{}' on property '{}' does not match the configured template '{}'; " + + "the unchanged href is passed through as the value", + href, + prop.getFullPathAsString(), + template); } - return href; + return reduced.orElse(href); } private static boolean shouldRouteXlinkHrefAsValue(FeatureSchema prop) { @@ -1112,21 +1165,19 @@ private static boolean shouldRouteXlinkHrefAsValue(FeatureSchema prop) { return prop.getConstraints().flatMap(SchemaConstraints::getCodelist).isPresent(); } - private static String applyReverseTemplate(String template, String href, String codelistId) { + private static Optional applyReverseTemplate(String template, String href) { if (template == null || template.isEmpty()) { - return href; + return Optional.empty(); } - String substituted = - codelistId == null ? template : template.replace("{{codelistId}}", codelistId); - int idx = substituted.indexOf("{{value}}"); + int idx = template.indexOf("{{value}}"); if (idx < 0) { - return href; + return Optional.empty(); } - String prefix = substituted.substring(0, idx); - String suffix = substituted.substring(idx + "{{value}}".length()); + String prefix = template.substring(0, idx); + String suffix = template.substring(idx + "{{value}}".length()); String regex = Pattern.quote(prefix) + "(.+?)" + Pattern.quote(suffix); Matcher m = Pattern.compile(regex).matcher(href); - return m.matches() ? m.group(1) : href; + return m.matches() ? Optional.of(m.group(1)) : Optional.empty(); } /** diff --git a/xtraplatform-features-gml/src/main/java/de/ii/xtraplatform/features/gml/domain/GeometryDecoderGml.java b/xtraplatform-features-gml/src/main/java/de/ii/xtraplatform/features/gml/domain/GeometryDecoderGml.java index ccc09ee25..b94b57b85 100644 --- a/xtraplatform-features-gml/src/main/java/de/ii/xtraplatform/features/gml/domain/GeometryDecoderGml.java +++ b/xtraplatform-features-gml/src/main/java/de/ii/xtraplatform/features/gml/domain/GeometryDecoderGml.java @@ -597,7 +597,7 @@ private Geometry buildGeometry(Frame f) throws IOException { case POLYGON_PATCH -> buildPolygon(f); case LINE_STRING_SEGMENT -> buildSinglePosList(f, false); case ARC, ARC_STRING -> buildSinglePosList(f, true); - case CIRCLE -> buildSinglePosList(f, true); + case CIRCLE -> buildCircle(f); default -> null; }; } @@ -611,6 +611,37 @@ private Point buildPoint(Frame f) throws IOException { return Point.of(Position.of(axes, c), f.crs); } + /** + * A {@code } is 3 points defining a full circle. Internally we store it as a + * 5-position closed CIRCULARSTRING — {@code (P1, P2, P3, antipode(P2), P1)} — so it satisfies + * ring-closure validation and round-trips through WKT/WKB into PostGIS as a full circle. The + * original 3-point form is recovered by {@link GeometryEncoderGml} when emitting GML. + */ + private Geometry buildCircle(Frame f) throws IOException { + double[] c = f.coords != null ? f.coords : new double[0]; + if (c.length == 0) { + throw new IOException("Empty ."); + } + Axes axes = axesOf(f); + if (axes != Axes.XY) { + // 3D circles would need the plane of the circle resolved in 3-space; no current data + // requires this. Fall back to the linear 3-point form (which downstream ring-closure + // validation will reject loudly). + return buildSinglePosList(f, true); + } + if (c.length != 6) { + throw new IOException( + " requires exactly 3 XY positions, got " + (c.length / 2) + "."); + } + double[] expanded; + try { + expanded = Circles.expandCircleToClosed(c); + } catch (IllegalArgumentException colinear) { + throw new IOException("Invalid : " + colinear.getMessage()); + } + return CircularString.of(PositionList.of(axes, expanded), f.crs); + } + private Geometry buildSinglePosList(Frame f, boolean curved) throws IOException { double[] c = f.coords != null ? f.coords : new double[0]; if (c.length == 0) { diff --git a/xtraplatform-features-gml/src/main/java/de/ii/xtraplatform/features/gml/domain/GeometryEncoderGml.java b/xtraplatform-features-gml/src/main/java/de/ii/xtraplatform/features/gml/domain/GeometryEncoderGml.java index 5753006fd..992182fb2 100644 --- a/xtraplatform-features-gml/src/main/java/de/ii/xtraplatform/features/gml/domain/GeometryEncoderGml.java +++ b/xtraplatform-features-gml/src/main/java/de/ii/xtraplatform/features/gml/domain/GeometryEncoderGml.java @@ -50,6 +50,7 @@ public class GeometryEncoderGml implements GeometryVisitor { private static final String LINE_STRING_SEGMENT = "LineStringSegment"; private static final String ARC = "Arc"; private static final String ARC_STRING = "ArcString"; + private static final String CIRCLE = "Circle"; private static final String MULTI_CURVE = "MultiCurve"; private static final String MULTI_LINE_STRING = "MultiLineString"; private static final String POLYGON = "Polygon"; @@ -436,10 +437,22 @@ private void writeCircularString(CircularString geometry, boolean asSegment) { writeStartTagObject(CURVE, false); writeStartTagProperty(SEGMENTS); } - String tagName = geometry.getValue().getNumPositions() == 3 ? ARC : ARC_STRING; - writeStartTagDataType(tagName); - writePositionList(geometry.getValue().getCoordinates(), geometry.getAxes()); - writeEndTag(); + double[] coords = geometry.getValue().getCoordinates(); + int numPositions = geometry.getValue().getNumPositions(); + if (geometry.getAxes() == Axes.XY && numPositions == 5 && Circles.isFullCircleClosed(coords)) { + // Round-trip our 5-position closed-circle representation back to gml:Circle (3 control + // points). See GeometryDecoderGml#buildCircle for the inverse expansion. + writeStartTagDataType(CIRCLE); + double[] threePoints = new double[6]; + System.arraycopy(coords, 0, threePoints, 0, 6); + writePositionList(threePoints, geometry.getAxes()); + writeEndTag(); + } else { + String tagName = numPositions == 3 ? ARC : ARC_STRING; + writeStartTagDataType(tagName); + writePositionList(coords, geometry.getAxes()); + writeEndTag(); + } if (!asSegment) { writeEndTag(); writeEndTag(); diff --git a/xtraplatform-features-gml/src/test/groovy/de/ii/xtraplatform/features/gml/domain/CirclesSpec.groovy b/xtraplatform-features-gml/src/test/groovy/de/ii/xtraplatform/features/gml/domain/CirclesSpec.groovy new file mode 100644 index 000000000..071b6f799 --- /dev/null +++ b/xtraplatform-features-gml/src/test/groovy/de/ii/xtraplatform/features/gml/domain/CirclesSpec.groovy @@ -0,0 +1,58 @@ +/* + * Copyright 2025 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.gml.domain + +import spock.lang.Specification + +class CirclesSpec extends Specification { + + def 'expandCircleToClosed produces (P1, P2, P3, antipode(P2), P1)'() { + // given: circle through (0,0), (1,1), (2,0) has center (1,0); antipode of (1,1) is (1,-1) + when: + double[] expanded = Circles.expandCircleToClosed([0d, 0d, 1d, 1d, 2d, 0d] as double[]) + + then: + expanded == [0d, 0d, 1d, 1d, 2d, 0d, 1d, -1d, 0d, 0d] as double[] + } + + def 'expandCircleToClosed rejects colinear points'() { + when: + Circles.expandCircleToClosed([0d, 0d, 1d, 0d, 2d, 0d] as double[]) + + then: + thrown(IllegalArgumentException) + } + + def 'isFullCircleClosed accepts the canonical 5-point expansion'() { + expect: + Circles.isFullCircleClosed([0d, 0d, 1d, 1d, 2d, 0d, 1d, -1d, 0d, 0d] as double[]) + } + + def 'isFullCircleClosed rejects a 5-point CIRCULARSTRING that is closed but is not a circle'() { + // Two arcs that form a closed shape but whose 4 distinct control points are not on a + // common circle. + expect: + !Circles.isFullCircleClosed([0d, 0d, 1d, 1d, 2d, 0d, 1d, -3d, 0d, 0d] as double[]) + } + + def 'isFullCircleClosed rejects when start != end'() { + expect: + !Circles.isFullCircleClosed([0d, 0d, 1d, 1d, 2d, 0d, 1d, -1d, 0d, 0.5d] as double[]) + } + + def 'isFullCircleClosed rejects non-5-position lists'() { + expect: + !Circles.isFullCircleClosed([0d, 0d, 1d, 1d, 0d, 0d] as double[]) + } + + def 'circumcenter of an axis-aligned right triangle'() { + // Right triangle at origin, (2,0), (0,2): hypotenuse midpoint (1,1) is the circumcenter. + expect: + Circles.circumcenter(0d, 0d, 2d, 0d, 0d, 2d) == [1d, 1d] as double[] + } +} diff --git a/xtraplatform-features-gml/src/test/groovy/de/ii/xtraplatform/features/gml/domain/FeatureTokenDecoderGmlNasSpec.groovy b/xtraplatform-features-gml/src/test/groovy/de/ii/xtraplatform/features/gml/domain/FeatureTokenDecoderGmlNasSpec.groovy index 62df8bc0b..654be3e34 100644 --- a/xtraplatform-features-gml/src/test/groovy/de/ii/xtraplatform/features/gml/domain/FeatureTokenDecoderGmlNasSpec.groovy +++ b/xtraplatform-features-gml/src/test/groovy/de/ii/xtraplatform/features/gml/domain/FeatureTokenDecoderGmlNasSpec.groovy @@ -122,6 +122,81 @@ class FeatureTokenDecoderGmlNasSpec extends Specification { return stream.on(runner).run().toCompletableFuture().join() as List } + def 'chunked feed: a buffer split mid-posList number must not lose the trailing coordinate text'() { + // When the async XML feeder splits a chunk inside the text content of a gml:posList, the + // trailing CHARACTERS event is delivered after the geometry decoder has paused + // (EVENT_INCOMPLETE). The outer FeatureTokenDecoderGml main switch buffers that trailing + // text; FeatureTokenDecoderGml#onEndElement must forward it to continueDecoding instead of + // passing "". Without that forwarding the coord frame finalises on an odd number of doubles + // and GeometryDecoderGml#buildSinglePosList's dimension heuristic promotes XY to XYZ. + given: + String feature = + '' + + '' + + '363334.849 5614983.888 363351.009 5615009.083' + + '' + + '' + byte[] all = feature.getBytes('UTF-8') + // Locate the first decimal in "363351.009" and split bytes right after the '.0' — places + // the boundary mid-number ("363351.0" | "09 5615009.083") so aalto-xml has to emit the + // posList CHARACTERS in two events. + int boundary = feature.indexOf('363351.0') + '363351.0'.length() + byte[] chunk1 = java.util.Arrays.copyOfRange(all, 0, boundary) + byte[] chunk2 = java.util.Arrays.copyOfRange(all, boundary, all.length) + + def decoder = newDecoder("AX_Flurstueck", GeometryType.LINE_STRING) + def emitted = [] + decoder.init({ emitted << it } as java.util.function.Consumer) + + when: + decoder.onPush(chunk1) + decoder.onPush(chunk2) + decoder.onComplete() + def geom = emitted.find { it instanceof Geometry } as Geometry + + then: + geom != null + geom.getAxes().toString() == 'XY' + geom.getValue().getNumPositions() == 2 + geom.getValue().getCoordinates() == [363334.849d, 5614983.888d, 363351.009d, 5615009.083d] as double[] + } + + def 'minimal AX_Flurstueck: extra xmlns on feature root must not flip geometry to XYZ'() { + given: + String feature4 = '' + + '' + + '' + + '363351.009 5615009.083 363332.428 5615021.012' + + '' + + '' + + '363332.428 5615021.012 363351.009 5615009.083' + + '' + + '' + + '' + String feature6 = '' + + '' + + '' + + '363351.009 5615009.083 363332.428 5615021.012' + + '' + + '' + + '363332.428 5615021.012 363351.009 5615009.083' + + '' + + '' + + '' + + when: + def tokens4 = runDecoder(newDecoder("AX_Flurstueck", GeometryType.MULTI_POLYGON), feature4.getBytes("UTF-8")) + def tokens6 = runDecoder(newDecoder("AX_Flurstueck", GeometryType.MULTI_POLYGON), feature6.getBytes("UTF-8")) + def geom4 = tokens4.find { it instanceof Geometry } as Geometry + def geom6 = tokens6.find { it instanceof Geometry } as Geometry + + then: + geom4 != null + geom6 != null + geom4.getAxes().toString() == 'XY' + geom6.getAxes().toString() == 'XY' + } + @Unroll def 'NAS feature #featureType: gml:id and geometry are routed to the schema source paths'() { given: diff --git a/xtraplatform-features-gml/src/test/groovy/de/ii/xtraplatform/features/gml/domain/FeatureTokenDecoderGmlSpec.groovy b/xtraplatform-features-gml/src/test/groovy/de/ii/xtraplatform/features/gml/domain/FeatureTokenDecoderGmlSpec.groovy index ea6a06b60..0c3c6c5f2 100644 --- a/xtraplatform-features-gml/src/test/groovy/de/ii/xtraplatform/features/gml/domain/FeatureTokenDecoderGmlSpec.groovy +++ b/xtraplatform-features-gml/src/test/groovy/de/ii/xtraplatform/features/gml/domain/FeatureTokenDecoderGmlSpec.groovy @@ -536,7 +536,11 @@ class FeatureTokenDecoderGmlSpec extends Specification { valueAtPath(tokens, ["11001-21008"]) == "urn:adv:oid:DENW36ALl800005x" } - def 'xlink:href is emitted unchanged when the configured template does not match'() { + def 'xlink:href is emitted unchanged when the configured template does not match, and a WARN is logged'() { + // The fall-through to the raw href is necessary (we have no better value to emit) but + // dangerous in practice — the URI is almost always wider than the storage column and longer + // than any expected value. A no-match is therefore a configuration mismatch the operator + // needs to see, not a silent best-effort. given: def decoder = newDecoder(axFlurstueckWithRefsSchema(), NAS_TEMPLATES) def xml = """""" when: - def tokens = runDecoder(decoder, xml) + List tokens = null + List warnings = captureWarnings { tokens = runDecoder(decoder, xml) } then: valueAtPath(tokens, ["11001-21008"]) == "https://example.org/items/some-other-id" + warnings.size() == 1 + def msg = warnings[0].formattedMessage + msg.contains("https://example.org/items/some-other-id") + msg.contains("does not match the configured template") + } + + def 'codelist xlink:href that does not match the configured template logs a WARN'() { + // Real-world manifestation: a service config whose codelistUriTemplate uses a namespace + // segment (e.g. 'de.adv.alkis') that differs from the namespace segment in the actual + // NAS data ('de.adv-online.gid'). Today the raw URI flowed silently into the SQL layer + // and surfaced as a varchar overflow at insert time; a WARN gives the operator a clear + // 'fix your template' signal at decode time instead. + given: + def mismatchedProfile = ImmutableFeatureTokenDecoderGmlInputProfile.builder() + .useAlias(true) + .codelistUriTemplate("https://registry.gdi-de.org/codelist/de.adv.alkis/{{codelistId}}/{{value}}") + .build() + def decoder = newDecoder(axFlurstueckWithRefsSchema(), mismatchedProfile) + def xml = """ + + """ + + when: + List tokens = null + List warnings = captureWarnings { tokens = runDecoder(decoder, xml) } + + then: 'the unchanged href is emitted as the value (will fail downstream)' + tokens.contains("https://registry.gdi-de.org/codelist/de.adv-online.gid/AA_Anlassart/010704") + + and: 'a WARN identifies the mismatching href and the configured template with the codelistId substituted' + warnings.size() == 1 + def msg = warnings[0].formattedMessage + msg.contains("https://registry.gdi-de.org/codelist/de.adv-online.gid/AA_Anlassart/010704") + msg.contains("https://registry.gdi-de.org/codelist/de.adv.alkis/AA_Anlassart/{{value}}") + } + + def 'matching xlink:href does not log a WARN'() { + given: + def decoder = newDecoder(axFlurstueckWithRefsSchema(), NAS_TEMPLATES) + def xml = """ + + + """ + + when: + List tokens = null + List warnings = captureWarnings { tokens = runDecoder(decoder, xml) } + + then: + warnings.empty + tokens.contains("DENW36ALl800005x") + tokens.contains("010704") } def 'wrap=OBJECT FEATURE_REF: xlink:href on the property element emits the reduced id at the .id child'() { @@ -664,11 +728,32 @@ class FeatureTokenDecoderGmlSpec extends Specification { !tokens.contains("urn:adv:oid:DENW36AL00000AAA") } - def 'xlink:href on a property that is neither codelist nor feature-ref is dropped'() { + def 'xlink:href on a STRING property with empty body is emitted as the value (fallback)'() { given: // qid (quellobjektID) is a plain STRING inherited from aa_objekt — no codelist, not a - // feature-ref. An xlink:href on it must not surface as the value, and xlink:* must not - // leak as additional attributes. + // feature-ref. When the element has no text content but carries an xlink:href, the + // decoder routes the href through the featureRefTemplate reverse substitution as a + // fallback so that the FEATURE_REF_ARRAY → VALUE_ARRAY workaround keeps working on the + // wire. The featureRefTemplate matches `urn:adv:oid:{{value}}` here, so an unrelated href + // like `urn:something:else:42` falls through unchanged. + def decoder = newDecoder(axFlurstueckWithRefsSchema(), NAS_TEMPLATES) + def xml = """ + + """ + + when: + def tokens = runDecoder(decoder, xml) + + then: + tokens.contains("DENW36AL10000QID") + !tokens.contains("urn:adv:oid:DENW36AL10000QID") + } + + def 'xlink:href on a STRING property whose href does not match featureRefTemplate is emitted unchanged'() { + given: def decoder = newDecoder(axFlurstueckWithRefsSchema(), NAS_TEMPLATES) def xml = """ @@ -145,7 +148,7 @@ class GeometryDecoderGmlRoundtripSpec extends Specification { 0 0 1 1 2 0 ''', - 'CIRCULARSTRING(0.0 0.0,1.0 1.0,2.0 0.0)', + 'CIRCULARSTRING(0.0 0.0,1.0 1.0,2.0 0.0,1.0 -1.0,0.0 0.0)', Set.of() ) } @@ -393,6 +396,47 @@ class GeometryDecoderGmlRoundtripSpec extends Specification { ) } + def 'Surface with PolygonPatch Ring containing a Circle (NAS shape)'() { + // The Bonn ALKIS data ships round road-intersection polygons whose outer Ring is a single + // gml:Circle (3 control points). Before circle-aware decoding these tripped the + // "All rings must be closed" check on Polygon/CurvePolygon construction. + expect: + roundtrip( + ''' + + + + + + + 0 0 1 1 2 0 + + + + + + + ''', + 'CURVEPOLYGON(CIRCULARSTRING(0.0 0.0,1.0 1.0,2.0 0.0,1.0 -1.0,0.0 0.0))', + Set.of(GeometryEncoderGml.Options.USE_SURFACE_RING_CURVE) + ) + } + + def 'Curve with one closed ArcString that is NOT a full circle stays an ArcString'() { + // 5-position closed CIRCULARSTRING whose four distinct control points are not on a + // common circle: the encoder must keep emitting gml:ArcString rather than gml:Circle. + expect: + roundtrip( + ''' + + 0 0 1 1 2 0 1 -3 0 0 + + ''', + 'CIRCULARSTRING(0.0 0.0,1.0 1.0,2.0 0.0,1.0 -3.0,0.0 0.0)', + Set.of() + ) + } + def 'MultiSurface of Surfaces (NAS shape) -> MultiPolygon when all linear'() { expect: roundtrip( diff --git a/xtraplatform-features-gml/src/test/groovy/de/ii/xtraplatform/features/gml/domain/GeometryDecoderGmlSpec.groovy b/xtraplatform-features-gml/src/test/groovy/de/ii/xtraplatform/features/gml/domain/GeometryDecoderGmlSpec.groovy index ebace9f19..565f4e744 100644 --- a/xtraplatform-features-gml/src/test/groovy/de/ii/xtraplatform/features/gml/domain/GeometryDecoderGmlSpec.groovy +++ b/xtraplatform-features-gml/src/test/groovy/de/ii/xtraplatform/features/gml/domain/GeometryDecoderGmlSpec.groovy @@ -32,6 +32,41 @@ class GeometryDecoderGmlSpec extends Specification { return new GeometryEncoderWkt().encode(g) } + def 'Surface geometry: extra unrelated xmlns declarations on the geometry root must not affect dimensionality'() { + // Same Surface geometry, wrapped in two different xmlns scopes. Extra namespace declarations + // on an enclosing element (xsi, fes) must not influence the geometry decoder's axes. + given: + String geom = ''' + + + 363351.009 5615009.083 363332.428 5615021.012 + + + 363332.428 5615021.012 363351.009 5615009.083 + + + ''' + String body = ''' + + + 363351.009 5615009.083 363332.428 5615021.012 + + + 363332.428 5615021.012 363351.009 5615009.083 + + ''' + String onlyGml = '' + body + String gmlPlusXsiFes = '' + body + + when: + def a = decode(onlyGml) + def b = decode(gmlPlusXsiFes) + + then: + a.axes.toString() == 'XY' + b.axes.toString() == 'XY' + } + def 'Point XY'() { when: def g = decode('10 20') @@ -135,7 +170,9 @@ class GeometryDecoderGmlSpec extends Specification { wkt(g) == 'CIRCULARSTRING(0.0 0.0,1.0 1.0,2.0 0.0)' } - def 'Curve with Circle segment -> CircularString'() { + def 'Curve with Circle segment -> 5-position closed CircularString'() { + // A gml:Circle is geometrically closed; we expand the 3 control points to a 5-position + // closed CIRCULARSTRING so isClosed() holds and PostGIS accepts it as a full circle. when: def g = decode(''' @@ -144,7 +181,7 @@ class GeometryDecoderGmlSpec extends Specification { ''') then: - wkt(g) == 'CIRCULARSTRING(0.0 0.0,1.0 1.0,2.0 0.0)' + wkt(g) == 'CIRCULARSTRING(0.0 0.0,1.0 1.0,2.0 0.0,1.0 -1.0,0.0 0.0)' } def 'Curve with multiple LineStringSegments -> merged LineString'() { diff --git a/xtraplatform-features-json/src/main/java/de/ii/xtraplatform/features/json/domain/FeatureTokenDecoderGeoJson.java b/xtraplatform-features-json/src/main/java/de/ii/xtraplatform/features/json/domain/FeatureTokenDecoderGeoJson.java index 39b78802b..5b137dfb1 100644 --- a/xtraplatform-features-json/src/main/java/de/ii/xtraplatform/features/json/domain/FeatureTokenDecoderGeoJson.java +++ b/xtraplatform-features-json/src/main/java/de/ii/xtraplatform/features/json/domain/FeatureTokenDecoderGeoJson.java @@ -11,6 +11,9 @@ import com.fasterxml.jackson.core.JsonParser; import com.fasterxml.jackson.core.JsonToken; import com.fasterxml.jackson.core.async.ByteArrayFeeder; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.util.TokenBuffer; import de.ii.xtraplatform.crs.domain.EpsgCrs; import de.ii.xtraplatform.features.domain.FeatureSchema; import de.ii.xtraplatform.features.domain.SchemaBase.Type; @@ -34,6 +37,7 @@ public class FeatureTokenDecoderGeoJson private static final String PROPERTIES = "properties"; private static final JsonFactory JSON_FACTORY = new JsonFactory(); + private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); private final JsonParser parser; private final ByteArrayFeeder feeder; @@ -49,6 +53,15 @@ public class FeatureTokenDecoderGeoJson private boolean inGeometry; private int lastNameIsArrayDepth; + // Tokens of a GeoJSON "geometry"/"place" sub-object are buffered until the matching END_OBJECT + // so geometry decoding always operates on a complete in-memory subtree. This is what makes the + // decoder safe when an async chunk boundary lands inside a coordinates array, which would + // otherwise surface as "Unexpected Token: NOT_AVAILABLE" from the sync GeometryDecoderJson. + private boolean inGeometryBuffer; + private TokenBuffer geometryBuffer; + private int geometryDepth; + private String geometryFieldName; + private ModifiableContext context; private FeatureTokenBufferSimple< FeatureSchema, SchemaMapping, ModifiableContext> @@ -121,6 +134,30 @@ public boolean advanceParser() { return true; // or completestage??? } + if (inGeometryBuffer) { + if (nextToken == JsonToken.NOT_AVAILABLE) { + return true; + } + geometryBuffer.copyCurrentEvent(parser); + if (nextToken == JsonToken.START_OBJECT || nextToken == JsonToken.START_ARRAY) { + geometryDepth++; + } else if (nextToken == JsonToken.END_OBJECT || nextToken == JsonToken.END_ARRAY) { + geometryDepth--; + if (geometryDepth == 0) { + JsonNode geomNode = OBJECT_MAPPER.readTree(geometryBuffer.asParser()); + Geometry geometry = + new GeometryDecoderJson().decode(geomNode, Optional.of(crs), Optional.of(axes)); + context.setGeometry(geometry); + context.pathTracker().track(geometryFieldName, 0); + downstream.onGeometry(context); + inGeometryBuffer = false; + geometryBuffer = null; + geometryFieldName = null; + } + } + return false; + } + String currentName = parser.currentName(); switch (nextToken) { @@ -134,11 +171,11 @@ public boolean advanceParser() { case START_OBJECT: if (Objects.nonNull(currentName) && ("geometry".equals(currentName) || "place".equals(currentName))) { - Geometry geometry = - new GeometryDecoderJson().decode(parser, Optional.of(crs), Optional.of(axes)); - context.setGeometry(geometry); - context.pathTracker().track(currentName, 0); - downstream.onGeometry(context); + geometryBuffer = new TokenBuffer(OBJECT_MAPPER, false); + geometryBuffer.copyCurrentEvent(parser); + geometryFieldName = currentName; + geometryDepth = 1; + inGeometryBuffer = true; } else { if (Objects.nonNull(currentName) && PROPERTIES.equals(currentName) && !started) { startIfNecessary(false); diff --git a/xtraplatform-features-json/src/test/groovy/de/ii/xtraplatform/features/json/app/FeatureTokenDecoderGeoJsonSpec2.groovy b/xtraplatform-features-json/src/test/groovy/de/ii/xtraplatform/features/json/app/FeatureTokenDecoderGeoJsonSpec2.groovy index 17bc0fc15..d818236e7 100644 --- a/xtraplatform-features-json/src/test/groovy/de/ii/xtraplatform/features/json/app/FeatureTokenDecoderGeoJsonSpec2.groovy +++ b/xtraplatform-features-json/src/test/groovy/de/ii/xtraplatform/features/json/app/FeatureTokenDecoderGeoJsonSpec2.groovy @@ -200,4 +200,37 @@ class FeatureTokenDecoderGeoJsonSpec2 extends Specification { tokens == FeatureTokenFixtures.COLLECTION } + + def 'large polygon feature: chunk boundary inside coordinates must not lose tokens'() { + + // Reproducer for a bug where a non-blocking chunk boundary falling inside the GeoJSON + // coordinates array surfaced as "Unexpected Token: NOT_AVAILABLE" from the sync + // GeometryDecoderJson. Fixture is a ~17 KB AX_Flurstueck polygon (701 vertices); we feed it + // in two byte chunks split mid-coordinates to force the async pause. + + given: + byte[] data = new File('src/test/resources/large_polygon_feature.json').bytes + int split = data.length.intdiv(2) + byte[] chunk1 = Arrays.copyOfRange(data, 0, split) + byte[] chunk2 = Arrays.copyOfRange(data, split, data.length) + Reactive.Source chunked = Reactive.Source.iterable([chunk1, chunk2]) + + Reactive.Stream> chunkedStream = chunked.via(decoder).to(ListSink()) + + FeatureTokenDecoderSimple> singleShotDecoder = + new FeatureTokenDecoderGeoJson(Optional.empty(), OgcCrs.CRS84, Axes.XY) + Reactive.Stream> singleShotStream = Reactive.Source.iterable([data]) + .via(singleShotDecoder) + .to(ListSink()) + + when: + List chunkedTokens = runStream(chunkedStream) + List singleShotTokens = runStream(singleShotStream) + + then: + // The split-feed must yield the same token sequence as a single-shot feed of the same + // bytes, and the run must not throw. + !chunkedTokens.isEmpty() + chunkedTokens == singleShotTokens + } } diff --git a/xtraplatform-features-json/src/test/resources/large_polygon_feature.json b/xtraplatform-features-json/src/test/resources/large_polygon_feature.json new file mode 100644 index 000000000..4c5d6ccc5 --- /dev/null +++ b/xtraplatform-features-json/src/test/resources/large_polygon_feature.json @@ -0,0 +1 @@ +{"type":"Feature","id":"DENW36AL10006zvZ","geometry":{"type":"Polygon","coordinates":[[[363903.852,5625977.392],[363820.182,5626032.114],[363736.522,5626086.834],[363678.921,5626124.504],[363595.88,5625997.512],[363587.468,5625984.647],[363585.622,5625981.834],[363620.917,5625957.363],[363642.629,5625942.795],[363650.207,5625937.692],[363651.469,5625934.848],[363653.476,5625935.739],[363725.768,5625887.383],[363787.656,5625848.814],[363788.579,5625854.655],[363792.704,5625854.471],[363792.643,5625850.533],[363799.736,5625839.862],[363809.491,5625833.631],[363815.175,5625828.724],[363823.9,5625821.193],[363875.929,5625791.084],[363894.581,5625780.157],[363932.748,5625756.198],[363980.235,5625729.709],[364062.909,5625677.44],[364063.678,5625675.005],[364072.681,5625669.382],[364123.744,5625638.324],[364149.25,5625622.245],[364182.003,5625601.949],[364233.811,5625571.463],[364272.154,5625543.285],[364312.391,5625518.886],[364345.377,5625496.041],[364357.652,5625485.199],[364388.404,5625462.359],[364410.031,5625445.999],[364413.99,5625447.503],[364416.385,5625445.167],[364418.213,5625439.488],[364466.385,5625405.917],[364508.318,5625373.72],[364535.816,5625352.061],[364539.888,5625348.884],[364597.454,5625301.41],[364619.964,5625281.941],[364653.097,5625249.308],[364657.389,5625244.762],[364674.365,5625229.096],[364683.14,5625219.462],[364708.346,5625194.14],[364742.074,5625156.895],[364742.554,5625157.376],[364742.55,5625151.49],[364742.268,5625151.205],[364743.077,5625150.322],[364744.821,5625148.418],[364745.134,5625148.076],[364745.627,5625148.572],[364752.293,5625148.574],[364760.334,5625148.424],[364761.401,5625148.403],[364773.276,5625141.053],[364780.939,5625136.254],[364805.422,5625116.236],[364822.541,5625100.964],[364931.442,5625005.784],[364938.574,5624999.603],[364953.881,5624986.337],[364996.598,5624949.317],[365038.0,5624913.437],[365068.048,5624884.788],[365073.617,5624881.033],[365073.086,5624880.296],[365084.312,5624872.487],[365148.813,5624827.619],[365174.201,5624810.384],[365200.151,5624793.329],[365211.398,5624786.082],[365212.142,5624787.331],[365226.344,5624778.256],[365240.607,5624769.278],[365254.932,5624760.397],[365269.316,5624751.615],[365283.761,5624742.931],[365298.264,5624734.346],[365312.826,5624725.86],[365327.445,5624717.474],[365342.121,5624709.188],[365356.854,5624701.002],[365371.642,5624692.917],[365386.385,5624684.946],[365461.166,5624644.51],[365480.72,5624633.937],[365484.051,5624630.692],[365528.957,5624605.77],[365578.815,5624578.108],[365603.418,5624564.458],[365601.13,5624560.757],[365651.86,5624532.237],[365653.077,5624531.553],[365653.119,5624531.529],[365655.452,5624530.218],[365656.658,5624529.539],[365682.748,5624514.872],[365705.366,5624498.709],[365732.984,5624475.085],[365750.05,5624459.273],[365775.67,5624433.405],[365810.251,5624392.809],[365815.923,5624384.737],[365817.412,5624382.617],[365817.936,5624381.871],[365820.221,5624378.62],[365824.897,5624371.992],[365827.335,5624368.536],[365832.113,5624361.765],[365834.465,5624358.431],[365834.959,5624357.732],[365836.672,5624355.304],[365847.201,5624340.381],[365848.311,5624338.808],[365849.811,5624336.681],[365851.158,5624334.771],[365861.73,5624319.787],[365898.15,5624273.075],[365912.449,5624248.91],[365936.131,5624206.921],[365940.956,5624198.367],[365941.527,5624197.354],[365942.854,5624195.003],[365943.873,5624195.636],[365944.862,5624194.176],[365957.969,5624174.826],[365964.27,5624162.99],[365988.555,5624117.374],[365999.103,5624097.562],[366014.061,5624067.445],[366016.598,5624062.335],[366021.491,5624049.45],[366032.979,5624019.202],[366055.991,5623957.602],[366058.407,5623951.134],[366062.664,5623939.74],[366080.783,5623892.695],[366084.201,5623883.82],[366084.842,5623882.156],[366083.27,5623882.078],[366118.945,5623781.396],[366135.195,5623726.281],[366140.995,5623705.255],[366143.118,5623698.319],[366143.874,5623696.975],[366152.078,5623694.036],[366155.344,5623683.17],[366147.29,5623678.534],[366170.643,5623601.933],[366181.47,5623560.106],[366184.296,5623560.928],[366187.889,5623538.778],[366187.905,5623538.677],[366193.21,5623521.746],[366194.886,5623516.397],[366195.65,5623513.958],[366195.851,5623513.316],[366198.244,5623505.679],[366203.535,5623484.27],[366207.927,5623466.497],[366209.221,5623461.259],[366209.446,5623460.301],[366213.053,5623444.973],[366230.833,5623369.397],[366235.158,5623367.75],[366237.025,5623362.359],[366246.127,5623336.079],[366249.709,5623312.553],[366257.378,5623262.953],[366265.094,5623213.048],[366267.649,5623198.089],[366272.319,5623172.086],[366282.651,5623114.561],[366283.917,5623107.593],[366275.506,5623108.23],[366268.96,5623108.726],[366273.072,5623084.717],[366281.95,5623040.678],[366285.383,5623023.454],[366286.052,5623020.06],[366297.727,5622960.859],[366302.528,5622935.749],[366304.316,5622926.394],[366304.347,5622926.234],[366307.228,5622925.888],[366319.431,5622924.42],[366320.1,5622924.34],[366320.951,5622920.033],[366322.068,5622914.384],[366324.421,5622902.476],[366332.601,5622903.174],[366336.473,5622903.505],[366348.048,5622904.493],[366353.144,5622904.928],[366369.867,5622906.356],[366383.791,5622831.365],[366411.966,5622679.744],[366432.567,5622568.889],[366460.265,5622401.388],[366464.159,5622402.102],[366465.84,5622392.201],[366469.871,5622371.582],[366470.095,5622370.437],[366470.8,5622366.833],[366470.235,5622364.702],[366469.682,5622362.778],[366478.756,5622300.026],[366485.369,5622258.719],[366495.035,5622204.898],[366506.282,5622147.18],[366514.743,5622107.67],[366523.149,5622068.837],[366525.899,5622056.131],[366536.247,5622013.983],[366602.188,5621773.923],[366603.096,5621770.586],[366618.297,5621714.755],[366603.198,5621709.181],[366604.522,5621704.41],[366656.417,5621562.657],[366657.549,5621562.709],[366658.681,5621562.67],[366659.807,5621562.541],[366660.918,5621562.321],[366662.009,5621562.012],[366663.07,5621561.617],[366664.097,5621561.138],[366665.082,5621560.578],[366666.018,5621559.932],[366666.899,5621559.212],[366667.719,5621558.424],[366668.473,5621557.572],[366669.156,5621556.662],[366669.763,5621555.699],[366670.29,5621554.691],[366670.734,5621553.644],[366671.132,5621552.422],[366671.417,5621551.169],[366671.587,5621549.896],[366671.64,5621548.612],[366671.575,5621547.328],[366671.394,5621546.056],[366671.098,5621544.806],[366670.746,5621543.727],[366670.308,5621542.679],[366669.788,5621541.671],[366669.187,5621540.707],[366668.511,5621539.795],[366667.764,5621538.941],[366666.95,5621538.149],[366666.075,5621537.426],[366729.698,5621380.332],[366744.238,5621342.241],[366767.615,5621299.383],[366772.494,5621290.439],[366796.78,5621246.681],[366823.722,5621202.048],[366853.357,5621155.819],[366880.875,5621115.276],[366907.249,5621078.193],[366934.776,5621041.907],[366963.241,5621005.689],[366993.652,5620969.352],[367025.979,5620931.625],[367057.575,5620896.7],[367087.142,5620864.132],[367118.163,5620830.734],[367148.467,5620798.886],[367179.351,5620767.019],[367202.956,5620742.546],[367227.495,5620717.955],[367260.12,5620685.308],[367291.801,5620654.448],[367324.655,5620622.972],[367357.383,5620592.71],[367377.096,5620575.223],[367403.764,5620553.208],[367422.115,5620538.873],[367427.211,5620534.892],[367454.774,5620514.608],[367486.677,5620493.069],[367519.058,5620471.57],[367550.687,5620450.382],[367571.903,5620436.709],[367596.18,5620421.499],[367626.011,5620403.639],[367658.21,5620385.341],[367691.424,5620367.382],[367725.476,5620350.033],[367737.729,5620344.098],[367759.129,5620333.731],[367794.603,5620317.584],[367840.114,5620298.169],[367879.281,5620282.834],[367920.084,5620267.748],[367954.501,5620256.019],[367963.1,5620252.533],[368033.086,5620429.78],[367991.622,5620446.159],[367898.543,5620482.929],[367827.555,5620510.946],[367820.526,5620513.744],[367813.51,5620516.574],[367806.507,5620519.435],[367794.965,5620524.235],[367783.459,5620529.123],[367771.991,5620534.096],[367760.56,5620539.156],[367749.168,5620544.302],[367737.815,5620549.534],[367726.502,5620554.851],[367715.229,5620560.253],[367703.999,5620565.739],[367692.81,5620571.31],[367681.663,5620576.965],[367670.56,5620582.704],[367659.5,5620588.526],[367648.484,5620594.431],[367637.513,5620600.42],[367626.587,5620606.49],[367615.706,5620612.641],[367604.871,5620618.874],[367594.084,5620625.188],[367583.344,5620631.584],[367572.653,5620638.06],[367562.011,5620644.617],[367551.419,5620651.254],[367540.877,5620657.97],[367530.386,5620664.763],[367519.946,5620671.636],[367509.558,5620678.586],[367499.223,5620685.615],[367488.941,5620692.721],[367478.713,5620699.905],[367468.539,5620707.165],[367458.42,5620714.502],[367448.356,5620721.915],[367438.349,5620729.403],[367428.398,5620736.966],[367418.504,5620744.604],[367408.668,5620752.316],[367398.89,5620760.102],[367389.171,5620767.962],[367379.512,5620775.894],[367369.913,5620783.9],[367360.374,5620791.979],[367350.897,5620800.129],[367341.481,5620808.351],[367332.128,5620816.643],[367322.837,5620825.006],[367313.61,5620833.438],[367304.446,5620841.94],[367295.347,5620850.511],[367286.314,5620859.151],[367277.345,5620867.859],[367268.442,5620876.634],[367259.606,5620885.475],[367250.837,5620894.383],[367242.134,5620903.357],[367233.5,5620912.396],[367224.933,5620921.499],[367216.434,5620930.667],[367208.005,5620939.898],[367199.646,5620949.193],[367191.357,5620958.55],[367183.139,5620967.97],[367174.992,5620977.451],[367166.916,5620986.993],[367158.914,5620996.596],[367150.984,5621006.259],[367143.127,5621015.981],[367135.344,5621025.762],[367127.634,5621035.602],[367119.999,5621045.499],[367112.439,5621055.454],[367104.954,5621065.465],[367097.545,5621075.532],[367090.212,5621085.654],[367082.956,5621095.831],[367075.776,5621106.062],[367068.674,5621116.347],[367061.649,5621126.685],[367054.702,5621137.076],[367047.834,5621147.519],[367041.045,5621158.014],[367034.335,5621168.559],[367027.704,5621179.155],[367021.154,5621189.8],[367014.684,5621200.494],[367008.294,5621211.237],[367001.986,5621222.027],[366995.759,5621232.865],[366990.361,5621242.412],[366985.026,5621251.995],[366979.755,5621261.613],[366974.547,5621271.266],[366969.287,5621281.182],[366964.093,5621291.133],[366958.967,5621301.118],[366953.909,5621311.139],[366948.919,5621321.193],[366943.441,5621332.428],[366938.047,5621343.703],[366932.738,5621355.019],[366927.515,5621366.375],[366922.378,5621377.769],[366917.326,5621389.202],[366912.361,5621400.673],[366907.482,5621412.181],[366902.69,5621423.726],[366897.985,5621435.306],[366893.367,5621446.922],[366888.837,5621458.572],[366884.395,5621470.256],[366880.041,5621481.972],[366875.775,5621493.722],[366871.598,5621505.503],[366867.51,5621517.315],[366863.51,5621529.157],[366859.601,5621541.029],[366855.78,5621552.93],[366852.049,5621564.859],[366848.409,5621576.816],[366844.858,5621588.8],[366841.398,5621600.811],[366837.848,5621613.495],[366834.399,5621626.206],[366831.051,5621638.944],[366827.804,5621651.709],[366824.659,5621664.498],[366821.615,5621677.313],[366818.673,5621690.151],[366817.022,5621697.562],[366795.268,5621795.112],[366773.518,5621892.663],[366751.774,5621990.21],[366730.029,5622087.773],[366708.289,5622185.329],[366686.534,5622282.887],[366664.784,5622380.443],[366643.053,5622478.009],[366621.315,5622575.563],[366599.582,5622673.108],[366577.84,5622770.646],[366556.11,5622868.18],[366534.374,5622965.712],[366512.635,5623063.244],[366490.871,5623160.795],[366469.121,5623258.343],[366447.368,5623355.894],[366425.622,5623453.456],[366403.883,5623550.997],[366382.129,5623648.527],[366360.37,5623746.05],[366356.847,5623761.848],[366354.186,5623773.546],[366351.429,5623785.221],[366348.576,5623796.873],[366345.628,5623808.502],[366342.583,5623820.106],[366339.444,5623831.684],[366336.209,5623843.236],[366332.74,5623855.234],[366329.169,5623867.202],[366325.495,5623879.139],[366321.72,5623891.044],[366317.843,5623902.917],[366313.864,5623914.756],[366309.784,5623926.56],[366305.604,5623938.329],[366301.323,5623950.065],[366296.941,5623961.763],[366292.459,5623973.424],[366287.878,5623985.046],[366283.197,5623996.628],[366278.418,5624008.17],[366273.539,5624019.671],[366268.563,5624031.129],[366263.488,5624042.545],[366258.316,5624053.917],[366253.047,5624065.245],[366247.681,5624076.527],[366242.218,5624087.763],[366236.66,5624098.952],[366231.006,5624110.092],[366225.257,5624121.184],[366219.413,5624132.227],[366213.474,5624143.219],[366207.442,5624154.16],[366201.316,5624165.049],[366195.097,5624175.885],[366188.786,5624186.667],[366182.382,5624197.395],[366175.887,5624208.068],[366169.3,5624218.685],[366162.623,5624229.245],[366155.856,5624239.747],[366148.999,5624250.191],[366142.053,5624260.576],[366135.018,5624270.902],[366127.895,5624281.167],[366120.685,5624291.37],[366113.388,5624301.512],[366106.005,5624311.591],[366098.535,5624321.607],[366090.98,5624331.558],[366083.34,5624341.444],[366075.616,5624351.265],[366067.807,5624361.018],[366059.916,5624370.705],[366051.943,5624380.322],[366043.888,5624389.87],[366035.751,5624399.349],[366027.534,5624408.758],[366019.236,5624418.097],[366010.859,5624427.364],[366002.403,5624436.559],[365993.868,5624445.681],[365987.532,5624452.331],[365981.154,5624458.941],[365974.734,5624465.51],[365974.026,5624466.232],[365973.318,5624466.954],[365972.608,5624467.674],[365972.054,5624468.238],[365971.102,5624469.204],[365970.149,5624470.168],[365969.195,5624471.132],[365967.229,5624473.113],[365965.259,5624475.09],[365963.285,5624477.063],[365962.28,5624478.065],[365961.274,5624479.065],[365960.267,5624480.065],[365958.361,5624481.952],[365956.451,5624483.836],[365954.538,5624485.717],[365953.583,5624486.653],[365952.627,5624487.588],[365951.67,5624488.522],[365951.445,5624488.742],[365951.219,5624488.962],[365950.993,5624489.181],[365950.339,5624489.818],[365949.685,5624490.455],[365949.03,5624491.092],[365942.543,5624497.369],[365936.017,5624503.605],[365929.453,5624509.801],[365922.851,5624515.957],[365913.64,5624524.394],[365904.358,5624532.752],[365895.004,5624541.031],[365885.579,5624549.229],[365876.085,5624557.346],[365866.522,5624565.381],[365856.89,5624573.334],[365847.191,5624581.205],[365837.424,5624588.992],[365827.59,5624596.695],[365817.691,5624604.314],[365807.727,5624611.848],[365797.699,5624619.297],[365787.607,5624626.659],[365777.453,5624633.934],[365767.237,5624641.123],[365756.961,5624648.223],[365746.624,5624655.235],[365736.228,5624662.158],[365725.773,5624668.992],[365715.26,5624675.736],[365704.689,5624682.39],[365694.062,5624688.954],[365683.379,5624695.426],[365673.41,5624701.352],[365663.394,5624707.2],[365653.332,5624712.967],[365643.225,5624718.654],[365633.073,5624724.261],[365622.876,5624729.786],[365612.636,5624735.231],[365602.353,5624740.594],[365600.097,5624741.764],[365597.84,5624742.933],[365595.582,5624744.101],[365580.985,5624752.155],[365566.35,5624760.138],[365551.676,5624768.05],[365536.964,5624775.891],[365522.214,5624783.66],[365507.426,5624791.357],[365493.067,5624799.833],[365478.668,5624808.24],[365464.227,5624816.577],[365449.747,5624824.843],[365435.226,5624833.039],[365420.666,5624841.165],[365406.559,5624850.056],[365392.41,5624858.878],[365378.218,5624867.631],[365363.983,5624876.316],[365349.707,5624884.931],[365335.389,5624893.477],[365321.548,5624902.773],[365307.663,5624912.002],[365293.733,5624921.163],[365279.758,5624930.257],[365265.74,5624939.282],[365251.678,5624948.24],[365238.116,5624957.933],[365224.507,5624967.559],[365210.851,5624977.12],[365197.15,5624986.614],[365183.402,5624996.042],[365169.609,5625005.403],[365156.335,5625015.485],[365143.012,5625025.503],[365129.64,5625035.456],[365116.221,5625045.344],[365102.754,5625055.167],[365089.239,5625064.924],[365076.263,5625075.387],[365063.237,5625085.788],[365050.16,5625096.125],[365037.034,5625106.398],[365023.858,5625116.608],[365010.632,5625126.754],[364997.964,5625137.591],[364985.243,5625148.366],[364972.471,5625159.079],[364959.646,5625169.731],[364946.77,5625180.32],[364933.843,5625190.846],[364921.227,5625201.744],[364908.664,5625212.703],[364896.154,5625223.723],[364883.698,5625234.803],[364871.296,5625245.944],[364858.948,5625257.145],[364846.655,5625268.406],[364834.416,5625279.727],[364822.233,5625291.107],[364810.104,5625302.546],[364798.032,5625314.044],[364786.015,5625325.6],[364781.962,5625329.523],[364777.915,5625333.452],[364773.874,5625337.388],[364761.774,5625348.679],[364749.733,5625360.034],[364737.753,5625371.452],[364725.834,5625382.934],[364713.976,5625394.479],[364701.455,5625405.434],[364688.993,5625416.457],[364676.59,5625427.545],[364664.246,5625438.699],[364651.962,5625449.92],[364639.738,5625461.205],[364626.873,5625471.753],[364614.065,5625482.37],[364601.313,5625493.055],[364588.619,5625503.808],[364575.982,5625514.628],[364563.403,5625525.516],[364550.207,5625535.656],[364537.065,5625545.867],[364523.978,5625556.147],[364510.946,5625566.497],[364497.97,5625576.917],[364485.049,5625587.406],[364471.535,5625597.12],[364458.073,5625606.906],[364444.663,5625616.764],[364431.306,5625626.693],[364418.003,5625636.694],[364404.753,5625646.765],[364391.186,5625656.396],[364377.567,5625665.954],[364363.898,5625675.439],[364350.179,5625684.851],[364336.409,5625694.19],[364322.59,5625703.456],[364318.488,5625706.159],[364314.382,5625708.856],[364310.271,5625711.547],[364300.091,5625718.312],[364238.51,5625758.568],[364154.84,5625813.267],[364071.182,5625867.972],[363987.513,5625922.684],[363903.852,5625977.392]]]}} \ No newline at end of file diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlMutationSession.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlMutationSession.java new file mode 100644 index 000000000..ab248a9ab --- /dev/null +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlMutationSession.java @@ -0,0 +1,537 @@ +/* + * 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 com.fasterxml.jackson.core.JsonParseException; +import com.google.common.collect.ImmutableList; +import de.ii.xtraplatform.base.domain.LogContext; +import de.ii.xtraplatform.crs.domain.CrsTransformerFactory; +import de.ii.xtraplatform.crs.domain.EpsgCrs; +import de.ii.xtraplatform.features.domain.FeatureTokenSource; +import de.ii.xtraplatform.features.domain.FeatureTransactions; +import de.ii.xtraplatform.features.domain.ImmutableMutationResult; +import de.ii.xtraplatform.features.domain.Tuple; +import de.ii.xtraplatform.features.sql.domain.FeatureTokenStatsCollector; +import de.ii.xtraplatform.features.sql.domain.SqlQueryMapping; +import de.ii.xtraplatform.features.sql.domain.SqlSession; +import de.ii.xtraplatform.streams.domain.Reactive; +import de.ii.xtraplatform.streams.domain.Reactive.Sink; +import de.ii.xtraplatform.streams.domain.Reactive.Source; +import de.ii.xtraplatform.streams.domain.Reactive.Transformer; +import java.time.ZoneId; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import java.util.function.Consumer; +import java.util.function.Supplier; +import java.util.stream.Collectors; +import org.postgresql.util.PSQLException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Synchronous {@link FeatureTransactions.Session} backed by a single JDBC connection. Reuses {@link + * FeatureMutationsSql} to derive insert/delete statements and executes them sequentially on the + * underlying {@link SqlSession} so that all mutations participate in one transaction. + */ +public class SqlMutationSession implements FeatureTransactions.Session { + + private static final Logger LOGGER = LoggerFactory.getLogger(SqlMutationSession.class); + + private final SqlSession sqlSession; + private final Map> queryMappings; + private final FeatureMutationsSql featureMutationsSql; + private final EpsgCrs nativeCrs; + private final CrsTransformerFactory crsTransformerFactory; + private final Optional nativeTimeZone; + private final Reactive.Runner streamRunner; + + public SqlMutationSession( + SqlSession sqlSession, + Map> queryMappings, + FeatureMutationsSql featureMutationsSql, + EpsgCrs nativeCrs, + CrsTransformerFactory crsTransformerFactory, + Optional nativeTimeZone, + Reactive.Runner streamRunner) { + this.sqlSession = sqlSession; + this.queryMappings = queryMappings; + this.featureMutationsSql = featureMutationsSql; + this.nativeCrs = nativeCrs; + this.crsTransformerFactory = crsTransformerFactory; + this.nativeTimeZone = nativeTimeZone; + this.streamRunner = streamRunner; + } + + @Override + public FeatureTransactions.MutationResult createFeatures( + String featureType, + FeatureTokenSource featureTokenSource, + EpsgCrs crs, + Optional featureId) { + return writeFeatures( + FeatureTransactions.MutationResult.Type.CREATE, + featureType, + featureTokenSource, + featureId, + crs, + false); + } + + // Multi-source CREATE: drains every source into one `collected` list against this session's + // single underlying transaction, then runs the existing cross-feature batched write. This is + // where the wfs:Insert N-feature win actually lands — the executor accumulates up to BATCH_SIZE + // per-feature sources and calls this overload, so writeFeaturesBatched sees collected.size()>1 + // and runMainInsertsGrouped can fold consecutive same-shape INSERTs into one multi-row INSERT. + @Override + public FeatureTransactions.MutationResult createFeatures( + String featureType, Iterable featureTokenSources, EpsgCrs crs) { + SqlQueryMapping mapping = requireMapping(featureType); + ImmutableMutationResult.Builder builder = + ImmutableMutationResult.builder() + .type(FeatureTransactions.MutationResult.Type.CREATE) + .hasFeatures(false); + + List collected = new ArrayList<>(); + try { + for (FeatureTokenSource src : featureTokenSources) { + drainSource(src, mapping, crs, builder, collected, false); + } + } catch (RuntimeException e) { + return builder.error(translateEncoderError(e)).build(); + } + + if (collected.isEmpty()) { + return builder.build(); + } + + RowCursor rowCursor = new RowCursor(mapping.getMainTable().getFullPath()); + Optional< + de.ii.xtraplatform.base.domain.util.Tuple< + de.ii.xtraplatform.features.sql.domain.SqlQuerySchema, + de.ii.xtraplatform.features.sql.domain.SqlQueryColumn>> + roleIdColumn = mapping.getColumnForId(); + String roleIdColumnName = roleIdColumn.map(t -> t.second().getName()).orElse(null); + de.ii.xtraplatform.features.sql.domain.SqlQuerySchema roleIdTable = + roleIdColumn.map(de.ii.xtraplatform.base.domain.util.Tuple::first).orElse(null); + + try { + writeFeaturesBatched( + collected, rowCursor, Optional.empty(), crs, roleIdTable, roleIdColumnName, builder); + } catch (RuntimeException e) { + builder.error(e); + } + + return builder.build(); + } + + @Override + public FeatureTransactions.MutationResult updateFeature( + String featureType, + String id, + FeatureTokenSource featureTokenSource, + EpsgCrs crs, + boolean partial) { + return writeFeatures( + partial + ? FeatureTransactions.MutationResult.Type.UPDATE + : FeatureTransactions.MutationResult.Type.REPLACE, + featureType, + featureTokenSource, + Optional.of(id), + crs, + partial); + } + + @Override + public FeatureTransactions.MutationResult deleteFeature(String featureType, String id) { + SqlQueryMapping mapping = requireMapping(featureType); + ImmutableMutationResult.Builder builder = + ImmutableMutationResult.builder() + .type(FeatureTransactions.MutationResult.Type.DELETE) + .hasFeatures(false); + try { + Supplier>> delete = + featureMutationsSql.createInstanceDelete(mapping, id); + Tuple> tuple = delete.get(); + String returned = + sqlSession.run( + ImmutableList.of(tuple::first), ImmutableList.of(tuple.second()), Optional.empty()); + if (returned != null) { + builder.addIds(returned); + } + } catch (RuntimeException e) { + builder.error(e); + } + return builder.build(); + } + + @Override + public void commit() { + sqlSession.commit(); + } + + @Override + public void rollback() { + sqlSession.rollback(); + } + + @Override + public void close() { + sqlSession.close(); + } + + private FeatureTransactions.MutationResult writeFeatures( + FeatureTransactions.MutationResult.Type type, + String featureType, + FeatureTokenSource featureTokenSource, + Optional featureId, + EpsgCrs crs, + boolean partial) { + SqlQueryMapping mapping = requireMapping(featureType); + + ImmutableMutationResult.Builder builder = + ImmutableMutationResult.builder().type(type).hasFeatures(false); + + List collected = new ArrayList<>(); + try { + drainSource(featureTokenSource, mapping, crs, builder, collected, partial); + } catch (RuntimeException e) { + return builder.error(translateEncoderError(e)).build(); + } + + RowCursor rowCursor = new RowCursor(mapping.getMainTable().getFullPath()); + boolean deleteFirst = + type == FeatureTransactions.MutationResult.Type.UPDATE + || type == FeatureTransactions.MutationResult.Type.REPLACE; + + // Role-id column on the main table — its value in the inserted feature is the externally + // visible feature id (e.g. ALKIS gml:id stored in 'objid'); fall back to the surrogate PK only + // when no role-id column / no value is present. + Optional< + de.ii.xtraplatform.base.domain.util.Tuple< + de.ii.xtraplatform.features.sql.domain.SqlQuerySchema, + de.ii.xtraplatform.features.sql.domain.SqlQueryColumn>> + roleIdColumn = mapping.getColumnForId(); + String roleIdColumnName = roleIdColumn.map(t -> t.second().getName()).orElse(null); + de.ii.xtraplatform.features.sql.domain.SqlQuerySchema roleIdTable = + roleIdColumn.map(de.ii.xtraplatform.base.domain.util.Tuple::first).orElse(null); + + try { + if (deleteFirst) { + writeFeaturesPerFeature( + collected, rowCursor, featureId, crs, true, roleIdTable, roleIdColumnName, builder); + } else { + writeFeaturesBatched( + collected, rowCursor, featureId, crs, roleIdTable, roleIdColumnName, builder); + } + } catch (RuntimeException e) { + builder.error(e); + } + + return builder.build(); + } + + // Assembles the token-source → FeatureEncoderSql pipeline and drains it into `collected`. A + // fresh FeatureTokenStatsCollector is instantiated per source so internal transformer state + // doesn't leak across runs; the result builder is shared, so accumulated stats (bbox, temporal + // extent) overwrite per source — matching the existing single-source-multi-feature semantics + // where the last feature's stats win. + private void drainSource( + FeatureTokenSource featureTokenSource, + SqlQueryMapping mapping, + EpsgCrs crs, + ImmutableMutationResult.Builder builder, + List collected, + boolean partial) { + FeatureTokenStatsCollector statsCollector = new FeatureTokenStatsCollector(builder, crs); + + Source featureSqlSource = + featureTokenSource + .via(statsCollector) + .via( + new FeatureEncoderSql( + mapping, + crs, + nativeCrs, + crsTransformerFactory, + nativeTimeZone, + partial ? Optional.of(FeatureTransactions.PATCH_NULL_VALUE) : Optional.empty())) + .via(Transformer.map(feature -> (FeatureDataSql) feature)); + + if (partial) { + featureSqlSource = + featureSqlSource.via( + Transformer.reduce( + ModifiableFeatureDataSql.create(), + (a, b) -> a.getRows().isEmpty() ? b : a.patchWith(b))); + } + + featureSqlSource + .to(Sink.foreach(collected::add)) + .on(streamRunner) + .run() + .toCompletableFuture() + .join(); + } + + // Original per-feature loop, kept for the UPDATE/REPLACE path (each feature is preceded by a + // DELETE so its statements can't be merged with another feature's at the SQL level). + private void writeFeaturesPerFeature( + List collected, + RowCursor rowCursor, + Optional featureId, + EpsgCrs crs, + boolean deleteFirst, + de.ii.xtraplatform.features.sql.domain.SqlQuerySchema roleIdTable, + String roleIdColumnName, + ImmutableMutationResult.Builder builder) { + for (FeatureDataSql feature : collected) { + // When the role-id value is already in the feature row (e.g. ALKIS gml:id parsed into + // 'objid'), don't pass it to createInstanceInserts — that path injects/quotes it again as + // the id column value, producing duplicate-quoted SQL. We still pass it to sqlSession.run + // so the returned id is the role-id rather than the surrogate PK. + Optional roleIdFromRow = extractRoleId(feature, roleIdTable, roleIdColumnName); + Optional effectiveFeatureId = featureId.or(() -> roleIdFromRow); + List>>> tuples = + featureMutationsSql.createInstanceInserts( + feature, rowCursor, featureId, crs, deleteFirst); + List> statements = + tuples.stream() + .map( + t -> + (Supplier) + () -> { + Tuple> v = t.get(); + return v.first(); + }) + .filter(Objects::nonNull) + .collect(Collectors.toList()); + List> idConsumers = + tuples.stream().map(t -> t.get().second()).collect(Collectors.toList()); + String id = sqlSession.run(statements, idConsumers, effectiveFeatureId); + if (id != null) { + builder.addIds(id); + } + } + } + + // CREATE path: materialize every feature's statement list upfront, fold consecutive main + // INSERTs that share the same "INSERT INTO t (...) VALUES " prefix and " RETURNING ;" suffix + // into one multi-row INSERT per group, distribute the returned PKs back to each feature's + // consumer (so child statements can read them from currentRow.ids), then send all children + // across all features through a single sqlSession.run — letting the existing JDBC batch path + // collapse the entire tail into one executeBatch. + private void writeFeaturesBatched( + List collected, + RowCursor rowCursor, + Optional featureId, + EpsgCrs crs, + de.ii.xtraplatform.features.sql.domain.SqlQuerySchema roleIdTable, + String roleIdColumnName, + ImmutableMutationResult.Builder builder) { + int n = collected.size(); + List> effectiveIds = new ArrayList<>(n); + String[] mainSqls = new String[n]; + Consumer[] mainConsumersRaw = new Consumer[n]; + List>>>> childTuples = new ArrayList<>(n); + + for (int i = 0; i < n; i++) { + FeatureDataSql feature = collected.get(i); + Optional roleIdFromRow = extractRoleId(feature, roleIdTable, roleIdColumnName); + effectiveIds.add(featureId.or(() -> roleIdFromRow)); + + List>>> tuples = + featureMutationsSql.createInstanceInserts(feature, rowCursor, featureId, crs, false); + + Tuple> main = tuples.isEmpty() ? null : tuples.get(0).get(); + mainSqls[i] = main == null ? null : main.first(); + mainConsumersRaw[i] = main == null ? null : main.second(); + childTuples.add(tuples.isEmpty() ? List.of() : tuples.subList(1, tuples.size())); + } + + String[] returnedPks = new String[n]; + + runMainInsertsGrouped(mainSqls, mainConsumersRaw, returnedPks); + + // Publish the per-feature ids as soon as the main inserts have returned; if the children + // phase then fails, the result still carries every id that made it into the transaction + // (the surrounding executor decides whether to roll back). + for (int i = 0; i < n; i++) { + String id = effectiveIds.get(i).orElse(returnedPks[i]); + if (id != null) { + builder.addIds(id); + } + } + + runChildren(childTuples); + } + + // Groups consecutive features whose main-insert SQL shares an "INSERT INTO t (...) VALUES " + // prefix and a " RETURNING ;" suffix into one combined multi-row INSERT, executed via + // sqlSession.runReturning so we receive every generated PK in insertion order. Features whose + // main SQL doesn't fit the shape (e.g. DEFAULT VALUES, or a different table) are flushed as a + // single-row insert via the same path; null-SQL slots are skipped entirely. + private void runMainInsertsGrouped( + String[] mainSqls, Consumer[] consumersRaw, String[] returnedPks) { + int n = mainSqls.length; + int i = 0; + while (i < n) { + if (mainSqls[i] == null) { + i++; + continue; + } + MainInsertParts head = splitMainInsert(mainSqls[i]); + if (head == null) { + // SQL doesn't match the standard shape — execute on its own. + executeAndDispatch(mainSqls[i], consumersRaw, returnedPks, new int[] {i}); + i++; + continue; + } + List groupIdx = new ArrayList<>(); + List valuesTuples = new ArrayList<>(); + groupIdx.add(i); + valuesTuples.add(head.values); + int j = i + 1; + while (j < n) { + if (mainSqls[j] == null) { + break; + } + MainInsertParts next = splitMainInsert(mainSqls[j]); + if (next == null || !head.prefix.equals(next.prefix) || !head.suffix.equals(next.suffix)) { + break; + } + groupIdx.add(j); + valuesTuples.add(next.values); + j++; + } + String combined = head.prefix + String.join(",", valuesTuples) + head.suffix; + int[] indices = groupIdx.stream().mapToInt(Integer::intValue).toArray(); + executeAndDispatch(combined, consumersRaw, returnedPks, indices); + i = j; + } + } + + @SuppressWarnings("unchecked") + private void executeAndDispatch( + String sql, Consumer[] consumersRaw, String[] returnedPks, int[] featureIndices) { + List ids = sqlSession.runReturning(sql); + for (int k = 0; k < featureIndices.length; k++) { + String returned = k < ids.size() ? ids.get(k) : null; + int featureIdx = featureIndices[k]; + returnedPks[featureIdx] = returned; + Consumer c = (Consumer) consumersRaw[featureIdx]; + if (c != null) { + c.accept(returned); + } + } + } + + private void runChildren(List>>>> childTuples) { + List> statements = new ArrayList<>(); + List> idConsumers = new ArrayList<>(); + for (List>>> tuples : childTuples) { + for (Supplier>> t : tuples) { + statements.add( + () -> { + Tuple> v = t.get(); + return v.first(); + }); + idConsumers.add(t.get().second()); + } + } + if (statements.isEmpty()) { + return; + } + sqlSession.run(statements, idConsumers, Optional.empty()); + } + + // Splits "INSERT INTO t (...) VALUES (...) RETURNING ;" into prefix (ending with "VALUES "), + // values tuple (the parenthesised values, including the outer parens), and suffix (starting with + // " RETURNING "). Returns null when the SQL doesn't match — e.g. DEFAULT VALUES, or some other + // shape — so the caller can fall back to a single-row execution. + private static MainInsertParts splitMainInsert(String sql) { + int retIdx = sql.lastIndexOf(" RETURNING "); + if (retIdx < 0 || !sql.endsWith(";")) { + return null; + } + String suffix = sql.substring(retIdx); + String body = sql.substring(0, retIdx); + int valIdx = body.lastIndexOf(" VALUES ("); + if (valIdx < 0 || !body.endsWith(")")) { + return null; + } + int prefixEnd = valIdx + " VALUES ".length(); + String prefix = sql.substring(0, prefixEnd); + String values = body.substring(prefixEnd); + return new MainInsertParts(prefix, values, suffix); + } + + private static final class MainInsertParts { + final String prefix; + final String values; + final String suffix; + + MainInsertParts(String prefix, String values, String suffix) { + this.prefix = prefix; + this.values = values; + this.suffix = suffix; + } + } + + private static Optional extractRoleId( + FeatureDataSql feature, + de.ii.xtraplatform.features.sql.domain.SqlQuerySchema roleIdTable, + String roleIdColumnName) { + if (roleIdTable == null || roleIdColumnName == null) { + return Optional.empty(); + } + return feature.getRows().stream() + .filter(r -> Objects.equals(r.first(), roleIdTable)) + .findFirst() + .map(r -> r.second().getValues().get(roleIdColumnName)) + .map(SqlMutationSession::unquoteSqlLiteral); + } + + /** + * Strips the SQL string-literal wrapping that {@link FeatureDataSql} stores in row values (text + * values land in the map as {@code 'foo''bar'}, numerics/booleans as-is). Returns the inner value + * with doubled single quotes collapsed; non-quoted inputs are returned unchanged. + */ + private static String unquoteSqlLiteral(String value) { + if (value == null || value.length() < 2) { + return value; + } + if (value.charAt(0) == '\'' && value.charAt(value.length() - 1) == '\'') { + return value.substring(1, value.length() - 1).replace("''", "'"); + } + return value; + } + + private SqlQueryMapping requireMapping(String featureType) { + List mappings = queryMappings.get(featureType); + if (mappings == null || mappings.isEmpty()) { + throw new IllegalArgumentException( + String.format("Feature type '%s' not found.", featureType)); + } + return mappings.get(0); + } + + private Throwable translateEncoderError(Throwable throwable) { + Throwable cause = throwable instanceof RuntimeException ? throwable.getCause() : throwable; + if (cause instanceof PSQLException || cause instanceof JsonParseException) { + LogContext.errorAsDebug(LOGGER, cause, "Error during feature mutation"); + return new IllegalArgumentException( + "Invalid feature data. You may be able to obtain more information about the problem by adding the header 'Prefer: handling=strict' to the request.", + cause); + } + return throwable; + } +} diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/FeatureProviderSql.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/FeatureProviderSql.java index 4274a15a0..83d6d0eac 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/FeatureProviderSql.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/FeatureProviderSql.java @@ -96,6 +96,7 @@ import de.ii.xtraplatform.features.sql.app.QuerySchemaDeriver; import de.ii.xtraplatform.features.sql.app.SqlInsertGenerator2; import de.ii.xtraplatform.features.sql.app.SqlMappingDeriver; +import de.ii.xtraplatform.features.sql.app.SqlMutationSession; import de.ii.xtraplatform.features.sql.app.SqlQueryTemplates; import de.ii.xtraplatform.features.sql.app.SqlQueryTemplatesDeriver; import de.ii.xtraplatform.features.sql.domain.FeatureProviderSqlData.QueryGeneratorSettings; @@ -1251,6 +1252,18 @@ public MutationResult updateFeature( partial); } + @Override + public Session openSession() { + return new SqlMutationSession( + getSqlClient().openSession(), + queryMappings, + featureMutationsSql, + getNativeCrs(), + crsTransformerFactory, + getData().getNativeTimeZone(), + getStreamRunner()); + } + @Override public MutationResult deleteFeature(String featureType, String id) { Optional> queryMapping = diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlClient.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlClient.java index 5d6d604e6..c0a79aad3 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlClient.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlClient.java @@ -39,4 +39,13 @@ Transformer getMutationFlow( Optional id); List getNotifications(Connection connection); + + /** + * Opens a synchronous, single-connection session for multi-statement transactions. The default + * implementation throws {@link UnsupportedOperationException}. + */ + default SqlSession openSession() { + throw new UnsupportedOperationException( + "Synchronous SQL sessions are not supported by this SqlClient implementation"); + } } diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlSession.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlSession.java new file mode 100644 index 000000000..a98e210d1 --- /dev/null +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlSession.java @@ -0,0 +1,57 @@ +/* + * 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.domain; + +import java.util.List; +import java.util.Optional; +import java.util.function.Consumer; +import java.util.function.Supplier; + +/** + * Synchronous, single-connection SQL session that executes mutation statements against one + * underlying JDBC transaction. {@link #commit()} makes the changes durable; {@link #close()} + * without a prior {@code commit()} rolls back. + */ +public interface SqlSession extends AutoCloseable { + + /** + * Executes the given statements in order on this session's connection. Each statement is expected + * to return a single string column (typically a {@code RETURNING} clause); the value is passed to + * the corresponding entry in {@code idConsumers}. + * + * @param statements lazily evaluated SQL statements + * @param idConsumers consumers receiving the returned id, one per statement + * @param featureId optional caller-supplied id; if absent, the first generated id is returned + * @return the feature id (either {@code featureId} or the first generated id) + */ + String run( + List> statements, + List> idConsumers, + Optional featureId); + + /** + * Executes a single SQL statement and returns every value in the first column of the result set, + * preserving the database's row order. Intended for multi-row {@code INSERT ... RETURNING} + * statements where the caller needs every generated id, not just the first. + * + * @param sql the statement to execute + * @return one entry per result-set row (the first column, stringified); empty if the statement + * produces no result set + */ + List runReturning(String sql); + + /** Commits all mutations performed against this session. */ + void commit(); + + /** Rolls back all mutations performed against this session. Idempotent. */ + void rollback(); + + /** Equivalent to {@link #rollback()} if {@link #commit()} has not been called. Idempotent. */ + @Override + void close(); +} diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/JdbcSqlSession.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/JdbcSqlSession.java new file mode 100644 index 000000000..3ede3b506 --- /dev/null +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/JdbcSqlSession.java @@ -0,0 +1,250 @@ +/* + * 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.infra.db; + +import de.ii.xtraplatform.base.domain.LogContext.MARKER; +import de.ii.xtraplatform.features.sql.domain.SqlSession; +import java.sql.Connection; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.sql.Statement; +import java.util.ArrayList; +import java.util.List; +import java.util.Locale; +import java.util.Objects; +import java.util.Optional; +import java.util.function.Consumer; +import java.util.function.Supplier; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +class JdbcSqlSession implements SqlSession { + + private static final Logger LOGGER = LoggerFactory.getLogger(JdbcSqlSession.class); + + // Safety cap on accumulated batch size — pathological feature shouldn't grow unbounded. + private static final int MAX_BATCH_SIZE = 1000; + + private final Connection connection; + private boolean finalised; + + JdbcSqlSession(Connection connection) { + this.connection = connection; + this.finalised = false; + try { + connection.setAutoCommit(false); + } catch (SQLException e) { + try { + connection.close(); + } catch (SQLException ignore) { + // suppressed + } + throw new IllegalStateException("Could not start SQL session", e); + } + } + + @Override + public String run( + List> statements, + List> idConsumers, + Optional featureId) { + if (finalised) { + throw new IllegalStateException("SQL session is closed"); + } + String firstGeneratedId = null; + Statement batchStmt = null; + List batchedSql = new ArrayList<>(); + List> batchedConsumers = new ArrayList<>(); + + try { + for (int i = 0; i < statements.size(); i++) { + String sql = statements.get(i).get(); + if (Objects.isNull(sql)) { + continue; + } + Consumer consumer = i < idConsumers.size() ? idConsumers.get(i) : null; + + if (isBatchable(sql)) { + try { + if (batchStmt == null) { + batchStmt = connection.createStatement(); + } + batchStmt.addBatch(sql); + } catch (SQLException e) { + throw new IllegalStateException( + "Mutation statement failed: " + e.getMessage() + " — statement: " + sql, e); + } + batchedSql.add(sql); + batchedConsumers.add(consumer); + if (batchedSql.size() >= MAX_BATCH_SIZE) { + flushBatch(batchStmt, batchedSql, batchedConsumers); + } + continue; + } + + // Non-batchable: flush any pending batch first so ordering is preserved. + if (!batchedSql.isEmpty()) { + flushBatch(batchStmt, batchedSql, batchedConsumers); + } + + if (LOGGER.isDebugEnabled(MARKER.SQL)) { + LOGGER.debug(MARKER.SQL, "Executing statement: {}", sql); + } + try (Statement statement = connection.createStatement()) { + boolean hasResultSet = statement.execute(sql); + String returnedId = null; + if (hasResultSet) { + try (ResultSet rs = statement.getResultSet()) { + if (rs.next()) { + returnedId = rs.getString(1); + } + } + } + if (consumer != null) { + consumer.accept(returnedId); + } + if (firstGeneratedId == null && returnedId != null) { + firstGeneratedId = returnedId; + } + } catch (SQLException e) { + throw new IllegalStateException( + "Mutation statement failed: " + e.getMessage() + " — statement: " + sql, e); + } + } + + if (!batchedSql.isEmpty()) { + flushBatch(batchStmt, batchedSql, batchedConsumers); + } + } finally { + if (batchStmt != null) { + try { + batchStmt.close(); + } catch (SQLException ignore) { + // suppressed + } + } + } + return featureId.orElse(firstGeneratedId); + } + + @Override + public List runReturning(String sql) { + if (finalised) { + throw new IllegalStateException("SQL session is closed"); + } + if (LOGGER.isDebugEnabled(MARKER.SQL)) { + LOGGER.debug(MARKER.SQL, "Executing statement: {}", sql); + } + try (Statement statement = connection.createStatement()) { + boolean hasResultSet = statement.execute(sql); + if (!hasResultSet) { + return List.of(); + } + List ids = new ArrayList<>(); + try (ResultSet rs = statement.getResultSet()) { + while (rs.next()) { + ids.add(rs.getString(1)); + } + } + return ids; + } catch (SQLException e) { + throw new IllegalStateException( + "Mutation statement failed: " + e.getMessage() + " — statement: " + sql, e); + } + } + + // Generators emit "RETURNING null" for child / junction / FK-update statements — these have + // no caller-meaningful return value and their consumers are no-ops. Main inserts use + // "RETURNING " and must run individually so their generated id can drive child SQL. + private static boolean isBatchable(String sql) { + String trimmed = sql.trim(); + if (trimmed.endsWith(";")) { + trimmed = trimmed.substring(0, trimmed.length() - 1).trim(); + } + return trimmed.toLowerCase(Locale.ROOT).endsWith("returning null"); + } + + private void flushBatch( + Statement batchStmt, List batchedSql, List> batchedConsumers) { + if (batchedSql.isEmpty()) { + return; + } + if (LOGGER.isDebugEnabled(MARKER.SQL)) { + LOGGER.debug( + MARKER.SQL, "Executing batched statements ({}): {}", batchedSql.size(), batchedSql); + } + try { + batchStmt.executeBatch(); + batchStmt.clearBatch(); + } catch (SQLException e) { + throw new IllegalStateException( + "Batched mutation failed: " + e.getMessage() + " — first statement: " + batchedSql.get(0), + e); + } + // Preserve the per-statement consumer contract: each batched statement returns no id, so + // call consumers with null in order (in practice these are no-ops for child/junction/FK + // generators, but the contract allows any Consumer). + for (Consumer c : batchedConsumers) { + if (c != null) { + c.accept(null); + } + } + batchedSql.clear(); + batchedConsumers.clear(); + } + + @Override + public void commit() { + if (finalised) { + throw new IllegalStateException("SQL session is closed"); + } + try { + connection.commit(); + finalised = true; + } catch (SQLException e) { + throw new IllegalStateException("Commit failed: " + e.getMessage(), e); + } finally { + releaseConnection(); + } + } + + @Override + public void rollback() { + if (finalised) { + return; + } + try { + connection.rollback(); + } catch (SQLException e) { + LOGGER.warn("Rollback failed: {}", e.getMessage()); + } finally { + finalised = true; + releaseConnection(); + } + } + + @Override + public void close() { + if (!finalised) { + rollback(); + } else { + releaseConnection(); + } + } + + private void releaseConnection() { + try { + if (!connection.isClosed()) { + connection.setAutoCommit(true); + connection.close(); + } + } catch (SQLException e) { + LOGGER.debug("Connection close failed: {}", e.getMessage()); + } + } +} diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/SqlClientRx.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/SqlClientRx.java index f228c229a..39693d796 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/SqlClientRx.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/SqlClientRx.java @@ -17,6 +17,7 @@ import de.ii.xtraplatform.features.sql.domain.SqlDialect; import de.ii.xtraplatform.features.sql.domain.SqlQueryOptions; import de.ii.xtraplatform.features.sql.domain.SqlRow; +import de.ii.xtraplatform.features.sql.domain.SqlSession; import de.ii.xtraplatform.streams.domain.Reactive; import de.ii.xtraplatform.streams.domain.Reactive.Transformer; import io.reactivex.rxjava3.core.Flowable; @@ -316,6 +317,12 @@ public Connection getConnection() { return session.connection().blockingGet(); } + @Override + public SqlSession openSession() { + Connection connection = session.connection().blockingGet(); + return new JdbcSqlSession(connection); + } + @Override public SqlDialect getSqlDialect() { return dialect; diff --git a/xtraplatform-features-sql/src/test/groovy/de/ii/xtraplatform/features/sql/app/SqlMutationSessionSpec.groovy b/xtraplatform-features-sql/src/test/groovy/de/ii/xtraplatform/features/sql/app/SqlMutationSessionSpec.groovy new file mode 100644 index 000000000..6bba391d8 --- /dev/null +++ b/xtraplatform-features-sql/src/test/groovy/de/ii/xtraplatform/features/sql/app/SqlMutationSessionSpec.groovy @@ -0,0 +1,95 @@ +/* + * 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.features.sql.domain.SqlSession +import spock.lang.Specification + +/** + * Locks the two narrow contracts the {@code ogcapi-transactions} executor relies on: + * + *
    + *
  • {@link SqlMutationSession#commit()}, {@link SqlMutationSession#rollback()} and + * {@link SqlMutationSession#close()} are pass-throughs to the underlying SQL session — the + * executor's atomic / batch flow assumes nothing else happens at that layer. + *
  • An unknown {@code featureType} fails fast with {@code IllegalArgumentException} before any + * SQL is generated, so a malformed transaction action can't slip through and execute + * arbitrary mutator code. + *
+ * + * The full insert/replace mutation path (driven via {@code FeatureMutationsSql} + + * {@code FeatureEncoderSql} + a Reactive stream runner) needs a heavier fixture and is exercised + * end-to-end via the gvd/alkis transactions smoke configuration. + */ +class SqlMutationSessionSpec extends Specification { + + SqlSession sqlSession + + def setup() { + sqlSession = Mock(SqlSession) + } + + private SqlMutationSession buildSession(Map mappings = [:]) { + // FeatureMutationsSql, CrsTransformerFactory, nativeCrs, nativeTimeZone and the Reactive + // runner are only touched by the create/update code paths; null is safe for the lifecycle + // and missing-type cases this spec covers. + return new SqlMutationSession(sqlSession, mappings, null, null, null, Optional.empty(), null) + } + + def 'commit delegates to the underlying SqlSession'() { + given: + def session = buildSession() + + when: + session.commit() + + then: + 1 * sqlSession.commit() + 0 * sqlSession.rollback() + 0 * sqlSession.close() + } + + def 'rollback delegates to the underlying SqlSession'() { + given: + def session = buildSession() + + when: + session.rollback() + + then: + 1 * sqlSession.rollback() + 0 * sqlSession.commit() + 0 * sqlSession.close() + } + + def 'close delegates to the underlying SqlSession'() { + given: + def session = buildSession() + + when: + session.close() + + then: + 1 * sqlSession.close() + 0 * sqlSession.commit() + 0 * sqlSession.rollback() + } + + def 'deleteFeature for an unknown feature type fails fast with IAE (no SQL is run)'() { + given: + def session = buildSession([:]) // empty mappings + + when: + session.deleteFeature('unknown_type', '42') + + then: + def ex = thrown(IllegalArgumentException) + ex.message.contains('unknown_type') + 0 * sqlSession.run(_, _, _) + } +} diff --git a/xtraplatform-features-sql/src/test/groovy/de/ii/xtraplatform/features/sql/infra/db/JdbcSqlSessionSpec.groovy b/xtraplatform-features-sql/src/test/groovy/de/ii/xtraplatform/features/sql/infra/db/JdbcSqlSessionSpec.groovy new file mode 100644 index 000000000..1a1b3c041 --- /dev/null +++ b/xtraplatform-features-sql/src/test/groovy/de/ii/xtraplatform/features/sql/infra/db/JdbcSqlSessionSpec.groovy @@ -0,0 +1,312 @@ +/* + * 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.infra.db + +import spock.lang.Specification + +import java.sql.Connection +import java.sql.ResultSet +import java.sql.SQLException +import java.sql.Statement +import java.util.function.Consumer +import java.util.function.Supplier + +/** + * Locks the JDBC-level session that backs every {@code FeatureTransactions.Session}: it owns the + * connection's auto-commit lifecycle, dispatches generated ids back to per-statement consumers, + * and guarantees idempotent commit/rollback/close semantics. + * + *

Why this matters: the executor in ogcapi-transactions relies on rollback being safe to call + * after a failed action, and on close() not double-rolling back a session that already committed. + * A regression here would silently leak connections or roll back already-committed work. + */ +class JdbcSqlSessionSpec extends Specification { + + Connection connection + Statement statement + ResultSet resultSet + + def setup() { + connection = Mock(Connection) + statement = Mock(Statement) + resultSet = Mock(ResultSet) + connection.isClosed() >> false + connection.createStatement() >> statement + } + + def 'constructor flips autoCommit off so all subsequent statements join one transaction'() { + when: + new JdbcSqlSession(connection) + + then: + 1 * connection.setAutoCommit(false) + } + + def 'constructor failure (setAutoCommit throws) closes the connection and surfaces the cause'() { + given: + connection.setAutoCommit(false) >> { throw new SQLException('boom') } + + when: + new JdbcSqlSession(connection) + + then: + thrown(IllegalStateException) + 1 * connection.close() + } + + def 'run executes each non-null statement, captures RETURNING id, dispatches to per-statement consumers'() { + given: + def session = new JdbcSqlSession(connection) + def captured = [] + Consumer cons1 = { captured << ['s1', it] } + Consumer cons2 = { captured << ['s2', it] } + statement.execute('INSERT 1 RETURNING id') >> true + statement.execute('INSERT 2 RETURNING id') >> true + statement.getResultSet() >>> [resultSet, resultSet] + resultSet.next() >>> [true, true] + resultSet.getString(1) >>> ['gen-1', 'gen-2'] + + when: + def returnedId = session.run( + [{ 'INSERT 1 RETURNING id' } as Supplier, + { 'INSERT 2 RETURNING id' } as Supplier], + [cons1, cons2], + Optional.empty()) + + then: + captured == [['s1', 'gen-1'], ['s2', 'gen-2']] + + and: 'with no caller-supplied id the first generated id is returned' + returnedId == 'gen-1' + } + + def 'run returns the caller-supplied featureId when present, even if the DB generated different ids'() { + given: + def session = new JdbcSqlSession(connection) + statement.execute(_ as String) >> true + statement.getResultSet() >> resultSet + resultSet.next() >> true + resultSet.getString(1) >> 'gen-42' + + when: + def id = session.run( + [{ 'INSERT ... RETURNING id' } as Supplier], + [{ String s -> } as Consumer], + Optional.of('caller-7')) + + then: + id == 'caller-7' + } + + def 'run skips statements that evaluate to null (Supplier returns null)'() { + given: + def session = new JdbcSqlSession(connection) + + when: + session.run( + [{ (String) null } as Supplier], + [{ String s -> } as Consumer], + Optional.empty()) + + then: 'no statement is ever executed' + 0 * connection.createStatement() + } + + def 'run wraps SQLException from statement.execute as IllegalStateException with statement text'() { + given: + def session = new JdbcSqlSession(connection) + statement.execute('BAD SQL') >> { throw new SQLException('syntax') } + + when: + session.run( + [{ 'BAD SQL' } as Supplier], + [{ String s -> } as Consumer], + Optional.empty()) + + then: + def ex = thrown(IllegalStateException) + ex.message.contains('BAD SQL') + ex.message.contains('syntax') + } + + def 'consecutive RETURNING null statements are batched and consumers are called with null'() { + given: + def session = new JdbcSqlSession(connection) + def captured = [] + Consumer c1 = { captured << ['c1', it] } + Consumer c2 = { captured << ['c2', it] } + + when: + session.run( + [{ 'INSERT INTO child1 VALUES (...) RETURNING null;' } as Supplier, + { 'INSERT INTO child2 VALUES (...) RETURNING null;' } as Supplier], + [c1, c2], + Optional.empty()) + + then: 'a single batch is executed, not two individual executes' + 1 * statement.addBatch('INSERT INTO child1 VALUES (...) RETURNING null;') + 1 * statement.addBatch('INSERT INTO child2 VALUES (...) RETURNING null;') + 1 * statement.executeBatch() + 0 * statement.execute(_ as String) + + and: 'both consumers receive null' + captured == [['c1', null], ['c2', null]] + } + + def 'a non-batchable statement flushes the pending batch first, then runs individually'() { + given: + def session = new JdbcSqlSession(connection) + statement.execute('UPDATE feat SET x=1 RETURNING id;') >> true + statement.getResultSet() >> resultSet + resultSet.next() >> true + resultSet.getString(1) >> 'gen-9' + + when: + session.run( + [{ 'INSERT INTO child VALUES (...) RETURNING null;' } as Supplier, + { 'UPDATE feat SET x=1 RETURNING id;' } as Supplier], + [{ String s -> } as Consumer, + { String s -> } as Consumer], + Optional.empty()) + + then: 'pending batch runs first, then the non-batchable statement runs individually' + 1 * statement.addBatch('INSERT INTO child VALUES (...) RETURNING null;') + 1 * statement.executeBatch() + 1 * statement.execute('UPDATE feat SET x=1 RETURNING id;') + } + + def 'batch is flushed at end of run even if last statement is batched'() { + given: + def session = new JdbcSqlSession(connection) + + when: + session.run( + [{ 'INSERT INTO child VALUES (...) RETURNING null;' } as Supplier], + [{ String s -> } as Consumer], + Optional.empty()) + + then: + 1 * statement.addBatch('INSERT INTO child VALUES (...) RETURNING null;') + 1 * statement.executeBatch() + } + + def 'RETURNING null detection ignores trailing semicolon and is case-insensitive'() { + given: + def session = new JdbcSqlSession(connection) + + when: + session.run( + [{ 'INSERT INTO a VALUES (1) returning NULL ' } as Supplier, + { 'INSERT INTO b VALUES (2) RETURNING NULL;' } as Supplier], + [{ String s -> } as Consumer, + { String s -> } as Consumer], + Optional.empty()) + + then: + 1 * statement.addBatch('INSERT INTO a VALUES (1) returning NULL ') + 1 * statement.addBatch('INSERT INTO b VALUES (2) RETURNING NULL;') + 1 * statement.executeBatch() + 0 * statement.execute(_ as String) + } + + def 'SQLException during executeBatch is wrapped as IllegalStateException with the first statement text'() { + given: + def session = new JdbcSqlSession(connection) + statement.executeBatch() >> { throw new SQLException('constraint violated') } + + when: + session.run( + [{ 'INSERT INTO child VALUES (1) RETURNING null;' } as Supplier], + [{ String s -> } as Consumer], + Optional.empty()) + + then: + def ex = thrown(IllegalStateException) + ex.message.contains('INSERT INTO child VALUES (1) RETURNING null;') + ex.message.contains('constraint violated') + } + + def 'commit calls connection.commit and releases the connection'() { + given: + def session = new JdbcSqlSession(connection) + + when: + session.commit() + + then: + 1 * connection.commit() + 1 * connection.setAutoCommit(true) + 1 * connection.close() + } + + def 'commit after finalisation throws (no double commit)'() { + given: + def session = new JdbcSqlSession(connection) + session.commit() + + when: + session.commit() + + then: + thrown(IllegalStateException) + } + + def 'rollback is idempotent after commit (no rollback is issued on the connection)'() { + given: + def session = new JdbcSqlSession(connection) + session.commit() + + when: + session.rollback() + + then: + 0 * connection.rollback() + } + + def 'close() before commit rolls back, then releases the connection'() { + given: + def session = new JdbcSqlSession(connection) + + when: + session.close() + + then: + 1 * connection.rollback() + 0 * connection.commit() + // connection.close() is called via releaseConnection (once) + (1.._) * connection.close() + } + + def 'close() after commit does not roll back; it just releases the connection'() { + given: + def session = new JdbcSqlSession(connection) + session.commit() + + when: + session.close() + + then: + 0 * connection.rollback() + } + + def 'rollback swallows SQLException from connection.rollback (still finalises)'() { + given: + def session = new JdbcSqlSession(connection) + connection.rollback() >> { throw new SQLException('rolling-back-failed') } + + when: + session.rollback() + + then: 'no exception bubbles out' + noExceptionThrown() + + and: 'subsequent rollback is a no-op' + session.rollback() + 1 * connection.rollback() // only the original attempt + } +} diff --git a/xtraplatform-features/src/main/java/de/ii/xtraplatform/features/domain/FeatureTransactions.java b/xtraplatform-features/src/main/java/de/ii/xtraplatform/features/domain/FeatureTransactions.java index 3072ecf0b..faee145e5 100644 --- a/xtraplatform-features/src/main/java/de/ii/xtraplatform/features/domain/FeatureTransactions.java +++ b/xtraplatform-features/src/main/java/de/ii/xtraplatform/features/domain/FeatureTransactions.java @@ -57,4 +57,74 @@ MutationResult updateFeature( String type, String id, FeatureTokenSource featureTokenSource, EpsgCrs crs, boolean partial); MutationResult deleteFeature(String featureType, String id); + + /** + * Opens a session that executes a sequence of mutator calls against a single underlying SQL + * transaction. {@link Session#commit()} makes the changes durable; {@link Session#close()} + * without a prior {@code commit()} rolls back. The default implementation throws {@link + * UnsupportedOperationException} for providers that have not yet adopted the API. + */ + default Session openSession() { + throw new UnsupportedOperationException( + "Multi-action transaction sessions are not supported by this feature provider"); + } + + /** + * Session-scoped transaction. The mutator methods have the same contract as their top-level + * counterparts on {@link FeatureTransactions}, but all calls execute against one underlying SQL + * transaction. + */ + interface Session extends AutoCloseable { + + MutationResult createFeatures( + String featureType, + FeatureTokenSource featureTokenSource, + EpsgCrs crs, + Optional featureId); + + /** + * Drains multiple feature token sources sequentially against the same underlying transaction, + * giving the implementation a chance to batch generated SQL across features. The default + * implementation iterates {@link #createFeatures(String, FeatureTokenSource, EpsgCrs, + * Optional)} once per source. + * + *

The returned result aggregates ids from every source in source order; on the first source + * that returns an error, iteration stops and the error is propagated with the ids collected so + * far. + */ + default MutationResult createFeatures( + String featureType, Iterable featureTokenSources, EpsgCrs crs) { + ImmutableMutationResult.Builder result = + ImmutableMutationResult.builder().type(MutationResult.Type.CREATE).hasFeatures(false); + for (FeatureTokenSource src : featureTokenSources) { + MutationResult one = createFeatures(featureType, src, crs, Optional.empty()); + for (String id : one.getIds()) { + result.addIds(id); + } + if (one.getError().isPresent()) { + return result.error(one.getError().get()).build(); + } + } + return result.build(); + } + + MutationResult updateFeature( + String type, + String id, + FeatureTokenSource featureTokenSource, + EpsgCrs crs, + boolean partial); + + MutationResult deleteFeature(String featureType, String id); + + /** Commits all mutations performed against this session. Throws if already finalised. */ + void commit(); + + /** Rolls back all mutations performed against this session. Idempotent. */ + void rollback(); + + /** Equivalent to {@link #rollback()} if {@link #commit()} has not been called. */ + @Override + void close(); + } } 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 9ee269a6c..b21553611 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 @@ -220,7 +220,7 @@ private Stream toRules( Type type = schema.isFeature() ? Type.OBJECT_ARRAY - : schema.getType() == Type.VALUE_ARRAY + : (schema.getType() == Type.VALUE_ARRAY || schema.getType() == Type.VALUE) ? schema.getValueType().orElse(Type.STRING) : schema.getType(); diff --git a/xtraplatform-features/src/test/groovy/de/ii/xtraplatform/features/domain/MappingRulesDeriverSpec.groovy b/xtraplatform-features/src/test/groovy/de/ii/xtraplatform/features/domain/MappingRulesDeriverSpec.groovy index 1df4cc8af..8d4c7a13d 100644 --- a/xtraplatform-features/src/test/groovy/de/ii/xtraplatform/features/domain/MappingRulesDeriverSpec.groovy +++ b/xtraplatform-features/src/test/groovy/de/ii/xtraplatform/features/domain/MappingRulesDeriverSpec.groovy @@ -7,7 +7,7 @@ */ package de.ii.xtraplatform.features.domain - +import de.ii.xtraplatform.features.domain.SchemaBase.Type import spock.lang.Shared import spock.lang.Specification @@ -72,6 +72,41 @@ class MappingRulesDeriverSpec extends Specification { "root concat with value concat with constant" | "landcoverunit" || "landcoverunit" "strassen_unfaelle2" | "strassen_unfaelle2" || "strassen_unfaelle2" "simple expression" | "simple_expression" || "simple_expression" + "bare VALUE resolves to leaf type" | "value_ref" || "value_ref" + } + + def 'bare Type.VALUE is resolved to its valueType (default STRING) in the emitted MappingRule'() { + // regression: a property declared as `type: VALUE, valueType: STRING` (used by xlink-reference + // properties in NAS/ALKIS provider configs) was previously emitting MappingRule.type=VALUE, + // which then propagated to SqlQueryColumn.type=VALUE — and FeatureEncoderSql.onValue only + // quotes STRING/DATETIME/DATE, so inserts produced unquoted SQL literals and Postgres parsed + // them as identifiers. MappingRule has no valueType field, so the resolution has to happen here. + + given: + def schema = new ImmutableFeatureSchema.Builder() + .name('flurstueck') + .sourcePath('/flurstueck') + .type(Type.OBJECT) + .putProperties2('rel_buchung', new ImmutableFeatureSchema.Builder() + .sourcePath('p_buchung') + .type(Type.VALUE) + .valueType(Type.STRING)) + .putProperties2('rel_default', new ImmutableFeatureSchema.Builder() + .sourcePath('p_default') + .type(Type.VALUE)) + + when: + List rules = schema.build().accept(deriver) + def byTarget = rules.collectEntries { [(it.target): it] } + + then: 'explicit valueType wins' + byTarget['rel_buchung'].type == Type.STRING + + and: 'missing valueType defaults to STRING (mirrors the existing VALUE_ARRAY handling)' + byTarget['rel_default'].type == Type.STRING + + and: 'Type.VALUE never appears in emitted rules' + rules.every { it.type != Type.VALUE } } diff --git a/xtraplatform-features/src/testFixtures/resources/feature-schemas/value_ref.yml b/xtraplatform-features/src/testFixtures/resources/feature-schemas/value_ref.yml new file mode 100644 index 000000000..0043bc0d7 --- /dev/null +++ b/xtraplatform-features/src/testFixtures/resources/feature-schemas/value_ref.yml @@ -0,0 +1,12 @@ +--- +sourcePath: /flurstueck +type: OBJECT +properties: + id: + sourcePath: objid + type: STRING + role: ID + rel_buchung: + sourcePath: p_buchung + type: VALUE + valueType: STRING diff --git a/xtraplatform-features/src/testFixtures/resources/mapping-rules/value_ref.yml b/xtraplatform-features/src/testFixtures/resources/mapping-rules/value_ref.yml new file mode 100644 index 000000000..f1179b5b6 --- /dev/null +++ b/xtraplatform-features/src/testFixtures/resources/mapping-rules/value_ref.yml @@ -0,0 +1,14 @@ +--- +- source: /flurstueck + target: $ + type: OBJECT_ARRAY + index: 0 +- source: /flurstueck/objid + target: id + type: STRING + role: ID + index: 0 +- source: /flurstueck/p_buchung + target: rel_buchung + type: STRING + index: 0 diff --git a/xtraplatform-geometries/src/main/java/de/ii/xtraplatform/geometries/domain/CircularString.java b/xtraplatform-geometries/src/main/java/de/ii/xtraplatform/geometries/domain/CircularString.java index 10a4df730..1db522489 100644 --- a/xtraplatform-geometries/src/main/java/de/ii/xtraplatform/geometries/domain/CircularString.java +++ b/xtraplatform-geometries/src/main/java/de/ii/xtraplatform/geometries/domain/CircularString.java @@ -44,8 +44,10 @@ default boolean isEmpty() { default void check() { Preconditions.checkState( isEmpty() || getValue().getNumPositions() > 2 && getValue().getNumPositions() % 2 == 1, - "A non-empty circular string must have an odd number of positions and at least three positions, found %d positions.", - getValue().getNumPositions()); + "A non-empty circular string must have an odd number of positions and at least three positions, found %d positions with axes %s: %s", + getValue().getNumPositions(), + getValue().getAxes(), + GeometryFormatting.formatPositions(getValue().getCoordinates(), getValue().getAxes(), 6)); } @Override diff --git a/xtraplatform-geometries/src/main/java/de/ii/xtraplatform/geometries/domain/CurvePolygon.java b/xtraplatform-geometries/src/main/java/de/ii/xtraplatform/geometries/domain/CurvePolygon.java index 526a65c48..d519a61a5 100644 --- a/xtraplatform-geometries/src/main/java/de/ii/xtraplatform/geometries/domain/CurvePolygon.java +++ b/xtraplatform-geometries/src/main/java/de/ii/xtraplatform/geometries/domain/CurvePolygon.java @@ -60,7 +60,9 @@ default boolean isEmpty() { default void check() { Preconditions.checkState(!getValue().isEmpty(), "A curve polygon must have at least one ring."); Preconditions.checkState( - getValue().stream().allMatch(Curve::isClosed), "All rings must be closed."); + getValue().stream().allMatch(Curve::isClosed), + "All rings must be closed. Not closed: %s", + getValue().stream().filter(c -> !c.isClosed()).toList()); Preconditions.checkArgument( getValue().stream().allMatch(g -> g.getAxes() == getAxes()), "All geometries must have the same axes."); diff --git a/xtraplatform-geometries/src/main/java/de/ii/xtraplatform/geometries/domain/GeometryFormatting.java b/xtraplatform-geometries/src/main/java/de/ii/xtraplatform/geometries/domain/GeometryFormatting.java new file mode 100644 index 000000000..045f32195 --- /dev/null +++ b/xtraplatform-geometries/src/main/java/de/ii/xtraplatform/geometries/domain/GeometryFormatting.java @@ -0,0 +1,46 @@ +/* + * 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.geometries.domain; + +/** + * Formatting helpers for validation error messages — keep diagnostics readable when the offending + * geometry has many positions. + */ +final class GeometryFormatting { + + private GeometryFormatting() {} + + /** + * Render a flat coordinate array as a positions list like {@code [(x1 y1), (x2 y2), …]}, capped + * at {@code maxPositions} with an ellipsis-and-count tail when more positions are present. + */ + static String formatPositions(double[] coordinates, Axes axes, int maxPositions) { + int dim = axes.size(); + if (coordinates.length == 0 || dim == 0) { + return "[]"; + } + int total = coordinates.length / dim; + int shown = Math.min(total, maxPositions); + StringBuilder sb = new StringBuilder(shown * 24); + sb.append('['); + for (int p = 0; p < shown; p++) { + if (p > 0) sb.append(", "); + sb.append('('); + for (int i = 0; i < dim; i++) { + if (i > 0) sb.append(' '); + sb.append(coordinates[p * dim + i]); + } + sb.append(')'); + } + if (total > shown) { + sb.append(", … (").append(total - shown).append(" more)"); + } + sb.append(']'); + return sb.toString(); + } +} diff --git a/xtraplatform-geometries/src/main/java/de/ii/xtraplatform/geometries/domain/LineString.java b/xtraplatform-geometries/src/main/java/de/ii/xtraplatform/geometries/domain/LineString.java index 5f4c2e86d..40803de3b 100644 --- a/xtraplatform-geometries/src/main/java/de/ii/xtraplatform/geometries/domain/LineString.java +++ b/xtraplatform-geometries/src/main/java/de/ii/xtraplatform/geometries/domain/LineString.java @@ -79,8 +79,10 @@ default boolean isEmpty() { default void check() { Preconditions.checkState( getValue().getNumPositions() != 1, - "A non-empty line string must have at least two positions, found %s positions.", - getValue().getNumPositions()); + "A non-empty line string must have at least two positions, found %s positions with axes %s: %s", + getValue().getNumPositions(), + getValue().getAxes(), + GeometryFormatting.formatPositions(getValue().getCoordinates(), getValue().getAxes(), 4)); } @Override diff --git a/xtraplatform-geometries/src/main/java/de/ii/xtraplatform/geometries/domain/Position.java b/xtraplatform-geometries/src/main/java/de/ii/xtraplatform/geometries/domain/Position.java index 95c3c88b1..34b9abe5d 100644 --- a/xtraplatform-geometries/src/main/java/de/ii/xtraplatform/geometries/domain/Position.java +++ b/xtraplatform-geometries/src/main/java/de/ii/xtraplatform/geometries/domain/Position.java @@ -113,11 +113,14 @@ public double m() { public void check() { Preconditions.checkState( getCoordinates().length == getAxes().size(), - "Position must have %d coordinates, but got %d.", + "Position must have %d coordinates for axes %s, but got %d: %s", getAxes().size(), - getCoordinates().length); + getAxes(), + getCoordinates().length, + Arrays.toString(getCoordinates())); Preconditions.checkState( isEmpty() || Arrays.stream(getCoordinates()).noneMatch(Double::isNaN), - "Position must not have NaN coordinates unless it is EMPTY."); + "Position must not have NaN coordinates unless it is EMPTY, got: %s", + Arrays.toString(getCoordinates())); } }