Merge pull request #268 from PDOK/PDOK-17398/null-geometry-3

feat: support null and empty geometries

internal/engine/templates/openapi/features.go.json

@@ -550,7 +550,12 @@
             ]
           },
           "geometry": {
-            "$ref": "#/components/schemas/geometryGeoJSON"
+            "nullable": true,
+            "allOf": [
+              {
+                "$ref": "#/components/schemas/geometryGeoJSON"
+              }
+            ]
           },
           "properties": {
             "type": "object",

internal/ogc/features/datasources/geopackage/geopackage.go

@@ -18,6 +18,7 @@ import (
 	"github.com/PDOK/gokoala/internal/ogc/features/datasources"
 	"github.com/PDOK/gokoala/internal/ogc/features/domain"
 	"github.com/go-spatial/geom"
+	"github.com/go-spatial/geom/cmp"
 	"github.com/go-spatial/geom/encoding/gpkg"
 	"github.com/go-spatial/geom/encoding/wkt"
 	"github.com/google/uuid"
@@ -447,11 +448,14 @@ func (g *GeoPackage) selectSpecificColumnsInOrder(propConfig *config.FeatureProp
 }
 
 func mapGpkgGeometry(rawGeom []byte) (geom.Geometry, error) {
-	geometry, err := gpkg.DecodeGeometry(rawGeom)
+	geomWithMetadata, err := gpkg.DecodeGeometry(rawGeom)
 	if err != nil {
 		return nil, err
 	}
-	return geometry.Geometry, nil
+	if geomWithMetadata == nil || cmp.IsEmptyGeo(geomWithMetadata.Geometry) {
+		return nil, nil
+	}
+	return geomWithMetadata.Geometry, nil
 }
 
 func propertyFiltersToSQL(pf map[string]string) (sql string, namedParams map[string]any) {

internal/ogc/features/datasources/geopackage/geopackage_test.go

@@ -22,7 +22,7 @@ func init() {
 	pwd = path.Dir(filename)
 }
 
-func newAddressesGeoPackage() geoPackageBackend {
+func newTestGeoPackage(file string) geoPackageBackend {
 	loadDriver()
 	return newLocalGeoPackage(&config.GeoPackageLocal{
 		GeoPackageCommon: config.GeoPackageCommon{
@@ -31,20 +31,7 @@ func newAddressesGeoPackage() geoPackageBackend {
 			MaxBBoxSizeToUseWithRTree: 30000,
 			InMemoryCacheSize:         -2000,
 		},
-		File: pwd + "/testdata/bag.gpkg",
-	})
-}
-
-func newTemporalAddressesGeoPackage() geoPackageBackend {
-	loadDriver()
-	return newLocalGeoPackage(&config.GeoPackageLocal{
-		GeoPackageCommon: config.GeoPackageCommon{
-			Fid:                       "feature_id",
-			QueryTimeout:              config.Duration{Duration: 15 * time.Second},
-			MaxBBoxSizeToUseWithRTree: 30000,
-			InMemoryCacheSize:         -2000,
-		},
-		File: pwd + "/testdata/bag-temporal.gpkg",
+		File: pwd + file,
 	})
 }
 
@@ -107,11 +94,12 @@ func TestGeoPackage_GetFeatures(t *testing.T) {
 		wantFC     *domain.FeatureCollection
 		wantCursor domain.Cursors
 		wantErr    bool
+		wantGeom   bool
 	}{
 		{
 			name: "get first page of features",
 			fields: fields{
-				backend:          newAddressesGeoPackage(),
+				backend:          newTestGeoPackage("/testdata/bag.gpkg"),
 				fidColumn:        "feature_id",
 				featureTableByID: map[string]*featureTable{"ligplaatsen": {TableName: "ligplaatsen", GeometryColumnName: "geom"}},
 				queryTimeout:     60 * time.Second,
@@ -145,12 +133,13 @@ func TestGeoPackage_GetFeatures(t *testing.T) {
 				Prev: "|",
 				Next: "Dv4|", // 3838
 			},
-			wantErr: false,
+			wantGeom: true,
+			wantErr:  false,
 		},
 		{
 			name: "get second page of features",
 			fields: fields{
-				backend:          newAddressesGeoPackage(),
+				backend:          newTestGeoPackage("/testdata/bag.gpkg"),
 				fidColumn:        "feature_id",
 				featureTableByID: map[string]*featureTable{"ligplaatsen": {TableName: "ligplaatsen", GeometryColumnName: "geom"}},
 				queryTimeout:     5 * time.Second,
@@ -194,12 +183,13 @@ func TestGeoPackage_GetFeatures(t *testing.T) {
 				Prev: "|",
 				Next: "DwE|",
 			},
-			wantErr: false,
+			wantGeom: true,
+			wantErr:  false,
 		},
 		{
 			name: "get first page of features with reference date",
 			fields: fields{
-				backend:          newTemporalAddressesGeoPackage(),
+				backend:          newTestGeoPackage("/testdata/bag-temporal.gpkg"),
 				fidColumn:        "feature_id",
 				featureTableByID: map[string]*featureTable{"ligplaatsen": {TableName: "ligplaatsen", GeometryColumnName: "geom"}},
 				queryTimeout:     60 * time.Second,
@@ -238,12 +228,13 @@ func TestGeoPackage_GetFeatures(t *testing.T) {
 				Prev: "|",
 				Next: "Dv4|", // 3838
 			},
-			wantErr: false,
+			wantGeom: true,
+			wantErr:  false,
 		},
 		{
 			name: "fail on non existing collection",
 			fields: fields{
-				backend:          newAddressesGeoPackage(),
+				backend:          newTestGeoPackage("/testdata/bag.gpkg"),
 				fidColumn:        "feature_id",
 				featureTableByID: map[string]*featureTable{"ligplaatsen": {TableName: "ligplaatsen", GeometryColumnName: "geom"}},
 				queryTimeout:     5 * time.Second,
@@ -260,6 +251,74 @@ func TestGeoPackage_GetFeatures(t *testing.T) {
 			wantCursor: domain.Cursors{},
 			wantErr:    true, // should fail
 		},
+		{
+			name: "get features with empty geometry",
+			fields: fields{
+				backend:          newTestGeoPackage("/testdata/null-empty-geoms.gpkg"),
+				fidColumn:        "feature_id",
+				featureTableByID: map[string]*featureTable{"ligplaatsen": {TableName: "ligplaatsen", GeometryColumnName: "geom"}},
+				queryTimeout:     60 * time.Second,
+			},
+			args: args{
+				ctx:        context.Background(),
+				collection: "ligplaatsen",
+				queryParams: datasources.FeaturesCriteria{
+					Cursor: domain.DecodedCursor{FID: 0, FiltersChecksum: []byte{}},
+					Limit:  1,
+				},
+			},
+			wantFC: &domain.FeatureCollection{
+				NumberReturned: 1,
+				Features: []*domain.Feature{
+					{
+						Properties: domain.NewFeaturePropertiesWithData(false, map[string]any{
+							"straatnaam": "Van Diemenkade",
+							"nummer_id":  "0363200000454013",
+						}),
+					},
+				},
+			},
+			wantCursor: domain.Cursors{
+				Prev: "|",
+				Next: "GSQ|",
+			},
+			wantGeom: false, // should be null
+			wantErr:  false,
+		},
+		{
+			name: "get features with null geometry",
+			fields: fields{
+				backend:          newTestGeoPackage("/testdata/null-empty-geoms.gpkg"),
+				fidColumn:        "feature_id",
+				featureTableByID: map[string]*featureTable{"ligplaatsen": {TableName: "ligplaatsen", GeometryColumnName: "geom"}},
+				queryTimeout:     60 * time.Second,
+			},
+			args: args{
+				ctx:        context.Background(),
+				collection: "ligplaatsen",
+				queryParams: datasources.FeaturesCriteria{
+					Cursor: domain.DecodedCursor{FID: 6436, FiltersChecksum: []byte{}},
+					Limit:  1,
+				},
+			},
+			wantFC: &domain.FeatureCollection{
+				NumberReturned: 1,
+				Features: []*domain.Feature{
+					{
+						Properties: domain.NewFeaturePropertiesWithData(false, map[string]any{
+							"straatnaam": "Bokkinghangen",
+							"nummer_id":  "0363200012163629",
+						}),
+					},
+				},
+			},
+			wantCursor: domain.Cursors{
+				Prev: "DdY|",
+				Next: "|",
+			},
+			wantGeom: false, // should be null
+			wantErr:  false,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -284,6 +343,9 @@ func TestGeoPackage_GetFeatures(t *testing.T) {
 			for i, wantedFeature := range tt.wantFC.Features {
 				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"))
+				if !tt.wantGeom {
+					assert.Nil(t, fc.Features[i].Geometry)
+				}
 			}
 			assert.Equal(t, tt.wantCursor.Prev, cursor.Prev)
 			assert.Equal(t, tt.wantCursor.Next, cursor.Next)
@@ -313,7 +375,7 @@ func TestGeoPackage_GetFeature(t *testing.T) {
 		{
 			name: "get feature",
 			fields: fields{
-				backend:          newAddressesGeoPackage(),
+				backend:          newTestGeoPackage("/testdata/bag.gpkg"),
 				fidColumn:        "feature_id",
 				featureTableByID: map[string]*featureTable{"ligplaatsen": {TableName: "ligplaatsen", GeometryColumnName: "geom"}},
 				queryTimeout:     5 * time.Second,
@@ -336,7 +398,7 @@ func TestGeoPackage_GetFeature(t *testing.T) {
 		{
 			name: "get non existing feature",
 			fields: fields{
-				backend:          newAddressesGeoPackage(),
+				backend:          newTestGeoPackage("/testdata/bag.gpkg"),
 				fidColumn:        "feature_id",
 				featureTableByID: map[string]*featureTable{"ligplaatsen": {TableName: "ligplaatsen", GeometryColumnName: "geom"}},
 				queryTimeout:     5 * time.Second,
@@ -352,7 +414,7 @@ func TestGeoPackage_GetFeature(t *testing.T) {
 		{
 			name: "fail on non existing collection",
 			fields: fields{
-				backend:          newAddressesGeoPackage(),
+				backend:          newTestGeoPackage("/testdata/bag.gpkg"),
 				fidColumn:        "feature_id",
 				featureTableByID: map[string]*featureTable{"ligplaatsen": {TableName: "ligplaatsen", GeometryColumnName: "geom"}},
 				queryTimeout:     5 * time.Second,
@@ -394,7 +456,7 @@ func TestGeoPackage_GetFeature(t *testing.T) {
 func TestGeoPackage_Warmup(t *testing.T) {
 	t.Run("warmup", func(t *testing.T) {
 		g := &GeoPackage{
-			backend:                    newAddressesGeoPackage(),
+			backend:                    newTestGeoPackage("/testdata/bag.gpkg"),
 			fidColumn:                  "feature_id",
 			featureTableByCollectionID: map[string]*featureTable{"ligplaatsen": {TableName: "ligplaatsen", GeometryColumnName: "geom"}},
 			queryTimeout:               5 * time.Second,

internal/ogc/features/domain/cursor_test.go

@@ -151,6 +151,14 @@ func TestEncodedCursor_Decode(t *testing.T) {
 				FiltersChecksum: []byte("foobar"),
 			},
 		},
+		{
+			name: "should decode cursor without checksum",
+			c:    "GSQ|",
+			want: DecodedCursor{
+				FID:             6436,
+				FiltersChecksum: nil,
+			},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

internal/ogc/features/domain/mapper.go

@@ -7,7 +7,6 @@ import (
 	"time"
 
 	"github.com/PDOK/gokoala/config"
-
 	"github.com/go-spatial/geom"
 	"github.com/go-spatial/geom/encoding/geojson"
 	"github.com/jmoiron/sqlx"
@@ -109,7 +108,9 @@ func mapColumnsToFeature(ctx context.Context, firstRow bool, feature *Feature, c
 			if err != nil {
 				return nil, fmt.Errorf("failed to map/decode geometry from datasource, error: %w", err)
 			}
-			feature.Geometry = geojson.Geometry{Geometry: mappedGeom}
+			if mappedGeom != nil {
+				feature.Geometry = geojson.Geometry{Geometry: mappedGeom}
+			}
 
 		case "minx", "miny", "maxx", "maxy", "min_zoom", "max_zoom":
 			// Skip these columns used for bounding box and zoom filtering

internal/ogc/features/main_test.go

@@ -1009,6 +1009,62 @@ func TestFeatures_Feature(t *testing.T) {
 				statusCode: http.StatusOK,
 			},
 		},
+		{
+			name: "Request GeoJSON for feature with null geom",
+			fields: fields{
+				configFile:   "internal/ogc/features/testdata/config_features_geom_null_empty.yaml",
+				url:          "http://localhost:8080/collections/:collectionId/items/:featureId",
+				collectionID: "foo",
+				featureID:    "6436",
+				format:       "json",
+			},
+			want: want{
+				body:       "internal/ogc/features/testdata/expected_feature_geom_null.json",
+				statusCode: http.StatusOK,
+			},
+		},
+		{
+			name: "Request JSON-FG for feature with null geom",
+			fields: fields{
+				configFile:   "internal/ogc/features/testdata/config_features_geom_null_empty.yaml",
+				url:          "http://localhost:8080/collections/:collectionId/items/:featureId?f=jsonfg",
+				collectionID: "foo",
+				featureID:    "6436",
+				format:       "json",
+			},
+			want: want{
+				body:       "internal/ogc/features/testdata/expected_feature_geom_null_jsonfg.json",
+				statusCode: http.StatusOK,
+			},
+		},
+		{
+			name: "Request GeoJSON for feature with empty point",
+			fields: fields{
+				configFile:   "internal/ogc/features/testdata/config_features_geom_null_empty.yaml",
+				url:          "http://localhost:8080/collections/:collectionId/items/:featureId",
+				collectionID: "foo",
+				featureID:    "3542",
+				format:       "json",
+			},
+			want: want{
+				body:       "internal/ogc/features/testdata/expected_feature_geom_empty_point.json",
+				statusCode: http.StatusOK,
+			},
+		},
+		{
+			name: "Request JSON-FG for feature with empty point",
+			fields: fields{
+				configFile:   "internal/ogc/features/testdata/config_features_geom_null_empty.yaml",
+				url:          "http://localhost:8080/collections/:collectionId/items/:featureId?f=jsonfg",
+				collectionID: "foo",
+				featureID:    "3542",
+				format:       "json",
+			},
+			want: want{
+				body:       "internal/ogc/features/testdata/expected_feature_geom_empty_point_jsonfg.json",
+				statusCode: http.StatusOK,
+			},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -1031,9 +1087,7 @@ func TestFeatures_Feature(t *testing.T) {
 			assert.Equal(t, tt.want.statusCode, rr.Code)
 			if tt.want.body != "" {
 				expectedBody, err := os.ReadFile(tt.want.body)
-				if err != nil {
-					log.Fatal(err)
-				}
+				assert.NoError(t, err)
 
 				printActual(rr)
 				switch {

internal/ogc/features/testdata/config_features_geom_null_empty.yaml

@@ -0,0 +1,35 @@
+---
+version: 1.0.2
+title: OGC API Features
+abstract: Contains a slimmed-down/example version of the BAG-dataset
+baseUrl: http://localhost:8080
+serviceIdentifier: Feats
+license:
+  name: CC0
+  url: https://www.tldrlegal.com/license/creative-commons-cc0-1-0-universal
+ogcApi:
+  features:
+    datasources:
+      defaultWGS84:
+        geopackage:
+          local:
+            file: ./internal/ogc/features/datasources/geopackage/testdata/null-empty-geoms.gpkg
+            fid: feature_id
+            queryTimeout: 15m # pretty high to allow debugging
+    collections:
+      - id: foo
+        tableName: ligplaatsen
+        filters:
+          properties:
+            - name: straatnaam
+            - name: postcode
+        metadata:
+          title: Foooo
+          description: Foooo
+      - id: bar
+        tableName: ligplaatsen
+        metadata:
+          title: Barrr
+          description: Barrr
+          tableName: ligplaatsen
+      - id: baz