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

feat: support handling messages with different minor version #714

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
4b6951a
refactor: message type object on messages
TimoGlastra Apr 19, 2022
5a9452b
feat: support message types with lower minor version
TimoGlastra Apr 19, 2022
869c784
fix: message type validation
TimoGlastra Apr 20, 2022
90d5e08
test: e2e tests for multi version protocol
TimoGlastra Apr 20, 2022
c32859c
feat: retrieve didcomm message using major version
TimoGlastra Apr 20, 2022
c559ad3
test: fix duplicate wallet name
TimoGlastra Apr 20, 2022
ee2601a
test: fix broken tests
TimoGlastra Apr 20, 2022
53312ba
Merge branch 'main' into feat/support-multiple-versions-protocol
TimoGlastra Apr 20, 2022
451cf62
Merge branch 'main' into feat/support-multiple-versions-protocol
TimoGlastra Apr 20, 2022
24f2d8c
fix: allow higher minor versions for protocol
TimoGlastra Apr 20, 2022
8552cfa
refactor: canHandleMessageType
TimoGlastra Apr 21, 2022
aa1484d
Merge branch 'main' into feat/support-multiple-versions-protocol
TimoGlastra Apr 26, 2022
1ea48d1
Merge branch 'main' into feat/support-multiple-versions-protocol
TimoGlastra Apr 27, 2022
aa07fa1
fix: message type for revocation messages
TimoGlastra Apr 27, 2022
fe4f843
Merge branch 'main' into feat/support-multiple-versions-protocol
TimoGlastra May 1, 2022
d46b570
Merge remote-tracking branch 'upstream/main' into feat/support-multip…
TimoGlastra May 11, 2022
b8186d7
fix: changes after merge
TimoGlastra May 11, 2022
42f45a1
Merge branch 'main' into feat/support-multiple-versions-protocol
TimoGlastra May 13, 2022
4392906
fix: fixes after merge
TimoGlastra May 13, 2022
2f1f0e5
test: wait for polling to stop in mediator tests
TimoGlastra May 14, 2022
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
24 changes: 11 additions & 13 deletions packages/core/src/agent/AgentMessage.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
import type { ParsedMessageType } from '../utils/messageType'
import type { Constructor } from '../utils/mixins'

import { AckDecorated } from '../decorators/ack/AckDecoratorExtension'
import { AttachmentDecorated } from '../decorators/attachment/AttachmentExtension'
import { L10nDecorated } from '../decorators/l10n/L10nDecoratorExtension'
Expand All @@ -7,21 +10,16 @@ import { TimingDecorated } from '../decorators/timing/TimingDecoratorExtension'
import { TransportDecorated } from '../decorators/transport/TransportDecoratorExtension'
import { JsonTransformer } from '../utils/JsonTransformer'
import { replaceNewDidCommPrefixWithLegacyDidSovOnMessage } from '../utils/messageType'
import { Compose } from '../utils/mixins'

import { BaseMessage } from './BaseMessage'

const DefaultDecorators = [
ThreadDecorated,
L10nDecorated,
TransportDecorated,
TimingDecorated,
AckDecorated,
AttachmentDecorated,
ServiceDecorated,
]

export class AgentMessage extends Compose(BaseMessage, DefaultDecorators) {
export type ConstructableAgentMessage = Constructor<AgentMessage> & { type: ParsedMessageType }

const Decorated = ThreadDecorated(
L10nDecorated(TransportDecorated(TimingDecorated(AckDecorated(AttachmentDecorated(ServiceDecorated(BaseMessage))))))
Copy link
Contributor

Choose a reason for hiding this comment

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

It almost looks like LISP here :)

)

export class AgentMessage extends Decorated {
public toJSON({ useLegacyDidSovPrefix = false }: { useLegacyDidSovPrefix?: boolean } = {}): Record<string, unknown> {
const json = JsonTransformer.toJSON(this)

Expand All @@ -33,6 +31,6 @@ export class AgentMessage extends Compose(BaseMessage, DefaultDecorators) {
}

public is<C extends typeof AgentMessage>(Class: C): this is InstanceType<C> {
return this.type === Class.type
return this.type === Class.type.messageTypeUri
}
}
3 changes: 2 additions & 1 deletion packages/core/src/agent/BaseMessage.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type { ParsedMessageType } from '../utils/messageType'
import type { Constructor } from '../utils/mixins'

import { Expose } from 'class-transformer'
Expand All @@ -18,7 +19,7 @@ export class BaseMessage {
@Expose({ name: '@type' })
@Matches(MessageTypeRegExp)
public readonly type!: string
public static readonly type: string
public static readonly type: ParsedMessageType

public generateId() {
return uuid()
Expand Down
11 changes: 8 additions & 3 deletions packages/core/src/agent/Dispatcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { Lifecycle, scoped } from 'tsyringe'

import { AgentConfig } from '../agent/AgentConfig'
import { AriesFrameworkError } from '../error/AriesFrameworkError'
import { canHandleMessageType, parseMessageType } from '../utils/messageType'

import { ProblemReportMessage } from './../modules/problem-reports/messages/ProblemReportMessage'
import { EventEmitter } from './EventEmitter'
Expand Down Expand Up @@ -92,17 +93,21 @@ class Dispatcher {
}

private getHandlerForType(messageType: string): Handler | undefined {
const incomingMessageType = parseMessageType(messageType)

for (const handler of this.handlers) {
for (const MessageClass of handler.supportedMessages) {
if (MessageClass.type === messageType) return handler
if (canHandleMessageType(MessageClass, incomingMessageType)) return handler
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this might be a big cleaner

return this.handlers.forEach((handler) => handler.supportedMessages.find((message) => message.type === messageType))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This won't work because the forEach doesn't return anything. I could change it to find, but then it would return the handler and not the message class.

Any other suggestions on how to approach?

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe that swapping the forEach with a find would work.


public getMessageClassForType(messageType: string): typeof AgentMessage | undefined {
const incomingMessageType = parseMessageType(messageType)

for (const handler of this.handlers) {
for (const MessageClass of handler.supportedMessages) {
if (MessageClass.type === messageType) return MessageClass
if (canHandleMessageType(MessageClass, incomingMessageType)) return MessageClass
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here, could be a bit cleaner.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as above

Copy link
Contributor

@berendsliedrecht berendsliedrecht May 2, 2022

Choose a reason for hiding this comment

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

return this.handlers().flatMap((handler) => handler.supportedMessages).find((MessageClass) => canHandleMessageType(MessageClass, incomingMessageType))

Copy link
Contributor

Choose a reason for hiding this comment

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

If flatMap is not supported:

return Array.prototype.concat(...this.handlers.map((handler) => handler.supportedMessages).find((MessageClass) => canHandleMessageType(MessageClass, incomingMessageType))

Expand All @@ -122,7 +127,7 @@ class Dispatcher {
* Protocol ID format is PIURI specified at https://github.com/hyperledger/aries-rfcs/blob/main/concepts/0003-protocols/README.md#piuri.
*/
public get supportedProtocols() {
return Array.from(new Set(this.supportedMessageTypes.map((m) => m.substring(0, m.lastIndexOf('/')))))
return Array.from(new Set(this.supportedMessageTypes.map((m) => m.protocolUri)))
}

public filterSupportedProtocolsByMessageFamilies(messageFamilies: string[]) {
Expand Down
6 changes: 3 additions & 3 deletions packages/core/src/agent/Handler.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import type { OutboundMessage, OutboundServiceMessage } from '../types'
import type { AgentMessage } from './AgentMessage'
import type { ConstructableAgentMessage } from './AgentMessage'
import type { InboundMessageContext } from './models/InboundMessageContext'

export interface Handler<T extends typeof AgentMessage = typeof AgentMessage> {
readonly supportedMessages: readonly T[]
export interface Handler {
readonly supportedMessages: readonly ConstructableAgentMessage[]

handle(messageContext: InboundMessageContext): Promise<OutboundMessage | OutboundServiceMessage | void>
}
Expand Down
7 changes: 4 additions & 3 deletions packages/core/src/agent/MessageReceiver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { ProblemReportError, ProblemReportMessage, ProblemReportReason } from '.
import { isValidJweStructure } from '../utils/JWE'
import { JsonTransformer } from '../utils/JsonTransformer'
import { MessageValidator } from '../utils/MessageValidator'
import { replaceLegacyDidSovPrefixOnMessage } from '../utils/messageType'
import { canHandleMessageType, parseMessageType, replaceLegacyDidSovPrefixOnMessage } from '../utils/messageType'

import { AgentConfig } from './AgentConfig'
import { Dispatcher } from './Dispatcher'
Expand Down Expand Up @@ -247,8 +247,9 @@ export class MessageReceiver {
connection: ConnectionRecord,
plaintextMessage: PlaintextMessage
) {
if (plaintextMessage['@type'] === ProblemReportMessage.type) {
throw new AriesFrameworkError(message)
const messageType = parseMessageType(plaintextMessage['@type'])
if (canHandleMessageType(ProblemReportMessage, messageType)) {
throw new AriesFrameworkError(`Not sending problem report in response to problem report: {message}`)
TimoGlastra marked this conversation as resolved.
Show resolved Hide resolved
}
const problemReportMessage = new ProblemReportMessage({
description: {
Expand Down
64 changes: 64 additions & 0 deletions packages/core/src/agent/__tests__/AgentMessage.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,14 @@
import { TestMessage } from '../../../tests/TestMessage'
import { JsonTransformer } from '../../utils'
import { MessageValidator } from '../../utils/MessageValidator'
import { IsValidMessageType, parseMessageType } from '../../utils/messageType'
import { AgentMessage } from '../AgentMessage'

class CustomProtocolMessage extends AgentMessage {
@IsValidMessageType(CustomProtocolMessage.type)
public readonly type = CustomProtocolMessage.type.messageTypeUri
public static readonly type = parseMessageType('https://didcomm.org/fake-protocol/1.5/message')
}

describe('AgentMessage', () => {
describe('toJSON', () => {
Expand All @@ -12,4 +22,58 @@ describe('AgentMessage', () => {
expect(jsonSov['@type']).toBe('did:sov:BzCbsNYhMrjHiqZDTUASHg;spec/connections/1.0/invitation')
})
})

describe('@IsValidMessageType', () => {
it('successfully validates if the message type is exactly the supported message type', async () => {
const json = {
'@id': 'd61c7e3d-d4af-469b-8d42-33fd14262e17',
'@type': 'https://didcomm.org/fake-protocol/1.5/message',
}

const message = JsonTransformer.fromJSON(json, CustomProtocolMessage)

await expect(MessageValidator.validate(message)).resolves.toBeUndefined()
})

it('successfully validates if the message type minor version is lower than the supported message type', async () => {
const json = {
'@id': 'd61c7e3d-d4af-469b-8d42-33fd14262e17',
'@type': 'https://didcomm.org/fake-protocol/1.2/message',
}

const message = JsonTransformer.fromJSON(json, CustomProtocolMessage)

await expect(MessageValidator.validate(message)).resolves.toBeUndefined()
})

it('successfully validates if the message type minor version is higher than the supported message type', async () => {
const json = {
'@id': 'd61c7e3d-d4af-469b-8d42-33fd14262e17',
'@type': 'https://didcomm.org/fake-protocol/1.8/message',
}

const message = JsonTransformer.fromJSON(json, CustomProtocolMessage)

await expect(MessageValidator.validate(message)).resolves.toBeUndefined()
})

it('throws a validation error if the message type major version differs from the supported message type', async () => {
expect.assertions(1)

const json = {
'@id': 'd61c7e3d-d4af-469b-8d42-33fd14262e17',
'@type': 'https://didcomm.org/fake-protocol/2.0/message',
}

const message = JsonTransformer.fromJSON(json, CustomProtocolMessage)

await expect(MessageValidator.validate(message)).rejects.toMatchObject([
{
TimoGlastra marked this conversation as resolved.
Show resolved Hide resolved
constraints: {
isValidMessageType: 'type does not match the expected message type (only minor version may be lower)',
},
},
])
})
})
})
93 changes: 79 additions & 14 deletions packages/core/src/agent/__tests__/Dispatcher.test.ts
Original file line number Diff line number Diff line change
@@ -1,27 +1,35 @@
import type { Handler } from '../Handler'

import { getAgentConfig } from '../../../tests/helpers'
import { parseMessageType } from '../../utils/messageType'
import { AgentMessage } from '../AgentMessage'
import { Dispatcher } from '../Dispatcher'
import { EventEmitter } from '../EventEmitter'
import { MessageSender } from '../MessageSender'
import { InboundMessageContext } from '../models/InboundMessageContext'

class ConnectionInvitationTestMessage extends AgentMessage {
public static readonly type = 'https://didcomm.org/connections/1.0/invitation'
public static readonly type = parseMessageType('https://didcomm.org/connections/1.0/invitation')
}
class ConnectionRequestTestMessage extends AgentMessage {
public static readonly type = 'https://didcomm.org/connections/1.0/request'
public static readonly type = parseMessageType('https://didcomm.org/connections/1.0/request')
}

class ConnectionResponseTestMessage extends AgentMessage {
public static readonly type = 'https://didcomm.org/connections/1.0/response'
public static readonly type = parseMessageType('https://didcomm.org/connections/1.0/response')
}

class NotificationAckTestMessage extends AgentMessage {
public static readonly type = 'https://didcomm.org/notification/1.0/ack'
public static readonly type = parseMessageType('https://didcomm.org/notification/1.0/ack')
}
class CredentialProposalTestMessage extends AgentMessage {
public static readonly type = 'https://didcomm.org/issue-credential/1.0/credential-proposal'
public readonly type = CredentialProposalTestMessage.type.messageTypeUri
public static readonly type = parseMessageType('https://didcomm.org/issue-credential/1.0/credential-proposal')
}

class CustomProtocolMessage extends AgentMessage {
public readonly type = CustomProtocolMessage.type.messageTypeUri
public static readonly type = parseMessageType('https://didcomm.org/fake-protocol/1.5/message')
}

class TestHandler implements Handler {
Expand All @@ -42,25 +50,31 @@ describe('Dispatcher', () => {
const agentConfig = getAgentConfig('DispatcherTest')
const MessageSenderMock = MessageSender as jest.Mock<MessageSender>
const eventEmitter = new EventEmitter(agentConfig)
const fakeProtocolHandler = new TestHandler([CustomProtocolMessage])
const connectionHandler = new TestHandler([
ConnectionInvitationTestMessage,
ConnectionRequestTestMessage,
ConnectionResponseTestMessage,
])

const dispatcher = new Dispatcher(new MessageSenderMock(), eventEmitter, agentConfig)

dispatcher.registerHandler(
new TestHandler([ConnectionInvitationTestMessage, ConnectionRequestTestMessage, ConnectionResponseTestMessage])
)
dispatcher.registerHandler(connectionHandler)
dispatcher.registerHandler(new TestHandler([NotificationAckTestMessage]))
dispatcher.registerHandler(new TestHandler([CredentialProposalTestMessage]))
dispatcher.registerHandler(fakeProtocolHandler)

describe('supportedMessageTypes', () => {
test('return all supported message types URIs', async () => {
const messageTypes = dispatcher.supportedMessageTypes

expect(messageTypes).toEqual([
'https://didcomm.org/connections/1.0/invitation',
'https://didcomm.org/connections/1.0/request',
'https://didcomm.org/connections/1.0/response',
'https://didcomm.org/notification/1.0/ack',
'https://didcomm.org/issue-credential/1.0/credential-proposal',
expect(messageTypes).toMatchObject([
{ messageTypeUri: 'https://didcomm.org/connections/1.0/invitation' },
{ messageTypeUri: 'https://didcomm.org/connections/1.0/request' },
{ messageTypeUri: 'https://didcomm.org/connections/1.0/response' },
{ messageTypeUri: 'https://didcomm.org/notification/1.0/ack' },
{ messageTypeUri: 'https://didcomm.org/issue-credential/1.0/credential-proposal' },
{ messageTypeUri: 'https://didcomm.org/fake-protocol/1.5/message' },
])
})
})
Expand All @@ -73,6 +87,7 @@ describe('Dispatcher', () => {
'https://didcomm.org/connections/1.0',
'https://didcomm.org/notification/1.0',
'https://didcomm.org/issue-credential/1.0',
'https://didcomm.org/fake-protocol/1.5',
])
})
})
Expand All @@ -98,4 +113,54 @@ describe('Dispatcher', () => {
expect(supportedProtocols).toEqual(['https://didcomm.org/connections/1.0'])
})
})

describe('getMessageClassForType()', () => {
it('should return the correct message class for a registered message type', () => {
const messageClass = dispatcher.getMessageClassForType('https://didcomm.org/connections/1.0/invitation')
expect(messageClass).toBe(ConnectionInvitationTestMessage)
})

it('should return undefined if no message class is registered for the message type', () => {
const messageClass = dispatcher.getMessageClassForType('https://didcomm.org/non-existing/1.0/invitation')
expect(messageClass).toBeUndefined()
})

it('should return the message class with a higher minor version for the message type', () => {
const messageClass = dispatcher.getMessageClassForType('https://didcomm.org/fake-protocol/1.0/message')
expect(messageClass).toBe(CustomProtocolMessage)
})

it('should not return the message class with a different major version', () => {
const messageClass = dispatcher.getMessageClassForType('https://didcomm.org/fake-protocol/2.0/message')
expect(messageClass).toBeUndefined()
})
})

describe('dispatch()', () => {
it('calls the handle method of the handler', async () => {
const dispatcher = new Dispatcher(new MessageSenderMock(), eventEmitter, agentConfig)
const customProtocolMessage = new CustomProtocolMessage()
const inboundMessageContext = new InboundMessageContext(customProtocolMessage)

const mockHandle = jest.fn()
dispatcher.registerHandler({ supportedMessages: [CustomProtocolMessage], handle: mockHandle })

await dispatcher.dispatch(inboundMessageContext)

expect(mockHandle).toHaveBeenNthCalledWith(1, inboundMessageContext)
})

it('throws an error if no handler for the message could be found', async () => {
const dispatcher = new Dispatcher(new MessageSenderMock(), eventEmitter, agentConfig)
const customProtocolMessage = new CustomProtocolMessage()
const inboundMessageContext = new InboundMessageContext(customProtocolMessage)

const mockHandle = jest.fn()
dispatcher.registerHandler({ supportedMessages: [], handle: mockHandle })

await expect(dispatcher.dispatch(inboundMessageContext)).rejects.toThrow(
'No handler for message type "https://didcomm.org/fake-protocol/1.5/message" found'
)
})
})
})
1 change: 1 addition & 0 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export * from './utils/JsonTransformer'
export * from './logger'
export * from './error'
export * from './wallet/error'
export { parseMessageType, IsValidMessageType } from './utils/messageType'

export * from './agent/Events'

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { Expose, Transform } from 'class-transformer'
import { Equals, IsDate, IsString } from 'class-validator'
import { IsDate, IsString } from 'class-validator'

import { AgentMessage } from '../../../agent/AgentMessage'
import { IsValidMessageType, parseMessageType } from '../../../utils/messageType'
import { DateParser } from '../../../utils/transformers'

export class BasicMessage extends AgentMessage {
Expand All @@ -21,9 +22,9 @@ export class BasicMessage extends AgentMessage {
}
}

@Equals(BasicMessage.type)
public readonly type = BasicMessage.type
public static readonly type = 'https://didcomm.org/basicmessage/1.0/message'
@IsValidMessageType(BasicMessage.type)
public readonly type = BasicMessage.type.messageTypeUri
public static readonly type = parseMessageType('https://didcomm.org/basicmessage/1.0/message')

@Expose({ name: 'sent_time' })
@Transform(({ value }) => DateParser(value))
Expand Down
Loading