Return 404 errors where appropriate in spec endpoints (#1777)

This commit is contained in:
Zachary Wasserman 2018-05-09 16:54:42 -07:00 committed by GitHub
parent 4dfc1ca25e
commit 87331b47e2
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 38 additions and 49 deletions

View file

@ -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)

View file

@ -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()

View file

@ -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))
}

View file

@ -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))
}

View file

@ -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,

View file

@ -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 {

View file

@ -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 {

View file

@ -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...)
}

View file

@ -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
}