From 6d5cb472edcfdeb6bdd168687aa2e229b6d49179 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Tamargo?= Date: Mon, 28 Oct 2024 16:53:45 +0100 Subject: [PATCH] MBS-13781: Support browsing genres by collection in the API We added the feature in the last schema change but I completely forgot the WS side of it. This implements it, plus some tests. I had to move genre_all to be the last one defined so that Catalyst would choose it first and avoid breaking genre lists, apparently; see https://metacpan.org/dist/Catalyst-Runtime/view/lib/Catalyst/RouteMatching.pod --- .../Server/Controller/WS/2/Genre.pm | 52 ++++++++++---- .../Server/Controller/WS/2/BrowseGenres.pm | 69 +++++++++++++++++++ .../Server/Controller/WS/2/Collection.pm | 6 +- .../Controller/WS/2/JSON/BrowseGenres.pm | 58 ++++++++++++++++ .../Server/Controller/WS/2/JSON/Collection.pm | 4 +- t/sql/webservice.sql | 2 + 6 files changed, 174 insertions(+), 17 deletions(-) create mode 100644 t/lib/t/MusicBrainz/Server/Controller/WS/2/BrowseGenres.pm create mode 100644 t/lib/t/MusicBrainz/Server/Controller/WS/2/JSON/BrowseGenres.pm diff --git a/lib/MusicBrainz/Server/Controller/WS/2/Genre.pm b/lib/MusicBrainz/Server/Controller/WS/2/Genre.pm index 6be893a65ad..594557530ae 100644 --- a/lib/MusicBrainz/Server/Controller/WS/2/Genre.pm +++ b/lib/MusicBrainz/Server/Controller/WS/2/Genre.pm @@ -7,6 +7,7 @@ extends 'MusicBrainz::Server::ControllerBase::WS::2'; use MusicBrainz::Server::WebService::TXTSerializer; use aliased 'MusicBrainz::Server::WebService::WebServiceStash'; +use MusicBrainz::Server::Validation qw( is_guid ); my $ws_defs = Data::OptList::mkopt([ genre => { @@ -14,6 +15,11 @@ my $ws_defs = Data::OptList::mkopt([ # TODO: Add include when implementing MBS-10062 # inc => [ qw(aliases) ], optional => [ qw(fmt limit offset) ], + linked => [ qw( collection ) ], + }, + genre => { + method => 'GET', + optional => [ qw(fmt limit offset) ], }, genre => { action => '/ws/2/genre/lookup', @@ -30,6 +36,8 @@ with 'MusicBrainz::Server::Controller::WS::2::Role::Lookup' => { model => 'Genre', }; +with 'MusicBrainz::Server::Controller::WS::2::Role::BrowseByCollection'; + sub serializers { [ 'MusicBrainz::Server::WebService::XMLSerializer', @@ -43,6 +51,38 @@ sub base : Chained('root') PathPart('genre') CaptureArgs(0) { } # Nothing extra to load yet, but this is required by Role::Lookup sub genre_toplevel {} +sub genre_browse : Private { + my ($self, $c) = @_; + + my ($resource, $id) = @{ $c->stash->{linked} }; + my ($limit, $offset) = $self->_limit_and_offset($c); + + if (!is_guid($id)) { + $c->stash->{error} = 'Invalid mbid.'; + $c->detach('bad_req'); + } + + my $genres; + + if ($resource eq 'collection') { + $genres = $self->browse_by_collection($c, 'genre', $id, $limit, $offset); + } + + my $stash = WebServiceStash->new; + + $self->genre_toplevel($c, $stash, $genres->{items}); + + $c->res->content_type($c->stash->{serializer}->mime_type . '; charset=utf-8'); + $c->res->body($c->stash->{serializer}->serialize('genre-list', $genres, $c->stash->{inc}, $stash)); +} + +sub genre_search : Chained('root') PathPart('genre') Args(0) { + my ($self, $c) = @_; + + $c->detach('genre_browse') if $c->stash->{linked}; + $c->detach('not_implemented'); +} + sub genre_all : Chained('base') PathPart('all') { my ($self, $c) = @_; @@ -74,18 +114,6 @@ sub genre_all : Chained('base') PathPart('all') { )); } -sub genre_browse : Private { - my ($self, $c) = @_; - - $c->detach('not_implemented'); -} - -sub genre_search : Chained('root') PathPart('genre') Args(0) { - my ($self, $c) = @_; - - $c->detach('genre_browse') if $c->stash->{linked}; - $c->detach('not_implemented'); -} __PACKAGE__->meta->make_immutable; 1; diff --git a/t/lib/t/MusicBrainz/Server/Controller/WS/2/BrowseGenres.pm b/t/lib/t/MusicBrainz/Server/Controller/WS/2/BrowseGenres.pm new file mode 100644 index 00000000000..0507351ed1a --- /dev/null +++ b/t/lib/t/MusicBrainz/Server/Controller/WS/2/BrowseGenres.pm @@ -0,0 +1,69 @@ +package t::MusicBrainz::Server::Controller::WS::2::BrowseGenres; +use utf8; +use strict; +use warnings; + +use Test::Routine; +use Test::More; +use HTTP::Status qw( :constants ); + +with 't::Mechanize', 't::Context'; + +use MusicBrainz::Server::Test::WS qw( + ws2_test_xml + ws2_test_xml_forbidden + ws2_test_xml_unauthorized +); + +test all => sub { + +my $test = shift; +my $c = $test->c; + +MusicBrainz::Server::Test->prepare_test_database($c, '+webservice'); + +ws2_test_xml 'browse genres via public collection', + '/genre?collection=7749c811-d77c-4ea5-9a9e-e2a4e7ae0d1a' => + ' + + + + grime + stuff + + +'; + +ws2_test_xml 'browse genres via private collection', + '/genre?collection=7749c811-d77c-4ea5-9a9e-e2a4e7ae0d1b' => + ' + + + + grime + stuff + + +', + { username => 'the-anti-kuno', password => 'notreally' }; + +ws2_test_xml_forbidden 'browse genres via private collection, no credentials', + '/genre?collection=7749c811-d77c-4ea5-9a9e-e2a4e7ae0d1b'; + +ws2_test_xml_unauthorized 'browse genres via private collection, bad credentials', + '/genre?collection=7749c811-d77c-4ea5-9a9e-e2a4e7ae0d1b', + { username => 'the-anti-kuno', password => 'idk' }; + +my $res = $test->mech->get('/ws/2/genre?collection=3c37b9fa-a6c1-37d2-9e90-657a116d337c&limit=-1'); +is($res->code, HTTP_BAD_REQUEST); + +$res = $test->mech->get('/ws/2/genre?collection=3c37b9fa-a6c1-37d2-9e90-657a116d337c&offset=a+bit'); +is($res->code, HTTP_BAD_REQUEST); + +$res = $test->mech->get('/ws/2/genre?collection=3c37b9fa-a6c1-37d2-9e90-657a116d337c&limit=10&offset=-1'); +is($res->code, HTTP_BAD_REQUEST); + +}; + +1; + diff --git a/t/lib/t/MusicBrainz/Server/Controller/WS/2/Collection.pm b/t/lib/t/MusicBrainz/Server/Controller/WS/2/Collection.pm index 8959d15d2be..c8d6ca6b7cd 100644 --- a/t/lib/t/MusicBrainz/Server/Controller/WS/2/Collection.pm +++ b/t/lib/t/MusicBrainz/Server/Controller/WS/2/Collection.pm @@ -44,7 +44,7 @@ test 'collection lookup' => sub { private genre collection the-anti-kuno - + private instrument collection @@ -104,7 +104,7 @@ test 'collection lookup' => sub { public genre collection the-anti-kuno - + public instrument collection @@ -230,7 +230,7 @@ test 'collection lookup' => sub { public genre collection the-anti-kuno - + public instrument collection diff --git a/t/lib/t/MusicBrainz/Server/Controller/WS/2/JSON/BrowseGenres.pm b/t/lib/t/MusicBrainz/Server/Controller/WS/2/JSON/BrowseGenres.pm new file mode 100644 index 00000000000..6fa21c1d298 --- /dev/null +++ b/t/lib/t/MusicBrainz/Server/Controller/WS/2/JSON/BrowseGenres.pm @@ -0,0 +1,58 @@ +package t::MusicBrainz::Server::Controller::WS::2::JSON::BrowseGenres; +use utf8; +use strict; +use warnings; + +use Test::Routine; +use MusicBrainz::Server::Test ws_test_json => { + version => 2, +}; +use MusicBrainz::Server::Test::WS qw( + ws2_test_json_forbidden + ws2_test_json_unauthorized +); + +with 't::Mechanize', 't::Context'; + +test 'browse genres via collection' => sub { + + MusicBrainz::Server::Test->prepare_test_database(shift->c, '+webservice'); + + ws_test_json 'browse genres via public collection', + '/genre?collection=7749c811-d77c-4ea5-9a9e-e2a4e7ae0d1a' => + { + 'genre-offset' => 0, + 'genre-count' => 1, + genres => [ + { + id => '51cfaac4-6696-480b-8f1b-27cfc789109c', + name => 'grime', + disambiguation => 'stuff', + }], + }; + + ws_test_json 'browse genres via private collection', + '/genre?collection=7749c811-d77c-4ea5-9a9e-e2a4e7ae0d1b' => + { + 'genre-offset' => 0, + 'genre-count' => 1, + genres => [ + { + id => '51cfaac4-6696-480b-8f1b-27cfc789109c', + name => 'grime', + disambiguation => 'stuff', + }], + }, + { username => 'the-anti-kuno', password => 'notreally' }; + + + ws2_test_json_forbidden 'browse genres via private collection, no credentials', + '/genre?collection=7749c811-d77c-4ea5-9a9e-e2a4e7ae0d1b'; + + ws2_test_json_unauthorized 'browse genres via private collection, bad credentials', + '/genre?collection=7749c811-d77c-4ea5-9a9e-e2a4e7ae0d1b', + { username => 'the-anti-kuno', password => 'idk' }; + +}; + +1; diff --git a/t/lib/t/MusicBrainz/Server/Controller/WS/2/JSON/Collection.pm b/t/lib/t/MusicBrainz/Server/Controller/WS/2/JSON/Collection.pm index 8c27186210f..423a912d8c0 100644 --- a/t/lib/t/MusicBrainz/Server/Controller/WS/2/JSON/Collection.pm +++ b/t/lib/t/MusicBrainz/Server/Controller/WS/2/JSON/Collection.pm @@ -114,7 +114,7 @@ test 'collection lookup' => sub { 'type' => 'Genre', 'type-id' => '1210f9ce-e138-3b08-94b5-05d21032b696', 'editor' => 'the-anti-kuno', - 'genre-count' => '0', + 'genre-count' => '1', }, { 'id' => '7749c811-d77c-4ea5-9a9e-e2a4e7ae0d1b', @@ -123,7 +123,7 @@ test 'collection lookup' => sub { 'type' => 'Genre', 'type-id' => '1210f9ce-e138-3b08-94b5-05d21032b696', 'editor' => 'the-anti-kuno', - 'genre-count' => '0', + 'genre-count' => '1', }, { 'id' => '7749c811-d77c-4ea5-9a9e-e2a4e7ae0d1f', diff --git a/t/sql/webservice.sql b/t/sql/webservice.sql index 4429409b7eb..9b8c6dd1aa2 100644 --- a/t/sql/webservice.sql +++ b/t/sql/webservice.sql @@ -1635,6 +1635,8 @@ INSERT INTO editor_collection_artist (collection, artist) VALUES (3, 9496); INSERT INTO editor_collection_artist (collection, artist) VALUES (4, 135345); INSERT INTO editor_collection_event (collection, event) VALUES (5, 7); INSERT INTO editor_collection_event (collection, event) VALUES (6, 8); +INSERT INTO editor_collection_genre (collection, genre) VALUES (23, 3); +INSERT INTO editor_collection_genre (collection, genre) VALUES (24, 3); INSERT INTO editor_collection_instrument (collection, instrument) VALUES (7, 7); INSERT INTO editor_collection_instrument (collection, instrument) VALUES (8, 7); INSERT INTO editor_collection_label (collection, label) VALUES (9, 8092);