From 5cc5220e5b61d7900f8c31b2ae6e199d33fd4ffc Mon Sep 17 00:00:00 2001
From: Mike Arpaia <mike@arpaia.co>
Date: Mon, 16 Jan 2017 15:20:15 -0700
Subject: [PATCH] Enforce uniqueness on query name (#915)

* Enforce uniqueness on query name

close #914

* catching the already exists error in MySQL
---
 glide.lock                                    |  8 ++++---
 glide.yaml                                    |  1 +
 server/datastore/datastore_queries_test.go    | 20 ++++++++++++++++
 server/datastore/datastore_test.go            |  1 +
 .../20170111133013_AddUniqueIndexToQueries.go | 24 +++++++++++++++++++
 server/datastore/mysql/queries.go             |  8 +++++++
 6 files changed, 59 insertions(+), 3 deletions(-)
 create mode 100644 server/datastore/mysql/migrations/tables/20170111133013_AddUniqueIndexToQueries.go

diff --git a/glide.lock b/glide.lock
index f0454a8da0..de9624f274 100644
--- a/glide.lock
+++ b/glide.lock
@@ -1,5 +1,5 @@
-hash: e9cd204c0dd7e3e9102a2e0c87cc722101473f0d8fd31b05ec7f88837dd1d529
-updated: 2017-01-04T15:27:20.412164218-08:00
+hash: ab1d436e71559ecedf062ac07e6086d96fbcfab9d20854b1b3f78867a6b3a94c
+updated: 2017-01-16T14:44:49.6595886-07:00
 imports:
 - name: github.com/alecthomas/template
   version: a0175ee3bccc567396460bf5acd36800cb10c49c
@@ -137,6 +137,8 @@ imports:
   subpackages:
   - assert
   - require
+- name: github.com/VividCortex/mysqlerr
+  version: 93e3dc264d50c8ea98844075ef84224cacdc0c91
 - name: github.com/WatchBeam/clock
   version: d5a6612f3d9a070b964adc65dbd94c6080458f83
 - name: golang.org/x/crypto
@@ -167,7 +169,7 @@ imports:
 - name: gopkg.in/go-playground/validator.v8
   version: 5f57d2222ad794d0dffb07e664ea05e2ee07d60c
 - name: gopkg.in/natefinch/lumberjack.v2
-  version: dd45e6a67c53f673bb49ca8a001fd3a63ceb640e
+  version: 514cbda263a734ae8caac038dadf05f8f3f9f738
 - name: gopkg.in/yaml.v2
   version: e4d366fc3c7938e2958e662b4258c7a89e1f0e3e
 testImports: []
diff --git a/glide.yaml b/glide.yaml
index 416a5c6e77..1ab6755c89 100644
--- a/glide.yaml
+++ b/glide.yaml
@@ -60,3 +60,4 @@ import:
   - redis
 - package: github.com/jmoiron/sqlx
 - package: github.com/kolide/goose
+- package: github.com/VividCortex/mysqlerr
diff --git a/server/datastore/datastore_queries_test.go b/server/datastore/datastore_queries_test.go
index 8b8d1fc1c9..93d8a69503 100644
--- a/server/datastore/datastore_queries_test.go
+++ b/server/datastore/datastore_queries_test.go
@@ -183,3 +183,23 @@ func testLoadPacksForQueries(t *testing.T, ds kolide.Datastore) {
 	checkPacks(t, []kolide.Pack{*p2, *p3}, q1.Packs)
 	checkPacks(t, []kolide.Pack{*p1, *p3}, q2.Packs)
 }
+
+func testDuplicateNewQuery(t *testing.T, ds kolide.Datastore) {
+	user := test.NewUser(t, ds, "Mike Arpaia", "marpaia", "mike@kolide.co", true)
+	q1, err := ds.NewQuery(&kolide.Query{
+		Name:     "foo",
+		Query:    "select * from time;",
+		AuthorID: user.ID,
+	})
+	require.Nil(t, err)
+	assert.NotZero(t, q1.ID)
+
+	_, err = ds.NewQuery(&kolide.Query{
+		Name:  "foo",
+		Query: "select * from osquery_info;",
+	})
+
+	// Note that we can't do the actual type assertion here because existsError
+	// is private to the individual datastore implementations
+	assert.Contains(t, err.Error(), "already exists in the datastore")
+}
diff --git a/server/datastore/datastore_test.go b/server/datastore/datastore_test.go
index d15bc692f0..6d3230c883 100644
--- a/server/datastore/datastore_test.go
+++ b/server/datastore/datastore_test.go
@@ -67,4 +67,5 @@ var testFunctions = [...]func(*testing.T, kolide.Datastore){
 	testAddLabelToPackTwice,
 	testGenerateHostStatusStatistics,
 	testMarkHostSeen,
+	testDuplicateNewQuery,
 }
diff --git a/server/datastore/mysql/migrations/tables/20170111133013_AddUniqueIndexToQueries.go b/server/datastore/mysql/migrations/tables/20170111133013_AddUniqueIndexToQueries.go
new file mode 100644
index 0000000000..5665ed26b2
--- /dev/null
+++ b/server/datastore/mysql/migrations/tables/20170111133013_AddUniqueIndexToQueries.go
@@ -0,0 +1,24 @@
+package tables
+
+import "database/sql"
+
+func init() {
+	MigrationClient.AddMigration(Up_20170111133013, Down_20170111133013)
+}
+
+func Up_20170111133013(tx *sql.Tx) error {
+	_, err := tx.Exec(
+		"ALTER TABLE `queries` " +
+			"ADD CONSTRAINT `constraint_query_name_unique` " +
+			"UNIQUE (`name`);",
+	)
+	return err
+}
+
+func Down_20170111133013(tx *sql.Tx) error {
+	_, err := tx.Exec(
+		"ALTER TABLE `queries` " +
+			"DROP INDEX `constraint_query_name_unique`;",
+	)
+	return err
+}
diff --git a/server/datastore/mysql/queries.go b/server/datastore/mysql/queries.go
index c84082ed59..9ad9702ad1 100644
--- a/server/datastore/mysql/queries.go
+++ b/server/datastore/mysql/queries.go
@@ -3,6 +3,8 @@ package mysql
 import (
 	"database/sql"
 
+	"github.com/VividCortex/mysqlerr"
+	"github.com/go-sql-driver/mysql"
 	"github.com/jmoiron/sqlx"
 	"github.com/kolide/kolide-ose/server/kolide"
 	"github.com/pkg/errors"
@@ -37,6 +39,12 @@ func (d *Datastore) NewQuery(query *kolide.Query) (*kolide.Query, error) {
 		) VALUES ( ?, ?, ?, ?, ? )
 	`
 	result, err := d.db.Exec(sql, query.Name, query.Description, query.Query, query.Saved, query.AuthorID)
+	if driverErr, ok := err.(*mysql.MySQLError); ok {
+		if driverErr.Number == mysqlerr.ER_DUP_ENTRY {
+			// TODO: this shouldn't require an ID parameter
+			return nil, alreadyExists("Query", 0)
+		}
+	}
 	if err != nil {
 		return nil, errors.Wrap(err, "inserting new query")
 	}