From 840f82e40692ea67f59c8b35a58565dcb69e42f7 Mon Sep 17 00:00:00 2001 From: Xiaoxuan Wang <103478229+wangxiaoxuan273@users.noreply.github.com> Date: Mon, 16 Dec 2024 10:53:28 +0800 Subject: [PATCH 1/2] refactor: apply metadata render interface to oras push command (#1583) Signed-off-by: Xiaoxuan Wang --- .../internal/display/metadata/interface.go | 14 +++++++------- .../internal/display/metadata/json/push.go | 10 ++++++---- .../internal/display/metadata/template/push.go | 10 ++++++---- .../internal/display/metadata/text/push.go | 12 +++++++----- .../display/metadata/text/push_test.go | 3 ++- cmd/oras/root/push.go | 18 +++++++++--------- 6 files changed, 37 insertions(+), 30 deletions(-) diff --git a/cmd/oras/internal/display/metadata/interface.go b/cmd/oras/internal/display/metadata/interface.go index 145a65d0a..71c26de5f 100644 --- a/cmd/oras/internal/display/metadata/interface.go +++ b/cmd/oras/internal/display/metadata/interface.go @@ -20,17 +20,17 @@ import ( "oras.land/oras/cmd/oras/internal/option" ) +// Renderer renders metadata information when an operation is complete. +type Renderer interface { + Render() error +} + // PushHandler handles metadata output for push events. type PushHandler interface { TaggedHandler + Renderer - OnCopied(opts *option.Target) error - OnCompleted(root ocispec.Descriptor) error -} - -// Renderer renders metadata information when an operation is complete. -type Renderer interface { - Render() error + OnCopied(opts *option.Target, root ocispec.Descriptor) error } // AttachHandler handles metadata output for attach events. diff --git a/cmd/oras/internal/display/metadata/json/push.go b/cmd/oras/internal/display/metadata/json/push.go index 6c3a03ebe..9433829e6 100644 --- a/cmd/oras/internal/display/metadata/json/push.go +++ b/cmd/oras/internal/display/metadata/json/push.go @@ -31,6 +31,7 @@ type PushHandler struct { path string out io.Writer tagged model.Tagged + root ocispec.Descriptor } // NewPushHandler creates a new handler for push events. @@ -47,15 +48,16 @@ func (ph *PushHandler) OnTagged(desc ocispec.Descriptor, tag string) error { } // OnCopied is called after files are copied. -func (ph *PushHandler) OnCopied(opts *option.Target) error { +func (ph *PushHandler) OnCopied(opts *option.Target, root ocispec.Descriptor) error { if opts.RawReference != "" && !contentutil.IsDigest(opts.Reference) { ph.tagged.AddTag(opts.Reference) } ph.path = opts.Path + ph.root = root return nil } -// OnCompleted is called after the push is completed. -func (ph *PushHandler) OnCompleted(root ocispec.Descriptor) error { - return output.PrintPrettyJSON(ph.out, model.NewPush(root, ph.path, ph.tagged.Tags())) +// Render implements PushHandler. +func (ph *PushHandler) Render() error { + return output.PrintPrettyJSON(ph.out, model.NewPush(ph.root, ph.path, ph.tagged.Tags())) } diff --git a/cmd/oras/internal/display/metadata/template/push.go b/cmd/oras/internal/display/metadata/template/push.go index 22de93876..bc40f0afa 100644 --- a/cmd/oras/internal/display/metadata/template/push.go +++ b/cmd/oras/internal/display/metadata/template/push.go @@ -32,6 +32,7 @@ type PushHandler struct { path string tagged model.Tagged out io.Writer + root ocispec.Descriptor } // NewPushHandler returns a new handler for push events. @@ -49,15 +50,16 @@ func (ph *PushHandler) OnTagged(desc ocispec.Descriptor, tag string) error { } // OnCopied is called after files are copied. -func (ph *PushHandler) OnCopied(opts *option.Target) error { +func (ph *PushHandler) OnCopied(opts *option.Target, root ocispec.Descriptor) error { if opts.RawReference != "" && !contentutil.IsDigest(opts.Reference) { ph.tagged.AddTag(opts.Reference) } ph.path = opts.Path + ph.root = root return nil } -// OnCompleted is called after the push is completed. -func (ph *PushHandler) OnCompleted(root ocispec.Descriptor) error { - return output.ParseAndWrite(ph.out, model.NewPush(root, ph.path, ph.tagged.Tags()), ph.template) +// Render implements PushHandler. +func (ph *PushHandler) Render() error { + return output.ParseAndWrite(ph.out, model.NewPush(ph.root, ph.path, ph.tagged.Tags()), ph.template) } diff --git a/cmd/oras/internal/display/metadata/text/push.go b/cmd/oras/internal/display/metadata/text/push.go index b84b510a3..317345a63 100644 --- a/cmd/oras/internal/display/metadata/text/push.go +++ b/cmd/oras/internal/display/metadata/text/push.go @@ -28,6 +28,7 @@ import ( type PushHandler struct { printer *output.Printer tagLock sync.Mutex + root ocispec.Descriptor } // NewPushHandler returns a new handler for push events. @@ -45,15 +46,16 @@ func (h *PushHandler) OnTagged(_ ocispec.Descriptor, tag string) error { } // OnCopied is called after files are copied. -func (h *PushHandler) OnCopied(opts *option.Target) error { +func (h *PushHandler) OnCopied(opts *option.Target, root ocispec.Descriptor) error { + h.root = root return h.printer.Println("Pushed", opts.AnnotatedReference()) } -// OnCompleted is called after the push is completed. -func (h *PushHandler) OnCompleted(root ocispec.Descriptor) error { - err := h.printer.Println("ArtifactType:", root.ArtifactType) +// Render implements PushHandler. +func (h *PushHandler) Render() error { + err := h.printer.Println("ArtifactType:", h.root.ArtifactType) if err != nil { return err } - return h.printer.Println("Digest:", root.Digest) + return h.printer.Println("Digest:", h.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 960de4134..8871b8e35 100644 --- a/cmd/oras/internal/display/metadata/text/push_test.go +++ b/cmd/oras/internal/display/metadata/text/push_test.go @@ -68,8 +68,9 @@ func TestPushHandler_OnCompleted(t *testing.T) { printer := output.NewPrinter(tt.out, os.Stderr) p := &PushHandler{ printer: printer, + root: tt.root, } - if err := p.OnCompleted(tt.root); (err != nil) != tt.wantErr { + if err := p.Render(); (err != nil) != tt.wantErr { t.Errorf("PushHandler.OnCompleted() error = %v, wantErr %v", err, tt.wantErr) } }) diff --git a/cmd/oras/root/push.go b/cmd/oras/root/push.go index bc47c4fd4..1b7396aa2 100644 --- a/cmd/oras/root/push.go +++ b/cmd/oras/root/push.go @@ -218,11 +218,11 @@ func runPush(cmd *cobra.Command, opts *pushOptions) error { } memoryStore := memory.New() union := contentutil.MultiReadOnlyTarget(memoryStore, store) - displayStatus, displayMetadata, err := display.NewPushHandler(opts.Printer, opts.Format, opts.TTY, union) + statusHandler, metadataHandler, err := display.NewPushHandler(opts.Printer, opts.Format, opts.TTY, union) if err != nil { return err } - descs, err := loadFiles(ctx, store, opts.Annotations, opts.FileRefs, displayStatus) + descs, err := loadFiles(ctx, store, opts.Annotations, opts.FileRefs, statusHandler) if err != nil { return err } @@ -243,15 +243,15 @@ func runPush(cmd *cobra.Command, opts *pushOptions) error { if err != nil { return err } - dst, stopTrack, err := displayStatus.TrackTarget(originalDst) + dst, stopTrack, err := statusHandler.TrackTarget(originalDst) if err != nil { return err } copyOptions := oras.DefaultCopyOptions copyOptions.Concurrency = opts.concurrency - copyOptions.CopyGraphOptions.OnCopySkipped = displayStatus.OnCopySkipped - copyOptions.CopyGraphOptions.PreCopy = displayStatus.PreCopy - copyOptions.CopyGraphOptions.PostCopy = displayStatus.PostCopy + copyOptions.CopyGraphOptions.OnCopySkipped = statusHandler.OnCopySkipped + copyOptions.CopyGraphOptions.PreCopy = statusHandler.PreCopy + copyOptions.CopyGraphOptions.PostCopy = statusHandler.PostCopy copyWithScopeHint := func(root ocispec.Descriptor) error { // add both pull and push scope hints for dst repository // to save potential push-scope token requests during copy @@ -270,7 +270,7 @@ func runPush(cmd *cobra.Command, opts *pushOptions) error { if err != nil { return err } - err = displayMetadata.OnCopied(&opts.Target) + err = metadataHandler.OnCopied(&opts.Target, root) if err != nil { return err } @@ -282,13 +282,13 @@ func runPush(cmd *cobra.Command, opts *pushOptions) error { } tagBytesNOpts := oras.DefaultTagBytesNOptions tagBytesNOpts.Concurrency = opts.concurrency - dst := listener.NewTagListener(originalDst, nil, displayMetadata.OnTagged) + dst := listener.NewTagListener(originalDst, nil, metadataHandler.OnTagged) if _, err = oras.TagBytesN(ctx, dst, root.MediaType, contentBytes, opts.extraRefs, tagBytesNOpts); err != nil { return err } } - err = displayMetadata.OnCompleted(root) + err = metadataHandler.Render() if err != nil { return err } From 245b141d3d92bbeae791a63429ea527bbb87f545 Mon Sep 17 00:00:00 2001 From: Xiaoxuan Wang <103478229+wangxiaoxuan273@users.noreply.github.com> Date: Mon, 16 Dec 2024 16:59:48 +0800 Subject: [PATCH 2/2] refactor: add handlers to `manifest push` commmand (#1555) Signed-off-by: Xiaoxuan Wang --- cmd/oras/internal/display/handler.go | 7 +++-- cmd/oras/internal/display/metadata/discard.go | 10 +++++++ .../internal/display/metadata/discard_test.go | 7 +++++ .../internal/display/metadata/interface.go | 3 +++ .../display/metadata/text/manifest_push.go | 17 +++++++++++- cmd/oras/internal/display/status/discard.go | 15 +++++++++++ .../internal/display/status/discard_test.go | 7 +++++ cmd/oras/internal/display/status/interface.go | 7 +++++ cmd/oras/internal/display/status/text.go | 26 +++++++++++++++++++ cmd/oras/internal/display/status/text_test.go | 7 +++++ cmd/oras/root/manifest/push.go | 20 +++++++------- 11 files changed, 113 insertions(+), 13 deletions(-) diff --git a/cmd/oras/internal/display/handler.go b/cmd/oras/internal/display/handler.go index ce6243d9d..56bb9700c 100644 --- a/cmd/oras/internal/display/handler.go +++ b/cmd/oras/internal/display/handler.go @@ -170,8 +170,11 @@ func NewTagHandler(printer *output.Printer, target option.Target) metadata.TagHa } // NewManifestPushHandler returns a manifest push handler. -func NewManifestPushHandler(printer *output.Printer) metadata.ManifestPushHandler { - return text.NewManifestPushHandler(printer) +func NewManifestPushHandler(printer *output.Printer, outputDescriptor bool, pretty bool, desc ocispec.Descriptor, target *option.Target) (status.ManifestPushHandler, metadata.ManifestPushHandler) { + if outputDescriptor { + return status.NewDiscardHandler(), metadata.NewDiscardHandler() + } + return status.NewTextManifestPushHandler(printer, desc), text.NewManifestPushHandler(printer, target) } // NewManifestIndexCreateHandler returns status, metadata and content handlers for index create command. diff --git a/cmd/oras/internal/display/metadata/discard.go b/cmd/oras/internal/display/metadata/discard.go index d5a3d3e6d..f2cf9d604 100644 --- a/cmd/oras/internal/display/metadata/discard.go +++ b/cmd/oras/internal/display/metadata/discard.go @@ -31,6 +31,16 @@ func (Discard) OnFetched(string, ocispec.Descriptor, []byte) error { return nil } +// OnManifestPushed implements ManifestPushHandler. +func (Discard) OnManifestPushed(ocispec.Descriptor) error { + return nil +} + +// Render implements ManifestPushHandler. +func (Discard) Render() error { + return nil +} + // OnTagged implements ManifestIndexCreateHandler. func (Discard) OnTagged(ocispec.Descriptor, string) error { return nil diff --git a/cmd/oras/internal/display/metadata/discard_test.go b/cmd/oras/internal/display/metadata/discard_test.go index 9e9e67a8f..c3d8f83f6 100644 --- a/cmd/oras/internal/display/metadata/discard_test.go +++ b/cmd/oras/internal/display/metadata/discard_test.go @@ -27,3 +27,10 @@ func TestDiscard_OnTagged(t *testing.T) { t.Errorf("testDiscard.OnTagged() error = %v, want nil", err) } } + +func TestDiscardHandler_OnManifestPushed(t *testing.T) { + testDiscard := NewDiscardHandler() + if err := testDiscard.OnManifestPushed(ocispec.Descriptor{}); err != nil { + t.Errorf("DiscardHandler.OnManifestPushed() error = %v, wantErr nil", err) + } +} diff --git a/cmd/oras/internal/display/metadata/interface.go b/cmd/oras/internal/display/metadata/interface.go index 71c26de5f..cc8d504c0 100644 --- a/cmd/oras/internal/display/metadata/interface.go +++ b/cmd/oras/internal/display/metadata/interface.go @@ -83,6 +83,9 @@ type TagHandler interface { // ManifestPushHandler handles metadata output for manifest push events. type ManifestPushHandler interface { TaggedHandler + Renderer + + OnManifestPushed(desc ocispec.Descriptor) error } // ManifestIndexCreateHandler handles metadata output for index create events. diff --git a/cmd/oras/internal/display/metadata/text/manifest_push.go b/cmd/oras/internal/display/metadata/text/manifest_push.go index 743ba7ee2..594de1245 100644 --- a/cmd/oras/internal/display/metadata/text/manifest_push.go +++ b/cmd/oras/internal/display/metadata/text/manifest_push.go @@ -18,18 +18,22 @@ package text import ( 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" ) // ManifestPushHandler handles text metadata output for manifest push events. type ManifestPushHandler struct { printer *output.Printer + target *option.Target + desc ocispec.Descriptor } // NewManifestPushHandler returns a new handler for manifest push events. -func NewManifestPushHandler(printer *output.Printer) metadata.ManifestPushHandler { +func NewManifestPushHandler(printer *output.Printer, target *option.Target) metadata.ManifestPushHandler { return &ManifestPushHandler{ printer: printer, + target: target, } } @@ -37,3 +41,14 @@ func NewManifestPushHandler(printer *output.Printer) metadata.ManifestPushHandle func (h *ManifestPushHandler) OnTagged(_ ocispec.Descriptor, tag string) error { return h.printer.Println("Tagged", tag) } + +// OnManifestPushed implements metadata.ManifestPushHandler. +func (h *ManifestPushHandler) OnManifestPushed(desc ocispec.Descriptor) error { + h.desc = desc + return h.printer.Println("Pushed:", h.target.AnnotatedReference()) +} + +// Render implements metadata.ManifestPushHandler. +func (h *ManifestPushHandler) Render() error { + return h.printer.Println("Digest:", h.desc.Digest) +} diff --git a/cmd/oras/internal/display/status/discard.go b/cmd/oras/internal/display/status/discard.go index 2bdc09ff4..cf7b40358 100644 --- a/cmd/oras/internal/display/status/discard.go +++ b/cmd/oras/internal/display/status/discard.go @@ -100,6 +100,21 @@ func (DiscardHandler) OnFetched(string, ocispec.Descriptor) error { return nil } +// OnManifestPushSkipped implements ManifestPushHandler. +func (DiscardHandler) OnManifestPushSkipped() error { + return nil +} + +// OnManifestPushing implements ManifestPushHandler. +func (DiscardHandler) OnManifestPushing() error { + return nil +} + +// OnManifestPushed implements ManifestPushHandler. +func (DiscardHandler) OnManifestPushed() error { + return nil +} + // OnManifestRemoved implements ManifestIndexUpdateHandler. func (DiscardHandler) OnManifestRemoved(digest.Digest) error { return nil diff --git a/cmd/oras/internal/display/status/discard_test.go b/cmd/oras/internal/display/status/discard_test.go index bf5a71363..757006941 100644 --- a/cmd/oras/internal/display/status/discard_test.go +++ b/cmd/oras/internal/display/status/discard_test.go @@ -21,6 +21,13 @@ import ( v1 "github.com/opencontainers/image-spec/specs-go/v1" ) +func TestDiscardHandler_OnPushSkipped(t *testing.T) { + testDiscard := NewDiscardHandler() + if err := testDiscard.OnManifestPushSkipped(); err != nil { + t.Errorf("DiscardHandler.OnPushSkipped() error = %v, wantErr nil", err) + } +} + func TestDiscardHandler_OnManifestRemoved(t *testing.T) { testDiscard := NewDiscardHandler() if err := testDiscard.OnManifestRemoved("sha256:44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a"); err != nil { diff --git a/cmd/oras/internal/display/status/interface.go b/cmd/oras/internal/display/status/interface.go index dafcdb408..b2e243d48 100644 --- a/cmd/oras/internal/display/status/interface.go +++ b/cmd/oras/internal/display/status/interface.go @@ -66,6 +66,13 @@ type CopyHandler interface { StopTracking() error } +// ManifestPushHandler handles status output for manifest push command. +type ManifestPushHandler interface { + OnManifestPushSkipped() error + OnManifestPushing() error + OnManifestPushed() error +} + // ManifestIndexCreateHandler handles status output for manifest index create command. type ManifestIndexCreateHandler interface { OnFetching(manifestRef string) error diff --git a/cmd/oras/internal/display/status/text.go b/cmd/oras/internal/display/status/text.go index 618f93725..ee6852abb 100644 --- a/cmd/oras/internal/display/status/text.go +++ b/cmd/oras/internal/display/status/text.go @@ -193,6 +193,32 @@ func (ch *TextCopyHandler) OnMounted(_ context.Context, desc ocispec.Descriptor) return ch.printer.PrintStatus(desc, copyPromptMounted) } +// TextManifestPushHandler handles text status output for manifest push events. +type TextManifestPushHandler struct { + desc ocispec.Descriptor + printer *output.Printer +} + +// NewTextManifestPushHandler returns a new handler for manifest push command. +func NewTextManifestPushHandler(printer *output.Printer, desc ocispec.Descriptor) ManifestPushHandler { + return &TextManifestPushHandler{ + desc: desc, + printer: printer, + } +} + +func (mph *TextManifestPushHandler) OnManifestPushSkipped() error { + return mph.printer.PrintStatus(mph.desc, PushPromptExists) +} + +func (mph *TextManifestPushHandler) OnManifestPushing() error { + return mph.printer.PrintStatus(mph.desc, PushPromptUploading) +} + +func (mph *TextManifestPushHandler) OnManifestPushed() error { + return mph.printer.PrintStatus(mph.desc, PushPromptUploaded) +} + // TextManifestIndexCreateHandler handles text status output for manifest index create events. type TextManifestIndexCreateHandler struct { printer *output.Printer diff --git a/cmd/oras/internal/display/status/text_test.go b/cmd/oras/internal/display/status/text_test.go index 85d354771..37770b533 100644 --- a/cmd/oras/internal/display/status/text_test.go +++ b/cmd/oras/internal/display/status/text_test.go @@ -194,6 +194,13 @@ func TestTextPushHandler_PreCopy(t *testing.T) { validatePrinted(t, "Uploading 0b442c23c1dd oci-image") } +func TestTextManifestPushHandler_OnPushSkipped(t *testing.T) { + mph := NewTextManifestPushHandler(printer, ocispec.Descriptor{}) + if mph.OnManifestPushSkipped() != nil { + t.Error("OnManifestExists() should not return an error") + } +} + func TestTextManifestIndexUpdateHandler_OnManifestAdded(t *testing.T) { tests := []struct { name string diff --git a/cmd/oras/root/manifest/push.go b/cmd/oras/root/manifest/push.go index 7fea60046..57f07b18b 100644 --- a/cmd/oras/root/manifest/push.go +++ b/cmd/oras/root/manifest/push.go @@ -101,7 +101,7 @@ Example - Push a manifest to an OCI image layout folder 'layout-dir' and tag wit return option.Parse(cmd, &opts) }, RunE: func(cmd *cobra.Command, args []string) error { - opts.Printer.Verbose = opts.verbose && !opts.OutputDescriptor + opts.Printer.Verbose = opts.verbose return pushManifest(cmd, opts) }, } @@ -151,6 +151,7 @@ func pushManifest(cmd *cobra.Command, opts pushOptions) error { // prepare manifest descriptor desc := content.NewDescriptorFromBytes(mediaType, contentBytes) + statusHandler, metadataHandler := display.NewManifestPushHandler(opts.Printer, opts.OutputDescriptor, opts.Pretty.Pretty, desc, &opts.Target) ref := opts.Reference if ref == "" { @@ -161,17 +162,17 @@ func pushManifest(cmd *cobra.Command, opts pushOptions) error { return err } if match { - if err := opts.Printer.PrintStatus(desc, "Exists"); err != nil { + if err := statusHandler.OnManifestPushSkipped(); err != nil { return err } } else { - if err = opts.Printer.PrintStatus(desc, "Uploading"); err != nil { + if err = statusHandler.OnManifestPushing(); err != nil { return err } if _, err := oras.TagBytes(ctx, target, mediaType, contentBytes, ref); err != nil { return err } - if err = opts.Printer.PrintStatus(desc, "Uploaded "); err != nil { + if err = statusHandler.OnManifestPushed(); err != nil { return err } } @@ -192,18 +193,17 @@ func pushManifest(cmd *cobra.Command, opts pushOptions) error { } return opts.Output(os.Stdout, descJSON) } - _ = opts.Printer.Println("Pushed", opts.AnnotatedReference()) + if err := metadataHandler.OnManifestPushed(desc); err != nil { + return err + } if len(opts.extraRefs) != 0 { - handler := display.NewManifestPushHandler(opts.Printer) - tagListener := listener.NewTaggedListener(target, handler.OnTagged) + tagListener := listener.NewTaggedListener(target, metadataHandler.OnTagged) if _, err = oras.TagBytesN(ctx, tagListener, mediaType, contentBytes, opts.extraRefs, tagBytesNOpts); err != nil { return err } } - _ = opts.Printer.Println("Digest:", desc.Digest) - - return nil + return metadataHandler.Render() } // matchDigest checks whether the manifest's digest matches to it in the remote