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
metabrainz · Oct 29, 2024 · 6d5cb47 · 6d5cb47
1 parent cfa2ee4
commit 6d5cb47
Show file tree

Hide file tree

Showing 6 changed files with 174 additions and 17 deletions.
diff --git a/lib/MusicBrainz/Server/Controller/WS/2/Genre.pm b/lib/MusicBrainz/Server/Controller/WS/2/Genre.pm
@@ -7,13 +7,19 @@ 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 => {
         method   => 'GET',
         # 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
@@ -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' =>
+    '<?xml version="1.0"?>
+<metadata xmlns="http://musicbrainz.org/ns/mmd-2.0#">
+    <genre-list count="1">
+        <genre id="51cfaac4-6696-480b-8f1b-27cfc789109c">
+            <name>grime</name>
+            <disambiguation>stuff</disambiguation>
+        </genre>
+    </genre-list>
+</metadata>';
+
+ws2_test_xml 'browse genres via private collection',
+    '/genre?collection=7749c811-d77c-4ea5-9a9e-e2a4e7ae0d1b' =>
+    '<?xml version="1.0"?>
+<metadata xmlns="http://musicbrainz.org/ns/mmd-2.0#">
+    <genre-list count="1">
+        <genre id="51cfaac4-6696-480b-8f1b-27cfc789109c">
+            <name>grime</name>
+            <disambiguation>stuff</disambiguation>
+        </genre>
+    </genre-list>
+</metadata>',
+    { 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
@@ -44,7 +44,7 @@ test 'collection lookup' => sub {
     <collection entity-type="genre" type="Genre" id="7749c811-d77c-4ea5-9a9e-e2a4e7ae0d1b">
       <name>private genre collection</name>
       <editor>the-anti-kuno</editor>
-      <genre-list count="0" />
+      <genre-list count="1" />
     </collection>
     <collection entity-type="instrument" type="Instrument" id="cdef54c4-2798-4d39-a0c9-5074191f9b6e">
       <name>private instrument collection</name>
@@ -104,7 +104,7 @@ test 'collection lookup' => sub {
     <collection entity-type="genre" type="Genre" id="7749c811-d77c-4ea5-9a9e-e2a4e7ae0d1a">
       <name>public genre collection</name>
       <editor>the-anti-kuno</editor>
-      <genre-list count="0" />
+      <genre-list count="1" />
     </collection>
     <collection id="7749c811-d77c-4ea5-9a9e-e2a4e7ae0d1f" type="Instrument" entity-type="instrument">
       <name>public instrument collection</name>
@@ -230,7 +230,7 @@ test 'collection lookup' => sub {
     <collection entity-type="genre" type="Genre" id="7749c811-d77c-4ea5-9a9e-e2a4e7ae0d1a">
       <name>public genre collection</name>
       <editor>the-anti-kuno</editor>
-      <genre-list count="0" />
+      <genre-list count="1" />
     </collection>
     <collection id="7749c811-d77c-4ea5-9a9e-e2a4e7ae0d1f" type="Instrument" entity-type="instrument">
       <name>public instrument collection</name>

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
@@ -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
@@ -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
@@ -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);