Skip to content

[sql-41] firewalldb: add migration code for kvstores from kvdb to SQL #1079

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
3 changes: 0 additions & 3 deletions accounts/sql_migration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,6 @@ func TestAccountStoreMigration(t *testing.T) {
*db.TransactionExecutor[SQLQueries]) {

testDBStore := NewTestDB(t, clock)
t.Cleanup(func() {
require.NoError(t, testDBStore.Close())
})

store, ok := testDBStore.(*SQLStore)
require.True(t, ok)
Expand Down
4 changes: 2 additions & 2 deletions accounts/test_postgres.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@ var ErrDBClosed = errors.New("database is closed")

// NewTestDB is a helper function that creates an SQLStore database for testing.
func NewTestDB(t *testing.T, clock clock.Clock) Store {
return NewSQLStore(db.NewTestPostgresDB(t).BaseDB, clock)
return createStore(t, db.NewTestPostgresDB(t).BaseDB, clock)
}

// NewTestDBFromPath is a helper function that creates a new SQLStore with a
// connection to an existing postgres database for testing.
func NewTestDBFromPath(t *testing.T, dbPath string,
clock clock.Clock) Store {

return NewSQLStore(db.NewTestPostgresDB(t).BaseDB, clock)
return createStore(t, db.NewTestPostgresDB(t).BaseDB, clock)
}
22 changes: 22 additions & 0 deletions accounts/test_sql.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
//go:build test_db_postgres || test_db_sqlite

package accounts

import (
"testing"

"github.com/lightninglabs/lightning-terminal/db"
"github.com/lightningnetwork/lnd/clock"
"github.com/stretchr/testify/require"
)

// createStore is a helper function that creates a new SQLStore and ensure that
// it is closed when during the test cleanup.
func createStore(t *testing.T, sqlDB *db.BaseDB, clock clock.Clock) *SQLStore {
store := NewSQLStore(sqlDB, clock)
t.Cleanup(func() {
require.NoError(t, store.Close())
})

return store
}
6 changes: 3 additions & 3 deletions accounts/test_sqlite.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,15 @@ var ErrDBClosed = errors.New("database is closed")

// NewTestDB is a helper function that creates an SQLStore database for testing.
func NewTestDB(t *testing.T, clock clock.Clock) Store {
return NewSQLStore(db.NewTestSqliteDB(t).BaseDB, clock)
return createStore(t, db.NewTestSqliteDB(t).BaseDB, clock)
}

// NewTestDBFromPath is a helper function that creates a new SQLStore with a
// connection to an existing SQL database for testing.
func NewTestDBFromPath(t *testing.T, dbPath string,
clock clock.Clock) Store {

return NewSQLStore(
db.NewTestSqliteDbHandleFromPath(t, dbPath).BaseDB, clock,
return createStore(
t, db.NewTestSqliteDbHandleFromPath(t, dbPath).BaseDB, clock,
)
}
36 changes: 36 additions & 0 deletions db/sqlc/kvstores.sql.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions db/sqlc/querier.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions db/sqlc/queries/kvstores.sql
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ VALUES ($1, $2, $3, $4, $5, $6);
DELETE FROM kvstores
WHERE perm = false;

-- name: ListAllKVStoresRecords :many
SELECT *
FROM kvstores;

-- name: GetGlobalKVStoreRecord :one
SELECT value
FROM kvstores
Expand Down
9 changes: 0 additions & 9 deletions firewalldb/actions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,6 @@ func TestActionStorage(t *testing.T) {
sessDB := session.NewTestDBWithAccounts(t, clock, accountsDB)

db := NewTestDBWithSessionsAndAccounts(t, sessDB, accountsDB, clock)
t.Cleanup(func() {
_ = db.Close()
})

// Assert that attempting to add an action for a session that does not
// exist returns an error.
Expand Down Expand Up @@ -198,9 +195,6 @@ func TestListActions(t *testing.T) {
sessDB := session.NewTestDB(t, clock)

db := NewTestDBWithSessions(t, sessDB, clock)
t.Cleanup(func() {
_ = db.Close()
})

// Add 2 sessions that we can reference.
sess1, err := sessDB.NewSession(
Expand Down Expand Up @@ -466,9 +460,6 @@ func TestListGroupActions(t *testing.T) {
}

db := NewTestDBWithSessions(t, sessDB, clock)
t.Cleanup(func() {
_ = db.Close()
})

// There should not be any actions in group 1 yet.
al, _, _, err := db.ListActions(ctx, nil, WithActionGroupID(group1))
Expand Down
14 changes: 3 additions & 11 deletions firewalldb/db.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,29 +14,21 @@ var (
ErrNoSuchKeyFound = fmt.Errorf("no such key found")
)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

your commit messages could be fleshed out a bit more pls. it isnt clear why this needs to be exported (same for other commits in this PR)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, completely forgot updating the commit messages in the latest push. I've updated them in the latest push though, so I hope the commits should be understandable now!

// firewallDBs is an interface that groups the RulesDB and PrivacyMapper
// interfaces.
type firewallDBs interface {
RulesDB
PrivacyMapper
ActionDB
}

// DB manages the firewall rules database.
type DB struct {
started sync.Once
stopped sync.Once

firewallDBs
FirewallDBs

cancel fn.Option[context.CancelFunc]
}

// NewDB creates a new firewall database. For now, it only contains the
// underlying rules' and privacy mapper databases.
func NewDB(dbs firewallDBs) *DB {
func NewDB(dbs FirewallDBs) *DB {
return &DB{
firewallDBs: dbs,
FirewallDBs: dbs,
}
}

Expand Down
8 changes: 8 additions & 0 deletions firewalldb/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,3 +134,11 @@ type ActionDB interface {
// and feature name.
GetActionsReadDB(groupID session.ID, featureName string) ActionsReadDB
}

// FirewallDBs is an interface that groups the RulesDB, PrivacyMapper and
// ActionDB interfaces.
type FirewallDBs interface {
RulesDB
PrivacyMapper
ActionDB
}
12 changes: 6 additions & 6 deletions firewalldb/kvstores_kvdb.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@ the temporary store changes instead of just keeping an in-memory store is that
we can then guarantee atomicity if changes are made to both the permanent and
temporary stores.

rules -> perm -> rule-name -> global -> {k:v}
-> sessions -> group ID -> session-kv-store -> {k:v}
-> feature-kv-stores -> feature-name -> {k:v}
"rules" -> "perm" -> rule-name -> "global" -> {k:v}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the update here 👍

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah thanks for the fix!! 🙏

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while we are here, can we add < & > around keys that vary? like <rule-name>

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and <group ID>& <feature name>

"session-kv-store" -> group ID -> {k:v}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing the -> here

-> "feature-kv-stores" -> feature-name -> {k:v}

-> temp -> rule-name -> global -> {k:v}
-> sessions -> group ID -> session-kv-store -> {k:v}
-> feature-kv-stores -> feature-name -> {k:v}
-> "temp" -> rule-name -> "global" -> {k:v}
"session-kv-store" -> group ID -> {k:v}
-> "feature-kv-stores" -> feature-name -> {k:v}
*/

var (
Expand Down
Loading
Loading