From 40d065ae65f9428f85c5b2738fc585a80885da9c Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Mon, 19 Jun 2023 16:56:19 +0200 Subject: [PATCH 1/4] removed projects-to-project_nodes --- ...837b0_remove_projects_to_projects_nodes.py | 137 ++++++++++++++++++ .../models/projects_nodes.py | 54 +++---- .../models/projects_to_projects_nodes.py | 51 ------- 3 files changed, 154 insertions(+), 88 deletions(-) create mode 100644 packages/postgres-database/src/simcore_postgres_database/migration/versions/71ea254837b0_remove_projects_to_projects_nodes.py delete mode 100644 packages/postgres-database/src/simcore_postgres_database/models/projects_to_projects_nodes.py diff --git a/packages/postgres-database/src/simcore_postgres_database/migration/versions/71ea254837b0_remove_projects_to_projects_nodes.py b/packages/postgres-database/src/simcore_postgres_database/migration/versions/71ea254837b0_remove_projects_to_projects_nodes.py new file mode 100644 index 00000000000..52311462c92 --- /dev/null +++ b/packages/postgres-database/src/simcore_postgres_database/migration/versions/71ea254837b0_remove_projects_to_projects_nodes.py @@ -0,0 +1,137 @@ +"""remove projects_to_projects_nodes + +Revision ID: 71ea254837b0 +Revises: c8f072c72adc +Create Date: 2023-06-19 13:52:39.161616+00:00 + +""" +import sqlalchemy as sa +from alembic import op +from sqlalchemy.dialects import postgresql + +# revision identifiers, used by Alembic. +revision = "71ea254837b0" +down_revision = "c8f072c72adc" +branch_labels = None +depends_on = None + + +# TRIGGERS ----------------- +drop_projects_to_projects_nodes_deleted_trigger = sa.DDL( + "DROP TRIGGER IF EXISTS entry_deleted on projects;" +) +projects_to_projects_nodes_deleted_trigger = sa.DDL( + """ +DROP TRIGGER IF EXISTS entry_deleted on projects; +CREATE TRIGGER entry_deleted +AFTER DELETE ON projects +FOR EACH ROW +EXECUTE FUNCTION delete_orphaned_project_nodes(); + """ +) + +# PROCEDURES ------------------- +drop_delete_orphaned_project_nodes_procedure = sa.DDL( + "DROP FUNCTION delete_orphaned_project_nodes();" +) +delete_orphaned_project_nodes_procedure = sa.DDL( + """ +CREATE OR REPLACE FUNCTION delete_orphaned_project_nodes() +RETURNS TRIGGER AS $$ +BEGIN + DELETE FROM projects_nodes + WHERE NOT EXISTS ( + SELECT 1 FROM projects_to_projects_nodes + WHERE projects_to_projects_nodes.node_id = projects_nodes.node_id + ); + RETURN NULL; +END; +$$ LANGUAGE plpgsql; + """ +) + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_table("projects_to_projects_nodes") + op.add_column( + "projects_nodes", sa.Column("project_uuid", sa.String(), nullable=False) + ) + op.create_unique_constraint( + "projects_nodes__project_uuid_node_id_ukey", + "projects_nodes", + ["project_uuid", "node_id"], + ) + op.create_foreign_key( + "fk_projects_to_projects_nodes_to_projects_uuid", + "projects_nodes", + "projects", + ["project_uuid"], + ["uuid"], + onupdate="CASCADE", + ondelete="CASCADE", + ) + # ### end Alembic commands ### + + # custom + op.drop_constraint("projects_nodes_pkey", "projects_nodes", type_="primary") + op.execute(drop_projects_to_projects_nodes_deleted_trigger) + op.execute(drop_delete_orphaned_project_nodes_procedure) + + +def downgrade(): + # custom + op.create_primary_key("projects_nodes_pkey", "projects_nodes", ["node_id"]) + # ### commands auto generated by Alembic - please adjust! ### + op.drop_constraint( + "fk_projects_to_projects_nodes_to_projects_uuid", + "projects_nodes", + type_="foreignkey", + ) + op.drop_constraint( + "projects_nodes__project_uuid_node_id_ukey", "projects_nodes", type_="unique" + ) + op.drop_column("projects_nodes", "project_uuid") + op.create_table( + "projects_to_projects_nodes", + sa.Column("project_uuid", sa.VARCHAR(), autoincrement=False, nullable=True), + sa.Column("node_id", sa.VARCHAR(), autoincrement=False, nullable=True), + sa.Column( + "created", + postgresql.TIMESTAMP(timezone=True), + server_default=sa.text("now()"), + autoincrement=False, + nullable=False, + ), + sa.Column( + "modified", + postgresql.TIMESTAMP(timezone=True), + server_default=sa.text("now()"), + autoincrement=False, + nullable=False, + ), + sa.ForeignKeyConstraint( + ["node_id"], + ["projects_nodes.node_id"], + name="fk_projects_to_projects_nodes_to_projects_nodes_node_id", + onupdate="CASCADE", + ondelete="CASCADE", + ), + sa.ForeignKeyConstraint( + ["project_uuid"], + ["projects.uuid"], + name="fk_projects_to_projects_nodes_to_projects_uuid", + onupdate="CASCADE", + ondelete="CASCADE", + ), + sa.UniqueConstraint( + "project_uuid", + "node_id", + name="projects_to_projects_nodes_project_uuid_node_id_key", + ), + ) + # ### end Alembic commands ### + + # custom + op.execute(delete_orphaned_project_nodes_procedure) + op.execute(projects_to_projects_nodes_deleted_trigger) diff --git a/packages/postgres-database/src/simcore_postgres_database/models/projects_nodes.py b/packages/postgres-database/src/simcore_postgres_database/models/projects_nodes.py index 43432dec41c..bf2b02287ed 100644 --- a/packages/postgres-database/src/simcore_postgres_database/models/projects_nodes.py +++ b/packages/postgres-database/src/simcore_postgres_database/models/projects_nodes.py @@ -5,7 +5,6 @@ """ import sqlalchemy as sa -from sqlalchemy import event from sqlalchemy.dialects.postgresql import JSONB from ._common import ( @@ -19,11 +18,24 @@ projects_nodes = sa.Table( "projects_nodes", metadata, + sa.Column( + "project_uuid", + sa.String, + sa.ForeignKey( + projects.c.uuid, + onupdate="CASCADE", + ondelete="CASCADE", + name="fk_projects_to_projects_nodes_to_projects_uuid", + ), + nullable=False, + primary_key=False, + doc="The project unique identifier", + ), sa.Column( "node_id", sa.String, nullable=False, - primary_key=True, + primary_key=False, doc="The node unique identifier", ), sa.Column( @@ -36,42 +48,10 @@ # TIME STAMPS ---- column_created_datetime(timezone=True), column_modified_datetime(timezone=True), + sa.UniqueConstraint( + "project_uuid", "node_id", name="projects_nodes__project_uuid_node_id_ukey" + ), ) register_modified_datetime_auto_update_trigger(projects_nodes) - - -# TRIGGERS ----------------- -projects_to_projects_nodes_deleted_trigger = sa.DDL( - """ -DROP TRIGGER IF EXISTS entry_deleted on projects; -CREATE TRIGGER entry_deleted -AFTER DELETE ON projects -FOR EACH ROW -EXECUTE FUNCTION delete_orphaned_project_nodes(); - """ -) - -# PROCEDURES ------------------- -delete_orphaned_project_nodes_procedure = sa.DDL( - """ -CREATE OR REPLACE FUNCTION delete_orphaned_project_nodes() -RETURNS TRIGGER AS $$ -BEGIN - DELETE FROM projects_nodes - WHERE NOT EXISTS ( - SELECT 1 FROM projects_to_projects_nodes - WHERE projects_to_projects_nodes.node_id = projects_nodes.node_id - ); - RETURN NULL; -END; -$$ LANGUAGE plpgsql; - """ -) - -# REGISTER THEM PROCEDURES/TRIGGERS - - -event.listen(projects_nodes, "after_create", delete_orphaned_project_nodes_procedure) -event.listen(projects, "after_create", projects_to_projects_nodes_deleted_trigger) diff --git a/packages/postgres-database/src/simcore_postgres_database/models/projects_to_projects_nodes.py b/packages/postgres-database/src/simcore_postgres_database/models/projects_to_projects_nodes.py deleted file mode 100644 index a3e30281b6a..00000000000 --- a/packages/postgres-database/src/simcore_postgres_database/models/projects_to_projects_nodes.py +++ /dev/null @@ -1,51 +0,0 @@ -""" Groups table - - - List of groups in the framework - - Groups have a ID, name and a list of users that belong to the group -""" - - -import sqlalchemy as sa - -from ._common import ( - column_created_datetime, - column_modified_datetime, - register_modified_datetime_auto_update_trigger, -) -from .base import metadata -from .projects import projects -from .projects_nodes import projects_nodes - -projects_to_projects_nodes = sa.Table( - "projects_to_projects_nodes", - metadata, - sa.Column( - "project_uuid", - sa.String, - sa.ForeignKey( - projects.c.uuid, - onupdate="CASCADE", - ondelete="CASCADE", - name="fk_projects_to_projects_nodes_to_projects_uuid", - ), - doc="The project unique identifier", - ), - sa.Column( - "node_id", - sa.String, - sa.ForeignKey( - projects_nodes.c.node_id, - onupdate="CASCADE", - ondelete="CASCADE", - name="fk_projects_to_projects_nodes_to_projects_nodes_node_id", - ), - doc="The node unique identifier", - ), - # TIME STAMPS ---- - column_created_datetime(timezone=True), - column_modified_datetime(timezone=True), - sa.UniqueConstraint("project_uuid", "node_id"), -) - - -register_modified_datetime_auto_update_trigger(projects_to_projects_nodes) From 2c96ac9cf179f78010cfca9e24589075896e772d Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Mon, 19 Jun 2023 17:26:35 +0200 Subject: [PATCH 2/4] removed mapping table --- .../utils_projects_nodes.py | 113 +++++++----------- .../tests/test_utils_projects_nodes.py | 68 ----------- 2 files changed, 40 insertions(+), 141 deletions(-) diff --git a/packages/postgres-database/src/simcore_postgres_database/utils_projects_nodes.py b/packages/postgres-database/src/simcore_postgres_database/utils_projects_nodes.py index 1a15bb4f907..81737a87fed 100644 --- a/packages/postgres-database/src/simcore_postgres_database/utils_projects_nodes.py +++ b/packages/postgres-database/src/simcore_postgres_database/utils_projects_nodes.py @@ -4,11 +4,9 @@ import sqlalchemy from aiopg.sa.connection import SAConnection -from sqlalchemy import literal_column from .errors import ForeignKeyViolation, UniqueViolation from .models.projects_nodes import projects_nodes -from .models.projects_to_projects_nodes import projects_to_projects_nodes # @@ -55,10 +53,9 @@ async def add( connection: SAConnection, *, node_id: uuid.UUID, - node: ProjectNodeCreate | None, + node: ProjectNodeCreate, ) -> ProjectNode: - """creates a new entry in *projects_nodes* and *projects_to_projects_nodes* tables or - attaches an existing one if node is set to `None` + """creates a new entry in *projects_nodes* and *projects_to_projects_nodes* tables NOTE: Do not use this in an asyncio.gather call as this will fail! @@ -68,28 +65,26 @@ async def add( ProjectsNodesNodeNotFound: in case the node does not exist """ - created_node = None async with connection.begin(): try: - if node: - result = await connection.execute( - projects_nodes.insert() - .values(node_id=f"{node_id}", **asdict(node)) - .returning(literal_column("*")) - ) - created_node_db = await result.first() - assert created_node_db # nosec - created_node = ProjectNode(**dict(created_node_db.items())) - else: - created_node = await self._get_node(connection, node_id=node_id) - result = await connection.execute( - projects_to_projects_nodes.insert().values( + projects_nodes.insert() + .values( project_uuid=f"{self.project_uuid}", node_id=f"{node_id}", + **asdict(node), + ) + .returning( + *[ + c + for c in projects_nodes.c + if c is not projects_nodes.c.project_uuid + ] ) ) - assert result.rowcount == 1 # nosec + created_node_db = await result.first() + assert created_node_db # nosec + created_node = ProjectNode(**dict(created_node_db.items())) return created_node except ForeignKeyViolation as exc: @@ -108,11 +103,9 @@ async def list(self, connection: SAConnection) -> list[ProjectNode]: NOTE: Do not use this in an asyncio.gather call as this will fail! """ - list_stmt = ( - sqlalchemy.select(projects_nodes) - .select_from(self._join_projects_to_projects_nodes()) - .where(projects_to_projects_nodes.c.project_uuid == f"{self.project_uuid}") - ) + list_stmt = sqlalchemy.select( + *[c for c in projects_nodes.c if c is not projects_nodes.c.project_uuid] + ).where(projects_nodes.c.project_uuid == f"{self.project_uuid}") nodes = [ ProjectNode(**dict(row.items())) async for row in connection.execute(list_stmt) @@ -128,13 +121,11 @@ async def get(self, connection: SAConnection, *, node_id: uuid.UUID) -> ProjectN ProjectsNodesNodeNotFound: _description_ """ - get_stmt = ( - sqlalchemy.select(projects_nodes) - .select_from(self._join_projects_to_projects_nodes()) - .where( - (projects_to_projects_nodes.c.project_uuid == f"{self.project_uuid}") - & (projects_to_projects_nodes.c.node_id == f"{node_id}") - ) + get_stmt = sqlalchemy.select( + *[c for c in projects_nodes.c if c is not projects_nodes.c.project_uuid] + ).where( + (projects_nodes.c.project_uuid == f"{self.project_uuid}") + & (projects_nodes.c.node_id == f"{node_id}") ) result = await connection.execute(get_stmt) @@ -145,9 +136,8 @@ async def get(self, connection: SAConnection, *, node_id: uuid.UUID) -> ProjectN assert row # nosec return ProjectNode(**dict(row.items())) - @staticmethod async def update( - connection: SAConnection, *, node_id: uuid.UUID, **values + self, connection: SAConnection, *, node_id: uuid.UUID, **values ) -> ProjectNode: """update a node in the current project @@ -159,8 +149,13 @@ async def update( update_stmt = ( projects_nodes.update() .values(**values) - .where(projects_nodes.c.node_id == f"{node_id}") - .returning(literal_column("*")) + .where( + (projects_nodes.c.project_uuid == f"{self.project_uuid}") + & (projects_nodes.c.node_id == f"{node_id}") + ) + .returning( + *[c for c in projects_nodes.c if c is not projects_nodes.c.project_uuid] + ) ) result = await connection.execute(update_stmt) updated_entry = await result.first() @@ -170,43 +165,15 @@ async def update( return ProjectNode(**dict(updated_entry.items())) async def delete(self, connection: SAConnection, *, node_id: uuid.UUID) -> None: - """delete a node in the current project (if the node is shared it will only unmap it)""" - async with connection.begin(): - # remove mapping - delete_stmt = sqlalchemy.delete(projects_to_projects_nodes).where( - (projects_to_projects_nodes.c.node_id == f"{node_id}") - & (projects_to_projects_nodes.c.project_uuid == f"{self.project_uuid}") - ) - await connection.execute(delete_stmt) - # if this was the last mapping then also delete the node itself - num_remaining_mappings = await connection.scalar( - sqlalchemy.select(sqlalchemy.func.count()) - .select_from(projects_to_projects_nodes) - .where(projects_to_projects_nodes.c.node_id == f"{node_id}") - ) - if num_remaining_mappings == 0: - delete_stmt = sqlalchemy.delete(projects_nodes).where( - projects_nodes.c.node_id == f"{node_id}" - ) - await connection.execute(delete_stmt) + """delete a node in the current project - @staticmethod - async def _get_node(connection: SAConnection, *, node_id: uuid.UUID) -> ProjectNode: - get_stmt = sqlalchemy.select(projects_nodes).where( - projects_nodes.c.node_id == f"{node_id}" - ) - - result = await connection.execute(get_stmt) - assert result # nosec - row = await result.first() - if row is None: - raise ProjectNodesNodeNotFound(f"Node with {node_id} not found") - assert row # nosec - return ProjectNode(**dict(row.items())) + NOTE: Do not use this in an asyncio.gather call as this will fail! - @staticmethod - def _join_projects_to_projects_nodes(): - return projects_to_projects_nodes.join( - projects_nodes, - projects_to_projects_nodes.c.node_id == projects_nodes.c.node_id, + Raises: + Nothing special + """ + delete_stmt = sqlalchemy.delete(projects_nodes).where( + (projects_nodes.c.project_uuid == f"{self.project_uuid}") + & (projects_nodes.c.node_id == f"{node_id}") ) + await connection.execute(delete_stmt) diff --git a/packages/postgres-database/tests/test_utils_projects_nodes.py b/packages/postgres-database/tests/test_utils_projects_nodes.py index e0db97710ea..5e69b8a1840 100644 --- a/packages/postgres-database/tests/test_utils_projects_nodes.py +++ b/packages/postgres-database/tests/test_utils_projects_nodes.py @@ -271,74 +271,6 @@ async def test_delete_node( await projects_nodes_repo.get(connection, node_id=new_node.node_id) -async def test_add_invalid_node_in_existing_project_raises( - connection: SAConnection, - projects_nodes_repo: ProjectNodesRepo, - create_fake_projects_node: Callable[..., ProjectNodeCreate], - create_fake_node_id: Callable[[], uuid.UUID], -): - # create a project node - with pytest.raises(ProjectNodesNodeNotFound): - await projects_nodes_repo.add( - connection, node_id=create_fake_node_id(), node=None - ) - - -async def test_share_nodes_between_projects( - connection: SAConnection, - registered_user: RowProxy, - create_fake_project: Callable[..., Awaitable[RowProxy]], - projects_nodes_repo: ProjectNodesRepo, - create_fake_projects_node: Callable[..., ProjectNodeCreate], - create_fake_node_id: Callable[[], uuid.UUID], -): - # create a project node - created_node = await projects_nodes_repo.add( - connection, node_id=create_fake_node_id(), node=create_fake_projects_node() - ) - assert ( - await projects_nodes_repo.get(connection, node_id=created_node.node_id) - == created_node - ) - - # create a second project and attach the same node - second_project = await create_fake_project(connection, registered_user) - assert second_project - second_projects_nodes_repo = ProjectNodesRepo(project_uuid=second_project.uuid) - assert second_projects_nodes_repo - attached_node = await second_projects_nodes_repo.add( - connection, node_id=created_node.node_id, node=None - ) - assert attached_node - assert attached_node == await second_projects_nodes_repo.get( - connection, node_id=created_node.node_id - ) - - # delete the node from the first project shall not delete the node as it is in the second project - await projects_nodes_repo.delete(connection, node_id=created_node.node_id) - with pytest.raises(ProjectNodesNodeNotFound): - await projects_nodes_repo.get(connection, node_id=created_node.node_id) - # the node is still in the second project - received_node = await second_projects_nodes_repo.get( - connection, node_id=created_node.node_id - ) - assert attached_node == received_node - - # delete the node from the second project shall also delete the node - await second_projects_nodes_repo.delete(connection, node_id=created_node.node_id) - with pytest.raises(ProjectNodesNodeNotFound): - await second_projects_nodes_repo.get(connection, node_id=created_node.node_id) - - # ensure the node was really deleted - result = await connection.execute( - sqlalchemy.select(projects_nodes).where( - projects_nodes.c.node_id == f"{created_node.node_id}" - ) - ) - assert result - assert await result.first() is None - - async def test_delete_project_delete_all_nodes( connection: SAConnection, projects_nodes_repo: ProjectNodesRepo, From 1211be6e361e372ae4648d8a7754d40d308fda2d Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Mon, 19 Jun 2023 17:53:31 +0200 Subject: [PATCH 3/4] @GitHK review: convert unique constraint to primary key --- .../71ea254837b0_remove_projects_to_projects_nodes.py | 11 +++++------ .../models/projects_nodes.py | 4 +--- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/packages/postgres-database/src/simcore_postgres_database/migration/versions/71ea254837b0_remove_projects_to_projects_nodes.py b/packages/postgres-database/src/simcore_postgres_database/migration/versions/71ea254837b0_remove_projects_to_projects_nodes.py index 52311462c92..883b6e48379 100644 --- a/packages/postgres-database/src/simcore_postgres_database/migration/versions/71ea254837b0_remove_projects_to_projects_nodes.py +++ b/packages/postgres-database/src/simcore_postgres_database/migration/versions/71ea254837b0_remove_projects_to_projects_nodes.py @@ -57,8 +57,10 @@ def upgrade(): op.add_column( "projects_nodes", sa.Column("project_uuid", sa.String(), nullable=False) ) - op.create_unique_constraint( - "projects_nodes__project_uuid_node_id_ukey", + # custom + op.drop_constraint("projects_nodes_pkey", "projects_nodes", type_="primary") + op.create_primary_key( + "projects_nodes_pkey", "projects_nodes", ["project_uuid", "node_id"], ) @@ -74,13 +76,13 @@ def upgrade(): # ### end Alembic commands ### # custom - op.drop_constraint("projects_nodes_pkey", "projects_nodes", type_="primary") op.execute(drop_projects_to_projects_nodes_deleted_trigger) op.execute(drop_delete_orphaned_project_nodes_procedure) def downgrade(): # custom + op.drop_constraint("projects_nodes_pkey", "projects_nodes", type_="primary") op.create_primary_key("projects_nodes_pkey", "projects_nodes", ["node_id"]) # ### commands auto generated by Alembic - please adjust! ### op.drop_constraint( @@ -88,9 +90,6 @@ def downgrade(): "projects_nodes", type_="foreignkey", ) - op.drop_constraint( - "projects_nodes__project_uuid_node_id_ukey", "projects_nodes", type_="unique" - ) op.drop_column("projects_nodes", "project_uuid") op.create_table( "projects_to_projects_nodes", diff --git a/packages/postgres-database/src/simcore_postgres_database/models/projects_nodes.py b/packages/postgres-database/src/simcore_postgres_database/models/projects_nodes.py index bf2b02287ed..aadf9934909 100644 --- a/packages/postgres-database/src/simcore_postgres_database/models/projects_nodes.py +++ b/packages/postgres-database/src/simcore_postgres_database/models/projects_nodes.py @@ -48,9 +48,7 @@ # TIME STAMPS ---- column_created_datetime(timezone=True), column_modified_datetime(timezone=True), - sa.UniqueConstraint( - "project_uuid", "node_id", name="projects_nodes__project_uuid_node_id_ukey" - ), + sa.PrimaryKeyConstraint("project_uuid", "node_id"), ) From 4460cda26684632b0c639feb4402fa2a78506b8b Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Mon, 19 Jun 2023 17:57:42 +0200 Subject: [PATCH 4/4] sql 2.0 --- .../postgres-database/tests/products/test_models_products.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/postgres-database/tests/products/test_models_products.py b/packages/postgres-database/tests/products/test_models_products.py index a2e80696867..c797799ba45 100644 --- a/packages/postgres-database/tests/products/test_models_products.py +++ b/packages/postgres-database/tests/products/test_models_products.py @@ -36,7 +36,7 @@ async def test_load_products( async with pg_engine.acquire() as conn: await make_products_table(conn) - stmt = sa.select([c for c in products.columns if c not in exclude]) + stmt = sa.select(*[c for c in products.columns if c not in exclude]) result: ResultProxy = await conn.execute(stmt) assert result.returns_rows @@ -102,7 +102,7 @@ async def test_jinja2_templates_table( # prints those products having customized templates j = products.join(jinja2_templates) stmt = sa.select( - [products.c.name, jinja2_templates.c.name, products.c.short_name] + products.c.name, jinja2_templates.c.name, products.c.short_name ).select_from(j) result: ResultProxy = await conn.execute(stmt)