Skip to content

Commit 05b12c7

Browse files
committed
fix: throw error if api key doesnt exist
1 parent 0bfeea7 commit 05b12c7

File tree

5 files changed

+37
-21
lines changed

5 files changed

+37
-21
lines changed

rulesets/tests/adapters.md

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ This document outlines all features that must be implemented and tested for each
1010
- Saves all metadata fields correctly
1111
- Handles concurrent saves
1212
- Returns without errors on success
13+
- Overwrites existing record with same ID
14+
- Preserves record integrity after save
1315

1416
- [ ] **findByHash()** - Lookup by key hash
1517
- Returns correct record when exists
@@ -34,16 +36,21 @@ This document outlines all features that must be implemented and tested for each
3436
- Preserves unchanged fields
3537
- Throws error if key not found
3638
- Atomically updates entire metadata object
39+
- Multiple updates to same key work correctly
3740

3841
- [ ] **delete()** - Single record deletion
3942
- Removes record from storage
4043
- No error if already deleted
4144
- Does not affect other records
45+
- Removes record from hash index (findByHash returns null after delete)
46+
- Idempotent operation (multiple deletes don't error)
4247

4348
- [ ] **deleteByOwner()** - Bulk deletion by owner
4449
- Deletes all keys for given owner
4550
- Does not delete keys from other owners
4651
- Returns without error when none exist
52+
- Removes all hash indexes for deleted keys
53+
- Idempotent operation (multiple calls don't error)
4754

4855
## Metadata Field Support
4956

@@ -178,10 +185,14 @@ This document outlines all features that must be implemented and tested for each
178185

179186
### Concurrent Operations
180187

181-
- [ ] Concurrent saves
182-
- [ ] Concurrent updates
183-
- [ ] Concurrent deletes
184-
- [ ] Read during write
188+
- [ ] Concurrent saves to different records
189+
- [ ] Concurrent saves to same record (last write wins)
190+
- [ ] Concurrent updates to different records
191+
- [ ] Concurrent updates to same record
192+
- [ ] Concurrent deletes don't conflict
193+
- [ ] Read during write operations
194+
- [ ] findByHash during concurrent saves
195+
- [ ] No data corruption under concurrency
185196

186197
### Error Handling
187198

@@ -300,11 +311,11 @@ This document outlines all features that must be implemented and tested for each
300311

301312
## Checklist Summary
302313

303-
Total features: **87**
304-
- Core Storage: 7 operations
314+
Total features: **97**
315+
- Core Storage: 7 operations (+5 sub-requirements)
305316
- Metadata Fields: 11 fields with various scenarios
306317
- Data Types: 8 type handling scenarios
307-
- Edge Cases: 10 edge case categories
318+
- Edge Cases: 10 edge case categories (+4 concurrency items)
308319
- Query Operations: 6 query patterns
309320
- Performance: 7 performance characteristics
310321
- Integration: 7 integration points

src/storage/memory.test.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -135,9 +135,10 @@ describe("MemoryStore", () => {
135135
expect(found?.metadata.ownerId).toBe("user_123");
136136
});
137137

138-
it("should do nothing for non-existent ID", async () => {
139-
await store.updateMetadata("non-existent", { name: "New Name" });
140-
// Should not throw
138+
it("should throw error for non-existent ID", async () => {
139+
await expect(
140+
store.updateMetadata("non-existent", { name: "New Name" })
141+
).rejects.toThrow("API key with id non-existent not found");
141142
});
142143
});
143144

src/storage/memory.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,10 @@ export class MemoryStore implements Storage {
3232
metadata: Partial<ApiKeyMetadata>
3333
): Promise<void> {
3434
const record = await this.keys.get(id);
35-
if (record) {
36-
record.metadata = { ...record.metadata, ...metadata };
35+
if (!record) {
36+
throw new Error(`API key with id ${id} not found`);
3737
}
38+
record.metadata = { ...record.metadata, ...metadata };
3839
}
3940

4041
async delete(id: string): Promise<void> {

src/storage/redis.test.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -172,9 +172,10 @@ describe("RedisStore", () => {
172172
expect(found?.metadata.ownerId).toBe("user_123");
173173
});
174174

175-
it("should do nothing for non-existent ID", async () => {
176-
await store.updateMetadata("non-existent", { name: "New Name" });
177-
// Should not throw
175+
it("should throw error for non-existent ID", async () => {
176+
await expect(
177+
store.updateMetadata("non-existent", { name: "New Name" })
178+
).rejects.toThrow("API key with id non-existent not found");
178179
});
179180
});
180181

src/storage/redis.ts

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -73,13 +73,15 @@ export class RedisStore implements Storage {
7373
metadata: Partial<ApiKeyMetadata>
7474
): Promise<void> {
7575
const record = await this.findById(id);
76-
if (record) {
77-
record.metadata = { ...record.metadata, ...metadata };
78-
await this.redis.set(this.key(id), JSON.stringify(record));
76+
if (!record) {
77+
throw new Error(`API key with id ${id} not found`);
78+
}
7979

80-
if (metadata.revokedAt) {
81-
await this.redis.del(this.hashKey(record.keyHash));
82-
}
80+
record.metadata = { ...record.metadata, ...metadata };
81+
await this.redis.set(this.key(id), JSON.stringify(record));
82+
83+
if (metadata.revokedAt) {
84+
await this.redis.del(this.hashKey(record.keyHash));
8385
}
8486
}
8587

0 commit comments

Comments
 (0)