| Message ID | 20200706011906.7214-1-kevr.gtalk@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v3] Support conjunctive keyword search in RPC interface | expand |
Alright, just left `name` as it exists in both cases, for rpc and web search. Should be all good now, hopefully? On Sun, Jul 5, 2020 at 6:19 PM Kevin Morris <kevr.gtalk@gmail.com> wrote: > Newly supported API Version 6 modifies `type=search` for _by_ type > `name-desc`: it now behaves the same as `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-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 | 33 +++++++++++++++++++++++---------- > web/lib/pkgfuncs.inc.php | 36 +++++++++++++++++++++++------------- > 3 files changed, 50 insertions(+), 23 deletions(-) > > diff --git a/doc/rpc.txt b/doc/rpc.txt > index 3148ebea..b0f5c4e1 100644 > --- a/doc/rpc.txt > +++ b/doc/rpc.txt > @@ -39,6 +39,10 @@ Examples > `/rpc/?v=5&type=search&by=makedepends&arg=boost` > `search` with callback:: > `/rpc/?v=5&type=search&arg=foobar&callback=jsonp1192244621103` > +`search` with API Version 6 for packages containing `cookie` AND `milk`:: > + `/rpc/?v=6&type=search&arg=cookie%20milk` > +`search` with API Version 6 for packages containing `cookie milk`:: > + `/rpc/?v=6&type=search&arg="cookie milk"` > `info`:: > `/rpc/?v=5&type=info&arg[]=foobar` > `info` with multiple packages:: > diff --git a/web/lib/aurjson.class.php b/web/lib/aurjson.class.php > index 0ac586fe..b92e9094 100644 > --- a/web/lib/aurjson.class.php > +++ b/web/lib/aurjson.class.php > @@ -1,6 +1,7 @@ > <?php > > include_once("aur.inc.php"); > +include_once("pkgfuncs.inc.php"); > > /* > * This class defines a remote interface for fetching data from the AUR > using > @@ -80,7 +81,7 @@ class AurJSON { > if (isset($http_data['v'])) { > $this->version = intval($http_data['v']); > } > - if ($this->version < 1 || $this->version > 5) { > + if ($this->version < 1 || $this->version > 6) { > return $this->json_error('Invalid version > specified.'); > } > > @@ -140,7 +141,7 @@ class AurJSON { > } > > /* > - * Check if an IP needs to be rate limited. > + * Check if an IP needs to be rate limited. > * > * @param $ip IP of the current request > * > @@ -192,7 +193,7 @@ class AurJSON { > $value = get_cache_value('ratelimit-ws:' . $ip, $status); > if (!$status || ($status && $value < $deletion_time)) { > if (set_cache_value('ratelimit-ws:' . $ip, $time, > $window_length) && > - set_cache_value('ratelimit:' . $ip, 1, > $window_length)) { > + set_cache_value('ratelimit:' . $ip, 1, > $window_length)) { > return; > } > } else { > @@ -370,7 +371,7 @@ class AurJSON { > } elseif ($this->version >= 2) { > if ($this->version == 2 || $this->version == 3) { > $fields = implode(',', self::$fields_v2); > - } else if ($this->version == 4 || $this->version > == 5) { > + } else if ($this->version >= 4 && $this->version > <= 6) { > $fields = implode(',', self::$fields_v4); > } > $query = "SELECT {$fields} " . > @@ -492,13 +493,25 @@ 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)"; > - } else if ($search_by === 'name-desc') { > - $where_condition = "(Packages.Name LIKE > $keyword_string OR "; > - $where_condition .= "Description LIKE > $keyword_string)"; > + if ($this->version >= 6 && $search_by === > 'name-desc') { > + > + // API Version 6 with _by_ `name-desc`, > create a conjunctive query. > + $where_condition = > construct_keyword_search($this->dbh, > + $keyword_string, true, false); > + > + } else { > + > + $keyword_string = $this->dbh->quote( > + "%" . addcslashes($keyword_string, > '%_') . "%"); > + > + if ($search_by === 'name') { > + $where_condition = "(Packages.Name > LIKE $keyword_string)"; > + } else if ($search_by === 'name-desc') { > + $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..83d49b0f 100644 > --- a/web/lib/pkgfuncs.inc.php > +++ b/web/lib/pkgfuncs.inc.php > @@ -696,8 +696,10 @@ function pkg_search_page($params, $show_headers=true, > $SID="") { > $q_where .= "AND (PackageBases.Name LIKE " . > $dbh->quote($K) . ") "; > } > elseif (isset($params["SeB"]) && $params["SeB"] == "k") { > - /* Search by keywords. */ > - $q_where .= construct_keyword_search($dbh, > $params['K'], false); > + /* Search by name. */ > + $q_where .= "AND ("; > + $q_where .= construct_keyword_search($dbh, > $params['K'], false, true); > + $q_where .= ") "; > } > 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 .= construct_keyword_search($dbh, > $params['K'], true); > + $q_where .= "AND ("; > + $q_where .= construct_keyword_search($dbh, > $params['K'], true, true); > + $q_where .= ") "; > } > } > > @@ -833,10 +837,11 @@ function pkg_search_page($params, > $show_headers=true, $SID="") { > * @param handle $dbh Database handle > * @param string $keywords The search term > * @param bool $namedesc Search name and description fields > + * @param bool $keyword Search packages with a matching > PackageBases.Keyword > * > * @return string WHERE part of the SQL clause > */ > -function construct_keyword_search($dbh, $keywords, $namedesc) { > +function construct_keyword_search($dbh, $keywords, $namedesc, > $keyword=false) { > $count = 0; > $where_part = ""; > $q_keywords = ""; > @@ -863,11 +868,20 @@ function construct_keyword_search($dbh, $keywords, > $namedesc) { > $q_keywords .= $op . " ("; > if ($namedesc) { > $q_keywords .= "Packages.Name LIKE " . > $dbh->quote($term) . " OR "; > - $q_keywords .= "Description LIKE " . > $dbh->quote($term) . " OR "; > + $q_keywords .= "Description LIKE " . > $dbh->quote($term) . " "; > + } > + else { > + $q_keywords .= "Packages.Name LIKE " . > $dbh->quote($term) . " "; > + } > + > + if ($keyword) { > + $q_keywords .= "OR EXISTS (SELECT * FROM > PackageKeywords WHERE "; > + $q_keywords .= "PackageKeywords.PackageBaseID = > Packages.PackageBaseID AND "; > + $q_keywords .= "PackageKeywords.Keyword LIKE " . > $dbh->quote($term) . ")) "; > + } > + else { > + $q_keywords .= ") "; > } > - $q_keywords .= "EXISTS (SELECT * FROM PackageKeywords > WHERE "; > - $q_keywords .= "PackageKeywords.PackageBaseID = > Packages.PackageBaseID AND "; > - $q_keywords .= "PackageKeywords.Keyword LIKE " . > $dbh->quote($term) . ")) "; > > $count++; > if ($count >= 20) { > @@ -876,11 +890,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 > >
On Sun, 05 Jul 2020 at 21:19:06, Kevin Morris wrote: > Newly supported API Version 6 modifies `type=search` for _by_ type > `name-desc`: it now behaves the same as `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-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 | 33 +++++++++++++++++++++++---------- > web/lib/pkgfuncs.inc.php | 36 +++++++++++++++++++++++------------- > 3 files changed, 50 insertions(+), 23 deletions(-) Awesome! Pushed to pu with some minor space changes and code deduplication. Thanks!
Sweet! Thanks for all your time/help with this, Lukas. I'm growing more familiar with the codebase, I'll work on reducing the amount of patchsets I send through for stuff. Sorry about all of them. On Mon, Jul 6, 2020 at 1:36 PM Lukas Fleischer <lfleischer@archlinux.org> wrote: > On Sun, 05 Jul 2020 at 21:19:06, Kevin Morris wrote: > > Newly supported API Version 6 modifies `type=search` for _by_ type > > `name-desc`: it now behaves the same as `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-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 | 33 +++++++++++++++++++++++---------- > > web/lib/pkgfuncs.inc.php | 36 +++++++++++++++++++++++------------- > > 3 files changed, 50 insertions(+), 23 deletions(-) > > Awesome! Pushed to pu with some minor space changes and code > deduplication. Thanks! >
On Mon, 06 Jul 2020 at 16:39:59, Kevin Morris wrote: > Sweet! Thanks for all your time/help with this, Lukas. I'm growing more > familiar with the codebase, I'll work on reducing the amount of patchsets > I send through for stuff. Sorry about all of them. No worries, as long as revisions are clearly marked, such that it's easy to keep track of what the latest revision is, that's not a problem at all. One more tip: You can use `git send-email --annotate` and add a comment between the "---" separator and the diff stat which eliminates the need to send a separate email with comments/updates (that message only appears in the email, not in the applied patch): > [...] > > > Signed-off-by: Kevin Morris <kevr.gtalk@gmail.com> > > > --- Add text here. > > > doc/rpc.txt | 4 ++++ > > > web/lib/aurjson.class.php | 33 +++++++++++++++++++++++---------- > > > web/lib/pkgfuncs.inc.php | 36 +++++++++++++++++++++++------------- > > > 3 files changed, 50 insertions(+), 23 deletions(-)
diff --git a/doc/rpc.txt b/doc/rpc.txt index 3148ebea..b0f5c4e1 100644 --- a/doc/rpc.txt +++ b/doc/rpc.txt @@ -39,6 +39,10 @@ Examples `/rpc/?v=5&type=search&by=makedepends&arg=boost` `search` with callback:: `/rpc/?v=5&type=search&arg=foobar&callback=jsonp1192244621103` +`search` with API Version 6 for packages containing `cookie` AND `milk`:: + `/rpc/?v=6&type=search&arg=cookie%20milk` +`search` with API Version 6 for packages containing `cookie milk`:: + `/rpc/?v=6&type=search&arg="cookie milk"` `info`:: `/rpc/?v=5&type=info&arg[]=foobar` `info` with multiple packages:: diff --git a/web/lib/aurjson.class.php b/web/lib/aurjson.class.php index 0ac586fe..b92e9094 100644 --- a/web/lib/aurjson.class.php +++ b/web/lib/aurjson.class.php @@ -1,6 +1,7 @@ <?php include_once("aur.inc.php"); +include_once("pkgfuncs.inc.php"); /* * This class defines a remote interface for fetching data from the AUR using @@ -80,7 +81,7 @@ class AurJSON { if (isset($http_data['v'])) { $this->version = intval($http_data['v']); } - if ($this->version < 1 || $this->version > 5) { + if ($this->version < 1 || $this->version > 6) { return $this->json_error('Invalid version specified.'); } @@ -140,7 +141,7 @@ class AurJSON { } /* - * Check if an IP needs to be rate limited. + * Check if an IP needs to be rate limited. * * @param $ip IP of the current request * @@ -192,7 +193,7 @@ class AurJSON { $value = get_cache_value('ratelimit-ws:' . $ip, $status); if (!$status || ($status && $value < $deletion_time)) { if (set_cache_value('ratelimit-ws:' . $ip, $time, $window_length) && - set_cache_value('ratelimit:' . $ip, 1, $window_length)) { + set_cache_value('ratelimit:' . $ip, 1, $window_length)) { return; } } else { @@ -370,7 +371,7 @@ class AurJSON { } elseif ($this->version >= 2) { if ($this->version == 2 || $this->version == 3) { $fields = implode(',', self::$fields_v2); - } else if ($this->version == 4 || $this->version == 5) { + } else if ($this->version >= 4 && $this->version <= 6) { $fields = implode(',', self::$fields_v4); } $query = "SELECT {$fields} " . @@ -492,13 +493,25 @@ 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)"; - } else if ($search_by === 'name-desc') { - $where_condition = "(Packages.Name LIKE $keyword_string OR "; - $where_condition .= "Description LIKE $keyword_string)"; + if ($this->version >= 6 && $search_by === 'name-desc') { + + // API Version 6 with _by_ `name-desc`, create a conjunctive query. + $where_condition = construct_keyword_search($this->dbh, + $keyword_string, true, false); + + } else { + + $keyword_string = $this->dbh->quote( + "%" . addcslashes($keyword_string, '%_') . "%"); + + if ($search_by === 'name') { + $where_condition = "(Packages.Name LIKE $keyword_string)"; + } else if ($search_by === 'name-desc') { + $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..83d49b0f 100644 --- a/web/lib/pkgfuncs.inc.php +++ b/web/lib/pkgfuncs.inc.php @@ -696,8 +696,10 @@ function pkg_search_page($params, $show_headers=true, $SID="") { $q_where .= "AND (PackageBases.Name LIKE " . $dbh->quote($K) . ") "; } elseif (isset($params["SeB"]) && $params["SeB"] == "k") { - /* Search by keywords. */ - $q_where .= construct_keyword_search($dbh, $params['K'], false); + /* Search by name. */ + $q_where .= "AND ("; + $q_where .= construct_keyword_search($dbh, $params['K'], false, true); + $q_where .= ") "; } 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 .= construct_keyword_search($dbh, $params['K'], true); + $q_where .= "AND ("; + $q_where .= construct_keyword_search($dbh, $params['K'], true, true); + $q_where .= ") "; } } @@ -833,10 +837,11 @@ function pkg_search_page($params, $show_headers=true, $SID="") { * @param handle $dbh Database handle * @param string $keywords The search term * @param bool $namedesc Search name and description fields + * @param bool $keyword Search packages with a matching PackageBases.Keyword * * @return string WHERE part of the SQL clause */ -function construct_keyword_search($dbh, $keywords, $namedesc) { +function construct_keyword_search($dbh, $keywords, $namedesc, $keyword=false) { $count = 0; $where_part = ""; $q_keywords = ""; @@ -863,11 +868,20 @@ function construct_keyword_search($dbh, $keywords, $namedesc) { $q_keywords .= $op . " ("; if ($namedesc) { $q_keywords .= "Packages.Name LIKE " . $dbh->quote($term) . " OR "; - $q_keywords .= "Description LIKE " . $dbh->quote($term) . " OR "; + $q_keywords .= "Description LIKE " . $dbh->quote($term) . " "; + } + else { + $q_keywords .= "Packages.Name LIKE " . $dbh->quote($term) . " "; + } + + if ($keyword) { + $q_keywords .= "OR EXISTS (SELECT * FROM PackageKeywords WHERE "; + $q_keywords .= "PackageKeywords.PackageBaseID = Packages.PackageBaseID AND "; + $q_keywords .= "PackageKeywords.Keyword LIKE " . $dbh->quote($term) . ")) "; + } + else { + $q_keywords .= ") "; } - $q_keywords .= "EXISTS (SELECT * FROM PackageKeywords WHERE "; - $q_keywords .= "PackageKeywords.PackageBaseID = Packages.PackageBaseID AND "; - $q_keywords .= "PackageKeywords.Keyword LIKE " . $dbh->quote($term) . ")) "; $count++; if ($count >= 20) { @@ -876,11 +890,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; } /**
Newly supported API Version 6 modifies `type=search` for _by_ type `name-desc`: it now behaves the same as `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-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 | 33 +++++++++++++++++++++++---------- web/lib/pkgfuncs.inc.php | 36 +++++++++++++++++++++++------------- 3 files changed, 50 insertions(+), 23 deletions(-)