[microsoft#1687][PORT][SkillConversationIdFactory] Add SkillConversationIdFactory to Python (microsoft#1691)

* Add SkillConversationIdFactory class * Update get_skill_conversation_reference to support deprecated method * Add unit test for SkillConversationIdFactory and update skill dialog test * Fix SkillConversationIdFactory behavior and complete unit test * Update tests * Apply black and pylint * Add copyright licence Co-authored-by: Giuliano Taliano <54330145+GiulianoDolceTalianoSantoro@users.noreply.github.com> Co-authored-by: sw-joelmut <joel.mut@southworks.com>

Author: 3 people

libraries/botbuilder-core/botbuilder/core/skills/__init__.py

@@ -8,6 +8,7 @@
 from .bot_framework_skill import BotFrameworkSkill
 from .bot_framework_client import BotFrameworkClient
 from .conversation_id_factory import ConversationIdFactoryBase
+from .skill_conversation_id_factory import SkillConversationIdFactory
 from .skill_handler import SkillHandler
 from .skill_conversation_id_factory_options import SkillConversationIdFactoryOptions
 from .skill_conversation_reference import SkillConversationReference
@@ -16,6 +17,7 @@
  "BotFrameworkSkill",
  "BotFrameworkClient",
  "ConversationIdFactoryBase",
+ "SkillConversationIdFactory",
  "SkillConversationIdFactoryOptions",
  "SkillConversationReference",
  "SkillHandler",

libraries/botbuilder-core/botbuilder/core/skills/conversation_id_factory.py

@@ -1,7 +1,7 @@
 # Copyright (c) Microsoft Corporation. All rights reserved.
 # Licensed under the MIT License.
 
-from abc import ABC, abstractmethod
+from abc import ABC
 from typing import Union
 from botbuilder.schema import ConversationReference
 from .skill_conversation_id_factory_options import SkillConversationIdFactoryOptions
@@ -17,7 +17,6 @@ class ConversationIdFactoryBase(ABC):
  SkillConversationReferences and deletion.
  """
 
- @abstractmethod
  async def create_skill_conversation_id(
  self,
  options_or_conversation_reference: Union[
@@ -41,23 +40,32 @@ async def create_skill_conversation_id(
  """
  raise NotImplementedError()
 
- @abstractmethod
  async def get_conversation_reference(
  self, skill_conversation_id: str
- ) -> Union[SkillConversationReference, ConversationReference]:
+ ) -> ConversationReference:
+ """
+ [DEPRECATED] Method is deprecated, please use get_skill_conversation_reference() instead.
+
+ Retrieves a :class:`ConversationReference` using a conversation id passed in.
+
+ :param skill_conversation_id: The conversation id for which to retrieve the :class:`ConversationReference`.
+ :type skill_conversation_id: str
+ :returns: `ConversationReference` for the specified ID.
+ """
+ raise NotImplementedError()
+
+ async def get_skill_conversation_reference(
+ self, skill_conversation_id: str
+ ) -> SkillConversationReference:
  """
  Retrieves a :class:`SkillConversationReference` using a conversation id passed in.
 
  :param skill_conversation_id: The conversation id for which to retrieve the :class:`SkillConversationReference`.
  :type skill_conversation_id: str
-
- .. note::
- SkillConversationReference is the preferred return type, while the :class:`SkillConversationReference`
- type is provided for backwards compatability.
+ :returns: `SkillConversationReference` for the specified ID.
  """
  raise NotImplementedError()
 
- @abstractmethod
  async def delete_conversation_reference(self, skill_conversation_id: str):
  """
  Removes any reference to objects keyed on the conversation id passed in.

libraries/botbuilder-core/botbuilder/core/skills/skill_conversation_id_factory.py

@@ -0,0 +1,86 @@
+# Copyright (c) Microsoft Corporation. All rights reserved.
+# Licensed under the MIT License.
+
+from uuid import uuid4 as uuid
+from botbuilder.core import TurnContext, Storage
+from .conversation_id_factory import ConversationIdFactoryBase
+from .skill_conversation_id_factory_options import SkillConversationIdFactoryOptions
+from .skill_conversation_reference import SkillConversationReference
+from .skill_conversation_reference import ConversationReference
+
+
+class SkillConversationIdFactory(ConversationIdFactoryBase):
+ def __init__(self, storage: Storage):
+ if not storage:
+ raise TypeError("storage can't be None")
+
+ self._storage = storage
+
+ async def create_skill_conversation_id( # pylint: disable=arguments-differ
+ self, options: SkillConversationIdFactoryOptions
+ ) -> str:
+ """
+ Creates a new `SkillConversationReference`.
+
+ :param options: Creation options to use when creating the `SkillConversationReference`.
+ :type options: :class:`botbuilder.core.skills.SkillConversationIdFactoryOptions`
+ :return: ID of the created `SkillConversationReference`.
+ """
+
+ if not options:
+ raise TypeError("options can't be None")
+
+ conversation_reference = TurnContext.get_conversation_reference(
+ options.activity
+ )
+
+ skill_conversation_id = str(uuid())
+
+ # Create the SkillConversationReference instance.
+ skill_conversation_reference = SkillConversationReference(
+ conversation_reference=conversation_reference,
+ oauth_scope=options.from_bot_oauth_scope,
+ )
+
+ # Store the SkillConversationReference using the skill_conversation_id as a key.
+ skill_conversation_info = {skill_conversation_id: skill_conversation_reference}
+
+ await self._storage.write(skill_conversation_info)
+
+ # Return the generated skill_conversation_id (that will be also used as the conversation ID to call the skill).
+ return skill_conversation_id
+
+ async def get_conversation_reference(
+ self, skill_conversation_id: str
+ ) -> ConversationReference:
+ return await super().get_conversation_reference(skill_conversation_id)
+
+ async def get_skill_conversation_reference(
+ self, skill_conversation_id: str
+ ) -> SkillConversationReference:
+ """
+ Retrieve a `SkillConversationReference` with the specified ID.
+
+ :param skill_conversation_id: The ID of the `SkillConversationReference` to retrieve.
+ :type skill_conversation_id: str
+ :return: `SkillConversationReference` for the specified ID; None if not found.
+ """
+
+ if not skill_conversation_id:
+ raise TypeError("skill_conversation_id can't be None")
+
+ # Get the SkillConversationReference from storage for the given skill_conversation_id.
+ skill_conversation_reference = await self._storage.read([skill_conversation_id])
+
+ return skill_conversation_reference.get(skill_conversation_id)
+
+ async def delete_conversation_reference(self, skill_conversation_id: str):
+ """
+ Deletes the `SkillConversationReference` with the specified ID.
+
+ :param skill_conversation_id: The ID of the `SkillConversationReference` to be deleted.
+ :type skill_conversation_id: str
+ """
+
+ # Delete the SkillConversationReference from storage.
+ await self._storage.delete([skill_conversation_id])

libraries/botbuilder-core/botbuilder/core/skills/skill_handler.py

@@ -2,6 +2,7 @@
 # Licensed under the MIT License.
 
 from uuid import uuid4
+from logging import Logger, getLogger
 
 from botbuilder.core import Bot, BotAdapter, ChannelServiceHandler, TurnContext
 from botbuilder.schema import (
@@ -37,7 +38,7 @@ def __init__(
  credential_provider: CredentialProvider,
  auth_configuration: AuthenticationConfiguration,
  channel_provider: ChannelProvider = None,
- logger: object = None,
+ logger: Logger = None,
  ):
  super().__init__(credential_provider, auth_configuration, channel_provider)
 
@@ -51,7 +52,7 @@ def __init__(
  self._adapter = adapter
  self._bot = bot
  self._conversation_id_factory = conversation_id_factory
- self._logger = logger
+ self._logger = logger or getLogger()
 
  async def on_send_to_conversation(
  self, claims_identity: ClaimsIdentity, conversation_id: str, activity: Activity,
@@ -181,20 +182,26 @@ async def callback(turn_context: TurnContext):
  async def _get_skill_conversation_reference(
  self, conversation_id: str
  ) -> SkillConversationReference:
- # Get the SkillsConversationReference
- conversation_reference_result = await self._conversation_id_factory.get_conversation_reference(
- conversation_id
- )
+ try:
+ skill_conversation_reference = await self._conversation_id_factory.get_skill_conversation_reference(
+ conversation_id
+ )
+ except NotImplementedError:
+ self._logger.warning(
+ "Got NotImplementedError when trying to call get_skill_conversation_reference() "
+ "on the SkillConversationIdFactory, attempting to use deprecated "
+ "get_conversation_reference() method instead."
+ )
+
+ # Attempt to get SkillConversationReference using deprecated method.
+ # this catch should be removed once we remove the deprecated method.
+ # We need to use the deprecated method for backward compatibility.
+ conversation_reference = await self._conversation_id_factory.get_conversation_reference(
+ conversation_id
+ )
 
- # ConversationIdFactory can return either a SkillConversationReference (the newer way),
- # or a ConversationReference (the old way, but still here for compatibility). If a
- # ConversationReference is returned, build a new SkillConversationReference to simplify
- # the remainder of this method.
- if isinstance(conversation_reference_result, SkillConversationReference):
- skill_conversation_reference: SkillConversationReference = conversation_reference_result
- else:
  skill_conversation_reference: SkillConversationReference = SkillConversationReference(
- conversation_reference=conversation_reference_result,
+ conversation_reference=conversation_reference,
  oauth_scope=(
  GovernmentConstants.TO_CHANNEL_FROM_BOT_OAUTH_SCOPE
  if self._channel_provider and self._channel_provider.is_government()

libraries/botbuilder-core/tests/skills/test_skill_conversation_id_factory.py

@@ -0,0 +1,116 @@
+# Copyright (c) Microsoft Corporation. All rights reserved.
+# Licensed under the MIT License.
+
+from uuid import uuid4 as uuid
+from aiounittest import AsyncTestCase
+from botbuilder.core import MemoryStorage
+from botbuilder.schema import (
+ Activity,
+ ConversationAccount,
+ ConversationReference,
+)
+from botbuilder.core.skills import (
+ BotFrameworkSkill,
+ SkillConversationIdFactory,
+ SkillConversationIdFactoryOptions,
+)
+
+
+class SkillConversationIdFactoryForTest(AsyncTestCase):
+ SERVICE_URL = "http://testbot.com/api/messages"
+ SKILL_ID = "skill"
+
+ @classmethod
+ def setUpClass(cls):
+ cls._skill_conversation_id_factory = SkillConversationIdFactory(MemoryStorage())
+ cls._application_id = str(uuid())
+ cls._bot_id = str(uuid())
+
+ async def test_skill_conversation_id_factory_happy_path(self):
+ conversation_reference = self._build_conversation_reference()
+
+ # Create skill conversation
+ skill_conversation_id = await self._skill_conversation_id_factory.create_skill_conversation_id(
+ options=SkillConversationIdFactoryOptions(
+ activity=self._build_message_activity(conversation_reference),
+ bot_framework_skill=self._build_bot_framework_skill(),
+ from_bot_id=self._bot_id,
+ from_bot_oauth_scope=self._bot_id,
+ )
+ )
+
+ assert (
+ skill_conversation_id and skill_conversation_id.strip()
+ ), "Expected a valid skill conversation ID to be created"
+
+ # Retrieve skill conversation
+ retrieved_conversation_reference = await self._skill_conversation_id_factory.get_skill_conversation_reference(
+ skill_conversation_id
+ )
+
+ # Delete
+ await self._skill_conversation_id_factory.delete_conversation_reference(
+ skill_conversation_id
+ )
+
+ # Retrieve again
+ deleted_conversation_reference = await self._skill_conversation_id_factory.get_skill_conversation_reference(
+ skill_conversation_id
+ )
+
+ self.assertIsNotNone(retrieved_conversation_reference)
+ self.assertIsNotNone(retrieved_conversation_reference.conversation_reference)
+ self.assertEqual(
+ conversation_reference,
+ retrieved_conversation_reference.conversation_reference,
+ )
+ self.assertIsNone(deleted_conversation_reference)
+
+ async def test_id_is_unique_each_time(self):
+ conversation_reference = self._build_conversation_reference()
+
+ # Create skill conversation
+ first_id = await self._skill_conversation_id_factory.create_skill_conversation_id(
+ options=SkillConversationIdFactoryOptions(
+ activity=self._build_message_activity(conversation_reference),
+ bot_framework_skill=self._build_bot_framework_skill(),
+ from_bot_id=self._bot_id,
+ from_bot_oauth_scope=self._bot_id,
+ )
+ )
+
+ second_id = await self._skill_conversation_id_factory.create_skill_conversation_id(
+ options=SkillConversationIdFactoryOptions(
+ activity=self._build_message_activity(conversation_reference),
+ bot_framework_skill=self._build_bot_framework_skill(),
+ from_bot_id=self._bot_id,
+ from_bot_oauth_scope=self._bot_id,
+ )
+ )
+
+ # Ensure that we get a different conversation_id each time we call create_skill_conversation_id
+ self.assertNotEqual(first_id, second_id)
+
+ def _build_conversation_reference(self) -> ConversationReference:
+ return ConversationReference(
+ conversation=ConversationAccount(id=str(uuid())),
+ service_url=self.SERVICE_URL,
+ )
+
+ def _build_message_activity(
+ self, conversation_reference: ConversationReference
+ ) -> Activity:
+ if not conversation_reference:
+ raise TypeError(str(conversation_reference))
+
+ activity = Activity.create_message_activity()
+ activity.apply_conversation_reference(conversation_reference)
+
+ return activity
+
+ def _build_bot_framework_skill(self) -> BotFrameworkSkill:
+ return BotFrameworkSkill(
+ app_id=self._application_id,
+ id=self.SKILL_ID,
+ skill_endpoint=self.SERVICE_URL,
+ )