[PATCH v2] Exclude suspended Users from being notified
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 --- The issue was in OwnershipEventNotification: `Users.Suspended = 0` was included in the bottom-most SQL fetch, which is not a fetch of the recipients. That part of the stanza was removed, and `Users.Suspended = 0` is only ever used when fetching recipients for notifications. aurweb/scripts/notify.py | 28 ++-- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/aurweb/scripts/notify.py b/aurweb/scripts/notify.py index 5b18a476..644a4b1f 100755 --- a/aurweb/scripts/notify.py +++ b/aurweb/scripts/notify.py @@ -124,7 +124,7 @@ class ResetKeyNotification(Notification): def __init__(self, conn, uid): cur = conn.execute('SELECT UserName, Email, BackupEmail, ' + 'LangPreference, ResetKey ' + - 'FROM Users WHERE ID = ?', [uid]) + 'FROM Users WHERE ID = ? AND Suspended = 0', [uid]) self._username, self._to, self._backup, self._lang, self._resetkey = cur.fetchone() super().__init__() @@ -171,7 +171,8 @@ class CommentNotification(Notification): 'ON PackageNotifications.UserID = Users.ID WHERE ' + 'Users.CommentNotify = 1 AND ' + 'PackageNotifications.UserID != ? AND ' + - 'PackageNotifications.PackageBaseID = ?', + 'PackageNotifications.PackageBaseID = ? AND ' + + 'Users.Suspended = 0', [uid, pkgbase_id]) self._recipients = cur.fetchall() cur = conn.execute('SELECT Comments FROM PackageComments WHERE ID = ?', @@ -218,7 +219,8 @@ class UpdateNotification(Notification): 'ON PackageNotifications.UserID = Users.ID WHERE ' + 'Users.UpdateNotify = 1 AND ' + 'PackageNotifications.UserID != ? AND ' + - 'PackageNotifications.PackageBaseID = ?', + 'PackageNotifications.PackageBaseID = ? AND ' + + 'Users.Suspended = 0', [uid, pkgbase_id]) self._recipients = cur.fetchall() super().__init__() @@ -264,7 +266,8 @@ class FlagNotification(Notification): 'INNER JOIN PackageBases ' + 'ON PackageBases.MaintainerUID = Users.ID OR ' + 'PackageBases.ID = PackageComaintainers.PackageBaseID ' + - 'WHERE PackageBases.ID = ?', [pkgbase_id]) + 'WHERE PackageBases.ID = ? AND ' + + 'Users.Suspended = 0', [pkgbase_id]) self._recipients = cur.fetchall() cur = conn.execute('SELECT FlaggerComment FROM PackageBases WHERE ' + 'ID = ?', [pkgbase_id]) @@ -302,7 +305,8 @@ class OwnershipEventNotification(Notification): 'ON PackageNotifications.UserID = Users.ID WHERE ' + 'Users.OwnershipNotify = 1 AND ' + 'PackageNotifications.UserID != ? AND ' + - 'PackageNotifications.PackageBaseID = ?', + 'PackageNotifications.PackageBaseID = ? AND ' + + 'Users.Suspended = 0', [uid, pkgbase_id]) self._recipients = cur.fetchall() cur = conn.execute('SELECT FlaggerComment FROM PackageBases WHERE ' + @@ -341,7 +345,7 @@ class ComaintainershipEventNotification(Notification): def __init__(self, conn, uid, pkgbase_id): self._pkgbase = pkgbase_from_id(conn, pkgbase_id) cur = conn.execute('SELECT Email, LangPreference FROM Users ' + - 'WHERE ID = ?', [uid]) + 'WHERE ID = ? AND Suspended = 0', [uid]) self._to, self._lang = cur.fetchone() super().__init__() @@ -384,7 +388,8 @@ class DeleteNotification(Notification): 'INNER JOIN PackageNotifications ' + 'ON PackageNotifications.UserID = Users.ID WHERE ' + 'PackageNotifications.UserID != ? AND ' + - 'PackageNotifica
Re: [PATCH] Exclude suspended Users from being notified
Okay -- I'm going to need some time to study/research how these tests work before I can confidently figure out what's going wrong. I'm on it, not sure how long it'll take me. On Wed, Jul 15, 2020 at 8:40 AM Kevin Morris wrote: > [Re-post to aur-dev] > > Yeah. Was planning to, had a bit too much on my plate to get started when > I saw Frederic's first message. I have a bit of an issue at this point -- > on my dev system, I don't run arch on my current dev machine, and > test/README.md seems to hint that this is an issue (dep: libalpm). I've got > an arch VM setup, so I'll test it out in there. I'll reply with findings in > some hours today. > > On Tue, Jul 14, 2020 at 3:33 PM Lukas Fleischer > wrote: > >> On Tue, 14 Jul 2020 at 18:25:04, Lukas Fleischer wrote: >> > On Mon, 13 Jul 2020 at 10:47:03, Frédéric Mangano-Tarumi wrote: >> > > In that last line, `+,` is not a valid Python syntax. The test suite >> is >> > > screaming. >> > >> > Thanks for the pointer Frédéric, I amended the patch. >> >> Seems like some tests are still failing, even after fixing the typo. >> Kevin, could you please have another look? >> > > > -- > 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 > -- 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] Exclude suspended Users from being notified
[Re-post to aur-dev] Yeah. Was planning to, had a bit too much on my plate to get started when I saw Frederic's first message. I have a bit of an issue at this point -- on my dev system, I don't run arch on my current dev machine, and test/README.md seems to hint that this is an issue (dep: libalpm). I've got an arch VM setup, so I'll test it out in there. I'll reply with findings in some hours today. On Tue, Jul 14, 2020 at 3:33 PM Lukas Fleischer wrote: > On Tue, 14 Jul 2020 at 18:25:04, Lukas Fleischer wrote: > > On Mon, 13 Jul 2020 at 10:47:03, Frédéric Mangano-Tarumi wrote: > > > In that last line, `+,` is not a valid Python syntax. The test suite is > > > screaming. > > > > Thanks for the pointer Frédéric, I amended the patch. > > Seems like some tests are still failing, even after fixing the typo. > Kevin, could you please have another look? > -- 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 v3] Support conjunctive keyword search in RPC interface
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 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 > > --- > > 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! > -- 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 v3] Support conjunctive keyword search in RPC interface
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 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 > --- > 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 @@ > > 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
[PATCH v3] Support conjunctive keyword search in RPC interface
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 --- 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 @@ 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 que
Re: [PATCH v2] Support conjunctive keyword search in RPC interface
Uh, unsure if adding conjunctive to `name` search in the search page is what you want? I figured that made sense for both name and name-desc, like it does on rpc currently... Thoughts? On Sun, Jul 5, 2020 at 5:57 PM 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`. > > Additionally, this change adds support for conjuctive search when > searching by > `name` in https://aur.archlinux.org/packages/. > > 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 | 34 ++-- > web/lib/pkgfuncs.inc.php | 41 +-- > 3 files changed, 54 insertions(+), 25 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..da1af9be 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,27 @@ class AurJSON { > if (strlen($keyword_string)
[PATCH v2] 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&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`. Additionally, this change adds support for conjuctive search when searching by `name` in https://aur.archlinux.org/packages/. 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 | 34 ++-- web/lib/pkgfuncs.inc.php | 41 +-- 3 files changed, 54 insertions(+), 25 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..da1af9be 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,27 @@ 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 .= "Descrip
Re: [PATCH] Support conjunctive keyword search in RPC interface
Okay, I was totally mistaken - `$keyword_string` is only ever sanitized in the right place (name/name-desc) in master. Yeah, I can rework this to be a bit cleaner as you recommended -- no problem. ++ on this, will be included in next rev. On Sun, Jul 5, 2020 at 11:51 AM Kevin Morris wrote: > 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. >> > > I noticed an issue with pre-sanitizing $keyword_string as per `master` -- > when searching by maintainer in rpc, a WHERE Username = $keyword_string is > used without wildcards (%...%). As far as I know, wildcards should only be > used in a LIKE expression? Could be wrong, but that's what I thought when > originally modifying `aurjson.inc.php`'s `function search`. Thoughts? > > On Sun, Jul 5, 2020 at 6:33 AM Lukas Fleischer > wrote: > >> On Sun, 05 Jul 2020 at 01:13:25, Kevin Morris wrote: >> > 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. >> >> Thanks for the revision! I added some comments inline. >> >> It's common to not use any versioning for the first revision of a patch >> (e.g. send without -v1) but then start using (-v2, -v3, ...) for >> followup revisions. The -v argument is documented in the >> git-format-patch(1) man page, if you are using git-format-patch and >> git-send-email separately, you need to specify the argument when >> formatting the patch (however, in most cases, you can also run `git >> format patch` directly). >> > > > -- > 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 > -- 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] Support conjunctive keyword search in RPC interface
> > 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? > This one can't hurt at all, the only thing it'd do is make construct_keyword_search more flexible. ++ on this, change will be in the next rev. On Sun, Jul 5, 2020 at 11:51 AM Kevin Morris wrote: > 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. >> > > I noticed an issue with pre-sanitizing $keyword_string as per `master` -- > when searching by maintainer in rpc, a WHERE Username = $keyword_string is > used without wildcards (%...%). As far as I know, wildcards should only be > used in a LIKE expression? Could be wrong, but that's what I thought when > originally modifying `aurjson.inc.php`'s `function search`. Thoughts? > > On Sun, Jul 5, 2020 at 6:33 AM Lukas Fleischer > wrote: > >> On Sun, 05 Jul 2020 at 01:13:25, Kevin Morris wrote: >> > 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. >> >> Thanks for the revision! I added some comments inline. >> >> It's common to not use any versioning for the first revision of a patch >> (e.g. send without -v1) but then start using (-v2, -v3, ...) for >> followup revisions. The -v argument is documented in the >> git-format-patch(1) man page, if you are using git-format-patch and >> git-send-email separately, you need to specify the argument when >> formatting the patch (however, in most cases, you can also run `git >> format patch` directly). >> > > > -- > 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 > -- 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] Support conjunctive keyword search in RPC interface
> > 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. > I noticed an issue with pre-sanitizing $keyword_string as per `master` -- when searching by maintainer in rpc, a WHERE Username = $keyword_string is used without wildcards (%...%). As far as I know, wildcards should only be used in a LIKE expression? Could be wrong, but that's what I thought when originally modifying `aurjson.inc.php`'s `function search`. Thoughts? On Sun, Jul 5, 2020 at 6:33 AM Lukas Fleischer wrote: > On Sun, 05 Jul 2020 at 01:13:25, Kevin Morris wrote: > > 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. > > Thanks for the revision! I added some comments inline. > > It's common to not use any versioning for the first revision of a patch > (e.g. send without -v1) but then start using (-v2, -v3, ...) for > followup revisions. The -v argument is documented in the > git-format-patch(1) man page, if you are using git-format-patch and > git-send-email separately, you need to specify the argument when > formatting the patch (however, in most cases, you can also run `git > format patch` directly). > -- 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] 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&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 --- 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&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..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( + "%" . addc
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] 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&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 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&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..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, $re
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] Exclude suspended Users from being notified
Alright, that's the final patch revision. This change ultimately just removes suspended users from the sql results in `notify.py`, which excludes them from all email notifications. On Fri, Jul 3, 2020 at 6:29 PM 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(-) > > diff --git a/aurweb/scripts/notify.py b/aurweb/scripts/notify.py > index 5b18a476..223ed61f 100755 > --- a/aurweb/scripts/notify.py > +++ b/aurweb/scripts/notify.py > @@ -124,7 +124,7 @@ class ResetKeyNotification(Notification): > def __init__(self, conn, uid): > cur = conn.execute('SELECT UserName, Email, BackupEmail, ' + > 'LangPreference, ResetKey ' + > - 'FROM Users WHERE ID = ?', [uid]) > + 'FROM Users WHERE ID = ? AND Suspended = 0', > [uid]) > self._username, self._to, self._backup, self._lang, > self._resetkey = cur.fetchone() > super().__init__() > > @@ -171,7 +171,8 @@ class CommentNotification(Notification): > 'ON PackageNotifications.UserID = Users.ID > WHERE ' + > 'Users.CommentNotify = 1 AND ' + > 'PackageNotifications.UserID != ? AND ' + > - 'PackageNotifications.PackageBaseID = ?', > + 'PackageNotifications.PackageBaseID = ? AND ' + > + 'Users.Suspended = 0' +, > [uid, pkgbase_id]) > self._recipients = cur.fetchall() > cur = conn.execute('SELECT Comments FROM PackageComments WHERE ID > = ?', > @@ -218,7 +219,8 @@ class UpdateNotification(Notification): > 'ON PackageNotifications.UserID = Users.ID > WHERE ' + > 'Users.UpdateNotify = 1 AND ' + > 'PackageNotifications.UserID != ? AND ' + > - 'PackageNotifications.PackageBaseID = ?', > + 'PackageNotifications.PackageBaseID = ? AND ' + > + 'Users.Suspended = 0', > [uid, pkgbase_id]) > self._recipients = cur.fetchall() > super().__init__() > @@ -264,7 +266,8 @@ class FlagNotification(Notification): > 'INNER JOIN PackageBases ' + > 'ON PackageBases.MaintainerUID = Users.ID OR ' > + > 'PackageBases.ID = > PackageComaintainers.PackageBaseID ' + > - 'WHERE PackageBases.ID = ?', [pkgbase_id]) > + 'WHERE PackageBases.ID = ? AND ' + > + 'Users.Suspended = 0', [pkgbase_id]) > self._recipients = cur.fetchall() > cur = conn.execute('SELECT FlaggerComment FROM PackageBases WHERE > ' + > 'ID = ?', [pkgbase_id]) > @@ -302,7 +305,8 @@ class OwnershipEventNotification(Notification): > 'ON PackageNotifications.UserID = Users.ID > WHERE ' + > 'Users.OwnershipNotify = 1 AND ' + > 'PackageNotifications.UserID != ? AND ' + > - 'PackageNotifications.PackageBaseID = ?', > + 'PackageNotifications.PackageBaseID = ? AND ' + > + 'Users.Suspended = 0', > [uid, pkgbase_id]) > self._recipients = cur.fetchall() > cur = conn.execute('SELECT FlaggerComment FROM PackageBases WHERE > ' + > @@ -341,7 +345,7 @@ class ComaintainershipEventNotification(Notification): > def __init__(self, conn, uid, pkgbase_id): > self._pkgbase = pkgbase_from_id(conn, pkgbase_id) > cur = conn.execute('SELECT Email, LangPreference FROM Users ' + > - 'WHERE ID = ?', [uid]) > + 'WHERE ID = ? AND Suspended = 0', [uid]) > self._to, self._lang = cur.fetchone() > s
[PATCH] Exclude suspended Users from being notified
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(-) diff --git a/aurweb/scripts/notify.py b/aurweb/scripts/notify.py index 5b18a476..223ed61f 100755 --- a/aurweb/scripts/notify.py +++ b/aurweb/scripts/notify.py @@ -124,7 +124,7 @@ class ResetKeyNotification(Notification): def __init__(self, conn, uid): cur = conn.execute('SELECT UserName, Email, BackupEmail, ' + 'LangPreference, ResetKey ' + - 'FROM Users WHERE ID = ?', [uid]) + 'FROM Users WHERE ID = ? AND Suspended = 0', [uid]) self._username, self._to, self._backup, self._lang, self._resetkey = cur.fetchone() super().__init__() @@ -171,7 +171,8 @@ class CommentNotification(Notification): 'ON PackageNotifications.UserID = Users.ID WHERE ' + 'Users.CommentNotify = 1 AND ' + 'PackageNotifications.UserID != ? AND ' + - 'PackageNotifications.PackageBaseID = ?', + 'PackageNotifications.PackageBaseID = ? AND ' + + 'Users.Suspended = 0' +, [uid, pkgbase_id]) self._recipients = cur.fetchall() cur = conn.execute('SELECT Comments FROM PackageComments WHERE ID = ?', @@ -218,7 +219,8 @@ class UpdateNotification(Notification): 'ON PackageNotifications.UserID = Users.ID WHERE ' + 'Users.UpdateNotify = 1 AND ' + 'PackageNotifications.UserID != ? AND ' + - 'PackageNotifications.PackageBaseID = ?', + 'PackageNotifications.PackageBaseID = ? AND ' + + 'Users.Suspended = 0', [uid, pkgbase_id]) self._recipients = cur.fetchall() super().__init__() @@ -264,7 +266,8 @@ class FlagNotification(Notification): 'INNER JOIN PackageBases ' + 'ON PackageBases.MaintainerUID = Users.ID OR ' + 'PackageBases.ID = PackageComaintainers.PackageBaseID ' + - 'WHERE PackageBases.ID = ?', [pkgbase_id]) + 'WHERE PackageBases.ID = ? AND ' + + 'Users.Suspended = 0', [pkgbase_id]) self._recipients = cur.fetchall() cur = conn.execute('SELECT FlaggerComment FROM PackageBases WHERE ' + 'ID = ?', [pkgbase_id]) @@ -302,7 +305,8 @@ class OwnershipEventNotification(Notification): 'ON PackageNotifications.UserID = Users.ID WHERE ' + 'Users.OwnershipNotify = 1 AND ' + 'PackageNotifications.UserID != ? AND ' + - 'PackageNotifications.PackageBaseID = ?', + 'PackageNotifications.PackageBaseID = ? AND ' + + 'Users.Suspended = 0', [uid, pkgbase_id]) self._recipients = cur.fetchall() cur = conn.execute('SELECT FlaggerComment FROM PackageBases WHERE ' + @@ -341,7 +345,7 @@ class ComaintainershipEventNotification(Notification): def __init__(self, conn, uid, pkgbase_id): self._pkgbase = pkgbase_from_id(conn, pkgbase_id) cur = conn.execute('SELECT Email, LangPreference FROM Users ' + - 'WHERE ID = ?', [uid]) + 'WHERE ID = ? AND Suspended = 0', [uid]) self._to, self._lang = cur.fetchone() super().__init__() @@ -384,7 +388,8 @@ class DeleteNotification(Notification): 'INNER JOIN PackageNotifications ' + 'ON PackageNotifications.UserID = Users.ID WHERE ' + 'PackageNotifications.UserID != ? AND ' + - 'PackageNotifications.PackageBaseID = ?', + 'PackageNotifications.PackageBaseID = ? AND ' + + 'Users.Suspended = 0', [uid, old_pkgbase_id]) self._recipients = cur.fetchall()
Re: [PATCH] Remove PackageNotifications when User is suspended
Updated patch inc which only modifies `scripts/notify.py` and preserves notification records. On Fri, Jul 3, 2020 at 12:02 PM Kevin Morris wrote: > This path will only trigger when suspending a user through an > account page; if the database is managed manually or through > another script, PackageNotifications will also need manual > modifications if intended. > > This change was written as a solution to > https://bugs.archlinux.org/task/65554. > > Signed-off-by: Kevin Morris > --- > web/lib/acctfuncs.inc.php | 4 > 1 file changed, 4 insertions(+) > > diff --git a/web/lib/acctfuncs.inc.php b/web/lib/acctfuncs.inc.php > index d238c0e0..fb7cca8c 100644 > --- a/web/lib/acctfuncs.inc.php > +++ b/web/lib/acctfuncs.inc.php > @@ -403,6 +403,10 @@ function > process_account_form($TYPE,$A,$U="",$T="",$S="",$E="",$BE="",$H="",$P=" > $q.= " WHERE ID = ".intval($UID); > $result = $dbh->exec($q); > > + // Clean up PackageNotifications for $UID as well. > + $q = "DELETE FROM PackageNotifications WHERE UserID = > ".intval($UID); > + $result = $dbh->exec($q); > + > if (isset($ssh_keys) && count($ssh_keys) > 0) { > $ssh_key_result = account_set_ssh_keys($UID, > $ssh_keys, $ssh_fingerprints); > } else { > -- > 2.20.1 > > -- 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] Exclude suspended Users from being notified
The existing notify.py script was grabbing entries regardless of user suspension. This has been modified to only send notifications to unsuspended users, except for a Trusted User Vote Notification. This change was written as a solution to https://bugs.archlinux.org/task/65554. Signed-off-by: Kevin Morris --- aurweb/scripts/notify.py | 22 +++--- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/aurweb/scripts/notify.py b/aurweb/scripts/notify.py index 5b18a476..94ca1936 100755 --- a/aurweb/scripts/notify.py +++ b/aurweb/scripts/notify.py @@ -170,6 +170,7 @@ class CommentNotification(Notification): 'FROM Users INNER JOIN PackageNotifications ' + 'ON PackageNotifications.UserID = Users.ID WHERE ' + 'Users.CommentNotify = 1 AND ' + + 'Users.Suspended = 0 AND ' + 'PackageNotifications.UserID != ? AND ' + 'PackageNotifications.PackageBaseID = ?', [uid, pkgbase_id]) @@ -217,6 +218,7 @@ class UpdateNotification(Notification): 'INNER JOIN PackageNotifications ' + 'ON PackageNotifications.UserID = Users.ID WHERE ' + 'Users.UpdateNotify = 1 AND ' + + 'Users.Suspended = 0 AND ' + 'PackageNotifications.UserID != ? AND ' + 'PackageNotifications.PackageBaseID = ?', [uid, pkgbase_id]) @@ -264,7 +266,8 @@ class FlagNotification(Notification): 'INNER JOIN PackageBases ' + 'ON PackageBases.MaintainerUID = Users.ID OR ' + 'PackageBases.ID = PackageComaintainers.PackageBaseID ' + - 'WHERE PackageBases.ID = ?', [pkgbase_id]) + 'WHERE PackageBases.ID = ? AND ' + + 'Users.Suspended = 0', [pkgbase_id]) self._recipients = cur.fetchall() cur = conn.execute('SELECT FlaggerComment FROM PackageBases WHERE ' + 'ID = ?', [pkgbase_id]) @@ -302,7 +305,8 @@ class OwnershipEventNotification(Notification): 'ON PackageNotifications.UserID = Users.ID WHERE ' + 'Users.OwnershipNotify = 1 AND ' + 'PackageNotifications.UserID != ? AND ' + - 'PackageNotifications.PackageBaseID = ?', + 'PackageNotifications.PackageBaseID = ? AND ' + + 'Users.Suspended = 0', [uid, pkgbase_id]) self._recipients = cur.fetchall() cur = conn.execute('SELECT FlaggerComment FROM PackageBases WHERE ' + @@ -341,7 +345,7 @@ class ComaintainershipEventNotification(Notification): def __init__(self, conn, uid, pkgbase_id): self._pkgbase = pkgbase_from_id(conn, pkgbase_id) cur = conn.execute('SELECT Email, LangPreference FROM Users ' + - 'WHERE ID = ?', [uid]) + 'WHERE ID = ? AND Suspended = 0', [uid]) self._to, self._lang = cur.fetchone() super().__init__() @@ -384,7 +388,8 @@ class DeleteNotification(Notification): 'INNER JOIN PackageNotifications ' + 'ON PackageNotifications.UserID = Users.ID WHERE ' + 'PackageNotifications.UserID != ? AND ' + - 'PackageNotifications.PackageBaseID = ?', + 'PackageNotifications.PackageBaseID = ? AND ' + + 'Users.Suspended = 0', [uid, old_pkgbase_id]) self._recipients = cur.fetchall() super().__init__() @@ -431,7 +436,8 @@ class RequestOpenNotification(Notification): 'INNER JOIN Users ' + 'ON Users.ID = PackageRequests.UsersID ' + 'OR Users.ID = PackageBases.MaintainerUID ' + - 'WHERE PackageRequests.ID = ?', [reqid]) + 'WHERE PackageRequests.ID = ? AND ' + + 'Users.Suspended = 0', [reqid]) self._to = aurweb.config.get('options', 'aur_request_ml') self._cc = [row[0] for row in cur.fetchall()]
[PATCH] Remove PackageNotifications when User is suspended
This path will only trigger when suspending a user through an account page; if the database is managed manually or through another script, PackageNotifications will also need manual modifications if intended. This change was written as a solution to https://bugs.archlinux.org/task/65554. Signed-off-by: Kevin Morris --- web/lib/acctfuncs.inc.php | 4 1 file changed, 4 insertions(+) diff --git a/web/lib/acctfuncs.inc.php b/web/lib/acctfuncs.inc.php index d238c0e0..fb7cca8c 100644 --- a/web/lib/acctfuncs.inc.php +++ b/web/lib/acctfuncs.inc.php @@ -403,6 +403,10 @@ function process_account_form($TYPE,$A,$U="",$T="",$S="",$E="",$BE="",$H="",$P=" $q.= " WHERE ID = ".intval($UID); $result = $dbh->exec($q); + // Clean up PackageNotifications for $UID as well. + $q = "DELETE FROM PackageNotifications WHERE UserID = ".intval($UID); + $result = $dbh->exec($q); + if (isset($ssh_keys) && count($ssh_keys) > 0) { $ssh_key_result = account_set_ssh_keys($UID, $ssh_keys, $ssh_fingerprints); } else { -- 2.20.1
Re: [PATCH] Add POST support for RPC API
Apologies, `application/data` in the commit message's example should be `application/json`, and will need to be updated after review(s) in the final commit. On Fri, Jul 3, 2020 at 10:53 AM Kevin Morris wrote: > When we received a POST method from a client, deduce our `$data` > by determining which `Content-Type` is used. > > Supported `Content-Type`s > = > > * `application/json` > * `multipart/form-data` > > `application/json` POST Example, searching for 'test': > ``` > curl -X POST -H 'Content-Type: application/data' \ > --data '{"v": "5", "type": "search", "arg": "test"}' \ > https://aur.archlinux.org/rpc/ > ``` > > `multipart/form-data` POST Example, search for 'test': > ``` > curl -X POST -H 'Content-Type: multipart/form-data' \ > -F v=5 -F type=search -F arg=test \ > https://aur.archlinux.org/rpc/ > ``` > > This change was written as a solution to > https://bugs.archlinux.org/task/49089. > > NOTE: This commit is not final; would like a review and a bit more > consideration on `multipart/form-data` and ensuring that we're > handling everything we need to. > > Signed-off-by: Kevin Morris > --- > web/html/rpc.php | 37 ++--- > 1 file changed, 34 insertions(+), 3 deletions(-) > > diff --git a/web/html/rpc.php b/web/html/rpc.php > index 64c95622..41762aa3 100644 > --- a/web/html/rpc.php > +++ b/web/html/rpc.php > @@ -2,16 +2,47 @@ > set_include_path(get_include_path() . PATH_SEPARATOR . '../lib'); > include_once("aurjson.class.php"); > > -if ( $_SERVER['REQUEST_METHOD'] != 'GET' ) { > +$exposed_methods = array('GET', 'POST'); > +$request_method = strtoupper($_SERVER['REQUEST_METHOD']); > + > +if ( !in_array($request_method, $exposed_methods) ) { > header('HTTP/1.1 405 Method Not Allowed'); > exit(); > } > > -if ( isset($_GET['type']) ) { > +if ( $request_method === 'GET' ) { > + $data = $_GET; > +} else { > + // Otherwise, we were given a POST method, and we'll do some more > + // work to deduce our data input. > + > + // Extract Content-Type; remove any trailing arguments from the > string. > + $content_type = $_SERVER['CONTENT_TYPE']; > + if ( strpos($content_type, ';') ) { > + // Example: `multipart/form-data; boundary blahblah` is > extracted as > + // `multipart/form-data`. > + $content_type = explode(';', $content_type)[0]; > + } > + $content_type = rtrim(trim($content_type)); > + > + if ($content_type === 'application/json') { > + $json = file_get_contents('php://input'); > + $data = json_decode($json, true); > + } elseif ($content_type === 'multipart/form-data') { > + $data = $_POST; > + } else { > + header('HTTP/1.1 400 Bad Request'); > + echo "Error: Unsupported Content-Type header.\n"; > + exit(); > + } > +} > + > +if ( isset($data['type']) ) { > $rpc_o = new AurJSON(); > - echo $rpc_o->handle($_GET); > + echo $rpc_o->handle($data); > } > else { > echo file_get_contents('../../doc/rpc.html'); > } > + > ?> > -- > 2.20.1 > > -- 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] Add POST support for RPC API
When we received a POST method from a client, deduce our `$data` by determining which `Content-Type` is used. Supported `Content-Type`s = * `application/json` * `multipart/form-data` `application/json` POST Example, searching for 'test': ``` curl -X POST -H 'Content-Type: application/data' \ --data '{"v": "5", "type": "search", "arg": "test"}' \ https://aur.archlinux.org/rpc/ ``` `multipart/form-data` POST Example, search for 'test': ``` curl -X POST -H 'Content-Type: multipart/form-data' \ -F v=5 -F type=search -F arg=test \ https://aur.archlinux.org/rpc/ ``` This change was written as a solution to https://bugs.archlinux.org/task/49089. NOTE: This commit is not final; would like a review and a bit more consideration on `multipart/form-data` and ensuring that we're handling everything we need to. Signed-off-by: Kevin Morris --- web/html/rpc.php | 37 ++--- 1 file changed, 34 insertions(+), 3 deletions(-) diff --git a/web/html/rpc.php b/web/html/rpc.php index 64c95622..41762aa3 100644 --- a/web/html/rpc.php +++ b/web/html/rpc.php @@ -2,16 +2,47 @@ set_include_path(get_include_path() . PATH_SEPARATOR . '../lib'); include_once("aurjson.class.php"); -if ( $_SERVER['REQUEST_METHOD'] != 'GET' ) { +$exposed_methods = array('GET', 'POST'); +$request_method = strtoupper($_SERVER['REQUEST_METHOD']); + +if ( !in_array($request_method, $exposed_methods) ) { header('HTTP/1.1 405 Method Not Allowed'); exit(); } -if ( isset($_GET['type']) ) { +if ( $request_method === 'GET' ) { + $data = $_GET; +} else { + // Otherwise, we were given a POST method, and we'll do some more + // work to deduce our data input. + + // Extract Content-Type; remove any trailing arguments from the string. + $content_type = $_SERVER['CONTENT_TYPE']; + if ( strpos($content_type, ';') ) { + // Example: `multipart/form-data; boundary blahblah` is extracted as + // `multipart/form-data`. + $content_type = explode(';', $content_type)[0]; + } + $content_type = rtrim(trim($content_type)); + + if ($content_type === 'application/json') { + $json = file_get_contents('php://input'); + $data = json_decode($json, true); + } elseif ($content_type === 'multipart/form-data') { + $data = $_POST; + } else { + header('HTTP/1.1 400 Bad Request'); + echo "Error: Unsupported Content-Type header.\n"; + exit(); + } +} + +if ( isset($data['type']) ) { $rpc_o = new AurJSON(); - echo $rpc_o->handle($_GET); + echo $rpc_o->handle($data); } else { echo file_get_contents('../../doc/rpc.html'); } + ?> -- 2.20.1
Re: [PATCH] Add search_type url parameter
Alright. Sent a new patch that implements all this -- wasn't able to figure out how to `git send-email` into the same thread as this one, it's titled "[PATCH] Bump RPC API Version 6". I included the `Message ID` for this thread.
[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&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 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 | 66 ++- 2 files changed, 62 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&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..a3224e57 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,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. +* +* @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
Re: [PATCH] Add search_type url parameter
If we bump the API version, #1 is unnecessary and would be removed. On Thu, Jul 2, 2020, 5:23 PM Lukas Fleischer wrote: > Hi Kevin, > > Thanks for your quick reply and your willingness to improve on your > first submission! > > On Thu, 02 Jul 2020 at 20:04:10, Kevin Morris wrote: > > The refraining from #2 is due to me being a bit worried due to it > changing > > how the API behaves, didn't think about the complete picture ("arch > linux" > > second_keyword). I'll start working on that now. > > Yes, we need to bump the API version and keep the old behavior for the > current API version. But I don't see any issues with that. > > > 1. Rename search_type values to something more sensible (waiting on this > > for feedback from ML) > > 2. Implement support for quoted strings in the search arg: "arch linux". > > Quoted keywords will be taken literally -- that is, "arch linux" would > > trigger a search for the literal 'arch linux' string. > > Would (1) still be relevant if (2) is implemented? > > Best, > Lukas >
Re: [PATCH] Add search_type url parameter
No problem; I love to help where I can! To address your two points: 1. There is no particular reason for `rpc` and `web` being chosen; I just needed separate values for this patch to be tested with. I'm up to renaming them to anything if we decide to use `search_type`. 2. It would not be much more work to add the quoted stanza -- I had an idea to at one point, but just decided to back off from it; figured just introducing the patch would get some conversation about it on the ML. I'll start working on a follow-up patch that implements this as well. The refraining from #2 is due to me being a bit worried due to it changing how the API behaves, didn't think about the complete picture ("arch linux" second_keyword). I'll start working on that now. My tasks: 1. Rename search_type values to something more sensible (waiting on this for feedback from ML) 2. Implement support for quoted strings in the search arg: "arch linux". Quoted keywords will be taken literally -- that is, "arch linux" would trigger a search for the literal 'arch linux' string. Thanks for the follow-up, Lukas; I'm on it! On Thu, Jul 2, 2020 at 4:46 PM Lukas Fleischer wrote: > Hi Kevin, > > Thanks for the patch! > > On Tue, 30 Jun 2020 at 19:36:01, Kevin Morris wrote: > > This commit introduces new functionality: When `search_type` is `web`, > > search behavior matches aurweb's HTML search page. > > > > New parameters: `search_type` > > Valid search_type values: `rpc` (default), `web` > > > > The `rpc` search type uses the existing legacy search method. > > > > The `web` search type clontes the web-based aurweb search page's behavior > > as closely as possible (see note at the bottom of this message). > > > > The following example searches for packages that contain `blah` AND > `abc`: > > > https://aur.archlinux.org/rpc/?v=5&type=search&search_type=web&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&search_type=rpc&arg=blah%20abc > > Before having a look at the implementation, is there any reason the two > search types are called "rpc" and "web" (other than "rpc" is how the RPC > interface used to behave and "web" is how the RPC interface used to > behave)? If not, I think we should choose a different naming that is > more intuitive and self-explanatory. > > Going a step further, would it be much more work to instead make the > interface more powerful, use conjunctive search by default and perform > exact matches for phrases put in quotes, such as > > "arch linux" package > > matching everything that contains the strings "arch linux" and > "package"? With that, we wouldn't even have to distinguish different > search types (but we may need to bump the API version). > > Best regards, > Lukas > -- 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] Add search_type url parameter
This commit introduces new functionality: When `search_type` is `web`, search behavior matches aurweb's HTML search page. New parameters: `search_type` Valid search_type values: `rpc` (default), `web` The `rpc` search type uses the existing legacy search method. The `web` search type clones the web-based aurweb search page's behavior as closely as possible (see note at the bottom of this message). The following example searches for packages that contain `blah` AND `abc`: https://aur.archlinux.org/rpc/?v=5&type=search&search_type=web&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&search_type=rpc&arg=blah%20abc An invalid `search_type` returns a json error message about it being unsupported. Additionally, `search_type` is only parsed and considered during a search of type `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 | 7 web/lib/aurjson.class.php | 75 +++ 2 files changed, 76 insertions(+), 6 deletions(-) diff --git a/doc/rpc.txt b/doc/rpc.txt index 3148ebea..eb97547d 100644 --- a/doc/rpc.txt +++ b/doc/rpc.txt @@ -21,6 +21,11 @@ The _by_ parameter can be skipped and defaults to `name-desc`. If a maintainer search is performed and the search argument is left empty, a list of orphan packages is returned. +The _search_type_ parameter can be skipped and defaults to `rpc`. + +_search_type_ is only ever used during a `name` or `name-desc` search and +supports the following values: `rpc`, `web`. + Package Details --- @@ -39,6 +44,8 @@ Examples `/rpc/?v=5&type=search&by=makedepends&arg=boost` `search` with callback:: `/rpc/?v=5&type=search&arg=foobar&callback=jsonp1192244621103` +`search` with `web` _search_type_ for packages containing `cookie` AND `milk`:: + `/rpc/?v=5&type=search&search_type=web&arg=cookie%20milk` `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..0e94b578 100644 --- a/web/lib/aurjson.class.php +++ b/web/lib/aurjson.class.php @@ -23,6 +23,9 @@ class AurJSON { private static $exposed_depfields = array( 'depends', 'makedepends', 'checkdepends', 'optdepends' ); + private static $exposed_search_types = array( + 'rpc', 'web' + ); private static $fields_v1 = array( 'Packages.ID', 'Packages.Name', 'PackageBases.ID AS PackageBaseID', @@ -140,7 +143,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 +195,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 { @@ -472,6 +475,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. +* +* @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) { +
aurweb tests and ci
Hello aur-dev, Working on some things in aurweb at the moment, and I noticed that on master tests are failing on my system, and .travis.yml doesn't seem to properly setup the environment needed to run tests (sqlalchemy is not installed, db tables missing) on travis. Just a few questions 1. What's the state of testing aurweb currently? 2. What are any test barriers to committing code and submitting patches? 2a. What are the testing goals? 3. Is there a thread on this mailing list about this topic? Thank you very much for your time. Regards, Kevin -- 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] Fix German translation for DISCLAIMER line on the home page.
Something was leftover (probably from a previous update?) Took out the extra line to sync up with how the other languages handle it. Signed-off-by: Kevin Morris --- po/de.po | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/po/de.po b/po/de.po index 00ab95c..ee5631d 100644 --- a/po/de.po +++ b/po/de.po @@ -233,7 +233,7 @@ msgstr "Manche Pakete könnten als Binär-Pakete in [community] bereitgestellt s #: html/home.php msgid "DISCLAIMER" -msgstr "DISCLAIMER: Offiziell nicht unterstützte PKGBUILDS werden von den Benutzern erstellt - durch den Download willigt man ein, diese auf eigene Gefahr zu benutzen." +msgstr "DISCLAIMER" #: html/home.php template/footer.php msgid "" -- 2.20.1
Re: Django-hosted aurweb Proposal
Thank you for the extra CC, that was perfect timing :). To address Foxboron, I would choose Django as a framework purely so that we could integrate it into the same wsgi engine that archweb uses on the current servers and have it work seamlessly. But additionally, Django provides a good baked in user model which we would otherwise have to either implement or use an existing user framework for Flask/SQLAlchemy? I believe that Django is more rigorous in it's approach to handling routes and would require less developer effort to add, modify, or remove features. It is heavier, but I believe that it is more stable when used correctly. Django also provides many automatic handling of sessions (redirection to login, automatic error responses, ...) and it's rest_framework extension is just as simple and robust as anything else. I'm a bit confused. You wrote one patch and you want to rewrite the whole > thing > now? I've seen this at least three times before with more or less fully > developed rewrites. They all failed. They were all written in Python > coincidentally. No. The patch was merely something that I wanted to help resolve before presenting any kind of proposal like this. It was brought up as an outstanding issue or possible patch to finish up for the RPC protocol. I only wanted to help finish the patch to help out (and I also thought provides would be quite useful). Additionally, I would like to apologize for the fact that the proposal is not as well written as it could be. On Sat, Feb 16, 2019 at 9:08 PM Loui Chang wrote: > On Sat 16 Feb 2019 17:21 -0800, Kevin Morris wrote: > > The following is a proposal for a Django-hosted aurweb application. It is > > meant to be a drop-in replacement for aur.archlinux.org; effectively > > cloning its user interface and capabilities into a Python Django > extension. > > > > Following https://patchwork.archlinux.org/patch/1000/, I would like to > put > > together a new aurweb. > > I'm a bit confused. You wrote one patch and you want to rewrite the whole > thing > now? I've seen this at least three times before with more or less fully > developed rewrites. They all failed. They were all written in Python > coincidentally. > > > The current revamped version of archweb runs inside of a django server > as a > > django extension (or app). I would like to do this same thing for > `aurweb`. > > The new django `aurweb` shall support all of the current v1-6 > capabilities > > that aurweb provides, as well as the front-end user website located at > > https://aur.archlinux.org. > > archweb was already written in python for django more than 10 yrs ago. > I guess it may not have been an extension (or app), whatever that is. > I imagine the porting/migration if any would have been much more trivial > than > a full rewrite. > > > It shall be an exact clone from the user's perspective. > > This is a very uncompelling reason to rewrite the whole thing. > > > The major differences between maintaining a PHP vs Django server would be > > that the Django server would be: > > Users don't care about any of your six bullet points. > > > I would like to hear your thoughts on this. If approved, I would love to > > begin this project within the next few weeks. > > Begin the project now and don't make it the same. Make it better. > Nobody is gonna approve vaporware. > Steal users from the AUR. > Convince people your system really is better. > Good luck. > > -- Kevin Morris Software Developer Business Inquiries: k...@coderesistance.com Personal Inquiries: kevr.gt...@gmail.com Personal Phone: (415) 571-0513 Technologies: C++, Python, Django, Ruby, Rails, ReactJS, jQuery, Javascript, SQL, Redux
Django-hosted aurweb Proposal
To whom it may concern, The following is a proposal for a Django-hosted aurweb application. It is meant to be a drop-in replacement for aur.archlinux.org; effectively cloning its user interface and capabilities into a Python Django extension. Following https://patchwork.archlinux.org/patch/1000/, I would like to put together a new aurweb. The current revamped version of archweb runs inside of a django server as a django extension (or app). I would like to do this same thing for `aurweb`. The new django `aurweb` shall support all of the current v1-6 capabilities that aurweb provides, as well as the front-end user website located at https://aur.archlinux.org. It shall be an exact clone from the user's perspective. The major differences between maintaining a PHP vs Django server would be that the Django server would be: 1. Integrated with any Django server and easily removed. 2. Developed via Python, which has vastly more support and familiarity than PHP within the community. 3. Django models, which greatly reduce the complexity of SQL queries inside of `aurweb` backend. 4. Baked-in extendable templates with templatetags. 5. View decorators which enforce requirements of a specific route. No more manually handling $http_data user errors. 6. Everybody loves Python; there will most likely be many more code-base contributions in Python than in PHP. I would like to hear your thoughts on this. If approved, I would love to begin this project within the next few weeks. Thanks, Kevin
[PATCH] Introduce API v6
Add new method of multiple by[] search in API v6. The 'provides' by field is also supported in the new multiple by[] method included with this patch. 'provides' may not be used in legacy (v5) singular by search. A helpful json error message is returned to the user to indicate so. This patch is a follow-up implementation to https://patchwork.archlinux.org/patch/479/ Signed-off-by: Kevin Morris --- doc/rpc.txt | 24 ++--- web/lib/aurjson.class.php | 105 -- 2 files changed, 116 insertions(+), 13 deletions(-) diff --git a/doc/rpc.txt b/doc/rpc.txt index 3148ebe..486a49d 100644 --- a/doc/rpc.txt +++ b/doc/rpc.txt @@ -5,7 +5,7 @@ Package Search -- Package searches can be performed by issuing HTTP GET requests of the form -+/rpc/?v=5&type=search&by=_field_&arg=_keywords_+ where _keywords_ is the ++/rpc/?v=6&type=search&by[]=_field_&arg[]=_keywords_+ where _keywords_ is the search argument and _field_ is one of the following values: * `name` (search by package name only) @@ -15,31 +15,39 @@ search argument and _field_ is one of the following values: * `makedepends` (search for packages that makedepend on _keywords_) * `optdepends` (search for packages that optdepend on _keywords_) * `checkdepends` (search for packages that checkdepend on _keywords_) +* `provides` (search by package provides; v6 multiple by[] only) The _by_ parameter can be skipped and defaults to `name-desc`. If a maintainer search is performed and the search argument is left empty, a list of orphan packages is returned. +Note: Legacy v5 support is still enabled for singular _by_ searches. + Package Details --- Package information can be obtained by issuing HTTP GET requests of the form -+/rpc/?v=5&type=info&arg[]=_pkg1_&arg[]=_pkg2_&...+ where _pkg1_, _pkg2_, ... ++/rpc/?v=6&type=info&arg[]=_pkg1_&arg[]=_pkg2_&...+ where _pkg1_, _pkg2_, ... are the names of packages to retrieve package details for. Examples `search`:: - `/rpc/?v=5&type=search&arg=foobar` + `/rpc/?v=6&type=search&arg=foobar` `search` by maintainer:: - `/rpc/?v=5&type=search&by=maintainer&arg=john` + `/rpc/?v=6&type=search&by[]=maintainer&arg=john` `search` packages that have _boost_ as `makedepends`:: - `/rpc/?v=5&type=search&by=makedepends&arg=boost` + `/rpc/?v=6&type=search&by[]=makedepends&arg=boost` `search` with callback:: - `/rpc/?v=5&type=search&arg=foobar&callback=jsonp1192244621103` + `/rpc/?v=6&type=search&arg=foobar&callback=jsonp1192244621103` +`search` by provides:: + `/rpc/?v=6&type=search&by[]=provides&arg[]=gcc` +`search` by provides and name:: + `/rpc/?v=6&type=search&by[]=name&by[]=provides&arg[]=gcc` `info`:: - `/rpc/?v=5&type=info&arg[]=foobar` + `/rpc/?v=6&type=info&arg[]=foobar` `info` with multiple packages:: - `/rpc/?v=5&type=info&arg[]=foo&arg[]=bar` + `/rpc/?v=6&type=info&arg[]=foo&arg[]=bar` + diff --git a/web/lib/aurjson.class.php b/web/lib/aurjson.class.php index c275d21..ab4eb19 100644 --- a/web/lib/aurjson.class.php +++ b/web/lib/aurjson.class.php @@ -80,7 +80,8 @@ 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.'); } @@ -94,8 +95,14 @@ class AurJSON { if (isset($http_data['search_by']) && !isset($http_data['by'])) { $http_data['by'] = $http_data['search_by']; } - if (isset($http_data['by']) && !in_array($http_data['by'], self::$exposed_fields)) { - return $this->json_error('Incorrect by field specified.'); + + if (isset($http_data['by']) && !is_array($http_data['by'])) { + if ($http_data['by'] === 'provides') { + return $this->json_error("The 'provides' by field " . + "may only be used via multiple by[] search."); + } elseif (!in_array($http_data['by'], self::$exposed_fields)) { + return $this->json_error('Incorrect by field specified.'); + } } $this->dbh = DB::connect(); @@
PR Regarding v6 multiple by and provides field
Hello AUR Maintainers, This pull request is in reference to https://patchwork.archlinux.org/patch/578/ and satisfies back-compat requirements and multiple by[] search implementation that the original requester intended, including the 'provides' field search. > The following changes since commit > 042f3f2622e1adbc7546cfa25a5d138486b99341: > Quote MySql 8.0 reserved keywords (2019-01-14 14:45:37 -0500) > are available in the Git repository at: > https://github.com/kevr/aurweb master > for you to fetch changes up to ac80bee46ae3fe0362c134fe598304d58d1db3fb: > Introduce API v6 (2019-02-12 13:39:08 -0800) > -------- > Kevin Morris (1): > Introduce API v6 > doc/rpc.txt | 15 ++- > web/lib/aurjson.class.php | 99 > +++ > 2 files changed, 109 insertions(+), 5 deletions(-) Please let me know if there are any blockers or questions you may have, I hope it works out well! Regards, Kevin -- Kevin Morris Personal Inquiries: kevr.gt...@gmail.com Technologies: C++, Python, Django, Ruby, Rails, ReactJS, jQuery, Javascript, SQL, Redux