Skip to content

Instantly share code, notes, and snippets.

@bmoyles0117
Last active December 17, 2015 11:49
Show Gist options
  • Save bmoyles0117/5604915 to your computer and use it in GitHub Desktop.
Save bmoyles0117/5604915 to your computer and use it in GitHub Desktop.
Creating a sample app for a StackOverflow question, to better illustrate the bigger picture and provide a working proof of concept
from django.db import models
class Car(models.Model):
make = models.CharField(max_length=50)
model = models.CharField(max_length=50)
class User(models.Model):
first_name = models.CharField(max_length=50)
last_name = models.CharField(max_length=50)
cars = models.ManyToManyField(Car)
def __str__(self):
return 'User(first_name=%s, last_name=%s, cars=%s)' % (self.first_name, self.last_name, self.cars.all(), )
class Member(models.Model):
user = models.ForeignKey(User)
def __str__(self):
return 'Member(user=%s)' % (self.user, )
from django.test import TestCase
from .models import Car
from .models import Member
from .models import User
from .utils import from_parent_instance
class TransferTestCase(TestCase):
def setUp(self):
self.car = Car.objects.create(make='Nissan', model='GTR')
self.user = User.objects.create(
first_name = 'Bryan',
last_name = 'Moyles'
)
self.user.cars.add(self.car)
def test_from_parent_instance(self):
member = Member.objects.create(user=self.user)
with self.assertNumQueries(0):
member = from_parent_instance(self.user, member)
self.assertEquals(self.user.first_name, member.first_name)
self.assertEquals([x.id for x in self.user.cars.all()], [x.id for x in member.cars.all()])
def test_distinct_identifiers(self):
user = User.objects.create(
first_name = 'Second',
last_name = 'Person'
)
user.cars.add(self.car)
member = Member.objects.create(user=user)
with self.assertNumQueries(0):
member = from_parent_instance(user, member)
self.assertEquals(user.first_name, member.first_name)
self.assertEquals([x.id for x in user.cars.all()], [x.id for x in member.cars.all()])
from copy import deepcopy
def from_parent_instance(source_instance, target_instance):
single_fields = source_instance._meta.fields
multiple_fields = source_instance._meta.many_to_many
for field in single_fields + multiple_fields:
setattr(target_instance, field.name, deepcopy(getattr(source_instance, field.name)))
return target_instance
@AdrianLC
Copy link

Hello, thanks for the snippet.

It does copy the many to many fields indeed, but... It makes queries, it's actually worse than doing:

return target_instance.objects.get(pk=user.pk)

This is on my test:

with self.assertNumQueries(0):
    member = Member.from_parent_user(user)

And it fails with:

AssertionError: 4 queries executed, 0 expected

I think it happens because of calling all(). Might be possible to deepcopy the whole manager, I'm not sure :/
The problem is, I think, the __setattr__ of the Manager class is overriden to magically assign Querysets to it so calling setattr() on it without the all() is not possible.

@bmoyles0117
Copy link
Author

Actually, why are you calling "all" at all? Because if you think about it, you're saying that member.calls should be user.calls.all(), even though you would still be calling "all" on the members object, so at the end you're going to be doing users.calls.all().all(), which is defeating the purpose. I say nix the all on the copying, and just copy the manager as you've said

@AdrianLC
Copy link

Yeah, the question is how? Would be nice if simply doing setattr(member, field.name, deepcopy(getattr(user, field.name))) worked. I could join both _meta.fields and _meta.many_to_many in a single loop. But it raises the exception :

File "/home/adrian/.virtualenvs/clusters/src/django/django/db/models/fields/related.py", line 837, in __set__
    value = tuple(value)
TypeError: 'ManyRelatedManager' object is not iterable

@bmoyles0117
Copy link
Author

I just updated all of the code to work as well as with the assertions, what version of django are you using? Maybe I can replicate the issue you're having with a different version and help from there :)

@AdrianLC
Copy link

The trunk 1.6 installed from a git clone. Had to upgrade from 1.5.1 because of a tiny bug that is already fixed there but not in the release :/
My model is a bit more complex though. I'll paste it here:

class Member(User):

    """Proxy of Django's default User model."""

    objects = MemberManager()

    class Meta:
        verbose_name = _('member')
        proxy = True

    @classmethod
    def from_parent_user(cls, user):
        # return cls.objects.get(pk=user.pk)
        member = cls()
        single_fields = cls._meta.fields
        for field in single_fields:
            setattr(member, field.name, deepcopy(getattr(user, field.name)))
        multiple_fields = cls._meta.many_to_many
        for field in multiple_fields:
            # seems to work, not sure if its safe:
            # getattr(member, field.name).__dict__.update(getattr(user, field.name).__dict__)
            setattr(member, field.name, deepcopy(getattr(user, field.name)))
        return member

Notice that it is a proxy model inheriting from the parent, I don't have any foreign key as in your test models.

@bmoyles0117
Copy link
Author

You have officially confused me haha... The Member already has access to all of the attributes of the user model being that it is extending it and you've declared that it's a proxy of the parent model. If you do Member.objects.get(id=USER_ID), it will have all of the same attributes as the user already. I feel like I'm missing something key here

@AdrianLC
Copy link

Yeah of course sorry. I have a proxy, because I wanted to add plenty of methods to the User model. I didn't paste them because they shouldn't make a difference for copying.

Now, I can't make use of the methods of my proxy, because django's request always contains an instance of type User, not of type Member.

The thing is I'm building an user-group app which I want to keep compatible with per object permissions from django-guardian. Otherwise I would have used a model as in your approach, with a foreign key, is much simpler.

@bmoyles0117
Copy link
Author

But why do you need to copy the attributes of the user model over to the member model, if they already exist? I completely understand that you want to implement additional functionality onto the Member model, but I need to stress again that all of the attributes are already accessible on the Member model, no copying required. A member instance IS a user instance, with extended functionality. An alternative approach you can take is monkey patching the django user model (just google python monkey patching)

@AdrianLC
Copy link

Now, that I'm reading this: http://django-guardian.readthedocs.org/en/latest/userguide/custom-user-model.html
I think I might change my proxy approach and use a onetoone to User. Otherwise, how could I access my proxy methods from the User instance in my requests? That's the reason I started all this copy thing :/

@bmoyles0117
Copy link
Author

As I just said, monkey patching :) Basically, what you do is something like this

def say_hello(cls):
    print "Hello %s!" % cls

from django.contrib.auth.models import User

User.say_hello = say_hello

user = User.objects.get(id=1)
user.say_hello()

The results will amaze you :) The way you get this to work everywhere, is by importing the module that "monkey patches" the user model, before anything else can use it

@AdrianLC
Copy link

I knew of that and also of contribute_to_class which I think it's basically the same. But I don't like it, too hackish.

The thing is django-guardian builds its per object permissions on top of the user and group models, and I want to extend the groups with management levels, owners, etc. So, my custom group class must extend the builtin group and the memberships of the group must come from a relationship with the main user model, so I can't make use of a onetoone to user and thus I need the proxy, monkey patching or whatever...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment