[PATCH] feat(rpc): return "providers" packages when querying by `name` or `name-desc`
From: actionless <actionless.loveless@gmail.com> --- doc/rpc.txt | 4 ++-- web/lib/aurjson.class.php | 16 +++++++++++++++- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/doc/rpc.txt b/doc/rpc.txt index f353ff0..83cdae3 100644 --- a/doc/rpc.txt +++ b/doc/rpc.txt @@ -8,8 +8,8 @@ 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 search argument and _field_ is one of the following values: -* `name` (search by package name only) -* `name-desc` (search by package name and description) +* `name` (search by package name or packages which provide that name) +* `name-desc` (the same as `name` and search by description) * `maintainer` (search by package maintainer) The _by_ parameter can be skipped and defaults to `name-desc`. diff --git a/web/lib/aurjson.class.php b/web/lib/aurjson.class.php index 9eeaafd..6e580a9 100644 --- a/web/lib/aurjson.class.php +++ b/web/lib/aurjson.class.php @@ -392,12 +392,26 @@ class AurJSON { if (strlen($keyword_string) < 2) { return $this->json_error('Query arg too small.'); } + + //packages which provide the package we are looking for: + $providers = pkg_providers(addcslashes($keyword_string, '%_')); + $provided_names = array(); + foreach ($providers as $provider) { + if ($provider[0] != 0) { // if package is not from repo + $name = $this->dbh->quote($provider[1]); + array_push($provided_names, $name); + } + } + $provided_query = "(" . join(", ", $provided_names) . ")"; + $keyword_string = $this->dbh->quote("%" . addcslashes($keyword_string, '%_') . "%"); if ($search_by === 'name') { - $where_condition = "(Packages.Name LIKE $keyword_string)"; + $where_condition = "(Packages.Name LIKE $keyword_string OR "; + $where_condition .= "Packages.Name IN $provided_query )"; } else if ($search_by === 'name-desc') { $where_condition = "(Packages.Name LIKE $keyword_string OR "; + $where_condition .= "Packages.Name IN $provided_query OR "; $where_condition .= "Description LIKE $keyword_string)"; } } else if ($search_by === 'maintainer') { -- 2.16.2
On 20 March 2018 at 04:49, morganamilo <morganamilo@gmail.com> wrote:
From: actionless <actionless.loveless@gmail.com>
--- doc/rpc.txt | 4 ++-- web/lib/aurjson.class.php | 16 +++++++++++++++- 2 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/doc/rpc.txt b/doc/rpc.txt index f353ff0..83cdae3 100644 --- a/doc/rpc.txt +++ b/doc/rpc.txt @@ -8,8 +8,8 @@ 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 search argument and _field_ is one of the following values:
-* `name` (search by package name only) -* `name-desc` (search by package name and description) +* `name` (search by package name or packages which provide that name) +* `name-desc` (the same as `name` and search by description) * `maintainer` (search by package maintainer)
The _by_ parameter can be skipped and defaults to `name-desc`. diff --git a/web/lib/aurjson.class.php b/web/lib/aurjson.class.php index 9eeaafd..6e580a9 100644 --- a/web/lib/aurjson.class.php +++ b/web/lib/aurjson.class.php @@ -392,12 +392,26 @@ class AurJSON { if (strlen($keyword_string) < 2) { return $this->json_error('Query arg too small.'); } + + //packages which provide the package we are looking for: + $providers = pkg_providers(addcslashes($keyword_string, '%_')); + $provided_names = array(); + foreach ($providers as $provider) { + if ($provider[0] != 0) { // if package is not from repo + $name = $this->dbh->quote($provider[1]); + array_push($provided_names, $name); + } + } + $provided_query = "(" . join(", ", $provided_names) . ")"; + $keyword_string = $this->dbh->quote("%" . addcslashes($keyword_string, '%_') . "%");
if ($search_by === 'name') { - $where_condition = "(Packages.Name LIKE $keyword_string)"; + $where_condition = "(Packages.Name LIKE $keyword_string OR "; + $where_condition .= "Packages.Name IN $provided_query )"; } else if ($search_by === 'name-desc') { $where_condition = "(Packages.Name LIKE $keyword_string OR "; + $where_condition .= "Packages.Name IN $provided_query OR "; $where_condition .= "Description LIKE $keyword_string)"; } } else if ($search_by === 'maintainer') { -- 2.16.2
Sent on behalf of actionless <actionless.loveless@gmail.com>. I have an intrest in this myself so if he decides he does not want to further this I wouldn't mind taking it over.
On Tue, 20 Mar 2018 at 05:49:57, morganamilo wrote:
From: actionless <actionless.loveless@gmail.com>
--- doc/rpc.txt | 4 ++-- web/lib/aurjson.class.php | 16 +++++++++++++++- 2 files changed, 17 insertions(+), 3 deletions(-) [...]
Thank you for submitting a patch! Before commenting on implementation details, I would like to discuss the overall approach, though. What is the rationale for making it part of name and name-desc? Wouldn't it make more sense to add a new search type ("providers")? Regards, Lukas
On 03/20/2018 02:36 PM, Lukas Fleischer wrote:
On Tue, 20 Mar 2018 at 05:49:57, morganamilo wrote:
From: actionless <actionless.loveless@gmail.com>
--- doc/rpc.txt | 4 ++-- web/lib/aurjson.class.php | 16 +++++++++++++++- 2 files changed, 17 insertions(+), 3 deletions(-) [...]
Thank you for submitting a patch! Before commenting on implementation details, I would like to discuss the overall approach, though. What is the rationale for making it part of name and name-desc? Wouldn't it make more sense to add a new search type ("providers")?
Well, we have no precedent here as archweb does not allow searching by provides either AFAICT. But I think, generally speaking, from a usability perspective any search that is not an exact-name search is probably *interested* in these provides packages. pacman -Ss will show them, pacman -S will install them, so the web interfaces arguably should do the same. ... Aside: that commit message feels somewhat jarring as it does not match the (admittedly loose) style in use. I have no idea why this commit is a "feat" but it feels oddly self-congratulatory, as though the commit is a circus performer demonstrating a trick. Am I missing out on the latest fashion or something? -- Eli Schwartz Bug Wrangler and Trusted User
2018-03-21 2:56 GMT+08:00 Eli Schwartz <eschwartz@archlinux.org>:
What is the rationale for making it part of name and name-desc? Wouldn't it make more sense to add a new search type ("providers")? I vote for a new search type "providers". For my use case, I'd appreciate if I can get precise information from the RPC, instead of parsing the aurweb html or crawling the entire AUR. I prefer having a bulk query here so I can keep the request count low.
Well, we have no precedent here as archweb does not allow searching by provides either AFAICT. But I think, generally speaking, from a usability perspective any search that is not an exact-name search is probably *interested* in these provides packages.
pacman -Ss will show them, pacman -S will install them, so the web interfaces arguably should do the same.
Actually I just found it confusing that: pacman -Ss 'gcc>7' shows nothing, yet pacman -S 'gcc>7' will do the right stuff I'd argue that pacman should provide a search providers. But at least it has alpm_find_satisifier.
On 03/20/2018 08:47 PM, 尤立宇 wrote:
I vote for a new search type "providers". For my use case, I'd appreciate if I can get precise information from the RPC, instead of parsing the aurweb html or crawling the entire AUR. I prefer having a bulk query here so I can keep the request count low.
So, like, return all the provides results in one search result, then do another RPC search for multiinfo on all those packages to see exact details on the pkgbase, provides, etc?
pacman -Ss will show them, pacman -S will install them, so the web interfaces arguably should do the same.
Actually I just found it confusing that: pacman -Ss 'gcc>7' shows nothing, yet pacman -S 'gcc>7' will do the right stuff I'd argue that pacman should provide a search providers. But at least it has alpm_find_satisifier.
That's because the >7 is a version specifier, not a search term. On top of which that is for the main package and has nothing to do with provides. -- Eli Schwartz Bug Wrangler and Trusted User
2018-03-21 8:56 GMT+08:00 Eli Schwartz <eschwartz@archlinux.org>:
On 03/20/2018 08:47 PM, 尤立宇 wrote:
I vote for a new search type "providers". For my use case, I'd appreciate if I can get precise information from the RPC, instead of parsing the aurweb html or crawling the entire AUR. I prefer having a bulk query here so I can keep the request count low.
So, like, return all the provides results in one search result, then do another RPC search for multiinfo on all those packages to see exact details on the pkgbase, provides, etc?
I think it would be like: HTTP GET /rpc/?v=5&type=provides&arg[]=java-environment&arg[]=libgala.so {"version":5,"type":"provides","resultcount":2,"results": "java-environment": [] // might be list of package names or list of "info" "libgala.so": [] }
pacman -Ss will show them, pacman -S will install them, so the web interfaces arguably should do the same.
Actually I just found it confusing that: pacman -Ss 'gcc>7' shows nothing, yet pacman -S 'gcc>7' will do the right stuff I'd argue that pacman should provide a search providers. But at least it has alpm_find_satisifier.
That's because the >7 is a version specifier, not a search term. On top of which that is for the main package and has nothing to do with provides. I got the reasoning about it. Still, versioned dependencies are valid in the depends array, so I'd like the provides search in the RPC mentioned above support this, much like the Dependencies section in the web interface [1].
[1] https://aur.archlinux.org/packages/nvidia-ck/ See how it lists nvidia-utils=390.42, linux-ck-headers>=4.15, etc
I think it would be like: HTTP GET /rpc/?v=5&type=provides&arg[]=java-environment&arg[]=libgala.so {"version":5,"type":"provides","resultcount":2,"results": "java-environment": [] // might be list of package names or list of "info" "libgala.so": [] } Pacman can "-S" and "-Ss" by provides so I would prefer to have the option to search by provides in type=info and type=search rather than a new type=provides. Also I would like each option to be indivudually settable, so "provides", "name" and "desc" can be set indipendantly.
2018-03-21 9:49 GMT+08:00 morganamilo <morganamilo@gmail.com>:
I think it would be like: HTTP GET /rpc/?v=5&type=provides&arg[]=java-environment&arg[]=libgala.so {"version":5,"type":"provides","resultcount":2,"results": "java-environment": [] // might be list of package names or list of "info" "libgala.so": [] }
Pacman can "-S" and "-Ss" by provides so I would prefer to have the option to search by provides in type=info and type=search rather than a new type=provides. Also I would like each option to be indivudually settable, so "provides", "name" and "desc" can be set indipendantly. I think I'm looking for an alpm_find_satisifier equivalent rather than a pacman -Ss equalviant, and that's why I'm proposing a new type=provides.
For type=info after thinking again, is it a) The dependencies providers' details are listed in the "Depends" "MakeDepends" "OptDepends" b) The "results" items are unchanged, but querying a name not only returns the package name that matches, but also their provides array matches. That is ?type=info&arg[]=java-environment would return resultcount > 1 or something else? I think b) would work for me, I will just have to verify the version afterwards with vercmp in that case.
On 21/03/18 02:33, 尤立宇 wrote:
2018-03-21 9:49 GMT+08:00 morganamilo <morganamilo@gmail.com>:
I think it would be like: HTTP GET /rpc/?v=5&type=provides&arg[]=java-environment&arg[]=libgala.so {"version":5,"type":"provides","resultcount":2,"results": "java-environment": [] // might be list of package names or list of "info" "libgala.so": [] } Pacman can "-S" and "-Ss" by provides so I would prefer to have the option to search by provides in type=info and type=search rather than a new type=provides. Also I would like each option to be indivudually settable, so "provides", "name" and "desc" can be set indipendantly. I think I'm looking for an alpm_find_satisifier equivalent rather than a pacman -Ss equalviant, and that's why I'm proposing a new type=provides.
For type=info after thinking again, is it a) The dependencies providers' details are listed in the "Depends" "MakeDepends" "OptDepends" b) The "results" items are unchanged, but querying a name not only returns the package name that matches, but also their provides array matches. That is ?type=info&arg[]=java-environment would return resultcount > 1 or something else?
I think b) would work for me, I will just have to verify the version afterwards with vercmp in that case.
An alpm_find_satisifier equivilent is what I want too, thing is once you get the package names from that alpm_find_satisifier equivelent what are you going to do with thoes names? Probably call "info" on all of them. So why not skip that step and just have info support it. Yes point b is what I was thinking. info("foo") would return "foo" because it matches by name, and it would also return "foo-git" because it provides "foo". Maybe it would be &type=info&by[]=provides&by[]=name rather than just "info". Point being you still get the same hunk of json that info gets you. Would your type=provides return just package names or the same stuff info does? If its the latter it's the same as what I want but under a different name.
An alpm_find_satisifier equivilent is what I want too, thing is once you get the package names from that alpm_find_satisifier equivelent what are you going to do with thoes names? Probably call "info" on all of them. So why not skip that step and just have info support it.
Yes point b is what I was thinking. info("foo") would return "foo" because it matches by name, and it would also return "foo-git" because it provides "foo". Maybe it would be &type=info&by[]=provides&by[]=name rather than just "info". Point being you still get the same hunk of json that info gets you.
Would your type=provides return just package names or the same stuff info does? If its the latter it's the same as what I want but under a different name. I'm convinced that just putting that into info would be better. Looking at the code also shows that adding by[]=provides is easier to implement as it fits better to the current architecture.
An alpm_find_satisifier equivilent is what I want too, thing is once you get the package names from that alpm_find_satisifier equivelent what are you going to do with thoes names? Probably call "info" on all of them. So why not skip that step and just have info support it.
Yes point b is what I was thinking. info("foo") would return "foo" because it matches by name, and it would also return "foo-git" because it provides "foo". Maybe it would be &type=info&by[]=provides&by[]=name rather than just "info". Point being you still get the same hunk of json that info gets you.
Would your type=provides return just package names or the same stuff info does? If its the latter it's the same as what I want but under a different name. I'm convinced that just putting that into info would be better. Looking at the code also shows that adding by[]=provides is easier to implement as it fits better to the current architecture. Glad you agree. This patch was more about opening up discussion before going in and changing a bunch of stuff. Is there any offial decision by
On 21/03/18 03:12, 尤立宇 wrote: the devs and a little guidence on how the implementation should look?
On Wed, 21 Mar 2018 at 04:20:18, morganamilo wrote:
An alpm_find_satisifier equivilent is what I want too, thing is once you get the package names from that alpm_find_satisifier equivelent what are you going to do with thoes names? Probably call "info" on all of them. So why not skip that step and just have info support it.
Yes point b is what I was thinking. info("foo") would return "foo" because it matches by name, and it would also return "foo-git" because it provides "foo". Maybe it would be &type=info&by[]=provides&by[]=name rather than just "info". Point being you still get the same hunk of json that info gets you.
Would your type=provides return just package names or the same stuff info does? If its the latter it's the same as what I want but under a different name. I'm convinced that just putting that into info would be better. Looking at the code also shows that adding by[]=provides is easier to implement as it fits better to the current architecture. Glad you agree. This patch was more about opening up discussion before going in and changing a bunch of stuff. Is there any offial decision by
On 21/03/18 03:12, 尤立宇 wrote: the devs and a little guidence on how the implementation should look?
As I already suggested before, I like the idea of adding multiple "by" arguments to the search. I wonder what to do with info queries. If we do the same thing there, wouldn't it be natural to add all the "by" types from search as well? Maybe info and search could/should be able to handle the same kind of queries and only differ in the amount of details returned...
I'm convinced that just putting that into info would be better. Looking at the code also shows that adding by[]=provides is easier to implement as it fits better to the current architecture. Glad you agree. This patch was more about opening up discussion before going in and changing a bunch of stuff. Is there any offial decision by the devs and a little guidence on how the implementation should look?
As I already suggested before, I like the idea of adding multiple "by" arguments to the search. I wonder what to do with info queries. If we do the same thing there, wouldn't it be natural to add all the "by" types from search as well? Maybe info and search could/should be able to handle the same kind of queries and only differ in the amount of details returned...
From my point of view, the search should look for the queried substring in the fields, while info looks for exact match. My study on the code shows the only difference now is how the $where_condition is built [1] and the level of detail is the same.
Here's some code highlights (v5 only). Except maintainer, search performs a LIKE query and info performs a IN query. Also, search only supports single argument, while info supports multiple. search/name: "(Packages.Name LIKE $keyword_string)" search/name-desc: "(Packages.Name LIKE $keyword_string OR Description LIKE $keyword_string)" search/maintainer: "Users.Username = $keyword_string " info: "Packages.Name IN ($names_value) " To support multiple "by" arguments, I came up with this, assuming by=by1&by=by2 search: any by fields contain the argument WHERE (Packages.By1 LILKE $keyword_string OR Packages.By2 LIKE $keyword_string) info: any by fields equals one of the argument WHERE (Packages.By1 IN ($names_value) OR Packages.By2 IN ($names_value) I think this way more fields can be easily supported. With this in mind, search by=name-desc would become by=name&by=desc search by=maintainer would become info by=maintainer. Does this make sense to you? [1] search: https://git.archlinux.org/aurweb.git/tree/web/lib/aurjson.class.php#n412 multiinfo: https://git.archlinux.org/aurweb.git/tree/web/lib/aurjson.class.php#n467
On Wed, 21 Mar 2018 at 09:10:10, 尤立宇 wrote:
[...] To support multiple "by" arguments, I came up with this, assuming by=by1&by=by2 search: any by fields contain the argument WHERE (Packages.By1 LILKE $keyword_string OR Packages.By2 LIKE $keyword_string) info: any by fields equals one of the argument WHERE (Packages.By1 IN ($names_value) OR Packages.By2 IN ($names_value) I think this way more fields can be easily supported.
With this in mind, search by=name-desc would become by=name&by=desc search by=maintainer would become info by=maintainer. Does this make sense to you? [...]
Yes, sounds reasonable. Note that we still need to support the old implementation and should probably create a new version of the RPC API. Regards, Lukas
--- web/lib/aurjson.class.php | 125 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 120 insertions(+), 5 deletions(-) diff --git a/web/lib/aurjson.class.php b/web/lib/aurjson.class.php index c51e9c2..db11117 100644 --- a/web/lib/aurjson.class.php +++ b/web/lib/aurjson.class.php @@ -20,6 +20,14 @@ class AurJSON { 'name', 'name-desc', 'maintainer', 'depends', 'makedepends', 'checkdepends', 'optdepends' ); + private static $exposed_fields_v6 = array( + 'name', 'description', 'maintainer', 'provides', + ); + private static $exposed_fields_map_v6 = array( + 'name' => 'Packages.Name', + 'description' => 'Packages.Description', + 'maintainer' => 'Packages.Maintainer', + ); private static $exposed_depfields = array( 'depends', 'makedepends', 'checkdepends', 'optdepends' ); @@ -80,7 +88,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.'); } @@ -94,8 +102,21 @@ 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'])) { + if ($this->version < 6) { + if (!in_array($http_data['by'], self::$exposed_fields)) { + return $this->json_error('Incorrect by field specified.'); + } + } else { + if (!is_array($http_data['by'])) { + $http_data['by'] = array($http_data['by']); + } + foreach ($http_data['by'] as $by) { + if (!in_array($by, self::$exposed_fields_v6)) { + return $this->json_error("Incorrect by field '$by' specified."); + } + } + } } $this->dbh = DB::connect(); @@ -109,7 +130,11 @@ class AurJSON { if ($type == 'info' && $this->version >= 5) { $type = 'multiinfo'; } - $json = call_user_func(array(&$this, $type), $http_data); + if ($this->version < 6) { + $json = call_user_func(array(&$this, $type), $http_data); + } else { + $json = $this->info_search_v6($type, $http_data); + } $etag = md5($json); header("Etag: \"$etag\""); @@ -374,6 +399,21 @@ class AurJSON { } $result = $this->dbh->query($query); + return $this->process_result($type, $result); + } + + + /* + * Retrieve package information from a dbh->query result + * + * @param $type The request type. + * @param $result A dbh->query result. + * + * @return mixed Returns an array of package matches. + */ + private function process_result($type, $result) { + $max_results = config_get_int('options', 'max_rpc_results'); + if ($result) { $resultcount = 0; $search_data = array(); @@ -515,6 +555,82 @@ class AurJSON { return $this->process_query('search', $where_condition); } + /* + * Performs a info or search query to the package database. + * + * @param $type The request type. + * @param array $http_data Query parameters. + * + * @return mixed Returns an array of package matches. + */ + private function info_search_v6($type, $http_data) { + if (isset($http_data['by'])) { + $query_by = $http_data['by']; + if (!is_array($query_by)) { + $query_by = array($query_by); + } + } else { + if ($type == "multiinfo") { + $query_by = array('name'); + } else { // search + $query_by = array('name', 'description'); + } + } + + if ($type == "multiinfo") { + $args = $http_data['arg']; + if (!is_array($args)) { + $args = array($args); + } + foreach ($args as $i => $arg) { + $args[$i] = $this->dbh->quote($arg); + } + $op_rhs = " IN (" . implode(",", $args) . ")"; + } else { + $keyword_string = $http_data['arg']; + $keyword_string = $this->dbh->quote("%" . addcslashes($keyword_string, '%_') . "%"); + $op_rhs = " LIKE " . $keyword_string; + } + + $has_provides_query = false; + $where_condition = ""; + foreach ($query_by as $index => $by) { + if ($index != 0) { + $where_condition .= " OR "; + } + if ($by == "provides") { + $has_provides_query = true; + $where_condition .= "(RelationTypes.Name = 'provides' AND "; + $where_condition .= "PackageRelations.RelName $op_rhs)"; + } else { + $where_condition .= self::$exposed_fields_map_v6[$by]; + $where_condition .= $op_rhs; + } + } + + $max_results = config_get_int('options', 'max_rpc_results'); + $fields = implode(',', self::$fields_v4); + $q = "SELECT {$fields} " . + "FROM Packages LEFT JOIN PackageBases " . + "ON PackageBases.ID = Packages.PackageBaseID " . + "LEFT JOIN Users " . + "ON PackageBases.MaintainerUID = Users.ID "; + if ($has_provides_query) { + $q .= "LEFT JOIN PackageRelations ON PackageRelations.PackageID = Packages.ID "; + $q .= "LEFT JOIN RelationTypes ON RelationTypes.ID = PackageRelations.RelTypeID "; + } + $q .= "WHERE ${where_condition} "; + $q .= "AND PackageBases.PackagerUID IS NOT NULL "; + if ($has_provides_query) { + $q .= "GROUP BY Packages.ID "; + } + $q .= "LIMIT $max_results"; + + $result = $this->dbh->query($q); + + return $this->process_result($type, $result); + } + /* * Returns the info on a specific package. * @@ -680,4 +796,3 @@ class AurJSON { return json_encode($output); } } - -- 2.17.1
Hi, this is a WIP patch following up the one at March: [PATCH] feat(rpc): return "providers" packages when querying by `name` or `name-desc` (I failed to make git-send-email to reply to the thread) The goal is to allow tools to use the RPC to resolve dependencies reliably, by introducing a new version of the RPC (v6): 1. Support querying by the provides field 2. Support querying with multiple _by_, for example, by[]=name&by[]=provides The design pretty much follows the one discussed (in the thread mentioned above) previously, I can bring up more context if needed. Here's a draft document of how it works: https://github.com/afg984/aurweb/wiki/aurweb-RPC-Interface-(v6-draft) This is a WIP patch, and I'd like to get some feedback before moving on. There are a few things worth noting: 1. commit 1ff40987 implemented search by depends, checkdepends, optdepends. They are currently left out. 2. v5 allowed searching for orphan packages by leaving out the arg= argument. This is left out as well. 3. as search and info is now a similar concept (and a little different than those in v5), I've implemented them in a separate function. I'd like to know if this is the way to go, or I should try to reuse the existing structure. 2018-06-01 10:50 GMT+08:00 Li-Yu Yu <afg984@gmail.com>:
--- web/lib/aurjson.class.php | 125 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 120 insertions(+), 5 deletions(-)
diff --git a/web/lib/aurjson.class.php b/web/lib/aurjson.class.php index c51e9c2..db11117 100644 --- a/web/lib/aurjson.class.php +++ b/web/lib/aurjson.class.php @@ -20,6 +20,14 @@ class AurJSON { 'name', 'name-desc', 'maintainer', 'depends', 'makedepends', 'checkdepends', 'optdepends' ); + private static $exposed_fields_v6 = array( + 'name', 'description', 'maintainer', 'provides', + ); + private static $exposed_fields_map_v6 = array( + 'name' => 'Packages.Name', + 'description' => 'Packages.Description', + 'maintainer' => 'Packages.Maintainer', + ); private static $exposed_depfields = array( 'depends', 'makedepends', 'checkdepends', 'optdepends' ); @@ -80,7 +88,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.'); }
@@ -94,8 +102,21 @@ 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'])) { + if ($this->version < 6) { + if (!in_array($http_data['by'], self::$exposed_fields)) { + return $this->json_error('Incorrect by field specified.'); + } + } else { + if (!is_array($http_data['by'])) { + $http_data['by'] = array($http_data['by']); + } + foreach ($http_data['by'] as $by) { + if (!in_array($by, self::$exposed_fields_v6)) { + return $this->json_error("Incorrect by field '$by' specified."); + } + } + } }
$this->dbh = DB::connect(); @@ -109,7 +130,11 @@ class AurJSON { if ($type == 'info' && $this->version >= 5) { $type = 'multiinfo'; } - $json = call_user_func(array(&$this, $type), $http_data); + if ($this->version < 6) { + $json = call_user_func(array(&$this, $type), $http_data); + } else { + $json = $this->info_search_v6($type, $http_data); + }
$etag = md5($json); header("Etag: \"$etag\""); @@ -374,6 +399,21 @@ class AurJSON { } $result = $this->dbh->query($query);
+ return $this->process_result($type, $result); + } + + + /* + * Retrieve package information from a dbh->query result + * + * @param $type The request type. + * @param $result A dbh->query result. + * + * @return mixed Returns an array of package matches. + */ + private function process_result($type, $result) { + $max_results = config_get_int('options', 'max_rpc_results'); + if ($result) { $resultcount = 0; $search_data = array(); @@ -515,6 +555,82 @@ class AurJSON { return $this->process_query('search', $where_condition); }
+ /* + * Performs a info or search query to the package database. + * + * @param $type The request type. + * @param array $http_data Query parameters. + * + * @return mixed Returns an array of package matches. + */ + private function info_search_v6($type, $http_data) { + if (isset($http_data['by'])) { + $query_by = $http_data['by']; + if (!is_array($query_by)) { + $query_by = array($query_by); + } + } else { + if ($type == "multiinfo") { + $query_by = array('name'); + } else { // search + $query_by = array('name', 'description'); + } + } + + if ($type == "multiinfo") { + $args = $http_data['arg']; + if (!is_array($args)) { + $args = array($args); + } + foreach ($args as $i => $arg) { + $args[$i] = $this->dbh->quote($arg); + } + $op_rhs = " IN (" . implode(",", $args) . ")"; + } else { + $keyword_string = $http_data['arg']; + $keyword_string = $this->dbh->quote("%" . addcslashes($keyword_string, '%_') . "%"); + $op_rhs = " LIKE " . $keyword_string; + } + + $has_provides_query = false; + $where_condition = ""; + foreach ($query_by as $index => $by) { + if ($index != 0) { + $where_condition .= " OR "; + } + if ($by == "provides") { + $has_provides_query = true; + $where_condition .= "(RelationTypes.Name = 'provides' AND "; + $where_condition .= "PackageRelations.RelName $op_rhs)"; + } else { + $where_condition .= self::$exposed_fields_map_v6[$by]; + $where_condition .= $op_rhs; + } + } + + $max_results = config_get_int('options', 'max_rpc_results'); + $fields = implode(',', self::$fields_v4); + $q = "SELECT {$fields} " . + "FROM Packages LEFT JOIN PackageBases " . + "ON PackageBases.ID = Packages.PackageBaseID " . + "LEFT JOIN Users " . + "ON PackageBases.MaintainerUID = Users.ID "; + if ($has_provides_query) { + $q .= "LEFT JOIN PackageRelations ON PackageRelations.PackageID = Packages.ID "; + $q .= "LEFT JOIN RelationTypes ON RelationTypes.ID = PackageRelations.RelTypeID "; + } + $q .= "WHERE ${where_condition} "; + $q .= "AND PackageBases.PackagerUID IS NOT NULL "; + if ($has_provides_query) { + $q .= "GROUP BY Packages.ID "; + } + $q .= "LIMIT $max_results"; + + $result = $this->dbh->query($q); + + return $this->process_result($type, $result); + } + /* * Returns the info on a specific package. * @@ -680,4 +796,3 @@ class AurJSON { return json_encode($output); } } - -- 2.17.1
On Fri, 01 Jun 2018 at 05:24:16, 尤立宇 wrote:
Hi, this is a WIP patch following up the one at March: [PATCH] feat(rpc): return "providers" packages when querying by `name` or `name-desc` (I failed to make git-send-email to reply to the thread)
The goal is to allow tools to use the RPC to resolve dependencies reliably, by introducing a new version of the RPC (v6): 1. Support querying by the provides field 2. Support querying with multiple _by_, for example, by[]=name&by[]=provides
The design pretty much follows the one discussed (in the thread mentioned above) previously, I can bring up more context if needed. Here's a draft document of how it works: https://github.com/afg984/aurweb/wiki/aurweb-RPC-Interface-(v6-draft)
The draft looks good to me. I only had a quick look at the implementation and did not spot any obvious problems, apart from some whitespace issues. Will have a more detailed look later. Thanks for working on this! Regards, Lukas
On 5/31/18 11:24 PM, 尤立宇 wrote:
Hi, this is a WIP patch following up the one at March: [PATCH] feat(rpc): return "providers" packages when querying by `name` or `name-desc` (I failed to make git-send-email to reply to the thread)
The goal is to allow tools to use the RPC to resolve dependencies reliably, by introducing a new version of the RPC (v6): 1. Support querying by the provides field 2. Support querying with multiple _by_, for example, by[]=name&by[]=provides
The design pretty much follows the one discussed (in the thread mentioned above) previously, I can bring up more context if needed. Here's a draft document of how it works: https://github.com/afg984/aurweb/wiki/aurweb-RPC-Interface-(v6-draft)
This is a WIP patch, and I'd like to get some feedback before moving on.
There are a few things worth noting: 1. commit 1ff40987 implemented search by depends, checkdepends, optdepends. They are currently left out. 2. v5 allowed searching for orphan packages by leaving out the arg= argument. This is left out as well. 3. as search and info is now a similar concept (and a little different than those in v5), I've implemented them in a separate function. I'd like to know if this is the way to go, or I should try to reuse the existing structure.
Can you update doc/rpc.txt to describe the new version? Looks like the github wiki contains partial work towards this, but it's not in asciidoc format. -- Eli Schwartz Bug Wrangler and Trusted User
On 5/31/18 11:24 PM, 尤立宇 wrote:
Hi, this is a WIP patch following up the one at March: [PATCH] feat(rpc): return "providers" packages when querying by `name` or `name-desc` (I failed to make git-send-email to reply to the thread)
The goal is to allow tools to use the RPC to resolve dependencies reliably, by introducing a new version of the RPC (v6): 1. Support querying by the provides field 2. Support querying with multiple _by_, for example, by[]=name&by[]=provides
The design pretty much follows the one discussed (in the thread mentioned above) previously, I can bring up more context if needed. Here's a draft document of how it works: https://github.com/afg984/aurweb/wiki/aurweb-RPC-Interface-(v6-draft)
This is a WIP patch, and I'd like to get some feedback before moving on.
There are a few things worth noting: 1. commit 1ff40987 implemented search by depends, checkdepends, optdepends. They are currently left out.
This means we're in a funny situation where in order to get depfields searches we would need to still recommend the v5 interface. Could we implement this as well? If not, we should document it...
2. v5 allowed searching for orphan packages by leaving out the arg= argument. This is left out as well. 3. as search and info is now a similar concept (and a little different than those in v5), I've implemented them in a separate function. I'd like to know if this is the way to go, or I should try to reuse the existing structure.
2018-06-01 10:50 GMT+08:00 Li-Yu Yu <afg984@gmail.com>:
--- web/lib/aurjson.class.php | 125 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 120 insertions(+), 5 deletions(-)
diff --git a/web/lib/aurjson.class.php b/web/lib/aurjson.class.php index c51e9c2..db11117 100644 --- a/web/lib/aurjson.class.php +++ b/web/lib/aurjson.class.php @@ -20,6 +20,14 @@ class AurJSON { 'name', 'name-desc', 'maintainer', 'depends', 'makedepends', 'checkdepends', 'optdepends' ); + private static $exposed_fields_v6 = array( + 'name', 'description', 'maintainer', 'provides', + ); + private static $exposed_fields_map_v6 = array( + 'name' => 'Packages.Name', + 'description' => 'Packages.Description', + 'maintainer' => 'Packages.Maintainer', + );
Why is this pulling indexes from a map? It's only ever used once, and I don't think listing search qualifiers here really helps code clarity vs. a case statement. Moreso when we need to drop the special handling of finding NULL maintainers, and don't do provides here because it doesn't fit into the architecture at all. -- Eli Schwartz Bug Wrangler and Trusted User
Eli Schwartz <eschwartz@archlinux.org>:
Can you update doc/rpc.txt to describe the new version? Looks like the github wiki contains partial work towards this, but it's not in asciidoc format. Sure.
Eli Schwartz <eschwartz@archlinux.org>:
This is a WIP patch, and I'd like to get some feedback before moving on.
There are a few things worth noting: 1. commit 1ff40987 implemented search by depends, checkdepends, optdepends. They are currently left out.
This means we're in a funny situation where in order to get depfields searches we would need to still recommend the v5 interface. Could we implement this as well? If not, we should document it... Of course, v6 should absolutely support what is possible now in v5. The previous patch is more about asking if we can agree on the interface. I'll work on to bring the v6 patch up to date with the new rpc methods supported in v5.
2. v5 allowed searching for orphan packages by leaving out the arg= argument. This is left out as well. 3. as search and info is now a similar concept (and a little different than those in v5), I've implemented them in a separate function. I'd like to know if this is the way to go, or I should try to reuse the existing structure.
2018-06-01 10:50 GMT+08:00 Li-Yu Yu <afg984@gmail.com>:
--- web/lib/aurjson.class.php | 125 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 120 insertions(+), 5 deletions(-)
diff --git a/web/lib/aurjson.class.php b/web/lib/aurjson.class.php index c51e9c2..db11117 100644 --- a/web/lib/aurjson.class.php +++ b/web/lib/aurjson.class.php @@ -20,6 +20,14 @@ class AurJSON { 'name', 'name-desc', 'maintainer', 'depends', 'makedepends', 'checkdepends', 'optdepends' ); + private static $exposed_fields_v6 = array( + 'name', 'description', 'maintainer', 'provides', + ); + private static $exposed_fields_map_v6 = array( + 'name' => 'Packages.Name', + 'description' => 'Packages.Description', + 'maintainer' => 'Packages.Maintainer', + );
Why is this pulling indexes from a map? It's only ever used once, and I don't think listing search qualifiers here really helps code clarity vs. a case statement. I'll fix it.
Moreso when we need to drop the special handling of finding NULL maintainers and don't do provides here because it doesn't fit into the architecture at all. I don't quite get it. Do you mean support for searching by NULL maintainers should be dropped?
On 11/6/18 2:07 AM, 尤立宇 wrote:
Eli Schwartz <eschwartz@archlinux.org>:
Can you update doc/rpc.txt to describe the new version? Looks like the github wiki contains partial work towards this, but it's not in asciidoc format. Sure.
Eli Schwartz <eschwartz@archlinux.org>:
This is a WIP patch, and I'd like to get some feedback before moving on.
There are a few things worth noting: 1. commit 1ff40987 implemented search by depends, checkdepends, optdepends. They are currently left out.
This means we're in a funny situation where in order to get depfields searches we would need to still recommend the v5 interface. Could we implement this as well? If not, we should document it... Of course, v6 should absolutely support what is possible now in v5. The previous patch is more about asking if we can agree on the interface. I'll work on to bring the v6 patch up to date with the new rpc methods supported in v5.
Great! Because other than that, the new interface looks reasonable to me too, so I'd like to see this go forward as well.
Why is this pulling indexes from a map? It's only ever used once, and I don't think listing search qualifiers here really helps code clarity vs. a case statement. I'll fix it.
Moreso when we need to drop the special handling of finding NULL maintainers and don't do provides here because it doesn't fit into the architecture at all. I don't quite get it. Do you mean support for searching by NULL maintainers should be dropped?
Sorry, that may have been confusing. I mean that keeping everything in a map over here doesn't make sense when in order to properly handle searching for NULL maintainers and searching for provides, we (I think) need a switch later on anyway. This is in addition to my earlier point about my belief that the reading of the logic flow would benefit from making the different cases all appear in a switch rather than some being in an if statement and some being included via a map lookup. -- Eli Schwartz Bug Wrangler and Trusted User
On 11/6/18 2:07 AM, 尤立宇 wrote:
Of course, v6 should absolutely support what is possible now in v5. The previous patch is more about asking if we can agree on the interface. I'll work on to bring the v6 patch up to date with the new rpc methods supported in v5.
Any update? -- Eli Schwartz Bug Wrangler and Trusted User
I'm a bit busy these days, but I think I can have a working patch around the weekend Eli Schwartz <eschwartz@archlinux.org> 於 2018年12月11日 週二 下午10:44寫道：
On 11/6/18 2:07 AM, 尤立宇 wrote:
Of course, v6 should absolutely support what is possible now in v5. The previous patch is more about asking if we can agree on the interface. I'll work on to bring the v6 patch up to date with the new rpc methods supported in v5.
Any update?
-- Eli Schwartz Bug Wrangler and Trusted User
On Wed, 12 Dec 2018 at 17:13, 尤立宇 <afg984@gmail.com> wrote:
I'm a bit busy these days, but I think I can have a working patch around the weekend
Eli Schwartz <eschwartz@archlinux.org> 於 2018年12月11日 週二 下午10:44寫道：
On 11/6/18 2:07 AM, 尤立宇 wrote:
Of course, v6 should absolutely support what is possible now in v5. The previous patch is more about asking if we can agree on the interface. I'll work on to bring the v6 patch up to date with the new rpc methods supported in v5.
Any update?
-- Eli Schwartz Bug Wrangler and Trusted User
Long weekend huh ;) Any update on this?
Morgan Adamiec <morganamilo@gmail.com> 於 2019年1月24日 週四 上午5:08寫道：
Long weekend huh ;)
Any update on this?
Unfortunately, I don't have time to update the patch, my limited PHP knowledge does not allow me to work on it casually. If anyone wants to, feel free to take them. I think the code would follow whatever license aurweb currently is. Looks like someone is already working on it though. Probably this can become a ticket in the issue tracker (if there isn't already one) so anyone interested can see the progress easier IMO, instead of digging through the mailing list.
participants (6)
-
Eli Schwartz
-
Li-Yu Yu
-
Lukas Fleischer
-
Morgan Adamiec
-
morganamilo
-
尤立宇