diff --git a/pkg/geo/s2.go b/pkg/geo/s2.go index 38d90f869..7a8562008 100644 --- a/pkg/geo/s2.go +++ b/pkg/geo/s2.go @@ -1,11 +1,8 @@ package geo import ( - "bufio" "bytes" "math" - "strconv" - "strings" "github.com/golang/geo/s1" "github.com/golang/geo/s2" @@ -134,50 +131,3 @@ func Covering(points []s2.Point) (s2.CellUnion, error) { } return RegionCoverer.Covering(loop), nil } - -// AreaToCellIDs parses "area" in the format 'lat0,lon0,lat1,lon1,...' -// and returns the resulting s2.CellUnion, or else: -// * ErrOddNumberOfCoordinatesInAreaString -// * ErrNotEnoughPointsInPolygon -// * ErrBadCoordSet -// -// TODO(tvoss): -// * Agree and implement a maximum number of points in area -func AreaToCellIDs(area string) (s2.CellUnion, error) { - var ( - lat, lng float64 - points = []s2.Point{} - counter = 0 - scanner = bufio.NewScanner(strings.NewReader(area)) - ) - numCoords := strings.Count(area, ",") + 1 - if numCoords%2 == 1 { - return nil, ErrOddNumberOfCoordinatesInAreaString - } - if numCoords/2 < 3 { - return nil, ErrNotEnoughPointsInPolygon - } - scanner.Split(splitAtComma) - - for scanner.Scan() { - trimmed := strings.TrimSpace(scanner.Text()) - switch counter % 2 { - case 0: - f, err := strconv.ParseFloat(trimmed, 64) - if err != nil { - return nil, stacktrace.Propagate(ErrBadCoordSet, "Unable to parse lat: %s", err.Error()) - } - lat = f - case 1: - f, err := strconv.ParseFloat(trimmed, 64) - if err != nil { - return nil, stacktrace.Propagate(ErrBadCoordSet, "Unable to parse lng: %s", err.Error()) - } - lng = f - points = append(points, s2.PointFromLatLng(s2.LatLngFromDegrees(lat, lng))) - } - - counter++ - } - return Covering(points) -} diff --git a/pkg/geo/s2_test.go b/pkg/geo/s2_test.go index aefba8a6e..1670dc3c0 100644 --- a/pkg/geo/s2_test.go +++ b/pkg/geo/s2_test.go @@ -1,82 +1,89 @@ -package geo_test +package geo import ( "testing" - "github.com/interuss/dss/pkg/geo" - "github.com/interuss/dss/pkg/geo/testdata" + "github.com/golang/geo/s2" "github.com/stretchr/testify/require" ) -func TestParseAreaSuccessForOddNumberOfPoints(t *testing.T) { - cells, err := geo.AreaToCellIDs(`37.4047,-122.1474,37.4037,-122.1485,37.4035,-122.1466`) - require.NoError(t, err) - require.NotNil(t, cells) -} - -func TestParseAreaSuccessForOppositeWindingOrder(t *testing.T) { - cells, err := geo.AreaToCellIDs(`0.000,0.000, 0.000,0.005, -0.005,0.0025`) - require.NoError(t, err) - require.NotNil(t, cells) -} - -func TestParseAreaSuccessForEvenNumberOfPoints(t *testing.T) { - cells, err := geo.AreaToCellIDs(`37.4047,-122.1474,37.4037,-122.1485,37.4035,-122.1466,37.4043,-122.146`) +func TestParseAreaSucceedsForValidLoop(t *testing.T) { + // square shape polygon + // d-c + // | | + // a-b + pts := []s2.Point{ + s2.PointFromLatLng(s2.LatLngFromDegrees(0.000, 0.000)), + s2.PointFromLatLng(s2.LatLngFromDegrees(0.000, 0.005)), + s2.PointFromLatLng(s2.LatLngFromDegrees(0.005, 0.005)), + s2.PointFromLatLng(s2.LatLngFromDegrees(0.005, 0.000)), + } + cells, err := Covering(pts) require.NoError(t, err) require.NotNil(t, cells) } -func TestParseAreaSucceedsForValidLoop(t *testing.T) { - cells, err := geo.AreaToCellIDs(testdata.Loop) +func TestGeoPolygonFromRestSuccessForOppositeWindingOrder(t *testing.T) { + // square shape polygon + // b-c + // | | + // a-d + pts := []s2.Point{ + s2.PointFromLatLng(s2.LatLngFromDegrees(0.000, 0.000)), + s2.PointFromLatLng(s2.LatLngFromDegrees(0.005, 0.000)), + s2.PointFromLatLng(s2.LatLngFromDegrees(0.005, 0.005)), + s2.PointFromLatLng(s2.LatLngFromDegrees(0.000, 0.005)), + } + cells, err := Covering(pts) require.NoError(t, err) require.NotNil(t, cells) } -func TestParseAreaFailsForEmptyString(t *testing.T) { - cells, err := geo.AreaToCellIDs("") - require.Error(t, err) - require.Nil(t, cells) -} - -func TestParseAreaFailsForIntersectingLoop(t *testing.T) { +func TestCoveringFailsForIntersectingLoop(t *testing.T) { // hourglass shape polygon // c-b // X // a-d - cells, err := geo.AreaToCellIDs(`0.000,0.000,0.005,0.005,0.000,0.005,0.005,0.000`) + pts := []s2.Point{ + s2.PointFromLatLng(s2.LatLngFromDegrees(0.000, 0.000)), + s2.PointFromLatLng(s2.LatLngFromDegrees(0.005, 0.005)), + s2.PointFromLatLng(s2.LatLngFromDegrees(0.005, 0.005)), + s2.PointFromLatLng(s2.LatLngFromDegrees(0.005, 0.000)), + } + cells, err := Covering(pts) require.Error(t, err) require.Nil(t, cells) } -func TestParseAreaFailsForSharedVertexLoop(t *testing.T) { +func TestCoveringFailsForSharedVertexLoop(t *testing.T) { // L shape polygon // a-b+d // | // c - cells, err := geo.AreaToCellIDs(`0.000,0.000,0.000,0.005,-0.005,-0.005,0.000,0.005`) + pts := []s2.Point{ + s2.PointFromLatLng(s2.LatLngFromDegrees(0.000, 0.000)), + s2.PointFromLatLng(s2.LatLngFromDegrees(0.000, 0.005)), + s2.PointFromLatLng(s2.LatLngFromDegrees(-0.005, -0.005)), + s2.PointFromLatLng(s2.LatLngFromDegrees(0.000, 0.005)), + } + cells, err := Covering(pts) require.Error(t, err) require.Nil(t, cells) } -func TestParseAreSucceedsForColinearLoop(t *testing.T) { +func TestCoveringSucceedsForColinearLoop(t *testing.T) { // s2 implements a consistent perturbation model such // that no three points are ever considered to be collinear // line shape polygon // a-b-c-d - cells, err := geo.AreaToCellIDs(`0.000,0.000,0.000,0.005,0.000,0.010,0.000,0.015`) + pts := []s2.Point{ + s2.PointFromLatLng(s2.LatLngFromDegrees(0.000, 0.000)), + s2.PointFromLatLng(s2.LatLngFromDegrees(0.000, 0.005)), + s2.PointFromLatLng(s2.LatLngFromDegrees(0.000, 0.010)), + s2.PointFromLatLng(s2.LatLngFromDegrees(0.000, 0.015)), + } + cells, err := Covering(pts) require.NoError(t, err) require.NotNil(t, cells) } - -func TestParseAreaFailsForLoopWithOnlyTwoPoints(t *testing.T) { - cells, err := geo.AreaToCellIDs(testdata.LoopWithOnlyTwoPoints) - require.Error(t, err) - require.Nil(t, cells) -} - -func TestParseAreaFailsForLoopWithOddNumberOfCoordinates(t *testing.T) { - cells, err := geo.AreaToCellIDs(testdata.LoopWithOddNumberOfCoordinates) - require.Error(t, err) - require.Nil(t, cells) -} diff --git a/pkg/rid/models/api/common/conversions.go b/pkg/rid/models/api/common/conversions.go new file mode 100644 index 000000000..821de829f --- /dev/null +++ b/pkg/rid/models/api/common/conversions.go @@ -0,0 +1,73 @@ +package common + +import ( + "bufio" + "bytes" + "strconv" + "strings" + + "github.com/interuss/dss/pkg/geo" + dssmodels "github.com/interuss/dss/pkg/models" + "github.com/interuss/stacktrace" +) + +func splitAtComma(data []byte, atEOF bool) (int, []byte, error) { + if atEOF && len(data) == 0 { + return 0, nil, nil + } + + if i := bytes.IndexByte(data, ','); i >= 0 { + return i + 1, data[:i], nil + } + + if atEOF { + return len(data), data, nil + } + + return 0, nil, nil +} + +// AreaFromRest parses "area" in the format 'lat0,lon0,lat1,lon1,...' +// and returns the resulting GeoPolygon, or else: +// * ErrOddNumberOfCoordinatesInAreaString +// * ErrNotEnoughPointsInPolygon +// * ErrBadCoordSet +// +// TODO(tvoss): +// * Agree and implement a maximum number of points in area +func FromGeoPolygonSring(area string) (*dssmodels.GeoPolygon, error) { + var ( + lat, lng float64 + err error + counter = 0 + scanner = bufio.NewScanner(strings.NewReader(area)) + ) + numCoords := strings.Count(area, ",") + 1 + if numCoords%2 == 1 { + return nil, geo.ErrOddNumberOfCoordinatesInAreaString + } + if numCoords/2 < 3 { + return nil, geo.ErrNotEnoughPointsInPolygon + } + scanner.Split(splitAtComma) + + vertices := make([]*dssmodels.LatLngPoint, 0, numCoords/2) + for scanner.Scan() { + trimmed := strings.TrimSpace(scanner.Text()) + switch counter % 2 { + case 0: + lat, err = strconv.ParseFloat(trimmed, 64) + if err != nil { + return nil, stacktrace.Propagate(geo.ErrBadCoordSet, "Unable to parse lat: %s", err.Error()) + } + case 1: + lng, err = strconv.ParseFloat(trimmed, 64) + if err != nil { + return nil, stacktrace.Propagate(geo.ErrBadCoordSet, "Unable to parse lng: %s", err.Error()) + } + vertices = append(vertices, &dssmodels.LatLngPoint{Lat: lat, Lng: lng}) + } + counter++ + } + return &dssmodels.GeoPolygon{Vertices: vertices}, nil +} diff --git a/pkg/rid/models/api/common/conversions_test.go b/pkg/rid/models/api/common/conversions_test.go new file mode 100644 index 000000000..11c1fb788 --- /dev/null +++ b/pkg/rid/models/api/common/conversions_test.go @@ -0,0 +1,64 @@ +package common + +import ( + "testing" + + dssmodels "github.com/interuss/dss/pkg/models" + "github.com/stretchr/testify/require" +) + +func TestFromGeoPolygonSring(t *testing.T) { + testCases := []struct { + name string + area string + wantErr bool + want *dssmodels.GeoPolygon + }{ + { + name: "OddNumberOfPoints", + area: `1.1,1.2,2.1,2.2,3.1,3.2`, + want: &dssmodels.GeoPolygon{Vertices: []*dssmodels.LatLngPoint{ + {Lat: 1.1, Lng: 1.2}, + {Lat: 2.1, Lng: 2.2}, + {Lat: 3.1, Lng: 3.2}, + }}, + }, + { + name: "EvenNumberOfPoints", + area: `1.1,1.2,2.1,2.2,3.1,3.2,4.1,4.2`, + want: &dssmodels.GeoPolygon{Vertices: []*dssmodels.LatLngPoint{ + {Lat: 1.1, Lng: 1.2}, + {Lat: 2.1, Lng: 2.2}, + {Lat: 3.1, Lng: 3.2}, + {Lat: 4.1, Lng: 4.2}, + }}, + }, + { + name: "Empty", + area: "", + wantErr: true, + }, + { + name: "TwoPoints", + area: "1,2,3,4", + wantErr: true, + }, + { + name: "OddNumberOfCoordinates", + area: "1,2,3", + wantErr: true, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + p, err := FromGeoPolygonSring(testCase.area) + if testCase.wantErr { + require.Error(t, err) + } else { + require.NoError(t, err) + require.Equal(t, testCase.want, p) + } + }) + } +} diff --git a/pkg/rid/models/api/v1/conversions.go b/pkg/rid/models/api/v1/conversions.go index c3fdd6711..69abe7b72 100644 --- a/pkg/rid/models/api/v1/conversions.go +++ b/pkg/rid/models/api/v1/conversions.go @@ -6,6 +6,7 @@ import ( restapi "github.com/interuss/dss/pkg/api/ridv1" dssmodels "github.com/interuss/dss/pkg/models" ridmodels "github.com/interuss/dss/pkg/rid/models" + "github.com/interuss/dss/pkg/rid/models/api/common" "github.com/interuss/stacktrace" ) @@ -56,6 +57,11 @@ func FromGeoPolygon(footprint *restapi.GeoPolygon) *dssmodels.GeoPolygon { return result } +// FromGeoPolygonString converts RID v1 REST model to business object +func FromGeoPolygonString(area restapi.GeoPolygonString) (*dssmodels.GeoPolygon, error) { + return common.FromGeoPolygonSring((string)(area)) +} + // FromLatLngPoint converts RID v1 REST model to business object func FromLatLngPoint(pt *restapi.LatLngPoint) *dssmodels.LatLngPoint { return &dssmodels.LatLngPoint{ diff --git a/pkg/rid/models/api/v2/conversions.go b/pkg/rid/models/api/v2/conversions.go index b84992b0a..7f3dfbe61 100644 --- a/pkg/rid/models/api/v2/conversions.go +++ b/pkg/rid/models/api/v2/conversions.go @@ -6,6 +6,7 @@ import ( restapi "github.com/interuss/dss/pkg/api/ridv2" dssmodels "github.com/interuss/dss/pkg/models" ridmodels "github.com/interuss/dss/pkg/rid/models" + "github.com/interuss/dss/pkg/rid/models/api/common" "github.com/interuss/stacktrace" ) @@ -122,6 +123,11 @@ func FromPolygon(polygon *restapi.Polygon) *dssmodels.GeoPolygon { return result } +// FromGeoPolygonString converts RID v2 REST model to business object +func FromGeoPolygonString(area restapi.GeoPolygonString) (*dssmodels.GeoPolygon, error) { + return common.FromGeoPolygonSring((string)(area)) +} + // FromCircle converts RID v2 REST model to business object func FromCircle(circle *restapi.Circle) (*dssmodels.GeoCircle, error) { if circle.Center == nil { diff --git a/pkg/rid/server/v1/isa_handler.go b/pkg/rid/server/v1/isa_handler.go index aa4e6b0b4..d04abcc76 100644 --- a/pkg/rid/server/v1/isa_handler.go +++ b/pkg/rid/server/v1/isa_handler.go @@ -240,12 +240,16 @@ func (s *Server) SearchIdentificationServiceAreas(ctx context.Context, req *rest return restapi.SearchIdentificationServiceAreasResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Missing area"))}} } - cu, err := geo.AreaToCellIDs(string(*req.Area)) + p, err := apiv1.FromGeoPolygonString(*req.Area) if err != nil { - if errors.Is(err, geo.ErrAreaTooLarge) { - return restapi.SearchIdentificationServiceAreasResponseSet{Response413: &restapi.ErrorResponse{ - Message: dsserr.Handle(ctx, stacktrace.Propagate(err, "Invalid area"))}} - } + return restapi.SearchIdentificationServiceAreasResponseSet{Response400: &restapi.ErrorResponse{ + Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Invalid area"))}} + } + cu, err := p.CalculateCovering() + if errors.Is(err, geo.ErrAreaTooLarge) { + return restapi.SearchIdentificationServiceAreasResponseSet{Response413: &restapi.ErrorResponse{ + Message: dsserr.Handle(ctx, stacktrace.Propagate(err, "Invalid area"))}} + } else if err != nil { return restapi.SearchIdentificationServiceAreasResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Invalid area"))}} } diff --git a/pkg/rid/server/v1/server_test.go b/pkg/rid/server/v1/server_test.go index dcb661e73..81927a9f0 100644 --- a/pkg/rid/server/v1/server_test.go +++ b/pkg/rid/server/v1/server_test.go @@ -10,7 +10,6 @@ import ( "github.com/interuss/dss/pkg/api" restapi "github.com/interuss/dss/pkg/api/ridv1" dsserr "github.com/interuss/dss/pkg/errors" - "github.com/interuss/dss/pkg/geo" "github.com/interuss/dss/pkg/geo/testdata" dssmodels "github.com/interuss/dss/pkg/models" ridmodels "github.com/interuss/dss/pkg/rid/models" @@ -679,7 +678,10 @@ func TestSearchIdentificationServiceAreas(t *testing.T) { } func TestDefaultRegionCovererProducesResults(t *testing.T) { - cover, err := geo.AreaToCellIDs(testdata.Loop) + p, err := apiv1.FromGeoPolygonString((restapi.GeoPolygonString)(testdata.Loop)) require.NoError(t, err) - require.NotNil(t, cover) + require.NotNil(t, p) + cu, err := p.CalculateCovering() + require.NoError(t, err) + require.NotNil(t, cu) } diff --git a/pkg/rid/server/v1/subscription_handler.go b/pkg/rid/server/v1/subscription_handler.go index af587657a..ebfad2093 100644 --- a/pkg/rid/server/v1/subscription_handler.go +++ b/pkg/rid/server/v1/subscription_handler.go @@ -67,12 +67,16 @@ func (s *Server) SearchSubscriptions(ctx context.Context, req *restapi.SearchSub return restapi.SearchSubscriptionsResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Missing area"))}} } - cu, err := geo.AreaToCellIDs(string(*req.Area)) + p, err := apiv1.FromGeoPolygonString(*req.Area) if err != nil { - if errors.Is(err, geo.ErrAreaTooLarge) { - return restapi.SearchSubscriptionsResponseSet{Response413: &restapi.ErrorResponse{ - Message: dsserr.Handle(ctx, stacktrace.Propagate(err, "Invalid area"))}} - } + return restapi.SearchSubscriptionsResponseSet{Response400: &restapi.ErrorResponse{ + Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Invalid area"))}} + } + cu, err := p.CalculateCovering() + if errors.Is(err, geo.ErrAreaTooLarge) { + return restapi.SearchSubscriptionsResponseSet{Response413: &restapi.ErrorResponse{ + Message: dsserr.Handle(ctx, stacktrace.Propagate(err, "Invalid area"))}} + } else if err != nil { return restapi.SearchSubscriptionsResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Invalid area"))}} } diff --git a/pkg/rid/server/v2/isa_handler.go b/pkg/rid/server/v2/isa_handler.go index 50cd94764..ff2d520f8 100644 --- a/pkg/rid/server/v2/isa_handler.go +++ b/pkg/rid/server/v2/isa_handler.go @@ -233,12 +233,16 @@ func (s *Server) SearchIdentificationServiceAreas(ctx context.Context, req *rest return restapi.SearchIdentificationServiceAreasResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Missing area"))}} } - cu, err := geo.AreaToCellIDs(string(*req.Area)) + p, err := apiv2.FromGeoPolygonString(*req.Area) if err != nil { - if errors.Is(err, geo.ErrAreaTooLarge) { - return restapi.SearchIdentificationServiceAreasResponseSet{Response413: &restapi.ErrorResponse{ - Message: dsserr.Handle(ctx, stacktrace.Propagate(err, "Invalid area"))}} - } + return restapi.SearchIdentificationServiceAreasResponseSet{Response400: &restapi.ErrorResponse{ + Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Invalid area"))}} + } + cu, err := p.CalculateCovering() + if errors.Is(err, geo.ErrAreaTooLarge) { + return restapi.SearchIdentificationServiceAreasResponseSet{Response413: &restapi.ErrorResponse{ + Message: dsserr.Handle(ctx, stacktrace.Propagate(err, "Invalid area"))}} + } else if err != nil { return restapi.SearchIdentificationServiceAreasResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Invalid area"))}} } diff --git a/pkg/rid/server/v2/subscription_handler.go b/pkg/rid/server/v2/subscription_handler.go index 7fb14bdb4..3e956b3b1 100644 --- a/pkg/rid/server/v2/subscription_handler.go +++ b/pkg/rid/server/v2/subscription_handler.go @@ -67,12 +67,16 @@ func (s *Server) SearchSubscriptions(ctx context.Context, req *restapi.SearchSub return restapi.SearchSubscriptionsResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Missing area"))}} } - cu, err := geo.AreaToCellIDs(string(*req.Area)) + p, err := apiv2.FromGeoPolygonString(*req.Area) if err != nil { - if errors.Is(err, geo.ErrAreaTooLarge) { - return restapi.SearchSubscriptionsResponseSet{Response413: &restapi.ErrorResponse{ - Message: dsserr.Handle(ctx, stacktrace.Propagate(err, "Invalid area"))}} - } + return restapi.SearchSubscriptionsResponseSet{Response400: &restapi.ErrorResponse{ + Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Invalid area"))}} + } + cu, err := p.CalculateCovering() + if errors.Is(err, geo.ErrAreaTooLarge) { + return restapi.SearchSubscriptionsResponseSet{Response413: &restapi.ErrorResponse{ + Message: dsserr.Handle(ctx, stacktrace.Propagate(err, "Invalid area"))}} + } else if err != nil { return restapi.SearchSubscriptionsResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Invalid area"))}} }