Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PL-24913]: Handle the error raised while creating a multipart input #181

Merged
merged 6 commits into from
May 12, 2022
Merged

[PL-24913]: Handle the error raised while creating a multipart input #181

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a290feb
[PL-24913]: Incorporate the review comments
DeepakPatankar May 11, 2022
70e5b51
[PL-24913]: Incorporate the review comments
DeepakPatankar May 11, 2022
8e64e65
[PL-25913]: Fix the issue when we are doing null check
DeepakPatankar May 11, 2022
e3467d9
[PL-24913]: Move the multipart to stash
DeepakPatankar May 11, 2022
7214157
[PL-24913]: Move the multipart to stash
DeepakPatankar May 11, 2022
9abffdf
[PL-24913]: Fix the compilation issue
DeepakPatankar May 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 11 additions & 8 deletions scm/driver/stash/stash.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,26 +79,29 @@ func (c *wrapper) do(ctx context.Context, method, path string, in, out interface
case *contentCreateUpdate:
// add the content to the multipart
var b bytes.Buffer
w := multipart.NewWriter(&b)
mw := &scm.MultipartWriter{Writer: multipart.NewWriter(&b)}
// add the other fields
if content.Message != "" {
_ = w.WriteField("content", string(content.Content))
if string(content.Content) != "" {
tphoney marked this conversation as resolved.
Show resolved Hide resolved
mw.Write("content", string(content.Content))
}
if content.Message != "" {
_ = w.WriteField("message", content.Message)
mw.Write("message", content.Message)
}
if content.Branch != "" {
_ = w.WriteField("branch", content.Branch)
mw.Write("branch", content.Branch)
}
if content.Sha != "" {
_ = w.WriteField("sourceCommitId", content.Sha)
mw.Write("sourceCommitId", content.Sha)
}
if mw.Error != nil {
return nil, fmt.Errorf("error writing multipart-content. err: %s", mw.err)
}
w.Close()
mw.Close()
// write the multipart response to the body
req.Body = &b
// write the content type that contains the length of the multipart
req.Header = map[string][]string{
"Content-Type": {w.FormDataContentType()},
"Content-Type": {mw.FormDataContentType()},
}
default:
buf := new(bytes.Buffer)
Expand Down
26 changes: 26 additions & 0 deletions scm/multipart.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package scm
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this functionality is only used by stash/content. I would move this helper method in under stash
or
into scm/driver/stash/stash.go

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since most of the providers uses multipart requests only, the idea is to use this helper function in new APIs. Hence I kept it at a top level

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

➜  go-scm git:(no_project) grep -irsn Multipart .
./scm/driver/bitbucket/bitbucket.go:13: "mime/multipart"
./scm/driver/bitbucket/bitbucket.go:74:                 // add the content to the multipart
./scm/driver/bitbucket/bitbucket.go:77:                 w := multipart.NewWriter(&b)
./scm/driver/bitbucket/bitbucket.go:95:                 // write the multipart response to the body
./scm/driver/bitbucket/bitbucket.go:97:                 // write the content type that contains the length of the multipart
./scm/driver/bitbucket/bitbucket.go:103:                        writer := multipart.NewWriter(body)

nothing else is using it yet. I would definitely make it only for stash for now. Until another git provider uses it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


import "mime/multipart"

func (mw *MultipartWriter) Write(f, v string) {
if mw.Error == nil {
return
}
if v == "" {
tphoney marked this conversation as resolved.
Show resolved Hide resolved
return
}
mw.Error = mw.Writer.WriteField(f, v)
}

func (mw *MultipartWriter) Close() {
mw.Writer.Close()
}

func (mw *MultipartWriter) FormDataContentType() string {
return mw.Writer.FormDataContentType()
}

type MultipartWriter struct {
tphoney marked this conversation as resolved.
Show resolved Hide resolved
Writer *multipart.Writer
Error error
}