From 8510179d20dfa3cdc00a8c0369ba18cd68fad889 Mon Sep 17 00:00:00 2001 From: Pouria Khalaj Date: Fri, 31 May 2024 18:45:31 +0200 Subject: [PATCH 01/26] Add tests for `get_id()` and `get_ids()` --- trolldb/test_utils/mongodb_database.py | 11 ++++++++--- trolldb/tests/tests_api/test_api.py | 2 +- trolldb/tests/tests_database/test_mongodb.py | 18 +++++++++++++++++- 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/trolldb/test_utils/mongodb_database.py b/trolldb/test_utils/mongodb_database.py index 3856348..57dab77 100644 --- a/trolldb/test_utils/mongodb_database.py +++ b/trolldb/test_utils/mongodb_database.py @@ -202,7 +202,7 @@ def write_test_data(cls) -> None: collection.insert_many(cls.documents) @classmethod - def get_all_documents_from_database(cls) -> list[dict]: + def get_documents_from_database(cls) -> list[dict]: """Retrieves all the documents from the database. Returns: @@ -218,6 +218,11 @@ def get_all_documents_from_database(cls) -> list[dict]: documents = list(collection.find({})) return documents + @classmethod + def get_document_ids_from_database(cls) -> list[str]: + """Retrieves all the document IDs from the database.""" + return [str(doc["_id"]) for doc in cls.get_documents_from_database()] + @classmethod def find_min_max_datetime(cls) -> dict[str, dict]: """Finds the minimum and the maximum for both the ``start_time`` and the ``end_time``. @@ -238,7 +243,7 @@ def find_min_max_datetime(cls) -> dict[str, dict]: _max=dict(_id=None, _time="1900-01-01T00:00:00")) ) - documents = cls.get_all_documents_from_database() + documents = cls.get_documents_from_database() for document in documents: for k in ["start_time", "end_time"]: @@ -290,7 +295,7 @@ def match_query(cls, platform=None, sensor=None, time_min=None, time_max=None) - we end up with those that match. When a query is ``None``, it does not have any effect on the results. This method will be used in testing the ``/queries`` route of the API. """ - documents = cls.get_all_documents_from_database() + documents = cls.get_documents_from_database() buffer = deepcopy(documents) for document in documents: diff --git a/trolldb/tests/tests_api/test_api.py b/trolldb/tests/tests_api/test_api.py index 5e47cf2..f96ecca 100644 --- a/trolldb/tests/tests_api/test_api.py +++ b/trolldb/tests/tests_api/test_api.py @@ -96,7 +96,7 @@ def test_queries_all(): """Tests that the queries route returns all documents when no actual queries are given.""" assert document_ids_are_correct( http_get("queries").json(), - [str(doc["_id"]) for doc in TestDatabase.get_all_documents_from_database()] + TestDatabase.get_document_ids_from_database() ) diff --git a/trolldb/tests/tests_database/test_mongodb.py b/trolldb/tests/tests_database/test_mongodb.py index 78958c1..9cc1c3e 100644 --- a/trolldb/tests/tests_database/test_mongodb.py +++ b/trolldb/tests/tests_database/test_mongodb.py @@ -8,13 +8,16 @@ import errno import time +from collections import Counter import pytest +from bson import ObjectId from pydantic import AnyUrl from pymongo.errors import InvalidOperation -from trolldb.database.mongodb import DatabaseConfig, MongoDB, mongodb_context +from trolldb.database.mongodb import DatabaseConfig, MongoDB, get_id, get_ids, mongodb_context from trolldb.test_utils.common import test_app_config +from trolldb.test_utils.mongodb_database import TestDatabase async def test_connection_timeout_negative(): @@ -89,3 +92,16 @@ async def test_main_database(mongodb_fixture): assert MongoDB.main_database() is not None assert MongoDB.main_database().name == test_app_config.database.main_database_name assert MongoDB.main_database() == await MongoDB.get_database(test_app_config.database.main_database_name) + + +async def test_get_id(mongodb_fixture): + """Tests :func:`trolldb.database.mongodb.get_id` using all documents (one at a time).""" + for _id in TestDatabase.get_document_ids_from_database(): + doc = MongoDB.main_collection().find_one({"_id": ObjectId(_id)}) + assert await get_id(doc) == _id + + +async def test_get_ids(mongodb_fixture): + """Tests :func:`trolldb.database.mongodb.get_ids` using all documents in one pass.""" + docs = MongoDB.main_collection().find({}) + assert Counter(await get_ids(docs)) == Counter(TestDatabase.get_document_ids_from_database()) From c0dee660253e7ab365010b82997367e5ad44c6ae Mon Sep 17 00:00:00 2001 From: Pouria Khalaj Date: Sat, 1 Jun 2024 20:05:54 +0200 Subject: [PATCH 02/26] Fix the bug: order of message and status This concerns two methods in the `ResponseError` class. This bug came to my attention while adding more tests! --- trolldb/errors/errors.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/trolldb/errors/errors.py b/trolldb/errors/errors.py index e0ea80b..6142ab4 100644 --- a/trolldb/errors/errors.py +++ b/trolldb/errors/errors.py @@ -204,7 +204,7 @@ def log_as_warning( extra_information: dict | None = None, status_code: int | None = None) -> None: """Same as :func:`~ResponseError.get_error_details` but logs the error as a warning and returns ``None``.""" - msg, _ = self.get_error_details(extra_information, status_code) + _, msg = self.get_error_details(extra_information, status_code) logger.warning(msg) def sys_exit_log( @@ -224,7 +224,7 @@ def sys_exit_log( Returns: Does not return anything, but logs the error and exits the program. """ - msg, _ = self.get_error_details(extra_information, status_code) + _, msg = self.get_error_details(extra_information, status_code) logger.error(msg) exit(exit_code) From 2e8e02576b8802c3de6f2f0e8d3a54374dcb7e05 Mon Sep 17 00:00:00 2001 From: Pouria Khalaj Date: Sat, 1 Jun 2024 20:08:55 +0200 Subject: [PATCH 03/26] Override the `caplog` fixture in pytest This is needed as we are using `loguru` instead of the Python built-in logging package. --- trolldb/tests/conftest.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/trolldb/tests/conftest.py b/trolldb/tests/conftest.py index 6a1e80c..ef252d0 100644 --- a/trolldb/tests/conftest.py +++ b/trolldb/tests/conftest.py @@ -5,6 +5,8 @@ import pytest import pytest_asyncio +from _pytest.logging import LogCaptureFixture +from loguru import logger from trolldb.api.api import api_server_process_context from trolldb.database.mongodb import mongodb_context @@ -13,6 +15,25 @@ from trolldb.test_utils.mongodb_instance import running_prepared_database_context +@pytest.fixture() +def caplog(caplog: LogCaptureFixture): + """This overrides the actual pytest ``caplog`` fixture. + + Reason: + We are using ``loguru`` instead of the Python built-in logging package. More information at: + https://loguru.readthedocs.io/en/latest/resources/migration.html#replacing-caplog-fixture-from-pytest-library + """ + handler_id = logger.add( + caplog.handler, + format="{message}", + level=0, + filter=lambda record: record["level"].no >= caplog.handler.level, + enqueue=True, # Set to 'True' if your test is spawning child processes. + ) + yield caplog + logger.remove(handler_id) + + @pytest.fixture(scope="session") def _run_mongodb_server_instance(): """Encloses all tests (session scope) in a context manager of a running MongoDB instance (in a separate process).""" From ad4e143652cb33fce4b190e8cc5df8644dc257f6 Mon Sep 17 00:00:00 2001 From: Pouria Khalaj Date: Sun, 2 Jun 2024 17:31:01 +0200 Subject: [PATCH 04/26] Set `enqueue=False` in `conftest.py` Our test is not spawning child processes. --- trolldb/tests/conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/trolldb/tests/conftest.py b/trolldb/tests/conftest.py index ef252d0..dbe1288 100644 --- a/trolldb/tests/conftest.py +++ b/trolldb/tests/conftest.py @@ -28,7 +28,7 @@ def caplog(caplog: LogCaptureFixture): format="{message}", level=0, filter=lambda record: record["level"].no >= caplog.handler.level, - enqueue=True, # Set to 'True' if your test is spawning child processes. + enqueue=False, # Set to 'True' if your test is spawning child processes. ) yield caplog logger.remove(handler_id) From 3aba9e655d36116506a7a6def1be0be5a42fa2af Mon Sep 17 00:00:00 2001 From: Pouria Khalaj Date: Sun, 2 Jun 2024 17:43:58 +0200 Subject: [PATCH 05/26] Fix the bug: store the `database_config` once the client is initialized This was missing --- trolldb/database/mongodb.py | 1 + 1 file changed, 1 insertion(+) diff --git a/trolldb/database/mongodb.py b/trolldb/database/mongodb.py index 4f8f3bc..b4d676b 100644 --- a/trolldb/database/mongodb.py +++ b/trolldb/database/mongodb.py @@ -178,6 +178,7 @@ async def initialize(cls, database_config: DatabaseConfig): logger.info("The main collection name exists.") cls.__main_collection = cls.__main_database.get_collection(database_config.main_collection_name) + cls.__database_config = database_config logger.info("MongoDB is successfully initialized.") @classmethod From 79a67e6d725f83db1f31b1bc56982205e8af4aca Mon Sep 17 00:00:00 2001 From: Pouria Khalaj Date: Sun, 2 Jun 2024 17:40:40 +0200 Subject: [PATCH 06/26] Add `is_initialized()` method to the `MongoDB` class This is to ensure that we are calling `MongoDB.close()` only if the client has been properly initialized. --- trolldb/database/mongodb.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/trolldb/database/mongodb.py b/trolldb/database/mongodb.py index b4d676b..31f0933 100644 --- a/trolldb/database/mongodb.py +++ b/trolldb/database/mongodb.py @@ -181,6 +181,11 @@ async def initialize(cls, database_config: DatabaseConfig): cls.__database_config = database_config logger.info("MongoDB is successfully initialized.") + @classmethod + def is_initialized(cls) -> bool: + """Checks if the motor client is initialized.""" + return cls.__client is not None + @classmethod def close(cls) -> None: """Closes the motor client.""" @@ -309,5 +314,8 @@ async def mongodb_context(database_config: DatabaseConfig) -> AsyncGenerator: await MongoDB.initialize(database_config) yield finally: - MongoDB.close() + if MongoDB.is_initialized(): + MongoDB.close() + else: + logger.info("The MongoDB client was not initialized!") logger.info("The MongoDB context manager is successfully closed.") From 71615057d9f01c73f0691d63c0558a1262ce5d9c Mon Sep 17 00:00:00 2001 From: Pouria Khalaj Date: Sun, 2 Jun 2024 17:45:32 +0200 Subject: [PATCH 07/26] Change the name of `make_test_app_config_as_dict()` and update docstring This is to clarify that the return value cannot be readily used in the initialization of MongoDB. As a result, `test_recorder.py` has been updated accordingly with the new `make_test_app_config_as_dict()`. --- trolldb/test_utils/common.py | 15 ++++++++++----- trolldb/tests/test_recorder.py | 8 ++++---- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/trolldb/test_utils/common.py b/trolldb/test_utils/common.py index 2007977..1643b64 100644 --- a/trolldb/test_utils/common.py +++ b/trolldb/test_utils/common.py @@ -10,8 +10,8 @@ from trolldb.config.config import AppConfig -def make_test_app_config(subscriber_address: Optional[FilePath] = None) -> dict[str, dict]: - """Makes the app configuration when used in testing. +def make_test_app_config_as_dict(subscriber_address: Optional[FilePath] = None) -> dict[str, dict]: + """Makes the app configuration (as a dictionary) when used in testing. Args: subscriber_address: @@ -19,7 +19,12 @@ def make_test_app_config(subscriber_address: Optional[FilePath] = None) -> dict[ config will be an empty dictionary. Returns: - A dictionary which resembles an object of type :obj:`~trolldb.config.config.AppConfig`. + A dictionary with a structure similar to that of an :obj:`~trolldb.config.config.AppConfig` object. + + Warning: + The return value of this function is a dictionary and not accepted as a valid input argument for + :func:`trolldb.database.mongodb.MongoDB.initialize`. As a result, one must cast it to the valid type by e.g. + ``AppConfig(**make_test_app_config_as_dict())``. """ app_config = dict( api_server=dict( @@ -41,7 +46,7 @@ def make_test_app_config(subscriber_address: Optional[FilePath] = None) -> dict[ return app_config -test_app_config = AppConfig(**make_test_app_config()) +test_app_config = AppConfig(**make_test_app_config_as_dict()) """The app configs for testing purposes assuming an empty configuration for the subscriber.""" @@ -49,7 +54,7 @@ def create_config_file(config_path: FilePath) -> FilePath: """Creates a config file for tests.""" config_file = config_path / "config.yaml" with open(config_file, "w") as f: - yaml.safe_dump(make_test_app_config(config_path), f) + yaml.safe_dump(make_test_app_config_as_dict(config_path), f) return config_file diff --git a/trolldb/tests/test_recorder.py b/trolldb/tests/test_recorder.py index 7bacd3d..858182f 100644 --- a/trolldb/tests/test_recorder.py +++ b/trolldb/tests/test_recorder.py @@ -12,7 +12,7 @@ record_messages_from_config, ) from trolldb.database.mongodb import MongoDB, mongodb_context -from trolldb.test_utils.common import AppConfig, create_config_file, make_test_app_config, test_app_config +from trolldb.test_utils.common import AppConfig, create_config_file, make_test_app_config_as_dict, test_app_config from trolldb.test_utils.mongodb_instance import running_prepared_database_context @@ -91,7 +91,7 @@ async def message_in_database_and_delete_count_is_one(msg: Message) -> bool: async def test_record_messages(config_file, tmp_path, file_message, tmp_data_filename): """Tests that message recording adds a message to the database.""" - config = AppConfig(**make_test_app_config(tmp_path)) + config = AppConfig(**make_test_app_config_as_dict(tmp_path)) msg = Message.decode(file_message) with running_prepared_database_context(): with patched_subscriber_recv([file_message]): @@ -101,7 +101,7 @@ async def test_record_messages(config_file, tmp_path, file_message, tmp_data_fil async def test_record_deletes_message(tmp_path, file_message, del_message): """Tests that message recording can delete a record in the database.""" - config = AppConfig(**make_test_app_config(tmp_path)) + config = AppConfig(**make_test_app_config_as_dict(tmp_path)) with running_prepared_database_context(): with patched_subscriber_recv([file_message, del_message]): await record_messages(config) @@ -113,7 +113,7 @@ async def test_record_deletes_message(tmp_path, file_message, del_message): async def test_record_dataset_messages(tmp_path, dataset_message): """Tests recording a dataset message and deleting the file.""" - config = AppConfig(**make_test_app_config(tmp_path)) + config = AppConfig(**make_test_app_config_as_dict(tmp_path)) msg = Message.decode(dataset_message) with running_prepared_database_context(): with patched_subscriber_recv([dataset_message]): From 742b84a84dc0ac7c19094aab28d8c98ac1bc05d5 Mon Sep 17 00:00:00 2001 From: Pouria Khalaj Date: Sun, 2 Jun 2024 17:50:35 +0200 Subject: [PATCH 08/26] Add `@validate_call` to `MongoDB.initialize()` A test will be added in the next commit. --- trolldb/database/mongodb.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/trolldb/database/mongodb.py b/trolldb/database/mongodb.py index 31f0933..1a652c3 100644 --- a/trolldb/database/mongodb.py +++ b/trolldb/database/mongodb.py @@ -106,6 +106,7 @@ class MongoDB: """MongoDB creates these databases by default for self usage.""" @classmethod + @validate_call async def initialize(cls, database_config: DatabaseConfig): """Initializes the motor client. Note that this method has to be awaited! @@ -121,6 +122,9 @@ async def initialize(cls, database_config: DatabaseConfig): On success ``None``. Raises: + ValidationError: + If the method is not called with arguments of valid type. + SystemExit(errno.EIO): If connection is not established, i.e. ``ConnectionFailure``. SystemExit(errno.EIO): From 9c4687d73d5a1b44c5d9939f0c5d474aeb8eecfc Mon Sep 17 00:00:00 2001 From: Pouria Khalaj Date: Sun, 2 Jun 2024 17:56:10 +0200 Subject: [PATCH 09/26] Add new tests for the `MongoDB.initialize()` method We also improve the structure of previous tests in the module. In particular, we now assert the log messages and use parametrization. --- trolldb/tests/tests_database/test_mongodb.py | 94 ++++++++++++++------ 1 file changed, 65 insertions(+), 29 deletions(-) diff --git a/trolldb/tests/tests_database/test_mongodb.py b/trolldb/tests/tests_database/test_mongodb.py index 9cc1c3e..8a19f3f 100644 --- a/trolldb/tests/tests_database/test_mongodb.py +++ b/trolldb/tests/tests_database/test_mongodb.py @@ -12,52 +12,88 @@ import pytest from bson import ObjectId -from pydantic import AnyUrl +from pydantic import AnyUrl, ValidationError from pymongo.errors import InvalidOperation +from trolldb.database.errors import Client from trolldb.database.mongodb import DatabaseConfig, MongoDB, get_id, get_ids, mongodb_context -from trolldb.test_utils.common import test_app_config +from trolldb.errors.errors import ResponseError +from trolldb.test_utils.common import make_test_app_config_as_dict, test_app_config from trolldb.test_utils.mongodb_database import TestDatabase -async def test_connection_timeout_negative(): - """Expect to see the connection attempt times out since the MongoDB URL is invalid.""" - timeout = 3 +async def test_connection_timeout_negative(caplog): + """Tests that the connection attempt times out after the expected time, since the MongoDB URL is invalid.""" + invalid_config = DatabaseConfig( + url=AnyUrl("mongodb://invalid_url_that_does_not_exist:8000"), + timeout=3, + main_database_name=test_app_config.database.main_database_name, + main_collection_name=test_app_config.database.main_collection_name, + ) + t1 = time.time() - with pytest.raises(SystemExit) as pytest_wrapped_e: - async with mongodb_context( - DatabaseConfig(url=AnyUrl("mongodb://invalid_url_that_does_not_exist:8000"), - timeout=timeout, main_database_name=" ", main_collection_name=" ")): + with pytest.raises(SystemExit) as exc: + async with mongodb_context(invalid_config): pass t2 = time.time() - assert pytest_wrapped_e.value.code == errno.EIO - assert t2 - t1 >= timeout + + assert exc.value.code == errno.EIO + assert check_log(caplog, "ERROR", Client.ConnectionError) + assert t2 - t1 >= invalid_config.timeout +@pytest.mark.parametrize("invalid_config", [ + dict(main_database_name=test_app_config.database.main_database_name, main_collection_name=" "), + dict(main_database_name=" ", main_collection_name=test_app_config.database.main_collection_name) +]) @pytest.mark.usefixtures("_run_mongodb_server_instance") -async def test_main_database_negative(): - """Expect to fail when giving an invalid name for the main database, given a valid collection name.""" - with pytest.raises(SystemExit) as pytest_wrapped_e: - async with mongodb_context(DatabaseConfig( - timeout=1, - url=test_app_config.database.url, - main_database_name=" ", - main_collection_name=test_app_config.database.main_collection_name)): +async def test_main_database_and_collection_negative(invalid_config): + """Tests that we fail when the name of the main database/collection is invalid, given a valid name for the other.""" + config = dict(timeout=1, url=test_app_config.database.url) | invalid_config + with pytest.raises(SystemExit) as exc: + async with mongodb_context(DatabaseConfig(**config)): pass - assert pytest_wrapped_e.value.code == errno.ENODATA + assert exc.value.code == errno.ENODATA + + +@pytest.mark.usefixtures("_run_mongodb_server_instance") +async def test_reinitialize_different_config_negative(caplog): + """Tests that we fail when trying to reinitialize with a different configuration.""" + different_config = DatabaseConfig(**(make_test_app_config_as_dict()["database"] | {"timeout": 0.1})) + with pytest.raises(SystemExit) as exc: + async with mongodb_context(test_app_config.database): + await MongoDB.initialize(different_config) + + assert exc.value.code == errno.EIO + assert check_log(caplog, "ERROR", Client.ReinitializeConfigError) +@pytest.mark.parametrize("config_with_wrong_type", [ + 1, "1", 1.0, {}, None, [], (), make_test_app_config_as_dict() +]) @pytest.mark.usefixtures("_run_mongodb_server_instance") -async def test_main_collection_negative(): - """Expect to fail when giving an invalid name for the main collection, given a valid database name.""" - with pytest.raises(SystemExit) as pytest_wrapped_e: - async with mongodb_context(DatabaseConfig( - timeout=1, - url=test_app_config.database.url, - main_database_name=test_app_config.database.main_database_name, - main_collection_name=" ")): +async def test_invalid_config_type(caplog, config_with_wrong_type): + """Tests that we fail when trying to initialize with a configuration of wrong type.""" + with pytest.raises(ValidationError): + async with mongodb_context(config_with_wrong_type): pass - assert pytest_wrapped_e.value.code == errno.ENODATA + + +def check_log(caplog, level: str, response_error: ResponseError) -> bool: + """An auxiliary function to check the log message.""" + for rec in caplog.records: + if rec.levelname == level and (response_error.get_error_details()[1] in rec.message): + return True + return False + + +@pytest.mark.usefixtures("_run_mongodb_server_instance") +async def test_reinitialize_same_config_warning(caplog): + """Tests the log (warning) when trying to reinitialize with the same configuration.""" + async with mongodb_context(test_app_config.database): + await MongoDB.initialize(test_app_config.database) + + assert check_log(caplog, "WARNING", Client.AlreadyOpenError) async def test_get_client(mongodb_fixture): From a206fbaeb0abbfe43698a4b060db599392392b78 Mon Sep 17 00:00:00 2001 From: Pouria Khalaj Date: Sun, 2 Jun 2024 19:31:36 +0200 Subject: [PATCH 10/26] Fix the bug: Set `MongoDB.__client=None` once it is closed --- trolldb/database/mongodb.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/trolldb/database/mongodb.py b/trolldb/database/mongodb.py index 1a652c3..b8ad2d8 100644 --- a/trolldb/database/mongodb.py +++ b/trolldb/database/mongodb.py @@ -195,8 +195,9 @@ def close(cls) -> None: """Closes the motor client.""" logger.info("Attempt to close the MongoDB client ...") if cls.__client: - cls.__database_config = None cls.__client.close() + cls.__client = None + cls.__database_config = None logger.info("The MongoDB client is closed successfully.") return Client.CloseNotAllowedError.sys_exit_log(errno.EIO) From fc74639e2d35902faf2a0865e7347902a98e7a53 Mon Sep 17 00:00:00 2001 From: Pouria Khalaj Date: Sun, 2 Jun 2024 19:41:21 +0200 Subject: [PATCH 11/26] Add a test for testing `MongoDB.close()` without initialization Moreover, perform some minor refactoring of the tests in the module. --- trolldb/tests/tests_database/test_mongodb.py | 72 ++++++++++++-------- 1 file changed, 42 insertions(+), 30 deletions(-) diff --git a/trolldb/tests/tests_database/test_mongodb.py b/trolldb/tests/tests_database/test_mongodb.py index 8a19f3f..976bac8 100644 --- a/trolldb/tests/tests_database/test_mongodb.py +++ b/trolldb/tests/tests_database/test_mongodb.py @@ -13,9 +13,8 @@ import pytest from bson import ObjectId from pydantic import AnyUrl, ValidationError -from pymongo.errors import InvalidOperation -from trolldb.database.errors import Client +from trolldb.database.errors import Client, Collections, Databases from trolldb.database.mongodb import DatabaseConfig, MongoDB, get_id, get_ids, mongodb_context from trolldb.errors.errors import ResponseError from trolldb.test_utils.common import make_test_app_config_as_dict, test_app_config @@ -42,28 +41,38 @@ async def test_connection_timeout_negative(caplog): assert t2 - t1 >= invalid_config.timeout -@pytest.mark.parametrize("invalid_config", [ - dict(main_database_name=test_app_config.database.main_database_name, main_collection_name=" "), - dict(main_database_name=" ", main_collection_name=test_app_config.database.main_collection_name) +def check_log(caplog, level: str, response_error: ResponseError) -> bool: + """An auxiliary function to check the log message.""" + for rec in caplog.records: + if rec.levelname == level and (response_error.get_error_details()[1] in rec.message): + return True + return False + + +@pytest.mark.parametrize(("error", "invalid_config"), [( + Collections.NotFoundError, + dict(main_database_name=test_app_config.database.main_database_name, main_collection_name=" ")), + ( + Databases.NotFoundError, + dict(main_database_name=" ", main_collection_name=test_app_config.database.main_collection_name)) ]) @pytest.mark.usefixtures("_run_mongodb_server_instance") -async def test_main_database_and_collection_negative(invalid_config): +async def test_main_database_and_collection_negative(caplog, error, invalid_config): """Tests that we fail when the name of the main database/collection is invalid, given a valid name for the other.""" config = dict(timeout=1, url=test_app_config.database.url) | invalid_config with pytest.raises(SystemExit) as exc: async with mongodb_context(DatabaseConfig(**config)): pass assert exc.value.code == errno.ENODATA + assert check_log(caplog, "ERROR", error) -@pytest.mark.usefixtures("_run_mongodb_server_instance") +@pytest.mark.usefixtures("mongodb_fixture") async def test_reinitialize_different_config_negative(caplog): """Tests that we fail when trying to reinitialize with a different configuration.""" different_config = DatabaseConfig(**(make_test_app_config_as_dict()["database"] | {"timeout": 0.1})) with pytest.raises(SystemExit) as exc: - async with mongodb_context(test_app_config.database): - await MongoDB.initialize(different_config) - + await MongoDB.initialize(different_config) assert exc.value.code == errno.EIO assert check_log(caplog, "ERROR", Client.ReinitializeConfigError) @@ -71,7 +80,7 @@ async def test_reinitialize_different_config_negative(caplog): @pytest.mark.parametrize("config_with_wrong_type", [ 1, "1", 1.0, {}, None, [], (), make_test_app_config_as_dict() ]) -@pytest.mark.usefixtures("_run_mongodb_server_instance") +@pytest.mark.usefixtures("mongodb_fixture") async def test_invalid_config_type(caplog, config_with_wrong_type): """Tests that we fail when trying to initialize with a configuration of wrong type.""" with pytest.raises(ValidationError): @@ -79,36 +88,36 @@ async def test_invalid_config_type(caplog, config_with_wrong_type): pass -def check_log(caplog, level: str, response_error: ResponseError) -> bool: - """An auxiliary function to check the log message.""" - for rec in caplog.records: - if rec.levelname == level and (response_error.get_error_details()[1] in rec.message): - return True - return False - - -@pytest.mark.usefixtures("_run_mongodb_server_instance") +@pytest.mark.usefixtures("mongodb_fixture") async def test_reinitialize_same_config_warning(caplog): """Tests the log (warning) when trying to reinitialize with the same configuration.""" - async with mongodb_context(test_app_config.database): - await MongoDB.initialize(test_app_config.database) - + await MongoDB.initialize(test_app_config.database) assert check_log(caplog, "WARNING", Client.AlreadyOpenError) -async def test_get_client(mongodb_fixture): - """This is our way of testing that MongoDB.client() returns the valid client object. +async def test_close_client_negative(caplog): + """Tests that we fail to close a MongoDB client which has not been initialized.""" + with pytest.raises(SystemExit) as exc: + MongoDB.close() + assert exc.value.code == errno.EIO + assert check_log(caplog, "ERROR", Client.CloseNotAllowedError) + + +@pytest.mark.usefixtures("mongodb_fixture") +async def test_client_close(): + """This is our way of testing :func:`trolldb.database.mongodb.MongoDB.close()`. Expect: - The `close` method can be called on the client and leads to the closure of the client - Further attempts to access the database after closing the client fails. """ MongoDB.close() - with pytest.raises(InvalidOperation): + with pytest.raises(AttributeError): await MongoDB.list_database_names() -async def test_main_collection(mongodb_fixture): +@pytest.mark.usefixtures("mongodb_fixture") +async def test_main_collection(): """Tests the properties of the main collection. Expect: @@ -123,21 +132,24 @@ async def test_main_collection(mongodb_fixture): test_app_config.database.main_collection_name] -async def test_main_database(mongodb_fixture): +@pytest.mark.usefixtures("mongodb_fixture") +async def test_main_database(): """Same as test_main_collection but for the main database.""" assert MongoDB.main_database() is not None assert MongoDB.main_database().name == test_app_config.database.main_database_name assert MongoDB.main_database() == await MongoDB.get_database(test_app_config.database.main_database_name) -async def test_get_id(mongodb_fixture): +@pytest.mark.usefixtures("mongodb_fixture") +async def test_get_id(): """Tests :func:`trolldb.database.mongodb.get_id` using all documents (one at a time).""" for _id in TestDatabase.get_document_ids_from_database(): doc = MongoDB.main_collection().find_one({"_id": ObjectId(_id)}) assert await get_id(doc) == _id -async def test_get_ids(mongodb_fixture): +@pytest.mark.usefixtures("mongodb_fixture") +async def test_get_ids(): """Tests :func:`trolldb.database.mongodb.get_ids` using all documents in one pass.""" docs = MongoDB.main_collection().find({}) assert Counter(await get_ids(docs)) == Counter(TestDatabase.get_document_ids_from_database()) From 7c93cda2022dab60560a229e89f5df911f1237a9 Mon Sep 17 00:00:00 2001 From: Pouria Khalaj Date: Sun, 2 Jun 2024 20:26:52 +0200 Subject: [PATCH 12/26] Set `None` as default values for the args of `MongoDB.get_collection()` and `MongoDB().get_database()` --- trolldb/database/mongodb.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/trolldb/database/mongodb.py b/trolldb/database/mongodb.py index b8ad2d8..3b01379 100644 --- a/trolldb/database/mongodb.py +++ b/trolldb/database/mongodb.py @@ -232,8 +232,8 @@ def main_database(cls) -> AsyncIOMotorDatabase: @validate_call async def get_collection( cls, - database_name: str | None, - collection_name: str | None) -> AsyncIOMotorCollection: + database_name: str | None = None, + collection_name: str | None = None) -> AsyncIOMotorCollection: """Gets the collection object given its name and the database name in which it resides. Args: @@ -274,7 +274,7 @@ async def get_collection( @classmethod @validate_call - async def get_database(cls, database_name: str | None) -> AsyncIOMotorDatabase: + async def get_database(cls, database_name: str | None = None) -> AsyncIOMotorDatabase: """Gets the database object given its name. Args: From f937d5974d58391cfc1340d6b3b295fa7fb3e56c Mon Sep 17 00:00:00 2001 From: Pouria Khalaj Date: Sun, 2 Jun 2024 20:29:00 +0200 Subject: [PATCH 13/26] Add tests for `MongoDB.get_collection()` and `MongoDB().get_database()` TODO: These tests need to be improved so that they cover all the code paths. --- trolldb/tests/tests_database/test_mongodb.py | 23 +++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/trolldb/tests/tests_database/test_mongodb.py b/trolldb/tests/tests_database/test_mongodb.py index 976bac8..957332a 100644 --- a/trolldb/tests/tests_database/test_mongodb.py +++ b/trolldb/tests/tests_database/test_mongodb.py @@ -134,12 +134,33 @@ async def test_main_collection(): @pytest.mark.usefixtures("mongodb_fixture") async def test_main_database(): - """Same as test_main_collection but for the main database.""" + """Same as ``test_main_collection()`` but for the main database.""" assert MongoDB.main_database() is not None assert MongoDB.main_database().name == test_app_config.database.main_database_name assert MongoDB.main_database() == await MongoDB.get_database(test_app_config.database.main_database_name) +@pytest.mark.usefixtures("mongodb_fixture") +async def test_get_database(caplog): + """Tests the ``get_database()`` method given different inputs.""" + assert await MongoDB.get_database(None) == MongoDB.main_database() + assert await MongoDB.get_database() == MongoDB.main_database() + assert await MongoDB.get_database(test_app_config.database.main_database_name) == MongoDB.main_database() + + +@pytest.mark.usefixtures("mongodb_fixture") +async def test_get_collection(caplog): + """Same as ``test_get_database()`` but for the ``get_collection()``.""" + assert await MongoDB.get_collection(None, None) == MongoDB.main_collection() + assert await MongoDB.get_collection() == MongoDB.main_collection() + + collection = await MongoDB.get_collection( + test_app_config.database.main_database_name, + test_app_config.database.main_collection_name + ) + assert collection == MongoDB.main_collection() + + @pytest.mark.usefixtures("mongodb_fixture") async def test_get_id(): """Tests :func:`trolldb.database.mongodb.get_id` using all documents (one at a time).""" From e8014ba8c5f996b3b5ddcb1cb956a4acb30c4e87 Mon Sep 17 00:00:00 2001 From: Pouria Khalaj Date: Mon, 3 Jun 2024 14:28:07 +0200 Subject: [PATCH 14/26] Remove unused `MongoDocument` model from `config.py` --- trolldb/config/config.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/trolldb/config/config.py b/trolldb/config/config.py index 99ea99e..26f5e44 100644 --- a/trolldb/config/config.py +++ b/trolldb/config/config.py @@ -53,11 +53,6 @@ def id_must_be_valid(id_like_string: str) -> ObjectId: """The type hint validator for object IDs.""" -class MongoDocument(BaseModel): - """Pydantic model for a MongoDB document.""" - _id: MongoObjectId - - class APIServerConfig(NamedTuple): """A named tuple to hold all the configurations of the API server (excluding the database). From 650f792f1dfab78702c7c2a1f2a71fc83e9bba29 Mon Sep 17 00:00:00 2001 From: Pouria Khalaj Date: Tue, 4 Jun 2024 10:47:39 +0200 Subject: [PATCH 15/26] Add a specific error message to `config.id_must_be_valid()` when raising an exception --- trolldb/config/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/trolldb/config/config.py b/trolldb/config/config.py index 26f5e44..2c8f772 100644 --- a/trolldb/config/config.py +++ b/trolldb/config/config.py @@ -46,7 +46,7 @@ def id_must_be_valid(id_like_string: str) -> ObjectId: try: return ObjectId(id_like_string) except InvalidId as e: - raise ValueError from e + raise ValueError(f"{id_like_string} is not a valid value for an ID.") from e MongoObjectId = Annotated[str, AfterValidator(id_must_be_valid)] From a6f136f820797fc86bd9b4a400e9a253adc3754d Mon Sep 17 00:00:00 2001 From: Pouria Khalaj Date: Tue, 4 Jun 2024 10:48:02 +0200 Subject: [PATCH 16/26] Add tests for `config.id_must_be_valid()` --- trolldb/tests/tests_config/test_config.py | 32 +++++++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 trolldb/tests/tests_config/test_config.py diff --git a/trolldb/tests/tests_config/test_config.py b/trolldb/tests/tests_config/test_config.py new file mode 100644 index 0000000..07cca42 --- /dev/null +++ b/trolldb/tests/tests_config/test_config.py @@ -0,0 +1,32 @@ +"""Tests the :obj:`trolldb.config.config` module.""" + +import pytest +from pydantic import ValidationError + +from trolldb.config.config import id_must_be_valid + + +@pytest.mark.parametrize("id_string", [ + 1, 1.0, {}, None, [], () +]) +def test_id_must_be_valid_bad_types(id_string): + """Tests that we fail to generate a valid ObjectId with wrong input types.""" + with pytest.raises(ValidationError): + id_must_be_valid(id_string) + + +def test_id_must_be_valid_bad_values(): + """Tests that we fail to generate a valid ObjectId with wrong input values.""" + number_of_chars = list(range(1, 24)) + list(range(25, 30)) + for i in number_of_chars: + id_string = "0" * i + with pytest.raises(ValueError, match=id_string): + id_must_be_valid(id_string) + + +@pytest.mark.parametrize("id_string", [ + "0" * 24, "f" * 24, "6255bbd29606404848162477" +]) +def test_id_must_be_valid_success(id_string): + """Tests that we succeed to generate a valid ObjectId with valid input values and type.""" + assert str(id_must_be_valid(id_string)) == id_string From 29f19a72115747a81cf73c69d76412eb961f7d9e Mon Sep 17 00:00:00 2001 From: Pouria Khalaj Date: Sun, 9 Jun 2024 21:07:27 +0200 Subject: [PATCH 17/26] Extract the part which creates the FastAPI app into a separate module. We have added `fastapi_app.py`. This simplifies the testing and also is aligned with separation of concerns. --- trolldb/api/api.py | 71 ++++++-------------------------------- trolldb/api/fastapi_app.py | 51 +++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 60 deletions(-) create mode 100644 trolldb/api/fastapi_app.py diff --git a/trolldb/api/api.py b/trolldb/api/api.py index a960ffe..88ff226 100644 --- a/trolldb/api/api.py +++ b/trolldb/api/api.py @@ -21,51 +21,24 @@ from typing import Any, Generator, NoReturn import uvicorn -from fastapi import FastAPI, status -from fastapi.responses import PlainTextResponse from loguru import logger -from pydantic import ValidationError -from trolldb.api.routes import api_router +from trolldb.api.fastapi_app import fastapi_app from trolldb.config.config import AppConfig, Timeout from trolldb.database.mongodb import mongodb_context -from trolldb.errors.errors import ResponseError - -API_INFO = dict( - title="pytroll-db", - summary="The database API of Pytroll", - description= - "The API allows you to perform CRUD operations as well as querying the database" - "At the moment only MongoDB is supported. It is based on the following Python packages" - "\n * **PyMongo** (https://github.com/mongodb/mongo-python-driver)" - "\n * **motor** (https://github.com/mongodb/motor)", - license_info=dict( - name="The GNU General Public License v3.0", - url="https://www.gnu.org/licenses/gpl-3.0.en.html" - ) -) -"""These will appear in the auto-generated documentation and are passed to the ``FastAPI`` class as keyword args.""" @logger.catch(onerror=lambda _: sys.exit(1)) -def run_server(config: AppConfig, **kwargs) -> None: - """Runs the API server with all the routes and connection to the database. +def run_server(app_config: AppConfig) -> None: + """Runs the API server with connection to the database. - It first creates a FastAPI application and runs it using `uvicorn `_ which is - ASGI (Asynchronous Server Gateway Interface) compliant. This function runs the event loop using + It runs the imported ``fastapi_app`` using `uvicorn `_ which is ASGI + (Asynchronous Server Gateway Interface) compliant. This function runs the event loop using `asyncio `_ and does not yield! Args: - config: - The configuration of the application which includes both the server and database configurations. - - **kwargs: - The keyword arguments are the same as those accepted by the - `FastAPI class `_ and are directly passed - to it. These keyword arguments will be first concatenated with the configurations of the API server which - are read from the ``config`` argument. The keyword arguments which are passed explicitly to the function - take precedence over ``config``. Finally, :obj:`API_INFO`, which are hard-coded information for the API - server, will be concatenated and takes precedence over all. + app_config: + The configuration of the API server and the database. Example: .. code-block:: python @@ -78,37 +51,15 @@ def run_server(config: AppConfig, **kwargs) -> None: """ logger.info("Attempt to run the API server ...") - # Concatenate the keyword arguments for the API server in the order of precedence (lower to higher). - app = FastAPI(**(config.api_server._asdict() | kwargs | API_INFO)) - - app.include_router(api_router) - - @app.exception_handler(ResponseError) - async def auto_handler_response_errors(_, exc: ResponseError) -> PlainTextResponse: - """Catches all the exceptions raised as a ResponseError, e.g. accessing non-existing databases/collections.""" - status_code, message = exc.get_error_details() - info = dict( - status_code=status_code if status_code else status.HTTP_500_INTERNAL_SERVER_ERROR, - content=message if message else "Generic Error [This is not okay, check why we have the generic error!]", - ) - logger.error(f"Response error caught by the API auto exception handler: {info}") - return PlainTextResponse(**info) - - @app.exception_handler(ValidationError) - async def auto_handler_pydantic_validation_errors(_, exc: ValidationError) -> PlainTextResponse: - """Catches all the exceptions raised as a Pydantic ValidationError.""" - logger.error(f"Response error caught by the API auto exception handler: {exc}") - return PlainTextResponse(str(exc), status_code=status.HTTP_500_INTERNAL_SERVER_ERROR) - async def _serve() -> NoReturn: """An auxiliary coroutine to be used in the asynchronous execution of the FastAPI application.""" - async with mongodb_context(config.database): + async with mongodb_context(app_config.database): logger.info("Attempt to start the uvicorn server ...") await uvicorn.Server( config=uvicorn.Config( - host=config.api_server.url.host, - port=config.api_server.url.port, - app=app + host=app_config.api_server.url.host, + port=app_config.api_server.url.port, + app=fastapi_app ) ).serve() diff --git a/trolldb/api/fastapi_app.py b/trolldb/api/fastapi_app.py new file mode 100644 index 0000000..b864711 --- /dev/null +++ b/trolldb/api/fastapi_app.py @@ -0,0 +1,51 @@ +"""The module which creates the main FastAPI app which will be used when running the API server.""" + +from fastapi import FastAPI, status +from fastapi.responses import PlainTextResponse +from loguru import logger +from pydantic import ValidationError + +from trolldb.api.routes import api_router +from trolldb.errors.errors import ResponseError + +API_INFO = dict( + title="pytroll-db", + summary="The database API of Pytroll", + description= + "The API allows you to perform CRUD operations as well as querying the database" + "At the moment only MongoDB is supported. It is based on the following Python packages" + "\n * **PyMongo** (https://github.com/mongodb/mongo-python-driver)" + "\n * **motor** (https://github.com/mongodb/motor)", + license_info=dict( + name="The GNU General Public License v3.0", + url="https://www.gnu.org/licenses/gpl-3.0.en.html" + ) +) +"""These will appear in the auto-generated documentation and are passed to the ``FastAPI`` class as keyword args.""" + +logger.info("Attempt to create the FastAPI app ...") +fastapi_app = FastAPI() + +fastapi_app.include_router(api_router) + + +@fastapi_app.exception_handler(ResponseError) +async def auto_handler_response_errors(_, exc: ResponseError) -> PlainTextResponse: + """Catches all the exceptions raised as a ResponseError, e.g. accessing non-existing databases/collections.""" + status_code, message = exc.get_error_details() + info = dict( + status_code=status_code if status_code else status.HTTP_500_INTERNAL_SERVER_ERROR, + content=message if message else "Generic Error [This is not okay, check why we have the generic error!]", + ) + logger.error(f"Response error caught by the API auto exception handler: {info}") + return PlainTextResponse(**info) + + +@fastapi_app.exception_handler(ValidationError) +async def auto_handler_pydantic_validation_errors(_, exc: ValidationError) -> PlainTextResponse: + """Catches all the exceptions raised as a Pydantic ValidationError.""" + logger.error(f"Response error caught by the API auto exception handler: {exc}") + return PlainTextResponse(str(exc), status_code=status.HTTP_500_INTERNAL_SERVER_ERROR) + + +logger.info("FastAPI app created successfully.") From 8453356ddce200806f7e22a5dc8d8908c6c41b2c Mon Sep 17 00:00:00 2001 From: Pouria Khalaj Date: Sun, 9 Jun 2024 21:22:01 +0200 Subject: [PATCH 18/26] Switch to `httpx.AsyncClient` for testing the API calls This results in a correct code coverage for tests. As a result, `api_server_process_context()` is no longer needed and has been removed. --- trolldb/api/api.py | 36 +-------- trolldb/tests/conftest.py | 8 -- trolldb/tests/tests_api/test_api.py | 115 ++++++++++++++++------------ 3 files changed, 67 insertions(+), 92 deletions(-) diff --git a/trolldb/api/api.py b/trolldb/api/api.py index 88ff226..ae4685d 100644 --- a/trolldb/api/api.py +++ b/trolldb/api/api.py @@ -15,16 +15,13 @@ import asyncio import sys -import time -from contextlib import contextmanager -from multiprocessing import Process -from typing import Any, Generator, NoReturn +from typing import NoReturn import uvicorn from loguru import logger from trolldb.api.fastapi_app import fastapi_app -from trolldb.config.config import AppConfig, Timeout +from trolldb.config.config import AppConfig from trolldb.database.mongodb import mongodb_context @@ -65,32 +62,3 @@ async def _serve() -> NoReturn: logger.info("Attempt to run the asyncio loop for the API server ...") asyncio.run(_serve()) - - -@contextmanager -def api_server_process_context(config: AppConfig, startup_time: Timeout = 2) -> Generator[Process, Any, None]: - """A synchronous context manager to run the API server in a separate process (non-blocking). - - It uses the `multiprocessing `_ package. The main use case - is envisaged to be in `TESTING` environments. - - Args: - config: - Same as ``config`` argument for :func:`run_server`. - - startup_time: - The overall time in seconds that is expected for the server and the database connections to be established - before actual requests can be sent to the server. For testing purposes ensure that this is sufficiently - large so that the tests will not time out. - """ - logger.info("Attempt to run the API server process in a context manager ...") - process = Process(target=run_server, args=(config,)) - try: - process.start() - time.sleep(startup_time) - yield process - finally: - logger.info("Attempt to terminate the API server process in the context manager ...") - process.terminate() - process.join() - logger.info("The API server process has terminated successfully.") diff --git a/trolldb/tests/conftest.py b/trolldb/tests/conftest.py index dbe1288..b3df21f 100644 --- a/trolldb/tests/conftest.py +++ b/trolldb/tests/conftest.py @@ -8,7 +8,6 @@ from _pytest.logging import LogCaptureFixture from loguru import logger -from trolldb.api.api import api_server_process_context from trolldb.database.mongodb import mongodb_context from trolldb.test_utils.common import test_app_config from trolldb.test_utils.mongodb_database import TestDatabase @@ -41,13 +40,6 @@ def _run_mongodb_server_instance(): yield -@pytest.fixture(scope="session") -def _test_server_fixture(_run_mongodb_server_instance): - """Encloses all tests (session scope) in a context manager of a running API server (in a separate process).""" - with api_server_process_context(test_app_config, startup_time=2): - yield - - @pytest_asyncio.fixture() async def mongodb_fixture(_run_mongodb_server_instance): """Fills the database with test data and then enclose each test in a mongodb context manager.""" diff --git a/trolldb/tests/tests_api/test_api.py b/trolldb/tests/tests_api/test_api.py index f96ecca..98d2bee 100644 --- a/trolldb/tests/tests_api/test_api.py +++ b/trolldb/tests/tests_api/test_api.py @@ -1,69 +1,81 @@ """Tests for the API server. Note: - The functionalities of the API server is not mocked! For the tests herein an actual API server will be running in a - separate process. Moreover, a MongoDB instance is run with databases which are pre-filled with random data having - similar characteristics to the real data. Actual requests will be sent to the API and the results will be asserted - against expectations. + The functionalities of the API server is not mocked! For the tests herein an actual MongoDB instance is run with + databases which are pre-filled with random data having similar characteristics to the real data. Actual requests + will be sent to the API and the results will be asserted against expectations. """ from collections import Counter from datetime import datetime import pytest +import pytest_asyncio from fastapi import status +from httpx import ASGITransport, AsyncClient -from trolldb.test_utils.common import http_get, test_app_config +from trolldb.api.fastapi_app import fastapi_app +from trolldb.database.mongodb import mongodb_context +from trolldb.test_utils.common import test_app_config from trolldb.test_utils.mongodb_database import TestDatabase, mongodb_for_test_context +from trolldb.test_utils.mongodb_instance import running_prepared_database_context main_database_name = test_app_config.database.main_database_name main_collection_name = test_app_config.database.main_collection_name -@pytest.mark.usefixtures("_test_server_fixture") -def test_root(): +@pytest_asyncio.fixture() +async def server_client(): + """A fixture to enclose the server async client context manager and start a prepared database. + + TODO: this needs to be optimized so that the database is not closed and reopened for each test! + """ + with running_prepared_database_context(): + async with mongodb_context(test_app_config.database): + async with AsyncClient(transport=ASGITransport(app=fastapi_app), base_url="http://localhost") as ac: + yield ac + + +async def test_root(server_client): """Checks that the server is up and running, i.e. the root routes responds with 200.""" - assert http_get().status == status.HTTP_200_OK + assert (await server_client.get("")).status_code == status.HTTP_200_OK -@pytest.mark.usefixtures("_test_server_fixture") -def test_platforms(): +async def test_platforms(server_client): """Checks that the retrieved platform names match the expected names.""" - assert set(http_get("platforms").json()) == set(TestDatabase.platform_names) + assert set((await server_client.get("/platforms")).json()) == set(TestDatabase.platform_names) -@pytest.mark.usefixtures("_test_server_fixture") -def test_sensors(): +async def test_sensors(server_client): """Checks that the retrieved sensor names match the expected names.""" - assert set(http_get("sensors").json()) == set(TestDatabase.sensors) + assert set((await server_client.get("/sensors")).json()) == set(TestDatabase.sensors) -@pytest.mark.usefixtures("_test_server_fixture") -def test_database_names(): +async def test_database_names(server_client): """Checks that the retrieved database names match the expected names.""" - assert Counter(http_get("databases").json()) == Counter(TestDatabase.database_names) - assert Counter(http_get("databases?exclude_defaults=True").json()) == Counter(TestDatabase.database_names) - assert Counter(http_get("databases?exclude_defaults=False").json()) == Counter(TestDatabase.all_database_names) + assert Counter((await server_client.get("/databases/")).json()) == Counter(TestDatabase.database_names) + assert Counter((await server_client.get("/databases/?exclude_defaults=True")).json()) == Counter( + TestDatabase.database_names) + assert Counter((await server_client.get("/databases/?exclude_defaults=False")).json()) == Counter( + TestDatabase.all_database_names) -@pytest.mark.usefixtures("_test_server_fixture") -def test_database_names_negative(): +async def test_database_names_negative(server_client): """Checks that the non-existing databases cannot be found.""" - assert http_get("databases/non_existing_database").status == status.HTTP_404_NOT_FOUND + assert (await server_client.get("/databases/non_existing_database")).status_code == status.HTTP_404_NOT_FOUND -@pytest.mark.usefixtures("_test_server_fixture") -def test_collections(): +async def test_collections(server_client): """Checks the presence of existing collections and that the ids of documents therein can be correctly retrieved.""" with mongodb_for_test_context() as client: for database_name, collection_name in zip(TestDatabase.database_names, TestDatabase.collection_names, strict=False): assert collections_exists( - http_get(f"databases/{database_name}").json(), + (await server_client.get(f"databases/{database_name}")).json(), [collection_name] ) assert document_ids_are_correct( - http_get(f"databases/{database_name}/{collection_name}").json(), + (await server_client.get(f"databases/{database_name}/{collection_name}")).json(), [str(doc["_id"]) for doc in client[database_name][collection_name].find({})] ) @@ -78,40 +90,38 @@ def document_ids_are_correct(test_ids: list[str], expected_ids: list[str]) -> bo return Counter(test_ids) == Counter(expected_ids) -@pytest.mark.usefixtures("_test_server_fixture") -def test_collections_negative(): +async def test_collections_negative(server_client): """Checks that the non-existing collections cannot be found.""" for database_name in TestDatabase.database_names: - assert http_get(f"databases/{database_name}/non_existing_collection").status == status.HTTP_404_NOT_FOUND + assert (await server_client.get( + f"databases/{database_name}/non_existing_collection")).status_code == status.HTTP_404_NOT_FOUND -@pytest.mark.usefixtures("_test_server_fixture") -def test_datetime(): +async def test_datetime(server_client): """Checks that the datetime route works properly.""" - assert http_get("datetime").json() == TestDatabase.find_min_max_datetime() + assert (await server_client.get("datetime")).json() == TestDatabase.find_min_max_datetime() -@pytest.mark.usefixtures("_test_server_fixture") -def test_queries_all(): +async def test_queries_all(server_client): """Tests that the queries route returns all documents when no actual queries are given.""" assert document_ids_are_correct( - http_get("queries").json(), + (await server_client.get("queries")).json(), TestDatabase.get_document_ids_from_database() ) -@pytest.mark.usefixtures("_test_server_fixture") @pytest.mark.parametrize(("key", "values"), [ ("platform", TestDatabase.unique_platform_names), ("sensor", TestDatabase.unique_sensors) ]) -def test_queries_platform_or_sensor(key: str, values: list[str]): +async def test_queries_platform_or_sensor(server_client, key: str, values: list[str]): """Tests the platform and sensor queries, one at a time. There is only a single key in the query, but it has multiple corresponding values. """ for i in range(len(values)): - assert query_results_are_correct( + assert await query_results_are_correct( + server_client, [key], [values[:i]] ) @@ -125,12 +135,16 @@ def make_query_string(keys: list[str], values_list: list[list[str] | datetime]) return "&".join(query_buffer) -def query_results_are_correct(keys: list[str], values_list: list[list[str] | datetime]) -> bool: +async def query_results_are_correct(server_client: AsyncClient, keys: list[str], + values_list: list[list[str] | datetime]) -> bool: """Checks if the retrieved result from querying the database via the API matches the expected result. There can be more than one query `key/value` pair. Args: + server_client: + The async client object to make API calls. + keys: A list of all query keys, e.g. ``keys=["platform", "sensor"]`` @@ -144,44 +158,45 @@ def query_results_are_correct(keys: list[str], values_list: list[list[str] | dat query_string = make_query_string(keys, values_list) return ( - Counter(http_get(f"queries?{query_string}").json()) == + Counter((await server_client.get(f"queries?{query_string}")).json()) == Counter(TestDatabase.match_query( **{label: value_list for label, value_list in zip(keys, values_list, strict=True)} )) ) -@pytest.mark.usefixtures("_test_server_fixture") -def test_queries_mix_platform_sensor(): +async def test_queries_mix_platform_sensor(server_client): """Tests a mix of platform and sensor queries.""" for n_plt, n_sns in zip([1, 1, 2, 3, 3], [1, 3, 2, 1, 3], strict=False): - assert query_results_are_correct( + assert await query_results_are_correct( + server_client, ["platform", "sensor"], [TestDatabase.unique_platform_names[:n_plt], TestDatabase.unique_sensors[:n_sns]] ) -@pytest.mark.usefixtures("_test_server_fixture") -def test_queries_time(): +async def test_queries_time(server_client): """Checks that a single time query works properly.""" - res = http_get("datetime").json() + res = (await server_client.get("/datetime")).json() time_min = datetime.fromisoformat(res["start_time"]["_min"]["_time"]) time_max = datetime.fromisoformat(res["end_time"]["_max"]["_time"]) - assert single_query_is_correct( + assert await single_query_is_correct( + server_client, "time_min", time_min ) - assert single_query_is_correct( + assert await single_query_is_correct( + server_client, "time_max", time_max ) -def single_query_is_correct(key: str, value: str | datetime) -> bool: +async def single_query_is_correct(server_client: AsyncClient, key: str, value: str | datetime) -> bool: """Checks if the given single query, denoted by ``key`` matches correctly against the ``value``.""" return ( - Counter(http_get(f"queries?{key}={value}").json()) == + Counter((await server_client.get(f"queries?{key}={value}")).json()) == Counter(TestDatabase.match_query(**{key: value})) ) From 2f4e9fa4d9741377985fd3b9b2d1665480eb397c Mon Sep 17 00:00:00 2001 From: Pouria Khalaj Date: Sun, 9 Jun 2024 22:06:27 +0200 Subject: [PATCH 19/26] Change "/databases/" to "/databases" The former scheme is not recommended when designing REST APIs. --- trolldb/api/routes/databases.py | 2 +- trolldb/tests/tests_api/test_api.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/trolldb/api/routes/databases.py b/trolldb/api/routes/databases.py index 046d57a..60e3ab5 100644 --- a/trolldb/api/routes/databases.py +++ b/trolldb/api/routes/databases.py @@ -22,7 +22,7 @@ router = APIRouter() -@router.get("/", +@router.get("", response_model=list[str], summary="Gets the list of all database names") async def database_names( diff --git a/trolldb/tests/tests_api/test_api.py b/trolldb/tests/tests_api/test_api.py index 98d2bee..aec98bc 100644 --- a/trolldb/tests/tests_api/test_api.py +++ b/trolldb/tests/tests_api/test_api.py @@ -53,10 +53,10 @@ async def test_sensors(server_client): async def test_database_names(server_client): """Checks that the retrieved database names match the expected names.""" - assert Counter((await server_client.get("/databases/")).json()) == Counter(TestDatabase.database_names) - assert Counter((await server_client.get("/databases/?exclude_defaults=True")).json()) == Counter( + assert Counter((await server_client.get("/databases")).json()) == Counter(TestDatabase.database_names) + assert Counter((await server_client.get("/databases?exclude_defaults=True")).json()) == Counter( TestDatabase.database_names) - assert Counter((await server_client.get("/databases/?exclude_defaults=False")).json()) == Counter( + assert Counter((await server_client.get("/databases?exclude_defaults=False")).json()) == Counter( TestDatabase.all_database_names) From ca5d3c3bfc49b0ceb0f1a9f46fd3ad4a0115c2e6 Mon Sep 17 00:00:00 2001 From: Pouria Khalaj Date: Mon, 10 Jun 2024 09:36:50 +0200 Subject: [PATCH 20/26] Add a test for the `api.run_server()` This led to the reintroduction of `api_server_process_context()` but it now lies in the `test_utils` module. --- trolldb/test_utils/common.py | 35 +++++++++++++++++++++++++++-- trolldb/tests/tests_api/test_api.py | 9 +++++++- 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/trolldb/test_utils/common.py b/trolldb/test_utils/common.py index 1643b64..563e3bc 100644 --- a/trolldb/test_utils/common.py +++ b/trolldb/test_utils/common.py @@ -1,13 +1,17 @@ """Common functionalities for testing, shared between tests and other test utility modules.""" -from typing import Any, Optional +import time +from contextlib import contextmanager +from multiprocessing import Process +from typing import Any, Generator, Optional from urllib.parse import urljoin import yaml from pydantic import AnyUrl, FilePath from urllib3 import BaseHTTPResponse, request -from trolldb.config.config import AppConfig +from trolldb.api.api import run_server +from trolldb.config.config import AppConfig, Timeout def make_test_app_config_as_dict(subscriber_address: Optional[FilePath] = None) -> dict[str, dict]: @@ -106,3 +110,30 @@ def compare_by_operator_name(operator: str, left: Any, right: Any) -> Any: return left == right case _: raise ValueError(f"Unknown operator: {operator}") + + +@contextmanager +def api_server_process_context( + config: AppConfig = test_app_config, startup_time: Timeout = 2) -> Generator[Process, Any, None]: + """A synchronous context manager to run the API server in a separate process (non-blocking). + + It uses the `multiprocessing `_ package. The main use case + is envisaged to be in `TESTING` environments. + + Args: + config: + Same as ``config`` argument for :func:`run_server`. + + startup_time: + The overall time in seconds that is expected for the server and the database connections to be established + before actual requests can be sent to the server. For testing purposes ensure that this is sufficiently + large so that the tests will not time out. + """ + process = Process(target=run_server, args=(config,)) + try: + process.start() + time.sleep(startup_time) + yield process + finally: + process.terminate() + process.join() diff --git a/trolldb/tests/tests_api/test_api.py b/trolldb/tests/tests_api/test_api.py index aec98bc..3b4d9d1 100644 --- a/trolldb/tests/tests_api/test_api.py +++ b/trolldb/tests/tests_api/test_api.py @@ -16,7 +16,7 @@ from trolldb.api.fastapi_app import fastapi_app from trolldb.database.mongodb import mongodb_context -from trolldb.test_utils.common import test_app_config +from trolldb.test_utils.common import api_server_process_context, http_get, test_app_config from trolldb.test_utils.mongodb_database import TestDatabase, mongodb_for_test_context from trolldb.test_utils.mongodb_instance import running_prepared_database_context @@ -200,3 +200,10 @@ async def single_query_is_correct(server_client: AsyncClient, key: str, value: s Counter((await server_client.get(f"queries?{key}={value}")).json()) == Counter(TestDatabase.match_query(**{key: value})) ) + + +def test_run_server(): + """Tests the ``run_server`` function by starting a running API server + MongoDB and a request to the '/' route.""" + with running_prepared_database_context(): + with api_server_process_context(): + assert http_get("/").status == status.HTTP_200_OK From 27733f7b6ac98cefd651c7129ec8b16c48759ca7 Mon Sep 17 00:00:00 2001 From: Pouria Khalaj Date: Mon, 10 Jun 2024 09:59:39 +0200 Subject: [PATCH 21/26] Add a test for `get_distinct_items_in_collection()` This concerns the case when the input to the function is a response. --- trolldb/tests/tests_api/test_api.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/trolldb/tests/tests_api/test_api.py b/trolldb/tests/tests_api/test_api.py index 3b4d9d1..2556e6e 100644 --- a/trolldb/tests/tests_api/test_api.py +++ b/trolldb/tests/tests_api/test_api.py @@ -11,10 +11,11 @@ import pytest import pytest_asyncio -from fastapi import status +from fastapi import Response, status from httpx import ASGITransport, AsyncClient from trolldb.api.fastapi_app import fastapi_app +from trolldb.api.routes.common import get_distinct_items_in_collection from trolldb.database.mongodb import mongodb_context from trolldb.test_utils.common import api_server_process_context, http_get, test_app_config from trolldb.test_utils.mongodb_database import TestDatabase, mongodb_for_test_context @@ -202,6 +203,14 @@ async def single_query_is_correct(server_client: AsyncClient, key: str, value: s ) +async def test_get_distinct_items_in_collection(): + """Tests that the function returns the same response in case of a response as input.""" + res_actual = Response(status_code=status.HTTP_418_IM_A_TEAPOT, content="Test Response") + res_retrieved = await get_distinct_items_in_collection(res_actual, "") + assert res_retrieved.status_code == res_actual.status_code + assert res_retrieved.body == b"Test Response" + + def test_run_server(): """Tests the ``run_server`` function by starting a running API server + MongoDB and a request to the '/' route.""" with running_prepared_database_context(): From aa8302c24055412516544205d2b929e4914a17e2 Mon Sep 17 00:00:00 2001 From: Pouria Khalaj Date: Mon, 10 Jun 2024 10:30:23 +0200 Subject: [PATCH 22/26] Add more tests for `databases.document_by_id()` This mainly concerns invalid ObjectID values and non-existing documents. --- trolldb/tests/tests_api/test_api.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/trolldb/tests/tests_api/test_api.py b/trolldb/tests/tests_api/test_api.py index 2556e6e..3965c98 100644 --- a/trolldb/tests/tests_api/test_api.py +++ b/trolldb/tests/tests_api/test_api.py @@ -16,6 +16,7 @@ from trolldb.api.fastapi_app import fastapi_app from trolldb.api.routes.common import get_distinct_items_in_collection +from trolldb.database.errors import Documents from trolldb.database.mongodb import mongodb_context from trolldb.test_utils.common import api_server_process_context, http_get, test_app_config from trolldb.test_utils.mongodb_database import TestDatabase, mongodb_for_test_context @@ -211,6 +212,33 @@ async def test_get_distinct_items_in_collection(): assert res_retrieved.body == b"Test Response" +async def test_document_by_id(server_client): + """Tests that one can query the documents by their IDs.""" + for _id, doc in zip(TestDatabase.get_document_ids_from_database(), TestDatabase.documents, strict=False): + doc["_id"] = _id + doc["end_time"] = doc["end_time"].isoformat() + doc["start_time"] = doc["start_time"].isoformat() + assert (await server_client.get(f"databases/{main_database_name}/{main_collection_name}/{_id}")).json() == doc + + +async def test_document_by_id_negative(server_client): + """Tests that we do not allow invalid IDs.""" + for _id in ["1", "1" * 25]: + res = await server_client.get(f"databases/{main_database_name}/{main_collection_name}/{_id}") + assert (res.status_code == status.HTTP_422_UNPROCESSABLE_ENTITY) + assert "Value error" in res.json()["detail"][0]["msg"] + + +async def test_document_by_id_not_found(server_client): + """Tests that we fail to retrieve non-existing document IDs.""" + _id = "1" * 24 + status_code, msg = Documents.NotFound.get_error_details() + + res = await server_client.get(f"databases/{main_database_name}/{main_collection_name}/{_id}") + assert (res.status_code == status_code) + assert (res.text == msg) + + def test_run_server(): """Tests the ``run_server`` function by starting a running API server + MongoDB and a request to the '/' route.""" with running_prepared_database_context(): From 7f5019867137e92f2aa3d198e7fccea9079980ba Mon Sep 17 00:00:00 2001 From: Pouria Khalaj Date: Mon, 10 Jun 2024 11:04:04 +0200 Subject: [PATCH 23/26] Make `check_log()` a fixture --- trolldb/tests/conftest.py | 20 +++++++++++ trolldb/tests/tests_database/test_mongodb.py | 35 ++++++++------------ 2 files changed, 33 insertions(+), 22 deletions(-) diff --git a/trolldb/tests/conftest.py b/trolldb/tests/conftest.py index b3df21f..44cd6bc 100644 --- a/trolldb/tests/conftest.py +++ b/trolldb/tests/conftest.py @@ -3,6 +3,8 @@ This module provides fixtures for running a Mongo DB instance in test mode and filling the database with test data. """ +from typing import Callable + import pytest import pytest_asyncio from _pytest.logging import LogCaptureFixture @@ -46,3 +48,21 @@ async def mongodb_fixture(_run_mongodb_server_instance): TestDatabase.prepare() async with mongodb_context(test_app_config.database): yield + + +@pytest.fixture() +def check_log(caplog) -> Callable: + """A fixture to check the logs. It relies on the ``caplog`` fixture. + + Returns: + A function which can be called to check the log level and message. + """ + + def _aux(level: str, message: str) -> bool: + """An auxiliary function to check the log level and message.""" + for rec in caplog.records: + if rec.levelname == level and (message in rec.message): + return True + return False + + return _aux diff --git a/trolldb/tests/tests_database/test_mongodb.py b/trolldb/tests/tests_database/test_mongodb.py index 957332a..d077a2e 100644 --- a/trolldb/tests/tests_database/test_mongodb.py +++ b/trolldb/tests/tests_database/test_mongodb.py @@ -16,12 +16,11 @@ from trolldb.database.errors import Client, Collections, Databases from trolldb.database.mongodb import DatabaseConfig, MongoDB, get_id, get_ids, mongodb_context -from trolldb.errors.errors import ResponseError from trolldb.test_utils.common import make_test_app_config_as_dict, test_app_config from trolldb.test_utils.mongodb_database import TestDatabase -async def test_connection_timeout_negative(caplog): +async def test_connection_timeout_negative(check_log): """Tests that the connection attempt times out after the expected time, since the MongoDB URL is invalid.""" invalid_config = DatabaseConfig( url=AnyUrl("mongodb://invalid_url_that_does_not_exist:8000"), @@ -37,18 +36,10 @@ async def test_connection_timeout_negative(caplog): t2 = time.time() assert exc.value.code == errno.EIO - assert check_log(caplog, "ERROR", Client.ConnectionError) + assert check_log("ERROR", Client.ConnectionError.get_error_details()[1]) assert t2 - t1 >= invalid_config.timeout -def check_log(caplog, level: str, response_error: ResponseError) -> bool: - """An auxiliary function to check the log message.""" - for rec in caplog.records: - if rec.levelname == level and (response_error.get_error_details()[1] in rec.message): - return True - return False - - @pytest.mark.parametrize(("error", "invalid_config"), [( Collections.NotFoundError, dict(main_database_name=test_app_config.database.main_database_name, main_collection_name=" ")), @@ -57,31 +48,31 @@ def check_log(caplog, level: str, response_error: ResponseError) -> bool: dict(main_database_name=" ", main_collection_name=test_app_config.database.main_collection_name)) ]) @pytest.mark.usefixtures("_run_mongodb_server_instance") -async def test_main_database_and_collection_negative(caplog, error, invalid_config): +async def test_main_database_and_collection_negative(check_log, error, invalid_config): """Tests that we fail when the name of the main database/collection is invalid, given a valid name for the other.""" config = dict(timeout=1, url=test_app_config.database.url) | invalid_config with pytest.raises(SystemExit) as exc: async with mongodb_context(DatabaseConfig(**config)): pass assert exc.value.code == errno.ENODATA - assert check_log(caplog, "ERROR", error) + assert check_log("ERROR", error.get_error_details()[1]) @pytest.mark.usefixtures("mongodb_fixture") -async def test_reinitialize_different_config_negative(caplog): +async def test_reinitialize_different_config_negative(check_log): """Tests that we fail when trying to reinitialize with a different configuration.""" different_config = DatabaseConfig(**(make_test_app_config_as_dict()["database"] | {"timeout": 0.1})) with pytest.raises(SystemExit) as exc: await MongoDB.initialize(different_config) assert exc.value.code == errno.EIO - assert check_log(caplog, "ERROR", Client.ReinitializeConfigError) + assert check_log("ERROR", Client.ReinitializeConfigError.get_error_details()[1]) @pytest.mark.parametrize("config_with_wrong_type", [ 1, "1", 1.0, {}, None, [], (), make_test_app_config_as_dict() ]) @pytest.mark.usefixtures("mongodb_fixture") -async def test_invalid_config_type(caplog, config_with_wrong_type): +async def test_invalid_config_type(check_log, config_with_wrong_type): """Tests that we fail when trying to initialize with a configuration of wrong type.""" with pytest.raises(ValidationError): async with mongodb_context(config_with_wrong_type): @@ -89,18 +80,18 @@ async def test_invalid_config_type(caplog, config_with_wrong_type): @pytest.mark.usefixtures("mongodb_fixture") -async def test_reinitialize_same_config_warning(caplog): +async def test_reinitialize_same_config_warning(check_log): """Tests the log (warning) when trying to reinitialize with the same configuration.""" await MongoDB.initialize(test_app_config.database) - assert check_log(caplog, "WARNING", Client.AlreadyOpenError) + assert check_log("WARNING", Client.AlreadyOpenError.get_error_details()[1]) -async def test_close_client_negative(caplog): +async def test_close_client_negative(check_log): """Tests that we fail to close a MongoDB client which has not been initialized.""" with pytest.raises(SystemExit) as exc: MongoDB.close() assert exc.value.code == errno.EIO - assert check_log(caplog, "ERROR", Client.CloseNotAllowedError) + assert check_log("ERROR", Client.CloseNotAllowedError.get_error_details()[1]) @pytest.mark.usefixtures("mongodb_fixture") @@ -141,7 +132,7 @@ async def test_main_database(): @pytest.mark.usefixtures("mongodb_fixture") -async def test_get_database(caplog): +async def test_get_database(check_log): """Tests the ``get_database()`` method given different inputs.""" assert await MongoDB.get_database(None) == MongoDB.main_database() assert await MongoDB.get_database() == MongoDB.main_database() @@ -149,7 +140,7 @@ async def test_get_database(caplog): @pytest.mark.usefixtures("mongodb_fixture") -async def test_get_collection(caplog): +async def test_get_collection(check_log): """Same as ``test_get_database()`` but for the ``get_collection()``.""" assert await MongoDB.get_collection(None, None) == MongoDB.main_collection() assert await MongoDB.get_collection() == MongoDB.main_collection() From 583284dfadf1de14a17e02fb926ce5b1177c05be Mon Sep 17 00:00:00 2001 From: Pouria Khalaj Date: Mon, 10 Jun 2024 11:06:46 +0200 Subject: [PATCH 24/26] Add a test for unknown messages sent to the recoder --- trolldb/tests/test_recorder.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/trolldb/tests/test_recorder.py b/trolldb/tests/test_recorder.py index 858182f..ceaf0bb 100644 --- a/trolldb/tests/test_recorder.py +++ b/trolldb/tests/test_recorder.py @@ -49,6 +49,13 @@ def del_message(tmp_data_filename): '"polarization": "hh", "sensor": "sar-c", "format": "GeoTIFF", "pass_direction": "ASCENDING"}') +@pytest.fixture() +def unknown_message(tmp_data_filename): + """Create a string for an unknown message.""" + return ("pytroll://deletion some_unknown_key a001673@c20969.ad.smhi.se 2019-11-05T13:00:10.366023 v1.01 " + "application/json {}") + + @pytest.fixture() def tmp_data_filename(tmp_path): """Create a filename for the messages.""" @@ -119,3 +126,13 @@ async def test_record_dataset_messages(tmp_path, dataset_message): with patched_subscriber_recv([dataset_message]): await record_messages(config) assert await message_in_database_and_delete_count_is_one(msg) + + +async def test_unknown_messages(check_log, tmp_path, unknown_message): + """Tests that we identify the message as being unknown, and we log that.""" + config = AppConfig(**make_test_app_config_as_dict(tmp_path)) + with running_prepared_database_context(): + with patched_subscriber_recv([unknown_message]): + await record_messages(config) + + assert check_log("DEBUG", "Don't know what to do with some_unknown_key message") From e67875633e63a4d0b178133e8c111e7fb937c93a Mon Sep 17 00:00:00 2001 From: Pouria Khalaj Date: Thu, 13 Jun 2024 09:39:24 +0200 Subject: [PATCH 25/26] Change `_aux()` to `check_log_message_at_level()` This concerns the internal function (auxiliary) in the `check_log()` fixture. --- trolldb/tests/conftest.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/trolldb/tests/conftest.py b/trolldb/tests/conftest.py index 44cd6bc..0821d85 100644 --- a/trolldb/tests/conftest.py +++ b/trolldb/tests/conftest.py @@ -58,11 +58,11 @@ def check_log(caplog) -> Callable: A function which can be called to check the log level and message. """ - def _aux(level: str, message: str) -> bool: + def check_log_message_at_level(level: str, message: str) -> bool: """An auxiliary function to check the log level and message.""" for rec in caplog.records: if rec.levelname == level and (message in rec.message): return True return False - return _aux + return check_log_message_at_level From 62974a430c11d4ecf80867d68ca6539d121132c9 Mon Sep 17 00:00:00 2001 From: Pouria Khalaj Date: Thu, 13 Jun 2024 10:31:33 +0200 Subject: [PATCH 26/26] Add a note to explain why we change error type in `id_must_be_valid()` --- trolldb/config/config.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/trolldb/config/config.py b/trolldb/config/config.py index 2c8f772..3d63904 100644 --- a/trolldb/config/config.py +++ b/trolldb/config/config.py @@ -42,6 +42,16 @@ def id_must_be_valid(id_like_string: str) -> ObjectId: ValueError: If the given string cannot be converted to a valid ObjectId. This will ultimately turn into a pydantic validation error. + + Note: + The reason that we change the type of the raised error is the following. As per the requirements of Pydantic, + one can either raise a ``ValueError`` or ``AssertionError`` in a custom validator. Here we have defined a custom + validator for a MongoDB object ID. When it fails it raises ``InvalidId`` which is not a valid exception to + signify validation failure in Pydantic, hence the need to catch the error and raise a different one. + + Reference: + https://docs.pydantic.dev/latest/concepts/validators/#handling-errors-in-validators + """ try: return ObjectId(id_like_string)