refactor: Pass printer to text handler (#1420)

Signed-off-by: Terry Howe <[email protected]> Signed-off-by: Billy Zha <[email protected]> Co-authored-by: Billy Zha <[email protected]>
oras-project · Jun 21, 2024 · 145ea7a · 145ea7a
1 parent 37941a0
commit 145ea7a
Show file tree

Hide file tree

Showing 13 changed files with 104 additions and 65 deletions.
diff --git a/cmd/oras/internal/display/handler.go b/cmd/oras/internal/display/handler.go
@@ -32,77 +32,78 @@ 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()
 	}
 
 	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)
 	case option.FormatTypeGoTemplate.Name:
-		metadataHandler = template.NewPushHandler(out, format.Template)
+		metadataHandler = template.NewPushHandler(printer, format.Template)
 	default:
 		return nil, nil, errors.UnsupportedFormatTypeError(format.Type)
 	}
 	return statusHandler, metadataHandler, nil
 }
 
 // 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()
 	}
 
 	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)
 	case option.FormatTypeGoTemplate.Name:
-		metadataHandler = template.NewAttachHandler(out, format.Template)
+		metadataHandler = template.NewAttachHandler(printer, format.Template)
 	default:
 		return nil, nil, errors.UnsupportedFormatTypeError(format.Type)
 	}
 	return statusHandler, metadataHandler, nil
 }
 
 // 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()
 	}
 
 	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, path)
 	case option.FormatTypeGoTemplate.Name:
-		metadataHandler = template.NewPullHandler(out, path, format.Template)
+		metadataHandler = template.NewPullHandler(printer, 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
@@ -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
@@ -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
@@ -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 <layout-dir>' 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 <layout-dir>' 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
@@ -16,48 +16,44 @@ 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,
 	}
 }
 
 // OnTagged implements metadata.TextTagHandler.
 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
@@ -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
@@ -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
@@ -19,10 +19,11 @@ import (
 	"context"
 	"fmt"
 	"io"
-	"oras.land/oras/internal/descriptor"
 	"os"
 	"sync"
 
+	"oras.land/oras/internal/descriptor"
+
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 	"oras.land/oras-go/v2/content"
 )
@@ -42,6 +43,13 @@ func NewPrinter(out io.Writer, verbose bool) *Printer {
 	return &Printer{out: out, verbose: verbose}
 }
 
+// Write implements the io.Writer interface.
+func (p *Printer) Write(b []byte) (int, error) {
+	p.lock.Lock()
+	defer p.lock.Unlock()
+	return p.out.Write(b)
+}
+
 // Println prints objects concurrent-safely with newline.
 func (p *Printer) Println(a ...any) error {
 	p.lock.Lock()
@@ -55,6 +63,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 {