Uploaded image for project: 'Doctrine 2 - ORM'
  1. Doctrine 2 - ORM
  2. DDC-193

Remove ClassMetadataInfo#inverseMappings

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: 2.0-ALPHA3
    • Fix Version/s: 2.0-BETA1
    • Component/s: Mapping Drivers, ORM
    • Security Level: All
    • Labels:
      None

      Description

      The ClassMetadataInfo#inverseMappings attribute is an unnecessary complication.

      The problem comes down to: "Given an association $assoc, how do I know if the association is bidirectional?".

      If $assoc is the inverse side ($assoc->isInverseSide()), its clear, its bidirectional. But if $assoc is the owning side, we dont know.
      What we currently do to determine that in such a case looks as follows:

                  if (isset($targetClass->inverseMappings[$assoc->sourceEntityName][$assoc->sourceFieldName])) {
                      // Bi-directional
                  }
      

      So we check whether the target class has an inverse mapping to the source class. This is quite cumbersome. Additionally, we're usually only interested in the sourceFieldName of the inverse association.

      Therefore, it would simplify internal code a lot if we simply introduced a "inversedBy" attribute on an association that is used on the owning side and that corresponds to "mappedBy" on the inverse side. Example:

      /** @Entity */
      class A {
      ...
         /**
          * @OneToOne(targetEntity="Other", inversedBy="a")
          * @JoinColumn(name="other_id", referencedColumnName="id")
          */
         private $other;
      ...
      }
      ...
      /** @Entity */
      class Other {
          /** @OneToOne(targetEntity="A", mappedBy="other") */
          private $a;
      }
      

      This breaks BC. However, I think this is really necessary as it simplifies and speeds up the code, together with removing the inverseMappings attribute altogether.

      So this should happen before beta.

      1. ddc193.patch
        54 kB
        Roman S. Borschel

        Activity

        romanb Roman S. Borschel created issue -
        romanb Roman S. Borschel made changes -
        Field Original Value New Value
        Description
        The ClassMetadataInfo#inverseMappings attribute is an unnecessary complication.

        The problem comes down to: "Given an association $assoc, how do I know if the association is bidirectional?".

        If $assoc is the inverse side ($assoc->isInverseSide()), its clear, its bidirectional. But if $assoc is the owning side, we dont know.
        What we currently do to determine that in such a case looks as follows:

        {code}
                    if (isset($targetClass->inverseMappings[$assoc->sourceEntityName][$assoc->sourceFieldName])) {
                        // Bi-directional
                    }
        {code}

        This is quite cumbersome. Additionally, we're usually only interested in the sourceFieldName of the inverse association.

        Therefore, it would simplify internal code a lot if we simply introduced a "inversedBy" attribute on an association that is used on the owning side and that corresponds to "mappedBy" on the inverse side. Example:

        {code}
        /** @Entity */
        class A {
        ...
           /**
            * @OneToOne(targetEntity="Other", inversedBy="a")
            * @JoinColumn(name="other_id", referencedColumnName="id")
            */
           private $other;
        ...
        }
        ...
        /** @Entity */
        class Other {
            /** @OneToOne(targetEntity="A", mappedBy="other") */
            private $a;
        }
        {code}

        This breaks BC. However, I think this is really necessary as it simplifies and speeds up the code, together with removing the inverseMappings attribute altogether.

        So this should happen before beta.
        The ClassMetadataInfo#inverseMappings attribute is an unnecessary complication.

        The problem comes down to: "Given an association $assoc, how do I know if the association is bidirectional?".

        If $assoc is the inverse side ($assoc->isInverseSide()), its clear, its bidirectional. But if $assoc is the owning side, we dont know.
        What we currently do to determine that in such a case looks as follows:

        {code}
                    if (isset($targetClass->inverseMappings[$assoc->sourceEntityName][$assoc->sourceFieldName])) {
                        // Bi-directional
                    }
        {code}

        So we check whether the target class has an inverse mapping to the source class. This is quite cumbersome. Additionally, we're usually only interested in the sourceFieldName of the inverse association.

        Therefore, it would simplify internal code a lot if we simply introduced a "inversedBy" attribute on an association that is used on the owning side and that corresponds to "mappedBy" on the inverse side. Example:

        {code}
        /** @Entity */
        class A {
        ...
           /**
            * @OneToOne(targetEntity="Other", inversedBy="a")
            * @JoinColumn(name="other_id", referencedColumnName="id")
            */
           private $other;
        ...
        }
        ...
        /** @Entity */
        class Other {
            /** @OneToOne(targetEntity="A", mappedBy="other") */
            private $a;
        }
        {code}

        This breaks BC. However, I think this is really necessary as it simplifies and speeds up the code, together with removing the inverseMappings attribute altogether.

        So this should happen before beta.
        romanb Roman S. Borschel made changes -
        Priority Major [ 3 ] Critical [ 2 ]
        romanb Roman S. Borschel made changes -
        Status Open [ 1 ] In Progress [ 3 ]
        romanb Roman S. Borschel made changes -
        Attachment ddc193.patch [ 10536 ]
        Hide
        romanb Roman S. Borschel added a comment -

        Patch is ready and will be committed within the next week.

        Show
        romanb Roman S. Borschel added a comment - Patch is ready and will be committed within the next week.
        romanb Roman S. Borschel made changes -
        Status In Progress [ 3 ] Closed [ 6 ]
        Resolution Fixed [ 1 ]
        beberlei Benjamin Eberlei made changes -
        Workflow jira [ 10569 ] jira-feedback [ 15502 ]
        beberlei Benjamin Eberlei made changes -
        Workflow jira-feedback [ 15502 ] jira-feedback2 [ 17366 ]
        beberlei Benjamin Eberlei made changes -
        Workflow jira-feedback2 [ 17366 ] jira-feedback3 [ 19623 ]

        This list may be incomplete, as errors occurred whilst retrieving source from linked applications:

        • Request to http://www.doctrine-project.org/fisheye/ failed: Error in remote call to 'FishEye 0 (http://www.doctrine-project.org/fisheye/)' (http://www.doctrine-project.org/fisheye) [AbstractRestCommand{path='/rest-service-fe/search-v1/crossRepositoryQuery', params={expand=changesets[0:20].revisions[0:29],reviews, query=DDC-193}, methodType=GET}] : Received status code 503 (Service Temporarily Unavailable)

          People

          • Assignee:
            romanb Roman S. Borschel
            Reporter:
            romanb Roman S. Borschel
          • Votes:
            1 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: