[MS-1244] Adding proper migration to updated ENROLMENT_V4 event#1463
Conversation
… but missed in migrations. Adding migrations to introduce empty array for all old events
… but missed in migrations. Adding migration tests
alexandr-simprints
requested review from
BurningAXE,
alex-vt,
luhmirin-s and
meladRaouf
…ScopesForType so that we have a Crashlytics non-fatal recorded in case any event fails to be parsed
There was a problem hiding this comment.
Pull Request Overview
This PR adds a database migration (version 16 to 17) to fix a bug where old ENROLMENT_V4 events stored before 2025.4.0 are missing the required externalCredentialIds field, causing JSON parsing exceptions during sync operations.
Key Changes:
- Database version bumped from 16 to 17
- New migration adds empty
externalCredentialIdsarray to existingENROLMENT_V4events that lack this field - Changed log level from info to error for JSON unmarshalling failures in sync task
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| EventMigration16to17.kt | Implements the migration logic to add externalCredentialIds field to ENROLMENT_V4 events |
| EventMigration16to17Test.kt | Comprehensive test coverage for the new migration |
| EventRoomDatabase.kt | Updates database version to 17 and registers the new migration |
| EventMigrationTest.kt | Updates test to validate migration to version 17 |
| EventUpSyncTask.kt | Changes log level from info to error for JSON parsing failures |
| 17.json | Schema file for database version 17 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ject class to deserialize the even string, and add missing 'externalCredentialIds' field to the payload
luhmirin-s
left a comment
luhmirin-s
left a comment
There was a problem hiding this comment.
Consider squashing the branch before merging :)
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
BurningAXE
left a comment
BurningAXE
left a comment
There was a problem hiding this comment.
Copilot is also right about the documentation :)
|
alexandr-simprints
deleted the
MS-1244-version-hops-incorrect-event-structure-after-sid-updates
branch
JIRA ticket
Will be released in: 2025.4.0
Root cause analysis (for bugfixes only)
First known affected version: 2025.4.0
When doing version hops from
2024.1.5to2025.4.0without BFSID data sync, the local events are stored with incorrect structure in the final 2025.4.0 SID version. This becomes visible when trying to sync in 2025.4.0, as we begin receivingCORRUPTED_EVENTSmessages on thesid-dump-filesSlack channel.The issue was that UpSync worker tried to deserialize old events in the DB to the latest
EnrolmentEventV4class that contains non-nullexternalCredentialIdsarray field. This led toJsonParseExceptionsinceexternalCredentialIdsfiled was missing in old events in the database.Notable changes
16to17"externalCredentialIds":[],JSON field to all existingENROLMENT_V4events (if doesn't contain already)Testing guidance
sid-dump-fileschannel on Slack contains no new messagesAdditional work checklist