Skip to content

Commit

Permalink
Merge pull request #6341 from seadowg/instance-provider
Browse files Browse the repository at this point in the history 
Don't allow inserting instances with custom submission URI
  • Loading branch information
@seadowg
seadowg authored Aug 16, 2024
2 parents 669ccde + 9f93bcb commit a6c9514
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 154 deletions.
68 changes: 2 additions & 66 deletions collect_app/src/main/java/org/odk/collect/android/external/FormsProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions collect_app/src/main/java/org/odk/collect/android/external/InstanceProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
}
Expand Down
130 changes: 42 additions & 88 deletions collect_app/src/test/java/org/odk/collect/android/external/FormsProviderTest.java
View file
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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");
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down
15 changes: 15 additions & 0 deletions collect_app/src/test/java/org/odk/collect/android/external/InstanceProviderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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");
Expand Down

0 comments on commit a6c9514

Please sign in to comment.