Skip to content

Commit

Permalink
Merge pull request #205 from PDOK/PDOK/persistent-feature-ids
Browse files Browse the repository at this point in the history
Allow use of persistent feature ids (UUIDv5)
  • Loading branch information
kad-korpem authored Jun 25, 2024
2 parents c3edfd7 + fd6da44 commit 725610e
Show file tree
Hide file tree
Showing 45 changed files with 206 additions and 172 deletions.
5 changes: 5 additions & 0 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -715,6 +715,11 @@ type GeoPackageCommon struct {
// +optional
Fid string `yaml:"fid,omitempty" json:"fid,omitempty" validate:"required" default:"fid"`

// External feature id column name. When specified this ID column will be exposed to clients instead of the regular FID column.
// It allows one to offer a more stable ID to clients instead of an auto-generated FID. External FID column should contain UUIDs.
// +optional
ExternalFid string `yaml:"externalFid" json:"externalFid"`

// Optional timeout after which queries are canceled
// +kubebuilder:default="15s"
// +optional
Expand Down
3 changes: 3 additions & 0 deletions examples/config_features_local.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -46,17 +46,20 @@ ogcApi:
geopackage:
local:
file: ./examples/resources/addresses-crs84.gpkg
externalFid: external_fid
queryTimeout: 5s
additional:
- srs: EPSG:28992
geopackage:
local:
file: ./examples/resources/addresses-rd.gpkg
externalFid: external_fid
- srs: EPSG:3035
geopackage:
local:
file: ./examples/resources/addresses-etrs89.gpkg
fid: fid
externalFid: external_fid
queryTimeout: 10s
collections:
- id: dutch-addresses
Expand Down
Binary file modified examples/resources/addresses-crs84.gpkg
Binary file not shown.
Binary file modified examples/resources/addresses-etrs89.gpkg
Binary file not shown.
Binary file modified examples/resources/addresses-rd.gpkg
Binary file not shown.
4 changes: 2 additions & 2 deletions internal/ogc/features/datasources/datasource.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ type Datasource interface {
// GetFeatures returns all Features matching the given criteria and Cursors for pagination
GetFeatures(ctx context.Context, collection string, criteria FeaturesCriteria) (*domain.FeatureCollection, domain.Cursors, error)

// GetFeature returns a specific Feature
GetFeature(ctx context.Context, collection string, featureID int64) (*domain.Feature, error)
// GetFeature returns a specific Feature, based on its feature id
GetFeature(ctx context.Context, collection string, featureID any) (*domain.Feature, error)

// GetFeatureTableMetadata returns metadata about a feature table associated with the given collection
GetFeatureTableMetadata(collection string) (FeatureTableMetadata, error)
Expand Down
32 changes: 27 additions & 5 deletions internal/ogc/features/datasources/geopackage/geopackage.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"github.com/go-spatial/geom"
"github.com/go-spatial/geom/encoding/gpkg"
"github.com/go-spatial/geom/encoding/wkt"
"github.com/google/uuid"
"github.com/jmoiron/sqlx"
"github.com/mattn/go-sqlite3"
"github.com/qustavo/sqlhooks/v2"
Expand Down Expand Up @@ -74,6 +75,7 @@ type GeoPackage struct {
preparedStmtCache *PreparedStatementCache

fidColumn string
externalFidColumn string
featureTableByCollectionID map[string]*featureTable
propertyFiltersByCollectionID map[string]datasources.PropertyFiltersWithAllowedValues
queryTimeout time.Duration
Expand All @@ -91,11 +93,13 @@ func NewGeoPackage(collections config.GeoSpatialCollections, gpkgConfig config.G
case gpkgConfig.Local != nil:
g.backend = newLocalGeoPackage(gpkgConfig.Local)
g.fidColumn = gpkgConfig.Local.Fid
g.externalFidColumn = gpkgConfig.Local.ExternalFid
g.queryTimeout = gpkgConfig.Local.QueryTimeout.Duration
g.maxBBoxSizeToUseWithRTree = gpkgConfig.Local.MaxBBoxSizeToUseWithRTree
case gpkgConfig.Cloud != nil:
g.backend = newCloudBackedGeoPackage(gpkgConfig.Cloud)
g.fidColumn = gpkgConfig.Cloud.Fid
g.externalFidColumn = gpkgConfig.Cloud.ExternalFid
g.queryTimeout = gpkgConfig.Cloud.QueryTimeout.Duration
g.maxBBoxSizeToUseWithRTree = gpkgConfig.Cloud.MaxBBoxSizeToUseWithRTree
warmUp = gpkgConfig.Cloud.Cache.WarmUp
Expand Down Expand Up @@ -193,7 +197,7 @@ func (g *GeoPackage) GetFeaturesByID(ctx context.Context, collection string, fea
defer rows.Close()

fc := domain.FeatureCollection{}
fc.Features, _, err = domain.MapRowsToFeatures(rows, g.fidColumn, table.GeometryColumnName, readGpkgGeometry)
fc.Features, _, err = domain.MapRowsToFeatures(rows, g.fidColumn, g.externalFidColumn, table.GeometryColumnName, readGpkgGeometry)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -223,7 +227,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, table.GeometryColumnName, readGpkgGeometry)
fc.Features, prevNext, err = domain.MapRowsToFeatures(rows, g.fidColumn, g.externalFidColumn, table.GeometryColumnName, readGpkgGeometry)
if err != nil {
return nil, domain.Cursors{}, err
}
Expand All @@ -234,7 +238,7 @@ func (g *GeoPackage) GetFeatures(ctx context.Context, collection string, criteri
return &fc, domain.NewCursors(*prevNext, criteria.Cursor.FiltersChecksum), nil
}

func (g *GeoPackage) GetFeature(ctx context.Context, collection string, featureID int64) (*domain.Feature, error) {
func (g *GeoPackage) GetFeature(ctx context.Context, collection string, featureID any) (*domain.Feature, error) {
table, err := g.getFeatureTable(collection)
if err != nil {
return nil, err
Expand All @@ -243,14 +247,32 @@ func (g *GeoPackage) GetFeature(ctx context.Context, collection string, featureI
queryCtx, cancel := context.WithTimeout(ctx, g.queryTimeout) // https://go.dev/doc/database/cancel-operations
defer cancel()

query := fmt.Sprintf("select * from %s f where f.%s = :fid limit 1", table.TableName, g.fidColumn)
var fidColumn string
switch featureID.(type) {
case int64:
if g.externalFidColumn != "" {
// Features should be retrieved by UUID
log.Println("feature requested by int while external fid column is defined")
return nil, nil
}
fidColumn = g.fidColumn
case uuid.UUID:
if g.externalFidColumn == "" {
// Features should be retrieved by int64
log.Println("feature requested by UUID while external fid column is not defined")
return nil, nil
}
fidColumn = g.externalFidColumn
}

query := fmt.Sprintf("select * from %s f where f.%s = :fid limit 1", table.TableName, fidColumn)
rows, err := g.backend.getDB().NamedQueryContext(queryCtx, query, map[string]any{"fid": featureID})
if err != nil {
return nil, fmt.Errorf("query '%s' failed: %w", query, err)
}
defer rows.Close()

features, _, err := domain.MapRowsToFeatures(rows, g.fidColumn, table.GeometryColumnName, readGpkgGeometry)
features, _, err := domain.MapRowsToFeatures(rows, g.fidColumn, g.externalFidColumn, table.GeometryColumnName, readGpkgGeometry)
if err != nil {
return nil, err
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ func TestGeoPackage_GetFeature(t *testing.T) {
featureID: 3837,
},
want: &domain.Feature{
ID: 0,
ID: "0",
Links: nil,
Feature: geojson.Feature{
Properties: map[string]any{
Expand Down
2 changes: 1 addition & 1 deletion internal/ogc/features/datasources/postgis/postgis.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func (pg PostGIS) GetFeatures(_ context.Context, _ string, _ datasources.Feature
return nil, domain.Cursors{}, nil
}

func (pg PostGIS) GetFeature(_ context.Context, _ string, _ int64) (*domain.Feature, error) {
func (pg PostGIS) GetFeature(_ context.Context, _ string, _ any) (*domain.Feature, error) {
log.Println("PostGIS support is not implemented yet, this just serves to demonstrate that we can support multiple types of datasources")
return nil, nil
}
Expand Down
6 changes: 3 additions & 3 deletions internal/ogc/features/domain/cursor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func TestNewCursor(t *testing.T) {
{
name: "test first page",
args: args{
features: []*Feature{{ID: 1}, {ID: 2}, {ID: 3}, {ID: 4}},
features: []*Feature{{ID: "1"}, {ID: "2"}, {ID: "3"}, {ID: "4"}},
id: PrevNextFID{
Prev: 0,
Next: 4,
Expand All @@ -35,7 +35,7 @@ func TestNewCursor(t *testing.T) {
{
name: "test last page",
args: args{
features: []*Feature{{ID: 5}, {ID: 6}, {ID: 7}, {ID: 8}},
features: []*Feature{{ID: "5"}, {ID: "6"}, {ID: "7"}, {ID: "8"}},
id: PrevNextFID{
Prev: 4,
Next: 0,
Expand All @@ -51,7 +51,7 @@ func TestNewCursor(t *testing.T) {
{
name: "test middle page",
args: args{
features: []*Feature{{ID: 3}, {ID: 4}, {ID: 5}, {ID: 6}},
features: []*Feature{{ID: "3"}, {ID: "4"}, {ID: "5"}, {ID: "6"}},
id: PrevNextFID{
Prev: 2,
Next: 7,
Expand Down
4 changes: 2 additions & 2 deletions internal/ogc/features/domain/geojson.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@ type FeatureCollection struct {
// Feature is a GeoJSON Feature with extras such as links
type Feature struct {
geojson.Feature
Links []Link `json:"links,omitempty"`

// 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 int64 `json:"id"`
ID string `json:"id"`
Links []Link `json:"links,omitempty"`
}

// Link according to RFC 8288, https://datatracker.ietf.org/doc/html/rfc8288
Expand Down
6 changes: 3 additions & 3 deletions internal/ogc/features/domain/jsonfg.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ type JSONFGFeatureCollection struct {
}

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
Expand All @@ -36,7 +39,4 @@ type JSONFGFeature struct {
CoordRefSys string `json:"coordRefSys,omitempty"`
Links []Link `json:"links,omitempty"`
ConformsTo []string `json:"conformsTo,omitempty"`
// We expect feature ids to be auto-incrementing integers (which is the default in geopackages)
// since we use it for cursor-based pagination.
ID int64 `json:"id"`
}
16 changes: 11 additions & 5 deletions internal/ogc/features/domain/mapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,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, geomColumn string,
func MapRowsToFeatures(rows *sqlx.Rows, fidColumn string, externalFidColumn string, geomColumn string,
geomMapper func([]byte) (geom.Geometry, error)) ([]*Feature, *PrevNextFID, error) {

result := make([]*Feature, 0)
Expand All @@ -58,7 +58,7 @@ func MapRowsToFeatures(rows *sqlx.Rows, fidColumn string, geomColumn string,
}

feature := &Feature{Feature: geojson.Feature{Properties: make(map[string]any)}}
np, err := mapColumnsToFeature(firstRow, feature, columns, values, fidColumn, geomColumn, geomMapper)
np, err := mapColumnsToFeature(firstRow, feature, columns, values, fidColumn, externalFidColumn, geomColumn, geomMapper)
if err != nil {
return result, nil, err
} else if firstRow {
Expand All @@ -71,16 +71,22 @@ func MapRowsToFeatures(rows *sqlx.Rows, fidColumn string, geomColumn string,
}

//nolint:cyclop,funlen
func mapColumnsToFeature(firstRow bool, feature *Feature, columns []string, values []any,
fidColumn string, geomColumn string, geomMapper func([]byte) (geom.Geometry, error)) (*PrevNextFID, error) {
func mapColumnsToFeature(firstRow bool, feature *Feature, columns []string, values []any, fidColumn string,
externalFidColum string, geomColumn string, geomMapper func([]byte) (geom.Geometry, error)) (*PrevNextFID, error) {

prevNextID := PrevNextFID{}
for i, columnName := range columns {
columnValue := values[i]

switch columnName {
case fidColumn:
feature.ID = columnValue.(int64)
// Assumes that `fid` column is first column in the table
feature.ID = fmt.Sprint(columnValue)

case externalFidColum:
// If externalFidColumn is configured, overwrite feature ID and drop column from feature
feature.ID = fmt.Sprint(columnValue)
delete(feature.Properties, columnName)

case geomColumn:
if columnValue == nil {
Expand Down
7 changes: 3 additions & 4 deletions internal/ogc/features/html.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package features

import (
"net/http"
"strconv"
"time"

"github.com/PDOK/gokoala/config"
Expand Down Expand Up @@ -64,7 +63,7 @@ type featurePage struct {
domain.Feature

CollectionID string
FeatureID int64
FeatureID string
Metadata *config.GeoSpatialCollectionMetadata
MapSheetProperties *config.MapSheetDownloadProperties
}
Expand Down Expand Up @@ -125,8 +124,8 @@ func (hf *htmlFeatures) feature(w http.ResponseWriter, r *http.Request, collecti
Path: collectionsCrumb + collectionID + "/items",
},
{
Name: strconv.FormatInt(feat.ID, 10),
Path: collectionsCrumb + collectionID + "/items/" + strconv.FormatInt(feat.ID, 10),
Name: feat.ID,
Path: collectionsCrumb + collectionID + "/items/" + feat.ID,
},
}...)

Expand Down
2 changes: 1 addition & 1 deletion internal/ogc/features/json.go
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ func (jf *jsonFeatures) createFeatureCollectionLinks(currentFormat string, colle
}

func (jf *jsonFeatures) createFeatureLinks(currentFormat string, url featureURL,
collectionID string, featureID int64) []domain.Link {
collectionID string, featureID string) []domain.Link {

links := make([]domain.Link, 0)
switch currentFormat {
Expand Down
18 changes: 12 additions & 6 deletions internal/ogc/features/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"time"

"github.com/PDOK/gokoala/config"
"github.com/google/uuid"

"github.com/PDOK/gokoala/internal/engine"
"github.com/PDOK/gokoala/internal/ogc/common/geospatial"
Expand Down Expand Up @@ -180,10 +181,15 @@ func (f *Features) Feature() http.HandlerFunc {
handleCollectionNotFound(w, collectionID)
return
}
featureID, err := strconv.Atoi(chi.URLParam(r, "featureId"))
var featureID any
featureID, err := uuid.Parse(chi.URLParam(r, "featureId"))
if err != nil {
engine.RenderProblem(engine.ProblemBadRequest, w, "feature ID must be a number")
return
// fallback to numerical feature id
featureID, err = strconv.ParseInt(chi.URLParam(r, "featureId"), 10, 0)
if err != nil {
engine.RenderProblem(engine.ProblemBadRequest, w, "feature ID must be a UUID or number")
return
}
}
url := featureURL{*f.engine.Config.BaseURL.URL, r.URL.Query()}
outputSRID, contentCrs, err := url.parse()
Expand All @@ -195,16 +201,16 @@ func (f *Features) Feature() http.HandlerFunc {
mapSheetProperties := cfg.OgcAPI.Features.MapSheetPropertiesForCollection(collectionID)

datasource := f.datasources[DatasourceKey{srid: outputSRID.GetOrDefault(), collectionID: collectionID}]
feat, err := datasource.GetFeature(r.Context(), collectionID, int64(featureID))
feat, err := datasource.GetFeature(r.Context(), collectionID, featureID)
if err != nil {
// log error, but sent generic message to client to prevent possible information leakage from datasource
msg := fmt.Sprintf("failed to retrieve feature %d in collection %s", featureID, collectionID)
msg := fmt.Sprintf("failed to retrieve feature %v in collection %s", featureID, collectionID)
log.Printf("%s, error: %v\n", msg, err)
engine.RenderProblem(engine.ProblemServerError, w, msg)
return
}
if feat == nil {
msg := fmt.Sprintf("the requested feature with id: %d does not exist in collection '%s'", featureID, collectionID)
msg := fmt.Sprintf("the requested feature with id: %v does not exist in collection '%s'", featureID, collectionID)
log.Println(msg)
engine.RenderProblem(engine.ProblemNotFound, w, msg)
return
Expand Down
12 changes: 6 additions & 6 deletions internal/ogc/features/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -650,11 +650,11 @@ func TestFeatures_Feature(t *testing.T) {
configFile: "internal/ogc/features/testdata/config_features_multiple_gpkgs.yaml",
url: "http://localhost:8080/collections/:collectionId/items/:featureId?crs=http://www.opengis.net/def/crs/OGC/1.3/CRS84",
collectionID: "dutch-addresses",
featureID: "10",
featureID: "b29c12b1-21a9-5e63-83b4-0ff9122ef80f",
format: "json",
},
want: want{
body: "internal/ogc/features/testdata/expected_multiple_gpkgs_feature_10_wgs84.json",
body: "internal/ogc/features/testdata/expected_multiple_gpkgs_feature_b29c12b1_wgs84.json",
statusCode: http.StatusOK,
},
},
Expand All @@ -664,11 +664,11 @@ func TestFeatures_Feature(t *testing.T) {
configFile: "internal/ogc/features/testdata/config_features_validation_disabled.yaml",
url: "http://localhost:8080/collections/:collectionId/items/:featureId?crs=http://www.opengis.net/def/crs/OGC/1.3/CRS84",
collectionID: "dutch-addresses",
featureID: "10",
featureID: "b29c12b1-21a9-5e63-83b4-0ff9122ef80f",
format: "json",
},
want: want{
body: "internal/ogc/features/testdata/expected_multiple_gpkgs_feature_10_wgs84.json",
body: "internal/ogc/features/testdata/expected_multiple_gpkgs_feature_b29c12b1_wgs84.json",
statusCode: http.StatusOK,
},
},
Expand All @@ -678,11 +678,11 @@ func TestFeatures_Feature(t *testing.T) {
configFile: "internal/ogc/features/testdata/config_features_multiple_gpkgs.yaml",
url: "http://localhost:8080/collections/:collectionId/items/:featureId?crs=http%3A%2F%2Fwww.opengis.net%2Fdef%2Fcrs%2FEPSG%2F0%2F28992",
collectionID: "dutch-addresses",
featureID: "10",
featureID: "b29c12b1-21a9-5e63-83b4-0ff9122ef80f",
format: "json",
},
want: want{
body: "internal/ogc/features/testdata/expected_multiple_gpkgs_feature_10_rd.json",
body: "internal/ogc/features/testdata/expected_multiple_gpkgs_feature_b29c12b1_rd.json",
statusCode: http.StatusOK,
},
},
Expand Down
Loading

0 comments on commit 725610e

Please sign in to comment.