pgsql: Do not insert entry in Vulnerability_FixedIn_Feature if existing

Fixes #238
quay · Nov 11, 2016 · cd23262 · cd23262
1 parent eed4a9a
commit cd23262
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 17 deletions.
diff --git a/database/pgsql/queries.go b/database/pgsql/queries.go
@@ -160,10 +160,16 @@ const (
 		VALUES($1, $2, $3, $4, $5, $6, CURRENT_TIMESTAMP)
 		RETURNING id`
 
-	insertVulnerabilityFixedInFeature = `
-		INSERT INTO Vulnerability_FixedIn_Feature(vulnerability_id, feature_id, version)
-		VALUES($1, $2, $3)
-		RETURNING id`
+	soiVulnerabilityFixedInFeature = `
+		WITH new_fixedinfeature AS (
+			INSERT INTO Vulnerability_FixedIn_Feature(vulnerability_id, feature_id, version)
+			SELECT CAST($1 AS INTEGER), CAST($2 AS INTEGER), CAST($3 AS VARCHAR)
+			WHERE NOT EXISTS (SELECT id FROM Vulnerability_FixedIn_Feature WHERE vulnerability_id = $1 AND feature_id = $2)
+			RETURNING id
+		)
+		SELECT 'exi', id FROM Vulnerability_FixedIn_Feature WHERE vulnerability_id = $1 AND feature_id = $2
+		UNION
+		SELECT 'new', id FROM new_fixedinfeature`
 
 	searchFeatureVersionByFeature = `SELECT id, version FROM FeatureVersion WHERE feature_id = $1`
 

diff --git a/database/pgsql/vulnerability.go b/database/pgsql/vulnerability.go
@@ -433,18 +433,25 @@ func (pgSQL *pgSQL) insertVulnerabilityFixedInFeatureVersions(tx *sql.Tx, vulner
 
 	for _, fv := range fixedIn {
 		var fixedInID int
+		var newOrExisting string
 
-		// Insert Vulnerability_FixedIn_Feature.
+		// Find or create entry in Vulnerability_FixedIn_Feature.
 		err = tx.QueryRow(
-			insertVulnerabilityFixedInFeature,
+			soiVulnerabilityFixedInFeature,
 			vulnerabilityID, fv.Feature.ID,
 			&fv.Version,
-		).Scan(&fixedInID)
+		).Scan(&newOrExisting, &fixedInID)
 
 		if err != nil {
 			return handleError("insertVulnerabilityFixedInFeature", err)
 		}
 
+		if newOrExisting == "exi" {
+			// The relationship between the feature and the vulnerability already
+			// exists, there's no need to update Vulnerability_Affects_FeatureVersion.
+			continue
+		}
+
 		// Insert Vulnerability_Affects_FeatureVersion.
 		err = linkVulnerabilityToFeatureVersions(tx, fixedInID, vulnerabilityID, fv.Feature.ID, fv.Version)
 		if err != nil {

diff --git a/database/pgsql/vulnerability_test.go b/database/pgsql/vulnerability_test.go
@@ -226,25 +226,24 @@ func TestInsertVulnerability(t *testing.T) {
 	v1.Description = "TestInsertVulnerabilityLink2"
 	v1.Link = "TestInsertVulnerabilityLink2"
 	v1.Severity = types.High
-	// Update f3 in f4, add fixed in f5, add fixed in f6 which already exists, removes fixed in f7 by
-	// adding f8 which is f7 but with MinVersion.
-	v1.FixedIn = []database.FeatureVersion{f4, f5, f6, f8}
+	// Update f3 in f4, add fixed in f5, add fixed in f6 which already exists,
+	// removes fixed in f7 by adding f8 which is f7 but with MinVersion, and
+	// add fixed by f5 a second time (duplicated).
+	v1.FixedIn = []database.FeatureVersion{f4, f5, f6, f8, f5}
 
 	err = datastore.InsertVulnerabilities([]database.Vulnerability{v1}, true)
 	if assert.Nil(t, err) {
 		v1f, err := datastore.FindVulnerability(n1.Name, v1.Name)
 		if assert.Nil(t, err) {
+			// Remove f8 from the struct for comparison as it was just here to cancel f7.
+			// Remove one of the f5 too as it was twice in the struct but the database
+			// implementation should have dedup'd it.
+			v1.FixedIn = v1.FixedIn[:len(v1.FixedIn)-2]
+
 			// We already had f1 before the update.
 			// Add it to the struct for comparison.
 			v1.FixedIn = append(v1.FixedIn, f1)
 
-			// Removes f8 from the struct for comparison as it was just here to cancel f7.
-			for i := 0; i < len(v1.FixedIn); i++ {
-				if v1.FixedIn[i].Feature.Name == f8.Feature.Name {
-					v1.FixedIn = append(v1.FixedIn[:i], v1.FixedIn[i+1:]...)
-				}
-			}
-
 			equalsVuln(t, &v1, &v1f)
 		}
 	}