Skip to content

Commit

Permalink
fix: add nil checks for some functions (#327)
Browse files Browse the repository at this point in the history
Signed-off-by: JasmineTang <jasminetang@microsoft.com>
  • Loading branch information
jasminetMSFT authored Sep 22, 2022
1 parent bedc4c4 commit 3f9653f
Show file tree
Hide file tree
Showing 4 changed files with 129 additions and 7 deletions.
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
19 changes: 12 additions & 7 deletions extendedcopy.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,10 +176,11 @@ func findRoots(ctx context.Context, storage content.ReadOnlyGraphStorage, node o
return roots, nil
}

// 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.
// FilterAnnotation will configure opts.FindPredecessors to filter the
// predecessors whose annotation matches a given regex pattern. 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 +224,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 +233,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

0 comments on commit 3f9653f

Please sign in to comment.