[2.0] Merge ClassMetadataInfo into ClassMetadata #1733

carusogabriel · 2018-01-12T17:59:45Z

This closes #1727 by merge ClassMetadataInfo into ClassMetadata as suggested by @alcaeus in #1714

Majkl578 · 2018-01-12T20:05:30Z

tests/Doctrine/ODM/MongoDB/Tests/Mapping/ClassMetadataInfoTest.php

@@ -5,7 +5,7 @@
 use Doctrine\ODM\MongoDB\DocumentRepository;
 use Doctrine\ODM\MongoDB\Events;
 use Doctrine\ODM\MongoDB\Mapping\Annotations as ODM;
-use Doctrine\ODM\MongoDB\Mapping\ClassMetadataInfo;
+use Doctrine\ODM\MongoDB\Mapping\ClassMetadata;


This file and test class should be renamed (or merged with ClassMetadataTest).

Thanks, this is the conflict, I was tring to find. I’ll merge them as we only have ClassMetadata now ☺️

There's still something going wrong, but is with tests, not the merge 😢

alcaeus · 2018-01-15T06:34:45Z

I only checked one test (ClassMetadataTest::testOwningSideAndInverseSide), but the issue is that the original test called ClassMetadataInfo::mapManyReference which doesn't work with the reflection fields and thus didn't fail when a property didn't exist. With the logic combined in one class, the code now errors because we're trying to map a non-existing property. I assume this is the reason for all the failures.

Solution: either create the properties that are causing failing tests or change the failing tests to map existing properties.

carusogabriel · 2018-01-15T12:28:21Z

@alcaeus These are the failing tests in ClassMetadataTest:

testOwningSideAndInverseSide;
testDefaultDiscriminatorField;
testDefaultStorageStrategyOfEmbeddedDocumentFields;
testNoIncrementFieldsAllowedInShardKey;
testNoCollectionsInShardKey;
testNoEmbedManyInShardKey;
testNoReferenceManyInShardKey;

May I ask you to fix them? I barely understand mongodb-odm to fix them 😞

alcaeus · 2018-01-15T12:29:51Z

Sure, I'll take a look at them tonight. I'll push changes to your branch 👍

alcaeus · 2018-01-15T18:48:13Z

@carusogabriel looks like I can't push changes for some reason, so I created https://github.com/carusogabriel/mongodb-odm/pull/1.

carusogabriel · 2018-01-15T18:58:53Z

Strange, but okay, merged 😄

alcaeus

LGTM. Thanks @carusogabriel!

malarzm · 2018-01-16T11:24:57Z

lib/Doctrine/ODM/MongoDB/Mapping/Driver/AnnotationDriver.php

@@ -32,7 +31,7 @@ class AnnotationDriver extends AbstractAnnotationDriver
     */
    public function loadMetadataForClass($className, ClassMetadata $class)


Can we use FQCN for Common's ClassMetadata and get rid of aliasing our own one?

I'd alias the common one. 😇

Sorry for being a layman, but what means FQCN? The full namespaces class?

Fully-Qualified Class Name.

malarzm · 2018-01-16T11:25:57Z

lib/Doctrine/ODM/MongoDB/Mapping/Driver/YamlDriver.php

@@ -32,7 +31,7 @@ public function __construct($locator, $fileExtension = self::DEFAULT_FILE_EXTENS
     */
    public function loadMetadataForClass($className, ClassMetadata $class)


No need to rename here as this driver is removed in #1734 anyway

So I'll wait and rebase!

No need, I first need to tackle documentation. Please change the other places and let's get this merged, I'll rebase my PR later :)

Sorry, you'll need to rebase after all as I've just finished with docs and merged YAML removal ;)

malarzm · 2018-01-16T11:27:57Z

lib/Doctrine/ODM/MongoDB/Tools/Console/Command/GeneratePersistentCollectionsCommand.php

@@ -2,7 +2,7 @@

 namespace Doctrine\ODM\MongoDB\Tools\Console\Command;

-use Doctrine\ODM\MongoDB\Mapping\ClassMetadataInfo;
+use Doctrine\ODM\MongoDB\Mapping\ClassMetadata;


If I'm seeing correctly this is unused, should be removed

I'll see that, thanks!

malarzm · 2018-01-16T11:29:19Z

lib/Doctrine/ODM/MongoDB/Mapping/Driver/XmlDriver.php

@@ -30,7 +29,7 @@ public function __construct($locator, $fileExtension = self::DEFAULT_FILE_EXTENS
     */
    public function loadMetadataForClass($className, ClassMetadata $class)


I'd appreciate getting rid of alias here as well

carusogabriel · 2018-01-18T13:24:25Z

The only thing that should be done is the rebase?

malarzm · 2018-01-18T14:08:33Z

Rebase to solve conflicts and then if you could apply my remarks it'd be great 👍

carusogabriel · 2018-01-18T15:56:25Z

@malarzm 96f2d7b: you were correct 👍

6ff884b: is that you were talking about? I didn't fully understand that 😅

malarzm · 2018-02-04T14:38:46Z

Thanks @carusogabriel, sorry for long merging I must have overlooked notification about last change you did :)

carusogabriel · 2018-02-04T14:45:39Z

@malarzm NP, now, let's finish #1714 🚧

carusogabriel changed the title ~~Class metedata~~ [2.0] Merge ClassMetadataInfo into ClassMetadata Jan 12, 2018

Majkl578 reviewed Jan 12, 2018

View reviewed changes

malarzm added this to the 2.0.0 milestone Jan 12, 2018

malarzm added Task BC Break labels Jan 12, 2018

alcaeus approved these changes Jan 15, 2018

View reviewed changes

alcaeus requested a review from malarzm January 15, 2018 19:24

malarzm reviewed Jan 16, 2018

View reviewed changes

carusogabriel added 15 commits January 18, 2018 13:39

Merge ClassMetadataInfo into ClassMetadata

07f96f8

Use ClassMetadata instead of ClassMetadataInfo in documentation

c99bb56

Use ClassMetadata instead of ClassMetadataInfo in GraphLookup

9520eaa

Use ClassMetadata instead of ClassMetadataInfo in Lookup

4f83847

Use ClassMetadata instead of ClassMetadataInfo in DocumentManager

cd7e9cd

Use ClassMetadata instead of ClassMetadataInfo in HydratorFactory

f50dacc

Use ClassMetadata instead of ClassMetadataInfo in AbstractDriverTest

972adde

Use ClassMetadata instead of ClassMetadataInfo in ClassMetadataInfoTest

dac8f2a

Use ClassMetadata instead of ClassMetadataInfo in Annotations

451a491

Use ClassMetadata instead of ClassMetadataInfo in ClassMetadataFactory

95aee2f

Use ClassMetadata instead of ClassMetadataInfo in AnnotationDriver

bd1c2e0

Use ClassMetadata instead of ClassMetadataInfo in Tests

eb558db

Use ClassMetadata instead of ClassMetadataInfo in lib

2d7a711

Fix conflict

fa46aeb

Merge ClassMetadataInfoTest into ClassMetadataTest

5aa04c2

alcaeus and others added 2 commits January 18, 2018 13:48

Fix failing tests

6565ee2

Removed unused import

96f2d7b

Use own ClassMetadata

265a169

malarzm merged commit fc4c802 into doctrine:master Feb 4, 2018

carusogabriel deleted the class-metedata branch February 4, 2018 14:45

alcaeus added the Needs 1.3 Compat Layer label Jan 8, 2019

alcaeus added a commit to alcaeus/mongodb-odm that referenced this pull request Jan 9, 2019

Deprecate and alias ClassMetadataInfo (doctrine#1733)

cb319d6

alcaeus mentioned this pull request Jan 9, 2019

Add deprecation layers #1924

Merged

alcaeus added a commit to alcaeus/mongodb-odm that referenced this pull request Jan 10, 2019

Deprecate and alias ClassMetadataInfo (doctrine#1733)

34fe949

alcaeus added a commit to alcaeus/mongodb-odm that referenced this pull request Jan 10, 2019

Deprecate and alias ClassMetadataInfo (doctrine#1733)

b796f50

alcaeus removed the Needs 1.3 Compat Layer label Jan 10, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[2.0] Merge ClassMetadataInfo into ClassMetadata #1733

[2.0] Merge ClassMetadataInfo into ClassMetadata #1733

carusogabriel commented Jan 12, 2018

Majkl578 Jan 12, 2018

carusogabriel Jan 12, 2018

carusogabriel Jan 13, 2018

alcaeus commented Jan 15, 2018

carusogabriel commented Jan 15, 2018

alcaeus commented Jan 15, 2018

alcaeus commented Jan 15, 2018

carusogabriel commented Jan 15, 2018

alcaeus left a comment

malarzm Jan 16, 2018

Majkl578 Jan 16, 2018

carusogabriel Jan 16, 2018

jmikola Jan 16, 2018

malarzm Jan 16, 2018

carusogabriel Jan 16, 2018

malarzm Jan 17, 2018

malarzm Jan 17, 2018

malarzm Jan 16, 2018

carusogabriel Jan 16, 2018

malarzm Jan 16, 2018

carusogabriel commented Jan 18, 2018

malarzm commented Jan 18, 2018 •

edited

Loading

carusogabriel commented Jan 18, 2018

malarzm commented Feb 4, 2018

carusogabriel commented Feb 4, 2018

		@@ -32,7 +31,7 @@ class AnnotationDriver extends AbstractAnnotationDriver
		*/
		public function loadMetadataForClass($className, ClassMetadata $class)

		@@ -32,7 +31,7 @@ public function __construct($locator, $fileExtension = self::DEFAULT_FILE_EXTENS
		*/
		public function loadMetadataForClass($className, ClassMetadata $class)

		@@ -30,7 +29,7 @@ public function __construct($locator, $fileExtension = self::DEFAULT_FILE_EXTENS
		*/
		public function loadMetadataForClass($className, ClassMetadata $class)

[2.0] Merge ClassMetadataInfo into ClassMetadata #1733

[2.0] Merge ClassMetadataInfo into ClassMetadata #1733

Conversation

carusogabriel commented Jan 12, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alcaeus commented Jan 15, 2018

carusogabriel commented Jan 15, 2018

alcaeus commented Jan 15, 2018

alcaeus commented Jan 15, 2018

carusogabriel commented Jan 15, 2018

alcaeus left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carusogabriel commented Jan 18, 2018

malarzm commented Jan 18, 2018 • edited Loading

carusogabriel commented Jan 18, 2018

malarzm commented Feb 4, 2018

carusogabriel commented Feb 4, 2018

malarzm commented Jan 18, 2018 •

edited

Loading