Re: [PATCH] Support conjunctive keyword search in RPC interface

5 Jul 2020

On Sun, 05 Jul 2020 at 01:10:07, Kevin Morris wrote:
...
Newly supported API Version 6 modifies `type=search` functionality; it
now behaves the same as `name` or `name-desc` search through the
https://aur.archlinux.org/packages/ search page.
Search for packages containing the literal keyword `blah blah` AND `haha`:
https://aur.archlinux.org/rpc/?v=6&type=search&arg="blah blah"%20haha
Search for packages containing the literal keyword `abc 123`:
https://aur.archlinux.org/rpc/?v=6&type=search&arg="abc 123"
The following example searches for packages that contain `blah` AND `abc`:
https://aur.archlinux.org/rpc/?v=6&type=search&arg=blah%20abc
The legacy method still searches for packages that contain `blah abc`:
https://aur.archlinux.org/rpc/?v=5&type=search&arg=blah%20abc
https://aur.archlinux.org/rpc/?v=5&type=search&arg=blah%20abc
API Version 6 is currently only considered during a `search` of `name` or
`name-desc`.
Note: This change was written as a solution to
https://bugs.archlinux.org/task/49133.
PS: + Some spacing issues fixed in comments.
Signed-off-by: Kevin Morris <kevr.gtalk@gmail.com>
---
 doc/rpc.txt               |  4 ++++
 web/lib/aurjson.class.php | 29 +++++++++++++++++++++--------
 web/lib/pkgfuncs.inc.php  | 10 +++++-----
 3 files changed, 30 insertions(+), 13 deletions(-)
[...]
@@ -492,13 +492,26 @@ class AurJSON {
                        if (strlen($keyword_string) < 2) {
                                return $this->json_error('Query arg too small.');
                        }
-                       $keyword_string = $this->dbh->quote("%" . addcslashes($keyword_string, '%_') . "%");
if ($search_by === 'name') {
-                               $where_condition = "(Packages.Name LIKE $keyword_string)";
+                               if ($this->version >= 6) {
+                                       $where_condition = construct_keyword_search($this->dbh,
+                                               $keyword_string, false);
Do name and name-desc now behave exactly the same (see below)? If so,
this change in behavior should be documented at least. I would have
expected some more refactoring in construct_keyword_search() and one
additional parameter being passed here though.

Should we include pkgfuncs.inc.php from aurjson.class.php now? How does
this currently get imported?

Also, since the code for both name and name-desc are now very similar,
can we refactor and share most of the code instead of copy pasting? To
this end, it might be easier to switch the blocks (i.e. check for API
version first, then check for request type). That would allow us to
reuse the same assignment to $keyword_string as before and possibly the
same construct_keyword_search() invocation too.
...
+                               } else {
+                                       $keyword_string = $this->dbh->quote(
+                                               "%" . addcslashes($keyword_string, '%_') . "%");
+                                       $where_condition = "(Packages.Name LIKE $keyword_string)";
+                               }
                        } else if ($search_by === 'name-desc') {
-                               $where_condition = "(Packages.Name LIKE $keyword_string OR ";
-                               $where_condition .= "Description LIKE $keyword_string)";
+                               if ($this->version >= 6) {
+                                       $where_condition = construct_keyword_search($this->dbh,
+                                               $keyword_string, true);
See above.
...
+                               } else {
+                                       $keyword_string = $this->dbh->quote(
+                                               "%" . addcslashes($keyword_string, '%_') . "%");
+                                       $where_condition = "(Packages.Name LIKE $keyword_string ";
+                                       $where_condition .= "OR Description LIKE $keyword_string)";
+                               }
                        }
                } else if ($search_by === 'maintainer') {
                        if (empty($keyword_string)) {

diff --git a/web/lib/pkgfuncs.inc.php b/web/lib/pkgfuncs.inc.php
index 8c915711..f6108e5a 100644
--- a/web/lib/pkgfuncs.inc.php
+++ b/web/lib/pkgfuncs.inc.php
@@ -697,7 +697,9 @@ function pkg_search_page($params, $show_headers=true, $SID="") {
                }
                elseif (isset($params["SeB"]) && $params["SeB"] == "k") {
                        /* Search by keywords. */
+                       $q_where .= " AND ( ";
Do we need the space at the end?
...
$q_where .= construct_keyword_search($dbh, $params['K'], false);
+                       $q_where .= " )";
Same here, do we need the space before the closing parentheses? More
importantly though, do we want to add a space after the parenthesis
instead to match the previous behavior of SQL strings always ending with
a space, so we can immediately append without a space?
...
}
                elseif (isset($params["SeB"]) && $params["SeB"] == "N") {
                        /* Search by name (exact match). */
@@ -709,7 +711,9 @@ function pkg_search_page($params, $show_headers=true, $SID="") {
                }
                else {
                        /* Keyword search (default). */
+                       $q_where .= " AND ( ";
                        $q_where .= construct_keyword_search($dbh, $params['K'], true);
+                       $q_where .= " )";
Same here.
...
}
        }
@@ -876,11 +880,7 @@ function construct_keyword_search($dbh, $keywords, $namedesc) {
                $op = "AND ";
        }
-       if (!empty($q_keywords)) {
-               $where_part = "AND (" . $q_keywords . ") ";
-       }
-
-       return $where_part;
+       return $q_keywords;
 }
/**
-- 
2.20.1

    

Re: [PATCH] Support conjunctive keyword search in RPC interface

Lukas Fleischer