Start work on supporting undo for book delete

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:
+                if path:
-                    paths.append(path)
+                    paths.add(path)
-        try:
+        self.move_book_files_to_trash(book_id, paths)
-            delete_service().delete_files(paths, self.library_path)
-        except:
-            import traceback
-            traceback.print_exc()
 
     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}
+        parent_paths = set()
-        paths = {x for x in paths if os.path.exists(x) and self.is_deletable(x)}
+        for book_id, path in path_map.items():
-        if permanent:
+            if path:
-            for path in paths:
+                path = os.path.abspath(os.path.join(self.library_path, path))
-                self.rmtree(path)
+                if os.path.exists(path) and self.is_deletable(path):
-                remove_dir_if_empty(os.path.dirname(path), ignore_metadata_caches=True)
+                    self.rmtree(path) if permanent else self.move_book_to_trash(book_id, path)
-        else:
+                    parent_paths.add(os.path.dirname(path))
-            delete_service().delete_books(paths, self.library_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:

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:
+        # ensure metadata.opf is written so we can restore the book
-            paths = (x.replace(os.sep, '/') for x in itervalues(path_map) if x)
+        if not permanent:
-            self.backend.windows_check_if_files_in_use(paths)
+            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:

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))
 

src/calibre/db/cli/cmd_remove_format.py

@@ -2,7 +2,6 @@
 # License: GPLv3 Copyright: 2017, Kovid Goyal <kovid at kovidgoyal.net>
 
 
-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))
 

src/calibre/db/delete_service.py

@@ -1,164 +0,0 @@
-#!/usr/bin/env python
-
-
-__license__ = 'GPL v3'
-__copyright__ = '2013, Kovid Goyal <kovid at kovidgoyal.net>'
-
-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

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:

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)
 

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)

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:

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()
-            server.serve_forever()
-        finally:
-            shutdown_delete_service()