Pull requests

#3 Open
Repository
Deleted repository
Branch
default (dffce96b4f26)
Repository
wkornewald
Branch
default

Allow setting an actual object in a ListField(ForeignKey)

Author
  1. Cezar Jenkins
Reviewers
Description

This allows me to do something list this:

from django.contrib.auth.models import Group

group = Group.objects.all()[0]

application = IntranetApplication.objects.create(title = 'Test', groups = [group]) application.save()

I have a pull request in on the mongodb engine to allow querying on the list: https://github.com/django-mongodb-engine/mongodb-engine/pull/83

  • Learn about pull requests

Comments (9)

  1. Cezar Jenkins author

    If I set a field as a ForeignKey I can set it. Say, IntranetApplication.object.create(title = 'Test', group = group) and it will work. It will store the ObjectId() in mongo.

    { "_id" : ObjectId("4eb3eacdbb6933701e000000"), "title" : "Test", "group_id" : ObjectId("4eaef623bb69335d51000000") }
    

    If I set ForeignKey as the sub field in ListField and try to set it as groups = [groups] I will get an exception.

    InvalidId: AutoField (default primary key) values must be strings representing an ObjectId on MongoDB (got u'Group Name' instead)

    In my experience, the expected functionality with a ForeignKey field in the DjangoORM is that I can use the objects in reference. This is how ForeignKey works elsewhere in nonrel and in Django itself.

    When searching about this error I came across more than one person attempting to use ForeignKey in a ListField and having errors and problems. This is part of my attempt to make it work in the way I think people expect it to.

  2. Cezar Jenkins author

    I think you meant ListField(models.ForeignKey(MySecondModel) above? If you pull it directly you just get the list of ids.

    If you just get it as a ForeignKey and pull it. Like how you have above. You will get the model.

    I believe to be in line with how ForeignKey works throughout the rest of the ORM it should pull a list of Objects. That is another patch though.

    class MyModel(models.Model):
        fk = models.ForeignKey(MySecondModel)
        fklist = ListField(ForeignKey(MySecondModel))
    

    MyModel.objects.get(...).fk will give me the instance of MySecondModel MyModel.objects.get(...).fklist will give me a list of ids.

    ForeignKey fields act differently inside the list than they do outside. Which was unexpected to me when I first came across it.

  3. Cezar Jenkins author

    I do agree with you. I can work on the merge so that it goes in and comes out of the DB as a list of objects. (I have a pull request against the engine that lets the filter query use an object).

    Consistency is important, which is why I did this. Outside of a list, a ForeignKey field works with Objects, inside a list, it doesn't. That I believe is the inconsistency here.

  4. Cezar Jenkins author

    That maybe a reason not to do so. I would think though that the way that ForeignKey works in ListField being different from how it works outside ListField is the wrong default behavior. I wouldn't mind a way to just explicitly pull the list of ids.

    The whole idea of a field acting differently because it's in a list hurts my nerd sensibilities.

    1. Flavio Percoco

      I agree with keeping consistency. The thing about iterating a ListField of FKs being costly is a developer issue. If we make listfields manage objects (Normal FK Objects, so, Lazy ones) we'll make them consistent with the rest of the ORM, even though, the patch needs to be reworked.

  5. Cezar Jenkins author

    I think, when the attribute is set, somehow translating it over using filter(pk__in = list_of_fks), might give us something lazy. That would essentially be a queryset.

    I also think, that for those concern with speed, it wouldn't be bad to have a helper field ObjectIdField, a subclass of CharField set to 24 max_length and that verifies that the string conforms to an ObjectId. Just throwing that out there. I only know Mongo, so I don't know how this would affect GAE, etc.

    1. Waldemar Kornewald repo owner

      I just don't think it's worth going down that rabbit hole. It would require too much effort to make this magically work such that we never execute unnecessary queries, never retrieve more entities than necessary, and cache everything automatically. Do we really want to do this just because it's a little bit nicer? It's not worth it.

      1. Cezar Jenkins author

        It's more than nicer. It's how ForeignKey is supposed to work. We would not be adding new functionality, but fixing a bug. I have to agree with the statement that optimization is a developer issue. It doesn't have to be 100% optimized, just as much as reasonable.

        Making sure contribute_to_class attaches correctly will solve a majority of the issues.

        If ForeignKey won't be any more than CharField inside a list then it shouldn't be allowed to be used inside the list. Because it is broken. ForeignKey field is supposed to work with objects. It does that everywhere else in the ORM, except here. If it won't be supported, then it should raise a NotImplimented. In my opinion

        So examples of confusion caused: http://groups.google.com/group/django-non-relational/browse_thread/thread/4bc5a6086415e055 http://groups.google.com/group/django-non-relational/browse_thread/thread/585bf35b7b7dbe53 http://groups.google.com/group/django-non-relational/browse_thread/thread/23f1cd79348c93eb https://bitbucket.org/wkornewald/djangotoolbox/issue/5/listfield-foreignkey-and-self http://stackoverflow.com/questions/7930485/django-nonrel-listfield-lookup-on-mongedb

        I'm willing to put work into the code. I'm working on the contribute_to_class issue right now.

  6. Cezar Jenkins author

    ForeignKey uses contribute_to_class to attach ReverseSingleRelatedObjectDescriptor to the model as the attribute. In a ListField contribute is never called. That is probably fine, since I'm not sure if that is the direction to go since it would be hard to give it something to attach to.

    The object descriptor does other things like trace down what the ForeignKey actually points to in case of inheritance and some other complex situations.