From 413190acbc3f39ffe7841f8e961a496b2abd402d Mon Sep 17 00:00:00 2001 From: Kristijan Rebernisak Date: Fri, 30 Oct 2020 15:21:02 +0100 Subject: [PATCH] Fix local in-memory cache --- CHANGELOG.md | 7 +++++ bootstrap/index.js | 6 ++--- bootstrap/lib/cache/index.js | 51 +++++++++++++++++++++--------------- bootstrap/lib/cache/local.js | 7 +++-- bootstrap/lib/cache/redis.js | 11 ++++++-- bootstrap/package.json | 2 +- bootstrap/test/cache.test.js | 24 ++++++++++------- package.json | 6 ++--- 8 files changed, 72 insertions(+), 42 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c9cf1e2c85..9c1e601f43 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,12 +6,19 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ## [Unreleased] +## [0.1.4] - 2020-10-30 + ### Added - New adapters: - `linkpool` to get ICE futures quotes - `onchain` to get ICE futures quotes +### Fixed + +- Local in-memory cache got broken in the last release, and was reinitialized on every request. +- `amberdata` adapter wasn't responding for some assets because they don't make cross-rates by default. Cross-rates are now included which enables this adapter to support markets like `BNB/USD`, not just the actual `BNB/USDT`. + ## [0.1.3] - 2020-10-29 ### Added diff --git a/bootstrap/index.js b/bootstrap/index.js index 73d8d38071..94157cce27 100644 --- a/bootstrap/index.js +++ b/bootstrap/index.js @@ -1,6 +1,6 @@ const { Requester, logger } = require('@chainlink/external-adapter') const { types } = require('util') -const { withCache, envOptions } = require('./lib/cache') +const { withCache, defaultOptions, redactOptions } = require('./lib/cache') const util = require('./lib/util') const server = require('./lib/server') const gcp = require('./lib/gcp') @@ -89,7 +89,7 @@ const expose = (execute, checkHealth) => { } // Log cache default options once -const cacheOptions = envOptions() -if (cacheOptions.enabled) logger.info('Cache enabled: ', cacheOptions) +const cacheOptions = defaultOptions() +if (cacheOptions.enabled) logger.info('Cache enabled: ', redactOptions(cacheOptions)) module.exports = { expose, util } diff --git a/bootstrap/lib/cache/index.js b/bootstrap/lib/cache/index.js index 88c7cb177c..39d53a35fd 100644 --- a/bootstrap/lib/cache/index.js +++ b/bootstrap/lib/cache/index.js @@ -14,11 +14,10 @@ const DEFAULT_RC_INTERVAL_COEFFICIENT = 2 const DEFAULT_RC_ENTROPY_MAX = 0 const env = process.env -const envOptions = () => ({ +const defaultOptions = () => ({ enabled: parseBool(env.CACHE_ENABLED), - type: env.CACHE_TYPE || DEFAULT_CACHE_TYPE, - local: local.envOptions(), - redis: redis.envOptions(), + cacheOptions: defaultCacheOptions(), + cacheBuilder: defaultCacheBuilder(), key: { group: env.CACHE_KEY_GROUP || DEFAULT_CACHE_KEY_GROUP, ignored: [ @@ -38,31 +37,40 @@ const envOptions = () => ({ entropyMax: Number(env.REQUEST_COALESCING_ENTROPY_MAX) || DEFAULT_RC_ENTROPY_MAX, }, }) - -const envOptionsSafe = () => { - const opts = envOptions() - const redis = opts.redis - if (redis.password) redis.password = redis.password.replace(/.+/g, '*****') - if (redis.url) redis.url = redis.url.replace(/:\/\/.+@/g, '://*****@') - return opts +const defaultCacheOptions = () => { + const type = env.CACHE_TYPE || DEFAULT_CACHE_TYPE + const options = type === 'redis' ? redis.defaultOptions() : local.defaultOptions() + return { ...options, type } } - -const getCacheImpl = (options) => { - switch (options.type) { - case 'redis': - return redis.RedisCache.build(options.redis) - default: - return new local.LocalLRUCache(options.local) +// TODO: Revisit this after we stop to reinitialize middleware on every request +// We store the local LRU cache instance, so it's not reinitialized on every request +let localLRUCache +const defaultCacheBuilder = () => { + return (options) => { + switch (options.type) { + case 'redis': + return redis.RedisCache.build(options) + default: + return localLRUCache || (localLRUCache = new local.LocalLRUCache(options)) + } } } +// Options without sensitive data +const redactOptions = (options) => ({ + ...options, + cacheOptions: + options.cacheOptions.type === 'redis' + ? redis.redactOptions(options.cacheOptions) + : local.redactOptions(options.cacheOptions), +}) const withCache = async (execute, options) => { // If no options read the env with sensible defaults - if (!options) options = envOptions() + if (!options) options = defaultOptions() // If disabled noop if (!options.enabled) return (data) => execute(data) - const cache = await getCacheImpl(options) + const cache = await options.cacheBuilder(options.cacheOptions) // Algorithm we use to derive entry key const hashOptions = { @@ -165,5 +173,6 @@ const withCache = async (execute, options) => { module.exports = { withCache, - envOptions: envOptionsSafe, + defaultOptions, + redactOptions, } diff --git a/bootstrap/lib/cache/local.js b/bootstrap/lib/cache/local.js index 34187054ba..d96ff97491 100644 --- a/bootstrap/lib/cache/local.js +++ b/bootstrap/lib/cache/local.js @@ -7,11 +7,13 @@ const DEFAULT_CACHE_MAX_AGE = 1000 * 30 // Maximum age in ms const DEFAULT_CACHE_UPDATE_AGE_ON_GET = false const env = process.env -const envOptions = () => ({ +const defaultOptions = () => ({ max: Number(env.CACHE_MAX_ITEMS) || DEFAULT_CACHE_MAX_ITEMS, maxAge: Number(env.CACHE_MAX_AGE) || DEFAULT_CACHE_MAX_AGE, updateAgeOnGet: parseBool(env.CACHE_UPDATE_AGE_ON_GET) || DEFAULT_CACHE_UPDATE_AGE_ON_GET, }) +// Options without sensitive data +const redactOptions = (opts) => opts class LocalLRUCache { constructor(options) { @@ -38,5 +40,6 @@ class LocalLRUCache { module.exports = { LocalLRUCache, - envOptions, + defaultOptions, + redactOptions, } diff --git a/bootstrap/lib/cache/redis.js b/bootstrap/lib/cache/redis.js index 506e1bdc68..fc7fd5f339 100644 --- a/bootstrap/lib/cache/redis.js +++ b/bootstrap/lib/cache/redis.js @@ -14,7 +14,7 @@ const DEFAULT_CACHE_REDIS_TIMEOUT = 500 // Timeout in ms const DEFAULT_CACHE_MAX_AGE = 1000 * 30 // Maximum age in ms const env = process.env -const envOptions = () => ({ +const defaultOptions = () => ({ host: env.CACHE_REDIS_HOST || DEFAULT_CACHE_REDIS_HOST, port: env.CACHE_REDIS_PORT || DEFAULT_CACHE_REDIS_PORT, path: env.CACHE_REDIS_PATH || DEFAULT_CACHE_REDIS_PATH, @@ -23,6 +23,12 @@ const envOptions = () => ({ maxAge: Number(env.CACHE_MAX_AGE) || DEFAULT_CACHE_MAX_AGE, timeout: Number(env.CACHE_REDIS_TIMEOUT) || DEFAULT_CACHE_REDIS_TIMEOUT, }) +// Options without sensitive data +const redactOptions = (opts) => { + if (opts.password) opts.password = opts.password.replace(/.+/g, '*****') + if (opts.url) redis.url = opts.url.replace(/:\/\/.+@/g, '://*****@') + return opts +} const retryStrategy = (options) => { logger.warn('Redis retry strategy activated.', options) @@ -98,5 +104,6 @@ class RedisCache { module.exports = { RedisCache, - envOptions, + defaultOptions, + redactOptions, } diff --git a/bootstrap/package.json b/bootstrap/package.json index 2e650a2aa3..c832014309 100644 --- a/bootstrap/package.json +++ b/bootstrap/package.json @@ -1,6 +1,6 @@ { "name": "@chainlink/ea-bootstrap", - "version": "0.1.2", + "version": "0.1.3", "description": "Bootstrap an external adapter with this package", "main": "index.js", "license": "MIT", diff --git a/bootstrap/test/cache.test.js b/bootstrap/test/cache.test.js index 6b6c1dd8d1..2561f9d0d2 100644 --- a/bootstrap/test/cache.test.js +++ b/bootstrap/test/cache.test.js @@ -1,6 +1,7 @@ const { expect } = require('chai') const { useFakeTimers } = require('sinon') -const { withCache, envOptions } = require('../lib/cache') +const { withCache, defaultOptions } = require('../lib/cache') +const { LocalLRUCache } = require('../lib/cache/local') const callAndExpect = async (fn, n, result) => { while (n--) { @@ -11,6 +12,8 @@ const callAndExpect = async (fn, n, result) => { // Helper test function: a stateful counter const counterFrom = (i = 0) => () => ({ statusCode: 200, data: i++ }) +// Build new cache every time +const cacheBuilder = (options) => new LocalLRUCache(options) describe('cache', () => { context('options defaults', () => { @@ -20,7 +23,7 @@ describe('cache', () => { }) it(`configures env options with cache enabled: false`, () => { - const options = envOptions() + const options = defaultOptions() expect(options).to.have.property('enabled', false) }) }) @@ -31,13 +34,13 @@ describe('cache', () => { }) it(`configures env options with cache enabled: true`, () => { - const options = envOptions() + const options = defaultOptions() expect(options).to.have.property('enabled', true) }) it(`configures env options with default maxAge: 1000 * 30`, () => { - const options = envOptions() - expect(options.local).to.have.property('maxAge', 1000 * 30) + const options = defaultOptions() + expect(options.cacheOptions).to.have.property('maxAge', 1000 * 30) }) }) }) @@ -57,9 +60,11 @@ describe('cache', () => { }) context('enabled', () => { + let options let clock beforeEach(() => { process.env.CACHE_ENABLED = 'true' + options = { ...defaultOptions(), cacheBuilder } clock = useFakeTimers() }) @@ -68,12 +73,12 @@ describe('cache', () => { }) it(`caches fn result`, async () => { - const counter = await withCache(counterFrom(0)) + const counter = await withCache(counterFrom(0), options) await callAndExpect(counter, 3, 0) }) it(`caches fn result - while entry still young (under 30s default)`, async () => { - const counter = await withCache(counterFrom(0)) + const counter = await withCache(counterFrom(0), options) await callAndExpect(counter, 3, 0) await callAndExpect(counter, 3, 0) @@ -87,7 +92,7 @@ describe('cache', () => { }) it(`invalidates cache - after default configured maxAge of 30s`, async () => { - const counter = await withCache(counterFrom(0)) + const counter = await withCache(counterFrom(0), options) await callAndExpect(counter, 3, 0) clock.tick(1000 * 25) @@ -102,8 +107,7 @@ describe('cache', () => { }) it(`invalidates cache - after configured maxAge of 10s`, async () => { - const options = envOptions() - options.local.maxAge = 1000 * 10 + options.cacheOptions.maxAge = 1000 * 10 const counter = await withCache(counterFrom(0), options) await callAndExpect(counter, 3, 0) diff --git a/package.json b/package.json index 69431d40ae..a68747bd3c 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@chainlink/external-adapters-js", - "version": "0.1.3", + "version": "0.1.4", "license": "MIT", "private": true, "workspaces": [ @@ -85,8 +85,8 @@ "test:example-start-server": "node ./helpers/server.js" }, "dependencies": { - "@chainlink/ea-bootstrap": "^0.1.0", - "@chainlink/external-adapter": "^0.2.4", + "@chainlink/ea-bootstrap": "^0.1.3", + "@chainlink/external-adapter": "^0.2.6", "@zeit/ncc": "^0.22.1", "boxhock_google-finance-data": "^0.1.0", "decimal.js": "^10.2.0",