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

initial mongo db adapter #4

Merged
merged 24 commits into from
Aug 9, 2015
Merged

Conversation

prolic
Copy link
Member

@prolic prolic commented Aug 6, 2015

No description provided.

prolic added a commit to prolic/proophessor that referenced this pull request Aug 6, 2015
@prolic
Copy link
Member Author

prolic commented Aug 6, 2015

@codeliner This is the first intial commit. Should be working for payloads smaller than 16 MB.

More to come these days... ;)

'version' => $e->version(),
'event_name' => $e->messageName(),
'event_class' => get_class($e),
'payload' => Serializer::serialize($e->payload(), $this->serializerAdapter),
Copy link
Member

Choose a reason for hiding this comment

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

Do we need the serializer for mongoDb? DomainEvent::payload must only contain scalar types and arrays so we can persist the payload as is. This would allow querying events with specific payload. Something that is not supported by the event store but custom "event replay scripts" may want to make use of such a feature.

Copy link
Member Author

Choose a reason for hiding this comment

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

ok

@codeliner
Copy link
Member

@prolic Many thanks for the PR. Great work! 👍
I have two questions (see comments) and we need to add at least one unit test ;)

Currently, I'm thinking about how to better support custom domain events with prooph/event-store 5.x. See prooph/event-store#49. What do you thing about the idea? It would have a high impact on the ES adapters because they would no longer handle DomainEvent objects but instead plain arrays.
Serialization would happen insight the Stream class (maybe by using the hydrator concept used in ZF2).
An adapter would be able to pass a \Traversable result to the Steam. In case of mongoDb a MongoCursor (if I remember correctly?).
Do you know if the MongoCursor provides the possiblity to rewind it?

@prolic
Copy link
Member Author

prolic commented Aug 7, 2015

The MongoCursor implements the Iterator interface and therefore has a rewind() method.

@prolic
Copy link
Member Author

prolic commented Aug 7, 2015

tests this evening ;)

@prolic
Copy link
Member Author

prolic commented Aug 7, 2015

If you got time, let's meet in IRC (zftalk.dev) or skype, if you have.

@codeliner
Copy link
Member

@prolic zftalk,dev is maybe not the right place because prooph/event-store is not a zend component but I set up gitter a few hours ago :)
This is the public channel: https://gitter.im/prooph/improoph
I also invited you as member to the prooph organisation (as maintainer of the mongodb adapter ;)). If you accept I can invite you to the private gitter prooph room too.

@prolic
Copy link
Member Author

prolic commented Aug 8, 2015

@codeliner I implemented transaction handling and made some additional changes, too. Please check and tell me what you think of it.

@prolic
Copy link
Member Author

prolic commented Aug 8, 2015

Question: Do we need support for nested transaction?

@prolic
Copy link
Member Author

prolic commented Aug 8, 2015

Next trouble I see coming: The backend should treat all Dates as UTC. The frontend is required to set the appropriate timezone.
Currently we use "$dateTime = new DateTime()", which relies on php's internal timezone setting. If you query the data from another server with different timezone settings, you get wrong DateTimes for the created_at property of an event.

@prolic
Copy link
Member Author

prolic commented Aug 8, 2015

100% coverage done.

codeliner added a commit that referenced this pull request Aug 9, 2015
@codeliner codeliner merged commit 1401fd3 into prooph:master Aug 9, 2015
@codeliner
Copy link
Member

@prolic 👍 great implementation. I like the transaction handling + TTL.
And storing the stream_name as a property of the event solves the issue with the 16MB limit, right?
One feature I can think of would be to store events in different collections based on the stream_name to not have all events in one big event collection. Would multiple event collections have advantages over one big collection when dealing with huge amount of events?

I merge the PR and create new issues for your and my questions. Thanks for the great work.

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

Successfully merging this pull request may close these issues.

2 participants