prooph / proophessor

Exploring prooph components
http://getprooph.org
BSD 3-Clause "New" or "Revised" License
85 stars 23 forks source link

Naming convention #20

Open prolic opened 9 years ago

prolic commented 9 years ago

Hi, first of all, many thanks for this great library. Expect some PR's in near future from me. (MongoDB EventStore Adapter could be the first!)

On thing I noticed during code review:

class Prooph\Proophessor\EventStore\AbstractRepositoryFactory

The name hints for an abstract class, but it's an abstract service factory (it should not get extended). So a better class name would be RepositoryAbstractFactory instead of AbstractRepositoryFactory.

Merely a small detail regarding the whole thing ;)

Keep on doing great stuff!

codeliner commented 9 years ago

Hi, good catch. I will change that.

PR's are always welcome and a mongoDB adapter is something I have on my todo for long time now. Problem with mongoDB is the document size of 16 megabytes. I think the only good option for mongoDB is to use the AggregateStreamStrategy or work with snapshots and clean up old events. But yeah, an adapter would be great :+1: I will create a repo for the adapter.

Btw I've seen your HumusAmqpModule :+1: Another point on my todo is a RabbitMQ message dispatcher for prooph/service-bus :-)

prolic commented 9 years ago

I could also imagine building that amqp integration for you. But first of all, I need to do some improvements on the amqp module itself and release 1.0.0 ;-)

prolic commented 9 years ago

For the mongodb adapter: I can also imagine modelling the documents slightly different. Instead of having all events mapped as embedded objects of the aggregate, simply persist all events to a distinct event collection. I can't imagine that you would store more than 16MB of references for a single entity, but even then, we have some choices left ;-)

codeliner commented 9 years ago

:+1: I'm looking forward to the implementation. I will create the repo tomorrow and post the link here. Now it's time to shutdown my system :-)

Thank you for your help and input. I really appreciate it.

codeliner commented 9 years ago

@prolic empty mongodb adapter repo is online: https://github.com/prooph/event-store-mongodb-adapter

prolic commented 9 years ago

Great thanks!

Two questions come into my mind first:

a) Do we make a pure mongodb adapter or a doctrine-mongodb adapter? I really like the abstraction doctrine-mongodb provides! b) MongoDB does not support transactions, but it has atomic modifiers and ttl documents, so enough tools to build transactions manually. For the event stream a transactional behavior would be useful, right? I think of something like this:

- Start transaction sets a property into the adapter that transaction is running
- During in-transaction persistence we add a "inTransaction":true property on the document.
- All findById and findAll and similar queries check for the inTransaction property
- After transaction is finished, we can remove the inTransaction property with an atomic modifier to enable the new state
- An entry marked "inTransation" has a ttl (time-to-life) on the server side, in case the process dies, we don't want to have pending transactions in the database, so they get cleaned up after some time.

If we do something like this, which TTL will we use? MySQL InnoDB has a innodb_lock_wait_timeout of 50 secs by default f.e.

About maximum document size: We can perhabs have 2 adapters? The first one has the 16MB per document limitation, but will work slightly faster. The second one stores a reference to a mongo grid fs entry and has no limitations at all.

codeliner commented 9 years ago

@prolic I've split your questions into issues ^^ so that we can discuss them individually

codeliner commented 7 years ago

crazy, this issue is still not closed. We should keep it open for historic reasons :D