From 99381a86cc778f7c97d633e0a7c06fede32107bf Mon Sep 17 00:00:00 2001 From: belthlemar Date: Mon, 9 Dec 2024 10:59:43 +0100 Subject: [PATCH] resolve almost all comments --- .../study/business/areas/renewable_management.py | 4 ++-- .../business/areas/st_storage_management.py | 6 ++---- .../study/business/areas/thermal_management.py | 4 ++-- .../business/binding_constraint_management.py | 6 +++--- .../model/filesystem/config/field_validators.py | 10 ++++------ .../model/command/create_district.py | 8 +------- .../model/command/remove_st_storage.py | 10 +++++----- antarest/study/web/study_data_blueprint.py | 16 ++++++++-------- .../variant_blueprint/test_thermal_cluster.py | 2 +- .../repository/filesystem/config/test_utils.py | 3 +-- .../model/command/test_create_cluster.py | 4 ++-- .../command/test_create_renewables_cluster.py | 2 +- .../model/command/test_remove_cluster.py | 2 +- .../model/command/test_remove_link.py | 2 +- .../command/test_remove_renewables_cluster.py | 2 +- 15 files changed, 35 insertions(+), 46 deletions(-) diff --git a/antarest/study/business/areas/renewable_management.py b/antarest/study/business/areas/renewable_management.py index 6725bcc85c..14ba0e60e7 100644 --- a/antarest/study/business/areas/renewable_management.py +++ b/antarest/study/business/areas/renewable_management.py @@ -17,7 +17,7 @@ from pydantic import field_validator from antarest.core.exceptions import DuplicateRenewableCluster, RenewableClusterConfigNotFound, RenewableClusterNotFound -from antarest.core.model import JSON, LowerCaseStr +from antarest.core.model import JSON from antarest.study.business.all_optional_meta import all_optional_model, camel_case_model from antarest.study.business.enum_ignore_case import EnumIgnoreCase from antarest.study.business.utils import execute_or_add_commands @@ -340,7 +340,7 @@ def duplicate_cluster( study: Study, area_id: str, source_id: str, - new_cluster_name: LowerCaseStr, + new_cluster_name: str, ) -> RenewableClusterOutput: """ Creates a duplicate cluster within the study area with a new name. diff --git a/antarest/study/business/areas/st_storage_management.py b/antarest/study/business/areas/st_storage_management.py index 2e42d73588..5786b4183b 100644 --- a/antarest/study/business/areas/st_storage_management.py +++ b/antarest/study/business/areas/st_storage_management.py @@ -27,7 +27,7 @@ STStorageMatrixNotFound, STStorageNotFound, ) -from antarest.core.model import JSON, LowerCaseStr +from antarest.core.model import JSON from antarest.core.requests import CaseInsensitiveDict from antarest.core.serialization import AntaresBaseModel from antarest.study.business.all_optional_meta import all_optional_model, camel_case_model @@ -534,9 +534,7 @@ def delete_storages( ) execute_or_add_commands(study, file_study, [command], self.storage_service) - def duplicate_cluster( - self, study: Study, area_id: str, source_id: str, new_cluster_name: LowerCaseStr - ) -> STStorageOutput: + def duplicate_cluster(self, study: Study, area_id: str, source_id: str, new_cluster_name: str) -> STStorageOutput: """ Creates a duplicate cluster within the study area with a new name. diff --git a/antarest/study/business/areas/thermal_management.py b/antarest/study/business/areas/thermal_management.py index 29a3cc8bde..150dd7adfa 100644 --- a/antarest/study/business/areas/thermal_management.py +++ b/antarest/study/business/areas/thermal_management.py @@ -24,7 +24,7 @@ ThermalClusterNotFound, WrongMatrixHeightError, ) -from antarest.core.model import JSON, LowerCaseStr +from antarest.core.model import JSON from antarest.study.business.all_optional_meta import all_optional_model, camel_case_model from antarest.study.business.utils import execute_or_add_commands from antarest.study.model import STUDY_VERSION_8_7, Study @@ -414,7 +414,7 @@ def duplicate_cluster( study: Study, area_id: str, source_id: str, - new_cluster_name: LowerCaseStr, + new_cluster_name: str, ) -> ThermalClusterOutput: """ Creates a duplicate cluster within the study area with a new name. diff --git a/antarest/study/business/binding_constraint_management.py b/antarest/study/business/binding_constraint_management.py index 1653bc6001..de461cf681 100644 --- a/antarest/study/business/binding_constraint_management.py +++ b/antarest/study/business/binding_constraint_management.py @@ -30,7 +30,7 @@ MatrixWidthMismatchError, WrongMatrixHeightError, ) -from antarest.core.model import JSON +from antarest.core.model import JSON, LowerCaseStr from antarest.core.requests import CaseInsensitiveDict from antarest.core.serialization import AntaresBaseModel from antarest.core.utils.string import to_camel_case @@ -340,7 +340,7 @@ class ConstraintOutput830(ConstraintOutputBase): class ConstraintOutput870(ConstraintOutput830): - group: str = DEFAULT_GROUP + group: LowerCaseStr = DEFAULT_GROUP # WARNING: Do not change the order of the following line, it is used to determine @@ -553,7 +553,7 @@ def constraint_model_adapter(constraint: t.Mapping[str, t.Any], study_version: S constraint_output["filter_year_by_year"] = _filter_year_by_year constraint_output["filter_synthesis"] = _filter_synthesis if study_version >= STUDY_VERSION_8_7: - constraint_output["group"] = constraint.get("group", DEFAULT_GROUP).lower() + constraint_output["group"] = constraint.get("group", DEFAULT_GROUP) # Choose the right model according to the version adapted_constraint: ConstraintOutput diff --git a/antarest/study/storage/rawstudy/model/filesystem/config/field_validators.py b/antarest/study/storage/rawstudy/model/filesystem/config/field_validators.py index 1ee382065c..5761cdc587 100644 --- a/antarest/study/storage/rawstudy/model/filesystem/config/field_validators.py +++ b/antarest/study/storage/rawstudy/model/filesystem/config/field_validators.py @@ -93,20 +93,18 @@ def validate_color_rgb(v: t.Any) -> str: _sub_invalid_chars = re.compile(r"[^a-zA-Z0-9_(),& -]+").sub -def transform_name_to_id(name: str, lower: bool = True) -> str: +def transform_name_to_id(name: str) -> str: """ Transform a name into an identifier by replacing consecutive - invalid characters by a single white space, and then whitespaces - are striped from both ends. + invalid characters by a single white space, then whitespaces + are striped from both ends and the id is lowered. Valid characters are `[a-zA-Z0-9_(),& -]` (including space). Args: name: The name to convert. - lower: The flag used to turn the identifier in lower case. """ - valid_id = _sub_invalid_chars(" ", name).strip() - return valid_id.lower() if lower else valid_id + return _sub_invalid_chars(" ", name).strip().lower() def validate_id_against_name(name: str) -> str: diff --git a/antarest/study/storage/variantstudy/model/command/create_district.py b/antarest/study/storage/variantstudy/model/command/create_district.py index 48f9b6c15d..e8ed7f69d9 100644 --- a/antarest/study/storage/variantstudy/model/command/create_district.py +++ b/antarest/study/storage/variantstudy/model/command/create_district.py @@ -15,6 +15,7 @@ from pydantic import field_validator +from antarest.core.model import LowerCaseStr from antarest.study.storage.rawstudy.model.filesystem.config.field_validators import transform_name_to_id from antarest.study.storage.rawstudy.model.filesystem.config.model import DistrictSet, FileStudyTreeConfig from antarest.study.storage.rawstudy.model.filesystem.factory import FileStudy @@ -49,13 +50,6 @@ class CreateDistrict(ICommand): output: bool = True comments: str = "" - @field_validator("name") - def validate_district_name(cls, val: str) -> str: - valid_name = transform_name_to_id(val, lower=False) - if valid_name != val: - raise ValueError("Area name must only contains [a-zA-Z0-9],&,-,_,(,) characters") - return val - def _apply_config(self, study_data: FileStudyTreeConfig) -> Tuple[CommandOutput, Dict[str, Any]]: district_id = transform_name_to_id(self.name) if district_id in study_data.sets: diff --git a/antarest/study/storage/variantstudy/model/command/remove_st_storage.py b/antarest/study/storage/variantstudy/model/command/remove_st_storage.py index 4b0b710c28..0c59dc0208 100644 --- a/antarest/study/storage/variantstudy/model/command/remove_st_storage.py +++ b/antarest/study/storage/variantstudy/model/command/remove_st_storage.py @@ -15,7 +15,10 @@ from pydantic import Field, field_validator from antarest.study.model import STUDY_VERSION_8_6 -from antarest.study.storage.rawstudy.model.filesystem.config.field_validators import transform_name_to_id +from antarest.study.storage.rawstudy.model.filesystem.config.field_validators import ( + transform_name_to_id, + validate_id_against_name, +) from antarest.study.storage.rawstudy.model.filesystem.config.model import Area, FileStudyTreeConfig from antarest.study.storage.rawstudy.model.filesystem.factory import FileStudy from antarest.study.storage.variantstudy.model.command.common import CommandName, CommandOutput @@ -46,10 +49,7 @@ class RemoveSTStorage(ICommand): @field_validator("storage_id", mode="before") def validate_cluster_name(cls, val: str) -> str: - to_return = transform_name_to_id(val) - if not to_return: - raise ValueError("Cluster name must only contains [a-zA-Z0-9],&,-,_,(,) characters") - return to_return + return validate_id_against_name(val) def _apply_config(self, study_data: FileStudyTreeConfig) -> t.Tuple[CommandOutput, t.Dict[str, t.Any]]: """ diff --git a/antarest/study/web/study_data_blueprint.py b/antarest/study/web/study_data_blueprint.py index 5adfca3bdb..ba9d2034a4 100644 --- a/antarest/study/web/study_data_blueprint.py +++ b/antarest/study/web/study_data_blueprint.py @@ -21,7 +21,7 @@ from antarest.core.config import Config from antarest.core.jwt import JWTUser -from antarest.core.model import JSON, StudyPermissionType +from antarest.core.model import JSON, LowerCaseStr, StudyPermissionType from antarest.core.requests import RequestParameters from antarest.core.utils.utils import sanitize_uuid from antarest.core.utils.web import APITag @@ -1955,7 +1955,7 @@ def create_renewable_cluster( def update_renewable_cluster( uuid: str, area_id: str, - cluster_id: str, + cluster_id: LowerCaseStr, cluster_data: RenewableClusterInput, current_user: JWTUser = Depends(auth.get_current_user), ) -> RenewableClusterOutput: @@ -1965,7 +1965,7 @@ def update_renewable_cluster( ) request_params = RequestParameters(user=current_user) study = study_service.check_study_access(uuid, StudyPermissionType.WRITE, request_params) - return study_service.renewable_manager.update_cluster(study, area_id, cluster_id.lower(), cluster_data) + return study_service.renewable_manager.update_cluster(study, area_id, cluster_id, cluster_data) @bp.put( path="/studies/{uuid}/areas/{area_id}/clusters/renewable/{cluster_id}/form", @@ -2129,7 +2129,7 @@ def create_thermal_cluster( def update_thermal_cluster( uuid: str, area_id: str, - cluster_id: str, + cluster_id: LowerCaseStr, cluster_data: ThermalClusterInput, current_user: JWTUser = Depends(auth.get_current_user), ) -> ThermalClusterOutput: @@ -2149,7 +2149,7 @@ def update_thermal_cluster( ) request_params = RequestParameters(user=current_user) study = study_service.check_study_access(uuid, StudyPermissionType.WRITE, request_params) - return study_service.thermal_manager.update_cluster(study, area_id, cluster_id.lower(), cluster_data) + return study_service.thermal_manager.update_cluster(study, area_id, cluster_id, cluster_data) @bp.put( path="/studies/{uuid}/areas/{area_id}/clusters/thermal/{cluster_id}/form", @@ -2556,8 +2556,8 @@ def duplicate_cluster( uuid: str, area_id: str, cluster_type: ClusterType, - source_cluster_id: str, - new_cluster_name: str = Query(..., alias="newName", title="New Cluster Name"), + source_cluster_id: LowerCaseStr, + new_cluster_name: LowerCaseStr = Query(..., alias="newName", title="New Cluster Name"), current_user: JWTUser = Depends(auth.get_current_user), ) -> t.Union[STStorageOutput, ThermalClusterOutput, RenewableClusterOutput]: logger.info( @@ -2577,6 +2577,6 @@ def duplicate_cluster( else: # pragma: no cover raise NotImplementedError(f"Cluster type {cluster_type} not implemented") - return manager.duplicate_cluster(study, area_id, source_cluster_id.lower(), new_cluster_name) + return manager.duplicate_cluster(study, area_id, source_cluster_id, new_cluster_name) return bp diff --git a/tests/integration/variant_blueprint/test_thermal_cluster.py b/tests/integration/variant_blueprint/test_thermal_cluster.py index ca9ceb0ae8..ef20d9d3ab 100644 --- a/tests/integration/variant_blueprint/test_thermal_cluster.py +++ b/tests/integration/variant_blueprint/test_thermal_cluster.py @@ -97,7 +97,7 @@ def test_cascade_update( "action": "create_cluster", "args": { "area_id": area_id, - "cluster_name": transform_name_to_id(cluster_name, lower=False), + "cluster_name": transform_name_to_id(cluster_name), "parameters": _create_thermal_params(cluster_name), "prepro": np.random.rand(8760, 6).tolist(), "modulation": np.random.rand(8760, 4).tolist(), diff --git a/tests/storage/repository/filesystem/config/test_utils.py b/tests/storage/repository/filesystem/config/test_utils.py index c22e2293fc..de31252956 100644 --- a/tests/storage/repository/filesystem/config/test_utils.py +++ b/tests/storage/repository/filesystem/config/test_utils.py @@ -29,8 +29,7 @@ def test_transform_name_to_id__nominal_case(name, expected): @pytest.mark.parametrize("name", VALID_CHARS) def test_transform_name_to_id__valid_chars(name): - assert transform_name_to_id(name, lower=True) == name.lower() - assert transform_name_to_id(name, lower=False) == name + assert transform_name_to_id(name) == name.lower() @pytest.mark.parametrize("name", sorted(set(string.punctuation) - set(VALID_CHARS))) diff --git a/tests/variantstudy/model/command/test_create_cluster.py b/tests/variantstudy/model/command/test_create_cluster.py index 8142edbca7..b6a03874f5 100644 --- a/tests/variantstudy/model/command/test_create_cluster.py +++ b/tests/variantstudy/model/command/test_create_cluster.py @@ -98,9 +98,9 @@ def test_validate_modulation(self, command_context: CommandContext): def test_apply(self, empty_study: FileStudy, command_context: CommandContext): study_path = empty_study.config.study_path area_name = "DE" - area_id = transform_name_to_id(area_name, lower=True) + area_id = transform_name_to_id(area_name) cluster_name = "Cluster-1" - cluster_id = transform_name_to_id(cluster_name, lower=True) + cluster_id = transform_name_to_id(cluster_name) CreateArea(area_name=area_name, command_context=command_context, study_version=STUDY_VERSION_8_8).apply( empty_study diff --git a/tests/variantstudy/model/command/test_create_renewables_cluster.py b/tests/variantstudy/model/command/test_create_renewables_cluster.py index c777840e82..aa4eeff60e 100644 --- a/tests/variantstudy/model/command/test_create_renewables_cluster.py +++ b/tests/variantstudy/model/command/test_create_renewables_cluster.py @@ -71,7 +71,7 @@ def test_apply(self, empty_study: FileStudy, command_context: CommandContext) -> empty_study.config.version = study_version study_path = empty_study.config.study_path area_name = "DE" - area_id = transform_name_to_id(area_name, lower=True) + area_id = transform_name_to_id(area_name) cluster_name = "Cluster-1" CreateArea(area_name=area_name, command_context=command_context, study_version=study_version).apply(empty_study) diff --git a/tests/variantstudy/model/command/test_remove_cluster.py b/tests/variantstudy/model/command/test_remove_cluster.py index 8614d520a8..6cb8cf2be2 100644 --- a/tests/variantstudy/model/command/test_remove_cluster.py +++ b/tests/variantstudy/model/command/test_remove_cluster.py @@ -37,7 +37,7 @@ def test_apply(self, empty_study: FileStudy, command_context: CommandContext) -> area_name = "Area_name" area_id = transform_name_to_id(area_name) cluster_name = "Cluster Name" - cluster_id = transform_name_to_id(cluster_name, lower=False) + cluster_id = transform_name_to_id(cluster_name) study_version = empty_study.config.version diff --git a/tests/variantstudy/model/command/test_remove_link.py b/tests/variantstudy/model/command/test_remove_link.py index 8fb6930349..4032c32384 100644 --- a/tests/variantstudy/model/command/test_remove_link.py +++ b/tests/variantstudy/model/command/test_remove_link.py @@ -98,7 +98,7 @@ def test_apply(self, tmpdir: Path, command_context: CommandContext, version: int study_version = empty_study.config.version # Create some areas - areas = {transform_name_to_id(area, lower=True): area for area in ["Area_X", "Area_Y", "Area_Z"]} + areas = {transform_name_to_id(area): area for area in ["Area_X", "Area_Y", "Area_Z"]} for area in areas.values(): output = CreateArea(area_name=area, command_context=command_context, study_version=study_version).apply( empty_study diff --git a/tests/variantstudy/model/command/test_remove_renewables_cluster.py b/tests/variantstudy/model/command/test_remove_renewables_cluster.py index b7af20cd48..26a4daade1 100644 --- a/tests/variantstudy/model/command/test_remove_renewables_cluster.py +++ b/tests/variantstudy/model/command/test_remove_renewables_cluster.py @@ -33,7 +33,7 @@ def test_apply(self, empty_study: FileStudy, command_context: CommandContext) -> area_name = "Area_name" area_id = transform_name_to_id(area_name) cluster_name = "Cluster Name" - cluster_id = transform_name_to_id(cluster_name, lower=False) + cluster_id = transform_name_to_id(cluster_name) output = CreateArea(area_name=area_name, command_context=command_context, study_version=study_version).apply( empty_study