search

thenativeweb / node-cqrs-eventdenormalizer

Node-cqrs-eventdenormalizer is a node.js module that implements the cqrs pattern. It can be very useful as eventdenormalizer component if you work with (d)ddd, cqrs, domain, host, etc.
http://cqrs.js.org/pages/eventdenormalizer.html
MIT License
38 stars 27 forks source link

getLastEventOfEachAggregate, onBeforeSet middleware #90

Open robinfehr opened 4 years ago

robinfehr commented 4 years ago
adrai commented 4 years ago

1) Maybe clarify with some comments, what is the difference between: prefix (https://github.com/adrai/node-cqrs-eventdenormalizer/pull/90/files?file-filters%5B%5D=.js&file-filters%5B%5D=.md#diff-04c6e90faac2675aa89e2176d2eec7d8R100) and prefix (https://github.com/adrai/node-cqrs-eventdenormalizer/pull/90/files#diff-0745fa2041110cff3082322255437920R499)

2) can there be a possibility to optionally omit the prefix and keep the old api? https://github.com/adrai/node-cqrs-eventdenormalizer/pull/90/files#diff-ab712d0ee8b0b73d36fe730eeefa8bebR452 btw. Maybe changing the signature is not needed at all, because the prefix is in the options?

3) add some info to the readme or changelog regarding v2 breaking changes (also for those directly using the store functionality...)

4) docs for getLastEventOfEachAggregate in the readme

nanov commented 4 years ago

can you explain the purpose of the PR a bit? Sorry but, by glance, it seems as you are trying to solve a problem in the wrong domain :/

robinfehr commented 4 years ago

can you explain the purpose of the PR a bit? Sorry but, by glance, it seems as you are trying to solve a problem in the wrong domain :/

we use the even'ts occuredAt date to fetch events that we missed. Our implementation of the queue / bus to our denormalizers are not persistent and therefore it can happen that we miss some events. We then fetch them using a replayer service - common pattern - to which we send a request to retrieve the missed events beginning a certain from date to the last saved event in the store. We have to find the oldest / smallest occuredAt date for all our aggregates in the denormalizer. To do so, having only the revision wihtin the revision guard per aggregate, we'd have to make a query to the eventstore (in our case via the replayer service) for each aggregate and it's revision to retrieve the from date. Putting that traffic onto the denormalizers db is better since it won't affect the evenstore and therefore the rest of the ecosystem. To achieve that the middleware got added, so that we can save the occuredAt or any other data to the denormalizers db and off-load the traffic from the eventstore. The last_seen_event can't be used since the handle fn is per aggregate and therefore an other aggregate can be behind the last_seen_event's time or any other meta data that would be stored there.

robinfehr commented 4 years ago
  1. Maybe clarify with some comments, what is the difference between: prefix (https://github.com/adrai/node-cqrs-eventdenormalizer/pull/90/files?file-filters%5B%5D=.js&file-filters%5B%5D=.md#diff-04c6e90faac2675aa89e2176d2eec7d8R100) and prefix (https://github.com/adrai/node-cqrs-eventdenormalizer/pull/90/files#diff-0745fa2041110cff3082322255437920R499)
  2. can there be a possibility to optionally omit the prefix and keep the old api? https://github.com/adrai/node-cqrs-eventdenormalizer/pull/90/files#diff-ab712d0ee8b0b73d36fe730eeefa8bebR452 btw. Maybe changing the signature is not needed at all, because the prefix is in the options?
  3. add some info to the readme or changelog regarding v2 breaking changes (also for those directly using the store functionality...)
  4. docs for getLastEventOfEachAggregate in the readme

  1. it is the same. the prefix can still be specified within the revisiongard's options. api-wise nothing changed. just the default prefix is now different.

  2. the old api is still intact. the prefix can be omitted. the signature of the stores got changed so that they're agnostic towards the denormalizer's options and threfore can be unit tested properly.

  3. will do :)

  4. will do :)

adrai commented 4 years ago
  1. I mean the store signature... Why not keep the old signature and use this.options.prefix within the function?