MBAPIHelper: introduce _make_inc_arg() to build inc parameter value

- it produces a more stable output (elements are sorted)
- it gets rid of empty elements (preventing a++b case)
- it accepts any iterable
- add a test for it

I also removed 2 useless conversion to tuple() (it can use a set() anyway)

picard/album.py

@@ -628,7 +628,7 @@ class Album(DataObject, Item):
         self.load_task = self.tagger.mb_api.get_release_by_id(
             self.id,
             self._release_request_finished,
-            inc=tuple(inc),
+            inc=inc,
             mblogin=require_authentication,
             priority=priority,
             refresh=refresh

picard/track.py

@@ -408,7 +408,7 @@ class NonAlbumTrack(Track):
         self.tagger.mb_api.get_track_by_id(
             self.id,
             self._recording_request_finished,
-            inc=tuple(inc),
+            inc=inc,
             mblogin=require_authentication,
             priority=priority,
             refresh=refresh

picard/webservice/api_helpers.py

@@ -129,7 +129,7 @@ class MBAPIHelper(APIHelper):
     def _get_by_id(self, entitytype, entityid, handler, inc=None, **kwargs):
         if inc:
             kwargs['unencoded_queryargs'] = kwargs.get('queryargs', {})
-            kwargs['unencoded_queryargs']["inc"] = "+".join(sorted(set(inc)))
+            kwargs['unencoded_queryargs']['inc'] = self._make_inc_arg(inc)
         return self.get(f"/{entitytype}/{entityid}", handler, **kwargs)
 
     def get_release_by_id(self, releaseid, handler, inc=None, **kwargs):
@@ -178,11 +178,21 @@ class MBAPIHelper(APIHelper):
     def find_artists(self, handler, **kwargs):
         return self._find('artist', handler, **kwargs)
 
+    @staticmethod
+    def _make_inc_arg(inc):
+        """
+        Convert an iterable to a string to be passed as inc paramater to MB
+
+        It drops non-unique and empty elements, and sort them before joining
+        them as a '+'-separated string
+        """
+        return '+'.join(sorted(set(str(e) for e in inc if e)))
+
     def _browse(self, entitytype, handler, inc=None, queryargs=None, mblogin=False):
         if queryargs is None:
             queryargs = {}
         if inc:
-            queryargs["inc"] = "+".join(inc)
+            queryargs["inc"] = self._make_inc_arg(inc)
         return self.get(f"/{entitytype}", handler, unencoded_queryargs=queryargs,
                         priority=True, important=True, mblogin=mblogin,
                         refresh=False)
@@ -214,7 +224,7 @@ class MBAPIHelper(APIHelper):
             inc = ("releases", "artist-credits", "media")
             path = f"/collection/{collection_id}/releases"
             queryargs = {
-                "inc": "+".join(inc),
+                "inc": self._make_inc_arg(inc),
                 "limit": limit,
                 "offset": offset,
             }

test/test_api_helpers.py

@@ -98,7 +98,7 @@ class MBAPITest(PicardTestCase):
         self.assertEqual(value, unencoded_query_args[argname])
 
     def _test_inc_args(self, ws_function, arg_list):
-        self.assertInQuery(self.ws.get_url, 'inc', "+".join(arg_list))
+        self.assertInQuery(self.ws.get_url, 'inc', self.api._make_inc_arg(arg_list))
 
     def test_get_release(self):
         inc_args_list = ['test']
@@ -212,6 +212,11 @@ class MBAPITest(PicardTestCase):
         with self.assertRaises(StopIteration):
             next(generator)
 
+    def test_make_inc_arg(self):
+        result = self.api._make_inc_arg(['b', 'a', '', 1, (), 0])
+        expected = '1+a+b'
+        self.assertEqual(result, expected)
+
 
 class AcoustdIdAPITest(PicardTestCase):