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.
This commit is contained in:
Kovid Goyal 2013-10-18 09:56:00 +05:30
parent bbac6e953d
commit 4e46d4a9b3
2 changed files with 37 additions and 9 deletions

View File

@ -1406,6 +1406,8 @@ class DB(object):
source_ok = current_path and os.path.exists(spath) source_ok = current_path and os.path.exists(spath)
wam = WindowsAtomicFolderMove(spath) if iswindows and source_ok else None wam = WindowsAtomicFolderMove(spath) if iswindows and source_ok else None
format_map = {}
original_format_map = {}
try: try:
if not os.path.exists(tpath): if not os.path.exists(tpath):
os.makedirs(tpath) os.makedirs(tpath)
@ -1416,22 +1418,34 @@ class DB(object):
windows_atomic_move=wam, use_hardlink=True) windows_atomic_move=wam, use_hardlink=True)
for fmt in formats: for fmt in formats:
dest = os.path.join(tpath, fname+'.'+fmt.lower()) 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) dest, windows_atomic_move=wam, use_hardlink=True)
# Update db to reflect new file locations # Update db to reflect new file locations
for fmt in formats: for fmt in formats:
formats_field.table.set_fname(book_id, fmt, fname, self) formats_field.table.set_fname(book_id, fmt, fname, self)
path_field.table.set_path(book_id, path, self) path_field.table.set_path(book_id, path, self)
# Delete not needed directories # Delete not needed files and directories
if source_ok: if source_ok:
if os.path.exists(spath) and not samefile(spath, tpath): if os.path.exists(spath):
if wam is not None: if samefile(spath, tpath):
wam.delete_originals() # The format filenames may have changed while the folder
self.rmtree(spath) # name remains the same
parent = os.path.dirname(spath) for fmt, opath in original_format_map.iteritems():
if len(os.listdir(parent)) == 0: npath = format_map.get(fmt, None)
self.rmtree(parent) 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: finally:
if wam is not None: if wam is not None:
wam.close_handles() wam.close_handles()

View File

@ -76,6 +76,8 @@ class FilesystemTest(BaseTest):
f = open(fpath, 'rb') f = open(fpath, 'rb')
with self.assertRaises(IOError): with self.assertRaises(IOError):
cache.set_field('title', {1:'Moved'}) cache.set_field('title', {1:'Moved'})
with self.assertRaises(IOError):
cache.remove_books({1})
f.close() f.close()
self.assertNotEqual(cache.field_for('title', 1), 'Moved', 'Title was changed despite file lock') 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) self.assertLessEqual(len(cache.field_for('path', 1)), cache.backend.PATH_LIMIT * 2)
fpath = cache.format_abspath(1, cache.formats(1)[0]) fpath = cache.format_abspath(1, cache.formats(1)[0])
self.assertLessEqual(len(fpath), len(cache.backend.library_path) + cache.backend.PATH_LIMIT * 4) 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))))