-
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 service impl #30
Changes from 2 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,7 @@ | ||
import { HttpException, HttpStatus } from "@nestjs/common"; | ||
|
||
export class ApiNotAvailable extends HttpException { | ||
constructor(apiName: string) { | ||
super(`The ${apiName} API is not available.`, HttpStatus.SERVICE_UNAVAILABLE); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
export * from "./apiNotAvailable.exception"; | ||
export * from "./rateLimitExceeded.exception"; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
import { HttpException, HttpStatus } from "@nestjs/common"; | ||
|
||
export class RateLimitExceeded extends HttpException { | ||
constructor() { | ||
super("Rate limit exceeded.", HttpStatus.TOO_MANY_REQUESTS); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,28 +1,145 @@ | ||
import { createMock } from "@golevelup/ts-jest"; | ||
import { HttpService } from "@nestjs/axios"; | ||
import { HttpException, HttpStatus } from "@nestjs/common"; | ||
import { Test, TestingModule } from "@nestjs/testing"; | ||
import { AxiosError, AxiosInstance, AxiosResponseHeaders } from "axios"; | ||
|
||
import { ApiNotAvailable, RateLimitExceeded } from "@zkchainhub/pricing/exceptions"; | ||
import { TokenPrices } from "@zkchainhub/pricing/types/tokenPrice.type"; | ||
|
||
import { CoingeckoService } from "./coingecko.service"; | ||
|
||
describe("CoingeckoService", () => { | ||
let service: CoingeckoService; | ||
let httpService: HttpService; | ||
const apiKey = "COINGECKO_API_KEY"; | ||
|
||
beforeEach(async () => { | ||
const module: TestingModule = await Test.createTestingModule({ | ||
providers: [ | ||
CoingeckoService, | ||
{ | ||
provide: CoingeckoService, | ||
useFactory: () => { | ||
useFactory: (httpService: HttpService) => { | ||
const apiKey = "COINGECKO_API_KEY"; | ||
const apiBaseUrl = "https://api.coingecko.com/api/v3/"; | ||
return new CoingeckoService(apiKey, apiBaseUrl); | ||
return new CoingeckoService(apiKey, apiBaseUrl, httpService); | ||
}, | ||
inject: [HttpService], | ||
}, | ||
{ | ||
provide: HttpService, | ||
useValue: createMock<HttpService>({ | ||
axiosRef: createMock<AxiosInstance>(), | ||
}), | ||
}, | ||
], | ||
}).compile(); | ||
|
||
service = module.get<CoingeckoService>(CoingeckoService); | ||
httpService = module.get<HttpService>(HttpService); | ||
}); | ||
|
||
it("should be defined", () => { | ||
expect(service).toBeDefined(); | ||
}); | ||
|
||
describe("getTokenPrices", () => { | ||
it("return token prices", async () => { | ||
const tokenIds = ["token1", "token2"]; | ||
const currency = "usd"; | ||
const expectedResponse: TokenPrices = { | ||
token1: { usd: 1.23 }, | ||
token2: { usd: 4.56 }, | ||
}; | ||
|
||
jest.spyOn(httpService.axiosRef, "get").mockResolvedValueOnce({ | ||
data: expectedResponse, | ||
}); | ||
|
||
const result = await service.getTokenPrices(tokenIds, { currency }); | ||
|
||
expect(result).toEqual({ | ||
token1: 1.23, | ||
token2: 4.56, | ||
}); | ||
expect(httpService.axiosRef.get).toHaveBeenCalledWith( | ||
`${service["apiBaseUrl"]}/simple/price`, | ||
{ | ||
params: { | ||
vs_currencies: currency, | ||
ids: tokenIds.join(","), | ||
precision: service["DECIMALS_PRECISION"].toString(), | ||
}, | ||
headers: { | ||
"x-cg-pro-api-key": apiKey, | ||
Accept: "application/json", | ||
}, | ||
}, | ||
); | ||
}); | ||
|
||
it("throw ApiNotAvailable when Coingecko returns a 500 family exception", async () => { | ||
const tokenIds = ["token1", "token2"]; | ||
const currency = "usd"; | ||
|
||
jest.spyOn(httpService.axiosRef, "get").mockRejectedValueOnce( | ||
new AxiosError("Service not available", "503", undefined, null, { | ||
status: 503, | ||
data: {}, | ||
statusText: "Too Many Requests", | ||
headers: createMock<AxiosResponseHeaders>(), | ||
config: { headers: createMock<AxiosResponseHeaders>() }, | ||
}), | ||
); | ||
|
||
await expect(service.getTokenPrices(tokenIds, { currency })).rejects.toThrow( | ||
new ApiNotAvailable("Coingecko"), | ||
); | ||
}); | ||
|
||
it("throw RateLimitExceeded when Coingecko returns 429 exception", async () => { | ||
const tokenIds = ["token1", "token2"]; | ||
const currency = "usd"; | ||
|
||
jest.spyOn(httpService.axiosRef, "get").mockRejectedValueOnce( | ||
new AxiosError("Rate limit exceeded", "429", undefined, null, { | ||
status: 429, | ||
data: {}, | ||
statusText: "Too Many Requests", | ||
headers: createMock<AxiosResponseHeaders>(), | ||
config: { headers: createMock<AxiosResponseHeaders>() }, | ||
}), | ||
); | ||
|
||
await expect(service.getTokenPrices(tokenIds, { currency })).rejects.toThrow( | ||
new RateLimitExceeded(), | ||
); | ||
}); | ||
|
||
it("throw an HttpException with the error message when an error occurs", async () => { | ||
const tokenIds = ["invalidTokenId", "token2"]; | ||
const currency = "usd"; | ||
|
||
jest.spyOn(httpService.axiosRef, "get").mockRejectedValueOnce( | ||
new AxiosError("Invalid token ID", "400"), | ||
); | ||
|
||
await expect(service.getTokenPrices(tokenIds, { currency })).rejects.toThrow(); | ||
}); | ||
|
||
it("throw an HttpException with the default error message when a non-network related error occurs", async () => { | ||
const tokenIds = ["token1", "token2"]; | ||
const currency = "usd"; | ||
|
||
jest.spyOn(httpService.axiosRef, "get").mockRejectedValueOnce(new Error()); | ||
|
||
await expect(service.getTokenPrices(tokenIds, { currency })).rejects.toThrow( | ||
new HttpException( | ||
"A non network related error occurred", | ||
HttpStatus.INTERNAL_SERVER_ERROR, | ||
), | ||
); | ||
}); | ||
}); | ||
}); |
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. natspec missing 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. for the service you mean right? because the method has it on the interface 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. I might consider elevating the constants and static values, but since it's a small service and those values are not expected to change in the future, there is no problem here. 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. to the shared module or outside the class? |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,18 +1,92 @@ | ||
import { Injectable } from "@nestjs/common"; | ||
import { HttpService } from "@nestjs/axios"; | ||
import { Injectable, Logger } from "@nestjs/common"; | ||
import { isAxiosError } from "axios"; | ||
|
||
import { ApiNotAvailable, RateLimitExceeded } from "@zkchainhub/pricing/exceptions"; | ||
import { IPricingService } from "@zkchainhub/pricing/interfaces"; | ||
import { TokenPrices } from "@zkchainhub/pricing/types/tokenPrice.type"; | ||
|
||
/** | ||
* Service for fetching token prices from Coingecko API. | ||
*/ | ||
@Injectable() | ||
export class CoingeckoService implements IPricingService { | ||
private readonly logger = new Logger(CoingeckoService.name); | ||
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. should this be an injectable from 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. we're changing this in a different PR as discussed in DM using nest-winston module |
||
|
||
private readonly AUTH_HEADER = "x-cg-pro-api-key"; | ||
private readonly DECIMALS_PRECISION = 3; | ||
|
||
/** | ||
* | ||
* @param apiKey * @param apiKey - Coingecko API key. | ||
* @param apiBaseUrl - Base URL for Coingecko API. If you have a Pro account, you can use the Pro API URL. | ||
*/ | ||
constructor( | ||
private readonly apiKey: string, | ||
private readonly apiBaseUrl: string = "https://api.coingecko.com/api/v3/", | ||
private readonly httpService: HttpService, | ||
) {} | ||
|
||
/** | ||
* @param tokenIds - An array of Coingecko Tokens IDs. | ||
* @param config.currency - The currency in which the prices should be returned (default: "usd"). | ||
*/ | ||
async getTokenPrices( | ||
_tokenIds: string[], | ||
_config: { currency: string } = { currency: "usd" }, | ||
tokenIds: string[], | ||
config: { currency: string } = { currency: "usd" }, | ||
): Promise<Record<string, number>> { | ||
throw new Error("Method not implemented."); | ||
const { currency } = config; | ||
return this.httpGet<TokenPrices>("/simple/price", { | ||
vs_currencies: currency, | ||
ids: tokenIds.join(","), | ||
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. Does the API that's being used here have any limit on the size of the query parameters? 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. isn't specified, i'll try on postman and check if it breaks at some point 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. i could fetch 86 at once so i think probably it doesnt have a limit 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. Sounds good! If it's not documented then yeah, probably a good idea to not limit that on our end. 👌 |
||
precision: this.DECIMALS_PRECISION.toString(), | ||
}).then((data) => { | ||
return Object.fromEntries(Object.entries(data).map(([key, value]) => [key, value.usd])); | ||
}); | ||
} | ||
|
||
/** | ||
* HTTP GET wrapper to perform a GET request to the specified endpoint with optional parameters. | ||
* Also injects the API key and sets the Accept header to "application/json". | ||
* @param endpoint - The endpoint to send the GET request to. | ||
* @param params - Optional parameters to include in the request. | ||
* @returns A promise that resolves to the response data. | ||
* @throws {ApiNotAvailable} If the Coingecko API is not available (status code >= 500). | ||
* @throws {RateLimitExceeded} If the rate limit for the API is exceeded (status code 429). | ||
* @throws {Error} If an error occurs while fetching data or a non-network related error occurs. | ||
*/ | ||
private async httpGet<ResponseType>(endpoint: string, params: Record<string, string> = {}) { | ||
try { | ||
const response = await this.httpService.axiosRef.get<ResponseType>( | ||
`${this.apiBaseUrl}${endpoint}`, | ||
{ | ||
params, | ||
headers: { | ||
[this.AUTH_HEADER]: this.apiKey, | ||
Accept: "application/json", | ||
}, | ||
}, | ||
); | ||
return response.data; | ||
} catch (error: unknown) { | ||
let exception; | ||
if (isAxiosError(error)) { | ||
const statusCode = error.response?.status ?? 0; | ||
if (statusCode >= 500) { | ||
exception = new ApiNotAvailable("Coingecko"); | ||
} else if (statusCode === 429) { | ||
exception = new RateLimitExceeded(); | ||
} else { | ||
exception = new Error( | ||
error.response?.data || "An error occurred while fetching data", | ||
); | ||
} | ||
|
||
throw exception; | ||
} else { | ||
this.logger.error(error); | ||
throw new Error("A non network related error occurred"); | ||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
export type TokenPrice = { | ||
usd: number; | ||
usd_market_cap?: number; | ||
usd_24h_vol?: number; | ||
usd_24h_change?: number; | ||
last_updated_at?: number; | ||
}; | ||
|
||
export type TokenPrices = { | ||
[address: string]: TokenPrice; | ||
}; |
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.
💯