Merge pull request #8017 from drew2a/fix/8016

Refactor magnet link copy function

src/tribler/gui/tests/test_utilities.py

@@ -93,27 +93,55 @@ def test_quote_plus_unicode_compound():
 def test_compose_magnetlink():
     infohash = "DC4B96CF85A85CEEDB8ADC4B96CF85A85CEEDB8A"
     name = "Some torrent name"
-    trackers = ['http://tracker1.example.com:8080/announce', 'http://tracker1.example.com:8080/announce']
+    trackers = [
+        {'url': 'http://tracker1.example.com:8080/announce'},
+        {'url': 'http://tracker1.example.com:8080/announce'},
+    ]
 
-    expected_link0 = ""
     expected_link1 = "magnet:?xt=urn:btih:DC4B96CF85A85CEEDB8ADC4B96CF85A85CEEDB8A"
-    expected_link2 = "magnet:?xt=urn:btih:DC4B96CF85A85CEEDB8ADC4B96CF85A85CEEDB8A&dn=Some+torrent+name"
     expected_link3 = (
         "magnet:?xt=urn:btih:DC4B96CF85A85CEEDB8ADC4B96CF85A85CEEDB8A&dn=Some+torrent+name"
         "&tr=http://tracker1.example.com:8080/announce&tr=http://tracker1.example.com:8080/announce"
     )
 
-    composed_link0 = compose_magnetlink(None)
     composed_link1 = compose_magnetlink(infohash)
-    composed_link2 = compose_magnetlink(infohash, name=name)
     composed_link3 = compose_magnetlink(infohash, name=name, trackers=trackers)
 
-    assert composed_link0 == expected_link0
     assert composed_link1 == expected_link1
-    assert composed_link2 == expected_link2
     assert composed_link3 == expected_link3
 
 
+def test_create_magnet_no_input():
+    # Test that the create_magnet method returns an empty string when no input is provided
+    magnet = compose_magnetlink(None, None, [])
+    assert magnet == ''
+
+
+def test_create_magnet_no_trackers():
+    # Test that the create_magnet method returns a magnet link without trackers when no trackers are provided
+    magnet = compose_magnetlink('infohash', 'name', [])
+    assert magnet == 'magnet:?xt=urn:btih:infohash&dn=name'
+
+
+def test_create_magnet_no_urls():
+    # Test that the create_magnet method returns a magnet link without trackers when no urls are provided
+    magnet = compose_magnetlink('infohash', 'name', [{}, {'url': 'tracker'}])
+    assert magnet == 'magnet:?xt=urn:btih:infohash&dn=name&tr=tracker'
+
+
+def test_create_magnet_invalid_urls():
+    # Test that the create_magnet method returns a magnet link only for trackers when valid urls
+    trackers = [{'url': '[PeX]'}, {'url': '[DHT]'}, {'url': 'tracker'}]
+    magnet = compose_magnetlink('infohash', 'name', trackers)
+    assert magnet == 'magnet:?xt=urn:btih:infohash&dn=name&tr=tracker'
+
+
+def test_create_magnet_multiple_trackers():
+    # Test that the create_magnet method returns a magnet link with multiple trackers
+    magnet = compose_magnetlink('infohash', 'name', [{'url': 'tracker1'}, {'url': 'tracker2'}])
+    assert magnet == 'magnet:?xt=urn:btih:infohash&dn=name&tr=tracker1&tr=tracker2'
+
+
 def test_is_dict_has():
     assert not dict_item_is_any_of(None, None, None)
     assert not dict_item_is_any_of({}, None, None)

src/tribler/gui/utilities.py

@@ -4,7 +4,6 @@
 import logging
 import math
 import os
-import sys
 import time
 import traceback
 import types
@@ -31,6 +30,8 @@
 from tribler.core.utilities.utilities import is_frozen
 from tribler.gui.defs import CORRUPTED_DB_WAS_FIXED_MESSAGE, HEALTH_DEAD, HEALTH_GOOD, HEALTH_MOOT, HEALTH_UNCHECKED
 
+INVALID_URLS = {'[DHT]', '[PeX]'}
+
 # fmt: off
 
 logger = logging.getLogger(__name__)
@@ -352,7 +353,8 @@ def get_health(seeders, leechers, last_tracker_check):
         return HEALTH_DEAD
 
 
-def compose_magnetlink(infohash, name=None, trackers=None):
+def compose_magnetlink(infohash: Optional[str] = None, name: Optional[str] = None,
+                       trackers: Optional[List[Dict]] = None):
     """
     Composes magnet link from infohash, display name and trackers. The format is:
         magnet:?xt=urn:btih:<infohash>&dn=<name>[&tr=<tracker>]
@@ -367,9 +369,15 @@ def compose_magnetlink(infohash, name=None, trackers=None):
     magnet = f"magnet:?xt=urn:btih:{infohash}"
     if name:
         magnet = f"{magnet}&dn={quote_plus_unicode(name)}"
-    if trackers and isinstance(trackers, list):
-        for tracker in trackers:
-            magnet = f"{magnet}&tr={tracker}"
+
+    if not trackers:
+        return magnet
+
+    urls = (t.get('url') for t in trackers)
+    valid_urls = [u for u in urls if u and u not in INVALID_URLS]
+
+    for url in valid_urls:
+        magnet = f"{magnet}&tr={url}"
     return magnet
 
 

src/tribler/gui/widgets/downloadsdetailstabwidget.py

@@ -198,12 +198,15 @@ def update_pages(self, new_download=False):
                 item = QTreeWidgetItem(self.window().download_peers_list)
                 DownloadsDetailsTabWidget.update_peer_row(item, peer)
 
-    def on_copy_magnet_clicked(self, checked):
-        trackers = [
-            tk['url'] for tk in self.current_download['trackers'] if 'url' in tk and tk['url'] not in ['[DHT]', '[PeX]']
-        ]
-        magnet_link = compose_magnetlink(
-            self.current_download['infohash'], name=self.current_download.get('name', None), trackers=trackers
+    def on_copy_magnet_clicked(self, _):
+        """ Copy the magnet link of the current download to the clipboard."""
+        if not self.current_download:
+            return
+
+        magnet = compose_magnetlink(
+            infohash=self.current_download.get('infohash'),
+            name=self.current_download.get('name'),
+            trackers=self.current_download.get('trackers', [])
         )
-        copy_to_clipboard(magnet_link)
-        self.window().tray_show_message(tr("Copying magnet link"), magnet_link)
+        copy_to_clipboard(magnet)
+        self.window().tray_show_message(tr("Copying magnet link"), magnet)