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

fix: add nil checks for some functions #327

Merged
merged 11 commits into from
Sep 22, 2022
4 changes: 4 additions & 0 deletions copy.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,16 @@ type CopyOptions struct {
// WithTargetPlatform configures opts.MapRoot to select the manifest whose
// platform matches the given platform. When MapRoot is provided, the platform
// selection will be applied on the mapped root node.
// - If the given platform is nil, no platform selection will be applied.
// - If the root node is a manifest, it will remain the same if platform
// matches, otherwise ErrNotFound will be returned.
// - If the root node is a manifest list, it will be mapped to the first
// matching manifest if exists, otherwise ErrNotFound will be returned.
// - Otherwise ErrUnsupported will be returned.
func (opts *CopyOptions) WithTargetPlatform(p *ocispec.Platform) {
if p == nil {
return
}
mapRoot := opts.MapRoot
opts.MapRoot = func(ctx context.Context, src content.ReadOnlyStorage, root ocispec.Descriptor) (desc ocispec.Descriptor, err error) {
if mapRoot != nil {
Expand Down
27 changes: 27 additions & 0 deletions copy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1058,6 +1058,33 @@ func TestCopy_WithTargetPlatformOptions(t *testing.T) {
if err.Error() != expected {
t.Fatalf("Copy() error = %v, wantErr %v", err, expected)
}

// test copy with no platform filter and nil opts.MapRoot
// opts.MapRoot should be nil
opts = oras.CopyOptions{}
opts.WithTargetPlatform(nil)
if opts.MapRoot != nil {
t.Fatal("opts.MapRoot not equal to nil when platform is not provided")
}

// test copy with no platform filter and custom opts.MapRoot
// should return ErrNotFound
opts = oras.CopyOptions{
MapRoot: func(ctx context.Context, src content.ReadOnlyStorage, root ocispec.Descriptor) (ocispec.Descriptor, error) {
if root.MediaType == "test" {
return root, nil
} else {
return ocispec.Descriptor{}, errdef.ErrNotFound
}
},
CopyGraphOptions: oras.DefaultCopyGraphOptions,
}
opts.WithTargetPlatform(nil)

_, err = oras.Copy(ctx, src, ref, dst, "", opts)
if !errors.Is(err, errdef.ErrNotFound) {
t.Fatalf("Copy() error = %v, wantErr %v", err, errdef.ErrNotFound)
}
}

func TestCopy_RestoreDuplicates(t *testing.T) {
Expand Down
22 changes: 16 additions & 6 deletions extendedcopy.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,9 +177,15 @@ func findRoots(ctx context.Context, storage content.ReadOnlyGraphStorage, node o
}

// FilterAnnotation will configure opts.FindPredecessors to filter the predecessors
// whose annotation matches a given regex pattern. For performance consideration,
// when using both FilterArtifactType and FilterAnnotation, it's recommended to call
// FilterArtifactType first.
// whose annotation matches a given regex pattern. For each predecessor,
// - If key is available in the predecessor's annotation and regex is not nil,
// return the predecessor if the annotation value matches the regex.
// - If key is available in the predecessor's annotation and regex is nil,
// return the predecessor.
// - If key is unavailable in the predecessor's annotation, the predecessor
// will not be returned.
Copy link
Contributor

@shizhMSFT shizhMSFT Sep 22, 2022

Choose a reason for hiding this comment

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

A predecessor is kept if the key is in its annotation and matches the regex if present.

// For performance consideration, when using both FilterArtifactType and
// FilterAnnotation, it's recommended to call FilterArtifactType first.
func (opts *ExtendedCopyGraphOptions) FilterAnnotation(key string, regex *regexp.Regexp) {
fp := opts.FindPredecessors
opts.FindPredecessors = func(ctx context.Context, src content.ReadOnlyGraphStorage, desc ocispec.Descriptor) ([]ocispec.Descriptor, error) {
Expand Down Expand Up @@ -223,7 +229,7 @@ func (opts *ExtendedCopyGraphOptions) FilterAnnotation(key string, regex *regexp
}
}
}
if regex.MatchString(p.Annotations[key]) {
if value, ok := p.Annotations[key]; ok && (regex == nil || regex.MatchString(value)) {
filtered = append(filtered, p)
}
}
Expand All @@ -232,10 +238,14 @@ func (opts *ExtendedCopyGraphOptions) FilterAnnotation(key string, regex *regexp
}

// FilterArtifactType will configure opts.FindPredecessors to filter the predecessors
// whose artifact type matches a given regex pattern. For performance consideration,
// when using both FilterArtifactType and FilterAnnotation, it's recommended to call
// whose artifact type matches a given regex pattern. When the regex pattern is nil,
// no artifact type filter will be applied. For performance consideration, when using both
// FilterArtifactType and FilterAnnotation, it's recommended to call
// FilterArtifactType first.
func (opts *ExtendedCopyGraphOptions) FilterArtifactType(regex *regexp.Regexp) {
if regex == nil {
return
}
fp := opts.FindPredecessors
opts.FindPredecessors = func(ctx context.Context, src content.ReadOnlyGraphStorage, desc ocispec.Descriptor) ([]ocispec.Descriptor, error) {
var predecessors []ocispec.Descriptor
Expand Down
86 changes: 86 additions & 0 deletions extendedcopy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -673,6 +673,33 @@ func TestExtendedCopyGraph_FilterAnnotationWithRegex(t *testing.T) {
copiedIndice := []int{0, 2, 3}
uncopiedIndice := []int{1, 4, 5}
verifyCopy(dst, copiedIndice, uncopiedIndice)

// test FilterAnnotation with key unavailable in predecessors' annotation
// should return nothing
dst = memory.New()
opts = oras.ExtendedCopyGraphOptions{}
exp = "black."
regex = regexp.MustCompile(exp)
opts.FilterAnnotation("bar1", regex)

if err := oras.ExtendedCopyGraph(ctx, src, dst, descs[0], opts); err != nil {
t.Fatalf("ExtendedCopyGraph() error = %v, wantErr %v", err, false)
}
copiedIndice = []int{0}
uncopiedIndice = []int{1, 2, 3, 4, 5}
verifyCopy(dst, copiedIndice, uncopiedIndice)

//test FilterAnnotation with key available in predecessors' annotation, regex equal to nil
//should return all predecessors with the provided key
dst = memory.New()
opts = oras.ExtendedCopyGraphOptions{}
opts.FilterAnnotation("bar", nil)
if err := oras.ExtendedCopyGraph(ctx, src, dst, descs[0], opts); err != nil {
t.Fatalf("ExtendedCopyGraph() error = %v, wantErr %v", err, false)
}
copiedIndice = []int{0, 1, 2, 3, 4, 5}
uncopiedIndice = []int{}
verifyCopy(dst, copiedIndice, uncopiedIndice)
}

func TestExtendedCopyGraph_FilterAnnotationWithMultipleRegex(t *testing.T) {
Expand Down Expand Up @@ -746,6 +773,39 @@ func TestExtendedCopyGraph_FilterAnnotationWithMultipleRegex(t *testing.T) {
copiedIndice := []int{0, 2}
uncopiedIndice := []int{1, 3, 4, 5, 6}
verifyCopy(dst, copiedIndice, uncopiedIndice)

// test extended copy by descs[0] with three annotation filters, nil included
dst = memory.New()
opts = oras.ExtendedCopyGraphOptions{}
exp1 = "black."
exp2 = ".pink|red"
regex1 = regexp.MustCompile(exp1)
regex2 = regexp.MustCompile(exp2)
opts.FilterAnnotation("bar", regex1)
opts.FilterAnnotation("bar", nil)
opts.FilterAnnotation("bar", regex2)
if err := oras.ExtendedCopyGraph(ctx, src, dst, descs[0], opts); err != nil {
t.Fatalf("ExtendedCopyGraph() error = %v, wantErr %v", err, false)
}
copiedIndice = []int{0, 2}
uncopiedIndice = []int{1, 3, 4, 5, 6}
verifyCopy(dst, copiedIndice, uncopiedIndice)

// test extended copy by descs[0] with two annotation filters, the second filter has an unavailable key
dst = memory.New()
opts = oras.ExtendedCopyGraphOptions{}
exp1 = "black."
exp2 = ".pink|red"
regex1 = regexp.MustCompile(exp1)
regex2 = regexp.MustCompile(exp2)
opts.FilterAnnotation("bar", regex1)
opts.FilterAnnotation("test", regex2)
if err := oras.ExtendedCopyGraph(ctx, src, dst, descs[0], opts); err != nil {
t.Fatalf("ExtendedCopyGraph() error = %v, wantErr %v", err, false)
}
copiedIndice = []int{0}
uncopiedIndice = []int{1, 2, 3, 4, 5, 6}
verifyCopy(dst, copiedIndice, uncopiedIndice)
}

func TestExtendedCopyGraph_FilterAnnotationWithRegexNoAnnotationInDescriptor(t *testing.T) {
Expand Down Expand Up @@ -887,6 +947,14 @@ func TestExtendedCopyGraph_FilterArtifactTypeWithRegex(t *testing.T) {
copiedIndice := []int{0, 1, 3, 4}
uncopiedIndice := []int{2, 5}
verifyCopy(dst, copiedIndice, uncopiedIndice)

// test extended copy by descs[0] with no regex
// type matches exp.
opts = oras.ExtendedCopyGraphOptions{}
opts.FilterArtifactType(nil)
if opts.FindPredecessors != nil {
t.Fatal("FindPredecessors not nil!")
}
}

func TestExtendedCopyGraph_FilterArtifactTypeWithMultipleRegex(t *testing.T) {
Expand Down Expand Up @@ -962,6 +1030,24 @@ func TestExtendedCopyGraph_FilterArtifactTypeWithMultipleRegex(t *testing.T) {
copiedIndice := []int{0, 3, 4}
uncopiedIndice := []int{1, 2, 5}
verifyCopy(dst, copiedIndice, uncopiedIndice)

// test extended copy by descs[0], include the predecessors whose artifact
// type matches exp1 and exp2 and nil
exp1 = ".foo|bar."
exp2 = "bad."
dst = memory.New()
opts = oras.ExtendedCopyGraphOptions{}
regex1 = regexp.MustCompile(exp1)
regex2 = regexp.MustCompile(exp2)
opts.FilterArtifactType(regex1)
opts.FilterArtifactType(regex2)
opts.FilterArtifactType(nil)
if err := oras.ExtendedCopyGraph(ctx, src, dst, descs[0], opts); err != nil {
t.Errorf("ExtendedCopyGraph() error = %v, wantErr %v", err, false)
}
copiedIndice = []int{0, 3, 4}
uncopiedIndice = []int{1, 2, 5}
verifyCopy(dst, copiedIndice, uncopiedIndice)
}

func TestExtendedCopyGraph_FilterArtifactTypeByReferrersWithMultipleRegex(t *testing.T) {
Expand Down