erickok / transdroid

Manage your torrents from your Android device
GNU General Public License v3.0
1.28k stars 201 forks source link

"Remove torrent and delete data" does not work with vanilla rTorrent #654

Open matoro opened 1 year ago

matoro commented 1 year ago

Hi, thank you for this great app. I use rTorrent coupled with Flood for a web interface. Everything works in Transdroid except for "Remove torrent and delete data" - it just removes the torrent, but the data remains on disk.

After researching this, it looks like Flood calls d.erase but then directly deletes the data off the filesystem itself.

Transdroid also calls d.erase but then instead sets custom5 to 1. I believe this is actually an ruTorrent-specific behavior and does not apply to vanilla rTorrent.

ruTorrent seems to call d.delete_tied I think?

What is the correct behavior here in order to make the "delete data" functionality work?

Thank you!

bwitt commented 1 year ago

maybe it's d.delete_tied, since that seems to delete the underlying file?

bwitt commented 1 year ago

oh but ruTorrent does send a d.set_custom5 with a 1 or 2 just before calling d.delete_tied, and then sends a d.erase just after the d.delete_tied. so maybe we need to send a d.delete_tied in addition to what we're sending now.

erickok commented 1 year ago

It's true, there is no vanilla rTorrent command for it afaik. Since ruTorrent is so ubiquitous we decided to implement it's command at some point, but yeah that won't work with Flood. And calling d.delete_tied won't help much as that would just delete the .torrent file, not the contents (as I understand it).

Flood can delete the files directly itself as it runs on the server itself; Transdroid can't do that.

matoro commented 1 year ago

@bwitt Unfortunately https://github.com/erickok/transdroid/pull/655 does not fix it. I found this in the rtorrent docs which explains better what a "tied file" is, it's if you have a watch directory that you drop torrents in, then the .torrent it "tied" to that entry, so calling d.delete_tied just cleans up that file.

This seems to be the general approach that needs to be taken: https://superuser.com/a/1580313

However I have not managed to figure out how to turn this into something that can be called via the XMLRPC API.

I've been using the stock python client to experiment with this and it works great, all you need is something like this:

import xmlrpc.client
x = xmlrpc.client.ServerProxy("https://username:password@myhostname/RPC2")
x.system.listMethods()
x.download_list()
x.d.name("SOMEHASH")

etc...

matoro commented 1 year ago

Never mind, I figured it out! Here is how we can implement this over the API:

>>> x.method.list_keys("", "event.download.erased")
['!_download_list', '~_delete_tied']
>>> x.method.set_key("", "event.download.erased", "delete_erased", "execute=rm,-rf,--,$d.base_path=")
0
>>> x.method.list_keys("", "event.download.erased")
['!_download_list', 'delete_erased', '~_delete_tied']
>>> # Now delete your torrent through Transdroid.  Data is correctly wiped from disk
>>> # Restore default behavior
>>> x.method.set_key("", "event.download.erased", "delete_erased")
0
>>> x.method.list_keys("", "event.download.erased")
['!_download_list', '~_delete_tied']

Downside is that this is a completely global setting. I am using the empty string as a target (based on the discussion in https://github.com/Sonarr/Sonarr/commit/444fcf5ae5041598e5afa661402179b20e558bbe and https://github.com/rakshasa/rtorrent/issues/227) but even when specifying the hash of a particular torrent, it seems to register the handler globally regardless. Therefore if you delete two torrents at the same time from two different clients, one with "delete data" enabled and the other without, it's possible that the data would get deleted for both. But this is kind of a stretch in terms of use cases...

Note also that it seems recent rtorrent have d.delete_tied as a built-in erase trigger anyway, so there's no point in doing so manually.

Either way, this should be reasonable to port into Transdroid. @erickok would you be willing to implement this approach?

erickok commented 1 year ago

I am not sure if we can call x.method.set_key via RPC?

If we can this would functionally work perhaps, but it seems dangerous. At the very least this completely overrides any comment a user would have bound to that event himself. I don't know... it feels kinda hacky.

Of course you can say the same of calling custom5 which might not be the remove command Transdroid currently expects...

matoro commented 1 year ago

I am not sure if we can call x.method.set_key via RPC?

If we can this would functionally work perhaps, but it seems dangerous. At the very least this completely overrides any comment a user would have bound to that event himself. I don't know... it feels kinda hacky.

Of course you can say the same of calling custom5 which might not be the remove command Transdroid currently expects...

Yes you can, I am doing all this with the Python XMLRPC client. That's what the x variable is, it's like this:

>>> import xmlrpc.client
>>> x = xmlrpc.client.ServerProxy("https://username:password@myhostname/RPC2")
>>> x.method.list_keys("", "event.download.erased")
['!_download_list', 'delete_erased', '~_delete_tied']

Furthermore I tested a few more things, method.set_key is additive, if the key is not in the list it is added, and rTorrent executes all keys on an event trigger in alphabetical order (that is why builtin keys start with either ! to put them at the front of the list, or ~ to put them at the end of the list). You can add and delete keys on demand without affecting any other keys.

The only problem comes in if the user has a custom key tied to the event.download.erased trigger that expects the files to still be present on disk. But as long as their trigger executes first (is alphabetically before our trigger) then it should be perfectly fine, since we are calling rm -rf which does not bail out if the files are already gone.

matoro commented 5 months ago

@erickok https://github.com/erickok/transdroid/pull/655 does not fix this, see my explanation above.

erickok commented 5 months ago

Yes I was afraid of that. We'll have to try your solution.

matoro commented 5 months ago

Thanks, can you re-open the issue since it's not resolved?