From 53ef74ec85637a5d8ade791b57369694d793450f Mon Sep 17 00:00:00 2001 From: Kovid Goyal Date: Mon, 10 Apr 2023 12:04:54 +0530 Subject: [PATCH] Start work on supporting undo for book delete --- src/calibre/db/backend.py | 93 +++++++++++--- src/calibre/db/cache.py | 11 +- src/calibre/db/cli/cmd_remove.py | 3 - src/calibre/db/cli/cmd_remove_format.py | 2 - src/calibre/db/delete_service.py | 164 ------------------------ src/calibre/db/restore.py | 4 +- src/calibre/db/tests/add_remove.py | 2 - src/calibre/gui2/ui.py | 10 -- src/calibre/library/check_library.py | 3 +- src/calibre/srv/standalone.py | 6 +- 10 files changed, 87 insertions(+), 211 deletions(-) delete mode 100644 src/calibre/db/delete_service.py diff --git a/src/calibre/db/backend.py b/src/calibre/db/backend.py index 4a29f9923c..10d2695185 100644 --- a/src/calibre/db/backend.py +++ b/src/calibre/db/backend.py @@ -24,7 +24,6 @@ from calibre.constants import ( ) from calibre.db import SPOOL_SIZE, FTSQueryError from calibre.db.annotations import annot_db_data, unicode_normalize -from calibre.db.delete_service import delete_service from calibre.db.errors import NoSuchFormat from calibre.db.schema_upgrades import SchemaUpgrade from calibre.db.tables import ( @@ -36,6 +35,7 @@ from calibre.library.field_metadata import FieldMetadata from calibre.ptempfile import PersistentTemporaryFile, TemporaryFile from calibre.utils import pickle_binary_string, unpickle_binary_string from calibre.utils.config import from_json, prefs, to_json, tweaks +from calibre.utils.copy_files import copy_tree, copy_files from calibre.utils.date import EPOCH, parse_date, utcfromtimestamp, utcnow from calibre.utils.filenames import ( WindowsAtomicFolderMove, ascii_filename, atomic_rename, copyfile_using_links, @@ -55,6 +55,7 @@ from polyglot.builtins import ( # }}} +TRASH_DIR_NAME = '.caltrash' BOOK_ID_PATH_TEMPLATE = ' ({})' CUSTOM_DATA_TYPES = frozenset(('rating', 'text', 'comments', 'datetime', 'int', 'float', 'bool', 'series', 'composite', 'enumeration')) @@ -501,6 +502,14 @@ class DB: if load_user_formatter_functions: set_global_state(self) + @property + def last_expired_trash_at(self) -> float: + return float(self.prefs['last_expired_trash_at']) + + @last_expired_trash_at.setter + def last_expired_trash_at(self, val: float) -> None: + self.prefs['last_expired_trash_at'] = float(val) + def get_template_functions(self): return self._template_functions @@ -549,6 +558,8 @@ class DB: defs['similar_tags_match_kind'] = 'match_all' defs['similar_series_search_key'] = 'series' defs['similar_series_match_kind'] = 'match_any' + defs['last_expired_trash_at'] = 0.0 + defs['expire_old_trash_after'] = 7 * 86400 defs['book_display_fields'] = [ ('title', False), ('authors', True), ('series', True), ('identifiers', True), ('tags', True), ('formats', True), @@ -1519,17 +1530,14 @@ class DB: return os.path.getsize(dest_path) def remove_formats(self, remove_map): - paths = [] + self.ensure_trash_dir() + paths = set() for book_id, removals in iteritems(remove_map): for fmt, fname, path in removals: path = self.format_abspath(book_id, fmt, fname, path) - if path is not None: - paths.append(path) - try: - delete_service().delete_files(paths, self.library_path) - except: - import traceback - traceback.print_exc() + if path: + paths.add(path) + self.move_book_files_to_trash(book_id, paths) def cover_last_modified(self, path): path = os.path.abspath(os.path.join(self.library_path, path, 'cover.jpg')) @@ -1856,17 +1864,68 @@ class DB: with open(path, 'rb') as f: return f.read() + @property + def trash_dir(self): + return os.path.abspath(os.path.join(self.library_path, TRASH_DIR_NAME)) + + def ensure_trash_dir(self): + tdir = self.trash_dir + os.makedirs(os.path.join(tdir, 'b'), exist_ok=True) + os.makedirs(os.path.join(tdir, 'f'), exist_ok=True) + if iswindows: + import calibre_extensions.winutil as winutil + winutil.set_file_attributes(tdir, getattr(winutil, 'FILE_ATTRIBUTE_HIDDEN', 2) | getattr(winutil, 'FILE_ATTRIBUTE_NOT_CONTENT_INDEXED', 8192)) + if time.monotonic() - self.last_expired_trash_at >= 3600: + self.expire_old_trash() + + def expire_old_trash(self, expire_age_in_seconds=-1): + if expire_age_in_seconds < 0: + expire_age_in_seconds = max(1 * 24 * 3600, float(self.prefs['expire_old_trash_after'])) + self.last_expired_trash_at = now = time.time() + removals = [] + for base in ('b', 'f'): + base = os.path.join(self.trash_dir, base) + for entries in os.scandir(base): + for x in entries: + try: + st = x.stat(follow_symlinks=False) + mtime = st.st_mtime + except OSError: + mtime = 0 + if mtime + expire_age_in_seconds < now: + removals.append(x.path) + for x in removals: + rmtree_with_retry(x) + + def move_book_to_trash(self, book_id, book_dir_abspath): + dest = os.path.join(self.trash_dir, 'b', str(book_id)) + if os.path.exists(dest): + rmtree_with_retry(dest) + copy_tree(book_dir_abspath, dest, delete_source=True) + + def move_book_files_to_trash(self, book_id, format_abspaths): + dest = os.path.join(self.trash_dir, 'f', str(book_id)) + if not os.path.exists(dest): + os.makedirs(dest) + fmap = {} + for path in format_abspaths: + ext = path.rpartition('.')[-1].lower() + fmap[path] = os.path.join(dest, ext) + copy_files(fmap, delete_source=True) + def remove_books(self, path_map, permanent=False): + self.ensure_trash_dir() self.executemany( 'DELETE FROM books WHERE id=?', [(x,) for x in path_map]) - paths = {os.path.join(self.library_path, x) for x in itervalues(path_map) if x} - paths = {x for x in paths if os.path.exists(x) and self.is_deletable(x)} - if permanent: - for path in paths: - self.rmtree(path) - remove_dir_if_empty(os.path.dirname(path), ignore_metadata_caches=True) - else: - delete_service().delete_books(paths, self.library_path) + parent_paths = set() + for book_id, path in path_map.items(): + if path: + path = os.path.abspath(os.path.join(self.library_path, path)) + if os.path.exists(path) and self.is_deletable(path): + self.rmtree(path) if permanent else self.move_book_to_trash(book_id, path) + parent_paths.add(os.path.dirname(path)) + for path in parent_paths: + remove_dir_if_empty(path, ignore_metadata_caches=True) def add_custom_data(self, name, val_map, delete_first): if delete_first: diff --git a/src/calibre/db/cache.py b/src/calibre/db/cache.py index 8d4230db78..bb88c83143 100644 --- a/src/calibre/db/cache.py +++ b/src/calibre/db/cache.py @@ -2038,18 +2038,17 @@ class Cache: def remove_books(self, book_ids, permanent=False): ''' Remove the books specified by the book_ids from the database and delete their format files. If ``permanent`` is False, then the format files - are placed in the recycle bin. ''' + are placed in the per-library trash directory. ''' path_map = {} for book_id in book_ids: try: path = self._field_for('path', book_id).replace('/', os.sep) - except: + except Exception: path = None path_map[book_id] = path - if iswindows: - paths = (x.replace(os.sep, '/') for x in itervalues(path_map) if x) - self.backend.windows_check_if_files_in_use(paths) - + # ensure metadata.opf is written so we can restore the book + if not permanent: + self._dump_metadata(book_ids=tuple(bid for bid, path in path_map.items() if path)) self.backend.remove_books(path_map, permanent=permanent) for field in itervalues(self.fields): try: diff --git a/src/calibre/db/cli/cmd_remove.py b/src/calibre/db/cli/cmd_remove.py index 7c5aae9df3..e940765540 100644 --- a/src/calibre/db/cli/cmd_remove.py +++ b/src/calibre/db/cli/cmd_remove.py @@ -4,7 +4,6 @@ from calibre.constants import trash_name from calibre.db.cli import integers_from_string -from calibre.db.delete_service import delete_service from calibre.srv.changes import books_deleted readonly = False @@ -13,8 +12,6 @@ version = 0 # change this if you change signature of implementation() def implementation(db, notify_changes, ids, permanent): db.remove_books(ids, permanent=permanent) - if not permanent: - delete_service().wait() if notify_changes is not None: notify_changes(books_deleted(ids)) diff --git a/src/calibre/db/cli/cmd_remove_format.py b/src/calibre/db/cli/cmd_remove_format.py index d9cf715beb..9710793fc6 100644 --- a/src/calibre/db/cli/cmd_remove_format.py +++ b/src/calibre/db/cli/cmd_remove_format.py @@ -2,7 +2,6 @@ # License: GPLv3 Copyright: 2017, Kovid Goyal -from calibre.db.delete_service import delete_service from calibre.srv.changes import formats_removed readonly = False @@ -13,7 +12,6 @@ def implementation(db, notify_changes, book_id, fmt): is_remote = notify_changes is not None fmt_map = {book_id: (fmt, )} db.remove_formats(fmt_map) - delete_service().wait() if is_remote: notify_changes(formats_removed(fmt_map)) diff --git a/src/calibre/db/delete_service.py b/src/calibre/db/delete_service.py deleted file mode 100644 index 8fff63d69c..0000000000 --- a/src/calibre/db/delete_service.py +++ /dev/null @@ -1,164 +0,0 @@ -#!/usr/bin/env python - - -__license__ = 'GPL v3' -__copyright__ = '2013, Kovid Goyal ' - -import os, tempfile, shutil, errno, time, atexit -from threading import Thread - -from calibre.constants import ismacos -from calibre.ptempfile import remove_dir -from calibre.utils.filenames import remove_dir_if_empty -from calibre.utils.recycle_bin import delete_tree, delete_file -from polyglot.queue import Queue - - -class DeleteService(Thread): - - ''' Provide a blocking file delete implementation with support for the - recycle bin. On windows, deleting files to the recycle bin spins the event - loop, which can cause locking errors in the main thread. We get around this - by only moving the files/folders to be deleted out of the library in the - main thread, they are deleted to recycle bin in a separate worker thread. - - This has the added advantage that doing a restore from the recycle bin won't - cause metadata.db and the file system to get out of sync. Also, deleting - becomes much faster, since in the common case, the move is done by a simple - os.rename(). The downside is that if the user quits calibre while a long - move to recycle bin is happening, the files may not all be deleted.''' - - daemon = True - - def __init__(self): - Thread.__init__(self) - self.requests = Queue() - if ismacos: - from calibre_extensions.cocoa import enable_cocoa_multithreading - enable_cocoa_multithreading() - - def shutdown(self, timeout=20): - self.requests.put(None) - self.join(timeout) - - def create_staging(self, library_path): - base_path = os.path.dirname(library_path) - base = os.path.basename(library_path) - try: - ans = tempfile.mkdtemp(prefix=base+' deleted ', dir=base_path) - except OSError: - ans = tempfile.mkdtemp(prefix=base+' deleted ') - atexit.register(remove_dir, ans) - return ans - - def remove_dir_if_empty(self, path): - try: - os.rmdir(path) - except OSError as e: - if e.errno == errno.ENOTEMPTY or len(os.listdir(path)) > 0: - # Some linux systems appear to raise an EPERM instead of an - # ENOTEMPTY, see https://bugs.launchpad.net/bugs/1240797 - return - raise - - def delete_books(self, paths, library_path): - tdir = self.create_staging(library_path) - self.queue_paths(tdir, paths, delete_empty_parent=True) - - def queue_paths(self, tdir, paths, delete_empty_parent=True): - try: - self._queue_paths(tdir, paths, delete_empty_parent=delete_empty_parent) - except: - if os.path.exists(tdir): - shutil.rmtree(tdir, ignore_errors=True) - raise - - def _queue_paths(self, tdir, paths, delete_empty_parent=True): - requests = [] - for path in paths: - if os.path.exists(path): - basename = os.path.basename(path) - c = 0 - while True: - dest = os.path.join(tdir, basename) - if not os.path.exists(dest): - break - c += 1 - basename = '%d - %s' % (c, os.path.basename(path)) - try: - shutil.move(path, dest) - except OSError: - if os.path.isdir(path): - # shutil.move may have partially copied the directory, - # so the subsequent call to move() will fail as the - # destination directory already exists - raise - # Wait a little in case something has locked a file - time.sleep(1) - shutil.move(path, dest) - if delete_empty_parent: - remove_dir_if_empty(os.path.dirname(path), ignore_metadata_caches=True) - requests.append(dest) - if not requests: - remove_dir_if_empty(tdir) - else: - self.requests.put(tdir) - - def delete_files(self, paths, library_path): - tdir = self.create_staging(library_path) - self.queue_paths(tdir, paths, delete_empty_parent=False) - - def run(self): - while True: - x = self.requests.get() - try: - if x is None: - break - try: - self.do_delete(x) - except: - import traceback - traceback.print_exc() - finally: - self.requests.task_done() - - def wait(self): - 'Blocks until all pending deletes have completed' - self.requests.join() - - def do_delete(self, tdir): - if os.path.exists(tdir): - try: - for x in os.listdir(tdir): - x = os.path.join(tdir, x) - if os.path.isdir(x): - delete_tree(x) - else: - delete_file(x) - finally: - shutil.rmtree(tdir) - - -__ds = None - - -def delete_service(): - global __ds - if __ds is None: - __ds = DeleteService() - __ds.start() - return __ds - - -def shutdown(timeout=20): - global __ds - if __ds is not None: - __ds.shutdown(timeout) - __ds = None - - -def has_jobs(): - global __ds - if __ds is not None: - return (not __ds.requests.empty()) or __ds.requests.unfinished_tasks - return False diff --git a/src/calibre/db/restore.py b/src/calibre/db/restore.py index 2e03435713..e6269e2cdc 100644 --- a/src/calibre/db/restore.py +++ b/src/calibre/db/restore.py @@ -16,7 +16,7 @@ from threading import Thread from calibre import force_unicode, isbytestring from calibre.constants import filesystem_encoding -from calibre.db.backend import DB, DBPrefs +from calibre.db.backend import DB, TRASH_DIR_NAME, DBPrefs from calibre.db.cache import Cache from calibre.ebooks.metadata.opf2 import OPF from calibre.ptempfile import TemporaryDirectory @@ -160,6 +160,8 @@ class Restore(Thread): def scan_library(self): for dirpath, dirnames, filenames in os.walk(self.src_library_path): + with suppress(ValueError): + dirnames.remove(TRASH_DIR_NAME) leaf = os.path.basename(dirpath) m = self.db_id_regexp.search(leaf) if m is None or 'metadata.opf' not in filenames: diff --git a/src/calibre/db/tests/add_remove.py b/src/calibre/db/tests/add_remove.py index 7b6f678064..290962d250 100644 --- a/src/calibre/db/tests/add_remove.py +++ b/src/calibre/db/tests/add_remove.py @@ -261,7 +261,6 @@ class AddRemoveTest(BaseTest): self.assertFalse(table.col_book_map) # Test the delete service - from calibre.db.delete_service import delete_service cache = self.init_cache(cl) # Check that files are removed fmtpath = cache.format_abspath(1, 'FMT1') @@ -269,7 +268,6 @@ class AddRemoveTest(BaseTest): authorpath = os.path.dirname(bookpath) item_id = {v:k for k, v in iteritems(cache.fields['#series'].table.id_map)}['My Series Two'] cache.remove_books((1,)) - delete_service().wait() for x in (fmtpath, bookpath, authorpath): af(os.path.exists(x), 'The file %s exists, when it should not' % x) diff --git a/src/calibre/gui2/ui.py b/src/calibre/gui2/ui.py index 93013cf814..efceb8843f 100644 --- a/src/calibre/gui2/ui.py +++ b/src/calibre/gui2/ui.py @@ -1141,14 +1141,6 @@ class Main(MainWindow, MainWindowMixin, DeviceMixin, EmailMixin, # {{{ if not question_dialog(self, _('Library updates waiting'), msg): return False - from calibre.db.delete_service import has_jobs - if has_jobs(): - msg = _('Some deleted books are still being moved to the recycle ' - 'bin, if you quit now, they will be left behind. Are you ' - 'sure you want to quit?') - if not question_dialog(self, _('Active jobs'), msg): - return False - return True def shutdown(self, write_settings=True): @@ -1229,8 +1221,6 @@ class Main(MainWindow, MainWindowMixin, DeviceMixin, EmailMixin, # {{{ self._spare_pool.shutdown() from calibre.scraper.simple import cleanup_overseers wait_for_cleanup = cleanup_overseers() - from calibre.db.delete_service import shutdown - shutdown() from calibre.live import async_stop_worker wait_for_stop = async_stop_worker() time.sleep(2) diff --git a/src/calibre/library/check_library.py b/src/calibre/library/check_library.py index dedbfef5db..09799d3959 100644 --- a/src/calibre/library/check_library.py +++ b/src/calibre/library/check_library.py @@ -15,10 +15,11 @@ from calibre.constants import filesystem_encoding from calibre.ebooks import BOOK_EXTENSIONS from calibre.utils.localization import _ from polyglot.builtins import iteritems +from calibre.db.backend import TRASH_DIR_NAME EBOOK_EXTENSIONS = frozenset(BOOK_EXTENSIONS) NORMALS = frozenset({'metadata.opf', 'cover.jpg'}) -IGNORE_AT_TOP_LEVEL = frozenset({'metadata.db', 'metadata_db_prefs_backup.json', 'metadata_pre_restore.db', 'full-text-search.db'}) +IGNORE_AT_TOP_LEVEL = frozenset({'metadata.db', 'metadata_db_prefs_backup.json', 'metadata_pre_restore.db', 'full-text-search.db', TRASH_DIR_NAME}) ''' Checks fields: diff --git a/src/calibre/srv/standalone.py b/src/calibre/srv/standalone.py index 5ad690e576..45276a555d 100644 --- a/src/calibre/srv/standalone.py +++ b/src/calibre/srv/standalone.py @@ -9,7 +9,6 @@ import sys from calibre import as_unicode from calibre.constants import is_running_from_develop, ismacos, iswindows -from calibre.db.delete_service import shutdown as shutdown_delete_service from calibre.db.legacy import LibraryDatabase from calibre.srv.bonjour import BonJour from calibre.srv.handler import Handler @@ -243,7 +242,4 @@ def main(args=sys.argv): from calibre.gui2 import ensure_app, load_builtin_fonts ensure_app(), load_builtin_fonts() with HandleInterrupt(server.stop): - try: - server.serve_forever() - finally: - shutdown_delete_service() + server.serve_forever()