From 56299f4f6df5496f51bbca7147aef77a9025f72d Mon Sep 17 00:00:00 2001 From: alexrecuenco <26118630+alexrecuenco@users.noreply.github.com> Date: Sun, 23 Mar 2025 02:01:43 +0100 Subject: [PATCH] feat(dialogs): Incorporate DialogDependencies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dialog dependencies is a feature in - [botbuilder-js](https://github.com/microsoft/botbuilder-js/blob/87d0ad1a889a38396e761f10614c78a4526590d1/libraries/botbuilder-dialogs/src/dialogSet.ts#L169), - and in [botbuilder-dotnet](https://github.com/microsoft/botbuilder-dotnet/blob/bc17a5db2c717fec0d4c109d884bea88d29f75cb/libraries/Microsoft.Bot.Builder.Dialogs/DialogSet.cs#L147). To maintain feature parity, and because this functionality is extremely useful this PR brings it to `botbuilder-python`. Tecnically in the `botbuilder-js` and dotnet implementations dialogs that are found to conflict get stored with a different ID, but I am sure that would likely be a breaking change (and IMO a very counter-intuitive way of working). If you are adding dialog dependencies and they conflict, the logic as it was would be retained. Note that, although by making it a `@runtime_checkable` Protocol with DialogDependencies one can check `isinstance(object, DialogDependencies)`, the recommendation from the python docs is: "check against a runtime-checkable protocol can be surprisingly slow" — [python typing](https://docs.python.org/3/library/typing.html#typing.Protocol) For that reason we use `hasattr` and `callable` instead. Also, note that protocol is avaialble from python 3.8 onwards, which is already the minimum version for this library. The purpose of using Iterable instead of List is just to keep it generic. Since python has the concepts of `tuple`, `List`, etc and to keep it as a structural type. --- .../botbuilder/dialogs/__init__.py | 2 + .../botbuilder/dialogs/dialog_dependencies.py | 16 ++++ .../botbuilder/dialogs/dialog_set.py | 12 ++- .../tests/test_dialog_set.py | 74 ++++++++++++++++++- 4 files changed, 102 insertions(+), 2 deletions(-) create mode 100644 libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_dependencies.py diff --git a/libraries/botbuilder-dialogs/botbuilder/dialogs/__init__.py b/libraries/botbuilder-dialogs/botbuilder/dialogs/__init__.py index 37c305536..13fa92829 100644 --- a/libraries/botbuilder-dialogs/botbuilder/dialogs/__init__.py +++ b/libraries/botbuilder-dialogs/botbuilder/dialogs/__init__.py @@ -9,6 +9,7 @@ from .component_dialog import ComponentDialog from .dialog_container import DialogContainer from .dialog_context import DialogContext +from .dialog_dependencies import DialogDependencies from .dialog_event import DialogEvent from .dialog_events import DialogEvents from .dialog_instance import DialogInstance @@ -35,6 +36,7 @@ "ComponentDialog", "DialogContainer", "DialogContext", + "DialogDependencies", "DialogEvent", "DialogEvents", "DialogInstance", diff --git a/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_dependencies.py b/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_dependencies.py new file mode 100644 index 000000000..292ea4d33 --- /dev/null +++ b/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_dependencies.py @@ -0,0 +1,16 @@ +from typing import Iterable, Protocol, runtime_checkable + +from .dialog import Dialog + + +@runtime_checkable +class DialogDependencies(Protocol): + """Protocol for dialogs that have dependencies on other dialogs. + + If implemented, when the dialog is added to a DialogSet all of its dependencies will be added as well. + """ + + def get_dependencies(self) -> Iterable[Dialog]: + """Returns an iterable of the dialogs that this dialog depends on. + + :return: The dialog dependencies.""" diff --git a/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_set.py b/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_set.py index ce2070cae..0ebcfb5e3 100644 --- a/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_set.py +++ b/libraries/botbuilder-dialogs/botbuilder/dialogs/dialog_set.py @@ -2,7 +2,7 @@ # Licensed under the MIT License. import inspect from hashlib import sha256 -from typing import Dict +from typing import Dict, TYPE_CHECKING from botbuilder.core import ( NullTelemetryClient, @@ -14,6 +14,9 @@ from .dialog import Dialog from .dialog_state import DialogState +if TYPE_CHECKING: + from .dialog_context import DialogContext + class DialogSet: def __init__(self, dialog_state: StatePropertyAccessor = None): @@ -92,6 +95,8 @@ def add(self, dialog: Dialog): ) if dialog.id in self._dialogs: + if self._dialogs[dialog.id] == dialog: + return self raise TypeError( "DialogSet.add(): A dialog with an id of '%s' already added." % dialog.id @@ -100,6 +105,11 @@ def add(self, dialog: Dialog): # dialog.telemetry_client = this._telemetry_client; self._dialogs[dialog.id] = dialog + # Automatically add any child dependencies the dialog might have, see DialogDependencies. + if hasattr(dialog, "get_dependencies") and callable(dialog.get_dependencies): + for child in dialog.get_dependencies(): + self.add(child) + return self async def create_context(self, turn_context: TurnContext) -> "DialogContext": diff --git a/libraries/botbuilder-dialogs/tests/test_dialog_set.py b/libraries/botbuilder-dialogs/tests/test_dialog_set.py index 993ed207a..66596c80d 100644 --- a/libraries/botbuilder-dialogs/tests/test_dialog_set.py +++ b/libraries/botbuilder-dialogs/tests/test_dialog_set.py @@ -2,7 +2,12 @@ # Licensed under the MIT License. import aiounittest -from botbuilder.dialogs import DialogSet, ComponentDialog, WaterfallDialog +from botbuilder.dialogs import ( + DialogDependencies, + DialogSet, + ComponentDialog, + WaterfallDialog, +) from botbuilder.core import ConversationState, MemoryStorage, NullTelemetryClient @@ -90,6 +95,73 @@ def test_dialogset_nulltelemetryset(self): ) ) + def test_dialogset_raises_on_repeated_id(self): + convo_state = ConversationState(MemoryStorage()) + dialog_state_property = convo_state.create_property("dialogstate") + dialog_set = DialogSet(dialog_state_property) + + dialog_set.add(WaterfallDialog("A")) + with self.assertRaises(TypeError): + dialog_set.add(WaterfallDialog("A")) + + self.assertTrue(dialog_set.find_dialog("A") is not None) + + def test_dialogset_idempotenticy_add(self): + convo_state = ConversationState(MemoryStorage()) + dialog_state_property = convo_state.create_property("dialogstate") + dialog_set = DialogSet(dialog_state_property) + dialog_a = WaterfallDialog("A") + dialog_set.add(dialog_a) + dialog_set.add(dialog_a) + + async def test_dialogset_dependency_tree_add(self): + class MyDialog(WaterfallDialog, DialogDependencies): + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self._dependencies = [] + + def add_dependency(self, dialog): + self._dependencies.append(dialog) + + def get_dependencies(self): + return self._dependencies + + convo_state = ConversationState(MemoryStorage()) + dialog_state_property = convo_state.create_property("dialogstate") + dialog_set = DialogSet(dialog_state_property) + + dialog_a = MyDialog("A") + dialog_b = MyDialog("B") + dialog_c = MyDialog("C") + dialog_d = MyDialog("D") + dialog_e = MyDialog("E") + dialog_i = MyDialog("I") + + dialog_a.add_dependency(dialog_b) + + # Multi-hierarchy should be OK + dialog_b.add_dependency(dialog_d) + dialog_b.add_dependency(dialog_e) + + # circular dependencies should be OK + dialog_c.add_dependency(dialog_d) + dialog_d.add_dependency(dialog_c) + + assert dialog_set.find_dialog(dialog_a.id) is None + dialog_set.add(dialog_a) + + for dialog in [ + dialog_a, + dialog_b, + dialog_c, + dialog_d, + dialog_e, + ]: + self.assertTrue(dialog_set.find_dialog(dialog.id) is dialog) + self.assertTrue(await dialog_set.find(dialog.id) is dialog) + + assert dialog_set.find_dialog(dialog_i.id) is None + # pylint: disable=pointless-string-statement """ This test will be enabled when telematry tests are fixed for DialogSet telemetry