From 43946098dc1f90877bd62f93622ca9a2ee96d888 Mon Sep 17 00:00:00 2001 From: Axel Suarez Date: Thu, 1 Oct 2020 15:53:42 -0700 Subject: [PATCH] skill dialog delete conversation id --- .../botbuilder/ai/luis/luis_recognizer.py | 4 +- .../application_insights_telemetry_client.py | 2 +- .../botbuilder/dialogs/skills/skill_dialog.py | 5 ++ .../tests/test_skill_dialog.py | 68 +++++++++++++++++-- 4 files changed, 71 insertions(+), 8 deletions(-) diff --git a/libraries/botbuilder-ai/botbuilder/ai/luis/luis_recognizer.py b/libraries/botbuilder-ai/botbuilder/ai/luis/luis_recognizer.py index 69807fca8..af1d229e5 100644 --- a/libraries/botbuilder-ai/botbuilder/ai/luis/luis_recognizer.py +++ b/libraries/botbuilder-ai/botbuilder/ai/luis/luis_recognizer.py @@ -268,9 +268,7 @@ async def _recognize_internal( options = self._options if not utterance or utterance.isspace(): - recognizer_result = RecognizerResult( - text=utterance - ) + recognizer_result = RecognizerResult(text=utterance) else: luis_recognizer = self._build_recognizer(options) diff --git a/libraries/botbuilder-applicationinsights/botbuilder/applicationinsights/application_insights_telemetry_client.py b/libraries/botbuilder-applicationinsights/botbuilder/applicationinsights/application_insights_telemetry_client.py index 9c9383cc7..7c70cc9d7 100644 --- a/libraries/botbuilder-applicationinsights/botbuilder/applicationinsights/application_insights_telemetry_client.py +++ b/libraries/botbuilder-applicationinsights/botbuilder/applicationinsights/application_insights_telemetry_client.py @@ -152,7 +152,7 @@ def track_metric( :type std_dev: float :param properties: the set of custom properties the client wants attached to this data item. (defaults to: None) :type properties: :class:`typing.Dict[str, object]` - """ + """ self._client.track_metric( name, value, tel_type, count, min_val, max_val, std_dev, properties ) diff --git a/libraries/botbuilder-dialogs/botbuilder/dialogs/skills/skill_dialog.py b/libraries/botbuilder-dialogs/botbuilder/dialogs/skills/skill_dialog.py index a2bdd7a57..62fee1ace 100644 --- a/libraries/botbuilder-dialogs/botbuilder/dialogs/skills/skill_dialog.py +++ b/libraries/botbuilder-dialogs/botbuilder/dialogs/skills/skill_dialog.py @@ -249,6 +249,11 @@ async def _send_to_skill( if from_skill_activity.type == ActivityTypes.end_of_conversation: # Capture the EndOfConversation activity if it was sent from skill eoc_activity = from_skill_activity + + # The conversation has ended, so cleanup the conversation id + await self.dialog_options.conversation_id_factory.delete_conversation_reference( + skill_conversation_id + ) elif await self._intercept_oauth_cards( context, from_skill_activity, self.dialog_options.connection_name ): diff --git a/libraries/botbuilder-dialogs/tests/test_skill_dialog.py b/libraries/botbuilder-dialogs/tests/test_skill_dialog.py index c5509e6a8..91b6dfcba 100644 --- a/libraries/botbuilder-dialogs/tests/test_skill_dialog.py +++ b/libraries/botbuilder-dialogs/tests/test_skill_dialog.py @@ -2,7 +2,7 @@ # Licensed under the MIT License. import uuid from http import HTTPStatus -from typing import Callable, Union +from typing import Callable, Union, List from unittest.mock import Mock import aiounittest @@ -46,6 +46,7 @@ class SimpleConversationIdFactory(ConversationIdFactoryBase): def __init__(self): self.conversation_refs = {} + self.create_count = 0 async def create_skill_conversation_id( self, @@ -53,6 +54,7 @@ async def create_skill_conversation_id( SkillConversationIdFactoryOptions, ConversationReference ], ) -> str: + self.create_count += 1 key = ( options_or_conversation_reference.activity.conversation.id + options_or_conversation_reference.activity.service_url @@ -72,7 +74,8 @@ async def get_conversation_reference( return self.conversation_refs[skill_conversation_id] async def delete_conversation_reference(self, skill_conversation_id: str): - raise NotImplementedError() + self.conversation_refs.pop(skill_conversation_id, None) + return class SkillDialogTests(aiounittest.AsyncTestCase): @@ -506,6 +509,57 @@ async def post_return(): self.assertIsNotNone(final_activity) self.assertEqual(len(final_activity.attachments), 1) + async def test_end_of_conversation_from_expect_replies_calls_delete_conversation_reference( + self, + ): + activity_sent: Activity = None + + # Callback to capture the parameters sent to the skill + async def capture_action( + from_bot_id: str, # pylint: disable=unused-argument + to_bot_id: str, # pylint: disable=unused-argument + to_uri: str, # pylint: disable=unused-argument + service_url: str, # pylint: disable=unused-argument + conversation_id: str, # pylint: disable=unused-argument + activity: Activity, + ): + # Capture values sent to the skill so we can assert the right parameters were used. + nonlocal activity_sent + activity_sent = activity + + eoc = Activity.create_end_of_conversation_activity() + expected_replies = list([eoc]) + + # Create a mock skill client to intercept calls and capture what is sent. + mock_skill_client = self._create_mock_skill_client( + capture_action, expected_replies=expected_replies + ) + + # Use Memory for conversation state + conversation_state = ConversationState(MemoryStorage()) + dialog_options = self.create_skill_dialog_options( + conversation_state, mock_skill_client + ) + + # Create the SkillDialogInstance and the activity to send. + sut = SkillDialog(dialog_options, dialog_id="dialog") + activity_to_send = Activity.create_message_activity() + activity_to_send.delivery_mode = DeliveryModes.expect_replies + activity_to_send.text = str(uuid.uuid4()) + client = DialogTestClient( + "test", + sut, + BeginSkillDialogOptions(activity_to_send), + conversation_state=conversation_state, + ) + + # Send something to the dialog to start it + await client.send_activity("hello") + + simple_id_factory: SimpleConversationIdFactory = dialog_options.conversation_id_factory + self.assertEqual(0, len(simple_id_factory.conversation_refs)) + self.assertEqual(1, simple_id_factory.create_count) + @staticmethod def create_skill_dialog_options( conversation_state: ConversationState, @@ -547,9 +601,15 @@ def create_oauth_card_attachment_activity(uri: str) -> Activity: return attachment_activity def _create_mock_skill_client( - self, callback: Callable, return_status: Union[Callable, int] = 200 + self, + callback: Callable, + return_status: Union[Callable, int] = 200, + expected_replies: List[Activity] = None, ) -> BotFrameworkClient: mock_client = Mock() + activity_list = ExpectedReplies( + activities=expected_replies or [MessageFactory.text("dummy activity")] + ) async def mock_post_activity( from_bot_id: str, @@ -572,7 +632,7 @@ async def mock_post_activity( if isinstance(return_status, Callable): return await return_status() - return InvokeResponse(status=return_status) + return InvokeResponse(status=return_status, body=activity_list) mock_client.post_activity.side_effect = mock_post_activity