From f2066cc7b4cfc226a919f93b0397c6435487b29e Mon Sep 17 00:00:00 2001 From: Terry Howe Date: Mon, 30 Sep 2024 07:49:39 -0600 Subject: [PATCH] refactor: simplify reader with manager and messenger Signed-off-by: Terry Howe --- .../display/status/console/console.go | 3 + .../display/status/console/discard.go | 100 ++++++++++++++++++ .../display/status/console/discard_test.go | 73 +++++++++++++ .../display/status/progress/manager.go | 12 ++- .../display/status/progress/manager_test.go | 29 +++++ .../display/status/progress/messenger.go | 28 ++++- .../display/status/progress/messenger_test.go | 42 ++++++-- .../internal/display/status/track/reader.go | 57 ++++------ .../internal/display/status/track/target.go | 28 +++-- cmd/oras/root/blob/fetch.go | 17 ++- cmd/oras/root/blob/push.go | 15 ++- 11 files changed, 335 insertions(+), 69 deletions(-) create mode 100644 cmd/oras/internal/display/status/console/discard.go create mode 100644 cmd/oras/internal/display/status/console/discard_test.go diff --git a/cmd/oras/internal/display/status/console/console.go b/cmd/oras/internal/display/status/console/console.go index fd9c06ec3..db924d1a2 100644 --- a/cmd/oras/internal/display/status/console/console.go +++ b/cmd/oras/internal/display/status/console/console.go @@ -49,6 +49,9 @@ type console struct { // NewConsole generates a console from a file. func NewConsole(f *os.File) (Console, error) { + if f != nil && f.Name() == os.DevNull { + return NewDiscardConsole(f), nil + } c, err := containerd.ConsoleFromFile(f) if err != nil { return nil, err diff --git a/cmd/oras/internal/display/status/console/discard.go b/cmd/oras/internal/display/status/console/discard.go new file mode 100644 index 000000000..2f79018e5 --- /dev/null +++ b/cmd/oras/internal/display/status/console/discard.go @@ -0,0 +1,100 @@ +/* +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 console + +import ( + "os" + + containerd "github.com/containerd/console" +) + +type discardConsole struct { + *os.File +} + +// NewDiscardConsole create a console that does not output. +func NewDiscardConsole(f *os.File) Console { + dc := discardConsole{ + File: f, + } + return &dc +} + +// Fd returns its file descriptor +func (mc *discardConsole) Fd() uintptr { + return os.Stderr.Fd() +} + +// Name returns its file name +func (mc *discardConsole) Name() string { + return mc.File.Name() +} + +// Resize ignored +func (mc *discardConsole) Resize(_ containerd.WinSize) error { + return nil +} + +// ResizeFrom ignored +func (mc *discardConsole) ResizeFrom(containerd.Console) error { + return nil +} + +// SetRaw ignored +func (mc *discardConsole) SetRaw() error { + return nil +} + +// DisableEcho ignored +func (mc *discardConsole) DisableEcho() error { + return nil +} + +// Reset ignored +func (mc *discardConsole) Reset() error { + return nil +} + +// Size return default size +func (mc *discardConsole) Size() (containerd.WinSize, error) { + ws := containerd.WinSize{ + Width: 80, + Height: 24, + } + return ws, nil +} + +// GetHeightWidth returns the width and height of the console. +func (mc *discardConsole) GetHeightWidth() (height, width int) { + windowSize, _ := mc.Size() + return int(windowSize.Height), int(windowSize.Width) +} + +// Save ignored +func (mc *discardConsole) Save() { +} + +// NewRow ignored +func (mc *discardConsole) NewRow() { +} + +// OutputTo ignored +func (mc *discardConsole) OutputTo(_ uint, _ string) { +} + +// Restore ignored +func (mc *discardConsole) Restore() { +} diff --git a/cmd/oras/internal/display/status/console/discard_test.go b/cmd/oras/internal/display/status/console/discard_test.go new file mode 100644 index 000000000..e0e36d47b --- /dev/null +++ b/cmd/oras/internal/display/status/console/discard_test.go @@ -0,0 +1,73 @@ +/* +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 console + +import ( + "os" + "testing" + + containerd "github.com/containerd/console" +) + +func TestConsole_New(t *testing.T) { + mockFile, err := os.OpenFile(os.DevNull, os.O_RDWR, 0666) + if err != nil { + t.Fatalf("Unexpected error %v", err) + } + + sut, err := NewConsole(mockFile) + if err != nil { + t.Errorf("Unexpected error %v", err) + } + + if err = sut.Resize(containerd.WinSize{}); err != nil { + t.Errorf("Unexpected erro for Resize: %v", err) + } + if err = sut.ResizeFrom(nil); err != nil { + t.Errorf("Unexpected erro for Resize: %v", err) + } + if err = sut.SetRaw(); err != nil { + t.Errorf("Unexpected erro for Resize: %v", err) + } + if err = sut.DisableEcho(); err != nil { + t.Errorf("Unexpected erro for Resize: %v", err) + } + if err = sut.Reset(); err != nil { + t.Errorf("Unexpected erro for Resize: %v", err) + } + windowSize, _ := sut.Size() + if windowSize.Height != 24 { + t.Errorf("Expected size 24 actual %d", windowSize.Height) + } + if windowSize.Width != 80 { + t.Errorf("Expected size 80 actual %d", windowSize.Width) + } + h, w := sut.GetHeightWidth() + if h != 24 { + t.Errorf("Expected size 24 actual %d", h) + } + if w != 80 { + t.Errorf("Expected size 80 actual %d", w) + } + if sut.Fd() != os.Stderr.Fd() { + t.Errorf("Expected size %d actual %d", sut.Fd(), os.Stderr.Fd()) + } + if sut.Name() != os.DevNull { + t.Errorf("Expected size %s actual %s", sut.Name(), os.DevNull) + } + sut.OutputTo(0, "ignored") + sut.Restore() +} diff --git a/cmd/oras/internal/display/status/progress/manager.go b/cmd/oras/internal/display/status/progress/manager.go index 28e61d5e0..a3c182650 100644 --- a/cmd/oras/internal/display/status/progress/manager.go +++ b/cmd/oras/internal/display/status/progress/manager.go @@ -45,19 +45,23 @@ type manager struct { status []*status statusLock sync.RWMutex console console.Console + actionPrompt string + donePrompt string updating sync.WaitGroup renderDone chan struct{} renderClosed chan struct{} } // NewManager initialized a new progress manager. -func NewManager(tty *os.File) (Manager, error) { +func NewManager(actionPrompt string, donePrompt string, tty *os.File) (Manager, error) { c, err := console.NewConsole(tty) if err != nil { return nil, err } m := &manager{ console: c, + actionPrompt: actionPrompt, + donePrompt: donePrompt, renderDone: make(chan struct{}), renderClosed: make(chan struct{}), } @@ -131,15 +135,15 @@ func (m *manager) SendAndStop(desc ocispec.Descriptor, prompt string) error { } func (m *manager) statusChan(s *status) *Messenger { - ch := make(chan *status, BufferSize) + messenger := NewMessenger(m.actionPrompt, m.donePrompt) m.updating.Add(1) go func() { defer m.updating.Done() - for newStatus := range ch { + for newStatus := range messenger.ch { s.update(newStatus) } }() - return &Messenger{ch: ch} + return messenger } // Close stops all status and waits for updating and rendering. diff --git a/cmd/oras/internal/display/status/progress/manager_test.go b/cmd/oras/internal/display/status/progress/manager_test.go index 43d0f2104..9a76aa2d4 100644 --- a/cmd/oras/internal/display/status/progress/manager_test.go +++ b/cmd/oras/internal/display/status/progress/manager_test.go @@ -19,6 +19,7 @@ package progress import ( "fmt" + "os" "testing" "oras.land/oras/cmd/oras/internal/display/status/console" @@ -55,3 +56,31 @@ func Test_manager_render(t *testing.T) { t.Fatal(err) } } + +func TestNewManager(t *testing.T) { + mockFile, err := os.OpenFile(os.DevNull, os.O_RDWR, 0666) + if err != nil { + t.Fatalf("Unexpected error %v", err) + } + + sut, err := NewManager("Action", "Done", mockFile) + if err != nil { + t.Errorf("Unexpected error %v", err) + } + + messenger, err := sut.Add() + if err != nil { + t.Errorf("Unexpected error %v", err) + } + if messenger.actionPrompt != "Action" { + t.Errorf("Expected prompt Action actual %v", messenger.actionPrompt) + } + if messenger.donePrompt != "Done" { + t.Errorf("Expected prompt Done actual %v", messenger.donePrompt) + } + + _, err = NewManager("Action", "Done", os.Stderr) + if err == nil { + t.Errorf("Expected error when using Stderr as console") + } +} diff --git a/cmd/oras/internal/display/status/progress/messenger.go b/cmd/oras/internal/display/status/progress/messenger.go index 9f0188b5a..83630e0b9 100644 --- a/cmd/oras/internal/display/status/progress/messenger.go +++ b/cmd/oras/internal/display/status/progress/messenger.go @@ -23,8 +23,20 @@ import ( // Messenger is progress message channel. type Messenger struct { - ch chan *status - closed bool + ch chan *status + actionPrompt string + donePrompt string + closed bool +} + +// NewMessenger create a new messenger object +func NewMessenger(actionPrompt, donePrompt string) *Messenger { + ch := make(chan *status, BufferSize) + return &Messenger{ + ch: ch, + actionPrompt: actionPrompt, + donePrompt: donePrompt, + } } // Start initializes the messenger. @@ -50,7 +62,17 @@ func (sm *Messenger) Send(prompt string, descriptor ocispec.Descriptor, offset i } } -// Stop the messenger after sending a end message. +// SendAction send the action status message. +func (sm *Messenger) SendAction(descriptor ocispec.Descriptor, offset int64) { + sm.Send(sm.actionPrompt, descriptor, offset) +} + +// SendDone send the done status message. +func (sm *Messenger) SendDone(descriptor ocispec.Descriptor, offset int64) { + sm.Send(sm.donePrompt, descriptor, offset) +} + +// Stop the messenger after sending end message. func (sm *Messenger) Stop() { if sm.closed { return diff --git a/cmd/oras/internal/display/status/progress/messenger_test.go b/cmd/oras/internal/display/status/progress/messenger_test.go index a8b782e55..235b909d2 100644 --- a/cmd/oras/internal/display/status/progress/messenger_test.go +++ b/cmd/oras/internal/display/status/progress/messenger_test.go @@ -22,12 +22,11 @@ import ( func Test_Messenger(t *testing.T) { var msg *status - ch := make(chan *status, BufferSize) - messenger := &Messenger{ch: ch} + messenger := NewMessenger("Action", "Done") messenger.Start() select { - case msg = <-ch: + case msg = <-messenger.ch: if msg.offset != -1 { t.Errorf("Expected start message with offset -1, got %d", msg.offset) } @@ -42,7 +41,7 @@ func Test_Messenger(t *testing.T) { expected := int64(50) messenger.Send("Reading", desc, expected) select { - case msg = <-ch: + case msg = <-messenger.ch: if msg.offset != expected { t.Errorf("Expected status message with offset %d, got %d", expected, msg.offset) } @@ -56,7 +55,7 @@ func Test_Messenger(t *testing.T) { messenger.Send("Reading", desc, expected) messenger.Send("Read", desc, desc.Size) select { - case msg = <-ch: + case msg = <-messenger.ch: if msg.offset != desc.Size { t.Errorf("Expected status message with offset %d, got %d", expected, msg.offset) } @@ -67,15 +66,42 @@ func Test_Messenger(t *testing.T) { t.Error("Expected status message") } select { - case msg = <-ch: + case msg = <-messenger.ch: t.Errorf("Unexpected status message %v", msg) default: } + messenger.SendAction(desc, expected) + select { + case msg = <-messenger.ch: + if msg.offset != expected { + t.Errorf("Expected status message with offset %d, got %d", expected, msg.offset) + } + if msg.prompt != "Action" { + t.Errorf("Expected status message prompt Action, got %s", msg.prompt) + } + default: + t.Error("Expected status message") + } + + expected += 1 + messenger.SendDone(desc, expected) + select { + case msg = <-messenger.ch: + if msg.offset != expected { + t.Errorf("Expected status message with offset %d, got %d", expected, msg.offset) + } + if msg.prompt != "Done" { + t.Errorf("Expected status message prompt Done, got %s", msg.prompt) + } + default: + t.Error("Expected status message") + } + expected = int64(-1) messenger.Stop() select { - case msg = <-ch: + case msg = <-messenger.ch: if msg.offset != expected { t.Errorf("Expected END status message with offset %d, got %d", expected, msg.offset) } @@ -85,7 +111,7 @@ func Test_Messenger(t *testing.T) { messenger.Stop() select { - case msg = <-ch: + case msg = <-messenger.ch: if msg != nil { t.Errorf("Unexpected status message %v", msg) } diff --git a/cmd/oras/internal/display/status/track/reader.go b/cmd/oras/internal/display/status/track/reader.go index 93919381f..7c9eec2c7 100644 --- a/cmd/oras/internal/display/status/track/reader.go +++ b/cmd/oras/internal/display/status/track/reader.go @@ -17,56 +17,39 @@ package track import ( "io" - "os" ocispec "github.com/opencontainers/image-spec/specs-go/v1" "oras.land/oras/cmd/oras/internal/display/status/progress" ) -type reader struct { - base io.Reader - offset int64 - actionPrompt string - donePrompt string - descriptor ocispec.Descriptor - manager progress.Manager - messenger *progress.Messenger +// Reader for progress tracked resource +type Reader interface { + io.Reader + Done() + Close() + Start() } -// NewReader returns a new reader with tracked progress. -func NewReader(r io.Reader, descriptor ocispec.Descriptor, actionPrompt string, donePrompt string, tty *os.File) (*reader, error) { - manager, err := progress.NewManager(tty) - if err != nil { - return nil, err - } - return managedReader(r, descriptor, manager, actionPrompt, donePrompt) +type reader struct { + base io.Reader + offset int64 + descriptor ocispec.Descriptor + messenger *progress.Messenger } -func managedReader(r io.Reader, descriptor ocispec.Descriptor, manager progress.Manager, actionPrompt string, donePrompt string) (*reader, error) { - messenger, err := manager.Add() - if err != nil { - return nil, err +// NewReader returns a new reader with tracked progress. +func NewReader(r io.Reader, descriptor ocispec.Descriptor, messenger *progress.Messenger) Reader { + tr := reader{ + base: r, + descriptor: descriptor, + messenger: messenger, } - - return &reader{ - base: r, - descriptor: descriptor, - actionPrompt: actionPrompt, - donePrompt: donePrompt, - manager: manager, - messenger: messenger, - }, nil -} - -// StopManager stops the messenger channel and related manager. -func (r *reader) StopManager() { - r.Close() - _ = r.manager.Close() + return &tr } // Done sends message to mark the tracked progress as complete. func (r *reader) Done() { - r.messenger.Send(r.donePrompt, r.descriptor, r.descriptor.Size) + r.messenger.SendDone(r.descriptor, r.descriptor.Size) r.messenger.Stop() } @@ -93,6 +76,6 @@ func (r *reader) Read(p []byte) (int, error) { return n, io.ErrUnexpectedEOF } } - r.messenger.Send(r.actionPrompt, r.descriptor, r.offset) + r.messenger.SendAction(r.descriptor, r.offset) return n, err } diff --git a/cmd/oras/internal/display/status/track/target.go b/cmd/oras/internal/display/status/track/target.go index 5c704ebbc..e13c9a990 100644 --- a/cmd/oras/internal/display/status/track/target.go +++ b/cmd/oras/internal/display/status/track/target.go @@ -35,9 +35,7 @@ type GraphTarget interface { type graphTarget struct { oras.GraphTarget - manager progress.Manager - actionPrompt string - donePrompt string + manager progress.Manager } type referenceGraphTarget struct { @@ -46,15 +44,13 @@ type referenceGraphTarget struct { // NewTarget creates a new tracked Target. func NewTarget(t oras.GraphTarget, actionPrompt, donePrompt string, tty *os.File) (GraphTarget, error) { - manager, err := progress.NewManager(tty) + manager, err := progress.NewManager(actionPrompt, donePrompt, tty) if err != nil { return nil, err } gt := &graphTarget{ - GraphTarget: t, - manager: manager, - actionPrompt: actionPrompt, - donePrompt: donePrompt, + GraphTarget: t, + manager: manager, } if _, ok := t.(registry.ReferencePusher); ok { @@ -74,7 +70,13 @@ func (t *graphTarget) Mount(ctx context.Context, desc ocispec.Descriptor, fromRe // Push pushes the content to the base oras.GraphTarget with tracking. func (t *graphTarget) Push(ctx context.Context, expected ocispec.Descriptor, content io.Reader) error { - r, err := managedReader(content, expected, t.manager, t.actionPrompt, t.donePrompt) + messenger, err := t.manager.Add() + if err != nil { + return err + } + defer messenger.Stop() + + r := NewReader(content, expected, messenger) if err != nil { return err } @@ -89,7 +91,13 @@ func (t *graphTarget) Push(ctx context.Context, expected ocispec.Descriptor, con // PushReference pushes the content to the base oras.GraphTarget with tracking. func (rgt *referenceGraphTarget) PushReference(ctx context.Context, expected ocispec.Descriptor, content io.Reader, reference string) error { - r, err := managedReader(content, expected, rgt.manager, rgt.actionPrompt, rgt.donePrompt) + messenger, err := rgt.manager.Add() + if err != nil { + return err + } + defer messenger.Stop() + + r := NewReader(content, expected, messenger) if err != nil { return err } diff --git a/cmd/oras/root/blob/fetch.go b/cmd/oras/root/blob/fetch.go index 44694c428..ea6fccd7e 100644 --- a/cmd/oras/root/blob/fetch.go +++ b/cmd/oras/root/blob/fetch.go @@ -28,6 +28,7 @@ import ( "oras.land/oras-go/v2/registry/remote" "oras.land/oras/cmd/oras/internal/argument" "oras.land/oras/cmd/oras/internal/command" + "oras.land/oras/cmd/oras/internal/display/status/progress" "oras.land/oras/cmd/oras/internal/display/status/track" oerrors "oras.land/oras/cmd/oras/internal/errors" "oras.land/oras/cmd/oras/internal/option" @@ -170,12 +171,20 @@ func (opts *fetchBlobOptions) doFetch(ctx context.Context, src oras.ReadOnlyTarg return ocispec.Descriptor{}, err } } else { - // TTY output - trackedReader, err := track.NewReader(vr, desc, "Downloading", "Downloaded ", opts.TTY) + manager, err := progress.NewManager("Downloading", "Downloaded ", opts.TTY) if err != nil { - return ocispec.Descriptor{}, err + return desc, err } - defer trackedReader.StopManager() + defer manager.Close() + + messenger, err := manager.Add() + if err != nil { + return desc, err + } + defer messenger.Stop() + + // TTY output + trackedReader := track.NewReader(vr, desc, messenger) trackedReader.Start() if _, err = io.Copy(writer, trackedReader); err != nil { return ocispec.Descriptor{}, err diff --git a/cmd/oras/root/blob/push.go b/cmd/oras/root/blob/push.go index 6bb126357..df0204870 100644 --- a/cmd/oras/root/blob/push.go +++ b/cmd/oras/root/blob/push.go @@ -26,6 +26,7 @@ import ( "oras.land/oras-go/v2" "oras.land/oras/cmd/oras/internal/argument" "oras.land/oras/cmd/oras/internal/command" + "oras.land/oras/cmd/oras/internal/display/status/progress" "oras.land/oras/cmd/oras/internal/display/status/track" oerrors "oras.land/oras/cmd/oras/internal/errors" "oras.land/oras/cmd/oras/internal/option" @@ -154,12 +155,20 @@ func (opts *pushBlobOptions) doPush(ctx context.Context, printer *output.Printer return printer.PrintStatus(desc, "Uploaded ") } - // TTY output - trackedReader, err := track.NewReader(r, desc, "Uploading", "Uploaded ", opts.TTY) + manager, err := progress.NewManager("Uploading", "Uploaded ", opts.TTY) if err != nil { return err } - defer trackedReader.StopManager() + defer manager.Close() + + messenger, err := manager.Add() + if err != nil { + return err + } + defer messenger.Stop() + + // TTY output + trackedReader := track.NewReader(r, desc, messenger) trackedReader.Start() r = trackedReader if err := t.Push(ctx, desc, r); err != nil {