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

Android: Fixes #6779: Fixed android filesystem sync (resources) #6789

Merged
merged 3 commits into from Aug 29, 2022

Conversation

jd1378
Copy link
Contributor

@jd1378 jd1378 commented Aug 29, 2022

Fixes #6779

It seems I had missed the retry part of copy function inside fs-driver-rn.ts, which were causing it to not return the proper error.

I also made sure its possible to copy files between scoped storage style and normal path style, but the functionality is limited to where the app has permission, which I assume it will work fine with joplin's use case. I tested adding a picture and syncing to make sure it works.

what concerns me more, is that the files still remain in the synced folder, even after the notes are deleted and synced. is this intended?

@jd1378
Copy link
Contributor Author

jd1378 commented Aug 29, 2022

@laurent22 I hope this fixes the remaining issue, the changes are small, so I hope this one gets merged faster than last time 😄

@laurent22
Copy link
Owner

what concerns me more, is that the files still remain in the synced folder, even after the notes are deleted and synced. is this intended?

No, the app calls detete() on the sync target so it should be deleted. Is that maybe you delete call that's not working properly?

@jd1378
Copy link
Contributor Author

jd1378 commented Aug 29, 2022

I have tested the unlink method alone and it worked. the thing is that access framework just returns false if it fails. should try to figure if it deleted and throw IOException if it fails?

Edit: should throw I believe
Edit2: It returns true if the file does not exist, so its easy to throw, unlike what I assumed

@jd1378
Copy link
Contributor Author

jd1378 commented Aug 29, 2022

does it also cleanup .resource folder ?

@laurent22
Copy link
Owner

if it deleted and throw IOException if it fails?

You need to check the type of error. If it fails because the file doesn't exist, you should ignore the exception (see file-api-driver-webdav for example). If it's anything else you should probably rethrow.

@jd1378
Copy link
Contributor Author

jd1378 commented Aug 29, 2022

Document file does not throw at any circumstances, then I have to throw simply if it just returns false.

edit: I was wrong, my library does, and I'm already re-throwing, the only remaining thing is the unexpected false return to check.

@jd1378
Copy link
Contributor Author

jd1378 commented Aug 29, 2022

@laurent22 well it seems the library is indeed removing files, it seems that there is an issue with cleaning up algorithm or sth else that the resources remain inside .resource folder

because I setup log points in unlink method on js side and any file it decided to unlink was indeed gone.

@jd1378
Copy link
Contributor Author

jd1378 commented Aug 29, 2022

you can merge this as a fix for the mentioned issue in the PR

about the remaining files in resource folder, this is probably the related issue #932

@laurent22
Copy link
Owner

Ok let's merge then, thanks for the fix!

@laurent22 laurent22 merged commit de94c35 into laurent22:dev Aug 29, 2022
@davidgomesdev
Copy link

Is there an ETA for when this is getting released?

@jd1378 jd1378 deleted the fix_android_sync_resource branch August 30, 2022 15:47
@wh201906
Copy link
Contributor

@LegendL3n the latest android pre-releases can be found there.
https://github.com/laurent22/joplin-android/tags

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Android: Building from source: Still unable to sync with filesystem
4 participants