diff mbox

[RFC] Allow to search packages by "*depends" fields

Message ID 20180128002055.6006-1-baptiste@bitsofnetworks.org
State Superseded, archived
Headers show

Commit Message

Baptiste Jonglez Jan. 28, 2018, 12:20 a.m. UTC
From: Baptiste Jonglez <git@bitsofnetworks.org>

It is now possible to search for packages that depend on a given package,
for instance:

    /rpc/?v=5&type=search&by=depends&arg=ocaml

It is also possible to match on "makedepends", "checkdepends" and
"optdepends".

Remaining tasks for a complete patch:

- actually test it...
- update documentation
- see if the additional SQL JOINs increase database load
- maybe add something like "by=anydepends" to match across all types of dependencies?

Signed-off-by: Baptiste Jonglez <git@bitsofnetworks.org>
---
 web/lib/aurjson.class.php | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

Comments

Lukas Fleischer Jan. 28, 2018, 12:32 p.m. UTC | #1
On Sun, 28 Jan 2018 at 01:20:55, Baptiste Jonglez wrote:
> From: Baptiste Jonglez <git@bitsofnetworks.org>
> 
> It is now possible to search for packages that depend on a given package,
> for instance:
> 
>     /rpc/?v=5&type=search&by=depends&arg=ocaml
> 
> It is also possible to match on "makedepends", "checkdepends" and
> "optdepends".
> 
> Remaining tasks for a complete patch:
> 
> - actually test it...
> - update documentation
> - see if the additional SQL JOINs increase database load
> - maybe add something like "by=anydepends" to match across all types of dependencies?
> 
> Signed-off-by: Baptiste Jonglez <git@bitsofnetworks.org>
> ---
>  web/lib/aurjson.class.php | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)

I like the overall idea and given the implementation is pretty simple
and straightforward, I do not see anything speaking against it.

The documentation definitely needs to be updated, preferably in the same
patch. One small comment below...

> 
> diff --git a/web/lib/aurjson.class.php b/web/lib/aurjson.class.php
> index 9eeaafd..addb92e 100644
> --- a/web/lib/aurjson.class.php
> +++ b/web/lib/aurjson.class.php
> [...]
> @@ -407,6 +412,15 @@ class AurJSON {
>                                 $keyword_string = $this->dbh->quote($keyword_string);
>                                 $where_condition = "Users.Username = $keyword_string ";
>                         }
> +               } else if ($search_by === 'depends' || $search_by === 'makedepends' || $search_by === 'checkdepends' || $search_by === 'optdepends') {

This can be written in a more compact way, using in_array().

Everything else looks good to me!

Thanks,
Lukas
Baptiste Jonglez Jan. 28, 2018, 9:34 p.m. UTC | #2
Hi Lukas,

Thanks for the positive feedback!

On 28-01-18, Lukas Fleischer wrote:
> I like the overall idea and given the implementation is pretty simple
> and straightforward, I do not see anything speaking against it.

Actually, there was a mistake in the code I sent: the additional SQL JOIN
causes the RPC to return duplicate results for the other types of search
(name, name-desc and maintainer).  Unfortunately, this makes the new code
less straightforward.

I have sent a new patch, can you please review the updated part?

> The documentation definitely needs to be updated, preferably in the same
> patch.

Done in the new patch.

> > @@ -407,6 +412,15 @@ class AurJSON {
> >                                 $keyword_string = $this->dbh->quote($keyword_string);
> >                                 $where_condition = "Users.Username = $keyword_string ";
> >                         }
> > +               } else if ($search_by === 'depends' || $search_by === 'makedepends' || $search_by === 'checkdepends' || $search_by === 'optdepends') {
> 
> This can be written in a more compact way, using in_array().

Updated in the new patch.

> Everything else looks good to me!

Thanks!
Baptiste
diff mbox

Patch

diff --git a/web/lib/aurjson.class.php b/web/lib/aurjson.class.php
index 9eeaafd..addb92e 100644
--- a/web/lib/aurjson.class.php
+++ b/web/lib/aurjson.class.php
@@ -17,7 +17,8 @@  class AurJSON {
 		'suggest-pkgbase', 'get-comment-form'
 	);
 	private static $exposed_fields = array(
-		'name', 'name-desc', 'maintainer'
+		'name', 'name-desc', 'maintainer',
+		'depends', 'makedepends', 'checkdepends', 'optdepends'
 	);
 	private static $fields_v1 = array(
 		'Packages.ID', 'Packages.Name',
@@ -278,6 +279,10 @@  class AurJSON {
 				"ON PackageBases.ID = Packages.PackageBaseID " .
 				"LEFT JOIN Users " .
 				"ON PackageBases.MaintainerUID = Users.ID " .
+				"LEFT JOIN PackageDepends " .
+				"ON Packages.ID = PackageDepends.PackageID " .
+				"LEFT JOIN DependencyTypes " .
+				"ON PackageDepends.DepTypeID = DependencyTypes.ID " .
 				"WHERE ${where_condition} " .
 				"AND PackageBases.PackagerUID IS NOT NULL " .
 				"LIMIT $max_results";
@@ -407,6 +412,15 @@  class AurJSON {
 				$keyword_string = $this->dbh->quote($keyword_string);
 				$where_condition = "Users.Username = $keyword_string ";
 			}
+		} else if ($search_by === 'depends' || $search_by === 'makedepends' || $search_by === 'checkdepends' || $search_by === 'optdepends') {
+			if (empty($keyword_string)) {
+				return $this->json_error('Query arg is empty.');
+			} else {
+				$keyword_string = $this->dbh->quote($keyword_string);
+				$search_string = $this->dbh->quote($search_by);
+				$where_condition = "PackageDepends.DepName = $keyword_string AND ";
+				$where_condition .= "DependencyTypes.Name = $search_string";
+			}
 		}
 
 		return $this->process_query('search', $where_condition);