From ed223a21cce4dc0ec7a4466108d8ef30567c3180 Mon Sep 17 00:00:00 2001 From: tracyboehrer Date: Fri, 12 Feb 2021 13:05:36 -0600 Subject: [PATCH 1/4] DialogContainer.telemetryclient --- .../botbuilder/dialogs/dialog_container.py | 26 +++++++++++++++++++ .../botbuilder/dialogs/dialog_set.py | 24 +++++++++++++++-- 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_container.py b/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_container.py index ad2326419..549ee08bc 100644 --- a/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_container.py +++ b/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_container.py @@ -4,6 +4,7 @@ from abc import ABC, abstractmethod +from botbuilder.core import NullTelemetryClient, BotTelemetryClient from .dialog import Dialog from .dialog_context import DialogContext from .dialog_event import DialogEvent @@ -17,6 +18,31 @@ def __init__(self, dialog_id: str = None): self.dialogs = DialogSet() + @property + def telemetry_client(self) -> BotTelemetryClient: + """ + Gets the telemetry client for logging events. + """ + return super._telemetry_client + + @telemetry_client.setter + def telemetry_client(self, value: BotTelemetryClient) -> None: + """ + Sets the telemetry client for all dialogs in this set. + """ + if value is None: + super._telemetry_client = NullTelemetryClient() + else: + super._telemetry_client = value + + # Care! Dialogs.TelemetryClient assignment internally assigns the + # TelemetryClient for each dialog which could lead to an eventual stack + # overflow in cyclical dialog structures. + # Don't set the telemetry client if the candidate instance is the same as + # the currently set one. + if self.dialogs.telemetry_client != value: + self.dialogs.telemetry_client = super._telemetry_client + @abstractmethod def create_child_context(self, dialog_context: DialogContext) -> DialogContext: raise NotImplementedError() diff --git a/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_set.py b/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_set.py index 5820a3422..205e78ab6 100644 --- a/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_set.py +++ b/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_set.py @@ -4,7 +4,7 @@ from hashlib import sha256 from typing import Dict -from botbuilder.core import TurnContext, BotAssert, StatePropertyAccessor +from botbuilder.core import NullTelemetryClient, BotTelemetryClient, TurnContext, BotAssert, StatePropertyAccessor from .dialog import Dialog from .dialog_state import DialogState @@ -34,11 +34,31 @@ def __init__(self, dialog_state: StatePropertyAccessor = None): del frame self._dialog_state = dialog_state - # self.__telemetry_client = NullBotTelemetryClient.Instance; + self.__telemetry_client = NullTelemetryClient() self._dialogs: Dict[str, Dialog] = {} self._version: str = None + @property + def telemetry_client(self) -> BotTelemetryClient: + """ + Gets the telemetry client for logging events. + """ + return self.__telemetry_client + + @telemetry_client.setter + def telemetry_client(self, value: BotTelemetryClient) -> None: + """ + Sets the telemetry client for all dialogs in this set. + """ + if value is None: + self.__telemetry_client = NullTelemetryClient() + else: + self.__telemetry_client = value + + for dialog in self._dialogs.values(): + dialog.telemetry_client = self.__telemetry_client + def get_version(self) -> str: """ Gets a unique string which represents the combined versions of all dialogs in this this dialogset. From 27c7536c0c5979c70dd5d6e990c31f8a14d29c79 Mon Sep 17 00:00:00 2001 From: Axel Suarez Date: Fri, 12 Feb 2021 12:11:57 -0800 Subject: [PATCH 2/4] Black formatting and self on DialogContainer --- .../botbuilder/dialogs/dialog_container.py | 8 ++++---- .../botbuilder-dialogs/botbuilder/dialogs/dialog_set.py | 8 +++++++- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_container.py b/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_container.py index 549ee08bc..a0e2f04e8 100644 --- a/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_container.py +++ b/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_container.py @@ -23,7 +23,7 @@ def telemetry_client(self) -> BotTelemetryClient: """ Gets the telemetry client for logging events. """ - return super._telemetry_client + return self._telemetry_client @telemetry_client.setter def telemetry_client(self, value: BotTelemetryClient) -> None: @@ -31,9 +31,9 @@ def telemetry_client(self, value: BotTelemetryClient) -> None: Sets the telemetry client for all dialogs in this set. """ if value is None: - super._telemetry_client = NullTelemetryClient() + self._telemetry_client = NullTelemetryClient() else: - super._telemetry_client = value + self._telemetry_client = value # Care! Dialogs.TelemetryClient assignment internally assigns the # TelemetryClient for each dialog which could lead to an eventual stack @@ -41,7 +41,7 @@ def telemetry_client(self, value: BotTelemetryClient) -> None: # Don't set the telemetry client if the candidate instance is the same as # the currently set one. if self.dialogs.telemetry_client != value: - self.dialogs.telemetry_client = super._telemetry_client + self.dialogs.telemetry_client = self._telemetry_client @abstractmethod def create_child_context(self, dialog_context: DialogContext) -> DialogContext: diff --git a/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_set.py b/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_set.py index 205e78ab6..ce2070cae 100644 --- a/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_set.py +++ b/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_set.py @@ -4,7 +4,13 @@ from hashlib import sha256 from typing import Dict -from botbuilder.core import NullTelemetryClient, BotTelemetryClient, TurnContext, BotAssert, StatePropertyAccessor +from botbuilder.core import ( + NullTelemetryClient, + BotTelemetryClient, + TurnContext, + BotAssert, + StatePropertyAccessor, +) from .dialog import Dialog from .dialog_state import DialogState From 8e7f57176aa25019694ed9ea708f856c67dfee6d Mon Sep 17 00:00:00 2001 From: Axel Suarez Date: Fri, 12 Feb 2021 20:24:36 -0800 Subject: [PATCH 3/4] Adding tests. Testing for method add() are pending due to update breaking fragile tests. --- .../tests/test_dialog_set.py | 93 ++++++++++++++++++- 1 file changed, 91 insertions(+), 2 deletions(-) diff --git a/libraries/botbuilder-dialogs/tests/test_dialog_set.py b/libraries/botbuilder-dialogs/tests/test_dialog_set.py index a58d12da0..20ef22d73 100644 --- a/libraries/botbuilder-dialogs/tests/test_dialog_set.py +++ b/libraries/botbuilder-dialogs/tests/test_dialog_set.py @@ -2,8 +2,15 @@ # Licensed under the MIT License. import aiounittest -from botbuilder.dialogs import DialogSet, ComponentDialog -from botbuilder.core import ConversationState, MemoryStorage +from botbuilder.dialogs import DialogSet, ComponentDialog, WaterfallDialog +from botbuilder.core import ConversationState, MemoryStorage, NullTelemetryClient + + +class MyBotTelemetryClient(NullTelemetryClient): + def __init__(self): + # pylint: disable=useless-return + super().__init__() + return class DialogSetTests(aiounittest.AsyncTestCase): @@ -18,3 +25,85 @@ def test_dialogset_constructor_null_property(self): def test_dialogset_constructor_null_from_componentdialog(self): ComponentDialog("MyId") + + def test_dialogset_telemetryset(self): + convo_state = ConversationState(MemoryStorage()) + dialog_state_property = convo_state.create_property("dialogstate") + dialog_set = DialogSet(dialog_state_property) + + dialog_set.add(WaterfallDialog("A")) + dialog_set.add(WaterfallDialog("B")) + + self.assertTrue( + isinstance( + dialog_set.find_dialog("A").telemetry_client, NullTelemetryClient + ) + ) + self.assertTrue( + isinstance( + dialog_set.find_dialog("B").telemetry_client, NullTelemetryClient + ) + ) + + dialog_set.telemetry_client = MyBotTelemetryClient() + + self.assertTrue( + isinstance( + dialog_set.find_dialog("A").telemetry_client, MyBotTelemetryClient + ) + ) + self.assertTrue( + isinstance( + dialog_set.find_dialog("B").telemetry_client, MyBotTelemetryClient + ) + ) + + def test_dialogset_nulltelemetryset(self): + convo_state = ConversationState(MemoryStorage()) + dialog_state_property = convo_state.create_property("dialogstate") + dialog_set = DialogSet(dialog_state_property) + + dialog_set.add(WaterfallDialog("A")) + dialog_set.add(WaterfallDialog("B")) + + dialog_set.telemetry_client = MyBotTelemetryClient() + dialog_set.telemetry_client = None + + self.assertFalse( + isinstance( + dialog_set.find_dialog("A").telemetry_client, MyBotTelemetryClient + ) + ) + self.assertFalse( + isinstance( + dialog_set.find_dialog("B").telemetry_client, MyBotTelemetryClient + ) + ) + self.assertTrue( + isinstance( + dialog_set.find_dialog("A").telemetry_client, NullTelemetryClient + ) + ) + self.assertTrue( + isinstance( + dialog_set.find_dialog("B").telemetry_client, NullTelemetryClient + ) + ) + + # pylint: disable=pointless-string-statement + """ + This test will be enabled when telematry tests are fixed for DialogSet telemetry + def test_dialogset_addtelemetryset(self): + convo_state = ConversationState(MemoryStorage()) + dialog_state_property = convo_state.create_property("dialogstate") + dialog_set = DialogSet(dialog_state_property) + + dialog_set.add(WaterfallDialog("A")) + dialog_set.add(WaterfallDialog("B")) + + dialog_set.telemetry_client = MyBotTelemetryClient() + + dialog_set.add(WaterfallDialog("C")) + + self.assertTrue(isinstance(dialog_set.find_dialog("C").telemetry_client, MyBotTelemetryClient)) + """ From d5cb637ecd764abf8aea9360d97a7757628374cc Mon Sep 17 00:00:00 2001 From: Axel Suarez Date: Tue, 16 Feb 2021 12:41:18 -0800 Subject: [PATCH 4/4] Pylint fix --- libraries/botbuilder-dialogs/tests/test_dialog_set.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/botbuilder-dialogs/tests/test_dialog_set.py b/libraries/botbuilder-dialogs/tests/test_dialog_set.py index 20ef22d73..993ed207a 100644 --- a/libraries/botbuilder-dialogs/tests/test_dialog_set.py +++ b/libraries/botbuilder-dialogs/tests/test_dialog_set.py @@ -7,8 +7,8 @@ class MyBotTelemetryClient(NullTelemetryClient): + # pylint: disable=useless-return def __init__(self): - # pylint: disable=useless-return super().__init__() return