From 721518aad213b335908d35cedd99cb88db99ee0d Mon Sep 17 00:00:00 2001 From: Junjie Gao Date: Wed, 22 Jan 2025 06:47:56 +0000 Subject: [PATCH] fix: resolve comments Signed-off-by: Junjie Gao --- cmd/notation/inspect_test.go | 4 +- cmd/notation/internal/display/handler.go | 4 +- .../internal/display/metadata/interface.go | 2 + .../internal/display/metadata/json/inspect.go | 9 ++-- .../internal/display/metadata/tree/inspect.go | 12 +---- cmd/notation/internal/option/format.go | 19 ++++---- cmd/notation/internal/output/json.go | 41 ---------------- cmd/notation/internal/output/json_test.go | 48 ------------------- cmd/notation/internal/output/print.go | 13 ++++- cmd/notation/internal/output/print_test.go | 18 ++++++- internal/cmd/flags.go | 9 ---- internal/ioutil/print.go | 13 ----- test/e2e/suite/command/inspect.go | 2 +- 13 files changed, 50 insertions(+), 144 deletions(-) delete mode 100644 cmd/notation/internal/output/json.go delete mode 100644 cmd/notation/internal/output/json_test.go diff --git a/cmd/notation/inspect_test.go b/cmd/notation/inspect_test.go index bd437c783..a9c8bcba3 100644 --- a/cmd/notation/inspect_test.go +++ b/cmd/notation/inspect_test.go @@ -24,7 +24,7 @@ func TestInspectCommand_SecretsFromArgs(t *testing.T) { opts := &inspectOpts{} command := inspectCommand(opts) format := option.Format{ - FormatFlag: "text", + CurrentFormat: "text", } format.SetTypes(option.FormatTypeText, option.FormatTypeJSON) expected := &inspectOpts{ @@ -60,7 +60,7 @@ func TestInspectCommand_SecretsFromEnv(t *testing.T) { format := option.Format{} format.SetTypes(option.FormatTypeText, option.FormatTypeJSON) - format.FormatFlag = "json" + format.CurrentFormat = "json" expected := &inspectOpts{ reference: "ref", SecureFlagOpts: SecureFlagOpts{ diff --git a/cmd/notation/internal/display/handler.go b/cmd/notation/internal/display/handler.go index 60a296786..99bdbd311 100644 --- a/cmd/notation/internal/display/handler.go +++ b/cmd/notation/internal/display/handler.go @@ -27,11 +27,11 @@ import ( // NewInpsectHandler creates a new InspectHandler based on the output format. func NewInpsectHandler(printer *output.Printer, format option.Format) (metadata.InspectHandler, error) { - switch option.FormatType(format.FormatFlag) { + switch option.FormatType(format.CurrentFormat) { case option.FormatTypeJSON: return json.NewInspectHandler(printer), nil case option.FormatTypeText: return tree.NewInspectHandler(printer), nil } - return nil, fmt.Errorf("unrecognized output format %s", format.FormatFlag) + return nil, fmt.Errorf("unrecognized output format %s", format.CurrentFormat) } diff --git a/cmd/notation/internal/display/metadata/interface.go b/cmd/notation/internal/display/metadata/interface.go index 9424abc5c..9dbae52cd 100644 --- a/cmd/notation/internal/display/metadata/interface.go +++ b/cmd/notation/internal/display/metadata/interface.go @@ -11,6 +11,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +// Package metadata provides interfaces for handlers that render metadata +// information for each command. package metadata import ( diff --git a/cmd/notation/internal/display/metadata/json/inspect.go b/cmd/notation/internal/display/metadata/json/inspect.go index 7f28fb3fe..8903d4c67 100644 --- a/cmd/notation/internal/display/metadata/json/inspect.go +++ b/cmd/notation/internal/display/metadata/json/inspect.go @@ -35,7 +35,7 @@ type inspectOutput struct { Signatures []*signature `json:"signatures"` } -// signature is the signature envelope for printing in human readable format. +// signature is the signature envelope for printing in JSON format. type signature struct { Digest string `json:"digest,omitempty"` SignatureAlgorithm string `json:"signatureAlgorithm"` @@ -46,8 +46,7 @@ type signature struct { SignedArtifact ocispec.Descriptor `json:"signedArtifact"` } -// certificate is the certificate information for printing in human readable -// format. +// certificate is the certificate information for printing in JSON format. type certificate struct { SHA256Fingerprint string `json:"SHA256Fingerprint"` IssuedTo string `json:"issuedTo"` @@ -55,7 +54,7 @@ type certificate struct { Expiry time.Time `json:"expiry"` } -// timestamp is the timestamp information for printing in human readable. +// timestamp is the timestamp information for printing in JSON format. type timestamp struct { Timestamp string `json:"timestamp,omitempty"` Certificates []*certificate `json:"certificates,omitempty"` @@ -83,7 +82,7 @@ func NewInspectHandler(printer *output.Printer) *InspectHandler { // OnReferenceResolved sets the artifact reference and media type for the // handler. // -// the reference is no-op for this handler. +// The reference is no-op for this handler. func (h *InspectHandler) OnReferenceResolved(_, mediaType string) { h.output.MediaType = mediaType } diff --git a/cmd/notation/internal/display/metadata/tree/inspect.go b/cmd/notation/internal/display/metadata/tree/inspect.go index 3391a9e6c..2fdcc1012 100644 --- a/cmd/notation/internal/display/metadata/tree/inspect.go +++ b/cmd/notation/internal/display/metadata/tree/inspect.go @@ -18,6 +18,7 @@ import ( "crypto/x509" "encoding/hex" "fmt" + "maps" "slices" "strconv" "strings" @@ -121,7 +122,7 @@ func addUserDefinedAttributes(node *tree.Node, annotations map[string]string) { userDefinedAttributesNode.Add("(empty)") return } - for _, k := range orderedKeys(annotations) { + for _, k := range slices.Sorted(maps.Keys(annotations)) { v := annotations[k] userDefinedAttributesNode.AddPair(k, v) } @@ -176,15 +177,6 @@ func addCertificates(node *tree.Node, certChain []*x509.Certificate) { } } -func orderedKeys(m map[string]string) []string { - keys := make([]string, 0, len(m)) - for k := range m { - keys = append(keys, k) - } - slices.Sort(keys) - return keys -} - func formatTime(t time.Time) string { return t.Format(time.ANSIC) } diff --git a/cmd/notation/internal/option/format.go b/cmd/notation/internal/option/format.go index 5585fa695..c0f4bdd40 100644 --- a/cmd/notation/internal/option/format.go +++ b/cmd/notation/internal/option/format.go @@ -30,6 +30,7 @@ package option import ( "fmt" + "slices" "strings" "github.com/spf13/cobra" @@ -49,13 +50,13 @@ var ( // Format contains input and parsed options for formatted output flags. type Format struct { - FormatFlag string - allowedTypes []FormatType + CurrentFormat string + allowedTypes []FormatType } // SetTypes sets the default format type and allowed format types. func (f *Format) SetTypes(defaultType FormatType, otherTypes ...FormatType) { - f.FormatFlag = string(defaultType) + f.CurrentFormat = string(defaultType) f.allowedTypes = append(otherTypes, defaultType) } @@ -67,16 +68,14 @@ func (f *Format) ApplyFlags(fs *pflag.FlagSet) { } usage := fmt.Sprintf("output format, options: %s", strings.Join(quotedAllowedTypes, ", ")) // apply flags - fs.StringVar(&f.FormatFlag, "output", f.FormatFlag, usage) + fs.StringVarP(&f.CurrentFormat, "output", "o", f.CurrentFormat, usage) } // Parse parses the input format flag. func (opts *Format) Parse(_ *cobra.Command) error { - for _, t := range opts.allowedTypes { - if opts.FormatFlag == string(t) { - // type validation passed - return nil - } + if ok := slices.Contains(opts.allowedTypes, FormatType(opts.CurrentFormat)); ok { + // type validation passed + return nil } - return fmt.Errorf("invalid format type: %q", opts.FormatFlag) + return fmt.Errorf("invalid format type: %q", opts.CurrentFormat) } diff --git a/cmd/notation/internal/output/json.go b/cmd/notation/internal/output/json.go deleted file mode 100644 index 667dff568..000000000 --- a/cmd/notation/internal/output/json.go +++ /dev/null @@ -1,41 +0,0 @@ -// Copyright The Notary Project 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. - -// copied and adopted from https://github.com/oras-project/oras with -// modification -/* -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 output - -import ( - "encoding/json" - "io" -) - -// PrintPrettyJSON prints the object to the writer in JSON format. -func PrintPrettyJSON(out io.Writer, object any) error { - encoder := json.NewEncoder(out) - encoder.SetIndent("", " ") - return encoder.Encode(object) -} diff --git a/cmd/notation/internal/output/json_test.go b/cmd/notation/internal/output/json_test.go deleted file mode 100644 index 893f4e42c..000000000 --- a/cmd/notation/internal/output/json_test.go +++ /dev/null @@ -1,48 +0,0 @@ -// Copyright The Notary Project 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. - -// copied and adopted from https://github.com/oras-project/oras with -// modification -/* -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 output - -import ( - "strings" - "testing" -) - -func Test_PrintPrettyJSON(t *testing.T) { - builder := &strings.Builder{} - given := map[string]int{"bob": 5} - expected := "{\n \"bob\": 5\n}\n" - err := PrintPrettyJSON(builder, given) - if err != nil { - t.Error("Expected no error got <" + err.Error() + ">") - } - actual := builder.String() - if expected != actual { - t.Error("Expected <" + expected + "> not equal to actual <" + actual + ">") - } -} diff --git a/cmd/notation/internal/output/print.go b/cmd/notation/internal/output/print.go index cf10931e3..3c95145da 100644 --- a/cmd/notation/internal/output/print.go +++ b/cmd/notation/internal/output/print.go @@ -29,6 +29,7 @@ limitations under the License. package output import ( + "encoding/json" "fmt" "io" "sync" @@ -50,6 +51,7 @@ func NewPrinter(out io.Writer, err io.Writer) *Printer { func (p *Printer) Write(b []byte) (int, error) { p.lock.Lock() defer p.lock.Unlock() + return p.out.Write(b) } @@ -57,6 +59,7 @@ func (p *Printer) Write(b []byte) (int, error) { func (p *Printer) Println(a ...any) error { p.lock.Lock() defer p.lock.Unlock() + _, err := fmt.Fprintln(p.out, a...) if err != nil { err = fmt.Errorf("display output error: %w", err) @@ -70,11 +73,19 @@ func (p *Printer) Println(a ...any) error { func (p *Printer) Printf(format string, a ...any) error { p.lock.Lock() defer p.lock.Unlock() + _, err := fmt.Fprintf(p.out, format, a...) if err != nil { err = fmt.Errorf("display output error: %w", err) _, _ = fmt.Fprint(p.err, err) + return err } - // Errors are handled above, so return nil return nil } + +// PrintPrettyJSON prints object to out in JSON format. +func PrintPrettyJSON(out io.Writer, object any) error { + encoder := json.NewEncoder(out) + encoder.SetIndent("", " ") + return encoder.Encode(object) +} diff --git a/cmd/notation/internal/output/print_test.go b/cmd/notation/internal/output/print_test.go index d8e6f58ee..0d07ff1f1 100644 --- a/cmd/notation/internal/output/print_test.go +++ b/cmd/notation/internal/output/print_test.go @@ -80,8 +80,22 @@ func TestPrinter_Print(t *testing.T) { if mockWriter.errorCount != 2 { t.Errorf("Expected two errors actual <%d>", mockWriter.errorCount) } - if err != nil { - t.Error("Expected error to be ignored") + if err == nil { + t.Error("Expected error got ") } }) } + +func Test_PrintPrettyJSON(t *testing.T) { + builder := &strings.Builder{} + given := map[string]int{"bob": 5} + expected := "{\n \"bob\": 5\n}\n" + err := PrintPrettyJSON(builder, given) + if err != nil { + t.Error("Expected no error got <" + err.Error() + ">") + } + actual := builder.String() + if expected != actual { + t.Error("Expected <" + expected + "> not equal to actual <" + actual + ">") + } +} diff --git a/internal/cmd/flags.go b/internal/cmd/flags.go index a2bed995b..3c7c04640 100644 --- a/internal/cmd/flags.go +++ b/internal/cmd/flags.go @@ -121,15 +121,6 @@ var ( SetPflagReferrersTag = func(fs *pflag.FlagSet, p *bool, usage string) { fs.BoolVar(p, PflagReferrersTag.Name, true, usage) } - - PflagOutput = &pflag.Flag{ - Name: "output", - Shorthand: "o", - } - PflagOutputUsage = fmt.Sprintf("output format, options: '%s', '%s'", OutputJSON, OutputPlaintext) - SetPflagOutput = func(fs *pflag.FlagSet, p *string, usage string) { - fs.StringVarP(p, PflagOutput.Name, PflagOutput.Shorthand, OutputPlaintext, usage) - } ) // KeyValueSlice is a flag with type int diff --git a/internal/ioutil/print.go b/internal/ioutil/print.go index a7a3bac6c..2eedb794e 100644 --- a/internal/ioutil/print.go +++ b/internal/ioutil/print.go @@ -14,7 +14,6 @@ package ioutil import ( - "encoding/json" "fmt" "io" "path/filepath" @@ -79,15 +78,3 @@ func PrintCertMap(w io.Writer, certPaths []string) error { } return tw.Flush() } - -// PrintObjectAsJSON takes an interface and prints it as an indented JSON string -func PrintObjectAsJSON(i interface{}) error { - jsonBytes, err := json.MarshalIndent(i, "", " ") - if err != nil { - return err - } - - fmt.Println(string(jsonBytes)) - - return nil -} diff --git a/test/e2e/suite/command/inspect.go b/test/e2e/suite/command/inspect.go index 3414a0dc9..e896e1bff 100644 --- a/test/e2e/suite/command/inspect.go +++ b/test/e2e/suite/command/inspect.go @@ -375,7 +375,7 @@ localhost:5000/e2e-inspect-invalid-timstamped@sha256:f1da8cd70d6d851fa2313c8d661 ] } ` - notation.Exec("inspect", "--output", "json", artifact.ReferenceWithDigest()). + notation.Exec("inspect", "-o", "json", artifact.ReferenceWithDigest()). MatchContent(expectedOutput) }) })