Base: support more image formats

By relying on PIL after our monkey-patched OFS.Image.getImageInfo.

We keep this monkey-patch for now, because it adds supports to svg

See merge request nexedi/erp5!1426

bt5/erp5_base/DocumentTemplateItem/portal_components/document.erp5.Image.py

@@ -31,7 +31,6 @@
 ##############################################################################
 
 import os
-import struct
 import subprocess
 from cStringIO import StringIO
 
@@ -45,9 +44,10 @@ from erp5.component.document.File import File
 from erp5.component.document.Document import Document, ConversionError,\
                      VALID_TEXT_FORMAT_LIST, VALID_TRANSPARENT_IMAGE_FORMAT_LIST,\
                      DEFAULT_DISPLAY_ID_LIST, _MARKER
-from os.path import splitext
+
 from OFS.Image import Image as OFSImage
 from OFS.Image import getImageInfo
+import PIL.Image
 from zLOG import LOG, WARNING
 
 from erp5.component.module.ImageUtil import transformUrlToDataURI
@@ -111,23 +111,26 @@ class Image(TextConvertableMixin, File, OFSImage):
   def _update_image_info(self):
     """
       This method tries to determine the content type of an image and
-      its geometry. It uses currently OFS.Image for this purpose.
-      However, this method is known to be too simplistic.
-
-      TODO:
-      - use image magick or PIL
+      its geometry.
     """
     self.size = len(self.data)
     content_type, width, height = getImageInfo(self.data)
     if not content_type:
-      if self.size >= 30 and self.data[:2] == 'BM':
-        header = struct.unpack('<III', self.data[14:26])
-        if header[0] >= 12:
-          content_type = 'image/x-bmp'
-          width, height = header[1:]
+      try:
+        image = PIL.Image.open(StringIO(str(self.data)))
+      except IOError:
+        width = height = -1
+        content_type = 'application/unknown'
+      else:
+        width, height = image.size
+        content_type = image.get_format_mimetype()
+        # normalize the mimetype using the registry
+        mimetype_list = self.getPortalObject().mimetypes_registry.lookup(content_type)
+        if mimetype_list:
+          content_type = mimetype_list[0].normalized()
     self.height = height
     self.width = width
-    self._setContentType(content_type or 'application/unknown')
+    self._setContentType(content_type)
 
   def _upgradeImage(self):
     """
@@ -303,8 +306,14 @@ class Image(TextConvertableMixin, File, OFSImage):
       kw['image_size'] = image_size
       display = kw.pop('display', None)
       crop = kw.pop('crop', None)
-      mime, image = self._makeDisplayPhoto(crop=crop, **kw)
-      image_data = image.data
+      mime, image_data = self._getContentTypeAndImageData(
+          format=format,
+          quality=quality,
+          resolution=kw.get('resolution'),
+          frame=kw.get('frame'),
+          image_size=image_size,
+          crop=crop,
+      )
       # as image will always be requested through a display not by passing exact
       # pixels we need to restore this way in cache
       if display is not None:
@@ -395,7 +404,7 @@ class Image(TextConvertableMixin, File, OFSImage):
       return StringIO(image)
     raise ConversionError('Image conversion failed (%s).' % err)
 
-  def _getDisplayData(
+  def _getContentTypeAndImageData(
       self,
       format,  # pylint: disable=redefined-builtin
       quality,
@@ -404,7 +413,7 @@ class Image(TextConvertableMixin, File, OFSImage):
       image_size,
       crop,
   ):
-    """Return raw photo data for given display."""
+    """Return the content type and the image data as str or PData."""
     if crop:
       width, height = image_size
     else:
@@ -413,29 +422,23 @@ class Image(TextConvertableMixin, File, OFSImage):
        and quality == self.getDefaultImageQuality(format) and resolution is None and frame is None\
        and not format:
       # No resizing, no conversion, return raw image
-      return self.getData()
-    return self._resize(quality, width, height, format, resolution, frame, crop)
-
-  def _makeDisplayPhoto(
-      self,
-      format=None,  # pylint: disable=redefined-builtin
-      quality=_MARKER,
-      resolution=None,
-      frame=None,
-      image_size=None,
-      crop=False,
-  ):
-    """Create given display."""
-    if quality is _MARKER:
-      quality = self.getDefaultImageQuality(format)
-    width, height = image_size  # pylint: disable=unpacking-non-sequence
-    base, ext = splitext(self.id)
-    id_ = '%s_%s_%s.%s'% (base, width, height, ext,)
-    image = OFSImage(id_, self.getTitle(),
-                     self._getDisplayData(format, quality, resolution,
-                                                            frame, image_size,
-                                                            crop))
-    return image.content_type, aq_base(image)
+      return self.getContentType(), self.getData()
+    image_file = self._resize(quality, width, height, format, resolution, frame, crop)
+    image = OFSImage('', '', image_file)
+    content_type = image.content_type
+    if content_type == 'application/octet-stream':
+      # If OFS Image could not guess content type, try with PIL
+      image_file.seek(0)
+      try:
+        pil_image = PIL.Image.open(image_file)
+      except IOError:
+        pass
+      else:
+        content_type = pil_image.get_format_mimetype()
+        mimetype_list = self.getPortalObject().mimetypes_registry.lookup(content_type)
+        if mimetype_list:
+          content_type = mimetype_list[0].normalized()
+    return content_type, image.data
 
   def _getAspectRatioSize(self, width, height):
     """Return proportional dimensions within desired size."""
@@ -455,10 +458,6 @@ class Image(TextConvertableMixin, File, OFSImage):
       width =  img_width * height / img_height
     return (width, height)
 
-  def _validImage(self):
-    """At least see if it *might* be valid."""
-    return self.getWidth() and self.getHeight() and self.getData() and self.getContentType()
-
   security.declareProtected(Permissions.AccessContentsInformation, 'getSizeFromImageDisplay')
   def getSizeFromImageDisplay(self, image_display):
     """Return the size for this image display,

bt5/erp5_core_test/TestTemplateItem/portal_components/test.erp5.testERP5Base.py

@@ -73,12 +73,6 @@ class TestERP5Base(ERP5TypeTestCase):
   ##  Usefull methods
   ##################################
 
-  def makeImageFileUpload(self, filename):
-    import Products.ERP5.tests
-    return FileUpload(
-            os.path.join(os.path.dirname(Products.ERP5.tests.__file__),
-            'test_data', 'images', filename))
-
   def login_as_auditor(self):
     """Create a new member user with Auditor role, and login
     """
@@ -936,52 +930,6 @@ class TestERP5Base(ERP5TypeTestCase):
     bank_account.setBankCountryCode('bank-country-code')
     self.assertEqual(bank_account.getReference(), 'iban')
 
-  def test_CreateImage(self):
-    # We can add Images inside Persons and Organisation
-    for entity in (self.getPersonModule().newContent(portal_type='Person'),
-        self.getOrganisationModule().newContent(portal_type='Organisation')):
-      image = entity.newContent(portal_type='Embedded File')
-      self.assertEqual([], image.checkConsistency())
-      image.view() # viewing the image does not cause error
-
-  def test_ConvertImage(self):
-    image = self.portal.newContent(portal_type='Image', id='test_image')
-    image.edit(file=self.makeImageFileUpload('erp5_logo.png'))
-    self.assertEqual('image/png', image.getContentType())
-    self.assertEqual((320, 250), (image.getWidth(), image.getHeight()))
-
-    def convert(**kw):
-      image_type, image_data = image.convert('jpg', display='thumbnail', **kw)
-      self.assertEqual('image/jpeg', image_type)
-      thumbnail = self.portal.newContent(temp_object=True, portal_type='Image',
-        id='thumbnail', data=image_data)
-      self.assertEqual(image_type, thumbnail.getContentType())
-      self.assertEqual((128, 100), (thumbnail.getWidth(),
-                                    thumbnail.getHeight()))
-      return thumbnail.getSize()
-    self.assertTrue(convert() < convert(quality=100))
-
-  def test_ConvertImagePdata(self):
-    image = self.portal.newContent(portal_type='Image', id='test_image')
-    image.edit(file=self.makeImageFileUpload('erp5_logo.bmp'))
-    from OFS.Image import Pdata
-    self.assertTrue(isinstance(image.data, Pdata))
-
-    image_type, image_data = image.convert('jpg', display='thumbnail')
-    self.assertEqual('image/jpeg', image_type)
-    # magic
-    self.assertEqual('\xff', image_data[0])
-    self.assertEqual('\xd8', image_data[1])
-
-  def test_ImageSize(self):
-    image = self.portal.newContent(portal_type='Image', id='test_image')
-    image.edit(file=self.makeImageFileUpload('erp5_logo.png'))
-    self.assertEqual(320, image.getWidth())
-    self.assertEqual(250, image.getHeight())
-    image.edit(file=self.makeImageFileUpload('erp5_logo_small.png'))
-    self.assertEqual(160, image.getWidth())
-    self.assertEqual(125, image.getHeight())
-
   def test_Person_getCareerStartDate(self):
     # Person_getCareerStartDate scripts returns the date when an employee
     # started to work for an employer
@@ -1960,3 +1908,110 @@ class Base_getDialogSectionCategoryItemListTest(ERP5TypeTestCase):
             ],
             ['Another Top Level Group', 'group/main_group_2'],
         ])
+
+
+class TestImage(ERP5TypeTestCase):
+  """Tests for images support.
+  """
+  def makeImageFileUpload(self, filename):
+    import Products.ERP5.tests
+    return FileUpload(
+            os.path.join(os.path.dirname(Products.ERP5.tests.__file__),
+            'test_data', 'images', filename))
+
+  def test_CreateImage(self):
+    # We can add Images inside Persons and Organisation
+    for entity in (self.getPersonModule().newContent(portal_type='Person'),
+        self.getOrganisationModule().newContent(portal_type='Organisation')):
+      image = entity.newContent(portal_type='Embedded File')
+      self.assertEqual([], image.checkConsistency())
+      image.view() # viewing the image does not cause error
+
+  def test_ConvertImage(self):
+    image = self.portal.newContent(portal_type='Image', id='test_image')
+    image.edit(file=self.makeImageFileUpload('erp5_logo.png'))
+    self.assertEqual('image/png', image.getContentType())
+    self.assertEqual((320, 250), (image.getWidth(), image.getHeight()))
+
+    def convert(**kw):
+      image_type, image_data = image.convert('jpg', display='thumbnail', **kw)
+      self.assertEqual('image/jpeg', image_type)
+      thumbnail = self.portal.newContent(temp_object=True, portal_type='Image',
+        id='thumbnail', data=image_data)
+      self.assertEqual(image_type, thumbnail.getContentType())
+      self.assertEqual((128, 100), (thumbnail.getWidth(),
+                                    thumbnail.getHeight()))
+      return thumbnail.getSize()
+    self.assertTrue(convert() < convert(quality=100))
+
+  def test_ConvertImagePdata(self):
+    image = self.portal.newContent(portal_type='Image', id='test_image')
+    image.edit(file=self.makeImageFileUpload('erp5_logo.bmp'))
+    from OFS.Image import Pdata
+    self.assertTrue(isinstance(image.data, Pdata))
+
+    image_type, image_data = image.convert('jpg', display='thumbnail')
+    self.assertEqual('image/jpeg', image_type)
+    # magic
+    self.assertEqual('\xff', image_data[0])
+    self.assertEqual('\xd8', image_data[1])
+
+  def test_ImageSize(self):
+    for filename, size in (
+        ('erp5_logo.png', (320, 250)),
+        ('erp5_logo_small.png', (160, 125)),
+        ('erp5_logo.jpg', (320, 250)),
+        ('erp5_logo.bmp', (320, 250)),
+        ('erp5_logo.gif', (320, 250)),
+        ('erp5_logo.tif', (320, 250)),
+        ('empty.png', (0, 0)),
+        ('broken.png', (-1, -1)),
+        ('../broken_html.html', (-1, -1)),
+      ):
+      image = self.portal.newContent(portal_type='Image', id=self.id())
+      image.edit(file=self.makeImageFileUpload(filename))
+      self.assertEqual(
+          (image.getWidth(), image.getHeight()),
+          size,
+          (filename, (image.getWidth(), image.getHeight()), size))
+      self.portal.manage_delObjects([self.id()])
+
+  def test_ImageContentTypeFromData(self):
+    for filename, content_type in (
+        ('erp5_logo.png', 'image/png'),
+        ('erp5_logo_small.png', 'image/png'),
+        ('erp5_logo.jpg', 'image/jpeg'),
+        ('erp5_logo.bmp', 'image/x-ms-bmp'),
+        ('erp5_logo.gif', 'image/gif'),
+        ('erp5_logo.tif', 'image/tiff'),
+        ('broken.png', 'application/unknown'),
+        ('empty.png', 'application/unknown'),
+        ('../broken_html.html', 'application/unknown'),
+      ):
+      image = self.portal.newContent(portal_type='Image', id=self.id())
+      image.edit(data=self.makeImageFileUpload(filename).read())
+      self.assertEqual(
+          image.getContentType(),
+          content_type,
+          (filename, image.getContentType(), content_type))
+      self.portal.manage_delObjects([self.id()])
+
+  def test_ImageContentTypeFromFile(self):
+    # with file= argument the filename also play a role in the type detection
+    for filename, content_type in (
+        ('erp5_logo.png', 'image/png'),
+        ('erp5_logo_small.png', 'image/png'),
+        ('erp5_logo.jpg', 'image/jpeg'),
+        ('erp5_logo.bmp', 'image/x-ms-bmp'),
+        ('erp5_logo.gif', 'image/gif'),
+        ('erp5_logo.tif', 'image/tiff'),
+        ('broken.png', 'image/png'),
+        ('empty.png', 'application/unknown'),
+      ):
+      image = self.portal.newContent(portal_type='Image', id=self.id())
+      image.edit(file=self.makeImageFileUpload(filename))
+      self.assertEqual(
+          image.getContentType(),
+          content_type,
+          (filename, image.getContentType(), content_type))
+      self.portal.manage_delObjects([self.id()])

bt5/erp5_dms/TestTemplateItem/portal_components/test.erp5.testDms.py

@@ -1249,16 +1249,51 @@ class TestDocument(TestDocumentMixin):
     self.assert_('I use reference to look up TEST' in
                  document.SearchableText())
 
-  def test_PDFToImage(self):
+  def test_PDFToPng(self):
     upload_file = makeFileUpload('REF-en-001.pdf')
     document = self.portal.portal_contributions.newContent(file=upload_file)
     self.assertEqual('PDF', document.getPortalType())
 
-    _, image_data = document.convert(format='png',
+    mime, image_data = document.convert(format='png',
                                      frame=0,
                                      display='thumbnail')
+    self.assertEqual(mime, 'image/png')
     # it's a valid PNG
-    self.assertEqual('PNG', image_data[1:4])
+    self.assertEqual(image_data[1:4], 'PNG')
+
+  def test_PDFToJpg(self):
+    upload_file = makeFileUpload('REF-en-001.pdf')
+    document = self.portal.portal_contributions.newContent(file=upload_file)
+    self.assertEqual('PDF', document.getPortalType())
+
+    mime, image_data = document.convert(format='jpg',
+                                     frame=0,
+                                     display='thumbnail')
+    self.assertEqual(mime, 'image/jpeg')
+    self.assertEqual(image_data[6:10], 'JFIF')
+
+  def test_PDFToGif(self):
+    upload_file = makeFileUpload('REF-en-001.pdf')
+    document = self.portal.portal_contributions.newContent(file=upload_file)
+    self.assertEqual('PDF', document.getPortalType())
+
+    mime, image_data = document.convert(format='gif',
+                                     frame=0,
+                                     display='thumbnail')
+    self.assertEqual(mime, 'image/gif')
+    self.assertEqual(image_data[0:4], 'GIF8')
+
+  def test_PDFToTiff(self):
+    upload_file = makeFileUpload('REF-en-001.pdf')
+    document = self.portal.portal_contributions.newContent(file=upload_file)
+    self.assertEqual('PDF', document.getPortalType())
+
+    mime, image_data = document.convert(format='tiff',
+                                     frame=0,
+                                     display='thumbnail')
+    self.assertEqual(mime, 'image/tiff')
+    self.assertIn(image_data[0:2], ('II', 'MM'))
+
 
   def test_PDF_content_information(self):
     upload_file = makeFileUpload('REF-en-001.pdf')
@@ -2935,12 +2970,13 @@ class TestDocumentPerformance(TestDocumentMixin):
       "Conversion took %s seconds and it is not less them 100.0 seconds" % \
         req_time)
 
+
 def test_suite():
   suite = unittest.TestSuite()
   suite.addTest(unittest.makeSuite(TestDocument))
   suite.addTest(unittest.makeSuite(TestDocumentWithSecurity))
   suite.addTest(unittest.makeSuite(TestDocumentPerformance))
+  # Run erp5_base's TestImage with dms installed (because dms has specific interactions)
+  from erp5.component.test.testERP5Base import TestImage
+  suite.addTest(unittest.makeSuite(TestImage))
   return suite
-
-
- # vim: syntax=python shiftwidth=2
\ No newline at end of file

bt5/erp5_dms/bt/test_dependency_list

@@ -2,3 +2,4 @@ erp5_full_text_mroonga_catalog
 erp5_core_proxy_field_legacy
 erp5_ingestion_mysql_innodb_catalog
 erp5_ingestion_test
+erp5_core_test
\ No newline at end of file

product/ERP5/tests/test_data/images/broken.png

+broken