[cas] Simplify UploadResult.Digest() signature (#362)
* [cas] Simplify UploadResult.Digest() signature
Instead of returning a possibly nil digest, introduce a new error
ErrNoDigest and return that instead. It simplifies most callsites: now
they check only 2 cases instead of 3.
diff --git a/go/pkg/cas/upload.go b/go/pkg/cas/upload.go
index 8737659..ae79d81 100644
--- a/go/pkg/cas/upload.go
+++ b/go/pkg/cas/upload.go
@@ -107,13 +107,19 @@
digest *repb.Digest // may be nil, e.g. for dangling symlinks
}
-// ErrSkip when returned by UploadOptions.Prelude, means the file/dir must be
-// not be uploaded.
-//
-// Note that if UploadOptions.PreserveSymlinks is true and the ErrSkip is
-// returned for a symlink target, but not the symlink itself, then it may
-// result in a dangling symlink.
-var ErrSkip = errors.New("skip file")
+var (
+ // ErrSkip when returned by UploadOptions.Prelude, means the file/dir must be
+ // not be uploaded.
+ //
+ // Note that if UploadOptions.PreserveSymlinks is true and the ErrSkip is
+ // returned for a symlink target, but not the symlink itself, then it may
+ // result in a dangling symlink.
+ ErrSkip = errors.New("skip file")
+
+ // ErrNoDigest indicates that the requested digest is uknown.
+ // Use errors.Is instead of direct equality check.
+ ErrNoDigest = errors.New("the requested digest is unknown")
+)
// UploadResult is the result of a Client.Upload call.
// It provides file/dir digests and statistics.
@@ -130,28 +136,28 @@
// To retrieve a digest of a directory or a symlink, ps.Exclude must match one
// of the PathSpecs passed to Client.Upload earlier.
//
-// If the digest is unknown, returns (nil, nil).
+// If the digest is unknown, returns (nil, err), where err is ErrDigestUnknown
+// according to errors.Is.
// If the file is a danging symlink, then its digest is unknown.
-func (r *UploadResult) Digest(ps *PathSpec) (*digest.Digest, error) {
+func (r *UploadResult) Digest(ps *PathSpec) (digest.Digest, error) {
if !filepath.IsAbs(ps.Path) {
- return nil, errors.Errorf("%q is not absolute", ps.Path)
+ return digest.Digest{}, errors.Errorf("%q is not absolute", ps.Path)
}
// TODO(nodir): cache this syscall too.
info, err := os.Lstat(ps.Path)
if err != nil {
- return nil, err
+ return digest.Digest{}, err
}
key := makeFSCacheKey(ps.Path, info.Mode().IsRegular(), ps.Exclude)
switch val, err, loaded := r.u.fsCache.Load(key); {
case !loaded:
- return nil, nil
+ return digest.Digest{}, errors.Wrapf(ErrNoDigest, "digest not found for %#v", ps)
case err != nil:
- return nil, err
+ return digest.Digest{}, err
default:
- dig := digest.NewFromProtoUnvalidated(val.(*digested).digest)
- return &dig, nil
+ return digest.NewFromProtoUnvalidated(val.(*digested).digest), nil
}
}
diff --git a/go/pkg/cas/upload_test.go b/go/pkg/cas/upload_test.go
index fae5598..550c67f 100644
--- a/go/pkg/cas/upload_test.go
+++ b/go/pkg/cas/upload_test.go
@@ -108,11 +108,10 @@
},
})
- digSlice := func(items ...*uploadItem) []*digest.Digest {
- ret := make([]*digest.Digest, len(items))
+ digSlice := func(items ...*uploadItem) []digest.Digest {
+ ret := make([]digest.Digest, len(items))
for i, item := range items {
- dig := digest.NewFromProtoUnvalidated(item.Digest)
- ret[i] = &dig
+ ret[i] = digest.NewFromProtoUnvalidated(item.Digest)
}
return ret
}
@@ -120,7 +119,7 @@
tests := []struct {
desc string
paths []*PathSpec
- wantDigests []*digest.Digest
+ wantDigests []digest.Digest
wantScheduledChecks []*uploadItem
wantErr error
opt UploadOptions
@@ -275,7 +274,7 @@
t.Errorf("unexpected scheduled checks (-want +got):\n%s", diff)
}
- gotDigests := make([]*digest.Digest, 0, len(tc.paths))
+ gotDigests := make([]digest.Digest, 0, len(tc.paths))
for _, ps := range tc.paths {
dig, err := res.Digest(ps)
if err != nil {
