diff --git a/package.json b/package.json index 14461aac9c..63c9f55988 100644 --- a/package.json +++ b/package.json @@ -81,7 +81,7 @@ "helmet": "^4.1.0", "joi": "^17.3.0", "js-yaml": "^3.14.0", - "knex": "0.21.15", + "knex": "0.95.2", "log4js": "^6.0.0", "memoizee": "^0.4.15", "mime": "^2.4.2", diff --git a/src/lib/db/access-store.ts b/src/lib/db/access-store.ts index 6a36a2ff5c..a2d3ca1c63 100644 --- a/src/lib/db/access-store.ts +++ b/src/lib/db/access-store.ts @@ -1,5 +1,5 @@ import { EventEmitter } from 'events'; -import Knex from 'knex'; +import { Knex } from 'knex'; import metricsHelper from '../metrics-helper'; import { DB_TIME } from '../events'; diff --git a/src/lib/db/client-applications-store.js b/src/lib/db/client-applications-store.js index b7f40a38f1..9680a9a56a 100644 --- a/src/lib/db/client-applications-store.js +++ b/src/lib/db/client-applications-store.js @@ -27,17 +27,27 @@ const mapRow = row => ({ icon: row.icon, }); -const remapRow = (input, old = {}) => ({ - app_name: input.appName, - updated_at: input.updatedAt, - description: input.description || old.description, - created_by: input.createdBy || old.createdBy, - announced: input.announced || old.announced || false, - url: input.url || old.url, - color: input.color || old.color, - icon: input.icon || old.icon, - strategies: JSON.stringify(input.strategies || old.strategies), -}); +const remapRow = input => { + const temp = { + app_name: input.appName, + updated_at: input.updatedAt || new Date(), + seen_at: input.lastSeen || new Date(), + description: input.description, + created_by: input.createdBy, + announced: input.announced, + url: input.url, + color: input.color, + icon: input.icon, + strategies: JSON.stringify(input.strategies), + }; + Object.keys(temp).forEach(k => { + if (temp[k] === undefined) { + // not using !temp[k] to allow false and null values to get through + delete temp[k]; + } + }); + return temp; +}; class ClientApplicationsDb { constructor(db, eventBus) { @@ -46,8 +56,9 @@ class ClientApplicationsDb { } async upsert(details) { + const row = remapRow(details); return this.db(TABLE) - .insert(remapRow(details)) + .insert(row) .onConflict('app_name') .merge(); } diff --git a/src/lib/db/event-store.ts b/src/lib/db/event-store.ts index cd3e857072..beccf3ad99 100644 --- a/src/lib/db/event-store.ts +++ b/src/lib/db/event-store.ts @@ -1,5 +1,5 @@ import { EventEmitter } from 'events'; -import Knex from 'knex'; +import { Knex } from 'knex'; import { DROP_FEATURES } from '../event-type'; import { LogProvider, Logger } from '../logger'; diff --git a/src/migrations/20210323073508-reset-application-announcements.js b/src/migrations/20210323073508-reset-application-announcements.js new file mode 100644 index 0000000000..e68d6d2f8a --- /dev/null +++ b/src/migrations/20210323073508-reset-application-announcements.js @@ -0,0 +1,11 @@ +'use strict'; + +exports.up = function(db, cb) { + db.runSql( + ` + DELETE FROM events WHERE type = 'application-created'; + UPDATE client_applications SET announced = false; + `, + cb, + ); +}; diff --git a/src/test/e2e/stores/client-application-store.e2e.test.js b/src/test/e2e/stores/client-application-store.e2e.test.js new file mode 100644 index 0000000000..661432679e --- /dev/null +++ b/src/test/e2e/stores/client-application-store.e2e.test.js @@ -0,0 +1,175 @@ +'use strict'; + +const test = require('ava'); +const faker = require('faker'); +const dbInit = require('../helpers/database-init'); +const getLogger = require('../../fixtures/no-logger'); + +let db; +let stores; +let clientApplicationsStore; + +test.beforeEach(async () => { + db = await dbInit('client_application_store_e2e_serial', getLogger); + stores = db.stores; + clientApplicationsStore = stores.clientApplicationsStore; +}); + +test.afterEach(async () => { + await db.destroy(); +}); + +test.serial("Should be able to keep track of what we've announced", async t => { + const clientRegistration = { + appName: faker.internet.domainName(), + instanceId: faker.random.uuid(), + strategies: ['default'], + started: Date.now(), + interval: faker.random.number(), + sdkVersion: '3.11.2', + icon: '', + description: faker.company.catchPhrase(), + color: faker.internet.color(), + }; + await clientApplicationsStore.upsert(clientRegistration); + let unannounced = await clientApplicationsStore.getUnannounced(); + t.is(unannounced.length, 1); + const announce = await clientApplicationsStore.setUnannouncedToAnnounced(); + t.is(announce.length, 1); + unannounced = await clientApplicationsStore.getUnannounced(); + t.is(unannounced.length, 0); +}); + +test.serial( + 'Multiple instances should still only announce once per app', + async t => { + const clientRegistration = { + appName: faker.internet.domainName(), + instanceId: faker.random.uuid(), + strategies: ['default'], + started: Date.now(), + interval: faker.random.number(), + sdkVersion: '3.11.2', + icon: '', + description: faker.company.catchPhrase(), + color: faker.internet.color(), + }; + const clientReg2 = { ...clientRegistration, instanceId: 'someotherid' }; + await clientApplicationsStore.upsert(clientRegistration); + await clientApplicationsStore.upsert(clientReg2); + let unannounced = await clientApplicationsStore.getUnannounced(); + t.is(unannounced.length, 1); + const announce = await clientApplicationsStore.setUnannouncedToAnnounced(); + t.is(announce.length, 1); + unannounced = await clientApplicationsStore.getUnannounced(); + t.is(unannounced.length, 0); + }, +); + +test.serial( + 'Multiple applications should also be possible to announce', + async t => { + const clients = []; + while (clients.length < 10) { + const clientRegistration = { + appName: `${faker.internet.domainName()}_${clients.length}`, + instanceId: faker.random.uuid(), + strategies: ['default'], + started: Date.now(), + interval: faker.random.number(), + sdkVersion: '3.11.2', + icon: '', + description: faker.company.catchPhrase(), + color: faker.internet.color(), + }; + clients.push(clientRegistration); + } + await clientApplicationsStore.bulkUpsert(clients); + let unannounced = await clientApplicationsStore.getUnannounced(); + t.is(unannounced.length, 10); + const announce = await clientApplicationsStore.setUnannouncedToAnnounced(); + t.is(announce.length, 10); + unannounced = await clientApplicationsStore.getUnannounced(); + t.is(unannounced.length, 0); + }, +); + +test.serial( + 'Same application registered multiple times should still only be announced once', + async t => { + const clientRegistration = { + appName: faker.internet.domainName(), + instanceId: faker.random.uuid(), + strategies: ['default'], + started: Date.now(), + interval: faker.random.number(), + sdkVersion: '3.11.2', + icon: '', + description: faker.company.catchPhrase(), + color: faker.internet.color(), + }; + await clientApplicationsStore.upsert(clientRegistration); + let unannounced = await clientApplicationsStore.getUnannounced(); + t.is(unannounced.length, 1); + let announce = await clientApplicationsStore.setUnannouncedToAnnounced(); + t.is(announce.length, 1); + unannounced = await clientApplicationsStore.getUnannounced(); + t.is(unannounced.length, 0); + + await clientApplicationsStore.upsert(clientRegistration); + announce = await clientApplicationsStore.setUnannouncedToAnnounced(); + t.is(announce.length, 0); + }, +); + +test.serial('Merge keeps value for single row in database', async t => { + const clientRegistration = { + appName: faker.internet.domainName(), + instanceId: faker.random.uuid(), + strategies: ['default'], + started: Date.now(), + icon: faker.internet.color(), + description: faker.company.catchPhrase(), + color: faker.internet.color(), + }; + await clientApplicationsStore.upsert(clientRegistration); + await clientApplicationsStore.upsert({ + appName: clientRegistration.appName, + description: 'new description', + }); + const stored = await clientApplicationsStore.getApplication( + clientRegistration.appName, + ); + t.is(stored.color, clientRegistration.color); + t.is(stored.description, 'new description'); +}); + +test.serial('Multi row merge also works', async t => { + const clients = []; + while (clients.length < 10) { + const clientRegistration = { + appName: `${faker.internet.domainName()}_${clients.length}`, + instanceId: faker.random.uuid(), + strategies: ['default'], + started: Date.now(), + icon: faker.internet.color(), + description: faker.company.catchPhrase(), + color: faker.internet.color(), + }; + clients.push(clientRegistration); + } + await clientApplicationsStore.bulkUpsert(clients); + const alteredClients = clients.map(c => { + return { appName: c.appName, icon: 'red' }; + }); + await clientApplicationsStore.bulkUpsert(alteredClients); + const stored = await Promise.all( + clients.map(async c => + clientApplicationsStore.getApplication(c.appName), + ), + ); + stored.forEach((s, i) => { + t.is(s.description, clients[i].description); + t.is(s.icon, 'red'); + }); +}); diff --git a/yarn.lock b/yarn.lock index aaa199b788..5eb2c3c402 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1442,6 +1442,11 @@ commander@^6.2.0: resolved "https://registry.npmjs.org/commander/-/commander-6.2.1.tgz" integrity sha512-U7VdrJFnJgo4xjrHpTzu0yrHPGImdsmD95ZlgYSEajAn2JKzDhDTPG9kBTefmObL2w/ngeZnilk+OV9CG3d7UA== +commander@^7.1.0: + version "7.2.0" + resolved "https://registry.yarnpkg.com/commander/-/commander-7.2.0.tgz#a36cb57d0b501ce108e4d20559a150a391d97ab7" + integrity sha512-QrWXB+ZQSVPmIWIhtEO9H+gwHaMGYiF5ChvoJ+K9ZGHG/sVsa6yiesAD1GC/x46sET00Xlwo1u49RVVVzvcSkw== + common-path-prefix@^3.0.0: version "3.0.0" resolved "https://registry.npmjs.org/common-path-prefix/-/common-path-prefix-3.0.0.tgz" @@ -2149,6 +2154,11 @@ escalade@^3.0.2: resolved "https://registry.npmjs.org/escalade/-/escalade-3.1.0.tgz" integrity sha512-mAk+hPSO8fLDkhV7V0dXazH5pDc6MrjBTPyD3VeKzxnVFjH1MIxbCdqGZB9O8+EwWakZs3ZCbDS4IpRt79V1ig== +escalade@^3.1.1: + version "3.1.1" + resolved "https://registry.yarnpkg.com/escalade/-/escalade-3.1.1.tgz#d8cfdc7000965c5a0174b4a82eaa5c0552742e40" + integrity sha512-k0er2gUkLf8O0zKJiAhmkTnJlTvINGv7ygDNPbeIsX/TJjGJZHuh9B2UxbsaEkmlEo9MfhrSzmhIlhRlI2GXnw== + escape-goat@^2.0.0: version "2.1.1" resolved "https://registry.npmjs.org/escape-goat/-/escape-goat-2.1.1.tgz" @@ -3452,6 +3462,13 @@ is-ci@^2.0.0: dependencies: ci-info "^2.0.0" +is-core-module@^2.2.0: + version "2.2.0" + resolved "https://registry.yarnpkg.com/is-core-module/-/is-core-module-2.2.0.tgz#97037ef3d52224d85163f5597b2b63d9afed981a" + integrity sha512-XRAfAdyyY5F5cOXn7hYQDqh2Xmii+DEfIcQGxK/uNwMHhIkPWO0g8msXcbzLe+MpGoR951MlqM/2iIlU4vKDdQ== + dependencies: + has "^1.0.3" + is-data-descriptor@^0.1.4: version "0.1.4" resolved "https://registry.npmjs.org/is-data-descriptor/-/is-data-descriptor-0.1.4.tgz" @@ -3928,23 +3945,24 @@ kind-of@^6.0.0, kind-of@^6.0.2, kind-of@^6.0.3: resolved "https://registry.yarnpkg.com/kind-of/-/kind-of-6.0.3.tgz#07c05034a6c349fa06e24fa35aa76db4580ce4dd" integrity sha512-dcS1ul+9tmeD95T+x28/ehLgd9mENa3LsvDTtzm3vyBEO7RPptvAD+t44WVXaUjTBRcrpFeFlC8WCruUR456hw== -knex@0.21.15: - version "0.21.15" - resolved "https://registry.npmjs.org/knex/-/knex-0.21.15.tgz" - integrity sha512-STHnPIIkExZVz0X3zIDaWC/q9EcTAfuuRE5Rev7NpOhF1QVh24K5iT2FXD0nWoJ1BUSeDs5QdgdTaz9H8oEtfg== +knex@0.95.2: + version "0.95.2" + resolved "https://registry.yarnpkg.com/knex/-/knex-0.95.2.tgz#ebc258b23c9b936c7c3c3f73bbada58b4e138059" + integrity sha512-USnukuNnoVAfeV6OXrkHPLcUQRsRbTl3Tk5E3x0hmKs4uEu5vGfZHxD4luyrndIysyDrKXBWfASQ1wFpJWDDFg== dependencies: colorette "1.2.1" - commander "^6.2.0" + commander "^7.1.0" debug "4.3.1" + escalade "^3.1.1" esm "^3.2.25" getopts "2.2.5" interpret "^2.2.0" - liftoff "3.1.0" - lodash "^4.17.20" + lodash "^4.17.21" pg-connection-string "2.4.0" + rechoir "^0.7.0" + resolve-from "^5.0.0" tarn "^3.0.1" tildify "2.0.0" - v8flags "^3.2.0" knex@^0.21.5: version "0.21.17" @@ -4112,6 +4130,11 @@ lodash@^4.17.11, lodash@^4.17.14, lodash@^4.17.15, lodash@^4.17.19, lodash@^4.17 resolved "https://registry.npmjs.org/lodash/-/lodash-4.17.20.tgz" integrity sha512-PlhdFcillOINfeV7Ni6oF1TAEayyZBoZ8bcshTHqOYJYlrqzRK5hagpagky5o4HfCzzd1TRkXPMFq6cKk9rGmA== +lodash@^4.17.21: + version "4.17.21" + resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.21.tgz#679591c564c3bffaae8454cf0b3df370c3d6911c" + integrity sha512-v2kDEe57lecTulaDIuNTPy3Ry4gLGJ6Z1O3vE1krgXZNrsQ+LFTGHVxVjcXPs17LhbZVGedAJv8XZ1tvj5FvSg== + log-driver@^1.2.7: version "1.2.7" resolved "https://registry.npmjs.org/log-driver/-/log-driver-1.2.7.tgz" @@ -5399,6 +5422,13 @@ rechoir@^0.6.2: dependencies: resolve "^1.1.6" +rechoir@^0.7.0: + version "0.7.0" + resolved "https://registry.yarnpkg.com/rechoir/-/rechoir-0.7.0.tgz#32650fd52c21ab252aa5d65b19310441c7e03aca" + integrity sha512-ADsDEH2bvbjltXEP+hTIAmeFekTFK0V2BTxMkok6qILyAJEXV0AFfoWcAq4yfll5VdIMd/RVXq0lR+wQi5ZU3Q== + dependencies: + resolve "^1.9.0" + regenerator-runtime@^0.13.4: version "0.13.7" resolved "https://registry.yarnpkg.com/regenerator-runtime/-/regenerator-runtime-0.13.7.tgz#cac2dacc8a1ea675feaabaeb8ae833898ae46f55" @@ -5526,6 +5556,14 @@ resolve@^1.1.6, resolve@^1.1.7, resolve@^1.10.0, resolve@^1.11.1, resolve@^1.13. dependencies: path-parse "^1.0.6" +resolve@^1.9.0: + version "1.20.0" + resolved "https://registry.yarnpkg.com/resolve/-/resolve-1.20.0.tgz#629a013fb3f70755d6f0b7935cc1c2c5378b1975" + integrity sha512-wENBPt4ySzg4ybFQW2TT1zMQucPK95HSh/nq2CFTZVOGut2+pQvSsgtda4d26YrYcr067wjbmzOG8byDPBX63A== + dependencies: + is-core-module "^2.2.0" + path-parse "^1.0.6" + response-time@^2.3.2: version "2.3.2" resolved "https://registry.npmjs.org/response-time/-/response-time-2.3.2.tgz"