From 2a4ddba9530b6dc0a88632192d03ee11b22e6549 Mon Sep 17 00:00:00 2001 From: Axel Suarez Date: Thu, 24 Jun 2021 18:32:25 -0700 Subject: [PATCH 1/4] DialogManager logic moved to DialogExtensions --- .../botbuilder/dialogs/dialog_extensions.py | 117 ++++++++++++++++-- .../botbuilder/dialogs/dialog_manager.py | 100 +++++---------- 2 files changed, 138 insertions(+), 79 deletions(-) diff --git a/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_extensions.py b/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_extensions.py index 027eb6904..a2e6beeca 100644 --- a/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_extensions.py +++ b/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_extensions.py @@ -1,6 +1,9 @@ # Copyright (c) Microsoft Corporation. All rights reserved. # Licensed under the MIT License. +from botbuilder.dialogs import dialog_turn_result +from botbuilder.dialogs.dialog_context import DialogContext +from botbuilder.dialogs.dialog_turn_result import DialogTurnResult from botframework.connector.auth import ( ClaimsIdentity, SkillValidation, @@ -17,6 +20,8 @@ ) from botbuilder.schema import Activity, ActivityTypes, EndOfConversationCodes +from botbuilder.dialogs.memory import DialogStateManager + class DialogExtensions: @staticmethod @@ -30,20 +35,73 @@ async def run_dialog( dialog_set = DialogSet(accessor) dialog_set.add(dialog) - dialog_context = await dialog_set.create_context(turn_context) + dialog_context: DialogContext = await dialog_set.create_context(turn_context) + + await DialogExtensions._internal_run(turn_context, dialog.id, dialog_context) + + @staticmethod + async def _internal_run( + context: TurnContext, dialog_id: str, dialog_context: DialogContext + ) -> DialogTurnResult: + # map TurnState into root dialog context.services + for key, service in context.turn_state.items(): + dialog_context.services[key] = service + + # get the DialogStateManager configuration + dialog_state_manager = DialogStateManager(dialog_context) + await dialog_state_manager.load_all_scopes() + dialog_context.context.turn_state[ + dialog_state_manager.__class__.__name__ + ] = dialog_state_manager + + turn_result: DialogTurnResult = None + + # Loop as long as we are getting valid OnError handled we should continue executing the actions for the turn. + + # NOTE: We loop around this block because each pass through we either complete the turn and break out of the + # loop or we have had an exception AND there was an OnError action which captured the error. We need to + # continue the turn based on the actions the OnError handler introduced. + end_of_turn = False + while not end_of_turn: + try: + dialog_turn_result = await DialogExtensions.__inner_run( + context, dialog_id, dialog_context + ) + + # turn successfully completed, break the loop + end_of_turn = True + except Exception as err: + # fire error event, bubbling from the leaf. + handled = await dialog_context.emit_event( + DialogEvents.error, err, bubble=True, from_leaf=True + ) + + if not handled: + # error was NOT handled, throw the exception and end the turn. (This will trigger the + # Adapter.OnError handler and end the entire dialog stack) + raise + + # save all state scopes to their respective botState locations. + await dialog_state_manager.save_all_changes() + # return the redundant result because the DialogManager contract expects it + return dialog_turn_result + + @staticmethod + async def __inner_run( + turn_context: TurnContext, dialog_id: str, dialog_context: DialogContext + ) -> DialogTurnResult: # Handle EoC and Reprompt event from a parent bot (can be root bot to skill or skill to skill) if DialogExtensions.__is_from_parent_to_skill(turn_context): # Handle remote cancellation request from parent. if turn_context.activity.type == ActivityTypes.end_of_conversation: if not dialog_context.stack: # No dialogs to cancel, just return. - return + return DialogTurnResult(DialogTurnStatus.Empty) # Send cancellation message to the dialog to ensure all the parents are canceled # in the right order. - await dialog_context.cancel_all_dialogs() - return + return await dialog_context.cancel_all_dialogs(True) # Handle a reprompt event sent from the parent. if ( @@ -52,15 +110,17 @@ async def run_dialog( ): if not dialog_context.stack: # No dialogs to reprompt, just return. - return + return DialogTurnResult(DialogTurnStatus.Empty) await dialog_context.reprompt_dialog() - return + return DialogTurnResult(DialogTurnStatus.Waiting) # Continue or start the dialog. result = await dialog_context.continue_dialog() if result.status == DialogTurnStatus.Empty: - result = await dialog_context.begin_dialog(dialog.id) + result = await dialog_context.begin_dialog(dialog_id) + + DialogExtensions._send_state_snapshot_trace(dialog_context) # Skills should send EoC when the dialog completes. if ( @@ -78,6 +138,8 @@ async def run_dialog( ) await turn_context.send_activity(activity) + return result + @staticmethod def __is_from_parent_to_skill(turn_context: TurnContext) -> bool: if turn_context.turn_state.get(SkillHandler.SKILL_CONVERSATION_REFERENCE_KEY): @@ -88,6 +150,34 @@ def __is_from_parent_to_skill(turn_context: TurnContext) -> bool: claims_identity, ClaimsIdentity ) and SkillValidation.is_skill_claim(claims_identity.claims) + @staticmethod + async def _send_state_snapshot_trace(dialog_context: DialogContext): + """ + Helper to send a trace activity with a memory snapshot of the active dialog DC. + :param dialog_context: + :return: + """ + claims_identity = dialog_context.context.turn_state.get( + BotAdapter.BOT_IDENTITY_KEY, None + ) + trace_label = ( + "Skill State" + if isinstance(claims_identity, ClaimsIdentity) + and SkillValidation.is_skill_claim(claims_identity) + else "Bot State" + ) + # send trace of memory + snapshot = DialogExtensions._get_active_dialog_context( + dialog_context + ).state.get_memory_snapshot() + trace_activity = Activity.create_trace_activity( + "BotState", + "https://www.botframework.com/schemas/botState", + snapshot, + trace_label, + ) + await dialog_context.context.send_activity(trace_activity) + @staticmethod def __send_eoc_to_parent(turn_context: TurnContext) -> bool: claims_identity = turn_context.turn_state.get(BotAdapter.BOT_IDENTITY_KEY) @@ -111,3 +201,16 @@ def __send_eoc_to_parent(turn_context: TurnContext) -> bool: return True return False + + @staticmethod + def _get_active_dialog_context(dialog_context: DialogContext) -> DialogContext: + """ + Recursively walk up the DC stack to find the active DC. + :param dialog_context: + :return: + """ + child = dialog_context.child + if not child: + return dialog_context + + return DialogExtensions._get_active_dialog_context(child) diff --git a/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_manager.py b/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_manager.py index 766519a57..39600c005 100644 --- a/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_manager.py +++ b/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_manager.py @@ -3,6 +3,7 @@ from datetime import datetime, timedelta from threading import Lock +from warnings import warn from botbuilder.core import ( BotAdapter, @@ -12,10 +13,7 @@ TurnContext, ) from botbuilder.core.skills import SkillConversationReference, SkillHandler -from botbuilder.dialogs.memory import ( - DialogStateManager, - DialogStateManagerConfiguration, -) +from botbuilder.dialogs.memory import DialogStateManagerConfiguration from botbuilder.schema import Activity, ActivityTypes, EndOfConversationCodes from botframework.connector.auth import ( AuthenticationConstants, @@ -27,6 +25,7 @@ from .dialog import Dialog from .dialog_context import DialogContext from .dialog_events import DialogEvents +from .dialog_extensions import DialogExtensions from .dialog_set import DialogSet from .dialog_state import DialogState from .dialog_manager_result import DialogManagerResult @@ -142,60 +141,10 @@ async def on_turn(self, context: TurnContext) -> DialogManagerResult: # Create DialogContext dialog_context = DialogContext(self.dialogs, context, dialog_state) - # promote initial TurnState into dialog_context.services for contextual services - for key, service in dialog_context.services.items(): - dialog_context.services[key] = service - - # map TurnState into root dialog context.services - for key, service in context.turn_state.items(): - dialog_context.services[key] = service - - # get the DialogStateManager configuration - dialog_state_manager = DialogStateManager( - dialog_context, self.state_configuration + # Call the common dialog "continue/begin" execution pattern shared with the classic RunAsync extension method + turn_result = await DialogExtensions._internal_run( # pylint: disable=protected-access + context, self._root_dialog_id, dialog_context ) - await dialog_state_manager.load_all_scopes() - dialog_context.context.turn_state[ - dialog_state_manager.__class__.__name__ - ] = dialog_state_manager - - turn_result: DialogTurnResult = None - - # Loop as long as we are getting valid OnError handled we should continue executing the actions for the turn. - - # NOTE: We loop around this block because each pass through we either complete the turn and break out of the - # loop or we have had an exception AND there was an OnError action which captured the error. We need to - # continue the turn based on the actions the OnError handler introduced. - end_of_turn = False - while not end_of_turn: - try: - claims_identity: ClaimsIdentity = context.turn_state.get( - BotAdapter.BOT_IDENTITY_KEY, None - ) - if isinstance( - claims_identity, ClaimsIdentity - ) and SkillValidation.is_skill_claim(claims_identity.claims): - # The bot is running as a skill. - turn_result = await self.handle_skill_on_turn(dialog_context) - else: - # The bot is running as root bot. - turn_result = await self.handle_bot_on_turn(dialog_context) - - # turn successfully completed, break the loop - end_of_turn = True - except Exception as err: - # fire error event, bubbling from the leaf. - handled = await dialog_context.emit_event( - DialogEvents.error, err, bubble=True, from_leaf=True - ) - - if not handled: - # error was NOT handled, throw the exception and end the turn. (This will trigger the - # Adapter.OnError handler and end the entire dialog stack) - raise - - # save all state scopes to their respective botState locations. - await dialog_state_manager.save_all_changes() # save BotState changes await bot_state_set.save_all_changes(dialog_context.context, False) @@ -204,7 +153,8 @@ async def on_turn(self, context: TurnContext) -> DialogManagerResult: @staticmethod async def send_state_snapshot_trace( - dialog_context: DialogContext, trace_label: str + dialog_context: DialogContext, + trace_label: str = None, # pylint: disable=unused-argument ): """ Helper to send a trace activity with a memory snapshot of the active dialog DC. @@ -212,17 +162,13 @@ async def send_state_snapshot_trace( :param trace_label: :return: """ - # send trace of memory - snapshot = DialogManager.get_active_dialog_context( + warn( + "This method will be deprecated as no longer is necesary", + PendingDeprecationWarning, + ) + DialogExtensions._send_state_snapshot_trace( # pylint: disable=protected-access dialog_context - ).state.get_memory_snapshot() - trace_activity = Activity.create_trace_activity( - "BotState", - "https://www.botframework.com/schemas/botState", - snapshot, - trace_label, ) - await dialog_context.context.send_activity(trace_activity) @staticmethod def is_from_parent_to_skill(turn_context: TurnContext) -> bool: @@ -246,11 +192,13 @@ def get_active_dialog_context(dialog_context: DialogContext) -> DialogContext: :param dialog_context: :return: """ - child = dialog_context.child - if not child: - return dialog_context - - return DialogManager.get_active_dialog_context(child) + warn( + "This method will be deprecated as no longer is necesary", + PendingDeprecationWarning, + ) + return DialogExtensions._get_active_dialog_context( # pylint: disable=protected-access + dialog_context + ) @staticmethod def should_send_end_of_conversation_to_parent( @@ -294,6 +242,10 @@ def should_send_end_of_conversation_to_parent( async def handle_skill_on_turn( self, dialog_context: DialogContext ) -> DialogTurnResult: + warn( + "This method will be deprecated as no longer is necesary", + PendingDeprecationWarning, + ) # the bot is running as a skill. turn_context = dialog_context.context @@ -348,6 +300,10 @@ async def handle_skill_on_turn( async def handle_bot_on_turn( self, dialog_context: DialogContext ) -> DialogTurnResult: + warn( + "This method will be deprecated as no longer is necesary", + PendingDeprecationWarning, + ) # the bot is running as a root bot. if dialog_context.active_dialog is None: # start root dialog From 4dbf7b0c799e2bf17db6f59aea0d2dc5a3b04eba Mon Sep 17 00:00:00 2001 From: Axel Suarez Date: Thu, 24 Jun 2021 19:10:55 -0700 Subject: [PATCH 2/4] missing await --- .../botbuilder/dialogs/dialog_extensions.py | 10 ++++++---- .../botbuilder/dialogs/dialog_manager.py | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_extensions.py b/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_extensions.py index a2e6beeca..496577ff7 100644 --- a/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_extensions.py +++ b/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_extensions.py @@ -1,7 +1,8 @@ # Copyright (c) Microsoft Corporation. All rights reserved. # Licensed under the MIT License. -from botbuilder.dialogs import dialog_turn_result +import botbuilder.dialogs as dialogs + from botbuilder.dialogs.dialog_context import DialogContext from botbuilder.dialogs.dialog_turn_result import DialogTurnResult from botframework.connector.auth import ( @@ -13,7 +14,6 @@ from botbuilder.core import BotAdapter, StatePropertyAccessor, TurnContext from botbuilder.core.skills import SkillHandler, SkillConversationReference from botbuilder.dialogs import ( - Dialog, DialogEvents, DialogSet, DialogTurnStatus, @@ -26,7 +26,9 @@ class DialogExtensions: @staticmethod async def run_dialog( - dialog: Dialog, turn_context: TurnContext, accessor: StatePropertyAccessor + dialog: "dialogs.Dialog", + turn_context: TurnContext, + accessor: StatePropertyAccessor, ): """ Creates a dialog stack and starts a dialog, pushing it onto the stack. @@ -120,7 +122,7 @@ async def __inner_run( if result.status == DialogTurnStatus.Empty: result = await dialog_context.begin_dialog(dialog_id) - DialogExtensions._send_state_snapshot_trace(dialog_context) + await DialogExtensions._send_state_snapshot_trace(dialog_context) # Skills should send EoC when the dialog completes. if ( diff --git a/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_manager.py b/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_manager.py index 39600c005..f0ab06055 100644 --- a/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_manager.py +++ b/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_manager.py @@ -166,7 +166,7 @@ async def send_state_snapshot_trace( "This method will be deprecated as no longer is necesary", PendingDeprecationWarning, ) - DialogExtensions._send_state_snapshot_trace( # pylint: disable=protected-access + await DialogExtensions._send_state_snapshot_trace( # pylint: disable=protected-access dialog_context ) From 0bb91c3beb9e9a2f56f3acfdfd2582c8ba6c2a8e Mon Sep 17 00:00:00 2001 From: Axel Suarez Date: Thu, 24 Jun 2021 19:42:28 -0700 Subject: [PATCH 3/4] fixing claims validation call --- .../botbuilder-dialogs/botbuilder/dialogs/dialog_extensions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_extensions.py b/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_extensions.py index 496577ff7..23375f525 100644 --- a/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_extensions.py +++ b/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_extensions.py @@ -165,7 +165,7 @@ async def _send_state_snapshot_trace(dialog_context: DialogContext): trace_label = ( "Skill State" if isinstance(claims_identity, ClaimsIdentity) - and SkillValidation.is_skill_claim(claims_identity) + and SkillValidation.is_skill_claim(claims_identity.claims) else "Bot State" ) # send trace of memory From 0778d649777c2c1b66cc6b6356e798314f865470 Mon Sep 17 00:00:00 2001 From: Axel Suarez Date: Thu, 24 Jun 2021 19:54:00 -0700 Subject: [PATCH 4/4] pylint --- .../botbuilder/dialogs/dialog_extensions.py | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_extensions.py b/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_extensions.py index 23375f525..be2055ffa 100644 --- a/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_extensions.py +++ b/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_extensions.py @@ -1,10 +1,6 @@ # Copyright (c) Microsoft Corporation. All rights reserved. # Licensed under the MIT License. -import botbuilder.dialogs as dialogs - -from botbuilder.dialogs.dialog_context import DialogContext -from botbuilder.dialogs.dialog_turn_result import DialogTurnResult from botframework.connector.auth import ( ClaimsIdentity, SkillValidation, @@ -13,6 +9,10 @@ ) from botbuilder.core import BotAdapter, StatePropertyAccessor, TurnContext from botbuilder.core.skills import SkillHandler, SkillConversationReference +import botbuilder.dialogs as dialogs # pylint: disable=unused-import +from botbuilder.dialogs.memory import DialogStateManager +from botbuilder.dialogs.dialog_context import DialogContext +from botbuilder.dialogs.dialog_turn_result import DialogTurnResult from botbuilder.dialogs import ( DialogEvents, DialogSet, @@ -20,8 +20,6 @@ ) from botbuilder.schema import Activity, ActivityTypes, EndOfConversationCodes -from botbuilder.dialogs.memory import DialogStateManager - class DialogExtensions: @staticmethod @@ -56,8 +54,6 @@ async def _internal_run( dialog_state_manager.__class__.__name__ ] = dialog_state_manager - turn_result: DialogTurnResult = None - # Loop as long as we are getting valid OnError handled we should continue executing the actions for the turn. # NOTE: We loop around this block because each pass through we either complete the turn and break out of the