File object does not consistently open itself in context manager use

[Raphael Gaschignard]

Django's File object has an open method that will reseek the file to 0, and re-open the file if necessary (after closure).

Unfortunately, the context manager does not automatically open a file. In particular, FieldFile relies on implicit opening via the file property to open a file, but caches the object internally. Since the context manager for File closes the file after use, this means that using the context manager twice in a row fails (since the file is not re-opened).

class C(Model):
    pdf = FileField(...)

c = C.objects.get()

with c.pdf as pdf: # file not opened yet
     print(c.size) # pdf.size accesses pdf.file, which will implicitly open the file
# file is closed at the end of a context manager

 with c.pdf as pdf:
    print(c.size) # pdf.size now uses cached file, which is already closed!

I think it makes sense to have the context manager(__enter__) call open on the object, so that files can be used in multiple context managers without having to to an open-ness check.

our current workaround is to check for closed-ness on all context manager uses:

with c.pdf as pdf:
     if pdf.closed():
         pdf.open() # seeks the file to 0 and opens if necessary
     # do "real" work

[Tim Graham]

Could you please provide a test case for Django's test suite that reproduces the problem? I tried with the attached test but it works. I'm not sure if the expected failure is a crash or something else.

[Raphael Gaschignard]

Sorry, I chose the wrong property. Here's two examples of behavior I found that didn't make sense to me at first.

ContentFile does not properly seek back to 0 on context manager re-use.

In [8]: from django.core.files.base import ContentFile

In [9]: my_file = ContentFile("Hi there")

In [10]: with my_file as f: print(f.read())
Hi there

In [11]: with my_file as f: print(f.read())
# empty string, no content

for Files that use actual files in their backend, multiple context manager usages results in a value error (I/O operation on closed file)

In [12]: from django.core.files.base import File

In [13]: my_file = File(open('hi.txt'))

In [14]: with my_file as f: print(f.read())
    ...: with my_file as f: print(f.read())
    ...:
Hi

---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-14-ea725ad0698a> in <module>()
      1 with my_file as f: print(f.read())
----> 2 with my_file as f: print(f.read())
      3

ValueError: I/O operation on closed file.

I figured out that this behaviour *is* consistent with the behavior of python's file object. But given that Django's File object includes an "idempotent" open, it would seem useful for the context manager to call open, so that we open the file and seek back to 0.

[Tim Graham]

From a quick look, I'm not sure about if the change can be done in a backwards compatible way. The attached patch seems what you have in mind but it causes a regression in one test. Accepting for further investigation.

======================================================================
ERROR: test_filefield_write (file_storage.tests.FileFieldStorageTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/opt/python3.5.3/lib/python3.5/unittest/case.py", line 59, in testPartExecutor
    yield
  File "/opt/python3.5.3/lib/python3.5/unittest/case.py", line 601, in run
    testMethod()
  File "/home/tim/code/django/tests/file_storage/tests.py", line 660, in test_filefield_write
    normal.write(b'updated')
io.UnsupportedOperation: write

[Ingo Klöcker]

I'll look into this.

[Ingo Klöcker]

​PR

[Florian Apolloner <apollo13@…>]

In c4536c4:

Fixed #27777 -- Made File.open() work with the with statement (#8310)

Fixed #27777 -- Made File.open() work with the with statement

[Tim Graham <timograham@…>]

In 6bb3b2b:

Refs #27777 -- Improved docs/added test for File context manager change.