Protect public links, an integration suite, and the bugs it found

CONTRIBUTING.md

@@ -95,6 +95,24 @@ fix. Tests talk to an `httptest` server rather than to Yandex; look at
 Keep tests deterministic. If one needs to wait, give it a timeout measured in
 milliseconds, not seconds, so the suite stays fast.
 
+### Against the real API
+
+`integration_test.go` checks the library against Yandex itself. It is behind
+a build tag, so `go test ./...` skips it. Run it with a token of an account
+you can spare:
+
+```sh
+YANDEX_DISK_ACCESS_TOKEN=... go test -tags integration -run Integration -v ./
+```
+
+It takes about two minutes and needs a few megabytes. Everything it creates
+lives in a new `disk:/disk-it-<date>-<time>` folder, which it deletes for
+good at the end. A guard in the HTTP transport refuses any change outside
+that folder, any trash item the test did not put there, and emptying the
+whole trash. Calls that read the whole Disk log only counts, never names.
+
+Run it before a release, and whenever you change how a request is built.
+
 ## Commit messages
 
 We use [Conventional Commits](https://www.conventionalcommits.org/):

api_coverage_test.go

@@ -5,6 +5,7 @@ import (
 	"net/http"
 	"net/url"
 	"testing"
+	"time"
 )
 
 // request is what a test server saw.
@@ -189,3 +190,40 @@ func TestCoordinateToleratesOddShapes(t *testing.T) {
 		}
 	}
 }
+
+func TestBatchCopyAndMoveHonourOverwrite(t *testing.T) {
+	opts := &BatchCopyMoveOptions{Overwrite: true}
+	ops := map[string]string{"disk:/a": "disk:/b"}
+
+	client, got := recordingClient(http.StatusCreated, linkJSON)
+	if _, err := client.BatchCopyFiles(context.Background(), ops, opts); err != nil {
+		t.Fatal(err)
+	}
+	expectQuery(t, got, http.MethodPost, "/v1/disk/resources/copy", map[string]string{"overwrite": "true"})
+
+	client, got = recordingClient(http.StatusCreated, linkJSON)
+	if _, err := client.BatchMoveFiles(context.Background(), ops, opts); err != nil {
+		t.Fatal(err)
+	}
+	expectQuery(t, got, http.MethodPost, "/v1/disk/resources/move", map[string]string{"overwrite": "true"})
+}
+
+// A copy that finished at once returns a link to the copied file, not to an
+// operation; waiting must not try to poll it.
+func TestWaitForBatchOperationSkipsFinishedResults(t *testing.T) {
+	client, got := recordingClient(http.StatusCreated,
+		`{"href": "https://cloud-api.yandex.net/v1/disk/resources?path=disk%3A%2Fb", "method": "GET"}`)
+	status, err := client.BatchCopyFiles(context.Background(), map[string]string{"disk:/a": "disk:/b"}, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	got.path = ""
+	ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
+	defer cancel()
+	if err := client.WaitForBatchOperation(ctx, status, 10*time.Millisecond); err != nil {
+		t.Fatalf("wait: %v", err)
+	}
+	if got.path != "" {
+		t.Errorf("waiting polled %s", got.path)
+	}
+}

batch.go

@@ -233,7 +233,7 @@ func (c *Client) BatchCopyFiles(ctx context.Context, operations map[string]strin
 			}
 
 			// Perform the copy operation
-			link, errResp := c.CopyResource(opCtx, from, to)
+			link, errResp := c.CopyResourceWithOptions(opCtx, from, to, &CopyMoveOptions{Overwrite: options.Overwrite})
 			result.Duration = time.Since(startTime)
 
 			if errResp != nil {
@@ -346,7 +346,7 @@ func (c *Client) BatchMoveFiles(ctx context.Context, operations map[string]strin
 			}
 
 			// Perform the move operation
-			link, errResp := c.MoveResource(opCtx, from, to)
+			link, errResp := c.MoveResourceWithOptions(opCtx, from, to, &CopyMoveOptions{Overwrite: options.Overwrite})
 			result.Duration = time.Since(startTime)
 
 			if errResp != nil {
@@ -693,7 +693,7 @@ func (c *Client) WaitForBatchOperation(ctx context.Context, status *BatchOperati
 	// Check if there are any async operations (operations that returned Links)
 	asyncOps := 0
 	for _, result := range status.Results {
-		if result != nil && result.Link != nil {
+		if result != nil && isOperationLink(result.Link) {
 			asyncOps++
 		}
 	}
@@ -715,7 +715,7 @@ func (c *Client) WaitForBatchOperation(ctx context.Context, status *BatchOperati
 		case <-ticker.C:
 			completed := 0
 			for _, result := range status.Results {
-				if result == nil || result.Link == nil {
+				if result == nil || !isOperationLink(result.Link) {
 					continue
 				}
 
@@ -815,3 +815,10 @@ func (c *Client) RetryFailedOperations(ctx context.Context, status *BatchOperati
 
 	return retryStatus, nil
 }
+
+// isOperationLink reports whether link points to a background operation.
+// An operation that finished at once returns a link to its result instead,
+// such as the copied file, and there is nothing to wait for.
+func isOperationLink(link *Link) bool {
+	return link != nil && strings.Contains(link.Href, "/operations/")
+}

client.go

@@ -1,6 +1,7 @@
 package disk
 
 import (
+	"bytes"
 	"context"
 	"crypto/tls"
 	"encoding/json"
@@ -182,8 +183,15 @@ func (c *Client) doRequest(ctx context.Context, method HttpMethod, resource stri
 	if method == GET || method == DELETE {
 		body = nil
 	} else if data != nil {
-		// Limit request body size to prevent memory exhaustion
-		body = io.LimitReader(data, 100*1024*1024) // 100MB limit
+		switch data.(type) {
+		case *bytes.Reader, *bytes.Buffer, *strings.Reader:
+			// The length is known, so the request carries Content-Length.
+			// Wrapping these would send the body chunked, which the API
+			// ignores: PATCH and PUT bodies arrived empty.
+		default:
+			// Limit request body size to prevent memory exhaustion
+			body = io.LimitReader(data, 100*1024*1024) // 100MB limit
+		}
 	}
 
 	requestURL := c.baseURL() + resource

client_test.go

@@ -26,6 +26,8 @@ func (t *testTransport) RoundTrip(req *http.Request) (*http.Response, error) {
 	if err != nil {
 		return nil, err
 	}
+	// Keep the length, so tests see the body as the real server would.
+	testReq.ContentLength = req.ContentLength
 
 	// Copy headers
 	testReq.Header = req.Header.Clone()