From 48afbf8da27d869a5b2df01374626a1efdbd627b Mon Sep 17 00:00:00 2001 From: Aleis <284336768+analeis@users.noreply.github.com> Date: Tue, 10 Nov 2026 23:28:06 +0530 Subject: [PATCH] feat(database)!: replace postgres with sqlite The bot stores its data for a single guild, and each instance was given its own postgres database to hold it. That cost a database server per instance: a second container to pull and run, credentials to configure, a healthcheck to wait on, and around 45 MiB of resident memory that the bot did not need. The data itself is a handful of rows per guild, so a file is enough. The database is now SQLite, through modernc.org/sqlite. The driver is pure Go, which keeps the build at CGO_ENABLED=0 and the runtime image unchanged. It binds $1-style placeholders by index the way postgres does, so the query text carried over without being rewritten, and it binds them correctly: mattn/go-sqlite3 and ncruces/go-sqlite3 both bind by order of appearance, which would have put the wrong values in the wrong place in the queries that build their argument list. Timestamps are stored as Unix milliseconds in INTEGER columns. SQLite has no timestamp type, and the obvious alternative is silently wrong: the driver writes a time.Time as text while a column default writes an integer, so the column holds two kinds of value and SQLite orders it by kind rather than by instant. Storing integers and letting the driver convert keeps the column and the comparisons exact. The subtleties of that representation are written down in AGENTS.md, since the two ways of getting it wrong are a query that returns rows out of order and one that fails to scan. The schema uses INTEGER PRIMARY KEY AUTOINCREMENT. The AUTOINCREMENT matters: without it SQLite reuses an id after a delete, and the pagination cursor compares (created_at, id), so a reused id would make a page skip rows. The moderation log index ends with id for the same reason, and without it SQLite sorts every page again with a temporary B-tree over the table. A test asserts the query plan, so the sort cannot come back unnoticed. The migration runner hands a file to the driver as one statement instead of splitting it on semicolons. The driver knows the quoting rules, so a semicolon inside a string literal no longer cuts a statement in half. Two indexes that the unique constraints and the composite index already covered are gone. DB_PATH names the file, and DB_BOT_INSTANCE_ID names it when DB_PATH is unset, so instances sharing a directory still get one file each. The instance id is filtered down to characters that are safe in a file name. The image creates the data directory owned by the user it runs as, so a named volume mounted over it is writable without anyone having to fix its permissions by hand. Breaking: DATABASE_URL, DB_HOST, DB_PORT, DB_USER, DB_PASSWORD, DB_NAME and POSTGRES_PASSWORD are gone, replaced by DB_PATH and DB_BOT_INSTANCE_ID. compose.yaml no longer starts a database service. There is no migration path for an existing postgres database. Nothing was deployed against it, so there is nothing to carry over. docs/OPERATIONS/DATABASE.md covers where the file lives, how to back it up, and how to read it. The backup uses VACUUM INTO, because the write-ahead log beside the file holds writes that a plain copy would miss. --- .env.example | 19 +- .gitignore | 3 + AGENTS.md | 60 +++- DOCUMENTATION.md | 4 +- Dockerfile | 10 + README.md | 5 + compose.yaml | 21 +- docs/INDEX.md | 14 +- docs/OPERATIONS/DATABASE.md | 126 +++++++ go.mod | 10 +- go.sum | 50 ++- internal/core/start.go | 7 + internal/db/connection.go | 170 +++++++-- internal/db/connection_internal_test.go | 83 +++++ internal/db/database_test.go | 400 +++++++++++++++++++++ internal/db/guild.go | 15 +- internal/db/migrations.go | 62 ++-- internal/db/migrations/001_init_schema.sql | 29 +- internal/db/migrations_internal_test.go | 90 +++++ internal/db/migrations_test.go | 65 ++++ internal/db/moderation.go | 12 +- internal/db/types.go | 20 ++ internal/db/user.go | 15 +- 23 files changed, 1160 insertions(+), 130 deletions(-) create mode 100644 docs/OPERATIONS/DATABASE.md create mode 100644 internal/db/connection_internal_test.go create mode 100644 internal/db/database_test.go create mode 100644 internal/db/migrations_internal_test.go create mode 100644 internal/db/migrations_test.go diff --git a/.env.example b/.env.example index 94b63a9..55ed330 100644 --- a/.env.example +++ b/.env.example @@ -1,15 +1,18 @@ ENV="staging" BOT_TOKEN="your-bot-token" GUILD_ID= #"123456789012345678" - uncomment to register commands to a single guild for faster dev/test updates -DATABASE_URL= #"postgres://pen_bot:pen_bot_password@postgres:5432/pen_bot?sslmode=disable" -DB_NAME="pen_bot" -DB_HOST="localhost" -DB_PORT="5432" -DB_USER="pen_bot" -DB_PASSWORD="pen_bot_password" + +## DATABASE +# Where the SQLite database file lives. The parent directory is created if it +# does not exist. The container image sets this to /data/pen-bot.db. +DB_PATH="/data/pen-bot.db" +# Distinguishes instances that share one directory, so that two of them can +# run side by side without seeing each other's data. Each instance gets its +# own file. Ignored when DB_PATH is set. DB_BOT_INSTANCE_ID="pen-bot-1" -DB_MAX_OPEN_CONNS="16" +# Size of the connection pool. SQLite serialises writes, so a small pool is +# enough. +DB_MAX_OPEN_CONNS="4" DB_MAX_IDLE_CONNS="4" DB_CONN_MAX_LIFETIME="30m" DB_CACHE_CLEANUP_INTERVAL="15m" -POSTGRES_PASSWORD="pen_bot_password" diff --git a/.gitignore b/.gitignore index 4127805..18457b6 100644 --- a/.gitignore +++ b/.gitignore @@ -36,6 +36,9 @@ Editor/IDE build/ /custom-gcl +# Local SQLite database, written by a bot run outside the container +/data/ + # Sphinx HTML build output, per the command in docs/conf.py and docs.yml /public/ diff --git a/AGENTS.md b/AGENTS.md index 154fa9d..1c32e4c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -37,9 +37,10 @@ uv run sphinx-build -W -b html docs public NilAway from `.custom-gcl.yml`; `nix develop` builds it on shell entry. Re-run `golangci-lint custom` after editing `.custom-gcl.yml` or the pinned version. - **Focused runs:** `go test ./internal/db/...`, `go test -run TestExpiry - ./internal/db/`. Tests are pure unit tests — no Postgres, Discord, or network - needed. Only `internal/db` has tests; everything else reports "no test files". - Unexported-helper tests live in `package db`, public-API tests in + ./internal/db/`. Tests are pure unit tests — no Discord or network needed, and + the database tests need no server either: they open a SQLite file in a + temporary directory. Only `internal/db` has tests; everything else reports "no + test files". Unexported-helper tests live in `package db`, public-API tests in `package db_test`. - `pre-commit run --all-files` is a superset of `ci.yml` (adds staticcheck, gosec, govulncheck). There is no hook for the docs build, so run that by hand. @@ -88,17 +89,52 @@ One root module, `github.com/Neon-Genesis-Linux/pen-bot`, one binary: ## Database +- The database is SQLite, through `modernc.org/sqlite`. The driver is pure Go, so + the build stays `CGO_ENABLED=0` and the distroless image is unchanged. It + depends on `modernc.org/libc`, and upstream asks that the two stay in step: + Go's module resolution will not select a `libc` older than the one + `modernc.org/sqlite` requires, but a newer one can be selected, and an + incompatible one fails to compile rather than misbehaving. +- Timestamps are **Unix milliseconds in an `INTEGER` column**, declared + `DATETIME`. Do not store them as text: the driver writes text while a column + default writes an integer, and SQLite then orders the column by kind rather + than by instant, so rows come back in the wrong order. The DSN parameters + `_time_integer_format=unix_milli` and `_inttotime=1` are what make the + driver read and write that representation. +- The driver converts a timestamp on the way out **only when the column's + declared type is `DATE`, `DATETIME` or `TIMESTAMP`**. An aggregate or an + arithmetic expression over such a column has no declared type, so it comes + back as an integer and scanning it into a `time.Time` fails. Select the + column itself, or convert the integer in SQL. +- The driver reads an integer back as milliseconds only when it is larger than + 1e12, which is September 2001. A timestamp before that is silently read as + seconds. Every timestamp here is written by the running bot, so this cannot + happen; a column holding a historic timestamp would need care. - Migrations are `//go:embed`ed from `internal/db/migrations/` and applied by the running bot at startup (`db.ApplyMigrations`). There is no migration CLI. Add `NNN_name.sql`; the filename is recorded in `schema_migrations` and the file runs in one transaction. -- The runner splits each file on `;`, so no semicolon may appear inside a string - literal, a dollar-quoted body, or a `DO $$` block. Keep migrations to plain - DDL. +- A migration file is handed to the driver as one statement, not split on + semicolons, so a semicolon inside a string literal is safe. Keep migrations + to plain DDL anyway. +- Primary keys are `INTEGER PRIMARY KEY AUTOINCREMENT`. The `AUTOINCREMENT` is + required: without it SQLite reuses an id after a delete, which breaks the + keyset pagination that walks a log by `(created_at, id)`. +- An index that serves a query has to carry the order the query asks for. The + moderation log index ends with `id` because the pagination cursor compares + the pair `(created_at, id)`: without it SQLite sorts every page again with a + temporary B-tree over the whole table. `internal/db/migrations_test.go` asserts + the query plan, so a change that reintroduces the sort fails the build. - `db.GlobalDB()` is filled asynchronously, after `Start` has already returned to - its signal wait (`internal/core/start.go`), and stays nil forever if Postgres is - unreachable — the bot runs happily without a database. Callers must tolerate - nil rather than assume a live handle. + its signal wait (`internal/core/start.go`), and stays nil forever if the + database cannot be opened — the bot runs happily without a database. Callers + must tolerate nil rather than assume a live handle. +- SQLite has one writer. The DSN sets `journal_mode(WAL)`, a `busy_timeout` and + `_txlock=immediate`, and the pool is small. Do not raise the pool size to + raise write throughput: writes are serialised regardless. +- `_txlock=immediate` is load-bearing, not a default. A deferred transaction + that reads and then writes cannot always take the write lock afterwards, and + fails with `SQLITE_BUSY` in a way the busy timeout does not retry. - The cache in `internal/db/cache.go` is a process-local map with TTL, not Redis. ## Configuration @@ -110,9 +146,9 @@ One root module, `github.com/Neon-Genesis-Linux/pen-bot`, one binary: - `ENV=production` selects INFO, anything else DEBUG (`internal/logger`). - `GUILD_ID` is optional: set it to sync commands to one guild (instant) instead of globally (slow to propagate). -- For the database, `DATABASE_URL` wins outright; otherwise `DB_HOST`, `DB_PORT`, - `DB_USER`, `DB_PASSWORD`, `DB_NAME` are composed, and `DB_BOT_INSTANCE_ID` - renames the database to `pen_bot_`. `POSTGRES_PASSWORD` is compose-only. +- `DB_PATH` names the database file. `DB_BOT_INSTANCE_ID` names the file when + `DB_PATH` is unset, so that instances sharing a directory each get their own. + The container image sets `DB_PATH=/data/pen-bot.db` and mounts a volume there. - The compose file is `compose.yaml` (Compose v2); use `docker compose up`, or `docker compose watch` to honour the `develop.watch` rebuild. Keep `.dockerignore` — it is what keeps `.env` and the 52MB `custom-gcl` out of the diff --git a/DOCUMENTATION.md b/DOCUMENTATION.md index df3b366..00114c4 100644 --- a/DOCUMENTATION.md +++ b/DOCUMENTATION.md @@ -210,8 +210,8 @@ func main() { ::: :::{note} -The bot retries its database connection in the background, so it starts and -stays up even when Postgres is not yet reachable. +The bot opens its database in the background, so it starts and stays up even +when the database file cannot be reached. ::: ```` diff --git a/Dockerfile b/Dockerfile index 498ebe9..6f1a785 100644 --- a/Dockerfile +++ b/Dockerfile @@ -13,10 +13,20 @@ RUN --mount=type=cache,target=/root/.cache/go-build \ --mount=type=cache,target=/go/pkg/mod \ go build -trimpath -ldflags="-s -w" -o /output/pen-bot ./cmd/pen-fun +# The database lives on a volume mounted here. The directory is created in +# this stage so that it can be given to the unprivileged user the runtime +# image runs as: Docker seeds a fresh named volume with the ownership of the +# directory it covers, so a volume created from this image is writable by that +# user without the operator having to fix its permissions by hand. +RUN mkdir -p /data && chown 65532:65532 /data + FROM gcr.io/distroless/static-debian13:nonroot WORKDIR /app COPY --from=build /output/pen-bot /app/pen-bot +COPY --from=build --chown=65532:65532 /data /data + +ENV DB_PATH=/data/pen-bot.db CMD ["/app/pen-bot"] diff --git a/README.md b/README.md index 19545c5..1709d60 100644 --- a/README.md +++ b/README.md @@ -73,6 +73,11 @@ go build -o pen-fun ./cmd/pen-fun The `compose.yaml` file handles the build and deployment process with hot-reload for development. +The bot stores its data in a SQLite file, so there is no database server to run +and no credentials to configure. The compose file mounts a volume that holds +the file. See [the database documentation](docs/OPERATIONS/DATABASE.md) for +where the file lives, how to back it up and how to inspect it. + ## Contributing Contributions are welcome! Please follow the established code style and include tests for new features. diff --git a/compose.yaml b/compose.yaml index c270361..ba01f1a 100644 --- a/compose.yaml +++ b/compose.yaml @@ -4,27 +4,12 @@ services: pull_policy: build env_file: - .env - depends_on: - postgres: - condition: service_healthy + volumes: + - bot_data:/data develop: watch: - action: rebuild path: . - postgres: - image: postgres:16-alpine - environment: - POSTGRES_DB: pen_bot - POSTGRES_USER: pen_bot - POSTGRES_PASSWORD: ${POSTGRES_PASSWORD:-pen_bot_password} - volumes: - - postgres_data:/var/lib/postgresql/data - healthcheck: - test: ["CMD-SHELL", "pg_isready -U $${POSTGRES_USER}"] - interval: 10s - timeout: 5s - retries: 5 - volumes: - postgres_data: + bot_data: diff --git a/docs/INDEX.md b/docs/INDEX.md index a3c1b14..57b3b7c 100644 --- a/docs/INDEX.md +++ b/docs/INDEX.md @@ -11,10 +11,11 @@ documented by the comment above its declaration, and `go doc` renders those. ## Contents -Pages are added to the toctree below as they are written. It stays commented out -until those pages exist: an enabled toctree naming a missing document warns -rather than failing, so the build would still go green and ship a broken -navigation tree. Uncomment it once the pages land. +Pages are added to the toctree below as they are written. Only documents that +exist are listed: an enabled toctree naming a missing document warns rather +than failing, so the build would still go green and ship a broken navigation +tree. A page that is not listed is also a warning, so add the entry in the same +change that adds the page. Sphinx treats that warning as an error only under `-W`, which `.github/workflows/docs.yml` passes on every push and pull request. Run it the @@ -27,15 +28,12 @@ uv run sphinx-build -W -b html docs public The output directory is `public/` and is not tracked. - ## Planned documents diff --git a/docs/OPERATIONS/DATABASE.md b/docs/OPERATIONS/DATABASE.md new file mode 100644 index 0000000..cd7177b --- /dev/null +++ b/docs/OPERATIONS/DATABASE.md @@ -0,0 +1,126 @@ +# Database + +The bot stores its data in a SQLite database, in a single file on disk. There is +no database server to run, no credentials to manage and no separate container +in the deployment. + +## Where the file lives + +`DB_PATH` names the file. The directory holding it is created if it does not +exist. + +```bash +DB_PATH=/data/pen-bot.db +``` + +When `DB_PATH` is unset, the file name comes from `DB_BOT_INSTANCE_ID`, which +exists so that several instances can share one directory without seeing each +other's data: + +```bash +DB_BOT_INSTANCE_ID=pen-bot-1 # writes data/pen-bot-pen-bot-1.db +``` + +Characters that are not letters, digits, `-` or `_` are dropped from the +instance id, so the name cannot escape the directory it is in. + +## The container + +The image sets `DB_PATH=/data/pen-bot.db`, and the compose file mounts a +volume there: + +```yaml +services: + app: + volumes: + - bot_data:/data +``` + +The image creates `/data` owned by the unprivileged user the container runs as. +Docker seeds a fresh named volume with the ownership of the directory it +covers, so the volume is writable without any manual permission fixing. + +## The schema + +Three tables hold the bot's data, plus one that records which migrations have +run. + +`guilds` +: One row per guild, holding the guild's settings as a JSON document. + +`users` +: One row per user per guild, holding the user's profile as a JSON document. + +`moderation_logs` +: One row per moderation action, with the moderator, the target, the action + and the reason. + +`schema_migrations` +: One row per applied migration, recording the file name and when it ran. + +Timestamps are stored as Unix milliseconds in an `INTEGER` column, declared +`DATETIME`. Read them by selecting them into a `time.Time`; the driver converts +them. + +The driver converts a timestamp only when the column it came from is declared +`DATE`, `DATETIME` or `TIMESTAMP`. An aggregate over such a column, such as +`MAX(created_at)`, has no declared type and comes back as an integer, so +scanning it into a `time.Time` fails. Select the column itself instead. + +## Migrations + +Migrations are files in `internal/db/migrations/`, named `NNN_name.sql`. The +running bot applies them at startup, before it serves any command, so a +deployment needs no separate migration step. Each file runs in one transaction, +and its name is recorded in `schema_migrations` so it is not applied twice. + +A migration must be plain DDL. The file is handed to the database as one +statement, so a semicolon inside a string literal is safe. + +The name of the file is what records it as applied. Editing a file that a +database has already run changes nothing for that database: it keeps the +schema it has. To change the schema of a database that is already in use, add a +new migration file rather than editing an old one. + +## Backing up + +The database is one file, but while the bot is running it also has a write-ahead +log beside it. Copying only the `.db` file would miss the most recent writes. + +Take a consistent copy with `VACUUM INTO`, which writes a complete snapshot +without stopping the bot: + +```bash +sqlite3 /data/pen-bot.db "VACUUM INTO '/data/backup-$(date +%F).db'" +``` + +Restore by stopping the bot and putting the snapshot back in place: + +```bash +docker compose stop app +cp backup-2026-11-10.db /data/pen-bot.db +rm -f /data/pen-bot.db-wal /data/pen-bot.db-shm +docker compose start app +``` + +The bot applies any pending migrations on the next start. + +## Inspecting the data + +Any SQLite client can open the file, and the `sqlite3` command line tool is +enough for most questions: + +```bash +sqlite3 /data/pen-bot.db ".tables" +sqlite3 /data/pen-bot.db "SELECT * FROM moderation_logs ORDER BY created_at DESC LIMIT 10;" +``` + +Stop the bot first if you intend to write, so that the two do not contend for +the same lock. + +## Concurrency + +SQLite allows one writer at a time. The bot runs with the write-ahead log +enabled, a busy timeout, and a small connection pool, which is enough for the +traffic one guild produces. Writes are serialised no matter how large the pool +is, so raising the pool size will not make them faster. diff --git a/go.mod b/go.mod index 7859301..aed37a2 100644 --- a/go.mod +++ b/go.mod @@ -8,16 +8,24 @@ require ( github.com/disgoorg/omit v1.0.0 github.com/disgoorg/snowflake/v2 v2.0.3 github.com/go-resty/resty/v2 v2.17.2 - github.com/lib/pq v1.12.3 + modernc.org/sqlite v1.62.0 ) require ( github.com/disgoorg/godave v0.3.0 // indirect github.com/disgoorg/json/v2 v2.0.0 // indirect + github.com/dustin/go-humanize v1.1.0 // indirect + github.com/google/uuid v1.6.0 // indirect github.com/gorilla/websocket v1.5.3 // indirect github.com/klauspost/compress v1.20.4 // indirect + github.com/mattn/go-isatty v0.0.24 // indirect + github.com/ncruces/go-strftime v1.1.0 // indirect + github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec // indirect github.com/sasha-s/go-csync v0.0.0-20240107134140-fcbab37b09ad // indirect golang.org/x/crypto v0.58.0 // indirect golang.org/x/net v0.62.0 // indirect golang.org/x/sys v0.50.0 // indirect + modernc.org/libc v1.80.1 // indirect + modernc.org/mathutil v1.7.1 // indirect + modernc.org/memory v1.12.1 // indirect ) diff --git a/go.sum b/go.sum index 5750737..7e5f217 100644 --- a/go.sum +++ b/go.sum @@ -10,27 +10,73 @@ github.com/disgoorg/omit v1.0.0 h1:y0LkVUOyUHT8ZlnhIAeOZEA22UYykeysK8bLJ0SfT78= github.com/disgoorg/omit v1.0.0/go.mod h1:RTmSARkf6PWT/UckwI0bV8XgWkWQoPppaT01rYKLcFQ= github.com/disgoorg/snowflake/v2 v2.0.3 h1:3B+PpFjr7j4ad7oeJu4RlQ+nYOTadsKapJIzgvSI2Ro= github.com/disgoorg/snowflake/v2 v2.0.3/go.mod h1:W6r7NUA7DwfZLwr00km6G4UnZ0zcoLBRufhkFWgAc4c= +github.com/dustin/go-humanize v1.1.0 h1:dbKTrvD0klcbBV/h4AWJdMuZogJACoMlvWIWZ5b2xWg= +github.com/dustin/go-humanize v1.1.0/go.mod h1:hc1CvRkJMsgxqjmjMQF3QNRAZBwY8AXBAzKYoSX9sFI= github.com/go-resty/resty/v2 v2.17.2 h1:FQW5oHYcIlkCNrMD2lloGScxcHJ0gkjshV3qcQAyHQk= github.com/go-resty/resty/v2 v2.17.2/go.mod h1:kCKZ3wWmwJaNc7S29BRtUhJwy7iqmn+2mLtQrOyQlVA= +github.com/google/pprof v0.0.0-20261016150608-1facf8f91b50 h1:PlNWJRxu5APSV0pWqfIsZ0qt9OOGHSWfZQ4zvx+WgWs= +github.com/google/pprof v0.0.0-20261016150608-1facf8f91b50/go.mod h1:yUo1AMMpsPIDFGuwDK8nLcf8+vvAd5l2O6JDZ6hpwrM= +github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= +github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/gorilla/websocket v1.5.3 h1:saDtZ6Pbx/0u+bgYQ3q96pZgCzfhKXGPqt7kZ72aNNg= github.com/gorilla/websocket v1.5.3/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE= +github.com/hashicorp/golang-lru/v2 v2.0.7 h1:a+bsQ5rvGLjzHuww6tVxozPZFVghXaHOwFs4luLUK2k= +github.com/hashicorp/golang-lru/v2 v2.0.7/go.mod h1:QeFd9opnmA6QUJc5vARoKUSoFhyfM2/ZepoAG6RGpeM= github.com/klauspost/compress v1.20.4 h1:dD0v/bvxtjCBqQu6sx1TNWkalEY5lNTIe+HY9w8TeNI= github.com/klauspost/compress v1.20.4/go.mod h1:LUdAzn7YLVvxLpc7y3V1m40wESHTgc1422pwwBSKYuI= -github.com/lib/pq v1.12.3 h1:tTWxr2YLKwIvK90ZXEw8GP7UFHtcbTtty8zsI+YjrfQ= -github.com/lib/pq v1.12.3/go.mod h1:/p+8NSbOcwzAEI7wiMXFlgydTwcgTr3OSKMsD2BitpA= +github.com/mattn/go-isatty v0.0.24 h1:tGZZoVgT/KiqK1c8ocVLeDS8BSWMRd47J3Lbz7vsReI= +github.com/mattn/go-isatty v0.0.24/go.mod h1:nMCL3Zebbrt45jsMDgnfIwz6ydEQApk5oEI3HqDio6A= +github.com/ncruces/go-strftime v1.1.0 h1:vtPmPWMkb539RGV9WpWT9EwJGvrOCKvMNI4obWL47Hk= +github.com/ncruces/go-strftime v1.1.0/go.mod h1:yVaq7iwCN09vHRrKBrrflXT0M9tTMG2KmZ9Fj7xeG/g= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec h1:W09IVJc94icq4NjY3clb7Lk8O1qJ8BdBEF8z0ibU0rE= +github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec/go.mod h1:qqbHyh8v60DhA7CoWK5oRCqLrMHRGoxYCSS9EjAz6Eo= github.com/sasha-s/go-csync v0.0.0-20240107134140-fcbab37b09ad h1:qIQkSlF5vAUHxEmTbaqt1hkJ/t6skqEGYiMag343ucI= github.com/sasha-s/go-csync v0.0.0-20240107134140-fcbab37b09ad/go.mod h1:/pA7k3zsXKdjjAiUhB5CjuKib9KJGCaLvZwtxGC8U0s= github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA= github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= golang.org/x/crypto v0.58.0 h1:COkYLr4k7nDI4r6QtMML2AfzeIswWHAJthLS6K/M/54= golang.org/x/crypto v0.58.0/go.mod h1:Xh+kl5A+M0gMiWDytH2Hhr5Z7WbNCpv/eB/EFgfSk+w= +golang.org/x/mod v0.42.0 h1:ICeL3t2L30ljSPHHxDnR4fqVcBt/zR2zJAb9juavmQg= +golang.org/x/mod v0.42.0/go.mod h1:u5fzIrRInkL8MaO1cbRkUHT64v303YIT5KEqpNr7jRs= golang.org/x/net v0.62.0 h1:18ELalfM5+XNRwzM2nYqXsZQ9ByS1uhj2O/U7+5jqRY= golang.org/x/net v0.62.0/go.mod h1:WgDCOTH8iwtB66T4/GfI94AHo/hhdb7Hah9NDseQfiI= +golang.org/x/sync v0.24.0 h1:iqiDrGoDgyuvAKrhLQthUEzU/R378N78gXbBjxed7oA= +golang.org/x/sync v0.24.0/go.mod h1:sUUOizhqBxiL6pEWpqNLUiaJn1ShEbZ6BBqskPbjZm0= golang.org/x/sys v0.50.0 h1:dkKZEFTEKaLr1sjhs0WiKqoi8Jj5nHbS3Njmx1UoMJs= golang.org/x/sys v0.50.0/go.mod h1:hNLxWAXmnKAxqDtdwIYC4bM9oQPEecfsnNMuSxOs3og= golang.org/x/time v0.12.0 h1:ScB/8o8olJvc+CQPWrK3fPZNfh7qgwCrY0zJmoEQLSE= golang.org/x/time v0.12.0/go.mod h1:CDIdPxbZBQxdj6cxyCIdrNogrJKMJ7pr37NYpMcMDSg= +golang.org/x/tools v0.52.0 h1:ryxg4U63oaUYk7uOLTF37n43f4qgtGvqV4Ioit1BaEs= +golang.org/x/tools v0.52.0/go.mod h1:ItcP5UNaVGG+Yc4eIYmaFLNJ4soEopkx5Y2d4lcEurs= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= +modernc.org/cc/v4 v4.30.6-0.20261101114202-1b9d1a076f6b h1:WE5SLOURcNd8s6uPR/n4WV4weQxxYKi5OpEZkfNVvdE= +modernc.org/cc/v4 v4.30.6-0.20261101114202-1b9d1a076f6b/go.mod h1:OnovgIhbbMXMu1aISnJ0wvVD1KnW+cAUJkIrAWh+kVI= +modernc.org/ccgo/v4 v4.38.1 h1:wdxJVBuFT8JufZnnQyS6Bbz3NOWhViD0xJOGR96Bpd4= +modernc.org/ccgo/v4 v4.38.1/go.mod h1:kOpEkJRbFwBvjUWwn4j1j9B7iFHbl1+orhgTMCDkDBk= +modernc.org/fileutil v1.4.0 h1:j6ZzNTftVS054gi281TyLjHPp6CPHr2KCxEXjEbD6SM= +modernc.org/fileutil v1.4.0/go.mod h1:EqdKFDxiByqxLk8ozOxObDSfcVOv/54xDs/DUHdvCUU= +modernc.org/gc/v2 v2.6.5 h1:nyqdV8q46KvTpZlsw66kWqwXRHdjIlJOhG6kxiV/9xI= +modernc.org/gc/v2 v2.6.5/go.mod h1:YgIahr1ypgfe7chRuJi2gD7DBQiKSLMPgBQe9oIiito= +modernc.org/gc/v3 v3.2.0 h1:/Y3LGDMQaoYf55PebP1qsZ5FDGgAZyG1RYKEHeQzh1o= +modernc.org/gc/v3 v3.2.0/go.mod h1:HFK/6AGESC7Ex+EZJhJ2Gni6cTaYpSMmU/cT9RmlfYY= +modernc.org/goabi0 v0.2.0 h1:HvEowk7LxcPd0eq6mVOAEMai46V+i7Jrj13t4AzuNks= +modernc.org/goabi0 v0.2.0/go.mod h1:CEFRnnJhKvWT1c1JTI3Avm+tgOWbkOu5oPA8eH8LnMI= +modernc.org/libc v1.80.1 h1:ejn7HtTV3GknNKbklqWQ1nvUrfX8mTxs1D+xp/JQwy4= +modernc.org/libc v1.80.1/go.mod h1:pvo25Rs3OJEbDqhUV9jdEGevxeLJrJeDmNid/AcQV1g= +modernc.org/mathutil v1.7.1 h1:GCZVGXdaN8gTqB1Mf/usp1Y/hSqgI2vAGGP4jZMCxOU= +modernc.org/mathutil v1.7.1/go.mod h1:4p5IwJITfppl0G4sUEDtCr4DthTaT47/N3aT6MhfgJg= +modernc.org/memory v1.12.1 h1:nFMiWrpStgZczNl6XI9GnIk/rWhYIyHGUaR04pGbp9g= +modernc.org/memory v1.12.1/go.mod h1:/JP4VbVC+K5sU2wZi9bHoq2MAkCnrt2r98UGeSK7Mjw= +modernc.org/opt v0.2.0 h1:tGyef5ApycA7FSEOMraay9SaTk5zmbx7Tu+cJs4QKZg= +modernc.org/opt v0.2.0/go.mod h1:03fq9lsNfvkYSfxrfUhZCWPk1lm4cq4N+Bh//bEtgns= +modernc.org/sortutil v1.2.1 h1:+xyoGf15mM3NMlPDnFqrteY07klSFxLElE2PVuWIJ7w= +modernc.org/sortutil v1.2.1/go.mod h1:7ZI3a3REbai7gzCLcotuw9AC4VZVpYMjDzETGsSMqJE= +modernc.org/sqlite v1.62.0 h1:gMaCjSM1qpl+iOjiWhR1LYP3zqrYDI+5GYLGt6veP2g= +modernc.org/sqlite v1.62.0/go.mod h1:Jzjh1mt4s0HuQ0KvtB1LIuHiGRmU2D5pgj8dUhW7/SE= +modernc.org/strutil v1.2.1 h1:UneZBkQA+DX2Rp35KcM69cSsNES9ly8mQWD71HKlOA0= +modernc.org/strutil v1.2.1/go.mod h1:EHkiggD70koQxjVdSBM3JKM7k6L0FbGE5eymy9i3B9A= +modernc.org/token v1.1.0 h1:Xl7Ap9dKaEs5kLoOQeQmPWevfnk/DM5qcLcYlA8ys6Y= +modernc.org/token v1.1.0/go.mod h1:UGzOrNV1mAFSEB63lOFHIpNRUVMvYTc6yu1SMY/XTDM= diff --git a/internal/core/start.go b/internal/core/start.go index 417dca1..1c47e30 100644 --- a/internal/core/start.go +++ b/internal/core/start.go @@ -86,6 +86,13 @@ func Start(ctx context.Context, token string, guildIDs []snowflake.ID) error { } } +// connectDBWithRetry opens the database and applies the migrations to it. +// +// The database is a local file, so opening it normally succeeds on the first +// attempt. The retry is for the case where the file is not reachable yet, +// such as a volume that has not been mounted when the container starts. The +// bot stays up while this runs, and runs without a database if it never +// succeeds. func connectDBWithRetry(ctx context.Context) { var dbClient *db.DB var err error diff --git a/internal/db/connection.go b/internal/db/connection.go index 588260f..6d76f52 100644 --- a/internal/db/connection.go +++ b/internal/db/connection.go @@ -4,33 +4,96 @@ import ( "context" "database/sql" "fmt" - "net" "net/url" "os" + "path/filepath" "strconv" + "strings" "sync" "time" - _ "github.com/lib/pq" + _ "modernc.org/sqlite" ) +// driverName is the name modernc.org/sqlite registers itself under. +const driverName = "sqlite" + +// defaultDir is the directory the database file lives in when DB_PATH is not +// set. It is relative to the working directory, so a local run keeps its data +// beside the binary rather than somewhere under the system root. +const defaultDir = "data" + +// defaultInstanceID is the fallback for DB_BOT_INSTANCE_ID when it is unset. +const defaultInstanceID = "default" + +// sqliteParams are the connection parameters that shape how SQLite behaves. +// +// The pragma parameters are applied to every connection the pool opens. +// busy_timeout makes a writer wait for a held lock instead of failing +// immediately, which is what keeps two writers arriving together from +// erroring with SQLITE_BUSY. It is set below the three seconds Discord allows +// an interaction to take, so a contended write still has time to report an +// error to the user. journal_mode and synchronous select the write-ahead log +// and its durability level, and foreign_keys turns on constraint enforcement. +// +// The time parameters make the driver store a time.Time as Unix milliseconds +// in an INTEGER column and read it back as a time.Time. SQLite has no +// timestamp type, and storing timestamps as text sorts them wrongly: Go trims +// trailing zeros from a fractional second, so two representations of the same +// instant can compare as different values. Integers compare exactly. +// +// The driver picks the unit when reading an integer back by its magnitude +// rather than by the declared unit, so a timestamp before September 2001 +// would be read as seconds and would come back as a wrong time rather than +// as an error. Every timestamp here is written by the running bot and so is +// later than that. +// +// _txlock=immediate takes the write lock when a transaction starts rather +// than when it first writes. A deferred transaction that reads before it +// writes cannot always be upgraded to a writer later, and fails with +// SQLITE_BUSY in a way the busy timeout does not retry. +const sqliteParams = "_pragma=busy_timeout(2000)" + + "&_pragma=journal_mode(WAL)" + + "&_pragma=synchronous(NORMAL)" + + "&_pragma=foreign_keys(1)" + + "&_time_integer_format=unix_milli" + + "&_inttotime=1" + + "&_timezone=UTC" + + "&_txlock=immediate" + +// DB is a handle on the bot's SQLite database. type DB struct { *sql.DB } +// Config describes how to open the database. type Config struct { - DSN string - MaxOpenConns int - MaxIdleConns int + // Path is the location of the database file. + Path string + // MaxOpenConns bounds the size of the connection pool. + MaxOpenConns int + // MaxIdleConns bounds the number of pooled connections kept for reuse. + MaxIdleConns int + // ConnMaxLifetime retires a connection after it has been open this long. ConnMaxLifetime time.Duration } +// New opens the database at the configured path, creating the file and its +// parent directory if they do not exist. +// +// The path is resolved against the working directory, so a relative path +// keeps referring to the same file for the lifetime of the process. func New(cfg Config) (*DB, error) { - if cfg.DSN == "" { - return nil, fmt.Errorf("database DSN is required") + if cfg.Path == "" { + return nil, fmt.Errorf("database path is required") } - db, err := sql.Open("postgres", cfg.DSN) + dsn, err := dsn(cfg.Path) + if err != nil { + return nil, err + } + + db, err := sql.Open(driverName, dsn) if err != nil { return nil, err } @@ -56,36 +119,22 @@ func New(cfg Config) (*DB, error) { return &DB{db}, nil } +// NewFromEnv opens the database described by the environment. +// +// DB_PATH selects the file. DB_BOT_INSTANCE_ID separates the instances that +// share one directory: each instance gets its own file, so two of them can +// run side by side without seeing each other's rows. A DB_PATH set in the +// environment wins over the instance id, which is how an operator points a +// single instance at a chosen file. func NewFromEnv() (*DB, error) { - dsn := os.Getenv("DATABASE_URL") - if dsn == "" { - host := getEnv("DB_HOST", "localhost") - port := getEnv("DB_PORT", "5432") - user := getEnv("DB_USER", "pen_bot") - password := os.Getenv("DB_PASSWORD") - dbName := os.Getenv("DB_NAME") - if dbName == "" { - botInstance := os.Getenv("DB_BOT_INSTANCE_ID") - if botInstance == "" { - dbName = "pen_bot" - } else { - dbName = fmt.Sprintf("pen_bot_%s", botInstance) - } - } - - u := &url.URL{ - Scheme: "postgres", - User: url.UserPassword(user, password), - Host: net.JoinHostPort(host, port), - Path: dbName, - } - u.RawQuery = "sslmode=disable" - dsn = u.String() + path := os.Getenv("DB_PATH") + if path == "" { + path = filepath.Join(defaultDir, instanceFileName(getEnv("DB_BOT_INSTANCE_ID", defaultInstanceID))) } cfg := Config{ - DSN: dsn, - MaxOpenConns: parseIntEnv("DB_MAX_OPEN_CONNS", 16), + Path: path, + MaxOpenConns: parseIntEnv("DB_MAX_OPEN_CONNS", 4), MaxIdleConns: parseIntEnv("DB_MAX_IDLE_CONNS", 4), ConnMaxLifetime: parseDurationEnv("DB_CONN_MAX_LIFETIME", 30*time.Minute), } @@ -93,6 +142,54 @@ func NewFromEnv() (*DB, error) { return New(cfg) } +// instanceFileName is the database file name for an instance id. +// +// The id is filtered down to characters that are safe in a file name, so a +// value taken from the environment cannot escape the directory the instances +// share or name a file outside it. +func instanceFileName(instanceID string) string { + safe := strings.Map(func(r rune) rune { + switch { + case r >= 'a' && r <= 'z', r >= 'A' && r <= 'Z', r >= '0' && r <= '9': + return r + case r == '-', r == '_': + return r + default: + return -1 + } + }, instanceID) + + if safe == "" { + safe = defaultInstanceID + } + + return fmt.Sprintf("pen-bot-%s.db", safe) +} + +// dsn builds the connection string for a database file. +// +// The path is made absolute so the connection refers to one file no matter +// what the working directory is, and the parent directory is created because +// SQLite will not create it itself. +func dsn(path string) (string, error) { + abs, err := filepath.Abs(path) + if err != nil { + return "", fmt.Errorf("resolve database path %q: %w", path, err) + } + + if err := os.MkdirAll(filepath.Dir(abs), 0o750); err != nil { + return "", fmt.Errorf("create database directory %q: %w", filepath.Dir(abs), err) + } + + u := url.URL{ + Scheme: "file", + Path: abs, + RawQuery: sqliteParams, + } + + return u.String(), nil +} + func getEnv(key, fallback string) string { value := os.Getenv(key) if value == "" { @@ -130,18 +227,22 @@ var ( globalDBMu sync.RWMutex ) +// SetGlobalDB installs the process-wide database handle. func SetGlobalDB(d *DB) { globalDBMu.Lock() defer globalDBMu.Unlock() globalDB = d } +// GlobalDB returns the process-wide database handle, or nil when no database +// has been opened yet. func GlobalDB() *DB { globalDBMu.RLock() defer globalDBMu.RUnlock() return globalDB } +// CloseDB closes the process-wide database handle, if one is open. func CloseDB() { globalDBMu.Lock() defer globalDBMu.Unlock() @@ -151,6 +252,7 @@ func CloseDB() { } } +// ParseCleanupIntervalEnv returns the interval between cache cleanups. func ParseCleanupIntervalEnv(key string, defaultMinutes int) time.Duration { return parseDurationEnv(key, time.Duration(defaultMinutes)*time.Minute) } diff --git a/internal/db/connection_internal_test.go b/internal/db/connection_internal_test.go new file mode 100644 index 0000000..8634f31 --- /dev/null +++ b/internal/db/connection_internal_test.go @@ -0,0 +1,83 @@ +package db + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestInstanceFileName(t *testing.T) { + const defaultName = "pen-bot-default.db" + + tests := []struct { + name string + instanceID string + want string + }{ + {name: "plain", instanceID: "pen-bot-1", want: "pen-bot-pen-bot-1.db"}, + {name: "empty falls back", instanceID: "", want: defaultName}, + {name: "only unsafe characters falls back", instanceID: "///", want: defaultName}, + {name: "unsafe characters are dropped", instanceID: "my instance/../../evil", want: "pen-bot-myinstanceevil.db"}, + {name: "dots are dropped", instanceID: "..", want: defaultName}, + {name: "underscores are kept", instanceID: "a_b-c1", want: "pen-bot-a_b-c1.db"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := instanceFileName(tt.instanceID) + if got != tt.want { + t.Errorf("got %q, want %q", got, tt.want) + } + if strings.ContainsAny(got, `/\`) { + t.Errorf("got %q, want no path separator", got) + } + if strings.Contains(got, "..") { + t.Errorf("got %q, want no parent directory reference", got) + } + }) + } +} + +// TestDSNResolvesToAnAbsolutePath checks that the connection string names one +// file whatever the working directory is. +func TestDSNResolvesToAnAbsolutePath(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "pen-bot.db") + + got, err := dsn(path) + if err != nil { + t.Fatalf("dsn: %v", err) + } + + if !strings.HasPrefix(got, "file:///") { + t.Errorf("got %q, want an absolute file URI", got) + } + if !strings.Contains(got, "pen-bot.db") { + t.Errorf("got %q, want it to name the database file", got) + } + for _, want := range []string{"_pragma=journal_mode(WAL)", "_time_integer_format=unix_milli", "_inttotime=1", "_txlock=immediate"} { + if !strings.Contains(got, want) { + t.Errorf("got %q, want it to contain %q", got, want) + } + } +} + +// TestDSNCreatesTheParentDirectory checks that the directory holding the +// database is created, because SQLite does not create it itself. +func TestDSNCreatesTheParentDirectory(t *testing.T) { + dir := filepath.Join(t.TempDir(), "nested", "deeper") + path := filepath.Join(dir, "pen-bot.db") + + if _, err := dsn(path); err != nil { + t.Fatalf("dsn: %v", err) + } + + info, err := os.Stat(dir) + if err != nil { + t.Fatalf("stat parent directory: %v", err) + } + if !info.IsDir() { + t.Error("got a file, want a directory") + } +} diff --git a/internal/db/database_test.go b/internal/db/database_test.go new file mode 100644 index 0000000..a1ef516 --- /dev/null +++ b/internal/db/database_test.go @@ -0,0 +1,400 @@ +package db_test + +import ( + "context" + "path/filepath" + "testing" + "time" + + "github.com/Neon-Genesis-Linux/pen-bot/internal/db" +) + +// testDB opens a database in a temporary directory and applies the migrations +// to it. +// +// The file is on disk rather than in memory because SQLite gives each +// connection its own in-memory database, so a pooled handle over ":memory:" +// loses rows written through one connection and read through another. +func testDB(t *testing.T) *db.DB { + t.Helper() + + database, err := db.New(db.Config{Path: filepath.Join(t.TempDir(), "pen-bot.db")}) + if err != nil { + t.Fatalf("open: %v", err) + } + t.Cleanup(func() { _ = database.Close() }) + + if err := db.ApplyMigrations(context.Background(), database); err != nil { + t.Fatalf("apply migrations: %v", err) + } + + return database +} + +func TestGuildUpsert(t *testing.T) { + ctx := context.Background() + database := testDB(t) + + guild, err := db.GetGuild(ctx, database, "g1") + if err != nil { + t.Fatalf("get missing guild: %v", err) + } + if guild != nil { + t.Fatalf("got %+v, want nil for an unknown guild", guild) + } + + if err := db.UpsertGuild(ctx, database, "g1", `{"theme":"dark"}`); err != nil { + t.Fatalf("upsert: %v", err) + } + + guild, err = db.GetGuild(ctx, database, "g1") + if err != nil { + t.Fatalf("get guild: %v", err) + } + if guild == nil { + t.Fatal("got nil, want the stored guild") + } + if guild.GuildID != "g1" { + t.Errorf("got guild id %q, want %q", guild.GuildID, "g1") + } + if string(guild.SettingsJSON) != `{"theme":"dark"}` { + t.Errorf("got settings %s, want %s", guild.SettingsJSON, `{"theme":"dark"}`) + } + if guild.CreatedAt.IsZero() { + t.Error("got zero created_at") + } + + // A second upsert replaces the settings and keeps the same row. + if err := db.UpsertGuild(ctx, database, "g1", `{"theme":"light"}`); err != nil { + t.Fatalf("upsert again: %v", err) + } + guild, err = db.GetGuild(ctx, database, "g1") + if err != nil { + t.Fatalf("get guild again: %v", err) + } + if guild == nil { + t.Fatal("got nil, want the stored guild") + } + if string(guild.SettingsJSON) != `{"theme":"light"}` { + t.Errorf("got settings %s, want %s", guild.SettingsJSON, `{"theme":"light"}`) + } + + if err := db.DeleteGuild(ctx, database, "g1"); err != nil { + t.Fatalf("delete: %v", err) + } + guild, err = db.GetGuild(ctx, database, "g1") + if err != nil { + t.Fatalf("get deleted guild: %v", err) + } + if guild != nil { + t.Fatalf("got %+v, want nil after delete", guild) + } +} + +func TestUserUpsert(t *testing.T) { + ctx := context.Background() + database := testDB(t) + + user, err := db.GetUser(ctx, database, "g1", "u1") + if err != nil { + t.Fatalf("get missing user: %v", err) + } + if user != nil { + t.Fatalf("got %+v, want nil for an unknown user", user) + } + + if err := db.UpsertUser(ctx, database, "g1", "u1", `{"name":"ada"}`); err != nil { + t.Fatalf("upsert: %v", err) + } + + user, err = db.GetUser(ctx, database, "g1", "u1") + if err != nil { + t.Fatalf("get user: %v", err) + } + if user == nil { + t.Fatal("got nil, want the stored user") + } + if user.UserID != "u1" { + t.Errorf("got user id %q, want %q", user.UserID, "u1") + } + if string(user.ProfileJSON) != `{"name":"ada"}` { + t.Errorf("got profile %s, want %s", user.ProfileJSON, `{"name":"ada"}`) + } + + // The same user in another guild is a different row. + if err := db.UpsertUser(ctx, database, "g2", "u1", `{"name":"grace"}`); err != nil { + t.Fatalf("upsert in second guild: %v", err) + } + other, err := db.GetUser(ctx, database, "g2", "u1") + if err != nil { + t.Fatalf("get user in second guild: %v", err) + } + if other == nil { + t.Fatal("got nil, want the stored user in the second guild") + } + if string(other.ProfileJSON) != `{"name":"grace"}` { + t.Errorf("got profile %s, want %s", other.ProfileJSON, `{"name":"grace"}`) + } + + if err := db.DeleteUser(ctx, database, "g1", "u1"); err != nil { + t.Fatalf("delete: %v", err) + } + user, err = db.GetUser(ctx, database, "g1", "u1") + if err != nil { + t.Fatalf("get deleted user: %v", err) + } + if user != nil { + t.Fatalf("got %+v, want nil after delete", user) + } +} + +// TestModerationLogPagination walks the whole history through the cursor and +// checks that every entry is returned exactly once. +func TestModerationLogPagination(t *testing.T) { + ctx := context.Background() + database := testDB(t) + + const entries = 25 + for i := range entries { + if err := db.LogModeration(ctx, database, "g1", "mod", "target", "timeout", "reason"); err != nil { + t.Fatalf("log %d: %v", i, err) + } + // Timestamps are stored to the millisecond, so entries written in + // the same millisecond share a timestamp and are ordered by id. + time.Sleep(2 * time.Millisecond) + } + + seen := make(map[int64]bool) + var cursor *db.ModerationCursor + pages := 0 + for { + logs, err := db.GetModerationLogs(ctx, database, "g1", 10, cursor) + if err != nil { + t.Fatalf("page %d: %v", pages, err) + } + if len(logs) == 0 { + break + } + pages++ + + for _, entry := range logs { + if seen[entry.ID] { + t.Fatalf("id %d returned twice", entry.ID) + } + seen[entry.ID] = true + } + + last := logs[len(logs)-1] + cursor = &db.ModerationCursor{CreatedAt: last.CreatedAt, ID: last.ID} + if pages > entries { + t.Fatal("pagination did not terminate") + } + } + + if len(seen) != entries { + t.Errorf("got %d entries across %d pages, want %d", len(seen), pages, entries) + } +} + +// TestModerationLogPaginationAcrossGuilds checks that the cursor pages within +// one guild and never crosses into another. +func TestModerationLogPaginationAcrossGuilds(t *testing.T) { + ctx := context.Background() + database := testDB(t) + + for range 3 { + if err := db.LogModeration(ctx, database, "g1", "mod", "target", "timeout", "reason"); err != nil { + t.Fatalf("log in first guild: %v", err) + } + } + for range 3 { + if err := db.LogModeration(ctx, database, "g2", "mod", "target", "timeout", "reason"); err != nil { + t.Fatalf("log in second guild: %v", err) + } + } + + logs, err := db.GetModerationLogs(ctx, database, "g1", 20, nil) + if err != nil { + t.Fatalf("get logs: %v", err) + } + if len(logs) != 3 { + t.Fatalf("got %d entries, want 3", len(logs)) + } + for _, entry := range logs { + if entry.GuildID != "g1" { + t.Errorf("got entry for guild %q, want %q", entry.GuildID, "g1") + } + } +} + +// TestTimestampsSurviveRoundTrip checks that a timestamp written by the +// application reads back as the same instant. +// +// The columns hold Unix milliseconds, so a value written by the application +// and a value written by a column default have to be read the same way. +func TestTimestampsSurviveRoundTrip(t *testing.T) { + ctx := context.Background() + database := testDB(t) + + written := time.Now().UTC().Truncate(time.Millisecond) + if err := db.UpsertGuild(ctx, database, "g1", `{}`); err != nil { + t.Fatalf("upsert: %v", err) + } + + guild, err := db.GetGuild(ctx, database, "g1") + if err != nil { + t.Fatalf("get guild: %v", err) + } + if guild == nil { + t.Fatal("got nil, want the stored guild") + } + if !guild.CreatedAt.Equal(written) && guild.CreatedAt.After(written.Add(time.Second)) { + t.Errorf("got created_at %v, want near %v", guild.CreatedAt, written) + } + if guild.CreatedAt.IsZero() { + t.Error("got zero created_at") + } +} + +// TestTimestampsOrderCorrectly checks that timestamps written by the +// application and timestamps written by a column default are ordered by the +// instant they record. +// +// Both are stored as Unix milliseconds. Storing them as text instead would +// mix two kinds of value in one column, because the driver writes text while +// the default writes an integer, and SQLite orders every integer before +// every piece of text whatever the instant each one records. +func TestTimestampsOrderCorrectly(t *testing.T) { + ctx := context.Background() + database := testDB(t) + + // The default on created_at is written by the database, so the row is + // inserted without naming the column. + if _, err := database.ExecContext(ctx, `INSERT INTO guilds (guild_id, settings_json) VALUES ($1, $2)`, "defaulted", `{}`); err != nil { + t.Fatalf("insert with default timestamp: %v", err) + } + + // A row written by the application, recording an earlier instant than + // the one above. + earlier := time.Now().UTC().Add(-time.Hour) + if _, err := database.ExecContext(ctx, `INSERT INTO guilds (guild_id, settings_json, created_at) VALUES ($1, $2, $3)`, "written", `{}`, earlier); err != nil { + t.Fatalf("insert with written timestamp: %v", err) + } + + var ids []string + rows, err := database.QueryContext(ctx, `SELECT guild_id FROM guilds ORDER BY created_at DESC`) + if err != nil { + t.Fatalf("query: %v", err) + } + defer func() { _ = rows.Close() }() + for rows.Next() { + var guildID string + if err := rows.Scan(&guildID); err != nil { + t.Fatalf("scan: %v", err) + } + ids = append(ids, guildID) + } + if err := rows.Err(); err != nil { + t.Fatalf("rows: %v", err) + } + + if len(ids) != 2 || ids[0] != "defaulted" || ids[1] != "written" { + t.Errorf("got order %v, want [defaulted written]", ids) + } +} + +// TestTimestampsShareOneStorageType checks that every timestamp in a column +// holds the same kind of value. +// +// A column that mixes text and integers sorts by kind rather than by +// instant, so the two kinds have to agree for the ordering to mean anything. +func TestTimestampsShareOneStorageType(t *testing.T) { + ctx := context.Background() + database := testDB(t) + + if _, err := database.ExecContext(ctx, `INSERT INTO guilds (guild_id, settings_json) VALUES ($1, $2)`, "defaulted", `{}`); err != nil { + t.Fatalf("insert with default timestamp: %v", err) + } + if _, err := database.ExecContext(ctx, `INSERT INTO guilds (guild_id, settings_json, created_at) VALUES ($1, $2, $3)`, "written", `{}`, time.Now().UTC()); err != nil { + t.Fatalf("insert with written timestamp: %v", err) + } + + rows, err := database.QueryContext(ctx, `SELECT DISTINCT typeof(created_at) FROM guilds`) + if err != nil { + t.Fatalf("query: %v", err) + } + defer func() { _ = rows.Close() }() + + var kinds []string + for rows.Next() { + var kind string + if err := rows.Scan(&kind); err != nil { + t.Fatalf("scan: %v", err) + } + kinds = append(kinds, kind) + } + if err := rows.Err(); err != nil { + t.Fatalf("rows: %v", err) + } + + if len(kinds) != 1 { + t.Fatalf("got timestamp kinds %v, want exactly one", kinds) + } + if kinds[0] != "integer" { + t.Errorf("got timestamp kind %q, want %q", kinds[0], "integer") + } +} + +// TestUpsertKeepsCreatedAtAndAdvancesUpdatedAt checks that replacing a row's +// settings leaves the row's creation time alone and moves its update time +// forward. +func TestUpsertKeepsCreatedAtAndAdvancesUpdatedAt(t *testing.T) { + ctx := context.Background() + database := testDB(t) + + if err := db.UpsertGuild(ctx, database, "g1", `{"v":1}`); err != nil { + t.Fatalf("upsert: %v", err) + } + first, err := db.GetGuild(ctx, database, "g1") + if err != nil { + t.Fatalf("get guild: %v", err) + } + if first == nil { + t.Fatal("got nil, want the stored guild") + } + + time.Sleep(20 * time.Millisecond) + + if err := db.UpsertGuild(ctx, database, "g1", `{"v":2}`); err != nil { + t.Fatalf("upsert again: %v", err) + } + second, err := db.GetGuild(ctx, database, "g1") + if err != nil { + t.Fatalf("get guild again: %v", err) + } + if second == nil { + t.Fatal("got nil, want the stored guild") + } + + if !second.CreatedAt.Equal(first.CreatedAt) { + t.Errorf("got created_at %v, want it unchanged at %v", second.CreatedAt, first.CreatedAt) + } + if !second.UpdatedAt.After(first.UpdatedAt) { + t.Errorf("got updated_at %v, want it later than %v", second.UpdatedAt, first.UpdatedAt) + } +} + +// TestMigrationsAreIdempotent checks that applying the migrations to a +// database that already has them leaves it usable. +func TestMigrationsAreIdempotent(t *testing.T) { + ctx := context.Background() + database := testDB(t) + + if err := db.ApplyMigrations(ctx, database); err != nil { + t.Fatalf("apply migrations again: %v", err) + } + + if err := db.UpsertGuild(ctx, database, "g1", `{}`); err != nil { + t.Fatalf("upsert after reapplying migrations: %v", err) + } +} diff --git a/internal/db/guild.go b/internal/db/guild.go index 4c79929..809e4a9 100644 --- a/internal/db/guild.go +++ b/internal/db/guild.go @@ -4,8 +4,11 @@ import ( "context" "database/sql" "fmt" + "time" ) +// GetGuild returns the stored settings for a guild, or nil when the guild has +// no row yet. func GetGuild(ctx context.Context, db *DB, guildID string) (*Guild, error) { if guildID == "" { return nil, fmt.Errorf("guildID required") @@ -31,20 +34,24 @@ func GetGuild(ctx context.Context, db *DB, guildID string) (*Guild, error) { return &guild, nil } +// UpsertGuild stores the settings for a guild, replacing any that are already +// there. func UpsertGuild(ctx context.Context, db *DB, guildID string, settingsJSON string) error { if guildID == "" { return fmt.Errorf("guildID required") } + now := time.Now().UTC() _, err := db.ExecContext(ctx, ` INSERT INTO guilds (guild_id, settings_json, created_at, updated_at) - VALUES ($1, $2::jsonb, now(), now()) + VALUES ($1, $2, $3, $4) ON CONFLICT (guild_id) DO UPDATE SET - settings_json = EXCLUDED.settings_json, - updated_at = now() - `, guildID, settingsJSON) + settings_json = excluded.settings_json, + updated_at = $4 + `, guildID, settingsJSON, now, now) return err } +// DeleteGuild removes the stored settings for a guild. func DeleteGuild(ctx context.Context, db *DB, guildID string) error { if guildID == "" { return fmt.Errorf("guildID required") diff --git a/internal/db/migrations.go b/internal/db/migrations.go index 67e244f..0aa0649 100644 --- a/internal/db/migrations.go +++ b/internal/db/migrations.go @@ -14,6 +14,14 @@ import ( //go:embed migrations/*.sql var migrationsFS embed.FS +// ApplyMigrations brings the database schema up to date. +// +// Every migration in internal/db/migrations that the database has not run +// yet is applied, in the order their file names sort in. A migration that has +// already run is skipped, so this is safe to call on every start. +// +// A migration that fails leaves the schema as it was and the error says which +// file failed. func ApplyMigrations(ctx context.Context, db *DB) error { if err := ensureSchemaMigrations(ctx, db); err != nil { return err @@ -54,31 +62,41 @@ func ApplyMigrations(ctx context.Context, db *DB) error { continue } - tx, err := db.BeginTx(ctx, nil) - if err != nil { + if err := applyMigration(ctx, db, name, sqlText); err != nil { return err } - defer func() { _ = tx.Rollback() }() - - for i, stmt := range strings.Split(sqlText, ";") { - stmt = strings.TrimSpace(stmt) - if stmt == "" { - continue - } - - if _, err := tx.ExecContext(ctx, stmt); err != nil { - slog.Warn("migration statement failed", "migration", name, "statement", i, "error", err) - return fmt.Errorf("migration %s statement %d failed: %w", name, i, err) - } - } + } - if _, err := tx.ExecContext(ctx, `INSERT INTO schema_migrations(version, applied_at) VALUES($1, $2)`, name, time.Now().UTC()); err != nil { - return fmt.Errorf("failed to record migration %s: %w", name, err) - } + return nil +} - if err := tx.Commit(); err != nil { - return fmt.Errorf("migration %s commit failed: %w", name, err) - } +// applyMigration runs one migration file and records it as applied. +// +// The whole file is handed to the driver as a single statement, rather than +// being split on semicolons. The driver understands the quoting rules of the +// language, so a semicolon inside a string literal does not end the +// statement early, and the error it reports names the text that was wrong +// rather than a position in a list. +// +// The file runs in one transaction, so a failure leaves the schema as it was. +func applyMigration(ctx context.Context, db *DB, name, sqlText string) error { + tx, err := db.BeginTx(ctx, nil) + if err != nil { + return err + } + defer func() { _ = tx.Rollback() }() + + if _, err := tx.ExecContext(ctx, sqlText); err != nil { + slog.Warn("migration failed", "migration", name, "error", err) + return fmt.Errorf("migration %s failed: %w", name, err) + } + + if _, err := tx.ExecContext(ctx, `INSERT INTO schema_migrations(version, applied_at) VALUES($1, $2)`, name, time.Now().UTC()); err != nil { + return fmt.Errorf("failed to record migration %s: %w", name, err) + } + + if err := tx.Commit(); err != nil { + return fmt.Errorf("migration %s commit failed: %w", name, err) } return nil @@ -94,7 +112,7 @@ func ensureSchemaMigrations(ctx context.Context, db *DB) error { _, err = tx.ExecContext(ctx, ` CREATE TABLE IF NOT EXISTS schema_migrations ( version TEXT PRIMARY KEY, - applied_at TIMESTAMPTZ NOT NULL + applied_at DATETIME NOT NULL DEFAULT (CAST(unixepoch('now', 'subsec') * 1000 AS INTEGER)) ) `) if err != nil { diff --git a/internal/db/migrations/001_init_schema.sql b/internal/db/migrations/001_init_schema.sql index 12bf54b..3ee162a 100644 --- a/internal/db/migrations/001_init_schema.sql +++ b/internal/db/migrations/001_init_schema.sql @@ -1,31 +1,34 @@ CREATE TABLE IF NOT EXISTS guilds ( - id BIGINT GENERATED ALWAYS AS IDENTITY PRIMARY KEY, + id INTEGER PRIMARY KEY AUTOINCREMENT, guild_id TEXT NOT NULL UNIQUE, - settings_json JSONB NOT NULL DEFAULT '{}'::jsonb, - created_at TIMESTAMPTZ NOT NULL DEFAULT now(), - updated_at TIMESTAMPTZ NOT NULL DEFAULT now() + settings_json TEXT NOT NULL DEFAULT '{}' CHECK (json_valid(settings_json)), + created_at DATETIME NOT NULL DEFAULT (CAST(unixepoch('now', 'subsec') * 1000 AS INTEGER)), + updated_at DATETIME NOT NULL DEFAULT (CAST(unixepoch('now', 'subsec') * 1000 AS INTEGER)) ); CREATE TABLE IF NOT EXISTS users ( - id BIGINT GENERATED ALWAYS AS IDENTITY PRIMARY KEY, + id INTEGER PRIMARY KEY AUTOINCREMENT, guild_id TEXT NOT NULL, user_id TEXT NOT NULL, - profile_json JSONB NOT NULL DEFAULT '{}'::jsonb, - created_at TIMESTAMPTZ NOT NULL DEFAULT now(), - updated_at TIMESTAMPTZ NOT NULL DEFAULT now(), + profile_json TEXT NOT NULL DEFAULT '{}' CHECK (json_valid(profile_json)), + created_at DATETIME NOT NULL DEFAULT (CAST(unixepoch('now', 'subsec') * 1000 AS INTEGER)), + updated_at DATETIME NOT NULL DEFAULT (CAST(unixepoch('now', 'subsec') * 1000 AS INTEGER)), UNIQUE (guild_id, user_id) ); CREATE TABLE IF NOT EXISTS moderation_logs ( - id BIGINT GENERATED ALWAYS AS IDENTITY PRIMARY KEY, + id INTEGER PRIMARY KEY AUTOINCREMENT, guild_id TEXT NOT NULL, moderator_id TEXT NOT NULL, target_id TEXT NOT NULL, action TEXT NOT NULL, reason TEXT NOT NULL, - created_at TIMESTAMPTZ NOT NULL DEFAULT now() + created_at DATETIME NOT NULL DEFAULT (CAST(unixepoch('now', 'subsec') * 1000 AS INTEGER)) ); -CREATE INDEX IF NOT EXISTS idx_moderation_logs_guild_id ON moderation_logs (guild_id); -CREATE INDEX IF NOT EXISTS idx_moderation_logs_guild_created ON moderation_logs (guild_id, created_at DESC); -CREATE INDEX IF NOT EXISTS idx_users_guild_id ON users (guild_id); \ No newline at end of file +-- The moderation log is read newest first within a guild, so the index carries +-- the sort order. It ends with id because the pagination cursor compares the +-- pair (created_at, id): an index without id leaves SQLite sorting each page +-- again to break the ties, which it does with a temporary B-tree over the +-- whole table. +CREATE INDEX IF NOT EXISTS idx_moderation_logs_guild_created ON moderation_logs (guild_id, created_at DESC, id DESC); diff --git a/internal/db/migrations_internal_test.go b/internal/db/migrations_internal_test.go new file mode 100644 index 0000000..ece07f5 --- /dev/null +++ b/internal/db/migrations_internal_test.go @@ -0,0 +1,90 @@ +package db + +import ( + "context" + "path/filepath" + "testing" +) + +// TestApplyMigrationHandlesSemicolonInLiteral checks that a migration +// containing a semicolon inside a string literal is applied whole. +// +// The file is handed to the driver as one statement rather than being split +// on semicolons, so a semicolon in a literal does not cut the statement in +// half. This is the case a split would get wrong. +func TestApplyMigrationHandlesSemicolonInLiteral(t *testing.T) { + database, err := New(Config{Path: filepath.Join(t.TempDir(), "pen-bot.db")}) + if err != nil { + t.Fatalf("open: %v", err) + } + defer func() { _ = database.Close() }() + + ctx := context.Background() + if err := ensureSchemaMigrations(ctx, database); err != nil { + t.Fatalf("ensure schema migrations: %v", err) + } + + const migration = ` +CREATE TABLE notes ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + body TEXT NOT NULL DEFAULT 'first; second; third' +); +INSERT INTO notes (body) VALUES ('a; b'); +INSERT INTO notes DEFAULT VALUES; +` + + if err := applyMigration(ctx, database, "001_notes.sql", migration); err != nil { + t.Fatalf("apply migration: %v", err) + } + + var body string + if err := database.QueryRowContext(ctx, `SELECT body FROM notes WHERE id = 1`).Scan(&body); err != nil { + t.Fatalf("select: %v", err) + } + if body != "a; b" { + t.Errorf("got body %q, want %q", body, "a; b") + } + + var defaultBody string + if err := database.QueryRowContext(ctx, `SELECT body FROM notes WHERE id = 2`).Scan(&defaultBody); err != nil { + t.Fatalf("select default: %v", err) + } + if defaultBody != "first; second; third" { + t.Errorf("got default %q, want %q", defaultBody, "first; second; third") + } +} + +// TestApplyMigrationRollsBackOnFailure checks that a migration that fails +// leaves the schema as it was. +// +// The whole file runs in one transaction, so a statement that fails undoes +// the statements before it. +func TestApplyMigrationRollsBackOnFailure(t *testing.T) { + database, err := New(Config{Path: filepath.Join(t.TempDir(), "pen-bot.db")}) + if err != nil { + t.Fatalf("open: %v", err) + } + defer func() { _ = database.Close() }() + + ctx := context.Background() + const migration = ` +CREATE TABLE before_failure (id INTEGER PRIMARY KEY); +CREATE TABLE after_failure (id INTEGER PRIMARY KEY, broken INTEGER NOT NULL); +CREATE TABLE never_reached (id INTEGER PRIMARY KEY); +` + + if err := applyMigration(ctx, database, "001_broken.sql", migration); err == nil { + t.Fatal("got nil error, want the migration to fail") + } + + var count int + if err := database.QueryRowContext(ctx, ` + SELECT count(*) FROM sqlite_master + WHERE type = 'table' AND name IN ('before_failure', 'after_failure', 'never_reached') + `).Scan(&count); err != nil { + t.Fatalf("count: %v", err) + } + if count != 0 { + t.Errorf("got %d tables left behind, want 0", count) + } +} diff --git a/internal/db/migrations_test.go b/internal/db/migrations_test.go new file mode 100644 index 0000000..b1fed78 --- /dev/null +++ b/internal/db/migrations_test.go @@ -0,0 +1,65 @@ +package db_test + +import ( + "context" + "strings" + "testing" + "time" + + "github.com/Neon-Genesis-Linux/pen-bot/internal/db" +) + +// TestModerationLogQueryPlan checks that the pagination query reads the +// moderation log in index order rather than sorting each page again. +// +// The cursor compares the pair (created_at, id), so the index has to carry +// both columns to break the ties between entries written in the same +// millisecond. When it does not, SQLite sorts the page with a temporary +// B-tree over the whole table, which grows with the table. +// +// The plan is asserted rather than the timing, because a plan is what decides +// the cost, and it holds however fast the machine is. +func TestModerationLogQueryPlan(t *testing.T) { + ctx := context.Background() + database := testDB(t) + + for range 50 { + if err := db.LogModeration(ctx, database, "g1", "mod", "target", "timeout", "reason"); err != nil { + t.Fatalf("log: %v", err) + } + } + + rows, err := database.QueryContext(ctx, ` + EXPLAIN QUERY PLAN + SELECT id, guild_id, moderator_id, target_id, action, reason, created_at + FROM moderation_logs + WHERE guild_id = $1 AND (created_at, id) < ($2, $3) + ORDER BY created_at DESC, id DESC + LIMIT $4 + `, "g1", time.Now().UTC().Add(time.Hour), int64(1)<<62, 20) + if err != nil { + t.Fatalf("explain: %v", err) + } + defer func() { _ = rows.Close() }() + + var details []string + for rows.Next() { + var id, parent, notUsed int64 + var detail string + if err := rows.Scan(&id, &parent, ¬Used, &detail); err != nil { + t.Fatalf("scan: %v", err) + } + details = append(details, detail) + } + if err := rows.Err(); err != nil { + t.Fatalf("rows: %v", err) + } + + plan := strings.Join(details, " | ") + if strings.Contains(plan, "TEMP B-TREE") { + t.Errorf("got plan %q, want no temporary sort", plan) + } + if !strings.Contains(plan, "idx_moderation_logs_guild_created") { + t.Errorf("got plan %q, want it to use idx_moderation_logs_guild_created", plan) + } +} diff --git a/internal/db/moderation.go b/internal/db/moderation.go index 48f1d9a..526231a 100644 --- a/internal/db/moderation.go +++ b/internal/db/moderation.go @@ -4,19 +4,27 @@ import ( "context" "fmt" "strings" + "time" ) +// LogModeration records a moderation action. func LogModeration(ctx context.Context, db *DB, guildID, moderatorID, targetID, action, reason string) error { if guildID == "" { return fmt.Errorf("guildID required") } _, err := db.ExecContext(ctx, ` INSERT INTO moderation_logs (guild_id, moderator_id, target_id, action, reason, created_at) - VALUES ($1, $2, $3, $4, $5, now()) - `, guildID, moderatorID, targetID, action, reason) + VALUES ($1, $2, $3, $4, $5, $6) + `, guildID, moderatorID, targetID, action, reason, time.Now().UTC()) return err } +// GetModerationLogs returns moderation logs for a guild, newest first. +// +// A cursor pages through the history. The cursor names the position to +// continue from, and the query returns the entries strictly after it, so a +// caller walking the pages sees every entry exactly once even while new +// entries are being written. func GetModerationLogs(ctx context.Context, db *DB, guildID string, limit int, cursor *ModerationCursor) ([]*ModerationLog, error) { if guildID == "" { return nil, fmt.Errorf("guildID required") diff --git a/internal/db/types.go b/internal/db/types.go index 2d845d0..fe6b8c6 100644 --- a/internal/db/types.go +++ b/internal/db/types.go @@ -1,7 +1,18 @@ +// Package db stores the bot's data in a SQLite database. +// +// The data is small and belongs to a single guild, so it lives in a file +// rather than in a database server. The package opens that file, applies the +// migrations that bring its schema up to date, and offers the queries the +// bot runs against it. +// +// Timestamps are stored as Unix milliseconds in INTEGER columns. The driver +// reads and writes that representation, so a caller passes and receives +// time.Time and never handles the integer form. package db import "time" +// Guild holds the settings stored for a guild. type Guild struct { ID int64 GuildID string @@ -10,6 +21,10 @@ type Guild struct { UpdatedAt time.Time } +// User holds the profile stored for a user in a guild. +// +// A user is identified by the guild they are in as well as by their own id, +// so the same user in two guilds is two rows. type User struct { ID int64 GuildID string @@ -19,6 +34,7 @@ type User struct { UpdatedAt time.Time } +// ModerationLog holds one recorded moderation action. type ModerationLog struct { ID int64 GuildID string @@ -29,6 +45,10 @@ type ModerationLog struct { CreatedAt time.Time } +// ModerationCursor names a position in the moderation log. +// +// It is the pair of the timestamp and the id of the last entry a caller has +// seen, and it is used to ask for the entries that come before it. type ModerationCursor struct { CreatedAt time.Time ID int64 diff --git a/internal/db/user.go b/internal/db/user.go index 5f7f75a..e91dcb1 100644 --- a/internal/db/user.go +++ b/internal/db/user.go @@ -4,8 +4,11 @@ import ( "context" "database/sql" "fmt" + "time" ) +// GetUser returns the stored profile for a user in a guild, or nil when the +// user has no row yet. func GetUser(ctx context.Context, db *DB, guildID, userID string) (*User, error) { if guildID == "" { return nil, fmt.Errorf("guildID required") @@ -35,6 +38,8 @@ func GetUser(ctx context.Context, db *DB, guildID, userID string) (*User, error) return &user, nil } +// UpsertUser stores the profile for a user in a guild, replacing any that is +// already there. func UpsertUser(ctx context.Context, db *DB, guildID, userID, profileJSON string) error { if guildID == "" { return fmt.Errorf("guildID required") @@ -42,16 +47,18 @@ func UpsertUser(ctx context.Context, db *DB, guildID, userID, profileJSON string if userID == "" { return fmt.Errorf("userID required") } + now := time.Now().UTC() _, err := db.ExecContext(ctx, ` INSERT INTO users (guild_id, user_id, profile_json, created_at, updated_at) - VALUES ($1, $2, $3::jsonb, now(), now()) + VALUES ($1, $2, $3, $4, $5) ON CONFLICT (guild_id, user_id) DO UPDATE SET - profile_json = EXCLUDED.profile_json, - updated_at = now() - `, guildID, userID, profileJSON) + profile_json = excluded.profile_json, + updated_at = $5 + `, guildID, userID, profileJSON, now, now) return err } +// DeleteUser removes the stored profile for a user in a guild. func DeleteUser(ctx context.Context, db *DB, guildID, userID string) error { if guildID == "" { return fmt.Errorf("guildID required")