proophessor icon indicating copy to clipboard operation
proophessor copied to clipboard

Naming convention

Open prolic opened this issue 10 years ago • 8 comments
trafficstars

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!

prolic avatar May 23 '15 20:05 prolic

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 :-)

codeliner avatar May 23 '15 21:05 codeliner

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 avatar May 23 '15 22:05 prolic

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 ;-)

prolic avatar May 23 '15 22:05 prolic

:+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 avatar May 23 '15 22:05 codeliner

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

codeliner avatar May 24 '15 18:05 codeliner

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.

prolic avatar May 25 '15 10:05 prolic

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

codeliner avatar May 25 '15 13:05 codeliner

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

codeliner avatar Oct 12 '17 17:10 codeliner