Merge pull request #6341 from seadowg/instance-provider

Don't allow inserting instances with custom submission URI
getodk · Aug 16, 2024 · a6c9514 · a6c9514
2 parents 669ccde + 9f93bcb
commit a6c9514
Show file tree

Hide file tree

Showing 4 changed files with 64 additions and 154 deletions.
diff --git a/collect_app/src/main/java/org/odk/collect/android/external/FormsProvider.java b/collect_app/src/main/java/org/odk/collect/android/external/FormsProvider.java
@@ -15,9 +15,6 @@
 package org.odk.collect.android.external;
 
 import static android.provider.BaseColumns._ID;
-import static org.odk.collect.android.database.DatabaseObjectMapper.getFormFromCurrentCursorPosition;
-import static org.odk.collect.android.database.DatabaseObjectMapper.getFormFromValues;
-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;
@@ -52,11 +49,9 @@
 import org.odk.collect.android.injection.DaggerUtils;
 import org.odk.collect.android.itemsets.FastExternalItemsetsRepository;
 import org.odk.collect.android.storage.StoragePathProvider;
-import org.odk.collect.android.storage.StorageSubdirectory;
 import org.odk.collect.android.utilities.ContentUriHelper;
 import org.odk.collect.android.utilities.FormsRepositoryProvider;
 import org.odk.collect.android.utilities.InstancesRepositoryProvider;
-import org.odk.collect.forms.Form;
 import org.odk.collect.forms.FormsRepository;
 import org.odk.collect.forms.instances.InstancesRepository;
 import org.odk.collect.projects.ProjectsRepository;
@@ -189,20 +184,7 @@ public String getType(@NonNull Uri uri) {
 
     @Override
     public synchronized Uri insert(@NonNull Uri uri, ContentValues initialValues) {
-        deferDaggerInit();
-
-        // Validate the requested uri
-        if (URI_MATCHER.match(uri) != FORMS) {
-            throw new IllegalArgumentException("Unknown URI " + uri);
-        }
-
-        String projectId = getProjectId(uri);
-        logServerEvent(projectId, AnalyticsEvents.FORMS_PROVIDER_INSERT);
-
-        String formsPath = storagePathProvider.getOdkDirPath(StorageSubdirectory.FORMS, projectId);
-        String cachePath = storagePathProvider.getOdkDirPath(StorageSubdirectory.CACHE, projectId);
-        Form form = getFormsRepository(projectId).save(getFormFromValues(initialValues, formsPath, cachePath));
-        return FormsContract.getUri(projectId, form.getDbId());
+        return null;
     }
 
     /**
@@ -248,53 +230,7 @@ public int delete(@NonNull Uri uri, String where, String[] whereArgs) {
 
     @Override
     public int update(Uri uri, ContentValues values, String where, String[] whereArgs) {
-        deferDaggerInit();
-
-        String projectId = getProjectId(uri);
-        logServerEvent(projectId, AnalyticsEvents.FORMS_PROVIDER_UPDATE);
-
-        FormsRepository formsRepository = getFormsRepository(projectId);
-        String formsPath = storagePathProvider.getOdkDirPath(StorageSubdirectory.FORMS, projectId);
-        String cachePath = storagePathProvider.getOdkDirPath(StorageSubdirectory.CACHE, projectId);
-
-        int count;
-
-        switch (URI_MATCHER.match(uri)) {
-            case FORMS:
-                try (Cursor cursor = databaseQuery(projectId, null, where, whereArgs, null, null, null)) {
-                    while (cursor.moveToNext()) {
-                        Form form = getFormFromCurrentCursorPosition(cursor, formsPath, cachePath);
-                        ContentValues existingValues = getValuesFromForm(form, formsPath);
-                        existingValues.putAll(values);
-
-                        formsRepository.save(getFormFromValues(existingValues, formsPath, cachePath));
-                    }
-
-                    count = cursor.getCount();
-                }
-                break;
-
-            case FORM_ID:
-                Form form = formsRepository.get(ContentUriHelper.getIdFromUri(uri));
-                if (form != null) {
-                    ContentValues existingValues = getValuesFromForm(form, formsPath);
-                    existingValues.putAll(values);
-
-                    formsRepository.save(getFormFromValues(existingValues, formsPath, cachePath));
-                    count = 1;
-                } else {
-                    count = 0;
-                }
-
-                break;
-
-            default:
-                throw new IllegalArgumentException("Unknown URI " + uri);
-        }
-
-        getContext().getContentResolver().notifyChange(uri, null);
-
-        return count;
+        return 0;
     }
 
     @NotNull

diff --git a/collect_app/src/main/java/org/odk/collect/android/external/InstanceProvider.java b/collect_app/src/main/java/org/odk/collect/android/external/InstanceProvider.java
@@ -31,6 +31,7 @@
 import org.odk.collect.android.analytics.AnalyticsEvents;
 import org.odk.collect.android.analytics.AnalyticsUtils;
 import org.odk.collect.android.dao.CursorLoaderFactory;
+import org.odk.collect.android.database.instances.DatabaseInstanceColumns;
 import org.odk.collect.android.database.instances.DatabaseInstancesRepository;
 import org.odk.collect.android.injection.DaggerUtils;
 import org.odk.collect.android.instancemanagement.InstanceDeleter;
@@ -132,6 +133,10 @@ public Uri insert(@NonNull Uri uri, ContentValues initialValues) {
             throw new IllegalArgumentException("Unknown URI " + uri);
         }
 
+        if (initialValues.containsKey(DatabaseInstanceColumns.SUBMISSION_URI)) {
+            throw new SecurityException();
+        }
+
         Instance newInstance = instancesRepositoryProvider.get(projectId).save(getInstanceFromValues(initialValues));
         return getUri(projectId, newInstance.getDbId());
     }

diff --git a/collect_app/src/test/java/org/odk/collect/android/external/FormsProviderTest.java b/collect_app/src/test/java/org/odk/collect/android/external/FormsProviderTest.java
@@ -1,5 +1,20 @@
 package org.odk.collect.android.external;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.isOneOf;
+import static org.odk.collect.android.database.forms.DatabaseFormColumns.DISPLAY_NAME;
+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.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.external.FormsContract.CONTENT_ITEM_TYPE;
+import static org.odk.collect.android.external.FormsContract.CONTENT_TYPE;
+import static org.odk.collect.android.external.FormsContract.getUri;
+
 import android.content.ContentResolver;
 import android.content.ContentValues;
 import android.content.Context;
@@ -14,140 +29,72 @@
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.odk.collect.android.injection.DaggerUtils;
+import org.odk.collect.android.injection.config.AppDependencyComponent;
 import org.odk.collect.android.storage.StoragePathProvider;
 import org.odk.collect.android.storage.StorageSubdirectory;
 import org.odk.collect.android.support.CollectHelpers;
 import org.odk.collect.android.utilities.FileUtils;
+import org.odk.collect.android.utilities.FormsRepositoryProvider;
+import org.odk.collect.forms.Form;
+import org.odk.collect.forms.FormsRepository;
 import org.odk.collect.formstest.FormUtils;
 import org.odk.collect.projects.Project;
 import org.odk.collect.shared.strings.Md5;
 
 import java.io.File;
 import java.io.IOException;
 
-import static org.hamcrest.MatcherAssert.assertThat;
-import static org.hamcrest.Matchers.is;
-import static org.hamcrest.Matchers.isOneOf;
-import static org.hamcrest.Matchers.notNullValue;
-import static org.odk.collect.android.database.forms.DatabaseFormColumns.DATE;
-import static org.odk.collect.android.database.forms.DatabaseFormColumns.DISPLAY_NAME;
-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.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.MD5_HASH;
-import static org.odk.collect.android.external.FormsContract.CONTENT_ITEM_TYPE;
-import static org.odk.collect.android.external.FormsContract.CONTENT_TYPE;
-import static org.odk.collect.android.external.FormsContract.getUri;
-
 @RunWith(AndroidJUnit4.class)
 public class FormsProviderTest {
 
     private ContentResolver contentResolver;
     private StoragePathProvider storagePathProvider;
     private String firstProjectId;
+    private AppDependencyComponent component;
 
     @Before
     public void setup() {
         Context context = ApplicationProvider.getApplicationContext();
-        storagePathProvider = DaggerUtils.getComponent(context).storagePathProvider();
+        component = DaggerUtils.getComponent(context);
+        storagePathProvider = component.storagePathProvider();
 
         firstProjectId = CollectHelpers.createDemoProject();
         contentResolver = context.getContentResolver();
     }
 
     @Test
-    public void insert_addsForm() {
+    public void insert_doesNotInsertForms_andReturnsNull() {
         String formId = "external_app_form";
         String formVersion = "1";
         String formName = "External app form";
         File formFile = addFormToFormsDir(firstProjectId, formId, formVersion, formName);
-        String md5Hash = Md5.getMd5Hash(formFile);
 
         ContentValues values = getContentValues(formId, formVersion, formName, formFile);
-        contentResolver.insert(getUri(firstProjectId), values);
+        Uri uri = contentResolver.insert(getUri(firstProjectId), values);
+        assertThat(uri, equalTo(null));
 
         try (Cursor cursor = contentResolver.query(getUri(firstProjectId), null, null, null, null)) {
-            assertThat(cursor.getCount(), is(1));
-
-            cursor.moveToNext();
-            assertThat(cursor.getString(cursor.getColumnIndex(DISPLAY_NAME)), is(formName));
-            assertThat(cursor.getString(cursor.getColumnIndex(JR_FORM_ID)), is(formId));
-            assertThat(cursor.getString(cursor.getColumnIndex(JR_VERSION)), is(formVersion));
-            assertThat(cursor.getString(cursor.getColumnIndex(FORM_FILE_PATH)), is(formFile.getName()));
-
-            assertThat(cursor.getString(cursor.getColumnIndex(DATE)), is(notNullValue()));
-            assertThat(cursor.getString(cursor.getColumnIndex(MD5_HASH)), is(md5Hash));
-            assertThat(cursor.getString(cursor.getColumnIndex(JRCACHE_FILE_PATH)), is(md5Hash + ".formdef"));
-            assertThat(cursor.getString(cursor.getColumnIndex(FORM_MEDIA_PATH)), is(mediaPathForFormFile(formFile)));
-        }
-    }
-
-    @Test
-    public void insert_returnsFormUri() {
-        String formId = "external_app_form";
-        String formVersion = "1";
-        String formName = "External app form";
-        File formFile = addFormToFormsDir(firstProjectId, formId, formVersion, formName);
-
-        ContentValues values = getContentValues(formId, formVersion, formName, formFile);
-        Uri newFormUri = contentResolver.insert(getUri(firstProjectId), values);
-
-        try (Cursor cursor = contentResolver.query(newFormUri, null, null, null, null)) {
-            assertThat(cursor.getCount(), is(1));
+            assertThat(cursor.getCount(), is(0));
         }
     }
 
     @Test
-    public void update_updatesForm_andReturns1() {
+    public void update_doesNotUpdateForms_andReturns0() {
         Uri formUri = addFormsToDirAndDb(firstProjectId, "external_app_form", "External app form", "1");
 
         ContentValues contentValues = new ContentValues();
         contentValues.put(LANGUAGE, "English");
 
         int updateCount = contentResolver.update(formUri, contentValues, null, null);
-        assertThat(updateCount, is(1));
+        assertThat(updateCount, is(0));
         try (Cursor cursor = contentResolver.query(formUri, null, null, null)) {
             assertThat(cursor.getCount(), is(1));
 
             cursor.moveToNext();
-            assertThat(cursor.getString(cursor.getColumnIndex(LANGUAGE)), is("English"));
-        }
-    }
-
-    @Test
-    public void update_withSelection_onlyUpdatesMatchingForms() {
-        addFormsToDirAndDb(firstProjectId, "form1", "Matching form", "1");
-        addFormsToDirAndDb(firstProjectId, "form2", "Not matching form", "1");
-        addFormsToDirAndDb(firstProjectId, "form3", "Matching form", "1");
-
-        ContentValues contentValues = new ContentValues();
-        contentValues.put(LANGUAGE, "English");
-
-        contentResolver.update(getUri(firstProjectId), contentValues, DISPLAY_NAME + "=?", new String[]{"Matching form"});
-        try (Cursor cursor = contentResolver.query(getUri(firstProjectId), null, null, null)) {
-            assertThat(cursor.getCount(), is(3));
-
-            cursor.moveToNext();
-            if (cursor.getString(cursor.getColumnIndex(DISPLAY_NAME)).equals("Matching form")) {
-                assertThat(cursor.getString(cursor.getColumnIndex(LANGUAGE)), is("English"));
-            } else {
-                assertThat(cursor.isNull(cursor.getColumnIndex(LANGUAGE)), is(true));
-            }
+            assertThat(cursor.getString(cursor.getColumnIndex(LANGUAGE)), equalTo(null));
         }
     }
 
-    @Test
-    public void update_whenFormDoesNotExist_returns0() {
-        ContentValues contentValues = new ContentValues();
-        contentValues.put(LANGUAGE, "English");
-
-        int updatedCount = contentResolver.update(Uri.withAppendedPath(getUri(firstProjectId), String.valueOf(1)), contentValues, null, null);
-        assertThat(updatedCount, is(0));
-    }
-
     @Test
     public void delete_deletesForm() {
         Uri formUri = addFormsToDirAndDb(firstProjectId, "form1", "Matching form", "1");
@@ -289,8 +236,19 @@ public void getType_returnsFormAndAllFormsTypes() {
 
     private Uri addFormsToDirAndDb(String projectId, String id, String name, String version) {
         File formFile = addFormToFormsDir(projectId, id, version, name);
-        ContentValues values = getContentValues(id, version, name, formFile);
-        return contentResolver.insert(getUri(projectId), values);
+
+        FormsRepositoryProvider formsRepositoryProvider = component.formsRepositoryProvider();
+        FormsRepository formsRepository = formsRepositoryProvider.get(projectId);
+        Form form = formsRepository.save(
+                new Form.Builder()
+                        .formId(id)
+                        .displayName(name)
+                        .version(version)
+                        .formFilePath(formFile.getAbsolutePath())
+                        .build()
+        );
+
+        return FormsContract.getUri(projectId, form.getDbId());
     }
 
     @NotNull
@@ -342,10 +300,6 @@ private void createExtraFormFiles(String projectId, File formFile, String md5Has
         }
     }
 
-    private String mediaPathForFormFile(File newFile) {
-        return newFile.getName().substring(0, newFile.getName().lastIndexOf(".")) + "-media";
-    }
-
     @NotNull
     private String getFormsDirPath(String projectId) {
         return storagePathProvider.getOdkDirPath(StorageSubdirectory.FORMS, projectId) + File.separator;

diff --git a/collect_app/src/test/java/org/odk/collect/android/external/InstanceProviderTest.java b/collect_app/src/test/java/org/odk/collect/android/external/InstanceProviderTest.java
@@ -13,6 +13,7 @@
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.odk.collect.android.database.forms.DatabaseFormColumns;
+import org.odk.collect.android.database.instances.DatabaseInstanceColumns;
 import org.odk.collect.android.injection.DaggerUtils;
 import org.odk.collect.android.storage.StoragePathProvider;
 import org.odk.collect.android.storage.StorageSubdirectory;
@@ -29,6 +30,7 @@
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.notNullValue;
 import static org.hamcrest.Matchers.nullValue;
+import static org.junit.Assert.fail;
 import static org.odk.collect.android.database.instances.DatabaseInstanceColumns.DELETED_DATE;
 import static org.odk.collect.android.database.instances.DatabaseInstanceColumns.DISPLAY_NAME;
 import static org.odk.collect.android.database.instances.DatabaseInstanceColumns.GEOMETRY;
@@ -80,6 +82,19 @@ public void insert_addsInstance() {
         }
     }
 
+    @Test
+    public void insert_withSubmissionUri_throwsException() {
+        ContentValues values = getContentValues("/blah", "External app form", "external_app_form", "1");
+        values.put(DatabaseInstanceColumns.SUBMISSION_URI, "https://blah.com/submission");
+
+        try {
+            contentResolver.insert(getUri(firstProjectId), values);
+            fail();
+        } catch (SecurityException e) {
+            // Expected
+        }
+    }
+
     @Test
     public void insert_returnsInstanceUri() {
         ContentValues values = getContentValues("/blah", "External app form", "external_app_form", "1");