From 8fe4db704090ab5fe18915bdaf6620409751edbb Mon Sep 17 00:00:00 2001 From: Charles Haley Date: Tue, 25 Feb 2014 14:41:53 +0100 Subject: [PATCH 1/4] Infrastructure for allowing a device to "synchronize" data with calibre's database. The exact meaning of "synchronize" is left to the device. This implementation adds almost zero overhead (nanoseconds per book) if the device driver does not do any synchronization. During book matching in gui2.device, whenever a book on the device is matched to a book in the DB, the (new) device method synchronize_with_db is called. It can copy information from the device to calibre's db or vice versa. If it returns True then the book's metadata is updated from calibre's DB even if it otherwise would not be. This permits the synchronization process to request that data in calibre be unconditionally sent to the device. Note that the synchronize_with_db method is called on the GUI thread. It is the device driver's responsibility to be threadsafe with respect to the device manager thread. A lightly-tested implementation in the wireless driver is included to show how the new method might be used. It implements syncing a yes/no "is read" column and a "read date" column from the device to calibre. --- src/calibre/devices/interface.py | 17 ++++++ .../devices/smart_device_app/driver.py | 57 ++++++++++++++++++- src/calibre/gui2/device.py | 12 ++-- 3 files changed, 79 insertions(+), 7 deletions(-) diff --git a/src/calibre/devices/interface.py b/src/calibre/devices/interface.py index b3ac714344..81c7e127c5 100644 --- a/src/calibre/devices/interface.py +++ b/src/calibre/devices/interface.py @@ -718,6 +718,23 @@ class DevicePlugin(Plugin): ''' return False + def synchronize_with_db(self, db, book_id, book_metadata): + ''' + Called during book matching when a book on the device is matched with + a book in calibre's db. The method is responsible for syncronizing + data from the device to calibre's db (if needed). + + The method must return True if either calibre's database or the device + book's metadata were changed, False otherwise. + + Extremely important: this method is called on the GUI thread. It must + be threadsafe with respect to the device manager's thread. + + book_id: the calibre id for the book in the database. + book_metadata: the Metadata object for the book coming from the device. + ''' + return False + class BookList(list): ''' A list of books. Each Book object must have the fields diff --git a/src/calibre/devices/smart_device_app/driver.py b/src/calibre/devices/smart_device_app/driver.py index 2a56396b7c..e0c3e637c9 100644 --- a/src/calibre/devices/smart_device_app/driver.py +++ b/src/calibre/devices/smart_device_app/driver.py @@ -1080,6 +1080,12 @@ class SMART_DEVICE_APP(DeviceConfig, DevicePlugin): elif hasattr(self, 'THUMBNAIL_WIDTH'): delattr(self, 'THUMBNAIL_WIDTH') + self.is_read_sync_col = result.get('isReadSyncCol', None) + self._debug('Device is_read sync col', self.is_read_sync_col) + + self.is_read_date_sync_col = result.get('isReadDateSyncCol', False) + self._debug('Device is_read_date sync col', self.is_read_date_sync_col) + if password: returned_hash = result.get('passwordHash', None) if result.get('passwordHash', None) is None: @@ -1196,7 +1202,8 @@ class SMART_DEVICE_APP(DeviceConfig, DevicePlugin): opcode, result = self._call_client('GET_BOOK_COUNT', {'canStream':True, 'canScan':True, - 'willUseCachedMetadata': self.client_can_use_metadata_cache}) + 'willUseCachedMetadata': self.client_can_use_metadata_cache, + 'supportsSync': True}) bl = CollectionsBookList(None, self.PREFIX, self.settings) if opcode == 'OK': count = result['count'] @@ -1219,6 +1226,9 @@ class SMART_DEVICE_APP(DeviceConfig, DevicePlugin): r['last_modified']) if book: bl.add_book(book, replace_metadata=True) + book.set('_is_read_', r.get('_is_read_', None)) + book.set('_is_read_changed_', r.get('_is_read_changed_', None)) + book.set('_last_read_date_', r.get('_last_read_date_', None)) else: books_to_send.append(r['priKey']) @@ -1239,6 +1249,9 @@ class SMART_DEVICE_APP(DeviceConfig, DevicePlugin): if '_series_sort_' in result: del result['_series_sort_'] book = self.json_codec.raw_to_book(result, SDBook, self.PREFIX) + book.set('_is_read_', result.get('_is_read_', None)) + book.set('_is_read_changed_', result.get('_is_read_changed_', None)) + book.set('_last_read_date_', r.get('_last_read_date_', None)) bl.add_book(book, replace_metadata=True) if '_new_book_' in result: book.set('_new_book_', True) @@ -1288,7 +1301,8 @@ class SMART_DEVICE_APP(DeviceConfig, DevicePlugin): count = len(books_to_send) self._call_client('SEND_BOOKLISTS', {'count': count, 'collections': coldict, - 'willStreamMetadata': True}, + 'willStreamMetadata': True, + 'supportsSync': True}, wait_for_response=False) if count: @@ -1297,7 +1311,7 @@ class SMART_DEVICE_APP(DeviceConfig, DevicePlugin): self._set_known_metadata(book) opcode, result = self._call_client( 'SEND_BOOK_METADATA', - {'index': i, 'count': count, 'data': book}, + {'index': i, 'count': count, 'data': book, 'supportsSync': True}, print_debug_info=False, wait_for_response=False) @@ -1443,6 +1457,41 @@ class SMART_DEVICE_APP(DeviceConfig, DevicePlugin): def specialize_global_preferences(self, device_prefs): device_prefs.set_overrides(manage_device_metadata='on_connect') + @synchronous('sync_lock') + def synchronize_with_db(self, db, id_, book): + is_changed = book.get('_is_read_changed_', None); + if is_changed: + made_changes = False + # is_read_changed == 1: standard sync. Update calibre with the new value + # is_read_changed == 2: special first-time sync after specifying the + # column. Update calibre if CC's value is not null, else update CC + # if calibre's value is not null + val = book.get('_is_read_', None) + if is_changed == 1 or (is_changed == 2 and val): + self._debug('standard update book', book.get('title', 'huh?'), 'to', val) + if self.is_read_sync_col: + db.new_api.set_field(self.is_read_sync_col, {id_: val}) + made_changes = True + if self.is_read_date_sync_col: + db.new_api.set_field(self.is_read_date_sync_col, + {id_: book.get('_last_read_date_', None)}) + made_changes = True + elif self.is_read_sync_col and is_changed == 2 and not val: + calibre_val = db.new_api.field_for(self.is_read_sync_col, + id_, default_value=None) + if calibre_val: + from calibre.utils.date import UNDEFINED_DATE + # This will force the metadata for the book to be sent even + # if the last_mod dates matched before. Note that because + # CC's last_read date is one-way sync, this could leave an + # empty date in CC. + self._debug('special update book', book.get('title', 'huh?'), + 'to', calibre_val) + book.set('last_modified', UNDEFINED_DATE) + book.set('_is_read_changed_', None) + return made_changes + return False + @synchronous('sync_lock') def startup(self): self.listen_socket = None @@ -1466,6 +1515,8 @@ class SMART_DEVICE_APP(DeviceConfig, DevicePlugin): self.noop_counter = 0 self.connection_attempts = {} self.client_wants_uuid_file_names = False + self.is_read_sync_col = None + self.is_read_date_sync_col = None message = None compression_quality_ok = True diff --git a/src/calibre/gui2/device.py b/src/calibre/gui2/device.py index 8fac7ed6a3..8dcf03dfec 100644 --- a/src/calibre/gui2/device.py +++ b/src/calibre/gui2/device.py @@ -1769,10 +1769,14 @@ class DeviceMixin(object): # {{{ def updateq(id_, book): try: return (update_metadata and - (db.metadata_last_modified(id_, index_is_id=True) != - getattr(book, 'last_modified', None) or - (isinstance(getattr(book, 'thumbnail', None), (list, tuple)) - and max(book.thumbnail[0], book.thumbnail[1]) != desired_thumbnail_height + ( + (self.device_manager.device is not None and + self.device_manager.device.synchronize_with_db(db, id_, book)) or + (db.metadata_last_modified(id_, index_is_id=True) != + getattr(book, 'last_modified', None) or + (isinstance(getattr(book, 'thumbnail', None), (list, tuple)) + and max(book.thumbnail[0], book.thumbnail[1]) != desired_thumbnail_height + ) ) ) ) From deea793c21aa75cde15bf2ca26c1dae569a94ede Mon Sep 17 00:00:00 2001 From: Charles Haley Date: Wed, 26 Feb 2014 11:30:22 +0100 Subject: [PATCH 2/4] 1) Change the new sync method to return a list of changed book ids instead of a simple boolean. 2) Make the wireless device driver synchronization more robust in the face of multiple libraries where the sync column might not always exist. --- src/calibre/devices/interface.py | 10 ++- .../devices/smart_device_app/driver.py | 90 ++++++++++++------- src/calibre/gui2/device.py | 48 ++++++---- 3 files changed, 97 insertions(+), 51 deletions(-) diff --git a/src/calibre/devices/interface.py b/src/calibre/devices/interface.py index 81c7e127c5..954f19560f 100644 --- a/src/calibre/devices/interface.py +++ b/src/calibre/devices/interface.py @@ -724,8 +724,12 @@ class DevicePlugin(Plugin): a book in calibre's db. The method is responsible for syncronizing data from the device to calibre's db (if needed). - The method must return True if either calibre's database or the device - book's metadata were changed, False otherwise. + The method must return a set of calibre book ids changed if calibre's + database was changed, None if the database was not changed. If the + method returns an empty set then the metadata for the book on the + device is updated with calibre's metadata and given back to the device, + but no GUI refresh of that book is done. This is useful when the calire + data is correct but must be sent to the device. Extremely important: this method is called on the GUI thread. It must be threadsafe with respect to the device manager's thread. @@ -733,7 +737,7 @@ class DevicePlugin(Plugin): book_id: the calibre id for the book in the database. book_metadata: the Metadata object for the book coming from the device. ''' - return False + return None class BookList(list): ''' diff --git a/src/calibre/devices/smart_device_app/driver.py b/src/calibre/devices/smart_device_app/driver.py index e0c3e637c9..8d26f87654 100644 --- a/src/calibre/devices/smart_device_app/driver.py +++ b/src/calibre/devices/smart_device_app/driver.py @@ -1295,7 +1295,8 @@ class SMART_DEVICE_APP(DeviceConfig, DevicePlugin): # given back by "books", and one that has been plugboarded. books_to_send = [] for book in booklists[0]: - if not self._metadata_already_on_device(book): + if (book.get('_force_send_metadata_', None) or + not self._metadata_already_on_device(book)): books_to_send.append(book) count = len(books_to_send) @@ -1459,38 +1460,63 @@ class SMART_DEVICE_APP(DeviceConfig, DevicePlugin): @synchronous('sync_lock') def synchronize_with_db(self, db, id_, book): + if not (self.is_read_sync_col or self.is_read_date_sync_col): + # Not syncing + return None + is_changed = book.get('_is_read_changed_', None); - if is_changed: - made_changes = False - # is_read_changed == 1: standard sync. Update calibre with the new value - # is_read_changed == 2: special first-time sync after specifying the - # column. Update calibre if CC's value is not null, else update CC - # if calibre's value is not null - val = book.get('_is_read_', None) - if is_changed == 1 or (is_changed == 2 and val): - self._debug('standard update book', book.get('title', 'huh?'), 'to', val) - if self.is_read_sync_col: - db.new_api.set_field(self.is_read_sync_col, {id_: val}) - made_changes = True - if self.is_read_date_sync_col: - db.new_api.set_field(self.is_read_date_sync_col, - {id_: book.get('_last_read_date_', None)}) - made_changes = True - elif self.is_read_sync_col and is_changed == 2 and not val: - calibre_val = db.new_api.field_for(self.is_read_sync_col, - id_, default_value=None) - if calibre_val: - from calibre.utils.date import UNDEFINED_DATE - # This will force the metadata for the book to be sent even - # if the last_mod dates matched before. Note that because - # CC's last_read date is one-way sync, this could leave an - # empty date in CC. - self._debug('special update book', book.get('title', 'huh?'), - 'to', calibre_val) - book.set('last_modified', UNDEFINED_DATE) - book.set('_is_read_changed_', None) - return made_changes - return False + is_read = book.get('_is_read_', None) + + if is_changed == 2 and is_read is None: + # This is a special case where the user just set the sync column. In + # this case the device value wins if it is not None by falling + # through to the normal sync situation below, otherwise the calibre + # value wins. + calibre_val = db.new_api.field_for(self.is_read_sync_col, + id_, default_value=None) + if calibre_val is not None: + # This will force the metadata for the book to be sent . Note + # that because the devices last_read date is one-way sync, this + # could leave an empty date in the device. + book.set('_force_send_metadata_', True) + self._debug('special update book', book.get('title', 'huh?'), + 'to', calibre_val) + return set(id_) + # Both values are None. Do nothing + return None + + orig_is_read = book.get(self.is_read_sync_col, None) + if is_read != orig_is_read: + # The value in the device's is_read checkbox is not the same as the + # last one that came to the device from calibre during the last + # connect, meaning that the user changed it. Write the one from the + # checkbox to calibre's db. + changed_books = set() + is_read_date = book.get('_last_read_date_', None); + self._debug('standard update book', book.get('title', 'huh?'), 'to', + is_read, is_read_date) + if self.is_read_sync_col: + try: + changed_books = db.new_api.set_field(self.is_read_sync_col, + {id_: is_read}) + except: + self._debug('setting read sync col tossed exception', + self.is_read_sync_col) + if self.is_read_date_sync_col: + try: + changed_books |= db.new_api.set_field(self.is_read_date_sync_col, + {id_: is_read_date}) + except: + self._debug('setting read date sync col tossed exception', + self.is_read_date_sync_col) + return changed_books + + # The user might have changed the value in calibre. If so, that value + # will be sent to the device in the normal way. Note that because any + # updated value has already been synced and so will also be sent, the + # device should put the calibre value into its checkbox (or whatever it + # uses) + return None @synchronous('sync_lock') def startup(self): diff --git a/src/calibre/gui2/device.py b/src/calibre/gui2/device.py index 8dcf03dfec..843ccffaa5 100644 --- a/src/calibre/gui2/device.py +++ b/src/calibre/gui2/device.py @@ -1751,12 +1751,9 @@ class DeviceMixin(object): # {{{ self.db_book_title_cache = db_book_title_cache self.db_book_uuid_cache = db_book_uuid_cache - # Now iterate through all the books on the device, setting the - # in_library field. If the UUID matches a book in the library, then - # do not consider that book for other matching. In all cases set - # the application_id to the db_id of the matching book. This value - # will be used by books_on_device to indicate matches. While we are - # going by, update the metadata for a book if automatic management is on + # This is really a local variable, but it must be made into a class + # attribute so that the updateq function (below) can change it. + self.sbil_book_ids_to_refresh = set() def update_book(id_, book) : if not update_metadata: @@ -1768,21 +1765,31 @@ class DeviceMixin(object): # {{{ def updateq(id_, book): try: - return (update_metadata and - ( - (self.device_manager.device is not None and - self.device_manager.device.synchronize_with_db(db, id_, book)) or - (db.metadata_last_modified(id_, index_is_id=True) != - getattr(book, 'last_modified', None) or - (isinstance(getattr(book, 'thumbnail', None), (list, tuple)) - and max(book.thumbnail[0], book.thumbnail[1]) != desired_thumbnail_height - ) - ) + if not update_metadata: + return False + + if self.device_manager.device is not None: + set_of_ids = self.device_manager.device.synchronize_with_db(db, id_, book) + if set_of_ids is not None: + self.sbil_book_ids_to_refresh |= set_of_ids + return True + + return (db.metadata_last_modified(id_, index_is_id=True) != + getattr(book, 'last_modified', None) or + (isinstance(getattr(book, 'thumbnail', None), (list, tuple)) + and max(book.thumbnail[0], book.thumbnail[1]) != desired_thumbnail_height ) ) except: return True + # Now iterate through all the books on the device, setting the + # in_library field. If the UUID matches a book in the library, then + # do not consider that book for other matching. In all cases set + # the application_id to the db_id of the matching book. This value + # will be used by books_on_device to indicate matches. While we are + # going by, update the metadata for a book if automatic management is on + total_book_count = 0 for booklist in booklists: for book in booklist: @@ -1877,6 +1884,15 @@ class DeviceMixin(object): # {{{ FunctionDispatcher(self.metadata_synced), booklists, plugboards, add_as_step_to_job) + if self.sbil_book_ids_to_refresh: + try: + prints('DeviceJob: set_books_in_library refreshing GUI for ', + len(self.sbil_book_ids_to_refresh), 'books') + self.library_view.model().refresh_ids(self.sbil_book_ids_to_refresh) + except: + # This shouldn't ever happen, but just in case ... + pass + if DEBUG: prints('DeviceJob: set_books_in_library finished: time=', time.time() - start_time) From cab3cba81379de0f8bf9d26dc3b6e00187f48f9b Mon Sep 17 00:00:00 2001 From: Charles Haley Date: Wed, 26 Feb 2014 12:11:21 +0100 Subject: [PATCH 3/4] Pass the current row to refresh_ids so that the book_data signal is emitted. --- src/calibre/gui2/device.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/calibre/gui2/device.py b/src/calibre/gui2/device.py index 843ccffaa5..f53e9f1d65 100644 --- a/src/calibre/gui2/device.py +++ b/src/calibre/gui2/device.py @@ -1888,10 +1888,11 @@ class DeviceMixin(object): # {{{ try: prints('DeviceJob: set_books_in_library refreshing GUI for ', len(self.sbil_book_ids_to_refresh), 'books') - self.library_view.model().refresh_ids(self.sbil_book_ids_to_refresh) + self.library_view.model().refresh_ids(self.sbil_book_ids_to_refresh, + current_row=self.library_view.currentIndex().row()) except: # This shouldn't ever happen, but just in case ... - pass + traceback.print_exc() if DEBUG: prints('DeviceJob: set_books_in_library finished: time=', From 8ede2d466677e4bf5b32a26707aac9bca8090cdb Mon Sep 17 00:00:00 2001 From: Charles Haley Date: Wed, 26 Feb 2014 12:36:46 +0100 Subject: [PATCH 4/4] Make the set of books to refresh into a local variable buy using it as an object instance instead of by assignment. --- src/calibre/gui2/device.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/calibre/gui2/device.py b/src/calibre/gui2/device.py index f53e9f1d65..3f3f8daf9c 100644 --- a/src/calibre/gui2/device.py +++ b/src/calibre/gui2/device.py @@ -1751,9 +1751,7 @@ class DeviceMixin(object): # {{{ self.db_book_title_cache = db_book_title_cache self.db_book_uuid_cache = db_book_uuid_cache - # This is really a local variable, but it must be made into a class - # attribute so that the updateq function (below) can change it. - self.sbil_book_ids_to_refresh = set() + book_ids_to_refresh = set() def update_book(id_, book) : if not update_metadata: @@ -1771,7 +1769,7 @@ class DeviceMixin(object): # {{{ if self.device_manager.device is not None: set_of_ids = self.device_manager.device.synchronize_with_db(db, id_, book) if set_of_ids is not None: - self.sbil_book_ids_to_refresh |= set_of_ids + book_ids_to_refresh.update(set_of_ids) return True return (db.metadata_last_modified(id_, index_is_id=True) != @@ -1884,11 +1882,11 @@ class DeviceMixin(object): # {{{ FunctionDispatcher(self.metadata_synced), booklists, plugboards, add_as_step_to_job) - if self.sbil_book_ids_to_refresh: + if book_ids_to_refresh: try: prints('DeviceJob: set_books_in_library refreshing GUI for ', - len(self.sbil_book_ids_to_refresh), 'books') - self.library_view.model().refresh_ids(self.sbil_book_ids_to_refresh, + len(book_ids_to_refresh), 'books') + self.library_view.model().refresh_ids(book_ids_to_refresh, current_row=self.library_view.currentIndex().row()) except: # This shouldn't ever happen, but just in case ...