Custom data for multipart request #129

Asker80 · 2018-02-20T09:41:13Z

Proposal for MultipartCustomData type, in case there's a need to inject custom data into multipart request. Since SetFile() and SetFileReader() functions are specifically file-oriented, I created a separate type to hold Content-Disposition header fields, Content-Type and the actual data.

codecov · 2018-02-20T10:26:03Z

Codecov Report

Merging #129 into master will decrease coverage by 0.3%.
The diff coverage is 90%.

@@            Coverage Diff             @@
##           master     #129      +/-   ##
==========================================
- Coverage   96.32%   96.02%   -0.31%     
==========================================
  Files          10       10              
  Lines        1034     1056      +22     
==========================================
+ Hits          996     1014      +18     
- Misses         21       23       +2     
- Partials       17       19       +2

Impacted Files	Coverage Δ
request17.go	`100% <ø> (ø)`	⬆️
request16.go	`100% <ø> (ø)`	⬆️
client.go	`97.71% <100%> (ø)`	⬆️
request.go	`100% <100%> (ø)`	⬆️
middleware.go	`91.32% <50%> (-0.77%)`	⬇️
util.go	`88.88% <84.61%> (-0.95%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cf9fbbb...e078baa. Read the comment docs.

jeevatkm · 2018-02-22T04:33:43Z

@Asker80 Thanks for the PR, I will have a look and get back.

jeevatkm

@Asker80 Thank you for your PR and idea to improve resty.

Please have a look on comments and make updates.
Also please add test cases into request_test.go for this implementation.
Bring PR into Green tick status

jeevatkm · 2018-02-22T19:14:02Z

client.go

@@ -865,3 +866,10 @@ type File struct {
 func (f *File) String() string {
 	return fmt.Sprintf("ParamName: %v; FileName: %v", f.ParamName, f.Name)
 }
+
+// MultipartCustomData represent custom data part for multipart request
+type MultipartCustomData struct {


@Asker80 Change struct name to multipartField. Basically making as unexported struct.

jeevatkm · 2018-02-22T19:15:46Z

client.go

+
+// MultipartCustomData represent custom data part for multipart request
+type MultipartCustomData struct {
+	Params      map[string]string


@Asker80 Remove map and add two fields-

Param string FileName string

jeevatkm · 2018-02-22T19:24:05Z

request.go

@@ -279,6 +279,19 @@ func (r *Request) SetFileReader(param, fileName string, reader io.Reader) *Reque
 	return r
 }

+// SetMultipartData method is to set custom data using io.Reader for multipart upload.
+func (r *Request) SetMultipartData(params map[string]string, contentType string, reader io.Reader) *Request {


@Asker80 Change method name to SetMultipartField. Your idea, it supports multiple fields.

Also update your method signature to

func (r *Request) SetMultipartField(param, fileName, contentType string, reader io.Reader) *Request {

jeevatkm · 2018-02-22T19:25:08Z

request16.go

@@ -43,6 +43,7 @@ type Request struct {
 	isSaveResponse      bool
 	outputFile          string
 	multipartFiles      []*File
+	multipartCustomData []*MultipartCustomData


@Asker80 Change attribute name to multipartFields

jeevatkm · 2018-02-22T19:25:21Z

request17.go

@@ -44,6 +44,7 @@ type Request struct {
 	isSaveResponse      bool
 	outputFile          string
 	multipartFiles      []*File
+	multipartCustomData []*MultipartCustomData


@Asker80 Change attribute name to multipartFields

jeevatkm · 2018-02-22T19:46:39Z

util.go

@@ -107,6 +107,24 @@ func escapeQuotes(s string) string {
 	return quoteEscaper.Replace(s)
 }

+func writeMultipartFormCustomData(w *multipart.Writer, params map[string]string, contentType string, r io.Reader) error {


@Asker80 We are gonna improve it further.

Create a new unexported method called createMultipartHeader

func createMultipartHeader(param, fileName, contentType string) textproto.MIMEHeader { hdr := make(textproto.MIMEHeader) hdr.Set("Content-Disposition", fmt.Sprintf(`form-data; name="%s"; filename="%s"`, escapeQuotes(param), escapeQuotes(fileName))) hdr.Set("Content-Type", contentType) return hdr }

Change method signature to

func addMultipartFormField(w *multipart.Writer, mf *multipartField) error {

Call method createMultipartHeader from method addMultipartFormField to get part header

End result may look like-

func addMultipartFormField(w *multipart.Writer, mf *multipartField) error { partWriter, err := w.CreatePart(createMultipartHeader(mf.Param, mf.FileName, mf.ContentType)) if err != nil { return err } _, err = io.Copy(partWriter, r) return err }

Optional:

Call createMultipartHeader from method writeMultipartFormFile. Otherwise I will take care after your PR merge.

jeevatkm · 2018-02-22T19:47:21Z

util.go

@@ -148,6 +166,10 @@ func addFileReader(w *multipart.Writer, f *File) error {
 	return writeMultipartFormFile(w, f.ParamName, f.Name, f.Reader)
 }

+func addCustomDataReader(w *multipart.Writer, cd *MultipartCustomData) error {


@Asker80 Please remove it, this method not needed after you take care of comments.

jeevatkm · 2018-02-22T19:52:16Z

middleware.go

@@ -300,6 +300,16 @@ func handleMultipart(c *Client, r *Request) (err error) {
 		}
 	}

+	// adding custom data support
+	if len(r.multipartCustomData) > 0 {


@Asker80 Based review comments/recommendation. This section will change to

// GitHub #130 adding multipart field support with content type if len(r.multipartFields) > 0 { for _, mf := range r.multipartFields { if err = addMultipartFormField(w, mf); err != nil { return } } }

jeevatkm · 2018-02-22T19:54:29Z

client.go

+		client:              c,
+		bodyBuf:             nil,
+		multipartFiles:      []*File{},
+		multipartCustomData: []*MultipartCustomData{},


@Asker80 Based on review comments/recommendation, please make appropriate updates.

jeevatkm · 2018-02-24T05:56:18Z

@Asker80 Thanks for the updates. I'm merging it.

Asker80 added 2 commits February 19, 2018 21:31

Add MultipartCustomData type to allow custom data for multipart request

162926e

Add MultipartCustomData type to allow custom data for multipart request

72cb41a

Asker80 mentioned this pull request Feb 20, 2018

custom content type and data for multipart request #130

Closed

Forgot about request16.go)

d25c26b

jeevatkm requested changes Feb 22, 2018

View reviewed changes

Asker80 and others added 3 commits February 24, 2018 15:19

Changes as requested

3707524

Merge branch 'master' into custom-data-for-multipart-request

a7f86e7

Changes after update from master

e078baa

jeevatkm approved these changes Feb 24, 2018

View reviewed changes

jeevatkm merged commit e008fe0 into go-resty:master Feb 24, 2018

Asker80 deleted the custom-data-for-multipart-request branch February 24, 2018 06:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Custom data for multipart request #129

Custom data for multipart request #129

Asker80 commented Feb 20, 2018

codecov bot commented Feb 20, 2018 •

edited

Loading

jeevatkm commented Feb 22, 2018

jeevatkm left a comment

jeevatkm Feb 22, 2018

jeevatkm Feb 22, 2018

jeevatkm Feb 22, 2018

jeevatkm Feb 22, 2018

jeevatkm Feb 22, 2018

jeevatkm Feb 22, 2018

jeevatkm Feb 22, 2018

jeevatkm Feb 22, 2018

jeevatkm Feb 22, 2018

jeevatkm commented Feb 24, 2018

Custom data for multipart request #129

Custom data for multipart request #129

Conversation

Asker80 commented Feb 20, 2018

codecov bot commented Feb 20, 2018 • edited Loading

Codecov Report

jeevatkm commented Feb 22, 2018

jeevatkm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeevatkm commented Feb 24, 2018

codecov bot commented Feb 20, 2018 •

edited

Loading