diff --git a/services/storage-users/README.md b/services/storage-users/README.md index ad467306e24..a0a7f451dd1 100644 --- a/services/storage-users/README.md +++ b/services/storage-users/README.md @@ -194,8 +194,8 @@ ocis storage-users blobstore ```plaintext COMMANDS: - check Check blobstore connectivity via an upload/download/delete round-trip - get Get a blob from the blobstore by ID + check check blobstore connectivity via an upload/download/delete round-trip + get get a blob from the blobstore by ID ``` #### Check diff --git a/services/storage-users/pkg/command/blobstore.go b/services/storage-users/pkg/command/blobstore.go index 9131c05a77c..55a551b4b2c 100644 --- a/services/storage-users/pkg/command/blobstore.go +++ b/services/storage-users/pkg/command/blobstore.go @@ -10,10 +10,10 @@ import ( "strings" "github.com/google/uuid" - "github.com/owncloud/reva/v2/pkg/bytesize" "github.com/owncloud/ocis/v2/ocis-pkg/config/configlog" "github.com/owncloud/ocis/v2/services/storage-users/pkg/config" "github.com/owncloud/ocis/v2/services/storage-users/pkg/config/parser" + "github.com/owncloud/reva/v2/pkg/bytesize" ocisbs "github.com/owncloud/reva/v2/pkg/storage/fs/ocis/blobstore" s3bs "github.com/owncloud/reva/v2/pkg/storage/fs/s3ng/blobstore" "github.com/owncloud/reva/v2/pkg/storage/utils/decomposedfs/node" @@ -144,7 +144,8 @@ func parseBlobPath(path string) (spaceID, blobID string, err error) { // it strips the directory separators that were inserted every two characters // up to the given depth. // e.g. depathify("61/03/ab/c3/-b08a-4556-9937-2bf3065c1202", 4) -// → "6103abc3-b08a-4556-9937-2bf3065c1202" +// +// → "6103abc3-b08a-4556-9937-2bf3065c1202" func depathify(path string, depth int) string { parts := strings.SplitN(path, "/", depth+1) return strings.Join(parts, "") @@ -181,7 +182,8 @@ func initBlobstore(cfg *config.Config) (tree.Blobstore, error) { // retrieved object size does not match node.Blobsize, and captures the actual size. var blobSizeMismatchRe = regexp.MustCompile(`blob has unexpected size\. \d+ bytes expected, got (\d+) bytes`) -// downloadBlob downloads a single blob identified by blobID and closes the reader. +// downloadBlob downloads a single blob identified by blobID, drains the reader +// to surface any streaming errors, then closes it. // If the s3ng blobstore rejects the download due to a size mismatch, the actual // size is extracted from the error and the download is retried with the correct value. func downloadBlob(bs tree.Blobstore, blobID, spaceID string, blobSize int64) error { @@ -203,7 +205,14 @@ func downloadBlob(bs tree.Blobstore, blobID, spaceID string, blobSize int64) err return fmt.Errorf("download failed: %w", err) } } - rc.Close() + defer func() { + if cerr := rc.Close(); cerr != nil { + fmt.Fprintf(os.Stderr, "warning: failed to close blob reader for blob %s in space %s: %v\n", blobID, spaceID, cerr) + } + }() + if _, err := io.Copy(io.Discard, rc); err != nil { + return fmt.Errorf("download failed while reading: %w", err) + } fmt.Println("Download: OK") return nil } diff --git a/services/storage-users/pkg/command/blobstore_test.go b/services/storage-users/pkg/command/blobstore_test.go new file mode 100644 index 00000000000..87d126084c3 --- /dev/null +++ b/services/storage-users/pkg/command/blobstore_test.go @@ -0,0 +1,104 @@ +package command + +import ( + "testing" +) + +func Test_depathify(t *testing.T) { + tests := []struct { + name string + path string + depth int + want string + }{ + { + name: "blob id depth 4", + path: "61/03/ab/c3/-b08a-4556-9937-2bf3065c1202", + depth: 4, + want: "6103abc3-b08a-4556-9937-2bf3065c1202", + }, + { + name: "space id depth 1", + path: "b1/9ec764-5398-458a-8ff1-1925bd906999", + depth: 1, + want: "b19ec764-5398-458a-8ff1-1925bd906999", + }, + { + name: "depth 0 is a no-op", + path: "abcd-1234", + depth: 0, + want: "abcd-1234", + }, + { + name: "fewer segments than depth leaves remainder intact", + path: "ab/cd", + depth: 4, + want: "abcd", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := depathify(tt.path, tt.depth) + if got != tt.want { + t.Errorf("depathify(%q, %d) = %q, want %q", tt.path, tt.depth, got, tt.want) + } + }) + } +} + +func Test_parseBlobPath(t *testing.T) { + tests := []struct { + name string + path string + wantSpaceID string + wantBlobID string + wantErr bool + }{ + { + // s3ng: / + name: "s3ng format", + path: "b19ec764-5398-458a-8ff1-1925bd906999/61/03/ab/c3/-b08a-4556-9937-2bf3065c1202", + wantSpaceID: "b19ec764-5398-458a-8ff1-1925bd906999", + wantBlobID: "6103abc3-b08a-4556-9937-2bf3065c1202", + }, + { + // ocis: …/spaces//blobs/ + name: "ocis filesystem format", + path: "/var/lib/ocis/storage/users/spaces/b1/9ec764-5398-458a-8ff1-1925bd906999/blobs/61/03/ab/c3/-b08a-4556-9937-2bf3065c1202", + wantSpaceID: "b19ec764-5398-458a-8ff1-1925bd906999", + wantBlobID: "6103abc3-b08a-4556-9937-2bf3065c1202", + }, + { + name: "ocis format missing /blobs/ segment", + path: "/var/lib/ocis/storage/users/spaces/b1/9ec764-5398-458a-8ff1-1925bd906999/noblobs/61/03/ab/c3", + wantErr: true, + }, + { + name: "s3ng format missing blob id (no slash after spaceID)", + path: "b19ec764-5398-458a-8ff1-1925bd906999", + wantErr: true, + }, + { + name: "empty path", + path: "", + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotSpaceID, gotBlobID, err := parseBlobPath(tt.path) + if (err != nil) != tt.wantErr { + t.Fatalf("parseBlobPath(%q) error = %v, wantErr %v", tt.path, err, tt.wantErr) + } + if err != nil { + return + } + if gotSpaceID != tt.wantSpaceID { + t.Errorf("parseBlobPath(%q) spaceID = %q, want %q", tt.path, gotSpaceID, tt.wantSpaceID) + } + if gotBlobID != tt.wantBlobID { + t.Errorf("parseBlobPath(%q) blobID = %q, want %q", tt.path, gotBlobID, tt.wantBlobID) + } + }) + } +}