From 491e9e8e5843a4766d5ea4b003d2cd7e483cd49e Mon Sep 17 00:00:00 2001 From: William Martin Date: Mon, 3 Jul 2023 06:33:49 +0200 Subject: [PATCH] Ensure gist edit request body matches desired schema (#7635) * Ensure gist edit request body matches desired schema * Immediately transform gist to gistToUpdate in edit flow * Remove maps package and change to NewFilename --- pkg/cmd/gist/edit/edit.go | 70 +++++++++++++++++++++++----------- pkg/cmd/gist/edit/edit_test.go | 31 ++------------- 2 files changed, 50 insertions(+), 51 deletions(-) diff --git a/pkg/cmd/gist/edit/edit.go b/pkg/cmd/gist/edit/edit.go index fffcc82ca..f2f04bd92 100644 --- a/pkg/cmd/gist/edit/edit.go +++ b/pkg/cmd/gist/edit/edit.go @@ -147,10 +147,25 @@ func editRun(opts *EditOptions) error { return errors.New("you do not own this gist") } + // Transform our gist into the schema that the update endpoint expects + filesToupdate := make(map[string]*gistFileToUpdate, len(gist.Files)) + for filename, file := range gist.Files { + filesToupdate[filename] = &gistFileToUpdate{ + Content: file.Content, + NewFilename: file.Filename, + } + } + + gistToUpdate := gistToUpdate{ + id: gist.ID, + Description: gist.Description, + Files: filesToupdate, + } + shouldUpdate := false if opts.Description != "" { shouldUpdate = true - gist.Description = opts.Description + gistToUpdate.Description = opts.Description } if opts.AddFilename != "" { @@ -188,18 +203,18 @@ func editRun(opts *EditOptions) error { return err } - gist.Files = files - return updateGist(apiClient, host, gist) + gistToUpdate.Files = files + return updateGist(apiClient, host, gistToUpdate) } // Remove a file from the gist if opts.RemoveFilename != "" { - err := removeFile(gist, opts.RemoveFilename) + err := removeFile(gistToUpdate, opts.RemoveFilename) if err != nil { return err } - return updateGist(apiClient, host, gist) + return updateGist(apiClient, host, gistToUpdate) } filesToUpdate := map[string]string{} @@ -207,7 +222,7 @@ func editRun(opts *EditOptions) error { for { filename := opts.EditFilename candidates := []string{} - for filename := range gist.Files { + for filename := range gistToUpdate.Files { candidates = append(candidates, filename) } @@ -228,7 +243,7 @@ func editRun(opts *EditOptions) error { } } - gistFile, found := gist.Files[filename] + gistFile, found := gistToUpdate.Files[filename] if !found { return fmt.Errorf("gist has no file %q", filename) } @@ -306,28 +321,38 @@ func editRun(opts *EditOptions) error { return nil } - return updateGist(apiClient, host, gist) + return updateGist(apiClient, host, gistToUpdate) } -func updateGist(apiClient *api.Client, hostname string, gist *shared.Gist) error { - body := shared.Gist{ - Description: gist.Description, - Files: gist.Files, - } +// https://docs.github.com/en/rest/gists/gists?apiVersion=2022-11-28#update-a-gist +type gistToUpdate struct { + // The id of the gist to update. Does not get marshalled to JSON. + id string + // The description of the gist + Description string `json:"description"` + // The gist files to be updated, renamed or deleted. The key must match the current + // filename of the file to change. To delete a file, set the value to nil + Files map[string]*gistFileToUpdate `json:"files"` +} - path := "gists/" + gist.ID +type gistFileToUpdate struct { + // The new content of the file + Content string `json:"content"` + // The new name for the file + NewFilename string `json:"filename,omitempty"` +} - requestByte, err := json.Marshal(body) +func updateGist(apiClient *api.Client, hostname string, gist gistToUpdate) error { + requestByte, err := json.Marshal(gist) if err != nil { return err } requestBody := bytes.NewReader(requestByte) - result := shared.Gist{} + path := "gists/" + gist.id err = apiClient.REST(hostname, "POST", path, requestBody, &result) - if err != nil { return err } @@ -335,7 +360,7 @@ func updateGist(apiClient *api.Client, hostname string, gist *shared.Gist) error return nil } -func getFilesToAdd(file string, content []byte) (map[string]*shared.GistFile, error) { +func getFilesToAdd(file string, content []byte) (map[string]*gistFileToUpdate, error) { if shared.IsBinaryContents(content) { return nil, fmt.Errorf("failed to upload %s: binary file not supported", file) } @@ -345,20 +370,19 @@ func getFilesToAdd(file string, content []byte) (map[string]*shared.GistFile, er } filename := filepath.Base(file) - return map[string]*shared.GistFile{ + return map[string]*gistFileToUpdate{ filename: { - Filename: filename, - Content: string(content), + NewFilename: filename, + Content: string(content), }, }, nil } -func removeFile(gist *shared.Gist, filename string) error { +func removeFile(gist gistToUpdate, filename string) error { if _, found := gist.Files[filename]; !found { return fmt.Errorf("gist has no file %q", filename) } gist.Files[filename] = nil - return nil } diff --git a/pkg/cmd/gist/edit/edit_test.go b/pkg/cmd/gist/edit/edit_test.go index 481dd0a9c..b57fa41cc 100644 --- a/pkg/cmd/gist/edit/edit_test.go +++ b/pkg/cmd/gist/edit/edit_test.go @@ -26,10 +26,10 @@ func Test_getFilesToAdd(t *testing.T) { gf, err := getFilesToAdd(filename, []byte("hello")) require.NoError(t, err) - assert.Equal(t, map[string]*shared.GistFile{ + assert.Equal(t, map[string]*gistFileToUpdate{ filename: { - Filename: filename, - Content: "hello", + NewFilename: filename, + Content: "hello", }, }, gf) } @@ -173,13 +173,10 @@ func Test_editRun(t *testing.T) { }, wantParams: map[string]interface{}{ "description": "", - "updated_at": "0001-01-01T00:00:00Z", - "public": false, "files": map[string]interface{}{ "cicada.txt": map[string]interface{}{ "content": "new file content", "filename": "cicada.txt", - "type": "text/plain", }, }, }, @@ -205,12 +202,10 @@ func Test_editRun(t *testing.T) { "cicada.txt": { Filename: "cicada.txt", Content: "bwhiizzzbwhuiiizzzz", - Type: "text/plain", }, "unix.md": { Filename: "unix.md", Content: "meow", - Type: "application/markdown", }, }, Owner: &shared.GistOwner{Login: "octocat"}, @@ -221,18 +216,14 @@ func Test_editRun(t *testing.T) { }, wantParams: map[string]interface{}{ "description": "catbug", - "updated_at": "0001-01-01T00:00:00Z", - "public": false, "files": map[string]interface{}{ "cicada.txt": map[string]interface{}{ "content": "bwhiizzzbwhuiiizzzz", "filename": "cicada.txt", - "type": "text/plain", }, "unix.md": map[string]interface{}{ "content": "new file content", "filename": "unix.md", - "type": "application/markdown", }, }, }, @@ -341,13 +332,10 @@ func Test_editRun(t *testing.T) { }, wantParams: map[string]interface{}{ "description": "my new description", - "updated_at": "0001-01-01T00:00:00Z", - "public": false, "files": map[string]interface{}{ "sample.txt": map[string]interface{}{ "content": "new file content", "filename": "sample.txt", - "type": "text/plain", }, }, }, @@ -375,8 +363,6 @@ func Test_editRun(t *testing.T) { }, wantParams: map[string]interface{}{ "description": "", - "updated_at": "0001-01-01T00:00:00Z", - "public": false, "files": map[string]interface{}{ "from_source.txt": map[string]interface{}{ "content": "hello", @@ -409,8 +395,6 @@ func Test_editRun(t *testing.T) { stdin: "data from stdin", wantParams: map[string]interface{}{ "description": "", - "updated_at": "0001-01-01T00:00:00Z", - "public": false, "files": map[string]interface{}{ "from_source.txt": map[string]interface{}{ "content": "data from stdin", @@ -464,13 +448,10 @@ func Test_editRun(t *testing.T) { }, wantParams: map[string]interface{}{ "description": "", - "updated_at": "0001-01-01T00:00:00Z", - "public": false, "files": map[string]interface{}{ "sample.txt": map[string]interface{}{ "filename": "sample.txt", "content": "bwhiizzzbwhuiiizzzz", - "type": "text/plain", }, "sample2.txt": nil, }, @@ -498,13 +479,10 @@ func Test_editRun(t *testing.T) { }, wantParams: map[string]interface{}{ "description": "", - "updated_at": "0001-01-01T00:00:00Z", - "public": false, "files": map[string]interface{}{ "sample.txt": map[string]interface{}{ "content": "hello", "filename": "sample.txt", - "type": "text/plain", }, }, }, @@ -532,13 +510,10 @@ func Test_editRun(t *testing.T) { stdin: "data from stdin", wantParams: map[string]interface{}{ "description": "", - "updated_at": "0001-01-01T00:00:00Z", - "public": false, "files": map[string]interface{}{ "sample.txt": map[string]interface{}{ "content": "data from stdin", "filename": "sample.txt", - "type": "text/plain", }, }, },