refactor(oci-layout): use Parser to process option input

cmd/oras/attach.go

@@ -65,7 +65,7 @@ Example - Attach file 'hi.txt' and export the pushed manifest to 'manifest.json'
 `,
 		Args: cobra.MinimumNArgs(1),
 		PreRunE: func(cmd *cobra.Command, args []string) error {
-			return opts.ReadPassword()
+			return option.Parse(&opts)
 		},
 		RunE: func(cmd *cobra.Command, args []string) error {
 			opts.targetRef = args[0]

cmd/oras/blob/delete.go

@@ -59,7 +59,7 @@ Example - Delete a blob and print its descriptor:
 			if opts.OutputDescriptor && !opts.Confirmed {
 				return errors.New("must apply --force to confirm the deletion if the descriptor is outputted")
 			}
-			return opts.ReadPassword()
+			return option.Parse(&opts)
 		},
 		RunE: func(cmd *cobra.Command, args []string) error {
 			opts.targetRef = args[0]

cmd/oras/blob/fetch.go

@@ -70,7 +70,7 @@ Example - Fetch the blob, save it to a local file and print the descriptor:
 				return errors.New("`--output -` cannot be used with `--descriptor` at the same time")
 			}
 
-			return opts.ReadPassword()
+			return option.Parse(&opts)
 		},
 		Aliases: []string{"get"},
 		RunE: func(cmd *cobra.Command, args []string) error {

cmd/oras/blob/push.go

@@ -81,7 +81,7 @@ Example - Push blob without TLS:
 					return errors.New("`--size` must be provided if the blob is read from stdin")
 				}
 			}
-			return opts.ReadPassword()
+			return option.Parse(&opts)
 		},
 		RunE: func(cmd *cobra.Command, args []string) error {
 			return pushBlob(opts)

cmd/oras/cp.go

@@ -68,6 +68,9 @@ Example - Copy the artifact tagged with 'v1' from repository 'localhost:5000/net
   oras cp --concurrency 6 localhost:5000/net-monitor:v1 localhost:5000/net-monitor-copy:v1,tag2,tag3
 `,
 		Args: cobra.ExactArgs(2),
+		PreRunE: func(cmd *cobra.Command, args []string) error {
+			return option.Parse(&opts)
+		},
 		RunE: func(cmd *cobra.Command, args []string) error {
 			opts.srcRef = args[0]
 			refs := strings.Split(args[1], ",")
@@ -88,10 +91,6 @@ Example - Copy the artifact tagged with 'v1' from repository 'localhost:5000/net
 
 func runCopy(opts copyOptions) error {
 	ctx, _ := opts.SetLoggerLevel()
-	targetPlatform, err := opts.Parse()
-	if err != nil {
-		return err
-	}
 
 	// Prepare source
 	src, err := opts.src.NewRepository(opts.srcRef, opts.Common)
@@ -145,8 +144,8 @@ func runCopy(opts copyOptions) error {
 			copyOptions := oras.CopyOptions{
 				CopyGraphOptions: extendedCopyOptions.CopyGraphOptions,
 			}
-			if targetPlatform != nil {
-				copyOptions.WithTargetPlatform(targetPlatform)
+			if opts.Platform.Platform != nil {
+				copyOptions.WithTargetPlatform(opts.Platform.Platform)
 			}
 			desc, err = oras.Copy(ctx, src, opts.srcRef, dst, opts.dstRef, copyOptions)
 		}

cmd/oras/discover.go

@@ -64,7 +64,7 @@ Example - Discover referrers with type 'test-artifact' of manifest 'hello:latest
 `,
 		Args: cobra.ExactArgs(1),
 		PreRunE: func(cmd *cobra.Command, args []string) error {
-			return opts.ReadPassword()
+			return option.Parse(&opts)
 		},
 		RunE: func(cmd *cobra.Command, args []string) error {
 			opts.targetRef = args[0]
@@ -87,14 +87,10 @@ func runDiscover(opts discoverOptions) error {
 	if repo.Reference.Reference == "" {
 		return errors.NewErrInvalidReference(repo.Reference)
 	}
-	targetPlatform, err := opts.Parse()
-	if err != nil {
-		return err
-	}
 
 	// discover artifacts
 	resolveOpts := oras.DefaultResolveOptions
-	resolveOpts.TargetPlatform = targetPlatform
+	resolveOpts.TargetPlatform = opts.Platform.Platform
 	desc, err := oras.Resolve(ctx, repo, repo.Reference.Reference, resolveOpts)
 	if err != nil {
 		return err

cmd/oras/internal/option/applier.go

@@ -16,8 +16,6 @@ limitations under the License.
 package option
 
 import (
-	"reflect"
-
 	"github.com/spf13/pflag"
 )
 
@@ -31,14 +29,8 @@ type FlagApplier interface {
 // NOTE: The option argument need to be a pointer to the options, so its value
 // becomes addressable.
 func ApplyFlags(optsPtr interface{}, target *pflag.FlagSet) {
-	v := reflect.ValueOf(optsPtr).Elem()
-	for i := 0; i < v.NumField(); i++ {
-		f := v.Field(i)
-		if f.CanSet() {
-			iface := f.Addr().Interface()
-			if a, ok := iface.(FlagApplier); ok {
-				a.ApplyFlags(target)
-			}
-		}
-	}
+	rangeFields(optsPtr, func(fa FlagApplier) error {
+		fa.ApplyFlags(target)
+		return nil
+	})
 }

cmd/oras/internal/option/applier_test.go

@@ -0,0 +1,49 @@
+/*
+Copyright The ORAS Authors.
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package option_test
+
+import (
+	"testing"
+
+	"github.com/spf13/pflag"
+	"oras.land/oras/cmd/oras/internal/option"
+)
+
+func (t *Test) ApplyFlags(fs *pflag.FlagSet) {
+	*t.CntPtr += 1
+}
+
+func TestApplyFlags(t *testing.T) {
+	cnt := 0
+	type args struct {
+		Test
+	}
+	tests := []struct {
+		name    string
+		args    args
+		wantErr bool
+	}{
+		{"flags should be applied once", args{Test{CntPtr: &cnt}}, false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			option.ApplyFlags(&tt.args, nil)
+			if cnt != 1 {
+				t.Errorf("Expect ApplyFlags() to be called once but got %v", cnt)
+			}
+		})
+	}
+}

cmd/oras/internal/option/parser.go

@@ -0,0 +1,51 @@
+/*
+Copyright The ORAS Authors.
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package option
+
+import (
+	"reflect"
+)
+
+// FlagParser parses flags in an option.
+type FlagParser interface {
+	Parse() error
+}
+
+// Parse parses applicable fields of the passed-in option pointer and returns
+// error during parsing.
+func Parse(optsPtr interface{}) error {
+	return rangeFields(optsPtr, func(fp FlagParser) error {
+		return fp.Parse()
+	})
+}
+
+// rangeFields goes through all fields of ptr, optionally run fn if a field is
+// public AND typed T.
+func rangeFields[T any](ptr any, fn func(T) error) error {
+	v := reflect.ValueOf(ptr).Elem()
+	for i := 0; i < v.NumField(); i++ {
+		f := v.Field(i)
+		if f.CanSet() {
+			iface := f.Addr().Interface()
+			if opts, ok := iface.(T); ok {
+				if err := fn(opts); err != nil {
+					return err
+				}
+			}
+		}
+	}
+	return nil
+}

cmd/oras/internal/option/parser_test.go

@@ -0,0 +1,88 @@
+/*
+Copyright The ORAS Authors.
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package option_test
+
+import (
+	"errors"
+	"testing"
+
+	"oras.land/oras/cmd/oras/internal/option"
+)
+
+type Test struct {
+	CntPtr *int
+}
+
+func (t *Test) Parse() error {
+	*t.CntPtr += 1
+	if *t.CntPtr == 2 {
+		return errors.New("should not be tried twice")
+	}
+	return nil
+}
+
+func TestParse_once(t *testing.T) {
+	cnt := 0
+	type args struct {
+		Test
+	}
+	tests := []struct {
+		name    string
+		args    args
+		wantErr bool
+	}{
+		{"parse should be called once", args{Test{CntPtr: &cnt}}, false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if err := option.Parse(&tt.args); (err != nil) != tt.wantErr {
+				t.Errorf("Parse() error = %v, wantErr %v", err, tt.wantErr)
+			}
+
+			if cnt != 1 {
+				t.Errorf("Expect Parse() to be called once but got %v", cnt)
+			}
+		})
+	}
+}
+
+func TestParse_err(t *testing.T) {
+	cnt := 0
+	type args struct {
+		Test1 Test
+		Test2 Test
+		Test3 Test
+		Test4 Test
+	}
+	tests := []struct {
+		name    string
+		args    args
+		wantErr bool
+	}{
+		{"parse should be called twice and aborted with error", args{Test{CntPtr: &cnt}, Test{CntPtr: &cnt}, Test{CntPtr: &cnt}, Test{CntPtr: &cnt}}, true},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if err := option.Parse(&tt.args); (err != nil) != tt.wantErr {
+				t.Errorf("Parse() error = %v, wantErr %v", err, tt.wantErr)
+			}
+
+			if cnt != 2 {
+				t.Errorf("Expect Parse() to be called twice but got %v", cnt)
+			}
+		})
+	}
+}

cmd/oras/internal/option/platform.go

@@ -26,25 +26,26 @@ import (
 
 // Platform option struct.
 type Platform struct {
-	Platform string
+	platform string
+	Platform *ocispec.Platform
 }
 
 // ApplyFlags applies flags to a command flag set.
 func (opts *Platform) ApplyFlags(fs *pflag.FlagSet) {
-	fs.StringVarP(&opts.Platform, "platform", "", "", "request platform in the form of `os[/arch][/variant][:os_version]`")
+	fs.StringVarP(&opts.platform, "platform", "", "", "request platform in the form of `os[/arch][/variant][:os_version]`")
 }
 
 // parse parses the input platform flag to an oci platform type.
-func (opts *Platform) Parse() (*ocispec.Platform, error) {
-	if opts.Platform == "" {
-		return nil, nil
+func (opts *Platform) Parse() error {
+	if opts.platform == "" {
+		return nil
 	}
 
 	// OS[/Arch[/Variant]][:OSVersion]
 	// If Arch is not provided, will use GOARCH instead
 	var platformStr string
 	var p ocispec.Platform
-	platformStr, p.OSVersion, _ = strings.Cut(opts.Platform, ":")
+	platformStr, p.OSVersion, _ = strings.Cut(opts.platform, ":")
 	parts := strings.Split(platformStr, "/")
 	switch len(parts) {
 	case 3:
@@ -55,14 +56,15 @@ func (opts *Platform) Parse() (*ocispec.Platform, error) {
 	case 1:
 		p.Architecture = runtime.GOARCH
 	default:
-		return nil, fmt.Errorf("failed to parse platform %q: expected format os[/arch[/variant]]", opts.Platform)
+		return fmt.Errorf("failed to parse platform %q: expected format os[/arch[/variant]]", opts.platform)
 	}
 	p.OS = parts[0]
 	if p.OS == "" {
-		return nil, fmt.Errorf("invalid platform: OS cannot be empty")
+		return fmt.Errorf("invalid platform: OS cannot be empty")
 	}
 	if p.Architecture == "" {
-		return nil, fmt.Errorf("invalid platform: Architecture cannot be empty")
+		return fmt.Errorf("invalid platform: Architecture cannot be empty")
 	}
-	return &p, nil
+	opts.Platform = &p
+	return nil
 }