Conversation
v1.5.1 crashed on launch when upgrading from v1.5.0 (or any earlier version) with Room reporting a TableInfo mismatch with two specific diffs: 1. transactions table had idx_tx_pending in the migrated DB but the entity only declared idx_tx_wallet_network_time. MIGRATION_5_6 created idx_tx_pending as a partial index in raw SQL because @Index cannot express WHERE clauses. The migration comment claimed Room ignores indices it didn't create, but Room's pragma-based introspection does see the index and validates it. 2. transactions.walletId column had DEFAULT '' from MIGRATION_2_3 but the entity field had no @ColumnInfo(defaultValue) annotation. balance_cache.walletId and dao_cells.walletId have the same shape. Adds: - @Index for idx_tx_pending on TransactionEntity (matches columns + name; Room compares by name/columns/orders, not WHERE clauses). - @ColumnInfo(defaultValue = "''") on the three walletId columns. Existing MigrationTest uses inMemoryDatabaseBuilder so it never exercises real migration paths. Proper instrumented MigrationTestHelper tests are tracked as follow-up.
UpdateRepository polled api.github.com/repos/AgustaRC/ckb-wallet-gateway which 404s. The fork URL was in the v1.5.0 + v1.5.1 builds, so the in-app 'new version available' notification never fired for any user. Fix: poll RaheemJnr/pocket-node. Users on v1.5.0/v1.5.1 still won't notice this fix until they manually grab v1.5.2 once; from v1.5.2 onward, auto-update works.
Hotfix for v1.5.1's migration crash and the broken auto-update URL.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThe Android app version is incremented from 1.5.1 to 1.5.2. Room entity Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~18 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
…) (#147) * test(db): walking-migration regression guard + Room schema export (#142) The pre-existing MigrationTest used Room.inMemoryDatabaseBuilder which creates the schema directly from the current entity declarations and never exercises Migration.migrate code paths. So when v1.5.1's entity declarations drifted from what the migrations produced, the in-memory test passed but real upgraders crashed at launch (#141, #143). Adds: 1. Room schema export (exportSchema = true + ksp arg pointing at app/schemas/). Generates a versioned JSON dump per @database version. v9 committed in this PR. Future schema drift becomes visible in PR diffs. 2. WalkingMigrationTest: bootstraps a SQLite file at v8 with a known historical shape, opens it via Room.databaseBuilder, and lets Room run MIGRATION_8_9 + validate the result against entity declarations. Two test methods cover the divergent v8 shapes that existed in the wild after v1.5.1: - Path A (upgrade-from-v1.5.0): walletId DEFAULT '' + partial idx_tx_pending already on disk. - Path B (fresh-install-on-v1.5.1): neither. Both must converge to v9 successfully. 3. Regression-guard test that registers a no-op MIGRATION_8_9 (what the first cut of #143 shipped) and asserts path B fails with the exact 'Migration didn't properly handle' exception that surfaced in production. Permanent in-codebase guard against the test losing its detection power. Note on scope: the JVM/Robolectric harness here covers the v8→v9 upgrade path because that is where the historical fork lives. Walking from v1 forward is harder without an exported schema for v1 and is tracked in #144 Phase 2 (instrumented MigrationTestHelper). * fix(ci): don't commit Room schema JSONs (kotlinx-serialization conflict) CI's clean build hit AbstractMethodError in Room's SchemaBundle.$serializer because Room 2.8.4's bundled kotlinx-serialization-core is binary-incompatible with the project's kotlinx-serialization-json:1.8.0 on a fresh classpath. Local builds didn't repro because of cached classloaders. When a schema JSON exists at the target version, Room's processor deserializes it to diff against the new export. With no JSON present, Room only writes a new one, no deserialize path, no crash. Workaround: don't commit the JSONs. They're still generated on every build (locally and in CI), they just don't get persisted. The walking-migration test doesn't depend on them — it validates against entity declarations directly. Trade-off lost: schema diffs aren't visible in PR review. We get them back when we either upgrade Room to a version compiled against kotlinx-serialization 1.8.x, or downgrade kotlinx-serialization-json to 1.7.x. Tracked alongside #142. * fix(ci): turn Room exportSchema off; track re-enable in #149 Same AbstractMethodError surfaced even after .gitignore'ing the JSONs: Room's deserialize path runs on every KSP cycle, not only when an existing schema file is present. The dep conflict between Room 2.8.4 and kotlinx-serialization 1.8.0 needs a real fix before exportSchema can be on. Reverts the @database exportSchema flag and the room.schemaLocation KSP arg. The walking-migration test under src/test/ still validates schema-vs-entity drift via Room's onMigrate path, so #142's regression-guard value is unaffected. Re-enabling tracked in #149.
Summary
Emergency hotfix for two blockers discovered after v1.5.1 shipped.
1. v1.5.1 crashes on upgrade
Users upgrading from v1.5.0 (or any earlier version) hit:
```
java.lang.IllegalStateException: Migration didn't properly handle: transactions(...).
```
Two specific diffs:
The existing `MigrationTest` uses `inMemoryDatabaseBuilder` which creates a fresh schema without exercising migrations, which is why this wasn't caught in CI. Adding a proper instrumented `MigrationTestHelper` test (with `exportSchema = true`) is tracked as follow-up.
2. In-app auto-update notification never fires
`UpdateRepository` polled `api.github.com/repos/AgustaRC/ckb-wallet-gateway/releases/latest` which 404s, so every release check returned an exception that was silently logged. Users have never received an auto-update notification.
Fixed by pointing at `RaheemJnr/pocket-node`.
Important caveat: users on v1.5.0 / v1.5.1 are running code that polls the broken URL. They will not get an auto-update notification for v1.5.2 either; they have to grab it from the website or GitHub release manually once. From v1.5.2 onward, auto-update works.
Version
Test plan
Follow-ups
Communication plan after merge
Summary by CodeRabbit
Bug Fixes