Skip to content
Draft
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 31 additions & 2 deletions packages/@uppy/golden-retriever/src/IndexedDBStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,16 @@ let cleanedUp = false
class IndexedDBStore {
#ready: Promise<IDBDatabase> | IDBDatabase

/**
* In-memory size cache eliminates the O(n) cursor walk inside getSize()
* on every put(), making sequential puts O(1) instead of O(n²).
* null means "not yet initialised"; the first getSize() call populates it.
* Incremented on successful put, reset to null on delete (since we don't
* track deleted-file sizes — the next getSize() rebuilds from scratch,
* which is fast because the store shrinks after deletions).
*/
#cachedSize: number | null = null

Comment on lines +103 to +112

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check if we can use this API to detect multiple instances on tab switch ? https://developer.mozilla.org/en-US/docs/Web/API/Broadcast_Channel_API

opts: Required<IndexedDBStoreOptions>

name: string
Expand Down Expand Up @@ -173,8 +183,13 @@ class IndexedDBStore {

/**
* Get the total size of all stored files.
*
* Uses an in-memory cache after the first call so that sequential puts()
* are O(1) rather than O(n) per call — without it, a batch of N puts
* cursor-walks the entire store N times (O(n²) reads overall).
*/
async getSize(): Promise<number> {
if (this.#cachedSize !== null) return this.#cachedSize

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Invalidate size cache when shared store is mutated elsewhere

Returning #cachedSize unconditionally after the first getSize() call makes the total-size guard stale when another Uppy instance/tab writes to the same IndexedDB namespace (the default storeName is derived from the Uppy ID, which is commonly shared). In that case, this instance never re-reads IndexedDB, so put() can allow growth past maxTotalSize (or reject valid writes after external deletions) because the cached value only tracks local put/delete calls.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@codex check if we can use this API to detect multiple instances on tab switch and then trigger a re-scan ? https://developer.mozilla.org/en-US/docs/Web/API/Broadcast_Channel_API

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Invalidate cached size when other tabs mutate IndexedDB

The new #cachedSize fast path is process-local, so it becomes stale as soon as another tab/Uppy instance adds or deletes blobs in the same uppy-blobs store. In that multi-tab scenario, put() can make decisions from an out-of-date total (maxTotalSize check at put) and either allow writes that exceed the quota or reject writes after space was freed, which is a behavior regression from the previous always-scan implementation. This cache needs a cross-context invalidation signal (e.g., BroadcastChannel/storage event) or a periodic/visibility-triggered rescan before trusting the cached value.

Useful? React with 👍 / 👎.

const db = await this.#ready
const transaction = db.transaction([STORE_NAME], 'readonly')
const store = transaction.objectStore(STORE_NAME)
Expand All @@ -187,6 +202,7 @@ class IndexedDBStore {
size += cursor.value.data.size
cursor.continue()
} else {
this.#cachedSize = size
resolve(size)
}
}
Expand Down Expand Up @@ -216,7 +232,15 @@ class IndexedDBStore {
expires: Date.now() + this.opts.expires,
data: file.data,
})
return waitForRequest(request)
const result = await waitForRequest<T>(request)
// Only update the cache if it is still live. If a concurrent delete()
// invalidated it (set it to null) while this put() was in flight, leave
// it null so the next getSize() does a fresh scan — resurrecting it from
// 0 here would silently drop the pre-existing total and the deletion.
if (this.#cachedSize !== null) {
this.#cachedSize += file.data.size
}
return result
}

/**
Expand All @@ -226,7 +250,12 @@ class IndexedDBStore {
const db = await this.#ready
const transaction = db.transaction([STORE_NAME], 'readwrite')
const request = transaction.objectStore(STORE_NAME).delete(this.key(fileID))
return waitForRequest(request)
const result = await waitForRequest(request)
// We don't track the deleted file's size, so reset the cache so the
// next getSize() does a fresh scan (which will be fast after deletions
// since the store has shrunk).
this.#cachedSize = null
return result
}

/**
Expand Down