[patch] [magic-removal] Equality test doesn't work between models and model wrappers, e.g. User and UserWrapper

[Antti Kaihola]

If I have a model with a ForeignKey(User), I can't do this in my template:

{% ifequal user myobject.user %}

but must use

{% ifequal user.id myobject.user.id %}

[Antti Kaihola]

models.User and middleware.UserWrapper equality tests for contrib.auth

[Antti Kaihola]

The above patch allows for the more intuitive user equality test syntax.

I hope the patch doesn't have bad side-effects.

[akaihola <antti.kaihola@…>]

Ok, as I feared, it wasn't that simple. Trouble in admin. I'll try to find what's causing it.

[Malcolm Tredinnick <malcolm@…>]

By the way, the patch has problems in the change to middleware.py: it defines __eq__ twice (the second one should be __ne__).

[Antti Kaihola]

Enhanced equality test for models; works correctly for wrappers like UserWrapper

[Antti Kaihola]

Thanks, Malcolm.

I decided to look at the problem from a broader perspective of camparing any models and their wrapper classes. I came up with a different patch, which refines the equality test at the django.db.models.Model level.

The current equality test is based on two criteria. If x is a subclass of django.db.models.Model, x == y is True if:

1. isinstance(y, x.class), and
2. both objects have the same primary key value.

I enhanced this logic to work correctly also for wrapper classes like django.contrib.auth.middleware.UserWrapper. The criteria are now:

1. a) isinstance(y, x.class), OR b) y._meta exists and is the same object as x._meta AND
2. both objects have the same primary key value.

This works, because UserWrapper passes through all attributes (_meta among them) of the wrapped User object.

I quickly tested several operations in Admin and didn't find anything broken after applying this patch.

I also wrote a unit test (see next patch).

[Antti Kaihola]

Unit test for enhanced equality test. Could go in tests/modeltests/wrappers/.

[Adrian Holovaty]

What problem did you encounter with the first patch? It would be *much* cleaner to make a change to UserWrapper than change the equality logic framework-wide.

[jkocherhans]

FWIW UserWrapper needs to die a horrible death, or at least be changed quite a bit. See #1488 for one possible patch. A reasonable solution to that would also resovle this ticket.

[Antti Kaihola]

Adrian, no problem with the first patch apart from the typo Malcolm found.

I simply noticed that a generic fix to make wrapper class equality tests work is easy to do on the model level. Is there a drawback there?

[Adrian Holovaty]

Yeah, the drawback is that your patch solves a non-problem: Wrapper classes aren't a common solution, and it's overengineering to solve this on a generic level. (I can't think of any wrapper class other than the UserWrapper thing.)

And the UserWrapper problem was solved in another ticket -- #1488 and [2590] -- so I'm marking this one as fixed.