Provide backwards compatibility for clients which request only specif…

…ic fields We claim in our docs¹ that our convenience endpoints revert the newer Elasticsearch behaviour of wrapping single values in arrays. This makes that so in more places, specifically the places where the client has requested a limited subset of fields. While this may seem like a pain for us and lead to uglier code, think about it this way: either we do it for our API one time, or every client that runs into the issue has to do it themselves many times over. A better solution may exist which centralizes application of single_valued_arrayref_to_scalar(), but this works for now. See also GH#580 for an example case.² This reverts a small change to tests which was made to accommodate the new ES behaviour. ¹ https://github.com/metacpan/metacpan-examples/blob/master/README.md#upgrading-from-v0 ² #580
metacpan · Nov 26, 2016 · f7850d3 · f7850d3
1 parent 2bf2d33
commit f7850d3
Show file tree

Hide file tree

Showing 7 changed files with 26 additions and 10 deletions.
diff --git a/lib/MetaCPAN/Server/Controller.pm b/lib/MetaCPAN/Server/Controller.pm
@@ -82,7 +82,8 @@ sub get : Path('') : Args(1) {
     if ( !defined $file ) {
         $c->detach( '/not_found', ['Not found'] );
     }
-    $c->stash( $file->{_source} || $file->{fields} )
+    $c->stash( $file->{_source}
+            || single_valued_arrayref_to_scalar( $file->{fields} ) )
         || $c->detach( '/not_found',
         ['The requested field(s) could not be found'] );
 }
@@ -149,8 +150,12 @@ sub join : ActionClass('Deserialize') {
         my $data
             = $is_get
             ? [ $c->stash ]
-            : [ map { $_->{_source} || $_->{fields} }
-                @{ $c->stash->{hits}->{hits} } ];
+            : [
+            map {
+                $_->{_source}
+                    || single_valued_arrayref_to_scalar( $_->{fields} )
+            } @{ $c->stash->{hits}->{hits} }
+            ];
         my @ids = List::MoreUtils::uniq grep {defined}
             map { ref $cself eq 'CODE' ? $cself->($_) : $_->{$cself} } @$data;
         my $filter = { terms => { $config->{foreign} => [@ids] } };

diff --git a/lib/MetaCPAN/Server/Controller/Author.pm b/lib/MetaCPAN/Server/Controller/Author.pm
@@ -4,6 +4,7 @@ use strict;
 use warnings;
 
 use Moose;
+use MetaCPAN::Util qw( single_valued_arrayref_to_scalar );
 
 BEGIN { extends 'MetaCPAN::Server::Controller' }
 
@@ -35,7 +36,8 @@ sub get : Path('') : Args(1) {
     if ( !defined $file ) {
         $c->detach( '/not_found', ['Not found'] );
     }
-    my $st = $file->{_source} || $file->{fields};
+    my $st = $file->{_source}
+        || single_valued_arrayref_to_scalar( $file->{fields} );
     if ( $st and $st->{pauseid} ) {
         $st->{release_count}
             = $c->model('CPAN::Release')

diff --git a/lib/MetaCPAN/Server/Controller/Favorite.pm b/lib/MetaCPAN/Server/Controller/Favorite.pm
@@ -4,6 +4,7 @@ use strict;
 use warnings;
 
 use Moose;
+use MetaCPAN::Util qw( single_valued_arrayref_to_scalar );
 
 BEGIN { extends 'MetaCPAN::Server::Controller' }
 
@@ -22,7 +23,8 @@ sub find : Path('') : Args(2) {
                 distribution => $distribution
             }
         );
-        $c->stash( $favorite->{_source} || $favorite->{fields} );
+        $c->stash( $favorite->{_source}
+                || single_valued_arrayref_to_scalar( $favorite->{fields} ) );
     } or $c->detach( '/not_found', [$@] );
 }
 

diff --git a/lib/MetaCPAN/Server/Controller/File.pm b/lib/MetaCPAN/Server/Controller/File.pm
@@ -5,6 +5,7 @@ use warnings;
 
 use ElasticSearchX::Model::Util;
 use Moose;
+use MetaCPAN::Util qw( single_valued_arrayref_to_scalar );
 
 BEGIN { extends 'MetaCPAN::Server::Controller' }
 
@@ -42,7 +43,8 @@ sub find : Path('') {
             }
         );
         if ( $file->{_source} || $file->{fields} ) {
-            $c->stash( $file->{_source} || $file->{fields} );
+            $c->stash( $file->{_source}
+                    || single_valued_arrayref_to_scalar( $file->{fields} ) );
         }
     } or $c->detach( '/not_found', [$@] );
 }

diff --git a/lib/MetaCPAN/Server/Controller/Module.pm b/lib/MetaCPAN/Server/Controller/Module.pm
@@ -4,6 +4,7 @@ use strict;
 use warnings;
 
 use Moose;
+use MetaCPAN::Util qw( single_valued_arrayref_to_scalar );
 
 BEGIN { extends 'MetaCPAN::Server::Controller::File' }
 
@@ -15,7 +16,8 @@ sub get : Path('') : Args(1) {
     if ( !defined $file ) {
         $c->detach( '/not_found', [] );
     }
-    $c->stash( $file->{_source} || $file->{fields} )
+    $c->stash( $file->{_source}
+            || single_valued_arrayref_to_scalar( $file->{fields} ) )
         || $c->detach( '/not_found',
         ['The requested field(s) could not be found'] );
 }

diff --git a/lib/MetaCPAN/Server/Controller/Release.pm b/lib/MetaCPAN/Server/Controller/Release.pm
@@ -4,6 +4,7 @@ use strict;
 use warnings;
 
 use Moose;
+use MetaCPAN::Util qw( single_valued_arrayref_to_scalar );
 
 BEGIN { extends 'MetaCPAN::Server::Controller' }
 
@@ -24,7 +25,8 @@ sub find : Path('') : Args(1) {
     if ( !defined $file ) {
         $c->detach( '/not_found', [] );
     }
-    $c->stash( $file->{_source} || $file->{fields} )
+    $c->stash( $file->{_source}
+            || single_valued_arrayref_to_scalar( $file->{fields} ) )
         || $c->detach( '/not_found',
         ['The requested field(s) could not be found'] );
 }
@@ -44,7 +46,8 @@ sub get : Path('') : Args(2) {
     if ( !defined $file ) {
         $c->detach( '/not_found', [] );
     }
-    $c->stash( $file->{_source} || $file->{fields} )
+    $c->stash( $file->{_source}
+            || single_valued_arrayref_to_scalar( $file->{fields} ) )
         || $c->detach( '/not_found',
         ['The requested field(s) could not be found'] );
 }

diff --git a/t/server/controller/module.t b/t/server/controller/module.t
@@ -73,7 +73,7 @@ test_psgi app, sub {
         elsif ( $k =~ /fields/ ) {
             is_deeply(
                 $json,
-                { documentation => ['Moose'], name => ['Moose.pm'] },
+                { documentation => 'Moose', name => 'Moose.pm' },
                 'controller proxies field query parameter to ES'
             );
         }