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

Prevent opening forms that use entities during form updates, and block form updates while filling out a form that uses entities #6440

Merged
merged 50 commits into from
Oct 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
e399924
Updated ChangeLock interface
grzesiek2010 Oct 3, 2024
7b16896
Added a new error message
grzesiek2010 Oct 3, 2024
e6932be
Added failing tests for starting a form during form updates
grzesiek2010 Oct 3, 2024
da8840a
Display an error when starting a form during form updates
grzesiek2010 Oct 3, 2024
987b0d7
Moved form opening errors to one section
grzesiek2010 Oct 3, 2024
7507bb7
Added failing tests for updating forms during form filling
grzesiek2010 Oct 3, 2024
cdcdc0d
Lock forms db before opening a form
grzesiek2010 Oct 3, 2024
ca948bd
Unlock forms db on form exit
grzesiek2010 Oct 3, 2024
74c312e
Improved unlocking changelock
grzesiek2010 Oct 4, 2024
5dd8441
Display title in form opening errors
grzesiek2010 Oct 4, 2024
9845ee3
Fixed testing form opening
grzesiek2010 Oct 4, 2024
8282699
Improved Scheduler to support suspend functions
grzesiek2010 Oct 16, 2024
620cfb6
Switch context instead of using runBlocking
grzesiek2010 Oct 16, 2024
afa286f
Improved lock acquiring
grzesiek2010 Oct 16, 2024
7bcc764
Updated the error message
grzesiek2010 Oct 16, 2024
3d07439
Fixed tests
grzesiek2010 Oct 16, 2024
fbedaa0
Updated the forms db
grzesiek2010 Oct 18, 2024
e1a4f89
Added new tests
grzesiek2010 Oct 16, 2024
72ec047
Added checking new column
grzesiek2010 Oct 18, 2024
d377140
Cleaned up tests
grzesiek2010 Oct 16, 2024
404f6d6
Updated Form class
grzesiek2010 Oct 18, 2024
2710f8e
Updated DatabaseObjectMapper class
grzesiek2010 Oct 18, 2024
3f6a3ec
Updated FormMetadataParser to read entities version
grzesiek2010 Oct 18, 2024
08224e6
Improved tests
grzesiek2010 Oct 18, 2024
5fdbd7a
Save entitiesversion when syncing with storage
grzesiek2010 Oct 18, 2024
edf288a
Save entitiesversion when downloading a form
grzesiek2010 Oct 18, 2024
656c08a
Fixed FormsProviderTest
grzesiek2010 Oct 18, 2024
1fb335c
Added failing tests
grzesiek2010 Oct 18, 2024
4d9be52
Allow opening forms when forms db is locked but they do not use entities
grzesiek2010 Oct 18, 2024
49e9e0b
Replace ReentrantChangeLock with ThreadSafeBooleanChangeLock
grzesiek2010 Oct 23, 2024
f787711
Remove redundant code
grzesiek2010 Oct 23, 2024
ea7e3f6
Revert "Improved Scheduler to support suspend functions"
grzesiek2010 Oct 23, 2024
61c9dd7
Code improvements in BooleanChangeLock
grzesiek2010 Oct 24, 2024
8e12297
Removed redundant dependencies
grzesiek2010 Oct 24, 2024
9529b97
Fixed duplicated lock acquiring
grzesiek2010 Oct 24, 2024
fca31ad
Improved tests
grzesiek2010 Oct 24, 2024
9c171e2
Removed redundant breaks
grzesiek2010 Oct 24, 2024
6932f09
Fixed the problem with missing cursor columns
grzesiek2010 Oct 24, 2024
d51935d
Fixed FormDatabaseMigratorTest
grzesiek2010 Oct 24, 2024
7e8fe3b
Updated comments to make them clearer
grzesiek2010 Oct 25, 2024
5f8f639
Updated the new column name
grzesiek2010 Oct 25, 2024
ec60681
Updated the form parser
grzesiek2010 Oct 25, 2024
5046168
Check if forms contain entity attachemnts as well
grzesiek2010 Oct 25, 2024
c135792
Added new tests
grzesiek2010 Oct 25, 2024
c4e1d30
Updated detecting forms with entities in FormUriActivity
grzesiek2010 Oct 25, 2024
39551d5
Only unlock lock if they were locked succesfully
grzesiek2010 Oct 25, 2024
1bd04b2
Removed redundant BooleanChangeLock
grzesiek2010 Oct 26, 2024
c1ea862
Added tests for ThreadSafeBooleanChangeLock
grzesiek2010 Oct 26, 2024
6cceed3
Keep ChangeLock inteface and its only implementation in one file
grzesiek2010 Oct 26, 2024
cc46949
Added a new test
grzesiek2010 Oct 26, 2024
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 @@ -226,4 +226,14 @@ class EntityFormTest {
.assertTextInDialog(R.string.unrecognized_entity_version, "2020.1.0")
.clickOKOnDialog(MainMenuPage())
}

@Test
fun closingEntityForm_releasesTheLockAndLetsOtherEntityFormsToBeStarted() {
rule.startAtFirstLaunch()
.clickTryCollect()
.copyForm("one-question-entity-registration.xml")
.startBlankForm("One Question Entity Registration")
.pressBackAndDiscardForm()
.startBlankForm("One Question Entity Registration")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -182,4 +182,45 @@ class MatchExactlyTest {

assertThat(testDependencies.scheduler.getDeferredTasks(), `is`(empty()))
}

@Test
fun whenMatchExactlyEnabled_getsLatestFormsFromServerDuringFillingAFormWithoutEntities() {
testDependencies.server.addForm(
"One Question Updated",
"one_question",
"2",
"one-question-updated.xml"
)

rule.startAtMainMenu()
.copyForm("one-question.xml")
.setServer(testDependencies.server.url)
.enableMatchExactly()
.startBlankForm("One Question")
.also { testDependencies.scheduler.runDeferredTasks() }
.pressBackAndDiscardForm()
.clickFillBlankForm()
.assertFormExists("One Question Updated")
}

@Test
fun whenMatchExactlyEnabled_doesNotGetLatestFormsFromServerDuringFillingAFormWithEntities() {
testDependencies.server.addForm(
"One Question Updated",
"one_question",
"2",
"one-question-updated.xml"
)

rule.startAtMainMenu()
.copyForm("one-question-entity-registration.xml")
.copyForm("one-question.xml")
.setServer(testDependencies.server.url)
.enableMatchExactly()
.startBlankForm("One Question Entity Registration")
.also { testDependencies.scheduler.runDeferredTasks() }
.pressBackAndDiscardForm()
.clickFillBlankForm()
.assertFormDoesNotExist("One Question Updated")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -156,4 +156,40 @@ class PreviouslyDownloadedOnlyTest {

assertThat(testDependencies.scheduler.getDeferredTasks(), equalTo(emptyList()))
}

@Test
fun whenPreviouslyDownloadedOnlyEnabledWithAutomaticDownload_getsLatestFormsFromServerDuringFillingAFormWithoutEntities() {
testDependencies.server.addForm(
"One Question Updated",
"one_question",
"2",
"one-question-updated.xml"
)

rule.withProject(testDependencies.server, "one-question.xml")
.enablePreviouslyDownloadedOnlyUpdatesWithAutomaticDownload()
.startBlankForm("One Question")
.also { testDependencies.scheduler.runDeferredTasks() }
.pressBackAndDiscardForm()
.clickFillBlankForm()
.assertFormExists("One Question Updated")
}

@Test
fun whenPreviouslyDownloadedOnlyEnabledWithAutomaticDownload_doesNotGetLatestFormsFromServerDuringFillingAFormWithEntities() {
testDependencies.server.addForm(
"One Question Updated",
"one_question",
"2",
"one-question-updated.xml"
)

rule.withProject(testDependencies.server, "one-question-entity-registration.xml", "one-question.xml")
.enablePreviouslyDownloadedOnlyUpdatesWithAutomaticDownload()
.startBlankForm("One Question Entity Registration")
.also { testDependencies.scheduler.runDeferredTasks() }
.pressBackAndDiscardForm()
.clickFillBlankForm()
.assertFormDoesNotExist("One Question Updated")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import org.odk.collect.android.instancemanagement.InstancesDataService
import org.odk.collect.android.instancemanagement.autosend.AutoSendSettingsProvider
import org.odk.collect.android.projects.ProjectsDataService
import org.odk.collect.android.utilities.ApplicationConstants
import org.odk.collect.android.utilities.ChangeLockProvider
import org.odk.collect.android.utilities.FormsRepositoryProvider
import org.odk.collect.android.utilities.InstancesRepositoryProvider
import org.odk.collect.android.utilities.MediaUtils
Expand Down Expand Up @@ -57,7 +58,8 @@ class FormEntryViewModelFactory(
private val savepointsRepositoryProvider: SavepointsRepositoryProvider,
private val qrCodeCreator: QRCodeCreator,
private val htmlPrinter: HtmlPrinter,
private val instancesDataService: InstancesDataService
private val instancesDataService: InstancesDataService,
private val changeLockProvider: ChangeLockProvider
) : AbstractSavedStateViewModelFactory(owner, null) {

override fun <T : ViewModel> create(
Expand All @@ -73,7 +75,8 @@ class FormEntryViewModelFactory(
scheduler,
formSessionRepository,
sessionId,
formsRepositoryProvider.create(projectId)
formsRepositoryProvider.create(projectId),
changeLockProvider.create(projectId)
)

FormSaveViewModel::class.java -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@
import org.odk.collect.android.formhierarchy.FormHierarchyActivity;
import org.odk.collect.android.formhierarchy.ViewOnlyFormHierarchyActivity;
import org.odk.collect.android.fragments.MediaLoadingFragment;
import org.odk.collect.android.utilities.ChangeLockProvider;
import org.odk.collect.android.widgets.datetime.pickers.CustomDatePickerDialog;
import org.odk.collect.android.widgets.datetime.pickers.CustomTimePickerDialog;
import org.odk.collect.android.fragments.dialogs.LocationProvidersDisabledDialog;
Expand Down Expand Up @@ -366,6 +367,9 @@ public void allowSwiping(boolean doSwipe) {
@Inject
public SavepointsRepositoryProvider savepointsRepositoryProvider;

@Inject
public ChangeLockProvider changeLockProvider;

private final LocationProvidersReceiver locationProvidersReceiver = new LocationProvidersReceiver();

private SwipeHandler swipeHandler;
Expand Down Expand Up @@ -434,7 +438,8 @@ public void onCreate(Bundle savedInstanceState) {
savepointsRepositoryProvider,
new QRCodeCreatorImpl(),
new HtmlPrinter(),
instancesDataService
instancesDataService,
changeLockProvider
);

this.getSupportFragmentManager().setFragmentFactory(new FragmentFactoryBuilder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ public final class DatabaseConstants {
public static final String FORMS_DATABASE_NAME = "forms.db";
public static final String FORMS_TABLE_NAME = "forms";
// Please always test upgrades manually when you change this value
public static final int FORMS_DATABASE_VERSION = 13;
public static final int FORMS_DATABASE_VERSION = 14;

public static final String INSTANCES_DATABASE_NAME = "instances.db";
public static final String INSTANCES_TABLE_NAME = "instances";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,46 +35,10 @@ object DatabaseObjectMapper {
values.put(DatabaseFormColumns.AUTO_DELETE, form.autoDelete)
values.put(DatabaseFormColumns.GEOMETRY_XPATH, form.geometryXpath)
values.put(DatabaseFormColumns.LAST_DETECTED_ATTACHMENTS_UPDATE_DATE, form.lastDetectedAttachmentsUpdateDate)
values.put(DatabaseFormColumns.USES_ENTITIES, Boolean.toString(form.usesEntities()))
return values
}

@JvmStatic
fun getFormFromValues(values: ContentValues, formsPath: String, cachePath: String): Form {
val formFilePath = getAbsoluteFilePath(
formsPath,
values.getAsString(DatabaseFormColumns.FORM_FILE_PATH)
)

val cacheFilePath = values.getAsString(DatabaseFormColumns.JRCACHE_FILE_PATH)?.let {
getAbsoluteFilePath(cachePath, it)
}

val mediaPath = values.getAsString(DatabaseFormColumns.FORM_MEDIA_PATH)?.let {
getAbsoluteFilePath(formsPath, it)
}

return Form.Builder()
.dbId(values.getAsLong(BaseColumns._ID))
.displayName(values.getAsString(DatabaseFormColumns.DISPLAY_NAME))
.description(values.getAsString(DatabaseFormColumns.DESCRIPTION))
.formId(values.getAsString(DatabaseFormColumns.JR_FORM_ID))
.version(values.getAsString(DatabaseFormColumns.JR_VERSION))
.formFilePath(formFilePath)
.submissionUri(values.getAsString(DatabaseFormColumns.SUBMISSION_URI))
.base64RSAPublicKey(values.getAsString(DatabaseFormColumns.BASE64_RSA_PUBLIC_KEY))
.md5Hash(values.getAsString(DatabaseFormColumns.MD5_HASH))
.date(values.getAsLong(DatabaseFormColumns.DATE))
.jrCacheFilePath(cacheFilePath)
.formMediaPath(mediaPath)
.language(values.getAsString(DatabaseFormColumns.LANGUAGE))
.autoSend(values.getAsString(DatabaseFormColumns.AUTO_SEND))
.autoDelete(values.getAsString(DatabaseFormColumns.AUTO_DELETE))
.geometryXpath(values.getAsString(DatabaseFormColumns.GEOMETRY_XPATH))
.deleted(values.getAsLong(DatabaseFormColumns.DELETED_DATE) != null)
.lastDetectedAttachmentsUpdateDate(values.getAsLong(DatabaseFormColumns.LAST_DETECTED_ATTACHMENTS_UPDATE_DATE))
.build()
}

@JvmStatic
fun getFormFromCurrentCursorPosition(
cursor: Cursor,
Expand All @@ -101,6 +65,7 @@ object DatabaseObjectMapper {
val geometryXpathColumnIndex = cursor.getColumnIndex(DatabaseFormColumns.GEOMETRY_XPATH)
val deletedDateColumnIndex = cursor.getColumnIndex(DatabaseFormColumns.DELETED_DATE)
val lastDetectedAttachmentsUpdateDateColumnIndex = cursor.getColumnIndex(DatabaseFormColumns.LAST_DETECTED_ATTACHMENTS_UPDATE_DATE)
val usesEntitiesColumnIndex = cursor.getColumnIndex(DatabaseFormColumns.USES_ENTITIES)
return Form.Builder()
.dbId(cursor.getLong(idColumnIndex))
.displayName(cursor.getString(displayNameColumnIndex))
Expand Down Expand Up @@ -135,6 +100,7 @@ object DatabaseObjectMapper {
.geometryXpath(cursor.getString(geometryXpathColumnIndex))
.deleted(!cursor.isNull(deletedDateColumnIndex))
.lastDetectedAttachmentsUpdateDate(if (cursor.isNull(lastDetectedAttachmentsUpdateDateColumnIndex)) null else cursor.getLong(lastDetectedAttachmentsUpdateDateColumnIndex))
.usesEntities(Boolean.valueOf(cursor.getString(usesEntitiesColumnIndex)))
.build()
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ object DatabaseFormColumns : BaseColumns {
const val SUBMISSION_URI = "submissionUri" // can be null
const val BASE64_RSA_PUBLIC_KEY = "base64RsaPublicKey" // can be null
const val AUTO_DELETE = "autoDelete" // can be null
const val USES_ENTITIES = "usesEntities" // can be null

// Column is called autoSubmit for legacy support but the attribute is auto-send
const val AUTO_SEND = "autoSubmit" // can be null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,24 @@
import static org.odk.collect.android.database.DatabaseConstants.FORMS_TABLE_NAME;
import static org.odk.collect.android.database.DatabaseObjectMapper.getFormFromCurrentCursorPosition;
import static org.odk.collect.android.database.DatabaseObjectMapper.getValuesFromForm;
import static org.odk.collect.android.database.forms.DatabaseFormColumns.AUTO_DELETE;
import static org.odk.collect.android.database.forms.DatabaseFormColumns.AUTO_SEND;
import static org.odk.collect.android.database.forms.DatabaseFormColumns.BASE64_RSA_PUBLIC_KEY;
import static org.odk.collect.android.database.forms.DatabaseFormColumns.DATE;
import static org.odk.collect.android.database.forms.DatabaseFormColumns.DELETED_DATE;
import static org.odk.collect.android.database.forms.DatabaseFormColumns.DESCRIPTION;
import static org.odk.collect.android.database.forms.DatabaseFormColumns.DISPLAY_NAME;
import static org.odk.collect.android.database.forms.DatabaseFormColumns.USES_ENTITIES;
import static org.odk.collect.android.database.forms.DatabaseFormColumns.FORM_FILE_PATH;
import static org.odk.collect.android.database.forms.DatabaseFormColumns.FORM_MEDIA_PATH;
import static org.odk.collect.android.database.forms.DatabaseFormColumns.GEOMETRY_XPATH;
import static org.odk.collect.android.database.forms.DatabaseFormColumns.JRCACHE_FILE_PATH;
import static org.odk.collect.android.database.forms.DatabaseFormColumns.JR_FORM_ID;
import static org.odk.collect.android.database.forms.DatabaseFormColumns.JR_VERSION;
import static org.odk.collect.android.database.forms.DatabaseFormColumns.LANGUAGE;
import static org.odk.collect.android.database.forms.DatabaseFormColumns.LAST_DETECTED_ATTACHMENTS_UPDATE_DATE;
import static org.odk.collect.android.database.forms.DatabaseFormColumns.MD5_HASH;
import static org.odk.collect.android.database.forms.DatabaseFormColumns.SUBMISSION_URI;
import static org.odk.collect.shared.PathUtils.getRelativeFilePath;

import android.content.ContentValues;
Expand Down Expand Up @@ -230,6 +240,37 @@ private Cursor queryAndReturnCursor(Map<String, String> projectionMap, String[]
SQLiteQueryBuilder qb = new SQLiteQueryBuilder();
qb.setTables(FORMS_TABLE_NAME);

if (projection == null) {
/*
For some reason passing null as the projection doesn't always give us all the
columns so we hardcode them here so it's explicit that we need these all back.
The problem can occur, for example, when a new column is added to a database and the
database needs to be updated. After the upgrade, the new column might not be returned,
even though it already exists.
*/
projection = new String[]{
_ID,
DISPLAY_NAME,
DESCRIPTION,
JR_FORM_ID,
JR_VERSION,
MD5_HASH,
DATE,
FORM_MEDIA_PATH,
FORM_FILE_PATH,
LANGUAGE,
SUBMISSION_URI,
BASE64_RSA_PUBLIC_KEY,
JRCACHE_FILE_PATH,
AUTO_SEND,
AUTO_DELETE,
GEOMETRY_XPATH,
DELETED_DATE,
LAST_DETECTED_ATTACHMENTS_UPDATE_DATE,
USES_ENTITIES
};
}

if (projectionMap != null) {
qb.setProjectionMap(projectionMap);
}
Expand Down
Loading