diff --git a/src/context/free-widget.context.tsx b/src/context/free-widget.context.tsx index aa7c4eb7..be9a4f81 100644 --- a/src/context/free-widget.context.tsx +++ b/src/context/free-widget.context.tsx @@ -31,6 +31,12 @@ import { type WidgetPosition, type WidgetSize, } from '@/layouts/widgets/layout-engine/types' +import { + applyInstanceIdMap, + buildInstanceIdMap, + dedupeInstanceIds, + isServerInstanceId, +} from '@/layouts/widgets/instance-id' import { migrateWidgetLayoutIfNeeded } from '@/layouts/widgets/migration' import { WIDGET_DEFINITIONS } from '@/layouts/widgets/widget-registry' import { @@ -123,7 +129,7 @@ function normalizeWidgetSizes(layout: StoredWidget[], cols: number): StoredWidge } function sanitizeLayout(layout: StoredWidget[], cols: number): StoredWidget[] { - const sized = normalizeWidgetSizes(layout, cols) + const sized = normalizeWidgetSizes(dedupeInstanceIds(layout), cols) if (validateLayout(sized, cols)) { return sized @@ -381,25 +387,10 @@ export function FreeWidgetProvider({ children }: { children: React.ReactNode }) if (synced && synced.length > 0) { setSavedLayout((prev) => { - const updated = prev.map((w, index) => { - const matching = - synced.find( - (s) => s.instanceId === w.instanceId - ) || - synced.find((s) => s.widgetKey === w.id) || - synced[index] - if ( - matching?.instanceId && - matching.instanceId !== w.instanceId - ) { - return { - ...w, - instanceId: matching.instanceId, - widgetId: matching.instanceId, - } - } - return w - }) + const idMap = buildInstanceIdMap(prev, synced) + if (idMap.size === 0) return prev + + const updated = applyInstanceIdMap(prev, idMap) savedLayoutRef.current = updated persistLayout(updated) return updated @@ -445,42 +436,16 @@ export function FreeWidgetProvider({ children }: { children: React.ReactNode }) .then((synced) => { if (!synced || synced.length === 0) return - const idMap = new Map() - currentLayout.forEach((w, index) => { - const isValidId = - typeof w.instanceId === 'string' && - /^[0-9a-fA-F]{24}$/.test(w.instanceId) - if (isValidId) return - const matching = - synced.find((s) => s.widgetKey === w.id) || synced[index] - if ( - matching?.instanceId && - matching.instanceId !== w.instanceId - ) { - idMap.set(w.instanceId, matching.instanceId) - } - }) - + const idMap = buildInstanceIdMap(currentLayout, synced) if (idMap.size === 0) return - const applyIdMap = (list: StoredWidget[]) => - list.map((w) => - idMap.has(w.instanceId) - ? { - ...w, - instanceId: idMap.get(w.instanceId) as string, - widgetId: idMap.get(w.instanceId) as string, - } - : w - ) - setSavedLayout((prev) => { - const updated = applyIdMap(prev) + const updated = applyInstanceIdMap(prev, idMap) savedLayoutRef.current = updated persistLayout(updated) return updated }) - applyRuntimeLayout((prev) => applyIdMap(prev)) + applyRuntimeLayout((prev) => applyInstanceIdMap(prev, idMap)) }) .catch(() => {}) }, 1000) @@ -899,7 +864,7 @@ export function FreeWidgetProvider({ children }: { children: React.ReactNode }) setSelectedInstanceId(null) } - if (isAuthenticated && typeof instanceId === 'string' && instanceId.trim()) { + if (isAuthenticated && isServerInstanceId(instanceId)) { deleteUserWidgetApi(instanceId).catch(() => {}) } diff --git a/src/layouts/widgets/__tests__/instance-id.test.ts b/src/layouts/widgets/__tests__/instance-id.test.ts new file mode 100644 index 00000000..10d7c15b --- /dev/null +++ b/src/layouts/widgets/__tests__/instance-id.test.ts @@ -0,0 +1,193 @@ +import { describe, expect, it } from 'bun:test' +import { + applyInstanceIdMap, + buildInstanceIdMap, + dedupeInstanceIds, + isServerInstanceId, +} from '../instance-id' +import { resolveLayoutChange } from '../layout-engine' +import type { StoredWidget } from '../layout-engine/types' +import { WidgetKeys } from '../layout-engine/types' + +function widget( + instanceId: string, + id: WidgetKeys = WidgetKeys.clock, + col = 0, + row = 0 +): StoredWidget { + return { id, instanceId, position: { col, row }, size: { w: 2, h: 1 } } +} + +const SERVER_A = 'aaaaaaaaaaaaaaaaaaaaaaaa' +const SERVER_B = 'bbbbbbbbbbbbbbbbbbbbbbbb' + +describe('isServerInstanceId', () => { + it('accepts only 24-char hex ids', () => { + expect(isServerInstanceId(SERVER_A)).toBe(true) + expect(isServerInstanceId('clock-abc123')).toBe(false) + expect(isServerInstanceId('')).toBe(false) + expect(isServerInstanceId(undefined)).toBe(false) + }) +}) + +describe('buildInstanceIdMap', () => { + it('never assigns a server id already held by another widget', () => { + const layout = [ + widget(SERVER_A, WidgetKeys.clock), + widget('clock-local', WidgetKeys.clock), + ] + const synced = [ + { instanceId: SERVER_A, widgetKey: WidgetKeys.clock }, + { instanceId: SERVER_B, widgetKey: WidgetKeys.clock }, + ] + + const idMap = buildInstanceIdMap(layout, synced) + + expect(idMap.get('clock-local')).toBe(SERVER_B) + + const updated = applyInstanceIdMap(layout, idMap) + const ids = updated.map((w) => w.instanceId) + expect(new Set(ids).size).toBe(ids.length) + }) + + it('does not map two local widgets onto the same server id', () => { + const layout = [widget('clock-1', WidgetKeys.clock), widget('clock-2', WidgetKeys.clock)] + const synced = [{ instanceId: SERVER_A, widgetKey: WidgetKeys.clock }] + + const idMap = buildInstanceIdMap(layout, synced) + + expect(idMap.size).toBe(1) + const updated = applyInstanceIdMap(layout, idMap) + const ids = updated.map((w) => w.instanceId) + expect(new Set(ids).size).toBe(ids.length) + }) + + it('leaves widgets untouched when there is nothing to map', () => { + const layout = [widget(SERVER_A), widget(SERVER_B)] + const synced = [ + { instanceId: SERVER_A, widgetKey: WidgetKeys.clock }, + { instanceId: SERVER_B, widgetKey: WidgetKeys.clock }, + ] + + expect(buildInstanceIdMap(layout, synced).size).toBe(0) + expect(applyInstanceIdMap(layout, new Map())).toBe(layout) + }) + + it('does not borrow a server id belonging to a different widget type', () => { + const layout = [widget('search-local', WidgetKeys.search)] + const synced = [{ instanceId: SERVER_A, widgetKey: WidgetKeys.clock }] + + expect(buildInstanceIdMap(layout, synced).size).toBe(0) + }) +}) + +describe('dedupeInstanceIds', () => { + it('repairs a layout where a duplicate shares the original instance id', () => { + const layout = [ + widget(SERVER_A, WidgetKeys.clock, 0, 0), + widget(SERVER_A, WidgetKeys.clock, 2, 0), + ] + + const repaired = dedupeInstanceIds(layout) + const ids = repaired.map((w) => w.instanceId) + + expect(new Set(ids).size).toBe(2) + expect(repaired[0].instanceId).toBe(SERVER_A) + expect(repaired[1].instanceId).not.toBe(SERVER_A) + expect(repaired[1].widgetId).toBe(repaired[1].instanceId) + }) + + it('returns the same array when every id is unique', () => { + const layout = [widget(SERVER_A), widget(SERVER_B)] + expect(dedupeInstanceIds(layout)).toBe(layout) + }) +}) + +describe('duplicate operation', () => { + it('keeps the server issued instance id instead of inventing one', () => { + const layout = [widget(SERVER_A, WidgetKeys.clock)] + + const result = resolveLayoutChange({ + layout, + operation: 'duplicate', + instanceId: SERVER_A, + newWidget: { + id: WidgetKeys.clock, + instanceId: SERVER_B, + widgetId: SERVER_B, + position: { col: 0, row: 0 }, + size: { w: 2, h: 1 }, + }, + cols: 8, + }) + + expect(result).not.toBeNull() + expect(result!.map((w) => w.instanceId).sort()).toEqual( + [SERVER_A, SERVER_B].sort() + ) + }) + + it('removing the duplicate leaves the original in place', () => { + const layout = [widget(SERVER_A, WidgetKeys.clock)] + + const duplicated = resolveLayoutChange({ + layout, + operation: 'duplicate', + instanceId: SERVER_A, + newWidget: { + id: WidgetKeys.clock, + instanceId: SERVER_B, + widgetId: SERVER_B, + position: { col: 0, row: 0 }, + size: { w: 2, h: 1 }, + }, + cols: 8, + })! + + const afterRemove = resolveLayoutChange({ + layout: duplicated, + operation: 'remove', + instanceId: SERVER_B, + cols: 8, + }) + + expect(afterRemove).not.toBeNull() + expect(afterRemove!.length).toBe(1) + expect(afterRemove![0].instanceId).toBe(SERVER_A) + }) + + it('rejects a duplicate whose instance id is already taken', () => { + const layout = [widget(SERVER_A, WidgetKeys.clock)] + + const result = resolveLayoutChange({ + layout, + operation: 'duplicate', + instanceId: SERVER_A, + newWidget: { + id: WidgetKeys.clock, + instanceId: SERVER_A, + widgetId: SERVER_A, + position: { col: 0, row: 0 }, + size: { w: 2, h: 1 }, + }, + cols: 8, + }) + + expect(result).toBeNull() + }) + + it('still generates an id when no widget is supplied', () => { + const layout = [widget(SERVER_A, WidgetKeys.clock)] + + const result = resolveLayoutChange({ + layout, + operation: 'duplicate', + instanceId: SERVER_A, + cols: 8, + }) + + expect(result).not.toBeNull() + expect(result!.length).toBe(2) + expect(result![1].instanceId).not.toBe(SERVER_A) + }) +}) diff --git a/src/layouts/widgets/instance-id.ts b/src/layouts/widgets/instance-id.ts new file mode 100644 index 00000000..7f1cc5f1 --- /dev/null +++ b/src/layouts/widgets/instance-id.ts @@ -0,0 +1,97 @@ +import type { StoredWidget } from './layout-engine/types' + +const SERVER_INSTANCE_ID_PATTERN = /^[0-9a-fA-F]{24}$/ + +export interface SyncedWidgetIdentity { + instanceId: string + widgetKey: string +} + +export function isServerInstanceId(value: unknown): value is string { + return typeof value === 'string' && SERVER_INSTANCE_ID_PATTERN.test(value) +} + +export function createLocalInstanceId(widgetKey: string): string { + return `${widgetKey}-${Date.now().toString(36)}-${Math.random().toString(36).slice(2, 8)}` +} + +export function dedupeInstanceIds(layout: StoredWidget[]): StoredWidget[] { + const seen = new Set() + let changed = false + + const result = layout.map((widget) => { + if (widget.instanceId && !seen.has(widget.instanceId)) { + seen.add(widget.instanceId) + return widget + } + + let nextId = createLocalInstanceId(widget.id) + while (seen.has(nextId)) { + nextId = createLocalInstanceId(widget.id) + } + seen.add(nextId) + changed = true + + return { ...widget, instanceId: nextId, widgetId: nextId } + }) + + return changed ? result : layout +} + +export function buildInstanceIdMap( + layout: StoredWidget[], + synced: SyncedWidgetIdentity[] +): Map { + const idMap = new Map() + const claimed = new Set() + + for (const widget of layout) { + if (isServerInstanceId(widget.instanceId)) { + claimed.add(widget.instanceId) + } + } + + for (const entry of synced) { + if (!entry?.instanceId) continue + const owner = layout.find((w) => w.instanceId === entry.instanceId) + if (owner) { + claimed.add(entry.instanceId) + } + } + + layout.forEach((widget, index) => { + if (isServerInstanceId(widget.instanceId)) return + + const candidates: SyncedWidgetIdentity[] = [] + const byIndex = synced[index] + if (byIndex?.instanceId && byIndex.widgetKey === widget.id) { + candidates.push(byIndex) + } + for (const entry of synced) { + if (entry?.instanceId && entry.widgetKey === widget.id) { + candidates.push(entry) + } + } + + const matching = candidates.find((entry) => !claimed.has(entry.instanceId)) + if (!matching) return + + claimed.add(matching.instanceId) + idMap.set(widget.instanceId, matching.instanceId) + }) + + return idMap +} + +export function applyInstanceIdMap( + layout: StoredWidget[], + idMap: Map +): StoredWidget[] { + if (idMap.size === 0) return layout + + return layout.map((widget) => { + const nextId = idMap.get(widget.instanceId) + if (!nextId) return widget + return { ...widget, instanceId: nextId, widgetId: nextId } + }) +} diff --git a/src/layouts/widgets/layout-engine/layout-engine.ts b/src/layouts/widgets/layout-engine/layout-engine.ts index bfabae6e..a90af7e3 100644 --- a/src/layouts/widgets/layout-engine/layout-engine.ts +++ b/src/layouts/widgets/layout-engine/layout-engine.ts @@ -153,12 +153,21 @@ export function resolveLayoutChange( const original = layout.find((w) => w.instanceId === instanceId) if (!original) return null - const newInstanceId = `${original.id}-${Date.now().toString(36)}-${Math.random().toString(36).slice(2, 6)}` - const duplicated: StoredWidget = { - id: original.id, - instanceId: newInstanceId, - size: { ...original.size }, - position: findAvailableSlot(layout, original.size, cols), + const duplicated: StoredWidget = newWidget + ? { + ...newWidget, + size: { ...newWidget.size }, + position: findAvailableSlot(layout, newWidget.size, cols), + } + : { + id: original.id, + instanceId: `${original.id}-${Date.now().toString(36)}-${Math.random().toString(36).slice(2, 6)}`, + size: { ...original.size }, + position: findAvailableSlot(layout, original.size, cols), + } + + if (layout.some((w) => w.instanceId === duplicated.instanceId)) { + return null } const appended = [...layout, duplicated]