refined the code

Signed-off-by: Xiaoxuan Wang <xiaoxuanwang@microsoft.com>
oras-project · Sep 20, 2024 · 6b0349b · 6b0349b
1 parent 119acee
commit 6b0349b
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 18 deletions.
diff --git a/cmd/oras/internal/option/annotation.go b/cmd/oras/internal/option/annotation.go
@@ -32,19 +32,21 @@ var (
 
 // Packer option struct.
 type Annotation struct {
-	ManifestAnnotationFlags []string                     // raw input of manifest annotation flags
-	Annotations             map[string]map[string]string // parsed manifest and config annotations
+	// ManifestAnnotations contains raw input of manifest annotation "key=value" pairs
+	ManifestAnnotations []string
+
+	// Annotations contains parsed manifest and config annotations
+	Annotations map[string]map[string]string
 }
 
 // ApplyFlags applies flags to a command flag set.
 func (opts *Annotation) ApplyFlags(fs *pflag.FlagSet) {
-	fs.StringArrayVarP(&opts.ManifestAnnotationFlags, "annotation", "a", nil, "manifest annotations")
+	fs.StringArrayVarP(&opts.ManifestAnnotations, "annotation", "a", nil, "manifest annotations")
 }
 
 func (opts *Annotation) Parse(*cobra.Command) error {
-	opts.Annotations = make(map[string]map[string]string)
 	manifestAnnotations := make(map[string]string)
-	for _, anno := range opts.ManifestAnnotationFlags {
+	for _, anno := range opts.ManifestAnnotations {
 		key, val, success := strings.Cut(anno, "=")
 		if !success {
 			return &oerrors.Error{
@@ -57,6 +59,8 @@ func (opts *Annotation) Parse(*cobra.Command) error {
 		}
 		manifestAnnotations[key] = val
 	}
-	opts.Annotations[AnnotationManifest] = manifestAnnotations
+	opts.Annotations = map[string]map[string]string{
+		AnnotationManifest: manifestAnnotations,
+	}
 	return nil
 }
diff --git a/cmd/oras/internal/option/packer.go b/cmd/oras/internal/option/packer.go
@@ -74,6 +74,7 @@ func (opts *Packer) ExportManifest(ctx context.Context, fetcher content.Fetcher,
 	}
 	return os.WriteFile(opts.ManifestExportPath, manifestBytes, 0666)
 }
+
 func (opts *Packer) Parse(cmd *cobra.Command) error {
 	if !opts.PathValidationDisabled {
 		var failedPaths []string
@@ -95,22 +96,22 @@ func (opts *Packer) Parse(cmd *cobra.Command) error {
 }
 
 // parseAnnotations loads the manifest annotation map.
-func (opts *Packer) parseAnnotations(cmd *cobra.Command) (err error) {
-	if opts.AnnotationFilePath != "" && len(opts.ManifestAnnotationFlags) != 0 {
+func (opts *Packer) parseAnnotations(cmd *cobra.Command) error {
+	if opts.AnnotationFilePath != "" && len(opts.ManifestAnnotations) != 0 {
 		return errAnnotationConflict
 	}
 	if opts.AnnotationFilePath != "" {
-		if err = decodeJSON(opts.AnnotationFilePath, &opts.Annotations); err != nil {
+		if err := decodeJSON(opts.AnnotationFilePath, &opts.Annotations); err != nil {
 			return &oerrors.Error{
 				Err:            fmt.Errorf(`invalid annotation json file: failed to load annotations from %s`, opts.AnnotationFilePath),
 				Recommendation: `Annotation file doesn't match the required format. Please refer to the document at https://oras.land/docs/how_to_guides/manifest_annotations`,
 			}
 		}
 	}
-	if len(opts.ManifestAnnotationFlags) != 0 {
+	if len(opts.ManifestAnnotations) != 0 {
 		return opts.Annotation.Parse(cmd)
 	}
-	return
+	return nil
 }
 
 // decodeJSON decodes a json file v to filename.

diff --git a/cmd/oras/internal/option/packer_test.go b/cmd/oras/internal/option/packer_test.go
@@ -40,7 +40,7 @@ func TestPacker_FlagInit(t *testing.T) {
 func TestPacker_parseAnnotations_err(t *testing.T) {
 	opts := Packer{
 		Annotation: Annotation{
-			ManifestAnnotationFlags: []string{"Key=Val"},
+			ManifestAnnotations: []string{"Key=Val"},
 		},
 		AnnotationFilePath: "this is not a file", // testFile,
 	}
@@ -57,7 +57,7 @@ func TestPacker_parseAnnotations_err(t *testing.T) {
 
 	opts = Packer{
 		Annotation: Annotation{
-			ManifestAnnotationFlags: []string{"KeyVal"},
+			ManifestAnnotations: []string{"KeyVal"},
 		},
 	}
 	if err := opts.parseAnnotations(nil); !errors.Is(err, errAnnotationFormat) {
@@ -66,7 +66,7 @@ func TestPacker_parseAnnotations_err(t *testing.T) {
 
 	opts = Packer{
 		Annotation: Annotation{
-			ManifestAnnotationFlags: []string{"Key=Val1", "Key=Val2"},
+			ManifestAnnotations: []string{"Key=Val1", "Key=Val2"},
 		},
 	}
 	if err := opts.parseAnnotations(nil); !errors.Is(err, errAnnotationDuplication) {
@@ -100,7 +100,7 @@ func TestPacker_parseAnnotations_annotationFlag(t *testing.T) {
 	}
 	opts := Packer{
 		Annotation: Annotation{
-			ManifestAnnotationFlags: invalidFlag0,
+			ManifestAnnotations: invalidFlag0,
 		},
 	}
 	err := opts.parseAnnotations(nil)
@@ -115,7 +115,7 @@ func TestPacker_parseAnnotations_annotationFlag(t *testing.T) {
 	}
 	opts = Packer{
 		Annotation: Annotation{
-			ManifestAnnotationFlags: invalidFlag1,
+			ManifestAnnotations: invalidFlag1,
 		},
 	}
 	err = opts.parseAnnotations(nil)
@@ -131,7 +131,7 @@ func TestPacker_parseAnnotations_annotationFlag(t *testing.T) {
 	}
 	opts = Packer{
 		Annotation: Annotation{
-			ManifestAnnotationFlags: validFlag,
+			ManifestAnnotations: validFlag,
 		},
 	}
 	err = opts.parseAnnotations(nil)

diff --git a/cmd/oras/root/attach_test.go b/cmd/oras/root/attach_test.go
@@ -34,7 +34,7 @@ func Test_runAttach_errType(t *testing.T) {
 	opts := &attachOptions{
 		Packer: option.Packer{
 			Annotation: option.Annotation{
-				ManifestAnnotationFlags: []string{"one", "two"},
+				ManifestAnnotations: []string{"one", "two"},
 			},
 			AnnotationFilePath: "/tmp/whatever",
 		},