From 0612258219d0090398382b81c3fc88b5c5129dae Mon Sep 17 00:00:00 2001 From: hatim dinia Date: Thu, 25 Jan 2024 16:30:01 +0100 Subject: [PATCH 1/7] test(api-bc): add case sensitivity tests for BC term IDs and improve coverage --- .../test_binding_constraints.py | 216 ++++++++++++++++-- 1 file changed, 203 insertions(+), 13 deletions(-) diff --git a/tests/integration/study_data_blueprint/test_binding_constraints.py b/tests/integration/study_data_blueprint/test_binding_constraints.py index f0f08049d3..0f32606b32 100644 --- a/tests/integration/study_data_blueprint/test_binding_constraints.py +++ b/tests/integration/study_data_blueprint/test_binding_constraints.py @@ -3,7 +3,7 @@ @pytest.mark.unit_test -class TestSTStorage: +class TestBindingConstraints: """ Test the end points related to binding constraints. """ @@ -11,6 +11,7 @@ class TestSTStorage: def test_lifecycle__nominal(self, client: TestClient, user_access_token: str) -> None: user_headers = {"Authorization": f"Bearer {user_access_token}"} + # Create a Study res = client.post( "/v1/studies", headers=user_headers, @@ -19,6 +20,7 @@ def test_lifecycle__nominal(self, client: TestClient, user_access_token: str) -> assert res.status_code == 201, res.json() study_id = res.json() + # Create Areas area1_name = "area1" area2_name = "area2" res = client.post( @@ -43,6 +45,7 @@ def test_lifecycle__nominal(self, client: TestClient, user_access_token: str) -> ) assert res.status_code == 200, res.json() + # Create a link between the two areas res = client.post( f"/v1/studies/{study_id}/links", headers=user_headers, @@ -53,6 +56,193 @@ def test_lifecycle__nominal(self, client: TestClient, user_access_token: str) -> ) assert res.status_code == 200, res.json() + # Create a cluster in area1 + res = client.post( + f"/v1/studies/{study_id}/areas/area1/clusters/thermal", + headers=user_headers, + json={ + "name": "cluster1", + "group": "Nuclear", + }, + ) + assert res.status_code == 200, res.json() + cluster_id = res.json()["id"] + assert cluster_id == "Cluster 1" + + # Get clusters list to check created cluster in area1 + res = client.get(f"/v1/studies/{study_id}/areas/area1/clusters/thermal", headers=user_headers) + clusters_list = res.json() + assert res.status_code == 200, res.json() + assert len(clusters_list) == 1 + assert clusters_list[0]["name"] == "cluster1" + assert clusters_list[0]["group"] == "Nuclear" + + # Create two binding constraints in the study + res = client.post( + f"/v1/studies/{study_id}/bindingconstraints", + json={ + "name": "binding_constraint_1", + "enabled": True, + "time_step": "hourly", + "operator": "less", + "coeffs": {}, + "comments": "", + }, + headers=user_headers, + ) + assert res.status_code == 200, res.json() + + res = client.post( + f"/v1/studies/{study_id}/bindingconstraints", + json={ + "name": "binding_constraint_2", + "enabled": True, + "time_step": "hourly", + "operator": "less", + "coeffs": {}, + "comments": "", + }, + headers=user_headers, + ) + assert res.status_code == 200, res.json() + + # Get binding constraints list to check created binding constraints + res = client.get(f"/v1/studies/{study_id}/bindingconstraints", headers=user_headers) + binding_constraints_list = res.json() + assert res.status_code == 200, res.json() + assert len(binding_constraints_list) == 2 + assert binding_constraints_list[0]["id"] == "binding_constraint_1" + assert binding_constraints_list[1]["id"] == "binding_constraint_2" + + bc_id = binding_constraints_list[0]["id"] + + # Add binding constraint link term + res = client.post( + f"/v1/studies/{study_id}/bindingconstraints/{bc_id}/term", + json={ + "weight": 1, + "offset": 2, + "data": {"area1": area1_name, "area2": area2_name}, + }, + headers=user_headers, + ) + assert res.status_code == 200, res.json() + + # Add binding constraint cluster term + res = client.post( + f"/v1/studies/{study_id}/bindingconstraints/{bc_id}/term", + json={ + "weight": 1, + "offset": 2, + "data": {"area": area1_name, "cluster": "cluster1"}, + }, + headers=user_headers, + ) + assert res.status_code == 200, res.json() + + # Get binding constraints list to check added terms + res = client.get( + f"/v1/studies/{study_id}/bindingconstraints/{bc_id}", + headers=user_headers, + ) + binding_constraint = res.json() + constraints = binding_constraint["constraints"] + assert res.status_code == 200, res.json() + assert binding_constraint["id"] == bc_id + assert len(constraints) == 2 + assert constraints[0]["id"] == f"{area1_name}%{area2_name}" + assert constraints[0]["weight"] == 1 + assert constraints[0]["offset"] == 2 + assert constraints[0]["data"]["area1"] == area1_name + assert constraints[0]["data"]["area2"] == area2_name + assert constraints[1]["id"] == f"{area1_name}.cluster1" + assert constraints[1]["weight"] == 1 + assert constraints[1]["offset"] == 2 + assert constraints[1]["data"]["area"] == area1_name + assert constraints[1]["data"]["cluster"] == "cluster1" + + # Update constraint cluster term + res = client.put( + f"/v1/studies/{study_id}/bindingconstraints/{bc_id}/term", + json={ + "id": f"{area1_name}.cluster1", + "weight": 3, + }, + headers=user_headers, + ) + assert res.status_code == 200, res.json() + + # Get binding constraints list to check updated term + res = client.get( + f"/v1/studies/{study_id}/bindingconstraints/{bc_id}", + headers=user_headers, + ) + binding_constraint = res.json() + constraints = binding_constraint["constraints"] + assert res.status_code == 200, res.json() + assert binding_constraint["id"] == bc_id + assert len(constraints) == 2 + assert constraints[1]["id"] == f"{area1_name}.cluster1" + assert constraints[1]["weight"] == 3 + assert constraints[1]["offset"] is None + assert constraints[1]["data"]["area"] == area1_name + assert constraints[1]["data"]["cluster"] == "cluster1" + + # Update constraint cluster term with case-insensitive id + res = client.put( + f"/v1/studies/{study_id}/bindingconstraints/{bc_id}/term", + json={ + "id": f"{area1_name}.Cluster1", + "weight": 4, + }, + headers=user_headers, + ) + assert res.status_code == 200, res.json() + + # The term should be successfully updated + res = client.get( + f"/v1/studies/{study_id}/bindingconstraints/{bc_id}", + headers=user_headers, + ) + binding_constraint = res.json() + constraints = binding_constraint["constraints"] + assert res.status_code == 200, res.json() + assert binding_constraint["id"] == bc_id + assert len(constraints) == 2 + assert constraints[1]["id"] == f"{area1_name}.cluster1" + assert constraints[1]["weight"] == 4 + + # Update constraint cluster term with invalid id + res = client.put( + f"/v1/studies/{study_id}/bindingconstraints/{bc_id}/term", + json={ + "id": f"{area1_name}.cluster2", + "weight": 4, + }, + headers=user_headers, + ) + assert res.status_code == 404, res.json() + assert res.json() == { + "description": f"{study_id}", + "exception": "ConstraintIdNotFoundError", + } + + # Update constraint cluster term with empty data + res = client.put( + f"/v1/studies/{study_id}/bindingconstraints/{bc_id}/term", + json={ + "id": f"{area1_name}.cluster1", + "data": {}, + }, + headers=user_headers, + ) + assert res.status_code == 422, res.json() + assert res.json() == { + "body": {"data": {}, "id": "area1.cluster1"}, + "description": "field required", + "exception": "RequestValidationError", + } + # Create Variant res = client.post( f"/v1/studies/{study_id}/variants", @@ -69,7 +259,7 @@ def test_lifecycle__nominal(self, client: TestClient, user_access_token: str) -> { "action": "create_binding_constraint", "args": { - "name": "binding_constraint_1", + "name": "binding_constraint_3", "enabled": True, "time_step": "hourly", "operator": "less", @@ -88,7 +278,7 @@ def test_lifecycle__nominal(self, client: TestClient, user_access_token: str) -> { "action": "create_binding_constraint", "args": { - "name": "binding_constraint_2", + "name": "binding_constraint_4", "enabled": True, "time_step": "hourly", "operator": "less", @@ -105,11 +295,11 @@ def test_lifecycle__nominal(self, client: TestClient, user_access_token: str) -> res = client.get(f"/v1/studies/{variant_id}/bindingconstraints", headers=user_headers) binding_constraints_list = res.json() assert res.status_code == 200, res.json() - assert len(binding_constraints_list) == 2 - assert binding_constraints_list[0]["id"] == "binding_constraint_1" - assert binding_constraints_list[1]["id"] == "binding_constraint_2" + assert len(binding_constraints_list) == 4 + assert binding_constraints_list[2]["id"] == "binding_constraint_3" + assert binding_constraints_list[3]["id"] == "binding_constraint_4" - bc_id = binding_constraints_list[0]["id"] + bc_id = binding_constraints_list[2]["id"] # Update element of Binding constraint new_comment = "We made it !" @@ -130,7 +320,7 @@ def test_lifecycle__nominal(self, client: TestClient, user_access_token: str) -> assert res.status_code == 200, res.json() assert comments == new_comment - # Add Constraint term + # Add Binding Constraint term area1_name = "area1" area2_name = "area2" @@ -209,7 +399,7 @@ def test_lifecycle__nominal(self, client: TestClient, user_access_token: str) -> res = client.post( f"/v1/studies/{variant_id}/bindingconstraints", json={ - "name": "binding_constraint_3", + "name": "binding_constraint_5", "enabled": True, "time_step": "hourly", "operator": "less", @@ -224,7 +414,7 @@ def test_lifecycle__nominal(self, client: TestClient, user_access_token: str) -> res = client.post( f"/v1/studies/{variant_id}/bindingconstraints", json={ - "name": "binding_constraint_3", + "name": "binding_constraint_5", "enabled": True, "time_step": "hourly", "operator": "less", @@ -235,7 +425,7 @@ def test_lifecycle__nominal(self, client: TestClient, user_access_token: str) -> ) assert res.status_code == 409, res.json() assert res.json() == { - "description": "A binding constraint with the same name already exists: binding_constraint_3.", + "description": "A binding constraint with the same name already exists: binding_constraint_5.", "exception": "DuplicateConstraintName", } @@ -277,10 +467,10 @@ def test_lifecycle__nominal(self, client: TestClient, user_access_token: str) -> "exception": "InvalidConstraintName", } - # Asserts that only 3 binding constraints have been created + # Asserts that 5 binding constraints have been created res = client.get(f"/v1/studies/{variant_id}/bindingconstraints", headers=user_headers) assert res.status_code == 200, res.json() - assert len(res.json()) == 3 + assert len(res.json()) == 5 # The user change the time_step to daily instead of hourly. # We must check that the matrix is a daily/weekly matrix. From 25c5c9a02262c52a33fa893e21e2cfa71985dc5b Mon Sep 17 00:00:00 2001 From: hatim dinia Date: Thu, 25 Jan 2024 16:32:05 +0100 Subject: [PATCH 2/7] fix(api-bc): correct typing errors and update `update_constraint_term` --- .../business/binding_constraint_management.py | 45 +++++++++---------- 1 file changed, 20 insertions(+), 25 deletions(-) diff --git a/antarest/study/business/binding_constraint_management.py b/antarest/study/business/binding_constraint_management.py index c9eb01d9fa..4816666c66 100644 --- a/antarest/study/business/binding_constraint_management.py +++ b/antarest/study/business/binding_constraint_management.py @@ -304,45 +304,40 @@ def update_constraint_term( self, study: Study, binding_constraint_id: str, - data: Union[ConstraintTermDTO, str], + term: Union[ConstraintTermDTO, str], ) -> None: file_study = self.storage_service.get_storage(study).get_raw(study) constraint = self.get_binding_constraint(study, binding_constraint_id) + if not isinstance(constraint, BindingConstraintDTO): raise NoBindingConstraintError(study.id) - constraints = constraint.constraints - if constraints is None: + constraint_terms = constraint.constraints # existing constraint terms + if constraint_terms is None: raise NoConstraintError(study.id) - data_id = data.id if isinstance(data, ConstraintTermDTO) else data - if data_id is None: - raise ConstraintIdNotFoundError(study.id) + term_id = term.id if isinstance(term, ConstraintTermDTO) else term + if term_id is None: + raise NoConstraintError(study.id) - data_term_index = BindingConstraintManager.find_constraint_term_id(constraints, data_id) - if data_term_index < 0: + term_id_index = BindingConstraintManager.find_constraint_term_id(constraint_terms, term_id) + if term_id_index < 0: raise ConstraintIdNotFoundError(study.id) - if isinstance(data, ConstraintTermDTO): - constraint_id = BindingConstraintManager.get_constraint_id(data.data) if data.data is not None else data_id - current_constraint = constraints[data_term_index] - constraints.append( - ConstraintTermDTO( - id=constraint_id, - weight=data.weight if data.weight is not None else current_constraint.weight, - offset=data.offset, - data=data.data if data.data is not None else current_constraint.data, - ) + if isinstance(term, ConstraintTermDTO): + updated_term_id = BindingConstraintManager.get_constraint_id(term.data) if term.data else term_id + current_constraint = constraint_terms[term_id_index] + + constraint_terms[term_id_index] = ConstraintTermDTO( + id=updated_term_id, + weight=term.weight or current_constraint.weight, + offset=term.offset, + data=term.data or current_constraint.data, ) - del constraints[data_term_index] else: - del constraints[data_term_index] + del constraint_terms[term_id_index] - coeffs = {} - for term in constraints: - coeffs[term.id] = [term.weight] - if term.offset is not None: - coeffs[term.id].append(term.offset) + coeffs = {term.id: [term.weight, term.offset] if term.offset else [term.weight] for term in constraint_terms} command = UpdateBindingConstraint( id=constraint.id, From 4c7a1a7486b155a823ab25b2efefc576824c4962 Mon Sep 17 00:00:00 2001 From: hatim dinia Date: Tue, 23 Jan 2024 14:54:57 +0100 Subject: [PATCH 3/7] fix(ui-bc): normalize cluster IDs to lowercase in BC options --- .../BindingConstView/ConstraintTerm/OptionsList.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/webapp/src/components/App/Singlestudy/explore/Modelization/BindingConstraints/BindingConstView/ConstraintTerm/OptionsList.tsx b/webapp/src/components/App/Singlestudy/explore/Modelization/BindingConstraints/BindingConstView/ConstraintTerm/OptionsList.tsx index 0cdf1ea8cd..3fbfae70f3 100644 --- a/webapp/src/components/App/Singlestudy/explore/Modelization/BindingConstraints/BindingConstView/ConstraintTerm/OptionsList.tsx +++ b/webapp/src/components/App/Singlestudy/explore/Modelization/BindingConstraints/BindingConstView/ConstraintTerm/OptionsList.tsx @@ -63,7 +63,7 @@ export default function OptionsList(props: Props) { ) .map((elm) => ({ name: elm.name, - id: elm.id, + id: elm.id.toLowerCase(), })); return tmp; }, [constraintsTerm, isLink, options, value1, value2]); @@ -144,7 +144,7 @@ export default function OptionsList(props: Props) { name="value2" list={options2} label={t(`study.${name2}`)} - data={value2} + data={value2.toLowerCase()} handleChange={(key, value) => handleValue2(value as string)} sx={{ flexGrow: 1, From dd31485af66daac59dc8f969f3db0b99780464b9 Mon Sep 17 00:00:00 2001 From: hatim dinia Date: Tue, 23 Jan 2024 15:05:39 +0100 Subject: [PATCH 4/7] style(api): update BC endpoints info --- antarest/study/web/study_data_blueprint.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/antarest/study/web/study_data_blueprint.py b/antarest/study/web/study_data_blueprint.py index 0ebd603a21..3bd394f0e6 100644 --- a/antarest/study/web/study_data_blueprint.py +++ b/antarest/study/web/study_data_blueprint.py @@ -873,7 +873,7 @@ def get_binding_constraint_list( @bp.get( "/studies/{uuid}/bindingconstraints/{binding_constraint_id}", tags=[APITag.study_data], - summary="Get binding constraint list", + summary="Get binding constraint", response_model=None, # Dict[str, bool], ) def get_binding_constraint( @@ -929,45 +929,45 @@ def create_binding_constraint( @bp.post( "/studies/{uuid}/bindingconstraints/{binding_constraint_id}/term", tags=[APITag.study_data], - summary="update constraint term", + summary="Create a binding constraint term", ) def add_constraint_term( uuid: str, binding_constraint_id: str, - data: ConstraintTermDTO, + term: ConstraintTermDTO, current_user: JWTUser = Depends(auth.get_current_user), ) -> Any: logger.info( - f"add constraint term from {binding_constraint_id} for study {uuid}", + f"Add constraint term {term.id} to {binding_constraint_id} for study {uuid}", extra={"user": current_user.id}, ) params = RequestParameters(user=current_user) study = study_service.check_study_access(uuid, StudyPermissionType.WRITE, params) - return study_service.binding_constraint_manager.add_new_constraint_term(study, binding_constraint_id, data) + return study_service.binding_constraint_manager.add_new_constraint_term(study, binding_constraint_id, term) @bp.put( "/studies/{uuid}/bindingconstraints/{binding_constraint_id}/term", tags=[APITag.study_data], - summary="update constraint term", + summary="Update a binding constraint term", ) def update_constraint_term( uuid: str, binding_constraint_id: str, - data: ConstraintTermDTO, + term: ConstraintTermDTO, current_user: JWTUser = Depends(auth.get_current_user), ) -> Any: logger.info( - f"update constraint term from {binding_constraint_id} for study {uuid}", + f"Update constraint term {term.id} from {binding_constraint_id} for study {uuid}", extra={"user": current_user.id}, ) params = RequestParameters(user=current_user) study = study_service.check_study_access(uuid, StudyPermissionType.WRITE, params) - return study_service.binding_constraint_manager.update_constraint_term(study, binding_constraint_id, data) + return study_service.binding_constraint_manager.update_constraint_term(study, binding_constraint_id, term) @bp.delete( "/studies/{uuid}/bindingconstraints/{binding_constraint_id}/term/{term_id}", tags=[APITag.study_data], - summary="update constraint term", + summary="Remove a binding constraint term", ) def remove_constraint_term( uuid: str, @@ -976,7 +976,7 @@ def remove_constraint_term( current_user: JWTUser = Depends(auth.get_current_user), ) -> None: logger.info( - f"delete constraint term {term_id} from {binding_constraint_id} for study {uuid}", + f"Remove constraint term {term_id} from {binding_constraint_id} for study {uuid}", extra={"user": current_user.id}, ) params = RequestParameters(user=current_user) From e8954595397fc38606811f972791b78eafbd3903 Mon Sep 17 00:00:00 2001 From: hatim dinia Date: Wed, 24 Jan 2024 14:53:25 +0100 Subject: [PATCH 5/7] test(api-bc): update BC tests to use upper and lower case names --- .../test_binding_constraints.py | 197 ++++++++++++------ 1 file changed, 128 insertions(+), 69 deletions(-) diff --git a/tests/integration/study_data_blueprint/test_binding_constraints.py b/tests/integration/study_data_blueprint/test_binding_constraints.py index 0f32606b32..627575097e 100644 --- a/tests/integration/study_data_blueprint/test_binding_constraints.py +++ b/tests/integration/study_data_blueprint/test_binding_constraints.py @@ -21,47 +21,47 @@ def test_lifecycle__nominal(self, client: TestClient, user_access_token: str) -> study_id = res.json() # Create Areas - area1_name = "area1" - area2_name = "area2" res = client.post( f"/v1/studies/{study_id}/areas", headers=user_headers, json={ - "name": area1_name, + "name": "Area 1", "type": "AREA", - "metadata": {"country": "FR"}, }, ) assert res.status_code == 200, res.json() + area1_id = res.json()["id"] + assert area1_id == "area 1" res = client.post( f"/v1/studies/{study_id}/areas", headers=user_headers, json={ - "name": area2_name, + "name": "Area 2", "type": "AREA", - "metadata": {"country": "DE"}, }, ) assert res.status_code == 200, res.json() + area2_id = res.json()["id"] + assert area2_id == "area 2" # Create a link between the two areas res = client.post( f"/v1/studies/{study_id}/links", headers=user_headers, json={ - "area1": area1_name, - "area2": area2_name, + "area1": area1_id, + "area2": area2_id, }, ) assert res.status_code == 200, res.json() # Create a cluster in area1 res = client.post( - f"/v1/studies/{study_id}/areas/area1/clusters/thermal", + f"/v1/studies/{study_id}/areas/{area1_id}/clusters/thermal", headers=user_headers, json={ - "name": "cluster1", + "name": "Cluster 1", "group": "Nuclear", }, ) @@ -70,18 +70,19 @@ def test_lifecycle__nominal(self, client: TestClient, user_access_token: str) -> assert cluster_id == "Cluster 1" # Get clusters list to check created cluster in area1 - res = client.get(f"/v1/studies/{study_id}/areas/area1/clusters/thermal", headers=user_headers) + res = client.get(f"/v1/studies/{study_id}/areas/{area1_id}/clusters/thermal", headers=user_headers) clusters_list = res.json() assert res.status_code == 200, res.json() assert len(clusters_list) == 1 - assert clusters_list[0]["name"] == "cluster1" + assert clusters_list[0]["id"] == cluster_id + assert clusters_list[0]["name"] == "Cluster 1" assert clusters_list[0]["group"] == "Nuclear" - # Create two binding constraints in the study + # Create Binding Constraints res = client.post( f"/v1/studies/{study_id}/bindingconstraints", json={ - "name": "binding_constraint_1", + "name": "Binding Constraint 1", "enabled": True, "time_step": "hourly", "operator": "less", @@ -95,7 +96,7 @@ def test_lifecycle__nominal(self, client: TestClient, user_access_token: str) -> res = client.post( f"/v1/studies/{study_id}/bindingconstraints", json={ - "name": "binding_constraint_2", + "name": "Binding Constraint 2", "enabled": True, "time_step": "hourly", "operator": "less", @@ -106,13 +107,51 @@ def test_lifecycle__nominal(self, client: TestClient, user_access_token: str) -> ) assert res.status_code == 200, res.json() - # Get binding constraints list to check created binding constraints + # Asserts that creating 2 binding constraints with the same name raises an Exception + res = client.post( + f"/v1/studies/{study_id}/bindingconstraints", + json={ + "name": "Binding Constraint 1", + "enabled": True, + "time_step": "hourly", + "operator": "less", + "coeffs": {}, + "comments": "", + }, + headers=user_headers, + ) + assert res.status_code == 409, res.json() + + # Get Binding Constraint list to check created binding constraints res = client.get(f"/v1/studies/{study_id}/bindingconstraints", headers=user_headers) binding_constraints_list = res.json() - assert res.status_code == 200, res.json() - assert len(binding_constraints_list) == 2 - assert binding_constraints_list[0]["id"] == "binding_constraint_1" - assert binding_constraints_list[1]["id"] == "binding_constraint_2" + expected = [ + { + "id": "binding constraint 1", + "name": "Binding Constraint 1", + "enabled": True, + "time_step": "hourly", + "operator": "less", + "constraints": None, # terms + "values": None, + "filter_year_by_year": "", + "filter_synthesis": "", + "comments": "", + }, + { + "id": "binding constraint 2", + "name": "Binding Constraint 2", + "enabled": True, + "time_step": "hourly", + "operator": "less", + "constraints": None, # terms + "values": None, + "filter_year_by_year": "", + "filter_synthesis": "", + "comments": "", + }, + ] + assert binding_constraints_list == expected bc_id = binding_constraints_list[0]["id"] @@ -122,7 +161,7 @@ def test_lifecycle__nominal(self, client: TestClient, user_access_token: str) -> json={ "weight": 1, "offset": 2, - "data": {"area1": area1_name, "area2": area2_name}, + "data": {"area1": area1_id, "area2": area2_id}, }, headers=user_headers, ) @@ -134,7 +173,7 @@ def test_lifecycle__nominal(self, client: TestClient, user_access_token: str) -> json={ "weight": 1, "offset": 2, - "data": {"area": area1_name, "cluster": "cluster1"}, + "data": {"area": area1_id, "cluster": cluster_id}, }, headers=user_headers, ) @@ -145,27 +184,30 @@ def test_lifecycle__nominal(self, client: TestClient, user_access_token: str) -> f"/v1/studies/{study_id}/bindingconstraints/{bc_id}", headers=user_headers, ) - binding_constraint = res.json() - constraints = binding_constraint["constraints"] assert res.status_code == 200, res.json() - assert binding_constraint["id"] == bc_id - assert len(constraints) == 2 - assert constraints[0]["id"] == f"{area1_name}%{area2_name}" - assert constraints[0]["weight"] == 1 - assert constraints[0]["offset"] == 2 - assert constraints[0]["data"]["area1"] == area1_name - assert constraints[0]["data"]["area2"] == area2_name - assert constraints[1]["id"] == f"{area1_name}.cluster1" - assert constraints[1]["weight"] == 1 - assert constraints[1]["offset"] == 2 - assert constraints[1]["data"]["area"] == area1_name - assert constraints[1]["data"]["cluster"] == "cluster1" + binding_constraint = res.json() + constraint_terms = binding_constraint["constraints"] + expected = [ + { + "data": {"area1": area1_id, "area2": area2_id}, + "id": f"{area1_id}%{area2_id}", + "offset": 2.0, + "weight": 1.0, + }, + { + "data": {"area": area1_id, "cluster": cluster_id}, + "id": f"{area1_id}.{cluster_id}", + "offset": 2.0, + "weight": 1.0, + }, + ] + assert constraint_terms == expected # Update constraint cluster term res = client.put( f"/v1/studies/{study_id}/bindingconstraints/{bc_id}/term", json={ - "id": f"{area1_name}.cluster1", + "id": f"{area1_id}.{cluster_id}", "weight": 3, }, headers=user_headers, @@ -177,22 +219,30 @@ def test_lifecycle__nominal(self, client: TestClient, user_access_token: str) -> f"/v1/studies/{study_id}/bindingconstraints/{bc_id}", headers=user_headers, ) - binding_constraint = res.json() - constraints = binding_constraint["constraints"] assert res.status_code == 200, res.json() - assert binding_constraint["id"] == bc_id - assert len(constraints) == 2 - assert constraints[1]["id"] == f"{area1_name}.cluster1" - assert constraints[1]["weight"] == 3 - assert constraints[1]["offset"] is None - assert constraints[1]["data"]["area"] == area1_name - assert constraints[1]["data"]["cluster"] == "cluster1" - - # Update constraint cluster term with case-insensitive id + binding_constraint = res.json() + constraint_terms = binding_constraint["constraints"] + expected = [ + { + "data": {"area1": area1_id, "area2": area2_id}, + "id": f"{area1_id}%{area2_id}", + "offset": 2.0, + "weight": 1.0, + }, + { + "data": {"area": area1_id, "cluster": cluster_id}, + "id": f"{area1_id}.{cluster_id}", + "offset": None, # updated + "weight": 3.0, # updated + }, + ] + assert constraint_terms == expected + + # Update constraint term regardless of the case of the cluster id res = client.put( f"/v1/studies/{study_id}/bindingconstraints/{bc_id}/term", json={ - "id": f"{area1_name}.Cluster1", + "id": f"{area1_id}.Cluster 1", "weight": 4, }, headers=user_headers, @@ -204,19 +254,30 @@ def test_lifecycle__nominal(self, client: TestClient, user_access_token: str) -> f"/v1/studies/{study_id}/bindingconstraints/{bc_id}", headers=user_headers, ) - binding_constraint = res.json() - constraints = binding_constraint["constraints"] assert res.status_code == 200, res.json() - assert binding_constraint["id"] == bc_id - assert len(constraints) == 2 - assert constraints[1]["id"] == f"{area1_name}.cluster1" - assert constraints[1]["weight"] == 4 + binding_constraint = res.json() + constraint_terms = binding_constraint["constraints"] + expected = [ + { + "data": {"area1": area1_id, "area2": area2_id}, + "id": f"{area1_id}%{area2_id}", + "offset": 2.0, + "weight": 1.0, + }, + { + "data": {"area": area1_id, "cluster": cluster_id}, + "id": f"{area1_id}.{cluster_id}", + "offset": None, # updated + "weight": 4.0, # updated + }, + ] + assert constraint_terms == expected # Update constraint cluster term with invalid id res = client.put( f"/v1/studies/{study_id}/bindingconstraints/{bc_id}/term", json={ - "id": f"{area1_name}.cluster2", + "id": f"{area1_id}.!!Invalid#cluster%%", "weight": 4, }, headers=user_headers, @@ -231,14 +292,14 @@ def test_lifecycle__nominal(self, client: TestClient, user_access_token: str) -> res = client.put( f"/v1/studies/{study_id}/bindingconstraints/{bc_id}/term", json={ - "id": f"{area1_name}.cluster1", + "id": f"{area1_id}.{cluster_id}", "data": {}, }, headers=user_headers, ) assert res.status_code == 422, res.json() assert res.json() == { - "body": {"data": {}, "id": "area1.cluster1"}, + "body": {"data": {}, "id": f"{area1_id}.{cluster_id}"}, "description": "field required", "exception": "RequestValidationError", } @@ -321,15 +382,13 @@ def test_lifecycle__nominal(self, client: TestClient, user_access_token: str) -> assert comments == new_comment # Add Binding Constraint term - area1_name = "area1" - area2_name = "area2" res = client.post( f"/v1/studies/{variant_id}/bindingconstraints/{bc_id}/term", json={ "weight": 1, "offset": 2, - "data": {"area1": area1_name, "area2": area2_name}, + "data": {"area1": area1_id, "area2": area2_id}, }, headers=user_headers, ) @@ -345,17 +404,17 @@ def test_lifecycle__nominal(self, client: TestClient, user_access_token: str) -> assert res.status_code == 200, res.json() assert binding_constraint["id"] == bc_id assert len(constraints) == 1 - assert constraints[0]["id"] == f"{area1_name}%{area2_name}" + assert constraints[0]["id"] == f"{area1_id}%{area2_id}" assert constraints[0]["weight"] == 1 assert constraints[0]["offset"] == 2 - assert constraints[0]["data"]["area1"] == area1_name - assert constraints[0]["data"]["area2"] == area2_name + assert constraints[0]["data"]["area1"] == area1_id + assert constraints[0]["data"]["area2"] == area2_id # Update Constraint term res = client.put( f"/v1/studies/{variant_id}/bindingconstraints/{bc_id}/term", json={ - "id": f"{area1_name}%{area2_name}", + "id": f"{area1_id}%{area2_id}", "weight": 3, }, headers=user_headers, @@ -372,15 +431,15 @@ def test_lifecycle__nominal(self, client: TestClient, user_access_token: str) -> assert res.status_code == 200, res.json() assert binding_constraint["id"] == bc_id assert len(constraints) == 1 - assert constraints[0]["id"] == f"{area1_name}%{area2_name}" + assert constraints[0]["id"] == f"{area1_id}%{area2_id}" assert constraints[0]["weight"] == 3 assert constraints[0]["offset"] is None - assert constraints[0]["data"]["area1"] == area1_name - assert constraints[0]["data"]["area2"] == area2_name + assert constraints[0]["data"]["area1"] == area1_id + assert constraints[0]["data"]["area2"] == area2_id # Remove Constraint term res = client.delete( - f"/v1/studies/{variant_id}/bindingconstraints/{bc_id}/term/{area1_name}%{area2_name}", + f"/v1/studies/{variant_id}/bindingconstraints/{bc_id}/term/{area1_id}%{area2_id}", headers=user_headers, ) assert res.status_code == 200, res.json() From 000c97e7610a26d7f619c093dd4f2d29098a5d4b Mon Sep 17 00:00:00 2001 From: hatim dinia Date: Thu, 25 Jan 2024 16:30:54 +0100 Subject: [PATCH 6/7] fix(api-bc): lower case cluster id for term comparison --- .../storage/variantstudy/business/utils_binding_constraint.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/antarest/study/storage/variantstudy/business/utils_binding_constraint.py b/antarest/study/storage/variantstudy/business/utils_binding_constraint.py index db6ea4d1db..f1ef937cdf 100644 --- a/antarest/study/storage/variantstudy/business/utils_binding_constraint.py +++ b/antarest/study/storage/variantstudy/business/utils_binding_constraint.py @@ -53,7 +53,7 @@ def apply_binding_constraint( # Cluster IDs are stored in lower case in the binding constraints file. area, cluster_id = link_or_cluster.split(".") thermal_ids = {thermal.id.lower() for thermal in study_data.config.areas[area].thermals} - if area not in study_data.config.areas or cluster_id not in thermal_ids: + if area not in study_data.config.areas or cluster_id.lower() not in thermal_ids: return CommandOutput( status=False, message=f"Cluster '{link_or_cluster}' does not exist in binding constraint '{bd_id}'", From b599f09b115f93b0d63610c0ba82cbcf902267c9 Mon Sep 17 00:00:00 2001 From: hatim dinia Date: Tue, 30 Jan 2024 09:29:18 +0100 Subject: [PATCH 7/7] feat(api-bc): improve the calculation of BC terms IDs --- antarest/core/exceptions.py | 2 +- .../business/binding_constraint_management.py | 88 ++++++++++---- .../test_binding_constraints.py | 111 +++++++++++------- 3 files changed, 135 insertions(+), 66 deletions(-) diff --git a/antarest/core/exceptions.py b/antarest/core/exceptions.py index 3414d6477b..9a2230c1d1 100644 --- a/antarest/core/exceptions.py +++ b/antarest/core/exceptions.py @@ -174,7 +174,7 @@ def __init__(self, message: str) -> None: super().__init__(HTTPStatus.BAD_REQUEST, message) -class NoBindingConstraintError(HTTPException): +class BindingConstraintNotFoundError(HTTPException): def __init__(self, message: str) -> None: super().__init__(HTTPStatus.NOT_FOUND, message) diff --git a/antarest/study/business/binding_constraint_management.py b/antarest/study/business/binding_constraint_management.py index 4816666c66..55112da1da 100644 --- a/antarest/study/business/binding_constraint_management.py +++ b/antarest/study/business/binding_constraint_management.py @@ -1,14 +1,14 @@ from typing import Any, Dict, List, Optional, Union -from pydantic import BaseModel +from pydantic import BaseModel, validator from antarest.core.exceptions import ( + BindingConstraintNotFoundError, ConstraintAlreadyExistError, ConstraintIdNotFoundError, DuplicateConstraintName, InvalidConstraintName, MissingDataError, - NoBindingConstraintError, NoConstraintError, ) from antarest.matrixstore.model import MatrixData @@ -29,21 +29,72 @@ from antarest.study.storage.variantstudy.model.command.update_binding_constraint import UpdateBindingConstraint -class LinkInfoDTO(BaseModel): +class AreaLinkDTO(BaseModel): + """ + DTO for a constraint term on a link between two areas. + + Attributes: + area1: the first area ID + area2: the second area ID + """ + area1: str area2: str + def generate_id(self) -> str: + """Return the constraint term ID for this link, of the form "area1%area2".""" + # Ensure IDs are in alphabetical order and lower case + ids = sorted((self.area1.lower(), self.area2.lower())) + return "%".join(ids) + + +class AreaClusterDTO(BaseModel): + """ + DTO for a constraint term on a cluster in an area. + + Attributes: + area: the area ID + cluster: the cluster ID + """ -class ClusterInfoDTO(BaseModel): area: str cluster: str + def generate_id(self) -> str: + """Return the constraint term ID for this Area/cluster constraint, of the form "area.cluster".""" + # Ensure IDs are in lower case + ids = [self.area.lower(), self.cluster.lower()] + return ".".join(ids) + class ConstraintTermDTO(BaseModel): + """ + DTO for a constraint term. + + Attributes: + id: the constraint term ID, of the form "area1%area2" or "area.cluster". + weight: the constraint term weight, if any. + offset: the constraint term offset, if any. + data: the constraint term data (link or cluster), if any. + """ + id: Optional[str] weight: Optional[float] offset: Optional[float] - data: Optional[Union[LinkInfoDTO, ClusterInfoDTO]] + data: Optional[Union[AreaLinkDTO, AreaClusterDTO]] + + @validator("id") + def id_to_lower(cls, v: Optional[str]) -> Optional[str]: + """Ensure the ID is lower case.""" + if v is None: + return None + return v.lower() + + def generate_id(self) -> str: + """Return the constraint term ID for this term based on its data.""" + if self.data is None: + return self.id or "" + return self.data.generate_id() class UpdateBindingConstProps(BaseModel): @@ -97,12 +148,12 @@ def parse_constraint(key: str, value: str, char: str, new_config: BindingConstra id=key, weight=weight, offset=offset if offset is not None else None, - data=LinkInfoDTO( + data=AreaLinkDTO( area1=value1, area2=value2, ) if char == "%" - else ClusterInfoDTO( + else AreaClusterDTO( area=value1, cluster=value2, ), @@ -208,7 +259,7 @@ def update_binding_constraint( file_study = self.storage_service.get_storage(study).get_raw(study) constraint = self.get_binding_constraint(study, binding_constraint_id) if not isinstance(constraint, BindingConstraintDTO): - raise NoBindingConstraintError(study.id) + raise BindingConstraintNotFoundError(study.id) if data.key == "time_step" and data.value != constraint.time_step: # The user changed the time step, we need to update the matrix accordingly @@ -243,16 +294,6 @@ def find_constraint_term_id(constraints_term: List[ConstraintTermDTO], constrain except ValueError: return -1 - @staticmethod - def get_constraint_id(data: Union[LinkInfoDTO, ClusterInfoDTO]) -> str: - if isinstance(data, ClusterInfoDTO): - constraint_id = f"{data.area}.{data.cluster}" - else: - area1 = data.area1 if data.area1 < data.area2 else data.area2 - area2 = data.area2 if area1 == data.area1 else data.area1 - constraint_id = f"{area1}%{area2}" - return constraint_id - def add_new_constraint_term( self, study: Study, @@ -262,12 +303,12 @@ def add_new_constraint_term( file_study = self.storage_service.get_storage(study).get_raw(study) constraint = self.get_binding_constraint(study, binding_constraint_id) if not isinstance(constraint, BindingConstraintDTO): - raise NoBindingConstraintError(study.id) + raise BindingConstraintNotFoundError(study.id) if constraint_term.data is None: raise MissingDataError("Add new constraint term : data is missing") - constraint_id = BindingConstraintManager.get_constraint_id(constraint_term.data) + constraint_id = constraint_term.data.generate_id() constraints_term = constraint.constraints or [] if BindingConstraintManager.find_constraint_term_id(constraints_term, constraint_id) >= 0: raise ConstraintAlreadyExistError(study.id) @@ -310,7 +351,7 @@ def update_constraint_term( constraint = self.get_binding_constraint(study, binding_constraint_id) if not isinstance(constraint, BindingConstraintDTO): - raise NoBindingConstraintError(study.id) + raise BindingConstraintNotFoundError(study.id) constraint_terms = constraint.constraints # existing constraint terms if constraint_terms is None: @@ -318,14 +359,14 @@ def update_constraint_term( term_id = term.id if isinstance(term, ConstraintTermDTO) else term if term_id is None: - raise NoConstraintError(study.id) + raise ConstraintIdNotFoundError(study.id) term_id_index = BindingConstraintManager.find_constraint_term_id(constraint_terms, term_id) if term_id_index < 0: raise ConstraintIdNotFoundError(study.id) if isinstance(term, ConstraintTermDTO): - updated_term_id = BindingConstraintManager.get_constraint_id(term.data) if term.data else term_id + updated_term_id = term.data.generate_id() if term.data else term_id current_constraint = constraint_terms[term_id_index] constraint_terms[term_id_index] = ConstraintTermDTO( @@ -353,6 +394,7 @@ def update_constraint_term( ) execute_or_add_commands(study, file_study, [command], self.storage_service) + # FIXME create a dedicated delete service def remove_constraint_term( self, study: Study, diff --git a/tests/integration/study_data_blueprint/test_binding_constraints.py b/tests/integration/study_data_blueprint/test_binding_constraints.py index 627575097e..93b5237f7f 100644 --- a/tests/integration/study_data_blueprint/test_binding_constraints.py +++ b/tests/integration/study_data_blueprint/test_binding_constraints.py @@ -1,6 +1,65 @@ import pytest from starlette.testclient import TestClient +from antarest.study.business.binding_constraint_management import AreaClusterDTO, AreaLinkDTO, ConstraintTermDTO + + +class TestAreaLinkDTO: + @pytest.mark.parametrize( + "area1, area2, expected", + [ + ("Area 1", "Area 2", "area 1%area 2"), + ("de", "fr", "de%fr"), + ("fr", "de", "de%fr"), + ("FR", "de", "de%fr"), + ], + ) + def test_constraint_id(self, area1: str, area2: str, expected: str) -> None: + info = AreaLinkDTO(area1=area1, area2=area2) + assert info.generate_id() == expected + + +class TestAreaClusterDTO: + @pytest.mark.parametrize( + "area, cluster, expected", + [ + ("Area 1", "Cluster X", "area 1.cluster x"), + ("de", "Nuclear", "de.nuclear"), + ("GB", "Gas", "gb.gas"), + ], + ) + def test_constraint_id(self, area: str, cluster: str, expected: str) -> None: + info = AreaClusterDTO(area=area, cluster=cluster) + assert info.generate_id() == expected + + +class TestConstraintTermDTO: + def test_constraint_id__link(self): + term = ConstraintTermDTO( + id="foo", + weight=3.14, + offset=123, + data=AreaLinkDTO(area1="Area 1", area2="Area 2"), + ) + assert term.generate_id() == term.data.generate_id() + + def test_constraint_id__cluster(self): + term = ConstraintTermDTO( + id="foo", + weight=3.14, + offset=123, + data=AreaClusterDTO(area="Area 1", cluster="Cluster X"), + ) + assert term.generate_id() == term.data.generate_id() + + def test_constraint_id__other(self): + term = ConstraintTermDTO( + id="foo", + weight=3.14, + offset=123, + ) + assert term.generate_id() == "foo" + @pytest.mark.unit_test class TestBindingConstraints: @@ -173,7 +232,10 @@ def test_lifecycle__nominal(self, client: TestClient, user_access_token: str) -> json={ "weight": 1, "offset": 2, - "data": {"area": area1_id, "cluster": cluster_id}, + "data": { + "area": area1_id, + "cluster": cluster_id, + }, # NOTE: cluster_id in term data can be uppercase, but it must be lowercase in the returned ini configuration file }, headers=user_headers, ) @@ -195,15 +257,15 @@ def test_lifecycle__nominal(self, client: TestClient, user_access_token: str) -> "weight": 1.0, }, { - "data": {"area": area1_id, "cluster": cluster_id}, - "id": f"{area1_id}.{cluster_id}", + "data": {"area": area1_id, "cluster": cluster_id.lower()}, + "id": f"{area1_id}.{cluster_id.lower()}", "offset": 2.0, "weight": 1.0, }, ] assert constraint_terms == expected - # Update constraint cluster term + # Update constraint cluster term with uppercase cluster_id res = client.put( f"/v1/studies/{study_id}/bindingconstraints/{bc_id}/term", json={ @@ -214,7 +276,7 @@ def test_lifecycle__nominal(self, client: TestClient, user_access_token: str) -> ) assert res.status_code == 200, res.json() - # Get binding constraints list to check updated term + # Check updated terms, cluster_id should be lowercase in the returned configuration res = client.get( f"/v1/studies/{study_id}/bindingconstraints/{bc_id}", headers=user_headers, @@ -230,49 +292,14 @@ def test_lifecycle__nominal(self, client: TestClient, user_access_token: str) -> "weight": 1.0, }, { - "data": {"area": area1_id, "cluster": cluster_id}, - "id": f"{area1_id}.{cluster_id}", + "data": {"area": area1_id, "cluster": cluster_id.lower()}, + "id": f"{area1_id}.{cluster_id.lower()}", "offset": None, # updated "weight": 3.0, # updated }, ] assert constraint_terms == expected - # Update constraint term regardless of the case of the cluster id - res = client.put( - f"/v1/studies/{study_id}/bindingconstraints/{bc_id}/term", - json={ - "id": f"{area1_id}.Cluster 1", - "weight": 4, - }, - headers=user_headers, - ) - assert res.status_code == 200, res.json() - - # The term should be successfully updated - res = client.get( - f"/v1/studies/{study_id}/bindingconstraints/{bc_id}", - headers=user_headers, - ) - assert res.status_code == 200, res.json() - binding_constraint = res.json() - constraint_terms = binding_constraint["constraints"] - expected = [ - { - "data": {"area1": area1_id, "area2": area2_id}, - "id": f"{area1_id}%{area2_id}", - "offset": 2.0, - "weight": 1.0, - }, - { - "data": {"area": area1_id, "cluster": cluster_id}, - "id": f"{area1_id}.{cluster_id}", - "offset": None, # updated - "weight": 4.0, # updated - }, - ] - assert constraint_terms == expected - # Update constraint cluster term with invalid id res = client.put( f"/v1/studies/{study_id}/bindingconstraints/{bc_id}/term",