From 598c764da54dada2490771b7ca7d18a01667664e Mon Sep 17 00:00:00 2001 From: Alexander Rolek Date: Mon, 8 Apr 2019 23:40:47 -0700 Subject: [PATCH] handle empty geometry collection (#592) * moved empty geometry collection check to the atlas package so all providers can benefit. * Added test case to atlas for empty geometry collection Reworked the TestEncode function to match the new testing convention. * added emptycollection test provider closes #429 --- atlas/map.go | 6 + atlas/map_test.go | 278 ++++++++++++---------- provider/postgis/postgis.go | 4 +- provider/postgis/postgis_test.go | 11 + provider/test/emptycollection/layer.go | 21 ++ provider/test/emptycollection/provider.go | 54 +++++ 6 files changed, 252 insertions(+), 122 deletions(-) create mode 100644 provider/test/emptycollection/layer.go create mode 100644 provider/test/emptycollection/provider.go diff --git a/atlas/map.go b/atlas/map.go index 9675fa491..cf0f8aa5f 100644 --- a/atlas/map.go +++ b/atlas/map.go @@ -154,6 +154,12 @@ func (m Map) Encode(ctx context.Context, tile *slippy.Tile) ([]byte, error) { // fetch layer from data provider err := l.Provider.TileFeatures(ctx, l.ProviderLayerName, tile, func(f *provider.Feature) error { + // skip row if geometry collection empty. + g, ok := f.Geometry.(geom.Collection) + if ok && len(g.Geometries()) == 0 { + return nil + } + // TODO: remove this geom conversion step once the mvt package has adopted the new geom package geo, err := convert.ToTegola(f.Geometry) if err != nil { diff --git a/atlas/map_test.go b/atlas/map_test.go index fef3ca8d4..cf0a6f03f 100644 --- a/atlas/map_test.go +++ b/atlas/map_test.go @@ -14,8 +14,9 @@ import ( "github.com/go-spatial/tegola" "github.com/go-spatial/tegola/atlas" "github.com/go-spatial/tegola/internal/p" - "github.com/go-spatial/tegola/mvt/vector_tile" + vectorTile "github.com/go-spatial/tegola/mvt/vector_tile" "github.com/go-spatial/tegola/provider/test" + "github.com/go-spatial/tegola/provider/test/emptycollection" ) func TestMapFilterLayersByZoom(t *testing.T) { @@ -142,12 +143,138 @@ func TestEncode(t *testing.T) { // linestring := vectorTile.Tile_LINESTRING polygon := vectorTile.Tile_POLYGON - testcases := []struct { + type tcase struct { grid atlas.Map tile *slippy.Tile expected vectorTile.Tile - }{ - { + } + + fn := func(tc tcase) func(t *testing.T) { + return func(t *testing.T) { + out, err := tc.grid.Encode(context.Background(), tc.tile) + if err != nil { + t.Errorf("err: %v", err) + return + } + + // decompress our output + var buf bytes.Buffer + r, err := gzip.NewReader(bytes.NewReader(out)) + if err != nil { + t.Errorf("err: %v", err) + return + } + + _, err = io.Copy(&buf, r) + if err != nil { + t.Errorf("err: %v", err) + return + } + + var tile vectorTile.Tile + + if err = proto.Unmarshal(buf.Bytes(), &tile); err != nil { + t.Errorf("error unmarshalling output: %v", err) + return + } + + // check the layer lengths match + if len(tile.Layers) != len(tc.expected.Layers) { + t.Errorf("expected (%d) layers, got (%d)", len(tc.expected.Layers), len(tile.Layers)) + return + } + + for j, tileLayer := range tile.Layers { + expectedLayer := tc.expected.Layers[j] + + if *tileLayer.Version != *expectedLayer.Version { + t.Errorf("expected %v got %v", *tileLayer.Version, *expectedLayer.Version) + return + } + + if *tileLayer.Name != *expectedLayer.Name { + t.Errorf("expected %v got %v", *tileLayer.Name, *expectedLayer.Name) + return + } + + // features check + for k, tileLayerFeature := range tileLayer.Features { + expectedTileLayerFeature := expectedLayer.Features[k] + + if *tileLayerFeature.Id != *expectedTileLayerFeature.Id { + t.Errorf("expected %v got %v", *tileLayerFeature.Id, *expectedTileLayerFeature.Id) + return + } + + // the vector tile layer tags output is not always consistent since it's generated from a map. + // because of that we're going to check everything but the tags values + + // if !reflect.DeepEqual(tileLayerFeature.Tags, expectedTileLayerFeature.Tags) { + // t.Errorf("expected %v got %v", tileLayerFeature.Tags, expectedTileLayerFeature.Tags) + // return + // } + + if *tileLayerFeature.Type != *expectedTileLayerFeature.Type { + t.Errorf("expected %v got %v", *tileLayerFeature.Type, *expectedTileLayerFeature.Type) + return + } + + if !reflect.DeepEqual(tileLayerFeature.Geometry, expectedTileLayerFeature.Geometry) { + t.Errorf("expected %v got %v", tileLayerFeature.Geometry, expectedTileLayerFeature.Geometry) + return + } + } + + if len(tileLayer.Keys) != len(expectedLayer.Keys) { + t.Errorf("layer keys length, expected %v got %v", len(expectedLayer.Keys), len(tileLayer.Keys)) + return + } + { + var keysmaps = make(map[string]struct{}) + for _, k := range expectedLayer.Keys { + keysmaps[k] = struct{}{} + } + var ferr bool + for _, k := range tileLayer.Keys { + if _, ok := keysmaps[k]; !ok { + t.Errorf("did not find key, expected %v got nil", k) + ferr = true + } + } + if ferr { + return + } + } + + if *tileLayer.Extent != *expectedLayer.Extent { + t.Errorf("expected %v got %v", *tileLayer.Extent, *expectedLayer.Extent) + return + } + + if len(expectedLayer.Keys) != len(tileLayer.Keys) { + t.Errorf("key len expected %v got %v", len(expectedLayer.Keys), len(tileLayer.Keys)) + return + + } + + var gotmap = make(map[string]interface{}) + var expmap = make(map[string]interface{}) + for i, k := range tileLayer.Keys { + gotmap[k] = tileLayer.Values[i] + } + for i, k := range expectedLayer.Keys { + expmap[k] = expectedLayer.Values[i] + } + + if !reflect.DeepEqual(expmap, gotmap) { + t.Errorf("constructed map expected %v got %v", expmap, gotmap) + } + } + } + } + + tests := map[string]tcase{ + "test_provider": { grid: atlas.Map{ Layers: []atlas.Layer{ { @@ -214,123 +341,34 @@ func TestEncode(t *testing.T) { }, }, }, + "empty_collection": { + grid: atlas.Map{ + Layers: []atlas.Layer{ + { + Name: "empty_geom_collection", + MinZoom: 0, + MaxZoom: 2, + Provider: &emptycollection.TileProvider{}, + }, + }, + }, + tile: slippy.NewTile(2, 3, 4, 64, tegola.WebMercator), + expected: vectorTile.Tile{ + Layers: []*vectorTile.Tile_Layer{ + { + Version: p.Uint32(2), + Name: p.String("empty_geom_collection"), + Features: []*vectorTile.Tile_Feature{}, + Keys: []string{}, + Values: []*vectorTile.Tile_Value{}, + Extent: p.Uint32(vectorTile.Default_Tile_Layer_Extent), + }, + }, + }, + }, } - for i, tc := range testcases { - out, err := tc.grid.Encode(context.Background(), tc.tile) - if err != nil { - t.Errorf("[%v] err: %v", i, err) - continue - } - - // decompress our output - var buf bytes.Buffer - r, err := gzip.NewReader(bytes.NewReader(out)) - if err != nil { - t.Errorf("[%v] err: %v", i, err) - continue - } - - _, err = io.Copy(&buf, r) - if err != nil { - t.Errorf("[%v] err: %v", i, err) - continue - } - - var tile vectorTile.Tile - - if err = proto.Unmarshal(buf.Bytes(), &tile); err != nil { - t.Errorf("[%v] error unmarshalling output: %v", i, err) - continue - } - - for j, tileLayer := range tile.Layers { - expectedLayer := tc.expected.Layers[j] - - if *tileLayer.Version != *expectedLayer.Version { - t.Errorf("[%v] expected %v got %v", i, *tileLayer.Version, *expectedLayer.Version) - continue - } - - if *tileLayer.Name != *expectedLayer.Name { - t.Errorf("[%v] expected %v got %v", i, *tileLayer.Name, *expectedLayer.Name) - continue - } - - // features check - for k, tileLayerFeature := range tileLayer.Features { - expectedTileLayerFeature := expectedLayer.Features[k] - - if *tileLayerFeature.Id != *expectedTileLayerFeature.Id { - t.Errorf("[%v] expected %v got %v", i, *tileLayerFeature.Id, *expectedTileLayerFeature.Id) - continue - } - - /* - // the vector tile layer tags output is not always consistent since it's generated from a map. - // because of that we're going to check everything but the tags values - - if !reflect.DeepEqual(tileLayerFeature.Tags, expectedTileLayerFeature.Tags) { - t.Errorf("[%v] expected %v got %v", i, tileLayerFeature.Tags, expectedTileLayerFeature.Tags) - continue - } - */ - - if *tileLayerFeature.Type != *expectedTileLayerFeature.Type { - t.Errorf("[%v] expected %v got %v", i, *tileLayerFeature.Type, *expectedTileLayerFeature.Type) - continue - } - - if !reflect.DeepEqual(tileLayerFeature.Geometry, expectedTileLayerFeature.Geometry) { - t.Errorf("[%v] expected %v got %v", i, tileLayerFeature.Geometry, expectedTileLayerFeature.Geometry) - continue - } - } - - if len(tileLayer.Keys) != len(expectedLayer.Keys) { - t.Errorf("[%v] layer keys length, expected %v got %v", i, len(expectedLayer.Keys), len(tileLayer.Keys)) - continue - } - { - var keysmaps = make(map[string]struct{}) - for _, k := range expectedLayer.Keys { - keysmaps[k] = struct{}{} - } - var ferr bool - for _, k := range tileLayer.Keys { - if _, ok := keysmaps[k]; !ok { - t.Errorf("[%v] did not find key, expected %v got nil", i, k) - ferr = true - } - } - if ferr { - continue - } - } - - if *tileLayer.Extent != *expectedLayer.Extent { - t.Errorf("[%v] expected %v got %v", i, *tileLayer.Extent, *expectedLayer.Extent) - continue - } - - if len(expectedLayer.Keys) != len(tileLayer.Keys) { - t.Errorf("[%v] key len expected %v got %v", i, len(expectedLayer.Keys), len(tileLayer.Keys)) - continue - - } - - var gotmap = make(map[string]interface{}) - var expmap = make(map[string]interface{}) - for i, k := range tileLayer.Keys { - gotmap[k] = tileLayer.Values[i] - } - for i, k := range expectedLayer.Keys { - expmap[k] = expectedLayer.Values[i] - } - if !reflect.DeepEqual(expmap, gotmap) { - t.Errorf("[%v] constructed map expected %v got %v", i, expmap, gotmap) - } - - } + for name, tc := range tests { + t.Run(name, fn(tc)) } } diff --git a/provider/postgis/postgis.go b/provider/postgis/postgis.go index ee3590fc1..792359f6b 100644 --- a/provider/postgis/postgis.go +++ b/provider/postgis/postgis.go @@ -582,7 +582,7 @@ func (p Provider) TileFeatures(ctx context.Context, layer string, tile provider. } // decode our WKB - geom, err := wkb.DecodeBytes(geobytes) + geometry, err := wkb.DecodeBytes(geobytes) if err != nil { switch err.(type) { case wkb.ErrUnknownGeometryType: @@ -594,7 +594,7 @@ func (p Provider) TileFeatures(ctx context.Context, layer string, tile provider. feature := provider.Feature{ ID: gid, - Geometry: geom, + Geometry: geometry, SRID: plyr.SRID(), Tags: tags, } diff --git a/provider/postgis/postgis_test.go b/provider/postgis/postgis_test.go index 1694cd2ab..474d11c9b 100644 --- a/provider/postgis/postgis_test.go +++ b/provider/postgis/postgis_test.go @@ -458,6 +458,17 @@ func TestTileFeatures(t *testing.T) { LayerName: "missing_geom_field_name", }, }, + "empty geometry collection": { + layerConfig: map[string]interface{}{ + postgis.ConfigKeyLayerName: "empty_geometry_collection", + postgis.ConfigKeyGeomField: "geom", + postgis.ConfigKeyGeomType: "polygon", // bypass the geometry type sniff on init + postgis.ConfigKeySQL: "SELECT ST_AsBinary(ST_GeomFromText('GEOMETRYCOLLECTION EMPTY')) AS geom, !BBOX! AS bbox", + }, + tile: slippy.NewTile(16, 11241, 26168, 64, tegola.WebMercator), + expectedFeatureCount: 1, + expectedTags: []string{}, + }, } for name, tc := range tests { diff --git a/provider/test/emptycollection/layer.go b/provider/test/emptycollection/layer.go new file mode 100644 index 000000000..4c5f395c7 --- /dev/null +++ b/provider/test/emptycollection/layer.go @@ -0,0 +1,21 @@ +package emptycollection + +import "github.com/go-spatial/geom" + +type layer struct { + name string + geomType geom.Geometry + srid uint64 +} + +func (l layer) Name() string { + return l.name +} + +func (l layer) GeomType() geom.Geometry { + return l.geomType +} + +func (l layer) SRID() uint64 { + return l.srid +} diff --git a/provider/test/emptycollection/provider.go b/provider/test/emptycollection/provider.go new file mode 100644 index 000000000..5e799323e --- /dev/null +++ b/provider/test/emptycollection/provider.go @@ -0,0 +1,54 @@ +package emptycollection + +import ( + "context" + + "github.com/go-spatial/geom" + "github.com/go-spatial/tegola" + "github.com/go-spatial/tegola/provider" + + "github.com/go-spatial/tegola/dict" +) + +const Name = "emptycollection" + +var Count int + +func init() { + provider.Register(Name, NewTileProvider, Cleanup) +} + +// NewProvider setups a test provider. there are not currently any config params supported +func NewTileProvider(config dict.Dicter) (provider.Tiler, error) { + Count++ + return &TileProvider{}, nil +} + +// Cleanup cleans up all the test providers. +func Cleanup() { Count = 0 } + +type TileProvider struct{} + +func (tp *TileProvider) Layers() ([]provider.LayerInfo, error) { + return []provider.LayerInfo{ + layer{ + name: "empty_geom_collection", + geomType: geom.Collection{}, + srid: tegola.WebMercator, + }, + }, nil +} + +// TilFeatures always returns a feature with a polygon outlining the tile's Extent (not Buffered Extent) +func (tp *TileProvider) TileFeatures(ctx context.Context, layer string, t provider.Tile, fn func(f *provider.Feature) error) error { + // get tile bounding box + _, srid := t.Extent() + + debugTileOutline := provider.Feature{ + ID: 0, + Geometry: geom.Collection{}, // empty geometry collection + SRID: srid, + } + + return fn(&debugTileOutline) +}