From d582ffe49cddd7940a55b41e1d12b6fe7562635e Mon Sep 17 00:00:00 2001 From: Fabio Bonelli Date: Sat, 9 Mar 2024 18:44:43 +0100 Subject: [PATCH] feat: implement JSON Patch for Software resource (#221) Implement JSON Patch (https://datatracker.ietf.org/doc/html/rfc6902) for Software resource and bump the json-patch library. SECURITY NOTE: This doesn't implement any authorization on resources, which is good *for now* as you either have the ability to write to resources or not have it. That MUST be implemented when there will be multiple write tokens with different scopes. (See #188) --- go.mod | 2 +- go.sum | 4 +- internal/handlers/software.go | 40 ++++++++++++----- internal/models/models.go | 42 ++++++++++++++--- main_test.go | 85 +++++++++++++++++++++++++++++++++++ 5 files changed, 152 insertions(+), 21 deletions(-) diff --git a/go.mod b/go.mod index 59ed550..665f7d6 100644 --- a/go.mod +++ b/go.mod @@ -17,7 +17,7 @@ require ( require ( github.com/ansrivas/fiberprometheus/v2 v2.4.1 - github.com/evanphx/json-patch/v5 v5.7.0 + github.com/evanphx/json-patch/v5 v5.9.0 golang.org/x/exp v0.0.0-20220827204233-334a2380cb91 ) diff --git a/go.sum b/go.sum index 054be10..e8960ed 100644 --- a/go.sum +++ b/go.sum @@ -80,8 +80,8 @@ github.com/envoyproxy/go-control-plane v0.9.0/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymF github.com/envoyproxy/go-control-plane v0.9.1-0.20191026205805-5f8ba28d4473/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4= github.com/envoyproxy/go-control-plane v0.9.4/go.mod h1:6rpuAdCZL397s3pYoYcLgu1mIlRU8Am5FuJP05cCM98= github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c= -github.com/evanphx/json-patch/v5 v5.7.0 h1:nJqP7uwL84RJInrohHfW0Fx3awjbm8qZeFv0nW9SYGc= -github.com/evanphx/json-patch/v5 v5.7.0/go.mod h1:VNkHZ/282BpEyt/tObQO8s5CMPmYYq14uClGH4abBuQ= +github.com/evanphx/json-patch/v5 v5.9.0 h1:kcBlZQbplgElYIlo/n1hJbls2z/1awpXxpRi0/FOJfg= +github.com/evanphx/json-patch/v5 v5.9.0/go.mod h1:VNkHZ/282BpEyt/tObQO8s5CMPmYYq14uClGH4abBuQ= github.com/go-gl/glfw v0.0.0-20190409004039-e6da0acd62b1/go.mod h1:vR7hzQXu2zJy9AVAgeJqvqgH9Q5CA+iKCZ2gyEVpxRU= github.com/go-gl/glfw/v3.3/glfw v0.0.0-20191125211704-12ad95a8df72/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8= github.com/go-gl/glfw/v3.3/glfw v0.0.0-20200222043503-6f7a984d4dc4/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8= diff --git a/internal/handlers/software.go b/internal/handlers/software.go index b71dc31..8cafdd2 100644 --- a/internal/handlers/software.go +++ b/internal/handlers/software.go @@ -30,8 +30,9 @@ type Software struct { } var ( - errLoadNotFound = errors.New("Software was not found") - errLoad = errors.New("error while loading Software") + errLoadNotFound = errors.New("Software was not found") + errLoad = errors.New("error while loading Software") + errMalformedJSONPatch = errors.New("malformed JSON Patch") ) func NewSoftware(db *gorm.DB) *Software { @@ -174,7 +175,6 @@ func (p *Software) PostSoftware(ctx *fiber.Ctx) error { func (p *Software) PatchSoftware(ctx *fiber.Ctx) error { //nolint:funlen,cyclop const errMsg = "can't update Software" - softwareReq := common.SoftwarePatch{} software := models.Software{} if err := loadSoftware(p.db, &software, ctx.Params("id")); err != nil { @@ -185,18 +185,36 @@ func (p *Software) PatchSoftware(ctx *fiber.Ctx) error { //nolint:funlen,cyclop return common.Error(fiber.StatusInternalServerError, errMsg, fiber.ErrInternalServerError.Message) } - if err := common.ValidateRequestEntity(ctx, &softwareReq, errMsg); err != nil { - return err //nolint:wrapcheck - } - softwareJSON, err := json.Marshal(&software) if err != nil { return common.Error(fiber.StatusInternalServerError, errMsg, err.Error()) } - updatedJSON, err := jsonpatch.MergePatch(softwareJSON, ctx.Body()) - if err != nil { - return common.Error(fiber.StatusInternalServerError, errMsg, err.Error()) + var updatedJSON []byte + + switch ctx.Get(fiber.HeaderContentType) { + case "application/json-patch+json": + patch, err := jsonpatch.DecodePatch(ctx.Body()) + if err != nil { + return common.Error(fiber.StatusBadRequest, errMsg, errMalformedJSONPatch.Error()) + } + + updatedJSON, err = patch.Apply(softwareJSON) + if err != nil { + return common.Error(fiber.StatusUnprocessableEntity, errMsg, err.Error()) + } + + // application/merge-patch+json by default + default: + softwareReq := common.SoftwarePatch{} + if err := common.ValidateRequestEntity(ctx, &softwareReq, errMsg); err != nil { + return err //nolint:wrapcheck + } + + updatedJSON, err = jsonpatch.MergePatch(softwareJSON, ctx.Body()) + if err != nil { + return common.Error(fiber.StatusInternalServerError, errMsg, err.Error()) + } } var updatedSoftware models.Software @@ -213,7 +231,7 @@ func (p *Software) PatchSoftware(ctx *fiber.Ctx) error { //nolint:funlen,cyclop } if err := p.db.Transaction(func(tran *gorm.DB) error { - //nolint:gocritic // it's fine, we want to another slice + //nolint:gocritic // it's fine, we want to append to another slice currentURLs := append(software.Aliases, software.URL) updatedURL, aliases, err := syncAliases( diff --git a/internal/models/models.go b/internal/models/models.go index a2185db..fa63869 100644 --- a/internal/models/models.go +++ b/internal/models/models.go @@ -71,13 +71,13 @@ type Software struct { // with SoftwareURLs (belongs to and has many). SoftwareURLID string `json:"-" gorm:"uniqueIndex;not null"` - URL SoftwareURL `json:"url"` - Aliases []SoftwareURL `json:"aliases"` - PubliccodeYml string `json:"publiccodeYml"` - Logs []Log `json:"-" gorm:"polymorphic:Entity;"` - Active *bool `json:"active" gorm:"default:true;not null"` - CreatedAt time.Time `json:"createdAt" gorm:"index"` - UpdatedAt time.Time `json:"updatedAt"` + URL SoftwareURL `json:"url"` + Aliases SoftwareURLSlice `json:"aliases"` + PubliccodeYml string `json:"publiccodeYml"` + Logs []Log `json:"-" gorm:"polymorphic:Entity;"` + Active *bool `json:"active" gorm:"default:true;not null"` + CreatedAt time.Time `json:"createdAt" gorm:"index"` + UpdatedAt time.Time `json:"updatedAt"` } func (Software) TableName() string { @@ -107,6 +107,34 @@ func (su *SoftwareURL) UnmarshalJSON(data []byte) error { return json.Unmarshal(data, &su.URL) } +type SoftwareURLSlice []SoftwareURL + +func (slice SoftwareURLSlice) MarshalJSON() ([]byte, error) { + urls := make([]string, len(slice)) + + for i, su := range slice { + urls[i] = su.URL + } + + return json.Marshal(urls) +} + +func (slice *SoftwareURLSlice) UnmarshalJSON(data []byte) error { + var urls []string + if err := json.Unmarshal(data, &urls); err != nil { + //nolint:wrapcheck // we want to pass along the error here + return err + } + + // Convert each string URL into a SoftwareURL object. + *slice = make(SoftwareURLSlice, len(urls)) + for i, urlStr := range urls { + (*slice)[i] = SoftwareURL{URL: urlStr} + } + + return nil +} + type Webhook struct { ID string `json:"id" gorm:"primaryKey"` URL string `json:"url" gorm:"index:idx_webhook_url,unique"` diff --git a/main_test.go b/main_test.go index 9326610..360a5fa 100644 --- a/main_test.go +++ b/main_test.go @@ -2078,6 +2078,91 @@ func TestSoftwareEndpoints(t *testing.T) { assert.Greater(t, updated, created) }, }, + { + description: "PATCH a software resource with JSON Patch - replace", + query: "PATCH /v1/software/59803fb7-8eec-4fe5-a354-8926009c364a", + body: `[{"op": "replace", "path": "/publiccodeYml", "value": "new publiccode data"}]`, + headers: map[string][]string{ + "Authorization": {goodToken}, + "Content-Type": {"application/json-patch+json"}, + }, + + expectedCode: 200, + expectedContentType: "application/json", + validateFunc: func(t *testing.T, response map[string]interface{}) { + assert.Equal(t, true, response["active"]) + assert.Equal(t, "https://18-a.example.org/code/repo", response["url"]) + + assert.IsType(t, []interface{}{}, response["aliases"]) + + aliases := response["aliases"].([]interface{}) + assert.Equal(t, 1, len(aliases)) + + assert.Equal(t, "https://18-b.example.org/code/repo", aliases[0]) + + assert.Equal(t, "new publiccode data", response["publiccodeYml"]) + assert.Equal(t, "59803fb7-8eec-4fe5-a354-8926009c364a", response["id"]) + + created, err := time.Parse(time.RFC3339, response["createdAt"].(string)) + assert.Nil(t, err) + + updated, err := time.Parse(time.RFC3339, response["updatedAt"].(string)) + assert.Nil(t, err) + + assert.Greater(t, updated, created) + }, + }, + { + description: "PATCH a software resource with JSON Patch - add", + query: "PATCH /v1/software/59803fb7-8eec-4fe5-a354-8926009c364a", + body: `[{"op": "add", "path": "/aliases/-", "value": "https://18-c.example.org"}]`, + headers: map[string][]string{ + "Authorization": {goodToken}, + "Content-Type": {"application/json-patch+json"}, + }, + + expectedCode: 200, + expectedContentType: "application/json", + validateFunc: func(t *testing.T, response map[string]interface{}) { + assert.Equal(t, true, response["active"]) + assert.Equal(t, "https://18-a.example.org/code/repo", response["url"]) + + assert.IsType(t, []interface{}{}, response["aliases"]) + + aliases := response["aliases"].([]interface{}) + assert.Equal(t, 2, len(aliases)) + + assert.Equal(t, "https://18-b.example.org/code/repo", aliases[0]) + assert.Equal(t, "https://18-c.example.org", aliases[1]) + + assert.Equal(t, "-", response["publiccodeYml"]) + assert.Equal(t, "59803fb7-8eec-4fe5-a354-8926009c364a", response["id"]) + + created, err := time.Parse(time.RFC3339, response["createdAt"].(string)) + assert.Nil(t, err) + + updated, err := time.Parse(time.RFC3339, response["updatedAt"].(string)) + assert.Nil(t, err) + + assert.Greater(t, updated, created) + }, + }, + { + description: "PATCH a software resource with JSON Patch as Content-Type, but non JSON Patch payload", + query: "PATCH /v1/software/59803fb7-8eec-4fe5-a354-8926009c364a", + body: `{"publiccodeYml": "publiccodedata", "url": "https://software-new.example.org"}`, + headers: map[string][]string{ + "Authorization": {goodToken}, + "Content-Type": {"application/json-patch+json"}, + }, + + expectedCode: 400, + expectedContentType: "application/problem+json", + validateFunc: func(t *testing.T, response map[string]interface{}) { + assert.Equal(t, `can't update Software`, response["title"]) + assert.Equal(t, "malformed JSON Patch", response["detail"]) + }, + }, { description: "PATCH software using an already taken URL as url", query: "PATCH /v1/software/59803fb7-8eec-4fe5-a354-8926009c364a",