RPC: Allow to search packages by "*depends" fields

From: Baptiste Jonglez <git@bitsofnetworks.org>

It is now possible to search for packages that depend on a given package,
for instance:

    /rpc/?v=5&type=search&by=depends&arg=ocaml

It is similarly possible to match on "makedepends", "checkdepends" and
"optdepends".

Signed-off-by: Baptiste Jonglez <git@bitsofnetworks.org>
---
 doc/rpc.txt               |  8 +++++++-
 web/lib/aurjson.class.php | 33 +++++++++++++++++++++++++++++----
 2 files changed, 36 insertions(+), 5 deletions(-)

On Sun, 28 Jan 2018 at 22:27:34, Baptiste Jonglez wrote:
> From: Baptiste Jonglez <git@bitsofnetworks.org>
> 
> It is now possible to search for packages that depend on a given package,
> for instance:
> 
>     /rpc/?v=5&type=search&by=depends&arg=ocaml
> 
> It is similarly possible to match on "makedepends", "checkdepends" and
> "optdepends".
> 
> Signed-off-by: Baptiste Jonglez <git@bitsofnetworks.org>
> ---
>  doc/rpc.txt               |  8 +++++++-
>  web/lib/aurjson.class.php | 33 +++++++++++++++++++++++++++++----
>  2 files changed, 36 insertions(+), 5 deletions(-)
> 
> diff --git a/doc/rpc.txt b/doc/rpc.txt
> index f353ff0..3148ebe 100644
> --- a/doc/rpc.txt
> +++ b/doc/rpc.txt
> @@ -11,6 +11,10 @@ search argument and _field_ is one of the following values:
>  * `name` (search by package name only)
>  * `name-desc` (search by package name and description)
>  * `maintainer` (search by package maintainer)
> +* `depends` (search for packages that depend on _keywords_)
> +* `makedepends` (search for packages that makedepend on _keywords_)
> +* `optdepends` (search for packages that optdepend on _keywords_)
> +* `checkdepends` (search for packages that checkdepend on _keywords_)

Great!

> [...]
> diff --git a/web/lib/aurjson.class.php b/web/lib/aurjson.class.php
> index 9eeaafd..30bdc89 100644
> --- a/web/lib/aurjson.class.php
> +++ b/web/lib/aurjson.class.php
> @@ -17,7 +17,8 @@ class AurJSON {
>                 'suggest-pkgbase', 'get-comment-form'
>         );
>         private static $exposed_fields = array(
> -               'name', 'name-desc', 'maintainer'
> +               'name', 'name-desc', 'maintainer',
> +               'depends', 'makedepends', 'checkdepends', 'optdepends'

Just another idea: maybe we can put the new fields in a separate array
$exposed_depfields and merge that array into $exposed_fields upon
initialization. Then we would not need to hardcode them again in the
in_array() check below.

>         );
>         private static $fields_v1 = array(
>                 'Packages.ID', 'Packages.Name',
> @@ -243,16 +244,27 @@ class AurJSON {
>  
>         /*
>          * Retrieve package information (used in info, multiinfo, search and
> -        * msearch requests).
> +        * depends requests).
>          *
>          * @param $type The request type.
>          * @param $where_condition An SQL WHERE-condition to filter packages.
> +        * @param $join_depends Whether to add a SQL JOIN on the PackageDepends table.
> +        *        It will produce duplicate packages unless $where_condition filters
> +        *        the result appropriately.
>          *
>          * @return mixed Returns an array of package matches.
>          */
> -       private function process_query($type, $where_condition) {
> +       private function process_query($type, $where_condition, $join_depends=false) {
>                 $max_results = config_get_int('options', 'max_rpc_results');
>  
> +               $additional_joins = "";
> +               if ($join_depends) {
> +                       $additional_joins .= "LEFT JOIN PackageDepends " .
> +                                       "ON Packages.ID = PackageDepends.PackageID " .
> +                                       "LEFT JOIN DependencyTypes " .
> +                                       "ON PackageDepends.DepTypeID = DependencyTypes.ID";
> +               }
> +
>                 if ($this->version == 1) {
>                         $fields = implode(',', self::$fields_v1);
>                         $query = "SELECT {$fields} " .
> @@ -264,6 +276,7 @@ class AurJSON {
>                                 "ON PackageLicenses.PackageID = Packages.ID " .
>                                 "LEFT JOIN Licenses " .
>                                 "ON Licenses.ID = PackageLicenses.LicenseID " .
> +                               "${additional_joins} " .
>                                 "WHERE ${where_condition} " .
>                                 "AND PackageBases.PackagerUID IS NOT NULL " .
>                                 "LIMIT $max_results";
> @@ -278,6 +291,7 @@ class AurJSON {
>                                 "ON PackageBases.ID = Packages.PackageBaseID " .
>                                 "LEFT JOIN Users " .
>                                 "ON PackageBases.MaintainerUID = Users.ID " .
> +                               "${additional_joins} " .
>                                 "WHERE ${where_condition} " .
>                                 "AND PackageBases.PackagerUID IS NOT NULL " .
>                                 "LIMIT $max_results";
> @@ -380,6 +394,7 @@ class AurJSON {
>          * @return mixed Returns an array of package matches.
>          */
>         private function search($http_data) {
> +               $join_depends = false;
>                 $keyword_string = $http_data['arg'];
>  
>                 if (isset($http_data['by'])) {
> @@ -407,9 +422,19 @@ class AurJSON {
>                                 $keyword_string = $this->dbh->quote($keyword_string);
>                                 $where_condition = "Users.Username = $keyword_string ";
>                         }
> +               } else if (in_array($search_by, ['depends', 'makedepends', 'checkdepends', 'optdepends'])) {
> +                       if (empty($keyword_string)) {
> +                               return $this->json_error('Query arg is empty.');
> +                       } else {
> +                               $keyword_string = $this->dbh->quote($keyword_string);
> +                               $search_string = $this->dbh->quote($search_by);
> +                               $where_condition = "PackageDepends.DepName = $keyword_string AND ";
> +                               $where_condition .= "DependencyTypes.Name = $search_string";
> [...]

This is a bit unfortunate, indeed. Can we, instead of doing this, use a
subselect? Something like the following statement might work:

    $keyword_string IN (SELECT PackageDepends.DepName FROM PackageDepends
                        LEFT JOIN DependencyTypes
                        ON PackageDepends.DepTypeID = DependencyTypes.ID
                        WHERE PackageDepends.PackageID = Packages.ID
                        AND DependencyTypes.Name = $search_string)

Note that this is fully untested, though.

Best regards,
Lukas

Thanks for the new review!  I have sent a v2 PATCH, comments inline below.

On 29-01-18, Lukas Fleischer wrote:
> > diff --git a/web/lib/aurjson.class.php b/web/lib/aurjson.class.php
> > index 9eeaafd..30bdc89 100644
> > --- a/web/lib/aurjson.class.php
> > +++ b/web/lib/aurjson.class.php
> > @@ -17,7 +17,8 @@ class AurJSON {
> >                 'suggest-pkgbase', 'get-comment-form'
> >         );
> >         private static $exposed_fields = array(
> > -               'name', 'name-desc', 'maintainer'
> > +               'name', 'name-desc', 'maintainer',
> > +               'depends', 'makedepends', 'checkdepends', 'optdepends'
> 
> Just another idea: maybe we can put the new fields in a separate array
> $exposed_depfields and merge that array into $exposed_fields upon
> initialization. Then we would not need to hardcode them again in the
> in_array() check below.

I have moved the new fields to a variable $exposed_depfields, it indeed
makes things slightly easier to read.  However, I think that merging the
two arrays later in the code would be rather confusing for the reader
(because at first glance, the dependency fields would seem to be absent
from $exposed_fields).

> This is a bit unfortunate, indeed. Can we, instead of doing this, use a
> subselect? Something like the following statement might work:
> 
>     $keyword_string IN (SELECT PackageDepends.DepName FROM PackageDepends
>                         LEFT JOIN DependencyTypes
>                         ON PackageDepends.DepTypeID = DependencyTypes.ID
>                         WHERE PackageDepends.PackageID = Packages.ID
>                         AND DependencyTypes.Name = $search_string)

Excellent idea, and it even worked on the first try!  It simplifies the
code a lot, and performance-wise it should be roughly similar to a JOIN,
possibly a bit slower (from what I could read on various stack overflow
posts).

Baptiste