LazyObject wrapped instances can no longer be coerced to Bool due to #20924

[Gabe Jackson]

https://code.djangoproject.com/ticket/20924 adds proxy methods for __contains__ and __len__ to support lists and dicts wrapped by LazyObject. The problem is that evaluating truth for such LazyObjects will break if __len__ is not defined on the wrapper instance.

This can be demonstrated as follows:

>>> from django.core.files.storage import default_storage
>>> if default_storage: print('yes')
... 
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/home/tisba/Programming/testbed/boogz/django/utils/functional.py", line 224, in inner
    return func(self._wrapped, *args)
TypeError: object of type 'FileSystemStorage' has no len()

this is because converting an object to a Bool calls object.__nonzero__ which in turn calls __len__ as stated here:
​http://docs.python.org/2/reference/datamodel.html#object.__nonzero

This will break a lot of apps using LazyObject. This problem exists in the current master of imagekit as well:

class JustInTime(object):
    """
    A strategy that ensures the file exists right before it's needed.

    """
    def on_existence_required(self, file):
        file.generate()

    def on_content_required(self, file):
        file.generate()

class StrategyWrapper(LazyObject):
    def __init__(self, strategy):
        if isinstance(strategy, six.string_types):
            strategy = get_singleton(strategy, 'cache file strategy')
        elif isinstance(strategy, dict):
            strategy = DictStrategy(strategy)
        elif callable(strategy):
            strategy = strategy()
        self._wrapped = strategy

    def __getstate__(self):
        return {'_wrapped': self._wrapped}

    def __setstate__(self, state):
        self._wrapped = state['_wrapped']

    def __unicode__(self):
        return force_text(self._wrapped)

    def __str__(self):
        return str(self._wrapped)

calling the LazyObject in a statement like if lazy_object will yield:

In [2]: p.avatar_thumbnail
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-2-860e30762713> in <module>()
----> 1 p.avatar_thumbnail

/Users/gabejackson/venv/hvad-test/lib/python2.7/site-packages/imagekit/models/fields/utils.pyc in __get__(self, instance, owner)
     15             spec = self.field.get_spec(source=source)
     16             import ipdb; ipdb.set_trace()
---> 17             file = ImageCacheFile(spec)
     18             instance.__dict__[self.attname] = file
     19             return file

/Users/gabejackson/venv/hvad-test/lib/python2.7/site-packages/imagekit/cachefiles/__init__.pyc in __init__(self, generator, name, storage, cachefile_backend, cachefile_strategy)
     55         self.cachefile_strategy = (
     56             cachefile_strategy
---> 57             or getattr(generator, 'cachefile_strategy', None)
     58             or get_singleton(settings.IMAGEKIT_DEFAULT_CACHEFILE_STRATEGY, 'cache file strategy')
     59         )

/Users/gabejackson/venv/hvad-test/lib/python2.7/site-packages/django/utils/functional.pyc in inner(self, *args)
    222         if self._wrapped is empty:
    223             self._setup()
--> 224         return func(self._wrapped, *args)
    225     return inner
    226

TypeError: object of type 'JustInTime' has no len()

You can see that the or statement on line 57 is forcing the coercion of the JustInTime object to a Boolean. Doing this calls __len__ which is now defined on LazyObject and will cause the TypeError stated.

[Gabe Jackson]

Just to comment why this worked before:

"If a class defines neither __len__() nor __nonzero__(), all its instances are considered true."

that is from ​http://docs.python.org/2/reference/datamodel.html#object.__nonzero

[Gabe Jackson]

further notes:
It appears many of the proxy methods should be on LazyObject instead of on SimpleLazyObject because the latter inherits from the LazyObject.

SimpleLazyObject was introduced here: ​https://github.com/django/django/commit/a2d8acbacda353248fd191b97155cd4cf27dcd66
Proxies were added here: ​https://github.com/django/django/commit/60cf3f2f842b6e56132b8880c70acc87bd753c2e

a simple solution seems to just add: __nonzero__ = new_method_proxy(bool) to LazyObject.
I'm not familiar with these undocumented classes, but wouldn't it be possible to move all the proxying to LazyObject?

[Aymeric Augustin]

I left a note on the commit, asking the authors for ideas: ​https://github.com/django/django/commit/aa01c99f5587b36c354e80c682ad52e1a3b41455#commitcomment-5180010

[Baptiste Mispelon]

Pull request here: ​https://github.com/django/django/pull/2221

As far as I can tell, there's no reason for all the dunder methods to exist on SimpleLazyObject instead of LazyObject and moving them all to the parent class fixes this particular issue.

I also added a regression test and the full test suite passes (on sqlite at least).

[Marc Tamlyn]

On discussion with bmispelon - we would like to add more tests for this before committing. That said, it can be merged as is if not landed before beta.

[Baptiste Mispelon]

The current PR is broken (which makes me glad I decided to hold off and write some tests) since not all dunder methods can be moved as-is (repr being one of them for example).

[Baptiste Mispelon]

Updated the PR. It should be ready for final review now: ​https://github.com/django/django/pull/2221/

[Tim Graham]

Reviewed by Shai and myself.

[Baptiste Mispelon <bmispelon@…>]

In 61917aa08b4ab2bc35f3ffe87b7693bd8b58e205:

Fixed #21840 -- Moved dunder methods from SimpleLazyObject to LazyObject.

This commit also added tests for LazyObject and refactored
the testsuite of SimpleLazyObject so that it can share
test cases with LazyObject.

[Andrew Godwin <andrew@…>]

In 356f064c49f926dd771e6ee590b43c8ac5dc4efc:

Merge pull request #2221 from bmispelon/LazyObject-refactor

Fixed #21840 -- Moved dunder methods from SimpleLazyObject to LazyObject...