#774 switching to new AnnotationDriver

[ppaulis]

Signed-off-by: Pascal Paulis ppaulis@gmail.com

fixes #774
linked to doctrine/orm#9668

[greg0ire]

Isn't this condition supposed to work for Doctrine ODM also? Maybe the previous FQCN should still be used in the condition, and former child classes be imported as well and use in the condition as well?

[greg0ire]

Maybe a more robust way to do this would be to use reflection to get the number of parameters? Something like (new ReflectionClass($class))->getConstructor()->getNumberOfParameters()?

[ppaulis]

@greg0ire About your second proposition, interesting thought, but wouldn't that be a bit "too implicit"? Perhaps prone to future errors?

By using the previous FQCN, won't it be easier to maintain this place in the code, once the already deprecated driver is completely dropped?

[greg0ire]

See #776 for my implementation of this

I think I see what you mean: if there are other drivers with 2 arguments, it might break those. But right now, such drivers are necessarily not handled by the code, since the only other case handles only one argument.

Using the previous FQCN would help with being compatible with previous versions of the ORM, but you would have to add at least 2 lines similar to the one that uses the persistence driver class in the if condition to add support for ORM and ODM… a bit unwieldy, although more explicit. Feel free to update your PR with that.

[ppaulis]

but you would have to add at least 2 lines similar to the one that uses the persistence driver class in the if condition to add support for ORM and ODM

OK, I see. I didn't think about the ODM part. Still a lot to learn about the Doctrine internals. I'll update the PR, and then you guys can decide which version you like better 👍

I also thought about an interface like AnnotationDriverInterface with a corresponding trait containing a setter for the reader. But that would mean changing the constructors. That would certainly be a more complex change.

[ppaulis]

@greg0ire updated the PR and added a test case for all 3 Annotation drivers (Persistence, ODM, ORM). The call to method_exists($driver, 'getReader') is necessary because __invoke() returns an instance of MappingDriver and not the definitive class.

As the deprecated Persistence-version is abstract and no longer parent to any specific driver, I created dummy classes for this one.

As for the doctrine.cache.array error you encountered in #776, this comes from the Options\Driver class where $cache is set to array by default. And so it tries to find a service doctrine.cache.array in the newly created ServiceManager.

Feel free to let me know what you think about this 👍

[derrabus]

Suggested change

	($class === AnnotationDriverPersistence::class || is_subclass_of($class, AnnotationDriverPersistence::class)) ||
	($class === AnnotationDriverORM::class || is_subclass_of($class, AnnotationDriverORM::class)) ||
	($class === AnnotationDriverODM::class || is_subclass_of($class, AnnotationDriverODM::class))
	is_a($class, AnnotationDriverPersistence::class, true) ||
	is_a($class, AnnotationDriverORM::class, true) ||
	is_a($class, AnnotationDriverODM::class, true)

[greg0ire]

Copy pasting my comment from #776 : Now that doctrine/orm#9671 is merged, this MR makes sense only after doctrine/persistence 3 is supported. Reworking the DriverFactory more in depth before adding support might be better than merging this.

Thank you and let's reopen this later if necessary.