Skip to content

Commit

Permalink
refactor(android): Ti.Filesystem external storage handling (tidev#12253)
Browse files Browse the repository at this point in the history 
* feat(android): change externalStorageDirectory to use scoped storage

Fixes TIMOB-28231

* feat(android): add Ti.Filesystem.externalCacheDirectory property

Fixes TIMOB-28230

* test(android): add external storage read/write tests

* refactor(android): only add storage permissions when needed

Fixes TIMOB-28182

* chore(android): update code comments for TIMOB-28231

Co-authored-by: Gary Mathews <contact@garymathews.com>
Co-authored-by: Lokesh Choudhary <lchoudhary@axway.com>
  • Loading branch information
@jquick-axway @garymathews @lokeshchdhry
3 people committed Nov 16, 2020
1 parent a529d4e commit a0430b7
Show file tree
Hide file tree
Showing 11 changed files with 289 additions and 100 deletions.
2 changes: 1 addition & 1 deletion android/app/src/main/AndroidManifest.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
<uses-permission android:name="android.permission.INTERNET"/>
<uses-permission android:name="android.permission.ACCESS_WIFI_STATE"/>
<uses-permission android:name="android.permission.ACCESS_NETWORK_STATE"/>
<uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE"/>
<uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE" android:maxSdkVersion="28"/>

<!-- Permissions needed to test Ti.Geolocation module. -->
<uses-permission android:name="android.permission.ACCESS_COARSE_LOCATION"/>
Expand Down
20 changes: 16 additions & 4 deletions android/cli/commands/_build.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3476,10 +3476,11 @@ AndroidBuilder.prototype.fetchNeededManifestSettings = function fetchNeededManif

// Define Android <uses-permission/> names needed by our core Titanium APIs.
const calendarPermissions = [ 'android.permission.READ_CALENDAR', 'android.permission.WRITE_CALENDAR' ];
const cameraPermissions = [ 'android.permission.CAMERA' ];
const cameraPermissions = [ 'android.permission.CAMERA', 'android.permission.WRITE_EXTERNAL_STORAGE' ];
const contactsPermissions = [ 'android.permission.READ_CONTACTS', 'android.permission.WRITE_CONTACTS' ];
const contactsReadPermissions = [ 'android.permission.READ_CONTACTS' ];
const geoPermissions = [ 'android.permission.ACCESS_COARSE_LOCATION', 'android.permission.ACCESS_FINE_LOCATION' ];
const storagePermissions = [ 'android.permission.WRITE_EXTERNAL_STORAGE' ];
const vibratePermissions = [ 'android.permission.VIBRATE' ];
const wallpaperPermissions = [ 'android.permission.SET_WALLPAPER' ];

Expand All @@ -3504,7 +3505,10 @@ AndroidBuilder.prototype.fetchNeededManifestSettings = function fetchNeededManif
'Contacts.getAllPeople': contactsReadPermissions,
'Contacts.getAllGroups': contactsReadPermissions,

'Filesystem.requestStoragePermissions': storagePermissions,

'Media.Android.setSystemWallpaper': wallpaperPermissions,
'Media.saveToPhotoGallery': storagePermissions,
'Media.showCamera': cameraPermissions,
'Media.vibrate': vibratePermissions,
};
Expand All @@ -3517,9 +3521,12 @@ AndroidBuilder.prototype.fetchNeededManifestSettings = function fetchNeededManif
neededPermissionDictionary['android.permission.INTERNET'] = true;
neededPermissionDictionary['android.permission.ACCESS_WIFI_STATE'] = true;
neededPermissionDictionary['android.permission.ACCESS_NETWORK_STATE'] = true;
neededPermissionDictionary['android.permission.WRITE_EXTERNAL_STORAGE'] = true;
}

// Set the max API Level the "WRITE_EXTERNAL_STORAGE" permission should use.
// Android 10 and higher doesn't need this permission unless requestStoragePermissions() method is used.
let storagePermissionMaxSdkVersion = 28;

// Define JavaScript methods that need manifest <queries> entries.
// The value strings are used as boolean property names in our "AndroidManifest.xml" EJS template.
const tiMethodQueries = {
Expand Down Expand Up @@ -3579,6 +3586,9 @@ AndroidBuilder.prototype.fetchNeededManifestSettings = function fetchNeededManif
for (const permission of permissionArray) {
neededPermissionDictionary[permission] = true;
}
if (symbol === 'Filesystem.requestStoragePermissions') {
storagePermissionMaxSdkVersion = undefined;
}
}
}

Expand All @@ -3592,8 +3602,9 @@ AndroidBuilder.prototype.fetchNeededManifestSettings = function fetchNeededManif

// Return the entries needed to be injected into the generated "AndroidManifest.xml" file.
const neededSettings = {
usesPermissions: Object.keys(neededPermissionDictionary),
queries: neededQueriesDictionary
queries: neededQueriesDictionary,
storagePermissionMaxSdkVersion: storagePermissionMaxSdkVersion,
usesPermissions: Object.keys(neededPermissionDictionary)
};
return neededSettings;
};
Expand Down Expand Up @@ -3688,6 +3699,7 @@ AndroidBuilder.prototype.generateAndroidManifest = async function generateAndroi
appLabel: this.tiapp.name,
appTheme: `@style/${this.defaultAppThemeName}`,
classname: this.classname,
storagePermissionMaxSdkVersion: neededManifestSettings.storagePermissionMaxSdkVersion,
packageName: this.appid,
queries: neededManifestSettings.queries,
usesPermissions: neededManifestSettings.usesPermissions
Expand Down
17 changes: 17 additions & 0 deletions android/cli/lib/android-manifest.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,23 @@ class AndroidManifest {
// Apply "tools:replace" attribute to <manifest/> element. (Must be done after setting namespace above.)
applyToolsReplaceToElement(manifestElement);

// Apply 'tools:node="replace"' to WRITE_EXTERNAL_STORAGE permission if no other tools attribute is set.
// Titanium adds "maxSdkVersion" attribute to this permission by default. This removes that attribute.
const permissionElement = getFirstChildElementByTagAndAndroidName(
manifestElement, 'uses-permission', 'android.permission.WRITE_EXTERNAL_STORAGE');
if (permissionElement) {
let hasToolsAttribute = false;
for (let index = 0; index < permissionElement.attributes.length; index++) {
if (permissionElement.attributes.item(index).name.startsWith('tools:')) {
hasToolsAttribute = true;
break;
}
}
if (!hasToolsAttribute) {
permissionElement.setAttribute('tools:node', 'replace');
}
}

// Fetch the <application/> element.
const appElement = getFirstChildElementByTagName(manifestElement, 'application');
if (!appElement) {
Expand Down
32 changes: 11 additions & 21 deletions android/modules/filesystem/src/java/ti/modules/titanium/filesystem/FilesystemModule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

import java.io.File;
import java.io.IOException;
import java.net.MalformedURLException;

import android.Manifest;
import android.app.Activity;
Expand All @@ -21,6 +20,7 @@
import org.appcelerator.kroll.KrollModule;
import org.appcelerator.kroll.annotations.Kroll;
import org.appcelerator.kroll.common.Log;
import org.appcelerator.titanium.io.TiFileFactory;
import org.appcelerator.titanium.TiApplication;
import org.appcelerator.titanium.TiBaseActivity;
import org.appcelerator.titanium.TiC;
Expand All @@ -40,8 +40,6 @@ public class FilesystemModule extends KrollModule
@Kroll.constant
public static final int MODE_APPEND = 2;

private static String[] RESOURCES_DIR = { "app://" };

// Methods
public FilesystemModule()
{
Expand Down Expand Up @@ -145,7 +143,7 @@ public FileProxy getApplicationDirectory()
@Kroll.getProperty
public String getApplicationDataDirectory()
{
return "appdata-private://";
return TiFileFactory.APPDATA_PRIVATE_URL_SCHEME + "://";
}

@Kroll.method
Expand All @@ -155,39 +153,31 @@ public String getResRawDirectory()
return "android.resource://" + TiApplication.getInstance().getPackageName() + "/raw/";
}

@SuppressWarnings("deprecation")
@Kroll.method
@Kroll.getProperty
public String getApplicationCacheDirectory()
{
TiApplication app = TiApplication.getInstance();
if (app == null) {
return null;
}

File cacheDir = app.getCacheDir();

try {
return cacheDir.toURL().toString();

} catch (MalformedURLException e) {
Log.e(TAG, "Exception converting cache directory to URL", e);
return null;
}
return "file://" + TiApplication.getInstance().getCacheDir().getAbsolutePath();
}

@Kroll.method
@Kroll.getProperty
public String getResourcesDirectory()
{
return "app://";
return TiC.URL_APP_PREFIX;
}

@Kroll.getProperty
public String getExternalCacheDirectory()
{
return TiFileFactory.APPCACHE_EXTERNAL_URL_SCHEME + "://";
}

@Kroll.method
@Kroll.getProperty
public String getExternalStorageDirectory()
{
return "appdata://";
return TiFileFactory.APPDATA_URL_SCHEME + "://";
}

@Kroll.method
Expand Down
4 changes: 4 additions & 0 deletions android/templates/build/AndroidManifest.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@
android:versionCode="1">

<% if (usesPermissions) { for (const nextPermission of usesPermissions) { %>
<% if ((nextPermission === 'android.permission.WRITE_EXTERNAL_STORAGE') && storagePermissionMaxSdkVersion) { %>
<uses-permission android:name="<%- nextPermission %>" android:maxSdkVersion="<%- storagePermissionMaxSdkVersion %>"/>
<% } else { %>
<uses-permission android:name="<%- nextPermission %>"/>
<% } %>
<% } } %>

<% if (queries && (Object.keys(queries).length > 0)) { %>
Expand Down
98 changes: 55 additions & 43 deletions android/titanium/src/java/org/appcelerator/titanium/io/TiFileFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,10 @@ public class TiFileFactory
{
private static final String TAG = "TiFileFactory";
private static final String ANDROID_RESOURCE_URL_SCHEME = ContentResolver.SCHEME_ANDROID_RESOURCE;
private static final String ANDROID_RESOURCE_URL_PREFIX = ANDROID_RESOURCE_URL_SCHEME + "://";
private static final String APPDATA_URL_SCHEME = "appdata";
private static final String APPDATA_URL_PREFIX = APPDATA_URL_SCHEME + "://";
private static final String APPDATA_PRIVATE_URL_SCHEME = "appdata-private";
private static final String APPDATA_PRIVATE_URL_PREFIX = APPDATA_PRIVATE_URL_SCHEME + "://";
public static final String APPCACHE_EXTERNAL_URL_SCHEME = "appcache-external";
public static final String APPDATA_URL_SCHEME = "appdata";
public static final String APPDATA_PRIVATE_URL_SCHEME = "appdata-private";
private static final String CONTENT_URL_SCHEME = ContentResolver.SCHEME_CONTENT;
private static final String CONTENT_URL_PREFIX = CONTENT_URL_SCHEME + "://";
private static final String FILE_URL_SCHEME = ContentResolver.SCHEME_FILE;
private static final String FILE_URL_PREFIX = FILE_URL_SCHEME + "://";
// strip file:// prefix off the special URL we need to handle like app:
Expand All @@ -45,6 +42,7 @@ public class TiFileFactory
{
localSchemeSet = new HashSet<String>();
localSchemeSet.add(TiC.URL_APP_SCHEME.toLowerCase());
localSchemeSet.add(APPCACHE_EXTERNAL_URL_SCHEME.toLowerCase());
localSchemeSet.add(APPDATA_URL_SCHEME.toLowerCase());
localSchemeSet.add(APPDATA_PRIVATE_URL_SCHEME.toLowerCase());
localSchemeSet.add(FILE_URL_SCHEME.toLowerCase());
Expand Down Expand Up @@ -76,8 +74,9 @@ public static TiBaseFile createTitaniumFile(String path, boolean stream)
*/
public static TiBaseFile createTitaniumFile(String[] parts, boolean stream)
{
// Parse for the URL scheme and file path.
String possibleURI = joinPathSegments(parts);
String scheme = null;
String scheme = "";
String path = "";
int colonIndex = possibleURI.indexOf(':');
if (colonIndex != -1) {
Expand Down Expand Up @@ -107,45 +106,59 @@ public static TiBaseFile createTitaniumFile(String[] parts, boolean stream)
}
}

if (TiC.URL_APP_SCHEME.equals(scheme)) {
return new TiResourceFile(trimFront(path, '/'));
}

if (APPDATA_PRIVATE_URL_SCHEME.equals(scheme)) {
File f = new File(getDataDirectory(true), path);
return new TiFile(f, possibleURI, stream);
}

if (APPDATA_URL_SCHEME.equals(scheme)) {
File f = new File(getDataDirectory(false), path);
return new TiFile(f, possibleURI, stream);
}

if (CONTENT_URL_SCHEME.equals(scheme) || ANDROID_RESOURCE_URL_SCHEME.equals(scheme)) {
return new TiContentFile(possibleURI); // TODO: Forward along the actual URI instance?
}

if (FILE_URL_SCHEME.equals(scheme)) {
// check for fake "file:///android_asset/Resources/" URL, treat like app:
if (path.startsWith(ANDROID_ASSET_RESOURCES)) {
// Strip this fake base path
path = path.substring(ANDROID_ASSET_RESOURCES.length());
return new TiResourceFile(trimFront(path, '/')); // remove leading '/' characters
// Create a Titanium file object capable of accessing the file referenced by given URL.
TiBaseFile tiFile = null;
switch (scheme) {
case TiC.URL_APP_SCHEME: {
tiFile = new TiResourceFile(trimFront(path, '/'));
break;
}
case APPCACHE_EXTERNAL_URL_SCHEME: {
File cacheDir = TiApplication.getInstance().getExternalCacheDir();
if (cacheDir != null) {
File file = new File(cacheDir, path);
tiFile = new TiFile(file, possibleURI, stream);
}
break;
}
case APPDATA_URL_SCHEME: // Use external storage.
case APPDATA_PRIVATE_URL_SCHEME: // Use internal storage.
case TI_URL_SCHEME: { // Use internal storage.
boolean isInternal = !scheme.equals(APPDATA_URL_SCHEME);
File dataDir = getDataDirectory(isInternal);
if (dataDir != null) {
File file = new File(dataDir, path);
tiFile = new TiFile(file, possibleURI, stream);
}
break;
}
case CONTENT_URL_SCHEME:
case ANDROID_RESOURCE_URL_SCHEME: {
tiFile = new TiContentFile(possibleURI);
break;
}
case FILE_URL_SCHEME: {
if (path.startsWith(ANDROID_ASSET_RESOURCES)) {
// This is a "file:///android_asset/Resources/" referencing an APK "assets" file.
path = path.substring(ANDROID_ASSET_RESOURCES.length());
tiFile = new TiResourceFile(trimFront(path, '/'));
} else {
// This is a normal file system path.
tiFile = new TiFile(new File(path), possibleURI, stream);
}
break;
}

// Normal file
return new TiFile(new File(path), possibleURI, stream);
}

if (TI_URL_SCHEME.equals(scheme)) { // treat like appdata-private
// TODO: Do we need to trim leading '/'?
File f = new File(getDataDirectory(true), path);
return new TiFile(f, possibleURI, stream);
// Create a mock file object whose methods no-op if we failed to handle given path. Can happen if:
// - Given an invalid/unknown URL scheme.
// - Unable to access destination, such as external storage not being available.
if (tiFile == null) {
tiFile = new TiMockFile(possibleURI);
}

// TODO: Throw an exception? Ideally this shouldn't ever happen, but could if an unhandled scheme URI came in here
// i.e. http:, ftp:, https:, mailto:, etc.
return null;
// Return a TiBaseFile wrapping the given path. Will never be null.
return tiFile;
}

private static String joinPathSegments(String[] parts)
Expand Down Expand Up @@ -204,8 +217,7 @@ private static String trimFront(String sourceString, char trimCharacter)
*/
public static File getDataDirectory(boolean privateStorage)
{
TiFileHelper tfh = new TiFileHelper(TiApplication.getInstance());
return tfh.getDataDirectory(privateStorage);
return TiFileHelper.getInstance().getDataDirectory(privateStorage);
}

public static boolean isLocalScheme(String url)
Expand Down

0 comments on commit a0430b7

Please sign in to comment.