[rid] Extract polygon conversion from geo module

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)
-}

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)
-}

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
+}

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)
+			}
+		})
+	}
+}

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{

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 {

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"))}}
 	}