kvdb: passing sqlite db test

kvdb/backend.go

@@ -240,8 +240,9 @@ func updateLastCompactionDate(dbFile string) error {
 
 // GetTestBackend opens (or creates if doesn't exist) a bbolt or etcd
 // backed database (for testing), and returns a kvdb.Backend and a cleanup
-// func. Whether to create/open bbolt or embedded etcd database is based
-// on the TestBackend constant which is conditionally compiled with build tag.
+// func. Whether to create/open bbolt, embedded etcd, postgres, or sqlite
+// database is based on the TestBackend constant which is conditionally compiled
+// with build tag.
 // The passed path is used to hold all db files, while the name is only used
 // for bbolt.
 func GetTestBackend(path, name string) (Backend, func(), error) {
@@ -288,9 +289,7 @@ func GetTestBackend(path, name string) (Backend, func(), error) {
 			return nil, nil, err
 		}
 
-		return f.DB(), func() {
-			_ = f.DB().Close()
-		}, nil
+		return f.DB(), f.Cleanup, nil
 	}
 
 	return nil, nil, fmt.Errorf("unknown backend")

kvdb/kvdb_sqlite.go

@@ -8,7 +8,7 @@ import "github.com/lightningnetwork/lnd/kvdb/sqlite"
 const TestBackend = SqliteBackendName
 
 func NewSqliteFixture() (sqlite.Fixture, error) {
-	f, err := sqlite.NewFixture()
+	f, err := sqlite.NewSqliteTestFixture("test")
 
 	return f, err
 }

kvdb/log.go

@@ -3,6 +3,7 @@ package kvdb
 import (
 	"github.com/btcsuite/btclog"
 	"github.com/lightningnetwork/lnd/kvdb/postgres"
+	"github.com/lightningnetwork/lnd/kvdb/sqlite"
 )
 
 // log is a logger that is initialized as disabled.  This means the package will
@@ -14,4 +15,5 @@ func UseLogger(logger btclog.Logger) {
 	log = logger
 
 	postgres.UseLogger(log)
+	sqlite.UseLogger(log)
 }

kvdb/sqlite/config.go

@@ -1,5 +1,10 @@
 package sqlite
 
+import "time"
+
 // Config holds sqlite configuration data.
 type Config struct {
+	TablePrefix string        `long:"table_prefix" description:"Prefix that will be added to each database table."`
+	Filename    string        `long:"filename" description:"Full path to sqlite database file."`
+	Timeout     time.Duration `long:"timeout" description:"Database connection timeout. Set to zero to disable."`
 }

kvdb/sqlite/db.go

@@ -10,8 +10,10 @@ import (
 	"fmt"
 	"io"
 	"sync"
+	"time"
 
 	"github.com/btcsuite/btcwallet/walletdb"
+	_ "github.com/mattn/go-sqlite3"
 )
 
 const (
@@ -31,10 +33,6 @@ type db struct {
 	// cfg is the sqlite connection config.
 	cfg *Config
 
-	// prefix is the table name prefix that is used to simulate namespaces.
-	// We don't use schemas because at least sqlite does not support that.
-	prefix string
-
 	// ctx is the overall context for the database driver.
 	//
 	// TODO: This is an anti-pattern that is in place until the kvdb
@@ -61,28 +59,90 @@ func Init() {
 }
 
 // newsqliteBackend returns a db object initialized with the passed backend
-// config. If sqlite connection cannot be estabished, then returns error.
-func newSqliteBackend(ctx context.Context, config *Config, prefix string) (
+// config. If the sqlite database cannot be opened, then returns error.
+func newSqliteBackend(ctx context.Context, config *Config) (
 	*db, error) {
 
-	if prefix == "" {
-		return nil, errors.New("empty sqlite prefix")
+	if config.Filename == "" {
+		return nil, errors.New("empty sqlite database file")
+	}
+
+	if config.TablePrefix == "" {
+		return nil, errors.New("empty sqlite table prefix")
+	}
+
+	dbConn, err := sql.Open("sqlite3", config.Filename)
+	if err != nil {
+		return nil, err
+	}
+
+	// Compose system table names.
+	table := fmt.Sprintf(
+		"%s_%s", config.TablePrefix, kvTableName,
+	)
+
+	// Execute the create statements to set up a kv table in postgres. Every
+	// row points to the bucket that it is one via its parent_id field. A
+	// NULL parent_id means that the key belongs to the upper-most bucket in
+	// this table. A constraint on parent_id is enforcing referential
+	// integrity.
+	//
+	// Furthermore there is a <table>_p index on parent_id that is required
+	// for the foreign key constraint.
+	//
+	// Finally there are unique indices on (parent_id, key) to prevent the
+	// same key being present in a bucket more than once (<table>_up and
+	// <table>_unp). In sqlite, a single index wouldn't enforce the unique
+	// constraint on rows with a NULL parent_id. Therefore two indices are
+	// defined.
+	_, err = dbConn.ExecContext(ctx, `
+PRAGMA foreign_keys = ON;
+CREATE TABLE IF NOT EXISTS `+table+`
+(
+    id INTEGER PRIMARY KEY AUTOINCREMENT,
+    key bytea NOT NULL,
+    value bytea,
+    parent_id bigint,
+    sequence bigint,
+    CONSTRAINT `+table+`_parent FOREIGN KEY (parent_id)
+        REFERENCES `+table+` (id) MATCH SIMPLE
+        ON DELETE CASCADE
+);
+
+CREATE INDEX IF NOT EXISTS `+table+`_p
+    ON `+table+` (parent_id);
+
+CREATE UNIQUE INDEX IF NOT EXISTS `+table+`_up
+    ON `+table+`
+    (parent_id, key) WHERE parent_id IS NOT NULL;
+
+CREATE UNIQUE INDEX IF NOT EXISTS `+table+`_unp 
+    ON `+table+` (key) WHERE parent_id IS NULL;
+`)
+
+	if err != nil {
+		_ = dbConn.Close()
+
+		return nil, err
 	}
 
 	backend := &db{
-		cfg:    config,
-		prefix: prefix,
-		ctx:    ctx,
-		db:     nil,
-		table:  "",
+		cfg:   config,
+		ctx:   ctx,
+		db:    dbConn,
+		table: table,
 	}
 
 	return backend, nil
 }
 
-// getPrefixedTableName returns a table name for this prefix (namespace).
-func (db *db) getPrefixedTableName(table string) string {
-	return fmt.Sprintf("%s_%s", db.prefix, table)
+// getTimeoutCtx gets a timeout context for database requests.
+func (db *db) getTimeoutCtx() (context.Context, func()) {
+	if db.cfg.Timeout == time.Duration(0) {
+		return db.ctx, func() {}
+	}
+
+	return context.WithTimeout(db.ctx, db.cfg.Timeout)
 }
 
 // catchPanic executes the specified function. If a panic occurs, it is returned
@@ -115,7 +175,12 @@ func catchPanic(f func() error) (err error) {
 // expect retries of the f closure (depending on the database backend used), the
 // reset function will be called before each retry respectively.
 func (db *db) View(f func(tx walletdb.ReadTx) error, reset func()) error {
-	return nil
+	return db.executeTransaction(
+		func(tx walletdb.ReadWriteTx) error {
+			return f(tx.(walletdb.ReadTx))
+		},
+		reset, true,
+	)
 }
 
 // Update opens a database read/write transaction and executes the function f
@@ -126,8 +191,7 @@ func (db *db) View(f func(tx walletdb.ReadTx) error, reset func()) error {
 // returned. As callers may expect retries of the f closure, the reset function
 // will be called before each retry respectively.
 func (db *db) Update(f func(tx walletdb.ReadWriteTx) error, reset func()) (err error) {
-	err = nil
-	return nil
+	return db.executeTransaction(f, reset, false)
 }
 
 // executeTransaction creates a new read-only or read-write transaction and
@@ -137,7 +201,21 @@ func (db *db) executeTransaction(f func(tx walletdb.ReadWriteTx) error,
 
 	reset()
 
-	return nil
+	tx, err := newReadWriteTx(db, readOnly)
+	if err != nil {
+		return err
+	}
+
+	err = catchPanic(func() error { return f(tx) })
+	if err != nil {
+		if rollbackErr := tx.Rollback(); rollbackErr != nil {
+			log.Errorf("Error rolling back tx: %v", rollbackErr)
+		}
+
+		return err
+	}
+
+	return tx.Commit()
 }
 
 // PrintStats returns all collected stats pretty printed into a string.
@@ -147,12 +225,12 @@ func (db *db) PrintStats() string {
 
 // BeginReadWriteTx opens a database read+write transaction.
 func (db *db) BeginReadWriteTx() (walletdb.ReadWriteTx, error) {
-	return nil, nil
+	return newReadWriteTx(db, false)
 }
 
 // BeginReadTx opens a database read transaction.
 func (db *db) BeginReadTx() (walletdb.ReadTx, error) {
-	return nil, nil
+	return newReadWriteTx(db, true)
 }
 
 // Copy writes a copy of the database to the provided writer. This call will
@@ -165,5 +243,7 @@ func (db *db) Copy(w io.Writer) error {
 // Close cleanly shuts down the database and syncs all data.
 // This function is part of the walletdb.Db interface implementation.
 func (db *db) Close() error {
-	return nil
+	log.Infof("Closing database %v", db.cfg.Filename)
+
+	return db.db.Close()
 }

kvdb/sqlite/db_test.go

@@ -0,0 +1,26 @@
+//go:build kvdb_sqlite
+// +build kvdb_sqlite
+
+package sqlite
+
+import (
+	"testing"
+
+	"github.com/btcsuite/btcwallet/walletdb/walletdbtest"
+	"github.com/stretchr/testify/require"
+	"golang.org/x/net/context"
+)
+
+// TestInterface performs all interfaces tests for this database driver.
+func TestInterface(t *testing.T) {
+	fixture, err := NewSqliteTestFixture("test")
+	require.NoError(t, err)
+	defer fixture.Cleanup()
+
+	// dbType is the database type name for this driver.
+	const dbType = "sqlite"
+
+	ctx := context.Background()
+
+	walletdbtest.TestInterface(t, dbType, ctx, fixture.Config)
+}

kvdb/sqlite/driver.go

@@ -16,61 +16,54 @@ const (
 
 // parseArgs parses the arguments from the walletdb Open/Create methods.
 func parseArgs(funcName string, args ...interface{}) (context.Context,
-	*Config, string, error) {
+	*Config, error) {
 
-	if len(args) != 3 {
-		return nil, nil, "", fmt.Errorf("invalid number of arguments "+
+	if len(args) != 2 {
+		return nil, nil, fmt.Errorf("invalid number of arguments "+
 			"to %s.%s -- expected: context.Context, "+
-			"sqlite.Config, string", dbType, funcName,
+			"sqlite.Config", dbType, funcName,
 		)
 	}
 
 	ctx, ok := args[0].(context.Context)
 	if !ok {
-		return nil, nil, "", fmt.Errorf("argument 0 to %s.%s is "+
+		return nil, nil, fmt.Errorf("argument 0 to %s.%s is "+
 			"invalid -- expected: context.Context",
 			dbType, funcName,
 		)
 	}
 
 	config, ok := args[1].(*Config)
 	if !ok {
-		return nil, nil, "", fmt.Errorf("argument 1 to %s.%s is "+
+		return nil, nil, fmt.Errorf("argument 1 to %s.%s is "+
 			"invalid -- expected: sqlite.Config",
 			dbType, funcName,
 		)
 	}
 
-	prefix, ok := args[2].(string)
-	if !ok {
-		return nil, nil, "", fmt.Errorf("argument 2 to %s.%s is "+
-			"invalid -- expected string", dbType,
-			funcName)
-	}
-
-	return ctx, config, prefix, nil
+	return ctx, config, nil
 }
 
 // createDBDriver is the callback provided during driver registration that
 // creates, initializes, and opens a database for use.
 func createDBDriver(args ...interface{}) (walletdb.DB, error) {
-	ctx, config, prefix, err := parseArgs("Create", args...)
+	ctx, config, err := parseArgs("Create", args...)
 	if err != nil {
 		return nil, err
 	}
 
-	return newSqliteBackend(ctx, config, prefix)
+	return newSqliteBackend(ctx, config)
 }
 
 // openDBDriver is the callback provided during driver registration that opens
 // an existing database for use.
 func openDBDriver(args ...interface{}) (walletdb.DB, error) {
-	ctx, config, prefix, err := parseArgs("Open", args...)
+	ctx, config, err := parseArgs("Open", args...)
 	if err != nil {
 		return nil, err
 	}
 
-	return newSqliteBackend(ctx, config, prefix)
+	return newSqliteBackend(ctx, config)
 }
 
 func init() {