Store dependency descriptions in a separate column
diff mbox

Message ID 20170419072001.9776-1-lfleischer@archlinux.org
State Accepted, archived
Headers show

Commit Message

Lukas Fleischer April 19, 2017, 7:20 a.m. UTC
Split optional dependency descriptions from dependency names before
storing them in the database and use a separate column to store the
descriptions.

This allows us to simplify and optimize the SQL queries in
pkg_dependencies() as well as pkg_required().

Suggested-by: Florian Pritz <bluewind@xinu.at>
Signed-off-by: Lukas Fleischer <lfleischer@archlinux.org>
---
 aurweb/git/update.py         | 16 +++++++---------
 schema/aur-schema.sql        |  1 +
 upgrading/4.6.0.txt          | 11 +++++++++++
 web/lib/pkgfuncs.inc.php     | 33 ++++++++-------------------------
 web/template/pkg_details.php |  2 +-
 5 files changed, 28 insertions(+), 35 deletions(-)
 create mode 100644 upgrading/4.6.0.txt

Comments

Lukas Fleischer via aur-dev April 19, 2017, 11:33 a.m. UTC | #1
On 19.04.2017 09:20, Lukas Fleischer wrote:
> Split optional dependency descriptions from dependency names before
> storing them in the database and use a separate column to store the
> descriptions.
> 
> This allows us to simplify and optimize the SQL queries in
> pkg_dependencies() as well as pkg_required().
> 
> Suggested-by: Florian Pritz <bluewind@xinu.at>
> Signed-off-by: Lukas Fleischer <lfleischer@archlinux.org>

The patch looks good, thanks!

Florian

Patch
diff mbox

diff --git a/aurweb/git/update.py b/aurweb/git/update.py
index 532db92..09a57ef 100755
--- a/aurweb/git/update.py
+++ b/aurweb/git/update.py
@@ -52,10 +52,7 @@  def parse_dep(depstring):
     depname = re.sub(r'(<|=|>).*', '', dep)
     depcond = dep[len(depname):]
 
-    if (desc):
-        return (depname + ': ' + desc, depcond)
-    else:
-        return (depname, depcond)
+    return (depname, desc, depcond)
 
 
 def create_pkgbase(conn, pkgbase, user):
@@ -141,12 +138,13 @@  def save_metadata(metadata, conn, user):
                                [deptype])
             deptypeid = cur.fetchone()[0]
             for dep_info in extract_arch_fields(pkginfo, deptype):
-                depname, depcond = parse_dep(dep_info['value'])
+                depname, depdesc, depcond = parse_dep(dep_info['value'])
                 deparch = dep_info['arch']
                 conn.execute("INSERT INTO PackageDepends (PackageID, " +
-                             "DepTypeID, DepName, DepCondition, DepArch) " +
-                             "VALUES (?, ?, ?, ?, ?)",
-                             [pkgid, deptypeid, depname, depcond, deparch])
+                             "DepTypeID, DepName, DepDesc, DepCondition, " +
+                             "DepArch) VALUES (?, ?, ?, ?, ?, ?)",
+                             [pkgid, deptypeid, depname, depdesc, depcond,
+                              deparch])
 
         # Add package relations (conflicts, provides, replaces).
         for reltype in ('conflicts', 'provides', 'replaces'):
@@ -154,7 +152,7 @@  def save_metadata(metadata, conn, user):
                                [reltype])
             reltypeid = cur.fetchone()[0]
             for rel_info in extract_arch_fields(pkginfo, reltype):
-                relname, relcond = parse_dep(rel_info['value'])
+                relname, _, relcond = parse_dep(rel_info['value'])
                 relarch = rel_info['arch']
                 conn.execute("INSERT INTO PackageRelations (PackageID, " +
                              "RelTypeID, RelName, RelCondition, RelArch) " +
diff --git a/schema/aur-schema.sql b/schema/aur-schema.sql
index 89167b3..be6f3e5 100644
--- a/schema/aur-schema.sql
+++ b/schema/aur-schema.sql
@@ -187,6 +187,7 @@  CREATE TABLE PackageDepends (
 	PackageID INTEGER UNSIGNED NOT NULL,
 	DepTypeID TINYINT UNSIGNED NOT NULL,
 	DepName VARCHAR(255) NOT NULL,
+	DepDesc VARCHAR(255) NULL DEFAULT NULL,
 	DepCondition VARCHAR(255),
 	DepArch VARCHAR(255) NULL DEFAULT NULL,
 	FOREIGN KEY (PackageID) REFERENCES Packages(ID) ON DELETE CASCADE,
diff --git a/upgrading/4.6.0.txt b/upgrading/4.6.0.txt
new file mode 100644
index 0000000..45740f4
--- /dev/null
+++ b/upgrading/4.6.0.txt
@@ -0,0 +1,11 @@ 
+1. Add DepDesc column to PackageDepends and split dependency names:
+
+---
+ALTER TABLE PackageDepends ADD COLUMN DepDesc VARCHAR(255) NULL DEFAULT NULL;
+UPDATE PackageDepends
+	SET DepDesc = SUBSTRING(DepName FROM POSITION(': ' IN DepName) + 2)
+	WHERE POSITION(': ' IN DepName) > 0;
+UPDATE PackageDepends
+	SET DepName = SUBSTRING(DepName FROM 1 FOR POSITION(': ' IN DepName) - 1)
+	WHERE POSITION(': ' IN DepName) > 0;
+---
diff --git a/web/lib/pkgfuncs.inc.php b/web/lib/pkgfuncs.inc.php
index adb21f6..5c48a8e 100644
--- a/web/lib/pkgfuncs.inc.php
+++ b/web/lib/pkgfuncs.inc.php
@@ -239,9 +239,10 @@  function pkg_dependencies($pkgid, $limit) {
 	$pkgid = intval($pkgid);
 	if ($pkgid > 0) {
 		$dbh = DB::connect();
-		$q = "SELECT pd.DepName, dt.Name, pd.DepCondition, pd.DepArch, p.ID FROM PackageDepends pd ";
+		$q = "SELECT pd.DepName, dt.Name, pd.DepDesc, ";
+		$q.= "pd.DepCondition, pd.DepArch, p.ID ";
+		$q.= "FROM PackageDepends pd ";
 		$q.= "LEFT JOIN Packages p ON pd.DepName = p.Name ";
-		$q.= "OR SUBSTRING(pd.DepName FROM 1 FOR POSITION(': ' IN pd.DepName) - 1) = p.Name ";
 		$q.= "LEFT JOIN DependencyTypes dt ON dt.ID = pd.DepTypeID ";
 		$q.= "WHERE pd.PackageID = ". $pkgid . " ";
 		$q.= "ORDER BY pd.DepName LIMIT " . intval($limit);
@@ -354,21 +355,14 @@  function pkg_provider_link($name, $official) {
  *
  * @param string $name The name of the dependency
  * @param string $type The name of the dependency type
+ * @param string $desc The (optional) description of the dependency
  * @param string $cond The package dependency condition string
  * @param string $arch The package dependency architecture
  * @param int $pkg_id The package of the package to display the dependency for
  *
  * @return string The HTML code of the label to display
  */
-function pkg_depend_link($name, $type, $cond, $arch, $pkg_id) {
-	if ($type == 'optdepends' && strpos($name, ':') !== false) {
-		$tokens = explode(':', $name, 2);
-		$name = $tokens[0];
-		$desc = $tokens[1];
-	} else {
-		$desc = '';
-	}
-
+function pkg_depend_link($name, $type, $desc, $cond, $arch, $pkg_id) {
 	/*
 	 * TODO: We currently perform one SQL query per nonexistent package
 	 * dependency. It would be much better if we could annotate dependency
@@ -432,25 +426,14 @@  function pkg_depend_link($name, $type, $cond, $arch, $pkg_id) {
  * @return string The HTML code of the link to display
  */
 function pkg_requiredby_link($name, $depends, $type, $arch, $pkgname) {
-	if ($type == 'optdepends' && strpos($name, ':') !== false) {
-		$tokens = explode(':', $name, 2);
-		$name = $tokens[0];
-	}
-
 	$link = '<a href="';
 	$link .= htmlspecialchars(get_pkg_uri($name), ENT_QUOTES);
 	$link .= '" title="' . __('View packages details for') .' ' . htmlspecialchars($name) . '">';
 	$link .= htmlspecialchars($name) . '</a>';
 
 	if ($depends != $pkgname) {
-		$depname = $depends;
-		if (strpos($depends, ':') !== false) {
-			$tokens = explode(':', $depname, 2);
-			$depname = $tokens[0];
-		}
-
 		$link .= ' <span class="virtual-dep">(';
-		$link .= __('requires %s', htmlspecialchars($depname));
+		$link .= __('requires %s', htmlspecialchars($depends));
 		$link .= ')</span>';
 	}
 
@@ -522,11 +505,11 @@  function pkg_required($name="", $provides, $limit) {
 			$name_list .= ',' . $dbh->quote($p[0]);
 		}
 
-		$q = "SELECT p.Name, pd.DepName, dt.Name, pd.DepArch FROM PackageDepends pd ";
+		$q = "SELECT p.Name, pd.DepName, dt.Name, pd.DepArch ";
+		$q.= "FROM PackageDepends pd ";
 		$q.= "LEFT JOIN Packages p ON p.ID = pd.PackageID ";
 		$q.= "LEFT JOIN DependencyTypes dt ON dt.ID = pd.DepTypeID ";
 		$q.= "WHERE pd.DepName IN (" . $name_list . ") ";
-		$q.= "OR SUBSTRING(pd.DepName FROM 1 FOR POSITION(': ' IN pd.DepName) - 1) IN (" . $name_list . ") ";
 		$q.= "ORDER BY p.Name LIMIT " . intval($limit);
 		$result = $dbh->query($q);
 		if (!$result) {return array();}
diff --git a/web/template/pkg_details.php b/web/template/pkg_details.php
index ed8974a..8a93417 100644
--- a/web/template/pkg_details.php
+++ b/web/template/pkg_details.php
@@ -277,7 +277,7 @@  endif;
 <?php if (count($deps) > 0): ?>
 			<ul id="pkgdepslist">
 <?php while (list($k, $darr) = each($deps)): ?>
-	<li><?= pkg_depend_link($darr[0], $darr[1], $darr[2], $darr[3], $darr[4]); ?></li>
+	<li><?= pkg_depend_link($darr[0], $darr[1], $darr[2], $darr[3], $darr[4], $darr[5]); ?></li>
 <?php endwhile; ?>
 			</ul>
 <?php endif; ?>