Attachment 98165 Details for Bug 24487 – Bug 24487: Don't apply matching criteria to path parameters

[patch] Bug 24487: Don't apply matching criteria to path parameters

Bug-24487-Dont-apply-matching-criteria-to-path-par.patch (text/plain), 5.75 KB, created by Martin Renvoize (ashimema) on 2020-01-30 14:57:47 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Renvoize (ashimema)

Created: 2020-01-30 14:57:47 UTC

Size: 5.75 KB

patch

obsolete

>From 77b1dde05f0c0e3f7bbe8c45d9bb4db04a8267fb Mon Sep 17 00:00:00 2001
>From: Agustin Moyano <agustinmoyano@theke.io>
>Date: Thu, 23 Jan 2020 01:50:29 -0300
>Subject: [PATCH] Bug 24487: Don't apply matching criteria to path parameters
>
>This patch separates query parameters from path parameters, and uses exact matching for the later.
>
>To test:
>1. Apply this patch
>2. prove t/Koha/REST/Plugin/Query.t t/db_dependent/Koha/REST/Plugin/Objects.t
>SUCCESS => tests ok
>3. Sign off
>
>Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
>Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
>---
> Koha/REST/Plugin/Objects.pm | 12 +++++++++++-
> Koha/REST/Plugin/Query.pm   |  9 +++++++--
> t/Koha/REST/Plugin/Query.t  | 37 ++++++++++++++++---------------------
> 3 files changed, 34 insertions(+), 24 deletions(-)
>
>diff --git a/Koha/REST/Plugin/Objects.pm b/Koha/REST/Plugin/Objects.pm
>index 4ce9c5134d..afe98560df 100644
>--- a/Koha/REST/Plugin/Objects.pm
>+++ b/Koha/REST/Plugin/Objects.pm
>@@ -50,7 +50,7 @@ sub register {
>             my $attributes = {};
> 
>             # Extract reserved params
>-            my ( $filtered_params, $reserved_params ) = $c->extract_reserved_params($args);
>+            my ( $filtered_params, $reserved_params, $path_params ) = $c->extract_reserved_params($args);
>             # Look for embeds
>             my $embed = $c->stash('koha.embed');
> 
>@@ -79,6 +79,16 @@ sub register {
>                 $filtered_params = $c->build_query_params( $filtered_params, $reserved_params );
>             }
> 
>+            if ( defined $path_params ) {
>+
>+                # Apply the mapping function to the passed params
>+                $filtered_params //= {};
>+                $path_params = $result_set->attributes_from_api($path_params);
>+                foreach my $param (keys %{$path_params}) {
>+                    $filtered_params->{$param} = $path_params->{$param};
>+                }
>+            }
>+
>             # Perform search
>             my $objects = $result_set->search( $filtered_params, $attributes );
> 
>diff --git a/Koha/REST/Plugin/Query.pm b/Koha/REST/Plugin/Query.pm
>index b2ebd8f77a..769fd0b614 100644
>--- a/Koha/REST/Plugin/Query.pm
>+++ b/Koha/REST/Plugin/Query.pm
>@@ -54,19 +54,24 @@ Generates the DBIC query from the query parameters.
> 
>             my $reserved_params;
>             my $filtered_params;
>+            my $path_params;
> 
>             my $reserved_words = _reserved_words();
>+            my @query_param_names = keys %{$c->req->params->to_hash};
> 
>             foreach my $param ( keys %{$params} ) {
>                 if ( grep { $param eq $_ } @{$reserved_words} ) {
>                     $reserved_params->{$param} = $params->{$param};
>                 }
>-                else {
>+                elsif ( grep { $param eq $_ } @query_param_names ) {
>                     $filtered_params->{$param} = $params->{$param};
>                 }
>+                else {
>+                    $path_params->{$param} = $params->{$param};
>+                }
>             }
> 
>-            return ( $filtered_params, $reserved_params );
>+            return ( $filtered_params, $reserved_params, $path_params );
>         }
>     );
> 
>diff --git a/t/Koha/REST/Plugin/Query.t b/t/Koha/REST/Plugin/Query.t
>index fa69b55848..44fc48b8be 100644
>--- a/t/Koha/REST/Plugin/Query.t
>+++ b/t/Koha/REST/Plugin/Query.t
>@@ -35,13 +35,7 @@ get '/empty' => sub {
> 
> get '/query' => sub {
>     my $c     = shift;
>-    my $input = {
>-        _page     => 2,
>-        _per_page => 3,
>-        firstname => 'Manuel',
>-        surname   => 'Cohen Arazi'
>-    };
>-    my ( $filtered_params, $reserved_params ) = $c->extract_reserved_params($input);
>+    my ( $filtered_params, $reserved_params ) = $c->extract_reserved_params($c->req->params->to_hash);
>     $c->render(
>         json => {
>             filtered_params => $filtered_params,
>@@ -51,21 +45,17 @@ get '/query' => sub {
>     );
> };
> 
>-get '/query_full' => sub {
>+get '/query_full/:id/:subid' => sub {
>     my $c     = shift;
>-    my $input = {
>-        _match    => 'exact',
>-        _order_by => 'blah',
>-        _page     => 2,
>-        _per_page => 3,
>-        firstname => 'Manuel',
>-        surname   => 'Cohen Arazi'
>-    };
>-    my ( $filtered_params, $reserved_params ) = $c->extract_reserved_params($input);
>+    my $params = $c->req->params->to_hash;
>+    $params->{id} = $c->stash->{id};
>+    $params->{subid} = $c->stash->{subid};
>+    my ( $filtered_params, $reserved_params, $path_params ) = $c->extract_reserved_params($params);
>     $c->render(
>         json => {
>             filtered_params => $filtered_params,
>-            reserved_params => $reserved_params
>+            reserved_params => $reserved_params,
>+            path_params => $path_params
>         },
>         status => 200
>     );
>@@ -202,16 +192,16 @@ use Test::Mojo;
> 
> subtest 'extract_reserved_params() tests' => sub {
> 
>-    plan tests => 8;
>+    plan tests => 9;
> 
>     my $t = Test::Mojo->new;
> 
>-    $t->get_ok('/query')->status_is(200)
>+    $t->get_ok('/query?_page=2&_per_page=3&firstname=Manuel&surname=Cohen%20Arazi')->status_is(200)
>       ->json_is( '/filtered_params' =>
>           { firstname => 'Manuel', surname => 'Cohen Arazi' } )
>       ->json_is( '/reserved_params' => { _page => 2, _per_page => 3 } );
> 
>-    $t->get_ok('/query_full')->status_is(200)
>+    $t->get_ok('/query_full/with/path?_match=exact&_order_by=blah&_page=2&_per_page=3&firstname=Manuel&surname=Cohen%20Arazi')->status_is(200)
>       ->json_is(
>         '/filtered_params' => {
>             firstname => 'Manuel',
>@@ -223,6 +213,11 @@ subtest 'extract_reserved_params() tests' => sub {
>             _per_page => 3,
>             _match    => 'exact',
>             _order_by => 'blah'
>+        } )
>+      ->json_is(
>+        '/path_params' => {
>+            id => 'with',
>+            subid => 'path'
>         } );
> 
> };
>-- 
>2.20.1

Actions: View | Diff | Splinter Review

Attachments on bug 24487: 97765 | 97772 | 97774 | 97775 | 97782 | 98029 | 98030 | 98031 | 98164 | 98165 | 98166 | 103105 | 103106 | 103107 | 103142 | 103143 | 103144