From bb6f4f4bd7e71f1b346568297821d9dfabadeb02 Mon Sep 17 00:00:00 2001 From: Kyle Watson Date: Wed, 29 Nov 2023 11:40:43 +0100 Subject: [PATCH] refactor(backstage-plugin): Refactor data service Reuse request logic in the data service. Rename data service from GroupDataService to DoraDataService. Reorganise tests to describe them better. Rename test to describe what is being tested. Addresses #71 --- .../plugins/open-dora/dev/index.tsx | 12 +- .../DashboardComponent.test.tsx | 10 +- .../src/hooks/MetricBenchmarkHook.ts | 8 +- .../open-dora/src/hooks/MetricDataHook.ts | 8 +- .../plugins/open-dora/src/plugin.ts | 10 +- .../src/services/DoraDataService.test.ts | 201 ++++++++++++++++++ ...GroupDataService.ts => DoraDataService.ts} | 46 ++-- .../src/services/GroupDataService.test.ts | 199 ----------------- 8 files changed, 242 insertions(+), 252 deletions(-) create mode 100644 backstage-plugin/plugins/open-dora/src/services/DoraDataService.test.ts rename backstage-plugin/plugins/open-dora/src/services/{GroupDataService.ts => DoraDataService.ts} (62%) delete mode 100644 backstage-plugin/plugins/open-dora/src/services/GroupDataService.test.ts diff --git a/backstage-plugin/plugins/open-dora/dev/index.tsx b/backstage-plugin/plugins/open-dora/dev/index.tsx index 6294fdb..3c1c562 100644 --- a/backstage-plugin/plugins/open-dora/dev/index.tsx +++ b/backstage-plugin/plugins/open-dora/dev/index.tsx @@ -1,11 +1,11 @@ import { createDevApp } from '@backstage/dev-utils'; +import { MockConfigApi } from '@backstage/test-utils'; import React from 'react'; import { openDoraPlugin, OpenDoraPluginPage } from '../src'; -import { MockConfigApi } from '@backstage/test-utils'; import { - GroupDataService, - groupDataServiceApiRef, -} from '../src/services/GroupDataService'; + DoraDataService, + doraDataServiceApiRef, +} from '../src/services/DoraDataService'; const mockConfig = new MockConfigApi({ 'open-dora': { @@ -16,9 +16,9 @@ const mockConfig = new MockConfigApi({ createDevApp() .registerPlugin(openDoraPlugin) .registerApi({ - api: groupDataServiceApiRef, + api: doraDataServiceApiRef, deps: {}, - factory: () => new GroupDataService({ configApi: mockConfig }), + factory: () => new DoraDataService({ configApi: mockConfig }), }) .addPage({ element: , diff --git a/backstage-plugin/plugins/open-dora/src/components/DashboardComponent/DashboardComponent.test.tsx b/backstage-plugin/plugins/open-dora/src/components/DashboardComponent/DashboardComponent.test.tsx index 1bf5dbb..0415afa 100644 --- a/backstage-plugin/plugins/open-dora/src/components/DashboardComponent/DashboardComponent.test.tsx +++ b/backstage-plugin/plugins/open-dora/src/components/DashboardComponent/DashboardComponent.test.tsx @@ -11,9 +11,9 @@ import { rest } from 'msw'; import React from 'react'; import { baseUrl, metricUrl } from '../../../testing/mswHandlers'; import { - GroupDataService, - groupDataServiceApiRef, -} from '../../services/GroupDataService'; + DoraDataService, + doraDataServiceApiRef, +} from '../../services/DoraDataService'; import { server } from '../../setupTests'; import { DashboardComponent, @@ -26,8 +26,8 @@ async function renderComponentWithApis(component: JSX.Element) { }); const apiRegistry = TestApiRegistry.from([ - groupDataServiceApiRef, - new GroupDataService({ configApi: mockConfig }), + doraDataServiceApiRef, + new DoraDataService({ configApi: mockConfig }), ]); return await renderInTestApp( diff --git a/backstage-plugin/plugins/open-dora/src/hooks/MetricBenchmarkHook.ts b/backstage-plugin/plugins/open-dora/src/hooks/MetricBenchmarkHook.ts index b3679d5..23a382a 100644 --- a/backstage-plugin/plugins/open-dora/src/hooks/MetricBenchmarkHook.ts +++ b/backstage-plugin/plugins/open-dora/src/hooks/MetricBenchmarkHook.ts @@ -1,18 +1,18 @@ import { useApi } from '@backstage/core-plugin-api'; import { useEffect, useState } from 'react'; import { dfBenchmarkKey } from '../models/DfBenchmarkData'; -import { groupDataServiceApiRef } from '../services/GroupDataService'; +import { doraDataServiceApiRef } from '../services/DoraDataService'; export const useMetricBenchmark = (type: string) => { - const groupDataService = useApi(groupDataServiceApiRef); + const doraDataService = useApi(doraDataServiceApiRef); const [benchmark, setDfBenchmark] = useState(); const [error, setError] = useState(); useEffect(() => { - groupDataService.retrieveBenchmarkData({ type: type }).then(response => { + doraDataService.retrieveBenchmarkData({ type: type }).then(response => { setDfBenchmark(response.key); }, setError); - }, [groupDataService, type]); + }, [doraDataService, type]); return { error: error, benchmark: benchmark }; }; diff --git a/backstage-plugin/plugins/open-dora/src/hooks/MetricDataHook.ts b/backstage-plugin/plugins/open-dora/src/hooks/MetricDataHook.ts index 99c2a5d..ba4de67 100644 --- a/backstage-plugin/plugins/open-dora/src/hooks/MetricDataHook.ts +++ b/backstage-plugin/plugins/open-dora/src/hooks/MetricDataHook.ts @@ -1,17 +1,17 @@ import { useApi } from '@backstage/core-plugin-api'; import { useContext, useEffect, useState } from 'react'; import { MetricData } from '../models/MetricData'; -import { groupDataServiceApiRef } from '../services/GroupDataService'; +import { doraDataServiceApiRef } from '../services/DoraDataService'; import { MetricContext } from '../services/MetricContext'; export const useMetricData = (type: string) => { - const groupDataService = useApi(groupDataServiceApiRef); + const doraDataService = useApi(doraDataServiceApiRef); const [chartData, setChartData] = useState(); const [error, setError] = useState(); const { aggregation, team, project } = useContext(MetricContext); useEffect(() => { - groupDataService + doraDataService .retrieveMetricDataPoints({ type: type, team: team, @@ -25,7 +25,7 @@ export const useMetricData = (type: string) => { setError(new Error('No data found')); } }, setError); - }, [aggregation, team, project, groupDataService, type]); + }, [aggregation, team, project, doraDataService, type]); return { error: error, chartData: chartData }; }; diff --git a/backstage-plugin/plugins/open-dora/src/plugin.ts b/backstage-plugin/plugins/open-dora/src/plugin.ts index 014ab1a..21acf70 100644 --- a/backstage-plugin/plugins/open-dora/src/plugin.ts +++ b/backstage-plugin/plugins/open-dora/src/plugin.ts @@ -6,9 +6,9 @@ import { } from '@backstage/core-plugin-api'; import { rootRouteRef } from './routes'; import { - GroupDataService, - groupDataServiceApiRef, -} from './services/GroupDataService'; + DoraDataService, + doraDataServiceApiRef, +} from './services/DoraDataService'; export const openDoraPlugin = createPlugin({ id: 'opendora', @@ -17,9 +17,9 @@ export const openDoraPlugin = createPlugin({ }, apis: [ createApiFactory({ - api: groupDataServiceApiRef, + api: doraDataServiceApiRef, deps: { configApi: configApiRef }, - factory: ({ configApi }) => new GroupDataService({ configApi }), + factory: ({ configApi }) => new DoraDataService({ configApi }), }), ], }); diff --git a/backstage-plugin/plugins/open-dora/src/services/DoraDataService.test.ts b/backstage-plugin/plugins/open-dora/src/services/DoraDataService.test.ts new file mode 100644 index 0000000..02236fc --- /dev/null +++ b/backstage-plugin/plugins/open-dora/src/services/DoraDataService.test.ts @@ -0,0 +1,201 @@ +import { MockConfigApi } from '@backstage/test-utils'; +import { rest } from 'msw'; +import { baseUrl, benchmarkUrl, metricUrl } from '../../testing/mswHandlers'; +import { server } from '../setupTests'; +import { DoraDataService } from './DoraDataService'; + +function createService() { + server.use( + rest.get(benchmarkUrl, (req, res, ctx) => { + const params = req.url.searchParams; + const type = params.get('type'); + + switch (type) { + case 'df': { + return res( + ctx.json({ + key: 'lt-6month', + }), + ); + } + default: { + return res(ctx.status(400)); + } + } + }), + rest.get(metricUrl, (req, res, ctx) => { + const params = req.url.searchParams; + const type = params.get('type'); + const aggregation = params.get('aggregation'); + const project = params.get('project'); + const team = params.get('team'); + + return res( + ctx.json({ + aggregation: aggregation || 'weekly', + dataPoints: [ + { + key: `${project}_${team}_${aggregation}_${type}_first_key`, + value: 2.3, + }, + ], + }), + ); + }), + ); + const mockConfig = new MockConfigApi({ + 'open-dora': { apiBaseUrl: baseUrl }, + }); + + return new DoraDataService({ configApi: mockConfig }); +} + +describe('DoraDataService', () => { + describe('retriveMetricDataPoints', () => { + it('should return data from the server', async () => { + const service = createService(); + + expect( + await service.retrieveMetricDataPoints({ type: 'df_count' }), + ).toEqual({ + aggregation: 'weekly', + dataPoints: [{ key: 'null_null_null_df_count_first_key', value: 2.3 }], + }); + }); + + it('should use provided details in the query parameters', async () => { + const service = createService(); + + expect( + await service.retrieveMetricDataPoints({ + type: 'df_count', + aggregation: 'monthly', + project: 'project1', + team: 'team1', + }), + ).toEqual({ + aggregation: 'monthly', + dataPoints: [ + { key: 'project1_team1_monthly_df_count_first_key', value: 2.3 }, + ], + }); + }); + + it('should throw an error if the response does not contain metric data', async () => { + const service = createService(); + + server.use( + rest.get(metricUrl, (_, res, ctx) => { + return res(ctx.json({ other: 'data' })); + }), + ); + await expect( + service.retrieveMetricDataPoints({ + type: 'df_count', + }), + ).rejects.toEqual(new Error('Unexpected response')); + }); + + it('should throw an error when the server is unreachable', async () => { + const service = createService(); + + server.use( + rest.get(metricUrl, (_, res) => { + return res.networkError('Host unreachable'); + }), + ); + await expect( + service.retrieveMetricDataPoints({ + type: 'df_count', + }), + ).rejects.toEqual(new Error('Failed to fetch')); + }); + + it('should throw an error when the server returns a non-ok status', async () => { + const service = createService(); + + server.use( + rest.get(metricUrl, (_, res, ctx) => { + return res(ctx.status(401)); + }), + ); + await expect( + service.retrieveMetricDataPoints({ + type: 'df_count', + }), + ).rejects.toEqual(new Error('Unauthorized')); + + server.use( + rest.get(metricUrl, (_, res, ctx) => { + return res(ctx.status(500)); + }), + ); + await expect( + service.retrieveMetricDataPoints({ + type: 'df_count', + }), + ).rejects.toEqual(new Error('Internal Server Error')); + }); + }); + + describe('retrieveBenchmarkData', () => { + it('should return deployment frequency overall data from the server', async () => { + const service = createService(); + + expect(await service.retrieveBenchmarkData({ type: 'df' })).toEqual({ + key: 'lt-6month', + }); + }); + + it('should throw an error if the response does not contain metric data', async () => { + const service = createService(); + + server.use( + rest.get(benchmarkUrl, (_, res, ctx) => { + return res(ctx.json({ other: 'data' })); + }), + ); + await expect( + service.retrieveBenchmarkData({ + type: 'df', + }), + ).rejects.toEqual(new Error('Unexpected response')); + }); + + it('should send the params in the request', async () => { + const service = createService(); + + await expect( + service.retrieveBenchmarkData({ + type: 'invalid_type', + }), + ).rejects.toEqual(new Error('Bad Request')); + }); + + it('should throw an error when the server returns a non-ok status', async () => { + const service = createService(); + + server.use( + rest.get(benchmarkUrl, (_, res, ctx) => { + return res(ctx.status(401)); + }), + ); + await expect( + service.retrieveBenchmarkData({ + type: 'df', + }), + ).rejects.toEqual(new Error('Unauthorized')); + + server.use( + rest.get(benchmarkUrl, (_, res, ctx) => { + return res(ctx.status(500)); + }), + ); + await expect( + service.retrieveBenchmarkData({ + type: 'df', + }), + ).rejects.toEqual(new Error('Internal Server Error')); + }); + }); +}); diff --git a/backstage-plugin/plugins/open-dora/src/services/GroupDataService.ts b/backstage-plugin/plugins/open-dora/src/services/DoraDataService.ts similarity index 62% rename from backstage-plugin/plugins/open-dora/src/services/GroupDataService.ts rename to backstage-plugin/plugins/open-dora/src/services/DoraDataService.ts index 15b6496..d4fd5c1 100644 --- a/backstage-plugin/plugins/open-dora/src/services/GroupDataService.ts +++ b/backstage-plugin/plugins/open-dora/src/services/DoraDataService.ts @@ -1,24 +1,19 @@ import { ConfigApi, createApiRef } from '@backstage/core-plugin-api'; -import { MetricData } from '../models/MetricData'; import { dfBenchmarkData } from '../models/DfBenchmarkData'; +import { MetricData } from '../models/MetricData'; -export const groupDataServiceApiRef = createApiRef({ - id: 'plugin.open-dora.group-data', +export const doraDataServiceApiRef = createApiRef({ + id: 'plugin.open-dora.data-service', }); -export class GroupDataService { +export class DoraDataService { constructor(private options: { configApi: ConfigApi }) {} - async retrieveMetricDataPoints(params: { - type: string; - team?: string; - project?: string; - aggregation?: string; - }) { + private async retrieveData(params: Record, path: string) { const baseUrl = this.options.configApi.getString('open-dora.apiBaseUrl'); const url = new URL(baseUrl); - url.pathname = 'dora/api/metric'; + url.pathname = path; for (const [key, value] of Object.entries(params)) { if (value) { url.searchParams.append(key, value); @@ -32,7 +27,16 @@ export class GroupDataService { throw new Error(data.statusText); } - const response = await data.json(); + return await data.json(); + } + + async retrieveMetricDataPoints(params: { + type: string; + team?: string; + project?: string; + aggregation?: string; + }) { + const response = await this.retrieveData(params, 'dora/api/metric'); if ( response.aggregation === undefined || response.dataPoints === undefined @@ -44,24 +48,8 @@ export class GroupDataService { } async retrieveBenchmarkData(params: { type: string }) { - const baseUrl = this.options.configApi.getString('open-dora.apiBaseUrl'); - - const url = new URL(baseUrl); - url.pathname = 'dora/api/benchmark'; - for (const [key, value] of Object.entries(params)) { - if (value) { - url.searchParams.append(key, value); - } - } - const data = await fetch(url.toString(), { - method: 'GET', - }); - - if (!data.ok) { - throw new Error(data.statusText); - } + const response = await this.retrieveData(params, 'dora/api/benchmark'); - const response = await data.json(); if (response.key === undefined) { throw new Error('Unexpected response'); } diff --git a/backstage-plugin/plugins/open-dora/src/services/GroupDataService.test.ts b/backstage-plugin/plugins/open-dora/src/services/GroupDataService.test.ts deleted file mode 100644 index 2e3a67a..0000000 --- a/backstage-plugin/plugins/open-dora/src/services/GroupDataService.test.ts +++ /dev/null @@ -1,199 +0,0 @@ -import { MockConfigApi } from '@backstage/test-utils'; -import { rest } from 'msw'; -import { baseUrl, benchmarkUrl, metricUrl } from '../../testing/mswHandlers'; -import { server } from '../setupTests'; -import { GroupDataService } from './GroupDataService'; - -function createService() { - server.use( - rest.get(benchmarkUrl, (req, res, ctx) => { - const params = req.url.searchParams; - const type = params.get('type'); - - switch (type) { - case 'df': { - return res( - ctx.json({ - key: 'lt-6month', - }), - ); - } - default: { - return res(ctx.status(400)); - } - } - }), - rest.get(metricUrl, (req, res, ctx) => { - const params = req.url.searchParams; - const type = params.get('type'); - const aggregation = params.get('aggregation'); - const project = params.get('project'); - const team = params.get('team'); - - return res( - ctx.json({ - aggregation: aggregation || 'weekly', - dataPoints: [ - { - key: `${project}_${team}_${aggregation}_${type}_first_key`, - value: 2.3, - }, - ], - }), - ); - }), - ); - const mockConfig = new MockConfigApi({ - 'open-dora': { apiBaseUrl: baseUrl }, - }); - - return new GroupDataService({ configApi: mockConfig }); -} - -describe('GroupDataService', () => { - it('should return data from the server', async () => { - const service = createService(); - - expect( - await service.retrieveMetricDataPoints({ type: 'df_count' }), - ).toEqual({ - aggregation: 'weekly', - dataPoints: [{ key: 'null_null_null_df_count_first_key', value: 2.3 }], - }); - }); - - it('should use provided details in the query parameters', async () => { - const service = createService(); - - expect( - await service.retrieveMetricDataPoints({ - type: 'df_count', - aggregation: 'monthly', - project: 'project1', - team: 'team1', - }), - ).toEqual({ - aggregation: 'monthly', - dataPoints: [ - { key: 'project1_team1_monthly_df_count_first_key', value: 2.3 }, - ], - }); - }); - - it('should throw an error if the response does not contain metric data', async () => { - const service = createService(); - - server.use( - rest.get(metricUrl, (_, res, ctx) => { - return res(ctx.json({ other: 'data' })); - }), - ); - await expect( - service.retrieveMetricDataPoints({ - type: 'df_count', - }), - ).rejects.toEqual(new Error('Unexpected response')); - }); - - it('should throw an error when the server is unreachable', async () => { - const service = createService(); - - server.use( - rest.get(metricUrl, (_, res) => { - return res.networkError('Host unreachable'); - }), - ); - await expect( - service.retrieveMetricDataPoints({ - type: 'df_count', - }), - ).rejects.toEqual(new Error('Failed to fetch')); - }); - - it('should throw an error when the server returns a non-ok status', async () => { - const service = createService(); - - server.use( - rest.get(metricUrl, (_, res, ctx) => { - return res(ctx.status(401)); - }), - ); - await expect( - service.retrieveMetricDataPoints({ - type: 'df_count', - }), - ).rejects.toEqual(new Error('Unauthorized')); - - server.use( - rest.get(metricUrl, (_, res, ctx) => { - return res(ctx.status(500)); - }), - ); - await expect( - service.retrieveMetricDataPoints({ - type: 'df_count', - }), - ).rejects.toEqual(new Error('Internal Server Error')); - }); -}); - -describe('BenchmarkService', () => { - it('should return deployment frequency overall data from the server', async () => { - const service = createService(); - - expect(await service.retrieveBenchmarkData({ type: 'df' })).toEqual({ - key: 'lt-6month', - }); - }); - - it('should throw an error if the response does not contain metric data', async () => { - const service = createService(); - - server.use( - rest.get(benchmarkUrl, (_, res, ctx) => { - return res(ctx.json({ other: 'data' })); - }), - ); - await expect( - service.retrieveBenchmarkData({ - type: 'df', - }), - ).rejects.toEqual(new Error('Unexpected response')); - }); - - it('should return 404 for invalid types', async () => { - const service = createService(); - - await expect( - service.retrieveBenchmarkData({ - type: 'invalid_type', - }), - ).rejects.toEqual(new Error('Bad Request')); - }); - - it('should throw an error when the server returns a non-ok status', async () => { - const service = createService(); - - server.use( - rest.get(benchmarkUrl, (_, res, ctx) => { - return res(ctx.status(401)); - }), - ); - await expect( - service.retrieveBenchmarkData({ - type: 'df', - }), - ).rejects.toEqual(new Error('Unauthorized')); - - server.use( - rest.get(benchmarkUrl, (_, res, ctx) => { - return res(ctx.status(500)); - }), - ); - await expect( - service.retrieveBenchmarkData({ - type: 'df', - }), - ).rejects.toEqual(new Error('Internal Server Error')); - }); -});