#982 suggestion
Ashley Moran

Assigning a parent id to a child should load the parent resource/do something

Reported by Ashley Moran | July 19th, 2009 @ 08:30 AM

This came about as an observed violation of principle of least surprise (on the basis that I saw someone in the process of being surprised). I explained that it was not necessary to load a parent resource to create a new child, because you can just do Child.create(:parent_id => parent_id).

To me, there's something not quite right here. What if this new child makes the parent invalid? Then there's a back-door to making a resource invalid.

WDYT about making setting parent ids load the parent resource? Is that a good idea? Could there be any problems? Is there another way of making the behaviour less surprising?

Comments and changes to this ticket

  • Dan Kubb (dkubb)

    Dan Kubb (dkubb) July 19th, 2009 @ 10:23 AM

    • Milestone cleared.
    • State changed from “new” to “suggestion”

    @Ashley: This is a good question.

    There's also a flip side to this, maybe there are children that are only valid if the parent is in a given state, so modifying the parent and then saving it would cause some of it's children to become invalid.

    In general I don't recommend ever referencing the FK directly in creation statements or in queries. It tightly couples the code to database-like storage engines, which I realize is not a huge problem for most people, but the intention with DM is to provide ways to avoid code that is tightly coupled with one set of adapters. I would write this as: Child.create(:parent => parent), or even better parent.children.create.

    With that said, I don't know the answer to your questions. I am sure there are cases where just updating a child could cause the parent to become invalid, or maybe the grandparent or even great-grandparent, etc.

    Maybe there needs to be a way for a model to communicate validation rules across a relationship that apply to target resources? It's an edge case for sure, but off the top of my head I can't think another way to do it without risking loading up the entire object graph.

  • Dan Kubb (dkubb)

    Dan Kubb (dkubb) July 19th, 2009 @ 11:05 AM

    I actually have thought about making the inferred FK properties private, or in the interim public but with deprecation warnings.

    I am sure this will cause alot of issues with people and will be rather controversial so we need to make sure it's the right approach before making the change.

    One other reason I generally don't use the FK properties directly is because depending on where you're getting the data from, you're just trusting that the parent_id maps to a real parent and risking breaking referential integrity if the parent_id is invalid.

    As for communication validation rules, it was just a half-baked idea. I don't even have an API in mind. Basically there'd be a way for the Parent model to tell the target model for the :children association (Child) validations that should be executed when it saves.

    I suppose you could just write those validations in the Child directly, but it doesn't seem right to have a Parent's concerns inside the Child model.

  • Dan Kubb (dkubb)

    Dan Kubb (dkubb) July 20th, 2009 @ 06:36 PM

    This issue comes up a lot. I suspect the answer is to have an object sitting above them both to represent the unit that is parent+children. Should this be a DataMapper Resource? That is something that interests me. I've already done some work on a DataMapper adapter that saves resources to other DataMapper resources, so it's definitely possible. There's something in there somewhere, but I suspect there are more fundamental issues to worry about first. It's definitely an area I need to give more thought to though.

    There is already an object that represents the parent/child relationship called a Relationship object :) Currently a Relationship only points in one direction, so for a typical has/belongs_to pair there are two Relationship objects. Given that this is part of the private API, it will likely undergo further refactoring into a single object that represents the Relationship for both directions. This won't happen until after other parts of DM get more attention and specs (like properties/types and sql query generation).

    The idea will be that the validation is attached to the Relationship, and the models that it connect have access to it, and can execute the validation when one side is being saved.

    There's also a weird case to deal with, in how models are generally loaded by frameworks. Due to the fact that models are either lazily loaded or serially eager loaded, there's a point where a relationship is "dangling" because only one of the models in the relationship is known, while the others aren't. There's also a special case of this where in a many to many, where only the source model might be loaded, but all of the intermediaries and the target model may not loaded yet. In those cases we need to come up with a way of "registering" a relationship so that when the other model is finally loaded, it can check the registry and link any dangling relationships to itself. This needs to happen in a very specific way so that FKs are inferred properly. I know you reported some related issues to this, and (I believe) I solved them by lazily initializing the relationships, but I am still not totally happy with it.. I like how explicit the registry loading process is.

  • Dan Kubb (dkubb)

    Dan Kubb (dkubb) August 12th, 2009 @ 11:54 AM

    @Ashley: There are a few cases where a distinction between validation that requires the adapter involvement or not would be useful.

    For example, if you are sending a query for something like User.all(:nick => nil), where the :nick property does not allow nil, you can short-circuit the query since it will obviously not return any results. For an RDBMS or another fast datastore, this might not make much of a difference, but some adapters (like those backed by an HTTP based API) short-circuiting impossible queries makes a huge difference.

    Any validations that require the adapter are generally several orders of magnitude more expensive than local checks. I think it's useful to split the distinction up even if we weren't doing query short-circuiting -- there's no point in doing a uniqueness check if a value doesn't meet the presence checks, or is an invalid format.

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

People watching this ticket

Pages