From d6492577be52390d0c14e653702d87588f65f625 Mon Sep 17 00:00:00 2001 From: Ahmedhossamdev Date: Mon, 26 Oct 2026 20:29:59 +0300 Subject: [PATCH 1/3] Expose the static GTFS hash in metadata Watchdog validates GTFS-RT data against its own copy of the static feed, which it refreshes every 24h. After an agency publishes a new bundle, Maglev can serve it long before Watchdog picks it up, and Watchdog reports problems that don't exist. Add staticGtfsHash, the hex sha256 of the raw static zip already stored in import_metadata, to /api/v2/metadata.json and to /healthz?verbose=true. A consumer can poll it, re-pull the feed when it changes, and verify the download with sha256sum. Both handlers built DataFreshness with the same code, so move it into one dataFreshness helper. --- internal/restapi/health_handler.go | 36 ++++++++++++------- internal/restapi/health_handler_test.go | 10 ++++++ internal/restapi/metadata_handler.go | 11 +----- internal/restapi/metadata_handler_test.go | 42 +++++++++++++++++++++++ 4 files changed, 77 insertions(+), 22 deletions(-) diff --git a/internal/restapi/health_handler.go b/internal/restapi/health_handler.go index 39f892a21..28daa4193 100644 --- a/internal/restapi/health_handler.go +++ b/internal/restapi/health_handler.go @@ -1,6 +1,7 @@ package restapi import ( + "context" "encoding/json" "net/http" "time" @@ -8,10 +9,28 @@ import ( "maglev.onebusaway.org/internal/logging" ) -// DataFreshness represents the last update timestamps for GTFS data. +// DataFreshness represents the last update timestamps for GTFS data and +// which static bundle is loaded. type DataFreshness struct { - StaticGtfsLastUpdated *time.Time `json:"staticGtfsLastUpdated,omitempty"` - RealtimeFeeds map[string]time.Time `json:"realtimeFeeds,omitempty"` + StaticGtfsLastUpdated *time.Time `json:"staticGtfsLastUpdated,omitempty"` + // StaticGtfsHash is the hex sha256 of the raw static GTFS zip, so an + // external service can tell whether it validates against the same bundle. + StaticGtfsHash string `json:"staticGtfsHash,omitempty"` + RealtimeFeeds map[string]time.Time `json:"realtimeFeeds,omitempty"` +} + +// dataFreshness reports when the static and realtime data were last updated +// and which static bundle is loaded. +func (api *RestAPI) dataFreshness(ctx context.Context) DataFreshness { + var staticTime *time.Time + if t := api.GtfsManager.GetStaticLastUpdated(ctx); !t.IsZero() { + staticTime = &t + } + return DataFreshness{ + StaticGtfsLastUpdated: staticTime, + StaticGtfsHash: api.GtfsManager.GetSystemETag(ctx), + RealtimeFeeds: api.GtfsManager.GetFeedUpdateTimes(), + } } // HealthResponse represents the JSON response from the health endpoint. @@ -64,15 +83,8 @@ func (api *RestAPI) healthHandler(w http.ResponseWriter, r *http.Request) { // Fetch data freshness from the manager only if verbose=true is passed var freshness *DataFreshness if r.URL.Query().Get("verbose") == "true" { - t := api.GtfsManager.GetStaticLastUpdated(r.Context()) - var staticTime *time.Time - if !t.IsZero() { - staticTime = &t - } - freshness = &DataFreshness{ - StaticGtfsLastUpdated: staticTime, - RealtimeFeeds: api.GtfsManager.GetFeedUpdateTimes(), - } + f := api.dataFreshness(r.Context()) + freshness = &f } // All checks passed diff --git a/internal/restapi/health_handler_test.go b/internal/restapi/health_handler_test.go index c445b78e4..23783bf6e 100644 --- a/internal/restapi/health_handler_test.go +++ b/internal/restapi/health_handler_test.go @@ -195,3 +195,13 @@ func TestHealthHandlerVerboseMode(t *testing.T) { assert.NotNil(t, healthRespVerbose.DataFreshness.StaticGtfsLastUpdated) assert.Contains(t, healthRespVerbose.DataFreshness.RealtimeFeeds, "feed-1") } + +func TestHealthHandler_VerboseIncludesHash(t *testing.T) { + api := createTestApi(t) + + resp, health := callAPIHandler[HealthResponse](t, api, "/healthz?verbose=true") + + assert.Equal(t, http.StatusOK, resp.StatusCode) + require.NotNil(t, health.DataFreshness) + assert.Equal(t, rabaZipSha256(t), health.DataFreshness.StaticGtfsHash) +} diff --git a/internal/restapi/metadata_handler.go b/internal/restapi/metadata_handler.go index c90a50f32..664482fb4 100644 --- a/internal/restapi/metadata_handler.go +++ b/internal/restapi/metadata_handler.go @@ -19,16 +19,7 @@ func (api *RestAPI) metadataHandler(w http.ResponseWriter, r *http.Request) { return } - t := api.GtfsManager.GetStaticLastUpdated(r.Context()) - var staticTime *time.Time - if !t.IsZero() { - staticTime = &t - } - - response := DataFreshness{ - StaticGtfsLastUpdated: staticTime, - RealtimeFeeds: api.GtfsManager.GetFeedUpdateTimes(), - } + response := api.dataFreshness(r.Context()) // Ensure the map isn't nil for JSON serialization if response.RealtimeFeeds == nil { diff --git a/internal/restapi/metadata_handler_test.go b/internal/restapi/metadata_handler_test.go index 065919971..da6d3f2d6 100644 --- a/internal/restapi/metadata_handler_test.go +++ b/internal/restapi/metadata_handler_test.go @@ -2,12 +2,18 @@ package restapi import ( "context" + "crypto/sha256" + "encoding/hex" "encoding/json" "net/http" "net/http/httptest" + "os" + "path/filepath" "testing" "time" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "maglev.onebusaway.org/internal/app" ) @@ -81,3 +87,39 @@ func TestMetadataHandler(t *testing.T) { t.Errorf("Expected StaticGtfsLastUpdated to match set time") } } + +// rabaZipSha256 hashes the static fixture the way an external consumer such +// as Watchdog would, independently of what Maglev stored. +func rabaZipSha256(t *testing.T) string { + t.Helper() + zipBytes, err := os.ReadFile(filepath.Join("../../testdata", "raba.zip")) + require.NoError(t, err) + sum := sha256.Sum256(zipBytes) + return hex.EncodeToString(sum[:]) +} + +func TestMetadataHandler_StaticGtfsHash(t *testing.T) { + api := createTestApi(t) + + resp, freshness := callAPIHandler[DataFreshness](t, api, "/api/v2/metadata.json?key=TEST") + + assert.Equal(t, http.StatusOK, resp.StatusCode) + assert.Equal(t, rabaZipSha256(t), freshness.StaticGtfsHash) +} + +func TestMetadataHandler_NoImportOmitsHash(t *testing.T) { + api := &RestAPI{ + Application: &app.Application{ + GtfsManager: newTestManagerNoData(t), + }, + } + + req := httptest.NewRequest(http.MethodGet, "/api/v2/metadata.json", nil) + rr := httptest.NewRecorder() + api.metadataHandler(rr, req) + + assert.Equal(t, http.StatusOK, rr.Code) + var body map[string]any + require.NoError(t, json.Unmarshal(rr.Body.Bytes(), &body)) + assert.NotContains(t, body, "staticGtfsHash") +} From ae4096895e7b4e9ff812889735ca129be89caa62 Mon Sep 17 00:00:00 2001 From: Ahmedhossamdev Date: Mon, 26 Oct 2026 21:56:58 +0300 Subject: [PATCH 2/3] Assert the staticGtfsHash JSON key in the test Decoding into DataFreshness reads the field through the same JSON tag the handler writes with, so a misspelled tag would still pass. Decode into a map and check the literal key Watchdog depends on. --- internal/restapi/metadata_handler_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/restapi/metadata_handler_test.go b/internal/restapi/metadata_handler_test.go index da6d3f2d6..5db695463 100644 --- a/internal/restapi/metadata_handler_test.go +++ b/internal/restapi/metadata_handler_test.go @@ -101,10 +101,10 @@ func rabaZipSha256(t *testing.T) string { func TestMetadataHandler_StaticGtfsHash(t *testing.T) { api := createTestApi(t) - resp, freshness := callAPIHandler[DataFreshness](t, api, "/api/v2/metadata.json?key=TEST") + resp, body := callAPIHandler[map[string]any](t, api, "/api/v2/metadata.json?key=TEST") assert.Equal(t, http.StatusOK, resp.StatusCode) - assert.Equal(t, rabaZipSha256(t), freshness.StaticGtfsHash) + assert.Equal(t, rabaZipSha256(t), body["staticGtfsHash"]) } func TestMetadataHandler_NoImportOmitsHash(t *testing.T) { From 9667cc542f9ed5cf04a592715066cf0a9224b274 Mon Sep 17 00:00:00 2001 From: Ahmedhossamdev Date: Mon, 26 Oct 2026 21:56:59 +0300 Subject: [PATCH 3/3] Return 503 when freshness metadata reads fail dataFreshness read the hash through GetSystemETag, which returns "" for any error. A failed import_metadata query then looked the same as a database with no import yet: 200 with no staticGtfsHash, so a consumer can't tell "no bundle" from "couldn't read it". Read import_metadata once in dataFreshness. sql.ErrNoRows still leaves the static fields empty; any other error is returned, and metadata.json and healthz?verbose=true answer 503 "unavailable". GetSystemETag keeps its empty-value behavior for the ETag middleware. --- internal/restapi/health_handler.go | 40 +++++++++++++++++------ internal/restapi/health_handler_test.go | 16 +++++++++ internal/restapi/metadata_handler.go | 13 +++++++- internal/restapi/metadata_handler_test.go | 30 +++++++++++++++++ 4 files changed, 88 insertions(+), 11 deletions(-) diff --git a/internal/restapi/health_handler.go b/internal/restapi/health_handler.go index 28daa4193..dd59c8b52 100644 --- a/internal/restapi/health_handler.go +++ b/internal/restapi/health_handler.go @@ -2,7 +2,9 @@ package restapi import ( "context" + "database/sql" "encoding/json" + "errors" "net/http" "time" @@ -20,17 +22,26 @@ type DataFreshness struct { } // dataFreshness reports when the static and realtime data were last updated -// and which static bundle is loaded. -func (api *RestAPI) dataFreshness(ctx context.Context) DataFreshness { - var staticTime *time.Time - if t := api.GtfsManager.GetStaticLastUpdated(ctx); !t.IsZero() { - staticTime = &t +// and which static bundle is loaded. A database with no import yet is not an +// error: the static fields are left empty. Any other query failure is +// returned, so callers don't report a missing bundle when the read failed. +func (api *RestAPI) dataFreshness(ctx context.Context) (DataFreshness, error) { + freshness := DataFreshness{RealtimeFeeds: api.GtfsManager.GetFeedUpdateTimes()} + + metadata, err := api.GtfsManager.GtfsDB.Queries.GetImportMetadata(ctx) + if errors.Is(err, sql.ErrNoRows) { + return freshness, nil + } + if err != nil { + return DataFreshness{}, err } - return DataFreshness{ - StaticGtfsLastUpdated: staticTime, - StaticGtfsHash: api.GtfsManager.GetSystemETag(ctx), - RealtimeFeeds: api.GtfsManager.GetFeedUpdateTimes(), + + if metadata.ImportTime != 0 { + importTime := time.Unix(0, metadata.ImportTime).UTC() + freshness.StaticGtfsLastUpdated = &importTime } + freshness.StaticGtfsHash = metadata.FileHash + return freshness, nil } // HealthResponse represents the JSON response from the health endpoint. @@ -83,7 +94,16 @@ func (api *RestAPI) healthHandler(w http.ResponseWriter, r *http.Request) { // Fetch data freshness from the manager only if verbose=true is passed var freshness *DataFreshness if r.URL.Query().Get("verbose") == "true" { - f := api.dataFreshness(r.Context()) + f, err := api.dataFreshness(r.Context()) + if err != nil { + logging.LogError(reqLogger, "reading data freshness failed", err) + w.WriteHeader(http.StatusServiceUnavailable) + _ = json.NewEncoder(w).Encode(HealthResponse{ + Status: "unavailable", + Detail: "reading data freshness failed", + }) + return + } freshness = &f } diff --git a/internal/restapi/health_handler_test.go b/internal/restapi/health_handler_test.go index 23783bf6e..ea0645209 100644 --- a/internal/restapi/health_handler_test.go +++ b/internal/restapi/health_handler_test.go @@ -205,3 +205,19 @@ func TestHealthHandler_VerboseIncludesHash(t *testing.T) { require.NotNil(t, health.DataFreshness) assert.Equal(t, rabaZipSha256(t), health.DataFreshness.StaticGtfsHash) } + +func TestHealthHandler_VerboseImportMetadataQueryFails(t *testing.T) { + manager := newTestManagerWithoutImportMetadata(t) + manager.MarkReady() + api := NewRestAPI(&app.Application{GtfsManager: manager}) + + req := httptest.NewRequest(http.MethodGet, "/healthz?verbose=true", nil) + rr := httptest.NewRecorder() + api.healthHandler(rr, req) + + assert.Equal(t, http.StatusServiceUnavailable, rr.Code) + var health HealthResponse + require.NoError(t, json.Unmarshal(rr.Body.Bytes(), &health)) + assert.Equal(t, "unavailable", health.Status) + assert.Nil(t, health.DataFreshness) +} diff --git a/internal/restapi/metadata_handler.go b/internal/restapi/metadata_handler.go index 664482fb4..5841e56e5 100644 --- a/internal/restapi/metadata_handler.go +++ b/internal/restapi/metadata_handler.go @@ -4,6 +4,8 @@ import ( "encoding/json" "net/http" "time" + + "maglev.onebusaway.org/internal/logging" ) // metadataHandler returns system metadata including data freshness indicators. @@ -19,7 +21,16 @@ func (api *RestAPI) metadataHandler(w http.ResponseWriter, r *http.Request) { return } - response := api.dataFreshness(r.Context()) + response, err := api.dataFreshness(r.Context()) + if err != nil { + logging.LogError(logging.ForComponent(r.Context(), "http_server"), "reading data freshness failed", err) + w.WriteHeader(http.StatusServiceUnavailable) + _ = json.NewEncoder(w).Encode(map[string]string{ + "status": "unavailable", + "detail": "reading data freshness failed", + }) + return + } // Ensure the map isn't nil for JSON serialization if response.RealtimeFeeds == nil { diff --git a/internal/restapi/metadata_handler_test.go b/internal/restapi/metadata_handler_test.go index 5db695463..92263ab9e 100644 --- a/internal/restapi/metadata_handler_test.go +++ b/internal/restapi/metadata_handler_test.go @@ -15,6 +15,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "maglev.onebusaway.org/internal/app" + "maglev.onebusaway.org/internal/gtfs" ) func TestMetadataHandler_NilManager(t *testing.T) { @@ -123,3 +124,32 @@ func TestMetadataHandler_NoImportOmitsHash(t *testing.T) { require.NoError(t, json.Unmarshal(rr.Body.Bytes(), &body)) assert.NotContains(t, body, "staticGtfsHash") } + +// newTestManagerWithoutImportMetadata returns a manager whose database is +// reachable but fails import metadata reads, unlike an empty database. +func newTestManagerWithoutImportMetadata(t *testing.T) *gtfs.Manager { + t.Helper() + manager := newTestManagerNoData(t) + _, err := manager.GtfsDB.DB.Exec("DROP TABLE import_metadata") + require.NoError(t, err) + return manager +} + +func TestMetadataHandler_ImportMetadataQueryFails(t *testing.T) { + manager := newTestManagerWithoutImportMetadata(t) + api := &RestAPI{ + Application: &app.Application{ + GtfsManager: manager, + }, + } + + req := httptest.NewRequest(http.MethodGet, "/api/v2/metadata.json", nil) + rr := httptest.NewRecorder() + api.metadataHandler(rr, req) + + assert.Equal(t, http.StatusServiceUnavailable, rr.Code) + var body map[string]any + require.NoError(t, json.Unmarshal(rr.Body.Bytes(), &body)) + assert.Equal(t, "unavailable", body["status"]) + assert.NotContains(t, body, "staticGtfsHash") +}