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): temp file handling #12161

Merged
merged 8 commits into from Oct 19, 2020
Merged

Conversation

jquick-axway
Copy link
Contributor

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

JIRA:

Summary:

  • Moved Ti.Filesystem.tempDirectory location:
    • From: file:///.../<PackageName>/cache/_tmp
    • To: file:///.../<PackageName>/cache/.titanium/tmp
  • Moved Ti.Filesystem.createTempFile() location:
    • From: Ti.Filesystem.applicationCacheDirectory
    • To: Ti.Filesystem.tempDirectory (Matches iOS' behavior.)
  • Moved Ti.Filesystem.createTempDirectory() location:
    • From a directory Titanium did not provide a constant for.
    • To: Ti.Filesystem.tempDirectory (Matches iOS' behavior.)
  • Modified HTTP response cache file handling:
    • Moved from external storage to internal storage for privacy. (Recommended by Google.)
    • New location: file:///.../<PackageName>/cache/.titanium/http-response-cache
    • Supports caching if app does not have WRITE_EXTERNAL_STORAGE permission. (This is new.)
    • Cache no longer auto-deleted when re-launching app. (Is limited to 25 MB.)
    • Removed external storage mounting/unmounting monitoring. (No longer needed.)
  • Simplified temp file deletion via hidden "trash" folder.
    • Old approach used to auto-delete files under temp folder 5 seconds after JS runtime starts, which means app can briefly find previous temp files at startup.
    • New approach "moves" temp folder under a hidden "trash" folder (this is very fast) and then the "trash" folder is deleted via a thread. A new temp folder will be created when requested in JS code. Temp files are trashed every time JS runtime terminates and when app starts up natively before JS runtime starts.
  • Removed TiTempFileHelper Java class.

Test:

  1. Build and run the below on Android.
  2. On app startup, verify temp files found is: <None>
  3. Tap on the "Create Temp Files" button.
  4. Verify you see 3 files listed:
    file:///.../cache/.titanium/tmp/RootTempFile.txt
    file:///.../cache/.titanium/tmp/tifile<Numbers>.tmp
    file:///.../cache/.titanium/tmp/tidir<Numbers>/SubdirectoryFile.txt
  5. Tap on the "Create Temp Files" button again.
  6. Verify you see 5 files listed: (A new tifile and tidir will be generated.)
    file:///.../cache/.titanium/tmp/RootTempFile.txt
    file:///.../cache/.titanium/tmp/tifile<Numbers>.tmp
    file:///.../cache/.titanium/tmp/tifile<Numbers>.tmp
    file:///.../cache/.titanium/tmp/tidir<Numbers>/SubdirectoryFile.txt
    file:///.../cache/.titanium/tmp/tidir<Numbers>/SubdirectoryFile.txt
  7. Back out of the app and quickly relaunch.
  8. Verify temp files found is: <None>
let label = null;
function readTempFiles() {
	let text = "";
	function loadFrom(file) {
		if (file.isDirectory()) {
			for (const nextFileName of file.getDirectoryListing()) {
				loadFrom(Ti.Filesystem.getFile(file.nativePath, nextFileName));
			}
		} else {
			text += "- " + file.nativePath + "\n";
		}
	}
	loadFrom(Ti.Filesystem.getFile(Ti.Filesystem.tempDirectory));
	label.text = "Temp Files Found:\n" + (text ? text : "<None>");
	console.log("@@@ " + label.text);
}

const window = Ti.UI.createWindow();
label = Ti.UI.createLabel({
	top: "5dp",
	left: "5dp",
	bottom: "5dp",
	right: "5dp",
});
window.add(label);
const button = Ti.UI.createButton({
	title: "Create Temp Files",
	bottom: "20dp",
});
button.addEventListener("click", function() {
	let tempFile = Ti.Filesystem.getFile(Ti.Filesystem.tempDirectory, "RootTempFile.txt");
	tempFile.write("Manually created file.")
	tempFile = Ti.Filesystem.createTempFile();
	tempFile.write("Generated file.");
	tempFile = Ti.Filesystem.getFile(Ti.Filesystem.createTempDirectory().nativePath, "SubdirectoryFile.txt");
	tempFile.write("Subdirectory file.");
	readTempFiles();
});
window.add(button);
window.addEventListener("open", function() {
	readTempFiles();
});
window.open();

- Moved Ti.Filesystem.tempDirectory location:
  * From:  ./<app>/cache/_tmp
  * To:  ./<app>/cache/.titanium/tmp
- Moved Ti.Filesystem.createTempFile() location:
  * From: Ti.Filesystem.applicationCacheDirectory
  * To: Ti.Filesystem.tempDirectory
- Moved Ti.Filesystem.createTempDirectory() location:
  * From a directory Titanium did not provide a constant for.
  * To: Ti.Filesystem.tempDirectory
- Modified HTTP response cache file handling:
  * Moved from external storage to internal storage for privacy.
  * Now supports caching if app does not have WRITE_EXTERNAL_STORAGE permission. (No longer requires this permission.)
  * No longer monitors external storage mounting/unmounting.
- Simplified temp file deletion via hidden "trash" folder.
- Partially removed usage of "TiTempFileHelper.java" class. Will fully remove in next commit.

Fixes TIMOB-28058
@build
Copy link
Contributor

build commented Oct 7, 2020

Warnings
⚠️

Commit 17150f925c42627eaa32582543537043868c604d has a message "refactor(android): temp file handling

  • Moved Ti.Filesystem.tempDirectory location:
    • From: .//cache/_tmp
    • To: .//cache/.titanium/tmp
  • Moved Ti.Filesystem.createTempFile() location:
    • From: Ti.Filesystem.applicationCacheDirectory
    • To: Ti.Filesystem.tempDirectory
  • Moved Ti.Filesystem.createTempDirectory() location:
    • From a directory Titanium did not provide a constant for.
    • To: Ti.Filesystem.tempDirectory
  • Modified HTTP response cache file handling:
    • Moved from external storage to internal storage for privacy.
    • Now supports caching if app does not have WRITE_EXTERNAL_STORAGE permission. (No longer requires this permission.)
    • No longer monitors external storage mounting/unmounting.
  • Simplified temp file deletion via hidden "trash" folder.
  • Partially removed usage of "TiTempFileHelper.java" class. Will fully remove in next commit.

Fixes TIMOB-28058" giving 1 errors:

  • body's lines must not be longer than 100 characters
Messages
📖 👍 Hey!, You deleted more code than you added. That's awesome!
📖

🚨 This PR has one or more commits with warnings/errors for commit messages not matching our configuration. You may want to squash merge this PR and edit the message to match our conventions, or ask the original developer to modify their history.

📖

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

Generated by 🚫 dangerJS against 97ba321

Copy link
Contributor

@sgtcoolguy sgtcoolguy left a comment

Choose a reason for hiding this comment

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

This is nice, looks like it simplifies things much more and feels like much less of a hassle to not need external storage/mounting to deal with here.

@ssekhri
Copy link

ssekhri commented Oct 19, 2020

FR Passed
Mac OS: 10.15.4
SDK: 9.3.0.v20201016171621
Appc CLI: 8.1.1
JDK: 11.0.4
Node: 10.17.0
Studio: 6.0.0.202005141803
Device: Nexus4(v5.1.1) device, Pixel 3 XL(v11.0) emulator

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