Changes to respond to Kovid's mail, and some cleanups.

src/calibre/ebooks/metadata/book/__init__.py

@@ -101,7 +101,9 @@ COPYABLE_METADATA_FIELDS = SOCIAL_METADATA_FIELDS.union(
                            BOOK_STRUCTURE_FIELDS).union(
                            DEVICE_METADATA_FIELDS).union(
                            CALIBRE_METADATA_FIELDS) - \
-                           frozenset(['title', 'authors', 'comments', 'cover_data'])
+                           frozenset(['title', 'title_sort', 'authors',
+                                      'author_sort', 'author_sort_map' 'comments',
+                                      'cover_data', 'tags', 'language'])
 
 SERIALIZABLE_FIELDS =      SOCIAL_METADATA_FIELDS.union(
                            USER_METADATA_FIELDS).union(

src/calibre/ebooks/metadata/book/base.py

@@ -66,7 +66,7 @@ class Metadata(object):
         raise AttributeError(
                 'Metadata object has no attribute named: '+ repr(field))
 
-    def __setattr__(self, field, val):
+    def __setattr__(self, field, val, extra=None):
         _data = object.__getattribute__(self, '_data')
         if field in STANDARD_METADATA_FIELDS:
             if val is None:
@@ -74,17 +74,23 @@ class Metadata(object):
             _data[field] = val
         elif field in _data['user_metadata'].iterkeys():
             _data['user_metadata'][field]['#value#'] = val
+            _data['user_metadata'][field]['#extra#'] = extra
         else:
             # You are allowed to stick arbitrary attributes onto this object as
             # long as they don't conflict with global or user metadata names
             # Don't abuse this privilege
             self.__dict__[field] = val
 
-    def get(self, field):
+    def get(self, field, default=None):
+        if default is not None:
+            try:
+                return self.__getattribute__(field)
+            except AttributeError:
+                return default
         return self.__getattribute__(field)
 
-    def set(self, field, val):
+    def set(self, field, val, extra=None):
-        self.__setattr__(field, val)
+        self.__setattr__(field, val, extra)
 
     @property
     def user_metadata_keys(self):
@@ -92,25 +98,25 @@ class Metadata(object):
         _data = object.__getattribute__(self, '_data')
         return frozenset(_data['user_metadata'].iterkeys())
 
-    @property
+    def get_all_user_metadata(self, make_copy):
-    def all_user_metadata(self):
         '''
         return a dict containing all the custom field metadata associated with
-        the book. Return a deep copy, just in case the user wants to change
+        the book.
-        values in the dict (json does).
         '''
         _data = object.__getattribute__(self, '_data')
-        _data = _data['user_metadata']
+        user_metadata = _data['user_metadata']
+        if not make_copy:
+            return user_metadata
         res = {}
-        for k in _data:
+        for k in user_metadata:
-            res[k] = copy.deepcopy(_data[k])
+            res[k] = copy.deepcopy(user_metadata[k])
         return res
 
     def get_user_metadata(self, field):
         '''
         return field metadata from the object if it is there. Otherwise return
-        None. field is the key name, not the label. Return a shallow copy,
+        None. field is the key name, not the label. Return a copy, just in case
-        just in case the user wants to change values in the dict (json does).
+        the user wants to change values in the dict (json does).
         '''
         _data = object.__getattribute__(self, '_data')
         _data = _data['user_metadata']
@@ -118,6 +124,14 @@ class Metadata(object):
             return copy.deepcopy(_data[field])
         return None
 
+    @classmethod
+    def get_user_metadata_value(user_mi):
+        return user_mi['#value#']
+
+    @classmethod
+    def get_user_metadata_extra(user_mi):
+        return user_mi['#extra#']
+
     def set_all_user_metadata(self, metadata):
         '''
         store custom field metadata into the object. Field is the key name
@@ -139,21 +153,30 @@ class Metadata(object):
                 traceback.print_stack()
             metadata = copy.deepcopy(metadata)
             if '#value#' not in metadata:
-                metadata['#value#'] = None
+                if metadata['datatype'] == 'text' and metadata['is_multiple']:
+                    metadata['#value#'] = []
+                else:
+                    metadata['#value#'] = None
             _data = object.__getattribute__(self, '_data')
             _data['user_metadata'][field] = metadata
 
-    @property
+    def get_all_non_none_attributes(self):
-    def all_attributes(self):
+        '''
+        Return a dictionary containing all non-None metadata fields, including
+        the custom ones.
+        '''
         result = {}
         _data = object.__getattribute__(self, '_data')
         for attr in STANDARD_METADATA_FIELDS:
             v = _data.get(attr, None)
             if v is not None:
                 result[attr] = v
-        for attr in self.user_metadata_keys:
+        for attr in _data['user_metadata'].iterkeys():
-            if self.get(attr) is not None:
+            v = _data['user_metadata'][attr]['#value#']
-                result[attr] = self.get(attr)
+            if v is not None:
+                result[attr] = v
+                if _data['user_metadata'][attr]['datatype'] == 'series':
+                    result[attr+'_index'] = _data['user_metadata'][attr]['#extra#']
         return result
 
     # Old Metadata API {{{
@@ -184,45 +207,49 @@ class Metadata(object):
         '''
         if other.title and other.title != _('Unknown'):
             self.title = other.title
+            if hasattr(other, 'title_sort'):
+                self.title_sort = other.title_sort
 
         if other.authors and other.authors[0] != _('Unknown'):
             self.authors = other.authors
+            if hasattr(other, 'author_sort_map'):
+                self.author_sort_map = other.author_sort_map
+            if hasattr(other, 'author_sort'):
+                self.author_sort = other.author_sort
 
-        for attr in COPYABLE_METADATA_FIELDS:
+        if replace_metadata:
-            if replace_metadata:
+            for attr in COPYABLE_METADATA_FIELDS:
                 setattr(self, attr, getattr(other, attr, 1.0 if \
                         attr == 'series_index' else None))
-            elif hasattr(other, attr):
-                val = getattr(other, attr)
-                if val is not None:
-                    setattr(self, attr, copy.deepcopy(val))
-
-        if replace_metadata:
             self.tags = other.tags
-        elif other.tags:
+            self.cover_data = getattr(other, 'cover_data', '')
-            self.tags += other.tags
+            self.set_all_user_metadata(other.get_all_user_metadata(make_copy=True))
-        self.tags = list(set(self.tags))
-
-        if getattr(other, 'author_sort_map', None):
-            self.author_sort_map.update(other.author_sort_map)
-
-        if getattr(other, 'cover_data', False):
-            other_cover = other.cover_data[-1]
-            self_cover = self.cover_data[-1] if self.cover_data else ''
-            if not self_cover: self_cover = ''
-            if not other_cover: other_cover = ''
-            if len(other_cover) > len(self_cover):
-                self.cover_data = other.cover_data
-
-        if getattr(other, 'user_metadata_keys', None):
-            for x in other.user_metadata_keys:
-                meta = other.get_user_metadata(x)
-                if meta is not None or replace_metadata:
-                    self.set_user_metadata(x, meta) # get... did the deepcopy
-
-        if replace_metadata:
             self.comments = getattr(other, 'comments', '')
+            self.language = getattr(other, 'language', None)
         else:
+            for attr in COPYABLE_METADATA_FIELDS:
+                if hasattr(other, attr):
+                    val = getattr(other, attr)
+                    if val is not None:
+                        setattr(self, attr, copy.deepcopy(val))
+
+            if other.tags:
+                self.tags += list(set(self.tags + other.tags))
+
+            if getattr(other, 'cover_data', False):
+                other_cover = other.cover_data[-1]
+                self_cover = self.cover_data[-1] if self.cover_data else ''
+                if not self_cover: self_cover = ''
+                if not other_cover: other_cover = ''
+                if len(other_cover) > len(self_cover):
+                    self.cover_data = other.cover_data
+
+            if getattr(other, 'user_metadata_keys', None):
+                for x in other.user_metadata_keys:
+                    meta = other.get_user_metadata(x)
+                    if meta is not None:
+                        self.set_user_metadata(x, meta) # get... did the deepcopy
+
             my_comments = getattr(self, 'comments', '')
             other_comments = getattr(other, 'comments', '')
             if not my_comments:
@@ -232,10 +259,9 @@ class Metadata(object):
             if len(other_comments.strip()) > len(my_comments.strip()):
                 self.comments = other_comments
 
-        other_lang = getattr(other, 'language', None)
+            other_lang = getattr(other, 'language', None)
-        if other_lang and other_lang.lower() != 'und':
+            if other_lang and other_lang.lower() != 'und':
-            self.language = other_lang
+                self.language = other_lang
-
 
     def format_series_index(self):
         from calibre.ebooks.metadata import fmt_sidx

src/calibre/ebooks/metadata/book/json_codec.py

@@ -70,7 +70,7 @@ class JsonCodec(object):
 
     def encode_metadata_attr(self, book, key):
         if key == 'user_metadata':
-            meta = book.all_user_metadata
+            meta = book.get_all_user_metadata(make_copy=True)
             for k in meta:
                 if meta[k]['datatype'] == 'datetime':
                     meta[k]['#value#'] = datetime_to_string(meta[k]['#value#'])

src/calibre/library/database2.py

@@ -22,6 +22,7 @@ from calibre.library.sqlite import connect, IntegrityError, DBThread
 from calibre.library.prefs import DBPrefs
 from calibre.ebooks.metadata import string_to_authors, authors_to_string, \
                                     MetaInformation
+from calibre.ebooks.metadata.book.base import Metadata
 from calibre.ebooks.metadata.meta import get_metadata, metadata_from_formats
 from calibre.constants import preferred_encoding, iswindows, isosx, filesystem_encoding
 from calibre.ptempfile import PersistentTemporaryFile
@@ -537,7 +538,10 @@ class LibraryDatabase2(LibraryDatabase, SchemaUpgrade, CustomColumns):
         for key,meta in self.field_metadata.iteritems():
             if meta['is_custom']:
                 mi.set_user_metadata(key, meta)
-                mi.set(key, self.get_custom(idx, label=meta['label'], index_is_id=index_is_id))
+                mi.set(key, val=self.get_custom(idx, label=meta['label'],
+                                                index_is_id=index_is_id),
+                            extra=self.get_custom_extra(idx, label=meta['label'],
+                                                        index_is_id=index_is_id))
         if get_cover:
             mi.cover = self.cover(id, index_is_id=True, as_path=True)
         return mi
@@ -1038,6 +1042,15 @@ class LibraryDatabase2(LibraryDatabase, SchemaUpgrade, CustomColumns):
         if getattr(mi, 'timestamp', None) is not None:
             doit(self.set_timestamp, id, mi.timestamp, notify=False)
         self.set_path(id, True)
+
+        user_mi = mi.get_all_user_metadata(make_copy=False)
+        for key in user_mi.iterkeys():
+            if key in self.field_metadata and \
+                    user_mi[key]['datatype'] == self.field_metadata[key]['datatype']:
+                doit(self.set_custom, id,
+                     val=Metadata.get_user_metadata_value(user_mi[key]),
+                     extra=Metadata.get_user_metadata_extra(user_mi[key]),
+                     label=user_mi[key]['label'])
         self.notify('metadata', [id])
 
     # Given a book, return the list of author sort strings for the book's authors

src/calibre/library/save_to_disk.py

@@ -115,7 +115,7 @@ def get_components(template, mi, id, timefmt='%b %Y', length=250,
     library_order = tweaks['save_template_title_series_sorting'] == 'library_order'
     tsfmt = title_sort if library_order else lambda x: x
     format_args = dict(**FORMAT_ARGS)
-    format_args.update(mi.all_attributes)
+    format_args.update(mi.get_all_non_none_attributes())
     if mi.title:
         format_args['title'] = tsfmt(mi.title)
     if mi.authors:
@@ -131,6 +131,8 @@ def get_components(template, mi, id, timefmt='%b %Y', length=250,
             format_args['series_index'] = mi.format_series_index()
     else:
         template = re.sub(r'\{series_index[^}]*?\}', '', template)
+    ## TODO: format custom values. Check all the datatypes.
+
     if mi.rating is not None:
         format_args['rating'] = mi.format_rating()
     if hasattr(mi.timestamp, 'timetuple'):
@@ -139,15 +141,6 @@ def get_components(template, mi, id, timefmt='%b %Y', length=250,
         format_args['pubdate'] = strftime(timefmt, mi.pubdate.timetuple())
     format_args['id'] = str(id)
 
-    # These are not necessary any more. The values are set by
-    # 'format_args.update' above, and there is no special formatting
-#    if mi.author_sort:
-#        format_args['author_sort'] = mi.author_sort
-#    if mi.isbn:
-#        format_args['isbn'] = mi.isbn
-#    if mi.publisher:
-#        format_args['publisher'] = mi.publisher
-
     components = [x.strip() for x in template.split('/') if x.strip()]
     components = [safe_format(x, format_args) for x in components]
     components = [sanitize_func(x) for x in components if x]