Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
92 changes: 92 additions & 0 deletions packages/account-usage/lib/usage.integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@
});

afterAll(async () => {
await redis.disconnect();

Check failure on line 22 in packages/account-usage/lib/usage.integration.test.ts

View workflow job for this annotation

GitHub Actions / tests

Unhandled error

Error: Disconnects client ❯ Commander.disconnect node_modules/@redis/client/dist/lib/client/index.js:342:72 ❯ packages/account-usage/lib/usage.integration.test.ts:22:21 ❯ node_modules/@vitest/runner/dist/chunk-hooks.js:1897:20 ❯ runWithTimeout node_modules/@vitest/runner/dist/chunk-hooks.js:1863:10 ❯ runHook node_modules/@vitest/runner/dist/chunk-hooks.js:1436:51 ❯ callSuiteHook node_modules/@vitest/runner/dist/chunk-hooks.js:1442:25 ❯ runSuite node_modules/@vitest/runner/dist/chunk-hooks.js:1738:10 ❯ runSuite node_modules/@vitest/runner/dist/chunk-hooks.js:1729:8 ❯ runFiles node_modules/@vitest/runner/dist/chunk-hooks.js:1787:3 ❯ startTests node_modules/@vitest/runner/dist/chunk-hooks.js:1820:3 ❯ node_modules/vitest/dist/chunks/runBaseTests.9Ij9_de-.js:117:26 ❯ withEnv node_modules/vitest/dist/chunks/runBaseTests.9Ij9_de-.js:84:3 ❯ run node_modules/vitest/dist/chunks/runBaseTests.9Ij9_de-.js:109:2 ❯ runBaseTests node_modules/vitest/dist/chunks/base.DfmxU-tU.js:32:2 ❯ ForksBaseWorker.executeTests node_modules/vitest/dist/workers/forks.js:29:4 This error originated in "packages/account-usage/lib/usage.integration.test.ts" test file. It doesn't mean the error was thrown inside the file itself, but while it was running.
});

beforeEach(async () => {

Check failure on line 25 in packages/account-usage/lib/usage.integration.test.ts

View workflow job for this annotation

GitHub Actions / tests

packages/account-usage/lib/usage.integration.test.ts > Usage > getAll > should trigger revalidation for stale entries

Error: Hook timed out in 10000ms. If this is a long-running hook, pass a timeout value as the last argument or configure it globally with "hookTimeout". ❯ packages/account-usage/lib/usage.integration.test.ts:25:5

Check failure on line 25 in packages/account-usage/lib/usage.integration.test.ts

View workflow job for this annotation

GitHub Actions / tests

packages/account-usage/lib/usage.integration.test.ts > Usage > getAll > should trigger revalidation for null entries

Error: Hook timed out in 10000ms. If this is a long-running hook, pass a timeout value as the last argument or configure it globally with "hookTimeout". ❯ packages/account-usage/lib/usage.integration.test.ts:25:5

Check failure on line 25 in packages/account-usage/lib/usage.integration.test.ts

View workflow job for this annotation

GitHub Actions / tests

packages/account-usage/lib/usage.integration.test.ts > Usage > incr > should increment monthly metric

Error: Hook timed out in 10000ms. If this is a long-running hook, pass a timeout value as the last argument or configure it globally with "hookTimeout". ❯ packages/account-usage/lib/usage.integration.test.ts:25:5

Check failure on line 25 in packages/account-usage/lib/usage.integration.test.ts

View workflow job for this annotation

GitHub Actions / tests

packages/account-usage/lib/usage.integration.test.ts > Usage > incr > should increment metric

Error: Hook timed out in 10000ms. If this is a long-running hook, pass a timeout value as the last argument or configure it globally with "hookTimeout". ❯ packages/account-usage/lib/usage.integration.test.ts:25:5

Check failure on line 25 in packages/account-usage/lib/usage.integration.test.ts

View workflow job for this annotation

GitHub Actions / tests

packages/account-usage/lib/usage.integration.test.ts > Usage > get > should not trigger revalidation when entry exists and is not stale

Error: Hook timed out in 10000ms. If this is a long-running hook, pass a timeout value as the last argument or configure it globally with "hookTimeout". ❯ packages/account-usage/lib/usage.integration.test.ts:25:5

Check failure on line 25 in packages/account-usage/lib/usage.integration.test.ts

View workflow job for this annotation

GitHub Actions / tests

packages/account-usage/lib/usage.integration.test.ts > Usage > get > should trigger revalidation when revalidateAfter has passed

Error: Hook timed out in 10000ms. If this is a long-running hook, pass a timeout value as the last argument or configure it globally with "hookTimeout". ❯ packages/account-usage/lib/usage.integration.test.ts:25:5

Check failure on line 25 in packages/account-usage/lib/usage.integration.test.ts

View workflow job for this annotation

GitHub Actions / tests

packages/account-usage/lib/usage.integration.test.ts > Usage > get > should trigger revalidation when entry is null

Error: Hook timed out in 10000ms. If this is a long-running hook, pass a timeout value as the last argument or configure it globally with "hookTimeout". ❯ packages/account-usage/lib/usage.integration.test.ts:25:5

Check failure on line 25 in packages/account-usage/lib/usage.integration.test.ts

View workflow job for this annotation

GitHub Actions / tests

packages/account-usage/lib/usage.integration.test.ts > Usage > get > should return the current value for an existing metric

Error: Hook timed out in 10000ms. If this is a long-running hook, pass a timeout value as the last argument or configure it globally with "hookTimeout". ❯ packages/account-usage/lib/usage.integration.test.ts:25:5

Check failure on line 25 in packages/account-usage/lib/usage.integration.test.ts

View workflow job for this annotation

GitHub Actions / tests

packages/account-usage/lib/usage.integration.test.ts > Usage > get > should return an error if entry is invalid

Error: Hook timed out in 10000ms. If this is a long-running hook, pass a timeout value as the last argument or configure it globally with "hookTimeout". ❯ packages/account-usage/lib/usage.integration.test.ts:25:5
await redis.flushAll(); // Clear all usage data before each test
vi.useFakeTimers({ shouldAdvanceTime: true });
});
Expand Down Expand Up @@ -57,6 +57,49 @@
const res = (await usageTracker.get({ accountId, metric })).unwrap();
expect(res).toEqual({ accountId, metric, current: 5 });
});
it('should trigger revalidation when entry is null', async () => {
const accountId = 1;
const metric = 'connections';

const revalidateSpy = vi.spyOn(usageTracker, 'revalidate');
revalidateSpy.mockReturnValue(Promise.resolve(Ok(undefined)));

const res = (await usageTracker.get({ accountId, metric })).unwrap();
expect(res).toEqual({ accountId, metric, current: 0 });
expect(revalidateSpy).toHaveBeenCalledTimes(1);
expect(revalidateSpy).toHaveBeenCalledWith({ accountId, metric });
});
it('should trigger revalidation when revalidateAfter has passed', async () => {
const accountId = 1;
const metric = 'connections';

// Set up an entry with a past revalidateAfter
await usageTracker.incr({ accountId, metric, delta: 5 });
// Move time forward by 1 day to pass revalidateAfter
vi.advanceTimersByTime(24 * 60 * 60 * 1000);

const revalidateSpy = vi.spyOn(usageTracker, 'revalidate');
revalidateSpy.mockReturnValue(Promise.resolve(Ok(undefined)));

const res = (await usageTracker.get({ accountId, metric })).unwrap();
expect(res).toEqual({ accountId, metric, current: 5 });
expect(revalidateSpy).toHaveBeenCalledTimes(1);
expect(revalidateSpy).toHaveBeenCalledWith({ accountId, metric });
});
it('should not trigger revalidation when entry exists and is not stale', async () => {
const accountId = 1;
const metric = 'connections';

await usageTracker.incr({ accountId, metric, delta: 5 });

const revalidateSpy = vi.spyOn(usageTracker, 'revalidate');
revalidateSpy.mockReturnValue(Promise.resolve(Ok(undefined)));

const res = (await usageTracker.get({ accountId, metric })).unwrap();
expect(res).toEqual({ accountId, metric, current: 5 });
// revalidateAfter hasn't passed yet
expect(revalidateSpy).not.toHaveBeenCalled();
});
});

describe('incr', () => {
Expand Down Expand Up @@ -89,4 +132,53 @@
expect(res).toEqual({ accountId, metric, current: 1 });
});
});

describe('getAll', () => {
it('should trigger revalidation for null entries', async () => {
const accountId = 1;

const revalidateSpy = vi.spyOn(usageTracker, 'revalidate');
revalidateSpy.mockReturnValue(Promise.resolve(Ok(undefined)));

const res = (await usageTracker.getAll(accountId)).unwrap();
expect(res).toBeDefined();
// Should trigger revalidation for all metrics that are null
expect(revalidateSpy).toHaveBeenCalled();
});
it('should trigger revalidation for stale entries', async () => {
const accountId = 1;
const metric = 'connections';

// Set up an entry with a past revalidateAfter
await usageTracker.incr({ accountId, metric, delta: 5 });
// Move time forward by 1 day to pass revalidateAfter
vi.advanceTimersByTime(24 * 60 * 60 * 1000);

const revalidateSpy = vi.spyOn(usageTracker, 'revalidate');
revalidateSpy.mockReturnValue(Promise.resolve(Ok(undefined)));

const res = (await usageTracker.getAll(accountId)).unwrap();
expect(res).toBeDefined();
expect(res[metric]).toEqual({ accountId, metric, current: 5 });
// Should trigger revalidation for the stale metric
expect(revalidateSpy).toHaveBeenCalledWith({ accountId, metric });
});
it('should not trigger revalidation for non-stale entries', async () => {
const accountId = 1;
const metric = 'connections';

await usageTracker.incr({ accountId, metric, delta: 5 });

const revalidateSpy = vi.spyOn(usageTracker, 'revalidate');
revalidateSpy.mockReturnValue(Promise.resolve(Ok(undefined)));

const res = (await usageTracker.getAll(accountId)).unwrap();
expect(res).toBeDefined();
expect(res[metric]).toEqual({ accountId, metric, current: 5 });
// revalidateAfter hasn't passed yet, so revalidation should not be called for this metric
// (but may be called for other null metrics)
const callsForMetric = revalidateSpy.mock.calls.filter((call) => call[0].metric === metric);
expect(callsForMetric).toHaveLength(0);
});
});
});
6 changes: 6 additions & 0 deletions packages/account-usage/lib/usage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,9 @@ export class UsageTracker implements IUsageTracker {
if (entry.isErr()) {
return Err(entry.error);
}
if (entry.value === null || entry.value.revalidateAfter < now.getTime()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

how can value be null?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On the first time trying to get it from the cache, when it doesn't exist yet. Down there it defaults to 0, but ideally we take the hint to revalidate.

void this.revalidate({ accountId, metric });
}
return Ok({
accountId,
metric,
Expand All @@ -105,6 +108,9 @@ export class UsageTracker implements IUsageTracker {
if (entry.isErr()) {
return;
}
if (entry.value === null || entry.value.revalidateAfter < now.getTime()) {
void this.revalidate({ accountId, metric: metric as UsageMetric });
}
result[metric as UsageMetric] = {
accountId,
metric: metric as UsageMetric,
Expand Down
Loading