Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

normalize pagination parameters and add size limits #1313

Merged
merged 2 commits into from
Nov 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions lib/MetaCPAN/API/Controller/Search.pm
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,13 @@ sub web {
return unless $c->openapi->valid_input;
my $args = $c->validation->output;

my @search = ( @{$args}{qw/q from size/} );
push @search, $args->{collapsed} if exists $args->{collapsed};
my $results = $c->model->search->search_web(@search);
my $query = $args->{q};
my $size = $args->{page_size} // $args->{size} // 20;
my $page = $args->{page} // ( 1 + int( ( $args->{from} // 0 ) / $size ) );
my $collapsed = $args->{collapsed};

my $results
= $c->model->search->search_web( $query, $page, $size, $collapsed );

return $c->render( json => $results );
}
Expand Down
10 changes: 9 additions & 1 deletion lib/MetaCPAN/Query/Favorite.pm
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package MetaCPAN::Query::Favorite;
use MetaCPAN::Moose;

use MetaCPAN::ESConfig qw( es_doc_path );
use MetaCPAN::Util qw(hit_total);
use MetaCPAN::Util qw( MAX_RESULT_WINDOW hit_total );

with 'MetaCPAN::Query::Role::Common';

Expand Down Expand Up @@ -148,6 +148,14 @@ sub recent {
$page //= 1;
$size //= 100;

if ( $page * $size >= MAX_RESULT_WINDOW ) {
return +{
favorites => [],
took => 0,
total => 0,
};
}

my $favs = $self->es->search(
es_doc_path('favorite'),
body => {
Expand Down
56 changes: 39 additions & 17 deletions lib/MetaCPAN/Query/Release.pm
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use MetaCPAN::Moose;

use MetaCPAN::ESConfig qw( es_doc_path );
use MetaCPAN::Util
qw( hit_total single_valued_arrayref_to_scalar true false );
qw( MAX_RESULT_WINDOW hit_total single_valued_arrayref_to_scalar true false );

with 'MetaCPAN::Query::Role::Common';

Expand Down Expand Up @@ -395,10 +395,18 @@ sub by_author_and_names {
}

sub by_author {
my ( $self, $pauseid, $size, $page ) = @_;
my ( $self, $pauseid, $page, $size ) = @_;
$size //= 1000;
$page //= 1;

if ( $page * $size >= MAX_RESULT_WINDOW ) {
return {
releases => [],
took => 0,
total => 0,
};
}

my $body = {
query => {
bool => {
Expand Down Expand Up @@ -487,10 +495,18 @@ sub latest_by_author {
}

sub all_by_author {
my ( $self, $author, $size, $page ) = @_;
my ( $self, $author, $page, $size ) = @_;
$size //= 100;
$page //= 1;

if ( $page * $size >= MAX_RESULT_WINDOW ) {
return {
releases => [],
took => 0,
total => 0,
};
}

my $body = {
query => { term => { author => uc($author) } },
sort => [ { date => 'desc' } ],
Expand Down Expand Up @@ -623,11 +639,11 @@ sub top_uploaders {
sub requires {
my ( $self, $module, $page, $page_size, $sort ) = @_;
return $self->_get_depended_releases( [$module], $page, $page_size,
$page_size, $sort );
$sort );
}

sub reverse_dependencies {
my ( $self, $distribution, $page, $page_size, $size, $sort ) = @_;
my ( $self, $distribution, $page, $page_size, $sort ) = @_;

# get the latest release of given distribution
my $release = $self->_get_latest_release($distribution) || return;
Expand All @@ -637,7 +653,7 @@ sub reverse_dependencies {

# return releases depended on those modules
return $self->_get_depended_releases( $modules, $page, $page_size,
$size, $sort );
$sort );
}

sub _get_latest_release {
Expand Down Expand Up @@ -709,10 +725,18 @@ sub _fix_sort_value {
}

sub _get_depended_releases {
my ( $self, $modules, $page, $page_size, $size, $sort ) = @_;
my ( $self, $modules, $page, $page_size, $sort ) = @_;
$page //= 1;
$page_size //= 50;

if ( $page * $page_size >= MAX_RESULT_WINDOW ) {
return +{
data => [],
took => 0,
total => 0,
};
}

$sort = _fix_sort_value($sort);

my $dependency_filter = {
Expand Down Expand Up @@ -754,8 +778,8 @@ sub _get_depended_releases {
],
},
},
size => $size || $page_size,
from => $page * $page_size - $page_size,
size => $page_size,
from => ( $page - 1 ) * $page_size,
sort => $sort,
}
);
Expand All @@ -768,22 +792,20 @@ sub _get_depended_releases {
}

sub recent {
my ( $self, $page, $page_size, $type ) = @_;
my ( $self, $type, $page, $page_size ) = @_;
$page //= 1;
$page_size //= 10000;
$type //= '';

my $query;
my $from = ( $page - 1 ) * $page_size;

if ( $from + $page_size > 10000 ) {
return {
if ( $page * $page_size >= MAX_RESULT_WINDOW ) {
return +{
releases => [],
total => 0,
took => 0,
total => 0,
};
}

my $query;
if ( $type eq 'n' ) {
$query = {
constant_score => {
Expand Down Expand Up @@ -813,7 +835,7 @@ sub recent {

my $body = {
size => $page_size,
from => $from,
from => ( $page - 1 ) * $page_size,
query => $query,
_source =>
[qw(name author status abstract date distribution maturity)],
Expand Down
28 changes: 19 additions & 9 deletions lib/MetaCPAN/Query/Search.pm
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use List::Util qw( min uniq );
use Log::Contextual qw( :log :dlog );
use MetaCPAN::ESConfig qw( es_doc_path );
use MetaCPAN::Types::TypeTiny qw( Object Str );
use MetaCPAN::Util qw( hit_total true false );
use MetaCPAN::Util qw( MAX_RESULT_WINDOW hit_total true false );
use MooseX::StrictConstructor;

with 'MetaCPAN::Query::Role::Common';
Expand Down Expand Up @@ -54,11 +54,20 @@ sub search_for_first_result {
=cut

sub search_web {
my ( $self, $search_term, $from, $page_size, $collapsed,
my ( $self, $search_term, $page, $page_size, $collapsed,
$max_collapsed_hits )
= @_;
$page_size //= 20;
$from //= 0;
$page //= 1;

if ( $page * $page_size >= MAX_RESULT_WINDOW ) {
return {
results => [],
total => 0,
tool => 0,
colapsed => $collapsed ? true : false,
};
}

$search_term =~ s{([+=><!&|\(\)\{\}[\]\^"~*?\\/])}{\\$1}g;

Expand All @@ -78,15 +87,15 @@ sub search_web {

my $results
= $collapsed // $search_term !~ /(distribution|module\.name\S*):/
? $self->_search_collapsed( $search_term, $from, $page_size,
? $self->_search_collapsed( $search_term, $page, $page_size,
$max_collapsed_hits )
: $self->_search_expanded( $search_term, $from, $page_size );
: $self->_search_expanded( $search_term, $page, $page_size );

return $results;
}

sub _search_expanded {
my ( $self, $search_term, $from, $page_size ) = @_;
my ( $self, $search_term, $page, $page_size ) = @_;

# Used for distribution and module searches, the limit is included in
# the query and ES does the right thing (because we are not collapsing
Expand All @@ -95,7 +104,7 @@ sub _search_expanded {
$search_term,
{
size => $page_size,
from => $from
from => ( $page - 1 ) * $page_size,
}
);

Expand All @@ -122,11 +131,12 @@ sub _search_expanded {
}

sub _search_collapsed {
my ( $self, $search_term, $from, $page_size, $max_collapsed_hits ) = @_;
my ( $self, $search_term, $page, $page_size, $max_collapsed_hits ) = @_;

$max_collapsed_hits ||= 5;

my $total_size = $from + $page_size;
my $from = ( $page - 1 ) * $page_size;
my $total_size = $page * $page_size;

my $es_query_opts = {
size => 0,
Expand Down
2 changes: 1 addition & 1 deletion lib/MetaCPAN/Server/Controller/Favorite.pm
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ sub recent : Path('recent') : Args(0) {
$c->stash_or_detach(
$self->model($c)->recent(
$c->req->param('page') || 1,
$c->req->param('size') || 100
$c->req->param('page_size') || $c->req->param('size') || 100,
)
);
}
Expand Down
20 changes: 14 additions & 6 deletions lib/MetaCPAN/Server/Controller/Release.pm
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,20 @@ sub modules : Path('modules') : Args(2) {

sub recent : Path('recent') : Args(0) {
my ( $self, $c ) = @_;
my @params = @{ $c->req->params }{qw( page page_size type )};
$c->stash_or_detach( $self->model($c)->recent(@params) );
my $params = $c->req->params;
my $type = $params->{type};
my $page = $params->{page};
my $size = $params->{page_size};
$c->stash_or_detach( $self->model($c)->recent( $type, $page, $size ) );
}

sub by_author : Path('by_author') : Args(1) {
my ( $self, $c, $pauseid ) = @_;
$c->stash_or_detach( $self->model($c)
->by_author( $pauseid, @{ $c->req->params }{qw( size page )} ) );
my $params = $c->req->params;
my $page = $params->{page};
my $size = $params->{page_size} // $params->{size};
$c->stash_or_detach(
$self->model($c)->by_author( $pauseid, $page, $size ) );
}

sub latest_by_distribution : Path('latest_by_distribution') : Args(1) {
Expand All @@ -69,9 +75,11 @@ sub latest_by_author : Path('latest_by_author') : Args(1) {

sub all_by_author : Path('all_by_author') : Args(1) {
my ( $self, $c, $pauseid ) = @_;
my @params = @{ $c->req->params }{qw( page_size page )};
my $params = $c->req->params;
my $page = $params->{page};
my $size = $params->{page_size};
$c->stash_or_detach(
$self->model($c)->all_by_author( $pauseid, @params ) );
$self->model($c)->all_by_author( $pauseid, $page, $size ) );
}

sub versions : Path('versions') : Args(1) {
Expand Down
2 changes: 1 addition & 1 deletion lib/MetaCPAN/Server/Controller/ReverseDependencies.pm
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ sub dist : Path('dist') : Args(1) {
my ( $self, $c, $dist ) = @_;
$c->stash_or_detach(
$c->model('ESModel')->doc('release')->reverse_dependencies(
$dist, @{ $c->req->params }{qw< page page_size size sort >}
$dist, @{ $c->req->params }{qw< page page_size sort >}
)
);
}
Expand Down
7 changes: 6 additions & 1 deletion lib/MetaCPAN/Server/Controller/Search/Web.pm
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,13 @@ sub web : Chained('/search/index') : PathPart('web') : Args(0) {
my ( $self, $c ) = @_;
my $args = $c->req->params;

my $query = $args->{q};
my $size = $args->{page_size} // $args->{size} // 20;
my $page = $args->{page} // ( 1 + int( ( $args->{from} // 0 ) / $size ) );
my $collapsed = $args->{collapsed};

my $model = $c->model('Search');
my $results = $model->search_web( @{$args}{qw( q from size collapsed )} );
my $results = $model->search_web( $query, $page, $size, $collapsed );

$c->stash($results);
}
Expand Down
3 changes: 3 additions & 0 deletions lib/MetaCPAN/Util.pm
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,12 @@ use Sub::Exporter -setup => {
true
false
is_bool
MAX_RESULT_WINDOW
) ]
};

use constant MAX_RESULT_WINDOW => 10000;

*true = \&Cpanel::JSON::XS::true;
*false = \&Cpanel::JSON::XS::false;
*is_bool = \&Cpanel::JSON::XS::is_bool;
Expand Down
16 changes: 14 additions & 2 deletions root/static/requests/search.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,26 @@ search_web:
See also `collapsed`
type: string
required: true
- name: page
in: query
description: The page of the results to return
type: integer
- name: page_size
in: query
description: Number of results per page
type: integer
- name: from
in: query
description: The offset to use in the result set
description: |
The offset to use in the result set. Deprecated. Only used if `page`
is not set.
type: integer
default: 0
- name: size
in: query
description: Number of results per page
description: |
Number of results per page. Deprecated. Only used if `page_size` is
not set.
type: integer
default: 20
- name: collapsed
Expand Down
4 changes: 2 additions & 2 deletions t/model/search.t
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,12 @@ ok( $search, 'search' );

ok( $collapsed_search->{collapsed}, 'results are flagged as collapsed' );

my $from = 0;
my $page = 1;
my $page_size = 20;
my $collapsed = 0;

my $expanded
= $search->search_web( 'Foo', $from, $page_size, $collapsed );
= $search->search_web( 'Foo', $page, $page_size, $collapsed );

ok( !$expanded->{collapsed}, 'results are flagged as expanded' );

Expand Down