Unexpected behavior of DetailView on a model defining __len__() returning False

[Max]

Description

When using Django's DetailView on a model defining a __len__() method, the model object is not always added to context. This is the case, when __len__() of the model object returns zero. The behavior is caused by BaseDetailView.get() calling SingleObjectMixin.get_context_data() which checks like this, if self.object was set before bySingleObjectMixin.get_object():

if self.object:
     # adds self.object to context

Because this check is happening implicitly and not against None, pythons default implementation calling self.object.__len__() (​https://docs.python.org/3/library/stdtypes.html#truth-value-testing) causes this condition to evaluate to False, at least if __len__() returns zero. This leads to not having the wanted object in context data.

Steps to reproduce:

Create an empty model defining a __len__ method returning zero and a DetailView like this:

class TestModel(models.Model):
    def __len__(self):
        return 0


class TestModelDetailView(DetailView):
    model = TestModel
    template_name = 'test.html'  # some template

After creating a TestModel object, the described behavior can be reproduced when calling the Detailview (url pattern like this /test/<int:pk>/).

Proposed solution

Simply checking if self.object is the default value None prevents this not easy to find bug.
SingleObjectMixin.get_queryset() also has a similar check

if self.model:
    # some action

which does not seem to cause problems, because self.model is a class and no object. It should be considered to also check against the default value for self.model here, which is None.

[Simon Charette]

I guess adding an is None would do for this case but I wouldn't be surprised if a lot of other cases still break. For example, there's probably a few model forms or admin APIs that do something along if self.instance and would need to be adjusted as well.

Given defining __len__() on a model instance is an edge case and your issue can be worked around by defining a def __bool__(self): return True I'm tempted to close this issue as _wont fix_ but I'll let other contributors chime in. I feel like the burden of making sure all APIs dealing with optional model instances truthiness appropriately is not worth the small benefit here.

[Max]

I agree that the case of having __len__ defined on a model may be rare, but i would strongly propose doing the modification of this check. Although this may not be exhaustive (you mentioned cases like self.instance), i think it is worth it, because the current situation leads to highly unexpected behavior, which can be definitely avoided in this particular case.

[Tim Graham]

I agree with Simon that trying to adapt the Django ecosystem to this convention is likely an unreasonable burden. You can write to the DevelopersMailingList if you want to try to convince the community otherwise. It would certainly help your argument if you explained why you want to define __len__() as such. Others may suggest a better solution for the problem you're trying to solve.