snatch-dev / Chronicle

Implementation of saga pattern for .NET Core

MIT License

397 stars 70 forks source link

Redis and Ef Core supporting work #30

Closed nick-cromwell closed 4 years ago

nick-cromwell commented 5 years ago

Info

This Pull Request is related to issue no. [24] ([https://github.com/chronicle-stack/Chronicle/issues/24]) [6] ([https://github.com/chronicle-stack/Chronicle/issues/6])

Changes

Add ChronicleConfig and DeleteOnCompleted option
Inject ISagaStateRepository and IChronicleConfig into SagaPostProcessor to support DeleteOnCompleted
Rename various 'Id' and 'Type' fields to 'SagaId' and 'SagaType'
Make SagaId constructor public - needed to deserialize from JSON (for Redis and EF Core) avoiding having to write a custom JsonConverter
Remove the last sentence from ReadMe step 5 - innacurate, ChronicleException is thrown and compensate is not started

nick-cromwell commented 5 years ago

@GooRiOn If you could create a Chronicle.Integrations.EFCore repository I have that integration done as well.

nick-cromwell commented 5 years ago

@GooRiOn Do you have any feedback on this PR?

GooRiOn commented 5 years ago

This looks good in general. I'm just no sure whether we should remove stuff completely from the store. After all this could be handy for some business related analysis. Maybe we should consider soft delete instead?

nick-cromwell commented 5 years ago

I think adding Redis and EFCore support is more important than the need to delete Completed sagas so I will remove it from this PR. Actually, thinking about it further, I think if this use case needs to be handled it would be better done with OnRejected()/OnCompleted().

I will keep the ChronicleConfig to centralize possible future config options for things like issue 28 [https://github.com/snatch-dev/Chronicle/issues/28].

nick-cromwell commented 5 years ago

DeleteOnComplete removed.

nick-cromwell commented 5 years ago

This is needed for Redis

nick-cromwell commented 4 years ago

@GooRiOn Could you take a look at this and approve or close it. I have an EF Core store to add if you can approve this and add a Chronicle.Integrations.EFCore repository. I'll handle the 3.1 migration once this is either closed or approved.

nick-cromwell commented 4 years ago

I'm going to close this and submit a new PR. The project structure has changed quite a bit and there is a better way to handle the serialization for Redis without the need to make SagaID public.

I'll open a new PR shortly.