8000 Merge pull request #1326 from microsoft/trboehre/skillconversationid · ToucanToco/botbuilder-python@0847fd5 · GitHub
[go: up one dir, main page]
Skip to content

Commit 0847fd5

Browse files
tracyboehrertracyboehrer
authored
Merge pull request microsoft#1326 from microsoft/trboehre/skillconversationid
Refactored SkillDialog to call ConversationFacotry.CreateConversationId only once
2 parents e3eedd4 + a251259 commit 0847fd5

File tree

2 files changed

+34
-11
lines changed

2 files changed

+34
-11
lines changed

‎libraries/botbuilder-dialogs/botbuilder/dialogs/skills/skill_dialog.py

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
from copy import deepcopy
55
from typing import List
66

7+
from botframework.connector.token_api.models import TokenExchangeRequest
78
from botbuilder.schema import (
89
Activity,
910
ActivityTypes,
@@ -22,13 +23,16 @@
2223
DialogReason,
2324
DialogInstance,
2425
)
25-
from botframework.connector.token_api.models import TokenExchangeRequest
2626

2727
from .begin_skill_dialog_options import BeginSkillDialogOptions
2828
from .skill_dialog_options import SkillDialogOptions
2929

3030

3131
class SkillDialog(Dialog):
32+
SKILLCONVERSATIONIDSTATEKEY = (
33+
"Microsoft.Bot.Builder.Dialogs.SkillDialog.SkillConversationId"
34+
)
35+
3236
def __init__(self, dialog_options: SkillDialogOptions, dialog_id: str):
3337
super().__init__(dialog_id)
3438
if not dialog_options:
@@ -65,8 +69,18 @@ async def begin_dialog(self, dialog_context: DialogContext, options: object = No
6569
self._deliver_mode_state_key
6670
] = dialog_args.activity.delivery_mode
6771

72+
# Create the conversationId and store it in the dialog context state so we can use it later
73+
skill_conversation_id = await self._create_skill_conversation_id(
74+
dialog_context.context, dialog_context.context.activity
75+
)
76+
dialog_context.active_dialog.state[
77+
SkillDialog.SKILLCONVERSATIONIDSTATEKEY
78+
] = skill_conversation_id
79+
6880
# Send the activity to the skill.
69-
eoc_activity = await self._send_to_skill(dialog_context.context, skill_activity)
81+
eoc_activity = await self._send_to_skill(
82+
dialog_context.context, skill_activity, skill_conversation_id
83+
)
7084
if eoc_activity:
7185
return await dialog_context.end_dialog(eoc_activity.value)
7286

@@ -101,7 +115,12 @@ async def continue_dialog(self, dialog_context: DialogContext):
101115
]
102116

103117
# Just forward to the remote skill
104-
eoc_activity = await self._send_to_skill(dialog_context.context, skill_activity)
118+
skill_conversation_id = dialog_context.active_dialog.state[
119+
SkillDialog.SKILLCONVERSATIONIDSTATEKEY
120+
]
121+
eoc_activity = await self._send_to_skill(
122+
dialog_context.context, skill_activity, skill_conversation_id
123+
)
105124
if eoc_activity:
106125
return await dialog_context.end_dialog(eoc_activity.value)
107126

@@ -123,7 +142,8 @@ async def reprompt_dialog( # pylint: disable=unused-argument
123142
)
124143

125144
# connection Name is not applicable for a RePrompt, as we don't expect as OAuthCard in response.
126-
await self._send_to_skill(context, reprompt_event)
145+
skill_conversation_id = instance.state[SkillDialog.SKILLCONVERSATIONIDSTATEKEY]
146+
await self._send_to_skill(context, reprompt_event, skill_conversation_id)
127147

128148
async def resume_dialog( # pylint: disable=unused-argument
129149
self, dialog_context: "DialogContext", reason: DialogReason, result: object
@@ -152,7 +172,10 @@ async def end_dialog(
152172
activity.additional_properties = context.activity.additional_properties
153173

154174
# connection Name is not applicable for an EndDialog, as we don't expect as OAuthCard in response.
155-
await self._send_to_skill(context, activity)
175+
skill_conversation_id = instance.state[
176+
SkillDialog.SKILLCONVERSATIONIDSTATEKEY
177+
]
178+
await self._send_to_skill(context, activity, skill_conversation_id)
156179

157180
await super().end_dialog(context, instance, reason)
158181

@@ -187,18 +210,14 @@ def _on_validate_activity(
187210
return True
188211

189212
async def _send_to_skill(
190-
self, context: TurnContext, activity: Activity
213+
self, context: TurnContext, activity: Activity, skill_conversation_id: str
191214
) -> Activity:
192215
if activity.type == ActivityTypes.invoke:
193216
# Force ExpectReplies for invoke activities so we can get the replies right away and send
194217
# them back to the channel if needed. This makes sure that the dialog will receive the Invoke
195218
# response from the skill and any other activities sent, including EoC.
196219
activity.delivery_mode = DeliveryModes.expect_replies
197220

198-
skill_conversation_id = await self._create_skill_conversation_id(
199-
context, activity
200-
)
201-
202221
# Always save state before forwarding
203222
# (the dialog stack won't get updated with the skillDialog and things won't work if you don't)
204223
await self.dialog_options.conversation_state.save_changes(context, True)

‎libraries/botbuilder-dialogs/tests/test_skill_dialog.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
from unittest.mock import Mock
77

88
import aiounittest
9+
from botframework.connector.token_api.models import TokenExchangeResource
910
from botbuilder.core import (
1011
ConversationState,
1112
MemoryStorage,
@@ -40,7 +41,6 @@
4041
BeginSkillDialogOptions,
4142
DialogTurnStatus,
4243
)
43-
from botframework.connector.token_api.models import TokenExchangeResource
4444

4545

4646
class SimpleConversationIdFactory(ConversationIdFactoryBase):
@@ -148,10 +148,13 @@ async def capture(
148148
conversation_state=conversation_state,
149149
)
150150

151+
assert len(dialog_options.conversation_id_factory.conversation_refs) == 0
152+
151153
# Send something to the dialog to start it
152154
await client.send_activity(MessageFactory.text("irrelevant"))
153155

154156
# Assert results and data sent to the SkillClient for fist turn
157+
assert len(dialog_options.conversation_id_factory.conversation_refs) == 1
155158
assert dialog_options.bot_id == from_bot_id_sent
156159
assert dialog_options.skill.app_id == to_bot_id_sent
157160
assert dialog_options.skill.skill_endpoint == to_url_sent
@@ -162,6 +165,7 @@ async def capture(
162165
await client.send_activity(MessageFactory.text("Second message"))
163166

164167
# Assert results for second turn
168+
assert len(dialog_options.conversation_id_factory.conversation_refs) == 1
165169
assert activity_sent.text == "Second message"
166170
assert DialogTurnStatus.Waiting == client.dialog_turn_result.status
167171

0 commit comments

Comments
 (0)
0