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")