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.

Sign up for GitHub

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

Upgrade File Access to DocumentFile for Scoped Storage Compatibility #274

Open
wants to merge 2 commits into
base: development
Choose a base branch
from
+56 −33
Open

Upgrade File Access to DocumentFile for Scoped Storage Compatibility #274

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d3bc7b2
Upgrade File Access to DocumentFile for Scoped Storage Compatibility
permission-error Feb 25, 2024
cb7ff7f
Change to document file
permission-error Feb 25, 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
19 changes: 12 additions & 7 deletions ...app/src/androidTest/java/org/opendatakit/database/service/OdkDatabaseServiceImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import android.content.ContentValues;
import android.database.Cursor;

import androidx.documentfile.provider.DocumentFile;
import androidx.test.core.app.ApplicationProvider;

import org.junit.FixMethodOrder;
Expand Down Expand Up @@ -999,7 +1000,7 @@ private void verifyRowExistsInLocalTable(int expectedNumRows, BaseTable baseTabl
@Test public void testDeleteAllCheckpointRowsWithId() {
try {

File instanceFolder = makeTemp("t3");
DocumentFile instanceFolder = makeTemp("t3");
createTeaHouses();
setSuperuser();
thInsert("t1", SavepointTypeManipulator.complete());
Expand Down Expand Up @@ -1029,7 +1030,7 @@ private void verifyRowExistsInLocalTable(int expectedNumRows, BaseTable baseTabl

try {

File instanceFolder = makeTemp("t3");
DocumentFile instanceFolder = makeTemp("t3");
createTeaHouses();
setSuperuser();
thInsert("t3", SavepointTypeManipulator.incomplete());
Expand Down Expand Up @@ -1563,17 +1564,21 @@ private static List<KeyValueStoreEntry> makeMetadata() {
return l;
}

private static File makeTemp(String id) throws Exception {
private static DocumentFile makeTemp(String id) throws Exception {
File instanceFolder = new File(
ODKFileUtils.getInstanceFolder(APPNAME, TEA_HOUSES_TBL_NAME, id));
ODKFileUtils.getInstanceFolder(APPNAME, TEA_HOUSES_TBL_NAME, id));
if (!instanceFolder.exists() && !instanceFolder.mkdirs()) {
throw new Exception("Should have been able to create " + instanceFolder.getPath());
}
assertTrue(instanceFolder.exists());
FileOutputStream f = new FileOutputStream(new File(instanceFolder.getPath() + "/temp"));
f.write(new byte[] { 97, 98, 99 });
File tempFile = new File(instanceFolder, "temp");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wbrunette what do you think, please. Although he converted the File object to a DocumentFile in line 1580

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are correct,

There are other ways to create a DocumentFile such as fromSingleUri

OutputStream outputStream = context.getContentResolver().openOutputStream(newFile.getUri());
if (outputStream != null) {
outputStream.write(content.getBytes());
outputStream.close();
}

FileOutputStream f = new FileOutputStream(tempFile);
f.write(new byte[]{97, 98, 99});
f.close();
return instanceFolder;

// Convert the File object to a DocumentFile
DocumentFile documentFile = DocumentFile.fromFile(tempFile);
return documentFile;
}

private void insertMetadata(String table, String partition, String aspect, String key,
Expand Down
25 changes: 18 additions & 7 deletions ...droidTest/java/org/opendatakit/services/sync/service/logic/AggregateSynchronizerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import android.app.Application;
import android.content.Context;

import androidx.documentfile.provider.DocumentFile;
import androidx.test.InstrumentationRegistry;
import androidx.test.filters.Suppress;
import androidx.test.rule.GrantPermissionRule;
Expand Down Expand Up @@ -290,14 +291,16 @@ public void testGetAppLevelFileManifestWithNoFiles_ExpectPass() {
try {
AggregateSynchronizer synchronizer = new AggregateSynchronizer(sharedContext);

DocumentFile[] nullFiles = null;

FileManifestDocument tableManifestEntries =
synchronizer.getAppLevelFileManifest(null, null, true);
synchronizer.getAppLevelFileManifest(String.valueOf(nullFiles), null, true);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why did you use this String.valueOf(nullFiles), please?


assertEquals(0, tableManifestEntries.entries.size());

} catch (Exception e) {
fail("testGetAppLevelFileManifestWithNoFiles_ExpectPass: expected pass but got exception");
e.printStackTrace();

}
}

Expand Down Expand Up @@ -417,6 +420,8 @@ public void testDeleteConfigFile_ExpectPass() {
// Now copy this file over to the correct place on the device
FileUtils.writeStringToFile(destFile, "This is a test", CharsetConsts.UTF_8);

DocumentFile documentFile = DocumentFile.fromFile(destFile);

synchronizer.uploadConfigFile(destFile);

synchronizer.deleteConfigFile(destFile);
Expand Down Expand Up @@ -1099,14 +1104,17 @@ public void testUploadInstanceFile_ExpectPass() {
}

RowOutcomeList rowOutList = synchronizer.pushLocalRows(testTableRes, orderedColumns,
listOfRowsToCreate);
listOfRowsToCreate);

String destDir = ODKFileUtils.getInstanceFolder(appName, testTableId, rowId);

File destFile = new File(destDir, fileName);

FileUtils.writeStringToFile(destFile, "This is a test", CharsetConsts.UTF_8);

// Convert destFile to DocumentFile
DocumentFile documentFile = DocumentFile.fromFile(destFile);

CommonFileAttachmentTerms cat1 = synchronizer.createCommonFileAttachmentTerms(testTableRes.getInstanceFilesUri(),
testTableId, rowId, ODKFileUtils.asRowpathUri(appName, testTableId, rowId, destFile));

Expand Down Expand Up @@ -1172,8 +1180,7 @@ public void testDownloadFile_ExpectPass() {
listOfRowsToCreate.add(new TypedRow(row, orderedColumns));
}

RowOutcomeList rowOutList = synchronizer.pushLocalRows(testTableRes, orderedColumns,
listOfRowsToCreate);
RowOutcomeList rowOutList = synchronizer.pushLocalRows(testTableRes, orderedColumns, listOfRowsToCreate);

String fileName = "testFile.txt";

Expand All @@ -1184,7 +1191,7 @@ public void testDownloadFile_ExpectPass() {
FileUtils.writeStringToFile(destFile, "This is a test", CharsetConsts.UTF_8);

CommonFileAttachmentTerms cat1 = synchronizer.createCommonFileAttachmentTerms(testTableRes.getInstanceFilesUri(),
testTableId, rowId, ODKFileUtils.asRowpathUri(appName, testTableId, rowId, destFile));
testTableId, rowId, ODKFileUtils.asRowpathUri(appName, testTableId, rowId, destFile));

synchronizer.uploadInstanceFile(destFile, cat1.instanceFileDownloadUri);

Expand All @@ -1193,6 +1200,10 @@ public void testDownloadFile_ExpectPass() {
String destDir2 = ODKFileUtils.getInstanceFolder(appName, testTableId, rowId);

File destFile2 = new File(destDir2, fileName2);

// Convert destFile2 to DocumentFile
DocumentFile documentFile = DocumentFile.fromFile(destFile2);

synchronizer.downloadFile(destFile2, cat1.instanceFileDownloadUri);

synchronizer.deleteTable(testTableRes);
Expand Down Expand Up @@ -1384,4 +1395,4 @@ public void testDownloadBatch_ExpectPass() {
fail("testDownloadBatch_ExpectPass: expected pass but got exception");
}
}
}
}
8 changes: 5 additions & 3 deletions services_app/src/androidTest/java/org/opendatakit/utilities/AbstractPermissionsTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
import android.Manifest;
import android.content.ContentValues;
import android.database.Cursor;

import androidx.documentfile.provider.DocumentFile;
import androidx.test.rule.GrantPermissionRule;
import android.util.Log;
import com.fasterxml.jackson.core.JsonProcessingException;
Expand Down Expand Up @@ -416,9 +418,9 @@ protected ContentValues buildUnprivilegedInsertableRowContent(String tableId) {
cvValues.put("col4", "this is a test"); // string
// string with 500 varchars allocated to it
cvValues.put("col5", "and a long string test"); // string(500)
File configFile = new File(
ODKFileUtils.getAssetsCsvInstanceFolder(getAppName(), tableId, rowIdFullCommon),
"sample.jpg");
DocumentFile configFile = DocumentFile.fromFile(
new File(ODKFileUtils.getAssetsCsvInstanceFolder(getAppName(), tableId, rowIdFullCommon),
"sample.jpg"));
cvValues.put("col6", ODKFileUtils.asConfigRelativePath(getAppName(), configFile)); // configpath
File rowFile = new File(ODKFileUtils.getInstanceFolder(getAppName(), tableId, rowIdFullCommon), "sample.jpg");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you skipped this

try {
Expand Down
1 change: 1 addition & 0 deletions ...pp/src/androidTest/java/org/opendatakit/webkitserver/service/OdkWebserverServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import java.nio.charset.StandardCharsets;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

/**
Expand Down
3 changes: 2 additions & 1 deletion .../src/main/java/org/opendatakit/services/preferences/fragments/DeviceSettingsFragment.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import android.provider.MediaStore;
import android.widget.Toast;

import androidx.documentfile.provider.DocumentFile;
import androidx.loader.app.LoaderManager;
import androidx.preference.CheckBoxPreference;
import androidx.preference.ListPreference;
Expand Down Expand Up @@ -290,7 +291,7 @@ public void onClick(DialogInterface dialog, int item) {
}

if (newMedia.exists()) {
String appRelativePath = ODKFileUtils.asRelativePath(props.getAppName(), newMedia);
String appRelativePath = ODKFileUtils.asRelativePath(props.getAppName(), DocumentFile.fromFile(newMedia));

props.setProperties(Collections.singletonMap(CommonToolProperties
.KEY_SPLASH_PATH, appRelativePath));
Expand Down
7 changes: 4 additions & 3 deletions ..._app/src/main/java/org/opendatakit/services/sync/service/logic/AggregateSynchronizer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
*/
package org.opendatakit.services.sync.service.logic;

import androidx.documentfile.provider.DocumentFile;

import org.apache.commons.fileupload.MultipartStream;
import org.opendatakit.aggregate.odktables.rest.SyncState;
import org.opendatakit.aggregate.odktables.rest.entity.AppNameList;
Expand Down Expand Up @@ -438,7 +440,6 @@ public ChangeSetList getChangeSets(TableResource table, String dataETag) throws
}
}


@Override
public RowResourceList getChangeSet(TableResource table, String dataETag, boolean activeOnly, String websafeResumeCursor)
throws HttpClientWebException, IOException {
Expand Down Expand Up @@ -890,7 +891,7 @@ public void downloadFile(File destFile, URI downloadUrl) throws HttpClientWebExc
@Override
public void deleteConfigFile(File localFile) throws HttpClientWebException, IOException {
String pathRelativeToConfigFolder = ODKFileUtils.asConfigRelativePath(sc.getAppName(),
localFile);
DocumentFile.fromFile(localFile));
URI filesUri = wrapper.constructConfigFileUri(pathRelativeToConfigFolder);
log.i(LOGTAG, "CLARICE:[deleteConfigFile] fileDeleteUri: " + filesUri.toString());

Expand All @@ -911,7 +912,7 @@ public void deleteConfigFile(File localFile) throws HttpClientWebException, IOEx
@Override
public void uploadConfigFile(File localFile) throws HttpClientWebException, IOException {
String pathRelativeToConfigFolder = ODKFileUtils.asConfigRelativePath(sc.getAppName(),
localFile);
DocumentFile.fromFile(localFile));
URI filesUri = wrapper.constructConfigFileUri(pathRelativeToConfigFolder);
log.i(LOGTAG, "[uploadConfigFile] filePostUri: " + filesUri.toString());
String ct = HttpRestProtocolWrapper.determineContentType(localFile.getName());
Expand Down
26 changes: 14 additions & 12 deletions ...ava/org/opendatakit/services/sync/service/logic/ProcessManifestContentAndFileChanges.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
*/
package org.opendatakit.services.sync.service.logic;

import androidx.documentfile.provider.DocumentFile;

import org.opendatakit.aggregate.odktables.rest.entity.OdkTablesFileManifestEntry;
import org.opendatakit.database.data.ColumnDefinition;
import org.opendatakit.database.service.DbHandle;
Expand Down Expand Up @@ -114,13 +116,13 @@ private List<String> getAppLevelFiles() {

LinkedList<File> unexploredDirs = new LinkedList<File>();
List<String> relativePaths = new ArrayList<String>();

unexploredDirs.add(baseFolder);

boolean haveFilteredTablesDir = false;
boolean haveFilteredAssetsCsvDir = false;
boolean haveFilteredTableInitFile = false;

while (!unexploredDirs.isEmpty()) {
File exploring = unexploredDirs.removeFirst();
File[] files = exploring.listFiles();
Expand Down Expand Up @@ -158,7 +160,7 @@ private List<String> getAppLevelFiles() {
}

// we'll add it to our list of files.
relativePaths.add(ODKFileUtils.asRelativePath(sc.getAppName(), f));
relativePaths.add(ODKFileUtils.asRelativePath(sc.getAppName(), DocumentFile.fromFile(f)));
}
}
}
Expand Down Expand Up @@ -199,7 +201,7 @@ private static List<String> filterInTableIdFiles(List<String> relativePaths, Str
* <p>
* If the baseFolder exists but is not a directory, logs an error and returns an
* empty list.
*
*
* @param baseFolder
* @param excludingNamedItemsUnderFolder
* can be null--nothing will be excluded. Should be relative to the
Expand Down Expand Up @@ -262,7 +264,7 @@ public boolean accept(File pathname) {
// we want the relative path, so drop the necessary bets.
for (File f : nondirFiles) {
// +1 to exclude the separator.
relativePaths.add(ODKFileUtils.asRelativePath(sc.getAppName(), f));
relativePaths.add(ODKFileUtils.asRelativePath(sc.getAppName(), DocumentFile.fromFile(f)));
}
return relativePaths;
}
Expand Down Expand Up @@ -323,7 +325,7 @@ public void syncAppLevelFiles(boolean pushLocalFiles, String serverReportedAppLe
serverFilesToDelete.add(localFile);
} else if (ODKFileUtils.getMd5Hash(sc.getAppName(), localFile).equals(entry.md5hash)) {
// we are ok -- no need to upload or delete
relativePathsOnDevice.remove(ODKFileUtils.asRelativePath(sc.getAppName(), localFile));
relativePathsOnDevice.remove(ODKFileUtils.asRelativePath(sc.getAppName(), DocumentFile.fromFile(localFile)));
}
}

Expand All @@ -345,7 +347,7 @@ public void syncAppLevelFiles(boolean pushLocalFiles, String serverReportedAppLe

for (File localFile : serverFilesToDelete) {

String relativePath = ODKFileUtils.asRelativePath(sc.getAppName(), localFile);
String relativePath = ODKFileUtils.asRelativePath(sc.getAppName(), DocumentFile.fromFile(localFile));
syncStatus.updateNotification(SyncProgressState.APP_FILES,
R.string.sync_deleting_file_on_server,
new Object[] { relativePath }, stepCount * stepSize, false);
Expand All @@ -362,7 +364,7 @@ public void syncAppLevelFiles(boolean pushLocalFiles, String serverReportedAppLe

for (OdkTablesFileManifestEntry entry : manifestDocument.entries) {
File localFile = ODKFileUtils.asConfigFile(sc.getAppName(), entry.filename);
String relativePath = ODKFileUtils.asRelativePath(sc.getAppName(), localFile);
String relativePath = ODKFileUtils.asRelativePath(sc.getAppName(), DocumentFile.fromFile(localFile));

syncStatus.updateNotification(SyncProgressState.APP_FILES,
R.string.sync_verifying_local_file,
Expand Down Expand Up @@ -455,7 +457,7 @@ public void syncTableLevelFiles(String tableId, String serverReportedTableLevelE
return;
}
String tableIdPropertiesFile = ODKFileUtils.asRelativePath(sc.getAppName(),
new File(ODKFileUtils.getTablePropertiesCsvFile(sc.getAppName(), tableId)));
DocumentFile.fromFile(new File(ODKFileUtils.getTablePropertiesCsvFile(sc.getAppName(), tableId))));

boolean tablePropertiesChanged = false;

Expand Down Expand Up @@ -491,7 +493,7 @@ public void syncTableLevelFiles(String tableId, String serverReportedTableLevelE
serverFilesToDelete.add(localFile);
} else if (ODKFileUtils.getMd5Hash(sc.getAppName(), localFile).equals(entry.md5hash)) {
// we are ok -- no need to upload or delete
relativePathsOnDevice.remove(ODKFileUtils.asRelativePath(sc.getAppName(), localFile));
relativePathsOnDevice.remove(ODKFileUtils.asRelativePath(sc.getAppName(), DocumentFile.fromFile(localFile)));
}
}

Expand All @@ -512,7 +514,7 @@ public void syncTableLevelFiles(String tableId, String serverReportedTableLevelE

for (File localFile : serverFilesToDelete) {

String relativePath = ODKFileUtils.asRelativePath(sc.getAppName(), localFile);
String relativePath = ODKFileUtils.asRelativePath(sc.getAppName(), DocumentFile.fromFile(localFile));
syncStatus.updateNotification(SyncProgressState.TABLE_FILES,
R.string.sync_deleting_file_on_server,
new Object[] { relativePath }, stepCount * stepSize, false);
Expand All @@ -529,7 +531,7 @@ public void syncTableLevelFiles(String tableId, String serverReportedTableLevelE

for (OdkTablesFileManifestEntry entry : manifestDocument.entries) {
File localFile = ODKFileUtils.asConfigFile(sc.getAppName(), entry.filename);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this?

String relativePath = ODKFileUtils.asRelativePath(sc.getAppName(), localFile);
String relativePath = ODKFileUtils.asRelativePath(sc.getAppName(), DocumentFile.fromFile(localFile));

syncStatus.updateNotification(SyncProgressState.TABLE_FILES,
R.string.sync_verifying_local_file,
Expand Down