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,7 @@ 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, err *error) {
+		fa.ApplyFlags(target)
+	})
 }

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, err *error) {
+		*err = 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 {
+				err := new(error)
+				fn(opts, err)
+				if *err != nil {
+					return *err
+				}
+			}
+		}
+	}
+	return nil
+}

cmd/oras/internal/option/parser_test.go

@@ -0,0 +1,86 @@
+/*
+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

@@ -3,9 +3,7 @@ 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.
@@ -26,25 +24,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 +54,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
 }