diff --git a/backend/internal/bootstrap/actors_bootstrap.go b/backend/internal/bootstrap/actors_bootstrap.go index ba671032..b9b8d116 100644 --- a/backend/internal/bootstrap/actors_bootstrap.go +++ b/backend/internal/bootstrap/actors_bootstrap.go @@ -1,7 +1,7 @@ package bootstrap import ( - "database/sql" + "context" "errors" "fmt" "log/slog" @@ -12,6 +12,7 @@ import ( "github.com/italypaleale/francis/builtin/ratelimit" "github.com/italypaleale/francis/components" "github.com/italypaleale/francis/components/postgres" + "github.com/italypaleale/francis/components/sqlite" "github.com/italypaleale/francis/host/local" "github.com/jackc/pgx/v5/pgxpool" "gorm.io/gorm" @@ -24,7 +25,6 @@ import ( ) type NewActorsOpts struct { - SQLite *sql.DB Postgres *pgxpool.Pool EnvConfig *common.EnvConfigSchema @@ -116,14 +116,6 @@ func (o *NewActorsOpts) getPSK() ([]byte, error) { // It's meant for short-lived contexts such as CLI commands that need to persist actor state (for example, one-time access tokens) without running the full actor host. // The returned host must NOT be Run(): only direct state operations (Get/Set/Delete on state) are supported, and they require the actor state tables to already exist, which is the case whenever the server has run at least once against this database. func NewActorStateStore(o NewActorsOpts) (*local.Host, error) { - if o.Postgres == nil { - sqlDB, err := o.DB.DB() - if err != nil { - return nil, fmt.Errorf("failed to get *sql.DB connection from Gorm: %w", err) - } - o.SQLite = sqlDB - } - providerOpt, err := o.getProviderOption() if err != nil { return nil, err @@ -156,10 +148,10 @@ func ActorsHostHealthCheckDeadline(haEnabled bool) time.Duration { return 90 * time.Second } -// ActorsProviderOptions builds the Francis provider options for the given database handles -// The actor host and the cluster admin must use the same options so they address the same cluster -// This is implemented separately and exported because the import method needs it too -func ActorsProviderOptions(pg *pgxpool.Pool, sqliteDB *sql.DB) (components.ProviderOptions, error) { +// ActorsProviderOptions builds the Francis provider options for the given database +// The actor host, the cluster admin, and the backup provider must all use these so they address the same cluster +// A Postgres deployment passes both handles, since the Gorm one wraps the same pool, and the pool is what the provider takes +func ActorsProviderOptions(db *gorm.DB, pg *pgxpool.Pool) (components.ProviderOptions, error) { // Log each provider operation, such as a lease renewal or an actor lookup, while debugging // The statements those operations run are logged separately, by the instrumentation attached to the connection in ConnectDatabase operationLog := components.OperationLogConfig{ @@ -167,26 +159,66 @@ func ActorsProviderOptions(pg *pgxpool.Pool, sqliteDB *sql.DB) (components.Provi } switch { - case pg != nil && sqliteDB != nil: - return nil, errors.New("cannot have both Postgres and SQLite connections") case pg != nil: return postgres.PostgresProviderOptions{ DB: pg, OperationLog: operationLog, }, nil - case sqliteDB != nil: + case db != nil: + // The SQLite provider takes the raw connection, which only Gorm holds + sqliteDB, err := db.DB() + if err != nil { + return nil, fmt.Errorf("failed to get *sql.DB connection from Gorm: %w", err) + } return local.SQLiteProviderOptions{ DB: sqliteDB, OperationLog: operationLog, }, nil default: - return nil, errors.New("one of Postgres and SQLite must be set") + return nil, errors.New("one of the Postgres pool and the database connection must be set") } } +// NewActorsBackupProvider creates a Francis provider that talks to the same cluster as the actor host, without registering a host or joining the cluster +// It's meant for backing up and restoring the actor host's own data +// The caller owns the returned provider and must Close it: the database connection stays owned by the caller and is not closed. +func NewActorsBackupProvider(ctx context.Context, providerOpts components.ProviderOptions) (components.ActorProvider, error) { + // The health check deadline must match the actor host's, since it decides when a host that stopped health-checking is considered gone, and a restore refuses to run while any host is still connected + // The remaining values are irrelevant here, because this provider never registers a host nor processes alarms + cfg := components.NewProviderConfig() + cfg.HostHealthCheckDeadline = ActorsHostHealthCheckDeadline(common.EnvConfig.HAEnabled) + + log := slog.Default().With("scope", "actors-backup") + + var ( + provider components.ActorProvider + err error + ) + switch v := providerOpts.(type) { + case postgres.PostgresProviderOptions: + provider, err = postgres.NewPostgresProvider(log, v, cfg) + case sqlite.SQLiteProviderOptions: + provider, err = sqlite.NewSQLiteProvider(log, v, cfg) + default: + err = fmt.Errorf("unsupported provider options type: %T", providerOpts) + } + if err != nil { + return nil, fmt.Errorf("failed to create actor provider: %w", err) + } + + // Init applies the provider's schema migrations, so this also works against a database the actor host has never run against + err = provider.Init(ctx) + if err != nil { + _ = provider.Close() + return nil, fmt.Errorf("failed to initialize actor provider: %w", err) + } + + return provider, nil +} + // getProviderOption wraps the shared provider options in the host option the local host expects func (o *NewActorsOpts) getProviderOption() (local.HostOption, error) { - providerOpts, err := ActorsProviderOptions(o.Postgres, o.SQLite) + providerOpts, err := ActorsProviderOptions(o.DB, o.Postgres) if err != nil { return nil, err } diff --git a/backend/internal/bootstrap/actors_bootstrap_test.go b/backend/internal/bootstrap/actors_bootstrap_test.go index 5b59a78a..4d4c6bc8 100644 --- a/backend/internal/bootstrap/actors_bootstrap_test.go +++ b/backend/internal/bootstrap/actors_bootstrap_test.go @@ -1,10 +1,14 @@ package bootstrap import ( + "bytes" "encoding/hex" + "path/filepath" "testing" + "github.com/libtnb/sqlite" "github.com/stretchr/testify/require" + "gorm.io/gorm" "github.com/pocket-id/pocket-id/backend/internal/common" ) @@ -26,3 +30,43 @@ func TestNewActorsOptsGetPSKUsesStableValue(t *testing.T) { require.NoError(t, err) require.Equalf(t, expected, actual, "actual result: %s", actual) } + +// TestNewActorsBackupProvider covers the provider the export and import use to back up and restore the actor host's data. +// It builds the provider from the same options the actor host uses, so a mismatch between those options and the concrete provider would otherwise only surface at runtime, when an export or import is attempted. +func TestNewActorsBackupProvider(t *testing.T) { + // Foreign keys must be enabled, which the provider validates on init and which the application enables on every connection + dbPath := filepath.Join(t.TempDir(), "pocket-id.db") + dsn := "file:" + dbPath + "?_txlock=immediate&_pragma=busy_timeout(2500)&_pragma=journal_mode(WAL)&_pragma=foreign_keys(1)" + db, err := gorm.Open(sqlite.Open(dsn), &gorm.Config{}) + require.NoError(t, err) + t.Cleanup(func() { + sqlDB, dbErr := db.DB() + if dbErr == nil { + _ = sqlDB.Close() + } + }) + + providerOpts, err := ActorsProviderOptions(db, nil) + require.NoError(t, err) + + provider, err := NewActorsBackupProvider(t.Context(), providerOpts) + require.NoError(t, err) + t.Cleanup(func() { + _ = provider.Close() + }) + + // Init applies the actor host's schema migrations, so an export also works against a database the actor host has never run against + var tables int64 + err = db.Raw(`SELECT count(*) FROM sqlite_master WHERE type = 'table' AND name LIKE 'francis_%'`).Scan(&tables).Error + require.NoError(t, err) + require.Positive(t, tables, "the provider must create the actor host's own tables") + + // Even an empty cluster produces a valid backup stream, which an import can restore + buf := &bytes.Buffer{} + err = provider.Backup(t.Context(), buf) + require.NoError(t, err) + require.Contains(t, buf.String(), "francis-backup") + + err = provider.Restore(t.Context(), bytes.NewReader(buf.Bytes())) + require.NoError(t, err) +} diff --git a/backend/internal/bootstrap/bootstrap.go b/backend/internal/bootstrap/bootstrap.go index 6581dc04..9753916e 100644 --- a/backend/internal/bootstrap/bootstrap.go +++ b/backend/internal/bootstrap/bootstrap.go @@ -92,12 +92,6 @@ func Bootstrap(ctx context.Context) error { DB: db, FileStorage: fileStorage, } - if pg == nil { - actorsOpts.SQLite, err = db.DB() - if err != nil { - return fmt.Errorf("failed to get *sql.DB connection from Gorm: %w", err) - } - } actors, rateLimitServices, err := NewActors(actorsOpts) if err != nil { return fmt.Errorf("failed to initialize actors: %w", err) diff --git a/backend/internal/cmds/export.go b/backend/internal/cmds/export.go index 853f7757..8e21d1e3 100644 --- a/backend/internal/cmds/export.go +++ b/backend/internal/cmds/export.go @@ -6,9 +6,10 @@ import ( "io" "os" + "github.com/spf13/cobra" + "github.com/pocket-id/pocket-id/backend/internal/bootstrap" "github.com/pocket-id/pocket-id/backend/internal/service" - "github.com/spf13/cobra" ) type exportFlags struct { @@ -33,7 +34,7 @@ func init() { // runExport orchestrates the export flow func runExport(ctx context.Context, flags exportFlags) error { - db, _, err := bootstrap.NewDatabase(ctx) + db, pg, err := bootstrap.NewDatabase(ctx) if err != nil { return fmt.Errorf("failed to connect to database: %w", err) } @@ -48,7 +49,20 @@ func runExport(ctx context.Context, flags exportFlags) error { _ = storage.Close() }() - exportService := service.NewExportService(db, storage) + // The actor host's data lives outside of the Pocket ID schema, so it's exported through Francis + providerOpts, err := bootstrap.ActorsProviderOptions(db, pg) + if err != nil { + return err + } + actorsProvider, err := bootstrap.NewActorsBackupProvider(ctx, providerOpts) + if err != nil { + return fmt.Errorf("failed to initialize the actor host's data provider: %w", err) + } + defer func() { + _ = actorsProvider.Close() + }() + + exportService := service.NewExportService(db, storage, actorsProvider) var w io.Writer if flags.Path == "-" { diff --git a/backend/internal/cmds/import.go b/backend/internal/cmds/import.go index 5cd5f1ff..27957348 100644 --- a/backend/internal/cmds/import.go +++ b/backend/internal/cmds/import.go @@ -3,7 +3,6 @@ package cmds import ( "archive/zip" "context" - "database/sql" "errors" "fmt" "io" @@ -83,14 +82,7 @@ func runImport(ctx context.Context, flags importFlags) error { } // The cluster admin talks to the same database as the actor host, so build its provider options the same way the host does - var sqliteDB *sql.DB - if pg == nil { - sqliteDB, err = db.DB() - if err != nil { - return fmt.Errorf("failed to get sql.DB connection: %w", err) - } - } - providerOpts, err := bootstrap.ActorsProviderOptions(pg, sqliteDB) + providerOpts, err := bootstrap.ActorsProviderOptions(db, pg) if err != nil { return err } @@ -124,8 +116,18 @@ func runImport(ctx context.Context, flags importFlags) error { _ = storage.Close() }() + // The actor host's data lives outside of the Pocket ID schema, so it's restored through Francis + // Restoring requires exclusive access to the cluster, which was acquired above + actorsProvider, err := bootstrap.NewActorsBackupProvider(importCtx, providerOpts) + if err != nil { + return fmt.Errorf("failed to initialize the actor host's data provider: %w", err) + } + defer func() { + _ = actorsProvider.Close() + }() + // Create the import service - importService := service.NewImportService(db, storage) + importService := service.NewImportService(db, storage, actorsProvider) // Load from ZIP err = importService.ImportFromZip(importCtx, &zipReader.Reader) diff --git a/backend/internal/service/actors_backup.go b/backend/internal/service/actors_backup.go new file mode 100644 index 00000000..53fb4eca --- /dev/null +++ b/backend/internal/service/actors_backup.go @@ -0,0 +1,21 @@ +package service + +import ( + "context" + "io" +) + +// actorsBackupFileName is the name of the entry in the export ZIP that contains the actor host's data +// The payload is Francis' own backup stream, which is a binary, versioned, provider-neutral format +const actorsBackupFileName = "francis.bin" + +// ActorsBackupProvider backs up and restores the actor host's data: actor state, alarms, and dead-lettered jobs. +type ActorsBackupProvider interface { + // Backup writes a snapshot of all the actor host's persistent data to w. + // It runs against a consistent snapshot without blocking writers, so it's safe to take while Pocket ID is running. + Backup(ctx context.Context, w io.Writer) error + + // Restore wipes all the actor host's persistent data and loads a snapshot produced by Backup from r. + // It fails if any host is currently connected, since restoring underneath a running Pocket ID instance would corrupt active actors. + Restore(ctx context.Context, r io.Reader) error +} diff --git a/backend/internal/service/actors_backup_test.go b/backend/internal/service/actors_backup_test.go new file mode 100644 index 00000000..c57f4943 --- /dev/null +++ b/backend/internal/service/actors_backup_test.go @@ -0,0 +1,147 @@ +package service + +import ( + "archive/zip" + "bytes" + "context" + "errors" + "io" + "testing" + + "github.com/stretchr/testify/require" +) + +// stubActorsBackupProvider stands in for the Francis provider and records what the export and import did with it +type stubActorsBackupProvider struct { + backupData []byte + backupErr error + + restored []byte + restoreCalls int + restoreErr error +} + +func (s *stubActorsBackupProvider) Backup(_ context.Context, w io.Writer) error { + if s.backupErr != nil { + return s.backupErr + } + _, err := w.Write(s.backupData) + return err +} + +func (s *stubActorsBackupProvider) Restore(_ context.Context, r io.Reader) error { + s.restoreCalls++ + if s.restoreErr != nil { + return s.restoreErr + } + var err error + s.restored, err = io.ReadAll(r) + return err +} + +func TestExportActorsBackup(t *testing.T) { + t.Run("adds the actor host's data to the archive", func(t *testing.T) { + actors := &stubActorsBackupProvider{backupData: []byte("francis-backup-payload")} + + files := writeActorsBackupZip(t, NewExportService(nil, nil, actors)) + + require.Equal(t, map[string][]byte{actorsBackupFileName: actors.backupData}, files) + }) + + t.Run("adds nothing without a provider", func(t *testing.T) { + files := writeActorsBackupZip(t, NewExportService(nil, nil, nil)) + + require.Empty(t, files) + }) + + t.Run("surfaces backup errors", func(t *testing.T) { + actors := &stubActorsBackupProvider{backupErr: errors.New("provider is unhappy")} + + zw := zip.NewWriter(&bytes.Buffer{}) + err := NewExportService(nil, nil, actors).addActorsBackupToZip(t.Context(), zw) + + require.ErrorContains(t, err, "provider is unhappy") + }) +} + +func TestImportActorsBackup(t *testing.T) { + t.Run("restores the actor host's data from the archive", func(t *testing.T) { + actors := &stubActorsBackupProvider{} + files := readZip(t, buildZip(t, map[string][]byte{ + "database.json": []byte("{}"), + actorsBackupFileName: []byte("francis-backup-payload"), + })) + + err := NewImportService(nil, nil, actors).importActorsBackup(t.Context(), files) + + require.NoError(t, err) + require.Equal(t, 1, actors.restoreCalls) + require.Equal(t, []byte("francis-backup-payload"), actors.restored) + }) + + // Archives created before Pocket ID exported the actor host's data must still import, leaving the existing actor data alone rather than wiping it + t.Run("leaves the actor host's data alone when the archive has none", func(t *testing.T) { + actors := &stubActorsBackupProvider{} + files := readZip(t, buildZip(t, map[string][]byte{"database.json": []byte("{}")})) + + err := NewImportService(nil, nil, actors).importActorsBackup(t.Context(), files) + + require.NoError(t, err) + require.Zero(t, actors.restoreCalls) + }) + + t.Run("surfaces restore errors", func(t *testing.T) { + actors := &stubActorsBackupProvider{restoreErr: errors.New("a host is still connected")} + files := readZip(t, buildZip(t, map[string][]byte{actorsBackupFileName: []byte("francis-backup-payload")})) + + err := NewImportService(nil, nil, actors).importActorsBackup(t.Context(), files) + + require.ErrorContains(t, err, "a host is still connected") + }) +} + +// writeActorsBackupZip runs the export's actor-host backup step into an archive and returns its contents +func writeActorsBackupZip(t *testing.T, s *ExportService) map[string][]byte { + t.Helper() + + buf := &bytes.Buffer{} + zw := zip.NewWriter(buf) + require.NoError(t, s.addActorsBackupToZip(t.Context(), zw)) + require.NoError(t, zw.Close()) + + res := make(map[string][]byte) + for _, f := range readZip(t, buf.Bytes()) { + rc, err := f.Open() + require.NoError(t, err) + data, err := io.ReadAll(rc) + rc.Close() + require.NoError(t, err) + res[f.Name] = data + } + return res +} + +func buildZip(t *testing.T, files map[string][]byte) []byte { + t.Helper() + + buf := &bytes.Buffer{} + zw := zip.NewWriter(buf) + for name, data := range files { + w, err := zw.Create(name) + require.NoError(t, err) + _, err = w.Write(data) + require.NoError(t, err) + } + require.NoError(t, zw.Close()) + + return buf.Bytes() +} + +func readZip(t *testing.T, data []byte) []*zip.File { + t.Helper() + + zr, err := zip.NewReader(bytes.NewReader(data), int64(len(data))) + require.NoError(t, err) + + return zr.File +} diff --git a/backend/internal/service/export_service.go b/backend/internal/service/export_service.go index 3b7539f7..9ba4793c 100644 --- a/backend/internal/service/export_service.go +++ b/backend/internal/service/export_service.go @@ -3,6 +3,7 @@ package service import ( "archive/zip" "context" + "database/sql" "encoding/json" "fmt" "io" @@ -20,18 +21,23 @@ import ( type ExportService struct { db *gorm.DB storage storage.FileStorage + actors ActorsBackupProvider } -func NewExportService(db *gorm.DB, storage storage.FileStorage) *ExportService { +// NewExportService creates a new ExportService. +// +// actors is used to back up the actor host's data, which is stored outside of the Pocket ID schema; when nil, the export does not include it. +func NewExportService(db *gorm.DB, storage storage.FileStorage, actors ActorsBackupProvider) *ExportService { return &ExportService{ db: db, storage: storage, + actors: actors, } } // ExportToZip performs the full export process and writes the ZIP data to the given writer. func (s *ExportService) ExportToZip(ctx context.Context, w io.Writer) error { - dbData, err := s.extractDatabase() + dbData, err := s.extractDatabase(ctx) if err != nil { return err } @@ -40,19 +46,52 @@ func (s *ExportService) ExportToZip(ctx context.Context, w io.Writer) error { } // extractDatabase reads all tables into a DatabaseExport struct -func (s *ExportService) extractDatabase() (DatabaseExport, error) { - schema, err := utils.LoadDBSchemaTypes(s.db) +// +// Every table is read inside a single read transaction, so the dump is a consistent snapshot of one point in time +func (s *ExportService) extractDatabase(ctx context.Context) (out DatabaseExport, err error) { + err = s.db. + WithContext(ctx). + Transaction(func(tx *gorm.DB) error { + out, err = extractDatabaseTx(tx) + return err + }, snapshotTxOptions(s.db.Name())) + if err != nil { + return DatabaseExport{}, err + } + + return out, nil +} + +// snapshotTxOptions returns the transaction options that give a read a consistent snapshot on the given database provider, without blocking concurrent writers +func snapshotTxOptions(provider string) *sql.TxOptions { + opts := &sql.TxOptions{ + ReadOnly: true, + } + + // Postgres defaults to read committed, which takes a new snapshot for every statement, so tables read later in the export would include writes that landed after it started + // Repeatable read instead pins a single snapshot for the whole transaction + // SQLite is left at the driver's default: in WAL mode (the one used by Pocket ID) a read transaction already sees one consistent snapshot, and the driver rejects nothing but also honors no other isolation level + if provider == "postgres" { + opts.Isolation = sql.LevelRepeatableRead + } + + return opts +} + +// extractDatabaseTx dumps every exported table using the given transaction +func extractDatabaseTx(tx *gorm.DB) (DatabaseExport, error) { + schema, err := utils.LoadDBSchemaTypes(tx) if err != nil { return DatabaseExport{}, fmt.Errorf("failed to load schema types: %w", err) } - version, err := s.schemaVersion() + version, err := schemaVersion(tx) if err != nil { return DatabaseExport{}, err } out := DatabaseExport{ - Provider: s.db.Name(), + Provider: tx.Name(), Version: version, Tables: map[string][]map[string]any{}, // These tables need to be inserted in a specific order because of foreign key constraints @@ -65,7 +104,7 @@ func (s *ExportService) extractDatabase() (DatabaseExport, error) { if table == "storage" || table == "schema_migrations" || strings.HasPrefix(table, "francis_") { continue } - err = s.dumpTable(table, schema[table], &out) + err = dumpTable(tx, table, schema[table], &out) if err != nil { return DatabaseExport{}, err } @@ -74,9 +113,9 @@ func (s *ExportService) extractDatabase() (DatabaseExport, error) { return out, nil } -func (s *ExportService) schemaVersion() (uint, error) { +func schemaVersion(tx *gorm.DB) (uint, error) { var version uint - err := s.db.Raw("SELECT version FROM schema_migrations").Row().Scan(&version) + err := tx.Raw("SELECT version FROM schema_migrations").Row().Scan(&version) if err != nil { return 0, fmt.Errorf("failed to query schema version: %w", err) } @@ -84,8 +123,8 @@ func (s *ExportService) schemaVersion() (uint, error) { } // dumpTable selects all rows from a table and appends them to out.Tables -func (s *ExportService) dumpTable(table string, types utils.DBSchemaTableTypes, out *DatabaseExport) error { - rows, err := s.db.Raw("SELECT * FROM " + table).Rows() +func dumpTable(tx *gorm.DB, table string, types utils.DBSchemaTableTypes, out *DatabaseExport) error { + rows, err := tx.Raw("SELECT * FROM " + table).Rows() if err != nil { return fmt.Errorf("failed to read table %s: %w", table, err) } @@ -98,7 +137,7 @@ func (s *ExportService) dumpTable(table string, types utils.DBSchemaTableTypes, } for rows.Next() { - vals := s.getScanValuesForTable(cols, types) + vals := getScanValuesForTable(cols, types) err = rows.Scan(vals...) if err != nil { return fmt.Errorf("failed to scan row in table %s: %w", table, err) @@ -115,7 +154,7 @@ func (s *ExportService) dumpTable(table string, types utils.DBSchemaTableTypes, return rows.Err() } -func (s *ExportService) getScanValuesForTable(cols []string, types utils.DBSchemaTableTypes) []any { +func getScanValuesForTable(cols []string, types utils.DBSchemaTableTypes) []any { res := make([]any, len(cols)) for i, col := range cols { // Store a pointer @@ -182,6 +221,12 @@ func (s *ExportService) writeExportZipStream(ctx context.Context, w io.Writer, d return fmt.Errorf("failed to encode database.json: %w", err) } + // Add the actor host's data + err = s.addActorsBackupToZip(ctx, zipWriter) + if err != nil { + return fmt.Errorf("error adding the actor host's data to the export zip: %w", err) + } + // Add uploaded files err = s.addUploadsToZip(ctx, zipWriter) if err != nil { @@ -196,6 +241,26 @@ func (s *ExportService) writeExportZipStream(ctx context.Context, w io.Writer, d return nil } +// addActorsBackupToZip adds the actor host's data (actor state, alarms, and dead-lettered jobs) to the ZIP archive as a Francis backup stream +// That data lives in the actor host's own tables, so it's exported through Francis' own portable backup format instead +func (s *ExportService) addActorsBackupToZip(ctx context.Context, zipWriter *zip.Writer) error { + if s.actors == nil { + return nil + } + + w, err := zipWriter.Create(actorsBackupFileName) + if err != nil { + return fmt.Errorf("failed to create %s in zip: %w", actorsBackupFileName, err) + } + + err = s.actors.Backup(ctx, w) + if err != nil { + return fmt.Errorf("failed to back up the actor host's data: %w", err) + } + + return nil +} + // addUploadsToZip adds all files from the storage to the ZIP archive under the "uploads/" directory func (s *ExportService) addUploadsToZip(ctx context.Context, zipWriter *zip.Writer) error { return s.storage.Walk(ctx, "/", func(p storage.ObjectInfo) error { diff --git a/backend/internal/service/export_service_test.go b/backend/internal/service/export_service_test.go index 5ef2f0af..58be88a3 100644 --- a/backend/internal/service/export_service_test.go +++ b/backend/internal/service/export_service_test.go @@ -1,11 +1,13 @@ package service import ( + "database/sql" "path/filepath" "strings" "testing" "github.com/stretchr/testify/require" + "gorm.io/gorm" "github.com/pocket-id/pocket-id/backend/internal/utils" ) @@ -21,10 +23,79 @@ func TestExportExcludesActorHostTables(t *testing.T) { require.NoError(t, utils.MigrateDatabase(t.Context(), sqlDB)) seedActorHostSchema(t, db) // creates francis_active_actors (with a row) and a view over it - export, err := NewExportService(db, nil).extractDatabase() + export, err := NewExportService(db, nil, nil).extractDatabase(t.Context()) require.NoError(t, err) for table := range export.Tables { require.Falsef(t, strings.HasPrefix(table, "francis_"), "export must not include actor host table %q", table) } } + +func TestExportSnapshotTxOptions(t *testing.T) { + t.Run("postgres pins one snapshot for the whole transaction", func(t *testing.T) { + require.Equal(t, &sql.TxOptions{Isolation: sql.LevelRepeatableRead, ReadOnly: true}, snapshotTxOptions("postgres")) + }) + + // SQLite must be left at the driver's default isolation, and read-only, or the driver would honor the DSN's "_txlock=immediate" and take a write lock for the whole export + t.Run("sqlite reads without taking a write lock", func(t *testing.T) { + require.Equal(t, &sql.TxOptions{ReadOnly: true}, snapshotTxOptions("sqlite")) + }) +} + +// TestExportRunsWhileAnotherConnectionWrites covers an export taken while Pocket ID is running and writing to the database. +// The DSN sets "_txlock=immediate", so an export that did not open its transaction read-only would take a write lock, block on the concurrent writer, and fail once the busy timeout expired. +func TestExportRunsWhileAnotherConnectionWrites(t *testing.T) { + db := newExportTestDB(t) + + require.NoError(t, db.Exec(`INSERT INTO kv ("key", "value") VALUES ('committed', 'v')`).Error) + + // Hold the write lock on another connection for the duration of the export + writer := db.Begin() + require.NoError(t, writer.Error) + defer writer.Rollback() + require.NoError(t, writer.Exec(`INSERT INTO kv ("key", "value") VALUES ('uncommitted', 'v')`).Error) + + export, err := NewExportService(db, nil, nil).extractDatabase(t.Context()) + require.NoError(t, err) + + // Only the committed row belongs in the export + require.Len(t, export.Tables["kv"], 1) + require.Equal(t, "committed", *(export.Tables["kv"][0]["key"].(*string))) +} + +// TestExportSnapshotIgnoresWritesDuringTheExport covers a write that is committed after the export has started. +// Each table is read with its own query, so without a single snapshot a row inserted midway would land in some tables but not others. +func TestExportSnapshotIgnoresWritesDuringTheExport(t *testing.T) { + db := newExportTestDB(t) + + tx := db.Begin(snapshotTxOptions(db.Name())) + require.NoError(t, tx.Error) + defer tx.Rollback() + + // A deferred read transaction takes its snapshot at the first read, which is what the export does when it loads the schema + var before int64 + require.NoError(t, tx.Raw(`SELECT count(*) FROM kv`).Scan(&before).Error) + + // A committed write on another connection must neither be blocked nor visible to the open snapshot + require.NoError(t, db.Exec(`INSERT INTO kv ("key", "value") VALUES ('added-mid-export', 'v')`).Error) + + var after int64 + require.NoError(t, tx.Raw(`SELECT count(*) FROM kv`).Scan(&after).Error) + require.Equal(t, before, after, "the export's snapshot must not include rows written after it started") +} + +// newExportTestDB opens a migrated SQLite database using the same DSN the application uses, on a file so the pool can hand out more than one connection +func newExportTestDB(t *testing.T) *gorm.DB { + t.Helper() + + db := openImportTestDB(t, filepath.Join(t.TempDir(), "pocket-id.db"), nil) + t.Cleanup(func() { + closeImportTestDB(db) + }) + + sqlDB, err := db.DB() + require.NoError(t, err) + require.NoError(t, utils.MigrateDatabase(t.Context(), sqlDB)) + + return db +} diff --git a/backend/internal/service/import_service.go b/backend/internal/service/import_service.go index 857c09cb..65f9597d 100644 --- a/backend/internal/service/import_service.go +++ b/backend/internal/service/import_service.go @@ -10,6 +10,7 @@ import ( "errors" "fmt" "io" + "log/slog" "slices" "strings" @@ -24,6 +25,7 @@ import ( type ImportService struct { db *gorm.DB storage storage.FileStorage + actors ActorsBackupProvider } type DatabaseExport struct { @@ -33,10 +35,14 @@ type DatabaseExport struct { TableOrder []string `json:"tableOrder"` } -func NewImportService(db *gorm.DB, storage storage.FileStorage) *ImportService { +// NewImportService creates a new ImportService. +// +// actors is used to restore the actor host's data, which is stored outside of the Pocket ID schema - when nil, the actor host's existing data is left untouched. +func NewImportService(db *gorm.DB, storage storage.FileStorage, actors ActorsBackupProvider) *ImportService { return &ImportService{ db: db, storage: storage, + actors: actors, } } @@ -47,6 +53,13 @@ func (s *ImportService) ImportFromZip(ctx context.Context, r *zip.Reader) error return err } + // The actor host's data is restored first because it's an atomic operation that validates its own preconditions, most importantly that no Pocket ID instance is still running + // Restoring it before the Pocket ID schema is dropped means such a failure leaves the existing data untouched + err = s.importActorsBackup(ctx, r.File) + if err != nil { + return err + } + err = s.ImportDatabase(ctx, dbData) if err != nil { return err @@ -60,6 +73,39 @@ func (s *ImportService) ImportFromZip(ctx context.Context, r *zip.Reader) error return nil } +// importActorsBackup restores the actor host's data (actor state, alarms, and dead-lettered jobs) from the Francis backup stream in the ZIP archive. +func (s *ImportService) importActorsBackup(ctx context.Context, files []*zip.File) error { + if s.actors == nil { + return nil + } + + var backupFile *zip.File + for _, f := range files { + if f.Name == actorsBackupFileName { + backupFile = f + break + } + } + if backupFile == nil { + // Archives exported before Pocket ID included the actor host's data don't have that entry, in which case the existing data is left untouched + slog.WarnContext(ctx, "The archive does not contain the actor host's data, which will be left unchanged", slog.String("file", actorsBackupFileName)) + return nil + } + + rc, err := backupFile.Open() + if err != nil { + return fmt.Errorf("failed to open %s: %w", actorsBackupFileName, err) + } + defer rc.Close() + + err = s.actors.Restore(ctx, rc) + if err != nil { + return fmt.Errorf("failed to restore the actor host's data: %w", err) + } + + return nil +} + // ImportDatabase only imports the database data from the given DatabaseExport struct. func (s *ImportService) ImportDatabase(ctx context.Context, dbData DatabaseExport) error { err := s.resetSchema(ctx, dbData.Version) diff --git a/backend/internal/service/import_service_test.go b/backend/internal/service/import_service_test.go index b793bafc..b92aced9 100644 --- a/backend/internal/service/import_service_test.go +++ b/backend/internal/service/import_service_test.go @@ -82,7 +82,7 @@ func importAndRestart(importCfg func(*sql.DB)) func(t *testing.T) { // Import: reset the schema (drop + migrate) and load data. Empty tables keep the test focused // on the schema reset. imp := openImportTestDB(t, dbPath, importCfg) - err = NewImportService(imp, nil).ImportDatabase(t.Context(), DatabaseExport{ + err = NewImportService(imp, nil, nil).ImportDatabase(t.Context(), DatabaseExport{ Provider: "sqlite", Version: importResetTargetVersion, Tables: map[string][]map[string]any{}, @@ -107,7 +107,8 @@ func TestImportResetSchema(t *testing.T) { // TestImportResetSchemaFreshConnections guards the schema reset against connection pooling. // resetSchema must drop tables with foreign keys disabled on the connection that performs the drops. -// With MaxIdleConns(0) every statement runs on a fresh connection (which the DSN opens with foreign_keys(1)); before dropPocketIDTablesSQLite this reproduced the CI failure where DROP TABLE tripped foreign-key cascades/triggers and left the database dirty, breaking the container restart. +// With MaxIdleConns(0) every statement runs on a fresh connection (which the DSN opens with foreign_keys(1)) +// Before dropPocketIDTablesSQLite this reproduced the CI failure where DROP TABLE tripped foreign-key cascades/triggers and left the database dirty, breaking the container restart. func TestImportResetSchemaFreshConnections(t *testing.T) { importAndRestart(func(db *sql.DB) { db.SetMaxIdleConns(0) })(t) } diff --git a/tests/resources/export/francis.bin b/tests/resources/export/francis.bin new file mode 100644 index 00000000..5894a15c Binary files /dev/null and b/tests/resources/export/francis.bin differ diff --git a/tests/specs/cli.spec.ts b/tests/specs/cli.spec.ts index 5a730af9..12152a9b 100644 --- a/tests/specs/cli.spec.ts +++ b/tests/specs/cli.spec.ts @@ -133,6 +133,20 @@ function compareExports(dir1: string, dir2: string): void { const normalizedExpected = normalizeJSON(expectedData); const normalizedActual = normalizeJSON(actualData); expect(normalizedActual).toEqual(normalizedExpected); + + // Compare francis.bin contents + const file1 = path.join(dir1, 'francis.bin'); + const file2 = path.join(dir2, 'francis.bin'); + + for (const filePath of [file1, file2]) { + expect(fs.existsSync(filePath), `${filePath} should exist`).toBe(true); + + const header = fs.readFileSync(filePath).subarray(0, 64).toString('latin1'); + expect(header).toContain('francis-backup'); + + const fileSize = fs.statSync(filePath).size; + expect(fileSize).toBeGreaterThan(64); + } } function archiveExampleExport(outputPath: string): Buffer { @@ -154,6 +168,7 @@ function archiveExampleExport(outputPath: string): Buffer { return buffer; } + // Helper to load JSON files function loadJSON(path: string) { return JSON.parse(fs.readFileSync(path, 'utf-8')); @@ -315,7 +330,8 @@ function hashFile(filePath: string): string { function getAllFiles(dir: string, root = dir): string[] { return fs.readdirSync(dir).flatMap((entry) => { - if (['.DS_Store', 'database.json'].includes(entry)) return []; + // The actor host's data is not part of the example export and its contents differ between runs, so it is checked separately + if (['.DS_Store', 'database.json', 'francis.bin'].includes(entry)) return []; const fullPath = path.join(dir, entry); const stat = fs.statSync(fullPath);