From bdb800058064e04d46b6ede9a9f8404d635a7909 Mon Sep 17 00:00:00 2001 From: apple050620312 Date: Thu, 10 Sep 2026 14:09:01 +0800 Subject: [PATCH] reliability: enforce data integrity and safe startup --- README.md | 4 ++ cogs/setup.py | 8 ++- ...2026_09_10_000000_add_integrity_indexes.py | 58 +++++++++++++++++++ database/models/AFilterModel.py | 16 +++-- database/models/CustomWebsite.py | 7 ++- database/models/DiscordRepresentation.py | 17 ++++-- database/models/Event.py | 36 ++++++++++-- database/models/Guild.py | 7 ++- database/models/Member.py | 20 ++++--- docker-compose.example.yml | 25 ++++++-- docker-entrypoint.sh | 52 +++++++++++------ src/settings.py | 42 +++++++++----- 12 files changed, 231 insertions(+), 61 deletions(-) create mode 100644 database/migrations/2026_09_10_000000_add_integrity_indexes.py diff --git a/README.md b/README.md index 6a9d1ae..3a372a6 100644 --- a/README.md +++ b/README.md @@ -116,6 +116,10 @@ To use it, uncomment the proper lines in your docker-compose: Then, simply run `docker compose up -d`. +Run database migrations once before starting or replacing bot instances. The example Compose file provides a +one-shot `migrate` service and starts the bot only after that service succeeds. Container startup no longer ignores +migration failures. Set `RUN_DATABASE_MIGRATIONS=true` only for a single-instance legacy deployment. + #### Available environment variables | Environment variable | Comment | diff --git a/cogs/setup.py b/cogs/setup.py index 8471c57..0e389ea 100644 --- a/cogs/setup.py +++ b/cogs/setup.py @@ -35,9 +35,15 @@ async def cog_unload(self): if self.topgg_autopost.is_running(): self.topgg_autopost.cancel() + await Event.close() + if utils.session is not None and not utils.session.closed: + await utils.session.close() + utils.session = None + @discore.Cog.listener() async def on_login(self): - utils.session = aiohttp.ClientSession(timeout=aiohttp.ClientTimeout(total=30)) + if utils.session is None or utils.session.closed: + utils.session = aiohttp.ClientSession(timeout=aiohttp.ClientTimeout(total=30)) if discore.config.dev_guild and discore.config.auto_sync: await self.bot.tree.sync(guild=discore.Object(discore.config.dev_guild)) _logger.info("Synced dev guild") diff --git a/database/migrations/2026_09_10_000000_add_integrity_indexes.py b/database/migrations/2026_09_10_000000_add_integrity_indexes.py new file mode 100644 index 0000000..819c54f --- /dev/null +++ b/database/migrations/2026_09_10_000000_add_integrity_indexes.py @@ -0,0 +1,58 @@ +"""AddIntegrityIndexes Migration.""" + +from masoniteorm.migrations import Migration +from masoniteorm.query import QueryBuilder + + +class AddIntegrityIndexes(Migration): + """Deduplicate sparse settings rows and enforce their natural keys.""" + + def _deduplicate(self, table_name: str, columns: list[str]) -> None: + duplicates = ( + QueryBuilder().on(self.connection).table(table_name) + .select(*columns) + .group_by(','.join(columns)) + .having_raw('COUNT(*) > 1') + .get() + ) + for duplicate in duplicates: + query = QueryBuilder().on(self.connection).table(table_name) + for column in columns: + query = query.where(column, duplicate[column]) + rows = query.select('id').order_by('id').get() + keep_id = next(iter(rows))['id'] + + delete_query = QueryBuilder().on(self.connection).table(table_name) + for column in columns: + delete_query = delete_query.where(column, duplicate[column]) + delete_query.where('id', '!=', keep_id).delete() + + def up(self): + self._deduplicate('members', ['user_id', 'guild_id']) + self._deduplicate('custom_websites', ['guild_id', 'domain']) + + with self.schema.table('members') as table: + table.unique( + ['user_id', 'guild_id'], + name='members_user_id_guild_id_unique', + ) + with self.schema.table('custom_websites') as table: + table.unique( + ['guild_id', 'domain'], + name='custom_websites_guild_id_domain_unique', + ) + with self.schema.table('events') as table: + table.string('name', 64).change() + table.index( + ['name', 'created_at'], + name='events_name_created_at_index', + ) + + def down(self): + with self.schema.table('events') as table: + table.drop_index('events_name_created_at_index') + table.text('name').change() + with self.schema.table('custom_websites') as table: + table.drop_unique('custom_websites_guild_id_domain_unique') + with self.schema.table('members') as table: + table.drop_unique('members_user_id_guild_id_unique') diff --git a/database/models/AFilterModel.py b/database/models/AFilterModel.py index ded0fd7..1d10227 100644 --- a/database/models/AFilterModel.py +++ b/database/models/AFilterModel.py @@ -51,11 +51,17 @@ def find_or_create( if guild is None: from database.models.Guild import Guild guild = Guild.find_or_create(d_element.guild, **(guild_kwargs or {})) - return cls.create({ - 'id': d_element.id, - 'guild_id': guild.id, - **kwargs - }).fresh() + try: + return cls.create({ + 'id': d_element.id, + 'guild_id': guild.id, + **kwargs + }).fresh() + except Exception: + element = cls.find(d_element.id) + if element: + return element + raise @classmethod def reset_lists(cls, guild: Guild) -> None: diff --git a/database/models/CustomWebsite.py b/database/models/CustomWebsite.py index 25f0ac6..a52068c 100644 --- a/database/models/CustomWebsite.py +++ b/database/models/CustomWebsite.py @@ -23,5 +23,10 @@ def find_or_create(cls, guild_id, website_id: int, guild_kwargs: dict | None = N guild = guild_id else: guild = Guild.find_or_create(guild_id, **(guild_kwargs or {})) - website = cls.create({'id': website_id, 'guild_id': guild.id, **kwargs}).fresh() + try: + website = cls.create({'id': website_id, 'guild_id': guild.id, **kwargs}).fresh() + except Exception: + website = cls.find(website_id) + if website is None: + raise return website diff --git a/database/models/DiscordRepresentation.py b/database/models/DiscordRepresentation.py index d2d4aa9..8488fb4 100644 --- a/database/models/DiscordRepresentation.py +++ b/database/models/DiscordRepresentation.py @@ -32,7 +32,16 @@ def find_or_create( if element: return element - return cls.create({ - 'id': d_element.id, - **kwargs - }).fresh() + try: + return cls.create({ + 'id': d_element.id, + **kwargs + }).fresh() + except Exception: + # Another message may have inserted the same Discord snowflake + # between the SELECT and INSERT. Only suppress the error if that + # record now exists; unrelated database failures still propagate. + element = cls.find(d_element.id) + if element: + return element + raise diff --git a/database/models/Event.py b/database/models/Event.py index 39e52dc..f8bc7d9 100644 --- a/database/models/Event.py +++ b/database/models/Event.py @@ -4,11 +4,16 @@ from typing import Self import datetime as dt import json +import logging from masoniteorm.models import Model import discore + +_logger = logging.getLogger(__name__) + + class Event(Model): """Event Model""" @@ -48,10 +53,33 @@ async def _flush_loop(cls) -> None: """Flush the buffer every 5 seconds""" while True: await asyncio.sleep(5) - async with cls._lock: - if cls._buffer: - cls.bulk_create(cls._buffer) - cls._buffer.clear() + try: + await cls.flush() + except Exception: + _logger.exception('Failed to flush analytics events') + + @classmethod + async def flush(cls) -> None: + """Flush pending analytics events without clearing failed writes.""" + + async with cls._lock: + if not cls._buffer: + return + cls.bulk_create(cls._buffer) + cls._buffer.clear() + + @classmethod + async def close(cls) -> None: + """Stop the background task and flush all pending analytics batches.""" + + if cls._flush_task is not None: + cls._flush_task.cancel() + await asyncio.gather(cls._flush_task, return_exceptions=True) + cls._flush_task = None + try: + await cls.flush() + except Exception: + _logger.exception('Failed to flush analytics events during shutdown') @classmethod async def buff_cr(cls, *events: dict) -> None: diff --git a/database/models/Guild.py b/database/models/Guild.py index 21ce797..f4eea9c 100644 --- a/database/models/Guild.py +++ b/database/models/Guild.py @@ -95,5 +95,10 @@ def custom_websites(self): def find_or_create(cls, d_guild: discore.Guild, **kwargs): guild = cls.find(d_guild.id) if guild is None: - guild = cls.create({'id': d_guild.id, **kwargs}).fresh() + try: + guild = cls.create({'id': d_guild.id, **kwargs}).fresh() + except Exception: + guild = cls.find(d_guild.id) + if guild is None: + raise return guild diff --git a/database/models/Member.py b/database/models/Member.py index 2f7c867..ef04559 100644 --- a/database/models/Member.py +++ b/database/models/Member.py @@ -31,13 +31,19 @@ def find_or_create( if member: return member - return cls.create({ - 'user_id': d_member.id, - 'guild_id': guild.id, - 'on_deny_list': True if d_member.bot else False, - 'bot': d_member.bot, - **kwargs - }).fresh() + try: + return cls.create({ + 'user_id': d_member.id, + 'guild_id': guild.id, + 'on_deny_list': True if d_member.bot else False, + 'bot': d_member.bot, + **kwargs + }).fresh() + except Exception: + member = cls.where('user_id', d_member.id).where('guild_id', guild.id).first() + if member: + return member + raise @classmethod def find_get_enabled(cls, d_member: discore.Member, guild: Guild | None = None) -> bool: diff --git a/docker-compose.example.yml b/docker-compose.example.yml index 70cf47b..943cc2c 100644 --- a/docker-compose.example.yml +++ b/docker-compose.example.yml @@ -3,7 +3,10 @@ services: image: kyrela/fixtweetbot:latest restart: unless-stopped depends_on: - - db + db: + condition: service_healthy + migrate: + condition: service_completed_successfully # You do NOT need to repeat these configuration values in the override file. They are automatically supplied to the bot. environment: # Should be equal to the name of the database container. @@ -21,7 +24,22 @@ services: # - DEV_GUILD=your_discord_dev_guild_id # uncomment and create file if you want to override any default settings # volumes: - # - ./override.config.yml:/usr/local/app/override.config.yml:ro + # - ./override.config.yml:/usr/local/app/override.config.yml:ro + + migrate: + image: kyrela/fixtweetbot:latest + restart: "no" + depends_on: + db: + condition: service_healthy + environment: + - DATABASE_HOST=db + - DATABASE_PORT=3306 + - DATABASE_NAME=fixtweetbot + - DATABASE_USER=fixtweetbot + - DATABASE_PASSWORD=changeme123 + - DISCORD_TOKEN= + command: ["masonite-orm", "migrate", "-C", "database/config.py", "-d", "database/migrations"] db: image: mariadb:12.0 @@ -41,9 +59,6 @@ services: timeout: 5s retries: 5 start_period: 30s - ports: - - "3306:3306" - volumes: mysql_data: driver: local diff --git a/docker-entrypoint.sh b/docker-entrypoint.sh index 6ce0133..ac4073d 100644 --- a/docker-entrypoint.sh +++ b/docker-entrypoint.sh @@ -1,34 +1,48 @@ #!/bin/sh set -e -__config=" -database: - host: $DATABASE_HOST - port: $DATABASE_PORT - user: $DATABASE_USER - driver: $DATABASE_DRIVER - password: $DATABASE_PASSWORD - database: $DATABASE_NAME - -token: $DISCORD_TOKEN" - -if [ -n "$DEV_GUILD" ]; then - __config=" -$__config -dev_guild: $DEV_GUILD" -fi - -echo "$__config" > /usr/local/app/docker.config.yml +python - <<'PY' +import json +import os +from pathlib import Path + +config = { + 'database': { + 'host': os.environ['DATABASE_HOST'], + 'port': int(os.environ['DATABASE_PORT']), + 'user': os.environ['DATABASE_USER'], + 'driver': os.environ['DATABASE_DRIVER'], + 'password': os.environ['DATABASE_PASSWORD'], + 'database': os.environ['DATABASE_NAME'], + }, + 'token': os.environ.get('DISCORD_TOKEN', ''), +} +if os.environ.get('DEV_GUILD'): + config['dev_guild'] = int(os.environ['DEV_GUILD']) + +path = Path('/usr/local/app/docker.config.yml') +path.write_text(json.dumps(config), encoding='utf-8') +path.chmod(0o600) +PY echo -n "Waiting for database.." +waited=0 +wait_timeout="${DATABASE_WAIT_TIMEOUT:-120}" while ! nc -z $DATABASE_HOST $DATABASE_PORT 2>/dev/null; do echo -n "." sleep 1 + waited=$((waited + 1)) + if [ "$waited" -ge "$wait_timeout" ]; then + echo " database wait timed out" + exit 1 + fi done echo -e \\n"Database ready" -masonite-orm migrate -C database/config.py -d database/migrations || echo "Migration failed but continuing..." +if [ "${RUN_DATABASE_MIGRATIONS:-false}" = "true" ]; then + masonite-orm migrate -C database/config.py -d database/migrations +fi exec "$@" diff --git a/src/settings.py b/src/settings.py index 0305f54..347fbc0 100644 --- a/src/settings.py +++ b/src/settings.py @@ -1550,21 +1550,35 @@ def clean_domain(domain: str) -> str: t('settings.custom_websites.modal.error.length_domain', max=61), ephemeral=True, delete_after=10) return - if self.website: - old_website = self.website - self.website.update({ - 'name': name_field, + try: + if self.website: + old_website = self.website + self.website.update({ + 'name': name_field, + 'domain': domain_field, + 'fix_domain': fix_domain_field + }) + self.setting.custom_websites._items.remove(old_website) + else: + self.website = CustomWebsite.create( + guild_id=interaction.guild.id, + name=name_field, + domain=domain_field, + fix_domain=fix_domain_field + ) + except Exception: + existing = CustomWebsite.where({ + 'guild_id': interaction.guild.id, 'domain': domain_field, - 'fix_domain': fix_domain_field - }) - self.setting.custom_websites._items.remove(old_website) - else: - self.website = CustomWebsite.create( - guild_id=interaction.guild.id, - name=name_field, - domain=domain_field, - fix_domain=fix_domain_field - ) + }).first() + if existing: + await interaction.response.send_message( + t('settings.custom_websites.modal.error.exists'), + ephemeral=True, + delete_after=10, + ) + return + raise self.setting.custom_websites._items.append(self.website) self.setting.selected = self.website await self.setting.view.refresh(interaction)