Skip to content

Persist the SQLite database instead of losing it on every restart - #918

Open
devin-ai-integration[bot] wants to merge 1 commit into
bsmitches/new-vuln-v3from
devin/1786538775-persistent-sqlite
Open

devin-ai-integration[bot] wants to merge 1 commit into
bsmitches/new-vuln-v3from
devin/1786538775-persistent-sqlite

Conversation

@devin-ai-integration

Copy link
Copy Markdown
Contributor

Summary

The app ran on :memory: in dev and on a different copy of init.js in Docker, so all data died on restart and the two schemas had silently drifted. This makes the real init.js the single source of truth, gives it DATABASE_PATH support, and adds a volume so the file actually survives.

The drift was a latent outage. docker/overrides/database/init.js was copied over backend/src/database/init.js at image build time, and its clients table had no department/email columns (and its users table no password_hash) — columns routes/clients.js and password auth both use. CREATE TABLE IF NOT EXISTS masked it, and in-memory DBs died before it mattered; the first persisted run would have failed with no such column: department. Deleting the override eliminates the class of bug, not just this instance.

// backend/src/database/init.js
-db = new sqlite3.Database(':memory:', ...)
+const dbPath = process.env.DATABASE_PATH || ':memory:';
+if (dbPath !== ':memory:') fs.mkdirSync(path.dirname(dbPath), { recursive: true });
+db = new sqlite3.Database(dbPath, ...)

 database.serialize(() => {
+  database.run('PRAGMA foreign_keys = ON');          // dev never enforced cascades before
+  if (fileBacked) {
+    database.run('PRAGMA journal_mode = WAL');       // concurrent reads
+    database.run('PRAGMA busy_timeout = 5000');      // kills SQLITE_BUSY under load
+  }

WAL/busy_timeout are deliberately file-only (meaningless for :memory:); foreign_keys is now on everywhere, which previously only the Docker override did. The in-memory connect log message is unchanged because __tests__/database/init.test.js asserts it verbatim.

Persistence itself is infra: new root docker-compose.yml mounts named volume timesheet-data at /app/data (without it, DATABASE_PATH buys nothing since /app/data dies with the container) and requires JWT_SECRET from the environment via ${JWT_SECRET:?...} rather than baking one in.

docker/overrides/server.js is untouched — it carries real production-only concerns (static frontend serving, CSP), unlike the DB override which existed only to swap :memory: for a path.

Verification

  • backend — 8 suites / 172 tests pass; frontend lint passes.
  • End-to-end persistence without Docker: ran the backend against a temp DATABASE_PATH, created user + client + work entry over HTTP, restarted the process, confirmed the rows (including department/email) came back.
  • docker compose config resolves; build context matches the Dockerfile's COPY paths.

Not included (deliberately)

Still needed before this is truly "productionized": a migration strategy (CREATE TABLE IF NOT EXISTS cannot alter a column once data is real), backups (sqlite3 .backup, never cp a live WAL DB), and a decision on SQLite-on-a-volume vs Postgres — the volume approach pins you to one container, so no horizontal scaling or zero-downtime deploys.

Link to Devin session: https://partner-workshops.devinenterprise.com/sessions/098a84db1e184eed97edde7d65e000e3
Requested by: @bsmitches

@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants