diff --git a/.gitignore b/.gitignore index 26854d1..6c0ca0a 100644 --- a/.gitignore +++ b/.gitignore @@ -4,6 +4,7 @@ .postman/ postman/ .env +.env.* .DS_Store .worktrees/ docs/superpowers/ diff --git a/api/openapi.yaml b/api/openapi.yaml index 7f887f9..cb17787 100644 --- a/api/openapi.yaml +++ b/api/openapi.yaml @@ -271,15 +271,25 @@ components: items: $ref: "#/components/schemas/SourceReport" entrance: - $ref: "#/components/schemas/EntranceProps" + nullable: true + allOf: + - $ref: "#/components/schemas/EntranceProps" pathways: - $ref: "#/components/schemas/PathwayProps" + nullable: true + allOf: + - $ref: "#/components/schemas/PathwayProps" restroom: - $ref: "#/components/schemas/RestroomProps" + nullable: true + allOf: + - $ref: "#/components/schemas/RestroomProps" parking: - $ref: "#/components/schemas/ParkingProps" + nullable: true + allOf: + - $ref: "#/components/schemas/ParkingProps" elevator: - $ref: "#/components/schemas/ElevatorProps" + nullable: true + allOf: + - $ref: "#/components/schemas/ElevatorProps" updated_at: type: string format: date-time diff --git a/cmd/api/integration_test.go b/cmd/api/integration_test.go index ffb2400..1c163bb 100644 --- a/cmd/api/integration_test.go +++ b/cmd/api/integration_test.go @@ -20,10 +20,12 @@ import ( "time" "github.com/InWheelOrg/inwheel-api/internal/a11y" + apiv1 "github.com/InWheelOrg/inwheel-api/internal/api/v1" "github.com/InWheelOrg/inwheel-api/internal/middleware" "github.com/InWheelOrg/inwheel-api/internal/place" "github.com/InWheelOrg/inwheel-api/internal/testhelpers" "github.com/InWheelOrg/inwheel-api/pkg/models" + "github.com/google/uuid" "golang.org/x/time/rate" "gopkg.in/yaml.v3" "gorm.io/gorm" @@ -188,6 +190,43 @@ func TestHandlePatchAccessibility_PlaceNotFound(t *testing.T) { } } +func TestHandlePatchAccessibility_InvalidPatchReturnsPopulatedFields(t *testing.T) { + t.Cleanup(func() { truncate(t) }) + + place := models.Place{Name: "Test Place", Lat: 52.5, Lng: 13.4, Category: models.CategoryCafe, Rank: models.RankEstablishment, Source: "test"} + testDB.Create(&place) + + srv := newTestServer(t) + id, err := uuid.Parse(place.ID) + if err != nil { + t.Fatalf("parse place id: %v", err) + } + + r := httptest.NewRequest(http.MethodPatch, "/v1/places/"+place.ID+"/accessibility", strings.NewReader(`not json`)) + ctx := context.WithValue(r.Context(), ctxKeyRawBody{}, []byte(`not json`)) + r = r.WithContext(ctx) + ctx = context.WithValue(r.Context(), ctxKeyRequest{}, r) + + resp, err := srv.PatchPlaceAccessibility(ctx, apiv1.PatchPlaceAccessibilityRequestObject{ + Id: id, + Body: &models.AccessibilityProfile{}, + }) + if err != nil { + t.Fatalf("PatchPlaceAccessibility: %v", err) + } + + got, ok := resp.(apiv1.PatchPlaceAccessibility400JSONResponse) + if !ok { + t.Fatalf("response = %T, want PatchPlaceAccessibility400JSONResponse", resp) + } + if got.Fields == nil { + t.Error("Fields is nil, want a populated (non-null) slice per the ValidationError schema") + } + if len(got.Fields) == 0 { + t.Error("Fields is empty, want at least one entry describing the invalid patch") + } +} + func TestHandlePatchAccessibility_CreatePath(t *testing.T) { t.Cleanup(func() { truncate(t) }) @@ -257,6 +296,92 @@ func TestHandlePatchAccessibility_UpdatesExistingProfile(t *testing.T) { } } +func TestHandlePatchAccessibility_PartialSubmissionPreservesOtherComponents(t *testing.T) { + t.Cleanup(func() { truncate(t) }) + + place := models.Place{ + Name: "Test Place", + Lat: 52.5, + Lng: 13.4, + Category: models.CategoryCafe, + Rank: models.RankEstablishment, + Source: "test", + Accessibility: &models.AccessibilityProfile{ + Entrance: &models.EntranceProps{IsLevel: boolPtr(true)}, + Restroom: &models.RestroomProps{IsAccessible: boolPtr(true)}, + Parking: &models.ParkingProps{HasDisabledSpaces: boolPtr(true)}, + }, + } + testDB.Create(&place) + + body, _ := json.Marshal(models.AccessibilityProfile{ + Entrance: &models.EntranceProps{IsLevel: boolPtr(false)}, + }) + + r := httptest.NewRequest(http.MethodPatch, "/v1/places/"+place.ID+"/accessibility", bytes.NewReader(body)) + r.Header.Set("Content-Type", "application/json") + r.SetPathValue("id", place.ID) + w := httptest.NewRecorder() + handlerNoAuth(t, newTestServer(t)).ServeHTTP(w, r) + + if w.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body: %s", w.Code, w.Body.String()) + } + + var profile models.AccessibilityProfile + testDB.Where("place_id = ?", place.ID).First(&profile) + + if profile.Entrance == nil || profile.Entrance.IsLevel == nil || *profile.Entrance.IsLevel { + t.Errorf("Entrance.IsLevel = %v, want false (updated)", profile.Entrance) + } + if profile.Restroom == nil || profile.Restroom.IsAccessible == nil || !*profile.Restroom.IsAccessible { + t.Errorf("Restroom = %v, want preserved (IsAccessible=true)", profile.Restroom) + } + if profile.Parking == nil || profile.Parking.HasDisabledSpaces == nil || !*profile.Parking.HasDisabledSpaces { + t.Errorf("Parking = %v, want preserved (HasDisabledSpaces=true)", profile.Parking) + } +} + +func TestHandlePatchAccessibility_ExplicitNullClearsComponent(t *testing.T) { + t.Cleanup(func() { truncate(t) }) + + place := models.Place{ + Name: "Test Place", + Lat: 52.5, + Lng: 13.4, + Category: models.CategoryCafe, + Rank: models.RankEstablishment, + Source: "test", + Accessibility: &models.AccessibilityProfile{ + Entrance: &models.EntranceProps{IsLevel: boolPtr(true)}, + Restroom: &models.RestroomProps{IsAccessible: boolPtr(true)}, + }, + } + testDB.Create(&place) + + body := []byte(`{"restroom":null}`) + + r := httptest.NewRequest(http.MethodPatch, "/v1/places/"+place.ID+"/accessibility", bytes.NewReader(body)) + r.Header.Set("Content-Type", "application/json") + r.SetPathValue("id", place.ID) + w := httptest.NewRecorder() + handlerNoAuth(t, newTestServer(t)).ServeHTTP(w, r) + + if w.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body: %s", w.Code, w.Body.String()) + } + + var profile models.AccessibilityProfile + testDB.Where("place_id = ?", place.ID).First(&profile) + + if profile.Entrance == nil || profile.Entrance.IsLevel == nil || !*profile.Entrance.IsLevel { + t.Errorf("Entrance = %v, want preserved (IsLevel=true)", profile.Entrance) + } + if profile.Restroom != nil { + t.Errorf("Restroom = %v, want nil (explicitly cleared)", profile.Restroom) + } +} + func TestHandleGetPlace_ReturnsPlaceWithAccessibility(t *testing.T) { t.Cleanup(func() { truncate(t) }) diff --git a/cmd/api/main.go b/cmd/api/main.go index 01380ba..3438e64 100644 --- a/cmd/api/main.go +++ b/cmd/api/main.go @@ -6,9 +6,11 @@ package main import ( + "bytes" "context" "encoding/json" "errors" + "io" "log/slog" "net/http" "os" @@ -36,12 +38,31 @@ import ( ) type ctxKeyRequest struct{} +type ctxKeyRawBody struct{} func requestFromCtx(ctx context.Context) *http.Request { r, _ := ctx.Value(ctxKeyRequest{}).(*http.Request) return r } +func rawBodyFromCtx(ctx context.Context) []byte { + b, _ := ctx.Value(ctxKeyRawBody{}).([]byte) + return b +} + +func captureRawBody(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, err := io.ReadAll(r.Body) + if err != nil { + http.Error(w, "failed to read request body", http.StatusBadRequest) + return + } + _ = r.Body.Close() + r.Body = io.NopCloser(bytes.NewReader(body)) + next.ServeHTTP(w, r.WithContext(context.WithValue(r.Context(), ctxKeyRawBody{}, body))) + }) +} + func injectRequest() apiv1.StrictMiddlewareFunc { return func(f apiv1.StrictHandlerFunc, operationID string) apiv1.StrictHandlerFunc { return func(ctx context.Context, w http.ResponseWriter, r *http.Request, req interface{}) (interface{}, error) { @@ -169,6 +190,7 @@ func buildV1Handler(srv *Server, corsOrigin string) (http.Handler, error) { BaseRouter: v1Mux, ErrorHandlerFunc: srv.validationErrorHandler, Middlewares: []apiv1.MiddlewareFunc{ + captureRawBody, bodySizeLimiter(1 << 20), }, }) @@ -323,23 +345,29 @@ func (s *Server) GetPlace(ctx context.Context, request apiv1.GetPlaceRequestObje func (s *Server) PatchPlaceAccessibility(ctx context.Context, request apiv1.PatchPlaceAccessibilityRequestObject) (apiv1.PatchPlaceAccessibilityResponseObject, error) { id := request.Id.String() - input := *request.Body keyID := middleware.APIKeyIDFromCtx(ctx) - s.engine.WithAuditFlags(&input) - - now := time.Now() - if keyID != "" { - input.SubmittedBy = &keyID - input.UserVerified = true + if errs := validation.AccessibilityProfile(request.Body); len(errs) > 0 { + return apiv1.PatchPlaceAccessibility400JSONResponse(validationError(errs)), nil } - input.SubmittedAt = &now - created, err := s.places.UpsertProfile(ctx, id, &input) + rawPatch := rawBodyFromCtx(requestFromCtx(ctx).Context()) + now := time.Now() + profile, created, err := s.places.UpsertProfile(ctx, id, rawPatch, func(p *models.AccessibilityProfile) { + s.engine.WithAuditFlags(p) + if keyID != "" { + p.SubmittedBy = &keyID + p.UserVerified = true + } + p.SubmittedAt = &now + }) if err != nil { if errors.Is(err, place.ErrPlaceNotFound) { return apiv1.PatchPlaceAccessibility404JSONResponse{Error: "place not found"}, nil } + if errors.Is(err, place.ErrInvalidPatch) { + return apiv1.PatchPlaceAccessibility400JSONResponse(validationError([]validation.FieldError{{Field: "body", Reason: err.Error()}})), nil + } return nil, err } @@ -347,9 +375,9 @@ func (s *Server) PatchPlaceAccessibility(ctx context.Context, request apiv1.Patc if created { auditAction = "create" } - audit.Log(s.db, "accessibility_profiles", input.ID, keyID, auditAction) + audit.Log(s.db, "accessibility_profiles", profile.ID, keyID, auditAction) - return apiv1.PatchPlaceAccessibility200JSONResponse(input), nil + return apiv1.PatchPlaceAccessibility200JSONResponse(profile), nil } func (s *Server) handleHealthz(w http.ResponseWriter, r *http.Request) { diff --git a/cmd/api/main_test.go b/cmd/api/main_test.go index a4e3896..f07d8fd 100644 --- a/cmd/api/main_test.go +++ b/cmd/api/main_test.go @@ -43,7 +43,7 @@ func handlerForServer(t *testing.T, srv *Server) http.Handler { BaseURL: "/v1", BaseRouter: v1Mux, ErrorHandlerFunc: srv.validationErrorHandler, - Middlewares: []apiv1.MiddlewareFunc{bodySizeLimiter(1 << 20)}, + Middlewares: []apiv1.MiddlewareFunc{captureRawBody, bodySizeLimiter(1 << 20)}, }) v1Handler := nethttp_middleware.OapiRequestValidatorWithOptions(swagger, &nethttp_middleware.Options{ @@ -85,7 +85,7 @@ func handlerNoAuth(t *testing.T, srv *Server) http.Handler { BaseURL: "/v1", BaseRouter: v1Mux, ErrorHandlerFunc: srv.validationErrorHandler, - Middlewares: []apiv1.MiddlewareFunc{bodySizeLimiter(1 << 20)}, + Middlewares: []apiv1.MiddlewareFunc{captureRawBody, bodySizeLimiter(1 << 20)}, }) v1Handler := nethttp_middleware.OapiRequestValidatorWithOptions(swagger, &nethttp_middleware.Options{ diff --git a/cmd/ingestion/main_integration_test.go b/cmd/ingestion/main_integration_test.go index 4a16f5f..f9b1bd3 100644 --- a/cmd/ingestion/main_integration_test.go +++ b/cmd/ingestion/main_integration_test.go @@ -280,7 +280,6 @@ func TestRunCanonical_DoesNotOverwriteUserVerified(t *testing.T) { ctx := context.Background() db := testDB - isLevel := false repo := place.NewRepository(db) seed := models.Place{ OSMID: 2001, OSMType: models.OSMNode, Name: "Verified", @@ -292,9 +291,8 @@ func TestRunCanonical_DoesNotOverwriteUserVerified(t *testing.T) { if err := db.Create(&seed).Error; err != nil { t.Fatalf("seed place: %v", err) } - _, err := repo.UpsertProfile(ctx, seed.ID, &models.AccessibilityProfile{ - Entrance: &models.EntranceProps{IsLevel: &isLevel}, - UserVerified: true, + _, _, err := repo.UpsertProfile(ctx, seed.ID, []byte(`{"entrance":{"is_level":false}}`), func(p *models.AccessibilityProfile) { + p.UserVerified = true }) if err != nil { t.Fatalf("seed profile: %v", err) diff --git a/go.mod b/go.mod index 4d0ef1d..85e975b 100644 --- a/go.mod +++ b/go.mod @@ -3,6 +3,7 @@ module github.com/InWheelOrg/inwheel-api go 1.26.4 require ( + github.com/evanphx/json-patch/v5 v5.9.11 github.com/getkin/kin-openapi v0.145.0 github.com/golang-migrate/migrate/v4 v4.19.1 github.com/google/uuid v1.6.0 diff --git a/go.sum b/go.sum index 6dfe6d1..176a2ff 100644 --- a/go.sum +++ b/go.sum @@ -45,6 +45,8 @@ github.com/docker/go-units v0.5.0 h1:69rxXcBk27SvSaaxTtLh/8llcHD8vYHT7WSdRZ/jvr4 github.com/docker/go-units v0.5.0/go.mod h1:fgPhTUdO+D/Jk86RDLlptpiXQzgHJF7gydDDbaIK4Dk= github.com/ebitengine/purego v0.10.0 h1:QIw4xfpWT6GWTzaW5XEKy3HXoqrJGx1ijYHzTF0/ISU= github.com/ebitengine/purego v0.10.0/go.mod h1:iIjxzd6CiRiOG0UyXP+V1+jWqUXVjPKLAI0mRfJZTmQ= +github.com/evanphx/json-patch/v5 v5.9.11 h1:/8HVnzMq13/3x9TPvjG08wUGqBTmZBsCWzjTM0wiaDU= +github.com/evanphx/json-patch/v5 v5.9.11/go.mod h1:3j+LviiESTElxA4p3EMKAB9HXj3/XEtnUf6OZxqIQTM= github.com/felixge/httpsnoop v1.0.4 h1:NFTV2Zj1bL4mc9sqWACXbQFVBBg2W3GPvqp8/ESS2Wg= github.com/felixge/httpsnoop v1.0.4/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U= github.com/getkin/kin-openapi v0.145.0 h1:htBX+Q7SevVaCUqymFegUKzH2WCbewl9tsmyn2FMGWY= diff --git a/internal/api/v1/server.gen.go b/internal/api/v1/server.gen.go index d01c94f..21875c9 100644 --- a/internal/api/v1/server.gen.go +++ b/internal/api/v1/server.gen.go @@ -1266,66 +1266,67 @@ var swaggerSpec = []string{ "GJZlIh1IMcsdGOaEmkGmDTAokNnKYIHKQSZQ8hhsNS2Ec8iBC4OpkwvakqMZKZcz1WzhCUxmWvMJMMVh", "IkUhHPIJTJniNvYPXY4wUXoCaeV0lsXADI4Ux0wo5FCiGXiSwGZMKOvg+uIcjodD0AbO3577I7hOK2IO", "OWgFLmc1m8lIRXGEqiqis48R8RHFUc1DFEdKR5/iyC1KjM4i64xQs+g+7grr0uhMSCRxlUaXaJxAL1KU", - "OGdOG/r8rwaz6Cz6l72VAvZq6e/9WK+7NLq0dDwqZ5hK8dGN9brlRsFpS6ZNwVx0FlWVoEsYZPy9kovo", - "zJkKe65TMnNDHx8hdxmWLamVzOW3bGEf3+fXrfZJluL47+TVoHVG6+Ixolf1uiVVqyuT4thgqU2wfuGw", - "eJT5a7/tyu+iY2qGmDFs4Y9trHxM12hdiDOHAycKfMqtqpKW/84zLJrxHI3IBHrZbtkx1VoiU9H96jZ6", - "+jdMXRRHd4OZHtQPC81R2qTX1lsrB6Lwsjn7EilWrDZGwUCis2gmXF5Nk1QXexfqP3JE+d7M9oS6pY8D", - "Voq98ma2V+8irn5gDmfaLDYj0PdGMw7egICIQ2WR+xCUCemQROHdXaRagUWJKW1MRmrS+FQdZpwWEt0E", - "OCrtEKxjijOpFQJr33dgvBMrN1IZMlcZtHUQAx/DskpKmKOq0P4ZJpp+mICwPmIJNUNL5CFjUk5ZejNS", - "xOm0EpILNbMhDHGt/o2CkaMoqg2CLTEVmUghrcXQjVEFkzKKIyaMF3scOcOEGlvHvIyCh7DKMEU/piwj", - "bdlcl7TU3zpaeXwr1sSRZ7833r3VTXTaCHJh7cM+RNs/0Lr7OLoVnIziS8SkfJ9FZx8f3tqTlu4/xWtW", - "8YNEZkCXqEj/nkICFMthVA2Hh/hmmJwOixjquA7D5NVwUD9TOixK6WEBL+rc8TJ5qn+sZLNbp1gKkTJL", - "bQuscrpgTqRRHBVMVUx6/c+1nAftKq2wV6PdnLOhVVZx4caZZLNupOwJx70xZhkeOZbPr242FQr8yV01", - "7yeHbTXvJ/vDQf1sqWZ6WMCL87fnpOI44lqb8S6Msmt/r7v2d0r293rN/k47jOXMjkkNuiX3ZegOP08N", - "EzIgj80FO3E0L/keB9sn52lL/pgk/2pN8setCz7NubpmulsH66KpHbqEfhwSroJKrepM3CEfG1aU281B", - "KIcmDdCof4XBQs/ZVOIDJwk7lg383/zVSl3iuESTonLPbF1XrCjBE2gbV/rm5E8ryzoZnCaHh39amRX6", - "7y2/+X+XYLp2uWMfMEabK7SlVravoqGfe4zeW/3nShgCnh/rZZ/Wb0fn3zk0iskrUsj66alWmeBYFz4r", - "EKyrqYecBbsTBSW+/TgqhAqfh0sqqiqmaFZl0IZjFsyl+cMg++F7+QqlxeWnp+mvdefdau8vVNH+2Oio", - "K1xf7W6LV5as/DGthhOW6/vU+/76XQNRug70vkR17Qyie8dKQBkaBR7Bu1zYGtHfMlujZgL1RhdJC/cq", - "zUlDt2zheZAB7vZhm06durs4nuoqhMCVXQ6Hw2GvbVJongXj5IKKjRTHTo/bhf5OoEf65uC4FZcOjgfH", - "7ZiETUZuig609Ft9oT+DQkbVBLA5E5LyBmRiVplQdq1gCkcuUl+/WjFTbIbbMxAXls7hY1uytF1B7Bq6", - "1EYBnm5fCD9Ijtoh/CAZDgf1s2UIp4dfC2E61rhb/+80WnZn+KRH67C0WwHEDZqpdVrhODO4xRpsZTL2", - "eI/rOizbZfn478wYfUt2XjJr2Qz7M/zXQvinmkdLZzs2D1nLG+9YUYa6odPtCKVlKyS1LebjpzY2DJZR", - "ayP0TYlEuuzcbLQlKClHhxk7Pc5OjgbHr/ZfDY6OTw4G08MsHRykr08Os5MTlrGTKI4kJeajk+To5PAg", - "jqSaRWcnyenRwUHcCOMnZhA4wl9xjpQQtC18R3H/4PDo+OTV6evwqIEGIXcYpm48eiCuKks2nTox94a1", - "5ivrYnmykTUdso4wHt6/bHfRHoO/uxGINeAYCx4uw7kgJTB52bnkg63lFmi578n0f2f3Vq7fawPbvW4n", - "0MHrPngnQ6v6IYB42jnFf904JliST9+/oJqRIR8cn/h9zff9nis0ltZiQCh3chQ9lvhX9viw6BsQFRrz", - "qNyWVvlm3PbmvQ693gpbSraA0ghthFv4bmnBSkilILoJ7MMbkEzxgpkbeGFzfauAObjVRvKR+k3rIgZM", - "ZgnUPUcbQ65tKRyT9mUMB/AG0Do2lcLmHte9CO1Uw0cq1UWBJhVM7pXVVIp02f58GcMhvIG6rQovCqG0", - "gcp5LwoER6reE/qWNobzD+8s/M9//TcELrWSC2I1F7MciNGXnV7pfnwQH37q00UYBqxp/+SoR6hNqHhk", - "xEGh9TosfbahgKsz9Tb/3dyw7qW/f6ywVgN4nwluHPywFeWeVg2FJPQNEt1ljUO7gZ0zx548+wm89gx9", - "FN65cVoZGyqt9VqHfa4Qws/e21yOQFugZDNM4HxqyUtuc1Sh+qkHBpLZZskVDiwqDkzKerIBJTOssFCp", - "NGdqFuaYyNIcbDW1+LmiI0lVaF1wgoc16QXRV8O1TXnjbr+IDNNFKv3ExCHoDJiC99fvBsGj6tlMAhOK", - "dr7dhHwCBTLl7zhSU5Z6HP7++t2yDqS6j6NEAlZVaZ1BVsC0cl4qoTQ0mGrDQdiRusHSgdNQGrRo5ghM", - "LaggMYPVxLmTv4Hu2h2i1Hk/jlKprZ/0tvjtLSqvcCasQ3MVRNzTGymYkB03C08eU0RY1aeJFclt/RhW", - "ivEN9szJrtgtnF9ewA0u6lCJd8yP4LUi9Vw7bRCEA4tpZVAukr6E8gQwsgk+GjE8fOuG83gppRaxfmG0", - "57m77c6O/4F6iKEc8zFBqNmY0vAD5TcWaGao0sW4rKTcvm5m2HRsmJD2gWaylnJMED/Xt2i21oKNK25j", - "K+T2sUXmxjmKWe520Q0ZJkeHg2FydNqR8BFJ+OiwAG3o2+lgmDT9EV05KziGGe2qu+wqQ/obG8ZFCJA7", - "GhntB05WE5WD4WA/OV6bqBx8db3Z9aPd5uHOuxMbLhtC+leGlxVwWxaxFLr7ls6ZrNZWLtA+Go9rCs3+", - "uMNnX2Rqdyjas1lb5kyGYfucTMY3RKzvIE+nEv3XKI5mhpE1+MZyatD5Jmd4DyllpvTjeick2t609Fcm", - "Bfcl9pa+77Jlv5LCfLkHMiYk9hYTvtv79NdkWq3nDbzUOx1YUtgUKamZ0pJwi2s6PlzkvBQ/4+K8CoFZ", - "kL/kyDjSUbXB/ufg/PJi8LNPKw0Lfld0T4cKlenNVHkZSoyrH68/+JTZALbaxNegRCmZI0tNRupcSo8s", - "ABUvtVDOAjMIczRWaEVQRnE0sDff30tG6tLoKbaWvtjLkUmX/xbDHuWkxW8v/fZKLQ+oYZxwXmsNP+eX", - "F2ScYVF0Fu0nw2To68sSFStFdBYdJsPksPZbL7s9Vrl87wYXNaQjqNUDGnCubzBgUQIO/t0ap4E2o3K+", - "xRsAa4MzI0/VeFu64MsjggZMDVs8zYPh0SbBn3EBxu/gdIGj4X49BnLNcLEsJZEVWu39rR5SBIt7tH/R", - "GWR59XdpvxPWUiLWBoTyHtEApo4B+sDeNr2Pnyhu26oomG9zhRt7mbXl1D0tyN/UeM47qQ4Ycg0jeAxk", - "CVM36K0xx5mYowIPloBxbtDaZKQ+5AiG3fqVXi+Un5B30F6olxvAJ4oCuWCOMN9IXVGsbXKM03AIgcmg", - "UgslGri49H9yXZlgkesqr28VvByt+17zxbMpch1333fDCYG4+w1b298B+e2m1KiqRrDBlIfPxsJ6iO/h", - "4KK24IClPf3X386Vzn0x5UXApI9lgHfCOlsbr7Atxg6+IWNk3QFBAd6liBxDl7rtvUG9/qVAgykKX092", - "nddXoN6yZuj6wiY5nQXma3cqicMGINRgkMN0AS9WnRh4++P1DzFQvLn+4WUyUtdV6btq4HKDCISfLLwo", - "KlcxKUmUqaysmOPLs5ECGMB3310afUe3Wnz33RlMpJpNYphI5uhPAKYT7/RMyvrQxmHA6RmGd7DDUd/r", - "SnGKg1N9508rhBrXJ/qP4dSC3S2f0kfmVhQyXZntBN5ixirp6OxCW+eFrCg01RKphTVSk88TeJEyiwOh", - "LCorvFHZahowCfjpPWgFE8r2k5fgLcKihVvhct8E0JkPlW05sqmeIwW6uqEieIjQ620VrQDnaBbtbopv", - "xfREvF+EdZfBJnyLlhXo0FifKjwu+VyhWaxgibfAKG5ZNQ9Cic4OhnFnfNzqIe9v9i3v434CdSeqTWED", - "4/bv/NzZ9FWdcDpyiSmb6Uzv9X2bcEXl9/XtO2SXI6Jeusw9le6jQ4cO1WPSUx/FuihsE11677vm+NB2", - "eICZ461vEbS56aNf++4zS3srpWeWby+dEHe+xY1CWHu+G33aQCXPBwlWDe6erHfJZkL50CqFdat89Afi", - "Ei9raIXKbhamgNokTR/MQ0AbTBlVIGW4D514Hz8GnEHhbdODPt9oAkPBFjBFECqVFUcOQkmhcLOOCec1", - "s4pd4Np6tvBt0WyL6JrN+Cb7HwdgV0sAmybCP2Q5GAyHsGAt6iV+3Psi+P1TQGR4FY7cQDgLmGUY8PVa", - "IyK8Z5CM1F+06TgP4VCDysWwEoRvLQiVoxHNu3VhuOJXhv9LIQS13NG8m4eQ5kJy4Nq/GuYg/NcdsYYy", - "S0bqYnloi1rKjFnARNjxkuYZkHHX/31T/y+W4JMGswVG+tDWT+gaP+zDWr43uYzi9VsIbX/qDej9I/Td", - "x+ztvpcREP8DQ/Svv168hVo+noujb+d8QQJkXbUUOtnhJ3TAgLxT4ha32tt4X6ekamHTzX4tLYZSa4s/", - "rfVbQiYZqfOKCwd+vORdiW5ZkdH7CaQZ+FGB96rVDLLuwy7AN5Jtn21fEpf+9p1c9c1M/fnTWv+7UE/J", - "csNvwMN636LPAOq69J9JsJsE/y+EhKcm4V+9CukaaZ2OVb+3B8rBh4OjVUZGZ9HefD+6/3T/vwEAAP//", + "OGdOG/rMpHyfRWcfv0T/ajCLzqJ/2VupYq/Ww96P9Y5Lo0sb3X+KI1VJyaZ0ujMV3hOrzjCV4lccWu94", + "4FDB6bhMm4K56CyqKkGXN8j4eyUXYVmPGEpmbujjk1m5DBse4KRkLr9lC/s1Z/odD50pWYrjv/OOBq0z", + "WhdPZ+iq3vEAR1ZXJsWxwVKb4InCYeE/PHTytd925XcRbzWzzBi2oO9LjxvTFVuX5czhwIkCn3LjqqTl", + "v/MMi2Y8RyMygV7uW3ZMtZbIVHS/uo2e/g1TF8XR3WCmB/XDQnOUNun1u9bKgSi8bM6+RIoVq41RMKzo", + "LJoJl1fTJNXF3oX6jxxRvjezPaFu6eOAlWKvvJnt1buIqx+Yw5k2i81o+L3RjIM3LiDiUFnkPhxmQjok", + "UfjQI1KtwKLElDYmIzVpfLgOeU4LiW4CHJV2CNYxxZnUCoG17zswPqAoN1IZMlcZtHVABR9Ps0pKmKOq", + "0P4ZJpp+mICwPnoKNUNL5CFjUk5ZejNSxOm0EpILNbMhJHKt/o0Co6OIrg2CLTEVmUghrcXQjZcFkzKK", + "IyaMF3scOcOEGlvHvIyC97DKMEU/piwjbdlcl7TU3zpaRZFWbIsjz35v7H2rm/i4EXDD2od9iLZ/oHX3", + "cXQrOBnFUx27J0WSd3et4geJzIAuUZH+PYUEKK/AqBoOD/HNMDkdFjHUOQaGyavhoH6mdFiU0sMCXtR5", + "7GXyVP9YyWa3TrEUImW52hZY5XTBnEijOCqYqpj0+p9rOQ/aVVphr0a7WW9Dq6ziwo0zyWbdSNkTqntj", + "zDI8ciyfX91sKhT4k7tq3k8O22reT/aHg/rZUs30sIAX52/PScVxxLU2410YZdf+Xnft75Ts7/Wa/Z12", + "GMuZHZMadEvuy9Adfp4aJmRAQZsLduJoXvI9DrZPztOW/DFJ/tWa5I9bF3yac3XNdLcO1sVsO3QJHeDp", + "YxGzxjJB1Zm4Qz42rCi3m4NQDk0aYFP/CoOFnhMqeuAkYceyKUU2f7VSlzgu0aSo3DNb1xUrSvAE2saV", + "vjn508qyTganyeHhn1Zmhf57y2/+3yWYrl3u2AeM0eYKbamV7auu6Oceo/dW/7kShoDnx3rZp/Xb0fl3", + "Do1i8ooUsn56qlUmONaF1goE62rqIWfB7kRBiW8/jgqhwufhkoqqiimaaFlabThmwVyaPwyyH76Xr15a", + "XH56mv5ad96t9v5C1fWPjY66wvWV97Z4ZcnKH9NqOGG5vk+976/fNRCl60DvS1TXziC6d6wElKFp4RG8", + "y4WtEf0tszVqJlBvdJG0cK/SnDR0yxaeBxngbh+26VS8u4vjqa5CCFzZ5XA4HPbaJoXmWTBOLqjYSHHs", + "9PjrGwtfCT3SNwfHrbh0cDw4bsckbDJyU3Sgpd/qC/0ZFDKqJoDNmfDVNGRiVplQdq1gCkcuUl+/WjFT", + "bIbbMxAXls7hY1uytF1B7Bq61EYBnm5fCD9Ijtoh/CAZDgf1s2UIp4dfC2E61rhb/++0ZXZn+KRH67C0", + "WwHEDZqpdVrhODO4xRpsZTKWPlo7Xodluywf/50Zo2/JzktmLZthf4b/Wgj/VPNo6WzH5iFreeMdK8pQ", + "N3S6HaG0bIWktsV8/NTGhsEyam2EHi6RSJedm422BCXl6DBjp8fZydHg+NX+q8HR8cnBYHqYpYOD9PXJ", + "YXZywjJ2EsWRpMR8dJIcnRwexJFUs+jsJDk9OjiIG2H8xAwCR/grzpESgraF7zbuHxweHZ+8On0dHjXQ", + "IOQOw9SNRw/EVWXJplMn5t6w1nxlXSxPNrKmQ9YRxsP7l+0u2mPwdzcCsQYcY8HDZTgXpAQmLzuXfLCB", + "3QIt9z2Z/u/s7Mr1e21gu9ftBDp43QfvZGh/PwQQTzun+K8bxwRL8un7F1QzMuSD4xO/r/m+33OFxtJa", + "DAjlTo6ixxL/yh4fFn0DokKzH5Xb0kbfjNvevNeh11thS8kWUBqhjXAL3y0tWAmpFEQ3gX14A5IpXjBz", + "Ay9srm8VMAe32kg+Ur9pXcSAySyBuudoY8i1LYVj0r6M4QDeAFrHplLY3OO6F6GdavhIpboo0KSCyb2y", + "mkqRLtufL2M4hDdQt1XhRSGUNlA570WB4EjVe0Lf0sZw/uGdhf/5r/+GwKVWckGs5mKWAzH6stMr3Y8P", + "4sNPfboIw4A17Z8c9Qi1CRWPDEQotF6Hpc82FHB1pt7mv5sb1r30948V1moA7zPBjYMftqLc06qhkIS+", + "QaK7rHFoN7Bz5tiTZz+B156hj8I7N04rY0OltV7rsM8VQvjZe5vLEWgLlGyGCZxPLXnJbY4qVD/1wEAy", + "2yy5woFFxYFJWU82oGSGFRYqleZMzcJMFVmag62mFj9XdCSpCq0LTvCwJr0g+mq4tilv3O0XkWG6SKWf", + "mDgEnQFT8P763SB4VD2bSWBC0c63m5BPoECm/B1HaspSj8PfX79b1oFU93GUSMCqKq0zyAqYVs5LJZSG", + "BlNtOAg7UjdYOnAaSoMWzRyBqQUVJGawmn538jfQXbtDlDrvx1EqtfVT5xa/vUXlFc6EdWiugoh7eiMF", + "E7LjZuHJY4oIq/o0sSK5rR/DSjG+wZ452RW7hfPLC7jBRR0q8Y751wG0IvVcO20QhAOLaWVQLpK+hPIE", + "MLIJPhoxPHzrhvN4KaUWsX5htCe8u+3Ojv+BeoihHPMxQajZ2IWZ97byGws0M1TpYlxWUm5fNzNsOjZM", + "SPtAM1lLOSaIn+tbNFtrwcYVt7EVcvvYInPjHMUsd7vohgyTo8PBMDk67Uj4iCR8dFiANvTtdDBMmv6I", + "rpwVHMOMdtVddpUh/Y0N46KyOxwZ7QdOVhOVg+FgPzlem6gcfHW92fWj3ebhzrsTGy4bQvpXhpcVcFsW", + "sRS6+5bOmazWVi7QPhqPawrN/rjDZ19kanco2rNZW+ZMhmH7nEzGN0Ss7yBPpxL91yiOZoaRNfjGcmrQ", + "+SZneCcqZab043onJNretPRXJgX3JfaWvu+yZb+Swny5BzImJPYWE77b+/TXZFqt5w281DsdWFLYFCmp", + "mdKScItrOj5c5LwUP+PivAqBWZC/5Mg40lG1wf7n4PzyYvCzTysNC35XdE+HCpXpzVR5GUqMqx+vP/iU", + "2QC22sTXoEQpmSNLTUbqXEqPLAAVL7VQzgIzCHM0VmhFUEZxNLA3399LRurS6Cm2lr7Yy5FJl/8Wwx7l", + "pMVvL/32Si0PqGGccF5rDT/nlxdknGFRdBbtJ8Nk6OvLEhUrRXQWHSbD5LD2Wy+7PVa5fO8GFzWkI6jV", + "Axpwrm8wYFECDv7dGqeBNqNyvsUbAGuDMyNP1XhbuuDLI4IGTA1bPM2D4dEmwZ9xAcbv4HSBo+F+PQZy", + "zXCxLCWRFVrt/a0eUgSLe7R/0RlkefV3ab8T1lIi1gaE8h7RAKaOAfrA3ja9j58obtuqKJhvc4Ube5m1", + "5dQ9Lcjf1HjOO6kOGHINI3gMZAlTN+itMceZmKMCD5aAcW7Q2mSkPuQIht36lV4vlJ+Qd9BeqJcbwCeK", + "ArlgjjDfSF1RrG1yjNNwCIHJoFILJRq4uPR/cl2ZYJHrKq9vFbwcrfte88WzKXIdd993w0n9ct+are3v", + "gPx2U2pUVSPYYMrDZ2NhPcT3cHBRW3DA0p7+62/nSue+mPIiYNLHMsA7YZ2tjVfYFmMH35Axsu6AoADv", + "UkSOoUvd9t6gXv9SoMEUha8nu87rK1BvWTN0fWGTnM4C87U7lcRhAxBqMMhhuoAXq04MvP3x+ocYKN5c", + "//AyGanrqvRdNXC5QQTCTxZeFJWrmJQkylRWVszx5dlIAQzgu+8ujb6jWy2+++4MJlLNJjFMJHP0JwDT", + "iXd6JmV9aOMw4PQMw/vg4ajvdaU4xcGpvvOnFUKN6xP9x3Bqwe6WT+kjcysKma7MdgJvMWOVdHR2oa3z", + "QlYUmmqJ1MIaqcnnCbxImcWBUBaVFd6obDUNmAT89B60ggll+8lL8BZh0cKtcLlvAujMh8q2HNlUz5EC", + "Xd1QETxE6PW2ilaAczSLdjfFt2J6It4vwrrLYBO+RcsKdGisTxUel3yu0CxWsMRbYBS3rJoHoURnB8O4", + "Mz5u9ZD3N/uW93E/gboT1aawgXH7d37ubPqqTjgducSUzXSm9/q+Tbii8vv69h2yyxFRL13mnkr30aFD", + "h+ox6amPYl0Utokuvfddc3xoOzzAzPHWtwja3PTRr333maW9ldIzy7eXTog73+JGIaw9340+baCS54ME", + "qwZ3T9a7ZDOhfGiVwrpVPvoDcYmXNbRCZTcLU0BtkqYP5iGgDaaMKpAy3IdOvI8fA86g8LbpQZ9vNIGh", + "YAuYIgiVyoojB6GkULhZx4TzmlnFLnBtPVv4tmi2RXTNZnyT/Y8DsKslgE0T4R+yHAyGQ1iwFvUSP+59", + "Efz+KSAyvApHbiCcBcwyDPh6rRER3jNIRuov2nSch3CoQeViWAnCtxaEytGI5t26MFzxK8P/pRCCWu5o", + "3s1DSHMhOXDtXw1zEP4DkFhDmSUjdbE8tEUtZcYsYCLseEnzDMi46/++qf8XS/BJg9kCI31o6yd0jR/2", + "YS3fm1xG8fothLY/9Qb0/hH67mP2dt/LCIj/gSH6118v3kItH8/F0bdzviABsq5aCp3s8BM6YEDeKXGL", + "W+1tvK9TUrWw6Wa/lhZDqbXFn9b6LSGTjNR5xYUDP17yrkS3rMjo/QTSDPyowHvVagZZ92EX4BvJts+2", + "L4lLf/tOrvpmpv78aa3/XainZLnhN+BhvW/RZwB1XfrPJNhNgv8XQsJTk/CvXoV0jbROx6rf2wPl4MPB", + "0Sojo7Nob74f3X+6/98AAAD//w==", } // decodeSpec returns the embedded OpenAPI spec as raw JSON bytes, diff --git a/internal/place/README.md b/internal/place/README.md index b4560b7..577c5a9 100644 --- a/internal/place/README.md +++ b/internal/place/README.md @@ -31,8 +31,8 @@ flowchart LR | `UpsertBatch(ctx, places)` | the ingestion batcher | Bulk insert/update on `(osm_id, osm_type)` conflict. Uses `RETURNING id` so GORM back-populates the `ID` field on every place in the slice — the batcher harvests these as `touchedIDs` for the retry sweep. | | `AttachExternalRef(ctx, placeID, source, ref)` | `identity.Resolver`, `identity.Sweeper` | Adds an `ExternalRef` to the place's `external_ids` JSONB map under the given source key, via Postgres `jsonb_set`. Concurrent attaches to different sources on the same place don't clobber each other. | | `FindCandidates(ctx, lat, lng, radiusM, categories)` | `identity.Match` | Active places within `radiusM` of the point whose category is in `categories`. Uses `ST_DWithin` over a `geography(ST_Point(lng, lat))` expression. Backed by a PostGIS GIST index. | -| `UpsertProfile(ctx, placeID, profile)` | `cmd/api` (`PatchPlaceAccessibility`) | Create-or-update the accessibility profile for a place. Always overwrites — user-driven write path. Returns `created=true` when a new row was inserted. | -| `UpsertProfileIngestion(ctx, placeID, profile)` | ingestion batcher | Same as `UpsertProfile` but skips the write when `user_verified=true`, preserving human corrections across automated re-ingests. | +| `UpsertProfile(ctx, placeID, rawPatch, prepare)` | `cmd/api` (`PatchPlaceAccessibility`) | Create-or-update the accessibility profile for a place by applying `rawPatch` as an RFC 7396 JSON Merge Patch on top of the current row — omitted components are left untouched, explicit `null` clears them. Reads the current row `FOR UPDATE` and merges/writes inside one transaction, so concurrent PATCHes for the same place serialize instead of one clobbering the other. `prepare` runs on the merged profile before persisting (audit flags, `submitted_by`/`submitted_at`). Returns `created=true` when a new row was inserted. | +| `UpsertProfileIngestion(ctx, placeID, profile)` | ingestion batcher | Full replace with the given profile (no merge), skipping the write when `user_verified=true` to preserve human corrections across automated re-ingests. | ## Compile-time contracts diff --git a/internal/place/repository.go b/internal/place/repository.go index d276bd3..a38ccfb 100644 --- a/internal/place/repository.go +++ b/internal/place/repository.go @@ -12,6 +12,8 @@ import ( "fmt" "time" + jsonpatch "github.com/evanphx/json-patch/v5" + "github.com/jackc/pgx/v5/pgconn" "gorm.io/gorm" "gorm.io/gorm/clause" @@ -20,6 +22,14 @@ import ( ) var ErrPlaceNotFound = errors.New("place not found") +var ErrInvalidPatch = errors.New("invalid merge patch") + +const pgUniqueViolation = "23505" + +func isUniqueViolation(err error) bool { + var pgErr *pgconn.PgError + return errors.As(err, &pgErr) && pgErr.Code == pgUniqueViolation +} type Repository struct { db *gorm.DB @@ -112,12 +122,19 @@ func (r *Repository) AttachExternalRef( return nil } -// UpsertProfile creates or replaces the accessibility profile. Always overwrites. -// Returns created=true when a new row was inserted, false on update. -func (r *Repository) UpsertProfile(ctx context.Context, placeID string, profile *models.AccessibilityProfile) (created bool, err error) { - if profile == nil { - return false, fmt.Errorf("upsert profile: nil profile") +type PreparePatch func(*models.AccessibilityProfile) + +func (r *Repository) UpsertProfile(ctx context.Context, placeID string, rawPatch []byte, prepare PreparePatch) (result models.AccessibilityProfile, created bool, err error) { + for attempt := 0; attempt < 2; attempt++ { + result, created, err = r.upsertProfileAttempt(ctx, placeID, rawPatch, prepare) + if err == nil || !isUniqueViolation(err) { + return result, created, err + } } + return result, created, err +} + +func (r *Repository) upsertProfileAttempt(ctx context.Context, placeID string, rawPatch []byte, prepare PreparePatch) (result models.AccessibilityProfile, created bool, err error) { now := time.Now() err = r.db.WithContext(ctx).Transaction(func(tx *gorm.DB) error { if err := tx.First(&models.Place{}, "id = ?", placeID).Error; err != nil { @@ -126,36 +143,65 @@ func (r *Repository) UpsertProfile(ctx context.Context, placeID string, profile } return fmt.Errorf("upsert profile: check place: %w", err) } + var existing models.AccessibilityProfile - loadErr := tx.Where("place_id = ?", placeID).First(&existing).Error + loadErr := tx.Clauses(clause.Locking{Strength: "UPDATE"}).Where("place_id = ?", placeID).First(&existing).Error if loadErr != nil && !errors.Is(loadErr, gorm.ErrRecordNotFound) { return fmt.Errorf("upsert profile: load existing: %w", loadErr) } - if errors.Is(loadErr, gorm.ErrRecordNotFound) { - profile.PlaceID = placeID - profile.UpdatedAt = now + exists := !errors.Is(loadErr, gorm.ErrRecordNotFound) + + existingJSON := []byte("{}") + if exists { + b, marshalErr := json.Marshal(existing) + if marshalErr != nil { + return fmt.Errorf("upsert profile: marshal existing: %w", marshalErr) + } + existingJSON = b + } + mergedJSON, mergeErr := jsonpatch.MergePatch(existingJSON, rawPatch) + if mergeErr != nil { + return fmt.Errorf("%w: %v", ErrInvalidPatch, mergeErr) + } + var merged models.AccessibilityProfile + if err := json.Unmarshal(mergedJSON, &merged); err != nil { + return fmt.Errorf("upsert profile: unmarshal merged: %w", err) + } + + if prepare != nil { + prepare(&merged) + } + merged.UpdatedAt = now + + if !exists { + merged.PlaceID = placeID created = true - return tx.Create(profile).Error + if err := tx.Create(&merged).Error; err != nil { + return err + } + result = merged + return nil } + updates := map[string]any{ - "source_reports": profile.SourceReports, - "entrance": profile.Entrance, - "pathways": profile.Pathways, - "restroom": profile.Restroom, - "parking": profile.Parking, - "elevator": profile.Elevator, + "source_reports": merged.SourceReports, + "entrance": merged.Entrance, + "pathways": merged.Pathways, + "restroom": merged.Restroom, + "parking": merged.Parking, + "elevator": merged.Elevator, "updated_at": now, - "submitted_by": profile.SubmittedBy, - "submitted_at": profile.SubmittedAt, - "user_verified": profile.UserVerified, + "submitted_by": merged.SubmittedBy, + "submitted_at": merged.SubmittedAt, + "user_verified": merged.UserVerified, } - if err := tx.Model(&existing).Clauses(clause.Returning{}).Updates(updates).Error; err != nil { + if err := tx.Model(&existing).Updates(updates).Error; err != nil { return err } - *profile = existing + result = merged return nil }) - return created, err + return result, created, err } // UpsertProfileIngestion creates or updates the accessibility profile but skips diff --git a/internal/place/repository_integration_test.go b/internal/place/repository_integration_test.go index 13b1b70..974f851 100644 --- a/internal/place/repository_integration_test.go +++ b/internal/place/repository_integration_test.go @@ -13,6 +13,7 @@ import ( "log" "os" "strings" + "sync" "testing" "time" @@ -249,21 +250,14 @@ func TestRepository_UpsertProfile_CreatesWhenAbsent(t *testing.T) { repo := place.NewRepository(gormDB) placeID := mustCreatePlace(ctx, t, gormDB, 1001, "Profile Test Place") - profile := &models.AccessibilityProfile{ - Entrance: &models.EntranceProps{IsLevel: boolPtr(true)}, - } - created, err := repo.UpsertProfile(ctx, placeID, profile) + patch := []byte(`{"entrance":{"is_level":true}}`) + got, created, err := repo.UpsertProfile(ctx, placeID, patch, nil) if err != nil { t.Fatalf("UpsertProfile: %v", err) } if !created { t.Errorf("created = false, want true on first insert") } - - var got models.AccessibilityProfile - if err := gormDB.Where("place_id = ?", placeID).First(&got).Error; err != nil { - t.Fatalf("load profile: %v", err) - } if got.Entrance == nil || got.Entrance.IsLevel == nil || !*got.Entrance.IsLevel { t.Errorf("Entrance.IsLevel = %v, want true", got.Entrance) } @@ -280,28 +274,19 @@ func TestRepository_UpsertProfile_UpdatesWhenPresent(t *testing.T) { repo := place.NewRepository(gormDB) placeID := mustCreatePlace(ctx, t, gormDB, 1002, "Profile Update Place") - first := &models.AccessibilityProfile{Entrance: &models.EntranceProps{IsLevel: boolPtr(false)}} - created, err := repo.UpsertProfile(ctx, placeID, first) - if err != nil { + if _, created, err := repo.UpsertProfile(ctx, placeID, []byte(`{"entrance":{"is_level":false}}`), nil); err != nil { t.Fatalf("first UpsertProfile: %v", err) - } - if !created { + } else if !created { t.Errorf("created = false, want true on first insert") } - second := &models.AccessibilityProfile{Entrance: &models.EntranceProps{IsLevel: boolPtr(true)}} - created, err = repo.UpsertProfile(ctx, placeID, second) + got, created, err := repo.UpsertProfile(ctx, placeID, []byte(`{"entrance":{"is_level":true}}`), nil) if err != nil { t.Fatalf("second UpsertProfile: %v", err) } if created { t.Errorf("created = true, want false on update") } - - var got models.AccessibilityProfile - if err := gormDB.Where("place_id = ?", placeID).First(&got).Error; err != nil { - t.Fatalf("load profile: %v", err) - } if got.Entrance == nil || got.Entrance.IsLevel == nil || !*got.Entrance.IsLevel { t.Errorf("Entrance.IsLevel = %v, want true (updated)", got.Entrance) } @@ -312,32 +297,136 @@ func TestRepository_UpsertProfile_UpdatesWhenPresent(t *testing.T) { } } -func TestRepository_UpsertProfile_OverwritesUserVerified(t *testing.T) { +func TestRepository_UpsertProfile_PrepareCallbackAppliesBeforePersist(t *testing.T) { t.Cleanup(func() { truncate(t) }) ctx := context.Background() gormDB := testDB repo := place.NewRepository(gormDB) - placeID := mustCreatePlace(ctx, t, gormDB, 1003, "User Verified Overwrite Place") + placeID := mustCreatePlace(ctx, t, gormDB, 1003, "Prepare Callback Place") - if _, err := repo.UpsertProfile(ctx, placeID, &models.AccessibilityProfile{Entrance: &models.EntranceProps{IsLevel: boolPtr(true)}, UserVerified: true}); err != nil { + if _, _, err := repo.UpsertProfile(ctx, placeID, []byte(`{"entrance":{"is_level":true}}`), func(p *models.AccessibilityProfile) { + p.UserVerified = true + }); err != nil { t.Fatalf("seed: %v", err) } - override := &models.AccessibilityProfile{ - Entrance: &models.EntranceProps{IsLevel: boolPtr(false)}, - UserVerified: false, - } - if _, err := repo.UpsertProfile(ctx, placeID, override); err != nil { + got, _, err := repo.UpsertProfile(ctx, placeID, []byte(`{"entrance":{"is_level":false}}`), func(p *models.AccessibilityProfile) { + p.UserVerified = false + }) + if err != nil { t.Fatalf("override UpsertProfile: %v", err) } + if got.Entrance == nil || got.Entrance.IsLevel == nil || *got.Entrance.IsLevel { + t.Errorf("Entrance.IsLevel = %v, want false (overwritten)", got.Entrance) + } + if got.UserVerified { + t.Errorf("UserVerified = true, want false (prepare callback applied)") + } +} + +func TestRepository_UpsertProfile_PartialSubmissionPreservesOtherComponents(t *testing.T) { + t.Cleanup(func() { truncate(t) }) + ctx := context.Background() + gormDB := testDB + + repo := place.NewRepository(gormDB) + placeID := mustCreatePlace(ctx, t, gormDB, 1004, "Full Profile Place") + + first := []byte(`{"entrance":{"is_level":true},"restroom":{"is_accessible":true},"parking":{"has_disabled_spaces":true}}`) + if _, _, err := repo.UpsertProfile(ctx, placeID, first, nil); err != nil { + t.Fatalf("first UpsertProfile: %v", err) + } + + entranceOnly := []byte(`{"entrance":{"is_level":false}}`) + got, _, err := repo.UpsertProfile(ctx, placeID, entranceOnly, nil) + if err != nil { + t.Fatalf("second UpsertProfile: %v", err) + } + + if got.Entrance == nil || got.Entrance.IsLevel == nil || *got.Entrance.IsLevel { + t.Errorf("Entrance.IsLevel = %v, want false (updated)", got.Entrance) + } + if got.Restroom == nil || got.Restroom.IsAccessible == nil || !*got.Restroom.IsAccessible { + t.Errorf("Restroom = %v, want preserved (IsAccessible=true)", got.Restroom) + } + if got.Parking == nil || got.Parking.HasDisabledSpaces == nil || !*got.Parking.HasDisabledSpaces { + t.Errorf("Parking = %v, want preserved (HasDisabledSpaces=true)", got.Parking) + } +} + +func TestRepository_UpsertProfile_ExplicitNullClearsComponent(t *testing.T) { + t.Cleanup(func() { truncate(t) }) + ctx := context.Background() + gormDB := testDB + + repo := place.NewRepository(gormDB) + placeID := mustCreatePlace(ctx, t, gormDB, 1005, "Explicit Null Place") + + first := []byte(`{"entrance":{"is_level":true},"restroom":{"is_accessible":true}}`) + if _, _, err := repo.UpsertProfile(ctx, placeID, first, nil); err != nil { + t.Fatalf("first UpsertProfile: %v", err) + } + + got, _, err := repo.UpsertProfile(ctx, placeID, []byte(`{"restroom":null}`), nil) + if err != nil { + t.Fatalf("second UpsertProfile: %v", err) + } + if got.Entrance == nil || got.Entrance.IsLevel == nil || !*got.Entrance.IsLevel { + t.Errorf("Entrance = %v, want preserved (IsLevel=true)", got.Entrance) + } + if got.Restroom != nil { + t.Errorf("Restroom = %v, want nil (explicitly cleared)", got.Restroom) + } +} + +func TestRepository_UpsertProfile_InvalidPatchReturnsErrInvalidPatch(t *testing.T) { + t.Cleanup(func() { truncate(t) }) + ctx := context.Background() + gormDB := testDB + + repo := place.NewRepository(gormDB) + placeID := mustCreatePlace(ctx, t, gormDB, 1006, "Invalid Patch Place") + + _, _, err := repo.UpsertProfile(ctx, placeID, []byte(`not json`), nil) + if !errors.Is(err, place.ErrInvalidPatch) { + t.Errorf("err = %v, want ErrInvalidPatch", err) + } +} + +func TestRepository_UpsertProfile_ConcurrentPatchesBothSurvive(t *testing.T) { + t.Cleanup(func() { truncate(t) }) + ctx := context.Background() + gormDB := testDB + + repo := place.NewRepository(gormDB) + placeID := mustCreatePlace(ctx, t, gormDB, 1007, "Concurrent Patch Place") + + var wg sync.WaitGroup + wg.Add(2) + go func() { + defer wg.Done() + if _, _, err := repo.UpsertProfile(ctx, placeID, []byte(`{"entrance":{"is_level":true}}`), nil); err != nil { + t.Errorf("entrance patch: %v", err) + } + }() + go func() { + defer wg.Done() + if _, _, err := repo.UpsertProfile(ctx, placeID, []byte(`{"restroom":{"is_accessible":true}}`), nil); err != nil { + t.Errorf("restroom patch: %v", err) + } + }() + wg.Wait() var got models.AccessibilityProfile if err := gormDB.Where("place_id = ?", placeID).First(&got).Error; err != nil { t.Fatalf("load profile: %v", err) } - if got.Entrance == nil || got.Entrance.IsLevel == nil || *got.Entrance.IsLevel { - t.Errorf("Entrance.IsLevel = %v, want false (overwritten)", got.Entrance) + if got.Entrance == nil || got.Entrance.IsLevel == nil || !*got.Entrance.IsLevel { + t.Errorf("Entrance = %v, want survived concurrent patch (IsLevel=true)", got.Entrance) + } + if got.Restroom == nil || got.Restroom.IsAccessible == nil || !*got.Restroom.IsAccessible { + t.Errorf("Restroom = %v, want survived concurrent patch (IsAccessible=true)", got.Restroom) } } @@ -347,7 +436,7 @@ func TestRepository_UpsertProfile_PlaceNotFound(t *testing.T) { db := testDB repo := place.NewRepository(db) - _, err := repo.UpsertProfile(ctx, "00000000-0000-0000-0000-000000000000", &models.AccessibilityProfile{}) + _, _, err := repo.UpsertProfile(ctx, "00000000-0000-0000-0000-000000000000", []byte(`{}`), nil) if !errors.Is(err, place.ErrPlaceNotFound) { t.Errorf("err = %v, want ErrPlaceNotFound", err) } @@ -405,7 +494,9 @@ func TestRepository_UpsertProfileIngestion_SkipsUserVerified(t *testing.T) { repo := place.NewRepository(db) placeID := mustCreatePlace(ctx, t, db, 9006, "Café Pascal Ingestion3") - if _, err := repo.UpsertProfile(ctx, placeID, &models.AccessibilityProfile{Entrance: &models.EntranceProps{IsLevel: boolPtr(true)}, UserVerified: true}); err != nil { + if _, _, err := repo.UpsertProfile(ctx, placeID, []byte(`{"entrance":{"is_level":true}}`), func(p *models.AccessibilityProfile) { + p.UserVerified = true + }); err != nil { t.Fatalf("seed: %v", err) } written, err := repo.UpsertProfileIngestion(ctx, placeID, &models.AccessibilityProfile{Entrance: &models.EntranceProps{IsLevel: boolPtr(false)}}) diff --git a/internal/validation/validation.go b/internal/validation/validation.go index f2aa522..e27824b 100644 --- a/internal/validation/validation.go +++ b/internal/validation/validation.go @@ -4,8 +4,8 @@ */ // Package validation enforces constraints that cannot be expressed in the -// OpenAPI spec: whitespace-only strings, tag/metadata size limits, mutual -// exclusivity of query-param groups, and cursor format. +// OpenAPI spec: whitespace-only strings, tag/metadata/source-report size +// limits, mutual exclusivity of query-param groups, and cursor format. // Structural checks (required fields, enum values, numeric bounds, UUID format) // are handled by the nethttp-middleware spec validator before handlers run. package validation @@ -26,12 +26,13 @@ type FieldError struct { } const ( - maxNameLength = 256 - maxSourceLength = 64 - maxTagEntries = 50 - maxTagKeyLength = 64 - maxTagValueLength = 256 - maxParkingCount = 10000 + maxNameLength = 256 + maxSourceLength = 64 + maxTagEntries = 50 + maxTagKeyLength = 64 + maxTagValueLength = 256 + maxParkingCount = 10000 + maxSourceReportEntries = 50 ) var emailRegex = regexp.MustCompile(`^[a-zA-Z0-9._%+\-]+@[a-zA-Z0-9.\-]+\.[a-zA-Z]{2,}$`) @@ -122,6 +123,27 @@ func PlacesQuery(p PlacesQueryParams) []FieldError { return errs } +func AccessibilityProfile(p *models.AccessibilityProfile) []FieldError { + if p == nil { + return nil + } + + if len(p.SourceReports) > maxSourceReportEntries { + return []FieldError{{Field: "source_reports", Reason: fmt.Sprintf("must contain ≤ %d entries", maxSourceReportEntries)}} + } + + var errs []FieldError + for i, sr := range p.SourceReports { + if len(sr.Source) > maxSourceLength { + errs = append(errs, FieldError{Field: "source_reports", Reason: fmt.Sprintf("entry %d: source exceeds %d characters", i, maxSourceLength)}) + } + if len(sr.Value) > maxSourceLength { + errs = append(errs, FieldError{Field: "source_reports", Reason: fmt.Sprintf("entry %d: value exceeds %d characters", i, maxSourceLength)}) + } + } + return errs +} + func validateTags(tags models.PlaceTags) []FieldError { if len(tags) > maxTagEntries { return []FieldError{{Field: "tags", Reason: fmt.Sprintf("must contain ≤ %d entries", maxTagEntries)}} diff --git a/internal/validation/validation_test.go b/internal/validation/validation_test.go index c92b2d6..b6fdc63 100644 --- a/internal/validation/validation_test.go +++ b/internal/validation/validation_test.go @@ -107,6 +107,53 @@ func TestPlace_Tags(t *testing.T) { }) } +// ── AccessibilityProfile ───────────────────────────────────────────────────── + +func TestAccessibilityProfile_Nil(t *testing.T) { + t.Parallel() + if errs := AccessibilityProfile(nil); len(errs) != 0 { + t.Errorf("AccessibilityProfile(nil) = %+v, want no errors", errs) + } +} + +func TestAccessibilityProfile_Valid(t *testing.T) { + t.Parallel() + p := &models.AccessibilityProfile{ + SourceReports: models.SourceReports{{Source: "osm", Value: "yes"}}, + } + if errs := AccessibilityProfile(p); len(errs) != 0 { + t.Errorf("expected no errors, got %+v", errs) + } +} + +func TestAccessibilityProfile_SourceReports(t *testing.T) { + t.Parallel() + t.Run("too many entries", func(t *testing.T) { + p := &models.AccessibilityProfile{ + SourceReports: make(models.SourceReports, maxSourceReportEntries+1), + } + if !errorsHaveField(AccessibilityProfile(p), "source_reports") { + t.Error("expected error on source_reports") + } + }) + t.Run("oversized source", func(t *testing.T) { + p := &models.AccessibilityProfile{ + SourceReports: models.SourceReports{{Source: strings.Repeat("s", maxSourceLength+1), Value: "yes"}}, + } + if !errorsHaveField(AccessibilityProfile(p), "source_reports") { + t.Error("expected error on source_reports") + } + }) + t.Run("oversized value", func(t *testing.T) { + p := &models.AccessibilityProfile{ + SourceReports: models.SourceReports{{Source: "osm", Value: strings.Repeat("v", maxSourceLength+1)}}, + } + if !errorsHaveField(AccessibilityProfile(p), "source_reports") { + t.Error("expected error on source_reports") + } + }) +} + // ── PlacesQuery ─────────────────────────────────────────────────────────────── // Mutual exclusivity and group completeness — these cannot be expressed in the