From c58c35509bea7c2c99c0755cc12dcbbd4ae8b843 Mon Sep 17 00:00:00 2001 From: Felipe Martin <812088+fmartingr@users.noreply.github.com> Date: Thu, 2 Jan 2025 09:46:39 +0100 Subject: [PATCH] feat: improve SQLite performance (#1024) * refactor: Improve SQLite performance with connection pooling and retry logic * feat: Add withTx and withTxRetry methods to SQLiteDatabase for handling database locks * refactor: add Init command to all databases * refactor: Improve transaction handling with retry and error management * refactor: Remove panic/recover pattern in transaction handling * refactor: Replace `errors.WithStack` with `fmt.Errorf` in transaction methods * docs: Add docstrings to `withTx` and `withTxRetry` methods in SQLite database implementation * feat: use new withTxRetry in SaveBookmarks * feat: sqlite inmmediate transactions by default * refactor: Split SQLiteDatabase into separate writer and reader dbbase instances * refactor: Update Init method to configure both reader and writer database connections * feat: use writer/reader sqlite databases * refactor: Replace all read calls to use the `reader` database instance * refactor: Replace errors.WithStack with fmt.Errorf and add nil checks refactor: Replace errors.WithStack with fmt.Errorf and add proper error handling fix: Handle potential database connection errors with improved error wrapping refactor: Replace errors.WithStack with fmt.Errorf and improve error handling refactor: Replace error handling with fmt.Errorf and proper nil checks refactor: Replace errors.WithStack with fmt.Errorf and add nil error checks refactor: Replace errors.WithStack with fmt.Errorf and add nil checks in sqlite.go refactor: Replace errors.WithStack with fmt.Errorf and add nil checks refactor: Replace errors.WithStack with fmt.Errorf and improve error handling refactor: Replace remaining errors.WithStack with fmt.Errorf in sqlite.go * refactor: Use withTxRetry for SetDatabaseSchemaVersion method * fix: Simplify error handling in GetBookmark and GetAccount methods * refactor: Remove duplicated non-nil error checks in sqlite.go fix: duplicated non-nil checks * tests: use testutil instead of a manual in memory sqlite db * fix: openbsd sqlite connection --- internal/config/config.go | 2 +- internal/database/database.go | 3 + internal/database/mysql.go | 5 + internal/database/pg.go | 5 + internal/database/sqlite.go | 258 ++++++++++++++++++++-------- internal/database/sqlite_noncgo.go | 21 ++- internal/database/sqlite_openbsd.go | 21 ++- internal/domains/bookmarks_test.go | 16 +- 8 files changed, 234 insertions(+), 97 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index f62769410..e70f00284 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -120,7 +120,7 @@ func (c Config) SetDefaults(logger *logrus.Logger, portableMode bool) { // Set default database url if not set if c.Database.DBMS == "" && c.Database.URL == "" { - c.Database.URL = fmt.Sprintf("sqlite:///%s", filepath.Join(c.Storage.DataDir, "shiori.db")) + c.Database.URL = fmt.Sprintf("sqlite:///%s?_txlock=immediate", filepath.Join(c.Storage.DataDir, "shiori.db")) } c.Http.SetDefaults(logger) diff --git a/internal/database/database.go b/internal/database/database.go index 211a91919..298fb81d2 100644 --- a/internal/database/database.go +++ b/internal/database/database.go @@ -67,6 +67,9 @@ type DB interface { // DBx is the underlying sqlx.DB DBx() *sqlx.DB + // Init initializes the database + Init(ctx context.Context) error + // Migrate runs migrations for this database Migrate(ctx context.Context) error diff --git a/internal/database/mysql.go b/internal/database/mysql.go index a23816b81..668db7ae2 100644 --- a/internal/database/mysql.go +++ b/internal/database/mysql.go @@ -95,6 +95,11 @@ func (db *MySQLDatabase) DBx() *sqlx.DB { return db.DB } +// Init initializes the database +func (db *MySQLDatabase) Init(ctx context.Context) error { + return nil +} + // Migrate runs migrations for this database engine func (db *MySQLDatabase) Migrate(ctx context.Context) error { if err := runMigrations(ctx, db, mysqlMigrations); err != nil { diff --git a/internal/database/pg.go b/internal/database/pg.go index a4403ff8b..05e1116c2 100644 --- a/internal/database/pg.go +++ b/internal/database/pg.go @@ -96,6 +96,11 @@ func (db *PGDatabase) DBx() *sqlx.DB { return db.DB } +// Init initializes the database +func (db *PGDatabase) Init(ctx context.Context) error { + return nil +} + // Migrate runs migrations for this database engine func (db *PGDatabase) Migrate(ctx context.Context) error { if err := runMigrations(ctx, db, postgresMigrations); err != nil { diff --git a/internal/database/sqlite.go b/internal/database/sqlite.go index 0675db6aa..c9e663f37 100644 --- a/internal/database/sqlite.go +++ b/internal/database/sqlite.go @@ -5,6 +5,7 @@ import ( "database/sql" "fmt" "log" + "runtime" "strings" "time" @@ -66,7 +67,98 @@ var sqliteMigrations = []migration{ // SQLiteDatabase is implementation of Database interface // for connecting to SQLite3 database. type SQLiteDatabase struct { - dbbase + writer *dbbase + reader *dbbase +} + +// withTx executes the given function within a transaction. +// If the function returns an error, the transaction is rolled back. +// Otherwise, the transaction is committed. +func (db *SQLiteDatabase) withTx(ctx context.Context, fn func(tx *sqlx.Tx) error) error { + tx, err := db.writer.BeginTxx(ctx, nil) + if err != nil { + return fmt.Errorf("failed to begin transaction: %w", err) + } + + err = fn(tx) + if err != nil { + rbErr := tx.Rollback() + if rbErr != nil { + return fmt.Errorf("error rolling back: %v (original error: %w)", rbErr, err) + } + return fmt.Errorf("transaction failed: %w", err) + } + + if err := tx.Commit(); err != nil { + return fmt.Errorf("failed to commit transaction: %w", err) + } + + return nil +} + +// withTxRetry executes the given function within a transaction with retry logic. +// It will retry up to 3 times if the database is locked, with exponential backoff. +// For other errors, it returns immediately. +func (db *SQLiteDatabase) withTxRetry(ctx context.Context, fn func(tx *sqlx.Tx) error) error { + maxRetries := 3 + var lastErr error + + for i := 0; i < maxRetries; i++ { + err := db.withTx(ctx, fn) + if err == nil { + return nil + } + + if strings.Contains(err.Error(), "database is locked") { + lastErr = err + time.Sleep(time.Duration(i+1) * 100 * time.Millisecond) + continue + } + + return fmt.Errorf("transaction failed after retry: %w", err) + } + + return fmt.Errorf("transaction failed after max retries, last error: %w", lastErr) +} + +// Init sets up the SQLite database with optimal settings for both reader and writer connections +func (db *SQLiteDatabase) Init(ctx context.Context) error { + // Initialize both connections with appropriate settings + for _, conn := range []*dbbase{db.writer, db.reader} { + // Reuse connections for up to one hour + conn.SetConnMaxLifetime(time.Hour) + + // Enable WAL mode for better concurrency + if _, err := conn.ExecContext(ctx, `PRAGMA journal_mode=WAL`); err != nil { + return fmt.Errorf("failed to set journal mode: %w", err) + } + + // Set busy timeout to avoid "database is locked" errors + if _, err := conn.ExecContext(ctx, `PRAGMA busy_timeout=5000`); err != nil { + return fmt.Errorf("failed to set busy timeout: %w", err) + } + + // Other performance and reliability settings + pragmas := []string{ + `PRAGMA synchronous=NORMAL`, + `PRAGMA cache_size=-2000`, // Use 2MB of memory for cache + `PRAGMA foreign_keys=ON`, + } + + for _, pragma := range pragmas { + if _, err := conn.ExecContext(ctx, pragma); err != nil { + return fmt.Errorf("failed to set pragma %s: %w", pragma, err) + } + } + } + + // Use a single connection on the writer to avoid database is locked errors + db.writer.SetMaxOpenConns(1) + + // Set maximum idle connections for the reader to number of CPUs (maxing at 4) + db.reader.SetMaxIdleConns(max(4, runtime.NumCPU())) + + return nil } type bookmarkContent struct { @@ -80,15 +172,20 @@ type tagContent struct { model.Tag } -// DBX returns the underlying sqlx.DB object +// DBX returns the underlying sqlx.DB object for writes func (db *SQLiteDatabase) DBx() *sqlx.DB { - return db.DB + return db.writer.DB +} + +// ReaderDBx returns the underlying sqlx.DB object for reading +func (db *SQLiteDatabase) ReaderDBx() *sqlx.DB { + return db.reader.DB } // Migrate runs migrations for this database engine func (db *SQLiteDatabase) Migrate(ctx context.Context) error { if err := runMigrations(ctx, db, sqliteMigrations); err != nil { - return errors.WithStack(err) + return fmt.Errorf("failed to run migrations: %w", err) } return nil @@ -98,9 +195,9 @@ func (db *SQLiteDatabase) Migrate(ctx context.Context) error { func (db *SQLiteDatabase) GetDatabaseSchemaVersion(ctx context.Context) (string, error) { var version string - err := db.GetContext(ctx, &version, "SELECT database_schema_version FROM shiori_system") + err := db.reader.GetContext(ctx, &version, "SELECT database_schema_version FROM shiori_system") if err != nil { - return "", errors.WithStack(err) + return "", fmt.Errorf("failed to get database schema version: %w", err) } return version, nil @@ -108,15 +205,17 @@ func (db *SQLiteDatabase) GetDatabaseSchemaVersion(ctx context.Context) (string, // SetDatabaseSchemaVersion sets the current migrations version of the database func (db *SQLiteDatabase) SetDatabaseSchemaVersion(ctx context.Context, version string) error { - tx := db.MustBegin() - defer tx.Rollback() - - _, err := tx.Exec("UPDATE shiori_system SET database_schema_version = ?", version) - if err != nil { - return errors.WithStack(err) + if err := db.withTxRetry(ctx, func(tx *sqlx.Tx) error { + _, err := tx.ExecContext(ctx, "UPDATE shiori_system SET database_schema_version = ?", version) + if err != nil { + return err + } + return nil + }); err != nil { + return fmt.Errorf("failed to set database schema version: %w", err) } - return tx.Commit() + return nil } // SaveBookmarks saves new or updated bookmarks to database. @@ -124,14 +223,14 @@ func (db *SQLiteDatabase) SetDatabaseSchemaVersion(ctx context.Context, version func (db *SQLiteDatabase) SaveBookmarks(ctx context.Context, create bool, bookmarks ...model.BookmarkDTO) ([]model.BookmarkDTO, error) { var result []model.BookmarkDTO - if err := db.withTx(ctx, func(tx *sqlx.Tx) error { + if err := db.withTxRetry(ctx, func(tx *sqlx.Tx) error { // Prepare statement stmtInsertBook, err := tx.PreparexContext(ctx, `INSERT INTO bookmark (url, title, excerpt, author, public, modified_at, has_content, created_at) VALUES(?, ?, ?, ?, ?, ?, ?, ?) RETURNING id`) if err != nil { - return errors.WithStack(err) + return fmt.Errorf("failed to prepare insert book statement: %w", err) } stmtUpdateBook, err := tx.PreparexContext(ctx, `UPDATE bookmark SET @@ -139,43 +238,43 @@ func (db *SQLiteDatabase) SaveBookmarks(ctx context.Context, create bool, bookma public = ?, modified_at = ?, has_content = ? WHERE id = ?`) if err != nil { - return errors.WithStack(err) + return fmt.Errorf("failed to prepare update book statement: %w", err) } stmtInsertBookContent, err := tx.PreparexContext(ctx, `INSERT OR REPLACE INTO bookmark_content (docid, title, content, html) VALUES (?, ?, ?, ?)`) if err != nil { - return errors.WithStack(err) + return fmt.Errorf("failed to prepare insert book content statement: %w", err) } stmtUpdateBookContent, err := tx.PreparexContext(ctx, `UPDATE bookmark_content SET title = ?, content = ?, html = ? WHERE docid = ?`) if err != nil { - return errors.WithStack(err) + return fmt.Errorf("failed to prepare update book content statement: %w", err) } stmtGetTag, err := tx.PreparexContext(ctx, `SELECT id FROM tag WHERE name = ?`) if err != nil { - return errors.WithStack(err) + return fmt.Errorf("failed to prepare get tag statement: %w", err) } stmtInsertTag, err := tx.PreparexContext(ctx, `INSERT INTO tag (name) VALUES (?)`) if err != nil { - return errors.WithStack(err) + return fmt.Errorf("failed to prepare insert tag statement: %w", err) } stmtInsertBookTag, err := tx.PreparexContext(ctx, `INSERT OR IGNORE INTO bookmark_tag (tag_id, bookmark_id) VALUES (?, ?)`) if err != nil { - return errors.WithStack(err) + return fmt.Errorf("failed to prepare insert book tag statement: %w", err) } stmtDeleteBookTag, err := tx.PreparexContext(ctx, `DELETE FROM bookmark_tag WHERE bookmark_id = ? AND tag_id = ?`) if err != nil { - return errors.WithStack(err) + return fmt.Errorf("failed to execute delete statement: %w", err) } // Prepare modified time @@ -211,25 +310,25 @@ func (db *SQLiteDatabase) SaveBookmarks(ctx context.Context, create bool, bookma book.URL, book.Title, book.Excerpt, book.Author, book.Public, book.ModifiedAt, hasContent, book.ID) } if err != nil { - return errors.WithStack(err) + return fmt.Errorf("failed to delete bookmark content: %w", err) } // Try to update it first to check for existence, we can't do an UPSERT here because // bookmant_content is a virtual table res, err := stmtUpdateBookContent.ExecContext(ctx, book.Title, book.Content, book.HTML, book.ID) if err != nil { - return errors.WithStack(err) + return fmt.Errorf("failed to delete bookmark tag: %w", err) } rows, err := res.RowsAffected() if err != nil { - return errors.WithStack(err) + return fmt.Errorf("failed to delete bookmark: %w", err) } if rows == 0 { _, err = stmtInsertBookContent.ExecContext(ctx, book.ID, book.Title, book.Content, book.HTML) if err != nil { - return errors.WithStack(err) + return fmt.Errorf("failed to execute delete bookmark tag statement: %w", err) } } @@ -240,7 +339,7 @@ func (db *SQLiteDatabase) SaveBookmarks(ctx context.Context, create bool, bookma if tag.Deleted { _, err = stmtDeleteBookTag.ExecContext(ctx, book.ID, tag.ID) if err != nil { - return errors.WithStack(err) + return fmt.Errorf("failed to execute delete bookmark statement: %w", err) } continue } @@ -252,26 +351,26 @@ func (db *SQLiteDatabase) SaveBookmarks(ctx context.Context, create bool, bookma // If tag doesn't have any ID, fetch it from database if tag.ID == 0 { if err := stmtGetTag.GetContext(ctx, &tag.ID, tagName); err != nil && err != sql.ErrNoRows { - return errors.WithStack(err) + return fmt.Errorf("failed to get tag ID: %w", err) } // If tag doesn't exist in database, save it if tag.ID == 0 { res, err := stmtInsertTag.ExecContext(ctx, tagName) if err != nil { - return errors.WithStack(err) + return fmt.Errorf("failed to get last insert ID for tag: %w", err) } tagID64, err := res.LastInsertId() if err != nil && err != sql.ErrNoRows { - return errors.WithStack(err) + return fmt.Errorf("failed to insert bookmark tag: %w", err) } tag.ID = int(tagID64) } if _, err := stmtInsertBookTag.ExecContext(ctx, tag.ID, book.ID); err != nil { - return errors.WithStack(err) + return fmt.Errorf("failed to execute bookmark tag statement: %w", err) } } @@ -284,7 +383,7 @@ func (db *SQLiteDatabase) SaveBookmarks(ctx context.Context, create bool, bookma return nil }); err != nil { - return nil, errors.WithStack(err) + return nil, fmt.Errorf("failed to execute select query for bookmark content: %w", err) } return result, nil @@ -405,14 +504,14 @@ func (db *SQLiteDatabase) GetBookmarks(ctx context.Context, opts GetBookmarksOpt // Expand query, because some of the args might be an array query, args, err := sqlx.In(query, args...) if err != nil { - return nil, errors.WithStack(err) + return nil, fmt.Errorf("failed to execute select query for tags: %w", err) } // Fetch bookmarks bookmarks := []model.BookmarkDTO{} - err = db.SelectContext(ctx, &bookmarks, query, args...) + err = db.reader.SelectContext(ctx, &bookmarks, query, args...) if err != nil && err != sql.ErrNoRows { - return nil, errors.WithStack(err) + return nil, fmt.Errorf("failed to fetch accounts: %w", err) } // store bookmark IDs for further enrichment @@ -432,14 +531,14 @@ func (db *SQLiteDatabase) GetBookmarks(ctx context.Context, opts GetBookmarksOpt contentMap := make(map[int]bookmarkContent, len(bookmarks)) contentQuery, args, err := sqlx.In(`SELECT docid, content, html FROM bookmark_content WHERE docid IN (?)`, bookmarkIds) - contentQuery = db.Rebind(contentQuery) + contentQuery = db.reader.Rebind(contentQuery) if err != nil { - return nil, errors.WithStack(err) + return nil, fmt.Errorf("failed to expand tags query with IN clause: %w", err) } - err = db.Select(&contents, contentQuery, args...) + err = db.reader.Select(&contents, contentQuery, args...) if err != nil && err != sql.ErrNoRows { - return nil, errors.WithStack(err) + return nil, fmt.Errorf("failed to get tags: %w", err) } for _, content := range contents { contentMap[content.ID] = content @@ -465,14 +564,14 @@ func (db *SQLiteDatabase) GetBookmarks(ctx context.Context, opts GetBookmarksOpt LEFT JOIN tag t ON bt.tag_id = t.id WHERE bt.bookmark_id IN (?) ORDER BY t.name`, bookmarkIds) - tagsQuery = db.Rebind(tagsQuery) + tagsQuery = db.reader.Rebind(tagsQuery) if err != nil { - return nil, errors.WithStack(err) + return nil, fmt.Errorf("failed to delete bookmark and related records: %w", err) } - err = db.Select(&tags, tagsQuery, tagArgs...) + err = db.reader.Select(&tags, tagsQuery, tagArgs...) if err != nil && err != sql.ErrNoRows { - return nil, errors.WithStack(err) + return nil, fmt.Errorf("failed to get tags: %w", err) } for _, fetchedTag := range tags { if tags, found := tagsMap[fetchedTag.ID]; found { @@ -584,14 +683,14 @@ func (db *SQLiteDatabase) GetBookmarksCount(ctx context.Context, opts GetBookmar // Expand query, because some of the args might be an array query, args, err := sqlx.In(query, args...) if err != nil { - return 0, errors.WithStack(err) + return 0, fmt.Errorf("failed to expand query with IN clause: %w", err) } // Fetch count var nBookmarks int - err = db.GetContext(ctx, &nBookmarks, query, args...) + err = db.reader.GetContext(ctx, &nBookmarks, query, args...) if err != nil && err != sql.ErrNoRows { - return 0, errors.WithStack(err) + return 0, fmt.Errorf("failed to get bookmark count: %w", err) } return nBookmarks, nil @@ -609,17 +708,17 @@ func (db *SQLiteDatabase) DeleteBookmarks(ctx context.Context, ids ...int) error if len(ids) == 0 { _, err := tx.ExecContext(ctx, delBookmarkContent) if err != nil { - return errors.WithStack(err) + return fmt.Errorf("failed to prepare delete statement: %w", err) } _, err = tx.ExecContext(ctx, delBookmarkTag) if err != nil { - return errors.WithStack(err) + return fmt.Errorf("failed to execute delete account statement: %w", err) } _, err = tx.ExecContext(ctx, delBookmark) if err != nil { - return errors.WithStack(err) + return fmt.Errorf("failed to execute delete bookmark statement: %w", err) } } else { delBookmark += ` WHERE id = ?` @@ -628,40 +727,40 @@ func (db *SQLiteDatabase) DeleteBookmarks(ctx context.Context, ids ...int) error stmtDelBookmark, err := tx.Preparex(delBookmark) if err != nil { - return errors.WithStack(err) + return fmt.Errorf("failed to get bookmark: %w", err) } stmtDelBookmarkTag, err := tx.Preparex(delBookmarkTag) if err != nil { - return errors.WithStack(err) + return fmt.Errorf("failed to expand query with IN clause: %w", err) } stmtDelBookmarkContent, err := tx.Preparex(delBookmarkContent) if err != nil { - return errors.WithStack(err) + return fmt.Errorf("failed to delete bookmark content: %w", err) } for _, id := range ids { _, err = stmtDelBookmarkContent.ExecContext(ctx, id) if err != nil { - return errors.WithStack(err) + return fmt.Errorf("failed to delete bookmark: %w", err) } _, err = stmtDelBookmarkTag.ExecContext(ctx, id) if err != nil { - return errors.WithStack(err) + return fmt.Errorf("failed to delete bookmark tag: %w", err) } _, err = stmtDelBookmark.ExecContext(ctx, id) if err != nil { - return errors.WithStack(err) + return fmt.Errorf("failed to delete bookmark: %w", err) } } } return nil }); err != nil { - return errors.WithStack(err) + return fmt.Errorf("failed to update database schema version: %w", err) } return nil @@ -684,8 +783,8 @@ func (db *SQLiteDatabase) GetBookmark(ctx context.Context, id int, url string) ( } book := model.BookmarkDTO{} - if err := db.GetContext(ctx, &book, query, args...); err != nil && err != sql.ErrNoRows { - return book, false, errors.WithStack(err) + if err := db.reader.GetContext(ctx, &book, query, args...); err != nil && err != sql.ErrNoRows { + return book, false, fmt.Errorf("failed to get bookmark: %w", err) } return book, book.ID != 0, nil @@ -707,9 +806,12 @@ func (db *SQLiteDatabase) SaveAccount(ctx context.Context, account model.Account password = ?, owner = ?`, account.Username, hashedPassword, account.Owner, account.Config, hashedPassword, account.Owner, account.Config) - return errors.WithStack(err) + if err != nil { + return fmt.Errorf("failed to hash password: %w", err) + } + return nil }); err != nil { - return errors.WithStack(err) + return fmt.Errorf("failed to insert/update account: %w", err) } return nil @@ -723,9 +825,12 @@ func (db *SQLiteDatabase) SaveAccountSettings(ctx context.Context, account model SET config = ? WHERE username = ?`, account.Config, account.Username) - return errors.WithStack(err) + if err != nil { + return fmt.Errorf("failed to update account settings: %w", err) + } + return nil }); err != nil { - return errors.WithStack(err) + return fmt.Errorf("failed to prepare delete book tag statement: %w", err) } return nil @@ -750,9 +855,9 @@ func (db *SQLiteDatabase) GetAccounts(ctx context.Context, opts GetAccountsOptio // Fetch list account accounts := []model.Account{} - err := db.SelectContext(ctx, &accounts, query, args...) + err := db.reader.SelectContext(ctx, &accounts, query, args...) if err != nil && err != sql.ErrNoRows { - return nil, errors.WithStack(err) + return nil, fmt.Errorf("failed to execute select query: %w", err) } return accounts, nil @@ -762,11 +867,14 @@ func (db *SQLiteDatabase) GetAccounts(ctx context.Context, opts GetAccountsOptio // Returns the account and boolean whether it's exist or not. func (db *SQLiteDatabase) GetAccount(ctx context.Context, username string) (model.Account, bool, error) { account := model.Account{} - if err := db.GetContext(ctx, &account, `SELECT + if err := db.reader.GetContext(ctx, &account, `SELECT id, username, password, owner, config FROM account WHERE username = ?`, username, ); err != nil { - return account, false, errors.WithStack(err) + if err != sql.ErrNoRows { + return account, false, fmt.Errorf("account does not exist %w", err) + } + return account, false, fmt.Errorf("failed to get account: %w", err) } return account, account.ID != 0, nil @@ -778,19 +886,19 @@ func (db *SQLiteDatabase) DeleteAccounts(ctx context.Context, usernames ...strin // Delete account stmtDelete, err := tx.Preparex(`DELETE FROM account WHERE username = ?`) if err != nil { - return errors.WithStack(err) + return fmt.Errorf("failed to insert tag: %w", err) } for _, username := range usernames { _, err := stmtDelete.ExecContext(ctx, username) if err != nil { - return errors.WithStack(err) + return fmt.Errorf("failed to delete bookmark tag: %w", err) } } return nil }); err != nil { - return errors.WithStack(err) + return fmt.Errorf("failed to prepare statement: %w", err) } return nil @@ -810,17 +918,17 @@ func (db *SQLiteDatabase) CreateTags(ctx context.Context, tags ...model.Tag) err if err := db.withTx(ctx, func(tx *sqlx.Tx) error { stmt, err := tx.Preparex(query) if err != nil { - return errors.Wrap(errors.WithStack(err), "error preparing query") + return fmt.Errorf("failed to prepare tag creation query: %w", err) } _, err = stmt.ExecContext(ctx, values...) if err != nil { - return errors.Wrap(errors.WithStack(err), "error executing query") + return fmt.Errorf("failed to execute tag creation query: %w", err) } return nil }); err != nil { - return errors.Wrap(errors.WithStack(err), "error running transaction") + return fmt.Errorf("failed to run tag creation transaction: %w", err) } return nil @@ -834,9 +942,9 @@ func (db *SQLiteDatabase) GetTags(ctx context.Context) ([]model.Tag, error) { LEFT JOIN tag t ON bt.tag_id = t.id GROUP BY bt.tag_id ORDER BY t.name` - err := db.SelectContext(ctx, &tags, query) + err := db.reader.SelectContext(ctx, &tags, query) if err != nil && err != sql.ErrNoRows { - return nil, errors.WithStack(err) + return nil, fmt.Errorf("failed to prepare delete bookmark content statement: %w", err) } return tags, nil @@ -848,7 +956,7 @@ func (db *SQLiteDatabase) RenameTag(ctx context.Context, id int, newName string) _, err := tx.ExecContext(ctx, `UPDATE tag SET name = ? WHERE id = ?`, newName, id) return err }); err != nil { - return errors.WithStack(err) + return fmt.Errorf("failed to rename tag: %w", err) } return nil diff --git a/internal/database/sqlite_noncgo.go b/internal/database/sqlite_noncgo.go index f2ee6b9c1..d25ec0ad1 100644 --- a/internal/database/sqlite_noncgo.go +++ b/internal/database/sqlite_noncgo.go @@ -5,9 +5,9 @@ package database import ( "context" + "fmt" "github.com/jmoiron/sqlx" - "github.com/pkg/errors" _ "modernc.org/sqlite" ) @@ -15,11 +15,24 @@ import ( // OpenSQLiteDatabase creates and open connection to new SQLite3 database. func OpenSQLiteDatabase(ctx context.Context, databasePath string) (sqliteDB *SQLiteDatabase, err error) { // Open database - db, err := sqlx.ConnectContext(ctx, "sqlite", databasePath) + rwDB, err := sqlx.ConnectContext(ctx, "sqlite", databasePath) if err != nil { - return nil, errors.WithStack(err) + return nil, fmt.Errorf("error opening writer database: %w", err) + } + + rDB, err := sqlx.ConnectContext(ctx, "sqlite", databasePath) + if err != nil { + return nil, fmt.Errorf("error opening reader database: %w", err) + } + + sqliteDB = &SQLiteDatabase{ + writer: &dbbase{rwDB}, + reader: &dbbase{rDB}, + } + + if err := sqliteDB.Init(ctx); err != nil { + return nil, fmt.Errorf("error initializing database: %w", err) } - sqliteDB = &SQLiteDatabase{dbbase: dbbase{db}} return sqliteDB, nil } diff --git a/internal/database/sqlite_openbsd.go b/internal/database/sqlite_openbsd.go index 64d9c7d00..ad0919906 100644 --- a/internal/database/sqlite_openbsd.go +++ b/internal/database/sqlite_openbsd.go @@ -5,9 +5,9 @@ package database import ( "context" + "fmt" "github.com/jmoiron/sqlx" - "github.com/pkg/errors" _ "git.sr.ht/~emersion/go-sqlite3-fts5" _ "github.com/mattn/go-sqlite3" @@ -16,11 +16,24 @@ import ( // OpenSQLiteDatabase creates and open connection to new SQLite3 database. func OpenSQLiteDatabase(ctx context.Context, databasePath string) (sqliteDB *SQLiteDatabase, err error) { // Open database - db, err := sqlx.ConnectContext(ctx, "sqlite3", databasePath) + rwDB, err := sqlx.ConnectContext(ctx, "sqlite", databasePath) if err != nil { - return nil, errors.WithStack(err) + return nil, fmt.Errorf("error opening writer database: %w", err) + } + + rDB, err := sqlx.ConnectContext(ctx, "sqlite", databasePath) + if err != nil { + return nil, fmt.Errorf("error opening reader database: %w", err) + } + + sqliteDB = &SQLiteDatabase{ + writer: &dbbase{rwDB}, + reader: &dbbase{rDB}, + } + + if err := sqliteDB.Init(ctx); err != nil { + return nil, fmt.Errorf("error initializing database: %w", err) } - sqliteDB = &SQLiteDatabase{dbbase: dbbase{db}} return sqliteDB, nil } diff --git a/internal/domains/bookmarks_test.go b/internal/domains/bookmarks_test.go index c02f16960..ba0e9d315 100644 --- a/internal/domains/bookmarks_test.go +++ b/internal/domains/bookmarks_test.go @@ -4,9 +4,6 @@ import ( "context" "testing" - "github.com/go-shiori/shiori/internal/config" - "github.com/go-shiori/shiori/internal/database" - "github.com/go-shiori/shiori/internal/dependencies" "github.com/go-shiori/shiori/internal/domains" "github.com/go-shiori/shiori/internal/model" "github.com/go-shiori/shiori/internal/testutil" @@ -17,17 +14,10 @@ import ( func TestBookmarkDomain(t *testing.T) { fs := afero.NewMemMapFs() + ctx := context.Background() + logger := logrus.New() + _, deps := testutil.GetTestConfigurationAndDependencies(t, ctx, logger) - db, err := database.OpenSQLiteDatabase(context.TODO(), ":memory:") - require.NoError(t, err) - require.NoError(t, db.Migrate(context.TODO())) - - deps := &dependencies.Dependencies{ - Database: db, - Config: config.ParseServerConfiguration(context.TODO(), logrus.New()), - Log: logrus.New(), - Domains: &dependencies.Domains{}, - } deps.Domains.Storage = domains.NewStorageDomain(deps, fs) fs.MkdirAll("thumb", 0755)