From c91d3f2fbae6ccdbcf01aab0b62a559ea7b3b7c4 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Fri, 2 May 2025 11:11:02 -0300 Subject: [PATCH] Bug 39817: Add some GET routes tests This patch adds tests for to-be mandatory parameters for plural endpoints: * match * order_by * page * per_page * q_param * q_body * request_id_header Exceptions should be added if found in the wild, but the tests are needed. Another check it adds is GET requests `operationId` should start with: * list * get To test: 1. Run: $ ktd --shell k$ prove xt/api.t => FAIL: Tests pass! But our spec is bad 2. Apply this patch 3. Repeat 1 => SUCCESS: All mistakes make the tests fail! 4. Sign off :-D Note: a follow-up patch/set of patches should make this tests pass! Signed-off-by: Tomas Cohen Arazi --- xt/api.t | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 75 insertions(+), 1 deletion(-) diff --git a/xt/api.t b/xt/api.t index 78cf588c245..7e77a16e86b 100755 --- a/xt/api.t +++ b/xt/api.t @@ -21,7 +21,7 @@ use Data::Dumper; use FindBin(); use IPC::Cmd qw(can_run); -use List::MoreUtils qw(any); +use List::MoreUtils qw(any none); use File::Slurp qw(read_file); my $t = Test::Mojo->new('Koha::REST::V1'); @@ -172,6 +172,80 @@ subtest '400 response tests' => sub { } }; +subtest 'GET routes parameters tests' => sub { + + plan tests => 1; + + my @errors; + + foreach my $route ( sort keys %{$paths} ) { + foreach my $verb ( keys %{ $paths->{$route} } ) { + + next unless $verb eq 'get'; + + if ( $paths->{$route}->{$verb}->{operationId} !~ m/^list/ + && $paths->{$route}->{$verb}->{operationId} !~ m/^get/ ) + { + push @errors, + sprintf( + "%s %s -> bad operationId: %s (must start with list or get)", $verb, $route, + $paths->{$route}->{$verb}->{operationId} + ); + next; + } + + my @parameters = @{ $paths->{$route}->{$verb}->{parameters} }; + + my $plural = ( any { $_->{in} eq 'body' && $_->{name} eq 'query' } @parameters ) + || ( any { $_->{in} eq 'query' && $_->{name} eq 'q' } @parameters ); + + if ($plural) { + + # 'q' as query parameter needs to be supported + if ( none { $_->{in} eq 'query' && $_->{name} eq 'q' } @parameters ) { + push @errors, sprintf( "%s %s -> missing 'q' parameter", $verb, $route ); + } + + # 'query' in body needs to be supported + if ( none { $_->{in} eq 'body' && $_->{name} eq 'query' } @parameters ) { + push @errors, sprintf( "%s %s -> missing 'query' parameter", $verb, $route ); + } + + # plural lists should have match for datatables compatibility + if ( none { $_->{ref} && $_->{ref} eq '../swagger.yaml#/parameters/match' } @parameters ) { + push @errors, sprintf( "%s %s -> missing 'match' parameter", $verb, $route ); + } + + # plural lists should have order_by for datatables compatibility + if ( none { $_->{ref} && $_->{ref} eq '../swagger.yaml#/parameters/order_by' } @parameters ) { + push @errors, sprintf( "%s %s -> missing 'order_by' parameter", $verb, $route ); + } + + # plural lists should have page for datatables compatibility + if ( none { $_->{ref} && $_->{ref} eq '../swagger.yaml#/parameters/page' } @parameters ) { + push @errors, sprintf( "%s %s -> missing 'page' parameter", $verb, $route ); + } + + # plural lists should have per_page for datatables compatibility + if ( none { $_->{ref} && $_->{ref} eq '../swagger.yaml#/parameters/per_page' } @parameters ) { + push @errors, sprintf( "%s %s -> missing 'per_page' parameter", $verb, $route ); + } + + # plural lists should have request_id_header for datatables compatibility + if ( none { $_->{ref} && $_->{ref} eq '../swagger.yaml#/parameters/request_id_header' } @parameters ) { + push @errors, sprintf( "%s %s -> missing request_id_header parameter", $verb, $route ); + } + } + } + } + + is( scalar @errors, 0, 'No errors in GET endpoints definitions in the spec' ); + + foreach my $error (@errors) { + print STDERR "$error\n"; + } +}; + subtest 'POST (201) have location header' => sub { my @files = `git ls-files 'Koha/REST/V1/**/*.pm'`; plan tests => scalar @files; -- 2.49.0