Skip to content

Commit e275e0c

Browse files
Eliran Eretz-KedoshaCopilot
authored andcommitted
Hide scoped-index put behind putInIndexAfterGet_DoNotUse + IScopedIndexPutProvider
Address Jeremie's PR review comment about the scoped-index put() overload being reachable via a direct cast on the public InMemoryProvider class, which made it too easy for consumers to accidentally break the all-indexes-stay-in-sync invariant. - Renamed InMemoryProvider's scoped put overload to putInIndexAfterGet_DoNotUse(storeName, itemOrItems, indexNames), with indexNames now required (not optional) since scoping is the entire point of this method. - Added an exported IScopedIndexPutProvider interface and an asScopedIndexPutProvider(provider) type-guard/cast helper as the only sanctioned way to discover/reach this capability -- InMemoryProvider does not declare 'implements IScopedIndexPutProvider', so it doesn't show up on the class surface itself. - Split InMemoryStore's internal put implementation into a plain put() (interface-compliant with DbStore, always populates every index), putInIndexAfterGet_DoNotUse() (scoped), and a shared private _putInternal() so both call sites reuse the same logic. - Updated the _getStoreTransaction() comment and the scoped-put test to use the new name/helper instead of an InMemoryProvider cast. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 009bd0a8-f98f-4048-a261-e0a627813a00
1 parent e40a070 commit e275e0c

3 files changed

Lines changed: 97 additions & 32 deletions

File tree

src/InMemoryProvider.ts

Lines changed: 78 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -151,25 +151,69 @@ export class InMemoryProvider extends DbProvider {
151151
}
152152

153153
/**
154-
* Overrides the base DbProvider.put() shortcut to expose InMemoryStore's indexNames scoping (see
155-
* InMemoryStore.put() for the full rationale). This is intentionally NOT part of the shared DbStore/
156-
* DbProvider interfaces: scoping which index(es) get populated only makes sense for an in-memory cache
157-
* that's re-derived from a real database, never for the database itself, which must always keep every
158-
* index consistent with the data it stores. Keeping it off the shared interfaces makes it a compile error
159-
* to pass indexNames to any other provider (e.g. IndexedDbProvider) -- callers must have a reference typed
160-
* as InMemoryProvider (not the generic DbProvider) to use this parameter at all.
154+
* Deliberately NOT named/overloaded as `put()`, and deliberately not reachable except through
155+
* `asScopedIndexPutProvider()` below -- see there for the full rationale. Exposes InMemoryStore's
156+
* indexNames scoping (see InMemoryStore.putInIndexAfterGet_DoNotUse() for the full rationale).
157+
* This is intentionally NOT part of the shared DbStore/DbProvider interfaces: scoping which
158+
* index(es) get populated only makes sense for an in-memory cache that's re-derived from a real
159+
* database, never for the database itself, which must always keep every index consistent with
160+
* the data it stores.
161+
*
162+
* DO NOT USE unless you're re-populating an in-memory cache from a ranged read that only ever
163+
* touched the listed index(es) -- e.g. right after a `getRange()`/`getMultiple()` served by a
164+
* single index. Using this for any other kind of write will leave the *other* indexes on this
165+
* store permanently missing the item(s), silently diverging from the normal guarantee (shared by
166+
* every other DbProvider, including this same InMemoryProvider's own `put()`) that every index on
167+
* a store always reflects every item in that store.
161168
*/
162-
put(
169+
putInIndexAfterGet_DoNotUse(
163170
storeName: string,
164171
itemOrItems: ItemType | ItemType[],
165-
indexNames?: string[]
172+
indexNames: string[]
166173
): Promise<void> {
167174
return this._getStoreTransaction(storeName, true).then((store) => {
168-
return (store as InMemoryStore).put(itemOrItems, indexNames);
175+
return (store as InMemoryStore).putInIndexAfterGet_DoNotUse(
176+
itemOrItems,
177+
indexNames
178+
);
169179
});
170180
}
171181
}
172182

183+
/**
184+
* Specialized capability for providers that can scope in-memory index writes to only the
185+
* index(es) that were actually queried, instead of populating every index on the store (see
186+
* `InMemoryProvider.putInIndexAfterGet_DoNotUse()` for the full rationale and warnings).
187+
*
188+
* This is intentionally kept off the public `InMemoryProvider` class surface -- and off the
189+
* shared `DbStore`/`DbProvider` interfaces entirely -- so that reaching for it always requires
190+
* going through `asScopedIndexPutProvider()` below rather than casting/typing a `DbProvider`
191+
* reference as `InMemoryProvider` and calling it directly.
192+
*/
193+
export interface IScopedIndexPutProvider {
194+
putInIndexAfterGet_DoNotUse(
195+
storeName: string,
196+
itemOrItems: ItemType | ItemType[],
197+
indexNames: string[]
198+
): Promise<void>;
199+
}
200+
201+
/**
202+
* The only supported way to reach the `IScopedIndexPutProvider` capability described above.
203+
*
204+
* Returns `provider` narrowed to `IScopedIndexPutProvider` when it actually supports scoped index
205+
* puts (currently: any `InMemoryProvider` instance), or `undefined` otherwise. Routing through this
206+
* helper -- instead of casting a `DbProvider` to `InMemoryProvider` -- makes every call site that
207+
* opts into breaking the normal "every index stays in sync" guarantee explicit and easy to find/audit.
208+
*/
209+
export function asScopedIndexPutProvider(
210+
provider: DbProvider
211+
): IScopedIndexPutProvider | undefined {
212+
return provider instanceof InMemoryProvider
213+
? (provider as unknown as IScopedIndexPutProvider)
214+
: undefined;
215+
}
216+
173217
// Notes: Doesn't limit the stores it can fetch to those in the stores it was "created" with, nor does it handle read-only transactions
174218
class InMemoryTransaction implements DbTransaction {
175219
private _stores: Map<string, InMemoryStore> = new Map();
@@ -348,18 +392,33 @@ class InMemoryStore implements DbStore {
348392
);
349393
}
350394

395+
put(itemOrItems: ItemType | ItemType[]): Promise<void> {
396+
return this._putInternal(itemOrItems);
397+
}
398+
351399
/**
352-
* @param indexNames Optional scoping hint: when provided, brand-new items (not already present in the
353-
* store) are only written into the primary key plus the listed index(es), instead of every index on the
354-
* store. This lets callers who fetched data through a single index (e.g. a ranged read served from that
355-
* index) cache the results without seeding "islands" of items into unrelated indexes that were never
356-
* actually queried/loaded for those items. Items that already exist in the store keep being kept in sync
357-
* across every index they were previously tracked by, so already-cached data never goes stale.
400+
* DO NOT USE unless you're re-populating an in-memory cache from a ranged read that only ever
401+
* touched the listed index(es) -- see `InMemoryProvider.putInIndexAfterGet_DoNotUse()` for the
402+
* full rationale and warnings. Deliberately named/kept separate from `put()` above (rather than
403+
* an optional 3rd parameter on it) so that the normal, always-safe `put()` required by the shared
404+
* `DbStore` interface can never accidentally be called with scoping semantics.
358405
*
359-
* NOTE: this parameter is intentionally NOT part of the shared DbStore interface -- it's only reachable
360-
* via InMemoryProvider.put() (see there), so it's a compile error to use it against any other provider.
406+
* @param indexNames Scoping hint: brand-new items (not already present in the store) are only
407+
* written into the primary key plus the listed index(es), instead of every index on the store.
408+
* This lets callers who fetched data through a single index (e.g. a ranged read served from that
409+
* index) cache the results without seeding "islands" of items into unrelated indexes that were
410+
* never actually queried/loaded for those items. Items that already exist in the store keep being
411+
* kept in sync across every index they were previously tracked by, so already-cached data never
412+
* goes stale.
361413
*/
362-
put(
414+
putInIndexAfterGet_DoNotUse(
415+
itemOrItems: ItemType | ItemType[],
416+
indexNames: string[]
417+
): Promise<void> {
418+
return this._putInternal(itemOrItems, indexNames);
419+
}
420+
421+
private _putInternal(
363422
itemOrItems: ItemType | ItemType[],
364423
indexNames?: string[]
365424
): Promise<void> {

src/ObjectStoreProvider.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -266,8 +266,8 @@ export abstract class DbProvider {
266266

267267
/**
268268
* Protected (rather than private) so that subclasses which need extra, provider-specific put() semantics
269-
* (e.g. InMemoryProvider's indexNames scoping -- see its put() override) can reuse this instead of
270-
* re-implementing store-transaction resolution.
269+
* (e.g. InMemoryProvider's `putInIndexAfterGet_DoNotUse()` indexNames scoping -- see there) can reuse
270+
* this instead of re-implementing store-transaction resolution.
271271
*/
272272
protected _getStoreTransaction(
273273
storeName: string,

src/tests/ObjectStoreProvider.spec.ts

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import {
1515
UpgradeCallback,
1616
} from "../ObjectStoreProvider";
1717

18-
import { InMemoryProvider } from "../InMemoryProvider";
18+
import { InMemoryProvider, asScopedIndexPutProvider } from "../InMemoryProvider";
1919
import { IndexedDbProvider, IndexedDbTransaction } from "../IndexedDbProvider";
2020
import * as IndexedDbProviderModule from "../IndexedDbProvider";
2121
import {
@@ -2126,14 +2126,20 @@ describe("ObjectStoreProvider", function () {
21262126
true
21272127
)
21282128
.then((prov) => {
2129-
// indexNames scoping is only reachable via a reference typed as InMemoryProvider -- it's
2130-
// intentionally not part of the shared DbStore/DbProvider interfaces so it's a compile error
2131-
// to use it against any other provider (e.g. the real IndexedDbProvider/db path).
2132-
const memoryProv = prov as InMemoryProvider;
2133-
return memoryProv
2134-
.put("test", { id: "item1", a: "valA1", b: "valB1" }, [
2135-
"indexA",
2136-
])
2129+
// indexNames scoping is only reachable via asScopedIndexPutProvider() -- it's
2130+
// intentionally not part of the shared DbStore/DbProvider interfaces (nor of
2131+
// InMemoryProvider's public put()), so this returns undefined for any provider that
2132+
// doesn't support it, and it'd be a compile error to call
2133+
// putInIndexAfterGet_DoNotUse() directly against a plain DbProvider reference.
2134+
const maybeScopedPutProv = asScopedIndexPutProvider(prov);
2135+
assert(!!maybeScopedPutProv);
2136+
const scopedPutProv = maybeScopedPutProv!!!;
2137+
return scopedPutProv
2138+
.putInIndexAfterGet_DoNotUse(
2139+
"test",
2140+
{ id: "item1", a: "valA1", b: "valB1" },
2141+
["indexA"]
2142+
)
21372143
.then(() => {
21382144
return Promise.all([
21392145
prov.get("test", "item1"),
@@ -2157,8 +2163,8 @@ describe("ObjectStoreProvider", function () {
21572163
// Once the item is already tracked in memory, subsequent scoped puts (e.g. an update
21582164
// fetched again via indexA) must keep it in sync across every index it's already part
21592165
// of, rather than leaving stale/missing entries in indexes that were skipped this time.
2160-
return memoryProv
2161-
.put(
2166+
return scopedPutProv
2167+
.putInIndexAfterGet_DoNotUse(
21622168
"test",
21632169
{ id: "item1", a: "valA1-updated", b: "valB1-updated" },
21642170
["indexA"]

0 commit comments

Comments
 (0)