Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,10 @@ DATABASE_POOL_ACQUIRE_TIMEOUT_MS=10000 # Timeout to get connection (default:
DATABASE_POOL_IDLE_TIMEOUT_MS=30000 # Close idle connections after (default: 30000ms)
DATABASE_POOL_LEAK_THRESHOLD_MS=60000 # Warn if connection held longer (default: 60000ms)

# Schema synchronization (default: false — migrations own the schema; see #1210)
# Set to true only for temporary dev-only schema synchronization
DATABASE_SYNCHRONIZE=false

# Read replicas (optional)
DATABASE_REPLICA_HOSTS=replica-1.local,replica-2.local,replica-3.local
DATABASE_REPLICA_PORT=5432
Expand Down
35 changes: 17 additions & 18 deletions docs/migrations.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ How to manage database schema changes safely.
The TeachLink backend uses **TypeORM migrations** for schema management. Migration files are standard TypeORM `MigrationInterface` classes located in `src/migrations/`.

There are two mechanisms for schema updates:

1. **TypeORM `synchronize`** (development onlyauto-creates tables from entities)
2. **Explicit migration files** (all environments — controlled, versioned changes)
1. **Explicit migration files** (all environmentscontrolled, versioned changes; default mechanism)
2. **TypeORM `synchronize`** (disabled by default across all environments; opt-in via `DATABASE_SYNCHRONIZE=true`)

---

Expand Down Expand Up @@ -125,16 +125,15 @@ npx typeorm migration:run -d src/config/datasource.ts

## Development mode (synchronize)

In development (`NODE_ENV=development`), TypeORM's `synchronize: true` is enabled. This means:
By default, TypeORM's `synchronize` is **disabled (`false`)** across all environments, including development (`#1210`). Schema changes are managed strictly via migrations to prevent schema drift and avoid dropping migration-managed columns.

If you need temporary schema auto-generation for local prototyping, you can opt in via the environment variable:

- Tables are **auto-created** from entity definitions on server startup
- You do NOT need to run migrations for schema changes during active development
- This is fast for prototyping but provides no version tracking
```bash
DATABASE_SYNCHRONIZE=true pnpm start:dev
```

> **Important:** When `synchronize` is on, running explicit migrations may fail with "relation already exists" because tables are already created. In that case, either:
>
> - Disable synchronize (`NODE_ENV=production` or edit `database.config.ts`)
> - Drop tables first, then run migrations
> **Important:** When `synchronize` is enabled, running explicit migrations may fight the schema or cause columns not declared in entity files to be dropped. For standard development workflows, keep `DATABASE_SYNCHRONIZE=false` and run migrations via `pnpm migrate:run`.

---

Expand Down Expand Up @@ -238,19 +237,19 @@ pnpm build
| `relation already exists` | Table created by `synchronize` or a prior migration | Drop the table or disable `synchronize` |
| `column "X" of relation "Y" already exists` | Duplicate migration | Create a new migration to handle the state |
| `Cannot roll back: later migrations depend` | Dependency chain | Roll back later migrations first |
| `migration:run` returns 404 | Migration endpoints not wired | Check if endpoints exist; use `synchronize` for dev |
| `migration:run` returns 404 | Migration endpoints not wired | Check if endpoints exist; ensure migrations are run |
| Foreign key violation during migration | Data integrity issue | Clean data, then retry |

---

## Environment-specific settings

| Environment | `synchronize` | Migrations |
| ----------- | ---------------- | ------------------------------------- |
| Development | `true` (default) | Optional (synchronize handles schema) |
| Test | `true` | Run before test suite |
| Staging | `false` | Run manually after deployment |
| Production | `false` | Run manually with backup |
| Environment | `synchronize` | Migrations |
| ----------- | ----------------------------------------------------- | -------------------------------- |
| Development | `false` (default; opt-in via `DATABASE_SYNCHRONIZE=true`) | Run migrations (`pnpm migrate:run`) |
| Test | `false` | Run before test suite |
| Staging | `false` | Run after deployment |
| Production | `false` | Run with backup |

---

Expand Down
13 changes: 7 additions & 6 deletions docs/runbook.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,21 +58,21 @@ docker exec -it teachlink-postgres psql -U postgres -d teachlink -c "SELECT curr
# List existing tables
docker exec -it teachlink-postgres psql -U postgres -d teachlink -c "\dt"

# Check if TypeORM synchronize is enabled (development default)
# Check if TypeORM synchronize is enabled (disabled by default; see #1210)
grep "synchronize" src/config/database.config.ts
```

### Fixes

```bash
# Option 1: Restart with synchronize (development only)
# In .env, ensure NODE_ENV=development (enables auto-sync)
# Then restart the server
# Option 1: Run pending migrations
pnpm migrate:run

# Option 2: Drop and recreate the database (development only)
docker compose down
docker volume rm teachlink_backend_postgres-data
docker compose up -d postgres redis
pnpm migrate:run
# Then start the server — tables will be created on startup

# Option 3: Manually create the database
Expand Down Expand Up @@ -140,13 +140,14 @@ docker exec -it teachlink-postgres psql -U postgres -d teachlink -c "\dt" | grep
curl http://localhost:3000/health

# If migration endpoints aren't available:
# The app uses TypeORM's synchronize in development mode
# Just restart the server and tables will auto-create
# Run migrations directly via CLI
pnpm migrate:run

# For a full reset (development only):
docker compose down
docker volume rm teachlink_backend_postgres-data
docker compose up -d postgres redis
pnpm migrate:run
pnpm start:dev
```

Expand Down
8 changes: 6 additions & 2 deletions docs/setup.md
Original file line number Diff line number Diff line change
Expand Up @@ -186,14 +186,18 @@ curl -X POST http://localhost:3000/migrations/run
curl http://localhost:3000/migrations
```

If migration endpoints are not yet wired, you can verify the database schema is set via TypeORM's `synchronize` (enabled in development):
Or run migrations via CLI:

```bash
pnpm migrate:run
```

```bash
# Check that tables were created
docker exec -it teachlink-postgres psql -U postgres -d teachlink -c "\dt"
```

> **Note:** The `getDatabaseConfig()` sets `synchronize: true` in non-production environments, which auto-creates tables from entities. For production, run explicit migrations.
> **Note:** The `getDatabaseConfig()` defaults `synchronize: false` across all environments (`#1210`). Database schema is managed via migrations (`pnpm migrate:run`). Temporary auto-sync can be opted into in development with `DATABASE_SYNCHRONIZE=true`.

---

Expand Down
17 changes: 10 additions & 7 deletions docs/troubleshooting.md
Original file line number Diff line number Diff line change
Expand Up @@ -202,20 +202,17 @@ Common required variables for local development:

### "Migration failed: relation already exists"

**Cause:** A migration is trying to create a table that already exists (often because `synchronize: true` auto-created it first).
**Cause:** A migration is trying to create a table that already exists (for example, if `DATABASE_SYNCHRONIZE=true` was previously enabled or a previous run partially created objects).

**Fix:**

```bash
# Drop the conflicting table and re-run migration
docker exec -it teachlink-postgres psql -U postgres -d teachlink -c "DROP TABLE IF EXISTS <tablename> CASCADE;"
curl -X POST http://localhost:3000/migrations/run
pnpm migrate:run
```

**Prevention:** In development, you can either:

- Use `synchronize: false` and rely entirely on migrations, or
- Accept that `synchronize` handles schema and skip migrations
**Prevention:** Keep `DATABASE_SYNCHRONIZE=false` (default) and manage all schema changes via migrations (`#1210`).

### "Cannot roll back: later migrations depend on this one"

Expand All @@ -235,7 +232,13 @@ curl -X DELETE http://localhost:3000/migrations/reset

**Cause:** The migration HTTP endpoints may not be wired into the application yet.

**Fix:** Verify tables are created via TypeORM's `synchronize` feature (enabled in development). Check directly in PostgreSQL:
**Fix:** Run migrations directly via TypeORM CLI:

```bash
pnpm migrate:run
```

Check tables directly in PostgreSQL:

```bash
docker exec -it teachlink-postgres psql -U postgres -d teachlink -c "\dt"
Expand Down
37 changes: 37 additions & 0 deletions src/config/database.config.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ describe('database config read replicas', () => {
delete process.env.DATABASE_REPLICA_USER;
delete process.env.DATABASE_REPLICA_PASSWORD;
delete process.env.DATABASE_REPLICA_NAME;
delete process.env.DATABASE_SYNCHRONIZE;
delete process.env.TYPEORM_SYNCHRONIZE;
delete process.env.NODE_ENV;
});

afterAll(() => {
Expand Down Expand Up @@ -88,4 +91,38 @@ describe('database config read replicas', () => {
},
]);
});

describe('schema synchronization (synchronize)', () => {
it.each(['development', 'test', 'ci', 'production', undefined])(
'defaults synchronize to false when NODE_ENV is %s',
(nodeEnv) => {
if (nodeEnv !== undefined) {
process.env.NODE_ENV = nodeEnv;
}
const config = getDatabaseConfig() as Record<string, unknown>;
expect(config.synchronize).toBe(false);
},
);

it.each(['true', '1', 'TRUE'])('enables synchronize when DATABASE_SYNCHRONIZE is %s', (val) => {
process.env.DATABASE_SYNCHRONIZE = val;
const config = getDatabaseConfig() as Record<string, unknown>;
expect(config.synchronize).toBe(true);
});

it.each(['true', '1', 'TRUE'])('enables synchronize when TYPEORM_SYNCHRONIZE is %s', (val) => {
process.env.TYPEORM_SYNCHRONIZE = val;
const config = getDatabaseConfig() as Record<string, unknown>;
expect(config.synchronize).toBe(true);
});

it.each(['false', '0', 'random', ''])(
'keeps synchronize false when DATABASE_SYNCHRONIZE is %s',
(val) => {
process.env.DATABASE_SYNCHRONIZE = val;
const config = getDatabaseConfig() as Record<string, unknown>;
expect(config.synchronize).toBe(false);
},
);
});
});
10 changes: 9 additions & 1 deletion src/config/database.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,14 @@ export function getReadReplicaConnections(
return parseReplicaHosts(primary);
}

function resolveSynchronize(): boolean {
const flag = process.env.DATABASE_SYNCHRONIZE ?? process.env.TYPEORM_SYNCHRONIZE;
if (flag === undefined) {
return false;
}
return flag.toLowerCase() === 'true' || flag === '1';
}

/**
* TypeORM connection options driven by DATABASE_* environment variables.
*/
Expand All @@ -78,7 +86,7 @@ export function getDatabaseConfig(): TypeOrmModuleOptions {
const slowQuery = resolveSlowQueryLoggerOptions();
const commonOptions = {
autoLoadEntities: true,
synchronize: process.env.NODE_ENV !== 'production',
synchronize: resolveSynchronize(),
// Drives TypeORM's logQuerySlow hook, consumed by SlowQueryLogger.
maxQueryExecutionTime: slowQuery.slowQueryThresholdMs,
...(slowQuery.enabled ? { logger: new SlowQueryLogger(slowQuery) } : {}),
Expand Down
16 changes: 16 additions & 0 deletions src/config/env.validation.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ const validEnv = {
SENDGRID_SENDER_EMAIL: 'sender@example.com',
SENDGRID_WEBHOOK_TOKEN: 'token',
SESSION_SECRET: 'a-very-secret-session-key',
REGION: 'us-east-1',
REPLICATION_REGIONS: 'us-east-1,us-west-2',
};

function validate(env: Record<string, string | undefined>) {
Expand Down Expand Up @@ -97,4 +99,18 @@ describe('envValidationSchema', () => {
expect(error?.message).toContain('BCRYPT_ROUNDS');
});
});

describe('DATABASE_SYNCHRONIZE validation', () => {
it('defaults DATABASE_SYNCHRONIZE to false when omitted', () => {
const { value, error } = validate(validEnv);
expect(error).toBeUndefined();
expect(value.DATABASE_SYNCHRONIZE).toBe(false);
});

it('accepts boolean values for DATABASE_SYNCHRONIZE', () => {
const { value, error } = validate({ ...validEnv, DATABASE_SYNCHRONIZE: 'true' });
expect(error).toBeUndefined();
expect(value.DATABASE_SYNCHRONIZE).toBe(true);
});
});
});
2 changes: 2 additions & 0 deletions src/config/env.validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ export const envValidationSchema = Joi.object({
DATABASE_POOL_MIN: Joi.number().integer().min(0).default(5),
DATABASE_POOL_ACQUIRE_TIMEOUT_MS: Joi.number().integer().min(1000).default(10000),
DATABASE_POOL_IDLE_TIMEOUT_MS: Joi.number().integer().min(1000).default(30000),
DATABASE_SYNCHRONIZE: Joi.boolean().default(false),
TYPEORM_SYNCHRONIZE: Joi.boolean().default(false),

// Redis Configuration
REDIS_HOST: Joi.string().required(),
Expand Down
Loading