Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Use os.tmpDir for file system cache #1182

Merged
merged 1 commit into from
Mar 30, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 17 additions & 6 deletions packages/core/lib/FileSystemCache.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,34 @@
const fs = require('fs').promises;
const {existsSync, mkdirSync} = require('fs');
const crypto = require('crypto');
const os = require('os');
const log = require('./Log');
const LRUCache = require('lru-cache');

const path = require('path');

const DEFAULT_OPTS = {
baseDir: path.join(__dirname, '.cache'),
log,
maxItems: 50,
};

class FileSystemCache {
static create(opts = {}) {
return new FileSystemCache(Object.assign(DEFAULT_OPTS, opts));
static create(customOpts = {}) {
const opts = Object.assign(DEFAULT_OPTS, customOpts);
// Try to create a tmp directory...
try {
if (!opts.baseDir) {
opts.baseDir = path.join(os.tmpdir(), 'ampproject-toolbox-optimizer');
}
if (!existsSync(opts.baseDir)) {
mkdirSync(opts.baseDir);
}
} catch (e) {
log.debug('No filesystem access, falling back to in-memory cache only', e);
// ... if this fails, we have no write access to the fs, use a simple memory cache instead
return new LRUCache(opts.maxItems);
}
return new FileSystemCache(opts);
}

constructor(opts) {
Expand All @@ -57,9 +71,6 @@ class FileSystemCache {
async set(key, value) {
try {
this.cache.set(key, value);
if (!existsSync(this.opts.baseDir)) {
mkdirSync(this.opts.baseDir);
}
const cacheFile = this.createCacheFileName(key);
return fs.writeFile(cacheFile, JSON.stringify(value, null, ''), 'utf-8');
} catch (e) {
Expand Down
11 changes: 9 additions & 2 deletions packages/core/spec/lib/FileSystemCacheSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/

const FileSystemCache = require('../../lib/FileSystemCache.js');

let cache;
Expand All @@ -22,7 +21,6 @@ beforeEach(async () => {
cache = FileSystemCache.create();
await cache.clear();
});
//afterEach(async () => await cache.clear());

test('returns null by default', async () => {
const result = await cache.get('key');
Expand All @@ -39,3 +37,12 @@ test('returns cached value', async () => {
const result = await cache.get('key');
expect(result).toBe('value');
});

test('falls back to in memory cache', async () => {
cache = FileSystemCache.create({
baseDir: {}, // invalid pathname to trigger exception
});
await cache.set('key', 'value');
const result = await cache.get('key');
expect(result).toBe('value');
});