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

Fix #1994 #1995

Merged
merged 7 commits into from
Aug 18, 2021
Merged

Fix #1994 #1995

merged 7 commits into from
Aug 18, 2021

Conversation

MrCheater
Copy link
Contributor

@MrCheater MrCheater commented Aug 12, 2021

Fix #1994
Incorrect: Timestamp from node.js Date.now()

const saveEvent = async (
runtime: AggregateRuntime,
aggregate: AggregateInterop,
command: Command,
event: Event
): Promise<any> => {
if (!isString(event.type)) {
throw generateCommandError(`Event "type" field is invalid`)
}
if (!isString(event.aggregateId)) {
throw generateCommandError('Event "aggregateId" field is invalid')
}
if (!isInteger(event.aggregateVersion)) {
throw generateCommandError('Event "aggregateVersion" field is invalid')
}
if (!isInteger(event.timestamp)) {
throw generateCommandError('Event "timestamp" field is invalid')
}
event.aggregateId = String(event.aggregateId)
const { eventstore, hooks: { preSaveEvent, postSaveEvent } = {} } = runtime
const allowSave =
typeof preSaveEvent === 'function'
? await preSaveEvent(aggregate, command, event)
: true
if (allowSave) {
const eventWithCursor = await eventstore.saveEvent(event)
if (typeof postSaveEvent === 'function') {
await postSaveEvent(aggregate, command, event, eventWithCursor)
}
}
return event
}

Correct: Timestamp from INSERT ... RETURNING "timestamp"
event: {
...event,
threadId: +rows[THREAD_COUNT].threadId,
threadCounter: +rows[THREAD_COUNT].newThreadCounter,
timestamp: +rows[THREAD_COUNT].timestamp,

@MrCheater MrCheater requested a review from const314 August 12, 2021 13:54
@MrCheater MrCheater requested a review from resolve-bot as a code owner August 12, 2021 13:54
@MrCheater MrCheater added the fix Use to fix bugs label Aug 12, 2021
Copy link
Collaborator

@const314 const314 left a comment

Choose a reason for hiding this comment

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

It would be nice to test that event modified by the event store is returned

@MrCheater MrCheater merged commit 520f8da into dev Aug 18, 2021
@MrCheater MrCheater deleted the fix/timestamp-from-command-result branch August 18, 2021 13:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix Use to fix bugs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Timestamps from CommandResult and Event are different
3 participants