From 2ce112635d825d7e8dcc959b85082a140aca7383 Mon Sep 17 00:00:00 2001 From: bufdev <4228796+bufdev@users.noreply.github.com> Date: Wed, 12 Jul 2023 16:30:04 -0400 Subject: [PATCH] Split manifest.NewBucket into storagemanifest package (#2284) --- private/bufpkg/bufmanifest/bucket.go | 21 +- private/bufpkg/bufmodule/module.go | 11 +- private/pkg/manifest/{module.go => blob.go} | 0 .../manifest/{module_test.go => blob_test.go} | 0 private/pkg/manifest/manifest.go | 88 ++++++-- private/pkg/manifest/storage.go | 193 ------------------ private/pkg/storage/storagemanifest/bucket.go | 140 +++++++++++++ .../storagemanifest/read_object_closer.go | 33 +++ .../storagemanifest/storagemanifest.go | 50 +++++ .../storagemanifest/storagemanifest_test.go} | 78 +++---- .../pkg/storage/storagemanifest/usage.gen.go | 19 ++ 11 files changed, 354 insertions(+), 279 deletions(-) rename private/pkg/manifest/{module.go => blob.go} (100%) rename private/pkg/manifest/{module_test.go => blob_test.go} (100%) delete mode 100644 private/pkg/manifest/storage.go create mode 100644 private/pkg/storage/storagemanifest/bucket.go create mode 100644 private/pkg/storage/storagemanifest/read_object_closer.go create mode 100644 private/pkg/storage/storagemanifest/storagemanifest.go rename private/pkg/{manifest/storage_test.go => storage/storagemanifest/storagemanifest_test.go} (74%) create mode 100644 private/pkg/storage/storagemanifest/usage.gen.go diff --git a/private/bufpkg/bufmanifest/bucket.go b/private/bufpkg/bufmanifest/bucket.go index 3bdef97518..3d5fea5e43 100644 --- a/private/bufpkg/bufmanifest/bucket.go +++ b/private/bufpkg/bufmanifest/bucket.go @@ -16,19 +16,18 @@ package bufmanifest import ( "context" - "fmt" modulev1alpha1 "github.com/bufbuild/buf/private/gen/proto/go/buf/alpha/module/v1alpha1" - "github.com/bufbuild/buf/private/pkg/manifest" "github.com/bufbuild/buf/private/pkg/storage" + "github.com/bufbuild/buf/private/pkg/storage/storagemanifest" ) -// NewBucketFromManifestBlobs builds a storage bucket from a manifest blob and a +// NewReadBucketFromManifestBlobs builds a storage bucket from a manifest blob and a // set of other blobs, provided in protobuf form. It makes sure that all blobs // (including manifest) content match with their digest, and additionally checks // that the blob set matches completely with the manifest paths (no missing nor // extra blobs). This bucket is suitable for building or exporting. -func NewBucketFromManifestBlobs( +func NewReadBucketFromManifestBlobs( ctx context.Context, manifestBlob *modulev1alpha1.Blob, blobs []*modulev1alpha1.Blob, @@ -41,14 +40,10 @@ func NewBucketFromManifestBlobs( if err != nil { return nil, err } - manifestBucket, err := manifest.NewBucket( - *parsedManifest, - *blobSet, - manifest.BucketWithAllManifestBlobsValidation(), - manifest.BucketWithNoExtraBlobsValidation(), + return storagemanifest.NewReadBucket( + parsedManifest, + blobSet, + storagemanifest.ReadBucketWithAllManifestBlobs(), + storagemanifest.ReadBucketWithNoExtraBlobs(), ) - if err != nil { - return nil, fmt.Errorf("new manifest bucket: %w", err) - } - return manifestBucket, nil } diff --git a/private/bufpkg/bufmodule/module.go b/private/bufpkg/bufmodule/module.go index f44b9e821f..63ba9dc464 100644 --- a/private/bufpkg/bufmodule/module.go +++ b/private/bufpkg/bufmodule/module.go @@ -28,6 +28,7 @@ import ( "github.com/bufbuild/buf/private/pkg/manifest" "github.com/bufbuild/buf/private/pkg/normalpath" "github.com/bufbuild/buf/private/pkg/storage" + "github.com/bufbuild/buf/private/pkg/storage/storagemanifest" "github.com/bufbuild/buf/private/pkg/storage/storagemem" ) @@ -203,11 +204,11 @@ func newModuleForManifestAndBlobSet( blobSet *manifest.BlobSet, options ...ModuleOption, ) (*module, error) { - bucket, err := manifest.NewBucket( - *moduleManifest, - *blobSet, - manifest.BucketWithAllManifestBlobsValidation(), - manifest.BucketWithNoExtraBlobsValidation(), + bucket, err := storagemanifest.NewReadBucket( + moduleManifest, + blobSet, + storagemanifest.ReadBucketWithAllManifestBlobs(), + storagemanifest.ReadBucketWithNoExtraBlobs(), ) if err != nil { return nil, err diff --git a/private/pkg/manifest/module.go b/private/pkg/manifest/blob.go similarity index 100% rename from private/pkg/manifest/module.go rename to private/pkg/manifest/blob.go diff --git a/private/pkg/manifest/module_test.go b/private/pkg/manifest/blob_test.go similarity index 100% rename from private/pkg/manifest/module_test.go rename to private/pkg/manifest/blob_test.go diff --git a/private/pkg/manifest/manifest.go b/private/pkg/manifest/manifest.go index d0d9813a17..33081f74a0 100644 --- a/private/pkg/manifest/manifest.go +++ b/private/pkg/manifest/manifest.go @@ -43,6 +43,7 @@ package manifest import ( "bufio" "bytes" + "context" "encoding" "errors" "fmt" @@ -51,6 +52,8 @@ import ( "strings" "github.com/bufbuild/buf/private/pkg/normalpath" + "github.com/bufbuild/buf/private/pkg/storage" + "go.uber.org/multierr" ) var errNoFinalNewline = errors.New("partial record: missing newline") @@ -65,6 +68,8 @@ func newErrorWrapped(lineno int, err error) error { // Manifest represents a list of paths and their digests. type Manifest struct { + // needed for ordering + paths []string pathToDigest map[string]Digest digestToPaths map[string][]string } @@ -102,6 +107,41 @@ func NewFromReader(manifest io.Reader) (*Manifest, error) { return &m, nil } +// NewFromBucket creates a manifest and blob set from the bucket's files. Blobs +// in the blob set use the [DigestTypeShake256] digest. +func NewFromBucket( + ctx context.Context, + bucket storage.ReadBucket, +) (*Manifest, *BlobSet, error) { + var m Manifest + digester, err := NewDigester(DigestTypeShake256) + if err != nil { + return nil, nil, err + } + var blobs []Blob + if walkErr := bucket.Walk(ctx, "", func(info storage.ObjectInfo) (retErr error) { + path := info.Path() + obj, err := bucket.Get(ctx, path) + if err != nil { + return err + } + defer func() { retErr = multierr.Append(retErr, obj.Close()) }() + blob, err := NewMemoryBlobFromReaderWithDigester(obj, digester) + if err != nil { + return err + } + blobs = append(blobs, blob) + return m.AddEntry(path, *blob.Digest()) + }); walkErr != nil { + return nil, nil, walkErr + } + blobSet, err := NewBlobSet(ctx, blobs) // no need to pass validation options, we're building and digesting the blobs + if err != nil { + return nil, nil, err + } + return &m, blobSet, nil +} + // AddEntry adds an entry to the manifest with a path and its digest. It fails // if the path already exists in the manifest with a different digest. func (m *Manifest) AddEntry(path string, digest Digest) error { @@ -124,6 +164,8 @@ func (m *Manifest) AddEntry(path string, digest Digest) error { digest.String(), path, existingDigest.String(), ) } + // Already guaranteed that the path is not in the slice due to above check + m.paths = append(m.paths, path) if m.pathToDigest == nil { m.pathToDigest = make(map[string]Digest) } @@ -136,24 +178,26 @@ func (m *Manifest) AddEntry(path string, digest Digest) error { return nil } -// Paths returns all unique paths in the manifest, order not guaranteed. If you want to iterate the -// paths and their digests, consider using `Range` instead. +// Paths returns all unique paths in the manifest by insertion order. func (m *Manifest) Paths() []string { - paths := make([]string, 0, len(m.pathToDigest)) - for path := range m.pathToDigest { - paths = append(paths, path) - } - return paths + pathsCopy := make([]string, len(m.paths)) + copy(pathsCopy, m.paths) + return pathsCopy } -// Digests returns all unique digests in the manifest, order not guaranteed. If you want to iterate -// the paths and their digests, consider using `Range` instead. +// Digests returns all unique digests in the manifest. +// Order is by insertion order. func (m *Manifest) Digests() []Digest { digests := make([]Digest, 0, len(m.digestToPaths)) addedDigests := make(map[string]struct{}, len(m.digestToPaths)) - // iterating over `pathToDigest` instead of `digestToPaths` to avoid handling/returning/panic on - // error if string -> digest parsing fails. It shouldn't. - for _, digest := range m.pathToDigest { + // Iterating over paths to guarantee ordering. + for _, path := range m.paths { + digest, ok := m.pathToDigest[path] + if !ok { + // This should be an error in the style of the rest of the codebase but + // this was refactored and we didn't want to change the function signature. + panic(fmt.Sprintf("path %q not present in pathToDigest", path)) + } if _, alreadyAdded := addedDigests[digest.String()]; alreadyAdded { continue } @@ -163,11 +207,18 @@ func (m *Manifest) Digests() []Digest { return digests } -// Range invokes a function for all the paths in the manifest, passing the path and its digest. The -// order in which the paths are iterated is not guaranteed. This func will stop iterating if an -// error is returned. +// Range invokes a function for all the paths in the manifest, passing the path and its digest. +// Paths are invoked by insertion order. +// This func will stop iterating if an error is returned. func (m *Manifest) Range(f func(path string, digest Digest) error) error { - for path, digest := range m.pathToDigest { + // Iterating over paths to guarantee ordering. + for _, path := range m.paths { + digest, ok := m.pathToDigest[path] + if !ok { + // This should be an error in the style of the rest of the codebase but + // this was refactored and we didn't want to change the function signature. + panic(fmt.Sprintf("path %q not present in pathToDigest", path)) + } if err := f(path, digest); err != nil { return err } @@ -176,7 +227,7 @@ func (m *Manifest) Range(f func(path string, digest Digest) error) error { } // PathsFor returns one or more matching path for a given digest. The digest is -// expected to be a lower-case hex encoded value. Returned paths are unordered. +// expected to be a lower-case hex encoded value. Returned paths are ordered by insertion time. // Paths is nil and ok is false if no paths are found. func (m *Manifest) PathsFor(digest string) ([]string, bool) { paths, ok := m.digestToPaths[digest] @@ -218,6 +269,7 @@ func (m *Manifest) UnmarshalText(text []byte) error { if err != nil { return err } + m.paths = newm.paths m.pathToDigest = newm.pathToDigest m.digestToPaths = newm.digestToPaths return nil @@ -234,7 +286,7 @@ func (m *Manifest) Blob() (Blob, error) { // Empty returns true if the manifest has no entries. func (m *Manifest) Empty() bool { - return len(m.pathToDigest) == 0 && len(m.digestToPaths) == 0 + return len(m.paths) == 0 && len(m.pathToDigest) == 0 && len(m.digestToPaths) == 0 } func splitManifest(data []byte, atEOF bool) (int, []byte, error) { diff --git a/private/pkg/manifest/storage.go b/private/pkg/manifest/storage.go deleted file mode 100644 index e70bfe197c..0000000000 --- a/private/pkg/manifest/storage.go +++ /dev/null @@ -1,193 +0,0 @@ -// Copyright 2020-2023 Buf Technologies, Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package manifest - -import ( - "context" - "fmt" - "io" - - "github.com/bufbuild/buf/private/pkg/normalpath" - "github.com/bufbuild/buf/private/pkg/storage" - "github.com/bufbuild/buf/private/pkg/storage/storageutil" - "go.uber.org/multierr" -) - -// manifestBucket is a storage.ReadBucket implementation from a manifest and an -// array of blobs. -type manifestBucket struct { - manifest Manifest - blobs BlobSet -} - -type manifestBucketObject struct { - path string - file io.ReadCloser -} - -func (o *manifestBucketObject) Path() string { return o.path } -func (o *manifestBucketObject) ExternalPath() string { return o.path } -func (o *manifestBucketObject) Read(p []byte) (int, error) { return o.file.Read(p) } -func (o *manifestBucketObject) Close() error { return o.file.Close() } - -// NewFromBucket creates a manifest and blob set from the bucket's files. Blobs -// in the blob set use the [DigestTypeShake256] digest. -func NewFromBucket( - ctx context.Context, - bucket storage.ReadBucket, -) (*Manifest, *BlobSet, error) { - var m Manifest - digester, err := NewDigester(DigestTypeShake256) - if err != nil { - return nil, nil, err - } - var blobs []Blob - if walkErr := bucket.Walk(ctx, "", func(info storage.ObjectInfo) (retErr error) { - path := info.Path() - obj, err := bucket.Get(ctx, path) - if err != nil { - return err - } - defer func() { retErr = multierr.Append(retErr, obj.Close()) }() - blob, err := NewMemoryBlobFromReaderWithDigester(obj, digester) - if err != nil { - return err - } - blobs = append(blobs, blob) - return m.AddEntry(path, *blob.Digest()) - }); walkErr != nil { - return nil, nil, walkErr - } - blobSet, err := NewBlobSet(ctx, blobs) // no need to pass validation options, we're building and digesting the blobs - if err != nil { - return nil, nil, err - } - return &m, blobSet, nil -} - -type bucketOptions struct { - allManifestBlobs bool - noExtraBlobs bool -} - -// BucketOption are options passed when creating a new manifest bucket. -type BucketOption func(*bucketOptions) - -// BucketWithAllManifestBlobsValidation validates that all manifest digests -// have a corresponding blob in the blob set. If this option is not passed, then -// buckets with partial/incomplete blobs are allowed. -func BucketWithAllManifestBlobsValidation() BucketOption { - return func(opts *bucketOptions) { - opts.allManifestBlobs = true - } -} - -// BucketWithNoExtraBlobsValidation validates that the passed blob set has no -// additional blobs beyond the ones in the manifest. -func BucketWithNoExtraBlobsValidation() BucketOption { - return func(opts *bucketOptions) { - opts.noExtraBlobs = true - } -} - -// NewBucket takes a manifest and a blob set and builds a readable storage -// bucket that contains the files in the manifest. -func NewBucket(m Manifest, blobs BlobSet, opts ...BucketOption) (storage.ReadBucket, error) { - var config bucketOptions - for _, option := range opts { - option(&config) - } - if config.allManifestBlobs { - if err := m.Range(func(path string, digest Digest) error { - if _, ok := blobs.BlobFor(digest.String()); !ok { - return fmt.Errorf("manifest path %q with digest %q has no associated blob", path, digest.String()) - } - return nil - }); err != nil { - return nil, err - } - } - if config.noExtraBlobs { - for digestStr := range blobs.digestToBlob { - if _, ok := m.PathsFor(digestStr); !ok { - return nil, fmt.Errorf("blob with digest %q is not present in the manifest", digestStr) - } - } - } - return &manifestBucket{ - manifest: m, - blobs: blobs, - }, nil -} - -// blobFor returns a blob for a given path. It returns the blob if found, or nil -// and ok=false if the path has no digest in the manifest, or if the blob for -// that digest is not present. -func (m *manifestBucket) blobFor(path string) (_ Blob, ok bool) { - digest, ok := m.manifest.DigestFor(path) - if !ok { - return nil, false - } - blob, ok := m.blobs.BlobFor(digest.String()) - if !ok { - return nil, false - } - return blob, true -} - -func (m *manifestBucket) Get(ctx context.Context, path string) (storage.ReadObjectCloser, error) { - blob, ok := m.blobFor(path) - if !ok { - return nil, storage.NewErrNotExist(path) - } - file, err := blob.Open(ctx) - if err != nil { - return nil, err - } - return &manifestBucketObject{ - path: path, - file: file, - }, nil -} - -func (m *manifestBucket) Stat(ctx context.Context, path string) (storage.ObjectInfo, error) { - if _, ok := m.blobFor(path); !ok { - return nil, storage.NewErrNotExist(path) - } - // storage.ObjectInfo only requires path - return &manifestBucketObject{path: path}, nil -} - -func (m *manifestBucket) Walk(ctx context.Context, prefix string, f func(storage.ObjectInfo) error) error { - prefix, err := storageutil.ValidatePrefix(prefix) - if err != nil { - return err - } - // walk order is not guaranteed - for _, path := range m.manifest.Paths() { - if !normalpath.EqualsOrContainsPath(prefix, path, normalpath.Relative) { - continue - } - if _, ok := m.blobFor(path); !ok { - // this could happen if the bucket was built with partial blobs - continue - } - // storage.ObjectInfo only requires path - if err := f(&manifestBucketObject{path: path}); err != nil { - return err - } - } - return nil -} diff --git a/private/pkg/storage/storagemanifest/bucket.go b/private/pkg/storage/storagemanifest/bucket.go new file mode 100644 index 0000000000..45975801fb --- /dev/null +++ b/private/pkg/storage/storagemanifest/bucket.go @@ -0,0 +1,140 @@ +// Copyright 2020-2023 Buf Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package storagemanifest + +import ( + "context" + "fmt" + + "github.com/bufbuild/buf/private/pkg/manifest" + "github.com/bufbuild/buf/private/pkg/normalpath" + "github.com/bufbuild/buf/private/pkg/storage" + "github.com/bufbuild/buf/private/pkg/storage/storageutil" +) + +type bucket struct { + manifest *manifest.Manifest + blobSet *manifest.BlobSet +} + +func newBucket( + m *manifest.Manifest, + blobSet *manifest.BlobSet, + options ...ReadBucketOption, +) (*bucket, error) { + readBucketOptions := newReadBucketOptions() + for _, option := range options { + option(readBucketOptions) + } + // TODO: why is this validation in newBucket, why is this not somewhere else? + // This should not be in storagemanifest. + if readBucketOptions.allManifestBlobs { + if err := m.Range(func(path string, digest manifest.Digest) error { + if _, ok := blobSet.BlobFor(digest.String()); !ok { + return fmt.Errorf("manifest path %q with digest %q has no associated blob", path, digest.String()) + } + return nil + }); err != nil { + return nil, err + } + } + if readBucketOptions.noExtraBlobs { + for _, blob := range blobSet.Blobs() { + digestString := blob.Digest().String() + if _, ok := m.PathsFor(digestString); !ok { + return nil, fmt.Errorf("blob with digest %q is not present in the manifest", digestString) + } + } + } + return &bucket{ + manifest: m, + blobSet: blobSet, + }, nil +} + +func (b *bucket) Get(ctx context.Context, path string) (storage.ReadObjectCloser, error) { + path, err := storageutil.ValidatePath(path) + if err != nil { + return nil, err + } + blob, ok := b.blobFor(path) + if !ok { + return nil, storage.NewErrNotExist(path) + } + file, err := blob.Open(ctx) + if err != nil { + return nil, err + } + return newReadObjectCloser(path, file), nil +} + +func (b *bucket) Stat(ctx context.Context, path string) (storage.ObjectInfo, error) { + path, err := storageutil.ValidatePath(path) + if err != nil { + return nil, err + } + if _, ok := b.blobFor(path); !ok { + return nil, storage.NewErrNotExist(path) + } + return storageutil.NewObjectInfo(path, path), nil +} + +func (b *bucket) Walk(ctx context.Context, prefix string, f func(storage.ObjectInfo) error) error { + prefix, err := storageutil.ValidatePrefix(prefix) + if err != nil { + return err + } + walkChecker := storageutil.NewWalkChecker() + for _, path := range b.manifest.Paths() { + if !normalpath.EqualsOrContainsPath(prefix, path, normalpath.Relative) { + continue + } + if err := walkChecker.Check(ctx); err != nil { + return err + } + if _, ok := b.blobFor(path); !ok { + // this could happen if the bucket was built with partial blobs + continue + } + if err := f(storageutil.NewObjectInfo(path, path)); err != nil { + return err + } + } + return nil +} + +// blobFor returns a blob for a given path. It returns the blob if found, or nil +// and ok=false if the path has no digest in the manifest, or if the blob for +// that digest is not present. +func (b *bucket) blobFor(path string) (_ manifest.Blob, ok bool) { + digest, ok := b.manifest.DigestFor(path) + if !ok { + return nil, false + } + blob, ok := b.blobSet.BlobFor(digest.String()) + if !ok { + return nil, false + } + return blob, true +} + +type readBucketOptions struct { + allManifestBlobs bool + noExtraBlobs bool +} + +func newReadBucketOptions() *readBucketOptions { + return &readBucketOptions{} +} diff --git a/private/pkg/storage/storagemanifest/read_object_closer.go b/private/pkg/storage/storagemanifest/read_object_closer.go new file mode 100644 index 0000000000..54d0109b21 --- /dev/null +++ b/private/pkg/storage/storagemanifest/read_object_closer.go @@ -0,0 +1,33 @@ +// Copyright 2020-2023 Buf Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package storagemanifest + +import ( + "io" + + "github.com/bufbuild/buf/private/pkg/storage/storageutil" +) + +type readObjectCloser struct { + storageutil.ObjectInfo + io.ReadCloser +} + +func newReadObjectCloser(path string, readCloser io.ReadCloser) *readObjectCloser { + return &readObjectCloser{ + ObjectInfo: storageutil.NewObjectInfo(path, path), + ReadCloser: readCloser, + } +} diff --git a/private/pkg/storage/storagemanifest/storagemanifest.go b/private/pkg/storage/storagemanifest/storagemanifest.go new file mode 100644 index 0000000000..48e5d35d32 --- /dev/null +++ b/private/pkg/storage/storagemanifest/storagemanifest.go @@ -0,0 +1,50 @@ +// Copyright 2020-2023 Buf Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package storagemanifest + +import ( + "github.com/bufbuild/buf/private/pkg/manifest" + "github.com/bufbuild/buf/private/pkg/storage" +) + +// NewReadBucket takes a Manifest and BlobSet and builds a ReadBucket +// that contains the files in the Manifest. +func NewReadBucket( + m *manifest.Manifest, + blobSet *manifest.BlobSet, + options ...ReadBucketOption, +) (storage.ReadBucket, error) { + return newBucket(m, blobSet, options...) +} + +// ReadBucketOption is an option for a passed when creating a new manifest bucket. +type ReadBucketOption func(*readBucketOptions) + +// ReadBucketWithAllManifestBlobs validates that all manifest digests +// have a corresponding blob in the blob set. If this option is not passed, then +// buckets with partial/incomplete blobs are allowed. +func ReadBucketWithAllManifestBlobs() ReadBucketOption { + return func(readBucketOptions *readBucketOptions) { + readBucketOptions.allManifestBlobs = true + } +} + +// ReadBucketWithNoExtraBlobs validates that the passed blob set has no +// additional blobs beyond the ones in the manifest. +func ReadBucketWithNoExtraBlobs() ReadBucketOption { + return func(readBucketOptions *readBucketOptions) { + readBucketOptions.noExtraBlobs = true + } +} diff --git a/private/pkg/manifest/storage_test.go b/private/pkg/storage/storagemanifest/storagemanifest_test.go similarity index 74% rename from private/pkg/manifest/storage_test.go rename to private/pkg/storage/storagemanifest/storagemanifest_test.go index beb9573bd2..54261f57a0 100644 --- a/private/pkg/manifest/storage_test.go +++ b/private/pkg/storage/storagemanifest/storagemanifest_test.go @@ -12,54 +12,24 @@ // See the License for the specific language governing permissions and // limitations under the License. -package manifest_test +package storagemanifest import ( "bytes" "context" - "fmt" "io" "strings" "testing" "github.com/bufbuild/buf/private/pkg/manifest" "github.com/bufbuild/buf/private/pkg/storage" - "github.com/bufbuild/buf/private/pkg/storage/storagemem" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -func TestFromBucket(t *testing.T) { - t.Parallel() - ctx := context.Background() - bucket, err := storagemem.NewReadBucket( - map[string][]byte{ - "null": nil, - "foo": []byte("bar"), - }) - require.NoError(t, err) - m, blobSet, err := manifest.NewFromBucket(ctx, bucket) - require.NoError(t, err) - // sorted by paths - var ( - fooDigest = mustDigestShake256(t, []byte("bar")) - nullDigest = mustDigestShake256(t, nil) - ) - expected := fmt.Sprintf("%s foo\n", fooDigest) - expected += fmt.Sprintf("%s null\n", nullDigest) - retContent, err := m.MarshalText() - assert.NoError(t, err) - assert.Equal(t, expected, string(retContent)) - // blobs - fooBlob, ok := blobSet.BlobFor(fooDigest.String()) - require.True(t, ok) - assert.True(t, fooDigest.Equal(*fooBlob.Digest())) - nullBlob, ok := blobSet.BlobFor(nullDigest.String()) - require.True(t, ok) - assert.True(t, nullDigest.Equal(*nullBlob.Digest())) -} +// TODO: adapt to the storagetesting framework -func TestNewBucket(t *testing.T) { +func TestNewReadBucket(t *testing.T) { t.Parallel() files := map[string][]byte{ "some_empty_file": {}, @@ -70,7 +40,7 @@ func TestNewBucket(t *testing.T) { // same "mypkg" prefix for `Walk` test purposes "mypkglongername/v1/baz.proto": []byte("repeated proto content"), } - var m manifest.Manifest + m := &manifest.Manifest{} var blobs []manifest.Blob digester, err := manifest.NewDigester(manifest.DigestTypeShake256) require.NoError(t, err) @@ -93,7 +63,7 @@ func TestNewBucket(t *testing.T) { ) require.NoError(t, err) - t.Run("BucketWithAllManifestBlobsValidation", func(t *testing.T) { + t.Run("BucketWithAllManifestBlobs", func(t *testing.T) { t.Parallel() // only send 3 blobs: there are 6 files with 4 different contents, // regardless of which blobs are sent, there will always be missing at least @@ -105,13 +75,13 @@ func TestNewBucket(t *testing.T) { ) require.NoError(t, err) - _, err = manifest.NewBucket( - m, *incompleteBlobSet, - manifest.BucketWithAllManifestBlobsValidation(), + _, err = NewReadBucket( + m, incompleteBlobSet, + ReadBucketWithAllManifestBlobs(), ) assert.Error(t, err) - bucket, err := manifest.NewBucket(m, *incompleteBlobSet) + bucket, err := NewReadBucket(m, incompleteBlobSet) assert.NoError(t, err) assert.NotNil(t, bucket) var bucketFilesCount int @@ -122,7 +92,7 @@ func TestNewBucket(t *testing.T) { assert.Less(t, bucketFilesCount, len(files)) // incomplete bucket }) - t.Run("BucketWithNoExtraBlobsValidation", func(t *testing.T) { + t.Run("BucketWithNoExtraBlobs", func(t *testing.T) { t.Parallel() const content = "some other file contents" digest := mustDigestShake256(t, []byte(content)) @@ -133,19 +103,19 @@ func TestNewBucket(t *testing.T) { append(blobs, orphanBlob), ) require.NoError(t, err) - _, err = manifest.NewBucket( - m, *tooLargeBlobSet, - manifest.BucketWithNoExtraBlobsValidation(), + _, err = NewReadBucket( + m, tooLargeBlobSet, + ReadBucketWithNoExtraBlobs(), ) assert.Error(t, err) }) t.Run("Valid", func(t *testing.T) { t.Parallel() - bucket, err := manifest.NewBucket( - m, *blobSet, - manifest.BucketWithAllManifestBlobsValidation(), - manifest.BucketWithNoExtraBlobsValidation(), + bucket, err := NewReadBucket( + m, blobSet, + ReadBucketWithAllManifestBlobs(), + ReadBucketWithNoExtraBlobs(), ) require.NoError(t, err) @@ -195,10 +165,9 @@ func TestNewBucket(t *testing.T) { }) } -func TestToBucketEmpty(t *testing.T) { +func TestNewReadBucketEmpty(t *testing.T) { t.Parallel() - var m manifest.Manifest - bucket, err := manifest.NewBucket(m, manifest.BlobSet{}) + bucket, err := NewReadBucket(&manifest.Manifest{}, &manifest.BlobSet{}) require.NoError(t, err) // make sure there are no files in the bucket require.NoError(t, bucket.Walk(context.Background(), "", func(obj storage.ObjectInfo) error { @@ -206,3 +175,12 @@ func TestToBucketEmpty(t *testing.T) { return nil })) } + +func mustDigestShake256(t *testing.T, content []byte) *manifest.Digest { + digester, err := manifest.NewDigester(manifest.DigestTypeShake256) + require.NoError(t, err) + require.NotNil(t, digester) + digest, err := digester.Digest(bytes.NewReader(content)) + require.NoError(t, err) + return digest +} diff --git a/private/pkg/storage/storagemanifest/usage.gen.go b/private/pkg/storage/storagemanifest/usage.gen.go new file mode 100644 index 0000000000..3fc2a2901b --- /dev/null +++ b/private/pkg/storage/storagemanifest/usage.gen.go @@ -0,0 +1,19 @@ +// Copyright 2020-2023 Buf Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Generated. DO NOT EDIT. + +package storagemanifest + +import _ "github.com/bufbuild/buf/private/usage"