Merge pull request ckan#8239 from ckan/8238-sync-migrations

autogenerate and sync migrations with models

changes/8238.feature

@@ -0,0 +1 @@
+add ``ckan generate migration --autogenerate`` option, sync models with migrations

ckan/cli/generate.py

@@ -196,7 +196,14 @@ def make_config(output_path: str, include_plugin: list[str]):
 @click.option(u"-m",
               u"--message",
               help=u"Message string to use with `revision`.")
-def migration(plugin: str, message: str):
+@click.option(
+    "--autogenerate",
+    is_flag=True,
+    show_default=True,
+    default=False,
+    help="Populate revision script with candidate migration operations, based"
+         " on comparison of database to model.")
+def migration(plugin: str, message: str, autogenerate: bool):
     """Create new alembic revision for DB migration.
     """
     if not config:
@@ -205,13 +212,15 @@ def migration(plugin: str, message: str):
     alembic_config = CKANAlembicConfig(_resolve_alembic_config(plugin))
     assert alembic_config.config_file_name
     migration_dir = os.path.dirname(alembic_config.config_file_name)
-    alembic_config.set_main_option("sqlalchemy.url", "")
-    alembic_config.set_main_option(u'script_location', migration_dir)
+    alembic_config.set_main_option("sqlalchemy.url", config["sqlalchemy.url"])
+    alembic_config.set_main_option('script_location', migration_dir)
 
     if not os.path.exists(os.path.join(migration_dir, u'script.py.mako')):
         alembic.command.init(alembic_config, migration_dir)
 
-    rev = alembic.command.revision(alembic_config, message)
+    rev = alembic.command.revision(
+        alembic_config, message, autogenerate=autogenerate
+    )
     rev_path = rev.path  # type: ignore
     click.secho(
         f"Revision file created. Now, you need to update it: \n\t{rev_path}",

ckan/migration/env.py

@@ -6,6 +6,7 @@
 # from logging.config import fileConfig
 from ckan.model import init_model
 from ckan.model.meta import metadata
+from ckan.plugins import plugin_loaded
 
 # When auto-generating migration scripts, uncomment these lines to include in
 # the model the revision tables - otherwise Alembic wants to delete them
@@ -32,6 +33,33 @@
 # ... etc.
 
 
+def include_name(name, type_, parent_names):
+    """
+    FIXME: A number of revision tables/indexes exist only in migrations.
+
+    Ignore for now but remove these exceptions once a migration is created
+    to delete them properly or create them in the models as well.
+    """
+    if type_ == 'table':
+        # FIXME: remove everything revision-related
+        if name == 'revision' or name.endswith('_revision'):
+            return False
+
+        if name.endswith('_alembic_version'):
+            # keep migration information from extensions
+            return False
+
+        # tracking and activity tables were created in a core migration
+        if not plugin_loaded('tracking') and name in (
+                'tracking_raw', 'tracking_summary'):
+            return False
+        if not plugin_loaded('activity') and name in (
+                'activity', 'activity_detail'):
+            return False
+
+    return True
+
+
 def run_migrations_offline():
     """Run migrations in 'offline' mode.
 
@@ -46,7 +74,8 @@ def run_migrations_offline():
     """
     url = config.get_main_option(u"sqlalchemy.url")
     context.configure(
-        url=url, target_metadata=target_metadata, literal_binds=True
+        url=url, target_metadata=target_metadata, literal_binds=True,
+        include_name=include_name,
     )
 
     with context.begin_transaction():
@@ -68,7 +97,10 @@ def run_migrations_online():
     connection = connectable.connect()
     init_model(connectable)
 
-    context.configure(connection=connection, target_metadata=target_metadata)
+    context.configure(
+        connection=connection, target_metadata=target_metadata,
+        include_name=include_name,
+    )
 
     with context.begin_transaction():
         context.run_migrations()

ckan/migration/versions/105_4a5e3465beb6_autogenerate_sync.py

@@ -0,0 +1,80 @@
+"""autogenerate sync models with migrations
+
+Revision ID: 4a5e3465beb6
+Revises: 9f33a0280c51
+Create Date: 2024-06-06 20:32:06.936114
+
+"""
+from alembic import op
+import sqlalchemy as sa
+from sqlalchemy.dialects import postgresql
+
+# revision identifiers, used by Alembic.
+revision = '4a5e3465beb6'
+down_revision = '9f33a0280c51'
+branch_labels = None
+depends_on = None
+
+
+def upgrade():
+    # removed feature
+    op.drop_index('idx_rating_id', table_name='rating')
+    op.drop_index('idx_rating_package_id', table_name='rating')
+    op.drop_index('idx_rating_user_id', table_name='rating')
+    op.drop_table('rating')
+    # enforce package/resource relationship
+    op.create_foreign_key(None, 'resource', 'package', ['package_id'], ['id'])
+    # long-forgotten columns
+    op.drop_column('resource', 'webstore_last_updated')
+    op.drop_column('resource', 'webstore_url')
+    # redundant indexes
+    op.drop_index('idx_package_group_group_id', table_name='member')
+    op.drop_index('idx_package_group_pkg_id', table_name='member')
+    op.drop_index('idx_package_group_pkg_id_group_id', table_name='member')
+    op.drop_index('idx_pkg_id', table_name='package')
+    op.drop_index('idx_pkg_name', table_name='package')
+    op.drop_index('idx_pkg_title', table_name='package')
+    op.drop_index('idx_package_tag_tag_id', table_name='package_tag')
+    op.drop_index('term', table_name='term_translation')
+
+
+def downgrade():
+    op.create_index('term', 'term_translation', ['term'], unique=False)
+    op.create_index('idx_package_tag_tag_id', 'package_tag', ['tag_id'],
+                    unique=False)
+    op.create_index('idx_pkg_title', 'package', ['title'], unique=False)
+    op.create_index('idx_pkg_name', 'package', ['name'], unique=False)
+    op.create_index('idx_pkg_id', 'package', ['id'], unique=False)
+    op.create_index('idx_package_group_pkg_id_group_id', 'member',
+                    ['group_id', 'table_id'], unique=False)
+    op.create_index('idx_package_group_pkg_id', 'member', ['table_id'],
+                    unique=False)
+    op.create_index('idx_package_group_group_id', 'member', ['group_id'],
+                    unique=False)
+    op.add_column('resource', sa.Column('webstore_url', sa.TEXT(),
+                  autoincrement=False, nullable=True))
+    op.add_column('resource', sa.Column('webstore_last_updated',
+                  postgresql.TIMESTAMP(), autoincrement=False,
+                  nullable=True))
+    op.drop_constraint(None, 'resource', type_='foreignkey')
+    op.create_table(
+        'rating',
+        sa.Column('id', sa.TEXT(), autoincrement=False, nullable=False),
+        sa.Column('user_id', sa.TEXT(), autoincrement=False, nullable=True),
+        sa.Column('user_ip_address', sa.TEXT(), autoincrement=False,
+                  nullable=True),
+        sa.Column('rating', postgresql.DOUBLE_PRECISION(precision=53),
+                  autoincrement=False, nullable=True),
+        sa.Column('created', postgresql.TIMESTAMP(), autoincrement=False,
+                  nullable=True),
+        sa.Column('package_id', sa.TEXT(), autoincrement=False, nullable=True),
+        sa.ForeignKeyConstraint(['package_id'], ['package.id'],
+                                name='rating_package_id_fkey'),
+        sa.ForeignKeyConstraint(['user_id'], ['user.id'],
+                                name='rating_user_id_fkey'),
+        sa.PrimaryKeyConstraint('id', name='rating_pkey')
+    )
+    op.create_index('idx_rating_user_id', 'rating', ['user_id'], unique=False)
+    op.create_index('idx_rating_package_id', 'rating', ['package_id'],
+                    unique=False)
+    op.create_index('idx_rating_id', 'rating', ['id'], unique=False)

ckan/model/group.py

@@ -7,7 +7,8 @@
 )
 from typing_extensions import Literal, Self
 
-from sqlalchemy import column, orm, types, Column, Table, ForeignKey, or_, and_, text
+from sqlalchemy import (column, orm, types, Column, Table, ForeignKey, or_,
+                        and_, text, Index)
 from sqlalchemy.ext.associationproxy import AssociationProxy
 
 import ckan.model.meta as meta
@@ -26,41 +27,32 @@
 Mapped = orm.Mapped
 
 member_table = Table('member', meta.metadata,
-                     Column('id', types.UnicodeText,
-                            primary_key=True,
-                            default=_types.make_uuid),
-                     Column('table_name', types.UnicodeText,
-                            nullable=False),
-                     Column('table_id', types.UnicodeText,
-                            nullable=False),
-                     Column('capacity', types.UnicodeText,
-                            nullable=False),
-                     Column('group_id', types.UnicodeText,
-                            ForeignKey('group.id')),
-                     Column('state', types.UnicodeText,
-                            default=core.State.ACTIVE),
-                     )
+    Column('id', types.UnicodeText, primary_key=True, default=_types.make_uuid),
+    Column('table_name', types.UnicodeText, nullable=False),
+    Column('table_id', types.UnicodeText, nullable=False),
+    Column('capacity', types.UnicodeText, nullable=False),
+    Column('group_id', types.UnicodeText, ForeignKey('group.id')),
+    Column('state', types.UnicodeText, default=core.State.ACTIVE),
+    Index('idx_group_pkg_id', 'table_id'),
+    Index('idx_extra_grp_id_pkg_id', 'group_id', 'table_id'),
+    Index('idx_package_group_id', 'id'),
+)
 
 
 group_table = Table('group', meta.metadata,
-                    Column('id', types.UnicodeText,
-                           primary_key=True,
-                           default=_types.make_uuid),
-                    Column('name', types.UnicodeText,
-                           nullable=False, unique=True),
-                    Column('title', types.UnicodeText),
-                    Column('type', types.UnicodeText,
-                           nullable=False),
-                    Column('description', types.UnicodeText),
-                    Column('image_url', types.UnicodeText),
-                    Column('created', types.DateTime,
-                           default=datetime.datetime.now),
-                    Column('is_organization', types.Boolean, default=False),
-                    Column('approval_status', types.UnicodeText,
-                           default=u"approved"),
-                    Column('state', types.UnicodeText,
-                           default=core.State.ACTIVE),
-                    )
+    Column('id', types.UnicodeText, primary_key=True, default=_types.make_uuid),
+    Column('name', types.UnicodeText, nullable=False, unique=True),
+    Column('title', types.UnicodeText),
+    Column('type', types.UnicodeText, nullable=False),
+    Column('description', types.UnicodeText),
+    Column('image_url', types.UnicodeText),
+    Column('created', types.DateTime, default=datetime.datetime.now),
+    Column('is_organization', types.Boolean, default=False),
+    Column('approval_status', types.UnicodeText, default=u"approved"),
+    Column('state', types.UnicodeText, default=core.State.ACTIVE),
+    Index('idx_group_id', 'id'),
+    Index('idx_group_name', 'name'),
+)
 
 
 class Member(core.StatefulObjectMixin,

ckan/model/group_extra.py

@@ -1,7 +1,7 @@
 # encoding: utf-8
 
 from typing import Any
-from sqlalchemy import orm, types, Column, Table, ForeignKey
+from sqlalchemy import orm, types, Column, Table, ForeignKey, Index
 from sqlalchemy.ext.associationproxy import association_proxy
 
 
@@ -21,6 +21,7 @@
     Column('key', types.UnicodeText),
     Column('value', types.UnicodeText),
     Column('state', types.UnicodeText, default=core.State.ACTIVE),
+    Index('idx_group_extra_group_id', 'group_id'),
 )
 
 

ckan/model/package.py

@@ -13,7 +13,7 @@
 from typing_extensions import TypeAlias, Self
 
 from sqlalchemy.sql import and_, or_
-from sqlalchemy import orm, types, Column, Table, ForeignKey
+from sqlalchemy import orm, types, Column, Table, ForeignKey, Index
 from sqlalchemy.dialects.postgresql import JSONB
 from sqlalchemy.ext.mutable import MutableDict
 from sqlalchemy.ext.associationproxy import AssociationProxy
@@ -54,27 +54,31 @@
 
 # Our Domain Object Tables
 package_table = Table('package', meta.metadata,
-        Column('id', types.UnicodeText, primary_key=True, default=_types.make_uuid),
-        Column('name', types.Unicode(PACKAGE_NAME_MAX_LENGTH),
-               nullable=False, unique=True),
-        Column('title', types.UnicodeText, doc='remove_if_not_provided'),
-        Column('version', types.Unicode(PACKAGE_VERSION_MAX_LENGTH),
-               doc='remove_if_not_provided'),
-        Column('url', types.UnicodeText, doc='remove_if_not_provided'),
-        Column('author', types.UnicodeText, doc='remove_if_not_provided'),
-        Column('author_email', types.UnicodeText, doc='remove_if_not_provided'),
-        Column('maintainer', types.UnicodeText, doc='remove_if_not_provided'),
-        Column('maintainer_email', types.UnicodeText, doc='remove_if_not_provided'),
-        Column('notes', types.UnicodeText, doc='remove_if_not_provided'),
-        Column('license_id', types.UnicodeText, doc='remove_if_not_provided'),
-        Column('type', types.UnicodeText, default=u'dataset'),
-        Column('owner_org', types.UnicodeText),
-        Column('creator_user_id', types.UnicodeText),
-        Column('metadata_created', types.DateTime, default=datetime.datetime.utcnow),
-        Column('metadata_modified', types.DateTime, default=datetime.datetime.utcnow),
-        Column('private', types.Boolean, default=False),
-        Column('state', types.UnicodeText, default=core.State.ACTIVE),
-        Column('plugin_data', MutableDict.as_mutable(JSONB)),
+    Column('id', types.UnicodeText, primary_key=True, default=_types.make_uuid),
+    Column('name', types.Unicode(PACKAGE_NAME_MAX_LENGTH),
+           nullable=False, unique=True),
+    Column('title', types.UnicodeText, doc='remove_if_not_provided'),
+    Column('version', types.Unicode(PACKAGE_VERSION_MAX_LENGTH),
+           doc='remove_if_not_provided'),
+    Column('url', types.UnicodeText, doc='remove_if_not_provided'),
+    Column('author', types.UnicodeText, doc='remove_if_not_provided'),
+    Column('author_email', types.UnicodeText, doc='remove_if_not_provided'),
+    Column('maintainer', types.UnicodeText, doc='remove_if_not_provided'),
+    Column('maintainer_email', types.UnicodeText, doc='remove_if_not_provided'),
+    Column('notes', types.UnicodeText, doc='remove_if_not_provided'),
+    Column('license_id', types.UnicodeText, doc='remove_if_not_provided'),
+    Column('type', types.UnicodeText, default=u'dataset'),
+    Column('owner_org', types.UnicodeText),
+    Column('creator_user_id', types.UnicodeText),
+    Column('metadata_created', types.DateTime, default=datetime.datetime.utcnow),
+    Column('metadata_modified', types.DateTime, default=datetime.datetime.utcnow),
+    Column('private', types.Boolean, default=False),
+    Column('state', types.UnicodeText, default=core.State.ACTIVE),
+    Column('plugin_data', MutableDict.as_mutable(JSONB)),
+    Index('idx_pkg_sid', 'id', 'state'),
+    Index('idx_pkg_sname', 'name', 'state'),
+    Index('idx_pkg_stitle', 'title', 'state'),
+    Index('idx_package_creator_user_id', 'creator_user_id'),
 )
 
 
@@ -84,7 +88,8 @@
     Column('package_id', ForeignKey('package.id'), primary_key=True),
     Column('user_id', ForeignKey('user.id'), primary_key = True),
     Column('capacity', types.UnicodeText, nullable=False),
-    Column('modified', types.DateTime, default=datetime.datetime.utcnow),
+    Column('modified', types.DateTime, default=datetime.datetime.utcnow,
+           nullable=False),
 )
 
 

ckan/model/package_extra.py

@@ -3,7 +3,7 @@
 
 from typing import Any
 
-from sqlalchemy import orm, types, Column, Table, ForeignKey
+from sqlalchemy import orm, types, Column, Table, ForeignKey, Index
 from sqlalchemy.ext.associationproxy import association_proxy
 
 import ckan.model.meta as meta
@@ -22,6 +22,8 @@
     Column('key', types.UnicodeText),
     Column('value', types.UnicodeText),
     Column('state', types.UnicodeText, default=core.State.ACTIVE),
+    Index('idx_extra_id_pkg_id', 'id', 'package_id'),
+    Index('idx_extra_pkg_id', 'package_id'),
 )
 
 

ckan/model/resource.py

@@ -9,7 +9,7 @@
 from sqlalchemy.ext.orderinglist import ordering_list
 from sqlalchemy import orm
 from ckan.common import config
-from sqlalchemy import types, Column, Table, ForeignKey
+from sqlalchemy import types, Column, Table, ForeignKey, Index
 from typing_extensions import Self
 
 import ckan.model.meta as meta
@@ -35,7 +35,7 @@
     Column('id', types.UnicodeText, primary_key=True,
            default=_types.make_uuid),
     Column('package_id', types.UnicodeText,
-           ForeignKey('package.id')),
+           ForeignKey('package.id'), nullable=False),
     Column('url', types.UnicodeText, nullable=False, doc='remove_if_not_provided'),
     # XXX: format doc='remove_if_not_provided' makes lots of tests fail, fix tests?
     Column('format', types.UnicodeText),
@@ -56,6 +56,9 @@
     Column('url_type', types.UnicodeText),
     Column('extras', _types.JsonDictType),
     Column('state', types.UnicodeText, default=core.State.ACTIVE),
+    Index('idx_package_resource_id', 'id'),
+    Index('idx_package_resource_package_id', 'package_id'),
+    Index('idx_package_resource_url', 'url'),
 )