fix(storages): use command when updating matrices (#1971)

Resolves [ANT-1352]

antarest/core/exceptions.py

@@ -34,13 +34,35 @@ class STStorageConfigNotFoundError(HTTPException):
     """Configuration for short-term storage is not found"""
 
     def __init__(self, study_id: str, area_id: str) -> None:
-        detail = f"The short-term storage configuration of area '{area_id}' not found:"
+        detail = f"The short-term storage configuration of area '{area_id}' not found"
         super().__init__(HTTPStatus.NOT_FOUND, detail)
 
     def __str__(self) -> str:
         return self.detail
 
 
+class STStorageNotFoundError(HTTPException):
+    """Short-term storage is not found"""
+
+    def __init__(self, study_id: str, area_id: str, st_storage_id: str) -> None:
+        detail = f"Short-term storage '{st_storage_id}' not found in area '{area_id}'"
+        super().__init__(HTTPStatus.NOT_FOUND, detail)
+
+    def __str__(self) -> str:
+        return self.detail
+
+
+class DuplicateSTStorageId(HTTPException):
+    """Exception raised when trying to create a short-term storage with an already existing id."""
+
+    def __init__(self, study_id: str, area_id: str, st_storage_id: str) -> None:
+        detail = f"Short term storage '{st_storage_id}' already exists in area '{area_id}'"
+        super().__init__(HTTPStatus.CONFLICT, detail)
+
+    def __str__(self) -> str:
+        return self.detail
+
+
 class UnknownModuleError(Exception):
     def __init__(self, message: str) -> None:
         super(UnknownModuleError, self).__init__(message)

antarest/study/business/areas/st_storage_management.py

@@ -8,10 +8,13 @@
 from typing_extensions import Literal
 
 from antarest.core.exceptions import (
+    AreaNotFound,
     ClusterAlreadyExists,
+    DuplicateSTStorageId,
     STStorageConfigNotFoundError,
     STStorageFieldsNotFoundError,
     STStorageMatrixNotFoundError,
+    STStorageNotFoundError,
 )
 from antarest.study.business.utils import AllOptionalMetaclass, camel_case_model, execute_or_add_commands
 from antarest.study.model import Study
@@ -262,6 +265,7 @@ def create_storage(
         """
         file_study = self._get_file_study(study)
         storage = form.to_config(study.version)
+        _check_creation_feasibility(file_study, area_id, storage.id)
         command = self._make_create_cluster_cmd(area_id, storage)
         execute_or_add_commands(
             study,
@@ -357,18 +361,11 @@ def update_storage(
         """
         study_version = study.version
 
-        # review: reading the configuration poses a problem for variants,
-        #  because it requires generating a snapshot, which takes time.
-        #  This reading could be avoided if we don't need the previous values
-        #  (no cross-field validation, no default values, etc.).
-        #  In return, we won't be able to return a complete `STStorageOutput` object.
-        #  So, we need to make sure the frontend doesn't need the missing fields.
-        #  This missing information could also be a problem for the API users.
-        #  The solution would be to avoid reading the configuration if the study is a variant
-        #  (we then use the default values), otherwise, for a RAW study, we read the configuration
-        #  and update the modified values.
+        #  For variants, this method requires generating a snapshot, which takes time.
+        #  But sadly, there's no other way to prevent creating wrong commands.
 
         file_study = self._get_file_study(study)
+        _check_update_feasibility(file_study, area_id, storage_id)
 
         path = STORAGE_LIST_PATH.format(area_id=area_id, storage_id=storage_id)
         try:
@@ -415,14 +412,16 @@ def delete_storages(
             area_id: The area ID of the short-term storage.
             storage_ids: IDs list of short-term storages to remove.
         """
+        file_study = self._get_file_study(study)
+        _check_deletion_feasibility(file_study, area_id, storage_ids)
+
         command_context = self.storage_service.variant_study_service.command_factory.command_context
         for storage_id in storage_ids:
             command = RemoveSTStorage(
                 area_id=area_id,
                 storage_id=storage_id,
                 command_context=command_context,
             )
-            file_study = self._get_file_study(study)
             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: str) -> STStorageOutput:
@@ -455,6 +454,7 @@ def duplicate_cluster(self, study: Study, area_id: str, source_id: str, new_clus
 
         # Matrix edition
         lower_source_id = source_id.lower()
+        # noinspection SpellCheckingInspection
         ts_names = ["pmax_injection", "pmax_withdrawal", "lower_rule_curve", "upper_rule_curve", "inflows"]
         source_paths = [
             STORAGE_SERIES_PATH.format(area_id=area_id, storage_id=lower_source_id, ts_name=ts_name)
@@ -533,22 +533,21 @@ def update_matrix(
             ts_name: Name of the time series to update.
             ts: Matrix of the time series to update.
         """
-        matrix_object = ts.dict()
-        self._save_matrix_obj(study, area_id, storage_id, ts_name, matrix_object)
+        self._save_matrix_obj(study, area_id, storage_id, ts_name, ts.data)
 
     def _save_matrix_obj(
         self,
         study: Study,
         area_id: str,
         storage_id: str,
         ts_name: STStorageTimeSeries,
-        matrix_obj: t.Dict[str, t.Any],
+        matrix_data: t.List[t.List[float]],
     ) -> None:
-        path = STORAGE_SERIES_PATH.format(area_id=area_id, storage_id=storage_id, ts_name=ts_name)
-        matrix = matrix_obj["data"]
+        file_study = self._get_file_study(study)
         command_context = self.storage_service.variant_study_service.command_factory.command_context
-        command = ReplaceMatrix(target=path, matrix=matrix, command_context=command_context)
-        execute_or_add_commands(study, self._get_file_study(study), [command], self.storage_service)
+        path = STORAGE_SERIES_PATH.format(area_id=area_id, storage_id=storage_id, ts_name=ts_name)
+        command = ReplaceMatrix(target=path, matrix=matrix_data, command_context=command_context)
+        execute_or_add_commands(study, file_study, [command], self.storage_service)
 
     def validate_matrices(
         self,
@@ -593,3 +592,31 @@ def validate_matrices(
 
         # Validation successful
         return True
+
+
+def _get_existing_storage_ids(file_study: FileStudy, area_id: str) -> t.Set[str]:
+    try:
+        area = file_study.config.areas[area_id]
+    except KeyError:
+        raise AreaNotFound(area_id) from None
+    else:
+        return {s.id for s in area.st_storages}
+
+
+def _check_deletion_feasibility(file_study: FileStudy, area_id: str, storage_ids: t.Sequence[str]) -> None:
+    existing_ids = _get_existing_storage_ids(file_study, area_id)
+    for storage_id in storage_ids:
+        if storage_id not in existing_ids:
+            raise STStorageNotFoundError(file_study.config.study_id, area_id, storage_id)
+
+
+def _check_update_feasibility(file_study: FileStudy, area_id: str, storage_id: str) -> None:
+    existing_ids = _get_existing_storage_ids(file_study, area_id)
+    if storage_id not in existing_ids:
+        raise STStorageNotFoundError(file_study.config.study_id, area_id, storage_id)
+
+
+def _check_creation_feasibility(file_study: FileStudy, area_id: str, storage_id: str) -> None:
+    existing_ids = _get_existing_storage_ids(file_study, area_id)
+    if storage_id in existing_ids:
+        raise DuplicateSTStorageId(file_study.config.study_id, area_id, storage_id)