-
Notifications
You must be signed in to change notification settings - Fork 164
13730 inconsistent behaviour of sormas mobile application #13749
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -235,6 +235,23 @@ public void fillInnerFromDto(Symptoms target, SymptomsDto source) { | |
| target.setHighOrLowBloodPressure(source.getHighOrLowBloodPressure()); | ||
| target.setUrinaryRetention(source.getUrinaryRetention()); | ||
| target.setShivering(source.getShivering()); | ||
| target.setAcuteEncephalitis(source.getAcuteEncephalitis()); | ||
| target.setEggyBurps(source.getEggyBurps()); | ||
| target.setWeightLoss(source.getWeightLoss()); | ||
| target.setWeightLossAmount(source.getWeightLossAmount()); | ||
| target.setReoccurrence(source.getReoccurrence()); | ||
| target.setSymptomCurrentStatus(source.getSymptomCurrentStatus()); | ||
| target.setDurationOfSymptoms(source.getDurationOfSymptoms()); | ||
| target.setOvernightStayRequired(source.getOvernightStayRequired()); | ||
| target.setBloating(source.getBloating()); | ||
| target.setDifficultyBreathing(source.getDifficultyBreathing()); | ||
| target.setParadoxicalBreathing(source.getParadoxicalBreathing()); | ||
| target.setRespiratoryFatigue(source.getRespiratoryFatigue()); | ||
| target.setParentTimeOffWork(source.getParentTimeOffWork()); | ||
| target.setTimeOffWorkDays(source.getTimeOffWorkDays()); | ||
| target.setUnknownSymptom(source.getUnknownSymptom()); | ||
| target.setSkinRashOnsetDate(source.getSkinRashOnsetDate()); | ||
| target.setAsymptomatic(source.getAsymptomatic()); | ||
|
Comment on lines
+238
to
+254
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. New difficultyBreathingDuringMeals field is not mapped; duplicate difficultyBreathing call is likely a bug The new DB column and model field are Suggest correcting both directions: @@ fillInnerFromDto
- target.setBloating(source.getBloating());
- target.setDifficultyBreathing(source.getDifficultyBreathing());
+ target.setBloating(source.getBloating());
+ target.setDifficultyBreathingDuringMeals(source.getDifficultyBreathingDuringMeals());
@@ fillInnerFromAdo
- target.setBloating(source.getBloating());
- target.setDifficultyBreathing(source.getDifficultyBreathing());
+ target.setBloating(source.getBloating());
+ target.setDifficultyBreathingDuringMeals(source.getDifficultyBreathingDuringMeals());All other newly added fields (acuteEncephalitis, eggyBurps, weightLoss, weightLossAmount, reoccurrence, symptomCurrentStatus, durationOfSymptoms, overnightStayRequired, bloating, paradoxicalBreathing, respiratoryFatigue, parentTimeOffWork, timeOffWorkDays, unknownSymptom, skinRashOnsetDate, asymptomatic) are mapped symmetrically, which is good. Also applies to: 441-457 🤖 Prompt for AI Agents |
||
|
|
||
| target.setPseudonymized(source.isPseudonymized()); | ||
| } | ||
|
|
@@ -421,6 +438,23 @@ public void fillInnerFromAdo(SymptomsDto target, Symptoms source) { | |
| target.setHighOrLowBloodPressure(source.getHighOrLowBloodPressure()); | ||
| target.setUrinaryRetention(source.getUrinaryRetention()); | ||
| target.setShivering(source.getShivering()); | ||
| target.setAcuteEncephalitis(source.getAcuteEncephalitis()); | ||
| target.setEggyBurps(source.getEggyBurps()); | ||
| target.setWeightLoss(source.getWeightLoss()); | ||
| target.setWeightLossAmount(source.getWeightLossAmount()); | ||
| target.setReoccurrence(source.getReoccurrence()); | ||
| target.setSymptomCurrentStatus(source.getSymptomCurrentStatus()); | ||
| target.setDurationOfSymptoms(source.getDurationOfSymptoms()); | ||
| target.setOvernightStayRequired(source.getOvernightStayRequired()); | ||
| target.setBloating(source.getBloating()); | ||
| target.setDifficultyBreathing(source.getDifficultyBreathing()); | ||
| target.setParadoxicalBreathing(source.getParadoxicalBreathing()); | ||
| target.setRespiratoryFatigue(source.getRespiratoryFatigue()); | ||
| target.setParentTimeOffWork(source.getParentTimeOffWork()); | ||
| target.setTimeOffWorkDays(source.getTimeOffWorkDays()); | ||
| target.setUnknownSymptom(source.getUnknownSymptom()); | ||
| target.setSkinRashOnsetDate(source.getSkinRashOnsetDate()); | ||
| target.setAsymptomatic(source.getAsymptomatic()); | ||
|
|
||
| target.setPseudonymized(source.isPseudonymized()); | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Using
switch (newVersion)in onUpgrade is likely incorrect and breaks multi‑step upgradesonUpgradeis called once with(oldVersion, newVersion)bySQLiteOpenHelper. With the current code:When upgrading from, say,
oldVersion = 200tonewVersion = 360, this switch enters atcase 360and skips all upgrade steps 201–359, which still need to be applied. The previous pattern withswitch (oldVersion)(orswitch (currentVersion)) and fall‑through is the standard way to ensure all intermediate migrations run.Unless you have custom code that calls
onUpgraderepeatedly for each intermediate version (which would be very unusual and is not visible here), this change will result in inconsistent schemas for users upgrading from older app versions.I strongly recommend reverting this to switch on the starting version (or
currentVersion) so that all cases fromoldVersion+1up tonewVersionexecute via fall‑through.To be safe, please:
onUpgradein this class.SQLiteOpenHelper.onUpgradeis only called once per upgrade with the finalnewVersion, and not iteratively.🌐 Web query:
💡 Result:
Short answer:
When you call getWritableDatabase() or getReadableDatabase() and the on-disk DB version (PRAGMA user_version) is less than the version you passed into your SQLiteOpenHelper constructor, Android opens the DB and invokes onUpgrade(db, oldVersion, newVersion) inside a transaction to move the schema from oldVersion to newVersion. onCreate is used only when the DB file does not yet exist. [1]
Recommended pattern: perform incremental, idempotent migration steps from the current version up to the target version—apply each intermediate migration in sequence (e.g. for (v = oldVersion+1; v <= newVersion; v++) { switch(v) { case 2: /* migrate 1→2 /; break; case 3: / migrate 2→3 */; break; ... } }). This ensures upgrades work for any skip-upgrade (e.g. 1→4) and keeps each step small and testable. Do not alter a released migration step for a given from→to pair; if it was incorrect, add a new migration to fix it. [1][2][3]
References
References list
[1] Android Developers — SQLiteOpenHelper (onUpgrade/onCreate/onConfigure).
[2] Stack Overflow examples: looping/switch pattern for incremental onUpgrade.
[3] Tutorials documenting switch/for-loop migration pattern for SQLiteOpenHelper.
I need to inspect the actual code to verify the severity and exact issue. Let me examine the DatabaseHelper.java file around the reported lines.
Length of output: 86
🏁 Script executed:
Length of output: 5221
🏁 Script executed:
Length of output: 29032
🏁 Script executed:
Length of output: 5287
Fix critical database migration bug: switch must target oldVersion, not newVersion
When you call getWritableDatabase() or getReadableDatabase() and the on-disk DB version is less than the version you passed into your SQLiteOpenHelper constructor, Android invokes onUpgrade(db, oldVersion, newVersion) inside a transaction to move the schema from oldVersion to newVersion. The current code uses
switch (newVersion)at line 401, which causes intermediate migrations to be skipped.Example: upgrading from version 200 to 360 will jump directly to
case 360:and skip all migrations from 201–359, leaving the schema in an inconsistent state.The recommended pattern is to perform incremental, idempotent migration steps from the current version up to the target version—apply each intermediate migration in sequence (e.g. for (v = oldVersion+1; v <= newVersion; v++) { switch(v) { case 2: /* migrate 1→2 /; break; case 3: / migrate 2→3 */; break; ... } }).
Change the switch to iterate from
oldVersion + 1tonewVersion, or switch onoldVersionto ensure fall-through executes all required steps.🤖 Prompt for AI Agents