From bb921bc9730ce15c467cbfdcd205be4be55d5802 Mon Sep 17 00:00:00 2001 From: Zach Wasserman Date: Fri, 4 Dec 2020 09:50:39 -0800 Subject: [PATCH] Allow scheduling same query more than once in UI (#96) Fixes #88 --- .../ScheduledQueriesListItem.jsx | 2 +- .../ScheduledQueriesListItem.tests.jsx | 4 +- frontend/test/stubs.js | 1 + server/service/service_scheduled_queries.go | 25 ++- .../service/service_scheduled_queries_test.go | 144 ++++++++++++++++++ 5 files changed, 172 insertions(+), 4 deletions(-) create mode 100644 server/service/service_scheduled_queries_test.go diff --git a/frontend/components/queries/ScheduledQueriesList/ScheduledQueriesListItem/ScheduledQueriesListItem.jsx b/frontend/components/queries/ScheduledQueriesList/ScheduledQueriesListItem/ScheduledQueriesListItem.jsx index bfbaf52b8434..a4db8fed6a32 100644 --- a/frontend/components/queries/ScheduledQueriesList/ScheduledQueriesListItem/ScheduledQueriesListItem.jsx +++ b/frontend/components/queries/ScheduledQueriesList/ScheduledQueriesListItem/ScheduledQueriesListItem.jsx @@ -76,7 +76,7 @@ class ScheduledQueriesListItem extends Component { render () { const { checked, disabled, isSelected, scheduledQuery } = this.props; - const { id, name, interval, shard, version } = scheduledQuery; + const { id, query_name: name, interval, shard, version } = scheduledQuery; const { loggingTypeString, onDblClick, onCheck, onSelect, renderPlatformIcon } = this; const rowClassname = classnames(baseClass, { [`${baseClass}--selected`]: isSelected, diff --git a/frontend/components/queries/ScheduledQueriesList/ScheduledQueriesListItem/ScheduledQueriesListItem.tests.jsx b/frontend/components/queries/ScheduledQueriesList/ScheduledQueriesListItem/ScheduledQueriesListItem.tests.jsx index e906e3b1cbe2..3a1c4cc99e18 100644 --- a/frontend/components/queries/ScheduledQueriesList/ScheduledQueriesListItem/ScheduledQueriesListItem.tests.jsx +++ b/frontend/components/queries/ScheduledQueriesList/ScheduledQueriesListItem/ScheduledQueriesListItem.tests.jsx @@ -15,7 +15,7 @@ const defaultProps = { describe('ScheduledQueriesListItem - component', () => { it('renders the scheduled query data', () => { const component = mount(); - expect(component.text()).toContain(scheduledQueryStub.name); + expect(component.text()).toContain(scheduledQueryStub.query_name); expect(component.text()).toContain(scheduledQueryStub.interval); expect(component.text()).toContain(scheduledQueryStub.shard); expect(component.find('PlatformIcon').length).toEqual(1); @@ -24,7 +24,7 @@ describe('ScheduledQueriesListItem - component', () => { it('renders when the platform attribute is null', () => { const scheduledQuery = { ...scheduledQueryStub, platform: null }; const component = mount(); - expect(component.text()).toContain(scheduledQueryStub.name); + expect(component.text()).toContain(scheduledQueryStub.query_name); expect(component.text()).toContain(scheduledQueryStub.interval); expect(component.text()).toContain(scheduledQueryStub.shard); expect(component.find('PlatformIcon').length).toEqual(1); diff --git a/frontend/test/stubs.js b/frontend/test/stubs.js index 30401c0efd41..cd3cd300be12 100644 --- a/frontend/test/stubs.js +++ b/frontend/test/stubs.js @@ -140,6 +140,7 @@ export const scheduledQueryStub = { id: 1, interval: 60, name: 'Get all users', + query_name: 'users', pack_id: 123, platform: 'darwin', query: 'SELECT * FROM users', diff --git a/server/service/service_scheduled_queries.go b/server/service/service_scheduled_queries.go index b03f087d9cb3..432a5481fe2f 100644 --- a/server/service/service_scheduled_queries.go +++ b/server/service/service_scheduled_queries.go @@ -23,12 +23,35 @@ func (svc service) ScheduleQuery(ctx context.Context, sq *kolide.ScheduledQuery) if err != nil { return nil, errors.Wrap(err, "lookup name for query") } - sq.Name = query.Name + + packQueries, err := svc.ds.ListScheduledQueriesInPack(sq.PackID, kolide.ListOptions{}) + if err != nil { + return nil, errors.Wrap(err, "find existing scheduled queries") + } + _ = packQueries + + sq.Name = findNextNameForQuery(query.Name, packQueries) + sq.QueryName = query.Name + } else if sq.QueryName == "" { + query, err := svc.ds.Query(sq.QueryID) + if err != nil { + return nil, errors.Wrap(err, "lookup name for query") + } sq.QueryName = query.Name } return svc.ds.NewScheduledQuery(sq) } +// Add "-1" suffixes to the query name until it is unique +func findNextNameForQuery(name string, scheduled []*kolide.ScheduledQuery) string { + for _, q := range scheduled { + if name == q.Name { + return findNextNameForQuery(name+"-1", scheduled) + } + } + return name +} + func (svc service) ModifyScheduledQuery(ctx context.Context, id uint, p kolide.ScheduledQueryPayload) (*kolide.ScheduledQuery, error) { sq, err := svc.GetScheduledQuery(ctx, id) if err != nil { diff --git a/server/service/service_scheduled_queries_test.go b/server/service/service_scheduled_queries_test.go new file mode 100644 index 000000000000..10d15e23903e --- /dev/null +++ b/server/service/service_scheduled_queries_test.go @@ -0,0 +1,144 @@ +package service + +import ( + "context" + "testing" + + "github.com/fleetdm/fleet/server/kolide" + "github.com/fleetdm/fleet/server/mock" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestScheduleQuery(t *testing.T) { + ds := new(mock.Store) + svc, err := newTestService(ds, nil, nil) + require.Nil(t, err) + + expectedQuery := &kolide.ScheduledQuery{ + Name: "foobar", + QueryName: "foobar", + QueryID: 3, + } + + ds.NewScheduledQueryFunc = func(q *kolide.ScheduledQuery, opts ...kolide.OptionalArg) (*kolide.ScheduledQuery, error) { + assert.Equal(t, expectedQuery, q) + return expectedQuery, nil + } + + _, err = svc.ScheduleQuery(context.Background(), expectedQuery) + assert.NoError(t, err) + assert.True(t, ds.NewScheduledQueryFuncInvoked) +} + +func TestScheduleQueryNoName(t *testing.T) { + ds := new(mock.Store) + svc, err := newTestService(ds, nil, nil) + require.Nil(t, err) + + expectedQuery := &kolide.ScheduledQuery{ + Name: "foobar", + QueryName: "foobar", + QueryID: 3, + } + + ds.QueryFunc = func(qid uint) (*kolide.Query, error) { + require.Equal(t, expectedQuery.QueryID, qid) + return &kolide.Query{Name: expectedQuery.QueryName}, nil + } + ds.ListScheduledQueriesInPackFunc = func(id uint, opts kolide.ListOptions) ([]*kolide.ScheduledQuery, error) { + // No matching query + return []*kolide.ScheduledQuery{ + &kolide.ScheduledQuery{ + Name: "froobling", + }, + }, nil + } + ds.NewScheduledQueryFunc = func(q *kolide.ScheduledQuery, opts ...kolide.OptionalArg) (*kolide.ScheduledQuery, error) { + assert.Equal(t, expectedQuery, q) + return expectedQuery, nil + } + + _, err = svc.ScheduleQuery( + context.Background(), + &kolide.ScheduledQuery{QueryID: expectedQuery.QueryID}, + ) + assert.NoError(t, err) + assert.True(t, ds.NewScheduledQueryFuncInvoked) +} + +func TestScheduleQueryNoNameMultiple(t *testing.T) { + ds := new(mock.Store) + svc, err := newTestService(ds, nil, nil) + require.Nil(t, err) + + expectedQuery := &kolide.ScheduledQuery{ + Name: "foobar-1", + QueryName: "foobar", + QueryID: 3, + } + + ds.QueryFunc = func(qid uint) (*kolide.Query, error) { + require.Equal(t, expectedQuery.QueryID, qid) + return &kolide.Query{Name: expectedQuery.QueryName}, nil + } + ds.ListScheduledQueriesInPackFunc = func(id uint, opts kolide.ListOptions) ([]*kolide.ScheduledQuery, error) { + // No matching query + return []*kolide.ScheduledQuery{ + &kolide.ScheduledQuery{ + Name: "foobar", + }, + }, nil + } + ds.NewScheduledQueryFunc = func(q *kolide.ScheduledQuery, opts ...kolide.OptionalArg) (*kolide.ScheduledQuery, error) { + assert.Equal(t, expectedQuery, q) + return expectedQuery, nil + } + + _, err = svc.ScheduleQuery( + context.Background(), + &kolide.ScheduledQuery{QueryID: expectedQuery.QueryID}, + ) + assert.NoError(t, err) + assert.True(t, ds.NewScheduledQueryFuncInvoked) +} + +func TestFindNextNameForQuery(t *testing.T) { + var testCases = []struct { + name string + scheduled []*kolide.ScheduledQuery + expected string + }{ + { + name: "foobar", + scheduled: []*kolide.ScheduledQuery{}, + expected: "foobar", + }, + { + name: "foobar", + scheduled: []*kolide.ScheduledQuery{ + { + Name: "foobar", + }, + }, + expected: "foobar-1", + }, { + name: "foobar", + scheduled: []*kolide.ScheduledQuery{ + { + Name: "foobar", + }, + { + Name: "foobar-1", + }, + }, + expected: "foobar-1-1", + }, + } + + for _, tt := range testCases { + t.Run("", func(t *testing.T) { + assert.Equal(t, tt.expected, findNextNameForQuery(tt.name, tt.scheduled)) + }) + } +}