Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kcqrs: simplify the aggregate to snapshot transformations #55

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mlesikov
Copy link
Contributor

Useless interfaces were deleted. The code were simplified.
Type adapters for the LocalDate and LocalDateTime were applied to the
snapshot output json, that will be stored in the datastore

Useless interfaces were deleted. The code were simplified.
Type adapters for the LocalDate and LocalDateTime were applied to the
snapshot output json, that will be stored in the datastore
@mlesikov
Copy link
Contributor Author

note that this can cause troubles with the existing format of the dates in the existing snapshots

@@ -49,7 +49,8 @@ class SimpleAggregateRepository(private val eventStore: EventStore,
is SaveEventsResponse.SnapshotRequired -> {
val currentAggregate = buildAggregateFromHistory(aggregateClass, response.currentEvents, aggregate.getId()!!, response.currentSnapshot)

val newSnapshot = currentAggregate.getSnapshotMapper().toSnapshot(currentAggregate)
val newSnapshot = Snapshot(currentAggregate.getExpectedVersion(), Binary(messageFormat.formatToString(currentAggregate)))
Copy link
Member

Choose a reason for hiding this comment

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

This couples the structure of the aggregate with the snapshot which doesn't sound like a good idea. getSnapshotMapper method could accept AggregateContext which to contain a reference to the MessageFormat. This will let the client code deside how and what snapshot to create.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

messageFormat.formatToString(currentAggregate)

the snapshot has a string content of what actually comes. I cannot understand your comment.
Maybe we can provide an interface SnapshotTransformer and use it instead of the messageFormat.

Copy link
Member

Choose a reason for hiding this comment

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

The idea is that different object could be used for the snapshot instead of the aggregate itself. The aggregate structure will change with time which means that there could be many versions of the Snapshot.

// get mapper which uses transfer to map -> the library enforces specific transformer.
getSnapshotMapper(SnapshotTransformer)

// get mapper which uses the provided snapshot context -> context is more implicit and the client code may use it or may not use it.
getSnapshotMapper(SnapshotContext)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants