[PATCH] Bump RPC API Version 6

Lukas Fleischer lfleischer at archlinux.org
Sat Jul 4 13:05:06 UTC 2020


On Thu, 02 Jul 2020 at 23:10:40, Kevin Morris wrote:
> [...]
> Signed-off-by: Kevin Morris <kevr.gtalk at gmail.com>
> ---
>  doc/rpc.txt               |  4 +++
>  web/lib/aurjson.class.php | 66 ++++++++++++++++++++++++++++++++++-----
>  2 files changed, 62 insertions(+), 8 deletions(-)

Thanks Kevin! A few comments below.

> diff --git a/doc/rpc.txt b/doc/rpc.txt
> index 3148ebea..b0f5c4e1 100644
> @@ -472,6 +472,33 @@ class AurJSON {
>                 return array('ids' => $id_args, 'names' => $name_args);
>         }
>  
> +       /*
> +        * Prepare a WHERE statement for each keyword: append $func($keyword)
> +        * separated by $delim. Each keyword is sanitized as wildcards before
> +        * it's passed to $func.

What does that last part of the comment mean? Doesn't sanitization
happen in $func itself?

> +        *
> +        * @param $delim Delimiter to use in the middle of two keywords.
> +        * @param $keywords Array of keywords to prepare.
> +        * @param $func A function that returns a string. This value is concatenated.
> +        *
> +        * @return A WHERE condition statement of keywords separated by $delim.
> +        */
> +       private function join_where($delim, $keywords, $func) {
> +               // Applied to each item to concatenate our entire statement.
> +               $reduce_func = function($carry, $item) use(&$func) {
> +                       array_push($carry, $func($item));
> +                       return $carry;
> +               };
> +
> +               // Manual array_reduce with a local lambda.
> +               $acc = array(); // Initial
> +               foreach ($keywords as &$keyword) {
> +                       $acc += $reduce_func($acc, $keyword);

I am slightly puzzled by the implementation here; why is there a need to
have array_push() above and also use the += operator? Can't we simply
call $func() directly and use array_push() here?

> +               }
> +
> +               return join(" $delim ", $acc);

It might make sense to just use $delim here and use " AND " in the
caller (I actually skipped this function on my first read and was asking
myself why the caller doesn't put spaces around "AND", that's the
behavior people expect from a join function). Either that or rename the
function slightly; in the latter case it could even make sense to drop
the parameter altogether and hardcode the AND, unless you think the
function could be used for something else in the future.


More information about the aur-dev mailing list