prooph / common Goto Github PK

We tried phpspec with proophessor and it turned out to be a great testing tool.

• Align MessageHeader
• Provide default Query message

Hi,
I practiced using prooph messages. And I had cases where payload was not required at all. The name of the message was enough to perform the desired action.

Can we make it optional in PayloadTrait?

ATM it is up to the user what datetime format is used when converting a message object into a PHP array and back.

However, we should force the format Y-m-d\TH:i:s.uO in the interfaces as well as in the DomainMessage implementation

Is there any reason why the ramsey/uuid library is not updated to the latest version?

The RemoteMessage is not really required because the DomainMessage includes the message type so a remote message dispatcher can distinguish between commands and events based on the message type.

Arrays are copied instead of passed by reference. If you ensure that the array only contains arrays and scalar types and you pass such an array to an object that only allows read access you easily get an immutable object (like our message classes)
so far so good, BUT:

$json = "{}";
$assoc = json_decode($json, true);
if ($json !== json_encode($assoc)) {
  echo "Empty assoc becomes empty json array\n";
}

$stdClassObj = json_decode($json);
if ($json === json_encode($stdClassObj)) {
  echo "Empty stdclass obj becomes empty json object\n";
}

This means, that I cannot have a message object that is both: immutable and 100% correctly json encodable/decodable at the same time except I use some kind of deep copy mechanism when passing data into the message and out. I can image the latter influences performance.

What can we do to improve the situation?

As requested on Gitter, I'm creating an issue with a BC breaking change I'd like to suggest for prooph v8.

In my setup all messages are simple value objects with properties and methods. They don't hold an array of values internally, nor they are able to convert themselves to an array. Such responsibilities belong to MessageConverter::convertToArray() and MessageFactory::createMessageFromArray().

Because of that I'd like to remove these methods in Prooph v8:

• Message::payload()
• DomainMessage::setPayload()
• DomainMessage::fromArray()
• DomainMessage::toArray()

fromArray and toArray don't need any replacement in my opinion - MessageConverter and MessageFactory have that covered.

payload and setPayload should go into separate interface named PayloadMessage or MessageWithPayload. Default implementations of MessageConverter and MessageFactory would trow an exception for messages not implementing PayloadMessage - the user needs to implement their own array conversion logic for no-payload messages.

I've already made the necessary changes in PDO event store: prooph/pdo-event-store#153

See prooph/event-store-mongodb-adapter#6 for details

Current MessageConverter actually doesn't convert the createdAt field but returns a DateTimeImmutable object, which means that in order to send via network you have to convert it again to string.

I would like to propose that the createdAt field will get converted to string by default with format 'Y-m-d\TH:i:s.u\Z

Thoughts?

/cc @fritz-gerneth @enumag @basz

I would like to propose that the messageName() method is abstract by default (which means you have to implement it on every class). The MessageFactory hence needs a message map, in order to create an object from message data, because the messageName doesn't have to be the class name. Class name as messsage name by default is removed.

Reasons:

• for prooph/event-store-client we need messageMap anyway
• it allows custom message names by default, lots of users asked on how not to have class name as message name and needed to write custom message factories in order to do this. Even with blog posts about this out there, the question came again and again. Actually nobody I'm aware of wants to have the class name as message name, hence why not change it now?

Thoughts?

@enumag @fritz-gerneth @basz

No need for an own ServiceLocator interface. The Interop\ContainerInterface gets a lot of attraction and will likely become a new PSR. We should support it by using it whenever a component needs to load dependencies. Removing the ServiceLocator interface forces our components to align accordingly.

An own event dispatcher helps us decoupling from ZF2.
The prooph components should work without any third party framework in the future!

From Gitter earlier:

Sascha-Oliver Prolic
@fritz-gerneth you should be able to decide one day to integrate rabbitmq with a few simple steps, you shouldn't need to rewrite big parts of your application. Also you shouldn't know if the message is sent over the wire or not. Your stuff is supposed to just work, no matter what infrastructure is on the other side. Allowing objects in payload for most of the time (and sometimes not) only makes much bigger problems on the other end. I understand that you know your infrastructure and that you don't send messages somewhere else, but your limited situation is no reason for me to make the thing much more complex. Also how would you store events payload in the event store? This at least is a common problem with objects already, even if all your consumers would be PHP.

[...]
Fritz Gerneth
but I think the correct behavor would be that the default factories / converter throw the exception, not the message itself. that's what happening most of the time anway already. aka it should not the message to care about that but the infrastructure level. the default one does not allow objects, that's probably easier for everyone

Use-Case:

• Event contains a composable VO (tree like structure with many different implementations of a shared composable interface)
• Currently needs static mapping from array to object structure, very closed for extension, very open for modification, breaks modular approach and extensibility
• Handle built-in types better (e.g. date)

My stance on this:

• Messages itself should allow objects as payload
• Validation should only be done on the infrastructure level
  -- MessageConverter / MessageFactgory implementations are responsible for this
  -- For the event store, the event store strategy is responsible for this
• Default behvaior should still be to throw exceptions
• This allows custom MessageConverters / .. capable of handling objects in the payload

Alternatives:

• Re-implement command / event / query message types
• Fix inconsistent behavior that messages can be created with objects in payload but fail validation when metadata is updated (which triggers payload validation, payload validation should be checked on set)

RFC for prooph/common 4.0

• remove \Prooph\Common\Event\ActionEventListener

reason: callable is just fine enough, this would lead to strict type hints and return values. No messing up with phpdoc and callable|ActionEventListener and all these additional checks in a method like:

if (! $listener instanceof ActionEventListener && !is_callable($listener)) {

thoughts? /cc @codeliner @sandrokeil @bweston92

and use after_success instead of after_script for it

The MessageFactory should replace the static factory method DomainMessage::fromRemoteMessage and allow custom implementations which may use a message serializer / hydrator to build commands / events from arbitrary data.

Emitter is the common term used for an object which emits events.

It makes the concept behind action events more clear and also difference between action events and domain events

The new abstract class should only define a abstract public function getPayload() without providing an implementation.

The base classes Command and DomainEvent should implement the getPayload message.

Furthermore, DomainMessage should be immutable and provide methods like andPayload, andMetadata to add additional data to a new instance of the class.

I'm aware this project will be deprecated, but I was wondering why would we not allow objects in the event's payload.

In my opinion the event object should be able to hold domain objects and get them serialised when persisting it or queueing it.

https://github.com/prooph/common/blob/master/src/Messaging/MessageDataAssertion.php#L71

Am I wrong in this assumption?

See prooph/event-store#45 for details

• Check proophessor dependencies
• Check prooph LINK dependencies
• Do we get BC problems when changing it?

[email protected] instead of [email protected]

prooph/common provides an own event manager implemenation. The ZF2 version should only be included in proophessor which integrates with ZF2.

PSR\Logger support will be included in the next version of ZF so no need to keep an own version in prooph/common.

Namespace did change, we should require the new version.

Depending on discussion of #47 we should upgrade to ramsey/uuid: ^3.0 with next major release.

Note: Other prooph components must also be updated.

Allow use of ramsey/uuid ^4.0