From a33e803ee8746b59281fb7d5c04bb18a0c25ef57 Mon Sep 17 00:00:00 2001 From: Rodrigo Brandao Date: Tue, 7 Oct 2025 13:11:32 -0700 Subject: [PATCH 01/22] Adding skeleton for sub-channel handling --- .../microsoft_agents/activity/__init__.py | 2 + .../microsoft_agents/activity/activity.py | 34 +++++++++++- .../microsoft_agents/activity/channel_id.py | 55 +++++++++++++++++++ tests/activity/test_channel_id.py | 19 +++++++ tests/activity/test_sub_channels.py | 4 ++ 5 files changed, 111 insertions(+), 3 deletions(-) create mode 100644 libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py create mode 100644 tests/activity/test_channel_id.py create mode 100644 tests/activity/test_sub_channels.py diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/__init__.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/__init__.py index cbd7bb7a..183f82ea 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/__init__.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/__init__.py @@ -17,6 +17,7 @@ from .card_image import CardImage from .channels import Channels from .channel_account import ChannelAccount +from .channel_id import ChannelId from .conversation_account import ConversationAccount from .conversation_members import ConversationMembers from .conversation_parameters import ConversationParameters @@ -114,6 +115,7 @@ "CardImage", "Channels", "ChannelAccount", + "ChannelId", "ConversationAccount", "ConversationMembers", "ConversationParameters", diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py index a573e10a..2fec8c90 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py @@ -1,7 +1,9 @@ from copy import copy from datetime import datetime, timezone -from typing import Optional -from pydantic import Field, SerializeAsAny +from typing import Optional, Any + +from pydantic import Field, SerializeAsAny, model_serializer, model_validator + from .activity_types import ActivityTypes from .channel_account import ChannelAccount from .conversation_account import ConversationAccount @@ -21,6 +23,7 @@ from .semantic_action import SemanticAction from .agents_model import AgentsModel from .role_types import RoleTypes +from .channel_id import ChannelId from ._model_utils import pick_model, SkipNone from ._type_aliases import NonEmptyString @@ -133,7 +136,7 @@ class Activity(AgentsModel): local_timestamp: datetime = None local_timezone: NonEmptyString = None service_url: NonEmptyString = None - channel_id: NonEmptyString = None + channel_id: ChannelId = None from_property: ChannelAccount = Field(None, alias="from") conversation: ConversationAccount = None recipient: ChannelAccount = None @@ -170,6 +173,31 @@ class Activity(AgentsModel): semantic_action: SemanticAction = None caller_id: NonEmptyString = None + @classmethod + @model_validator(mode="before") + def _channel_id_extension(cls, data: Any) -> Any: + if "channel_id" in data and data["channel_id"]: + if data["entities"]: + for entity in data["entities"]: + if entity.get("type", "") == "ProductInfo": + sub_channel = entity.get("id") + + # TODO: check if mutation occurs + + data["channel_id"] = { + "channel": data["channel_id"], + "sub_channel": sub_channel, + } + break + return data + + @classmethod + @model_serializer(mode="plain") + def _add_product_info(cls, channel_id: ChannelId) -> str: + if channel_id.sub_channel: + return f"{channel_id.channel}:{channel_id.sub_channel}" + return channel_id.channel + def apply_conversation_reference( self, reference: ConversationReference, is_incoming: bool = False ): diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py new file mode 100644 index 00000000..2fb8a3e3 --- /dev/null +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py @@ -0,0 +1,55 @@ +from typing import Optional, Any + +from pydantic import model_validator, model_serializer + +from .agents_model import AgentsModel + +class ChannelId(AgentsModel): + channel: str + sub_channel: Optional[str] = None + + def __init__(self, channel_id: str): + + split = channel_id.strip().split(":", 1) + if len(split) == 2: + self.channel = split[0].strip() + self.sub_channel = split[1].strip() + else: + self.channel = channel_id + self.sub_channel = None + + @model_validator(mode="before") + @classmethod + def split_channel_ids(cls, data: Any) -> Any: + if isinstance(data, str): + split = data.strip().split(":", 1) + return { + "channel": split[0].strip(), + "sub_channel": split[1].strip() if len(split) == 2 else None, + } + elif isinstance(data, dict): + return data + else: + raise ValueError("Invalid data type for ChannelId") + + + @model_serializer(model="plain") + def serialize_modeL(self) -> str: + return self.channel + + def is_parent_channel(self, channel_id: str) -> bool: + return self.channel.lower() == channel_id.lower() + + def is_sub_channel(self) -> bool: + return bool(self.sub_channel) + + def __eq__(self, other: Any) -> bool: + return str(self) == str(other) + + def __hash__(self) -> int: + return hash(str(self)) + + def __str__(self) -> str: + if self.sub_channel: + return f"{self.channel}:{self.sub_channel}" + return self.channel \ No newline at end of file diff --git a/tests/activity/test_channel_id.py b/tests/activity/test_channel_id.py new file mode 100644 index 00000000..9eb36c5e --- /dev/null +++ b/tests/activity/test_channel_id.py @@ -0,0 +1,19 @@ +import pytest +import pydantic + +from microsoft_agents.activity import ChannelId + +from tests._common.data import TEST_DEFAULTS + +DEFAULTS = TEST_DEFAULTS() + +class TestChannelId: + + def test_validation(self): + pass + + def test_validation_error(self): + pass + + def test_serialization(self): + pass \ No newline at end of file diff --git a/tests/activity/test_sub_channels.py b/tests/activity/test_sub_channels.py new file mode 100644 index 00000000..b68819ee --- /dev/null +++ b/tests/activity/test_sub_channels.py @@ -0,0 +1,4 @@ +from microsoft_agents.activity import Activity, ChannelId, Entity + +class TestSubChannels: + pass \ No newline at end of file From ac741d567073d48cf31158e7b07b87c040752e64 Mon Sep 17 00:00:00 2001 From: Rodrigo Brandao Date: Wed, 8 Oct 2025 07:24:53 -0700 Subject: [PATCH 02/22] ChannelId test setup --- .../microsoft_agents/activity/channel_id.py | 10 -- tests/activity/test_channel_id.py | 98 +++++++++++++++++-- 2 files changed, 92 insertions(+), 16 deletions(-) diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py index 2fb8a3e3..f165cda9 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py @@ -8,16 +8,6 @@ class ChannelId(AgentsModel): channel: str sub_channel: Optional[str] = None - def __init__(self, channel_id: str): - - split = channel_id.strip().split(":", 1) - if len(split) == 2: - self.channel = split[0].strip() - self.sub_channel = split[1].strip() - else: - self.channel = channel_id - self.sub_channel = None - @model_validator(mode="before") @classmethod def split_channel_ids(cls, data: Any) -> Any: diff --git a/tests/activity/test_channel_id.py b/tests/activity/test_channel_id.py index 9eb36c5e..ab3ceb90 100644 --- a/tests/activity/test_channel_id.py +++ b/tests/activity/test_channel_id.py @@ -9,11 +9,97 @@ class TestChannelId: - def test_validation(self): - pass + @pytest.mark.parametrize( + "input, expected", + [ + [ + "email:support", + ChannelId(channel="email", sub_channel="support") + ], + [ + "email", + ChannelId(channel="email", sub_channel=None) + ], + [ + " email:support ", + ChannelId(channel="email", sub_channel="support") + ], + [ + " email ", + ChannelId(channel="email", sub_channel=None) + ], + [ + "email:support:extra", + ChannelId(channel="email", sub_channel="support:extra") + ], + [ + { + "channel": "email", + "sub_channel": "support" + }, + ChannelId(channel="email", sub_channel="support") + ] + ] + ) + def test_validation_dict_form(self, data, expected): + channel_id = ChannelId.model_validate(data) + assert channel_id == expected - def test_validation_error(self): - pass + @pytest.mark.parametrize( + "channel_a, channel_b, expected", + [ + [ + ChannelId(channel="email", sub_channel="support"), + ChannelId(channel="email", sub_channel="support"), + True + ], + [ + ChannelId(channel="email", sub_channel="support"), + ChannelId(channel="word", sub_channel="support"), + True + ], + [ + ChannelId(channel="email", sub_channel="support"), + ChannelId(channel="email", sub_channel="info"), + False + ], + [ + ChannelId(channel="email", sub_channel=None), + ChannelId(channel="email", sub_channel=None), + True + ], + [ + ChannelId(channel="email", sub_channel=None), + ChannelId(channel="email", sub_channel="support"), + False + ], + [ + ChannelId(channel="email", sub_channel=None), + ChannelId(channel="sms", sub_channel=None), + False + ] + ] + ) + def test_eq(self, channel_a, channel_b, expected): + assert (a == b) == expected + assert (a == a) == True - def test_serialization(self): - pass \ No newline at end of file + def test_hash(self): + a = ChannelId(channel="email", sub_channel="SUPPORT") + assert hash(a) == hash(str(a)) + + def test_str(self): + a = ChannelId(channel="email", sub_channel="support") + assert str(a) == "email:support" + b = ChannelId(channel="email", sub_channel=None) + assert str(b) == "email" + c = ChannelId({"channel": "agents", "sub_channel": "word"}) + assert str(c) == "agents:word" + d = ChannelId("agents:COPILOT") + assert str(d) == "agents:COPILOT" + + # def test_validation_error(self): + # pass + + # def test_serialization(self): + # pass \ No newline at end of file From 0fca8465b0daaebd1ff1e501eea4e71bc58e6352 Mon Sep 17 00:00:00 2001 From: Rodrigo Brandao Date: Wed, 8 Oct 2025 08:41:59 -0700 Subject: [PATCH 03/22] Defining serialized and validators for Activity and ChannelId --- .../microsoft_agents/activity/activity.py | 73 ++++++++++++++++--- .../microsoft_agents/activity/channel_id.py | 8 ++ .../activity/conversation_reference.py | 5 +- .../activity/entity/__init__.py | 2 + .../activity/entity/entity.py | 4 +- .../activity/entity/entity_types.py | 9 +++ .../activity/entity/product_info.py | 16 ++++ .../microsoft-agents-activity/pyproject.toml | 1 + 8 files changed, 101 insertions(+), 17 deletions(-) create mode 100644 libraries/microsoft-agents-activity/microsoft_agents/activity/entity/entity_types.py create mode 100644 libraries/microsoft-agents-activity/microsoft_agents/activity/entity/product_info.py diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py index 2fec8c90..69795827 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py @@ -1,8 +1,17 @@ +from __future__ import annotations + from copy import copy from datetime import datetime, timezone -from typing import Optional, Any - -from pydantic import Field, SerializeAsAny, model_serializer, model_validator +from typing import Optional, Any, Union + +from pydantic import ( + Field, + SerializeAsAny, + model_serializer, + model_validator, + field_validator, + SerializerFunctionWrapHandler +) from .activity_types import ActivityTypes from .channel_account import ChannelAccount @@ -13,6 +22,7 @@ from .attachment import Attachment from .entity import ( Entity, + EntityTypes, Mention, AIEntity, ClientCitation, @@ -50,8 +60,8 @@ class Activity(AgentsModel): :type local_timezone: str :param service_url: Contains the URL that specifies the channel's service endpoint. Set by the channel. :type service_url: str - :param channel_id: Contains an ID that uniquely identifies the channel. Set by the channel. - :type channel_id: str + :param channel_id: Contains an ID that uniquely identifies the channel (and possibly the sub-channel). Set by the channel. + :type channel_id: ~microsoft_agents.activity.ChannelId :param from_property: Identifies the sender of the message. :type from_property: ~microsoft_agents.activity.ChannelAccount :param conversation: Identifies the conversation to which the activity belongs. @@ -173,9 +183,16 @@ class Activity(AgentsModel): semantic_action: SemanticAction = None caller_id: NonEmptyString = None + @field_validator("channel_id", mode="before") @classmethod + def _channel_id_str_to_obj(cls, value: Union[str, ChannelId]) -> ChannelId: + if isinstance(value, str): + return ChannelId.from_string(value) + return value + @model_validator(mode="before") - def _channel_id_extension(cls, data: Any) -> Any: + @classmethod + def _channel_validation(cls, data: Any) -> Any: if "channel_id" in data and data["channel_id"]: if data["entities"]: for entity in data["entities"]: @@ -190,13 +207,38 @@ def _channel_id_extension(cls, data: Any) -> Any: } break return data + + @model_validator(mode="after") + def _channel_validation(self) -> Activity: + product_info = self.get_product_info_entity() + if product_info: + if self.channel_id: + self.channel_id.sub_channel = product_info.id + return self - @classmethod - @model_serializer(mode="plain") - def _add_product_info(cls, channel_id: ChannelId) -> str: - if channel_id.sub_channel: - return f"{channel_id.channel}:{channel_id.sub_channel}" - return channel_id.channel + @model_serializer(mode="wrap") + def serialize_model(self, handler: SerializerFunctionWrapHandler) -> dict[str, object]: + + product_info = self.get_product_info_entity() + + serialized = handler(self) + product_info = None + for i, entity in enumerate(serialized.get("entities", [])): + if entity.get("type", "") == EntityTypes.PRODUCT_INFO: + product_info = entity + break + + if self.channel_id and self.channel_id.sub_channel: + if product_info: + product_info.id = self.channel_id.sub_channel + else: + if not serialized["entities"]: + serialized["entities"] = [] + serialized["entities"].append({"type": EntityTypes.PRODUCT_INFO, "id": self.channel_id.sub_channel}) + elif product_info: + del serialized["entities"][i] + + return serialized def apply_conversation_reference( self, reference: ConversationReference, is_incoming: bool = False @@ -555,6 +597,13 @@ def get_conversation_reference(self) -> ConversationReference: locale=self.locale, service_url=self.service_url, ) + + def get_product_info_entity(self) -> Optional[ProductInfo]: + if not self.entities: + return None + target = EntityTypes.PRODUCT_INFO.lower() + return next(filter(lambda e: e.type.lower() == target, self.entities), None) + def get_mentions(self) -> list[Mention]: """ diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py index f165cda9..76c77a98 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py @@ -1,3 +1,5 @@ +from __future__ import annotations + from typing import Optional, Any from pydantic import model_validator, model_serializer @@ -8,6 +10,12 @@ class ChannelId(AgentsModel): channel: str sub_channel: Optional[str] = None + @staticmethod + def from_str(channel_id: str) -> ChannelId: + return ChannelId( + **ChannelId.split_channel_ids(channel_id) # type: ignore + ) + @model_validator(mode="before") @classmethod def split_channel_ids(cls, data: Any) -> Any: diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/conversation_reference.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/conversation_reference.py index 85c117a7..9475d469 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/conversation_reference.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/conversation_reference.py @@ -4,6 +4,7 @@ from pydantic import Field from .channel_account import ChannelAccount +from .channel_id import ChannelId from .conversation_account import ConversationAccount from .agents_model import AgentsModel from ._type_aliases import NonEmptyString @@ -23,7 +24,7 @@ class ConversationReference(AgentsModel): :param conversation: Conversation reference :type conversation: ~microsoft_agents.activity.ConversationAccount :param channel_id: Channel ID - :type channel_id: str + :type channel_id: ~microsoft_agents.activity.ChannelId :param locale: A locale name for the contents of the text field. The locale name is a combination of an ISO 639 two- or three-letter culture code associated with a language and an ISO 3166 two-letter @@ -40,7 +41,7 @@ class ConversationReference(AgentsModel): user: Optional[ChannelAccount] = None agent: ChannelAccount = Field(None, alias="bot") conversation: ConversationAccount - channel_id: NonEmptyString + channel_id: ChannelId locale: Optional[NonEmptyString] = None service_url: NonEmptyString = None diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/__init__.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/__init__.py index b35460d8..cc6f5023 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/__init__.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/__init__.py @@ -1,5 +1,6 @@ from .mention import Mention from .entity import Entity +from .entity_types import EntityTypes from .ai_entity import ( ClientCitation, ClientCitationAppearance, @@ -15,6 +16,7 @@ __all__ = [ "Entity", + "EntityTypes", "AIEntity", "ClientCitation", "ClientCitationAppearance", diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/entity.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/entity.py index eee24060..ff19f0c5 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/entity.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/entity.py @@ -1,11 +1,9 @@ -from typing import Any, Optional -from enum import Enum +from typing import Any from pydantic import model_serializer, model_validator from pydantic.alias_generators import to_camel, to_snake from ..agents_model import AgentsModel, ConfigDict -from .._type_aliases import NonEmptyString class Entity(AgentsModel): diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/entity_types.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/entity_types.py new file mode 100644 index 00000000..2381222c --- /dev/null +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/entity_types.py @@ -0,0 +1,9 @@ +from strenum import StrEnum + +class EntityTypes(StrEnum): + """Enumeration of entity types.""" + + GEO_COORDINATES = "GeoCoordinates" + PLACE = "Place" + THING = "Thing" + PRODUCT_INFO = "ProductInfo" \ No newline at end of file diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/product_info.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/product_info.py new file mode 100644 index 00000000..2d8d3d8e --- /dev/null +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/product_info.py @@ -0,0 +1,16 @@ +from .entity import Entity +from .entity_types import EntityTypes + +class ProductInfo(Entity): + """Product information (entity type: "productInfo"). + + :param mentioned: The mentioned user + :type mentioned: ~microsoft_agents.activity.ChannelAccount + :param text: Sub Text which represents the mention (can be null or empty) + :type text: str + :param type: Type of this entity (RFC 3987 IRI) + :type type: str + """ + + type: str = EntityTypes.PRODUCT_INFO + id: str = None \ No newline at end of file diff --git a/libraries/microsoft-agents-activity/pyproject.toml b/libraries/microsoft-agents-activity/pyproject.toml index 2369332e..7584c55e 100644 --- a/libraries/microsoft-agents-activity/pyproject.toml +++ b/libraries/microsoft-agents-activity/pyproject.toml @@ -15,6 +15,7 @@ classifiers = [ ] dependencies = [ "pydantic>=2.10.4", + "strenum", ] [project.urls] From 1c3fd96f5d026f281806349c63ecf36ad43e79ce Mon Sep 17 00:00:00 2001 From: Rodrigo Brandao Date: Wed, 8 Oct 2025 09:06:25 -0700 Subject: [PATCH 04/22] Passing ChannelId tests --- .../microsoft_agents/activity/activity.py | 2 +- .../microsoft_agents/activity/channel_id.py | 5 ----- tests/activity/test_channel_id.py | 21 +++++++++++++------ 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py index 69795827..bf85db5c 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py @@ -187,7 +187,7 @@ class Activity(AgentsModel): @classmethod def _channel_id_str_to_obj(cls, value: Union[str, ChannelId]) -> ChannelId: if isinstance(value, str): - return ChannelId.from_string(value) + return ChannelId.model_validate(value) return value @model_validator(mode="before") diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py index 76c77a98..11f42a88 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py @@ -29,11 +29,6 @@ def split_channel_ids(cls, data: Any) -> Any: return data else: raise ValueError("Invalid data type for ChannelId") - - - @model_serializer(model="plain") - def serialize_modeL(self) -> str: - return self.channel def is_parent_channel(self, channel_id: str) -> bool: return self.channel.lower() == channel_id.lower() diff --git a/tests/activity/test_channel_id.py b/tests/activity/test_channel_id.py index ab3ceb90..622263cd 100644 --- a/tests/activity/test_channel_id.py +++ b/tests/activity/test_channel_id.py @@ -10,7 +10,7 @@ class TestChannelId: @pytest.mark.parametrize( - "input, expected", + "data, expected", [ [ "email:support", @@ -56,7 +56,7 @@ def test_validation_dict_form(self, data, expected): [ ChannelId(channel="email", sub_channel="support"), ChannelId(channel="word", sub_channel="support"), - True + False ], [ ChannelId(channel="email", sub_channel="support"), @@ -81,8 +81,9 @@ def test_validation_dict_form(self, data, expected): ] ) def test_eq(self, channel_a, channel_b, expected): - assert (a == b) == expected - assert (a == a) == True + assert (channel_a == channel_b) == expected + assert (channel_a == channel_a) == True + assert (channel_b == channel_b) == True def test_hash(self): a = ChannelId(channel="email", sub_channel="SUPPORT") @@ -93,11 +94,19 @@ def test_str(self): assert str(a) == "email:support" b = ChannelId(channel="email", sub_channel=None) assert str(b) == "email" - c = ChannelId({"channel": "agents", "sub_channel": "word"}) + c = ChannelId(**{"channel": "agents", "sub_channel": "word"}) assert str(c) == "agents:word" - d = ChannelId("agents:COPILOT") + d = ChannelId.model_validate("agents:COPILOT") assert str(d) == "agents:COPILOT" + def test_round_trip(self): + a = ChannelId(channel="email", sub_channel="support") + b = ChannelId.model_validate(str(a)) + assert a == b + c = ChannelId.model_validate(b.model_dump()) + assert a == c + assert b == c + # def test_validation_error(self): # pass From 9b8532abeee472ffce28d2aabfdfcfd191a56cf4 Mon Sep 17 00:00:00 2001 From: Rodrigo Brandao Date: Wed, 8 Oct 2025 09:12:35 -0700 Subject: [PATCH 05/22] Fixing imports --- .../microsoft_agents/activity/__init__.py | 2 + .../microsoft_agents/activity/channel_id.py | 3 -- .../activity/entity/__init__.py | 2 + tests/activity/test_activity.py | 54 +++++++++++++++++++ tests/activity/test_channel_id.py | 14 +++-- 5 files changed, 67 insertions(+), 8 deletions(-) diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/__init__.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/__init__.py index 183f82ea..1dc89973 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/__init__.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/__init__.py @@ -37,6 +37,7 @@ SensitivityPattern, GeoCoordinates, Place, + ProductInfo, Thing, ) from .error import Error @@ -146,6 +147,7 @@ "OAuthCard", "PagedMembersResult", "Place", + "ProductInfo", "ReceiptCard", "ReceiptItem", "ResourceResponse", diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py index 11f42a88..d680cf71 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py @@ -32,9 +32,6 @@ def split_channel_ids(cls, data: Any) -> Any: def is_parent_channel(self, channel_id: str) -> bool: return self.channel.lower() == channel_id.lower() - - def is_sub_channel(self) -> bool: - return bool(self.sub_channel) def __eq__(self, other: Any) -> bool: return str(self) == str(other) diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/__init__.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/__init__.py index cc6f5023..94854ad1 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/__init__.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/__init__.py @@ -12,6 +12,7 @@ ) from .geo_coordinates import GeoCoordinates from .place import Place +from .product_info import ProductInfo from .thing import Thing __all__ = [ @@ -27,5 +28,6 @@ "SensitivityPattern", "GeoCoordinates", "Place", + "ProductInfo", "Thing", ] diff --git a/tests/activity/test_activity.py b/tests/activity/test_activity.py index 40d695fe..b4751139 100644 --- a/tests/activity/test_activity.py +++ b/tests/activity/test_activity.py @@ -16,6 +16,7 @@ AIEntity, Place, Thing, + ProductInfo RoleTypes, ) @@ -373,6 +374,59 @@ def test_get_mentions(self): Entity(type="mention", text="Another mention"), ] + @pytest.mark.parametrize( + "entities, expected", + [ + [ + [ + Entity( + type="ProductInfo", + id="product_123", + ), + Entity(type="other"), + Entity(type="mention", text="Another mention"), + ], + ProductInfo( + type="PRODUCTINFO", + id="product_123", + ) + ], + [ + [ + Entity(type="other"), + Entity(type="mention", text="Another mention"), + ], + None + ], + [ + [ + Entity( + type="ProductInfo", + id="product_123", + ), + Entity( + type="ProductInfo", + id="product_456", + ), + Entity(type="mention", text="Another mention"), + ], + Entity(type="ProductInfo", id="product_123") + ], + [ + [], + None + ], + [ + None, + None + ] + ] + ) + def test_get_product_info_entity_single(self, entities, expected): + activity = Activity(type="message", entities=entities) + retrieved_product_info = activity.get_product_info_entity() + assert retrieved_product_info == expected + class TestActivityAgenticOps: diff --git a/tests/activity/test_channel_id.py b/tests/activity/test_channel_id.py index 622263cd..6f44c28a 100644 --- a/tests/activity/test_channel_id.py +++ b/tests/activity/test_channel_id.py @@ -107,8 +107,12 @@ def test_round_trip(self): assert a == c assert b == c - # def test_validation_error(self): - # pass - - # def test_serialization(self): - # pass \ No newline at end of file + def test_is_parent_channel(self): + a = ChannelId(channel="email", sub_channel="support") + assert a.is_parent_channel("email") == True + assert a.is_parent_channel("EMAIL") == True + assert a.is_parent_channel("word") == False + b = ChannelId(channel="sms", sub_channel=None) + assert b.is_parent_channel("sms") == True + assert b.is_parent_channel("SMS") == True + assert b.is_parent_channel("email") == False \ No newline at end of file From 14d2f3d0d6604016c3d8804ef003d1893bc6ac0d Mon Sep 17 00:00:00 2001 From: Rodrigo Brandao Date: Wed, 8 Oct 2025 09:59:22 -0700 Subject: [PATCH 06/22] Reorganizing Activity tests --- .../microsoft_agents/activity/activity.py | 20 +------------------ tests/activity/pydantic/__init__.py | 0 .../activity/pydantic/test_activity_model.py | 7 +++++++ tests/activity/test_activity.py | 16 +++------------ 4 files changed, 11 insertions(+), 32 deletions(-) create mode 100644 tests/activity/pydantic/__init__.py create mode 100644 tests/activity/pydantic/test_activity_model.py diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py index bf85db5c..b91803e1 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py @@ -189,25 +189,7 @@ def _channel_id_str_to_obj(cls, value: Union[str, ChannelId]) -> ChannelId: if isinstance(value, str): return ChannelId.model_validate(value) return value - - @model_validator(mode="before") - @classmethod - def _channel_validation(cls, data: Any) -> Any: - if "channel_id" in data and data["channel_id"]: - if data["entities"]: - for entity in data["entities"]: - if entity.get("type", "") == "ProductInfo": - sub_channel = entity.get("id") - - # TODO: check if mutation occurs - - data["channel_id"] = { - "channel": data["channel_id"], - "sub_channel": sub_channel, - } - break - return data - + @model_validator(mode="after") def _channel_validation(self) -> Activity: product_info = self.get_product_info_entity() diff --git a/tests/activity/pydantic/__init__.py b/tests/activity/pydantic/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/activity/pydantic/test_activity_model.py b/tests/activity/pydantic/test_activity_model.py new file mode 100644 index 00000000..77cfaeea --- /dev/null +++ b/tests/activity/pydantic/test_activity_model.py @@ -0,0 +1,7 @@ +class TestActivityModel: + + def test_serialize_basic(self, activity): + activity_copy = Activity( + **activity.model_dump(mode="json", exclude_unset=True, by_alias=True) + ) + assert activity_copy == activity \ No newline at end of file diff --git a/tests/activity/test_activity.py b/tests/activity/test_activity.py index b4751139..62035677 100644 --- a/tests/activity/test_activity.py +++ b/tests/activity/test_activity.py @@ -16,7 +16,7 @@ AIEntity, Place, Thing, - ProductInfo + ProductInfo, RoleTypes, ) @@ -353,12 +353,6 @@ def test_is_from_streaming_connection(self, service_url, expected): activity = Activity(type="message", service_url=service_url) assert activity.is_from_streaming_connection() == expected - def test_serialize_basic(self, activity): - activity_copy = Activity( - **activity.model_dump(mode="json", exclude_unset=True, by_alias=True) - ) - assert activity_copy == activity - def test_get_mentions(self): activity = Activity( type="message", @@ -386,8 +380,8 @@ def test_get_mentions(self): Entity(type="other"), Entity(type="mention", text="Another mention"), ], - ProductInfo( - type="PRODUCTINFO", + Entity( + type="ProductInfo", id="product_123", ) ], @@ -415,10 +409,6 @@ def test_get_mentions(self): [ [], None - ], - [ - None, - None ] ] ) From e10f9ab3373776fa75a3d4d0149ca858cda36046 Mon Sep 17 00:00:00 2001 From: Rodrigo Brandao Date: Wed, 8 Oct 2025 12:18:59 -0700 Subject: [PATCH 07/22] Test adjustment --- .../microsoft_agents/activity/activity.py | 2 +- .../activity/pydantic/test_activity_model.py | 33 ++++++++++++++++++- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py index b91803e1..d5b7760c 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py @@ -2,7 +2,7 @@ from copy import copy from datetime import datetime, timezone -from typing import Optional, Any, Union +from typing import Optional, Union from pydantic import ( Field, diff --git a/tests/activity/pydantic/test_activity_model.py b/tests/activity/pydantic/test_activity_model.py index 77cfaeea..f43e234c 100644 --- a/tests/activity/pydantic/test_activity_model.py +++ b/tests/activity/pydantic/test_activity_model.py @@ -1,7 +1,38 @@ +import pytest + +from microsoft_agents.activity import ( + Activity, + ChannelId +) + class TestActivityModel: def test_serialize_basic(self, activity): activity_copy = Activity( **activity.model_dump(mode="json", exclude_unset=True, by_alias=True) ) - assert activity_copy == activity \ No newline at end of file + assert activity_copy == activity + + @pytest.mark.parametrize( + "data, expected, + [ + (ChannelId.MSTEAMS, ChannelId(channel=C)), + ("msteams", "msteams",), + ("msteams/subchannel", "msteams/subchannel"), + ("channel:sub", "channel:sub"), + ] + + def test_channel_id_field_validator_basic(self): + activity = Activity(type="message") + activity.channel_id = ChannelId.MSTEAMS + assert activity.channel_id == ChannelId.MSTEAMS + assert isinstance(activity.channel_id, ChannelId) + + def test_channel_id_field_validator_from_str_basic(self): + activity = Activity(type="message") + activity.channel_id = "msteams" + assert activity.channel_id == "msteams" + assert isinstance(activity.channel_id, ChannelId) + + def test_channel_id_field_validator_from_str_with_sub_channel(self): + pass \ No newline at end of file From 2b139022f6945e1d5ca63506eb2274b84a83cc3c Mon Sep 17 00:00:00 2001 From: Rodrigo Brandao Date: Wed, 8 Oct 2025 14:07:45 -0700 Subject: [PATCH 08/22] Adjusting setter/getter for _channel_id in Activity --- .../microsoft_agents/activity/activity.py | 47 ++++++++++++++----- .../microsoft_agents/activity/channel_id.py | 25 +++++----- .../activity/pydantic/test_activity_model.py | 30 +++++------- tests/activity/test_channel_id.py | 14 ++---- 4 files changed, 63 insertions(+), 53 deletions(-) diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py index d5b7760c..ec1c713a 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py @@ -1,16 +1,19 @@ from __future__ import annotations +importl logging from copy import copy from datetime import datetime, timezone -from typing import Optional, Union +from typing import Optional, Union, Any from pydantic import ( Field, SerializeAsAny, model_serializer, model_validator, - field_validator, - SerializerFunctionWrapHandler + SerializerFunctionWrapHandler, + ModelWrapValidatorHandler, + computed_field, + ValidationError ) from .activity_types import ActivityTypes @@ -37,6 +40,7 @@ from ._model_utils import pick_model, SkipNone from ._type_aliases import NonEmptyString +logger = logging.getLogger(__name__) # TODO: A2A Agent 2 is responding with None as id, had to mark it as optional (investigate) class Activity(AgentsModel): @@ -146,7 +150,7 @@ class Activity(AgentsModel): local_timestamp: datetime = None local_timezone: NonEmptyString = None service_url: NonEmptyString = None - channel_id: ChannelId = None + _channel_id: ChannelId = None from_property: ChannelAccount = Field(None, alias="from") conversation: ConversationAccount = None recipient: ChannelAccount = None @@ -183,19 +187,37 @@ class Activity(AgentsModel): semantic_action: SemanticAction = None caller_id: NonEmptyString = None - @field_validator("channel_id", mode="before") + @property + def channel_id(self): + """Gets the _channel_id field""" + return self._channel_id + + # necessary for backward compatibility + # previously, channel_id was directly assigned with strings + @channel_id.setter + def channel_id(self, value: Any): + """Sets the channel_id after validating and converting to ChannelId model.""" + self._channel_id = ChannelId.model_validate(value) + + @model_validator(mode="wrap") @classmethod - def _channel_id_str_to_obj(cls, value: Union[str, ChannelId]) -> ChannelId: - if isinstance(value, str): - return ChannelId.model_validate(value) - return value + def _validate(self, data: Any, handler: ModelWrapValidatorHandler[Activity]) -> Activity: + try: + activity = handler(data) + data_channel_id = data.get("channel_id", None) + if data_channel_id: + activity.channel_id = data_channel_id + return activity + except ValidationError: + logger.error("Validation error for Activity") + raise @model_validator(mode="after") - def _channel_validation(self) -> Activity: + def _validate_channel(self) -> Activity: product_info = self.get_product_info_entity() if product_info: - if self.channel_id: - self.channel_id.sub_channel = product_info.id + if self._channel_id: + self._channel_id.sub_channel = product_info.id return self @model_serializer(mode="wrap") @@ -204,6 +226,7 @@ def serialize_model(self, handler: SerializerFunctionWrapHandler) -> dict[str, o product_info = self.get_product_info_entity() serialized = handler(self) + product_info = None for i, entity in enumerate(serialized.get("entities", [])): if entity.get("type", "") == EntityTypes.PRODUCT_INFO: diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py index d680cf71..edbb7b67 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py @@ -2,37 +2,36 @@ from typing import Optional, Any -from pydantic import model_validator, model_serializer +from pydantic import model_validator from .agents_model import AgentsModel class ChannelId(AgentsModel): + """A class representing a channel identifier with optional sub-channel. + + :param channel: The main channel identifier (e.g., "msteams"). + :type channel: str + :param sub_channel: An optional sub-channel identifier (e.g., "subchannel"). + :type sub_channel: Optional[str] + """ channel: str sub_channel: Optional[str] = None - @staticmethod - def from_str(channel_id: str) -> ChannelId: - return ChannelId( - **ChannelId.split_channel_ids(channel_id) # type: ignore - ) - @model_validator(mode="before") @classmethod - def split_channel_ids(cls, data: Any) -> Any: - if isinstance(data, str): + def _split_channel_ids(cls, data: Any) -> Any: + """Validator to split a string into channel and sub_channel if needed.""" + if isinstance(data, str) and data: split = data.strip().split(":", 1) return { "channel": split[0].strip(), "sub_channel": split[1].strip() if len(split) == 2 else None, } - elif isinstance(data, dict): + elif isinstance(data, dict) and data: return data else: raise ValueError("Invalid data type for ChannelId") - def is_parent_channel(self, channel_id: str) -> bool: - return self.channel.lower() == channel_id.lower() - def __eq__(self, other: Any) -> bool: return str(self) == str(other) diff --git a/tests/activity/pydantic/test_activity_model.py b/tests/activity/pydantic/test_activity_model.py index f43e234c..530f4404 100644 --- a/tests/activity/pydantic/test_activity_model.py +++ b/tests/activity/pydantic/test_activity_model.py @@ -14,25 +14,19 @@ def test_serialize_basic(self, activity): assert activity_copy == activity @pytest.mark.parametrize( - "data, expected, + "data, expected", [ - (ChannelId.MSTEAMS, ChannelId(channel=C)), - ("msteams", "msteams",), - ("msteams/subchannel", "msteams/subchannel"), - ("channel:sub", "channel:sub"), - ] - - def test_channel_id_field_validator_basic(self): + ("msteams:subchannel", ChannelId(channel="msteams", sub_channel="subchannel")), + ("msteams/subchannel", ChannelId(channel="msteams/subchannel")), + ("channel:sub", ChannelId(channel="channel", sub_channel="sub")), + (ChannelId(channel="msteams", sub_channel="subchannel"), ChannelId(channel="msteams", sub_channel="subchannel")), + (ChannelId(channel="msteams"), ChannelId(channel="msteams")), + ]) + def test_channel_id_field_validator(self, data, expected): activity = Activity(type="message") - activity.channel_id = ChannelId.MSTEAMS - assert activity.channel_id == ChannelId.MSTEAMS - assert isinstance(activity.channel_id, ChannelId) + activity.channel_id = data - def test_channel_id_field_validator_from_str_basic(self): - activity = Activity(type="message") - activity.channel_id = "msteams" - assert activity.channel_id == "msteams" + assert activity.channel_id == expected + breakpoint() assert isinstance(activity.channel_id, ChannelId) - - def test_channel_id_field_validator_from_str_with_sub_channel(self): - pass \ No newline at end of file + assert activity.channel_id == data \ No newline at end of file diff --git a/tests/activity/test_channel_id.py b/tests/activity/test_channel_id.py index 6f44c28a..529d9ad6 100644 --- a/tests/activity/test_channel_id.py +++ b/tests/activity/test_channel_id.py @@ -1,5 +1,5 @@ import pytest -import pydantic +from pydantic import ValidationError from microsoft_agents.activity import ChannelId @@ -107,12 +107,6 @@ def test_round_trip(self): assert a == c assert b == c - def test_is_parent_channel(self): - a = ChannelId(channel="email", sub_channel="support") - assert a.is_parent_channel("email") == True - assert a.is_parent_channel("EMAIL") == True - assert a.is_parent_channel("word") == False - b = ChannelId(channel="sms", sub_channel=None) - assert b.is_parent_channel("sms") == True - assert b.is_parent_channel("SMS") == True - assert b.is_parent_channel("email") == False \ No newline at end of file + def test_channel_id_validation_error(self): + with pytest.raises(ValidationError): + ChannelId(channel="") \ No newline at end of file From dbdedbcf8a253b50196a696769bf9fe58d8f2ce2 Mon Sep 17 00:00:00 2001 From: Rodrigo Brandao Date: Wed, 8 Oct 2025 15:38:33 -0700 Subject: [PATCH 09/22] Fixing test cases and finalizing Activity serializer --- .../microsoft_agents/activity/__init__.py | 1 + .../microsoft_agents/activity/activity.py | 65 +++-- .../microsoft_agents/activity/channel_id.py | 19 +- .../activity/entity/entity_types.py | 11 +- .../activity/entity/product_info.py | 11 +- tests/activity/pydantic/test_activity_io.py | 225 ++++++++++++++++++ .../activity/pydantic/test_activity_model.py | 32 --- tests/activity/test_activity.py | 13 +- tests/activity/test_channel_id.py | 52 ++-- tests/activity/test_sub_channels.py | 3 +- 10 files changed, 317 insertions(+), 115 deletions(-) create mode 100644 tests/activity/pydantic/test_activity_io.py delete mode 100644 tests/activity/pydantic/test_activity_model.py diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/__init__.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/__init__.py index 1dc89973..2d84011e 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/__init__.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/__init__.py @@ -27,6 +27,7 @@ from .expected_replies import ExpectedReplies from .entity import ( Entity, + EntityTypes, AIEntity, ClientCitation, ClientCitationAppearance, diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py index ec1c713a..b83e6e1b 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py @@ -1,6 +1,6 @@ from __future__ import annotations -importl logging +import logging from copy import copy from datetime import datetime, timezone from typing import Optional, Union, Any @@ -13,8 +13,9 @@ SerializerFunctionWrapHandler, ModelWrapValidatorHandler, computed_field, - ValidationError + ValidationError, ) +from yaml import serialize from .activity_types import ActivityTypes from .channel_account import ChannelAccount @@ -29,6 +30,7 @@ Mention, AIEntity, ClientCitation, + ProductInfo, SensitivityUsageInfo, ) from .conversation_reference import ConversationReference @@ -42,6 +44,7 @@ logger = logging.getLogger(__name__) + # TODO: A2A Agent 2 is responding with None as id, had to mark it as optional (investigate) class Activity(AgentsModel): """An Activity is the basic communication type for the protocol. @@ -187,61 +190,76 @@ class Activity(AgentsModel): semantic_action: SemanticAction = None caller_id: NonEmptyString = None + @computed_field(return_type=Optional[ChannelId]) @property def channel_id(self): """Gets the _channel_id field""" return self._channel_id - + # necessary for backward compatibility # previously, channel_id was directly assigned with strings @channel_id.setter def channel_id(self, value: Any): """Sets the channel_id after validating and converting to ChannelId model.""" self._channel_id = ChannelId.model_validate(value) - + @model_validator(mode="wrap") @classmethod - def _validate(self, data: Any, handler: ModelWrapValidatorHandler[Activity]) -> Activity: + def _validate_sub_channel_data( + self, data: Any, handler: ModelWrapValidatorHandler[Activity] + ) -> Activity: try: activity = handler(data) + + # needed to assign to a computed field data_channel_id = data.get("channel_id", None) if data_channel_id: activity.channel_id = data_channel_id + + # sync sub_channel with productInfo entity + product_info = activity.get_product_info_entity() + if product_info and activity.channel_id: + activity.channel_id.sub_channel = product_info.id + return activity except ValidationError: logger.error("Validation error for Activity") raise - - @model_validator(mode="after") - def _validate_channel(self) -> Activity: - product_info = self.get_product_info_entity() - if product_info: - if self._channel_id: - self._channel_id.sub_channel = product_info.id - return self - - @model_serializer(mode="wrap") - def serialize_model(self, handler: SerializerFunctionWrapHandler) -> dict[str, object]: - product_info = self.get_product_info_entity() + @model_serializer(mode="wrap") + def _serialize_sub_channel_data( + self, handler: SerializerFunctionWrapHandler + ) -> dict[str, object]: serialized = handler(self) product_info = None - for i, entity in enumerate(serialized.get("entities", [])): + for i, entity in enumerate(serialized.get("entities") or []): if entity.get("type", "") == EntityTypes.PRODUCT_INFO: product_info = entity break if self.channel_id and self.channel_id.sub_channel: + # maintain consistency between productInfo entity and sub channel if product_info: - product_info.id = self.channel_id.sub_channel + product_info["id"] = self.channel_id.sub_channel else: - if not serialized["entities"]: + if not serialized.get("entities"): serialized["entities"] = [] - serialized["entities"].append({"type": EntityTypes.PRODUCT_INFO, "id": self.channel_id.sub_channel}) - elif product_info: + serialized["entities"].append( + { + "type": EntityTypes.PRODUCT_INFO, + "id": self.channel_id.sub_channel, + } + ) + elif product_info: # remove productInfo entity if sub_channel is not set del serialized["entities"][i] + if not serialized["entities"]: + del serialized["entities"] + + # do not include unset value + if not self.channel_id: + del serialized["channelId"] return serialized @@ -602,14 +620,13 @@ def get_conversation_reference(self) -> ConversationReference: locale=self.locale, service_url=self.service_url, ) - + def get_product_info_entity(self) -> Optional[ProductInfo]: if not self.entities: return None target = EntityTypes.PRODUCT_INFO.lower() return next(filter(lambda e: e.type.lower() == target, self.entities), None) - def get_mentions(self) -> list[Mention]: """ Resolves the mentions from the entities of this activity. diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py index edbb7b67..9b028505 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py @@ -2,19 +2,22 @@ from typing import Optional, Any -from pydantic import model_validator +from pydantic import model_validator, model_serializer +from ._type_aliases import NonEmptyString from .agents_model import AgentsModel + class ChannelId(AgentsModel): """A class representing a channel identifier with optional sub-channel. - + :param channel: The main channel identifier (e.g., "msteams"). :type channel: str :param sub_channel: An optional sub-channel identifier (e.g., "subchannel"). :type sub_channel: Optional[str] """ - channel: str + + channel: NonEmptyString sub_channel: Optional[str] = None @model_validator(mode="before") @@ -31,14 +34,18 @@ def _split_channel_ids(cls, data: Any) -> Any: return data else: raise ValueError("Invalid data type for ChannelId") - + + @model_serializer(mode="plain") + def _serialize_plain(self) -> str: + return str(self) + def __eq__(self, other: Any) -> bool: return str(self) == str(other) def __hash__(self) -> int: return hash(str(self)) - + def __str__(self) -> str: if self.sub_channel: return f"{self.channel}:{self.sub_channel}" - return self.channel \ No newline at end of file + return self.channel diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/entity_types.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/entity_types.py index 2381222c..a2bd4e3f 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/entity_types.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/entity_types.py @@ -1,9 +1,10 @@ from strenum import StrEnum + class EntityTypes(StrEnum): - """Enumeration of entity types.""" + """Well-known enumeration of entity types.""" - GEO_COORDINATES = "GeoCoordinates" - PLACE = "Place" - THING = "Thing" - PRODUCT_INFO = "ProductInfo" \ No newline at end of file + GEO_COORDINATES = "geoCoordinates" + PLACE = "place" + THING = "thing" + PRODUCT_INFO = "productInfo" diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/product_info.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/product_info.py index 2d8d3d8e..31265122 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/product_info.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/product_info.py @@ -1,16 +1,15 @@ from .entity import Entity from .entity_types import EntityTypes + class ProductInfo(Entity): """Product information (entity type: "productInfo"). - :param mentioned: The mentioned user - :type mentioned: ~microsoft_agents.activity.ChannelAccount - :param text: Sub Text which represents the mention (can be null or empty) - :type text: str - :param type: Type of this entity (RFC 3987 IRI) + :param type: The type of the entity, always "productInfo". :type type: str + :param id: The unique identifier for the product. + :type id: str """ type: str = EntityTypes.PRODUCT_INFO - id: str = None \ No newline at end of file + id: str = None diff --git a/tests/activity/pydantic/test_activity_io.py b/tests/activity/pydantic/test_activity_io.py new file mode 100644 index 00000000..72d6f95d --- /dev/null +++ b/tests/activity/pydantic/test_activity_io.py @@ -0,0 +1,225 @@ +from turtle import mode +import pytest + +from pydantic import ValidationError + +from microsoft_agents.activity import ( + Activity, + ChannelId, + Entity, + EntityTypes, + ProductInfo, +) + +from tests import activity + + +# validation / serialization tests +class TestActivityIO: + + def test_serialize_basic(self): + activity = Activity(type="message") + activity_copy = Activity( + **activity.model_dump(mode="json", exclude_unset=True, by_alias=True) + ) + assert activity_copy == activity + + @pytest.mark.parametrize( + "data, expected", + [ + ( + "msteams:subchannel", + ChannelId(channel="msteams", sub_channel="subchannel"), + ), + ("msteams/subchannel", ChannelId(channel="msteams/subchannel")), + ("channel:sub", ChannelId(channel="channel", sub_channel="sub")), + ( + ChannelId(channel="msteams", sub_channel="subchannel"), + ChannelId(channel="msteams", sub_channel="subchannel"), + ), + (ChannelId(channel="msteams"), ChannelId(channel="msteams")), + ( + {"channel": "msteams", "sub_channel": "subchannel"}, + ChannelId(channel="msteams", sub_channel="subchannel"), + ), + ({"channel": "msteams"}, ChannelId(channel="msteams")), + ], + ) + def test_channel_id_setter_validation(self, data, expected): + activity = Activity(type="message") + activity.channel_id = data + + assert activity.channel_id == expected + assert isinstance(activity.channel_id, ChannelId) + if not isinstance(data, dict): + assert activity.channel_id == data + + def test_channel_id_setter_validation_error(self): + activity = Activity(type="message") + with pytest.raises(ValidationError): + activity.channel_id = {} + + def test_channel_id_validate_without_product_info(self): + data = {"type": "message", "channel_id": "msteams:subchannel"} + activity = Activity(**data) + assert activity.channel_id == ChannelId( + channel="msteams", sub_channel="subchannel" + ) + assert not activity.get_product_info_entity() + + @pytest.mark.parametrize( + "data, expected", + [ + [ + { + "type": "message", + "channel_id": "msteams:subchannel", + "entities": [ + {"type": "productInfo", "id": "other"}, + {"type": "productInfo", "id": "other"}, + {"type": "productInfo", "id": "wow"}, + ], + }, + Activity( + type="message", + channel_id="msteams:subchannel", + entities=[ + Entity(type=EntityTypes.PRODUCT_INFO, id="other"), + Entity(type=EntityTypes.PRODUCT_INFO, id="other"), + Entity(type=EntityTypes.PRODUCT_INFO, id="wow"), + ], + ), + ], + [ + { + "type": "message", + "channel_id": "parent:misc", + "entities": [{"type": "some_entity"}], + }, + Activity( + type="message", + channel_id="parent:misc", + entities=[Entity(type="some_entity")], + ), + ], + [ + { + "type": "message", + "channel_id": "parent:misc", + "entities": [ProductInfo(id="sub_channel")], + }, + Activity( + type="message", + channel_id="parent:sub_channel", + entities=[ProductInfo(id="sub_channel")], + ), + ], + ], + ) + def test_channel_id_sub_channel_changed_with_product_info(self, data, expected): + activity = Activity(**data) + assert activity == expected + + def test_channel_id_unset_becomes_set_at_init(self): + activity = Activity(type="message") + activity.channel_id = "channel:sub_channel" + data = activity.model_dump(mode="json", exclude_unset=True, by_alias=True) + assert data["channelId"] == "channel:sub_channel" + + def test_channel_id_unset_at_init_not_included(self): + activity = Activity(type="message") + data = activity.model_dump(mode="json", exclude_unset=True, by_alias=True) + assert "channelId" not in data + + def test_product_info_avoids_error_no_parent_channel(self): + activity = Activity(type="message", entities=[ProductInfo(id="sub_channel")]) + assert activity.channel_id is None + + @pytest.mark.parametrize( + "activity, expected", + [ + [Activity(type="message"), {"type": "message"}], + [ + Activity(type="message", channel_id="msteams"), + {"type": "message", "channelId": "msteams"}, + ], + [ + Activity(type="message", channel_id="msteams:subchannel"), + { + "type": "message", + "channelId": "msteams:subchannel", + "entities": [ + {"type": str(EntityTypes.PRODUCT_INFO), "id": "subchannel"} + ], + }, + ], + [ + Activity( + type="message", + channel_id="msteams:subchannel", + entities=[Entity(type="other")], + ), + { + "type": "message", + "channelId": "msteams:subchannel", + "entities": [ + {"type": "other"}, + {"type": str(EntityTypes.PRODUCT_INFO), "id": "subchannel"}, + ], + }, + ], + [ + Activity( + type="message", + channel_id="msteams:misc", + entities=[{"type": "other"}, ProductInfo(id="misc")], + ), + { + "type": "message", + "channelId": "msteams:misc", + "entities": [ + {"type": "other"}, + {"type": str(EntityTypes.PRODUCT_INFO), "id": "misc"}, + ], + }, + ], + [ + Activity(type="message", entities=[ProductInfo(id="misc")]), + {"type": "message"}, + ], + ], + ) + def test_serialize(self, activity, expected): + data = activity.model_dump(mode="json", exclude_unset=True, by_alias=True) + assert data == expected + + def test_serialize_misconfiguration_no_sub_channel(self): + activity = Activity( + type="message", channel_id="msteams", entities=[{"type": "other"}] + ) + activity.entities.append(ProductInfo(id="sub_channel")) + + data = activity.model_dump(mode="json", exclude_unset=True, by_alias=True) + assert data == { + "type": "message", + "channelId": "msteams", + "entities": [ + {"type": "other"}, + ], + } + + def test_serialize_misconfiguration_wrong_sub_channel(self): + activity = Activity( + type="message", channel_id="msteams:misc", entities=[{"type": "other"}] + ) + activity.entities.append(ProductInfo(id="sub_channel")) + + data = activity.model_dump(mode="json", exclude_unset=True, by_alias=True) + assert data == { + "type": "message", + "channelId": "msteams:misc", + "entities": [ + {"type": "other"}, + {"type": str(EntityTypes.PRODUCT_INFO), "id": "misc"}, + ], + } diff --git a/tests/activity/pydantic/test_activity_model.py b/tests/activity/pydantic/test_activity_model.py deleted file mode 100644 index 530f4404..00000000 --- a/tests/activity/pydantic/test_activity_model.py +++ /dev/null @@ -1,32 +0,0 @@ -import pytest - -from microsoft_agents.activity import ( - Activity, - ChannelId -) - -class TestActivityModel: - - def test_serialize_basic(self, activity): - activity_copy = Activity( - **activity.model_dump(mode="json", exclude_unset=True, by_alias=True) - ) - assert activity_copy == activity - - @pytest.mark.parametrize( - "data, expected", - [ - ("msteams:subchannel", ChannelId(channel="msteams", sub_channel="subchannel")), - ("msteams/subchannel", ChannelId(channel="msteams/subchannel")), - ("channel:sub", ChannelId(channel="channel", sub_channel="sub")), - (ChannelId(channel="msteams", sub_channel="subchannel"), ChannelId(channel="msteams", sub_channel="subchannel")), - (ChannelId(channel="msteams"), ChannelId(channel="msteams")), - ]) - def test_channel_id_field_validator(self, data, expected): - activity = Activity(type="message") - activity.channel_id = data - - assert activity.channel_id == expected - breakpoint() - assert isinstance(activity.channel_id, ChannelId) - assert activity.channel_id == data \ No newline at end of file diff --git a/tests/activity/test_activity.py b/tests/activity/test_activity.py index 62035677..a6874186 100644 --- a/tests/activity/test_activity.py +++ b/tests/activity/test_activity.py @@ -383,14 +383,14 @@ def test_get_mentions(self): Entity( type="ProductInfo", id="product_123", - ) + ), ], [ [ Entity(type="other"), Entity(type="mention", text="Another mention"), ], - None + None, ], [ [ @@ -404,13 +404,10 @@ def test_get_mentions(self): ), Entity(type="mention", text="Another mention"), ], - Entity(type="ProductInfo", id="product_123") + Entity(type="ProductInfo", id="product_123"), ], - [ - [], - None - ] - ] + [[], None], + ], ) def test_get_product_info_entity_single(self, entities, expected): activity = Activity(type="message", entities=entities) diff --git a/tests/activity/test_channel_id.py b/tests/activity/test_channel_id.py index 529d9ad6..389b2b47 100644 --- a/tests/activity/test_channel_id.py +++ b/tests/activity/test_channel_id.py @@ -7,39 +7,25 @@ DEFAULTS = TEST_DEFAULTS() + class TestChannelId: @pytest.mark.parametrize( "data, expected", [ - [ - "email:support", - ChannelId(channel="email", sub_channel="support") - ], - [ - "email", - ChannelId(channel="email", sub_channel=None) - ], - [ - " email:support ", - ChannelId(channel="email", sub_channel="support") - ], - [ - " email ", - ChannelId(channel="email", sub_channel=None) - ], + ["email:support", ChannelId(channel="email", sub_channel="support")], + ["email", ChannelId(channel="email", sub_channel=None)], + [" email:support ", ChannelId(channel="email", sub_channel="support")], + [" email ", ChannelId(channel="email", sub_channel=None)], [ "email:support:extra", - ChannelId(channel="email", sub_channel="support:extra") + ChannelId(channel="email", sub_channel="support:extra"), ], [ - { - "channel": "email", - "sub_channel": "support" - }, - ChannelId(channel="email", sub_channel="support") - ] - ] + {"channel": "email", "sub_channel": "support"}, + ChannelId(channel="email", sub_channel="support"), + ], + ], ) def test_validation_dict_form(self, data, expected): channel_id = ChannelId.model_validate(data) @@ -51,34 +37,34 @@ def test_validation_dict_form(self, data, expected): [ ChannelId(channel="email", sub_channel="support"), ChannelId(channel="email", sub_channel="support"), - True + True, ], [ ChannelId(channel="email", sub_channel="support"), ChannelId(channel="word", sub_channel="support"), - False + False, ], [ ChannelId(channel="email", sub_channel="support"), ChannelId(channel="email", sub_channel="info"), - False + False, ], [ ChannelId(channel="email", sub_channel=None), ChannelId(channel="email", sub_channel=None), - True + True, ], [ ChannelId(channel="email", sub_channel=None), ChannelId(channel="email", sub_channel="support"), - False + False, ], [ ChannelId(channel="email", sub_channel=None), ChannelId(channel="sms", sub_channel=None), - False - ] - ] + False, + ], + ], ) def test_eq(self, channel_a, channel_b, expected): assert (channel_a == channel_b) == expected @@ -109,4 +95,4 @@ def test_round_trip(self): def test_channel_id_validation_error(self): with pytest.raises(ValidationError): - ChannelId(channel="") \ No newline at end of file + ChannelId(channel="") diff --git a/tests/activity/test_sub_channels.py b/tests/activity/test_sub_channels.py index b68819ee..67c6d92c 100644 --- a/tests/activity/test_sub_channels.py +++ b/tests/activity/test_sub_channels.py @@ -1,4 +1,5 @@ from microsoft_agents.activity import Activity, ChannelId, Entity + class TestSubChannels: - pass \ No newline at end of file + pass From 0aeb75e0e2647393f81160f0a53d25befdf7ea0d Mon Sep 17 00:00:00 2001 From: Rodrigo Brandao Date: Wed, 8 Oct 2025 15:42:29 -0700 Subject: [PATCH 10/22] Tweaks to docstrings --- .../microsoft_agents/activity/activity.py | 15 ++++++++++++++- .../microsoft_agents/activity/channel_id.py | 5 +++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py index b83e6e1b..30acee8d 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py @@ -190,6 +190,7 @@ class Activity(AgentsModel): semantic_action: SemanticAction = None caller_id: NonEmptyString = None + # required to define the setter below @computed_field(return_type=Optional[ChannelId]) @property def channel_id(self): @@ -200,7 +201,7 @@ def channel_id(self): # previously, channel_id was directly assigned with strings @channel_id.setter def channel_id(self, value: Any): - """Sets the channel_id after validating and converting to ChannelId model.""" + """Sets the channel_id after validating it as a ChannelId model.""" self._channel_id = ChannelId.model_validate(value) @model_validator(mode="wrap") @@ -208,7 +209,14 @@ def channel_id(self, value: Any): def _validate_sub_channel_data( self, data: Any, handler: ModelWrapValidatorHandler[Activity] ) -> Activity: + """Validate the Activity, ensuring consistency between channel_id.sub_channel and productInfo entity. + + :param data: The input data to validate. + :param handler: The validation handler provided by Pydantic. + :return: The validated Activity instance. + """ try: + # run Pydantic's standard validation first activity = handler(data) # needed to assign to a computed field @@ -230,7 +238,12 @@ def _validate_sub_channel_data( def _serialize_sub_channel_data( self, handler: SerializerFunctionWrapHandler ) -> dict[str, object]: + """Serialize the Activity, ensuring consistency between channel_id.sub_channel and productInfo entity. + :param handler: The serialization handler provided by Pydantic. + :return: A dictionary representing the serialized Activity. + """ + # run Pydantic's standard serialization first serialized = handler(self) product_info = None diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py index 9b028505..430ebca7 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py @@ -1,3 +1,6 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + from __future__ import annotations from typing import Optional, Any @@ -24,6 +27,7 @@ class ChannelId(AgentsModel): @classmethod def _split_channel_ids(cls, data: Any) -> Any: """Validator to split a string into channel and sub_channel if needed.""" + if isinstance(data, str) and data: split = data.strip().split(":", 1) return { @@ -31,6 +35,7 @@ def _split_channel_ids(cls, data: Any) -> Any: "sub_channel": split[1].strip() if len(split) == 2 else None, } elif isinstance(data, dict) and data: + # let Pydantic handle the dict case return data else: raise ValueError("Invalid data type for ChannelId") From f0144435de9bc80546ef607ea7d7bb1fe11b5ae9 Mon Sep 17 00:00:00 2001 From: Rodrigo Brandao Date: Fri, 10 Oct 2025 09:13:15 -0700 Subject: [PATCH 11/22] Addressing review comments --- .../microsoft_agents/activity/activity.py | 2 +- .../microsoft_agents/activity/entity/entity_types.py | 9 +++++---- .../microsoft_agents/activity/entity/geo_coordinates.py | 5 ++++- .../microsoft_agents/activity/entity/mention.py | 4 ++-- .../microsoft_agents/activity/entity/place.py | 5 ++++- .../microsoft_agents/activity/entity/product_info.py | 7 ++++++- .../microsoft_agents/activity/entity/thing.py | 5 ++++- .../hosting/aiohttp/jwt_authorization_middleware.py | 2 ++ .../microsoft_agents/hosting/core/turn_context.py | 3 ++- tests/activity/pydantic/test_activity_io.py | 6 +++--- tests/activity/test_channel_id.py | 5 +++++ 11 files changed, 38 insertions(+), 15 deletions(-) diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py index 4412cb35..1a8d40ce 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py @@ -655,7 +655,7 @@ def get_mentions(self) -> list[Mention]: """ if not self.entities: return [] - return [x for x in self.entities if x.type.lower() == "mention"] + return [x for x in self.entities if x.type.lower() == EntityTypes.MENTION] def get_reply_conversation_reference( self, reply: ResourceResponse diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/entity_types.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/entity_types.py index a2bd4e3f..3f53481c 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/entity_types.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/entity_types.py @@ -4,7 +4,8 @@ class EntityTypes(StrEnum): """Well-known enumeration of entity types.""" - GEO_COORDINATES = "geoCoordinates" - PLACE = "place" - THING = "thing" - PRODUCT_INFO = "productInfo" + GEO_COORDINATES = "GeoCoordinates" + MENTION = "mention" + PLACE = "Place" + THING = "Thing" + PRODUCT_INFO = "ProductInfo" diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/geo_coordinates.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/geo_coordinates.py index 15a9e0a5..0880cadd 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/geo_coordinates.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/geo_coordinates.py @@ -1,8 +1,11 @@ # Copyright (c) Microsoft Corporation. All rights reserved. # Licensed under the MIT License. +from typing import Literal + from ..agents_model import AgentsModel from .._type_aliases import NonEmptyString +from .entity_types import EntityTypes class GeoCoordinates(AgentsModel): @@ -26,5 +29,5 @@ class GeoCoordinates(AgentsModel): elevation: float = None latitude: float = None longitude: float = None - type: NonEmptyString = None + type: Literal[EntityTypes.GEO_COORDINATES] = EntityTypes.GEO_COORDINATES name: NonEmptyString = None diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/mention.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/mention.py index ce8b5084..1223c045 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/mention.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/mention.py @@ -5,7 +5,7 @@ from ..channel_account import ChannelAccount from .entity import Entity -from .._type_aliases import NonEmptyString +from .entity_types import EntityTypes class Mention(Entity): @@ -21,4 +21,4 @@ class Mention(Entity): mentioned: ChannelAccount = None text: str = None - type: Literal["mention"] = "mention" + type: Literal[EntityTypes.MENTION] = EntityTypes.MENTION diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/place.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/place.py index ea0b347f..1aba1f2f 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/place.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/place.py @@ -1,8 +1,11 @@ # Copyright (c) Microsoft Corporation. All rights reserved. # Licensed under the MIT License. +from typing import Literal + from ..agents_model import AgentsModel from .._type_aliases import NonEmptyString +from .entity_types import EntityTypes class Place(AgentsModel): @@ -26,5 +29,5 @@ class Place(AgentsModel): address: object = None geo: object = None has_map: object = None - type: NonEmptyString = None + type: Literal[EntityTypes.PLACE] = EntityTypes.PLACE name: NonEmptyString = None diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/product_info.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/product_info.py index 31265122..17bbc091 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/product_info.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/product_info.py @@ -1,3 +1,8 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + +from typing import Literal + from .entity import Entity from .entity_types import EntityTypes @@ -11,5 +16,5 @@ class ProductInfo(Entity): :type id: str """ - type: str = EntityTypes.PRODUCT_INFO + type: Literal[EntityTypes.PRODUCT_INFO] = EntityTypes.PRODUCT_INFO id: str = None diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/thing.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/thing.py index 040b850a..a05672ab 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/thing.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/thing.py @@ -1,8 +1,11 @@ # Copyright (c) Microsoft Corporation. All rights reserved. # Licensed under the MIT License. +from typing import Literal + from ..agents_model import AgentsModel from .._type_aliases import NonEmptyString +from .entity_types import EntityTypes class Thing(AgentsModel): @@ -14,5 +17,5 @@ class Thing(AgentsModel): :type name: str """ - type: NonEmptyString = None + type: Literal[EntityTypes.THING] = EntityTypes.THING name: NonEmptyString = None diff --git a/libraries/microsoft-agents-hosting-aiohttp/microsoft_agents/hosting/aiohttp/jwt_authorization_middleware.py b/libraries/microsoft-agents-hosting-aiohttp/microsoft_agents/hosting/aiohttp/jwt_authorization_middleware.py index d28618cd..0a37c49c 100644 --- a/libraries/microsoft-agents-hosting-aiohttp/microsoft_agents/hosting/aiohttp/jwt_authorization_middleware.py +++ b/libraries/microsoft-agents-hosting-aiohttp/microsoft_agents/hosting/aiohttp/jwt_authorization_middleware.py @@ -31,6 +31,8 @@ async def jwt_authorization_middleware(request: Request, handler): {"error": "Authorization header not found"}, status=401 ) + c = await request.json() + return await handler(request) diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/turn_context.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/turn_context.py index f39e8428..a3320b4b 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/turn_context.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/turn_context.py @@ -18,6 +18,7 @@ ResourceResponse, DeliveryModes, ) +from microsoft_agents.activity.entity.entity_types import EntityTypes from microsoft_agents.hosting.core.authorization.claims_identity import ClaimsIdentity @@ -428,7 +429,7 @@ def get_mentions(activity: Activity) -> list[Mention]: result: list[Mention] = [] if activity.entities is not None: for entity in activity.entities: - if entity.type.lower() == "mention": + if entity.type.lower() == EntityTypes.MENTION: result.append(entity) return result diff --git a/tests/activity/pydantic/test_activity_io.py b/tests/activity/pydantic/test_activity_io.py index 72d6f95d..f9a24f8b 100644 --- a/tests/activity/pydantic/test_activity_io.py +++ b/tests/activity/pydantic/test_activity_io.py @@ -75,9 +75,9 @@ def test_channel_id_validate_without_product_info(self): "type": "message", "channel_id": "msteams:subchannel", "entities": [ - {"type": "productInfo", "id": "other"}, - {"type": "productInfo", "id": "other"}, - {"type": "productInfo", "id": "wow"}, + {"type": "ProductInfo", "id": "other"}, + {"type": "ProductInfo", "id": "other"}, + {"type": "ProductInfo", "id": "wow"}, ], }, Activity( diff --git a/tests/activity/test_channel_id.py b/tests/activity/test_channel_id.py index 389b2b47..80f2795f 100644 --- a/tests/activity/test_channel_id.py +++ b/tests/activity/test_channel_id.py @@ -96,3 +96,8 @@ def test_round_trip(self): def test_channel_id_validation_error(self): with pytest.raises(ValidationError): ChannelId(channel="") + + def test_multiple_colons(self): + a = ChannelId.model_validate("email:support:extra") + assert a.channel == "email" + assert a.sub_channel == "support:extra" From 3da2c64fd22df8ea0f2a1545f7073ecac8b5724d Mon Sep 17 00:00:00 2001 From: Rodrigo Brandao Date: Fri, 10 Oct 2025 09:54:08 -0700 Subject: [PATCH 12/22] Addressing edge case --- .../microsoft_agents/activity/activity.py | 11 +- .../activity/conversation_reference.py | 71 ++++++- tests/activity/pydantic/test_activity_io.py | 80 +++++++- .../test_conversation_reference_io.py | 185 ++++++++++++++++++ 4 files changed, 332 insertions(+), 15 deletions(-) create mode 100644 tests/activity/pydantic/test_conversation_reference_io.py diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py index 1a8d40ce..22cca247 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py @@ -210,7 +210,7 @@ def channel_id(self, value: Any): @model_validator(mode="wrap") @classmethod def _validate_sub_channel_data( - self, data: Any, handler: ModelWrapValidatorHandler[Activity] + cls, data: Any, handler: ModelWrapValidatorHandler[Activity] ) -> Activity: """Validate the Activity, ensuring consistency between channel_id.sub_channel and productInfo entity. @@ -223,7 +223,7 @@ def _validate_sub_channel_data( activity = handler(data) # needed to assign to a computed field - data_channel_id = data.get("channel_id", None) + data_channel_id = data.get("channel_id", data.get("channelId", None)) if data_channel_id: activity.channel_id = data_channel_id @@ -270,12 +270,15 @@ def _serialize_sub_channel_data( ) elif product_info: # remove productInfo entity if sub_channel is not set del serialized["entities"][i] - if not serialized["entities"]: + if not serialized["entities"]: # after removal above, list may be empty del serialized["entities"] # do not include unset value if not self.channel_id: - del serialized["channelId"] + if "channelId" in serialized: + del serialized["channelId"] + elif "channel_id" in serialized: + del serialized["channel_id"] return serialized diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/conversation_reference.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/conversation_reference.py index 98426860..cefc9825 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/conversation_reference.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/conversation_reference.py @@ -1,10 +1,13 @@ # Copyright (c) Microsoft Corporation. All rights reserved. # Licensed under the MIT License. +from __future__ import annotations + from uuid import uuid4 as uuid -from typing import Optional +from typing import Optional, Any +import logging -from pydantic import Field +from pydantic import Field, computed_field, model_validator, ModelWrapValidatorHandler, model_serializer, SerializerFunctionWrapHandler, ValidationError from .channel_account import ChannelAccount from .channel_id import ChannelId @@ -14,6 +17,8 @@ from .activity_types import ActivityTypes from .activity_event_names import ActivityEventNames +logger = logging.getLogger(__name__) + class ConversationReference(AgentsModel): """An object relating to a particular point in a conversation. @@ -44,10 +49,70 @@ class ConversationReference(AgentsModel): user: Optional[ChannelAccount] = None agent: ChannelAccount = Field(None, alias="bot") conversation: ConversationAccount - channel_id: ChannelId + _channel_id: ChannelId = None locale: Optional[NonEmptyString] = None service_url: NonEmptyString = None + # required to define the setter below + @computed_field(return_type=Optional[ChannelId]) + @property + def channel_id(self): + """Gets the _channel_id field""" + return self._channel_id + + # necessary for backward compatibility + # previously, channel_id was directly assigned with strings + @channel_id.setter + def channel_id(self, value: Any): + """Sets the channel_id after validating it as a ChannelId model.""" + self._channel_id = ChannelId.model_validate(value) + + @model_validator(mode="wrap") + @classmethod + def _validate_sub_channel_data( + cls, data: Any, handler: ModelWrapValidatorHandler[ConversationReference] + ) -> ConversationReference: + """Validate the ConversationReference, ensuring consistency between channel_id.sub_channel and productInfo entity. + + :param data: The input data to validate. + :param handler: The validation handler provided by Pydantic. + :return: The validated ConversationReference instance. + """ + try: + # run Pydantic's standard validation first + conversation_reference = handler(data) + + # needed to assign to a computed field + data_channel_id = data.get("channel_id", data.get("channelId")) + if data_channel_id: + conversation_reference.channel_id = data_channel_id + + return conversation_reference + except ValidationError: + logger.error("Validation error for ConversationReference") + raise + + @model_serializer(mode="wrap") + def _serialize_sub_channel_data( + self, handler: SerializerFunctionWrapHandler + ) -> dict[str, object]: + """Serialize the ConversationReference, ensuring consistency between channel_id.sub_channel and productInfo entity. + + :param handler: The serialization handler provided by Pydantic. + :return: A dictionary representing the serialized ConversationReference. + """ + # run Pydantic's standard serialization first + serialized = handler(self) + + # do not include unset value + if not self.channel_id: + if "channelId" in serialized: + del serialized["channelId"] + elif "channel_id" in serialized: + del serialized["channel_id"] + + return serialized + def get_continuation_activity(self) -> "Activity": # type: ignore from .activity import Activity diff --git a/tests/activity/pydantic/test_activity_io.py b/tests/activity/pydantic/test_activity_io.py index f9a24f8b..423ca79f 100644 --- a/tests/activity/pydantic/test_activity_io.py +++ b/tests/activity/pydantic/test_activity_io.py @@ -1,4 +1,3 @@ -from turtle import mode import pytest from pydantic import ValidationError @@ -9,10 +8,10 @@ Entity, EntityTypes, ProductInfo, + ConversationReference, + ConversationAccount ) -from tests import activity - # validation / serialization tests class TestActivityIO: @@ -68,7 +67,7 @@ def test_channel_id_validate_without_product_info(self): assert not activity.get_product_info_entity() @pytest.mark.parametrize( - "data, expected", + "data, data_with_alias, expected", [ [ { @@ -80,6 +79,15 @@ def test_channel_id_validate_without_product_info(self): {"type": "ProductInfo", "id": "wow"}, ], }, + { + "type": "message", + "channelId": "msteams:subchannel", + "entities": [ + {"type": "ProductInfo", "id": "other"}, + {"type": "ProductInfo", "id": "other"}, + {"type": "ProductInfo", "id": "wow"}, + ], + }, Activity( type="message", channel_id="msteams:subchannel", @@ -96,6 +104,11 @@ def test_channel_id_validate_without_product_info(self): "channel_id": "parent:misc", "entities": [{"type": "some_entity"}], }, + { + "type": "message", + "channelId": "parent:misc", + "entities": [{"type": "some_entity"}], + }, Activity( type="message", channel_id="parent:misc", @@ -107,18 +120,37 @@ def test_channel_id_validate_without_product_info(self): "type": "message", "channel_id": "parent:misc", "entities": [ProductInfo(id="sub_channel")], + "conversation_referenece": { + "channelId": "parent:misc", + "conversation": {"id": "conv1"}, + } + }, + { + "type": "message", + "channelId": "parent:misc", + "entities": [ProductInfo(id="sub_channel")], + "conversation_referenece": { + "channelId": "parent:misc", + "conversation": {"id": "conv1"}, + } }, Activity( type="message", channel_id="parent:sub_channel", entities=[ProductInfo(id="sub_channel")], + conversation_reference=ConversationReference( + channel_id="parent:sub_channel", + conversation=ConversationAccount(id="conv1") + ) ), ], ], ) - def test_channel_id_sub_channel_changed_with_product_info(self, data, expected): + def test_channel_id_sub_channel_changed_with_product_info(self, data, data_with_alias, expected): activity = Activity(**data) + activity_from_alias = Activity(**data_with_alias) assert activity == expected + assert activity_from_alias == expected def test_channel_id_unset_becomes_set_at_init(self): activity = Activity(type="message") @@ -136,12 +168,13 @@ def test_product_info_avoids_error_no_parent_channel(self): assert activity.channel_id is None @pytest.mark.parametrize( - "activity, expected", + "activity, expected, expected_no_alias", [ - [Activity(type="message"), {"type": "message"}], + [Activity(type="message"), {"type": "message"}, {"type": "message"}], [ Activity(type="message", channel_id="msteams"), {"type": "message", "channelId": "msteams"}, + {"type": "message", "channel_id": "msteams"}, ], [ Activity(type="message", channel_id="msteams:subchannel"), @@ -152,6 +185,13 @@ def test_product_info_avoids_error_no_parent_channel(self): {"type": str(EntityTypes.PRODUCT_INFO), "id": "subchannel"} ], }, + { + "type": "message", + "channel_id": "msteams:subchannel", + "entities": [ + {"type": str(EntityTypes.PRODUCT_INFO), "id": "subchannel"} + ], + }, ], [ Activity( @@ -167,6 +207,14 @@ def test_product_info_avoids_error_no_parent_channel(self): {"type": str(EntityTypes.PRODUCT_INFO), "id": "subchannel"}, ], }, + { + "type": "message", + "channel_id": "msteams:subchannel", + "entities": [ + {"type": "other"}, + {"type": str(EntityTypes.PRODUCT_INFO), "id": "subchannel"}, + ], + }, ], [ Activity( @@ -182,16 +230,32 @@ def test_product_info_avoids_error_no_parent_channel(self): {"type": str(EntityTypes.PRODUCT_INFO), "id": "misc"}, ], }, + { + "type": "message", + "channel_id": "msteams:misc", + "entities": [ + {"type": "other"}, + {"type": str(EntityTypes.PRODUCT_INFO), "id": "misc"}, + ], + }, ], [ Activity(type="message", entities=[ProductInfo(id="misc")]), {"type": "message"}, + {"type": "message"} ], ], ) - def test_serialize(self, activity, expected): + def test_serialize(self, activity, expected, expected_no_alias): data = activity.model_dump(mode="json", exclude_unset=True, by_alias=True) + data_no_alias = activity.model_dump(exclude_unset=True, by_alias=False) assert data == expected + assert data_no_alias == expected_no_alias + + def test_model_dump(self): + activity = Activity(type="message") + data = activity.model_dump(exclude_unset=True) + assert data == {"type": "message"} def test_serialize_misconfiguration_no_sub_channel(self): activity = Activity( diff --git a/tests/activity/pydantic/test_conversation_reference_io.py b/tests/activity/pydantic/test_conversation_reference_io.py new file mode 100644 index 00000000..fead0b17 --- /dev/null +++ b/tests/activity/pydantic/test_conversation_reference_io.py @@ -0,0 +1,185 @@ +import pytest + +from microsoft_agents.activity import ( + ChannelId, + ConversationReference, + ConversationAccount +) + +DATA = [ + [ + ConversationReference( + channel_id="parent:sub_channel", + conversation=ConversationAccount(id="conv1") + ), + { + "channel_id": "parent:sub_channel", + "conversation": {"id": "conv1"}, + }, + ], + [ + ConversationReference( + channel_id=ChannelId(channel="parent", sub_channel="sub_channel"), + conversation=ConversationAccount(id="conv1") + ), + { + "channel_id": "parent:sub_channel", + "conversation": {"id": "conv1"}, + } + ], + [ + ConversationReference( + conversation=ConversationAccount(id="conv1") + ), + { + "conversation": {"id": "conv1"}, + } + ] +] + +DATA_WITH_ALIAS = [ + [ + ConversationReference( + channel_id="parent:sub_channel", + conversation=ConversationAccount(id="conv1") + ), + { + "channelId": "parent:sub_channel", + "conversation": {"id": "conv1"}, + }, + ], + [ + ConversationReference( + channel_id=ChannelId(channel="parent", sub_channel="sub_channel"), + conversation=ConversationAccount(id="conv1") + ), + { + "channelId": "parent:sub_channel", + "conversation": {"id": "conv1"}, + } + ], + [ + ConversationReference( + conversation=ConversationAccount(id="conv1") + ), + { + "conversation": {"id": "conv1"}, + } + ] +] + +VALIDATION_ERR_DATA_FROM_JSON = [ + { + "channel_id": "parent:sub_channel", + }, + { + "channel_id": "parent:sub_channel", + "conversation": {}, + }, + { + "channel_id": "parent:sub_channel", + "conversation": {"id": ""}, + }, + { + "channel_id": "parent:sub_channel", + "conversation": {"id": None}, + }, + { + "channel_id": { "sub_channel": "sub_channel" }, + "conversation": {"id": "conv1"}, + }, + { + "channelId": "parent:sub_channel", + }, + { + "channelId": "parent:sub_channel", + "conversation": {}, + }, + { + "channelId": "parent:sub_channel", + "conversation": {"id": ""}, + }, + { + "channelId": "parent:sub_channel", + "conversation": {"id": None}, + }, + { + "channelId": { "sub_channel": "sub_channel" }, + "conversation": {"id": "conv1"}, + } +] + +VALIDATION_ERR_DATA_FROM_KWARGS = [ + { + "channel_id": "parent:sub_channel", + }, + { + "channel_id": "parent:sub_channel", + "conversation": {}, + }, + { + "channel_id": "parent:sub_channel", + "conversation": {"id": ""}, + }, + { + "channel_id": "parent:sub_channel", + "conversation": {"id": None}, + }, + { + "channel_id": None, + "conversation": {"id": "conv1"}, + }, + { + "channel_id": "", + "conversation": {"id": "conv1"}, + }, + { + "channel_id": { "sub_channel": "sub_channel" }, + "conversation": {"id": "conv1"}, + }, + { + "channelId": "parent:sub_channel", + }, + { + "channelId": "parent:sub_channel", + "conversation": {}, + }, + { + "channelId": "parent:sub_channel", + "conversation": {"id": ""}, + }, + { + "channelId": "parent:sub_channel", + "conversation": {"id": None}, + }, + { + "channelId": None, + "conversation": {"id": "conv1"}, + }, + { + "channelId": "", + "conversation": {"id": "conv1"}, + }, + { + "channelId": { "sub_channel": "sub_channel" }, + "conversation": {"id": "conv1"}, + } +] + +@pytest.mark.parametrize("instance, expected", DATA) +def test_conversation_reference_io(instance, expected): + if "channel_id" in expected: + assert instance.model_dump(exclude_unset=True) == expected + else: + assert instance.model_dump(exclude_unset=True, by_alias=True) == expected + assert ConversationReference.model_validate(expected) == instance + +@pytest.mark.parametrize("data", VALIDATION_ERR_DATA_FROM_JSON) +def test_conversation_reference_validation_error(data): + with pytest.raises(Exception): + ConversationReference.model_validate(data) + +@pytest.mark.parametrize("data", VALIDATION_ERR_DATA_FROM_KWARGS) +def test_conversation_reference_validation_error_kwargs(data): + with pytest.raises(Exception): + ConversationReference.model_validate(**data) \ No newline at end of file From c361f9d78cf5ae2ba36c109478cbdf8daed5d48d Mon Sep 17 00:00:00 2001 From: Rodrigo Brandao Date: Fri, 10 Oct 2025 10:12:59 -0700 Subject: [PATCH 13/22] Completed fix for serializing a None --- .../microsoft_agents/activity/activity.py | 5 +- .../activity/conversation_reference.py | 33 ++++++++----- tests/activity/pydantic/test_activity_io.py | 17 ++++--- .../test_conversation_reference_io.py | 49 +++++++++---------- tests/hosting_core/_oauth/test_oauth_flow.py | 7 ++- 5 files changed, 65 insertions(+), 46 deletions(-) diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py index 22cca247..0cd1fa00 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py @@ -246,6 +246,9 @@ def _serialize_sub_channel_data( :param handler: The serialization handler provided by Pydantic. :return: A dictionary representing the serialized Activity. """ + if self is None: + raise Exception("THIS SHOULD NEVER HAPPEN") + # run Pydantic's standard serialization first serialized = handler(self) @@ -270,7 +273,7 @@ def _serialize_sub_channel_data( ) elif product_info: # remove productInfo entity if sub_channel is not set del serialized["entities"][i] - if not serialized["entities"]: # after removal above, list may be empty + if not serialized["entities"]: # after removal above, list may be empty del serialized["entities"] # do not include unset value diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/conversation_reference.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/conversation_reference.py index cefc9825..fe0171fb 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/conversation_reference.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/conversation_reference.py @@ -7,7 +7,15 @@ from typing import Optional, Any import logging -from pydantic import Field, computed_field, model_validator, ModelWrapValidatorHandler, model_serializer, SerializerFunctionWrapHandler, ValidationError +from pydantic import ( + Field, + computed_field, + model_validator, + ModelWrapValidatorHandler, + model_serializer, + SerializerFunctionWrapHandler, + ValidationError, +) from .channel_account import ChannelAccount from .channel_id import ChannelId @@ -82,10 +90,11 @@ def _validate_sub_channel_data( # run Pydantic's standard validation first conversation_reference = handler(data) - # needed to assign to a computed field - data_channel_id = data.get("channel_id", data.get("channelId")) - if data_channel_id: - conversation_reference.channel_id = data_channel_id + if isinstance(data, dict): + # needed to assign to a computed field + data_channel_id = data.get("channel_id", data.get("channelId")) + if data_channel_id: + conversation_reference.channel_id = data_channel_id return conversation_reference except ValidationError: @@ -101,15 +110,17 @@ def _serialize_sub_channel_data( :param handler: The serialization handler provided by Pydantic. :return: A dictionary representing the serialized ConversationReference. """ + # run Pydantic's standard serialization first serialized = handler(self) - # do not include unset value - if not self.channel_id: - if "channelId" in serialized: - del serialized["channelId"] - elif "channel_id" in serialized: - del serialized["channel_id"] + if serialized: + # do not include unset value + if not self.channel_id: + if "channelId" in serialized: + del serialized["channelId"] + elif "channel_id" in serialized: + del serialized["channel_id"] return serialized diff --git a/tests/activity/pydantic/test_activity_io.py b/tests/activity/pydantic/test_activity_io.py index 423ca79f..669f43ed 100644 --- a/tests/activity/pydantic/test_activity_io.py +++ b/tests/activity/pydantic/test_activity_io.py @@ -9,7 +9,7 @@ EntityTypes, ProductInfo, ConversationReference, - ConversationAccount + ConversationAccount, ) @@ -123,7 +123,7 @@ def test_channel_id_validate_without_product_info(self): "conversation_referenece": { "channelId": "parent:misc", "conversation": {"id": "conv1"}, - } + }, }, { "type": "message", @@ -132,7 +132,7 @@ def test_channel_id_validate_without_product_info(self): "conversation_referenece": { "channelId": "parent:misc", "conversation": {"id": "conv1"}, - } + }, }, Activity( type="message", @@ -140,17 +140,20 @@ def test_channel_id_validate_without_product_info(self): entities=[ProductInfo(id="sub_channel")], conversation_reference=ConversationReference( channel_id="parent:sub_channel", - conversation=ConversationAccount(id="conv1") - ) + conversation=ConversationAccount(id="conv1"), + ), ), ], ], ) - def test_channel_id_sub_channel_changed_with_product_info(self, data, data_with_alias, expected): + def test_channel_id_sub_channel_changed_with_product_info( + self, data, data_with_alias, expected + ): activity = Activity(**data) activity_from_alias = Activity(**data_with_alias) assert activity == expected assert activity_from_alias == expected + assert activity.model_copy() == activity_from_alias.model_copy() def test_channel_id_unset_becomes_set_at_init(self): activity = Activity(type="message") @@ -242,7 +245,7 @@ def test_product_info_avoids_error_no_parent_channel(self): [ Activity(type="message", entities=[ProductInfo(id="misc")]), {"type": "message"}, - {"type": "message"} + {"type": "message"}, ], ], ) diff --git a/tests/activity/pydantic/test_conversation_reference_io.py b/tests/activity/pydantic/test_conversation_reference_io.py index fead0b17..63144fcc 100644 --- a/tests/activity/pydantic/test_conversation_reference_io.py +++ b/tests/activity/pydantic/test_conversation_reference_io.py @@ -3,14 +3,14 @@ from microsoft_agents.activity import ( ChannelId, ConversationReference, - ConversationAccount + ConversationAccount, ) DATA = [ [ ConversationReference( channel_id="parent:sub_channel", - conversation=ConversationAccount(id="conv1") + conversation=ConversationAccount(id="conv1"), ), { "channel_id": "parent:sub_channel", @@ -20,28 +20,26 @@ [ ConversationReference( channel_id=ChannelId(channel="parent", sub_channel="sub_channel"), - conversation=ConversationAccount(id="conv1") + conversation=ConversationAccount(id="conv1"), ), { "channel_id": "parent:sub_channel", "conversation": {"id": "conv1"}, - } + }, ], [ - ConversationReference( - conversation=ConversationAccount(id="conv1") - ), + ConversationReference(conversation=ConversationAccount(id="conv1")), { "conversation": {"id": "conv1"}, - } - ] + }, + ], ] DATA_WITH_ALIAS = [ [ ConversationReference( channel_id="parent:sub_channel", - conversation=ConversationAccount(id="conv1") + conversation=ConversationAccount(id="conv1"), ), { "channelId": "parent:sub_channel", @@ -51,21 +49,19 @@ [ ConversationReference( channel_id=ChannelId(channel="parent", sub_channel="sub_channel"), - conversation=ConversationAccount(id="conv1") + conversation=ConversationAccount(id="conv1"), ), { "channelId": "parent:sub_channel", "conversation": {"id": "conv1"}, - } + }, ], [ - ConversationReference( - conversation=ConversationAccount(id="conv1") - ), + ConversationReference(conversation=ConversationAccount(id="conv1")), { "conversation": {"id": "conv1"}, - } - ] + }, + ], ] VALIDATION_ERR_DATA_FROM_JSON = [ @@ -85,7 +81,7 @@ "conversation": {"id": None}, }, { - "channel_id": { "sub_channel": "sub_channel" }, + "channel_id": {"sub_channel": "sub_channel"}, "conversation": {"id": "conv1"}, }, { @@ -104,9 +100,9 @@ "conversation": {"id": None}, }, { - "channelId": { "sub_channel": "sub_channel" }, + "channelId": {"sub_channel": "sub_channel"}, "conversation": {"id": "conv1"}, - } + }, ] VALIDATION_ERR_DATA_FROM_KWARGS = [ @@ -134,10 +130,10 @@ "conversation": {"id": "conv1"}, }, { - "channel_id": { "sub_channel": "sub_channel" }, + "channel_id": {"sub_channel": "sub_channel"}, "conversation": {"id": "conv1"}, }, - { + { "channelId": "parent:sub_channel", }, { @@ -161,11 +157,12 @@ "conversation": {"id": "conv1"}, }, { - "channelId": { "sub_channel": "sub_channel" }, + "channelId": {"sub_channel": "sub_channel"}, "conversation": {"id": "conv1"}, - } + }, ] + @pytest.mark.parametrize("instance, expected", DATA) def test_conversation_reference_io(instance, expected): if "channel_id" in expected: @@ -174,12 +171,14 @@ def test_conversation_reference_io(instance, expected): assert instance.model_dump(exclude_unset=True, by_alias=True) == expected assert ConversationReference.model_validate(expected) == instance + @pytest.mark.parametrize("data", VALIDATION_ERR_DATA_FROM_JSON) def test_conversation_reference_validation_error(data): with pytest.raises(Exception): ConversationReference.model_validate(data) + @pytest.mark.parametrize("data", VALIDATION_ERR_DATA_FROM_KWARGS) def test_conversation_reference_validation_error_kwargs(data): with pytest.raises(Exception): - ConversationReference.model_validate(**data) \ No newline at end of file + ConversationReference.model_validate(**data) diff --git a/tests/hosting_core/_oauth/test_oauth_flow.py b/tests/hosting_core/_oauth/test_oauth_flow.py index f92c09b5..fd08f5a3 100644 --- a/tests/hosting_core/_oauth/test_oauth_flow.py +++ b/tests/hosting_core/_oauth/test_oauth_flow.py @@ -33,10 +33,13 @@ def testing_Activity( text="a", ): # mock_conversation_ref = mocker.MagicMock(ConversationReference) + conversation_reference = ConversationReference( + conversation={"id": "conv1"}, + ) mocker.patch.object( Activity, "get_conversation_reference", - return_value=mocker.MagicMock(ConversationReference), + return_value=conversation_reference, ) return Activity( type=type, @@ -44,7 +47,7 @@ def testing_Activity( from_property=ChannelAccount(id=DEFAULTS.user_id), channel_id=DEFAULTS.channel_id, # get_conversation_reference=mocker.Mock(return_value=conv_ref), - relates_to=mocker.MagicMock(ConversationReference), + relates_to=conversation_reference, value=value, text=text, ) From affcdd74b6f2b30ff21cf3b859c32346425b9179 Mon Sep 17 00:00:00 2001 From: Rodrigo Brandao Date: Fri, 10 Oct 2025 15:02:16 -0700 Subject: [PATCH 14/22] Refactoring to make ChannelId a subclass of str --- .../microsoft_agents/activity/__init__.py | 2 + .../activity/_channel_id_field_mixin.py | 79 ++++++++ .../microsoft_agents/activity/activity.py | 43 ++-- .../microsoft_agents/activity/channel_id.py | 99 +++++----- .../activity/conversation_reference.py | 80 +------- .../oauth/_handlers/_user_authorization.py | 2 +- .../pydantic/test_channel_id_field_mixin.py | 80 ++++++++ .../test_conversation_reference_io.py | 184 ------------------ tests/activity/test_channel_id.py | 142 +++++--------- 9 files changed, 283 insertions(+), 428 deletions(-) create mode 100644 libraries/microsoft-agents-activity/microsoft_agents/activity/_channel_id_field_mixin.py create mode 100644 tests/activity/pydantic/test_channel_id_field_mixin.py delete mode 100644 tests/activity/pydantic/test_conversation_reference_io.py diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/__init__.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/__init__.py index 24ecaff2..d467a123 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/__init__.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/__init__.py @@ -17,6 +17,7 @@ from .card_image import CardImage from .channels import Channels from .channel_account import ChannelAccount +from ._channel_id_field_mixin import _ChannelIdFieldMixin from .channel_id import ChannelId from .conversation_account import ConversationAccount from .conversation_members import ConversationMembers @@ -119,6 +120,7 @@ "Channels", "ChannelAccount", "ChannelId", + "_ChannelIdFieldMixin", "ConversationAccount", "ConversationMembers", "ConversationParameters", diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/_channel_id_field_mixin.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/_channel_id_field_mixin.py new file mode 100644 index 00000000..48d90954 --- /dev/null +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/_channel_id_field_mixin.py @@ -0,0 +1,79 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + +import logging +from typing import Optional, Any, Self + +from pydantic import ( + ModelWrapValidatorHandler, + SerializerFunctionWrapHandler, + computed_field, + model_validator, + model_serializer +) + +from .channel_id import ChannelId + +logger = logging.getLogger(__name__) + +class _ChannelIdFieldMixin: + """A mixin to add a computed field channel_id of type ChannelId to a Pydantic model.""" + + _channel_id: Optional[ChannelId] = None + + # required to define the setter below + @computed_field(return_type=Optional[ChannelId], alias="channelId") + @property + def channel_id(self) -> Optional[ChannelId]: + """Gets the _channel_id field""" + return self._channel_id + + # necessary for backward compatibility + # previously, channel_id was directly assigned with strings + @channel_id.setter + def channel_id(self, value: Any): + """Sets the channel_id after validating it as a ChannelId model.""" + if isinstance(value, ChannelId): + self._channel_id = value + elif isinstance(value, str): + self._channel_id = ChannelId(value) + else: + raise ValueError( + f"Invalid type for channel_id: {type(value)}. " + "Expected ChannelId or str." + ) + + @model_validator(mode="wrap") + @classmethod + def _validate_channel_id(cls, data: Any, handler: ModelWrapValidatorHandler) -> Self: + """Validate the _channel_id field after model initialization. + + :return: The model instance itself. + """ + try: + model = handler(data) + if "channelId" in data: + model.channel_id = data["channelId"] + elif "channel_id" in data: + model.channel_id = data["channel_id"] + return model + except Exception: + logging.error("Model %s failed to validate with data %s", cls, data) + raise + + @model_serializer(mode="wrap") + def _serialize_channel_id( + self, handler: SerializerFunctionWrapHandler + ) -> dict[str, object]: + """Serialize the model using Pydantic's standard serialization. + + :param handler: The serialization handler provided by Pydantic. + :return: A dictionary representing the serialized model. + """ + serialized = handler(self) + if not self._channel_id: # do not include unset value + if "channelId" in serialized: + del serialized["channelId"] + elif "channel_id" in serialized: + del serialized["channel_id"] + return serialized \ No newline at end of file diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py index 0cd1fa00..0befb697 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py @@ -41,6 +41,7 @@ from .semantic_action import SemanticAction from .agents_model import AgentsModel from .role_types import RoleTypes +from ._channel_id_field_mixin import _ChannelIdFieldMixin from .channel_id import ChannelId from ._model_utils import pick_model, SkipNone from ._type_aliases import NonEmptyString @@ -49,7 +50,7 @@ # TODO: A2A Agent 2 is responding with None as id, had to mark it as optional (investigate) -class Activity(AgentsModel): +class Activity(AgentsModel, _ChannelIdFieldMixin): """An Activity is the basic communication type for the protocol. :param type: Contains the activity type. Possible values include: @@ -156,7 +157,6 @@ class Activity(AgentsModel): local_timestamp: datetime = None local_timezone: NonEmptyString = None service_url: NonEmptyString = None - _channel_id: ChannelId = None from_property: ChannelAccount = Field(None, alias="from") conversation: ConversationAccount = None recipient: ChannelAccount = None @@ -193,23 +193,12 @@ class Activity(AgentsModel): semantic_action: SemanticAction = None caller_id: NonEmptyString = None - # required to define the setter below - @computed_field(return_type=Optional[ChannelId]) - @property - def channel_id(self): - """Gets the _channel_id field""" - return self._channel_id - - # necessary for backward compatibility - # previously, channel_id was directly assigned with strings - @channel_id.setter - def channel_id(self, value: Any): - """Sets the channel_id after validating it as a ChannelId model.""" - self._channel_id = ChannelId.model_validate(value) + # annotated as Optional but cannot be set to None in practice + # _channel_id: Optional[ChannelId] = None # inherited from _ChannelIdFieldMixin @model_validator(mode="wrap") @classmethod - def _validate_sub_channel_data( + def _validate_channel_id( cls, data: Any, handler: ModelWrapValidatorHandler[Activity] ) -> Activity: """Validate the Activity, ensuring consistency between channel_id.sub_channel and productInfo entity. @@ -223,14 +212,19 @@ def _validate_sub_channel_data( activity = handler(data) # needed to assign to a computed field - data_channel_id = data.get("channel_id", data.get("channelId", None)) - if data_channel_id: - activity.channel_id = data_channel_id + # needed because we override the mixin validator + if "channelId" in data: + activity.channel_id = data["channelId"] + elif "channel_id" in data: + activity.channel_id = data["channel_id"] # sync sub_channel with productInfo entity product_info = activity.get_product_info_entity() if product_info and activity.channel_id: - activity.channel_id.sub_channel = product_info.id + activity.channel_id = ChannelId( + channel=activity.channel_id.channel, + sub_channel=product_info.id, + ) return activity except ValidationError: @@ -246,8 +240,6 @@ def _serialize_sub_channel_data( :param handler: The serialization handler provided by Pydantic. :return: A dictionary representing the serialized Activity. """ - if self is None: - raise Exception("THIS SHOULD NEVER HAPPEN") # run Pydantic's standard serialization first serialized = handler(self) @@ -276,13 +268,6 @@ def _serialize_sub_channel_data( if not serialized["entities"]: # after removal above, list may be empty del serialized["entities"] - # do not include unset value - if not self.channel_id: - if "channelId" in serialized: - del serialized["channelId"] - elif "channel_id" in serialized: - del serialized["channel_id"] - return serialized def apply_conversation_reference( diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py index 430ebca7..b1eb35fb 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py @@ -3,54 +3,61 @@ from __future__ import annotations +from token import OP from typing import Optional, Any -from pydantic import model_validator, model_serializer +from pydantic_core import CoreSchema, core_schema +from pydantic import GetCoreSchemaHandler, BaseModel -from ._type_aliases import NonEmptyString -from .agents_model import AgentsModel +class ChannelId(str): + """A ChannelId represents a channel and optional sub-channel in the format 'channel:sub_channel'.""" - -class ChannelId(AgentsModel): - """A class representing a channel identifier with optional sub-channel. - - :param channel: The main channel identifier (e.g., "msteams"). - :type channel: str - :param sub_channel: An optional sub-channel identifier (e.g., "subchannel"). - :type sub_channel: Optional[str] - """ - - channel: NonEmptyString - sub_channel: Optional[str] = None - - @model_validator(mode="before") - @classmethod - def _split_channel_ids(cls, data: Any) -> Any: - """Validator to split a string into channel and sub_channel if needed.""" - - if isinstance(data, str) and data: - split = data.strip().split(":", 1) - return { - "channel": split[0].strip(), - "sub_channel": split[1].strip() if len(split) == 2 else None, - } - elif isinstance(data, dict) and data: - # let Pydantic handle the dict case - return data + def __init__(self, value: Optional[str] = None, channel: Optional[str] = None, sub_channel: Optional[str] = None): + super().__init__() + if not channel: + split = self.strip().split(":", 1) + self._channel = split[0].strip() + self._sub_channel = split[1].strip() if len(split) == 2 else None else: - raise ValueError("Invalid data type for ChannelId") - - @model_serializer(mode="plain") - def _serialize_plain(self) -> str: - return str(self) - - def __eq__(self, other: Any) -> bool: - return str(self) == str(other) - - def __hash__(self) -> int: - return hash(str(self)) - - def __str__(self) -> str: - if self.sub_channel: - return f"{self.channel}:{self.sub_channel}" - return self.channel + self._channel = channel + self._sub_channel = sub_channel + + def __new__( + cls, + value: Optional[str] = None, + channel: Optional[str] = None, + sub_channel: Optional[str] = None + ) -> ChannelId: + """Create a new ChannelId instance.""" + if isinstance(value, str): + value = value.strip() + if value: + return str.__new__(cls, value) + raise TypeError("value must be a non empty string if provided") + else: + if not isinstance(channel, str) or len(channel.strip()) == 0 or ":" in channel: + raise TypeError("channel must be a non empty string, and must not contain the ':' character") + if sub_channel is not None and (not isinstance(sub_channel, str)): + raise TypeError("sub_channel must be a string if provided") + channel = channel.strip() + sub_channel = sub_channel.strip() if sub_channel else None + if sub_channel: + return str.__new__(cls, f"{channel}:{sub_channel}") + return str.__new__(cls, channel) + + @property + def channel(self) -> str: + """The main channel, e.g. 'email' in 'email:work'.""" + return self._channel # type: ignore[return-value] + + @property + def sub_channel(self) -> Optional[str]: + """The sub-channel, e.g. 'work' in 'email:work'. May be None.""" + return self._sub_channel + + # https://docs.pydantic.dev/dev/concepts/types/#customizing-validation-with-__get_pydantic_core_schema__ + @classmethod + def __get_pydantic_core_schema__( + cls, source_type: Any, handler: GetCoreSchemaHandler + ) -> CoreSchema: + return core_schema.no_info_after_validator_function(cls, handler(str)) \ No newline at end of file diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/conversation_reference.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/conversation_reference.py index fe0171fb..49ffe9fd 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/conversation_reference.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/conversation_reference.py @@ -4,20 +4,13 @@ from __future__ import annotations from uuid import uuid4 as uuid -from typing import Optional, Any +from typing import Optional import logging -from pydantic import ( - Field, - computed_field, - model_validator, - ModelWrapValidatorHandler, - model_serializer, - SerializerFunctionWrapHandler, - ValidationError, -) +from pydantic import Field from .channel_account import ChannelAccount +from ._channel_id_field_mixin import _ChannelIdFieldMixin from .channel_id import ChannelId from .conversation_account import ConversationAccount from .agents_model import AgentsModel @@ -28,7 +21,7 @@ logger = logging.getLogger(__name__) -class ConversationReference(AgentsModel): +class ConversationReference(AgentsModel, _ChannelIdFieldMixin): """An object relating to a particular point in a conversation. :param activity_id: (Optional) ID of the activity to refer to @@ -57,72 +50,9 @@ class ConversationReference(AgentsModel): user: Optional[ChannelAccount] = None agent: ChannelAccount = Field(None, alias="bot") conversation: ConversationAccount - _channel_id: ChannelId = None locale: Optional[NonEmptyString] = None service_url: NonEmptyString = None - - # required to define the setter below - @computed_field(return_type=Optional[ChannelId]) - @property - def channel_id(self): - """Gets the _channel_id field""" - return self._channel_id - - # necessary for backward compatibility - # previously, channel_id was directly assigned with strings - @channel_id.setter - def channel_id(self, value: Any): - """Sets the channel_id after validating it as a ChannelId model.""" - self._channel_id = ChannelId.model_validate(value) - - @model_validator(mode="wrap") - @classmethod - def _validate_sub_channel_data( - cls, data: Any, handler: ModelWrapValidatorHandler[ConversationReference] - ) -> ConversationReference: - """Validate the ConversationReference, ensuring consistency between channel_id.sub_channel and productInfo entity. - - :param data: The input data to validate. - :param handler: The validation handler provided by Pydantic. - :return: The validated ConversationReference instance. - """ - try: - # run Pydantic's standard validation first - conversation_reference = handler(data) - - if isinstance(data, dict): - # needed to assign to a computed field - data_channel_id = data.get("channel_id", data.get("channelId")) - if data_channel_id: - conversation_reference.channel_id = data_channel_id - - return conversation_reference - except ValidationError: - logger.error("Validation error for ConversationReference") - raise - - @model_serializer(mode="wrap") - def _serialize_sub_channel_data( - self, handler: SerializerFunctionWrapHandler - ) -> dict[str, object]: - """Serialize the ConversationReference, ensuring consistency between channel_id.sub_channel and productInfo entity. - - :param handler: The serialization handler provided by Pydantic. - :return: A dictionary representing the serialized ConversationReference. - """ - - # run Pydantic's standard serialization first - serialized = handler(self) - - if serialized: - # do not include unset value - if not self.channel_id: - if "channelId" in serialized: - del serialized["channelId"] - elif "channel_id" in serialized: - del serialized["channel_id"] - - return serialized + # _channel_id: ChannelId = None # inherited from _ChannelIdFieldMixin def get_continuation_activity(self) -> "Activity": # type: ignore from .activity import Activity diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/oauth/_handlers/_user_authorization.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/oauth/_handlers/_user_authorization.py index 902b0dd4..b7f52c3d 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/oauth/_handlers/_user_authorization.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/oauth/_handlers/_user_authorization.py @@ -66,7 +66,7 @@ async def _load_flow( ): raise ValueError("Channel ID and User ID are required") - channel_id = context.activity.channel_id + channel_id = str(context.activity.channel_id) user_id = context.activity.from_property.id ms_app_id = context.turn_state.get(context.adapter.AGENT_IDENTITY_KEY).claims[ diff --git a/tests/activity/pydantic/test_channel_id_field_mixin.py b/tests/activity/pydantic/test_channel_id_field_mixin.py new file mode 100644 index 00000000..620fb039 --- /dev/null +++ b/tests/activity/pydantic/test_channel_id_field_mixin.py @@ -0,0 +1,80 @@ +import pytest + +from typing import Optional +from pydantic import BaseModel, ValidationError + +from microsoft_agents.activity import ChannelId, _ChannelIdFieldMixin + +class DummyModel(BaseModel, _ChannelIdFieldMixin): + ... + +def channel_id_eq(a: Optional[ChannelId], b: Optional[ChannelId]) -> bool: + return a.channel == b.channel and a.sub_channel == b.sub_channel and a == b + +class TestChannelIdFieldMixin: + + def test_validation_basic(self): + model = DummyModel(channel_id="email:support") + assert channel_id_eq(model.channel_id, ChannelId("email:support")) + model = DummyModel(channel_id="email") + assert channel_id_eq(model.channel_id, ChannelId("email")) + model = DummyModel(channel_id="channel:sub_channel:extra") + assert channel_id_eq(model.channel_id, ChannelId("channel:sub_channel:extra")) + + def test_validation_from_channel_id(self): + model = DummyModel(channel_id=ChannelId("email:support")) + assert channel_id_eq(model.channel_id, ChannelId("email:support")) + + def test_validation_dict(self): + model = DummyModel.model_validate({"channelId": "email:support"}) + assert channel_id_eq(model.channel_id, ChannelId("email:support")) + + def test_validation_dict_camel_case(self): + model = DummyModel.model_validate({"channel_id": "email:support"}) + assert channel_id_eq(model.channel_id, ChannelId("email:support")) + + def test_validation_none(self): + model = DummyModel.model_validate({}) + assert model.channel_id is None + + def test_validation_invalid_type(self): + with pytest.raises(ValidationError): + DummyModel.model_validate({"channelId": 123}) + with pytest.raises(ValidationError): + DummyModel.model_validate({"channel_id": 123}) + with pytest.raises(ValidationError): + DummyModel.model_validate({"channelId": None}) + with pytest.raises(ValidationError): + DummyModel(channel_id=123) + + def test_serialize(self): + model = DummyModel(channel_id="email:support") + assert model.model_dump() == {"channel_id": "email:support"} + assert model.model_dump_json() == '{"channel_id":"email:support"}' + assert model.model_dump(by_alias=True) == {"channelId": "email:support"} + assert model.model_dump_json(by_alias=True) == '{"channelId":"email:support"}' + assert model.model_dump(exclude_unset=True) == {"channel_id": "email:support"} + + def test_serialize_none(self): + model = DummyModel() + assert model.model_dump() == {} + assert model.model_dump_json() == '{}' + assert model.model_dump(by_alias=True) == {} + assert model.model_dump_json(by_alias=True) == '{}' + assert model.model_dump(exclude_unset=True) == {} + + def test_set(self): + model = DummyModel() + assert model.channel_id is None + model.channel_id = "email:support" + assert channel_id_eq(model.channel_id, ChannelId("email:support")) + model.channel_id = "a:b:c" + assert channel_id_eq(model.channel_id, ChannelId("a:b:c")) + model.channel_id = ChannelId("email") + assert channel_id_eq(model.channel_id, ChannelId("email")) + with pytest.raises(Exception): + model.channel_id = 123 + with pytest.raises(Exception): + model.channel_id = "" + with pytest.raises(Exception): + model.channel_id = None \ No newline at end of file diff --git a/tests/activity/pydantic/test_conversation_reference_io.py b/tests/activity/pydantic/test_conversation_reference_io.py deleted file mode 100644 index 63144fcc..00000000 --- a/tests/activity/pydantic/test_conversation_reference_io.py +++ /dev/null @@ -1,184 +0,0 @@ -import pytest - -from microsoft_agents.activity import ( - ChannelId, - ConversationReference, - ConversationAccount, -) - -DATA = [ - [ - ConversationReference( - channel_id="parent:sub_channel", - conversation=ConversationAccount(id="conv1"), - ), - { - "channel_id": "parent:sub_channel", - "conversation": {"id": "conv1"}, - }, - ], - [ - ConversationReference( - channel_id=ChannelId(channel="parent", sub_channel="sub_channel"), - conversation=ConversationAccount(id="conv1"), - ), - { - "channel_id": "parent:sub_channel", - "conversation": {"id": "conv1"}, - }, - ], - [ - ConversationReference(conversation=ConversationAccount(id="conv1")), - { - "conversation": {"id": "conv1"}, - }, - ], -] - -DATA_WITH_ALIAS = [ - [ - ConversationReference( - channel_id="parent:sub_channel", - conversation=ConversationAccount(id="conv1"), - ), - { - "channelId": "parent:sub_channel", - "conversation": {"id": "conv1"}, - }, - ], - [ - ConversationReference( - channel_id=ChannelId(channel="parent", sub_channel="sub_channel"), - conversation=ConversationAccount(id="conv1"), - ), - { - "channelId": "parent:sub_channel", - "conversation": {"id": "conv1"}, - }, - ], - [ - ConversationReference(conversation=ConversationAccount(id="conv1")), - { - "conversation": {"id": "conv1"}, - }, - ], -] - -VALIDATION_ERR_DATA_FROM_JSON = [ - { - "channel_id": "parent:sub_channel", - }, - { - "channel_id": "parent:sub_channel", - "conversation": {}, - }, - { - "channel_id": "parent:sub_channel", - "conversation": {"id": ""}, - }, - { - "channel_id": "parent:sub_channel", - "conversation": {"id": None}, - }, - { - "channel_id": {"sub_channel": "sub_channel"}, - "conversation": {"id": "conv1"}, - }, - { - "channelId": "parent:sub_channel", - }, - { - "channelId": "parent:sub_channel", - "conversation": {}, - }, - { - "channelId": "parent:sub_channel", - "conversation": {"id": ""}, - }, - { - "channelId": "parent:sub_channel", - "conversation": {"id": None}, - }, - { - "channelId": {"sub_channel": "sub_channel"}, - "conversation": {"id": "conv1"}, - }, -] - -VALIDATION_ERR_DATA_FROM_KWARGS = [ - { - "channel_id": "parent:sub_channel", - }, - { - "channel_id": "parent:sub_channel", - "conversation": {}, - }, - { - "channel_id": "parent:sub_channel", - "conversation": {"id": ""}, - }, - { - "channel_id": "parent:sub_channel", - "conversation": {"id": None}, - }, - { - "channel_id": None, - "conversation": {"id": "conv1"}, - }, - { - "channel_id": "", - "conversation": {"id": "conv1"}, - }, - { - "channel_id": {"sub_channel": "sub_channel"}, - "conversation": {"id": "conv1"}, - }, - { - "channelId": "parent:sub_channel", - }, - { - "channelId": "parent:sub_channel", - "conversation": {}, - }, - { - "channelId": "parent:sub_channel", - "conversation": {"id": ""}, - }, - { - "channelId": "parent:sub_channel", - "conversation": {"id": None}, - }, - { - "channelId": None, - "conversation": {"id": "conv1"}, - }, - { - "channelId": "", - "conversation": {"id": "conv1"}, - }, - { - "channelId": {"sub_channel": "sub_channel"}, - "conversation": {"id": "conv1"}, - }, -] - - -@pytest.mark.parametrize("instance, expected", DATA) -def test_conversation_reference_io(instance, expected): - if "channel_id" in expected: - assert instance.model_dump(exclude_unset=True) == expected - else: - assert instance.model_dump(exclude_unset=True, by_alias=True) == expected - assert ConversationReference.model_validate(expected) == instance - - -@pytest.mark.parametrize("data", VALIDATION_ERR_DATA_FROM_JSON) -def test_conversation_reference_validation_error(data): - with pytest.raises(Exception): - ConversationReference.model_validate(data) - - -@pytest.mark.parametrize("data", VALIDATION_ERR_DATA_FROM_KWARGS) -def test_conversation_reference_validation_error_kwargs(data): - with pytest.raises(Exception): - ConversationReference.model_validate(**data) diff --git a/tests/activity/test_channel_id.py b/tests/activity/test_channel_id.py index 80f2795f..68287f67 100644 --- a/tests/activity/test_channel_id.py +++ b/tests/activity/test_channel_id.py @@ -1,5 +1,4 @@ import pytest -from pydantic import ValidationError from microsoft_agents.activity import ChannelId @@ -7,97 +6,54 @@ DEFAULTS = TEST_DEFAULTS() - class TestChannelId: - @pytest.mark.parametrize( - "data, expected", - [ - ["email:support", ChannelId(channel="email", sub_channel="support")], - ["email", ChannelId(channel="email", sub_channel=None)], - [" email:support ", ChannelId(channel="email", sub_channel="support")], - [" email ", ChannelId(channel="email", sub_channel=None)], - [ - "email:support:extra", - ChannelId(channel="email", sub_channel="support:extra"), - ], - [ - {"channel": "email", "sub_channel": "support"}, - ChannelId(channel="email", sub_channel="support"), - ], - ], - ) - def test_validation_dict_form(self, data, expected): - channel_id = ChannelId.model_validate(data) - assert channel_id == expected - - @pytest.mark.parametrize( - "channel_a, channel_b, expected", - [ - [ - ChannelId(channel="email", sub_channel="support"), - ChannelId(channel="email", sub_channel="support"), - True, - ], - [ - ChannelId(channel="email", sub_channel="support"), - ChannelId(channel="word", sub_channel="support"), - False, - ], - [ - ChannelId(channel="email", sub_channel="support"), - ChannelId(channel="email", sub_channel="info"), - False, - ], - [ - ChannelId(channel="email", sub_channel=None), - ChannelId(channel="email", sub_channel=None), - True, - ], - [ - ChannelId(channel="email", sub_channel=None), - ChannelId(channel="email", sub_channel="support"), - False, - ], - [ - ChannelId(channel="email", sub_channel=None), - ChannelId(channel="sms", sub_channel=None), - False, - ], - ], - ) - def test_eq(self, channel_a, channel_b, expected): - assert (channel_a == channel_b) == expected - assert (channel_a == channel_a) == True - assert (channel_b == channel_b) == True - - def test_hash(self): - a = ChannelId(channel="email", sub_channel="SUPPORT") - assert hash(a) == hash(str(a)) - - def test_str(self): - a = ChannelId(channel="email", sub_channel="support") - assert str(a) == "email:support" - b = ChannelId(channel="email", sub_channel=None) - assert str(b) == "email" - c = ChannelId(**{"channel": "agents", "sub_channel": "word"}) - assert str(c) == "agents:word" - d = ChannelId.model_validate("agents:COPILOT") - assert str(d) == "agents:COPILOT" - - def test_round_trip(self): - a = ChannelId(channel="email", sub_channel="support") - b = ChannelId.model_validate(str(a)) - assert a == b - c = ChannelId.model_validate(b.model_dump()) - assert a == c - assert b == c - - def test_channel_id_validation_error(self): - with pytest.raises(ValidationError): - ChannelId(channel="") - - def test_multiple_colons(self): - a = ChannelId.model_validate("email:support:extra") - assert a.channel == "email" - assert a.sub_channel == "support:extra" + def test_init_from_str(self): + channel_id = ChannelId("email:support") + assert channel_id.channel == "email" + assert channel_id.sub_channel == "support" + assert str(channel_id) == "email:support" + assert channel_id == "email:support" + assert channel_id in [ "email:support", "other" ] + assert channel_id not in [ "email:other", "other" ] + assert channel_id != "email:other" + assert channel_id in [ "wow", ChannelId("email:support") ] + assert channel_id == ChannelId("email:support") + + def test_init_multiple_colons(self): + assert ChannelId("email:support:extra").channel == "email" + assert ChannelId("email:support:extra").sub_channel == "support:extra" + + def test_init_multiple_args(self): + assert ChannelId("email:support", channel="a", sub_channel="b") == "email:support" + + def test_init_from_parts(self): + channel_id = ChannelId(channel="email", sub_channel="support") + assert channel_id.channel == "email" + assert channel_id.sub_channel == "support" + assert str(channel_id) == "email:support" + + channel_id2 = ChannelId(channel="email") + assert channel_id2.channel == "email" + assert channel_id2.sub_channel is None + assert str(channel_id2) == "email" + + def test_init_errors(self): + with pytest.raises(Exception): + ChannelId(channel="email", sub_channel=123) + with pytest.raises(Exception): + ChannelId(channel="", sub_channel="support") + with pytest.raises(Exception): + ChannelId("") + with pytest.raises(Exception): + ChannelId() + with pytest.raises(Exception): + ChannelId(channel=None) + with pytest.raises(Exception): + ChannelId(sub_channel="sub_channel") + with pytest.raises(Exception): + ChannelId(" \t\n ") + with pytest.raises(Exception): + ChannelId("", channel=" ", sub_channel="support") + with pytest.raises(Exception): + ChannelId(channel="a:b", sub_channel="support") \ No newline at end of file From 2f2894bd7e92242776382c6eaf37f4a972a4f521 Mon Sep 17 00:00:00 2001 From: Rodrigo Brandao Date: Mon, 13 Oct 2025 09:52:04 -0700 Subject: [PATCH 15/22] Updated implementation details --- .../activity/_channel_id_field_mixin.py | 39 +++++++++++++------ .../microsoft_agents/activity/activity.py | 13 ++++--- .../microsoft_agents/activity/channel_id.py | 36 +++++++++++------ tests/activity/pydantic/test_activity_io.py | 9 ++--- .../pydantic/test_channel_id_field_mixin.py | 12 +++--- tests/activity/test_channel_id.py | 15 ++++--- 6 files changed, 78 insertions(+), 46 deletions(-) diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/_channel_id_field_mixin.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/_channel_id_field_mixin.py index 48d90954..48286eee 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/_channel_id_field_mixin.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/_channel_id_field_mixin.py @@ -9,13 +9,15 @@ SerializerFunctionWrapHandler, computed_field, model_validator, - model_serializer + model_serializer, ) from .channel_id import ChannelId logger = logging.getLogger(__name__) + +# can be generalized in the future, if needed class _ChannelIdFieldMixin: """A mixin to add a computed field channel_id of type ChannelId to a Pydantic model.""" @@ -43,24 +45,40 @@ def channel_id(self, value: Any): "Expected ChannelId or str." ) + def _set_validated_channel_id(self, data: Any) -> None: + """Sets the channel_id after validating it as a ChannelId model.""" + if "channelId" in data: + self.channel_id = data["channelId"] + elif "channel_id" in data: + self.channel_id = data["channel_id"] + @model_validator(mode="wrap") @classmethod - def _validate_channel_id(cls, data: Any, handler: ModelWrapValidatorHandler) -> Self: + def _validate_channel_id( + cls, data: Any, handler: ModelWrapValidatorHandler + ) -> Self: """Validate the _channel_id field after model initialization. :return: The model instance itself. """ try: model = handler(data) - if "channelId" in data: - model.channel_id = data["channelId"] - elif "channel_id" in data: - model.channel_id = data["channel_id"] + model._set_validated_channel_id(data) return model except Exception: logging.error("Model %s failed to validate with data %s", cls, data) raise + def _remove_serialized_unset_channel_id( + self, serialized: dict[str, object] + ) -> None: + """Remove the _channel_id field if it is not set.""" + if not self._channel_id: + if "channelId" in serialized: + del serialized["channelId"] + elif "channel_id" in serialized: + del serialized["channel_id"] + @model_serializer(mode="wrap") def _serialize_channel_id( self, handler: SerializerFunctionWrapHandler @@ -71,9 +89,6 @@ def _serialize_channel_id( :return: A dictionary representing the serialized model. """ serialized = handler(self) - if not self._channel_id: # do not include unset value - if "channelId" in serialized: - del serialized["channelId"] - elif "channel_id" in serialized: - del serialized["channel_id"] - return serialized \ No newline at end of file + if self: # serialization can be called with None + self._remove_serialized_unset_channel_id(serialized) + return serialized diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py index 0befb697..b6059870 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py @@ -213,10 +213,7 @@ def _validate_channel_id( # needed to assign to a computed field # needed because we override the mixin validator - if "channelId" in data: - activity.channel_id = data["channelId"] - elif "channel_id" in data: - activity.channel_id = data["channel_id"] + activity._set_validated_channel_id(data) # sync sub_channel with productInfo entity product_info = activity.get_product_info_entity() @@ -243,15 +240,18 @@ def _serialize_sub_channel_data( # run Pydantic's standard serialization first serialized = handler(self) + if not self: # serialization can be called with None + return serialized + # find the ProductInfo entity product_info = None for i, entity in enumerate(serialized.get("entities") or []): if entity.get("type", "") == EntityTypes.PRODUCT_INFO: product_info = entity break + # maintain consistency between ProductInfo entity and sub channel if self.channel_id and self.channel_id.sub_channel: - # maintain consistency between productInfo entity and sub channel if product_info: product_info["id"] = self.channel_id.sub_channel else: @@ -268,6 +268,9 @@ def _serialize_sub_channel_data( if not serialized["entities"]: # after removal above, list may be empty del serialized["entities"] + # necessary due to computed_field serialization + self._remove_serialized_unset_channel_id(serialized) + return serialized def apply_conversation_reference( diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py index b1eb35fb..f771bc87 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py @@ -9,10 +9,16 @@ from pydantic_core import CoreSchema, core_schema from pydantic import GetCoreSchemaHandler, BaseModel + class ChannelId(str): """A ChannelId represents a channel and optional sub-channel in the format 'channel:sub_channel'.""" - def __init__(self, value: Optional[str] = None, channel: Optional[str] = None, sub_channel: Optional[str] = None): + def __init__( + self, + value: Optional[str] = None, + channel: Optional[str] = None, + sub_channel: Optional[str] = None, + ): super().__init__() if not channel: split = self.strip().split(":", 1) @@ -23,11 +29,11 @@ def __init__(self, value: Optional[str] = None, channel: Optional[str] = None, s self._sub_channel = sub_channel def __new__( - cls, - value: Optional[str] = None, - channel: Optional[str] = None, - sub_channel: Optional[str] = None - ) -> ChannelId: + cls, + value: Optional[str] = None, + channel: Optional[str] = None, + sub_channel: Optional[str] = None, + ) -> ChannelId: """Create a new ChannelId instance.""" if isinstance(value, str): value = value.strip() @@ -35,8 +41,14 @@ def __new__( return str.__new__(cls, value) raise TypeError("value must be a non empty string if provided") else: - if not isinstance(channel, str) or len(channel.strip()) == 0 or ":" in channel: - raise TypeError("channel must be a non empty string, and must not contain the ':' character") + if ( + not isinstance(channel, str) + or len(channel.strip()) == 0 + or ":" in channel + ): + raise TypeError( + "channel must be a non empty string, and must not contain the ':' character" + ) if sub_channel is not None and (not isinstance(sub_channel, str)): raise TypeError("sub_channel must be a string if provided") channel = channel.strip() @@ -48,16 +60,16 @@ def __new__( @property def channel(self) -> str: """The main channel, e.g. 'email' in 'email:work'.""" - return self._channel # type: ignore[return-value] - + return self._channel # type: ignore[return-value] + @property def sub_channel(self) -> Optional[str]: """The sub-channel, e.g. 'work' in 'email:work'. May be None.""" return self._sub_channel - + # https://docs.pydantic.dev/dev/concepts/types/#customizing-validation-with-__get_pydantic_core_schema__ @classmethod def __get_pydantic_core_schema__( cls, source_type: Any, handler: GetCoreSchemaHandler ) -> CoreSchema: - return core_schema.no_info_after_validator_function(cls, handler(str)) \ No newline at end of file + return core_schema.no_info_after_validator_function(cls, handler(str)) diff --git a/tests/activity/pydantic/test_activity_io.py b/tests/activity/pydantic/test_activity_io.py index 669f43ed..b7af0116 100644 --- a/tests/activity/pydantic/test_activity_io.py +++ b/tests/activity/pydantic/test_activity_io.py @@ -37,11 +37,6 @@ def test_serialize_basic(self): ChannelId(channel="msteams", sub_channel="subchannel"), ), (ChannelId(channel="msteams"), ChannelId(channel="msteams")), - ( - {"channel": "msteams", "sub_channel": "subchannel"}, - ChannelId(channel="msteams", sub_channel="subchannel"), - ), - ({"channel": "msteams"}, ChannelId(channel="msteams")), ], ) def test_channel_id_setter_validation(self, data, expected): @@ -55,8 +50,10 @@ def test_channel_id_setter_validation(self, data, expected): def test_channel_id_setter_validation_error(self): activity = Activity(type="message") - with pytest.raises(ValidationError): + with pytest.raises(Exception): activity.channel_id = {} + with pytest.raises(Exception): + activity.channel_id = 123 def test_channel_id_validate_without_product_info(self): data = {"type": "message", "channel_id": "msteams:subchannel"} diff --git a/tests/activity/pydantic/test_channel_id_field_mixin.py b/tests/activity/pydantic/test_channel_id_field_mixin.py index 620fb039..ab51b056 100644 --- a/tests/activity/pydantic/test_channel_id_field_mixin.py +++ b/tests/activity/pydantic/test_channel_id_field_mixin.py @@ -5,12 +5,14 @@ from microsoft_agents.activity import ChannelId, _ChannelIdFieldMixin -class DummyModel(BaseModel, _ChannelIdFieldMixin): - ... + +class DummyModel(BaseModel, _ChannelIdFieldMixin): ... + def channel_id_eq(a: Optional[ChannelId], b: Optional[ChannelId]) -> bool: return a.channel == b.channel and a.sub_channel == b.sub_channel and a == b + class TestChannelIdFieldMixin: def test_validation_basic(self): @@ -58,9 +60,9 @@ def test_serialize(self): def test_serialize_none(self): model = DummyModel() assert model.model_dump() == {} - assert model.model_dump_json() == '{}' + assert model.model_dump_json() == "{}" assert model.model_dump(by_alias=True) == {} - assert model.model_dump_json(by_alias=True) == '{}' + assert model.model_dump_json(by_alias=True) == "{}" assert model.model_dump(exclude_unset=True) == {} def test_set(self): @@ -77,4 +79,4 @@ def test_set(self): with pytest.raises(Exception): model.channel_id = "" with pytest.raises(Exception): - model.channel_id = None \ No newline at end of file + model.channel_id = None diff --git a/tests/activity/test_channel_id.py b/tests/activity/test_channel_id.py index 68287f67..cfec3a55 100644 --- a/tests/activity/test_channel_id.py +++ b/tests/activity/test_channel_id.py @@ -6,6 +6,7 @@ DEFAULTS = TEST_DEFAULTS() + class TestChannelId: def test_init_from_str(self): @@ -14,18 +15,20 @@ def test_init_from_str(self): assert channel_id.sub_channel == "support" assert str(channel_id) == "email:support" assert channel_id == "email:support" - assert channel_id in [ "email:support", "other" ] - assert channel_id not in [ "email:other", "other" ] + assert channel_id in ["email:support", "other"] + assert channel_id not in ["email:other", "other"] assert channel_id != "email:other" - assert channel_id in [ "wow", ChannelId("email:support") ] + assert channel_id in ["wow", ChannelId("email:support")] assert channel_id == ChannelId("email:support") - + def test_init_multiple_colons(self): assert ChannelId("email:support:extra").channel == "email" assert ChannelId("email:support:extra").sub_channel == "support:extra" def test_init_multiple_args(self): - assert ChannelId("email:support", channel="a", sub_channel="b") == "email:support" + assert ( + ChannelId("email:support", channel="a", sub_channel="b") == "email:support" + ) def test_init_from_parts(self): channel_id = ChannelId(channel="email", sub_channel="support") @@ -56,4 +59,4 @@ def test_init_errors(self): with pytest.raises(Exception): ChannelId("", channel=" ", sub_channel="support") with pytest.raises(Exception): - ChannelId(channel="a:b", sub_channel="support") \ No newline at end of file + ChannelId(channel="a:b", sub_channel="support") From bc799869b6fe5aa37463e3e3ba08990230d26ed3 Mon Sep 17 00:00:00 2001 From: Rodrigo Brandao Date: Mon, 13 Oct 2025 10:01:38 -0700 Subject: [PATCH 16/22] Removing Self import from typing --- .../microsoft_agents/activity/_channel_id_field_mixin.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/_channel_id_field_mixin.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/_channel_id_field_mixin.py index 48286eee..77f60c02 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/_channel_id_field_mixin.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/_channel_id_field_mixin.py @@ -1,8 +1,10 @@ # Copyright (c) Microsoft Corporation. All rights reserved. # Licensed under the MIT License. +from __future__ import annotations + import logging -from typing import Optional, Any, Self +from typing import Optional, Any from pydantic import ( ModelWrapValidatorHandler, @@ -56,7 +58,7 @@ def _set_validated_channel_id(self, data: Any) -> None: @classmethod def _validate_channel_id( cls, data: Any, handler: ModelWrapValidatorHandler - ) -> Self: + ) -> _ChannelIdFieldMixin: """Validate the _channel_id field after model initialization. :return: The model instance itself. From c39c5262be43b3099847f68c72ef332d7be5ad3a Mon Sep 17 00:00:00 2001 From: Rodrigo Brandao Date: Mon, 13 Oct 2025 11:15:57 -0700 Subject: [PATCH 17/22] Addressing PR comments --- .../microsoft_agents/activity/activity.py | 9 +++--- .../microsoft_agents/activity/channel_id.py | 28 ++++++++++++++++--- .../aiohttp/jwt_authorization_middleware.py | 2 -- tests/activity/test_channel_id.py | 5 ++-- 4 files changed, 31 insertions(+), 13 deletions(-) diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py index b6059870..92551983 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py @@ -6,7 +6,7 @@ import logging from copy import copy from datetime import datetime, timezone -from typing import Optional, Union, Any +from typing import Optional, Any from pydantic import ( Field, @@ -18,7 +18,6 @@ computed_field, ValidationError, ) -from yaml import serialize from .activity_types import ActivityTypes from .channel_account import ChannelAccount @@ -224,8 +223,8 @@ def _validate_channel_id( ) return activity - except ValidationError: - logger.error("Validation error for Activity") + except ValidationError as exc: + logger.error("Validation error for Activity: %s", exc, exc_info=True) raise @model_serializer(mode="wrap") @@ -635,6 +634,8 @@ def get_product_info_entity(self) -> Optional[ProductInfo]: if not self.entities: return None target = EntityTypes.PRODUCT_INFO.lower() + # validated entities can be Entity, and that prevents us from + # making assumptions about the casing of the 'type' attribute return next(filter(lambda e: e.type.lower() == target, self.entities), None) def get_mentions(self) -> list[Mention]: diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py index f771bc87..52298175 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py @@ -3,11 +3,10 @@ from __future__ import annotations -from token import OP from typing import Optional, Any from pydantic_core import CoreSchema, core_schema -from pydantic import GetCoreSchemaHandler, BaseModel +from pydantic import GetCoreSchemaHandler class ChannelId(str): @@ -16,9 +15,17 @@ class ChannelId(str): def __init__( self, value: Optional[str] = None, + *, channel: Optional[str] = None, sub_channel: Optional[str] = None, - ): + ) -> None: + """Initialize a ChannelId instance. + + :param value: The full channel ID string in the format 'channel:sub_channel'. Must be provided if channel is not provided. + :param channel: The main channel string. Must be provided if value is not provided. + :param sub_channel: The sub-channel string. + :raises ValueError: If the input parameters are invalid. value and channel cannot both be provided. + """ super().__init__() if not channel: split = self.strip().split(":", 1) @@ -31,11 +38,24 @@ def __init__( def __new__( cls, value: Optional[str] = None, + *, channel: Optional[str] = None, sub_channel: Optional[str] = None, ) -> ChannelId: - """Create a new ChannelId instance.""" + """Create a new ChannelId instance. + + :param value: The full channel ID string in the format 'channel:sub_channel'. Must be provided if channel is not provided. + :param channel: The main channel string. Must be provided if value is not provided. + :param sub_channel: The sub-channel string. + :return: A new ChannelId instance. + :raises ValueError: If the input parameters are invalid. value and channel cannot both be provided. + """ if isinstance(value, str): + if channel or sub_channel: + raise ValueError( + "If value is provided, channel and sub_channel must be None" + ) + value = value.strip() if value: return str.__new__(cls, value) diff --git a/libraries/microsoft-agents-hosting-aiohttp/microsoft_agents/hosting/aiohttp/jwt_authorization_middleware.py b/libraries/microsoft-agents-hosting-aiohttp/microsoft_agents/hosting/aiohttp/jwt_authorization_middleware.py index 0a37c49c..d28618cd 100644 --- a/libraries/microsoft-agents-hosting-aiohttp/microsoft_agents/hosting/aiohttp/jwt_authorization_middleware.py +++ b/libraries/microsoft-agents-hosting-aiohttp/microsoft_agents/hosting/aiohttp/jwt_authorization_middleware.py @@ -31,8 +31,6 @@ async def jwt_authorization_middleware(request: Request, handler): {"error": "Authorization header not found"}, status=401 ) - c = await request.json() - return await handler(request) diff --git a/tests/activity/test_channel_id.py b/tests/activity/test_channel_id.py index cfec3a55..ef592243 100644 --- a/tests/activity/test_channel_id.py +++ b/tests/activity/test_channel_id.py @@ -26,9 +26,8 @@ def test_init_multiple_colons(self): assert ChannelId("email:support:extra").sub_channel == "support:extra" def test_init_multiple_args(self): - assert ( - ChannelId("email:support", channel="a", sub_channel="b") == "email:support" - ) + with pytest.raises(ValueError): + ChannelId("email:support", channel="a", sub_channel="b") def test_init_from_parts(self): channel_id = ChannelId(channel="email", sub_channel="support") From d4516ff55b67226141ad916735b5008156455a8a Mon Sep 17 00:00:00 2001 From: Rodrigo Brandao Date: Mon, 13 Oct 2025 12:07:20 -0700 Subject: [PATCH 18/22] Addressing PR review and making entities subclass from Entity --- .../microsoft_agents/activity/channel_id.py | 2 +- .../microsoft_agents/activity/entity/geo_coordinates.py | 4 ++-- .../microsoft_agents/activity/entity/place.py | 4 ++-- .../microsoft_agents/activity/entity/thing.py | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py index 52298175..ad4890fe 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/channel_id.py @@ -45,7 +45,7 @@ def __new__( """Create a new ChannelId instance. :param value: The full channel ID string in the format 'channel:sub_channel'. Must be provided if channel is not provided. - :param channel: The main channel string. Must be provided if value is not provided. + :param channel: The main channel string. Must be provided if value is not provided. Must not contain ':', as it delimits channels and sub channels. :param sub_channel: The sub-channel string. :return: A new ChannelId instance. :raises ValueError: If the input parameters are invalid. value and channel cannot both be provided. diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/geo_coordinates.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/geo_coordinates.py index 0880cadd..1f758a5c 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/geo_coordinates.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/geo_coordinates.py @@ -3,12 +3,12 @@ from typing import Literal -from ..agents_model import AgentsModel from .._type_aliases import NonEmptyString +from .entity import Entity from .entity_types import EntityTypes -class GeoCoordinates(AgentsModel): +class GeoCoordinates(Entity): """GeoCoordinates (entity type: "https://schema.org/GeoCoordinates"). :param elevation: Elevation of the location [WGS diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/place.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/place.py index 1aba1f2f..fa179d91 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/place.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/place.py @@ -3,12 +3,12 @@ from typing import Literal -from ..agents_model import AgentsModel from .._type_aliases import NonEmptyString +from .entity import Entity from .entity_types import EntityTypes -class Place(AgentsModel): +class Place(Entity): """Place (entity type: "https://schema.org/Place"). :param address: Address of the place (may be `string` or complex object of diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/thing.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/thing.py index a05672ab..73d28de4 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/thing.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/thing.py @@ -3,12 +3,12 @@ from typing import Literal -from ..agents_model import AgentsModel from .._type_aliases import NonEmptyString +from .entity import Entity from .entity_types import EntityTypes -class Thing(AgentsModel): +class Thing(Entity): """Thing (entity type: "https://schema.org/Thing"). :param type: The type of the thing From f508ae86bcb1b22dc6f9b3eeb11ff85950b094dc Mon Sep 17 00:00:00 2001 From: Rodrigo Brandao Date: Mon, 13 Oct 2025 13:20:58 -0700 Subject: [PATCH 19/22] Raising exceptions when ProductInfo and channel_id.sub_channel conflict --- .../microsoft_agents/activity/activity.py | 15 +++- tests/activity/pydantic/test_activity_io.py | 89 +++++++------------ 2 files changed, 42 insertions(+), 62 deletions(-) diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py index 92551983..8dd4d1d3 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py @@ -217,6 +217,13 @@ def _validate_channel_id( # sync sub_channel with productInfo entity product_info = activity.get_product_info_entity() if product_info and activity.channel_id: + if ( + activity.channel_id.sub_channel + and activity.channel_id.sub_channel != product_info.id + ): + raise Exception( + "Conflict between channel_id.sub_channel and productInfo entity" + ) activity.channel_id = ChannelId( channel=activity.channel_id.channel, sub_channel=product_info.id, @@ -251,9 +258,11 @@ def _serialize_sub_channel_data( # maintain consistency between ProductInfo entity and sub channel if self.channel_id and self.channel_id.sub_channel: - if product_info: - product_info["id"] = self.channel_id.sub_channel - else: + if product_info and product_info.get("id") != self.channel_id.sub_channel: + raise Exception( + "Conflict between channel_id.sub_channel and productInfo entity" + ) + elif not product_info: if not serialized.get("entities"): serialized["entities"] = [] serialized["entities"].append( diff --git a/tests/activity/pydantic/test_activity_io.py b/tests/activity/pydantic/test_activity_io.py index b7af0116..6af9b186 100644 --- a/tests/activity/pydantic/test_activity_io.py +++ b/tests/activity/pydantic/test_activity_io.py @@ -66,35 +66,6 @@ def test_channel_id_validate_without_product_info(self): @pytest.mark.parametrize( "data, data_with_alias, expected", [ - [ - { - "type": "message", - "channel_id": "msteams:subchannel", - "entities": [ - {"type": "ProductInfo", "id": "other"}, - {"type": "ProductInfo", "id": "other"}, - {"type": "ProductInfo", "id": "wow"}, - ], - }, - { - "type": "message", - "channelId": "msteams:subchannel", - "entities": [ - {"type": "ProductInfo", "id": "other"}, - {"type": "ProductInfo", "id": "other"}, - {"type": "ProductInfo", "id": "wow"}, - ], - }, - Activity( - type="message", - channel_id="msteams:subchannel", - entities=[ - Entity(type=EntityTypes.PRODUCT_INFO, id="other"), - Entity(type=EntityTypes.PRODUCT_INFO, id="other"), - Entity(type=EntityTypes.PRODUCT_INFO, id="wow"), - ], - ), - ], [ { "type": "message", @@ -115,30 +86,27 @@ def test_channel_id_validate_without_product_info(self): [ { "type": "message", - "channel_id": "parent:misc", - "entities": [ProductInfo(id="sub_channel")], - "conversation_referenece": { - "channelId": "parent:misc", - "conversation": {"id": "conv1"}, - }, + "channel_id": "parent", + "entities": [ + {"type": "some_entity"}, + {"type": EntityTypes.PRODUCT_INFO, "id": "misc"}, + ], }, { "type": "message", - "channelId": "parent:misc", - "entities": [ProductInfo(id="sub_channel")], - "conversation_referenece": { - "channelId": "parent:misc", - "conversation": {"id": "conv1"}, - }, + "channelId": "parent", + "entities": [ + {"type": "some_entity"}, + {"type": EntityTypes.PRODUCT_INFO, "id": "misc"}, + ], }, Activity( type="message", - channel_id="parent:sub_channel", - entities=[ProductInfo(id="sub_channel")], - conversation_reference=ConversationReference( - channel_id="parent:sub_channel", - conversation=ConversationAccount(id="conv1"), - ), + channel_id="parent:misc", + entities=[ + Entity(type="some_entity"), + Entity(type=EntityTypes.PRODUCT_INFO, id="misc"), + ], ), ], ], @@ -152,6 +120,14 @@ def test_channel_id_sub_channel_changed_with_product_info( assert activity_from_alias == expected assert activity.model_copy() == activity_from_alias.model_copy() + def test_channel_id_sub_channel_conflict_on_validation(self): + with pytest.raises(Exception): + activity = Activity( + type="message", + channel_id="parent:misc", + entities=[Entity(type="some_type"), ProductInfo(id="sub_channel")], + ) + def test_channel_id_unset_becomes_set_at_init(self): activity = Activity(type="message") activity.channel_id = "channel:sub_channel" @@ -272,18 +248,13 @@ def test_serialize_misconfiguration_no_sub_channel(self): ], } - def test_serialize_misconfiguration_wrong_sub_channel(self): + def test_serialize_sub_channel_conflict(self): activity = Activity( - type="message", channel_id="msteams:misc", entities=[{"type": "other"}] + type="message", + channel_id="msteams:subchannel", + entities=[{"type": "other"}], ) - activity.entities.append(ProductInfo(id="sub_channel")) + activity.entities.append(ProductInfo(id="other_sub_channel")) - data = activity.model_dump(mode="json", exclude_unset=True, by_alias=True) - assert data == { - "type": "message", - "channelId": "msteams:misc", - "entities": [ - {"type": "other"}, - {"type": str(EntityTypes.PRODUCT_INFO), "id": "misc"}, - ], - } + with pytest.raises(Exception): + activity.model_dump(mode="json", exclude_unset=True, by_alias=True) From 26f1704089e9d491762f3deb6146e9d516e6bd9a Mon Sep 17 00:00:00 2001 From: Rodrigo Brandao Date: Mon, 13 Oct 2025 13:32:45 -0700 Subject: [PATCH 20/22] Adding copyright comment --- .../microsoft_agents/activity/__init__.py | 3 +++ .../microsoft_agents/activity/entity/__init__.py | 3 +++ .../microsoft_agents/activity/entity/entity_types.py | 3 +++ 3 files changed, 9 insertions(+) diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/__init__.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/__init__.py index d467a123..b18336b7 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/__init__.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/__init__.py @@ -1,3 +1,6 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + from .agents_model import AgentsModel from .action_types import ActionTypes from .activity import Activity diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/__init__.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/__init__.py index 94854ad1..42fe69fd 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/__init__.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/__init__.py @@ -1,3 +1,6 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + from .mention import Mention from .entity import Entity from .entity_types import EntityTypes diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/entity_types.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/entity_types.py index 3f53481c..3bbb9f0a 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/entity_types.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/entity_types.py @@ -1,3 +1,6 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + from strenum import StrEnum From 946da690d7e2988a12ad78d60c6351b9044f035b Mon Sep 17 00:00:00 2001 From: Rodrigo Brandao Date: Mon, 13 Oct 2025 14:10:54 -0700 Subject: [PATCH 21/22] Reverting strenum usage --- .../microsoft_agents/activity/entity/entity_types.py | 4 ++-- libraries/microsoft-agents-activity/pyproject.toml | 3 +-- tests/activity/pydantic/test_activity_io.py | 12 ++++++------ 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/entity_types.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/entity_types.py index 3bbb9f0a..4af74397 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/entity_types.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/entity/entity_types.py @@ -1,10 +1,10 @@ # Copyright (c) Microsoft Corporation. All rights reserved. # Licensed under the MIT License. -from strenum import StrEnum +from enum import Enum -class EntityTypes(StrEnum): +class EntityTypes(str, Enum): """Well-known enumeration of entity types.""" GEO_COORDINATES = "GeoCoordinates" diff --git a/libraries/microsoft-agents-activity/pyproject.toml b/libraries/microsoft-agents-activity/pyproject.toml index b9f109a3..0c6938ab 100644 --- a/libraries/microsoft-agents-activity/pyproject.toml +++ b/libraries/microsoft-agents-activity/pyproject.toml @@ -16,8 +16,7 @@ classifiers = [ "Operating System :: OS Independent", ] dependencies = [ - "pydantic>=2.10.4", - "strenum", + "pydantic>=2.10.4" ] [project.urls] diff --git a/tests/activity/pydantic/test_activity_io.py b/tests/activity/pydantic/test_activity_io.py index 6af9b186..d60b11c3 100644 --- a/tests/activity/pydantic/test_activity_io.py +++ b/tests/activity/pydantic/test_activity_io.py @@ -158,14 +158,14 @@ def test_product_info_avoids_error_no_parent_channel(self): "type": "message", "channelId": "msteams:subchannel", "entities": [ - {"type": str(EntityTypes.PRODUCT_INFO), "id": "subchannel"} + {"type": EntityTypes.PRODUCT_INFO.value, "id": "subchannel"} ], }, { "type": "message", "channel_id": "msteams:subchannel", "entities": [ - {"type": str(EntityTypes.PRODUCT_INFO), "id": "subchannel"} + {"type": EntityTypes.PRODUCT_INFO.value, "id": "subchannel"} ], }, ], @@ -180,7 +180,7 @@ def test_product_info_avoids_error_no_parent_channel(self): "channelId": "msteams:subchannel", "entities": [ {"type": "other"}, - {"type": str(EntityTypes.PRODUCT_INFO), "id": "subchannel"}, + {"type": EntityTypes.PRODUCT_INFO.value, "id": "subchannel"}, ], }, { @@ -188,7 +188,7 @@ def test_product_info_avoids_error_no_parent_channel(self): "channel_id": "msteams:subchannel", "entities": [ {"type": "other"}, - {"type": str(EntityTypes.PRODUCT_INFO), "id": "subchannel"}, + {"type": EntityTypes.PRODUCT_INFO.value, "id": "subchannel"}, ], }, ], @@ -203,7 +203,7 @@ def test_product_info_avoids_error_no_parent_channel(self): "channelId": "msteams:misc", "entities": [ {"type": "other"}, - {"type": str(EntityTypes.PRODUCT_INFO), "id": "misc"}, + {"type": EntityTypes.PRODUCT_INFO.value, "id": "misc"}, ], }, { @@ -211,7 +211,7 @@ def test_product_info_avoids_error_no_parent_channel(self): "channel_id": "msteams:misc", "entities": [ {"type": "other"}, - {"type": str(EntityTypes.PRODUCT_INFO), "id": "misc"}, + {"type": EntityTypes.PRODUCT_INFO.value, "id": "misc"}, ], }, ], From 32487fba2f1694caba2c50d7f2c8eb8c0fd4b13f Mon Sep 17 00:00:00 2001 From: Rodrigo Brandao Date: Tue, 14 Oct 2025 10:44:25 -0700 Subject: [PATCH 22/22] Removing unnecessary str conversion and unnecessary comments --- .../microsoft_agents/activity/activity.py | 3 --- .../microsoft_agents/activity/conversation_reference.py | 1 - .../hosting/core/app/oauth/_handlers/_user_authorization.py | 2 +- 3 files changed, 1 insertion(+), 5 deletions(-) diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py index 8dd4d1d3..6b610579 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/activity.py @@ -192,9 +192,6 @@ class Activity(AgentsModel, _ChannelIdFieldMixin): semantic_action: SemanticAction = None caller_id: NonEmptyString = None - # annotated as Optional but cannot be set to None in practice - # _channel_id: Optional[ChannelId] = None # inherited from _ChannelIdFieldMixin - @model_validator(mode="wrap") @classmethod def _validate_channel_id( diff --git a/libraries/microsoft-agents-activity/microsoft_agents/activity/conversation_reference.py b/libraries/microsoft-agents-activity/microsoft_agents/activity/conversation_reference.py index 49ffe9fd..4ec1b4a8 100644 --- a/libraries/microsoft-agents-activity/microsoft_agents/activity/conversation_reference.py +++ b/libraries/microsoft-agents-activity/microsoft_agents/activity/conversation_reference.py @@ -52,7 +52,6 @@ class ConversationReference(AgentsModel, _ChannelIdFieldMixin): conversation: ConversationAccount locale: Optional[NonEmptyString] = None service_url: NonEmptyString = None - # _channel_id: ChannelId = None # inherited from _ChannelIdFieldMixin def get_continuation_activity(self) -> "Activity": # type: ignore from .activity import Activity diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/oauth/_handlers/_user_authorization.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/oauth/_handlers/_user_authorization.py index b7f52c3d..902b0dd4 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/oauth/_handlers/_user_authorization.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/oauth/_handlers/_user_authorization.py @@ -66,7 +66,7 @@ async def _load_flow( ): raise ValueError("Channel ID and User ID are required") - channel_id = str(context.activity.channel_id) + channel_id = context.activity.channel_id user_id = context.activity.from_property.id ms_app_id = context.turn_state.get(context.adapter.AGENT_IDENTITY_KEY).claims[