Don't apply auto-grouping until torrent is demagnetized

[skurtix]

If you add a magnet-link torrent it will bypass the auto-group conditions and be added without a group. (latest snow leopard, transmission 1.82, added from latest Safari)

[livings124]

When you set the milestone, you're either give a patch or giving us marching orders. And I just tested - my autogroup rule for name was applied just fine.

[Crypticus]

When a magnet link is added, while it is in it's red state (Locating the file info via the hash??) spaces and dots appear to be replaced by '+' characters. It appears that the code the performs the conditional matching is done before the torrent is downloaded from the DHT and the proper name of the file is generated. This causes the conditional group matching to fail on anything other than the most simplistic of matching.

Example: Condition is set to match "Name contains" = fringe.

This works for both the magnet link and downloading the actual torrent file.

However, Condition is set to match "Name contains" = how.i.met.your.mother

This work for the torrent, fails for the magnet link.

Proposed solution is to change to order in which conditions are tested again. Wait till after the torrent leave the red state, then test against conditionals.

As a side note it would be nice if there was a menu option to retest all torrent in list against the conditions, in the event you add a bunch of torrents, then decide to make a new group, this would allow transmisson to regroup automatically.

[skurtix]

Sorry about the mistakes in the ticket livings, but there is indeed a problem with auto-group conditions and magnet links.

[elpasi]

I made a ticket about this, but it was closed for being a dupe. I've found that:

Not only is it noticeable if matching by name and the dots become plusses (which I have not encountered), but also if the magnet link that is distributed doesn't contain a &dn= parameter at all, in which case all my name based rules fail miserably.

[yusf]

+1 For fixing this!

[livings124]

#4845 is a duplicate, but provides a nice potential change.

[evands]

Change-in-progress from #4845 (not claimed to be a complete or workable solution yet):

Index: macosx/Torrent.m
===================================================================
--- macosx/Torrent.m	(revision 13257)
+++ macosx/Torrent.m	(working copy)
@@ -1869,6 +1869,16 @@
     
     [self createFileList];
     
+	if ([self groupValue] == -1)
+		[self setGroupValue:[[GroupsController groups] groupIndexForTorrent: self]];
+
+	//change the location if the group calls for it and it's not already set
+	if (!tr_torrentGetCurrentDir(fHandle) && [[GroupsController groups] usesCustomDownloadLocationForIndex: [self groupValue]])
+	{
+		NSString *location = [[GroupsController groups] customDownloadLocationForIndex: [self groupValue]];
+		[self changeDownloadFolderBeforeUsing: location];
+	}
+	
     [[NSNotificationCenter defaultCenter] postNotificationName: @"ResetInspector" object: self];
 }

livings124 wrote:

...The patch looks pretty good, though. My main concern is that manually-set groups/locations will be lost when it's demagnetized. How do we strike a balance between re-applying groups and respecting user settings?

I included the if ([self groupValue] == -1) for that purpose; if a group has been assigned somehow, that'll no longer be the default -1 and we won't change anything.

Similarly, I checked the current directory for the torrent to avoid overwriting a manually selected download location. I'm not sure if architecturally that'll work as expected for a magnet link, though -- at what point is the planned download location for a magnet link resolved? I couldn't determine it in a brief perusal of the source.

[livings124]

I'm not sure that the groupValue check would cut it. What if it was put automatically in the wrong group before, for example? Maybe that shouldn't be a concern, though.

The location set for the magnet link wouldn't be used before demagnetizing, so that shouldn't be a concern. I'm not sure when the value is set in code, either, though.

[evands]

Replying to livings124:

I'm not sure that the groupValue check would cut it. What if it was put automatically in the wrong group before, for example? Maybe that shouldn't be a concern, though.

Good point. Perhaps we need to create a separate variable to track if this groupValue was user assigned or automatically determined, and if it was automatic (or not determined at all) then run the group assignment process.

[livings124]

That's what I was thinking, too, at least in terms of which group its in. Still not sure in regards to location. Perhaps in the magnet add window it should be an option?

[evands]

Hate to increase complexity with an extra option... how about if we check the location versus the default (the user's downloads directory, I assume? Haven't checked), and if it's still the default, we assume it's okay to follow group-location assignments, and if it's not, then it probably isn't because the user explicitly picked a new location.

[livings124]

Perhaps changing the location could set off the new flag, same as changing the group value.

[evands]

Yeah, I like that.

If that plan sounds good, I can put together a patch to implement this. It'll be a couple weeks before I have time to dive into it; no offense taken if you're interested and want to do it first.

[livings124]

Sounds good evands!

[jordan]

evands: any news?

[evands]

I haven't had a chance to work on this yet as I've mostly been on the road, but I still plan to.

[jordan]

Okay. Thanks for checking in :)

[jurek]

Any news? I just checked and in 2.51 on mac the bug still persists to annoy.. :-) As other people, I have a customized group based on "any file contains avi/mkv/mp4" and it works perfectly with .torrents, but won't with magnets. Please fix it:)

[TheDov]

I'm noticing this bug, too. Any updates? I'm on 2.61

[livings124]

evands: any news with this?

[evands]

Really, it's been 5 months? Time flies. Never again can I complain about someone following up really-late after promising to work on something...

I'm alive and still interested in fixing this bug. I don't have any progress to report because I've gotten caught up in a million other things. Thanks for the ping, and if nobody beats me to it I'll hopefully have a chance to knock it out soon.

[jurek]

Thank you!

[livings124]

Sounds good!

[evands]

Patch to fix reanalyzing groups/destinations for magnet links

[evands]

fix_magnet_link_grouping.patch fixes this issue and the potential concerns brought up by livings124 above, by tracking whether group and download folder changes are initiated by the user or automatically determined. When a magnet link's tracker data is downloaded, each of group and download location are redetermined, each only if it was not manually set by the user. The UI (in the Mac app) is properly updated when the group changes.

Patch is against [13561].

[livings124]

Thanks! I'll take a closer look and should get it in for 2.8.

[livings124]

The one thing I noticed is that in the add window, when setting the initial location (AddWindowController:130) you set the location with TorrentDeterminationAutomatic?. Should it be TorrentDeterminationUserSpecified? if fLockDestination is YES?

[livings124]

I've made the change in the above comment and submitted in r13602. If that's not right let me know. Thanks for the patch!

[evands]

Tracing the path of invocation, fLockDestination is only YES if the @"OpenCreatedTorrentFile" notification was posted and specified a path. That only happens from the CreatorWindowController. I think that means that your change to my patch is correct.