feat: implement ExtendedCopy and ExtendedCopyOptions#318
feat: implement ExtendedCopy and ExtendedCopyOptions#318wangxiaoxuan273 wants to merge 34 commits intooras-project:mainfrom
Conversation
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #318 +/- ##
==========================================
+ Coverage 91.63% 91.70% +0.06%
==========================================
Files 64 65 +1
Lines 2678 2700 +22
Branches 361 366 +5
==========================================
+ Hits 2454 2476 +22
Misses 135 135
Partials 89 89 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR implements the ExtendedCopyAsync method and supporting types to enable copying tagged OCI artifacts along with their directed acyclic graph (DAG) of referrers from a source to a destination target. This builds on the existing ExtendedCopyGraphAsync functionality by adding reference resolution and tagging capabilities.
Changes:
- Introduces
IReadOnlyGraphTargetinterface combining graph storage and reference resolution - Adds
ExtendedCopyOptionsclass for configuring extended copy operations - Implements
ExtendedCopyAsyncextension method for copying tagged artifacts with their DAG - Updates
MemoryStoreto implement the newIReadOnlyGraphTargetinterface - Adds comprehensive test coverage for the new functionality
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/OrasProject.Oras/IReadOnlyGraphTarget.cs |
New interface combining IReadOnlyGraphStorage and IResolvable for read-only graph operations with reference resolution |
src/OrasProject.Oras/ExtendedCopyOptions.cs |
New options class inheriting from ExtendedCopyGraphOptions for configuring ExtendedCopyAsync operations |
src/OrasProject.Oras/ReadOnlyGraphTargetExtensions.cs |
New extension method ExtendedCopyAsync that resolves source reference, copies the DAG, and tags the destination |
src/OrasProject.Oras/Content/MemoryStore.cs |
Updated to implement IReadOnlyGraphTarget instead of just IReadOnlyGraphStorage |
tests/OrasProject.Oras.Tests/ExtendedCopyTest.cs |
Added three tests covering happy path, null parameter validation, and empty destination reference handling |
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
| /// <exception cref="ArgumentNullException">Thrown when src or dst is null</exception> | ||
| public static async Task<Descriptor> ExtendedCopyAsync( | ||
| this IReadOnlyGraphTarget src, | ||
| string srcRef, |
There was a problem hiding this comment.
Should we also add a null check for srcRef and opts?
There was a problem hiding this comment.
Added these null checks
|
|
||
| await src.ExtendedCopyGraphAsync(dst, node, opts, cancellationToken).ConfigureAwait(false); | ||
|
|
||
| await dst.TagAsync(node, dstRef, cancellationToken).ConfigureAwait(false); |
There was a problem hiding this comment.
Just wondering what should be the expected behavior if the graph copy succeeds but tagging fails?
There was a problem hiding this comment.
TagAsync will throw an exception and ExtendedCopy would fail.
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
| namespace OrasProject.Oras.Content; | ||
|
|
||
| public class MemoryStore : ITarget, IReadOnlyGraphStorage | ||
| public class MemoryStore : ITarget, IReadOnlyGraphTarget |
There was a problem hiding this comment.
qq: What's the intended usage for ExtendedCopyAsync ? Currently only MemoryStore implements IReadOnlyGraphTarget. The Repository class implements IRepository: ITarget but not IReadOnlyGraphTarget, which means
var repo = new Repository(...);
await repo.ExtendedCopyAsync(...);
would not work
There was a problem hiding this comment.
That's a good question. By design, ExtendedCopy copies the artifact and any predecessor artifacts that reference it (common case is referrer artifact whose subject field is this artifact). Therefore ExtendedCopy can only work on a storage that supports finding the predecessors (implement the IPredecessorFindable interface). The Repository class currently does not implement the IPredecessorFindable interface and ExtendedCopy would not work on it. But it really should, we can open a feature request issue and complete it in the future. The implementation would be straightforward, just wrap FetchReferrersAsync which is already available.
This is the Go implementation:
// Predecessors returns the descriptors of image or artifact manifests directly
// referencing the given manifest descriptor.
// Predecessors internally leverages Referrers.
// Reference: https://github.com/opencontainers/distribution-spec/blob/v1.1.1/spec.md#listing-referrers
func (r *Repository) Predecessors(ctx context.Context, desc ocispec.Descriptor) ([]ocispec.Descriptor, error) {
var res []ocispec.Descriptor
if err := r.Referrers(ctx, desc, "", func(referrers []ocispec.Descriptor) error {
res = append(res, referrers...)
return nil
}); err != nil {
return nil, err
}
return res, nil
}| namespace OrasProject.Oras; | ||
|
|
||
| /// <summary> | ||
| /// ReadOnlyGraphTarget represents a read-only GraphTarget. |
There was a problem hiding this comment.
The XML doc summary refers to "ReadOnlyGraphTarget" but the public interface name is IReadOnlyGraphTarget (consistent with IReadOnlyTarget). Please update the summary text (and any <see cref> references if added) to use the correct interface name to avoid confusion in generated docs.
| /// ReadOnlyGraphTarget represents a read-only GraphTarget. | |
| /// IReadOnlyGraphTarget represents a read-only graph target. |
| } | ||
|
|
||
| /// <summary> | ||
| /// ExtendedCopyAsync throws when source target, destination target, source target reference or ExtendedCopyOption is null. |
There was a problem hiding this comment.
This test XML doc says "ExtendedCopyOption" but the type is ExtendedCopyOptions (plural). Please correct the wording to match the actual type name.
| /// ExtendedCopyAsync throws when source target, destination target, source target reference or ExtendedCopyOption is null. | |
| /// ExtendedCopyAsync throws when source target, destination target, source target reference or | |
| /// ExtendedCopyOptions is null. |
| var node = await src.ResolveAsync(srcRef, cancellationToken).ConfigureAwait(false); | ||
|
|
||
| await src.ExtendedCopyGraphAsync(dst, node, opts, cancellationToken).ConfigureAwait(false); | ||
|
|
||
| await dst.TagAsync(node, dstRef, cancellationToken).ConfigureAwait(false); |
There was a problem hiding this comment.
ExtendedCopyAsync resolves the source reference via ResolveAsync and then the copy path will fetch the same root content again during graph copy. For remote repositories, ResolveAsync is a HEAD request (see ManifestStore.ResolveAsync), so this introduces an avoidable extra network round-trip. Consider adding an optimization similar to ReadOnlyTargetExtensions.ResolveRootAsync (use IReferenceFetchable/ReferenceProxy) so the first fetch both resolves and populates the cache for the subsequent copy.
What this PR does / why we need it
This PR implements ExtendedCopy and ExtendedCopyOptions for oras-dotnet.
Which issue(s) this PR resolves / fixes
Resolves / Fixes #155
Please check the following list