diff --git a/alembic/versions/bae9c99bc42d_add_user_name_and_updated_at_to_commands.py b/alembic/versions/bae9c99bc42d_add_user_name_and_updated_at_to_commands.py new file mode 100644 index 0000000000..cd91acb203 --- /dev/null +++ b/alembic/versions/bae9c99bc42d_add_user_name_and_updated_at_to_commands.py @@ -0,0 +1,36 @@ +"""add_user_name_and_updated_at_to_commands + +Revision ID: bae9c99bc42d +Revises: 00a9ceb38842 +Create Date: 2024-11-29 09:13:04.292874 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = 'bae9c99bc42d' +down_revision = '00a9ceb38842' +branch_labels = None +depends_on = None + +USER_ID_FKEY = 'commandblock_user_id_fk' + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + with op.batch_alter_table('commandblock', schema=None) as batch_op: + batch_op.add_column(sa.Column('user_id', sa.Integer(), nullable=True)) + batch_op.add_column(sa.Column('updated_at', sa.DateTime(), nullable=True)) + batch_op.create_foreign_key(USER_ID_FKEY, 'identities', ['user_id'], ['id'], ondelete='SET NULL') + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + with op.batch_alter_table('commandblock', schema=None) as batch_op: + batch_op.drop_constraint(USER_ID_FKEY, type_='foreignkey') + batch_op.drop_column('updated_at') + batch_op.drop_column('user_id') + # ### end Alembic commands ### diff --git a/antarest/study/storage/variantstudy/model/dbmodel.py b/antarest/study/storage/variantstudy/model/dbmodel.py index f09231cc86..180b7105ef 100644 --- a/antarest/study/storage/variantstudy/model/dbmodel.py +++ b/antarest/study/storage/variantstudy/model/dbmodel.py @@ -66,6 +66,8 @@ class CommandBlock(Base): # type: ignore version: int = Column(Integer) args: str = Column(String()) study_version: str = Column(String(36)) + user_id: int = Column(Integer, ForeignKey("identities.id", ondelete="SET NULL"), nullable=True) + updated_at: datetime.datetime = Column(DateTime, nullable=True) def to_dto(self) -> CommandDTO: # Database may lack a version number, defaulting to 1 if so. @@ -76,6 +78,8 @@ def to_dto(self) -> CommandDTO: args=from_json(self.args), version=version, study_version=self.study_version, + user_id=self.user_id, + updated_at=self.updated_at, ) def __str__(self) -> str: @@ -87,6 +91,8 @@ def __str__(self) -> str: f" version={self.version!r}," f" args={self.args!r})" f" study_version={self.study_version!r}" + f" user_id={self.user_id!r}" + f" updated_at={self.updated_at!r}" ) diff --git a/antarest/study/storage/variantstudy/model/model.py b/antarest/study/storage/variantstudy/model/model.py index a303e39020..0d0c308581 100644 --- a/antarest/study/storage/variantstudy/model/model.py +++ b/antarest/study/storage/variantstudy/model/model.py @@ -9,7 +9,7 @@ # SPDX-License-Identifier: MPL-2.0 # # This file is part of the Antares project. - +import datetime import typing as t import uuid @@ -73,6 +73,8 @@ class CommandDTOAPI(AntaresBaseModel): action: str args: t.Union[t.MutableSequence[JSON], JSON] version: int = 1 + user_name: t.Optional[str] = None + updated_at: t.Optional[datetime.datetime] = None class CommandDTO(AntaresBaseModel): @@ -85,6 +87,8 @@ class CommandDTO(AntaresBaseModel): args: The arguments for the command action. version: The version of the command. study_version: The version of the study associated to the command. + user_id: id of the author of the command. + updated_at: The time the command was last updated. """ id: t.Optional[str] = None @@ -92,9 +96,13 @@ class CommandDTO(AntaresBaseModel): args: t.Union[t.MutableSequence[JSON], JSON] version: int = 1 study_version: StudyVersionStr + user_id: t.Optional[int] = None + updated_at: t.Optional[datetime.datetime] = None - def to_api(self) -> CommandDTOAPI: - return CommandDTOAPI.model_validate(self.model_dump(mode="json", exclude={"study_version"})) + def to_api(self, user_name: t.Optional[str] = None) -> CommandDTOAPI: + data = self.model_dump(mode="json", exclude={"study_version", "user_id"}) + data["user_name"] = user_name + return CommandDTOAPI.model_validate(data) class CommandResultDTO(AntaresBaseModel): diff --git a/antarest/study/storage/variantstudy/variant_study_service.py b/antarest/study/storage/variantstudy/variant_study_service.py index ea57a82d60..a57b7f1b2f 100644 --- a/antarest/study/storage/variantstudy/variant_study_service.py +++ b/antarest/study/storage/variantstudy/variant_study_service.py @@ -41,7 +41,7 @@ from antarest.core.filetransfer.model import FileDownloadTaskDTO from antarest.core.interfaces.cache import ICache from antarest.core.interfaces.eventbus import Event, EventChannelDirectory, EventType, IEventBus -from antarest.core.jwt import DEFAULT_ADMIN_USER +from antarest.core.jwt import DEFAULT_ADMIN_USER, JWTUser from antarest.core.model import JSON, PermissionInfo, PublicMode, StudyPermissionType from antarest.core.requests import RequestParameters, UserHasNotPermissionError from antarest.core.serialization import to_json_string @@ -49,6 +49,7 @@ from antarest.core.tasks.service import DEFAULT_AWAIT_MAX_TIMEOUT, ITaskNotifier, ITaskService, NoopNotifier from antarest.core.utils.fastapi_sqlalchemy import db from antarest.core.utils.utils import assert_this, suppress_exception +from antarest.login.model import Identity from antarest.matrixstore.service import MatrixService from antarest.study.model import RawStudy, Study, StudyAdditionalData, StudyMetadataDTO, StudySimResultDTO from antarest.study.repository import AccessPermissions, StudyFilter @@ -106,6 +107,17 @@ def __init__( self.command_factory = command_factory self.generator = VariantCommandGenerator(self.study_factory) + @staticmethod + def _get_user_name_from_id(user_id: int) -> str: + """ + Utility method that retrieves a user's name based on their id. + Args: + user_id: user id (user must exist) + Returns: String representing the user's name + """ + user_obj: Identity = db.session.query(Identity).get(user_id) + return user_obj.name # type: ignore # `name` attribute is always a string + def get_command(self, study_id: str, command_id: str, params: RequestParameters) -> CommandDTOAPI: """ Get command lists @@ -118,8 +130,10 @@ def get_command(self, study_id: str, command_id: str, params: RequestParameters) study = self._get_variant_study(study_id, params) try: - index = [command.id for command in study.commands].index(command_id) # Maybe add Try catch for this - return t.cast(CommandDTOAPI, study.commands[index].to_dto().to_api()) + index = [command.id for command in study.commands].index(command_id) + command: CommandBlock = study.commands[index] + user_name = self._get_user_name_from_id(command.user_id) if command.user_id else None + return command.to_dto().to_api(user_name) except ValueError: raise CommandNotFoundError(f"Command with id {command_id} not found") from None @@ -132,7 +146,16 @@ def get_commands(self, study_id: str, params: RequestParameters) -> t.List[Comma Returns: List of commands """ study = self._get_variant_study(study_id, params) - return [command.to_dto().to_api() for command in study.commands] + + id_to_name: t.Dict[int, str] = {} + command_list = [] + + for command in study.commands: + if command.user_id and command.user_id not in id_to_name.keys(): + user_name: str = self._get_user_name_from_id(command.user_id) + id_to_name[command.user_id] = user_name + command_list.append(command.to_dto().to_api(id_to_name.get(command.user_id))) + return command_list def convert_commands( self, study_id: str, api_commands: t.List[CommandDTOAPI], params: RequestParameters @@ -201,6 +224,7 @@ def append_commands( command_objs = self._check_commands_validity(study_id, commands) validated_commands = transform_command_to_dto(command_objs, commands) first_index = len(study.commands) + # noinspection PyArgumentList new_commands = [ CommandBlock( @@ -209,6 +233,9 @@ def append_commands( index=(first_index + i), version=command.version, study_version=str(command.study_version), + # params.user cannot be None, since previous checks were successful + user_id=params.user.id, # type: ignore + updated_at=datetime.utcnow(), ) for i, command in enumerate(validated_commands) ] @@ -249,6 +276,8 @@ def replace_commands( index=i, version=command.version, study_version=str(command.study_version), + user_id=params.user.id, # type: ignore + updated_at=datetime.utcnow(), ) for i, command in enumerate(validated_commands) ] @@ -893,6 +922,8 @@ def copy( index=command.index, version=command.version, study_version=str(command.study_version), + user_id=command.user_id, + updated_at=command.updated_at, ) for command in src_meta.commands ] diff --git a/tests/integration/study_data_blueprint/test_hydro_inflow_structure.py b/tests/integration/study_data_blueprint/test_hydro_inflow_structure.py index ec1327093d..79965cc81e 100644 --- a/tests/integration/study_data_blueprint/test_hydro_inflow_structure.py +++ b/tests/integration/study_data_blueprint/test_hydro_inflow_structure.py @@ -9,7 +9,6 @@ # SPDX-License-Identifier: MPL-2.0 # # This file is part of the Antares project. - from http import HTTPStatus from unittest.mock import ANY @@ -137,6 +136,8 @@ def test_get_inflow_structure( "data": {"intermonthly-correlation": 0.9}, }, "version": 1, + "updated_at": ANY, + "user_name": ANY, } assert actual[1] == expected diff --git a/tests/integration/study_data_blueprint/test_st_storage.py b/tests/integration/study_data_blueprint/test_st_storage.py index 2780e098cf..ae5a25dacd 100644 --- a/tests/integration/study_data_blueprint/test_st_storage.py +++ b/tests/integration/study_data_blueprint/test_st_storage.py @@ -669,6 +669,8 @@ def test__default_values( "inflows": ANY, }, "version": 1, + "updated_at": ANY, + "user_name": ANY, } assert actual == expected @@ -693,6 +695,8 @@ def test__default_values( "target": "input/st-storage/clusters/fr/list/siemens battery/initiallevel", }, "version": 1, + "updated_at": ANY, + "user_name": ANY, } assert actual == expected @@ -723,6 +727,8 @@ def test__default_values( }, ], "version": 1, + "updated_at": ANY, + "user_name": ANY, } assert actual == expected diff --git a/tests/integration/test_integration_token_end_to_end.py b/tests/integration/test_integration_token_end_to_end.py index 01b92788d4..5b78d0ac58 100644 --- a/tests/integration/test_integration_token_end_to_end.py +++ b/tests/integration/test_integration_token_end_to_end.py @@ -10,6 +10,7 @@ # # This file is part of the Antares project. +import datetime import io import typing as t from unittest.mock import ANY @@ -173,6 +174,21 @@ def test_nominal_case_of_an_api_user(client: TestClient, admin_access_token: str res = client.post(f"/v1/studies/{variant_id}/commands", headers=bot_headers, json=commands) assert res.status_code == 200 + # Check if the author's name and date of update are retrieved with commands created by a bot + commands_res = client.get(f"/v1/studies/{variant_id}/commands", headers=bot_headers) + + for command in commands_res.json(): + # FIXME: Some commands, such as those that modify study configurations, are run by admin user + # Thus the `user_name` for such type of command will be the admin's name + # Here we detect those commands by their `action` and their `target` values + if command["action"] == "update_playlist" or ( + command["action"] == "update_config" and "settings/generaldata" in command["args"]["target"] + ): + assert command["user_name"] == "admin" + else: + assert command["user_name"] == "admin_bot" + assert command["updated_at"] + # generate variant before running a simulation res = client.put(f"/v1/studies/{variant_id}/generate", headers=bot_headers) assert res.status_code == 200 diff --git a/tests/study/storage/variantstudy/model/test_dbmodel.py b/tests/study/storage/variantstudy/model/test_dbmodel.py index e89ea48355..529c9f04b3 100644 --- a/tests/study/storage/variantstudy/model/test_dbmodel.py +++ b/tests/study/storage/variantstudy/model/test_dbmodel.py @@ -127,7 +127,7 @@ def test_init__with_command(self, db_session: Session, variant_study_id: str) -> class TestCommandBlock: - def test_init(self, db_session: Session, variant_study_id: str) -> None: + def test_init(self, db_session: Session, variant_study_id: str, user_id: int) -> None: """ Check the creation of an instance of CommandBlock """ @@ -136,6 +136,7 @@ def test_init(self, db_session: Session, variant_study_id: str) -> None: command = "dummy command" version = 42 args = '{"foo": "bar"}' + updated_at = datetime.datetime.utcnow() with db_session: block = CommandBlock( @@ -146,6 +147,8 @@ def test_init(self, db_session: Session, variant_study_id: str) -> None: version=version, args=args, study_version="860", + updated_at=updated_at, + user_id=user_id, ) db_session.add(block) db_session.commit() @@ -172,6 +175,8 @@ def test_init(self, db_session: Session, variant_study_id: str) -> None: "args": json.loads(args), "version": 42, "study_version": StudyVersion.parse("860"), + "updated_at": updated_at, + "user_id": user_id, } diff --git a/tests/study/storage/variantstudy/test_variant_study_service.py b/tests/study/storage/variantstudy/test_variant_study_service.py index ba0cb93277..6375dcc546 100644 --- a/tests/study/storage/variantstudy/test_variant_study_service.py +++ b/tests/study/storage/variantstudy/test_variant_study_service.py @@ -134,7 +134,7 @@ def test_generate_task( ) -> None: ## Prepare database objects # noinspection PyArgumentList - user = User(id=0, name="admin") + user = User(id=1, name="admin") db.session.add(user) db.session.commit() diff --git a/tests/variantstudy/model/command/test_create_cluster.py b/tests/variantstudy/model/command/test_create_cluster.py index b6a03874f5..adf9f221f6 100644 --- a/tests/variantstudy/model/command/test_create_cluster.py +++ b/tests/variantstudy/model/command/test_create_cluster.py @@ -210,6 +210,8 @@ def test_to_dto(self, command_context: CommandContext): "id": None, "version": 1, "study_version": STUDY_VERSION_8_8, + "user_id": None, + "updated_at": None, } diff --git a/tests/variantstudy/model/command/test_create_renewables_cluster.py b/tests/variantstudy/model/command/test_create_renewables_cluster.py index aa4eeff60e..78b73e0208 100644 --- a/tests/variantstudy/model/command/test_create_renewables_cluster.py +++ b/tests/variantstudy/model/command/test_create_renewables_cluster.py @@ -163,6 +163,8 @@ def test_to_dto(self, command_context: CommandContext) -> None: "id": None, "version": 1, "study_version": STUDY_VERSION_8_8, + "updated_at": None, + "user_id": None, } diff --git a/tests/variantstudy/model/test_variant_model.py b/tests/variantstudy/model/test_variant_model.py index 2fd8a47881..e2b607a30e 100644 --- a/tests/variantstudy/model/test_variant_model.py +++ b/tests/variantstudy/model/test_variant_model.py @@ -11,13 +11,17 @@ # This file is part of the Antares project. import datetime +import typing as t import uuid from pathlib import Path +from unittest.mock import patch import pytest from antares.study.version import StudyVersion +from sqlalchemy import event from antarest.core.jwt import JWTGroup, JWTUser +from antarest.core.model import PublicMode from antarest.core.requests import RequestParameters from antarest.core.roles import RoleType from antarest.core.utils.fastapi_sqlalchemy import db @@ -59,7 +63,11 @@ def root_study_id_fixture( raw_study_service: RawStudyService, variant_study_service: VariantStudyService, jwt_user: JWTUser, + request: t.Any, ) -> str: + # Get public mode argument + public_mode = request.param + # Prepare a RAW study in the temporary folder study_dir = tmp_path / "my-study" root_study_id = str(uuid.uuid4()) @@ -72,6 +80,7 @@ def root_study_id_fixture( updated_at=datetime.datetime.utcnow(), additional_data=StudyAdditionalData(author="john.doe"), owner_id=jwt_user.id, + public_mode=PublicMode.EDIT if public_mode else PublicMode.NONE, ) root_study = raw_study_service.create(root_study) with db(): @@ -79,6 +88,7 @@ def root_study_id_fixture( variant_study_service.repository.save(root_study) return root_study_id + @pytest.mark.parametrize("root_study_id", [False], indirect=True) @with_db_context def test_commands_service( self, @@ -89,10 +99,9 @@ def test_commands_service( ) -> None: # Initialize the default matrix constants generator_matrix_constants.init_constant_matrices() - params = RequestParameters(user=jwt_user) - # Create un new variant + # Create a new variant variant_study = variant_study_service.create_variant_study(root_study_id, "my-variant", params=params) study_version = StudyVersion.parse(variant_study.version) saved_id = variant_study.id @@ -101,7 +110,7 @@ def test_commands_service( assert study.id == saved_id assert study.parent_id == root_study_id - # Append command + # Append commands one at the time command_count = 0 command_1 = CommandDTO(action="create_area", args={"area_name": "Yes"}, study_version=study_version) variant_study_service.append_command(saved_id, command_1, params=params) @@ -190,3 +199,119 @@ def test_commands_service( ], } assert study.snapshot.id == study.id + + @pytest.mark.parametrize("root_study_id", [True], indirect=True) + @with_db_context + def test_command_several_authors( + self, + jwt_user: JWTUser, + variant_study_service: VariantStudyService, + root_study_id: str, + ): + """ + Test two different users that are authors on two different commands of the same variant + Set up: + Retrieve the user that will be the owner of the study and variant + Create a second user + Create a study and a variant study + Each user creates a command + + Tests: + Test whether the commands have the `user_name` and `updated_at` attributes + Test authors of the commands + """ + # Get the owner request parameters + owner_params = RequestParameters(user=jwt_user) + + # create another user that has the write privilege + user2 = User(id=3, name="jane.doe", type="users") + db.session.add(user2) + db.session.commit() + + user2_params = RequestParameters( + user=JWTUser( + id=user2.id, + impersonator=user2.id, + type="users", + groups=[JWTGroup(id="writers", name="writers", role=RoleType.WRITER)], + ) + ) + + # Generate a variant on a study that allow other user to edit it + variant_study = variant_study_service.create_variant_study(root_study_id, "new variant", params=owner_params) + study_version = StudyVersion.parse(variant_study.version) + variant_id = variant_study.id + + # Create two new commands on the existing variant + command_6 = CommandDTO(action="update_comments", args={"comments": "new comment"}, study_version=study_version) + command_7 = CommandDTO( + action="update_comments", args={"comments": "another new comment"}, study_version=study_version + ) + + variant_study_service.append_command(variant_id, command_6, params=owner_params) + variant_study_service.append_command(variant_id, command_7, params=user2_params) + + # Make sure there are commands generated by both users + commands = variant_study_service.get_commands(variant_id, params=owner_params) + assert len(commands) == 2 + + # Make sure their `user_name` and `updated_at` attributes are not None + for command in commands: + assert command.user_name and command.updated_at + + # Make sure commands has not the same author + assert commands[0] != commands[1] + assert commands[0].user_name == "john.doe" + assert commands[1].user_name == "jane.doe" + + @pytest.mark.parametrize("root_study_id", [False], indirect=True) + @with_db_context + def test_command_same_author( + self, + jwt_user: JWTUser, + variant_study_service: VariantStudyService, + root_study_id: str, + ): + """ + Test the case of multiple commands was created by the same user. + Set up: + Initialize a counter of queries to database + Define a watcher on the orm queries to database that updates the counter + Create a user + Create a variant study + Make the user generates five commands on the newly created variant + Test: + Each time a command is retrieved, the database must be accessed only if + the author of the currently retrieved command is not already known during + the process + """ + nb_queries = 0 # Store number of orm queries to database + + # Watch orm events and update `nb_queries` + @event.listens_for(db.session, "do_orm_execute") + def check_orm_operations(orm_execute_state): + if orm_execute_state.is_select: + nonlocal nb_queries + nb_queries += 1 + + owner_params = RequestParameters(user=jwt_user) + + # Generate a variant on a study that allow other user to edit it + variant_study = variant_study_service.create_variant_study(root_study_id, "new_variant", params=owner_params) + + commands = [] + + # Create two new commands on the existing variant + for index in range(5): + commands.append( + CommandDTO( + action="update_comments", + args={"comments": f"new comment {index}"}, + study_version=StudyVersion.parse(variant_study.version), + ) + ) + variant_study_service.append_commands(variant_study.id, commands, params=owner_params) + + nb_queries_before = nb_queries # store initial state + variant_study_service.get_commands(variant_study.id, params=owner_params) # execute database query + assert nb_queries_before + 1 == nb_queries # compare with initial state to make sure database was queried once