changeset 3583:16ade4ad63f3 sqlalchemy

core (memory/sqla_mapping): fix some technical debt: a first Alembic revision file is joined to migrate existing databases to new mapping. This patch: - set `nullable=False` in various places - drop legacy `message_types` table, and use `Enum` instead - use naming convention, and get rid of anonymous constraint (this is the main reason why the revision file is so long)
author Goffi <goffi@goffi.org>
date Sun, 27 Jun 2021 00:15:40 +0200
parents 71516731d0aa
children edc79cefe968
files sat/memory/migration/versions/602caf848068_drop_message_types_table_fix_nullable.py sat/memory/sqla.py sat/memory/sqla_mapping.py
diffstat 3 files changed, 466 insertions(+), 36 deletions(-) [+]
line wrap: on
line diff
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/sat/memory/migration/versions/602caf848068_drop_message_types_table_fix_nullable.py	Sun Jun 27 00:15:40 2021 +0200
@@ -0,0 +1,410 @@
+"""drop message_types table + fix nullable + rename constraints
+
+Revision ID: 602caf848068
+Revises:
+Create Date: 2021-06-26 12:42:54.148313
+
+"""
+from alembic import op
+from sqlalchemy import (
+    Table,
+    Column,
+    MetaData,
+    TEXT,
+    INTEGER,
+    Text,
+    Integer,
+    Float,
+    Enum,
+    ForeignKey,
+    Index,
+    PrimaryKeyConstraint,
+)
+from sqlalchemy.sql import table, column
+
+
+# revision identifiers, used by Alembic.
+revision = "602caf848068"
+down_revision = None
+branch_labels = None
+depends_on = None
+
+
+def upgrade():
+    # we have to recreate former tables for batch_alter_table's reflexion, otherwise the
+    # database will be used, and this will keep unammed UNIQUE constraints in addition
+    # to the named ones that we create
+    metadata = MetaData(
+        naming_convention={
+            "ix": "ix_%(column_0_label)s",
+            "uq": "uq_%(table_name)s_%(column_0_name)s",
+            "ck": "ck_%(table_name)s_%(constraint_name)s",
+            "fk": "fk_%(table_name)s_%(column_0_name)s_%(referred_table_name)s",
+            "pk": "pk_%(table_name)s",
+        },
+    )
+
+    old_profiles_table = Table(
+        "profiles",
+        metadata,
+        Column("id", Integer, primary_key=True, nullable=True, autoincrement=False),
+        Column("name", Text, unique=True),
+    )
+
+    old_components_table = Table(
+        "components",
+        metadata,
+        Column(
+            "profile_id",
+            ForeignKey("profiles.id", ondelete="CASCADE"),
+            nullable=True,
+            primary_key=True,
+        ),
+        Column("entry_point", Text, nullable=False),
+    )
+
+    old_message_table = Table(
+        "message",
+        metadata,
+        Column("id", Integer, primary_key=True, nullable=True, autoincrement=False),
+        Column("history_uid", ForeignKey("history.uid", ondelete="CASCADE")),
+        Column("message", Text),
+        Column("language", Text),
+        Index("message__history_uid", "history_uid"),
+    )
+
+    old_subject_table = Table(
+        "subject",
+        metadata,
+        Column("id", Integer, primary_key=True, nullable=True, autoincrement=False),
+        Column("history_uid", ForeignKey("history.uid", ondelete="CASCADE")),
+        Column("subject", Text),
+        Column("language", Text),
+        Index("subject__history_uid", "history_uid"),
+    )
+
+    old_thread_table = Table(
+        "thread",
+        metadata,
+        Column("id", Integer, primary_key=True, nullable=True, autoincrement=False),
+        Column("history_uid", ForeignKey("history.uid", ondelete="CASCADE")),
+        Column("thread_id", Text),
+        Column("parent_id", Text),
+        Index("thread__history_uid", "history_uid"),
+    )
+
+    old_history_table = Table(
+        "history",
+        metadata,
+        Column("uid", Text, primary_key=True, nullable=True),
+        Column("stanza_id", Text),
+        Column("update_uid", Text),
+        Column("profile_id", Integer, ForeignKey("profiles.id", ondelete="CASCADE")),
+        Column("source", Text),
+        Column("dest", Text),
+        Column("source_res", Text),
+        Column("dest_res", Text),
+        Column("timestamp", Float, nullable=False),
+        Column("received_timestamp", Float),
+        Column("type", Text, ForeignKey("message_types.type")),
+        Column("extra", Text),
+        Index("history__profile_id_timestamp", "profile_id", "timestamp"),
+        Index(
+            "history__profile_id_received_timestamp", "profile_id", "received_timestamp"
+        ),
+    )
+
+    old_param_gen_table = Table(
+        "param_gen",
+        metadata,
+        Column("category", Text, primary_key=True),
+        Column("name", Text, primary_key=True),
+        Column("value", Text),
+    )
+
+    old_param_ind_table = Table(
+        "param_ind",
+        metadata,
+        Column("category", Text, primary_key=True),
+        Column("name", Text, primary_key=True),
+        Column(
+            "profile_id", ForeignKey("profiles.id", ondelete="CASCADE"), primary_key=True
+        ),
+        Column("value", Text),
+    )
+
+    old_private_gen_table = Table(
+        "private_gen",
+        metadata,
+        Column("namespace", Text, primary_key=True),
+        Column("key", Text, primary_key=True),
+        Column("value", Text),
+    )
+
+    old_private_ind_table = Table(
+        "private_ind",
+        metadata,
+        Column("namespace", Text, primary_key=True),
+        Column("key", Text, primary_key=True),
+        Column(
+            "profile_id", ForeignKey("profiles.id", ondelete="CASCADE"), primary_key=True
+        ),
+        Column("value", Text),
+    )
+
+    old_private_gen_bin_table = Table(
+        "private_gen_bin",
+        metadata,
+        Column("namespace", Text, primary_key=True),
+        Column("key", Text, primary_key=True),
+        Column("value", Text),
+    )
+
+    old_private_ind_bin_table = Table(
+        "private_ind_bin",
+        metadata,
+        Column("namespace", Text, primary_key=True),
+        Column("key", Text, primary_key=True),
+        Column(
+            "profile_id", ForeignKey("profiles.id", ondelete="CASCADE"), primary_key=True
+        ),
+        Column("value", Text),
+    )
+
+    old_files_table = Table(
+        "files",
+        metadata,
+        Column("id", Text, primary_key=True),
+        Column("public_id", Text, unique=True),
+        Column("version", Text, primary_key=True),
+        Column("parent", Text, nullable=False),
+        Column(
+            "type",
+            Enum("file", "directory", name="file_type", create_constraint=True),
+            nullable=False,
+            server_default="file",
+        ),
+        Column("file_hash", Text),
+        Column("hash_algo", Text),
+        Column("name", Text, nullable=False),
+        Column("size", Integer),
+        Column("namespace", Text),
+        Column("media_type", Text),
+        Column("media_subtype", Text),
+        Column("created", Float, nullable=False),
+        Column("modified", Float),
+        Column("owner", Text),
+        Column("access", Text),
+        Column("extra", Text),
+        Column("profile_id", ForeignKey("profiles.id", ondelete="CASCADE")),
+        Index("files__profile_id_owner_parent", "profile_id", "owner", "parent"),
+        Index(
+            "files__profile_id_owner_media_type_media_subtype",
+            "profile_id",
+            "owner",
+            "media_type",
+            "media_subtype",
+        ),
+    )
+
+    op.drop_table("message_types")
+
+    with op.batch_alter_table(
+        "profiles", copy_from=old_profiles_table, schema=None
+    ) as batch_op:
+        batch_op.create_unique_constraint(batch_op.f("uq_profiles_name"), ["name"])
+
+    with op.batch_alter_table(
+        "components",
+        copy_from=old_components_table,
+        naming_convention={
+            "fk": "fk_%(table_name)s_%(column_0_name)s_%(referred_table_name)s",
+        },
+        schema=None,
+    ) as batch_op:
+        batch_op.create_unique_constraint(batch_op.f("uq_profiles_name"), ["name"])
+
+    with op.batch_alter_table(
+        "history",
+        copy_from=old_history_table,
+        naming_convention={
+            "fk": "fk_%(table_name)s_%(column_0_name)s_%(referred_table_name)s",
+        },
+        schema=None,
+    ) as batch_op:
+        batch_op.alter_column("uid", existing_type=TEXT(), nullable=False)
+        batch_op.alter_column(
+            "type",
+            type_=Enum(
+                "chat",
+                "error",
+                "groupchat",
+                "headline",
+                "normal",
+                "info",
+                name="message_type",
+                create_constraint=True,
+            ),
+            existing_type=TEXT(),
+            nullable=False,
+        )
+        batch_op.create_unique_constraint(
+            batch_op.f("uq_history_profile_id"),
+            ["profile_id", "stanza_id", "source", "dest"],
+        )
+        batch_op.drop_constraint("fk_history_type_message_types", type_="foreignkey")
+
+    with op.batch_alter_table(
+        "message", copy_from=old_message_table, schema=None
+    ) as batch_op:
+        batch_op.alter_column(
+            "id", existing_type=INTEGER(), nullable=False, autoincrement=False
+        )
+
+    with op.batch_alter_table(
+        "subject", copy_from=old_subject_table, schema=None
+    ) as batch_op:
+        batch_op.alter_column(
+            "id", existing_type=INTEGER(), nullable=False, autoincrement=False
+        )
+
+    with op.batch_alter_table(
+        "thread", copy_from=old_thread_table, schema=None
+    ) as batch_op:
+        batch_op.alter_column(
+            "id", existing_type=INTEGER(), nullable=False, autoincrement=False
+        )
+
+    with op.batch_alter_table(
+        "param_gen", copy_from=old_param_gen_table, schema=None
+    ) as batch_op:
+        batch_op.alter_column("category", existing_type=TEXT(), nullable=False)
+        batch_op.alter_column("name", existing_type=TEXT(), nullable=False)
+
+    with op.batch_alter_table(
+        "param_ind", copy_from=old_param_ind_table, schema=None
+    ) as batch_op:
+        batch_op.alter_column("category", existing_type=TEXT(), nullable=False)
+        batch_op.alter_column("name", existing_type=TEXT(), nullable=False)
+        batch_op.alter_column("profile_id", existing_type=INTEGER(), nullable=False)
+
+    with op.batch_alter_table(
+        "private_gen", copy_from=old_private_gen_table, schema=None
+    ) as batch_op:
+        batch_op.alter_column("namespace", existing_type=TEXT(), nullable=False)
+        batch_op.alter_column("key", existing_type=TEXT(), nullable=False)
+
+    with op.batch_alter_table(
+        "private_ind", copy_from=old_private_ind_table, schema=None
+    ) as batch_op:
+        batch_op.alter_column("namespace", existing_type=TEXT(), nullable=False)
+        batch_op.alter_column("key", existing_type=TEXT(), nullable=False)
+        batch_op.alter_column("profile_id", existing_type=INTEGER(), nullable=False)
+
+    with op.batch_alter_table(
+        "private_gen_bin", copy_from=old_private_gen_bin_table, schema=None
+    ) as batch_op:
+        batch_op.alter_column("namespace", existing_type=TEXT(), nullable=False)
+        batch_op.alter_column("key", existing_type=TEXT(), nullable=False)
+
+    # found some invalid rows in local database, maybe old values made during development,
+    # but in doubt we have to delete them
+    op.execute("DELETE FROM private_ind_bin WHERE namespace IS NULL")
+
+    with op.batch_alter_table(
+        "private_ind_bin", copy_from=old_private_ind_bin_table, schema=None
+    ) as batch_op:
+        batch_op.alter_column("namespace", existing_type=TEXT(), nullable=False)
+        batch_op.alter_column("key", existing_type=TEXT(), nullable=False)
+        batch_op.alter_column("profile_id", existing_type=INTEGER(), nullable=False)
+
+    with op.batch_alter_table(
+        "files", copy_from=old_files_table, schema=None
+    ) as batch_op:
+        batch_op.create_unique_constraint(batch_op.f("uq_files_public_id"), ["public_id"])
+        batch_op.alter_column(
+            "type",
+            type_=Enum("file", "directory", name="file_type", create_constraint=True),
+            existing_type=Text(),
+            nullable=False,
+        )
+
+
+def downgrade():
+    # downgrade doesn't restore the exact same state as before upgrade, as it
+    # would be useless and waste of resource to restore broken things such as
+    # anonymous constraints
+    with op.batch_alter_table("thread", schema=None) as batch_op:
+        batch_op.alter_column(
+            "id", existing_type=INTEGER(), nullable=True, autoincrement=False
+        )
+
+    with op.batch_alter_table("subject", schema=None) as batch_op:
+        batch_op.alter_column(
+            "id", existing_type=INTEGER(), nullable=True, autoincrement=False
+        )
+
+    with op.batch_alter_table("private_ind_bin", schema=None) as batch_op:
+        batch_op.alter_column("profile_id", existing_type=INTEGER(), nullable=True)
+        batch_op.alter_column("key", existing_type=TEXT(), nullable=True)
+        batch_op.alter_column("namespace", existing_type=TEXT(), nullable=True)
+
+    with op.batch_alter_table("private_ind", schema=None) as batch_op:
+        batch_op.alter_column("profile_id", existing_type=INTEGER(), nullable=True)
+        batch_op.alter_column("key", existing_type=TEXT(), nullable=True)
+        batch_op.alter_column("namespace", existing_type=TEXT(), nullable=True)
+
+    with op.batch_alter_table("private_gen_bin", schema=None) as batch_op:
+        batch_op.alter_column("key", existing_type=TEXT(), nullable=True)
+        batch_op.alter_column("namespace", existing_type=TEXT(), nullable=True)
+
+    with op.batch_alter_table("private_gen", schema=None) as batch_op:
+        batch_op.alter_column("key", existing_type=TEXT(), nullable=True)
+        batch_op.alter_column("namespace", existing_type=TEXT(), nullable=True)
+
+    with op.batch_alter_table("param_ind", schema=None) as batch_op:
+        batch_op.alter_column("profile_id", existing_type=INTEGER(), nullable=True)
+        batch_op.alter_column("name", existing_type=TEXT(), nullable=True)
+        batch_op.alter_column("category", existing_type=TEXT(), nullable=True)
+
+    with op.batch_alter_table("param_gen", schema=None) as batch_op:
+        batch_op.alter_column("name", existing_type=TEXT(), nullable=True)
+        batch_op.alter_column("category", existing_type=TEXT(), nullable=True)
+
+    with op.batch_alter_table("message", schema=None) as batch_op:
+        batch_op.alter_column(
+            "id", existing_type=INTEGER(), nullable=True, autoincrement=False
+        )
+
+    op.create_table(
+        "message_types",
+        Column("type", TEXT(), nullable=True),
+        PrimaryKeyConstraint("type"),
+    )
+    message_types_table = table("message_types", column("type", TEXT()))
+    op.bulk_insert(
+        message_types_table,
+        [
+            {"type": "chat"},
+            {"type": "error"},
+            {"type": "groupchat"},
+            {"type": "headline"},
+            {"type": "normal"},
+            {"type": "info"},
+        ],
+    )
+
+    with op.batch_alter_table("history", schema=None) as batch_op:
+        batch_op.alter_column(
+            "type",
+            type_=TEXT(),
+            existing_type=TEXT(),
+            nullable=True,
+        )
+        batch_op.create_foreign_key(
+            batch_op.f("fk_history_type_message_types"),
+            "message_types",
+            ["type"],
+            ["type"],
+        )
+        batch_op.alter_column("uid", existing_type=TEXT(), nullable=True)
--- a/sat/memory/sqla.py	Fri Jun 25 17:55:23 2021 +0200
+++ b/sat/memory/sqla.py	Sun Jun 27 00:15:40 2021 +0200
@@ -149,14 +149,13 @@
     @aio
     async def initialise(self) -> None:
         log.info(_("Connecting database"))
+
         db_config = sqla_config.getDbConfig()
         engine = create_async_engine(
             db_config["url"],
             future=True
         )
-        self.session = sessionmaker(
-            engine, expire_on_commit=False, class_=AsyncSession
-        )
+
         new_base = not db_config["path"].exists()
         if new_base:
             log.info(_("The database is new, creating the tables"))
@@ -164,6 +163,10 @@
         else:
             await self.checkAndUpdateDB(engine, db_config)
 
+        self.session = sessionmaker(
+            engine, expire_on_commit=False, class_=AsyncSession
+        )
+
         async with self.session() as session:
             result = await session.execute(select(Profile))
             for p in result.scalars():
--- a/sat/memory/sqla_mapping.py	Fri Jun 25 17:55:23 2021 +0200
+++ b/sat/memory/sqla_mapping.py	Sun Jun 27 00:15:40 2021 +0200
@@ -19,7 +19,7 @@
 import pickle
 import json
 from sqlalchemy import (
-    Column, Integer, Text, Float, Enum, ForeignKey, UniqueConstraint, Index,
+    MetaData, Column, Integer, Text, Float, Enum, ForeignKey, UniqueConstraint, Index,
 )
 
 from sqlalchemy.orm import declarative_base, relationship
@@ -28,7 +28,17 @@
 from datetime import datetime
 
 
-Base = declarative_base()
+Base = declarative_base(
+    metadata=MetaData(
+        naming_convention={
+            "ix": 'ix_%(column_0_label)s',
+            "uq": "uq_%(table_name)s_%(column_0_name)s",
+            "ck": "ck_%(table_name)s_%(constraint_name)s",
+            "fk": "fk_%(table_name)s_%(column_0_name)s_%(referred_table_name)s",
+            "pk": "pk_%(table_name)s"
+        }
+    )
+)
 # keys which are in message data extra but not stored in extra field this is
 # because those values are stored in separate fields
 NOT_IN_EXTRA = ('stanza_id', 'received_timestamp', 'update_uid')
@@ -137,12 +147,6 @@
     profile = relationship("Profile")
 
 
-class MessageType(Base):
-    __tablename__ = "message_types"
-
-    type = Column(Text, primary_key=True, nullable=True)
-
-
 class History(Base):
     __tablename__ = "history"
     __table_args__ = (
@@ -153,7 +157,7 @@
         )
     )
 
-    uid = Column(Text, primary_key=True, nullable=True)
+    uid = Column(Text, primary_key=True)
     stanza_id = Column(Text)
     update_uid = Column(Text)
     profile_id = Column(ForeignKey("profiles.id", ondelete="CASCADE"))
@@ -163,11 +167,24 @@
     dest_res = Column(Text)
     timestamp = Column(Float, nullable=False)
     received_timestamp = Column(Float)
-    type = Column(ForeignKey("message_types.type"))
+    type = Column(
+        Enum(
+            "chat",
+            "error",
+            "groupchat",
+            "headline",
+            "normal",
+            # info is not XMPP standard, but used to keep track of info like join/leave
+            # in a MUC
+            "info",
+            name="message_type",
+            create_constraint=True,
+        ),
+        nullable=False,
+    )
     extra = Column(LegacyPickle)
 
     profile = relationship("Profile")
-    message_type = relationship("MessageType")
     messages = relationship("Message", backref="history", passive_deletes=True)
     subjects = relationship("Subject", backref="history", passive_deletes=True)
     thread = relationship(
@@ -268,7 +285,6 @@
     id = Column(
         Integer,
         primary_key=True,
-        nullable=True,
         # cf. note for Profile.id
         autoincrement=False
     )
@@ -292,7 +308,6 @@
     id = Column(
         Integer,
         primary_key=True,
-        nullable=True,
         # cf. note for Profile.id
         autoincrement=False,
     )
@@ -316,7 +331,6 @@
     id = Column(
         Integer,
         primary_key=True,
-        nullable=True,
         # cf. note for Profile.id
         autoincrement=False,
     )
@@ -333,18 +347,18 @@
 class ParamGen(Base):
     __tablename__ = "param_gen"
 
-    category = Column(Text, primary_key=True, nullable=True)
-    name = Column(Text, primary_key=True, nullable=True)
+    category = Column(Text, primary_key=True)
+    name = Column(Text, primary_key=True)
     value = Column(Text)
 
 
 class ParamInd(Base):
     __tablename__ = "param_ind"
 
-    category = Column(Text, primary_key=True, nullable=True)
-    name = Column(Text, primary_key=True, nullable=True)
+    category = Column(Text, primary_key=True)
+    name = Column(Text, primary_key=True)
     profile_id = Column(
-        ForeignKey("profiles.id", ondelete="CASCADE"), primary_key=True, nullable=True
+        ForeignKey("profiles.id", ondelete="CASCADE"), primary_key=True
     )
     value = Column(Text)
 
@@ -354,18 +368,18 @@
 class PrivateGen(Base):
     __tablename__ = "private_gen"
 
-    namespace = Column(Text, primary_key=True, nullable=True)
-    key = Column(Text, primary_key=True, nullable=True)
+    namespace = Column(Text, primary_key=True)
+    key = Column(Text, primary_key=True)
     value = Column(Text)
 
 
 class PrivateInd(Base):
     __tablename__ = "private_ind"
 
-    namespace = Column(Text, primary_key=True, nullable=True)
-    key = Column(Text, primary_key=True, nullable=True)
+    namespace = Column(Text, primary_key=True)
+    key = Column(Text, primary_key=True)
     profile_id = Column(
-        ForeignKey("profiles.id", ondelete="CASCADE"), primary_key=True, nullable=True
+        ForeignKey("profiles.id", ondelete="CASCADE"), primary_key=True
     )
     value = Column(Text)
 
@@ -375,18 +389,18 @@
 class PrivateGenBin(Base):
     __tablename__ = "private_gen_bin"
 
-    namespace = Column(Text, primary_key=True, nullable=True)
-    key = Column(Text, primary_key=True, nullable=True)
+    namespace = Column(Text, primary_key=True)
+    key = Column(Text, primary_key=True)
     value = Column(LegacyPickle)
 
 
 class PrivateIndBin(Base):
     __tablename__ = "private_ind_bin"
 
-    namespace = Column(Text, primary_key=True, nullable=True)
-    key = Column(Text, primary_key=True, nullable=True)
+    namespace = Column(Text, primary_key=True)
+    key = Column(Text, primary_key=True)
     profile_id = Column(
-        ForeignKey("profiles.id", ondelete="CASCADE"), primary_key=True, nullable=True
+        ForeignKey("profiles.id", ondelete="CASCADE"), primary_key=True
     )
     value = Column(LegacyPickle)
 
@@ -406,15 +420,18 @@
         )
     )
 
-    id = Column(Text, primary_key=True, nullable=False)
+    id = Column(Text, primary_key=True)
     public_id = Column(Text, unique=True)
-    version = Column(Text, primary_key=True, nullable=False)
+    version = Column(Text, primary_key=True)
     parent = Column(Text, nullable=False)
     type = Column(
-        Enum("file", "directory", create_constraint=True),
+        Enum(
+            "file", "directory",
+            name="file_type",
+            create_constraint=True
+        ),
         nullable=False,
         server_default="file",
-        # name="file_type",
     )
     file_hash = Column(Text)
     hash_algo = Column(Text)