From 607b2a6657bbf5df27a1b85fe3a9eedca1b484c7 Mon Sep 17 00:00:00 2001 From: Martin Lehmann Date: Tue, 26 Oct 2021 20:13:30 +0200 Subject: [PATCH] fix: use date-fns for date/time maths instead of (wrong) Date#setHours (#1070) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Ivar Conradi Østhus --- package.json | 1 + src/lib/db/client-metrics-store-v2.ts | 12 ++++------ .../e2e/api/admin/client-metrics.e2e.test.ts | 16 +++++++------- .../client-metrics-store-v2.e2e.test.ts | 22 ++++--------------- yarn.lock | 5 +++++ 5 files changed, 22 insertions(+), 34 deletions(-) diff --git a/package.json b/package.json index 51a51e8070..aabf90de90 100644 --- a/package.json +++ b/package.json @@ -81,6 +81,7 @@ "cookie-parser": "^1.4.5", "cookie-session": "^2.0.0-rc.1", "cors": "^2.8.5", + "date-fns": "^2.25.0", "db-migrate": "0.11.12", "db-migrate-pg": "1.2.2", "db-migrate-shared": "1.2.0", diff --git a/src/lib/db/client-metrics-store-v2.ts b/src/lib/db/client-metrics-store-v2.ts index e3ac091369..dfc8494f52 100644 --- a/src/lib/db/client-metrics-store-v2.ts +++ b/src/lib/db/client-metrics-store-v2.ts @@ -1,4 +1,3 @@ -/* eslint-disable @typescript-eslint/no-unused-vars */ import { Knex } from 'knex'; import { Logger, LogProvider } from '../logger'; import { @@ -7,6 +6,7 @@ import { IClientMetricsStoreV2, } from '../types/stores/client-metrics-store-v2'; import NotFoundError from '../error/notfound-error'; +import { startOfHour } from 'date-fns'; interface ClientMetricsEnvTable { feature_name: string; @@ -19,10 +19,6 @@ interface ClientMetricsEnvTable { const TABLE = 'client_metrics_env'; -export function roundDownToHour(date: Date): Date { - return new Date(date.getTime() - (date.getTime() % 3600000)); -} - const fromRow = (row: ClientMetricsEnvTable) => ({ featureName: row.feature_name, appName: row.app_name, @@ -36,7 +32,7 @@ const toRow = (metric: IClientMetricsEnv) => ({ feature_name: metric.featureName, app_name: metric.appName, environment: metric.environment, - timestamp: roundDownToHour(metric.timestamp), + timestamp: startOfHour(metric.timestamp), yes: metric.yes, no: metric.no, }); @@ -57,7 +53,7 @@ export class ClientMetricsStoreV2 implements IClientMetricsStoreV2 { feature_name: key.featureName, app_name: key.appName, environment: key.environment, - timestamp: roundDownToHour(key.timestamp), + timestamp: startOfHour(key.timestamp), }) .first(); if (row) { @@ -88,7 +84,7 @@ export class ClientMetricsStoreV2 implements IClientMetricsStoreV2 { feature_name: key.featureName, app_name: key.appName, environment: key.environment, - timestamp: roundDownToHour(key.timestamp), + timestamp: startOfHour(key.timestamp), }) .del(); } diff --git a/src/test/e2e/api/admin/client-metrics.e2e.test.ts b/src/test/e2e/api/admin/client-metrics.e2e.test.ts index 088cfc7af7..6558c076a8 100644 --- a/src/test/e2e/api/admin/client-metrics.e2e.test.ts +++ b/src/test/e2e/api/admin/client-metrics.e2e.test.ts @@ -2,6 +2,7 @@ import dbInit, { ITestDb } from '../../helpers/database-init'; import { setupAppWithCustomConfig } from '../../helpers/test-helper'; import getLogger from '../../../fixtures/no-logger'; import { IClientMetricsEnv } from '../../../../lib/types/stores/client-metrics-store-v2'; +import { subHours } from 'date-fns'; let app; let db: ITestDb; @@ -166,15 +167,14 @@ test('should return toggle summary', async () => { }); test('should only include last hour of metrics return toggle summary', async () => { - const date = new Date(); - const dateHoneHourAgo = new Date(); - dateHoneHourAgo.setHours(-1); + const now = new Date(); + const dateOneHourAgo = subHours(now, 1); const metrics: IClientMetricsEnv[] = [ { featureName: 'demo', appName: 'web', environment: 'default', - timestamp: date, + timestamp: now, yes: 2, no: 2, }, @@ -182,7 +182,7 @@ test('should only include last hour of metrics return toggle summary', async () featureName: 'demo', appName: 'web', environment: 'default', - timestamp: date, + timestamp: now, yes: 3, no: 2, }, @@ -190,7 +190,7 @@ test('should only include last hour of metrics return toggle summary', async () featureName: 'demo', appName: 'web', environment: 'test', - timestamp: date, + timestamp: now, yes: 1, no: 3, }, @@ -198,7 +198,7 @@ test('should only include last hour of metrics return toggle summary', async () featureName: 'demo', appName: 'backend-api', environment: 'test', - timestamp: date, + timestamp: now, yes: 1, no: 3, }, @@ -206,7 +206,7 @@ test('should only include last hour of metrics return toggle summary', async () featureName: 'demo', appName: 'backend-api', environment: 'test', - timestamp: dateHoneHourAgo, + timestamp: dateOneHourAgo, yes: 55, no: 55, }, diff --git a/src/test/e2e/stores/client-metrics-store-v2.e2e.test.ts b/src/test/e2e/stores/client-metrics-store-v2.e2e.test.ts index 5afb48edcb..f9619547ef 100644 --- a/src/test/e2e/stores/client-metrics-store-v2.e2e.test.ts +++ b/src/test/e2e/stores/client-metrics-store-v2.e2e.test.ts @@ -1,3 +1,4 @@ +import { subDays } from 'date-fns'; import dbInit from '../helpers/database-init'; import getLogger from '../../fixtures/no-logger'; import { IUnleashStores } from '../../../lib/types'; @@ -5,7 +6,6 @@ import { IClientMetricsEnv, IClientMetricsStoreV2, } from '../../../lib/types/stores/client-metrics-store-v2'; -import { roundDownToHour } from '../../../lib/db/client-metrics-store-v2'; let db; let stores: IUnleashStores; @@ -265,8 +265,7 @@ test('Should not fail on undefined list of metrics', async () => { }); test('Should return delete old metric', async () => { - const twoDaysAgo = new Date(); - twoDaysAgo.setHours(-48); + const twoDaysAgo = subDays(new Date(), 2); const metrics: IClientMetricsEnv[] = [ { @@ -312,8 +311,7 @@ test('Should return delete old metric', async () => { }); test('Should get metric', async () => { - const twoDaysAgo = new Date(); - twoDaysAgo.setHours(-48); + const twoDaysAgo = subDays(new Date(), 2); const metrics: IClientMetricsEnv[] = [ { @@ -362,7 +360,7 @@ test('Should get metric', async () => { expect(metric.no).toBe(42); }); -test('Should not exists after delete', async () => { +test('Should not exist after delete', async () => { const metric = { featureName: 'demo4', appName: 'backend-api', @@ -383,15 +381,3 @@ test('Should not exists after delete', async () => { const existAfter = await clientMetricsStore.exists(metric); expect(existAfter).toBe(false); }); - -test('should floor hours as expected', () => { - expect( - roundDownToHour(new Date('2019-11-12T08:44:32.499Z')).toISOString(), - ).toBe('2019-11-12T08:00:00.000Z'); - expect( - roundDownToHour(new Date('2019-11-12T08:59:59.999Z')).toISOString(), - ).toBe('2019-11-12T08:00:00.000Z'); - expect( - roundDownToHour(new Date('2019-11-12T09:01:00.999Z')).toISOString(), - ).toBe('2019-11-12T09:00:00.000Z'); -}); diff --git a/yarn.lock b/yarn.lock index 8c835133c3..4003e4b5f8 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1961,6 +1961,11 @@ data-urls@^2.0.0: whatwg-mimetype "^2.3.0" whatwg-url "^8.0.0" +date-fns@^2.25.0: + version "2.25.0" + resolved "https://registry.yarnpkg.com/date-fns/-/date-fns-2.25.0.tgz#8c5c8f1d958be3809a9a03f4b742eba894fc5680" + integrity sha512-ovYRFnTrbGPD4nqaEqescPEv1mNwvt+UTqI3Ay9SzNtey9NZnYu6E2qCcBBgJ6/2VF1zGGygpyTDITqpQQ5e+w== + date-format@^2.1.0: version "2.1.0" resolved "https://registry.npmjs.org/date-format/-/date-format-2.1.0.tgz"