Skip to content

Eventsourced entity testkit#584

Merged
raboof merged 10 commits intolightbend:mainfrom
raboof:eventsourced-entity-testkit
Oct 8, 2021
Merged

Eventsourced entity testkit#584
raboof merged 10 commits intolightbend:mainfrom
raboof:eventsourced-entity-testkit

Conversation

@raboof
Copy link
Copy Markdown
Contributor

@raboof raboof commented Oct 8, 2021

Part of #372

Still needs:

  • codegen unit tests
  • more Scala-esque nextEventOfType type

@raboof raboof force-pushed the eventsourced-entity-testkit branch from 680121c to 17c9bf6 Compare October 8, 2021 08:46
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

current.reply.value shouldBe(42)

testKit.increase(eventsourcedentity.IncreaseValue(counterId, 15))
val next = testKit.getCurrentCounter(eventsourcedentity.GetCounter(counterId))
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.

what is the thought around the naming "current" and "next"? Isn't it a "result", and here "result1" and "result2"?

effect.javasdkEffect.secondaryEffect(state).isInstanceOf[ForwardReplyImpl[_]]

override def forwardedTo: ServiceCallDetails[R] =
??? // FIXME
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.

yeah, we should probably create a ticket for this (also for value entity)

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.

added #587

* INTERNAL API
*/
object EventSourcedResultImpl {
def eventsOf(effect: EventSourcedEntity.Effect[_]): Seq[Any] = {
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.

immutable.Seq

override def isNoReply: Boolean =
effect.javasdkEffect.secondaryEffect(state).isInstanceOf[NoReply[_]]

override def updatedState: AnyRef = state.asInstanceOf[AnyRef]
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.

can't we use S instead of AnyRef?

@raboof raboof marked this pull request as ready for review October 8, 2021 11:21
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

effect.javasdkEffect.secondaryEffect(state).isInstanceOf[ForwardReplyImpl[_]]

override def forwardedTo: ServiceCallDetails[R] =
??? // FIXME #587
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 wrote in 587 but, the context is already wired up with TestKitServiceCallFactory, so forwards will always be the Scala TestKitServiceCall and just matching should work fine here?

}

private[scalasdk] final case class EventSourcedEntityEffectImpl[S](
private[scalasdk] final case class EventSourcedEntityEffectImpl[R, S](
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.

So, R is the return type, when do we need state S as well, is that for the next step with thenReply?

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, looks like we pass the state to the secondary effect callbacks, so it's nice to keep track of it with S

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 think I skipped it on the Java side because it became too noisy but here it seems nicely hidden. (Maybe I just didn't think of only doing it to the impl side)

@raboof raboof mentioned this pull request Oct 8, 2021
21 tasks
@raboof raboof merged commit b6868c5 into lightbend:main Oct 8, 2021
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.

3 participants