feat(be): cache golemio requests #38
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe changes involve the deletion of the Changes
Sequence Diagram(s)sequenceDiagram
participant D as DepartureService
participant G as GolemioService
participant A as Golemio API
D->>G: getGolemioData(path)
G->>G: Check cache for data
alt Cache hit
G-->>D: Return cached data
else Cache miss
G->>A: GET request to Golemio API
A-->>G: Response with data
G->>G: Parse response and cache data
G-->>D: Return API response
end
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (4)
apps/backend/src/modules/departure/departure.module.ts (1)
5-5: LGTM! Well-structured module configuration.The addition of
GolemioServiceas a provider follows good architectural practices:
- Proper separation of concerns by isolating Golemio API interactions
- Centralized service for implementing caching strategy
- Clean dependency injection setup that supports testability
Also applies to: 9-9
apps/backend/src/modules/golemio/golemio.service.ts (2)
8-10: Add error handling for cache manager initialization.The cache manager injection looks good, but consider adding validation to ensure it's properly initialized.
Consider adding validation in the constructor:
@Injectable() export class GolemioService { - constructor(@Inject(CACHE_MANAGER) private cacheManager: Cache) {} + constructor(@Inject(CACHE_MANAGER) private cacheManager: Cache) { + if (!this.cacheManager) { + throw new Error('Cache manager not properly initialized'); + } + }
29-32: Improve cache handling logic.The current implementation caches the response even if it might not be valid JSON, and the error handling for cache operations could be improved.
Consider validating the response before caching:
if (res.ok) { try { const parsed = await res.clone().json(); if (parsed && typeof parsed === 'object') { await this.cacheManager.set(url, parsed, TTL); } } catch (error) { console.error('Failed to parse or cache response:', error); // Continue execution as this is non-critical } }apps/backend/src/modules/departure/departure.service.ts (1)
63-64: Consider extracting the time window constantThe
minutesAfter=600represents a 10-hour window for departures. Consider extracting this value to a named constant or configuration parameter for better maintainability and clarity.+private static readonly DEPARTURE_WINDOW_MINUTES = 600; // 10 hours + async getDepartures(args: { // ... }) { // ... - const res = await this.golemioService.getGolemioData( - `/v2/pid/departureboards?minutesAfter=600&${searchParams.toString()}`, - ); + const res = await this.golemioService.getGolemioData( + `/v2/pid/departureboards?minutesAfter=${DepartureService.DEPARTURE_WINDOW_MINUTES}&${searchParams.toString()}`, + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
apps/backend/src/constants/golemio.const.ts(0 hunks)apps/backend/src/modules/departure/departure.module.ts(1 hunks)apps/backend/src/modules/departure/departure.service.ts(2 hunks)apps/backend/src/modules/golemio/golemio.service.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/backend/src/constants/golemio.const.ts
🔇 Additional comments (3)
apps/backend/src/modules/departure/departure.service.ts (3)
6-6: LGTM: Import added correctly
The GolemioService import follows the project's conventions and is appropriately placed with other service imports.
12-15: LGTM: Constructor dependency injection is correct
The GolemioService is properly injected following NestJS best practices. The ordering of dependencies is logical, with the database service (PrismaService) followed by the API service (GolemioService).
63-64: LGTM: Successfully migrated to GolemioService
The implementation correctly uses the new GolemioService for API requests, which will enable caching as per the PR objectives.
| const res = await fetch(url, { | ||
| method: "GET", | ||
| headers: { | ||
| "Content-Type": "application/json", | ||
| "X-Access-Token": process.env.GOLEMIO_API_KEY ?? "", | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Implement retry mechanism for failed requests.
Network requests to external APIs should implement a retry mechanism to handle temporary failures.
Consider implementing a retry mechanism:
import { retry } from '@nestjs/common';
@retry(3, { delay: 1000 }) // Retry 3 times with 1s delay
async fetchWithRetry(url: string, options: RequestInit): Promise<Response> {
return fetch(url, options);
}Then use this method in getGolemioData instead of direct fetch.
There was a problem hiding this comment.
I don't think this is neccessary
Notes
Changes
Summary by CodeRabbit
New Features
GolemioServicefor improved interaction with the Golemio API, including caching capabilities.DepartureService.Bug Fixes
GOLEMIO_APIconstant, streamlining API endpoint management.Documentation
GolemioServicein theDepartureModule.