From f51d0a1c312b8c2312474ec5827b1802b097671c Mon Sep 17 00:00:00 2001 From: Charles Haley <> Date: Thu, 9 Sep 2010 21:56:36 +0100 Subject: [PATCH 1/5] Remove print in gui.devices when books match but db_ids don't. --- src/calibre/gui2/device.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/calibre/gui2/device.py b/src/calibre/gui2/device.py index 98e6a5d9da..65a4b62cf2 100644 --- a/src/calibre/gui2/device.py +++ b/src/calibre/gui2/device.py @@ -1365,13 +1365,10 @@ class DeviceMixin(object): # {{{ if mi.authors and \ re.sub('(?u)\W|[_]', '', authors_to_string(mi.authors).lower()) \ in cache['authors']: - # We really shouldn't get here, because set_books_in_library - # should have set the db_ids for the books, and therefore - # the if just above should have found them. Mark the book - # anyway, and print a message about the situation + # If we get here, then two library books have the same title + # and author. This can happen, especially in the case of + # news. Mark a match and go on. loc[i] = True - prints('book_on_device: matched title/author but not db_id!', - mi.title, authors_to_string(mi.authors)) continue # Also check author sort, because it can be used as author in # some formats From 947fa2769df049e859284c958fd0177c34885c3e Mon Sep 17 00:00:00 2001 From: Charles Haley <> Date: Fri, 10 Sep 2010 09:58:39 +0100 Subject: [PATCH 2/5] Fix matching books with no --- src/calibre/gui2/device.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/calibre/gui2/device.py b/src/calibre/gui2/device.py index e002fe916b..3cab6ab21d 100644 --- a/src/calibre/gui2/device.py +++ b/src/calibre/gui2/device.py @@ -1357,6 +1357,14 @@ class DeviceMixin(object): # {{{ c = self.book_db_id_counts.get(db_id, 0) self.book_db_id_counts[db_id] = c + 1 uuid = getattr(book, 'uuid', None) + if uuid is None and db_id is not None: + # Catch the case where a book on the device has no UUID + # but was matched against some book in the library. + try: + uuid = self.library_view.model().db.uuid(db_id, + index_is_id=True) + except: + pass if uuid is not None: self.book_db_uuid_cache[i].add(uuid) self.book_db_uuid_path_map[uuid] = book.path From 98bc0e53ec16b37a34f8d54dbf2350251c1ceef4 Mon Sep 17 00:00:00 2001 From: Charles Haley <> Date: Fri, 10 Sep 2010 11:33:17 +0100 Subject: [PATCH 3/5] Fix several ondevice display problems, such as news overwriting books with the flag not changing. Also repair/add some explanatory comments. --- src/calibre/gui2/device.py | 46 +++++++++++++++--------------- src/calibre/gui2/library/models.py | 2 +- 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/src/calibre/gui2/device.py b/src/calibre/gui2/device.py index 3cab6ab21d..4a2789171a 100644 --- a/src/calibre/gui2/device.py +++ b/src/calibre/gui2/device.py @@ -791,12 +791,13 @@ class DeviceMixin(object): # {{{ self.booklists()) model.paths_deleted(paths) self.upload_booklists() - # Clear the ondevice info so it will be recomputed + # Force recomputation the library's ondevice info. We don't need to call + # set_books_in_library, because no books were added to the device and + # the deleted book is no longer displayed. self.book_on_device(None, None, reset=True) - # We want to reset all the ondevice flags in the library. Use a big - # hammer, so we don't need to worry about whether some succeeded or not - self.library_view.model().refresh() - + # We need to reset the ondevice flags in the library. Use a big hammer, + # so we don't need to worry about whether some succeeded or not. + self.refresh_ondevice_info(device_connected=True, reset_only=False) def dispatch_sync_event(self, dest, delete, specific): rows = self.library_view.selectionModel().selectedRows() @@ -1286,8 +1287,14 @@ class DeviceMixin(object): # {{{ books_to_be_deleted = memory[1] self.library_view.model().delete_books_by_id(books_to_be_deleted) - self.set_books_in_library(self.booklists(), - reset=bool(books_to_be_deleted)) + # There are some cases where sending a book to the device overwrites a + # book already there with a different book. This happens frequently in + # news. When this happens, the book match indication will be wrong + # because the UUID changed. Force both the device and the library view + # to refresh the flags. + self.set_books_in_library(self.booklists(), reset=True) + self.book_on_device(None, reset=True) + self.refresh_ondevice_info(device_connected = True) view = self.card_a_view if on_card == 'carda' else self.card_b_view if on_card == 'cardb' else self.memory_view view.model().resort(reset=False) @@ -1295,16 +1302,6 @@ class DeviceMixin(object): # {{{ for f in files: getattr(f, 'close', lambda : True)() - self.book_on_device(None, reset=True) - if metadata: - changed = set([]) - for mi in metadata: - id_ = getattr(mi, 'application_id', None) - if id_ is not None: - changed.add(id_) - if changed: - self.library_view.model().refresh_ids(list(changed)) - def book_on_device(self, id, format=None, reset=False): ''' Return an indication of whether the given book represented by its db id @@ -1396,7 +1393,7 @@ class DeviceMixin(object): # {{{ # We really shouldn't get here, because set_books_in_library # should have set the db_ids for the books, and therefore # the if just above should have found them. Mark the book - # anyway, and print a message about the situation + # anyway, just in case. loc[i] = True loc[4] = 'metadata' loc[5] = cache['paths'] @@ -1434,6 +1431,10 @@ class DeviceMixin(object): # {{{ if title not in self.db_book_title_cache: self.db_book_title_cache[title] = \ {'authors':{}, 'author_sort':{}, 'db_ids':{}} + # If there are multiple books in the library with the same title + # and author, then remember the last one. That is OK, because as + # we can't tell the difference between the books, one is as good + # as another. if mi.authors: authors = clean_string(authors_to_string(mi.authors)) self.db_book_title_cache[title]['authors'][authors] = mi @@ -1446,10 +1447,9 @@ class DeviceMixin(object): # {{{ # Now iterate through all the books on the device, setting the # in_library field. Fastest and most accurate key is the uuid. Second is # the application_id, which is really the db key, but as this can - # accidentally match across libraries we also verify the title. The - # db_id exists on Sony devices. Fallback is title and author match. - # We set the application ID so that we can reproduce book matching, - # necessary for identifying copies of books. + # accidentally match across libraries we also verify the title. Fallback + # is title and author match. We set the application ID so that we can + # reproduce book matching, necessary for identifying copies of books. update_metadata = prefs['manage_device_metadata'] == 'on_connect' for booklist in booklists: @@ -1470,7 +1470,7 @@ class DeviceMixin(object): # {{{ if d is not None: if getattr(book, 'application_id', None) in d['db_ids']: book.in_library = True - # application already matches db_id, so no need to set it + # application already matches a db_id. No need to set it if update_metadata: book.smart_update(d['db_ids'][book.application_id], replace_metadata=True) diff --git a/src/calibre/gui2/library/models.py b/src/calibre/gui2/library/models.py index ae9487f801..8e7bdc9b72 100644 --- a/src/calibre/gui2/library/models.py +++ b/src/calibre/gui2/library/models.py @@ -121,10 +121,10 @@ class BooksModel(QAbstractTableModel): # {{{ def set_device_connected(self, is_connected): self.device_connected = is_connected self.db.refresh_ondevice() + self.refresh() if is_connected and self.sorted_on[0] == 'ondevice': self.resort() - def set_book_on_device_func(self, func): self.book_on_device = func From 0b6b78713aef43c0084562446541dbcf766ccb18 Mon Sep 17 00:00:00 2001 From: Charles Haley <> Date: Fri, 10 Sep 2010 14:07:11 +0100 Subject: [PATCH 4/5] Last changes for book matching. Make both book on device and book in library respect the exact-match semantics that Kovid implemented. --- src/calibre/gui2/device.py | 56 ++++++++++++++++++++++++------ src/calibre/gui2/library/models.py | 1 + 2 files changed, 46 insertions(+), 11 deletions(-) diff --git a/src/calibre/gui2/device.py b/src/calibre/gui2/device.py index 4a2789171a..3b7d1bb9e5 100644 --- a/src/calibre/gui2/device.py +++ b/src/calibre/gui2/device.py @@ -760,8 +760,8 @@ class DeviceMixin(object): # {{{ def refresh_ondevice_info(self, device_connected, reset_only = False): ''' - Force the library view to refresh, taking into consideration - books information + Force the library view to refresh, taking into consideration new + device books information ''' self.book_on_device(None, reset=True) if reset_only: @@ -791,9 +791,10 @@ class DeviceMixin(object): # {{{ self.booklists()) model.paths_deleted(paths) self.upload_booklists() - # Force recomputation the library's ondevice info. We don't need to call - # set_books_in_library, because no books were added to the device and - # the deleted book is no longer displayed. + # Force recomputation the library's ondevice info. We need to call + # set_books_in_library even though books were not added because + # the deleted book might have been an exact match. + self.set_books_in_library(self.booklists(), reset=True) self.book_on_device(None, None, reset=True) # We need to reset the ondevice flags in the library. Use a big hammer, # so we don't need to worry about whether some succeeded or not. @@ -1366,6 +1367,9 @@ class DeviceMixin(object): # {{{ self.book_db_uuid_cache[i].add(uuid) self.book_db_uuid_path_map[uuid] = book.path if uuid in self.db_book_uuid_cache: + # This book exactly matches a book on the device. + # Set the flag that prevents the book from + # participating in metadata matching self.book_db_title_cache[i][book_title]\ ['uuid_in_library'] = True self.book_db_title_cache[i][book_title]['paths'].add(book.path) @@ -1380,6 +1384,12 @@ class DeviceMixin(object): # {{{ db_title = clean_string(mi.title) cache = self.book_db_title_cache[i].get(db_title, None) if cache and not cache['uuid_in_library']: + # We really shouldn't get here, because set_books_in_library + # should have set the db_ids for the books, the cache-builder + # will add the uuid for those matched books to the cache, and + # therefore the if just above should have found them. Check + # anyway, just in case. Also print that we got here... + prints('checking metadata matches:', mi.title) if id in cache['db_ids']: loc[i] = True loc[4] = 'db_id' @@ -1390,10 +1400,6 @@ class DeviceMixin(object): # {{{ if (mi.authors and clean_string(authors_to_string(mi.authors)) in cache['authors']) or (mi.author_sort and clean_string(mi.author_sort) in cache['authors']): - # We really shouldn't get here, because set_books_in_library - # should have set the db_ids for the books, and therefore - # the if just above should have found them. Mark the book - # anyway, just in case. loc[i] = True loc[4] = 'metadata' loc[5] = cache['paths'] @@ -1454,6 +1460,7 @@ class DeviceMixin(object): # {{{ update_metadata = prefs['manage_device_metadata'] == 'on_connect' for booklist in booklists: for book in booklist: + book.in_library = None if getattr(book, 'uuid', None) in self.db_book_uuid_cache: if update_metadata: book.smart_update(self.db_book_uuid_cache[book.uuid], @@ -1462,19 +1469,38 @@ class DeviceMixin(object): # {{{ # ensure that the correct application_id is set book.application_id = \ self.db_book_uuid_cache[book.uuid].application_id + # We had an exact UUID match. The rules are that once the + # uuid of a book on the device exactly matches the uuid of a + # book in the database, don't do metadata matching for that + # book. This prevents the device-book from matching other + # instances of library-books that happen to have the same + # metadata. Of course, this means that if there are more + # copies of the book on the device, then they must also + # exactly match. + book_title = clean_string(book.title) + if book_title in self.db_book_title_cache: + del self.db_book_title_cache[book_title] continue + for booklist in booklists: + for book in booklist: + if book.in_library: + continue book_title = clean_string(book.title) - book.in_library = None d = self.db_book_title_cache.get(book_title, None) if d is not None: + # At this point we know that the title matches. The book + # will match if any of the db_id, author, or author_sort + # also match. if getattr(book, 'application_id', None) in d['db_ids']: book.in_library = True - # application already matches a db_id. No need to set it + # app_id already matches a db_id. No need to set it. if update_metadata: book.smart_update(d['db_ids'][book.application_id], replace_metadata=True) continue + # Sonys know their db_id independent of the application_id + # in the metadata cache. Check that as well. if book.db_id in d['db_ids']: book.in_library = True book.application_id = \ @@ -1483,6 +1509,11 @@ class DeviceMixin(object): # {{{ book.smart_update(d['db_ids'][book.db_id], replace_metadata=True) continue + # We now know that the application_id is not right. Set it + # to None to prevent book_on_device from accidentally + # matching on it. It will be set to a correct value below if + # the book is matched with one in the library + book.application_id = None if book.authors: # Compare against both author and author sort, because # either can appear as the author @@ -1501,6 +1532,9 @@ class DeviceMixin(object): # {{{ if update_metadata: book.smart_update(d['author_sort'][book_authors], replace_metadata=True) + else: + # Book definitely not matched. Clear its application ID + book.application_id = None # Set author_sort if it isn't already asort = getattr(book, 'author_sort', None) if not asort and book.authors: diff --git a/src/calibre/gui2/library/models.py b/src/calibre/gui2/library/models.py index 8e7bdc9b72..bb47508531 100644 --- a/src/calibre/gui2/library/models.py +++ b/src/calibre/gui2/library/models.py @@ -122,6 +122,7 @@ class BooksModel(QAbstractTableModel): # {{{ self.device_connected = is_connected self.db.refresh_ondevice() self.refresh() + self.research() if is_connected and self.sorted_on[0] == 'ondevice': self.resort() From b71c37f000c81fd748f4f111873bcbf523002c7d Mon Sep 17 00:00:00 2001 From: Charles Haley <> Date: Fri, 10 Sep 2010 15:13:12 +0100 Subject: [PATCH 5/5] Simplify code, and make non-UUID matches sane. --- src/calibre/gui2/device.py | 101 +++++++------------------------------ 1 file changed, 17 insertions(+), 84 deletions(-) diff --git a/src/calibre/gui2/device.py b/src/calibre/gui2/device.py index 3b7d1bb9e5..41f5143190 100644 --- a/src/calibre/gui2/device.py +++ b/src/calibre/gui2/device.py @@ -1317,8 +1317,7 @@ class DeviceMixin(object): # {{{ loc = [None, None, None, 0, None, set([])] if reset: - self.book_db_title_cache = None - self.book_db_uuid_cache = None + self.book_db_id_cache = None self.book_db_id_counts = None self.book_db_uuid_path_map = None return @@ -1328,83 +1327,33 @@ class DeviceMixin(object): # {{{ x = x.lower() if x else '' return string_pat.sub('', x) - if self.book_db_title_cache is None: - self.book_db_title_cache = [] - self.book_db_uuid_cache = [] - self.book_db_uuid_path_map = {} + if self.book_db_id_cache is None: + self.book_db_id_cache = [] self.book_db_id_counts = {} + self.book_db_uuid_path_map = {} for i, l in enumerate(self.booklists()): - self.book_db_title_cache.append({}) - self.book_db_uuid_cache.append(set()) + self.book_db_id_cache.append(set()) for book in l: - book_title = clean_string(book.title) - if book_title not in self.book_db_title_cache[i]: - self.book_db_title_cache[i][book_title] = \ - {'authors':set(), 'db_ids':set(), - 'uuids':set(), 'paths':set(), - 'uuid_in_library':False} - book_authors = clean_string(authors_to_string(book.authors)) - self.book_db_title_cache[i][book_title]['authors'].add(book_authors) db_id = getattr(book, 'application_id', None) if db_id is None: db_id = book.db_id if db_id is not None: - self.book_db_title_cache[i][book_title]['db_ids'].add(db_id) # increment the count of books on the device with this # db_id. + self.book_db_id_cache[i].add(db_id) + if db_id not in self.book_db_uuid_path_map: + self.book_db_uuid_path_map[db_id] = set() + self.book_db_uuid_path_map[db_id].add(book.lpath) c = self.book_db_id_counts.get(db_id, 0) self.book_db_id_counts[db_id] = c + 1 - uuid = getattr(book, 'uuid', None) - if uuid is None and db_id is not None: - # Catch the case where a book on the device has no UUID - # but was matched against some book in the library. - try: - uuid = self.library_view.model().db.uuid(db_id, - index_is_id=True) - except: - pass - if uuid is not None: - self.book_db_uuid_cache[i].add(uuid) - self.book_db_uuid_path_map[uuid] = book.path - if uuid in self.db_book_uuid_cache: - # This book exactly matches a book on the device. - # Set the flag that prevents the book from - # participating in metadata matching - self.book_db_title_cache[i][book_title]\ - ['uuid_in_library'] = True - self.book_db_title_cache[i][book_title]['paths'].add(book.path) - mi = self.library_view.model().db.get_metadata(id, index_is_id=True) for i, l in enumerate(self.booklists()): - if mi.uuid in self.book_db_uuid_cache[i]: + if id in self.book_db_id_cache[i]: loc[i] = True + loc[3] = self.book_db_id_counts.get(id, 0) loc[4] = 'uuid' - loc[5].add(self.book_db_uuid_path_map[mi.uuid]) + loc[5] |= self.book_db_uuid_path_map[id] continue - db_title = clean_string(mi.title) - cache = self.book_db_title_cache[i].get(db_title, None) - if cache and not cache['uuid_in_library']: - # We really shouldn't get here, because set_books_in_library - # should have set the db_ids for the books, the cache-builder - # will add the uuid for those matched books to the cache, and - # therefore the if just above should have found them. Check - # anyway, just in case. Also print that we got here... - prints('checking metadata matches:', mi.title) - if id in cache['db_ids']: - loc[i] = True - loc[4] = 'db_id' - loc[5] = cache['paths'] - continue - # Also check author sort, because it can be used as author in - # some formats - if (mi.authors and clean_string(authors_to_string(mi.authors)) - in cache['authors']) or (mi.author_sort and - clean_string(mi.author_sort) in cache['authors']): - loc[i] = True - loc[4] = 'metadata' - loc[5] = cache['paths'] - continue - loc[3] = self.book_db_id_counts.get(id, 0) return loc def set_books_in_library(self, booklists, reset=False): @@ -1451,11 +1400,10 @@ class DeviceMixin(object): # {{{ self.db_book_uuid_cache[mi.uuid] = mi # Now iterate through all the books on the device, setting the - # in_library field. Fastest and most accurate key is the uuid. Second is - # the application_id, which is really the db key, but as this can - # accidentally match across libraries we also verify the title. Fallback - # is title and author match. We set the application ID so that we can - # reproduce book matching, necessary for identifying copies of books. + # 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. update_metadata = prefs['manage_device_metadata'] == 'on_connect' for booklist in booklists: @@ -1469,23 +1417,8 @@ class DeviceMixin(object): # {{{ # ensure that the correct application_id is set book.application_id = \ self.db_book_uuid_cache[book.uuid].application_id - # We had an exact UUID match. The rules are that once the - # uuid of a book on the device exactly matches the uuid of a - # book in the database, don't do metadata matching for that - # book. This prevents the device-book from matching other - # instances of library-books that happen to have the same - # metadata. Of course, this means that if there are more - # copies of the book on the device, then they must also - # exactly match. - book_title = clean_string(book.title) - if book_title in self.db_book_title_cache: - del self.db_book_title_cache[book_title] - continue - - for booklist in booklists: - for book in booklist: - if book.in_library: continue + # No UUID exact match. Try metadata matching. book_title = clean_string(book.title) d = self.db_book_title_cache.get(book_title, None) if d is not None: