firewalldb+rpcserver: refactor ListActions

Here we move the filter logic behind the interface so that our sql
implementation can make use of indexes.

Author: ellemouton

firewalldb/action_paginator.go

@@ -16,7 +16,7 @@ type actionPaginator struct {
 
 	// filterFn is the filter function which we are using to determine which
 	// actions should be included in the return list.
-	filterFn ListActionsFilterFn
+	filterFn listActionsFilterFn
 
 	// readAction is a closure which we use to read an action from the db
 	// given a key value pair.
@@ -32,7 +32,7 @@ type actionPaginator struct {
 // cfg.CountAll is set).
 func paginateActions(cfg *ListActionsQuery, c kvdb.RCursor,
 	readAction func(k, v []byte) (*Action, error),
-	filterFn ListActionsFilterFn) ([]*Action, uint64, uint64, error) {
+	filterFn listActionsFilterFn) ([]*Action, uint64, uint64, error) {
 
 	if cfg == nil {
 		cfg = &ListActionsQuery{}

firewalldb/actions.go

@@ -72,7 +72,7 @@ type Action struct {
 }
 
 // ListActionsQuery can be used to tweak the query to ListActions and
-// ListSessionActions.
+// listSessionActions.
 type ListActionsQuery struct {
 	// IndexOffset is index of the action to inspect.
 	IndexOffset uint64
@@ -91,6 +91,93 @@ type ListActionsQuery struct {
 	CountAll bool
 }
 
+// listActionsOptions holds the options that can be used to filter the actions
+// that are returned by the ListActions method.
+type listActionOptions struct {
+	sessionID   session.ID
+	groupID     session.ID
+	featureName string
+	actorName   string
+	methodName  string
+	state       ActionState
+	endTime     time.Time
+	startTime   time.Time
+}
+
+// newListActionOptions creates a new listActionOptions instance with default
+// query values.
+func newListActionOptions() *listActionOptions {
+	return &listActionOptions{}
+}
+
+// ListActionOption is a functional option that can be used to tweak the
+// behaviour of the ListActions method.
+type ListActionOption func(*listActionOptions)
+
+// WithActionSessionID is a ListActionOption that can be select all Actions
+// with the given session ID.
+func WithActionSessionID(sessionID session.ID) ListActionOption {
+	return func(o *listActionOptions) {
+		o.sessionID = sessionID
+	}
+}
+
+// WithActionGroupID is a ListActionOption that can be select all Actions with
+// performed under the give group ID.
+func WithActionGroupID(groupID session.ID) ListActionOption {
+	return func(o *listActionOptions) {
+		o.groupID = groupID
+	}
+}
+
+// WithActionStartTime is a ListActionOption that can be used to select all
+// Actions that were attempted after the given time.
+func WithActionStartTime(startTime time.Time) ListActionOption {
+	return func(o *listActionOptions) {
+		o.startTime = startTime
+	}
+}
+
+// WithActionEndTime is a ListActionOption that can be used to select all
+// Actions that were attempted before the given time.
+func WithActionEndTime(endTime time.Time) ListActionOption {
+	return func(o *listActionOptions) {
+		o.endTime = endTime
+	}
+}
+
+// WithActionFeatureName is a ListActionOption that can be used to select all
+// Actions that were performed by the given feature.
+func WithActionFeatureName(featureName string) ListActionOption {
+	return func(o *listActionOptions) {
+		o.featureName = featureName
+	}
+}
+
+// WithActionActorName is a ListActionOption that can be used to select all
+// Actions that were performed by the given actor.
+func WithActionActorName(actorName string) ListActionOption {
+	return func(o *listActionOptions) {
+		o.actorName = actorName
+	}
+}
+
+// WithActionMethodName is a ListActionOption that can be used to select all
+// Actions that called the given RPC method.
+func WithActionMethodName(methodName string) ListActionOption {
+	return func(o *listActionOptions) {
+		o.methodName = methodName
+	}
+}
+
+// WithActionState is a ListActionOption that can be used to select all Actions
+// that are in the given state.
+func WithActionState(state ActionState) ListActionOption {
+	return func(o *listActionOptions) {
+		o.state = state
+	}
+}
+
 // ActionsWriteDB is an abstraction over the Actions DB that will allow a
 // caller to add new actions as well as change the values of an existing action.
 type ActionsWriteDB interface {
@@ -174,10 +261,10 @@ var _ ActionsListDB = (*groupActionsReadDB)(nil)
 func (s *groupActionsReadDB) ListActions(ctx context.Context) ([]*RuleAction,
 	error) {
 
-	sessionActions, err := s.db.ListGroupActions(
-		ctx, s.groupID, func(a *Action, _ bool) (bool, bool) {
-			return a.State == ActionStateDone, true
-		},
+	sessionActions, _, _, err := s.db.ListActions(
+		ctx, nil,
+		WithActionGroupID(s.groupID),
+		WithActionState(ActionStateDone),
 	)
 	if err != nil {
 		return nil, err
@@ -205,11 +292,11 @@ var _ ActionsListDB = (*groupFeatureActionsReadDB)(nil)
 func (a *groupFeatureActionsReadDB) ListActions(ctx context.Context) (
 	[]*RuleAction, error) {
 
-	featureActions, err := a.db.ListGroupActions(
-		ctx, a.groupID, func(action *Action, _ bool) (bool, bool) {
-			return action.State == ActionStateDone &&
-				action.FeatureName == a.featureName, true
-		},
+	featureActions, _, _, err := a.db.ListActions(
+		ctx, nil,
+		WithActionGroupID(a.groupID),
+		WithActionState(ActionStateDone),
+		WithActionFeatureName(a.featureName),
 	)
 	if err != nil {
 		return nil, err

firewalldb/actions_kvdb.go

@@ -198,19 +198,91 @@ func (db *BoltDB) SetActionState(al *ActionLocator, state ActionState,
 	})
 }
 
-// ListActionsFilterFn defines a function that can be used to determine if an
-// action should be included in a set of results or not. The reversed parameter
-// indicates if the actions are being traversed in reverse order or not.
-// The first return boolean indicates if the action should be included or not
-// and the second one indicates if the iteration should be stopped or not.
-type ListActionsFilterFn func(a *Action, reversed bool) (bool, bool)
-
 // ListActions returns a list of Actions that pass the filterFn requirements.
 // The indexOffset and maxNum params can be used to control the number of
 // actions returned. The return values are the list of actions, the last index
 // and the total count (iff query.CountTotal is set).
-func (db *BoltDB) ListActions(filterFn ListActionsFilterFn,
-	query *ListActionsQuery) ([]*Action, uint64, uint64, error) {
+// ListActions returns a list of Actions. The query IndexOffset and MaxNum
+// params can be used to control the number of actions returned.
+// ListActionOptions may be used to filter on specific Action values. The return
+// values are the list of actions, the last index and the total count (iff
+// query.CountTotal is set).
+func (db *BoltDB) ListActions(ctx context.Context, query *ListActionsQuery,
+	options ...ListActionOption) ([]*Action, uint64, uint64, error) {
+
+	opts := newListActionOptions()
+	for _, o := range options {
+		o(opts)
+	}
+
+	filterFn := func(a *Action, reversed bool) (bool, bool) {
+		timeStamp := a.AttemptedAt
+		if !opts.endTime.IsZero() {
+			// If actions are being considered in order and the
+			// timestamp of this action exceeds the given end
+			// timestamp, then there is no need to continue
+			// traversing.
+			if !reversed && timeStamp.After(opts.endTime) {
+				return false, false
+			}
+
+			// If the actions are in reverse order and the timestamp
+			// comes after the end timestamp, then the actions is
+			// not included but the search can continue.
+			if reversed && timeStamp.After(opts.endTime) {
+				return false, true
+			}
+		}
+
+		if !opts.startTime.IsZero() {
+			// If actions are being considered in order and the
+			// timestamp of this action comes before the given start
+			// timestamp, then the action is not included but the
+			// search can continue.
+			if !reversed && timeStamp.Before(opts.startTime) {
+				return false, true
+			}
+
+			// If the actions are in reverse order and the timestamp
+			// comes before the start timestamp, then there is no
+			// need to continue traversing.
+			if reversed && timeStamp.Before(opts.startTime) {
+				return false, false
+			}
+		}
+
+		if opts.featureName != "" && a.FeatureName != opts.featureName {
+			return false, true
+		}
+
+		if opts.actorName != "" && a.ActorName != opts.actorName {
+			return false, true
+		}
+
+		if opts.methodName != "" && a.RPCMethod != opts.methodName {
+			return false, true
+		}
+
+		if opts.state != ActionStateUnknown && a.State != opts.state {
+			return false, true
+		}
+
+		return true, true
+	}
+
+	if opts.sessionID != session.EmptyID {
+		return db.listSessionActions(
+			opts.sessionID, filterFn, query,
+		)
+	}
+	if opts.groupID != session.EmptyID {
+		actions, err := db.listGroupActions(ctx, opts.groupID, filterFn)
+		if err != nil {
+			return nil, 0, 0, err
+		}
+
+		return actions, 0, uint64(len(actions)), nil
+	}
 
 	var (
 		actions    []*Action
@@ -222,30 +294,24 @@ func (db *BoltDB) ListActions(filterFn ListActionsFilterFn,
 		if err != nil {
 			return err
 		}
-
 		actionsBucket := mainActionsBucket.Bucket(actionsKey)
 		if actionsBucket == nil {
 			return ErrNoSuchKeyFound
 		}
-
 		actionsIndexBucket := mainActionsBucket.Bucket(actionsIndex)
 		if actionsIndexBucket == nil {
 			return ErrNoSuchKeyFound
 		}
-
 		readAction := func(index, locatorBytes []byte) (*Action,
 			error) {
-
 			locator, err := deserializeActionLocator(
 				bytes.NewReader(locatorBytes),
 			)
 			if err != nil {
 				return nil, err
 			}
-
 			return getAction(actionsBucket, locator)
 		}
-
 		actions, lastIndex, totalCount, err = paginateActions(
 			query, actionsIndexBucket.Cursor(), readAction,
 			filterFn,
@@ -255,14 +321,20 @@ func (db *BoltDB) ListActions(filterFn ListActionsFilterFn,
 	if err != nil {
 		return nil, 0, 0, err
 	}
-
 	return actions, lastIndex, totalCount, nil
 }
 
-// ListSessionActions returns a list of the given session's Actions that pass
+// listActionsFilterFn defines a function that can be used to determine if an
+// action should be included in a set of results or not. The reversed parameter
+// indicates if the actions are being traversed in reverse order or not.
+// The first return boolean indicates if the action should be included or not
+// and the second one indicates if the iteration should be stopped or not.
+type listActionsFilterFn func(a *Action, reversed bool) (bool, bool)
+
+// listSessionActions returns a list of the given session's Actions that pass
 // the filterFn requirements.
-func (db *BoltDB) ListSessionActions(sessionID session.ID,
-	filterFn ListActionsFilterFn, query *ListActionsQuery) ([]*Action,
+func (db *BoltDB) listSessionActions(sessionID session.ID,
+	filterFn listActionsFilterFn, query *ListActionsQuery) ([]*Action,
 	uint64, uint64, error) {
 
 	var (
@@ -303,12 +375,12 @@ func (db *BoltDB) ListSessionActions(sessionID session.ID,
 	return actions, lastIndex, totalCount, nil
 }
 
-// ListGroupActions returns a list of the given session group's Actions that
+// listGroupActions returns a list of the given session group's Actions that
 // pass the filterFn requirements.
 //
 // TODO: update to allow for pagination.
-func (db *BoltDB) ListGroupActions(ctx context.Context, groupID session.ID,
-	filterFn ListActionsFilterFn) ([]*Action, error) {
+func (db *BoltDB) listGroupActions(ctx context.Context, groupID session.ID,
+	filterFn listActionsFilterFn) ([]*Action, error) {
 
 	if filterFn == nil {
 		filterFn = func(a *Action, reversed bool) (bool, bool) {