- 
                Notifications
    You must be signed in to change notification settings 
- Fork 11
feat(api): add cpu utilization query and subscription #1590
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
Conversation
| Caution Review failedThe pull request is closed. WalkthroughRefactors Info GraphQL types into modular Info* submodels, replaces the monolithic InfoService with per-domain services (Cpu, Memory, Os, Versions, Devices, Display), adds a Metrics subsystem with polling and pubsub-backed subscriptions for CPU/memory, introduces subscription helper/tracker/polling services with tests, updates schema and web typings, and adjusts module wiring and configs. Changes
 Sequence Diagram(s)sequenceDiagram
  autonumber
  actor Client
  participant GQL as GraphQL API
  participant MR as MetricsResolver
  participant CPU as CpuService
  participant MEM as MemoryService
  participant TRK as SubscriptionTracker
  participant POLL as SubscriptionPolling
  participant PUB as PubSub
  rect rgb(245,248,255)
  note right of MR: onModuleInit registers polling topics
  MR->>TRK: registerTopic(CPU_UTILIZATION, 1000ms, callback)
  MR->>TRK: registerTopic(MEMORY_UTILIZATION, 2000ms, callback)
  TRK->>POLL: startPolling(name, interval, callback)
  end
  Client->>GQL: subscribe systemMetricsCpu
  GQL->>MR: systemMetricsCpuSubscription()
  MR->>TRK: subscribe(CPU_UTILIZATION)
  MR->>PUB: createTrackedSubscription(CPU_UTILIZATION)
  activate POLL
  loop every 1s
    POLL->>CPU: generateCpuLoad()
    CPU-->>POLL: CpuUtilization
    POLL->>PUB: publish { systemMetricsCpu }
  end
  PUB-->>GQL: payload { systemMetricsCpu }
  GQL-->>Client: CpuUtilization events
sequenceDiagram
  autonumber
  actor Client
  participant GQL as GraphQL API
  participant IR as InfoResolver
  participant CPU as CpuService
  participant MEM as MemoryService
  participant OS as OsService
  participant VER as VersionsService
  participant DISP as DisplayService
  participant DEV as DevicesResolver
  Client->>GQL: query { info { id time cpu memory os versions display devices { gpu pci usb network } } }
  GQL->>IR: info()
  IR-->>GQL: { id: "info" }
  GQL->>IR: time()
  IR-->>GQL: DateTime
  GQL->>IR: cpu()
  IR->>CPU: generateCpu()
  CPU-->>IR: InfoCpu
  IR-->>GQL: InfoCpu
  GQL->>IR: memory()
  IR->>MEM: generateMemory()
  MEM-->>IR: InfoMemory
  IR-->>GQL: InfoMemory
  GQL->>IR: os()
  IR->>OS: generateOs()
  OS-->>IR: InfoOs
  IR-->>GQL: InfoOs
  GQL->>IR: versions()
  IR->>VER: generateVersions()
  VER-->>IR: InfoVersions
  IR-->>GQL: InfoVersions
  GQL->>IR: display()
  IR->>DISP: getDisplay()
  DISP-->>IR: InfoDisplay
  IR-->>GQL: InfoDisplay
  GQL->>IR: devices()
  IR-->>GQL: { id: "info/devices" }
  GQL->>DEV: gpu/pci/usb/network
  DEV-->>GQL: arrays of Info* devices
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
 Poem
 Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration: 
 You can enable these sources in your CodeRabbit configuration. ⛔ Files ignored due to path filters (1)
 📒 Files selected for processing (16)
 ✨ Finishing Touches
 🧪 Generate unit tests
 🪧 TipsChatThere are 3 ways to chat with CodeRabbit: 
 SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type  Other keywords and placeholders
 CodeRabbit Configuration File ( | 
| Claude finished @elibosley's task —— View job Code Review - Critical Issues AnalysisReviewing only the diff/changeset for high-priority issues: 
 🚨 Critical Issues Found1. Line 183: Runtime crash on undefined access return id.replace('p', ':');If regex match fails,  2. Line 46: Inverted boolean logic blacklisted: entry.allowed,This maps  3. Lines 34, 44: Unhandled promise rejections 4. Module structure: Multiple ScheduleModule.forRoot() | 
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.
Actionable comments posted: 8
🔭 Outside diff range comments (1)
api/src/unraid-api/graph/resolvers/info/info.resolver.spec.ts (1)
373-382: Add tests for the new CPU query and subscriptionThis spec doesn’t cover:
- Query: cpuUtilization
- Subscription: cpuUtilizationSubscription
- Field resolver: InfoCpu.utilization
Add targeted tests to validate behavior and interactions with SubscriptionTrackerService and CpuDataService.
Here’s a concise set you can drop in:
describe('cpuUtilization (query)', () => { it('returns CpuUtilization snapshot', async () => { const res = await resolver.cpuUtilization(); expect(res).toEqual({ id: 'info/cpu-load', load: 10, cpus: [] }); }); }); describe('cpuUtilizationSubscription (subscription)', () => { it('subscribes/unsubscribes via SubscriptionTrackerService', async () => { const { createSubscription } = await import('@app/core/pubsub.js'); const iterator = { [Symbol.asyncIterator]: vi.fn().mockReturnValue({}), return: vi.fn() }; vi.mocked(createSubscription).mockReturnValue(iterator as any); const asyncIt = await resolver.cpuUtilizationSubscription(); // Subscribe asyncIt[Symbol.asyncIterator](); expect(mockSubscriptionTrackerService.subscribe).toHaveBeenCalledWith('CPU_UTILIZATION'); // Unsubscribe await asyncIt.return?.(); expect(mockSubscriptionTrackerService.unsubscribe).toHaveBeenCalledWith('CPU_UTILIZATION'); }); }); describe('InfoCpu.utilization (field)', () => { it('resolves from CpuDataService cache', async () => { // Resolve field via separate resolver if exported, or call directly if available // Assuming InfoCpuResolver is provided similarly in the test module: const { InfoCpuResolver } = await import('@app/unraid-api/graph/resolvers/info/info.resolver.js'); const infoCpuResolver = module.get<InstanceType<typeof InfoCpuResolver>>(InfoCpuResolver as any); const val = await infoCpuResolver.utilization({} as any); expect(val).toBe(10); }); });
🧹 Nitpick comments (9)
api/src/unraid-api/graph/services/subscription-tracker.service.ts (1)
19-29: Use nullish coalescing for 0-safe increment
|| 0is fine here but for symmetry and 0-safety consistency withunsubscribe, prefer?? 0.Apply this diff:
- const currentCount = this.subscriberCounts.get(topic) || 0; + const currentCount = this.subscriberCounts.get(topic) ?? 0;api/src/unraid-api/graph/resolvers/info/info.model.ts (1)
151-155: Add description for CpuUtilization.load for consistencyPer-core
CpuLoad.loadis described as percent, but the top-levelCpuUtilization.loadlacks a description. Align docs to avoid ambiguity.Apply this diff:
- @Field(() => Float) + @Field(() => Float, { description: 'Overall CPU utilization in percent.' }) load!: number;api/src/unraid-api/graph/resolvers/info/info.resolver.spec.ts (4)
322-327: Use .rejects.toThrow() without asserting message textPer testing guidelines, avoid brittle assertions on error message strings.
Apply:
- await expect(resolver.machineId()).rejects.toThrow('Machine ID error'); + await expect(resolver.machineId()).rejects.toThrow();
385-390: Use .rejects.toThrow() without asserting message textAvoid asserting on exact error messages unless message format is under test.
- await expect(resolver.baseboard()).rejects.toThrow('Baseboard error'); + await expect(resolver.baseboard()).rejects.toThrow();
392-397: Use .rejects.toThrow() without asserting message textSame guideline applies here.
- await expect(resolver.system()).rejects.toThrow('System error'); + await expect(resolver.system()).rejects.toThrow();
399-403: Use .rejects.toThrow() without asserting message textSame guideline applies here.
- await expect(resolver.apps()).rejects.toThrow('Service error'); + await expect(resolver.apps()).rejects.toThrow();api/src/unraid-api/graph/resolvers/info/info.resolver.ts (3)
60-73: Guard the polling loop and handle errors to avoid overlapping calls and unhandled rejectionssetInterval with an async callback can overlap if the operation takes longer than the interval or throw unhandled rejections. Add an in-flight guard and try/catch.
onModuleInit() { this.subscriptionTracker.registerTopic( CPU_UTILIZATION, () => { - this.cpuPollingTimer = setInterval(async () => { - const payload = await this.infoService.generateCpuLoad(); - pubsub.publish(CPU_UTILIZATION, { cpuUtilization: payload }); - }, 1000); + let inFlight = false; + this.cpuPollingTimer = setInterval(async () => { + if (inFlight) return; + inFlight = true; + try { + const payload = await this.infoService.generateCpuLoad(); + await pubsub.publish(CPU_UTILIZATION, { cpuUtilization: payload }); + } catch { + // swallow or log; avoid unhandled rejection + } finally { + inFlight = false; + } + }, 1000); }, () => { clearInterval(this.cpuPollingTimer); } ); }
204-211: Use Float GraphQL type for utilizationCPU load is fractional. Use Float instead of Number to avoid being inferred as Int.
- @ResolveField(() => Number, { + @ResolveField(() => Float, { description: 'CPU utilization in percent', nullable: true, }) public async utilization(@Parent() cpu: InfoCpu): Promise<number> { const { currentLoad } = await this.cpuDataService.getCpuLoad(); return currentLoad; }And add to imports:
-import { - Parent, - Query, - Resolver, - Subscription, - ResolveField, -} from '@nestjs/graphql'; +import { Parent, Query, Resolver, Subscription, ResolveField, Float } from '@nestjs/graphql';
9-9: Unused import: PubSubPubSub isn’t used here. Consider removing to satisfy lint rules.
-import { PubSub } from 'graphql-subscriptions';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
- api/src/unraid-api/graph/resolvers/info/cpu-data.service.ts(1 hunks)
- api/src/unraid-api/graph/resolvers/info/info.model.ts(1 hunks)
- api/src/unraid-api/graph/resolvers/info/info.resolver.spec.ts(3 hunks)
- api/src/unraid-api/graph/resolvers/info/info.resolver.ts(3 hunks)
- api/src/unraid-api/graph/resolvers/info/info.service.ts(3 hunks)
- api/src/unraid-api/graph/resolvers/resolvers.module.ts(2 hunks)
- api/src/unraid-api/graph/services/services.module.ts(1 hunks)
- api/src/unraid-api/graph/services/subscription-tracker.service.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
TypeScript source files must use import specifiers with .js extensions for ESM compatibility
Files:
- api/src/unraid-api/graph/resolvers/info/info.resolver.spec.ts
- api/src/unraid-api/graph/resolvers/info/cpu-data.service.ts
- api/src/unraid-api/graph/services/services.module.ts
- api/src/unraid-api/graph/resolvers/info/info.service.ts
- api/src/unraid-api/graph/resolvers/resolvers.module.ts
- api/src/unraid-api/graph/services/subscription-tracker.service.ts
- api/src/unraid-api/graph/resolvers/info/info.model.ts
- api/src/unraid-api/graph/resolvers/info/info.resolver.ts
api/src/unraid-api/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Place new API source files in api/src/unraid-api/ rather than legacy locations
Prefer adding new files to the Nest repo at api/src/unraid-api/ instead of legacy code
Files:
- api/src/unraid-api/graph/resolvers/info/info.resolver.spec.ts
- api/src/unraid-api/graph/resolvers/info/cpu-data.service.ts
- api/src/unraid-api/graph/services/services.module.ts
- api/src/unraid-api/graph/resolvers/info/info.service.ts
- api/src/unraid-api/graph/resolvers/resolvers.module.ts
- api/src/unraid-api/graph/services/subscription-tracker.service.ts
- api/src/unraid-api/graph/resolvers/info/info.model.ts
- api/src/unraid-api/graph/resolvers/info/info.resolver.ts
api/**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Prefer not to mock simple dependencies in API tests
Files:
- api/src/unraid-api/graph/resolvers/info/info.resolver.spec.ts
**/*.{test,spec}.{js,ts,jsx,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.{test,spec}.{js,ts,jsx,tsx}: General testing: use .rejects.toThrow() without arguments when asserting thrown errors
General testing: focus on behavior, not exact error message wording
General testing: avoid brittle tests tied to non-essential details (exact messages, log formats)
General testing: use mocks as nouns (objects), not verbs (behavioral overuse)
Files:
- api/src/unraid-api/graph/resolvers/info/info.resolver.spec.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Avoid unnecessary or obvious comments; only add comments when needed for clarity
Files:
- api/src/unraid-api/graph/resolvers/info/info.resolver.spec.ts
- api/src/unraid-api/graph/resolvers/info/cpu-data.service.ts
- api/src/unraid-api/graph/services/services.module.ts
- api/src/unraid-api/graph/resolvers/info/info.service.ts
- api/src/unraid-api/graph/resolvers/resolvers.module.ts
- api/src/unraid-api/graph/services/subscription-tracker.service.ts
- api/src/unraid-api/graph/resolvers/info/info.model.ts
- api/src/unraid-api/graph/resolvers/info/info.resolver.ts
api/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/api-rules.mdc)
api/**/*.{test,spec}.{js,jsx,ts,tsx}: Use Vitest for tests in the api; do not use Jest
Prefer not to mock simple dependencies in tests
For error testing, use.rejects.toThrow()without arguments; avoid asserting exact error messages unless the message format is the subject under test
Files:
- api/src/unraid-api/graph/resolvers/info/info.resolver.spec.ts
🧠 Learnings (10)
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to **/__test__/components/**/*.ts : Mock external dependencies and services in component tests (e.g., vi.mock for helper modules)
Applied to files:
- api/src/unraid-api/graph/resolvers/info/info.resolver.spec.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to {**/*.test.ts,**/__test__/{components,store}/**/*.ts} : Use factory functions in vi.mock calls to define mocks and avoid hoisting pitfalls
Applied to files:
- api/src/unraid-api/graph/resolvers/info/info.resolver.spec.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to {**/*.test.ts,**/__test__/{components,store}/**/*.ts} : Remember that vi.mock calls are hoisted; avoid mixing mock declarations and module mocks incorrectly
Applied to files:
- api/src/unraid-api/graph/resolvers/info/info.resolver.spec.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to {**/*.test.ts,**/__test__/{components,store}/**/*.ts} : Place module mock declarations (vi.mock) at the top level of the test file to avoid hoisting issues
Applied to files:
- api/src/unraid-api/graph/resolvers/info/info.resolver.spec.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to {**/*.test.ts,**/__test__/{components,store}/**/*.ts} : Reset/clear mocks between tests using vi.clearAllMocks() (and vi.resetAllMocks() when appropriate) to ensure isolation
Applied to files:
- api/src/unraid-api/graph/resolvers/info/info.resolver.spec.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to **/__test__/store/**/*.ts : In store tests, mock external dependencies used by the store and verify interactions with those mocks
Applied to files:
- api/src/unraid-api/graph/resolvers/info/info.resolver.spec.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to {**/*.test.ts,**/__test__/{components,store}/**/*.ts} : Use vi.spyOn() to specify return values or behavior of methods under test
Applied to files:
- api/src/unraid-api/graph/resolvers/info/info.resolver.spec.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to **/__test__/mocks/**/*.ts : Frequently used mocks can also be placed under __test__/mocks
Applied to files:
- api/src/unraid-api/graph/resolvers/info/info.resolver.spec.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to **/__test__/components/**/*.ts : Use createTestingPinia() to mock Pinia stores when testing Vue components
Applied to files:
- api/src/unraid-api/graph/resolvers/info/info.resolver.spec.ts
📚 Learning: 2025-08-11T15:06:30.877Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-11T15:06:30.877Z
Learning: Applies to web/__test__/**/*.{test,spec}.{ts,js,tsx,jsx} : Web (Vue) component tests should mock external dependencies and services
Applied to files:
- api/src/unraid-api/graph/resolvers/info/info.resolver.spec.ts
🧬 Code Graph Analysis (5)
api/src/unraid-api/graph/resolvers/info/cpu-data.service.ts (2)
api/src/unraid-api/graph/resolvers/info/info.service.ts (1)
Injectable(29-113)api/src/unraid-api/graph/services/subscription-tracker.service.ts (1)
Injectable(3-44)
api/src/unraid-api/graph/services/services.module.ts (1)
api/src/unraid-api/graph/resolvers/resolvers.module.ts (1)
Module(43-88)
api/src/unraid-api/graph/resolvers/resolvers.module.ts (1)
api/src/unraid-api/graph/services/services.module.ts (1)
Module(4-8)
api/src/unraid-api/graph/resolvers/info/info.model.ts (3)
api/src/unraid-api/graph/resolvers/api-key/api-key.model.ts (3)
ObjectType(19-30)
ObjectType(32-59)
ObjectType(61-66)api/src/unraid-api/graph/resolvers/config/config.model.ts (1)
ObjectType(5-14)web/composables/gql/graphql.ts (1)
Node(1102-1104)
api/src/unraid-api/graph/resolvers/info/info.resolver.ts (2)
api/src/unraid-api/graph/resolvers/notifications/notifications.resolver.ts (1)
Resolver(24-173)api/src/core/pubsub.ts (2)
pubsub(12-12)
createSubscription(18-20)
🪛 GitHub Check: Test API
api/src/unraid-api/graph/resolvers/info/cpu-data.service.ts
[failure] 2-2:
Insert ⏎
api/src/unraid-api/graph/resolvers/info/info.model.ts
[failure] 145-145:
Replace ·description:·'The·percentage·of·time·the·CPU·spent·servicing·hardware·interrupts.' with ⏎········description:·'The·percentage·of·time·the·CPU·spent·servicing·hardware·interrupts.',⏎···
[failure] 139-139:
Replace ·description:·'The·percentage·of·time·the·CPU·spent·on·low-priority·(niced)·user·space·processes.' with ⏎········description:⏎············'The·percentage·of·time·the·CPU·spent·on·low-priority·(niced)·user·space·processes.',⏎···
api/src/unraid-api/graph/resolvers/info/info.resolver.ts
[failure] 23-23:
Replace ⏎····createSubscription,⏎····PUBSUB_CHANNEL,⏎····pubsub,⏎}·from·'@app/core/pubsub.js';⏎ with ·DisplayService·}·from·'@app/unraid-api/graph/resolvers/display/display.service.js';
[failure] 22-22:
Insert createSubscription,·pubsub,·PUBSUB_CHANNEL·}·from·'@app/core/pubsub.js';⏎import·{·
[failure] 17-17:
Replace ⏎····baseboard·as·getBaseboard,⏎····system·as·getSystem,⏎ with ·PubSub·}·from·'graphql-subscriptions';⏎import·{·baseboard·as·getBaseboard,·system·as·getSystem·
[failure] 2-2:
Replace ⏎····Parent,⏎····Query,⏎····Resolver,⏎····Subscription,⏎····ResolveField,⏎}·from·'@nestjs/graphql';⏎import·{·PubSub·}·from·'graphql-subscriptions with ·Parent,·Query,·ResolveField,·Resolver,·Subscription·}·from·'@nestjs/graphql
[failure] 41-41:
Replace ····CpuUtilization,⏎}·from·'@app/unraid-api/graph/resolvers/info/info.model.js';⏎import·{·CpuDataService·}·from·'@app/unraid-api/graph/resolvers/info/cpu-data.service with }·from·'@app/unraid-api/graph/resolvers/info/info.model
🪛 GitHub Actions: CI - Main (API)
api/src/unraid-api/graph/resolvers/info/cpu-data.service.ts
[error] 2-2: Step 'pnpm run lint' failed: Prettier formatting issue in src/unraid-api/graph/resolvers/info/cpu-data.service.ts. Insert ⏎ at line 2. (prettier/prettier)
🔇 Additional comments (8)
api/src/unraid-api/graph/resolvers/info/info.model.ts (1)
121-126: LGTM: new utilization field on InfoCpuOptional Float with clear description; matches the new query/subscription semantics.
api/src/unraid-api/graph/resolvers/info/info.service.ts (1)
104-112: Verify CPU payload structureI wasn’t able to inspect
systeminformation’s type definitions here, so please ensure that each item incpusmatches your GraphQLCpuLoadfields exactly. Confirm you have these properties (or map from snake_case variants):
load
loadUser(or map fromload_user)
loadSystem(or map fromload_system)
loadNice(or map fromload_nice)
loadIdle(or map fromload_idle)
loadIrq(or map fromload_irq)If the library returns snake_case keys, transform before returning. For example:
const { currentLoad: load, cpus: rawCpus } = await currentLoad(); const cpus = rawCpus.map((c: any) => ({ load: c.load, loadUser: c.loadUser ?? c.load_user, loadSystem: c.loadSystem ?? c.load_system, loadNice: c.loadNice ?? c.load_nice, loadIdle: c.loadIdle ?? c.load_idle, loadIrq: c.loadIrq ?? c.load_irq, })); return { id: 'info/cpu-load', load, cpus };api/src/unraid-api/graph/services/services.module.ts (1)
4-8: LGTM: services module wiringProvider/export for SubscriptionTrackerService looks correct and matches import usage in ResolversModule.
api/src/unraid-api/graph/resolvers/resolvers.module.ts (2)
36-46: LGTM: DI wiring adds ServicesModule (for SubscriptionTrackerService)Importing
ServicesModuleensures the tracker is available in resolver DI. ESM.jsspecifiers are correct per repo guidelines.
83-85: LGTM: providers registered for CPU data flowRegistering
InfoCpuResolverandCpuDataServiceintegrates the new CPU utilization endpoints cleanly.api/src/unraid-api/graph/resolvers/info/info.resolver.spec.ts (2)
192-204: Good DI: targeted mocks for CpuDataService and SubscriptionTrackerServiceNice job providing lightweight, focused mocks for both services. This keeps the resolver tests fast and deterministic while letting you assert interactions where needed.
225-233: Providers wiring looks correctRegistering the new services in the TestingModule aligns with the resolver’s constructor dependencies. This ensures the resolver can be instantiated without leaking real implementations into the unit tests.
api/src/unraid-api/graph/resolvers/info/info.resolver.ts (1)
200-212: No changes needed—InfoCpuResolver and CpuDataService are correctly wired
- InfoCpuResolver is imported and listed in
providersofResolversModule(api/src/unraid-api/graph/resolvers/resolvers.module.ts:83).- CpuDataService is imported, provided (with
@Injectable({ scope: Scope.REQUEST })), and listed in the same module (api/src/unraid-api/graph/resolvers/resolvers.module.ts:84).The new field resolver will be active as expected.
Adds a new query and subscription to the info resolver for CPU to get CPU utilization on Unraid. - Adds `cpuUtilization` query to get a one-time snapshot of CPU load. - Adds `cpuUtilizationSubscription` to get real-time updates on CPU load. - Adds a `utilization` field to the `InfoCpu` type. - Creates a generic `SubscriptionTrackerService` to manage polling for subscriptions, ensuring that polling only occurs when there are active subscribers. - Creates a request-scoped `CpuDataService` to cache CPU load data within a single GraphQL request to improve performance. - Updates tests to cover the new functionality. - Adds detailed documentation to the `CpuLoad` object type.
…ndling - Consolidated imports in the info resolver and service for better readability. - Added `CpuDataService` to manage CPU load data more efficiently. - Updated `InfoResolver` to include `InfoCpuResolver` for improved CPU data handling. - Enhanced documentation for CPU load fields in the model to improve clarity. - Streamlined the `SubscriptionTrackerService` for better topic management.
- Updated `CpuDataService` to use nullish coalescing for initializing `cpuLoadData`, enhancing readability and efficiency. - Ensured `cpuLoadData` can be undefined, allowing for more flexible handling of CPU load data retrieval.
- Changed CPU utilization topic references to use the new PUBSUB_CHANNEL enum for consistency. - Updated InfoResolver to handle CPU polling timer as potentially undefined, improving type safety. - Adjusted import statement for SubscriptionTrackerService to include the correct file extension.
- Removed unnecessary imports and parameters in InfoResolver for cleaner code. - Updated CPU utilization method to directly use infoService for generating CPU load. - Enhanced SubscriptionTrackerService to improve subscriber count management and added early return for idempotency in unsubscribe method.
07f9626    to
    3afc421      
    Compare
  
    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.
Actionable comments posted: 1
♻️ Duplicate comments (1)
api/src/unraid-api/graph/resolvers/info/info.resolver.ts (1)
33-41: Good: Resolver remains singleton (no request-scoped deps injected)Removing CpuDataService from InfoResolver avoids request-scope escalation and prevents onModuleInit from running per request. This addresses the earlier concern.
🧹 Nitpick comments (2)
api/src/unraid-api/graph/resolvers/info/info.resolver.ts (2)
42-55: Harden polling lifecycle: avoid overlap, handle errors, and clear interval safelyCurrent setInterval body is async without guarding reentrancy; if a tick runs longer than 1s, calls can overlap. Also, errors in the async body are unhandled, and clearInterval is called with a possibly undefined handle. Recommend the following refinements.
Apply this diff within this block:
this.subscriptionTracker.registerTopic( PUBSUB_CHANNEL.CPU_UTILIZATION, () => { - this.cpuPollingTimer = setInterval(async () => { - const payload = await this.infoService.generateCpuLoad(); - pubsub.publish(PUBSUB_CHANNEL.CPU_UTILIZATION, { cpuUtilization: payload }); - }, 1000); + let inFlight = false; + this.cpuPollingTimer = setInterval(async () => { + if (inFlight) return; + inFlight = true; + try { + const payload = await this.infoService.generateCpuLoad(); + await pubsub.publish(PUBSUB_CHANNEL.CPU_UTILIZATION, { cpuUtilization: payload }); + } catch (err) { + // TODO: optionally log error + } finally { + inFlight = false; + } + }, 1000); }, () => { - clearInterval(this.cpuPollingTimer); + if (this.cpuPollingTimer) { + clearInterval(this.cpuPollingTimer); + this.cpuPollingTimer = undefined; + } } );
181-188: Prefer GraphQL Float over Number for clarityUse Float from @nestjs/graphql to emit a GraphQLFloat. Number works but is less explicit.
Apply this diff here:
- @ResolveField(() => Number, { + @ResolveField(() => Float, { description: 'CPU utilization in percent', nullable: true, })And update the import at the top of this file:
// Add Float to the imports from @nestjs/graphql import { Query, ResolveField, Resolver, Subscription, Float } from '@nestjs/graphql';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
- api/src/unraid-api/graph/resolvers/info/cpu-data.service.ts(1 hunks)
- api/src/unraid-api/graph/resolvers/info/info.model.ts(1 hunks)
- api/src/unraid-api/graph/resolvers/info/info.resolver.spec.ts(3 hunks)
- api/src/unraid-api/graph/resolvers/info/info.resolver.ts(4 hunks)
- api/src/unraid-api/graph/resolvers/info/info.service.ts(2 hunks)
- api/src/unraid-api/graph/resolvers/resolvers.module.ts(3 hunks)
- api/src/unraid-api/graph/services/services.module.ts(1 hunks)
- api/src/unraid-api/graph/services/subscription-tracker.service.ts(1 hunks)
- packages/unraid-shared/src/pubsub/graphql.pubsub.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- api/src/unraid-api/graph/resolvers/info/cpu-data.service.ts
- api/src/unraid-api/graph/resolvers/info/info.service.ts
- api/src/unraid-api/graph/services/services.module.ts
- api/src/unraid-api/graph/services/subscription-tracker.service.ts
- api/src/unraid-api/graph/resolvers/resolvers.module.ts
- api/src/unraid-api/graph/resolvers/info/info.resolver.spec.ts
- api/src/unraid-api/graph/resolvers/info/info.model.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
TypeScript source files must use import specifiers with .js extensions for ESM compatibility
Files:
- packages/unraid-shared/src/pubsub/graphql.pubsub.ts
- api/src/unraid-api/graph/resolvers/info/info.resolver.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Avoid unnecessary or obvious comments; only add comments when needed for clarity
Files:
- packages/unraid-shared/src/pubsub/graphql.pubsub.ts
- api/src/unraid-api/graph/resolvers/info/info.resolver.ts
api/src/unraid-api/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Place new API source files in api/src/unraid-api/ rather than legacy locations
Prefer adding new files to the Nest repo at api/src/unraid-api/ instead of legacy code
Files:
- api/src/unraid-api/graph/resolvers/info/info.resolver.ts
🧬 Code Graph Analysis (1)
api/src/unraid-api/graph/resolvers/info/info.resolver.ts (3)
api/src/unraid-api/graph/resolvers/display/display.resolver.ts (1)
Resolver(14-37)api/src/unraid-api/graph/resolvers/array/array.resolver.ts (1)
Resolver(14-37)api/src/core/pubsub.ts (2)
pubsub(12-12)
createSubscription(18-20)
🔇 Additional comments (2)
packages/unraid-shared/src/pubsub/graphql.pubsub.ts (1)
7-9: New CPU_UTILIZATION pubsub channel looks goodEnum entry is consistent with existing channels and unblocks the resolver/subscription wiring that targets this topic.
api/src/unraid-api/graph/resolvers/info/info.resolver.ts (1)
142-150: Query cpuUtilization wired correctly to InfoServiceReturning CpuUtilization via InfoService keeps resolver singleton and matches the subscription payload shape.
…ion management - Added `SubscriptionHelperService` to streamline the creation of tracked GraphQL subscriptions with automatic cleanup. - Updated `InfoResolver` to utilize the new service for managing CPU utilization subscriptions, enhancing code clarity and maintainability. - Introduced unit tests for `SubscriptionHelperService` and `SubscriptionTrackerService` to ensure robust functionality and reliability. - Refactored `SubscriptionTrackerService` to include logging for subscription events, improving observability. - Removed deprecated types and methods related to previous subscription handling, simplifying the codebase.
…ote access and connection settings - Introduced new types including `AccessUrl`, `Cloud`, `RemoteAccess`, and `Connect` to enhance remote access capabilities. - Added enums for `URL_TYPE`, `WAN_ACCESS_TYPE`, and `WAN_FORWARD_TYPE` to standardize access configurations. - Implemented new input types for connection settings and remote access setup, improving API usability. - Updated the `Query` and `Mutation` types to include new fields for managing remote access and connection settings. - Bumped API version to 4.13.1 in configuration.
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.
Actionable comments posted: 0
♻️ Duplicate comments (2)
api/src/unraid-api/graph/resolvers/info/info.resolver.ts (2)
12-12: Good: central PUBSUB channel + ESM import extensions are correctly usedUsing PUBSUB_CHANNEL.CPU_UTILIZATION with .js import specifiers aligns with our ESM and central-channel guidelines. Thanks for addressing earlier feedback.
144-152: Query uses InfoService (singleton) — avoids request-scope escalationRouting cpuUtilization through InfoService keeps InfoResolver singleton and prevents OnModuleInit from running per request. This resolves the prior concern.
🧹 Nitpick comments (9)
api/src/unraid-api/graph/services/subscription-helper.service.ts (2)
19-21: Simplify iterator creation (avoid unnecessary cast and symbol lookup)createSubscription already returns an AsyncIterableIterator. No need to cast to AsyncIterable and then call Symbol.asyncIterator. This also reduces the chance of subtle iterator shape mismatches.
Apply this diff:
- const iterator = createSubscription(topic) as AsyncIterable<T>; - const innerIterator = iterator[Symbol.asyncIterator](); + const innerIterator = createSubscription(topic) as AsyncIterableIterator<T>;
26-28: Ensure cleanup on next() errors or iterator completionIf innerIterator.next() throws or resolves with done: true, the tracker never unsubscribes, potentially leaking a "subscriber" count. Wrap next() to unsubscribe on error or completion.
Apply this diff:
- const proxyIterator: AsyncIterableIterator<T> = { - next: () => innerIterator.next(), + const proxyIterator: AsyncIterableIterator<T> = { + next: async () => { + try { + const result = await innerIterator.next(); + if (result?.done) { + // Cleanup on natural completion + this.subscriptionTracker.unsubscribe(topic); + } + return result; + } catch (err) { + // Cleanup on error during next() + this.subscriptionTracker.unsubscribe(topic); + throw err; + } + },api/generated-schema.graphql (1)
2239-2239: Align subscription field naming for consistencyThe
Subscriptiontype currently mixes suffixed and non-suffixed field names:• Suffix present:
- displaySubscription
- infoSubscription
- ownerSubscription
- serversSubscription
- parityHistorySubscription
- arraySubscription
• Suffix missing:
- cpuUtilization
- logFile
- notificationAdded
- notificationsOverview
- upsUpdates
To keep a clear, uniform convention (and avoid confusing GraphQL clients), consider appending
Subscriptionto each non-suffixed field:• cpuUtilization → cpuUtilizationSubscription
• logFile → logFileSubscription
• notificationAdded → notificationAddedSubscription
• notificationsOverview → notificationsOverviewSubscription
• upsUpdates → upsUpdatesSubscriptionLocations to update:
- Schema (
api/generated-schema.graphql): rename fields undertype Subscription.- Resolvers (e.g.,
info.resolver.ts,logs.resolver.ts,ups.resolver.ts, etc.): adjust@Subscription({ name: '…' })decorators and/or method names to match the new GraphQL names.This refactor will ensure all subscription endpoints follow the same naming pattern.
api/src/unraid-api/graph/services/subscription-tracker.service.spec.ts (2)
67-75: Relax log assertions to avoid brittle testsAsserting exact log strings is fragile. Prefer expect.stringContaining() to focus on behavior over wording per our testing guidelines.
Apply these diffs:
@@ - expect(loggerSpy).toHaveBeenCalledWith( - "Subscription added for topic 'TEST_TOPIC': 1 active subscriber(s)" - ); + expect(loggerSpy).toHaveBeenCalledWith( + expect.stringContaining("Subscription added for topic 'TEST_TOPIC'") + ); @@ - expect(loggerSpy).toHaveBeenCalledWith( - "Subscription added for topic 'TEST_TOPIC': 2 active subscriber(s)" - ); + expect(loggerSpy).toHaveBeenCalledWith( + expect.stringContaining("Subscription added for topic 'TEST_TOPIC'") + ); @@ - expect(loggerSpy).toHaveBeenCalledWith("Starting topic 'TEST_TOPIC' (first subscriber)"); + expect(loggerSpy).toHaveBeenCalledWith( + expect.stringContaining("Starting topic 'TEST_TOPIC'") + ); @@ - expect(loggerSpy).toHaveBeenCalledWith( - "Unsubscribe called for topic 'TEST_TOPIC' but no active subscribers (idempotent)" - ); + expect(loggerSpy).toHaveBeenCalledWith( + expect.stringContaining("no active subscribers (idempotent)") + ); @@ - expect(loggerSpy).toHaveBeenCalledWith( - "Subscription removed for topic 'TEST_TOPIC': 1 active subscriber(s) remaining" - ); + expect(loggerSpy).toHaveBeenCalledWith( + expect.stringContaining("Subscription removed for topic 'TEST_TOPIC'") + ); @@ - expect(loggerSpy).toHaveBeenCalledWith( - "Subscription removed for topic 'TEST_TOPIC': 0 active subscriber(s) remaining" - ); + expect(loggerSpy).toHaveBeenCalledWith( + expect.stringContaining("Subscription removed for topic 'TEST_TOPIC'") + ); @@ - expect(loggerSpy).toHaveBeenCalledWith( - "Stopping topic 'TEST_TOPIC' (last subscriber removed)" - ); + expect(loggerSpy).toHaveBeenCalledWith( + expect.stringContaining("Stopping topic 'TEST_TOPIC'") + );Also applies to: 84-85, 137-139, 149-156, 169-171
9-9: Type the spy for clarityPrefer a specific spy type instead of any.
Apply this diff:
- let loggerSpy: any; + let loggerSpy: vi.SpyInstance;api/src/unraid-api/graph/services/subscription-helper.service.spec.ts (2)
55-67: Use.rejects.toThrow()for async error assertionAligns with our testing guideline to assert thrown errors succinctly.
Apply this diff:
- try { - await iterator.throw?.(new Error('Test error')); - } catch (e) { - // Expected to throw - } + await expect(iterator.throw?.(new Error('Test error'))).rejects.toThrow();
168-183: Simplify error test with.rejects.toThrow()and avoid matching specific error objectFocus on behavior, not exact error identity, to reduce brittleness.
Apply this diff:
- const testError = new Error('Test error'); - - try { - await iterator.throw?.(testError); - expect.fail('Should have thrown'); - } catch (error) { - expect(error).toBe(testError); - } + await expect(iterator.throw?.(new Error('Test error'))).rejects.toThrow();api/src/unraid-api/graph/resolvers/info/info.resolver.ts (2)
44-57: Make polling start/stop idempotent and handle errors inside the intervalGuard against multiple intervals, clear safely, and prevent unhandled rejections in the async interval callback.
Apply this diff:
- onModuleInit() { - this.subscriptionTracker.registerTopic( - PUBSUB_CHANNEL.CPU_UTILIZATION, - () => { - this.cpuPollingTimer = setInterval(async () => { - const payload = await this.infoService.generateCpuLoad(); - pubsub.publish(PUBSUB_CHANNEL.CPU_UTILIZATION, { cpuUtilization: payload }); - }, 1000); - }, - () => { - clearInterval(this.cpuPollingTimer); - } - ); - } + onModuleInit() { + this.subscriptionTracker.registerTopic( + PUBSUB_CHANNEL.CPU_UTILIZATION, + () => { + if (this.cpuPollingTimer) { + // Already polling + return; + } + this.cpuPollingTimer = setInterval(async () => { + try { + const payload = await this.infoService.generateCpuLoad(); + pubsub.publish(PUBSUB_CHANNEL.CPU_UTILIZATION, { cpuUtilization: payload }); + } catch (_err) { + // Swallow or log as needed to avoid unhandled rejection noise + } + }, 1000); + }, + () => { + if (this.cpuPollingTimer) { + clearInterval(this.cpuPollingTimer); + this.cpuPollingTimer = undefined; + } + } + ); + }Optionally, consider adding OnModuleDestroy to clear any leftover interval on shutdown.
172-179: Prefer GraphQL Float for utilization scalarNumber works but Float communicates intent and aligns with GraphQL scalar mappings in Nest.
Apply this diff in-place:
- @ResolveField(() => Number, { + @ResolveField(() => Float, { description: 'CPU utilization in percent', nullable: true, })Also add the import (outside this hunk):
// at the top import from @nestjs/graphql import { Query, ResolveField, Resolver, Subscription, Float } from '@nestjs/graphql';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
- api/dev/configs/api.json(1 hunks)
- api/generated-schema.graphql(3 hunks)
- api/src/unraid-api/graph/resolvers/info/info.resolver.ts(4 hunks)
- api/src/unraid-api/graph/services/services.module.ts(1 hunks)
- api/src/unraid-api/graph/services/subscription-helper.service.spec.ts(1 hunks)
- api/src/unraid-api/graph/services/subscription-helper.service.ts(1 hunks)
- api/src/unraid-api/graph/services/subscription-tracker.service.spec.ts(1 hunks)
- api/src/unraid-api/graph/services/subscription-tracker.service.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- api/dev/configs/api.json
🚧 Files skipped from review as they are similar to previous changes (1)
- api/src/unraid-api/graph/services/subscription-tracker.service.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
TypeScript source files must use import specifiers with .js extensions for ESM compatibility
Files:
- api/src/unraid-api/graph/services/services.module.ts
- api/src/unraid-api/graph/services/subscription-tracker.service.spec.ts
- api/src/unraid-api/graph/services/subscription-helper.service.spec.ts
- api/src/unraid-api/graph/services/subscription-helper.service.ts
- api/src/unraid-api/graph/resolvers/info/info.resolver.ts
api/src/unraid-api/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Place new API source files in api/src/unraid-api/ rather than legacy locations
Prefer adding new files to the Nest repo at api/src/unraid-api/ instead of legacy code
Files:
- api/src/unraid-api/graph/services/services.module.ts
- api/src/unraid-api/graph/services/subscription-tracker.service.spec.ts
- api/src/unraid-api/graph/services/subscription-helper.service.spec.ts
- api/src/unraid-api/graph/services/subscription-helper.service.ts
- api/src/unraid-api/graph/resolvers/info/info.resolver.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Avoid unnecessary or obvious comments; only add comments when needed for clarity
Files:
- api/src/unraid-api/graph/services/services.module.ts
- api/src/unraid-api/graph/services/subscription-tracker.service.spec.ts
- api/src/unraid-api/graph/services/subscription-helper.service.spec.ts
- api/src/unraid-api/graph/services/subscription-helper.service.ts
- api/src/unraid-api/graph/resolvers/info/info.resolver.ts
api/**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Prefer not to mock simple dependencies in API tests
Files:
- api/src/unraid-api/graph/services/subscription-tracker.service.spec.ts
- api/src/unraid-api/graph/services/subscription-helper.service.spec.ts
**/*.{test,spec}.{js,ts,jsx,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.{test,spec}.{js,ts,jsx,tsx}: General testing: use .rejects.toThrow() without arguments when asserting thrown errors
General testing: focus on behavior, not exact error message wording
General testing: avoid brittle tests tied to non-essential details (exact messages, log formats)
General testing: use mocks as nouns (objects), not verbs (behavioral overuse)
Files:
- api/src/unraid-api/graph/services/subscription-tracker.service.spec.ts
- api/src/unraid-api/graph/services/subscription-helper.service.spec.ts
api/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/api-rules.mdc)
api/**/*.{test,spec}.{js,jsx,ts,tsx}: Use Vitest for tests in the api; do not use Jest
Prefer not to mock simple dependencies in tests
For error testing, use.rejects.toThrow()without arguments; avoid asserting exact error messages unless the message format is the subject under test
Files:
- api/src/unraid-api/graph/services/subscription-tracker.service.spec.ts
- api/src/unraid-api/graph/services/subscription-helper.service.spec.ts
🧠 Learnings (4)
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to {**/*.test.ts,**/__test__/{components,store}/**/*.ts} : Focus tests on observable behavior and outcomes, not implementation details such as exact error messages
Applied to files:
- api/src/unraid-api/graph/services/subscription-tracker.service.spec.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to {**/*.test.ts,**/__test__/{components,store}/**/*.ts} : Use vi.spyOn() to specify return values or behavior of methods under test
Applied to files:
- api/src/unraid-api/graph/services/subscription-tracker.service.spec.ts
📚 Learning: 2025-07-22T13:29:32.175Z
Learnt from: elibosley
PR: unraid/api#1526
File: api/src/unraid-api/graph/resolvers/ups/ups.resolver.ts:82-85
Timestamp: 2025-07-22T13:29:32.175Z
Learning: In GraphQL subscriptions using the graphql-subscriptions PubSub, the correct method is `asyncIterableIterator()`, not `asyncIterator()`. The `asyncIterableIterator` method returns an `AsyncIterableIterator` that conforms to both `AsyncIterable` and `AsyncIterator` interfaces, which is required for GraphQL subscription resolvers. Using `asyncIterator()` would cause TypeScript type errors as it only returns an `AsyncIterator` that doesn't satisfy the `AsyncIterable` interface requirement.
Applied to files:
- api/src/unraid-api/graph/services/subscription-helper.service.spec.ts
- api/src/unraid-api/graph/services/subscription-helper.service.ts
- api/src/unraid-api/graph/resolvers/info/info.resolver.ts
📚 Learning: 2025-07-22T13:29:32.175Z
Learnt from: elibosley
PR: unraid/api#1526
File: api/src/unraid-api/graph/resolvers/ups/ups.resolver.ts:82-85
Timestamp: 2025-07-22T13:29:32.175Z
Learning: In GraphQL subscriptions using the graphql-subscriptions PubSub, the correct method to create an async iterator is `asyncIterableIterator()`, not `asyncIterator()`. This is the modern/current method used in the graphql-subscriptions library.
Applied to files:
- api/src/unraid-api/graph/services/subscription-helper.service.ts
- api/src/unraid-api/graph/resolvers/info/info.resolver.ts
🧬 Code Graph Analysis (4)
api/src/unraid-api/graph/services/services.module.ts (1)
api/src/unraid-api/graph/resolvers/resolvers.module.ts (1)
Module(42-87)
api/src/unraid-api/graph/services/subscription-helper.service.spec.ts (1)
api/src/core/pubsub.ts (1)
pubsub(12-12)
api/src/unraid-api/graph/services/subscription-helper.service.ts (1)
api/src/core/pubsub.ts (1)
createSubscription(18-20)
api/src/unraid-api/graph/resolvers/info/info.resolver.ts (1)
api/src/core/pubsub.ts (1)
pubsub(12-12)
🔇 Additional comments (8)
api/src/unraid-api/graph/services/services.module.ts (1)
6-10: Module wiring looks correct and ESM-compatibleProviders/exports are properly declared, imports use .js extensions per our ESM guideline, and the file lives under api/src/unraid-api/. This should integrate cleanly with ResolversModule.
api/src/unraid-api/graph/services/subscription-helper.service.ts (1)
22-24: Good ordering to avoid initial publish raceCreating the async iterator before calling subscribe() helps ensure listeners are attached before onStart-triggered polling/publishing begins.
api/generated-schema.graphql (4)
982-984: InfoCpu.utilization addition looks goodOptional utilization field is a sensible extension and aligns with the PR’s goals.
986-1007: CpuLoad type is well-documented and clearThe per-core load fields and descriptions look consistent and useful for clients.
1009-1013: CpuUtilization Node type LGTMNon-null id and top-level load with per-core cpus array matches expected usage.
1955-1955: Query.cpuUtilization resolver is correctly implemented and wired
- A
@Query(() => CpuUtilization)method namedcpuUtilizationexists inInfoResolver(lines 150–152 ofinfo.resolver.ts) and returnsthis.infoService.generateCpuLoad().- The
CpuUtilizationGraphQL type is defined ininfo.model.tsas
@ObjectType({ implements: () => Node })
extendingNode, so it inherits a@Field(() => PrefixedID) id: stringfield.- Both
InfoResolver(withcpuUtilization) andInfoCpuResolverare listed underprovidersinResolversModule, andServicesModuleis imported.No further changes are needed.
api/src/unraid-api/graph/resolvers/info/info.resolver.ts (2)
168-171: Ok to inject request-scoped CpuDataService in the field resolverKeeping request-scoped caching at InfoCpuResolver while InfoResolver remains singleton is the right split.
154-165: SubscriptionHelper returns AsyncIterableIterator & manages lifecycle correctlyThe
createTrackedSubscriptionmethod insubscription-helper.service.tshas a return type ofAsyncIterableIterator<T>and usesSymbol.asyncIterator. It also callsthis.subscriptionTracker.subscribe(topic)when the iterator starts andunsubscribe(topic)on both return and error, fulfilling the expected subscribe/unsubscribe behavior.
…etrics resolver (#1594) - also enables subscription resolution for CPU and memory usage
- Updated `createSubscription` and `createTrackedSubscription` methods to include generic type parameters, improving type safety and ensuring correct async iterable handling.
…ption handling - Removed legacy polling methods from `MetricsResolver` and integrated polling logic into `SubscriptionPollingService` for better separation of concerns. - Updated `SubscriptionTrackerService` to support polling configuration directly, allowing for cleaner topic registration. - Adjusted unit tests to accommodate changes in the subscription handling and polling logic. - Introduced `SubscriptionPollingService` to manage polling intervals and ensure efficient execution of polling tasks.
…ptionPollingService - Integrated `ScheduleModule` to manage polling intervals effectively. - Updated tests to utilize `SubscriptionPollingService` for CPU and memory polling, ensuring single execution during concurrent attempts. - Improved error handling in polling tests to verify graceful error logging. - Ensured proper cleanup of polling subscriptions and timers during module destruction.
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.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️  Outside diff range comments (2)
api/src/unraid-api/graph/resolvers/info/devices/devices.service.ts (1)
103-108: Missing bus and device fields for USB devices.The
generateUsb()method only mapsidandnamefields, but theInfoUsbtype also includes optionalbusanddevicefields. The privategetSystemUSBDevices()returns aUsbDevicetype that doesn't include these fields.Consider extracting bus and device information from the USB device data. The
lsusboutput typically includes bus and device numbers that could be parsed and included:- async generateUsb(): Promise<InfoUsb[]> { + async generateUsb(): Promise<InfoUsb[]> { try { const usbDevices = await this.getSystemUSBDevices(); return usbDevices.map((device) => ({ id: `usb/${device.id}`, name: device.name, + bus: device.bus, + device: device.device, })); } catch (error: unknown) {This would also require updating the
UsbDeviceinterface and the parsing logic ingetSystemUSBDevices().api/src/unraid-api/graph/resolvers/info/info.resolver.ts (1)
75-79: Fix GraphQL type mismatch: machineId should be ID, not String.The Info model declares machineId as ID; resolver annotates it as String. Align resolver decorator to ID to match the generated schema and avoid confusion.
- @ResolveField(() => String, { nullable: true }) + @ResolveField(() => ID, { nullable: true }) public async machineId(): Promise<string | undefined> { return getMachineId(); }Note: Also import ID from @nestjs/graphql (see diff below).
🧹 Nitpick comments (29)
api/src/unraid-api/graph/resolvers/info/cpu/cpu.model.ts (1)
88-90: Preferunknownoveranyfor JSON payloadsTighten typing for the JSON cache to avoid
anyleakage.- cache?: Record<string, any>; + cache?: Record<string, unknown>;api/src/unraid-api/graph/resolvers/info/display/display.model.ts (1)
1-1: Remove unusedFloatimport
Floatisn’t used in this file.-import { Field, Float, Int, ObjectType, registerEnumType } from '@nestjs/graphql'; +import { Field, Int, ObjectType, registerEnumType } from '@nestjs/graphql';api/src/unraid-api/graph/resolvers/metrics/metrics.module.ts (1)
8-12: Avoid duplicating CpuService/CpuDataService/MemoryService providers across modules to preserve request-scoped cachingMetricsModule re-declares CpuService, CpuDataService, and MemoryService, which are also provided by InfoModule. For request-scoped CpuDataService, this results in separate instances per module per request, defeating the goal of sharing cached CPU data across resolvers within a single GraphQL request. Centralize these providers in one module and re-use via exports/imports.
Recommended approach:
- Export CpuService, CpuDataService, and MemoryService from InfoModule.
- Import InfoModule in MetricsModule and remove the duplicate providers.
Apply this diff to MetricsModule:
import { Module } from '@nestjs/common'; import { CpuDataService, CpuService } from '@app/unraid-api/graph/resolvers/info/cpu/cpu.service.js'; import { MemoryService } from '@app/unraid-api/graph/resolvers/info/memory/memory.service.js'; import { MetricsResolver } from '@app/unraid-api/graph/resolvers/metrics/metrics.resolver.js'; import { ServicesModule } from '@app/unraid-api/graph/services/services.module.js'; +import { InfoModule } from '@app/unraid-api/graph/resolvers/info/info.module.js'; @Module({ - imports: [ServicesModule], - providers: [MetricsResolver, CpuService, CpuDataService, MemoryService], + imports: [ServicesModule, InfoModule], + providers: [MetricsResolver], exports: [MetricsResolver], }) export class MetricsModule {}And update InfoModule exports as suggested in the InfoModule review comment.
api/src/unraid-api/graph/resolvers/info/info.module.ts (2)
14-34: Module wiring looks solid; consider exporting CPU/memory services for reuseInfoModule correctly registers the Info resolver and sub-services. To prevent duplicate provider instances and to share request-scoped CpuDataService across resolvers (including Metrics), export CpuService, CpuDataService, and MemoryService here and have MetricsModule import InfoModule instead of re-providing them.
Apply this diff to export the services:
@Module({ imports: [ConfigModule, ServicesModule], providers: [ // Main resolver InfoResolver, // Sub-resolvers DevicesResolver, // Services CpuService, CpuDataService, MemoryService, DevicesService, OsService, VersionsService, DisplayService, ], - exports: [InfoResolver, DevicesResolver, DisplayService], + exports: [ + InfoResolver, + DevicesResolver, + DisplayService, + CpuService, + CpuDataService, + MemoryService, + ], }) export class InfoModule {}
32-32: Optional: avoid exporting resolvers unless another module consumes themExporting resolvers is usually unnecessary unless another module injects them. You can simplify the surface by only exporting services that are re-used elsewhere.
packages/unraid-shared/src/pubsub/graphql.pubsub.ts (1)
11-11: Add brief docs for new channels to aid discoverabilityConsider documenting payload shape or linking to the CpuLoad type where appropriate. This helps consumers of the shared package know what data to expect.
Example:
/** Publishes CpuLoad snapshots/stream data; see CpuLoad GraphQL type for fields */ CPU_UTILIZATION = "CPU_UTILIZATION", /** Publishes memory utilization snapshots/stream data */ MEMORY_UTILIZATION = "MEMORY_UTILIZATION",api/src/core/pubsub.ts (1)
18-21: Preferunknownoveranyas the default genericThis strengthens typing without affecting call sites that explicitly specify T.
Apply:
-export const createSubscription = <T = any>( +export const createSubscription = <T = unknown>( channel: GRAPHQL_PUBSUB_CHANNEL ): AsyncIterableIterator<T> => { return pubsub.asyncIterableIterator<T>(channel); };api/src/unraid-api/graph/resolvers/info/os/os.service.ts (1)
14-19: Optional: makehostnameresolution resilient with a fallbackIf getters.emhttp().var.name is ever unset, we can fall back to the system hostname. This avoids returning undefined and better matches the schema’s intent.
Apply:
@@ -import { osInfo } from 'systeminformation'; +import { osInfo } from 'systeminformation'; +import { hostname as getHostname } from 'node:os'; @@ - hostname: getters.emhttp().var.name, + hostname: getters.emhttp()?.var?.name ?? getHostname(), uptime: bootTimestamp.toISOString(),api/src/unraid-api/graph/resolvers/info/display/display.service.spec.ts (1)
40-40: Optional: reduce reliance on “dev files” in testsThe test mentions using actual dev files for configuration merging. That can be brittle across environments. Prefer deterministic fixtures (e.g., fixtures/default.cfg and fixtures/dynamix.cfg) and mock fs.readFile to return those contents. This keeps behavior-focused tests stable and faster.
I can extract those contents into fixtures and wire up the vi.mock readFile to use them if you want.
api/src/unraid-api/graph/resolvers/info/os/os.model.ts (2)
46-48: Unify nullable boolean typing (avoid boolean | null union)For consistency with other fields that use
nullable: true, preferuefi?: booleaninstead ofboolean | null.- @Field(() => Boolean, { nullable: true, description: 'OS started via UEFI' }) - uefi?: boolean | null; + @Field(() => Boolean, { nullable: true, description: 'OS started via UEFI' }) + uefi?: boolean;
37-38: Clarifyuptimedescription (name vs. meaning)Field name suggests "time since boot", but description says "Boot time ISO string". Recommend clarifying to avoid confusion.
- @Field(() => String, { nullable: true, description: 'Boot time ISO string' }) + @Field(() => String, { nullable: true, description: 'System uptime as ISO-8601 duration string (e.g., PT1H35M)' })If the value is actually the boot timestamp, consider renaming the field to
bootTimein a follow-up breaking-change PR.api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.spec.ts (3)
1-11: Use topic constants instead of raw stringsImport the shared pubsub topic constants to avoid string drift and improve refactor safety.
import { SubscriptionHelperService } from '@app/unraid-api/graph/services/subscription-helper.service.js'; import { SubscriptionTrackerService } from '@app/unraid-api/graph/services/subscription-tracker.service.js'; +import { CPU_UTILIZATION, MEMORY_UTILIZATION } from '@unraid/shared/pubsub/graphql.pubsub.js';
123-127: Avoid asserting exact error messages in testsPer the testing guidelines, use
.rejects.toThrow()without arguments to prevent brittle tests tied to message wording.- await expect(resolver.cpu()).rejects.toThrow('CPU error'); + await expect(resolver.cpu()).rejects.toThrow(); - await expect(resolver.memory()).rejects.toThrow('Memory error'); + await expect(resolver.memory()).rejects.toThrow();Also applies to: 151-155
175-183: Replace hardcoded topic names with constantsUse imported
CPU_UTILIZATIONandMEMORY_UTILIZATIONto keep tests aligned with production topics.- expect(subscriptionTracker.registerTopic).toHaveBeenCalledWith( - 'CPU_UTILIZATION', + expect(subscriptionTracker.registerTopic).toHaveBeenCalledWith( + CPU_UTILIZATION, expect.any(Function), 1000 ); - expect(subscriptionTracker.registerTopic).toHaveBeenCalledWith( - 'MEMORY_UTILIZATION', + expect(subscriptionTracker.registerTopic).toHaveBeenCalledWith( + MEMORY_UTILIZATION, expect.any(Function), 2000 );api/src/unraid-api/graph/resolvers/info/memory/memory.service.ts (2)
16-22: Prefer stable IDs for memory layout entries over index-based IDsIndex-based IDs can shuffle across reboots or hardware changes, making caching/normalization brittle. Derive IDs from stable properties when available (e.g., serialNum/partNum/bank/size), falling back to index only if necessary.
- dims.map( - (dim, index) => - ({ - ...dim, - id: `memory-layout-${index}`, - }) as MemoryLayout - ) + dims.map((dim, index) => { + const stableKey = + [dim.serialNum, dim.partNum, dim.bank, dim.size].filter(Boolean).join(':') || + String(index); + return { ...dim, id: `memory-layout-${stableKey}` } as MemoryLayout; + })
32-49: Harden memory load collection with error handling and bounded percentagesIf
systeminformation.mem()throws (e.g., permissions or unsupported platform in CI), the resolver will reject and bubble errors. Also, percent calculations can produce out-of-range results on corner values. Add a try/catch and clamp to [0, 100].- async generateMemoryLoad(): Promise<MemoryUtilization> { - const memInfo = await mem(); + async generateMemoryLoad(): Promise<MemoryUtilization> { + let memInfo; + try { + memInfo = await mem(); + } catch { + return { + id: 'memory-utilization', + total: 0, + used: 0, + free: 0, + available: 0, + active: 0, + buffcache: 0, + usedPercent: 0, + swapTotal: 0, + swapUsed: 0, + swapFree: 0, + swapUsedPercent: 0, + }; + } return { id: 'memory-utilization', total: Math.floor(memInfo.total), used: Math.floor(memInfo.used), free: Math.floor(memInfo.free), available: Math.floor(memInfo.available), active: Math.floor(memInfo.active), buffcache: Math.floor(memInfo.buffcache), - usedPercent: - memInfo.total > 0 ? ((memInfo.total - memInfo.available) / memInfo.total) * 100 : 0, + usedPercent: + memInfo.total > 0 + ? Math.min(100, Math.max(0, ((memInfo.total - memInfo.available) / memInfo.total) * 100)) + : 0, swapTotal: Math.floor(memInfo.swaptotal), swapUsed: Math.floor(memInfo.swapused), swapFree: Math.floor(memInfo.swapfree), - swapUsedPercent: memInfo.swaptotal > 0 ? (memInfo.swapused / memInfo.swaptotal) * 100 : 0, + swapUsedPercent: + memInfo.swaptotal > 0 + ? Math.min(100, Math.max(0, (memInfo.swapused / memInfo.swaptotal) * 100)) + : 0, }; }api/src/unraid-api/graph/resolvers/info/system/system.model.ts (1)
46-50: Optional: Consider Int for slot counts if schema allowsIf
memSlotsis semantically an integer, using@Field(() => Int)can better convey intent. This would require aligning the generated schema (currently Float) and any frontend types first.api/src/unraid-api/graph/resolvers/info/display/display.service.ts (2)
147-164: Avoid NaN creeping into numeric config fields
Number.parseInt(...)can yieldNaNfor missing/invalid values, which then propagates to the API asNaN. Coerce toundefinedso the defaults ingenerateDisplay()apply cleanly.- return { - ...display, - theme: theme as ThemeName, - unit: unit as Temperature, - scale: toBoolean(display.scale), - tabs: toBoolean(display.tabs), - resize: toBoolean(display.resize), - wwn: toBoolean(display.wwn), - total: toBoolean(display.total), - usage: toBoolean(display.usage), - text: toBoolean(display.text), - warning: Number.parseInt(display.warning, 10), - critical: Number.parseInt(display.critical, 10), - hot: Number.parseInt(display.hot, 10), - max: Number.parseInt(display.max, 10), - locale: display.locale || 'en_US', - }; + const toIntOrUndef = (v: unknown) => { + const n = Number.parseInt(String(v ?? ''), 10); + return Number.isFinite(n) ? n : undefined; + }; + return { + ...display, + theme: theme as ThemeName, + unit: unit as Temperature, + scale: toBoolean(display.scale), + tabs: toBoolean(display.tabs), + resize: toBoolean(display.resize), + wwn: toBoolean(display.wwn), + total: toBoolean(display.total), + usage: toBoolean(display.usage), + text: toBoolean(display.text), + warning: toIntOrUndef(display.warning), + critical: toIntOrUndef(display.critical), + hot: toIntOrUndef(display.hot), + max: toIntOrUndef(display.max), + locale: display.locale || 'en_US', + };
132-165: Validate incoming enum values (ThemeName, Temperature) defensivelyCasting
theme/unitblindly may accept invalid values from config. Optionally, validate against the enum keys and fall back to defaults.Example (outside this hunk):
const isTheme = (t: unknown): t is ThemeName => Object.values(ThemeName).includes(t as ThemeName); const isTemp = (u: unknown): u is Temperature => Object.values(Temperature).includes(u as Temperature); ... theme: isTheme(theme) ? theme : ThemeName.white, unit: isTemp(unit) ? unit : Temperature.CELSIUS,api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.ts (1)
19-35: Consider making the ID format consistent across services.The
generateCpu()method returnsid: 'info/cpu'whilegenerateCpuLoad()returnsid: 'info/cpu-load'. Consider standardizing the ID format for consistency - either use hyphens or forward slashes consistently.Apply this diff to align ID format:
async generateCpuLoad(): Promise<CpuUtilization> { const { currentLoad: load, cpus } = await currentLoad(); return { - id: 'info/cpu-load', + id: 'info/cpu/load', load, cpus, }; }api/src/unraid-api/graph/resolvers/info/devices/devices.service.ts (1)
87-99: Consider implementing network device generation or documenting the placeholder.The
generateNetwork()method returns an empty array with a comment about future implementation. Consider either implementing basic network interface detection or adding a TODO comment to track this.Would you like me to implement basic network interface detection using the
systeminformationlibrary'snetworkInterfaces()function, or would you prefer to create a tracking issue for this?api/src/unraid-api/graph/services/subscription-tracker.service.ts (4)
37-40: Prefer nullish-coalescing-safe default for onStop (avoid logical OR on functions).Use an explicit typeof check to avoid accidental fallback and for clarity.
- this.topicHandlers.set(topic, { - onStart: callbackOrOnStart, - onStop: intervalMsOrOnStop || (() => {}), - }); + this.topicHandlers.set(topic, { + onStart: callbackOrOnStart, + onStop: typeof intervalMsOrOnStop === 'function' ? intervalMsOrOnStop : () => {}, + });
51-56: Guard onStart with try/catch to prevent unhandled exceptions from breaking subscription flows.Starting polling can throw (e.g., scheduler issues). Log and continue.
- const handlers = this.topicHandlers.get(topic); - if (handlers?.onStart) { - handlers.onStart(); - } + const handlers = this.topicHandlers.get(topic); + if (handlers?.onStart) { + try { + handlers.onStart(); + } catch (err) { + this.logger.error( + `Error in onStart handler for topic '${topic}': ${ + err instanceof Error ? err.message : String(err) + }` + ); + } + }
100-104: Guard onStop with try/catch to ensure cleanup doesn’t crash the caller.Stopping polling should never throw back to subscribers. Log and proceed.
- const handlers = this.topicHandlers.get(topic); - if (handlers?.onStop) { - handlers.onStop(); - } + const handlers = this.topicHandlers.get(topic); + if (handlers?.onStop) { + try { + handlers.onStop(); + } catch (err) { + this.logger.error( + `Error in onStop handler for topic '${topic}': ${ + err instanceof Error ? err.message : String(err) + }` + ); + } + }
19-42: Tighten topic typing to PUBSUB_CHANNEL for end-to-end type safetyCurrently, SubscriptionTrackerService uses
stringfor its topic keys and method signatures, which leaves us open to typos and gaps in compile-time coverage. SincePUBSUB_CHANNELis already the source-of-truth in SubscriptionHelperService and our resolvers, update this service to usePUBSUB_CHANNELinstead.Required refactors:
- Import the enum at the top of api/src/unraid-api/graph/services/subscription-tracker.service.ts:import type { PUBSUB_CHANNEL } from '@app/core/pubsub.js';
- Change your maps:
•private subscriberCounts = new Map<PUBSUB_CHANNEL, number>();
•private topicHandlers = new Map<PUBSUB_CHANNEL, { onStart: () => void; onStop: () => void }>();- Update all method signatures:
•registerTopic(topic: PUBSUB_CHANNEL, …)
•subscribe(topic: PUBSUB_CHANNEL)
•unsubscribe(topic: PUBSUB_CHANNEL)
•getSubscriberCount(topic: PUBSUB_CHANNEL)- Adjust any internal calls (e.g.
this.topicHandlers.set(topic, …)) to compile against the new enum-typed keys.This alignment will ensure end-to-end type safety across tracker, helper service, and resolvers.
api/src/unraid-api/graph/services/subscription-polling.service.ts (3)
41-45: Log error message and stack clearly for easier debugging.Current Logger.error usage passes the whole error object. Prefer message + stack.
- } catch (error) { - this.logger.error(`Error in polling task '${name}'`, error); + } catch (error) { + this.logger.error( + `Error in polling task '${name}': ${ + error instanceof Error ? error.message : String(error) + }`, + error instanceof Error ? error.stack : undefined + );
52-57: Prime the poller immediately so first subscribers get data without waiting a full interval.Kick off one run right after registration.
// Create and register the interval const interval = setInterval(pollFunction, intervalMs); this.schedulerRegistry.addInterval(name, interval); + // Prime once immediately + void pollFunction(); + this.logger.debug(`Started polling for '${name}' every ${intervalMs}ms`);
24-31: Validate interval input to avoid misconfiguration (non-positive/NaN).Defensive guard prevents scheduling nonsense intervals.
startPolling(config: PollingConfig): void { const { name, intervalMs, callback } = config; + if (!Number.isFinite(intervalMs) || intervalMs <= 0) { + this.logger.warn(`Skipping polling for '${name}': invalid interval ${intervalMs}`); + return; + } + // Clean up any existing interval this.stopPolling(name);api/src/unraid-api/graph/resolvers/info/info.resolver.ts (1)
1-1: Add ID to imports for resolver decorator above.-import { GraphQLISODateTime, Query, ResolveField, Resolver } from '@nestjs/graphql'; +import { GraphQLISODateTime, ID, Query, ResolveField, Resolver } from '@nestjs/graphql';
        
          
                api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.integration.spec.ts
          
            Show resolved
            Hide resolved
        
      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.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️  Outside diff range comments (4)
api/src/unraid-api/graph/resolvers/info/display/display.service.ts (1)
132-165: NaN integers can leak to GraphQL (will throw at runtime)Number.parseInt(...) can return NaN (e.g., undefined, empty string). Using ?? later won’t catch NaN, so fields like warning, critical, hot, and max can be NaN, which is invalid for GraphQL Int and will cause runtime errors.
Return undefined on invalid parses so the defaults in generateDisplay apply cleanly.
Apply this diff to sanitize numeric and enum parsing:
- const { theme, unit, ...display } = state.display; - return { - ...display, - theme: theme as ThemeName, - unit: unit as Temperature, - scale: toBoolean(display.scale), - tabs: toBoolean(display.tabs), - resize: toBoolean(display.resize), - wwn: toBoolean(display.wwn), - total: toBoolean(display.total), - usage: toBoolean(display.usage), - text: toBoolean(display.text), - warning: Number.parseInt(display.warning, 10), - critical: Number.parseInt(display.critical, 10), - hot: Number.parseInt(display.hot, 10), - max: Number.parseInt(display.max, 10), - locale: display.locale || 'en_US', - }; + const { theme, unit, ...display } = state.display; + const parseIntOrUndef = (v: unknown): number | undefined => { + const n = Number.parseInt(String(v ?? ''), 10); + return Number.isNaN(n) ? undefined : n; + }; + const validTheme = Object.values(ThemeName).includes(theme as ThemeName) + ? (theme as ThemeName) + : undefined; + const validUnit = Object.values(Temperature).includes(unit as Temperature) + ? (unit as Temperature) + : undefined; + return { + ...display, + theme: validTheme, + unit: validUnit, + scale: toBoolean(display.scale), + tabs: toBoolean(display.tabs), + resize: toBoolean(display.resize), + wwn: toBoolean(display.wwn), + total: toBoolean(display.total), + usage: toBoolean(display.usage), + text: toBoolean(display.text), + warning: parseIntOrUndef(display.warning), + critical: parseIntOrUndef(display.critical), + hot: parseIntOrUndef(display.hot), + max: parseIntOrUndef(display.max), + locale: display.locale || 'en_US', + };Follow-up: with this change, the defaults in generateDisplay will kick in as intended:
- warning: config.warning ?? 60
- critical: config.critical ?? 80
- hot: config.hot ?? 90
- max: config.max (now undefined when invalid)
api/src/unraid-api/graph/resolvers/info/devices/devices.service.ts (2)
42-52: Fix inverted GPU 'blacklisted' mapping (and verify filter semantics)
blacklistedis currently set fromentry.allowed, which inverts the intended meaning. Ifallowedmeans "not blacklisted", thenblacklistedshould be!entry.allowed. Also, double-check whether the!device.allowedfilter is intended (return only blacklisted GPUs) or if you meant to return all GPUs and annotate their blacklist status.Apply minimal fix to correct the mapping:
- blacklisted: entry.allowed, + blacklisted: !entry.allowed,Optionally, if you intend to return all GPUs (recommended), drop the
!device.allowedpredicate:- .filter((device) => device.class === 'vga' && !device.allowed) + .filter((device) => device.class === 'vga')
179-186: Prevent runtime error when parsing USB hubs (id may be undefined)If
matchfails,idisundefined, andid.replace('p', ':')will throw. Add a guard and filter nulls.- const usbHubs = await execa('cat /sys/bus/usb/drivers/hub/*/modalias', { shell: true }) - .then(({ stdout }) => - stdout.split('\n').map((line) => { - const [, id] = line.match(/usb:v(\w{9})/) ?? []; - return id.replace('p', ':'); - }) - ) + const usbHubs = await execa('cat /sys/bus/usb/drivers/hub/*/modalias', { shell: true }) + .then(({ stdout }) => + stdout + .split('\n') + .map((line) => { + const [, id] = line.match(/usb:v(\w{9})/) ?? []; + return id ? id.replace('p', ':') : null; + }) + .filter((v): v is string => v !== null) + ) .catch(() => [] as string[]);api/generated-schema.graphql (1)
2310-2326: Add top-level cpuUtilization query for convenienceThe schema currently exposes
Query.metrics.cpu, but not a directQuery.cpuUtilizationas stated in the PR objective. To satisfy client expectations (and keep the change backward-compatible), add this field:type Query { … docker: Docker! disks: [Disk!]! disk(id: PrefixedID!): Disk! rclone: RCloneBackupSettings! info: Info! + """One-time snapshot of CPU utilization""" + cpuUtilization: CpuUtilization settings: Settings! … metrics: Metrics! … }– Confirmed
type Metricsalready defines"""Current CPU utilization metrics""" cpu: CpuUtilization– No existing queries, docs or tests reference
Query.cpuUtilization; safe to add.
🧹 Nitpick comments (35)
api/src/core/pubsub.ts (1)
18-22: Optional: strengthen type-safety by mapping channels to payloadsIf GRAPHQL_PUBSUB_CHANNEL has stable, known payloads per channel, consider overloads or a mapped type (Record<Channel, Payload>) so createSubscription infers T from channel at call sites. This prevents accidental mismatches between channel and payload.
api/src/unraid-api/graph/resolvers/info/display/display.model.ts (1)
80-82: Note: TS aliases don’t create GraphQL schema aliasesRe-exporting InfoDisplay as Display is a TypeScript alias only; it won’t add a “Display” GraphQL type. Ensure all GraphQL clients (schema, codegen, queries) are aligned to InfoDisplay/InfoDisplayCase names.
api/src/unraid-api/graph/resolvers/info/display/display.service.ts (1)
124-129: Minor duplication: id is already provided by states.defaultYou spread states.default (which includes id: 'display/case') and then set id again. Safe but redundant.
- return { - ...states.default, - id: 'display/case', - icon: serverCase, - }; + return { + ...states.default, + icon: serverCase, + };api/src/unraid-api/graph/resolvers/info/memory/memory.model.ts (1)
6-40: MemoryLayout: consider bigint typing for GraphQLBigInt fieldssize is exposed as GraphQLBigInt but typed as number. If you anticipate very large values, consider using bigint (or string per your scalar config) to avoid precision loss.
api/src/unraid-api/graph/resolvers/display/display.resolver.ts (1)
34-36: Remove unnecessary async from subscription method.The method returns the subscription immediately without awaiting; returning a Promise is unnecessary. Prefer returning the AsyncIterable directly.
- public async displaySubscription() { + public displaySubscription() { return createSubscription(PUBSUB_CHANNEL.DISPLAY); }api/src/unraid-api/graph/resolvers/info/devices/devices.resolver.spec.ts (1)
70-71: Potential type mismatch for blacklisted field in test data.'blacklisted' appears as a string ('false') instead of a boolean. If the GraphQL type expects a boolean, this can mask type issues in tests.
If the schema defines this field as boolean, apply:
- blacklisted: 'false', + blacklisted: false,api/src/unraid-api/graph/resolvers/metrics/metrics.module.ts (1)
9-11: Avoid duplicating CpuService/CpuDataService/MemoryService providers across modules.These services are also provided in InfoModule. Registering the same class token in multiple modules produces separate singleton instances, which can fragment caches (especially if CpuDataService is request-scoped in one place and used cross-module). Centralize their provision in a single module and export them for reuse.
Option A (export from InfoModule, consume here):
- @Module({ - imports: [ServicesModule], - providers: [MetricsResolver, CpuService, CpuDataService, MemoryService], - exports: [MetricsResolver], - }) + @Module({ + imports: [ServicesModule, + // Reuse CPU/Memory services from InfoModule + // so the same provider instances are injected app-wide + ] // add InfoModule import after applying the InfoModule diff below + providers: [MetricsResolver], + exports: [MetricsResolver], + })And in MetricsModule header add:
+import { InfoModule } from '@app/unraid-api/graph/resolvers/info/info.module.js';Option B (move CPU/Memory services to ServicesModule) is also viable if you prefer to keep InfoModule light; export them there and import only ServicesModule here.
I can generate full diffs for both modules once you pick an option.
api/src/unraid-api/graph/resolvers/info/info.module.ts (1)
32-33: Export CPU/Memory services to prevent duplicate registrations in other modules.Since MetricsModule also registers CpuService, CpuDataService, and MemoryService, prefer exporting them from one place (e.g., InfoModule) and reusing them to avoid multiple singleton instances and duplicated caches.
Apply this diff to InfoModule:
@Module({ imports: [ConfigModule, ServicesModule], providers: [ // ...omitted for brevity... DisplayService, ], - exports: [InfoResolver, DevicesResolver, DisplayService], + exports: [ + InfoResolver, + DevicesResolver, + DisplayService, + CpuService, + CpuDataService, + MemoryService, + ], }) export class InfoModule {}Then update MetricsModule to import InfoModule and remove local CPU/Memory providers (see the paired suggestion in metrics.module.ts).
api/src/unraid-api/graph/resolvers/info/os/os.service.ts (1)
17-17: Avoid potential runtime crash when store is not initialized; add safe fallback for hostname.If
getters.emhttp()or its nested fields are undefined during early startup or tests, this will throw. Use optional chaining and fall back toos.hostname.- hostname: getters.emhttp().var.name, + hostname: getters.emhttp?.()?.var?.name ?? os.hostname,api/src/unraid-api/graph/resolvers/info/display/display.service.spec.ts (2)
20-29: Restore spies/mocks after each test to prevent cross-test pollution.
vi.clearAllMocks()resets call history but doesn’t restore overridden implementations. Addvi.restoreAllMocks()in anafterEach.-import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; @@ beforeEach(async () => { // Reset all mocks vi.clearAllMocks(); @@ service = module.get<DisplayService>(DisplayService); }); + + afterEach(() => { + vi.restoreAllMocks(); + });
99-119: Tests rely on “dev files” content; consider using controlled fixtures to reduce brittleness.Asserting exact values like theme/unit from dev configs couples tests to non-test assets. Prefer dedicated fixtures or focused behavior assertions so changes to dev files don’t break tests.
api/src/unraid-api/graph/resolvers/info/versions/versions.service.ts (1)
13-13: Prefer null/undefined over sentinel strings for unknown values.Returning 'unknown' conflates “missing” with a concrete value, and makes filtering harder. Use null/undefined to align with GraphQL Maybe semantics.
- const unraid = this.configService.get<string>('store.emhttp.var.version') || 'unknown'; + const unraid = this.configService.get<string>('store.emhttp.var.version') ?? undefined;api/src/unraid-api/graph/resolvers/info/system/system.model.ts (2)
46-50: Make nullability handling consistent for memMax (avoid redundant| null)You already mark the field as
nullable: truein the GraphQL decorator, so?:plus| nullis redundant and inconsistent with the other fields. Prefer a single convention (optional or| null, not both).- @Field(() => Number, { nullable: true, description: 'Maximum memory capacity in bytes' }) - memMax?: number | null; + @Field(() => Number, { nullable: true, description: 'Maximum memory capacity in bytes' }) + memMax?: number;
49-50: Double-check scalar choice for memSlots (Int vs Float)A slot count is an integer conceptually. If the schema allows it, consider using the Int scalar for better semantic accuracy.
If acceptable, this change aligns the TypeScript type and GraphQL scalar:
-import { Field, ObjectType } from '@nestjs/graphql'; +import { Field, ObjectType, Int } from '@nestjs/graphql'; @@ - @Field(() => Number, { nullable: true, description: 'Number of memory slots' }) - memSlots?: number; + @Field(() => Int, { nullable: true, description: 'Number of memory slots' }) + memSlots?: number;Note: The generated CLI types you shared currently expose
FloatformemSlots. If that contract is relied upon, keep as-is to avoid breaking clients.api/src/unraid-api/graph/resolvers/resolvers.module.ts (1)
39-39: Consider removing direct ServicesModule import (if already provided transitively)If both InfoModule and MetricsModule already import ServicesModule (they do), you may not need to import it here as well. Not harmful, but slightly redundant.
api/src/unraid-api/graph/services/subscription-helper.service.ts (1)
25-27: Optionally auto-unsubscribe when iteration ends (done: true) from next()Most GraphQL subscriptions don’t complete naturally, but guarding this path makes the helper more robust and prevents leaks if a producer completes.
- const proxyIterator: AsyncIterableIterator<T> = { - next: () => innerIterator.next(), + const proxyIterator: AsyncIterableIterator<T> = { + next: async () => { + const res = await innerIterator.next(); + if (res?.done) { + // Ensure tracker is cleaned up if iterator completes naturally + this.subscriptionTracker.unsubscribe(topic); + } + return res as IteratorResult<T>; + },api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.spec.ts (4)
123-127: Follow testing guideline: avoid asserting exact error message textUse
.rejects.toThrow()without message arguments to keep tests less brittle.- await expect(resolver.cpu()).rejects.toThrow('CPU error'); + await expect(resolver.cpu()).rejects.toThrow();
151-155: Follow testing guideline: avoid asserting exact error message textSame rationale for memory error path.
- await expect(resolver.memory()).rejects.toThrow('Memory error'); + await expect(resolver.memory()).rejects.toThrow();
71-83: Minimize unnecessary mocks in unit testsSubscriptionTrackerService and SubscriptionHelperService aren’t exercised in this suite. Removing them simplifies the test bed and matches the guideline to avoid over-mocking.
124-124: Type-safety of vi.mocked(...) on service methodsDepending on tsconfig,
vi.mocked(cpuService.generateCpuLoad)andvi.mocked(memoryService.generateMemoryLoad)may need casting. If you see TS friction, prefer casting tovi.Mock:Example:
( cpuService.generateCpuLoad as unknown as vi.Mock ).mockRejectedValueOnce(new Error('CPU error'));If your setup already infers correct mocked types, ignore this.
Also applies to: 152-152
api/src/unraid-api/graph/services/subscription-polling.service.ts (1)
42-45: Improve Logger.error payload to capture stack tracesNest’s Logger.error expects a message and optionally a stack trace. Passing the raw error object can omit useful details.
- } catch (error) { - this.logger.error(`Error in polling task '${name}'`, error); + } catch (error) { + const err = error as Error; + this.logger.error(`Error in polling task '${name}': ${err.message}`, err?.stack);api/src/unraid-api/graph/resolvers/info/memory/memory.service.ts (1)
35-49: Clamp and normalize usedPercent; consider consistent precisionThe usedPercent calculation can exceed bounds in edge cases or produce overly precise floats. Clamp to [0, 100] and optionally normalize precision to reduce noisy diffs/outputs.
- usedPercent: - memInfo.total > 0 ? ((memInfo.total - memInfo.available) / memInfo.total) * 100 : 0, + usedPercent: + memInfo.total > 0 + ? Math.min( + 100, + Math.max( + 0, + Number((((memInfo.total - memInfo.available) / memInfo.total) * 100).toFixed(2)) + ) + ) + : 0,Additionally, verify that the GraphQL BigInt scalar accepts JS numbers for total/used/free on your target platforms. If your scalar expects bigint/string, we should convert accordingly (e.g., BigInt(memInfo.total)).
api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.ts (1)
7-15: Import Systeminformation as a type-only importSysteminformation’s “Systeminformation” is a type. Mark it as a type-only import to avoid any chance of runtime named import emission under ESM.
-import { cpu, cpuFlags, currentLoad, Systeminformation } from 'systeminformation'; +import { cpu, cpuFlags, currentLoad } from 'systeminformation'; +import type { Systeminformation } from 'systeminformation';api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.ts (1)
29-49: Wrap polling callbacks with try/catch to avoid unhandled rejectionsIf generateCpuLoad()/generateMemoryLoad() throws, the unhandled rejection could bubble out of the poller. Log and swallow to keep the polling loop healthy (and to match the previous behavior your tests expected).
this.subscriptionTracker.registerTopic( PUBSUB_CHANNEL.CPU_UTILIZATION, - async () => { - const payload = await this.cpuService.generateCpuLoad(); - pubsub.publish(PUBSUB_CHANNEL.CPU_UTILIZATION, { systemMetricsCpu: payload }); - }, + async () => { + try { + const payload = await this.cpuService.generateCpuLoad(); + await pubsub.publish(PUBSUB_CHANNEL.CPU_UTILIZATION, { systemMetricsCpu: payload }); + } catch (err) { + // Keep polling resilient + console.error('Error polling CPU utilization:', err); + } + }, 1000 ); // Register memory polling with 2 second interval this.subscriptionTracker.registerTopic( PUBSUB_CHANNEL.MEMORY_UTILIZATION, - async () => { - const payload = await this.memoryService.generateMemoryLoad(); - pubsub.publish(PUBSUB_CHANNEL.MEMORY_UTILIZATION, { systemMetricsMemory: payload }); - }, + async () => { + try { + const payload = await this.memoryService.generateMemoryLoad(); + await pubsub.publish(PUBSUB_CHANNEL.MEMORY_UTILIZATION, { systemMetricsMemory: payload }); + } catch (err) { + console.error('Error polling memory utilization:', err); + } + }, 2000 );api/src/unraid-api/graph/resolvers/info/devices/devices.service.ts (3)
67-77: Guard vendorid derivation to avoid crashes on malformed typeidDeriving
vendoridviasubstring(0, 4)will throw iftypeidis undefined/null. While unlikely, a defensive guard here avoids hard-to-diagnose crashes.- vendorid: device.typeid.substring(0, 4), // Extract vendor ID from type ID + vendorid: device.typeid?.slice(0, 4) ?? '', // Extract vendor ID from type ID
55-61: Reduce CI noise: downgrade expected environment failures to warningsIn CI (and non-Unraid/dev machines), PCI/USB discovery often fails and is expected. Logging these as errors clutters CI output. Consider logging at warn level for these code paths.
- this.logger.error( + this.logger.warn( `Failed to generate GPU devices: ${error instanceof Error ? error.message : String(error)}`, error instanceof Error ? error.stack : undefined );- this.logger.error( + this.logger.warn( `Failed to generate PCI devices: ${error instanceof Error ? error.message : String(error)}`, error instanceof Error ? error.stack : undefined );Also applies to: 79-83
87-99: Network stub is fine as a placeholderReturning an empty array with proper error handling is acceptable until implementation lands. Add a TODO with the intended source (e.g., systeminformation) to make next steps clear.
api/src/unraid-api/graph/services/subscription-tracker.service.ts (1)
9-12: Broaden handler types and prefer nullish coalescingAllow async lifecycle handlers in type signatures, and prefer
??over||for the onStop fallback.- private topicHandlers = new Map<string, { onStart: () => void; onStop: () => void }>(); + private topicHandlers = new Map<string, { onStart: () => void | Promise<void>; onStop: () => void | Promise<void> }>(); @@ - onStop: intervalMsOrOnStop || (() => {}), + onStop: intervalMsOrOnStop ?? (() => {}),Also applies to: 37-40
api/src/unraid-api/graph/resolvers/info/info.resolver.ts (2)
53-57: Optional: Avoid casting and explicitly map baseboard fields from systeminformationCasting the entire result can hide shape drift. Explicitly pick fields we expose to the client to make the mapping robust and self-documenting.
Apply this diff:
- public async baseboard(): Promise<InfoBaseboard> { - const baseboard = await getBaseboard(); - return { id: 'info/baseboard', ...baseboard } as InfoBaseboard; - } + public async baseboard(): Promise<InfoBaseboard> { + const { + manufacturer, + model, + version, + serial, + assetTag, + memMax, + memSlots, + } = await getBaseboard(); + return { + id: 'info/baseboard', + manufacturer, + model, + version, + serial, + assetTag, + memMax, + memSlots, + }; + }
90-94: Optional: Same explicit field-pick for system() mappingMirror the baseboard suggestion for system information to avoid broad casts.
Apply this diff:
- public async system(): Promise<InfoSystem> { - const system = await getSystem(); - return { id: 'info/system', ...system } as InfoSystem; - } + public async system(): Promise<InfoSystem> { + const { manufacturer, model, serial, sku, uuid, version, virtual } = await getSystem(); + return { + id: 'info/system', + manufacturer, + model, + serial, + sku, + uuid, + version, + virtual, + }; + }api/generated-schema.graphql (5)
943-959: Doc inconsistency: Port scalar says “TCP” but used for UDP tooContainerPortType includes UDP while Port’s doc references TCP only. Suggest generalizing the description to “valid port (0–65535)”.
Apply this diff to update the description:
-""" -A field whose value is a valid TCP port within the range of 0 to 65535: https://en.wikipedia.org/wiki/Transmission_Control_Protocol#TCP_ports -""" +""" +A field whose value is a valid port number within the range 0 to 65535. +""" scalar Port
1239-1270: Consider clarifying CPU load semantics and add a timestampDocs are clear on per-field percentages, but clients often need:
- Whether “load” ≈ 100 - loadIdle (expected), and whether values are 0–100 bounded.
- A timestamp to correlate/dedupe samples.
Possible schema enhancement:
type CpuLoad { """The total CPU load on a single core, in percent.""" load: Float! + """ISO timestamp when this sample was taken""" + timestamp: DateTime ... }Optionally document that load + loadIdle ~ 100 and values are percentages in [0, 100].
1272-1325: PR objective mentions InfoCpu.utilization, but the field isn’t presentThe PR description says a utilization field is added to InfoCpu, but InfoCpu here only holds static CPU info. If the intent is to expose utilization under Info.cpu.utilization for convenience, add the field (even if Query.metrics.cpu exists).
Apply this diff to add the field:
type InfoCpu implements Node { id: PrefixedID! ... """CPU feature flags""" flags: [String!] + + """Realtime CPU utilization snapshot for this CPU""" + utilization: CpuUtilization }If intentionally omitted (preferring Query.metrics), consider updating the PR description and any docs/tests expecting InfoCpu.utilization.
1660-1669: Nullability of Metrics fieldsMetrics implements Node and returns cpu/memory as nullable. If these are always resolvable on supported platforms, prefer non-null to tighten the contract. If they can legitimately be unavailable, keep as nullable but document conditions (e.g., unsupported platform/permission issues).
2565-2576: Subscription naming consistency and alias for PR objectiveSubscriptions expose systemMetricsCpu/systemMetricsMemory, while Query uses metrics. For symmetry or to match the PR’s “cpuUtilizationSubscription” goal, consider exposing additional alias fields (keep existing for compatibility):
type Subscription { ... - systemMetricsCpu: CpuUtilization! - systemMetricsMemory: MemoryUtilization! + systemMetricsCpu: CpuUtilization! + systemMetricsMemory: MemoryUtilization! + """Realtime CPU utilization updates""" + cpuUtilization: CpuUtilization! + """Realtime memory utilization updates""" + memoryUtilization: MemoryUtilization! }Optionally deprecate the “systemMetrics*” names later, but keep them now to avoid breaking clients.
        
          
                api/src/unraid-api/graph/resolvers/info/info.resolver.integration.spec.ts
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                api/src/unraid-api/graph/resolvers/info/info.resolver.integration.spec.ts
          
            Show resolved
            Hide resolved
        
              
          
                api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.integration.spec.ts
          
            Show resolved
            Hide resolved
        
              
          
                api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.integration.spec.ts
          
            Show resolved
            Hide resolved
        
              
          
                api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.integration.spec.ts
          
            Show resolved
            Hide resolved
        
              
          
                api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.integration.spec.ts
          
            Show resolved
            Hide resolved
        
              
          
                api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.integration.spec.ts
          
            Show resolved
            Hide resolved
        
      - Added `ScheduleModule` to the main application module for managing scheduled tasks. - Removed redundant `ScheduleModule` imports from `CronModule` and `ServicesModule` to streamline module dependencies.
- Added a null check for the `module` before calling `close()` in the `afterEach` hook to prevent potential errors during test teardown.
- Renamed CPU and memory metric fields for consistency, changing `load` to `percentTotal`, `loadUser` to `percentUser`, and similar adjustments for other fields. - Updated integration tests to reflect the new naming conventions, ensuring accurate property checks for CPU and memory utilization. - Enhanced the `CpuService` and `MemoryService` to return the updated metric names, improving clarity in the API response.
- Renamed CPU and memory metric fields for consistency, changing `load` to `percentTotal`, `loadUser` to `percentUser`, and similar adjustments for other fields across the GraphQL schema and resolvers. - Updated integration tests to reflect the new naming conventions, ensuring accurate property checks for CPU and memory utilization. - Enhanced the `CpuService` and `MemoryService` to return the updated metric names, improving clarity in the API response.
- Renamed `percentSwapUsed` to `percentSwapTotal` and `percentUsed` to `percentTotal` in the GraphQL schema to align with recent naming conventions. - Adjusted export statements in the index file for consistency in quotation style.
- Introduced `isValidEnumValue` and `validateEnumValue` functions to validate enum values, enhancing type safety in the application. - Updated `DisplayService` to utilize `validateEnumValue` for theme and unit properties, ensuring only valid enum values are assigned.
- Eliminated the `CpuDataService` from the `info.module.ts`, `metrics.module.ts`, and related integration tests, streamlining the CPU service implementation. - Updated imports and provider lists to reflect the removal, ensuring the application remains functional without the redundant service.
| This plugin has been deployed to Cloudflare R2 and is available for testing.  | 
Adds a new query and subscription to the info resolver for CPU to get CPU utilization on Unraid.
cpuUtilizationquery to get a one-time snapshot of CPU load.cpuUtilizationSubscriptionto get real-time updates on CPU load.utilizationfield to theInfoCputype.SubscriptionTrackerServiceto manage polling for subscriptions, ensuring that polling only occurs when there are active subscribers.CpuDataServiceto cache CPU load data within a single GraphQL request to improve performance.CpuLoadobject type.Summary by CodeRabbit
New Features
Schema
Chores
Tests