Merge pull request #4438 from jaimeMF/singleton_unique_paths

Add path template "sunique" to disambiguate between singleton tracks

6 files changed, 219 insertions, 24 deletions

diff --git a/beets/config_default.yaml b/beets/config_default.yaml
index fd2dbf551..db36ef080 100644
--- a/beets/config_default.yaml
+++ b/beets/config_default.yaml
@@ -55,6 +55,11 @@ aunique:
     disambiguators: albumtype year label catalognum albumdisambig releasegroupdisambig
     bracket: '[]'
 
+sunique:
+    keys: artist title
+    disambiguators: year trackdisambig
+    bracket: '[]'
+
 overwrite_null:
   album: []
   track: []
diff --git a/beets/library.py b/beets/library.py
index c8fa2b5fc..3b8a85685 100644
--- a/beets/library.py
+++ b/beets/library.py
@@ -1683,15 +1683,89 @@ class DefaultTemplateFunctions:
         if album_id is None:
             return ''
 
-        memokey = ('aunique', keys, disam, album_id)
+        memokey = self._tmpl_unique_memokey('aunique', keys, disam, album_id)
         memoval = self.lib._memotable.get(memokey)
         if memoval is not None:
             return memoval
 
-        keys = keys or beets.config['aunique']['keys'].as_str()
-        disam = disam or beets.config['aunique']['disambiguators'].as_str()
+        album = self.lib.get_album(album_id)
+
+        return self._tmpl_unique(
+            'aunique', keys, disam, bracket, album_id, album, album.item_keys,
+            # Do nothing for singletons.
+            lambda a: a is None)
+
+    def tmpl_sunique(self, keys=None, disam=None, bracket=None):
+        """Generate a string that is guaranteed to be unique among all
+        singletons in the library who share the same set of keys.
+
+        A fields from "disam" is used in the string if one is sufficient to
+        disambiguate the albums. Otherwise, a fallback opaque value is
+        used. Both "keys" and "disam" should be given as
+        whitespace-separated lists of field names, while "bracket" is a
+        pair of characters to be used as brackets surrounding the
+        disambiguator or empty to have no brackets.
+        """
+        # Fast paths: no album, no item or library, or memoized value.
+        if not self.item or not self.lib:
+            return ''
+
+        if isinstance(self.item, Item):
+            item_id = self.item.id
+        else:
+            raise NotImplementedError("sunique is only implemented for items")
+
+        if item_id is None:
+            return ''
+
+        return self._tmpl_unique(
+            'sunique', keys, disam, bracket, item_id, self.item,
+            Item.all_keys(),
+            # Do nothing for non singletons.
+            lambda i: i.album_id is not None,
+            initial_subqueries=[dbcore.query.NoneQuery('album_id', True)])
+
+    def _tmpl_unique_memokey(self, name, keys, disam, item_id):
+        """Get the memokey for the unique template named "name" for the
+        specific parameters.
+        """
+        return (name, keys, disam, item_id)
+
+    def _tmpl_unique(self, name, keys, disam, bracket, item_id, db_item,
+                     item_keys, skip_item, initial_subqueries=None):
+        """Generate a string that is guaranteed to be unique among all items of
+        the same type as "db_item" who share the same set of keys.
+
+        A field from "disam" is used in the string if one is sufficient to
+        disambiguate the items. Otherwise, a fallback opaque value is
+        used. Both "keys" and "disam" should be given as
+        whitespace-separated lists of field names, while "bracket" is a
+        pair of characters to be used as brackets surrounding the
+        disambiguator or empty to have no brackets.
+
+        "name" is the name of the templates. It is also the name of the
+        configuration section where the default values of the parameters
+        are stored.
+
+        "skip_item" is a function that must return True when the template
+        should return an empty string.
+
+        "initial_subqueries" is a list of subqueries that should be included
+        in the query to find the ambigous items.
+        """
+        memokey = self._tmpl_unique_memokey(name, keys, disam, item_id)
+        memoval = self.lib._memotable.get(memokey)
+        if memoval is not None:
+            return memoval
+
+        if skip_item(db_item):
+            self.lib._memotable[memokey] = ''
+            return ''
+
+        keys = keys or beets.config[name]['keys'].as_str()
+        disam = disam or beets.config[name]['disambiguators'].as_str()
         if bracket is None:
-            bracket = beets.config['aunique']['bracket'].as_str()
+            bracket = beets.config[name]['bracket'].as_str()
         keys = keys.split()
         disam = disam.split()
 
@@ -1703,46 +1777,44 @@ class DefaultTemplateFunctions:
             bracket_l = ''
             bracket_r = ''
 
-        album = self.lib.get_album(album_id)
-        if not album:
-            # Do nothing for singletons.
-            self.lib._memotable[memokey] = ''
-            return ''
-
-        # Find matching albums to disambiguate with.
+        # Find matching items to disambiguate with.
         subqueries = []
+        if initial_subqueries is not None:
+            subqueries.extend(initial_subqueries)
         for key in keys:
-            value = album.get(key, '')
+            value = db_item.get(key, '')
             # Use slow queries for flexible attributes.
-            fast = key in album.item_keys
+            fast = key in item_keys
             subqueries.append(dbcore.MatchQuery(key, value, fast))
-        albums = self.lib.albums(dbcore.AndQuery(subqueries))
+        query = dbcore.AndQuery(subqueries)
+        ambigous_items = (self.lib.items(query)
+                          if isinstance(db_item, Item)
+                          else self.lib.albums(query))
 
-        # If there's only one album to matching these details, then do
+        # If there's only one item to matching these details, then do
         # nothing.
-        if len(albums) == 1:
+        if len(ambigous_items) == 1:
             self.lib._memotable[memokey] = ''
             return ''
 
-        # Find the first disambiguator that distinguishes the albums.
+        # Find the first disambiguator that distinguishes the items.
         for disambiguator in disam:
-            # Get the value for each album for the current field.
-            disam_values = {a.get(disambiguator, '') for a in albums}
+            # Get the value for each item for the current field.
+            disam_values = {s.get(disambiguator, '') for s in ambigous_items}
 
             # If the set of unique values is equal to the number of
-            # albums in the disambiguation set, we're done -- this is
+            # items in the disambiguation set, we're done -- this is
             # sufficient disambiguation.
-            if len(disam_values) == len(albums):
+            if len(disam_values) == len(ambigous_items):
                 break
-
         else:
             # No disambiguator distinguished all fields.
-            res = f' {bracket_l}{album.id}{bracket_r}'
+            res = f' {bracket_l}{item_id}{bracket_r}'
             self.lib._memotable[memokey] = res
             return res
 
         # Flatten disambiguation value into a string.
-        disam_value = album.formatted(for_path=True).get(disambiguator)
+        disam_value = db_item.formatted(for_path=True).get(disambiguator)
 
         # Return empty string if disambiguator is empty.
         if disam_value:
diff --git a/docs/changelog.rst b/docs/changelog.rst
index 31861af24..d21a55d37 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -35,6 +35,8 @@ New features:
 * :ref:`import-options`: Add support for re-running the importer on paths in
   log files that were created with the ``-l`` (or ``--logfile``) argument.
   :bug:`4379` :bug:`4387`
+* Add :ref:`%sunique{} <sunique>` template to disambiguate between singletons.
+  :bug:`4438`
 
 Bug fixes:
 
diff --git a/docs/reference/config.rst b/docs/reference/config.rst
index 6e7df1b59..58656256f 100644
--- a/docs/reference/config.rst
+++ b/docs/reference/config.rst
@@ -326,6 +326,23 @@ The defaults look like this::
 
 See :ref:`aunique` for more details.
 
+.. _config-sunique:
+
+sunique
+~~~~~~~
+
+These options are used to generate a string that is guaranteed to be unique
+among all singletons in the library who share the same set of keys.
+
+The defaults look like this::
+
+    sunique:
+        keys: artist title
+        disambiguators: year trackdisambig
+        bracket: '[]'
+
+See :ref:`sunique` for more details.
+
 
 .. _terminal_encoding:
 
diff --git a/docs/reference/pathformat.rst b/docs/reference/pathformat.rst
index f6f2e06cc..b52c2b32a 100644
--- a/docs/reference/pathformat.rst
+++ b/docs/reference/pathformat.rst
@@ -73,6 +73,8 @@ These functions are built in to beets:
   option.
 * ``%aunique{identifiers,disambiguators,brackets}``: Provides a unique string
   to disambiguate similar albums in the database. See :ref:`aunique`, below.
+* ``%sunique{identifiers,disambiguators,brackets}``: Provides a unique string
+  to disambiguate similar singletons in the database. See :ref:`sunique`, below.
 * ``%time{date_time,format}``: Return the date and time in any format accepted
   by `strftime`_. For example, to get the year some music was added to your
   library, use ``%time{$added,%Y}``.
@@ -145,6 +147,18 @@ its import time. Only the second album will receive a disambiguation string. If
 you want to add the disambiguation string to both albums, just run ``beet move``
 (possibly restricted by a query) to update the paths for the albums.
 
+.. _sunique:
+
+Singleton Disambiguation
+------------------------
+
+It is also possible to have singleton tracks with the same name and the same
+artist. Beets provides the ``%sunique{}`` template to avoid having the same
+file path.
+
+It has the same arguments as the :ref:`%aunique <aunique>` template, but the default
+values are different. The default identifiers are ``artist title`` and the
+default disambiguators are ``year trackdisambig``.
 
 Syntax Details
 --------------
diff --git a/test/test_library.py b/test/test_library.py
index 6981b87f9..31ced7a2c 100644
--- a/test/test_library.py
+++ b/test/test_library.py
@@ -805,6 +805,91 @@ class DisambiguationTest(_common.TestCase, PathFormattingMixin):
         self._assert_dest(b'/base/foo/the title', self.i1)
 
 
+class SingletonDisambiguationTest(_common.TestCase, PathFormattingMixin):
+    def setUp(self):
+        super().setUp()
+        self.lib = beets.library.Library(':memory:')
+        self.lib.directory = b'/base'
+        self.lib.path_formats = [('default', 'path')]
+
+        self.i1 = item()
+        self.i1.year = 2001
+        self.lib.add(self.i1)
+        self.i2 = item()
+        self.i2.year = 2002
+        self.lib.add(self.i2)
+        self.lib._connection().commit()
+
+        self._setf('foo/$title%sunique{artist title,year}')
+
+    def tearDown(self):
+        super().tearDown()
+        self.lib._connection().close()
+
+    def test_sunique_expands_to_disambiguating_year(self):
+        self._assert_dest(b'/base/foo/the title [2001]', self.i1)
+
+    def test_sunique_with_default_arguments_uses_trackdisambig(self):
+        self.i1.trackdisambig = 'live version'
+        self.i1.year = self.i2.year
+        self.i1.store()
+        self._setf('foo/$title%sunique{}')
+        self._assert_dest(b'/base/foo/the title [live version]', self.i1)
+
+    def test_sunique_expands_to_nothing_for_distinct_singletons(self):
+        self.i2.title = 'different track'
+        self.i2.store()
+
+        self._assert_dest(b'/base/foo/the title', self.i1)
+
+    def test_sunique_does_not_match_album(self):
+        self.lib.add_album([self.i2])
+        self._assert_dest(b'/base/foo/the title', self.i1)
+
+    def test_sunique_use_fallback_numbers_when_identical(self):
+        self.i2.year = self.i1.year
+        self.i2.store()
+
+        self._assert_dest(b'/base/foo/the title [1]', self.i1)
+        self._assert_dest(b'/base/foo/the title [2]', self.i2)
+
+    def test_sunique_falls_back_to_second_distinguishing_field(self):
+        self._setf('foo/$title%sunique{albumartist album,month year}')
+        self._assert_dest(b'/base/foo/the title [2001]', self.i1)
+
+    def test_sunique_sanitized(self):
+        self.i2.year = self.i1.year
+        self.i1.trackdisambig = 'foo/bar'
+        self.i2.store()
+        self.i1.store()
+        self._setf('foo/$title%sunique{artist title,trackdisambig}')
+        self._assert_dest(b'/base/foo/the title [foo_bar]', self.i1)
+
+    def test_drop_empty_disambig_string(self):
+        self.i1.trackdisambig = None
+        self.i2.trackdisambig = 'foo'
+        self.i1.store()
+        self.i2.store()
+        self._setf('foo/$title%sunique{albumartist album,trackdisambig}')
+        self._assert_dest(b'/base/foo/the title', self.i1)
+
+    def test_change_brackets(self):
+        self._setf('foo/$title%sunique{artist title,year,()}')
+        self._assert_dest(b'/base/foo/the title (2001)', self.i1)
+
+    def test_remove_brackets(self):
+        self._setf('foo/$title%sunique{artist title,year,}')
+        self._assert_dest(b'/base/foo/the title 2001', self.i1)
+
+    def test_key_flexible_attribute(self):
+        self.i1.flex = 'flex1'
+        self.i2.flex = 'flex2'
+        self.i1.store()
+        self.i2.store()
+        self._setf('foo/$title%sunique{artist title flex,year}')
+        self._assert_dest(b'/base/foo/the title', self.i1)
+
+
 class PluginDestinationTest(_common.TestCase):
     def setUp(self):
         super().setUp()