Skip to content

Commit

Permalink
Separate matchers as a bucket filter (#3229)
Browse files Browse the repository at this point in the history
Adds a new read bucket Filter that works exclusively on Matchers. Buckets that use Matchers as Mappers have been split as a Mapper and Filter bucket. The method MapPrefix on Mappers has been removed. Matchers no longer implement Mappers.
  • Loading branch information
emcfarlane authored Aug 21, 2024
1 parent 5b37673 commit bc5ed90
Show file tree
Hide file tree
Showing 14 changed files with 188 additions and 141 deletions.
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 {
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)
// 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

0 comments on commit bc5ed90

Please sign in to comment.