From 3f5ef8bebd5a7bd676f6de4900b559e648f0d973 Mon Sep 17 00:00:00 2001 From: dojolew Date: Sat, 15 Jul 2023 09:37:48 +0100 Subject: [PATCH 1/6] Add image upload by url --- images.go | 77 +++++++++++++++++++++++++++++++++++++ images_test.go | 101 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 178 insertions(+) diff --git a/images.go b/images.go index b93b2f46de5..281f0537867 100644 --- a/images.go +++ b/images.go @@ -84,6 +84,43 @@ func (b UploadImageParams) write(mpw *multipart.Writer) error { return nil } +// UploadImageByUrlParams is the data required for an Image Upload by URL request. +type UploadImageByUrlParams struct { + Url string + RequireSignedURLs bool + Metadata map[string]interface{} +} + +func (b UploadImageByUrlParams) write(mpw *multipart.Writer) error { + err := mpw.WriteField("url", b.Url) + if err != nil { + return err + } + + // According to the Cloudflare docs, this field defaults to false. + // For simplicity, we will only send it if the value is true, however + // if the default is changed to true, this logic will need to be updated. + if b.RequireSignedURLs { + err = mpw.WriteField("requireSignedURLs", "true") + if err != nil { + return err + } + } + + if b.Metadata != nil { + part, err := mpw.CreateFormField("metadata") + if err != nil { + return err + } + err = json.NewEncoder(part).Encode(b.Metadata) + if err != nil { + return err + } + } + + return nil +} + // UpdateImageParams is the data required for an UpdateImage request. type UpdateImageParams struct { ID string `json:"-"` @@ -183,6 +220,46 @@ func (api *API) UploadImage(ctx context.Context, rc *ResourceContainer, params U return imageDetailsResponse.Result, nil } +// UploadImageByUrl uploads a single image from a given url. +// +// API Reference: https://api.cloudflare.com/#cloudflare-images-upload-an-image-using-a-single-http-request +func (api *API) UploadImageByUrl(ctx context.Context, rc *ResourceContainer, params UploadImageByUrlParams) (Image, error) { + if rc.Level != AccountRouteLevel { + return Image{}, ErrRequiredAccountLevelResourceContainer + } + + uri := fmt.Sprintf("/accounts/%s/images/v1", rc.Identifier) + + body := &bytes.Buffer{} + w := multipart.NewWriter(body) + if err := params.write(w); err != nil { + _ = w.Close() + return Image{}, fmt.Errorf("error writing multipart body: %w", err) + } + _ = w.Close() + + res, err := api.makeRequestContextWithHeaders( + ctx, + http.MethodPost, + uri, + body, + http.Header{ + "Accept": []string{"application/json"}, + "Content-Type": []string{w.FormDataContentType()}, + }, + ) + if err != nil { + return Image{}, err + } + + var imageDetailsResponse ImageDetailsResponse + err = json.Unmarshal(res, &imageDetailsResponse) + if err != nil { + return Image{}, fmt.Errorf("%s: %w", errUnmarshalError, err) + } + return imageDetailsResponse.Result, nil +} + // UpdateImage updates an existing image's metadata. // // API Reference: https://api.cloudflare.com/#cloudflare-images-update-image diff --git a/images_test.go b/images_test.go index 3640d9f3131..d4fa864838f 100644 --- a/images_test.go +++ b/images_test.go @@ -96,6 +96,61 @@ func TestUploadImage(t *testing.T) { } } +func TestUploadImageByUrl(t *testing.T) { + setup() + defer teardown() + + handler := func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, http.MethodPost, r.Method, "Expected method 'POST', got %s", r.Method) + + u, err := parseImageMultipartUploadByUrl(r) + if !assert.NoError(t, err) { + w.WriteHeader(http.StatusBadRequest) + return + } + assert.Equal(t, u.RequireSignedURLs, true) + assert.Equal(t, u.Metadata, map[string]interface{}{"meta": "metaID"}) + assert.Equal(t, u.Url, "https://www.images-elsewhere.com/avatar.png") + + w.Header().Set("content-type", "application/json") + fmt.Fprintf(w, `{ + "success": true, + "errors": [], + "messages": [], + "result": { + "id": "ZxR0pLaXRldlBtaFhhO2FiZGVnaA", + "filename": "avatar.png", + "metadata": { + "meta": "metaID" + }, + "requireSignedURLs": true, + "variants": [ + "https://imagedelivery.net/MTt4OTd0b0w5aj/ZxR0pLaXRldlBtaFhhO2FiZGVnaA/hero", + "https://imagedelivery.net/MTt4OTd0b0w5aj/ZxR0pLaXRldlBtaFhhO2FiZGVnaA/original", + "https://imagedelivery.net/MTt4OTd0b0w5aj/ZxR0pLaXRldlBtaFhhO2FiZGVnaA/thumbnail" + ], + "uploaded": "2014-01-02T02:20:00Z" + } + } + `) + } + + mux.HandleFunc("/accounts/"+testAccountID+"/images/v1", handler) + want := expectedImageStruct + + actual, err := client.UploadImageByUrl(context.Background(), AccountIdentifier(testAccountID), UploadImageByUrlParams{ + Url: "https://www.images-elsewhere.com/avatar.png", + RequireSignedURLs: true, + Metadata: map[string]interface{}{ + "meta": "metaID", + }, + }) + + if assert.NoError(t, err) { + assert.Equal(t, want, actual) + } +} + func TestUpdateImage(t *testing.T) { setup() defer teardown() @@ -432,6 +487,52 @@ func parseImageMultipartUpload(r *http.Request) (imageMultipartUpload, error) { return u, nil } +type imageMultipartUploadByUrl struct { + Url string + RequireSignedURLs bool + Metadata map[string]interface{} +} + +func parseImageMultipartUploadByUrl(r *http.Request) (imageMultipartUploadByUrl, error) { + var u imageMultipartUploadByUrl + mdBytes, err := getImageFormValue(r, "metadata") + if err != nil { + if !strings.HasPrefix(err.Error(), "no value found for key") { + return u, err + } + } + if mdBytes != nil { + err = json.Unmarshal(mdBytes, &u.Metadata) + if err != nil { + return u, err + } + } + + rsuBytes, err := getImageFormValue(r, "requireSignedURLs") + if err != nil { + if !strings.HasPrefix(err.Error(), "no value found for key") { + return u, err + } + } + if rsuBytes != nil { + if bytes.Equal(rsuBytes, []byte("true")) { + u.RequireSignedURLs = true + } + } + + urlBytes, err := getImageFormValue(r, "url") + if err != nil { + if !strings.HasPrefix(err.Error(), "no value found for key") { + return u, err + } + } + if urlBytes != nil { + u.Url = string(urlBytes) + } + + return u, nil +} + // See getFormValue for more information, the only difference between // getFormValue and this one is the max memory. func getImageFormValue(r *http.Request, key string) ([]byte, error) { From fcecaaad0c0de098be4d5454712b516c8342fe0f Mon Sep 17 00:00:00 2001 From: lewisyearsley Date: Sat, 15 Jul 2023 10:59:59 +0100 Subject: [PATCH 2/6] changelog --- .changelog/1336.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/1336.txt diff --git a/.changelog/1336.txt b/.changelog/1336.txt new file mode 100644 index 00000000000..13b520854ae --- /dev/null +++ b/.changelog/1336.txt @@ -0,0 +1,3 @@ +```release-note:enhancement +images: adds ability to upload image by url +``` From 8c5b7b7203520e902045b274746279f22d9e941b Mon Sep 17 00:00:00 2001 From: lewisyearsley Date: Sat, 15 Jul 2023 11:08:48 +0100 Subject: [PATCH 3/6] correct changelog name --- .changelog/{1336.txt => 1335.txt} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename .changelog/{1336.txt => 1335.txt} (100%) diff --git a/.changelog/1336.txt b/.changelog/1335.txt similarity index 100% rename from .changelog/1336.txt rename to .changelog/1335.txt From 75b6a902a2ad2dd06dc0f1662ada1d85bfb2da63 Mon Sep 17 00:00:00 2001 From: lewisyearsley Date: Mon, 17 Jul 2023 08:54:43 +0100 Subject: [PATCH 4/6] combine image upload methods into one --- images.go | 94 ++++++++------------------------------------------ images_test.go | 67 +++++++++-------------------------- 2 files changed, 31 insertions(+), 130 deletions(-) diff --git a/images.go b/images.go index 281f0537867..ed2d331a207 100644 --- a/images.go +++ b/images.go @@ -37,6 +37,7 @@ type Image struct { // UploadImageParams is the data required for an Image Upload request. type UploadImageParams struct { File io.ReadCloser + Url string Name string RequireSignedURLs bool Metadata map[string]interface{} @@ -45,63 +46,36 @@ type UploadImageParams struct { // write writes the image upload data to a multipart writer, so // it can be used in an HTTP request. func (b UploadImageParams) write(mpw *multipart.Writer) error { - if b.File == nil { - return errors.New("a file to upload must be specified") + if b.File == nil && b.Url == "" { + return errors.New("a file or url to upload must be specified") } - name := b.Name - part, err := mpw.CreateFormFile("file", name) - if err != nil { - return err - } - _, err = io.Copy(part, b.File) - if err != nil { - _ = b.File.Close() - return err - } - _ = b.File.Close() - // According to the Cloudflare docs, this field defaults to false. - // For simplicity, we will only send it if the value is true, however - // if the default is changed to true, this logic will need to be updated. - if b.RequireSignedURLs { - err = mpw.WriteField("requireSignedURLs", "true") + if b.File != nil { + name := b.Name + part, err := mpw.CreateFormFile("file", name) if err != nil { return err } - } - - if b.Metadata != nil { - part, err = mpw.CreateFormField("metadata") + _, err = io.Copy(part, b.File) if err != nil { + _ = b.File.Close() return err } - err = json.NewEncoder(part).Encode(b.Metadata) + _ = b.File.Close() + } + + if b.Url != "" { + err := mpw.WriteField("url", b.Url) if err != nil { return err } } - return nil -} - -// UploadImageByUrlParams is the data required for an Image Upload by URL request. -type UploadImageByUrlParams struct { - Url string - RequireSignedURLs bool - Metadata map[string]interface{} -} - -func (b UploadImageByUrlParams) write(mpw *multipart.Writer) error { - err := mpw.WriteField("url", b.Url) - if err != nil { - return err - } - // According to the Cloudflare docs, this field defaults to false. // For simplicity, we will only send it if the value is true, however // if the default is changed to true, this logic will need to be updated. if b.RequireSignedURLs { - err = mpw.WriteField("requireSignedURLs", "true") + err := mpw.WriteField("requireSignedURLs", "true") if err != nil { return err } @@ -220,46 +194,6 @@ func (api *API) UploadImage(ctx context.Context, rc *ResourceContainer, params U return imageDetailsResponse.Result, nil } -// UploadImageByUrl uploads a single image from a given url. -// -// API Reference: https://api.cloudflare.com/#cloudflare-images-upload-an-image-using-a-single-http-request -func (api *API) UploadImageByUrl(ctx context.Context, rc *ResourceContainer, params UploadImageByUrlParams) (Image, error) { - if rc.Level != AccountRouteLevel { - return Image{}, ErrRequiredAccountLevelResourceContainer - } - - uri := fmt.Sprintf("/accounts/%s/images/v1", rc.Identifier) - - body := &bytes.Buffer{} - w := multipart.NewWriter(body) - if err := params.write(w); err != nil { - _ = w.Close() - return Image{}, fmt.Errorf("error writing multipart body: %w", err) - } - _ = w.Close() - - res, err := api.makeRequestContextWithHeaders( - ctx, - http.MethodPost, - uri, - body, - http.Header{ - "Accept": []string{"application/json"}, - "Content-Type": []string{w.FormDataContentType()}, - }, - ) - if err != nil { - return Image{}, err - } - - var imageDetailsResponse ImageDetailsResponse - err = json.Unmarshal(res, &imageDetailsResponse) - if err != nil { - return Image{}, fmt.Errorf("%s: %w", errUnmarshalError, err) - } - return imageDetailsResponse.Result, nil -} - // UpdateImage updates an existing image's metadata. // // API Reference: https://api.cloudflare.com/#cloudflare-images-update-image diff --git a/images_test.go b/images_test.go index d4fa864838f..d723b4b75e4 100644 --- a/images_test.go +++ b/images_test.go @@ -103,7 +103,7 @@ func TestUploadImageByUrl(t *testing.T) { handler := func(w http.ResponseWriter, r *http.Request) { assert.Equal(t, http.MethodPost, r.Method, "Expected method 'POST', got %s", r.Method) - u, err := parseImageMultipartUploadByUrl(r) + u, err := parseImageMultipartUpload(r) if !assert.NoError(t, err) { w.WriteHeader(http.StatusBadRequest) return @@ -138,7 +138,7 @@ func TestUploadImageByUrl(t *testing.T) { mux.HandleFunc("/accounts/"+testAccountID+"/images/v1", handler) want := expectedImageStruct - actual, err := client.UploadImageByUrl(context.Background(), AccountIdentifier(testAccountID), UploadImageByUrlParams{ + actual, err := client.UploadImage(context.Background(), AccountIdentifier(testAccountID), UploadImageParams{ Url: "https://www.images-elsewhere.com/avatar.png", RequireSignedURLs: true, Metadata: map[string]interface{}{ @@ -442,6 +442,7 @@ type imageMultipartUpload struct { // this is for testing, never read an entire file into memory, // especially when being done on a per-http request basis. File []byte + Url string RequireSignedURLs bool Metadata map[string]interface{} } @@ -473,62 +474,28 @@ func parseImageMultipartUpload(r *http.Request) (imageMultipartUpload, error) { } } - f, _, err := r.FormFile("file") - if err != nil { - return u, err - } - defer f.Close() - - u.File, err = io.ReadAll(f) - if err != nil { - return u, err - } - - return u, nil -} - -type imageMultipartUploadByUrl struct { - Url string - RequireSignedURLs bool - Metadata map[string]interface{} -} - -func parseImageMultipartUploadByUrl(r *http.Request) (imageMultipartUploadByUrl, error) { - var u imageMultipartUploadByUrl - mdBytes, err := getImageFormValue(r, "metadata") - if err != nil { - if !strings.HasPrefix(err.Error(), "no value found for key") { - return u, err - } - } - if mdBytes != nil { - err = json.Unmarshal(mdBytes, &u.Metadata) + if _, ok := r.MultipartForm.Value["url"]; ok { + urlBytes, err := getImageFormValue(r, "url") if err != nil { - return u, err + if !strings.HasPrefix(err.Error(), "no value found for key") { + return u, err + } } - } - - rsuBytes, err := getImageFormValue(r, "requireSignedURLs") - if err != nil { - if !strings.HasPrefix(err.Error(), "no value found for key") { - return u, err + if urlBytes != nil { + u.Url = string(urlBytes) } - } - if rsuBytes != nil { - if bytes.Equal(rsuBytes, []byte("true")) { - u.RequireSignedURLs = true + } else { + f, _, err := r.FormFile("file") + if err != nil { + return u, err } - } + defer f.Close() - urlBytes, err := getImageFormValue(r, "url") - if err != nil { - if !strings.HasPrefix(err.Error(), "no value found for key") { + u.File, err = io.ReadAll(f) + if err != nil { return u, err } } - if urlBytes != nil { - u.Url = string(urlBytes) - } return u, nil } From 4165acced5447870fa7053fa8b32584a01d46c2d Mon Sep 17 00:00:00 2001 From: Jacob Bednarz Date: Tue, 18 Jul 2023 13:43:03 +1000 Subject: [PATCH 5/6] `Url` => `URL` --- images.go | 8 ++++---- images_test.go | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/images.go b/images.go index ed2d331a207..281b49bf50f 100644 --- a/images.go +++ b/images.go @@ -37,7 +37,7 @@ type Image struct { // UploadImageParams is the data required for an Image Upload request. type UploadImageParams struct { File io.ReadCloser - Url string + URL string Name string RequireSignedURLs bool Metadata map[string]interface{} @@ -46,7 +46,7 @@ type UploadImageParams struct { // write writes the image upload data to a multipart writer, so // it can be used in an HTTP request. func (b UploadImageParams) write(mpw *multipart.Writer) error { - if b.File == nil && b.Url == "" { + if b.File == nil && b.URL == "" { return errors.New("a file or url to upload must be specified") } @@ -64,8 +64,8 @@ func (b UploadImageParams) write(mpw *multipart.Writer) error { _ = b.File.Close() } - if b.Url != "" { - err := mpw.WriteField("url", b.Url) + if b.URL != "" { + err := mpw.WriteField("url", b.URL) if err != nil { return err } diff --git a/images_test.go b/images_test.go index d723b4b75e4..7a37cf5a451 100644 --- a/images_test.go +++ b/images_test.go @@ -139,7 +139,7 @@ func TestUploadImageByUrl(t *testing.T) { want := expectedImageStruct actual, err := client.UploadImage(context.Background(), AccountIdentifier(testAccountID), UploadImageParams{ - Url: "https://www.images-elsewhere.com/avatar.png", + URL: "https://www.images-elsewhere.com/avatar.png", RequireSignedURLs: true, Metadata: map[string]interface{}{ "meta": "metaID", From 5a38e52cca775cd8e98a4b2e0becccbc82d0e9a3 Mon Sep 17 00:00:00 2001 From: Jacob Bednarz Date: Tue, 18 Jul 2023 13:43:39 +1000 Subject: [PATCH 6/6] add test coverage for mutually exclusive options --- images.go | 4 ++++ images_test.go | 14 ++++++++++++++ 2 files changed, 18 insertions(+) diff --git a/images.go b/images.go index 281b49bf50f..59209ad1f78 100644 --- a/images.go +++ b/images.go @@ -162,6 +162,10 @@ func (api *API) UploadImage(ctx context.Context, rc *ResourceContainer, params U return Image{}, ErrRequiredAccountLevelResourceContainer } + if params.File != nil && params.URL != "" { + return Image{}, errors.New("file and url uploads are mutually exclusive and can only be performed individually") + } + uri := fmt.Sprintf("/accounts/%s/images/v1", rc.Identifier) body := &bytes.Buffer{} diff --git a/images_test.go b/images_test.go index 7a37cf5a451..43cd8ad3498 100644 --- a/images_test.go +++ b/images_test.go @@ -252,6 +252,20 @@ func TestCreateImageDirectUploadURL(t *testing.T) { } } +func TestCreateImageConflictingTypes(t *testing.T) { + setup() + defer teardown() + + _, err := client.UploadImage(context.Background(), AccountIdentifier(testAccountID), UploadImageParams{ + URL: "https://example.com/foo.jpg", + File: fakeFile{ + Buffer: bytes.NewBufferString("this is definitely an image"), + }, + }) + + assert.Error(t, err) +} + func TestCreateImageDirectUploadURLV2(t *testing.T) { setup() defer teardown()