Adds support for NuGet V2 upstream package sources

[patriksvensson]

This PR adds support for NuGet V2 upstream package sources.

I opted for not rewriting the NuGetClient to use the official API, and instead, I'm just wrapping the existing NuGetClient in an adapter that implements IMirrorNuGetClient.

To configure the mirror service to use the NuGet V2 client:

"Mirror": {
  "Enabled": true,
  "PackageSource": "https://www.nuget.org/api/v2/",
  "Legacy": true
},

[loic-sharma]

Hey this is a tiny bit of feature creep, but I think you can leverage the options validation here by adding attributes on the PackageSource property:

/// <summary>
/// The v3 index that will be mirrored.
/// </summary>
[RequiredIf(nameof(Enabled))]
[RequiredIf(nameof(Legacy))]
public Uri PackageSource { get; set; }

[patriksvensson]

Excellent idea. However when trying this it complains that only one IfRequired attribute can be used.

As I workaround, we can do the following, that will work but will show the error message Invalid 'Mirror' configs: 'PackageSource' is required because 'EnabledOrLegacy' has a value of 'True'. which does not reflect the settings properly, but maybe this is OK?

/// <summary>
/// If true, packages that aren't found locally will be indexed
/// using the upstream source.
/// </summary>
public bool Enabled { get; set; }

/// <summary>
/// The v3 index that will be mirrored.
/// </summary>
[RequiredIf(nameof(EnabledOrLegacy), true)]
public Uri PackageSource { get; set; }

/// <summary>
/// Whether or not the package source is a v2 package source feed.
/// </summary>
public bool Legacy { get; set; }

[NotMapped]
public bool EnabledOrLegacy => Enabled || Legacy;

[loic-sharma]

Ah good catch! It looks like you can support multiple attribute usages if we:

1. Set the attribute's AttributeUsageAttribute.AllowMultiple to true
2. Override the TypeId property (see this)

For an example of a validation attribute that supports multiple uses, see CustomValidationAttribute.

I'll do this in a follow-up PR since this is minor and I've blocked your changes for a little too long already!

[loic-sharma]

Ideally this should be done at startup by ASP.NET Core's options validation. See my comment up above: https://github.com/loic-sharma/BaGet/pull/630/files#r582447197

If that idea works, you should be able to remove this check.

[loic-sharma]

This is amazing. I really like your IMirrorNuGetClient abstraction, good idea there! This change will make it much easier to add support for private upstreams in the future too.

I left some minor comments, I'll merge this in once you've addressed them. Thanks!

P.S. Did you have any pain points when contributing to BaGet? Let me know if you have any feedback, would love to improve the experience however I can :)

[patriksvensson]

@loic-sharma I've fixed all points except the validation part (see my comment). I'm happy to implement the changes if you want to. Btw, do you want me to squash the commits?

P.S. Did you have any pain points when contributing to BaGet? Let me know if you have any feedback, would love to improve the experience however I can :)

No pain points! I found the code base intuitive and easy to follow. Of course, there are aspects of how NuGet works that I'm not familiar with, but I think it's unavoidable to have to learn that.

One thing that I was a bit confused about was a test (RegistrationBuilderTests.TheRegistrationIndexResponseIsSortedByVersion) that fails consistently for me but the CI checks seem to be green. Not sure if this is something culture-related since I'm using a computer with Swedish regional settings.

[loic-sharma]

Thanks for the contribution, this is awesome! I'll work on publishing a new version of BaGet.

I've fixed all points except the validation part (see my comment). I'm happy to implement the changes if you want to.

That's alright, I'm very happy with your implementation already. I'd rather release the feature now, we can save incremental improvements for later :)

Btw, do you want me to squash the commits?

Feel free to do whatever works best for you! I squash the commits at the very end when I merge.

One thing that I was a bit confused about was a test (RegistrationBuilderTests.TheRegistrationIndexResponseIsSortedByVersion) that fails consistently for me

Thanks for the heads up, I'll take a look at that!