feat(copy): introduces CopyGraphOptions with events support

[leonardochaia]

What this PR does / why we need it

This PR improves CopyAsync and CopyGraphAsync to include the CopyGraphOptions, inspired from oras-go.

The current implementation:

1. adds support for CopyOptions events, like OnPreCopy, these were implemented in the form of Action<T>s

Pending items

• Define Copy signature

Which issue(s) this PR resolves / fixes

Please check the following list

• Does the affected code have corresponding tests, e.g. unit test, E2E test?
• Does this change require a documentation update?
• Does this introduce breaking changes that would require an announcement or bumping the major version?
• Do all new files have an appropriate license header?

[leonardochaia]

This is a work in progress. Seems to be working running locally. Tests are failing and need more work.

[leonardochaia]

Hi. I propose concurrency support is implemented in a separate PR. Otherwise this should be ready to review :+

Leo.

[leonardochaia]

Example usage:

List<string> knownReferences = [];
foreach (var toReplicate in toReplicateArray)
{
    var copyOpts = new CopyOptions()
    {
        MountFrom = _ => knownReferences.ToArray(),
    };

    copyOpts.OnCopySkipped += d => this.logger.LogTrace("{Digest}: Already exists.", d.Digest);
    copyOpts.OnPreCopy += d => this.logger.LogTrace("{Digest}: Copying..", d.Digest);
    copyOpts.OnPostCopy += d => this.logger.LogTrace("{Digest}: Copied", d.Digest);
    copyOpts.OnMounted += (d, from) => this.logger.LogTrace("{Digest}: Mounted from {From}", d.Digest, from);

    await sourceRepository.CopyAsync(
        toReplicate.SourceImage,
        destinationRepository,
        toReplicate.DestinationImage,
        cancellationToken,
        copyOpts);

    knownReferences.Add(toReplicate.DestinationImage);
}

[leonardochaia]

Hi all, any feedback is appreciated 🥇

Thanks!
Leo.

[codecov]

Codecov Report

Attention: Patch coverage is 93.54839% with 2 lines in your changes missing coverage. Please review.

Project coverage is 82.62%. Comparing base (3fa86d6) to head (827f62b).
Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
src/OrasProject.Oras/AsyncInvocationExtensions.cs	66.66%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #145      +/-   ##
==========================================
+ Coverage   81.70%   82.62%   +0.92%     
==========================================
  Files          35       37       +2     
  Lines        1104     1128      +24     
  Branches      128      134       +6     
==========================================
+ Hits          902      932      +30     
+ Misses        146      139       -7     
- Partials       56       57       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Wwwsylvia]

Hi @leonardochaia , thank you for your contribution! Sorry for the delay. We need a bit more time to review your PR, but we’ll provide feedback as soon as we can. Thank you for your understanding!

[Wwwsylvia]

Hi. I propose concurrency support is implemented in a separate PR. Otherwise this should be ready to review :+

Do we want to move the mounting support to a separate PR as well?

[leonardochaia]

Hi. I propose concurrency support is implemented in a separate PR. Otherwise this should be ready to review :+

Do we want to move the mounting support to a separate PR as well?

Hi @Wwwsylvia , thanks for the review. I'll apply the changes and report back.
We can move the mounting to a separate PR. In such case we would leave only the CopyOptions and related events on this PR, and implement the IMounter and mounting support on copy on a separate PR.

My goal is getting the mounting support when copying graphs since we are using this to replicate a bunch of images between registries and need to make it faster.

[Wwwsylvia]

@leonardochaia Sounds good. Thank you!

[leonardochaia]

PR simplified to only introduce the CopyGraphOptions. Next step is to update the tests to cover the events.

Separate PRs/issues to be created:

1. Copy to support concurrency
2. Repository to support mounting
3. Copy to support mounting

[leonardochaia]

Usually CancellationToken is left as the last parameter, however, that would introduce a breaking change. Are we good with introducing a breaking change?

public static async Task<Descriptor> CopyAsync(this ITarget src, string srcRef, ITarget dst, string dstRef, CopyGraphOptions? copyOptions = default, CancellationToken cancellationToken = default)
    
public static async Task CopyGraphAsync(this ITarget src, ITarget dst, Descriptor node, , CopyGraphOptions? copyOptions = default, CancellationToken cancellationToken)

EDIT: Disregard, I've added an overload to keep the CancellationToken last but also support the current signature

[leonardochaia]

Quick update, I've converted this PR to only include the CopyGraphOptions with events support and corresponding tests.

Next steps:

1. Add support for mounting to Repository feat(repository): mounting support #152
2. Add support for mounting to Copy (after this and feat(repository): mounting support #152 are merged)

[Wwwsylvia]

Quick update, I've converted this PR to only include the CopyGraphOptions with events support and corresponding tests.

Next steps:

Add support for mounting to Repository #152
Add support for mounting to Copy (after this and #152 are merged)

Thank you! I just created the following issues to track:

• Support cross-repository mounting of blobs #153
• Support copy options #154

[leonardochaia]

Build should be fixed, tests passed locally.

@Wwwsylvia thanks for creating the issues, JIC: only CopyGraphOptions was introduced in this PR. I propose we leave CopyOptions for a separate PR with the MountFrom feature, as per oras-go:

// CopyOptions contains parameters for [oras.Copy].
type CopyOptions struct {
	CopyGraphOptions
	// MapRoot maps the resolved root node to a desired root node for copy.
	// When MapRoot is provided, the descriptor resolved from the source
	// reference will be passed to MapRoot, and the mapped descriptor will be
	// used as the root node for copy.
	MapRoot func(ctx context.Context, src content.ReadOnlyStorage, root ocispec.Descriptor) (ocispec.Descriptor, error)
}

https://github.com/oras-project/oras-go/blob/bf570bcc78b3aa1bad84401e3390cc79a03e426e/copy.go#L50-L57

When CopyOptions is introduced, there should be no breaking change since it should inherit from CopyGraphOptions

Thanks!
Leo.

[Wwwsylvia]

@leonardochaia The MountFrom and OnMounted options are included in CopyGraphOptions (See https://github.com/oras-project/oras-go/blob/bf570bcc78b3aa1bad84401e3390cc79a03e426e/copy.go#L108-L113).

Since oras-dotnet is still under initial development, breaking changes are acceptable if they are necessary.

---

Update:

only CopyGraphOptions was introduced in this PR. I propose we leave CopyOptions for a separate PR with the MountFrom feature, as per oras-go:

Oh I might have misunderstood. I'm good with leaving CopyOptions in another PR.

[leonardochaia]

Hi @Wwwsylvia , sorry I meant to say MapRoot instead of MountFrom 👍

Thanks!

[leonardochaia]

This should be good to go.

Once this is merged I'll create a separate PR to support mounting when copying 🥇

[Wwwsylvia]

LGTM

[Wwwsylvia]

Do you mean InvokeAsync?

[Wwwsylvia]

In the PoC that plays with SomeEvent, we set up the tasks for the async events before invoking the sync event handler. While in this implementation we first invoke the sync event and then the async events. I think the PoC approach is more efficient. I'm wondering if we can achieve something similar with a reusable extension method.

How about something like this:

    internal static IEnumerable<Task> InvokeAsyncEvents<TEventArgs>(
        this Func<TEventArgs, Task>? eventDelegate, TEventArgs args)
    {
        if (eventDelegate == null)
        {
            return [Task.CompletedTask];
        }
        return eventDelegate.GetInvocationList()
            .Select(d => (Task?)d.DynamicInvoke(args) ?? Task.CompletedTask);
    }

    internal Task OnPreCopyAsync(Descriptor descriptor)
    {
        var tasks = PreCopyAsync.InvokeAsyncEvents(descriptor);
        PreCopy?.Invoke(descriptor);
        return Task.WhenAll(tasks);
    }

Naming is hard though...