database: do insert/find layers (with their features and vulnerabilities)

database/pgsql/feature.go

@@ -2,11 +2,11 @@ package pgsql
 
 import (
 	"github.com/coreos/clair/database"
+	cerrors "github.com/coreos/clair/utils/errors"
 	"github.com/coreos/clair/utils/types"
-  cerrors "github.com/coreos/clair/utils/errors"
 )
 
-func (pgSQL *pgSQL) insertFeature(feature database.Feature) (id int, err error) {
+func (pgSQL *pgSQL) insertFeature(feature database.Feature) (int, error) {
 	if feature.Name == "" {
 		return 0, cerrors.NewBadRequestError("could not find/insert invalid Feature")
 	}
@@ -20,25 +20,29 @@ func (pgSQL *pgSQL) insertFeature(feature database.Feature) (id int, err error)
 	// Find or create Namespace.
 	namespaceID, err := pgSQL.insertNamespace(feature.Namespace)
 	if err != nil {
-		return -1, err
+		return 0, err
 	}
 
 	// Find or create Feature.
+	var id int
 	err = pgSQL.QueryRow(getQuery("soi_feature"), feature.Name, namespaceID).Scan(&id)
+	if err != nil {
+		return 0, err
+	}
 
 	if pgSQL.cache != nil {
 		pgSQL.cache.Add("feature:"+feature.Name, id)
 	}
 
-	return
+	return id, nil
 }
 
 func (pgSQL *pgSQL) insertFeatureVersion(featureVersion database.FeatureVersion) (id int, err error) {
-  if featureVersion.Version.String() == "" {
-    return 0, cerrors.NewBadRequestError("could not find/insert invalid FeatureVersion")
-  }
+	if featureVersion.Version.String() == "" {
+		return 0, cerrors.NewBadRequestError("could not find/insert invalid FeatureVersion")
+	}
 
-  if pgSQL.cache != nil {
+	if pgSQL.cache != nil {
 		if id, found := pgSQL.cache.Get("featureversion:" + featureVersion.Feature.Name + ":" +
 			featureVersion.Version.String()); found {
 			return id.(int), nil
@@ -48,23 +52,23 @@ func (pgSQL *pgSQL) insertFeatureVersion(featureVersion database.FeatureVersion)
 	// Find or create Feature first.
 	featureID, err := pgSQL.insertFeature(featureVersion.Feature)
 	if err != nil {
-		return -1, err
+		return 0, err
 	}
 
 	// Begin transaction.
 	tx, err := pgSQL.Begin()
 	if err != nil {
 		tx.Rollback()
-		return -1, err
+		return 0, err
 	}
 
 	// Find or create FeatureVersion.
 	var newOrExisting string
-	err = tx.QueryRow(getQuery("soi_featureversion"), featureID, featureVersion.Version).
+	err = tx.QueryRow(getQuery("soi_featureversion"), featureID, &featureVersion.Version).
 		Scan(&newOrExisting, &featureVersion.ID)
 	if err != nil {
 		tx.Rollback()
-		return -1, err
+		return 0, err
 	}
 	if newOrExisting == "exi" {
 		// That featureVersion already exists, return its id.
@@ -79,14 +83,14 @@ func (pgSQL *pgSQL) insertFeatureVersion(featureVersion database.FeatureVersion)
 	_, err = tx.Exec(getQuery("l_share_vulnerability_fixedin_feature"))
 	if err != nil {
 		tx.Rollback()
-		return -1, err
+		return 0, err
 	}
 
 	// Select every vulnerability and the fixed version that affect this Feature.
 	rows, err := tx.Query(getQuery("s_vulnerability_fixedin_feature"), featureID)
 	if err != nil {
 		tx.Rollback()
-		return -1, err
+		return 0, err
 	}
 	defer rows.Close()
 
@@ -96,17 +100,18 @@ func (pgSQL *pgSQL) insertFeatureVersion(featureVersion database.FeatureVersion)
 		err := rows.Scan(&fixedInID, &vulnerabilityID, &fixedInVersion)
 		if err != nil {
 			tx.Rollback()
-			return -1, err
+			return 0, err
 		}
 
 		if featureVersion.Version.Compare(fixedInVersion) < 0 {
 			// The version of the FeatureVersion we are inserting is lower than the fixed version on this
 			// Vulnerability, thus, this FeatureVersion is affected by it.
+			// TODO(Quentin-M): Prepare.
 			_, err := tx.Exec(getQuery("i_vulnerability_affects_featureversion"), vulnerabilityID,
 				featureVersion.ID, fixedInID)
 			if err != nil {
 				tx.Rollback()
-				return -1, err
+				return 0, err
 			}
 		}
 	}
@@ -115,7 +120,7 @@ func (pgSQL *pgSQL) insertFeatureVersion(featureVersion database.FeatureVersion)
 	err = tx.Commit()
 	if err != nil {
 		tx.Rollback()
-		return -1, err
+		return 0, err
 	}
 
 	if pgSQL.cache != nil {
@@ -123,5 +128,20 @@ func (pgSQL *pgSQL) insertFeatureVersion(featureVersion database.FeatureVersion)
 			featureVersion.Version.String(), featureVersion.ID)
 	}
 
-	return 0, nil
+	return featureVersion.ID, nil
+}
+
+// TODO(Quentin-M): Batch me
+func (pgSQL *pgSQL) insertFeatureVersions(featureVersions []database.FeatureVersion) ([]int, error) {
+	IDs := make([]int, 0, len(featureVersions))
+
+	for i := 0; i < len(featureVersions); i++ {
+		id, err := pgSQL.insertFeatureVersion(featureVersions[i])
+		if err != nil {
+			return IDs, err
+		}
+		IDs = append(IDs, id)
+	}
+
+	return IDs, nil
 }

database/pgsql/layer.go

@@ -4,6 +4,7 @@ import (
 	"database/sql"
 
 	"github.com/coreos/clair/database"
+	"github.com/coreos/clair/utils"
 	cerrors "github.com/coreos/clair/utils/errors"
 	"github.com/guregu/null/zero"
 )
@@ -11,11 +12,14 @@ import (
 func (pgSQL *pgSQL) FindLayer(name string, withFeatures, withVulnerabilities bool) (database.Layer, error) {
 	// Find the layer
 	var layer database.Layer
-	var parentName sql.NullString
+	var parentID zero.Int
+	var parentName zero.String
+	var namespaceID zero.Int
 	var namespaceName sql.NullString
 
 	err := pgSQL.QueryRow(getQuery("s_layer"), name).
-		Scan(&layer.ID, &layer.Name, &layer.EngineVersion, &parentName, &namespaceName)
+		Scan(&layer.ID, &layer.Name, &layer.EngineVersion, &parentID, &parentName, &namespaceID,
+		&namespaceName)
 
 	if err == sql.ErrNoRows {
 		return layer, cerrors.ErrNotFound
@@ -24,11 +28,17 @@ func (pgSQL *pgSQL) FindLayer(name string, withFeatures, withVulnerabilities boo
 		return layer, err
 	}
 
-	if parentName.Valid {
-		layer.Parent = &database.Layer{Name: parentName.String}
+	if !parentID.IsZero() {
+		layer.Parent = &database.Layer{
+			Model: database.Model{ID: int(parentID.Int64)},
+			Name:  parentName.String,
+		}
 	}
-	if namespaceName.Valid {
-		layer.Namespace = &database.Namespace{Name: namespaceName.String}
+	if !namespaceID.IsZero() {
+		layer.Namespace = &database.Namespace{
+			Model: database.Model{ID: int(namespaceID.Int64)},
+			Name:  namespaceName.String,
+		}
 	}
 
 	// Find its features
@@ -210,6 +220,11 @@ func (pgSQL *pgSQL) InsertLayer(layer database.Layer) error {
 			return err
 		}
 		namespaceID = zero.IntFrom(int64(n))
+	} else if layer.Namespace == nil && layer.Parent != nil {
+		// Import the Namespace from the parent if it has one and this layer doesn't specify one.
+		if layer.Parent.Namespace != nil {
+			namespaceID = zero.IntFrom(int64(layer.Parent.Namespace.ID))
+		}
 	}
 
 	if layer.ID == 0 {
@@ -222,11 +237,6 @@ func (pgSQL *pgSQL) InsertLayer(layer database.Layer) error {
 			}
 
 			parentID = zero.IntFrom(int64(layer.Parent.ID))
-
-			// Import the Namespace from the parent is this layer doesn't specify one.
-			if zero.IsNull(namespaceID) {
-				namespaceID = zero.IntFrom(int64(layer.Parent.Namespace.ID))
-			}
 		}
 
 		err = tx.QueryRow(getQuery("i_layer"), layer.Name, layer.EngineVersion, parentID, namespaceID).
@@ -247,10 +257,20 @@ func (pgSQL *pgSQL) InsertLayer(layer database.Layer) error {
 			tx.Rollback()
 			return err
 		}
+
+		// Remove all existing Layer_diff_FeatureVersion.
+		_, err = tx.Exec(getQuery("r_layer_diff_featureversion"), layer.ID)
+		if err != nil {
+			tx.Rollback()
+			return err
+		}
 	}
 
 	// Update Layer_diff_FeatureVersion now.
-	updateDiffFeatureVersions(tx, &layer, &existingLayer)
+	err = pgSQL.updateDiffFeatureVersions(tx, &layer, &existingLayer)
+	if err != nil {
+		return err
+	}
 
 	// Commit transaction.
 	err = tx.Commit()
@@ -262,19 +282,73 @@ func (pgSQL *pgSQL) InsertLayer(layer database.Layer) error {
 	return nil
 }
 
-func updateDiffFeatureVersions(tx *sql.Tx, layer, existingLayer *database.Layer) {
-	// TODO
-
-	if existingLayer != nil {
-		// We are updating a layer, we need to diff the Features with the existing Layer.
+func (pgSQL *pgSQL) updateDiffFeatureVersions(tx *sql.Tx, layer, existingLayer *database.Layer) error {
+	// add and del are the FeatureVersion diff we should insert.
+	var add []database.FeatureVersion
+	var del []database.FeatureVersion
 
-	} else if layer.Parent == nil {
+	if layer.Parent == nil {
 		// There is no parent, every Features are added.
-
+		add = append(add, layer.Features...)
 	} else if layer.Parent != nil {
 		// There is a parent, we need to diff the Features with it.
 
+		// Build name:version strctures.
+		layerFeaturesMapNV, layerFeaturesNV := createNV(layer.Features)
+		parentLayerFeaturesMapNV, parentLayerFeaturesNV := createNV(layer.Parent.Features)
+
+		// Calculate the added and deleted FeatureVersions name:version.
+		addNV := utils.CompareStringLists(layerFeaturesNV, parentLayerFeaturesNV)
+		delNV := utils.CompareStringLists(parentLayerFeaturesNV, layerFeaturesNV)
+
+		// Fill the structures containing the added and deleted FeatureVersions
+		for _, nv := range addNV {
+			add = append(add, *layerFeaturesMapNV[nv])
+		}
+		for _, nv := range delNV {
+			del = append(del, *parentLayerFeaturesMapNV[nv])
+		}
+	}
+
+	// Insert FeatureVersions in the database.
+	addIDs, err := pgSQL.insertFeatureVersions(add)
+	if err != nil {
+		return err
+	}
+	delIDs, err := pgSQL.insertFeatureVersions(del)
+	if err != nil {
+		return err
+	}
+
+	// Insert diff in the database.
+	if len(addIDs) > 0 {
+		_, err = tx.Exec(getQuery("i_layer_diff_featureversion"), layer.ID, "add", buildInputArray(addIDs))
+		if err != nil {
+			return err
+		}
+	}
+	if len(delIDs) > 0 {
+		_, err = tx.Exec(getQuery("i_layer_diff_featureversion"), layer.ID, "del", buildInputArray(delIDs))
+		if err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+func createNV(features []database.FeatureVersion) (map[string]*database.FeatureVersion, []string) {
+	mapNV := make(map[string]*database.FeatureVersion, 0)
+	sliceNV := make([]string, 0, len(features))
+
+	for i := 0; i < len(features); i++ {
+		featureVersion := &features[i]
+		nv := featureVersion.Feature.Name + ":" + featureVersion.Version.String()
+		mapNV[nv] = featureVersion
+		sliceNV = append(sliceNV, nv)
 	}
+
+	return mapNV, sliceNV
 }
 
 func (pgSQL *pgSQL) DeleteLayer(name string) error {

database/pgsql/layer_test.go

@@ -121,6 +121,8 @@ func testInsertLayerInvalid(t *testing.T, datastore database.Datastore) {
 }
 
 func testInsertLayerTree(t *testing.T, datastore database.Datastore) {
+	fmt.Println("- testInsertLayerTree")
+
 	f1 := database.FeatureVersion{
 		Feature: database.Feature{
 			Namespace: database.Namespace{Name: "TestInsertLayerNamespace2"},
@@ -152,7 +154,7 @@ func testInsertLayerTree(t *testing.T, datastore database.Datastore) {
 	f5 := database.FeatureVersion{
 		Feature: database.Feature{
 			Namespace: database.Namespace{Name: "TestInsertLayerNamespace3"},
-			Name:      "TestInsertLayerFeature2",
+			Name:      "TestInsertLayerFeature3",
 		},
 		Version: types.NewVersionUnsafe("0.57"),
 	}
@@ -180,10 +182,11 @@ func testInsertLayerTree(t *testing.T, datastore database.Datastore) {
 			Namespace: &database.Namespace{Name: "TestInsertLayerNamespace2"},
 			Features:  []database.FeatureVersion{f1, f2, f3},
 		},
-		// This layer covers the case where the last layer doesn't provide any Feature.
+		// This layer covers the case where the last layer doesn't provide any new Feature.
 		database.Layer{
-			Name:   "TestInsertLayer4a",
-			Parent: &database.Layer{Name: "TestInsertLayer3"},
+			Name:     "TestInsertLayer4a",
+			Parent:   &database.Layer{Name: "TestInsertLayer3"},
+			Features: []database.FeatureVersion{f1, f2, f3},
 		},
 		// This layer covers the case where the last layer provides Features.
 		// It also modifies the Namespace ("upgrade") but keeps some Features not upgraded, their
@@ -221,7 +224,9 @@ func testInsertLayerTree(t *testing.T, datastore database.Datastore) {
 	}
 
 	l4a := retrievedLayers["TestInsertLayer4a"]
-	assert.Equal(t, "TestInsertLayerNamespace2", l4a.Namespace.Name)
+	if assert.NotNil(t, l4a.Namespace) {
+		assert.Equal(t, "TestInsertLayerNamespace2", l4a.Namespace.Name)
+	}
 	assert.Len(t, l4a.Features, 3)
 	for _, featureVersion := range l4a.Features {
 		if cmpFV(featureVersion, f1) && cmpFV(featureVersion, f2) && cmpFV(featureVersion, f3) {
@@ -230,9 +235,11 @@ func testInsertLayerTree(t *testing.T, datastore database.Datastore) {
 	}
 
 	l4b := retrievedLayers["TestInsertLayer4b"]
-	assert.Equal(t, "TestInsertLayerNamespace3", l4a.Namespace.Name)
-	assert.Len(t, l4a.Features, 3)
-	for _, featureVersion := range l4a.Features {
+	if assert.NotNil(t, l4b.Namespace) {
+		assert.Equal(t, "TestInsertLayerNamespace3", l4b.Namespace.Name)
+	}
+	assert.Len(t, l4b.Features, 3)
+	for _, featureVersion := range l4b.Features {
 		if cmpFV(featureVersion, f2) && cmpFV(featureVersion, f5) && cmpFV(featureVersion, f6) {
 			assert.Error(t, fmt.Errorf("TestInsertLayer4a contains an unexpected package: %#v. Should contain %#v and %#v and %#v.", featureVersion, f2, f4, f6))
 		}

database/pgsql/pgsql.go

@@ -6,7 +6,6 @@ import (
 	"io/ioutil"
 	"path"
 	"runtime"
-	"strconv"
 	"strings"
 
 	"bitbucket.org/liamstask/goose/lib/goose"
@@ -171,18 +170,6 @@ func OpenForTest(name string, withTestData bool) (*pgSQLTest, error) {
 	return &pgSQLTest{pgSQL: db.(*pgSQL), dataSource: dataSource, dbName: dbName}, nil
 }
 
-// buildInputArray constructs a PostgreSQL input array from the specified integers.
-// Useful to use the `= ANY($1::integer[])` syntax that let us use a IN clause while using
-// a single placeholder.
-func buildInputArray(ints []int) string {
-	str := "{"
-	for i := 0; i < len(ints)-1; i++ {
-		str = str + strconv.Itoa(ints[i]) + ","
-	}
-	str = str + strconv.Itoa(ints[len(ints)-1]) + "}"
-	return str
-}
-
 // isErrUniqueViolation determines is the given error is a unique contraint violation.
 func isErrUniqueViolation(err error) bool {
 	pqErr, ok := err.(*pq.Error)

database/pgsql/queries.go

@@ -1,6 +1,9 @@
 package pgsql
 
-import "fmt"
+import (
+	"fmt"
+	"strconv"
+)
 
 var queries map[string]string
 
@@ -28,7 +31,7 @@ func init() {
 	queries["soi_feature"] = `
     WITH new_feature AS (
       INSERT INTO Feature(name, namespace_id)
-      SELECT CAST($1 AS VARCHAR), CAST($2 AS VARCHAR)
+      SELECT CAST($1 AS VARCHAR), CAST($2 AS INTEGER)
       WHERE NOT EXISTS (SELECT id FROM Feature WHERE name = $1 AND namespace_id = $2)
       RETURNING id
     )
@@ -36,23 +39,25 @@ func init() {
     UNION
     SELECT id FROM new_feature`
 
-	queries["l_share_vulnerability_fixedin_feature"] = `LOCK Vulnerability_FixedIn_Feature IN SHARE MODE`
+	queries["l_share_vulnerability_fixedin_feature"] = `
+    LOCK Vulnerability_FixedIn_Feature IN SHARE MODE
+  `
 
 	queries["soi_featureversion"] = `
     WITH new_featureversion AS (
       INSERT INTO FeatureVersion(feature_id, version)
-      SELECT CAST($1 AS VARCHAR), CAST($2 AS VARCHAR)
-      WHERE NOT EXISTS (SELECT id FROM Feature WHERE feature_id = $1 AND version = $2)
+      SELECT CAST($1 AS INTEGER), CAST($2 AS VARCHAR)
+      WHERE NOT EXISTS (SELECT id FROM FeatureVersion WHERE feature_id = $1 AND version = $2)
       RETURNING id
     )
-    SELECT 'exi', id FROM Feature WHERE feature_id = $1 AND version = $2
+    SELECT 'exi', id FROM FeatureVersion WHERE feature_id = $1 AND version = $2
     UNION
     SELECT 'new', id FROM new_featureversion
   `
 
 	queries["s_vulnerability_fixedin_feature"] = `
     SELECT id, vulnerability_id, version FROM Vulnerability_FixedIn_Feature
-    WHERE feature_id = ?`
+    WHERE feature_id = $1`
 
 	queries["i_vulnerability_affects_featureversion"] = `
     INSERT INTO Vulnerability_Affects_FeatureVersion(vulnerability_id,
@@ -60,7 +65,7 @@ func init() {
 
 	// layer.go
 	queries["s_layer"] = `
-    SELECT l.id, l.name, l.engineversion, p.name, n.name
+    SELECT l.id, l.name, l.engineversion, p.id, p.name, n.id, n.name
     FROM Layer l
       LEFT JOIN Layer p ON l.parent_id = p.id
       LEFT JOIN Namespace n ON l.namespace_id = n.id
@@ -102,7 +107,8 @@ func init() {
     ORDER BY ltree.ordering`
 
 	queries["s_featureversions_vulnerabilities"] = `
-    SELECT vafv.featureversion_id, v.id, v.name, v.description, v.link, v.severity, vn.name, vfif.version
+    SELECT vafv.featureversion_id, v.id, v.name, v.description, v.link, v.severity, vn.name,
+      vfif.version
     FROM Vulnerability_Affects_FeatureVersion vafv, Vulnerability v,
          Namespace vn, Vulnerability_FixedIn_Feature vfif
     WHERE vafv.featureversion_id = ANY($1::integer[])
@@ -110,9 +116,21 @@ func init() {
           AND vafv.fixedin_id = vfif.id
           AND v.namespace_id = vn.id`
 
-	queries["i_layer"] = `INSERT INTO Layer(name, engine_version, parent_id, namespace_id) VALUES($1, $2, $3, $4) RETURNING id`
+	queries["i_layer"] = `
+    INSERT INTO Layer(name, engineversion, parent_id, namespace_id)
+    VALUES($1, $2, $3, $4) RETURNING id`
+
+	queries["u_layer"] = `UPDATE LAYER SET engineversion = $2, namespace_id = $3) WHERE id = $1`
+
+	queries["r_layer_diff_featureversion"] = `
+    DELETE FROM Layer_diff_FeatureVersion
+    WHERE layer_id = $1`
 
-	queries["u_layer"] = `UPDATE LAYER SET engine_version = $2, namespace_id = $3) WHERE id = $1`
+	queries["i_layer_diff_featureversion"] = `
+    INSERT INTO Layer_diff_FeatureVersion(layer_id, featureversion_id, modification)
+      SELECT $1, fv.id, $2
+	    FROM FeatureVersion fv
+	    WHERE fv.id = ANY($3::integer[])`
 }
 
 func getQuery(name string) string {
@@ -121,3 +139,15 @@ func getQuery(name string) string {
 	}
 	panic(fmt.Sprintf("pgsql: unknown query %v", name))
 }
+
+// buildInputArray constructs a PostgreSQL input array from the specified integers.
+// Useful to use the `= ANY($1::integer[])` syntax that let us use a IN clause while using
+// a single placeholder.
+func buildInputArray(ints []int) string {
+	str := "{"
+	for i := 0; i < len(ints)-1; i++ {
+		str = str + strconv.Itoa(ints[i]) + ","
+	}
+	str = str + strconv.Itoa(ints[len(ints)-1]) + "}"
+	return str
+}

worker/worker.go

@@ -156,6 +156,14 @@ func detectContent(name, path, imageFormat string, parent *database.Layer) (name
 		return
 	}
 
+	// If there are no feature detected, use parent's features if possible.
+	// TODO(Quentin-M): We eventually want to give the choice to each detectors to use none/some
+	// parent's Features. It would be useful for dectectors that can't find their entire result using
+	// one Layer.
+	if len(features) == 0 && parent != nil {
+		features = parent.Features
+	}
+
 	log.Debugf("layer %s: detected %d features", name, len(features))
 	return
 }