From 414e8bfd547da79f14dd98bd85715d85c0e8f7db Mon Sep 17 00:00:00 2001 From: hugocasa Date: Mon, 14 Sep 2026 15:59:55 +0200 Subject: [PATCH] fix: ignore over-long synced display names, move name loaders Co-Authored-By: Claude Opus 5 (1M context) --- backend/src/main.rs | 10 +++- backend/summarized_schema.txt | 2 +- backend/windmill-api-settings/src/lib.rs | 14 ++++-- .../src/lib/components/ImportSetupStep.svelte | 2 +- .../LightweightResourcePicker.svelte | 2 +- .../components/ResourceEditorDrawer.svelte | 2 +- .../src/lib/components/displayNameLoaders.ts | 46 +++++++++++++++++++ .../flows/pickers/PickHubApp.svelte | 2 +- .../flows/pickers/PickHubFlow.svelte | 2 +- .../src/lib/components/pickerPopularity.ts | 41 +---------------- 10 files changed, 73 insertions(+), 50 deletions(-) create mode 100644 frontend/src/lib/components/displayNameLoaders.ts diff --git a/backend/src/main.rs b/backend/src/main.rs index ce9f7efbdd..3a45898928 100644 --- a/backend/src/main.rs +++ b/backend/src/main.rs @@ -597,8 +597,16 @@ pub async fn sync_cached_resource_types(db: &sqlx::Pool) -> anyh None => stored_extension.clone(), } }; - // No key in the cache leaves the stored name alone, as for the extension. + // No key in the cache leaves the stored name alone, as for the extension. So does a name + // too long for the column: one bad entry must not fail the upsert and end the sync. let display_name = match &rt.display_name { + Some(Some(name)) if name.chars().count() > 100 => { + tracing::warn!( + "Ignoring the display_name of resource type {}: longer than 100 characters", + rt.name + ); + stored_display_name.clone() + } Some(from_cache) => from_cache.clone(), None => stored_display_name.clone(), }; diff --git a/backend/summarized_schema.txt b/backend/summarized_schema.txt index d0f9d63647..bb68d1ed11 100644 --- a/backend/summarized_schema.txt +++ b/backend/summarized_schema.txt @@ -166,7 +166,7 @@ raw_app: path(char), version(int), workspace_id(char), summary(char), edited_at( FK: (workspace_id) -> workspace(id) resource: workspace_id(char), path(char), value(jsonb), description(text), resource_type(char), extra_perms(jsonb), edited_at(ts), created_by(char), labels(text[]) FK: (workspace_id) -> workspace(id) -resource_type: workspace_id(char), name(char), schema(jsonb), description(text), edited_at(ts), created_by(char), format_extension(char), is_fileset(bool) +resource_type: workspace_id(char), name(char), schema(jsonb), description(text), edited_at(ts), created_by(char), format_extension(char), is_fileset(bool), display_name(char) FK: (workspace_id) -> workspace(id) resume_job: id(uuid), job(uuid), flow(uuid), created_at(ts), value(jsonb), approver(char), resume_id(int), approved(bool) FK: (flow) -> v2_job_queue(id) diff --git a/backend/windmill-api-settings/src/lib.rs b/backend/windmill-api-settings/src/lib.rs index c3a60f220a..a92b336e83 100644 --- a/backend/windmill-api-settings/src/lib.rs +++ b/backend/windmill-api-settings/src/lib.rs @@ -2177,6 +2177,12 @@ async fn sync_cached_resource_types( let mut synced_count = 0; for rt in &resource_types { + // A name too long for the column counts as absent, leaving the stored one alone: one bad + // entry must not fail the upsert and end the rest of the sync. + let display_name = match &rt.display_name { + Some(Some(name)) if name.chars().count() > 100 => None, + other => other.clone(), + }; let exists: Option = sqlx::query_scalar!( "SELECT EXISTS(SELECT 1 FROM resource_type WHERE workspace_id = 'admins' AND name = $1 AND schema IS NOT DISTINCT FROM $2 AND description IS NOT DISTINCT FROM $3 AND ($5 IS NOT TRUE OR format_extension IS NOT DISTINCT FROM $4) AND ($7 IS NOT TRUE OR display_name IS NOT DISTINCT FROM $6))", &rt.name, @@ -2184,8 +2190,8 @@ async fn sync_cached_resource_types( rt.description.as_deref(), rt.format_extension.clone().flatten(), rt.format_extension.is_some(), - rt.display_name.clone().flatten(), - rt.display_name.is_some(), + display_name.clone().flatten(), + display_name.is_some(), ) .fetch_one(&db) .await?; @@ -2217,8 +2223,8 @@ async fn sync_cached_resource_types( rt.description.as_deref(), rt.format_extension.clone().flatten(), rt.format_extension.is_some(), - rt.display_name.clone().flatten(), - rt.display_name.is_some(), + display_name.clone().flatten(), + display_name.is_some(), ) .execute(&db) .await?; diff --git a/frontend/src/lib/components/ImportSetupStep.svelte b/frontend/src/lib/components/ImportSetupStep.svelte index e84d81b19a..62a1ba8f77 100644 --- a/frontend/src/lib/components/ImportSetupStep.svelte +++ b/frontend/src/lib/components/ImportSetupStep.svelte @@ -23,7 +23,7 @@ resourceTypeDisplayName, setResourceTypeDisplayNames } from '$lib/components/resourceTypeDisplay' - import { loadResourceTypeDisplayName } from '$lib/components/pickerPopularity' + import { loadResourceTypeDisplayName } from '$lib/components/displayNameLoaders' import { applyOneMigration } from '$lib/components/workspaceSettings/projectInstall' import { probeMigrationsApplied } from '$lib/importWizard/probe' import { diff --git a/frontend/src/lib/components/LightweightResourcePicker.svelte b/frontend/src/lib/components/LightweightResourcePicker.svelte index 457e9aebba..58557e8c8b 100644 --- a/frontend/src/lib/components/LightweightResourcePicker.svelte +++ b/frontend/src/lib/components/LightweightResourcePicker.svelte @@ -10,7 +10,7 @@ import Select from './select/Select.svelte' import IconedResourceType from './IconedResourceType.svelte' import { addResourceTitle } from './resourceTypeDisplay' - import { loadResourceTypeDisplayName } from './pickerPopularity' + import { loadResourceTypeDisplayName } from './displayNameLoaders' interface Props { value: string | undefined diff --git a/frontend/src/lib/components/ResourceEditorDrawer.svelte b/frontend/src/lib/components/ResourceEditorDrawer.svelte index b61d582e26..358c7e03d6 100644 --- a/frontend/src/lib/components/ResourceEditorDrawer.svelte +++ b/frontend/src/lib/components/ResourceEditorDrawer.svelte @@ -18,7 +18,7 @@ import ResourceVersionHistory from './ResourceVersionHistory.svelte' import IconedResourceType from './IconedResourceType.svelte' import { addResourceTitle } from './resourceTypeDisplay' - import { loadResourceTypeDisplayName } from './pickerPopularity' + import { loadResourceTypeDisplayName } from './displayNameLoaders' let { workspace = undefined, diff --git a/frontend/src/lib/components/displayNameLoaders.ts b/frontend/src/lib/components/displayNameLoaders.ts new file mode 100644 index 0000000000..aaa5e2db15 --- /dev/null +++ b/frontend/src/lib/components/displayNameLoaders.ts @@ -0,0 +1,46 @@ +import { get } from 'svelte/store' +import { IntegrationService, ResourceService } from '$lib/gen' +import { disableHubStore } from '$lib/stores' +import { createCache } from '$lib/utils' +import { setHubIntegrationDisplayNames, setResourceTypeDisplayNames } from './resourceTypeDisplay' + +/** + * Loads the names `resourceTypeDisplayName` and `integrationDisplayName` read, for a surface that + * shows a label without already fetching the rows it comes from. Apart from `resourceTypeDisplay`, + * which makes no API calls so it can be unit-tested alone. Cached briefly: drawers and pickers + * reopen often, and a name rarely changes. + */ +const CACHE_MS = 60_000 + +const resourceTypeRowCached = createCache( + ({ workspace, name }: { workspace: string; name: string }) => + ResourceService.getResourceType({ workspace, path: name }).then( + (rt) => setResourceTypeDisplayNames([rt]), + () => {} + ), + { invalidateMs: CACHE_MS, maxSize: 50 } +) + +/** + * Fill `resourceTypeDisplayName` for one type, for a surface titled with a type it holds no row + * for. The name is stored with the type, so this reads the row rather than the hub. + */ +export function loadResourceTypeDisplayName(workspace: string, name: string): Promise { + return resourceTypeRowCached({ workspace, name }) +} + +const hubIntegrationNamesCached = createCache( + (_: Record) => + IntegrationService.listHubIntegrations().then(setHubIntegrationDisplayNames, () => {}), + { invalidateMs: CACHE_MS } +) + +/** + * Fill `integrationDisplayName` for a picker whose integrations come from its own items rather + * than the hub's integration list, as the hub app and flow pickers do. Unfiltered: `kind` + * narrows by script kind, so asking for an app or a flow would name nothing. + */ +export function loadHubIntegrationDisplayNames(): Promise { + if (get(disableHubStore)) return Promise.resolve() + return hubIntegrationNamesCached({}) +} diff --git a/frontend/src/lib/components/flows/pickers/PickHubApp.svelte b/frontend/src/lib/components/flows/pickers/PickHubApp.svelte index c55e7dfd43..15177d6cd6 100644 --- a/frontend/src/lib/components/flows/pickers/PickHubApp.svelte +++ b/frontend/src/lib/components/flows/pickers/PickHubApp.svelte @@ -6,7 +6,7 @@ import NoItemFound from '$lib/components/home/NoItemFound.svelte' import RowIcon from '$lib/components/common/table/RowIcon.svelte' import { loadHubApps } from '$lib/hub' - import { loadHubIntegrationDisplayNames } from '$lib/components/pickerPopularity' + import { loadHubIntegrationDisplayNames } from '$lib/components/displayNameLoaders' import TextInput from '$lib/components/text_input/TextInput.svelte' import { Alert } from '$lib/components/common' import { disableHubStore } from '$lib/stores' diff --git a/frontend/src/lib/components/flows/pickers/PickHubFlow.svelte b/frontend/src/lib/components/flows/pickers/PickHubFlow.svelte index f6fa3b3c77..8c07ad1506 100644 --- a/frontend/src/lib/components/flows/pickers/PickHubFlow.svelte +++ b/frontend/src/lib/components/flows/pickers/PickHubFlow.svelte @@ -6,7 +6,7 @@ import NoItemFound from '$lib/components/home/NoItemFound.svelte' import RowIcon from '$lib/components/common/table/RowIcon.svelte' import { loadHubFlows } from '$lib/hub' - import { loadHubIntegrationDisplayNames } from '$lib/components/pickerPopularity' + import { loadHubIntegrationDisplayNames } from '$lib/components/displayNameLoaders' import TextInput from '$lib/components/text_input/TextInput.svelte' import { Alert } from '$lib/components/common' import { disableHubStore } from '$lib/stores' diff --git a/frontend/src/lib/components/pickerPopularity.ts b/frontend/src/lib/components/pickerPopularity.ts index dc48741813..2d585ff858 100644 --- a/frontend/src/lib/components/pickerPopularity.ts +++ b/frontend/src/lib/components/pickerPopularity.ts @@ -1,12 +1,8 @@ import { get } from 'svelte/store' -import { IntegrationService, ResourceService } from '$lib/gen' +import { ResourceService } from '$lib/gen' import { disableHubStore } from '$lib/stores' import { createCache } from '$lib/utils' -import { - isCustomResourceTypeName, - setHubIntegrationDisplayNames, - setResourceTypeDisplayNames -} from './resourceTypeDisplay' +import { isCustomResourceTypeName } from './resourceTypeDisplay' /** * How often something has been picked or used, keyed by integration or resource type name. @@ -57,39 +53,6 @@ export async function hubResourceTypePicks(workspace: string): Promise [rt.name, rt.picks])) } -const resourceTypeRowCached = createCache( - ({ workspace, name }: { workspace: string; name: string }) => - ResourceService.getResourceType({ workspace, path: name }).then( - (rt) => setResourceTypeDisplayNames([rt]), - () => {} - ), - { invalidateMs: CACHE_MS, maxSize: 50 } -) - -/** - * Fill `resourceTypeDisplayName` for one type, for a surface titled with a type it holds no row - * for. The name is stored with the type, so this reads the row rather than the hub. - */ -export function loadResourceTypeDisplayName(workspace: string, name: string): Promise { - return resourceTypeRowCached({ workspace, name }) -} - -const hubIntegrationNamesCached = createCache( - (_: Record) => - IntegrationService.listHubIntegrations().then(setHubIntegrationDisplayNames, () => {}), - { invalidateMs: CACHE_MS } -) - -/** - * Fill `integrationDisplayName` for a picker whose integrations come from its own items rather - * than the hub's integration list, as the hub app and flow pickers do. Unfiltered: `kind` - * narrows by script kind, so asking for an app or a flow would name nothing. - */ -export function loadHubIntegrationDisplayNames(): Promise { - if (get(disableHubStore)) return Promise.resolve() - return hubIntegrationNamesCached({}) -} - /** * How many resources of each type this workspace holds — the only evidence about this * particular team. Keyed by resource type, which is what the add-resource drawer lists.