add tests, rename folders to ava defaults for helpers/fixtures, remove migration og

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({

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 () {
+    // toArrayReverse () {
-        const result = [];
+    //     const result = [];
 
-        if (this.tail) {
+    //     if (this.tail) {
-            let cursor = this.tail;
+    //         let cursor = this.tail;
-            while (cursor) {
+    //         while (cursor) {
-                result.push(cursor.value);
+    //             result.push(cursor.value);
-                cursor = cursor.prev;
+    //             cursor = cursor.prev;
-            }
+    //         }
-        }
+    //     }
 
-        return result;
+    //     return result;
-    }
+    // }
 };

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);
+});

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) {}
+    }
 };

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();
+        });
+    });
+});

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(() => {
+    startTimer () {
-            this.timedCheck();
+        if (this.list) {
-        }, interval);
+            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);
+        // https://github.com/nodejs/node/issues/9561
-        delete this.timer;
+        // clearTimeout(this.timer);
+        // this.timer = null;
         this.list = null;
     }
 };

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 = {

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) => {

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 }
+module.exports = { clientMetricsSchema, clientRegisterSchema }

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",

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(() =>  {

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(() =>  {

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(() =>  {

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;

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(() =>  {

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(() =>  {

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(() =>  {

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');
 

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)

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');
 

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');