Doctrine\Common metadata drivers reuse #314

Ocramius · 2012-03-30T16:10:26Z

This PR is strictly related with doctrine/common#98, doctrine/common#131 and doctrine/common#150 and tests won't pass until the doctrine-common submodule points to a merged version of it (will do so later, so please don't merge now ).

Basically, I just stripped any code duplicate of what already available in dcom master under Doctrine\Common\Persistence\Mapping\Driver.

Tests are OK on my environment when using the new commons submodule.

(This is a cleanup for #263, where I sadly did pull from the remote branch after rebasing)

stof · 2012-03-30T18:31:14Z

@Ocramius the PR in common has been merged so you can update the submodule to make travis happy

Ocramius · 2012-03-30T18:43:42Z

Purrrrrrrrrrrrrrrfect, will do :)
Thank you!

Ocramius · 2012-03-30T19:42:34Z

Build is passing. Time to get this merged. Please let me know if there is more that should be handled in here.

beberlei · 2012-04-07T07:32:08Z

lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php

Why not extend AbstractClassMetadataFactory? This gets rid of most the code!

When I started working on this PR you told me this would happen somewhen later.. I can include that anyway :)

Ocramius · 2012-04-07T19:34:55Z

Build is not really broken, travis halted somehow on one of the environments...

Ocramius · 2012-04-07T19:43:37Z

lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php

+     */
+    protected function doLoadMetadata($class, $parent, $rootEntityFound)
+    {
+        throw new \LogicException(


Had to introduce this dirty fix because of

https://github.com/doctrine/doctrine2/blob/beae0865dbe7416f2639100c45dcb477b734af9d/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php#L329

https://github.com/doctrine/doctrine2/blob/beae0865dbe7416f2639100c45dcb477b734af9d/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php#L337

These steps cannot be reproduced within doLoadMetadata with the given parameters, and patching Doctrine\Common\Persistence\Mapping\AbstractClassMetadataFactory didn't work either...
Ideas?

asm89 · 2012-05-04T06:58:54Z

@Ocramius Can you rebase (and perhaps squash?) the PR?

@beberlei @guilhermeblanco Can you look at it? I think it's a nice internal refactoring to make it in 2.3. Common is already up to date for this.

Ocramius · 2012-05-04T07:14:06Z

@asm89 I can do, didn't yet find a decent solution for #314 (comment) though.

travisbot · 2012-05-23T22:32:52Z

This pull request fails (merged 305e865f into b32bb26).

Ocramius · 2012-05-23T22:37:28Z

@asm89 rebased, but the PR Still needs doctrine/common#131 and some code could be removed with doctrine/common#138
Still some failures though.

Ocramius · 2012-05-23T23:02:46Z

Failures seem to be completely unrelated with the modified code (maybe because I'm using latest doctrine/common master):

Time: 26 seconds, Memory: 357.75Mb

There was 1 error:

1) Doctrine\Tests\ORM\Functional\QueryCacheTest::testQueryCache_HitDoesNotSaveParserResult
Exception: [PHPUnit_Framework_Error] Object of class Mock_ParserResult_1b2988e1 could not be converted to string

With queries:

Trace:
/home/ocramius/Desktop/doctrine2/lib/vendor/doctrine-common/lib/Doctrine/Common/Cache/CacheProvider.php:143
/home/ocramius/Desktop/doctrine2/lib/vendor/doctrine-common/lib/Doctrine/Common/Cache/CacheProvider.php:73
/home/ocramius/Desktop/doctrine2/lib/Doctrine/ORM/Query.php:217
/home/ocramius/Desktop/doctrine2/lib/Doctrine/ORM/Query.php:241
/home/ocramius/Desktop/doctrine2/lib/Doctrine/ORM/AbstractQuery.php:737
/home/ocramius/Desktop/doctrine2/lib/Doctrine/ORM/AbstractQuery.php:539
/home/ocramius/Desktop/doctrine2/tests/Doctrine/Tests/ORM/Functional/QueryCacheTest.php:148


/home/ocramius/Desktop/doctrine2/tests/Doctrine/Tests/OrmFunctionalTestCase.php:402

--


There were 4 failures:

1) Doctrine\Tests\ORM\Functional\QueryCacheTest::testQueryCache_DependsOnHints
Failed asserting that 2 matches expected 1.

/home/ocramius/Desktop/doctrine2/tests/Doctrine/Tests/ORM/Functional/QueryCacheTest.php:47

2) Doctrine\Tests\ORM\Functional\QueryCacheTest::testQueryCache_NoHitSaveParserResult
Expectation failed for method name is equal to <string:doSave> when invoked at sequence index 1
Parameter 1 for invocation Doctrine\Common\Cache\ArrayCache::doSave('DoctrineNamespaceCacheKey[]', 1, 0) does not match expected value.
Failed asserting that 1 is an instance of class "Doctrine\ORM\Query\ParserResult".

/home/ocramius/Desktop/doctrine2/lib/vendor/doctrine-common/lib/Doctrine/Common/Cache/CacheProvider.php:173
/home/ocramius/Desktop/doctrine2/lib/vendor/doctrine-common/lib/Doctrine/Common/Cache/CacheProvider.php:141
/home/ocramius/Desktop/doctrine2/lib/vendor/doctrine-common/lib/Doctrine/Common/Cache/CacheProvider.php:73
/home/ocramius/Desktop/doctrine2/lib/Doctrine/ORM/Query.php:217
/home/ocramius/Desktop/doctrine2/lib/Doctrine/ORM/Query.php:241
/home/ocramius/Desktop/doctrine2/lib/Doctrine/ORM/AbstractQuery.php:737
/home/ocramius/Desktop/doctrine2/lib/Doctrine/ORM/AbstractQuery.php:539
/home/ocramius/Desktop/doctrine2/tests/Doctrine/Tests/ORM/Functional/QueryCacheTest.php:118

3) Doctrine\Tests\ORM\Functional\ResultCacheTest::testNativeQueryResultCaching
Failed asserting that 2 matches expected 1.

/home/ocramius/Desktop/doctrine2/tests/Doctrine/Tests/ORM/Functional/ResultCacheTest.php:146

4) Doctrine\Tests\ORM\Functional\SQLFilterTest::testQueryCache_DependsOnFilters
Failed asserting that 2 matches expected 1.

/home/ocramius/Desktop/doctrine2/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php:304

FAILURES!
Tests: 1675, Assertions: 5588, Failures: 4, Errors: 1, Skipped: 52.

beberlei · 2012-05-24T10:00:32Z

Have you rebased to latest master?

Ocramius · 2012-05-24T10:02:18Z

@beberlei yep. Will give it a spin when at home again, but I'm pretty sure everything was based on latest master(s) (common/orm)

travisbot · 2012-06-09T14:15:39Z

This pull request passes (merged b5446d9c into 7b75849).

travisbot · 2012-06-10T07:03:06Z

This pull request passes (merged 6dbb1539 into 7b75849).

stof · 2012-06-10T09:53:59Z

lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php

-    /**
-     * @param EntityManager $$em
+     * @param EntityManager $em
+     * @return void


you should remove the @return void here

Ocramius · 2012-06-10T21:26:35Z

@stof thx for stoffing it, I'm fixing asap :)

travisbot · 2012-06-10T21:57:11Z

This pull request passes (merged 6f13b38d into 7b75849).

Ocramius · 2012-06-10T22:37:59Z

@beberlei @guilhermeblanco @asm89 This can be merged

travisbot · 2012-06-20T23:10:56Z

This pull request fails (merged b0759c64 into 9aabdba).

beberlei · 2012-06-21T10:09:02Z

Does this break DoctrineBundle with references to drivers? If yes then we should prepare a commit for thta, otherwise i like it very much and +1 for merg enow

stof · 2012-06-21T10:34:11Z

lib/Doctrine/ORM/Mapping/Driver/DriverChain.php

-     * @param ClassMetadataInfo $metadata
+     * @param ClassMetadata $metadata
+     * @throws MappingException
+     * @return


you should remove this empty @return if the method does not return anything, and fix it otherwise

Will fix later today, but at least this is done :)

stof · 2012-06-21T10:40:20Z

I think it should work properly in DoctrineBundle as the classes have not changed (only the parent classes have been moved, and the methods are the same)

travisbot · 2012-06-21T14:01:08Z

This pull request fails (merged 0c177c75 into 9aabdba).

Ocramius · 2012-06-26T17:56:15Z

@guilhermeblanco rebased

travisbot · 2012-06-26T17:56:36Z

This pull request fails (merged 027d6189 into cc29f85).

…istence\Mapping\Driver\MappingDriver

SimplifiedXmlDriver and SimplifiedYamlDriver are still not valid after this commit

…ver interface

…n\Persistence\Mapping\Driver\FileDriver

…\Driver\AnnotationDriver

Interface has been moved to Doctrine\Common\Persistence\Mapping\Driver\MappingDriver

…g\Driver\FileDriver instead

…tead

…o the old Driver interface

Those exceptions are now in the Common\Persistence\Mapping namespace

…ce as of doctrine/common#150

… and removing duplicate docs.

travisbot · 2012-07-04T19:40:11Z

This pull request fails (merged 379e698 into f20a95f).

Doctrine\Common metadata drivers reuse

Ocramius mentioned this pull request Mar 30, 2012

File driver as orm behavior doctrine/common#98

Merged

beberlei reviewed Apr 7, 2012
View reviewed changes

Ocramius reviewed Apr 7, 2012
View reviewed changes

Ocramius mentioned this pull request Jun 9, 2012

Updating interface to allow passing parent class names to the process of... doctrine/common#150

Merged

stof reviewed Jun 10, 2012
View reviewed changes

stof reviewed Jun 21, 2012
View reviewed changes

Ocramius added 22 commits July 4, 2012 20:59

Updating mapping driver interface to comply with Doctrine\Common\Pers…

e6a2bae

…istence\Mapping\Driver\MappingDriver

Reducing code duplication

34bb0c4

SimplifiedXmlDriver and SimplifiedYamlDriver are still not valid after this commit

Removing unused code

e9f23d5

Emulating feature that has to be moved to Doctrine\Common

bb998d1

Fixing reflection exceptions caused by changes in the AbstractFileDri…

82cfda3

…ver interface

Removing API that should be moved to common

03fb734

Moving exception throwing for invalid file mappings to Doctrine\Commo…

ccace8c

…n\Persistence\Mapping\Driver\FileDriver

Removing code duplication, reusing Doctrine\Common\Persistenc\Mapping…

abd3ddc

…\Driver\AnnotationDriver

Removing unused imports

5b97357

Removing Doctrine\ORM\Mapping\Driver\Driver interface

905acf9

Interface has been moved to Doctrine\Common\Persistence\Mapping\Driver\MappingDriver

Removing Doctrine\ORM\Mapping\Driver\Driver interface

e9e36dc

Interface has been moved to Doctrine\Common\Persistence\Mapping\Driver\MappingDriver

Removing AbstractFileDriver, using Doctrine\Common\Persistence\Mappin…

245718c

…g\Driver\FileDriver instead

Adding notes about upgrading to 2.3 (metadata drivers changes)

5c05a43

Removing code from the simplified drivers, use SymfonyFileLocator ins…

01b1b0b

…tead

Rebasing on master

737b74e

Fixing broken tests because of wrong method signature and reference t…

63fbf7c

…o the old Driver interface

Fixing test expecting a ORM\Mapping exception

943cff6

Those exceptions are now in the Common\Persistence\Mapping namespace

Reusing Doctrine\Common's AbstractClassMetadataFactory

e522391

Fixing code sniff issues before continuing refactoring of

94714db

Updating ClassMetadataFactory to comply with new abstract CMF interfa…

f6381d7

…ce as of doctrine/common#150

Stoffing and including changes of doctrine/common#138

97d7cae

Reusing chained driver now available in common, fixing cs/code smells…

379e698

… and removing duplicate docs.

guilhermeblanco added a commit that referenced this pull request Jul 4, 2012

Merge pull request #314 from Ocramius/dcom-metadata-drivers-reuse

6ba205f

Doctrine\Common metadata drivers reuse

guilhermeblanco merged commit 6ba205f into doctrine:master Jul 4, 2012

Ocramius mentioned this pull request Jul 4, 2012

Remove dependency to Doctrine\ORM from DoctrineModule doctrine/DoctrineModule#44

Closed

doctrinebot mentioned this pull request Dec 6, 2015

DDC-1739: [GH-314] [WIP] Doctrine\Common metadata drivers reuse #2389

Closed

greg0ire mentioned this pull request Apr 27, 2022

Reference ClassMetadaInfo less #9691

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Doctrine\Common metadata drivers reuse #314

Doctrine\Common metadata drivers reuse #314

Ocramius commented Mar 30, 2012

stof commented Mar 30, 2012

Ocramius commented Mar 30, 2012

Ocramius commented Mar 30, 2012

beberlei Apr 7, 2012

Ocramius Apr 7, 2012

Ocramius commented Apr 7, 2012

Ocramius Apr 7, 2012

asm89 commented May 4, 2012

Ocramius commented May 4, 2012

travisbot commented May 23, 2012

Ocramius commented May 23, 2012

Ocramius commented May 23, 2012

beberlei commented May 24, 2012

Ocramius commented May 24, 2012

travisbot commented Jun 9, 2012

travisbot commented Jun 10, 2012

stof Jun 10, 2012

Ocramius commented Jun 10, 2012

travisbot commented Jun 10, 2012

Ocramius commented Jun 10, 2012

travisbot commented Jun 20, 2012

beberlei commented Jun 21, 2012

stof Jun 21, 2012

Ocramius Jun 21, 2012

stof commented Jun 21, 2012

travisbot commented Jun 21, 2012

Ocramius commented Jun 26, 2012

travisbot commented Jun 26, 2012

travisbot commented Jul 4, 2012

Doctrine\Common metadata drivers reuse #314

Doctrine\Common metadata drivers reuse #314

Conversation

Ocramius commented Mar 30, 2012

stof commented Mar 30, 2012

Ocramius commented Mar 30, 2012

Ocramius commented Mar 30, 2012

beberlei Apr 7, 2012

Choose a reason for hiding this comment

Ocramius Apr 7, 2012

Choose a reason for hiding this comment

Ocramius commented Apr 7, 2012

Ocramius Apr 7, 2012

Choose a reason for hiding this comment

asm89 commented May 4, 2012

Ocramius commented May 4, 2012

travisbot commented May 23, 2012

Ocramius commented May 23, 2012

Ocramius commented May 23, 2012

beberlei commented May 24, 2012

Ocramius commented May 24, 2012

travisbot commented Jun 9, 2012

travisbot commented Jun 10, 2012

stof Jun 10, 2012

Choose a reason for hiding this comment

Ocramius commented Jun 10, 2012

travisbot commented Jun 10, 2012

Ocramius commented Jun 10, 2012

travisbot commented Jun 20, 2012

beberlei commented Jun 21, 2012

stof Jun 21, 2012

Choose a reason for hiding this comment

Ocramius Jun 21, 2012

Choose a reason for hiding this comment

stof commented Jun 21, 2012

travisbot commented Jun 21, 2012

Ocramius commented Jun 26, 2012

travisbot commented Jun 26, 2012

travisbot commented Jul 4, 2012