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

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

7 files changed, 17 insertions, 87 deletions

diff --git a/src/exiv2wrapper.cpp b/src/exiv2wrapper.cpp
index c579b87..4ecbf48 100644
--- a/src/exiv2wrapper.cpp
+++ b/src/exiv2wrapper.cpp
@@ -328,13 +328,6 @@ const IptcTag Image::getIptcTag(std::string key)
     return IptcTag(key, &_iptcData);
 }
 
-void Image::setIptcTagValues(std::string key, boost::python::list values)
-{
-    CHECK_METADATA_READ
-
-    set_iptc_tag_values(key, &_iptcData, values);
-}
-
 void Image::deleteIptcTag(std::string key)
 {
     CHECK_METADATA_READ
@@ -686,6 +679,15 @@ void IptcTag::setRawValues(const boost::python::list& values)
     set_iptc_tag_values(_key.key(), _data, values);
 }
 
+void IptcTag::setParentImage(Image& image)
+{
+    const boost::python::list values = getRawValues();
+    delete _data;
+    _from_data = true;
+    _data = image.getIptcData();
+    setRawValues(values);
+}
+
 const std::string IptcTag::getKey()
 {
     return _key.key();
diff --git a/src/exiv2wrapper.hpp b/src/exiv2wrapper.hpp
index 6ba62f9..43c1431 100644
--- a/src/exiv2wrapper.hpp
+++ b/src/exiv2wrapper.hpp
@@ -81,6 +81,7 @@ public:
     ~IptcTag();
 
     void setRawValues(const boost::python::list& values);
+    void setParentImage(Image& image);
 
     const std::string getKey();
     const std::string getType();
@@ -206,10 +207,6 @@ public:
     // Throw an exception if the tag is not set.
     const IptcTag getIptcTag(std::string key);
 
-    // Set the IPTC tag's values. If the tag was not previously set, it is
-    // created.
-    void setIptcTagValues(std::string key, boost::python::list values);
-
     // Delete (all the repetitions of) the required IPTC tag.
     // Throw an exception if the tag was not set.
     void deleteIptcTag(std::string key);
@@ -239,6 +236,7 @@ public:
 
     // Accessors
     Exiv2::ExifData* getExifData() { return &_exifData; };
+    Exiv2::IptcData* getIptcData() { return &_iptcData; };
     // ...
 
 private:
diff --git a/src/exiv2wrapper_python.cpp b/src/exiv2wrapper_python.cpp
index e970c69..77a7df7 100644
--- a/src/exiv2wrapper_python.cpp
+++ b/src/exiv2wrapper_python.cpp
@@ -64,6 +64,7 @@ BOOST_PYTHON_MODULE(libexiv2python)
     class_<IptcTag>("_IptcTag", init<std::string>())
 
         .def("_setRawValues", &IptcTag::setRawValues)
+        .def("_setParentImage", &IptcTag::setParentImage)
 
         .def("_getKey", &IptcTag::getKey)
         .def("_getType", &IptcTag::getType)
@@ -135,7 +136,6 @@ BOOST_PYTHON_MODULE(libexiv2python)
 
         .def("_iptcKeys", &Image::iptcKeys)
         .def("_getIptcTag", &Image::getIptcTag)
-        .def("_setIptcTagValues", &Image::setIptcTagValues)
         .def("_deleteIptcTag", &Image::deleteIptcTag)
 
         .def("_xmpKeys", &Image::xmpKeys)
diff --git a/src/pyexiv2/iptc.py b/src/pyexiv2/iptc.py
index f92e883..ac1954d 100644
--- a/src/pyexiv2/iptc.py
+++ b/src/pyexiv2/iptc.py
@@ -72,9 +72,6 @@ class IptcTag(ListenerInterface):
     - Date: :class:`datetime.date`
     - Time: :class:`datetime.time`
     - Undefined: string
-
-    :attribute metadata: the parent metadata if any, or None
-    :type metadata: :class:`pyexiv2.metadata.ImageMetadata`
     """
 
     # strptime is not flexible enough to handle all valid Time formats, we use a
@@ -96,13 +93,15 @@ class IptcTag(ListenerInterface):
             self._tag = _tag
         else:
             self._tag = libexiv2python._IptcTag(key)
-        self.metadata = None
         self._raw_values = None
         self._values = None
         self._values_cookie = False
         if values is not None:
             self._set_values(values)
 
+    def _set_owner(self, metadata):
+        self._tag._setParentImage(metadata._image)
+
     @staticmethod
     def _from_existing_tag(_tag):
         # Build a tag from an already existing libexiv2python._IptcTag
@@ -168,8 +167,6 @@ class IptcTag(ListenerInterface):
         if not isinstance(values, (list, tuple)):
             raise TypeError('Expecting a list of values')
         self._tag._setRawValues(values)
-        if self.metadata is not None:
-            self.metadata._set_iptc_tag_values(self.key, values)
         self._raw_values = values
         self._values_cookie = True
 
diff --git a/src/pyexiv2/metadata.py b/src/pyexiv2/metadata.py
index 93ab4bb..3d8892b 100644
--- a/src/pyexiv2/metadata.py
+++ b/src/pyexiv2/metadata.py
@@ -149,7 +149,6 @@ class ImageMetadata(object):
         except KeyError:
             _tag = self._image._getIptcTag(key)
             tag = IptcTag._from_existing_tag(_tag)
-            tag.metadata = self
             self._tags['iptc'][key] = tag
             return tag
 
@@ -202,28 +201,12 @@ class ImageMetadata(object):
         else:
             # As a handy shortcut, accept direct value assignment.
             tag = IptcTag(key, tag_or_values)
-        self._image._setIptcTagValues(tag.key, tag.raw_values)
+        tag._set_owner(self)
         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_iptc_tag_values(self, key, values):
-        # Overwrite the tag values 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).
-        # FIXME: this is sub-optimal as it sets all the values regardless of how
-        # many of them really changed. Need to implement the same method with an
-        # index/range parameter (here and in the C++ wrapper).
-        if key not in self.iptc_keys:
-            raise KeyError('Cannot set the value of an inexistent tag')
-        if type(values) is not list or not \
-            reduce(lambda x, y: x and type(y) is str, values, True):
-            raise TypeError('Expecting a list of strings')
-        self._image._setIptcTagValues(key, values)
-
     def _set_xmp_tag(self, key, tag_or_value):
         # Set an XMP tag. If the tag already exists, its value is overwritten.
         if isinstance(tag_or_value, XmpTag):
diff --git a/test/iptc.py b/test/iptc.py
index 160c599..3b151a5 100644
--- a/test/iptc.py
+++ b/test/iptc.py
@@ -32,14 +32,6 @@ from pyexiv2.utils import FixedOffset
 import datetime
 
 
-class ImageMetadataMock(object):
-
-    tags = {}
-
-    def _set_iptc_tag_values(self, key, values):
-        self.tags[key] = values
-
-
 class TestIptcTag(unittest.TestCase):
 
     def test_convert_to_python_short(self):
@@ -192,17 +184,9 @@ class TestIptcTag(unittest.TestCase):
         tag = IptcTag('Iptc.Application2.City', ['Seattle'])
         self.failUnlessRaises(TypeError, tag._set_values, 'Barcelona')
 
-    def test_set_values_no_metadata(self):
-        tag = IptcTag('Iptc.Application2.City', ['Seattle'])
-        old_values = tag.values
-        tag.values = ['Barcelona']
-        self.failIfEqual(tag.values, old_values)
-
-    def test_set_values_with_metadata(self):
+    def test_set_values(self):
         tag = IptcTag('Iptc.Application2.City', ['Seattle'])
-        tag.metadata = ImageMetadataMock()
         old_values = tag.values
         tag.values = ['Barcelona']
         self.failIfEqual(tag.values, old_values)
-        self.assertEqual(tag.metadata.tags[tag.key], ['Barcelona'])
 
diff --git a/test/metadata.py b/test/metadata.py
index 3756d7d..3ba162d 100644
--- a/test/metadata.py
+++ b/test/metadata.py
@@ -203,7 +203,6 @@ class TestImageMetadata(unittest.TestCase):
         key = 'Iptc.Application2.DateCreated'
         tag = self.metadata._get_iptc_tag(key)
         self.assertEqual(type(tag), IptcTag)
-        self.assertEqual(tag.metadata, self.metadata)
         self.assertEqual(self.metadata._tags['iptc'][key], tag)
         # Try to get an nonexistent tag
         key = 'Iptc.Application2.Copyright'
@@ -222,10 +221,8 @@ class TestImageMetadata(unittest.TestCase):
         self.assertEqual(self.metadata._tags['iptc'], {})
         # Create a new tag
         tag = IptcTag('Iptc.Application2.Writer', ['Nobody'])
-        self.assertEqual(tag.metadata, None)
         self.assert_(tag.key not in self.metadata.iptc_keys)
         self.metadata._set_iptc_tag(tag.key, tag)
-        self.assertEqual(tag.metadata, self.metadata)
         self.assert_(tag.key in self.metadata.iptc_keys)
         self.assertEqual(self.metadata._tags['iptc'], {tag.key: tag})
         self.assert_(tag.key in self.metadata._image._iptcKeys())
@@ -237,9 +234,7 @@ class TestImageMetadata(unittest.TestCase):
         self.assertEqual(self.metadata._tags['iptc'], {})
         # Overwrite an existing tag
         tag = IptcTag('Iptc.Application2.Caption', ['A picture.'])
-        self.assertEqual(tag.metadata, None)
         self.metadata._set_iptc_tag(tag.key, tag)
-        self.assertEqual(tag.metadata, self.metadata)
         self.assertEqual(self.metadata._tags['iptc'], {tag.key: tag})
         self.assert_(tag.key in self.metadata._image._iptcKeys())
         self.assertEqual(self.metadata._image._getIptcTag(tag.key)._getRawValues(),
@@ -253,9 +248,7 @@ class TestImageMetadata(unittest.TestCase):
         tag = self.metadata._get_iptc_tag(key)
         self.assertEqual(self.metadata._tags['iptc'][key], tag)
         new_tag = IptcTag(key, ['A picture.'])
-        self.assertEqual(new_tag.metadata, None)
         self.metadata._set_iptc_tag(key, new_tag)
-        self.assertEqual(new_tag.metadata, self.metadata)
         self.assertEqual(self.metadata._tags['iptc'], {key: new_tag})
         self.assert_(key in self.metadata._image._iptcKeys())
         self.assertEqual(self.metadata._image._getIptcTag(key)._getRawValues(),
@@ -272,37 +265,10 @@ class TestImageMetadata(unittest.TestCase):
         self.assert_(key in self.metadata._image._iptcKeys())
         tag = self.metadata._get_iptc_tag(key)
         self.assertEqual(tag.values, values)
-        self.assertEqual(tag.metadata, self.metadata)
         self.assertEqual(self.metadata._tags['iptc'], {key: tag})
         self.assertEqual(self.metadata._image._getIptcTag(key)._getRawValues(),
                          tag.raw_values)
 
-    def test_set_iptc_tag_values_inexistent(self):
-        self.metadata.read()
-        key = 'Iptc.Application2.Urgency'
-        values = ['1']
-        self.failUnlessRaises(KeyError, self.metadata._set_iptc_tag_values,
-                              key, values)
-
-    def test_set_iptc_tag_values_wrong_type(self):
-        self.metadata.read()
-        key = 'Iptc.Application2.DateCreated'
-        value = '20090324'
-        self.failUnlessRaises(TypeError, self.metadata._set_iptc_tag_values,
-                              key, value)
-        values = [datetime.date(2009, 3, 24)]
-        self.failUnlessRaises(TypeError, self.metadata._set_iptc_tag_values,
-                              key, values)
-
-    def test_set_iptc_tag_values(self):
-        self.metadata.read()
-        key = 'Iptc.Application2.DateCreated'
-        tag = self.metadata._get_iptc_tag(key)
-        values = ['2009-04-07']
-        self.failIfEqual(self.metadata._image._getIptcTag(key)._getRawValues(), values)
-        self.metadata._set_iptc_tag_values(key, values)
-        self.assertEqual(self.metadata._image._getIptcTag(key)._getRawValues(), values)
-
     def test_delete_iptc_tag_inexistent(self):
         self.metadata.read()
         key = 'Iptc.Application2.LocationCode'