[aur-dev] [PATCH] Extended JSON query method
From: Sylvester Johansson <scj@konservburken.localdomain> --- web/html/rpc.php | 8 +++++- web/lib/aurjson.class.php | 59 +++++++++++++++++++++++++++++++++++++++----- 2 files changed, 59 insertions(+), 8 deletions(-) diff --git a/web/html/rpc.php b/web/html/rpc.php index 033cba5..8ca0f4b 100644 --- a/web/html/rpc.php +++ b/web/html/rpc.php @@ -19,11 +19,17 @@ if ( $_SERVER['REQUEST_METHOD'] == 'GET' ) { echo '<ul>'; echo '<li>search</li>'; echo '<li>info</li>'; + echo '<li>query</li>'; echo '</ul><br />'; - echo 'Each method requires the following HTTP GET syntax:<br />'; + echo '<i>search</i> and <i>info</i> requires the following HTTP GET syntax:<br />'; echo ' type=<i>methodname</i>&arg=<i>data</i> <br /><br />'; echo 'Where <i>methodname</i> is the name of an allowed method, and <i>data</i> is the argument to the call.<br />'; echo '<br />'; + echo '<br />'; + echo '<i>query<i> has the following syntax: <br />'; + echo 'type=query&arg=<<i>term</i>>&include=<<i>field1</i>>:<<i>field2</i>>:...:<<i>fieldN</i>> <br />'; + echo 'where <i>fieldN</i> is a field to be included in the result. Allowed fields are: <br />'; + echo 'ID, Name, Version, Description, URL, URLPath, License, NumVotes and OutOfDate <br />'; echo 'If you need jsonp type callback specification, you can provide an additional variable <i>callback</i>.<br />'; echo 'Example URL: <br /> http://aur-url/rpc.php?type=search&arg=foobar&callback=jsonp1192244621103'; echo '</body></html>'; diff --git a/web/lib/aurjson.class.php b/web/lib/aurjson.class.php index be92c25..770a80a 100644 --- a/web/lib/aurjson.class.php +++ b/web/lib/aurjson.class.php @@ -21,7 +21,9 @@ if (!extension_loaded('json')) **/ class AurJSON { private $dbh = false; - private $exposed_methods = array('search','info'); + private $exposed_methods = array('search','info','query'); + private $valid_parameters = array('ID','Name','Version','Description','URL','URLPath','License','NumVotes','OutOfDate'); + /** * Handles post data, and routes the request. @@ -42,13 +44,18 @@ class AurJSON { // do the routing if ( in_array($http_data['type'], $this->exposed_methods) ) { // ugh. this works. I hate you php. - $json = call_user_func_array(array(&$this,$http_data['type']),$http_data['arg']); - // allow rpc callback for XDomainAjax - if ( isset($http_data['callback']) ) { - return $http_data['callback'] . "({$json})"; + if ($http_data['type'] === 'query') { + return $this->query($http_data['arg'],$http_data['include']); } - else { - return $json; + else { + $json = call_user_func_array(array(&$this,$http_data['type']),$http_data['arg']); + // allow rpc callback for XDomainAjax + if ( isset($http_data['callback']) ) { + return $http_data['callback'] . "({$json})"; + } + else { + return $json; + } } } else { @@ -137,5 +144,43 @@ class AurJSON { return $this->json_error('No result found'); } } + + private function query($value,$pqdata) { + $pqdata = explode(":",$pqdata); + $value = mysql_real_escape_string($value); + + if (! $this->parameter_sanity($pqdata)) { + return $this->json_error('Parameter error'); + } + + $base_query = "SELECT " . implode(',',$pqdata) . " FROM Packages WHERE DummyPkg=0 AND " . sprintf("( Name LIKE '%%%s%%' OR Description LIKE '%%%s%%' )",$value,$value); + $result = db_query($base_query,$this->dbh); + if($result && (mysql_num_rows($result)>0)){ + $search_data = array(); + while($row = mysql_fetch_assoc($result)) { + array_push($search_data,$row); + } + mysql_free_result($result); + return $this->json_results('query',$search_data); + } + else { + return $this->json_error('No result found'); + } + + } + + /** + * @param $parameters is a semicolon separated string of column names + * @return True if the parameters are acceptable, otherwise false + **/ + private function parameter_sanity($parameters) { + foreach($parameters as $param) { + if (!in_array($param, $this->valid_parameters)) { + return false; + } + } + return true; + } } ?> + -- 1.5.5.3
On Tue, 3 Jun 2008 20:25:33 +0200 Sylvester Johansson <syljo361@gmail.com> wrote: Hey Sylvester. I think I like this method better than the other. Could you fix the indentation (especially in function query), put a more descriptive commit message, and resubmit? Thanks.
On 6/3/08, Loui <louipc.ist@gmail.com> wrote:
On Tue, 3 Jun 2008 20:25:33 +0200 Sylvester Johansson <syljo361@gmail.com> wrote:
Hey Sylvester. I think I like this method better than the other. Could you fix the indentation (especially in function query), put a more descriptive commit message, and resubmit? Thanks.
I already outlined on the bugtracker why I thought this wasn't exactly a great idea, as it was implemented. oh well.
On 6/3/08, Loui <louipc.ist@gmail.com> wrote:
On Tue, 3 Jun 2008 20:25:33 +0200 Sylvester Johansson <syljo361@gmail.com> wrote:
Hey Sylvester. I think I like this method better than the other. Could you fix the indentation (especially in function query), put a more descriptive commit message, and resubmit? Thanks.
I already outlined on the bugtracker why I thought this wasn't exactly a great idea, as it was implemented. oh well.
This is a different implementation than the one discussed on the bugtracker. The difference is that with this, you can specify on the client-side what fields the json result should contain, while the other was set server-side. There is no sql joins going on, so it would be a
On Thu, Jun 5, 2008 at 2:14 AM, eliott <eliott@cactuswax.net> wrote: performance increase due to the fact that currently the rpc clients have to do N+1 databasel queries, where N is the number of hits on the initial search. This cuts it down the number of queries to one.
On Wed, 4 Jun 2008 17:14:35 -0700 eliott <eliott@cactuswax.net> wrote:
On 6/3/08, Loui <louipc.ist@gmail.com> wrote:
On Tue, 3 Jun 2008 20:25:33 +0200 Sylvester Johansson <syljo361@gmail.com> wrote:
Hey Sylvester. I think I like this method better than the other. Could you fix the indentation (especially in function query), put a more descriptive commit message, and resubmit? Thanks.
I already outlined on the bugtracker why I thought this wasn't exactly a great idea, as it was implemented. oh well.
This is a different patch from the one in the bug tracker (implementation looks a little cleaner). Do you have the same qualms about this one? I think there should be a way of specifying more info in the search. What's good about this method is that it will only return what's needed, no more, no less. I guess what we could really use are examples of it's usage and potential abuse. At least with this method the server won't be hit as hard as if the client did the search and requested info for every single search result. Maybe we can patch the behaviour later on when the implications are clearer from real life testing eh? Heh, it's obviously a highly requested feature by AUR application devs. I would have written a patch like this myself. :P
On Thu, Jun 05, 2008 at 11:14:22AM -0400, Loui wrote:
This is a different patch from the one in the bug tracker (implementation looks a little cleaner). Do you have the same qualms about this one? I think there should be a way of specifying more info in the search. What's good about this method is that it will only return what's needed, no more, no less.
I think it adds needless complexity, why not just return all data on the specified packages?
I guess what we could really use are examples of it's usage and potential abuse. At least with this method the server won't be hit as hard as if the client did the search and requested info for every single search result.
Bandwidth is cheap, it's mostly looking up on one table for package info, it's not really a performance hit to return more info. -S
On 6/5/08, Simo Leone <simo@archlinux.org> wrote:
On Thu, Jun 05, 2008 at 11:14:22AM -0400, Loui wrote:
This is a different patch from the one in the bug tracker (implementation looks a little cleaner). Do you have the same qualms about this one? I think there should be a way of specifying more info in the search. What's good about this method is that it will only return what's needed, no more, no less.
I think it adds needless complexity, why not just return all data on the specified packages?
That was my argument in the bugtracker ticket also. Making on the fly decisions about what to return for simple select queries is: 1) more expensive than just returning all fields 2) makes it harder for the mysql query cacher to cache it 3) would make it harder for any potential future memcache layer to cache it And this appears at a first look to be doing the same thing as the other ticket, that is: passing in which fields the client wants back, and selecting only those fields from the table, then returning them to the client. I don't see how it is a different implementation. Things just got moved around a bit.
On Thu, 5 Jun 2008 14:47:51 -0700 eliott <eliott@cactuswax.net> wrote:
On 6/5/08, Simo Leone <simo@archlinux.org> wrote:
On Thu, Jun 05, 2008 at 11:14:22AM -0400, Loui wrote:
This is a different patch from the one in the bug tracker (implementation looks a little cleaner). Do you have the same qualms about this one? I think there should be a way of specifying more info in the search. What's good about this method is that it will only return what's needed, no more, no less.
I think it adds needless complexity, why not just return all data on the specified packages?
That was my argument in the bugtracker ticket also.
Making on the fly decisions about what to return for simple select queries is: 1) more expensive than just returning all fields 2) makes it harder for the mysql query cacher to cache it 3) would make it harder for any potential future memcache layer to cache it
And this appears at a first look to be doing the same thing as the other ticket, that is: passing in which fields the client wants back, and selecting only those fields from the table, then returning them to the client.
I don't see how it is a different implementation. Things just got moved around a bit.
Alright I understand. I hadn't thought about caching. So maybe the extra parameter should just enable full info retrieval on searches.
eliott, there are 2 proposed solutions: a) return all fields when searching. b) add a way to get all fields when you search. choose one or propose an alternate solution. On Thu, Jun 5, 2008 at 11:56 PM, Loui <louipc.ist@gmail.com> wrote:
On Thu, 5 Jun 2008 14:47:51 -0700 eliott <eliott@cactuswax.net> wrote:
On Thu, Jun 05, 2008 at 11:14:22AM -0400, Loui wrote:
This is a different patch from the one in the bug tracker (implementation looks a little cleaner). Do you have the same qualms about this one? I think there should be a way of specifying more info in the search. What's good about this method is that it will only return what's needed, no more, no less.
I think it adds needless complexity, why not just return all data on
On 6/5/08, Simo Leone <simo@archlinux.org> wrote: the
specified packages?
That was my argument in the bugtracker ticket also.
Making on the fly decisions about what to return for simple select queries is: 1) more expensive than just returning all fields 2) makes it harder for the mysql query cacher to cache it 3) would make it harder for any potential future memcache layer to cache it
And this appears at a first look to be doing the same thing as the other ticket, that is: passing in which fields the client wants back, and selecting only those fields from the table, then returning them to the client.
I don't see how it is a different implementation. Things just got moved around a bit.
Alright I understand. I hadn't thought about caching. So maybe the extra parameter should just enable full info retrieval on searches.
On 6/6/08, Francesc Ortiz <francescortiz@gmail.com> wrote:
eliott,
there are 2 proposed solutions: a) return all fields when searching. b) add a way to get all fields when you search.
choose one or propose an alternate solution.
I thought I had already stated that I felt returning either all, or probably more optimally returning a reasonable hard-coded subset of fields, was a good idea. I only found fault with the on the fly determination of which fields to return. However, the AUR is now in the capable hands of Simo, Loui, and Callan. The ultimate decision now lay with them. This will be my last post on this mailing list.
participants (5)
-
eliott
-
Francesc Ortiz
-
Loui
-
Simo Leone
-
Sylvester Johansson