From cf4f25f035a0b5f307d9a8b456a38f4bd0358d38 Mon Sep 17 00:00:00 2001 From: Terry Howe Date: Thu, 20 Jun 2024 08:48:46 -0600 Subject: [PATCH] refactor: Pass printer to text handler Signed-off-by: Terry Howe --- cmd/oras/internal/display/handler.go | 31 ++++++++++--------- cmd/oras/internal/display/handler_test.go | 10 ++++-- .../internal/display/metadata/text/attach.go | 14 ++++----- .../internal/display/metadata/text/pull.go | 23 +++++++------- .../internal/display/metadata/text/push.go | 20 +++++------- .../display/metadata/text/push_test.go | 6 ++-- cmd/oras/internal/display/status/text.go | 13 ++++---- cmd/oras/internal/output/print.go | 18 +++++++++++ cmd/oras/internal/output/print_test.go | 6 +++- cmd/oras/root/attach.go | 4 ++- cmd/oras/root/pull.go | 4 ++- cmd/oras/root/push.go | 4 ++- cmd/oras/root/push_test.go | 2 +- 13 files changed, 92 insertions(+), 63 deletions(-) diff --git a/cmd/oras/internal/display/handler.go b/cmd/oras/internal/display/handler.go index 367df14ec..4a5b85e36 100644 --- a/cmd/oras/internal/display/handler.go +++ b/cmd/oras/internal/display/handler.go @@ -32,15 +32,16 @@ import ( "oras.land/oras/cmd/oras/internal/display/status" "oras.land/oras/cmd/oras/internal/errors" "oras.land/oras/cmd/oras/internal/option" + "oras.land/oras/cmd/oras/internal/output" ) // NewPushHandler returns status and metadata handlers for push command. -func NewPushHandler(out io.Writer, format option.Format, tty *os.File, verbose bool) (status.PushHandler, metadata.PushHandler, error) { +func NewPushHandler(printer *output.Printer, format option.Format, tty *os.File) (status.PushHandler, metadata.PushHandler, error) { var statusHandler status.PushHandler if tty != nil { statusHandler = status.NewTTYPushHandler(tty) } else if format.Type == option.FormatTypeText.Name { - statusHandler = status.NewTextPushHandler(out, verbose) + statusHandler = status.NewTextPushHandler(printer) } else { statusHandler = status.NewDiscardHandler() } @@ -48,11 +49,11 @@ func NewPushHandler(out io.Writer, format option.Format, tty *os.File, verbose b var metadataHandler metadata.PushHandler switch format.Type { case option.FormatTypeText.Name: - metadataHandler = text.NewPushHandler(out) + metadataHandler = text.NewPushHandler(printer) case option.FormatTypeJSON.Name: - metadataHandler = json.NewPushHandler(out) + metadataHandler = json.NewPushHandler(printer.GetOut()) case option.FormatTypeGoTemplate.Name: - metadataHandler = template.NewPushHandler(out, format.Template) + metadataHandler = template.NewPushHandler(printer.GetOut(), format.Template) default: return nil, nil, errors.UnsupportedFormatTypeError(format.Type) } @@ -60,12 +61,12 @@ func NewPushHandler(out io.Writer, format option.Format, tty *os.File, verbose b } // NewAttachHandler returns status and metadata handlers for attach command. -func NewAttachHandler(out io.Writer, format option.Format, tty *os.File, verbose bool) (status.AttachHandler, metadata.AttachHandler, error) { +func NewAttachHandler(printer *output.Printer, format option.Format, tty *os.File) (status.AttachHandler, metadata.AttachHandler, error) { var statusHandler status.AttachHandler if tty != nil { statusHandler = status.NewTTYAttachHandler(tty) } else if format.Type == option.FormatTypeText.Name { - statusHandler = status.NewTextAttachHandler(out, verbose) + statusHandler = status.NewTextAttachHandler(printer) } else { statusHandler = status.NewDiscardHandler() } @@ -73,11 +74,11 @@ func NewAttachHandler(out io.Writer, format option.Format, tty *os.File, verbose var metadataHandler metadata.AttachHandler switch format.Type { case option.FormatTypeText.Name: - metadataHandler = text.NewAttachHandler(out) + metadataHandler = text.NewAttachHandler(printer) case option.FormatTypeJSON.Name: - metadataHandler = json.NewAttachHandler(out) + metadataHandler = json.NewAttachHandler(printer.GetOut()) case option.FormatTypeGoTemplate.Name: - metadataHandler = template.NewAttachHandler(out, format.Template) + metadataHandler = template.NewAttachHandler(printer.GetOut(), format.Template) default: return nil, nil, errors.UnsupportedFormatTypeError(format.Type) } @@ -85,12 +86,12 @@ func NewAttachHandler(out io.Writer, format option.Format, tty *os.File, verbose } // NewPullHandler returns status and metadata handlers for pull command. -func NewPullHandler(out io.Writer, format option.Format, path string, tty *os.File, verbose bool) (status.PullHandler, metadata.PullHandler, error) { +func NewPullHandler(printer *output.Printer, format option.Format, path string, tty *os.File) (status.PullHandler, metadata.PullHandler, error) { var statusHandler status.PullHandler if tty != nil { statusHandler = status.NewTTYPullHandler(tty) } else if format.Type == option.FormatTypeText.Name { - statusHandler = status.NewTextPullHandler(out, verbose) + statusHandler = status.NewTextPullHandler(printer) } else { statusHandler = status.NewDiscardHandler() } @@ -98,11 +99,11 @@ func NewPullHandler(out io.Writer, format option.Format, path string, tty *os.Fi var metadataHandler metadata.PullHandler switch format.Type { case option.FormatTypeText.Name: - metadataHandler = text.NewPullHandler(out) + metadataHandler = text.NewPullHandler(printer) case option.FormatTypeJSON.Name: - metadataHandler = json.NewPullHandler(out, path) + metadataHandler = json.NewPullHandler(printer.GetOut(), path) case option.FormatTypeGoTemplate.Name: - metadataHandler = template.NewPullHandler(out, path, format.Template) + metadataHandler = template.NewPullHandler(printer.GetOut(), path, format.Template) default: return nil, nil, errors.UnsupportedFormatTypeError(format.Type) } diff --git a/cmd/oras/internal/display/handler_test.go b/cmd/oras/internal/display/handler_test.go index 0373aec4c..dc84de56f 100644 --- a/cmd/oras/internal/display/handler_test.go +++ b/cmd/oras/internal/display/handler_test.go @@ -20,24 +20,28 @@ import ( "testing" "oras.land/oras/cmd/oras/internal/option" + "oras.land/oras/cmd/oras/internal/output" ) func TestNewPushHandler(t *testing.T) { - _, _, err := NewPushHandler(os.Stdout, option.Format{Type: option.FormatTypeText.Name}, os.Stdout, false) + printer := output.NewPrinter(os.Stdout, false) + _, _, err := NewPushHandler(printer, option.Format{Type: option.FormatTypeText.Name}, os.Stdout) if err != nil { t.Errorf("NewPushHandler() error = %v, want nil", err) } } func TestNewAttachHandler(t *testing.T) { - _, _, err := NewAttachHandler(os.Stdout, option.Format{Type: option.FormatTypeText.Name}, os.Stdout, false) + printer := output.NewPrinter(os.Stdout, false) + _, _, err := NewAttachHandler(printer, option.Format{Type: option.FormatTypeText.Name}, os.Stdout) if err != nil { t.Errorf("NewAttachHandler() error = %v, want nil", err) } } func TestNewPullHandler(t *testing.T) { - _, _, err := NewPullHandler(os.Stdout, option.Format{Type: option.FormatTypeText.Name}, "", os.Stdout, false) + printer := output.NewPrinter(os.Stdout, false) + _, _, err := NewPullHandler(printer, option.Format{Type: option.FormatTypeText.Name}, "", os.Stdout) if err != nil { t.Errorf("NewPullHandler() error = %v, want nil", err) } diff --git a/cmd/oras/internal/display/metadata/text/attach.go b/cmd/oras/internal/display/metadata/text/attach.go index d73d32bbb..2032704cf 100644 --- a/cmd/oras/internal/display/metadata/text/attach.go +++ b/cmd/oras/internal/display/metadata/text/attach.go @@ -17,36 +17,36 @@ package text import ( "fmt" - "io" "strings" ocispec "github.com/opencontainers/image-spec/specs-go/v1" "oras.land/oras/cmd/oras/internal/display/metadata" "oras.land/oras/cmd/oras/internal/option" + "oras.land/oras/cmd/oras/internal/output" ) // AttachHandler handles text metadata output for attach events. type AttachHandler struct { - out io.Writer + printer *output.Printer } // NewAttachHandler returns a new handler for attach events. -func NewAttachHandler(out io.Writer) metadata.AttachHandler { +func NewAttachHandler(printer *output.Printer) metadata.AttachHandler { return &AttachHandler{ - out: out, + printer: printer, } } -// OnCompleted is called when the attach command is completed. +// OnCompleted is called when the attach command is complete. func (ah *AttachHandler) OnCompleted(opts *option.Target, root, subject ocispec.Descriptor) error { digest := subject.Digest.String() if !strings.HasSuffix(opts.RawReference, digest) { opts.RawReference = fmt.Sprintf("%s@%s", opts.Path, subject.Digest) } - _, err := fmt.Fprintln(ah.out, "Attached to", opts.AnnotatedReference()) + err := ah.printer.Println("Attached to", opts.AnnotatedReference()) if err != nil { return err } - _, err = fmt.Fprintln(ah.out, "Digest:", root.Digest) + err = ah.printer.Println("Digest:", root.Digest) return err } diff --git a/cmd/oras/internal/display/metadata/text/pull.go b/cmd/oras/internal/display/metadata/text/pull.go index 2413b5957..1b94de7bd 100644 --- a/cmd/oras/internal/display/metadata/text/pull.go +++ b/cmd/oras/internal/display/metadata/text/pull.go @@ -16,34 +16,33 @@ limitations under the License. package text import ( - "fmt" - "io" "sync/atomic" ocispec "github.com/opencontainers/image-spec/specs-go/v1" "oras.land/oras/cmd/oras/internal/display/metadata" "oras.land/oras/cmd/oras/internal/option" + "oras.land/oras/cmd/oras/internal/output" ) // PullHandler handles text metadata output for pull events. type PullHandler struct { - out io.Writer + printer *output.Printer layerSkipped atomic.Bool } // OnCompleted implements metadata.PullHandler. -func (p *PullHandler) OnCompleted(opts *option.Target, desc ocispec.Descriptor) error { - if p.layerSkipped.Load() { - _, _ = fmt.Fprintf(p.out, "Skipped pulling layers without file name in %q\n", ocispec.AnnotationTitle) - _, _ = fmt.Fprintf(p.out, "Use 'oras copy %s --to-oci-layout ' to pull all layers.\n", opts.RawReference) +func (ph *PullHandler) OnCompleted(opts *option.Target, desc ocispec.Descriptor) error { + if ph.layerSkipped.Load() { + _ = ph.printer.Printf("Skipped pulling layers without file name in %q\n", ocispec.AnnotationTitle) + _ = ph.printer.Printf("Use 'oras copy %s --to-oci-layout ' to pull all layers.\n", opts.RawReference) } else { - _, _ = fmt.Fprintln(p.out, "Pulled", opts.AnnotatedReference()) - _, _ = fmt.Fprintln(p.out, "Digest:", desc.Digest) + _ = ph.printer.Println("Pulled", opts.AnnotatedReference()) + _ = ph.printer.Println("Digest:", desc.Digest) } return nil } -func (p *PullHandler) OnFilePulled(name string, outputDir string, desc ocispec.Descriptor, descPath string) error { +func (ph *PullHandler) OnFilePulled(_ string, _ string, _ ocispec.Descriptor, _ string) error { return nil } @@ -54,8 +53,8 @@ func (ph *PullHandler) OnLayerSkipped(ocispec.Descriptor) error { } // NewPullHandler returns a new handler for Pull events. -func NewPullHandler(out io.Writer) metadata.PullHandler { +func NewPullHandler(printer *output.Printer) metadata.PullHandler { return &PullHandler{ - out: out, + printer: printer, } } diff --git a/cmd/oras/internal/display/metadata/text/push.go b/cmd/oras/internal/display/metadata/text/push.go index d8659b9f0..b90852ff5 100644 --- a/cmd/oras/internal/display/metadata/text/push.go +++ b/cmd/oras/internal/display/metadata/text/push.go @@ -16,25 +16,24 @@ limitations under the License. package text import ( - "fmt" - "io" "sync" ocispec "github.com/opencontainers/image-spec/specs-go/v1" "oras.land/oras/cmd/oras/internal/display/metadata" "oras.land/oras/cmd/oras/internal/option" + "oras.land/oras/cmd/oras/internal/output" ) // PushHandler handles text metadata output for push events. type PushHandler struct { - out io.Writer + printer *output.Printer tagLock sync.Mutex } // NewPushHandler returns a new handler for push events. -func NewPushHandler(out io.Writer) metadata.PushHandler { +func NewPushHandler(printer *output.Printer) metadata.PushHandler { return &PushHandler{ - out: out, + printer: printer, } } @@ -42,22 +41,19 @@ func NewPushHandler(out io.Writer) metadata.PushHandler { func (h *PushHandler) OnTagged(_ ocispec.Descriptor, tag string) error { h.tagLock.Lock() defer h.tagLock.Unlock() - _, err := fmt.Fprintln(h.out, "Tagged", tag) - return err + return h.printer.Println("Tagged", tag) } // OnCopied is called after files are copied. func (h *PushHandler) OnCopied(opts *option.Target) error { - _, err := fmt.Fprintln(h.out, "Pushed", opts.AnnotatedReference()) - return err + return h.printer.Println("Pushed", opts.AnnotatedReference()) } // OnCompleted is called after the push is completed. func (h *PushHandler) OnCompleted(root ocispec.Descriptor) error { - _, err := fmt.Fprintln(h.out, "ArtifactType:", root.ArtifactType) + err := h.printer.Println("ArtifactType:", root.ArtifactType) if err != nil { return err } - _, err = fmt.Fprintln(h.out, "Digest:", root.Digest) - return err + return h.printer.Println("Digest:", root.Digest) } diff --git a/cmd/oras/internal/display/metadata/text/push_test.go b/cmd/oras/internal/display/metadata/text/push_test.go index 706f4dc75..95372c26f 100644 --- a/cmd/oras/internal/display/metadata/text/push_test.go +++ b/cmd/oras/internal/display/metadata/text/push_test.go @@ -23,6 +23,7 @@ import ( "github.com/opencontainers/go-digest" ocispec "github.com/opencontainers/image-spec/specs-go/v1" + "oras.land/oras/cmd/oras/internal/output" ) type errorWriter struct{} @@ -58,13 +59,14 @@ func TestPushHandler_OnCompleted(t *testing.T) { Digest: digest.FromBytes(content), Size: int64(len(content)), }, - true, + false, // Printer ignores error }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + printer := output.NewPrinter(tt.out, false) p := &PushHandler{ - out: tt.out, + printer: printer, } if err := p.OnCompleted(tt.root); (err != nil) != tt.wantErr { t.Errorf("PushHandler.OnCompleted() error = %v, wantErr %v", err, tt.wantErr) diff --git a/cmd/oras/internal/display/status/text.go b/cmd/oras/internal/display/status/text.go index 6198c6436..403155e9e 100644 --- a/cmd/oras/internal/display/status/text.go +++ b/cmd/oras/internal/display/status/text.go @@ -17,7 +17,6 @@ package status import ( "context" - "io" "sync" "oras.land/oras/cmd/oras/internal/output" @@ -33,9 +32,9 @@ type TextPushHandler struct { } // NewTextPushHandler returns a new handler for push command. -func NewTextPushHandler(out io.Writer, verbose bool) PushHandler { +func NewTextPushHandler(printer *output.Printer) PushHandler { return &TextPushHandler{ - printer: output.NewPrinter(out, verbose), + printer: printer, } } @@ -74,8 +73,8 @@ func (ph *TextPushHandler) UpdateCopyOptions(opts *oras.CopyGraphOptions, fetche } // NewTextAttachHandler returns a new handler for attach command. -func NewTextAttachHandler(out io.Writer, verbose bool) AttachHandler { - return NewTextPushHandler(out, verbose) +func NewTextAttachHandler(printer *output.Printer) AttachHandler { + return NewTextPushHandler(printer) } // TextPullHandler handles text status output for pull events. @@ -114,8 +113,8 @@ func (ph *TextPullHandler) OnNodeSkipped(desc ocispec.Descriptor) error { } // NewTextPullHandler returns a new handler for pull command. -func NewTextPullHandler(out io.Writer, verbose bool) PullHandler { +func NewTextPullHandler(printer *output.Printer) PullHandler { return &TextPullHandler{ - printer: output.NewPrinter(out, verbose), + printer: printer, } } diff --git a/cmd/oras/internal/output/print.go b/cmd/oras/internal/output/print.go index efce7c940..9a8295710 100644 --- a/cmd/oras/internal/output/print.go +++ b/cmd/oras/internal/output/print.go @@ -42,6 +42,11 @@ func NewPrinter(out io.Writer, verbose bool) *Printer { return &Printer{out: out, verbose: verbose} } +// Println prints objects concurrent-safely with newline. +func (p *Printer) GetOut() io.Writer { + return p.out +} + // Println prints objects concurrent-safely with newline. func (p *Printer) Println(a ...any) error { p.lock.Lock() @@ -55,6 +60,19 @@ func (p *Printer) Println(a ...any) error { return nil } +// Printf prints objects concurrent-safely with newline. +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(os.Stderr, err) + } + // Errors are handled above, so return nil + return nil +} + // PrintVerbose prints when verbose is true. func (p *Printer) PrintVerbose(a ...any) error { if !p.verbose { diff --git a/cmd/oras/internal/output/print_test.go b/cmd/oras/internal/output/print_test.go index 74d1f8eb3..2394957a9 100644 --- a/cmd/oras/internal/output/print_test.go +++ b/cmd/oras/internal/output/print_test.go @@ -56,11 +56,15 @@ func TestPrinter_PrintVerbose_noError(t *testing.T) { builder := &strings.Builder{} printer := NewPrinter(builder, false) - expected := "normal\n" + expected := "normal\nthing one\n" err := printer.Println("normal") if err != nil { t.Error("Expected no error got <" + err.Error() + ">") } + err = printer.Printf("thing %s\n", "one") + if err != nil { + t.Error("Expected no error got <" + err.Error() + ">") + } err = printer.PrintVerbose("verbose") if err != nil { t.Error("Expected no error got <" + err.Error() + ">") diff --git a/cmd/oras/root/attach.go b/cmd/oras/root/attach.go index c7d447927..d9a75cdc0 100644 --- a/cmd/oras/root/attach.go +++ b/cmd/oras/root/attach.go @@ -31,6 +31,7 @@ import ( "oras.land/oras/cmd/oras/internal/display" oerrors "oras.land/oras/cmd/oras/internal/errors" "oras.land/oras/cmd/oras/internal/option" + "oras.land/oras/cmd/oras/internal/output" "oras.land/oras/internal/graph" "oras.land/oras/internal/registryutil" ) @@ -109,7 +110,8 @@ Example - Attach file to the manifest tagged 'v1' in an OCI image layout folder func runAttach(cmd *cobra.Command, opts *attachOptions) error { ctx, logger := command.GetLogger(cmd, &opts.Common) - displayStatus, displayMetadata, err := display.NewAttachHandler(cmd.OutOrStdout(), opts.Format, opts.TTY, opts.Verbose) + printer := output.NewPrinter(cmd.OutOrStdout(), opts.Verbose) + displayStatus, displayMetadata, err := display.NewAttachHandler(printer, opts.Format, opts.TTY) if err != nil { return err } diff --git a/cmd/oras/root/pull.go b/cmd/oras/root/pull.go index 1cf4cda60..816d48ed9 100644 --- a/cmd/oras/root/pull.go +++ b/cmd/oras/root/pull.go @@ -35,6 +35,7 @@ import ( oerrors "oras.land/oras/cmd/oras/internal/errors" "oras.land/oras/cmd/oras/internal/fileref" "oras.land/oras/cmd/oras/internal/option" + "oras.land/oras/cmd/oras/internal/output" "oras.land/oras/internal/descriptor" "oras.land/oras/internal/graph" ) @@ -112,7 +113,8 @@ Example - Pull artifact files from an OCI layout archive 'layout.tar': func runPull(cmd *cobra.Command, opts *pullOptions) error { ctx, logger := command.GetLogger(cmd, &opts.Common) - statusHandler, metadataHandler, err := display.NewPullHandler(cmd.OutOrStdout(), opts.Format, opts.Path, opts.TTY, opts.Verbose) + printer := output.NewPrinter(cmd.OutOrStdout(), opts.Verbose) + statusHandler, metadataHandler, err := display.NewPullHandler(printer, opts.Format, opts.Path, opts.TTY) if err != nil { return err } diff --git a/cmd/oras/root/push.go b/cmd/oras/root/push.go index 689c36491..5d7df1da7 100644 --- a/cmd/oras/root/push.go +++ b/cmd/oras/root/push.go @@ -33,6 +33,7 @@ import ( oerrors "oras.land/oras/cmd/oras/internal/errors" "oras.land/oras/cmd/oras/internal/fileref" "oras.land/oras/cmd/oras/internal/option" + "oras.land/oras/cmd/oras/internal/output" "oras.land/oras/internal/contentutil" "oras.land/oras/internal/listener" "oras.land/oras/internal/registryutil" @@ -151,7 +152,8 @@ Example - Push file "hi.txt" into an OCI image layout folder 'layout-dir' with t func runPush(cmd *cobra.Command, opts *pushOptions) error { ctx, logger := command.GetLogger(cmd, &opts.Common) - displayStatus, displayMetadata, err := display.NewPushHandler(cmd.OutOrStdout(), opts.Format, opts.TTY, opts.Verbose) + printer := output.NewPrinter(cmd.OutOrStdout(), opts.Verbose) + displayStatus, displayMetadata, err := display.NewPushHandler(printer, opts.Format, opts.TTY) if err != nil { return err } diff --git a/cmd/oras/root/push_test.go b/cmd/oras/root/push_test.go index 296711979..e06de14a9 100644 --- a/cmd/oras/root/push_test.go +++ b/cmd/oras/root/push_test.go @@ -25,7 +25,7 @@ import ( ) func Test_runPush_errType(t *testing.T) { - // prpare + // prepare cmd := &cobra.Command{} cmd.SetContext(context.Background())