Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: status, metadata and content handlers for manifest index commands #1509

Open
wants to merge 19 commits into
base: main
Choose a base branch
from
13 changes: 9 additions & 4 deletions cmd/oras/internal/display/content/discard.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,19 @@ package content

import ocispec "github.com/opencontainers/image-spec/specs-go/v1"

type discardHandler struct{}
type DiscardHandler struct{}

// OnContentFetched implements ManifestFetchHandler.
func (discardHandler) OnContentFetched(ocispec.Descriptor, []byte) error {
func (DiscardHandler) OnContentFetched(ocispec.Descriptor, []byte) error {
return nil
}

// OnContentCreated implements ManifestIndexCreateHandler.
func (DiscardHandler) OnContentCreated([]byte) error {
return nil
}

// NewDiscardHandler returns a new discard handler.
func NewDiscardHandler() ManifestFetchHandler {
return discardHandler{}
func NewDiscardHandler() DiscardHandler {
return DiscardHandler{}
}
9 changes: 9 additions & 0 deletions cmd/oras/internal/display/content/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,12 @@ type ManifestFetchHandler interface {
// OnContentFetched is called after the manifest content is fetched.
OnContentFetched(desc ocispec.Descriptor, content []byte) error
}

// ManifestIndexCreateHandler handles raw output for manifest index create events.
type ManifestIndexCreateHandler interface {
// OnContentCreated is called after the index content is created.
OnContentCreated(content []byte) error
}

// ManifestIndexUpdateHandler handles raw output for manifest index update events.
type ManifestIndexUpdateHandler ManifestIndexCreateHandler
4 changes: 4 additions & 0 deletions cmd/oras/internal/display/content/manifest_fetch.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ func (h *manifestFetch) OnContentFetched(desc ocispec.Descriptor, manifest []byt

// NewManifestFetchHandler creates a new handler.
func NewManifestFetchHandler(out io.Writer, pretty bool, outputPath string) ManifestFetchHandler {
// ignore --pretty when output to a file
if outputPath != "" && outputPath != "-" {
pretty = false
}
return &manifestFetch{
pretty: pretty,
stdout: out,
Expand Down
58 changes: 58 additions & 0 deletions cmd/oras/internal/display/content/manifest_index.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/*
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 content

import (
"fmt"
"io"
"os"

"oras.land/oras/cmd/oras/internal/output"
)

// manifestIndexCreate handles raw content output.
type manifestIndexCreate struct {
pretty bool
stdout io.Writer
outputPath string
}

// OnContentCreated is called after index content is created.
func (h *manifestIndexCreate) OnContentCreated(manifest []byte) error {
out := h.stdout
if h.outputPath != "" && h.outputPath != "-" {
f, err := os.Create(h.outputPath)
if err != nil {
return fmt.Errorf("failed to open %q: %w", h.outputPath, err)
}

Check warning on line 40 in cmd/oras/internal/display/content/manifest_index.go

View check run for this annotation

Codecov / codecov/patch

cmd/oras/internal/display/content/manifest_index.go#L39-L40

Added lines #L39 - L40 were not covered by tests
defer f.Close()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to create an issue to track handling errors returned by Close() in all write paths.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created #1516

out = f
}
return output.PrintJSON(out, manifest, h.pretty)
qweeah marked this conversation as resolved.
Show resolved Hide resolved
}

// NewManifestIndexCreateHandler creates a new handler.
func NewManifestIndexCreateHandler(out io.Writer, pretty bool, outputPath string) ManifestIndexCreateHandler {
// ignore --pretty when output to a file
if outputPath != "" && outputPath != "-" {
pretty = false
}
Comment on lines +49 to +52
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is different from NewManifestFetchHandler. Is NewManifestFetchHandler wrong? /cc @qweeah

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NewManifestFetchHandler is used only once by oras manifest fetch and such code snippet is in command code

// ignore --pretty when output to a file
case opts.outputPath != "" && opts.outputPath != "-":
opts.Pretty.Pretty = false
}

I think we need to remove it and change the flag reset to NewManifestFetchHandler too.

Copy link
Contributor Author

@wangxiaoxuan273 wangxiaoxuan273 Oct 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added pretty value reset in manifest fetch handler too.

return &manifestIndexCreate{
qweeah marked this conversation as resolved.
Show resolved Hide resolved
pretty: pretty,
stdout: out,
outputPath: outputPath,
}
}
37 changes: 34 additions & 3 deletions cmd/oras/internal/display/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,9 +174,40 @@ func NewManifestPushHandler(printer *output.Printer) metadata.ManifestPushHandle
return text.NewManifestPushHandler(printer)
}

// NewManifestIndexCreateHandler returns an index create handler.
func NewManifestIndexCreateHandler(printer *output.Printer) metadata.ManifestIndexCreateHandler {
return text.NewManifestIndexCreateHandler(printer)
// NewManifestIndexCreateHandler returns status, metadata and content handlers for index create command.
func NewManifestIndexCreateHandler(outputPath string, printer *output.Printer, pretty bool) (
status.ManifestIndexCreateHandler,
metadata.ManifestIndexCreateHandler,
content.ManifestIndexCreateHandler) {
statusHandler := status.NewTextManifestIndexCreateHandler(printer)
metadataHandler := text.NewManifestIndexCreateHandler(printer)
contentHandler := content.NewManifestIndexCreateHandler(printer, pretty, outputPath)
switch outputPath {
case "":
contentHandler = content.NewDiscardHandler()
case "-":
statusHandler = status.NewDiscardHandler()
metadataHandler = metadata.NewDiscardHandler()
}
return statusHandler, metadataHandler, contentHandler
}

// NewManifestIndexUpdateHandler returns status, metadata and content handlers for index update command.
func NewManifestIndexUpdateHandler(outputPath string, printer *output.Printer, pretty bool) (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should there just be a NewManivestIndexHandler method for both for now and someone can add the other in the future if needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently we need both create and update handlers.

status.ManifestIndexUpdateHandler,
metadata.ManifestIndexUpdateHandler,
content.ManifestIndexUpdateHandler) {
statusHandler := status.NewTextManifestIndexUpdateHandler(printer)
metadataHandler := text.NewManifestIndexCreateHandler(printer)
contentHandler := content.NewManifestIndexCreateHandler(printer, pretty, outputPath)
switch outputPath {
case "":
contentHandler = content.NewDiscardHandler()
case "-":
statusHandler = status.NewDiscardHandler()
metadataHandler = metadata.NewDiscardHandler()
}
return statusHandler, metadataHandler, contentHandler
}

// NewCopyHandler returns copy handlers.
Expand Down
22 changes: 17 additions & 5 deletions cmd/oras/internal/display/metadata/discard.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,28 @@

package metadata

import ocispec "github.com/opencontainers/image-spec/specs-go/v1"
import (
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
)

type discard struct{}
type Discard struct{}

// NewDiscardHandler creates a new handler that discards output for all events.
func NewDiscardHandler() discard {
return discard{}
func NewDiscardHandler() Discard {
return Discard{}
}

// OnFetched implements ManifestFetchHandler.
func (discard) OnFetched(string, ocispec.Descriptor, []byte) error {
func (Discard) OnFetched(string, ocispec.Descriptor, []byte) error {
return nil
}

// OnTagged implements ManifestIndexCreateHandler.
func (Discard) OnTagged(ocispec.Descriptor, string) error {
return nil

Check warning on line 36 in cmd/oras/internal/display/metadata/discard.go

View check run for this annotation

Codecov / codecov/patch

cmd/oras/internal/display/metadata/discard.go#L35-L36

Added lines #L35 - L36 were not covered by tests
}

// OnCompleted implements ManifestIndexCreateHandler.
func (Discard) OnCompleted(ocispec.Descriptor) error {
return nil
}
4 changes: 4 additions & 0 deletions cmd/oras/internal/display/metadata/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,12 @@ type ManifestPushHandler interface {
// ManifestIndexCreateHandler handles metadata output for index create events.
type ManifestIndexCreateHandler interface {
TaggedHandler
OnCompleted(desc ocispec.Descriptor) error
}

// ManifestIndexUpdateHandler handles metadata output for index update events.
type ManifestIndexUpdateHandler ManifestIndexCreateHandler

// CopyHandler handles metadata output for cp events.
type CopyHandler interface {
TaggedHandler
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,12 @@ func NewManifestIndexCreateHandler(printer *output.Printer) metadata.ManifestInd
}
}

// OnTagged implements metadata.TaggedHandler.
// OnTagged implements TaggedHandler.
func (h *ManifestIndexCreateHandler) OnTagged(_ ocispec.Descriptor, tag string) error {
return h.printer.Println("Tagged", tag)
}

// OnCompleted implements ManifestIndexCreateHandler.
func (h *ManifestIndexCreateHandler) OnCompleted(desc ocispec.Descriptor) error {
return h.printer.Println("Digest:", desc.Digest)
}
36 changes: 36 additions & 0 deletions cmd/oras/internal/display/status/discard.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import (
"context"

"github.com/opencontainers/go-digest"
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
"oras.land/oras-go/v2"
)
Expand Down Expand Up @@ -88,3 +89,38 @@
func (DiscardHandler) OnNodeSkipped(desc ocispec.Descriptor) error {
return nil
}

// OnFetching implements referenceFetchHandler.
func (DiscardHandler) OnFetching(string) error {
return nil
}

// OnFetched implements referenceFetchHandler.
func (DiscardHandler) OnFetched(string, ocispec.Descriptor) error {
return nil
}

// OnManifestRemoved implements ManifestIndexUpdateHandler.
func (DiscardHandler) OnManifestRemoved(digest.Digest) error {
return nil

Check warning on line 105 in cmd/oras/internal/display/status/discard.go

View check run for this annotation

Codecov / codecov/patch

cmd/oras/internal/display/status/discard.go#L104-L105

Added lines #L104 - L105 were not covered by tests
}

// OnManifestAdded implements ManifestIndexUpdateHandler.
func (DiscardHandler) OnManifestAdded(string, ocispec.Descriptor) error {
return nil
}

// OnIndexMerged implements ManifestIndexUpdateHandler.
func (DiscardHandler) OnIndexMerged(string, ocispec.Descriptor) error {
return nil

Check warning on line 115 in cmd/oras/internal/display/status/discard.go

View check run for this annotation

Codecov / codecov/patch

cmd/oras/internal/display/status/discard.go#L114-L115

Added lines #L114 - L115 were not covered by tests
}

// OnIndexPacked implements ManifestIndexCreateHandler.
func (DiscardHandler) OnIndexPacked(ocispec.Descriptor) error {
return nil
}

// OnIndexPushed implements ManifestIndexCreateHandler.
func (DiscardHandler) OnIndexPushed(string) error {
return nil

Check warning on line 125 in cmd/oras/internal/display/status/discard.go

View check run for this annotation

Codecov / codecov/patch

cmd/oras/internal/display/status/discard.go#L124-L125

Added lines #L124 - L125 were not covered by tests
}
18 changes: 18 additions & 0 deletions cmd/oras/internal/display/status/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ package status

import (
"context"

"github.com/opencontainers/go-digest"
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
"oras.land/oras-go/v2"
)
Expand Down Expand Up @@ -61,3 +63,19 @@ type CopyHandler interface {
PostCopy(ctx context.Context, desc ocispec.Descriptor) error
OnMounted(ctx context.Context, desc ocispec.Descriptor) error
}

// ManifestIndexCreateHandler handles status output for manifest index create command.
type ManifestIndexCreateHandler interface {
OnFetching(manifestRef string) error
OnFetched(manifestRef string, desc ocispec.Descriptor) error
OnIndexPacked(desc ocispec.Descriptor) error
OnIndexPushed(path string) error
}

// ManifestIndexUpdateHandler handles status output for manifest index update command.
type ManifestIndexUpdateHandler interface {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Contains duplicated methods ManifestIndexCreateHandler as below

	OnIndexFetching(indexRef string) error
	OnIndexFetched(indexRef string, digest digest.Digest) error

You can create a new interface and use it in both interfaces.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created an interface ManifestReferenceFetchHandler and both index create and index update handlers contain it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on this comment, this interface is now removed.

ManifestIndexCreateHandler
OnManifestRemoved(digest digest.Digest) error
OnManifestAdded(manifestRef string, desc ocispec.Descriptor) error
OnIndexMerged(indexRef string, desc ocispec.Descriptor) error
}
93 changes: 93 additions & 0 deletions cmd/oras/internal/display/status/text.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,11 @@
"context"
"sync"

"oras.land/oras/internal/contentutil"
"oras.land/oras/internal/descriptor"
"oras.land/oras/internal/graph"

"github.com/opencontainers/go-digest"
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
"oras.land/oras-go/v2"
"oras.land/oras-go/v2/content"
Expand Down Expand Up @@ -179,3 +182,93 @@
ch.committed.Store(desc.Digest.String(), desc.Annotations[ocispec.AnnotationTitle])
return ch.printer.PrintStatus(desc, copyPromptMounted)
}

// TextManifestIndexCreateHandler handles text status output for manifest index create events.
type TextManifestIndexCreateHandler struct {
printer *output.Printer
}

// OnFetching implements ManifestIndexCreateHandler.
func (mich TextManifestIndexCreateHandler) OnFetching(source string) error {
return mich.printer.Println(IndexPromptFetching, source)
}

// OnFetched implements ManifestIndexCreateHandler.
func (mich TextManifestIndexCreateHandler) OnFetched(source string, _ ocispec.Descriptor) error {
return mich.printer.Println(IndexPromptFetched, source)
}

// OnIndexPacked implements ManifestIndexCreateHandler.
func (mich TextManifestIndexCreateHandler) OnIndexPacked(desc ocispec.Descriptor) error {
return mich.printer.Println(IndexPromptPacked, descriptor.ShortDigest(desc), ocispec.MediaTypeImageIndex)
}

// OnIndexPushed implements ManifestIndexCreateHandler.
func (mich TextManifestIndexCreateHandler) OnIndexPushed(path string) error {
return mich.printer.Println(IndexPromptPushed, path)
}

// NewTextManifestIndexCreateHandler returns a new handler for manifest index create command.
func NewTextManifestIndexCreateHandler(printer *output.Printer) ManifestIndexCreateHandler {
tmich := TextManifestIndexCreateHandler{
printer: printer,
}
return &tmich
Comment on lines +213 to +216
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current naming style is aligned with the rest of the file.

}

// TextManifestIndexUpdateHandler handles text status output for manifest index update events.
type TextManifestIndexUpdateHandler struct {
printer *output.Printer
}

// OnFetching implements ManifestIndexUpdateHandler.
func (miuh TextManifestIndexUpdateHandler) OnFetching(ref string) error {
return miuh.printer.Println(IndexPromptFetching, ref)
}

// OnFetched implements ManifestIndexUpdateHandler.
func (miuh TextManifestIndexUpdateHandler) OnFetched(ref string, desc ocispec.Descriptor) error {
if contentutil.IsDigest(ref) {
return miuh.printer.Println(IndexPromptFetched, ref)
}
return miuh.printer.Println(IndexPromptFetched, desc.Digest, ref)
}

// OnManifestRemoved implements metadata.ManifestIndexUpdateHandler.
func (miuh TextManifestIndexUpdateHandler) OnManifestRemoved(digest digest.Digest) error {
return miuh.printer.Println("Removed", digest)
}

// OnManifestAdded implements metadata.ManifestIndexUpdateHandler.
func (miuh TextManifestIndexUpdateHandler) OnManifestAdded(ref string, desc ocispec.Descriptor) error {
if contentutil.IsDigest(ref) {
return miuh.printer.Println("Added", ref)
}
return miuh.printer.Println("Added", desc.Digest, ref)

Check warning on line 247 in cmd/oras/internal/display/status/text.go

View check run for this annotation

Codecov / codecov/patch

cmd/oras/internal/display/status/text.go#L247

Added line #L247 was not covered by tests
}

// OnIndexMerged implements metadata.ManifestIndexUpdateHandler.
func (miuh TextManifestIndexUpdateHandler) OnIndexMerged(ref string, desc ocispec.Descriptor) error {
if contentutil.IsDigest(ref) {
return miuh.printer.Println("Merged", ref)
}

Check warning on line 254 in cmd/oras/internal/display/status/text.go

View check run for this annotation

Codecov / codecov/patch

cmd/oras/internal/display/status/text.go#L253-L254

Added lines #L253 - L254 were not covered by tests
return miuh.printer.Println("Merged", desc.Digest, ref)
}

// OnIndexUpdated implements metadata.ManifestIndexUpdateHandler.
func (miuh TextManifestIndexUpdateHandler) OnIndexPacked(desc ocispec.Descriptor) error {
return miuh.printer.Println("Updated", desc.Digest)
}

// OnIndexPushed implements metadata.ManifestIndexUpdateHandler.
func (miuh TextManifestIndexUpdateHandler) OnIndexPushed(indexRef string) error {
return miuh.printer.Println("Pushed", indexRef)
}

// NewTextManifestIndexUpdateHandler returns a new handler for manifest index create command.
func NewTextManifestIndexUpdateHandler(printer *output.Printer) ManifestIndexUpdateHandler {
miuh := TextManifestIndexUpdateHandler{
printer: printer,
}
return &miuh
Comment on lines +270 to +273
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current naming style is aligned with the rest of the file.

}
Loading
Loading