From 923160ab7fb8d106ce43c42fc88be27703dc814f Mon Sep 17 00:00:00 2001 From: Richard Kettelerij Date: Wed, 18 Sep 2024 16:43:50 +0200 Subject: [PATCH 1/8] feat(oaf): Add support to return feature properties in a specific order and/or exclude certain properties. --- config/ogcapi_features.go | 24 +++++++ config/zz_generated.deepcopy.go | 25 ++++++++ .../datasources/geopackage/geopackage.go | 62 +++++++++++++------ internal/ogc/features/domain/geojson.go | 11 ++++ internal/ogc/features/domain/jsonfg.go | 13 ++-- internal/ogc/features/domain/mapper.go | 25 ++++---- internal/ogc/features/domain/mapper_test.go | 2 +- internal/ogc/features/json.go | 8 +-- .../ogc/features/templates/feature.go.html | 4 +- .../ogc/features/templates/features.go.html | 4 +- internal/ogc/processes/main.go | 1 - 11 files changed, 135 insertions(+), 44 deletions(-) diff --git a/config/ogcapi_features.go b/config/ogcapi_features.go index f281e39e..d76a93ac 100644 --- a/config/ogcapi_features.go +++ b/config/ogcapi_features.go @@ -79,6 +79,10 @@ type CollectionEntryFeatures struct { // +optional Filters FeatureFilters `yaml:"filters,omitempty" json:"filters,omitempty"` + // Optional way to exclude feature properties and/or determine the ordering of properties in the response. + // +optional + FeatureProperties *FeatureProperties `yaml:",inline" json:",inline"` + // Downloads available for this collection through map sheets. Note that 'map sheets' refer to a map // divided in rectangle areas that can be downloaded individually. // +optional @@ -304,6 +308,26 @@ type FeatureFilters struct { // } +// +kubebuilder:object:generate=true +type FeatureProperties struct { + // Properties of features in this collection that should be included in the output, in the given order. + // + // This setting controls two things: + // + // A) allows one to exclude certain properties (only when propertiesIncludeUnknown=false) + // B) allows one sort the properties irrespective of their order in the datastore + // + // When not set all available properties are returned in API responses. + // +optional + Properties []string `yaml:"properties,omitempty" json:"properties,omitempty"` + + // When true (default) properties not listed under 'properties' are still available API responses. When false + // unlisted properties are excluded from API responses. + // +optional + // +kubebuilder:default=true + PropertiesIncludeUnknown *bool `yaml:"propertiesIncludeUnknown,omitempty" json:"propertiesIncludeUnknown,omitempty" default:"true"` // ptr due to https://github.com/creasty/defaults/issues/49 +} + // +kubebuilder:object:generate=true type MapSheetDownloads struct { // Properties that provide the download details per map sheet. Note that 'map sheets' refer to a map diff --git a/config/zz_generated.deepcopy.go b/config/zz_generated.deepcopy.go index a7d7ee7b..6cd1a6f5 100644 --- a/config/zz_generated.deepcopy.go +++ b/config/zz_generated.deepcopy.go @@ -75,6 +75,11 @@ func (in *CollectionEntryFeatures) DeepCopyInto(out *CollectionEntryFeatures) { (*in).DeepCopyInto(*out) } in.Filters.DeepCopyInto(&out.Filters) + if in.FeatureProperties != nil { + in, out := &in.FeatureProperties, &out.FeatureProperties + *out = new(FeatureProperties) + (*in).DeepCopyInto(*out) + } if in.MapSheetDownloads != nil { in, out := &in.MapSheetDownloads, &out.MapSheetDownloads *out = new(MapSheetDownloads) @@ -326,6 +331,26 @@ func (in *FeatureFilters) DeepCopy() *FeatureFilters { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *FeatureProperties) DeepCopyInto(out *FeatureProperties) { + *out = *in + if in.Properties != nil { + in, out := &in.Properties, &out.Properties + *out = make([]string, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new FeatureProperties. +func (in *FeatureProperties) DeepCopy() *FeatureProperties { + if in == nil { + return nil + } + out := new(FeatureProperties) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *FeaturesViewer) DeepCopyInto(out *FeaturesViewer) { *out = *in diff --git a/internal/ogc/features/datasources/geopackage/geopackage.go b/internal/ogc/features/datasources/geopackage/geopackage.go index da643483..e392a8aa 100644 --- a/internal/ogc/features/datasources/geopackage/geopackage.go +++ b/internal/ogc/features/datasources/geopackage/geopackage.go @@ -8,6 +8,7 @@ import ( "maps" "os" "path" + "strings" "sync" "time" @@ -79,6 +80,7 @@ type GeoPackage struct { externalFidColumn string featureTableByCollectionID map[string]*featureTable propertyFiltersByCollectionID map[string]datasources.PropertyFiltersWithAllowedValues + propertiesByCollectionID map[string]*config.FeatureProperties queryTimeout time.Duration maxBBoxSizeToUseWithRTree int } @@ -88,6 +90,7 @@ func NewGeoPackage(collections config.GeoSpatialCollections, gpkgConfig config.G g := &GeoPackage{} g.preparedStmtCache = NewCache() + g.propertiesByCollectionID = cacheFeatureProperties(collections) warmUp := false switch { @@ -151,7 +154,7 @@ func (g *GeoPackage) GetFeatureIDs(ctx context.Context, collection string, crite queryCtx, cancel := context.WithTimeout(ctx, g.queryTimeout) // https://go.dev/doc/database/cancel-operations defer cancel() - stmt, query, queryArgs, err := g.makeFeaturesQuery(queryCtx, table, true, criteria) //nolint:sqlclosecheck // prepared statement is cached, will be closed when evicted from cache + stmt, query, queryArgs, err := g.makeFeaturesQuery(queryCtx, nil, table, true, criteria) //nolint:sqlclosecheck // prepared statement is cached, will be closed when evicted from cache if err != nil { return nil, domain.Cursors{}, fmt.Errorf("failed to create query '%s' error: %w", query, err) } @@ -204,7 +207,8 @@ func (g *GeoPackage) GetFeaturesByID(ctx context.Context, collection string, fea } fc := domain.FeatureCollection{} - fc.Features, _, err = domain.MapRowsToFeatures(rows, g.fidColumn, g.externalFidColumn, table.GeometryColumnName, mapGpkgGeometry, profile.MapRelationUsingProfile) + fc.Features, _, err = domain.MapRowsToFeatures(rows, g.fidColumn, g.externalFidColumn, table.GeometryColumnName, + mapGpkgGeometry, profile.MapRelationUsingProfile) if err != nil { return nil, err } @@ -221,7 +225,7 @@ func (g *GeoPackage) GetFeatures(ctx context.Context, collection string, criteri queryCtx, cancel := context.WithTimeout(ctx, g.queryTimeout) // https://go.dev/doc/database/cancel-operations defer cancel() - stmt, query, queryArgs, err := g.makeFeaturesQuery(queryCtx, table, false, criteria) //nolint:sqlclosecheck // prepared statement is cached, will be closed when evicted from cache + stmt, query, queryArgs, err := g.makeFeaturesQuery(queryCtx, g.propertiesByCollectionID[collection], table, false, criteria) //nolint:sqlclosecheck // prepared statement is cached, will be closed when evicted from cache if err != nil { return nil, domain.Cursors{}, fmt.Errorf("failed to create query '%s' error: %w", query, err) } @@ -237,7 +241,8 @@ func (g *GeoPackage) GetFeatures(ctx context.Context, collection string, criteri var prevNext *domain.PrevNextFID fc := domain.FeatureCollection{} - fc.Features, prevNext, err = domain.MapRowsToFeatures(rows, g.fidColumn, g.externalFidColumn, table.GeometryColumnName, mapGpkgGeometry, profile.MapRelationUsingProfile) + fc.Features, prevNext, err = domain.MapRowsToFeatures(rows, g.fidColumn, g.externalFidColumn, table.GeometryColumnName, + mapGpkgGeometry, profile.MapRelationUsingProfile) if err != nil { return nil, domain.Cursors{}, err } @@ -285,7 +290,8 @@ func (g *GeoPackage) GetFeature(ctx context.Context, collection string, featureI return nil, queryCtx.Err() } - features, _, err := domain.MapRowsToFeatures(rows, g.fidColumn, g.externalFidColumn, table.GeometryColumnName, mapGpkgGeometry, profile.MapRelationUsingProfile) + features, _, err := domain.MapRowsToFeatures(rows, g.fidColumn, g.externalFidColumn, table.GeometryColumnName, + mapGpkgGeometry, profile.MapRelationUsingProfile) if err != nil { return nil, err } @@ -309,24 +315,38 @@ func (g *GeoPackage) GetPropertyFiltersWithAllowedValues(collection string) data // Build specific features queries based on the given options. // Make sure to use SQL bind variables and return named params: https://jmoiron.github.io/sqlx/#namedParams -func (g *GeoPackage) makeFeaturesQuery(ctx context.Context, table *featureTable, onlyFIDs bool, - criteria datasources.FeaturesCriteria) (stmt *sqlx.NamedStmt, query string, queryArgs map[string]any, err error) { +func (g *GeoPackage) makeFeaturesQuery(ctx context.Context, propConfig *config.FeatureProperties, table *featureTable, + onlyFIDs bool, criteria datasources.FeaturesCriteria) (stmt *sqlx.NamedStmt, query string, queryArgs map[string]any, err error) { + selectClause := g.makeSelectClause(onlyFIDs, propConfig, table) // make query if criteria.Bbox != nil { - query, queryArgs, err = g.makeBboxQuery(table, onlyFIDs, criteria) + query, queryArgs, err = g.makeBboxQuery(table, selectClause, criteria) if err != nil { return } } else { - query, queryArgs = g.makeDefaultQuery(table, criteria) + query, queryArgs = g.makeDefaultQuery(table, selectClause, criteria) } // lookup prepared statement for given query, or create new one stmt, err = g.preparedStmtCache.Lookup(ctx, g.backend.getDB(), query) return } -func (g *GeoPackage) makeDefaultQuery(table *featureTable, criteria datasources.FeaturesCriteria) (string, map[string]any) { +func (g *GeoPackage) makeSelectClause(onlyFIDs bool, propConfig *config.FeatureProperties, table *featureTable) string { + if onlyFIDs { + return "\"" + g.fidColumn + "\", prevfid, nextfid" + } else if propConfig != nil && propConfig.Properties != nil { + clause := "\"" + g.fidColumn + "\", prevfid, nextfid, \"" + table.GeometryColumnName + "\", " + strings.Join(propConfig.Properties, ",") + if *propConfig.PropertiesIncludeUnknown { + clause += ", *" + } + return clause + } + return "*" +} + +func (g *GeoPackage) makeDefaultQuery(table *featureTable, selectClause string, criteria datasources.FeaturesCriteria) (string, map[string]any) { pfClause, pfNamedParams := propertyFiltersToSQL(criteria.PropertyFilters) temporalClause, temporalNamedParams := temporalCriteriaToSQL(criteria.TemporalCriteria) @@ -336,8 +356,8 @@ with prev as (select * from "%[1]s" where "%[2]s" < :fid %[3]s %[4]s order by %[2]s desc limit :limit), nextprev as (select * from next union all select * from prev), nextprevfeat as (select *, lag("%[2]s", :limit) over (order by %[2]s) as prevfid, lead("%[2]s", :limit) over (order by "%[2]s") as nextfid from nextprev) -select * from nextprevfeat where "%[2]s" >= :fid %[3]s %[4]s limit :limit -`, table.TableName, g.fidColumn, temporalClause, pfClause) // don't add user input here, use named params for user input! +select %[5]s from nextprevfeat where "%[2]s" >= :fid %[3]s %[4]s limit :limit +`, table.TableName, g.fidColumn, temporalClause, pfClause, selectClause) // don't add user input here, use named params for user input! namedParams := map[string]any{ "fid": criteria.Cursor.FID, @@ -348,12 +368,7 @@ select * from nextprevfeat where "%[2]s" >= :fid %[3]s %[4]s limit :limit return defaultQuery, namedParams } -func (g *GeoPackage) makeBboxQuery(table *featureTable, onlyFIDs bool, criteria datasources.FeaturesCriteria) (string, map[string]any, error) { - selectClause := "*" - if onlyFIDs { - selectClause = "\"" + g.fidColumn + "\", prevfid, nextfid" - } - +func (g *GeoPackage) makeBboxQuery(table *featureTable, selectClause string, criteria datasources.FeaturesCriteria) (string, map[string]any, error) { btreeIndexHint := fmt.Sprintf("indexed by \"%s_spatial_idx\"", table.TableName) pfClause, pfNamedParams := propertyFiltersToSQL(criteria.PropertyFilters) @@ -468,3 +483,14 @@ func temporalCriteriaToSQL(temporalCriteria datasources.TemporalCriteria) (sql s } return sql, namedParams } + +func cacheFeatureProperties(collections config.GeoSpatialCollections) map[string]*config.FeatureProperties { + result := make(map[string]*config.FeatureProperties) + for _, collection := range collections { + if collection.Features == nil { + continue + } + result[collection.ID] = collection.Features.FeatureProperties + } + return result +} diff --git a/internal/ogc/features/domain/geojson.go b/internal/ogc/features/domain/geojson.go index 603c9104..986c464a 100644 --- a/internal/ogc/features/domain/geojson.go +++ b/internal/ogc/features/domain/geojson.go @@ -2,6 +2,7 @@ package domain import ( "github.com/go-spatial/geom/encoding/geojson" + orderedmap "github.com/wk8/go-ordered-map/v2" ) // featureCollectionType allows the GeoJSON type to be automatically set during json marshalling @@ -30,6 +31,16 @@ type Feature struct { // auto-incrementing integers (which is the default in geopackages) since we use it for cursor-based pagination. ID string `json:"id"` Links []Link `json:"links,omitempty"` + + Properties orderedmap.OrderedMap[string, any] `json:"properties"` +} + +func (f *Feature) Keys() []string { + result := make([]string, 0, f.Properties.Len()) + for pair := f.Properties.Oldest(); pair != nil; pair = pair.Next() { + result = append(result, pair.Key) + } + return result } // Link according to RFC 8288, https://datatracker.ietf.org/doc/html/rfc8288 diff --git a/internal/ogc/features/domain/jsonfg.go b/internal/ogc/features/domain/jsonfg.go index ddb904da..5449903b 100644 --- a/internal/ogc/features/domain/jsonfg.go +++ b/internal/ogc/features/domain/jsonfg.go @@ -2,6 +2,7 @@ package domain import ( "github.com/go-spatial/geom" + orderedmap "github.com/wk8/go-ordered-map/v2" ) const ( @@ -33,10 +34,10 @@ type JSONFGFeature struct { Time any `json:"time"` // we don't implement the JSON-FG "3D" conformance class. So Place only // supports simple/2D geometries, no 3D geometries like Polyhedron, Prism, etc. - Place geom.Geometry `json:"place"` // may only contain non-WGS84 geometries - Geometry geom.Geometry `json:"geometry"` // may only contain WGS84 geometries - Properties map[string]any `json:"properties"` - CoordRefSys string `json:"coordRefSys,omitempty"` - Links []Link `json:"links,omitempty"` - ConformsTo []string `json:"conformsTo,omitempty"` + Place geom.Geometry `json:"place"` // may only contain non-WGS84 geometries + Geometry geom.Geometry `json:"geometry"` // may only contain WGS84 geometries + Properties orderedmap.OrderedMap[string, any] `json:"properties"` + CoordRefSys string `json:"coordRefSys,omitempty"` + Links []Link `json:"links,omitempty"` + ConformsTo []string `json:"conformsTo,omitempty"` } diff --git a/internal/ogc/features/domain/mapper.go b/internal/ogc/features/domain/mapper.go index 51c699e2..81d08831 100644 --- a/internal/ogc/features/domain/mapper.go +++ b/internal/ogc/features/domain/mapper.go @@ -8,6 +8,7 @@ import ( "github.com/go-spatial/geom" "github.com/go-spatial/geom/encoding/geojson" "github.com/jmoiron/sqlx" + orderedmap "github.com/wk8/go-ordered-map/v2" ) // MapRelation abstract function type to map feature relations @@ -64,7 +65,7 @@ func MapRowsToFeatures(rows *sqlx.Rows, fidColumn string, externalFidColumn stri return result, nil, err } - feature := &Feature{Feature: geojson.Feature{Properties: make(map[string]any)}} + feature := &Feature{Feature: geojson.Feature{}, Properties: *orderedmap.New[string, any]()} np, err := mapColumnsToFeature(firstRow, feature, columns, values, fidColumn, externalFidColumn, geomColumn, mapGeom, mapRel) if err != nil { @@ -92,7 +93,7 @@ func mapColumnsToFeature(firstRow bool, feature *Feature, columns []string, valu case geomColumn: if columnValue == nil { - feature.Properties[columnName] = nil + feature.Properties.Set(columnName, nil) continue } rawGeom, ok := columnValue.([]byte) @@ -123,7 +124,7 @@ func mapColumnsToFeature(firstRow bool, feature *Feature, columns []string, valu default: if columnValue == nil { - feature.Properties[columnName] = nil + feature.Properties.Set(columnName, nil) continue } // Grab any non-nil, non-id, non-bounding box, & non-geometry column as a tag @@ -131,17 +132,17 @@ func mapColumnsToFeature(firstRow bool, feature *Feature, columns []string, valu case []uint8: asBytes := make([]byte, len(v)) copy(asBytes, v) - feature.Properties[columnName] = string(asBytes) + feature.Properties.Set(columnName, string(asBytes)) case int64: - feature.Properties[columnName] = v + feature.Properties.Set(columnName, v) case float64: - feature.Properties[columnName] = v + feature.Properties.Set(columnName, v) case time.Time: - feature.Properties[columnName] = v + feature.Properties.Set(columnName, v) case string: - feature.Properties[columnName] = v + feature.Properties.Set(columnName, v) case bool: - feature.Properties[columnName] = v + feature.Properties.Set(columnName, v) default: return nil, fmt.Errorf("unexpected type for sqlite column data: %v: %T", columns[i], v) } @@ -165,14 +166,14 @@ func mapExternalFid(columns []string, values []any, externalFidColumn string, fe // Note: This happens in a second pass over the feature, since we want to overwrite the // feature ID irrespective of the order of columns in the table feature.ID = fmt.Sprint(columnValue) - delete(feature.Properties, columnName) + feature.Properties.Delete(columnName) case strings.Contains(columnName, externalFidColumn): // When externalFidColumn is part of the column name (e.g. 'foobar_external_fid') we treat // it as a relation to another feature. newColumnName, newColumnValue := mapRel(columnName, columnValue, externalFidColumn) if newColumnName != "" { - feature.Properties[newColumnName] = newColumnValue - delete(feature.Properties, columnName) + feature.Properties.Set(newColumnName, newColumnValue) + feature.Properties.Delete(columnName) } } } diff --git a/internal/ogc/features/domain/mapper_test.go b/internal/ogc/features/domain/mapper_test.go index 3fa4b13c..6d11797f 100644 --- a/internal/ogc/features/domain/mapper_test.go +++ b/internal/ogc/features/domain/mapper_test.go @@ -88,7 +88,7 @@ func TestMapColumnsToFeature(t *testing.T) { expectedPrevNext: &PrevNextFID{}, }, { - name: "Test unexpected yype", + name: "Test unexpected type", feature: &Feature{Feature: geojson.Feature{Properties: make(map[string]any)}}, columns: []string{"str_col", "unexpected_col"}, values: []any{"str", []complex128{complex(1, 2)}}, diff --git a/internal/ogc/features/json.go b/internal/ogc/features/json.go index 0c7a1e20..763ef560 100644 --- a/internal/ogc/features/json.go +++ b/internal/ogc/features/json.go @@ -64,7 +64,7 @@ func (jf *jsonFeatures) featureAsGeoJSON(w http.ResponseWriter, r *http.Request, Rel: "enclosure", Title: "Download feature", Type: mapSheetProperties.MediaType.String(), - Href: fmt.Sprintf("%v", feat.Properties[mapSheetProperties.AssetURL]), + Href: fmt.Sprintf("%v", feat.Properties.Value(mapSheetProperties.AssetURL)), }) } @@ -126,7 +126,7 @@ func (jf *jsonFeatures) featureAsJSONFG(w http.ResponseWriter, r *http.Request, Rel: "enclosure", Title: "Download feature", Type: mapSheetProperties.MediaType.String(), - Href: fmt.Sprintf("%v", fgF.Properties[mapSheetProperties.AssetURL]), + Href: fmt.Sprintf("%v", fgF.Properties.Value(mapSheetProperties.AssetURL)), }) } @@ -272,7 +272,7 @@ func (jf *jsonFeatures) createFeatureDownloadLinks(configuredFC *config.Collecti Rel: "enclosure", Title: "Download feature", Type: mapSheetProperties.MediaType.String(), - Href: fmt.Sprintf("%v", feature.Properties[mapSheetProperties.AssetURL]), + Href: fmt.Sprintf("%v", feature.Properties.Value(mapSheetProperties.AssetURL)), }) feature.Links = links } @@ -287,7 +287,7 @@ func (jf *jsonFeatures) createJSONFGFeatureDownloadLinks(configuredFC *config.Co Rel: "enclosure", Title: "Download feature", Type: mapSheetProperties.MediaType.String(), - Href: fmt.Sprintf("%v", feature.Properties[mapSheetProperties.AssetURL]), + Href: fmt.Sprintf("%v", feature.Properties.Value(mapSheetProperties.AssetURL)), }) feature.Links = links } diff --git a/internal/ogc/features/templates/feature.go.html b/internal/ogc/features/templates/feature.go.html index 5d3a5559..bf60a5ae 100644 --- a/internal/ogc/features/templates/feature.go.html +++ b/internal/ogc/features/templates/feature.go.html @@ -47,11 +47,13 @@

{{ .Config.Title }} - {{ if and .Params.Metadata {{ if $mapSheetProperties }} {{ $skipKeys = append $skipKeys $mapSheetProperties.Size }} {{ end }} - {{ range $key, $value := .Params.Properties }} + + {{ range $key := .Params.Keys }} {{- if not (has $key $skipKeys) -}} {{ $key }} + {{ $value := $.Params.Properties.Value $key }} {{- if isdate $value -}} {{ $value | date "2006/01/02 15:04:05" }} diff --git a/internal/ogc/features/templates/features.go.html b/internal/ogc/features/templates/features.go.html index 5e62687d..bc3a9766 100644 --- a/internal/ogc/features/templates/features.go.html +++ b/internal/ogc/features/templates/features.go.html @@ -276,11 +276,13 @@

{{ if $mapSheetProperties }} {{ $skipKeys = append $skipKeys $mapSheetProperties.Size }} {{ end }} - {{ range $key, $value := $feat.Properties }} + + {{ range $key := $feat.Keys }} {{- if not (has $key $skipKeys) -}} {{ $key }} + {{ $value := $feat.Properties.Value $key }} {{- if isdate $value -}} {{ $value | date "2006/01/02 15:04:05" }} diff --git a/internal/ogc/processes/main.go b/internal/ogc/processes/main.go index b8c00895..daf2662a 100644 --- a/internal/ogc/processes/main.go +++ b/internal/ogc/processes/main.go @@ -4,7 +4,6 @@ import ( "net/http" "github.com/PDOK/gokoala/config" - "github.com/PDOK/gokoala/internal/engine" ) From 9ab711bb45377bac1bf27aa6ac7b56c93df2c38d Mon Sep 17 00:00:00 2001 From: Richard Kettelerij Date: Wed, 18 Sep 2024 16:46:37 +0200 Subject: [PATCH 2/8] feat(oaf): Add support for return feature properties in a specific order. --- internal/ogc/features/datasources/geopackage/geopackage.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/ogc/features/datasources/geopackage/geopackage.go b/internal/ogc/features/datasources/geopackage/geopackage.go index e392a8aa..8c86917c 100644 --- a/internal/ogc/features/datasources/geopackage/geopackage.go +++ b/internal/ogc/features/datasources/geopackage/geopackage.go @@ -154,7 +154,7 @@ func (g *GeoPackage) GetFeatureIDs(ctx context.Context, collection string, crite queryCtx, cancel := context.WithTimeout(ctx, g.queryTimeout) // https://go.dev/doc/database/cancel-operations defer cancel() - stmt, query, queryArgs, err := g.makeFeaturesQuery(queryCtx, nil, table, true, criteria) //nolint:sqlclosecheck // prepared statement is cached, will be closed when evicted from cache + stmt, query, queryArgs, err := g.makeFeaturesQuery(queryCtx, g.propertiesByCollectionID[collection], table, true, criteria) //nolint:sqlclosecheck // prepared statement is cached, will be closed when evicted from cache if err != nil { return nil, domain.Cursors{}, fmt.Errorf("failed to create query '%s' error: %w", query, err) } From 749cbc11c3aa893bd0781cec8c95039abab03e8b Mon Sep 17 00:00:00 2001 From: Richard Kettelerij Date: Wed, 18 Sep 2024 21:18:52 +0200 Subject: [PATCH 3/8] feat(oaf): Add support for return feature properties in a specific order. --- internal/ogc/features/domain/geojson.go | 4 ++++ internal/ogc/features/templates/features.go.html | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/internal/ogc/features/domain/geojson.go b/internal/ogc/features/domain/geojson.go index 986c464a..35db8912 100644 --- a/internal/ogc/features/domain/geojson.go +++ b/internal/ogc/features/domain/geojson.go @@ -35,6 +35,10 @@ type Feature struct { Properties orderedmap.OrderedMap[string, any] `json:"properties"` } +// Keys of the Feature properties. +// +// Note: In the future we might replace this with Go 1.23 iterators (range-over-func) however at the moment this +// isn't supported in Go templates: https://github.com/golang/go/pull/68329 func (f *Feature) Keys() []string { result := make([]string, 0, f.Properties.Len()) for pair := f.Properties.Oldest(); pair != nil; pair = pair.Next() { diff --git a/internal/ogc/features/templates/features.go.html b/internal/ogc/features/templates/features.go.html index bc3a9766..bf0a4424 100644 --- a/internal/ogc/features/templates/features.go.html +++ b/internal/ogc/features/templates/features.go.html @@ -280,7 +280,7 @@

{{ range $key := $feat.Keys }} {{- if not (has $key $skipKeys) -}} - {{ $key }} + {{ $key }} {{ $value := $feat.Properties.Value $key }} {{- if isdate $value -}} From 8bb2246492695cee1b2fca00c75d2c7a282bfea6 Mon Sep 17 00:00:00 2001 From: Richard Kettelerij Date: Thu, 19 Sep 2024 15:37:29 +0200 Subject: [PATCH 4/8] feat(oaf): Add support for return feature properties in a specific order. --- config/ogcapi_features.go | 20 +++-- .../datasources/geopackage/geopackage.go | 8 +- internal/ogc/features/domain/geojson.go | 34 ++++---- internal/ogc/features/domain/jsonfg.go | 20 ++--- internal/ogc/features/domain/mapper.go | 9 ++- internal/ogc/features/domain/mapper_test.go | 24 +++--- internal/ogc/features/domain/props.go | 77 +++++++++++++++++++ 7 files changed, 132 insertions(+), 60 deletions(-) create mode 100644 internal/ogc/features/domain/props.go diff --git a/config/ogcapi_features.go b/config/ogcapi_features.go index d76a93ac..e06e2fbf 100644 --- a/config/ogcapi_features.go +++ b/config/ogcapi_features.go @@ -310,22 +310,26 @@ type FeatureFilters struct { // +kubebuilder:object:generate=true type FeatureProperties struct { - // Properties of features in this collection that should be included in the output, in the given order. + // Properties of features in this collection. This setting controls two things: // - // This setting controls two things: + // A) allows one to exclude certain properties, when propertiesIncludeUnknown=false + // B) allows one sort the properties in the given order, when propertiesInSpecificOrder=true // - // A) allows one to exclude certain properties (only when propertiesIncludeUnknown=false) - // B) allows one sort the properties irrespective of their order in the datastore - // - // When not set all available properties are returned in API responses. + // When not set all available properties are returned in API responses, in alphabetical order. // +optional Properties []string `yaml:"properties,omitempty" json:"properties,omitempty"` // When true (default) properties not listed under 'properties' are still available API responses. When false // unlisted properties are excluded from API responses. // +optional - // +kubebuilder:default=true - PropertiesIncludeUnknown *bool `yaml:"propertiesIncludeUnknown,omitempty" json:"propertiesIncludeUnknown,omitempty" default:"true"` // ptr due to https://github.com/creasty/defaults/issues/49 + // +kubebuilder:default=false + PropertiesExcludeUnknown bool `yaml:"propertiesExcludeUnknown,omitempty" json:"propertiesExcludeUnknown,omitempty" default:"false"` + + // When true properties are returned according to the specific ordering specified in the "properties" setting. + // When false properties are returned in alphabetical order. + // +optional + // +kubebuilder:default=false + PropertiesInSpecificOrder bool `yaml:"propertiesInSpecificOrder,omitempty" json:"propertiesInSpecificOrder,omitempty" default:"false"` } // +kubebuilder:object:generate=true diff --git a/internal/ogc/features/datasources/geopackage/geopackage.go b/internal/ogc/features/datasources/geopackage/geopackage.go index 8c86917c..9ee5da5f 100644 --- a/internal/ogc/features/datasources/geopackage/geopackage.go +++ b/internal/ogc/features/datasources/geopackage/geopackage.go @@ -208,7 +208,7 @@ func (g *GeoPackage) GetFeaturesByID(ctx context.Context, collection string, fea fc := domain.FeatureCollection{} fc.Features, _, err = domain.MapRowsToFeatures(rows, g.fidColumn, g.externalFidColumn, table.GeometryColumnName, - mapGpkgGeometry, profile.MapRelationUsingProfile) + g.propertiesByCollectionID[collection], mapGpkgGeometry, profile.MapRelationUsingProfile) if err != nil { return nil, err } @@ -242,7 +242,7 @@ func (g *GeoPackage) GetFeatures(ctx context.Context, collection string, criteri var prevNext *domain.PrevNextFID fc := domain.FeatureCollection{} fc.Features, prevNext, err = domain.MapRowsToFeatures(rows, g.fidColumn, g.externalFidColumn, table.GeometryColumnName, - mapGpkgGeometry, profile.MapRelationUsingProfile) + g.propertiesByCollectionID[collection], mapGpkgGeometry, profile.MapRelationUsingProfile) if err != nil { return nil, domain.Cursors{}, err } @@ -291,7 +291,7 @@ func (g *GeoPackage) GetFeature(ctx context.Context, collection string, featureI } features, _, err := domain.MapRowsToFeatures(rows, g.fidColumn, g.externalFidColumn, table.GeometryColumnName, - mapGpkgGeometry, profile.MapRelationUsingProfile) + g.propertiesByCollectionID[collection], mapGpkgGeometry, profile.MapRelationUsingProfile) if err != nil { return nil, err } @@ -338,7 +338,7 @@ func (g *GeoPackage) makeSelectClause(onlyFIDs bool, propConfig *config.FeatureP return "\"" + g.fidColumn + "\", prevfid, nextfid" } else if propConfig != nil && propConfig.Properties != nil { clause := "\"" + g.fidColumn + "\", prevfid, nextfid, \"" + table.GeometryColumnName + "\", " + strings.Join(propConfig.Properties, ",") - if *propConfig.PropertiesIncludeUnknown { + if !propConfig.PropertiesExcludeUnknown { clause += ", *" } return clause diff --git a/internal/ogc/features/domain/geojson.go b/internal/ogc/features/domain/geojson.go index 35db8912..83ef151d 100644 --- a/internal/ogc/features/domain/geojson.go +++ b/internal/ogc/features/domain/geojson.go @@ -1,8 +1,7 @@ package domain import ( - "github.com/go-spatial/geom/encoding/geojson" - orderedmap "github.com/wk8/go-ordered-map/v2" + "github.com/go-spatial/geom" ) // featureCollectionType allows the GeoJSON type to be automatically set during json marshalling @@ -12,6 +11,13 @@ func (fc *featureCollectionType) MarshalJSON() ([]byte, error) { return []byte(`"FeatureCollection"`), nil } +// featureType allows the type for Feature to be automatically set during json Marshalling +type featureType struct{} + +func (ft *featureType) MarshalJSON() ([]byte, error) { + return []byte(`"Feature"`), nil +} + // FeatureCollection is a GeoJSON FeatureCollection with extras such as links type FeatureCollection struct { Type featureCollectionType `json:"type"` @@ -25,26 +31,18 @@ type FeatureCollection struct { // Feature is a GeoJSON Feature with extras such as links type Feature struct { - geojson.Feature - - // we overwrite ID since we want to make it a required attribute. We also expect feature ids to be - // auto-incrementing integers (which is the default in geopackages) since we use it for cursor-based pagination. - ID string `json:"id"` - Links []Link `json:"links,omitempty"` - - Properties orderedmap.OrderedMap[string, any] `json:"properties"` + // We expect feature ids to be auto-incrementing integers (which is the default in geopackages) + // since we use it for cursor-based pagination. + ID string `json:"id"` + Type featureType `json:"type"` + Geometry geom.Geometry `json:"geometry"` + Links []Link `json:"links,omitempty"` + Properties FeatureProperties `json:"properties"` } // Keys of the Feature properties. -// -// Note: In the future we might replace this with Go 1.23 iterators (range-over-func) however at the moment this -// isn't supported in Go templates: https://github.com/golang/go/pull/68329 func (f *Feature) Keys() []string { - result := make([]string, 0, f.Properties.Len()) - for pair := f.Properties.Oldest(); pair != nil; pair = pair.Next() { - result = append(result, pair.Key) - } - return result + return f.Properties.Keys() } // Link according to RFC 8288, https://datatracker.ietf.org/doc/html/rfc8288 diff --git a/internal/ogc/features/domain/jsonfg.go b/internal/ogc/features/domain/jsonfg.go index 5449903b..a7eb8559 100644 --- a/internal/ogc/features/domain/jsonfg.go +++ b/internal/ogc/features/domain/jsonfg.go @@ -2,20 +2,12 @@ package domain import ( "github.com/go-spatial/geom" - orderedmap "github.com/wk8/go-ordered-map/v2" ) const ( ConformanceJSONFGCore = "http://www.opengis.net/spec/json-fg-1/0.2/conf/core" ) -// featureType allows the type for Feature to be automatically set during json Marshalling -type featureType struct{} - -func (ft *featureType) MarshalJSON() ([]byte, error) { - return []byte(`"Feature"`), nil -} - type JSONFGFeatureCollection struct { Type featureCollectionType `json:"type"` Timestamp string `json:"timeStamp,omitempty"` @@ -34,10 +26,10 @@ type JSONFGFeature struct { Time any `json:"time"` // we don't implement the JSON-FG "3D" conformance class. So Place only // supports simple/2D geometries, no 3D geometries like Polyhedron, Prism, etc. - Place geom.Geometry `json:"place"` // may only contain non-WGS84 geometries - Geometry geom.Geometry `json:"geometry"` // may only contain WGS84 geometries - Properties orderedmap.OrderedMap[string, any] `json:"properties"` - CoordRefSys string `json:"coordRefSys,omitempty"` - Links []Link `json:"links,omitempty"` - ConformsTo []string `json:"conformsTo,omitempty"` + Place geom.Geometry `json:"place"` // may only contain non-WGS84 geometries + Geometry geom.Geometry `json:"geometry"` // may only contain WGS84 geometries + Properties FeatureProperties `json:"properties"` + CoordRefSys string `json:"coordRefSys,omitempty"` + Links []Link `json:"links,omitempty"` + ConformsTo []string `json:"conformsTo,omitempty"` } diff --git a/internal/ogc/features/domain/mapper.go b/internal/ogc/features/domain/mapper.go index 81d08831..b24b346e 100644 --- a/internal/ogc/features/domain/mapper.go +++ b/internal/ogc/features/domain/mapper.go @@ -5,10 +5,11 @@ import ( "strings" "time" + "github.com/PDOK/gokoala/config" + "github.com/go-spatial/geom" "github.com/go-spatial/geom/encoding/geojson" "github.com/jmoiron/sqlx" - orderedmap "github.com/wk8/go-ordered-map/v2" ) // MapRelation abstract function type to map feature relations @@ -49,7 +50,7 @@ func MapRowsToFeatureIDs(rows *sqlx.Rows) (featureIDs []int64, prevNextID *PrevN // MapRowsToFeatures datasource agnostic mapper from SQL rows/result set to Features domain model func MapRowsToFeatures(rows *sqlx.Rows, fidColumn string, externalFidColumn string, geomColumn string, - mapGeom MapGeom, mapRel MapRelation) ([]*Feature, *PrevNextFID, error) { + propConfig *config.FeatureProperties, mapGeom MapGeom, mapRel MapRelation) ([]*Feature, *PrevNextFID, error) { result := make([]*Feature, 0) columns, err := rows.Columns() @@ -57,6 +58,7 @@ func MapRowsToFeatures(rows *sqlx.Rows, fidColumn string, externalFidColumn stri return result, nil, err } + orderProps := propConfig != nil && propConfig.PropertiesInSpecificOrder firstRow := true var prevNextID *PrevNextFID for rows.Next() { @@ -64,8 +66,7 @@ func MapRowsToFeatures(rows *sqlx.Rows, fidColumn string, externalFidColumn stri if values, err = rows.SliceScan(); err != nil { return result, nil, err } - - feature := &Feature{Feature: geojson.Feature{}, Properties: *orderedmap.New[string, any]()} + feature := &Feature{Properties: NewFeatureProperties(orderProps)} np, err := mapColumnsToFeature(firstRow, feature, columns, values, fidColumn, externalFidColumn, geomColumn, mapGeom, mapRel) if err != nil { diff --git a/internal/ogc/features/domain/mapper_test.go b/internal/ogc/features/domain/mapper_test.go index 6d11797f..c6ab0ea1 100644 --- a/internal/ogc/features/domain/mapper_test.go +++ b/internal/ogc/features/domain/mapper_test.go @@ -34,27 +34,27 @@ func TestMapColumnsToFeature(t *testing.T) { }{ { name: "Test FID", - feature: &Feature{Feature: geojson.Feature{Properties: make(map[string]any)}}, + feature: &Feature{Properties: NewFeatureProperties(false)}, columns: []string{"id", "name"}, values: []any{1, "test"}, fidColumn: "id", - expectedFeature: &Feature{ID: "1", Feature: geojson.Feature{Properties: map[string]any{"name": "test"}}}, + expectedFeature: &Feature{ID: "1", Properties: NewFeaturePropertiesWithData(false, map[string]any{"name": "test"})}, expectedPrevNext: &PrevNextFID{}, }, { name: "Test Geometry valid", - feature: &Feature{Feature: geojson.Feature{Properties: make(map[string]any)}}, + feature: &Feature{Properties: NewFeatureProperties(false)}, columns: []string{"id", "geom"}, values: []any{1, []byte("good")}, fidColumn: "id", geomColumn: "geom", mapGeom: mockMapGeom, - expectedFeature: &Feature{ID: "1", Feature: geojson.Feature{Properties: map[string]any{}, Geometry: geojson.Geometry{Geometry: geom.Point{1.0, 2.0}}}}, + expectedFeature: &Feature{ID: "1", Properties: NewFeaturePropertiesWithData(false, map[string]any{}), Geometry: geojson.Geometry{Geometry: geom.Point{1.0, 2.0}}}, expectedPrevNext: &PrevNextFID{}, }, { name: "Test Geometry invalid", - feature: &Feature{Feature: geojson.Feature{Properties: make(map[string]any)}}, + feature: &Feature{Properties: NewFeatureProperties(false)}, columns: []string{"id", "geom"}, values: []any{1, []byte("mock error")}, fidColumn: "id", @@ -65,31 +65,31 @@ func TestMapColumnsToFeature(t *testing.T) { { name: "Test prevfid and nextfid", firstRow: true, - feature: &Feature{Feature: geojson.Feature{Properties: make(map[string]any)}}, + feature: &Feature{Properties: NewFeatureProperties(false)}, columns: []string{"prevfid", "nextfid"}, values: []any{int64(1), int64(2)}, - expectedFeature: &Feature{Feature: geojson.Feature{Properties: make(map[string]any)}}, + expectedFeature: &Feature{Properties: NewFeatureProperties(false)}, expectedPrevNext: &PrevNextFID{Prev: int64(1), Next: int64(2)}, }, { name: "Test different types", - feature: &Feature{Feature: geojson.Feature{Properties: make(map[string]any)}}, + feature: &Feature{Properties: NewFeatureProperties(false)}, columns: []string{"str_col", "int_col", "float_col", "time_col", "bool_col"}, values: []any{"str", int64(42), 3.14, time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC), true}, - expectedFeature: &Feature{Feature: geojson.Feature{Properties: map[string]any{"str_col": "str", "int_col": int64(42), "float_col": 3.14, "time_col": time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC), "bool_col": true}}}, + expectedFeature: &Feature{Properties: NewFeaturePropertiesWithData(false, map[string]any{"str_col": "str", "int_col": int64(42), "float_col": 3.14, "time_col": time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC), "bool_col": true})}, expectedPrevNext: &PrevNextFID{}, }, { name: "Test nil value", - feature: &Feature{Feature: geojson.Feature{Properties: make(map[string]any)}}, + feature: &Feature{Properties: NewFeatureProperties(false)}, columns: []string{"str_col", "nil_col"}, values: []any{"str", nil}, - expectedFeature: &Feature{Feature: geojson.Feature{Properties: map[string]any{"str_col": "str", "nil_col": nil}}}, + expectedFeature: &Feature{Properties: NewFeaturePropertiesWithData(false, map[string]any{"str_col": "str", "nil_col": nil})}, expectedPrevNext: &PrevNextFID{}, }, { name: "Test unexpected type", - feature: &Feature{Feature: geojson.Feature{Properties: make(map[string]any)}}, + feature: &Feature{Properties: NewFeatureProperties(false)}, columns: []string{"str_col", "unexpected_col"}, values: []any{"str", []complex128{complex(1, 2)}}, expectedError: errors.New("unexpected type for sqlite column data: unexpected_col: []complex128"), diff --git a/internal/ogc/features/domain/props.go b/internal/ogc/features/domain/props.go new file mode 100644 index 00000000..ffd9e194 --- /dev/null +++ b/internal/ogc/features/domain/props.go @@ -0,0 +1,77 @@ +package domain + +import ( + "github.com/PDOK/gokoala/internal/engine/util" + perfjson "github.com/goccy/go-json" + orderedmap "github.com/wk8/go-ordered-map/v2" +) + +// FeatureProperties the properties of a GeoJSON Feature +// Either unordered (= default, and has the best performance) or in a specific order when configured as such. +type FeatureProperties struct { + unordered map[string]any + ordered orderedmap.OrderedMap[string, any] +} + +func NewFeatureProperties(order bool) FeatureProperties { + return NewFeaturePropertiesWithData(order, make(map[string]any)) +} + +func NewFeaturePropertiesWithData(order bool, data map[string]any) FeatureProperties { + if order { + ordered := *orderedmap.New[string, any]() + for k, v := range data { + ordered.Set(k, v) + } + return FeatureProperties{ordered: ordered} + } + return FeatureProperties{unordered: data} +} + +// MarshalJSON returns the JSON representation of either the ordered or unordered properties +func (p *FeatureProperties) MarshalJSON() ([]byte, error) { + if p.unordered != nil { + // properties are allowed to contain anything, including for example XML/GML. + b, e := perfjson.MarshalWithOption(p.unordered, perfjson.DisableHTMLEscape()) + return b, e + } + return p.ordered.MarshalJSON() +} + +func (p *FeatureProperties) Set(key string, value any) { + if p.unordered != nil { + p.unordered[key] = value + } else { + p.ordered.Set(key, value) + } +} + +func (p *FeatureProperties) Value(key string) any { + if p.unordered != nil { + return p.unordered[key] + } + return p.ordered.Value(key) +} + +func (p *FeatureProperties) Delete(key string) { + if p.unordered != nil { + delete(p.unordered, key) + } else { + p.ordered.Delete(key) + } +} + +// Keys of the Feature properties. +// +// Note: In the future we might replace this with Go 1.23 iterators (range-over-func) however at the moment this +// isn't supported in Go templates: https://github.com/golang/go/pull/68329 +func (p *FeatureProperties) Keys() []string { + if p.unordered != nil { + return util.Keys(p.unordered) + } + result := make([]string, 0, p.ordered.Len()) + for pair := p.ordered.Oldest(); pair != nil; pair = pair.Next() { + result = append(result, pair.Key) + } + return result +} From 65455947890b03775935d0511ae2daf2a6474d9c Mon Sep 17 00:00:00 2001 From: Richard Kettelerij Date: Thu, 19 Sep 2024 16:15:14 +0200 Subject: [PATCH 5/8] feat(oaf): Add support for return feature properties in a specific order. --- .../datasources/geopackage/geopackage_test.go | 90 ++++++++----------- internal/ogc/features/domain/props.go | 6 +- .../ogc/features/templates/feature.go.html | 6 +- .../ogc/features/templates/features.go.html | 6 +- ...ted_features_with_rel_as_link_snippet.html | 2 +- .../expected_foo_collection_snippet.html | 28 +++--- 6 files changed, 63 insertions(+), 75 deletions(-) diff --git a/internal/ogc/features/datasources/geopackage/geopackage_test.go b/internal/ogc/features/datasources/geopackage/geopackage_test.go index 50b8bbcf..897d65e6 100644 --- a/internal/ogc/features/datasources/geopackage/geopackage_test.go +++ b/internal/ogc/features/datasources/geopackage/geopackage_test.go @@ -12,7 +12,6 @@ import ( "github.com/PDOK/gokoala/internal/ogc/features/datasources" "github.com/PDOK/gokoala/internal/ogc/features/domain" - "github.com/go-spatial/geom/encoding/geojson" "github.com/stretchr/testify/assert" ) @@ -129,20 +128,16 @@ func TestGeoPackage_GetFeatures(t *testing.T) { NumberReturned: 2, Features: []*domain.Feature{ { - Feature: geojson.Feature{ - Properties: map[string]any{ - "straatnaam": "Van Diemenkade", - "nummer_id": "0363200000454013", - }, - }, + Properties: domain.NewFeaturePropertiesWithData(false, map[string]any{ + "straatnaam": "Van Diemenkade", + "nummer_id": "0363200000454013", + }), }, { - Feature: geojson.Feature{ - Properties: map[string]any{ - "straatnaam": "Realengracht", - "nummer_id": "0363200000398886", - }, - }, + Properties: domain.NewFeaturePropertiesWithData(false, map[string]any{ + "straatnaam": "Realengracht", + "nummer_id": "0363200000398886", + }), }, }, }, @@ -175,28 +170,23 @@ func TestGeoPackage_GetFeatures(t *testing.T) { NumberReturned: 3, Features: []*domain.Feature{ { - Feature: geojson.Feature{ - Properties: map[string]any{ - "straatnaam": "Realengracht", - "nummer_id": "0363200000398887", - }, - }, + Properties: domain.NewFeaturePropertiesWithData(false, map[string]any{ + "straatnaam": "Realengracht", + "nummer_id": "0363200000398887", + }), }, { - Feature: geojson.Feature{ - Properties: map[string]any{ - "straatnaam": "Realengracht", - "nummer_id": "0363200000398888", - }, - }, + + Properties: domain.NewFeaturePropertiesWithData(false, map[string]any{ + "straatnaam": "Realengracht", + "nummer_id": "0363200000398888", + }), }, { - Feature: geojson.Feature{ - Properties: map[string]any{ - "straatnaam": "Realengracht", - "nummer_id": "0363200000398889", - }, - }, + Properties: domain.NewFeaturePropertiesWithData(false, map[string]any{ + "straatnaam": "Realengracht", + "nummer_id": "0363200000398889", + }), }, }, }, @@ -231,20 +221,16 @@ func TestGeoPackage_GetFeatures(t *testing.T) { NumberReturned: 2, Features: []*domain.Feature{ { - Feature: geojson.Feature{ - Properties: map[string]any{ - "straatnaam": "Van Diemenkade", - "nummer_id": "0363200000454013", - }, - }, + Properties: domain.NewFeaturePropertiesWithData(false, map[string]any{ + "straatnaam": "Van Diemenkade", + "nummer_id": "0363200000454013", + }), }, { - Feature: geojson.Feature{ - Properties: map[string]any{ - "straatnaam": "Realengracht", - "nummer_id": "0363200000398886", - }, - }, + Properties: domain.NewFeaturePropertiesWithData(false, map[string]any{ + "straatnaam": "Realengracht", + "nummer_id": "0363200000398886", + }), }, }, }, @@ -296,8 +282,8 @@ func TestGeoPackage_GetFeatures(t *testing.T) { assert.Equal(t, tt.wantFC.NumberReturned, fc.NumberReturned) assert.Equal(t, len(tt.wantFC.Features), fc.NumberReturned) for i, wantedFeature := range tt.wantFC.Features { - assert.Equal(t, wantedFeature.Properties["straatnaam"], fc.Features[i].Properties["straatnaam"]) - assert.Equal(t, wantedFeature.Properties["nummer_id"], fc.Features[i].Properties["nummer_id"]) + assert.Equal(t, wantedFeature.Properties.Value("straatnaam"), fc.Features[i].Properties.Value("straatnaam")) + assert.Equal(t, wantedFeature.Properties.Value("nummer_id"), fc.Features[i].Properties.Value("nummer_id")) } assert.Equal(t, tt.wantCursor.Prev, cursor.Prev) assert.Equal(t, tt.wantCursor.Next, cursor.Next) @@ -340,12 +326,10 @@ func TestGeoPackage_GetFeature(t *testing.T) { want: &domain.Feature{ ID: "0", Links: nil, - Feature: geojson.Feature{ - Properties: map[string]any{ - "straatnaam": "Realengracht", - "nummer_id": "0363200000398886", - }, - }, + Properties: domain.NewFeaturePropertiesWithData(false, map[string]any{ + "straatnaam": "Realengracht", + "nummer_id": "0363200000398886", + }), }, wantErr: false, }, @@ -400,8 +384,8 @@ func TestGeoPackage_GetFeature(t *testing.T) { return } if tt.want != nil { - assert.Equal(t, tt.want.Properties["straatnaam"], got.Properties["straatnaam"]) - assert.Equal(t, tt.want.Properties["nummer_id"], got.Properties["nummer_id"]) + assert.Equal(t, tt.want.Properties.Value("straatnaam"), got.Properties.Value("straatnaam")) + assert.Equal(t, tt.want.Properties.Value("nummer_id"), got.Properties.Value("nummer_id")) } }) } diff --git a/internal/ogc/features/domain/props.go b/internal/ogc/features/domain/props.go index ffd9e194..86502b0b 100644 --- a/internal/ogc/features/domain/props.go +++ b/internal/ogc/features/domain/props.go @@ -1,6 +1,8 @@ package domain import ( + "slices" + "github.com/PDOK/gokoala/internal/engine/util" perfjson "github.com/goccy/go-json" orderedmap "github.com/wk8/go-ordered-map/v2" @@ -67,7 +69,9 @@ func (p *FeatureProperties) Delete(key string) { // isn't supported in Go templates: https://github.com/golang/go/pull/68329 func (p *FeatureProperties) Keys() []string { if p.unordered != nil { - return util.Keys(p.unordered) + keys := util.Keys(p.unordered) + slices.Sort(keys) // preserve alphabetical order + return keys } result := make([]string, 0, p.ordered.Len()) for pair := p.ordered.Oldest(); pair != nil; pair = pair.Next() { diff --git a/internal/ogc/features/templates/feature.go.html b/internal/ogc/features/templates/feature.go.html index bf60a5ae..9cb99d9d 100644 --- a/internal/ogc/features/templates/feature.go.html +++ b/internal/ogc/features/templates/feature.go.html @@ -72,11 +72,11 @@

{{ .Config.Title }} - {{ if and .Params.Metadata {{- end -}} {{ end }} {{/* for map sheets collection, add download button */}} - {{ if $mapSheetProperties }} + {{ if and $mapSheetProperties .Params.Properties }} - + Download - ({{ i18n "Size" }}: {{ humansize (get .Params.Properties $mapSheetProperties.Size) }}) + ({{ i18n "Size" }}: {{ humansize (.Params.Properties.Value $mapSheetProperties.Size) }}) {{ end }} diff --git a/internal/ogc/features/templates/features.go.html b/internal/ogc/features/templates/features.go.html index bf0a4424..873cf228 100644 --- a/internal/ogc/features/templates/features.go.html +++ b/internal/ogc/features/templates/features.go.html @@ -301,11 +301,11 @@

{{- end -}} {{ end }} {{/* for map sheets collection, add download button */}} - {{ if $mapSheetProperties }} + {{ if and $mapSheetProperties $feat.Properties }} - + Download - ({{ i18n "Size" }}: {{ humansize (get $feat.Properties $mapSheetProperties.Size) }}) + ({{ i18n "Size" }}: {{ humansize ($feat.Properties.Value $mapSheetProperties.Size) }}) {{ end }} diff --git a/internal/ogc/features/testdata/expected_features_with_rel_as_link_snippet.html b/internal/ogc/features/testdata/expected_features_with_rel_as_link_snippet.html index 4ad68c1b..cd914566 100644 --- a/internal/ogc/features/testdata/expected_features_with_rel_as_link_snippet.html +++ b/internal/ogc/features/testdata/expected_features_with_rel_as_link_snippet.html @@ -1,4 +1,4 @@ - ligplaatsen.href + ligplaatsen.href http://localhost:8080/collections/ligplaatsen/items/093ae17f-8e83-5560-bbd0-b199227af170 \ No newline at end of file diff --git a/internal/ogc/features/testdata/expected_foo_collection_snippet.html b/internal/ogc/features/testdata/expected_foo_collection_snippet.html index 88f74e79..5e398647 100644 --- a/internal/ogc/features/testdata/expected_foo_collection_snippet.html +++ b/internal/ogc/features/testdata/expected_foo_collection_snippet.html @@ -7,72 +7,72 @@ - datum_doc + datum_doc 1900-01-01 - datum_eind + datum_eind - datum_strt + datum_strt 1900-01-01 - document + document GV00000402 - huisletter + huisletter - huisnummer + huisnummer 14 - nummer_id + nummer_id 0363200000454013 - postcode + postcode 1013CR - rdf_seealso + rdf_seealso http://bag.basisregistraties.overheid.nl/bag/id/nummeraanduiding/0363200000454013 - status + status Naamgeving uitgegeven - straatnaam + straatnaam Van Diemenkade - toevoeging + toevoeging - type + type Ligplaats - woonplaats + woonplaats Amsterdam From 66c2ea17d10df8bc4279a66d49ada012143fbf4f Mon Sep 17 00:00:00 2001 From: Richard Kettelerij Date: Thu, 19 Sep 2024 16:34:06 +0200 Subject: [PATCH 6/8] feat(oaf): Add support for return feature properties in a specific order. --- config/ogcapi_features.go | 12 ++++++------ internal/ogc/features/domain/geojson.go | 25 +++++++++++++------------ internal/ogc/features/domain/jsonfg.go | 6 +++++- 3 files changed, 24 insertions(+), 19 deletions(-) diff --git a/config/ogcapi_features.go b/config/ogcapi_features.go index e06e2fbf..6fab334a 100644 --- a/config/ogcapi_features.go +++ b/config/ogcapi_features.go @@ -310,23 +310,23 @@ type FeatureFilters struct { // +kubebuilder:object:generate=true type FeatureProperties struct { - // Properties of features in this collection. This setting controls two things: + // Properties/fields of features in this collection. This setting controls two things: // - // A) allows one to exclude certain properties, when propertiesIncludeUnknown=false + // A) allows one to exclude certain properties, when propertiesExcludeUnknown=true // B) allows one sort the properties in the given order, when propertiesInSpecificOrder=true // // When not set all available properties are returned in API responses, in alphabetical order. // +optional Properties []string `yaml:"properties,omitempty" json:"properties,omitempty"` - // When true (default) properties not listed under 'properties' are still available API responses. When false - // unlisted properties are excluded from API responses. + // When true properties not listed under 'properties' are excluded from API responses. When false + // unlisted properties are also included in API responses. // +optional // +kubebuilder:default=false PropertiesExcludeUnknown bool `yaml:"propertiesExcludeUnknown,omitempty" json:"propertiesExcludeUnknown,omitempty" default:"false"` - // When true properties are returned according to the specific ordering specified in the "properties" setting. - // When false properties are returned in alphabetical order. + // When true properties are returned according to the ordering specified under 'properties'. When false + // properties are returned in alphabetical order. // +optional // +kubebuilder:default=false PropertiesInSpecificOrder bool `yaml:"propertiesInSpecificOrder,omitempty" json:"propertiesInSpecificOrder,omitempty" default:"false"` diff --git a/internal/ogc/features/domain/geojson.go b/internal/ogc/features/domain/geojson.go index 83ef151d..99c37973 100644 --- a/internal/ogc/features/domain/geojson.go +++ b/internal/ogc/features/domain/geojson.go @@ -19,25 +19,25 @@ func (ft *featureType) MarshalJSON() ([]byte, error) { } // FeatureCollection is a GeoJSON FeatureCollection with extras such as links +// Note: fields in this struct are sorted for optimal memory usage (field alignment) type FeatureCollection struct { - Type featureCollectionType `json:"type"` - Timestamp string `json:"timeStamp,omitempty"` - Links []Link `json:"links,omitempty"` - - Features []*Feature `json:"features"` - - NumberReturned int `json:"numberReturned"` + Type featureCollectionType `json:"type"` + Timestamp string `json:"timeStamp,omitempty"` + Links []Link `json:"links,omitempty"` + Features []*Feature `json:"features"` + NumberReturned int `json:"numberReturned"` } // Feature is a GeoJSON Feature with extras such as links +// Note: fields in this struct are sorted for optimal memory usage (field alignment) type Feature struct { - // We expect feature ids to be auto-incrementing integers (which is the default in geopackages) - // since we use it for cursor-based pagination. - ID string `json:"id"` Type featureType `json:"type"` - Geometry geom.Geometry `json:"geometry"` - Links []Link `json:"links,omitempty"` Properties FeatureProperties `json:"properties"` + Geometry geom.Geometry `json:"geometry"` + // We expect feature ids to be auto-incrementing integers (which is the default in geopackages) + // since we use it for cursor-based pagination. + ID string `json:"id"` + Links []Link `json:"links,omitempty"` } // Keys of the Feature properties. @@ -46,6 +46,7 @@ func (f *Feature) Keys() []string { } // Link according to RFC 8288, https://datatracker.ietf.org/doc/html/rfc8288 +// Note: fields in this struct are sorted for optimal memory usage (field alignment) type Link struct { Rel string `json:"rel"` Title string `json:"title,omitempty"` diff --git a/internal/ogc/features/domain/jsonfg.go b/internal/ogc/features/domain/jsonfg.go index a7eb8559..887d5265 100644 --- a/internal/ogc/features/domain/jsonfg.go +++ b/internal/ogc/features/domain/jsonfg.go @@ -8,6 +8,8 @@ const ( ConformanceJSONFGCore = "http://www.opengis.net/spec/json-fg-1/0.2/conf/core" ) +// JSONFGFeatureCollection FeatureCollection according to the JSON-FG standard +// Note: fields in this struct are sorted for optimal memory usage (field alignment) type JSONFGFeatureCollection struct { Type featureCollectionType `json:"type"` Timestamp string `json:"timeStamp,omitempty"` @@ -18,13 +20,15 @@ type JSONFGFeatureCollection struct { NumberReturned int `json:"numberReturned"` } +// JSONFGFeature Feature according to the JSON-FG standard +// Note: fields in this struct are sorted for optimal memory usage (field alignment) type JSONFGFeature struct { // We expect feature ids to be auto-incrementing integers (which is the default in geopackages) // since we use it for cursor-based pagination. ID string `json:"id"` Type featureType `json:"type"` Time any `json:"time"` - // we don't implement the JSON-FG "3D" conformance class. So Place only + // We don't implement the JSON-FG "3D" conformance class. So Place only // supports simple/2D geometries, no 3D geometries like Polyhedron, Prism, etc. Place geom.Geometry `json:"place"` // may only contain non-WGS84 geometries Geometry geom.Geometry `json:"geometry"` // may only contain WGS84 geometries From a0f274afe1d0499c79336636e38bd5954ab04522 Mon Sep 17 00:00:00 2001 From: Richard Kettelerij Date: Fri, 20 Sep 2024 10:06:18 +0200 Subject: [PATCH 7/8] feat(oaf): Add support to return feature properties in a specific order + take relations into account. --- internal/ogc/features/domain/mapper.go | 7 ++-- internal/ogc/features/domain/props.go | 47 +++++++++++++++++++------- 2 files changed, 39 insertions(+), 15 deletions(-) diff --git a/internal/ogc/features/domain/mapper.go b/internal/ogc/features/domain/mapper.go index b24b346e..9fcc16dc 100644 --- a/internal/ogc/features/domain/mapper.go +++ b/internal/ogc/features/domain/mapper.go @@ -58,7 +58,7 @@ func MapRowsToFeatures(rows *sqlx.Rows, fidColumn string, externalFidColumn stri return result, nil, err } - orderProps := propConfig != nil && propConfig.PropertiesInSpecificOrder + propertiesOrder := propConfig != nil && propConfig.PropertiesInSpecificOrder firstRow := true var prevNextID *PrevNextFID for rows.Next() { @@ -66,7 +66,7 @@ func MapRowsToFeatures(rows *sqlx.Rows, fidColumn string, externalFidColumn stri if values, err = rows.SliceScan(); err != nil { return result, nil, err } - feature := &Feature{Properties: NewFeatureProperties(orderProps)} + feature := &Feature{Properties: NewFeatureProperties(propertiesOrder)} np, err := mapColumnsToFeature(firstRow, feature, columns, values, fidColumn, externalFidColumn, geomColumn, mapGeom, mapRel) if err != nil { @@ -173,7 +173,8 @@ func mapExternalFid(columns []string, values []any, externalFidColumn string, fe // it as a relation to another feature. newColumnName, newColumnValue := mapRel(columnName, columnValue, externalFidColumn) if newColumnName != "" { - feature.Properties.Set(newColumnName, newColumnValue) + columnNameWithoutExternalFID := strings.ReplaceAll(columnName, externalFidColumn, "") + feature.Properties.SetRelation(newColumnName, newColumnValue, columnNameWithoutExternalFID) feature.Properties.Delete(columnName) } } diff --git a/internal/ogc/features/domain/props.go b/internal/ogc/features/domain/props.go index 86502b0b..05719dd8 100644 --- a/internal/ogc/features/domain/props.go +++ b/internal/ogc/features/domain/props.go @@ -2,14 +2,15 @@ package domain import ( "slices" + "strings" "github.com/PDOK/gokoala/internal/engine/util" perfjson "github.com/goccy/go-json" orderedmap "github.com/wk8/go-ordered-map/v2" ) -// FeatureProperties the properties of a GeoJSON Feature -// Either unordered (= default, and has the best performance) or in a specific order when configured as such. +// FeatureProperties the properties of a GeoJSON Feature. Properties are either unordered +// (default, and has the best performance!) or ordered in a specific way as described in the config. type FeatureProperties struct { unordered map[string]any ordered orderedmap.OrderedMap[string, any] @@ -34,20 +35,11 @@ func NewFeaturePropertiesWithData(order bool, data map[string]any) FeatureProper func (p *FeatureProperties) MarshalJSON() ([]byte, error) { if p.unordered != nil { // properties are allowed to contain anything, including for example XML/GML. - b, e := perfjson.MarshalWithOption(p.unordered, perfjson.DisableHTMLEscape()) - return b, e + return perfjson.MarshalWithOption(p.unordered, perfjson.DisableHTMLEscape()) } return p.ordered.MarshalJSON() } -func (p *FeatureProperties) Set(key string, value any) { - if p.unordered != nil { - p.unordered[key] = value - } else { - p.ordered.Set(key, value) - } -} - func (p *FeatureProperties) Value(key string) any { if p.unordered != nil { return p.unordered[key] @@ -63,6 +55,37 @@ func (p *FeatureProperties) Delete(key string) { } } +func (p *FeatureProperties) Set(key string, value any) { + if p.unordered != nil { + p.unordered[key] = value + } else { + p.ordered.Set(key, value) + } +} + +func (p *FeatureProperties) SetRelation(key string, value any, existingKeyPrefix string) { + p.Set(key, value) + p.moveKeyBeforePrefix(key, existingKeyPrefix) +} + +// moveKeyBeforePrefix best-effort algorithm to place the feature relation BEFORE any similarly named keys. +// For example, places "building.href" before "building_fk" or "building_fid". +func (p *FeatureProperties) moveKeyBeforePrefix(key string, keyPrefix string) { + if p.unordered != nil { + return + } + var existingKey string + for pair := p.ordered.Oldest(); pair != nil; pair = pair.Next() { + if strings.HasPrefix(pair.Key, keyPrefix) { + existingKey = pair.Key + break + } + } + if existingKey != "" { + _ = p.ordered.MoveBefore(key, existingKey) + } +} + // Keys of the Feature properties. // // Note: In the future we might replace this with Go 1.23 iterators (range-over-func) however at the moment this From e269e1c8abffe6f4b44940380083a82cc972e04c Mon Sep 17 00:00:00 2001 From: Richard Kettelerij Date: Fri, 20 Sep 2024 10:06:54 +0200 Subject: [PATCH 8/8] chore(go): Upgrade to Go 1.23 --- .github/workflows/lint-go.yml | 2 +- .github/workflows/test-go.yml | 2 +- Dockerfile | 2 +- go.mod | 2 +- hack/crd/Dockerfile | 4 ++-- hack/crd/go.mod | 2 +- hack/crd/go.sum | 1 + 7 files changed, 8 insertions(+), 7 deletions(-) diff --git a/.github/workflows/lint-go.yml b/.github/workflows/lint-go.yml index 8f6fde0c..23dadf16 100644 --- a/.github/workflows/lint-go.yml +++ b/.github/workflows/lint-go.yml @@ -17,7 +17,7 @@ jobs: - uses: actions/setup-go@v4 with: - go-version: '1.22' + go-version: '1.23' cache: false - uses: actions/checkout@v3 diff --git a/.github/workflows/test-go.yml b/.github/workflows/test-go.yml index bd4fa6b9..ac5dde16 100644 --- a/.github/workflows/test-go.yml +++ b/.github/workflows/test-go.yml @@ -19,7 +19,7 @@ jobs: - name: Set up Go uses: actions/setup-go@v4 with: - go-version: '1.22' + go-version: '1.23' - name: Download run: go mod download all diff --git a/Dockerfile b/Dockerfile index cf8c0250..55d0f89d 100644 --- a/Dockerfile +++ b/Dockerfile @@ -7,7 +7,7 @@ RUN npm install RUN npm run build ####### Go build -FROM docker.io/golang:1.22-bookworm AS build-env +FROM docker.io/golang:1.23-bookworm AS build-env WORKDIR /go/src/service ADD . /go/src/service diff --git a/go.mod b/go.mod index 83cd2702..403f46a3 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/PDOK/gokoala -go 1.22.5 +go 1.23.1 require ( dario.cat/mergo v1.0.0 diff --git a/hack/crd/Dockerfile b/hack/crd/Dockerfile index 79a93c81..7b9dfb8c 100644 --- a/hack/crd/Dockerfile +++ b/hack/crd/Dockerfile @@ -1,7 +1,7 @@ -FROM docker.io/golang:1.22-bookworm AS build-env +FROM docker.io/golang:1.23-bookworm AS build-env ADD hack/build-controller-gen.sh /build-controller-gen.sh RUN /build-controller-gen.sh -FROM docker.io/golang:1.22-bookworm +FROM docker.io/golang:1.23-bookworm COPY --from=build-env /bin/controller-gen /bin/controller-gen ENTRYPOINT ["/bin/controller-gen"] \ No newline at end of file diff --git a/hack/crd/go.mod b/hack/crd/go.mod index 39c762d6..648ccea3 100644 --- a/hack/crd/go.mod +++ b/hack/crd/go.mod @@ -1,6 +1,6 @@ module crd -go 1.22.5 +go 1.23.1 require ( github.com/PDOK/gokoala v0.0.0 diff --git a/hack/crd/go.sum b/hack/crd/go.sum index 0e7e037f..c636c51a 100644 --- a/hack/crd/go.sum +++ b/hack/crd/go.sum @@ -57,6 +57,7 @@ github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsT github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= github.com/wk8/go-ordered-map/v2 v2.1.8 h1:5h/BUHu93oj4gIdvHHHGsScSTMijfx5PeYkE/fJgbpc= github.com/wk8/go-ordered-map/v2 v2.1.8/go.mod h1:5nJHM5DyteebpVlHnWMV0rPz6Zp7+xBAnxjb1X5vnTw= +github.com/wk8/go-ordered-map/v2 v2.1.9-0.20240816141633-0a40785b4f41/go.mod h1:DbzwytT4g/odXquuOCqroKvtxxldI4nb3nuesHF/Exo= github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=