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

refactor(android): Ti.Filesystem external storage handling #12253

Merged
merged 9 commits into from Nov 16, 2020

Conversation

jquick-axway
Copy link
Contributor

@jquick-axway jquick-axway commented Nov 7, 2020

JIRA:

Summary:

  • TIMOB-28231 Changed Ti.Filesystem.externalStorageDirectory to use scoped storage.
    • Only directory location is not accessible when targeting and running on Android 11.
    • Changed to a new location which does not require WRITE_EXTERNAL_STORAGE permission.
  • TIMOB-28230 Added property Ti.Filesystem.externalCacheDirectory.
    • Intended to store large temporary files.
  • TIMOB-28182 Modified build system to only add WRITE_EXTERNAL_STORAGE permission when needed.
    • Only adds permission if using API: Ti.Fileystem.requestStoragePermission(), Ti.Media.showCamera(), Ti.Media.saveToPhotoGallery().
  • Added new TiMockFile Java class for handling inaccessible/invalid file URLs.
    • Handles case where we can't fetch external storage path if volume is not mounted. (Natively returns null.)
    • This way Ti.Filesystem.getFile() no longer returns null and instead returns a file object which no-ops.

API Test:

  1. Build and run FileExternalTest.js attached to TIMOB-28231 on Android.
  2. Tap on the "Write" button.
  3. Verify the below is shown on screen.
    External Storage File
    Write Time: <CurrentTime>
    External Cache File
    Write Time: <CurrentTime>
  4. Tap on the "Back" button.
  5. Re-launch the app.
  6. Verify the above 4 lines of text are shown.
  7. Tap on the "Back" button.
  8. Go to: Setting -> Apps -> App Info -> [AppName] -> Storage
  9. Tap on the Clear Cacher button.
  10. Re-launch the app.
  11. Verify only 2 lines are shown. The "External Cache File" text should NOT be shown.
    External Storage File
    Write Time: <CurrentTime>
  12. Tap on the "Delete" button. (Will delete external storage files.)
  13. Back out of the app.
  14. Re-launch the app.
  15. Verify no text is shown.

General Permission Test:

  1. Build FileExternalTest.js attached to TIMOB-28231 for Android.
  2. Open file under: ./build/android/app/intermediates/merged_manifests/debug/AndroidManifest.xml
  3. Verify the file does NOT contain the WRITE_EXTERNAL_STORAGE permission.
  4. Add the below permission to your "tiapp.xml" file.
  5. Build for Android.
  6. Verify "AndroidManifest.xml" file does contain the added permission.
<ti:app>
	<android>
		<manifest>
			<uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE"/>
		</manifest>
	</android>
</ti:app>

requestStoragePermissions() Test:

  1. Build and run RequestStoragePermissionsTest.js attached to TIMOB-28182 on Android.
  2. Tap on the "Request Storage Permission" button.
  3. Verify a permission request dialog is shown.
  4. Tap on the "Allow" button.
  5. Verify the onscreen "hasPermissions:" label is set true.
  6. Open file under: ./build/android/app/intermediates/merged_manifests/debug/AndroidManifest.xml
  7. Verify the file contains the following line.
    <uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE"/>

saveToPhotoGallery() Test:

  1. Build and run ImageToGalleryTest.js attached to TIMOB-28182 on Android 10 or 11.
  2. Tap on the "Save Screenshot to Gallery" button.
  3. A permission request dialog will only appear on Android 9 and older.
  4. Go to the Photos gallery app.
  5. Verify a screenshot of the app is in the gallery.
  6. Open file under: ./build/android/app/intermediates/merged_manifests/debug/AndroidManifest.xml
  7. Verify the file contains the following. (Notice the maxSdkVersion part.)
    <uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE" android:maxSdkVersion="28"/>
  8. Repeat steps 1-5 on Android 9 or older.

showCamera() Test:

  1. Build and run CameraPhotoExternalTest.js attached to TIMOB-28182 on Android 10 or 11.
  2. Tap on the "Take Picture" button.
  3. A permission request dialog will only appear on Android 9 and older.
  4. Take a photo.
  5. Verify the photo is shown in your app.
  6. Repeat steps 1-5 on Android 9 or older.

@build build requested review from a team November 7, 2020 06:44
@build
Copy link
Contributor

build commented Nov 7, 2020

Messages
📖

💾 Here's the generated SDK zipfile.

📖 ✊ The commits in this PR match our conventions! Feel free to Rebase and Merge this PR when ready.
📖

✅ All tests are passing
Nice one! All 13468 tests are passing.
(There are 979 skipped tests not included in that total)

Generated by 🚫 dangerJS against 46c698b

Copy link
Contributor

@garymathews garymathews left a comment

Choose a reason for hiding this comment

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

CR: PASS

@lokeshchdhry
Copy link
Contributor

FR Passed.

@lokeshchdhry lokeshchdhry merged commit 347adb1 into tidev:master Nov 16, 2020
garymathews added a commit to garymathews/titanium_mobile that referenced this pull request Nov 16, 2020
* 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants