From 27678a85c6342aeb48e56b3464e0fdc84ed0f374 Mon Sep 17 00:00:00 2001 From: Kovid Goyal Date: Fri, 21 Jun 2013 16:33:43 +0530 Subject: [PATCH] Add tests for set_metadata() --- src/calibre/db/cache.py | 36 +++++++++++++++++++++++------ src/calibre/db/tests/base.py | 6 ++--- src/calibre/db/tests/writing.py | 40 +++++++++++++++++++++++++++++++-- src/calibre/db/write.py | 2 +- 4 files changed, 71 insertions(+), 13 deletions(-) diff --git a/src/calibre/db/cache.py b/src/calibre/db/cache.py index b79ff2a31b..88f06b43ba 100644 --- a/src/calibre/db/cache.py +++ b/src/calibre/db/cache.py @@ -843,9 +843,25 @@ class Cache(object): @write_api def set_metadata(self, book_id, mi, ignore_errors=False, force_changes=False, set_title=True, set_authors=True): - if callable(getattr(mi, 'to_book_metadata', None)): + ''' + Set metadata for the book `id` from the `Metadata` object `mi` + + Setting force_changes=True will force set_metadata to update fields even + if mi contains empty values. In this case, 'None' is distinguished from + 'empty'. If mi.XXX is None, the XXX is not replaced, otherwise it is. + The tags, identifiers, and cover attributes are special cases. Tags and + identifiers cannot be set to None so then will always be replaced if + force_changes is true. You must ensure that mi contains the values you + want the book to have. Covers are always changed if a new cover is + provided, but are never deleted. Also note that force_changes has no + effect on setting title or authors. + ''' + + try: # Handle code passing in an OPF object instead of a Metadata object mi = mi.to_book_metadata() + except (AttributeError, TypeError): + pass def set_field(name, val, **kwargs): self._set_field(name, {book_id:val}, **kwargs) @@ -864,7 +880,7 @@ class Cache(object): set_field('authors', authors, do_path_update=False) if path_changed: - self._update_path((book_id,)) + self._update_path({book_id}) def protected_set_field(name, val, **kwargs): try: @@ -890,12 +906,16 @@ class Cache(object): if cdata is not None: self._set_cover({book_id: cdata}) - for field in ('title_sort', 'author_sort', 'publisher', 'series', - 'tags', 'comments', 'languages', 'pubdate'): + for field in ('author_sort', 'publisher', 'series', 'tags', 'comments', + 'languages', 'pubdate'): val = mi.get(field, None) if (force_changes and val is not None) or not mi.is_null(field): protected_set_field(field, val) + val = mi.get('title_sort', None) + if (force_changes and val is not None) or not mi.is_null('title_sort'): + protected_set_field('sort', val) + # identifiers will always be replaced if force_changes is True mi_idents = mi.get_identifiers() if force_changes: @@ -917,9 +937,11 @@ class Cache(object): val = mi.get(key, None) if force_changes or val is not None: protected_set_field(key, val) - extra = mi.get_extra(key) - if extra is not None: - protected_set_field(key+'_index', extra) + idx = key + '_index' + if idx in self.fields: + extra = mi.get_extra(key) + if extra is not None or force_changes: + protected_set_field(idx, extra) # }}} diff --git a/src/calibre/db/tests/base.py b/src/calibre/db/tests/base.py index cc8da89b05..b57b017ba3 100644 --- a/src/calibre/db/tests/base.py +++ b/src/calibre/db/tests/base.py @@ -78,7 +78,7 @@ class BaseTest(unittest.TestCase): def cloned_library(self): return self.clone_library(self.library_path) - def compare_metadata(self, mi1, mi2): + def compare_metadata(self, mi1, mi2, exclude=()): allfk1 = mi1.all_field_keys() allfk2 = mi2.all_field_keys() self.assertEqual(allfk1, allfk2) @@ -88,7 +88,7 @@ class BaseTest(unittest.TestCase): 'ondevice_col', 'last_modified', 'has_cover', 'cover_data'}.union(allfk1) for attr in all_keys: - if attr == 'user_metadata': + if attr == 'user_metadata' or attr in exclude: continue attr1, attr2 = getattr(mi1, attr), getattr(mi2, attr) if attr == 'formats': @@ -97,7 +97,7 @@ class BaseTest(unittest.TestCase): attr1, attr2 = set(attr1), set(attr2) self.assertEqual(attr1, attr2, '%s not the same: %r != %r'%(attr, attr1, attr2)) - if attr.startswith('#'): + if attr.startswith('#') and attr + '_index' not in exclude: attr1, attr2 = mi1.get_extra(attr), mi2.get_extra(attr) self.assertEqual(attr1, attr2, '%s {#extra} not the same: %r != %r'%(attr, attr1, attr2)) diff --git a/src/calibre/db/tests/writing.py b/src/calibre/db/tests/writing.py index 6d3169b905..5d04c11def 100644 --- a/src/calibre/db/tests/writing.py +++ b/src/calibre/db/tests/writing.py @@ -376,7 +376,43 @@ class WritingTest(BaseTest): self.assertTrue(old.has_cover(book_id)) # }}} - def test_set_metadata(self): + def test_set_metadata(self): # {{{ ' Test setting of metadata ' - self.assertTrue(False, 'TODO: test set_metadata()') + ae = self.assertEqual + cache = self.init_cache(self.cloned_library) + + # Check that changing title/author updates the path + mi = cache.get_metadata(1) + old_path = cache.field_for('path', 1) + old_title, old_author = mi.title, mi.authors[0] + ae(old_path, '%s/%s (1)' % (old_author, old_title)) + mi.title, mi.authors = 'New Title', ['New Author'] + cache.set_metadata(1, mi) + ae(cache.field_for('path', 1), '%s/%s (1)' % (mi.authors[0], mi.title)) + p = cache.format_abspath(1, 'FMT1') + self.assertTrue(mi.authors[0] in p and mi.title in p) + + # Compare old and new set_metadata() + db = self.init_old(self.cloned_library) + mi = db.get_metadata(1, index_is_id=True, get_cover=True, cover_as_data=True) + mi2 = db.get_metadata(3, index_is_id=True, get_cover=True, cover_as_data=True) + db.set_metadata(2, mi) + db.set_metadata(1, mi2, force_changes=True) + oldmi = db.get_metadata(2, index_is_id=True, get_cover=True, cover_as_data=True) + oldmi2 = db.get_metadata(1, index_is_id=True, get_cover=True, cover_as_data=True) + db.close() + del db + cache = self.init_cache(self.cloned_library) + cache.set_metadata(2, mi) + nmi = cache.get_metadata(2, get_cover=True, cover_as_data=True) + ae(oldmi.cover_data, nmi.cover_data) + self.compare_metadata(nmi, oldmi, exclude={'last_modified', 'format_metadata'}) + cache.set_metadata(1, mi2, force_changes=True) + nmi2 = cache.get_metadata(1, get_cover=True, cover_as_data=True) + # The new code does not allow setting of #series_index to None, instead + # it is reset to 1.0 + ae(nmi2.get_extra('#series'), 1.0) + self.compare_metadata(nmi2, oldmi2, exclude={'last_modified', 'format_metadata', '#series_index'}) + + # }}} diff --git a/src/calibre/db/write.py b/src/calibre/db/write.py index 7fdb2070c0..9bae3e6abb 100644 --- a/src/calibre/db/write.py +++ b/src/calibre/db/write.py @@ -212,7 +212,7 @@ def custom_series_index(book_id_val_map, db, field, *args): ids = series_field.ids_for_book(book_id) if ids: sequence.append((sidx, book_id, ids[0])) - field.table.book_col_map[book_id] = sidx + field.table.book_col_map[book_id] = sidx if sequence: db.conn.executemany('UPDATE %s SET %s=? WHERE book=? AND value=?'%( field.metadata['table'], field.metadata['column']), sequence)