Skip to content

Commit 1c1bae8

Browse files
pujitmpujitm
authored
fix(docker): remove aggressive caching to ensure data correctness (#1864)
## Summary - Removes 60-second TTL caching from Docker container and network queries - Fixes stale data issues when containers are created/deleted externally (e.g., via CA) - Ensures API queries return live, up-to-date Docker state without requiring page refreshes ## Test plan - [x] Type checking passes (`pnpm type-check`) - [x] All Docker-related tests pass - [x] Verify container installs via CA appear immediately without refresh - [x] Verify container deletions are reflected immediately without refresh 🤖 Generated with [Claude Code](https://claude.ai/code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Docker container and network information are now always fetched fresh from the API without caching, ensuring real-time accuracy. * **Deprecations** * The skipCache parameter for Docker queries is deprecated and ignored; caching functionality has been completely removed. <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent c084e25 commit 1c1bae8

12 files changed

+102
-232
lines changed

‎api/src/unraid-api/graph/resolvers/docker/docker-event.service.spec.ts‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,6 @@ vi.mock('./utils/docker-client.js', () => ({
6161
vi.mock('./docker.service.js', () => ({
6262
DockerService: vi.fn().mockImplementation(() => ({
6363
getDockerClient: vi.fn(),
64-
clearContainerCache: vi.fn(),
6564
getAppInfo: vi.fn().mockResolvedValue({ info: { apps: { installed: 1, running: 1 } } }),
6665
})),
6766
}));
@@ -76,7 +75,6 @@ describe('DockerEventService', () => {
7675
// Create a mock Docker service *instance*
7776
const mockDockerServiceImpl = {
7877
getDockerClient: vi.fn(),
79-
clearContainerCache: vi.fn(),
8078
getAppInfo: vi.fn().mockResolvedValue({ info: { apps: { installed: 1, running: 1 } } }),
8179
};
8280

@@ -132,7 +130,6 @@ describe('DockerEventService', () => {
132130

133131
await waitForEventProcessing();
134132

135-
expect(dockerService.clearContainerCache).toHaveBeenCalled();
136133
expect(dockerService.getAppInfo).toHaveBeenCalled();
137134
expect(pubsub.publish).toHaveBeenCalledWith(PUBSUB_CHANNEL.INFO, expect.any(Object));
138135
});
@@ -154,7 +151,6 @@ describe('DockerEventService', () => {
154151

155152
await waitForEventProcessing();
156153

157-
expect(dockerService.clearContainerCache).not.toHaveBeenCalled();
158154
expect(dockerService.getAppInfo).not.toHaveBeenCalled();
159155
expect(pubsub.publish).not.toHaveBeenCalled();
160156
});
@@ -172,7 +168,6 @@ describe('DockerEventService', () => {
172168
await waitForEventProcessing();
173169

174170
expect(service.isActive()).toBe(true);
175-
expect(dockerService.clearContainerCache).toHaveBeenCalledTimes(1);
176171
expect(dockerService.getAppInfo).toHaveBeenCalledTimes(1);
177172
expect(pubsub.publish).toHaveBeenCalledTimes(1);
178173
});
@@ -190,7 +185,6 @@ describe('DockerEventService', () => {
190185

191186
await waitForEventProcessing();
192187

193-
expect(dockerService.clearContainerCache).toHaveBeenCalledTimes(2);
194188
expect(dockerService.getAppInfo).toHaveBeenCalledTimes(2);
195189
expect(pubsub.publish).toHaveBeenCalledTimes(2);
196190
});
@@ -206,7 +200,6 @@ describe('DockerEventService', () => {
206200

207201
await waitForEventProcessing();
208202

209-
expect(dockerService.clearContainerCache).toHaveBeenCalledTimes(1);
210203
expect(dockerService.getAppInfo).toHaveBeenCalledTimes(1);
211204
expect(pubsub.publish).toHaveBeenCalledTimes(1);
212205

@@ -223,7 +216,6 @@ describe('DockerEventService', () => {
223216

224217
await waitForEventProcessing();
225218

226-
expect(dockerService.clearContainerCache).toHaveBeenCalledTimes(1);
227219
expect(dockerService.getAppInfo).toHaveBeenCalledTimes(1);
228220
expect(pubsub.publish).toHaveBeenCalledTimes(1);
229221

‎api/src/unraid-api/graph/resolvers/docker/docker-event.service.ts‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -124,14 +124,12 @@ export class DockerEventService implements OnModuleDestroy, OnModuleInit {
124124
if (shouldProcess) {
125125
this.logger.debug(`[${dockerEvent.from}] ${dockerEvent.Type}->${actionName}`);
126126

127-
// For container lifecycle events, update the container cache
127+
// For container lifecycle events, publish updated app info
128128
if (
129129
dockerEvent.Type === DockerEventType.CONTAINER &&
130130
typeof actionName === 'string' &&
131131
this.containerActions.includes(actionName as DockerEventAction)
132132
) {
133-
await this.dockerService.clearContainerCache();
134-
// Get updated counts and publish
135133
const appInfo = await this.dockerService.getAppInfo();
136134
await pubsub.publish(PUBSUB_CHANNEL.INFO, appInfo);
137135
this.logger.debug(`Published app info update due to event: ${actionName}`);

‎api/src/unraid-api/graph/resolvers/docker/docker-network.service.spec.ts‎

Lines changed: 7 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import { CACHE_MANAGER } from '@nestjs/cache-manager';
21
import { Test, TestingModule } from '@nestjs/testing';
32

43
import { beforeEach, describe, expect, it, vi } from 'vitest';
@@ -17,27 +16,14 @@ vi.mock('@app/unraid-api/graph/resolvers/docker/utils/docker-client.js', () => (
1716
getDockerClient: vi.fn().mockReturnValue(mockDockerInstance),
1817
}));
1918

20-
const mockCacheManager = {
21-
get: vi.fn(),
22-
set: vi.fn(),
23-
};
24-
2519
describe('DockerNetworkService', () => {
2620
let service: DockerNetworkService;
2721

2822
beforeEach(async () => {
2923
mockListNetworks.mockReset();
30-
mockCacheManager.get.mockReset();
31-
mockCacheManager.set.mockReset();
3224

3325
const module: TestingModule = await Test.createTestingModule({
34-
providers: [
35-
DockerNetworkService,
36-
{
37-
provide: CACHE_MANAGER,
38-
useValue: mockCacheManager,
39-
},
40-
],
26+
providers: [DockerNetworkService],
4127
}).compile();
4228

4329
service = module.get<DockerNetworkService>(DockerNetworkService);
@@ -48,16 +34,7 @@ describe('DockerNetworkService', () => {
4834
});
4935

5036
describe('getNetworks', () => {
51-
it('should return cached networks if available and not skipped', async () => {
52-
const cached = [{ id: 'net1', name: 'test-net' }];
53-
mockCacheManager.get.mockResolvedValue(cached);
54-
55-
const result = await service.getNetworks({ skipCache: false });
56-
expect(result).toEqual(cached);
57-
expect(mockListNetworks).not.toHaveBeenCalled();
58-
});
59-
60-
it('should fetch networks from docker if cache skipped', async () => {
37+
it('should fetch networks from docker', async () => {
6138
const rawNetworks = [
6239
{
6340
Id: 'net1',
@@ -67,22 +44,18 @@ describe('DockerNetworkService', () => {
6744
];
6845
mockListNetworks.mockResolvedValue(rawNetworks);
6946

70-
const result = await service.getNetworks({ skipCache: true });
47+
const result = await service.getNetworks();
7148
expect(result).toHaveLength(1);
7249
expect(result[0].id).toBe('net1');
50+
expect(result[0].name).toBe('test-net');
7351
expect(mockListNetworks).toHaveBeenCalled();
74-
expect(mockCacheManager.set).toHaveBeenCalledWith(
75-
DockerNetworkService.NETWORK_CACHE_KEY,
76-
expect.anything(),
77-
expect.anything()
78-
);
7952
});
8053

81-
it('should fetch networks from docker if cache miss', async () => {
82-
mockCacheManager.get.mockResolvedValue(undefined);
54+
it('should return empty array when no networks exist', async () => {
8355
mockListNetworks.mockResolvedValue([]);
8456

85-
await service.getNetworks({ skipCache: false });
57+
const result = await service.getNetworks();
58+
expect(result).toEqual([]);
8659
expect(mockListNetworks).toHaveBeenCalled();
8760
});
8861
});

‎api/src/unraid-api/graph/resolvers/docker/docker-network.service.ts‎

Lines changed: 3 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,42 +1,20 @@
1-
import { CACHE_MANAGER } from '@nestjs/cache-manager';
2-
import { Inject, Injectable, Logger } from '@nestjs/common';
3-
4-
import { type Cache } from 'cache-manager';
1+
import { Injectable, Logger } from '@nestjs/common';
52

63
import { catchHandlers } from '@app/core/utils/misc/catch-handlers.js';
74
import { DockerNetwork } from '@app/unraid-api/graph/resolvers/docker/docker.model.js';
85
import { getDockerClient } from '@app/unraid-api/graph/resolvers/docker/utils/docker-client.js';
96

10-
interface NetworkListingOptions {
11-
skipCache: boolean;
12-
}
13-
147
@Injectable()
158
export class DockerNetworkService {
169
private readonly logger = new Logger(DockerNetworkService.name);
1710
private readonly client = getDockerClient();
1811

19-
public static readonly NETWORK_CACHE_KEY = 'docker_networks';
20-
private static readonly CACHE_TTL_SECONDS = 60;
21-
22-
constructor(@Inject(CACHE_MANAGER) private cacheManager: Cache) {}
23-
2412
/**
2513
* Get all Docker networks
2614
* @returns All the in/active Docker networks on the system.
2715
*/
28-
public async getNetworks({ skipCache }: NetworkListingOptions): Promise<DockerNetwork[]> {
29-
if (!skipCache) {
30-
const cachedNetworks = await this.cacheManager.get<DockerNetwork[]>(
31-
DockerNetworkService.NETWORK_CACHE_KEY
32-
);
33-
if (cachedNetworks) {
34-
this.logger.debug('Using docker network cache');
35-
return cachedNetworks;
36-
}
37-
}
38-
39-
this.logger.debug('Updating docker network cache');
16+
public async getNetworks(): Promise<DockerNetwork[]> {
17+
this.logger.debug('Fetching docker networks');
4018
const rawNetworks = await this.client.listNetworks().catch(catchHandlers.docker);
4119
const networks = rawNetworks.map(
4220
(network) =>
@@ -59,11 +37,6 @@ export class DockerNetworkService {
5937
}) as DockerNetwork
6038
);
6139

62-
await this.cacheManager.set(
63-
DockerNetworkService.NETWORK_CACHE_KEY,
64-
networks,
65-
DockerNetworkService.CACHE_TTL_SECONDS * 1000
66-
);
6740
return networks;
6841
}
6942
}

‎api/src/unraid-api/graph/resolvers/docker/docker-template-scanner.service.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ export class DockerTemplateScannerService {
8787
const templates = await this.loadAllTemplates(result);
8888

8989
try {
90-
const containers = await this.dockerService.getContainers({ skipCache: true });
90+
const containers = await this.dockerService.getContainers();
9191
const config = this.dockerConfigService.getConfig();
9292
const currentMappings = config.templateMappings || {};
9393
const skipSet = new Set(config.skipTemplatePaths || []);

‎api/src/unraid-api/graph/resolvers/docker/docker.module.spec.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ describe('DockerModule', () => {
7373
const module: TestingModule = await Test.createTestingModule({
7474
providers: [
7575
DockerResolver,
76-
{ provide: DockerService, useValue: { clearContainerCache: vi.fn() } },
76+
{ provide: DockerService, useValue: {} },
7777
{
7878
provide: DockerConfigService,
7979
useValue: {

‎api/src/unraid-api/graph/resolvers/docker/docker.resolver.spec.ts‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@ describe('DockerResolver', () => {
4040
getNetworks: vi.fn(),
4141
getContainerLogSizes: vi.fn(),
4242
getContainerLogs: vi.fn(),
43-
clearContainerCache: vi.fn(),
4443
},
4544
},
4645
{
@@ -161,7 +160,7 @@ describe('DockerResolver', () => {
161160
expect(GraphQLFieldHelper.isFieldRequested).toHaveBeenCalledWith(mockInfo, 'sizeRootFs');
162161
expect(GraphQLFieldHelper.isFieldRequested).toHaveBeenCalledWith(mockInfo, 'sizeRw');
163162
expect(GraphQLFieldHelper.isFieldRequested).toHaveBeenCalledWith(mockInfo, 'sizeLog');
164-
expect(dockerService.getContainers).toHaveBeenCalledWith({ skipCache: false, size: false });
163+
expect(dockerService.getContainers).toHaveBeenCalledWith({ size: false });
165164
});
166165

167166
it('should request size when sizeRootFs field is requested', async () => {
@@ -191,7 +190,7 @@ describe('DockerResolver', () => {
191190
const result = await resolver.containers(false, mockInfo);
192191
expect(result).toEqual(mockContainers);
193192
expect(GraphQLFieldHelper.isFieldRequested).toHaveBeenCalledWith(mockInfo, 'sizeRootFs');
194-
expect(dockerService.getContainers).toHaveBeenCalledWith({ skipCache: false, size: true });
193+
expect(dockerService.getContainers).toHaveBeenCalledWith({ size: true });
195194
});
196195

197196
it('should request size when sizeRw field is requested', async () => {
@@ -205,7 +204,7 @@ describe('DockerResolver', () => {
205204

206205
await resolver.containers(false, mockInfo);
207206
expect(GraphQLFieldHelper.isFieldRequested).toHaveBeenCalledWith(mockInfo, 'sizeRw');
208-
expect(dockerService.getContainers).toHaveBeenCalledWith({ skipCache: false, size: true });
207+
expect(dockerService.getContainers).toHaveBeenCalledWith({ size: true });
209208
});
210209

211210
it('should fetch log sizes when sizeLog field is requested', async () => {
@@ -240,7 +239,7 @@ describe('DockerResolver', () => {
240239
expect(GraphQLFieldHelper.isFieldRequested).toHaveBeenCalledWith(mockInfo, 'sizeLog');
241240
expect(dockerService.getContainerLogSizes).toHaveBeenCalledWith(['test-container']);
242241
expect(result[0]?.sizeLog).toBe(42);
243-
expect(dockerService.getContainers).toHaveBeenCalledWith({ skipCache: false, size: false });
242+
expect(dockerService.getContainers).toHaveBeenCalledWith({ size: false });
244243
});
245244

246245
it('should request size when GraphQLFieldHelper indicates sizeRootFs is requested', async () => {
@@ -254,7 +253,7 @@ describe('DockerResolver', () => {
254253

255254
await resolver.containers(false, mockInfo);
256255
expect(GraphQLFieldHelper.isFieldRequested).toHaveBeenCalledWith(mockInfo, 'sizeRootFs');
257-
expect(dockerService.getContainers).toHaveBeenCalledWith({ skipCache: false, size: true });
256+
expect(dockerService.getContainers).toHaveBeenCalledWith({ size: true });
258257
});
259258

260259
it('should not request size when GraphQLFieldHelper indicates sizeRootFs is not requested', async () => {
@@ -266,18 +265,19 @@ describe('DockerResolver', () => {
266265

267266
await resolver.containers(false, mockInfo);
268267
expect(GraphQLFieldHelper.isFieldRequested).toHaveBeenCalledWith(mockInfo, 'sizeRootFs');
269-
expect(dockerService.getContainers).toHaveBeenCalledWith({ skipCache: false, size: false });
268+
expect(dockerService.getContainers).toHaveBeenCalledWith({ size: false });
270269
});
271270

272-
it('should handle skipCache parameter', async () => {
271+
it('skipCache parameter is deprecated and ignored', async () => {
273272
const mockContainers: DockerContainer[] = [];
274273
vi.mocked(dockerService.getContainers).mockResolvedValue(mockContainers);
275274
vi.mocked(GraphQLFieldHelper.isFieldRequested).mockReturnValue(false);
276275

277276
const mockInfo = {} as any;
278277

278+
// skipCache parameter is now deprecated and ignored
279279
await resolver.containers(true, mockInfo);
280-
expect(dockerService.getContainers).toHaveBeenCalledWith({ skipCache: true, size: false });
280+
expect(dockerService.getContainers).toHaveBeenCalledWith({ size: false });
281281
});
282282

283283
it('should fetch container logs with provided arguments', async () => {

0 commit comments

Comments
 (0)