8000 SkillDialog activity type handling updates (#1057) · microsoft/botbuilder-python@cecd772 · GitHub
[go: up one dir, main page]

Skip to content

Commit cecd772

Browse files
tracyboehreraxelsrz
authored andcommitted
SkillDialog activity type handling updates (#1057)
1 parent 0f279cf commit cecd772

File tree

6 files changed

+163
-62
lines changed

6 files changed

+163
-62
lines changed

libraries/botbuilder-core/botbuilder/core/bot_framework_adapter.py

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -481,14 +481,8 @@ async def process_activity_with_identity(
481481

482482
await self.run_pipeline(context, logic)
483483

484-
if activity.type == ActivityTypes.invoke:
485-
invoke_response = context.turn_state.get(
486-
BotFrameworkAdapter._INVOKE_RESPONSE_KEY # pylint: disable=protected-access
487-
)
488-
if invoke_response is None:
489-
return InvokeResponse(status=int(HTTPStatus.NOT_IMPLEMENTED))
490-
return invoke_response.value
491-
484+
# Handle ExpectedReplies scenarios where the all the activities have been buffered and sent back at once
485+
# in an invoke response.
492486
# Return the buffered activities in the response. In this case, the invoker
493487
# should deserialize accordingly:
494488
# activities = ExpectedReplies().deserialize(response.body).activities
@@ -498,6 +492,16 @@ async def process_activity_with_identity(
498492
).serialize()
499493
return InvokeResponse(status=int(HTTPStatus.OK), body=expected_replies)
500494

495+
# Handle Invoke scenarios, which deviate from the request/request model in that
496+
# the Bot will return a specific body and return code.
497+
if activity.type == ActivityTypes.invoke:
498+
invoke_response = context.turn_state.get(
499+
BotFrameworkAdapter._INVOKE_RESPONSE_KEY # pylint: disable=protected-access
500+
)
501+
if invoke_response is None:
502+
return InvokeResponse(status=int(HTTPStatus.NOT_IMPLEMENTED))
503+
return invoke_response.value
504+
501505
return None
502506

503507
async def __generate_callerid(self, claims_identity: ClaimsIdentity) -> str:

libraries/botbuilder-core/botbuilder/core/invoke_response.py

Lines changed: 8 additions & 0 deletions
< 6D40 tr class="diff-line-row">
Original file line numberDiff line numberDiff line change
@@ -24,3 +24,11 @@ def __init__(self, status: int = None, body: object = None):
2424
"""
2525
self.status = status
2626
self.body = body
27+
28+
def is_successful_status_code(self) -> bool:
29+
"""
30+
Gets a value indicating whether the invoke response was successful.
31+
:return: A value that indicates if the HTTP response was successful. true if status is in
32+
the Successful range (200-299); otherwise false.
33+
"""
34+
return 200 <= self.status <= 299

libraries/botbuilder-dialogs/botbuilder/dialogs/skills/begin_skill_dialog_options.py

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,19 +5,13 @@
55

66

77
class BeginSkillDialogOptions:
8-
def __init__(
9-
self, activity: Activity, connection_name: str = None
10-
): # pylint: disable=unused-argument
8+
def __init__(self, activity: Activity):
119
self.activity = activity
12-
self.connection_name = connection_name
1310

1411
@staticmethod
1512
def from_object(obj: object) -> "BeginSkillDialogOptions":
1613
if isinstance(obj, dict) and "activity" in obj:
17-
return BeginSkillDialogOptions(obj["activity"], obj.get("connection_name"))
14+
return BeginSkillDialogOptions(obj["activity"])
1815
if hasattr(obj, "activity"):
19-
return BeginSkillDialogOptions(
20-
obj.activity,
21-
obj.connection_name if hasattr(obj, "connection_name") else None,
22-
)
16+
return BeginSkillDialogOptions(obj.activity)
2317
return None

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

Lines changed: 54 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -36,15 +36,14 @@ def __init__(self, dialog_options: SkillDialogOptions, dialog_id: str):
3636

3737
self.dialog_options = dialog_options
3838
self._deliver_mode_state_key = "deliverymode"
39-
self._sso_connection_name_key = "SkillDialog.SSOConnectionName"
4039

4140
async def begin_dialog(self, dialog_context: DialogContext, options: object = None):
42 F438 41
"""
4342
Method called when a new dialog has been pushed onto the stack and is being activated.
4443
:param dialog_context: The dialog context for the current turn of conversation.
4544
:param options: (Optional) additional argument(s) to pass to the dialog being started.
4645
"""
47-
dialog_args = SkillDialog._validate_begin_dialog_args(options)
46+
dialog_args = self._validate_begin_dialog_args(options)
4847

4948
await dialog_context.context.send_trace_activity(
5049
f"{SkillDialog.__name__}.BeginDialogAsync()",
@@ -61,24 +60,22 @@ async def begin_dialog(self, dialog_context: DialogContext, options: object = No
6160
is_incoming=True,
6261
)
6362

63+
# Store delivery mode in dialog state for later use.
6464
dialog_context.active_dialog.state[
6565
self._deliver_mode_state_key
6666
] = dialog_args.activity.delivery_mode
6767

68-
dialog_context.active_dialog.state[
69-
self._sso_connection_name_key
70-
] = dialog_args.connection_name
71-
7268
# Send the activity to the skill.
73-
eoc_activity = await self._send_to_skill(
74-
dialog_context.context, skill_activity, dialog_args.connection_name
75-
)
69+
eoc_activity = await self._send_to_skill(dialog_context.context, skill_activity)
7670
if eoc_activity:
7771
return await dialog_context.end_dialog(eoc_activity.value)
7872

7973
return self.end_of_turn
8074

8175
async def continue_dialog(self, dialog_context: DialogContext):
76+
if not self._on_validate_activity(dialog_context.context.activity):
77+
return self.end_of_turn
78+
8279
await dialog_context.context.send_trace_activity(
8380
f"{SkillDialog.__name__}.continue_dialog()",
8481
label=f"ActivityType: {dialog_context.context.activity.type}",
@@ -98,17 +95,13 @@ async def continue_dialog(self, dialog_context: DialogContext):
9895

9996
# Create deep clone of the original activity to avoid altering it before forwarding it.
10097
skill_activity = deepcopy(dialog_context.context.activity)
98+
10199
skill_activity.delivery_mode = dialog_context.active_dialog.state[
102100
self._deliver_mode_state_key
103101
]
104-
connection_name = dialog_context.active_dialog.state[
105-
self._sso_connection_name_key
106-
]
107102

108103
# Just forward to the remote skill
109-
eoc_activity = await self._send_to_skill(
110-
dialog_context.context, skill_activity, connection_name
111-
)
104+
eoc_activity = await self._send_to_skill(dialog_context.context, skill_activity)
112105
if eoc_activity:
113106
return await dialog_context.end_dialog(eoc_activity.value)
114107

@@ -163,8 +156,7 @@ async def end_dialog(
163156

164157
await super().end_dialog(context, instance, reason)
165158

166-
@staticmethod
167-
def _validate_begin_dialog_args(options: object) -> BeginSkillDialogOptions:
159+
def _validate_begin_dialog_args(self, options: object) -> BeginSkillDialogOptions:
168160
if not options:
169161
raise TypeError("options cannot be None.")
170162

@@ -182,26 +174,36 @@ def _validate_begin_dialog_args(options: object) -> BeginSkillDialogOptions:
182174

183175
return dialog_args
184176

177+
def _on_validate_activity(
178+
self, activity: Activity # pylint: disable=unused-argument
179+
) -> bool:
180+
"""
181+
Validates the activity sent during continue_dialog.
182+
183+
Override this method to implement a custom validator for the activity being sent during continue_dialog.
184+
This method can be used to ignore activities of a certain type if needed.
185+
If this method returns false, the dialog will end the turn without processing the activity.
186+
"""
187+
return True
188+
185189
async def _send_to_skill(
186-
self, context: TurnContext, activity: Activity, connection_name: str = None
190+
self, context: TurnContext, activity: Activity
187191
) -> Activity:
188-
# Create a conversationId to interact with the skill and send the activity
189-
conversation_id_factory_options = SkillConversationIdFactoryOptions(
190-
from_bot_oauth_scope=context.turn_state.get(BotAdapter.BOT_OAUTH_SCOPE_KEY),
191-
from_bot_id=self.dialog_options.bot_id,
192-
activity=activity,
193-
bot_framework_skill=self.dialog_options.skill,
194-
)
195-
196-
skill_conversation_id = await self.dialog_options.conversation_id_factory.create_skill_conversation_id(
197-
conversation_id_factory_options
192+
if activity.type == ActivityTypes.invoke:
193+
# Force ExpectReplies for invoke activities so we can get the replies right away and send
194+
# them back to the channel if needed. This makes sure that the dialog will receive the Invoke
195+
# response from the skill and any other activities sent, including EoC.
196+
activity.delivery_mode = DeliveryModes.expect_replies
197+
198+
skill_conversation_id = await self._create_skill_conversation_id(
199+
context, activity
198200
)
199201

200202
# Always save state before forwarding
201203
# (the dialog stack won't get updated with the skillDialog and things won't work if you don't)
202-
skill_info = self.dialog_options.skill
203204
await self.dialog_options.conversation_state.save_changes(context, True)
204205

206+
skill_info = self.dialog_options.skill
205207
response = await self.dialog_options.skill_client.post_activity(
206208
self.dialog_options.bot_id,
207209
skill_info.app_id,
@@ -229,7 +231,7 @@ async def _send_to_skill(
229231
# Capture the EndOfConversation activity if it was sent from skill
230232
eoc_activity = from_skill_activity
231233
elif await self._intercept_oauth_cards(
232-
context, from_skill_activity, connection_name
234+
context, from_skill_activity, self.dialog_options.connection_name
233235
):
234236
# do nothing. Token exchange succeeded, so no oauthcard needs to be shown to the user
235237
pass
@@ -239,6 +241,21 @@ async def _send_to_skill(
239241

240242
return eoc_activity
241243

244+
async def _create_skill_conversation_id(
245+
self, context: TurnContext, activity: Activity
246+
) -> str:
247+
# Create a conversationId to interact with the skill and send the activity
248+
conversation_id_factory_options = SkillConversationIdFactoryOptions(
249+
from_bot_oauth_scope=context.turn_state.get(BotAdapter.BOT_OAUTH_SCOPE_KEY),
250+
from_bot_id=self.dialog_options.bot_id,
251+
activity=activity,
252+
bot_framework_skill=self.dialog_options.skill,
253+
)
254+
skill_conversation_id = await self.dialog_options.conversation_id_factory.create_skill_conversation_id(
255+
conversation_id_factory_options
256+
)
257+
return skill_conversation_id
258+
242259
async def _intercept_oauth_cards(
243260
self, context: TurnContext, activity: Activity, connection_name: str
244261
):
@@ -248,6 +265,8 @@ async def _intercept_oauth_cards(
248265
if not connection_name or not isinstance(
249266
context.adapter, ExtendedUserTokenProvider
250267
):
268+
# The adapter may choose not to support token exchange, in which case we fallback to
269+
# showing an oauth card to the user.
251270
return False
252271

253272
oauth_card_attachment = next(
@@ -273,13 +292,17 @@ async def _intercept_oauth_cards(
273292
)
274293

275294
if result and result.token:
295+
# If token above is null, then SSO has failed and hence we return false.
296+
# If not, send an invoke to the skill with the token.
276297
return await self._send_token_exchange_invoke_to_skill(
277298
activity,
278299
oauth_card.token_exchange_resource.id,
279300
oauth_card.connection_name,
280301
result.token,
281302
)
282303
except:
304+
# Failures in token exchange are not fatal. They simply mean that the user needs
305+
# to be shown the OAuth card.
283306
return False
284307

285308
return False
@@ -310,4 +333,4 @@ async def _send_token_exchange_invoke_to_skill(
310333
)
311334

312335
# Check response status: true if success, false if failure
313-
return response.status / 100 == 2
336+
return response.is_successful_status_code()

libraries/botbuilder-dialogs/botbuilder/dialogs/skills/skill_dialog_options.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,12 @@ def __init__(
1818
skill: BotFrameworkSkill = None,
1919
conversation_id_factory: ConversationIdFactoryBase = None,
2020
conversation_state: ConversationState = None,
21+
connection_name: str = None,
2122
):
2223
self.bot_id = bot_id
2324
self.skill_client = skill_client
2425
self.skill_host_endpoint = skill_host_endpoint
2526
self.skill = skill
2627
self.conversation_id_factory = conversation_id_factory
2728
self.conversation_state = conversation_state
29+
self.connection_name = connection_name

0 commit comments

Comments
 (0)
0