PXD-3049: force create resource

arborist/resource.go

@@ -114,6 +114,7 @@ func (resourceFromQuery *ResourceFromQuery) standardize() ResourceOut {
 //
 //     formatDbPath("/a/b/c") == "a.b.c"
 func formatPathForDb(path string) string {
+	// -1 means replace everything
 	return strings.TrimLeft(strings.Replace(path, "/", ".", -1), ".")
 }
 
@@ -122,6 +123,7 @@ func formatPathForDb(path string) string {
 //
 //     formatDbPath("a.b.c") == "/a/b/c"
 func formatDbPath(path string) string {
+	// -1 means replace everything
 	return "/" + strings.Replace(path, ".", "/", -1)
 }
 
@@ -248,42 +250,20 @@ func (resource *ResourceIn) validate() *ErrorResponse {
 	return nil
 }
 
-func (resource *ResourceIn) createInDb(db *sqlx.DB) (*ResourceFromQuery, *ErrorResponse) {
+func (resource *ResourceIn) createInDb(tx *sqlx.Tx) *ErrorResponse {
 	errResponse := resource.validate()
 	if errResponse != nil {
-		return nil, errResponse
-	}
-	tx, err := db.Beginx()
-	if err != nil {
-		msg := fmt.Sprintf("couldn't open database transaction: %s", err.Error())
-		return nil, newErrorResponse(msg, 500, &err)
+		return errResponse
 	}
 	errResponse = resource.createRecursively(tx)
 	if errResponse != nil {
-		return nil, errResponse
-	}
-	err = tx.Commit()
-	if err != nil {
-		_ = tx.Rollback()
-		// TODO: more specific error handling (make sure resource really is
-		// invalid)
-
-		// assume that this error is because the resource failed validation on
-		// database side, because of missing parent or similar; return 400.
-		errMsg := strings.TrimPrefix(err.Error(), "pq: ")
-		msg := fmt.Sprintf("couldn't create resource: %s", errMsg)
-		return nil, newErrorResponse(msg, 400, &err)
+		return errResponse
 	}
-	resourceFromQuery, err := resourceWithPath(db, resource.Path)
-	if err != nil {
-		return nil, newErrorResponse(err.Error(), 500, &err)
-	}
-	return resourceFromQuery, nil
+	return nil
 }
 
 func (resource *ResourceIn) createRecursively(tx *sqlx.Tx) *ErrorResponse {
 	// arborist uses `/` for path separator; ltree in postgres uses `.`
-	// -1 means replace everything
 	path := formatPathForDb(resource.Path)
 	if resource.Name == "" {
 		segments := strings.Split(path, ".")
@@ -314,16 +294,24 @@ func (resource *ResourceIn) createRecursively(tx *sqlx.Tx) *ErrorResponse {
 	return nil
 }
 
-func (resource *ResourceIn) deleteInDb(db *sqlx.DB) *ErrorResponse {
+func (resource *ResourceIn) deleteInDb(tx *sqlx.Tx) *ErrorResponse {
 	if resource.Path == "" {
 		msg := "resource missing required field `path`"
 		return newErrorResponse(msg, 400, nil)
 	}
 	stmt := "DELETE FROM resource WHERE path = $1"
-	_, err := db.Exec(stmt, formatPathForDb(resource.Path))
+	_, err := tx.Exec(stmt, formatPathForDb(resource.Path))
 	if err != nil {
 		// resource already doesn't exist; this is fine
 		return nil
 	}
 	return nil
 }
+
+func (resource *ResourceIn) overwriteInDb(tx *sqlx.Tx) *ErrorResponse {
+	errResponse := resource.deleteInDb(tx)
+	if errResponse != nil {
+		return errResponse
+	}
+	return resource.createInDb(tx)
+}

arborist/server.go

@@ -96,10 +96,10 @@ func (server *Server) MakeRouter(out io.Writer) http.Handler {
 	router.Handle("/policy/{policyID}", http.HandlerFunc(server.handlePolicyDelete)).Methods("DELETE")
 
 	router.Handle("/resource", http.HandlerFunc(server.handleResourceList)).Methods("GET")
-	router.Handle("/resource", http.HandlerFunc(server.parseJSON(server.handleResourceCreate))).Methods("POST")
+	router.Handle("/resource", http.HandlerFunc(server.parseJSON(server.handleResourceCreate))).Methods("POST", "PUT")
 	router.Handle("/resource/tag/{tag}", http.HandlerFunc(server.handleResourceReadByTag)).Methods("GET")
 	router.Handle("/resource"+resourcePath, http.HandlerFunc(server.handleResourceRead)).Methods("GET")
-	router.Handle("/resource"+resourcePath, http.HandlerFunc(server.parseJSON(server.handleSubresourceCreate))).Methods("POST")
+	router.Handle("/resource"+resourcePath, http.HandlerFunc(server.parseJSON(server.handleSubresourceCreate))).Methods("POST", "PUT")
 	router.Handle("/resource"+resourcePath, http.HandlerFunc(server.handleResourceDelete)).Methods("DELETE")
 
 	router.Handle("/role", http.HandlerFunc(server.handleRoleList)).Methods("GET")
@@ -639,12 +639,24 @@ func (server *Server) handleResourceCreate(w http.ResponseWriter, r *http.Reques
 		server.handleSubresourceCreate(w, r, body)
 		return
 	}
-	resourceFromQuery, errResponse := resource.createInDb(server.db)
+	var errResponse *ErrorResponse
+	if r.Method == "PUT" {
+		errResponse = transactify(server.db, resource.overwriteInDb)
+	} else {
+		errResponse = transactify(server.db, resource.createInDb)
+	}
 	if errResponse != nil {
 		errResponse.log.write(server.logger)
 		_ = errResponse.write(w, r)
 		return
 	}
+	resourceFromQuery, err := resourceWithPath(server.db, resource.Path)
+	if err != nil {
+		errResponse := newErrorResponse(err.Error(), 500, &err)
+		errResponse.log.write(server.logger)
+		_ = errResponse.write(w, r)
+		return
+	}
 	created := resourceFromQuery.standardize()
 	server.logger.Info("created resource %s (%s)", created.Path, created.Tag)
 	result := struct {
@@ -674,12 +686,25 @@ func (server *Server) handleSubresourceCreate(w http.ResponseWriter, r *http.Req
 	}
 	parentPath := parseResourcePath(r)
 	resource.Path = parentPath + "/" + resource.Name
-	resourceFromQuery, errResponse := resource.createInDb(server.db)
+
+	var errResponse *ErrorResponse
+	if r.Method == "PUT" {
+		errResponse = transactify(server.db, resource.overwriteInDb)
+	} else {
+		errResponse = transactify(server.db, resource.createInDb)
+	}
 	if errResponse != nil {
 		errResponse.log.write(server.logger)
 		_ = errResponse.write(w, r)
 		return
 	}
+	resourceFromQuery, err := resourceWithPath(server.db, resource.Path)
+	if err != nil {
+		errResponse := newErrorResponse(err.Error(), 500, &err)
+		errResponse.log.write(server.logger)
+		_ = errResponse.write(w, r)
+		return
+	}
 	created := resourceFromQuery.standardize()
 	server.logger.Info("created subresource %s (%s)", created.Path, created.Tag)
 	result := struct {
@@ -733,7 +758,7 @@ func (server *Server) handleResourceReadByTag(w http.ResponseWriter, r *http.Req
 func (server *Server) handleResourceDelete(w http.ResponseWriter, r *http.Request) {
 	path := parseResourcePath(r)
 	resource := ResourceIn{Path: path}
-	errResponse := resource.deleteInDb(server.db)
+	errResponse := transactify(server.db, resource.deleteInDb)
 	if errResponse != nil {
 		errResponse.log.write(server.logger)
 		_ = errResponse.write(w, r)

arborist/server_test.go

@@ -720,6 +720,66 @@ func TestServer(t *testing.T) {
 			if w.Code != http.StatusOK {
 				httpError(t, w, "couldn't find subresource")
 			}
+
+			// re-POST the same x with different subresources should fail
+			w = httptest.NewRecorder()
+			body = []byte(`{
+				"name": "x",
+				"subresources": [
+					{
+						"name": "b",
+						"subresources": [{"name": "c"}]
+					}
+				]
+			}`)
+			req = newRequest("POST", "/resource", bytes.NewBuffer(body))
+			handler.ServeHTTP(w, req)
+			if w.Code != http.StatusConflict {
+				httpError(t, w, "didn't conflict")
+			}
+
+			// use PUT (force-create) shall recreate the whole tree under x
+			w = httptest.NewRecorder()
+			req = newRequest("PUT", "/resource", bytes.NewBuffer(body))
+			handler.ServeHTTP(w, req)
+			if w.Code != http.StatusCreated {
+				httpError(t, w, "couldn't create resource")
+			}
+			expected = struct {
+				_ interface{} `json:"created"`
+			}{}
+			err = json.Unmarshal(w.Body.Bytes(), &expected)
+			if err != nil {
+				httpError(t, w, "couldn't read response from resource creation")
+			}
+
+			// previous child resources should be gone
+			w = httptest.NewRecorder()
+			req = newRequest("GET", "/resource/x/y", nil)
+			handler.ServeHTTP(w, req)
+			if w.Code != http.StatusNotFound {
+				httpError(t, w, "could find subresource")
+			}
+			w = httptest.NewRecorder()
+			req = newRequest("GET", "/resource/x/y/z", nil)
+			handler.ServeHTTP(w, req)
+			if w.Code != http.StatusNotFound {
+				httpError(t, w, "could find subresource")
+			}
+
+			// now check that the new child resources exist
+			w = httptest.NewRecorder()
+			req = newRequest("GET", "/resource/x/b", nil)
+			handler.ServeHTTP(w, req)
+			if w.Code != http.StatusOK {
+				httpError(t, w, "couldn't find subresource")
+			}
+			w = httptest.NewRecorder()
+			req = newRequest("GET", "/resource/x/b/c", nil)
+			handler.ServeHTTP(w, req)
+			if w.Code != http.StatusOK {
+				httpError(t, w, "couldn't find subresource")
+			}
 		})
 
 		t.Run("ListSubresources", func(t *testing.T) {

docs/openapi.yaml

@@ -181,14 +181,14 @@ paths:
       description: >-
         Overwrite an existing resource. This endpoint requires a fully-formed
         resource model (and cannot patch over individual fields on the existing
-        resources).
+        resources). If the specified resource doesn't exist, it will be created.
       requestBody:
         content:
           application/json:
             schema:
-              $ref: '#/components/schemas/Resource'
+              $ref: '#/components/schemas/ResourceInput'
       responses:
-        200:
+        201:
           description: JSON representation of successfully-updated resource
           content:
             application/json:
@@ -203,12 +203,6 @@ paths:
             application/json:
               schema:
                 $ref: '#/components/schemas/UserError'
-        404:
-          description: no resource exists with the given `resourcePath`
-          content:
-            application/json:
-              schema:
-                $ref: '#/components/schemas/NotFound'
     delete:
       tags:
         - resource