Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fromJournal method in Typed Event Adapter should return a list of events #26909

Closed
michaltomanski opened this issue May 13, 2019 · 4 comments
Closed
Assignees
Milestone

Comments

@michaltomanski
Copy link

Currently, the EventAdapter in Akka Typed is:

abstract class EventAdapter[Event, PersistedEvent] {
  def toJournal(e: Event): PersistedEvent
  def fromJournal(p: PersistedEvent): Event
}

I suggest changing the fromJournal signature to return a sequence of events, which would give us something like

abstract class EventAdapter[Event, PersistedEvent] {
  def toJournal(e: Event): PersistedEvent
  def fromJournal(p: PersistedEvent): Seq[Event]
}

Alternatively, we can use a specialized structure for the return type

sealed abstract class EventSeq[E] {
  def events: immutable.Seq[E]
}

This modification has several advantages:

  • compatibility with untyped version of EventAdapter (where an EventSeq is returned by fromJournalmethod)
  • possibility to handle schema evolution case where one event has to be split into two, as described in Akka docs
  • possibility to ignore events (by returning an empty list), as described in Akka docs

This request is a follow up to the discussion:
https://discuss.lightbend.com/t/recommended-approach-to-unused-events-with-akka-typed-persistence/4159

@patriknw
Copy link
Member

It was added in #25050 but I can't see that we discussed that matter. Maybe we were looking for simplest possible API, but went too far in that direction.

Any thoughts @chbatey ?

@patriknw patriknw added 1 - triaged Tickets that are safe to pick up for contributing in terms of likeliness of being accepted t:persistence t:typed labels May 14, 2019
@chbatey
Copy link
Member

chbatey commented May 14, 2019

I recall us discussing simplifying it but nothing specific, I have nothing against making it a Seq

@patriknw patriknw self-assigned this Jun 12, 2019
@patriknw patriknw added 3 - in progress Someone is working on this ticket and removed 1 - triaged Tickets that are safe to pick up for contributing in terms of likeliness of being accepted labels Jun 12, 2019
patriknw added a commit that referenced this issue Jun 13, 2019
* Tests for usage of EventSeq
* Moved event adapter tests from EventSourcedBehaviorSpec to
  new EventSourcedEventAdapterSpec
* Also support for the event adapter manifest
* Note in migration guide
patriknw added a commit that referenced this issue Jun 13, 2019
* Tests for usage of EventSeq
* Moved event adapter tests from EventSourcedBehaviorSpec to
  new EventSourcedEventAdapterSpec
* Also support for the event adapter manifest
* Note in migration guide
patriknw added a commit that referenced this issue Jul 2, 2019
* Tests for usage of EventSeq
* Moved event adapter tests from EventSourcedBehaviorSpec to
  new EventSourcedEventAdapterSpec
* Also support for the event adapter manifest
* Note in migration guide
patriknw added a commit that referenced this issue Jul 3, 2019
* Tests for usage of EventSeq
* Moved event adapter tests from EventSourcedBehaviorSpec to
  new EventSourcedEventAdapterSpec
* Also support for the event adapter manifest
* Note in migration guide
raboof pushed a commit that referenced this issue Jul 5, 2019
EventSeq in Typed EventAdapter, refs #26909
@patriknw patriknw removed the 3 - in progress Someone is working on this ticket label Jul 5, 2019
@patriknw patriknw added this to the 2.6.0-M4 milestone Jul 5, 2019
@patriknw patriknw closed this as completed Jul 5, 2019
@michaltomanski
Copy link
Author

Thanks @patriknw !

@patriknw
Copy link
Member

patriknw commented Jul 5, 2019

thanks for reporting, will be included in 2.6.0-M4 that is to be released today

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants