Merge pull request #199 from ehdgua01/fix-ddl-format-and-writing-version-file

[Enhancement] Fix version file formats

aerich/ddl/__init__.py

@@ -23,7 +23,12 @@ class BaseDDL:
     _DROP_INDEX_TEMPLATE = 'ALTER TABLE "{table_name}" DROP INDEX "{index_name}"'
     _ADD_FK_TEMPLATE = 'ALTER TABLE "{table_name}" ADD CONSTRAINT "{fk_name}" FOREIGN KEY ("{db_column}") REFERENCES "{table}" ("{field}") ON DELETE {on_delete}'
     _DROP_FK_TEMPLATE = 'ALTER TABLE "{table_name}" DROP FOREIGN KEY "{fk_name}"'
-    _M2M_TABLE_TEMPLATE = 'CREATE TABLE "{table_name}" ("{backward_key}" {backward_type} NOT NULL REFERENCES "{backward_table}" ("{backward_field}") ON DELETE CASCADE,"{forward_key}" {forward_type} NOT NULL REFERENCES "{forward_table}" ("{forward_field}") ON DELETE {on_delete}){extra}{comment}'
+    _M2M_TABLE_TEMPLATE = (
+        'CREATE TABLE "{table_name}" (\n'
+        '    "{backward_key}" {backward_type} NOT NULL REFERENCES "{backward_table}" ("{backward_field}") ON DELETE CASCADE,\n'
+        '    "{forward_key}" {forward_type} NOT NULL REFERENCES "{forward_table}" ("{forward_field}") ON DELETE {on_delete}\n'
+        "){extra}{comment}"
+    )
     _MODIFY_COLUMN_TEMPLATE = 'ALTER TABLE "{table_name}" MODIFY COLUMN {column}'
     _CHANGE_COLUMN_TEMPLATE = (
         'ALTER TABLE "{table_name}" CHANGE {old_column_name} {new_column_name} {new_column_type}'

aerich/ddl/mysql/__init__.py

@@ -22,6 +22,11 @@ class MysqlDDL(BaseDDL):
     _DROP_INDEX_TEMPLATE = "ALTER TABLE `{table_name}` DROP INDEX `{index_name}`"
     _ADD_FK_TEMPLATE = "ALTER TABLE `{table_name}` ADD CONSTRAINT `{fk_name}` FOREIGN KEY (`{db_column}`) REFERENCES `{table}` (`{field}`) ON DELETE {on_delete}"
     _DROP_FK_TEMPLATE = "ALTER TABLE `{table_name}` DROP FOREIGN KEY `{fk_name}`"
-    _M2M_TABLE_TEMPLATE = "CREATE TABLE `{table_name}` (`{backward_key}` {backward_type} NOT NULL REFERENCES `{backward_table}` (`{backward_field}`) ON DELETE CASCADE,`{forward_key}` {forward_type} NOT NULL REFERENCES `{forward_table}` (`{forward_field}`) ON DELETE CASCADE){extra}{comment}"
+    _M2M_TABLE_TEMPLATE = (
+        "CREATE TABLE `{table_name}` (\n"
+        "    `{backward_key}` {backward_type} NOT NULL REFERENCES `{backward_table}` (`{backward_field}`) ON DELETE CASCADE,\n"
+        "    `{forward_key}` {forward_type} NOT NULL REFERENCES `{forward_table}` (`{forward_field}`) ON DELETE CASCADE\n"
+        "){extra}{comment}"
+    )
     _MODIFY_COLUMN_TEMPLATE = "ALTER TABLE `{table_name}` MODIFY COLUMN {column}"
     _RENAME_TABLE_TEMPLATE = "ALTER TABLE `{old_table_name}` RENAME TO `{new_table_name}`"

aerich/utils.py

@@ -96,8 +96,8 @@ def get_version_content_from_file(version_file: Union[str, Path]) -> Dict:
     upgrade_content = content[first + len(_UPGRADE) : second].strip()  # noqa:E203
     downgrade_content = content[second + len(_DOWNGRADE) :].strip()  # noqa:E203
     ret = {
-        "upgrade": list(filter(lambda x: x or False, upgrade_content.split(";\n"))),
+        "upgrade": [line for line in upgrade_content.split(";\n") if line],
-        "downgrade": list(filter(lambda x: x or False, downgrade_content.split(";\n"))),
+        "downgrade": [line for line in downgrade_content.split(";\n") if line],
     }
     return ret
 
@@ -109,25 +109,22 @@ def write_version_file(version_file: Path, content: Dict):
     :param content:
     :return:
     """
-    text = _UPGRADE
+
-    upgrade = content.get("upgrade")
+    def append_ddl(ddl: str):
-    if len(upgrade) > 1:
+        if not ddl.endswith(";"):
-        text += ";\n".join(upgrade)
+            ddl += ";"
-        if not upgrade[-1].endswith(";"):
+        ddl += "\n"
-            text += ";\n"
+        data.append(ddl)
-    else:
+
-        text += f"{upgrade[0]}"
+    data = [_UPGRADE]
-        if not upgrade[0].endswith(";"):
+    for upgrade_ddl in content.get("upgrade"):
-            text += ";"
+        append_ddl(upgrade_ddl)
-        text += "\n"
     downgrade = content.get("downgrade")
     if downgrade:
-        text += _DOWNGRADE
+        data.append(_DOWNGRADE)
-        if len(downgrade) > 1:
+        for downgrade_ddl in downgrade:
-            text += ";\n".join(downgrade) + ";\n"
+            append_ddl(downgrade_ddl)
-        else:
+    version_file.write_text("".join(data), encoding="utf-8")
-            text += f"{downgrade[0]};\n"
-    version_file.write_text(text, encoding="utf-8")
 
 
 def get_models_describe(app: str) -> Dict:

tests/test_migrate.py

@@ -802,8 +802,7 @@ def test_migrate(mocker: MockerFixture):
         Migrate.diff_models(models_describe, old_models_describe, False)
     Migrate._merge_operators()
     if isinstance(Migrate.ddl, MysqlDDL):
-        expected_upgrade_operators = set(
+        expected_upgrade_operators = {
-            [
             "ALTER TABLE `category` MODIFY COLUMN `name` VARCHAR(200)",
             "ALTER TABLE `category` MODIFY COLUMN `slug` VARCHAR(100) NOT NULL",
             "ALTER TABLE `config` ADD `user_id` INT NOT NULL  COMMENT 'User'",
@@ -812,7 +811,6 @@ def test_migrate(mocker: MockerFixture):
             "ALTER TABLE `config` MODIFY COLUMN `value` JSON NOT NULL",
             "ALTER TABLE `email` ADD `address` VARCHAR(200) NOT NULL",
             "ALTER TABLE `email` DROP COLUMN `user_id`",
-                "CREATE TABLE `email_user` (`email_id` INT NOT NULL REFERENCES `email` (`email_id`) ON DELETE CASCADE,`user_id` INT NOT NULL REFERENCES `user` (`id`) ON DELETE CASCADE) CHARACTER SET utf8mb4",
             "ALTER TABLE `configs` RENAME TO `config`",
             "ALTER TABLE `product` RENAME COLUMN `image` TO `pic`",
             "ALTER TABLE `email` RENAME COLUMN `id` TO `email_id`",
@@ -830,14 +828,13 @@ def test_migrate(mocker: MockerFixture):
             "ALTER TABLE `user` MODIFY COLUMN `is_superuser` BOOL NOT NULL  COMMENT 'Is SuperUser' DEFAULT 0",
             "ALTER TABLE `user` MODIFY COLUMN `longitude` DECIMAL(10,8) NOT NULL",
             "ALTER TABLE `user` ADD UNIQUE INDEX `uid_user_usernam_9987ab` (`username`)",
+            "CREATE TABLE `email_user` (\n    `email_id` INT NOT NULL REFERENCES `email` (`email_id`) ON DELETE CASCADE,\n    `user_id` INT NOT NULL REFERENCES `user` (`id`) ON DELETE CASCADE\n) CHARACTER SET utf8mb4",
             "CREATE TABLE IF NOT EXISTS `newmodel` (\n    `id` INT NOT NULL PRIMARY KEY AUTO_INCREMENT,\n    `name` VARCHAR(50) NOT NULL\n) CHARACTER SET utf8mb4;",
             "ALTER TABLE `category` MODIFY COLUMN `created_at` DATETIME(6) NOT NULL  DEFAULT CURRENT_TIMESTAMP(6)",
             "ALTER TABLE `product` MODIFY COLUMN `body` LONGTEXT NOT NULL",
             "ALTER TABLE `email` MODIFY COLUMN `is_primary` BOOL NOT NULL  DEFAULT 0",
-            ]
+        }
-        )
+        expected_downgrade_operators = {
-        expected_downgrade_operators = set(
-            [
             "ALTER TABLE `category` MODIFY COLUMN `name` VARCHAR(200) NOT NULL",
             "ALTER TABLE `category` MODIFY COLUMN `slug` VARCHAR(200) NOT NULL",
             "ALTER TABLE `config` DROP COLUMN `user_id`",
@@ -868,8 +865,7 @@ def test_migrate(mocker: MockerFixture):
             "ALTER TABLE `user` MODIFY COLUMN `longitude` DECIMAL(12,9) NOT NULL",
             "ALTER TABLE `product` MODIFY COLUMN `body` LONGTEXT NOT NULL",
             "ALTER TABLE `email` MODIFY COLUMN `is_primary` BOOL NOT NULL  DEFAULT 0",
-            ]
+        }
-        )
         assert not set(Migrate.upgrade_operators).symmetric_difference(expected_upgrade_operators)
 
         assert not set(Migrate.downgrade_operators).symmetric_difference(
@@ -877,8 +873,7 @@ def test_migrate(mocker: MockerFixture):
         )
 
     elif isinstance(Migrate.ddl, PostgresDDL):
-        expected_upgrade_operators = set(
+        expected_upgrade_operators = {
-            [
             'ALTER TABLE "category" ALTER COLUMN "name" DROP NOT NULL',
             'ALTER TABLE "category" ALTER COLUMN "slug" TYPE VARCHAR(100) USING "slug"::VARCHAR(100)',
             'ALTER TABLE "category" ALTER COLUMN "created_at" TYPE TIMESTAMPTZ USING "created_at"::TIMESTAMPTZ',
@@ -905,14 +900,12 @@ def test_migrate(mocker: MockerFixture):
             'ALTER TABLE "user" ALTER COLUMN "longitude" TYPE DECIMAL(10,8) USING "longitude"::DECIMAL(10,8)',
             'CREATE INDEX "idx_product_name_869427" ON "product" ("name", "type_db_alias")',
             'CREATE INDEX "idx_email_email_4a1a33" ON "email" ("email")',
-                'CREATE TABLE "email_user" ("email_id" INT NOT NULL REFERENCES "email" ("email_id") ON DELETE CASCADE,"user_id" INT NOT NULL REFERENCES "user" ("id") ON DELETE CASCADE)',
+            'CREATE TABLE "email_user" (\n    "email_id" INT NOT NULL REFERENCES "email" ("email_id") ON DELETE CASCADE,\n    "user_id" INT NOT NULL REFERENCES "user" ("id") ON DELETE CASCADE\n)',
             'CREATE TABLE IF NOT EXISTS "newmodel" (\n    "id" SERIAL NOT NULL PRIMARY KEY,\n    "name" VARCHAR(50) NOT NULL\n);\nCOMMENT ON COLUMN "config"."user_id" IS \'User\';',
             'CREATE UNIQUE INDEX "uid_product_name_869427" ON "product" ("name", "type_db_alias")',
             'CREATE UNIQUE INDEX "uid_user_usernam_9987ab" ON "user" ("username")',
-            ]
+        }
-        )
+        expected_downgrade_operators = {
-        expected_downgrade_operators = set(
-            [
             'ALTER TABLE "category" ALTER COLUMN "name" SET NOT NULL',
             'ALTER TABLE "category" ALTER COLUMN "slug" TYPE VARCHAR(200) USING "slug"::VARCHAR(200)',
             'ALTER TABLE "category" ALTER COLUMN "created_at" TYPE TIMESTAMPTZ USING "created_at"::TIMESTAMPTZ',
@@ -943,8 +936,7 @@ def test_migrate(mocker: MockerFixture):
             'DROP INDEX "uid_product_name_869427"',
             'DROP TABLE IF EXISTS "email_user"',
             'DROP TABLE IF EXISTS "newmodel"',
-            ]
+        }
-        )
         assert not set(Migrate.upgrade_operators).symmetric_difference(expected_upgrade_operators)
         assert not set(Migrate.downgrade_operators).symmetric_difference(
             expected_downgrade_operators

tests/test_utils.py

@@ -0,0 +1,34 @@
+import tempfile
+from pathlib import Path
+
+from aerich.utils import write_version_file
+
+
+def test_write_version_file():
+    content = {
+        "upgrade": [
+            "CREATE TABLE IF NOT EXISTS `newmodel` (\n    `id` INT NOT NULL PRIMARY KEY AUTO_INCREMENT,\n    `name` VARCHAR(50) NOT NULL\n) CHARACTER SET utf8mb4;",
+            "CREATE TABLE `email_user` (\n    `email_id` INT NOT NULL REFERENCES `email` (`email_id`) ON DELETE CASCADE,\n    `user_id` INT NOT NULL REFERENCES `user` (`id`) ON DELETE CASCADE\n) CHARACTER SET utf8mb4",
+        ],
+        "downgrade": [
+            "DROP TABLE IF EXISTS `email_user`",
+            "DROP TABLE IF EXISTS `newmodel`",
+        ],
+    }
+    with tempfile.NamedTemporaryFile(mode="r", delete=True) as f:
+        write_version_file(Path(f.name), content)
+        result = f.read()
+        assert result == (
+            "-- upgrade --\n"
+            "CREATE TABLE IF NOT EXISTS `newmodel` (\n"
+            "    `id` INT NOT NULL PRIMARY KEY AUTO_INCREMENT,\n"
+            "    `name` VARCHAR(50) NOT NULL\n"
+            ") CHARACTER SET utf8mb4;\n"
+            "CREATE TABLE `email_user` (\n"
+            "    `email_id` INT NOT NULL REFERENCES `email` (`email_id`) ON DELETE CASCADE,\n"
+            "    `user_id` INT NOT NULL REFERENCES `user` (`id`) ON DELETE CASCADE\n"
+            ") CHARACTER SET utf8mb4;\n"
+            "-- downgrade --\n"
+            "DROP TABLE IF EXISTS `email_user`;\n"
+            "DROP TABLE IF EXISTS `newmodel`;\n"
+        )