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 17 commits into
base: main
Choose a base branch
from
7 changes: 6 additions & 1 deletion cmd/oras/internal/display/content/discard.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,12 @@ func (discardHandler) OnContentFetched(ocispec.Descriptor, []byte) error {
return nil
}

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

// NewDiscardHandler returns a new discard handler.
func NewDiscardHandler() ManifestFetchHandler {
func NewDiscardHandler() discardHandler {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we export discardHandler just like status.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
54 changes: 54 additions & 0 deletions cmd/oras/internal/display/content/manifest_index.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/*
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 != "" {
Copy link
Member

Choose a reason for hiding this comment

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

if you changed outputPath to "" for "-" this logic would look less odd.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did you mean to put "" before "-"? Changed the if condition to if outputPath != "" && 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 39 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

Added line #L39 was 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.

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 {
return &manifestIndexCreate{
qweeah marked this conversation as resolved.
Show resolved Hide resolved
pretty: pretty,
stdout: out,
outputPath: outputPath,
}
}
53 changes: 50 additions & 3 deletions cmd/oras/internal/display/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,9 +174,56 @@ 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,
error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

When will an error be returned?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch. Removed the error return.

var statusHandler status.ManifestIndexCreateHandler
var metadataHandler metadata.ManifestIndexCreateHandler
var contentHandler content.ManifestIndexCreateHandler
switch outputPath {
case "":
statusHandler = status.NewTextManifestIndexCreateHandler(printer)
metadataHandler = text.NewManifestIndexCreateHandler(printer)
contentHandler = content.NewDiscardHandler()
case "-":
statusHandler = status.NewDiscardHandler()
metadataHandler = metadata.NewDiscardHandler()
contentHandler = content.NewManifestIndexCreateHandler(printer, pretty, outputPath)
default:
statusHandler = status.NewTextManifestIndexCreateHandler(printer)
metadataHandler = text.NewManifestIndexCreateHandler(printer)
contentHandler = content.NewManifestIndexCreateHandler(printer, pretty, outputPath)
}
wangxiaoxuan273 marked this conversation as resolved.
Show resolved Hide resolved
return statusHandler, metadataHandler, contentHandler, nil
}

// 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,
error) {
var statusHandler status.ManifestIndexUpdateHandler
var metadataHandler metadata.ManifestIndexUpdateHandler
var contentHandler content.ManifestIndexUpdateHandler
switch outputPath {
case "":
statusHandler = status.NewTextManifestIndexUpdateHandler(printer)
metadataHandler = text.NewManifestIndexCreateHandler(printer)
contentHandler = content.NewDiscardHandler()
case "-":
statusHandler = status.NewDiscardHandler()
metadataHandler = metadata.NewDiscardHandler()
contentHandler = content.NewManifestIndexCreateHandler(printer, pretty, outputPath)
default:
statusHandler = status.NewTextManifestIndexUpdateHandler(printer)
metadataHandler = text.NewManifestIndexCreateHandler(printer)
contentHandler = content.NewManifestIndexCreateHandler(printer, pretty, outputPath)
}
return statusHandler, metadataHandler, contentHandler, nil
}

// NewCopyHandler returns copy handlers.
Expand Down
15 changes: 14 additions & 1 deletion cmd/oras/internal/display/metadata/discard.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@

package metadata

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

type discard struct{}
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you want to export it just like other discard handlers?


Expand All @@ -28,3 +31,13 @@
func (discard) OnFetched(string, ocispec.Descriptor, []byte) error {
return nil
}

// OnTagged implements ManifestIndexCreateHandler.
func (discard) OnTagged(desc ocispec.Descriptor, tag string) error {
wangxiaoxuan273 marked this conversation as resolved.
Show resolved Hide resolved
return nil

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

View check run for this annotation

Codecov / codecov/patch

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

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

// OnCompleted implements ManifestIndexCreateHandler.
func (discard) OnCompleted(digest digest.Digest) error {
wangxiaoxuan273 marked this conversation as resolved.
Show resolved Hide resolved
return nil
}
5 changes: 5 additions & 0 deletions cmd/oras/internal/display/metadata/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ limitations under the License.
package metadata

import (
"github.com/opencontainers/go-digest"
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
"oras.land/oras/cmd/oras/internal/option"
)
Expand Down Expand Up @@ -81,8 +82,12 @@ type ManifestPushHandler interface {
// ManifestIndexCreateHandler handles metadata output for index create events.
type ManifestIndexCreateHandler interface {
TaggedHandler
OnCompleted(digest digest.Digest) error
wangxiaoxuan273 marked this conversation as resolved.
Show resolved Hide resolved
}

// 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 @@ -16,6 +16,7 @@ limitations under the License.
package text

import (
"github.com/opencontainers/go-digest"
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
"oras.land/oras/cmd/oras/internal/display/metadata"
"oras.land/oras/cmd/oras/internal/output"
Expand All @@ -37,3 +38,8 @@ func NewManifestIndexCreateHandler(printer *output.Printer) metadata.ManifestInd
func (h *ManifestIndexCreateHandler) OnTagged(_ ocispec.Descriptor, tag string) error {
return h.printer.Println("Tagged", tag)
}

// OnCompleted implements ManifestIndexCreateHandler.
func (h *ManifestIndexCreateHandler) OnCompleted(digest digest.Digest) error {
wangxiaoxuan273 marked this conversation as resolved.
Show resolved Hide resolved
return h.printer.Println("Digest:", digest)
}
61 changes: 61 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,63 @@
func (DiscardHandler) OnNodeSkipped(desc ocispec.Descriptor) error {
return nil
}

// OnSourceManifestFetching implements ManifestIndexCreateHandler.
func (DiscardHandler) OnSourceManifestFetching(source string) error {
return nil
}

// OnSourceManifestFetched implements ManifestIndexCreateHandler.
func (DiscardHandler) OnSourceManifestFetched(source string) error {
return nil
}

// OnManifestFetching implements ManifestIndexUpdateHandler.
func (DiscardHandler) OnManifestFetching(ref string) error {
return nil
}

// OnManifestFetched implements ManifestIndexUpdateHandler.
func (DiscardHandler) OnManifestFetched(ref string, digest digest.Digest) error {
return nil
}

// OnManifestRemoved implements ManifestIndexUpdateHandler.
func (DiscardHandler) OnManifestRemoved(digest digest.Digest) 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
}

// OnManifestAdded implements ManifestIndexUpdateHandler.
func (DiscardHandler) OnManifestAdded(ref string, digest digest.Digest) error {
return nil
}

// OnIndexFetching implements ManifestIndexUpdateHandler.
func (DiscardHandler) OnIndexFetching(indexRef string) error {
return nil
}

// OnIndexFetched implements ManifestIndexUpdateHandler.
func (DiscardHandler) OnIndexFetched(indexRef string, digest digest.Digest) error {
return nil
}

// OnIndexMerged implements ManifestIndexUpdateHandler.
func (DiscardHandler) OnIndexMerged(indexRef string, digest digest.Digest) error {
return nil

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

View check run for this annotation

Codecov / codecov/patch

cmd/oras/internal/display/status/discard.go#L134-L135

Added lines #L134 - L135 were not covered by tests
}

// OnIndexPacked implements ManifestIndexCreateHandler.
func (DiscardHandler) OnIndexPacked(shortDigest string) error {
return nil
}

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

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

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

View check run for this annotation

Codecov / codecov/patch

cmd/oras/internal/display/status/discard.go#L149-L150

Added lines #L149 - L150 were not covered by tests
}
23 changes: 23 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,24 @@ 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 {
OnSourceManifestFetching(source string) error
OnSourceManifestFetched(source string) error
qweeah marked this conversation as resolved.
Show resolved Hide resolved
OnIndexPacked(shortDigest string) error
qweeah marked this conversation as resolved.
Show resolved Hide resolved
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.

OnIndexFetching(indexRef string) error
OnIndexFetched(indexRef string, digest digest.Digest) error
OnManifestFetching(manifestRef string) error
OnManifestFetched(manifestRef string, digest digest.Digest) error
OnManifestRemoved(digest digest.Digest) error
OnManifestAdded(manifestRef string, digest digest.Digest) error
OnIndexMerged(indexRef string, digest digest.Digest) error
qweeah marked this conversation as resolved.
Show resolved Hide resolved
OnIndexUpdated(digest digest.Digest) error
OnIndexPushed(path string) error
}
Loading
Loading