Skip to content

feat: added prefix option to redis#88

Merged
NatalieShaked merged 10 commits intomasterfrom
feat/add-prefix-to-redis
Dec 25, 2025
Merged

feat: added prefix option to redis#88
NatalieShaked merged 10 commits intomasterfrom
feat/add-prefix-to-redis

Conversation

@NatalieShaked
Copy link
Contributor

Question Answer
Bug fix
New feature
Breaking change
Deprecations
Documentation
Tests added
Chore

Further information:
Added the option to use prefix in Redis

@NatalieShaked NatalieShaked marked this pull request as draft December 24, 2025 11:13
@NatalieShaked NatalieShaked removed the request for review from NivGreenstein December 24, 2025 11:13
@NatalieShaked NatalieShaked marked this pull request as ready for review December 24, 2025 11:57
"dbIndex": 0,
"ttl": 600
"ttl": 600,
"prefix": "geocoding"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want it to use prefix by default?
If not, please remove.
you may enrich the docs instead

config/test.json Outdated
"dbIndex": 1
"dbIndex": 1,
"ttl": 300,
"prefix": "test"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it should check with and without prefix

const { ttl: redisTtl } = this.config.get(redisConfigPath);

const originalJson = res.json;
const fullRequestId = prefix != undefined ? `${prefix}:${reqId as string}` : (reqId as string);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

!== ?

Comment on lines +95 to +109
describe('Redis uses prefix key', () => {
it('should return 200 status code and add key to Redis with prefix', async function () {
const config = depContainer.resolve<IConfig>(SERVICES.CONFIG);
const prefix = config.get<string | undefined>('redis.prefix');
const redisConnection = depContainer.resolve<RedisClient>(SERVICES.REDIS);

const response = await requestSender.getTile({ tile: '18SUJ2339007393' });

const keys = prefix !== undefined ? await redisConnection.keys(prefix + '*') : await redisConnection.keys('*');

expect(keys.length).toBeGreaterThanOrEqual(1);

expect(response.status).toBe(httpStatusCodes.OK);
});
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

manipulate the code to add prefix for this test only

> Feedback API source code is built in a different repository at <TODO: ADD LINK TO FEEDBACK API REPO>.
> Feedback API source code is built in a different repository at [feedback-api](https://github.com/MapColonies/feedback-api).
> Geocoding API inserts the request and response to Redis before the response is sent.
Speaking of Redis, in the redis config you can add a `prefix` flag, if you'd like to add keys with prefixes to redis.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be somewhere else that is more relevant

@NatalieShaked NatalieShaked merged commit 0ca7111 into master Dec 25, 2025
10 checks passed
@NivGreenstein NivGreenstein deleted the feat/add-prefix-to-redis branch December 25, 2025 13:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants