Skip to content

Commit 77cd398

Browse files
ericallamericallam
committed
move to an external registry approach for storing and access the idempotency key user supplied key and options instead of using a String object so we don't break the contract of idempotencyKeys.create
1 parent 45934cc commit 77cd398

File tree

7 files changed

+290
-31
lines changed

7 files changed

+290
-31
lines changed

‎packages/core/src/v3/idempotency-key-catalog-api.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
// Split module-level variable definition into separate files to allow
2+
// tree-shaking on each api instance.
3+
import { IdempotencyKeyCatalogAPI } from "./idempotency-key-catalog/index.js";
4+
/** Entrypoint for idempotency key catalog API */
5+
export const idempotencyKeyCatalog = IdempotencyKeyCatalogAPI.getInstance();

‎packages/core/src/v3/idempotency-key-catalog/catalog.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
export type IdempotencyKeyScope = "run" | "attempt" | "global";
2+
3+
export type IdempotencyKeyOptions = {
4+
key: string;
5+
scope: IdempotencyKeyScope;
6+
};
7+
8+
export interface IdempotencyKeyCatalog {
9+
registerKeyOptions(hash: string, options: IdempotencyKeyOptions): void;
10+
getKeyOptions(hash: string): IdempotencyKeyOptions | undefined;
11+
}

‎packages/core/src/v3/idempotency-key-catalog/index.ts‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
const API_NAME = "idempotency-key-catalog";
2+
3+
import { getGlobal, registerGlobal } from "../utils/globals.js";
4+
import type { IdempotencyKeyCatalog, IdempotencyKeyOptions } from "./catalog.js";
5+
import { LRUIdempotencyKeyCatalog } from "./lruIdempotencyKeyCatalog.js";
6+
7+
export class IdempotencyKeyCatalogAPI {
8+
private static _instance?: IdempotencyKeyCatalogAPI;
9+
10+
private constructor() {}
11+
12+
public static getInstance(): IdempotencyKeyCatalogAPI {
13+
if (!this._instance) {
14+
this._instance = new IdempotencyKeyCatalogAPI();
15+
}
16+
return this._instance;
17+
}
18+
19+
public registerKeyOptions(hash: string, options: IdempotencyKeyOptions): void {
20+
this.#getCatalog().registerKeyOptions(hash, options);
21+
}
22+
23+
public getKeyOptions(hash: string): IdempotencyKeyOptions | undefined {
24+
return this.#getCatalog().getKeyOptions(hash);
25+
}
26+
27+
#getCatalog(): IdempotencyKeyCatalog {
28+
let catalog = getGlobal(API_NAME);
29+
if (!catalog) {
30+
// Auto-initialize with LRU catalog on first access
31+
catalog = new LRUIdempotencyKeyCatalog();
32+
registerGlobal(API_NAME, catalog, true);
33+
}
34+
return catalog;
35+
}
36+
}

‎packages/core/src/v3/idempotency-key-catalog/lruIdempotencyKeyCatalog.test.ts‎

Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
1+
import { describe, it, expect } from "vitest";
2+
import { LRUIdempotencyKeyCatalog } from "./lruIdempotencyKeyCatalog.js";
3+
4+
describe("LRUIdempotencyKeyCatalog", () => {
5+
describe("registerKeyOptions and getKeyOptions", () => {
6+
it("should store and retrieve options", () => {
7+
const catalog = new LRUIdempotencyKeyCatalog();
8+
const options = { key: "my-key", scope: "global" as const };
9+
10+
catalog.registerKeyOptions("hash1", options);
11+
12+
expect(catalog.getKeyOptions("hash1")).toEqual(options);
13+
});
14+
15+
it("should return undefined for non-existent keys", () => {
16+
const catalog = new LRUIdempotencyKeyCatalog();
17+
18+
expect(catalog.getKeyOptions("non-existent")).toBeUndefined();
19+
});
20+
21+
it("should store multiple keys", () => {
22+
const catalog = new LRUIdempotencyKeyCatalog();
23+
const options1 = { key: "key1", scope: "global" as const };
24+
const options2 = { key: "key2", scope: "run" as const };
25+
const options3 = { key: "key3", scope: "attempt" as const };
26+
27+
catalog.registerKeyOptions("hash1", options1);
28+
catalog.registerKeyOptions("hash2", options2);
29+
catalog.registerKeyOptions("hash3", options3);
30+
31+
expect(catalog.getKeyOptions("hash1")).toEqual(options1);
32+
expect(catalog.getKeyOptions("hash2")).toEqual(options2);
33+
expect(catalog.getKeyOptions("hash3")).toEqual(options3);
34+
});
35+
36+
it("should update options when registering same key twice", () => {
37+
const catalog = new LRUIdempotencyKeyCatalog();
38+
const options1 = { key: "key1", scope: "global" as const };
39+
const options2 = { key: "key1-updated", scope: "run" as const };
40+
41+
catalog.registerKeyOptions("hash1", options1);
42+
catalog.registerKeyOptions("hash1", options2);
43+
44+
expect(catalog.getKeyOptions("hash1")).toEqual(options2);
45+
});
46+
});
47+
48+
describe("LRU eviction", () => {
49+
it("should evict oldest entry when over capacity", () => {
50+
const catalog = new LRUIdempotencyKeyCatalog(3);
51+
52+
catalog.registerKeyOptions("hash1", { key: "key1", scope: "global" });
53+
catalog.registerKeyOptions("hash2", { key: "key2", scope: "global" });
54+
catalog.registerKeyOptions("hash3", { key: "key3", scope: "global" });
55+
56+
// All three should exist
57+
expect(catalog.getKeyOptions("hash1")).toBeDefined();
58+
expect(catalog.getKeyOptions("hash2")).toBeDefined();
59+
expect(catalog.getKeyOptions("hash3")).toBeDefined();
60+
61+
// Add a fourth - hash1 should be evicted (it was least recently used after the gets above moved others)
62+
// Note: After the gets above, the order is hash1, hash2, hash3 (hash1 was accessed first in the gets)
63+
// Actually let's reset and test more carefully
64+
});
65+
66+
it("should evict least recently registered entry when capacity exceeded", () => {
67+
const catalog = new LRUIdempotencyKeyCatalog(3);
68+
69+
catalog.registerKeyOptions("hash1", { key: "key1", scope: "global" });
70+
catalog.registerKeyOptions("hash2", { key: "key2", scope: "global" });
71+
catalog.registerKeyOptions("hash3", { key: "key3", scope: "global" });
72+
73+
// Adding fourth should evict hash1 (oldest)
74+
catalog.registerKeyOptions("hash4", { key: "key4", scope: "global" });
75+
76+
expect(catalog.getKeyOptions("hash1")).toBeUndefined();
77+
expect(catalog.getKeyOptions("hash2")).toBeDefined();
78+
expect(catalog.getKeyOptions("hash3")).toBeDefined();
79+
expect(catalog.getKeyOptions("hash4")).toBeDefined();
80+
});
81+
82+
it("should evict multiple entries when adding many at once would exceed capacity", () => {
83+
const catalog = new LRUIdempotencyKeyCatalog(2);
84+
85+
catalog.registerKeyOptions("hash1", { key: "key1", scope: "global" });
86+
catalog.registerKeyOptions("hash2", { key: "key2", scope: "global" });
87+
catalog.registerKeyOptions("hash3", { key: "key3", scope: "global" });
88+
catalog.registerKeyOptions("hash4", { key: "key4", scope: "global" });
89+
90+
// Only hash3 and hash4 should remain
91+
expect(catalog.getKeyOptions("hash1")).toBeUndefined();
92+
expect(catalog.getKeyOptions("hash2")).toBeUndefined();
93+
expect(catalog.getKeyOptions("hash3")).toBeDefined();
94+
expect(catalog.getKeyOptions("hash4")).toBeDefined();
95+
});
96+
97+
it("should work with maxSize of 1", () => {
98+
const catalog = new LRUIdempotencyKeyCatalog(1);
99+
100+
catalog.registerKeyOptions("hash1", { key: "key1", scope: "global" });
101+
expect(catalog.getKeyOptions("hash1")).toBeDefined();
102+
103+
catalog.registerKeyOptions("hash2", { key: "key2", scope: "global" });
104+
expect(catalog.getKeyOptions("hash1")).toBeUndefined();
105+
expect(catalog.getKeyOptions("hash2")).toBeDefined();
106+
});
107+
});
108+
109+
describe("LRU ordering", () => {
110+
it("should move accessed key to most recent position", () => {
111+
const catalog = new LRUIdempotencyKeyCatalog(3);
112+
113+
catalog.registerKeyOptions("hash1", { key: "key1", scope: "global" });
114+
catalog.registerKeyOptions("hash2", { key: "key2", scope: "global" });
115+
catalog.registerKeyOptions("hash3", { key: "key3", scope: "global" });
116+
117+
// Access hash1, moving it to most recent
118+
catalog.getKeyOptions("hash1");
119+
120+
// Add hash4 - should evict hash2 (now the oldest)
121+
catalog.registerKeyOptions("hash4", { key: "key4", scope: "global" });
122+
123+
expect(catalog.getKeyOptions("hash1")).toBeDefined();
124+
expect(catalog.getKeyOptions("hash2")).toBeUndefined();
125+
expect(catalog.getKeyOptions("hash3")).toBeDefined();
126+
expect(catalog.getKeyOptions("hash4")).toBeDefined();
127+
});
128+
129+
it("should move re-registered key to most recent position", () => {
130+
const catalog = new LRUIdempotencyKeyCatalog(3);
131+
132+
catalog.registerKeyOptions("hash1", { key: "key1", scope: "global" });
133+
catalog.registerKeyOptions("hash2", { key: "key2", scope: "global" });
134+
catalog.registerKeyOptions("hash3", { key: "key3", scope: "global" });
135+
136+
// Re-register hash1, moving it to most recent
137+
catalog.registerKeyOptions("hash1", { key: "key1-updated", scope: "run" });
138+
139+
// Add hash4 - should evict hash2 (now the oldest)
140+
catalog.registerKeyOptions("hash4", { key: "key4", scope: "global" });
141+
142+
expect(catalog.getKeyOptions("hash1")).toEqual({ key: "key1-updated", scope: "run" });
143+
expect(catalog.getKeyOptions("hash2")).toBeUndefined();
144+
expect(catalog.getKeyOptions("hash3")).toBeDefined();
145+
expect(catalog.getKeyOptions("hash4")).toBeDefined();
146+
});
147+
148+
it("should not affect order when getting non-existent key", () => {
149+
const catalog = new LRUIdempotencyKeyCatalog(2);
150+
151+
catalog.registerKeyOptions("hash1", { key: "key1", scope: "global" });
152+
catalog.registerKeyOptions("hash2", { key: "key2", scope: "global" });
153+
154+
// Try to get non-existent key
155+
catalog.getKeyOptions("non-existent");
156+
157+
// Add hash3 - should still evict hash1 (oldest)
158+
catalog.registerKeyOptions("hash3", { key: "key3", scope: "global" });
159+
160+
expect(catalog.getKeyOptions("hash1")).toBeUndefined();
161+
expect(catalog.getKeyOptions("hash2")).toBeDefined();
162+
expect(catalog.getKeyOptions("hash3")).toBeDefined();
163+
});
164+
});
165+
166+
describe("default maxSize", () => {
167+
it("should use default maxSize of 1000", () => {
168+
const catalog = new LRUIdempotencyKeyCatalog();
169+
170+
// Register 1001 entries
171+
for (let i = 0; i < 1001; i++) {
172+
catalog.registerKeyOptions(`hash${i}`, { key: `key${i}`, scope: "global" });
173+
}
174+
175+
// First entry should be evicted
176+
expect(catalog.getKeyOptions("hash0")).toBeUndefined();
177+
// Last entry should exist
178+
expect(catalog.getKeyOptions("hash1000")).toBeDefined();
179+
});
180+
});
181+
});

‎packages/core/src/v3/idempotency-key-catalog/lruIdempotencyKeyCatalog.ts‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
import type { IdempotencyKeyCatalog, IdempotencyKeyOptions } from "./catalog.js";
2+
3+
export class LRUIdempotencyKeyCatalog implements IdempotencyKeyCatalog {
4+
private cache: Map<string, IdempotencyKeyOptions>;
5+
private readonly maxSize: number;
6+
7+
constructor(maxSize: number = 1_000) {
8+
this.cache = new Map();
9+
this.maxSize = maxSize;
10+
}
11+
12+
registerKeyOptions(hash: string, options: IdempotencyKeyOptions): void {
13+
// Delete and re-add to update position (most recently used)
14+
this.cache.delete(hash);
15+
this.cache.set(hash, options);
16+
17+
// Evict oldest entries if over capacity
18+
while (this.cache.size > this.maxSize) {
19+
const oldest = this.cache.keys().next().value;
20+
if (oldest !== undefined) {
21+
this.cache.delete(oldest);
22+
}
23+
}
24+
}
25+
26+
getKeyOptions(hash: string): IdempotencyKeyOptions | undefined {
27+
const options = this.cache.get(hash);
28+
if (options) {
29+
// Move to end (most recently used)
30+
this.cache.delete(hash);
31+
this.cache.set(hash, options);
32+
}
33+
return options;
34+
}
35+
}

‎packages/core/src/v3/idempotencyKeys.ts‎

Lines changed: 20 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,16 @@
11
import { apiClientManager } from "./apiClientManager-api.js";
2+
import { idempotencyKeyCatalog } from "./idempotency-key-catalog-api.js";
3+
import type {
4+
IdempotencyKeyOptions,
5+
IdempotencyKeyScope,
6+
} from "./idempotency-key-catalog/catalog.js";
27
import { taskContext } from "./task-context-api.js";
38
import { IdempotencyKey } from "./types/idempotencyKeys.js";
49
import { digestSHA256 } from "./utils/crypto.js";
510
import type { ZodFetchOptions } from "./apiClient/core.js";
611

7-
export type IdempotencyKeyScope = "run" | "attempt" | "global";
8-
9-
export type IdempotencyKeyOptions = {
10-
key: string;
11-
scope: IdempotencyKeyScope;
12-
};
13-
14-
const IDEMPOTENCY_KEY_OPTIONS_SYMBOL = Symbol.for("__idempotencyKeyOptions");
12+
// Re-export types from catalog for backwards compatibility
13+
export type { IdempotencyKeyScope, IdempotencyKeyOptions } from "./idempotency-key-catalog/catalog.js";
1514

1615
/**
1716
* Extracts the user-provided key and scope from an idempotency key created with `idempotencyKeys.create()`.
@@ -29,34 +28,21 @@ const IDEMPOTENCY_KEY_OPTIONS_SYMBOL = Symbol.for("__idempotencyKeyOptions");
2928
export function getIdempotencyKeyOptions(
3029
idempotencyKey: IdempotencyKey | string
3130
): IdempotencyKeyOptions | undefined {
32-
return (idempotencyKey as any)[IDEMPOTENCY_KEY_OPTIONS_SYMBOL];
33-
}
34-
35-
/**
36-
* Attaches idempotency key options to a String object for later extraction.
37-
* @internal
38-
*/
39-
function attachIdempotencyKeyOptions(
40-
idempotencyKey: string,
41-
options: IdempotencyKeyOptions
42-
): IdempotencyKey {
43-
const result = new String(idempotencyKey) as IdempotencyKey;
44-
(result as any)[IDEMPOTENCY_KEY_OPTIONS_SYMBOL] = options;
45-
return result;
31+
// Look up options from the catalog using the hash string
32+
if (typeof idempotencyKey === "string") {
33+
return idempotencyKeyCatalog.getKeyOptions(idempotencyKey);
34+
}
35+
return undefined;
4636
}
4737

4838
export function isIdempotencyKey(
4939
value: string | string[] | IdempotencyKey
5040
): value is IdempotencyKey {
5141
// Cannot check the brand at runtime because it doesn't exist (it's a TypeScript-only construct)
52-
// Check for both primitive strings and String objects (created via new String())
53-
// String objects have typeof "object" so we also check instanceof String
42+
// Check for primitive strings only (we no longer use String objects)
5443
if (typeof value === "string") {
5544
return value.length === 64;
5645
}
57-
if (value instanceof String) {
58-
return value.length === 64;
59-
}
6046
return false;
6147
}
6248

@@ -148,8 +134,11 @@ export async function createIdempotencyKey(
148134

149135
const idempotencyKey = await generateIdempotencyKey(keyArray.concat(injectScope(scope)));
150136

151-
// Attach the original key and scope as metadata for later extraction
152-
return attachIdempotencyKeyOptions(idempotencyKey, { key: userKey, scope });
137+
// Register the original key and scope in the catalog for later extraction
138+
idempotencyKeyCatalog.registerKeyOptions(idempotencyKey, { key: userKey, scope });
139+
140+
// Return primitive string cast as IdempotencyKey
141+
return idempotencyKey as IdempotencyKey;
153142
}
154143

155144
function injectScope(scope: IdempotencyKeyScope): string[] {
@@ -238,8 +227,8 @@ export async function resetIdempotencyKey(
238227

239228
// Try to extract options from an IdempotencyKey created with idempotencyKeys.create()
240229
const attachedOptions =
241-
typeof idempotencyKey === "string" || idempotencyKey instanceof String
242-
? getIdempotencyKeyOptions(idempotencyKey as IdempotencyKey)
230+
typeof idempotencyKey === "string"
231+
? getIdempotencyKeyOptions(idempotencyKey)
243232
: undefined;
244233

245234
const scope = attachedOptions?.scope ?? options?.scope ?? "run";

‎packages/core/src/v3/utils/globals.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { ApiClientConfiguration } from "../apiClientManager/types.js";
22
import { Clock } from "../clock/clock.js";
33
import { HeartbeatsManager } from "../heartbeats/types.js";
4+
import type { IdempotencyKeyCatalog } from "../idempotency-key-catalog/catalog.js";
45
import { LifecycleHooksManager } from "../lifecycleHooks/types.js";
56
import { LocalsManager } from "../locals/types.js";
67
import { RealtimeStreamsManager } from "../realtimeStreams/types.js";
@@ -61,6 +62,7 @@ type TriggerDotDevGlobalAPI = {
6162
clock?: Clock;
6263
usage?: UsageManager;
6364
["resource-catalog"]?: ResourceCatalog;
65+
["idempotency-key-catalog"]?: IdempotencyKeyCatalog;
6466
["task-context"]?: TaskContext;
6567
["api-client"]?: ApiClientConfiguration;
6668
["run-metadata"]?: RunMetadataManager;

0 commit comments

Comments
 (0)