-
Notifications
You must be signed in to change notification settings - Fork 3
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
Introduce Flavours #108
Introduce Flavours #108
Conversation
ping @sandrokeil @martin-schilling @enumag @sebastianblum @prolic @gquemener @camuthig /cc @Ocramius I remember you said if one finds a way to get rid of command handler boilerplate they should tell you. Well, here we go ^^ ;) |
use Prooph\EventMachine\Util\DetermineVariableType; | ||
use Prooph\EventMachine\Util\MapIterator; | ||
|
||
final class OOPAggregateCallInterceptor implements CallInterceptor, MessageFactoryAware |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should always use camel case names e.g. OopAggregateCallInterceptor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree because you used acronyms like Json
and Html
therefore I also think it should be Oop
for consistency reasons. Just so that acronyms are handeled in a similar way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
|
||
private function mapToMessage($event, string $aggregateType, Message $command): Message | ||
{ | ||
if (! \is_array($event) || ! \array_key_exists(0, $event) || ! \array_key_exists(1, $event) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of using \is_array
we should import it use function is_array;
and so on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please not. I really prefer it this way so you easily see that it is a function available in global namespace.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this defined in a PSR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it comes from Zend Coding Standard (seen in Zend\Diactoros). It was only a suggestion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, if it is not defined in a PSR I vote against it. It looks a bit nicer, but I see it like @prolic All the additional imports at the top of the file for no real benefit. And especially for exceptions it is very useful to see directly if a PHP standard exception is thrown or a project or namespace specific exception is used.
PHPStorm does not import global namespace by default, too. I think that's configurable, but it's not the default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please use the \ only, this is the Symfony way and I think blackfire.io did a benchmark and it is much faster
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it would be slower if you imported the function using use function is_array;
. To my knowledge the reason it is slower is because PHP has to look in the local namespace and afterwards in the global namespace for a given function if you omit the backslash. But this should not be the case if you explicitly import the function into the local namespace and it should therefore not matter performance wise. That said I also do not like importing the functions either and the backslash makes it pretty clear that you're referring to something in the global namespace
I would leave like this. You don't want to have 50 imports in a class
additionally.
…On Sat, Nov 10, 2018, 18:57 Alexander Miertsch ***@***.*** wrote:
***@***.**** commented on this pull request.
------------------------------
In src/Runtime/StandardCallInterceptor.php
<#108 (comment)>
:
> + public function prepareNetworkTransmission(Message $message): Message
+ {
+ return $message;
+ }
+
+ /**
+ * ***@***.***}
+ */
+ public function convertMessageReceivedFromNetwork(Message $message, $receivedFromEventStore = false): Message
+ {
+ return $message;
+ }
+
+ private function mapToMessage($event, string $aggregateType, Message $command): Message
+ {
+ if (! \is_array($event) || ! \array_key_exists(0, $event) || ! \array_key_exists(1, $event)
is this defined in a PSR?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#108 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAYEvMJMbhTkvm8RVYp225Fv2R3CplvLks5utrEZgaJpZM4YXo5b>
.
|
I'm thinking about renaming the feature. CallInterceptor is soo boring and I'm not sure if it is even correct naming from a technical point of view. Just was the first thing I had in mind. For marketing reasons I'd call the feature Flavour Event Machine - The world's only CQRS / ES framework, that lets you pick your Flavour
|
This PR introduces a new concept called Flavour to Event Machine.
A
Flavour
can be used to hook into communication between Event Machine and the userland implementation.Three different Flavours are shipped with Event Machine to support different programming styles. Finally one can implement the
Flavour
interface to customize it even more.PrototypingFlavour
The
PrototypingFlavour
is used by default if Event Machine cannot find a service with idEventMachine::SERVICE_ID_FLAVOUR
in the container. This Flavour behaves just like Event Machine did before. You can upgrade without any change to an existing project. I've changed message +factory interfaces a bit, but this should not cause trouble as long as you don't use your ownMessageFactory
with Event Machine.FunctionalFlavour
The
FunctionalFlavour
allows you to use your own messages instead of generic Event Machine messages. Custom messages don't need to implement an interface or follow a convention. Use whatever you want to use. The only thing you have to do: implementProoph\EventMachine\Runtime\Functional\Port
, pass it to aFunctionalFlavour
and register the Flavour in the container used by Event Machine using the service id:EventMachine::SERVICE_ID_FLAVOUR
.Take a look at
examples/FunctionalFlavour
. The directory contains an example implementation of a port, custom messages and aUserDescription
that uses custom messages in handle and apply functions.OopFlavour
The
OopFlavour
is for OOP lovers :D . Not everybody likes pure functions + immutable state. Hence, I decided to finally add full featured OOP support to Event Machine. What you need is theOopFlavour
with an Implementation ofProoph\EventMachine\Runtime\Oop\Port
+ a workingFunctionalFlavour
that is used by theOopFlavour
internally. If you have all ingredients just register theOopFlavour
as serviceEventMachine::SERVICE_ID_FLAVOUR
in the container and enjoy nice and clean event sourced aggregates without the need to write command handlers or repositories. Event Machine is still there to help you with the annoying parts of event sourcing.See
examples/OopFlavour
for a reference implementation. Please note that commands and events are reused fromexamples/FunctionalFlavour
, but instead of user functions and immutableUser\State
we now have aUser
aggregate that handles state internally. But again, no interface needed, no traits and no conventions. For the example I use a very simple aggregate Implementation with a public apply method and so on. But all this is under full control of the developer due to their implementation ofProoph\EventMachine\Runtime\Oop\Port
.Tests
Functionality is tested with
EventMachineFunctionalFlavourTest
andEventMachineOopFlavourTest
.Todo
Please note: Since the new feature does not introduce BC breaks for standard event machine implementations, I've decided to add it before v1.0. It will be tagged once the feature is merged. After 1.0 I start development of v2.0. The new version will replace prooph/event-sourcing and prooph/service-bus with Event Machine specific implementations. This will reduce the call stack and complexity insight Event Machine and pave the way for new features.
Closes #107