-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: pricing & metadata caching #51
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
import { z } from "zod"; | ||
|
||
import { ICache } from "@grants-stack-indexer/repository"; | ||
import { ILogger } from "@grants-stack-indexer/shared"; | ||
|
||
import { IMetadataProvider } from "../internal.js"; | ||
|
||
/** | ||
* A metadata provider that caches metadata lookups from the underlying provider. | ||
* When a metadata is requested, it first checks the cache. If found, returns the cached metadata. | ||
* If not found in cache, fetches from the underlying provider and caches the result before returning. | ||
* Cache failures (both reads and writes) are logged but do not prevent the provider from functioning. | ||
*/ | ||
export class CachingMetadataProvider implements IMetadataProvider { | ||
constructor( | ||
private readonly provider: IMetadataProvider, | ||
private readonly cache: ICache<string, unknown>, | ||
private readonly logger: ILogger, | ||
) {} | ||
|
||
/** @inheritdoc */ | ||
async getMetadata<T>( | ||
ipfsCid: string, | ||
validateContent?: z.ZodSchema<T>, | ||
): Promise<T | undefined> { | ||
let cachedMetadata: T | undefined = undefined; | ||
try { | ||
cachedMetadata = (await this.cache.get(ipfsCid)) as T | undefined; | ||
} catch (error) { | ||
this.logger.debug(`Failed to get cached metadata for IPFS CID ${ipfsCid}`, { | ||
error, | ||
}); | ||
} | ||
|
||
if (cachedMetadata) { | ||
return cachedMetadata; | ||
} | ||
|
||
const metadata = await this.provider.getMetadata<T>(ipfsCid, validateContent); | ||
|
||
if (metadata) { | ||
try { | ||
await this.cache.set(ipfsCid, metadata); | ||
} catch (error) { | ||
this.logger.debug(`Failed to cache metadata for IPFS CID ${ipfsCid}`, { | ||
error, | ||
}); | ||
} | ||
} | ||
|
||
return metadata; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,2 @@ | ||
export * from "./ipfs.provider.js"; | ||
export * from "./cachingProxy.provider.js"; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,96 @@ | ||
import { beforeEach, describe, expect, it, vi } from "vitest"; | ||
import { z } from "zod"; | ||
|
||
import { ICache } from "@grants-stack-indexer/repository"; | ||
import { ILogger } from "@grants-stack-indexer/shared"; | ||
|
||
import { IMetadataProvider } from "../../src/internal.js"; | ||
import { CachingMetadataProvider } from "../../src/providers/cachingProxy.provider.js"; | ||
|
||
describe("CachingMetadataProvider", () => { | ||
const mockProvider = { | ||
getMetadata: vi.fn(), | ||
} as unknown as IMetadataProvider; | ||
|
||
const mockCache = { | ||
get: vi.fn(), | ||
set: vi.fn(), | ||
} as unknown as ICache<string, unknown>; | ||
|
||
const mockLogger = { | ||
debug: vi.fn(), | ||
} as unknown as ILogger; | ||
|
||
let provider: CachingMetadataProvider; | ||
|
||
beforeEach(() => { | ||
vi.clearAllMocks(); | ||
provider = new CachingMetadataProvider(mockProvider, mockCache, mockLogger); | ||
}); | ||
|
||
describe("getMetadata", () => { | ||
const testCid = "QmTest123"; | ||
const testData = { foo: "bar" }; | ||
const testSchema = z.object({ foo: z.string() }); | ||
|
||
it("returns cached metadata when available", async () => { | ||
vi.spyOn(mockCache, "get").mockResolvedValue(testData); | ||
|
||
const result = await provider.getMetadata(testCid, testSchema); | ||
|
||
expect(result).toEqual(testData); | ||
expect(mockCache.get).toHaveBeenCalledWith(testCid); | ||
expect(mockProvider.getMetadata).not.toHaveBeenCalled(); | ||
}); | ||
|
||
it("fetches and caches metadata when cache misses", async () => { | ||
vi.spyOn(mockCache, "get").mockResolvedValue(undefined); | ||
vi.spyOn(mockProvider, "getMetadata").mockResolvedValue(testData); | ||
|
||
const result = await provider.getMetadata(testCid, testSchema); | ||
|
||
expect(result).toEqual(testData); | ||
expect(mockCache.get).toHaveBeenCalledWith(testCid); | ||
expect(mockProvider.getMetadata).toHaveBeenCalledWith(testCid, testSchema); | ||
expect(mockCache.set).toHaveBeenCalledWith(testCid, testData); | ||
}); | ||
|
||
it("handles cache read failures gracefully", async () => { | ||
vi.spyOn(mockCache, "get").mockRejectedValue(new Error("Cache read error")); | ||
vi.spyOn(mockProvider, "getMetadata").mockResolvedValue(testData); | ||
|
||
const result = await provider.getMetadata(testCid, testSchema); | ||
|
||
expect(result).toEqual(testData); | ||
expect(mockLogger.debug).toHaveBeenCalledWith( | ||
`Failed to get cached metadata for IPFS CID ${testCid}`, | ||
expect.any(Object), | ||
); | ||
expect(mockProvider.getMetadata).toHaveBeenCalledWith(testCid, testSchema); | ||
}); | ||
|
||
it("handles cache write failures gracefully", async () => { | ||
vi.spyOn(mockCache, "get").mockResolvedValue(undefined); | ||
vi.spyOn(mockCache, "set").mockRejectedValue(new Error("Cache write error")); | ||
vi.spyOn(mockProvider, "getMetadata").mockResolvedValue(testData); | ||
|
||
const result = await provider.getMetadata(testCid, testSchema); | ||
|
||
expect(result).toEqual(testData); | ||
expect(mockLogger.debug).toHaveBeenCalledWith( | ||
`Failed to cache metadata for IPFS CID ${testCid}`, | ||
expect.any(Object), | ||
); | ||
}); | ||
|
||
it("returns undefined when metadata is not found", async () => { | ||
vi.spyOn(mockCache, "get").mockResolvedValue(undefined); | ||
vi.spyOn(mockProvider, "getMetadata").mockResolvedValue(undefined); | ||
|
||
const result = await provider.getMetadata(testCid, testSchema); | ||
|
||
expect(result).toBeUndefined(); | ||
expect(mockCache.set).not.toHaveBeenCalled(); | ||
}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
import { ICache, PriceCacheKey } from "@grants-stack-indexer/repository"; | ||
import { ILogger, TokenCode } from "@grants-stack-indexer/shared"; | ||
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. it doesn't have to be in this PR but probably we should rename tokencode to tokensymbol, they are different things (and I've never heard of token code used to describe a symbol like "ETH") 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. could be, TokenCode is just a way to identify token generically (ie. not tied to the ID on a pricing provider like Coingecko) or to the symbol (since many tokens can have the same symbol), but in this case we decided to pick the token symbol as TokenCode but could be anything else. what name do you suggest? 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. Ahh gotcha, I think that name is good then |
||
|
||
import { IPricingProvider, TokenPrice } from "../internal.js"; | ||
|
||
/** | ||
* A pricing provider that caches token price lookups from the underlying provider. | ||
* When a price is requested, it first checks the cache. If found, returns the cached price. | ||
* If not found in cache, fetches from the underlying provider and caches the result before returning. | ||
* Cache failures (both reads and writes) are logged but do not prevent the provider from functioning. | ||
*/ | ||
export class CachingPricingProvider implements IPricingProvider { | ||
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. can this data ever become stale? do we need TTL/eviction logic? 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. no, is always data from the past that isn't supposed to change |
||
constructor( | ||
private readonly provider: IPricingProvider, | ||
private readonly cache: ICache<PriceCacheKey, TokenPrice>, | ||
private readonly logger: ILogger, | ||
) {} | ||
|
||
/** @inheritdoc */ | ||
async getTokenPrice( | ||
tokenCode: TokenCode, | ||
startTimestampMs: number, | ||
endTimestampMs?: number, | ||
): Promise<TokenPrice | undefined> { | ||
let cachedPrice: TokenPrice | undefined = undefined; | ||
try { | ||
cachedPrice = await this.cache.get({ | ||
tokenCode, | ||
timestampMs: startTimestampMs, | ||
}); | ||
} catch (error) { | ||
this.logger.debug( | ||
`Failed to get cached price for token ${tokenCode} at ${startTimestampMs}`, | ||
{ error }, | ||
); | ||
} | ||
|
||
if (cachedPrice) { | ||
return cachedPrice; | ||
} | ||
|
||
const price = await this.provider.getTokenPrice( | ||
tokenCode, | ||
startTimestampMs, | ||
endTimestampMs, | ||
); | ||
|
||
if (price) { | ||
try { | ||
await this.cache.set( | ||
{ | ||
tokenCode, | ||
timestampMs: startTimestampMs, | ||
}, | ||
price, | ||
); | ||
} catch (error) { | ||
this.logger.debug( | ||
`Failed to cache price for token ${tokenCode} at ${startTimestampMs}`, | ||
{ error }, | ||
); | ||
} | ||
} | ||
|
||
return price; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,3 @@ | ||
export * from "./coingecko.provider.js"; | ||
export * from "./dummy.provider.js"; | ||
export * from "./cachingProxy.provider.js"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it might be good to name _metadataProvider more specific to differentiate it from metadataProvider--maybe ipfsProvider, ipfsInstance?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
4885d84