#23329 PersistentFSM: andThen callbacks are not executed when stay() (#23943)

* Apply [Boy scout rule](https://github.com/akka/akka/blob/master/CONTRIBUTING.md#additional-guidelines)

* [Bug#23329] PersistentFSM: andThen callbacks are not executed when stay()

* [fixes#23329] PersistentFSM: andThen callbacks go execute when stay()

 * At documentation: there is nothing said about events applied should decide to invoke andThen or not.
 * At code: andThen callback can be specified whether any events/state transition applied or not.

 Added call to nextState.afterTransitionDo(stateData) even if there are no eventsToPersist
This commit is contained in:
sovaalexandr 2017-11-08 10:17:41 +02:00 committed by Konrad `ktoso` Malawski
parent 32a23dd5ec
commit b19d9860d7
2 changed files with 31 additions and 18 deletions

View file

@ -140,6 +140,7 @@ trait PersistentFSM[S <: FSMState, D, E] extends PersistentActor with Persistent
if (eventsToPersist.isEmpty) { if (eventsToPersist.isEmpty) {
//If there are no events to persist, just apply the state //If there are no events to persist, just apply the state
super.applyState(nextState) super.applyState(nextState)
nextState.afterTransitionDo(stateData)
} else { } else {
//Persist the events and apply the new state after all event handlers were executed //Persist the events and apply the new state after all event handlers were executed
var nextData: D = stateData var nextData: D = stateData

View file

@ -9,7 +9,6 @@ import java.io.File
import akka.actor.{ ActorSystem, _ } import akka.actor.{ ActorSystem, _ }
import akka.persistence._ import akka.persistence._
import akka.persistence.fsm.PersistentFSM._ import akka.persistence.fsm.PersistentFSM._
import akka.persistence.fsm.PersistentFSMSpec.IntAdded
import akka.testkit._ import akka.testkit._
import com.typesafe.config.{ Config, ConfigFactory } import com.typesafe.config.{ Config, ConfigFactory }
import org.apache.commons.io.FileUtils import org.apache.commons.io.FileUtils
@ -22,7 +21,7 @@ abstract class PersistentFSMSpec(config: Config) extends PersistenceSpec(config)
import PersistentFSMSpec._ import PersistentFSMSpec._
//Dummy report actor, for tests that don't need it //Dummy report actor, for tests that don't need it
val dummyReportActorRef = TestProbe().ref private val dummyReportActorRef = TestProbe().ref
"PersistentFSM" must { "PersistentFSM" must {
"function as a regular FSM " in { "function as a regular FSM " in {
@ -199,7 +198,7 @@ abstract class PersistentFSMSpec(config: Config) extends PersistenceSpec(config)
expectMsg(CurrentState(fsmRef, LookingAround, None)) expectMsg(CurrentState(fsmRef, LookingAround, None))
expectMsg(Transition(fsmRef, LookingAround, Shopping, Some(1 second))) expectMsg(Transition(fsmRef, LookingAround, Shopping, Some(1 second)))
expectNoMsg(0.6 seconds) // arbitrarily chosen delay, less than the timeout, before stopping the FSM expectNoMessage(0.6 seconds) // arbitrarily chosen delay, less than the timeout, before stopping the FSM
fsmRef ! PoisonPill fsmRef ! PoisonPill
expectTerminated(fsmRef) expectTerminated(fsmRef)
@ -216,7 +215,7 @@ abstract class PersistentFSMSpec(config: Config) extends PersistenceSpec(config)
expectMsg(Transition(recoveredFsmRef, Shopping, Inactive, Some(2 seconds))) expectMsg(Transition(recoveredFsmRef, Shopping, Inactive, Some(2 seconds)))
} }
expectNoMsg(0.6 seconds) // arbitrarily chosen delay, less than the timeout, before stopping the FSM expectNoMessage(0.6 seconds) // arbitrarily chosen delay, less than the timeout, before stopping the FSM
recoveredFsmRef ! PoisonPill recoveredFsmRef ! PoisonPill
expectTerminated(recoveredFsmRef) expectTerminated(recoveredFsmRef)
@ -239,7 +238,19 @@ abstract class PersistentFSMSpec(config: Config) extends PersistenceSpec(config)
probe.expectMsg(3.seconds, "LookingAround -> LookingAround") probe.expectMsg(3.seconds, "LookingAround -> LookingAround")
fsmRef ! "stay" // causes stay() fsmRef ! "stay" // causes stay()
probe.expectNoMsg(3.seconds) probe.expectNoMessage(3.seconds)
}
"execute andThen callback for stay" in {
val persistenceId = name
val probe = TestProbe()
val fsmRef = system.actorOf(SimpleTransitionFSM.props(persistenceId, probe.ref))
fsmRef ! "stay andThen report"
probe.fishForMessage(hint = "expect I'm staying message") {
case "I'm staying" true
case "LookingAround -> LookingAround" false
}
} }
"not persist state change event when staying in the same state" in { "not persist state change event when staying in the same state" in {
@ -319,7 +330,7 @@ abstract class PersistentFSMSpec(config: Config) extends PersistenceSpec(config)
expectMsg(NonEmptyShoppingCart(List(shirt, shoes, coat))) expectMsg(NonEmptyShoppingCart(List(shirt, shoes, coat)))
expectMsg(NonEmptyShoppingCart(List(shirt, shoes, coat))) expectMsg(NonEmptyShoppingCart(List(shirt, shoes, coat)))
expectNoMsg(1 second) expectNoMessage(1 second)
fsmRef ! PoisonPill fsmRef ! PoisonPill
expectTerminated(fsmRef) expectTerminated(fsmRef)
@ -357,7 +368,7 @@ abstract class PersistentFSMSpec(config: Config) extends PersistenceSpec(config)
fsm ! TimeoutFSM.OverrideTimeoutToInf fsm ! TimeoutFSM.OverrideTimeoutToInf
p.expectMsg(TimeoutFSM.OverrideTimeoutToInf) p.expectMsg(TimeoutFSM.OverrideTimeoutToInf)
p.expectNoMsg(1.seconds) p.expectNoMessage(1.seconds)
} }
@ -425,7 +436,7 @@ object PersistentFSMSpec {
} }
case object EmptyShoppingCart extends ShoppingCart { case object EmptyShoppingCart extends ShoppingCart {
def addItem(item: Item) = NonEmptyShoppingCart(item :: Nil) def addItem(item: Item) = NonEmptyShoppingCart(item :: Nil)
def empty() = this def empty(): EmptyShoppingCart.type = this
} }
case class NonEmptyShoppingCart(items: Seq[Item]) extends ShoppingCart { case class NonEmptyShoppingCart(items: Seq[Item]) extends ShoppingCart {
def addItem(item: Item) = NonEmptyShoppingCart(items :+ item) def addItem(item: Item) = NonEmptyShoppingCart(items :+ item)
@ -454,13 +465,14 @@ object PersistentFSMSpec {
case object ShoppingCardDiscarded extends ReportEvent case object ShoppingCardDiscarded extends ReportEvent
class SimpleTransitionFSM(_persistenceId: String, reportActor: ActorRef)(implicit val domainEventClassTag: ClassTag[DomainEvent]) extends PersistentFSM[UserState, ShoppingCart, DomainEvent] { class SimpleTransitionFSM(_persistenceId: String, reportActor: ActorRef)(implicit val domainEventClassTag: ClassTag[DomainEvent]) extends PersistentFSM[UserState, ShoppingCart, DomainEvent] {
override val persistenceId = _persistenceId override val persistenceId: String = _persistenceId
startWith(LookingAround, EmptyShoppingCart) startWith(LookingAround, EmptyShoppingCart)
when(LookingAround) { when(LookingAround) {
case Event("stay", _) stay case Event("stay andThen report", _) stay andThen { _ reportActor ! "I'm staying" }
case Event(e, _) goto(LookingAround) case Event("stay", _) stay()
case Event(e, _) goto(LookingAround)
} }
onTransition { onTransition {
@ -478,7 +490,7 @@ object PersistentFSMSpec {
class WebStoreCustomerFSM(_persistenceId: String, reportActor: ActorRef)(implicit val domainEventClassTag: ClassTag[DomainEvent]) class WebStoreCustomerFSM(_persistenceId: String, reportActor: ActorRef)(implicit val domainEventClassTag: ClassTag[DomainEvent])
extends PersistentFSM[UserState, ShoppingCart, DomainEvent] { extends PersistentFSM[UserState, ShoppingCart, DomainEvent] {
override def persistenceId = _persistenceId override def persistenceId: String = _persistenceId
//#customer-fsm-body //#customer-fsm-body
startWith(LookingAround, EmptyShoppingCart) startWith(LookingAround, EmptyShoppingCart)
@ -507,7 +519,7 @@ object PersistentFSMSpec {
case Event(Leave, _) case Event(Leave, _)
//#customer-snapshot-example //#customer-snapshot-example
stop applying OrderDiscarded andThen { stop applying OrderDiscarded andThen {
case _ _
reportActor ! ShoppingCardDiscarded reportActor ! ShoppingCardDiscarded
saveStateSnapshot() saveStateSnapshot()
} }
@ -523,7 +535,7 @@ object PersistentFSMSpec {
goto(Shopping) applying ItemAdded(item) forMax (1 seconds) goto(Shopping) applying ItemAdded(item) forMax (1 seconds)
case Event(StateTimeout, _) case Event(StateTimeout, _)
stop applying OrderDiscarded andThen { stop applying OrderDiscarded andThen {
case _ reportActor ! ShoppingCardDiscarded _ reportActor ! ShoppingCardDiscarded
} }
} }
@ -559,12 +571,12 @@ object PersistentFSMSpec {
class PersistentEventsStreamer(id: String, client: ActorRef) extends PersistentActor { class PersistentEventsStreamer(id: String, client: ActorRef) extends PersistentActor {
override val persistenceId: String = id override val persistenceId: String = id
def receiveRecover = { def receiveRecover: Receive = {
case RecoveryCompleted // do nothing case RecoveryCompleted // do nothing
case persistentEvent client ! persistentEvent case persistentEvent client ! persistentEvent
} }
def receiveCommand = { def receiveCommand: Receive = {
case _ // do nothing case _ // do nothing
} }
} }
@ -628,7 +640,7 @@ object PersistentFSMSpec {
case Event("4x", _) case Event("4x", _)
goto(Persist4xAtOnce) goto(Persist4xAtOnce)
case Event(SaveSnapshotSuccess(metadata), _) case Event(SaveSnapshotSuccess(metadata), _)
probe ! s"SeqNo=${metadata.sequenceNr}, StateData=${stateData}" probe ! s"SeqNo=${metadata.sequenceNr}, StateData=$stateData"
stay() stay()
} }
@ -636,7 +648,7 @@ object PersistentFSMSpec {
case Event(i: Int, _) case Event(i: Int, _)
stay applying (IntAdded(i), IntAdded(i), IntAdded(i), IntAdded(i)) stay applying (IntAdded(i), IntAdded(i), IntAdded(i), IntAdded(i))
case Event(SaveSnapshotSuccess(metadata), _) case Event(SaveSnapshotSuccess(metadata), _)
probe ! s"SeqNo=${metadata.sequenceNr}, StateData=${stateData}" probe ! s"SeqNo=${metadata.sequenceNr}, StateData=$stateData"
stay() stay()
} }
} }