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

[TIMOB-25231] Android: List encrypted assets #10031

Merged
merged 6 commits into from May 16, 2018

Conversation

garymathews
Copy link
Contributor

@garymathews garymathews commented May 8, 2018

  • List encrypted assets when using getDirectoryListing()
TEST CASE BY @jquick-axway
function logFileTree (file) {
    if (file) {
        Ti.API.info('- ' + file.nativePath + ' (directory: ' + file.isDirectory() + ')');
        let files = file.getDirectoryListing();
        if (files) {
            for (let f of files) {
                logFileTree(Ti.Filesystem.getFile(file.nativePath, f));
            }
        }
    }
}
logFileTree(Ti.Filesystem.getFile(Ti.Filesystem.getResourcesDirectory()));

JIRA Ticket

@jquick-axway
Copy link
Contributor

Notes:

  • The File.getDirectoryListing() returns an array of file names and subdirectory names. This array must not contain absolute paths. They are relative to the File object that the getDirectoryListing() method was called upon.
  • The File.nativePath property must return an absolute path. It cannot be relative.
  • I've verified that "Live View" builds correctly list JS files from the Resources directory on both Android and iOS. This PR fixes the JS file listing issue for non-LiveView builds on Android.

@jquick-axway
Copy link
Contributor

@garymathews, I have one minor nitpick. I think thegetAssets() and getEncryptedAssets() methods should be renamed to either getAssetPaths() or getPaths() since it returns an array of paths and not the asset data itself. This makes it clear on the caller side what is actually returned.

The rest of the code looks fine. 👍

@sgtcoolguy
Copy link
Contributor

I would assume we should have some way of being able to unit test this. It may require changes to the test suite code itself to handle adding assets to encrypt into the test project, but then we should be able to add a test to the Ti.Filesystem.File suite to ensure the listing contains the encrypted assets.

@jquick-axway
Copy link
Contributor

@sgtcoolguy,

For your info, our mocha test suite uses the Ti.Filesystem.getDirectoryListing() method to find "addon" JS files. See the "app.js" file's loadAddonTestFiles() method here...
https://github.com/appcelerator/titanium-mobile-mocha-suite/blob/master/Resources/app.js#L117

This will currently fail to find "addon" JS files for Android builds... unless you do a LiveView build. Gary's fix resolves this issue.

@jquick-axway
Copy link
Contributor

@garymathews,

I wrote up a quick unit test for you. This should replace the ti.filesystem.file.test.js file's "#directoryListing()" unit test. I have verified that the below works/passes on iOS. I have not tested it with your PR.

it('#directoryListing()', function () {
	var rootDir = Ti.Filesystem.getFile(Ti.Filesystem.resourcesDirectory),
		rootPath,
		filesFound = {};
	should(rootDir.exists()).be.true;
	should(rootDir.getDirectoryListing).be.a.Function;
	should(rootDir.getDirectoryListing()).be.an.Array;

	// Traverse entire Resources directory tree looking for files/directories in "filesFound".
	rootPath = rootDir.nativePath;
	filesFound[rootPath + 'app.js'] = false;
	filesFound[rootPath + 'ti.ui.webview.test.html'] = false;
	filesFound[rootPath + 'fixtures/'] = false; // Subdirectory containing only JS files.
	filesFound[rootPath + 'fixtures/empty-double.js'] = false;
	filesFound[rootPath + 'txtFiles/'] = false; // Subdirectory containing only assets.
	filesFound[rootPath + 'txtFiles/text.txt'] = false;
	function searchFileTree(file) {
		if (file) {
			var fileList = file.getDirectoryListing();
			if (fileList) {
				for (var index = 0; index < fileList.length; index++) {
					var nextFile = Ti.Filesystem.getFile(file.nativePath, fileList[index]);
					if (nextFile) {
						var absolutePath = nextFile.nativePath;
						if (absolutePath in filesFound) {
							filesFound[absolutePath] = true;
						}
						searchFileTree(nextFile);
					}
				}
			}
		}
	}
	searchFileTree(rootDir);
	for (var key in filesFound) {
		Ti.API.info('Checking if found file: ' + key);
		should(filesFound[key]).be.true;
	}
});

Note that we need to double check the trailing slash / handling for the nativePath returned directories/subdirectories. iOS always returns them with a trailing /. We should try to be consistent.

@jquick-axway
Copy link
Contributor

jquick-axway commented May 11, 2018

Hmm... we must be doing a --liveview build when running our unit tests. Otherwise the "addontest" fetching code and the "ti.ui.webview.test.js" data unit test would fail on a normal build. I think this explains the differences we're seeing when we run the unit tests ourselves.

Edit: We're not doing a --liveview build. I've confirmed that a build to the emulator does not auto-encrypt our JS files and the getDirectoryListing() API is able to retrieve them in this case. Gary's PR fixes this issue for device test builds and production builds.

@garymathews
Copy link
Contributor Author

Android emulator builds do not encrypt JS assets, so writing a mocha test won't be testing the changes I made to include encrypted JS assets in the listing.

@build
Copy link
Contributor

build commented May 14, 2018

Messages
📖

💾 Here's the generated SDK zipfile.

Generated by 🚫 dangerJS

@lokeshchdhry
Copy link
Contributor

FR Passed.

The encrypted files are listed as expected on a device & emulator.

Studio Ver: 5.1.0.201804230827
SDK Ver: 7.2.0 local build
OS Ver: 10.13.4
Xcode Ver: Xcode 9.3.1
Appc NPM: 4.2.13
Appc CLI: 7.0.3
Daemon Ver: 1.1.1
Ti CLI Ver: 5.1.0
Alloy Ver: 1.12.0
Node Ver: 8.9.1
NPM Ver: 5.5.1
Java Ver: 10
Devices: ⇨ google Nexus 5 --- Android 6.0.1
⇨ google Nexus 6P --- Android 8.1.0
Emulator: ⇨ Android 4.1.2

@lokeshchdhry
Copy link
Contributor

@jquick-axway , Is CR done for this PR?

@jquick-axway
Copy link
Contributor

The only discrepancy we have left is with the nativePath returned by directories. On Android, they do not end with a trailing slash /, but on iOS they do.

Although, perhaps this is not a major issue. If you do the below, then all platforms will properly append the file to the directory path and inject a slash if needed.

var file = Ti.Filesystem.getFile(directory.nativePath, 'MyFile.txt');

@garymathews garymathews force-pushed the TIMOB-25231 branch 2 times, most recently from d4a5900 to 5c44d77 Compare May 15, 2018 23:27
@sgtcoolguy sgtcoolguy removed this from the 7.2.0 milestone May 16, 2018
@sgtcoolguy sgtcoolguy added this to the 7.3.0 milestone May 16, 2018
@lokeshchdhry
Copy link
Contributor

lokeshchdhry commented May 16, 2018

FR Passed.

All the encrypted assets are listed as expected for device builds.

[INFO] :   [Nexus 6P] - file:///android_asset/Resources/ (directory: true)
[INFO] :   [Nexus 6P] - file:///android_asset/Resources/appicon.png (directory: false)
[INFO] :   [Nexus 6P] - file:///android_asset/Resources/assets/ (directory: true)
[INFO] :   [Nexus 6P] - file:///android_asset/Resources/assets/images/ (directory: true)
[INFO] :   [Nexus 6P] - file:///android_asset/Resources/assets/images/tab1.png (directory: false)
[INFO] :   [Nexus 6P] - file:///android_asset/Resources/assets/images/tab1@2x.png (directory: false)
[INFO] :   [Nexus 6P] - file:///android_asset/Resources/assets/images/tab1@3x.png (directory: false)
[INFO] :   [Nexus 6P] - file:///android_asset/Resources/assets/images/tab2.png (directory: false)
[INFO] :   [Nexus 6P] - file:///android_asset/Resources/assets/images/tab2@2x.png (directory: false)
[INFO] :   [Nexus 6P] - file:///android_asset/Resources/assets/images/tab2@3x.png (directory: false)
[INFO] :   [Nexus 6P] - file:///android_asset/Resources/ti.cloud/ (directory: true)
[INFO] :   [Nexus 6P] - file:///android_asset/Resources/ti.cloud/package.json (directory: false)
[INFO] :   [Nexus 6P] - file:///android_asset/Resources/ti.cloud/ti.cloud.js (directory: false)
[INFO] :   [Nexus 6P] - file:///android_asset/Resources/app.js (directory: false)
[INFO] :   [Nexus 6P] - file:///android_asset/Resources/_app_props_.json (directory: false)

Studio Ver: 5.1.0.201804230827
SDK Ver: 7.3.0 local build
OS Ver: 10.13.4
Xcode Ver: Xcode 9.3.1
Appc NPM: 4.2.13
Appc CLI: 7.0.3
Daemon Ver: 1.1.1
Ti CLI Ver: 5.1.0
Alloy Ver: 1.12.0
Node Ver: 8.9.1
NPM Ver: 5.5.1
Java Ver: 10
Devices: ⇨ google Nexus 6P --- Android 8.1.0
⇨ google Nexus 5 --- Android 6.0.1
Emulator: ⇨ Android 4.1.2

@lokeshchdhry lokeshchdhry merged commit fbb7eae into tidev:master May 16, 2018
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

6 participants