Skip to content

Commit ed96f93

Browse files
committed
firewalldb: fix BBolt ListGroupActions when Reversed is set
Add a test covering a call to ListActions with group ID set and Reversed set and assert that the actions are returned in the correct order. This reveals a bug in the BoltDB implementation which was not making use of the query.Reversed parameter when group ID is set. NOTE: it still wont do pagination correctly (ie the pagination params still dont get used) but we at least here can easily let it use the Reversed param.
1 parent e40328c commit ed96f93

File tree

2 files changed

+31
-6
lines changed

2 files changed

+31
-6
lines changed

firewalldb/actions_kvdb.go

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,13 @@ func (db *BoltDB) ListActions(ctx context.Context, query *ListActionsQuery,
330330
)
331331
}
332332
if opts.groupID != session.EmptyID {
333-
actions, err := db.listGroupActions(ctx, opts.groupID, filterFn)
333+
var reversed bool
334+
if query != nil {
335+
reversed = query.Reversed
336+
}
337+
actions, err := db.listGroupActions(
338+
ctx, opts.groupID, filterFn, reversed,
339+
)
334340
if err != nil {
335341
return nil, 0, 0, err
336342
}
@@ -439,11 +445,11 @@ func (db *BoltDB) listSessionActions(sessionID session.ID,
439445
//
440446
// TODO: update to allow for pagination.
441447
func (db *BoltDB) listGroupActions(ctx context.Context, groupID session.ID,
442-
filterFn listActionsFilterFn) ([]*Action, error) {
448+
filterFn listActionsFilterFn, reversed bool) ([]*Action, error) {
443449

444450
if filterFn == nil {
445451
filterFn = func(a *Action, reversed bool) (bool, bool) {
446-
return true, true
452+
return true, reversed
447453
}
448454
}
449455

@@ -482,9 +488,18 @@ func (db *BoltDB) listGroupActions(ctx context.Context, groupID session.ID,
482488
return err
483489
}
484490

485-
include, cont := filterFn(action, false)
491+
include, cont := filterFn(action, reversed)
486492
if include {
487-
actions = append(actions, action)
493+
if !reversed {
494+
actions = append(
495+
actions, action,
496+
)
497+
} else {
498+
actions = append(
499+
[]*Action{action},
500+
actions...,
501+
)
502+
}
488503
}
489504

490505
if !cont {

firewalldb/actions_test.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -497,12 +497,22 @@ func TestListGroupActions(t *testing.T) {
497497
_, err = db.AddAction(ctx, action2Req)
498498
require.NoError(t, err)
499499

500-
// There should now be actions in the group.
500+
// There should now be two actions in the group.
501501
al, _, _, err = db.ListActions(ctx, nil, WithActionGroupID(group1))
502502
require.NoError(t, err)
503503
require.Len(t, al, 2)
504504
assertEqualActions(t, action1, al[0])
505505
assertEqualActions(t, action2, al[1])
506+
507+
// Try the reversed query too.
508+
al, _, _, err = db.ListActions(
509+
ctx, &ListActionsQuery{Reversed: true},
510+
WithActionGroupID(group1),
511+
)
512+
require.NoError(t, err)
513+
require.Len(t, al, 2)
514+
assertEqualActions(t, action2, al[0])
515+
assertEqualActions(t, action1, al[1])
506516
}
507517

508518
func assertEqualActions(t *testing.T, expected, got *Action) {

0 commit comments

Comments
 (0)