From 6acbbc57e10b47d59c93bef65d20ed840983a30f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=B0nan=C3=A7=20G=C3=BCm=C3=BC=C5=9F?= Date: Tue, 10 Sep 2024 14:53:47 -0400 Subject: [PATCH] Improve file persister for consistency Some small changes to make it more readable (subjectively!) - Move checkStatus utility to bottom to reduce noise while reading - pre-signed to presigned and preSigned to presigned for consistency - getPresignedURL to requestPresignedURL for saying it's a request - newUploadRequest to newFileUploadRequest for specificity - readResponseBody generic naming to specific readPresignedURLResponse - preSignedURLGetterURL to presignedURLRequestURL for consistency --- storage/file_persister.go | 65 +++++++++++++++++++++------------------ 1 file changed, 35 insertions(+), 30 deletions(-) diff --git a/storage/file_persister.go b/storage/file_persister.go index 51392360c..038f8f624 100644 --- a/storage/file_persister.go +++ b/storage/file_persister.go @@ -53,13 +53,13 @@ func (l *LocalFilePersister) Persist(_ context.Context, path string, data io.Rea } // RemoteFilePersister is to be used when files created by the browser module need -// to be uploaded to a remote location. This uses a preSignedURLGetterURL to -// retrieve one pre-signed URL. The pre-signed url is used to upload the file +// to be uploaded to a remote location. This uses a presignedURLRequestURL to +// retrieve one presigned URL. The presigned url is used to upload the file // to the remote location. type RemoteFilePersister struct { - preSignedURLGetterURL string - headers map[string]string - basePath string + presignedURLRequestURL string + headers map[string]string + basePath string httpClient *http.Client } @@ -69,7 +69,7 @@ type PresignedURLResponse struct { Service string `json:"service"` URLs []struct { Name string `json:"name"` - PreSignedURL string `json:"pre_signed_url"` //nolint:tagliatelle + PresignedURL string `json:"pre_signed_url"` //nolint:tagliatelle Method string `json:"method"` FormFields map[string]string `json:"form_fields"` //nolint:tagliatelle } `json:"urls"` @@ -77,14 +77,14 @@ type PresignedURLResponse struct { // NewRemoteFilePersister creates a new instance of RemoteFilePersister. func NewRemoteFilePersister( - preSignedURLGetterURL string, + presignedURLRequestURL string, headers map[string]string, basePath string, ) *RemoteFilePersister { return &RemoteFilePersister{ - preSignedURLGetterURL: preSignedURLGetterURL, - headers: headers, - basePath: basePath, + presignedURLRequestURL: presignedURLRequestURL, + headers: headers, + basePath: basePath, httpClient: &http.Client{ Timeout: time.Second * 10, }, @@ -93,12 +93,12 @@ func NewRemoteFilePersister( // Persist will upload the contents of data to a remote location. func (r *RemoteFilePersister) Persist(ctx context.Context, path string, data io.Reader) (err error) { - psResp, err := r.getPreSignedURL(ctx, path) + psResp, err := r.requestPresignedURL(ctx, path) if err != nil { - return fmt.Errorf("getting presigned url: %w", err) + return fmt.Errorf("requesting presigned url: %w", err) } - req, err := newUploadRequest(ctx, psResp, data) + req, err := newFileUploadRequest(ctx, psResp, data) if err != nil { return fmt.Errorf("creating upload request: %w", err) } @@ -120,23 +120,20 @@ func (r *RemoteFilePersister) Persist(ctx context.Context, path string, data io. return nil } -func checkStatusCode(resp *http.Response) error { - if resp.StatusCode < http.StatusOK || resp.StatusCode >= http.StatusMultipleChoices { - return fmt.Errorf("server returned %d (%s)", resp.StatusCode, strings.ToLower(http.StatusText(resp.StatusCode))) - } - - return nil -} - -// getPreSignedURL will request a new presigned URL from the remote server for the given path. -// Returns a [PresignedURLResponse] that contains the presigned URL details. -func (r *RemoteFilePersister) getPreSignedURL(ctx context.Context, path string) (PresignedURLResponse, error) { +// requestPresignedURL will request a new presigned URL from the remote server +// and returns a [PresignedURLResponse] that contains the presigned URL details. +func (r *RemoteFilePersister) requestPresignedURL(ctx context.Context, path string) (PresignedURLResponse, error) { b, err := buildPresignedRequestBody(r.basePath, path) if err != nil { return PresignedURLResponse{}, fmt.Errorf("building request body: %w", err) } - req, err := http.NewRequestWithContext(ctx, http.MethodPost, r.preSignedURLGetterURL, bytes.NewReader(b)) + req, err := http.NewRequestWithContext( + ctx, + http.MethodPost, + r.presignedURLRequestURL, + bytes.NewReader(b), + ) if err != nil { return PresignedURLResponse{}, fmt.Errorf("creating request: %w", err) } @@ -155,7 +152,7 @@ func (r *RemoteFilePersister) getPreSignedURL(ctx context.Context, path string) return PresignedURLResponse{}, err } - return readResponseBody(resp) + return readPresignedURLResponse(resp) } func buildPresignedRequestBody(basePath, path string) ([]byte, error) { @@ -185,7 +182,7 @@ func buildPresignedRequestBody(basePath, path string) ([]byte, error) { return bb, nil } -func readResponseBody(resp *http.Response) (PresignedURLResponse, error) { +func readPresignedURLResponse(resp *http.Response) (PresignedURLResponse, error) { var rb PresignedURLResponse decoder := json.NewDecoder(resp.Body) @@ -201,9 +198,9 @@ func readResponseBody(resp *http.Response) (PresignedURLResponse, error) { return rb, nil } -// newUploadRequest creates a new HTTP request to upload a file as a multipart +// newFileUploadRequest creates a new HTTP request to upload a file as a multipart // form to the presigned URL received from the server. -func newUploadRequest( +func newFileUploadRequest( ctx context.Context, resp PresignedURLResponse, data io.Reader, @@ -235,7 +232,7 @@ func newUploadRequest( req, err := http.NewRequestWithContext( ctx, psu.Method, - psu.PreSignedURL, + psu.PresignedURL, &form, ) if err != nil { @@ -245,3 +242,11 @@ func newUploadRequest( return req, nil } + +func checkStatusCode(resp *http.Response) error { + if resp.StatusCode < http.StatusOK || resp.StatusCode >= http.StatusMultipleChoices { + return fmt.Errorf("server returned %d (%s)", resp.StatusCode, strings.ToLower(http.StatusText(resp.StatusCode))) + } + + return nil +}