Bugzilla – Attachment 107837 Details for
Bug 26143
The API does not handle requesting all resources
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 26143: Regression tests
Bug-26143-Regression-tests.patch (text/plain), 5.95 KB, created by
Tomás Cohen Arazi (tcohen)
on 2020-08-05 15:25:54 UTC
(
hide
)
Description:
Bug 26143: Regression tests
Filename:
MIME Type:
Creator:
Tomás Cohen Arazi (tcohen)
Created:
2020-08-05 15:25:54 UTC
Size:
5.95 KB
patch
obsolete
>From 6768d762781473312b62e6746875cdeea3324959 Mon Sep 17 00:00:00 2001 >From: Tomas Cohen Arazi <tomascohen@theke.io> >Date: Wed, 5 Aug 2020 12:04:47 -0300 >Subject: [PATCH] Bug 26143: Regression tests > >This patch introduces tests for the per_page=-1 handling use case. From >now on per_page=-1 means 'all resources'. > >On writing this I noticed that we always paginate results no matter >what, but there was a weird condition under which on pagination headers >were sent back to the API consumer. This is highlighted in the precedent >patch, which is not the -1 situation this one tries to tackle. > >Both pagination and searching are broken with per_page=-1, which is a >standard, and we actually didn't explicitly set a way to request all >resources. > >To verify this: >1. Apply the previous tests patch and this one >2. Run: > $ kshell > k$ prove t/Koha/REST/Plugin/Pagination.t \ > t/db_dependent/Koha/REST/Plugin/Objects.t >=> FAIL: Things are damn broken > >Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> >--- > t/Koha/REST/Plugin/Pagination.t | 52 ++++++++++++++++++++++- > t/db_dependent/Koha/REST/Plugin/Objects.t | 14 +++++- > 2 files changed, 64 insertions(+), 2 deletions(-) > >diff --git a/t/Koha/REST/Plugin/Pagination.t b/t/Koha/REST/Plugin/Pagination.t >index adf3001c4f..dff13fb770 100644 >--- a/t/Koha/REST/Plugin/Pagination.t >+++ b/t/Koha/REST/Plugin/Pagination.t >@@ -70,6 +70,28 @@ get '/pagination_headers_without_page' => sub { > $c->render( json => { ok => 1 }, status => 200 ); > }; > >+get '/pagination_headers_with_minus_one' => sub { >+ my $c = shift; >+ $c->add_pagination_headers( >+ { >+ total => 10, >+ params => { _per_page => -1, firstname => 'Jonathan' } >+ } >+ ); >+ $c->render( json => { ok => 1 }, status => 200 ); >+}; >+ >+get '/pagination_headers_with_minus_one_and_invalid_page' => sub { >+ my $c = shift; >+ $c->add_pagination_headers( >+ { >+ total => 10, >+ params => { page => 100, _per_page => -1, firstname => 'Jonathan' } >+ } >+ ); >+ $c->render( json => { ok => 1 }, status => 200 ); >+}; >+ > # The tests > > use Test::More tests => 2; >@@ -79,7 +101,7 @@ use t::lib::Mocks; > > subtest 'add_pagination_headers() tests' => sub { > >- plan tests => 75; >+ plan tests => 101; > > my $t = Test::Mojo->new; > >@@ -164,6 +186,34 @@ subtest 'add_pagination_headers() tests' => sub { > ->header_like( 'Link' => qr/<http:\/\/.*\?.*per_page=3.*>; rel="last"/ ) > ->header_like( 'Link' => qr/<http:\/\/.*\?.*page=4.*>; rel="last"/ ) > ->header_like( 'Link' => qr/<http:\/\/.*\?.*firstname=Jonathan.*>; rel="last"/ ); >+ >+ $t->get_ok('/pagination_headers_with_minus_one') >+ ->status_is( 200 ) >+ ->header_is( 'X-Total-Count' => 10, 'X-Total-Count header present, with per_page=-1' ) >+ ->header_unlike( 'Link' => qr/<http:\/\/.*\?.*per_page=-1.*>; rel="prev",/, 'First page, no previous' ) >+ ->header_unlike( 'Link' => qr/<http:\/\/.*\?.*page=1.*>; rel="prev",/, 'First page, no previous' ) >+ ->header_unlike( 'Link' => qr/<http:\/\/.*\?.*firstname=Jonathan.*>; rel="prev",/, 'First page, no previous' ) >+ ->header_unlike( 'Link' => qr/<http:\/\/.*\?.*per_page=-1.*>; rel="next",/, 'No next page, all resources are fetched' ) >+ ->header_like( 'Link' => qr/<http:\/\/.*\?.*per_page=-1.*>; rel="first",/ ) >+ ->header_like( 'Link' => qr/<http:\/\/.*\?.*page=1.*>; rel="first",/ ) >+ ->header_like( 'Link' => qr/<http:\/\/.*\?.*firstname=Jonathan.*>; rel="first",/ ) >+ ->header_like( 'Link' => qr/<http:\/\/.*\?.*per_page=-1.*>; rel="last"/ ) >+ ->header_like( 'Link' => qr/<http:\/\/.*\?.*page=1.*>; rel="last"/ ) >+ ->header_like( 'Link' => qr/<http:\/\/.*\?.*firstname=Jonathan.*>; rel="last"/ ); >+ >+ $t->get_ok('/pagination_headers_with_minus_one_and_invalid_page') >+ ->status_is( 200 ) >+ ->header_is( 'X-Total-Count' => 10, 'X-Total-Count header present, with per_page=-1' ) >+ ->header_unlike( 'Link' => qr/<http:\/\/.*\?.*per_page=-1.*>; rel="prev",/, 'First page, no previous' ) >+ ->header_unlike( 'Link' => qr/<http:\/\/.*\?.*page=1.*>; rel="prev",/, 'First page, no previous' ) >+ ->header_unlike( 'Link' => qr/<http:\/\/.*\?.*firstname=Jonathan.*>; rel="prev",/, 'First page, no previous' ) >+ ->header_unlike( 'Link' => qr/<http:\/\/.*\?.*per_page=-1.*>; rel="next",/, 'No next page, all resources are fetched' ) >+ ->header_like( 'Link' => qr/<http:\/\/.*\?.*per_page=-1.*>; rel="first",/ ) >+ ->header_like( 'Link' => qr/<http:\/\/.*\?.*page=1.*>; rel="first",/ ) >+ ->header_like( 'Link' => qr/<http:\/\/.*\?.*firstname=Jonathan.*>; rel="first",/ ) >+ ->header_like( 'Link' => qr/<http:\/\/.*\?.*per_page=-1.*>; rel="last"/ ) >+ ->header_like( 'Link' => qr/<http:\/\/.*\?.*page=1.*>; rel="last"/ ) >+ ->header_like( 'Link' => qr/<http:\/\/.*\?.*firstname=Jonathan.*>; rel="last"/ ); > }; > > subtest 'dbic_merge_pagination() tests' => sub { >diff --git a/t/db_dependent/Koha/REST/Plugin/Objects.t b/t/db_dependent/Koha/REST/Plugin/Objects.t >index 4445bdea9d..d5db5e7c66 100644 >--- a/t/db_dependent/Koha/REST/Plugin/Objects.t >+++ b/t/db_dependent/Koha/REST/Plugin/Objects.t >@@ -91,7 +91,7 @@ my $builder = t::lib::TestBuilder->new; > > subtest 'objects.search helper' => sub { > >- plan tests => 44; >+ plan tests => 50; > > $schema->storage->txn_begin; > >@@ -192,6 +192,18 @@ subtest 'objects.search helper' => sub { > $response_count = scalar @{ $t->tx->res->json }; > is( $response_count, 5, 'RESTdefaultPageSize is honoured by default (5)' ); > >+ $t->get_ok('/cities?_page=1&_per_page=-1') >+ ->status_is(200); >+ >+ $response_count = scalar @{ $t->tx->res->json }; >+ is( $response_count, 24, '_per_page=-1 means all resources' ); >+ >+ $t->get_ok('/cities?_page=100&_per_page=-1') >+ ->status_is(200); >+ >+ $response_count = scalar @{ $t->tx->res->json }; >+ is( $response_count, 24, 'When _per_page=-1 the page param is not considered' ); >+ > $schema->storage->txn_rollback; > }; > >-- >2.28.0
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 26143
:
107836
|
107837
|
107838
|
107839
|
107840
|
107841