mirror of
https://github.com/t8y2/dbx.git
synced 2026-10-19 16:16:45 +00:00
fix(schema-diff): skip unsafe metadata queries for views
This commit is contained in:
@@ -18,7 +18,7 @@ import SchemaDiffOptionsPanel from "@/components/diff/SchemaDiffOptionsPanel.vue
|
||||
|
||||
import { getSchemaDiffOptionsForDbType } from "@/lib/schema/schemaDiffOptions";
|
||||
import { buildDeployTxResult } from "@/lib/schema/deployTxResult";
|
||||
import { createConcurrencyLimiter, mapWithConcurrency, schemaDiffMetadataConcurrency, shouldFetchSchemaDiffDdl } from "@/lib/schema/schemaDiffMetadataLoad";
|
||||
import { createConcurrencyLimiter, mapWithConcurrency, schemaDiffMetadataConcurrency, schemaDiffMetadataLoadPlan } from "@/lib/schema/schemaDiffMetadataLoad";
|
||||
import { normalizeSchemaDiffCompareOptions } from "@/types/schemaDiff";
|
||||
import type { SchemaDiffCompareOptions, SchemaDiffConfig, FieldMappingEntry } from "@/types/schemaDiff";
|
||||
import type { ObjectSourceKind, TableInfo } from "@/types/database";
|
||||
@@ -43,7 +43,7 @@ import {
|
||||
type PermissionDiff,
|
||||
type DependencyGraph,
|
||||
} from "@/lib/schema/schemaDiff";
|
||||
import { compileSchemaDiffTableFilter, filterSchemaDiffTables } from "@/lib/schema/schemaDiffTableFilter";
|
||||
import { compileSchemaDiffTableFilter, filterSchemaDiffTables, isSchemaDiffView } from "@/lib/schema/schemaDiffTableFilter";
|
||||
import { Splitpanes, Pane } from "splitpanes";
|
||||
import "splitpanes/dist/splitpanes.css";
|
||||
|
||||
@@ -350,23 +350,19 @@ interface SchemaDetailLoadContext {
|
||||
options: SchemaDiffCompareOptions;
|
||||
}
|
||||
|
||||
function shouldLoadIndexes(options: SchemaDiffCompareOptions): boolean {
|
||||
return options.indexes || options.primaryKeys || options.uniqueKeys;
|
||||
}
|
||||
|
||||
async function loadSchemaDetails(tables: TableInfo[], context: SchemaDetailLoadContext): Promise<TableSchemaDetail[]> {
|
||||
const concurrency = schemaDiffMetadataConcurrency(context.dbType, tables.length);
|
||||
const runMetadataQuery = createConcurrencyLimiter(concurrency);
|
||||
|
||||
return mapWithConcurrency(tables, concurrency, async (table) => {
|
||||
const objectType = isViewOrMaterializedView(table.table_type);
|
||||
const shouldFetchDdl = shouldFetchSchemaDiffDdl(Boolean(objectType), context.options);
|
||||
const ddlPromise = shouldFetchDdl ? runMetadataQuery(() => api.getTableDdl(context.connectionId, context.database, context.schema, table.name, objectType)) : Promise.resolve("");
|
||||
const loadPlan = schemaDiffMetadataLoadPlan(isSchemaDiffView(table), context.options);
|
||||
const ddlPromise = loadPlan.ddl ? runMetadataQuery(() => api.getTableDdl(context.connectionId, context.database, context.schema, table.name, objectType)) : Promise.resolve("");
|
||||
const [columns, indexes, foreignKeys, triggers, ddl] = await Promise.all([
|
||||
runMetadataQuery(() => api.getColumns(context.connectionId, context.database, context.schema, table.name)),
|
||||
shouldLoadIndexes(context.options) ? runMetadataQuery(() => api.listIndexes(context.connectionId, context.database, context.schema, table.name)) : Promise.resolve([]),
|
||||
context.options.foreignKeys ? runMetadataQuery(() => api.listForeignKeys(context.connectionId, context.database, context.schema, table.name)) : Promise.resolve([]),
|
||||
context.options.triggers ? runMetadataQuery(() => api.listTriggers(context.connectionId, context.database, context.schema, table.name)) : Promise.resolve([]),
|
||||
loadPlan.columns ? runMetadataQuery(() => api.getColumns(context.connectionId, context.database, context.schema, table.name)) : Promise.resolve([]),
|
||||
loadPlan.indexes ? runMetadataQuery(() => api.listIndexes(context.connectionId, context.database, context.schema, table.name)) : Promise.resolve([]),
|
||||
loadPlan.foreignKeys ? runMetadataQuery(() => api.listForeignKeys(context.connectionId, context.database, context.schema, table.name)) : Promise.resolve([]),
|
||||
loadPlan.triggers ? runMetadataQuery(() => api.listTriggers(context.connectionId, context.database, context.schema, table.name)) : Promise.resolve([]),
|
||||
ddlPromise,
|
||||
]);
|
||||
|
||||
@@ -399,7 +395,7 @@ async function handleCompare() {
|
||||
await store.ensureConnected(targetConnectionId.value);
|
||||
|
||||
const [srcTables, tgtTables] = await Promise.all([api.listTables(sourceConnectionId.value, sourceDatabase.value, sourceSchema.value), api.listTables(targetConnectionId.value, targetDatabase.value, targetSchema.value)]);
|
||||
const { sourceTables, targetTables } = filterSchemaDiffTables(srcTables, tgtTables, tableFilter);
|
||||
const { sourceTables, targetTables } = filterSchemaDiffTables(srcTables, tgtTables, tableFilter, opts);
|
||||
|
||||
const sourceDetails = await loadSchemaDetails(sourceTables, {
|
||||
connectionId: sourceConnectionId.value,
|
||||
|
||||
@@ -14,3 +14,14 @@
|
||||
- `diagram`, `document`, `export`, `imports`: feature-specific utilities that are shared by more than one component.
|
||||
|
||||
Tests under `src/lib/__tests__` mirror the same domain folders. When moving a module, update both runtime imports and colocated tests so stale root-level aliases do not return.
|
||||
|
||||
## Schema Diff Metadata Guardrails
|
||||
|
||||
Schema comparison must filter disabled object types before loading metadata. Tables load columns and the relational metadata enabled by the compare options; views load DDL only.
|
||||
|
||||
Do not call column, index, foreign-key, or trigger metadata APIs for views. MySQL 5.7 can validate a `SQL SECURITY DEFINER` view during column discovery and return error 1143 when its definer account no longer exists, even though `SHOW CREATE VIEW` still succeeds. Keep view detection centralized in `schema/schemaDiffTableFilter.ts` and preserve the DDL-only plan in `schema/schemaDiffMetadataLoad.ts`.
|
||||
|
||||
Regression coverage belongs in:
|
||||
|
||||
- `src/lib/__tests__/schema/schemaDiffTableFilter.spec.ts`
|
||||
- `src/lib/__tests__/schema/schemaDiffMetadataLoad.spec.ts`
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { createConcurrencyLimiter, mapWithConcurrency, schemaDiffMetadataConcurrency, shouldFetchSchemaDiffDdl } from "@/lib/schema/schemaDiffMetadataLoad";
|
||||
import { createConcurrencyLimiter, mapWithConcurrency, schemaDiffMetadataConcurrency, schemaDiffMetadataLoadPlan, shouldFetchSchemaDiffDdl } from "@/lib/schema/schemaDiffMetadataLoad";
|
||||
|
||||
function wait(ms: number) {
|
||||
return new Promise<void>((resolve) => setTimeout(resolve, ms));
|
||||
@@ -24,6 +24,66 @@ describe("schemaDiffMetadataLoad", () => {
|
||||
expect(shouldFetchSchemaDiffDdl(false, { tables: true, views: false })).toBe(true);
|
||||
});
|
||||
|
||||
it("uses DDL-only metadata for views so invalid definers cannot break column discovery", () => {
|
||||
expect(
|
||||
schemaDiffMetadataLoadPlan(true, {
|
||||
tables: true,
|
||||
views: true,
|
||||
indexes: true,
|
||||
primaryKeys: true,
|
||||
uniqueKeys: true,
|
||||
foreignKeys: true,
|
||||
triggers: true,
|
||||
}),
|
||||
).toEqual({
|
||||
columns: false,
|
||||
indexes: false,
|
||||
foreignKeys: false,
|
||||
triggers: false,
|
||||
ddl: true,
|
||||
});
|
||||
});
|
||||
|
||||
it("does not load any metadata for disabled views", () => {
|
||||
expect(
|
||||
schemaDiffMetadataLoadPlan(true, {
|
||||
tables: true,
|
||||
views: false,
|
||||
indexes: true,
|
||||
primaryKeys: true,
|
||||
uniqueKeys: true,
|
||||
foreignKeys: true,
|
||||
triggers: true,
|
||||
}),
|
||||
).toEqual({
|
||||
columns: false,
|
||||
indexes: false,
|
||||
foreignKeys: false,
|
||||
triggers: false,
|
||||
ddl: false,
|
||||
});
|
||||
});
|
||||
|
||||
it("preserves enabled relational metadata for tables", () => {
|
||||
expect(
|
||||
schemaDiffMetadataLoadPlan(false, {
|
||||
tables: true,
|
||||
views: false,
|
||||
indexes: false,
|
||||
primaryKeys: true,
|
||||
uniqueKeys: false,
|
||||
foreignKeys: true,
|
||||
triggers: false,
|
||||
}),
|
||||
).toEqual({
|
||||
columns: true,
|
||||
indexes: true,
|
||||
foreignKeys: true,
|
||||
triggers: false,
|
||||
ddl: true,
|
||||
});
|
||||
});
|
||||
|
||||
it("maps items with limited concurrency and preserves output order", async () => {
|
||||
let active = 0;
|
||||
let maxActive = 0;
|
||||
|
||||
@@ -0,0 +1,47 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { compileSchemaDiffTableFilter, filterSchemaDiffTables, isSchemaDiffView } from "@/lib/schema/schemaDiffTableFilter";
|
||||
import { DEFAULT_MYSQL_OPTIONS } from "@/types/schemaDiff";
|
||||
import type { TableInfo } from "@/types/database";
|
||||
|
||||
const tables: TableInfo[] = [
|
||||
{ name: "users", table_type: "BASE TABLE" },
|
||||
{ name: "active_users", table_type: "VIEW" },
|
||||
{ name: "monthly_users", table_type: "MATERIALIZED VIEW" },
|
||||
];
|
||||
|
||||
describe("schemaDiffTableFilter", () => {
|
||||
it("recognizes regular and materialized views", () => {
|
||||
expect(isSchemaDiffView(tables[0])).toBe(false);
|
||||
expect(isSchemaDiffView(tables[1])).toBe(true);
|
||||
expect(isSchemaDiffView(tables[2])).toBe(true);
|
||||
});
|
||||
|
||||
it("completely excludes views when view comparison is disabled", () => {
|
||||
const options = { ...DEFAULT_MYSQL_OPTIONS, tables: true, views: false };
|
||||
const result = filterSchemaDiffTables(tables, tables, compileSchemaDiffTableFilter(options), options);
|
||||
|
||||
expect(result.sourceTables.map((table) => table.name)).toEqual(["users"]);
|
||||
expect(result.targetTables.map((table) => table.name)).toEqual(["users"]);
|
||||
});
|
||||
|
||||
it("completely excludes tables when table comparison is disabled", () => {
|
||||
const options = { ...DEFAULT_MYSQL_OPTIONS, tables: false, views: true };
|
||||
const result = filterSchemaDiffTables(tables, tables, compileSchemaDiffTableFilter(options), options);
|
||||
|
||||
expect(result.sourceTables.map((table) => table.name)).toEqual(["active_users", "monthly_users"]);
|
||||
expect(result.targetTables.map((table) => table.name)).toEqual(["active_users", "monthly_users"]);
|
||||
});
|
||||
|
||||
it("applies name filters after object type options", () => {
|
||||
const options = {
|
||||
...DEFAULT_MYSQL_OPTIONS,
|
||||
tables: true,
|
||||
views: true,
|
||||
tableIncludePattern: "users$",
|
||||
tableExcludePattern: "^active_",
|
||||
};
|
||||
const result = filterSchemaDiffTables(tables, tables, compileSchemaDiffTableFilter(options), options);
|
||||
|
||||
expect(result.sourceTables.map((table) => table.name)).toEqual(["users", "monthly_users"]);
|
||||
});
|
||||
});
|
||||
@@ -24,6 +24,34 @@ export function shouldFetchSchemaDiffDdl(isView: boolean, options: Pick<SchemaDi
|
||||
return isView ? options.views : options.tables;
|
||||
}
|
||||
|
||||
export interface SchemaDiffMetadataLoadPlan {
|
||||
columns: boolean;
|
||||
indexes: boolean;
|
||||
foreignKeys: boolean;
|
||||
triggers: boolean;
|
||||
ddl: boolean;
|
||||
}
|
||||
|
||||
export function schemaDiffMetadataLoadPlan(isView: boolean, options: Pick<SchemaDiffCompareOptions, "tables" | "views" | "indexes" | "primaryKeys" | "uniqueKeys" | "foreignKeys" | "triggers">): SchemaDiffMetadataLoadPlan {
|
||||
if (isView) {
|
||||
return {
|
||||
columns: false,
|
||||
indexes: false,
|
||||
foreignKeys: false,
|
||||
triggers: false,
|
||||
ddl: shouldFetchSchemaDiffDdl(true, options),
|
||||
};
|
||||
}
|
||||
|
||||
return {
|
||||
columns: options.tables,
|
||||
indexes: options.tables && (options.indexes || options.primaryKeys || options.uniqueKeys),
|
||||
foreignKeys: options.tables && options.foreignKeys,
|
||||
triggers: options.tables && options.triggers,
|
||||
ddl: shouldFetchSchemaDiffDdl(false, options),
|
||||
};
|
||||
}
|
||||
|
||||
export async function mapWithConcurrency<T, R>(items: readonly T[], limit: number, worker: (item: T, index: number) => Promise<R>): Promise<R[]> {
|
||||
const workerCount = Math.min(normalizeConcurrencyLimit(limit), items.length);
|
||||
if (workerCount === 0) return [];
|
||||
|
||||
@@ -41,9 +41,19 @@ export function matchesSchemaDiffTableFilter(tableName: string, filter: Compiled
|
||||
return includeMatches && !excludeMatches;
|
||||
}
|
||||
|
||||
export function filterSchemaDiffTables(sourceTables: TableInfo[], targetTables: TableInfo[], filter: CompiledSchemaDiffTableFilter): FilteredSchemaDiffTables {
|
||||
export function isSchemaDiffView(table: Pick<TableInfo, "table_type">): boolean {
|
||||
return table.table_type.toUpperCase().replace(/\s+/g, "_").includes("VIEW");
|
||||
}
|
||||
|
||||
function isSchemaDiffObjectEnabled(table: TableInfo, options: Pick<SchemaDiffCompareOptions, "tables" | "views">): boolean {
|
||||
return isSchemaDiffView(table) ? options.views : options.tables;
|
||||
}
|
||||
|
||||
export function filterSchemaDiffTables(sourceTables: TableInfo[], targetTables: TableInfo[], filter: CompiledSchemaDiffTableFilter, options: Pick<SchemaDiffCompareOptions, "tables" | "views"> = { tables: true, views: true }): FilteredSchemaDiffTables {
|
||||
const includeTable = (table: TableInfo) => isSchemaDiffObjectEnabled(table, options) && matchesSchemaDiffTableFilter(table.name, filter);
|
||||
|
||||
return {
|
||||
sourceTables: sourceTables.filter((table) => matchesSchemaDiffTableFilter(table.name, filter)),
|
||||
targetTables: targetTables.filter((table) => matchesSchemaDiffTableFilter(table.name, filter)),
|
||||
sourceTables: sourceTables.filter(includeTable),
|
||||
targetTables: targetTables.filter(includeTable),
|
||||
};
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user