blob: specify "oh, nevermind" error value for optional SubFetcher interface

To support composition, all optional interfaces in Go need a way to
say, at runtime, that "oh, nevermind, I defensively implemented this
interface but it turns out the thing I'm wrapping doesn't support it".

We learned that lesson only in the past few years, but SubFetcher
predates that.

So do that here and make proxycache implemented SubFetcher and make
everything respect the fallback oh-nevermind error value.

Fixes #1170

Change-Id: I3c0958cf6692a324e98f526571d10a352a8e18b4
This commit is contained in:
Brad Fitzpatrick 2018-05-17 17:56:04 -04:00
parent 6ddd633871
commit 0ccf258ca4
4 changed files with 35 additions and 2 deletions

View File

@ -49,6 +49,10 @@ type Fetcher interface {
Fetch(context.Context, Ref) (blob io.ReadCloser, size uint32, err error)
}
// ErrUnimplemented is returned by optional interfaces when their
// wrapped values don't implemented the optional interface.
var ErrUnimplemented = errors.New("optional method not implemented")
// A SubFetcher is a Fetcher that can retrieve part of a blob.
type SubFetcher interface {
// SubFetch returns part of a blob.
@ -57,7 +61,8 @@ type SubFetcher interface {
// check. The returned error should be: ErrNegativeSubFetch if any of
// offset or length is negative, or os.ErrNotExist if the blob
// doesn't exist, or ErrOutOfRangeOffsetSubFetch if offset goes over
// the size of the blob.
// the size of the blob. If the error is ErrUnimplemented, the caller should
// treat this Fetcher as if it doesn't implement SubFetcher.
SubFetch(ctx context.Context, ref Ref, offset, length int64) (io.ReadCloser, error)
}

View File

@ -47,7 +47,10 @@ func (s *storage) SubFetch(ctx context.Context, ref blob.Ref, offset, length int
return s.large.SubFetch(ctx, m.largeRef, int64(m.largeOff)+offset, length)
}
if sf, ok := s.small.(blob.SubFetcher); ok {
return sf.SubFetch(ctx, ref, offset, length)
rc, err := sf.SubFetch(ctx, ref, offset, length)
if err != blob.ErrUnimplemented {
return rc, err
}
}
rc, size, err := s.small.Fetch(ctx, ref)
if err != nil {

View File

@ -67,6 +67,7 @@ type Storage struct {
var (
_ blobserver.Storage = (*Storage)(nil)
_ blob.SubFetcher = (*Storage)(nil)
// TODO:
// _ blobserver.Generationer = (*Storage)(nil)
)
@ -176,6 +177,22 @@ func (sto *Storage) Fetch(ctx context.Context, b blob.Ref) (rc io.ReadCloser, si
return ioutil.NopCloser(bytes.NewReader(all)), size, nil
}
func (sto *Storage) SubFetch(ctx context.Context, ref blob.Ref, offset, length int64) (io.ReadCloser, error) {
if sf, ok := sto.cache.(blob.SubFetcher); ok {
rc, err := sf.SubFetch(ctx, ref, offset, length)
if err == nil {
return rc, nil
}
if err != os.ErrNotExist && err != blob.ErrUnimplemented {
log.Printf("proxycache: error fetching from cache %T: %v", sto.cache, err)
}
}
if sf, ok := sto.origin.(blob.SubFetcher); ok {
return sf.SubFetch(ctx, ref, offset, length)
}
return nil, blob.ErrUnimplemented
}
type errList []error
func (e errList) OrNil() error {

View File

@ -213,6 +213,10 @@ func (r *run) testSubFetcher() {
}
for _, tt := range regions {
r, err := sf.SubFetch(context.Background(), big.BlobRef(), tt.off, tt.limit)
if err == blob.ErrUnimplemented {
t.Logf("%T implements SubFetcher but its wrapped value doesn't", sto)
return
}
if err != nil {
t.Fatalf("Error fetching big blob for SubFetch: %v", err)
}
@ -239,6 +243,10 @@ func (r *run) testSubFetcher() {
}
for _, tt := range invalids {
r, err := sf.SubFetch(context.Background(), big.BlobRef(), tt.off, tt.limit)
if err == blob.ErrUnimplemented {
t.Logf("%T implements SubFetcher but its wrapped value doesn't", sto)
return
}
if err == nil {
r.Close()
t.Errorf("No error fetching with off=%d limit=%d; wanted an error", tt.off, tt.limit)