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(core): messaging api #4340

Merged
merged 118 commits into from
Mar 9, 2021
Merged

feat(core): messaging api #4340

merged 118 commits into from
Mar 9, 2021

Conversation

samuelmasse
Copy link
Contributor

@samuelmasse samuelmasse commented Jan 5, 2021

The messaging API is meant to make it easier to send messages by abstracting some stuff from the events system. It also records a list of messages and conversations in its own tables. The goal is to move the behavior from the web channel to a more generic one in the core. This will allow, for example, to remove de dependency that the new hitl module has on the channel-web module.

Usage

Basic message sending

  const conversation = await bp.conversations.forBot(botId).recent(userId)

  const payload = { type: 'text', text: 'hello bot!' }
  const message = await bp.messages.forBot(botId).send(conversation.id, payload)

{userId, botId} being a user endpoint

The api remembers the last channel used by a user, so if you want to programmatically send a message you don't need to know the channel (as shown above).

However you will need to specify the channel in the args if you are programming a channel module. So for example the web channel does something like this :

Sending a message and specifying the channel of origin

const message = await bp.messages.forBot(botId).receive(conversationId, payload, {
  channel: 'web',
  credentials,
  debugger: useDebugger
})

It uses the optionnal args parameter to provide additionnal parameters to the event constructor. In this case channel will be used for the event and registered as the last channel used by the user.

Getting 100 most recent messages

const messages = await bp.messages.forBot(botId).list({ conversationId, limit: 100 })

Getting 100 most recent conversations from a user

const recentConversations = await bp.conversations.forBot(botId).list({ userId, limit: 100 })

This produces an array of conversations that each contain a lastMessage property

Storing conversations without using the event engine

const conversation = await bp.conversations.forBot(botId).create(userId)
const message = await bp.messages.forBot(botId).create(
  conversation.id,
  payload,
  'user',
)

@samuelmasse samuelmasse marked this pull request as draft January 5, 2021 19:52
EFF
EFF previously approved these changes Feb 23, 2021
Copy link
Member

@EFF EFF left a comment

Choose a reason for hiding this comment

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

Love this! Let's wait for @allardy to test with your telegram port then we're good !

@samuelmasse
Copy link
Contributor Author

Conversation and message ids are now uuids instead of auto increment integers.

I did some benchmarks to test if there was a difference in performance and to my surprise it wasn't noticeable at all. In fact it comes within the margin of error compared to integers. Here are some benchmarks I did using the sdk directly (100 000 users, 2 000 000 messages)

Create 100 000 users, create a converation for each and add 20 messages in each conversations

integers : 1193 seconds (1676 message inserts / sec)
uuid : 1177 second (1699 message inserts / sec)

For every user, get the most recent conversation

integers : 19 seconds (5263 / sec)
uuid : 17 seconds (5882 / sec)

For 1/10 of the users, get the most recent conversation. Then loop back and get the most recent conversations again 20 times (to test caching)

integers : 12.72 seconds (15 723 / sec)
uuid : 12.78 seconds (15 649 / sec)

For every user, get the most recent conversation and all of its messages

integers : 150 seconds (666 / sec)
uuid : 155 seconds (645 / sec)

I didn't see much of a difference on sqlite either. From what I've researched it's possible it doesn't scale as well with very large amounts of data. For example sqlite implements the field as a char(36). I think many other database handle uuids like that, however postgres actually has a builtin type for uuids. They are handled as 128 bit integers so the concerns for performance are probably not really justified. It's still worth considering options like in this article exist but I don't think it's really needed.

The only drawback to uuids in node is that they are represented in strings instead of 128 bit integers. So you get way more memory usage when stored in the cache. I also thought having a string as an id migth mislead sdk users to think they can put in anything as an id, so I created a uuid type to differentiate between actual string ids (like botId) and ids that need to be valid uuids

@samuelmasse samuelmasse requested a review from EFF February 24, 2021 16:17
src/bp/core/database/helpers.ts Outdated Show resolved Hide resolved
src/bp/core/database/helpers.ts Show resolved Hide resolved
src/bp/sdk/botpress.d.ts Outdated Show resolved Hide resolved
src/bp/core/services/messaging/messages.ts Show resolved Hide resolved
src/bp/sdk/botpress.d.ts Outdated Show resolved Hide resolved
src/bp/sdk/botpress.d.ts Outdated Show resolved Hide resolved
src/bp/sdk/botpress.d.ts Show resolved Hide resolved
src/bp/sdk/botpress.d.ts Show resolved Hide resolved
Copy link
Member

@EFF EFF left a comment

Choose a reason for hiding this comment

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

I think we're good to go !

@EFF EFF merged commit 6790ea7 into master Mar 9, 2021
@EFF EFF mentioned this pull request Mar 15, 2021
@samuelmasse samuelmasse deleted the sm-messaging branch April 14, 2021 21:14
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.

None yet

4 participants