Skip to content

Commit 9b3349b

Browse files
authored
Merge pull request #1068 from ellemouton/sql35
[sql-35]: firewalldb+rpcserver: refactor ListActions
2 parents c92e536 + 4f8c3ae commit 9b3349b

File tree

6 files changed

+284
-147
lines changed

6 files changed

+284
-147
lines changed

firewalldb/action_paginator.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ type actionPaginator struct {
1616

1717
// filterFn is the filter function which we are using to determine which
1818
// actions should be included in the return list.
19-
filterFn ListActionsFilterFn
19+
filterFn listActionsFilterFn
2020

2121
// readAction is a closure which we use to read an action from the db
2222
// given a key value pair.
@@ -32,7 +32,7 @@ type actionPaginator struct {
3232
// cfg.CountAll is set).
3333
func paginateActions(cfg *ListActionsQuery, c kvdb.RCursor,
3434
readAction func(k, v []byte) (*Action, error),
35-
filterFn ListActionsFilterFn) ([]*Action, uint64, uint64, error) {
35+
filterFn listActionsFilterFn) ([]*Action, uint64, uint64, error) {
3636

3737
if cfg == nil {
3838
cfg = &ListActionsQuery{}

firewalldb/actions.go

Lines changed: 97 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ type Action struct {
7272
}
7373

7474
// ListActionsQuery can be used to tweak the query to ListActions and
75-
// ListSessionActions.
75+
// listSessionActions.
7676
type ListActionsQuery struct {
7777
// IndexOffset is index of the action to inspect.
7878
IndexOffset uint64
@@ -91,6 +91,93 @@ type ListActionsQuery struct {
9191
CountAll bool
9292
}
9393

94+
// listActionsOptions holds the options that can be used to filter the actions
95+
// that are returned by the ListActions method.
96+
type listActionOptions struct {
97+
sessionID session.ID
98+
groupID session.ID
99+
featureName string
100+
actorName string
101+
methodName string
102+
state ActionState
103+
endTime time.Time
104+
startTime time.Time
105+
}
106+
107+
// newListActionOptions creates a new listActionOptions instance with default
108+
// query values.
109+
func newListActionOptions() *listActionOptions {
110+
return &listActionOptions{}
111+
}
112+
113+
// ListActionOption is a functional option that can be used to tweak the
114+
// behaviour of the ListActions method.
115+
type ListActionOption func(*listActionOptions)
116+
117+
// WithActionSessionID is a ListActionOption that can be used to select all
118+
// Actions performed under the given session ID.
119+
func WithActionSessionID(sessionID session.ID) ListActionOption {
120+
return func(o *listActionOptions) {
121+
o.sessionID = sessionID
122+
}
123+
}
124+
125+
// WithActionGroupID is a ListActionOption that can be used to select all
126+
// Actions performed under the give group ID.
127+
func WithActionGroupID(groupID session.ID) ListActionOption {
128+
return func(o *listActionOptions) {
129+
o.groupID = groupID
130+
}
131+
}
132+
133+
// WithActionStartTime is a ListActionOption that can be used to select all
134+
// Actions that were attempted after the given time.
135+
func WithActionStartTime(startTime time.Time) ListActionOption {
136+
return func(o *listActionOptions) {
137+
o.startTime = startTime
138+
}
139+
}
140+
141+
// WithActionEndTime is a ListActionOption that can be used to select all
142+
// Actions that were attempted before the given time.
143+
func WithActionEndTime(endTime time.Time) ListActionOption {
144+
return func(o *listActionOptions) {
145+
o.endTime = endTime
146+
}
147+
}
148+
149+
// WithActionFeatureName is a ListActionOption that can be used to select all
150+
// Actions that were performed by the given feature.
151+
func WithActionFeatureName(featureName string) ListActionOption {
152+
return func(o *listActionOptions) {
153+
o.featureName = featureName
154+
}
155+
}
156+
157+
// WithActionActorName is a ListActionOption that can be used to select all
158+
// Actions that were performed by the given actor.
159+
func WithActionActorName(actorName string) ListActionOption {
160+
return func(o *listActionOptions) {
161+
o.actorName = actorName
162+
}
163+
}
164+
165+
// WithActionMethodName is a ListActionOption that can be used to select all
166+
// Actions that called the given RPC method.
167+
func WithActionMethodName(methodName string) ListActionOption {
168+
return func(o *listActionOptions) {
169+
o.methodName = methodName
170+
}
171+
}
172+
173+
// WithActionState is a ListActionOption that can be used to select all Actions
174+
// that are in the given state.
175+
func WithActionState(state ActionState) ListActionOption {
176+
return func(o *listActionOptions) {
177+
o.state = state
178+
}
179+
}
180+
94181
// ActionsWriteDB is an abstraction over the Actions DB that will allow a
95182
// caller to add new actions as well as change the values of an existing action.
96183
type ActionsWriteDB interface {
@@ -174,10 +261,10 @@ var _ ActionsListDB = (*groupActionsReadDB)(nil)
174261
func (s *groupActionsReadDB) ListActions(ctx context.Context) ([]*RuleAction,
175262
error) {
176263

177-
sessionActions, err := s.db.ListGroupActions(
178-
ctx, s.groupID, func(a *Action, _ bool) (bool, bool) {
179-
return a.State == ActionStateDone, true
180-
},
264+
sessionActions, _, _, err := s.db.ListActions(
265+
ctx, nil,
266+
WithActionGroupID(s.groupID),
267+
WithActionState(ActionStateDone),
181268
)
182269
if err != nil {
183270
return nil, err
@@ -205,11 +292,11 @@ var _ ActionsListDB = (*groupFeatureActionsReadDB)(nil)
205292
func (a *groupFeatureActionsReadDB) ListActions(ctx context.Context) (
206293
[]*RuleAction, error) {
207294

208-
featureActions, err := a.db.ListGroupActions(
209-
ctx, a.groupID, func(action *Action, _ bool) (bool, bool) {
210-
return action.State == ActionStateDone &&
211-
action.FeatureName == a.featureName, true
212-
},
295+
featureActions, _, _, err := a.db.ListActions(
296+
ctx, nil,
297+
WithActionGroupID(a.groupID),
298+
WithActionState(ActionStateDone),
299+
WithActionFeatureName(a.featureName),
213300
)
214301
if err != nil {
215302
return nil, err

firewalldb/actions_kvdb.go

Lines changed: 93 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -198,19 +198,87 @@ func (db *BoltDB) SetActionState(al *ActionLocator, state ActionState,
198198
})
199199
}
200200

201-
// ListActionsFilterFn defines a function that can be used to determine if an
202-
// action should be included in a set of results or not. The reversed parameter
203-
// indicates if the actions are being traversed in reverse order or not.
204-
// The first return boolean indicates if the action should be included or not
205-
// and the second one indicates if the iteration should be stopped or not.
206-
type ListActionsFilterFn func(a *Action, reversed bool) (bool, bool)
201+
// ListActions returns a list of Actions. The query IndexOffset and MaxNum
202+
// params can be used to control the number of actions returned.
203+
// ListActionOptions may be used to filter on specific Action values. The return
204+
// values are the list of actions, the last index and the total count (iff
205+
// query.CountTotal is set).
206+
func (db *BoltDB) ListActions(ctx context.Context, query *ListActionsQuery,
207+
options ...ListActionOption) ([]*Action, uint64, uint64, error) {
208+
209+
opts := newListActionOptions()
210+
for _, o := range options {
211+
o(opts)
212+
}
213+
214+
filterFn := func(a *Action, reversed bool) (bool, bool) {
215+
timeStamp := a.AttemptedAt
216+
if !opts.endTime.IsZero() {
217+
// If actions are being considered in order and the
218+
// timestamp of this action exceeds the given end
219+
// timestamp, then there is no need to continue
220+
// traversing.
221+
if !reversed && timeStamp.After(opts.endTime) {
222+
return false, false
223+
}
224+
225+
// If the actions are in reverse order and the timestamp
226+
// comes after the end timestamp, then the actions is
227+
// not included but the search can continue.
228+
if reversed && timeStamp.After(opts.endTime) {
229+
return false, true
230+
}
231+
}
232+
233+
if !opts.startTime.IsZero() {
234+
// If actions are being considered in order and the
235+
// timestamp of this action comes before the given start
236+
// timestamp, then the action is not included but the
237+
// search can continue.
238+
if !reversed && timeStamp.Before(opts.startTime) {
239+
return false, true
240+
}
241+
242+
// If the actions are in reverse order and the timestamp
243+
// comes before the start timestamp, then there is no
244+
// need to continue traversing.
245+
if reversed && timeStamp.Before(opts.startTime) {
246+
return false, false
247+
}
248+
}
249+
250+
if opts.featureName != "" && a.FeatureName != opts.featureName {
251+
return false, true
252+
}
253+
254+
if opts.actorName != "" && a.ActorName != opts.actorName {
255+
return false, true
256+
}
207257

208-
// ListActions returns a list of Actions that pass the filterFn requirements.
209-
// The indexOffset and maxNum params can be used to control the number of
210-
// actions returned. The return values are the list of actions, the last index
211-
// and the total count (iff query.CountTotal is set).
212-
func (db *BoltDB) ListActions(filterFn ListActionsFilterFn,
213-
query *ListActionsQuery) ([]*Action, uint64, uint64, error) {
258+
if opts.methodName != "" && a.RPCMethod != opts.methodName {
259+
return false, true
260+
}
261+
262+
if opts.state != ActionStateUnknown && a.State != opts.state {
263+
return false, true
264+
}
265+
266+
return true, true
267+
}
268+
269+
if opts.sessionID != session.EmptyID {
270+
return db.listSessionActions(
271+
opts.sessionID, filterFn, query,
272+
)
273+
}
274+
if opts.groupID != session.EmptyID {
275+
actions, err := db.listGroupActions(ctx, opts.groupID, filterFn)
276+
if err != nil {
277+
return nil, 0, 0, err
278+
}
279+
280+
return actions, 0, uint64(len(actions)), nil
281+
}
214282

215283
var (
216284
actions []*Action
@@ -242,7 +310,6 @@ func (db *BoltDB) ListActions(filterFn ListActionsFilterFn,
242310
if err != nil {
243311
return nil, err
244312
}
245-
246313
return getAction(actionsBucket, locator)
247314
}
248315

@@ -255,14 +322,20 @@ func (db *BoltDB) ListActions(filterFn ListActionsFilterFn,
255322
if err != nil {
256323
return nil, 0, 0, err
257324
}
258-
259325
return actions, lastIndex, totalCount, nil
260326
}
261327

262-
// ListSessionActions returns a list of the given session's Actions that pass
328+
// listActionsFilterFn defines a function that can be used to determine if an
329+
// action should be included in a set of results or not. The reversed parameter
330+
// indicates if the actions are being traversed in reverse order or not.
331+
// The first return boolean indicates if the action should be included or not
332+
// and the second one indicates if the iteration should be continued or not.
333+
type listActionsFilterFn func(a *Action, reversed bool) (bool, bool)
334+
335+
// listSessionActions returns a list of the given session's Actions that pass
263336
// the filterFn requirements.
264-
func (db *BoltDB) ListSessionActions(sessionID session.ID,
265-
filterFn ListActionsFilterFn, query *ListActionsQuery) ([]*Action,
337+
func (db *BoltDB) listSessionActions(sessionID session.ID,
338+
filterFn listActionsFilterFn, query *ListActionsQuery) ([]*Action,
266339
uint64, uint64, error) {
267340

268341
var (
@@ -303,12 +376,12 @@ func (db *BoltDB) ListSessionActions(sessionID session.ID,
303376
return actions, lastIndex, totalCount, nil
304377
}
305378

306-
// ListGroupActions returns a list of the given session group's Actions that
379+
// listGroupActions returns a list of the given session group's Actions that
307380
// pass the filterFn requirements.
308381
//
309382
// TODO: update to allow for pagination.
310-
func (db *BoltDB) ListGroupActions(ctx context.Context, groupID session.ID,
311-
filterFn ListActionsFilterFn) ([]*Action, error) {
383+
func (db *BoltDB) listGroupActions(ctx context.Context, groupID session.ID,
384+
filterFn listActionsFilterFn) ([]*Action, error) {
312385

313386
if filterFn == nil {
314387
filterFn = func(a *Action, reversed bool) (bool, bool) {

0 commit comments

Comments
 (0)