On Fri, Oct 27, 2017 at 12:19 PM, Yaroslav Klymko <[email protected]> wrote:

> Sure,
>
> But then I won't see the potential problems that during recovery for
> instance we are renaming missing entry.
> With ongoing development and more complicated data structures, when you
> might have tens nesting deep it is becoming crucial to validate recovery
> against previous versions and to understand what went wrong.
> Thus having similar flow during command validation and event validation
> also helps.
>

That's for catching bugs when evolving the model. I think that should be
written as precondition and invariant checks, typically throwing exceptions.


>
> So I'm just wanted to point out that your new approach is becoming less
> flexible, however more type safe. But I believe you can preserve
> flexibility and still make it type safe :)
>

Thanks for feedback.

PersistentActor is extremely flexible, but also very easy to get wrong.

Btw, you could also change validateCommand to be a factory for the event if
you like:

def createEvent(cmd: Cmd): Either[String, Event] = {
      cmd match {
        case Cmd.AddEntry(id, name) =>
          entries.get(id) match {
            case None    => Right(Event.EntryAdded(id, name))
            case Some(_) => Left("entry already exists")
          }
        case Cmd.RenameEntry(id, newName) =>
          entries.get(id) match {
            case None => Left("entry is missing")
            case Some(entry) =>
              if (entry.name == newName) Left("old and new names are equal")
              else Right(Event.EntryRenamed(id, newName))
          }
      }
    }



>
>
> On Friday, October 27, 2017 at 12:09:40 PM UTC+2, Patrik Nordwall wrote:
>>
>> You can rewrite that to:
>>
>>   case class State(entries: Map[String, Entry]) {
>>
>>     def applyEvent(event: Event): State = {
>>       event match {
>>         case Event.EntryAdded(id, name) =>
>>           copy(entries + (id -> Entry(name)))
>>         case Event.EntryRenamed(id, newName) =>
>>           copy(entries + (id -> Entry(newName)))
>>       }
>>     }
>>
>>     def validateCmd(cmd: Cmd): Either[String, Done.type] = {
>>       cmd match {
>>         case Cmd.AddEntry(id, name) =>
>>           entries.get(id) match {
>>             case None    => Right(Done)
>>             case Some(_) => Left("entry already exists")
>>           }
>>         case Cmd.RenameEntry(id, newName) =>
>>           entries.get(id) match {
>>             case None => Left("entry is missing")
>>             case Some(entry) =>
>>               if (entry.name == newName) Left("old and new names are
>> equal")
>>               else Right(Done)
>>           }
>>
>>       }
>>     }
>>   }
>>
>>
>>
>> On Fri, Oct 27, 2017 at 11:41 AM, Yaroslav Klymko <[email protected]>
>> wrote:
>>
>>> Hi Patrik,
>>> here is very simplified example:
>>>
>>>
>>> object Example {
>>>
>>>   case class Entry(name: String)
>>>
>>>   case class State(entries: Map[String, Entry]) {
>>>
>>>     def applyEvent(event: Event): Either[String, State] = {
>>>       event match {
>>>         case Event.EntryAdded(id, name) =>
>>>           entries.get(id) match {
>>>             case None    => Right(copy(entries + (id -> Entry(name))))
>>>             case Some(_) => Left("entry already exists")
>>>           }
>>>
>>>         case Event.EntryRenamed(id, newName) =>
>>>           entries.get(id) match {
>>>             case None        => Left("entry is missing")
>>>             case Some(entry) =>
>>>               if (entry.name == newName) Left("old and new names are equal")
>>>               else Right(copy(entries + (id -> Entry(newName))))
>>>           }
>>>       }
>>>     }
>>>
>>>
>>>     def validateCmd(cmd: Cmd): Either[String, State] = {
>>>       cmd match {
>>>         case Cmd.AddEntry(id, name)       => 
>>> applyEvent(Event.EntryAdded(id, name))
>>>         case Cmd.RenameEntry(id, newName) => 
>>> applyEvent(Event.EntryRenamed(id, newName))
>>>       }
>>>     }
>>>   }
>>>
>>>   sealed trait Cmd
>>>   object Cmd {
>>>     case class AddEntry(id: String, name: String) extends Cmd
>>>     case class RenameEntry(id: String, newName: String) extends Cmd
>>>   }
>>>
>>>   sealed trait Event
>>>   object Event {
>>>     case class EntryAdded(id: String, name: String) extends Event
>>>     case class EntryRenamed(id: String, newName: String) extends Event
>>>   }
>>> }
>>>
>>>
>>>
>>> So for instance after validating command we already have target state,
>>> and as we are using the same code for applying event and validating
>>> command, we don't need to perform it twice, no need in second sanity check
>>> after event has been persisted.
>>> Does this help to understand my point?
>>>
>>>
>>> On Friday, October 27, 2017 at 11:24:10 AM UTC+2, Patrik Nordwall wrote:
>>>
>>>>
>>>>
>>>> On Fri, Oct 20, 2017 at 5:29 PM, Yaroslav Klymko <[email protected]>
>>>> wrote:
>>>>
>>>>>
>>>>> On Friday, October 20, 2017 at 4:44:35 PM UTC+2, Patrik Nordwall wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On Thu, Oct 19, 2017 at 2:52 PM, Yaroslav Klymko <[email protected]>
>>>>>> wrote:
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Thursday, October 19, 2017 at 9:33:59 PM UTC+2, Patrik Nordwall
>>>>>>> wrote:
>>>>>>>>
>>>>>>>> Invalid events must not be stored, because then the actor will not
>>>>>>>> be able to recover after a restart (later startup). Validation should
>>>>>>>> typically be done by validating the incoming command before persisting 
>>>>>>>> the
>>>>>>>> event.
>>>>>>>>
>>>>>>>
>>>>>>> That depends on what you mean by invalid event:
>>>>>>>  * completely invalid event within state
>>>>>>>  * valid event that does not change state
>>>>>>>  * given batch of events, and some of them are invalid, what to do
>>>>>>> with others?
>>>>>>>
>>>>>>>
>>>>>>>> Validation should not be placed in applyEvent, but as an additional
>>>>>>>> protection in case applyEvent throws exception (for validation or other
>>>>>>>> reasons) we have chosen to call applyEvent before calling persistAll.
>>>>>>>>
>>>>>>>
>>>>>>> For instance we would like to avoid exceptions, thus we have `state:
>>>>>>> Either[Error, State]` and validation result is also of type `state:
>>>>>>> Either[Error, State]`
>>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>> I'm not sure I understand what you are missing or suggest should be
>>>>>>>> done differently, so please clarify. Isn't the problem that you mix the
>>>>>>>> concerns of validating cmd/event with applying the event? If you have a
>>>>>>>> separate step for validation you can do that before persisting, and the
>>>>>>>> applyEvent is intended purely for changing the state and cannot fail.
>>>>>>>>
>>>>>>>
>>>>>>> Let's consider we want to avoid exceptions, thus we have verified
>>>>>>> that event can be applied to state and then either return
>>>>>>> PersistentActor.persist or proceed further without persistence.
>>>>>>> However now we cannot avoid useless in our case call `state =
>>>>>>> events.foldLeft(state)(applyEvent)` as we had to do that on our own
>>>>>>> already.
>>>>>>>
>>>>>>
>>>>>> Ok, I still think you can do all that with a separate
>>>>>> validation/decision step (without exceptions) before persisting and keep
>>>>>> applyEvent for the events that have already been persisted.
>>>>>>
>>>>>
>>>>> It might be heavy operation (it is in my case) and I'd rather avoid
>>>>> unnecessary calls, this will over compensate many optimisations like the
>>>>> one you have for Options :)
>>>>>
>>>>
>>>> I don't understand what you mean. I'm not suggesting that you perform
>>>> the validation twice. I suggest that you separate the validation from the
>>>> actual update. In that way it will actually be more efficient, because you
>>>> don't have to validate when replaying events during recovery.
>>>> Perhaps you can share a code example illustrating your point, if you
>>>> still think I'm wrong?
>>>>
>>>>
>>>>>
>>>>>
>>>>>>
>>>>>> I tried to explain that in the blog post
>>>>>> <https://akka.io/blog/2017/10/13/typed-persistence> as:
>>>>>>
>>>>>> After processing a message an ordinary, non-persistent, typed actor
>>>>>> returns the Behavior that is used for next message. As you can see in the
>>>>>> above examples that is not supported by typed persistent actors. Instead,
>>>>>> the state is returned by applyEvent. The reason a new behavior can’t be
>>>>>> returned is that behavior is part of the actor’s state and must also
>>>>>> carefully be reconstructed during recovery. If it would have been 
>>>>>> supported
>>>>>> it would mean that the behavior must be restored when replaying events 
>>>>>> and
>>>>>> also encoded in the state anyway when snapshots are used. That would be
>>>>>> very prone to mistakes.
>>>>>>
>>>>>
>>>>> Behaviour might be just retrieved by state, like `onRecoveryCompleted:
>>>>> State => PersistentBehavior[Command, Event, State]` so you don't need to
>>>>> encode it into state.
>>>>>
>>>>>
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> So I propose to remove concept of `state` from command handling
>>>>>>> phase, and have similar to typed actor approach with Behavior
>>>>>>>
>>>>>>> And the transition might look like `onRecoveryCompleted: State =>
>>>>>>> PersistentBehavior[Command, Event, State]`
>>>>>>>
>>>>>>>
>>>>>>> What do you think ?
>>>>>>>
>>>>>>>
>>>>>>>> On Thu, Oct 19, 2017 at 5:48 AM, Yaroslav Klymko <[email protected]>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>> I also have other concern regarding.
>>>>>>>>>
>>>>>>>>> case PersistAll(events) ⇒
>>>>>>>>>   // apply the event before persist so that validation exception is 
>>>>>>>>> handled before persisting
>>>>>>>>>   // the invalid event, in case such validation is implemented in the 
>>>>>>>>> event handler.
>>>>>>>>>   state = events.foldLeft(state)(applyEvent)
>>>>>>>>>   persistAll(scala.collection.immutable.Seq(events)) { _ ⇒
>>>>>>>>>     sideEffects.foreach(applySideEffect)
>>>>>>>>>   }
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> We have local practice to apply events before persisting to make
>>>>>>>>> sure those are valid and then reusing resulting state.
>>>>>>>>> We also have explicit logic on what to do with event that is not
>>>>>>>>> valid - Keep | DropEvent | DropCmd
>>>>>>>>>
>>>>>>>>> But this logic enforces users to apply events after persistence
>>>>>>>>> and rely on exceptions.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Please let me know in case I'm wrong, I'd really like to
>>>>>>>>> understand your logic :)
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Wednesday, October 18, 2017 at 10:28:43 PM UTC+2, Yaroslav
>>>>>>>>> Klymko wrote:
>>>>>>>>>>
>>>>>>>>>> I think renaming
>>>>>>>>>> actions:       State => Actions[Command, Event, State]
>>>>>>>>>> to
>>>>>>>>>> onRecoveryCompleted:       State => Actions[Command, Event, State]
>>>>>>>>>>
>>>>>>>>>> Makes it really clear. Also you might just decide to stop the
>>>>>>>>>> actor in case the state is wrong, etc.
>>>>>>>>>>
>>>>>>>>>> On Wednesday, October 18, 2017 at 10:22:12 PM UTC+2, Patrik
>>>>>>>>>> Nordwall wrote:
>>>>>>>>>>>
>>>>>>>>>>> Do you suggest something like this?
>>>>>>>>>>>
>>>>>>>>>>>   def immutable[Command, Event, State](
>>>>>>>>>>>     persistenceId: String,
>>>>>>>>>>>     initialState:  State,
>>>>>>>>>>>     actions:       State => Actions[Command, Event, State],
>>>>>>>>>>>     applyEvent:    (Event, State) ⇒ State):
>>>>>>>>>>> PersistentBehavior[Command, Event, State]
>>>>>>>>>>>
>>>>>>>>>>> I it would be rather difficult to understand what state that is,
>>>>>>>>>>> especially since there is also Actions.byState. A typical usage of
>>>>>>>>>>> onRecoveryCompleted is to perform side effects, such as resending 
>>>>>>>>>>> to an
>>>>>>>>>>> external service, and it's better to not mixing that concern with 
>>>>>>>>>>> the
>>>>>>>>>>> construction of the command handlers.
>>>>>>>>>>>
>>>>>>>>>>> /Patrik
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On Wed, Oct 18, 2017 at 2:54 PM, Yaroslav Klymko <
>>>>>>>>>>> [email protected]> wrote:
>>>>>>>>>>>
>>>>>>>>>>>> Hi guys,
>>>>>>>>>>>> Not sure this is the right place to ask:
>>>>>>>>>>>> I'm looking into akka typed for persistence and wondering
>>>>>>>>>>>> whether we really need onRecoveryCompleted at
>>>>>>>>>>>> https://github.com/akka/akka/blob/master/akka-typed/src/main
>>>>>>>>>>>> /scala/akka/typed/persistence/scaladsl/PersistentActor.
>>>>>>>>>>>> scala#L174
>>>>>>>>>>>> Why not just have something like actions: State =>
>>>>>>>>>>>> PersistentActor.Actions[Command, Event, State] ?
>>>>>>>>>>>>
>>>>>>>>>>>> --
>>>>>>>>>>>> >>>>>>>>>> Read the docs: http://akka.io/docs/
>>>>>>>>>>>> >>>>>>>>>> Check the FAQ: http://doc.akka.io/docs/akka/c
>>>>>>>>>>>> urrent/additional/faq.html
>>>>>>>>>>>> >>>>>>>>>> Search the archives: https://groups.google.com/grou
>>>>>>>>>>>> p/akka-user
>>>>>>>>>>>> ---
>>>>>>>>>>>> You received this message because you are subscribed to the
>>>>>>>>>>>> Google Groups "Akka User List" group.
>>>>>>>>>>>> To unsubscribe from this group and stop receiving emails from
>>>>>>>>>>>> it, send an email to [email protected].
>>>>>>>>>>>> To post to this group, send email to [email protected].
>>>>>>>>>>>> Visit this group at https://groups.google.com/group/akka-user.
>>>>>>>>>>>> For more options, visit https://groups.google.com/d/optout.
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> --
>>>>>>>>>>>
>>>>>>>>>>> Patrik Nordwall
>>>>>>>>>>> Akka Tech Lead
>>>>>>>>>>> Lightbend <http://www.lightbend.com/> -  Reactive apps on the
>>>>>>>>>>> JVM
>>>>>>>>>>> Twitter: @patriknw
>>>>>>>>>>>
>>>>>>>>>>> --
>>>>>>>>> >>>>>>>>>> Read the docs: http://akka.io/docs/
>>>>>>>>> >>>>>>>>>> Check the FAQ: http://doc.akka.io/docs/akka/c
>>>>>>>>> urrent/additional/faq.html
>>>>>>>>> >>>>>>>>>> Search the archives: https://groups.google.com/grou
>>>>>>>>> p/akka-user
>>>>>>>>> ---
>>>>>>>>> You received this message because you are subscribed to the Google
>>>>>>>>> Groups "Akka User List" group.
>>>>>>>>> To unsubscribe from this group and stop receiving emails from it,
>>>>>>>>> send an email to [email protected].
>>>>>>>>> To post to this group, send email to [email protected].
>>>>>>>>> Visit this group at https://groups.google.com/group/akka-user.
>>>>>>>>> For more options, visit https://groups.google.com/d/optout.
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>>
>>>>>>>> Patrik Nordwall
>>>>>>>> Akka Tech Lead
>>>>>>>> Lightbend <http://www.lightbend.com/> -  Reactive apps on the JVM
>>>>>>>> Twitter: @patriknw
>>>>>>>>
>>>>>>>> --
>>>>>>> >>>>>>>>>> Read the docs: http://akka.io/docs/
>>>>>>> >>>>>>>>>> Check the FAQ: http://doc.akka.io/docs/akka/c
>>>>>>> urrent/additional/faq.html
>>>>>>> >>>>>>>>>> Search the archives: https://groups.google.com/grou
>>>>>>> p/akka-user
>>>>>>> ---
>>>>>>> You received this message because you are subscribed to the Google
>>>>>>> Groups "Akka User List" group.
>>>>>>> To unsubscribe from this group and stop receiving emails from it,
>>>>>>> send an email to [email protected].
>>>>>>> To post to this group, send email to [email protected].
>>>>>>> Visit this group at https://groups.google.com/group/akka-user.
>>>>>>> For more options, visit https://groups.google.com/d/optout.
>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>>
>>>>>> Patrik Nordwall
>>>>>> Akka Tech Lead
>>>>>> Lightbend <http://www.lightbend.com/> -  Reactive apps on the JVM
>>>>>> Twitter: @patriknw
>>>>>>
>>>>>> --
>>>>> >>>>>>>>>> Read the docs: http://akka.io/docs/
>>>>> >>>>>>>>>> Check the FAQ: http://doc.akka.io/docs/akka/c
>>>>> urrent/additional/faq.html
>>>>> >>>>>>>>>> Search the archives: https://groups.google.com/grou
>>>>> p/akka-user
>>>>> ---
>>>>> You received this message because you are subscribed to the Google
>>>>> Groups "Akka User List" group.
>>>>> To unsubscribe from this group and stop receiving emails from it, send
>>>>> an email to [email protected].
>>>>> To post to this group, send email to [email protected].
>>>>> Visit this group at https://groups.google.com/group/akka-user.
>>>>> For more options, visit https://groups.google.com/d/optout.
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>>
>>>> Patrik Nordwall
>>>> Akka Tech Lead
>>>> Lightbend <http://www.lightbend.com/> -  Reactive apps on the JVM
>>>> Twitter: @patriknw
>>>>
>>>> --
>>> >>>>>>>>>> Read the docs: http://akka.io/docs/
>>> >>>>>>>>>> Check the FAQ: http://doc.akka.io/docs/akka/c
>>> urrent/additional/faq.html
>>> >>>>>>>>>> Search the archives: https://groups.google.com/grou
>>> p/akka-user
>>> ---
>>> You received this message because you are subscribed to the Google
>>> Groups "Akka User List" group.
>>> To unsubscribe from this group and stop receiving emails from it, send
>>> an email to [email protected].
>>> To post to this group, send email to [email protected].
>>> Visit this group at https://groups.google.com/group/akka-user.
>>> For more options, visit https://groups.google.com/d/optout.
>>>
>>
>>
>>
>> --
>>
>> Patrik Nordwall
>> Akka Tech Lead
>> Lightbend <http://www.lightbend.com/> -  Reactive apps on the JVM
>> Twitter: @patriknw
>>
>> --
> >>>>>>>>>> Read the docs: http://akka.io/docs/
> >>>>>>>>>> Check the FAQ: http://doc.akka.io/docs/akka/
> current/additional/faq.html
> >>>>>>>>>> Search the archives: https://groups.google.com/group/akka-user
> ---
> You received this message because you are subscribed to the Google Groups
> "Akka User List" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to [email protected].
> To post to this group, send email to [email protected].
> Visit this group at https://groups.google.com/group/akka-user.
> For more options, visit https://groups.google.com/d/optout.
>



-- 

Patrik Nordwall
Akka Tech Lead
Lightbend <http://www.lightbend.com/> -  Reactive apps on the JVM
Twitter: @patriknw

-- 
>>>>>>>>>>      Read the docs: http://akka.io/docs/
>>>>>>>>>>      Check the FAQ: 
>>>>>>>>>> http://doc.akka.io/docs/akka/current/additional/faq.html
>>>>>>>>>>      Search the archives: https://groups.google.com/group/akka-user
--- 
You received this message because you are subscribed to the Google Groups "Akka 
User List" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To post to this group, send email to [email protected].
Visit this group at https://groups.google.com/group/akka-user.
For more options, visit https://groups.google.com/d/optout.

Reply via email to