Skip to content
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

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

Closed
dbemke opened this issue Nov 4, 2024 · 3 comments · Fixed by #6494
Closed

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

dbemke opened this issue Nov 4, 2024 · 3 comments · Fixed by #6494
Assignees

Comments

@dbemke
Copy link

dbemke commented Nov 4, 2024

ODK Collect version

the master version 1a10357

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
Copy link
Member

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 (#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
Copy link
Member

grzesiek2010 commented Nov 4, 2024

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
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: done
Development

Successfully merging a pull request may close this issue.

3 participants