diff --git a/src/proxy/execute.ts b/src/proxy/execute.ts index 97dbca7..314e9d0 100644 --- a/src/proxy/execute.ts +++ b/src/proxy/execute.ts @@ -6,14 +6,15 @@ * for `intersects_feature` / `travel_time` filters. * * Unlike the LLM-facing `executeQueryFeatures` (which strips geometry to `null` - * via `attachFeatureRefs` to save tokens), the proxy needs the OPPOSITE: a + * via `postProcessFeatureCollection` to save tokens), the proxy needs the OPPOSITE: a * FeatureCollection with FULL geometry, because MCP Carto renders it on a map. * * This module reuses the WFS query-compilation primitives (`compileQueryParts`, * `buildMainRequest`, reference-geometry resolution) but: * - forces the geometry column into the request `propertyName` itself, without - * touching `buildSelectList` (which stays coupled to the LLM `select`/`spatial_extras` knobs); - * - returns the RAW FeatureCollection, never `attachFeatureRefs`; + * touching `buildPropertyName` / `buildPropertyNameWithGeometry`, which stay + * coupled to the LLM `select`/`spatial_extras` knobs); + * - returns the RAW FeatureCollection, never `postProcessFeatureCollection`; * - runs against an INJECTED WfsClient, so it is fully testable without network * and lets the HTTP layer supply a size-bounded, rate-limited client. */ @@ -31,7 +32,8 @@ import { getSpatialFilter, type ResolvedFeatureGeometryRef, } from "../wfs/queryPreparation.js"; -import { buildPropertyName, requireSingleFeatureById } from "../wfs/byId.js"; +import { requireSingleFeatureById } from "../wfs/byId.js"; +import { buildPropertyNameWithGeometry } from "../wfs/properties.js" import { resolveFeatureGeometryEwkt } from "../wfs/referenceGeometry.js"; import { rethrowIdentifiedCatalogDesyncError } from "../wfs/catalogDesync.js"; import { ServiceResponseError, extractJsonServiceError } from "../helpers/http.js"; @@ -72,20 +74,16 @@ export type GeometryFeatureQueryDeps = { /** * Appends the geometry column to a compiled `propertyName` selection so the WFS - * returns full geometry. An empty/undefined selection means "all properties", - * which already includes geometry, so it is left untouched. + * returns full geometry. * - * @param propertyName Comma-separated selection from `compileQueryParts`, if any. + * @param propertyName Comma-separated selection from `compileQueryParts`. * @param geometryProperty Geometry property resolved for the feature type. - * @returns A selection guaranteed to include the geometry column, or `undefined`. + * @returns A selection guaranteed to include the geometry column. */ function ensureGeometrySelected( - propertyName: string | undefined, + propertyName: string, geometryProperty: CollectionProperty, -): string | undefined { - if (!propertyName) { - return undefined; - } +): string { const columns = propertyName.split(","); if (columns.includes(geometryProperty.name)) { return propertyName; @@ -255,7 +253,7 @@ export type GeometryFeatureByIdQueryDeps = { * returned; * - requests WGS84 (`EPSG:4326`, [lon, lat]) like the query path; * - enforces strict cardinality (0 or >1 results throw); - * - returns the untransformed single-feature collection — never `attachFeatureRefs` + * - returns the untransformed single-feature collection — never `postProcessFeatureCollection` * (which would null the geometry). * * @param input Validated by-id layer input (`{ typename, feature_id, select? }`). @@ -272,10 +270,7 @@ export async function runGeometryFeatureByIdQuery( // Validate `select` against the same embedded catalog used at URL generation, // then force the geometry column into the WFS selection. Re-validating here is // required because decoded proxy tokens remain untrusted input. - const propertyName = buildPropertyName(featureType, { - includeGeometry: true, - select: input.select, - }); + const propertyName = buildPropertyNameWithGeometry(featureType, input.select); const request = buildGetFeatureByIdRequest( input.typename, input.feature_id, diff --git a/src/tools/GpfGetFeatureByIdLayerTool.ts b/src/tools/GpfGetFeatureByIdLayerTool.ts index a3be79d..2f695fa 100644 --- a/src/tools/GpfGetFeatureByIdLayerTool.ts +++ b/src/tools/GpfGetFeatureByIdLayerTool.ts @@ -32,7 +32,7 @@ import { getEnv } from "../config/env.js"; import { encodeToken } from "../proxy/token.js"; import { buildDataUrl } from "../proxy/dataUrl.js"; import { wfsClient } from "../wfs/execution.js"; -import { buildPropertyName } from "../wfs/byId.js"; +import { buildPropertyNameWithGeometry, getGeometryProperty } from "../wfs/properties.js"; import { PROXY_TOKEN_KIND, gpfGetFeatureByIdLayerInputObjectSchema, @@ -125,10 +125,10 @@ class GpfGetFeatureByIdLayerTool extends BaseTool { // The `feature_id` cannot be validated here — that is a network lookup resolved // at fetch time by the proxy (runGeometryFeatureByIdQuery). const featureType = await wfsClient.getFeatureType(tokenParams.typename); - buildPropertyName(featureType, { - includeGeometry: true, - select: tokenParams.select, - }); + buildPropertyNameWithGeometry(featureType, tokenParams.select); + + // Also check that the typename corresponds to a collection that has a geometry. + getGeometryProperty(featureType); logger.info(`[tool] execute ${this.name} ...`, { input: tokenParams, diff --git a/src/tools/GpfGetFeaturesLayerTool.ts b/src/tools/GpfGetFeaturesLayerTool.ts index 8d76cea..2127f67 100644 --- a/src/tools/GpfGetFeaturesLayerTool.ts +++ b/src/tools/GpfGetFeaturesLayerTool.ts @@ -120,6 +120,9 @@ class GpfGetFeaturesLayerTool extends BaseTool { const featureType = await wfsClient.getFeatureType(compiledInput.typename); const spatialFilter = getSpatialFilter(compiledInput); + // Also check that the typename corresponds to a collection that has a geometry. + getGeometryProperty(featureType); + // Also validate the reference feature's typename for an intersects_feature // filter — it is a SECOND typename the proxy would resolve at fetch time // (execute.ts resolveReferenceGeometry), and an unknown one would otherwise diff --git a/src/wfs/byId.ts b/src/wfs/byId.ts index c023173..c4c5daf 100644 --- a/src/wfs/byId.ts +++ b/src/wfs/byId.ts @@ -14,9 +14,9 @@ import type { WfsFeatureCollectionResponse, WfsFeatureResponse, } from "./types.js"; -import { validateSelectProperty, getGeometryProperty } from "./queryPreparation.js"; import { buildGetFeatureByIdRequest } from "./request.js"; -import { attachFeatureRefs } from "./response.js"; +import { buildPropertyName } from "./properties.js"; +import { postProcessFeatureCollection } from "./response.js"; // --- Input Types --- @@ -32,59 +32,18 @@ export type GetFeatureByIdExecutionInput = { // --- Internal Types --- -type PropertySelectionInput = { - includeGeometry?: boolean; - select?: string[]; -}; - type FetchFeatureByIdInput = { typename: string; feature_id: string; - propertyName?: string; + propertyName: string; }; -// --- Property Selection --- - -/** - * Builds the optional `propertyName` request parameter from `select`. - * - * @param featureType Feature type definition loaded from the embedded catalog. - * @param input Property selection options derived from the caller's output mode. - * @returns A comma-separated property list, or `undefined` when all properties should be returned. - */ -export function buildPropertyName( - featureType: Collection, - input: PropertySelectionInput, -) { - // `includeGeometry` is also an invariant check: even without `select`, a - // cartographic/derived-geometry caller must fail against a geometry-less type - // before issuing a WFS request. - const geometryProperty = input.includeGeometry - ? getGeometryProperty(featureType) - : undefined; - - if (!input.select || input.select.length === 0) { - return undefined; - } - - const selectionGeometryProperty = geometryProperty ?? getGeometryProperty(featureType); - const selectedProperties = input.select.map((propertyName) => - validateSelectProperty(featureType, selectionGeometryProperty, propertyName), - ); - - if (geometryProperty) { - return [...selectedProperties, geometryProperty.name].join(","); - } - - return selectedProperties.join(","); -} - // --- Live Lookup --- /** * Executes the live WFS lookup targeting a single `featureID`. * - * @param input Target layer, expected feature id, and optional property selection. + * @param input Target layer, expected feature id, and property selection. * @returns The raw FeatureCollection returned by the WFS service. */ export async function fetchFeatureById( @@ -176,7 +135,7 @@ export function requireSingleFeatureById( * * This function: * - loads the feature type from the embedded catalog - * - builds the optional `propertyName` selection + * - builds the `propertyName` selection * - executes the WFS request for the requested `feature_id` * - enforces strict cardinality on the returned FeatureCollection * - attaches reusable `feature_ref` metadata to the final response @@ -194,10 +153,7 @@ export async function executeGetFeatureById( input: GetFeatureByIdExecutionInput, ) { const featureType: Collection = await wfsClient.getFeatureType(input.typename); - const propertyName = buildPropertyName(featureType, { - includeGeometry: (input.spatial_extras ?? []).length > 0, - select: input.select, - }); + const propertyName = buildPropertyName(featureType, input.select, input.spatial_extras); const featureCollection = await fetchFeatureById({ typename: input.typename, feature_id: input.feature_id, @@ -213,5 +169,5 @@ export async function executeGetFeatureById( numberMatched: 1, }; - return attachFeatureRefs(singleFeatureCollection, input.typename, input.spatial_extras); + return postProcessFeatureCollection(singleFeatureCollection, input); } diff --git a/src/wfs/features.ts b/src/wfs/features.ts index c913e79..6ad2159 100644 --- a/src/wfs/features.ts +++ b/src/wfs/features.ts @@ -28,7 +28,7 @@ import { buildMainRequest, type CompiledRequest, } from "./request.js"; -import { attachFeatureRefs } from "./response.js"; +import { postProcessFeatureCollection } from "./response.js"; import type { GpfQueryFeaturesInput } from "./schema.js"; // --- Types --- @@ -197,12 +197,14 @@ export async function executeQueryFeatures(input: GpfQueryFeaturesInput) { } catch (error: unknown) { // Rewrite an embedded-catalog geometry-column desync into a clear diagnostic // (shared with the proxy path); any other error passes through unchanged. - rethrowIdentifiedCatalogDesyncError(error, compiled.geometryProperty.name, input.typename); + if (compiled.geometryProperty) { + rethrowIdentifiedCatalogDesyncError(error, compiled.geometryProperty.name, input.typename); + } throw error; } if (isGetFeaturesQuery) { - return attachFeatureRefs(featureCollection, input.typename, input.spatial_extras); + return postProcessFeatureCollection(featureCollection, input); } else { return { numberMatched: getMatchedFeatureCount(featureCollection), diff --git a/src/wfs/properties.ts b/src/wfs/properties.ts index 6364e04..3ac75c6 100644 --- a/src/wfs/properties.ts +++ b/src/wfs/properties.ts @@ -8,7 +8,6 @@ */ import type { Collection, CollectionProperty } from "@ignfab/gpf-schema-store"; -import type { GpfGetFeaturesInput } from "./schema.js"; // --- Property Listing --- @@ -78,14 +77,13 @@ function getPropertyOrThrow(featureType: Collection, propertyName: string) { * of the feature type. * * @param featureType Feature type definition loaded from the embedded catalog. - * @param geometryProperty Geometry property already resolved for the feature type. * @param propertyName Exact property name requested by the caller. * @param message Error message template used when the property is geometric. * @returns The matching non-geometric property metadata. */ -export function resolveNonGeometryProperty(featureType: Collection, geometryProperty: CollectionProperty, propertyName: string, message: string) { +export function resolveNonGeometryProperty(featureType: Collection, propertyName: string, message: string) { const property = getPropertyOrThrow(featureType, propertyName); - if (property.name === geometryProperty.name || property.defaultCrs) { + if (property.defaultCrs) { throw new Error(message.replace("{property}", property.name)); } return property; @@ -97,14 +95,12 @@ export function resolveNonGeometryProperty(featureType: Collection, geometryProp * Validates a selected property name and returns the exact property name to expose. * * @param featureType Feature type definition loaded from the embedded catalog. - * @param geometryProperty Geometry property already resolved for the feature type. * @param propertyName Raw selected property name. * @returns The validated non-geometric property name. */ -export function validateSelectProperty(featureType: Collection, geometryProperty: CollectionProperty, propertyName: string) { +function validateSelectProperty(featureType: Collection, propertyName: string) { return resolveNonGeometryProperty( featureType, - geometryProperty, propertyName, "La propriété '{property}' est géométrique. `select` accepte uniquement des propriétés non géométriques." ).name; @@ -121,42 +117,61 @@ export function validateSelectProperty(featureType: Collection, geometryProperty * - when `spatial_extras` is non-empty, the geometry column is appended so elements of GPF_GET_FEATURES_SPATIAL_EXTRAS (bbox, centroid, ...) can be derived * * @param featureType Feature type definition loaded from the embedded catalog. + * @param select The list of selected non-geometric properties. + * @param spatial_extras The list of selected extra properties to compute on the geometry. * @param geometryProperty Geometry property already resolved for the feature type. - * @param input Normalized tool input. + * @param includeGeometry Override boolean to force including the geometry in the return query. * @returns The list of property names to expose in the WFS `propertyName` parameter. */ -export function buildSelectList( +export function buildPropertyName( featureType: Collection, - geometryProperty: CollectionProperty, - input: GpfGetFeaturesInput, -) { - const shouldIncludeGeometry = (input.spatial_extras ?? []).length > 0; + select?: string[], + spatial_extras?: string[], + geometryProperty?: CollectionProperty, + includeGeometry: boolean = (spatial_extras ?? []).length > 0, +) : string { // If `select` is specified, only the requested properties are returned // after validation against the embedded catalog. - if (input.select && input.select.length > 0) { - const selectedProperties = input.select.map((propertyName) => - validateSelectProperty(featureType, geometryProperty, propertyName), + if (select && select.length > 0) { + const selectedProperties = select.map((propertyName) => + validateSelectProperty(featureType, propertyName), ); - // Include geometry when `spatial_extras` needs it to derive bbox/centroid/... - if (shouldIncludeGeometry) { - return [...selectedProperties, geometryProperty.name]; + if (includeGeometry) { + return [...selectedProperties, (geometryProperty ?? getGeometryProperty(featureType)).name].join(","); } - return selectedProperties; + return selectedProperties.join(","); } // If `select` is omitted, return every non-geometric property from the - // feature type, appending the geometry column only when `spatial_extras` - // needs it. + // feature type, appending the geometry column only when it is required, + // for example when `spatial_extras` needs it to derive bbox/centroid/... + + if (includeGeometry) { + // Ensure that the geometric property exists and is unique. + geometryProperty ?? getGeometryProperty(featureType); + return featureType.properties + .map((property: CollectionProperty) => property.name) + .join(","); // return all properties + } + const nonGeometryProperties = featureType.properties .filter((property: CollectionProperty) => !property.defaultCrs) .map((property: CollectionProperty) => property.name); - if (shouldIncludeGeometry) { - return [...nonGeometryProperties, geometryProperty.name]; - } + return nonGeometryProperties.join(","); +} - return nonGeometryProperties; +/** + * `buildPropertyName` for cartographic callers: the geometry column is always + * selected, and a geometry-less type must fail here rather than at map load. + */ +export function buildPropertyNameWithGeometry( + featureType: Collection, + select?: string[], + geometryProperty: CollectionProperty = getGeometryProperty(featureType), +) { + return buildPropertyName(featureType, select, [], geometryProperty, true); } diff --git a/src/wfs/queryPreparation.ts b/src/wfs/queryPreparation.ts index abfe9ec..09059ad 100644 --- a/src/wfs/queryPreparation.ts +++ b/src/wfs/queryPreparation.ts @@ -11,8 +11,7 @@ import type { Collection, CollectionProperty } from "@ignfab/gpf-schema-store"; import { - validateSelectProperty, - buildSelectList, + buildPropertyName, resolveNonGeometryProperty, getGeometryProperty, } from "./properties.js"; @@ -41,7 +40,7 @@ import { // --- Re-exports --- export { geometryToEwkt } from "./geometry.js"; -export { validateSelectProperty, getGeometryProperty } from "./properties.js"; +export { getGeometryProperty } from "./properties.js"; export { getSpatialFilter } from "./spatialFilter.js"; // --- Internal Constants --- @@ -64,9 +63,9 @@ export type ResolvedFeatureGeometryRef = { }; export type CompiledQuery = { - geometryProperty: CollectionProperty; + geometryProperty?: CollectionProperty; + propertyName: string; cqlFilter?: string; - propertyName?: string; sortBy?: string; }; @@ -125,14 +124,12 @@ function compileIsNullClause(property: CollectionProperty) { * Compiles a structured where clause into a CQL fragment. * * @param featureType Feature type definition loaded from the embedded catalog. - * @param geometryProperty Geometry property already resolved for the feature type. * @param clause Raw where clause received from the tool input. * @returns A CQL predicate fragment. */ -function compileWhereClause(featureType: Collection, geometryProperty: CollectionProperty, clause: WhereClause) { +function compileWhereClause(featureType: Collection, clause: WhereClause) { const property = resolveNonGeometryProperty( featureType, - geometryProperty, clause.property, "La propriété '{property}' est géométrique. Utiliser un filtre spatial dédié (`bbox_filter`, `intersects_point_filter`, `dwithin_point_filter`, `intersects_feature_filter` ou `travel_time_filter`)." ); @@ -158,14 +155,12 @@ function compileWhereClause(featureType: Collection, geometryProperty: Collectio * Compiles a structured sort clause into a WFS `sortBy` fragment. * * @param featureType Feature type definition loaded from the embedded catalog. - * @param geometryProperty Geometry property already resolved for the feature type. * @param clause Raw order-by clause received from the tool input. * @returns A WFS `sortBy` fragment. */ -function compileOrderByClause(featureType: Collection, geometryProperty: CollectionProperty, clause: OrderByClause) { +function compileOrderByClause(featureType: Collection, clause: OrderByClause) { const property = resolveNonGeometryProperty( featureType, - geometryProperty, clause.property, "La propriété '{property}' est géométrique. Utiliser une propriété non géométrique pour `order_by`." ); @@ -187,13 +182,14 @@ export function compileQueryParts( featureType: Collection, resolvedGeometryRef?: ResolvedFeatureGeometryRef, ): CompiledQuery { - const geometryProperty = getGeometryProperty(featureType); + let geometryProperty: undefined | CollectionProperty; const spatialFilter = getSpatialFilter(input); const fragments: string[] = []; // Keep the spatial predicate first: the GeoPlateforme GeoServer is sensitive // to filter ordering and may reject equivalent filters when attributes come first. if (spatialFilter) { + geometryProperty = getGeometryProperty(featureType); switch (spatialFilter.operator) { case "bbox": fragments.push(compileBboxSpatialFilter(geometryProperty, spatialFilter)); @@ -220,21 +216,39 @@ export function compileQueryParts( } for (const clause of input.where ?? []) { - fragments.push(compileWhereClause(featureType, geometryProperty, clause)); + fragments.push(compileWhereClause(featureType, clause)); + } + + const cqlFilter = fragments.length > 0 ? fragments.join(" AND ") : undefined; + + if (!("limit" in input)) { + // for CountFeatures: only return the required parts + return { + cqlFilter, + propertyName: "", + }; } - const isGetFeaturesQuery = "limit" in input; + // for GetFeatures and GetFeatureById: compute sortBy, propertyName and + // ensure that geometryProperty is set when it is among the returned columns. - const sortBy = isGetFeaturesQuery && input.order_by && input.order_by.length > 0 - ? input.order_by.map((clause) => compileOrderByClause(featureType, geometryProperty, clause)).join(",") + const sortBy = input.order_by && input.order_by.length > 0 + ? input.order_by.map((clause) => compileOrderByClause(featureType, clause)).join(",") : undefined; - const propertyNames = isGetFeaturesQuery ? buildSelectList(featureType, geometryProperty, input) : []; + if (!geometryProperty && input.spatial_extras.length > 0) { + geometryProperty = getGeometryProperty(featureType); + } + + const propertyName = buildPropertyName(featureType, input.select, input.spatial_extras, geometryProperty); + + // geometryProperty must always be set if it is among the returned columns. + // It may also be set even if not required in the returned columns. return { geometryProperty, - cqlFilter: fragments.length > 0 ? fragments.join(" AND ") : undefined, - propertyName: propertyNames.length > 0 ? propertyNames.join(",") : undefined, + cqlFilter, + propertyName, sortBy, }; } diff --git a/src/wfs/request.ts b/src/wfs/request.ts index d396f16..479a799 100644 --- a/src/wfs/request.ts +++ b/src/wfs/request.ts @@ -7,6 +7,7 @@ */ import { GPF_WFS_URL } from "./catalog.js"; +import { CompiledQuery } from "./queryPreparation.js"; import type { GpfQueryFeaturesInput } from "./schema.js"; // --- Transport Types --- @@ -36,7 +37,7 @@ function buildBody(cqlFilter?: string) { // --- Public Builders --- /** - * Builds the main WFS GetFeature or CountFeature request from normalized tool input and compiled query parts. + * Builds the main WFS GetFeature or CountFeatures request from normalized tool input and compiled query parts. * * @param input Normalized tool input. * @param compiled Compiled query fragments produced from the input and feature type. @@ -44,7 +45,7 @@ function buildBody(cqlFilter?: string) { */ export function buildMainRequest( input: GpfQueryFeaturesInput, - compiled: { cqlFilter?: string; propertyName?: string; sortBy?: string }, + compiled: CompiledQuery, ): CompiledRequest { const query: Record = { service: "WFS", @@ -57,6 +58,7 @@ export function buildMainRequest( }; if (compiled.propertyName) { + // propertyName is always empty for CountFeatures and never empty for GetFeatures query.propertyName = compiled.propertyName; } if (compiled.sortBy) { @@ -77,13 +79,13 @@ export function buildMainRequest( * * @param typename Typename of the target layer. * @param featureId Identifier of the target feature. - * @param propertyName Optional comma-separated property list. + * @param propertyName Comma-separated property list. * @returns A POST request split into base URL, query-string parameters, and empty body. */ export function buildGetFeatureByIdRequest( typename: string, featureId: string, - propertyName?: string, + propertyName: string, ): CompiledRequest { const query: Record = { service: "WFS", diff --git a/src/wfs/response.ts b/src/wfs/response.ts index 76bd289..2b7c194 100644 --- a/src/wfs/response.ts +++ b/src/wfs/response.ts @@ -146,18 +146,25 @@ export function transformFeatureCollectionResponse( return { ...restCollection, features: transformedFeatures }; } -// --- Feature References --- +// --- Feature Collection post-processing --- + +type FeatureCollectionPostProcessInput = { + typename: string, + spatial_extras?: string[], +} /** - * Transforms a FeatureCollection and injects the exact queried typename into each `feature_ref`. + * Transforms a FeatureCollection obtained from upstream. + * + * - Inject the exact queried typename into each `feature_ref`. + * - Compute the required `spatial_extras`. * * @param featureCollection Raw FeatureCollection returned by the WFS endpoint. - * @param typename Typename of the queried layer. - * @param spatial_extras List of extra geometry data to include for each feature. - * @returns A transformed FeatureCollection whose `feature_ref` objects carry the exact typename. + * @param input GpfGetFeatures or GpfGetFeatureById input query. + * @returns A FeatureCollection with the required transformations done. */ -export function attachFeatureRefs(featureCollection: GenericFeatureCollection, typename: string, spatial_extras: string[] = []) { - const transformed = transformFeatureCollectionResponse(featureCollection, spatial_extras); +export function postProcessFeatureCollection(featureCollection: GenericFeatureCollection, input: FeatureCollectionPostProcessInput) { + const transformed = transformFeatureCollectionResponse(featureCollection, input.spatial_extras); if (!Array.isArray(transformed.features)) { return transformed; } @@ -174,7 +181,7 @@ export function attachFeatureRefs(featureCollection: GenericFeatureCollection, t ...feature, feature_ref: { ...featureRef, - typename, + typename: input.typename, }, }; }); diff --git a/test/proxy/execute.test.ts b/test/proxy/execute.test.ts index 28dac91..2c331bd 100644 --- a/test/proxy/execute.test.ts +++ b/test/proxy/execute.test.ts @@ -317,7 +317,7 @@ describe("proxy/execute · runGeometryFeatureByIdQuery", () => { // No `propertyName` is sent (WFS returns all props incl. geometry); srsName is // forced to WGS84 like the query path. expect(requests).toHaveLength(1); - expect(requests[0].query.propertyName).toBeUndefined(); + expect(requests[0].query.propertyName).toBe("code_insee,population,geometrie"); expect(requests[0].query.featureID).toBe("commune.1"); expect(requests[0].query.srsName).toBe("EPSG:4326"); }); diff --git a/test/tools/wfs/getFeatureById.test.ts b/test/tools/wfs/getFeatureById.test.ts index 70b49e6..bf73e34 100644 --- a/test/tools/wfs/getFeatureById.test.ts +++ b/test/tools/wfs/getFeatureById.test.ts @@ -44,6 +44,18 @@ describe("Test GpfGetFeatureByIdTool", () => { ], }; + const tableFeatureType: Collection = { + id: "wfs_scot:doc_urba", + namespace: "wfs_scot", + name: "doc_urba", + title: "Document d'urbanisme", + description: "Description de test", + properties: [ + { name: "partition", type: "string" }, + { name: "idurba", type: "string" }, + ], + }; + afterEach(() => { vi.clearAllMocks(); mockGetFeatureType.mockReset(); @@ -132,6 +144,7 @@ describe("Test GpfGetFeatureByIdTool", () => { expect(response.isError).toBeUndefined(); expect(requests).toHaveLength(1); expect(requests[0].query.exceptions).toEqual("application/json"); + expect(requests[0].query.propertyName).toEqual("code_insee,nom_officiel"); const textContent = response.content[0]; if (textContent.type !== "text") { throw new Error("expected text content"); @@ -205,6 +218,54 @@ describe("Test GpfGetFeatureByIdTool", () => { expect(results.features[0].centroid).toBeUndefined(); }); + it("should not append the geometry column to propertyName when spatial_extras is empty", async () => { + const tool = new GpfGetFeatureByIdTool(); + const requests: Array<{ url: string; query: Record }> = []; + mockGetFeatureType.mockResolvedValue(polygonFeatureType); + mockFetchJSONPost.mockImplementation(async (url, _body) => { + const [baseUrl, queryString = ""] = url.split("?"); + requests.push({ + url: baseUrl, + query: Object.fromEntries(new URLSearchParams(queryString).entries()), + }); + + return { + type: "FeatureCollection", + totalFeatures: 1, + features: [ + { + type: "Feature", + id: "commune.1", + geometry: { + type: "Polygon", + coordinates: [[[2.3, 48.8], [2.4, 48.8], [2.4, 48.9], [2.3, 48.9], [2.3, 48.8]]], + }, + geometry_name: "geometrie", + properties: { + code_insee: "01001", + }, + }, + ], + }; + }); + + const response = await tool.toolCall({ + params: { + name: "gpf_get_feature_by_id", + arguments: { + typename: "ADMINEXPRESS-COG.LATEST:commune", + feature_id: "commune.1", + select: ["code_insee"], + spatial_extras: [], + }, + }, + }); + + expect(response.isError).toBeUndefined(); + expect(requests).toHaveLength(1); + expect(requests[0].query.propertyName).toEqual("code_insee"); + }); + it("should fail clearly when the feature is missing", async () => { const tool = new GpfGetFeatureByIdTool(); mockGetFeatureType.mockResolvedValue(polygonFeatureType); @@ -331,4 +392,69 @@ describe("Test GpfGetFeatureByIdTool", () => { detail: expect.stringContaining("gpf_get_feature_by_id"), }); }); + + it("should work on a geometry-less table when select and spatial_extras are empty", async () => { + const tool = new GpfGetFeatureByIdTool(); + const requests: Array<{ url: string; query: Record }> = []; + mockGetFeatureType.mockResolvedValue(tableFeatureType); + mockFetchJSONPost.mockImplementation(async (url) => { + const [baseUrl, queryString = ""] = url.split("?"); + requests.push({ + url: baseUrl, + query: Object.fromEntries(new URLSearchParams(queryString).entries()), + }); + return { + type: "FeatureCollection", + totalFeatures: 1, + features: [ + { + type: "Feature", + id: "doc_urba.1", + properties: { + partition: "A", + idurba: "S123", + }, + }, + ], + }; + }); + + const response = await tool.toolCall({ + params: { + name: "gpf_get_feature_by_id", + arguments: { + typename: "wfs_scot:doc_urba", + feature_id: "doc_urba.1", + }, + }, + }); + + expect(response.isError).toBeUndefined(); + expect(requests).toHaveLength(1); + expect(requests[0].query.propertyName).toEqual("partition,idurba"); + }); + + it("should fail before WFS request on a geometry-less table when spatial_extras is not empty", async () => { + const tool = new GpfGetFeatureByIdTool(); + mockGetFeatureType.mockResolvedValue(tableFeatureType); + + const response = await tool.toolCall({ + params: { + name: "gpf_get_feature_by_id", + arguments: { + typename: "wfs_scot:doc_urba", + feature_id: "doc_urba.1", + spatial_extras: ["bbox"], + }, + }, + }); + + expect(response.isError).toBe(true); + const textContent = response.content[0]; + if (textContent.type !== "text") { + throw new Error("expected text content"); + } + expect(textContent.text).toContain("n'expose aucune propriété géométrique exploitable"); + expect(mockFetchJSONPost).not.toHaveBeenCalled(); + }); }); diff --git a/test/tools/wfs/getFeatures.test.ts b/test/tools/wfs/getFeatures.test.ts index 70a460d..b406331 100644 --- a/test/tools/wfs/getFeatures.test.ts +++ b/test/tools/wfs/getFeatures.test.ts @@ -74,6 +74,18 @@ describe("Test GpfGetFeaturesTool", () => { ], }; + const tableFeatureType: Collection = { + id: "wfs_scot:doc_urba", + namespace: "wfs_scot", + name: "doc_urba", + title: "Document d'urbanisme", + description: "Description de test", + properties: [ + { name: "partition", type: "string" }, + { name: "idurba", type: "string" }, + ], + }; + const featureCollection: { type: string; features: Array<{ @@ -421,6 +433,12 @@ describe("Test GpfGetFeaturesTool", () => { name: "gpf_get_features", arguments: { typename: "ADMINEXPRESS-COG.LATEST:commune", + bbox_filter: { + west: 2.1, + south: 48.7, + east: 2.5, + north: 48.9, + }, }, }, }); @@ -440,7 +458,7 @@ describe("Test GpfGetFeaturesTool", () => { it("should return feature_ref for non point layers with geometry set to null", async () => { const tool = new GpfGetFeaturesTool(); mockFeatureTypes({ [polygonFeatureType.id]: polygonFeatureType }); - captureRequests({ + const requests = captureRequests({ ...featureCollection, crs: null, features: [ @@ -464,6 +482,8 @@ describe("Test GpfGetFeaturesTool", () => { }); expect(response.isError).toBeUndefined(); + expect(requests).toHaveLength(1); + expect(requests[0].query.propertyName).toEqual("code_insee,population,actif"); const textContent = response.content[0]; if (textContent.type !== "text") { throw new Error("expected text content"); @@ -524,6 +544,48 @@ describe("Test GpfGetFeaturesTool", () => { }); }); + it("should not append the geometry column to propertyName when spatial_extras is empty", async () => { + const tool = new GpfGetFeaturesTool(); + mockFeatureTypes({ [polygonFeatureType.id]: polygonFeatureType }); + const requests = captureRequests(featureCollection); + + const response = await tool.toolCall({ + params: { + name: "gpf_get_features", + arguments: { + typename: "ADMINEXPRESS-COG.LATEST:commune", + select: ["code_insee"], + spatial_extras: [], + limit: 1, + }, + }, + }); + + expect(response.isError).toBeUndefined(); + expect(requests).toHaveLength(1); + expect(requests[0].query.propertyName).toEqual("code_insee"); + }); + + it("should request only non-geometric properties when select is omitted", async () => { + const tool = new GpfGetFeaturesTool(); + mockFeatureTypes({ [polygonFeatureType.id]: polygonFeatureType }); + const requests = captureRequests(featureCollection); + + const response = await tool.toolCall({ + params: { + name: "gpf_get_features", + arguments: { + typename: "ADMINEXPRESS-COG.LATEST:commune", + limit: 1, + }, + }, + }); + + expect(response.isError).toBeUndefined(); + expect(requests).toHaveLength(1); + expect(requests[0].query.propertyName).toEqual("code_insee,population,actif"); + }); + it("should set point geometry to null and keep feature_ref", async () => { const tool = new GpfGetFeaturesTool(); mockFeatureTypes({ [pointFeatureType.id]: pointFeatureType }); @@ -672,4 +734,61 @@ describe("Test GpfGetFeaturesTool", () => { }); expect(requests).toHaveLength(0); }); + + it("should work on a geometry-less table when select and spatial_extras are empty", async () => { + const tool = new GpfGetFeaturesTool(); + mockFeatureTypes({ [tableFeatureType.id]: tableFeatureType }); + const requests = captureRequests({ + type: "FeatureCollection", + features: [ + { + type: "Feature", + id: "doc_urba.1", + properties: { + partition: "A", + idurba: "S123", + }, + }, + ], + totalFeatures: 1, + }); + + const response = await tool.toolCall({ + params: { + name: "gpf_get_features", + arguments: { + typename: "wfs_scot:doc_urba", + limit: 1, + }, + }, + }); + + expect(response.isError).toBeUndefined(); + expect(requests).toHaveLength(1); + expect(requests[0].query.propertyName).toEqual("partition,idurba"); + }); + + it("should fail before WFS request on a geometry-less table when spatial_extras is not empty", async () => { + const tool = new GpfGetFeaturesTool(); + mockFeatureTypes({ [tableFeatureType.id]: tableFeatureType }); + + const response = await tool.toolCall({ + params: { + name: "gpf_get_features", + arguments: { + typename: "wfs_scot:doc_urba", + spatial_extras: ["bbox"], + limit: 1, + }, + }, + }); + + expect(response.isError).toBe(true); + const textContent = response.content[0]; + if (textContent.type !== "text") { + throw new Error("expected text content"); + } + expect(textContent.text).toContain("n'expose aucune propriété géométrique exploitable"); + expect(mockFetchJSONPost).not.toHaveBeenCalled(); + }); }); diff --git a/test/tools/wfs/getFeaturesLayer.test.ts b/test/tools/wfs/getFeaturesLayer.test.ts index 9b44676..2199c6d 100644 --- a/test/tools/wfs/getFeaturesLayer.test.ts +++ b/test/tools/wfs/getFeaturesLayer.test.ts @@ -237,9 +237,47 @@ describe("Test GpfGetFeaturesLayerTool", () => { }); expect(response.isError).toBe(true); - expect(response.structuredContent).toMatchObject({ - type: "urn:geocontext:problem:invalid-tool-params", + }); + + it("rejects a typename with NO geometry column BEFORE minting the URL (catalog pre-flight)", async () => { + mockGetEnv.mockReturnValue(makeEnv({})); + // A type that EXISTS in the catalog but has no geometry column (attribute-only + // table). The tool must refuse to mint a data_url because a geometry-less type + // can never produce a cartographiable layer — failing here (early, at the tool + // call) is better than handing the LLM an opaque proxy 5xx at map-load. + const tableType: Collection = { + id: "INSEE.FILOSOFI.INDICATORS:filosofi_iris_2019", + namespace: "INSEE.FILOSOFI.INDICATORS", + name: "filosofi_iris_2019", + title: "Indicateurs Filosofi (table, sans géométrie)", + description: "Fixture de test : type attributaire sans propriété géométrique", + properties: [ + { name: "code_iris", type: "string" }, + { name: "men", type: "integer" }, + { name: "ind_snv", type: "float" }, + ], + }; + mockGetFeatureType.mockResolvedValue(tableType); + const tool = new GpfGetFeaturesLayerTool(); + + const response = await tool.toolCall({ + params: { + name: "gpf_get_features_layer", + arguments: { typename: "INSEE.FILOSOFI.INDICATORS:filosofi_iris_2019" }, + }, }); + + // Fails at the tool call (no data_url minted); the typename WAS resolved. + expect(response.isError).toBe(true); + const textContent = response.content[0]; + if (textContent.type !== "text") { + throw new Error("expected text content"); + } + expect(textContent.text).not.toContain("data_url"); + expect(textContent.text).toContain("géométri"); + expect(mockGetFeatureType).toHaveBeenCalledWith( + "INSEE.FILOSOFI.INDICATORS:filosofi_iris_2019", + ); }); it("rejects an unknown property BEFORE minting the URL (catalog pre-flight)", async () => { diff --git a/test/wfs/queryPreparation.test.ts b/test/wfs/queryPreparation.test.ts index f4c3eea..d0b3671 100644 --- a/test/wfs/queryPreparation.test.ts +++ b/test/wfs/queryPreparation.test.ts @@ -124,6 +124,30 @@ describe("gpfGetFeatures/queryPreparation", () => { expect(compiled.propertyName).toEqual("code_insee,population,geometrie"); }); + it("should set geometryProperty when spatial_extras is requested without a spatial filter", () => { + const compiled = compileQueryParts({ + ...baseInput, + spatial_extras: ["bbox"], + }, featureType); + + expect(compiled.geometryProperty).toBeDefined(); + expect(compiled.geometryProperty?.name).toEqual("geometrie"); + }); + + it("should throw catalog desync error when spatial_extras is requested but the feature type has no geometry property", () => { + const nonGeometricFeatureType: Collection = { + ...featureType, + properties: featureType.properties.filter((p) => !p.defaultCrs), + }; + + expect(() => compileQueryParts({ + ...baseInput, + spatial_extras: ["bbox"], + }, nonGeometricFeatureType)).toThrow( + `Le type '${nonGeometricFeatureType.id}' n'expose aucune propriété géométrique exploitable dans le catalogue embarqué.` + ); + }); + it("should build sortBy from structured order_by", () => { const compiled = compileQueryParts({ ...baseInput, diff --git a/test/wfs/response.test.ts b/test/wfs/response.test.ts index cad21e7..12e2856 100644 --- a/test/wfs/response.test.ts +++ b/test/wfs/response.test.ts @@ -4,7 +4,7 @@ import { mapToFlatItems, mapToFlatItemsWithGeometry, transformFeatureCollectionResponse, - attachFeatureRefs, + postProcessFeatureCollection, } from "../../src/wfs/response"; describe("wfs_engine/response", () => { @@ -111,23 +111,23 @@ describe("wfs_engine/response", () => { }); }); - // --- attachFeatureRefs --- + // --- postProcessFeatureCollection --- - describe("attachFeatureRefs", () => { + describe("postProcessFeatureCollection", () => { it("should pass through when transformed result has no features array", () => { const input = { type: "FeatureCollection" }; - const result = attachFeatureRefs(input, "TEST:type"); + const result = postProcessFeatureCollection(input, { typename: "TEST:type" }); expect(result).toEqual({ type: "FeatureCollection" }); }); it("should inject typename into feature_ref for features with string id", () => { - const result = attachFeatureRefs( + const result = postProcessFeatureCollection( { features: [ { id: "commune.1", geometry: { type: "Point", coordinates: [2.35, 48.85] }, properties: { nom: "Test" } }, ], }, - "ADMINEXPRESS-COG.LATEST:commune", + { typename: "ADMINEXPRESS-COG.LATEST:commune" }, ); const features = getFeatures(result); @@ -139,13 +139,13 @@ describe("wfs_engine/response", () => { }); it("should skip features without feature_ref (non-string id)", () => { - const result = attachFeatureRefs( + const result = postProcessFeatureCollection( { features: [ { id: 42, properties: { name: "no-string-id" } }, ], }, - "TEST:type", + { typename: "TEST:type" }, ); const features = getFeatures(result); @@ -154,7 +154,7 @@ describe("wfs_engine/response", () => { }); it("should inject typename into feature_ref for multiple features", () => { - const result = attachFeatureRefs( + const result = postProcessFeatureCollection( { features: [ { id: "commune.1", geometry: null, properties: { nom: "A" } }, @@ -162,7 +162,7 @@ describe("wfs_engine/response", () => { { id: 42, properties: { nom: "C" } }, ], }, - "ADMINEXPRESS-COG.LATEST:commune", + { typename: "ADMINEXPRESS-COG.LATEST:commune" }, ); const features = getFeatures(result);