Entity listeners are now processed by exporters #5864
Conversation
The current implementation of the exporters are not taking the entity listeners into account. I have added test cases for most of the edge cases I could think of and implemented the Exporter handling. Again there are some test cases failing but I cannot see how they are related.
|
+1 |
|
+1-ing issues serve no purpose besides making me want to archive email notifications: don't do it, please. |
| @@ -82,6 +82,16 @@ public function exportClassMetadata(ClassMetadataInfo $metadata) | |||
| } | |||
| } | |||
|
|
|||
| if (0 !== count($metadata->entityListeners)) { | |||
There was a problem hiding this comment.
To be moved to a private method
| @@ -389,6 +389,27 @@ public function exportClassMetadata(ClassMetadataInfo $metadata) | |||
| } | |||
| } | |||
|
|
|||
| if (0 !== count($metadata->entityListeners)) { | |||
There was a problem hiding this comment.
To be moved to one or multiple private methods
| @@ -214,6 +214,23 @@ public function exportClassMetadata(ClassMetadataInfo $metadata) | |||
| $array['lifecycleCallbacks'] = $metadata->lifecycleCallbacks; | |||
| } | |||
|
|
|||
| if (0 !== count($metadata->entityListeners)) { | |||
There was a problem hiding this comment.
To be moved to one or multiple private methods
| $array['entityListeners'][$entityListener['class']] = null; | ||
| } | ||
|
|
||
| if (isset($entityListener['method'])) { |
There was a problem hiding this comment.
Are there scenarios where 'method' is not defined?
| foreach ($entityListenerConfig as $entityListener) { | ||
| $class = $entityListener['class']; | ||
| $method = $entityListener['method']; | ||
| $lines[] = "\$metadata->addEntityListener('$event', '$class', '$method');"; |
There was a problem hiding this comment.
Please don't interpolate. Use sprintf() instead. Also, please do use var_export() on values that end up being in PHP code.
| * @Table(name="cms_users",options={"engine"="MyISAM","foo"={"bar"="baz"}}) | ||
| * @EntityListeners({ | ||
| * "Doctrine\Tests\ORM\Tools\Export\UserListener", |
There was a problem hiding this comment.
Can we use the ::class syntax here?
|
@Ocramius I have been browsing my open PRs (of which I have forgotten half of them). I would actually finish this if you think it makes sense and would be worth it. |
|
@tPl0ch this is more a question for @guilhermeblanco, who just rewrote most of the metadata stuff. @guilhermeblanco is this kind of exporter API still existing in 3.x? |
|
@Ocramius it does, since this is part of convert mapping support (transition from yml to xml for example) |
The current implementation of the exporters are not taking the entity listeners into account. I have added test cases for most of the edge cases I could think of and implemented the Exporter handling. This PR originates from doctrine#5864, I was overwhelmed by the amount of conflicts I had to resolve so I started anew on a clean master HEAD.
|
@guilhermeblanco The sheer amount of conflicts drove me creating a new PR for this hotfix: #6593 I'll close this one here. |
The current implementation of the exporters are not taking the entity listeners into account. I have added test cases for most of the edge cases I could think of and implemented the Exporter handling. This PR originates from doctrine#5864, I was overwhelmed by the amount of conflicts I had to resolve so I started anew on a clean master HEAD.
The current implementation of the exporters are not taking the entity listeners into account. I have added test cases for most of the edge cases I could think of and implemented the Exporter handling. This PR originates from doctrine#5864, I was overwhelmed by the amount of conflicts I had to resolve so I started anew on a clean master HEAD. Squashed commits: - Code review aftermath - Add even more return type declarations - Added `return null` to methods declared with nullable return types
The current implementation of the exporters are not taking the entity listeners into account. I have added test cases for most of the edge cases I could think of and implemented the Exporter handling. This PR originates from doctrine#5864, I was overwhelmed by the amount of conflicts I had to resolve so I started anew on a clean master HEAD. Squashed commits: - Code review aftermath - Add even more return type declarations - Added `return null` to methods declared with nullable return types - Removed unneeded docblocks when types are self-explanatory
…pe changes That's still to be considered a BC break, since child classes are broken if incompatible. Sorry @tPl0ch :-(
The current implementation of the exporters are not taking the entity
listeners into account. I have added test cases for most of the edge
cases I could think of and implemented the Exporter handling.
Again there are some test cases failing but I cannot see how they
are related.