diff --git a/lib/client-metrics/client-metrics.test.js b/lib/client-metrics/client-metrics.test.js index 4c1eee55a4..b8d738a7b0 100644 --- a/lib/client-metrics/client-metrics.test.js +++ b/lib/client-metrics/client-metrics.test.js @@ -12,10 +12,53 @@ test('should work without state', (t) => { t.truthy(metrics.getMetricsOverview()); t.truthy(metrics.getTogglesMetrics()); + t.truthy(metrics.toJSON()); metrics.destroy(); }); +test.cb('data should expire', (t) => { + const clock = sinon.useFakeTimers(); + + const metrics = new UnleashClientMetrics(); + + metrics.addPayload({ + appName, + instanceId, + bucket: { + start: Date.now() - 2000, + stop: Date.now() - 1000, + toggles: { + toggleX: { + yes: 123, + no: 0, + }, + }, + }, + }); + + let lastHourExpires = 0; + metrics.lastHourList.on('expire', () => { + lastHourExpires++; + }); + + let lastMinExpires = 0; + metrics.lastMinuteList.on('expire', () => { + lastMinExpires++; + }); + + clock.tick(60 * 1000); + t.true(lastMinExpires === 1); + t.true(lastHourExpires === 0); + + clock.tick(60 * 60 * 1000); + t.true(lastMinExpires === 1); + t.true(lastHourExpires === 1); + + sinon.restore(); + t.end(); +}); + test('addPayload', t => { const metrics = new UnleashClientMetrics(); metrics.addPayload({ diff --git a/lib/client-metrics/list.js b/lib/client-metrics/list.js index 1b53a121a0..0e83c0abc7 100644 --- a/lib/client-metrics/list.js +++ b/lib/client-metrics/list.js @@ -15,13 +15,6 @@ class Node { } } - -/* - * linked list - * ranged list, assumes start to end(tail) is a known order - * remove() is only implemented in reverse order for the usecase - * emits events on eviction -*/ module.exports = class List extends EventEmitter { constructor () { super(); @@ -116,17 +109,17 @@ module.exports = class List extends EventEmitter { return result; } - toArrayReverse () { - const result = []; + // toArrayReverse () { + // const result = []; - if (this.tail) { - let cursor = this.tail; - while (cursor) { - result.push(cursor.value); - cursor = cursor.prev; - } - } + // if (this.tail) { + // let cursor = this.tail; + // while (cursor) { + // result.push(cursor.value); + // cursor = cursor.prev; + // } + // } - return result; - } + // return result; + // } }; diff --git a/lib/client-metrics/list.test.js b/lib/client-metrics/list.test.js index f60e342f82..c65415cdcd 100644 --- a/lib/client-metrics/list.test.js +++ b/lib/client-metrics/list.test.js @@ -77,6 +77,16 @@ test('list can be cleared and re-add entries', (t) => { t.true(list.toArray().length === 3); }); +test('should not iterate empty list ', (t) => { + const list = new List(); + + let iterateCount = 0; + list.iterate(() => { + iterateCount++; + }); + t.true(iterateCount === 0); +}); + test('should iterate', (t) => { const list = getList(); @@ -105,3 +115,13 @@ test('should reverse iterate', (t) => { }); t.true(iterateCount === 5); }); + +test('should not reverse iterate empty list', (t) => { + const list = new List(); + + let iterateCount = 0; + list.iterateReverse(() => { + iterateCount++; + }); + t.true(iterateCount === 0); +}); diff --git a/lib/client-metrics/service.js b/lib/client-metrics/service.js index cb87582ff1..231efbbe5f 100644 --- a/lib/client-metrics/service.js +++ b/lib/client-metrics/service.js @@ -1,17 +1,19 @@ 'use strict'; -const POLL_INTERVAL = 10000; const { EventEmitter } = require('events'); module.exports = class UnleashClientMetrics extends EventEmitter { - constructor (metricsDb) { + constructor (metricsDb, interval = 10000) { super(); + this.interval = interval; this.db = metricsDb; this.highestIdSeen = 0; this.db.getMetricsLastHour().then(metrics => { this.addMetrics(metrics); this.startPoller(); + this.emit('ready'); }); + this.timer = null; } addMetrics (metrics) { @@ -22,13 +24,20 @@ module.exports = class UnleashClientMetrics extends EventEmitter { } startPoller () { - setInterval(() => { + this.timer = setInterval(() => { this.db.getNewMetrics(this.highestIdSeen) .then(metrics => this.addMetrics(metrics)); - }, POLL_INTERVAL).unref(); + }, this.interval); + this.timer.unref(); } insert (metrics) { return this.db.insert(metrics); } + + destroy () { + try { + clearTimeout(this.timer); + } catch (e) {} + } }; diff --git a/lib/client-metrics/service.test.js b/lib/client-metrics/service.test.js new file mode 100644 index 0000000000..bb0a541528 --- /dev/null +++ b/lib/client-metrics/service.test.js @@ -0,0 +1,66 @@ +'use strict'; + +const { test } = require('ava'); +const MetricsService = require('./service'); +const sinon = require('sinon'); + +function getMockDb () { + const list = [{ id: 2 }, { id: 3 }, { id: 4 }]; + const db = { + getMetricsLastHour () { + return Promise.resolve([{ id: 1 }]); + }, + + getNewMetrics () { + return Promise.resolve([list.pop() || { id: 0 }]); + }, + }; + + return { + db, + }; +} + +test.cb('should call database on startup', (t) => { + const mock = getMockDb(); + const service = new MetricsService(mock.db); + t.plan(2); + + service.on('metrics', ([metric]) => { + t.true(service.highestIdSeen === 1); + t.true(metric.id === 1); + t.end(); + service.destroy(); + }); +}); + +test.cb('should poll for updates', (t) => { + const clock = sinon.useFakeTimers(); + + const mock = getMockDb(); + const service = new MetricsService(mock.db, 100); + + const metrics = []; + service.on('metrics', (_metrics) => { + _metrics.forEach(m => m && metrics.push(m)); + }); + + t.true(metrics.length === 0); + + service.on('ready', () => { + t.true(metrics.length === 1); + + clock.tick(300); + clock.restore(); + + process.nextTick(() => { + t.true(metrics.length === 4); + t.true(metrics[0].id === 1); + t.true(metrics[1].id === 4); + t.true(metrics[2].id === 3); + t.true(metrics[3].id === 2); + service.destroy(); + t.end(); + }); + }); +}); diff --git a/lib/client-metrics/ttl-list.js b/lib/client-metrics/ttl-list.js index 2d4d9d0dd3..208277fcc9 100644 --- a/lib/client-metrics/ttl-list.js +++ b/lib/client-metrics/ttl-list.js @@ -5,13 +5,14 @@ const List = require('./list'); const moment = require('moment'); // this list must have entries with sorted ttl range -module.exports = class FIFOTTLList extends EventEmitter { +module.exports = class TTLList extends EventEmitter { constructor ({ interval = 1000, expireAmount = 1, expireType = 'hours', } = {}) { super(); + this.interval = interval; this.expireAmount = expireAmount; this.expireType = expireType; @@ -20,10 +21,18 @@ module.exports = class FIFOTTLList extends EventEmitter { this.list.on('evicted', ({ value, ttl }) => { this.emit('expire', value, ttl); }); + this.startTimer(); + } - this.timer = setInterval(() => { - this.timedCheck(); - }, interval); + startTimer () { + if (this.list) { + this.timer = setTimeout(() => { + if (this.list) { + this.timedCheck(); + } + }, this.interval); + this.timer.unref(); + } } add (value, timestamp = new Date()) { @@ -34,11 +43,13 @@ module.exports = class FIFOTTLList extends EventEmitter { timedCheck () { const now = moment(new Date()); this.list.reverseRemoveUntilTrue(({ value }) => now.isBefore(value.ttl)); + this.startTimer(); } destroy () { - clearTimeout(this.timer); - delete this.timer; + // https://github.com/nodejs/node/issues/9561 + // clearTimeout(this.timer); + // this.timer = null; this.list = null; } }; diff --git a/lib/helper/legacy-feature-mapper.js b/lib/data-helper/legacy-feature-mapper.js similarity index 100% rename from lib/helper/legacy-feature-mapper.js rename to lib/data-helper/legacy-feature-mapper.js diff --git a/test/unit/helper/legacy-feature-mapper.test.js b/lib/data-helper/legacy-feature-mapper.test.js similarity index 96% rename from test/unit/helper/legacy-feature-mapper.test.js rename to lib/data-helper/legacy-feature-mapper.test.js index 0a181b5775..61cf455e98 100644 --- a/test/unit/helper/legacy-feature-mapper.test.js +++ b/lib/data-helper/legacy-feature-mapper.test.js @@ -1,7 +1,7 @@ 'use strict'; const test = require('ava'); -const mapper = require('../../../lib/helper/legacy-feature-mapper'); +const mapper = require('./legacy-feature-mapper'); test('adds old fields to feature', t => { const feature = { diff --git a/lib/routes/feature.js b/lib/routes/feature.js index 70c1653812..be2e18e28b 100644 --- a/lib/routes/feature.js +++ b/lib/routes/feature.js @@ -8,7 +8,7 @@ const ValidationError = require('../error/validation-error.js'); const validateRequest = require('../error/validate-request'); const extractUser = require('../extract-user'); -const legacyFeatureMapper = require('../helper/legacy-feature-mapper'); +const legacyFeatureMapper = require('../data-helper/legacy-feature-mapper'); const version = 1; const handleErrors = (req, res, error) => { diff --git a/lib/routes/metrics-schema.js b/lib/routes/metrics-schema.js index c24b6c1358..b660fad914 100644 --- a/lib/routes/metrics-schema.js +++ b/lib/routes/metrics-schema.js @@ -1,3 +1,5 @@ +'use strict'; + const joi = require('joi'); const clientMetricsSchema = joi.object().keys({ @@ -21,4 +23,4 @@ const clientRegisterSchema = joi.object().keys({ interval: joi.number().required(), }); -module.exports = { clientMetricsSchema, clientRegisterSchema } \ No newline at end of file +module.exports = { clientMetricsSchema, clientRegisterSchema } diff --git a/package.json b/package.json index 69f088d6d4..3c76a6ddac 100644 --- a/package.json +++ b/package.json @@ -39,7 +39,7 @@ "start:dev:pg-chain": "export DATABASE_URL=postgres://$PGUSER:$PGPASSWORD@localhost:$PGPORT/postgres ; db-migrate up && npm run start:dev", "db-migrate": "db-migrate up", "db-migrate:down": "db-migrate down", - "test": "PORT=4243 ava **/**/*test.js", + "test": "PORT=4243 ava test lib/*/*.test.js", "test:docker": "./scripts/docker-postgres.sh", "test:watch": "npm run test -- --watch", "test:pg-virtualenv": "pg_virtualenv npm run test:pg-virtualenv-chai", diff --git a/test/e2e/event-api.test.js b/test/e2e/event-api.test.js index 22a3221a25..46cbf5ec29 100644 --- a/test/e2e/event-api.test.js +++ b/test/e2e/event-api.test.js @@ -1,7 +1,7 @@ 'use strict'; const test = require('ava'); -const { setupApp } = require('./util/test-helper'); +const { setupApp } = require('./helpers/test-helper'); const logger = require('../../lib/logger'); test.beforeEach(() => { diff --git a/test/e2e/feature-api.test.js b/test/e2e/feature-api.test.js index a30bef0197..73f0dac032 100644 --- a/test/e2e/feature-api.test.js +++ b/test/e2e/feature-api.test.js @@ -1,7 +1,7 @@ 'use strict'; const { test } = require('ava'); -const { setupApp } = require('./util/test-helper'); +const { setupApp } = require('./helpers/test-helper'); const logger = require('../../lib/logger'); test.beforeEach(() => { diff --git a/test/e2e/feature-archive-api.test.js b/test/e2e/feature-archive-api.test.js index a3b5c95090..d1c6b46ddb 100644 --- a/test/e2e/feature-archive-api.test.js +++ b/test/e2e/feature-archive-api.test.js @@ -1,7 +1,7 @@ 'use strict'; const test = require('ava'); -const { setupApp } = require('./util/test-helper'); +const { setupApp } = require('./helpers/test-helper'); const logger = require('../../lib/logger'); test.beforeEach(() => { diff --git a/test/e2e/util/database-config.js b/test/e2e/helpers/database-config.js similarity index 100% rename from test/e2e/util/database-config.js rename to test/e2e/helpers/database-config.js diff --git a/test/e2e/util/test-helper.js b/test/e2e/helpers/test-helper.js similarity index 98% rename from test/e2e/util/test-helper.js rename to test/e2e/helpers/test-helper.js index ca5b2347c3..d82d1ca495 100644 --- a/test/e2e/util/test-helper.js +++ b/test/e2e/helpers/test-helper.js @@ -7,6 +7,7 @@ const migrator = require('../../../migrator'); const { createStores } = require('../../../lib/db'); const { createDb } = require('../../../lib/db/db-pool'); const _app = require('../../../app'); +require('db-migrate-shared').log.silence(true); // because of migrator bug delete process.env.DATABASE_URL; diff --git a/test/e2e/metrics-api.test.js b/test/e2e/metrics-api.test.js index b6f326607c..a4c91410d5 100644 --- a/test/e2e/metrics-api.test.js +++ b/test/e2e/metrics-api.test.js @@ -1,6 +1,6 @@ 'use strict'; const test = require('ava'); -const { setupApp } = require('./util/test-helper'); +const { setupApp } = require('./helpers/test-helper'); const logger = require('../../lib/logger'); test.beforeEach(() => { diff --git a/test/e2e/router.test.js b/test/e2e/router.test.js index b56a1e710e..d314011efa 100644 --- a/test/e2e/router.test.js +++ b/test/e2e/router.test.js @@ -1,7 +1,7 @@ 'use strict'; const test = require('ava'); -const { setupApp } = require('./util/test-helper'); +const { setupApp } = require('./helpers/test-helper'); const logger = require('../../lib/logger'); test.beforeEach(() => { diff --git a/test/e2e/strategy-api.test.js b/test/e2e/strategy-api.test.js index 53214e751c..9eee739d32 100644 --- a/test/e2e/strategy-api.test.js +++ b/test/e2e/strategy-api.test.js @@ -1,7 +1,7 @@ 'use strict'; const test = require('ava'); -const { setupApp } = require('./util/test-helper'); +const { setupApp } = require('./helpers/test-helper'); const logger = require('../../lib/logger'); test.beforeEach(() => { diff --git a/test/unit/routes/feature.test.js b/test/unit/routes/feature.test.js index 16d0ef5339..275c0ef8cb 100644 --- a/test/unit/routes/feature.test.js +++ b/test/unit/routes/feature.test.js @@ -1,7 +1,7 @@ 'use strict'; const test = require('ava'); -const store = require('./mocks/store'); +const store = require('./fixtures/store'); const supertest = require('supertest'); const logger = require('../../../lib/logger'); diff --git a/test/unit/routes/mocks/fake-client-instance-store.js b/test/unit/routes/fixtures/fake-client-instance-store.js similarity index 100% rename from test/unit/routes/mocks/fake-client-instance-store.js rename to test/unit/routes/fixtures/fake-client-instance-store.js diff --git a/test/unit/routes/mocks/fake-client-strategy-store.js b/test/unit/routes/fixtures/fake-client-strategy-store.js similarity index 100% rename from test/unit/routes/mocks/fake-client-strategy-store.js rename to test/unit/routes/fixtures/fake-client-strategy-store.js diff --git a/test/unit/routes/mocks/fake-feature-toggle-store.js b/test/unit/routes/fixtures/fake-feature-toggle-store.js similarity index 100% rename from test/unit/routes/mocks/fake-feature-toggle-store.js rename to test/unit/routes/fixtures/fake-feature-toggle-store.js diff --git a/test/unit/routes/mocks/fake-metrics-store.js b/test/unit/routes/fixtures/fake-metrics-store.js similarity index 100% rename from test/unit/routes/mocks/fake-metrics-store.js rename to test/unit/routes/fixtures/fake-metrics-store.js diff --git a/test/unit/routes/mocks/fake-strategies-store.js b/test/unit/routes/fixtures/fake-strategies-store.js similarity index 100% rename from test/unit/routes/mocks/fake-strategies-store.js rename to test/unit/routes/fixtures/fake-strategies-store.js diff --git a/test/unit/routes/mocks/store.js b/test/unit/routes/fixtures/store.js similarity index 100% rename from test/unit/routes/mocks/store.js rename to test/unit/routes/fixtures/store.js diff --git a/test/unit/routes/health-check.test.js b/test/unit/routes/health-check.test.js index 7b564f2c00..793bbf2fe2 100644 --- a/test/unit/routes/health-check.test.js +++ b/test/unit/routes/health-check.test.js @@ -1,7 +1,7 @@ 'use strict'; const test = require('ava'); -const store = require('./mocks/store'); +const store = require('./fixtures/store'); const supertest = require('supertest'); const logger = require('../../../lib/logger'); @@ -29,7 +29,6 @@ test('should give 500 when db is failing', t => { db.select = () => ({ from: () => Promise.reject(new Error('db error')), }); - return request .get('/health') .expect(500) diff --git a/test/unit/routes/metrics.test.js b/test/unit/routes/metrics.test.js index 1d9e555b3b..3466bf2292 100644 --- a/test/unit/routes/metrics.test.js +++ b/test/unit/routes/metrics.test.js @@ -1,7 +1,7 @@ 'use strict'; const test = require('ava'); -const store = require('./mocks/store'); +const store = require('./fixtures/store'); const supertest = require('supertest'); const logger = require('../../../lib/logger'); diff --git a/test/unit/routes/strategies.test.js b/test/unit/routes/strategies.test.js index 92edc91202..a7e2360dcb 100644 --- a/test/unit/routes/strategies.test.js +++ b/test/unit/routes/strategies.test.js @@ -1,7 +1,7 @@ 'use strict'; const test = require('ava'); -const store = require('./mocks/store'); +const store = require('./fixtures/store'); const supertest = require('supertest'); const logger = require('../../../lib/logger');