Skip to content
This repository has been archived by the owner on Mar 14, 2024. It is now read-only.

Add the concept of saga completion. #109

Merged
merged 12 commits into from
Jun 11, 2018
Merged

Add the concept of saga completion. #109

merged 12 commits into from
Jun 11, 2018

Conversation

jmalloc
Copy link
Owner

@jmalloc jmalloc commented Jun 11, 2018

Fixes #16

Saga's can now be "completable". When an instance is completed, the saga persistence and mapping systems have an opportunity to remove persisted data. The exact behaviour is implementation depending, with the following behaviour implemented currently:

When a saga instance is completed, the:

  • eventsourcing persistence layer deletes all snapshots of the instance
  • crud persistence layer hard-deletes the instance
  • keyset mapping strategy hard-deletes message->instance mapping data

@jmalloc jmalloc requested a review from koden-km June 11, 2018 06:14
@codecov-io
Copy link

codecov-io commented Jun 11, 2018

Codecov Report

Merging #109 into master will decrease coverage by 0.95%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #109      +/-   ##
==========================================
- Coverage   30.24%   29.28%   -0.96%     
==========================================
  Files          65       65              
  Lines        2169     2240      +71     
==========================================
  Hits          656      656              
- Misses       1503     1574      +71     
  Partials       10       10
Impacted Files Coverage Δ
src/ax/aggregate/saga.go 0% <ø> (ø) ⬆️
.../ax/saga/persistence/eventsourcing/messagestore.go 0% <ø> (ø) ⬆️
src/ax/saga/persistence/eventsourcing/persister.go 0% <0%> (ø) ⬆️
src/ax/saga/handler.go 0% <0%> (ø) ⬆️
src/ax/saga/persistence/crud/persister.go 0% <0%> (ø) ⬆️
src/axmysql/saga/snapshot.go 0% <0%> (ø) ⬆️
src/ax/saga/mapping/direct/mapper.go 0% <0%> (ø) ⬆️
src/axmysql/saga/keyset.go 0% <0%> (ø) ⬆️
src/ax/saga/behavior.go 0% <0%> (ø) ⬆️
src/axmysql/saga/crud.go 0% <0%> (ø) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 892b84f...e88551d. Read the comment docs.

@@ -24,3 +24,18 @@ type ErrorIfNotFound struct{}
func (ErrorIfNotFound) HandleNotFound(_ context.Context, _ ax.Sender, _ ax.Envelope) error {
return errors.New("could not find a saga instance to handle message")
}

// CompletableByData is an embeddable struct implements a
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might need a "that" inserted? Something like "struct that implements" ?

embeddable struct implements a

type CompletableByData struct{}

// IsComplete returns true if i.Data implements CompletableData and
// i.Data.IsComplete() returns true.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be i.Data.IsInstanceComplete() ?

// DeleteSagaSnapshots deletes any snapshots associated with a saga instance.
//
// The implementation may return an error if snapshots for this instance
// already exists, but belong to a different saga, as identified by pk, the
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An error if the snapshots for this instance already exist? Is that correct?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is trying to delete them.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's correct, but maybe taking a way the word "already" would be good:

	// It returns an error if the instance exists but belongs to a
	// different saga, as identified by pk, the saga's persistence key.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or even just "if the instance belongs to a different saga"

Also improved some documentation around the verification of persistence keys.
@jmalloc jmalloc merged commit 1dc6148 into master Jun 11, 2018
@jmalloc jmalloc deleted the 16-saga-completion branch June 11, 2018 22:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants