You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by jasongin <gi...@git.apache.org> on 2015/10/30 17:53:27 UTC
[GitHub] cordova-plugin-file pull request: Fix CB-9752: FileEntry.getDirect...
GitHub user jasongin opened a pull request:
https://github.com/apache/cordova-plugin-file/pull/144
Fix CB-9752: FileEntry.getDirectory fails with assets filesystem
Calls to FileEntry.getDirectory() on an asset entry would internally call
the AssetFileSystem.listAssets() method using an asset path with a
trailing slash, regardless of whether a trailing slash was included by
the user. But neither the asset list cache (from cdvasset.manifest) nor
Android's AssetManager.list() API support matching a path with a trailing
slash. That resulted in a TypeMismatchException thrown from
AssetFileSystem.getFileForLocalURL(), because the asset path was found
but not matched as a directory as expected by the caller.
This fix removes the trailing slash from the asset path before matching
against the asset list cache or calling Android's AssetManager.list() API.
A new unit test for getting asset directories fails without and passes
with the fix.
While I'm here, I'm also adding a unit test for reading asset file
contents. There are a couple old bug reports about that (CB-7273,
CB-8350). The bugs don't repro anymore, but I'm adding the test anyway
since there wasn't good test coverage before.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jasongin/cordova-plugin-file CB-9752
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cordova-plugin-file/pull/144.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #144
----
commit 76c8fd5e43598ea660c2db64eb08e2659b45a575
Author: Jason Ginchereau <ja...@microsoft.com>
Date: 2015-10-28T21:30:24Z
Fix CB-9752: getDirectory fails on valid directory with assets filesystem
Calls to FileEntry.getDirectory() on an asset entry would internally call
the AssetFileSystem.listAssets() method using an asset path with a
trailing slash, regardless of whether a trailing slash was included by
the user. But neither the asset list cache (from cdvasset.manifest) nor
Android's AssetManager.list() API support matching a path with a trailing
slash. That resulted in a TypeMismatchException thrown from
AssetFileSystem.getFileForLocalURL(), because the asset path was found
but not matched as a directory as expected by the caller.
This fix removes the trailing slash from the asset path before matching
against the asset list cache or calling Android's AssetManager.list() API.
A new unit test for getting asset directories fails without and passes
with the fix.
While I'm here, I'm also adding a unit test for reading asset file
contents. There are a couple old bug reports about that (CB-7273,
CB-8350). The bugs don't repro anymore, but I'm adding the test anyway
since there wasn't good test coverage before.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-plugin-file pull request: Fix CB-9752: FileEntry.getDirect...
Posted by jasongin <gi...@git.apache.org>.
Github user jasongin commented on the pull request:
https://github.com/apache/cordova-plugin-file/pull/144#issuecomment-152657405
@infil00p or @riknoll can you review?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-plugin-file pull request: Fix CB-9752: FileEntry.getDirect...
Posted by infil00p <gi...@git.apache.org>.
Github user infil00p commented on the pull request:
https://github.com/apache/cordova-plugin-file/pull/144#issuecomment-152660125
Where are you adding the unit test? We currently don't have a way to unit test plugins.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-plugin-file pull request: Fix CB-9752: FileEntry.getDirect...
Posted by jasongin <gi...@git.apache.org>.
Github user jasongin commented on the pull request:
https://github.com/apache/cordova-plugin-file/pull/144#issuecomment-152661034
Oh, yeah "unit test" was probably not the right term.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-plugin-file pull request: Fix CB-9752: FileEntry.getDirect...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cordova-plugin-file/pull/144
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-plugin-file pull request: Fix CB-9752: FileEntry.getDirect...
Posted by riknoll <gi...@git.apache.org>.
Github user riknoll commented on the pull request:
https://github.com/apache/cordova-plugin-file/pull/144#issuecomment-153517264
LGTM. Tested on an Android 6.0 device and on an Android 5.1.1 emulator and ran mobilespec. I'll merge this in at the end of the day.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org