Merge pull request #1076 from OneBusAway/fix/shape-endpoint-spec-gaps

fix: shape endpoint polyline parity with Java (floor encoding, no point reduction)

Author: burma-shave

internal/restapi/shapes_handler.go

@@ -3,8 +3,8 @@ package restapi
 import (
 	"net/http"
 
-	"github.com/twpayne/go-polyline"
 	"maglev.onebusaway.org/internal/models"
+	"maglev.onebusaway.org/internal/utils"
 )
 
 // shapesHandler returns the encoded polyline shape for a route's geographic path.
@@ -34,18 +34,16 @@ func (api *RestAPI) shapesHandler(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	// Include every point with no simplification or consecutive-duplicate
+	// filtering, matching the Java reference (ShapeBeanServiceImpl.getPolylineForShapeId).
 	lineCoords := make([][]float64, 0, len(shapes))
-
-	for i, point := range shapes {
-		// Filter consecutive duplicate points to avoid zero-length segments
-		if i > 0 && point.Lat == shapes[i-1].Lat && point.Lon == shapes[i-1].Lon {
-			continue
-		}
+	for _, point := range shapes {
 		lineCoords = append(lineCoords, []float64{point.Lat, point.Lon})
 	}
 
-	// Encode as a single continuous polyline to ensure valid delta offsets
-	encodedPoints := string(polyline.EncodeCoords(lineCoords))
+	// Encode using a floor-based encoder to stay byte-for-byte identical to the
+	// Java PolylineEncoder (which floors coordinates rather than rounding).
+	encodedPoints := utils.EncodePolyline(lineCoords)
 
 	shapeEntry := models.ShapeEntry{
 		Length: len(lineCoords),

internal/restapi/shapes_handler_test.go

@@ -187,15 +187,15 @@ func TestShapesHandler_PolylineDecoding(t *testing.T) {
 			expected: [][2]float64{{0.0, 0.0}, {1.0, 1.0}, {2.0, 2.0}, {1.0, 1.0}, {0.0, 0.0}},
 		},
 		{
-			name:    "Consecutive duplicates (B repeats) are deduplicated",
+			name:    "Consecutive duplicates are preserved (no point reduction)",
 			shapeID: "duplicate_shape",
 			input: []shapePoint{
 				{0.0, 0.0, 1},
 				{1.0, 1.0, 2},
-				{1.0, 1.0, 3}, // duplicate of previous point — handler filters this
+				{1.0, 1.0, 3}, // duplicate of previous point — must NOT be filtered (spec: all points included)
 				{2.0, 2.0, 4},
 			},
-			expected: [][2]float64{{0.0, 0.0}, {1.0, 1.0}, {2.0, 2.0}},
+			expected: [][2]float64{{0.0, 0.0}, {1.0, 1.0}, {1.0, 1.0}, {2.0, 2.0}},
 		},
 	}
 

internal/utils/polyline.go

@@ -0,0 +1,45 @@
+package utils
+
+import "math"
+
+// EncodePolyline encodes an ordered sequence of [lat, lon] coordinate pairs into a
+// Google Encoded Polyline string.
+//
+// It deliberately mirrors the OneBusAway Java reference implementation
+// (org.onebusaway.geospatial.services.PolylineEncoder), which floors coordinates with
+// floor(coord * 1e5) rather than rounding. Matching the floor behaviour keeps maglev's
+// `points` output byte-for-byte identical to the Java server. All points are included;
+// no simplification or consecutive-duplicate filtering is performed.
+func EncodePolyline(coords [][]float64) string {
+	var b []byte
+	var plat, plng int
+	for _, c := range coords {
+		late5 := floor1e5(c[0])
+		lnge5 := floor1e5(c[1])
+		b = encodeSignedNumber(b, late5-plat)
+		b = encodeSignedNumber(b, lnge5-plng)
+		plat = late5
+		plng = lnge5
+	}
+	return string(b)
+}
+
+func floor1e5(coordinate float64) int {
+	return int(math.Floor(coordinate * 1e5))
+}
+
+func encodeSignedNumber(b []byte, num int) []byte {
+	sgnNum := num << 1
+	if num < 0 {
+		sgnNum = ^sgnNum
+	}
+	return encodeNumber(b, sgnNum)
+}
+
+func encodeNumber(b []byte, num int) []byte {
+	for num >= 0x20 {
+		b = append(b, byte((0x20|(num&0x1f))+63))
+		num >>= 5
+	}
+	return append(b, byte(num+63))
+}

internal/utils/polyline_test.go

@@ -0,0 +1,59 @@
+package utils
+
+import "testing"
+
+func TestEncodePolyline_GoogleExample(t *testing.T) {
+	// Canonical example from Google's polyline algorithm documentation.
+	coords := [][]float64{
+		{38.5, -120.2},
+		{40.7, -120.95},
+		{43.252, -126.453},
+	}
+	got := EncodePolyline(coords)
+	want := "_p~iF~ps|U_ulLnnqC_mqNvxq`@"
+	if got != want {
+		t.Errorf("EncodePolyline() = %q, want %q", got, want)
+	}
+}
+
+func TestEncodePolyline_FloorsNotRounds(t *testing.T) {
+	// 0.000019 * 1e5 = 1.9. Java floors to 1 (same as 0.00001); a rounding
+	// encoder would yield 2 (same as 0.00002). Verify we floor.
+	got := EncodePolyline([][]float64{{0.000019, 0}})
+	floored := EncodePolyline([][]float64{{0.00001, 0}})
+	rounded := EncodePolyline([][]float64{{0.00002, 0}})
+	if got != floored {
+		t.Errorf("floor(0.000019*1e5) should match 0.00001 encoding; got %q want %q", got, floored)
+	}
+	if got == rounded {
+		t.Errorf("floored encoding should differ from rounded 0.00002 encoding %q", rounded)
+	}
+
+	// Negative boundary: -0.000019 * 1e5 = -1.9. Java floors to -2 (same as
+	// -0.00002); truncation or rounding would give -1 (same as -0.00001).
+	gotNeg := EncodePolyline([][]float64{{-0.000019, 0}})
+	flooredNeg := EncodePolyline([][]float64{{-0.00002, 0}})
+	truncOrRoundNeg := EncodePolyline([][]float64{{-0.00001, 0}})
+	if gotNeg != flooredNeg {
+		t.Errorf("floor(-0.000019*1e5) should match -0.00002 encoding; got %q want %q", gotNeg, flooredNeg)
+	}
+	if gotNeg == truncOrRoundNeg {
+		t.Errorf("floored negative encoding should differ from -0.00001 (truncate/round) encoding %q", truncOrRoundNeg)
+	}
+}
+
+func TestEncodePolyline_PreservesDuplicates(t *testing.T) {
+	// A consecutive duplicate point must still be encoded (delta 0,0), not dropped.
+	coords := [][]float64{{1.0, 1.0}, {1.0, 1.0}, {2.0, 2.0}}
+	withDup := EncodePolyline(coords)
+	withoutDup := EncodePolyline([][]float64{{1.0, 1.0}, {2.0, 2.0}})
+	if withDup == withoutDup {
+		t.Errorf("duplicate point should add a zero-delta segment; encodings should differ")
+	}
+}
+
+func TestEncodePolyline_Empty(t *testing.T) {
+	if got := EncodePolyline(nil); got != "" {
+		t.Errorf("EncodePolyline(nil) = %q, want empty string", got)
+	}
+}