Reworked savepoints to support recovering from older form versions

[grzesiek2010]

Closes #4879
Closes #3494

Why is this the best possible solution? Were any other approaches considered?

The original implementation of handling savepoints was based on files saved in the cache directory (a savepoint file is a normal XML file just like we store saved forms and send them to a server). Let me explain the process of creating and then identifying such a savepoint. For the purpose of this example let's use the demo form called All widgets.

1. Every time a user starts a new form (the All widgets in this case) we create an empty instance folder that consists of the form name and the date of creation and might look like All widgets_2024-02-29_09-27-51. This value is saved to the FormController as the instance name.
2. When we have to create a savepoint file for such a form (for example when a sure navigates in a form or the app is killed), we create a file in the cache directory with a name that is based on the instance name mentioned above. In this case, the savepoint file would be named All widgets_2024-02-29_09-27-51.xml.save.
3. When a user opens the blank All widgets again we try to figure out if there is a savepoint for this form and if it exists we want to load it. The problem is that we don't have enough data to correctly identify a savepoint that belongs to our form so we try to do that in a pretty cumbersome way:
   • We don't have any instance saved in the instances.db because we were filling a blank form so lost that date of creation that was used to create the instance name and was a second part of the savepoint file name.
   • The only thing we know is our form is called All widgets so we browse the cache directory looking for a file that has a name starting with All widgets and ending with .xml.save completely ignoring the part of the name that was built using the date of creation.
   • In our case All widgets_2024-02-29_09-27-51.xml.save matches so we take it and based on this file name we try to find the instance directory (that at this moment is empty but we want to continue using it) that was saved when we started the form for the first time (before creating the savepoint), what I have described in the first point.

We have only supported recovering savepoints for the newest form versions so it worked. However, it was cumbersome enough. Now as we want to support recovering savepoints from older versions too, looking for savepoint files that belong to older versions of a form that we are opening in a similar way would be a spaghetti code.
Taking all of that into account I decided we need a database to keep all the needed information in a clear and structured way. Thanks to this database based on our form id and version we can easily figure out if there is a savepoint for our form or not. If it exists we can read its full name instead of looping the whole cache dir and try to find the file we want based on its prefix and suffix.

This pull request does not contain any mechanism for data migration. This will be implemented separately #5997.

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

We need to test opening blank and saved forms with and without savepoints to make sure everything works fine. There are a lot of combinations if there are multiple versions of the same form so it will be fun. We can chat about it as well so that I can tell you more about this issue.

Do we need any specific form for testing your changes? If so, please attach one.

No, it can be any form.

Does this change require updates to documentation? If so, please file an issue here and include the link below.

Yes getodk/docs#1749

Before submitting this PR, please make sure you have:

• added or modified tests for any new or changed behavior
• run ./gradlew connectedAndroidTest (or ./gradlew testLab) and confirmed all checks still pass
• added a comment above any new strings describing it for translators
• verified that any code or assets from external sources are properly credited in comments and/or in the about file.
• verified that any new UI elements use theme colors. UI Components Style guidelines

[grzesiek2010]

@seadowg you have assigned me to #3494 as well. Is there anything particular that should be done as part of that issue that is not in the scope of #4879? As I understand there is nothing like that.

[seadowg]

@seadowg you have assigned me to #3494 as well. Is there anything particular that should be done as part of that issue that is not in the scope of #4879?

Nope! The fact that we now always prompt about save points is enough.

[seadowg]

I'm really liking the switch to a DB backed repo for save points.

Some high level thoughts from my initial look through (plus one inline):

1. It looks like we load the save point in FormUriActivity and then again in FormLoaderTask. Could we just pass some indication to use the save point from FormUriActivity to FormFillingActivity (the SavePoint itself is small enough to parcelize for example) to avoid loading it twice?
2. It looks to me like bulk finalization will still use the old way of loading save points using FormEntryUseCases.getSavePoint(). I think it should be easy enough to fix that by either using SavePointFinder there instead or (and I think in a way that I prefer) replace FormEntryUseCases.getSavePoint()'s implementation with SavePointFinder#getSavePoint and remove SavePointFinder.

[grzesiek2010]

It looks like we load the save point in FormUriActivity and then again in FormLoaderTask. Could we just pass some indication to use the save point from FormUriActivity to FormFillingActivity (the SavePoint itself is small enough to parcelize for example) to avoid loading it twice?

This was my initial plan too but it would make dealing with savepoints more complex. Imagine a scenario where:

1. You start a form that has a savepoint and you click Recover
2. You add some changes and save the form (this removes the savepoint)
3. You put the app in the background and when you reopen it the activity needs to be recreated

In such a scenario the savepoint object that represents a non-existing savepoint will be passed to the activity and used.
To handle this scenario we would need to always check if such an object passed from FormUriActivity to FormFillingActivity represents a savepoint that really exists. It's doable but I think always reading it from the database is simpler.

[grzesiek2010]

It looks to me like bulk finalization will still use the old way of loading save points using FormEntryUseCases.getSavePoint(). I think it should be easy enough to fix that by either using SavePointFinder there instead or (and I think in a way that I prefer) replace FormEntryUseCases.getSavePoint()'s implementation with SavePointFinder#getSavePoint and remove SavePointFinder.

Good catch, I've just fixed that but just with SavepointsRepository as we want to check if there is a savepoint for a particular instance (not for older versions of a blank form that was used to fill that instance). The responsibility of SavePointFinder is to find savepoints for older versions of a selected form if they exist.

[seadowg]

This was my initial plan too but it would make dealing with savepoints more complex. Imagine a scenario where:

1. You start a form that has a savepoint and you click Recover
2. You add some changes and save the form (this removes the savepoint)
3. You put the app in the background and when you reopen it the activity needs to be recreated

Great point! I think we're really hitting a point where FormUriActivity feels more awkward than useful. Now that the majority of the logic lives in FormUriViewModel, I think we should be able to move away from it (and just use the view model in FormFillingActivity) and that will reduce the duplication I'm feeling uncomfortable with (and also hopefully reduce the amount of logic in FormLoaderTask). I'll schedule some work to do that rather than block these changes (which I think just put us in an even better place to do it anyway).

[grzesiek2010]

Great point! I think we're really hitting a point where FormUriActivity feels more awkward than useful. Now that the majority of the logic lives in FormUriViewModel, I think we should be able to move away from it (and just use the view model in FormFillingActivity) and that will reduce the duplication I'm feeling uncomfortable with (and also hopefully reduce the amount of logic in FormLoaderTask). I'll schedule some work to do that rather than block these changes (which I think just put us in an even better place to do it anyway).

I agree it's been quite common recently to run into problems cased by the fact that we use two activities to start form filling.

[seadowg]

I've had another look through and added some comments. There's probably one more proper pass I need to do to review SavePointFinder and the repositories, but I've managed to get a good sense of the structure of everything now.

[seadowg]

#5951 (comment)

[dbemke]

Tested with Success!

Verified on devices with Androids: 10

Verified cases:

• savepoints after a form version update
• savepoint in Start new form and Drafts
• manual, match exactly, previously downloaded form update modes
• hide old form versions enabled and disabled
• when there 2 versions of a form opening the newest version and the second version
• savepoint in different versions of a form
• opening a savepoint from the form map
• updating a form version with and without changing fields
• savepoint in form with and without a version
• creating savepoint while audio recording, video recording
• recovery with many files attached
• update forms with different questions and different media
• recovery in entities updates forms
• deleting a project with a savepoint and adding the same project
• checking if recovery dialog doesn’t appear in other parts of the app
• clearing cache when there is a savepoint
• changing to „moving backwards not allowed” setting before opening a savepoint
• saveIncomplete, last saved forms
• don't keep activities enabled/disabled

[WKobus]

Tested with Success!

Verified on device with Android 11

[srujner]

Tested with Success!

Verified on device with Android 12,13