Add a "minimal crate metadata" API endpoint

[nightkr]

As mentioned in #4503 and renovatebot/renovate#3486, this is intended to be a lighter-weight API than /crates/:crate_id, which can be subject to looser rate limits.

This adds a new /crates/:crate_id/crate endpoint for metadata about the crate itself, without information about related objects (such as versions or categories).

[nightkr]

7256d79 and 817f87a are tangential housekeeping tasks, feel free to merge without them (or I can split them into a separate PR, if you prefer that).

[nightkr]

I'm not quite sure about where the rate limits come from, I assume PublishRateLimit only covers cargo publish in particular. Is this managed in your hosting environment?

[lfrancke]

Thanks @teozkr

Yes, the rate limit is not enforced in code.

Let's see what others say, this adds an additional endpoint while the original compromise called for an additional parameter on the existing show endpoint.

[Turbo87]

I can split them into a separate PR, if you prefer that

that would be great, yeah 👍 ideally two separate PRs since the two changes are itself unrelated too :)

This adds a new /crates/:crate_id/crate endpoint for metadata about the crate itself, without information about related objects (such as versions or categories).

IMHO having two endpoints for the same resource makes it a bit confusing. was there a reason why you went that way instead of optimizing the existing endpoint?

could you also make sure to add a few tests for the new code? 🙏

[nightkr]

IMHO having two endpoints for the same resource makes it a bit confusing. was there a reason why you went that way instead of optimizing the existing endpoint?

To me, "overview of X and subresources" and "info about X" are two fairly separate code paths. A client would also want to have completely separate type definitions for them, for example.

There's also the server code structure issue, that doing it as a separate flag for show would end up cluttering show with a lot of branching, while also making it less clear what queries are actually performed for each mode (important both for validating this PR in particular, and to ensure that the basic mode doesn't regress with future features).

[nightkr]

Added a test based on the full metadata endpoint test. Ideally it'd validate that we only make one DB query, but it doesn't look like Diesel has a way to query for that, as far as I can tell.

[Turbo87]

To me, "overview of X and subresources" and "info about X" are two fairly separate code paths.

I tend to disagree. That endpoint is returning a resource and in the default case it currently includes a bunch of related resources (aka. relationships). The suggestion in the in linked issue was to make that automatic relationship inclusion opt-out. Having two different endpoint for the same resource may make it easier right now to implement things, but it also means we have to maintain two different endpoints that basically do the same thing. I'd prefer to be able to use something like https://jsonapi.org/format/#fetching-includes, even though we're currently not using the JSON:API schema for our responses.

[nightkr]

Okay, b9c7fff merges the show_minimal endpoint into show?include=, with the existing show endpoint becoming show?include=full (which show is an alias of, so that we don't break old clients).

28ead19 takes this a bit further, and breaks show?include=full down into components (so that you can request show?include=versions,categories, for example). show?include=full is still interpreted as "enable all components", ending up behaving the same as the current show.

[soenkeliebau]

I think I like the second approch to allow retrieving specific data.
Haven't really looked at the code, would what the uses request impact the number of database lookups needed, or is it pretty much always the same "weight" for a request?

[nightkr]

Haven't really looked at the code, would what the uses request impact the number of database lookups needed, or is it pretty much always the same "weight" for a request?

Yes, ~1 additional database query is performed per extra include that the user requests.

This already happened before this PR, the PR just makes those queries conditional.

[jtgeibel]

This looks good to me overall. Just one minor nit in the wording of an error message. Thanks!

[soenkeliebau]

This looks excellent, thanks!

[jtgeibel]

Thanks!

@bors r+

[bors]

📌 Commit fae83f1 has been approved by jtgeibel

[bors]

⌛ Testing commit fae83f1 with merge ae50554...

[bors]

☀️ Test successful - checks-actions
Approved by: jtgeibel
Pushing ae50554 to master...