diff --git a/REVERT_TOCHANGE_EXPLANATION.md b/REVERT_TOCHANGE_EXPLANATION.md new file mode 100644 index 000000000..c84864c2b --- /dev/null +++ b/REVERT_TOCHANGE_EXPLANATION.md @@ -0,0 +1,164 @@ +# Explanation of toChange Special Case in Revert Method + +## Executive Summary + +The `toChange` special case in the `LaunchQLProject.revert()` method uses workspace-wide resolution to ensure database safety when reverting specific changes. This document explains why this approach is necessary and correct, addressing concerns about why it differs from the verify method's approach. + +## The Problem: Dependency Chain Violations + +### Test Fixture Analysis + +From the test fixtures in `__fixtures__/sqitch/simple-w-tags/`: + +1. **my-first** module has tags: + - `@v1.0.0` (after table_users) + - `@v1.1.0` (after table_products) + +2. **my-third:create_schema** depends on `my-first:@v1.1.0` (which includes table_products) + +3. **Test scenario**: `revertModule('my-first:@v1.0.0')` tries to revert my-first back to before table_products existed + +4. **The Problem**: If we only revert my-first using module-specific resolution: + - my-first gets reverted to @v1.0.0 (table_products is removed) + - my-third:create_schema still exists in the database + - my-third:create_schema depends on table_products that no longer exists + - Result: Database constraint violation "Cannot revert table_products: required by my-third:create_schema" + +### Why Workspace-Wide Resolution Solves This + +The workspace-wide resolution (`this.resolveWorkspaceExtensionDependencies()`) ensures: + +1. **Dependency Discovery**: All modules in the workspace are analyzed for dependencies +2. **Proper Ordering**: my-third gets reverted BEFORE my-first +3. **Safe Revert**: table_products can be safely removed after dependent modules are gone + +## Database Safety Requirements + +### Revert Operations Modify State + +Unlike verify operations, revert operations: +- **Modify database state** by removing tables, functions, schemas, etc. +- **Must respect dependency constraints** enforced by PostgreSQL +- **Require careful ordering** to prevent "Cannot drop X: required by Y" errors + +### PostgreSQL Constraint Enforcement + +PostgreSQL enforces referential integrity and dependency constraints: +- Foreign key constraints prevent dropping referenced tables +- Function dependencies prevent dropping functions used by other objects +- Schema dependencies prevent dropping schemas containing referenced objects + +## Why Verify Doesn't Need This Special Case + +### Verify Operations Are Read-Only + +The verify method: +- **Only checks existence** of database objects +- **Doesn't modify state** so no dependency ordering concerns +- **Can safely use module-specific resolution** since it's just validation + +### Different Use Cases + +| Operation | Purpose | State Change | Dependency Concerns | +|-----------|---------|--------------|-------------------| +| **Verify** | Check if modules exist in database | None | None | +| **Revert** | Remove database objects | Yes | Critical | + +## Alternative Approaches Considered + +### Option 1: Module-Specific Resolution (Like Verify) +```typescript +// This would fail with dependency violations +const moduleProject = this.getModuleProject(name); +extensionsToRevert = moduleProject.getModuleExtensions(); +``` + +**Problems**: +- Fails with "Cannot revert X: required by Y" errors +- Doesn't account for cross-module dependencies +- Unsafe for database operations + +### Option 2: Targeted Dependent Module Resolution +```typescript +// Find only modules that depend on the target +const dependentModules = findModulesThatDependOn(name); +extensionsToRevert = resolveDependencies(dependentModules); +``` + +**Problems**: +- More complex to implement and maintain +- Similar results to workspace-wide resolution in practice +- Additional complexity without clear benefits + +### Option 3: Dependency-Aware Revert Order +```typescript +// Track cross-module dependencies at specific versions +const versionAwareDeps = resolveVersionSpecificDependencies(name, toChange); +extensionsToRevert = orderByDependencies(versionAwareDeps); +``` + +**Problems**: +- Significantly more complex implementation +- Requires tracking version-specific cross-module dependencies +- Overkill for the current use case + +## Code Implementation Details + +### Current Implementation +```typescript +} else if (toChange) { + // When reverting to a specific change (toChange), we need workspace-wide resolution + // to prevent "Cannot revert X: required by Y" database dependency violations. + extensionsToRevert = this.resolveWorkspaceExtensionDependencies(); +} +``` + +### Why This Works + +1. **Comprehensive Scope**: `resolveWorkspaceExtensionDependencies()` creates a virtual module that depends on all workspace modules +2. **Proper Ordering**: Returns modules in dependency order (dependencies before dependents) +3. **Reverse Execution**: The revert loop processes in reverse order, ensuring dependents are reverted first + +## Test Evidence + +### Failing Test Without Special Case +``` +LaunchQLError: Revert failed for module: my-first +Cannot revert table_products: required by my-third:create_schema +``` + +### Passing Test With Special Case +The workspace-wide resolution ensures my-third is reverted before my-first, allowing table_products to be safely removed. + +## Conclusion + +The `toChange` special case in the revert method is **correct and necessary** for database safety. While it may seem inconsistent with the verify pattern, the fundamental difference between state-modifying and read-only operations justifies the different approaches. + +### Key Insights + +1. **Revert operations require dependency-aware ordering** due to database constraints +2. **Verify operations don't need this complexity** since they're read-only +3. **Workspace-wide resolution is the most robust approach** for ensuring safe revert operations +4. **The special case prevents real database errors** that would occur with simpler approaches + +### Recommendation + +**Keep the current implementation** as it correctly handles the complex dependency scenarios that arise when reverting specific changes across modules with inter-dependencies. + +## Truncation Optimization + +### Performance Improvement + +The revert method now includes truncation logic to avoid processing unnecessary modules beyond the target. When reverting with a toChange parameter: + +1. **Workspace-wide resolution** is still used for dependency safety +2. **Truncation is applied** before reversal to remove modules that come after the target +3. **Reversal proceeds** with only the necessary modules + +### Example + +When reverting `my-second:@v2.0.0` in a workspace with `[my-first, my-second, my-third]`: +- **Before truncation**: Process all modules `[my-third, my-second, my-first]` (reversed) +- **After truncation**: Process only `[my-third, my-second]` (reversed) + +This prevents unnecessary processing of `my-first` while maintaining database safety through proper dependency ordering. We must revert `my-third` first since it depends on `my-second`. diff --git a/packages/core/__tests__/core/__snapshots__/workspace-extensions-dependency-order.test.ts.snap b/packages/core/__tests__/core/__snapshots__/workspace-extensions-dependency-order.test.ts.snap new file mode 100644 index 000000000..725868ef3 --- /dev/null +++ b/packages/core/__tests__/core/__snapshots__/workspace-extensions-dependency-order.test.ts.snap @@ -0,0 +1,113 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`getWorkspaceExtensionsInDependencyOrder with complex existing fixtures handles different fixture types consistently 1`] = ` +{ + "external": [ + "citext", + "plpgsql", + "pgcrypto", + ], + "resolved": [ + "citext", + "plpgsql", + "pgcrypto", + "my-first", + "my-second", + "my-third", + ], +} +`; + +exports[`getWorkspaceExtensionsInDependencyOrder with complex existing fixtures returns extensions in dependency order for complex workspace 1`] = ` +{ + "external": [ + "plpgsql", + "uuid-ossp", + "pgcrypto", + ], + "resolved": [ + "plpgsql", + "uuid-ossp", + "pg-utilities", + "pg-verify", + "pgcrypto", + "totp", + "utils", + "secrets", + ], +} +`; + +exports[`getWorkspaceExtensionsInDependencyOrder with complex existing fixtures verifies dependency ordering properties 1`] = ` +{ + "external": [ + "citext", + "plpgsql", + "pgcrypto", + ], + "resolved": [ + "citext", + "plpgsql", + "pgcrypto", + "my-first", + "my-second", + "my-third", + ], +} +`; + +exports[`getWorkspaceExtensionsInDependencyOrder with existing fixtures handles workspace with minimal modules 1`] = ` +{ + "external": [ + "citext", + "plpgsql", + "pgcrypto", + ], + "resolved": [ + "citext", + "plpgsql", + "pgcrypto", + "my-first", + "my-second", + "my-third", + ], +} +`; + +exports[`getWorkspaceExtensionsInDependencyOrder with existing fixtures returns extensions in dependency order for launchql workspace 1`] = ` +{ + "external": [ + "plpgsql", + "uuid-ossp", + "pgcrypto", + ], + "resolved": [ + "plpgsql", + "uuid-ossp", + "pg-utilities", + "pg-verify", + "pgcrypto", + "totp", + "utils", + "secrets", + ], +} +`; + +exports[`getWorkspaceExtensionsInDependencyOrder with existing fixtures returns extensions in dependency order for simple-w-tags workspace 1`] = ` +{ + "external": [ + "citext", + "plpgsql", + "pgcrypto", + ], + "resolved": [ + "citext", + "plpgsql", + "pgcrypto", + "my-first", + "my-second", + "my-third", + ], +} +`; diff --git a/packages/core/__tests__/core/workspace-extensions-dependency-order.test.ts b/packages/core/__tests__/core/workspace-extensions-dependency-order.test.ts new file mode 100644 index 000000000..d6b7e8c2e --- /dev/null +++ b/packages/core/__tests__/core/workspace-extensions-dependency-order.test.ts @@ -0,0 +1,84 @@ +process.env.LAUNCHQL_DEBUG = 'true'; + +import { LaunchQLProject } from '../../src/core/class/launchql'; +import { TestFixture } from '../../test-utils'; + +let fixture: TestFixture; + +beforeAll(() => { + fixture = new TestFixture('sqitch'); +}); + +afterAll(() => { + fixture.cleanup(); +}); + +describe('getWorkspaceExtensionsInDependencyOrder', () => { + describe('with existing fixtures', () => { + it('returns extensions in dependency order for simple-w-tags workspace', () => { + const workspacePath = fixture.getFixturePath('simple-w-tags'); + const project = new LaunchQLProject(workspacePath); + + const result = project.resolveWorkspaceExtensionDependencies(); + expect(result).toMatchSnapshot(); + }); + + it('returns extensions in dependency order for launchql workspace', () => { + const workspacePath = fixture.getFixturePath('launchql'); + const project = new LaunchQLProject(workspacePath); + + const result = project.resolveWorkspaceExtensionDependencies(); + expect(result).toMatchSnapshot(); + }); + + it('handles workspace with minimal modules', () => { + const workspacePath = fixture.getFixturePath('simple'); + const project = new LaunchQLProject(workspacePath); + + const result = project.resolveWorkspaceExtensionDependencies(); + expect(result).toMatchSnapshot(); + + expect(result).toHaveProperty('resolved'); + expect(result).toHaveProperty('external'); + }); + }); + + describe('with complex existing fixtures', () => { + it('returns extensions in dependency order for complex workspace', () => { + const workspacePath = fixture.getFixturePath('launchql'); + const project = new LaunchQLProject(workspacePath); + + const result = project.resolveWorkspaceExtensionDependencies(); + expect(result).toMatchSnapshot(); + + expect(Array.isArray(result.resolved)).toBe(true); + expect(Array.isArray(result.external)).toBe(true); + }); + + it('verifies dependency ordering properties', () => { + const workspacePath = fixture.getFixturePath('simple-w-tags'); + const project = new LaunchQLProject(workspacePath); + + const result = project.resolveWorkspaceExtensionDependencies(); + expect(result).toMatchSnapshot(); + + expect(result).toHaveProperty('resolved'); + expect(result).toHaveProperty('external'); + expect(Array.isArray(result.resolved)).toBe(true); + expect(Array.isArray(result.external)).toBe(true); + + expect(result.resolved.length).toBeGreaterThanOrEqual(0); + expect(result.external.length).toBeGreaterThanOrEqual(0); + }); + + it('handles different fixture types consistently', () => { + const workspacePath = fixture.getFixturePath('simple'); + const project = new LaunchQLProject(workspacePath); + + const result = project.resolveWorkspaceExtensionDependencies(); + expect(result).toMatchSnapshot(); + + expect(result.resolved.length).toBeGreaterThan(0); + }); + }); +}); diff --git a/packages/core/__tests__/projects/deployment-scenarios.test.ts b/packages/core/__tests__/projects/deployment-scenarios.test.ts new file mode 100644 index 000000000..67f36a376 --- /dev/null +++ b/packages/core/__tests__/projects/deployment-scenarios.test.ts @@ -0,0 +1,51 @@ +process.env.LAUNCHQL_DEBUG = 'true'; + +import { TestDatabase } from '../../test-utils'; +import { CoreDeployTestFixture } from '../../test-utils/CoreDeployTestFixture'; + +describe('Deployment Scenarios with Undefined Targets', () => { + let fixture: CoreDeployTestFixture; + let db: TestDatabase; + + beforeEach(async () => { + fixture = new CoreDeployTestFixture('sqitch', 'simple-w-tags'); + db = await fixture.setupTestDatabase(); + }); + + afterEach(async () => { + await fixture.cleanup(); + }); + + test('handles undefined target scenarios for deploy, revert, and verify operations', async () => { + await fixture.deployModule(undefined as any, db.name, ['sqitch', 'simple-w-tags']); + + expect(await db.exists('schema', 'metaschema')).toBe(true); + expect(await db.exists('table', 'metaschema.customers')).toBe(true); + + await expect( + fixture.verifyModule(undefined as any, db.name, ['sqitch', 'simple-w-tags']) + ).resolves.not.toThrow(); + + await fixture.revertModule(undefined as any, db.name, ['sqitch', 'simple-w-tags']); + + expect(await db.exists('schema', 'metaschema')).toBe(false); + expect(await db.exists('table', 'metaschema.customers')).toBe(false); + + await fixture.deployModule('my-third', db.name, ['sqitch', 'simple-w-tags']); + + expect(await db.exists('schema', 'metaschema')).toBe(true); + expect(await db.exists('table', 'metaschema.customers')).toBe(true); + + await fixture.revertModule('my-first:@v1.0.0', db.name, ['sqitch', 'simple-w-tags']); + + expect(await db.exists('schema', 'metaschema')).toBe(false); + expect(await db.exists('table', 'metaschema.customers')).toBe(false); + + await fixture.deployModule(undefined as any, db.name, ['sqitch', 'simple-w-tags']); + + expect(await db.exists('schema', 'metaschema')).toBe(true); + expect(await db.exists('table', 'metaschema.customers')).toBe(true); + + await fixture.verifyModule(undefined as any, db.name, ['sqitch', 'simple-w-tags']); + }); +}); diff --git a/packages/core/__tests__/projects/revert-truncation-scenarios.test.ts b/packages/core/__tests__/projects/revert-truncation-scenarios.test.ts new file mode 100644 index 000000000..798a081f1 --- /dev/null +++ b/packages/core/__tests__/projects/revert-truncation-scenarios.test.ts @@ -0,0 +1,91 @@ +process.env.LAUNCHQL_DEBUG = 'true'; + +import { TestDatabase } from '../../test-utils'; +import { CoreDeployTestFixture } from '../../test-utils/CoreDeployTestFixture'; + +describe('Revert Truncation Scenarios', () => { + let fixture: CoreDeployTestFixture; + let db: TestDatabase; + + beforeEach(async () => { + fixture = new CoreDeployTestFixture('sqitch', 'simple-w-tags'); + db = await fixture.setupTestDatabase(); + }); + + afterAll(async () => { + await fixture.cleanup(); + }); + + test('deploys my-third and reverts to my-first with toChange - should revert all dependent modules', async () => { + await fixture.deployModule('my-third', db.name, ['sqitch', 'simple-w-tags']); + + expect(await db.exists('schema', 'myapp')).toBe(true); // my-first + expect(await db.exists('table', 'myapp.products')).toBe(true); // my-first @v1.1.0 + expect(await db.exists('schema', 'otherschema')).toBe(true); // my-second + expect(await db.exists('table', 'otherschema.users')).toBe(true); // my-second @v2.0.0 + expect(await db.exists('schema', 'metaschema')).toBe(true); // my-third + expect(await db.exists('table', 'metaschema.customers')).toBe(true); // my-third + + await fixture.revertModule('my-first:@v1.0.0', db.name, ['sqitch', 'simple-w-tags']); + + expect(await db.exists('schema', 'metaschema')).toBe(false); + expect(await db.exists('table', 'metaschema.customers')).toBe(false); + expect(await db.exists('schema', 'otherschema')).toBe(false); + expect(await db.exists('table', 'otherschema.users')).toBe(false); + + expect(await db.exists('schema', 'myapp')).toBe(true); + expect(await db.exists('table', 'myapp.users')).toBe(true); + expect(await db.exists('table', 'myapp.products')).toBe(false); // @v1.1.0 change reverted + }); + + test('deploys my-third and reverts my-second without toChange - should use module-specific resolution', async () => { + await fixture.deployModule('my-third', db.name, ['sqitch', 'simple-w-tags']); + + expect(await db.exists('schema', 'metaschema')).toBe(true); + expect(await db.exists('table', 'metaschema.customers')).toBe(true); + expect(await db.exists('schema', 'otherschema')).toBe(true); + expect(await db.exists('table', 'otherschema.users')).toBe(true); + + await fixture.revertModule('my-second', db.name, ['sqitch', 'simple-w-tags']); + + expect(await db.exists('schema', 'otherschema')).toBe(false); + expect(await db.exists('table', 'otherschema.users')).toBe(false); + + expect(await db.exists('schema', 'myapp')).toBe(true); + expect(await db.exists('table', 'myapp.products')).toBe(true); + + expect(await db.exists('schema', 'metaschema')).toBe(false); + }); + + test('null name with cwd inside my-second calling revert - should revert all modules', async () => { + await fixture.deployModule('my-third', db.name, ['sqitch', 'simple-w-tags']); + + expect(await db.exists('schema', 'metaschema')).toBe(true); + expect(await db.exists('schema', 'otherschema')).toBe(true); + expect(await db.exists('schema', 'myapp')).toBe(true); + + await fixture.revertModule(undefined as any, db.name, ['sqitch', 'simple-w-tags']); + + expect(await db.exists('schema', 'metaschema')).toBe(false); + expect(await db.exists('schema', 'otherschema')).toBe(false); + expect(await db.exists('schema', 'myapp')).toBe(false); + }); + + test('verifies workspace-wide resolution with toChange ensures proper dependency handling', async () => { + + await fixture.deployModule('my-third', db.name, ['sqitch', 'simple-w-tags']); + + expect(await db.exists('schema', 'myapp')).toBe(true); // my-first + expect(await db.exists('schema', 'otherschema')).toBe(true); // my-second + expect(await db.exists('schema', 'metaschema')).toBe(true); // my-third + + await fixture.revertModule('my-first:@v1.0.0', db.name, ['sqitch', 'simple-w-tags']); + + expect(await db.exists('schema', 'metaschema')).toBe(false); + expect(await db.exists('schema', 'otherschema')).toBe(false); + + expect(await db.exists('schema', 'myapp')).toBe(true); + expect(await db.exists('table', 'myapp.users')).toBe(true); + expect(await db.exists('table', 'myapp.products')).toBe(false); // @v1.1.0 change reverted + }); +}); diff --git a/packages/core/__tests__/resolution/dependency-resolution-basic.test.ts b/packages/core/__tests__/resolution/dependency-resolution-basic.test.ts index 946080953..7aa4ca711 100644 --- a/packages/core/__tests__/resolution/dependency-resolution-basic.test.ts +++ b/packages/core/__tests__/resolution/dependency-resolution-basic.test.ts @@ -1,5 +1,5 @@ import { listModules } from '../../src/modules/modules'; -import { extDeps,resolveDependencies } from '../../src/resolution/deps'; +import { resolveExtensionDependencies, resolveDependencies } from '../../src/resolution/deps'; import { TestFixture } from '../../test-utils'; let fixture: TestFixture; @@ -47,7 +47,7 @@ it('sqitch package dependencies [simple/3rd]', async () => { it('launchql project extensions dependencies', async () => { const modules = listModules(fixture.getFixturePath('launchql')); - const utils = await extDeps('utils', modules); + const utils = await resolveExtensionDependencies('utils', modules); expect(utils).toEqual({ external: ['plpgsql', 'uuid-ossp', 'pgcrypto'], resolved: [ @@ -61,7 +61,7 @@ it('launchql project extensions dependencies', async () => { ] }); - const secrets = await extDeps('secrets', modules); + const secrets = await resolveExtensionDependencies('secrets', modules); expect(secrets).toEqual({ external: ['plpgsql', 'uuid-ossp', 'pgcrypto'], resolved: [ @@ -75,7 +75,7 @@ it('launchql project extensions dependencies', async () => { ] }); - const totp = await extDeps('totp', modules); + const totp = await resolveExtensionDependencies('totp', modules); expect(totp).toEqual({ external: ['plpgsql', 'uuid-ossp', 'pgcrypto'], resolved: [ diff --git a/packages/core/src/core/class/launchql.ts b/packages/core/src/core/class/launchql.ts index a1919287c..485f8140c 100644 --- a/packages/core/src/core/class/launchql.ts +++ b/packages/core/src/core/class/launchql.ts @@ -33,7 +33,7 @@ import { ModuleMap } from '../../modules/modules'; import { packageModule } from '../../packaging/package'; -import { extDeps, resolveDependencies } from '../../resolution/deps'; +import { resolveExtensionDependencies, resolveDependencies } from '../../resolution/deps'; import { parseTarget } from '../../utils/target-utils'; @@ -340,7 +340,7 @@ export class LaunchQLProject { this.ensureModule(); const moduleName = this.getModuleName(); const moduleMap = this.getModuleMap(); - return extDeps(moduleName, moduleMap); + return resolveExtensionDependencies(moduleName, moduleMap); } getModuleDependencies(moduleName: string): { native: string[]; modules: string[] } { @@ -679,8 +679,59 @@ export class LaunchQLProject { // ──────────────── Project Operations ──────────────── - private parseProjectTarget(target?: string): { name: string; toChange: string | undefined } { - let name: string; + public resolveWorkspaceExtensionDependencies(): { resolved: string[]; external: string[] } { + const modules = this.getModuleMap(); + const allModuleNames = Object.keys(modules); + + if (allModuleNames.length === 0) { + return { resolved: [], external: [] }; + } + + // Create a virtual module that depends on all workspace modules + const virtualModuleName = '_virtual/workspace'; + const virtualModuleMap = { + ...modules, + [virtualModuleName]: { + requires: allModuleNames + } + }; + + const { resolved, external } = resolveExtensionDependencies(virtualModuleName, virtualModuleMap); + + // Filter out the virtual module and return the result + return { + resolved: resolved.filter((moduleName: string) => moduleName !== virtualModuleName), + external: external + }; + } + + /** + * Truncates workspace extensions to include only modules from the target onwards. + * This prevents processing unnecessary modules that come before the target in dependency order. + * + * @param workspaceExtensions - The full workspace extension dependencies + * @param targetName - The target module name to truncate from + * @returns Truncated extensions starting from the target module + */ + private truncateExtensionsToTarget( + workspaceExtensions: { resolved: string[]; external: string[] }, + targetName: string + ): { resolved: string[]; external: string[] } { + const targetIndex = workspaceExtensions.resolved.indexOf(targetName); + + if (targetIndex === -1) { + return workspaceExtensions; + } + + return { + resolved: workspaceExtensions.resolved.slice(targetIndex), + external: workspaceExtensions.external + }; + } + + + private parseProjectTarget(target?: string): { name: string | null; toChange: string | undefined } { + let name: string | null; let toChange: string | undefined; if (!target) { @@ -688,7 +739,12 @@ export class LaunchQLProject { if (context === ProjectContext.Module || context === ProjectContext.ModuleInsideWorkspace) { name = this.getModuleName(); } else if (context === ProjectContext.Workspace) { - throw new Error('Module name is required when running from workspace root'); + const modules = this.getModuleMap(); + const moduleNames = Object.keys(modules); + if (moduleNames.length === 0) { + throw new Error('No modules found in workspace'); + } + name = null; // Indicates workspace-wide operation } else { throw new Error('Not in a LaunchQL workspace or module'); } @@ -724,11 +780,20 @@ export class LaunchQLProject { }; const modules = this.getModuleMap(); - const moduleProject = this.getModuleProject(name); - const extensions = moduleProject.getModuleExtensions(); + + let extensions: { resolved: string[]; external: string[] }; + + if (name === null) { + // When name is null, deploy ALL modules in the workspace + extensions = this.resolveWorkspaceExtensionDependencies(); + } else { + const moduleProject = this.getModuleProject(name); + extensions = moduleProject.getModuleExtensions(); + } const pgPool = getPgPool(opts.pg); + const targetDescription = name === null ? 'all modules' : name; log.success(`🚀 Starting deployment to database ${opts.pg.database}...`); for (const extension of extensions.resolved) { @@ -790,6 +855,7 @@ export class LaunchQLProject { try { const client = new LaunchQLMigrate(opts.pg as PgConfig); + // Only apply toChange to the target module, not its dependencies const moduleToChange = extension === name ? toChange : undefined; const result = await client.deploy({ modulePath, @@ -814,8 +880,11 @@ export class LaunchQLProject { } } - log.success(`✅ Deployment complete for ${name}.`); + log.success(`✅ Deployment complete for ${targetDescription}.`); } else { + if (name === null) { + throw new Error('Cannot perform non-recursive operation on workspace. Use recursive=true or specify a target module.'); + } const moduleProject = this.getModuleProject(name); const modulePath = moduleProject.getModulePath(); if (!modulePath) { @@ -838,6 +907,11 @@ export class LaunchQLProject { } } + /** + * Reverts database changes for modules. Unlike verify operations, revert operations + * modify database state and must ensure dependent modules are reverted before their + * dependencies to prevent database constraint violations. + */ async revert( opts: LaunchQLOptions, target?: string, @@ -853,42 +927,19 @@ export class LaunchQLProject { // Mirror deploy logic: find all modules that depend on the target module let extensionsToRevert: { resolved: string[]; external: string[] }; - if (toChange) { - const allModuleNames = Object.keys(modules); - const dependentModules = new Set(); - - for (const moduleName of allModuleNames) { - const moduleProject = this.getModuleProject(moduleName); - const moduleExtensions = moduleProject.getModuleExtensions(); - if (moduleExtensions.resolved.includes(name)) { - dependentModules.add(moduleName); - } - } - - if (dependentModules.size === 0) { - dependentModules.add(name); - } - - let maxDependencies = 0; - let topModule = name; - for (const depModule of dependentModules) { - const moduleProject = this.getModuleProject(depModule); - const moduleExtensions = moduleProject.getModuleExtensions(); - if (moduleExtensions.resolved.length > maxDependencies) { - maxDependencies = moduleExtensions.resolved.length; - topModule = depModule; - } - } - - const topModuleProject = this.getModuleProject(topModule); - extensionsToRevert = topModuleProject.getModuleExtensions(); + if (name === null) { + // When name is null, revert ALL modules in the workspace + extensionsToRevert = this.resolveWorkspaceExtensionDependencies(); } else { - const moduleProject = this.getModuleProject(name); - extensionsToRevert = moduleProject.getModuleExtensions(); + // Always use workspace-wide resolution to prevent "Cannot revert X: required by Y" database dependency violations. + // This ensures all dependent modules are reverted before their dependencies. + const workspaceExtensions = this.resolveWorkspaceExtensionDependencies(); + extensionsToRevert = this.truncateExtensionsToTarget(workspaceExtensions, name); } const pgPool = getPgPool(opts.pg); + const targetDescription = name === null ? 'all modules' : name; log.success(`🧹 Starting revert process on database ${opts.pg.database}...`); const reversedExtensions = [...extensionsToRevert.resolved].reverse(); @@ -914,7 +965,7 @@ export class LaunchQLProject { try { const client = new LaunchQLMigrate(opts.pg as PgConfig); - // Mirror deploy logic: apply toChange to target module, undefined to dependencies + // Only apply toChange to the target module, not its dependencies const moduleToChange = extension === name ? toChange : undefined; const result = await client.revert({ modulePath, @@ -937,8 +988,11 @@ export class LaunchQLProject { } } - log.success(`✅ Revert complete for ${name}.`); + log.success(`✅ Revert complete for ${targetDescription}.`); } else { + if (name === null) { + throw new Error('Cannot perform non-recursive operation on workspace. Use recursive=true or specify a target module.'); + } const moduleProject = this.getModuleProject(name); const modulePath = moduleProject.getModulePath(); if (!modulePath) { @@ -971,12 +1025,21 @@ export class LaunchQLProject { if (recursive) { const modules = this.getModuleMap(); - const moduleProject = this.getModuleProject(name); - const extensions = moduleProject.getModuleExtensions(); + + let extensions: { resolved: string[]; external: string[] }; + + if (name === null) { + // When name is null, verify ALL modules in the workspace + extensions = this.resolveWorkspaceExtensionDependencies(); + } else { + const moduleProject = this.getModuleProject(name); + extensions = moduleProject.getModuleExtensions(); + } const pgPool = getPgPool(opts.pg); - log.success(`🔎 Verifying deployment of ${name} on database ${opts.pg.database}...`); + const targetDescription = name === null ? 'all modules' : name; + log.success(`🔎 Verifying deployment of ${targetDescription} on database ${opts.pg.database}...`); for (const extension of extensions.resolved) { try { @@ -991,7 +1054,7 @@ export class LaunchQLProject { try { const client = new LaunchQLMigrate(opts.pg as PgConfig); - // Only apply toChange to the target module being verified, not its dependencies. + // Only apply toChange to the target module, not its dependencies const moduleToChange = extension === name ? toChange : undefined; const result = await client.verify({ modulePath, @@ -1013,8 +1076,11 @@ export class LaunchQLProject { } } - log.success(`✅ Verification complete for ${name}.`); + log.success(`✅ Verification complete for ${targetDescription}.`); } else { + if (name === null) { + throw new Error('Cannot perform non-recursive operation on workspace. Use recursive=true or specify a target module.'); + } const moduleProject = this.getModuleProject(name); const modulePath = moduleProject.getModulePath(); if (!modulePath) { diff --git a/packages/core/src/resolution/deps.ts b/packages/core/src/resolution/deps.ts index a55ee4c04..56c8609c5 100644 --- a/packages/core/src/resolution/deps.ts +++ b/packages/core/src/resolution/deps.ts @@ -36,7 +36,7 @@ export interface DependencyResult { interface DependencyResolverOptions { /** * Function to handle external dependencies when they are not found in the dependency graph. - * Used by extDeps to add external dependencies to the external array. + * Used by resolveExtensionDependencies to add external dependencies to the external array. * @param dep - The dependency module name that was not found * @param deps - The dependency graph to potentially add the dependency to * @param external - Array to collect external dependencies @@ -70,7 +70,7 @@ interface DependencyResolverOptions { /** * Core dependency resolution algorithm that handles circular dependency detection * and topological sorting of dependencies. This unified implementation eliminates - * code duplication between getDeps, extDeps, and resolveDependencies. + * code duplication between getDeps, resolveExtensionDependencies, and resolveDependencies. * * @param deps - The dependency graph mapping modules to their dependencies * @param external - Array to collect external dependencies @@ -157,7 +157,7 @@ const makeKey = (sqlmodule: string): string => `/deploy/${sqlmodule}.sql`; * @param modules - Record mapping module names to their dependency requirements * @returns Object containing external dependencies and resolved dependency order */ -export const extDeps = ( +export const resolveExtensionDependencies = ( name: string, modules: Record ): { external: string[]; resolved: string[] } => { @@ -171,16 +171,16 @@ export const extDeps = ( return memo; }, {} as DependencyGraph); - // Handle external dependencies for extDeps - simpler than getDeps + // Handle external dependencies for resolveExtensionDependencies - simpler than getDeps const handleExternalDep = (dep: string, deps: DependencyGraph, external: string[]): void => { external.push(dep); deps[dep] = []; }; - // Create the dependency resolver with extDeps-specific configuration + // Create the dependency resolver with resolveExtensionDependencies-specific configuration const dep_resolve = createDependencyResolver(deps, external, { handleExternalDep, - extname: name // For extDeps, we use the module name as extname + extname: name // For resolveExtensionDependencies, we use the module name as extname }); const resolved: string[] = []; @@ -516,16 +516,16 @@ export const resolveDependencies = ( const unresolved: string[] = []; // Add synthetic root node - exactly as in original - deps[makeKey('apps/index')] = Object.keys(deps) + deps[makeKey('_virtual/app')] = Object.keys(deps) .filter((dep) => dep.startsWith('/deploy/')) .map((dep) => dep.replace(/^\/deploy\//, '').replace(/\.sql$/, '')); - dep_resolve('apps/index', resolved, unresolved); + dep_resolve('_virtual/app', resolved, unresolved); // Remove synthetic root - const index = resolved.indexOf('apps/index'); + const index = resolved.indexOf('_virtual/app'); resolved.splice(index, 1); - delete deps[makeKey('apps/index')]; + delete deps[makeKey('_virtual/app')]; // Sort extensions first - exactly as in original const extensions = resolved.filter((module) => module.startsWith('extensions/')); diff --git a/packages/core/test-utils/CoreDeployTestFixture.ts b/packages/core/test-utils/CoreDeployTestFixture.ts index 297486c59..007abf2be 100644 --- a/packages/core/test-utils/CoreDeployTestFixture.ts +++ b/packages/core/test-utils/CoreDeployTestFixture.ts @@ -87,9 +87,6 @@ export class CoreDeployTestFixture extends TestFixture { } async cleanup(): Promise { - for (const db of this.databases) { - await db.close(); - } this.databases = []; if (this.migrateFixture) {