From 0aaa451479911d047e4cc83f063afa68a122ba9b Mon Sep 17 00:00:00 2001 From: Jonathan Fishner Date: Mon, 18 Aug 2025 21:39:24 +0300 Subject: [PATCH] fix: prevent false change detection in DBML editor by stripping public schema on import (#858) * fix: prevent false change detection in DBML editor by stripping public schema on import * fix(dbml): preserve self-referencing relationships and character varying lengths in DBML import/export --- src/lib/dbml/apply-dbml/apply-dbml.ts | 17 +- .../dbml-export-sanitization.test.ts | 37 +- .../__tests__/dbml-self-referencing.test.ts | 172 +++++++ src/lib/dbml/dbml-export/dbml-export.ts | 66 ++- .../__tests__/dbml-character-varying.test.ts | 172 +++++++ .../dbml-import-fantasy-examples.test.ts | 6 +- .../__tests__/dbml-schema-handling.test.ts | 437 ++++++++++++++++++ src/lib/dbml/dbml-import/dbml-import.ts | 42 +- 8 files changed, 916 insertions(+), 33 deletions(-) create mode 100644 src/lib/dbml/dbml-export/__tests__/dbml-self-referencing.test.ts create mode 100644 src/lib/dbml/dbml-import/__tests__/dbml-character-varying.test.ts create mode 100644 src/lib/dbml/dbml-import/__tests__/dbml-schema-handling.test.ts diff --git a/src/lib/dbml/apply-dbml/apply-dbml.ts b/src/lib/dbml/apply-dbml/apply-dbml.ts index bba2142d..064563c5 100644 --- a/src/lib/dbml/apply-dbml/apply-dbml.ts +++ b/src/lib/dbml/apply-dbml/apply-dbml.ts @@ -226,6 +226,16 @@ const updateTables = ({ const targetKey = createObjectKeyFromTable(targetTable); let sourceTable = sourceTablesByKey.get(targetKey); + // If no match and target has a schema, try without schema + if (!sourceTable && targetTable.schema) { + const noSchemaKey = createObjectKeyFromTable({ + ...targetTable, + schema: undefined, + }); + sourceTable = sourceTablesByKey.get(noSchemaKey); + } + + // If still no match, try with default schema if (!sourceTable && defaultDatabaseSchema) { if (!targetTable.schema) { // If target table has no schema, try matching with default schema @@ -235,12 +245,7 @@ const updateTables = ({ }); sourceTable = sourceTablesByKey.get(defaultKey); } else if (targetTable.schema === defaultDatabaseSchema) { - // If target table's schema matches default, try matching without schema - const noSchemaKey = createObjectKeyFromTable({ - ...targetTable, - schema: undefined, - }); - sourceTable = sourceTablesByKey.get(noSchemaKey); + // Already tried without schema above } } diff --git a/src/lib/dbml/dbml-export/__tests__/dbml-export-sanitization.test.ts b/src/lib/dbml/dbml-export/__tests__/dbml-export-sanitization.test.ts index c47fc864..d01eeea5 100644 --- a/src/lib/dbml/dbml-export/__tests__/dbml-export-sanitization.test.ts +++ b/src/lib/dbml/dbml-export/__tests__/dbml-export-sanitization.test.ts @@ -93,17 +93,38 @@ ALTER TABLE wizard_spellbooks ADD CONSTRAINT fk_mentor FOREIGN KEY (owner_id) RE ); }); - it('should comment out self-referential foreign keys', () => { - const sql = `ALTER TABLE quest_prerequisites ADD CONSTRAINT fk_quest_prereq FOREIGN KEY (quest_id) REFERENCES quest_prerequisites (quest_id); + it('should preserve valid self-referential foreign keys but filter invalid ones', () => { + const sql = `-- Valid self-references (different fields) ALTER TABLE spell_components ADD CONSTRAINT fk_component_substitute FOREIGN KEY (substitute_id) REFERENCES spell_components (id); -ALTER TABLE guild_hierarchy ADD CONSTRAINT fk_parent_guild FOREIGN KEY (parent_guild_id) REFERENCES guild_hierarchy (guild_id);`; +ALTER TABLE guild_hierarchy ADD CONSTRAINT fk_parent_guild FOREIGN KEY (parent_guild_id) REFERENCES guild_hierarchy (guild_id); +ALTER TABLE "finance"."general_ledger" ADD CONSTRAINT fk_reversal FOREIGN KEY("reversal_id") REFERENCES "finance"."general_ledger"("ledger_id"); + +-- Invalid self-references (same field referencing itself) +ALTER TABLE quest_prerequisites ADD CONSTRAINT fk_quest_prereq FOREIGN KEY (quest_id) REFERENCES quest_prerequisites (quest_id); +ALTER TABLE "finance"."general_ledger" ADD CONSTRAINT fk_ledger_self FOREIGN KEY("ledger_id") REFERENCES "finance"."general_ledger"("ledger_id"); +ALTER TABLE wizards ADD CONSTRAINT fk_wizard_self FOREIGN KEY (id) REFERENCES wizards (id);`; const sanitized = sanitizeSQLforDBML(sql); - // Self-referential constraints should be commented out + // Valid self-referential constraints should be preserved + expect(sanitized).toContain( + 'ALTER TABLE spell_components ADD CONSTRAINT' + ); + expect(sanitized).toContain( + 'ALTER TABLE guild_hierarchy ADD CONSTRAINT' + ); + expect(sanitized).toMatch( + /ALTER TABLE "finance"\."general_ledger".*fk_reversal.*FOREIGN KEY\("reversal_id"\)/ + ); + + // Invalid self-referential constraints (same field to itself) should be commented out expect(sanitized).toContain('-- ALTER TABLE quest_prerequisites'); - expect(sanitized).toContain('-- ALTER TABLE spell_components'); - expect(sanitized).toContain('-- ALTER TABLE guild_hierarchy'); + expect(sanitized).toMatch( + /-- ALTER TABLE "finance"\."general_ledger".*fk_ledger_self.*FOREIGN KEY\("ledger_id"\).*REFERENCES.*\("ledger_id"\)/ + ); + expect(sanitized).toContain( + '-- ALTER TABLE wizards ADD CONSTRAINT fk_wizard_self' + ); }); it('should not comment out normal foreign keys', () => { @@ -246,7 +267,11 @@ ALTER TABLE spell_component_links ADD CONSTRAINT fk_creator FOREIGN KEY (link_id expect(sanitized).toContain("DEFAULT 'F'"); expect(sanitized).toContain("DEFAULT 'NOW'"); // NOW is quoted as a single word expect(sanitized).toContain('(matrix_pattern)'); // Deduplicated + // Valid self-referencing relationships (different fields) are preserved expect(sanitized).toContain( + 'ALTER TABLE spell_matrices ADD CONSTRAINT fk_self_ref' + ); + expect(sanitized).not.toContain( '-- ALTER TABLE spell_matrices ADD CONSTRAINT fk_self_ref' ); expect(sanitized).toContain( diff --git a/src/lib/dbml/dbml-export/__tests__/dbml-self-referencing.test.ts b/src/lib/dbml/dbml-export/__tests__/dbml-self-referencing.test.ts new file mode 100644 index 00000000..30031907 --- /dev/null +++ b/src/lib/dbml/dbml-export/__tests__/dbml-self-referencing.test.ts @@ -0,0 +1,172 @@ +import { describe, it, expect } from 'vitest'; +import { generateDBMLFromDiagram } from '../dbml-export'; +import { importDBMLToDiagram } from '../../dbml-import/dbml-import'; +import { DatabaseType } from '@/lib/domain/database-type'; + +describe('DBML Self-Referencing Relationships', () => { + it('should preserve self-referencing relationships in DBML export', async () => { + // Create a DBML with self-referencing relationship (general_ledger example) + const inputDBML = ` +Table "finance"."general_ledger" { + "ledger_id" bigint [pk] + "account_name" varchar(100) + "amount" decimal(10,2) + "reversal_id" bigint [ref: > "finance"."general_ledger"."ledger_id"] + "created_at" timestamp +} +`; + + // Import the DBML + const diagram = await importDBMLToDiagram(inputDBML, { + databaseType: DatabaseType.POSTGRESQL, + }); + + // Verify the relationship was imported + expect(diagram.relationships).toBeDefined(); + expect(diagram.relationships?.length).toBe(1); + + // Verify it's a self-referencing relationship + const relationship = diagram.relationships![0]; + expect(relationship.sourceTableId).toBe(relationship.targetTableId); + + // Export back to DBML + const exportResult = generateDBMLFromDiagram(diagram); + + // Check inline format + expect(exportResult.inlineDbml).toContain('reversal_id'); + // The DBML parser correctly interprets FK as: target < source + expect(exportResult.inlineDbml).toMatch( + /ref:\s*<\s*"finance"\."general_ledger"\."ledger_id"/ + ); + + // Check standard format + expect(exportResult.standardDbml).toContain('Ref '); + expect(exportResult.standardDbml).toMatch( + /"finance"\."general_ledger"\."ledger_id"\s*<\s*"finance"\."general_ledger"\."reversal_id"/ + ); + + console.log( + '✅ Self-referencing relationship preserved in DBML export' + ); + }); + + it('should handle self-referencing relationships in employee hierarchy', async () => { + // Create an employee table with manager relationship + const inputDBML = ` +Table "employees" { + "id" int [pk] + "name" varchar(100) + "manager_id" int [ref: > "employees"."id"] + "department" varchar(50) +} +`; + + const diagram = await importDBMLToDiagram(inputDBML, { + databaseType: DatabaseType.MYSQL, + }); + + // Verify the relationship + expect(diagram.relationships?.length).toBe(1); + const rel = diagram.relationships![0]; + expect(rel.sourceTableId).toBe(rel.targetTableId); + + // Export and verify + const exportResult = generateDBMLFromDiagram(diagram); + + // Check that the self-reference is preserved + expect(exportResult.inlineDbml).toContain('manager_id'); + // The DBML parser correctly interprets FK as: target < source + expect(exportResult.inlineDbml).toMatch(/ref:\s*<\s*"employees"\."id"/); + }); + + it('should handle multiple self-referencing relationships', async () => { + // Create a category table with parent-child relationships + const inputDBML = ` +Table "categories" { + "id" int [pk] + "name" varchar(100) + "parent_id" int [ref: > "categories"."id"] + "related_id" int [ref: > "categories"."id"] + "description" text +} +`; + + const diagram = await importDBMLToDiagram(inputDBML, { + databaseType: DatabaseType.POSTGRESQL, + }); + + // Should have 2 self-referencing relationships + expect(diagram.relationships?.length).toBe(2); + + // Both should be self-referencing + diagram.relationships?.forEach((rel) => { + expect(rel.sourceTableId).toBe(rel.targetTableId); + }); + + // Export and verify both relationships are preserved + const exportResult = generateDBMLFromDiagram(diagram); + + expect(exportResult.inlineDbml).toContain('parent_id'); + expect(exportResult.inlineDbml).toContain('related_id'); + + // Count the number of ref: statements + // The DBML parser correctly interprets FK as: target < source + const refMatches = exportResult.inlineDbml.match(/ref:\s* { + // Test with explicit schema names + const inputDBML = ` +Table "hr"."staff" { + "staff_id" int [pk] + "name" varchar(100) + "supervisor_id" int [ref: > "hr"."staff"."staff_id"] + "level" int +} +`; + + const diagram = await importDBMLToDiagram(inputDBML, { + databaseType: DatabaseType.POSTGRESQL, + }); + + expect(diagram.relationships?.length).toBe(1); + + const exportResult = generateDBMLFromDiagram(diagram); + + // Should preserve the schema in the reference + // The DBML parser correctly interprets FK as: target < source + expect(exportResult.inlineDbml).toMatch( + /ref:\s*<\s*"hr"\."staff"\."staff_id"/ + ); + }); + + it('should handle circular references in graph structures', async () => { + // Create a node table for graph structures + const inputDBML = ` +Table "graph_nodes" { + "node_id" bigint [pk] + "value" varchar(100) + "next_node_id" bigint [ref: > "graph_nodes"."node_id"] + "prev_node_id" bigint [ref: > "graph_nodes"."node_id"] +} +`; + + const diagram = await importDBMLToDiagram(inputDBML, { + databaseType: DatabaseType.POSTGRESQL, + }); + + // Should have 2 self-referencing relationships + expect(diagram.relationships?.length).toBe(2); + + const exportResult = generateDBMLFromDiagram(diagram); + + // Both references should be preserved + expect(exportResult.inlineDbml).toContain('next_node_id'); + expect(exportResult.inlineDbml).toContain('prev_node_id'); + + // Verify no lines are commented out + expect(exportResult.standardDbml).not.toContain('-- ALTER TABLE'); + expect(exportResult.inlineDbml).not.toContain('-- ALTER TABLE'); + }); +}); diff --git a/src/lib/dbml/dbml-export/dbml-export.ts b/src/lib/dbml/dbml-export/dbml-export.ts index 36d42bdf..f49a54d0 100644 --- a/src/lib/dbml/dbml-export/dbml-export.ts +++ b/src/lib/dbml/dbml-export/dbml-export.ts @@ -155,14 +155,25 @@ export const sanitizeSQLforDBML = (sql: string): string => { } ); - // Comment out self-referencing foreign keys to prevent "Two endpoints are the same" error - // Example: ALTER TABLE public.class ADD CONSTRAINT ... FOREIGN KEY (class_id) REFERENCES public.class (class_id); + // Comment out invalid self-referencing foreign keys where the same field references itself + // Example: ALTER TABLE table ADD CONSTRAINT ... FOREIGN KEY (field_a) REFERENCES table (field_a); + // But keep valid self-references like: FOREIGN KEY (field_a) REFERENCES table (field_b); const lines = sanitized.split('\n'); const processedLines = lines.map((line) => { + // Match pattern: ALTER TABLE [schema.]table ADD CONSTRAINT ... FOREIGN KEY(field) REFERENCES [schema.]table(field) + // Capture the table name, source field, and target field const selfRefFKPattern = - /ALTER\s+TABLE\s+(?:\S+\.)?(\S+)\s+ADD\s+CONSTRAINT\s+\S+\s+FOREIGN\s+KEY\s*\([^)]+\)\s+REFERENCES\s+(?:\S+\.)?\1\s*\([^)]+\)\s*;/i; - if (selfRefFKPattern.test(line)) { - return `-- ${line}`; // Comment out the line + /ALTER\s+TABLE\s+(?:["[]?(\S+?)[\]"]?\.)?["[]?(\S+?)[\]"]?\s+ADD\s+CONSTRAINT\s+\S+\s+FOREIGN\s+KEY\s*\(["[]?([^)"]+)[\]"]?\)\s+REFERENCES\s+(?:["[]?\S+?[\]"]?\.)?"?[[]?\2[\]]?"?\s*\(["[]?([^)"]+)[\]"]?\)\s*;/i; + const match = selfRefFKPattern.exec(line); + + if (match) { + const sourceField = match[3].trim(); + const targetField = match[4].trim(); + + // Only comment out if source and target fields are the same + if (sourceField === targetField) { + return `-- ${line}`; // Comment out invalid self-reference + } } return line; }); @@ -491,9 +502,21 @@ const convertToInlineRefs = (dbml: string): string => { return cleanedDbml; }; +// Function to check for DBML reserved keywords +const isDBMLKeyword = (name: string): boolean => { + const keywords = new Set([ + 'YES', + 'NO', + 'TRUE', + 'FALSE', + 'NULL', // DBML reserved keywords (boolean literals) + ]); + return keywords.has(name.toUpperCase()); +}; + // Function to check for SQL keywords (add more if needed) const isSQLKeyword = (name: string): boolean => { - const keywords = new Set(['CASE', 'ORDER', 'GROUP', 'FROM', 'TO', 'USER']); // Add common keywords + const keywords = new Set(['CASE', 'ORDER', 'GROUP', 'FROM', 'TO', 'USER']); // Common SQL keywords return keywords.has(name.toUpperCase()); }; @@ -758,6 +781,8 @@ export function generateDBMLFromDiagram(diagram: Diagram): DBMLExportResult { const cleanDiagram = fixProblematicFieldNames(filteredDiagram); // --- Final sanitization and renaming pass --- + // Only rename keywords for PostgreSQL/SQLite + // For other databases, we'll wrap problematic names in quotes instead const shouldRenameKeywords = diagram.databaseType === DatabaseType.POSTGRESQL || diagram.databaseType === DatabaseType.SQLITE; @@ -777,14 +802,21 @@ export function generateDBMLFromDiagram(diagram: Diagram): DBMLExportResult { safeTableName = `"${originalName.replace(/"/g, '\\"')}"`; } - // Rename table if SQL keyword (PostgreSQL only) - if (shouldRenameKeywords && isSQLKeyword(originalName)) { + // Rename table if it's a keyword (PostgreSQL/SQLite only) + if ( + shouldRenameKeywords && + (isDBMLKeyword(originalName) || isSQLKeyword(originalName)) + ) { const newName = `${originalName}_table`; sqlRenamedTables.set(newName, originalName); safeTableName = /[^\w]/.test(newName) ? `"${newName.replace(/"/g, '\\"')}"` : newName; } + // For other databases, just quote DBML keywords + else if (!shouldRenameKeywords && isDBMLKeyword(originalName)) { + safeTableName = `"${originalName.replace(/"/g, '\\"')}"`; + } const fieldNameCounts = new Map(); const processedFields = table.fields.map((field) => { @@ -811,8 +843,11 @@ export function generateDBMLFromDiagram(diagram: Diagram): DBMLExportResult { name: finalSafeName, }; - // Rename field if SQL keyword (PostgreSQL only) - if (shouldRenameKeywords && isSQLKeyword(field.name)) { + // Rename field if it's a keyword (PostgreSQL/SQLite only) + if ( + shouldRenameKeywords && + (isDBMLKeyword(field.name) || isSQLKeyword(field.name)) + ) { const newFieldName = `${field.name}_field`; fieldRenames.push({ table: safeTableName, @@ -823,6 +858,10 @@ export function generateDBMLFromDiagram(diagram: Diagram): DBMLExportResult { ? `"${newFieldName.replace(/"/g, '\\"')}"` : newFieldName; } + // For other databases, just quote DBML keywords + else if (!shouldRenameKeywords && isDBMLKeyword(field.name)) { + sanitizedField.name = `"${field.name.replace(/"/g, '\\"')}"`; + } return sanitizedField; }); @@ -875,8 +914,11 @@ export function generateDBMLFromDiagram(diagram: Diagram): DBMLExportResult { baseScript = sanitizeSQLforDBML(baseScript); - // Append comments for renamed tables and fields (PostgreSQL only) - if (shouldRenameKeywords) { + // Append comments for renamed tables and fields (PostgreSQL/SQLite only) + if ( + shouldRenameKeywords && + (sqlRenamedTables.size > 0 || fieldRenames.length > 0) + ) { baseScript = appendRenameComments( baseScript, sqlRenamedTables, diff --git a/src/lib/dbml/dbml-import/__tests__/dbml-character-varying.test.ts b/src/lib/dbml/dbml-import/__tests__/dbml-character-varying.test.ts new file mode 100644 index 00000000..57c9b9f6 --- /dev/null +++ b/src/lib/dbml/dbml-import/__tests__/dbml-character-varying.test.ts @@ -0,0 +1,172 @@ +import { describe, it, expect } from 'vitest'; +import { importDBMLToDiagram } from '../dbml-import'; +import { generateDBMLFromDiagram } from '../../dbml-export/dbml-export'; +import { DatabaseType } from '@/lib/domain/database-type'; + +describe('DBML Character Varying Length Preservation', () => { + it('should preserve character varying length when quoted', async () => { + const inputDBML = ` +Table "finance"."general_ledger" { + "ledger_id" integer [pk] + "currency_code" "character varying(3)" + "reference_number" "character varying(50)" + "description" text +} +`; + + const diagram = await importDBMLToDiagram(inputDBML, { + databaseType: DatabaseType.POSTGRESQL, + }); + + // Check that the lengths were captured + const table = diagram.tables?.find((t) => t.name === 'general_ledger'); + expect(table).toBeDefined(); + + const currencyField = table?.fields.find( + (f) => f.name === 'currency_code' + ); + const referenceField = table?.fields.find( + (f) => f.name === 'reference_number' + ); + + expect(currencyField?.characterMaximumLength).toBe('3'); + expect(referenceField?.characterMaximumLength).toBe('50'); + + // Export and verify lengths are preserved + const exportResult = generateDBMLFromDiagram(diagram); + + // Should contain the character varying with lengths + expect(exportResult.inlineDbml).toMatch( + /"currency_code".*(?:character varying|varchar)\(3\)/ + ); + expect(exportResult.inlineDbml).toMatch( + /"reference_number".*(?:character varying|varchar)\(50\)/ + ); + }); + + it('should preserve varchar length without quotes', async () => { + const inputDBML = ` +Table "users" { + "id" int [pk] + "username" varchar(100) + "email" varchar(255) + "bio" text +} +`; + + const diagram = await importDBMLToDiagram(inputDBML, { + databaseType: DatabaseType.MYSQL, + }); + + const table = diagram.tables?.find((t) => t.name === 'users'); + expect(table).toBeDefined(); + + const usernameField = table?.fields.find((f) => f.name === 'username'); + const emailField = table?.fields.find((f) => f.name === 'email'); + + expect(usernameField?.characterMaximumLength).toBe('100'); + expect(emailField?.characterMaximumLength).toBe('255'); + + // Export and verify + const exportResult = generateDBMLFromDiagram(diagram); + expect(exportResult.inlineDbml).toContain('varchar(100)'); + expect(exportResult.inlineDbml).toContain('varchar(255)'); + }); + + it('should handle complex quoted types with schema and length', async () => { + const inputDBML = ` +Enum "public"."transaction_type" { + "debit" + "credit" +} + +Table "finance"."general_ledger" { + "ledger_id" integer [pk, not null] + "transaction_date" date [not null] + "account_id" integer + "transaction_type" transaction_type + "amount" numeric(15,2) [not null] + "currency_code" "character varying(3)" + "exchange_rate" numeric(10,6) + "reference_number" "character varying(50)" + "description" text + "posted_by" integer + "posting_date" timestamp + "is_reversed" boolean + "reversal_id" integer [ref: < "finance"."general_ledger"."ledger_id"] +} +`; + + const diagram = await importDBMLToDiagram(inputDBML, { + databaseType: DatabaseType.POSTGRESQL, + }); + + const table = diagram.tables?.find((t) => t.name === 'general_ledger'); + expect(table).toBeDefined(); + + // Check all field types are preserved + const currencyField = table?.fields.find( + (f) => f.name === 'currency_code' + ); + const referenceField = table?.fields.find( + (f) => f.name === 'reference_number' + ); + const amountField = table?.fields.find((f) => f.name === 'amount'); + const exchangeRateField = table?.fields.find( + (f) => f.name === 'exchange_rate' + ); + + expect(currencyField?.characterMaximumLength).toBe('3'); + expect(referenceField?.characterMaximumLength).toBe('50'); + expect(amountField?.precision).toBe(15); + expect(amountField?.scale).toBe(2); + expect(exchangeRateField?.precision).toBe(10); + expect(exchangeRateField?.scale).toBe(6); + + // Export and verify all types are preserved correctly + const exportResult = generateDBMLFromDiagram(diagram); + + // Check that numeric types have their precision/scale + expect(exportResult.inlineDbml).toMatch(/numeric\(15,\s*2\)/); + expect(exportResult.inlineDbml).toMatch(/numeric\(10,\s*6\)/); + + // Check that character varying has lengths + expect(exportResult.inlineDbml).toMatch( + /(?:character varying|varchar)\(3\)/ + ); + expect(exportResult.inlineDbml).toMatch( + /(?:character varying|varchar)\(50\)/ + ); + }); + + it('should handle char types with length', async () => { + const inputDBML = ` +Table "products" { + "product_code" char(5) [pk] + "category" "char(2)" + "status" character(1) + "description" varchar +} +`; + + const diagram = await importDBMLToDiagram(inputDBML, { + databaseType: DatabaseType.POSTGRESQL, + }); + + const table = diagram.tables?.find((t) => t.name === 'products'); + + const productCodeField = table?.fields.find( + (f) => f.name === 'product_code' + ); + const categoryField = table?.fields.find((f) => f.name === 'category'); + const statusField = table?.fields.find((f) => f.name === 'status'); + const descriptionField = table?.fields.find( + (f) => f.name === 'description' + ); + + expect(productCodeField?.characterMaximumLength).toBe('5'); + expect(categoryField?.characterMaximumLength).toBe('2'); + expect(statusField?.characterMaximumLength).toBe('1'); + expect(descriptionField?.characterMaximumLength).toBeUndefined(); // varchar without length + }); +}); diff --git a/src/lib/dbml/dbml-import/__tests__/dbml-import-fantasy-examples.test.ts b/src/lib/dbml/dbml-import/__tests__/dbml-import-fantasy-examples.test.ts index eae700ef..7c5a3292 100644 --- a/src/lib/dbml/dbml-import/__tests__/dbml-import-fantasy-examples.test.ts +++ b/src/lib/dbml/dbml-import/__tests__/dbml-import-fantasy-examples.test.ts @@ -817,8 +817,9 @@ Table admin.users { ]); // Verify fields reference correct enums + // Note: 'public' schema is converted to empty string const publicUsersTable = diagram.tables?.find( - (t) => t.name === 'users' && t.schema === 'public' + (t) => t.name === 'users' && t.schema === '' ); const adminUsersTable = diagram.tables?.find( (t) => t.name === 'users' && t.schema === 'admin' @@ -1075,8 +1076,9 @@ Table "public_3"."comments" { // Verify tables expect(diagram.tables).toHaveLength(3); + // Note: 'public' schema is converted to empty string const usersTable = diagram.tables?.find( - (t) => t.name === 'users' && t.schema === 'public' + (t) => t.name === 'users' && t.schema === '' ); const postsTable = diagram.tables?.find( (t) => t.name === 'posts' && t.schema === 'public_2' diff --git a/src/lib/dbml/dbml-import/__tests__/dbml-schema-handling.test.ts b/src/lib/dbml/dbml-import/__tests__/dbml-schema-handling.test.ts new file mode 100644 index 00000000..27c26f4c --- /dev/null +++ b/src/lib/dbml/dbml-import/__tests__/dbml-schema-handling.test.ts @@ -0,0 +1,437 @@ +import { describe, it, expect } from 'vitest'; +import { importDBMLToDiagram } from '../dbml-import'; +import { generateDBMLFromDiagram } from '../../dbml-export/dbml-export'; +import { applyDBMLChanges } from '../../apply-dbml/apply-dbml'; +import { DatabaseType } from '@/lib/domain/database-type'; +import type { Diagram } from '@/lib/domain/diagram'; + +describe('DBML Schema Handling - Fantasy Realm Database', () => { + describe('MySQL - No Schema Support', () => { + it('should not add public schema for MySQL databases', async () => { + // Fantasy realm DBML with tables that would typically get 'public' schema + const dbmlContent = ` + Table "wizards" { + "id" bigint [pk] + "name" varchar(100) + "magic_level" int + "Yes" varchar(10) // Reserved DBML keyword + "No" varchar(10) // Reserved DBML keyword + } + + Table "dragons" { + "id" bigint [pk] + "name" varchar(100) + "treasure_count" int + "is_friendly" boolean + } + + Table "spells" { + "id" bigint [pk] + "spell_name" varchar(200) + "wizard_id" bigint + "power_level" int + } + + Ref: "spells"."wizard_id" > "wizards"."id" + `; + + const diagram = await importDBMLToDiagram(dbmlContent, { + databaseType: DatabaseType.MYSQL, + }); + + // Verify no 'public' schema was added + expect(diagram.tables).toBeDefined(); + diagram.tables?.forEach((table) => { + expect(table.schema).toBe(''); + console.log( + `✓ Table "${table.name}" has no schema (MySQL behavior)` + ); + }); + + // Check specific tables + const wizardsTable = diagram.tables?.find( + (t) => t.name === 'wizards' + ); + expect(wizardsTable).toBeDefined(); + expect(wizardsTable?.schema).toBe(''); + + // Check that reserved keywords are preserved as field names + const yesField = wizardsTable?.fields.find((f) => f.name === 'Yes'); + const noField = wizardsTable?.fields.find((f) => f.name === 'No'); + expect(yesField).toBeDefined(); + expect(noField).toBeDefined(); + }); + + it('should preserve IDs when re-importing DBML (no false changes)', async () => { + // Create initial diagram + const initialDBML = ` + Table "kingdoms" { + "id" bigint [pk] + "name" varchar(100) + "ruler" varchar(100) + "Yes" varchar(10) // Acceptance status + "No" varchar(10) // Rejection status + } + + Table "knights" { + "id" bigint [pk] + "name" varchar(100) + "kingdom_id" bigint + "honor_points" int + } + + Ref: "knights"."kingdom_id" > "kingdoms"."id" + `; + + // Import initial DBML + const sourceDiagram = await importDBMLToDiagram(initialDBML, { + databaseType: DatabaseType.MYSQL, + }); + + // Export to DBML + const exported = generateDBMLFromDiagram(sourceDiagram); + + // Re-import the exported DBML (simulating edit mode) + const reimportedDiagram = await importDBMLToDiagram( + exported.inlineDbml, + { + databaseType: DatabaseType.MYSQL, + } + ); + + // Apply DBML changes (should preserve IDs) + const targetDiagram: Diagram = { + ...sourceDiagram, + tables: reimportedDiagram.tables, + relationships: reimportedDiagram.relationships, + customTypes: reimportedDiagram.customTypes, + }; + + const resultDiagram = applyDBMLChanges({ + sourceDiagram, + targetDiagram, + }); + + // Verify IDs are preserved + expect(resultDiagram.tables?.length).toBe( + sourceDiagram.tables?.length + ); + + sourceDiagram.tables?.forEach((sourceTable, idx) => { + const resultTable = resultDiagram.tables?.[idx]; + expect(resultTable?.id).toBe(sourceTable.id); + expect(resultTable?.name).toBe(sourceTable.name); + + // Check field IDs are preserved + sourceTable.fields.forEach((sourceField, fieldIdx) => { + const resultField = resultTable?.fields[fieldIdx]; + expect(resultField?.id).toBe(sourceField.id); + expect(resultField?.name).toBe(sourceField.name); + }); + }); + + console.log('✓ All IDs preserved after DBML round-trip'); + }); + }); + + describe('PostgreSQL - Schema Support', () => { + it('should handle schemas correctly for PostgreSQL', async () => { + // Fantasy realm with multiple schemas + const dbmlContent = ` + Table "public"."heroes" { + "id" bigint [pk] + "name" varchar(100) + "class" varchar(50) + } + + Table "private"."secret_quests" { + "id" bigint [pk] + "quest_name" varchar(200) + "hero_id" bigint + } + + Table "artifacts" { + "id" bigint [pk] + "name" varchar(100) + "power" int + } + + Ref: "private"."secret_quests"."hero_id" > "public"."heroes"."id" + `; + + const diagram = await importDBMLToDiagram(dbmlContent, { + databaseType: DatabaseType.POSTGRESQL, + }); + + // Check schemas are preserved correctly + const heroesTable = diagram.tables?.find( + (t) => t.name === 'heroes' + ); + expect(heroesTable?.schema).toBe(''); // 'public' should be converted to empty + + const secretQuestsTable = diagram.tables?.find( + (t) => t.name === 'secret_quests' + ); + expect(secretQuestsTable?.schema).toBe('private'); // Other schemas preserved + + const artifactsTable = diagram.tables?.find( + (t) => t.name === 'artifacts' + ); + expect(artifactsTable?.schema).toBe(''); // No schema = empty string + }); + + it('should rename reserved keywords for PostgreSQL', async () => { + const dbmlContent = ` + Table "magic_items" { + "id" bigint [pk] + "name" varchar(100) + "Order" int // SQL keyword + "Yes" varchar(10) // DBML keyword + "No" varchar(10) // DBML keyword + } + `; + + const diagram = await importDBMLToDiagram(dbmlContent, { + databaseType: DatabaseType.POSTGRESQL, + }); + + const exported = generateDBMLFromDiagram(diagram); + + // For PostgreSQL, keywords should be renamed in export + expect(exported.standardDbml).toContain('Order_field'); + expect(exported.standardDbml).toContain('Yes_field'); + expect(exported.standardDbml).toContain('No_field'); + }); + }); + + describe('Public Schema Handling - The Core Fix', () => { + it('should strip public schema for MySQL to prevent ID mismatch', async () => { + // This test verifies the core fix - that 'public' schema is converted to empty string + const dbmlWithPublicSchema = ` + Table "public"."enchanted_items" { + "id" bigint [pk] + "item_name" varchar(100) + "power" int + } + + Table "public"."spell_books" { + "id" bigint [pk] + "title" varchar(200) + "author" varchar(100) + } + `; + + const mysqlDiagram = await importDBMLToDiagram( + dbmlWithPublicSchema, + { + databaseType: DatabaseType.MYSQL, + } + ); + + // For MySQL, 'public' schema should be stripped + mysqlDiagram.tables?.forEach((table) => { + expect(table.schema).toBe(''); + console.log( + `✓ MySQL: Table "${table.name}" has no schema (public was stripped)` + ); + }); + + // Now test with PostgreSQL - public should also be stripped (it's the default) + const pgDiagram = await importDBMLToDiagram(dbmlWithPublicSchema, { + databaseType: DatabaseType.POSTGRESQL, + }); + + pgDiagram.tables?.forEach((table) => { + expect(table.schema).toBe(''); + console.log( + `✓ PostgreSQL: Table "${table.name}" has no schema (public is default)` + ); + }); + }); + + it('should preserve non-public schemas', async () => { + const dbmlWithCustomSchema = ` + Table "fantasy"."magic_users" { + "id" bigint [pk] + "name" varchar(100) + "class" varchar(50) + } + + Table "adventure"."quests" { + "id" bigint [pk] + "title" varchar(200) + "reward" int + } + `; + + const diagram = await importDBMLToDiagram(dbmlWithCustomSchema, { + databaseType: DatabaseType.POSTGRESQL, + }); + + // Non-public schemas should be preserved + const magicTable = diagram.tables?.find( + (t) => t.name === 'magic_users' + ); + const questTable = diagram.tables?.find((t) => t.name === 'quests'); + + expect(magicTable?.schema).toBe('fantasy'); + expect(questTable?.schema).toBe('adventure'); + console.log('✓ Custom schemas preserved correctly'); + }); + }); + + describe('Edge Cases - The Dungeon of Bugs', () => { + it('should handle tables with names that need quoting', async () => { + const dbmlContent = ` + Table "dragons_lair" { + "id" bigint [pk] + "treasure_amount" decimal + } + + Table "wizard_tower" { + "id" bigint [pk] + "floor_count" int + } + + Table "quest_log" { + "id" bigint [pk] + "quest_name" varchar(200) + } + `; + + const diagram = await importDBMLToDiagram(dbmlContent, { + databaseType: DatabaseType.MYSQL, + }); + + // Tables should be imported correctly + expect(diagram.tables?.length).toBe(3); + expect( + diagram.tables?.find((t) => t.name === 'dragons_lair') + ).toBeDefined(); + expect( + diagram.tables?.find((t) => t.name === 'wizard_tower') + ).toBeDefined(); + expect( + diagram.tables?.find((t) => t.name === 'quest_log') + ).toBeDefined(); + }); + + it('should handle the Work_Order_Page_Debug case with Yes/No fields', async () => { + // This is the exact case that was causing the original bug + const dbmlContent = ` + Table "Work_Order_Page_Debug" { + "ID" bigint [pk, not null] + "Work_Order_For" varchar(255) + "Quan_to_Make" int + "Text_Gen" text + "Gen_Info" text + "Yes" varchar(255) + "No" varchar(255) + } + `; + + const diagram = await importDBMLToDiagram(dbmlContent, { + databaseType: DatabaseType.MYSQL, + }); + + const table = diagram.tables?.find( + (t) => t.name === 'Work_Order_Page_Debug' + ); + expect(table).toBeDefined(); + + // Check Yes and No fields are preserved + const yesField = table?.fields.find((f) => f.name === 'Yes'); + const noField = table?.fields.find((f) => f.name === 'No'); + + expect(yesField).toBeDefined(); + expect(noField).toBeDefined(); + expect(yesField?.name).toBe('Yes'); + expect(noField?.name).toBe('No'); + + // Export and verify it doesn't cause errors + const exported = generateDBMLFromDiagram(diagram); + expect(exported.standardDbml).toContain('"Yes"'); + expect(exported.standardDbml).toContain('"No"'); + + // Re-import should work without errors + const reimported = await importDBMLToDiagram(exported.inlineDbml, { + databaseType: DatabaseType.MYSQL, + }); + + expect(reimported.tables?.length).toBe(1); + }); + }); + + describe('Round-trip Testing - The Eternal Cycle', () => { + it('should maintain data integrity through multiple import/export cycles', async () => { + const originalDBML = ` + Table "guild_members" { + "id" bigint [pk] + "name" varchar(100) + "level" int + "Yes" varchar(10) // Active status + "No" varchar(10) // Inactive status + "Order" int // SQL keyword - rank order + } + + Table "guild_quests" { + "id" bigint [pk] + "quest_name" varchar(200) + "assigned_to" bigint + "difficulty" int + } + + Ref: "guild_quests"."assigned_to" > "guild_members"."id" + `; + + let currentDiagram = await importDBMLToDiagram(originalDBML, { + databaseType: DatabaseType.MYSQL, + }); + + // Store original IDs + const originalTableIds = currentDiagram.tables?.map((t) => ({ + name: t.name, + id: t.id, + })); + + // Perform 3 round-trips + for (let cycle = 1; cycle <= 3; cycle++) { + console.log(`🔄 Round-trip cycle ${cycle}`); + + // Export + const exported = generateDBMLFromDiagram(currentDiagram); + + // Re-import + const reimported = await importDBMLToDiagram( + exported.inlineDbml, + { + databaseType: DatabaseType.MYSQL, + } + ); + + // Apply changes + const targetDiagram: Diagram = { + ...currentDiagram, + tables: reimported.tables, + relationships: reimported.relationships, + customTypes: reimported.customTypes, + }; + + currentDiagram = applyDBMLChanges({ + sourceDiagram: currentDiagram, + targetDiagram, + }); + + // Verify IDs are still the same as original + originalTableIds?.forEach((original) => { + const currentTable = currentDiagram.tables?.find( + (t) => t.name === original.name + ); + expect(currentTable?.id).toBe(original.id); + }); + } + + console.log('✓ Data integrity maintained through 3 cycles'); + }); + }); +}); diff --git a/src/lib/dbml/dbml-import/dbml-import.ts b/src/lib/dbml/dbml-import/dbml-import.ts index 52ff114a..8d590f5a 100644 --- a/src/lib/dbml/dbml-import/dbml-import.ts +++ b/src/lib/dbml/dbml-import/dbml-import.ts @@ -246,21 +246,47 @@ export const importDBMLToDiagram = async ( field: Field, enums: DBMLEnum[] ): Partial => { - if (!field.type || !field.type.args) { - return {}; + // First check if the type name itself contains the length (e.g., "character varying(50)") + const typeName = field.type.type_name; + let extractedArgs: string[] | undefined; + + // Check for types with embedded length like "character varying(50)" or varchar(255) + const typeWithLengthMatch = typeName.match(/^(.+?)\(([^)]+)\)$/); + if (typeWithLengthMatch) { + // Extract the args from the type name itself + extractedArgs = typeWithLengthMatch[2] + .split(',') + .map((arg: string) => arg.trim()); } - const args = field.type.args.split(',') as string[]; + // Use extracted args or fall back to field.type.args + const args = + extractedArgs || + (field.type.args ? field.type.args.split(',') : undefined); + + if (!args || args.length === 0) { + return {}; + } const dataType = mapDBMLTypeToDataType(field.type.type_name, { ...options, enums, }); - if (dataType.fieldAttributes?.hasCharMaxLength) { - const charMaxLength = args?.[0]; + // Check if this is a character type that should have a max length + const baseTypeName = typeName + .replace(/\(.*\)/, '') + .toLowerCase() + .replace(/['"]/g, ''); + const isCharType = + baseTypeName.includes('char') || + baseTypeName.includes('varchar') || + baseTypeName === 'text' || + baseTypeName === 'string'; + + if (isCharType && args[0]) { return { - characterMaximumLength: charMaxLength, + characterMaximumLength: args[0], }; } else if ( dataType.fieldAttributes?.precision && @@ -500,7 +526,9 @@ export const importDBMLToDiagram = async ( name: table.name.replace(/['"]/g, ''), schema: typeof table.schema === 'string' - ? table.schema + ? table.schema === 'public' + ? '' + : table.schema : table.schema?.name || '', order: index, fields,