From 920c4b58133067d788dd18ade7437fa2efa92d1f Mon Sep 17 00:00:00 2001 From: John McCann Date: Sat, 29 Jan 2022 18:33:28 -0800 Subject: [PATCH 1/4] feat(PUT /resource): add merge parameter --- arborist/resource.go | 62 +++++++++++++++++++++++--------------------- arborist/server.go | 7 ++++- 2 files changed, 39 insertions(+), 30 deletions(-) diff --git a/arborist/resource.go b/arborist/resource.go index a443733..46552fb 100644 --- a/arborist/resource.go +++ b/arborist/resource.go @@ -18,7 +18,7 @@ import ( type ResourceIn struct { Name string `json:"name"` Path string `json:"path"` - Description string `json:"description"` + Description *string `json:"description"` Subresources []ResourceIn `json:"subresources"` } @@ -329,7 +329,7 @@ func (resource *ResourceIn) addPath(parent string) *ErrorResponse { return nil } -func (resource *ResourceIn) overwriteInDb(tx *sqlx.Tx) *ErrorResponse { +func (resource *ResourceIn) updateInDb(tx *sqlx.Tx, merge bool) *ErrorResponse { // arborist uses `/` for path separator; ltree in postgres uses `.` path := FormatPathForDb(resource.Path) stmt := "INSERT INTO resource(path) VALUES ($1) ON CONFLICT DO NOTHING" @@ -345,34 +345,38 @@ func (resource *ResourceIn) overwriteInDb(tx *sqlx.Tx) *ErrorResponse { return newErrorResponse(msg, 409, &err) } - // update description - stmt = "UPDATE resource SET description = $2 WHERE path = $1" - _, err = tx.Exec(stmt, path, resource.Description) + if resource.Description != nil { + // update description + stmt = "UPDATE resource SET description = $2 WHERE path = $1" + _, err = tx.Exec(stmt, path, resource.Description) + } - // delete the subresources not in the new request - if len(resource.Subresources) > 0 { - subPathsKeep := []string{} - for _, subresource := range resource.Subresources { - subresource.addPath(resource.Path) - subpath := fmt.Sprintf("'%s'", FormatPathForDb(subresource.Path)) - subPathsKeep = append(subPathsKeep, subpath) + if !merge { + // delete the subresources not in the new request + if len(resource.Subresources) > 0 { + subPathsKeep := []string{} + for _, subresource := range resource.Subresources { + subresource.addPath(resource.Path) + subpath := fmt.Sprintf("'%s'", FormatPathForDb(subresource.Path)) + subPathsKeep = append(subPathsKeep, subpath) + } + stmtFormat := ` + DELETE FROM resource + WHERE ( + path != $1 + AND path ~ (CAST ((ltree2text($1) || '.*{1}') AS lquery)) + AND path NOT IN (%s) + ) + ` + stmt = fmt.Sprintf(stmtFormat, strings.Join(subPathsKeep, ", ")) + _, _ = tx.Exec(stmt, path) + } else { + stmt := ` + DELETE FROM resource + WHERE path != $1 AND path ~ (CAST ((ltree2text($1) || '.*{1}') AS lquery)) + ` + _, _ = tx.Exec(stmt, path) } - stmtFormat := ` - DELETE FROM resource - WHERE ( - path != $1 - AND path ~ (CAST ((ltree2text($1) || '.*{1}') AS lquery)) - AND path NOT IN (%s) - ) - ` - stmt = fmt.Sprintf(stmtFormat, strings.Join(subPathsKeep, ", ")) - _, _ = tx.Exec(stmt, path) - } else { - stmt := ` - DELETE FROM resource - WHERE path != $1 AND path ~ (CAST ((ltree2text($1) || '.*{1}') AS lquery)) - ` - _, _ = tx.Exec(stmt, path) } // TODO (rudyardrichter, 2019-04-09): optimize (could be non-recursive) @@ -382,7 +386,7 @@ func (resource *ResourceIn) overwriteInDb(tx *sqlx.Tx) *ErrorResponse { if errResponse != nil { return errResponse } - errResponse = subresource.overwriteInDb(tx) + errResponse = subresource.updateInDb(tx, merge) if errResponse != nil { return errResponse } diff --git a/arborist/server.go b/arborist/server.go index c6368a3..1bd6a28 100644 --- a/arborist/server.go +++ b/arborist/server.go @@ -864,7 +864,12 @@ func (server *Server) handleResourceCreate(w http.ResponseWriter, r *http.Reques errResponse = nil if r.Method == "PUT" { - errResponse = transactify(server.db, resource.overwriteInDb) + _, mergeFlag := r.URL.Query()["merge"] + updateResource := func(tx *sqlx.Tx) *ErrorResponse { + resource.updateInDb(tx, mergeFlag) + return nil + } + errResponse = transactify(server.db, updateResource) } else { errResponse = transactify(server.db, resource.createInDb) } From 7206a0535b08d3dbd7defd2531eaa5a34149d8a8 Mon Sep 17 00:00:00 2001 From: John McCann Date: Thu, 10 Feb 2022 18:39:38 -0800 Subject: [PATCH 2/4] test(PUT /resource?merge): confirm no overwriting --- arborist/server_test.go | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/arborist/server_test.go b/arborist/server_test.go index d5768c4..b1ccc35 100644 --- a/arborist/server_test.go +++ b/arborist/server_test.go @@ -1105,6 +1105,35 @@ func TestServer(t *testing.T) { getResourceWithPath(t, "/Godel,/completeness_theorem") }) + t.Run("Merge", func(t *testing.T) { + createResourceBytes(t, []byte(`{ + "name": "animal", + "subresources": [ + {"name": "dinosaur", "subresources": [{"name": "pterodactyl"}]}, + {"name": "dog", "subresources": [{"name": "corgi"}]} + ] + }`)) + w := httptest.NewRecorder() + body := []byte(`{ + "name": "animal", + "subresources": [ + {"name": "bird", "subresources": [{"name": "goose"}]}, + {"name": "cat", "subresources": [{"name": "lion"}]} + ] + }`) + req := newRequest("PUT", "/resource?merge", bytes.NewBuffer(body)) + handler.ServeHTTP(w, req) + if w.Code != http.StatusCreated { + httpError(t, w, "couldn't update resource using PUT") + } + // verify that resources created before update were not overwritten + getResourceWithPath(t, "/animal/dinosaur/pterodactyl") + getResourceWithPath(t, "/animal/dog/corgi") + // verify that update actually created resources + getResourceWithPath(t, "/animal/bird/goose") + getResourceWithPath(t, "/animal/cat/lion") + }) + t.Run("Delete", func(t *testing.T) { w := httptest.NewRecorder() req := newRequest("DELETE", "/resource/a", nil) From 94b124106eb5610be93d703a7f3fc13740dc048f Mon Sep 17 00:00:00 2001 From: John McCann Date: Thu, 10 Feb 2022 20:55:50 -0800 Subject: [PATCH 3/4] docs(PUT /resource): include merge param details --- .secrets.baseline | 4 ++-- docs/openapi.yaml | 22 ++++++++++++++++++---- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/.secrets.baseline b/.secrets.baseline index 95cc6a4..833bcd0 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -3,7 +3,7 @@ "files": "go.sum", "lines": null }, - "generated_at": "2021-11-29T14:09:22Z", + "generated_at": "2022-02-11T04:54:33Z", "plugins_used": [ { "name": "AWSKeyDetector" @@ -62,7 +62,7 @@ { "hashed_secret": "f9fdc64928c96c7ad56bf7da557f70345d83a6ed", "is_verified": false, - "line_number": 1643, + "line_number": 1657, "type": "Base64 High Entropy String" } ], diff --git a/docs/openapi.yaml b/docs/openapi.yaml index ae2b03f..d2733b8 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -383,16 +383,14 @@ paths: schema: $ref: '#/components/schemas/ResourceInput' parameters: - - in: path + - in: query name: p description: >- Ordinarily arborist will return an error if parent resources of a resource you're trying to create do not exist yet. If the `p` parameter is included, it behaves like `mkdir -p` and creates all parent resources as necessary. - schema: - type: integer - required: true + required: false responses: 201: description: JSON representation of successfully-created resource @@ -421,6 +419,22 @@ paths: application/json: schema: $ref: '#/components/schemas/ResourceInput' + parameters: + - in: query + name: p + description: >- + Ordinarily arborist will return an error if parent resources of a + resource you're trying to create do not exist yet. If the `p` + parameter is included, it behaves like `mkdir -p` and creates all + parent resources as necessary. + required: false + - in: query + name: merge + description: >- + Ordinarily arborist will overwrite the resource by deleting + existing subresources. If the `merge` parameter is included, + those existing subresources are not deleted. + required: false responses: 201: description: JSON representation of successfully-updated resource From e7a028bd62a49edbd1b360ec15d243b1f67ba3a0 Mon Sep 17 00:00:00 2001 From: John McCann Date: Mon, 14 Feb 2022 11:35:11 -0800 Subject: [PATCH 4/4] test(PUT /resource?merge): update subresource --- arborist/server_test.go | 63 ++++++++++++++++++++++++++++------------- 1 file changed, 44 insertions(+), 19 deletions(-) diff --git a/arborist/server_test.go b/arborist/server_test.go index 040d952..aa284af 100644 --- a/arborist/server_test.go +++ b/arborist/server_test.go @@ -1113,25 +1113,50 @@ func TestServer(t *testing.T) { {"name": "dog", "subresources": [{"name": "corgi"}]} ] }`)) - w := httptest.NewRecorder() - body := []byte(`{ - "name": "animal", - "subresources": [ - {"name": "bird", "subresources": [{"name": "goose"}]}, - {"name": "cat", "subresources": [{"name": "lion"}]} - ] - }`) - req := newRequest("PUT", "/resource?merge", bytes.NewBuffer(body)) - handler.ServeHTTP(w, req) - if w.Code != http.StatusCreated { - httpError(t, w, "couldn't update resource using PUT") - } - // verify that resources created before update were not overwritten - getResourceWithPath(t, "/animal/dinosaur/pterodactyl") - getResourceWithPath(t, "/animal/dog/corgi") - // verify that update actually created resources - getResourceWithPath(t, "/animal/bird/goose") - getResourceWithPath(t, "/animal/cat/lion") + + t.Run("UpdateTopLevelResource", func(t *testing.T) { + w := httptest.NewRecorder() + body := []byte(`{ + "name": "animal", + "subresources": [ + {"name": "bird", "subresources": [{"name": "goose"}]}, + {"name": "cat", "subresources": [{"name": "lion"}]} + ] + }`) + req := newRequest("PUT", "/resource?merge", bytes.NewBuffer(body)) + handler.ServeHTTP(w, req) + if w.Code != http.StatusCreated { + httpError(t, w, "couldn't update resource using PUT") + } + // verify that resources created before update were not overwritten + getResourceWithPath(t, "/animal/dinosaur/pterodactyl") + getResourceWithPath(t, "/animal/dog/corgi") + // verify that update actually created resources + getResourceWithPath(t, "/animal/bird/goose") + getResourceWithPath(t, "/animal/cat/lion") + }) + + t.Run("UpdateSubresource", func(t *testing.T) { + w := httptest.NewRecorder() + body := []byte(`{ + "path": "/animal/dog", + "subresources": [ + {"name": "goldenretriever", "subresources": []} + ] + }`) + req := newRequest("PUT", "/resource?merge", bytes.NewBuffer(body)) + handler.ServeHTTP(w, req) + if w.Code != http.StatusCreated { + httpError(t, w, "couldn't update resource using PUT") + } + // verify that resources created before update were not overwritten + getResourceWithPath(t, "/animal/dinosaur/pterodactyl") + getResourceWithPath(t, "/animal/dog/corgi") + getResourceWithPath(t, "/animal/bird/goose") + getResourceWithPath(t, "/animal/cat/lion") + // verify that update actually created resource + getResourceWithPath(t, "/animal/dog/goldenretriever") + }) }) t.Run("Delete", func(t *testing.T) {