Skip to content

Commit

Permalink
Fix bug in handling change of filename but not foldername
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
kovidgoyal committed Oct 18, 2013
1 parent bbac6e9 commit 4e46d4a
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 9 deletions.
32 changes: 23 additions & 9 deletions src/calibre/db/backend.py
Expand Up @@ -1406,6 +1406,8 @@ def update_path(self, book_id, title, author, path_field, formats_field):

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)
Expand All @@ -1416,22 +1418,34 @@ def update_path(self, book_id, title, author, path_field, formats_field):
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()
Expand Down
14 changes: 14 additions & 0 deletions src/calibre/db/tests/filesystem.py
Expand Up @@ -76,6 +76,8 @@ def test_windows_atomic_move(self):
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')

Expand Down Expand Up @@ -106,3 +108,15 @@ def test_long_filenames(self):
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))))

0 comments on commit 4e46d4a

Please sign in to comment.