FIX0rzdzca: sort scroller vs. given items:

Accept a scroller or a set of items, given: * You can't have both * You can't have none * Always check for existing scroller with predicate * Check isa using Safe::Isa Also: * Clean up dead code * Update examples * Update tests (use better fake Scroller object) OMG MICKEY DID SO MUCH WORK ON THIS THANK YOU
metacpan · May 26, 2014 · eb77a27 · eb77a27
1 parent aa20d59
commit eb77a27
Show file tree

Hide file tree

Showing 5 changed files with 42 additions and 60 deletions.
diff --git a/examples/rev_deps-recursive.pl b/examples/rev_deps-recursive.pl
@@ -17,16 +17,16 @@ sub dig {
 
     my $res   = $mcpan->reverse_dependencies($dist);
 
-    for ( @{$res} ) {
+    while ( my $item = $res->next ) {
         if ( $level ) {
             printf "%s%s\n",
                 colored( '....' x $level, 'yellow' ),
-                $_->distribution;
+                $item->distribution;
         } else {
             printf "\n>> %s\n",
-                colored( $_->distribution, 'blue' );
+                colored( $item->distribution, 'blue' );
         }
 
-        dig( $_->distribution, $level + 1 );
+        dig( $item->distribution, $level + 1 );
     }
-};
+}
diff --git a/examples/rev_deps.pl b/examples/rev_deps.pl
@@ -13,6 +13,6 @@
         name   => $rel->name,
         author => $rel->author,
     };
-};
+}
 
 p @output;
diff --git a/lib/MetaCPAN/Client.pm b/lib/MetaCPAN/Client.pm
@@ -209,8 +209,8 @@ sub _reverse_deps {
     };
 
     return MetaCPAN::Client::ResultSet->new(
-        list => $res->{'hits'}{'hits'},
-        type => 'release',
+        items => $res->{'hits'}{'hits'},
+        type  => 'release',
     );
 }
 

diff --git a/lib/MetaCPAN/Client/ResultSet.pm b/lib/MetaCPAN/Client/ResultSet.pm
@@ -18,86 +18,62 @@ has type => (
 
 # in case we're returning from a scrolled search
 has scroller => (
-    is       => 'ro',
-    isa      => sub {
-        ref $_[0] eq 'Search::Elasticsearch::Scroll'
+    is        => 'ro',
+    isa       => sub {
+        use Safe::Isa;
+        $_[0]->$_isa('Search::Elasticsearch::Scroll')
             or croak 'scroller must be an Search::Elasticsearch::Scroll object';
     },
-    required => 0,
+    predicate => 'has_scroller',
 );
 
 # in case we're returning from a fetch
-has list => (
-    is       => 'ro',
-    isa      => sub {
+has items => (
+    is  => 'ro',
+    isa => sub {
         ref $_[0] eq 'ARRAY'
-            or croak 'list must be an array ref';
+            or croak 'items must be an array ref';
     },
-    required => 0,
 );
 
 has total => (
     is      => 'ro',
-    default => 0,
-);
+    default => sub {
+        my $self = shift;
 
-has facets => (
-    is      => 'ro',
-    lazy    => 1,
-    builder => '_get_facets',
+        return $self->has_scroller ? $self->scroller->total
+                                   : scalar @{ $self->items };
+    },
 );
 
-sub _get_facets {
-    my $self = shift;
-
-    return $self->scroller->facets || {};
-}
-
 sub BUILDARGS {
     my ( $class, %args ) = @_;
 
-    exists $args{scroller} or exists $args{list}
-        or croak "wrong args to ResultSet";
+    exists $args{scroller} or exists $args{items}
+        or croak 'ResultSet must get either scroller or items';
 
-    $args{total} = exists $args{scroller}
-        ? $args{scroller}->total
-        : scalar(@{ $args{list} });
+    exists $args{scroller} and exists $args{items}
+        and croak 'ResultSet must get either scroller or items, not both';
 
     return \%args;
 }
 
-
-sub _total {
-    my $self = shift;
-
-    ref $self->scroller and return $self->scroller->total;
-
-    return scalar(@{ $self->list });
-}
-
 sub next {
-    my $self = shift;
-    my $result;
-
-    # list:
-    if ( ref $self->list ) {
-        scalar @{ $self->list } > 0
-            or return;
+    my $self   = shift;
+    my $result = $self->has_scroller ? $self->scroller->next
+                                     : shift @{ $self->items };
 
-        $result = shift @{ $self->list };
-
-    } else {
-        $result = $self->scroller->next;
-    }
-
-    defined $result
-        or return;
+    defined $result or return;
 
     my $class = 'MetaCPAN::Client::' . ucfirst $self->type;
-
     return $class->new_from_request( $result->{'_source'} || $result->{'fields'} );
 }
 
+sub facets {
+    my $self = shift;
+
+    return $self->has_scroller ? $self->scroller->facets : {};
+}
 
 1;
 

diff --git a/t/resultset.t b/t/resultset.t
@@ -7,11 +7,17 @@ use Test::More tests => 3;
 use Test::Fatal;
 use MetaCPAN::Client::ResultSet;
 
+{
+    package MetaCPAN::Client::Test::ScrollerZ;
+    use base 'Search::Elasticsearch::Scroll'; # < 5.10 FTW (except, no)
+    sub total {0}
+}
+
 like(
     exception {
         MetaCPAN::Client::ResultSet->new(
             type     => 'failZZ',
-            scroller => bless {}, 'Search::Elasticsearch::Scroll',
+            scroller => bless {}, 'MetaCPAN::Client::Test::ScrollerZ',
         )
     },
     qr/Invalid type/,