Changeset 7120 for main

Timestamp:

16 Nov 2011, 18:05:03 (13 years ago)

Author:

uli

Message:

Make imagestorage sensible for different filename extensions per upload doc.

Location:

main/waeup.sirp/trunk/src/waeup/sirp

Files:

• imagestorage.py (modified) (10 diffs)
• tests/test_imagestorage.py (modified) (6 diffs)

main/waeup.sirp/trunk/src/waeup/sirp/imagestorage.py

@@ -20 +20 @@
 ## Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 ##
-"""A storage for image files.
+"""A storage for image (and other) files.
 
 A few words about storing files with ``waeup.sirp``. The need for this
@@ -84 +84 @@
 Later, we can get the file back like this:
 
-  >>> store.getFile('myfile.txt')
+  >>> store.getFile('myfile')
   <open file ...>
+
+Please note, that we ask for ``myfile`` instead of ``myfile.jpg`` as
+the file id should not make a difference for different filename
+extensions. The file id for ``sample.jpg`` thus could simply be
+``sample``.
 
 What we get back is a file or file-like object already opened for
 reading:
 
-  >>> store.getFile('myfile.txt').read()
+  >>> store.getFile('myfile').read()
   'some file content'
 
@@ -106 +111 @@
 this::
 
-  __<MARKER-STRING>__real-filename.jpg
+  __<MARKER-STRING>__real-filename
 
 Please note the double underscores before and after the marker
@@ -128 +133 @@
 case. For example if the file id would be
 
-  ``__IMG_USER__manfred.jpg``
+  ``__IMG_USER__manfred``
 
 then the looked up utility should be registered under name
@@ -137 +142 @@
 such utility can be found, a default handler is used instead
 (see :class:`DefaultFileStoreHandler`).
+
+**About File IDs and Filenames**
+
+In the waeup.sirp package we want to store documents like CVs,
+photographs, and similar. Each of this documents might come into the
+system with different filename extensions. This could be a problem as
+the browser components might have to set different response headers
+for different filetypes and we nevertheless want to make sure that
+only one file is stored per document. For instance we don't want
+``passport.jpg`` *and* ``passport.png`` but only one of them.
+
+The default components like :class:`DefaultFileStoreHandler` take care
+of this by searching the filesystem for already existing files with
+same file id and eventually removing them.
+
+Therefore file ids should never include filename extensions (except if
+you only support exactly one filename extension for a certain
+document). The only part where you should add an extension (and it is
+important to do so) is when creating new files: when a file was
+uploaded you can pass in the filename (including the filename
+extension) and the file stored in external file store will (most
+probably) have a different name but the same extension as the original
+file.
+
+When looking for the file, you however only have to give the file id
+and the handlers should find the right file for you, regardless of the
+filename extension it has.
 
 **Context Adapters: Knowing Your Family**
@@ -148 +180 @@
 filename. You then would create filenames of a format like this::
 
-  __<MARKER-STRING>__applicant0001.jpg
+  __<MARKER-STRING>__applicant0001
 
 where ``applicant0001`` would tell exactly which applicant you can see
@@ -182 +214 @@
 
 """
+import glob
 import grok
 import os
@@ -401 +434 @@
         """Store a file.
         """
-        file_id = filename
         root = self.root # Calls to self.root are expensive
-        marker, filename, base, ext = self.extractMarker(file_id)
+        file_id = os.path.splitext(filename)[0]
+        marker, filename, base, ext = self.extractMarker(filename)
         handler = queryUtility(IFileStoreHandler, name=marker,
                                default=DefaultFileStoreHandler())
         f, path, file_obj = handler.createFile(
-            self, root, file_id, filename, f)
+            self, root, filename, file_id, f)
         dirname = os.path.dirname(path)
         if not os.path.exists(dirname):
@@ -480 +513 @@
     grok.implements(IFileStoreHandler)
 
+    def _searchInPath(self, path):
+        """Get complete path of any existing file starting with `path`.
+
+        If no such file can be found, return input path.
+
+        If multiple such files exist, return the first one.
+
+        **Example:**
+
+        Looking for a `path`::
+
+          '/tmp/myfile'
+
+        will find any file like ``'/tmp/myfile.txt'``,
+        ``'/tmp/myfile.jpg'`` and so on, if it exists.
+        """
+        result = path
+        if os.path.isdir(os.path.dirname(path)):
+            file_iter = glob.iglob('%s*' % (path,))
+            try:
+                result = file_iter.next()
+            except StopIteration:
+                pass
+        return result
+
     def pathFromFileID(self, store, root, file_id):
-        """Return the root path of external file store appended by file id.
-        """
-        return os.path.join(root, file_id)
+        """Return a path for getting/storing a file with given file id.
+
+        If there is already a file stored for the given file id, the
+        path to this file is returned.
+
+        If no such file exists yet (or the the only file existing has
+        no filename extension at all) a path to store the file but
+        without any filename extension is returned.
+        """
+        path = os.path.join(root, file_id)
+        return self._searchInPath(path)
 
     def createFile(self, store, root, filename, file_id, f):
@@ -503 +569 @@
         file itself, it should leave that task to the calling file
         store.
-        """
+
+        This method does, however, remove any existing files stored
+        under the given file id.
+        """
+        ext = os.path.splitext(filename)[1]
         path = self.pathFromFileID(store, root, file_id)
-        return f, path, HurryFile(filename, file_id)
+        base, old_ext = os.path.splitext(path)
+        if old_ext != ext:
+            if os.path.exists(path):
+                os.unlink(path)
+            path = base + ext
+        return f, path, HurryFile(filename, file_id + ext)

main/waeup.sirp/trunk/src/waeup/sirp/tests/test_imagestorage.py

@@ -104 +104 @@
         return
 
+    def test_create_file_w_ext(self):
+        # We can store files with filename extension
+        storage = ExtFileStore(root=self.workdir)
+        dummy_file = StringIO('sample file')
+        image_file = storage.createFile('mysample.txt', dummy_file)
+        self.assertTrue('mysample.txt' in os.listdir(storage.root))
+        self.assertEqual('mysample.txt', image_file.data)
+        return
+
     def test_get_file(self):
         # We can get files after having them stored
@@ -111 +120 @@
         result = storage.getFile(image_file.data)
         self.assertEqual(result.read(), 'sample file')
+        return
+
+    def test_get_file_w_ext(self):
+        # We can get files with filename extension after having them
+        # stored
+        storage = ExtFileStore(root=self.workdir)
+        dummy_file = StringIO('sample file')
+        image_file = storage.createFile('mysample.txt', dummy_file)
+        result = storage.getFile('mysample')
+        self.assertEqual(result.read(), 'sample file')
+        return
+
+    def test_replace_file_w_new_ext(self):
+        # when we store a file with the same file_id but different
+        # filename extension, the old file will be deleted
+        storage = ExtFileStore(root=self.workdir)
+        dummy_file = StringIO('sample_file')
+        image_file = storage.createFile('mysample.jpg', dummy_file)
+        file1_path = storage.getFile('mysample').name
+        new_file = storage.createFile(
+            'mysample.png', StringIO('new file'))
+        file2 = storage.getFile('mysample')
+        self.assertEqual(file2.name[-12:], 'mysample.png')
+        self.assertEqual(file2.read(), 'new file')
+        # the old file was deleted
+        self.assertFalse(os.path.exists(file1_path))
         return
 
@@ -147 +182 @@
         return os.path.join(root, file_id[12:])
 
-    def createFile(self, store, root, file_id, filename, f):
-        path = self.pathFromFileID(store, root, file_id)
-        return f, path, HurryFile(filename, file_id)
+    def createFile(self, store, root, filename, file_id, f):
+        ext = os.path.splitext(filename)[1]
+        path = self.pathFromFileID(store, root, file_id) + ext
+        return f, path, HurryFile(filename, file_id + ext)
 
 class CustomContext(object):
@@ -162 +198 @@
         # the `attr` parameter, a simple string.
         if attr=='img':
-            return '__mymarker__mysample.jpg'
+            return '__mymarker__mysample_img.jpg'
         elif attr=='doc':
-            return '__mymarker__mysample.doc'
+            return '__mymarker__mysample_doc.doc'
         return '__mymarker__mysample.txt'
 
@@ -246 +282 @@
         return
 
+    def test_customized_handler_create_file_w_ext(self):
+        # when we create a file of img type, the filename ext is taken
+        # from input file.
+        fs = ExtFileStore()
+        result = fs.createFile(
+            '__MYMARKER__sample_img.png', StringIO('sample text'))
+        self.assertEqual(result.data, '__MYMARKER__sample_img.png')
+        self.assertTrue('sample_img.png' in os.listdir(fs.root))
+        return
+
     def test_customized_handler_get_file(self):
         # we consider registered filename handlers when asking for
@@ -279 +325 @@
         # each file has a different file id
         self.assertEqual(file_id1, '__mymarker__mysample.txt')
-        self.assertEqual(file_id2, '__mymarker__mysample.jpg')
-        self.assertEqual(file_id3, '__mymarker__mysample.doc')
+        self.assertEqual(file_id2, '__mymarker__mysample_img.jpg')
+        self.assertEqual(file_id3, '__mymarker__mysample_doc.doc')
         # each file has different content
         self.assertEqual(result1.read(), 'my sample 1')