IBLCATALOG-616: quick draft for a new refresh method - #94
Open
notten-bbc wants to merge 4 commits into
Open
Conversation
Co-authored-by: Nicolas Otten <nicolas.otten@bbc.co.uk>
BenSymons
marked this pull request as ready for review
August 17, 2026 13:07
BenSymons
reviewed
Aug 17, 2026
| it('sets the TTL if the second argument is an integer', async () => { | ||
| const func = ceych.wrap(wrappable, 5); | ||
|
|
||
| return func() |
There was a problem hiding this comment.
lots of tests used a return then a .then to assert on the cache client. We refactored it to async await to make it much simpler
BenSymons
reviewed
Aug 17, 2026
| cacheClient.get.onFirstCall().returns(null) | ||
| .onSecondCall().returns({ item: 1 }) | ||
| .onThirdCall().returns(null); | ||
| const dropStub = sandbox.stub().resolves(); |
There was a problem hiding this comment.
many of the tests re-mocked the whole of the cache client for some reason. We changed this so it only re-mocked the get method making it much cleaner
BenSymons
reviewed
Aug 17, 2026
|
|
||
| describe('ceych', () => { | ||
| let ceych; | ||
| const wrappable = sandbox.stub().returns(Promise.resolve(1)); |
There was a problem hiding this comment.
the mocks weren't defined in the beforeEach which was causing issues with the tests
There was a problem hiding this comment.
Pull request overview
This PR introduces a new “manual cache update” API on Ceych and updates tests/docs to support and describe this behavior.
Changes:
- Add
Ceych#set()for manually overwriting a cache entry (with randomized TTL and optional StatsD metric). - Refactor existing tests to reuse a shared Catbox client setup and modernize some assertions to
async/await. - Update README docs and add a
packageManagerpin inpackage.json.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| lib/ceych.js | Adds set() API and refactors option parsing helper used by invalidate(). |
| test/lib/ceych.js | Adds tests for set() and refactors cache client stubbing/setup across suites. |
| README.md | Documents the new ceych.set API (section added/edited). |
| package.json | Adds packageManager field (pnpm pin). |
Suppressed comments (2)
test/lib/ceych.js:300
ceych.set()returns the underlyingcache.set()promise; the tests shouldawaitit to avoid race conditions (and to fail properly ifcache.set()rejects).
// Manually set a new value in the cache
ceych.set(wrappable, [], 100);
await wrapped();
test/lib/ceych.js:322
- This test stubs
Math.randomoutside the Sinon sandbox (so it won't be restored inafterEach), and it asserts a TTL value in seconds even though the cache layer is called with TTL in milliseconds elsewhere (memoizemultiplies by 1000).
it('sets the TTL to a random value between defaultTTL and defaultTTL / 2', async () => {
const cacheKey = createCacheKey(wrappable, [], '');
const wrapped = ceych.wrap(wrappable);
sinon.stub(Math, 'random').returns(1);
ceych.set(wrappable, [], 100);
sinon.assert.calledOnce(cacheClient.set);
const setArgs = cacheClient.set.getCall(0).args;
assert.strictEqual(setArgs[0].id, cacheKey.id);
assert.strictEqual(setArgs[1], 100);
assert.strictEqual(setArgs[2], 15);
});
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+35
to
38
| function getOptions(opts) { | ||
| if (!opts) { | ||
| throw new Error('Incorrect invalidate opts received, you must pass a function or options object to invalidate.'); | ||
| } |
Comment on lines
+80
to
+84
| #### `ceych.set() | ||
|
|
||
| Use this to manually sets the cache entry for the given function and args combination. You can use this to overrwrite an existing cache entry to a newer one. | ||
|
|
||
| The new cache key will have a TTL set randomly between this.defaultTtl/2 and this.defaultTtl. This is to ensure that when manually setting a lot of cache keys at the same time, they don't end up all expiring at the same time and causing lots of caches misses. |
Comment on lines
+129
to
+134
| * The new cache key will have a TTL set randomly between this.defaultTtl/2 and this.defaultTtl. | ||
| * This is to ensure that when manually setting a lot of cache keys at the same time, they don't end | ||
| * up all expiring at the same time and causing lots of caches misses. | ||
| * @param {function | {func: function, suffix: string}} funcOrOpts Either a function or options including `func` and optional `suffix`. | ||
| * @param {...any} functionArgs The args that identify the target cache key. | ||
| * @param {*} updatedValue The value to store in cache. |
Co-authored-by: Nicolas Otten <nicolas.otten@bbc.co.uk>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.