From 0f4dd6b64a9d2a138d78887bfb6e5c441f8eda54 Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Sun, 10 Nov 2019 18:01:24 -0600 Subject: [PATCH] feat(server): automatically migrate outdated statistics fixes #4 --- packages/server/src/api/storage/sql/sql.js | 22 +++++++++++++++-- .../server/src/api/storage/storage-method.js | 17 +++++++++++-- packages/server/src/server.js | 3 ++- packages/server/test/postgres-server.test.js | 3 ++- packages/server/test/server-test-suite.js | 24 +++++++++++++++++++ packages/server/test/sqlite-server.test.js | 3 ++- packages/utils/src/api-client.js | 10 ++++++++ 7 files changed, 75 insertions(+), 7 deletions(-) diff --git a/packages/server/src/api/storage/sql/sql.js b/packages/server/src/api/storage/sql/sql.js index 867d0349d..c58c46fa9 100644 --- a/packages/server/src/api/storage/sql/sql.js +++ b/packages/server/src/api/storage/sql/sql.js @@ -114,6 +114,14 @@ function createUmzug(sequelize, options) { }); } +/** + * @param {LHCI.ServerCommand.Statistic} statistic + * @return {LHCI.ServerCommand.Statistic} + */ +function normalizeStatistic(statistic) { + return {...statistic, version: Number(statistic.version), value: Number(statistic.value)}; +} + /** @typedef {LHCI.ServerCommand.TableAttributes} ProjectAttrs */ /** @typedef {LHCI.ServerCommand.TableAttributes} BuildAttrs */ /** @typedef {LHCI.ServerCommand.TableAttributes} RunAttrs */ @@ -534,7 +542,7 @@ class SqlStorageMethod { statistic = await statisticModel.create({...unsavedStatistic, id: uuid.v4()}, {transaction}); } - return clone(statistic); + return normalizeStatistic(clone(statistic)); } /** @@ -545,7 +553,17 @@ class SqlStorageMethod { async _getStatistics(projectId, buildId) { const {statisticModel} = this._sql(); const statistics = await this._findAll(statisticModel, {where: {projectId, buildId}, order}); - return clone(statistics); + return clone(statistics).map(normalizeStatistic); + } + + /** + * @param {string} projectId + * @param {string} buildId + * @return {Promise} + */ + async _invalidateStatistics(projectId, buildId) { + const {statisticModel} = this._sql(); + await statisticModel.update({version: 0}, {where: {projectId, buildId}}); } } diff --git a/packages/server/src/api/storage/storage-method.js b/packages/server/src/api/storage/storage-method.js index a728bea8d..e8c09be75 100644 --- a/packages/server/src/api/storage/storage-method.js +++ b/packages/server/src/api/storage/storage-method.js @@ -199,6 +199,16 @@ class StorageMethod { throw new Error('Unimplemented'); } + /** + * @param {string} projectId + * @param {string} buildId + * @return {Promise} + */ + // eslint-disable-next-line no-unused-vars + async _invalidateStatistics(projectId, buildId) { + throw new Error('Unimplemented'); + } + /** * @param {StorageMethod} storageMethod * @param {string} projectId @@ -214,7 +224,10 @@ class StorageMethod { const urls = await storageMethod.getUrls(projectId, buildId); const statisicDefinitionEntries = Object.entries(statisticDefinitions); const existingStatistics = await storageMethod._getStatistics(projectId, buildId); - if (existingStatistics.length === urls.length * statisicDefinitionEntries.length) { + if ( + existingStatistics.length === urls.length * statisicDefinitionEntries.length && + existingStatistics.every(stat => stat.version === STATISTIC_VERSION) + ) { return existingStatistics; } @@ -250,7 +263,7 @@ class StorageMethod { const existing = existingStatistics.find( s => s.name === name && s.value === value && s.url === url ); - if (existing) return existing; + if (existing && existing.version === STATISTIC_VERSION) return existing; return storageMethod._createOrUpdateStatistic( { diff --git a/packages/server/src/server.js b/packages/server/src/server.js index b74dc3bb6..9f7300e58 100644 --- a/packages/server/src/server.js +++ b/packages/server/src/server.js @@ -50,7 +50,7 @@ async function createApp(options) { /** * @param {LHCI.ServerCommand.Options} options - * @return {Promise<{port: number, close: () => void}>} + * @return {Promise<{port: number, close: () => void, storageMethod: StorageMethod}>} */ async function createServer(options) { const {app, storageMethod} = await createApp(options); @@ -63,6 +63,7 @@ async function createServer(options) { typeof serverAddress === 'string' || !serverAddress ? options.port : serverAddress.port; resolve({ + storageMethod, port: listenPort, close: () => { server.close(); diff --git a/packages/server/test/postgres-server.test.js b/packages/server/test/postgres-server.test.js index dbfca7b96..61ec7552b 100644 --- a/packages/server/test/postgres-server.test.js +++ b/packages/server/test/postgres-server.test.js @@ -22,7 +22,7 @@ describe('postgres server', () => { }; beforeAll(async () => { - const {port, close} = await runServer({ + const {port, close, storageMethod} = await runServer({ logLevel: 'silent', port: 0, storage: { @@ -36,6 +36,7 @@ describe('postgres server', () => { state.port = port; state.closeServer = close; + state.storageMethod = storageMethod; }); afterAll(() => { diff --git a/packages/server/test/server-test-suite.js b/packages/server/test/server-test-suite.js index 7d46b7dcb..d695ff37f 100644 --- a/packages/server/test/server-test-suite.js +++ b/packages/server/test/server-test-suite.js @@ -666,6 +666,30 @@ function runTests(state) { }, ]); }); + + it('should not recompute on every call', async () => { + const stat1 = await client.getStatistics(projectA.id, buildA.id); + const stat2 = await client.getStatistics(projectA.id, buildA.id); + + for (const pre of stat1) { + const post = stat2.find(stat => stat.url === pre.url && stat.name === pre.name); + expect(post).toEqual(pre); // ensure that updatedAt has not changed + } + }); + + it('should invalidate statistics and get updated statistics', async () => { + const preInvalidated = await client.getStatistics(projectA.id, buildA.id); + await state.storageMethod._invalidateStatistics(projectA.id, buildA.id); + const postInvalidated = await client.getStatistics(projectA.id, buildA.id); + + for (const pre of preInvalidated) { + const post = postInvalidated.find(stat => stat.url === pre.url && stat.name === pre.name); + expect({...post, updatedAt: ''}).toEqual({...pre, updatedAt: ''}); + expect(new Date(post.updatedAt).getTime()).toBeGreaterThan( + new Date(pre.updatedAt).getTime() + ); + } + }); }); describe('/:projectId/urls', () => { diff --git a/packages/server/test/sqlite-server.test.js b/packages/server/test/sqlite-server.test.js index f07daef18..8e11cc1f8 100644 --- a/packages/server/test/sqlite-server.test.js +++ b/packages/server/test/sqlite-server.test.js @@ -23,7 +23,7 @@ describe('sqlite server', () => { beforeAll(async () => { if (fs.existsSync(dbPath)) fs.unlinkSync(dbPath); - const {port, close} = await runServer({ + const {port, close, storageMethod} = await runServer({ logLevel: 'silent', port: 0, storage: { @@ -35,6 +35,7 @@ describe('sqlite server', () => { state.port = port; state.closeServer = close; + state.storageMethod = storageMethod; }); afterAll(() => { diff --git a/packages/utils/src/api-client.js b/packages/utils/src/api-client.js index 1f04bb0e4..033544e90 100644 --- a/packages/utils/src/api-client.js +++ b/packages/utils/src/api-client.js @@ -299,6 +299,16 @@ class ApiClient { throw new Error('Unimplemented'); } + /** + * @param {string} projectId + * @param {string} buildId + * @return {Promise} + */ + // eslint-disable-next-line no-unused-vars + async _invalidateStatistics(projectId, buildId) { + throw new Error('Unimplemented'); + } + async close() {} }