diff --git a/operations.go b/operations.go index d3590f9..af47c73 100644 --- a/operations.go +++ b/operations.go @@ -13,9 +13,7 @@ import ( // TODO: add tests and use generics instead of interface{} func (c *Client) OperationStatus(ctx context.Context, operationID string) (any, *http.Response, error) { - query := url.Values{} - query.Set("operation_id", operationID) - resp, err := c.doRequest(ctx, GET, "operations?"+query.Encode(), nil) + resp, err := c.doRequest(ctx, GET, "operations/"+url.PathEscape(operationID), nil) if err != nil { return nil, nil, fmt.Errorf("failed to make request: %w", err) } diff --git a/operations_status_test.go b/operations_status_test.go index deefcb9..074a8de 100644 --- a/operations_status_test.go +++ b/operations_status_test.go @@ -26,8 +26,9 @@ func TestOperationIDFromHref(t *testing.T) { func TestGetOperationStatus(t *testing.T) { t.Run("returns the operation", func(t *testing.T) { client := mockedHttpClient(func(w http.ResponseWriter, r *http.Request) { - if got := r.URL.Query().Get("operation_id"); got != "123abc" { - t.Errorf("operation_id = %q, want %q", got, "123abc") + // The ID is a path segment, as documented, not a query parameter. + if r.URL.Path != "/v1/disk/operations/123abc" { + t.Errorf("path = %q, want /v1/disk/operations/123abc", r.URL.Path) } _, _ = w.Write([]byte(`{"status":"success"}`)) }) diff --git a/resources.go b/resources.go index d2dbf20..87061ac 100644 --- a/resources.go +++ b/resources.go @@ -47,7 +47,7 @@ func validatePath(p string) error { func (c *Client) buildDeleteResourceURL(path string, permanently bool) string { query := url.Values{} query.Set("path", path) - query.Set("permanent", strconv.FormatBool(permanently)) + query.Set("permanently", strconv.FormatBool(permanently)) return fmt.Sprintf("resources?%s", query.Encode()) } diff --git a/resources_test.go b/resources_test.go index d7dd7c2..5d40f15 100644 --- a/resources_test.go +++ b/resources_test.go @@ -23,19 +23,19 @@ func TestBuildDeleteResourceURL(t *testing.T) { name: "Delete temporary file", path: "/path/to/file.txt", permanently: false, - want: "resources?path=%2Fpath%2Fto%2Ffile.txt&permanent=false", + want: "resources?path=%2Fpath%2Fto%2Ffile.txt&permanently=false", }, { name: "Delete permanent file", path: "/another/path/to/file.jpg", permanently: true, - want: "resources?path=%2Fanother%2Fpath%2Fto%2Ffile.jpg&permanent=true", + want: "resources?path=%2Fanother%2Fpath%2Fto%2Ffile.jpg&permanently=true", }, { name: "Delete file with special characters", path: "/path with spaces/file with &.txt", permanently: false, - want: "resources?path=%2Fpath+with+spaces%2Ffile+with+%26.txt&permanent=false", + want: "resources?path=%2Fpath+with+spaces%2Ffile+with+%26.txt&permanently=false", }, } diff --git a/upload.go b/upload.go index 6b64147..d546d58 100644 --- a/upload.go +++ b/upload.go @@ -7,8 +7,10 @@ import ( "io" "mime" "net/http" + "net/url" "os" "path/filepath" + "strconv" "strings" ) @@ -100,7 +102,7 @@ func (c *Client) validateLocalFile(localPath string) (os.FileInfo, error) { // uploadFileSingle handles single file upload without chunking func (c *Client) uploadFileSingle(ctx context.Context, localPath string, remotePath string, fileInfo os.FileInfo, options *UploadOptions) (*Resource, error) { // Step 1: Get upload link from Yandex Disk API - uploadLink, linkErr := c.GetLinkForUpload(ctx, remotePath) + uploadLink, linkErr := c.GetLinkForUploadWithOverwrite(ctx, remotePath, options.Overwrite) if linkErr != nil { c.Logger.LogError("get upload link", fmt.Errorf("failed to get upload link: %v", linkErr)) return nil, fmt.Errorf("failed to get upload link: %v", linkErr) @@ -150,11 +152,6 @@ func (c *Client) uploadFileSingle(ctx context.Context, localPath string, remoteP req.Header.Set("Content-Type", contentType) req.ContentLength = fileInfo.Size() - // Handle overwrite policy - if options.Overwrite { - req.Header.Set("X-Overwrite", "true") - } - c.Logger.Debug("Uploading file with content type: %s", contentType) // Step 5: Execute the upload using a transfer-scoped HTTP client @@ -211,7 +208,7 @@ func (c *Client) uploadFileMultipart(ctx context.Context, localPath string, remo defer file.Close() // Get upload link - uploadLink, linkErr := c.GetLinkForUpload(ctx, remotePath) + uploadLink, linkErr := c.GetLinkForUploadWithOverwrite(ctx, remotePath, options.Overwrite) if linkErr != nil { c.Logger.LogError("get upload link for multipart", fmt.Errorf("failed to get upload link: %v", linkErr)) return nil, fmt.Errorf("failed to get upload link: %v", linkErr) @@ -243,10 +240,6 @@ func (c *Client) uploadFileMultipart(ctx context.Context, localPath string, remo req.Header.Set("Content-Type", contentType) req.ContentLength = fileInfo.Size() - if options.Overwrite { - req.Header.Set("X-Overwrite", "true") - } - c.Logger.Debug("Starting multipart upload with content type: %s", contentType) // Large uploads need far more headroom than the default per-request timeout. @@ -425,6 +418,21 @@ func GetFileSize(filePath string) (int64, error) { return fileInfo.Size(), nil } +// GetLinkForUploadWithOverwrite is [Client.GetLinkForUpload] with a choice of +// what happens when a file already exists at path: with overwrite it is +// replaced, without it the API refuses with 409. +func (c *Client) GetLinkForUploadWithOverwrite(ctx context.Context, path string, overwrite bool) (*ResourceUploadLink, *ErrorResponse) { + if len(path) < 1 { + return nil, &ErrorResponse{Error: "path cannot be empty"} + } + + query := url.Values{} + query.Set("path", path) + query.Set("overwrite", strconv.FormatBool(overwrite)) + + return requestJSON[ResourceUploadLink](ctx, c, GET, "resources/upload?"+query.Encode(), nil) +} + // FormatFileSize formats a file size in bytes to a human-readable string func FormatFileSize(bytes int64) string { const unit = 1024 diff --git a/upload_test.go b/upload_test.go index fa1ac30..1e56287 100644 --- a/upload_test.go +++ b/upload_test.go @@ -3,8 +3,10 @@ package disk import ( "context" "errors" + "fmt" "net/http" "os" + "strconv" "strings" "testing" ) @@ -451,34 +453,37 @@ func TestUploadFileSingle(t *testing.T) { } }) - t.Run("upload with Overwrite option sends X-Overwrite header", func(t *testing.T) { - var gotOverwrite string - handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch r.URL.Path { - case "/v1/disk/resources/upload": - addr := r.Host - href := "http://" + addr + "/do-upload" - w.Write([]byte(`{"href":"` + href + `","method":"PUT","templated":false}`)) - case "/do-upload": - gotOverwrite = r.Header.Get("X-Overwrite") - w.WriteHeader(http.StatusCreated) - case "/v1/disk/resources": - w.Write([]byte(testResourceJSON)) - default: - http.NotFound(w, r) + // The API takes overwrite as a query parameter of the upload link request; + // without it, uploading over an existing file fails with 409. + for _, overwrite := range []bool{true, false} { + t.Run(fmt.Sprintf("upload link asks for overwrite=%t", overwrite), func(t *testing.T) { + var gotOverwrite string + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/v1/disk/resources/upload": + gotOverwrite = r.URL.Query().Get("overwrite") + href := "http://" + r.Host + "/do-upload" + w.Write([]byte(`{"href":"` + href + `","method":"PUT","templated":false}`)) + case "/do-upload": + w.WriteHeader(http.StatusCreated) + case "/v1/disk/resources": + w.Write([]byte(testResourceJSON)) + default: + http.NotFound(w, r) + } + }) + client := mockedHttpClient(handler) + + localPath := makeTempFile(t, "overwrite me") + _, err := client.UploadFileFromPath(context.Background(), localPath, "/test/file.txt", &UploadOptions{Overwrite: overwrite}) + if err != nil { + t.Fatalf("expected no error, got: %v", err) + } + if want := strconv.FormatBool(overwrite); gotOverwrite != want { + t.Errorf("overwrite = %q, want %q", gotOverwrite, want) } }) - client := mockedHttpClient(handler) - - localPath := makeTempFile(t, "overwrite me") - _, err := client.UploadFileFromPath(context.Background(), localPath, "/test/file.txt", &UploadOptions{Overwrite: true}) - if err != nil { - t.Fatalf("expected no error, got: %v", err) - } - if gotOverwrite != "true" { - t.Errorf("expected X-Overwrite: true, got %q", gotOverwrite) - } - }) + } t.Run("upload link API error returns error", func(t *testing.T) { client := mockedHttpClient(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {