0

I'm junior dev and I'm trying to monkey patch django.contrib.auth.models Group manager. (Python 2.7.15, Django 1.8)

There is my code:

class DefaultGroupManager(models.Manager):
    def get_queryset(self):
        tests = Test.objects.values()
        tests_ids = [test['virtual_group_id'] for test in tests]
        return super(DefaultGroupManager, self).get_queryset().exclude(id__in=tests_ids)


Group.objects = DefaultGroupManager()

Then I open python shell to test it:

python manage.py shell

from django.contrib.auth.models import Group

t = Group.objects.all()

Right after this command i'm getting error:

  File "<console>", line 1, in <module>
  File "/home/adrian/Dokumenty/Pycharm Projects/backend/venv/local/lib/python2.7/site-packages/django/db/models/manager.py", line 228, in all
    return self.get_queryset()
  File "/home/adrian/Dokumenty/Pycharm Projects/backend/registration/models.py", line 13, in get_queryset
    return super(DefaultGroupManager, self).get_queryset().exclude(id__in=tests_ids)
  File "/home/adrian/Dokumenty/Pycharm Projects/backend/venv/local/lib/python2.7/site-packages/django/db/models/query.py", line 686, in exclude
    return self._filter_or_exclude(True, *args, **kwargs)
  File "/home/adrian/Dokumenty/Pycharm Projects/backend/venv/local/lib/python2.7/site-packages/django/db/models/query.py", line 695, in _filter_or_exclude
    clone.query.add_q(~Q(*args, **kwargs))
  File "/home/adrian/Dokumenty/Pycharm Projects/backend/venv/local/lib/python2.7/site-packages/django/db/models/sql/query.py", line 1310, in add_q
    clause, require_inner = self._add_q(where_part, self.used_aliases)
  File "/home/adrian/Dokumenty/Pycharm Projects/backend/venv/local/lib/python2.7/site-packages/django/db/models/sql/query.py", line 1338, in _add_q
    allow_joins=allow_joins, split_subq=split_subq,
  File "/home/adrian/Dokumenty/Pycharm Projects/backend/venv/local/lib/python2.7/site-packages/django/db/models/sql/query.py", line 1150, in build_filter
    lookups, parts, reffed_expression = self.solve_lookup_type(arg)
  File "/home/adrian/Dokumenty/Pycharm Projects/backend/venv/local/lib/python2.7/site-packages/django/db/models/sql/query.py", line 1036, in solve_lookup_type
    _, field, _, lookup_parts = self.names_to_path(lookup_splitted, self.get_meta())
  File "/home/adrian/Dokumenty/Pycharm Projects/backend/venv/local/lib/python2.7/site-packages/django/db/models/sql/query.py", line 246, in get_meta
    return self.model._meta
AttributeError: 'NoneType' object has no attribute '_meta'

I have no idea whats wrong :(

I will appreciate any help!

Community
  • 1
  • 1
Adrian Kurzeja
  • 797
  • 1
  • 11
  • 27

2 Answers2

3

You should not monkey patch like that: Django has some logic in place at the construction of the class, that will "inject" extra data in the fields, managers, etc.

By simply monkey patching, you omit those steps. A manager has however a contribute_to_class method, so you can monkey patch it like:

dgm = DefaultGroupManager()
dgm.contribute_to_class(Group, 'objects')
Group.objects = dgm

That being said, in my opinion it typically results in a lot of troubly if you overwrite the .objects manager, since a lot of Django code will assume that with Group.objects, you get all objects. Furthermore it is still possible that a lot of Django tooling will fetch all Groups, since these might work with the Model._base_manager [Django-doc].

So although it is (probably) not violating any contracts, it will probably result in more trouble than it is worth.

If you run a test, typically you do that in an isolated environment. Furthermore integration tests, typically run on a separate database as well.

Willem Van Onsem
  • 443,496
  • 30
  • 428
  • 555
  • 1
    Also we could also subclass Group model with a proxy model and add extra manager if we want. That way we don't have to monkey patch and use the proxy model instead. – Mohit Solanki Sep 26 '18 at 08:57
  • 1
    @MohitSolanki: that would, in my opinion be a better idea, since overriding `.objects` tends to have bad consequences. This violates the **O** of the SOLID principles. – Willem Van Onsem Sep 26 '18 at 08:58
  • @Mohit Solanki Yeah I thought about it but in my project I just need to to this that way. I'm saving time and avoiding changeing "milions" lines of code. – Adrian Kurzeja Sep 26 '18 at 09:07
2

The manager needs to have it's model attribute set. This is usually done at import time by the Model's metaclass, but since you bypass it, you have to set it yourself:

class DefaultGroupManager(models.Manager):
    model = Group

    def get_queryset(self):
        # this avoids a useless db query + python loop
        tests_ids = Test.objects.values_list('virtual_group_id', flat=True)
        return super(DefaultGroupManager, self).get_queryset().exclude(id__in=tests_ids)

EDIT: stupid me, there are other steps needed for correct initialisation of the manager, which are partly implemented in Manager.contribute_to_class and partly not (sorry, dont have time to investigate this further but you can just read Django source code to find out)... IOW to make it (kind of) "work" you would need:

# You also want to inherit from GroupManager, some code
# relies on extra stuff defined here

from django.contrib.auth.models import GroupManager

class DefaultGroupManager(GroupManager):    
    def get_queryset(self):
        tests_ids = Test.objects.values_list('virtual_group_id', flat=True)
        return super(DefaultGroupManager, self).get_queryset().exclude(id__in=tests_ids)


manager = DefaultManager()
manager.contribute_to_class(Group, "objects")
Group._meta.managers_map["objects"] = manager

BUT: this is brittle at best (actually I can't even garantee it works on Django > 1.10.x), and might not work as you expect for all situations.

bruno desthuilliers
  • 75,974
  • 6
  • 88
  • 118
  • It didn't work for me but thanks for refactored code, you are totally right to avoid unnecessary queries, I'm still learning to make things the easiest as possible, anyway thanks for answer! ;) – Adrian Kurzeja Sep 26 '18 at 09:14