From 63c140df0a25976f82383a8491241ff5d33a87de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20Brand=C3=A3o?= Date: Thu, 17 Jul 2025 14:40:30 -0700 Subject: [PATCH 1/4] Refactoring of Blob Storage and unified storage tests added --- .../microsoft/agents/blob/__init__.py | 5 +- .../microsoft/agents/blob/blob_storage.py | 211 +++---- .../agents/blob/blob_storage_config.py | 27 + .../tests/test_blob_storage.py | 549 ++++-------------- .../agents/storage/error_handling.py | 28 + .../agents/storage/memory_storage.py | 15 + .../microsoft/agents/storage/storage.py | 78 ++- .../agents/storage/storage_test_utils.py | 410 +++++++++++++ .../tests/test_error_handling.py | 47 ++ .../tests/test_memory_storage.py | 19 + .../tests/test_utils.py | 79 +++ 11 files changed, 887 insertions(+), 581 deletions(-) create mode 100644 libraries/Storage/microsoft-agents-blob/microsoft/agents/blob/blob_storage_config.py create mode 100644 libraries/Storage/microsoft-agents-storage/microsoft/agents/storage/error_handling.py create mode 100644 libraries/Storage/microsoft-agents-storage/microsoft/agents/storage/storage_test_utils.py create mode 100644 libraries/Storage/microsoft-agents-storage/tests/test_error_handling.py create mode 100644 libraries/Storage/microsoft-agents-storage/tests/test_memory_storage.py create mode 100644 libraries/Storage/microsoft-agents-storage/tests/test_utils.py diff --git a/libraries/Storage/microsoft-agents-blob/microsoft/agents/blob/__init__.py b/libraries/Storage/microsoft-agents-blob/microsoft/agents/blob/__init__.py index dec949a6..16d2a106 100644 --- a/libraries/Storage/microsoft-agents-blob/microsoft/agents/blob/__init__.py +++ b/libraries/Storage/microsoft-agents-blob/microsoft/agents/blob/__init__.py @@ -1,3 +1,4 @@ -from .blob_storage import BlobStorage, BlobStorageSettings +from .blob_storage import BlobStorage +from .blob_storage_config import BlobStorageConfig -__all__ = ["BlobStorage", "BlobStorageSettings"] +__all__ = ["BlobStorage", "BlobStorageConfig"] diff --git a/libraries/Storage/microsoft-agents-blob/microsoft/agents/blob/blob_storage.py b/libraries/Storage/microsoft-agents-blob/microsoft/agents/blob/blob_storage.py index 1925b7e2..4b802505 100644 --- a/libraries/Storage/microsoft-agents-blob/microsoft/agents/blob/blob_storage.py +++ b/libraries/Storage/microsoft-agents-blob/microsoft/agents/blob/blob_storage.py @@ -1,174 +1,95 @@ -# based on -# https://github.com/microsoft/botbuilder-python/blob/main/libraries/botbuilder-azure/botbuilder/azure/blob_storage.py +import json from typing import TypeVar from io import BytesIO -import json -from azure.core.exceptions import ( - HttpResponseError, - ResourceExistsError, - ResourceNotFoundError, -) from azure.storage.blob.aio import ( ContainerClient, BlobServiceClient, ) +from microsoft.agents.storage import StoreItem +from microsoft.agents.storage.storage import AsyncStorageBase from microsoft.agents.storage._type_aliases import JSON -from microsoft.agents.storage import Storage, StoreItem - -StoreItemT = TypeVar("StoreItemT", bound=StoreItem) - - -class BlobStorageSettings: - - def __init__( - self, - container_name: str, - account_name: str = "", - account_key: str = "", - connection_string: str = "", - ): - self.container_name = container_name - self.account_name = account_name - self.account_key = account_key - self.connection_string = connection_string +from microsoft.agents.storage.error_handling import ignore_error, is_status_code_error +from .blob_storage_config import BlobStorageConfig -def convert_account_name_and_key_to_connection_string(settings: BlobStorageSettings): - if not settings.account_name or not settings.account_key: - raise ValueError( - "account_name and account_key are both required for BlobStorageSettings if not using a connections string." - ) - return ( - f"DefaultEndpointsProtocol=https;AccountName={settings.account_name};" - f"AccountKey={settings.account_key};EndpointSuffix=core.windows.net" - ) +StoreItemT = TypeVar("StoreItemT", bound=StoreItem) +class BlobStorage(AsyncStorageBase): -class BlobStorage(Storage): + def __init__(self, config: BlobStorageConfig): - def __init__(self, settings: BlobStorageSettings): - if not settings.container_name: + if not config.container_name: raise ValueError("BlobStorage: Container name is required.") - connection_string: str = settings.connection_string - if not connection_string: - # New Azure Blob SDK only allows connection strings, but our SDK allows key+name. - # This is here for backwards compatibility. - connection_string = convert_account_name_and_key_to_connection_string( - settings - ) - - blob_service_client: BlobServiceClient = ( - BlobServiceClient.from_connection_string(connection_string) - ) + self.config = config + blob_service_client: BlobServiceClient = self._create_client() self._container_client: ContainerClient = ( - blob_service_client.get_container_client(settings.container_name) + blob_service_client.get_container_client(config.container_name) ) self._initialized: bool = False - async def _initialize_container(self): + def _create_client(self) -> BlobServiceClient: + if self.config.url: # connect with URL and credentials + if not self.config.credential: + raise ValueError( + "BlobStorage: Credential is required when using a custom service URL." + ) + return BlobServiceClient(account_url=self.config.url, credential=self.config.credential) + + else: # connect with connection string + return BlobServiceClient.from_connection_string(self.config.connection_string) + + async def initialize(self) -> None: """Initializes the storage container""" - if self._initialized is False: + if not self._initialized: # This should only happen once - assuming this is a singleton. - # ContainerClient.exists() method is available in an unreleased version of the SDK. Until then, we use: - try: - await self._container_client.create_container() - except ResourceExistsError: - pass + await ignore_error( + self._container_client.create_container(), + is_status_code_error(409) + ) self._initialized = True - return self._initialized - - async def read( - self, keys: list[str], *, target_cls: StoreItemT = None, **kwargs - ) -> dict[str, StoreItemT]: - """Retrieve entities from the configured blob container. - - :param keys: An array of entity keys. - :type keys: dict[str, StoreItem] - :param target_cls: The StoreItem class to deserialize retrieved values into. - :type target_cls: StoreItem - :return dict: - """ - if not keys: - raise ValueError("BlobStorage.read(): Keys are required when reading.") - if not target_cls: - raise ValueError("BlobStorage.read(): target_cls cannot be None.") - - await self._initialize_container() - - result: dict[str, StoreItem] = {} - for key in keys: - - try: - item_rep: str = await ( - await self._container_client.download_blob(blob=key) - ).readall() - item_JSON: JSON = json.loads(item_rep) - except HttpResponseError as error: - if error.status_code == 404: - continue - else: - raise HttpResponseError( - f"BlobStorage.read(): Error reading blob '{key}': {error}" - ) - - try: - result[key] = target_cls.from_json_to_store_item(item_JSON) - except AttributeError as error: - raise TypeError( - f"BlobStorage.read(): could not deserialize blob item into {target_cls} class. Error: {error}" - ) - - return result - - async def write(self, changes: dict[str, StoreItem]): - """Stores a new entity in the configured blob container. - - :param changes: The changes to write to storage. - :type changes: dict[str, StoreItem] - :return: - """ - if not changes: - raise ValueError("BlobStorage.write(): changes cannot be None nor empty") - - await self._initialize_container() - - for key, item in changes.items(): - - item_JSON: JSON = item.store_item_to_json() - if item_JSON is None: - raise ValueError( - "BlobStorage.write(): StoreItem serialization cannot return None" - ) - item_rep_bytes = json.dumps(item_JSON).encode("utf-8") - - # providing the length parameter may improve performance - await self._container_client.upload_blob( - name=key, - data=BytesIO(item_rep_bytes), - overwrite=True, - length=len(item_rep_bytes), + async def _read_item( + self, key: str, *, target_cls: StoreItemT = None, **kwargs + ) -> tuple[str | None, StoreItemT | None]: + item = await ignore_error( + self._container_client.download_blob(blob=key), + is_status_code_error(404), + ) + if not item: + return None, None + + item_rep: str = await item.readall() + item_JSON: JSON = json.loads(item_rep) + try: + return key, target_cls.from_json_to_store_item(item_JSON) + except AttributeError as error: + raise TypeError( + f"BlobStorage.read_item(): could not deserialize blob item into {target_cls} class. Error: {error}" ) - async def delete(self, keys: list[str]): - """Deletes entity blobs from the configured container. - - :param keys: An array of entity keys. - :type keys: list[str] - """ - if keys is None: - raise ValueError("BlobStorage.delete(): keys parameter can't be null") - - await self._initialize_container() + async def _write_item(self, key: str, item: StoreItem) -> None: + item_JSON: JSON = item.store_item_to_json() + if item_JSON is None: + raise ValueError( + "BlobStorage.write(): StoreItem serialization cannot return None" + ) + item_rep_bytes = json.dumps(item_JSON).encode("utf-8") + + # getting the length is important for performance with large blobs + await self._container_client.upload_blob( + name=key, + data=BytesIO(item_rep_bytes), + overwrite=True, + length=len(item_rep_bytes), + ) - for key in keys: - try: - await self._container_client.delete_blob(blob=key) - # We can't delete what's already gone. - except ResourceNotFoundError: - pass + async def _delete_item(self, key: str) -> None: + await ignore_error( + self._container_client.delete_blob(blob=key), + is_status_code_error(404) + ) \ No newline at end of file diff --git a/libraries/Storage/microsoft-agents-blob/microsoft/agents/blob/blob_storage_config.py b/libraries/Storage/microsoft-agents-blob/microsoft/agents/blob/blob_storage_config.py new file mode 100644 index 00000000..f5157c96 --- /dev/null +++ b/libraries/Storage/microsoft-agents-blob/microsoft/agents/blob/blob_storage_config.py @@ -0,0 +1,27 @@ +from typing import Union + +from azure.core.credentials import TokenCredential + +class BlobStorageConfig: + """Configuration settings for BlobStorage.""" + + def __init__( + self, + container_name: str, + connection_string: str = "", + url: str = "", + credential: Union[TokenCredential, None] = None, + ): + """Configuration settings for BlobStorage. + + container_name: The name of the blob container. + connection_string: The connection string to the storage account. + url: The URL of the blob service. If provided, credential must also be provided. + credential: The TokenCredential to use for authentication when using a custom URL. + + credential-based authentication is prioritized over connection string authentication. + """ + self.container_name: str = container_name + self.connection_string: str = connection_string + self.url: str = url + self.credential: Union[TokenCredential, None] = credential \ No newline at end of file diff --git a/libraries/Storage/microsoft-agents-blob/tests/test_blob_storage.py b/libraries/Storage/microsoft-agents-blob/tests/test_blob_storage.py index e68a31a9..0db95603 100644 --- a/libraries/Storage/microsoft-agents-blob/tests/test_blob_storage.py +++ b/libraries/Storage/microsoft-agents-blob/tests/test_blob_storage.py @@ -1,257 +1,26 @@ -# temporary fix for pytest import issue. There are two separate scripts here - -# Copyright (c) Microsoft Corporation. All rights reserved. -# Licensed under the MIT License. - -""" -Adapted from https://github.com/microsoft/botbuilder-python/blob/main/libraries/botbuilder-testing/botbuilder/testing/storage_base_tests.py - -Base tests that all storage providers should implement in their own tests. -They handle the storage-based assertions, internally. - -All tests return true if assertions pass to indicate that the code ran to completion, passing internal assertions. -Therefore, all tests using theses static tests should strictly check that the method returns true. - -:Example: - async def test_handle_null_keys_when_reading(self): - await reset() - - test_ran = await StorageBaseTests.handle_null_keys_when_reading(get_storage()) - - assert test_ran -""" - -import pytest - -from microsoft.agents.storage import MemoryStorage, StoreItem -from microsoft.agents.storage._type_aliases import JSON - - -class MockStoreItem(StoreItem): - - def __init__(self, data: JSON = None): - self.data = data or {} - - def store_item_to_json(self) -> JSON: - return self.data - - @staticmethod - def from_json_to_store_item(json_data: JSON) -> "MockStoreItem": - return MockStoreItem(json_data) - - -class StorageBaseTests: - - # pylint: disable=pointless-string-statement - @staticmethod - async def return_empty_object_when_reading_unknown_key(storage) -> bool: - result = await storage.read(["unknown"], target_cls=MockStoreItem) - - assert result is not None - assert len(result) == 0 - - return True - - @staticmethod - async def handle_null_keys_when_reading(storage) -> bool: - if isinstance(storage, (MemoryStorage)): - result = await storage.read(None, target_cls=MockStoreItem) - assert len(result.keys()) == 0 - # Catch-all - else: - with pytest.raises(Exception) as err: - await storage.read(None, target_cls=MockStoreItem) - - return True - - @staticmethod - async def handle_null_keys_when_writing(storage) -> bool: - with pytest.raises(Exception) as err: - await storage.write(None) - # assert err.value.args[0] == "Changes are required when writing" - - return True - - @staticmethod - async def does_raise_when_writing_no_items(storage) -> bool: - # noinspection PyBroadException - with pytest.raises(Exception) as err: - await storage.write(dict()) - return True - - @staticmethod - async def create_object(storage) -> bool: - store_items = { - "createPoco": MockStoreItem({"id": 1}), - "createPocoStoreItem": MockStoreItem({"id": 2, "value": "*"}), - } - - await storage.write(store_items) - - read_store_items = await storage.read( - store_items.keys(), target_cls=MockStoreItem - ) - - assert ( - store_items["createPoco"].data["id"] - == read_store_items["createPoco"].data["id"] - ) - assert ( - store_items["createPocoStoreItem"].data["id"] - == read_store_items["createPocoStoreItem"].data["id"] - ) - assert read_store_items["createPocoStoreItem"].data["value"] == "*" - - return True - - @staticmethod - async def handle_crazy_keys(storage) -> bool: - key = '!@#$%^&*()_+??><":QASD~`' - store_item = MockStoreItem({"id": 1}) - store_items = {key: store_item} - - await storage.write(store_items) - - read_store_items = await storage.read( - store_items.keys(), target_cls=MockStoreItem - ) - - assert read_store_items[key] is not None - assert read_store_items[key].data["id"] == 1 - - return True - - @staticmethod - async def update_object(storage) -> bool: - original_store_items = { - "pocoItem": MockStoreItem({"id": 1, "count": 1}), - "pocoStoreItem": MockStoreItem({"id": 1, "count": 1, "value": "*"}), - } - - # 1st write should work - await storage.write(original_store_items) - - loaded_store_items = await storage.read( - ["pocoItem", "pocoStoreItem"], target_cls=MockStoreItem - ) - - update_poco_item = loaded_store_items["pocoItem"] - update_poco_item.data["value"] = None - update_poco_store_item = loaded_store_items["pocoStoreItem"] - assert update_poco_store_item.data["value"] == "*" - - # 2nd write should work - update_poco_item.data["count"] += 1 - update_poco_store_item.data["count"] += 1 - - await storage.write( - { - key: MockStoreItem(value.data) - for key, value in loaded_store_items.items() - } - ) - - reloaded_store_items = await storage.read( - loaded_store_items.keys(), target_cls=MockStoreItem - ) - - reloaded_update_poco_item = reloaded_store_items["pocoItem"] - reloaded_update_poco_store_item = reloaded_store_items["pocoStoreItem"] - - assert reloaded_update_poco_item.data["count"] == 2 - assert reloaded_update_poco_store_item.data["count"] == 2 - assert reloaded_update_poco_item.data["value"] is None - assert reloaded_update_poco_store_item.data["value"] == "*" - - return True - - @staticmethod - async def delete_object(storage) -> bool: - store_items = {"delete1": MockStoreItem({"id": 1, "count": 1, "value": "*"})} - - await storage.write(store_items) - - read_store_items = await storage.read(["delete1"], target_cls=MockStoreItem) - - assert read_store_items["delete1"].data["value"] - assert read_store_items["delete1"].data["count"] == 1 - - await storage.delete(["delete1"]) - - reloaded_store_items = await storage.read(["delete1"], target_cls=MockStoreItem) - - assert reloaded_store_items.get("delete1", None) is None - - return True - - @staticmethod - async def delete_unknown_object(storage) -> bool: - # noinspection PyBroadException - try: - await storage.delete(["unknown_key"]) - except: - pytest.fail("Should not raise") - - return True - - @staticmethod - async def perform_batch_operations(storage) -> bool: - await storage.write( - { - "batch1": MockStoreItem({"count": 10}), - "batch2": MockStoreItem({"count": 20}), - "batch3": MockStoreItem({"count": 30}), - } - ) - - result = await storage.read( - ["batch1", "batch2", "batch3"], target_cls=MockStoreItem - ) - - assert result.get("batch1", None) is not None - assert result.get("batch2", None) is not None - assert result.get("batch3", None) is not None - assert result["batch1"].data["count"] == 10 - assert result["batch2"].data["count"] == 20 - assert result["batch3"].data["count"] == 30 - - await storage.delete(["batch1", "batch2", "batch3"]) - - result = await storage.read( - ["batch1", "batch2", "batch3"], target_cls=MockStoreItem - ) - - assert result.get("batch1", None) is None - assert result.get("batch2", None) is None - assert result.get("batch3", None) is None - - return True - - -# Copyright (c) Microsoft Corporation. All rights reserved. -# Licensed under the MIT License. - -# based on https://github.com/microsoft/botbuilder-python/blob/main/libraries/botbuilder-azure/tests/test_blob_storage.py +import json +import gc +import os +from io import BytesIO import pytest import pytest_asyncio -from azure.core.exceptions import ResourceNotFoundError +from microsoft.agents.blob import BlobStorage, BlobStorageConfig from azure.storage.blob.aio import BlobServiceClient +from azure.core.exceptions import ResourceNotFoundError -from microsoft.agents.storage import StoreItem -from microsoft.agents.storage._type_aliases import JSON -from microsoft.agents.blob import BlobStorage, BlobStorageSettings - +from microsoft.agents.storage.storage_test_utils import ( + CRUDStorageTests, + StorageMock, + StorageBaseline, + MockStoreItem, + MockStoreItemB +) EMULATOR_RUNNING = False - -# constructs an emulated blob storage instance -@pytest_asyncio.fixture -async def blob_storage(): - - # setup +async def blob_storage_instance(existing=False): # Default Azure Storage Emulator connection string connection_string = ( @@ -263,226 +32,140 @@ async def blob_storage(): blob_service_client = BlobServiceClient.from_connection_string(connection_string) - container_name = "test" - container_client = blob_service_client.get_container_client(container_name) + container_name = "asdkunittest" + + if not existing: - # reset state of test container - try: - await container_client.delete_container() - except ResourceNotFoundError: - pass - await container_client.create_container() + # reset state of test container + try: + container_client = blob_service_client.get_container_client(container_name) + await container_client.delete_container() + except ResourceNotFoundError: + pass + + container_client = await blob_service_client.create_container(container_name) + else: + container_client = blob_service_client.get_container_client(container_name) - blob_storage_settings = BlobStorageSettings( - account_name="", - account_key="", + blob_storage_config = BlobStorageConfig( container_name=container_name, connection_string=connection_string, ) - storage = BlobStorage(blob_storage_settings) + storage = BlobStorage(blob_storage_config) + return storage, container_client + +@pytest_asyncio.fixture +async def blob_storage(): + + # setup + storage, container_client = await blob_storage_instance() yield storage # teardown await container_client.delete_container() +class BlobStorageMock(StorageMock): -class SimpleStoreItem(StoreItem): - - def __init__(self, counter: int = 1, value: str = "*"): - self.counter = counter - self.value = value - - def store_item_to_json(self) -> JSON: - return { - "counter": self.counter, - "value": self.value, - } - - @staticmethod - def from_json_to_store_item(json_data: JSON) -> "StoreItem": - return SimpleStoreItem(json_data["counter"], json_data["value"]) - + def __init__(self, blob_storage): + self.storage = blob_storage -class TestBlobStorageConstructor: + def get_backing_store(self): + return self.storage - @pytest.mark.asyncio - async def test_blob_storage_init_should_error_without_container_name(self): - settings = BlobStorageSettings("") - with pytest.raises(Exception) as err: - BlobStorage(settings) - - assert err.value.args[0] == "BlobStorage: Container name is required." +@pytest.mark.skipif(not EMULATOR_RUNNING, reason="Needs the emulator to run.") +class TestBlobStorage(CRUDStorageTests): + + async def storage(self, initial_data = None, existing=False): + if not initial_data: + initial_data = {} + storage, container_client = await blob_storage_instance(existing=existing) - @pytest.mark.asyncio - async def test_blob_storage_init_should_error_without_blob_config(self): - try: - BlobStorage(BlobStorageSettings()) # pylint: disable=no-value-for-parameter - except Exception as error: - assert error + for key, value in initial_data.items(): + value_rep = json.dumps(value.store_item_to_json()) + await container_client.upload_blob(name=key, data=value_rep, overwrite=True) + return BlobStorageMock(storage) + @pytest.mark.asyncio - async def test_blob_storage_init_should_error_with_insufficient_settings(self): - settings_0 = BlobStorageSettings("norway", account_name="some_account_name") - settings_1 = BlobStorageSettings("sweden", account_key="some_account_key") - with pytest.raises(Exception) as err: - BlobStorage(settings_0) - with pytest.raises(Exception) as err: - BlobStorage(settings_1) + async def test_initialize(self, blob_storage): + await blob_storage.initialize() + await blob_storage.initialize() + await blob_storage.write({"key": MockStoreItem({"id": "item", "value": "data"})}) + await blob_storage.initialize() + assert (await blob_storage.read(["key"], target_cls=MockStoreItem)) == {"key": MockStoreItem({"id": "item", "value": "data"})} @pytest.mark.asyncio - async def test_blob_storage_init_from_account_key_and_name(self): - settings = BlobStorageSettings( - "norway", account_name="some_account_name", account_key="some_account_key" - ) - BlobStorage(settings) - + async def test_blob_storage_flow_existing_container_and_persistence(self): -class TestBlobStorageBaseTests: - @pytest.mark.skipif(not EMULATOR_RUNNING, reason="Needs the emulator to run.") - @pytest.mark.asyncio - async def test_return_empty_object_when_reading_unknown_key(self, blob_storage): - test_ran = await StorageBaseTests.return_empty_object_when_reading_unknown_key( - blob_storage + connection_string = ( + "AccountName=devstoreaccount1;AccountKey=Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq" + + "2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw==;DefaultEndpointsProtocol=http;BlobEndpoint=" + + "http://127.0.0.1:10000/devstoreaccount1;QueueEndpoint=http://127.0.0.1:10001/devstoreaccount1;" + + "TableEndpoint=http://127.0.0.1:10002/devstoreaccount1;" ) - assert test_ran - - @pytest.mark.skipif(not EMULATOR_RUNNING, reason="Needs the emulator to run.") - @pytest.mark.asyncio - async def test_handle_null_keys_when_reading(self, blob_storage): - test_ran = await StorageBaseTests.handle_null_keys_when_reading(blob_storage) - assert test_ran - - @pytest.mark.skipif(not EMULATOR_RUNNING, reason="Needs the emulator to run.") - @pytest.mark.asyncio - async def test_handle_null_keys_when_writing(self, blob_storage): - test_ran = await StorageBaseTests.handle_null_keys_when_writing(blob_storage) - assert test_ran + blob_service_client = BlobServiceClient.from_connection_string(connection_string) + container_name = "asdkunittestpopulated" + container_client = blob_service_client.get_container_client(container_name) - @pytest.mark.skipif(not EMULATOR_RUNNING, reason="Needs the emulator to run.") - @pytest.mark.asyncio - async def does_raise_when_writing_no_items(self, blob_storage): - test_ran = await StorageBaseTests.does_raise_when_writing_no_items(blob_storage) - assert test_ran + # reset state of test container + try: + await container_client.delete_container() + except ResourceNotFoundError: + pass + await container_client.create_container() + + initial_data = { + "item1": MockStoreItem({"id": "item1", "value": "data1"}), + "__some_key": MockStoreItem({"id": "item2", "value": "data2"}), + "!another_key": MockStoreItem({"id": "item3", "value": "data3"}), + "1230": MockStoreItemB({"id": "item8", "value": "data"}, False), + "key-with-dash": MockStoreItem({"id": "item4", "value": "data"}), + "key.with.dot": MockStoreItem({"id": "item5", "value": "data"}), + "key/with/slash": MockStoreItem({"id": "item6", "value": "data"}), + "another key": MockStoreItemB({"id": "item7", "value": "data"}, True), + } - @pytest.mark.skipif(not EMULATOR_RUNNING, reason="Needs the emulator to run.") - @pytest.mark.asyncio - async def test_create_object(self, blob_storage): - test_ran = await StorageBaseTests.create_object(blob_storage) - assert test_ran + baseline_storage = StorageBaseline(initial_data) - @pytest.mark.skipif(not EMULATOR_RUNNING, reason="Needs the emulator to run.") - @pytest.mark.asyncio - async def test_handle_crazy_keys(self, blob_storage): - test_ran = await StorageBaseTests.handle_crazy_keys(blob_storage) - assert test_ran + for key, value in initial_data.items(): + value_rep = json.dumps(value.store_item_to_json()).encode("utf-8") + await container_client.upload_blob(name=key, data=BytesIO(value_rep), overwrite=True) - @pytest.mark.skipif(not EMULATOR_RUNNING, reason="Needs the emulator to run.") - @pytest.mark.asyncio - async def test_update_object(self, blob_storage): - test_ran = await StorageBaseTests.update_object(blob_storage) - assert test_ran + blob_storage_config = BlobStorageConfig( + container_name=container_name, + connection_string=connection_string + ) - @pytest.mark.skipif(not EMULATOR_RUNNING, reason="Needs the emulator to run.") - @pytest.mark.asyncio - async def test_delete_object(self, blob_storage): - test_ran = await StorageBaseTests.delete_object(blob_storage) - assert test_ran + storage = BlobStorage(blob_storage_config) - @pytest.mark.skipif(not EMULATOR_RUNNING, reason="Needs the emulator to run.") - @pytest.mark.asyncio - async def test_perform_batch_operations(self, blob_storage): - test_ran = await StorageBaseTests.perform_batch_operations(blob_storage) - assert test_ran + assert await baseline_storage.equals(storage) + assert (await storage.read(["1230", "another key"], target_cls=MockStoreItemB)) == baseline_storage.read(["1230", "another key"]) + changes = { + "item1": MockStoreItem({"id": "item1", "value": "data1_changed"}), + "__some_key": MockStoreItem({"id": "item2", "value": "data2_changed"}), + "new_item": MockStoreItem({"id": "new_item", "value": "new_data"}), + } -class TestBlobStorage: + baseline_storage.write(changes) + await storage.write(changes) - @pytest.mark.skipif(not EMULATOR_RUNNING, reason="Needs the emulator to run.") - @pytest.mark.asyncio - async def test_blob_storage_read_update_same_data(self, blob_storage): - await blob_storage.write({"test": SimpleStoreItem(counter=1)}) - data_result = await blob_storage.read(["test"], target_cls=SimpleStoreItem) - data_result["test"].counter = 2 - await blob_storage.write(data_result) - data_updated = await blob_storage.read(["test"], target_cls=SimpleStoreItem) - assert data_updated["test"].counter == 2 - assert data_updated["test"].value == data_result["test"].value - - @pytest.mark.skipif(not EMULATOR_RUNNING, reason="Needs the emulator to run.") - @pytest.mark.asyncio - async def test_blob_storage_write_should_overwrite(self, blob_storage): - await blob_storage.write({"user": SimpleStoreItem()}) - await blob_storage.write({"user": SimpleStoreItem(counter=10, value="*")}) - data = await blob_storage.read(["user"], target_cls=SimpleStoreItem) - assert data["user"].counter == 10 - assert data["user"].value == "*" - - @pytest.mark.skipif(not EMULATOR_RUNNING, reason="Needs the emulator to run.") - @pytest.mark.asyncio - async def test_blob_storage_delete_should_delete_according_cached_data( - self, blob_storage - ): - await blob_storage.write({"test": SimpleStoreItem()}) - try: - await blob_storage.delete(["test"]) - except Exception as error: - raise error - else: - data = await blob_storage.read(["test"], target_cls=SimpleStoreItem) + baseline_storage.delete(["!another_key", "item1"]) + await storage.delete(["!another_key", "item1"]) + assert await baseline_storage.equals(storage) - assert isinstance(data, dict) - assert not data.keys() + del storage + gc.collect() - @pytest.mark.skipif(not EMULATOR_RUNNING, reason="Needs the emulator to run.") - @pytest.mark.asyncio - async def test_blob_storage_delete_should_delete_multiple_values_when_given_multiple_valid_keys( - self, blob_storage - ): - await blob_storage.write( - {"test": SimpleStoreItem(), "test2": SimpleStoreItem(2)} - ) - await blob_storage.delete(["test", "test2"]) - data = await blob_storage.read(["test", "test2"], target_cls=SimpleStoreItem) - assert not data.keys() + blob_client = container_client.get_blob_client("item1") + with pytest.raises(ResourceNotFoundError): + await (await blob_client.download_blob()).readall() - @pytest.mark.skipif(not EMULATOR_RUNNING, reason="Needs the emulator to run.") - @pytest.mark.asyncio - async def test_blob_storage_delete_should_delete_values_when_given_multiple_valid_keys_and_ignore_other_data( - self, blob_storage - ): - await blob_storage.write( - { - "test": SimpleStoreItem(), - "test2": SimpleStoreItem(counter=2), - "test3": SimpleStoreItem(counter=3), - } - ) - await blob_storage.delete(["test", "test2"]) - data = await blob_storage.read( - ["test", "test2", "test3"], target_cls=SimpleStoreItem - ) - assert len(data.keys()) == 1 + blob_client = container_client.get_blob_client("1230") + item = await(await blob_client.download_blob()).readall() + assert MockStoreItemB.from_json_to_store_item(json.loads(item)) == initial_data["1230"] - @pytest.mark.skipif(not EMULATOR_RUNNING, reason="Needs the emulator to run.") - @pytest.mark.asyncio - async def test_blob_storage_delete_invalid_key_should_do_nothing_and_not_affect_cached_data( - self, blob_storage - ): - await blob_storage.write({"test": SimpleStoreItem()}) - await blob_storage.delete(["foo"]) - data = await blob_storage.read(["test"], target_cls=SimpleStoreItem) - assert len(data.keys()) == 1 - data = await blob_storage.read(["foo"], target_cls=SimpleStoreItem) - assert not data.keys() - - @pytest.mark.skipif(not EMULATOR_RUNNING, reason="Needs the emulator to run.") - @pytest.mark.asyncio - async def test_blob_storage_delete_invalid_keys_should_do_nothing_and_not_affect_cached_data( - self, blob_storage - ): - await blob_storage.write({"test": SimpleStoreItem()}) - await blob_storage.delete(["foo", "bar"]) - data = await blob_storage.read(["test"], target_cls=SimpleStoreItem) - assert len(data.keys()) == 1 + # teardown + await container_client.delete_container() \ No newline at end of file diff --git a/libraries/Storage/microsoft-agents-storage/microsoft/agents/storage/error_handling.py b/libraries/Storage/microsoft-agents-storage/microsoft/agents/storage/error_handling.py new file mode 100644 index 00000000..a58a6932 --- /dev/null +++ b/libraries/Storage/microsoft-agents-storage/microsoft/agents/storage/error_handling.py @@ -0,0 +1,28 @@ +from collections.abc import Callable, Awaitable +from typing import TypeVar + +error_filter = TypeVar("error_filter", bound=Callable[[Exception], bool]) + +async def ignore_error(promise: Awaitable, ignore_error_filter: error_filter): + """ + Ignores errors based on the provided filter function. + promise: the awaitable to execute + ignore_error_filter: a function that takes an Exception and returns True if the error should be + """ + try: + return await promise + except Exception as err: + if ignore_error_filter(err): + return None + raise err + +def is_status_code_error(*ignored_codes: list[int]) -> error_filter: + """ + Creates an error filter function that ignores errors with specific status codes. + """ + def func(err: Exception) -> bool: + if hasattr(err, "status_code") and err.status_code in ignored_codes: + return True + return False + + return func \ No newline at end of file diff --git a/libraries/Storage/microsoft-agents-storage/microsoft/agents/storage/memory_storage.py b/libraries/Storage/microsoft-agents-storage/microsoft/agents/storage/memory_storage.py index 8b0845f2..3fac5ae1 100644 --- a/libraries/Storage/microsoft-agents-storage/microsoft/agents/storage/memory_storage.py +++ b/libraries/Storage/microsoft-agents-storage/microsoft/agents/storage/memory_storage.py @@ -17,9 +17,17 @@ def __init__(self, state: dict[str, JSON] = None): async def read( self, keys: list[str], *, target_cls: StoreItemT = None, **kwargs ) -> dict[str, StoreItemT]: + + if not keys: + raise ValueError("Storage.read(): Keys are required when reading.") + if not target_cls: + raise ValueError("Storage.read(): target_cls cannot be None.") + result: dict[str, StoreItem] = {} with self._lock: for key in keys: + if key == "": + raise ValueError("MemoryStorage.read(): key cannot be empty") if key in self._memory: if not target_cls: result[key] = self._memory[key] @@ -40,10 +48,17 @@ async def write(self, changes: dict[str, StoreItem]): with self._lock: for key in changes: + if key == "": + raise ValueError("MemoryStorage.write(): key cannot be empty") self._memory[key] = changes[key].store_item_to_json() async def delete(self, keys: list[str]): + if not keys: + raise ValueError("Storage.delete(): Keys are required when deleting.") + with self._lock: for key in keys: + if key == "": + raise ValueError("MemoryStorage.delete(): key cannot be empty") if key in self._memory: del self._memory[key] diff --git a/libraries/Storage/microsoft-agents-storage/microsoft/agents/storage/storage.py b/libraries/Storage/microsoft-agents-storage/microsoft/agents/storage/storage.py index 2ec1d292..28372e56 100644 --- a/libraries/Storage/microsoft-agents-storage/microsoft/agents/storage/storage.py +++ b/libraries/Storage/microsoft-agents-storage/microsoft/agents/storage/storage.py @@ -1,4 +1,6 @@ from typing import Protocol, TypeVar, Type +from abc import ABC, abstractmethod +from asyncio import gather from ._type_aliases import JSON from .store_item import StoreItem @@ -6,15 +8,89 @@ StoreItemT = TypeVar("StoreItemT", bound=StoreItem) - class Storage(Protocol): async def read( self, keys: list[str], *, target_cls: Type[StoreItemT] = None, **kwargs ) -> dict[str, StoreItemT]: + """Reads multiple items from storage. + + keys: A list of keys to read. + target_cls: The class to deserialize the stored JSON into. + Returns a dictionary of key to StoreItem. + + missing keys are omitted from the result. + """ pass async def write(self, changes: dict[str, StoreItemT]) -> None: + """Writes multiple items to storage. + + changes: A dictionary of key to StoreItem to write.""" pass async def delete(self, keys: list[str]) -> None: + """Deletes multiple items from storage. + + If a key does not exist, it is ignored. + + keys: A list of keys to delete. + """ + pass + +class AsyncStorageBase(Storage): + """Base class for asynchronous storage implementations.""" + + async def initialize(self) -> None: + """Initializes the storage container""" pass + + @abstractmethod + async def _read_item( + self, key: str, *, target_cls: Type[StoreItemT] = None, **kwargs + ) -> tuple[str | None, StoreItemT | None]: + """Reads a single item from storage by key. + + Returns a tuple of (key, StoreItem) if found, or (None, None) if not found. + """ + pass + + async def read( + self, keys: list[str], *, target_cls: Type[StoreItemT] = None, **kwargs + ) -> dict[str, StoreItemT]: + if not keys: + raise ValueError("Storage.read(): Keys are required when reading.") + if not target_cls: + raise ValueError("Storage.read(): target_cls cannot be None.") + + await self.initialize() + + items: list[tuple[str | None, StoreItemT | None]] = await gather(*[ + self._read_item(key, target_cls=target_cls, **kwargs) for key in keys + ]) + return {key: value for key, value in items if key is not None} + + @abstractmethod + async def _write_item(self, key: str, value: StoreItemT) -> None: + """Writes a single item to storage by key.""" + pass + + async def write(self, changes: dict[str, StoreItemT]) -> None: + if not changes: + raise ValueError("Storage.write(): Changes are required when writing.") + + await self.initialize() + + await gather(*[self._write_item(key, value) for key, value in changes.items()]) + + @abstractmethod + async def _delete_item(self, key: str) -> None: + """Deletes a single item from storage by key.""" + pass + + async def delete(self, keys: list[str]) -> None: + if not keys: + raise ValueError("Storage.delete(): Keys are required when deleting.") + + await self.initialize() + + await gather(*[self._delete_item(key) for key in keys]) diff --git a/libraries/Storage/microsoft-agents-storage/microsoft/agents/storage/storage_test_utils.py b/libraries/Storage/microsoft-agents-storage/microsoft/agents/storage/storage_test_utils.py new file mode 100644 index 00000000..9e15f027 --- /dev/null +++ b/libraries/Storage/microsoft-agents-storage/microsoft/agents/storage/storage_test_utils.py @@ -0,0 +1,410 @@ +import pytest +import gc +from copy import deepcopy +from abc import ABC +from typing import Any + +from .storage import Storage +from .store_item import StoreItem +from ._type_aliases import JSON +from .memory_storage import MemoryStorage + +class MockStoreItem(StoreItem): + """Test implementation of StoreItem for testing purposes""" + + def __init__(self, data: dict[str, Any] = None): + self.data = data or {} + + def store_item_to_json(self) -> JSON: + return self.data + + @staticmethod + def from_json_to_store_item(json_data: JSON) -> "MockStoreItem": + return MockStoreItem(json_data) + + def __eq__(self, other): + if not isinstance(other, MockStoreItem): + return False + return self.data == other.data + + def __repr__(self): + return f"MockStoreItem({self.data})" + + def deepcopy(self): + return MockStoreItem(my_deepcopy(self.data)) + +class MockStoreItemB(MockStoreItem): + """Another test implementation of StoreItem for testing purposes""" + + def __init__(self, data: dict[str, Any] = None, other_field: bool = True): + super().__init__(data or {}) + self.other_field = other_field + + def store_item_to_json(self) -> JSON: + return [ self.data, self.other_field ] + + @staticmethod + def from_json_to_store_item(json_data: JSON) -> "MockStoreItem": + return MockStoreItemB(json_data[0], json_data[1]) + + def __eq__(self, other): + if not isinstance(other, MockStoreItemB): + return False + return self.data == other.data and self.other_field == other.other_field + + def deepcopy(self): + return MockStoreItemB(my_deepcopy(self.data), self.other_field) + +def my_deepcopy(original): + """Deep copy an object, including StoreItem instances.""" + + iter_obj = None + if isinstance(original, list): + iter_obj = enumerate(original) + elif isinstance(original, dict): + iter_obj = original.items() + elif isinstance(original, MockStoreItem): + return original.deepcopy() + else: + return deepcopy(original) + + obj = {} if isinstance(original, dict) else ([None] * len(original)) + for key, value in iter_obj: + obj[key] = my_deepcopy(value) + return obj + +def subsets(lst, n=-1): + """Generate all subsets of a list up to length n. If n is -1, all subsets are generated. + + Only contiguous subsets are generated. + """ + if n < 0: + n = len(lst) + subsets = [] + for i in range(len(lst) + 1): + for j in range(0, i): + if 1 <= i - j <= n: + subsets.append(lst[j:i]) + return subsets + +class StorageMock(ABC): + """A mock wrapper around a Storage implementation to be used in tests.""" + + def get_backing_store(self) -> Storage: + raise NotImplementedError("Subclasses must implement this") + + async def read(self, *args, **kwargs): + return await self.get_backing_store().read(*args, **kwargs) + + async def write(self, *args, **kwargs): + return await self.get_backing_store().write(*args, **kwargs) + + async def delete(self, *args, **kwargs): + return await self.get_backing_store().delete(*args, **kwargs) + +# bootstrapping class to compare against +# if this class is correct, then the tests are correct +class StorageBaseline(Storage): + """"A simple in-memory storage implementation for testing purposes.""" + + def __init__(self, initial_data: dict = None): + self._memory = deepcopy(initial_data) or {} + self._key_history = set(initial_data.keys()) if initial_data else set() + + def read(self, keys: list[str]) -> dict[str, Any]: + self._key_history.update(keys) + return {key: self._memory.get(key) for key in keys if key in self._memory} + + def write(self, changes: dict[str, Any]) -> None: + self._key_history.update(changes.keys()) + self._memory.update(changes) + + def delete(self, keys: list[str]) -> None: + self._key_history.update(keys) + for key in keys: + if key in self._memory: + del self._memory[key] + + async def equals(self, other) -> bool: + """ + Compare the items for all keys seenby this mock instance. + This is an extra safety measure, and I've made the + executive decision to not test this method itself + as it is not the main focus of the test suite. + """ + for key in self._key_history: + if key not in self._memory: + if len(await other.read([key], target_cls=MockStoreItem)) > 0: + return False # key should not exist in other + continue + + # key exists in baseline instance, so let's see if the values match + item = self._memory.get(key, None) + target_cls = type(item) + res = (await other.read([key], target_cls=target_cls)) + + if key not in res or item != res[key]: + return False + return True + +class StorageTestsCommon(ABC): + + KEY_LIST = ([ + "f", + "a!0dslfj", "\\?/#\t\n\r*", + "527", "test.txt", "_-__--", "VAR", + "None", "multi word key" + ]) + + READ_KEY_LIST = KEY_LIST + (["5", "20", "100", "nonexistent_key", "-"]) + + STATE_LIST = [ + { key: MockStoreItem({"id": key, "value": f"value{key}"}) for key in subset } + for subset in subsets(KEY_LIST, 3) if len(subset) == 3 + ] + + @pytest.fixture(params=[dict()] + STATE_LIST) + def initial_state(self, request): + return request.param + + @pytest.fixture(params=KEY_LIST) + def key(self, request): + return request.param + + @pytest.fixture(params=[subset for subset in subsets(READ_KEY_LIST, 2) if len(subset) == 2]) + def keys(self, request): + return request.param + + @pytest.fixture(params=subsets(KEY_LIST, 2)) + def changes(self, request): + changes_obj = {} + keys = request.param + changes_obj["new_key"] = MockStoreItemB({"field": "new_value_for_new_key"}, True) + for i, key in enumerate(keys): + if i % 2 == 0: + changes_obj[key] = MockStoreItemB({"data": f"value{key}"}, (i // 2) % 2 == 0) + else: + changes_obj[key] = MockStoreItem({"id": key, "value": f"new_value_for_{key}"}) + changes_obj["new_key_2"] = MockStoreItem({"field": "new_value_for_new_key_2"}) + return changes_obj + +class CRUDStorageTests(StorageTestsCommon): + + async def storage(self, initial_data=None, existing=False): + raise NotImplementedError("Subclasses must implement this") + + @pytest.mark.asyncio + async def test_read_individual(self, initial_state, key): + initial_state_copy = my_deepcopy(initial_state) + baseline_storage = StorageBaseline(initial_state) + storage = await self.storage(initial_state) + expected = baseline_storage.read([key]) + actual = await storage.read([key], target_cls=MockStoreItem) + assert actual == expected + assert await baseline_storage.equals(storage) + assert initial_state == initial_state_copy + + @pytest.mark.asyncio + async def test_read(self, initial_state, keys): + initial_state_copy = my_deepcopy(initial_state) + baseline_storage = StorageBaseline(initial_state) + storage = await self.storage(initial_state) + expected = baseline_storage.read(keys) + actual = await storage.read(keys, target_cls=MockStoreItem) + assert actual == expected + assert await baseline_storage.equals(storage) + assert initial_state == initial_state_copy + + @pytest.mark.asyncio + async def test_read_missing_key(self, initial_state): + initial_state_copy = my_deepcopy(initial_state) + baseline_storage = StorageBaseline(initial_state) + storage = await self.storage(initial_state) + keys = [ "5", "20", "100", "nonexistent_key", "-" ] + expected = baseline_storage.read(keys) + actual = await storage.read(keys, target_cls=MockStoreItem) + assert actual == expected + assert await baseline_storage.equals(storage) + assert initial_state == initial_state_copy + + @pytest.mark.asyncio + async def test_read_errors(self, initial_state): + initial_state_copy = my_deepcopy(initial_state) + storage = await self.storage(initial_state) + with pytest.raises(ValueError): + await storage.read([], target_cls=MockStoreItem) + with pytest.raises(ValueError): + await storage.read(None, target_cls=MockStoreItem) + with pytest.raises(ValueError): + await storage.read([""], target_cls=MockStoreItem) + with pytest.raises(ValueError): + await storage.read(["key"], target_cls=None) + assert initial_state == initial_state_copy + + @pytest.mark.asyncio + async def test_write_individual(self, initial_state, key): + initial_state_copy = my_deepcopy(initial_state) + baseline_storage = StorageBaseline(initial_state) + storage = await self.storage(initial_state) + change = { key: MockStoreItem ({ key: f"new_value_for_{key}!" }) } + baseline_storage.write(change) + await storage.write(change) + assert await baseline_storage.equals(storage) + assert initial_state == initial_state_copy + + @pytest.mark.asyncio + async def test_write_individual_different_target_cls(self, initial_state, key): + initial_state_copy = my_deepcopy(initial_state) + baseline_storage = StorageBaseline(initial_state) + storage = await self.storage(initial_state) + change = { key: MockStoreItemB({ key: f"new_value_for_{key}!" }, other_field=False) } + baseline_storage.write(change) + await storage.write(change) + assert await baseline_storage.equals(storage) + change = { key: MockStoreItemB({ key: f"new_{key}" }, other_field=True) } + baseline_storage.write(change) + await storage.write(change) + assert await baseline_storage.equals(storage) + assert initial_state == initial_state_copy + + @pytest.mark.asyncio + async def test_write_same_values(self, initial_state): + if not initial_state: + return + initial_state_copy = my_deepcopy(initial_state) + baseline_storage = StorageBaseline(initial_state) + storage = await self.storage(initial_state) + changes = { key: value for key, value in initial_state.items() } + baseline_storage.write(changes) + await storage.write(changes) + assert await baseline_storage.equals(storage) + assert initial_state == initial_state_copy + + @pytest.mark.asyncio + async def test_write(self, initial_state, changes): + initial_state_copy = my_deepcopy(initial_state) + baseline_storage = StorageBaseline(initial_state) + storage = await self.storage(initial_state) + baseline_storage.write(changes) + await storage.write(changes) + assert await baseline_storage.equals(storage) + baseline_storage.write(initial_state) + if initial_state: + await storage.write(initial_state) + assert await baseline_storage.equals(storage) + assert initial_state == initial_state_copy + + @pytest.mark.asyncio + async def test_write_errors(self, initial_state): + initial_state_copy = my_deepcopy(initial_state) + baseline_storage = StorageBaseline(initial_state) + storage = await self.storage(initial_state) + with pytest.raises(ValueError): + await storage.write({}) + with pytest.raises(ValueError): + await storage.write(None) + assert await baseline_storage.equals(storage) + assert initial_state == initial_state_copy + + @pytest.mark.asyncio + async def test_delete_individual(self, initial_state, key): + initial_state_copy = my_deepcopy(initial_state) + baseline_storage = StorageBaseline(initial_state) + storage = await self.storage(initial_state) + baseline_storage.delete([key]) + await storage.delete([key]) + assert await baseline_storage.equals(storage) + assert initial_state == initial_state_copy + + @pytest.mark.asyncio + async def test_delete(self, initial_state, keys): + initial_state_copy = my_deepcopy(initial_state) + baseline_storage = StorageBaseline(initial_state) + storage = await self.storage(initial_state) + baseline_storage.delete(keys) + await storage.delete(keys) + assert await baseline_storage.equals(storage) + assert initial_state == initial_state_copy + + @pytest.mark.asyncio + async def test_delete_missing_key(self, initial_state): + initial_state_copy = my_deepcopy(initial_state) + baseline_storage = StorageBaseline(initial_state) + storage = await self.storage(initial_state) + keys = [ "5", "20", "100", "nonexistent_key", "-" ] + baseline_storage.delete(keys) + await storage.delete(keys) + assert await baseline_storage.equals(storage) + assert initial_state == initial_state_copy + + @pytest.mark.asyncio + async def test_delete_errors(self, initial_state): + initial_state_copy = my_deepcopy(initial_state) + storage = await self.storage(initial_state) + with pytest.raises(ValueError): + await storage.read([]) + with pytest.raises(ValueError): + await storage.read(None) + assert initial_state == initial_state_copy + + @pytest.mark.asyncio + async def test_flow(self): + baseline_storage = StorageBaseline() + storage = await self.storage() + + res = await storage.read(["key"], target_cls=MockStoreItemB) + assert len(res) == 0 + assert await baseline_storage.equals(storage) + + changes = { + "key_a": MockStoreItem({"id": "key_a", "value": "value_a"}), + "key_b": MockStoreItemB({"id": "key_b", "value": "value_b"}, other_field=False) + } + changes_copy = my_deepcopy(changes) + + baseline_storage.write(changes) + await storage.write(changes) + + assert (await storage.read(["key_a"], target_cls=MockStoreItem)) == baseline_storage.read(["key_a"]) + assert (await storage.read(["key_b"], target_cls=MockStoreItemB)) == baseline_storage.read(["key_b"]) + assert changes_copy == changes + + baseline_storage.delete(["key_a"]) + await storage.delete(["key_a"]) + assert await baseline_storage.equals(storage) + + change = {"key_b": MockStoreItem({"id": "key_b", "value": "new_value_b"})} + baseline_storage.write(change) + await storage.write(change) + + assert await baseline_storage.equals(storage) + assert (await storage.read(["key_b"], target_cls=MockStoreItem)) == baseline_storage.read(["key_b"]) + + with pytest.raises(ValueError): + await storage.read([], target_cls=MockStoreItem) + with pytest.raises(ValueError): + await storage.read(["key_b"], target_cls=None) + + change = {"key_c": MockStoreItemB({"id": "key_c", "value": "value_c"}, other_field=True)} + baseline_storage.write(change) + await storage.write(change) + assert (await storage.read(["key_a", "key_b"], target_cls=MockStoreItem)) == baseline_storage.read(["key_a", "key_b"]) + assert (await storage.read(["key_a", "key_c"], target_cls=MockStoreItemB)) == baseline_storage.read(["key_a", "key_c"]) + + item_parent_class = (await storage.read(["key_c"], target_cls=MockStoreItem))["key_c"] + item_child_class = (await storage.read(["key_c"], target_cls=MockStoreItemB))["key_c"] + assert item_parent_class.data[0] == item_child_class.data + assert item_child_class.other_field == True + + with pytest.raises(ValueError): + await storage.write({}) + with pytest.raises(Exception): + await storage.read(["key_b"], target_cls=MockStoreItemB) + assert await baseline_storage.equals(storage) + + if not isinstance(storage.get_backing_store(), MemoryStorage): + # if not memory storage, then items should persist + del storage + gc.collect() + storage_alt = await self.storage(existing=True) + assert await baseline_storage.equals(storage_alt) \ No newline at end of file diff --git a/libraries/Storage/microsoft-agents-storage/tests/test_error_handling.py b/libraries/Storage/microsoft-agents-storage/tests/test_error_handling.py new file mode 100644 index 00000000..d26078cf --- /dev/null +++ b/libraries/Storage/microsoft-agents-storage/tests/test_error_handling.py @@ -0,0 +1,47 @@ +import pytest +from microsoft.agents.storage.error_handling import ignore_error, is_status_code_error + +class CustomError(Exception): + def __init__(self, status_code: int): + self.status_code = status_code + +async def raise_custom_error(code: int): + raise CustomError(code) + +@pytest.mark.asyncio +async def test_ignore_error_without_error(): + + async def func(): + return 42 + + assert await ignore_error(func(), lambda e: False) == 42 + assert await ignore_error(func(), lambda e: True) == 42 + +@pytest.mark.asyncio +async def test_ignore_error_with_error(): + with pytest.raises(CustomError): + await ignore_error(raise_custom_error(500), lambda e: False) + +@pytest.mark.asyncio +async def test_ignore_error_with_ignored_error(): + assert await ignore_error(raise_custom_error(500), lambda e: True) is None + +@pytest.mark.asyncio +async def test_is_status_code_with_status_code_check(): + + async def func(): + return 42 + + assert await ignore_error(func(), is_status_code_error(404)) == 42 + assert await ignore_error(raise_custom_error(403), is_status_code_error(403)) is None + + with pytest.raises(CustomError) as err: + assert await ignore_error(raise_custom_error(404), is_status_code_error(500)) is None + + assert err.value.status_code == 404 + + async def raise_exception(): + raise Exception() + + with pytest.raises(Exception): + await ignore_error(raise_exception, is_status_code_error(404)) \ No newline at end of file diff --git a/libraries/Storage/microsoft-agents-storage/tests/test_memory_storage.py b/libraries/Storage/microsoft-agents-storage/tests/test_memory_storage.py new file mode 100644 index 00000000..747c22e5 --- /dev/null +++ b/libraries/Storage/microsoft-agents-storage/tests/test_memory_storage.py @@ -0,0 +1,19 @@ +from microsoft.agents.storage.memory_storage import MemoryStorage +from microsoft.agents.storage.storage_test_utils import CRUDStorageTests, StorageMock + +class MemoryStorageMock(StorageMock): + + def __init__(self, initial_data: dict = None): + + data = { + key: value.store_item_to_json() for key, value in (initial_data or {}).items() + } + self.storage = MemoryStorage(data) + + def get_backing_store(self): + return self.storage + +class TestMemoryStorage(CRUDStorageTests): + + async def storage(self, initial_state = None): + return MemoryStorageMock(initial_state) \ No newline at end of file diff --git a/libraries/Storage/microsoft-agents-storage/tests/test_utils.py b/libraries/Storage/microsoft-agents-storage/tests/test_utils.py new file mode 100644 index 00000000..7e70adc8 --- /dev/null +++ b/libraries/Storage/microsoft-agents-storage/tests/test_utils.py @@ -0,0 +1,79 @@ +from microsoft.agents.storage.storage_test_utils import MockStoreItem, MockStoreItemB, my_deepcopy, subsets + +def eq_helper(a, b): + def key(x): + return str(x) + return sorted(a, key=key) == sorted(b, key=key) + +def test_eq_helper(): + + a1 = [{"a": 1}, {"b": 2}, {"c": 3}] + a2 = [{"c": 3}, {"b": 2}, {"a": 1}] + assert eq_helper(a1, a2) + + a1 = [[2], [3], [4], [3]] + a2 = [[4], [3], [5], [2]] + assert not eq_helper(a1, a2) + + a1 = [["a"], [3], [4], [3]] + a2 = [[4], [3], [3], ["a"]] + assert eq_helper(a1, a2) + +def test_my_deepcopy(): + original = { + "a": MockStoreItem({"id": "a", "value": 1}), + "b": { + "b1": MockStoreItemB({"key": "b1"}, other_field=False), + "b2": [1, 2, 3], + "b3": { "nested": MockStoreItem({"id": "nested", "value": 42}) } + }, + "c": [ MockStoreItem({"id": "c1"}), MockStoreItemB({"id": "c2"}, other_field=True) ], + "d": "just a string", + "e": 12345 + } + copy = my_deepcopy(original) + assert copy == original + assert copy is not original + assert copy["a"] is not original["a"] + assert copy["b"] is not original["b"] + assert copy["b"]["b1"] is not original["b"]["b1"] + assert copy["b"]["b3"] is not original["b"]["b3"] + assert copy["b"]["b3"]["nested"] is not original["b"]["b3"]["nested"] + assert copy["c"] is not original["c"] + assert copy["c"][0] is not original["c"][0] + assert copy["c"][1] is not original["c"][1] + assert copy["d"] == original["d"] + assert copy["e"] == original["e"] + +def test_subsets(): + assert eq_helper(subsets(["a", "b", "c"], -1,), [ + ["a"], ["a", "b"], ["a", "b", "c"], + ["b"], ["b", "c"], + ["c"] + ]) + assert eq_helper(subsets(["a", "b", "c"]), [ + ["a"], ["a", "b"], ["a", "b", "c"], + ["b"], ["b", "c"], + ["c"] + ]) + +def test_subsets_0(): + assert subsets(["a", "b", "c", "d"], 0) == [] + +def test_subsets_1(): + assert eq_helper(subsets(["a", "b", "c", 3, 2], 1), [ + ["a"], ["b"], ["c"], [3], [2] + ]) + +def test_subsets_2(): + assert eq_helper(subsets(["a", "b", "c"], 2), [ + ["a"], ["b"], ["c"], + ["a", "b"], ["b", "c"] + ]) + +def test_subsets_3(): + assert eq_helper(subsets(["a", "b", "c"]), [ + ["a"], ["a", "b"], ["a", "b", "c"], + ["b"], ["b", "c"], + ["c"] + ]) \ No newline at end of file From 1860b318ecbcc97a62c925de6f025d66e9173825 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20Brand=C3=A3o?= Date: Thu, 17 Jul 2025 14:44:06 -0700 Subject: [PATCH 2/4] Reformatted with black --- .../microsoft/agents/blob/blob_storage.py | 25 +-- .../agents/blob/blob_storage_config.py | 5 +- .../tests/test_blob_storage.py | 44 ++++-- .../agents/storage/error_handling.py | 9 +- .../agents/storage/memory_storage.py | 2 +- .../microsoft/agents/storage/storage.py | 26 ++-- .../agents/storage/storage_test_utils.py | 146 ++++++++++++------ .../tests/test_error_handling.py | 19 ++- .../tests/test_memory_storage.py | 11 +- .../tests/test_utils.py | 66 ++++---- 10 files changed, 225 insertions(+), 128 deletions(-) diff --git a/libraries/Storage/microsoft-agents-blob/microsoft/agents/blob/blob_storage.py b/libraries/Storage/microsoft-agents-blob/microsoft/agents/blob/blob_storage.py index 4b802505..0fb021b9 100644 --- a/libraries/Storage/microsoft-agents-blob/microsoft/agents/blob/blob_storage.py +++ b/libraries/Storage/microsoft-agents-blob/microsoft/agents/blob/blob_storage.py @@ -17,6 +17,7 @@ StoreItemT = TypeVar("StoreItemT", bound=StoreItem) + class BlobStorage(AsyncStorageBase): def __init__(self, config: BlobStorageConfig): @@ -33,23 +34,26 @@ def __init__(self, config: BlobStorageConfig): self._initialized: bool = False def _create_client(self) -> BlobServiceClient: - if self.config.url: # connect with URL and credentials + if self.config.url: # connect with URL and credentials if not self.config.credential: raise ValueError( "BlobStorage: Credential is required when using a custom service URL." ) - return BlobServiceClient(account_url=self.config.url, credential=self.config.credential) - - else: # connect with connection string - return BlobServiceClient.from_connection_string(self.config.connection_string) + return BlobServiceClient( + account_url=self.config.url, credential=self.config.credential + ) + + else: # connect with connection string + return BlobServiceClient.from_connection_string( + self.config.connection_string + ) async def initialize(self) -> None: """Initializes the storage container""" if not self._initialized: # This should only happen once - assuming this is a singleton. await ignore_error( - self._container_client.create_container(), - is_status_code_error(409) + self._container_client.create_container(), is_status_code_error(409) ) self._initialized = True @@ -62,7 +66,7 @@ async def _read_item( ) if not item: return None, None - + item_rep: str = await item.readall() item_JSON: JSON = json.loads(item_rep) try: @@ -90,6 +94,5 @@ async def _write_item(self, key: str, item: StoreItem) -> None: async def _delete_item(self, key: str) -> None: await ignore_error( - self._container_client.delete_blob(blob=key), - is_status_code_error(404) - ) \ No newline at end of file + self._container_client.delete_blob(blob=key), is_status_code_error(404) + ) diff --git a/libraries/Storage/microsoft-agents-blob/microsoft/agents/blob/blob_storage_config.py b/libraries/Storage/microsoft-agents-blob/microsoft/agents/blob/blob_storage_config.py index f5157c96..af8f21e2 100644 --- a/libraries/Storage/microsoft-agents-blob/microsoft/agents/blob/blob_storage_config.py +++ b/libraries/Storage/microsoft-agents-blob/microsoft/agents/blob/blob_storage_config.py @@ -2,6 +2,7 @@ from azure.core.credentials import TokenCredential + class BlobStorageConfig: """Configuration settings for BlobStorage.""" @@ -13,7 +14,7 @@ def __init__( credential: Union[TokenCredential, None] = None, ): """Configuration settings for BlobStorage. - + container_name: The name of the blob container. connection_string: The connection string to the storage account. url: The URL of the blob service. If provided, credential must also be provided. @@ -24,4 +25,4 @@ def __init__( self.container_name: str = container_name self.connection_string: str = connection_string self.url: str = url - self.credential: Union[TokenCredential, None] = credential \ No newline at end of file + self.credential: Union[TokenCredential, None] = credential diff --git a/libraries/Storage/microsoft-agents-blob/tests/test_blob_storage.py b/libraries/Storage/microsoft-agents-blob/tests/test_blob_storage.py index 0db95603..8bea6fce 100644 --- a/libraries/Storage/microsoft-agents-blob/tests/test_blob_storage.py +++ b/libraries/Storage/microsoft-agents-blob/tests/test_blob_storage.py @@ -15,11 +15,12 @@ StorageMock, StorageBaseline, MockStoreItem, - MockStoreItemB + MockStoreItemB, ) EMULATOR_RUNNING = False + async def blob_storage_instance(existing=False): # Default Azure Storage Emulator connection string @@ -55,6 +56,7 @@ async def blob_storage_instance(existing=False): storage = BlobStorage(blob_storage_config) return storage, container_client + @pytest_asyncio.fixture async def blob_storage(): @@ -66,6 +68,7 @@ async def blob_storage(): # teardown await container_client.delete_container() + class BlobStorageMock(StorageMock): def __init__(self, blob_storage): @@ -74,10 +77,11 @@ def __init__(self, blob_storage): def get_backing_store(self): return self.storage + @pytest.mark.skipif(not EMULATOR_RUNNING, reason="Needs the emulator to run.") class TestBlobStorage(CRUDStorageTests): - - async def storage(self, initial_data = None, existing=False): + + async def storage(self, initial_data=None, existing=False): if not initial_data: initial_data = {} storage, container_client = await blob_storage_instance(existing=existing) @@ -87,14 +91,18 @@ async def storage(self, initial_data = None, existing=False): await container_client.upload_blob(name=key, data=value_rep, overwrite=True) return BlobStorageMock(storage) - + @pytest.mark.asyncio async def test_initialize(self, blob_storage): await blob_storage.initialize() await blob_storage.initialize() - await blob_storage.write({"key": MockStoreItem({"id": "item", "value": "data"})}) + await blob_storage.write( + {"key": MockStoreItem({"id": "item", "value": "data"})} + ) await blob_storage.initialize() - assert (await blob_storage.read(["key"], target_cls=MockStoreItem)) == {"key": MockStoreItem({"id": "item", "value": "data"})} + assert (await blob_storage.read(["key"], target_cls=MockStoreItem)) == { + "key": MockStoreItem({"id": "item", "value": "data"}) + } @pytest.mark.asyncio async def test_blob_storage_flow_existing_container_and_persistence(self): @@ -105,7 +113,9 @@ async def test_blob_storage_flow_existing_container_and_persistence(self): + "http://127.0.0.1:10000/devstoreaccount1;QueueEndpoint=http://127.0.0.1:10001/devstoreaccount1;" + "TableEndpoint=http://127.0.0.1:10002/devstoreaccount1;" ) - blob_service_client = BlobServiceClient.from_connection_string(connection_string) + blob_service_client = BlobServiceClient.from_connection_string( + connection_string + ) container_name = "asdkunittestpopulated" container_client = blob_service_client.get_container_client(container_name) @@ -131,17 +141,20 @@ async def test_blob_storage_flow_existing_container_and_persistence(self): for key, value in initial_data.items(): value_rep = json.dumps(value.store_item_to_json()).encode("utf-8") - await container_client.upload_blob(name=key, data=BytesIO(value_rep), overwrite=True) + await container_client.upload_blob( + name=key, data=BytesIO(value_rep), overwrite=True + ) blob_storage_config = BlobStorageConfig( - container_name=container_name, - connection_string=connection_string + container_name=container_name, connection_string=connection_string ) storage = BlobStorage(blob_storage_config) assert await baseline_storage.equals(storage) - assert (await storage.read(["1230", "another key"], target_cls=MockStoreItemB)) == baseline_storage.read(["1230", "another key"]) + assert ( + await storage.read(["1230", "another key"], target_cls=MockStoreItemB) + ) == baseline_storage.read(["1230", "another key"]) changes = { "item1": MockStoreItem({"id": "item1", "value": "data1_changed"}), @@ -164,8 +177,11 @@ async def test_blob_storage_flow_existing_container_and_persistence(self): await (await blob_client.download_blob()).readall() blob_client = container_client.get_blob_client("1230") - item = await(await blob_client.download_blob()).readall() - assert MockStoreItemB.from_json_to_store_item(json.loads(item)) == initial_data["1230"] + item = await (await blob_client.download_blob()).readall() + assert ( + MockStoreItemB.from_json_to_store_item(json.loads(item)) + == initial_data["1230"] + ) # teardown - await container_client.delete_container() \ No newline at end of file + await container_client.delete_container() diff --git a/libraries/Storage/microsoft-agents-storage/microsoft/agents/storage/error_handling.py b/libraries/Storage/microsoft-agents-storage/microsoft/agents/storage/error_handling.py index a58a6932..396c8d2f 100644 --- a/libraries/Storage/microsoft-agents-storage/microsoft/agents/storage/error_handling.py +++ b/libraries/Storage/microsoft-agents-storage/microsoft/agents/storage/error_handling.py @@ -3,6 +3,7 @@ error_filter = TypeVar("error_filter", bound=Callable[[Exception], bool]) + async def ignore_error(promise: Awaitable, ignore_error_filter: error_filter): """ Ignores errors based on the provided filter function. @@ -15,14 +16,16 @@ async def ignore_error(promise: Awaitable, ignore_error_filter: error_filter): if ignore_error_filter(err): return None raise err - + + def is_status_code_error(*ignored_codes: list[int]) -> error_filter: """ Creates an error filter function that ignores errors with specific status codes. """ + def func(err: Exception) -> bool: if hasattr(err, "status_code") and err.status_code in ignored_codes: return True return False - - return func \ No newline at end of file + + return func diff --git a/libraries/Storage/microsoft-agents-storage/microsoft/agents/storage/memory_storage.py b/libraries/Storage/microsoft-agents-storage/microsoft/agents/storage/memory_storage.py index 3fac5ae1..320eed37 100644 --- a/libraries/Storage/microsoft-agents-storage/microsoft/agents/storage/memory_storage.py +++ b/libraries/Storage/microsoft-agents-storage/microsoft/agents/storage/memory_storage.py @@ -17,7 +17,7 @@ def __init__(self, state: dict[str, JSON] = None): async def read( self, keys: list[str], *, target_cls: StoreItemT = None, **kwargs ) -> dict[str, StoreItemT]: - + if not keys: raise ValueError("Storage.read(): Keys are required when reading.") if not target_cls: diff --git a/libraries/Storage/microsoft-agents-storage/microsoft/agents/storage/storage.py b/libraries/Storage/microsoft-agents-storage/microsoft/agents/storage/storage.py index 28372e56..193ecf55 100644 --- a/libraries/Storage/microsoft-agents-storage/microsoft/agents/storage/storage.py +++ b/libraries/Storage/microsoft-agents-storage/microsoft/agents/storage/storage.py @@ -8,12 +8,13 @@ StoreItemT = TypeVar("StoreItemT", bound=StoreItem) + class Storage(Protocol): async def read( self, keys: list[str], *, target_cls: Type[StoreItemT] = None, **kwargs ) -> dict[str, StoreItemT]: """Reads multiple items from storage. - + keys: A list of keys to read. target_cls: The class to deserialize the stored JSON into. Returns a dictionary of key to StoreItem. @@ -24,19 +25,20 @@ async def read( async def write(self, changes: dict[str, StoreItemT]) -> None: """Writes multiple items to storage. - + changes: A dictionary of key to StoreItem to write.""" pass async def delete(self, keys: list[str]) -> None: """Deletes multiple items from storage. - + If a key does not exist, it is ignored. - + keys: A list of keys to delete. """ pass + class AsyncStorageBase(Storage): """Base class for asynchronous storage implementations.""" @@ -49,11 +51,11 @@ async def _read_item( self, key: str, *, target_cls: Type[StoreItemT] = None, **kwargs ) -> tuple[str | None, StoreItemT | None]: """Reads a single item from storage by key. - + Returns a tuple of (key, StoreItem) if found, or (None, None) if not found. """ pass - + async def read( self, keys: list[str], *, target_cls: Type[StoreItemT] = None, **kwargs ) -> dict[str, StoreItemT]: @@ -61,12 +63,12 @@ async def read( raise ValueError("Storage.read(): Keys are required when reading.") if not target_cls: raise ValueError("Storage.read(): target_cls cannot be None.") - + await self.initialize() - items: list[tuple[str | None, StoreItemT | None]] = await gather(*[ - self._read_item(key, target_cls=target_cls, **kwargs) for key in keys - ]) + items: list[tuple[str | None, StoreItemT | None]] = await gather( + *[self._read_item(key, target_cls=target_cls, **kwargs) for key in keys] + ) return {key: value for key, value in items if key is not None} @abstractmethod @@ -77,7 +79,7 @@ async def _write_item(self, key: str, value: StoreItemT) -> None: async def write(self, changes: dict[str, StoreItemT]) -> None: if not changes: raise ValueError("Storage.write(): Changes are required when writing.") - + await self.initialize() await gather(*[self._write_item(key, value) for key, value in changes.items()]) @@ -90,7 +92,7 @@ async def _delete_item(self, key: str) -> None: async def delete(self, keys: list[str]) -> None: if not keys: raise ValueError("Storage.delete(): Keys are required when deleting.") - + await self.initialize() await gather(*[self._delete_item(key) for key in keys]) diff --git a/libraries/Storage/microsoft-agents-storage/microsoft/agents/storage/storage_test_utils.py b/libraries/Storage/microsoft-agents-storage/microsoft/agents/storage/storage_test_utils.py index 9e15f027..9f805dae 100644 --- a/libraries/Storage/microsoft-agents-storage/microsoft/agents/storage/storage_test_utils.py +++ b/libraries/Storage/microsoft-agents-storage/microsoft/agents/storage/storage_test_utils.py @@ -9,30 +9,32 @@ from ._type_aliases import JSON from .memory_storage import MemoryStorage + class MockStoreItem(StoreItem): """Test implementation of StoreItem for testing purposes""" - + def __init__(self, data: dict[str, Any] = None): self.data = data or {} - + def store_item_to_json(self) -> JSON: return self.data - + @staticmethod def from_json_to_store_item(json_data: JSON) -> "MockStoreItem": return MockStoreItem(json_data) - + def __eq__(self, other): if not isinstance(other, MockStoreItem): return False return self.data == other.data - + def __repr__(self): return f"MockStoreItem({self.data})" - + def deepcopy(self): return MockStoreItem(my_deepcopy(self.data)) - + + class MockStoreItemB(MockStoreItem): """Another test implementation of StoreItem for testing purposes""" @@ -41,8 +43,8 @@ def __init__(self, data: dict[str, Any] = None, other_field: bool = True): self.other_field = other_field def store_item_to_json(self) -> JSON: - return [ self.data, self.other_field ] - + return [self.data, self.other_field] + @staticmethod def from_json_to_store_item(json_data: JSON) -> "MockStoreItem": return MockStoreItemB(json_data[0], json_data[1]) @@ -51,10 +53,11 @@ def __eq__(self, other): if not isinstance(other, MockStoreItemB): return False return self.data == other.data and self.other_field == other.other_field - + def deepcopy(self): return MockStoreItemB(my_deepcopy(self.data), self.other_field) - + + def my_deepcopy(original): """Deep copy an object, including StoreItem instances.""" @@ -67,15 +70,16 @@ def my_deepcopy(original): return original.deepcopy() else: return deepcopy(original) - + obj = {} if isinstance(original, dict) else ([None] * len(original)) for key, value in iter_obj: obj[key] = my_deepcopy(value) return obj + def subsets(lst, n=-1): """Generate all subsets of a list up to length n. If n is -1, all subsets are generated. - + Only contiguous subsets are generated. """ if n < 0: @@ -87,6 +91,7 @@ def subsets(lst, n=-1): subsets.append(lst[j:i]) return subsets + class StorageMock(ABC): """A mock wrapper around a Storage implementation to be used in tests.""" @@ -95,17 +100,18 @@ def get_backing_store(self) -> Storage: async def read(self, *args, **kwargs): return await self.get_backing_store().read(*args, **kwargs) - + async def write(self, *args, **kwargs): return await self.get_backing_store().write(*args, **kwargs) - + async def delete(self, *args, **kwargs): return await self.get_backing_store().delete(*args, **kwargs) + # bootstrapping class to compare against # if this class is correct, then the tests are correct class StorageBaseline(Storage): - """"A simple in-memory storage implementation for testing purposes.""" + """ "A simple in-memory storage implementation for testing purposes.""" def __init__(self, initial_data: dict = None): self._memory = deepcopy(initial_data) or {} @@ -114,7 +120,7 @@ def __init__(self, initial_data: dict = None): def read(self, keys: list[str]) -> dict[str, Any]: self._key_history.update(keys) return {key: self._memory.get(key) for key in keys if key in self._memory} - + def write(self, changes: dict[str, Any]) -> None: self._key_history.update(changes.keys()) self._memory.update(changes) @@ -135,61 +141,77 @@ async def equals(self, other) -> bool: for key in self._key_history: if key not in self._memory: if len(await other.read([key], target_cls=MockStoreItem)) > 0: - return False # key should not exist in other + return False # key should not exist in other continue - + # key exists in baseline instance, so let's see if the values match item = self._memory.get(key, None) target_cls = type(item) - res = (await other.read([key], target_cls=target_cls)) + res = await other.read([key], target_cls=target_cls) if key not in res or item != res[key]: return False return True + class StorageTestsCommon(ABC): - KEY_LIST = ([ + KEY_LIST = [ "f", - "a!0dslfj", "\\?/#\t\n\r*", - "527", "test.txt", "_-__--", "VAR", - "None", "multi word key" - ]) + "a!0dslfj", + "\\?/#\t\n\r*", + "527", + "test.txt", + "_-__--", + "VAR", + "None", + "multi word key", + ] READ_KEY_LIST = KEY_LIST + (["5", "20", "100", "nonexistent_key", "-"]) STATE_LIST = [ - { key: MockStoreItem({"id": key, "value": f"value{key}"}) for key in subset } - for subset in subsets(KEY_LIST, 3) if len(subset) == 3 + {key: MockStoreItem({"id": key, "value": f"value{key}"}) for key in subset} + for subset in subsets(KEY_LIST, 3) + if len(subset) == 3 ] @pytest.fixture(params=[dict()] + STATE_LIST) def initial_state(self, request): return request.param - + @pytest.fixture(params=KEY_LIST) def key(self, request): return request.param - @pytest.fixture(params=[subset for subset in subsets(READ_KEY_LIST, 2) if len(subset) == 2]) + @pytest.fixture( + params=[subset for subset in subsets(READ_KEY_LIST, 2) if len(subset) == 2] + ) def keys(self, request): return request.param - + @pytest.fixture(params=subsets(KEY_LIST, 2)) def changes(self, request): changes_obj = {} keys = request.param - changes_obj["new_key"] = MockStoreItemB({"field": "new_value_for_new_key"}, True) + changes_obj["new_key"] = MockStoreItemB( + {"field": "new_value_for_new_key"}, True + ) for i, key in enumerate(keys): if i % 2 == 0: - changes_obj[key] = MockStoreItemB({"data": f"value{key}"}, (i // 2) % 2 == 0) + changes_obj[key] = MockStoreItemB( + {"data": f"value{key}"}, (i // 2) % 2 == 0 + ) else: - changes_obj[key] = MockStoreItem({"id": key, "value": f"new_value_for_{key}"}) + changes_obj[key] = MockStoreItem( + {"id": key, "value": f"new_value_for_{key}"} + ) changes_obj["new_key_2"] = MockStoreItem({"field": "new_value_for_new_key_2"}) return changes_obj + class CRUDStorageTests(StorageTestsCommon): - + async def storage(self, initial_data=None, existing=False): raise NotImplementedError("Subclasses must implement this") @@ -220,7 +242,7 @@ async def test_read_missing_key(self, initial_state): initial_state_copy = my_deepcopy(initial_state) baseline_storage = StorageBaseline(initial_state) storage = await self.storage(initial_state) - keys = [ "5", "20", "100", "nonexistent_key", "-" ] + keys = ["5", "20", "100", "nonexistent_key", "-"] expected = baseline_storage.read(keys) actual = await storage.read(keys, target_cls=MockStoreItem) assert actual == expected @@ -246,7 +268,7 @@ async def test_write_individual(self, initial_state, key): initial_state_copy = my_deepcopy(initial_state) baseline_storage = StorageBaseline(initial_state) storage = await self.storage(initial_state) - change = { key: MockStoreItem ({ key: f"new_value_for_{key}!" }) } + change = {key: MockStoreItem({key: f"new_value_for_{key}!"})} baseline_storage.write(change) await storage.write(change) assert await baseline_storage.equals(storage) @@ -257,11 +279,13 @@ async def test_write_individual_different_target_cls(self, initial_state, key): initial_state_copy = my_deepcopy(initial_state) baseline_storage = StorageBaseline(initial_state) storage = await self.storage(initial_state) - change = { key: MockStoreItemB({ key: f"new_value_for_{key}!" }, other_field=False) } + change = { + key: MockStoreItemB({key: f"new_value_for_{key}!"}, other_field=False) + } baseline_storage.write(change) await storage.write(change) assert await baseline_storage.equals(storage) - change = { key: MockStoreItemB({ key: f"new_{key}" }, other_field=True) } + change = {key: MockStoreItemB({key: f"new_{key}"}, other_field=True)} baseline_storage.write(change) await storage.write(change) assert await baseline_storage.equals(storage) @@ -274,7 +298,7 @@ async def test_write_same_values(self, initial_state): initial_state_copy = my_deepcopy(initial_state) baseline_storage = StorageBaseline(initial_state) storage = await self.storage(initial_state) - changes = { key: value for key, value in initial_state.items() } + changes = {key: value for key, value in initial_state.items()} baseline_storage.write(changes) await storage.write(changes) assert await baseline_storage.equals(storage) @@ -331,7 +355,7 @@ async def test_delete_missing_key(self, initial_state): initial_state_copy = my_deepcopy(initial_state) baseline_storage = StorageBaseline(initial_state) storage = await self.storage(initial_state) - keys = [ "5", "20", "100", "nonexistent_key", "-" ] + keys = ["5", "20", "100", "nonexistent_key", "-"] baseline_storage.delete(keys) await storage.delete(keys) assert await baseline_storage.equals(storage) @@ -358,15 +382,21 @@ async def test_flow(self): changes = { "key_a": MockStoreItem({"id": "key_a", "value": "value_a"}), - "key_b": MockStoreItemB({"id": "key_b", "value": "value_b"}, other_field=False) + "key_b": MockStoreItemB( + {"id": "key_b", "value": "value_b"}, other_field=False + ), } changes_copy = my_deepcopy(changes) baseline_storage.write(changes) await storage.write(changes) - assert (await storage.read(["key_a"], target_cls=MockStoreItem)) == baseline_storage.read(["key_a"]) - assert (await storage.read(["key_b"], target_cls=MockStoreItemB)) == baseline_storage.read(["key_b"]) + assert ( + await storage.read(["key_a"], target_cls=MockStoreItem) + ) == baseline_storage.read(["key_a"]) + assert ( + await storage.read(["key_b"], target_cls=MockStoreItemB) + ) == baseline_storage.read(["key_b"]) assert changes_copy == changes baseline_storage.delete(["key_a"]) @@ -378,21 +408,35 @@ async def test_flow(self): await storage.write(change) assert await baseline_storage.equals(storage) - assert (await storage.read(["key_b"], target_cls=MockStoreItem)) == baseline_storage.read(["key_b"]) + assert ( + await storage.read(["key_b"], target_cls=MockStoreItem) + ) == baseline_storage.read(["key_b"]) with pytest.raises(ValueError): await storage.read([], target_cls=MockStoreItem) with pytest.raises(ValueError): await storage.read(["key_b"], target_cls=None) - change = {"key_c": MockStoreItemB({"id": "key_c", "value": "value_c"}, other_field=True)} + change = { + "key_c": MockStoreItemB( + {"id": "key_c", "value": "value_c"}, other_field=True + ) + } baseline_storage.write(change) await storage.write(change) - assert (await storage.read(["key_a", "key_b"], target_cls=MockStoreItem)) == baseline_storage.read(["key_a", "key_b"]) - assert (await storage.read(["key_a", "key_c"], target_cls=MockStoreItemB)) == baseline_storage.read(["key_a", "key_c"]) - - item_parent_class = (await storage.read(["key_c"], target_cls=MockStoreItem))["key_c"] - item_child_class = (await storage.read(["key_c"], target_cls=MockStoreItemB))["key_c"] + assert ( + await storage.read(["key_a", "key_b"], target_cls=MockStoreItem) + ) == baseline_storage.read(["key_a", "key_b"]) + assert ( + await storage.read(["key_a", "key_c"], target_cls=MockStoreItemB) + ) == baseline_storage.read(["key_a", "key_c"]) + + item_parent_class = (await storage.read(["key_c"], target_cls=MockStoreItem))[ + "key_c" + ] + item_child_class = (await storage.read(["key_c"], target_cls=MockStoreItemB))[ + "key_c" + ] assert item_parent_class.data[0] == item_child_class.data assert item_child_class.other_field == True @@ -407,4 +451,4 @@ async def test_flow(self): del storage gc.collect() storage_alt = await self.storage(existing=True) - assert await baseline_storage.equals(storage_alt) \ No newline at end of file + assert await baseline_storage.equals(storage_alt) diff --git a/libraries/Storage/microsoft-agents-storage/tests/test_error_handling.py b/libraries/Storage/microsoft-agents-storage/tests/test_error_handling.py index d26078cf..bb644c67 100644 --- a/libraries/Storage/microsoft-agents-storage/tests/test_error_handling.py +++ b/libraries/Storage/microsoft-agents-storage/tests/test_error_handling.py @@ -1,31 +1,37 @@ import pytest from microsoft.agents.storage.error_handling import ignore_error, is_status_code_error + class CustomError(Exception): def __init__(self, status_code: int): self.status_code = status_code + async def raise_custom_error(code: int): raise CustomError(code) + @pytest.mark.asyncio async def test_ignore_error_without_error(): async def func(): return 42 - + assert await ignore_error(func(), lambda e: False) == 42 assert await ignore_error(func(), lambda e: True) == 42 + @pytest.mark.asyncio async def test_ignore_error_with_error(): with pytest.raises(CustomError): await ignore_error(raise_custom_error(500), lambda e: False) + @pytest.mark.asyncio async def test_ignore_error_with_ignored_error(): assert await ignore_error(raise_custom_error(500), lambda e: True) is None + @pytest.mark.asyncio async def test_is_status_code_with_status_code_check(): @@ -33,10 +39,15 @@ async def func(): return 42 assert await ignore_error(func(), is_status_code_error(404)) == 42 - assert await ignore_error(raise_custom_error(403), is_status_code_error(403)) is None + assert ( + await ignore_error(raise_custom_error(403), is_status_code_error(403)) is None + ) with pytest.raises(CustomError) as err: - assert await ignore_error(raise_custom_error(404), is_status_code_error(500)) is None + assert ( + await ignore_error(raise_custom_error(404), is_status_code_error(500)) + is None + ) assert err.value.status_code == 404 @@ -44,4 +55,4 @@ async def raise_exception(): raise Exception() with pytest.raises(Exception): - await ignore_error(raise_exception, is_status_code_error(404)) \ No newline at end of file + await ignore_error(raise_exception, is_status_code_error(404)) diff --git a/libraries/Storage/microsoft-agents-storage/tests/test_memory_storage.py b/libraries/Storage/microsoft-agents-storage/tests/test_memory_storage.py index 747c22e5..f7b59aa1 100644 --- a/libraries/Storage/microsoft-agents-storage/tests/test_memory_storage.py +++ b/libraries/Storage/microsoft-agents-storage/tests/test_memory_storage.py @@ -1,19 +1,22 @@ from microsoft.agents.storage.memory_storage import MemoryStorage from microsoft.agents.storage.storage_test_utils import CRUDStorageTests, StorageMock + class MemoryStorageMock(StorageMock): def __init__(self, initial_data: dict = None): data = { - key: value.store_item_to_json() for key, value in (initial_data or {}).items() + key: value.store_item_to_json() + for key, value in (initial_data or {}).items() } self.storage = MemoryStorage(data) def get_backing_store(self): return self.storage + class TestMemoryStorage(CRUDStorageTests): - - async def storage(self, initial_state = None): - return MemoryStorageMock(initial_state) \ No newline at end of file + + async def storage(self, initial_state=None): + return MemoryStorageMock(initial_state) diff --git a/libraries/Storage/microsoft-agents-storage/tests/test_utils.py b/libraries/Storage/microsoft-agents-storage/tests/test_utils.py index 7e70adc8..3ea0b83f 100644 --- a/libraries/Storage/microsoft-agents-storage/tests/test_utils.py +++ b/libraries/Storage/microsoft-agents-storage/tests/test_utils.py @@ -1,10 +1,18 @@ -from microsoft.agents.storage.storage_test_utils import MockStoreItem, MockStoreItemB, my_deepcopy, subsets +from microsoft.agents.storage.storage_test_utils import ( + MockStoreItem, + MockStoreItemB, + my_deepcopy, + subsets, +) + def eq_helper(a, b): def key(x): return str(x) + return sorted(a, key=key) == sorted(b, key=key) + def test_eq_helper(): a1 = [{"a": 1}, {"b": 2}, {"c": 3}] @@ -19,17 +27,21 @@ def test_eq_helper(): a2 = [[4], [3], [3], ["a"]] assert eq_helper(a1, a2) + def test_my_deepcopy(): original = { "a": MockStoreItem({"id": "a", "value": 1}), "b": { "b1": MockStoreItemB({"key": "b1"}, other_field=False), "b2": [1, 2, 3], - "b3": { "nested": MockStoreItem({"id": "nested", "value": 42}) } + "b3": {"nested": MockStoreItem({"id": "nested", "value": 42})}, }, - "c": [ MockStoreItem({"id": "c1"}), MockStoreItemB({"id": "c2"}, other_field=True) ], + "c": [ + MockStoreItem({"id": "c1"}), + MockStoreItemB({"id": "c2"}, other_field=True), + ], "d": "just a string", - "e": 12345 + "e": 12345, } copy = my_deepcopy(original) assert copy == original @@ -45,35 +57,37 @@ def test_my_deepcopy(): assert copy["d"] == original["d"] assert copy["e"] == original["e"] + def test_subsets(): - assert eq_helper(subsets(["a", "b", "c"], -1,), [ - ["a"], ["a", "b"], ["a", "b", "c"], - ["b"], ["b", "c"], - ["c"] - ]) - assert eq_helper(subsets(["a", "b", "c"]), [ - ["a"], ["a", "b"], ["a", "b", "c"], - ["b"], ["b", "c"], - ["c"] - ]) + assert eq_helper( + subsets( + ["a", "b", "c"], + -1, + ), + [["a"], ["a", "b"], ["a", "b", "c"], ["b"], ["b", "c"], ["c"]], + ) + assert eq_helper( + subsets(["a", "b", "c"]), + [["a"], ["a", "b"], ["a", "b", "c"], ["b"], ["b", "c"], ["c"]], + ) + def test_subsets_0(): assert subsets(["a", "b", "c", "d"], 0) == [] + def test_subsets_1(): - assert eq_helper(subsets(["a", "b", "c", 3, 2], 1), [ - ["a"], ["b"], ["c"], [3], [2] - ]) + assert eq_helper(subsets(["a", "b", "c", 3, 2], 1), [["a"], ["b"], ["c"], [3], [2]]) + def test_subsets_2(): - assert eq_helper(subsets(["a", "b", "c"], 2), [ - ["a"], ["b"], ["c"], - ["a", "b"], ["b", "c"] - ]) + assert eq_helper( + subsets(["a", "b", "c"], 2), [["a"], ["b"], ["c"], ["a", "b"], ["b", "c"]] + ) + def test_subsets_3(): - assert eq_helper(subsets(["a", "b", "c"]), [ - ["a"], ["a", "b"], ["a", "b", "c"], - ["b"], ["b", "c"], - ["c"] - ]) \ No newline at end of file + assert eq_helper( + subsets(["a", "b", "c"]), + [["a"], ["a", "b"], ["a", "b", "c"], ["b"], ["b", "c"], ["c"]], + ) From ac7e4f987ca08f5e78d5614fecaf659023307396 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20Brand=C3=A3o?= Date: Thu, 17 Jul 2025 14:50:14 -0700 Subject: [PATCH 3/4] Removed unsupported typing syntax for Python 3.9 --- .../microsoft/agents/blob/blob_storage.py | 5 ++--- .../microsoft/agents/storage/storage.py | 6 +++--- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/libraries/Storage/microsoft-agents-blob/microsoft/agents/blob/blob_storage.py b/libraries/Storage/microsoft-agents-blob/microsoft/agents/blob/blob_storage.py index 0fb021b9..39c54a52 100644 --- a/libraries/Storage/microsoft-agents-blob/microsoft/agents/blob/blob_storage.py +++ b/libraries/Storage/microsoft-agents-blob/microsoft/agents/blob/blob_storage.py @@ -1,6 +1,5 @@ import json - -from typing import TypeVar +from typing import TypeVar, Union from io import BytesIO from azure.storage.blob.aio import ( @@ -59,7 +58,7 @@ async def initialize(self) -> None: async def _read_item( self, key: str, *, target_cls: StoreItemT = None, **kwargs - ) -> tuple[str | None, StoreItemT | None]: + ) -> tuple[Union[str, None], Union[StoreItemT, None]]: item = await ignore_error( self._container_client.download_blob(blob=key), is_status_code_error(404), diff --git a/libraries/Storage/microsoft-agents-storage/microsoft/agents/storage/storage.py b/libraries/Storage/microsoft-agents-storage/microsoft/agents/storage/storage.py index 193ecf55..e9b0fcdd 100644 --- a/libraries/Storage/microsoft-agents-storage/microsoft/agents/storage/storage.py +++ b/libraries/Storage/microsoft-agents-storage/microsoft/agents/storage/storage.py @@ -1,4 +1,4 @@ -from typing import Protocol, TypeVar, Type +from typing import Protocol, TypeVar, Type, Union from abc import ABC, abstractmethod from asyncio import gather @@ -49,7 +49,7 @@ async def initialize(self) -> None: @abstractmethod async def _read_item( self, key: str, *, target_cls: Type[StoreItemT] = None, **kwargs - ) -> tuple[str | None, StoreItemT | None]: + ) -> tuple[Union[str, None], Union[StoreItemT, None]]: """Reads a single item from storage by key. Returns a tuple of (key, StoreItem) if found, or (None, None) if not found. @@ -66,7 +66,7 @@ async def read( await self.initialize() - items: list[tuple[str | None, StoreItemT | None]] = await gather( + items: list[tuple[Union[str, None], Union[StoreItemT, None]]] = await gather( *[self._read_item(key, target_cls=target_cls, **kwargs) for key in keys] ) return {key: value for key, value in items if key is not None} From 508a2cfd4c36815cf62e0de13c836f533614f297 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20Brand=C3=A3o?= Date: Thu, 17 Jul 2025 14:53:20 -0700 Subject: [PATCH 4/4] Fixed usage of storage interface in test case --- .../Builder/microsoft-agents-builder/tests/test_agent_state.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/Builder/microsoft-agents-builder/tests/test_agent_state.py b/libraries/Builder/microsoft-agents-builder/tests/test_agent_state.py index 250e59c4..40f5079a 100644 --- a/libraries/Builder/microsoft-agents-builder/tests/test_agent_state.py +++ b/libraries/Builder/microsoft-agents-builder/tests/test_agent_state.py @@ -491,7 +491,7 @@ async def test_memory_storage_integration(self): # Verify data exists in memory storage storage_key = user_state.get_storage_key(self.context) - stored_data = await memory_storage.read([storage_key]) + stored_data = await memory_storage.read([storage_key], target_cls=TestDataItem) assert storage_key in stored_data assert stored_data[storage_key] is not None