Fixed #7614: the quickening has come, and there now is only one Uploa…

…dedFile. On top of that, UploadedFile's interface has been improved:

  * The API now more closely matches a proper file API. This unfortunately means a few backwards-incompatible renamings; see BackwardsIncompatibleChanges. This refs #7593.
  * While we were at it, renamed chunk() to chunks() to clarify that it's an iterator.
  * Temporary uploaded files now property use the tempfile library behind the scenes which should ensure better cleanup of tempfiles (refs #7593 again).

Thanks to Mike Axiak for the bulk of this patch.


git-svn-id: http://code.djangoproject.com/svn/django/trunk@7859 bcc190cf-cafb-0310-a4f2-bffc1f526a37

django/core/files/uploadedfile.py

@@ -3,12 +3,40 @@
 """
 
 import os
+import tempfile
+import warnings
 try:
     from cStringIO import StringIO
 except ImportError:
     from StringIO import StringIO
 
-__all__ = ('UploadedFile', 'TemporaryUploadedFile', 'InMemoryUploadedFile')
+from django.conf import settings
+
+__all__ = ('UploadedFile', 'TemporaryUploadedFile', 'InMemoryUploadedFile', 'SimpleUploadedFile')
+
+# Because we fooled around with it a bunch, UploadedFile has a bunch
+# of deprecated properties. This little shortcut helps define 'em
+# without too much code duplication.
+def deprecated_property(old, new, readonly=False):
+    def issue_warning():
+        warnings.warn(
+            message = "UploadedFile.%s is deprecated; use UploadedFile.%s instead." % (old, new),
+            category = DeprecationWarning,
+            stacklevel = 3
+        )
+
+    def getter(self):
+        issue_warning()
+        return getattr(self, new)
+
+    def setter(self, value):
+        issue_warning()
+        setattr(self, new, value)
+
+    if readonly:
+        return property(getter)
+    else:
+        return property(getter, setter)
 
 class UploadedFile(object):
     """
@@ -20,34 +48,34 @@ class UploadedFile(object):
     """
     DEFAULT_CHUNK_SIZE = 64 * 2**10
 
-    def __init__(self, file_name=None, content_type=None, file_size=None, charset=None):
-        self.file_name = file_name
-        self.file_size = file_size
+    def __init__(self, name=None, content_type=None, size=None, charset=None):
+        self.name = name
+        self.size = size
         self.content_type = content_type
         self.charset = charset
 
     def __repr__(self):
-        return "<%s: %s (%s)>" % (self.__class__.__name__, self.file_name, self.content_type)
+        return "<%s: %s (%s)>" % (self.__class__.__name__, self.name, self.content_type)
 
-    def _set_file_name(self, name):
+    def _get_name(self):
+        return self._name
+
+    def _set_name(self, name):
         # Sanitize the file name so that it can't be dangerous.
         if name is not None:
             # Just use the basename of the file -- anything else is dangerous.
             name = os.path.basename(name)
-            
+
             # File names longer than 255 characters can cause problems on older OSes.
             if len(name) > 255:
                 name, ext = os.path.splitext(name)
                 name = name[:255 - len(ext)] + ext
-
-        self._file_name = name
-
-    def _get_file_name(self):
-        return self._file_name
-
-    file_name = property(_get_file_name, _set_file_name)
 
-    def chunk(self, chunk_size=None):
+        self._name = name
+
+    name = property(_get_name, _set_name)
+
+    def chunks(self, chunk_size=None):
         """
         Read the file and yield chucks of ``chunk_size`` bytes (defaults to
         ``UploadedFile.DEFAULT_CHUNK_SIZE``).
@@ -58,12 +86,18 @@ def chunk(self, chunk_size=None):
         if hasattr(self, 'seek'):
             self.seek(0)
         # Assume the pointer is at zero...
-        counter = self.file_size
+        counter = self.size
 
         while counter > 0:
             yield self.read(chunk_size)
             counter -= chunk_size
 
+    # Deprecated properties
+    file_name = deprecated_property(old="file_name", new="name")
+    file_size = deprecated_property(old="file_size", new="size")
+    data = deprecated_property(old="data", new="read", readonly=True)
+    chunk = deprecated_property(old="chunk", new="chunks", readonly=True)
+
     def multiple_chunks(self, chunk_size=None):
         """
         Returns ``True`` if you can expect multiple chunks.
@@ -74,9 +108,9 @@ def multiple_chunks(self, chunk_size=None):
         """
         if not chunk_size:
             chunk_size = UploadedFile.DEFAULT_CHUNK_SIZE
-        return self.file_size < chunk_size
+        return self.size > chunk_size
 
-    # Abstract methods; subclasses *must* default read() and probably should
+    # Abstract methods; subclasses *must* define read() and probably should
     # define open/close.
     def read(self, num_bytes=None):
         raise NotImplementedError()
@@ -87,23 +121,49 @@ def open(self):
     def close(self):
         pass
 
+    def xreadlines(self):
+        return self
+
+    def readlines(self):
+        return list(self.xreadlines())
+
+    def __iter__(self):
+        # Iterate over this file-like object by newlines
+        buffer_ = None
+        for chunk in self.chunks():
+            chunk_buffer = StringIO(chunk)
+
+            for line in chunk_buffer:
+                if buffer_:
+                    line = buffer_ + line
+                    buffer_ = None
+
+                # If this is the end of a line, yield
+                # otherwise, wait for the next round
+                if line[-1] in ('\n', '\r'):
+                    yield line
+                else:
+                    buffer_ = line
+
+        if buffer_ is not None:
+            yield buffer_
+
     # Backwards-compatible support for uploaded-files-as-dictionaries.
     def __getitem__(self, key):
-        import warnings
         warnings.warn(
             message = "The dictionary access of uploaded file objects is deprecated. Use the new object interface instead.",
             category = DeprecationWarning,
             stacklevel = 2
         )
         backwards_translate = {
-            'filename': 'file_name',
+            'filename': 'name',
             'content-type': 'content_type',
-            }
+        }
 
         if key == 'content':
             return self.read()
         elif key == 'filename':
-            return self.file_name
+            return self.name
         elif key == 'content-type':
             return self.content_type
         else:
@@ -113,34 +173,36 @@ class TemporaryUploadedFile(UploadedFile):
     """
     A file uploaded to a temporary location (i.e. stream-to-disk).
     """
-
-    def __init__(self, file, file_name, content_type, file_size, charset):
-        super(TemporaryUploadedFile, self).__init__(file_name, content_type, file_size, charset)
-        self.file = file
-        self.path = file.name
-        self.file.seek(0)
+    def __init__(self, name, content_type, size, charset):
+        super(TemporaryUploadedFile, self).__init__(name, content_type, size, charset)
+        if settings.FILE_UPLOAD_TEMP_DIR:
+            self._file = tempfile.NamedTemporaryFile(suffix='.upload', dir=settings.FILE_UPLOAD_TEMP_DIR)
+        else:
+            self._file = tempfile.NamedTemporaryFile(suffix='.upload')
 
     def temporary_file_path(self):
         """
         Returns the full path of this file.
         """
-        return self.path
-
-    def read(self, *args, **kwargs):
-        return self.file.read(*args, **kwargs)
-
-    def open(self):
-        self.seek(0)
-
-    def seek(self, *args, **kwargs):
-        self.file.seek(*args, **kwargs)
+        return self.name
+
+    # Most methods on this object get proxied to NamedTemporaryFile.
+    # We can't directly subclass because NamedTemporaryFile is actually a
+    # factory function
+    def read(self, *args):          return self._file.read(*args)
+    def seek(self, offset):         return self._file.seek(offset)
+    def write(self, s):             return self._file.write(s)
+    def close(self):                return self._file.close()
+    def __iter__(self):             return iter(self._file)
+    def readlines(self, size=None): return self._file.readlines(size)
+    def xreadlines(self):           return self._file.xreadlines()
 
 class InMemoryUploadedFile(UploadedFile):
     """
     A file uploaded into memory (i.e. stream-to-memory).
     """
-    def __init__(self, file, field_name, file_name, content_type, file_size, charset):
-        super(InMemoryUploadedFile, self).__init__(file_name, content_type, file_size, charset)
+    def __init__(self, file, field_name, name, content_type, size, charset):
+        super(InMemoryUploadedFile, self).__init__(name, content_type, size, charset)
         self.file = file
         self.field_name = field_name
         self.file.seek(0)
@@ -154,7 +216,7 @@ def open(self):
     def read(self, *args, **kwargs):
         return self.file.read(*args, **kwargs)
 
-    def chunk(self, chunk_size=None):
+    def chunks(self, chunk_size=None):
         self.file.seek(0)
         yield self.read()
 
@@ -168,9 +230,9 @@ class SimpleUploadedFile(InMemoryUploadedFile):
     """
     def __init__(self, name, content, content_type='text/plain'):
         self.file = StringIO(content or '')
-        self.file_name = name
+        self.name = name
         self.field_name = None
-        self.file_size = len(content or '')
+        self.size = len(content or '')
         self.content_type = content_type
         self.charset = None
         self.file.seek(0)

django/core/files/uploadhandler.py

@@ -132,21 +132,15 @@ def new_file(self, file_name, *args, **kwargs):
         Create the file object to append to as data is coming in.
         """
         super(TemporaryFileUploadHandler, self).new_file(file_name, *args, **kwargs)
-        self.file = TemporaryFile(settings.FILE_UPLOAD_TEMP_DIR)
-        self.write = self.file.write
+        self.file = TemporaryUploadedFile(self.file_name, self.content_type, 0, self.charset)
 
     def receive_data_chunk(self, raw_data, start):
-        self.write(raw_data)
+        self.file.write(raw_data)
 
     def file_complete(self, file_size):
         self.file.seek(0)
-        return TemporaryUploadedFile(
-            file = self.file, 
-            file_name = self.file_name, 
-            content_type = self.content_type, 
-            file_size = file_size, 
-            charset = self.charset
-        )
+        self.file.size = file_size
+        return self.file
 
 class MemoryFileUploadHandler(FileUploadHandler):
     """
@@ -189,37 +183,12 @@ def file_complete(self, file_size):
         return InMemoryUploadedFile(
             file = self.file,
             field_name = self.field_name,
-            file_name = self.file_name,
+            name = self.file_name,
             content_type = self.content_type,
-            file_size = file_size,
+            size = file_size,
             charset = self.charset
         )
 
-class TemporaryFile(object):
-    """
-    A temporary file that tries to delete itself when garbage collected.
-    """
-    def __init__(self, dir):
-        if not dir:
-            dir = tempfile.gettempdir()
-        try:
-            (fd, name) = tempfile.mkstemp(suffix='.upload', dir=dir)
-            self.file = os.fdopen(fd, 'w+b')
-        except (OSError, IOError):
-            raise OSError("Could not create temporary file for uploading, have you set settings.FILE_UPLOAD_TEMP_DIR correctly?")
-        self.name = name
-
-    def __getattr__(self, name):
-        a = getattr(self.__dict__['file'], name)
-        if type(a) != type(0):
-            setattr(self, name, a)
-        return a
-
-    def __del__(self):
-        try:
-            os.unlink(self.name)
-        except OSError:
-            pass
 
 def load_handler(path, *args, **kwargs):
     """

django/db/models/base.py

@@ -536,7 +536,7 @@ def _save_FIELD_file(self, field, filename, raw_field, save=True):
             # This is a normal uploadedfile that we can stream.
             fp = open(full_filename, 'wb')
             locks.lock(fp, locks.LOCK_EX)
-            for chunk in raw_field.chunk():
+            for chunk in raw_field.chunks():
                 fp.write(chunk)
             locks.unlock(fp)
             fp.close()

django/db/models/fields/__init__.py

@@ -766,9 +766,12 @@ def get_internal_type(self):
     def get_db_prep_save(self, value):
         "Returns field's value prepared for saving into a database."
         # Need to convert UploadedFile objects provided via a form to unicode for database insertion
-        if value is None:
+        if hasattr(value, 'name'):
+            return value.name
+        elif value is None:
             return None
-        return unicode(value)
+        else:
+            return unicode(value)
 
     def get_manipulator_fields(self, opts, manipulator, change, name_prefix='', rel=False, follow=True):
         field_list = Field.get_manipulator_fields(self, opts, manipulator, change, name_prefix, rel, follow)
@@ -842,7 +845,7 @@ def save_file(self, new_data, new_object, original_object, change, rel, save=Tru
             # We don't need to raise a warning because Model._save_FIELD_file will
             # do so for us.
             try:
-                file_name = file.file_name
+                file_name = file.name
             except AttributeError:
                 file_name = file['filename']
 
@@ -857,9 +860,9 @@ def get_filename(self, filename):
         return os.path.normpath(f)
 
     def save_form_data(self, instance, data):
-        from django.newforms.fields import UploadedFile
+        from django.core.files.uploadedfile import UploadedFile
         if data and isinstance(data, UploadedFile):
-            getattr(instance, "save_%s_file" % self.name)(data.filename, data.data, save=False)
+            getattr(instance, "save_%s_file" % self.name)(data.name, data, save=False)
 
     def formfield(self, **kwargs):
         defaults = {'form_class': forms.FileField}