-
-
Notifications
You must be signed in to change notification settings - Fork 256
feat: Add StorageService for offloading large controller data #7192
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
278eaa9
e844fdd
19c7d6f
5fedbc0
5af0861
de5388c
e493d3e
81100c2
f5c5aec
04939f6
3fe4314
84adfd5
7ac8bfc
24b150e
65efd41
60efad1
2222e57
ce68a05
afde837
ae501a3
81c9cf8
625eee6
bda7243
d3a7e4e
389e50a
3aeafac
c46e2e2
1b48670
8050f20
9a65054
d6fe459
5b4faeb
363533a
f5ccb18
b5c5d79
8557995
7ccd865
a139816
5598df9
bbd8e39
cf634fb
6bf384a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Replace unknown with Json type from @metamask/utils for type safety - Add @metamask/utils as dependency (required for public API types) - Remove StoredDataWrapper - just stringify/parse values directly - Update StorageAdapter interface, StorageService, and InMemoryStorageAdapter
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,17 +1,8 @@ | ||
| import type { Json } from '@metamask/utils'; | ||
|
|
||
| import type { StorageAdapter } from './types'; | ||
| import { STORAGE_KEY_PREFIX } from './types'; | ||
|
|
||
| /** | ||
| * Wrapper for stored data with metadata. | ||
| * Each adapter can define its own wrapper structure. | ||
| */ | ||
| type StoredDataWrapper<T = unknown> = { | ||
| /** Timestamp when data was stored (milliseconds since epoch). */ | ||
| timestamp: number; | ||
| /** The actual data being stored. */ | ||
| data: T; | ||
| }; | ||
|
|
||
| /** | ||
| * In-memory storage adapter (default fallback). | ||
| * Implements the {@link StorageAdapter} interface using a Map. | ||
|
|
@@ -30,8 +21,8 @@ type StoredDataWrapper<T = unknown> = { | |
| * @example | ||
| * ```typescript | ||
| * const adapter = new InMemoryStorageAdapter(); | ||
| * await adapter.setItem('key', 'value'); | ||
| * const value = await adapter.getItem('key'); // Returns 'value' | ||
| * await adapter.setItem('SnapController', 'snap-id:sourceCode', 'const x = 1;'); | ||
| * const value = await adapter.getItem('SnapController', 'snap-id:sourceCode'); // 'const x = 1;' | ||
| * // After restart: data is lost | ||
| * ``` | ||
| */ | ||
|
|
@@ -51,13 +42,13 @@ export class InMemoryStorageAdapter implements StorageAdapter { | |
|
|
||
| /** | ||
| * Retrieve an item from in-memory storage. | ||
| * Deserializes and unwraps the stored data. | ||
| * Deserializes JSON data from storage. | ||
| * | ||
| * @param namespace - The controller namespace. | ||
| * @param key - The data key. | ||
| * @returns The unwrapped data, or null if not found. | ||
| * @returns The parsed JSON data, or null if not found. | ||
| */ | ||
| async getItem(namespace: string, key: string): Promise<unknown> { | ||
| async getItem(namespace: string, key: string): Promise<Json | null> { | ||
| const fullKey = `${STORAGE_KEY_PREFIX}${namespace}:${key}`; | ||
| const serialized = this.#storage.get(fullKey); | ||
|
|
||
|
|
@@ -66,8 +57,7 @@ export class InMemoryStorageAdapter implements StorageAdapter { | |
| } | ||
|
|
||
| try { | ||
| const wrapper: StoredDataWrapper = JSON.parse(serialized); | ||
| return wrapper.data; | ||
| return JSON.parse(serialized) as Json; | ||
andrepimenta marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } catch (error) { | ||
| // istanbul ignore next - defensive error handling for corrupted data | ||
|
||
| console.error(`Failed to parse stored data for ${fullKey}:`, error); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems a bit dangerous. The caller would have no way to differentiate empty data from data that we failed to retrieve. The caller might then proceed when it's unsafe to do so, or overwrite the data, or something like that. Given that we don't know precisely why the data is being stored/retrieved, or what the consequences of this failure might be, it would be safer to not catch the error and let the caller deal with it. We can highlight that it can throw with a TSDoc |
||
|
|
@@ -78,19 +68,15 @@ export class InMemoryStorageAdapter implements StorageAdapter { | |
|
|
||
| /** | ||
| * Store an item in in-memory storage. | ||
| * Wraps with metadata and serializes to string. | ||
| * Serializes JSON data to string. | ||
| * | ||
| * @param namespace - The controller namespace. | ||
| * @param key - The data key. | ||
| * @param value - The value to store (will be wrapped and serialized). | ||
| * @param value - The JSON value to store. | ||
| */ | ||
| async setItem(namespace: string, key: string, value: unknown): Promise<void> { | ||
| async setItem(namespace: string, key: string, value: Json): Promise<void> { | ||
| const fullKey = `${STORAGE_KEY_PREFIX}${namespace}:${key}`; | ||
| const wrapper: StoredDataWrapper = { | ||
| timestamp: Date.now(), | ||
| data: value, | ||
| }; | ||
| this.#storage.set(fullKey, JSON.stringify(wrapper)); | ||
| this.#storage.set(fullKey, JSON.stringify(value)); | ||
| } | ||
|
|
||
| /** | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.