Refactor selectlist attach feature #175
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,7 +8,6 @@ | |
| */ | ||
|
|
||
| import type { Collection, CollectionProperty } from "@ignfab/gpf-schema-store"; | ||
|
|
||
| // --- 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 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, propertyName: string, message: string) { | ||
| const property = getPropertyOrThrow(featureType, propertyName); | ||
| 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 propertyName Raw selected property name. | ||
| * @returns The validated non-geometric property name. | ||
| */ | ||
| function validateSelectProperty(featureType: Collection, propertyName: string) { | ||
| return resolveNonGeometryProperty( | ||
| featureType, | ||
| 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 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. | ||
|
Member
There was a problem hiding this comment. Will have to be updated based on decisions deriving from other comments
Contributor
Author
There was a problem hiding this comment. Nothing to do so far, but keeping this open while we don't have a common conclusion on the issues above. |
||
| */ | ||
| export function buildPropertyName( | ||
| featureType: Collection, | ||
| select?: string[], | ||
| spatial_extras?: string[], | ||
|
Member
There was a problem hiding this comment.
Maybe drop
Contributor
Author
There was a problem hiding this comment. I prefer the current design, it avoids duplicating the logic to detect whether or not However, I agree that |
||
| geometryProperty?: CollectionProperty, | ||
| includeGeometry: boolean = (spatial_extras ?? []).length > 0, | ||
| ) : string { | ||
|
|
||
|
LionelZoubritzky-IGN marked this conversation as resolved.
|
||
| // If `select` is specified, only the requested properties are returned | ||
| // after validation against the embedded catalog. | ||
| if (select && select.length > 0) { | ||
| const selectedProperties = select.map((propertyName) => | ||
| validateSelectProperty(featureType, propertyName), | ||
| ); | ||
|
|
||
| if (includeGeometry) { | ||
| return [...selectedProperties, (geometryProperty ?? getGeometryProperty(featureType)).name].join(","); | ||
| } | ||
|
|
||
| return selectedProperties.join(","); | ||
| } | ||
|
|
||
| // If `select` is omitted, return every non-geometric property from the | ||
| // 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); | ||
|
Member
There was a problem hiding this comment.
Contributor
Author
There was a problem hiding this comment. Yes, the result is discarded, it only exists to throw. But no, the right-hand side can run, because |
||
| 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); | ||
|
|
||
| return nonGeometryProperties.join(","); | ||
| } | ||
|
|
||
| /** | ||
| * `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( | ||
|
LionelZoubritzky-IGN marked this conversation as resolved.
|
||
| featureType: Collection, | ||
| select?: string[], | ||
| geometryProperty: CollectionProperty = getGeometryProperty(featureType), | ||
| ) { | ||
| return buildPropertyName(featureType, select, [], geometryProperty, true); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
compileQueryPartsnow only populatescompiled.geometryPropertyinsideif (spatialFilter), butbuildPropertyNamestill forces the geometry column intopropertyNamewheneverspatial_extrasis non-empty resolving it locally (properties.ts:142) without handing it back. So for a query withspatial_extrasand no spatial filter, the column is in the request while this field isundefined, and the rewrite is skipped.The two names have drifted apart:
compiled.geometryPropertynow means "column used by the spatial filter"catalogDesyncneeds "column injected into the request"There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed and tested in 1842373