Private value setter for IPTC tags.

6 files changed, 152 insertions, 53 deletions

diff --git a/src/exiv2wrapper.cpp b/src/exiv2wrapper.cpp
index 4f0ceb9..d29c332 100644
--- a/src/exiv2wrapper.cpp
+++ b/src/exiv2wrapper.cpp
@@ -123,7 +123,7 @@ boost::python::tuple Image::getExifTag(std::string key)
     }
 }
 
-void Image::setExifTag(std::string key, std::string value)
+void Image::setExifTagValue(std::string key, std::string value)
 {
     if(_dataRead)
     {
@@ -221,36 +221,10 @@ boost::python::tuple Image::getIptcTag(std::string key)
     }
 }
 
-/*boost::python::list Image::getIptcTag(std::string key)
+/*void Image::setIptcTag(std::string key, std::string value, unsigned int index=0)
 {
     if(_dataRead)
     {
-        boost::python::list valuesList;
-        unsigned int valueOccurences = 0;
-        Exiv2::IptcKey iptcKey = Exiv2::IptcKey(key);
-        for (Exiv2::IptcMetadata::iterator dataIterator = _iptcData.begin();
-            dataIterator != _iptcData.end(); ++dataIterator)
-        {
-            if (dataIterator->key() == key)
-            {
-                valuesList.append(boost::python::make_tuple(std::string(dataIterator->typeName()), dataIterator->toString()));
-                ++valueOccurences;
-            }
-        }
-        if (valueOccurences > 0)
-            return valuesList;
-        else
-            throw Exiv2::Error(KEY_NOT_FOUND, key);
-    }
-    else
-        throw Exiv2::Error(METADATA_NOT_READ);
-}
-
-boost::python::tuple Image::setIptcTag(std::string key, std::string value, unsigned int index=0)
-{
-    if(_dataRead)
-    {
-        boost::python::tuple returnValue;
         unsigned int indexCounter = index;
         Exiv2::IptcKey iptcKey = Exiv2::IptcKey(key);
         Exiv2::IptcMetadata::iterator dataIterator = _iptcData.findKey(iptcKey);
@@ -263,7 +237,6 @@ boost::python::tuple Image::setIptcTag(std::string key, std::string value, unsig
         if (dataIterator != _iptcData.end())
         {
             // The tag at given index already exists, override it
-            returnValue = boost::python::make_tuple(std::string(dataIterator->typeName()), dataIterator->toString());
             dataIterator->setValue(value);
         }
         else
@@ -271,24 +244,65 @@ boost::python::tuple Image::setIptcTag(std::string key, std::string value, unsig
             // Either index is greater than the index of the last repetition
             // of the tag, or the tag does not exist yet.
             // In both cases, it is created.
-            returnValue = boost::python::make_tuple(std::string(""), std::string(""));
             Exiv2::Iptcdatum iptcDatum(iptcKey);
             iptcDatum.setValue(value);
             int state = _iptcData.add(iptcDatum);
             if (state == 6)
                 throw Exiv2::Error(NON_REPEATABLE);
         }
-        return returnValue;
     }
     else
         throw Exiv2::Error(METADATA_NOT_READ);
+}*/
+
+void Image::setIptcTagValues(std::string key, boost::python::tuple values)
+{
+    if (!_dataRead)
+    {
+        throw Exiv2::Error(METADATA_NOT_READ);
+    }
+
+    Exiv2::IptcKey iptcKey = Exiv2::IptcKey(key);
+    unsigned int index = 0;
+    unsigned int max = len(values);
+    Exiv2::IptcMetadata::iterator dataIterator = _iptcData.findKey(iptcKey);
+    while (index < max)
+    {
+        std::string value = boost::python::extract<std::string>(values[index++]);
+        if (dataIterator != _iptcData.end())
+        {
+            // Override an existing value
+            dataIterator->setValue(value);
+            dataIterator = std::find_if(++dataIterator, _iptcData.end(),
+                Exiv2::FindMetadatumById::FindMetadatumById(iptcKey.tag(),
+                                                            iptcKey.record()));
+        }
+        else
+        {
+            // Append a new value
+            Exiv2::Iptcdatum iptcDatum(iptcKey);
+            iptcDatum.setValue(value);
+            int state = _iptcData.add(iptcDatum);
+            if (state == 6)
+            {
+                throw Exiv2::Error(NON_REPEATABLE);
+            }
+        }
+    }
+    // Erase the remaining values if any
+    while (dataIterator != _iptcData.end())
+    {
+        _iptcData.erase(dataIterator);
+        dataIterator = std::find_if(dataIterator, _iptcData.end(),
+                Exiv2::FindMetadatumById::FindMetadatumById(iptcKey.tag(),
+                                                            iptcKey.record()));
+    }
 }
 
-boost::python::tuple Image::deleteIptcTag(std::string key, unsigned int index=0)
+/*void Image::deleteIptcTag(std::string key, unsigned int index=0)
 {
     if(_dataRead)
     {
-        boost::python::tuple returnValue;
         unsigned int indexCounter = index;
         Exiv2::IptcKey iptcKey = Exiv2::IptcKey(key);
         Exiv2::IptcMetadata::iterator dataIterator = _iptcData.findKey(iptcKey);
@@ -301,9 +315,7 @@ boost::python::tuple Image::deleteIptcTag(std::string key, unsigned int index=0)
         if (dataIterator != _iptcData.end())
         {
             // The tag at given index already exists, delete it
-            returnValue = boost::python::make_tuple(std::string(dataIterator->typeName()), dataIterator->toString());
             _iptcData.erase(dataIterator);
-            return returnValue;
         }
         else
             throw Exiv2::Error(KEY_NOT_FOUND, key);
diff --git a/src/exiv2wrapper.hpp b/src/exiv2wrapper.hpp
index 2ad8483..3bc5314 100644
--- a/src/exiv2wrapper.hpp
+++ b/src/exiv2wrapper.hpp
@@ -69,7 +69,7 @@ public:
 
     // Set the EXIF tag's value. If the tag was not previously set, it is
     // created.
-    void setExifTag(std::string key, std::string value);
+    void setExifTagValue(std::string key, std::string value);
 
     // Delete the required EXIF tag.
     // Throw an exception if the tag was not set.
@@ -95,23 +95,22 @@ public:
     // tagvalue (list)
     boost::python::tuple getIptcTag(std::string key);
 
-    // Set the IPTC tag's value and return a tuple containing the
-    // type and previous value of the tag (empty strings if not previously
-    // set). If the tag was not previously set, it is created.
+    // Set the IPTC tag's value. If the tag was not previously set, it is
+    // created.
     // If the key references a repeatable tag, the parameter index (starting
     // from 0 like a list index) is used to determine which of the
     // repetitions is to be set. In case of an index greater than the
     // highest existing one, adds a repetition of the tag.
-    //boost::python::tuple setIptcTag(std::string key, std::string value, unsigned int index);
+    //void setIptcTag(std::string key, std::string value, unsigned int index);
+    void setIptcTagValues(std::string key, boost::python::tuple values);
 
-    // Delete the required IPTC tag and return a tuple containing the
-    // type and previous value.
+    // Delete the required IPTC tag.
     // If the key references a repeatable tag, the parameter index (starting
     // from 0 like a list index) is used to determine which of the
     // repetitions is to be deleted.
     // Throw an exception if the tag was not set or if the index is greater
     // than the highest existing one.
-    //boost::python::tuple deleteIptcTag(std::string key, unsigned int index);
+    //void deleteIptcTag(std::string key, unsigned int index);
 
     boost::python::list xmpKeys();
 
diff --git a/src/exiv2wrapper_python.cpp b/src/exiv2wrapper_python.cpp
index e9b78eb..238e39b 100644
--- a/src/exiv2wrapper_python.cpp
+++ b/src/exiv2wrapper_python.cpp
@@ -54,13 +54,13 @@ BOOST_PYTHON_MODULE(libexiv2python)
 
         .def("exifKeys", &Image::exifKeys)
         .def("getExifTag", &Image::getExifTag)
-        .def("setExifTag", &Image::setExifTag)
+        .def("setExifTagValue", &Image::setExifTagValue)
         .def("deleteExifTag", &Image::deleteExifTag)
 
         .def("iptcKeys", &Image::iptcKeys)
         .def("getIptcTag", &Image::getIptcTag)
-//        .def("_Image__setIptcTag", &Image::setIptcTag)
-//        .def("_Image__deleteIptcTag", &Image::deleteIptcTag)
+        .def("setIptcTagValues", &Image::setIptcTagValues)
+//        .def("deleteIptcTag", &Image::deleteIptcTag)
 
         .def("xmpKeys", &Image::xmpKeys)
         .def("getXmpTag", &Image::getXmpTag)
diff --git a/src/pyexiv2.py b/src/pyexiv2.py
index ba1734a..b70cc65 100644
--- a/src/pyexiv2.py
+++ b/src/pyexiv2.py
@@ -533,6 +533,7 @@ class IptcTag(MetadataTag):
         Constructor.
         """
         super(IptcTag, self).__init__(key, name, label, description, xtype, values)
+        # FIXME: make values either a tuple (immutable) or a notifying list
         self.values = map(lambda x: IptcTag._convert_to_python(x, xtype), values)
 
     @staticmethod
@@ -658,6 +659,14 @@ class IptcTag(MetadataTag):
 
         raise IptcValueError(value, xtype)
 
+    def to_string(self):
+        """
+        Return a list of string representations of the IPTC tag values suitable
+        to pass to libexiv2 to set the values of the tag.
+        DOCME
+        """
+        return map(lambda x: IptcTag._convert_to_string(x, self.xtype), self.values)
+
     def __str__(self):
         """
         Return a string representation of the IPTC tag.
@@ -1065,7 +1074,7 @@ class ImageMetadata(object):
     def _set_exif_tag(self, tag):
         if type(tag) is not ExifTag:
             raise TypeError('Expecting an ExifTag')
-        self._image.setExifTag(tag.key, tag.to_string())
+        self._image.setExifTagValue(tag.key, tag.to_string())
         self._tags['exif'][tag.key] = tag
         tag.metadata = self
 
@@ -1079,7 +1088,18 @@ class ImageMetadata(object):
             raise KeyError('Cannot set the value of an inexistent tag')
         if type(value) is not str:
             raise TypeError('Expecting a string')
-        self._image.setExifTag(key, value)
+        self._image.setExifTagValue(key, value)
+
+    def _set_iptc_tag(self, tag):
+        if type(tag) is not IptcTag:
+            raise TypeError('Expecting an IptcTag')
+        self._image.setIptcTagValues(tag.key, tag.to_string())
+        self._tags['iptc'][tag.key] = tag
+        tag.metadata = self
+
+    def _set_iptc_tag_values(self, key, values):
+        # TODO
+        raise NotImplementedError()
 
     def _delete_exif_tag(self, key):
         if key not in self.exif_keys:
@@ -1091,6 +1111,10 @@ class ImageMetadata(object):
             # The tag was not cached.
             pass
 
+    def _delete_iptc_tag(self, key):
+        # TODO
+        raise NotImplementedError()
+
 
 class Image(libexiv2python.Image):
 
diff --git a/todo b/todo
index 9a7d91c..89fb3de 100644
--- a/todo
+++ b/todo
@@ -1,5 +1,6 @@
 todo list
 
+- Unit test the C++ wrapper (in python, if possible)
 - Use logging to log, among other things, decoding and type conversion errors
 - The constructor of the Image class should accept a buffered file
 - Tag getters/setters should support unicode strings as well as regular strings (see bug #146313).
@@ -8,6 +9,6 @@ todo list
 - Add a 'doc' builder to the SConstruct to build the module's documentation
 - Export docstrings for the C++ binding
 - Rewrite the exiv2 command-line tool and the test binaries in Python and (Python) scripts to run the same tests that are run to test exiv2
-- Write a complete documentation for the binding and it uses (use other tools, maybe pydoctor)
+- Write a complete documentation for the binding and its uses (use other tools, maybe pydoctor)
 - New architecture and support of XMP metadata (see bug #183337)
 - Consider upgrading the license to GPL v3
diff --git a/unittest/metadata.py b/unittest/metadata.py
index 661a5be..4512350 100644
--- a/unittest/metadata.py
+++ b/unittest/metadata.py
@@ -49,7 +49,7 @@ class ImageMock(object):
     def getExifTag(self, key):
         return self.tags['exif'][key]
 
-    def setExifTag(self, key, value):
+    def setExifTagValue(self, key, value):
         self.tags['exif'][key] = value
 
     def deleteExifTag(self, key):
@@ -64,6 +64,9 @@ class ImageMock(object):
     def getIptcTag(self, key):
         return self.tags['iptc'][key]
 
+    def setIptcTagValues(self, key, values):
+        self.tags['iptc'][key] = values
+
     def xmpKeys(self):
         return self.tags['xmp'].keys()
 
@@ -166,9 +169,8 @@ class TestImageMetadata(unittest.TestCase):
         self._set_exif_tags()
         self.assertEqual(self.metadata._tags['exif'], {})
         # Try to set a tag with wrong type
-        key = 'Exif.Image.Make'
-        value = 'Not an exif tag'
-        self.failUnlessRaises(TypeError, self.metadata._set_exif_tag, key, value)
+        tag = 'Not an exif tag'
+        self.failUnlessRaises(TypeError, self.metadata._set_exif_tag, tag)
         self.assertEqual(self.metadata._tags['exif'], {})
 
     def test_set_exif_tag_create(self):
@@ -297,6 +299,67 @@ class TestImageMetadata(unittest.TestCase):
         key = 'Iptc.Application2.Copyright'
         self.failUnlessRaises(KeyError, self.metadata._get_iptc_tag, key)
 
+    def test_set_iptc_tag_wrong(self):
+        self.metadata.read()
+        self._set_iptc_tags()
+        self.assertEqual(self.metadata._tags['iptc'], {})
+        # Try to set a tag with wrong type
+        tag = 'Not an iptc tag'
+        self.failUnlessRaises(TypeError, self.metadata._set_iptc_tag, tag)
+        self.assertEqual(self.metadata._tags['iptc'], {})
+
+    def test_set_iptc_tag_create(self):
+        self.metadata.read()
+        self._set_iptc_tags()
+        self.assertEqual(self.metadata._tags['iptc'], {})
+        # Create a new tag
+        tag = IptcTag('Iptc.Application2.Writer', 'Writer', 'Writer',
+                      'Identification of the name of the person involved in ' \
+                      'the writing, editing or correcting the object data or ' \
+                      'caption/abstract.', 'String', ['Nobody'])
+        self.assertEqual(tag.metadata, None)
+        self.metadata._set_iptc_tag(tag)
+        self.assertEqual(tag.metadata, self.metadata)
+        self.assertEqual(self.metadata._tags['iptc'], {tag.key: tag})
+        self.assert_(self.metadata._image.tags['iptc'].has_key(tag.key))
+        self.assertEqual(self.metadata._image.tags['iptc'][tag.key],
+                         tag.raw_value)
+
+    def test_set_iptc_tag_overwrite(self):
+        self.metadata.read()
+        self._set_iptc_tags()
+        self.assertEqual(self.metadata._tags['iptc'], {})
+        # Overwrite an existing tag
+        tag = IptcTag('Iptc.Application2.Caption', 'Caption', 'Caption',
+                      'A textual description of the object data.', 'String',
+                      ['A picture.'])
+        self.assertEqual(tag.metadata, None)
+        self.metadata._set_iptc_tag(tag)
+        self.assertEqual(tag.metadata, self.metadata)
+        self.assertEqual(self.metadata._tags['iptc'], {tag.key: tag})
+        self.assert_(self.metadata._image.tags['iptc'].has_key(tag.key))
+        self.assertEqual(self.metadata._image.tags['iptc'][tag.key],
+                         tag.raw_value)
+
+    def test_set_iptc_tag_overwrite_already_cached(self):
+        self.metadata.read()
+        self._set_iptc_tags()
+        self.assertEqual(self.metadata._tags['iptc'], {})
+        # Overwrite an existing tag already cached
+        key = 'Iptc.Application2.Caption'
+        tag = self.metadata._get_iptc_tag(key)
+        self.assertEqual(self.metadata._tags['iptc'][key], tag)
+        new_tag = IptcTag(key, 'Caption', 'Caption',
+                      'A textual description of the object data.', 'String',
+                      ['A picture.'])
+        self.assertEqual(new_tag.metadata, None)
+        self.metadata._set_iptc_tag(new_tag)
+        self.assertEqual(new_tag.metadata, self.metadata)
+        self.assertEqual(self.metadata._tags['iptc'], {key: new_tag})
+        self.assert_(self.metadata._image.tags['iptc'].has_key(key))
+        self.assertEqual(self.metadata._image.tags['iptc'][key],
+                         new_tag.raw_value)
+
     def test_xmp_keys(self):
         self.metadata.read()
         self._set_xmp_tags()