From f5a78e4e4d7b9279326cfb1abb4db87e0f9a1a56 Mon Sep 17 00:00:00 2001 From: Randy Stauner Date: Mon, 3 Oct 2011 12:01:58 -0700 Subject: [PATCH] Move duplicate release-info api calls to role --- lib/MetaCPAN/Web/Controller/Module.pm | 37 ++++++-------- lib/MetaCPAN/Web/Controller/Release.pm | 38 +++++++------- lib/MetaCPAN/Web/Role/ReleaseInfo.pm | 71 ++++++++++++++++++++++++++ 3 files changed, 107 insertions(+), 39 deletions(-) create mode 100644 lib/MetaCPAN/Web/Role/ReleaseInfo.pm diff --git a/lib/MetaCPAN/Web/Controller/Module.pm b/lib/MetaCPAN/Web/Controller/Module.pm index 0ef1d03193..6ec0b657c8 100644 --- a/lib/MetaCPAN/Web/Controller/Module.pm +++ b/lib/MetaCPAN/Web/Controller/Module.pm @@ -5,6 +5,10 @@ use namespace::autoclean; BEGIN { extends 'MetaCPAN::Web::Controller' } +with qw( + MetaCPAN::Web::Role::ReleaseInfo +); + sub index : PathPart('module') : Chained('/') : Args { my ( $self, $c, @module ) = @_; my $data @@ -13,30 +17,21 @@ sub index : PathPart('module') : Chained('/') : Args { : $c->model('API::Module')->get(@module)->recv; $c->detach('/not_found') unless ( $data->{name} ); - my $pod = $c->model('API')->request( '/pod/' . join( '/', @module ) ); - my $release - = $c->model('API::Release')->get( $data->{author}, $data->{release} ); - my $author = $c->model('API::Author')->get( $data->{author} ); - my $versions - = $c->model('API::Release')->versions( $data->{distribution} ); - my $favorites - = $c->model('API::Favorite') - ->get( $c->user_exists ? $c->user->id : undef, - $data->{distribution} ); - my $rating = $c->model('API::Rating')->get( $data->{distribution} ); - ( $pod, $author, $release, $versions, $rating, $favorites ) - = ( $pod & $author & $release & $versions & $rating & $favorites )->recv; - $data->{myfavorite} = $favorites->{myfavorites}->{ $data->{distribution} }; - $data->{favorites} = $favorites->{favorites}->{ $data->{distribution} }; + + my $reqs = $self->api_requests($c, { + pod => $c->model('API')->request( '/pod/' . join( '/', @module ) ), + release => $c->model('API::Release')->get( @{$data}{qw(author release)} ), + }, + $data, + ); + $reqs = $self->recv_all($reqs); + $self->stash_api_results($c, $reqs, $data); + $self->add_favorites_data($data, $reqs->{favorites}, $data); $c->stash( { module => $data, - author => $author, - pod => $pod->{raw}, - release => $release->{hits}->{hits}->[0]->{_source}, - rating => $rating->{ratings}->{ $data->{distribution} }, - versions => - [ map { $_->{fields} } @{ $versions->{hits}->{hits} } ], + pod => $reqs->{pod}->{raw}, + release => $reqs->{release}->{hits}->{hits}->[0]->{_source}, template => 'module.html', } ); diff --git a/lib/MetaCPAN/Web/Controller/Release.pm b/lib/MetaCPAN/Web/Controller/Release.pm index 63bb11495f..ffd8a1128c 100644 --- a/lib/MetaCPAN/Web/Controller/Release.pm +++ b/lib/MetaCPAN/Web/Controller/Release.pm @@ -6,6 +6,10 @@ use namespace::autoclean; BEGIN { extends 'MetaCPAN::Web::Controller' } use List::Util (); +with qw( + MetaCPAN::Web::Role::ReleaseInfo +); + sub index : PathPart('release') : Chained('/') : Args { my ( $self, $c, $author, $release ) = @_; my $model = $c->model('API::Release'); @@ -15,22 +19,23 @@ sub index : PathPart('release') : Chained('/') : Args { ? $model->get( $author, $release ) : $model->find($author); my $out = $data->recv->{hits}->{hits}->[0]->{_source}; + $c->detach('/not_found') unless ($out); + ( $author, $release ) = ( $out->{author}, $out->{name} ); - my $modules = $model->modules( $author, $release ); - my $root = $model->root_files( $author, $release ); - my $versions = $model->versions( $out->{distribution} ); - $author = $c->model('API::Author')->get($author); - my $favorites - = $c->model('API::Favorite') - ->get( $c->user_exists ? $c->user->id : undef, - $out->{distribution} ); - my $rating = $c->model('API::Rating')->get( $out->{distribution} ); - ( $modules, $versions, $author, $root, $rating, $favorites ) - = ( $modules & $versions & $author & $root & $rating & $favorites )->recv; - $out->{myfavorite} = $favorites->{myfavorites}->{ $out->{distribution} }; - $out->{favorites} = $favorites->{favorites}->{ $out->{distribution} }; + my $reqs = $self->api_requests($c, { + root => $model->root_files( $author, $release ), + modules => $model->modules( $author, $release ), + }, + $out, + ); + $reqs = $self->recv_all($reqs); + $self->stash_api_results($c, $reqs, $out); + $self->add_favorites_data($out, $reqs->{favorites}, $out); + + # shortcuts + my ($root, $modules) = @{$reqs}{qw(root modules)}; my @root_files = ( sort @@ -45,19 +50,16 @@ sub index : PathPart('release') : Chained('/') : Args { } } + # TODO: make took more automatic (to include all) $c->stash( { template => 'release.html', release => $out, - author => $author, changes => $changes, total => $modules->{hits}->{total}, took => List::Util::max( - $modules->{took}, $root->{took}, $versions->{took} + $modules->{took}, $root->{took}, $reqs->{versions}->{took} ), - rating => $rating->{ratings}->{ $out->{distribution} }, root => \@root_files, - versions => - [ map { $_->{fields} } @{ $versions->{hits}->{hits} } ], files => [ map { { diff --git a/lib/MetaCPAN/Web/Role/ReleaseInfo.pm b/lib/MetaCPAN/Web/Role/ReleaseInfo.pm new file mode 100644 index 0000000000..9de5ca2356 --- /dev/null +++ b/lib/MetaCPAN/Web/Role/ReleaseInfo.pm @@ -0,0 +1,71 @@ +package MetaCPAN::Web::Role::ReleaseInfo; + +use Moose::Role; + +# TODO: are there other controllers that do (or should) include this? + +# TODO: should some of this be in a separate (instantiable) model +# so you don't have to keep passing $data? +# then wouldn't have to pass favorites back in. +# Role/API/Aggregator?, Model/APIAggregator/ReleaseInfo? + +# add favorites and myfavorite data into $main hash +sub add_favorites_data { + my ( $self, $main, $favorites, $data ) = @_; + $main->{myfavorite} = $favorites->{myfavorites}->{ $data->{distribution} }; + $main->{favorites} = $favorites->{favorites}->{ $data->{distribution} }; + return; +} + +# TODO: should the api_requests be in the base controller role, +# and then the default extras be defined in other roles? + +# pass in any api request condvars and combine them with these defaults +sub api_requests { + my ( $self, $c, $reqs, $data ) = @_; + + return { + author => $c->model('API::Author')->get( $data->{author} ), + + favorites => $c->model('API::Favorite') + ->get( $c->user_exists ? $c->user->id : undef, $data->{distribution} ), + + rating => $c->model('API::Rating')->get( $data->{distribution} ), + + versions => $c->model('API::Release')->versions( $data->{distribution} ), + + %$reqs, + }; +} + +# organize the api results into simple variables for the template +sub stash_api_results { + my ( $self, $c, $reqs, $data ) = @_; + + $c->stash({ + author => $reqs->{author}, + #release => $release->{hits}->{hits}->[0]->{_source}, + rating => $reqs->{rating}->{ratings}->{ $data->{distribution} }, + versions => + [ map { $_->{fields} } @{ $reqs->{versions}->{hits}->{hits} } ], + }); +} + +# call recv() on all values in the provided hashref +sub recv_all { + my ( $self, $condvars ) = @_; + + my @keys = keys %$condvars; + + my $recv = $condvars->{ $keys[0] }; + $recv &= $condvars->{ $_ } + for @keys[ 1 .. $#keys ]; + + # don't overwrite \%condars, seems like that should be controller's choice + my %results; + @results{ @keys } = $recv->recv; + + return \%results; +}; + +1;