EventSourcedEntityHandler generation and registration via provider #231

patriknw · 2021-08-23T13:28:44Z

codegen of EventSourcedEntityHandler (no reflection)
registration via EventSourcedEntityProvider
also split of java-customer-registry sampel into two;
one for value entity and another for event sourced

Refs #52

* also split of java-customer-registry sampel into two; one for value entity and another for event sourced

johanandren

LGTM

maven-java/akkaserverless-maven-archetype-event-sourced-entity/pom.xml

johanandren · 2021-08-24T07:07:56Z

samples/java-eventsourced-customer-registry/src/main/java/customer/domain/CustomerEntity.java

+import com.google.protobuf.Empty;
+import customer.api.CustomerApi;
+
+/** An event sourced entity. */


Not necessarily for this PR but one thing that would be nice would be if we could include what protobuf it corresponds to here (like we have manually in some of the serverless framework test classes).

johanandren · 2021-08-24T07:12:08Z

sdk/src/main/java/com/akkaserverless/javasdk/eventsourcedentity/EventSourcedEntityProvider.java

+import com.akkaserverless.javasdk.impl.eventsourcedentity.EventSourcedEntityHandler;
+import com.google.protobuf.Descriptors;
+
+public interface EventSourcedEntityProvider<S, E extends EventSourcedEntityBase<S>> {


Should we add some "Not for user extension" here or do we consider it actual API for user extension as well as common (internal) interface for generated code?

I'll add a comment that the concrete implementation is generated, to start with.

Good question.

Let's be restrictive and eventually relax the rules if needed. We have all the interest in keep it 'private'.

we can't make it completely internal because it's the parameter to register

what I mean by 'private' is that it's not real private, but marked as "Not for user extension"

patriknw · 2021-08-24T09:01:58Z

This is ready. I'll continue in a new PR by looking at the differences between event sourced and value entities and try to align. Part of that also look at the serialization second step in #150

octonato

Looking good.

I left some comments and suggestions. Don't need to be impl in this PR, so approving already. We can eventually add in follow-up PRs.

octonato · 2021-08-24T08:25:07Z

.../java-gen/src/main/scala/com/lightbend/akkasls/codegen/java/ValueEntitySourceGenerator.scala

@@ -23,6 +23,7 @@ import org.bitbucket.inkytonik.kiama.output.PrettyPrinterTypes.Document

 object ValueEntitySourceGenerator {
  import SourceGenerator._
+  import EntityServiceSourceGenerator.generateImports


Why generateImports needed to move to EntityServiceSourceGenerator?

it's used by both ValueEntitySourceGenerator and EntityServiceSourceGenerator, should probably be in some third utility class but EntityServiceSourceGenerator anyway looked like it wasn't only about EventSourced

Correct, the code for value entity and event sourced entity was previously in EntityServiceSourceGenerator and we started to split it into two different generators.

At some point, we spined-off ValueEntitySourceGenerator and EntityServiceSourceGenerator should have been renamed to EntitySourcedServiceSourceGenerator, but we had other PRs in the making and it was better to postpone the rename to avoid conflicts.

octonato · 2021-08-24T08:27:00Z

codegen/java-gen/src/test/scala/com/lightbend/akkasls/codegen/java/SourceGeneratorSuite.scala

+                generatedSourceDirectory.resolve(
+                  "com/example/service/persistence/MyEntity1Handler.java"
+                ),
+                generatedSourceDirectory.resolve(
+                  "com/example/service/persistence/MyEntity1Provider.java"
+                ),


Ah! I missed this extra tests when introducing the handler and provider. Thanks.

octonato · 2021-08-24T09:20:26Z

...ava-gen/src/main/scala/com/lightbend/akkasls/codegen/java/EntityServiceSourceGenerator.scala

@@ -316,7 +482,7 @@ object EntityServiceSourceGenerator {
                                      className: String): Option[String] = {
    entity match {
      case eventSourcedEntity: ModelBuilder.EventSourcedEntity =>
-        None
+        Some(eventSourcedEntityProvider(service, eventSourcedEntity, packageName, className))


I made it an option (here and above) because we didn't have a provider and a handler for ES.

We can remove the Option now.

ok, I'll remove that in next round

octonato · 2021-08-24T09:34:34Z

...ava-gen/src/main/scala/com/lightbend/akkasls/codegen/java/EntityServiceSourceGenerator.scala

      `class`("public", s"$className extends $interfaceClassName") {
        "@SuppressWarnings" <> parens(dquotes("unused")) <> line <>
        "private" <+> "final" <+> "String" <+> "entityId" <> semi <> line <>
        line <>
        constructor(
          "public",
          className,
-          List("@EntityId" <+> "String" <+> "entityId")
+          List("EventSourcedContext" <+> "context")


Value entity is only passing the String entityId. We need to align this.

The context is not so useful right now. It only has the entityId and the serviceCallFactory, but the serviceCallFactory is already available from the CommandContext.

We probably want to have something better than the exiting (untyped) ServiceCallFactory and then we want it to be injected in the constructor so users can instantiate clients at construction time (and not per command using the CommandContext).

So, passing the context is probably something we may need to keep (and introduce in VE), but not yet clear how we will really use it.

On the other hand, passing context make unit testing harder. It will be better to not have a mock context of any kind.

ok, there are more things that are different, I'll align them

octonato · 2021-08-24T09:50:52Z

sdk/src/main/java/com/akkaserverless/javasdk/eventsourcedentity/EventSourcedEntityProvider.java

+import com.akkaserverless.javasdk.impl.eventsourcedentity.EventSourcedEntityHandler;
+import com.google.protobuf.Descriptors;
+
+public interface EventSourcedEntityProvider<S, E extends EventSourcedEntityBase<S>> {


what I mean by 'private' is that it's not real private, but marked as "Not for user extension"

patriknw added 3 commits August 23, 2021 15:22

Registration of event sourced entity via provider

1d2d4fe

* also split of java-customer-registry sampel into two; one for value entity and another for event sourced

Codegen for EventSourcedEntityHandler

bcd481a

Remove generation of obsolete unit tests

a80a138

patriknw force-pushed the wip-es-provider-patriknw branch from e9e5d34 to a80a138 Compare August 23, 2021 17:34

patriknw changed the title ~~Registration of event sourced entity via provider~~ Event sourced generation and registration via provider Aug 23, 2021

patriknw marked this pull request as ready for review August 23, 2021 17:36

patriknw changed the title ~~Event sourced generation and registration via provider~~ EventSourcedEntityHandler generation and registration via provider Aug 23, 2021

patriknw mentioned this pull request Aug 24, 2021

Replace reflection with generated code #52

Closed

20 tasks

johanandren approved these changes Aug 24, 2021

View reviewed changes

patriknw added 5 commits August 24, 2021 09:53

revert maven-java

ae3af41

rsync java-eventsourced-customer-registry for docs

e285d34

Disable assert of unit tests

bf781c3

Comments about generated concrete implementations

af06aad

wrong name

7bf25e5

octonato approved these changes Aug 24, 2021

View reviewed changes

patriknw merged commit aa9889a into sdk-codegen-dev Aug 24, 2021

patriknw deleted the wip-es-provider-patriknw branch August 24, 2021 11:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

EventSourcedEntityHandler generation and registration via provider #231

EventSourcedEntityHandler generation and registration via provider #231

patriknw commented Aug 23, 2021 •

edited

Loading

johanandren left a comment

johanandren Aug 24, 2021

johanandren Aug 24, 2021

patriknw Aug 24, 2021

octonato Aug 24, 2021

patriknw Aug 24, 2021

octonato Aug 24, 2021

patriknw commented Aug 24, 2021

octonato left a comment

octonato Aug 24, 2021

patriknw Aug 24, 2021

octonato Aug 24, 2021

octonato Aug 24, 2021

octonato Aug 24, 2021

patriknw Aug 24, 2021

octonato Aug 24, 2021

patriknw Aug 24, 2021

octonato Aug 24, 2021

EventSourcedEntityHandler generation and registration via provider #231

EventSourcedEntityHandler generation and registration via provider #231

Conversation

patriknw commented Aug 23, 2021 • edited Loading

johanandren left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

patriknw commented Aug 24, 2021

octonato left a 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

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

Choose a reason for hiding this comment

patriknw commented Aug 23, 2021 •

edited

Loading