From 6d58b4d38e7aae0eb9250ae06d7eeaaab23ac2f2 Mon Sep 17 00:00:00 2001 From: Asif Date: Wed, 3 Jun 2026 11:00:04 +0300 Subject: [PATCH 1/4] fix: redis float bug --- src/redis/subscribe.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/redis/subscribe.ts b/src/redis/subscribe.ts index 8a7263d..97e0f20 100644 --- a/src/redis/subscribe.ts +++ b/src/redis/subscribe.ts @@ -36,10 +36,11 @@ export const redisSubscribe = async (deps: DependencyContainer): Promise { - if (!message.startsWith(prefixWithTtl)) { + const isTtlKey = message.startsWith(TTL_PREFIX) || message.includes(`:${TTL_PREFIX}`); + if (!isTtlKey) { logger.info(`Redis: Got new request ${message}`); - const noPrefixMessage = redisPrefix !== undefined ? message.split(':')[1] : message; + const noPrefixMessage = redisPrefix !== undefined ? message.substring(`${redisPrefix}:`.length) : message; const ttlMessage = prefixWithTtl + noPrefixMessage; // eslint-disable-next-line @typescript-eslint/naming-convention await redisClient.set(ttlMessage, '', { EX: redisTTL }); From 7a8fc529b78cec6baeaa378d74e572fbd276a41c Mon Sep 17 00:00:00 2001 From: Asif Date: Wed, 3 Jun 2026 14:06:26 +0300 Subject: [PATCH 2/4] fix: redis float bug --- tests/unit/redis/subscribe.spec.ts | 146 +++++++++++++++++++++++++++++ 1 file changed, 146 insertions(+) create mode 100644 tests/unit/redis/subscribe.spec.ts diff --git a/tests/unit/redis/subscribe.spec.ts b/tests/unit/redis/subscribe.spec.ts new file mode 100644 index 0000000..0d21759 --- /dev/null +++ b/tests/unit/redis/subscribe.spec.ts @@ -0,0 +1,146 @@ +/* eslint-disable @typescript-eslint/naming-convention */ +import jsLogger from '@map-colonies/js-logger'; +import { DependencyContainer } from 'tsyringe'; +import { Producer } from 'kafkajs'; +import { redisSubscribe } from '../../../src/redis/subscribe'; +import { REDIS_SUB, SERVICES } from '../../../src/common/constants'; + +type SetCallback = (message: string) => Promise; +type ExpiredCallback = (message: string) => Promise; + +const buildConfig = (prefix?: string): { get: jest.Mock; has: jest.Mock } => ({ + get: jest.fn().mockImplementation((key: string) => { + if (key === 'redis.expiredResponseTtl') return 300; + if (key === 'redis.prefix') return prefix; + if (key === 'outputTopic') return 'test-topic'; + return undefined; + }), + has: jest.fn().mockImplementation((key: string) => key === 'redis.prefix' && prefix !== undefined), +}); + +describe('redisSubscribe', () => { + let mockRedisClient: { get: jest.Mock; set: jest.Mock; del: jest.Mock }; + let mockSubscriber: { subscribe: jest.Mock }; + let mockProducer: { send: jest.Mock }; + let setCallback: SetCallback; + let expiredCallback: ExpiredCallback; + + beforeEach(() => { + mockRedisClient = { + get: jest.fn(), + set: jest.fn().mockResolvedValue(undefined), + del: jest.fn().mockResolvedValue(undefined), + }; + + mockSubscriber = { + subscribe: jest.fn().mockImplementation((channel: string, callback: SetCallback | ExpiredCallback) => { + if (channel.includes(':set')) setCallback = callback as SetCallback; + if (channel.includes(':expired')) expiredCallback = callback as ExpiredCallback; + return Promise.resolve(); + }), + }; + + mockProducer = { send: jest.fn().mockResolvedValue(undefined) }; + }); + + const setup = async (prefix?: string): Promise => { + const mockContainer = { + resolve: jest.fn().mockImplementation((token: symbol) => { + if (token === SERVICES.REDIS) return mockRedisClient; + if (token === REDIS_SUB) return mockSubscriber; + if (token === SERVICES.CONFIG) return buildConfig(prefix); + if (token === SERVICES.KAFKA) return mockProducer as unknown as Producer; + if (token === SERVICES.LOGGER) return jsLogger({ enabled: false }); + return undefined; + }), + } as unknown as DependencyContainer; + + await redisSubscribe(mockContainer); + }; + + // ─── SET event: isTtlKey guard ──────────────────────────────────────────── + + describe('SET event — isTtlKey guard (no prefix)', () => { + beforeEach(async () => setup()); + + it('should NOT create a TTL key when message starts with "ttl:" (own sentinel)', async () => { + await setCallback('ttl:some-uuid'); + expect(mockRedisClient.set).not.toHaveBeenCalled(); + }); + + it('should NOT create a TTL key when message contains ":ttl:" (cross-instance sentinel)', async () => { + await setCallback('geo:ttl:some-uuid'); + expect(mockRedisClient.set).not.toHaveBeenCalled(); + }); + + it('should NOT create a TTL key for deeply nested TTL keys (the loop pattern)', async () => { + await setCallback('ttl:ttl:ttl:ttl:some-uuid'); + expect(mockRedisClient.set).not.toHaveBeenCalled(); + }); + + it('should create a TTL key for a normal UUID key', async () => { + await setCallback('abc123-def456'); + expect(mockRedisClient.set).toHaveBeenCalledWith('ttl:abc123-def456', '', { EX: 300 }); + }); + }); + + describe('SET event — isTtlKey guard (with prefix)', () => { + beforeEach(async () => setup('geocoding')); + + it('should NOT create a TTL key when message starts with "ttl:" (different instance, no prefix)', async () => { + await setCallback('ttl:some-uuid'); + expect(mockRedisClient.set).not.toHaveBeenCalled(); + }); + + it('should NOT create a TTL key when message contains own prefix sentinel', async () => { + await setCallback('geocoding:ttl:some-uuid'); + expect(mockRedisClient.set).not.toHaveBeenCalled(); + }); + + it('should NOT create a TTL key when message contains ":ttl:" from a different prefix (cross-instance)', async () => { + await setCallback('other_service:ttl:some-uuid'); + expect(mockRedisClient.set).not.toHaveBeenCalled(); + }); + + it('should create a TTL key for a prefixed original key', async () => { + await setCallback('geocoding:some-uuid'); + expect(mockRedisClient.set).toHaveBeenCalledWith('geocoding:ttl:some-uuid', '', { EX: 300 }); + }); + + it('should preserve the full key when stripping prefix — not truncate at second colon', async () => { + // old bug: split(':')[1] would give "part1" instead of "part1:part2:part3" + await setCallback('geocoding:part1:part2:part3'); + expect(mockRedisClient.set).toHaveBeenCalledWith('geocoding:ttl:part1:part2:part3', '', { EX: 300 }); + }); + }); + + // ─── EXPIRED event handler ──────────────────────────────────────────────── + + describe('EXPIRED event handler', () => { + beforeEach(async () => setup()); + + it('should send no-chosen result to Kafka when wasUsed is false', async () => { + mockRedisClient.get.mockResolvedValue(JSON.stringify({ wasUsed: false })); + await expiredCallback('ttl:some-uuid'); + expect(mockProducer.send).toHaveBeenCalled(); + }); + + it('should NOT send to Kafka when wasUsed is true', async () => { + mockRedisClient.get.mockResolvedValue(JSON.stringify({ wasUsed: true })); + await expiredCallback('ttl:some-uuid'); + expect(mockProducer.send).not.toHaveBeenCalled(); + }); + + it('should always delete the original key after expiry', async () => { + mockRedisClient.get.mockResolvedValue(JSON.stringify({ wasUsed: true })); + await expiredCallback('ttl:some-uuid'); + expect(mockRedisClient.del).toHaveBeenCalledWith('some-uuid'); + }); + + it('should ignore expired keys that are not TTL sentinels', async () => { + await expiredCallback('regular-key'); + expect(mockRedisClient.get).not.toHaveBeenCalled(); + expect(mockRedisClient.del).not.toHaveBeenCalled(); + }); + }); +}); From 955c50862c3bf8861d679474d2a49c3f420215b9 Mon Sep 17 00:00:00 2001 From: Asif Date: Wed, 3 Jun 2026 14:19:23 +0300 Subject: [PATCH 3/4] fix: add tests with redis logic --- tests/unit/redis/subscribe.spec.ts | 44 +++++++++++++++++++++--------- 1 file changed, 31 insertions(+), 13 deletions(-) diff --git a/tests/unit/redis/subscribe.spec.ts b/tests/unit/redis/subscribe.spec.ts index 0d21759..bfe092b 100644 --- a/tests/unit/redis/subscribe.spec.ts +++ b/tests/unit/redis/subscribe.spec.ts @@ -1,4 +1,3 @@ -/* eslint-disable @typescript-eslint/naming-convention */ import jsLogger from '@map-colonies/js-logger'; import { DependencyContainer } from 'tsyringe'; import { Producer } from 'kafkajs'; @@ -10,9 +9,15 @@ type ExpiredCallback = (message: string) => Promise; const buildConfig = (prefix?: string): { get: jest.Mock; has: jest.Mock } => ({ get: jest.fn().mockImplementation((key: string) => { - if (key === 'redis.expiredResponseTtl') return 300; - if (key === 'redis.prefix') return prefix; - if (key === 'outputTopic') return 'test-topic'; + if (key === 'redis.expiredResponseTtl') { + return 300; + } + if (key === 'redis.prefix') { + return prefix; + } + if (key === 'outputTopic') { + return 'test-topic'; + } return undefined; }), has: jest.fn().mockImplementation((key: string) => key === 'redis.prefix' && prefix !== undefined), @@ -33,10 +38,13 @@ describe('redisSubscribe', () => { }; mockSubscriber = { - subscribe: jest.fn().mockImplementation((channel: string, callback: SetCallback | ExpiredCallback) => { - if (channel.includes(':set')) setCallback = callback as SetCallback; - if (channel.includes(':expired')) expiredCallback = callback as ExpiredCallback; - return Promise.resolve(); + subscribe: jest.fn().mockImplementation(async (channel: string, callback: SetCallback | ExpiredCallback) => { + if (channel.includes(':set')) { + setCallback = callback as SetCallback; + } + if (channel.includes(':expired')) { + expiredCallback = callback as ExpiredCallback; + } }), }; @@ -46,11 +54,21 @@ describe('redisSubscribe', () => { const setup = async (prefix?: string): Promise => { const mockContainer = { resolve: jest.fn().mockImplementation((token: symbol) => { - if (token === SERVICES.REDIS) return mockRedisClient; - if (token === REDIS_SUB) return mockSubscriber; - if (token === SERVICES.CONFIG) return buildConfig(prefix); - if (token === SERVICES.KAFKA) return mockProducer as unknown as Producer; - if (token === SERVICES.LOGGER) return jsLogger({ enabled: false }); + if (token === SERVICES.REDIS) { + return mockRedisClient; + } + if (token === REDIS_SUB) { + return mockSubscriber; + } + if (token === SERVICES.CONFIG) { + return buildConfig(prefix); + } + if (token === SERVICES.KAFKA) { + return mockProducer as unknown as Producer; + } + if (token === SERVICES.LOGGER) { + return jsLogger({ enabled: false }); + } return undefined; }), } as unknown as DependencyContainer; From 73a845a5563a3d386e34b95879753ac5891b73b4 Mon Sep 17 00:00:00 2001 From: Asif Date: Wed, 3 Jun 2026 14:22:46 +0300 Subject: [PATCH 4/4] fix: add tests with redis logic --- tests/unit/redis/subscribe.spec.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/unit/redis/subscribe.spec.ts b/tests/unit/redis/subscribe.spec.ts index bfe092b..32710bb 100644 --- a/tests/unit/redis/subscribe.spec.ts +++ b/tests/unit/redis/subscribe.spec.ts @@ -1,3 +1,4 @@ +/* eslint-disable @typescript-eslint/naming-convention */ import jsLogger from '@map-colonies/js-logger'; import { DependencyContainer } from 'tsyringe'; import { Producer } from 'kafkajs'; @@ -38,7 +39,7 @@ describe('redisSubscribe', () => { }; mockSubscriber = { - subscribe: jest.fn().mockImplementation(async (channel: string, callback: SetCallback | ExpiredCallback) => { + subscribe: jest.fn().mockImplementation((channel: string, callback: SetCallback | ExpiredCallback) => { if (channel.includes(':set')) { setCallback = callback as SetCallback; }