wolpi / prim-ftpd

FTP server app for android
Other
580 stars 78 forks source link

Fix new file upload on SAF and VirtualFS with SSHFS #346

Closed lmagyar closed 1 week ago

lmagyar commented 1 month ago

SSHFS after opening a new, non-existent file for upload, assumes it is created and calls a STAT on it (see). And that STAT fails on SAF and virtual FS, because the file doesn't created, doesn't exists.

UPDATE: And SSHFS even calls FSTAT on this open file, so the created file's properties have to be updated on the already created object.

It is created on the "plain old" file-system, and that works with SSHFS. So I've added it for SAF and virtual. I'm sure it is not fast, but at least works. Tested on real phone: Android 9.0, Samsung A8.

I didn't change the root FS implementation, because I can't test that, my phone is not rooted.

wolpi commented 1 month ago

Interesting to see, I want to do some tests on that before merging

lmagyar commented 1 month ago

Yes, yes, test everything, these are "works on my phone" modifications. :) And my first ever Android/java modifications.

The concept is that in case of real files, when they are created, the next call to isSomething() returns the changed fact/reality, but in case of SAF this "background" communication between the methods doesn't happen, because nearly all these information are "cached" in the objects, so we have to update the facts inside the objects. It is against the more or less "immutable" state of these classes, it smells to me a bit, but couldnt come up with a better idea.

wolpi commented 3 weeks ago

For me creation of files, even in SAF below virtual, works. E.g. I can create a file virtual SAF and switch to virtual FS and it is present.

What was your issue you want to solve with this?

lmagyar commented 3 weeks ago

This is for SSHFS specifically, because SSHFS after opens the file (for creation), it calls a STAT (not FSTAT) on the filename immediately, and that fails on SAF and Virtual, because these (not like plain-old FS), do not create the real file, and the STAT tries to access the nonexistent file and fails.

It worked for SSHFS in case of plain-old, because the file was really created, but SAF and virtual did nothing.

It is not a problem for eg. WinSCP, that opens, writes, closes the file, simple, works, but SSHFS makes more administrative steps (also the main reason it is slower)


The other part of the modifications is also for SSHFS specifically, because later it calls an FSTAT also, and the existing handle's, ie. the SAF and Virtual file's "cached" info about the real file also has to be updated.

In case of plain-old it is not a problem, because there are mush less "cached" info in the FsFile class, most of it comes from the file itself.

lmagyar commented 3 weeks ago

Rebased it to resolve conflicts.

wolpi commented 1 week ago

Ok, I see.

Than I would like to have code like this:

Would you change it like that? Otherwise I can merge and change it myself.

lmagyar commented 1 week ago

I've tried to move it "down" to SafSshFile now, but the createNewFile() method uses writable, documentFile, parentDocumentFile properties, and all these are private, so SafSshFile can't access them. So it seems to be a better place to be is SafFile. This is similar to FsSshFile's create(), that calls file.createNewFile(), though that is part of the OS, we have to put our SAF equivalent of File.createNewFile() somewhere, and SafFile seems to be a better place.

I've added the comment.

But please feel free to modify anything as you see it a better solution, it's your project, your style, your concept, anything. :)

wolpi commented 1 week ago

ok, for now it is fine :smile: