From 7b46393f081532b5af5d17635782a812df7100f1 Mon Sep 17 00:00:00 2001 From: JasmineTang Date: Tue, 20 Sep 2022 15:09:22 +0800 Subject: [PATCH 1/8] fix: add nil checks for some functions Signed-off-by: JasmineTang --- copy.go | 3 +++ extendedcopy.go | 8 +++++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/copy.go b/copy.go index a2f1e315..9192ec5d 100644 --- a/copy.go +++ b/copy.go @@ -64,6 +64,9 @@ type CopyOptions struct { // 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 { diff --git a/extendedcopy.go b/extendedcopy.go index 12e8611e..e5563c58 100644 --- a/extendedcopy.go +++ b/extendedcopy.go @@ -181,6 +181,9 @@ func findRoots(ctx context.Context, storage content.ReadOnlyGraphStorage, node o // when using both FilterArtifactType and FilterAnnotation, it's recommended to call // FilterArtifactType first. func (opts *ExtendedCopyGraphOptions) FilterAnnotation(key string, regex *regexp.Regexp) { + if key == "" { + return + } fp := opts.FindPredecessors opts.FindPredecessors = func(ctx context.Context, src content.ReadOnlyGraphStorage, desc ocispec.Descriptor) ([]ocispec.Descriptor, error) { var predecessors []ocispec.Descriptor @@ -223,7 +226,7 @@ func (opts *ExtendedCopyGraphOptions) FilterAnnotation(key string, regex *regexp } } } - if regex.MatchString(p.Annotations[key]) { + if regex == nil || regex.MatchString(p.Annotations[key]) { filtered = append(filtered, p) } } @@ -236,6 +239,9 @@ func (opts *ExtendedCopyGraphOptions) FilterAnnotation(key string, regex *regexp // 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 From 76dcf0704c0d0b1d815674199110080efb5c4795 Mon Sep 17 00:00:00 2001 From: JasmineTang Date: Tue, 20 Sep 2022 16:53:58 +0800 Subject: [PATCH 2/8] add unit test Signed-off-by: JasmineTang --- copy_test.go | 8 +++ extendedcopy_test.go | 162 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 170 insertions(+) diff --git a/copy_test.go b/copy_test.go index 159fb4c0..debaebb9 100644 --- a/copy_test.go +++ b/copy_test.go @@ -1058,6 +1058,14 @@ 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 + // 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") + } } func TestCopy_RestoreDuplicates(t *testing.T) { diff --git a/extendedcopy_test.go b/extendedcopy_test.go index d2835d06..2e9ce3a6 100644 --- a/extendedcopy_test.go +++ b/extendedcopy_test.go @@ -675,6 +675,120 @@ func TestExtendedCopyGraph_FilterAnnotationWithRegex(t *testing.T) { verifyCopy(dst, copiedIndice, uncopiedIndice) } +func TestExtendedCopyGraph_FilterAnnotationWithNoKey(t *testing.T) { + // generate test content + var blobs [][]byte + var descs []ocispec.Descriptor + appendBlob := func(mediaType string, blob []byte, key string, value string) { + blobs = append(blobs, blob) + descs = append(descs, ocispec.Descriptor{ + MediaType: mediaType, + Digest: digest.FromBytes(blob), + Size: int64(len(blob)), + Annotations: map[string]string{key: value}, + }) + } + generateArtifactManifest := func(subject ocispec.Descriptor, key string, value string) { + var manifest artifactspec.Manifest + artifactSubject := descriptor.OCIToArtifact(subject) + manifest.Subject = &artifactSubject + manifest.Annotations = map[string]string{key: value} + manifestJSON, err := json.Marshal(manifest) + if err != nil { + t.Fatal(err) + } + appendBlob(artifactspec.MediaTypeArtifactManifest, manifestJSON, key, value) + } + appendBlob(ocispec.MediaTypeImageLayer, []byte("foo"), "bar", "blackpink") // descs[0] + generateArtifactManifest(descs[0], "bar", "bluebrown") // descs[1] + generateArtifactManifest(descs[0], "bar", "blackred") // descs[2] + generateArtifactManifest(descs[0], "bar", "blackviolet") // descs[3] + generateArtifactManifest(descs[0], "bar", "greengrey") // descs[4] + generateArtifactManifest(descs[0], "bar", "brownblack") // descs[5] + ctx := context.Background() + src := memory.New() + for i := range blobs { + err := src.Push(ctx, descs[i], bytes.NewReader(blobs[i])) + if err != nil { + t.Fatalf("failed to push test content to src: %d: %v", i, err) + } + } + opts := oras.ExtendedCopyGraphOptions{} + exp := "black." + regex := regexp.MustCompile(exp) + opts.FilterAnnotation("", regex) + if opts.FindPredecessors != nil { + t.Fatal("FindPredecessors not nil!") + } +} + +func TestExtendedCopyGraph_FilterAnnotationWithNoRegex(t *testing.T) { + // generate test content + var blobs [][]byte + var descs []ocispec.Descriptor + appendBlob := func(mediaType string, blob []byte, key string, value string) { + blobs = append(blobs, blob) + descs = append(descs, ocispec.Descriptor{ + MediaType: mediaType, + Digest: digest.FromBytes(blob), + Size: int64(len(blob)), + Annotations: map[string]string{key: value}, + }) + } + generateArtifactManifest := func(subject ocispec.Descriptor, key string, value string) { + var manifest artifactspec.Manifest + artifactSubject := descriptor.OCIToArtifact(subject) + manifest.Subject = &artifactSubject + manifest.Annotations = map[string]string{key: value} + manifestJSON, err := json.Marshal(manifest) + if err != nil { + t.Fatal(err) + } + appendBlob(artifactspec.MediaTypeArtifactManifest, manifestJSON, key, value) + } + appendBlob(ocispec.MediaTypeImageLayer, []byte("foo"), "bar", "blackpink") // descs[0] + generateArtifactManifest(descs[0], "bar", "bluebrown") // descs[1] + generateArtifactManifest(descs[0], "bar", "blackred") // descs[2] + generateArtifactManifest(descs[0], "bar", "blackviolet") // descs[3] + generateArtifactManifest(descs[0], "bar", "greengrey") // descs[4] + generateArtifactManifest(descs[0], "bar", "brownblack") // descs[5] + ctx := context.Background() + verifyCopy := func(dst content.Fetcher, copiedIndice []int, uncopiedIndice []int) { + for _, i := range copiedIndice { + got, err := content.FetchAll(ctx, dst, descs[i]) + if err != nil { + t.Errorf("content[%d] error = %v, wantErr %v", i, err, false) + continue + } + if want := blobs[i]; !bytes.Equal(got, want) { + t.Errorf("content[%d] = %v, want %v", i, got, want) + } + } + for _, i := range uncopiedIndice { + if _, err := content.FetchAll(ctx, dst, descs[i]); !errors.Is(err, errdef.ErrNotFound) { + t.Errorf("content[%d] error = %v, wantErr %v", i, err, errdef.ErrNotFound) + } + } + } + src := memory.New() + for i := range blobs { + err := src.Push(ctx, descs[i], bytes.NewReader(blobs[i])) + if err != nil { + t.Fatalf("failed to push test content to src: %d: %v", i, err) + } + } + // test extended copy by descs[0] with annotation filter + 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) { // generate test content var blobs [][]byte @@ -889,6 +1003,54 @@ func TestExtendedCopyGraph_FilterArtifactTypeWithRegex(t *testing.T) { verifyCopy(dst, copiedIndice, uncopiedIndice) } +func TestExtendedCopyGraph_FilterArtifactTypeWithNoRegex(t *testing.T) { + // generate test content + var blobs [][]byte + var descs []ocispec.Descriptor + appendBlob := func(mediaType string, blob []byte) { + blobs = append(blobs, blob) + descs = append(descs, ocispec.Descriptor{ + MediaType: mediaType, + Digest: digest.FromBytes(blob), + Size: int64(len(blob)), + }) + } + generateArtifactManifest := func(subject ocispec.Descriptor, artifactType string) { + var manifest artifactspec.Manifest + artifactSubject := descriptor.OCIToArtifact(subject) + manifest.Subject = &artifactSubject + manifest.ArtifactType = artifactType + manifestJSON, err := json.Marshal(manifest) + if err != nil { + t.Fatal(err) + } + appendBlob(artifactspec.MediaTypeArtifactManifest, manifestJSON) + } + + appendBlob(ocispec.MediaTypeImageConfig, []byte("foo")) // descs[0] + generateArtifactManifest(descs[0], "good-bar-yellow") // descs[1] + generateArtifactManifest(descs[0], "bad-woo-red") // descs[2] + generateArtifactManifest(descs[0], "bad-bar-blue") // descs[3] + generateArtifactManifest(descs[0], "bad-bar-red") // descs[4] + generateArtifactManifest(descs[0], "good-woo-pink") // descs[5] + + ctx := context.Background() + + src := memory.New() + for i := range blobs { + err := src.Push(ctx, descs[i], bytes.NewReader(blobs[i])) + if err != nil { + t.Errorf("failed to push test content to src: %d: %v", i, err) + } + } + + opts := oras.ExtendedCopyGraphOptions{} + opts.FilterArtifactType(nil) + if opts.FindPredecessors != nil { + t.Fatal("FindPredecessors not nil!") + } +} + func TestExtendedCopyGraph_FilterArtifactTypeWithMultipleRegex(t *testing.T) { // generate test content var blobs [][]byte From 9a97082f9d82404e498327afe0b7c1b7f2f03803 Mon Sep 17 00:00:00 2001 From: JasmineTang Date: Wed, 21 Sep 2022 17:09:31 +0800 Subject: [PATCH 3/8] modify nil check in annotation filter and tests Signed-off-by: JasmineTang --- copy.go | 1 + copy_test.go | 20 ++++- extendedcopy.go | 9 +-- extendedcopy_test.go | 171 ++++++------------------------------------- 4 files changed, 46 insertions(+), 155 deletions(-) diff --git a/copy.go b/copy.go index 9192ec5d..cbb751e0 100644 --- a/copy.go +++ b/copy.go @@ -58,6 +58,7 @@ 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 operations will be done. // - 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 diff --git a/copy_test.go b/copy_test.go index debaebb9..819df5d4 100644 --- a/copy_test.go +++ b/copy_test.go @@ -1059,13 +1059,31 @@ func TestCopy_WithTargetPlatformOptions(t *testing.T) { t.Fatalf("Copy() error = %v, wantErr %v", err, expected) } - // test copy with no platform filter + // 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, + } + + _, 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) { diff --git a/extendedcopy.go b/extendedcopy.go index e5563c58..a2e6cd0e 100644 --- a/extendedcopy.go +++ b/extendedcopy.go @@ -181,9 +181,6 @@ func findRoots(ctx context.Context, storage content.ReadOnlyGraphStorage, node o // when using both FilterArtifactType and FilterAnnotation, it's recommended to call // FilterArtifactType first. func (opts *ExtendedCopyGraphOptions) FilterAnnotation(key string, regex *regexp.Regexp) { - if key == "" { - return - } fp := opts.FindPredecessors opts.FindPredecessors = func(ctx context.Context, src content.ReadOnlyGraphStorage, desc ocispec.Descriptor) ([]ocispec.Descriptor, error) { var predecessors []ocispec.Descriptor @@ -226,8 +223,10 @@ func (opts *ExtendedCopyGraphOptions) FilterAnnotation(key string, regex *regexp } } } - if regex == nil || regex.MatchString(p.Annotations[key]) { - filtered = append(filtered, p) + if _, ok := p.Annotations[key]; ok { + if regex == nil || regex.MatchString(p.Annotations[key]) { + filtered = append(filtered, p) + } } } return filtered, nil diff --git a/extendedcopy_test.go b/extendedcopy_test.go index 2e9ce3a6..b05cc059 100644 --- a/extendedcopy_test.go +++ b/extendedcopy_test.go @@ -673,119 +673,32 @@ func TestExtendedCopyGraph_FilterAnnotationWithRegex(t *testing.T) { copiedIndice := []int{0, 2, 3} uncopiedIndice := []int{1, 4, 5} verifyCopy(dst, copiedIndice, uncopiedIndice) -} -func TestExtendedCopyGraph_FilterAnnotationWithNoKey(t *testing.T) { - // generate test content - var blobs [][]byte - var descs []ocispec.Descriptor - appendBlob := func(mediaType string, blob []byte, key string, value string) { - blobs = append(blobs, blob) - descs = append(descs, ocispec.Descriptor{ - MediaType: mediaType, - Digest: digest.FromBytes(blob), - Size: int64(len(blob)), - Annotations: map[string]string{key: value}, - }) - } - generateArtifactManifest := func(subject ocispec.Descriptor, key string, value string) { - var manifest artifactspec.Manifest - artifactSubject := descriptor.OCIToArtifact(subject) - manifest.Subject = &artifactSubject - manifest.Annotations = map[string]string{key: value} - manifestJSON, err := json.Marshal(manifest) - if err != nil { - t.Fatal(err) - } - appendBlob(artifactspec.MediaTypeArtifactManifest, manifestJSON, key, value) - } - appendBlob(ocispec.MediaTypeImageLayer, []byte("foo"), "bar", "blackpink") // descs[0] - generateArtifactManifest(descs[0], "bar", "bluebrown") // descs[1] - generateArtifactManifest(descs[0], "bar", "blackred") // descs[2] - generateArtifactManifest(descs[0], "bar", "blackviolet") // descs[3] - generateArtifactManifest(descs[0], "bar", "greengrey") // descs[4] - generateArtifactManifest(descs[0], "bar", "brownblack") // descs[5] - ctx := context.Background() - src := memory.New() - for i := range blobs { - err := src.Push(ctx, descs[i], bytes.NewReader(blobs[i])) - if err != nil { - t.Fatalf("failed to push test content to src: %d: %v", i, err) - } - } - opts := oras.ExtendedCopyGraphOptions{} - exp := "black." - regex := regexp.MustCompile(exp) - opts.FilterAnnotation("", regex) - if opts.FindPredecessors != nil { - t.Fatal("FindPredecessors not nil!") - } -} + // 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) -func TestExtendedCopyGraph_FilterAnnotationWithNoRegex(t *testing.T) { - // generate test content - var blobs [][]byte - var descs []ocispec.Descriptor - appendBlob := func(mediaType string, blob []byte, key string, value string) { - blobs = append(blobs, blob) - descs = append(descs, ocispec.Descriptor{ - MediaType: mediaType, - Digest: digest.FromBytes(blob), - Size: int64(len(blob)), - Annotations: map[string]string{key: value}, - }) - } - generateArtifactManifest := func(subject ocispec.Descriptor, key string, value string) { - var manifest artifactspec.Manifest - artifactSubject := descriptor.OCIToArtifact(subject) - manifest.Subject = &artifactSubject - manifest.Annotations = map[string]string{key: value} - manifestJSON, err := json.Marshal(manifest) - if err != nil { - t.Fatal(err) - } - appendBlob(artifactspec.MediaTypeArtifactManifest, manifestJSON, key, value) - } - appendBlob(ocispec.MediaTypeImageLayer, []byte("foo"), "bar", "blackpink") // descs[0] - generateArtifactManifest(descs[0], "bar", "bluebrown") // descs[1] - generateArtifactManifest(descs[0], "bar", "blackred") // descs[2] - generateArtifactManifest(descs[0], "bar", "blackviolet") // descs[3] - generateArtifactManifest(descs[0], "bar", "greengrey") // descs[4] - generateArtifactManifest(descs[0], "bar", "brownblack") // descs[5] - ctx := context.Background() - verifyCopy := func(dst content.Fetcher, copiedIndice []int, uncopiedIndice []int) { - for _, i := range copiedIndice { - got, err := content.FetchAll(ctx, dst, descs[i]) - if err != nil { - t.Errorf("content[%d] error = %v, wantErr %v", i, err, false) - continue - } - if want := blobs[i]; !bytes.Equal(got, want) { - t.Errorf("content[%d] = %v, want %v", i, got, want) - } - } - for _, i := range uncopiedIndice { - if _, err := content.FetchAll(ctx, dst, descs[i]); !errors.Is(err, errdef.ErrNotFound) { - t.Errorf("content[%d] error = %v, wantErr %v", i, err, errdef.ErrNotFound) - } - } - } - src := memory.New() - for i := range blobs { - err := src.Push(ctx, descs[i], bytes.NewReader(blobs[i])) - if err != nil { - t.Fatalf("failed to push test content to src: %d: %v", i, err) - } + if err := oras.ExtendedCopyGraph(ctx, src, dst, descs[0], opts); err != nil { + t.Fatalf("ExtendedCopyGraph() error = %v, wantErr %v", err, false) } - // test extended copy by descs[0] with annotation filter - dst := memory.New() - opts := oras.ExtendedCopyGraphOptions{} + 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{} + copiedIndice = []int{0, 1, 2, 3, 4, 5} + uncopiedIndice = []int{} verifyCopy(dst, copiedIndice, uncopiedIndice) } @@ -1001,50 +914,10 @@ func TestExtendedCopyGraph_FilterArtifactTypeWithRegex(t *testing.T) { copiedIndice := []int{0, 1, 3, 4} uncopiedIndice := []int{2, 5} verifyCopy(dst, copiedIndice, uncopiedIndice) -} -func TestExtendedCopyGraph_FilterArtifactTypeWithNoRegex(t *testing.T) { - // generate test content - var blobs [][]byte - var descs []ocispec.Descriptor - appendBlob := func(mediaType string, blob []byte) { - blobs = append(blobs, blob) - descs = append(descs, ocispec.Descriptor{ - MediaType: mediaType, - Digest: digest.FromBytes(blob), - Size: int64(len(blob)), - }) - } - generateArtifactManifest := func(subject ocispec.Descriptor, artifactType string) { - var manifest artifactspec.Manifest - artifactSubject := descriptor.OCIToArtifact(subject) - manifest.Subject = &artifactSubject - manifest.ArtifactType = artifactType - manifestJSON, err := json.Marshal(manifest) - if err != nil { - t.Fatal(err) - } - appendBlob(artifactspec.MediaTypeArtifactManifest, manifestJSON) - } - - appendBlob(ocispec.MediaTypeImageConfig, []byte("foo")) // descs[0] - generateArtifactManifest(descs[0], "good-bar-yellow") // descs[1] - generateArtifactManifest(descs[0], "bad-woo-red") // descs[2] - generateArtifactManifest(descs[0], "bad-bar-blue") // descs[3] - generateArtifactManifest(descs[0], "bad-bar-red") // descs[4] - generateArtifactManifest(descs[0], "good-woo-pink") // descs[5] - - ctx := context.Background() - - src := memory.New() - for i := range blobs { - err := src.Push(ctx, descs[i], bytes.NewReader(blobs[i])) - if err != nil { - t.Errorf("failed to push test content to src: %d: %v", i, err) - } - } - - opts := oras.ExtendedCopyGraphOptions{} + // 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!") From 4acb6c5a4307e34c5d559e79c6713c6321f26040 Mon Sep 17 00:00:00 2001 From: JasmineTang Date: Thu, 22 Sep 2022 10:10:58 +0800 Subject: [PATCH 4/8] add test for multiple regex Signed-off-by: JasmineTang --- extendedcopy_test.go | 51 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/extendedcopy_test.go b/extendedcopy_test.go index b05cc059..9479d34b 100644 --- a/extendedcopy_test.go +++ b/extendedcopy_test.go @@ -773,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) { @@ -997,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) { From 8c0cb4eef3f00ed9e98d76d4246bc9005d822e78 Mon Sep 17 00:00:00 2001 From: JasmineTang Date: Thu, 22 Sep 2022 15:35:02 +0800 Subject: [PATCH 5/8] add to docs and refactor Signed-off-by: JasmineTang --- extendedcopy.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/extendedcopy.go b/extendedcopy.go index a2e6cd0e..856d2c65 100644 --- a/extendedcopy.go +++ b/extendedcopy.go @@ -223,10 +223,8 @@ func (opts *ExtendedCopyGraphOptions) FilterAnnotation(key string, regex *regexp } } } - if _, ok := p.Annotations[key]; ok { - if regex == nil || regex.MatchString(p.Annotations[key]) { - filtered = append(filtered, p) - } + if value, ok := p.Annotations[key]; ok && (regex == nil || regex.MatchString(value)) { + filtered = append(filtered, p) } } return filtered, nil @@ -234,8 +232,9 @@ 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 operations will be done. 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 { From 346339302127f603184e762deff4c33a43304364 Mon Sep 17 00:00:00 2001 From: JasmineTang Date: Thu, 22 Sep 2022 16:42:38 +0800 Subject: [PATCH 6/8] modify docs Signed-off-by: JasmineTang --- copy.go | 2 +- copy_test.go | 1 + extendedcopy.go | 14 ++++++++++---- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/copy.go b/copy.go index cbb751e0..ce012bdd 100644 --- a/copy.go +++ b/copy.go @@ -58,7 +58,7 @@ 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 operations will be done. +// - 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 diff --git a/copy_test.go b/copy_test.go index d9e81c43..8698cdbc 100644 --- a/copy_test.go +++ b/copy_test.go @@ -1079,6 +1079,7 @@ func TestCopy_WithTargetPlatformOptions(t *testing.T) { }, CopyGraphOptions: oras.DefaultCopyGraphOptions, } + opts.WithTargetPlatform(nil) _, err = oras.Copy(ctx, src, ref, dst, "", opts) if !errors.Is(err, errdef.ErrNotFound) { diff --git a/extendedcopy.go b/extendedcopy.go index 856d2c65..7f3d01c0 100644 --- a/extendedcopy.go +++ b/extendedcopy.go @@ -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. +// - If the key is available in current predecessor's annotation and regex exists, +// return the predecessor if it's annotation matches the regex. +// - If the key is available in current predecessor's annotation and regex does not +// exist, return current predecessor. +// - If the key is unavailable in current predecessor's annotation, current predecessor +// will not be returned. +// 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) { @@ -233,7 +239,7 @@ 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. When the regex pattern is nil, -// no operations will be done. For performance consideration, when using both +// 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) { From 107dcd562658bbea52bd10d3d1dc444a93ec26d8 Mon Sep 17 00:00:00 2001 From: JasmineTang Date: Thu, 22 Sep 2022 17:04:23 +0800 Subject: [PATCH 7/8] revise docs Signed-off-by: JasmineTang --- extendedcopy.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/extendedcopy.go b/extendedcopy.go index 7f3d01c0..f9eca1c5 100644 --- a/extendedcopy.go +++ b/extendedcopy.go @@ -177,12 +177,12 @@ 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. -// - If the key is available in current predecessor's annotation and regex exists, -// return the predecessor if it's annotation matches the regex. -// - If the key is available in current predecessor's annotation and regex does not -// exist, return current predecessor. -// - If the key is unavailable in current predecessor's annotation, current predecessor +// 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. // For performance consideration, when using both FilterArtifactType and // FilterAnnotation, it's recommended to call FilterArtifactType first. From 9fecd8ab0512ee26ef4af235bcd99c3de79e9e0a Mon Sep 17 00:00:00 2001 From: JasmineTang Date: Thu, 22 Sep 2022 17:18:18 +0800 Subject: [PATCH 8/8] revise docs Signed-off-by: JasmineTang --- extendedcopy.go | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/extendedcopy.go b/extendedcopy.go index f9eca1c5..befb1424 100644 --- a/extendedcopy.go +++ b/extendedcopy.go @@ -176,14 +176,9 @@ 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 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. +// 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) {