Merge branch 'db-streamlining' into 'master'

database streamlining

Closes #33

See merge request litecord/litecord!32

.gitlab-ci.yml

@@ -19,7 +19,6 @@ tests:
   script:
     - ls
     - cp config.ci.py config.py
-    - psql -h postgres -U postgres -f schema.sql
     - pipenv run ./manage.py migrate
     - pipenv run ./manage.py setup_tests
     - tox

README.md

@@ -92,9 +92,6 @@ It's recommended to create a separate user for the `litecord` database.
 ```sh
 # Create the PostgreSQL database.
 $ createdb litecord
-
-# Apply the base schema to the database.
-$ psql -f schema.sql litecord
 ```
 
 Copy the `config.example.py` file and edit it to configure your instance (

manage/cmd/migration/command.py

@@ -19,6 +19,8 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import inspect
 import os
+import datetime
+
 from pathlib import Path
 from dataclasses import dataclass
 from collections import namedtuple
@@ -32,6 +34,12 @@ log = Logger(__name__)
 
 Migration = namedtuple('Migration', 'id name path')
 
+# line of change, 4 april 2019, at 1am (gmt+0)
+BREAK = datetime.datetime(2019, 4, 4, 1)
+
+# if a database has those tables, it ran 0_base.sql.
+HAS_BASE = ['users', 'guilds', 'e']
+
 
 @dataclass
 class MigrationContext:
@@ -42,7 +50,8 @@ class MigrationContext:
     @property
     def latest(self):
         """Return the latest migration ID."""
-        return 0 if len(self.scripts) == 0 else max(self.scripts.keys())
+        return 0 if not self.scripts else max(self.scripts.keys())
+
 
 def make_migration_ctx() -> MigrationContext:
     """Create the MigrationContext instance."""
@@ -73,7 +82,6 @@ def make_migration_ctx() -> MigrationContext:
 
 async def _ensure_changelog(app, ctx):
     # make sure we have the migration table up
-
     try:
         await app.db.execute("""
         CREATE TABLE migration_log (
@@ -87,52 +95,123 @@ async def _ensure_changelog(app, ctx):
             PRIMARY KEY (change_num)
         );
         """)
-
-        # if we were able to create the
-        # migration_log table, insert that we are
-        # on the latest version.
-        await app.db.execute("""
-        INSERT INTO migration_log (change_num, description)
-        VALUES ($1, $2)
-        """, ctx.latest, 'migration setup')
     except asyncpg.DuplicateTableError:
         log.debug('existing migration table')
 
+        # NOTE: this is a migration breakage,
+        # only applying to databases that had their first migration
+        # before 4 april 2019 (more on BREAK)
 
-async def apply_migration(app, migration: Migration):
+        # if migration_log is empty, just assume this is new
-    """Apply a single migration."""
+        first = await app.db.fetchval("""
-    migration_sql = migration.path.read_text(encoding='utf-8')
+        SELECT apply_ts FROM migration_log
+        ORDER BY apply_ts ASC
+        LIMIT 1
+        """) or BREAK
+        if first < BREAK:
+            log.info('deleting migration_log due to migration structure change')
+            await app.db.execute("DROP TABLE migration_log")
+            await _ensure_changelog(app, ctx)
 
+
+async def _insert_log(app, migration_id: int, description) -> bool:
     try:
         await app.db.execute("""
         INSERT INTO migration_log (change_num, description)
         VALUES ($1, $2)
-        """, migration.id, f'migration: {migration.name}')
+        """, migration_id, description)
-    except asyncpg.UniqueViolationError:
-        log.warning('already applied {}', migration.id)
-        return
 
+        return True
+    except asyncpg.UniqueViolationError:
+        log.warning('already inserted {}', migration_id)
+        return False
+
+
+async def _delete_log(app, migration_id: int):
+    await app.db.execute("""
+    DELETE FROM migration_log WHERE change_num = $1
+    """, migration_id)
+
+
+async def apply_migration(app, migration: Migration) -> bool:
+    """Apply a single migration.
+
+    Tries to insert it to the migration logs first, and if it exists,
+    skips it.
+
+    If any error happens while migrating, this will rollback the log,
+    by removing it from the logs.
+
+    Returns a boolean signaling if this failed or not.
+    """
+    migration_sql = migration.path.read_text(encoding='utf-8')
+
+    res = await _insert_log(
+        app, migration.id, f'migration: {migration.name}')
+
+    if not res:
+        return False
+
+    try:
         await app.db.execute(migration_sql)
-    log.info('applied {}', migration.id)
+        log.info('applied {} {}', migration.id, migration.name)
+
+        return True
+    except:
+        log.exception('failed to run migration, rollbacking log')
+        await _delete_log(app, migration.id)
+
+        return False
+
+
+async def _check_base(app) -> bool:
+    """Return if the current database has ran the 0_base.sql
+    file."""
+    try:
+        for table in HAS_BASE:
+            await app.db.execute(f"""
+            SELECT * FROM {table} LIMIT 0
+            """)
+    except asyncpg.UndefinedTableError:
+        return False
+
+    return True
 
 
 async def migrate_cmd(app, _args):
     """Main migration command.
 
-    This makes sure the database
+    This makes sure the database is updated, here's the steps:
-    is updated.
+     - create the migration_log table, or recreate it (due to migration
+        changes in 4 april 2019)
+     - check the latest local point in migration_log
+     - check if the database is on the base schema
     """
-
     ctx = make_migration_ctx()
 
+    # ensure there is a migration_log table
     await _ensure_changelog(app, ctx)
 
-    # local point in the changelog
+    # check HAS_BASE tables, and if they exist, implicitly
+    # assume this has the base schema.
+    has_base = await _check_base(app)
+
+    # fetch latest local migration that has been run on this database
     local_change = await app.db.fetchval("""
     SELECT max(change_num)
     FROM migration_log
     """)
 
+    # if base exists, add it to logs, if not, apply (and add to logs)
+    if has_base:
+        await _insert_log(app, 0, 'migration setup (from existing)')
+    else:
+        await apply_migration(app, ctx.scripts[0])
+
+    # after that check the current local_change
+    # and the latest migration to be run
+
+    # if no migrations, then we are on migration 0 (which is base)
     local_change = local_change or 0
     latest_change = ctx.latest
 
@@ -149,7 +228,7 @@ async def migrate_cmd(app, _args):
         migration = ctx.scripts.get(idx)
 
         print('applying', migration.id, migration.name)
-        await apply_migration(app, migration)
+        # await apply_migration(app, migration)
 
 
 def setup(subparser):

manage/cmd/migration/scripts/0_base.sql

@@ -27,18 +27,8 @@ CREATE TABLE IF NOT EXISTS user_conn_apps (
     name text NOT NULL
 );
 
-INSERT INTO user_conn_apps (id, name) VALUES (0, 'Twitch');
+-- there was a chain of INSERTs here with hardcoded names and stuff.
-INSERT INTO user_conn_apps (id, name) VALUES (1, 'Youtube');
+-- removed it because we aren't in the best business of hardcoding.
-INSERT INTO user_conn_apps (id, name) VALUES (2, 'Steam');
-INSERT INTO user_conn_apps (id, name) VALUES (3, 'Reddit');
-INSERT INTO user_conn_apps (id, name) VALUES (4, 'Facebook');
-INSERT INTO user_conn_apps (id, name) VALUES (5, 'Twitter');
-INSERT INTO user_conn_apps (id, name) VALUES (6, 'Spotify');
-INSERT INTO user_conn_apps (id, name) VALUES (7, 'XBOX');
-INSERT INTO user_conn_apps (id, name) VALUES (8, 'Battle.net');
-INSERT INTO user_conn_apps (id, name) VALUES (9, 'Skype');
-INSERT INTO user_conn_apps (id, name) VALUES (10, 'League of Legends');
-
 
 CREATE TABLE IF NOT EXISTS instance_invites (
     code text PRIMARY KEY,
@@ -52,24 +42,6 @@ CREATE TABLE IF NOT EXISTS instance_invites (
 );
 
 
--- main attachments table
-CREATE TABLE IF NOT EXISTS attachments (
-    id bigint PRIMARY KEY,
-
-    -- keeping channel_id and message_id
-    -- make a way "better" attachment url.
-    channel_id bigint REFERENCES channels (id),
-    message_id bigint REFERENCES messages (id),
-
-    filename text NOT NULL,
-    filesize integer,
-
-    image boolean DEFAULT FALSE,
-
-    -- only not null if image=true
-    height integer DEFAULT NULL,
-    width integer DEFAULT NULL
-);
 
 
 CREATE TABLE IF NOT EXISTS icons (
@@ -607,7 +579,10 @@ CREATE TABLE IF NOT EXISTS channel_overwrites (
 -- columns in private keys can't have NULL values,
 -- so instead we use a custom constraint with UNIQUE
 
-ALTER TABLE channel_overwrites ADD CONSTRAINT channel_overwrites_uniq
+ALTER TABLE channel_overwrites
+    DROP CONSTRAINT IF EXISTS channel_overwrites_uniq;
+ALTER TABLE channel_overwrites
+    ADD CONSTRAINT channel_overwrites_uniq
     UNIQUE (channel_id, target_role, target_user);
 
 
@@ -688,7 +663,11 @@ CREATE TABLE IF NOT EXISTS message_reactions (
     emoji_text text
 );
 
-ALTER TABLE message_reactions ADD CONSTRAINT message_reactions_main_uniq
+-- unique constraint over multiple columns instead of a primary key
+ALTER TABLE message_reactions
+    DROP CONSTRAINT IF EXISTS message_reactions_main_uniq;
+ALTER TABLE message_reactions
+    ADD CONSTRAINT message_reactions_main_uniq
     UNIQUE (message_id, user_id, emoji_id, emoji_text);
 
 CREATE TABLE IF NOT EXISTS channel_pins (
@@ -696,3 +675,23 @@ CREATE TABLE IF NOT EXISTS channel_pins (
     message_id bigint REFERENCES messages (id) ON DELETE CASCADE,
     PRIMARY KEY (channel_id, message_id)
 );
+
+
+-- main attachments table
+CREATE TABLE IF NOT EXISTS attachments (
+    id bigint PRIMARY KEY,
+
+    -- keeping channel_id and message_id
+    -- make a way "better" attachment url.
+    channel_id bigint REFERENCES channels (id),
+    message_id bigint REFERENCES messages (id),
+
+    filename text NOT NULL,
+    filesize integer,
+
+    image boolean DEFAULT FALSE,
+
+    -- only not null if image=true
+    height integer DEFAULT NULL,
+    width integer DEFAULT NULL
+);

manage/cmd/migration/scripts/10_add_attachments_table.sql

@@ -1,15 +0,0 @@
-CREATE TABLE IF NOT EXISTS attachments (
-    id bigint PRIMARY KEY,
-
-    channel_id bigint REFERENCES channels (id),
-    message_id bigint REFERENCES messages (id),
-
-    filename text NOT NULL,
-    filesize integer,
-
-    image boolean DEFAULT FALSE,
-
-    -- only not null if image=true
-    height integer DEFAULT NULL,
-    width integer DEFAULT NULL
-);

manage/cmd/migration/scripts/11_voice_regions_servers.sql

@@ -1,37 +0,0 @@
--- voice region data
--- NOTE: do NOT remove any rows. use deprectated=true and
--- DELETE FROM voice_servers instead.
-CREATE TABLE IF NOT EXISTS voice_regions (
-    -- always lowercase
-    id text PRIMARY KEY,
-
-    -- "Russia", "Brazil", "Antartica", etc
-    name text NOT NULL,
-
-    -- we don't have the concept of vip guilds yet, but better
-    -- future proof.
-    vip boolean DEFAULT FALSE,
-
-    deprecated boolean DEFAULT FALSE,
-
-    -- we don't have the concept of custom regions too. we don't have the
-    -- concept of official guilds either, but i'm keeping this in
-    custom boolean DEFAULT FALSE
-);
-
--- voice server pool. when someone wants to connect to voice, we choose
--- a server that is in the same region the guild is too, and choose the one
--- with the best health value
-CREATE TABLE IF NOT EXISTS voice_servers (
-    -- hostname is a reachable url, e.g "brazil2.example.com"
-    hostname text PRIMARY KEY,
-    region_id text REFERENCES voice_regions (id),
-
-    -- health values are more thoroughly defined in the LVSP documentation
-    last_health float default 0.5
-);
-
-
-ALTER TABLE guilds DROP COLUMN IF EXISTS region;
-ALTER TABLE guilds ADD COLUMN
-    region text REFERENCES voice_regions (id);

manage/cmd/migration/scripts/12_remove_features_table.sql

@@ -1,5 +0,0 @@
-DROP TABLE guild_features;
-DROP TABLE features;
-
--- this should do the trick
-ALTER TABLE guilds ADD COLUMN features text[] NOT NULL DEFAULT '{}';

manage/cmd/migration/scripts/13_add_vanity_invites_table.sql

@@ -1,5 +0,0 @@
--- vanity url table, the mapping is 1-1 for guilds and vanity urls
-CREATE TABLE IF NOT EXISTS vanity_invites (
-    guild_id bigint REFERENCES guilds (id) PRIMARY KEY,
-    code text REFERENCES invites (code) ON DELETE CASCADE
-);

manage/cmd/migration/scripts/14_add_guild_description.sql

@@ -1,2 +0,0 @@
-ALTER TABLE guilds ADD COLUMN description text DEFAULT NULL;
-ALTER TABLE guilds ADD COLUMN banner text DEFAULT NULL;

manage/cmd/migration/scripts/15_drop_nullable_webhook_avatar.sql

@@ -1,2 +0,0 @@
-ALTER TABLE webhooks ALTER COLUMN avatar DROP NOT NULL;
-ALTER TABLE webhooks ALTER COLUMN avatar SET DEFAULT NULL;

manage/cmd/migration/scripts/16_messages_webhooks.sql

@@ -1,17 +0,0 @@
--- this is a tricky one. blame discord
-
--- first, remove all messages made by webhooks (safety check)
-DELETE FROM messages WHERE author_id is null;
-
--- delete the column, removing the fkey. no connection anymore.
-ALTER TABLE messages DROP COLUMN webhook_id;
-
--- add a message_webhook_info table. more on that in Storage._inject_author
-CREATE TABLE IF NOT EXISTS message_webhook_info (
-    message_id bigint REFERENCES messages (id) PRIMARY KEY,
-
-    webhook_id bigint,
-    name text DEFAULT '<invalid>',
-    avatar text DEFAULT NULL
-);
-

manage/cmd/migration/scripts/1_message_embed_type.sql

@@ -1,6 +0,0 @@
--- unused tables
-DROP TABLE message_embeds;
-DROP TABLE embeds;
-
-ALTER TABLE messages
-    ADD COLUMN embeds jsonb DEFAULT '[]'

manage/cmd/migration/scripts/2_icons_table.sql

@@ -1,35 +0,0 @@
-
--- new icons table
-CREATE TABLE IF NOT EXISTS icons (
-    scope text NOT NULL,
-    key text,
-    hash text UNIQUE NOT NULL,
-    mime text NOT NULL,
-    PRIMARY KEY (scope, hash, mime)
-);
-
--- dummy attachments table for now.
-CREATE TABLE IF NOT EXISTS attachments (
-    id bigint NOT NULL,
-    PRIMARY KEY (id)
-);
-
--- remove the old columns referencing the files table
-ALTER TABLE users DROP COLUMN avatar;
-ALTER TABLE users ADD COLUMN avatar text REFERENCES icons (hash) DEFAULT NULL;
-
-ALTER TABLE group_dm_channels DROP COLUMN icon;
-ALTER TABLE group_dm_channels ADD COLUMN icon text REFERENCES icons (hash);
-
-ALTER TABLE guild_emoji DROP COLUMN image;
-ALTER TABLE guild_emoji ADD COLUMN image text REFERENCES icons (hash);
-
-ALTER TABLE guilds DROP COLUMN icon;
-ALTER TABLE guilds ADD COLUMN icon text REFERENCES icons (hash) DEFAULT NULL;
-
--- this one is a change from files to the attachments table
-ALTER TABLE message_attachments DROP COLUMN attachment;
-ALTER TABLE guild_emoji ADD COLUMN attachment bigint REFERENCES attachments (id);
-
--- remove files table
-DROP TABLE files;

manage/cmd/migration/scripts/3_drop_constraints_icons_hash.sql

@@ -1,9 +0,0 @@
--- drop main primary key
--- since hash can now be nullable
-ALTER TABLE icons DROP CONSTRAINT "icons_pkey";
-
--- remove not null from hash column
-ALTER TABLE icons ALTER COLUMN hash DROP NOT NULL;
-
--- add new primary key, without hash
-ALTER TABLE icons ADD CONSTRAINT icons_pkey PRIMARY KEY (scope, key);

manage/cmd/migration/scripts/4_add_instance_invites.sql

@@ -1,8 +0,0 @@
-CREATE TABLE IF NOT EXISTS instance_invites (
-    code text PRIMARY KEY,
-
-    created_at timestamp without time zone default (now() at time zone 'utc'),
-
-    uses bigint DEFAULT 0,
-    max_uses bigint DEFAULT -1
-);

manage/cmd/migration/scripts/5_add_messages_guild_id.sql

@@ -1 +0,0 @@
-ALTER TABLE messages ADD COLUMN guild_id bigint REFERENCES guilds (id) ON DELETE CASCADE;

manage/cmd/migration/scripts/6_emoji_require_colon_true.sql

@@ -1,5 +0,0 @@
--- require_colons seems to be true for all custom emoji.
-ALTER TABLE guild_emoji ALTER COLUMN require_colons SET DEFAULT true;
-
--- retroactively update all other emojis
-UPDATE guild_emoji SET require_colons=true;

manage/cmd/migration/scripts/7_text_channels_rate_limit_per_user.sql

@@ -1,2 +0,0 @@
-ALTER TABLE guild_text_channels
-ADD COLUMN rate_limit_per_user bigint DEFAULT 0;

manage/cmd/migration/scripts/8_roles_default_color.sql

@@ -1,9 +0,0 @@
--- update roles.color default to 0
-ALTER TABLE roles
-  ALTER COLUMN color SET DEFAULT 0;
-
--- update all existing guild default roles to
--- color=0
-UPDATE roles
-  SET color = 0
-WHERE roles.id = roles.guild_id;

manage/cmd/migration/scripts/9_nullable_emails.sql

@@ -1,2 +0,0 @@
-ALTER TABLE users ALTER COLUMN email DROP NOT NULL;
-ALTER TABLE users ALTER COLUMN email SET DEFAULT NULL;

manage/main.py

@@ -62,17 +62,22 @@ def main(config):
     cfg = getattr(config, config.MODE)
     app = FakeApp(cfg.__dict__)
 
-    loop.run_until_complete(init_app_db(app))
-    init_app_managers(app)
-
     # initialize argparser
     parser = init_parser()
 
+    loop.run_until_complete(init_app_db(app))
+
     try:
         if len(argv) < 2:
             parser.print_help()
             return
 
+        # only init app managers when we aren't migrating
+        # as the managers require it
+        # and the migrate command also sets the db up
+        if argv[1] != 'migrate':
+            init_app_managers(app)
+
         args = parser.parse_args()
         loop.run_until_complete(args.func(app, args))
     except Exception: