Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Separate matchers as a bucket filter #3229

Merged
merged 4 commits into from
Aug 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion private/buf/bufformat/bufformat.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func FormatModuleSet(ctx context.Context, moduleSet bufmodule.ModuleSet) (_ stor
// FormatBucket formats the .proto files in the bucket and returns a new bucket with the formatted files.
func FormatBucket(ctx context.Context, bucket storage.ReadBucket) (_ storage.ReadBucket, retErr error) {
readWriteBucket := storagemem.NewReadWriteBucket()
paths, err := storage.AllPaths(ctx, storage.MapReadBucket(bucket, storage.MatchPathExt(".proto")), "")
paths, err := storage.AllPaths(ctx, storage.FilterReadBucket(bucket, storage.MatchPathExt(".proto")), "")
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion private/buf/bufprotoc/bufprotoc.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func NewModuleSetForProtoc(
}
// need to do match extension here
// https://github.com/bufbuild/buf/issues/113
rootBuckets = append(rootBuckets, storage.MapReadBucket(rootBucket, storage.MatchPathExt(".proto")))
rootBuckets = append(rootBuckets, storage.FilterReadBucket(rootBucket, storage.MatchPathExt(".proto")))
}
targetPaths, err := slicesext.MapError(
absFilePaths,
Expand Down
17 changes: 11 additions & 6 deletions private/buf/bufworkspace/workspace_targeting.go
Original file line number Diff line number Diff line change
Expand Up @@ -527,10 +527,12 @@ func getMappedModuleBucketAndModuleTargeting(
for root, excludes := range rootToExcludes {
// Roots only applies to .proto files.
mappers := []storage.Mapper{
storage.MapOnPrefix(root),
}
matchers := []storage.Matcher{
// need to do match extension here
// https://github.com/bufbuild/buf/issues/113
storage.MatchPathExt(".proto"),
storage.MapOnPrefix(root),
}
if len(excludes) != 0 {
var notOrMatchers []storage.Matcher
Expand All @@ -540,8 +542,8 @@ func getMappedModuleBucketAndModuleTargeting(
storage.MatchPathContained(exclude),
)
}
mappers = append(
mappers,
matchers = append(
matchers,
storage.MatchNot(
storage.MatchOr(
notOrMatchers...,
Expand All @@ -551,9 +553,12 @@ func getMappedModuleBucketAndModuleTargeting(
}
rootBuckets = append(
rootBuckets,
storage.MapReadBucket(
moduleBucket,
mappers...,
storage.FilterReadBucket(
storage.MapReadBucket(
moduleBucket,
mappers...,
),
matchers...,
),
)
}
Expand Down
4 changes: 2 additions & 2 deletions private/bufpkg/bufmodule/digest.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ func getB4Digest(
ctx,
// This is extreme defensive programming. We've gone out of our way to make sure
// that the bucket is already filtered, but it's just too important to mess up here.
storage.MapReadBucket(bucketWithStorageMatcherApplied, getStorageMatcher(ctx, bucketWithStorageMatcherApplied)),
storage.FilterReadBucket(bucketWithStorageMatcherApplied, getStorageMatcher(ctx, bucketWithStorageMatcherApplied)),
"",
func(readObject storage.ReadObject) error {
digest, err := bufcas.NewDigestForContent(readObject)
Expand Down Expand Up @@ -373,7 +373,7 @@ func getFilesDigestForB5Digest(
ctx,
// This is extreme defensive programming. We've gone out of our way to make sure
// that the bucket is already filtered, but it's just too important to mess up here.
storage.MapReadBucket(bucketWithStorageMatcherApplied, getStorageMatcher(ctx, bucketWithStorageMatcherApplied)),
storage.FilterReadBucket(bucketWithStorageMatcherApplied, getStorageMatcher(ctx, bucketWithStorageMatcherApplied)),
"",
func(readObject storage.ReadObject) error {
digest, err := bufcas.NewDigestForContent(readObject)
Expand Down
2 changes: 1 addition & 1 deletion private/bufpkg/bufmodule/paths.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ func getSyncOnceValuesGetBucketWithStorageMatcherApplied(
if err != nil {
return nil, err
}
return storage.MapReadBucket(bucket, getStorageMatcher(ctx, bucket)), nil
return storage.FilterReadBucket(bucket, getStorageMatcher(ctx, bucket)), nil
},
)
}
4 changes: 2 additions & 2 deletions private/pkg/git/cloner.go
Original file line number Diff line number Diff line change
Expand Up @@ -240,8 +240,8 @@ func (c *cloner) CloneToBucket(
return err
}
var readBucket storage.ReadBucket = tmpReadWriteBucket
if options.Mapper != nil {
readBucket = storage.MapReadBucket(readBucket, options.Mapper)
if options.Matcher != nil {
readBucket = storage.FilterReadBucket(readBucket, options.Matcher)
}
_, err = storage.Copy(ctx, readBucket, writeBucket)
return err
Expand Down
2 changes: 1 addition & 1 deletion private/pkg/git/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ type Cloner interface {

// CloneToBucketOptions are options for Clone.
type CloneToBucketOptions struct {
Mapper storage.Mapper
Matcher storage.Matcher
Name Name
RecurseSubmodules bool
}
Expand Down
2 changes: 1 addition & 1 deletion private/pkg/git/git_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ func readBucketForName(ctx context.Context, t *testing.T, runner command.Runner,
depth,
readWriteBucket,
CloneToBucketOptions{
Mapper: storage.MatchPathExt(".proto"),
Matcher: storage.MatchPathExt(".proto"),
Name: name,
RecurseSubmodules: recurseSubmodules,
},
Expand Down
2 changes: 1 addition & 1 deletion private/pkg/protostat/protostatstorage/file_walker.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ type fileWalker struct {

func newFileWalker(readBucket storage.ReadBucket) *fileWalker {
return &fileWalker{
readBucket: storage.MapReadBucket(
readBucket: storage.FilterReadBucket(
readBucket,
storage.MatchPathExt(".proto"),
),
Expand Down
108 changes: 108 additions & 0 deletions private/pkg/storage/filter.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
// Copyright 2020-2024 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 storage

import (
"context"
"io/fs"

"github.com/bufbuild/buf/private/pkg/normalpath"
)

// FilterReadBucket filters the ReadBucket.
//
// If the Matchers are empty, the original ReadBucket is returned.
// If there is more than one Matcher, the Matchers are anded together.
func FilterReadBucket(readBucket ReadBucket, matchers ...Matcher) ReadBucket {
Copy link
Contributor Author

@emcfarlane emcfarlane Aug 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chose FilterReadBucket over MatchReadBucket, the naming of filter with the filter funcs being called matchers I think is still clear. We could also rename Matchers to FIlters?

if len(matchers) == 0 {
return readBucket
}
return newFilterReadBucketCloser(readBucket, nil, MatchAnd(matchers...))
}

// FilterReadBucketCloser filters the ReadBucketCloser.
//
// If the Matchers are empty, the original ReadBucketCloser is returned.
// If there is more than one Matcher, the Matchers are anded together.
func FilterReadBucketCloser(readBucketCloser ReadBucketCloser, matchers ...Matcher) ReadBucketCloser {
if len(matchers) == 0 {
return readBucketCloser
}
return newFilterReadBucketCloser(readBucketCloser, readBucketCloser.Close, MatchAnd(matchers...))
}

type filterReadBucketCloser struct {
delegate ReadBucket
closeFunc func() error
matcher Matcher
}

func newFilterReadBucketCloser(
delegate ReadBucket,
closeFunc func() error,
matcher Matcher,
) *filterReadBucketCloser {
return &filterReadBucketCloser{
delegate: delegate,
closeFunc: closeFunc,
matcher: matcher,
}
}

func (r *filterReadBucketCloser) Get(ctx context.Context, path string) (ReadObjectCloser, error) {
path, err := normalpath.NormalizeAndValidate(path)
if err != nil {
return nil, err
}
if !r.matcher.MatchPath(path) {
return nil, &fs.PathError{Op: "read", Path: path, Err: fs.ErrNotExist}
}
return r.delegate.Get(ctx, path)
}

func (r *filterReadBucketCloser) Stat(ctx context.Context, path string) (ObjectInfo, error) {
path, err := normalpath.NormalizeAndValidate(path)
if err != nil {
return nil, err
}
if !r.matcher.MatchPath(path) {
return nil, &fs.PathError{Op: "read", Path: path, Err: fs.ErrNotExist}
}
return r.delegate.Stat(ctx, path)
}

func (r *filterReadBucketCloser) Walk(ctx context.Context, prefix string, f func(ObjectInfo) error) error {
prefix, err := normalpath.NormalizeAndValidate(prefix)
if err != nil {
return err
}
return r.delegate.Walk(
ctx,
prefix,
func(objectInfo ObjectInfo) error {
if !r.matcher.MatchPath(objectInfo.Path()) {
return nil
}
return f(objectInfo)
},
)
}

func (r *filterReadBucketCloser) Close() error {
if r.closeFunc != nil {
return r.closeFunc()
}
return nil
}
16 changes: 8 additions & 8 deletions private/pkg/storage/map.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import (
//
// If the Mappers are empty, the original ReadBucket is returned.
// If there is more than one Mapper, the Mappers are called in order
// for UnmapFullPath, with the order reversed for MapPath and MapPrefix.
// for UnmapFullPath, with the order reversed for MapPath.
//
// That is, order these assuming you are starting with a full path and
// working to a path.
Expand All @@ -44,7 +44,7 @@ func MapReadBucket(readBucket ReadBucket, mappers ...Mapper) ReadBucket {
//
// If the Mappers are empty, the original ReadBucketCloser is returned.
// If there is more than one Mapper, the Mappers are called in order
// for UnmapFullPath, with the order reversed for MapPath and MapPrefix.
// for UnmapFullPath, with the order reversed for MapPath.
//
// That is, order these assuming you are starting with a full path and
// working to a path.
Expand All @@ -59,7 +59,7 @@ func MapReadBucketCloser(readBucketCloser ReadBucketCloser, mappers ...Mapper) R
//
// If the Mappers are empty, the original WriteBucket is returned.
// If there is more than one Mapper, the Mappers are called in order
// for UnmapFullPath, with the order reversed for MapPath and MapPrefix.
// for UnmapFullPath, with the order reversed for MapPath.
//
// That is, order these assuming you are starting with a full path and
// working to a path.
Expand All @@ -76,7 +76,7 @@ func MapWriteBucket(writeBucket WriteBucket, mappers ...Mapper) WriteBucket {
//
// If the Mappers are empty, the original WriteBucketCloser is returned.
// If there is more than one Mapper, the Mappers are called in order
// for UnmapFullPath, with the order reversed for MapPath and MapPrefix.
// for UnmapFullPath, with the order reversed for MapPath.
//
// That is, order these assuming you are starting with a full path and
// working to a path.
Expand All @@ -93,7 +93,7 @@ func MapWriteBucketCloser(writeBucketCloser WriteBucketCloser, mappers ...Mapper
//
// If the Mappers are empty, the original ReadWriteBucket is returned.
// If there is more than one Mapper, the Mappers are called in order
// for UnmapFullPath, with the order reversed for MapPath and MapPrefix.
// for UnmapFullPath, with the order reversed for MapPath.
//
// That is, order these assuming you are starting with a full path and
// working to a path.
Expand All @@ -113,7 +113,7 @@ func MapReadWriteBucket(readWriteBucket ReadWriteBucket, mappers ...Mapper) Read
//
// If the Mappers are empty, the original ReadWriteBucketCloser is returned.
// If there is more than one Mapper, the Mappers are called in order
// for UnmapFullPath, with the order reversed for MapPath and MapPrefix.
// for UnmapFullPath, with the order reversed for MapPath.
//
// That is, order these assuming you are starting with a full path and
// working to a path.
Expand Down Expand Up @@ -178,7 +178,7 @@ func (r *mapReadBucketCloser) Walk(ctx context.Context, prefix string, f func(Ob
if err != nil {
return err
}
fullPrefix, matches := r.mapper.MapPrefix(prefix)
fullPrefix, matches := r.mapper.MapPath(prefix)
if !matches {
return nil
}
Expand Down Expand Up @@ -264,7 +264,7 @@ func (w *mapWriteBucketCloser) DeleteAll(ctx context.Context, prefix string) err
if err != nil {
return err
}
fullPrefix, matches := w.mapper.MapPrefix(prefix)
fullPrefix, matches := w.mapper.MapPath(prefix)
if !matches {
return nil
}
Expand Down
20 changes: 1 addition & 19 deletions private/pkg/storage/mapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,6 @@ type Mapper interface {
// The returned path is expected to be normalized and validated.
// If the path cannot be mapped, this returns false.
MapPath(path string) (string, bool)
// Map maps the prefix to the full prefix.
//
// The path is expected to be normalized and validated.
// The returned path is expected to be normalized and validated.
// If the path cannot be mapped, this returns false.
MapPrefix(prefix string) (string, bool)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MapPrefix can be removed and replaced with calls to MapPath now that Matchers won't restrict the mappers.

// UnmapFullPath maps the full path to the path.
//
// Returns false if the full path does not apply.
Expand All @@ -58,7 +52,7 @@ func MapOnPrefix(prefix string) Mapper {
//
// If the Mappers are empty, a no-op Mapper is returned.
// If there is more than one Mapper, the Mappers are called in order
// for UnmapFullPath, with the order reversed for MapPath and MapPrefix.
// for UnmapFullPath, with the order reversed for MapPath.
//
// That is, order these assuming you are starting with a full path and
// working to a path.
Expand All @@ -83,10 +77,6 @@ func (p prefixMapper) MapPath(path string) (string, bool) {
return normalpath.Join(p.prefix, path), true
}

func (p prefixMapper) MapPrefix(prefix string) (string, bool) {
return normalpath.Join(p.prefix, prefix), true
}

func (p prefixMapper) UnmapFullPath(fullPath string) (string, bool, error) {
if !normalpath.EqualsOrContainsPath(p.prefix, fullPath, normalpath.Relative) {
return "", false, nil
Expand All @@ -108,10 +98,6 @@ func (c chainMapper) MapPath(path string) (string, bool) {
return c.mapFunc(path, Mapper.MapPath)
}

func (c chainMapper) MapPrefix(prefix string) (string, bool) {
return c.mapFunc(prefix, Mapper.MapPrefix)
}

func (c chainMapper) UnmapFullPath(fullPath string) (string, bool, error) {
path := fullPath
var matches bool
Expand Down Expand Up @@ -152,10 +138,6 @@ func (n nopMapper) MapPath(path string) (string, bool) {
return path, true
}

func (n nopMapper) MapPrefix(prefix string) (string, bool) {
return prefix, true
}

func (nopMapper) UnmapFullPath(fullPath string) (string, bool, error) {
return fullPath, true, nil
}
Expand Down
Loading
Loading