Edit metadata dialog: Improve performance by only writing changed fields to the databse when clicking OK or Next

Edit metadata dialog: Do not auto change the title sort field when
clicking OK if the title was changed. Instead the title sort field must
be changed explicitly.

src/calibre/gui2/custom_column_widgets.py

@@ -31,17 +31,20 @@ class Base(object):
 
     def initialize(self, book_id):
         val = self.db.get_custom(book_id, num=self.col_id, index_is_id=True)
-        self.initial_val = val
         val = self.normalize_db_val(val)
         self.setter(val)
+        self.initial_val = self.current_val  # self.current_val might be different from val thanks to normalization
+
+    @property
+    def current_val(self):
+        return self.normalize_ui_val(self.gui_val)
 
     @property
     def gui_val(self):
         return self.getter()
 
     def commit(self, book_id, notify=False):
-        val = self.gui_val
-        val = self.normalize_ui_val(val)
+        val = self.current_val
         if val != self.initial_val:
             return self.db.set_custom(book_id, val, num=self.col_id,
                             notify=notify, commit=False, allow_case_change=True)
@@ -329,13 +332,13 @@ class Text(Base):
         if isinstance(val, list):
             if not self.col_metadata.get('display', {}).get('is_names', False):
                 val.sort(key=sort_key)
-        self.initial_val = val
         val = self.normalize_db_val(val)
 
         if self.col_metadata['is_multiple']:
             self.setter(val)
         else:
             self.widgets[1].show_initial_value(val)
+        self.initial_val = self.current_val
 
     def setter(self, val):
         if self.col_metadata['is_multiple']:
@@ -374,7 +377,7 @@ class Text(Base):
             if d == QMessageBox.Yes:
                 self.commit(self.book_id)
                 self.db.commit()
-                self.initial_val = self.getter()
+                self.initial_val = self.current_val
             else:
                 self.setter(self.initial_val)
         d = TagEditor(self.parent, self.db, self.book_id, self.key)
@@ -404,9 +407,7 @@ class Series(Base):
         values = list(self.db.all_custom(num=self.col_id))
         values.sort(key=sort_key)
         val = self.db.get_custom(book_id, num=self.col_id, index_is_id=True)
-        self.initial_val = val
         s_index = self.db.get_custom_extra(book_id, num=self.col_id, index_is_id=True)
-        self.initial_index = s_index
         try:
             s_index = float(s_index)
         except (ValueError, TypeError):
@@ -417,6 +418,7 @@ class Series(Base):
         self.name_widget.update_items_cache(values)
         self.name_widget.show_initial_value(val)
         self.name_widget.blockSignals(False)
+        self.initial_val, self.initial_index = self.current_val
 
     def getter(self):
         n = unicode(self.name_widget.currentText()).strip()
@@ -434,11 +436,16 @@ class Series(Base):
                                                      num=self.col_id)
         self.idx_widget.setValue(s_index)
 
-    def commit(self, book_id, notify=False):
+    @property
+    def current_val(self):
         val, s_index = self.gui_val
         val = self.normalize_ui_val(val)
+        return val, s_index
+
+    def commit(self, book_id, notify=False):
+        val, s_index = self.current_val
         if val != self.initial_val or s_index != self.initial_index:
-            if val == '':
+            if not val:
                 val = s_index = None
             return self.db.set_custom(book_id, val, extra=s_index, num=self.col_id,
                                notify=notify, commit=False, allow_case_change=True)
@@ -460,7 +467,6 @@ class Enumeration(Base):
     def initialize(self, book_id):
         val = self.db.get_custom(book_id, num=self.col_id, index_is_id=True)
         val = self.normalize_db_val(val)
-        self.initial_val = val
         idx = self.widgets[1].findText(val)
         if idx < 0:
             error_dialog(self.parent, '',
@@ -471,6 +477,7 @@ class Enumeration(Base):
 
             idx = 0
         self.widgets[1].setCurrentIndex(idx)
+        self.initial_val = self.current_val
 
     def setter(self, val):
         self.widgets[1].setCurrentIndex(self.widgets[1].findText(val))

src/calibre/gui2/metadata/basic_widgets.py

@@ -7,7 +7,7 @@ __license__   = 'GPL v3'
 __copyright__ = '2011, Kovid Goyal <kovid@kovidgoyal.net>'
 __docformat__ = 'restructuredtext en'
 
-import textwrap, re, os, errno, shutil
+import textwrap, re, os, shutil
 
 from PyQt4.Qt import (
     Qt, QDateTimeEdit, pyqtSignal, QMessageBox, QIcon, QToolButton, QWidget,
@@ -26,7 +26,7 @@ from calibre.gui2 import (file_icon_provider, UNDEFINED_QDATETIME,
 from calibre.gui2.complete2 import EditWithComplete
 from calibre.utils.date import (
     local_tz, qt_to_dt, as_local_time, UNDEFINED_DATE, is_date_undefined)
-from calibre import strftime, force_unicode
+from calibre import strftime
 from calibre.ebooks import BOOK_EXTENSIONS
 from calibre.customize.ui import run_plugins_on_import
 from calibre.utils.date import utcfromtimestamp
@@ -74,7 +74,6 @@ class BasicMetadataWidget(object):
 class TitleEdit(EnLineEdit):
 
     TITLE_ATTR = 'title'
-    COMMIT = True
     TOOLTIP = _('Change the title of this book')
     LABEL = _('&Title:')
 
@@ -92,29 +91,16 @@ class TitleEdit(EnLineEdit):
         self.current_val = title
         self.original_val = self.current_val
 
+    @property
+    def changed(self):
+        return self.original_val != self.current_val
+
     def commit(self, db, id_):
         title = self.current_val
-        if title != self.original_val:
+        if self.changed:
             # Only try to commit if changed. This allow setting of other fields
             # to work even if some of the book files are opened in windows.
-            try:
-                if self.COMMIT:
-                    getattr(db, 'set_'+ self.TITLE_ATTR)(id_, title, notify=False)
-                else:
-                    getattr(db, 'set_'+ self.TITLE_ATTR)(id_, title, notify=False,
-                            commit=False)
-            except (IOError, OSError) as err:
-                if getattr(err, 'errno', None) == errno.EACCES:  # Permission denied
-                    import traceback
-                    fname = getattr(err, 'filename', None)
-                    p = 'Locked file: %s\n\n'%fname if fname else ''
-                    error_dialog(self, _('Permission denied'),
-                            _('Could not change the on disk location of this'
-                                ' book. Is it open in another program?'),
-                            det_msg=p+traceback.format_exc(), show=True)
-                    return False
-                raise
-        return True
+            getattr(db, 'set_'+ self.TITLE_ATTR)(id_, title, notify=False)
 
     @dynamic_property
     def current_val(self):
@@ -141,7 +127,6 @@ class TitleEdit(EnLineEdit):
 class TitleSortEdit(TitleEdit):
 
     TITLE_ATTR = 'title_sort'
-    COMMIT = False
     TOOLTIP = _('Specify how this book should be sorted when by title.'
             ' For example, The Exorcist might be sorted as Exorcist, The.')
     LABEL = _('Title &sort:')
@@ -170,6 +155,10 @@ class TitleSortEdit(TitleEdit):
         languages_edit.currentIndexChanged.connect(self.update_state)
         self.update_state()
 
+    @property
+    def changed(self):
+        return self.title_edit.changed or self.original_val != self.current_val
+
     @property
     def book_lang(self):
         try:
@@ -219,7 +208,7 @@ class AuthorsEdit(EditWithComplete):
 
     def __init__(self, parent, manage_authors):
         self.dialog = parent
-        self.books_to_refresh = set([])
+        self.books_to_refresh = set()
         EditWithComplete.__init__(self, parent)
         self.setToolTip(self.TOOLTIP)
         self.setWhatsThis(self.TOOLTIP)
@@ -267,6 +256,10 @@ class AuthorsEdit(EditWithComplete):
     def get_default(self):
         return _('Unknown')
 
+    @property
+    def changed(self):
+        return self.original_val != self.current_val
+
     def initialize(self, db, id_):
         self.books_to_refresh = set([])
         self.set_separator('&')
@@ -287,21 +280,8 @@ class AuthorsEdit(EditWithComplete):
         if authors != self.original_val:
             # Only try to commit if changed. This allow setting of other fields
             # to work even if some of the book files are opened in windows.
-            try:
-                self.books_to_refresh |= db.set_authors(id_, authors, notify=False,
-                    allow_case_change=True)
-            except (IOError, OSError) as err:
-                if getattr(err, 'errno', None) == errno.EACCES:  # Permission denied
-                    import traceback
-                    fname = getattr(err, 'filename', None)
-                    p = 'Locked file: %s\n\n'%fname if fname else ''
-                    error_dialog(self, _('Permission denied'),
-                            _('Could not change the on disk location of this'
-                                ' book. Is it open in another program?'),
-                            det_msg=p+force_unicode(traceback.format_exc()), show=True)
-                    return False
-                raise
-        return True
+            self.books_to_refresh |= db.set_authors(id_, authors, notify=False,
+                allow_case_change=True)
 
     @dynamic_property
     def current_val(self):
@@ -364,6 +344,7 @@ class AuthorSortEdit(EnLineEdit):
         copy_as_to_a_action.triggered.connect(self.copy_to_authors)
         a_to_as.triggered.connect(self.author_to_sort)
         as_to_a.triggered.connect(self.sort_to_author)
+        self.original_val = ''
         self.update_state()
 
     @dynamic_property
@@ -437,10 +418,12 @@ class AuthorSortEdit(EnLineEdit):
 
     def initialize(self, db, id_):
         self.current_val = db.author_sort(id_, index_is_id=True)
+        self.original_val = self.current_val
 
     def commit(self, db, id_):
         aus = self.current_val
-        db.set_author_sort(id_, aus, notify=False, commit=False)
+        if aus != self.original_val or self.authors_edit.original_val != self.authors_edit.current_val:
+            db.set_author_sort(id_, aus, notify=False, commit=False)
         return True
 
     def break_cycles(self):
@@ -511,13 +494,13 @@ class SeriesEdit(EditWithComplete):
             if i[0] == series_id:
                 inval = i[1]
                 break
-        self.original_val = self.current_val = inval
+        self.current_val = inval
+        self.original_val = self.current_val
 
     def commit(self, db, id_):
         series = self.current_val
-        self.books_to_refresh |= db.set_series(id_, series, notify=False,
-                                            commit=True, allow_case_change=True)
-        return True
+        if series != self.original_val:
+            self.books_to_refresh |= db.set_series(id_, series, notify=False, commit=True, allow_case_change=True)
 
     def break_cycles(self):
         self.dialog = None
@@ -567,8 +550,8 @@ class SeriesIndexEdit(QDoubleSpinBox):
         self.original_series_name = self.series_edit.original_val
 
     def commit(self, db, id_):
-        db.set_series_index(id_, self.current_val, notify=False, commit=False)
-        return True
+        if self.series_edit.original_val != self.series_edit.current_val or self.current_val != self.original_val:
+            db.set_series_index(id_, self.current_val, notify=False, commit=False)
 
     def increment(self):
         if tweaks['series_index_auto_increment'] != 'no_change' and self.db is not None:
@@ -743,7 +726,7 @@ class FormatsManager(QWidget):
 
     def commit(self, db, id_):
         if not self.changed:
-            return True
+            return
         old_extensions, new_extensions, paths = set(), set(), {}
         for row in range(self.formats.count()):
             fmt = self.formats.item(row)
@@ -771,7 +754,7 @@ class FormatsManager(QWidget):
                 db.remove_format(id_, ext, notify=False, index_is_id=True)
 
         self.changed = False
-        return True
+        return
 
     def add_format(self, *args):
         files = choose_files(self, 'add formats dialog',
@@ -1087,7 +1070,6 @@ class Cover(ImageView):  # {{{
                 db.set_cover(id_, self.current_val, notify=False, commit=False)
             else:
                 db.remove_cover(id_, notify=False, commit=False)
-        return True
 
     def break_cycles(self):
         try:
@@ -1118,8 +1100,9 @@ class CommentsEdit(Editor):  # {{{
         self.original_val = self.current_val
 
     def commit(self, db, id_):
-        db.set_comment(id_, self.current_val, notify=False, commit=False)
-        return True
+        val = self.current_val
+        if val != self.original_val:
+            db.set_comment(id_, self.current_val, notify=False, commit=False)
 # }}}
 
 class RatingEdit(QSpinBox):  # {{{
@@ -1251,21 +1234,20 @@ class LanguagesEdit(LE):  # {{{
         langs = db.languages(id_, index_is_id=True)
         if langs:
             lc = [x.strip() for x in langs.split(',')]
-        self.current_val = self.original_val = lc
+        self.current_val = lc
+        self.original_val = self.current_val
 
-    def commit(self, db, id_):
+    def validate_for_commit(self):
         bad = self.validate()
         if bad:
-            error_dialog(self, _('Unknown language'),
-                    ngettext('The language %s is not recognized',
-                        'The languages %s are not recognized', len(bad))%(
-                            ', '.join(bad)),
-                    show=True)
-            return False
+            msg = ngettext('The language %s is not recognized', 'The languages %s are not recognized', len(bad)) % (', '.join(bad))
+            return _('Unknown language'), msg, ''
+        return None, None, None
+
+    def commit(self, db, id_):
         cv = self.current_val
         if cv != self.original_val:
             db.set_languages(id_, cv)
-        return True
 # }}}
 
 class IdentifiersEdit(QLineEdit):  # {{{
@@ -1324,7 +1306,6 @@ class IdentifiersEdit(QLineEdit):  # {{{
     def commit(self, db, id_):
         if self.original_val != self.current_val:
             db.set_identifiers(id_, self.current_val, notify=False, commit=False)
-        return True
 
     def validate(self, *args):
         identifiers = self.current_val

src/calibre/gui2/metadata/single.py

@@ -299,8 +299,7 @@ class MetadataSingleDialogBase(ResizableDialog):
             title = title[:50] + u'\u2026'
         self.setWindowTitle(BASE_TITLE + ' - ' +
                 title + ' - ' +
-                _(' [%(num)d of %(tot)d]')%dict(num=
-                    self.current_row+1,
+                _(' [%(num)d of %(tot)d]')%dict(num=self.current_row+1,
                 tot=len(self.row_list)))
 
     def swap_title_author(self, *args):
@@ -472,10 +471,13 @@ class MetadataSingleDialogBase(ResizableDialog):
             return True
         for widget in self.basic_metadata_widgets:
             try:
-                if not widget.commit(self.db, self.book_id):
-                    return False
-                self.books_to_refresh |= getattr(widget, 'books_to_refresh',
-                        set([]))
+                if hasattr(widget, 'validate_for_commit'):
+                    title, msg, det_msg = widget.validate_for_commit()
+                    if title is not None:
+                        error_dialog(self, title, msg, det_msg=det_msg, show=True)
+                        return False
+                widget.commit(self.db, self.book_id)
+                self.books_to_refresh |= getattr(widget, 'books_to_refresh', set())
             except (IOError, OSError) as err:
                 if getattr(err, 'errno', None) == errno.EACCES:  # Permission denied
                     import traceback
@@ -1036,8 +1038,8 @@ def edit_metadata(db, row_list, current_row, parent=None, view_slot=None,
 if __name__ == '__main__':
     from calibre.gui2 import Application as QApplication
     app = QApplication([])
-    from calibre.library import db as db_
-    db = db_()
+    from calibre.library import db
+    db = db()
     row_list = list(range(len(db.data)))
     edit_metadata(db, row_list, 0)