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.

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

Context stamp #3157

Merged
merged 5 commits into from
Oct 14, 2019
Merged

Context stamp #3157

merged 5 commits into from
Oct 14, 2019

Conversation

sergepavle
Copy link
Contributor

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #3082
License MIT
Doc PR api-platform/docs#1234

sergepavle added a commit to sergepavle/core that referenced this pull request Oct 6, 2019
sergepavle added a commit to sergepavle/core that referenced this pull request Oct 6, 2019
sergepavle added a commit to sergepavle/core that referenced this pull request Oct 6, 2019
sergepavle added a commit to sergepavle/core that referenced this pull request Oct 6, 2019
Copy link
Member

@dunglas dunglas left a comment

Choose a reason for hiding this comment

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

Awesome! Thank you for this Pull Request! Just few more changes and it will be ready to be merged.

src/Bridge/Symfony/Messenger/ContextStamp.php Outdated Show resolved Hide resolved
src/Bridge/Symfony/Messenger/ContextStamp.php Outdated Show resolved Hide resolved
tests/Bridge/Symfony/Messenger/ContextStampTest.php Outdated Show resolved Hide resolved
tests/Bridge/Symfony/Messenger/ContextStampTest.php Outdated Show resolved Hide resolved
@@ -56,7 +57,9 @@ public function testPersist()
$dummy = new Dummy();

$messageBus = $this->prophesize(MessageBusInterface::class);
$messageBus->dispatch($dummy)->willReturn(new Envelope($dummy))->shouldBeCalled();
$messageBus->dispatch(Argument::that(function (Envelope $envelope) use ($dummy) {
return $dummy === $envelope->getMessage() && null !== $envelope->last(ContextStamp::class);
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 change necessary?

Copy link
Contributor Author

@sergepavle sergepavle Oct 6, 2019

Choose a reason for hiding this comment

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

This changes is consistent with code from testRemove().
By this changes we make sure that have envelop with ContextStamp.

@dunglas dunglas merged commit 1931f16 into api-platform:master Oct 14, 2019
@dunglas
Copy link
Member

dunglas commented Oct 14, 2019

Thanks @sergepavle! Could you also open a docs PR? At least, adding an example of how to access the context using a middleware would be nice!

@teohhanhui
Copy link
Contributor

teohhanhui commented Oct 17, 2019

This seems to make sense at first glance, but actually it doesn't work for the stated use case:

I expect that all input DTO's (commands\queries) will be passed to their handlers

(It's most likely still useful for some cases.)

You're not supposed to receive an Envelope in your message handler, e.g.:

<?php

declare(strict_types=1);

namespace App\MessageHandler\Api\Subscription;

use App\Model\Api\Subscription\PaymentDetails;
use Symfony\Component\Messenger\Envelope;
use Symfony\Component\Messenger\Handler\MessageHandlerInterface;

final class PaymentDetailsHandler implements MessageHandlerInterface
{
    // wtf?
    // public function __invoke(Envelope $envelope): void
    public function __invoke(PaymentDetails $paymentDetails): void
    {
        // ...
    }
}

See symfony/symfony#31075

@dunglas
Copy link
Member

dunglas commented Oct 17, 2019

You need to create a middleware. It’s why we must add docs!

@teohhanhui
Copy link
Contributor

Creating a middleware still would not help the stated use case at all. 😄

warslett pushed a commit to BiffBangPow/core that referenced this pull request Oct 18, 2019
* Issue api-platform#3082: Add and use ContextStamp.

* Issue api-platform#3082: Add tests.

* Issue api-platform#3157: Correct passing of context.

* Issue api-platform#3157: Minor corrections.
norkunas pushed a commit to norkunas/core that referenced this pull request Dec 2, 2019
* Issue api-platform#3082: Add and use ContextStamp.

* Issue api-platform#3082: Add tests.

* Issue api-platform#3157: Correct passing of context.

* Issue api-platform#3157: Minor corrections.
@entermix
Copy link

entermix commented Jan 12, 2021

@dunglas, what about this use case?

#3082 (comment)

This is similar to what @teohhanhui writes about.

For example, how can I implement the command to update/delete a resource without using middleware? It's impossible?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants