-
-
Notifications
You must be signed in to change notification settings - Fork 3k
feat(assets): add conservative explicit cleanup for local files #854
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
Zhao0335
wants to merge
16
commits into
pascalorg:main
Choose a base branch
from
Zhao0335:fix/asset-storage-orphan-cleanup
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
7f20ecf
fix(editor): clean up orphaned local asset:// files from IndexedDB
Zhao0335 d44b09d
fix(editor): drop scene-load asset sweep that wiped other scenes
Zhao0335 7ae66a2
chore: remove local PR_BODY.md from the branch
Zhao0335 cb97731
fix(editor): skip delayed asset delete after a scene switch
Zhao0335 7da0daa
fix(core): schedule local asset cleanup from deleteNodes lifecycle
Zhao0335 1f0a96c
fix(core): fire-time asset recheck, post-commit replace, epoch on set…
Zhao0335 d442411
chore: fix biome organizeImports for quality CI
Zhao0335 ab8f3ca
style: apply biome format to asset lifecycle files
Zhao0335 55e04ae
fix(editor): explicit multi-scene GC instead of active-graph deletes
Zhao0335 0023ec0
fix(editor): refuse GC on truncated scene lists; re-read live graph
Zhao0335 6ee6091
style: organize imports in scene-loader
Zhao0335 3bea691
fix(app): type scene list JSON without never-narrowing
Zhao0335 3cc6f50
fix(editor): do not auto-run local asset GC on scene mount
Zhao0335 276e5b0
fix(core): collect nested asset:// urls for GC keep-sets
Zhao0335 b8a6af3
fix(assets): preserve live material URLs during explicit GC
Zhao0335 88f1dce
fix(assets): abort GC on incomplete scene inventory
Zhao0335 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,142 @@ | ||
| import 'fake-indexeddb/auto' | ||
| import { describe, expect, test } from 'bun:test' | ||
| import { deleteAsset, loadAssetUrl, saveAsset } from '@pascal-app/core' | ||
| import { collectNodeAssetUrlList, runLocalAssetGc } from './local-asset-gc' | ||
|
|
||
| type FetchHandler = (url: string) => Response | Promise<Response> | ||
|
|
||
| function mockFetch(handler: FetchHandler): () => void { | ||
| const original = globalThis.fetch | ||
| globalThis.fetch = (async (input: RequestInfo | URL) => { | ||
| const url = typeof input === 'string' ? input : input.toString() | ||
| return handler(url) | ||
| }) as typeof fetch | ||
| return () => { | ||
| globalThis.fetch = original | ||
| } | ||
| } | ||
|
|
||
| function json(body: unknown, ok = true, status = 200): Response { | ||
| return new Response(JSON.stringify(body), { | ||
| status, | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| }) | ||
| } | ||
|
|
||
| describe('local-asset-gc collectors', () => { | ||
| test('collectNodeAssetUrlList walks url and src', () => { | ||
| const urls = collectNodeAssetUrlList({ | ||
| a: { url: 'asset://guide' }, | ||
| b: { src: 'asset://model' }, | ||
| c: { src: 'https://cdn.example.com/x.glb' }, | ||
| }) | ||
| expect(urls.sort()).toEqual(['asset://guide', 'asset://model']) | ||
| }) | ||
| }) | ||
|
|
||
| describe('runLocalAssetGc safety', () => { | ||
| test('skips GC when the scene list is truncated at the API limit', async () => { | ||
| const scenes = Array.from({ length: 500 }, (_, i) => ({ id: `scene-${i}` })) | ||
| const restore = mockFetch((url) => { | ||
| if (url.includes('/api/scenes?')) return json({ scenes }) | ||
| return json({ graph: { nodes: {} } }) | ||
| }) | ||
| try { | ||
| const result = await runLocalAssetGc(() => ({ nodes: {} })) | ||
| expect(result).toBeNull() | ||
| } finally { | ||
| restore() | ||
| } | ||
| }) | ||
|
|
||
| test('skips GC when listing scenes fails', async () => { | ||
| const restore = mockFetch(() => json({ error: 'no' }, false, 500)) | ||
| try { | ||
| expect(await runLocalAssetGc(() => ({ nodes: {} }))).toBeNull() | ||
| } finally { | ||
| restore() | ||
| } | ||
| }) | ||
|
|
||
| test('skips GC when the scene inventory is malformed', async () => { | ||
| for (const body of [{}, { scenes: [{ id: null }] }]) { | ||
| const restore = mockFetch(() => json(body)) | ||
| try { | ||
| expect(await runLocalAssetGc(() => ({ nodes: {} }))).toBeNull() | ||
| } finally { | ||
| restore() | ||
| } | ||
| } | ||
| }) | ||
|
|
||
| test('skips GC when a listed scene has no graph', async () => { | ||
| const restore = mockFetch((url) => | ||
| url.includes('/api/scenes?') ? json({ scenes: [{ id: 'scene-1' }] }) : json({}), | ||
| ) | ||
| try { | ||
| expect(await runLocalAssetGc(() => ({ nodes: {} }))).toBeNull() | ||
| } finally { | ||
| restore() | ||
| } | ||
| }) | ||
|
|
||
| test('skips GC when the local scene cannot be parsed', async () => { | ||
| const previous = Object.getOwnPropertyDescriptor(globalThis, 'localStorage') | ||
| Object.defineProperty(globalThis, 'localStorage', { | ||
| configurable: true, | ||
| value: { getItem: () => '{not-json' }, | ||
| }) | ||
| const restore = mockFetch(() => json({ scenes: [] })) | ||
| try { | ||
| expect(await runLocalAssetGc(() => ({ nodes: {} }))).toBeNull() | ||
| } finally { | ||
| restore() | ||
| if (previous) Object.defineProperty(globalThis, 'localStorage', previous) | ||
| else Reflect.deleteProperty(globalThis, 'localStorage') | ||
| } | ||
| }) | ||
|
|
||
| test('re-reads live nodes so mid-GC uploads stay in the keep-set', async () => { | ||
| // Live graph is empty when GC starts, then gains a guide during the | ||
| // per-scene fetch — the File must survive the sweep. | ||
| let live: Record<string, unknown> = {} | ||
| const lateAsset = 'asset://uploaded-mid-gc' | ||
| const restore = mockFetch(async (url) => { | ||
| if (url.includes('/api/scenes?')) { | ||
| // Simulate latency while the user uploads. | ||
| live = { guide: { url: lateAsset } } | ||
| return json({ scenes: [] }) | ||
| } | ||
| return json({ graph: { nodes: {} } }) | ||
| }) | ||
| try { | ||
| const result = await runLocalAssetGc(() => ({ nodes: live })) | ||
| // Keep-set included the late upload, so sweep ran but did not need to | ||
| // delete it (result is a count of removals — File itself is not present | ||
| // in this unit test's IDB; the contract is "keep-set includes live URLs"). | ||
| expect(result).not.toBeNull() | ||
| expect(collectNodeAssetUrlList(live)).toContain(lateAsset) | ||
| } finally { | ||
| restore() | ||
| } | ||
| }) | ||
|
|
||
| test('keeps a material texture added to the live graph during inventory', async () => { | ||
| const assetUrl = await saveAsset(new File(['texture'], 'texture.png')) | ||
| let materials: unknown = {} | ||
| const restore = mockFetch((url) => { | ||
| if (url.includes('/api/scenes?')) { | ||
| materials = { floor: { texture: { url: assetUrl } } } | ||
| return json({ scenes: [] }) | ||
| } | ||
| return json({ graph: { nodes: {} } }) | ||
| }) | ||
| try { | ||
| await runLocalAssetGc(() => ({ nodes: {}, materials })) | ||
| expect(await loadAssetUrl(assetUrl)).not.toBeNull() | ||
| } finally { | ||
| restore() | ||
| await deleteAsset(assetUrl) | ||
| } | ||
| }) | ||
| }) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,124 @@ | ||
| import { | ||
| collectGraphAssetUrlsFromParts, | ||
| collectNodeAssetUrls, | ||
| sweepLocalAssetsExcept, | ||
| } from '@pascal-app/core' | ||
|
|
||
| const LOCAL_STORAGE_SCENE_KEY = 'pascal-editor-scene' | ||
| /** Must match apps/editor/app/api/scenes/route.ts listQuerySchema max. */ | ||
| const SCENES_LIST_MAX = 500 | ||
|
|
||
| type SceneGraphLike = { | ||
| nodes?: Record<string, unknown> | ||
| materials?: unknown | ||
| } | ||
|
|
||
| function collectGraphAssetUrls(graph: SceneGraphLike | null | undefined): string[] { | ||
| if (!graph?.nodes && !graph?.materials) return [] | ||
| return collectGraphAssetUrlsFromParts(graph.nodes as never, graph.materials) | ||
| } | ||
|
|
||
| /** Asset URLs still referenced by the browser's localStorage scene, if any. */ | ||
| export function collectLocalStorageSceneAssetUrls(): string[] | null { | ||
| if (typeof localStorage === 'undefined') return [] | ||
| try { | ||
| const raw = localStorage.getItem(LOCAL_STORAGE_SCENE_KEY) | ||
| if (!raw) return [] | ||
| return collectGraphAssetUrls(JSON.parse(raw) as SceneGraphLike) | ||
| } catch { | ||
| return null | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Union of asset URLs across every graph this origin can still open. | ||
| * Returns null when the inventory cannot be proven complete — callers must | ||
| * skip GC rather than sweep from a partial keep-set. | ||
| */ | ||
| export async function collectAllPersistedAssetUrls( | ||
| currentGraph: SceneGraphLike, | ||
| ): Promise<string[] | null> { | ||
| const keep = new Set<string>(collectGraphAssetUrls(currentGraph)) | ||
| const localUrls = collectLocalStorageSceneAssetUrls() | ||
| if (localUrls === null) return null | ||
| for (const url of localUrls) keep.add(url) | ||
|
|
||
| // Server-side scenes may share the same asset:// handles after duplication. | ||
| // If listing fails or is truncated we cannot prove the keep-set is complete. | ||
| let scenesJson: { data?: { scenes?: unknown[] }; scenes?: unknown[] } | null = null | ||
| try { | ||
| const res = await fetch(`/api/scenes?limit=${SCENES_LIST_MAX}`) | ||
| if (!res.ok) return null | ||
| scenesJson = (await res.json()) as { | ||
| data?: { scenes?: unknown[] } | ||
| scenes?: unknown[] | ||
| } | ||
| } catch { | ||
| return null | ||
| } | ||
|
|
||
| const list = scenesJson?.data?.scenes ?? scenesJson?.scenes | ||
| if (!Array.isArray(list)) return null | ||
| // The API has no cursor/total. A full page means older scenes were dropped | ||
| // by `limit` — treating it as complete would delete their Files. | ||
| if (list.length >= SCENES_LIST_MAX) return null | ||
|
|
||
| for (const entry of list) { | ||
| const id = (entry as { id?: unknown } | null)?.id | ||
| if (typeof id !== 'string' || !id) return null | ||
| try { | ||
| const res = await fetch(`/api/scenes/${encodeURIComponent(id)}`) | ||
| if (!res.ok) return null | ||
| const body = (await res.json()) as { | ||
| data?: { graph?: SceneGraphLike; scene?: { graph?: SceneGraphLike } } | ||
| graph?: SceneGraphLike | ||
| } | ||
| const graph = body.data?.graph ?? body.data?.scene?.graph ?? body.graph ?? null | ||
| if (!graph || typeof graph !== 'object') return null | ||
| for (const url of collectGraphAssetUrls(graph)) keep.add(url) | ||
| } catch { | ||
| return null | ||
| } | ||
| } | ||
|
|
||
| return [...keep] | ||
|
cursor[bot] marked this conversation as resolved.
|
||
| } | ||
|
|
||
| /** | ||
| * Explicit GC: delete IndexedDB Files that no persisted scene references. | ||
| * | ||
| * **Do not call this from scene mount / load.** Automatic mount-time GC races | ||
| * keepalive flushes, other tabs, and uploads that have hit IndexedDB but are | ||
| * not yet on a node; a sequential GET per scene also burns the shared API rate | ||
| * bucket used by autosave (#733 review). Call only when the host can prove: | ||
| * no in-flight writes, a complete scene inventory, and spare rate budget. | ||
| * | ||
| * `getLiveGraph` is re-read immediately before sweeping so both nodes and | ||
| * unsaved material texture references survive. | ||
| * No-ops when the full keep-set cannot be built (never partial-sweeps). | ||
| */ | ||
| export async function runLocalAssetGc( | ||
| getLiveGraph: () => SceneGraphLike, | ||
| extraKeepUrls: Iterable<string> = [], | ||
| ): Promise<number | null> { | ||
| const keep = await collectAllPersistedAssetUrls(getLiveGraph()) | ||
| if (keep === null) return null | ||
|
|
||
| const finalKeep = new Set<string>(keep) | ||
| for (const url of collectGraphAssetUrls(getLiveGraph())) { | ||
| finalKeep.add(url) | ||
| } | ||
|
cursor[bot] marked this conversation as resolved.
|
||
| for (const url of extraKeepUrls) { | ||
| if (typeof url === 'string' && url.startsWith('asset://')) finalKeep.add(url) | ||
| } | ||
| return sweepLocalAssetsExcept(finalKeep) | ||
|
cursor[bot] marked this conversation as resolved.
|
||
| } | ||
|
|
||
| /** For tests: collect urls from an arbitrary node map. */ | ||
| export function collectNodeAssetUrlList(nodes: Record<string, unknown>): string[] { | ||
| const urls: string[] = [] | ||
| for (const node of Object.values(nodes)) { | ||
| urls.push(...collectNodeAssetUrls(node as never)) | ||
| } | ||
| return urls | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.