[MongoDB] Mercure support #3290
Conversation
|
|
||
| $uow = $eventArgs->getEntityManager()->getUnitOfWork(); | ||
|
|
||
| foreach ($uow->getScheduledEntityInsertions() as $entity) { |
There was a problem hiding this comment.
If the only difference between ODM and ORM is the method name, maybe could we merge all classes and do something like:
$methodName = eventArgs instanceof OdmOnFlushEventArgs ? getScheduledDocumentInsertions : getScheduledEntityInsertions;
foreach ($uow->$methodName() //...WDYT?
There was a problem hiding this comment.
I've no strong opinion about this. Calling a method dynamically is often considered a bad practice. And having a separate class allows to customize things easily without adding conditional statements.
But on the other hand it multiplies the number of classes and adds a complexity by having an inherited abstract class.
|
You think it should target master? I thought we could consider it like a bug fix since we didn't say that Mercure was only for ORM. |
|
It's definitely a new feature :) |
alanpoulain
force-pushed
the
mongodb-mercure
branch
4 times, most recently
from
a3c29f7
to
6a2a4f4
Compare
|
It targets master now. The code is merged inside one class too. I've added another commit in case we want to go back to the previous version (I think I prefer it). |
|
@dunglas are you OK with the current implementation? |
| @@ -8,7 +8,7 @@ | |||
|
|
|||
| <!-- Event listener --> | |||
|
|
|||
| <service id="api_platform.doctrine.listener.mercure.publish" class="ApiPlatform\Core\Bridge\Doctrine\EventListener\PublishMercureUpdatesListener"> | |||
| <service id="api_platform.doctrine.orm.listener.mercure.publish" class="ApiPlatform\Core\Bridge\Doctrine\EventListener\PublishMercureUpdatesListener"> | |||
There was a problem hiding this comment.
It's a BC break isn't it? We should at least add a (deprecated) alias to the old service name.
There was a problem hiding this comment.
Even if it's experimental?
There was a problem hiding this comment.
Please, yes. The class is marked as experimental but not the definition, and I know that Arte rely on this for their doctrine-less implementation.
There was a problem hiding this comment.
And it doesn't hurt. It's just one line to add, that will be dropped in 3.0.
alanpoulain
force-pushed
the
mongodb-mercure
branch
from
94695d3
to
3d77a79
Compare
alanpoulain
force-pushed
the
mongodb-mercure
branch
from
3d77a79
to
5d17bf1
Compare
Not really a new feature nor a bug fix. Mercure was not supported with MongoDB. This PR fixes it.
It seems it's also the case for the
PurgeHttpCacheListener.