From 8c984b5d769c8b2f272984d11984243018ddbb53 Mon Sep 17 00:00:00 2001 From: Pujit Mehrotra Date: Thu, 8 Jan 2026 14:07:28 -0500 Subject: [PATCH 1/7] fix(docker): rm premature caching to ensure correctness --- .../docker/docker-event.service.spec.ts | 8 -- .../resolvers/docker/docker-event.service.ts | 4 +- .../docker/docker-network.service.spec.ts | 41 ++----- .../docker/docker-network.service.ts | 33 +----- .../docker/docker-template-scanner.service.ts | 2 +- .../resolvers/docker/docker.module.spec.ts | 2 +- .../resolvers/docker/docker.resolver.spec.ts | 18 +-- .../graph/resolvers/docker/docker.resolver.ts | 40 +++++-- .../docker/docker.service.integration.spec.ts | 16 +-- .../resolvers/docker/docker.service.spec.ts | 45 ++++---- .../graph/resolvers/docker/docker.service.ts | 104 ++++-------------- .../organizer/docker-organizer.service.ts | 21 +--- 12 files changed, 102 insertions(+), 232 deletions(-) diff --git a/api/src/unraid-api/graph/resolvers/docker/docker-event.service.spec.ts b/api/src/unraid-api/graph/resolvers/docker/docker-event.service.spec.ts index 83314b910b..431f69cb11 100644 --- a/api/src/unraid-api/graph/resolvers/docker/docker-event.service.spec.ts +++ b/api/src/unraid-api/graph/resolvers/docker/docker-event.service.spec.ts @@ -61,7 +61,6 @@ vi.mock('./utils/docker-client.js', () => ({ vi.mock('./docker.service.js', () => ({ DockerService: vi.fn().mockImplementation(() => ({ getDockerClient: vi.fn(), - clearContainerCache: vi.fn(), getAppInfo: vi.fn().mockResolvedValue({ info: { apps: { installed: 1, running: 1 } } }), })), })); @@ -76,7 +75,6 @@ describe('DockerEventService', () => { // Create a mock Docker service *instance* const mockDockerServiceImpl = { getDockerClient: vi.fn(), - clearContainerCache: vi.fn(), getAppInfo: vi.fn().mockResolvedValue({ info: { apps: { installed: 1, running: 1 } } }), }; @@ -132,7 +130,6 @@ describe('DockerEventService', () => { await waitForEventProcessing(); - expect(dockerService.clearContainerCache).toHaveBeenCalled(); expect(dockerService.getAppInfo).toHaveBeenCalled(); expect(pubsub.publish).toHaveBeenCalledWith(PUBSUB_CHANNEL.INFO, expect.any(Object)); }); @@ -154,7 +151,6 @@ describe('DockerEventService', () => { await waitForEventProcessing(); - expect(dockerService.clearContainerCache).not.toHaveBeenCalled(); expect(dockerService.getAppInfo).not.toHaveBeenCalled(); expect(pubsub.publish).not.toHaveBeenCalled(); }); @@ -172,7 +168,6 @@ describe('DockerEventService', () => { await waitForEventProcessing(); expect(service.isActive()).toBe(true); - expect(dockerService.clearContainerCache).toHaveBeenCalledTimes(1); expect(dockerService.getAppInfo).toHaveBeenCalledTimes(1); expect(pubsub.publish).toHaveBeenCalledTimes(1); }); @@ -190,7 +185,6 @@ describe('DockerEventService', () => { await waitForEventProcessing(); - expect(dockerService.clearContainerCache).toHaveBeenCalledTimes(2); expect(dockerService.getAppInfo).toHaveBeenCalledTimes(2); expect(pubsub.publish).toHaveBeenCalledTimes(2); }); @@ -206,7 +200,6 @@ describe('DockerEventService', () => { await waitForEventProcessing(); - expect(dockerService.clearContainerCache).toHaveBeenCalledTimes(1); expect(dockerService.getAppInfo).toHaveBeenCalledTimes(1); expect(pubsub.publish).toHaveBeenCalledTimes(1); @@ -223,7 +216,6 @@ describe('DockerEventService', () => { await waitForEventProcessing(); - expect(dockerService.clearContainerCache).toHaveBeenCalledTimes(1); expect(dockerService.getAppInfo).toHaveBeenCalledTimes(1); expect(pubsub.publish).toHaveBeenCalledTimes(1); diff --git a/api/src/unraid-api/graph/resolvers/docker/docker-event.service.ts b/api/src/unraid-api/graph/resolvers/docker/docker-event.service.ts index 63fdd0d482..8cd8158812 100644 --- a/api/src/unraid-api/graph/resolvers/docker/docker-event.service.ts +++ b/api/src/unraid-api/graph/resolvers/docker/docker-event.service.ts @@ -124,14 +124,12 @@ export class DockerEventService implements OnModuleDestroy, OnModuleInit { if (shouldProcess) { this.logger.debug(`[${dockerEvent.from}] ${dockerEvent.Type}->${actionName}`); - // For container lifecycle events, update the container cache + // For container lifecycle events, publish updated app info if ( dockerEvent.Type === DockerEventType.CONTAINER && typeof actionName === 'string' && this.containerActions.includes(actionName as DockerEventAction) ) { - await this.dockerService.clearContainerCache(); - // Get updated counts and publish const appInfo = await this.dockerService.getAppInfo(); await pubsub.publish(PUBSUB_CHANNEL.INFO, appInfo); this.logger.debug(`Published app info update due to event: ${actionName}`); diff --git a/api/src/unraid-api/graph/resolvers/docker/docker-network.service.spec.ts b/api/src/unraid-api/graph/resolvers/docker/docker-network.service.spec.ts index ca29501437..fed869e5dd 100644 --- a/api/src/unraid-api/graph/resolvers/docker/docker-network.service.spec.ts +++ b/api/src/unraid-api/graph/resolvers/docker/docker-network.service.spec.ts @@ -1,4 +1,3 @@ -import { CACHE_MANAGER } from '@nestjs/cache-manager'; import { Test, TestingModule } from '@nestjs/testing'; import { beforeEach, describe, expect, it, vi } from 'vitest'; @@ -17,27 +16,14 @@ vi.mock('@app/unraid-api/graph/resolvers/docker/utils/docker-client.js', () => ( getDockerClient: vi.fn().mockReturnValue(mockDockerInstance), })); -const mockCacheManager = { - get: vi.fn(), - set: vi.fn(), -}; - describe('DockerNetworkService', () => { let service: DockerNetworkService; beforeEach(async () => { mockListNetworks.mockReset(); - mockCacheManager.get.mockReset(); - mockCacheManager.set.mockReset(); const module: TestingModule = await Test.createTestingModule({ - providers: [ - DockerNetworkService, - { - provide: CACHE_MANAGER, - useValue: mockCacheManager, - }, - ], + providers: [DockerNetworkService], }).compile(); service = module.get(DockerNetworkService); @@ -48,16 +34,7 @@ describe('DockerNetworkService', () => { }); describe('getNetworks', () => { - it('should return cached networks if available and not skipped', async () => { - const cached = [{ id: 'net1', name: 'test-net' }]; - mockCacheManager.get.mockResolvedValue(cached); - - const result = await service.getNetworks({ skipCache: false }); - expect(result).toEqual(cached); - expect(mockListNetworks).not.toHaveBeenCalled(); - }); - - it('should fetch networks from docker if cache skipped', async () => { + it('should fetch networks from docker', async () => { const rawNetworks = [ { Id: 'net1', @@ -67,22 +44,18 @@ describe('DockerNetworkService', () => { ]; mockListNetworks.mockResolvedValue(rawNetworks); - const result = await service.getNetworks({ skipCache: true }); + const result = await service.getNetworks(); expect(result).toHaveLength(1); expect(result[0].id).toBe('net1'); + expect(result[0].name).toBe('test-net'); expect(mockListNetworks).toHaveBeenCalled(); - expect(mockCacheManager.set).toHaveBeenCalledWith( - DockerNetworkService.NETWORK_CACHE_KEY, - expect.anything(), - expect.anything() - ); }); - it('should fetch networks from docker if cache miss', async () => { - mockCacheManager.get.mockResolvedValue(undefined); + it('should return empty array when no networks exist', async () => { mockListNetworks.mockResolvedValue([]); - await service.getNetworks({ skipCache: false }); + const result = await service.getNetworks(); + expect(result).toEqual([]); expect(mockListNetworks).toHaveBeenCalled(); }); }); diff --git a/api/src/unraid-api/graph/resolvers/docker/docker-network.service.ts b/api/src/unraid-api/graph/resolvers/docker/docker-network.service.ts index 19ddf80172..2b780e8155 100644 --- a/api/src/unraid-api/graph/resolvers/docker/docker-network.service.ts +++ b/api/src/unraid-api/graph/resolvers/docker/docker-network.service.ts @@ -1,42 +1,20 @@ -import { CACHE_MANAGER } from '@nestjs/cache-manager'; -import { Inject, Injectable, Logger } from '@nestjs/common'; - -import { type Cache } from 'cache-manager'; +import { Injectable, Logger } from '@nestjs/common'; import { catchHandlers } from '@app/core/utils/misc/catch-handlers.js'; import { DockerNetwork } from '@app/unraid-api/graph/resolvers/docker/docker.model.js'; import { getDockerClient } from '@app/unraid-api/graph/resolvers/docker/utils/docker-client.js'; -interface NetworkListingOptions { - skipCache: boolean; -} - @Injectable() export class DockerNetworkService { private readonly logger = new Logger(DockerNetworkService.name); private readonly client = getDockerClient(); - public static readonly NETWORK_CACHE_KEY = 'docker_networks'; - private static readonly CACHE_TTL_SECONDS = 60; - - constructor(@Inject(CACHE_MANAGER) private cacheManager: Cache) {} - /** * Get all Docker networks * @returns All the in/active Docker networks on the system. */ - public async getNetworks({ skipCache }: NetworkListingOptions): Promise { - if (!skipCache) { - const cachedNetworks = await this.cacheManager.get( - DockerNetworkService.NETWORK_CACHE_KEY - ); - if (cachedNetworks) { - this.logger.debug('Using docker network cache'); - return cachedNetworks; - } - } - - this.logger.debug('Updating docker network cache'); + public async getNetworks(): Promise { + this.logger.debug('Fetching docker networks'); const rawNetworks = await this.client.listNetworks().catch(catchHandlers.docker); const networks = rawNetworks.map( (network) => @@ -59,11 +37,6 @@ export class DockerNetworkService { }) as DockerNetwork ); - await this.cacheManager.set( - DockerNetworkService.NETWORK_CACHE_KEY, - networks, - DockerNetworkService.CACHE_TTL_SECONDS * 1000 - ); return networks; } } diff --git a/api/src/unraid-api/graph/resolvers/docker/docker-template-scanner.service.ts b/api/src/unraid-api/graph/resolvers/docker/docker-template-scanner.service.ts index 0d68810724..a3434077c7 100644 --- a/api/src/unraid-api/graph/resolvers/docker/docker-template-scanner.service.ts +++ b/api/src/unraid-api/graph/resolvers/docker/docker-template-scanner.service.ts @@ -87,7 +87,7 @@ export class DockerTemplateScannerService { const templates = await this.loadAllTemplates(result); try { - const containers = await this.dockerService.getContainers({ skipCache: true }); + const containers = await this.dockerService.getContainers(); const config = this.dockerConfigService.getConfig(); const currentMappings = config.templateMappings || {}; const skipSet = new Set(config.skipTemplatePaths || []); diff --git a/api/src/unraid-api/graph/resolvers/docker/docker.module.spec.ts b/api/src/unraid-api/graph/resolvers/docker/docker.module.spec.ts index a4e41cc9e4..0454a376a6 100644 --- a/api/src/unraid-api/graph/resolvers/docker/docker.module.spec.ts +++ b/api/src/unraid-api/graph/resolvers/docker/docker.module.spec.ts @@ -73,7 +73,7 @@ describe('DockerModule', () => { const module: TestingModule = await Test.createTestingModule({ providers: [ DockerResolver, - { provide: DockerService, useValue: { clearContainerCache: vi.fn() } }, + { provide: DockerService, useValue: {} }, { provide: DockerConfigService, useValue: { diff --git a/api/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.ts b/api/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.ts index 7b6826a8a5..5109fecab8 100644 --- a/api/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.ts +++ b/api/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.ts @@ -40,7 +40,6 @@ describe('DockerResolver', () => { getNetworks: vi.fn(), getContainerLogSizes: vi.fn(), getContainerLogs: vi.fn(), - clearContainerCache: vi.fn(), }, }, { @@ -161,7 +160,7 @@ describe('DockerResolver', () => { expect(GraphQLFieldHelper.isFieldRequested).toHaveBeenCalledWith(mockInfo, 'sizeRootFs'); expect(GraphQLFieldHelper.isFieldRequested).toHaveBeenCalledWith(mockInfo, 'sizeRw'); expect(GraphQLFieldHelper.isFieldRequested).toHaveBeenCalledWith(mockInfo, 'sizeLog'); - expect(dockerService.getContainers).toHaveBeenCalledWith({ skipCache: false, size: false }); + expect(dockerService.getContainers).toHaveBeenCalledWith({ size: false }); }); it('should request size when sizeRootFs field is requested', async () => { @@ -191,7 +190,7 @@ describe('DockerResolver', () => { const result = await resolver.containers(false, mockInfo); expect(result).toEqual(mockContainers); expect(GraphQLFieldHelper.isFieldRequested).toHaveBeenCalledWith(mockInfo, 'sizeRootFs'); - expect(dockerService.getContainers).toHaveBeenCalledWith({ skipCache: false, size: true }); + expect(dockerService.getContainers).toHaveBeenCalledWith({ size: true }); }); it('should request size when sizeRw field is requested', async () => { @@ -205,7 +204,7 @@ describe('DockerResolver', () => { await resolver.containers(false, mockInfo); expect(GraphQLFieldHelper.isFieldRequested).toHaveBeenCalledWith(mockInfo, 'sizeRw'); - expect(dockerService.getContainers).toHaveBeenCalledWith({ skipCache: false, size: true }); + expect(dockerService.getContainers).toHaveBeenCalledWith({ size: true }); }); it('should fetch log sizes when sizeLog field is requested', async () => { @@ -240,7 +239,7 @@ describe('DockerResolver', () => { expect(GraphQLFieldHelper.isFieldRequested).toHaveBeenCalledWith(mockInfo, 'sizeLog'); expect(dockerService.getContainerLogSizes).toHaveBeenCalledWith(['test-container']); expect(result[0]?.sizeLog).toBe(42); - expect(dockerService.getContainers).toHaveBeenCalledWith({ skipCache: false, size: false }); + expect(dockerService.getContainers).toHaveBeenCalledWith({ size: false }); }); it('should request size when GraphQLFieldHelper indicates sizeRootFs is requested', async () => { @@ -254,7 +253,7 @@ describe('DockerResolver', () => { await resolver.containers(false, mockInfo); expect(GraphQLFieldHelper.isFieldRequested).toHaveBeenCalledWith(mockInfo, 'sizeRootFs'); - expect(dockerService.getContainers).toHaveBeenCalledWith({ skipCache: false, size: true }); + expect(dockerService.getContainers).toHaveBeenCalledWith({ size: true }); }); it('should not request size when GraphQLFieldHelper indicates sizeRootFs is not requested', async () => { @@ -266,18 +265,19 @@ describe('DockerResolver', () => { await resolver.containers(false, mockInfo); expect(GraphQLFieldHelper.isFieldRequested).toHaveBeenCalledWith(mockInfo, 'sizeRootFs'); - expect(dockerService.getContainers).toHaveBeenCalledWith({ skipCache: false, size: false }); + expect(dockerService.getContainers).toHaveBeenCalledWith({ size: false }); }); - it('should handle skipCache parameter', async () => { + it('skipCache parameter is deprecated and ignored', async () => { const mockContainers: DockerContainer[] = []; vi.mocked(dockerService.getContainers).mockResolvedValue(mockContainers); vi.mocked(GraphQLFieldHelper.isFieldRequested).mockReturnValue(false); const mockInfo = {} as any; + // skipCache parameter is now deprecated and ignored await resolver.containers(true, mockInfo); - expect(dockerService.getContainers).toHaveBeenCalledWith({ skipCache: true, size: false }); + expect(dockerService.getContainers).toHaveBeenCalledWith({ size: false }); }); it('should fetch container logs with provided arguments', async () => { diff --git a/api/src/unraid-api/graph/resolvers/docker/docker.resolver.ts b/api/src/unraid-api/graph/resolvers/docker/docker.resolver.ts index 2048dc6217..c03ad15462 100644 --- a/api/src/unraid-api/graph/resolvers/docker/docker.resolver.ts +++ b/api/src/unraid-api/graph/resolvers/docker/docker.resolver.ts @@ -77,7 +77,7 @@ export class DockerResolver { }) @ResolveField(() => DockerContainer, { nullable: true }) public async container(@Args('id', { type: () => PrefixedID }) id: string) { - const containers = await this.dockerService.getContainers({ skipCache: false }); + const containers = await this.dockerService.getContainers(); return containers.find((c) => c.id === id) ?? null; } @@ -87,14 +87,18 @@ export class DockerResolver { }) @ResolveField(() => [DockerContainer]) public async containers( - @Args('skipCache', { defaultValue: false, type: () => Boolean }) skipCache: boolean, + @Args('skipCache', { + defaultValue: false, + type: () => Boolean, + deprecationReason: 'Caching has been removed; this parameter is now ignored', + }) + _skipCache: boolean, @Info() info: GraphQLResolveInfo ) { const requestsRootFsSize = GraphQLFieldHelper.isFieldRequested(info, 'sizeRootFs'); const requestsRwSize = GraphQLFieldHelper.isFieldRequested(info, 'sizeRw'); const requestsLogSize = GraphQLFieldHelper.isFieldRequested(info, 'sizeLog'); const containers = await this.dockerService.getContainers({ - skipCache, size: requestsRootFsSize || requestsRwSize, }); @@ -114,7 +118,7 @@ export class DockerResolver { } const wasSynced = await this.dockerTemplateScannerService.syncMissingContainers(containers); - return wasSynced ? await this.dockerService.getContainers({ skipCache: true }) : containers; + return wasSynced ? await this.dockerService.getContainers() : containers; } @UsePermissions({ @@ -139,9 +143,14 @@ export class DockerResolver { }) @ResolveField(() => [DockerNetwork]) public async networks( - @Args('skipCache', { defaultValue: false, type: () => Boolean }) skipCache: boolean + @Args('skipCache', { + defaultValue: false, + type: () => Boolean, + deprecationReason: 'Caching has been removed; this parameter is now ignored', + }) + _skipCache: boolean ) { - return this.dockerService.getNetworks({ skipCache }); + return this.dockerService.getNetworks(); } @UsePermissions({ @@ -150,9 +159,14 @@ export class DockerResolver { }) @ResolveField(() => DockerPortConflicts) public async portConflicts( - @Args('skipCache', { defaultValue: false, type: () => Boolean }) skipCache: boolean + @Args('skipCache', { + defaultValue: false, + type: () => Boolean, + deprecationReason: 'Caching has been removed; this parameter is now ignored', + }) + _skipCache: boolean ) { - return this.dockerService.getPortConflicts({ skipCache }); + return this.dockerService.getPortConflicts(); } @UseFeatureFlag('ENABLE_NEXT_DOCKER_RELEASE') @@ -162,9 +176,14 @@ export class DockerResolver { }) @ResolveField(() => ResolvedOrganizerV1) public async organizer( - @Args('skipCache', { defaultValue: false, type: () => Boolean }) skipCache: boolean + @Args('skipCache', { + defaultValue: false, + type: () => Boolean, + deprecationReason: 'Caching has been removed; this parameter is now ignored', + }) + _skipCache: boolean ) { - return this.dockerOrganizerService.resolveOrganizer(undefined, { skipCache }); + return this.dockerOrganizerService.resolveOrganizer(); } @UseFeatureFlag('ENABLE_NEXT_DOCKER_RELEASE') @@ -351,7 +370,6 @@ export class DockerResolver { }; const validated = await this.dockerConfigService.validate(resetConfig); this.dockerConfigService.replaceConfig(validated); - await this.dockerService.clearContainerCache(); return true; } diff --git a/api/src/unraid-api/graph/resolvers/docker/docker.service.integration.spec.ts b/api/src/unraid-api/graph/resolvers/docker/docker.service.integration.spec.ts index 0f9bef965e..942e0a204b 100644 --- a/api/src/unraid-api/graph/resolvers/docker/docker.service.integration.spec.ts +++ b/api/src/unraid-api/graph/resolvers/docker/docker.service.integration.spec.ts @@ -1,4 +1,3 @@ -import { CACHE_MANAGER } from '@nestjs/cache-manager'; import { Test, TestingModule } from '@nestjs/testing'; import { mkdtemp, readFile, rm } from 'fs/promises'; import { tmpdir } from 'os'; @@ -29,12 +28,6 @@ const mockDockerManifestService = { isUpdateAvailableCached: vi.fn().mockResolvedValue(false), }; -const mockCacheManager = { - get: vi.fn(), - set: vi.fn(), - del: vi.fn(), -}; - // Hoisted mock for paths const { mockPaths } = vi.hoisted(() => ({ mockPaths: { @@ -81,7 +74,6 @@ describe.runIf(dockerAvailable)('DockerService Integration', () => { DockerLogService, DockerNetworkService, DockerPortService, - { provide: CACHE_MANAGER, useValue: mockCacheManager }, { provide: DockerConfigService, useValue: mockDockerConfigService }, { provide: DockerManifestService, useValue: mockDockerManifestService }, { provide: NotificationsService, useValue: mockNotificationsService }, @@ -99,7 +91,7 @@ describe.runIf(dockerAvailable)('DockerService Integration', () => { }); it('should fetch containers from docker daemon', async () => { - const containers = await service.getContainers({ skipCache: true }); + const containers = await service.getContainers(); expect(Array.isArray(containers)).toBe(true); if (containers.length > 0) { expect(containers[0]).toHaveProperty('id'); @@ -109,7 +101,7 @@ describe.runIf(dockerAvailable)('DockerService Integration', () => { }); it('should fetch networks from docker daemon', async () => { - const networks = await service.getNetworks({ skipCache: true }); + const networks = await service.getNetworks(); expect(Array.isArray(networks)).toBe(true); // Default networks (bridge, host, null) should always exist expect(networks.length).toBeGreaterThan(0); @@ -118,7 +110,7 @@ describe.runIf(dockerAvailable)('DockerService Integration', () => { }); it('should manage autostart configuration in temp files', async () => { - const containers = await service.getContainers({ skipCache: true }); + const containers = await service.getContainers(); if (containers.length === 0) { console.warn('No containers found, skipping autostart write test'); return; @@ -150,7 +142,7 @@ describe.runIf(dockerAvailable)('DockerService Integration', () => { }); it('should get container logs using dockerode', async () => { - const containers = await service.getContainers({ skipCache: true }); + const containers = await service.getContainers(); const running = containers.find((c) => c.state === 'RUNNING'); // Enum value is string 'RUNNING' if (!running) { diff --git a/api/src/unraid-api/graph/resolvers/docker/docker.service.spec.ts b/api/src/unraid-api/graph/resolvers/docker/docker.service.spec.ts index 445dc05a6b..48994aee16 100644 --- a/api/src/unraid-api/graph/resolvers/docker/docker.service.spec.ts +++ b/api/src/unraid-api/graph/resolvers/docker/docker.service.spec.ts @@ -1,5 +1,4 @@ import type { TestingModule } from '@nestjs/testing'; -import { CACHE_MANAGER } from '@nestjs/cache-manager'; import { Test } from '@nestjs/testing'; import Docker from 'dockerode'; @@ -107,13 +106,6 @@ vi.mock('fs/promises', () => ({ stat: statMock, })); -// Mock Cache Manager -const mockCacheManager = { - get: vi.fn(), - set: vi.fn(), - del: vi.fn(), -}; - // Mock DockerConfigService const mockDockerConfigService = { getConfig: vi.fn().mockReturnValue({ @@ -188,9 +180,6 @@ describe('DockerService', () => { mockContainer.unpause.mockReset(); mockContainer.inspect.mockReset(); - mockCacheManager.get.mockReset(); - mockCacheManager.set.mockReset(); - mockCacheManager.del.mockReset(); statMock.mockReset(); statMock.mockResolvedValue({ size: 0 }); @@ -223,10 +212,6 @@ describe('DockerService', () => { const module: TestingModule = await Test.createTestingModule({ providers: [ DockerService, - { - provide: CACHE_MANAGER, - useValue: mockCacheManager, - }, { provide: DockerConfigService, useValue: mockDockerConfigService, @@ -287,9 +272,8 @@ describe('DockerService', () => { ]; mockListContainers.mockResolvedValue(mockContainers); - mockCacheManager.get.mockResolvedValue(undefined); - const result = await service.getContainers({ skipCache: true }); + const result = await service.getContainers(); expect(result).toEqual( expect.arrayContaining([ @@ -322,7 +306,6 @@ describe('DockerService', () => { expect.any(Array), { persistUserPreferences: true } ); - expect(mockCacheManager.del).toHaveBeenCalledWith(DockerService.CONTAINER_CACHE_KEY); }); it('should delegate getContainerLogSizes to DockerLogService', async () => { @@ -332,13 +315,25 @@ describe('DockerService', () => { }); describe('getAppInfo', () => { - const mockContainersForMethods = [ - { id: 'abc1', state: ContainerState.RUNNING }, - { id: 'def2', state: ContainerState.EXITED }, - ] as DockerContainer[]; - it('should return correct app info object', async () => { - mockCacheManager.get.mockResolvedValue(mockContainersForMethods); + mockListContainers.mockResolvedValue([ + { + Id: 'abc1', + Names: ['/test1'], + State: 'running', + Ports: [], + Labels: {}, + HostConfig: {}, + }, + { + Id: 'def2', + Names: ['/test2'], + State: 'exited', + Ports: [], + Labels: {}, + HostConfig: {}, + }, + ]); const result = await service.getAppInfo(); expect(result).toEqual({ @@ -346,7 +341,7 @@ describe('DockerService', () => { apps: { installed: 2, running: 1 }, }, }); - expect(mockCacheManager.get).toHaveBeenCalledWith(DockerService.CONTAINER_CACHE_KEY); + expect(mockListContainers).toHaveBeenCalled(); }); }); diff --git a/api/src/unraid-api/graph/resolvers/docker/docker.service.ts b/api/src/unraid-api/graph/resolvers/docker/docker.service.ts index 87374db4bc..55acc78709 100644 --- a/api/src/unraid-api/graph/resolvers/docker/docker.service.ts +++ b/api/src/unraid-api/graph/resolvers/docker/docker.service.ts @@ -1,7 +1,5 @@ -import { CACHE_MANAGER } from '@nestjs/cache-manager'; -import { Inject, Injectable, Logger } from '@nestjs/common'; +import { Injectable, Logger } from '@nestjs/common'; -import { type Cache } from 'cache-manager'; import Docker from 'dockerode'; import { execa } from 'execa'; @@ -26,26 +24,12 @@ import { } from '@app/unraid-api/graph/resolvers/docker/docker.model.js'; import { getDockerClient } from '@app/unraid-api/graph/resolvers/docker/utils/docker-client.js'; -interface ContainerListingOptions extends Docker.ContainerListOptions { - skipCache: boolean; -} - -interface NetworkListingOptions { - skipCache: boolean; -} - @Injectable() export class DockerService { private client: Docker; private readonly logger = new Logger(DockerService.name); - public static readonly CONTAINER_CACHE_KEY = 'docker_containers'; - public static readonly CONTAINER_WITH_SIZE_CACHE_KEY = 'docker_containers_with_size'; - public static readonly NETWORK_CACHE_KEY = 'docker_networks'; - public static readonly CACHE_TTL_SECONDS = 60; - constructor( - @Inject(CACHE_MANAGER) private cacheManager: Cache, private readonly dockerConfigService: DockerConfigService, private readonly dockerManifestService: DockerManifestService, private readonly autostartService: DockerAutostartService, @@ -57,7 +41,7 @@ export class DockerService { } public async getAppInfo() { - const containers = await this.getContainers({ skipCache: false }); + const containers = await this.getContainers(); const installedCount = containers.length; const runningCount = containers.filter( (container) => container.state === ContainerState.RUNNING @@ -141,27 +125,12 @@ export class DockerService { return transformed; } - public async getContainers( - { - skipCache = false, - all = true, - size = false, - ...listOptions - }: Partial = { skipCache: false } - ): Promise { - const cacheKey = size - ? DockerService.CONTAINER_WITH_SIZE_CACHE_KEY - : DockerService.CONTAINER_CACHE_KEY; - - if (!skipCache) { - const cachedContainers = await this.cacheManager.get(cacheKey); - if (cachedContainers) { - this.logger.debug(`Using docker container cache (${size ? 'with' : 'without'} size)`); - return cachedContainers; - } - } - - this.logger.debug(`Updating docker container cache (${size ? 'with' : 'without'} size)`); + public async getContainers({ + all = true, + size = false, + ...listOptions + }: Partial = {}): Promise { + this.logger.debug(`Fetching docker containers (${size ? 'with' : 'without'} size)`); let rawContainers: Docker.ContainerInfo[] = []; try { rawContainers = await this.client.listContainers({ @@ -187,20 +156,11 @@ export class DockerService { }; }); - await this.cacheManager.set( - cacheKey, - containersWithTemplatePaths, - DockerService.CACHE_TTL_SECONDS * 1000 - ); return containersWithTemplatePaths; } - public async getPortConflicts({ - skipCache = false, - }: { - skipCache?: boolean; - } = {}): Promise { - const containers = await this.getContainers({ skipCache }); + public async getPortConflicts(): Promise { + const containers = await this.getContainers(); return this.dockerPortService.calculateConflicts(containers); } @@ -219,24 +179,14 @@ export class DockerService { * Get all Docker networks * @returns All the in/active Docker networks on the system. */ - public async getNetworks(options: NetworkListingOptions): Promise { - return this.dockerNetworkService.getNetworks(options); - } - - public async clearContainerCache(): Promise { - await Promise.all([ - this.cacheManager.del(DockerService.CONTAINER_CACHE_KEY), - this.cacheManager.del(DockerService.CONTAINER_WITH_SIZE_CACHE_KEY), - ]); - this.logger.debug('Invalidated container caches due to external event.'); + public async getNetworks(): Promise { + return this.dockerNetworkService.getNetworks(); } public async start(id: string): Promise { const container = this.client.getContainer(id); await container.start(); - await this.clearContainerCache(); - this.logger.debug(`Invalidated container caches after starting ${id}`); - const containers = await this.getContainers({ skipCache: true }); + const containers = await this.getContainers(); const updatedContainer = containers.find((c) => c.id === id); if (!updatedContainer) { throw new Error(`Container ${id} not found after starting`); @@ -265,8 +215,6 @@ export class DockerService { } } - await this.clearContainerCache(); - this.logger.debug(`Invalidated container caches after removing ${id}`); const appInfo = await this.getAppInfo(); await pubsub.publish(PUBSUB_CHANNEL.INFO, appInfo); return true; @@ -280,22 +228,19 @@ export class DockerService { entries: DockerAutostartEntryInput[], options?: { persistUserPreferences?: boolean } ): Promise { - const containers = await this.getContainers({ skipCache: true }); + const containers = await this.getContainers(); await this.autostartService.updateAutostartConfiguration(entries, containers, options); - await this.clearContainerCache(); } public async stop(id: string): Promise { const container = this.client.getContainer(id); await container.stop({ t: 10 }); - await this.clearContainerCache(); - this.logger.debug(`Invalidated container caches after stopping ${id}`); - let containers = await this.getContainers({ skipCache: true }); + let containers = await this.getContainers(); let updatedContainer: DockerContainer | undefined; for (let i = 0; i < 5; i++) { await sleep(500); - containers = await this.getContainers({ skipCache: true }); + containers = await this.getContainers(); updatedContainer = containers.find((c) => c.id === id); this.logger.debug( `Container ${id} state after stop attempt ${i + 1}: ${updatedContainer?.state}` @@ -318,14 +263,12 @@ export class DockerService { public async pause(id: string): Promise { const container = this.client.getContainer(id); await container.pause(); - await this.cacheManager.del(DockerService.CONTAINER_CACHE_KEY); - this.logger.debug(`Invalidated container cache after pausing ${id}`); let containers: DockerContainer[]; let updatedContainer: DockerContainer | undefined; for (let i = 0; i < 5; i++) { await sleep(500); - containers = await this.getContainers({ skipCache: true }); + containers = await this.getContainers(); updatedContainer = containers.find((c) => c.id === id); this.logger.debug( `Container ${id} state after pause attempt ${i + 1}: ${updatedContainer?.state}` @@ -346,14 +289,12 @@ export class DockerService { public async unpause(id: string): Promise { const container = this.client.getContainer(id); await container.unpause(); - await this.cacheManager.del(DockerService.CONTAINER_CACHE_KEY); - this.logger.debug(`Invalidated container cache after unpausing ${id}`); let containers: DockerContainer[]; let updatedContainer: DockerContainer | undefined; for (let i = 0; i < 5; i++) { await sleep(500); - containers = await this.getContainers({ skipCache: true }); + containers = await this.getContainers(); updatedContainer = containers.find((c) => c.id === id); this.logger.debug( `Container ${id} state after unpause attempt ${i + 1}: ${updatedContainer?.state}` @@ -372,7 +313,7 @@ export class DockerService { } public async updateContainer(id: string): Promise { - const containers = await this.getContainers({ skipCache: true }); + const containers = await this.getContainers(); const container = containers.find((c) => c.id === id); if (!container) { throw new Error(`Container ${id} not found`); @@ -396,10 +337,7 @@ export class DockerService { throw new Error(`Failed to update container ${containerName}`); } - await this.clearContainerCache(); - this.logger.debug(`Invalidated container caches after updating ${id}`); - - const updatedContainers = await this.getContainers({ skipCache: true }); + const updatedContainers = await this.getContainers(); const updatedContainer = updatedContainers.find( (c) => c.names?.some((name) => name.replace(/^\//, '') === containerName) || c.id === id ); @@ -426,7 +364,7 @@ export class DockerService { * Updates every container with an available update. Mirrors the legacy webgui "Update All" flow. */ public async updateAllContainers(): Promise { - const containers = await this.getContainers({ skipCache: true }); + const containers = await this.getContainers(); if (!containers.length) { return []; } diff --git a/api/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.ts b/api/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.ts index 699770fa4f..7ae6075823 100644 --- a/api/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.ts +++ b/api/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.ts @@ -54,14 +54,8 @@ export class DockerOrganizerService { private readonly dockerService: DockerService ) {} - async getResources( - opts?: Partial & { skipCache?: boolean } - ): Promise { - const { skipCache = false, ...listOptions } = opts ?? {}; - const containers = await this.dockerService.getContainers({ - skipCache, - ...(listOptions as any), - }); + async getResources(opts?: Partial): Promise { + const containers = await this.dockerService.getContainers(opts); return containerListToResourcesObject(containers); } @@ -83,20 +77,17 @@ export class DockerOrganizerService { return newOrganizer; } - async syncAndGetOrganizer(opts?: { skipCache?: boolean }): Promise { + async syncAndGetOrganizer(): Promise { let organizer = this.dockerConfigService.getConfig(); - organizer.resources = await this.getResources(opts); + organizer.resources = await this.getResources(); organizer = await this.syncDefaultView(organizer, organizer.resources); organizer = await this.dockerConfigService.validate(organizer); this.dockerConfigService.replaceConfig(organizer); return organizer; } - async resolveOrganizer( - organizer?: OrganizerV1, - opts?: { skipCache?: boolean } - ): Promise { - organizer ??= await this.syncAndGetOrganizer(opts); + async resolveOrganizer(organizer?: OrganizerV1): Promise { + organizer ??= await this.syncAndGetOrganizer(); return resolveOrganizer(organizer); } From e67134a05a3cb2905742096f619b914e8fe5cf4c Mon Sep 17 00:00:00 2001 From: Pujit Mehrotra Date: Thu, 8 Jan 2026 14:44:02 -0500 Subject: [PATCH 2/7] fix(docker): sync template mappings in organizer to prevent false orphan warnings When a new container is installed, the GraphQL query fetches both containers and organizer data. The containers resolver correctly syncs template mappings before returning data, but the organizer service was fetching containers independently without syncing. This caused a race condition where the organizer showed isOrphaned=true for newly installed containers until page refresh. Added template sync to DockerOrganizerService.getResources() to ensure containers have correct template mappings before being returned. Co-Authored-By: Claude Opus 4.5 --- .../docker/organizer/docker-organizer.service.spec.ts | 7 +++++++ .../docker/organizer/docker-organizer.service.ts | 10 ++++++++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/api/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.spec.ts b/api/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.spec.ts index 12c80735a4..a2e012b1a8 100644 --- a/api/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.spec.ts +++ b/api/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.spec.ts @@ -3,6 +3,7 @@ import { Test } from '@nestjs/testing'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import { DockerTemplateIconService } from '@app/unraid-api/graph/resolvers/docker/docker-template-icon.service.js'; +import { DockerTemplateScannerService } from '@app/unraid-api/graph/resolvers/docker/docker-template-scanner.service.js'; import { ContainerPortType, ContainerState, @@ -227,6 +228,12 @@ describe('DockerOrganizerService', () => { getIconsForContainers: vi.fn().mockResolvedValue(new Map()), }, }, + { + provide: DockerTemplateScannerService, + useValue: { + syncMissingContainers: vi.fn().mockResolvedValue(false), + }, + }, ], }).compile(); diff --git a/api/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.ts b/api/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.ts index 7ae6075823..0d40157311 100644 --- a/api/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.ts +++ b/api/src/unraid-api/graph/resolvers/docker/organizer/docker-organizer.service.ts @@ -3,6 +3,7 @@ import { Injectable, Logger } from '@nestjs/common'; import type { ContainerListOptions } from 'dockerode'; import { AppError } from '@app/core/errors/app-error.js'; +import { DockerTemplateScannerService } from '@app/unraid-api/graph/resolvers/docker/docker-template-scanner.service.js'; import { DockerContainer } from '@app/unraid-api/graph/resolvers/docker/docker.model.js'; import { DockerService } from '@app/unraid-api/graph/resolvers/docker/docker.service.js'; import { DockerOrganizerConfigService } from '@app/unraid-api/graph/resolvers/docker/organizer/docker-organizer-config.service.js'; @@ -51,11 +52,16 @@ export class DockerOrganizerService { private readonly logger = new Logger(DockerOrganizerService.name); constructor( private readonly dockerConfigService: DockerOrganizerConfigService, - private readonly dockerService: DockerService + private readonly dockerService: DockerService, + private readonly dockerTemplateScannerService: DockerTemplateScannerService ) {} async getResources(opts?: Partial): Promise { - const containers = await this.dockerService.getContainers(opts); + let containers = await this.dockerService.getContainers(opts); + const wasSynced = await this.dockerTemplateScannerService.syncMissingContainers(containers); + if (wasSynced) { + containers = await this.dockerService.getContainers(opts); + } return containerListToResourcesObject(containers); } From b6169df614c8d9e942aef07f4ee74580216c9c7e Mon Sep 17 00:00:00 2001 From: Pujit Mehrotra Date: Fri, 9 Jan 2026 11:22:34 -0500 Subject: [PATCH 3/7] fix(web): improve text contrast in orphaned containers alert Replace text-warning color with text-foreground and text-muted-foreground for proper accessibility in both light and dark modes. The warning icon retains the warning color as visual indicator. Co-Authored-By: Claude Opus 4.5 --- web/src/components/Docker/DockerOrphanedAlert.vue | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/web/src/components/Docker/DockerOrphanedAlert.vue b/web/src/components/Docker/DockerOrphanedAlert.vue index 7d1d8c0d5a..eccdb72c1f 100644 --- a/web/src/components/Docker/DockerOrphanedAlert.vue +++ b/web/src/components/Docker/DockerOrphanedAlert.vue @@ -44,7 +44,7 @@ function formatContainerName(container: DockerContainer): string {