diff --git a/server/datastore/datastore_queries_test.go b/server/datastore/datastore_queries_test.go index caf7a4002f..64dcd289b6 100644 --- a/server/datastore/datastore_queries_test.go +++ b/server/datastore/datastore_queries_test.go @@ -96,15 +96,14 @@ func testDeleteQuery(t *testing.T, ds kolide.Datastore) { func testGetQueryByName(t *testing.T, ds kolide.Datastore) { user := test.NewUser(t, ds, "Zach", "zwass", "zwass@kolide.co", true) test.NewQuery(t, ds, "q1", "select * from time", user.ID, true) - actual, ok, err := ds.QueryByName("q1") + actual, err := ds.QueryByName("q1") require.Nil(t, err) - assert.True(t, ok) assert.Equal(t, "q1", actual.Name) assert.Equal(t, "select * from time", actual.Query) - actual, ok, err = ds.QueryByName("xxx") + actual, err = ds.QueryByName("xxx") assert.Nil(t, err) - assert.False(t, ok) + assert.True(t, kolide.IsNotFound(err)) } func testDeleteQueries(t *testing.T, ds kolide.Datastore) { @@ -215,14 +214,12 @@ func testLoadPacksForQueries(t *testing.T, ds kolide.Datastore) { err = ds.ApplyPackSpecs(specs) require.Nil(t, err) - q0, exists, err := ds.QueryByName(queries[0].Name) + q0, err := ds.QueryByName(queries[0].Name) require.Nil(t, err) - require.True(t, exists) assert.Empty(t, q0.Packs) - q1, exists, err := ds.QueryByName(queries[1].Name) + q1, err := ds.QueryByName(queries[1].Name) require.Nil(t, err) - require.True(t, exists) assert.Empty(t, q1.Packs) specs = []*kolide.PackSpec{ @@ -239,16 +236,14 @@ func testLoadPacksForQueries(t *testing.T, ds kolide.Datastore) { err = ds.ApplyPackSpecs(specs) require.Nil(t, err) - q0, exists, err = ds.QueryByName(queries[0].Name) + q0, err = ds.QueryByName(queries[0].Name) require.Nil(t, err) - require.True(t, exists) if assert.Len(t, q0.Packs, 1) { assert.Equal(t, "p2", q0.Packs[0].Name) } - q1, exists, err = ds.QueryByName(queries[1].Name) + q1, err = ds.QueryByName(queries[1].Name) require.Nil(t, err) - require.True(t, exists) assert.Empty(t, q1.Packs) specs = []*kolide.PackSpec{ @@ -274,16 +269,14 @@ func testLoadPacksForQueries(t *testing.T, ds kolide.Datastore) { err = ds.ApplyPackSpecs(specs) require.Nil(t, err) - q0, exists, err = ds.QueryByName(queries[0].Name) + q0, err = ds.QueryByName(queries[0].Name) require.Nil(t, err) - require.True(t, exists) if assert.Len(t, q0.Packs, 1) { assert.Equal(t, "p2", q0.Packs[0].Name) } - q1, exists, err = ds.QueryByName(queries[1].Name) + q1, err = ds.QueryByName(queries[1].Name) require.Nil(t, err) - require.True(t, exists) if assert.Len(t, q1.Packs, 2) { sort.Slice(q1.Packs, func(i, j int) bool { return q1.Packs[i].Name < q1.Packs[j].Name }) assert.Equal(t, "p1", q1.Packs[0].Name) @@ -308,18 +301,16 @@ func testLoadPacksForQueries(t *testing.T, ds kolide.Datastore) { err = ds.ApplyPackSpecs(specs) require.Nil(t, err) - q0, exists, err = ds.QueryByName(queries[0].Name) + q0, err = ds.QueryByName(queries[0].Name) require.Nil(t, err) - require.True(t, exists) if assert.Len(t, q0.Packs, 2) { sort.Slice(q0.Packs, func(i, j int) bool { return q0.Packs[i].Name < q0.Packs[j].Name }) assert.Equal(t, "p2", q0.Packs[0].Name) assert.Equal(t, "p3", q0.Packs[1].Name) } - q1, exists, err = ds.QueryByName(queries[1].Name) + q1, err = ds.QueryByName(queries[1].Name) require.Nil(t, err) - require.True(t, exists) if assert.Len(t, q1.Packs, 2) { sort.Slice(q1.Packs, func(i, j int) bool { return q1.Packs[i].Name < q1.Packs[j].Name }) assert.Equal(t, "p1", q1.Packs[0].Name) diff --git a/server/datastore/inmem/queries.go b/server/datastore/inmem/queries.go index 91eaeff2b6..336d7d3527 100644 --- a/server/datastore/inmem/queries.go +++ b/server/datastore/inmem/queries.go @@ -26,17 +26,6 @@ func (d *Datastore) NewQuery(query *kolide.Query, opts ...kolide.OptionalArg) (* return &newQuery, nil } -func (d *Datastore) QueryByName(name string, opts ...kolide.OptionalArg) (*kolide.Query, bool, error) { - d.mtx.Lock() - defer d.mtx.Unlock() - for _, q := range d.queries { - if name == q.Name { - return q, true, nil - } - } - return nil, false, nil -} - func (d *Datastore) SaveQuery(query *kolide.Query) error { d.mtx.Lock() defer d.mtx.Unlock() diff --git a/server/datastore/mysql/labels.go b/server/datastore/mysql/labels.go index 43b9489a3d..1cd493a49b 100644 --- a/server/datastore/mysql/labels.go +++ b/server/datastore/mysql/labels.go @@ -82,8 +82,10 @@ WHERE name = ? if err := d.db.Select(&specs, query, name); err != nil { return nil, errors.Wrap(err, "get label") } - - if len(specs) != 1 { + if len(specs) == 0 { + return nil, notFound("Label").WithName(name) + } + if len(specs) > 1 { return nil, errors.Errorf("expected 1 label row, got %d", len(specs)) } diff --git a/server/datastore/mysql/packs.go b/server/datastore/mysql/packs.go index 09e03ddd0e..e017ef7476 100644 --- a/server/datastore/mysql/packs.go +++ b/server/datastore/mysql/packs.go @@ -197,7 +197,10 @@ func (d *Datastore) GetPackSpec(name string) (spec *kolide.PackSpec, err error) if err := tx.Select(&specs, query, name); err != nil { return nil, errors.Wrap(err, "get packs") } - if len(specs) != 1 { + if len(specs) == 0 { + return nil, notFound("Pack").WithName(name) + } + if len(specs) > 1 { return nil, errors.Errorf("expected 1 pack row, got %d", len(specs)) } diff --git a/server/datastore/mysql/queries.go b/server/datastore/mysql/queries.go index 2f460b9add..2c3614fa99 100644 --- a/server/datastore/mysql/queries.go +++ b/server/datastore/mysql/queries.go @@ -61,7 +61,7 @@ func (d *Datastore) ApplyQueries(authorID uint, queries []*kolide.Query) (err er return errors.Wrap(err, "commit ApplyQueries transaction") } -func (d *Datastore) QueryByName(name string, opts ...kolide.OptionalArg) (*kolide.Query, bool, error) { +func (d *Datastore) QueryByName(name string, opts ...kolide.OptionalArg) (*kolide.Query, error) { db := d.getTransaction(opts) sqlStatement := ` SELECT * @@ -72,16 +72,16 @@ func (d *Datastore) QueryByName(name string, opts ...kolide.OptionalArg) (*kolid err := db.Get(&query, sqlStatement, name) if err != nil { if err == sql.ErrNoRows { - return nil, false, nil + return nil, notFound("Query").WithName(name) } - return nil, false, errors.Wrap(err, "selecting query by name") + return nil, errors.Wrap(err, "selecting query by name") } if err := d.loadPacksForQueries([]*kolide.Query{&query}); err != nil { - return nil, false, errors.Wrap(err, "loading packs for query") + return nil, errors.Wrap(err, "loading packs for query") } - return &query, true, nil + return &query, nil } // NewQuery creates a New Query. If a query with the same name was soft-deleted, diff --git a/server/kolide/datastore.go b/server/kolide/datastore.go index 0d83a3e5e6..b83b089fc9 100644 --- a/server/kolide/datastore.go +++ b/server/kolide/datastore.go @@ -44,6 +44,14 @@ type NotFoundError interface { IsNotFound() bool } +func IsNotFound(err error) bool { + e, ok := err.(NotFoundError) + if !ok { + return false + } + return e.IsNotFound() +} + // AlreadyExists is returned when creating a datastore resource that already // exists. type AlreadyExistsError interface { diff --git a/server/kolide/queries.go b/server/kolide/queries.go index 6ce594d44b..79341dfba0 100644 --- a/server/kolide/queries.go +++ b/server/kolide/queries.go @@ -32,9 +32,8 @@ type QueryStore interface { // ListQueries returns a list of queries with the provided sorting and // paging options. Associated packs should also be loaded. ListQueries(opt ListOptions) ([]*Query, error) - // QueryByName looks up a query by name, the second bool is true if a query - // by the name exists. - QueryByName(name string, opts ...OptionalArg) (*Query, bool, error) + // QueryByName looks up a query by name. + QueryByName(name string, opts ...OptionalArg) (*Query, error) } type QueryService interface { diff --git a/server/mock/datastore_queries.go b/server/mock/datastore_queries.go index 0b3a714ba8..53bced7e27 100644 --- a/server/mock/datastore_queries.go +++ b/server/mock/datastore_queries.go @@ -20,7 +20,7 @@ type QueryFunc func(id uint) (*kolide.Query, error) type ListQueriesFunc func(opt kolide.ListOptions) ([]*kolide.Query, error) -type QueryByNameFunc func(name string, opts ...kolide.OptionalArg) (*kolide.Query, bool, error) +type QueryByNameFunc func(name string, opts ...kolide.OptionalArg) (*kolide.Query, error) type QueryStore struct { ApplyQueriesFunc ApplyQueriesFunc @@ -83,7 +83,7 @@ func (s *QueryStore) ListQueries(opt kolide.ListOptions) ([]*kolide.Query, error return s.ListQueriesFunc(opt) } -func (s *QueryStore) QueryByName(name string, opts ...kolide.OptionalArg) (*kolide.Query, bool, error) { +func (s *QueryStore) QueryByName(name string, opts ...kolide.OptionalArg) (*kolide.Query, error) { s.QueryByNameFuncInvoked = true return s.QueryByNameFunc(name, opts...) } diff --git a/server/service/service_queries.go b/server/service/service_queries.go index 2f330ed853..8f24ec0ba6 100644 --- a/server/service/service_queries.go +++ b/server/service/service_queries.go @@ -53,12 +53,9 @@ func (svc service) GetQuerySpecs(ctx context.Context) ([]*kolide.QuerySpec, erro } func (svc service) GetQuerySpec(ctx context.Context, name string) (*kolide.QuerySpec, error) { - query, ok, err := svc.ds.QueryByName(name) - if !ok { - return nil, errors.Errorf("no query with name %s", name) - } + query, err := svc.ds.QueryByName(name) if err != nil { - return nil, errors.Wrap(err, "get query by name") + return nil, err } return specFromQuery(query), nil }