From bc6109fb563c2c5cdc12562e8f7f45f0d4c52a42 Mon Sep 17 00:00:00 2001 From: Zachary Wasserman Date: Tue, 6 Dec 2016 10:16:04 -0800 Subject: [PATCH] Fixes for saved queries (#576) - Only saved queries should be returned by ListQueries - Bugfixes Addresses #388 --- server/datastore/datastore_queries_test.go | 12 +++++++++++- server/datastore/inmem/queries.go | 4 +++- server/datastore/mysql/queries.go | 21 ++++++++++++--------- server/kolide/queries.go | 5 ++++- server/service/service_queries_test.go | 8 +++++--- 5 files changed, 35 insertions(+), 15 deletions(-) diff --git a/server/datastore/datastore_queries_test.go b/server/datastore/datastore_queries_test.go index bc8d2c63e6..b33d4ff8ea 100644 --- a/server/datastore/datastore_queries_test.go +++ b/server/datastore/datastore_queries_test.go @@ -6,6 +6,7 @@ import ( "github.com/kolide/kolide-ose/server/kolide" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func testDeleteQuery(t *testing.T, ds kolide.Datastore) { @@ -50,10 +51,19 @@ func testListQuery(t *testing.T, ds kolide.Datastore) { _, err := ds.NewQuery(&kolide.Query{ Name: fmt.Sprintf("name%02d", i), Query: fmt.Sprintf("query%02d", i), + Saved: true, }) - assert.Nil(t, err) + require.Nil(t, err) } + // One unsaved query should not be returned + _, err := ds.NewQuery(&kolide.Query{ + Name: "unsaved", + Query: "select * from time", + Saved: false, + }) + require.Nil(t, err) + opts := kolide.ListOptions{} results, err := ds.ListQueries(opts) assert.Nil(t, err) diff --git a/server/datastore/inmem/queries.go b/server/datastore/inmem/queries.go index 727eb86437..3fb6f58225 100644 --- a/server/datastore/inmem/queries.go +++ b/server/datastore/inmem/queries.go @@ -74,7 +74,9 @@ func (orm *Datastore) ListQueries(opt kolide.ListOptions) ([]*kolide.Query, erro queries := []*kolide.Query{} for _, k := range keys { - queries = append(queries, orm.queries[uint(k)]) + if orm.queries[uint(k)].Saved { + queries = append(queries, orm.queries[uint(k)]) + } } // Apply ordering diff --git a/server/datastore/mysql/queries.go b/server/datastore/mysql/queries.go index 1f731cf9d9..554b26bc4a 100644 --- a/server/datastore/mysql/queries.go +++ b/server/datastore/mysql/queries.go @@ -9,13 +9,14 @@ import ( func (d *Datastore) NewQuery(query *kolide.Query) (*kolide.Query, error) { sql := ` - INSERT INTO queries ( name, description, query, - snapshot, differential, platform, version, ` + "`interval`" + `) - VALUES ( ?, ?, ?, ?, ?, ?, ?, ? ) + INSERT INTO queries (name, description, query, + saved, snapshot, differential, platform, version, ` + "`interval`" + `) + VALUES ( ?, ?, ?, ?, ?, ?, ?, ?, ? ) ` - result, err := d.db.Exec(sql, query.Name, query.Description, query.Query, query.Snapshot, - query.Differential, query.Platform, query.Version, query.Interval) + result, err := d.db.Exec(sql, query.Name, query.Description, query.Query, + query.Saved, query.Snapshot, query.Differential, query.Platform, + query.Version, query.Interval) if err != nil { return nil, errors.DatabaseError(err) } @@ -29,12 +30,12 @@ func (d *Datastore) NewQuery(query *kolide.Query) (*kolide.Query, error) { func (d *Datastore) SaveQuery(q *kolide.Query) error { sql := ` UPDATE queries - SET name = ?, description = ?, query = ?, ` + "`interval`" + ` = ?, snapshot = ?, - differential = ?, platform = ?, version = ? + SET name = ?, description = ?, query = ?, ` + "`interval`" + ` = ?, + saved = ?, snapshot = ?, differential = ?, platform = ?, version = ? WHERE id = ? AND NOT deleted ` _, err := d.db.Exec(sql, q.Name, q.Description, q.Query, q.Interval, - q.Snapshot, q.Differential, q.Platform, q.Version, q.ID) + q.Saved, q.Snapshot, q.Differential, q.Platform, q.Version, q.ID) if err != nil { return errors.DatabaseError(err) } @@ -76,7 +77,9 @@ func (d *Datastore) Query(id uint) (*kolide.Query, error) { // determined by passed in kolide.ListOptions func (d *Datastore) ListQueries(opt kolide.ListOptions) ([]*kolide.Query, error) { sql := ` - SELECT * FROM queries WHERE NOT deleted + SELECT * FROM queries + WHERE saved = true + AND NOT deleted ` sql = appendListOptionsToSQL(sql, opt) results := []*kolide.Query{} diff --git a/server/kolide/queries.go b/server/kolide/queries.go index c852dcc8e7..f74386cdb0 100644 --- a/server/kolide/queries.go +++ b/server/kolide/queries.go @@ -16,6 +16,9 @@ type QueryStore interface { } type QueryService interface { + // ListQueries returns a list of saved queries. Note only saved queries + // should be returned (those that are created for distributed queries + // but not saved should not be returned). ListQueries(ctx context.Context, opt ListOptions) ([]*Query, error) GetQuery(ctx context.Context, id uint) (*Query, error) NewQuery(ctx context.Context, p QueryPayload) (*Query, error) @@ -38,11 +41,11 @@ type Query struct { UpdateCreateTimestamps DeleteFields ID uint `json:"id"` - Saved bool `json:"saved"` Name string `json:"name"` Description string `json:"description"` Query string `json:"query"` Interval uint `json:"interval"` + Saved bool `json:"saved"` Snapshot bool `json:"snapshot"` Differential bool `json:"differential"` Platform string `json:"platform"` diff --git a/server/service/service_queries_test.go b/server/service/service_queries_test.go index d8d6f5dd85..cda5168136 100644 --- a/server/service/service_queries_test.go +++ b/server/service/service_queries_test.go @@ -23,9 +23,11 @@ func TestListQueries(t *testing.T) { assert.Nil(t, err) assert.Len(t, queries, 0) - _, err = ds.NewQuery(&kolide.Query{ - Name: "foo", - Query: "select * from time;", + name := "foo" + query := "select * from time" + _, err = svc.NewQuery(ctx, kolide.QueryPayload{ + Name: &name, + Query: &query, }) assert.Nil(t, err)