Skip to content

Commit

Permalink
storage: add StartOffset and more tests for NewRangeReader
Browse files Browse the repository at this point in the history
* 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 googleapis#1540.

Change-Id: I56b7777cc46e3c501593e92e27a7c083a729fed5
Reviewed-on: https://code-review.googlesource.com/c/gocloud/+/44610
Reviewed-by: Frank Natividad <franknatividad@google.com>
Reviewed-by: kokoro <noreply+kokoro@google.com>
  • Loading branch information
odeke-em committed Aug 28, 2019
1 parent b099174 commit 6f3ae29
Show file tree
Hide file tree
Showing 4 changed files with 137 additions and 8 deletions.
45 changes: 43 additions & 2 deletions storage/example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
65 changes: 63 additions & 2 deletions storage/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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 {
Expand All @@ -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()
}
Expand Down
22 changes: 18 additions & 4 deletions storage/reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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,
}
Expand Down
13 changes: 13 additions & 0 deletions storage/reader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 6f3ae29

Please sign in to comment.