Skip to content

Commit 87360e0

Browse files
dominique-pfisterdominique-pfister
authored
fix: ignore changes when data has no Account key (#961)
* fix: check data contains non-empty Account * fix: check presence of Account key before storing * chore(test): fix title * chore(test): fix title * fix: show info message when Account is empty
1 parent d570310 commit 87360e0

8 files changed

Lines changed: 222 additions & 108 deletions

File tree

‎packages/helix-shared-tokencache/src/MemCachePlugin.js‎

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -83,17 +83,24 @@ export class MemCachePlugin {
8383
* @param {TokenCacheContext} cacheContext
8484
*/
8585
async afterCacheAccess(cacheContext) {
86-
if (cacheContext.cacheHasChanged) {
87-
this.log.debug('mem: write token cache', this.key);
88-
const cache = this.#getOrCreateCache();
89-
cache.data = cacheContext.tokenCache.serialize();
90-
if (this.base) {
91-
this.log.debug('mem: write token cache done. telling base', this.key);
92-
return this.base.afterCacheAccess(cacheContext);
93-
}
94-
return true;
86+
const { log } = this;
87+
88+
if (!cacheContext.cacheHasChanged) {
89+
return false;
9590
}
96-
return false;
91+
const data = JSON.parse(cacheContext.tokenCache.serialize());
92+
if (Object.keys(data.Account ?? {}).length === 0) {
93+
log.info('mem: write token cache done, ignoring empty data', this.key);
94+
return false;
95+
}
96+
log.debug('mem: write token cache', this.key);
97+
const cache = this.#getOrCreateCache();
98+
cache.data = JSON.stringify(data);
99+
if (this.base) {
100+
log.debug('mem: write token cache done. telling base', this.key);
101+
return this.base.afterCacheAccess(cacheContext);
102+
}
103+
return true;
97104
}
98105

99106
get location() {

‎packages/helix-shared-tokencache/src/S3CachePlugin.js‎

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -142,19 +142,23 @@ export class S3CachePlugin {
142142
async afterCacheAccess(cacheContext) {
143143
const { log } = this;
144144

145-
if (cacheContext.cacheHasChanged) {
146-
if (!this.meta) {
147-
await this.#loadData();
148-
}
149-
const data = JSON.parse(cacheContext.tokenCache.serialize());
150-
if (!isDeepStrictEqual(data, this.data)) {
151-
this.data = data;
152-
return this.#saveData();
153-
} else {
154-
log.debug('s3: we were told cache has changed, but contents didn\'t');
155-
}
145+
if (!cacheContext.cacheHasChanged) {
146+
return false;
156147
}
157-
return false;
148+
if (!this.meta) {
149+
await this.#loadData();
150+
}
151+
const data = JSON.parse(cacheContext.tokenCache.serialize());
152+
if (Object.keys(data.Account ?? {}).length === 0) {
153+
log.info('s3: write token cache, ignoring empty data', this.key);
154+
return false;
155+
}
156+
if (isDeepStrictEqual(data, this.data)) {
157+
log.debug('s3: we were told cache has changed, but contents didn\'t');
158+
return false;
159+
}
160+
this.data = data;
161+
return this.#saveData();
158162
}
159163

160164
async getPluginMetadata() {

‎packages/helix-shared-tokencache/test/MockTokenCacheContext.js‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,18 @@
1717
*/
1818
export class MockTokenCacheContext {
1919
constructor(opts = {}) {
20-
const context = this;
21-
Object.assign(this, {
22-
cacheHasChanged: false,
23-
tokens: '',
24-
/** @type ISerializableTokenCache */
25-
tokenCache: {
26-
serialize: () => context.tokens,
27-
deserialize: (value) => { context.tokens = value; },
20+
this.cacheHasChanged = opts.cacheHasChanged;
21+
this.data = opts.data || { Account: {}, AccessToken: {} };
22+
23+
this.tokenCache = {
24+
serialize: () => JSON.stringify(this.data),
25+
deserialize: (value) => {
26+
this.data = JSON.parse(value);
2827
},
29-
}, opts);
28+
};
29+
}
30+
31+
get token() {
32+
return this.data.AccessToken?.['foo-id']?.secret ?? '';
3033
}
3134
}

‎packages/helix-shared-tokencache/test/fs-cache-manager.test.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import crypto from 'crypto';
1717
import { promises as fs } from 'fs';
1818
import { FSCacheManager, FSCachePlugin } from '../src/index.js';
1919
import { MockTokenCacheContext } from './MockTokenCacheContext.js';
20+
import { toAuthContent } from './utils.js';
2021

2122
describe('FSCacheManager Test', () => {
2223
let testRoot;
@@ -108,7 +109,7 @@ describe('FSCacheManager Test', () => {
108109
const p = await mgr.getCache('content');
109110
const ctx = new MockTokenCacheContext({
110111
cacheHasChanged: true,
111-
tokens: '{ "access_token": "1234" }',
112+
data: toAuthContent('1234'),
112113
});
113114
await p.afterCacheAccess(ctx);
114115

‎packages/helix-shared-tokencache/test/fs-cache-plugin.test.js‎

Lines changed: 24 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import crypto from 'crypto';
1717
import { promises as fs } from 'fs';
1818
import { FSCachePlugin } from '../src/index.js';
1919
import { MockTokenCacheContext } from './MockTokenCacheContext.js';
20+
import { toAuthContent } from './utils.js';
2021

2122
describe('FSCachePlugin Test', () => {
2223
let testRoot;
@@ -37,21 +38,20 @@ describe('FSCachePlugin Test', () => {
3738
filePath: testFilePath,
3839
});
3940

41+
const data = toAuthContent('1234');
4042
const ctx = new MockTokenCacheContext({
4143
cacheHasChanged: true,
42-
tokens: '{ "access_token": "1234" }',
44+
data,
4345
});
4446
const ret = await p.afterCacheAccess(ctx);
4547
assert.strictEqual(ret, true);
46-
assert.deepStrictEqual(JSON.parse(await fs.readFile(testFilePath, 'utf-8')), {
47-
access_token: '1234',
48-
});
48+
assert.deepStrictEqual(JSON.parse(await fs.readFile(testFilePath, 'utf-8')), data);
4949
assert.strictEqual(p.location, testFilePath);
5050
});
5151

5252
it('writes the fresh cache data does not overwrite metadata', async () => {
5353
await fs.writeFile(testFilePath, JSON.stringify({
54-
access_token: '1234',
54+
...toAuthContent('1234'),
5555
cachePluginMetadata: {
5656
foo: 'bar',
5757
},
@@ -61,14 +61,15 @@ describe('FSCachePlugin Test', () => {
6161
filePath: testFilePath,
6262
});
6363

64+
const data = toAuthContent('2345');
6465
const ctx = new MockTokenCacheContext({
6566
cacheHasChanged: true,
66-
tokens: '{ "access_token": "1234" }',
67+
data,
6768
});
6869
const ret = await p.afterCacheAccess(ctx);
6970
assert.strictEqual(ret, true);
7071
assert.deepStrictEqual(JSON.parse(await fs.readFile(testFilePath, 'utf-8')), {
71-
access_token: '1234',
72+
...data,
7273
cachePluginMetadata: {
7374
foo: 'bar',
7475
},
@@ -81,19 +82,18 @@ describe('FSCachePlugin Test', () => {
8182
filePath: testFilePath,
8283
});
8384

85+
const data = toAuthContent('2345');
8486
const ctx = new MockTokenCacheContext({
8587
cacheHasChanged: true,
86-
tokens: '{ "access_token": "1234" }',
88+
data,
8789
});
8890
const ret = await p.afterCacheAccess(ctx);
8991
assert.strictEqual(ret, true);
90-
assert.deepStrictEqual(JSON.parse(await fs.readFile(testFilePath, 'utf-8')), {
91-
access_token: '1234',
92-
});
92+
assert.deepStrictEqual(JSON.parse(await fs.readFile(testFilePath, 'utf-8')), data);
9393

9494
await p.setPluginMetadata({ foo: 'bar' });
9595
assert.deepStrictEqual(JSON.parse(await fs.readFile(testFilePath, 'utf-8')), {
96-
access_token: '1234',
96+
...data,
9797
cachePluginMetadata: {
9898
foo: 'bar',
9999
},
@@ -115,8 +115,9 @@ describe('FSCachePlugin Test', () => {
115115
});
116116

117117
it('can clear plugin metadata', async () => {
118+
const data = toAuthContent('2345');
118119
await fs.writeFile(testFilePath, JSON.stringify({
119-
access_token: '1234',
120+
...data,
120121
cachePluginMetadata: {
121122
foo: 'bar',
122123
},
@@ -126,9 +127,7 @@ describe('FSCachePlugin Test', () => {
126127
filePath: testFilePath,
127128
});
128129
await p.setPluginMetadata();
129-
assert.deepStrictEqual(JSON.parse(await fs.readFile(testFilePath, 'utf-8')), {
130-
access_token: '1234',
131-
});
130+
assert.deepStrictEqual(JSON.parse(await fs.readFile(testFilePath, 'utf-8')), data);
132131
});
133132

134133
it('does not the cache data to the filesystem if context not changed', async () => {
@@ -138,7 +137,7 @@ describe('FSCachePlugin Test', () => {
138137

139138
const ctx = new MockTokenCacheContext({
140139
cacheHasChanged: false,
141-
tokens: '{ "access_token": "1234" }',
140+
data: toAuthContent('2345'),
142141
});
143142
const ret = await p.afterCacheAccess(ctx);
144143
assert.strictEqual(ret, false);
@@ -153,7 +152,8 @@ describe('FSCachePlugin Test', () => {
153152
});
154153

155154
it('read cache data from the filesystem', async () => {
156-
await fs.writeFile(testFilePath, '{ "access_token": "1234" }', 'utf-8');
155+
const data = toAuthContent('1234');
156+
await fs.writeFile(testFilePath, JSON.stringify(data), 'utf-8');
157157

158158
const p = new FSCachePlugin({
159159
filePath: testFilePath,
@@ -163,12 +163,12 @@ describe('FSCachePlugin Test', () => {
163163
});
164164
const ret = await p.beforeCacheAccess(ctx);
165165
assert.strictEqual(ret, true);
166-
assert.strictEqual(ctx.tokens, '{ "access_token": "1234" }');
166+
assert.strictEqual(ctx.token, '1234');
167167
});
168168

169169
it('read cache plugin metadata from the filesystem', async () => {
170170
await fs.writeFile(testFilePath, JSON.stringify({
171-
access_token: '1234',
171+
...toAuthContent('1234'),
172172
cachePluginMetadata: {
173173
foo: 'bar',
174174
},
@@ -182,15 +182,15 @@ describe('FSCachePlugin Test', () => {
182182
});
183183
const ret = await p.beforeCacheAccess(ctx);
184184
assert.strictEqual(ret, true);
185-
assert.strictEqual(ctx.tokens, '{"access_token":"1234"}');
185+
assert.strictEqual(ctx.token, '1234');
186186
assert.deepStrictEqual(await p.getPluginMetadata(), {
187187
foo: 'bar',
188188
});
189189
});
190190

191191
it('read cache plugin metadata from pristine plugin', async () => {
192192
await fs.writeFile(testFilePath, JSON.stringify({
193-
access_token: '1234',
193+
...toAuthContent('1234'),
194194
cachePluginMetadata: {
195195
foo: 'bar',
196196
},
@@ -213,23 +213,11 @@ describe('FSCachePlugin Test', () => {
213213
});
214214
const ret = await p.beforeCacheAccess(ctx);
215215
assert.strictEqual(ret, false);
216-
assert.strictEqual(ctx.tokens, '');
217-
});
218-
219-
it('read cache data warns about other errors', async () => {
220-
const p = new FSCachePlugin({
221-
filePath: testFilePath,
222-
});
223-
224-
const ctx = new MockTokenCacheContext({
225-
});
226-
const ret = await p.beforeCacheAccess(ctx);
227-
assert.strictEqual(ret, false);
228-
assert.strictEqual(ctx.tokens, '');
216+
assert.strictEqual(ctx.token, '');
229217
});
230218

231219
it('deletes the cache from the filesystem', async () => {
232-
await fs.writeFile(testFilePath, '{ "access_token": "1234" }', 'utf-8');
220+
await fs.writeFile(testFilePath, JSON.stringify(toAuthContent('1234')), 'utf-8');
233221
const p = new FSCachePlugin({
234222
filePath: testFilePath,
235223
});

0 commit comments

Comments
 (0)