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] 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