Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ public class DatabaseHelper extends OrmLiteSqliteOpenHelper {
public static final String DATABASE_NAME = "sormas.db";
// any time you make changes to your database objects, you may have to increase the database version

public static final int DATABASE_VERSION = 359;
public static final int DATABASE_VERSION = 360;

private static DatabaseHelper instance = null;

Expand Down Expand Up @@ -398,7 +398,7 @@ public void onUpgrade(SQLiteDatabase db, ConnectionSource connectionSource, int
// IMPORTANT: table and column names are CASE SENSITIVE!
int currentVersion = oldVersion;
try {
switch (oldVersion) {
switch (newVersion) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Using switch (newVersion) in onUpgrade is likely incorrect and breaks multi‑step upgrades

onUpgrade is called once with (oldVersion, newVersion) by SQLiteOpenHelper. With the current code:

int currentVersion = oldVersion;
switch (newVersion) {
    case 91:
        currentVersion = 91;
        // ...
    // ...
    case 360:
        currentVersion = 360;
        // ...
        break;
}

When upgrading from, say, oldVersion = 200 to newVersion = 360, this switch enters at case 360 and skips all upgrade steps 201–359, which still need to be applied. The previous pattern with switch (oldVersion) (or switch (currentVersion)) and fall‑through is the standard way to ensure all intermediate migrations run.

Unless you have custom code that calls onUpgrade repeatedly 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 from oldVersion+1 up to newVersion execute via fall‑through.

To be safe, please:

  • Re‑check the previous implementation of onUpgrade in this class.
  • Confirm via Android docs or a small test app that SQLiteOpenHelper.onUpgrade is only called once per upgrade with the final newVersion, and not iteratively.

🌐 Web query:

How does Android's SQLiteOpenHelper.onUpgrade get invoked with oldVersion and newVersion, and what is the recommended pattern for running incremental schema migrations?

💡 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

  • Android SQLiteOpenHelper docs (onCreate/onUpgrade behavior, transaction guarantee, and migration guidance). [1]
  • Common community patterns / examples showing loop or switch/fall‑through incremental upgrades (StackOverflow / tutorials). [2][3]

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.


---

🏁 Script executed:

```shell
#!/bin/bash
# Examine the onUpgrade method in DatabaseHelper.java around line 401
find . -name "DatabaseHelper.java" -type f | head -5

Length of output: 86


🏁 Script executed:

#!/bin/bash
# Once we find the file, examine the onUpgrade method structure
find . -name "DatabaseHelper.java" -path "*/backend/common/*" -type f -exec head -450 {} \; | tail -100

Length of output: 5221


🏁 Script executed:

#!/bin/bash
# Search for the onUpgrade method definition and structure
rg -n "onUpgrade" --type=java -A 100 | head -200

Length of output: 29032


🏁 Script executed:

#!/bin/bash
# Get specific context around line 401
fd "DatabaseHelper.java" | xargs -I {} sh -c 'sed -n "350,450p" "{}"'

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 + 1 to newVersion, or switch on oldVersion to ensure fall-through executes all required steps.

🤖 Prompt for AI Agents
In
sormas-app/app/src/main/java/de/symeda/sormas/app/backend/common/DatabaseHelper.java
around line 401, the code currently switches on newVersion which skips
intermediate migration steps; change the logic to perform incremental migrations
from oldVersion+1 through newVersion (for example: loop v from oldVersion+1 to
newVersion and switch on v to apply each migration case), ensuring each
migration step is idempotent and applied in sequence so no intermediate versions
are skipped.

case 91:
currentVersion = 91;
getDao(District.class).executeRaw("ALTER TABLE district ADD COLUMN epidCode varchar(255);");
Expand Down Expand Up @@ -3201,8 +3201,28 @@ public void onUpgrade(SQLiteDatabase db, ConnectionSource connectionSource, int
getDao(Symptoms.class).executeRaw("ALTER TABLE symptoms ADD COLUMN septicaemia varchar(255);");
getDao(Symptoms.class).executeRaw("ALTER TABLE symptoms ADD COLUMN whoopsound varchar(255);");

// ATTENTION: break should only be done after last version
break;
case 360:
currentVersion = 360;
getDao(Symptoms.class).executeRaw("ALTER TABLE symptoms ADD COLUMN acuteEncephalitis varchar(255);");
getDao(Symptoms.class).executeRaw("ALTER TABLE symptoms ADD COLUMN eggyBurps varchar(255);");
getDao(Symptoms.class).executeRaw("ALTER TABLE symptoms ADD COLUMN weightLoss varchar(255);");
getDao(Symptoms.class).executeRaw("ALTER TABLE symptoms ADD COLUMN weightLossAmount float8;");
getDao(Symptoms.class).executeRaw("ALTER TABLE symptoms ADD COLUMN reoccurrence varchar(255);");
getDao(Symptoms.class).executeRaw("ALTER TABLE symptoms ADD COLUMN symptomCurrentStatus varchar(255);");
getDao(Symptoms.class).executeRaw("ALTER TABLE symptoms ADD COLUMN durationOfSymptoms int;");
getDao(Symptoms.class).executeRaw("ALTER TABLE symptoms ADD COLUMN overnightStayRequired varchar(255);");
getDao(Symptoms.class).executeRaw("ALTER TABLE symptoms ADD COLUMN bloating varchar(255);");
getDao(Symptoms.class).executeRaw("ALTER TABLE symptoms ADD COLUMN difficultyBreathingDuringMeals varchar(255);");
getDao(Symptoms.class).executeRaw("ALTER TABLE symptoms ADD COLUMN paradoxicalBreathing varchar(255);");
getDao(Symptoms.class).executeRaw("ALTER TABLE symptoms ADD COLUMN respiratoryFatigue varchar(255);");
getDao(Symptoms.class).executeRaw("ALTER TABLE symptoms ADD COLUMN parentTimeOffWork varchar(255);");
getDao(Symptoms.class).executeRaw("ALTER TABLE symptoms ADD COLUMN timeOffWorkDays float8;");
getDao(Symptoms.class).executeRaw("ALTER TABLE symptoms ADD COLUMN unknownSymptom varchar(255);");
getDao(Symptoms.class).executeRaw("ALTER TABLE symptoms ADD COLUMN skinRashOnsetDate timestamp;");


// ATTENTION: break should only be done after last version
break;



Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,19 @@ private QueryBuilder<Contact, Long> buildQueryBuilder(ContactCriteria contactCri

// Only use user filter if no restricting case is specified
if (contactCriteria.getIncludeContactsFromOtherJurisdictions().equals(false)) {
whereStatements.add(where.or(createJurisdictionFilterForCase(where), createJurisdictionFilter(where)));
// whereStatements.add(where.or(createJurisdictionFilterForCase(where), createJurisdictionFilter(where)));
Where<Contact, Long> caseJurisdictionWhere = createJurisdictionFilterForCase(where);
Where<Contact, Long> jurisdictionWhere = createJurisdictionFilter(where);
if (caseJurisdictionWhere != null && jurisdictionWhere != null) {
whereStatements.add(where.or(caseJurisdictionWhere, jurisdictionWhere));
} else {
if (caseJurisdictionWhere != null) {
whereStatements.add(caseJurisdictionWhere);
}
if (jurisdictionWhere != null) {
whereStatements.add(jurisdictionWhere);
}
}
}

if (contactCriteria != null) {
Expand Down Expand Up @@ -283,8 +295,9 @@ public Where<Contact, Long> createJurisdictionFilterForCase(Where<Contact, Long>

if (!whereJurisdictionFilterStatements.isEmpty()) {
where.or(whereJurisdictionFilterStatements.size());
return where;
}
return where;
return null;
}

public Where<Contact, Long> createJurisdictionFilter(Where<Contact, Long> where) throws SQLException {
Expand Down Expand Up @@ -316,9 +329,10 @@ public Where<Contact, Long> createJurisdictionFilter(Where<Contact, Long> where)

if (!whereUserFilterStatements.isEmpty()) {
where.or(whereUserFilterStatements.size());
return where;
}

return where;
return null;
}

public Where<Contact, Long> createCriteriaFilter(List<Where<Contact, Long>> whereStatements, Where<Contact, Long> where, ContactCriteria criteria)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,41 @@ public class Symptoms extends PseudonymizableAdo {
private SymptomState shivering;
@Enumerated(EnumType.STRING)
private ClinicalPresentationStatus clinicalPresentationStatus;
@Enumerated(EnumType.STRING)
private SymptomState acuteEncephalitis;
@Enumerated(EnumType.STRING)
private SymptomState eggyBurps;
@Enumerated(EnumType.STRING)
private SymptomState weightLoss;
@Column
private Float weightLossAmount;
@Enumerated(EnumType.STRING)
private SymptomState reoccurrence;
@Enumerated(EnumType.STRING)
private SymptomState symptomCurrentStatus;
@Column
private Integer durationOfSymptoms;
@Enumerated(EnumType.STRING)
private SymptomState overnightStayRequired;
@Enumerated(EnumType.STRING)
private SymptomState bloating;

@Enumerated(EnumType.STRING)
private SymptomState difficultyBreathingDuringMeals;
@Enumerated(EnumType.STRING)
private SymptomState paradoxicalBreathing;
@Enumerated(EnumType.STRING)
private SymptomState respiratoryFatigue;
@Enumerated(EnumType.STRING)
private YesNoUnknown parentTimeOffWork;
@Column
private Float timeOffWorkDays;
@Enumerated(EnumType.STRING)
private SymptomState unknownSymptom;
@DatabaseField(dataType = DataType.DATE_LONG)
private Date skinRashOnsetDate;



@Override
public String getI18nPrefix() {
Expand Down Expand Up @@ -2045,4 +2080,136 @@ public ClinicalPresentationStatus getClinicalPresentationStatus() {
public void setClinicalPresentationStatus(ClinicalPresentationStatus clinicalPresentationStatus) {
this.clinicalPresentationStatus = clinicalPresentationStatus;
}

public SymptomState getBloating() {
return bloating;
}

public void setBloating(SymptomState bloating) {
this.bloating = bloating;
}

public SymptomState getOvernightStayRequired() {
return overnightStayRequired;
}

public void setOvernightStayRequired(SymptomState overnightStayRequired) {
this.overnightStayRequired = overnightStayRequired;
}

public Integer getDurationOfSymptoms() {
return durationOfSymptoms;
}

public void setDurationOfSymptoms(Integer durationOfSymptoms) {
this.durationOfSymptoms = durationOfSymptoms;
}

public SymptomState getSymptomCurrentStatus() {
return symptomCurrentStatus;
}

public void setSymptomCurrentStatus(SymptomState symptomCurrentStatus) {
this.symptomCurrentStatus = symptomCurrentStatus;
}

public SymptomState getReoccurrence() {
return reoccurrence;
}

public void setReoccurrence(SymptomState reoccurrence) {
this.reoccurrence = reoccurrence;
}

public Float getWeightLossAmount() {
return weightLossAmount;
}

public void setWeightLossAmount(Float weightLossAmount) {
this.weightLossAmount = weightLossAmount;
}

public SymptomState getWeightLoss() {
return weightLoss;
}

public void setWeightLoss(SymptomState weightLoss) {
this.weightLoss = weightLoss;
}

public SymptomState getEggyBurps() {
return eggyBurps;
}

public void setEggyBurps(SymptomState eggyBurps) {
this.eggyBurps = eggyBurps;
}

public SymptomState getAcuteEncephalitis() {
return acuteEncephalitis;
}

public void setAcuteEncephalitis(SymptomState acuteEncephalitis) {
this.acuteEncephalitis = acuteEncephalitis;
}

public Float getTimeOffWorkDays() {
return timeOffWorkDays;
}

public void setTimeOffWorkDays(Float timeOffWorkDays) {
this.timeOffWorkDays = timeOffWorkDays;
}

public YesNoUnknown getParentTimeOffWork() {
return parentTimeOffWork;
}

public void setParentTimeOffWork(YesNoUnknown parentTimeOffWork) {
this.parentTimeOffWork = parentTimeOffWork;
}

public SymptomState getRespiratoryFatigue() {
return respiratoryFatigue;
}

public void setRespiratoryFatigue(SymptomState respiratoryFatigue) {
this.respiratoryFatigue = respiratoryFatigue;
}

public SymptomState getParadoxicalBreathing() {
return paradoxicalBreathing;
}

public void setParadoxicalBreathing(SymptomState paradoxicalBreathing) {
this.paradoxicalBreathing = paradoxicalBreathing;
}

public SymptomState getDifficultyBreathingDuringMeals() {
return difficultyBreathingDuringMeals;
}

public void setDifficultyBreathingDuringMeals(SymptomState difficultyBreathingDuringMeals) {
this.difficultyBreathingDuringMeals = difficultyBreathingDuringMeals;
}

public void setNocturnalCough(SymptomState nocturnalCough) {
this.nocturnalCough = nocturnalCough;
}

public Date getSkinRashOnsetDate() {
return skinRashOnsetDate;
}

public void setSkinRashOnsetDate(Date skinRashOnsetDate) {
this.skinRashOnsetDate = skinRashOnsetDate;
}

public SymptomState getUnknownSymptom() {
return unknownSymptom;
}

public void setUnknownSymptom(SymptomState unknownSymptom) {
this.unknownSymptom = unknownSymptom;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

New difficultyBreathingDuringMeals field is not mapped; duplicate difficultyBreathing call is likely a bug

The new DB column and model field are difficultyBreathingDuringMeals, but both DTO⇄entity mappings added here call setDifficultyBreathing a second time. This leaves difficultyBreathingDuringMeals unmapped and can cause lost or never‑synced data for that field.

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
In
sormas-app/app/src/main/java/de/symeda/sormas/app/backend/symptoms/SymptomsDtoHelper.java
around lines 238-254 (and the symmetric mapping at 441-457), the mapping
mistakenly calls setDifficultyBreathing twice so difficultyBreathingDuringMeals
is never mapped; change the duplicate setDifficultyBreathing call to use the
difficultyBreathingDuringMeals accessor/mutator (use
source.getDifficultyBreathingDuringMeals when copying into target and
target.setDifficultyBreathingDuringMeals when copying back) so the new field is
mapped symmetrically like the other fields.


target.setPseudonymized(source.isPseudonymized());
}
Expand Down Expand Up @@ -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());
}
Expand Down
Loading