Skip to content

Automatically dispatch updates to clients using the Mercure protocol #2282

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Nov 8, 2018

Conversation

dunglas
Copy link
Member

@dunglas dunglas commented Oct 26, 2018

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets n/a
License MIT
Doc PR #2282

This PR adds support for the Mercure Protocol to API Platform.
It introduces a simple new attribute called mercure:

<?php

namespace App\Entity;

use ApiPlatform\Core\Annotation\ApiResource;

/**
 * @ApiResource(mercure=true)
 */
class Book
{
    // ...
}

When set, every time an object of this type is created, updates or deleted through the API, the new version is sent to all connected clients through the Mercure hub

Checkout this YouTube video to see how it works with the API Platform client generator: http://www.youtube.com/watch?v=JdlAcfTJsOI

The code of I used for this video is available here: api-platform/demo#66

It's also possible to send the update to users having some targets only:

<?php

namespace App\Entity;

use ApiPlatform\Core\Annotation\ApiResource;

/**
 * @ApiResource(mercure={"a-group", "another-group"})
 */
class Book
{
    // ...
}

Finally, it's possible to execute an expression (using the Expression Language component), to retrieve a dynamic list of targets:

<?php

namespace App\Entity;

use ApiPlatform\Core\Annotation\ApiResource;

/**
 * @ApiResource(mercure="object.owners")
 */
class Book
{
    public $owners = [];
   // ...
}

To enable Mercure support, you must install the brand new Symfony Mercure Bundle, that comes with the Symfony Mercure Component.

Take a look to my Forum PHP's slide deck for further information about Mercure and its support in Symfony and API Platform.

@dunglas dunglas changed the title Mercure Automatically dispatch updates to clients using the Mercure protocol Oct 26, 2018

public function __construct(ResourceClassResolverInterface $resourceClassResolver, IriConverterInterface $iriConverter, ResourceMetadataFactoryInterface $resourceMetadataFactory, SerializerInterface $serializer, MessageBusInterface $messageBus = null, callable $publisher = null, ExpressionLanguage $expressionLanguage = null)
{
if (null === $messageBus && null === $publisher) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

possibly ensure only 1 is provided?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would make the wiring a bit harder.


private function reset(): void
{
$this->createdEntities = new \SplObjectStorage();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this the same as removeAll?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removeAll takes a mandatory argument as parameter.

Copy link
Contributor

@bendavies bendavies Oct 29, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indeed, $this->createdEntities->removeAll($this->createdEntities).

I was wondering if there would be any different from a memory/gc point of view.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No idea :) I would keep it as is for now because it's "safer", but if your suggestion improve GC or something similar, let me know and I'll update!


if (\is_string($value)) {
if (null === $this->expressionLanguage) {
throw new RuntimeException('The Expression Language component is not installed. Try running "composer require symfony/expression-language".');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this the proper place to throw this, or can it be done in the metadata factory somehow?

}

if (!\is_array($value)) {
throw new InvalidArgumentException(sprintf('The value of the "mercure" attribute of the "%s" resource class must be a boolean, an array of targets or a valid expression, "%s" given.', $resourceClass, \gettype($value)));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as above, proper place for this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We currently don't handle metadata validation in the factory, but it would definitely be a better place

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would be nice huh!?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I'll keep it as is because it out of this PR's scope, and I'll try to add a proper validator (for all existing attributes) in a subsequent PR. Is it ok for you?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

of course! sorry, my ?! wasn't meant to come off as rude or aggressive, but enthusiastic!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understood it like that no worries :)

@@ -15,7 +15,7 @@

<tag name="doctrine.event_listener" event="preUpdate" />
<tag name="doctrine.event_listener" event="onFlush" />
<tag name="doctrine.event_listener" event="postFlush" />
<tag name="kernel.event_listener" event="kernel.terminate" method="postFlush" />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, wow ok. has your thinking changed since #1286?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hum, you're right, I'll revert this change as it's not related to this PR. (I forgot about this discussion)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but the discussion point is similar, i guess.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really, to me it's a big deal if a cache is not flushed because you'll serve stale data. Not dispatching a Mercure update is less a problem, because it breaks nothing, and it's explicitly stated in the Mercure protocol that the client MUST call the original API if it wants to be sure to have fresh data. So we can use kernel.terminate safely for Mercure, I'm still unsure for Varnish.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nicely argued.

@@ -145,7 +145,10 @@ private function storeEntityToPublish($entity, string $property): void
}

if ('deletedEntities' === $property) {
$this->deletedEntities[$this->iriConverter->getIriFromItem($entity, UrlGeneratorInterface::ABS_URL)] = $value;
$this->deletedEntities[(object) [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we transform this to a real class? We had issues in the past with StdClass and it's better for debugging.

Copy link
Member Author

@dunglas dunglas Nov 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it worth it? It's a purely internal state.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/api-platform/core/pull/1780/files this was kinda the same, internal state but eventually it hit a bug :p. As you want.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ooo. mine! that was for anonymous classes, not stdClass?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was in the serialization context, and indeed an anonymous class. \stdClass is serializable: https://3v4l.org/2VWST

@dunglas dunglas merged commit 516c9c7 into api-platform:master Nov 8, 2018
@dunglas dunglas deleted the mercure branch November 8, 2018 11:39
soullivaneuh pushed a commit to soullivaneuh/core that referenced this pull request Nov 13, 2018
…pi-platform#2282)

* Automatically dispatch updates to clients using the Mercure protocol

* Allow to use different URL to publish and subscribe

* Fix PHPStan

* Use public repos

* Remove useless arg

* Review

* Revert unrelated change

* Don't deal with absolute IRIs in documents for now
@dunglas dunglas mentioned this pull request Dec 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants