Merge branch 'ws-json-fixes'

* ws-json-fixes: Only output existent iso code properties in /ws/2/area JSON Hyphenate iso_3166_x_codes in /ws/2/area JSON Remove most PUID support from /ws/2 Implement /ws/2/isrc/?fmt=json (MBS-7921) Distinguish non-existent and not-loaded relationships (MBS-8746) Output 'target-type' in /ws/2 relationship JSON (MBS-5676) Replace is_json with cmp_deeply inc=recordings should imply inc=media (MBS-8367) Output 'ordering-key' in /ws/2 relationship JSON (MBS-8396) Return 'video' as a boolean in /ws/2/recording JSON (MBS-7735) Force numeric context on recording length
metabrainz · Jan 20, 2016 · 2ad3fb2 · 2ad3fb2
2 parents 8123e28 + 55e4a6e
commit 2ad3fb2
Show file tree

Hide file tree

Showing 38 changed files with 626 additions and 506 deletions.
diff --git a/lib/MusicBrainz/Server/Controller/WS/2/PUID.pm b/lib/MusicBrainz/Server/Controller/WS/2/PUID.pm
diff --git a/lib/MusicBrainz/Server/Controller/WS/2/Release.pm b/lib/MusicBrainz/Server/Controller/WS/2/Release.pm
@@ -103,13 +103,7 @@ sub release_toplevel
 
     if ($c->stash->{inc}->recordings)
     {
-        my @mediums;
-        if (!$c->stash->{inc}->media)
-        {
-            $c->model('Medium')->load_for_releases($release);
-        }
-
-        @mediums = $release->all_mediums;
+        my @mediums = $release->all_mediums;
 
         if (!$c->stash->{inc}->discids)
         {

diff --git a/lib/MusicBrainz/Server/ControllerBase/WS/2.pm b/lib/MusicBrainz/Server/ControllerBase/WS/2.pm
@@ -421,7 +421,7 @@ sub linked_releases
     $c->model('Release')->load_release_events(@$releases);
 
     my @mediums;
-    if ($c->stash->{inc}->media)
+    if ($c->stash->{inc}->media || $c->stash->{inc}->recordings)
     {
         @mediums = map { $_->all_mediums } @$releases;
 

diff --git a/lib/MusicBrainz/Server/Data/Relationship.pm b/lib/MusicBrainz/Server/Data/Relationship.pm
@@ -188,6 +188,9 @@ sub _load
             }
         }
     }
+    for my $obj (@objs) {
+        $obj->has_loaded_relationships(1);
+    }
     return @rels;
 }
 

diff --git a/lib/MusicBrainz/Server/Entity/Role/Linkable.pm b/lib/MusicBrainz/Server/Entity/Role/Linkable.pm
@@ -18,6 +18,12 @@ has 'relationships' => (
     }
 );
 
+has has_loaded_relationships => (
+    is => 'rw',
+    isa => 'Bool',
+    default => 0,
+);
+
 sub grouped_relationships
 {
     my ($self, @types) = @_;
@@ -86,8 +92,7 @@ around TO_JSON => sub {
 
     my $json = $self->$orig;
 
-    # FIXME: Need a way to distinguish between relationships not being loaded vs. not existing.
-    if ($self->all_relationships) {
+    if ($self->has_loaded_relationships) {
         $json->{relationships} = [map { $_->TO_JSON } $self->all_relationships];
     }
 

diff --git a/lib/MusicBrainz/Server/Test.pm b/lib/MusicBrainz/Server/Test.pm
@@ -9,7 +9,7 @@ use FindBin '$Bin';
 use Getopt::Long;
 use HTTP::Headers;
 use HTTP::Request;
-use JSON qw( encode_json );
+use JSON qw( decode_json encode_json );
 use List::UtilsBy qw( nsort_by );
 use MusicBrainz::Server::CacheManager;
 use MusicBrainz::Server::Context;
@@ -20,8 +20,9 @@ use MusicBrainz::WWW::Mechanize;
 use Sql;
 use Template;
 use Test::Builder;
+use Test::Deep qw( cmp_deeply );
 use Test::Differences;
-use Test::JSON import => [qw( is_json is_valid_json )];
+use Test::JSON import => [qw( is_valid_json )];
 use Test::Mock::Class ':all';
 use Test::WWW::Mechanize::Catalyst;
 use Test::XML::SemanticCompare;
@@ -404,8 +405,7 @@ sub _build_ws_test_json {
             $mech->get_ok($end_point . $url, 'fetching');
             is_valid_json($mech->content, "validating (is_valid_json)");
 
-            is_json($mech->content, $expected);
-            $Test->note($mech->content);
+            cmp_deeply(decode_json($mech->content), $expected);
         });
     };
 }
@@ -486,7 +486,7 @@ sub page_test_jsonld {
     my $jsonld = encode('UTF-8', $tx->find_value('//script[@type="application/ld+json"]'));
 
     is_valid_json($jsonld, 'has valid JSON-LD');
-    is_json($jsonld, encode_json($expected), 'has expected JSON-LD');
+    cmp_deeply(decode_json($jsonld), $expected, 'has expected JSON-LD');
 }
 
 1;

diff --git a/lib/MusicBrainz/Server/WebService/Serializer/JSON/2/Area.pm b/lib/MusicBrainz/Server/WebService/Serializer/JSON/2/Area.pm
@@ -17,9 +17,9 @@ sub serialize
     $body{name} = $entity->name;
     $body{"sort-name"} = $entity->name;
     $body{disambiguation} = $entity->comment // "";
-    $body{iso_3166_1_codes} = $entity->iso_3166_1 ? [ map { $_ } @{ $entity->iso_3166_1 } ] : JSON::null;
-    $body{iso_3166_2_codes} = $entity->iso_3166_2 ? [ map { $_ } @{ $entity->iso_3166_2 } ] : JSON::null;
-    $body{iso_3166_3_codes} = $entity->iso_3166_3 ? [ map { $_ } @{ $entity->iso_3166_3 } ] : JSON::null;
+    $body{'iso-3166-1-codes'} = [$entity->iso_3166_1_codes] if $entity->iso_3166_1_codes;
+    $body{'iso-3166-2-codes'} = [$entity->iso_3166_2_codes] if $entity->iso_3166_2_codes;
+    $body{'iso-3166-3-codes'} = [$entity->iso_3166_3_codes] if $entity->iso_3166_3_codes;
 
     if ($toplevel)
     {

diff --git a/lib/MusicBrainz/Server/WebService/Serializer/JSON/2/ISRC.pm b/lib/MusicBrainz/Server/WebService/Serializer/JSON/2/ISRC.pm
@@ -0,0 +1,30 @@
+package MusicBrainz::Server::WebService::Serializer::JSON::2::ISRC;
+
+use Moose;
+use MusicBrainz::Server::WebService::Serializer::JSON::2::Utils qw( serialize_entity );
+
+extends 'MusicBrainz::Server::WebService::Serializer::JSON::2';
+
+sub serialize {
+    my ($self, $isrcs, $inc, $stash) = @_;
+
+    return {
+        isrc => $isrcs->[0]->name,
+        recordings => [map { serialize_entity($_->recording, $inc, $stash) } @$isrcs],
+    };
+};
+
+__PACKAGE__->meta->make_immutable;
+
+no Moose;
+
+1;
+
+=head1 COPYRIGHT
+
+This file is part of MusicBrainz, the open internet music database.
+Copyright (C) 2016 MetaBrainz Foundation
+Licensed under the GPL version 2, or (at your option) any later version:
+http://www.gnu.org/licenses/gpl-2.0.txt
+
+=cut
diff --git a/lib/MusicBrainz/Server/WebService/Serializer/JSON/2/Recording.pm b/lib/MusicBrainz/Server/WebService/Serializer/JSON/2/Recording.pm
@@ -1,6 +1,11 @@
 package MusicBrainz::Server::WebService::Serializer::JSON::2::Recording;
 use Moose;
-use MusicBrainz::Server::WebService::Serializer::JSON::2::Utils qw( serialize_entity list_of );
+use MusicBrainz::Server::WebService::Serializer::JSON::2::Utils qw(
+    boolean
+    number
+    serialize_entity
+    list_of
+);
 use List::UtilsBy 'sort_by';
 
 extends 'MusicBrainz::Server::WebService::Serializer::JSON::2';
@@ -18,8 +23,8 @@ sub serialize
 
     $body{title} = $entity->name;
     $body{disambiguation} = $entity->comment // "";
-    $body{length} = $entity->length // JSON::null;
-    $body{video} = $entity->video ? 1 : 0;
+    $body{length} = number($entity->length);
+    $body{video} = boolean($entity->video);
 
     $body{"artist-credit"} = serialize_entity($entity->artist_credit)
         if ($entity->artist_credit &&
@@ -28,13 +33,12 @@ sub serialize
     $body{releases} = list_of($entity, $inc, $stash, "releases")
         if ($toplevel && $inc && $inc->releases);
 
-    return \%body unless defined $inc && ($inc->isrcs || $inc->puids);
+    return \%body unless defined $inc && $inc->isrcs;
 
     my $opts = $stash->store($entity);
     $body{isrcs} = [
         map { $_->isrc } sort_by { $_->isrc } @{ $opts->{isrcs} }
-        ] if $inc->isrcs;
-    $body{puids} = [ ] if $inc->puids;
+    ] if $inc->isrcs;
 
     return \%body;
 };

diff --git a/lib/MusicBrainz/Server/WebService/Serializer/JSON/2/Relationship.pm b/lib/MusicBrainz/Server/WebService/Serializer/JSON/2/Relationship.pm
@@ -3,7 +3,11 @@ use Moose;
 use Hash::Merge qw( merge );
 use List::AllUtils qw( any );
 use MusicBrainz::Server::Data::Utils qw( non_empty );
-use MusicBrainz::Server::WebService::Serializer::JSON::2::Utils qw( date_period serialize_entity );
+use MusicBrainz::Server::WebService::Serializer::JSON::2::Utils qw(
+    date_period
+    number
+    serialize_entity
+);
 
 extends 'MusicBrainz::Server::WebService::Serializer::JSON::2';
 
@@ -18,6 +22,7 @@ sub serialize
     $body->{type} = $entity->link->type->name;
     $body->{"type-id"} = $entity->link->type->gid;
     $body->{direction} = $entity->direction == 2 ? "backward" : "forward";
+    $body->{'ordering-key'} = number($entity->link_order) if $entity->link_order;
 
     $body = merge($body, date_period($entity->link));
     $body->{attributes} = [ map { $_->type->name } @attributes ];
@@ -36,6 +41,7 @@ sub serialize
         @attributes
     } if any { $_->type->creditable } @attributes;
 
+    $body->{'target-type'} = $entity->target_type;
     $body->{$entity->target_type} = serialize_entity($entity->target, $inc, $opts);
     $body->{'source-credit'} = $entity->source_credit // '';
     $body->{'target-credit'} = $entity->target_credit // '';

diff --git a/lib/MusicBrainz/Server/WebService/Serializer/JSON/2/Role/Relationships.pm b/lib/MusicBrainz/Server/WebService/Serializer/JSON/2/Role/Relationships.pm
@@ -11,11 +11,17 @@ around serialize => sub
     my ($orig, $self, $entity, $inc, $opts, $toplevel) = @_;
     my $ret = $self->$orig($entity, $inc, $opts, $toplevel);
 
-    return $ret unless defined $inc && $inc->has_rels;
+    return $ret unless
+        defined $inc &&
+        $inc->has_rels &&
+        $entity->has_loaded_relationships;
 
     my @rels = map { serialize_entity($_, $inc, $opts) }
-        sort_by { $_->target_key . $_->link->type->name }
-            @{ $entity->relationships };
+        sort_by { join("\t",
+                       $_->link->type->name,
+                       (sprintf "%09d", $_->link_order // 0),
+                       $_->target_key)
+                } $entity->all_relationships;
 
     $ret->{relations} = \@rels;
 

diff --git a/lib/MusicBrainz/Server/WebService/Serializer/JSON/2/Utils.pm b/lib/MusicBrainz/Server/WebService/Serializer/JSON/2/Utils.pm
@@ -31,6 +31,7 @@ my %serializers =
         Collection
         Event
         Instrument
+        ISRC
         Label
         Place
         Medium
@@ -71,6 +72,10 @@ sub serializer
 
     my $serializer;
 
+    if (ref $entity eq 'ARRAY') {
+        $entity = $entity->[0];
+    }
+
     for my $class (keys %serializers) {
         if ($entity->isa($class)) {
             return $serializers{$class};

diff --git a/root/static/scripts/release-editor/duplicates.js b/root/static/scripts/release-editor/duplicates.js
@@ -132,15 +132,9 @@ const request = require('../common/utility/request');
 
         clean.dates = pluck(events, "date").value();
 
-        // XXX annoying inconsistency!!!
-        // browse requests return iso_3166_1_codes, the search server returns
-        // iso-3166-1-codes.
-
-        var areas = pluck(events, "area");
-
-        clean.countries = areas.pluck("iso-3166-1-codes")
-                               .concat(areas.pluck("iso_3166_1_codes").value())
-                               .flatten().compact().uniq().value();
+        clean.countries = pluck(events, "area")
+            .pluck("iso-3166-1-codes")
+            .flatten().compact().uniq().value();
 
         clean.labels = pluck(labels, "label").map(function (info) {
             return MB.entity.Label({ gid: info.id, name: info.name });

diff --git a/script/dump-entities-sql.pl b/script/dump-entities-sql.pl
@@ -164,6 +164,7 @@ sub relationships {
         my $link_ids = pluck('link', $results);
         print_rows($c, 'link', 'id', $link_ids);
         print_rows($c, 'link_attribute', 'link', $link_ids);
+        print_rows($c, 'link_attribute_text_value', 'link', $link_ids);
 
         print_inserts($c, $table, $results);
     }

diff --git a/t/lib/t/MusicBrainz/Server/Controller/UnconfirmedEmailAddresses.pm b/t/lib/t/MusicBrainz/Server/Controller/UnconfirmedEmailAddresses.pm
@@ -438,7 +438,6 @@ test 'Paths that allow browsing without a confirmed email address' => sub {
   "Controller::WS::2::Label::base",
   "Controller::WS::2::Label::label",
   "Controller::WS::2::Label::label_search",
-  "Controller::WS::2::PUID::puid",
   "Controller::WS::2::Place::base",
   "Controller::WS::2::Place::place",
   "Controller::WS::2::Place::place_search",

diff --git a/t/lib/t/MusicBrainz/Server/Controller/WS/2/JSON/BrowseArtists.pm b/t/lib/t/MusicBrainz/Server/Controller/WS/2/JSON/BrowseArtists.pm
@@ -14,7 +14,7 @@ test 'browse artists via release group' => sub {
     MusicBrainz::Server::Test->prepare_test_database(shift->c, '+webservice');
 
     ws_test_json 'browse artists via release group',
-    '/artist?release-group=22b54315-6e51-350b-bb34-e6e16f7688bd' => encode_json(
+    '/artist?release-group=22b54315-6e51-350b-bb34-e6e16f7688bd' =>
         {
             "artist-offset" => 0,
             "artist-count" => 1,
@@ -37,15 +37,15 @@ test 'browse artists via release group' => sub {
                     ipis => [],
                     gender => JSON::null,
                 }]
-        });
+        };
 };
 
 test 'browse artists via recording' => sub {
 
     MusicBrainz::Server::Test->prepare_test_database(shift->c, '+webservice');
 
     ws_test_json 'browse artists via recording',
-    '/artist?inc=aliases&recording=0cf3008f-e246-428f-abc1-35f87d584d60' => encode_json(
+    '/artist?inc=aliases&recording=0cf3008f-e246-428f-abc1-35f87d584d60' =>
         {
             "artist-offset" => 0,
             "artist-count" => 2,
@@ -101,15 +101,15 @@ test 'browse artists via recording' => sub {
                     ipis => [],
                     gender => JSON::null,
                 }]
-        });
+        };
 };
 
 test 'browse artists via release, inc=tags+ratings' => sub {
 
     MusicBrainz::Server::Test->prepare_test_database(shift->c, '+webservice');
 
     ws_test_json 'browse artists via release, inc=tags+ratings',
-    '/artist?release=aff4a693-5970-4e2e-bd46-e2ee49c22de7&inc=tags+ratings' => encode_json(
+    '/artist?release=aff4a693-5970-4e2e-bd46-e2ee49c22de7&inc=tags+ratings' =>
         {
             "artist-offset" => 0,
             "artist-count" => 3,
@@ -182,7 +182,7 @@ test 'browse artists via release, inc=tags+ratings' => sub {
                     ipis => [],
                     gender => JSON::null,
                 }]
-        });
+        };
 };
 
 1;