690326:2212 Fixing Refactor ADR-019 Naming convention uuid #08
This commit is contained in:
@@ -0,0 +1,215 @@
|
||||
---
|
||||
title: Hybrid Identifier Strategy (ADR-019)
|
||||
impact: CRITICAL
|
||||
impactDescription: Use INT PK internally + UUID for public API per project ADR-019
|
||||
tags: database, uuid, identifier, adr-019, api-design, typeorm
|
||||
---
|
||||
|
||||
## Hybrid Identifier Strategy (ADR-019)
|
||||
|
||||
**This project follows ADR-019: INT Primary Key (internal) + UUIDv7 (public API)**
|
||||
|
||||
Unlike standard practices that use UUID as the primary key, this project uses a **hybrid approach** optimized for MariaDB performance and API consistency.
|
||||
|
||||
### The Strategy
|
||||
|
||||
| Layer | Field | Type | Usage |
|
||||
|-------|-------|------|-------|
|
||||
| **Database PK** | `id` | `INT AUTO_INCREMENT` | Internal foreign keys only |
|
||||
| **Public API** | `uuid` | `MariaDB UUID` (native) | External references, URLs |
|
||||
| **DTO Input** | `xxxUuid` | `string` | Accept UUID in create/update |
|
||||
| **DTO Output** | `id` | `string` | API returns UUID as `id` via `@Expose` |
|
||||
|
||||
### Why Hybrid IDs?
|
||||
|
||||
- **Performance**: INT PK is faster for joins and indexing than UUID
|
||||
- **Security**: Internal IDs never exposed in API (enumerable IDs are a risk)
|
||||
- **Compatibility**: UUID works well with distributed systems and external integrations
|
||||
- **MariaDB Native**: Uses MariaDB's native UUID type (stored as BINARY(16), auto-converts to string)
|
||||
|
||||
### Entity Definition
|
||||
|
||||
```typescript
|
||||
import { Entity, PrimaryGeneratedColumn, Column, Index } from 'typeorm';
|
||||
import { Exclude, Expose } from 'class-transformer';
|
||||
|
||||
@Entity('contracts')
|
||||
export class Contract {
|
||||
@PrimaryGeneratedColumn()
|
||||
@Exclude() // Never expose in API response
|
||||
id: number; // Internal INT PK - used for FK relationships
|
||||
|
||||
@Column({ type: 'uuid', unique: true })
|
||||
@Expose({ name: 'id' }) // Exposed as 'id' in API
|
||||
uuid: string; // Public UUIDv7 - what API consumers see
|
||||
|
||||
@Column()
|
||||
contractCode: string;
|
||||
|
||||
@Column()
|
||||
contractName: string;
|
||||
}
|
||||
```
|
||||
|
||||
### DTO Pattern (Accept UUID, Resolve to INT)
|
||||
|
||||
```typescript
|
||||
// dto/create-contract.dto.ts
|
||||
import { IsUUID, IsNotEmpty } from 'class-validator';
|
||||
|
||||
export class CreateContractDto {
|
||||
@IsNotEmpty()
|
||||
@IsUUID('4')
|
||||
projectUuid: string; // Accept UUID from client
|
||||
|
||||
@IsNotEmpty()
|
||||
contractCode: string;
|
||||
|
||||
@IsNotEmpty()
|
||||
contractName: string;
|
||||
}
|
||||
|
||||
// dto/contract-response.dto.ts
|
||||
import { Exclude, Expose } from 'class-transformer';
|
||||
|
||||
export class ContractResponseDto {
|
||||
@Expose({ name: 'id' })
|
||||
uuid: string; // Returned as 'id' field in JSON
|
||||
|
||||
contractCode: string;
|
||||
contractName: string;
|
||||
}
|
||||
```
|
||||
|
||||
### Service/Controller Pattern
|
||||
|
||||
```typescript
|
||||
@Controller('contracts')
|
||||
export class ContractsController {
|
||||
constructor(
|
||||
private contractsService: ContractsService,
|
||||
private uuidResolver: UuidResolver, // Helper to convert UUID → INT
|
||||
) {}
|
||||
|
||||
@Post()
|
||||
async create(@Body() dto: CreateContractDto) {
|
||||
// Resolve UUID to INT PK for database operations
|
||||
const projectId = await this.uuidResolver.resolveProject(dto.projectUuid);
|
||||
|
||||
// Create with INT FK
|
||||
const contract = await this.contractsService.create({
|
||||
...dto,
|
||||
projectId, // INT for database
|
||||
});
|
||||
|
||||
// Response automatically transforms via @Expose
|
||||
return contract;
|
||||
}
|
||||
|
||||
@Get(':id')
|
||||
async findOne(@Param('id') uuid: string) {
|
||||
// Controller receives UUID string
|
||||
// Service handles UUID → INT resolution internally
|
||||
return this.contractsService.findByUuid(uuid);
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### UUID Resolver Helper
|
||||
|
||||
```typescript
|
||||
@Injectable()
|
||||
export class UuidResolver {
|
||||
constructor(
|
||||
@InjectRepository(Project)
|
||||
private projectRepo: Repository<Project>,
|
||||
@InjectRepository(Contract)
|
||||
private contractRepo: Repository<Contract>,
|
||||
) {}
|
||||
|
||||
async resolveProject(uuid: string): Promise<number> {
|
||||
const project = await this.projectRepo.findOne({
|
||||
where: { uuid },
|
||||
select: ['id'], // Only fetch INT PK
|
||||
});
|
||||
if (!project) throw new NotFoundException('Project not found');
|
||||
return project.id;
|
||||
}
|
||||
|
||||
async resolveContract(uuid: string): Promise<number> {
|
||||
const contract = await this.contractRepo.findOne({
|
||||
where: { uuid },
|
||||
select: ['id'],
|
||||
});
|
||||
if (!contract) throw new NotFoundException('Contract not found');
|
||||
return contract.id;
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### TransformInterceptor (Required)
|
||||
|
||||
```typescript
|
||||
// Must be configured globally to handle @Exclude/@Expose
|
||||
@Injectable()
|
||||
export class TransformInterceptor implements NestInterceptor {
|
||||
intercept(context: ExecutionContext, next: CallHandler): Observable<any> {
|
||||
return next.handle().pipe(
|
||||
map((data) => instanceToPlain(data)), // Applies class-transformer decorators
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// app.module.ts
|
||||
@Module({
|
||||
providers: [
|
||||
{
|
||||
provide: APP_INTERCEPTOR,
|
||||
useClass: TransformInterceptor,
|
||||
},
|
||||
],
|
||||
})
|
||||
export class AppModule {}
|
||||
```
|
||||
|
||||
### Critical: NEVER ParseInt on UUID
|
||||
|
||||
```typescript
|
||||
// ❌ WRONG - parseInt on UUID gives garbage value
|
||||
const id = parseInt(projectUuid); // "0195a1b2-..." → 195 (wrong!)
|
||||
|
||||
// ❌ WRONG - Number() on UUID
|
||||
const id = Number(projectUuid); // NaN
|
||||
|
||||
// ❌ WRONG - Unary plus on UUID
|
||||
const id = +projectUuid; // NaN
|
||||
|
||||
// ✅ CORRECT - Resolve via database lookup
|
||||
const projectId = await uuidResolver.resolveProject(projectUuid);
|
||||
|
||||
// ✅ CORRECT - Use TypeORM find with UUID column
|
||||
const project = await projectRepo.findOne({ where: { uuid: projectUuid } });
|
||||
const id = project.id; // Get INT PK from entity
|
||||
```
|
||||
|
||||
### Query with UUID (No Resolution Needed)
|
||||
|
||||
```typescript
|
||||
// Direct UUID lookup in TypeORM
|
||||
const project = await this.projectRepo.findOne({
|
||||
where: { uuid: projectUuid }, // Query by UUID column
|
||||
});
|
||||
|
||||
// Relations use INT FK internally
|
||||
const contracts = await this.contractRepo.find({
|
||||
where: { projectId: project.id }, // INT for FK query
|
||||
});
|
||||
```
|
||||
|
||||
### Reference
|
||||
|
||||
- [ADR-019 Hybrid Identifier Strategy](../../../../specs/06-Decision-Records/ADR-019-hybrid-identifier-strategy.md)
|
||||
- [UUID Implementation Plan](../../../../specs/05-Engineering-Guidelines/05-07-hybrid-uuid-implementation-plan.md)
|
||||
- [Data Dictionary](../../../../specs/03-Data-and-Storage/03-01-data-dictionary.md)
|
||||
|
||||
> **Warning**: Using `parseInt()`, `Number()`, or unary `+` on UUID values violates ADR-019 and will cause data corruption. Always resolve UUIDs via database lookup.
|
||||
@@ -1,129 +1,128 @@
|
||||
---
|
||||
title: Use Database Migrations
|
||||
title: No TypeORM Migrations (ADR-009)
|
||||
impact: HIGH
|
||||
impactDescription: Enables safe, repeatable database schema changes
|
||||
tags: database, migrations, typeorm, schema
|
||||
impactDescription: Use direct SQL schema files instead of TypeORM migrations per project ADR
|
||||
tags: database, schema, typeorm, migrations, adr-009
|
||||
---
|
||||
|
||||
## Use Database Migrations
|
||||
## No TypeORM Migrations (ADR-009)
|
||||
|
||||
Never use `synchronize: true` in production. Use migrations for all schema changes. Migrations provide version control for your database, enable safe rollbacks, and ensure consistency across all environments.
|
||||
**This project follows ADR-009: Direct SQL Schema Management**
|
||||
|
||||
**Incorrect (using synchronize or manual SQL):**
|
||||
Unlike standard NestJS/TypeORM practices, this project does **NOT** use TypeORM migrations. Instead, we manage database schema through direct SQL files.
|
||||
|
||||
```typescript
|
||||
// Use synchronize in production
|
||||
TypeOrmModule.forRoot({
|
||||
type: 'postgres',
|
||||
synchronize: true, // DANGEROUS in production!
|
||||
// Can drop columns, tables, or data
|
||||
});
|
||||
### Why No Migrations?
|
||||
|
||||
// Manual SQL in production
|
||||
@Injectable()
|
||||
export class DatabaseService {
|
||||
async addColumn(): Promise<void> {
|
||||
await this.dataSource.query('ALTER TABLE users ADD COLUMN age INT');
|
||||
// No version control, no rollback, inconsistent across envs
|
||||
}
|
||||
}
|
||||
- **ADR-009 Decision**: Explicit schema control over auto-generated migrations
|
||||
- **MariaDB-specific features**: Native UUID type, virtual columns, custom indexing
|
||||
- **Team workflow**: Schema changes reviewed as SQL, not TypeORM migration classes
|
||||
- **Audit trail**: Single source of truth in `specs/03-Data-and-Storage/`
|
||||
|
||||
// Modify entities without migration
|
||||
@Entity()
|
||||
export class User {
|
||||
@Column()
|
||||
email: string;
|
||||
### Schema File Locations
|
||||
|
||||
@Column() // Added without migration
|
||||
newField: string; // Will crash in production if synchronize is false
|
||||
}
|
||||
```
|
||||
specs/03-Data-and-Storage/
|
||||
├── lcbp3-v1.8.0-schema-01-drop.sql # Drop statements (dev only)
|
||||
├── lcbp3-v1.8.0-schema-02-tables.sql # CREATE TABLE statements
|
||||
├── lcbp3-v1.8.0-schema-03-views-indexes.sql # Views, indexes, constraints
|
||||
└── deltas/ # Incremental changes
|
||||
├── 01-add-reference-date.sql
|
||||
├── 02-add-rbac-bulk-permission.sql
|
||||
└── 03-fix-numbering-enums.sql
|
||||
```
|
||||
|
||||
**Correct (use migrations for all schema changes):**
|
||||
### Correct: Using SQL Schema Files
|
||||
|
||||
```typescript
|
||||
// Configure TypeORM for migrations
|
||||
// data-source.ts
|
||||
export const dataSource = new DataSource({
|
||||
type: 'postgres',
|
||||
host: process.env.DB_HOST,
|
||||
port: parseInt(process.env.DB_PORT),
|
||||
username: process.env.DB_USERNAME,
|
||||
password: process.env.DB_PASSWORD,
|
||||
database: process.env.DB_NAME,
|
||||
entities: ['dist/**/*.entity.js'],
|
||||
migrations: ['dist/migrations/*.js'],
|
||||
synchronize: false, // Always false in production
|
||||
migrationsRun: true, // Run migrations on startup
|
||||
});
|
||||
|
||||
// app.module.ts
|
||||
// TypeORM configuration - NO migrationsRun
|
||||
TypeOrmModule.forRootAsync({
|
||||
inject: [ConfigService],
|
||||
useFactory: (config: ConfigService) => ({
|
||||
type: 'postgres',
|
||||
type: 'mariadb',
|
||||
host: config.get('DB_HOST'),
|
||||
synchronize: config.get('NODE_ENV') === 'development', // Only in dev
|
||||
migrations: ['dist/migrations/*.js'],
|
||||
migrationsRun: true,
|
||||
port: config.get('DB_PORT'),
|
||||
username: config.get('DB_USERNAME'),
|
||||
password: config.get('DB_PASSWORD'),
|
||||
database: config.get('DB_NAME'),
|
||||
entities: ['dist/**/*.entity.js'],
|
||||
synchronize: false, // NEVER true, even in development
|
||||
migrationsRun: false, // Disabled per ADR-009
|
||||
// Migrations are managed via SQL files, not TypeORM
|
||||
}),
|
||||
});
|
||||
```
|
||||
|
||||
// migrations/1705312800000-AddUserAge.ts
|
||||
import { MigrationInterface, QueryRunner } from 'typeorm';
|
||||
### Schema Change Process (ADR-009)
|
||||
|
||||
export class AddUserAge1705312800000 implements MigrationInterface {
|
||||
name = 'AddUserAge1705312800000';
|
||||
1. **Modify SQL file directly**:
|
||||
|
||||
public async up(queryRunner: QueryRunner): Promise<void> {
|
||||
// Add column with default to handle existing rows
|
||||
await queryRunner.query(`
|
||||
ALTER TABLE "users" ADD "age" integer DEFAULT 0
|
||||
`);
|
||||
```sql
|
||||
-- specs/03-Data-and-Storage/lcbp3-v1.8.0-schema-02-tables.sql
|
||||
ALTER TABLE correspondences
|
||||
ADD COLUMN priority VARCHAR(20) DEFAULT 'normal';
|
||||
```
|
||||
|
||||
// Add index for frequently queried columns
|
||||
await queryRunner.query(`
|
||||
CREATE INDEX "IDX_users_age" ON "users" ("age")
|
||||
`);
|
||||
}
|
||||
2. **Create delta for existing databases**:
|
||||
|
||||
public async down(queryRunner: QueryRunner): Promise<void> {
|
||||
// Always implement down for rollback
|
||||
await queryRunner.query(`DROP INDEX "IDX_users_age"`);
|
||||
await queryRunner.query(`ALTER TABLE "users" DROP COLUMN "age"`);
|
||||
}
|
||||
}
|
||||
```sql
|
||||
-- specs/03-Data-and-Storage/deltas/04-add-priority-column.sql
|
||||
ALTER TABLE correspondences
|
||||
ADD COLUMN priority VARCHAR(20) DEFAULT 'normal';
|
||||
```
|
||||
|
||||
// Safe column rename (two-step)
|
||||
export class RenameNameToFullName1705312900000 implements MigrationInterface {
|
||||
public async up(queryRunner: QueryRunner): Promise<void> {
|
||||
// Step 1: Add new column
|
||||
await queryRunner.query(`
|
||||
ALTER TABLE "users" ADD "full_name" varchar(255)
|
||||
`);
|
||||
3. **Apply to database manually or via deployment script**:
|
||||
```bash
|
||||
mysql -u root -p lcbp3 < specs/03-Data-and-Storage/deltas/04-add-priority-column.sql
|
||||
```
|
||||
|
||||
// Step 2: Copy data
|
||||
await queryRunner.query(`
|
||||
UPDATE "users" SET "full_name" = "name"
|
||||
`);
|
||||
### Entity Definition (No Migration Needed)
|
||||
|
||||
// Step 3: Add NOT NULL constraint
|
||||
await queryRunner.query(`
|
||||
ALTER TABLE "users" ALTER COLUMN "full_name" SET NOT NULL
|
||||
`);
|
||||
```typescript
|
||||
@Entity('correspondences')
|
||||
export class Correspondence {
|
||||
@PrimaryGeneratedColumn()
|
||||
id: number; // Internal INT PK
|
||||
|
||||
// Step 4: Drop old column (after verifying app works)
|
||||
await queryRunner.query(`
|
||||
ALTER TABLE "users" DROP COLUMN "name"
|
||||
`);
|
||||
}
|
||||
@Column({ type: 'uuid' })
|
||||
uuid: string; // Public UUID
|
||||
|
||||
public async down(queryRunner: QueryRunner): Promise<void> {
|
||||
await queryRunner.query(`ALTER TABLE "users" ADD "name" varchar(255)`);
|
||||
await queryRunner.query(`UPDATE "users" SET "name" = "full_name"`);
|
||||
await queryRunner.query(`ALTER TABLE "users" DROP COLUMN "full_name"`);
|
||||
}
|
||||
@Column({ name: 'priority', default: 'normal' })
|
||||
priority: string;
|
||||
|
||||
// No migration class needed - schema managed via SQL
|
||||
}
|
||||
```
|
||||
|
||||
Reference: [TypeORM Migrations](https://typeorm.io/migrations)
|
||||
### Anti-Pattern: TypeORM Migrations (Do NOT Use)
|
||||
|
||||
```typescript
|
||||
// ❌ WRONG - Do not create migration files
|
||||
// migrations/1705312800000-AddUserAge.ts
|
||||
export class AddUserAge1705312800000 implements MigrationInterface {
|
||||
public async up(queryRunner: QueryRunner): Promise<void> {
|
||||
await queryRunner.query(`ALTER TABLE "users" ADD "age" integer`);
|
||||
}
|
||||
}
|
||||
|
||||
// ❌ WRONG - Do not enable migrationsRun
|
||||
TypeOrmModule.forRoot({
|
||||
migrationsRun: true, // Disabled per ADR-009
|
||||
migrations: ['dist/migrations/*.js'],
|
||||
});
|
||||
```
|
||||
|
||||
### When You Need Schema Changes
|
||||
|
||||
1. Check `specs/03-Data-and-Storage/lcbp3-v1.8.0-schema-02-tables.sql`
|
||||
2. Add your DDL to the appropriate SQL file
|
||||
3. Create delta file in `deltas/` directory
|
||||
4. Apply SQL to your database
|
||||
5. Update corresponding Entity class
|
||||
|
||||
### Reference
|
||||
|
||||
- [ADR-009 Database Strategy](../../../../specs/06-Decision-Records/ADR-009-db-strategy.md)
|
||||
- [Schema SQL Files](../../../../specs/03-Data-and-Storage/)
|
||||
- [Data Dictionary](../../../../specs/03-Data-and-Storage/03-01-data-dictionary.md)
|
||||
|
||||
> **Warning**: Attempting to use TypeORM migrations in this project violates ADR-009 and will be rejected in code review.
|
||||
|
||||
Reference in New Issue
Block a user