From bb5ea45470d117a2ccc55218df0384db65c534bb Mon Sep 17 00:00:00 2001 From: zipg Date: Tue, 29 Sep 2026 21:04:41 +0800 Subject: [PATCH] fix(structure): trim trailing spaces from edited column names --- .../structure/TableStructureEditor.vue | 23 +- ...ructureEditor.columnNameWhitespace.spec.ts | 370 ++++++++++++++++++ .../table/tableStructureEditorState.spec.ts | 23 ++ .../lib/table/tableStructureEditorState.ts | 15 + 4 files changed, 427 insertions(+), 4 deletions(-) create mode 100644 apps/desktop/src/components/structure/__tests__/TableStructureEditor.columnNameWhitespace.spec.ts diff --git a/apps/desktop/src/components/structure/TableStructureEditor.vue b/apps/desktop/src/components/structure/TableStructureEditor.vue index e38493ac1c..05027a47db 100644 --- a/apps/desktop/src/components/structure/TableStructureEditor.vue +++ b/apps/desktop/src/components/structure/TableStructureEditor.vue @@ -129,6 +129,7 @@ import { copySourceColumnDetails, matchesCopySourceColumnSearch, foldCreatedTableName, + draftColumnNameForSql, } from "@/lib/table/tableStructureEditorState"; import { CREATE_DATABASE_CHARSET_OPTIONS, createDatabaseCollationOptionsForCharset, fallbackCreateDatabaseCharsetMetadata, normalizeCreateDatabaseCharsetKey, parseCreateDatabaseCharsetMetadata } from "@/lib/database/createDatabaseCharsetOptions"; import type { CreateDatabaseCharsetMetadata } from "@/lib/database/createDatabaseCharsetOptions"; @@ -2240,9 +2241,16 @@ function structureChangeOptions(): BuildTableStructureChangeSqlOptions { driverProfile: connection.value?.driver_profile, schema: props.schema, tableName: isCreateMode.value ? newTableName.value.trim() : props.tableName || "", - // Do not let a draft created by an older build submit properties that the - // current database cannot represent (notably PostgreSQL-style identity on openGauss). - columns: showExtendedProperties.value ? columns.value : columns.value.map((column) => ({ ...column, extra: {} })), + // User-entered names are normalized here so the preview and the executed + // batch agree: MySQL rejects identifiers that end with a space (ERROR 1166), + // while a metadata name the user never touched keeps its exact spelling. + columns: columns.value.map((column) => ({ + ...column, + name: draftColumnNameForSql(column.name, column.original?.name), + // Do not let a draft created by an older build submit properties that the + // current database cannot represent (notably PostgreSQL-style identity on openGauss). + ...(showExtendedProperties.value ? {} : { extra: {} }), + })), indexes: sanitizeStructureIndexesForCapabilities(indexes.value, structureCapabilities.value), foreignKeys: foreignKeys.value, triggers: triggers.value, @@ -3955,6 +3963,13 @@ function isColumnNameDisabled(column: EditableStructureColumn): boolean { return column.markedForDrop || (!!column.original && !structureCapabilities.value.renameColumn); } +/** Mirror in the input what the DDL builder does with the name, so the field the + * user sees matches the statement that will run. */ +function commitColumnNameInput(column: EditableStructureColumn) { + const normalized = draftColumnNameForSql(column.name, column.original?.name); + if (normalized !== column.name) column.name = normalized; +} + function isColumnTypeDisabled(column: EditableStructureColumn): boolean { return column.markedForDrop || (!!column.original && !structureCapabilities.value.alterType); } @@ -5289,7 +5304,7 @@ watch( - + ({ + connection: { + id: "structure-charset-test", + name: "MySQL", + db_type: "mysql", + driver_profile: "mysql", + driver_label: "MySQL", + }, + ensureConnected: vi.fn(), + executeQuery: vi.fn(), + listDataTypes: vi.fn(), + buildTableStructureChangeSql: vi.fn(), + buildMysqlAutoIncrementSql: vi.fn(), + getMysqlTableAutoIncrement: vi.fn(), + executeBatch: vi.fn(), + updateEditorSettings: vi.fn(), + loadObjectDdl: vi.fn(), + invalidateObjectDdl: vi.fn(), + loadObjectMetadataFacet: vi.fn(), + invalidateObjectMetadataCache: vi.fn(), + invalidateTableMetadataCache: vi.fn(), + toast: vi.fn(), +})); + +vi.mock("vue-i18n", () => ({ useI18n: () => ({ t: (key: string) => key }) })); + +vi.mock("@lucide/vue", async () => { + const { defineComponent, h } = await import("vue"); + const Icon = defineComponent({ name: "Icon", setup: () => () => h("span") }); + return { + AlertTriangle: Icon, + Check: Icon, + ChevronDown: Icon, + ChevronUp: Icon, + ClipboardList: Icon, + Copy: Icon, + Database: Icon, + Info: Icon, + Keyboard: Icon, + KeyRound: Icon, + ListChevronsUpDown: Icon, + Loader2: Icon, + Maximize2: Icon, + Pencil: Icon, + Plus: Icon, + RefreshCw: Icon, + Rows3: Icon, + Save: Icon, + Search: Icon, + Settings: Icon, + SlidersHorizontal: Icon, + Trash2: Icon, + UserRound: Icon, + X: Icon, + }; +}); + +vi.mock("@/components/ui/button", async () => { + const { defineComponent, h } = await import("vue"); + return { + Button: defineComponent({ + name: "Button", + inheritAttrs: false, + setup: + (_props, { attrs, slots }) => + () => + h("button", attrs, slots.default?.()), + }), + }; +}); +vi.mock("@/components/ui/input", async () => { + const { defineComponent, h } = await import("vue"); + return { + Input: defineComponent({ + name: "Input", + inheritAttrs: false, + props: { modelValue: { type: [String, Number], default: "" } }, + emits: ["update:modelValue"], + setup: + (props, { attrs, emit }) => + () => + h("input", { + ...attrs, + value: props.modelValue, + onInput: (event: Event) => emit("update:modelValue", (event.target as HTMLInputElement).value), + }), + }), + }; +}); +vi.mock("@/components/ui/badge", async () => { + const { defineComponent, h } = await import("vue"); + return { + Badge: defineComponent({ + name: "Badge", + inheritAttrs: false, + setup: + (_props, { attrs, slots }) => + () => + h("span", attrs, slots.default?.()), + }), + }; +}); +vi.mock("@/components/ui/tabs", async () => { + const { defineComponent, h } = await import("vue"); + const Div = defineComponent({ + inheritAttrs: false, + setup: + (_props, { attrs, slots }) => + () => + h("div", attrs, slots.default?.()), + }); + const Button = defineComponent({ + inheritAttrs: false, + setup: + (_props, { attrs, slots }) => + () => + h("button", attrs, slots.default?.()), + }); + return { Tabs: Div, TabsContent: Div, TabsList: Div, TabsTrigger: Button }; +}); +vi.mock("@/components/ui/dropdown-menu", async () => { + const { defineComponent, h } = await import("vue"); + const Div = defineComponent({ + inheritAttrs: false, + setup: + (_props, { attrs, slots }) => + () => + h("div", attrs, slots.default?.()), + }); + const Button = defineComponent({ + inheritAttrs: false, + setup: + (_props, { attrs, slots }) => + () => + h("button", attrs, slots.default?.()), + }); + return { DropdownMenu: Div, DropdownMenuCheckboxItem: Div, DropdownMenuContent: Div, DropdownMenuItem: Button, DropdownMenuTrigger: Div }; +}); +vi.mock("@/components/ui/popover", async () => { + const { defineComponent, h } = await import("vue"); + const Div = defineComponent({ + inheritAttrs: false, + setup: + (_props, { attrs, slots }) => + () => + h("div", attrs, slots.default?.()), + }); + return { Popover: Div, PopoverContent: Div, PopoverTrigger: Div }; +}); +vi.mock("@/components/ui/tooltip", async () => { + const { defineComponent, h } = await import("vue"); + const Div = defineComponent({ + inheritAttrs: false, + setup: + (_props, { attrs, slots }) => + () => + h("div", attrs, slots.default?.()), + }); + return { Tooltip: Div, TooltipContent: Div, TooltipTrigger: Div }; +}); +vi.mock("@/components/ui/searchable-select", async () => { + const { defineComponent, h } = await import("vue"); + return { + SearchableSelect: defineComponent({ + name: "SearchableSelect", + inheritAttrs: false, + props: { + modelValue: { type: String, default: "" }, + options: { type: Array, default: () => [] }, + allowCustom: { type: Boolean, default: false }, + }, + emits: ["update:modelValue"], + setup: + (props, { attrs, emit }) => + () => + h("button", { + ...attrs, + type: "button", + "data-searchable-select": "true", + "data-model-value": props.modelValue, + "data-options": JSON.stringify(props.options), + "data-allow-custom": String(props.allowCustom), + onClick: () => emit("update:modelValue", "custom_domain"), + }), + }), + }; +}); +vi.mock("@/components/ui/select", async () => { + const { defineComponent, h } = await import("vue"); + const Div = defineComponent({ + inheritAttrs: false, + setup: + (_props, { attrs, slots }) => + () => + h("div", attrs, slots.default?.()), + }); + return { Select: Div, SelectContent: Div, SelectItem: Div, SelectTrigger: Div, SelectValue: Div }; +}); + +vi.mock("@/stores/connectionStore", () => ({ + useConnectionStore: () => ({ + ensureConnected: mocks.ensureConnected, + getConfig: (connectionId: string) => (connectionId === mocks.connection.id ? mocks.connection : undefined), + }), +})); +vi.mock("@/stores/productionSafetyStore", () => ({ useProductionSafetyStore: () => ({ requestConfirmation: vi.fn() }) })); +vi.mock("@/stores/queryStore", () => ({ useQueryStore: () => ({ tableStructureRefreshVersion: () => 0 }) })); +vi.mock("@/stores/historyStore", () => ({ useHistoryStore: () => ({ add: vi.fn() }) })); +vi.mock("@/stores/settingsStore", () => ({ + useSettingsStore: () => ({ + editorSettings: { structureEditorDensity: "compact", sqlFormatter: {}, tableColumnTemplateFields: [], generateSqlQuoteIdentifiers: true }, + updateEditorSettings: mocks.updateEditorSettings, + }), +})); +vi.mock("@/composables/useTheme", () => ({ useTheme: () => ({ isDark: { value: false } }) })); +vi.mock("@/composables/useToast", () => ({ useToast: () => ({ toast: mocks.toast }) })); +vi.mock("@/lib/sql/sqlHighlighter", () => ({ createShikiSqlHighlighter: vi.fn(async () => (sql: string) => sql) })); +vi.mock("@/lib/metadata/objectDdlCache", () => ({ + loadObjectDdl: mocks.loadObjectDdl, + invalidateObjectDdl: mocks.invalidateObjectDdl, +})); +vi.mock("@/lib/metadata/objectMetadataCache", () => ({ loadObjectMetadataFacet: mocks.loadObjectMetadataFacet, invalidateObjectMetadataCache: mocks.invalidateObjectMetadataCache })); +vi.mock("@/lib/metadata/tableMetadataCache", () => ({ invalidateTableMetadataCache: mocks.invalidateTableMetadataCache })); +vi.mock("@/lib/backend/api", () => ({ + executeQuery: mocks.executeQuery, + listDataTypes: mocks.listDataTypes, + buildTableStructureChangeSql: mocks.buildTableStructureChangeSql, + buildMysqlAutoIncrementSql: mocks.buildMysqlAutoIncrementSql, + getMysqlTableAutoIncrement: mocks.getMysqlTableAutoIncrement, + executeBatch: mocks.executeBatch, +})); + +import TableStructureEditor from "@/components/structure/TableStructureEditor.vue"; + +const mountedApps: App[] = []; + +type ColumnSeed = { name: string; originalName: string }; + +/** + * Mount the editor in edit mode over `columns`. The column name the user typed + * is `name`; `originalName` is the metadata name the draft was hydrated from. + */ +function draftWithColumns(columns: ColumnSeed[]) { + return { + initialized: true, + activeTab: "columns" as const, + newTableName: "", + tableComment: "", + originalTableComment: "", + columns: columns.map((column, index) => ({ + id: `existing:${column.originalName}`, + name: column.name, + dataType: "int", + isNullable: true, + defaultValue: "", + comment: "", + isPrimaryKey: false, + characterSet: "", + collation: "", + extra: {}, + original: { + name: column.originalName, + data_type: "int", + is_nullable: true, + column_default: null, + is_primary_key: false, + extra: null, + comment: null, + }, + originalPosition: index, + markedForDrop: false, + })), + indexes: [], + foreignKeys: [], + triggers: [], + }; +} + +async function mountEditor(draft: ReturnType) { + mocks.ensureConnected.mockResolvedValue(undefined); + mocks.listDataTypes.mockResolvedValue([]); + mocks.buildTableStructureChangeSql.mockResolvedValue({ statements: [], warnings: [] }); + + const root = document.createElement("div"); + document.body.append(root); + const app = createApp(TableStructureEditor, { + connectionId: mocks.connection.id, + database: "test", + schema: "test", + tableName: "user", + draft, + }); + mountedApps.push(app); + app.mount(root); + await nextTick(); + await Promise.resolve(); + await nextTick(); + return root; +} + +/** Column names exactly as they reached the DDL builder. */ +async function previewColumnNames(): Promise { + await vi.waitFor(() => expect(mocks.buildTableStructureChangeSql).toHaveBeenCalled(), { timeout: 3000 }); + const options = mocks.buildTableStructureChangeSql.mock.calls.at(-1)![0] as { columns: Array<{ name: string }> }; + return options.columns.map((column) => column.name); +} + +beforeEach(() => { + vi.clearAllMocks(); + mocks.connection.db_type = "mysql"; + mocks.connection.driver_profile = "mysql"; + mocks.loadObjectDdl.mockResolvedValue({ ddl: "CREATE TABLE user (id int)", cacheStatus: "remote" }); + mocks.loadObjectMetadataFacet.mockResolvedValue({ value: [], cacheStatus: "remote" }); + mocks.getMysqlTableAutoIncrement.mockResolvedValue(null); + mocks.buildMysqlAutoIncrementSql.mockResolvedValue(""); + mocks.executeBatch.mockResolvedValue({ affected_rows: 0 }); +}); + +afterEach(() => { + for (const app of mountedApps.splice(0)) app.unmount(); + document.body.innerHTML = ""; +}); + +describe("TableStructureEditor column name whitespace", () => { + // MySQL rejects identifiers that end with a space (ERROR 1166 "Incorrect column + // name"), so a pasted name carrying a trailing space produced an unexecutable + // `ALTER TABLE ... CHANGE COLUMN ...` statement reported as "可执行". + it("trims a user-entered trailing space before the name reaches the DDL builder", async () => { + await mountEditor(draftWithColumns([{ name: "device_app_face_status ", originalName: "app_auth_status" }])); + + expect(await previewColumnNames()).toEqual(["device_app_face_status"]); + }); + + it("drops trailing whitespace a user pasted around a renamed column", async () => { + await mountEditor(draftWithColumns([{ name: "display_name\t ", originalName: "name" }])); + + expect(await previewColumnNames()).toEqual(["display_name"]); + }); + + it("trims the visible field on blur so the input matches the generated SQL", async () => { + const root = await mountEditor(draftWithColumns([{ name: "app_auth_status", originalName: "app_auth_status" }])); + const input = root.querySelector("[data-column-name-input]"); + expect(input).not.toBeNull(); + + input!.value = "device_app_face_status "; + input!.dispatchEvent(new Event("input", { bubbles: true })); + await nextTick(); + input!.dispatchEvent(new Event("blur")); + await nextTick(); + + expect(input!.value).toBe("device_app_face_status"); + }); + + // Metadata-backed names must stay byte-exact: MySQL keeps leading spaces in a + // backtick-quoted identifier, so rewriting a name the user never touched would + // turn every unrelated edit into a bogus rename + // (see crates/dbx-core/tests/live_mysql_column_name_whitespace.rs, #9654). + it("leaves an untouched metadata column name byte-exact", async () => { + const draft = draftWithColumns([{ name: " content1", originalName: " content1" }]); + draft.columns[0]!.comment = "leading space kept"; + await mountEditor(draft); + + expect(await previewColumnNames()).toEqual([" content1"]); + }); +}); diff --git a/apps/desktop/src/lib/__tests__/table/tableStructureEditorState.spec.ts b/apps/desktop/src/lib/__tests__/table/tableStructureEditorState.spec.ts index ffd4c57f53..d5e8086afb 100644 --- a/apps/desktop/src/lib/__tests__/table/tableStructureEditorState.spec.ts +++ b/apps/desktop/src/lib/__tests__/table/tableStructureEditorState.spec.ts @@ -13,6 +13,7 @@ import { dataTypeLengthUnitValue, DATA_TYPE_OPTIONS, defaultNewColumnDataType, + draftColumnNameForSql, getDataTypeLengthUnitOptions, getDefaultLengthForType, hasExistingColumnTypeChange, @@ -1039,3 +1040,25 @@ describe("structureColumnCommentsForCopy", () => { expect(comments.size).toBe(0); }); }); + +describe("draftColumnNameForSql", () => { + // MySQL rejects identifiers that end with a space (ERROR 1166), so a name the + // user pasted with a trailing space must not reach the DDL builder as typed. + it("drops the trailing whitespace of a name the user typed", () => { + expect(draftColumnNameForSql("device_app_face_status ", "app_auth_status")).toBe("device_app_face_status"); + expect(draftColumnNameForSql("display_name\t", "name")).toBe("display_name"); + expect(draftColumnNameForSql("new_column\n ")).toBe("new_column"); + }); + + // #9654: leading spaces are legal inside a backtick-quoted identifier, so the + // user's spelling is kept for a new column and for a metadata name alike. + it("keeps leading spaces of a name the user typed", () => { + expect(draftColumnNameForSql(" content1")).toBe(" content1"); + }); + + it("keeps an untouched metadata name byte-exact so whitespace is never read as a rename", () => { + expect(draftColumnNameForSql(" content1", " content1")).toBe(" content1"); + expect(draftColumnNameForSql("content1 ", "content1 ")).toBe("content1 "); + expect(draftColumnNameForSql("plain", "plain")).toBe("plain"); + }); +}); diff --git a/apps/desktop/src/lib/table/tableStructureEditorState.ts b/apps/desktop/src/lib/table/tableStructureEditorState.ts index 06736c3c5a..1c2106b833 100644 --- a/apps/desktop/src/lib/table/tableStructureEditorState.ts +++ b/apps/desktop/src/lib/table/tableStructureEditorState.ts @@ -52,6 +52,21 @@ export function structureColumnCommentsForCopy(columns: readonly Pick !!column.original && !column.markedForDrop && column.dataType !== column.original.data_type); }