From cd23262e41fc51eefe6aad403f777a21df93bb22 Mon Sep 17 00:00:00 2001 From: Quentin Machu Date: Fri, 11 Nov 2016 19:16:40 +0100 Subject: [PATCH 1/2] pgsql: Do not insert entry in Vulnerability_FixedIn_Feature if existing Fixes #238 --- database/pgsql/queries.go | 14 ++++++++++---- database/pgsql/vulnerability.go | 13 ++++++++++--- database/pgsql/vulnerability_test.go | 19 +++++++++---------- 3 files changed, 29 insertions(+), 17 deletions(-) diff --git a/database/pgsql/queries.go b/database/pgsql/queries.go index 9cff94b8..a7d04ca1 100644 --- 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 index 74ee9828..6fbb7251 100644 --- 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 index d20c2e35..86edd1c1 100644 --- 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) } } From ec0aad9b7a4e8021b633f983f2e1d9ef496bf49c Mon Sep 17 00:00:00 2001 From: Quentin Machu Date: Sat, 12 Nov 2016 15:42:59 +0100 Subject: [PATCH 2/2] pgsql: Use booleans instead of varchar to return creation status --- database/pgsql/feature.go | 9 +++++---- database/pgsql/queries.go | 8 ++++---- database/pgsql/vulnerability.go | 8 ++++---- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/database/pgsql/feature.go b/database/pgsql/feature.go index a2f2abe8..c0b5a350 100644 --- a/database/pgsql/feature.go +++ b/database/pgsql/feature.go @@ -129,11 +129,11 @@ func (pgSQL *pgSQL) insertFeatureVersion(featureVersion database.FeatureVersion) } // Find or create FeatureVersion. - var newOrExisting string + var created bool t = time.Now() err = tx.QueryRow(soiFeatureVersion, featureID, &featureVersion.Version). - Scan(&newOrExisting, &featureVersion.ID) + Scan(&created, &featureVersion.ID) observeQueryTime("insertFeatureVersion", "soiFeatureVersion", t) if err != nil { @@ -141,8 +141,9 @@ func (pgSQL *pgSQL) insertFeatureVersion(featureVersion database.FeatureVersion) return 0, handleError("soiFeatureVersion", err) } - if newOrExisting == "exi" { - // That featureVersion already exists, return its id. + if !created { + // The featureVersion already existed, no need to link it to + // vulnerabilities. tx.Commit() if pgSQL.cache != nil { diff --git a/database/pgsql/queries.go b/database/pgsql/queries.go index a7d04ca1..99dec448 100644 --- a/database/pgsql/queries.go +++ b/database/pgsql/queries.go @@ -63,9 +63,9 @@ const ( WHERE NOT EXISTS (SELECT id FROM FeatureVersion WHERE feature_id = $1 AND version = $2) RETURNING id ) - SELECT 'exi', id FROM FeatureVersion WHERE feature_id = $1 AND version = $2 + SELECT false, id FROM FeatureVersion WHERE feature_id = $1 AND version = $2 UNION - SELECT 'new', id FROM new_featureversion` + SELECT true, id FROM new_featureversion` searchVulnerabilityFixedInFeature = ` SELECT id, vulnerability_id, version FROM Vulnerability_FixedIn_Feature @@ -167,9 +167,9 @@ const ( 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 + SELECT false, id FROM Vulnerability_FixedIn_Feature WHERE vulnerability_id = $1 AND feature_id = $2 UNION - SELECT 'new', id FROM new_fixedinfeature` + SELECT true, 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 index 6fbb7251..022aafec 100644 --- a/database/pgsql/vulnerability.go +++ b/database/pgsql/vulnerability.go @@ -433,22 +433,22 @@ func (pgSQL *pgSQL) insertVulnerabilityFixedInFeatureVersions(tx *sql.Tx, vulner for _, fv := range fixedIn { var fixedInID int - var newOrExisting string + var created bool // Find or create entry in Vulnerability_FixedIn_Feature. err = tx.QueryRow( soiVulnerabilityFixedInFeature, vulnerabilityID, fv.Feature.ID, &fv.Version, - ).Scan(&newOrExisting, &fixedInID) + ).Scan(&created, &fixedInID) if err != nil { return handleError("insertVulnerabilityFixedInFeature", err) } - if newOrExisting == "exi" { + if !created { // The relationship between the feature and the vulnerability already - // exists, there's no need to update Vulnerability_Affects_FeatureVersion. + // existed, no need to update Vulnerability_Affects_FeatureVersion. continue }