Performance optimizations for API requests #789
Conversation
github-actions
bot
added
documentation
There was a problem hiding this comment.
Pull request overview
This PR implements several performance optimizations for API requests to reduce database queries and HTTP calls. The main focus is on addressing N+1 query problems through caching, parallel execution, and selective data fetching.
Key changes:
- Introduces a credentials cache with 60-second TTL to avoid repeated database queries for API credentials
- Creates a new
local_network_and_membercountfunction for lightweight network listings without fetching full member details - Optimizes member synchronization to use a single bulk database query instead of N individual queries
- Converts sequential member detail fetching to parallel execution using
Promise.all
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/ztApi.ts | Adds credentials caching, implements new lightweight network fetching functions, adds timeout to GET requests, and parallelizes member detail fetching |
| src/server/api/services/memberService.ts | Optimizes member synchronization by replacing N database queries with a single bulk query and in-memory Map lookups |
| src/server/api/routers/networkRouter.ts | Updates function calls to use renamed local_network_and_members function |
| src/server/api/routers/adminRoute.ts | Updates function calls to use renamed local_network_and_members function |
| src/pages/api/v1/org/[orgid]/network/index.ts | Switches to lightweight local_network_and_membercount for list operations |
| src/pages/api/v1/org/[orgid]/network/[nwid]/member/[memberId]/index.ts | Optimizes single member fetch to use member_details instead of fetching all members |
| src/pages/api/v1/org/[orgid]/network/[nwid]/index.ts | Switches to lightweight local_network_and_membercount for individual network operations |
| src/pages/api/v1/network/index.ts | Switches to lightweight local_network_and_membercount for user network listings |
| src/pages/api/v1/network/[id]/member/index.ts | Updates function call to use renamed local_network_and_members function |
| src/pages/api/v1/network/[id]/index.ts | Switches to lightweight local_network_and_membercount for individual network fetching |
| src/pages/api/tests/v1/network/networkById.test.ts | Updates test mocks to reflect renamed function and new memberCount field |
| src/cronTasks.ts | Updates function call to use renamed local_network_and_members function |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/utils/ztApi.ts
Outdated
| } | ||
|
|
||
| return { | ||
| network: { ...network }, |
There was a problem hiding this comment.
Unnecessary object spread. The { ...network } spread creates a shallow copy of the network object, but there's no apparent reason to do so here. The network object is returned directly from getData and not modified. Consider returning network directly to avoid the unnecessary allocation.
| network: { ...network }, | |
| network, |
src/utils/ztApi.ts
Outdated
| ); | ||
|
|
||
| return { | ||
| network: { ...network }, |
There was a problem hiding this comment.
Unnecessary object spread. The { ...network } spread creates a shallow copy of the network object, but there's no apparent reason to do so here. Consider returning network directly to avoid the unnecessary allocation.
| network: { ...network }, | |
| network: network, |
| // Store in cache | ||
| credentialsCache.set(userId, { credentials, timestamp: now }); |
There was a problem hiding this comment.
The credentials cache lacks cache invalidation when user credentials are updated. If a user updates their API keys or controller URLs, they'll continue to receive stale credentials for up to 60 seconds. Consider implementing a cache invalidation mechanism that's triggered when user options are updated, or documenting this limitation.
| * @param isCentral - Whether to use central API (default: false) | ||
| * @returns Network details with member count only | ||
| */ | ||
| export const local_network_and_membercount = async ( |
There was a problem hiding this comment.
Function name inconsistency: This function is named local_network_and_membercount but accepts an isCentral parameter and can work with both local and central APIs. The name suggests it only works with local controllers, but it's actually API-agnostic. Consider renaming to network_and_membercount to match the actual functionality and be consistent with the similarly generic local_network_and_members function (which also has a misleading name).
| export const local_network_and_membercount = async ( | |
| export const network_and_membercount = async ( |
sinamics
changed the title
No description provided.