Add tests for set_metadata()

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',
+        for field in ('author_sort', 'publisher', 'series', 'tags', 'comments',
-            'tags', 'comments', 'languages', 'pubdate'):
+            '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)
+                    idx = key + '_index'
+                    if idx in self.fields:
                         extra = mi.get_extra(key)
-                    if extra is not None:
+                        if extra is not None or force_changes:
-                        protected_set_field(key+'_index', extra)
+                            protected_set_field(idx, extra)
 
     # }}}
 

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

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'})
+
+    # }}}