952550 - Default objects Manager gets used for relations

Status: RESOLVED WONTFIX

(Marketplace Graveyard :: Code Quality, defect, P3)

Description

[Mark Striemer [:mstriemer]]

The objects manager that we are defining on many objects get used for associations even though django uses a "plain" manager by default. django-cache-machine is the culprit (https://github.com/jbalogh/django-cache-machine/blob/v0.8/caching/base.py#L33). When defining our objects manager we might want to disable that flag.

amo.ManagerBase (https://github.com/mozilla/zamboni/blob/master/apps/amo/models.py#L156) inherits from caching.base.CachingManager (see above) which gets used for objects on Version and Addon at a minimum.

Comment 1

[Mark Striemer [:mstriemer]]

If the objects manager gets changed for version and addon then `File.addon` should be removed as well. See https://github.com/mozilla/zamboni/commit/371a85e6b9c6e508404f7f2925b0f780ef788ece#diff-7962dc902d031e550d31c50d17f2ca20R286.

Comment 2

[Mark Striemer [:mstriemer]]

You will run into this if you are working with a model that uses django-cache-machine for it's `objects` manager and it can be soft-deleted. When I saw this it was with Addon/Version/File and you would get an exception when trying to navigate the associations from File to Addon.

Since the caching manager is used and it will not find soft-deleted entries the File cannot see its associated Version or Addon objects without falling back to a manual query using `Addon.with_deleted` or `Version.with_deleted`.

Comment 3

[Mathieu Pillard [:mat]]

Note that this might be more complex than it looks like. The right thing to do is almost certainly to put use_for_related_fields = False on our objects Manager, this will unbreak all relations to soft-deleted instances, but at the same time we might have code relying on this behaviour...

Comment 4

[Mathieu Pillard [:mat]]

When removing cache-machine in bug 881405, I set use_for_related_fields = True in ManagerBase to keep compatibility with what cache-machine was doing.

This should still be removed, it still poses a number of problems (see above comments) but it was breaking a few tests and I didn't have the time to figure it out.