Doctrine PHPCR
  1. Doctrine PHPCR
  2. PHPCR-42

sync the xml/yml mapping driver with the annotation driver

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Labels:
      None

      Description

      especially the locale/version stuff is not yet supported. but also the reference mapping seems to look quite different.

        Activity

        Lukas Kahwe created issue -
        Hide
        David Buchmann added a comment -

        versioning is done now: https://github.com/doctrine/phpcr-odm/pull/96

        but we miss tests for the non-annotation mappings

        Show
        David Buchmann added a comment - versioning is done now: https://github.com/doctrine/phpcr-odm/pull/96 but we miss tests for the non-annotation mappings
        Hide
        David Buchmann added a comment -

        how to proceed:

        Show
        David Buchmann added a comment - how to proceed: Refactor the tests about mapping to have abstract base tests to assert things, and one test for each annotation/yml/xml/php so we really test the same outcome https://github.com/doctrine/phpcr-odm/tree/master/tests/Doctrine/Tests/ODM/PHPCR/Mapping Check with code coverage of https://github.com/doctrine/phpcr-odm/blob/master/lib/Doctrine/ODM/PHPCR/Mapping/Driver/AnnotationDriver.php and https://github.com/doctrine/phpcr-odm/blob/master/lib/Doctrine/ODM/PHPCR/Mapping/ClassMetadata.php if everything is covered Fix missing annotations Also test invalid metadata values or combinations - they should throw the same errors with all mappings Check the documentation for meta data in the readme. add some note about xml/yml/php format (but do not repeat the doc, just explain the differences)
        Hide
        David Buchmann added a comment -

        initial test for annotations is added here, please evolve from this
        https://github.com/doctrine/phpcr-odm/commit/38f8bbc8b655aafd099287e18401248c44b40492

        Show
        David Buchmann added a comment - initial test for annotations is added here, please evolve from this https://github.com/doctrine/phpcr-odm/commit/38f8bbc8b655aafd099287e18401248c44b40492
        Hide
        Luis Cordova added a comment -

        hi dbu, lsmith, i gave thought to this and did some corrections https://github.com/doctrine/phpcr-odm/pull/133

        however I am totally disoriented as i see two Mapping folders as follows:

        1. https://github.com/doctrine/phpcr-odm/tree/master/tests/Doctrine/Tests/ODM/PHPCR/Functional/Mapping
        There is only 1 class here with a TODO which i found not very clear about what to do next

        2. https://github.com/doctrine/phpcr-odm/tree/master/tests/Doctrine/Tests/ODM/PHPCR/Mapping
        These set of classes was already refactored by someone else before I came in and I guess the work is done

        I check the coverage as you said on the annotation driver class so it basically lacks the lifecycle callbacks usage, but where to put these tests in as i am confused


        160 0 : if ($annot instanceof ODM\PrePersist) { 161 0 : $metadata->addLifecycleCallback($method->getName(), Event::prePersist); 162 0 : } elseif ($annot instanceof ODM\PostPersist) { 163 0 : $metadata->addLifecycleCallback($method->getName(), Event::postPersist); 164 0 : } elseif ($annot instanceof ODM\PreUpdate) { 165 0 : $metadata->addLifecycleCallback($method->getName(), Event::preUpdate); 166 0 : } elseif ($annot instanceof ODM\PostUpdate) { 167 0 : $metadata->addLifecycleCallback($method->getName(), Event::postUpdate); 168 0 : } elseif ($annot instanceof ODM\PreRemove) { 169 0 : $metadata->addLifecycleCallback($method->getName(), Event::preRemove); 170 0 : } elseif ($annot instanceof ODM\PostRemove) { 171 0 : $metadata->addLifecycleCallback($method->getName(), Event::postRemove); 172 0 : } elseif ($annot instanceof ODM\PostLoad) { 173 0 : $metadata->addLifecycleCallback($method->getName(), Event::postLoad); 174 0 : }

        Also i am not very familiar with the schema of the annotation driver how to test so any explanation or help pointers would be great, thanks

        Show
        Luis Cordova added a comment - hi dbu, lsmith, i gave thought to this and did some corrections https://github.com/doctrine/phpcr-odm/pull/133 however I am totally disoriented as i see two Mapping folders as follows: 1. https://github.com/doctrine/phpcr-odm/tree/master/tests/Doctrine/Tests/ODM/PHPCR/Functional/Mapping There is only 1 class here with a TODO which i found not very clear about what to do next 2. https://github.com/doctrine/phpcr-odm/tree/master/tests/Doctrine/Tests/ODM/PHPCR/Mapping These set of classes was already refactored by someone else before I came in and I guess the work is done I check the coverage as you said on the annotation driver class so it basically lacks the lifecycle callbacks usage, but where to put these tests in as i am confused 160 0 : if ($annot instanceof ODM\PrePersist) { 161 0 : $metadata->addLifecycleCallback($method->getName(), Event::prePersist); 162 0 : } elseif ($annot instanceof ODM\PostPersist) { 163 0 : $metadata->addLifecycleCallback($method->getName(), Event::postPersist); 164 0 : } elseif ($annot instanceof ODM\PreUpdate) { 165 0 : $metadata->addLifecycleCallback($method->getName(), Event::preUpdate); 166 0 : } elseif ($annot instanceof ODM\PostUpdate) { 167 0 : $metadata->addLifecycleCallback($method->getName(), Event::postUpdate); 168 0 : } elseif ($annot instanceof ODM\PreRemove) { 169 0 : $metadata->addLifecycleCallback($method->getName(), Event::preRemove); 170 0 : } elseif ($annot instanceof ODM\PostRemove) { 171 0 : $metadata->addLifecycleCallback($method->getName(), Event::postRemove); 172 0 : } elseif ($annot instanceof ODM\PostLoad) { 173 0 : $metadata->addLifecycleCallback($method->getName(), Event::postLoad); 174 0 : } Also i am not very familiar with the schema of the annotation driver how to test so any explanation or help pointers would be great, thanks
        Hide
        Luis Cordova added a comment -

        I am so not using jira sorry guys, ---> https://github.com/doctrine/phpcr-odm/pull/133

        Show
        Luis Cordova added a comment - I am so not using jira sorry guys, ---> https://github.com/doctrine/phpcr-odm/pull/133
        Hide
        David Buchmann added a comment -

        @craigmarvelley started working on this, promised a pull request soon

        Show
        David Buchmann added a comment - @craigmarvelley started working on this, promised a pull request soon
        Hide
        Craig Marvelley added a comment -

        I've opened a PR here, there's still a bit left to be done that I'm hoping to add in the next few days.

        https://github.com/doctrine/phpcr-odm/pull/144

        Show
        Craig Marvelley added a comment - I've opened a PR here, there's still a bit left to be done that I'm hoping to add in the next few days. https://github.com/doctrine/phpcr-odm/pull/144
        Hide
        David Buchmann added a comment - - edited

        the pull request by craig has meanwhile been merged.

        there is still a bit of issues:

        the driver looks for the non-namespaced file. i.e. No mapping file found named '/home/david/liip/symfony-cmf/cmf-sandbox/vendor/symfony-cmf/routing-extra-bundle/Symfony/Cmf/Bundle/RoutingExtraBundle/Resources/config/doctrine/Route.phpcr.xml' for class 'Symfony\Cmf\Bundle\RoutingExtraBundle\Document\Route'.

        but if i put that file there and have an error, he tells me Invalid mapping file 'Symfony.Cmf.Bundle.RoutingExtraBundle.Document.Route.phpcr.xml'

        so the name determiner seems to be off a bit

        Show
        David Buchmann added a comment - - edited the pull request by craig has meanwhile been merged. there is still a bit of issues: the driver looks for the non-namespaced file. i.e. No mapping file found named '/home/david/liip/symfony-cmf/cmf-sandbox/vendor/symfony-cmf/routing-extra-bundle/Symfony/Cmf/Bundle/RoutingExtraBundle/Resources/config/doctrine/Route.phpcr.xml' for class 'Symfony\Cmf\Bundle\RoutingExtraBundle\Document\Route'. but if i put that file there and have an error, he tells me Invalid mapping file 'Symfony.Cmf.Bundle.RoutingExtraBundle.Document.Route.phpcr.xml' so the name determiner seems to be off a bit
        Hide
        David Buchmann added a comment -

        also, the mapped superclass feature makes no sense with phpcr-odm, we can simply inherit from document classes.

        Show
        David Buchmann added a comment - also, the mapped superclass feature makes no sense with phpcr-odm, we can simply inherit from document classes.
        Hide
        David Buchmann added a comment -

        actually mapped superclass seems to be relevant, but i did not yet understand it and the doc about it should be updated. see discussion in https://github.com/doctrine/phpcr-odm/pull/144#issuecomment-7095570

        Show
        David Buchmann added a comment - actually mapped superclass seems to be relevant, but i did not yet understand it and the doc about it should be updated. see discussion in https://github.com/doctrine/phpcr-odm/pull/144#issuecomment-7095570
        Hide
        Lukas Kahwe added a comment -

        seems to me like reference/referrer/translation/versioning is all done .. so if there is anything left .. please open a new ticket

        Show
        Lukas Kahwe added a comment - seems to me like reference/referrer/translation/versioning is all done .. so if there is anything left .. please open a new ticket
        Lukas Kahwe made changes -
        Field Original Value New Value
        Status Open [ 1 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]

        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={query=PHPCR-42, expand=changesets[0:20].revisions[0:29],reviews}, methodType=GET}] : Received status code 503 (Service Temporarily Unavailable)

          People

          • Assignee:
            Lukas Kahwe
            Reporter:
            Lukas Kahwe
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: