From 4e46d4a9b3893b93dd6b22e339d2ca1eaf37310a Mon Sep 17 00:00:00 2001 From: Kovid Goyal Date: Fri, 18 Oct 2013 09:56:00 +0530 Subject: [PATCH] Fix bug in handling change of filename but not foldername Fix a bug triggered in very special circumstances (primarily on windows) that could cause duplicate files entries (hard links) to be created for a books' files when changing the title and author. The bug was triggered only if the title or author were between 32 and 35 characters in length and the book entry had originally been created by a pre 1.x version of calibre. --- src/calibre/db/backend.py | 32 +++++++++++++++++++++--------- src/calibre/db/tests/filesystem.py | 14 +++++++++++++ 2 files changed, 37 insertions(+), 9 deletions(-) diff --git a/src/calibre/db/backend.py b/src/calibre/db/backend.py index 9806ad927e..03187036f9 100644 --- a/src/calibre/db/backend.py +++ b/src/calibre/db/backend.py @@ -1406,6 +1406,8 @@ class DB(object): source_ok = current_path and os.path.exists(spath) wam = WindowsAtomicFolderMove(spath) if iswindows and source_ok else None + format_map = {} + original_format_map = {} try: if not os.path.exists(tpath): os.makedirs(tpath) @@ -1416,22 +1418,34 @@ class DB(object): windows_atomic_move=wam, use_hardlink=True) for fmt in formats: dest = os.path.join(tpath, fname+'.'+fmt.lower()) - self.copy_format_to(book_id, fmt, formats_field.format_fname(book_id, fmt), current_path, + format_map[fmt] = dest + ofmt_fname = formats_field.format_fname(book_id, fmt) + original_format_map[fmt] = os.path.join(spath, ofmt_fname+'.'+fmt.lower()) + self.copy_format_to(book_id, fmt, ofmt_fname, current_path, dest, windows_atomic_move=wam, use_hardlink=True) # Update db to reflect new file locations for fmt in formats: formats_field.table.set_fname(book_id, fmt, fname, self) path_field.table.set_path(book_id, path, self) - # Delete not needed directories + # Delete not needed files and directories if source_ok: - if os.path.exists(spath) and not samefile(spath, tpath): - if wam is not None: - wam.delete_originals() - self.rmtree(spath) - parent = os.path.dirname(spath) - if len(os.listdir(parent)) == 0: - self.rmtree(parent) + if os.path.exists(spath): + if samefile(spath, tpath): + # The format filenames may have changed while the folder + # name remains the same + for fmt, opath in original_format_map.iteritems(): + npath = format_map.get(fmt, None) + if npath and os.path.abspath(npath.lower()) != os.path.abspath(opath.lower()) and samefile(opath, npath): + # opath and npath are different hard links to the same file + os.unlink(opath) + else: + if wam is not None: + wam.delete_originals() + self.rmtree(spath) + parent = os.path.dirname(spath) + if len(os.listdir(parent)) == 0: + self.rmtree(parent) finally: if wam is not None: wam.close_handles() diff --git a/src/calibre/db/tests/filesystem.py b/src/calibre/db/tests/filesystem.py index 5821c41433..132be6961b 100644 --- a/src/calibre/db/tests/filesystem.py +++ b/src/calibre/db/tests/filesystem.py @@ -76,6 +76,8 @@ class FilesystemTest(BaseTest): f = open(fpath, 'rb') with self.assertRaises(IOError): cache.set_field('title', {1:'Moved'}) + with self.assertRaises(IOError): + cache.remove_books({1}) f.close() self.assertNotEqual(cache.field_for('title', 1), 'Moved', 'Title was changed despite file lock') @@ -106,3 +108,15 @@ class FilesystemTest(BaseTest): self.assertLessEqual(len(cache.field_for('path', 1)), cache.backend.PATH_LIMIT * 2) fpath = cache.format_abspath(1, cache.formats(1)[0]) self.assertLessEqual(len(fpath), len(cache.backend.library_path) + cache.backend.PATH_LIMIT * 4) + + def test_fname_change(self): + ' Test the changing of the filename but not the folder name ' + cache = self.init_cache() + title = 'a'*30 + 'bbb' + cache.backend.PATH_LIMIT = 100 + cache.set_field('title', {3:title}) + cache.add_format(3, 'TXT', BytesIO(b'xxx')) + cache.backend.PATH_LIMIT = 40 + cache.set_field('title', {3:title}) + fpath = cache.format_abspath(3, 'TXT') + self.assertEqual(sorted([os.path.basename(fpath)]), sorted(os.listdir(os.path.dirname(fpath))))