diff --git a/beets/library.py b/beets/library.py index 7fbabbc82..0e5c17319 100644 --- a/beets/library.py +++ b/beets/library.py @@ -23,6 +23,8 @@ from string import Template import logging from beets.mediafile import MediaFile, FileTypeError +MAX_FILENAME_LENGTH = 200 + # Fields in the "items" table; all the metadata available for items in the # library. These are used directly in SQL; they are vulnerable to injection if # accessible to the user. The fields are divided into read-write @@ -86,8 +88,14 @@ def _ancestry(path): and so on. For instance, _ancestry('/a/b/c') == ['/', '/a', '/a/b']. """ out = [] - while path and path != '/': + last_path = None + while path: path = os.path.dirname(path) + + if path == last_path: + break + last_path = path + if path: # don't yield '' out.insert(0, path) return out @@ -104,7 +112,24 @@ def _walk_files(path): for filebase in files: yield os.path.join(root, filebase) - +def _components(path): + """Return a list of the path components in path. For instance, + _components('/a/b/c') == ['a', 'b', 'c']. + """ + comps = [] + ances = _ancestry(path) + for anc in ances: + comp = os.path.basename(anc) + if comp: + comps.append(comp) + else: # root + comps.append(anc) + + last = os.path.basename(path) + if last: + comps.append(last) + + return comps @@ -298,13 +323,21 @@ class Item(object): else: value = str(value) mapping[key] = value - - # one additional substitution: extension - _, extension = os.path.splitext(self.path) - extension = extension[1:] # remove leading . - mapping[u'extension'] = extension + # Perform substitution. subpath = subpath_tmpl.substitute(mapping) + + # Truncate path components. + comps = _components(subpath) + for i, comp in enumerate(comps): + if len(comp) > MAX_FILENAME_LENGTH: + comps[i] = comp[:MAX_FILENAME_LENGTH] + subpath = os.path.join(*comps) + + # Preserve extension. + _, extension = os.path.splitext(self.path) + subpath += extension + return _normpath(os.path.join(libpath, subpath)) def move(self, copy=False): @@ -566,7 +599,7 @@ class ResultIterator(object): class Library(object): def __init__(self, path='library.blb', directory='~/Music', - path_format='$artist/$album/$track $title.$extension'): + path_format='$artist/$album/$track $title'): self.path = path self.directory = directory self.path_format = path_format diff --git a/test/test_db.py b/test/test_db.py index dcb9468b9..ebcdaf4c1 100755 --- a/test/test_db.py +++ b/test/test_db.py @@ -169,11 +169,11 @@ class DestinationTest(unittest.TestCase): self.i.album = 'one' self.assertEqual(self.i.destination(), np('base/one/two three')) - def test_destination_substitutes_extension(self): + def test_destination_preserves_extension(self): self.lib.directory = 'base' - self.lib.path_format = '$extension' + self.lib.path_format = '$title' self.i.path = 'hey.audioFormat' - self.assertEqual(self.i.destination(), np('base/audioFormat')) + self.assertEqual(self.i.destination(),np('base/the title.audioFormat')) def test_destination_pads_some_indices(self): self.lib.directory = 'base' @@ -186,6 +186,25 @@ class DestinationTest(unittest.TestCase): self.i.bpm = 5 self.i.year = 6 self.assertEqual(self.i.destination(), np('base/01 02 03 04 5 6')) + + def test_destination_escapes_slashes(self): + self.i.album = 'one/two' + dest = self.i.destination() + self.assertTrue('one' in dest) + self.assertTrue('two' in dest) + self.assertFalse('one/two' in dest) + + def test_destination_long_names_truncated(self): + self.i.title = 'X'*300 + self.i.artist = 'Y'*300 + for c in self.i.destination().split(os.path.sep): + self.assertTrue(len(c) <= 255) + + def test_destination_long_names_keep_extension(self): + self.i.title = 'X'*300 + self.i.path = 'something.extn' + dest = self.i.destination() + self.assertEqual(dest[-5:], '.extn') def suite(): return unittest.TestLoader().loadTestsFromName(__name__) diff --git a/test/test_files.py b/test/test_files.py index e9b8b14b9..92ace3d05 100755 --- a/test/test_files.py +++ b/test/test_files.py @@ -39,12 +39,11 @@ class MoveTest(unittest.TestCase): # set up the destination self.libdir = join('rsrc', 'testlibdir') self.lib.directory = self.libdir - self.lib.path_format = join('$artist', - '$album', '$title') + self.lib.path_format = join('$artist', '$album', '$title') self.i.artist = 'one' self.i.album = 'two' self.i.title = 'three' - self.dest = join(self.libdir, 'one', 'two', 'three') + self.dest = join(self.libdir, 'one', 'two', 'three.mp3') def tearDown(self): if os.path.exists(self.path): @@ -149,20 +148,34 @@ class AddTest(unittest.TestCase): def test_library_add_copies(self): self.lib.add(os.path.join('rsrc', 'full.mp3'), copy=True) - self.assertTrue(os.path.isfile(os.path.join(self.dir, 'item'))) - -class DestinationTest(unittest.TestCase): - def setUp(self): - self.lib = beets.library.Library(':memory:') - self.i = beets.library.Item.from_path(join('rsrc', 'full.mp3')) - self.i.library = self.lib + self.assertTrue(os.path.isfile(os.path.join(self.dir, 'item.mp3'))) - def test_destination_escapes_slashes(self): - self.i.album = 'one/two' - dest = self.i.destination() - self.assertTrue('one' in dest) - self.assertTrue('two' in dest) - self.assertFalse('one/two' in dest) +class HelperTest(unittest.TestCase): + def test_ancestry_works_on_file(self): + p = '/a/b/c' + a = ['/','/a','/a/b'] + self.assertEqual(beets.library._ancestry(p), a) + def test_ancestry_works_on_dir(self): + p = '/a/b/c/' + a = ['/', '/a', '/a/b', '/a/b/c'] + self.assertEqual(beets.library._ancestry(p), a) + def test_ancestry_works_on_relative(self): + p = 'a/b/c' + a = ['a', 'a/b'] + self.assertEqual(beets.library._ancestry(p), a) + + def test_components_works_on_file(self): + p = '/a/b/c' + a = ['/', 'a', 'b', 'c'] + self.assertEqual(beets.library._components(p), a) + def test_components_works_on_dir(self): + p = '/a/b/c/' + a = ['/', 'a', 'b', 'c'] + self.assertEqual(beets.library._components(p), a) + def test_components_works_on_relative(self): + p = 'a/b/c' + a = ['a', 'b', 'c'] + self.assertEqual(beets.library._components(p), a) def suite(): return unittest.TestLoader().loadTestsFromName(__name__)