From c2874ed671eb000cf2eb76e860f7d85deff7670d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ionu=C8=9B=20Cioc=C3=AErlan?= Date: Tue, 11 Jun 2013 18:19:15 +0300 Subject: [PATCH] tests and subsequent fixes --- picard/util/__init__.py | 50 +++++++++++++++++---------------- test/test_utils.py | 62 ++++++++++++++++++++++++++++++++--------- 2 files changed, 75 insertions(+), 37 deletions(-) diff --git a/picard/util/__init__.py b/picard/util/__init__.py index 38ca25b65..297027e78 100644 --- a/picard/util/__init__.py +++ b/picard/util/__init__.py @@ -221,42 +221,38 @@ def _shorten_to_ratio(lst, ratio): ) -def make_win_short_filename(target, relpath, gain=0): - """Shorten a filename according to WinAPI quirks.""" +def _make_win_short_filename(relpath, reserved=0): + """Shorten a relative file path according to WinAPI quirks. + + relpath: The file's path. + reserved: Number of characters which will be reserved. + """ # See: # http://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx # # The MAX_PATH is 260 characters, with this possible format for a file: # "X:\<244-char dir path>\<11-char filename>". - # This means on Windows we'll need to generate a result of the form: - # "<247-char dir path>, <11-char filename>". - # If we're on *nix we'll reserve 3 chars for "X:\". # Our constraints: MAX_FILEPATH_LEN = 259 MAX_DIRPATH_LEN = 247 MAX_NODE_LEN = 226 # This seems to be the case for older NTFS - if sys.platform == "win32": - reserved = len(target) + 1 # ending slash - else: - reserved = len(target) - gain + 3 # "X:\" - remaining = MAX_DIRPATH_LEN - reserved relpath = shorten_path(relpath, MAX_NODE_LEN) if remaining >= len(relpath): # we're home free - return os.path.join(target, relpath) + return relpath # compute the directory path and the maximum number of characters # in a filename, and cache them dirpath, filename = os.path.split(relpath) try: - computed = make_win_short_filename._computed + computed = _make_win_short_filename._computed except AttributeError: - computed = make_win_short_filename._computed = {} + computed = _make_win_short_filename._computed = {} try: - fulldirpath, filename_max = computed[(target, dirpath, gain)] + fulldirpath, filename_max = computed[(dirpath, reserved)] except KeyError: dirnames = dirpath.split(os.path.sep) # allocate space for the separators @@ -296,22 +292,22 @@ def make_win_short_filename(target, relpath, gain=0): dirnames = _shorten_to_ratio(dirnames, float(totalchars) / remaining) # here it is: - fulldirpath = os.path.join(target, *dirnames) + finaldirpath = os.path.join(*dirnames) # did we win back some chars from .floor()s and .strip()s? recovered = remaining - sum(map(len, dirnames)) # so how much do we have left for the filename? filename_max = MAX_FILEPATH_LEN - MAX_DIRPATH_LEN - 1 + recovered - # ^ the ending separator + # ^ the final separator # and don't forget to cache - computed[(target, dirpath, gain)] = (fulldirpath, filename_max) + computed[(dirpath, reserved)] = (finaldirpath, filename_max) # finally... fileroot, ext = os.path.splitext(filename) filename = fileroot[:filename_max - len(ext)].strip() + ext - return os.path.join(fulldirpath, filename) + return os.path.join(finaldirpath, filename) def make_short_filename(target, relpath, win_compat=False, relative_to=""): @@ -329,12 +325,18 @@ def make_short_filename(target, relpath, win_compat=False, relative_to=""): assert target.startswith(relative_to) and \ target.split(relative_to)[1][:1] in (os.path.sep, ''), \ "`relative_to` must be an ancestor of `target`" + # always strip the relpath parts + relpath = os.path.join(*[part.strip() for part in relpath.split(os.path.sep)]) # if we're on windows, fire away if sys.platform == "win32": - return make_win_short_filename(target, relpath) - # if we're being compatible, we can gain some characters + reserved = len(target) + if not target.endswith(os.path.sep): + reserved += 1 + return os.path.join(target, _make_win_short_filename(relpath, reserved)) + # if we're being compatible, figure out how much + # needs to be reserved for the target part if win_compat: - if target and not relative_to: + if not relative_to: # try to find out the parent mount point, # caching it for future lookups try: @@ -354,9 +356,9 @@ def make_short_filename(target, relpath, win_compat=False, relative_to=""): relative_to = os.path.dirname(target) # cache it mounts[target] = relative_to - gain = len(target) - len(relative_to) - return make_win_short_filename(target, relpath, gain=gain) - # on regular *nix, find the name length limit (or at most 255 bytes), + reserved = len(target) - len(relative_to) + 3 # for the drive name + relpath = _make_win_short_filename(relpath, reserved) + # find the name length limit (or at most 255 bytes), # and cache it try: limits = make_short_filename._limits diff --git a/test/test_utils.py b/test/test_utils.py index 131cf5c1b..b113189b9 100644 --- a/test/test_utils.py +++ b/test/test_utils.py @@ -2,6 +2,7 @@ import os.path import unittest +import sys from picard import util @@ -60,26 +61,61 @@ class SanitizeDateTest(unittest.TestCase): self.failIfEqual(util.sanitize_date("2006--02"), "2006-02") self.failIfEqual(util.sanitize_date("2006.03.02"), "2006-03-02") + class ShortFilenameTest(unittest.TestCase): - def test_short(self): - fn = util.make_short_filename("/home/me/", os.path.join("a1234567890", "b1234567890"), 255) - self.failUnlessEqual(fn, os.path.join("a1234567890", "b1234567890")) + def __init__(self, *args, **kwargs): + self.root = os.path.join(sys.platform == "win32" and "X:\\" or "/", "x" * 10) + super(ShortFilenameTest, self).__init__(*args, **kwargs) - def test_long(self): - fn = util.make_short_filename("/home/me/", os.path.join("a1234567890", "b1234567890"), 20) - self.failUnlessEqual(fn, os.path.join("a123456", "b1")) + @unittest.skipUnless(sys.platform == "win32", "windows test") + def test_unicode_on_windows(self): + fn = util.make_short_filename(self.root, os.path.join(*[u"\U00010916" * 100] * 2)) + self.assertEqual(fn, os.path.join(self.root, *[u"\U00010916" * 100] * 2)) - def test_long_2(self): - fn = util.make_short_filename("/home/me/", os.path.join("a1234567890", "b1234567890"), 22) - self.failUnlessEqual(fn, os.path.join("a12345678", "b1")) + @unittest.skipUnless(sys.platform != "win32", "non-windows test") + def test_unicode_on_nix(self): + fn = util.make_short_filename(self.root, os.path.join(*[u"\U00010916" * 100] * 2)) + self.assertEqual(fn, os.path.join(self.root, *[u"\U00010916" * (255/4)] * 2)) - def test_too_long(self): - self.failUnlessRaises(IOError, util.make_short_filename, "/home/me/", os.path.join("a1234567890", "b1234567890"), 10) + @unittest.skipUnless(sys.platform != "win32", "non-windows test") + def test_windows_compat_with_unicode_on_nix(self): + fn = util.make_short_filename(self.root, os.path.join(*[u"\U00010916" * 100] * 2), win_compat=True) + self.assertEqual(fn, os.path.join(self.root, *[u"\U00010916" * (255/4)] * 2)) + + def test_windows_shortening(self): + fn = util.make_short_filename(self.root, os.path.join("a" * 200, "b" * 200, "c" * 200), win_compat=True) + self.assertEqual(fn, os.path.join(self.root, "a" * 116, "b" * 116, "c" * 11)) + + @unittest.skipUnless(sys.platform != "win32", "non-windows test") + def test_windows_shortening_with_ancestor_on_nix(self): + fn = util.make_short_filename( + os.path.join(self.root, "w" * 10, "x" * 10, "y" * 9, "z" * 9), os.path.join("b" * 200, "c" * 200, "d" * 200), + win_compat=True, relative_to = self.root) + self.assertEqual(fn, os.path.join(self.root, "w" * 10, "x" * 10, "y" * 9, "z" * 9, "b" * 100, "c" * 100, "d" * 11)) + + def test_windows_selective_shortening(self): + root = self.root + "x" * (44 - 10 - 3) + fn = util.make_short_filename(root, os.path.join( + os.path.join(*["a" * 9] * 10 + ["b" * 15] * 10), "c" * 10), win_compat=True) + self.assertEqual(fn, os.path.join(root, os.path.join(*["a" * 9] * 10 + ["b" * 9] * 10), "c" * 10)) + + def test_windows_shortening_not_needed(self): + fn = util.make_short_filename(self.root + "x" * 33, os.path.join( + os.path.join(*["a" * 9] * 20), "b" * 10), win_compat=True) + self.assertEqual(fn, os.path.join(self.root + "x" * 33, os.path.join(*["a" * 9] * 20), "b" * 10)) + + def test_windows_path_too_long(self): + self.assertRaises(IOError, util.make_short_filename, + self.root + "x" * 230, os.path.join("a", "b", "c", "d"), win_compat=True) + + def test_windows_path_not_too_long(self): + fn = util.make_short_filename(self.root + "x" * 230, os.path.join("a", "b", "c"), win_compat=True) + self.assertEqual(fn, os.path.join(self.root + "x" * 230, "a", "b", "c")) def test_whitespace(self): - fn = util.make_short_filename("/home/me/", os.path.join("a1234567890 ", " b1234567890 "), 22) - self.failUnlessEqual(fn, os.path.join("a12345678", "b1")) + fn = util.make_short_filename(self.root, os.path.join("a1234567890 ", " b1234567890 ")) + self.assertEqual(fn, os.path.join(self.root, "a1234567890", "b1234567890")) class TranslateArtistTest(unittest.TestCase):