Attach the image's XmpData to a tag when it is assigned to an image.

Remove redundant code that would set the value of a tag twice (in the tag itself, and in the image).
Remove the now useless metadata attribute.

7 files changed, 44 insertions, 132 deletions

diff --git a/src/exiv2wrapper.cpp b/src/exiv2wrapper.cpp
index 83a5712..d8ee8e5 100644
--- a/src/exiv2wrapper.cpp
+++ b/src/exiv2wrapper.cpp
@@ -329,47 +329,6 @@ const XmpTag Image::getXmpTag(std::string key)
     return XmpTag(key, &_xmpData[key]);
 }
 
-void Image::setXmpTagTextValue(const std::string& key, const std::string& value)
-{
-    CHECK_METADATA_READ
-
-    _xmpData[key].setValue(value);
-}
-
-void Image::setXmpTagArrayValue(const std::string& key, const boost::python::list& values)
-{
-    CHECK_METADATA_READ
-
-    Exiv2::Xmpdatum& datum = _xmpData[key];
-    // Reset the value
-    datum.setValue(0);
-
-    for(boost::python::stl_input_iterator<std::string> iterator(values);
-        iterator != boost::python::stl_input_iterator<std::string>();
-        ++iterator)
-    {
-        datum.setValue(*iterator);
-    }
-}
-
-void Image::setXmpTagLangAltValue(const std::string& key, const boost::python::dict& values)
-{
-    CHECK_METADATA_READ
-
-    Exiv2::Xmpdatum& datum = _xmpData[key];
-    // Reset the value
-    datum.setValue(0);
-
-    for(boost::python::stl_input_iterator<std::string> iterator(values);
-        iterator != boost::python::stl_input_iterator<std::string>();
-        ++iterator)
-    {
-        std::string key = *iterator;
-        std::string value = boost::python::extract<std::string>(values.get(key));
-        datum.setValue("lang=\"" + key + "\" " + value);
-    }
-}
-
 void Image::deleteXmpTag(std::string key)
 {
     CHECK_METADATA_READ
@@ -814,6 +773,42 @@ void XmpTag::setLangAltValue(const boost::python::dict& values)
     }
 }
 
+void XmpTag::setParentImage(Image& image)
+{
+    switch (Exiv2::XmpProperties::propertyType(_key))
+    {
+        case Exiv2::xmpText:
+        {
+            const std::string value = getTextValue();
+            delete _datum;
+            _from_datum = true;
+            _datum = &(*image.getXmpData())[_key.key()];
+            setTextValue(value);
+            break;
+        }
+        case Exiv2::xmpAlt:
+        case Exiv2::xmpBag:
+        case Exiv2::xmpSeq:
+        {
+            const boost::python::list value = getArrayValue();
+            delete _datum;
+            _from_datum = true;
+            _datum = &(*image.getXmpData())[_key.key()];
+            setArrayValue(value);
+            break;
+        }
+        case Exiv2::langAlt:
+        {
+            const boost::python::dict value = getLangAltValue();
+            delete _datum;
+            _from_datum = true;
+            _datum = &(*image.getXmpData())[_key.key()];
+            setLangAltValue(value);
+            break;
+        }
+    }
+}
+
 const std::string XmpTag::getKey()
 {
     return _key.key();
diff --git a/src/exiv2wrapper.hpp b/src/exiv2wrapper.hpp
index 43c1431..d929f8b 100644
--- a/src/exiv2wrapper.hpp
+++ b/src/exiv2wrapper.hpp
@@ -120,6 +120,7 @@ public:
     void setTextValue(const std::string& value);
     void setArrayValue(const boost::python::list& values);
     void setLangAltValue(const boost::python::dict& values);
+    void setParentImage(Image& image);
 
     const std::string getKey();
     const std::string getExiv2Type();
@@ -217,10 +218,6 @@ public:
     // Throw an exception if the tag is not set.
     const XmpTag getXmpTag(std::string key);
 
-    void setXmpTagTextValue(const std::string& key, const std::string& value);
-    void setXmpTagArrayValue(const std::string& key, const boost::python::list& values);
-    void setXmpTagLangAltValue(const std::string& key, const boost::python::dict& values);
-
     // Delete the required XMP tag.
     // Throw an exception if the tag was not set.
     void deleteXmpTag(std::string key);
@@ -237,7 +234,7 @@ public:
     // Accessors
     Exiv2::ExifData* getExifData() { return &_exifData; };
     Exiv2::IptcData* getIptcData() { return &_iptcData; };
-    // ...
+    Exiv2::XmpData* getXmpData() { return &_xmpData; };
 
 private:
     std::string _filename;
diff --git a/src/exiv2wrapper_python.cpp b/src/exiv2wrapper_python.cpp
index 77a7df7..faa9b81 100644
--- a/src/exiv2wrapper_python.cpp
+++ b/src/exiv2wrapper_python.cpp
@@ -83,6 +83,7 @@ BOOST_PYTHON_MODULE(libexiv2python)
         .def("_setTextValue", &XmpTag::setTextValue)
         .def("_setArrayValue", &XmpTag::setArrayValue)
         .def("_setLangAltValue", &XmpTag::setLangAltValue)
+        .def("_setParentImage", &XmpTag::setParentImage)
 
         .def("_getKey", &XmpTag::getKey)
         .def("_getExiv2Type", &XmpTag::getExiv2Type)
@@ -140,9 +141,6 @@ BOOST_PYTHON_MODULE(libexiv2python)
 
         .def("_xmpKeys", &Image::xmpKeys)
         .def("_getXmpTag", &Image::getXmpTag)
-        .def("_setXmpTagTextValue", &Image::setXmpTagTextValue)
-        .def("_setXmpTagArrayValue", &Image::setXmpTagArrayValue)
-        .def("_setXmpTagLangAltValue", &Image::setXmpTagLangAltValue)
         .def("_deleteXmpTag", &Image::deleteXmpTag)
 
         .def("_previews", &Image::previews)
diff --git a/src/pyexiv2/metadata.py b/src/pyexiv2/metadata.py
index 3d8892b..5cbfeb5 100644
--- a/src/pyexiv2/metadata.py
+++ b/src/pyexiv2/metadata.py
@@ -160,7 +160,6 @@ class ImageMetadata(object):
         except KeyError:
             _tag = self._image._getXmpTag(key)
             tag = XmpTag._from_existing_tag(_tag)
-            tag.metadata = self
             self._tags['xmp'][key] = tag
             return tag
 
@@ -205,7 +204,6 @@ class ImageMetadata(object):
         self._tags['iptc'][tag.key] = tag
         if tag.key not in self.iptc_keys:
             self._keys['iptc'].append(tag.key)
-        tag.metadata = self
 
     def _set_xmp_tag(self, key, tag_or_value):
         # Set an XMP tag. If the tag already exists, its value is overwritten.
@@ -214,35 +212,10 @@ class ImageMetadata(object):
         else:
             # As a handy shortcut, accept direct value assignment.
             tag = XmpTag(key, tag_or_value)
-        type = tag._tag._getExiv2Type()
-        if type == 'XmpText':
-            self._image._setXmpTagTextValue(tag.key, tag.raw_value)
-        elif type in ('XmpAlt', 'XmpBag', 'XmpSeq'):
-            self._image._setXmpTagArrayValue(tag.key, tag.raw_value)
-        elif type == 'LangAlt':
-            self._image._setXmpTagLangAltValue(tag.key, tag.raw_value)
+        tag._set_owner(self)
         self._tags['xmp'][tag.key] = tag
         if tag.key not in self.xmp_keys:
             self._keys['xmp'].append(tag.key)
-        tag.metadata = self
-
-    def _set_xmp_tag_value(self, key, value):
-        # Overwrite the tag value for an already existing tag.
-        # The tag is already in cache.
-        # Warning: this is not meant to be called directly as it doesn't update
-        # the internal cache (which would leave the object in an inconsistent
-        # state).
-        if key not in self.xmp_keys:
-            raise KeyError('Cannot set the value of an inexistent tag')
-        type = self._tags['xmp'][key]._tag._getExiv2Type()
-        if type == 'XmpText' and isinstance(value, str):
-            self._image._setXmpTagTextValue(key, value)
-        elif type in ('XmpAlt', 'XmpBag', 'XmpSeq') and isinstance(value, (list, tuple)):
-            self._image._setXmpTagArrayValue(key, value)
-        elif type == 'LangAlt' and isinstance(value, dict):
-            self._image._setXmpTagLangAltValue(key, value)
-        else:
-            raise TypeError('Expecting either a string, a list, a tuple or a dict')
 
     def __setitem__(self, key, tag_or_value):
         """
diff --git a/src/pyexiv2/xmp.py b/src/pyexiv2/xmp.py
index 63fe0a4..2142c41 100644
--- a/src/pyexiv2/xmp.py
+++ b/src/pyexiv2/xmp.py
@@ -80,9 +80,6 @@ class XmpTag(object):
     - Thumbnail: *[not implemented yet]*
     - URI, URL: string
     - XPath: *[not implemented yet]*
-
-    :attribute metadata: the parent metadata if any, or None
-    :type metadata: :class:`pyexiv2.metadata.ImageMetadata`
     """
 
     # FIXME: should inherit from ListenerInterface and implement observation of
@@ -108,13 +105,15 @@ class XmpTag(object):
             self._tag = _tag
         else:
             self._tag = libexiv2python._XmpTag(key)
-        self.metadata = None
         self._raw_value = None
         self._value = None
         self._value_cookie = False
         if value is not None:
             self._set_value(value)
 
+    def _set_owner(self, metadata):
+        self._tag._setParentImage(metadata._image)
+
     @staticmethod
     def _from_existing_tag(_tag):
         # Build a tag from an already existing libexiv2python._XmpTag
@@ -174,8 +173,6 @@ class XmpTag(object):
                 raise ValueError('Empty LangAlt')
             self._tag._setLangAltValue(value)
 
-        if self.metadata is not None:
-            self.metadata._set_xmp_tag_value(self.key, value)
         self._raw_value = value
         self._value_cookie = True
 
diff --git a/test/metadata.py b/test/metadata.py
index 3ba162d..86935ce 100644
--- a/test/metadata.py
+++ b/test/metadata.py
@@ -311,7 +311,6 @@ class TestImageMetadata(unittest.TestCase):
         key = 'Xmp.dc.subject'
         tag = self.metadata._get_xmp_tag(key)
         self.assertEqual(type(tag), XmpTag)
-        self.assertEqual(tag.metadata, self.metadata)
         self.assertEqual(self.metadata._tags['xmp'][key], tag)
         # Try to get an nonexistent tag
         key = 'Xmp.xmp.Label'
@@ -331,10 +330,8 @@ class TestImageMetadata(unittest.TestCase):
         # Create a new tag
         tag = XmpTag('Xmp.dc.title', {'x-default': 'This is not a title',
                                       'fr-FR': "Ceci n'est pas un titre"})
-        self.assertEqual(tag.metadata, None)
         self.assert_(tag.key not in self.metadata.xmp_keys)
         self.metadata._set_xmp_tag(tag.key, tag)
-        self.assertEqual(tag.metadata, self.metadata)
         self.assert_(tag.key in self.metadata.xmp_keys)
         self.assertEqual(self.metadata._tags['xmp'], {tag.key: tag})
         self.assert_(tag.key in self.metadata._image._xmpKeys())
@@ -346,9 +343,7 @@ class TestImageMetadata(unittest.TestCase):
         self.assertEqual(self.metadata._tags['xmp'], {})
         # Overwrite an existing tag
         tag = XmpTag('Xmp.dc.format', ('image', 'png'))
-        self.assertEqual(tag.metadata, None)
         self.metadata._set_xmp_tag(tag.key, tag)
-        self.assertEqual(tag.metadata, self.metadata)
         self.assertEqual(self.metadata._tags['xmp'], {tag.key: tag})
         self.assert_(tag.key in self.metadata._image._xmpKeys())
         self.assertEqual(self.metadata._image._getXmpTag(tag.key)._getTextValue(),
@@ -362,9 +357,7 @@ class TestImageMetadata(unittest.TestCase):
         tag = self.metadata._get_xmp_tag(key)
         self.assertEqual(self.metadata._tags['xmp'][key], tag)
         new_tag = XmpTag(key, ['hello', 'world'])
-        self.assertEqual(new_tag.metadata, None)
         self.metadata._set_xmp_tag(key, new_tag)
-        self.assertEqual(new_tag.metadata, self.metadata)
         self.assertEqual(self.metadata._tags['xmp'], {key: new_tag})
         self.assert_(key in self.metadata._image._xmpKeys())
         self.assertEqual(self.metadata._image._getXmpTag(key)._getArrayValue(),
@@ -382,34 +375,9 @@ class TestImageMetadata(unittest.TestCase):
         self.assert_(key in self.metadata._image._xmpKeys())
         tag = self.metadata._get_xmp_tag(key)
         self.assertEqual(tag.value, value)
-        self.assertEqual(tag.metadata, self.metadata)
         self.assertEqual(self.metadata._tags['xmp'], {key: tag})
         self.assertEqual(self.metadata._image._getXmpTag(key)._getLangAltValue(), tag.raw_value)
 
-    def test_set_xmp_tag_value_inexistent(self):
-        self.metadata.read()
-        key = 'Xmp.xmp.Nickname'
-        value = 'oSoMoN'
-        self.failUnlessRaises(KeyError, self.metadata._set_xmp_tag_value,
-                              key, value)
-
-    def test_set_xmp_tag_value_wrong_type(self):
-        self.metadata.read()
-        key = 'Xmp.dc.subject'
-        tag = self.metadata[key]
-        value = datetime.datetime(2009, 4, 21, 20, 11, 0)
-        self.failUnlessRaises(TypeError, self.metadata._set_xmp_tag_value,
-                              key, value)
-
-    def test_set_xmp_tag_value(self):
-        self.metadata.read()
-        key = 'Xmp.dc.subject'
-        tag = self.metadata._get_xmp_tag(key)
-        value = ['Hello', 'World']
-        self.failIfEqual(self.metadata._image._getXmpTag(key)._getArrayValue(), value)
-        self.metadata._set_xmp_tag_value(key, value)
-        self.assertEqual(self.metadata._image._getXmpTag(key)._getArrayValue(), value)
-
     def test_delete_xmp_tag_inexistent(self):
         self.metadata.read()
         key = 'Xmp.xmp.CreatorTool'
diff --git a/test/xmp.py b/test/xmp.py
index a4a5832..10e03a8 100644
--- a/test/xmp.py
+++ b/test/xmp.py
@@ -32,14 +32,6 @@ from pyexiv2.utils import FixedOffset
 import datetime
 
 
-class ImageMetadataMock(object):
-
-    tags = {}
-
-    def _set_xmp_tag_value(self, key, value):
-        self.tags[key] = value
-
-
 class TestXmpTag(unittest.TestCase):
 
     def test_convert_to_python_bag(self):
@@ -292,19 +284,11 @@ class TestXmpTag(unittest.TestCase):
     # TODO: other types
 
 
-    def test_set_value_no_metadata(self):
-        tag = XmpTag('Xmp.xmp.ModifyDate', datetime.datetime(2005, 9, 7, 15, 9, 51, tzinfo=FixedOffset('-', 7, 0)))
-        old_value = tag.value
-        tag.value = datetime.datetime(2009, 4, 22, 8, 30, 27, tzinfo=FixedOffset())
-        self.failIfEqual(tag.value, old_value)
-
-    def test_set_value_with_metadata(self):
+    def test_set_value(self):
         tag = XmpTag('Xmp.xmp.ModifyDate', datetime.datetime(2005, 9, 7, 15, 9, 51, tzinfo=FixedOffset('-', 7, 0)))
-        tag.metadata = ImageMetadataMock()
         old_value = tag.value
         tag.value = datetime.datetime(2009, 4, 22, 8, 30, 27, tzinfo=FixedOffset())
         self.failIfEqual(tag.value, old_value)
-        self.assertEqual(tag.metadata.tags[tag.key], '2009-04-22T08:30:27Z')
 
     def test_set_value_empty(self):
         tag = XmpTag('Xmp.dc.creator')