Fixed #350: Error when trying to access width/height after url

[vstoykov]

If the file is closed and something is calling open now the file will be opened correctly event if it was already closed

Fixes #350

[vstoykov]

This issue probably solves #369 also.

[mgalgs]

I'm still seeing a ValueError: The file cannot be reopened. while trying to access an ImageSpecField.url with this patch, although it might be in a different place from the one you were trying to fix...

Traceback (most recent call last):
  File "/home/mgalgs/sites/str-prod/env/lib/python2.7/site-packages/django/core/handlers/base.py", line 149, in get_response
    response = self.process_exception_by_middleware(e, request)
  File "/home/mgalgs/sites/str-prod/env/lib/python2.7/site-packages/django/core/handlers/base.py", line 147, in get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/home/mgalgs/sites/str-prod/env/lib/python2.7/site-packages/django/contrib/auth/decorators.py", line 23, in _wrapped_view
    return view_func(request, *args, **kwargs)
  File "/home/mgalgs/sites/str-prod/tracking/decorators.py", line 44, in wrapped
    return function(request, *args, **kwargs)
  File "/home/mgalgs/sites/str-prod/tracking/views.py", line 1501, in rotate_right
    'news': ii.all_urls,
  File "/home/mgalgs/sites/str-prod/tracking/models.py", line 564, in all_urls
    'img_medium': self.image_medium.url,
  File "/home/mgalgs/sites/str-prod/env/lib/python2.7/site-packages/imagekit/cachefiles/__init__.py", line 84, in url
    return self._storage_attr('url')
  File "/home/mgalgs/sites/str-prod/env/lib/python2.7/site-packages/imagekit/cachefiles/__init__.py", line 74, in _storage_attr
    existence_required.send(sender=self, file=self)
  File "/home/mgalgs/sites/str-prod/env/lib/python2.7/site-packages/django/dispatch/dispatcher.py", line 192, in send
    response = receiver(signal=self, sender=sender, **named)
  File "/home/mgalgs/sites/str-prod/env/lib/python2.7/site-packages/imagekit/registry.py", line 53, in existence_required_receiver
    self._receive(file, 'on_existence_required')
  File "/home/mgalgs/sites/str-prod/env/lib/python2.7/site-packages/imagekit/registry.py", line 61, in _receive
    call_strategy_method(file, callback)
  File "/home/mgalgs/sites/str-prod/env/lib/python2.7/site-packages/imagekit/utils.py", line 151, in call_strategy_method
    fn(file)
  File "/home/mgalgs/sites/str-prod/env/lib/python2.7/site-packages/imagekit/cachefiles/strategies.py", line 15, in on_existence_required
    file.generate()
  File "/home/mgalgs/sites/str-prod/env/lib/python2.7/site-packages/imagekit/cachefiles/__init__.py", line 93, in generate
    self.cachefile_backend.generate(self, force)
  File "/home/mgalgs/sites/str-prod/env/lib/python2.7/site-packages/imagekit/cachefiles/backends.py", line 109, in generate
    self.generate_now(file, force=force)
  File "/home/mgalgs/sites/str-prod/env/lib/python2.7/site-packages/imagekit/cachefiles/backends.py", line 96, in generate_now
    file._generate()
  File "/home/mgalgs/sites/str-prod/env/lib/python2.7/site-packages/imagekit/cachefiles/__init__.py", line 97, in _generate
    content = generate(self.generator)
  File "/home/mgalgs/sites/str-prod/env/lib/python2.7/site-packages/imagekit/utils.py", line 134, in generate
    content = generator.generate()
  File "/home/mgalgs/sites/str-prod/env/lib/python2.7/site-packages/imagekit/specs/__init__.py", line 153, in generate
    self.source.open()
  File "/home/mgalgs/sites/str-prod/env/lib/python2.7/site-packages/django/db/models/fields/files.py", line 81, in open
    self.file.open(mode)
  File "/home/mgalgs/sites/str-prod/env/lib/python2.7/site-packages/django/core/files/base.py", line 141, in open
    raise ValueError("The file cannot be reopened.")
ValueError: The file cannot be reopened.

My view_func looks like this:

def rotate_right(request, pk):
    ii = get_object_or_404(InvItemImage, pk=pk)
    oldies = ii.all_urls
    ii.rotate_right()
    if request.method == 'POST':
        return JSONResponse(request, {
            'olds': oldies,
            'news': ii.all_urls,
        })
    return HttpResponseRedirect(request.GET.get('next', '/'))

And my InvItemImage model look like this:

class InvItemImage(models.Model):
    image_full = ProcessedImageField(upload_to='invitem_images',
                                     processors=[Transpose(), ResizeToFit(1000, 1000)],
                                     format='JPEG',
                                     options={'quality': 75})
    image_thumbnail = ImageSpecField(source='image_full',
                                     processors=[Transpose(), ResizeToFit(100, 100)],
                                     format='JPEG',
                                     options={'quality': 60})
    image_teeny = ImageSpecField(source='image_full',
                                 processors=[Transpose(), ResizeToFit(30, 30)],
                                 format='JPEG',
                                 options={'quality': 40})
    image_medium = ImageSpecField(source='image_full',
                                  processors=[Transpose(), ResizeToFit(350, 350)],
                                  format='JPEG',
                                  options={'quality': 60})

    @property
    def all_urls(self):
        return {
            'img_thumb': self.image_thumbnail.url,
            'img_medium': self.image_medium.url,
            'img_full': self.image_full.url,
        }

    def __unicode__(self):
        return str(self.image_full)

    def _rotate(self, degrees):
        import StringIO
        # Without the following open I get:
        # ValueError: I/O operation on closed file
        # Apprently it's a bug? http://stackoverflow.com/a/3033986/209050
        self.image_full.open()
        rotated = PIL.Image.open(self.image_full).rotate(degrees, expand=1)
        sio = StringIO.StringIO()
        rotated.save(sio, format='JPEG')
        imuf = InMemoryUploadedFile(sio, None, 'rotated.jpg', 'image/jpeg',
                                    sio.len, None)
        self.image_full = imuf
        self.save()

    def rotate_left(self):
        self._rotate(90)

    def rotate_right(self):
        self._rotate(-90)

I'll try and come up with a minimal django app that reproduces the problem, but maybe the snippets above are enough to get an idea of what's going on...

[vstoykov]

@mgalgs in your code you rotate the image right on every hit of the view and throw the original version? It looks very strange but probably this is what you want.

As a workaround can you try to put this at the end of InvItemImage._rotate:

def _rotate(self):
    ...
    self.save()
    del self.image_full.file

And if it works you can put a breakpoint before the deletion of the attribute and show me what is tha value of self.image_full.file.

[mgalgs]

Thanks @vstoykov!

@mgalgs in your code you rotate the image right on every hit of the view and throw the original version? It looks very strange but probably this is what you want.

Yes, it's actually a rotate view. The reason I just throw away the original is that I want to ensure that all new thumbnails get generated, but maybe there's a better way of doing that?

As a workaround can you try to put this at the end of InvItemImage._rotate: [...] And if it works you can put a breakpoint before the deletion of the attribute and show me what is tha value of self.image_full.file.

Seems to work! Here's the value of self.image_full.file just before the del statement:

>>> self.image_full.file
<File: invitem_images/rotated_kgyhPtd.jpg>
>>> self.image_full
<ProcessedImageFieldFile: invitem_images/rotated_kgyhPtd.jpg>
>>> self.image_full.file.closed
True
>>> self.image_full.file.file
<tempfile.SpooledTemporaryFile instance at 0x7f5f0c5745a8>
>>> self.image_full.file.file.closed
True

Can't say I understand why this works, which is a bit concerning to me... Is this a temporary workaround for something else or is this just the right way to do some manual processing of a ProcessedImageField? It would be nice to have formal support for "manual" (i.e. non-Processor) processing of ProcessedImageFields.

[mgalgs]

Also, just to be clear, your suggestion of deleting self.image_full.file after the save seems to clear up my issue even without this PR, so I might be exercising a completely different code path...

[vstoykov]

About deletion of the attribute you can see the changes in this exact PR and see that I'm doing exactly this to solve the issue.

The reason that this works is because of the file property of the ProcessedImageFieldFile. It is inherited from BaseIKFile (https://github.com/matthewwithanm/django-imagekit/blob/develop/imagekit/files.py#L26-L38). The field file is actually a wrapper around "real" (or some other) file. By "deleting" the property you actually delete the underlying file which will force opening it again from the storage the next time when the file attribute was acceded.

About your rotation process it is failing because your code path is different than the issue described in #350 (but still similar).

I will ask you for one more favor. Can you try to modify temporary your code not to use ProcessedImageField but normal ImageField and to remove the "fix" (the row where you delete self.image_full.file attribute) to see if it will work?

If it fails then it is intended behavior and not bug in IK. If it works I need to fix it in IK.

P.S. I have a feeling that it will work and even I think that I know where is the problem in IK but still.

[mgalgs]

I will ask you for one more favor. Can you try to modify temporary your code not to use ProcessedImageField but normal ImageField and to remove the "fix" (the row where you delete self.image_full.file attribute) to see if it will work?

It actually fails in the exact same way (generating the cachefile when I access .url the second time (after rotating the image)). hmmm...

[vstoykov]

And this is with changes in this PR or without them?

[mgalgs]

Sorry, I should have mentioned I tried it with and without the changes in this PR and got the same result. So:

ProcessedImageField	ImageField	PR17	del workaround	Result
x				Fail
x			x	Pass
x		x		Fail
x		x	x	Pass
	x		n/a	Fail
	x	x	n/a	Fail

[vstoykov]

By PR17 you mean PR384 (this pull request) right?

By n/a in the del workaround column you mean that just the workaround was not applied like in 1st and 3rd rows right? (counting start from 1 not zero ;) )

From my understanding it seems that your code is doing some non standard stuff. In your case you can use the del workaround and for now probably this will be enough.

I still think (believe) that there is something more that can be done in dajngo imagekit and probably your case will be fixed.

For now I think that that we need to open the new issue for your problem because it's not the same as #350 (If you still want something to be done in django imagekit).

[mgalgs]

By PR17 you mean PR384 (this pull request) right?

Doh, yes. Sorry about that...

By n/a in the del workaround column you mean that just the workaround was not applied like in 1st and 3rd rows right? (counting start from 1 not zero ;) )

Actually I tried the workaround but it didn't work. I forget the exact error but it seems like you can't del an ImageField, at least not in this case.

For now I think that that we need to open the new issue for your problem because it's not the same as #350 (If you still want something to be done in django imagekit).

Sounds good. I'll go with the workaround for now and will file a new issue to track this further since it seems like doing manual processing of an image should be supported in imagekit.

[Terr]

I responded in the original issue (#350) as well and for me the error has disappeared with your changes in this branch. Thanks for putting in time to fix this!

[vstoykov]

Ok I think that I should now accept this PR and soon we will have new version of django-imagekit