feat: talk to the optimizer in Doormile's own vocabulary

The client was speaking jupiter's provider dialect -- deliveryid,
pickuplat, coordinates as strings -- because that was the only endpoint
the Route Optimization API offered. rider-bike now has
/api/v1/optimization/doormile/sequence, which takes bookingid and
pickuplatitude, so the translation layer is gone.

The new endpoint validates its body; the provider one cannot, because
jupiter is live on it and tightening it would break real deliveries.
That matters here: sending the provider endpoint the wrong field names
returns HTTP 200 "Success" with every coordinate defaulted to 0.0, no
reordering and all distances zero. The Doormile endpoint rejects that
outright, and rejects 0,0 coordinates, which are inside the valid range
but are a point in the Atlantic that drags a whole route toward it.

Responses now come back properly typed, so the loose float/string
coercion is deleted rather than kept for a shape that no longer arrives.
Tests replaced to match: they run the real client against a stub server
and pin the outbound field names, the mapping back onto assignment ids,
that steps for assignments we never sent are discarded, that step 0 is
not persisted as a position, and that a failed optimise surfaces an
error instead of quietly looking like success.

Needs rider-bike deployed first; until then sequencing fails
best-effort, which leaves bookings assigned but unordered.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Suriya
2026-08-11 15:29:08 +05:30
parent 0288fb7af8
commit cb2660a3da
2 changed files with 220 additions and 136 deletions

View File

@@ -1,68 +1,172 @@
package routing
import "testing"
import (
"encoding/json"
"net/http"
"net/http/httptest"
"testing"
)
// The optimizer returns the same logical field as a number in one place and a
// string in another — previouskms came back as 4, actualkms as "5.09", eta as
// "20". Binding those to concrete types would have silently zeroed half the
// response, so the coercion is worth pinning.
func TestAsFloat(t *testing.T) {
// stubOptimizer stands in for the Route Optimization API. It captures the
// request body so the outbound contract can be asserted, and returns whatever
// the test tells it to.
func stubOptimizer(t *testing.T, captured *optimizeRequest, respond func(optimizeRequest) optimizeResponse) *httptest.Server {
t.Helper()
return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path != optimizePath {
t.Errorf("posted to %s, want %s", r.URL.Path, optimizePath)
}
var req optimizeRequest
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
t.Errorf("stub could not decode request: %v", err)
}
if captured != nil {
*captured = req
}
w.Header().Set("Content-Type", "application/json")
_ = json.NewEncoder(w).Encode(respond(req))
}))
}
func testStops() []stop {
return []stop{
{AssignmentID: 1, BookingID: 101, BookingNo: "A", PickupLat: 11.0045, PickupLng: 76.9612, DeliveryLat: 11.0510, DeliveryLng: 76.9300},
{AssignmentID: 2, BookingID: 102, BookingNo: "B", PickupLat: 11.0045, PickupLng: 76.9612, DeliveryLat: 11.0168, DeliveryLng: 76.9558},
{AssignmentID: 3, BookingID: 103, BookingNo: "C", PickupLat: 11.0045, PickupLng: 76.9612, DeliveryLat: 10.9938, DeliveryLng: 76.9954},
}
}
// The whole point of the Doormile endpoint is that it takes Doormile's field
// names. Sending the provider ones (pickuplat/deliverylat) against the provider
// endpoint returns HTTP 200 with everything silently zeroed, so this contract is
// worth pinning rather than trusting.
func TestOptimizeSendsDoormileFieldNames(t *testing.T) {
var got optimizeRequest
srv := stubOptimizer(t, &got, func(req optimizeRequest) optimizeResponse {
return optimizeResponse{Success: true, Stops: []optimizeResponseStop{
{Bookingassignmentid: 1, Bookingid: 101, Step: 1},
}}
})
defer srv.Close()
BaseURL = srv.URL
defer func() { BaseURL = "" }()
if _, err := optimize(testStops()); err != nil {
t.Fatalf("optimize: %v", err)
}
if len(got.Bookings) != 3 {
t.Fatalf("sent %d bookings, want 3", len(got.Bookings))
}
first := got.Bookings[0]
if first.Bookingid != 101 || first.Bookingassignmentid != 1 {
t.Errorf("identity fields wrong: %+v", first)
}
// Coordinates must survive as real numbers, not be rounded or stringified
// into a different place.
if first.Pickuplatitude != 11.0045 || first.Deliverylongitude != 76.93 {
t.Errorf("coordinates mangled: %+v", first)
}
}
func TestOptimizeMapsResultsBackToAssignments(t *testing.T) {
srv := stubOptimizer(t, nil, func(req optimizeRequest) optimizeResponse {
// Reverse the order, as a real resequencing would.
return optimizeResponse{Success: true, Stops: []optimizeResponseStop{
{Bookingassignmentid: 3, Bookingid: 103, Step: 1, Previouskms: 4, Cumulativekms: 4, Etaminutes: 14, Cumulativeeta: 14},
{Bookingassignmentid: 2, Bookingid: 102, Step: 2, Previouskms: 5, Cumulativekms: 9, Etaminutes: 8, Cumulativeeta: 22},
{Bookingassignmentid: 1, Bookingid: 101, Step: 3, Previouskms: 5, Cumulativekms: 14, Etaminutes: 8, Cumulativeeta: 30},
}}
})
defer srv.Close()
BaseURL = srv.URL
defer func() { BaseURL = "" }()
results, err := optimize(testStops())
if err != nil {
t.Fatalf("optimize: %v", err)
}
if len(results) != 3 {
t.Fatalf("got %d results, want 3", len(results))
}
if results[0].AssignmentID != 3 || results[0].Step != 1 || results[0].CumulativeETA != 14 {
t.Errorf("first result wrong: %+v", results[0])
}
if results[2].AssignmentID != 1 || results[2].CumulativeKM != 14 {
t.Errorf("last result wrong: %+v", results[2])
}
}
// These steps get written straight onto assignment rows, so a step for an
// assignment we never sent must never make it through — it would reorder some
// other rider's work.
func TestOptimizeDiscardsUnknownAssignments(t *testing.T) {
srv := stubOptimizer(t, nil, func(req optimizeRequest) optimizeResponse {
return optimizeResponse{Success: true, Stops: []optimizeResponseStop{
{Bookingassignmentid: 999, Bookingid: 999, Step: 1},
{Bookingassignmentid: 2, Bookingid: 102, Step: 2, Cumulativekms: 9},
}}
})
defer srv.Close()
BaseURL = srv.URL
defer func() { BaseURL = "" }()
results, err := optimize(testStops())
if err != nil {
t.Fatalf("optimize: %v", err)
}
if len(results) != 1 || results[0].AssignmentID != 2 {
t.Fatalf("unknown assignment leaked through: %+v", results)
}
}
// Step 0 means "not sequenced". Persisting it would read as a position.
func TestOptimizeDropsZeroSteps(t *testing.T) {
srv := stubOptimizer(t, nil, func(req optimizeRequest) optimizeResponse {
return optimizeResponse{Success: true, Stops: []optimizeResponseStop{
{Bookingassignmentid: 1, Bookingid: 101, Step: 0},
{Bookingassignmentid: 2, Bookingid: 102, Step: 1},
}}
})
defer srv.Close()
BaseURL = srv.URL
defer func() { BaseURL = "" }()
results, err := optimize(testStops())
if err != nil {
t.Fatalf("optimize: %v", err)
}
if len(results) != 1 || results[0].AssignmentID != 2 {
t.Fatalf("step 0 was kept: %+v", results)
}
}
func TestOptimizeErrorsAreSurfacedNotSilent(t *testing.T) {
cases := []struct {
name string
in interface{}
want float64
name string
handler http.HandlerFunc
}{
{"number", float64(4), 4},
{"decimal string", "5.09", 5.09},
{"integer string", "14", 14},
{"zero string the API sends for missing coords", "0.0", 0},
{"nil", nil, 0},
{"unparseable", "n/a", 0},
{"wrong type", true, 0},
{"http 500", func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(500) }},
{"success false", func(w http.ResponseWriter, r *http.Request) {
_ = json.NewEncoder(w).Encode(optimizeResponse{Success: false})
}},
{"empty stops", func(w http.ResponseWriter, r *http.Request) {
_ = json.NewEncoder(w).Encode(optimizeResponse{Success: true, Stops: nil})
}},
{"garbage body", func(w http.ResponseWriter, r *http.Request) {
_, _ = w.Write([]byte("not json"))
}},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
if got := asFloat(tc.in); got != tc.want {
t.Fatalf("asFloat(%#v) = %v, want %v", tc.in, got, tc.want)
srv := httptest.NewServer(tc.handler)
defer srv.Close()
BaseURL = srv.URL
defer func() { BaseURL = "" }()
if _, err := optimize(testStops()); err == nil {
t.Fatal("expected an error, got nil — a failed optimise must not look like success")
}
})
}
}
func TestAsInt(t *testing.T) {
cases := []struct {
name string
in interface{}
want int
}{
{"number", float64(3), 3},
{"integer string", "20", 20},
// Atoi would fail on this and yield 0, which as a step number would
// silently drop the stop from the sequence.
{"decimal string", "20.0", 20},
{"truncates rather than rounds", "20.9", 20},
{"nil", nil, 0},
{"unparseable", "", 0},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
if got := asInt(tc.in); got != tc.want {
t.Fatalf("asInt(%#v) = %v, want %v", tc.in, got, tc.want)
}
})
}
}
// Coordinates must go out as strings with real precision. Formatting with too
// few decimals would collapse neighbouring delivery addresses onto the same
// point and make the ordering meaningless.
func TestCoordKeepsPrecision(t *testing.T) {
if got := coord(11.0045); got != "11.004500" {
t.Fatalf("coord(11.0045) = %q", got)
}
// ~11m apart in Coimbatore; these must not format identically.
a, b := coord(11.004500), coord(11.004600)
if a == b {
t.Fatalf("distinct coordinates formatted identically: %q", a)
}
}