Skip to content
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.

Sign up for GitHub

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

feat(graphql): added support for graphql subscriptions to work for actions #6904

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+140 −16
Open

feat(graphql): added support for graphql subscriptions to work for actions #6904

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
251f902
feat(graphql): added support for graphql subscriptions to work for al…
psihius Jan 8, 2025
ead72e1
feat(graphql): adjusted some of the formatting and method placement t…
psihius Jan 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions src/GraphQl/Subscription/SubscriptionIdentifierGenerator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,21 @@ final class SubscriptionIdentifierGenerator implements SubscriptionIdentifierGen
public function generateSubscriptionIdentifier(array $fields): string
{
unset($fields['mercureUrl'], $fields['clientSubscriptionId']);
$fields = $this->removeTypename($fields);
Copy link
Member

Choose a reason for hiding this comment

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

why is this needed?

Copy link
Contributor Author

@psihius psihius Jan 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

When executing a subscription GraphQL query, the __typename key is not present in fields in the SubscriptionProcessor, but when you get fields in doctrine subscriber to publish updates, the __typename is present in the fields. That results in different sha256 hashes as subscription id, which breaks publishing.


return hash('sha256', print_r($fields, true));
}

private function removeTypename(array $data): array
{
foreach ($data as $key => $value) {
if ($key === '__typename') {
unset($data[$key]);
} elseif (is_array($value)) {
$data[$key] = $this->removeTypename($value);
}
}

return $data;
}
}
145 changes: 128 additions & 17 deletions src/GraphQl/Subscription/SubscriptionManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,24 @@ public function __construct(private readonly CacheItemPoolInterface $subscriptio

public function retrieveSubscriptionId(array $context, ?array $result, ?Operation $operation = null): ?string
{

/** @var ResolveInfo $info */
$info = $context['info'];
$fields = $info->getFieldSelection(\PHP_INT_MAX);
$this->arrayRecursiveSort($fields, 'ksort');
$iri = $operation ? $this->getIdentifierFromOperation($operation, $context['args'] ?? []) : $this->getIdentifierFromContext($context);
if (null === $iri) {
if (empty($iri)) {
Copy link
Member

Choose a reason for hiding this comment

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

let's avoid a function call here if this can return an empty array we should fix the return type of the above functions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It also returns an empty string if you provide a value, but it's not a valid URI. That's why I made empty call instead of doing null === $iri || '' === $iri || [] === $iri.
Those 2 methods called above are part of a IdentifierTrait trait, means all call sides will need modification.

return null;
}
$options = $operation->getMercure() ?? false;
$private = $options['private'] ?? false;
$privateFields = $options['private_fields'] ?? [];
$previousObject = $context['graphql_context']['previous_object'] ?? null;
if ($private && $privateFields && $previousObject) {
foreach ($options['private_fields'] as $privateField) {
$fields['__private_field_'.$privateField] = $this->getResourceId($privateField, $previousObject);
}
}
$subscriptionsCacheItem = $this->subscriptionsCache->getItem($this->encodeIriToCacheKey($iri));
$subscriptions = [];
if ($subscriptionsCacheItem->isHit()) {
Expand All @@ -63,26 +73,129 @@ public function retrieveSubscriptionId(array $context, ?array $result, ?Operatio

$subscriptionId = $this->subscriptionIdentifierGenerator->generateSubscriptionIdentifier($fields);
unset($result['clientSubscriptionId']);
if ($private && $privateFields && $previousObject) {
foreach ($options['private_fields'] as $privateField) {
unset($result['__private_field_'.$privateField]);
}
}
$subscriptions[] = [$subscriptionId, $fields, $result];
$subscriptionsCacheItem->set($subscriptions);
$this->subscriptionsCache->save($subscriptionsCacheItem);

$this->updateSubscriptionCollectionCacheData(
$iri,
$fields,
$subscriptions,
);

return $subscriptionId;
}

public function getPushPayloads(object $object): array
public function getPushPayloads(object $object, string $type): array
{
if ('delete' === $type) {
$payloads = $this->getDeletePushPayloads($object);
} else {
$payloads = $this->getCreatedOrUpdatedPayloads($object);
}

return $payloads;
}

/**
* @return array<array>
*/
private function getSubscriptionsFromIri(string $iri): array
{
$subscriptionsCacheItem = $this->subscriptionsCache->getItem($this->encodeIriToCacheKey($iri));

if ($subscriptionsCacheItem->isHit()) {
return $subscriptionsCacheItem->get();
}

return [];
}

private function removeItemFromSubscriptionCache(string $iri): void
{
$cacheKey = $this->encodeIriToCacheKey($iri);
if ($this->subscriptionsCache->hasItem($cacheKey)) {
$this->subscriptionsCache->deleteItem($cacheKey);
}
}

private function encodeIriToCacheKey(string $iri): string
{
return str_replace('/', '_', $iri);
}

private function updateSubscriptionCollectionCacheData(
?string $iri,
array $fields,
array $subscriptions,
): void
{
$subscriptionCollectionCacheItem = $this->subscriptionsCache->getItem(
$this->encodeIriToCacheKey($this->getCollectionIri($iri)),
);
if ($subscriptionCollectionCacheItem->isHit()) {
$collectionSubscriptions = $subscriptionCollectionCacheItem->get();
foreach ($collectionSubscriptions as [$subscriptionId, $subscriptionFields, $subscriptionResult]) {
if ($subscriptionFields === $fields) {
return;
}
}
}
$subscriptionCollectionCacheItem->set($subscriptions);
$this->subscriptionsCache->save($subscriptionCollectionCacheItem);
}

private function getResourceId(mixed $privateField, object $previousObject): string
{
$id = $previousObject->{'get' . ucfirst($privateField)}()->getId();
if ($id instanceof \Stringable) {
return (string)$id;
}
return $id;
}
Comment on lines +147 to +154
Copy link
Member

Choose a reason for hiding this comment

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

This is usually using identifiersExtractor could you explain why you need this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the part that extract the value from an object filed that is used for restrictive updates to be limited to specific context (in my case to a specific owner_id).

It also is extracting a defined field, not an ID, so the name for the method should be changed.
I am all for doing it in a better way :)


private function getCollectionIri(string $iri): string
{
return substr($iri, 0, strrpos($iri, '/'));
}
Copy link
Member

Choose a reason for hiding this comment

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

This is too hackish we need to find a better way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm all ears, I haven't found one :)


private function getCreatedOrUpdatedPayloads(object $object): array
{
$iri = $this->iriConverter->getIriFromResource($object);
$subscriptions = $this->getSubscriptionsFromIri($iri);
if ($subscriptions === []) {
// Get subscriptions from collection Iri
$subscriptions = $this->getSubscriptionsFromIri($this->getCollectionIri($iri));
}

$resourceClass = $this->getObjectClass($object);
$resourceMetadata = $this->resourceMetadataCollectionFactory->create($resourceClass);
$shortName = $resourceMetadata->getOperation()->getShortName();

$mercure = $resourceMetadata->getOperation()->getMercure() ?? false;
$private = $mercure['private'] ?? false;
$privateFieldsConfig = $mercure['private_fields'] ?? [];
$privateFieldData = [];
if ($private && $privateFieldsConfig) {
foreach ($privateFieldsConfig as $privateField) {
$privateFieldData['__private_field_'.$privateField] = $this->getResourceId($privateField, $object);
}
}
Copy link
Member

Choose a reason for hiding this comment

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

I've seen this logic twice, could you use a function to remove a few lines?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A Trait then? This logic is used in 2 different files.


$payloads = [];
foreach ($subscriptions as [$subscriptionId, $subscriptionFields, $subscriptionResult]) {
if ($privateFieldData) {
$fieldDiff = array_intersect_assoc($subscriptionFields, $privateFieldData);
if ($fieldDiff !== $privateFieldData) {
continue;
}
}
$resolverContext = ['fields' => $subscriptionFields, 'is_collection' => false, 'is_mutation' => false, 'is_subscription' => true];
/** @var Operation */
$operation = (new Subscription())->withName('update_subscription')->withShortName($shortName);
$data = $this->normalizeProcessor->process($object, $operation, [], $resolverContext);

Expand All @@ -92,26 +205,24 @@ public function getPushPayloads(object $object): array
$payloads[] = [$subscriptionId, $data];
}
}

return $payloads;
}

/**
* @return array<array>
*/
private function getSubscriptionsFromIri(string $iri): array
private function getDeletePushPayloads(object $object): array
{
$subscriptionsCacheItem = $this->subscriptionsCache->getItem($this->encodeIriToCacheKey($iri));

if ($subscriptionsCacheItem->isHit()) {
return $subscriptionsCacheItem->get();
$iri = $object->id;
$subscriptions = $this->getSubscriptionsFromIri($iri);
if ($subscriptions === []) {
// Get subscriptions from collection Iri
$subscriptions = $this->getSubscriptionsFromIri($this->getCollectionIri($iri));
}

return [];
$payloads = [];
foreach ($subscriptions as [$subscriptionId, $subscriptionFields, $subscriptionResult]) {
$payloads[] = [$subscriptionId, ['type' => 'delete', 'payload' => $object]];
}
$this->removeItemFromSubscriptionCache($iri);
return $payloads;
}

private function encodeIriToCacheKey(string $iri): string
{
return str_replace('/', '_', $iri);
}
}
2 changes: 1 addition & 1 deletion src/GraphQl/Subscription/SubscriptionManagerInterface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,5 @@ interface SubscriptionManagerInterface
{
public function retrieveSubscriptionId(array $context, ?array $result): ?string;

public function getPushPayloads(object $object): array;
public function getPushPayloads(object $object, string $type): array;
Copy link
Member

Choose a reason for hiding this comment

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

this is a public interface, the added argument should be optional to cover the BC layer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok

}
5 changes: 3 additions & 2 deletions src/Symfony/Doctrine/EventListener/PublishMercureUpdatesListener.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ final class PublishMercureUpdatesListener
'topics' => true,
'data' => true,
'private' => true,
'private_fields' => true,
Copy link
Member

Choose a reason for hiding this comment

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

what are private_fields? couldn't find that in the specification.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are fields that are also read into the fields array based on witch the sha256 hash for the subscription id is generated. In the REST side of things this is called "Issuing restrictive updates": https://api-platform.com/docs/core/mercure/#dispatching-restrictive-updates-security-mode
There you just generate a topic prefix, but since topics are not part of GraphQL subscription, we are incorporating value of the field so it generates unique subscription id that works only for that specific combo of private fields.

'id' => true,
'type' => true,
'retry' => true,
Expand Down Expand Up @@ -281,11 +282,11 @@ private function evaluateTopics(array &$options, object $object): void
*/
private function getGraphQlSubscriptionUpdates(object $object, array $options, string $type): array
{
if ('update' !== $type || !$this->graphQlSubscriptionManager || !$this->graphQlMercureSubscriptionIriGenerator) {
if (!$this->graphQlSubscriptionManager || !$this->graphQlMercureSubscriptionIriGenerator) {
return [];
}

$payloads = $this->graphQlSubscriptionManager->getPushPayloads($object);
$payloads = $this->graphQlSubscriptionManager->getPushPayloads($object, $type);

$updates = [];
foreach ($payloads as [$subscriptionId, $data]) {
Expand Down
Loading