search

getodk / collect

ODK Collect is an Android app for filling out forms. It's been used to collect billions of data points in challenging environments around the world. Contribute and make the world a better place! ✨📋✨
https://docs.getodk.org/collect-intro
Other
718 stars 1.38k forks source link

"usesEntities" in forms.db after upgrading Collect #6493

Closed dbemke closed 2 weeks ago

dbemke commented 3 weeks ago

ODK Collect version

the master version 1a10357ec4ed3f2b38ba93b4080d577404044c2c

Android version

10, 14

Device used

Redmi 9T, Pixel 7a

Problem description

It seems that setting "true" in "usesEntities" column in forms.db doesn't work as expected after upgrading Collect. After upgrading the version of Collect only publishing new versions of forms changes the value to "true". Manually refreshing the list of blank forms (with a change in the dataset or not) doesn't set "true". Details and differences in types of forms are described in steps below.

Steps to reproduce the problem

  1. Install the store version of Collect.
  2. Scan user "trees” Qr code (with old entities spec forms and entities in the dataset). https://test.getodk.cloud/#/projects/576/app-users
  3. Upgrade to the current master version (the apk is on Slack).
  4. Go to the db files and check forms.db column "usesEntities” (the values are "null").
  5. Go to "Start new form” and tap the arrow to refresh the list of blank forms.
  6. Go to the db files and check forms.db column "usesEntities” (the values are "null").
  7. In Central add a new entity, then go to "Start new form” and tap the arrow to refresh the list of blank forms.
  8. Go to the db files and check forms.db column "usesEntities” ( follow-up form gets "false”, registration is "null").

    Expected behavior

    After upgrading the value should change to "true" in forms using entities.

lognaturel commented 3 weeks ago

This looks like the current expected behavior to me and I agree it's worth really thinking through carefully.

Only forms with the v2024.1.0 Entities spec can create or update Entities offline and it's not possible to have a form with the v2024.1.0 Entities spec in an earlier version of Collect. This makes it seem like the current behavior is ok.

If there are only forms that create Entities without attaching the Entity List, I believe that indeed everything is fine. Entities will not be created offline until there's a form update. It's ok for form updates to happen while the form is being filled because the in-memory representation of the Entity List will be used.

The complicating factor is that Central v2024.2 will always set the type attribute in the manifest to entityList when it serves an Entity List. Once Collect has seen that a form attachment has that type, whenever it sees the Entity List's name referenced, Collect will use its internal Entity List representation.

There's a problem any time a form uses the database representation of the Entity List but doesn't have the usesEntities flag set to true. I'm fairly confident it's impossible for a form to attempt an update or create offline without having the flag set because a form update to Entities spec v2024.1.0 is required to get offline updates or creates.

What I believe does happen now is that forms that access an Entity List will use the database representation without a lock on updates. This can lead to inconsistent data being saved to a form and crashes if the update includes deletes.

What I observe is that the moment I get an automatic update from the server, Collect detects Entity Lists attached forms and processes them into their database representations. I would expect that it would also set the usesEntities flag to true at that point. This is hopefully a straightforward update to make.

Even if we made that change, I think there's still a case where it's possible to get in an inconsistent state. If multiple forms attach the same Entity List, it would be possible for the form update to fail with only some of those forms being updated. Because Collect uses the Entity List name to decide whether to open a database representation or not (https://github.com/getodk/collect/issues/6425), forms that hadn't been updated and therefore weren't marked as usesEntities would use that database representation without blocking updates. This feels rare enough that it would be ok to leave in initially. I think we will address it as part of improvements to download when we will specifically target the case of multiple forms attaching the same Entity List.

grzesiek2010 commented 3 weeks ago

What I observe is that the moment I get an automatic update from the server, Collect detects Entity Lists attached forms and processes them into their database representations. I would expect that it would also set the usesEntities flag to true at that point.

@lognaturel is it this scenario or something else? https://github.com/getodk/collect/pull/6494/files#diff-5eb1afcd487fe07f5caccf89e6c38534b07e0d49dd218e0929d91bb16cac8174R822

lognaturel commented 3 weeks ago

I've responded there but for completeness -- the manifest is always checked for Entity Lists regardless of whether there's a form or attachment update and Entity Lists are always processed if identified from the manifest. This means the flag for the form should be set to true any time its manifest is downloaded and there's an Entity List declared in it.