add projection example for multiple aggregates #16

svalaskevicius · 2015-10-27T23:39:10Z

No description provided.

svalaskevicius · 2015-10-29T17:30:50Z

@Fristi what do you think? can you spot anything that could be improved easily?

svalaskevicius · 2015-11-01T15:30:18Z

build.sbt

 libraryDependencies += "com.chuusai" %% "shapeless" % "2.2.5"

 resolvers += Resolver.sonatypeRepo("releases")

 addCompilerPlugin("org.spire-math" %% "kind-projector" % "0.6.3")

-wartremoverErrors ++= Warts.allBut(Wart.Any, Wart.Nothing, Wart.Throw, Wart.IsInstanceOf, Wart.AsInstanceOf, Wart.NoNeedForMonad)
+// wartremoverErrors ++= Warts.allBut(Wart.Any, Wart.Nothing, Wart.Throw, Wart.IsInstanceOf, Wart.AsInstanceOf, Wart.NoNeedForMonad)


TODO: this is removed as there are lots of errors when using upickle. Options so far:

investigate how to ignore specifically upickle errors

swap upickle with an alternative lib

disable wart remover :(

Decouple encoding/decoding of events? So we let our users pick their own serialization lib

Yep, that should be already possible :)

I wonder if the wart remover was a good idea to have.. It works very well for the code in this repo but also limits the libraries we can use and a couldn't find a good way to disable it for implicit lookups from ext libs...

svalaskevicius · 2015-11-01T15:42:37Z

src/main/scala/Cqrs/Projection.scala

-import InMemoryDb._
-
-import scala.collection.immutable.TreeMap
+import DbAdapters.InMemoryDb._


TODO, referencing the actual db adapter is still bad here - extract consumeEvents?

svalaskevicius · 2015-11-01T15:47:15Z

Hi @Fristi, any feedback on this version?

currently the projection can be replayed from the start (events ordered by their creation) regardless of where in the process was it added.

there are still some items to refactor, esp direct usage of the inMemoryDb in projections

svalaskevicius · 2015-11-01T15:50:52Z

src/main/scala/Domain/Counter.scala

-    }
-  }
+  def emptyCounterProjection = Projection.empty[Data](TreeMap.empty, List(
+    (Counter.tag, createEventDataConsumer( (d: Data, t: Tag, id: AggregateId, v: Int, e: Counter.Event) => {


TODO: create intermediate type here

extract serialisation to a level above DB adapter operations and leave DB only for storing / retrieving strings?

Fristi · 2015-11-01T18:22:23Z

src/main/scala/Cqrs/Aggregate.scala

-  final case class ReadAggregateExistence[E](id: AggregateId) extends EventDatabaseOp[E, Error Xor Boolean]
-  final case class ReadAggregate[E](id: AggregateId, fromVersion: Int) extends EventDatabaseOp[E, Error Xor List[VersionedEvents[E]]]
-  final case class AppendAggregateEvents[E](id: AggregateId, events: VersionedEvents[E]) extends EventDatabaseOp[E, Error Xor Unit]
+  final case class ReadAggregateExistence[E](tag: Tag, id: AggregateId) extends EventDatabaseOp[E, Error Xor Boolean]


In akka-persistence events get a persistenceId (which is a aggregateId) and zero or more tags. In these cases you have to supply both. It would be nice to read events by a specific tag or a aggregateId. When writing you should write them along with one aggregatedId, but zero or more tags. Checking if a aggregate exists should only be done with a aggregateId I think?

The semantics of the tags in this PR are a bit different from akka - its more like aggregate class - eg all counters, or all doors. Checking if exists also needs it in this case - "is there a counter (by tag) identified by this id".

Projections tell what classes / tags of aggregates they are interested in and receive all aggregates within them

I see, makes abit sense. However, how would you read all events of a counter for example without knowing all the aggregateIds? case class ReadAggregate[E](tag: Tag, id: AggregateId, fromVersion: Int) requires you to specify a AggregateId

for this, there is currently a separate Database method as defined by the typeclass - https://github.com/svalaskevicius/eventflow/blob/multiEventsProjection/src/main/scala/Cqrs/Database.scala#L12

its not available in the aggregate actions as no aggregate should ever need that.. I think :)

Fristi · 2015-11-01T18:31:02Z

Added some comments here and there.

Overall I need to take a closer look what all constructs are doing conceptually.

svalaskevicius · 2015-11-03T00:10:44Z

src/main/scala/Eventflow.scala

@@ -82,7 +80,12 @@ object Eventflow {
        fold(err => { println("Error occurred: " + err._2); err._1 }, r => { println("OK"); r._1 })

      @SuppressWarnings(Array("org.brianmckenna.wartremover.warts.OptionPartial", "org.brianmckenna.wartremover.warts.ExplicitImplicitTypes"))
-      val _ = pprint.pprintln(db_, colors = pprint.Colors.Colored)
+      val _ignore1 = pprint.pprintln(runner1, colors = pprint.Colors.Colored)


interesting that after some commit in this PR the pprint lost colours :/ (now its just showing toString result)

svalaskevicius · 2015-11-06T00:04:11Z

ok, looks a bit better now I hope :)

@Fristi can you spot anything that has to be fixed before merging?

Fristi · 2015-11-06T12:09:00Z

build.sbt

 libraryDependencies += "com.chuusai" %% "shapeless" % "2.2.5"

 resolvers += Resolver.sonatypeRepo("releases")

 addCompilerPlugin("org.spire-math" %% "kind-projector" % "0.6.3")

-wartremoverErrors ++= Warts.allBut(Wart.Any, Wart.Nothing, Wart.Throw, Wart.IsInstanceOf, Wart.AsInstanceOf, Wart.NoNeedForMonad)


No more wartremover? :(

I like wartremover.. when it works, but it seems to restrict libraries we can use conveniently at this point.. e.g. uPickle fails quite dramatically and I couldnt find a way to selectively disable it (as it fails when looking for implicits)..

do you have any ideas how can I restore it? (should I use another serialisation library?)

Fristi · 2015-11-06T12:32:24Z

src/main/scala/lib/foldM.scala

+
+import scala.language.higherKinds
+
+object foldM {


Isn't this already in cats somewhere? :)

sadly no... there is an open PR for it - https://github.com/non/cats/pull/356 but its on hold

Ah cool, let's address that later then!

Fristi · 2015-11-06T12:42:00Z

Overall I think it's nice to get projections out there! There are some things we could improve here :) Sometimes I find it not easy to digest some of you code. How could we improve on that?
I think with a few modifications we can clarify it a lot, what do you think?

Private/public or local functions. If a function does not need public scope, make it local (if you need it for recursive loop or something) or make it a private one if you use it multiple times. This makes modules a bit easier to digest.
Don't use tuples in public API's, instead use case classes. They can describe what the values in the tuples are by a good name. I essence case classes are tuples, but with names :)
Use descriptive parameter names in case classes and functions. In the example I saw you use parameters names like k, does not really reveal what the intent of parameter is
Rather use (implicit e: Backend[T]) for implicits if you use it multiple times. Makes the code more concise (and easier to read), since you don't have to write implictly each time.
For concepts like Backend or EventDataConsumerQuery it doesn't reveal it's intention immediately. Would be nice to introduce some docs maybe?
Group concepts and put them in packages or separate files.

I can help you out with refactoring some parts! For now it would be good to merge this 👍

svalaskevicius · 2015-11-06T14:10:34Z

completely agree! :) in fact, I thought I am already doing most of those items :D apart from short names and no docs.

happy to update the missed parts - in the end, I will have to read this as well even after I forget how it was implemented :)

will have a skim over again and see what have I missed, thanks!

svalaskevicius · 2015-11-06T17:12:27Z

@Fristi are these places that you meant - 8a34878 ? happy to improve more if you see something obvious I missed again :)

will merge - but we can always improve this in other PRs (and I hope we will :) )

and thanks for the review!

add projection example for multiple aggregates

add projection example for multiple aggregates

4de0642

svalaskevicius added the in progress label Oct 27, 2015

remove duplication

02a2fbe

svalaskevicius self-assigned this Oct 31, 2015

serialise in db backend

185943a

svalaskevicius added the WIP - not to be merged yet label Oct 31, 2015

svalaskevicius added 4 commits October 31, 2015 15:37

Merge remote-tracking branch 'origin/master' into multiEventsProjection

2b8bf0b

simplify batch runner

a346153

tag db entries by aggregate type

8c0343e

use safer types

3d5140b

This was referenced Nov 1, 2015

Replace HList with shapeless' HList #17

Closed

improve projection replay #18

Closed

svalaskevicius added 2 commits November 1, 2015 15:14

allow projections to be re-run from scratch in the same order

989b04d

fix duplicate aggregate version on initialisation

a4b11cb

svalaskevicius reviewed Nov 1, 2015
View reviewed changes

remove debug log

c7006f6

svalaskevicius reviewed Nov 1, 2015
View reviewed changes

remove duplication

eff39d4

Fristi reviewed Nov 1, 2015
View reviewed changes

svalaskevicius added 3 commits November 2, 2015 22:35

extract db typeclass

49bcf7c

add missing file, propagate failures

f6d04f3

extract foldM

8592ee7

svalaskevicius force-pushed the multiEventsProjection branch from a0ae9bd to 8592ee7 Compare November 3, 2015 00:07

svalaskevicius reviewed Nov 3, 2015
View reviewed changes

svalaskevicius added 4 commits November 5, 2015 23:11

option => xor[error, ?]

7ea8c6e

add colours back

d0fc3e2

remove wart remover

6d5464e

improve api

f047983

svalaskevicius removed the WIP - not to be merged yet label Nov 6, 2015

svalaskevicius added 2 commits November 6, 2015 10:55

improve projection building - disallow partial projections

814f113

run scalariform

9409d8e

svalaskevicius mentioned this pull request Nov 6, 2015

Scalacheck for aggregates and projections #22

Merged

Fristi reviewed Nov 6, 2015
View reviewed changes

rename functions for clarity

181d191

Fristi reviewed Nov 6, 2015
View reviewed changes

rename functions for clarity + add docs

8a34878

svalaskevicius pushed a commit that referenced this pull request Nov 6, 2015

Merge pull request #16 from svalaskevicius/multiEventsProjection

344f0bb

add projection example for multiple aggregates

svalaskevicius merged commit 344f0bb into master Nov 6, 2015

svalaskevicius removed the in progress label Nov 6, 2015

svalaskevicius deleted the multiEventsProjection branch November 6, 2015 17:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add projection example for multiple aggregates #16

add projection example for multiple aggregates #16

svalaskevicius commented Oct 27, 2015

svalaskevicius commented Oct 29, 2015

svalaskevicius Nov 1, 2015

Fristi Nov 1, 2015

svalaskevicius Nov 1, 2015

svalaskevicius Nov 1, 2015

svalaskevicius Nov 3, 2015

svalaskevicius commented Nov 1, 2015

svalaskevicius Nov 1, 2015

Fristi Nov 1, 2015

svalaskevicius Nov 1, 2015

Fristi Nov 6, 2015

svalaskevicius Nov 6, 2015

Fristi commented Nov 1, 2015

svalaskevicius Nov 3, 2015

svalaskevicius commented Nov 6, 2015

Fristi Nov 6, 2015

svalaskevicius Nov 6, 2015

Fristi Nov 6, 2015

svalaskevicius Nov 6, 2015

Fristi Nov 6, 2015

Fristi commented Nov 6, 2015

svalaskevicius commented Nov 6, 2015

svalaskevicius commented Nov 6, 2015

add projection example for multiple aggregates #16

add projection example for multiple aggregates #16

Conversation

svalaskevicius commented Oct 27, 2015

svalaskevicius commented Oct 29, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

svalaskevicius commented Nov 1, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Fristi commented Nov 1, 2015

Choose a reason for hiding this comment

svalaskevicius commented Nov 6, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Fristi commented Nov 6, 2015

svalaskevicius commented Nov 6, 2015

svalaskevicius commented Nov 6, 2015