Re: [PATCH] Support conjunctive keyword search in RPC interface
Alright, patch sent; I used `-v1` this time with `git send-email`... I
can't find documentation for that switch yet. I've tested basic search
through both paths; the only refactoring that needed to be done was to
remove the extra "AND (" and ")" from the generic function, and add it
where we need it in `pkg_search_page`. Then, we can reuse the same
`construct_keyword_search` in rpc.
[PATCH] Support conjunctive keyword search in RPC interface
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=search="blah blah"%20haha
Search for packages containing the literal keyword `abc 123`:
https://aur.archlinux.org/rpc/?v=6=search="abc 123"
The following example searches for packages that contain `blah` AND `abc`:
https://aur.archlinux.org/rpc/?v=6=search=blah%20abc
The legacy method still searches for packages that contain `blah abc`:
https://aur.archlinux.org/rpc/?v=5=search=blah%20abc
https://aur.archlinux.org/rpc/?v=5=search=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
---
doc/rpc.txt | 4
web/lib/aurjson.class.php | 29 +
web/lib/pkgfuncs.inc.php | 10 +-
3 files changed, 30 insertions(+), 13 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=search=makedepends=boost`
`search` with callback::
`/rpc/?v=5=search=foobar=jsonp1192244621103`
+`search` with API Version 6 for packages containing `cookie` AND `milk`::
+ `/rpc/?v=6=search=cookie%20milk`
+`search` with API Version 6 for packages containing `cookie milk`::
+ `/rpc/?v=6=search="cookie milk"`
`info`::
`/rpc/?v=5=info[]=foobar`
`info` with multiple packages::
diff --git a/web/lib/aurjson.class.php b/web/lib/aurjson.class.php
index 0ac586fe..83ce502a 100644
--- a/web/lib/aurjson.class.php
+++ b/web/lib/aurjson.class.php
@@ -80,7 +80,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 +140,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 +192,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 +370,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 +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);
+ } 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) {
+
Re: [PATCH] Bump RPC API Version 6
On Sat, 04 Jul 2020 at 20:52:09, Kevin Morris wrote: > Awesome! I do like the sound better of using the same exact search sql for > both RPC and Web, and so your suggestion seems like a great idea to me. > I'll start looking at this today! Sounds great, I am looking forward to a new patch. And sorry for not thinking this through more carefully earlier!
Re: [PATCH] Bump RPC API Version 6
Hey Lukas, Awesome! I do like the sound better of using the same exact search sql for both RPC and Web, and so your suggestion seems like a great idea to me. I'll start looking at this today! On Sat, Jul 4, 2020 at 5:30 PM Lukas Fleischer wrote: > On Sat, 04 Jul 2020 at 17:14:26, Kevin Morris wrote: > > API Version 6 modifies `type=search` functionality: Split `arg` into > > keywords separated by spaces (quoted keywords are preserved). > > [...] > > Signed-off-by: Kevin Morris > > --- > > doc/rpc.txt | 4 +++ > > web/lib/aurjson.class.php | 60 +-- > > 2 files changed, 56 insertions(+), 8 deletions(-) > > Thanks for the quick revision. Looks great now, I merged this as > > Support conjunctive keyword search in RPC interface > > to pu (I think it is better to refer to the new feature in the subject > line and put aside "Bump RPC API Version" for patches that just bump the > API version in case it was forgotten in a previous patch). > > Now that this is implemented, I just wanted to suggest implementing the > same functionality for the web interface until I noticed that we already > do and it's even more powerful: we additionally support "OR" and "NOT" > there; see construct_keyword_search() in pkgfuncs.inc.php. Would you be > open to working on another revision of this patch that refactors > construct_keyword_search() a little bit (to make it usable for the RPC > interface) and then uses that for API version 6 instead? > -- Kevin Morris Software Developer Personal Inquiries: kevr.gt...@gmail.com Personal Phone: (415) 583-9687 Technologies: C, C++, Python, Django, Ruby, Rails, ReactJS, jQuery, Javascript, SQL, Redux
Re: [PATCH] Bump RPC API Version 6
On Sat, 04 Jul 2020 at 17:14:26, Kevin Morris wrote: > API Version 6 modifies `type=search` functionality: Split `arg` into > keywords separated by spaces (quoted keywords are preserved). > [...] > Signed-off-by: Kevin Morris > --- > doc/rpc.txt | 4 +++ > web/lib/aurjson.class.php | 60 +-- > 2 files changed, 56 insertions(+), 8 deletions(-) Thanks for the quick revision. Looks great now, I merged this as Support conjunctive keyword search in RPC interface to pu (I think it is better to refer to the new feature in the subject line and put aside "Bump RPC API Version" for patches that just bump the API version in case it was forgotten in a previous patch). Now that this is implemented, I just wanted to suggest implementing the same functionality for the web interface until I noticed that we already do and it's even more powerful: we additionally support "OR" and "NOT" there; see construct_keyword_search() in pkgfuncs.inc.php. Would you be open to working on another revision of this patch that refactors construct_keyword_search() a little bit (to make it usable for the RPC interface) and then uses that for API version 6 instead?
Re: [PATCH] Exclude suspended Users from being notified
No problem, Lukas! Yeah, basically all my patches except the most recent one are obsolete.. pretty new to git send-email, wasn't sure about that -- I'll check out -v for future submissions. Thanks for the tip! On Sat, Jul 4, 2020 at 5:50 AM Lukas Fleischer wrote: > On Fri, 03 Jul 2020 at 21:29:16, Kevin Morris wrote: > > The existing notify.py script was grabbing entries regardless > > of user suspension. This has been modified to only send notifications > > to unsuspended users. > > > > This change was written as a solution to > > https://bugs.archlinux.org/task/65554. > > > > Signed-off-by: Kevin Morris > > --- > > aurweb/scripts/notify.py | 31 --- > > 1 file changed, 20 insertions(+), 11 deletions(-) > > Merged into pu, thanks Kevin! From your emails, I assume that all other > patches in this thread are obsolete? I recommend using the -v option of > git-send-email (e.g. -v2, -v3, ...) to mark revisions of the same patch. > -- Kevin Morris Software Developer Personal Inquiries: kevr.gt...@gmail.com Personal Phone: (415) 583-9687 Technologies: C, C++, Python, Django, Ruby, Rails, ReactJS, jQuery, Javascript, SQL, Redux
[PATCH] Bump RPC API Version 6
API Version 6 modifies `type=search` functionality: Split `arg` into
keywords separated by spaces (quoted keywords are preserved).
Search for packages containing the literal keyword `blah blah` AND `haha`:
https://aur.archlinux.org/rpc/?v=6=search="blah blah"%20haha
Search for packages containing the literal keyword `abc 123`:
https://aur.archlinux.org/rpc/?v=6=search="abc 123"
The following example searches for packages that contain `blah` AND `abc`:
https://aur.archlinux.org/rpc/?v=6=search=blah%20abc
The legacy method still searches for packages that contain 'blah abc':
https://aur.archlinux.org/rpc/?v=5=search=blah%20abc
https://aur.archlinux.org/rpc/?v=5=search=blah%20abc
API Version 6 is 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
---
doc/rpc.txt | 4 +++
web/lib/aurjson.class.php | 60 +--
2 files changed, 56 insertions(+), 8 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=search=makedepends=boost`
`search` with callback::
`/rpc/?v=5=search=foobar=jsonp1192244621103`
+`search` with API Version 6 for packages containing `cookie` AND `milk`::
+ `/rpc/?v=6=search=cookie%20milk`
+`search` with API Version 6 for packages containing `cookie milk`::
+ `/rpc/?v=6=search="cookie milk"`
`info`::
`/rpc/?v=5=info[]=foobar`
`info` with multiple packages::
diff --git a/web/lib/aurjson.class.php b/web/lib/aurjson.class.php
index 0ac586fe..b639653f 100644
--- a/web/lib/aurjson.class.php
+++ b/web/lib/aurjson.class.php
@@ -80,7 +80,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 +140,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 +192,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 +370,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} " .
@@ -472,6 +472,27 @@ class AurJSON {
return array('ids' => $id_args, 'names' => $name_args);
}
+ /*
+* Prepare a WHERE statement for each $transform($keyword), separated
+* by $delim.
+*
+* @param $delim Delimiter used to join several keywords.
+* @param $keywords Array of keywords.
+* @param $transform Transformation function that receives a keyword
+* and returns a transformed string.
+*
+* @return string WHERE condition statement of transformed keywords
+* separated by $delim.
+*/
+ private function join_where($delim, $keywords, $transform) {
+ $reduce_func = function($carry, $item) use(&$transform) {
+ array_push($carry, $transform($item));
+ return $carry;
+ };
+ $result = array_reduce($keywords, $reduce_func, array());
+ return join($delim, $result);
+ }
+
/*
* Performs a fulltext mysql search of the package database.
*
@@ -492,13 +513,36 @@ class AurJSON {
if (strlen($keyword_string) < 2) {
return $this->json_error('Query arg too
small.');
}
- $keyword_string = $this->dbh->quote("%" .
addcslashes($keyword_string, '%_') . "%");
Re: [PATCH] Bump RPC API Version 6
Thanks for the look/review Lukas; fixed up some things in the code that
clears all these points up -- the comments for `join_where` were outdated,
forgot to update it -- renamed some variables and swapped to `array_reduce`
instead of the manual loop. Now that I can see, there's no reason that it
wasn't `array_reduce` before other than some of my personal confusion with
php and it's error messages.
`$delim` is fixed up so the caller just includes spaces if they want like
you thought -- and i do like it more that it's standardized.
I'd like to leave join_where there because I use it in two places and
didn't really want to repeat the logic -- I'm also foreseeing that it can
be used later to create more WHERE statements out of lists.
(Hopefully) final patch being submitted within a few minutes.
On Sat, Jul 4, 2020 at 6:05 AM Lukas Fleischer
wrote:
> On Thu, 02 Jul 2020 at 23:10:40, Kevin Morris wrote:
> > [...]
> > Signed-off-by: Kevin Morris
> > ---
> > 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.
>
--
Kevin Morris
Software Developer
Personal Inquiries: kevr.gt...@gmail.com
Personal Phone: (415) 583-9687
Technologies: C, C++, Python, Django, Ruby, Rails, ReactJS, jQuery,
Javascript, SQL, Redux
Re: [PATCH] Bump RPC API Version 6
On Thu, 02 Jul 2020 at 23:10:40, Kevin Morris wrote:
> [...]
> Signed-off-by: Kevin Morris
> ---
> 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.
Re: [PATCH] Exclude suspended Users from being notified
On Fri, 03 Jul 2020 at 21:29:16, Kevin Morris wrote: > The existing notify.py script was grabbing entries regardless > of user suspension. This has been modified to only send notifications > to unsuspended users. > > This change was written as a solution to > https://bugs.archlinux.org/task/65554. > > Signed-off-by: Kevin Morris > --- > aurweb/scripts/notify.py | 31 --- > 1 file changed, 20 insertions(+), 11 deletions(-) Merged into pu, thanks Kevin! From your emails, I assume that all other patches in this thread are obsolete? I recommend using the -v option of git-send-email (e.g. -v2, -v3, ...) to mark revisions of the same patch.
