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.

Sign up for GitHub

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: clone record before emitting event #833

Merged
merged 1 commit into from
Jun 3, 2022
Merged

fix: clone record before emitting event #833

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions packages/core/src/modules/basic-messages/services/BasicMessageService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import type { BasicMessageTags } from '../repository'
import { Lifecycle, scoped } from 'tsyringe'

import { EventEmitter } from '../../../agent/EventEmitter'
import { JsonTransformer } from '../../../utils'
import { BasicMessageEventTypes } from '../BasicMessageEvents'
import { BasicMessageRole } from '../BasicMessageRole'
import { BasicMessage } from '../messages'
Expand Down Expand Up @@ -33,10 +34,7 @@ export class BasicMessageService {
})

await this.basicMessageRepository.save(basicMessageRecord)
this.eventEmitter.emit<BasicMessageStateChangedEvent>({
type: BasicMessageEventTypes.BasicMessageStateChanged,
payload: { message: basicMessage, basicMessageRecord },
})
this.emitStateChangedEvent(basicMessageRecord, basicMessage)

return basicMessage
}
Expand All @@ -53,9 +51,14 @@ export class BasicMessageService {
})

await this.basicMessageRepository.save(basicMessageRecord)
this.emitStateChangedEvent(basicMessageRecord, message)
}

private emitStateChangedEvent(basicMessageRecord: BasicMessageRecord, basicMessage: BasicMessage) {
const clonedBasicMessageRecord = JsonTransformer.clone(basicMessageRecord)
this.eventEmitter.emit<BasicMessageStateChangedEvent>({
type: BasicMessageEventTypes.BasicMessageStateChanged,
payload: { message, basicMessageRecord },
payload: { message: basicMessage, basicMessageRecord: clonedBasicMessageRecord },
})
}

Expand Down
17 changes: 9 additions & 8 deletions packages/core/src/modules/connections/services/ConnectionService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,13 +181,7 @@ export class ConnectionService {
})

await this.connectionRepository.update(connectionRecord)
this.eventEmitter.emit<ConnectionStateChangedEvent>({
type: ConnectionEventTypes.ConnectionStateChanged,
payload: {
connectionRecord,
previousState: null,
},
})
this.emitStateChangedEvent(connectionRecord, null)

this.logger.debug(`Process message ${ConnectionRequestMessage.type} end`, connectionRecord)
return connectionRecord
Expand Down Expand Up @@ -509,10 +503,17 @@ export class ConnectionService {
connectionRecord.state = newState
await this.connectionRepository.update(connectionRecord)

this.emitStateChangedEvent(connectionRecord, previousState)
}

private emitStateChangedEvent(connectionRecord: ConnectionRecord, previousState: DidExchangeState | null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not

Suggested change
private emitStateChangedEvent(connectionRecord: ConnectionRecord, previousState: DidExchangeState | null) {
private emitStateChangedEvent(connectionRecord: ConnectionRecord, previousState?: DidExchangeState) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Events are emitted with previousState value null (which has a different meaning than undefined). If we change this now it would be a breaking change to events (because previousState will now be undefined)

// Connection record in event should be static
const clonedConnection = JsonTransformer.clone(connectionRecord)

this.eventEmitter.emit<ConnectionStateChangedEvent>({
type: ConnectionEventTypes.ConnectionStateChanged,
payload: {
connectionRecord,
connectionRecord: clonedConnection,
previousState,
},
})
Expand Down
36 changes: 6 additions & 30 deletions packages/core/src/modules/credentials/protocol/v1/V1CredentialService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import type { HandlerInboundMessage } from '../../../../agent/Handler'
import type { InboundMessageContext } from '../../../../agent/models/InboundMessageContext'
import type { Attachment } from '../../../../decorators/attachment/Attachment'
import type { ConnectionRecord } from '../../../connections'
import type { CredentialStateChangedEvent } from '../../CredentialEvents'
import type {
ServiceAcceptCredentialOptions,
CredentialOfferTemplate,
Expand Down Expand Up @@ -36,7 +35,6 @@ import { ConnectionService } from '../../../connections/services'
import { DidResolverService } from '../../../dids'
import { MediationRecipientService } from '../../../routing'
import { AutoAcceptCredential } from '../../CredentialAutoAcceptType'
import { CredentialEventTypes } from '../../CredentialEvents'
import { CredentialProtocolVersion } from '../../CredentialProtocolVersion'
import { CredentialState } from '../../CredentialState'
import { CredentialUtils } from '../../CredentialUtils'
Expand Down Expand Up @@ -177,13 +175,7 @@ export class V1CredentialService extends CredentialService {
associatedRecordId: credentialRecord.id,
})

this.eventEmitter.emit<CredentialStateChangedEvent>({
type: CredentialEventTypes.CredentialStateChanged,
payload: {
credentialRecord,
previousState: null,
},
})
this.emitStateChangedEvent(credentialRecord, null)

return { credentialRecord, message }
}
Expand Down Expand Up @@ -360,13 +352,8 @@ export class V1CredentialService extends CredentialService {

// Save record
await this.credentialRepository.save(credentialRecord)
this.eventEmitter.emit<CredentialStateChangedEvent>({
type: CredentialEventTypes.CredentialStateChanged,
payload: {
credentialRecord,
previousState: null,
},
})
this.emitStateChangedEvent(credentialRecord, null)

await this.didCommMessageRepository.saveAgentMessage({
agentMessage: proposalMessage,
role: DidCommMessageRole.Receiver,
Expand Down Expand Up @@ -536,13 +523,8 @@ export class V1CredentialService extends CredentialService {
const { credentialRecord, message } = await this.createOfferProcessing(template, credentialOptions.connection)

await this.credentialRepository.save(credentialRecord)
this.eventEmitter.emit<CredentialStateChangedEvent>({
type: CredentialEventTypes.CredentialStateChanged,
payload: {
credentialRecord,
previousState: null,
},
})
this.emitStateChangedEvent(credentialRecord, null)

await this.didCommMessageRepository.saveAgentMessage({
agentMessage: message,
role: DidCommMessageRole.Sender,
Expand Down Expand Up @@ -643,13 +625,7 @@ export class V1CredentialService extends CredentialService {
role: DidCommMessageRole.Receiver,
associatedRecordId: credentialRecord.id,
})
this.eventEmitter.emit<CredentialStateChangedEvent>({
type: CredentialEventTypes.CredentialStateChanged,
payload: {
credentialRecord,
previousState: null,
},
})
this.emitStateChangedEvent(credentialRecord, null)
}

return credentialRecord
Expand Down
33 changes: 4 additions & 29 deletions packages/core/src/modules/credentials/protocol/v2/V2CredentialService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import type { AgentMessage } from '../../../../agent/AgentMessage'
import type { HandlerInboundMessage } from '../../../../agent/Handler'
import type { InboundMessageContext } from '../../../../agent/models/InboundMessageContext'
import type { Attachment } from '../../../../decorators/attachment/Attachment'
import type { CredentialStateChangedEvent } from '../../CredentialEvents'
import type {
ServiceAcceptCredentialOptions,
CredentialProtocolMsgReturnType,
Expand Down Expand Up @@ -38,7 +37,6 @@ import { ConnectionService } from '../../../connections/services/ConnectionServi
import { DidResolverService } from '../../../dids'
import { MediationRecipientService } from '../../../routing'
import { AutoAcceptCredential } from '../../CredentialAutoAcceptType'
import { CredentialEventTypes } from '../../CredentialEvents'
import { CredentialProtocolVersion } from '../../CredentialProtocolVersion'
import { CredentialState } from '../../CredentialState'
import { CredentialFormatType } from '../../CredentialsModuleOptions'
Expand Down Expand Up @@ -128,14 +126,7 @@ export class V2CredentialService extends CredentialService {
this.logger.debug('Save meta data and emit state change event')

await this.credentialRepository.save(credentialRecord)

this.eventEmitter.emit<CredentialStateChangedEvent>({
type: CredentialEventTypes.CredentialStateChanged,
payload: {
credentialRecord,
previousState: null,
},
})
this.emitStateChangedEvent(credentialRecord, null)

await this.didCommMessageRepository.saveOrUpdateAgentMessage({
agentMessage: proposalMessage,
Expand Down Expand Up @@ -211,7 +202,7 @@ export class V2CredentialService extends CredentialService {
role: DidCommMessageRole.Receiver,
associatedRecordId: credentialRecord.id,
})
await this.emitEvent(credentialRecord)
this.emitStateChangedEvent(credentialRecord, null)
}
return credentialRecord
}
Expand Down Expand Up @@ -357,7 +348,7 @@ export class V2CredentialService extends CredentialService {
credentialRecord.connectionId = connection?.id

await this.credentialRepository.save(credentialRecord)
await this.emitEvent(credentialRecord)
this.emitStateChangedEvent(credentialRecord, null)

await this.didCommMessageRepository.saveOrUpdateAgentMessage({
agentMessage: credentialOfferMessage,
Expand Down Expand Up @@ -513,13 +504,7 @@ export class V2CredentialService extends CredentialService {
role: DidCommMessageRole.Receiver,
associatedRecordId: credentialRecord.id,
})
this.eventEmitter.emit<CredentialStateChangedEvent>({
type: CredentialEventTypes.CredentialStateChanged,
payload: {
credentialRecord,
previousState: null,
},
})
this.emitStateChangedEvent(credentialRecord, null)
}

return credentialRecord
Expand Down Expand Up @@ -1006,16 +991,6 @@ export class V2CredentialService extends CredentialService {
return this.serviceFormatMap[credentialFormatType]
}

private async emitEvent(credentialRecord: CredentialExchangeRecord) {
this.eventEmitter.emit<CredentialStateChangedEvent>({
type: CredentialEventTypes.CredentialStateChanged,
payload: {
credentialRecord,
previousState: null,
},
})
}

/**
* Retrieve a credential record by connection id and thread id
*
Expand Down
6 changes: 3 additions & 3 deletions ...ages/core/src/modules/credentials/protocol/v2/__tests__/v1credentials-auto-accept.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ describe('credentials', () => {
credentialRecordId: expect.any(String),
},
],
state: CredentialState.Done,
state: CredentialState.CredentialReceived,
})
expect(faberCredentialRecord).toMatchObject({
type: CredentialExchangeRecord.type,
Expand Down Expand Up @@ -249,7 +249,7 @@ describe('credentials', () => {
credentialRecordId: expect.any(String),
},
],
state: CredentialState.Done,
state: CredentialState.CredentialReceived,
})

expect(faberCredentialExchangeRecord).toMatchObject({
Expand Down Expand Up @@ -344,7 +344,7 @@ describe('credentials', () => {
credentialRecordId: expect.any(String),
},
],
state: CredentialState.Done,
state: CredentialState.CredentialReceived,
})

expect(faberCredentialExchangeRecord).toMatchObject({
Expand Down
6 changes: 3 additions & 3 deletions ...ages/core/src/modules/credentials/protocol/v2/__tests__/v2credentials-auto-accept.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ describe('credentials', () => {
},
},
},
state: CredentialState.Done,
state: CredentialState.CredentialReceived,
})
expect(faberCredentialRecord).toMatchObject({
type: CredentialExchangeRecord.type,
Expand Down Expand Up @@ -244,7 +244,7 @@ describe('credentials', () => {
},
},
},
state: CredentialState.Done,
state: CredentialState.CredentialReceived,
})

expect(faberCredentialRecord).toMatchObject({
Expand Down Expand Up @@ -332,7 +332,7 @@ describe('credentials', () => {
},
},
},
state: CredentialState.Done,
state: CredentialState.CredentialReceived,
})

expect(faberCredentialRecord).toMatchObject({
Expand Down
11 changes: 10 additions & 1 deletion packages/core/src/modules/credentials/services/CredentialService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ import type { V2RequestCredentialMessage } from './../protocol/v2/messages/V2Req
import type { CredentialExchangeRecord, CredentialRepository } from './../repository'
import type { RevocationService } from './RevocationService'

import { JsonTransformer } from '../../../utils'

import { CredentialEventTypes } from './../CredentialEvents'
import { CredentialState } from './../CredentialState'

Expand Down Expand Up @@ -210,14 +212,21 @@ export abstract class CredentialService {
credentialRecord.state = newState
await this.credentialRepository.update(credentialRecord)

this.emitStateChangedEvent(credentialRecord, previousState)
}

protected emitStateChangedEvent(credentialRecord: CredentialExchangeRecord, previousState: CredentialState | null) {
const clonedCredential = JsonTransformer.clone(credentialRecord)

this.eventEmitter.emit<CredentialStateChangedEvent>({
type: CredentialEventTypes.CredentialStateChanged,
payload: {
credentialRecord,
credentialRecord: clonedCredential,
previousState: previousState,
},
})
}

/**
* Retrieve a credential record by id
*
Expand Down
18 changes: 3 additions & 15 deletions packages/core/src/modules/oob/OutOfBandModule.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import type { Logger } from '../../logger'
import type { ConnectionRecord, Routing } from '../../modules/connections'
import type { PlaintextMessage } from '../../types'
import type { Key } from '../dids'
import type { HandshakeReusedEvent, OutOfBandStateChangedEvent } from './domain/OutOfBandEvents'
import type { HandshakeReusedEvent } from './domain/OutOfBandEvents'

import { parseUrl } from 'query-string'
import { catchError, EmptyError, first, firstValueFrom, map, of, timeout } from 'rxjs'
Expand Down Expand Up @@ -207,13 +207,7 @@ export class OutOfBandModule {
})

await this.outOfBandService.save(outOfBandRecord)
this.eventEmitter.emit<OutOfBandStateChangedEvent>({
type: OutOfBandEventTypes.OutOfBandStateChanged,
payload: {
outOfBandRecord,
previousState: null,
},
})
this.outOfBandService.emitStateChangedEvent(outOfBandRecord, null)

return outOfBandRecord
}
Expand Down Expand Up @@ -358,13 +352,7 @@ export class OutOfBandModule {
autoAcceptConnection,
})
await this.outOfBandService.save(outOfBandRecord)
this.eventEmitter.emit<OutOfBandStateChangedEvent>({
type: OutOfBandEventTypes.OutOfBandStateChanged,
payload: {
outOfBandRecord,
previousState: null,
},
})
this.outOfBandService.emitStateChangedEvent(outOfBandRecord, null)

if (autoAcceptInvitation) {
return await this.acceptInvitation(outOfBandRecord.id, {
Expand Down
9 changes: 8 additions & 1 deletion packages/core/src/modules/oob/OutOfBandService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { scoped, Lifecycle } from 'tsyringe'

import { EventEmitter } from '../../agent/EventEmitter'
import { AriesFrameworkError } from '../../error'
import { JsonTransformer } from '../../utils'

import { OutOfBandEventTypes } from './domain/OutOfBandEvents'
import { OutOfBandRole } from './domain/OutOfBandRole'
Expand Down Expand Up @@ -132,10 +133,16 @@ export class OutOfBandService {
outOfBandRecord.state = newState
await this.outOfBandRepository.update(outOfBandRecord)

this.emitStateChangedEvent(outOfBandRecord, previousState)
}

public emitStateChangedEvent(outOfBandRecord: OutOfBandRecord, previousState: OutOfBandState | null) {
const clonedOutOfBandRecord = JsonTransformer.clone(outOfBandRecord)

this.eventEmitter.emit<OutOfBandStateChangedEvent>({
type: OutOfBandEventTypes.OutOfBandStateChanged,
payload: {
outOfBandRecord,
outOfBandRecord: clonedOutOfBandRecord,
previousState,
},
})
Expand Down
Loading