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

Automatically dispatch updates to clients using the Mercure protocol #2282

Merged
merged 8 commits into from Nov 8, 2018

Conversation

Projects
None yet
4 participants
@dunglas
Member

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 todo

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 referenced this pull request Oct 26, 2018

Open

Mercure demo #66

@dunglas dunglas changed the title from Mercure to 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) {

This comment has been minimized.

@bendavies

bendavies Oct 29, 2018

Contributor

possibly ensure only 1 is provided?

This comment has been minimized.

@dunglas

dunglas Oct 29, 2018

Member

It would make the wiring a bit harder.

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

This comment has been minimized.

@bendavies

bendavies Oct 29, 2018

Contributor

is this the same as removeAll?

This comment has been minimized.

@dunglas

dunglas Oct 29, 2018

Member

removeAll takes a mandatory argument as parameter.

This comment has been minimized.

@bendavies

bendavies Oct 29, 2018

Contributor

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

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

This comment has been minimized.

@dunglas

dunglas Nov 5, 2018

Member

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".');

This comment has been minimized.

@bendavies

bendavies Oct 29, 2018

Contributor

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)));

This comment has been minimized.

@bendavies

bendavies Oct 29, 2018

Contributor

as above, proper place for this?

This comment has been minimized.

@dunglas

dunglas Oct 29, 2018

Member

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

This comment has been minimized.

@bendavies

bendavies Oct 29, 2018

Contributor

would be nice huh!?

This comment has been minimized.

@dunglas

dunglas Nov 5, 2018

Member

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?

This comment has been minimized.

@bendavies

bendavies Nov 5, 2018

Contributor

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

This comment has been minimized.

@dunglas

dunglas Nov 5, 2018

Member

I understood it like that no worries :)

dunglas added some commits Oct 23, 2018

@@ -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" />

This comment has been minimized.

@bendavies

bendavies Nov 5, 2018

Contributor

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

This comment has been minimized.

@dunglas

dunglas Nov 5, 2018

Member

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

This comment has been minimized.

@bendavies

bendavies Nov 5, 2018

Contributor

but the discussion point is similar, i guess.

This comment has been minimized.

@dunglas

dunglas Nov 5, 2018

Member

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.

This comment has been minimized.

@bendavies

bendavies Nov 5, 2018

Contributor

nicely argued.

dunglas added some commits Nov 5, 2018

@@ -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) [

This comment has been minimized.

@soyuka

soyuka Nov 7, 2018

Member

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

This comment has been minimized.

@dunglas

dunglas Nov 7, 2018

Member

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

This comment has been minimized.

@soyuka

soyuka Nov 7, 2018

Member

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.

This comment has been minimized.

@bendavies

bendavies Nov 7, 2018

Contributor

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

This comment has been minimized.

@dunglas

dunglas Nov 7, 2018

Member

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

3 of 4 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
Scrutinizer Analysis: 1 new issues, 11 updated code elements – Tests: passed
Details
SymfonyInsight Code quality OK.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@dunglas dunglas deleted the dunglas:mercure branch Nov 8, 2018

Soullivaneuh added a commit to Soullivaneuh/core that referenced this pull request Nov 13, 2018

Automatically dispatch updates to clients using the Mercure protocol (a…
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment