diff --git a/docs/persisted-review.md b/docs/persisted-review.md index b4da8f2..b5950fe 100644 --- a/docs/persisted-review.md +++ b/docs/persisted-review.md @@ -58,8 +58,24 @@ when requested; the migration does not backfill an invented fingerprint onto its old choices. Malformed or stale snapshots are likewise rebuilt. Only the question snapshot is discarded, and database read/delete failures remain errors. +Version 0.1.2 does not supply a downgrade callback. With the pinned sqflite +dependency, opening a schema 4 database with that old app lowers `user_version` +to 3 while leaving the fingerprint column in place. Upgrading again recognizes +that existing column if it is nullable `TEXT`, is not a primary key, and has no +default. It preserves its values and all earned progress; NULL snapshots newly +created by the old app are rebuilt normally. A conflicting column shape fails +the migration instead of silently replacing it or marking it upgraded. + +Starting with 0.1.3, opening a database whose version is newer than the app +supports is explicitly rejected without lowering its version or deleting data. +This cannot change how already released older apps behave. The tested 4 to 3 to +4 recovery is not general downgrade support, and does not make an older app's +answer validation safe. Keep backups before changing app versions and use a +compatible app to open a newer database. + `test/persisted_review_content_test.dart` uses fresh file-backed SQLite databases, -closes and reopens them, exercises the schema upgrade, injects one-shot read and +closes and reopens them, exercises schema upgrades and the legacy downgrade open, +rejects incompatible column shapes and future versions, injects one-shot read and retirement failures, interrupts an attempt write with a SQLite abort trigger, and holds candidate loading across an import. Widget regressions cover updating a visible question and retrying after a preparation failure. diff --git a/lib/services/learning_repository.dart b/lib/services/learning_repository.dart index 45e74b9..1ed2072 100644 --- a/lib/services/learning_repository.dart +++ b/lib/services/learning_repository.dart @@ -248,6 +248,7 @@ class LearningRepository { version: _databaseVersion, onCreate: createSchema, onUpgrade: upgradeSchema, + onDowngrade: rejectSchemaDowngrade, onOpen: (db) async => db.execute('PRAGMA foreign_keys = ON'), ); return _database!; @@ -340,12 +341,38 @@ class LearningRepository { if (oldVersion < 2) await _createSyncStateTable(db); if (oldVersion < 3) { await _createReviewQuestionsTable(db); - } else if (oldVersion < 4) { - // NULL identifies a legacy snapshot. Its original content cannot be - // inferred from the current dictionary; rebuild it when next requested. - await db.execute( - 'ALTER TABLE review_questions ADD COLUMN content_fingerprint TEXT'); } + if (oldVersion < 4) { + final columns = await db.rawQuery('PRAGMA table_info(review_questions)'); + final existing = columns.where((column) => + (column['name'] as String).toLowerCase() == 'content_fingerprint'); + if (existing.isNotEmpty) { + // v0.1.2 opens with version 3 and no onDowngrade handler. sqflite can + // lower user_version without removing this column. Accept that known + // shape on re-upgrade, preserving both bound and legacy NULL snapshots. + final column = existing.single; + if ((column['type'] as String).trim().toUpperCase() != 'TEXT' || + column['notnull'] != 0 || + column['pk'] != 0 || + column['dflt_value'] != null) { + throw StateError('Incompatible review question fingerprint column.'); + } + } else { + // NULL identifies a legacy snapshot. Its original content cannot be + // inferred from the current dictionary; rebuild it when next requested. + await db.execute( + 'ALTER TABLE review_questions ADD COLUMN content_fingerprint TEXT'); + } + } + } + + @visibleForTesting + static Future rejectSchemaDowngrade( + Database db, int oldVersion, int newVersion) async { + // Do not silently relabel, delete, or reinterpret a future database. + throw StateError( + 'Learning database version $oldVersion requires a newer app ' + '(this app supports version $newVersion).'); } static Future _createReviewQuestionsTable(DatabaseExecutor db) => diff --git a/test/persisted_review_content_test.dart b/test/persisted_review_content_test.dart index fd47cc8..a4ef337 100644 --- a/test/persisted_review_content_test.dart +++ b/test/persisted_review_content_test.dart @@ -308,6 +308,118 @@ void main() { await _answer(disk.repo, session, fresh); }); + test('re-upgrade preserves a v4 column after an actual legacy v3 open', + () async { + final disk = await _Disk.create(); + final first = (await disk.repo.createSession('local', queries: ['cat']))!; + await _answer( + disk.repo, first, (await disk.repo.buildQuestion(first, 'en'))!); + await disk.repo.finishSession( + uid: 'local', sessionId: first.id, completed: true, activeMs: 10); + final session = (await disk.repo.createSession('local', queries: ['cat']))!; + final english = (await disk.repo.buildQuestion(session, 'en'))!; + final originalQuestions = await disk.db.query('review_questions'); + final originalProgress = await disk.db.query('learning_progress'); + final originalAttempts = await disk.db.query('review_attempts'); + + await disk.db.close(); + // This is the legacy open behavior, not a manual setVersion(3): the old + // app supplied version 3 with no onDowngrade callback to sqflite. + disk.db = await databaseFactoryFfi.openDatabase(disk.path, + options: OpenDatabaseOptions(version: 3, singleInstance: false)); + expect(await disk.db.getVersion(), 3); + expect(await disk.db.query('review_questions'), originalQuestions); + final legacy = ReviewQuestion( + target: english.target, + options: const ['猫', '河流', '书籍', '花园'], + correctIndex: 0, + languageCode: 'zh_Hans'); + // A question created by that old app omits the unknown nullable column. + await disk.db.insert('review_questions', { + 'session_id': session.id, + 'target_id': legacy.target.targetId, + 'language_code': legacy.languageCode, + 'options_json': jsonEncode(legacy.options), + 'correct_index': legacy.correctIndex, + 'created_at': DateTime.now().millisecondsSinceEpoch, + }); + + await disk.reopen(); + expect(await disk.db.getVersion(), 4); + expect(await disk.db.query('learning_progress'), originalProgress); + expect(await disk.db.query('review_attempts'), originalAttempts); + for (final original in originalQuestions) { + expect( + (await disk.db.query('review_questions', + where: 'session_id = ? AND language_code = ?', + whereArgs: [ + original['session_id'], + original['language_code'] + ])) + .single, + original); + } + final unbound = (await disk.db.query('review_questions', + where: 'session_id = ? AND language_code = ?', + whereArgs: [session.id, 'zh_Hans'])) + .single; + expect(unbound['content_fingerprint'], isNull); + await expectLater(_answer(disk.repo, session, legacy), + throwsA(isA())); + expect((await disk.repo.buildQuestion(session, 'en'))!.options, + english.options); + final refreshed = (await disk.repo.buildQuestion(session, 'zh_Hans'))!; + expect(refreshed.type, ReviewTestType.independent); + await _answer(disk.repo, session, refreshed); + await disk.reopen(); + expect((await disk.repo.targetById(legacy.target.targetId))!.stage, + LearningStage.learned); + expect(await disk.db.query('review_attempts'), hasLength(2)); + }); + + test( + 're-upgrade rejects conflicting fingerprint column shapes without writes', + () async { + for (final declaration in [ + 'BLOB', + "TEXT NOT NULL DEFAULT ''", + "TEXT DEFAULT 'unbound'", + ]) { + final disk = await _Disk.create(); + final progress = await disk.db.query('learning_progress'); + await disk.db.execute( + 'ALTER TABLE review_questions DROP COLUMN content_fingerprint'); + await disk.db.execute('ALTER TABLE review_questions ' + 'ADD COLUMN content_fingerprint $declaration'); + await disk.db.setVersion(3); + final columns = + await disk.db.rawQuery('PRAGMA table_info(review_questions)'); + await expectLater(disk.reopen(), throwsStateError, reason: declaration); + disk.db = await databaseFactoryFfi.openDatabase(disk.path, + options: OpenDatabaseOptions(singleInstance: false)); + expect(await disk.db.getVersion(), 3, reason: declaration); + expect(await disk.db.query('learning_progress'), progress); + expect(await disk.db.rawQuery('PRAGMA table_info(review_questions)'), + columns); + } + }); + + test( + 'a newer database is rejected without lowering its version or losing data', + () async { + final disk = await _Disk.create(); + final progress = await disk.db.query('learning_progress'); + await disk.db.setVersion(5); + await expectLater( + disk.reopen(), + throwsA(isA().having((error) => error.message, 'message', + contains('requires a newer app')))); + disk.db = await databaseFactoryFfi.openDatabase(disk.path, + options: OpenDatabaseOptions(singleInstance: false)); + expect(await disk.db.getVersion(), 5); + expect(await disk.db.query('learning_progress'), progress); + }); + test('an aborted attempt insert rolls back progress and retries after reopen', () async { final disk = await _Disk.create(); @@ -390,6 +502,7 @@ class _Disk { singleInstance: false, onCreate: LearningRepository.createSchema, onUpgrade: LearningRepository.upgradeSchema, + onDowngrade: LearningRepository.rejectSchemaDowngrade, onConfigure: (db) => db.execute('PRAGMA foreign_keys = ON'))); Future reopen() async {