Repository: oras-project/oras
Pull request #538 (2022-09-07 02:52:34)

refactor: use read only target to replace oras.Target for the target storage with caching
Use `oras.ReadOnlyTarget` instead of `oras.Target` for the target storage with caching

Signed-off-by: Zoey Li <zoeyli@microsoft.com>

codecov-commenter: # [Codecov](https://codecov.io/gh/oras-project/oras/pull/538?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oras-project) Report
> Merging [#538](https://codecov.io/gh/oras-project/oras/pull/538?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oras-project) (539abca) into [main](https://codecov.io/gh/oras-project/oras/commit/2bf9456a58daef2193b3583ab1bb5dc3dd6fc666?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oras-project) (2bf9456) will **decrease** coverage by `1.90%`.
> The diff coverage is `71.42%`.

```diff
@@            Coverage Diff             @@
##             main     #538      +/-   ##
==========================================
- Coverage   73.26%   71.36%   -1.91%     
==========================================
  Files          11       11              
  Lines         419      419              
==========================================
- Hits          307      299       -8     
- Misses         89       98       +9     
+ Partials       23       22       -1     
```


| [Impacted Files](https://codecov.io/gh/oras-project/oras/pull/538?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oras-project) | Coverage Δ | |
|---|---|---|
| [internal/cache/target.go](https://codecov.io/gh/oras-project/oras/pull/538/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oras-project#diff-aW50ZXJuYWwvY2FjaGUvdGFyZ2V0Lmdv) | `60.86% <60.00%> (-11.60%)` | :arrow_down: |
| [internal/cas/fetch.go](https://codecov.io/gh/oras-project/oras/pull/538/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oras-project#diff-aW50ZXJuYWwvY2FzL2ZldGNoLmdv) | `90.90% <100.00%> (ø)` | |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oras-project). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oras-project)


Patch:
diff --git a/cmd/oras/pull.go b/cmd/oras/pull.go
index ed34233..0af49aa 100644
--- a/cmd/oras/pull.go
+++ b/cmd/oras/pull.go
@@ -94,7 +94,7 @@ func runPull(opts pullOptions) error {
 	if repo.Reference.Reference == "" {
 		return errors.NewErrInvalidReference(repo.Reference)
 	}
-	var src oras.Target = repo
+	var src oras.ReadOnlyTarget = repo
 	if opts.cacheRoot != "" {
 		ociStore, err := oci.New(opts.cacheRoot)
 		if err != nil {
diff --git a/internal/cache/target.go b/internal/cache/target.go
index 2955233..0d1e37a 100644
--- a/internal/cache/target.go
+++ b/internal/cache/target.go
@@ -34,15 +34,15 @@ func (fn closer) Close() error {
 
 // Cache target struct.
 type target struct {
-	oras.Target
+	oras.ReadOnlyTarget
 	cache content.Storage
 }
 
 // New generates a new target storage with caching.
-func New(source oras.Target, cache content.Storage) oras.Target {
+func New(source oras.ReadOnlyTarget, cache content.Storage) oras.ReadOnlyTarget {
 	t := &target{
-		Target: source,
-		cache:  cache,
+		ReadOnlyTarget: source,
+		cache:          cache,
 	}
 	if refFetcher, ok := source.(registry.ReferenceFetcher); ok {
 		return &referenceTarget{
@@ -61,7 +61,7 @@ func (t *target) Fetch(ctx context.Context, target ocispec.Descriptor) (io.ReadC
 		return rc, nil
 	}
 
-	if rc, err = t.Target.Fetch(ctx, target); err != nil {
+	if rc, err = t.ReadOnlyTarget.Fetch(ctx, target); err != nil {
 		return nil, err
 	}
 
@@ -105,7 +105,7 @@ func (t *target) Exists(ctx context.Context, desc ocispec.Descriptor) (bool, err
 	if err == nil && exists {
 		return true, nil
 	}
-	return t.Target.Exists(ctx, desc)
+	return t.ReadOnlyTarget.Exists(ctx, desc)
 }
 
 // Cache referenceTarget struct.
diff --git a/internal/cas/fetch.go b/internal/cas/fetch.go
index ff357ea..2763157 100644
--- a/internal/cas/fetch.go
+++ b/internal/cas/fetch.go
@@ -29,7 +29,7 @@ import (
 
 // FetchDescriptor fetches a minimal descriptor of reference from target.
 // If platform flag not empty, will fetch the specified platform.
-func FetchDescriptor(ctx context.Context, target oras.Target, reference string, p *ocispec.Platform) ([]byte, error) {
+func FetchDescriptor(ctx context.Context, target oras.ReadOnlyTarget, reference string, p *ocispec.Platform) ([]byte, error) {
 	desc, err := oras.Resolve(ctx, target, reference, oras.ResolveOptions{TargetPlatform: p})
 	if err != nil {
 		return nil, err
@@ -43,7 +43,7 @@ func FetchDescriptor(ctx context.Context, target oras.Target, reference string,
 
 // FetchManifest fetches the manifest content of reference from target.
 // If platform flag not empty, will fetch the specified platform.
-func FetchManifest(ctx context.Context, target oras.Target, reference string, p *ocispec.Platform) ([]byte, error) {
+func FetchManifest(ctx context.Context, target oras.ReadOnlyTarget, reference string, p *ocispec.Platform) ([]byte, error) {
 	// TODO: improve implementation once oras-go#102 is resolved
 	if p == nil {
 		if rf, ok := target.(registry.ReferenceFetcher); ok {


Test patch:
diff --git a/internal/cache/target_test.go b/internal/cache/target_test.go
index bc44855..8f9cf22 100644
--- a/internal/cache/target_test.go
+++ b/internal/cache/target_test.go
@@ -31,6 +31,7 @@ import (
 
 	"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"
 	"oras.land/oras-go/v2/content/memory"
 	"oras.land/oras-go/v2/registry"
@@ -45,7 +46,17 @@ func TestProxy_fetchCache(t *testing.T) {
 		Size:      int64(len(blob)),
 	}
 
-	p := New(memory.New(), memory.New())
+	target := memory.New()
+	p := struct {
+		oras.Target
+		oras.ReadOnlyTarget
+		cache content.Storage
+	}{
+		target,
+		target,
+		memory.New(),
+	}
+
 	ctx := context.Background()
 
 	err := p.Push(ctx, desc, bytes.NewReader(blob))
@@ -54,14 +65,14 @@ func TestProxy_fetchCache(t *testing.T) {
 	}
 
 	// first fetch
-	exists, err := p.Exists(ctx, desc)
+	exists, err := p.Target.Exists(ctx, desc)
 	if err != nil {
 		t.Fatal("Proxy.Exists() error =", err)
 	}
 	if !exists {
 		t.Errorf("Proxy.Exists() = %v, want %v", exists, true)
 	}
-	got, err := content.FetchAll(ctx, p, desc)
+	got, err := content.FetchAll(ctx, p.Target, desc)
 	if err != nil {
 		t.Fatal("Proxy.Fetch() error =", err)
 	}
@@ -71,16 +82,16 @@ func TestProxy_fetchCache(t *testing.T) {
 
 	// repeated fetch should not touch base CAS
 	// nil base will generate panic if the base CAS is touched
-	p.(*target).Target = nil
+	p.Target = nil
 
-	exists, err = p.Exists(ctx, desc)
+	exists, err = p.ReadOnlyTarget.Exists(ctx, desc)
 	if err != nil {
 		t.Fatal("Proxy.Exists() error =", err)
 	}
 	if !exists {
 		t.Errorf("Proxy.Exists() = %v, want %v", exists, true)
 	}
-	got, err = content.FetchAll(ctx, p, desc)
+	got, err = content.FetchAll(ctx, p.ReadOnlyTarget, desc)
 	if err != nil {
 		t.Fatal("Proxy.Fetch() error =", err)
 	}
@@ -97,7 +108,13 @@ func TestProxy_pushPassThrough(t *testing.T) {
 		Size:      int64(len(blob)),
 	}
 
-	p := New(memory.New(), memory.New())
+	p := struct {
+		oras.Target
+		cache content.Storage
+	}{
+		memory.New(),
+		memory.New(),
+	}
 	ctx := context.Background()
 
 	// before push
@@ -206,7 +223,7 @@ func TestProxy_fetchReference(t *testing.T) {
 	}
 
 	// repeated fetch should not touch base CAS
-	p.(*referenceTarget).Target = nil
+	p.(*referenceTarget).ReadOnlyTarget = nil
 	got, err = content.FetchAll(ctx, p, desc)
 	if err != nil {
 		t.Fatal("ReferenceTarget.Fetch() error =", err)