MBS-13943: Lookup multiple URLs at once with the API #3483
Conversation
|
Wow, that was fast! Now I feel dumb for not having already requested that a few years ago 😂 I don't recall any precedent for a query parameter which can be specified multiple times. |
Yeah, there's no precedent for it, but the only way I can see to allow joining multiple of them in the same field would be to require users to double-encode spaces in the URL, which is...weird. Specifying them as separate parameters seems a bit more elegant. The URL "browse" endpoint is already very weird, since it's not actually a browse endpoint. It meets neither of the criteria that all other browse endpoints share:
Specifying multiple |
Multiple `resource` query parameters (up to 100) can now be specified: metabrainz/musicbrainz-server#3483 Also rename the method since this closer to a "lookup" than to a "browse" request, although https://musicbrainz.org/doc/MusicBrainz_API currently lists it as a "browse" request.
Multiple `resource` query parameters (up to 100) can now be specified: metabrainz/musicbrainz-server#3483 Also rename the method since this closer to a "lookup" than to a "browse" request, although https://musicbrainz.org/doc/MusicBrainz_API currently lists it as a "browse" request.
|
I agree, the /ws/2/url endpoint is (or was before this PR at least) much more similar to a lookup endpoint. Funnily enough, it accepts the paging parameters P.S. Might be a bit soon, but I've already added support for this in my MB client 😁 |
Relationships that are equal according to all other criteria currently in `Data::Relationship::_load` (including target entity name) are now sorted by target entity ID and relationship ID. This should fix some random test failures that occur depending on which system they are executed on.
There's no reason that this should return a list containing one URL. I kept a bare `return;` at the end so that it still works properly in list context.
This just allows specifying multiple `resource` query parameters on the existing /ws/2/url endpoint. It returns a `url-list` in that case, rather than a single URL.
Direct lookups of URLs via the `resource` parameter cannot be considered a "browse" query, as they don't find URLs linked via another entity type. The `url.url` column has a UNIQUE index; it's just as much of a lookup as using the MBID. This commit doesn't change any behavior of the /ws/2/url endpoint, but clarifies the code a bit more. Note that tests for `resource` lookups were already contained in files named LookupURL.pm.
Right,
I'll echo what you said earlier: That was fast! Nice work!
Only I also pushed a commit here to get rid of some "browse" language from the endpoint in MBS. |
There was a problem hiding this comment.
This seems good, but it seems https://wiki.musicbrainz.org/MusicBrainz_API needs to be updated too to document the new options ;) (url is listed there as being browsed by resource too)
| if (defined $row) { | ||
| return $self->_new_from_row($row); | ||
| } |
There was a problem hiding this comment.
I'm going to be annoying, but if find_by_url always returns one result, shouldn't it also be renamed to get_by_url? 😝 find_by_ methods seem to mostly return lists.
Problem
https://tickets.metabrainz.org/browse/MBS-13943
Solution
This just allows specifying multiple
resourcequery parameters on the existing /ws/2/url endpoint. It returns aurl-listin that case, rather than a single URL.Testing
Added some automated test cases.