Skip to content

feat(codegen): Scala EventSourcedEntity.#561

Merged
RayRoestenburg merged 3 commits intomainfrom
codegen-event-sourced
Oct 5, 2021
Merged

feat(codegen): Scala EventSourcedEntity.#561
RayRoestenburg merged 3 commits intomainfrom
codegen-event-sourced

Conversation

@RayRoestenburg
Copy link
Copy Markdown
Contributor

References #560

Copy link
Copy Markdown
Contributor

@johanandren johanandren left a comment

Choose a reason for hiding this comment

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

LGTM with some small things

rawEntity.getEntityType,
State(FullyQualifiedName(rawEntity.getState, protoReference)),
// FIXME this assumes the state is defined in the same proto file as the
// entity, which I don't think is necessarily true.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

At least that is how all our samples does it, I think that if we want to support that the state message is in some other package that will likely need changes elsewhere as well.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yeah I copied this comment :-)

ValueEntityTestKitGenerator.generateManagedTest(entity, service)
case _: ModelBuilder.EventSourcedEntity =>
Nil // FIXME
Nil
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

still to be done

Suggested change
Nil
Nil // FIXME

Copy link
Copy Markdown
Member

@octonato octonato left a comment

Choose a reason for hiding this comment

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

LGTM, left a few minor suggestions


def provider(entity: ModelBuilder.EventSourcedEntity, service: ModelBuilder.EntityService): File = {
val packageName = entity.fqn.parent.scalaPackage
val className = entity.fqn.name + "Provider"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can be moved in Entity model, like the 'Abstract' and the 'Handler'

sealed abstract class Entity(val fqn: FullyQualifiedName, val entityType: String)
sealed abstract class Entity(val fqn: FullyQualifiedName, val entityType: String) {
val abstractEntityName = "Abstract" + fqn.name
val handlerName = fqn.name + "Handler"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

what about the "Provider"? :-)

case ModelBuilder.EventSourcedEntity(_, _, _, events) =>
events.map { event =>
s"""|def ${lowerFirst(event.fqn.name)}(currentState: ${typeName(
eventSourcedEntity.state.fqn)}, ${lowerFirst(event.fqn.name)}: ${typeName(event.fqn)}): ${typeName(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We can use $stateType instead of ${typeName(eventSourcedEntity.state.fqn)}. Will make is shorter to read.

Copy link
Copy Markdown
Contributor

@patriknw patriknw left a comment

Choose a reason for hiding this comment

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

looking good

|class MyEntityHandler(entity: MyEntity) extends EventSourcedEntityHandler[MyState, MyEntity](entity) {
| def handleEvent(state: MyState, event: Any): MyState = {
| event match {
| case SetEvent =>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

that looks strange, matching on the companion object?

shouldn't it be case setEvent: SetEvent, and then the asInstanceOf isn't needed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes fixed (I merged the fix in now)

object EventSourcedEntityHandler {
final case class CommandHandlerNotFound(commandName: String) extends RuntimeException

final case class EventHandlerNotFound(commandName: String) extends RuntimeException
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

we should use the exceptions from the javasdk

com.akkaserverless.javasdk.impl.eventsourcedentity.EventSourcedEntityHandler.CommandHandlerNotFound and same with EventHandlerNotFound

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ah leftover, I'll remove this.

| * and the command handler methods in the <code>Counter</code> class.
| */
|class MyEntityHandler(entity: MyEntity) extends EventSourcedEntityHandler[MyState, MyEntity](entity) {
| def handleEvent(state: MyState, event: Any): MyState = {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd prefer to have handleCommand before handleEvent

* doc: scala eventsourced counter example.

* fix(scalasdk): Scala PB Any vs Scala Any. (#563)

* fix(scalasdk): scala eventsourced counter
* doc: scala eventsourced counter example.

* fix(scalasdk): Scala PB Any vs Scala Any. (#563)

* fix(scalasdk): scala eventsourced counter
@RayRoestenburg RayRoestenburg merged commit a7bd934 into main Oct 5, 2021
@RayRoestenburg RayRoestenburg deleted the codegen-event-sourced branch October 5, 2021 12:39
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.

4 participants