Skip to content

Commit 087b7a1

Browse files
authored
[cache] Fix defaultSupplier called on every get invocation (#518)
Reported on the community: https://community.openhab.org/t/cache-private-get-defaultsupplier-is-always-called/168578 Signed-off-by: Florian Hotze <dev@florianhotze.com>
1 parent f48b624 commit 087b7a1

3 files changed

Lines changed: 27 additions & 14 deletions

File tree

src/cache.js

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,13 @@ class JSCache {
3232
* @returns {*|null} the current object for the supplied key, the value returned by defaultSupplier (if provided), or `null`
3333
*/
3434
get (key, defaultSupplier) {
35-
if (typeof defaultSupplier === 'function') {
36-
if (defaultSupplier() === null) return null;
37-
return this.#valueCache.get(key, defaultSupplier);
38-
} else {
39-
return this.#valueCache.get(key);
40-
}
35+
if (this.exists(key)) return this.#valueCache.get(key);
36+
// key doesn't exist in cache: invoke supplier if provided
37+
if (typeof defaultSupplier !== 'function') return null;
38+
const supplied = defaultSupplier();
39+
if (supplied === null) return null; // do not store null values in cache
40+
this.#valueCache.put(key, supplied);
41+
return supplied;
4142
}
4243

4344
/**

test/cache.spec.js

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -42,19 +42,31 @@ describe('cache.js', () => {
4242
expect(console.warn).not.toHaveBeenCalled();
4343
});
4444

45-
it('passes supplier, if present.', () => {
46-
const supplier = () => 'value';
47-
cache.get(key, supplier);
48-
expect(mockGet).toHaveBeenCalledWith(key, supplier);
49-
expect(console.warn).not.toHaveBeenCalled();
50-
});
51-
5245
it('returns value from cache instance.', () => {
5346
const returnValue = 'value';
5447
mockGet.mockImplementation(() => returnValue);
5548
expect(cache.get(key)).toBe(returnValue);
5649
expect(console.warn).not.toHaveBeenCalled();
5750
});
51+
52+
it('returns null, if key does not exist in cache instance and no supplier is provided.', () => {
53+
mockGet.mockImplementation(() => null);
54+
expect(cache.get(key)).toBe(null);
55+
});
56+
57+
it('returns value from supplier, if key does not exist in cache instance and supplier is provided.', () => {
58+
const supplier = jest.fn(() => 'value');
59+
mockGet.mockImplementation(() => null);
60+
expect(cache.get(key, supplier)).toBe('value');
61+
expect(supplier).toHaveBeenCalledTimes(1);
62+
});
63+
64+
it('puts value from supplier into cache instance, if key does not exist in cache instance and supplier is provided.', () => {
65+
const supplier = () => 'value';
66+
mockGet.mockImplementation(() => null);
67+
cache.get(key, supplier);
68+
expect(mockPut).toHaveBeenCalledWith(key, 'value');
69+
});
5870
});
5971

6072
describe('put', () => {

types/cache.d.ts.map

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)