Doctrine 1
  1. Doctrine 1
  2. DC-21

Option to prevent autoinstantiation of foreign records during get() operations

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Won't Fix
    • Affects Version/s: 1.2.0-ALPHA1
    • Fix Version/s: 1.2.0-ALPHA1
    • Component/s: None
    • Labels:
      None

      Description

      By default, accessing $record->Related automatically instantiates a new related Record object if one doesn't already exist. This is not always the desired behavior, especially in cases where the related record is "optional" (i.e. 1 to 0/1).

      There should be an 'autoInstantiate' option when defining a hasOne() relationship. For backwards compatability, the default behavior should not change. But if one sets 'autoInstantiate' => false, then a non-existing related object must be explicitly created.

      I'm working on a test and patch for this, which I'll attach when I have them.

      1. diff.txt
        27 kB
        Roman S. Borschel

        Activity

        Hide
        Roman S. Borschel added a comment -

        Changed 11 months ago by adamthehutt ¶

        • has_patch set

        I've attached a patch which seems to add this feature without breaking anything (existing tests pass).
        Changed 11 months ago by romanb ¶

        Just as a side note, this behavior will be gone completely in 2.0. No auto"magic" object creations behind the scene. We've had enough "WTF?" experiences and bugs or bogus bugs with the current behavior. The minor convenience this feature offers is not worth it.
        Changed 11 months ago by adamthehutt ¶

        romanb - Do you think this can make it into 1.0.5 or 1.1? Have you had a chance to look at the patch? I'm not positive it does everything it needs to do, but it addresses the primary use case ("hasZeroOrOne" foreign key relations).
        Changed 11 months ago by jwage ¶

        It would only be possible in 1.1 because it is a new feature.
        Changed 10 months ago by jwage ¶

        • description modified (diff)
        • milestone changed from 1.1.0 to 1.2.0

        Can you provide a patch that shows the individual changes?
        Changed 3 months ago by jwage ¶

        • severity set to abc

        Adam, we can add this to 1.2. Maybe you could expand the patch to also include an attribute so you can turn off this behavior globally.

        I looked at your patch and it looks like it was generated weirdly. It shows just a massive remove and then readd.
        Changed 3 months ago by adamthehutt ¶

        I've got a new patch for this that mostly works, but I've run into a snag.

        Ideally this should be separately configurable for each side of the relation. For example:

        • Customer has either 1 or 0 Address records
        • Address must have exactly 1 Customer record

        In this case, I would want $address->Customer to autoinstantiate the Customer, since it must exist. But I wouldn't want $customer->Address to autoinstantiate the Address, since it might not exist.

        The problem is that, as far as I can tell, there is only one object that represents the relation between the two classes. There doesn't seem to be a way to have different settings depending on the direction.

        Am I missing something?

        If there's no way to accommodate the desired behavior I can live with requiring the setting to be bi-directional, although it isn't ideal.
        Changed 3 months ago by jwage ¶

        The auto instantiating creates the object and sets references on both sides. Can you delete the old patch and upload the new one and I can review and see if I can help.
        Changed 3 months ago by adamthehutt

        • attachment diff.txt added

        Changed 3 months ago by adamthehutt ¶

        Okay, I've added a new diff based on my changes. This includes some tests, one of which is failing because of the issue I described above.

        Show
        Roman S. Borschel added a comment - Changed 11 months ago by adamthehutt ¶ has_patch set I've attached a patch which seems to add this feature without breaking anything (existing tests pass). Changed 11 months ago by romanb ¶ Just as a side note, this behavior will be gone completely in 2.0. No auto"magic" object creations behind the scene. We've had enough "WTF?" experiences and bugs or bogus bugs with the current behavior. The minor convenience this feature offers is not worth it. Changed 11 months ago by adamthehutt ¶ romanb - Do you think this can make it into 1.0.5 or 1.1? Have you had a chance to look at the patch? I'm not positive it does everything it needs to do, but it addresses the primary use case ("hasZeroOrOne" foreign key relations). Changed 11 months ago by jwage ¶ It would only be possible in 1.1 because it is a new feature. Changed 10 months ago by jwage ¶ description modified (diff) milestone changed from 1.1.0 to 1.2.0 Can you provide a patch that shows the individual changes? Changed 3 months ago by jwage ¶ severity set to abc Adam, we can add this to 1.2. Maybe you could expand the patch to also include an attribute so you can turn off this behavior globally. I looked at your patch and it looks like it was generated weirdly. It shows just a massive remove and then readd. Changed 3 months ago by adamthehutt ¶ I've got a new patch for this that mostly works, but I've run into a snag. Ideally this should be separately configurable for each side of the relation. For example: Customer has either 1 or 0 Address records Address must have exactly 1 Customer record In this case, I would want $address->Customer to autoinstantiate the Customer, since it must exist. But I wouldn't want $customer->Address to autoinstantiate the Address, since it might not exist. The problem is that, as far as I can tell, there is only one object that represents the relation between the two classes. There doesn't seem to be a way to have different settings depending on the direction. Am I missing something? If there's no way to accommodate the desired behavior I can live with requiring the setting to be bi-directional, although it isn't ideal. Changed 3 months ago by jwage ¶ The auto instantiating creates the object and sets references on both sides. Can you delete the old patch and upload the new one and I can review and see if I can help. Changed 3 months ago by adamthehutt attachment diff.txt added Changed 3 months ago by adamthehutt ¶ Okay, I've added a new diff based on my changes. This includes some tests, one of which is failing because of the issue I described above.

          People

          • Assignee:
            Jonathan H. Wage
            Reporter:
            Roman S. Borschel
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: