eventuate-tram / eventuate-tram-sagas

Sagas for microservices
Other
994 stars 225 forks source link

When using reactive sagas, the data is not persisted as expected #99

Open rrrship opened 7 months ago

rrrship commented 7 months ago

I'm not sure if this is intentional or not, but with non-reactive sagas we're able to update the saga data in the reply handlers and it gets persisted automatically. Right now this doesn't seem to be the case with reactive approach, unless I'm doing something totally wrong myself? I've created an example of this issue in my fork repo: link to repo.

In this example both of these tests throw NPEs when trying to assert the data. These same validations are successful with non-reactive approach.

rrrship commented 7 months ago

Actually I found out one issue with my tests. I will try to make a new test case.

rrrship commented 7 months ago

Here is the updated version with the class which has the failing tests in my fork.

I looked around a little bit in the code and I noticed one difference between reactive and non-reactive, and it's in the ReactiveSagaManagerImpl.java file, line 182. There the data variable is passed to handleReply method, but it's coming from getSagaData(si) method call, while the data variable is also initialized separately on line 180. When I change line 182 to this, then these tests actually pass: Mono<SagaActions<Data>> actions = Mono.from(getStateDefinition().handleReply(sagaType, sagaId, currentState, data, message));

I'm wondering though if there might be some reason for doing it like this or it's a bug, @cer