apache / cordova-plugin-file-transfer

Apache Cordova File Transfer Plugin
https://cordova.apache.org/
Apache License 2.0
599 stars 887 forks source link

CB-9969 Filetransfer upload error deletes original file #115

Closed daserge closed 8 years ago

daserge commented 8 years ago

Adds a corresponding test and fixes for iOS and Windows.

Jira issue

Also fixes filetransfer.spec.9, which used an incorrect path in the root.getFile call.

dblotsky commented 8 years ago

Other than the small comment above, this LGTM. DId you try it with mobilespec? Did the new test fail with the old code and does it pass now?

daserge commented 8 years ago

Yes, I've tested it with test-framework plugin.

daserge commented 8 years ago

@dblotsky, thanks for reviewing! Looks like the Android code does not need any changes - I confused targetFile with source, which was not being deleted. I will also improve the test now.

dblotsky commented 8 years ago

Hey @daserge, the comment about line 453 is still unresolved. According to the docs, we should be using localFilePath.

daserge commented 8 years ago

@dblotsky, getFile does work with absolute paths - it successfully resolves /testFile.txt for example as well as relative (to the fs root) testFile.txt so I think the bug was in the test and not in this function (it works with filesystem' abstract paths rather than with native ones). I also don't think it should work with nativeURL - there is resolveLocalFileSystemURL for this case.