fix: add nil checks for some functions (#327)

Signed-off-by: JasmineTang <jasminetang@microsoft.com>

Author: jasminetMSFT

copy.go

@@ -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 {

copy_test.go

@@ -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) {

extendedcopy.go

@@ -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) {
@@ -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)
 			}
 		}
@@ -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

extendedcopy_test.go

@@ -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) {
@@ -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) {
@@ -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) {
@@ -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) {