diff --git a/src/config/config.py b/src/config/config.py index f72c22c7..fdb2040b 100644 --- a/src/config/config.py +++ b/src/config/config.py @@ -16,14 +16,13 @@ from typing import Dict, Optional from urllib.parse import urljoin -from pydantic import BaseModel, ConfigDict, PrivateAttr +from pydantic import BaseModel, PrivateAttr from pydantic_settings import BaseSettings, SettingsConfigDict from requests import PreparedRequest from requests.auth import AuthBase from src.blueprints.custom_data_types import MTUrl from src.blueprints.storage_boxes import StorageTypesEnum -from src.mytardis_client.objects import MyTardisObject logger = logging.getLogger(__name__) @@ -161,37 +160,11 @@ class FilesystemStorageBoxConfig(StorageBoxConfig): target_root_dir: Path -class IntrospectionConfig(BaseModel): - """MyTardis introspection data. - - Pydantic model for MyTardis introspection data. NOTE: this class relies on - data from the MyTardis introspection API and therefore can't be instantiated - without a request to the specific MyTardis instance. - - Attributes: - old_acls : bool - the MyTardis instance uses experiment only ACLs if `True` - projects_enabled : bool - the MyTardis instance uses projects if `True` - objects_with_ids : Optional[list[MyTardisObject]] - """ - - model_config = ConfigDict(use_enum_values=False) - - old_acls: bool - projects_enabled: bool - identifiers_enabled: bool - profiles_enabled: bool - objects_with_ids: list[MyTardisObject] - objects_with_profiles: list[MyTardisObject] - - class ConfigFromEnv(BaseSettings): """Full MyTardis settings model. This class holds the configuration to access and run an ingestion on - MyTardis. It also provides access to the introspection API via the - mytardis_setup property. + MyTardis. Attributes: general : GeneralConfig @@ -207,12 +180,6 @@ class ConfigFromEnv(BaseSettings): archive: TimeOffsetConfig instance of Pydantic time offset model - - Properties: - mytardis_setup : Optional[IntrospectionConfig] (default: None) - instance of Pydantic introspection model either from private - attribute or new request - ## Usage Requires a .env file in the current working direction: ''' @@ -237,7 +204,6 @@ class ConfigFromEnv(BaseSettings): ## Example '''python settings = ConfigFromEnv() - setup = settings.mytardis_setup # <- only has value after first call ''' """ diff --git a/src/mytardis_client/response_data.py b/src/mytardis_client/response_data.py new file mode 100644 index 00000000..ac3c00ad --- /dev/null +++ b/src/mytardis_client/response_data.py @@ -0,0 +1,46 @@ +"""Dataclasses for validating/storing MyTardis API response data.""" + +from typing import Optional + +from pydantic import BaseModel, ConfigDict, Field, model_validator +from typing_extensions import Self + +from src.mytardis_client.objects import MyTardisObject + + +class MyTardisIntrospection(BaseModel): + """MyTardis introspection data. + + NOTE: this class relies on data from the MyTardis introspection API and therefore + can't be instantiated without a request to the specific MyTardis instance. + """ + + model_config = ConfigDict(use_enum_values=False) + + data_classification_enabled: Optional[bool] + identifiers_enabled: bool + objects_with_ids: list[MyTardisObject] = Field( + validation_alias="identified_objects" + ) + objects_with_profiles: list[MyTardisObject] = Field( + validation_alias="profiled_objects" + ) + old_acls: bool = Field(validation_alias="experiment_only_acls") + projects_enabled: bool + profiles_enabled: bool + + @model_validator(mode="after") + def validate_consistency(self) -> Self: + """Check that the introspection data is consistent.""" + + if not self.identifiers_enabled and len(self.objects_with_ids) > 0: + raise ValueError( + "Identifiers are disabled in MyTardis but it reports identifiable types" + ) + + if not self.profiles_enabled and len(self.objects_with_profiles) > 0: + raise ValueError( + "Profiles are disabled in MyTardis but it reports profiled types" + ) + + return self diff --git a/src/overseers/overseer.py b/src/overseers/overseer.py index 53a7b0c0..c6accd03 100644 --- a/src/overseers/overseer.py +++ b/src/overseers/overseer.py @@ -9,11 +9,11 @@ from pydantic import ValidationError from requests.exceptions import HTTPError -from src.config.config import IntrospectionConfig from src.mytardis_client.data_types import URI from src.mytardis_client.endpoints import MyTardisEndpoint from src.mytardis_client.mt_rest import MyTardisRESTFactory from src.mytardis_client.objects import MyTardisObject, get_type_info +from src.mytardis_client.response_data import MyTardisIntrospection from src.utils.types.singleton import Singleton logger = logging.getLogger(__name__) @@ -82,7 +82,7 @@ class Overseer(metaclass=Singleton): rest_factory: An instance of MyTardisRESTFactory providing access to the API """ - _mytardis_setup: IntrospectionConfig | None = None + _mytardis_setup: MyTardisIntrospection | None = None def __init__( self, @@ -99,14 +99,16 @@ def __init__( instance connection : ConnectionConfig Pydantic config class containing information about connecting to a MyTardis instance - mytardis_setup : IntrospectionConfig + mytardis_setup : MyTardisIntrospection """ self.rest_factory = rest_factory @property - def mytardis_setup(self) -> IntrospectionConfig: - """Getter for mytardis_setup. Sends API request if self._mytardis_setup is None""" - return self._mytardis_setup or self.get_mytardis_setup() + def mytardis_setup(self) -> MyTardisIntrospection: + """Getter for mytardis_setup. Sends API request on first call and caches the result""" + if self._mytardis_setup is None: + self._mytardis_setup = self.fetch_mytardis_setup() + return self._mytardis_setup def check_identifiers_enabled_for_type(self, object_type: MyTardisObject) -> None: """Check if identifiers are enabled for the given object type. @@ -258,7 +260,7 @@ def get_uris_by_identifier( return self.get_uris(object_type, {"identifier": identifier}) - def get_mytardis_setup(self) -> IntrospectionConfig: + def fetch_mytardis_setup(self) -> MyTardisIntrospection: """Query introspection API Requests introspection info from MyTardis instance configured in connection @@ -268,12 +270,6 @@ def get_mytardis_setup(self) -> IntrospectionConfig: response_dict = response.json() if response_dict == {} or response_dict["objects"] == []: - logger.error( - ( - "MyTardis introspection did not return any data when called from " - "ConfigFromEnv.get_mytardis_setup" - ) - ) raise ValueError( ( "MyTardis introspection did not return any data when called from " @@ -281,7 +277,7 @@ def get_mytardis_setup(self) -> IntrospectionConfig: ) ) if len(response_dict["objects"]) > 1: - logger.error( + raise ValueError( ( """MyTardis introspection returned more than one object when called from ConfigFromEnv.get_mytardis_setup\n @@ -289,39 +285,9 @@ def get_mytardis_setup(self) -> IntrospectionConfig: response_dict, ) ) - raise ValueError( - ( - "MyTardis introspection returned more than one object when called from " - "ConfigFromEnv.get_mytardis_setup" - ) - ) - response_dict = response_dict["objects"][0] - - identifiers_enabled: bool = response_dict["identifiers_enabled"] - objects_with_ids = [ - MyTardisObject(obj) for obj in response_dict["identified_objects"] - ] - if not identifiers_enabled and len(objects_with_ids) > 0: - raise ValueError( - "Identifiers are disabled in MyTardis but it reports identifiable types" - ) - profiles_enabled: bool = response_dict["profiles_enabled"] - objects_with_profiles = [ - MyTardisObject(obj) for obj in response_dict["profiled_objects"] - ] - if not profiles_enabled and len(objects_with_profiles) > 0: - raise ValueError( - "Profiles are disabled in MyTardis but it reports profiled types" - ) - - mytardis_setup = IntrospectionConfig( - old_acls=response_dict["experiment_only_acls"], - projects_enabled=response_dict["projects_enabled"], - identifiers_enabled=identifiers_enabled, - profiles_enabled=profiles_enabled, - objects_with_ids=objects_with_ids, - objects_with_profiles=objects_with_profiles, + introspection = MyTardisIntrospection.model_validate( + response_dict["objects"][0] ) - self._mytardis_setup = mytardis_setup - return mytardis_setup + + return introspection diff --git a/tests/conftest.py b/tests/conftest.py index a5bb59ec..faef7343 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -11,6 +11,7 @@ import tests.fixtures.fixtures_data_classification_app as dclass import tests.fixtures.fixtures_dataclasses as dcls import tests.fixtures.fixtures_ingestion_classes as ingestion_classes +import tests.fixtures.fixtures_mytardis_client as mt_client import tests.fixtures.fixtures_responses as rsps import tests.fixtures.mock_rest_factory as mock_rest from src.blueprints.common_models import GroupACL, UserACL @@ -541,7 +542,7 @@ def preconditioned_datafile_dictionary() -> Dict[str, Any]: experiment_response_dict = rsps.experiment_response_dict project_response_dict = rsps.project_response_dict instrument_response_dict = rsps.instrument_response_dict -introspection_response_dict = rsps.introspection_response_dict +introspection_response = rsps.introspection_response institution_response_dict = rsps.institution_response_dict storage_box_response_dict = rsps.storage_box_response_dict project_creation_response_dict = rsps.project_creation_response_dict @@ -554,14 +555,12 @@ def preconditioned_datafile_dictionary() -> Dict[str, Any]: # # ========================================= -processed_introspection_response = cfg.processed_introspection_response general = cfg.general auth = cfg.auth connection = cfg.connection active_store = cfg.active_store archive_store = cfg.archive_store default_schema = cfg.default_schema -mytardis_setup = cfg.mytardis_setup mytardis_settings = cfg.mytardis_settings storage_box_name = cfg.storage_box_name storage_attributes = cfg.storage_attributes @@ -596,3 +595,12 @@ def preconditioned_datafile_dictionary() -> Dict[str, Any]: project_data_classification = dclass.project_data_classification experiment_data_classification = dclass.experiment_data_classification dataset_data_classification = dclass.dataset_data_classification + + +# ======================================== +# +# MyTardis client data fixtures +# +# ========================================= + +mytardis_introspection = mt_client.mytardis_introspection diff --git a/tests/fixtures/fixtures_config_from_env.py b/tests/fixtures/fixtures_config_from_env.py index bb7e3902..21d1765d 100644 --- a/tests/fixtures/fixtures_config_from_env.py +++ b/tests/fixtures/fixtures_config_from_env.py @@ -1,7 +1,7 @@ # pylint: disable=missing-function-docstring,redefined-outer-name # pylint: disable=missing-module-docstring -from typing import Any, Dict, List +from typing import Any, Dict from pytest import fixture @@ -11,12 +11,10 @@ ConfigFromEnv, ConnectionConfig, GeneralConfig, - IntrospectionConfig, ProxyConfig, SchemaConfig, StorageBoxConfig, ) -from src.mytardis_client.objects import MyTardisObject @fixture @@ -116,25 +114,6 @@ def archive_store( ) -@fixture -def processed_introspection_response() -> Dict[str, bool | List[str]]: - return { - "old_acls": False, - "projects_enabled": True, - "identifiers_enabled": True, - "objects_with_ids": [ - "dataset", - "experiment", - "facility", - "instrument", - "project", - "institution", - ], - "profiles_enabled": False, - "objects_with_profiles": [], - } - - @fixture def general( default_institution: str, @@ -208,30 +187,6 @@ def default_schema( ) -@fixture -def mytardis_setup( - processed_introspection_response: dict[str, Any], -) -> IntrospectionConfig: - - objects_with_ids = [ - MyTardisObject(obj) - for obj in processed_introspection_response["objects_with_ids"] - ] - objects_with_profiles = [ - MyTardisObject(obj) - for obj in processed_introspection_response["objects_with_profiles"] - ] - - return IntrospectionConfig( - old_acls=processed_introspection_response["old_acls"], - projects_enabled=processed_introspection_response["projects_enabled"], - identifiers_enabled=processed_introspection_response["identifiers_enabled"], - profiles_enabled=processed_introspection_response["profiles_enabled"], - objects_with_ids=objects_with_ids, - objects_with_profiles=objects_with_profiles, - ) - - @fixture def mytardis_settings( general: GeneralConfig, diff --git a/tests/fixtures/fixtures_ingestion_classes.py b/tests/fixtures/fixtures_ingestion_classes.py index 13608860..f33d6492 100644 --- a/tests/fixtures/fixtures_ingestion_classes.py +++ b/tests/fixtures/fixtures_ingestion_classes.py @@ -11,7 +11,6 @@ ConnectionConfig, FilesystemStorageBoxConfig, GeneralConfig, - IntrospectionConfig, SchemaConfig, StorageBoxConfig, ) @@ -20,6 +19,7 @@ from src.forges.forge import Forge from src.ingestion_factory import IngestionFactory from src.mytardis_client.mt_rest import MyTardisRESTFactory +from src.mytardis_client.response_data import MyTardisIntrospection from src.overseers.overseer import Overseer from src.smelters.smelter import Smelter from tests.fixtures.mock_rest_factory import MockMtRest @@ -41,10 +41,10 @@ def rest_factory( @fixture def overseer( rest_factory: MyTardisRESTFactory, - mytardis_setup: IntrospectionConfig, + mytardis_introspection: MyTardisIntrospection, ) -> Overseer: overseer = Overseer(rest_factory) - overseer._mytardis_setup = mytardis_setup # pylint: disable=W0212 + overseer._mytardis_setup = mytardis_introspection # pylint: disable=W0212 return overseer diff --git a/tests/fixtures/fixtures_mytardis_client.py b/tests/fixtures/fixtures_mytardis_client.py new file mode 100644 index 00000000..b952d068 --- /dev/null +++ b/tests/fixtures/fixtures_mytardis_client.py @@ -0,0 +1,19 @@ +"""Test fixtures related to the MyTardis client.""" + +# pylint: disable=missing-function-docstring + +from typing import Any + +from pytest import fixture + +from src.mytardis_client.response_data import MyTardisIntrospection + + +@fixture +def mytardis_introspection( + introspection_response: dict[str, Any], +) -> MyTardisIntrospection: + + object_data = introspection_response["objects"][0] + + return MyTardisIntrospection.model_validate(object_data) diff --git a/tests/fixtures/fixtures_responses.py b/tests/fixtures/fixtures_responses.py index 175e4c6c..f4190665 100644 --- a/tests/fixtures/fixtures_responses.py +++ b/tests/fixtures/fixtures_responses.py @@ -357,7 +357,7 @@ def instrument_response_dict( @fixture -def introspection_response_dict() -> Dict[str, Any]: +def introspection_response() -> dict[str, Any]: return { "meta": { "limit": 20, @@ -368,6 +368,7 @@ def introspection_response_dict() -> Dict[str, Any]: }, "objects": [ { + "data_classification_enabled": None, "experiment_only_acls": False, "identified_objects": [ "dataset", diff --git a/tests/test_overseers.py b/tests/test_overseers.py index b890eaf9..6ddacfa9 100644 --- a/tests/test_overseers.py +++ b/tests/test_overseers.py @@ -14,10 +14,11 @@ from requests import HTTPError from responses import matchers -from src.config.config import ConnectionConfig, IntrospectionConfig +from src.config.config import ConnectionConfig from src.mytardis_client.data_types import URI from src.mytardis_client.mt_rest import MyTardisRESTFactory from src.mytardis_client.objects import MyTardisObject +from src.mytardis_client.response_data import MyTardisIntrospection from src.overseers.overseer import Overseer logger = logging.getLogger(__name__) @@ -365,8 +366,8 @@ def test_get_uris_general_error( def test_get_mytardis_setup( overseer_plain: Overseer, connection: ConnectionConfig, - introspection_response_dict: dict[str, Any], - mytardis_setup: IntrospectionConfig, + introspection_response: dict[str, Any], + mytardis_introspection: MyTardisIntrospection, ) -> None: overseer_plain._mytardis_setup = None # pylint: disable=protected-access assert overseer_plain._mytardis_setup is None # pylint: disable=protected-access @@ -377,11 +378,11 @@ def test_get_mytardis_setup( connection.api_template, "introspection", ), - json=(introspection_response_dict), + json=(introspection_response), status=200, ) - assert overseer_plain.mytardis_setup == mytardis_setup + assert overseer_plain.mytardis_setup == mytardis_introspection Overseer.clear() @@ -400,12 +401,12 @@ def test_get_mytardis_setup_http_error( caplog.set_level(logging.ERROR) error_str = "Introspection returned error:" with pytest.raises(HTTPError): - _ = overseer.get_mytardis_setup() + _ = overseer.fetch_mytardis_setup() assert error_str in caplog.text Overseer.clear() -@mock.patch("src.overseers.overseer.Overseer.get_mytardis_setup") +@mock.patch("src.overseers.overseer.Overseer.fetch_mytardis_setup") def test_get_mytardis_setup_general_error( mock_get_mytardis_setup: Any, caplog: LogCaptureFixture, @@ -414,7 +415,7 @@ def test_get_mytardis_setup_general_error( mock_get_mytardis_setup.side_effect = IOError() error_str = "Non-HTTP exception in ConfigFromEnv.get_mytardis_setup" with pytest.raises(IOError): - _ = overseer.get_mytardis_setup() + _ = overseer.fetch_mytardis_setup() assert error_str in caplog.text Overseer.clear() @@ -441,19 +442,18 @@ def test_get_mytardis_setup_no_objects( "ConfigFromEnv.get_mytardis_setup" ) with pytest.raises(ValueError, match=error_str): - _ = overseer.get_mytardis_setup() + _ = overseer.fetch_mytardis_setup() assert error_str in caplog.text Overseer.clear() @responses.activate def test_get_mytardis_setup_too_many_objects( - caplog: LogCaptureFixture, overseer: Overseer, connection: ConnectionConfig, - introspection_response_dict: dict[str, Any], + introspection_response: dict[str, Any], ) -> None: - test_dict = introspection_response_dict + test_dict = introspection_response test_dict["objects"].append("Some Fake Data") responses.add( responses.GET, @@ -464,16 +464,8 @@ def test_get_mytardis_setup_too_many_objects( json=(test_dict), status=200, ) - caplog.set_level(logging.ERROR) - log_error_str = f"""MyTardis introspection returned more than one object when called from - ConfigFromEnv.get_mytardis_setup\n - Returned response was: {test_dict}""" - error_str = ( - "MyTardis introspection returned more than one object when called from " - "ConfigFromEnv.get_mytardis_setup" - ) - with pytest.raises(ValueError, match=error_str): - _ = overseer.get_mytardis_setup() - assert log_error_str in caplog.text + with pytest.raises(ValueError): + _ = overseer.fetch_mytardis_setup() + Overseer.clear()