Migrate issue and response attachments to blobs.

There is an upgrade step for this.
Fixes issue #32.

CHANGES.rst

@@ -2,10 +2,13 @@ Changelog for Poi
 =================
 
 
-2.2.11 (unreleased)
--------------------
+2.3 (unreleased)
+----------------
 
-- Nothing changed yet.
+- Use blob file for attachments in issues and responses.  This comes
+  with an upgrade step to migrate them.
+  Fixes `issue #32 <https://github.com/collective/Products.Poi/issues/32>`_.
+  [maurits]
 
 
 2.2.10 (2016-12-23)

Products/Poi/adapters.py

@@ -1,20 +1,23 @@
-import logging
-
 from AccessControl import getSecurityManager
-from DateTime import DateTime
 from collective.watcherlist.watchers import WatcherList
+from DateTime import DateTime
 from persistent import Persistent
 from persistent.list import PersistentList
+from plone.namedfile.interfaces import NotStorable
+from plone.namedfile.storages import MAXCHUNKSIZE
+from Products.Poi.interfaces import IIssue
 from zope.annotation.interfaces import IAnnotations
-from zope.lifecycleevent import ObjectAddedEvent
-from zope.lifecycleevent import ObjectRemovedEvent
 from zope.component import adapts
 from zope.event import notify
 from zope.interface import Attribute
-from zope.interface import Interface
 from zope.interface import implements
+from zope.interface import Interface
+from zope.lifecycleevent import ObjectAddedEvent
+from zope.lifecycleevent import ObjectRemovedEvent
+from ZPublisher.HTTPRequest import FileUpload
+
+import logging
 
-from Products.Poi.interfaces import IIssue
 
 logger = logging.getLogger('Products.Poi.adapters')
 
@@ -201,3 +204,23 @@ def __init__(self, context):
 
     def export(self, export_context, subdir, root=False):
         return
+
+
+class FileUploadStorable(object):
+    # Adapted from plone.namedfile.storages.FileUploadStorable.  That one only
+    # handles data from zope.publisher.browser.FileUpload.  Poi uses a hand
+    # crafted, non-z3c form, so we just get an old-style FileUpload, which
+    # means we need our own storage adapter.
+
+    def store(self, data, blob):
+        if not isinstance(data, FileUpload):
+            raise NotStorable('Could not store data (not of "FileUpload").')
+
+        data.seek(0)
+
+        fp = blob.open('w')
+        block = data.read(MAXCHUNKSIZE)
+        while block:
+            fp.write(block)
+            block = data.read(MAXCHUNKSIZE)
+        fp.close()

Products/Poi/browser/notifications.py

@@ -189,7 +189,7 @@ def options(self):
             name = translate(name, 'Poi', context=self.request)
             changes.append(dict(name=name, before=before, after=after))
         if response.attachment:
-            attachment_id = response.attachment.getId()
+            attachment_id = getattr(response.attachment, 'filename', u'')
         else:
             attachment_id = u''
 

Products/Poi/browser/response.py

@@ -2,9 +2,9 @@
 
 from AccessControl import Unauthorized
 from Acquisition import aq_inner
-from OFS.Image import File
+from plone.namedfile import NamedBlobFile
+from plone.namedfile.browser import Download as BlobDownload
 from Products.Archetypes.atapi import DisplayList
-from Products.Archetypes.utils import contentDispositionHeader
 from Products.CMFCore.utils import getToolByName
 from Products.CMFPlone import PloneMessageFactory as PMF
 from Products.CMFPlone.utils import safe_unicode
@@ -150,7 +150,12 @@ def attachment_info(self, id):
         mtr = getToolByName(context, 'mimetypes_registry', None)
         if mtr is None:
             icon = context.getIcon()
-        lookup = mtr.lookup(attachment.content_type)
+        content_type = getattr(
+            attachment, 'contentType', 'application/octet-stream')
+        size = getattr(attachment, 'get_size', 0)
+        if callable(size):
+            size = size()
+        lookup = mtr.lookup(content_type)
         if lookup:
             mti = lookup[0]
             try:
@@ -160,13 +165,13 @@ def attachment_info(self, id):
                 pass
         if icon is None:
             icon = context.getIcon()
-        filename = getattr(attachment, 'filename', attachment.getId())
+        filename = getattr(attachment, 'filename', '')
         info = dict(
             icon=self.portal_url + '/' + icon,
             url=context.absolute_url() +
             '/@@poi_response_attachment?response_id=' + str(id),
-            content_type=attachment.content_type,
-            size=pretty_size(attachment.size),
+            content_type=content_type,
+            size=pretty_size(size),
             filename=filename,
         )
         return info
@@ -441,7 +446,9 @@ def __call__(self):
         attachment = form.get('attachment')
         if attachment:
             # File(id, title, file)
-            data = File(attachment.filename, attachment.filename, attachment)
+            # data = File(attachment.filename, attachment.filename, attachment)
+            data = NamedBlobFile(
+                attachment, filename=safe_unicode(attachment.filename))
             new_response.attachment = data
             issue_has_changed = True
 
@@ -564,7 +571,7 @@ def __call__(self):
         self.request.response.redirect(context.absolute_url())
 
 
-class Download(Base):
+class Download(Base, BlobDownload):
     """Download the attachment of a response.
     """
 
@@ -585,17 +592,19 @@ def __call__(self):
         if file is None:
             request.response.redirect(context.absolute_url())
 
-        # From now on file exists.
-        # Code mostly taken from Archetypes/Field.py:FileField.download
-        filename = getattr(file, 'filename', file.getId())
+        # From now on we know that an attachment exists.
+        # Set attributes on self, so BlobDownload can do its work.
+        self.fieldname = 'attachment'
+        self.file = file
+        filename = getattr(file, 'filename', self.fieldname)
         if filename is not None:
             if FILE_NORMALIZER:
                 filename = IUserPreferredFileNameNormalizer(request).normalize(
                     safe_unicode(filename, context.getCharset()))
             else:
                 filename = safe_unicode(filename, context.getCharset())
-            header_value = contentDispositionHeader(
-                disposition='attachment',
-                filename=filename)
-            request.response.setHeader("Content-disposition", header_value)
-        return file.index_html(request, request.response)
+        self.filename = filename
+        return BlobDownload.__call__(self)
+
+    def _getFile(self):
+        return self.file

Products/Poi/configure.zcml

@@ -84,6 +84,12 @@
       for="Products.Poi.interfaces.IIssue"
       />
 
+  <utility
+      name="ZPublisher.HTTPRequest.FileUpload"
+      provides="plone.namedfile.interfaces.IStorage"
+      factory=".adapters.FileUploadStorable"
+      />
+
   <!-- Avoid deprecation warnings for manage_afterAdd and friends. -->
   <five:deprecatedManageAddDelete
       class=".content.PoiTracker.PoiTracker" />

Products/Poi/content/PoiIssue.py

@@ -54,6 +54,7 @@
 from Products.CMFPlone.utils import getSiteEncoding
 from Products.CMFPlone.utils import safe_unicode
 from collective.watcherlist.utils import get_member_email
+from plone.app.blob.field import BlobField
 from plone.memoize import instance
 from zope.interface import implements
 import transaction
@@ -158,7 +159,7 @@
         searchable=True
     ),
 
-    FileField(
+    BlobField(
         name='attachment',
         widget=FileWidget(
             label=_(u'Poi_label_attachment',

Products/Poi/migration.py

@@ -1,13 +1,15 @@
-import logging
-
-from ZODB.POSException import ConflictError
+from collective.watcherlist.interfaces import IWatcherList
+from plone.namedfile import NamedBlobFile
 from Products.CMFCore.utils import getToolByName
 from Products.CMFFormController.FormAction import FormActionKey
-from collective.watcherlist.interfaces import IWatcherList
+from Products.CMFPlone.utils import safe_unicode
+from Products.Poi.adapters import IResponseContainer
+from ZODB.POSException import ConflictError
 from zope.annotation.interfaces import IAnnotations
+
+import logging
 import transaction
 
-from Products.Poi.adapters import IResponseContainer
 
 logger = logging.getLogger("Poi")
 PROFILE_ID = 'profile-Products.Poi:default'
@@ -236,3 +238,58 @@ def migrate_tracker_watchers(context):
 def recook_resources(context):
     context.portal_javascripts.cookResources()
     context.portal_css.cookResources()
+
+
+def migrate_response_attachments_to_blobstorage(context):
+    logger.info('Migrating response attachments to blob storage.')
+    catalog = getToolByName(context, 'portal_catalog')
+    already_migrated = 0
+    migrated = 0
+    for brain in catalog.unrestrictedSearchResults(portal_type='PoiIssue'):
+        path = brain.getPath()
+        try:
+            issue = brain.getObject()
+        except (AttributeError, ValueError, TypeError):
+            logger.warn('Error getting object from catalog for path %s', path)
+            continue
+        folder = IResponseContainer(issue)
+        for id, response in enumerate(folder):
+            if response is None:
+                # Has been removed.
+                continue
+            attachment = response.attachment
+            if attachment is None:
+                continue
+            if isinstance(attachment, NamedBlobFile):
+                # Already migrated
+                logger.debug('Response %d already migrated, at %s.', id, path)
+                already_migrated += 1
+                continue
+            content_type = getattr(attachment, 'content_type', '')
+            filename = getattr(attachment, 'filename', '')
+            if not filename and hasattr(attachment, 'getId'):
+                filename = attachment.getId()
+            data = NamedBlobFile(
+                attachment.data,
+                contentType=content_type,
+                filename=safe_unicode(filename))
+            response.attachment = data
+            logger.debug('Response %d migrated, at %s.', id, path)
+            migrated += 1
+
+    logger.info('Migrated %d response attachments to blobs. '
+                '%d already migrated.', migrated, already_migrated)
+
+
+def migrate_issue_attachments_to_blobstorage(context):
+    from plone.app.blob.migrations import migrate
+    logger.info('Migrating to blob attachments for issues. '
+                'This can take a long time...')
+    # Technically, the plone.app.blob migration says it is only for non blob
+    # fields that are still in the schema but are overridden using
+    # archetypes.schemaextender with a blob field with the same name.  But it
+    # seems to go fine for Issues where we have simply changed the schema
+    # directly.  The getters of a BlobFileField and normal FileField don't
+    # differ that much.
+    migrate(context, 'PoiIssue')
+    logger.info("Done migrating to blob attachment for issues.")

Products/Poi/profiles.zcml

@@ -127,5 +127,16 @@
       handler="Products.Poi.migration.recook_resources"
       profile="Products.Poi:default" />
 
+  <gs:upgradeSteps
+      source="2007"
+      destination="2008"
+      profile="Products.Poi:default">
+    <gs:upgradeStep
+        title="Migrate attachments of issues to blob storage"
+        handler="Products.Poi.migration.migrate_issue_attachments_to_blobstorage" />
+    <gs:upgradeStep
+        title="Migrate attachments of responses to blob storage"
+        handler="Products.Poi.migration.migrate_response_attachments_to_blobstorage" />
+  </gs:upgradeSteps>
 
 </configure>

Products/Poi/profiles/default/metadata.xml

@@ -1,6 +1,6 @@
 <?xml version="1.0"?>
 <metadata>
-  <version>2007</version>
+  <version>2008</version>
   <dependencies>
     <dependency>profile-Products.AddRemoveWidget:default</dependency>
     <dependency>profile-Products.DataGridField:default</dependency>

Products/Poi/skins/Poi/poi_issue_view.pt

@@ -194,7 +194,7 @@
       <dd tal:content="structure steps" />
     </dl>
 
-    <div class="issue-attachment" tal:condition="python:here.getAttachment().get_size() &gt; 0">
+    <div class="issue-attachment" tal:condition="python:context.getAttachment()">
       <div class="issue-attachment-label" i18n:translate="poi_label_attachment">Attached:</div>
       <metal:attachment use-macro="python:here.widget('attachment')"/>
     </div>

setup.py

@@ -11,7 +11,7 @@
 long_description = readme + "\n\n" + history
 
 setup(name='Products.Poi',
-      version='2.2.11.dev0',
+      version='2.3.dev0',
       description="Poi: A friendly issue tracker",
       long_description=long_description,
       # Get more strings from
@@ -43,6 +43,8 @@
           'Products.AddRemoveWidget>=1.4.2',
           'Products.DataGridField>=1.9.2',
           'collective.watcherlist>=0.2',
+          'plone.app.blob',
+          'plone.namedfile',
       ],
       extras_require={
           'test': [