DB refactor for v1.5

[user4752]

closes https://github.com/SocksTheWolf/AntiScamBot/issues/7

A less opinionated database refactor to attempt a one to one conversion of the current raw SQL queries to an ORM library (SQLAlchemy) in order for the data returned from the database to be agnostic to the database engine selected, and allow for clarity on which fields are referenced, i.e. object._namedvalue versus object[_randomindex].

Will also allow for easier future migrations of the database should the project needs grow, or the database engine of choice change, e.g. sqlite to mysql/mariadb or postgres.

Opinions chosen are:

• a migrations table has been added to internally track database migrations and the date which they occurred.
• table names are the pluralized form of the object type class name, i.e. each row in the servers table is a Server object, et cet.
• all fields are now properly typed for safety with sane defaults to allow cleaner object creation in code.
• all fields are now named to more clearly identify the values to be stored in them should an external editor be used for raw access
• all data contains basic historical record of last value updated timestamp for possible audit purposes.
• all fields referencing a discord value have been set to width of 32, as maximum discord id numbers are 18 in length, and maximum textual identifiers (nicknames) are 32 in length.
• fields using datetime stamps are using the database internal function to generate the timestamp for uniformity; thus all timestamps are in UTC ensuring no timezone 'drift' which could cause confusion. This also allows for displaying time in either server location (hardcoded) / client location (possibly dynamic) timezone without need to worry about the source timezone due to said uniformity.
• field names follow generic naming convention with the following 'rules': (object reference) (object qualifier) (qualifier type) (object type). For example:
  • bot_instance_id would be an instance _(qualifiertype) id _(objecttype) for a bot _(objectqualifier)
  • owner_discord_user_id would be the owner (object reference) of a discord _(objectqualifier) user _(qualifiertype) id _(objecttype).
  • The reasons for this is to more easily describe the data inside the column, and more so due to the fact discord snowflakes cannot be decoded, nor is there a function to determine the type of snowflake (user/guild/message id) without actually querying for a result against every single query function via the API to see which one does not error out.

Visualization of the new database schema:

Examples of the new database objects:

Recommendations / To-dos:

• logic checks are currently spread and varying between the calling function, the internals of called function, and the processing of returned data. ideal situation would be functions which directly interface with the database should be privatized to prevent bad data from being passed, and all logic checks should be moved to a public function to which the data is passed, and then returned to the caller.
• there are currently 3 different ways to add a server to the database, AddBotGuilds , SetBotActivationForOwner and ReconcileServers all which handle it in a different way.
  • Ideally SetBotActivationForOwner would only switch activation state
  • With this codebase, activator_discord_user_id cannot be null, thus it is set to the owner of the server on first join, and updated by the interaction.user_id on state change via SetBotActivationForOwner. Ideally, it would initially be set to the person who invited the bot to the server in on_guild_join

Current Caveats:

• do not have a discord to validate all functionality, thus testing has been limited to the database functions only; however all code has been confirmed to execute properly and data modification has been validated.

[user4752]

Example incremental migration which adds a new table to the database

BotDatabaseSchema.py update

 class Report(Base):
    __tablename__ = "reports"

    id = Column(Integer, primary_key=True, autoincrement=True)
    discord_user_id = Column(String(32), unique=True, nullable=False)
    reporter_discord_user_id = Column(String(32), nullable=False)

BotSetup.py update

    def upgrade_version3to4(self) -> bool:
        session = Session(self.DatabaseCon)

        Base.metadata.tables[Report.__tablename__].create(self.DatabaseCon)

        appVersion = Migration(
            database_version = DatabaseMigrator.DATABASE_VERSION
        )

        session.add(appVersion)
        session.execute(text(f"PRAGMA user_version = {DatabaseMigrator.DATABASE_VERSION}"))
        session.commit()

        return True

[user4752]

Example incremental migration which alters an existing table in the database

Note: This is a simplistic example, as Sqlite does NOT have an ALTER COLUMN function so enforcing NOT NULL in the schema is difficult and would require actual data migration via temp tables

BotDatabaseSchema.py update

_thread_discord_messageid is the newly added column incremental from the previous example

 class Report(Base):
    __tablename__ = "reports"

    id = Column(Integer, primary_key=True, autoincrement=True)
    discord_user_id = Column(String(32), unique=True, nullable=False)
    reporter_discord_user_id = Column(String(32), nullable=False)
    thread_discord_message_id = Column(String(32), nullable=False, default=-1)

BotSetup.py update

    def upgrade_version4to5(self) -> bool:
        session = Session(self.DatabaseCon)

        session.execute(text("ALTER TABLE reports ADD COLUMN thread_discord_message_id VARCHAR(32) NOT NULL DEFAULT -1"))

        appVersion = Migration(
            database_version = DatabaseMigrator.DATABASE_VERSION
        )

        session.add(appVersion)
        session.execute(text(f"PRAGMA user_version = {DatabaseMigrator.DATABASE_VERSION}"))
        session.commit()

        return True

[SocksTheWolf]

Thank you for these updates. I did have to move some code around, so I'm going to see if I can merge your changes in without you having to rewrite more. I know continuous development can be rough haha.

I'll look over and then pull locally with my test accounts for testing later!

Thank you very much for this contribution.

[user4752]

Similar to the 1.0 to 1.5 release, if need be I can close, rebase, and issue a new pull request within a day or two.

[SocksTheWolf]

Similar to the 1.0 to 1.5 release, if need be I can close, rebase, and issue a new pull request within a day or two.

The changes are mostly minimal, specifically they are in the retry instance command handling and moving the ban embed code into the base class of the bot instead of in ScamGuard.py

So just some minor updates.

[user4752]

Everything looks good from my end