From ce304abd2c67ce1a68774159ab1b2b9fb05a2856 Mon Sep 17 00:00:00 2001 From: Michael Mayer Date: Mon, 20 Oct 2025 16:46:59 +0200 Subject: [PATCH] API: Update endpoints to return HTTP 201 when a new resource was created Signed-off-by: Michael Mayer --- internal/api/albums.go | 12 +++++- internal/api/albums_test.go | 29 ++++++++++---- internal/api/batch_photos_test.go | 2 +- internal/api/markers.go | 12 ++++-- internal/api/markers_test.go | 10 ++++- internal/api/services.go | 8 +++- internal/api/services_test.go | 12 ++++-- internal/api/swagger.json | 26 ++++++++++--- internal/api/users_passcode.go | 5 ++- internal/api/users_passcode_test.go | 5 ++- pkg/http/header/location.go | 60 +++++++++++++++++++++++++++++ pkg/http/header/location_test.go | 43 +++++++++++++++++++++ 12 files changed, 194 insertions(+), 30 deletions(-) create mode 100644 pkg/http/header/location.go create mode 100644 pkg/http/header/location_test.go diff --git a/internal/api/albums.go b/internal/api/albums.go index b59838874..4cf77f0ed 100644 --- a/internal/api/albums.go +++ b/internal/api/albums.go @@ -17,6 +17,7 @@ import ( "github.com/photoprism/photoprism/internal/photoprism/get" "github.com/photoprism/photoprism/pkg/clean" "github.com/photoprism/photoprism/pkg/fs" + "github.com/photoprism/photoprism/pkg/http/header" "github.com/photoprism/photoprism/pkg/i18n" ) @@ -114,6 +115,7 @@ func GetAlbum(router *gin.RouterGroup) { // @Accept json // @Produce json // @Success 200 {object} entity.Album +// @Success 201 {object} entity.Album // @Failure 400,401,403,429,500 {object} i18n.Response // @Param album body form.Album true "properties of the album to be created (currently supports Title and Favorite)" // @Router /api/v1/albums [post] @@ -141,6 +143,8 @@ func CreateAlbum(router *gin.RouterGroup) { album := entity.NewUserAlbum(frm.AlbumTitle, entity.AlbumManual, conf.Settings().Albums.Order.Album, s.UserUID) album.AlbumFavorite = frm.AlbumFavorite + status := http.StatusOK + // Existing album? if found := album.Find(); found == nil { // Not found, create new album. @@ -150,6 +154,7 @@ func CreateAlbum(router *gin.RouterGroup) { AbortUnexpectedError(c) return } + status = http.StatusCreated } else { // Exists, restore if necessary. album = found @@ -169,8 +174,13 @@ func CreateAlbum(router *gin.RouterGroup) { // Update album YAML backup. SaveAlbumYaml(*album) + // Add location header if newly created. + if status == http.StatusCreated { + header.SetLocation(c, c.FullPath(), album.AlbumUID) + } + // Return as JSON. - c.JSON(http.StatusOK, album) + c.JSON(status, album) }) } diff --git a/internal/api/albums_test.go b/internal/api/albums_test.go index e78d400b0..8159b1397 100644 --- a/internal/api/albums_test.go +++ b/internal/api/albums_test.go @@ -40,7 +40,10 @@ func TestCreateAlbum(t *testing.T) { assert.Equal(t, "new-created-album", val.String()) val2 := gjson.Get(r.Body.String(), "Favorite") assert.Equal(t, "true", val2.String()) - assert.Equal(t, http.StatusOK, r.Code) + uid := gjson.Get(r.Body.String(), "UID").String() + assert.NotEmpty(t, uid) + assert.Equal(t, "/api/v1/albums/"+uid, r.Header().Get("Location")) + assert.Equal(t, http.StatusCreated, r.Code) }) t.Run("Invalid", func(t *testing.T) { app, router, _ := NewApiTest() @@ -53,8 +56,10 @@ func TestUpdateAlbum(t *testing.T) { app, router, _ := NewApiTest() CreateAlbum(router) r := PerformRequestWithBody(app, "POST", "/api/v1/albums", `{"Title": "Update", "Description": "To be updated", "Notes": "", "Favorite": true}`) - assert.Equal(t, http.StatusOK, r.Code) + assert.Equal(t, http.StatusCreated, r.Code) uid := gjson.Get(r.Body.String(), "UID").String() + assert.NotEmpty(t, uid) + assert.Equal(t, "/api/v1/albums/"+uid, r.Header().Get("Location")) t.Run("Successful", func(t *testing.T) { app, router, _ := NewApiTest() @@ -85,8 +90,10 @@ func TestDeleteAlbum(t *testing.T) { createApp, createRouter, _ := NewApiTest() CreateAlbum(createRouter) createResp := PerformRequestWithBody(createApp, "POST", "/api/v1/albums", `{"Title": "Delete", "Description": "To be deleted", "Notes": "", "Favorite": true}`) - assert.Equal(t, http.StatusOK, createResp.Code) + assert.Equal(t, http.StatusCreated, createResp.Code) albumUid := gjson.Get(createResp.Body.String(), "UID").String() + assert.NotEmpty(t, albumUid) + assert.Equal(t, "/api/v1/albums/"+albumUid, createResp.Header().Get("Location")) t.Run("ExistingAlbum", func(t *testing.T) { app, router, _ := NewApiTest() @@ -124,7 +131,7 @@ func TestDeleteAlbum(t *testing.T) { DeleteAlbum(router) r := PerformRequestWithBody(app, "POST", "/api/v1/albums", `{"Title": "ForceDelete", "Favorite": false}`) - assert.Equal(t, http.StatusOK, r.Code) + assert.Equal(t, http.StatusCreated, r.Code) uid := gjson.Get(r.Body.String(), "UID").String() slug := gjson.Get(r.Body.String(), "Slug").String() @@ -132,9 +139,11 @@ func TestDeleteAlbum(t *testing.T) { assert.Equal(t, http.StatusOK, delResp.Code) recreateResp := PerformRequestWithBody(app, "POST", "/api/v1/albums", `{"Title": "ForceDelete", "Favorite": false}`) - assert.Equal(t, http.StatusOK, recreateResp.Code) + assert.Equal(t, http.StatusCreated, recreateResp.Code) newUID := gjson.Get(recreateResp.Body.String(), "UID").String() newSlug := gjson.Get(recreateResp.Body.String(), "Slug").String() + assert.NotEmpty(t, newUID) + assert.Equal(t, "/api/v1/albums/"+newUID, recreateResp.Header().Get("Location")) assert.NotEmpty(t, uid) assert.NotEmpty(t, newUID) @@ -192,8 +201,10 @@ func TestAddPhotosToAlbum(t *testing.T) { app, router, _ := NewApiTest() CreateAlbum(router) r := PerformRequestWithBody(app, "POST", "/api/v1/albums", `{"Title": "Add photos", "Description": "", "Notes": "", "Favorite": true}`) - assert.Equal(t, http.StatusOK, r.Code) + assert.Equal(t, http.StatusCreated, r.Code) uid := gjson.Get(r.Body.String(), "UID").String() + assert.NotEmpty(t, uid) + assert.Equal(t, "/api/v1/albums/"+uid, r.Header().Get("Location")) t.Run("AddMultiplePhotos", func(t *testing.T) { app, router, _ := NewApiTest() @@ -263,8 +274,10 @@ func TestRemovePhotosFromAlbum(t *testing.T) { AddPhotosToAlbum(router) r := PerformRequestWithBody(app, "POST", "/api/v1/albums", `{"Title": "Remove photos", "Description": "", "Notes": "", "Favorite": true}`) - assert.Equal(t, http.StatusOK, r.Code) + assert.Equal(t, http.StatusCreated, r.Code) uid := gjson.Get(r.Body.String(), "UID").String() + assert.NotEmpty(t, uid) + assert.Equal(t, "/api/v1/albums/"+uid, r.Header().Get("Location")) r2 := PerformRequestWithBody(app, "POST", "/api/v1/albums/"+uid+"/photos", `{"photos": ["ps6sg6be2lvl0y12", "ps6sg6be2lvl0y11"]}`) assert.Equal(t, http.StatusOK, r2.Code) @@ -311,7 +324,7 @@ func TestCloneAlbums(t *testing.T) { app, router, _ := NewApiTest() CreateAlbum(router) r := PerformRequestWithBody(app, "POST", "/api/v1/albums", `{"Title": "Update", "Description": "To be updated", "Notes": "", "Favorite": true}`) - assert.Equal(t, http.StatusOK, r.Code) + assert.Equal(t, http.StatusCreated, r.Code) uid := gjson.Get(r.Body.String(), "UID").String() t.Run("CloneEmptyAlbum", func(t *testing.T) { diff --git a/internal/api/batch_photos_test.go b/internal/api/batch_photos_test.go index dbce8016e..42619021c 100644 --- a/internal/api/batch_photos_test.go +++ b/internal/api/batch_photos_test.go @@ -97,7 +97,7 @@ func TestBatchAlbumsDelete(t *testing.T) { app, router, _ := NewApiTest() CreateAlbum(router) r := PerformRequestWithBody(app, "POST", "/api/v1/albums", `{"Title": "BatchDelete", "Description": "To be deleted", "Notes": "", "Favorite": true}`) - assert.Equal(t, http.StatusOK, r.Code) + assert.Equal(t, http.StatusCreated, r.Code) uid := gjson.Get(r.Body.String(), "UID").String() t.Run("Success", func(t *testing.T) { diff --git a/internal/api/markers.go b/internal/api/markers.go index c73fcdfcc..5745a544b 100644 --- a/internal/api/markers.go +++ b/internal/api/markers.go @@ -18,6 +18,7 @@ import ( "github.com/photoprism/photoprism/internal/photoprism/get" "github.com/photoprism/photoprism/internal/thumb/crop" "github.com/photoprism/photoprism/pkg/clean" + "github.com/photoprism/photoprism/pkg/http/header" "github.com/photoprism/photoprism/pkg/i18n" ) @@ -75,8 +76,10 @@ func findFileMarker(c *gin.Context) (file *entity.File, marker *entity.Marker, e // // See internal/form/marker.go for the values required to create a new marker. // -// @Tags Files -// @Router /api/v1/markers [post] +// @Tags Files +// @Produce json +// @Success 201 {object} entity.Marker +// @Router /api/v1/markers [post] func CreateMarker(router *gin.RouterGroup) { router.POST("/markers", func(c *gin.Context) { s := Auth(c, acl.ResourceFiles, acl.ActionUpdate) @@ -164,8 +167,9 @@ func CreateMarker(router *gin.RouterGroup) { // Display success message. event.SuccessMsg(i18n.MsgChangesSaved) - // Return new marker. - c.JSON(http.StatusOK, marker) + // Return new marker with location header. + header.SetLocation(c, c.FullPath(), marker.MarkerUID) + c.JSON(http.StatusCreated, marker) }) } diff --git a/internal/api/markers_test.go b/internal/api/markers_test.go index 77f4c8040..30db0974f 100644 --- a/internal/api/markers_test.go +++ b/internal/api/markers_test.go @@ -53,7 +53,10 @@ func TestCreateMarker(t *testing.T) { r = PerformRequestWithBody(app, "POST", u, string(b)) } - assert.Equal(t, http.StatusOK, r.Code) + assert.Equal(t, http.StatusCreated, r.Code) + newUID := gjson.Get(r.Body.String(), "UID").String() + assert.NotEmpty(t, newUID) + assert.Equal(t, "/api/v1/markers/"+newUID, r.Header().Get("Location")) }) t.Run("SuccessWithName", func(t *testing.T) { app, router, _ := NewApiTest() @@ -95,7 +98,10 @@ func TestCreateMarker(t *testing.T) { r = PerformRequestWithBody(app, "POST", u, string(b)) } - assert.Equal(t, http.StatusOK, r.Code) + assert.Equal(t, http.StatusCreated, r.Code) + newUID := gjson.Get(r.Body.String(), "UID").String() + assert.NotEmpty(t, newUID) + assert.Equal(t, "/api/v1/markers/"+newUID, r.Header().Get("Location")) }) t.Run("InvalidArea", func(t *testing.T) { app, router, _ := NewApiTest() diff --git a/internal/api/services.go b/internal/api/services.go index 8c7574e09..1bbbc1ecc 100644 --- a/internal/api/services.go +++ b/internal/api/services.go @@ -3,6 +3,7 @@ package api import ( "fmt" "net/http" + "strconv" "time" "github.com/gin-gonic/gin" @@ -15,6 +16,7 @@ import ( "github.com/photoprism/photoprism/internal/workers" "github.com/photoprism/photoprism/pkg/clean" "github.com/photoprism/photoprism/pkg/fs" + "github.com/photoprism/photoprism/pkg/http/header" "github.com/photoprism/photoprism/pkg/i18n" ) @@ -126,7 +128,7 @@ func GetServiceFolders(router *gin.RouterGroup) { // @Tags Services // @Accept json // @Produce json -// @Success 200 {object} entity.Service +// @Success 201 {object} entity.Service // @Failure 400,401,403,429 {object} i18n.Response // @Param service body form.Service true "properties of the service to be created" // @Router /api/v1/services [post] @@ -166,7 +168,9 @@ func AddService(router *gin.RouterGroup) { return } - c.JSON(http.StatusOK, m) + // Return new service with location header. + header.SetLocation(c, c.FullPath(), strconv.FormatUint(uint64(m.ID), 10)) + c.JSON(http.StatusCreated, m) }) } diff --git a/internal/api/services_test.go b/internal/api/services_test.go index b02ac3504..d9336ef26 100644 --- a/internal/api/services_test.go +++ b/internal/api/services_test.go @@ -1,6 +1,7 @@ package api import ( + "fmt" "net/http" "testing" @@ -77,7 +78,10 @@ func TestCreateService(t *testing.T) { "SyncPath": "", "SyncInterval": 3, "SyncUpload": false, "SyncDownload": false, "SyncFilenames": false, "SyncRaw": false}`) val := gjson.Get(r.Body.String(), "AccOwner") assert.Equal(t, "Test", val.String()) - assert.Equal(t, http.StatusOK, r.Code) + id := gjson.Get(r.Body.String(), "ID").Int() + assert.NotZero(t, id) + assert.Equal(t, fmt.Sprintf("/api/v1/services/%d", id), r.Header().Get("Location")) + assert.Equal(t, http.StatusCreated, r.Code) }) } @@ -93,7 +97,7 @@ func TestUpdateService(t *testing.T) { assert.Equal(t, int64(5), val2.Int()) val3 := gjson.Get(r.Body.String(), "AccName") assert.Equal(t, "Dummy-Webdav", val3.String()) - assert.Equal(t, http.StatusOK, r.Code) + assert.Equal(t, http.StatusCreated, r.Code) id := gjson.Get(r.Body.String(), "ID").String() t.Run("Success", func(t *testing.T) { @@ -132,8 +136,10 @@ func TestDeleteService(t *testing.T) { r := PerformRequestWithBody(app, "POST", "/api/v1/services", `{"AccName": "DeleteTest", "AccOwner": "TestDelete", "AccUrl": "http://dummy-webdav/", "AccType": "webdav", "AccKey": "123", "AccUser": "admin", "AccPass": "photoprism", "AccError": "", "AccShare": false, "AccSync": false, "RetryLimit": 3, "SharePath": "", "ShareSize": "", "ShareExpires": 0, "SyncPath": "", "SyncInterval": 5, "SyncUpload": false, "SyncDownload": false, "SyncFilenames": false, "SyncRaw": false}`) - assert.Equal(t, http.StatusOK, r.Code) + assert.Equal(t, http.StatusCreated, r.Code) id := gjson.Get(r.Body.String(), "ID").String() + assert.NotEmpty(t, id) + assert.Equal(t, "/api/v1/services/"+id, r.Header().Get("Location")) t.Run("Success", func(t *testing.T) { app, router, _ := NewApiTest() diff --git a/internal/api/swagger.json b/internal/api/swagger.json index 49b7f34ca..5d5232a64 100644 --- a/internal/api/swagger.json +++ b/internal/api/swagger.json @@ -5009,6 +5009,12 @@ "$ref": "#/definitions/entity.Album" } }, + "201": { + "description": "Created", + "schema": { + "$ref": "#/definitions/entity.Album" + } + }, "400": { "description": "Bad Request", "schema": { @@ -8282,7 +8288,17 @@ }, "/api/v1/markers": { "post": { - "responses": {}, + "produces": [ + "application/json" + ], + "responses": { + "201": { + "description": "Created", + "schema": { + "$ref": "#/definitions/entity.Marker" + } + } + }, "tags": [ "Files" ] @@ -10030,8 +10046,8 @@ "application/json" ], "responses": { - "200": { - "description": "OK", + "201": { + "description": "Created", "schema": { "$ref": "#/definitions/entity.Service" } @@ -11457,8 +11473,8 @@ "application/json" ], "responses": { - "200": { - "description": "OK", + "201": { + "description": "Created", "schema": { "$ref": "#/definitions/entity.Passcode" } diff --git a/internal/api/users_passcode.go b/internal/api/users_passcode.go index e6b0e1cbc..77afd7aec 100644 --- a/internal/api/users_passcode.go +++ b/internal/api/users_passcode.go @@ -30,7 +30,7 @@ import ( // @Produce json // @Param uid path string true "user uid" // @Param request body form.Passcode true "passcode setup (password required)" -// @Success 200 {object} entity.Passcode +// @Success 201 {object} entity.Passcode // @Failure 400,401,403,429 {object} i18n.Response // @Router /api/v1/users/{uid}/passcode [post] func CreateUserPasscode(router *gin.RouterGroup) { @@ -84,7 +84,8 @@ func CreateUserPasscode(router *gin.RouterGroup) { event.AuditInfo([]string{ClientIP(c), "session %s", authn.Users, user.UserName, authn.Passcode, authn.Created}, s.RefID) - c.JSON(http.StatusOK, passcode) + header.SetLocation(c) + c.JSON(http.StatusCreated, passcode) }) } diff --git a/internal/api/users_passcode_test.go b/internal/api/users_passcode_test.go index 2de7d2d79..08d10eb6f 100644 --- a/internal/api/users_passcode_test.go +++ b/internal/api/users_passcode_test.go @@ -103,7 +103,8 @@ func TestCreateUserPasscode(t *testing.T) { log.Fatal(err) } else { r := AuthenticatedRequestWithBody(app, "POST", "/api/v1/users/uqxetse3cy5eo9z2/passcode", string(pcStr), sessId) - assert.Equal(t, http.StatusOK, r.Code) + assert.Equal(t, http.StatusCreated, r.Code) + assert.Equal(t, "/api/v1/users/uqxetse3cy5eo9z2/passcode", r.Header().Get("Location")) } }) } @@ -248,7 +249,7 @@ func TestUserPasscode(t *testing.T) { } r := AuthenticatedRequestWithBody(app, "POST", "/api/v1/users/uqxetse3cy5eo9z2/passcode", string(pcStr), sessId) - assert.Equal(t, http.StatusOK, r.Code) + assert.Equal(t, http.StatusCreated, r.Code) secret := gjson.Get(r.Body.String(), "Secret").String() activatedAt := gjson.Get(r.Body.String(), "ActivatedAt").String() diff --git a/pkg/http/header/location.go b/pkg/http/header/location.go new file mode 100644 index 000000000..5f4990fe1 --- /dev/null +++ b/pkg/http/header/location.go @@ -0,0 +1,60 @@ +package header + +import ( + "strings" + + "github.com/gin-gonic/gin" +) + +// SetLocation adds a Location header with a relative path based on the provided segments. +// When the first segment is non-empty it is treated as the base path; +// otherwise the request URL path is used. +func SetLocation(c *gin.Context, segments ...string) { + // Return if context is missing. + if c == nil { + return + } + + base := "" + + if len(segments) > 0 && segments[0] != "" { + base = segments[0] + segments = segments[1:] + } else if c.Request != nil && c.Request.URL != nil { + base = c.Request.URL.Path + } + + // Return if base is missing. + if base == "" { + return + } + + // Compose redirect location string. + prefixSlash := strings.HasPrefix(base, "/") + base = strings.Trim(base, "/") + + parts := make([]string, 0, 1+len(segments)) + if base != "" { + parts = append(parts, base) + } + + for _, segment := range segments { + segment = strings.Trim(segment, "/") + if segment == "" { + continue + } + parts = append(parts, segment) + } + + location := strings.Join(parts, "/") + if prefixSlash { + location = "/" + location + } + + // Add Location header to response. + if location == "" && prefixSlash { + c.Header(Location, "/") + } else { + c.Header(Location, location) + } +} diff --git a/pkg/http/header/location_test.go b/pkg/http/header/location_test.go new file mode 100644 index 000000000..30ed35707 --- /dev/null +++ b/pkg/http/header/location_test.go @@ -0,0 +1,43 @@ +package header + +import ( + "net/http/httptest" + "testing" + + "github.com/gin-gonic/gin" + "github.com/stretchr/testify/assert" +) + +func TestSetLocationWithBase(t *testing.T) { + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest("POST", "/api/v1/albums", nil) + + SetLocation(c, "/api/v1/albums", "abc123") + + assert.Equal(t, "/api/v1/albums/abc123", c.Writer.Header().Get(Location)) +} + +func TestSetLocationUsesRequestPath(t *testing.T) { + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest("POST", "/api/v1/services", nil) + + SetLocation(c, "", "99") + + assert.Equal(t, "/api/v1/services/99", c.Writer.Header().Get(Location)) +} + +func TestSetLocationTrimsSegments(t *testing.T) { + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest("POST", "/api/v1/markers/", nil) + + SetLocation(c, "/api/v1/markers/", "/m1/") + + assert.Equal(t, "/api/v1/markers/m1", c.Writer.Header().Get(Location)) +} + +func TestSetLocationEmpty(t *testing.T) { + SetLocation(nil) +}