From 85a785617b2672c46d2cfc62ef5c370bf84fd10d Mon Sep 17 00:00:00 2001 From: belthlemar Date: Fri, 5 Jan 2024 13:29:39 +0100 Subject: [PATCH] feat(bc): add endpoint for bc deletion and tests --- .../business/binding_constraint_management.py | 25 ++++ .../command/create_binding_constraint.py | 18 ++- .../command/update_binding_constraint.py | 9 +- antarest/study/web/study_data_blueprint.py | 19 ++- .../test_binding_constraints.py | 138 ++++++++++-------- 5 files changed, 134 insertions(+), 75 deletions(-) diff --git a/antarest/study/business/binding_constraint_management.py b/antarest/study/business/binding_constraint_management.py index a5165134fc..36f3351e6f 100644 --- a/antarest/study/business/binding_constraint_management.py +++ b/antarest/study/business/binding_constraint_management.py @@ -3,6 +3,7 @@ from pydantic import BaseModel from antarest.core.exceptions import ( + CommandApplicationError, ConstraintAlreadyExistError, ConstraintIdNotFoundError, DuplicateConstraintName, @@ -35,7 +36,9 @@ BindingConstraintProperties870, CreateBindingConstraint, ) +from antarest.study.storage.variantstudy.model.command.remove_binding_constraint import RemoveBindingConstraint from antarest.study.storage.variantstudy.model.command.update_binding_constraint import UpdateBindingConstraint +from antarest.study.storage.variantstudy.model.dbmodel import VariantStudy class LinkInfoDTO(BaseModel): @@ -227,6 +230,11 @@ def create_binding_constraint( group=data.group, command_context=self.storage_service.variant_study_service.command_factory.command_context, ) + + # Validates the matrices. Needed when the study is a variant because we only append the command to the list + if isinstance(study, VariantStudy): + command.fill_matrices(version, True) + file_study = self.storage_service.get_storage(study).get_raw(study) execute_or_add_commands(study, file_study, [command], self.storage_service) @@ -270,6 +278,23 @@ def update_binding_constraint( ) command = UpdateBindingConstraint(**args) + # Validates the matrices. Needed when the study is a variant because we only append the command to the list + if isinstance(study, VariantStudy): + command.fill_matrices(study_version, False) + + execute_or_add_commands(study, file_study, [command], self.storage_service) + + def remove_binding_constraint(self, study: Study, binding_constraint_id: str) -> None: + command = RemoveBindingConstraint( + id=binding_constraint_id, + command_context=self.storage_service.variant_study_service.command_factory.command_context, + ) + file_study = self.storage_service.get_storage(study).get_raw(study) + + # Needed when the study is a variant because we only append the command to the list + if isinstance(study, VariantStudy) and not self.get_binding_constraint(study, binding_constraint_id): + raise CommandApplicationError("Binding constraint not found") + execute_or_add_commands(study, file_study, [command], self.storage_service) @staticmethod diff --git a/antarest/study/storage/variantstudy/model/command/create_binding_constraint.py b/antarest/study/storage/variantstudy/model/command/create_binding_constraint.py index 748fa66225..df6800fffb 100644 --- a/antarest/study/storage/variantstudy/model/command/create_binding_constraint.py +++ b/antarest/study/storage/variantstudy/model/command/create_binding_constraint.py @@ -178,6 +178,16 @@ def get_corresponding_matrices(self, v: Optional[Union[MatrixType, str]], old: b # pragma: no cover raise TypeError(repr(v)) + def fill_matrices(self, version: int, create: bool) -> None: + if version < 870: + self.values = self.get_corresponding_matrices(self.values, old=True, create=create) + else: + self.less_term_matrix = self.get_corresponding_matrices(self.less_term_matrix, old=False, create=create) + self.greater_term_matrix = self.get_corresponding_matrices( + self.greater_term_matrix, old=False, create=create + ) + self.equal_term_matrix = self.get_corresponding_matrices(self.equal_term_matrix, old=False, create=create) + class CreateBindingConstraint(AbstractBindingConstraintCommand): """ @@ -199,13 +209,7 @@ def _apply(self, study_data: FileStudy) -> CommandOutput: binding_constraints = study_data.tree.get(["input", "bindingconstraints", "bindingconstraints"]) new_key = len(binding_constraints) bd_id = transform_name_to_id(self.name) - - if study_data.config.version < 870: - self.values = self.get_corresponding_matrices(self.values, old=True, create=True) - else: - self.less_term_matrix = self.get_corresponding_matrices(self.less_term_matrix, old=False, create=True) - self.greater_term_matrix = self.get_corresponding_matrices(self.greater_term_matrix, old=False, create=True) - self.equal_term_matrix = self.get_corresponding_matrices(self.equal_term_matrix, old=False, create=True) + self.fill_matrices(study_data.config.version, True) return apply_binding_constraint( study_data, diff --git a/antarest/study/storage/variantstudy/model/command/update_binding_constraint.py b/antarest/study/storage/variantstudy/model/command/update_binding_constraint.py index 8aa1087809..0028b1807b 100644 --- a/antarest/study/storage/variantstudy/model/command/update_binding_constraint.py +++ b/antarest/study/storage/variantstudy/model/command/update_binding_constraint.py @@ -53,14 +53,7 @@ def _apply(self, study_data: FileStudy) -> CommandOutput: message="Failed to retrieve existing binding constraint", ) - if study_data.config.version < 870: - self.values = self.get_corresponding_matrices(self.values, old=True, create=False) - else: - self.less_term_matrix = self.get_corresponding_matrices(self.less_term_matrix, old=False, create=False) - self.greater_term_matrix = self.get_corresponding_matrices( - self.greater_term_matrix, old=False, create=False - ) - self.equal_term_matrix = self.get_corresponding_matrices(self.equal_term_matrix, old=False, create=False) + self.fill_matrices(study_data.config.version, False) return apply_binding_constraint( study_data, diff --git a/antarest/study/web/study_data_blueprint.py b/antarest/study/web/study_data_blueprint.py index 967ae41fc3..cbc0ceb03b 100644 --- a/antarest/study/web/study_data_blueprint.py +++ b/antarest/study/web/study_data_blueprint.py @@ -878,6 +878,23 @@ def create_binding_constraint( study = study_service.check_study_access(uuid, StudyPermissionType.READ, params) return study_service.binding_constraint_manager.create_binding_constraint(study, data) + @bp.delete( + "/studies/{uuid}/bindingconstraints/{binding_constraint_id}", + tags=[APITag.study_data], + summary="Delete a binding constraint", + response_model=None, + ) + def delete_binding_constraint( + uuid: str, binding_constraint_id: str, current_user: JWTUser = Depends(auth.get_current_user) + ) -> None: + logger.info( + f"Deleting the binding constraint {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.remove_binding_constraint(study, binding_constraint_id) + @bp.post( "/studies/{uuid}/bindingconstraints/{binding_constraint_id}/term", tags=[APITag.study_data], @@ -919,7 +936,7 @@ def update_constraint_term( @bp.delete( "/studies/{uuid}/bindingconstraints/{binding_constraint_id}/term/{term_id}", tags=[APITag.study_data], - summary="update constraint term", + summary="Delete a constraint term", ) def remove_constraint_term( uuid: str, diff --git a/tests/integration/study_data_blueprint/test_binding_constraints.py b/tests/integration/study_data_blueprint/test_binding_constraints.py index 43fa0b5b71..e6fafbf716 100644 --- a/tests/integration/study_data_blueprint/test_binding_constraints.py +++ b/tests/integration/study_data_blueprint/test_binding_constraints.py @@ -11,11 +11,12 @@ class TestBindingConstraint: Test the end points related to binding constraints. """ - def test_lifecycle__nominal(self, client: TestClient, user_access_token: str) -> None: + @pytest.mark.parametrize("study_type", ["raw", "variant"]) + def test_lifecycle__nominal(self, client: TestClient, user_access_token: str, study_type: str) -> None: user_headers = {"Authorization": f"Bearer {user_access_token}"} # ============================= - # PREPARE STUDY + # STUDY PREPARATION # ============================= res = client.post( @@ -60,22 +61,23 @@ def test_lifecycle__nominal(self, client: TestClient, user_access_token: str) -> ) assert res.status_code == 200, res.json() - # Create Variant - res = client.post( - f"/v1/studies/{study_id}/variants", - headers=user_headers, - params={"name": "Variant 1"}, - ) - assert res.status_code == 200, res.json() - variant_id = res.json() + if study_type == "variant": + # Create Variant + res = client.post( + f"/v1/studies/{study_id}/variants", + headers=user_headers, + params={"name": "Variant 1"}, + ) + assert res.status_code == 200, res.json() + study_id = res.json() # ============================= - # BINDING CONSTRAINT CREATION + # CREATION # ============================= # Create Binding constraints res = client.post( - f"/v1/studies/{variant_id}/commands", + f"/v1/studies/{study_id}/commands", json=[ { "action": "create_binding_constraint", @@ -94,7 +96,7 @@ def test_lifecycle__nominal(self, client: TestClient, user_access_token: str) -> assert res.status_code == 200, res.json() res = client.post( - f"/v1/studies/{variant_id}/commands", + f"/v1/studies/{study_id}/commands", json=[ { "action": "create_binding_constraint", @@ -114,7 +116,7 @@ def test_lifecycle__nominal(self, client: TestClient, user_access_token: str) -> # Creates a binding constraint with the new API res = client.post( - f"/v1/studies/{variant_id}/bindingconstraints", + f"/v1/studies/{study_id}/bindingconstraints", json={ "name": "binding_constraint_3", "enabled": True, @@ -128,7 +130,7 @@ def test_lifecycle__nominal(self, client: TestClient, user_access_token: str) -> assert res.status_code == 200, res.json() # Get Binding Constraint list - res = client.get(f"/v1/studies/{variant_id}/bindingconstraints", headers=user_headers) + 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) == 3 @@ -140,7 +142,7 @@ def test_lifecycle__nominal(self, client: TestClient, user_access_token: str) -> assert "group" not in binding_constraints_list[0] # ============================= - # BINDING CONSTRAINT EDITION + # GENERAL EDITION # ============================= bc_id = binding_constraints_list[0]["id"] @@ -148,7 +150,7 @@ def test_lifecycle__nominal(self, client: TestClient, user_access_token: str) -> # Update element of Binding constraint new_comment = "We made it !" res = client.put( - f"v1/studies/{variant_id}/bindingconstraints/{bc_id}", + f"v1/studies/{study_id}/bindingconstraints/{bc_id}", json={"key": "comments", "value": new_comment}, headers=user_headers, ) @@ -156,7 +158,7 @@ def test_lifecycle__nominal(self, client: TestClient, user_access_token: str) -> # Get Binding Constraint res = client.get( - f"/v1/studies/{variant_id}/bindingconstraints/{bc_id}", + f"/v1/studies/{study_id}/bindingconstraints/{bc_id}", headers=user_headers, ) binding_constraint = res.json() @@ -170,7 +172,7 @@ def test_lifecycle__nominal(self, client: TestClient, user_access_token: str) -> area2_name = "area2" res = client.post( - f"/v1/studies/{variant_id}/bindingconstraints/{bc_id}/term", + f"/v1/studies/{study_id}/bindingconstraints/{bc_id}/term", json={ "weight": 1, "offset": 2, @@ -182,7 +184,7 @@ def test_lifecycle__nominal(self, client: TestClient, user_access_token: str) -> # Get Binding Constraint res = client.get( - f"/v1/studies/{variant_id}/bindingconstraints/{bc_id}", + f"/v1/studies/{study_id}/bindingconstraints/{bc_id}", headers=user_headers, ) binding_constraint = res.json() @@ -198,7 +200,7 @@ def test_lifecycle__nominal(self, client: TestClient, user_access_token: str) -> # Update Constraint term res = client.put( - f"/v1/studies/{variant_id}/bindingconstraints/{bc_id}/term", + f"/v1/studies/{study_id}/bindingconstraints/{bc_id}/term", json={ "id": f"{area1_name}%{area2_name}", "weight": 3, @@ -209,7 +211,7 @@ def test_lifecycle__nominal(self, client: TestClient, user_access_token: str) -> # Get Binding Constraint res = client.get( - f"/v1/studies/{variant_id}/bindingconstraints/{bc_id}", + f"/v1/studies/{study_id}/bindingconstraints/{bc_id}", headers=user_headers, ) binding_constraint = res.json() @@ -225,14 +227,14 @@ def test_lifecycle__nominal(self, client: TestClient, user_access_token: str) -> # Remove Constraint term res = client.delete( - f"/v1/studies/{variant_id}/bindingconstraints/{bc_id}/term/{area1_name}%{area2_name}", + f"/v1/studies/{study_id}/bindingconstraints/{bc_id}/term/{area1_name}%{area2_name}", headers=user_headers, ) assert res.status_code == 200, res.json() # Get Binding Constraint res = client.get( - f"/v1/studies/{variant_id}/bindingconstraints/{bc_id}", + f"/v1/studies/{study_id}/bindingconstraints/{bc_id}", headers=user_headers, ) binding_constraint = res.json() @@ -243,22 +245,23 @@ def test_lifecycle__nominal(self, client: TestClient, user_access_token: str) -> # The user change the time_step to daily instead of hourly. # We must check that the matrix is a daily/weekly matrix. res = client.put( - f"/v1/studies/{variant_id}/bindingconstraints/{bc_id}", + f"/v1/studies/{study_id}/bindingconstraints/{bc_id}", json={"key": "time_step", "value": "daily"}, headers=user_headers, ) assert res.status_code == 200, res.json() - # Check the last command is a change time_step - res = client.get(f"/v1/studies/{variant_id}/commands", headers=user_headers) - commands = res.json() - args = commands[-1]["args"] - assert args["time_step"] == "daily" - assert args["values"] is not None, "We should have a matrix ID (sha256)" + if study_type == "variant": + # Check the last command is a change time_step + res = client.get(f"/v1/studies/{study_id}/commands", headers=user_headers) + commands = res.json() + args = commands[-1]["args"] + assert args["time_step"] == "daily" + assert args["values"] is not None, "We should have a matrix ID (sha256)" # Check that the matrix is a daily/weekly matrix res = client.get( - f"/v1/studies/{variant_id}/raw", + f"/v1/studies/{study_id}/raw", params={"path": f"input/bindingconstraints/{bc_id}", "depth": 1, "formatted": True}, headers=user_headers, ) @@ -267,6 +270,14 @@ def test_lifecycle__nominal(self, client: TestClient, user_access_token: str) -> assert len(dataframe["index"]) == 366 assert len(dataframe["columns"]) == 3 # less, equal, greater + # Delete a binding constraint + res = client.delete(f"/v1/studies/{study_id}/bindingconstraints/{bc_id}", headers=user_headers) + assert res.status_code == 200, res.json() + + # Asserts that the deletion worked + res = client.get(f"/v1/studies/{study_id}/bindingconstraints", headers=user_headers) + assert len(res.json()) == 2 + # ============================= # MATRIX EDITION # ============================= @@ -290,7 +301,7 @@ def test_lifecycle__nominal(self, client: TestClient, user_access_token: str) -> # Check that the matrix is a daily/weekly matrix res = client.get( - f"/v1/studies/{variant_id}/raw", + f"/v1/studies/{study_id}/raw", params={"path": f"input/bindingconstraints/{bc_id_with_matrix}", "depth": 1, "formatted": True}, headers=user_headers, ) @@ -299,35 +310,13 @@ def test_lifecycle__nominal(self, client: TestClient, user_access_token: str) -> assert len(dataframe["index"]) == 366 assert len(dataframe["columns"]) == 3 # less, equal, greater - wrong_matrix = np.ones((352, 3)) - bc_id_with_wrong_matrix = "binding_constraint_5" - res = client.post( - f"/v1/studies/{study_id}/bindingconstraints", - json={ - "name": f"{bc_id_with_wrong_matrix}", - "enabled": True, - "time_step": "daily", - "operator": "less", - "coeffs": {}, - "comments": "Creation with matrix", - "values": wrong_matrix.tolist(), - }, - headers=user_headers, - ) - assert res.status_code == 500 - assert res.json()["exception"] == "CommandApplicationError" - assert ( - res.json()["description"] - == f"Unexpected exception occurred when trying to apply command CommandName.CREATE_BINDING_CONSTRAINT: Invalid matrix shape {wrong_matrix.shape}, expected (366, 3)" - ) - # ============================= - # BINDING CONSTRAINT ERRORS + # ERRORS # ============================= # Asserts that creating 2 binding constraints with the same name raises an Exception res = client.post( - f"/v1/studies/{variant_id}/bindingconstraints", + f"/v1/studies/{study_id}/bindingconstraints", json={ "name": "binding_constraint_3", "enabled": True, @@ -346,7 +335,7 @@ def test_lifecycle__nominal(self, client: TestClient, user_access_token: str) -> # Assert empty name res = client.post( - f"/v1/studies/{variant_id}/bindingconstraints", + f"/v1/studies/{study_id}/bindingconstraints", json={ "name": " ", "enabled": True, @@ -365,7 +354,7 @@ def test_lifecycle__nominal(self, client: TestClient, user_access_token: str) -> # Assert invalid special characters res = client.post( - f"/v1/studies/{variant_id}/bindingconstraints", + f"/v1/studies/{study_id}/bindingconstraints", json={ "name": "%%**", "enabled": True, @@ -420,6 +409,37 @@ def test_lifecycle__nominal(self, client: TestClient, user_access_token: str) -> assert res.status_code == 400 assert res.json()["description"] == "You cannot fill a 'matrix_term' as these values refer to v8.7+ studies" + # Wrong matrix shape + wrong_matrix = np.ones((352, 3)) + wrong_request_args = { + "name": "binding_constraint_5", + "enabled": True, + "time_step": "daily", + "operator": "less", + "coeffs": {}, + "comments": "Creation with matrix", + "values": wrong_matrix.tolist(), + } + res = client.post( + f"/v1/studies/{study_id}/bindingconstraints", + json=wrong_request_args, + headers=user_headers, + ) + assert res.status_code == 500 + exception = res.json()["exception"] + description = res.json()["description"] + assert exception == "ValueError" if study_type == "variant" else "CommandApplicationError" + assert f"Invalid matrix shape {wrong_matrix.shape}, expected (366, 3)" in description + + # Delete a fake binding constraint + res = client.delete(f"/v1/studies/{study_id}/bindingconstraints/fake_bc", headers=user_headers) + assert res.status_code == 500 + assert res.json()["exception"] == "CommandApplicationError" + assert res.json()["description"] == "Binding constraint not found" + + +# todo : add lots of test for v8.7 + def test_87(self, client: TestClient, admin_access_token: str, study_id: str) -> None: admin_headers = {"Authorization": f"Bearer {admin_access_token}"}