From 6f3ae29f9332d876a44530a3450db73834bdfcfb Mon Sep 17 00:00:00 2001 From: Emmanuel T Odeke Date: Mon, 26 Aug 2019 19:50:08 -0600 Subject: [PATCH] storage: add StartOffset and more tests for NewRangeReader * Adds StartOffset to ReaderObjectAttrs, which for range requests is the value from which the read began * Updates the range reader integration tests to also test negative offsets aka offsets from the end of the file * Adds integration tests for the various styles of reading the same last n bytes from a file * Adds example tests to show the various styles of using (*ObjectHandle).NewRangeReader * Replaces CL https://code-review.googlesource.com/c/gocloud/+/44132 for which the original author @brimworks no longer has Gerrit access due to job changes Updates #1540. Change-Id: I56b7777cc46e3c501593e92e27a7c083a729fed5 Reviewed-on: https://code-review.googlesource.com/c/gocloud/+/44610 Reviewed-by: Frank Natividad Reviewed-by: kokoro --- storage/example_test.go | 45 +++++++++++++++++++++++-- storage/integration_test.go | 65 +++++++++++++++++++++++++++++++++++-- storage/reader.go | 22 ++++++++++--- storage/reader_test.go | 13 ++++++++ 4 files changed, 137 insertions(+), 8 deletions(-) diff --git a/storage/example_test.go b/storage/example_test.go index 2bfdc9852bb5..8bd64ec057e1 100644 --- a/storage/example_test.go +++ b/storage/example_test.go @@ -365,12 +365,53 @@ func ExampleObjectHandle_NewRangeReader() { if err != nil { // TODO: handle error. } + defer rc.Close() + + slurp, err := ioutil.ReadAll(rc) + if err != nil { + // TODO: handle error. + } + fmt.Printf("first 64K of file contents:\n%s\n", slurp) +} + +func ExampleObjectHandle_NewRangeReader_lastNBytes() { + ctx := context.Background() + client, err := storage.NewClient(ctx) + if err != nil { + // TODO: handle error. + } + // Read only the last 10 bytes until the end of the file. + rc, err := client.Bucket("bucketname").Object("filename1").NewRangeReader(ctx, -10, -1) + if err != nil { + // TODO: handle error. + } + defer rc.Close() + + slurp, err := ioutil.ReadAll(rc) + if err != nil { + // TODO: handle error. + } + fmt.Printf("Last 10 bytes from the end of the file:\n%s\n", slurp) +} + +func ExampleObjectHandle_NewRangeReader_untilEnd() { + ctx := context.Background() + client, err := storage.NewClient(ctx) + if err != nil { + // TODO: handle error. + } + // Read from the 101st byte until the end of the file. + rc, err := client.Bucket("bucketname").Object("filename1").NewRangeReader(ctx, 100, -1) + if err != nil { + // TODO: handle error. + } + defer rc.Close() + slurp, err := ioutil.ReadAll(rc) - rc.Close() if err != nil { // TODO: handle error. } - fmt.Println("first 64K of file contents:", slurp) + fmt.Printf("From 101st byte until the end:\n%s\n", slurp) } func ExampleObjectHandle_NewWriter() { diff --git a/storage/integration_test.go b/storage/integration_test.go index 85be01683002..01229e5b2c94 100644 --- a/storage/integration_test.go +++ b/storage/integration_test.go @@ -468,6 +468,54 @@ func TestIntegration_ConditionalDelete(t *testing.T) { } } +func TestIntegration_ObjectsRangeReader(t *testing.T) { + ctx := context.Background() + client := testConfig(ctx, t) + defer client.Close() + bkt := client.Bucket(bucketName) + + objName := uidSpace.New() + obj := bkt.Object(objName) + w := obj.NewWriter(ctx) + + contents := []byte("Hello, world this is a range request") + if _, err := w.Write(contents); err != nil { + t.Fatalf("Failed to write contents: %v", err) + } + if err := w.Close(); err != nil { + t.Fatalf("Failed to close writer: %v", err) + } + + last5s := []struct { + name string + start int64 + length int64 + }{ + {name: "negative offset", start: -5, length: -1}, + {name: "offset with specified length", start: int64(len(contents)) - 5, length: 5}, + {name: "offset and read till end", start: int64(len(contents)) - 5, length: -1}, + } + + for _, last5 := range last5s { + t.Run(last5.name, func(t *testing.T) { + r, err := obj.NewRangeReader(ctx, last5.start, last5.length) + if err != nil { + t.Fatalf("Failed to make range read: %v", err) + } + defer r.Close() + + if got, want := r.Attrs.StartOffset, int64(len(contents))-5; got != want { + t.Fatalf("StartOffset mismatch, got %d want %d", got, want) + } + + nr, _ := io.Copy(ioutil.Discard, r) + if got, want := nr, int64(5); got != want { + t.Fatalf("Body length mismatch, got %d want %d", got, want) + } + }) + } +} + func TestIntegration_Objects(t *testing.T) { // TODO(jba): Use subtests (Go 1.7). ctx := context.Background() @@ -566,6 +614,9 @@ func TestIntegration_Objects(t *testing.T) { {objlen / 2, 0, 0}, {objlen / 2, -1, objlen / 2}, {0, objlen * 2, objlen}, + {-2, -1, 2}, + {-objlen, -1, objlen}, + {-(objlen / 2), -1, objlen / 2}, } { rc, err := bkt.Object(obj).NewRangeReader(ctx, r.offset, r.length) if err != nil { @@ -587,8 +638,18 @@ func TestIntegration_Objects(t *testing.T) { t.Errorf("%+v: RangeReader (%d, %d): Read %d bytes, wanted %d bytes", i, r.offset, r.length, len(slurp), r.want) continue } - if got, want := slurp, contents[obj][r.offset:r.offset+r.want]; !bytes.Equal(got, want) { - t.Errorf("RangeReader (%d, %d) = %q; want %q", r.offset, r.length, got, want) + + switch { + case r.offset < 0: // The case of reading the last N bytes. + start := objlen + r.offset + if got, want := slurp, contents[obj][start:]; !bytes.Equal(got, want) { + t.Errorf("RangeReader (%d, %d) = %q; want %q", r.offset, r.length, got, want) + } + + default: + if got, want := slurp, contents[obj][r.offset:r.offset+r.want]; !bytes.Equal(got, want) { + t.Errorf("RangeReader (%d, %d) = %q; want %q", r.offset, r.length, got, want) + } } rc.Close() } diff --git a/storage/reader.go b/storage/reader.go index b8d80c87ac1a..5c83651bd9b7 100644 --- a/storage/reader.go +++ b/storage/reader.go @@ -43,6 +43,11 @@ type ReaderObjectAttrs struct { // Size is the length of the object's content. Size int64 + // StartOffset is the byte offset within the object + // from which reading begins. + // This value is only non-zero for range requests. + StartOffset int64 + // ContentType is the MIME type of the object's content. ContentType string @@ -181,20 +186,28 @@ func (o *ObjectHandle) NewRangeReader(ctx context.Context, offset, length int64) return nil, err } var ( - size int64 // total size of object, even if a range was requested. - checkCRC bool - crc uint32 + size int64 // total size of object, even if a range was requested. + checkCRC bool + crc uint32 + startOffset int64 // non-zero if range request. ) if res.StatusCode == http.StatusPartialContent { cr := strings.TrimSpace(res.Header.Get("Content-Range")) if !strings.HasPrefix(cr, "bytes ") || !strings.Contains(cr, "/") { - return nil, fmt.Errorf("storage: invalid Content-Range %q", cr) } size, err = strconv.ParseInt(cr[strings.LastIndex(cr, "/")+1:], 10, 64) if err != nil { return nil, fmt.Errorf("storage: invalid Content-Range %q", cr) } + + dashIndex := strings.Index(cr, "-") + if dashIndex >= 0 { + startOffset, err = strconv.ParseInt(cr[len("bytes="):dashIndex], 10, 64) + if err != nil { + return nil, fmt.Errorf("storage: invalid Content-Range %q: %v", cr, err) + } + } } else { size = res.ContentLength // Check the CRC iff all of the following hold: @@ -240,6 +253,7 @@ func (o *ObjectHandle) NewRangeReader(ctx context.Context, offset, length int64) ContentEncoding: res.Header.Get("Content-Encoding"), CacheControl: res.Header.Get("Cache-Control"), LastModified: lm, + StartOffset: startOffset, Generation: gen, Metageneration: metaGen, } diff --git a/storage/reader_test.go b/storage/reader_test.go index ff59aa2daef4..4b35f3c94c0f 100644 --- a/storage/reader_test.go +++ b/storage/reader_test.go @@ -251,6 +251,19 @@ func TestRangeReaderRetry(t *testing.T) { if got := string(gotb); got != test.want { t.Errorf("#%d: got %q, want %q", i, got, test.want) } + if r.Attrs.Size != int64(len(readData)) { + t.Errorf("#%d: got Attrs.Size=%q, want %q", i, r.Attrs.Size, len(readData)) + } + wantOffset := test.offset + if wantOffset < 0 { + wantOffset += int64(len(readData)) + if wantOffset < 0 { + wantOffset = 0 + } + } + if got := r.Attrs.StartOffset; got != wantOffset { + t.Errorf("#%d: got Attrs.Offset=%q, want %q", i, got, wantOffset) + } } r, err := obj.NewRangeReader(ctx, -100, 10) if err == nil {