Azure DevOps Artifacts Nuget Feed Badge, run [nuget myget chocolatey resharper]

[devhawk]

@calebcartwright marked #4182 as a good first issue so I decided to give it a shot.

With this shield, you can get the same kind of shield that you get from the existing nuget shield, but this one uses Azure Artifacts as a back end instead of nuget.org. The azure artifacts shield requires more information than the traditional nuget shield.

An example shield path would be /azure-devops/vpre/dotnet/RX.Net/RxNet/System.Interactive.

Azure Artifacts feeds support multiple views. You can specify a specific view as an optional parameter in the shield path right before the package name. For example, /azure-devops/vpre/dotnet/RX.Net/RxNet/release/System.Interactive

I felt that the way that the nuget shield is currently implemented is kind wonky with the withTenant and withFeed options. I didn't think that approach would scale to multiple different kinds of nuget feeds. So I factored the fetch logic into a separate "nuget-v3-helpers" file. This way, different nuget package feeds (nuget.org, myget, azure devops artifacts, github package registry, sleet, etc) could share the core nuget feed search logic while each one can have it's own custom route configuration to generate the base nuget feed URL.

I did modify the package fetch logic a bit. I was hitting an issue where the package query returned more than one package. For example, searching for Some.Package will return Some.Package and Some.Package.MoreDetails. So I modified the logic to explicitly check package name. I also added a getLatestVersion helper function that explicitly sorts by decending semver + does the prerelease filtering.

I'm creating a draft pull request because I still need to go back and update the existing nuget and myget shields to use this new approach. But I wanted feedback that I was on the right path before doing that work.

[shields-ci]

	Warnings
⚠️	This PR modified service code for azure-devops but not its test code. That's okay so long as it's refactoring existing code.

	Messages
📖	✨ Thanks for your contribution to Shields, @devhawk!

Generated by 🚫 dangerJS against 79453a9

[calebcartwright]

Hi @devhawk thanks for the PR!

I still think there's a need to investigate whether the Azure DevOps REST APIs for Artifacts (like this) can be leveraged directly, allowing Shields to provide AzDO Artifact badges for all package types (not just nuget) with a single implementation.

The other thing we have to keep in mind both with AzDO and other providers is support of private projects/feeds/etc. Shields already supports providing various AzDO badges for content from private team projects (test results, code coverage, and build/release status), and we'd naturally want to be able to do the same for artifact-related badges.

This way, different nuget package feeds (nuget.org, myget, azure devops artifacts, github package registry, sleet, etc) could share the core nuget feed search logic while each one can have it's own custom route configuration to generate the base nuget feed URL

I'm not familiar with sleet, but there are some relevant service/platform specific contexts to keep in mind. For example, we've actually already got an implementation for GitHub Package Registry (like #4184) that supports badges for all package types, and handles the authentication, token pooling for rate limiting mitigation, support for private artifacts, etc. that are associated with GitHub. It does so via the native GitHub v4 APIs.

I'm also not sure I follow what you had in mind regarding sharing the core nuget feed search logic, could you elaborate? The core code dealing directly with nuget feeds is already pretty well encapsulated and reused IMO, so the various service classes that are dealing directly with nuget feeds are really lightweight (mostly just example listings that display on the front end)

https://github.com/badges/shields/blob/master/services/nuget/nuget.service.js
https://github.com/badges/shields/blob/master/services/myget/myget.service.js
https://github.com/badges/shields/blob/master/services/chocolatey/chocolatey.service.js
https://github.com/badges/shields/blob/master/services/resharper/resharper.service.js

[devhawk]

I took a cursory look at the azure artifact REST api using insomnia - that approach should would work. Probably need to use the Get packages endpoint because Get Pacakge needs the package to be specified by GUID ID which doesn't seem to be easily discoverable (it's buried in a huge pile of JSON on the azure artifacts package web page).

[calebcartwright]

I took a cursory look at the azure artifact REST api using insomnia - that approach should would work. Probably need to use the Get packages endpoint because Get Pacakge needs the package to be specified by GUID ID which doesn't seem to be easily discoverable (it's buried in a huge pile of JSON on the azure artifacts package web page).

Nice! And yeah if there's an API available that allows Shields to accept more user-friendly params that's always worthwhile 👍

[devhawk]

What I meant by sharing the core nuget feed search logic was that - at least for the nuget/myget v3 services - I didn't think the nuget query logic was well factored. Because there is a single base NugetVersionService used for both nuget and myget, there's a bunch of switching logic in buildRoute that feels like it should be in sub classes rather than in the base class.

If you look at this PR, you'll see that I factored the fetch logic out so that it could be shared by multiple different nuget based shields. Each shield would be responsible for it's own route, using the route information to construct a nuget base URL which it then passes to the fetch logic.

Clearly, the REST/universal approach is the right choice for this service. And the other nuget feeds all seem to have a simple custom base URL except for myget so I wouldn't think it would be worth refactoring the pure nuget feed approach.

[devhawk]

Sleet is a static nuget feed generator. Like Jeckyll or Gatsby but for nuget feeds.

[calebcartwright]

What I meant by sharing the core nuget feed search logic was that - at least for the nuget/myget v3 services - I didn't think the nuget query logic was well factored. Because there is a single base NugetVersionService used for both nuget and myget, there's a bunch of switching logic in buildRoute that feels like it should be in sub classes rather than in the base class.

Thanks for elaborating. I've not worked with any of the nuget-related Shields code myself, but I believe that encapsulation and generalization was done deliberately to make it really simple to provide the range of package-related badges for any additional platforms/services that we'd have to integrate with at the feed level (it's not just for nuget.org/myget).

That's exposed via the service family generator functions and helps avoid a lot of the duplicative code in the various service classes that was repeated in each of the service classes, for each of the platforms previously. There was a discussion at one point about extending those utilities to also support dynamic/arbitrary feed urls (or really any feed url that's not known ahead of time and able to be baked in) which is still an outstanding TODO

We're always open to improvements though so if you feel it could be better and are willing, feel free to open a new PR and we'll be happy to take a look and discuss!

[paulmelnikow]

Hey @devhawk, I wrote what's there, and while I have opinions on things 😃 I am by no means attached to the way this is written now.

A point where Caleb and I agree is that helper functions + composition are often clearer ways of decomposing services, preferable to complicated inheritance hierarchies. It seems like you're largely on the same page with us there.

I'm happy for you to take a shot at revisiting and refactoring these, especially if there are ways to make things work better by breaking out some helper functions. Moving some of the feed URL generation responsibilities to the service class seems fine. We probably don't want to repeat a bunch of code, but it's also possible to replace the create... functions with a small amount of boilerplate that delegates to a "build route" function or a feed URL generation function that uses the withTenant and withFeed abstractions. Also, if these don't make sense at all let's toss 'em out the window, and replace with an abstraction that better fits the way we think this rather large family of services will grow going forward.

For what it's worth, I have never used any of these services, so my rewrite came largely from the existing implementations – plus one of the services had just migrated from v2 to v3. It was not with any deep understanding of NuGet or what might be possible or desirable. They've been pretty stable since then but that's not signal that they're factored correctly, as much as signal that they have good tests. It's our service model rewrite (#963) which prompted that.

By all means, if you are interested in continuing to work on, or refactor these, we'd both welcome you to do that. 😀

[devhawk]

A point where Caleb and I agree is that helper functions + composition are often clearer ways of decomposing services, preferable to complicated inheritance hierarchies. It seems like you're largely on the same page with us there.

That's a much better explanation of what I was trying to say! Thanks @paulmelnikow.

My primary interest is in getting an azure artifacts shield. Not sure when I will have time again to work on it - today was kind of a fluke - but when I do I'll also take a stab at reworking the nuget shields - the v3 ones anyway since it's only the myget shield that requires anything more complex then a custom hard-coded base url