Author: jacob
Date: 2009-05-08 10:08:09 -0500 (Fri, 08 May 2009)
New Revision: 10717

Added:
   django/trunk/django/core/files/utils.py
Modified:
   django/trunk/django/core/files/base.py
   django/trunk/django/core/files/temp.py
   django/trunk/django/core/files/uploadedfile.py
   django/trunk/django/db/models/fields/files.py
   django/trunk/tests/modeltests/files/models.py
   django/trunk/tests/regressiontests/file_storage/models.py
Log:
Fixed #7712, #9404, #10249, #10300: a light refactor and cleanup of file 
storage and the `File` object. Thanks to Armin Ronacher and Alex Gaynor.

Modified: django/trunk/django/core/files/base.py
===================================================================
--- django/trunk/django/core/files/base.py      2009-05-08 13:39:37 UTC (rev 
10716)
+++ django/trunk/django/core/files/base.py      2009-05-08 15:08:09 UTC (rev 
10717)
@@ -1,20 +1,23 @@
 import os
 
-from django.utils.encoding import smart_str, smart_unicode
-
 try:
     from cStringIO import StringIO
 except ImportError:
     from StringIO import StringIO
 
-class File(object):
+from django.utils.encoding import smart_str, smart_unicode
+from django.core.files.utils import FileProxyMixin
+
+class File(FileProxyMixin):
     DEFAULT_CHUNK_SIZE = 64 * 2**10
 
-    def __init__(self, file):
+    def __init__(self, file, name=None):
         self.file = file
-        self._name = file.name
-        self._mode = file.mode
-        self._closed = False
+        if name is None:
+            name = getattr(file, 'name', None)
+        self.name = name
+        self.mode = getattr(file, 'mode', None)
+        self.closed = False
 
     def __str__(self):
         return smart_str(self.name or '')
@@ -26,25 +29,11 @@
         return "<%s: %s>" % (self.__class__.__name__, self or "None")
 
     def __nonzero__(self):
-        return not not self.name
+        return bool(self.name)
 
     def __len__(self):
         return self.size
 
-    def _get_name(self):
-        if not hasattr(self, '_name'):
-            raise ValueError("This operation requires the file to have a 
name.")
-        return self._name
-    name = property(_get_name)
-
-    def _get_mode(self):
-        return self._mode
-    mode = property(_get_mode)
-
-    def _get_closed(self):
-        return self._closed
-    closed = property(_get_closed)
-
     def _get_size(self):
         if not hasattr(self, '_size'):
             if hasattr(self.file, 'size'):
@@ -66,7 +55,7 @@
         ``UploadedFile.DEFAULT_CHUNK_SIZE``).
         """
         if not chunk_size:
-            chunk_size = self.__class__.DEFAULT_CHUNK_SIZE
+            chunk_size = self.DEFAULT_CHUNK_SIZE
 
         if hasattr(self, 'seek'):
             self.seek(0)
@@ -89,12 +78,6 @@
             chunk_size = self.DEFAULT_CHUNK_SIZE
         return self.size > chunk_size
 
-    def xreadlines(self):
-        return iter(self)
-
-    def readlines(self):
-        return list(self.xreadlines())
-
     def __iter__(self):
         # Iterate over this file-like object by newlines
         buffer_ = None
@@ -121,43 +104,22 @@
             self.seek(0)
         elif os.path.exists(self.file.name):
             self.file = open(self.file.name, mode or self.file.mode)
+            self.closed = False
         else:
             raise ValueError("The file cannot be reopened.")
 
-    def seek(self, position):
-        self.file.seek(position)
-
-    def tell(self):
-        return self.file.tell()
-
-    def read(self, num_bytes=None):
-        if num_bytes is None:
-            return self.file.read()
-        return self.file.read(num_bytes)
-
-    def write(self, content):
-        if not self.mode.startswith('w'):
-            raise IOError("File was not opened with write access.")
-        self.file.write(content)
-
-    def flush(self):
-        if not self.mode.startswith('w'):
-            raise IOError("File was not opened with write access.")
-        self.file.flush()
-
     def close(self):
         self.file.close()
-        self._closed = True
+        self.closed = True
 
 class ContentFile(File):
     """
     A File-like object that takes just raw content, rather than an actual file.
     """
     def __init__(self, content):
-        self.file = StringIO(content or '')
-        self.size = len(content or '')
-        self.file.seek(0)
-        self._closed = False
+        content = content or ''
+        super(ContentFile, self).__init__(StringIO(content))
+        self.size = len(content)
 
     def __str__(self):
         return 'Raw content'
@@ -166,6 +128,6 @@
         return True
 
     def open(self, mode=None):
-        if self._closed:
-            self._closed = False
+        if self.closed:
+            self.closed = False
         self.seek(0)

Modified: django/trunk/django/core/files/temp.py
===================================================================
--- django/trunk/django/core/files/temp.py      2009-05-08 13:39:37 UTC (rev 
10716)
+++ django/trunk/django/core/files/temp.py      2009-05-08 15:08:09 UTC (rev 
10717)
@@ -11,11 +11,12 @@
 
 import os
 import tempfile
+from django.core.files.utils import FileProxyMixin
 
 __all__ = ('NamedTemporaryFile', 'gettempdir',)
 
 if os.name == 'nt':
-    class TemporaryFile(object):
+    class TemporaryFile(FileProxyMixin):
         """
         Temporary file object constructor that works in Windows and supports
         reopening of the temporary file in windows.
@@ -48,12 +49,6 @@
         def __del__(self):
             self.close()
 
-        # Proxy to the file object.
-        def __getattr__(self, name):
-            return getattr(self.file, name)
-        def __iter__(self):
-            return iter(self.file)
-
     NamedTemporaryFile = TemporaryFile
 else:
     NamedTemporaryFile = tempfile.NamedTemporaryFile

Modified: django/trunk/django/core/files/uploadedfile.py
===================================================================
--- django/trunk/django/core/files/uploadedfile.py      2009-05-08 13:39:37 UTC 
(rev 10716)
+++ django/trunk/django/core/files/uploadedfile.py      2009-05-08 15:08:09 UTC 
(rev 10717)
@@ -26,14 +26,15 @@
     """
     DEFAULT_CHUNK_SIZE = 64 * 2**10
 
-    def __init__(self, name=None, content_type=None, size=None, charset=None):
-        self.name = name
+    def __init__(self, file=None, name=None, content_type=None, size=None, 
charset=None):
+        super(UploadedFile, self).__init__(file, name)
         self.size = size
         self.content_type = content_type
         self.charset = charset
 
     def __repr__(self):
-        return "<%s: %s (%s)>" % (self.__class__.__name__, 
smart_str(self.name), self.content_type)
+        return "<%s: %s (%s)>" % (
+            self.__class__.__name__, smart_str(self.name), self.content_type)
 
     def _get_name(self):
         return self._name
@@ -53,95 +54,66 @@
 
     name = property(_get_name, _set_name)
 
-    # Abstract methods; subclasses *must* define read() and probably should
-    # define open/close.
-    def read(self, num_bytes=None):
-        raise NotImplementedError()
-
-    def open(self):
-        pass
-
-    def close(self):
-        pass
-
 class TemporaryUploadedFile(UploadedFile):
     """
     A file uploaded to a temporary location (i.e. stream-to-disk).
     """
     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)
+            file = tempfile.NamedTemporaryFile(suffix='.upload',
+                dir=settings.FILE_UPLOAD_TEMP_DIR)
         else:
-            self._file = tempfile.NamedTemporaryFile(suffix='.upload')
+            file = tempfile.NamedTemporaryFile(suffix='.upload')
+        super(TemporaryUploadedFile, self).__init__(file, name, content_type, 
size, charset)
 
     def temporary_file_path(self):
         """
         Returns the full path of this file.
         """
-        return self._file.name
+        return self.file.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, *args):          return self._file.seek(*args)
-    def write(self, s):             return self._file.write(s)
-    def tell(self, *args):          return self._file.tell(*args)
-    def __iter__(self):             return iter(self._file)
-    def readlines(self, size=None): return self._file.readlines(size)
-    def xreadlines(self):           return self._file.xreadlines()
     def close(self):
         try:
-            return self._file.close()
-        except OSError, e:
-            if e.errno == 2:
-                # Means the file was moved or deleted before the tempfile 
could unlink it.
-                # Still sets self._file.close_called and calls 
self._file.file.close()
-                # before the exception
-                return
-            else:
-                raise e
+            try:
+                return self.file.close()
+            except OSError, e:
+                if e.errno != 2:
+                    # Means the file was moved or deleted before the tempfile
+                    # could unlink it.  Still sets self.file.close_called and
+                    # calls self.file.file.close() before the exception
+                    raise
+        finally:
+            self.closed = True
 
 class InMemoryUploadedFile(UploadedFile):
     """
     A file uploaded into memory (i.e. stream-to-memory).
     """
     def __init__(self, file, field_name, name, content_type, size, charset):
-        super(InMemoryUploadedFile, self).__init__(name, content_type, size, 
charset)
-        self._file = file
+        super(InMemoryUploadedFile, self).__init__(file, name, content_type, 
size, charset)
         self.field_name = field_name
-        self._file.seek(0)
 
     def open(self):
-        self._file.seek(0)
+        self.closed = False
+        self.file.seek(0)
 
     def chunks(self, chunk_size=None):
-        self._file.seek(0)
+        self.file.seek(0)
         yield self.read()
 
     def multiple_chunks(self, chunk_size=None):
         # Since it's in memory, we'll never have multiple chunks.
         return False
 
-    # proxy methods to StringIO
-    def read(self, *args): return self._file.read(*args)
-    def seek(self, *args): return self._file.seek(*args)
-    def tell(self, *args): return self._file.tell(*args)
-    def close(self):       return self._file.close()
 
 class SimpleUploadedFile(InMemoryUploadedFile):
     """
     A simple representation of a file, which just has content, size, and a 
name.
     """
     def __init__(self, name, content, content_type='text/plain'):
-        self._file = StringIO(content or '')
-        self.name = name
-        self.field_name = None
-        self.size = len(content or '')
-        self.content_type = content_type
-        self.charset = None
-        self._file.seek(0)
+        content = content or ''
+        super(SimpleUploadedFile, self).__init__(StringIO(content), None, name,
+                                                 content_type, len(content), 
None)
 
     def from_dict(cls, file_dict):
         """
@@ -154,5 +126,4 @@
         return cls(file_dict['filename'],
                    file_dict['content'],
                    file_dict.get('content-type', 'text/plain'))
-
     from_dict = classmethod(from_dict)

Added: django/trunk/django/core/files/utils.py
===================================================================
--- django/trunk/django/core/files/utils.py                             (rev 0)
+++ django/trunk/django/core/files/utils.py     2009-05-08 15:08:09 UTC (rev 
10717)
@@ -0,0 +1,29 @@
+class FileProxyMixin(object):
+    """
+    A mixin class used to forward file methods to an underlaying file
+    object.  The internal file object has to be called "file"::
+
+        class FileProxy(FileProxyMixin):
+            def __init__(self, file):
+                self.file = file
+    """
+
+    encoding = property(lambda self: self.file.encoding)
+    fileno = property(lambda self: self.file.fileno)
+    flush = property(lambda self: self.file.flush)
+    isatty = property(lambda self: self.file.isatty)
+    newlines = property(lambda self: self.file.newlines)
+    read = property(lambda self: self.file.read)
+    readinto = property(lambda self: self.file.readinto)
+    readline = property(lambda self: self.file.readline)
+    readlines = property(lambda self: self.file.readlines)
+    seek = property(lambda self: self.file.seek)
+    softspace = property(lambda self: self.file.softspace)
+    tell = property(lambda self: self.file.tell)
+    truncate = property(lambda self: self.file.truncate)
+    write = property(lambda self: self.file.write)
+    writelines = property(lambda self: self.file.writelines)
+    xreadlines = property(lambda self: self.file.xreadlines)
+
+    def __iter__(self):
+        return iter(self.file)

Modified: django/trunk/django/db/models/fields/files.py
===================================================================
--- django/trunk/django/db/models/fields/files.py       2009-05-08 13:39:37 UTC 
(rev 10716)
+++ django/trunk/django/db/models/fields/files.py       2009-05-08 15:08:09 UTC 
(rev 10717)
@@ -17,11 +17,10 @@
 
 class FieldFile(File):
     def __init__(self, instance, field, name):
+        super(FieldFile, self).__init__(None, name)
         self.instance = instance
         self.field = field
         self.storage = field.storage
-        self._name = name or u''
-        self._closed = False
         self._committed = True
 
     def __eq__(self, other):
@@ -48,11 +47,18 @@
 
     def _get_file(self):
         self._require_file()
-        if not hasattr(self, '_file'):
+        if not hasattr(self, '_file') or self._file is None:
             self._file = self.storage.open(self.name, 'rb')
         return self._file
-    file = property(_get_file)
 
+    def _set_file(self, file):
+        self._file = file
+
+    def _del_file(self):
+        del self._file
+
+    file = property(_get_file, _set_file, _del_file)
+
     def _get_path(self):
         self._require_file()
         return self.storage.path(self.name)
@@ -65,6 +71,8 @@
 
     def _get_size(self):
         self._require_file()
+        if not self._committed:
+            return len(self.file)
         return self.storage.size(self.name)
     size = property(_get_size)
 
@@ -80,7 +88,7 @@
 
     def save(self, name, content, save=True):
         name = self.field.generate_filename(self.instance, name)
-        self._name = self.storage.save(name, content)
+        self.name = self.storage.save(name, content)
         setattr(self.instance, self.field.name, self.name)
 
         # Update the filesize cache
@@ -97,11 +105,11 @@
         # presence of self._file
         if hasattr(self, '_file'):
             self.close()
-            del self._file
-            
+            del self.file
+
         self.storage.delete(self.name)
 
-        self._name = None
+        self.name = None
         setattr(self.instance, self.field.name, self.name)
 
         # Delete the filesize cache
@@ -113,12 +121,18 @@
             self.instance.save()
     delete.alters_data = True
 
+    def close(self):
+        file = getattr(self, '_file', None)
+        if file is not None:
+            file.close()
+            self.closed = True
+
     def __getstate__(self):
         # FieldFile needs access to its associated model field and an instance
         # it's attached to in order to work properly, but the only necessary
         # data to be pickled is the file's name itself. Everything else will
         # be restored later, by FileDescriptor below.
-        return {'_name': self.name, '_closed': False, '_committed': True}
+        return {'name': self.name, 'closed': False, '_committed': True, 
'_file': None}
 
 class FileDescriptor(object):
     def __init__(self, field):
@@ -134,12 +148,8 @@
         elif isinstance(file, File) and not isinstance(file, FieldFile):
             # Other types of files may be assigned as well, but they need to
             # have the FieldFile interface added to them
-            file_copy = copy.copy(file)
-            file_copy.__class__ = type(file.__class__.__name__, 
-                                       (file.__class__, 
self.field.attr_class), {})
-            file_copy.instance = instance
-            file_copy.field = self.field
-            file_copy.storage = self.field.storage
+            file_copy = self.field.attr_class(instance, self.field, file.name)
+            file_copy.file = file
             file_copy._committed = False
             instance.__dict__[self.field.name] = file_copy
         elif isinstance(file, FieldFile) and not hasattr(file, 'field'):

Modified: django/trunk/tests/modeltests/files/models.py
===================================================================
--- django/trunk/tests/modeltests/files/models.py       2009-05-08 13:39:37 UTC 
(rev 10716)
+++ django/trunk/tests/modeltests/files/models.py       2009-05-08 15:08:09 UTC 
(rev 10717)
@@ -6,6 +6,7 @@
 """
 
 import shutil
+import random
 import tempfile
 from django.db import models
 from django.core.files.base import ContentFile
@@ -26,7 +27,6 @@
     def random_upload_to(self, filename):
         # This returns a different result each time,
         # to make sure it only gets called once.
-        import random
         return '%s/%s' % (random.randint(100, 999), filename)
 
     normal = models.FileField(storage=temp_storage, upload_to='tests')

Modified: django/trunk/tests/regressiontests/file_storage/models.py
===================================================================
--- django/trunk/tests/regressiontests/file_storage/models.py   2009-05-08 
13:39:37 UTC (rev 10716)
+++ django/trunk/tests/regressiontests/file_storage/models.py   2009-05-08 
15:08:09 UTC (rev 10717)
@@ -29,7 +29,7 @@
         mug_width = models.PositiveSmallIntegerField()
 
     __test__ = {'API_TESTS': """
-
+>>> from django.core.files import File
 >>> image_data = open(os.path.join(os.path.dirname(__file__), "test.png"), 
 >>> 'rb').read()
 >>> p = Person(name="Joe")
 >>> p.mugshot.save("mug", ContentFile(image_data))
@@ -76,14 +76,15 @@
 # It won't have an opened file. This is a bit brittle since it depends on the
 # the internals of FieldFile, but there's no other way of telling if the
 # file's been opened or not.
->>> hasattr(p3.mugshot, '_file')
+>>> p3.mugshot._file is not None
 False
 
 # After asking for the size, the file should still be closed.
 >>> _ = p3.mugshot.size
->>> hasattr(p3.mugshot, '_file')
+>>> p3.mugshot._file is not None
 False
 
+>>> p = Person.objects.create(name="Bob", mugshot=File(p3.mugshot.file))
+
 >>> shutil.rmtree(temp_storage_dir)
 """}
-


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en
-~----------~----~----~----~------~----~------~--~---

Reply via email to