From f00a76dc968a912ab4624306256c7a895d631810 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20DOREAU?= Date: Fri, 19 Jun 2020 00:46:11 +0200 Subject: [PATCH 01/35] feat(tech-radar): add unit tests --- .../RadarBubble/RadarBubble.test.tsx | 52 +++++++++++++++ .../components/RadarEntry/RadarEntry.test.tsx | 52 +++++++++++++++ .../RadarFooter/RadarFooter.test.tsx | 50 +++++++++++++++ .../components/RadarGrid/RadarGrid.test.tsx | 50 +++++++++++++++ .../RadarLegend/RadarLegend.test.tsx | 60 ++++++++++++++++++ .../components/RadarPlot/RadarPlot.test.tsx | 63 +++++++++++++++++++ 6 files changed, 327 insertions(+) create mode 100644 plugins/tech-radar/src/components/RadarBubble/RadarBubble.test.tsx create mode 100644 plugins/tech-radar/src/components/RadarEntry/RadarEntry.test.tsx create mode 100644 plugins/tech-radar/src/components/RadarFooter/RadarFooter.test.tsx create mode 100644 plugins/tech-radar/src/components/RadarGrid/RadarGrid.test.tsx create mode 100644 plugins/tech-radar/src/components/RadarLegend/RadarLegend.test.tsx create mode 100644 plugins/tech-radar/src/components/RadarPlot/RadarPlot.test.tsx diff --git a/plugins/tech-radar/src/components/RadarBubble/RadarBubble.test.tsx b/plugins/tech-radar/src/components/RadarBubble/RadarBubble.test.tsx new file mode 100644 index 0000000000..688fd55f9f --- /dev/null +++ b/plugins/tech-radar/src/components/RadarBubble/RadarBubble.test.tsx @@ -0,0 +1,52 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import React from 'react'; +import { render } from '@testing-library/react'; +import { ThemeProvider } from '@material-ui/core'; +import { lightTheme } from '@backstage/theme'; +import GetBBoxPolyfill from '../../utils/polyfills/getBBox'; + +import RadarBubble from './RadarBubble'; + +const minProps = { + visible: true, + text: 'RadarBubble', + x: 2, + y: 2, +}; + +describe('RadarBubble', () => { + beforeAll(() => { + GetBBoxPolyfill.create(0, 0, 1000, 500); + }); + + afterAll(() => { + GetBBoxPolyfill.remove(); + }); + + it('should render', () => { + const rendered = render( + + + + + , + ); + + expect(rendered).not.toBeNull(); + }); +}); diff --git a/plugins/tech-radar/src/components/RadarEntry/RadarEntry.test.tsx b/plugins/tech-radar/src/components/RadarEntry/RadarEntry.test.tsx new file mode 100644 index 0000000000..20bc6fa7be --- /dev/null +++ b/plugins/tech-radar/src/components/RadarEntry/RadarEntry.test.tsx @@ -0,0 +1,52 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import React from 'react'; +import { render } from '@testing-library/react'; +import { ThemeProvider } from '@material-ui/core'; +import { lightTheme } from '@backstage/theme'; +import GetBBoxPolyfill from '../../utils/polyfills/getBBox'; + +import RadarEntry from './RadarEntry'; + +const minProps = { + x: 2, + y: 2, + value: 2, + color: 'red', +}; + +describe('RadarEntry', () => { + beforeAll(() => { + GetBBoxPolyfill.create(0, 0, 1000, 500); + }); + + afterAll(() => { + GetBBoxPolyfill.remove(); + }); + + it('should render', () => { + const rendered = render( + + + + + , + ); + + expect(rendered).not.toBeNull(); + }); +}); diff --git a/plugins/tech-radar/src/components/RadarFooter/RadarFooter.test.tsx b/plugins/tech-radar/src/components/RadarFooter/RadarFooter.test.tsx new file mode 100644 index 0000000000..3e0ee7e407 --- /dev/null +++ b/plugins/tech-radar/src/components/RadarFooter/RadarFooter.test.tsx @@ -0,0 +1,50 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import React from 'react'; +import { render } from '@testing-library/react'; +import { ThemeProvider } from '@material-ui/core'; +import { lightTheme } from '@backstage/theme'; +import GetBBoxPolyfill from '../../utils/polyfills/getBBox'; + +import RadarFooter from './RadarFooter'; + +const minProps = { + x: 2, + y: 2, +}; + +describe('RadarFooter', () => { + beforeAll(() => { + GetBBoxPolyfill.create(0, 0, 1000, 500); + }); + + afterAll(() => { + GetBBoxPolyfill.remove(); + }); + + it('should render', () => { + const rendered = render( + + + + + , + ); + + expect(rendered).not.toBeNull(); + }); +}); diff --git a/plugins/tech-radar/src/components/RadarGrid/RadarGrid.test.tsx b/plugins/tech-radar/src/components/RadarGrid/RadarGrid.test.tsx new file mode 100644 index 0000000000..8a2873564d --- /dev/null +++ b/plugins/tech-radar/src/components/RadarGrid/RadarGrid.test.tsx @@ -0,0 +1,50 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import React from 'react'; +import { render } from '@testing-library/react'; +import { ThemeProvider } from '@material-ui/core'; +import { lightTheme } from '@backstage/theme'; +import GetBBoxPolyfill from '../../utils/polyfills/getBBox'; + +import RadarGrid from './RadarGrid'; + +const minProps = { + radius: 5, + rings: [{ id: 'use', name: 'USE', color: '#93c47d' }], +}; + +describe('RadarGrid', () => { + beforeAll(() => { + GetBBoxPolyfill.create(0, 0, 1000, 500); + }); + + afterAll(() => { + GetBBoxPolyfill.remove(); + }); + + it('should render', () => { + const rendered = render( + + + + + , + ); + + expect(rendered).not.toBeNull(); + }); +}); diff --git a/plugins/tech-radar/src/components/RadarLegend/RadarLegend.test.tsx b/plugins/tech-radar/src/components/RadarLegend/RadarLegend.test.tsx new file mode 100644 index 0000000000..8168688fdd --- /dev/null +++ b/plugins/tech-radar/src/components/RadarLegend/RadarLegend.test.tsx @@ -0,0 +1,60 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import React from 'react'; +import { render } from '@testing-library/react'; +import { ThemeProvider } from '@material-ui/core'; +import { lightTheme } from '@backstage/theme'; +import GetBBoxPolyfill from '../../utils/polyfills/getBBox'; + +import RadarLegend from './RadarLegend'; + +const minProps = { + quadrants: [{ id: 'languages', name: 'Languages' }], + rings: [{ id: 'use', name: 'USE', color: '#93c47d' }], + entries: [ + { + id: 'typescript', + title: 'TypeScript', + quadrant: { id: 'languages', name: 'Languages' }, + moved: 0, + ring: { id: 'use', name: 'USE', color: '#93c47d' }, + url: '#', + }, + ], +}; + +describe('RadarLegend', () => { + beforeAll(() => { + GetBBoxPolyfill.create(0, 0, 1000, 500); + }); + + afterAll(() => { + GetBBoxPolyfill.remove(); + }); + + it('should render', () => { + const rendered = render( + + + + + , + ); + + expect(rendered).not.toBeNull(); + }); +}); diff --git a/plugins/tech-radar/src/components/RadarPlot/RadarPlot.test.tsx b/plugins/tech-radar/src/components/RadarPlot/RadarPlot.test.tsx new file mode 100644 index 0000000000..31f450baeb --- /dev/null +++ b/plugins/tech-radar/src/components/RadarPlot/RadarPlot.test.tsx @@ -0,0 +1,63 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import React from 'react'; +import { render } from '@testing-library/react'; +import { ThemeProvider } from '@material-ui/core'; +import { lightTheme } from '@backstage/theme'; +import GetBBoxPolyfill from '../../utils/polyfills/getBBox'; + +import RadarPlot from './RadarPlot'; + +const minProps = { + width: 500, + height: 200, + radius: 50, + quadrants: [{ id: 'languages', name: 'Languages' }], + rings: [{ id: 'use', name: 'USE', color: '#93c47d' }], + entries: [ + { + id: 'typescript', + title: 'TypeScript', + quadrant: { id: 'languages', name: 'Languages' }, + moved: 0, + ring: { id: 'use', name: 'USE', color: '#93c47d' }, + url: '#', + }, + ], +}; + +describe('RadarPlot', () => { + beforeAll(() => { + GetBBoxPolyfill.create(0, 0, 1000, 500); + }); + + afterAll(() => { + GetBBoxPolyfill.remove(); + }); + + it('should render', () => { + const rendered = render( + + + + + , + ); + + expect(rendered).not.toBeNull(); + }); +}); From de1fe3c0e84f0be71b9d9ea8cf5a693ac7048749 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20DOREAU?= Date: Fri, 19 Jun 2020 00:47:25 +0200 Subject: [PATCH 02/35] refactor(tech-radar): style + code quality --- .../tech-radar/src/components/Radar/Radar.tsx | 6 +- .../tech-radar/src/components/Radar/utils.ts | 2 +- .../components/RadarBubble/RadarBubble.tsx | 4 +- .../src/components/RadarComponent.tsx | 28 ++--- .../src/components/RadarEntry/RadarEntry.tsx | 47 ++++---- .../components/RadarFooter/RadarFooter.tsx | 4 +- .../components/RadarLegend/RadarLegend.tsx | 103 +++++++++--------- .../tech-radar/src/components/RadarPage.tsx | 4 +- .../src/components/RadarPlot/RadarPlot.tsx | 6 +- plugins/tech-radar/src/utils/components.tsx | 35 ++++++ 10 files changed, 134 insertions(+), 105 deletions(-) create mode 100644 plugins/tech-radar/src/utils/components.tsx diff --git a/plugins/tech-radar/src/components/Radar/Radar.tsx b/plugins/tech-radar/src/components/Radar/Radar.tsx index 1db655730d..a1e4d2da78 100644 --- a/plugins/tech-radar/src/components/Radar/Radar.tsx +++ b/plugins/tech-radar/src/components/Radar/Radar.tsx @@ -14,7 +14,7 @@ * limitations under the License. */ -import React, { FC, useState, useRef } from 'react'; +import React, { useState, useRef } from 'react'; import RadarPlot from '../RadarPlot'; import { Ring, Quadrant, Entry } from '../../utils/types'; import { adjustQuadrants, adjustRings, adjustEntries } from './utils'; @@ -28,7 +28,7 @@ type Props = { svgProps?: object; }; -const Radar: FC = props => { +const Radar = (props: Props): JSX.Element => { const { width, height, quadrants, rings, entries } = props; const radius = Math.min(width, height) / 2; @@ -38,7 +38,7 @@ const Radar: FC = props => { // TODO(dflemstr): most of this can be heavily memoized if performance becomes a problem adjustQuadrants(quadrants, radius, width, height); adjustRings(rings, radius); - adjustEntries(entries, activeEntry, quadrants, rings, radius); + adjustEntries(entries, quadrants, rings, radius, activeEntry); return ( diff --git a/plugins/tech-radar/src/components/Radar/utils.ts b/plugins/tech-radar/src/components/Radar/utils.ts index a8896ddf31..65268613fe 100644 --- a/plugins/tech-radar/src/components/Radar/utils.ts +++ b/plugins/tech-radar/src/components/Radar/utils.ts @@ -98,10 +98,10 @@ export const adjustQuadrants = ( export const adjustEntries = ( entries: Entry[], - activeEntry: Entry | null | undefined, quadrants: Quadrant[], rings: Ring[], radius: number, + activeEntry?: Entry | null, ) => { let seed = 42; entries.forEach((entry, idx) => { diff --git a/plugins/tech-radar/src/components/RadarBubble/RadarBubble.tsx b/plugins/tech-radar/src/components/RadarBubble/RadarBubble.tsx index dcf9a37ed7..723a408885 100644 --- a/plugins/tech-radar/src/components/RadarBubble/RadarBubble.tsx +++ b/plugins/tech-radar/src/components/RadarBubble/RadarBubble.tsx @@ -14,7 +14,7 @@ * limitations under the License. */ -import React, { FC, useRef, useLayoutEffect } from 'react'; +import React, { useRef, useLayoutEffect } from 'react'; import { makeStyles, Theme } from '@material-ui/core'; type Props = { @@ -46,7 +46,7 @@ const useStyles = makeStyles(() => ({ }, })); -const RadarBubble: FC = props => { +const RadarBubble = (props: Props): JSX.Element => { const classes = useStyles(props); const { visible, text } = props; diff --git a/plugins/tech-radar/src/components/RadarComponent.tsx b/plugins/tech-radar/src/components/RadarComponent.tsx index 981d1f1369..8d56c6aa0f 100644 --- a/plugins/tech-radar/src/components/RadarComponent.tsx +++ b/plugins/tech-radar/src/components/RadarComponent.tsx @@ -14,7 +14,7 @@ * limitations under the License. */ -import React, { useEffect, useState, FC } from 'react'; +import React, { useEffect, useState } from 'react'; import { Progress, useApi, errorApiRef, ErrorApi } from '@backstage/core'; import Radar from '../components/Radar'; import { TechRadarComponentProps, TechRadarLoaderResponse } from '../api'; @@ -22,15 +22,9 @@ import getSampleData from '../sampleData'; const useTechRadarLoader = (props: TechRadarComponentProps) => { const errorApi = useApi(errorApiRef); - const [state, setState] = useState<{ - loading: boolean; - error?: Error; - data?: TechRadarLoaderResponse; - }>({ - loading: true, - error: undefined, - data: undefined, - }); + const [error, setError] = useState(); + const [loading, setLoading] = useState(true); + const [data, setData] = useState(); const { getData } = props; @@ -41,22 +35,20 @@ const useTechRadarLoader = (props: TechRadarComponentProps) => { getData() .then((payload: TechRadarLoaderResponse) => { - setState({ loading: false, error: undefined, data: payload }); + setLoading(false); + setData(payload); }) .catch((err: Error) => { errorApi.post(err); - setState({ - loading: false, - error: err, - data: undefined, - }); + setLoading(false); + setError(err); }); }, [getData, errorApi]); - return state; + return { data, loading, error }; }; -const RadarComponent: FC = props => { +const RadarComponent = (props: TechRadarComponentProps): JSX.Element => { const { loading, error, data } = useTechRadarLoader(props); return ( diff --git a/plugins/tech-radar/src/components/RadarEntry/RadarEntry.tsx b/plugins/tech-radar/src/components/RadarEntry/RadarEntry.tsx index 6006979e67..ff499f4575 100644 --- a/plugins/tech-radar/src/components/RadarEntry/RadarEntry.tsx +++ b/plugins/tech-radar/src/components/RadarEntry/RadarEntry.tsx @@ -14,13 +14,14 @@ * limitations under the License. */ -import React, { FC } from 'react'; +import React from 'react'; import { makeStyles, Theme } from '@material-ui/core'; +import { WithLink } from '../../utils/components'; type Props = { x: number; y: number; - number: number; + value: number; color: string; url?: string; moved?: number; @@ -43,14 +44,27 @@ const useStyles = makeStyles(() => ({ }, })); -const RadarEntry: FC = props => { +const makeBlip = (color: string, moved?: number) => { + const style = { fill: color }; + + let blip = ; + if (moved && moved > 0) { + blip = ; // triangle pointing up + } else if (moved && moved < 0) { + blip = ; // triangle pointing down + } + + return blip; +}; + +const RadarEntry = (props: Props): JSX.Element => { const classes = useStyles(props); const { moved, color, url, - number, + value, x, y, onMouseEnter, @@ -58,24 +72,7 @@ const RadarEntry: FC = props => { onClick, } = props; - const style = { fill: color }; - - let blip; - if (moved && moved > 0) { - blip = ; // triangle pointing up - } else if (moved && moved < 0) { - blip = ; // triangle pointing down - } else { - blip = ; - } - - if (url) { - blip = ( - - {blip} - - ); - } + const blip = makeBlip(color, moved); return ( = props => { onMouseLeave={onMouseLeave} onClick={onClick} > - {blip} + + {blip} + - {number} + {value} ); diff --git a/plugins/tech-radar/src/components/RadarFooter/RadarFooter.tsx b/plugins/tech-radar/src/components/RadarFooter/RadarFooter.tsx index 7d60a63f59..b6284644eb 100644 --- a/plugins/tech-radar/src/components/RadarFooter/RadarFooter.tsx +++ b/plugins/tech-radar/src/components/RadarFooter/RadarFooter.tsx @@ -14,7 +14,7 @@ * limitations under the License. */ -import React, { FC } from 'react'; +import React from 'react'; import { makeStyles, Theme } from '@material-ui/core'; type Props = { @@ -31,7 +31,7 @@ const useStyles = makeStyles(() => ({ }, })); -const RadarFooter: FC = props => { +const RadarFooter = (props: Props): JSX.Element => { const { x, y } = props; const classes = useStyles(props); diff --git a/plugins/tech-radar/src/components/RadarLegend/RadarLegend.tsx b/plugins/tech-radar/src/components/RadarLegend/RadarLegend.tsx index 03146a8071..c2509e9cac 100644 --- a/plugins/tech-radar/src/components/RadarLegend/RadarLegend.tsx +++ b/plugins/tech-radar/src/components/RadarLegend/RadarLegend.tsx @@ -13,9 +13,10 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import React, { FC } from 'react'; +import React from 'react'; import { makeStyles, Theme } from '@material-ui/core'; import { Quadrant, Ring, Entry } from '../../utils/types'; +import { WithLink } from '../../utils/components'; type Segments = { [k: number]: { [k: number]: Entry[] }; @@ -29,20 +30,7 @@ type Props = { onEntryMouseLeave?: (entry: Entry) => void; }; -const useStyles = makeStyles(() => ({ - quadrant: { - height: '100%', - width: '100%', - overflow: 'hidden', - pointerEvents: 'none', - }, - quadrantHeading: { - pointerEvents: 'none', - userSelect: 'none', - marginTop: 0, - marginBottom: 'calc(18px * 0.375)', - fontSize: '18px', - }, +const ringStyles = { rings: { columns: 3, }, @@ -69,6 +57,25 @@ const useStyles = makeStyles(() => ({ '-webkit-font-feature-settings': 'pnum', 'font-feature-settings': 'pnum', }, +}; + +const quadrantStyles = { + quadrant: { + height: '100%', + width: '100%', + overflow: 'hidden', + pointerEvents: 'none', + }, + quadrantHeading: { + pointerEvents: 'none', + userSelect: 'none', + marginTop: 0, + marginBottom: 'calc(18px * 0.375)', + fontSize: '18px', + }, +}; + +const entryStyle = { entry: { pointerEvents: 'none', userSelect: 'none', @@ -77,12 +84,18 @@ const useStyles = makeStyles(() => ({ entryLink: { pointerEvents: 'none', }, +}; + +const useStyles = makeStyles(() => ({ + ringStyles, + quadrantStyles, + entryStyle, })); -const RadarLegend: FC = props => { +const RadarLegend = (props: Props): JSX.Element => { const classes = useStyles(props); - const _getSegment = ( + const getSegment = ( segmented: Segments, quadrant: Quadrant, ring: Ring, @@ -94,7 +107,7 @@ const RadarLegend: FC = props => { return ridx === undefined ? [] : segmentedData[ridx + ringOffset] || []; }; - const _renderRing = ( + const renderRing = ( ring: Ring, entries: Entry[], onEntryMouseEnter?: Props['onEntryMouseEnter'], @@ -107,39 +120,29 @@ const RadarLegend: FC = props => {

(empty)

) : (
    - {entries.map(entry => { - let node = {entry.title}; - - if (entry.url) { - node = ( - - {node} - - ); - } - - return ( -
  1. onEntryMouseEnter(entry)) - } - onMouseLeave={ - onEntryMouseLeave && (() => onEntryMouseLeave(entry)) - } - > - {node} -
    2. - ); - })} + {entries.map(entry => ( +
  2. onEntryMouseEnter(entry)) + } + onMouseLeave={ + onEntryMouseLeave && (() => onEntryMouseLeave(entry)) + } + > + + {entry.title} + +
    3. + ))}
)} ); }; - const _renderQuadrant = ( + const renderQuadrant = ( segments: Segments, quadrant: Quadrant, rings: Ring[], @@ -158,9 +161,9 @@ const RadarLegend: FC = props => {

{quadrant.name}

{rings.map(ring => - _renderRing( + renderRing( ring, - _getSegment(segments, quadrant, ring), + getSegment(segments, quadrant, ring), onEntryMouseEnter, onEntryMouseLeave, ), @@ -171,7 +174,7 @@ const RadarLegend: FC = props => { ); }; - const _setupSegments = (entries: Entry[]) => { + const setupSegments = (entries: Entry[]) => { const segments: Segments = {}; for (const entry of entries) { @@ -209,12 +212,12 @@ const RadarLegend: FC = props => { onEntryMouseLeave, } = props; - const segments: Segments = _setupSegments(entries); + const segments: Segments = setupSegments(entries); return ( {quadrants.map(quadrant => - _renderQuadrant( + renderQuadrant( segments, quadrant, rings, diff --git a/plugins/tech-radar/src/components/RadarPage.tsx b/plugins/tech-radar/src/components/RadarPage.tsx index 4c7236e01f..1aefb72eca 100644 --- a/plugins/tech-radar/src/components/RadarPage.tsx +++ b/plugins/tech-radar/src/components/RadarPage.tsx @@ -14,7 +14,7 @@ * limitations under the License. */ -import React, { FC } from 'react'; +import React from 'react'; import { Grid } from '@material-ui/core'; import { Content, @@ -29,7 +29,7 @@ import { import RadarComponent from '../components/RadarComponent'; import { techRadarApiRef, TechRadarApi } from '../api'; -const RadarPage: FC<{}> = () => { +const RadarPage = (): JSX.Element => { const techRadarApi = useApi(techRadarApiRef); return ( diff --git a/plugins/tech-radar/src/components/RadarPlot/RadarPlot.tsx b/plugins/tech-radar/src/components/RadarPlot/RadarPlot.tsx index 48ad660dde..0315afde5f 100644 --- a/plugins/tech-radar/src/components/RadarPlot/RadarPlot.tsx +++ b/plugins/tech-radar/src/components/RadarPlot/RadarPlot.tsx @@ -14,7 +14,7 @@ * limitations under the License. */ -import React, { FC } from 'react'; +import React from 'react'; import { Quadrant, Ring, Entry } from '../../utils/types'; import RadarGrid from '../RadarGrid'; @@ -36,7 +36,7 @@ type Props = { }; // A component that draws the radar circle. -const RadarPlot: FC = props => { +const RadarPlot = (props: Props): JSX.Element => { const { width, height, @@ -71,7 +71,7 @@ const RadarPlot: FC = props => { x={entry.x || 0} y={entry.y || 0} color={entry.color || ''} - number={((entry && entry.idx) || 0) + 1} + value={((entry && entry.idx) || 0) + 1} url={entry.url} moved={entry.moved} onMouseEnter={onEntryMouseEnter && (() => onEntryMouseEnter(entry))} diff --git a/plugins/tech-radar/src/utils/components.tsx b/plugins/tech-radar/src/utils/components.tsx new file mode 100644 index 0000000000..af5a7b05bd --- /dev/null +++ b/plugins/tech-radar/src/utils/components.tsx @@ -0,0 +1,35 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import React from 'react'; + +type WithLinkProps = { + url?: string; + className: string; + children: React.ReactNode; +}; + +export const WithLink = ({ + url, + className, + children, +}: WithLinkProps): JSX.Element => + url ? ( + + {children} + + ) : ( + <>{children} + ); From 3d318efeaedb55ab59c3d899daaaf153828b9dc7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20DOREAU?= Date: Fri, 19 Jun 2020 16:20:37 +0200 Subject: [PATCH 03/35] refactor(tech-radar): variable names + undefined check --- .../tech-radar/src/components/Radar/Radar.tsx | 6 ++-- .../tech-radar/src/components/Radar/utils.ts | 28 +++++++-------- .../src/components/RadarGrid/RadarGrid.tsx | 6 ++-- .../components/RadarLegend/RadarLegend.tsx | 35 ++++++++++--------- .../src/components/RadarPlot/RadarPlot.tsx | 2 +- plugins/tech-radar/src/utils/types.ts | 6 ++-- 6 files changed, 44 insertions(+), 39 deletions(-) diff --git a/plugins/tech-radar/src/components/Radar/Radar.tsx b/plugins/tech-radar/src/components/Radar/Radar.tsx index a1e4d2da78..8fe3c15fba 100644 --- a/plugins/tech-radar/src/components/Radar/Radar.tsx +++ b/plugins/tech-radar/src/components/Radar/Radar.tsx @@ -32,7 +32,7 @@ const Radar = (props: Props): JSX.Element => { const { width, height, quadrants, rings, entries } = props; const radius = Math.min(width, height) / 2; - const [activeEntry, setActiveEntry] = useState(); + const [activeEntry, setActiveEntry] = useState(); const node = useRef(null); // TODO(dflemstr): most of this can be heavily memoized if performance becomes a problem @@ -49,9 +49,9 @@ const Radar = (props: Props): JSX.Element => { entries={entries} quadrants={quadrants} rings={rings} - activeEntry={activeEntry || undefined} + activeEntry={activeEntry} onEntryMouseEnter={entry => setActiveEntry(entry)} - onEntryMouseLeave={() => setActiveEntry(null)} + onEntryMouseLeave={() => setActiveEntry(undefined)} /> ); diff --git a/plugins/tech-radar/src/components/Radar/utils.ts b/plugins/tech-radar/src/components/Radar/utils.ts index 65268613fe..25d9e7dac8 100644 --- a/plugins/tech-radar/src/components/Radar/utils.ts +++ b/plugins/tech-radar/src/components/Radar/utils.ts @@ -81,14 +81,14 @@ export const adjustQuadrants = ( }, ]; - quadrants.forEach((quadrant, idx) => { - const legendParam = legendParams[idx % 4]; + quadrants.forEach((quadrant, index) => { + const legendParam = legendParams[index % 4]; - quadrant.idx = idx; - quadrant.radialMin = (idx * Math.PI) / 2; - quadrant.radialMax = ((idx + 1) * Math.PI) / 2; - quadrant.offsetX = idx % 4 === 0 || idx % 4 === 3 ? 1 : -1; - quadrant.offsetY = idx % 4 === 0 || idx % 4 === 1 ? 1 : -1; + quadrant.index = index; + quadrant.radialMin = (index * Math.PI) / 2; + quadrant.radialMax = ((index + 1) * Math.PI) / 2; + quadrant.offsetX = index % 4 === 0 || index % 4 === 3 ? 1 : -1; + quadrant.offsetY = index % 4 === 0 || index % 4 === 1 ? 1 : -1; quadrant.legendX = legendParam.x; quadrant.legendY = legendParam.y; quadrant.legendWidth = legendParam.width; @@ -101,10 +101,10 @@ export const adjustEntries = ( quadrants: Quadrant[], rings: Ring[], radius: number, - activeEntry?: Entry | null, + activeEntry?: Entry, ) => { let seed = 42; - entries.forEach((entry, idx) => { + entries.forEach((entry, index) => { const quadrant = quadrants.find(q => { const match = typeof entry.quadrant === 'object' ? entry.quadrant.id : entry.quadrant; @@ -124,7 +124,7 @@ export const adjustEntries = ( throw new Error(`Unknown ring ${entry.ring} for entry ${entry.id}!`); } - entry.idx = idx; + entry.index = index; entry.quadrant = quadrant; entry.ring = ring; entry.segment = new Segment(quadrant, ring, radius, () => seed++); @@ -163,10 +163,10 @@ export const adjustEntries = ( }; export const adjustRings = (rings: Ring[], radius: number) => { - rings.forEach((ring, idx) => { - ring.idx = idx; - ring.outerRadius = ((idx + 2) / (rings.length + 1)) * radius; + rings.forEach((ring, index) => { + ring.index = index; + ring.outerRadius = ((index + 2) / (rings.length + 1)) * radius; ring.innerRadius = - ((idx === 0 ? 0 : idx + 1) / (rings.length + 1)) * radius; + ((index === 0 ? 0 : index + 1) / (rings.length + 1)) * radius; }); }; diff --git a/plugins/tech-radar/src/components/RadarGrid/RadarGrid.tsx b/plugins/tech-radar/src/components/RadarGrid/RadarGrid.tsx index 514afbe958..68c84e2443 100644 --- a/plugins/tech-radar/src/components/RadarGrid/RadarGrid.tsx +++ b/plugins/tech-radar/src/components/RadarGrid/RadarGrid.tsx @@ -49,7 +49,7 @@ const RadarGrid = (props: Props) => { const { radius, rings } = props; const classes = useStyles(props); - const makeRingNode = (ringRadius: number | undefined, ringIndex: number) => [ + const makeRingNode = (ringIndex: number, ringRadius?: number) => [ { />, ]; - const ringNodes = rings.map(r => r.outerRadius).map(makeRingNode); + const ringNodes = rings + .map(r => r.outerRadius) + .map((ringRadius, ringIndex) => makeRingNode(ringIndex, ringRadius)); return <>{axisNodes.concat(...ringNodes)}; }; diff --git a/plugins/tech-radar/src/components/RadarLegend/RadarLegend.tsx b/plugins/tech-radar/src/components/RadarLegend/RadarLegend.tsx index c2509e9cac..ad204f4beb 100644 --- a/plugins/tech-radar/src/components/RadarLegend/RadarLegend.tsx +++ b/plugins/tech-radar/src/components/RadarLegend/RadarLegend.tsx @@ -101,10 +101,13 @@ const RadarLegend = (props: Props): JSX.Element => { ring: Ring, ringOffset = 0, ) => { - const qidx = quadrant.idx; - const ridx = ring.idx; - const segmentedData = qidx === undefined ? {} : segmented[qidx] || {}; - return ridx === undefined ? [] : segmentedData[ridx + ringOffset] || []; + const quadrantIndex = quadrant.index; + const ringIndex = ring.index; + const segmentedData = + quadrantIndex === undefined ? {} : segmented[quadrantIndex] || {}; + return ringIndex === undefined + ? [] + : segmentedData[ringIndex + ringOffset] || []; }; const renderRing = ( @@ -116,14 +119,14 @@ const RadarLegend = (props: Props): JSX.Element => { return (

{ring.name}

- {entries.length === 0 ? ( + {!entries.length ? (

(empty)

) : (
    {entries.map(entry => (
  1. onEntryMouseEnter(entry)) } @@ -178,24 +181,24 @@ const RadarLegend = (props: Props): JSX.Element => { const segments: Segments = {}; for (const entry of entries) { - const qidx = entry.quadrant.idx; - const ridx = entry.ring.idx; + const quadrantIndex = entry.quadrant.index; + const ringIndex = entry.ring.index; let quadrantData: { [k: number]: Entry[] } = {}; - if (qidx !== undefined) { - if (segments[qidx] === undefined) { - segments[qidx] = {}; + if (quadrantIndex !== undefined) { + if (segments[quadrantIndex] === undefined) { + segments[quadrantIndex] = {}; } - quadrantData = segments[qidx]; + quadrantData = segments[quadrantIndex]; } let ringData = []; - if (ridx !== undefined) { - if (quadrantData[ridx] === undefined) { - quadrantData[ridx] = []; + if (ringIndex !== undefined) { + if (quadrantData[ringIndex] === undefined) { + quadrantData[ringIndex] = []; } - ringData = quadrantData[ridx]; + ringData = quadrantData[ringIndex]; } ringData.push(entry); diff --git a/plugins/tech-radar/src/components/RadarPlot/RadarPlot.tsx b/plugins/tech-radar/src/components/RadarPlot/RadarPlot.tsx index 0315afde5f..7fd46b95b7 100644 --- a/plugins/tech-radar/src/components/RadarPlot/RadarPlot.tsx +++ b/plugins/tech-radar/src/components/RadarPlot/RadarPlot.tsx @@ -71,7 +71,7 @@ const RadarPlot = (props: Props): JSX.Element => { x={entry.x || 0} y={entry.y || 0} color={entry.color || ''} - value={((entry && entry.idx) || 0) + 1} + value={((entry && entry.index) || 0) + 1} url={entry.url} moved={entry.moved} onMouseEnter={onEntryMouseEnter && (() => onEntryMouseEnter(entry))} diff --git a/plugins/tech-radar/src/utils/types.ts b/plugins/tech-radar/src/utils/types.ts index 14a0554a1f..51f3cee7f8 100644 --- a/plugins/tech-radar/src/utils/types.ts +++ b/plugins/tech-radar/src/utils/types.ts @@ -17,7 +17,7 @@ // Parameters for a ring; its index in an array determines how close to the center this ring is. export type Ring = { id: string; - idx?: number; + index?: number; name: string; color: string; outerRadius?: number; @@ -27,7 +27,7 @@ export type Ring = { // Parameters for a quadrant (there should be exactly 4 of course) export type Quadrant = { id: string; - idx?: number; + index?: number; name: string; legendX?: number; legendY?: number; @@ -47,7 +47,7 @@ export type Segment = { export type Entry = { id: string; - idx?: number; + index?: number; x?: number; y?: number; color?: string; From 4cae750c72875087988e0818f8d830130eaf61d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20DOREAU?= Date: Fri, 19 Jun 2020 22:46:23 +0200 Subject: [PATCH 04/35] refactor(tech-radar): update moved prop type --- plugins/tech-radar/src/api.ts | 2 +- plugins/tech-radar/src/components/Radar/Radar.test.tsx | 4 ++-- plugins/tech-radar/src/components/Radar/Radar.tsx | 2 +- .../src/components/RadarBubble/RadarBubble.test.tsx | 4 ++-- plugins/tech-radar/src/components/RadarBubble/RadarBubble.tsx | 2 +- .../tech-radar/src/components/RadarEntry/RadarEntry.test.tsx | 4 ++-- plugins/tech-radar/src/components/RadarEntry/RadarEntry.tsx | 2 +- .../src/components/RadarFooter/RadarFooter.test.tsx | 4 ++-- plugins/tech-radar/src/components/RadarFooter/RadarFooter.tsx | 2 +- .../tech-radar/src/components/RadarGrid/RadarGrid.test.tsx | 4 ++-- plugins/tech-radar/src/components/RadarGrid/RadarGrid.tsx | 2 +- .../src/components/RadarLegend/RadarLegend.test.tsx | 4 ++-- plugins/tech-radar/src/components/RadarLegend/RadarLegend.tsx | 2 +- .../tech-radar/src/components/RadarPlot/RadarPlot.test.tsx | 4 ++-- plugins/tech-radar/src/components/RadarPlot/RadarPlot.tsx | 2 +- plugins/tech-radar/src/utils/types.ts | 2 +- 16 files changed, 23 insertions(+), 23 deletions(-) diff --git a/plugins/tech-radar/src/api.ts b/plugins/tech-radar/src/api.ts index 3dec964ddb..d7f5b57a43 100644 --- a/plugins/tech-radar/src/api.ts +++ b/plugins/tech-radar/src/api.ts @@ -34,7 +34,7 @@ export interface RadarQuadrant { export interface RadarEntry { key: string; // react key id: string; - moved: number; + moved: -1 | 0 | 1; quadrant: RadarQuadrant; ring: RadarRing; title: string; diff --git a/plugins/tech-radar/src/components/Radar/Radar.test.tsx b/plugins/tech-radar/src/components/Radar/Radar.test.tsx index 469f0d85cf..f0ad0d66ed 100644 --- a/plugins/tech-radar/src/components/Radar/Radar.test.tsx +++ b/plugins/tech-radar/src/components/Radar/Radar.test.tsx @@ -20,9 +20,9 @@ import { ThemeProvider } from '@material-ui/core'; import { lightTheme } from '@backstage/theme'; import GetBBoxPolyfill from '../../utils/polyfills/getBBox'; -import Radar from './Radar'; +import Radar, { Props } from './Radar'; -const minProps = { +const minProps: Props = { width: 500, height: 200, quadrants: [{ id: 'languages', name: 'Languages' }], diff --git a/plugins/tech-radar/src/components/Radar/Radar.tsx b/plugins/tech-radar/src/components/Radar/Radar.tsx index 8fe3c15fba..2c7838e803 100644 --- a/plugins/tech-radar/src/components/Radar/Radar.tsx +++ b/plugins/tech-radar/src/components/Radar/Radar.tsx @@ -19,7 +19,7 @@ import RadarPlot from '../RadarPlot'; import { Ring, Quadrant, Entry } from '../../utils/types'; import { adjustQuadrants, adjustRings, adjustEntries } from './utils'; -type Props = { +export type Props = { width: number; height: number; quadrants: Quadrant[]; diff --git a/plugins/tech-radar/src/components/RadarBubble/RadarBubble.test.tsx b/plugins/tech-radar/src/components/RadarBubble/RadarBubble.test.tsx index 688fd55f9f..9e402b9a91 100644 --- a/plugins/tech-radar/src/components/RadarBubble/RadarBubble.test.tsx +++ b/plugins/tech-radar/src/components/RadarBubble/RadarBubble.test.tsx @@ -20,9 +20,9 @@ import { ThemeProvider } from '@material-ui/core'; import { lightTheme } from '@backstage/theme'; import GetBBoxPolyfill from '../../utils/polyfills/getBBox'; -import RadarBubble from './RadarBubble'; +import RadarBubble, { Props } from './RadarBubble'; -const minProps = { +const minProps: Props = { visible: true, text: 'RadarBubble', x: 2, diff --git a/plugins/tech-radar/src/components/RadarBubble/RadarBubble.tsx b/plugins/tech-radar/src/components/RadarBubble/RadarBubble.tsx index 723a408885..be38231087 100644 --- a/plugins/tech-radar/src/components/RadarBubble/RadarBubble.tsx +++ b/plugins/tech-radar/src/components/RadarBubble/RadarBubble.tsx @@ -17,7 +17,7 @@ import React, { useRef, useLayoutEffect } from 'react'; import { makeStyles, Theme } from '@material-ui/core'; -type Props = { +export type Props = { visible: boolean; text: string; x: number; diff --git a/plugins/tech-radar/src/components/RadarEntry/RadarEntry.test.tsx b/plugins/tech-radar/src/components/RadarEntry/RadarEntry.test.tsx index 20bc6fa7be..14614e88e1 100644 --- a/plugins/tech-radar/src/components/RadarEntry/RadarEntry.test.tsx +++ b/plugins/tech-radar/src/components/RadarEntry/RadarEntry.test.tsx @@ -20,9 +20,9 @@ import { ThemeProvider } from '@material-ui/core'; import { lightTheme } from '@backstage/theme'; import GetBBoxPolyfill from '../../utils/polyfills/getBBox'; -import RadarEntry from './RadarEntry'; +import RadarEntry, { Props } from './RadarEntry'; -const minProps = { +const minProps: Props = { x: 2, y: 2, value: 2, diff --git a/plugins/tech-radar/src/components/RadarEntry/RadarEntry.tsx b/plugins/tech-radar/src/components/RadarEntry/RadarEntry.tsx index ff499f4575..98841dce87 100644 --- a/plugins/tech-radar/src/components/RadarEntry/RadarEntry.tsx +++ b/plugins/tech-radar/src/components/RadarEntry/RadarEntry.tsx @@ -18,7 +18,7 @@ import React from 'react'; import { makeStyles, Theme } from '@material-ui/core'; import { WithLink } from '../../utils/components'; -type Props = { +export type Props = { x: number; y: number; value: number; diff --git a/plugins/tech-radar/src/components/RadarFooter/RadarFooter.test.tsx b/plugins/tech-radar/src/components/RadarFooter/RadarFooter.test.tsx index 3e0ee7e407..321947a24c 100644 --- a/plugins/tech-radar/src/components/RadarFooter/RadarFooter.test.tsx +++ b/plugins/tech-radar/src/components/RadarFooter/RadarFooter.test.tsx @@ -20,9 +20,9 @@ import { ThemeProvider } from '@material-ui/core'; import { lightTheme } from '@backstage/theme'; import GetBBoxPolyfill from '../../utils/polyfills/getBBox'; -import RadarFooter from './RadarFooter'; +import RadarFooter, { Props } from './RadarFooter'; -const minProps = { +const minProps: Props = { x: 2, y: 2, }; diff --git a/plugins/tech-radar/src/components/RadarFooter/RadarFooter.tsx b/plugins/tech-radar/src/components/RadarFooter/RadarFooter.tsx index b6284644eb..9ba0ed0405 100644 --- a/plugins/tech-radar/src/components/RadarFooter/RadarFooter.tsx +++ b/plugins/tech-radar/src/components/RadarFooter/RadarFooter.tsx @@ -17,7 +17,7 @@ import React from 'react'; import { makeStyles, Theme } from '@material-ui/core'; -type Props = { +export type Props = { x: number; y: number; }; diff --git a/plugins/tech-radar/src/components/RadarGrid/RadarGrid.test.tsx b/plugins/tech-radar/src/components/RadarGrid/RadarGrid.test.tsx index 8a2873564d..9ba170a0e7 100644 --- a/plugins/tech-radar/src/components/RadarGrid/RadarGrid.test.tsx +++ b/plugins/tech-radar/src/components/RadarGrid/RadarGrid.test.tsx @@ -20,9 +20,9 @@ import { ThemeProvider } from '@material-ui/core'; import { lightTheme } from '@backstage/theme'; import GetBBoxPolyfill from '../../utils/polyfills/getBBox'; -import RadarGrid from './RadarGrid'; +import RadarGrid, { Props } from './RadarGrid'; -const minProps = { +const minProps: Props = { radius: 5, rings: [{ id: 'use', name: 'USE', color: '#93c47d' }], }; diff --git a/plugins/tech-radar/src/components/RadarGrid/RadarGrid.tsx b/plugins/tech-radar/src/components/RadarGrid/RadarGrid.tsx index 68c84e2443..cd06bf6ef8 100644 --- a/plugins/tech-radar/src/components/RadarGrid/RadarGrid.tsx +++ b/plugins/tech-radar/src/components/RadarGrid/RadarGrid.tsx @@ -18,7 +18,7 @@ import React from 'react'; import { makeStyles, Theme } from '@material-ui/core'; import { Ring } from '../../utils/types'; -type Props = { +export type Props = { radius: number; rings: Ring[]; }; diff --git a/plugins/tech-radar/src/components/RadarLegend/RadarLegend.test.tsx b/plugins/tech-radar/src/components/RadarLegend/RadarLegend.test.tsx index 8168688fdd..5900f5c1bc 100644 --- a/plugins/tech-radar/src/components/RadarLegend/RadarLegend.test.tsx +++ b/plugins/tech-radar/src/components/RadarLegend/RadarLegend.test.tsx @@ -20,9 +20,9 @@ import { ThemeProvider } from '@material-ui/core'; import { lightTheme } from '@backstage/theme'; import GetBBoxPolyfill from '../../utils/polyfills/getBBox'; -import RadarLegend from './RadarLegend'; +import RadarLegend, { Props } from './RadarLegend'; -const minProps = { +const minProps: Props = { quadrants: [{ id: 'languages', name: 'Languages' }], rings: [{ id: 'use', name: 'USE', color: '#93c47d' }], entries: [ diff --git a/plugins/tech-radar/src/components/RadarLegend/RadarLegend.tsx b/plugins/tech-radar/src/components/RadarLegend/RadarLegend.tsx index ad204f4beb..59bd2e2337 100644 --- a/plugins/tech-radar/src/components/RadarLegend/RadarLegend.tsx +++ b/plugins/tech-radar/src/components/RadarLegend/RadarLegend.tsx @@ -22,7 +22,7 @@ type Segments = { [k: number]: { [k: number]: Entry[] }; }; -type Props = { +export type Props = { quadrants: Quadrant[]; rings: Ring[]; entries: Entry[]; diff --git a/plugins/tech-radar/src/components/RadarPlot/RadarPlot.test.tsx b/plugins/tech-radar/src/components/RadarPlot/RadarPlot.test.tsx index 31f450baeb..876d3009e4 100644 --- a/plugins/tech-radar/src/components/RadarPlot/RadarPlot.test.tsx +++ b/plugins/tech-radar/src/components/RadarPlot/RadarPlot.test.tsx @@ -20,9 +20,9 @@ import { ThemeProvider } from '@material-ui/core'; import { lightTheme } from '@backstage/theme'; import GetBBoxPolyfill from '../../utils/polyfills/getBBox'; -import RadarPlot from './RadarPlot'; +import RadarPlot, { Props } from './RadarPlot'; -const minProps = { +const minProps: Props = { width: 500, height: 200, radius: 50, diff --git a/plugins/tech-radar/src/components/RadarPlot/RadarPlot.tsx b/plugins/tech-radar/src/components/RadarPlot/RadarPlot.tsx index 7fd46b95b7..c1ce70eecc 100644 --- a/plugins/tech-radar/src/components/RadarPlot/RadarPlot.tsx +++ b/plugins/tech-radar/src/components/RadarPlot/RadarPlot.tsx @@ -23,7 +23,7 @@ import RadarBubble from '../RadarBubble'; import RadarFooter from '../RadarFooter'; import RadarLegend from '../RadarLegend'; -type Props = { +export type Props = { width: number; height: number; radius: number; diff --git a/plugins/tech-radar/src/utils/types.ts b/plugins/tech-radar/src/utils/types.ts index 51f3cee7f8..5ee27c29c7 100644 --- a/plugins/tech-radar/src/utils/types.ts +++ b/plugins/tech-radar/src/utils/types.ts @@ -61,7 +61,7 @@ export type Entry = { // An URL to a longer description as to why this entry is where it is url?: string; // How this entry has recently moved; -1 for "down", +1 for "up", 0 for not moved - moved?: number; + moved?: -1 | 0 | 1; active?: boolean; }; From 984f777b677157b37603f226d8b081b55c4a240e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20DOREAU?= Date: Sat, 20 Jun 2020 17:45:12 +0200 Subject: [PATCH 05/35] refactor(tech-radar): update test + RadarLegend refacto --- .../RadarBubble/RadarBubble.test.tsx | 2 +- .../components/RadarBubble/RadarBubble.tsx | 1 + .../components/RadarEntry/RadarEntry.test.tsx | 6 +- .../src/components/RadarEntry/RadarEntry.tsx | 1 + .../RadarFooter/RadarFooter.test.tsx | 6 +- .../components/RadarFooter/RadarFooter.tsx | 6 +- .../components/RadarGrid/RadarGrid.test.tsx | 3 +- .../src/components/RadarGrid/RadarGrid.tsx | 2 + .../RadarLegend/RadarLegend.test.tsx | 4 +- .../components/RadarLegend/RadarLegend.tsx | 84 +++++++++++-------- .../components/RadarPlot/RadarPlot.test.tsx | 6 +- .../src/components/RadarPlot/RadarPlot.tsx | 2 +- 12 files changed, 81 insertions(+), 42 deletions(-) diff --git a/plugins/tech-radar/src/components/RadarBubble/RadarBubble.test.tsx b/plugins/tech-radar/src/components/RadarBubble/RadarBubble.test.tsx index 9e402b9a91..d56ca743c2 100644 --- a/plugins/tech-radar/src/components/RadarBubble/RadarBubble.test.tsx +++ b/plugins/tech-radar/src/components/RadarBubble/RadarBubble.test.tsx @@ -47,6 +47,6 @@ describe('RadarBubble', () => { , ); - expect(rendered).not.toBeNull(); + expect(rendered.getByText(minProps.text)).toBeInTheDocument(); }); }); diff --git a/plugins/tech-radar/src/components/RadarBubble/RadarBubble.tsx b/plugins/tech-radar/src/components/RadarBubble/RadarBubble.tsx index be38231087..abaed92253 100644 --- a/plugins/tech-radar/src/components/RadarBubble/RadarBubble.tsx +++ b/plugins/tech-radar/src/components/RadarBubble/RadarBubble.tsx @@ -98,6 +98,7 @@ const RadarBubble = (props: Props): JSX.Element => { x={0} y={0} className={visible ? classes.visibleBubble : classes.bubble} + data-testid="radar-bubble" > diff --git a/plugins/tech-radar/src/components/RadarEntry/RadarEntry.test.tsx b/plugins/tech-radar/src/components/RadarEntry/RadarEntry.test.tsx index 14614e88e1..2d24f301d1 100644 --- a/plugins/tech-radar/src/components/RadarEntry/RadarEntry.test.tsx +++ b/plugins/tech-radar/src/components/RadarEntry/RadarEntry.test.tsx @@ -47,6 +47,10 @@ describe('RadarEntry', () => { , ); - expect(rendered).not.toBeNull(); + const radarEntry = rendered.getByTestId('radar-entry'); + const { x, y } = minProps; + expect(radarEntry).toBeInTheDocument(); + expect(radarEntry.getAttribute('transform')).toBe(`translate(${x}, ${y})`); + expect(rendered.getByText(String(minProps.value))).toBeInTheDocument(); }); }); diff --git a/plugins/tech-radar/src/components/RadarEntry/RadarEntry.tsx b/plugins/tech-radar/src/components/RadarEntry/RadarEntry.tsx index 98841dce87..29d35e01e2 100644 --- a/plugins/tech-radar/src/components/RadarEntry/RadarEntry.tsx +++ b/plugins/tech-radar/src/components/RadarEntry/RadarEntry.tsx @@ -80,6 +80,7 @@ const RadarEntry = (props: Props): JSX.Element => { onMouseEnter={onMouseEnter} onMouseLeave={onMouseLeave} onClick={onClick} + data-testid="radar-entry" > {blip} diff --git a/plugins/tech-radar/src/components/RadarFooter/RadarFooter.test.tsx b/plugins/tech-radar/src/components/RadarFooter/RadarFooter.test.tsx index 321947a24c..77f21b6e63 100644 --- a/plugins/tech-radar/src/components/RadarFooter/RadarFooter.test.tsx +++ b/plugins/tech-radar/src/components/RadarFooter/RadarFooter.test.tsx @@ -44,7 +44,9 @@ describe('RadarFooter', () => { , ); - - expect(rendered).not.toBeNull(); + const radarFooter = rendered.getByTestId('radar-footer'); + const { x, y } = minProps; + expect(radarFooter).toBeInTheDocument(); + expect(radarFooter.getAttribute('transform')).toBe(`translate(${x}, ${y})`); }); }); diff --git a/plugins/tech-radar/src/components/RadarFooter/RadarFooter.tsx b/plugins/tech-radar/src/components/RadarFooter/RadarFooter.tsx index 9ba0ed0405..ced1ab8054 100644 --- a/plugins/tech-radar/src/components/RadarFooter/RadarFooter.tsx +++ b/plugins/tech-radar/src/components/RadarFooter/RadarFooter.tsx @@ -36,7 +36,11 @@ const RadarFooter = (props: Props): JSX.Element => { const classes = useStyles(props); return ( - + {'▲ moved up\u00a0\u00a0\u00a0\u00a0\u00a0▼ moved down'} ); diff --git a/plugins/tech-radar/src/components/RadarGrid/RadarGrid.test.tsx b/plugins/tech-radar/src/components/RadarGrid/RadarGrid.test.tsx index 9ba170a0e7..3027cc4adb 100644 --- a/plugins/tech-radar/src/components/RadarGrid/RadarGrid.test.tsx +++ b/plugins/tech-radar/src/components/RadarGrid/RadarGrid.test.tsx @@ -45,6 +45,7 @@ describe('RadarGrid', () => { , ); - expect(rendered).not.toBeNull(); + expect(rendered.getByTestId('radar-grid-x-line')).toBeInTheDocument(); + expect(rendered.getByTestId('radar-grid-y-line')).toBeInTheDocument(); }); }); diff --git a/plugins/tech-radar/src/components/RadarGrid/RadarGrid.tsx b/plugins/tech-radar/src/components/RadarGrid/RadarGrid.tsx index cd06bf6ef8..63c164bc7d 100644 --- a/plugins/tech-radar/src/components/RadarGrid/RadarGrid.tsx +++ b/plugins/tech-radar/src/components/RadarGrid/RadarGrid.tsx @@ -76,6 +76,7 @@ const RadarGrid = (props: Props) => { x2={0} y2={radius} className={classes.axis} + data-testid="radar-grid-x-line" />, // Y axis { x2={radius} y2={0} className={classes.axis} + data-testid="radar-grid-y-line" />, ]; diff --git a/plugins/tech-radar/src/components/RadarLegend/RadarLegend.test.tsx b/plugins/tech-radar/src/components/RadarLegend/RadarLegend.test.tsx index 5900f5c1bc..68bfe892ea 100644 --- a/plugins/tech-radar/src/components/RadarLegend/RadarLegend.test.tsx +++ b/plugins/tech-radar/src/components/RadarLegend/RadarLegend.test.tsx @@ -55,6 +55,8 @@ describe('RadarLegend', () => { , ); - expect(rendered).not.toBeNull(); + expect(rendered.getByTestId('radar-legend')).toBeInTheDocument(); + expect(rendered.getAllByTestId('radar-quadrant')).toHaveLength(1); + expect(rendered.getAllByTestId('radar-ring')).toHaveLength(1); }); }); diff --git a/plugins/tech-radar/src/components/RadarLegend/RadarLegend.tsx b/plugins/tech-radar/src/components/RadarLegend/RadarLegend.tsx index 59bd2e2337..57a453205a 100644 --- a/plugins/tech-radar/src/components/RadarLegend/RadarLegend.tsx +++ b/plugins/tech-radar/src/components/RadarLegend/RadarLegend.tsx @@ -15,7 +15,7 @@ */ import React from 'react'; import { makeStyles, Theme } from '@material-ui/core'; -import { Quadrant, Ring, Entry } from '../../utils/types'; +import type { Quadrant, Ring, Entry } from '../../utils/types'; import { WithLink } from '../../utils/components'; type Segments = { @@ -110,14 +110,21 @@ const RadarLegend = (props: Props): JSX.Element => { : segmentedData[ringIndex + ringOffset] || []; }; - const renderRing = ( - ring: Ring, - entries: Entry[], - onEntryMouseEnter?: Props['onEntryMouseEnter'], - onEntryMouseLeave?: Props['onEntryMouseEnter'], - ) => { + type RadarLegendRingProps = { + ring: Ring; + entries: Entry[]; + onEntryMouseEnter?: Props['onEntryMouseEnter']; + onEntryMouseLeave?: Props['onEntryMouseEnter']; + }; + + const RadarLegendRing = ({ + ring, + entries, + onEntryMouseEnter, + onEntryMouseLeave, + }: RadarLegendRingProps) => { return ( -
    +

    {ring.name}

    {!entries.length ? (

    (empty)

    @@ -145,13 +152,21 @@ const RadarLegend = (props: Props): JSX.Element => { ); }; - const renderQuadrant = ( - segments: Segments, - quadrant: Quadrant, - rings: Ring[], - onEntryMouseEnter: Props['onEntryMouseEnter'], - onEntryMouseLeave: Props['onEntryMouseLeave'], - ) => { + type RadarLegendQuadrantProps = { + segments: Segments; + quadrant: Quadrant; + rings: Ring[]; + onEntryMouseEnter: Props['onEntryMouseEnter']; + onEntryMouseLeave: Props['onEntryMouseLeave']; + }; + + const RadarLegendQuadrant = ({ + segments, + quadrant, + rings, + onEntryMouseEnter, + onEntryMouseLeave, + }: RadarLegendQuadrantProps) => { return ( { y={quadrant.legendY} width={quadrant.legendWidth} height={quadrant.legendHeight} + data-testid="radar-quadrant" >

    {quadrant.name}

    - {rings.map(ring => - renderRing( - ring, - getSegment(segments, quadrant, ring), - onEntryMouseEnter, - onEntryMouseLeave, - ), - )} + {rings.map(ring => ( + + ))}
    @@ -218,16 +235,17 @@ const RadarLegend = (props: Props): JSX.Element => { const segments: Segments = setupSegments(entries); return ( - - {quadrants.map(quadrant => - renderQuadrant( - segments, - quadrant, - rings, - onEntryMouseEnter, - onEntryMouseLeave, - ), - )} + + {quadrants.map(quadrant => ( + + ))} ); }; diff --git a/plugins/tech-radar/src/components/RadarPlot/RadarPlot.test.tsx b/plugins/tech-radar/src/components/RadarPlot/RadarPlot.test.tsx index 876d3009e4..5b17578478 100644 --- a/plugins/tech-radar/src/components/RadarPlot/RadarPlot.test.tsx +++ b/plugins/tech-radar/src/components/RadarPlot/RadarPlot.test.tsx @@ -58,6 +58,10 @@ describe('RadarPlot', () => { , ); - expect(rendered).not.toBeNull(); + expect(rendered.getByTestId('radar-plot')).toBeInTheDocument(); + expect(rendered.getByTestId('radar-legend')).toBeInTheDocument(); + expect(rendered.getByTestId('radar-footer')).toBeInTheDocument(); + expect(rendered.getByTestId('radar-bubble')).toBeInTheDocument(); + expect(rendered.getAllByTestId('radar-entry')).toHaveLength(1); }); }); diff --git a/plugins/tech-radar/src/components/RadarPlot/RadarPlot.tsx b/plugins/tech-radar/src/components/RadarPlot/RadarPlot.tsx index c1ce70eecc..f1b41373ca 100644 --- a/plugins/tech-radar/src/components/RadarPlot/RadarPlot.tsx +++ b/plugins/tech-radar/src/components/RadarPlot/RadarPlot.tsx @@ -50,7 +50,7 @@ const RadarPlot = (props: Props): JSX.Element => { } = props; return ( - + Date: Sat, 20 Jun 2020 18:41:12 +0200 Subject: [PATCH 06/35] fix(tech-radar): moved @types/react to devDependencies --- plugins/tech-radar/package.json | 2 +- plugins/tech-radar/src/components/Radar/Radar.tsx | 2 +- plugins/tech-radar/src/components/Radar/utils.ts | 2 +- plugins/tech-radar/src/components/RadarGrid/RadarGrid.tsx | 2 +- plugins/tech-radar/src/components/RadarPlot/RadarPlot.tsx | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/plugins/tech-radar/package.json b/plugins/tech-radar/package.json index 16a36dba5d..ee9ce910ec 100644 --- a/plugins/tech-radar/package.json +++ b/plugins/tech-radar/package.json @@ -27,7 +27,6 @@ "@material-ui/core": "^4.9.1", "@material-ui/icons": "^4.9.1", "@material-ui/lab": "4.0.0-alpha.45", - "@types/react": "^16.9", "color": "^3.1.2", "d3-force": "^2.0.1", "prop-types": "^15.7.2", @@ -43,6 +42,7 @@ "@testing-library/user-event": "^10.2.4", "@types/color": "^3.0.1", "@types/d3-force": "^1.2.1", + "@types/react": "^16.9", "@types/jest": "^25.2.2", "@types/node": "^12.0.0", "@types/testing-library__jest-dom": "^5.0.4", diff --git a/plugins/tech-radar/src/components/Radar/Radar.tsx b/plugins/tech-radar/src/components/Radar/Radar.tsx index 2c7838e803..124e8bb68a 100644 --- a/plugins/tech-radar/src/components/Radar/Radar.tsx +++ b/plugins/tech-radar/src/components/Radar/Radar.tsx @@ -16,7 +16,7 @@ import React, { useState, useRef } from 'react'; import RadarPlot from '../RadarPlot'; -import { Ring, Quadrant, Entry } from '../../utils/types'; +import type { Ring, Quadrant, Entry } from '../../utils/types'; import { adjustQuadrants, adjustRings, adjustEntries } from './utils'; export type Props = { diff --git a/plugins/tech-radar/src/components/Radar/utils.ts b/plugins/tech-radar/src/components/Radar/utils.ts index 25d9e7dac8..d7a826543c 100644 --- a/plugins/tech-radar/src/components/Radar/utils.ts +++ b/plugins/tech-radar/src/components/Radar/utils.ts @@ -17,7 +17,7 @@ import color from 'color'; import { forceCollide, forceSimulation } from 'd3-force'; import Segment from '../../utils/segment'; -import { Ring, Quadrant, Entry } from '../../utils/types'; +import type { Ring, Quadrant, Entry } from '../../utils/types'; export const adjustQuadrants = ( quadrants: Quadrant[], diff --git a/plugins/tech-radar/src/components/RadarGrid/RadarGrid.tsx b/plugins/tech-radar/src/components/RadarGrid/RadarGrid.tsx index 63c164bc7d..eae85d4e64 100644 --- a/plugins/tech-radar/src/components/RadarGrid/RadarGrid.tsx +++ b/plugins/tech-radar/src/components/RadarGrid/RadarGrid.tsx @@ -16,7 +16,7 @@ import React from 'react'; import { makeStyles, Theme } from '@material-ui/core'; -import { Ring } from '../../utils/types'; +import type { Ring } from '../../utils/types'; export type Props = { radius: number; diff --git a/plugins/tech-radar/src/components/RadarPlot/RadarPlot.tsx b/plugins/tech-radar/src/components/RadarPlot/RadarPlot.tsx index f1b41373ca..1e28cbaf02 100644 --- a/plugins/tech-radar/src/components/RadarPlot/RadarPlot.tsx +++ b/plugins/tech-radar/src/components/RadarPlot/RadarPlot.tsx @@ -15,7 +15,7 @@ */ import React from 'react'; -import { Quadrant, Ring, Entry } from '../../utils/types'; +import type { Quadrant, Ring, Entry } from '../../utils/types'; import RadarGrid from '../RadarGrid'; import RadarEntry from '../RadarEntry'; From fac4893555ec2f4bccb15a1beffd7b8fc5e27853 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20DOREAU?= Date: Sat, 20 Jun 2020 18:52:15 +0200 Subject: [PATCH 07/35] fix(tech-radar): missing state setters --- plugins/tech-radar/src/components/RadarComponent.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/plugins/tech-radar/src/components/RadarComponent.tsx b/plugins/tech-radar/src/components/RadarComponent.tsx index 8d56c6aa0f..86ef10e07d 100644 --- a/plugins/tech-radar/src/components/RadarComponent.tsx +++ b/plugins/tech-radar/src/components/RadarComponent.tsx @@ -37,11 +37,13 @@ const useTechRadarLoader = (props: TechRadarComponentProps) => { .then((payload: TechRadarLoaderResponse) => { setLoading(false); setData(payload); + setError(undefined); }) .catch((err: Error) => { errorApi.post(err); setLoading(false); setError(err); + setData(undefined); }); }, [getData, errorApi]); From 542d209a6bf20be666f562abe01b4c5adefff9b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20DOREAU?= Date: Sat, 20 Jun 2020 23:45:25 +0200 Subject: [PATCH 08/35] refactor(tech-radar): requested changes --- .../src/components/RadarComponent.tsx | 38 +++++++--------- .../components/RadarLegend/RadarLegend.tsx | 44 +++++++------------ .../src/components/RadarPlot/RadarPlot.tsx | 8 ++-- 3 files changed, 36 insertions(+), 54 deletions(-) diff --git a/plugins/tech-radar/src/components/RadarComponent.tsx b/plugins/tech-radar/src/components/RadarComponent.tsx index 86ef10e07d..99190202b9 100644 --- a/plugins/tech-radar/src/components/RadarComponent.tsx +++ b/plugins/tech-radar/src/components/RadarComponent.tsx @@ -14,44 +14,38 @@ * limitations under the License. */ -import React, { useEffect, useState } from 'react'; +import React, { useEffect } from 'react'; import { Progress, useApi, errorApiRef, ErrorApi } from '@backstage/core'; +import { useAsync } from 'react-use'; import Radar from '../components/Radar'; import { TechRadarComponentProps, TechRadarLoaderResponse } from '../api'; import getSampleData from '../sampleData'; const useTechRadarLoader = (props: TechRadarComponentProps) => { const errorApi = useApi(errorApiRef); - const [error, setError] = useState(); - const [loading, setLoading] = useState(true); - const [data, setData] = useState(); const { getData } = props; - useEffect(() => { - if (!getData) { - return; + const state = useAsync(async () => { + if (getData) { + const response: TechRadarLoaderResponse = await getData(); + return response; } - - getData() - .then((payload: TechRadarLoaderResponse) => { - setLoading(false); - setData(payload); - setError(undefined); - }) - .catch((err: Error) => { - errorApi.post(err); - setLoading(false); - setError(err); - setData(undefined); - }); + return undefined; }, [getData, errorApi]); - return { data, loading, error }; + useEffect(() => { + const { error } = state; + if (error) { + errorApi.post(error); + } + }, [errorApi, state]); + + return state; }; const RadarComponent = (props: TechRadarComponentProps): JSX.Element => { - const { loading, error, data } = useTechRadarLoader(props); + const { loading, error, value: data } = useTechRadarLoader(props); return ( <> diff --git a/plugins/tech-radar/src/components/RadarLegend/RadarLegend.tsx b/plugins/tech-radar/src/components/RadarLegend/RadarLegend.tsx index 57a453205a..e319846d78 100644 --- a/plugins/tech-radar/src/components/RadarLegend/RadarLegend.tsx +++ b/plugins/tech-radar/src/components/RadarLegend/RadarLegend.tsx @@ -30,7 +30,20 @@ export type Props = { onEntryMouseLeave?: (entry: Entry) => void; }; -const ringStyles = { +const useStyles = makeStyles(theme => ({ + quadrant: { + height: '100%', + width: '100%', + overflow: 'hidden', + pointerEvents: 'none', + }, + quadrantHeading: { + pointerEvents: 'none', + userSelect: 'none', + marginTop: 0, + marginBottom: theme.spacing(8 / (18 * 0.375)), + fontSize: '18px', + }, rings: { columns: 3, }, @@ -44,7 +57,7 @@ const ringStyles = { pointerEvents: 'none', userSelect: 'none', marginTop: 0, - marginBottom: 'calc(12px * 0.375)', + marginBottom: theme.spacing(8 / (12 * 0.375)), fontSize: '12px', fontWeight: 800, }, @@ -57,25 +70,6 @@ const ringStyles = { '-webkit-font-feature-settings': 'pnum', 'font-feature-settings': 'pnum', }, -}; - -const quadrantStyles = { - quadrant: { - height: '100%', - width: '100%', - overflow: 'hidden', - pointerEvents: 'none', - }, - quadrantHeading: { - pointerEvents: 'none', - userSelect: 'none', - marginTop: 0, - marginBottom: 'calc(18px * 0.375)', - fontSize: '18px', - }, -}; - -const entryStyle = { entry: { pointerEvents: 'none', userSelect: 'none', @@ -84,12 +78,6 @@ const entryStyle = { entryLink: { pointerEvents: 'none', }, -}; - -const useStyles = makeStyles(() => ({ - ringStyles, - quadrantStyles, - entryStyle, })); const RadarLegend = (props: Props): JSX.Element => { @@ -126,7 +114,7 @@ const RadarLegend = (props: Props): JSX.Element => { return (

    {ring.name}

    - {!entries.length ? ( + {entries.length === 0 ? (

    (empty)

    ) : (
      diff --git a/plugins/tech-radar/src/components/RadarPlot/RadarPlot.tsx b/plugins/tech-radar/src/components/RadarPlot/RadarPlot.tsx index 1e28cbaf02..7f68ffe021 100644 --- a/plugins/tech-radar/src/components/RadarPlot/RadarPlot.tsx +++ b/plugins/tech-radar/src/components/RadarPlot/RadarPlot.tsx @@ -71,7 +71,7 @@ const RadarPlot = (props: Props): JSX.Element => { x={entry.x || 0} y={entry.y || 0} color={entry.color || ''} - value={((entry && entry.index) || 0) + 1} + value={(entry?.index || 0) + 1} url={entry.url} moved={entry.moved} onMouseEnter={onEntryMouseEnter && (() => onEntryMouseEnter(entry))} @@ -80,9 +80,9 @@ const RadarPlot = (props: Props): JSX.Element => { ))} From f42a541ade0b5bdce148b559ef0b1530e9abe488 Mon Sep 17 00:00:00 2001 From: blam Date: Wed, 24 Jun 2020 11:18:58 +0200 Subject: [PATCH 09/35] chore(backend-common): Fixing the headers sent issue so we dont send the response twice --- .../src/middleware/errorHandler.test.ts | 23 +++++++++++++++++++ .../src/middleware/errorHandler.ts | 3 +++ 2 files changed, 26 insertions(+) diff --git a/packages/backend-common/src/middleware/errorHandler.test.ts b/packages/backend-common/src/middleware/errorHandler.test.ts index 022802dff8..e8a5a7f47a 100644 --- a/packages/backend-common/src/middleware/errorHandler.test.ts +++ b/packages/backend-common/src/middleware/errorHandler.test.ts @@ -34,6 +34,29 @@ describe('errorHandler', () => { expect(response.text).toBe('some message'); }); + it('doesnt try to send the response again if its already been sent', async () => { + const app = express(); + const mockSend = jest.fn(); + + app.use('/works_with_async_fail', (_, res) => { + res.status(200).send('hello'); + + // mutate the response object to test the middlware. + // it's hard to catch errors inside middleware from the outside. + // @ts-ignore + res.send = mockSend; + throw new Error('some message'); + }); + + app.use(errorHandler()); + const response = await request(app).get('/works_with_async_fail'); + + expect(response.status).toBe(200); + expect(response.text).toBe('hello'); + + expect(mockSend).not.toHaveBeenCalled(); + }); + it('takes code from http-errors library errors', async () => { const app = express(); app.use('/breaks', () => { diff --git a/packages/backend-common/src/middleware/errorHandler.ts b/packages/backend-common/src/middleware/errorHandler.ts index 14f6cfa8d7..ad8f170dcd 100644 --- a/packages/backend-common/src/middleware/errorHandler.ts +++ b/packages/backend-common/src/middleware/errorHandler.ts @@ -53,7 +53,10 @@ export function errorHandler( next: NextFunction, ) => { if (response.headersSent) { + // If the headers have already been sent, do not send the response again + // as this will throw an error in the backend. next(error); + return; } const status = getStatusCode(error); From 73c8c69c1ce88aa83848fc3f283392d586b22443 Mon Sep 17 00:00:00 2001 From: blam Date: Wed, 24 Jun 2020 13:56:25 +0200 Subject: [PATCH 10/35] feat(scaffolder): starting to write a job processor --- plugins/scaffolder-backend/package.json | 1 + .../src/scaffolder/index.ts | 1 + .../src/scaffolder/templater/index.ts | 3 +- .../scaffolder-backend/src/service/router.ts | 44 ++++++++++++------- 4 files changed, 33 insertions(+), 16 deletions(-) diff --git a/plugins/scaffolder-backend/package.json b/plugins/scaffolder-backend/package.json index 061f6ca814..5a8ebddce5 100644 --- a/plugins/scaffolder-backend/package.json +++ b/plugins/scaffolder-backend/package.json @@ -36,6 +36,7 @@ "helmet": "^3.22.0", "morgan": "^1.10.0", "nodegit": "0.26.5", + "uuid": "^8.2.0", "winston": "^3.2.1" }, "devDependencies": { diff --git a/plugins/scaffolder-backend/src/scaffolder/index.ts b/plugins/scaffolder-backend/src/scaffolder/index.ts index 4cc3f21a9d..f2f7d3dad3 100644 --- a/plugins/scaffolder-backend/src/scaffolder/index.ts +++ b/plugins/scaffolder-backend/src/scaffolder/index.ts @@ -16,3 +16,4 @@ export * from './templater'; export * from './prepare'; export * from './templater/cookiecutter'; +export * from './jobs'; diff --git a/plugins/scaffolder-backend/src/scaffolder/templater/index.ts b/plugins/scaffolder-backend/src/scaffolder/templater/index.ts index 8885e65308..d0bbb0aa6c 100644 --- a/plugins/scaffolder-backend/src/scaffolder/templater/index.ts +++ b/plugins/scaffolder-backend/src/scaffolder/templater/index.ts @@ -16,6 +16,7 @@ import type { Writable } from 'stream'; import Docker from 'dockerode'; +import { JsonValue } from '@backstage/config'; export interface RequiredTemplateValues { component_id: string; @@ -23,7 +24,7 @@ export interface RequiredTemplateValues { export interface TemplaterRunOptions { directory: string; - values: RequiredTemplateValues & object; + values: RequiredTemplateValues & Record; logStream?: Writable; dockerClient: Docker; } diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index ab39864e76..a0e31718aa 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -17,10 +17,10 @@ import { Logger } from 'winston'; import Router from 'express-promise-router'; import express from 'express'; -import { PreparerBuilder, TemplaterBase } from '../scaffolder'; +import { PreparerBuilder, TemplaterBase, JobProcessor } from '../scaffolder'; import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; import Docker from 'dockerode'; - +import {} from '@backstage/backend-common'; export interface RouterOptions { preparers: PreparerBuilder; templater: TemplaterBase; @@ -35,10 +35,32 @@ export async function createRouter( const { preparers, templater, logger: parentLogger, dockerClient } = options; const logger = parentLogger.child({ plugin: 'scaffolder' }); + const jobProcessor = new JobProcessor({ + preparers, + templater, + logger, + dockerClient, + }); + + router.get('/v1/job/:jobId', ({ params }, res) => { + const job = jobProcessor.get(params.jobId); + + if (!job) { + return res.status(404).send({ error: 'job not found' }); + } + + res.send({ + id: job.id, + metadata: job.metadata, + status: job.status, + log: job.log, + error: job.error, + }); + }); + router.post('/v1/jobs', async (_, res) => { // TODO(blam): Create a unique job here and return the ID so that // The end user can poll for updates on the current job - res.status(201).json({ accepted: true }); // TODO(blam): Take this entity from the post body sent from the frontend const mockEntity: TemplateEntityV1alpha1 = { @@ -64,20 +86,12 @@ export async function createRouter( }, }; - // Get the preparer for the mock entity - const preparer = preparers.get(mockEntity); + const job = jobProcessor.create(mockEntity, { component_id: 'test' }); + res.status(201).json({ jobId: job.id }); - // Run the preparer for the mock entity to produce a temporary directory with template in - const skeletonPath = await preparer.prepare(mockEntity); + jobProcessor.run(job); - // Run the templater on the mock directory with values from the post body - const templatedPath = await templater.run({ - directory: skeletonPath, - values: { component_id: 'test' }, - dockerClient, - }); - - console.warn(templatedPath); + // console.warn(templatedPath); }); const app = express(); From 7787eb99865ef163aadb142957518477c282d258 Mon Sep 17 00:00:00 2001 From: blam Date: Wed, 24 Jun 2020 13:56:43 +0200 Subject: [PATCH 11/35] feat(scaffolder): implementing a basic API for the processor to mutate the entity state --- .../src/scaffolder/jobs/index.ts | 16 +++ .../src/scaffolder/jobs/processor.ts | 110 ++++++++++++++++++ .../src/scaffolder/jobs/types.ts | 57 +++++++++ 3 files changed, 183 insertions(+) create mode 100644 plugins/scaffolder-backend/src/scaffolder/jobs/index.ts create mode 100644 plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts create mode 100644 plugins/scaffolder-backend/src/scaffolder/jobs/types.ts diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/index.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/index.ts new file mode 100644 index 0000000000..303987c5b1 --- /dev/null +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/index.ts @@ -0,0 +1,16 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +export * from './processor'; diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts new file mode 100644 index 0000000000..bc9fe4698f --- /dev/null +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts @@ -0,0 +1,110 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { Processor, Job, ProcessorContstructorArgs } from './types'; +import { JsonValue } from '@backstage/config'; +import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; +import { PassThrough, Writable } from 'stream'; +import uuid from 'uuid'; +import winston from 'winston'; +import { RequiredTemplateValues } from '../templater'; +import { createNewRootLogger } from '@backstage/backend-common'; + +export class JobProcessor implements Processor { + private preparers: ProcessorContstructorArgs['preparers']; + private templater: ProcessorContstructorArgs['templater']; + private dockerClient: ProcessorContstructorArgs['dockerClient']; + private jobs = new Map(); + + constructor({ + preparers, + templater, + dockerClient, + }: ProcessorContstructorArgs) { + this.preparers = preparers; + this.templater = templater; + this.dockerClient = dockerClient; + return this; + } + + create( + entity: TemplateEntityV1alpha1, + values: RequiredTemplateValues & Record, + ): Job { + const id = uuid.v4(); + const log: string[] = []; + const logStream = new PassThrough(); + logStream.on('data', chunk => log.push(chunk.toString())); + + const logger = createNewRootLogger(); + logger.add(new winston.transports.Stream({ stream: logStream })); + + const job: Job = { + id, + logStream, + logger, + log, + status: 'PENDING', + metadata: { + entity, + values, + }, + }; + + this.jobs.set(job.id, job); + return job; + } + get(id: string): Job | undefined { + return this.jobs.get(id); + } + async run(job: Job) { + if (job.status !== 'PENDING') { + throw new Error('Job is not in pending state'); + } + + const { logger, logStream } = job; + + try { + logger.debug('Prepare started'); + job.status = 'PREPARING'; + const entity = job.metadata.entity; + const preparer = this.preparers.get(entity); + const skeletonPath = await preparer.prepare(entity); + logger.debug('Prepare finished', { + skeletonPath, + }); + + logger.debug('Templating started'); + job.status = 'TEMPLATING'; + // Run the templater on the mock directory with values from the post body + const templatedPath = await this.templater.run({ + directory: skeletonPath, + values: job.metadata.values, + dockerClient: this.dockerClient, + logStream, + }); + logger.debug('Template finished', { templatedPath }); + + job.status = 'STORING'; + // TODO(blam): Implement VCS Push here + + job.status = 'COMPLETE'; + } catch (ex) { + job.error = ex; + job.status = 'FAILED'; + logger.error(`job ${job.id} failed with reason`, { ex }); + } + } +} diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts new file mode 100644 index 0000000000..05630e2c63 --- /dev/null +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts @@ -0,0 +1,57 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import type { Writable } from 'stream'; +import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; +import { JsonValue } from '@backstage/config'; +import { PreparerBuilder } from '../prepare'; +import Docker from 'dockerode'; +import { TemplaterBase, RequiredTemplateValues } from '../templater'; +import { Logger } from 'winston'; + +export type Job = { + id: string; + metadata: { + entity: TemplateEntityV1alpha1; + values: RequiredTemplateValues & Record; + }; + status: + | 'PENDING' + | 'PREPARING' + | 'TEMPLATING' + | 'STORING' + | 'COMPLETE' + | 'FAILED'; + logStream: Writable; + log: string[]; + logger: Logger; + error?: Error; +}; + +export type ProcessorContstructorArgs = { + preparers: PreparerBuilder; + templater: TemplaterBase; + logger: Logger; + dockerClient: Docker; +}; + +export type Processor = { + create( + entity: TemplateEntityV1alpha1, + values: RequiredTemplateValues & Record, + ): Job; + + get(id: string): Job | undefined; +}; From 85d1fbaf3c8c4642e3dc587cc070bd3cbe9be482 Mon Sep 17 00:00:00 2001 From: blam Date: Wed, 24 Jun 2020 13:57:19 +0200 Subject: [PATCH 12/35] chore(backend-common): expose a create logger function --- .../backend-common/src/logging/rootLogger.ts | 40 ++++++++++--------- yarn.lock | 5 +++ 2 files changed, 27 insertions(+), 18 deletions(-) diff --git a/packages/backend-common/src/logging/rootLogger.ts b/packages/backend-common/src/logging/rootLogger.ts index 11db57c1ed..47379b027c 100644 --- a/packages/backend-common/src/logging/rootLogger.ts +++ b/packages/backend-common/src/logging/rootLogger.ts @@ -16,24 +16,28 @@ import * as winston from 'winston'; -let rootLogger: winston.Logger = winston.createLogger({ - level: process.env.LOG_LEVEL || 'info', - format: - process.env.NODE_ENV === 'production' - ? winston.format.json() - : winston.format.combine( - winston.format.colorize(), - winston.format.timestamp(), - winston.format.simple(), - ), - defaultMeta: { service: 'backstage' }, - transports: [ - new winston.transports.Console({ - silent: - process.env.JEST_WORKER_ID !== undefined && !process.env.LOG_LEVEL, - }), - ], -}); +export function createNewRootLogger(): winston.Logger { + return winston.createLogger({ + level: process.env.LOG_LEVEL || 'info', + format: + process.env.NODE_ENV === 'production' + ? winston.format.json() + : winston.format.combine( + winston.format.colorize(), + winston.format.timestamp(), + winston.format.simple(), + ), + defaultMeta: { service: 'backstage' }, + transports: [ + new winston.transports.Console({ + silent: + process.env.JEST_WORKER_ID !== undefined && !process.env.LOG_LEVEL, + }), + ], + }); +} + +let rootLogger: winston.Logger = createNewRootLogger(); export function getRootLogger(): winston.Logger { return rootLogger; diff --git a/yarn.lock b/yarn.lock index c9128c5350..ba53ee7513 100644 --- a/yarn.lock +++ b/yarn.lock @@ -18918,6 +18918,11 @@ uuid@^8.0.0: resolved "https://registry.npmjs.org/uuid/-/uuid-8.1.0.tgz#6f1536eb43249f473abc6bd58ff983da1ca30d8d" integrity sha512-CI18flHDznR0lq54xBycOVmphdCYnQLKn8abKn7PXUiKUGdEd+/l9LWNJmugXel4hXq7S+RMNl34ecyC9TntWg== +uuid@^8.2.0: + version "8.2.0" + resolved "https://registry.npmjs.org/uuid/-/uuid-8.2.0.tgz#cb10dd6b118e2dada7d0cd9730ba7417c93d920e" + integrity sha512-CYpGiFTUrmI6OBMkAdjSDM0k5h8SkkiTP4WAjQgDgNB1S3Ou9VBEvr6q0Kv2H1mMk7IWfxYGpMH5sd5AvcIV2Q== + v8-compile-cache@^2.0.3: version "2.1.0" resolved "https://registry.npmjs.org/v8-compile-cache/-/v8-compile-cache-2.1.0.tgz#e14de37b31a6d194f5690d67efc4e7f6fc6ab30e" From 1baa5309a17111633aea2e3e7f09a29df4b9ad5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20DOREAU?= Date: Wed, 24 Jun 2020 14:03:03 +0200 Subject: [PATCH 13/35] refactor(tech-radar): add moved type --- plugins/tech-radar/src/api.ts | 3 ++- plugins/tech-radar/src/utils/types.ts | 8 +++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/plugins/tech-radar/src/api.ts b/plugins/tech-radar/src/api.ts index d7f5b57a43..856cf4ff3c 100644 --- a/plugins/tech-radar/src/api.ts +++ b/plugins/tech-radar/src/api.ts @@ -15,6 +15,7 @@ */ import { createApiRef } from '@backstage/core'; +import { MovedState } from './utils/types'; /** * Types related to the Radar's visualization. @@ -34,7 +35,7 @@ export interface RadarQuadrant { export interface RadarEntry { key: string; // react key id: string; - moved: -1 | 0 | 1; + moved: MovedState; quadrant: RadarQuadrant; ring: RadarRing; title: string; diff --git a/plugins/tech-radar/src/utils/types.ts b/plugins/tech-radar/src/utils/types.ts index 5ee27c29c7..f3933ee584 100644 --- a/plugins/tech-radar/src/utils/types.ts +++ b/plugins/tech-radar/src/utils/types.ts @@ -45,6 +45,12 @@ export type Segment = { random: Function; }; +export enum MovedState { + Down = -1, + NoChange = 0, + Up = 1, +} + export type Entry = { id: string; index?: number; @@ -61,7 +67,7 @@ export type Entry = { // An URL to a longer description as to why this entry is where it is url?: string; // How this entry has recently moved; -1 for "down", +1 for "up", 0 for not moved - moved?: -1 | 0 | 1; + moved?: MovedState; active?: boolean; }; From bca0ebf3c9e08577ddaec2d6e5b1680b97e4fc6f Mon Sep 17 00:00:00 2001 From: Ivan Shmidt Date: Thu, 25 Jun 2020 00:15:58 +0200 Subject: [PATCH 14/35] fix(scaffolder): stringify error --- plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts index bc9fe4698f..29b67b5e14 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts @@ -104,7 +104,7 @@ export class JobProcessor implements Processor { } catch (ex) { job.error = ex; job.status = 'FAILED'; - logger.error(`job ${job.id} failed with reason`, { ex }); + logger.error(`job ${job.id} failed with reason: ${ex}`); } } } From 1dd56d28a9fc8245f97c4bcfd51f4a1d8b2d17dd Mon Sep 17 00:00:00 2001 From: Ivan Shmidt Date: Thu, 25 Jun 2020 00:16:26 +0200 Subject: [PATCH 15/35] feat(scaffolder): create new tempdir for result --- .../src/scaffolder/templater/cookiecutter.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/templater/cookiecutter.ts b/plugins/scaffolder-backend/src/scaffolder/templater/cookiecutter.ts index a95eee7fb4..cb11bf04af 100644 --- a/plugins/scaffolder-backend/src/scaffolder/templater/cookiecutter.ts +++ b/plugins/scaffolder-backend/src/scaffolder/templater/cookiecutter.ts @@ -48,10 +48,7 @@ export class CookieCutter implements TemplaterBase { await fs.writeJSON(`${options.directory}/cookiecutter.json`, cookieInfo); const templateDir = options.directory; - - // TODO(blam): This should be an entirely different directory on the host machine - // not in the template directory - const resultDir = `${templateDir}/result`; + const resultDir = await fs.promises.mkdtemp(`${options.directory}-result`); await runDockerContainer({ imageName: 'backstage/cookiecutter', From 82321cb4002e5adc850bec3988957e7bf9140212 Mon Sep 17 00:00:00 2001 From: blam Date: Thu, 25 Jun 2020 04:21:43 +0200 Subject: [PATCH 16/35] chore(scaffolder): fixing issues with scaffolder --- .../src/scaffolder/jobs/processor.test.ts | 16 +++++++++++++++ .../src/scaffolder/jobs/processor.ts | 20 ++++++++++++++----- 2 files changed, 31 insertions(+), 5 deletions(-) create mode 100644 plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts new file mode 100644 index 0000000000..1db114f597 --- /dev/null +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts @@ -0,0 +1,16 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +describe('JobProcessor', () => {}); diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts index bc9fe4698f..0c2a6ceae2 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts @@ -16,7 +16,7 @@ import { Processor, Job, ProcessorContstructorArgs } from './types'; import { JsonValue } from '@backstage/config'; import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; -import { PassThrough, Writable } from 'stream'; +import { PassThrough } from 'stream'; import uuid from 'uuid'; import winston from 'winston'; import { RequiredTemplateValues } from '../templater'; @@ -45,9 +45,16 @@ export class JobProcessor implements Processor { ): Job { const id = uuid.v4(); const log: string[] = []; + + // Create an empty stream to collect all the log lines into + // one variable for the API. const logStream = new PassThrough(); logStream.on('data', chunk => log.push(chunk.toString())); + // TODO(blam): Maybe this is not the right way to build the logger + // Maybe we want to be more ux specific and drop the json support. + // Child loggers can not have specific transports which sucks, so we have to + // create another here. const logger = createNewRootLogger(); logger.add(new winston.transports.Stream({ stream: logStream })); @@ -64,6 +71,7 @@ export class JobProcessor implements Processor { }; this.jobs.set(job.id, job); + return job; } get(id: string): Job | undefined { @@ -77,6 +85,7 @@ export class JobProcessor implements Processor { const { logger, logStream } = job; try { + // Prepare a folder for the templater to run in logger.debug('Prepare started'); job.status = 'PREPARING'; const entity = job.metadata.entity; @@ -86,9 +95,9 @@ export class JobProcessor implements Processor { skeletonPath, }); + // Run the templater on the directory with values passed in logger.debug('Templating started'); job.status = 'TEMPLATING'; - // Run the templater on the mock directory with values from the post body const templatedPath = await this.templater.run({ directory: skeletonPath, values: job.metadata.values, @@ -97,14 +106,15 @@ export class JobProcessor implements Processor { }); logger.debug('Template finished', { templatedPath }); + // Store the template somewhere when finished job.status = 'STORING'; // TODO(blam): Implement VCS Push here job.status = 'COMPLETE'; - } catch (ex) { - job.error = ex; + } catch (error) { + job.error = error; job.status = 'FAILED'; - logger.error(`job ${job.id} failed with reason`, { ex }); + logger.error(`Job failed with error ${error.message}`); } } } From 5a88ef753ba1efb5af3d8168366df4c8f6f26118 Mon Sep 17 00:00:00 2001 From: blam Date: Thu, 25 Jun 2020 23:29:54 +0200 Subject: [PATCH 17/35] chore(scaffolder): Reworking how the processor works. It's starting to look a lot cleaner now --- .../src/scaffolder/jobs/processor.test.ts | 34 ++++++- .../src/scaffolder/jobs/processor.ts | 93 +++++++++++-------- .../src/scaffolder/jobs/types.ts | 11 +-- .../scaffolder-backend/src/service/router.ts | 86 ++++++++--------- 4 files changed, 129 insertions(+), 95 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts index 1db114f597..aacbcf61f0 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts @@ -13,4 +13,36 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -describe('JobProcessor', () => {}); +import { JobProcessor } from './processor'; +import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; +describe('JobProcessor', () => { + describe('create', () => { + const mockEntity: TemplateEntityV1alpha1 = { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Template', + metadata: { + annotations: { + 'backstage.io/managed-by-location': + 'github:https://github.com/benjdlambert/backstage-graphql-template/blob/master/template.yaml', + }, + name: 'graphql-starter', + title: 'GraphQL Service', + description: + 'A GraphQL starter template for backstage to get you up and running\nthe best pracices with GraphQL\n', + uid: '9cf16bad-16e0-4213-b314-c4eec773c50b', + etag: 'ZTkxMjUxMjUtYWY3Yi00MjU2LWFkYWMtZTZjNjU5ZjJhOWM2', + + generation: 1, + }, + spec: { + type: 'cookiecutter', + path: './template', + }, + }; + const processor = new JobProcessor(); + + it('should create a unique id for the job', async () => { + const job = processor.create(); + }); + }); +}); diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts index 0c2a6ceae2..4e90b457a7 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts @@ -13,30 +13,38 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { Processor, Job, ProcessorContstructorArgs } from './types'; +import { Processor, Job } from './types'; import { JsonValue } from '@backstage/config'; import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; import { PassThrough } from 'stream'; import uuid from 'uuid'; +import Docker from 'dockerode'; import winston from 'winston'; -import { RequiredTemplateValues } from '../templater'; +import { RequiredTemplateValues, TemplaterBase } from '../templater'; import { createNewRootLogger } from '@backstage/backend-common'; +import { PreparerBuilder } from '../prepare'; + +export type JobProcessorArguments = { + preparers: PreparerBuilder; + templater: TemplaterBase; + dockerClient: Docker; +}; + +export type JobAndDirectoryTuple = { + job: Job; + directory: string; +}; export class JobProcessor implements Processor { - private preparers: ProcessorContstructorArgs['preparers']; - private templater: ProcessorContstructorArgs['templater']; - private dockerClient: ProcessorContstructorArgs['dockerClient']; + private preparers: PreparerBuilder; + private templater: TemplaterBase; + private dockerClient: Docker; private jobs = new Map(); - constructor({ - preparers, - templater, - dockerClient, - }: ProcessorContstructorArgs) { + constructor({ preparers, templater, dockerClient }: JobProcessorArguments) { this.preparers = preparers; this.templater = templater; this.dockerClient = dockerClient; - return this; } create( @@ -74,47 +82,50 @@ export class JobProcessor implements Processor { return job; } + get(id: string): Job | undefined { return this.jobs.get(id); } - async run(job: Job) { + + private async prepare(job: Job): Promise { + job.status = 'PREPARING'; + const entity = job.metadata.entity; + const preparer = this.preparers.get(entity); + return await preparer.prepare(entity); + } + + private async run(job: Job, directory: string): Promise { + job.status = 'TEMPLATING'; + return await this.templater.run({ + directory, + values: job.metadata.values, + dockerClient: this.dockerClient, + logStream: job.logStream, + }); + } + + private async store(job: Job): Promise { + job.status = 'STORING'; + } + + private async complete(job: Job): Promise { + job.status = 'COMPLETE'; + } + + async process(job: Job) { if (job.status !== 'PENDING') { throw new Error('Job is not in pending state'); } - const { logger, logStream } = job; - try { - // Prepare a folder for the templater to run in - logger.debug('Prepare started'); - job.status = 'PREPARING'; - const entity = job.metadata.entity; - const preparer = this.preparers.get(entity); - const skeletonPath = await preparer.prepare(entity); - logger.debug('Prepare finished', { - skeletonPath, - }); - - // Run the templater on the directory with values passed in - logger.debug('Templating started'); - job.status = 'TEMPLATING'; - const templatedPath = await this.templater.run({ - directory: skeletonPath, - values: job.metadata.values, - dockerClient: this.dockerClient, - logStream, - }); - logger.debug('Template finished', { templatedPath }); - - // Store the template somewhere when finished - job.status = 'STORING'; - // TODO(blam): Implement VCS Push here - - job.status = 'COMPLETE'; + const skeletonPath = await this.prepare(job); + await this.run(job, skeletonPath); + await this.store(job); + await this.complete(job); } catch (error) { job.error = error; job.status = 'FAILED'; - logger.error(`Job failed with error ${error.message}`); + job.logger.error(`Job failed with error ${error.message}`); } } } diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts index 05630e2c63..e6c5436e95 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts @@ -16,9 +16,7 @@ import type { Writable } from 'stream'; import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; import { JsonValue } from '@backstage/config'; -import { PreparerBuilder } from '../prepare'; -import Docker from 'dockerode'; -import { TemplaterBase, RequiredTemplateValues } from '../templater'; +import { RequiredTemplateValues } from '../templater'; import { Logger } from 'winston'; export type Job = { @@ -40,13 +38,6 @@ export type Job = { error?: Error; }; -export type ProcessorContstructorArgs = { - preparers: PreparerBuilder; - templater: TemplaterBase; - logger: Logger; - dockerClient: Docker; -}; - export type Processor = { create( entity: TemplateEntityV1alpha1, diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index a0e31718aa..4d64bc159f 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -42,57 +42,57 @@ export async function createRouter( dockerClient, }); - router.get('/v1/job/:jobId', ({ params }, res) => { - const job = jobProcessor.get(params.jobId); + router + .get('/v1/job/:jobId', ({ params }, res) => { + const job = jobProcessor.get(params.jobId); - if (!job) { - return res.status(404).send({ error: 'job not found' }); - } + if (!job) { + return res.status(404).send({ error: 'job not found' }); + } - res.send({ - id: job.id, - metadata: job.metadata, - status: job.status, - log: job.log, - error: job.error, - }); - }); + res.send({ + id: job.id, + metadata: job.metadata, + status: job.status, + log: job.log, + error: job.error, + }); + }) + .post('/v1/jobs', async (_, res) => { + // TODO(blam): Create a unique job here and return the ID so that + // The end user can poll for updates on the current job - router.post('/v1/jobs', async (_, res) => { - // TODO(blam): Create a unique job here and return the ID so that - // The end user can poll for updates on the current job + // TODO(blam): Take this entity from the post body sent from the frontend + const mockEntity: TemplateEntityV1alpha1 = { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Template', + metadata: { + annotations: { + 'backstage.io/managed-by-location': + 'github:https://github.com/benjdlambert/backstage-graphql-template/blob/master/template.yaml', + }, + name: 'graphql-starter', + title: 'GraphQL Service', + description: + 'A GraphQL starter template for backstage to get you up and running\nthe best pracices with GraphQL\n', + uid: '9cf16bad-16e0-4213-b314-c4eec773c50b', + etag: 'ZTkxMjUxMjUtYWY3Yi00MjU2LWFkYWMtZTZjNjU5ZjJhOWM2', - // TODO(blam): Take this entity from the post body sent from the frontend - const mockEntity: TemplateEntityV1alpha1 = { - apiVersion: 'backstage.io/v1alpha1', - kind: 'Template', - metadata: { - annotations: { - 'backstage.io/managed-by-location': - 'github:https://github.com/benjdlambert/backstage-graphql-template/blob/master/template.yaml', + generation: 1, }, - name: 'graphql-starter', - title: 'GraphQL Service', - description: - 'A GraphQL starter template for backstage to get you up and running\nthe best pracices with GraphQL\n', - uid: '9cf16bad-16e0-4213-b314-c4eec773c50b', - etag: 'ZTkxMjUxMjUtYWY3Yi00MjU2LWFkYWMtZTZjNjU5ZjJhOWM2', + spec: { + type: 'cookiecutter', + path: './template', + }, + }; - generation: 1, - }, - spec: { - type: 'cookiecutter', - path: './template', - }, - }; + const job = jobProcessor.create(mockEntity, { component_id: 'test' }); + res.status(201).json({ jobId: job.id }); - const job = jobProcessor.create(mockEntity, { component_id: 'test' }); - res.status(201).json({ jobId: job.id }); + jobProcessor.run(job); - jobProcessor.run(job); - - // console.warn(templatedPath); - }); + // console.warn(templatedPath); + }); const app = express(); app.set('logger', logger); From 1655b8c16e93882070a58ba280b750e275019d14 Mon Sep 17 00:00:00 2001 From: blam Date: Thu, 25 Jun 2020 23:39:36 +0200 Subject: [PATCH 18/35] chore(scaffolder): added some more tests and more refactoring --- .../src/scaffolder/jobs/processor.test.ts | 56 ++++++++++--------- .../scaffolder/templater/cookiecutter.test.ts | 6 +- 2 files changed, 33 insertions(+), 29 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts index aacbcf61f0..49b5ef7f54 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts @@ -16,33 +16,37 @@ import { JobProcessor } from './processor'; import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; describe('JobProcessor', () => { + const mockEntity: TemplateEntityV1alpha1 = { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Template', + metadata: { + annotations: { + 'backstage.io/managed-by-location': + 'github:https://github.com/benjdlambert/backstage-graphql-template/blob/master/template.yaml', + }, + name: 'graphql-starter', + title: 'GraphQL Service', + description: + 'A GraphQL starter template for backstage to get you up and running\nthe best pracices with GraphQL\n', + uid: '9cf16bad-16e0-4213-b314-c4eec773c50b', + etag: 'ZTkxMjUxMjUtYWY3Yi00MjU2LWFkYWMtZTZjNjU5ZjJhOWM2', + + generation: 1, + }, + spec: { + type: 'cookiecutter', + path: './template', + }, + }; + describe('create', () => { - const mockEntity: TemplateEntityV1alpha1 = { - apiVersion: 'backstage.io/v1alpha1', - kind: 'Template', - metadata: { - annotations: { - 'backstage.io/managed-by-location': - 'github:https://github.com/benjdlambert/backstage-graphql-template/blob/master/template.yaml', - }, - name: 'graphql-starter', - title: 'GraphQL Service', - description: - 'A GraphQL starter template for backstage to get you up and running\nthe best pracices with GraphQL\n', - uid: '9cf16bad-16e0-4213-b314-c4eec773c50b', - etag: 'ZTkxMjUxMjUtYWY3Yi00MjU2LWFkYWMtZTZjNjU5ZjJhOWM2', + it.todo('creates a new job'); + }); - generation: 1, - }, - spec: { - type: 'cookiecutter', - path: './template', - }, - }; - const processor = new JobProcessor(); - - it('should create a unique id for the job', async () => { - const job = processor.create(); - }); + describe('process', () => { + it.todo('allows running of a job in a pending state'); + it.todo('fails when the job is not in a pending state'); + it.todo('calls the preparer with the entity'); + it.todo('calls the templater with the correct directory'); }); }); diff --git a/plugins/scaffolder-backend/src/scaffolder/templater/cookiecutter.test.ts b/plugins/scaffolder-backend/src/scaffolder/templater/cookiecutter.test.ts index 7c78badff1..f7e2d68719 100644 --- a/plugins/scaffolder-backend/src/scaffolder/templater/cookiecutter.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/templater/cookiecutter.test.ts @@ -100,7 +100,7 @@ describe('CookieCutter Templater', () => { imageName: 'backstage/cookiecutter', args: ['cookiecutter', '--no-input', '-o', '/result', '/template'], templateDir: tempdir, - resultDir: `${tempdir}/result`, + resultDir: `${tempdir}-result`, logStream: undefined, dockerClient: mockDocker, }); @@ -119,7 +119,7 @@ describe('CookieCutter Templater', () => { dockerClient: mockDocker, }); - expect(path).toBe(`${tempdir}/result`); + expect(path).toBe(`${tempdir}-result`); }); it('should pass through the streamer to the run docker helper', async () => { @@ -143,7 +143,7 @@ describe('CookieCutter Templater', () => { imageName: 'backstage/cookiecutter', args: ['cookiecutter', '--no-input', '-o', '/result', '/template'], templateDir: tempdir, - resultDir: `${tempdir}/result`, + resultDir: `${tempdir}-result`, logStream: stream, dockerClient: mockDocker, }); From e0f026bb392a440ab660aaffa22cbe84eef7c2da Mon Sep 17 00:00:00 2001 From: blam Date: Fri, 26 Jun 2020 13:53:14 +0200 Subject: [PATCH 19/35] chore(scaffolder): fixing cookiecutter templater tests --- .../src/scaffolder/templater/cookiecutter.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/templater/cookiecutter.test.ts b/plugins/scaffolder-backend/src/scaffolder/templater/cookiecutter.test.ts index f7e2d68719..f31dd7cdd0 100644 --- a/plugins/scaffolder-backend/src/scaffolder/templater/cookiecutter.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/templater/cookiecutter.test.ts @@ -100,7 +100,7 @@ describe('CookieCutter Templater', () => { imageName: 'backstage/cookiecutter', args: ['cookiecutter', '--no-input', '-o', '/result', '/template'], templateDir: tempdir, - resultDir: `${tempdir}-result`, + resultDir: expect.stringContaining(`${tempdir}-result`), logStream: undefined, dockerClient: mockDocker, }); @@ -119,7 +119,7 @@ describe('CookieCutter Templater', () => { dockerClient: mockDocker, }); - expect(path).toBe(`${tempdir}-result`); + expect(path.startsWith(`${tempdir}-result`)).toBeTruthy(); }); it('should pass through the streamer to the run docker helper', async () => { @@ -143,7 +143,7 @@ describe('CookieCutter Templater', () => { imageName: 'backstage/cookiecutter', args: ['cookiecutter', '--no-input', '-o', '/result', '/template'], templateDir: tempdir, - resultDir: `${tempdir}-result`, + resultDir: expect.stringContaining(`${tempdir}-result`), logStream: stream, dockerClient: mockDocker, }); From f2d01c5cb4fe529ee48f99f025bbd9c608d0c775 Mon Sep 17 00:00:00 2001 From: blam Date: Fri, 26 Jun 2020 14:19:13 +0200 Subject: [PATCH 20/35] chore(scaffolder): adding some more tests for scaffolder processor --- .../src/scaffolder/jobs/processor.test.ts | 21 ++++++++++++++++++- .../src/scaffolder/jobs/processor.ts | 2 +- .../scaffolder/templater/cookiecutter.test.ts | 1 + .../scaffolder-backend/src/service/router.ts | 8 +++---- 4 files changed, 25 insertions(+), 7 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts index 49b5ef7f54..8c3643b10b 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts @@ -15,6 +15,10 @@ */ import { JobProcessor } from './processor'; import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; +import Docker from 'dockerode'; +import { CookieCutter } from '../templater/cookiecutter'; +import { Preparers } from '../'; + describe('JobProcessor', () => { const mockEntity: TemplateEntityV1alpha1 = { apiVersion: 'backstage.io/v1alpha1', @@ -40,7 +44,22 @@ describe('JobProcessor', () => { }; describe('create', () => { - it.todo('creates a new job'); + const templater = new CookieCutter(); + const preparers = new Preparers(); + const mockDocker = {} as jest.Mocked; + it('creates a new job', async () => { + const processor = new JobProcessor({ + dockerClient: mockDocker, + preparers, + templater, + }); + + const job = processor.create(mockEntity, { component_id: 'bob' }); + + expect(job.id).toMatch( + /^[0-9A-F]{8}-[0-9A-F]{4}-4[0-9A-F]{3}-[89AB][0-9A-F]{3}-[0-9A-F]{12}$/i, + ); + }); }); describe('process', () => { diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts index 4e90b457a7..341ae6724e 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts @@ -17,7 +17,7 @@ import { Processor, Job } from './types'; import { JsonValue } from '@backstage/config'; import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; import { PassThrough } from 'stream'; -import uuid from 'uuid'; +import * as uuid from 'uuid'; import Docker from 'dockerode'; import winston from 'winston'; import { RequiredTemplateValues, TemplaterBase } from '../templater'; diff --git a/plugins/scaffolder-backend/src/scaffolder/templater/cookiecutter.test.ts b/plugins/scaffolder-backend/src/scaffolder/templater/cookiecutter.test.ts index f31dd7cdd0..82e84bc1d3 100644 --- a/plugins/scaffolder-backend/src/scaffolder/templater/cookiecutter.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/templater/cookiecutter.test.ts @@ -105,6 +105,7 @@ describe('CookieCutter Templater', () => { dockerClient: mockDocker, }); }); + it('should return the result path to the end templated folder', async () => { const tempdir = os.tmpdir(); diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 4d64bc159f..5957b66915 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -38,7 +38,6 @@ export async function createRouter( const jobProcessor = new JobProcessor({ preparers, templater, - logger, dockerClient, }); @@ -47,7 +46,8 @@ export async function createRouter( const job = jobProcessor.get(params.jobId); if (!job) { - return res.status(404).send({ error: 'job not found' }); + res.status(404).send({ error: 'job not found' }); + return; } res.send({ @@ -89,9 +89,7 @@ export async function createRouter( const job = jobProcessor.create(mockEntity, { component_id: 'test' }); res.status(201).json({ jobId: job.id }); - jobProcessor.run(job); - - // console.warn(templatedPath); + jobProcessor.process(job); }); const app = express(); From b401cf2f1789b03f647bf9452e9d55322b9a730b Mon Sep 17 00:00:00 2001 From: blam Date: Fri, 26 Jun 2020 17:11:46 +0200 Subject: [PATCH 21/35] chore(scaffolder): Updating scaffolder tests to start runnning some stuff --- .../src/scaffolder/jobs/processor.test.ts | 82 +++++++++++++++++-- .../src/scaffolder/jobs/processor.ts | 2 +- 2 files changed, 78 insertions(+), 6 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts index 8c3643b10b..b8f1c8f86e 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts @@ -18,6 +18,7 @@ import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; import Docker from 'dockerode'; import { CookieCutter } from '../templater/cookiecutter'; import { Preparers } from '../'; +import { Job } from './types'; describe('JobProcessor', () => { const mockEntity: TemplateEntityV1alpha1 = { @@ -43,6 +44,8 @@ describe('JobProcessor', () => { }, }; + const mockValues = { component_id: 'bob' }; + describe('create', () => { const templater = new CookieCutter(); const preparers = new Preparers(); @@ -54,18 +57,87 @@ describe('JobProcessor', () => { templater, }); - const job = processor.create(mockEntity, { component_id: 'bob' }); + const job = processor.create(mockEntity, mockValues); expect(job.id).toMatch( /^[0-9A-F]{8}-[0-9A-F]{4}-4[0-9A-F]{3}-[89AB][0-9A-F]{3}-[0-9A-F]{12}$/i, ); + + expect(job.log).toEqual([]); + expect(job.status).toBe('PENDING'); + expect(job.metadata.entity).toBe(mockEntity); + expect(job.metadata.values).toBe(mockValues); }); }); describe('process', () => { - it.todo('allows running of a job in a pending state'); - it.todo('fails when the job is not in a pending state'); - it.todo('calls the preparer with the entity'); - it.todo('calls the templater with the correct directory'); + const preparers = new Preparers(); + const mockDocker = {} as jest.Mocked; + const mockPreparer = { prepare: jest.fn() }; + const templater = { run: jest.fn() }; + + const createJob = (): { job: Job; processor: JobProcessor } => { + preparers.register('github', mockPreparer); + + const processor = new JobProcessor({ + preparers, + dockerClient: mockDocker, + templater, + }); + + return { job: processor.create(mockEntity, mockValues), processor }; + }; + + // TODO(blam): make this better. + // Wait 10ms for processor to finish. + const waitForProcessor = () => + new Promise(resolve => setTimeout(resolve, 10)); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('fails when the job is not in a pending state', async () => { + const { job, processor } = createJob(); + job.status = 'TEMPLATING'; + + await expect(processor.process(job)).rejects.toThrow( + /Job is not in a 'PENDING' state/, + ); + }); + + it('calls the preparer with the entity', async () => { + const { job, processor } = createJob(); + + // Create a promise to hold it at this step so we can test + mockPreparer.prepare.mockImplementationOnce(() => new Promise(() => {})); + + processor.process(job); + + await waitForProcessor(); + + expect(mockPreparer.prepare).toHaveBeenCalledWith(mockEntity); + expect(job.status).toBe('PREPARING'); + }); + + it('calls the templater with the correct directory', async () => { + const { job, processor } = createJob(); + const mockDirectory = '/test/blam/bo'; + mockPreparer.prepare.mockResolvedValueOnce(mockDirectory); + + // Create a promise to hold it at this step so we can test + templater.run.mockImplementationOnce(() => new Promise(() => {})); + + processor.process(job); + + await waitForProcessor(); + + expect(templater.run).toHaveBeenCalledWith({ + directory: mockDirectory, + values: mockValues, + dockerClient: mockDocker, + logStream: job.logStream, + }); + }); }); }); diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts index 341ae6724e..c97d5ced79 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts @@ -114,7 +114,7 @@ export class JobProcessor implements Processor { async process(job: Job) { if (job.status !== 'PENDING') { - throw new Error('Job is not in pending state'); + throw new Error("Job is not in a 'PENDING' state"); } try { From ce43dce8ff7b9daf6feeac6c3d2523baf8734ff2 Mon Sep 17 00:00:00 2001 From: blam Date: Fri, 26 Jun 2020 18:12:48 +0200 Subject: [PATCH 22/35] feat(scaffolder): Adjusting the types for the scaffolder --- .../src/scaffolder/jobs/types.ts | 26 +++++++++++++------ 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts index e6c5436e95..060d5d400e 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts @@ -19,21 +19,31 @@ import { JsonValue } from '@backstage/config'; import { RequiredTemplateValues } from '../templater'; import { Logger } from 'winston'; +// Context will be a mutable object which is passed between stages +// To share data, but also thinking that we can pass in functions here too +// To maybe create sub steps or fail the entire thing, or skip stages down the line. +export type StageContext = T & { + values: RequiredTemplateValues & Record; + entity: TemplateEntityV1alpha1; + logger: Logger; +}; + +export type Stage = { + log: string[]; + status: 'PENDING' | 'STARTED' | 'COMPLETE' | 'FAILED'; + name: string; + handler: (ctx: StageContext) => Promise; +}; + export type Job = { id: string; metadata: { entity: TemplateEntityV1alpha1; values: RequiredTemplateValues & Record; }; - status: - | 'PENDING' - | 'PREPARING' - | 'TEMPLATING' - | 'STORING' - | 'COMPLETE' - | 'FAILED'; + status: 'PENDING' | 'STARTED' | 'COMPLETE' | 'FAILED'; + stages: Stage[]; logStream: Writable; - log: string[]; logger: Logger; error?: Error; }; From ad2354909eb1af6fc3b9274453621c3ce3b11f52 Mon Sep 17 00:00:00 2001 From: blam Date: Fri, 26 Jun 2020 21:27:48 +0200 Subject: [PATCH 23/35] chore(scaffolder): work on a better api for holding stages and metadata --- .../src/scaffolder/jobs/processor.test.ts | 1 + .../src/scaffolder/jobs/processor.ts | 27 ++++++++------- .../src/scaffolder/jobs/types.ts | 34 ++++++++++++------- .../{ => stages}/prepare/file.test.ts | 0 .../scaffolder/{ => stages}/prepare/file.ts | 0 .../{ => stages}/prepare/github.test.ts | 0 .../scaffolder/{ => stages}/prepare/github.ts | 0 .../{ => stages}/prepare/helpers.test.ts | 0 .../{ => stages}/prepare/helpers.ts | 0 .../scaffolder/{ => stages}/prepare/index.ts | 0 .../{ => stages}/prepare/preparers.test.ts | 0 .../{ => stages}/prepare/preparers.ts | 0 .../scaffolder/{ => stages}/prepare/types.ts | 0 .../templater/cookiecutter.test.ts | 0 .../{ => stages}/templater/cookiecutter.ts | 0 .../{ => stages}/templater/helpers.test.ts | 0 .../{ => stages}/templater/helpers.ts | 0 .../{ => stages}/templater/index.ts | 0 18 files changed, 36 insertions(+), 26 deletions(-) rename plugins/scaffolder-backend/src/scaffolder/{ => stages}/prepare/file.test.ts (100%) rename plugins/scaffolder-backend/src/scaffolder/{ => stages}/prepare/file.ts (100%) rename plugins/scaffolder-backend/src/scaffolder/{ => stages}/prepare/github.test.ts (100%) rename plugins/scaffolder-backend/src/scaffolder/{ => stages}/prepare/github.ts (100%) rename plugins/scaffolder-backend/src/scaffolder/{ => stages}/prepare/helpers.test.ts (100%) rename plugins/scaffolder-backend/src/scaffolder/{ => stages}/prepare/helpers.ts (100%) rename plugins/scaffolder-backend/src/scaffolder/{ => stages}/prepare/index.ts (100%) rename plugins/scaffolder-backend/src/scaffolder/{ => stages}/prepare/preparers.test.ts (100%) rename plugins/scaffolder-backend/src/scaffolder/{ => stages}/prepare/preparers.ts (100%) rename plugins/scaffolder-backend/src/scaffolder/{ => stages}/prepare/types.ts (100%) rename plugins/scaffolder-backend/src/scaffolder/{ => stages}/templater/cookiecutter.test.ts (100%) rename plugins/scaffolder-backend/src/scaffolder/{ => stages}/templater/cookiecutter.ts (100%) rename plugins/scaffolder-backend/src/scaffolder/{ => stages}/templater/helpers.test.ts (100%) rename plugins/scaffolder-backend/src/scaffolder/{ => stages}/templater/helpers.ts (100%) rename plugins/scaffolder-backend/src/scaffolder/{ => stages}/templater/index.ts (100%) diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts index b8f1c8f86e..367d140f7e 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts @@ -79,6 +79,7 @@ describe('JobProcessor', () => { const createJob = (): { job: Job; processor: JobProcessor } => { preparers.register('github', mockPreparer); + new JobProcessor(1); const processor = new JobProcessor({ preparers, dockerClient: mockDocker, diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts index c97d5ced79..a1b8795156 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts @@ -13,16 +13,16 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { Processor, Job } from './types'; +import { Processor, Job, Stage, StageContext } from './types'; import { JsonValue } from '@backstage/config'; import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; import { PassThrough } from 'stream'; import * as uuid from 'uuid'; import Docker from 'dockerode'; import winston from 'winston'; -import { RequiredTemplateValues, TemplaterBase } from '../templater'; +import { RequiredTemplateValues, TemplaterBase } from '../stages/templater'; import { createNewRootLogger } from '@backstage/backend-common'; -import { PreparerBuilder } from '../prepare'; +import { PreparerBuilder } from '../stages/prepare'; export type JobProcessorArguments = { preparers: PreparerBuilder; @@ -36,15 +36,10 @@ export type JobAndDirectoryTuple = { }; export class JobProcessor implements Processor { - private preparers: PreparerBuilder; - private templater: TemplaterBase; - private dockerClient: Docker; private jobs = new Map(); - - constructor({ preparers, templater, dockerClient }: JobProcessorArguments) { - this.preparers = preparers; - this.templater = templater; - this.dockerClient = dockerClient; + private stages: Stage[]; + constructor({ stages }: { stages: Stage[] }) { + this.stages = stages; } create( @@ -66,11 +61,17 @@ export class JobProcessor implements Processor { const logger = createNewRootLogger(); logger.add(new winston.transports.Stream({ stream: logStream })); + const context: StageContext = { + entity, + values, + logger, + }; + const job: Job = { id, logStream, - logger, - log, + context, + stages: this.stages.map((stage) => ({})) status: 'PENDING', metadata: { entity, diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts index 060d5d400e..b01fae9625 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts @@ -16,13 +16,13 @@ import type { Writable } from 'stream'; import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; import { JsonValue } from '@backstage/config'; -import { RequiredTemplateValues } from '../templater'; +import { RequiredTemplateValues } from '../stages/templater'; import { Logger } from 'winston'; // Context will be a mutable object which is passed between stages // To share data, but also thinking that we can pass in functions here too // To maybe create sub steps or fail the entire thing, or skip stages down the line. -export type StageContext = T & { +export type StageContext = T & { values: RequiredTemplateValues & Record; entity: TemplateEntityV1alpha1; logger: Logger; @@ -35,12 +35,9 @@ export type Stage = { handler: (ctx: StageContext) => Promise; }; -export type Job = { +export type Job = { id: string; - metadata: { - entity: TemplateEntityV1alpha1; - values: RequiredTemplateValues & Record; - }; + context: StageContext; status: 'PENDING' | 'STARTED' | 'COMPLETE' | 'FAILED'; stages: Stage[]; logStream: Writable; @@ -48,11 +45,22 @@ export type Job = { error?: Error; }; -export type Processor = { - create( - entity: TemplateEntityV1alpha1, - values: RequiredTemplateValues & Record, - ): Job; +export interface ProcessorConstructor { + new (t: string): Processor; +} + +export interface Processor { + create({ + entity, + values, + stages, + }: { + entity: TemplateEntityV1alpha1; + values: RequiredTemplateValues & Record; + stages: Stage[]; + }): Job; get(id: string): Job | undefined; -}; +} + +declare let Processor: ProcessorConstructor; diff --git a/plugins/scaffolder-backend/src/scaffolder/prepare/file.test.ts b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/file.test.ts similarity index 100% rename from plugins/scaffolder-backend/src/scaffolder/prepare/file.test.ts rename to plugins/scaffolder-backend/src/scaffolder/stages/prepare/file.test.ts diff --git a/plugins/scaffolder-backend/src/scaffolder/prepare/file.ts b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/file.ts similarity index 100% rename from plugins/scaffolder-backend/src/scaffolder/prepare/file.ts rename to plugins/scaffolder-backend/src/scaffolder/stages/prepare/file.ts diff --git a/plugins/scaffolder-backend/src/scaffolder/prepare/github.test.ts b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/github.test.ts similarity index 100% rename from plugins/scaffolder-backend/src/scaffolder/prepare/github.test.ts rename to plugins/scaffolder-backend/src/scaffolder/stages/prepare/github.test.ts diff --git a/plugins/scaffolder-backend/src/scaffolder/prepare/github.ts b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/github.ts similarity index 100% rename from plugins/scaffolder-backend/src/scaffolder/prepare/github.ts rename to plugins/scaffolder-backend/src/scaffolder/stages/prepare/github.ts diff --git a/plugins/scaffolder-backend/src/scaffolder/prepare/helpers.test.ts b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/helpers.test.ts similarity index 100% rename from plugins/scaffolder-backend/src/scaffolder/prepare/helpers.test.ts rename to plugins/scaffolder-backend/src/scaffolder/stages/prepare/helpers.test.ts diff --git a/plugins/scaffolder-backend/src/scaffolder/prepare/helpers.ts b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/helpers.ts similarity index 100% rename from plugins/scaffolder-backend/src/scaffolder/prepare/helpers.ts rename to plugins/scaffolder-backend/src/scaffolder/stages/prepare/helpers.ts diff --git a/plugins/scaffolder-backend/src/scaffolder/prepare/index.ts b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/index.ts similarity index 100% rename from plugins/scaffolder-backend/src/scaffolder/prepare/index.ts rename to plugins/scaffolder-backend/src/scaffolder/stages/prepare/index.ts diff --git a/plugins/scaffolder-backend/src/scaffolder/prepare/preparers.test.ts b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/preparers.test.ts similarity index 100% rename from plugins/scaffolder-backend/src/scaffolder/prepare/preparers.test.ts rename to plugins/scaffolder-backend/src/scaffolder/stages/prepare/preparers.test.ts diff --git a/plugins/scaffolder-backend/src/scaffolder/prepare/preparers.ts b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/preparers.ts similarity index 100% rename from plugins/scaffolder-backend/src/scaffolder/prepare/preparers.ts rename to plugins/scaffolder-backend/src/scaffolder/stages/prepare/preparers.ts diff --git a/plugins/scaffolder-backend/src/scaffolder/prepare/types.ts b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/types.ts similarity index 100% rename from plugins/scaffolder-backend/src/scaffolder/prepare/types.ts rename to plugins/scaffolder-backend/src/scaffolder/stages/prepare/types.ts diff --git a/plugins/scaffolder-backend/src/scaffolder/templater/cookiecutter.test.ts b/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.test.ts similarity index 100% rename from plugins/scaffolder-backend/src/scaffolder/templater/cookiecutter.test.ts rename to plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.test.ts diff --git a/plugins/scaffolder-backend/src/scaffolder/templater/cookiecutter.ts b/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.ts similarity index 100% rename from plugins/scaffolder-backend/src/scaffolder/templater/cookiecutter.ts rename to plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.ts diff --git a/plugins/scaffolder-backend/src/scaffolder/templater/helpers.test.ts b/plugins/scaffolder-backend/src/scaffolder/stages/templater/helpers.test.ts similarity index 100% rename from plugins/scaffolder-backend/src/scaffolder/templater/helpers.test.ts rename to plugins/scaffolder-backend/src/scaffolder/stages/templater/helpers.test.ts diff --git a/plugins/scaffolder-backend/src/scaffolder/templater/helpers.ts b/plugins/scaffolder-backend/src/scaffolder/stages/templater/helpers.ts similarity index 100% rename from plugins/scaffolder-backend/src/scaffolder/templater/helpers.ts rename to plugins/scaffolder-backend/src/scaffolder/stages/templater/helpers.ts diff --git a/plugins/scaffolder-backend/src/scaffolder/templater/index.ts b/plugins/scaffolder-backend/src/scaffolder/stages/templater/index.ts similarity index 100% rename from plugins/scaffolder-backend/src/scaffolder/templater/index.ts rename to plugins/scaffolder-backend/src/scaffolder/stages/templater/index.ts From 17c6030ca0400bd27bddb73811c9483a2b3b285a Mon Sep 17 00:00:00 2001 From: blam Date: Sat, 27 Jun 2020 03:41:06 +0200 Subject: [PATCH 24/35] feat(scaffolder): finally managed to get typescript to play nice with the new stage approach --- .../backend-common/src/logging/rootLogger.ts | 41 +++++++--------- .../src/scaffolder/jobs/logger.ts | 44 +++++++++++++++++ .../src/scaffolder/jobs/processor.ts | 45 ++++++------------ .../src/scaffolder/jobs/types.ts | 45 +++++++++--------- .../scaffolder-backend/src/service/router.ts | 47 ++++++++++++++++--- 5 files changed, 140 insertions(+), 82 deletions(-) create mode 100644 plugins/scaffolder-backend/src/scaffolder/jobs/logger.ts diff --git a/packages/backend-common/src/logging/rootLogger.ts b/packages/backend-common/src/logging/rootLogger.ts index 47379b027c..fd2f3a1c8b 100644 --- a/packages/backend-common/src/logging/rootLogger.ts +++ b/packages/backend-common/src/logging/rootLogger.ts @@ -13,31 +13,26 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - import * as winston from 'winston'; -export function createNewRootLogger(): winston.Logger { - return winston.createLogger({ - level: process.env.LOG_LEVEL || 'info', - format: - process.env.NODE_ENV === 'production' - ? winston.format.json() - : winston.format.combine( - winston.format.colorize(), - winston.format.timestamp(), - winston.format.simple(), - ), - defaultMeta: { service: 'backstage' }, - transports: [ - new winston.transports.Console({ - silent: - process.env.JEST_WORKER_ID !== undefined && !process.env.LOG_LEVEL, - }), - ], - }); -} - -let rootLogger: winston.Logger = createNewRootLogger(); +let rootLogger: winston.Logger = winston.createLogger({ + level: process.env.LOG_LEVEL || 'info', + format: + process.env.NODE_ENV === 'production' + ? winston.format.json() + : winston.format.combine( + winston.format.colorize(), + winston.format.timestamp(), + winston.format.simple(), + ), + defaultMeta: { service: 'backstage' }, + transports: [ + new winston.transports.Console({ + silent: + process.env.JEST_WORKER_ID !== undefined && !process.env.LOG_LEVEL, + }), + ], +}); export function getRootLogger(): winston.Logger { return rootLogger; diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/logger.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/logger.ts new file mode 100644 index 0000000000..61d95b6b08 --- /dev/null +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/logger.ts @@ -0,0 +1,44 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { PassThrough } from 'stream'; +import winston from 'winston'; +import { JsonValue } from '@backstage/config'; + +export const useLogStream = (meta: Record) => { + const log: string[] = []; + + // Create an empty stream to collect all the log lines into + // one variable for the API. + const stream = new PassThrough(); + stream.on('data', chunk => log.push(chunk.toString())); + + const logger = winston.createLogger({ + level: process.env.LOG_LEVEL || 'info', + format: winston.format.combine( + winston.format.colorize(), + winston.format.timestamp(), + ), + defaultMeta: meta, + }); + + logger.add(new winston.transports.Stream({ stream })); + + return { + log, + stream, + logger, + }; +}; diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts index a1b8795156..69d02fa7d1 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts @@ -13,16 +13,14 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { Processor, Job, Stage, StageContext } from './types'; +import { Processor, Job, Stage, StageContext, StageInput } from './types'; import { JsonValue } from '@backstage/config'; import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; -import { PassThrough } from 'stream'; import * as uuid from 'uuid'; import Docker from 'dockerode'; -import winston from 'winston'; import { RequiredTemplateValues, TemplaterBase } from '../stages/templater'; -import { createNewRootLogger } from '@backstage/backend-common'; import { PreparerBuilder } from '../stages/prepare'; +import { useLogStream } from './logger'; export type JobProcessorArguments = { preparers: PreparerBuilder; @@ -37,29 +35,18 @@ export type JobAndDirectoryTuple = { export class JobProcessor implements Processor { private jobs = new Map(); - private stages: Stage[]; - constructor({ stages }: { stages: Stage[] }) { - this.stages = stages; - } - create( - entity: TemplateEntityV1alpha1, - values: RequiredTemplateValues & Record, - ): Job { + create({ + entity, + values, + stages, + }: { + entity: TemplateEntityV1alpha1; + values: RequiredTemplateValues & Record; + stages: StageInput[]; + }): Job { const id = uuid.v4(); - const log: string[] = []; - - // Create an empty stream to collect all the log lines into - // one variable for the API. - const logStream = new PassThrough(); - logStream.on('data', chunk => log.push(chunk.toString())); - - // TODO(blam): Maybe this is not the right way to build the logger - // Maybe we want to be more ux specific and drop the json support. - // Child loggers can not have specific transports which sucks, so we have to - // create another here. - const logger = createNewRootLogger(); - logger.add(new winston.transports.Stream({ stream: logStream })); + const { logger, stream } = useLogStream({ id }); const context: StageContext = { entity, @@ -69,14 +56,10 @@ export class JobProcessor implements Processor { const job: Job = { id, - logStream, + logStream: stream, context, - stages: this.stages.map((stage) => ({})) + stages, status: 'PENDING', - metadata: { - entity, - values, - }, }; this.jobs.set(job.id, job); diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts index b01fae9625..6c66a65355 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts @@ -22,45 +22,46 @@ import { Logger } from 'winston'; // Context will be a mutable object which is passed between stages // To share data, but also thinking that we can pass in functions here too // To maybe create sub steps or fail the entire thing, or skip stages down the line. -export type StageContext = T & { +export type StageContext = { values: RequiredTemplateValues & Record; entity: TemplateEntityV1alpha1; logger: Logger; -}; + logStream: Writable; +} & T; -export type Stage = { +export type ProcessorStatus = 'PENDING' | 'STARTED' | 'COMPLETED' | 'FAILED'; + +export interface Stage extends StageInput { log: string[]; - status: 'PENDING' | 'STARTED' | 'COMPLETE' | 'FAILED'; - name: string; - handler: (ctx: StageContext) => Promise; -}; + status: ProcessorStatus; + startedAt?: number; + endedAt?: number; +} -export type Job = { +export interface StageInput { + name: string; + handler(ctx: StageContext): Promise; +} + +export type Job = { id: string; - context: StageContext; - status: 'PENDING' | 'STARTED' | 'COMPLETE' | 'FAILED'; + context: StageContext; + status: ProcessorStatus; stages: Stage[]; logStream: Writable; - logger: Logger; error?: Error; }; -export interface ProcessorConstructor { - new (t: string): Processor; -} - -export interface Processor { - create({ +export type Processor = { + create({ entity, values, stages, }: { entity: TemplateEntityV1alpha1; values: RequiredTemplateValues & Record; - stages: Stage[]; - }): Job; + stages: StageInput[]; + }): Job; get(id: string): Job | undefined; -} - -declare let Processor: ProcessorConstructor; +}; diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 5957b66915..b340190b41 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -21,6 +21,7 @@ import { PreparerBuilder, TemplaterBase, JobProcessor } from '../scaffolder'; import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; import Docker from 'dockerode'; import {} from '@backstage/backend-common'; +import { StageContext, Stage } from '../scaffolder/jobs/types'; export interface RouterOptions { preparers: PreparerBuilder; templater: TemplaterBase; @@ -35,11 +36,7 @@ export async function createRouter( const { preparers, templater, logger: parentLogger, dockerClient } = options; const logger = parentLogger.child({ plugin: 'scaffolder' }); - const jobProcessor = new JobProcessor({ - preparers, - templater, - dockerClient, - }); + const jobProcessor = new JobProcessor(); router .get('/v1/job/:jobId', ({ params }, res) => { @@ -86,7 +83,45 @@ export async function createRouter( }, }; - const job = jobProcessor.create(mockEntity, { component_id: 'test' }); + const job = jobProcessor.create({ + entity: mockEntity, + values: { component_id: 'blob' }, + stages: [ + { + name: 'Prepare the skeleton', + handler: async ctx => { + const preparer = preparers.get(ctx.entity); + const skeletonDir = await preparer.prepare(ctx.entity); + return { skeletonDir }; + }, + }, + { + name: 'Run the templater', + handler: async (ctx: StageContext<{ skeletonDir: string }>) => { + const resultDir = await templater.run({ + directory: ctx.skeletonDir, + dockerClient, + logStream: ctx.logStream, + values: ctx.values, + }); + + return { resultDir }; + }, + }, + { + name: 'Create VCS Repo', + handler: async (ctx: StageContext<{ resultDir: string }>) => { + ctx.logger.info('Should now create the VCS repo'); + }, + }, + { + name: 'Push to remote', + handler: async ctx => { + ctx.logger.info('Should now push to the remote'); + }, + }, + ], + }); res.status(201).json({ jobId: job.id }); jobProcessor.process(job); From c600d64db8e2eac524c371b66b123d2a953d7e13 Mon Sep 17 00:00:00 2001 From: blam Date: Sat, 27 Jun 2020 05:21:42 +0200 Subject: [PATCH 25/35] feat(scaffolder): reworking how the loading of stages goes and made it super flexible --- .../src/scaffolder/index.ts | 6 +- .../src/scaffolder/jobs/logger.ts | 1 + .../src/scaffolder/jobs/processor.ts | 76 +++++++++++-------- .../src/scaffolder/jobs/types.ts | 2 + .../src/scaffolder/stages/prepare/types.ts | 6 +- .../stages/templater/cookiecutter.ts | 11 ++- .../scaffolder-backend/src/service/router.ts | 20 +++-- 7 files changed, 78 insertions(+), 44 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/index.ts b/plugins/scaffolder-backend/src/scaffolder/index.ts index f2f7d3dad3..6010d31e98 100644 --- a/plugins/scaffolder-backend/src/scaffolder/index.ts +++ b/plugins/scaffolder-backend/src/scaffolder/index.ts @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -export * from './templater'; -export * from './prepare'; -export * from './templater/cookiecutter'; +export * from './stages/templater'; +export * from './stages/prepare'; +export * from './stages/templater/cookiecutter'; export * from './jobs'; diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/logger.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/logger.ts index 61d95b6b08..d2eaa13c3b 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/logger.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/logger.ts @@ -30,6 +30,7 @@ export const useLogStream = (meta: Record) => { format: winston.format.combine( winston.format.colorize(), winston.format.timestamp(), + winston.format.simple(), ), defaultMeta: meta, }); diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts index 69d02fa7d1..795e07291d 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts @@ -52,13 +52,19 @@ export class JobProcessor implements Processor { entity, values, logger, + logStream: stream, }; const job: Job = { id, logStream: stream, context, - stages, + stages: stages.map(stage => ({ + handler: stage.handler, + log: [], + name: stage.name, + status: 'PENDING', + })), status: 'PENDING', }; @@ -71,45 +77,49 @@ export class JobProcessor implements Processor { return this.jobs.get(id); } - private async prepare(job: Job): Promise { - job.status = 'PREPARING'; - const entity = job.metadata.entity; - const preparer = this.preparers.get(entity); - return await preparer.prepare(entity); - } - - private async run(job: Job, directory: string): Promise { - job.status = 'TEMPLATING'; - return await this.templater.run({ - directory, - values: job.metadata.values, - dockerClient: this.dockerClient, - logStream: job.logStream, - }); - } - - private async store(job: Job): Promise { - job.status = 'STORING'; - } - - private async complete(job: Job): Promise { - job.status = 'COMPLETE'; - } - - async process(job: Job) { + async run(job: Job): Promise { if (job.status !== 'PENDING') { throw new Error("Job is not in a 'PENDING' state"); } + job.status = 'STARTED'; + try { - const skeletonPath = await this.prepare(job); - await this.run(job, skeletonPath); - await this.store(job); - await this.complete(job); + for (const entry of job.stages) { + const { logger, log, stream } = useLogStream({ + id: job.id, + stage: entry.name, + }); + try { + entry.startedAt = Date.now(); + + entry.log = log; + + const handler = await entry.handler({ + ...job.context, + logger, + logStream: stream, + }); + + job.context = { + ...job.context, + ...handler, + }; + + entry.status = 'COMPLETED'; + } catch (error) { + logger.error(error); + entry.status = 'FAILED'; + throw error; + } finally { + entry.endedAt = Date.now(); + } + } + job.status = 'COMPLETED'; } catch (error) { - job.error = error; + job.error = { name: error.name, message: error.message }; job.status = 'FAILED'; - job.logger.error(`Job failed with error ${error.message}`); + job.context.logger.error(`Job failed with error ${error.message}`); } } } diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts index 6c66a65355..92e4e1cbf4 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts @@ -64,4 +64,6 @@ export type Processor = { }): Job; get(id: string): Job | undefined; + + run(job: Job): Promise; }; diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/types.ts b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/types.ts index a6e42c465f..adbc38ef58 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/types.ts @@ -14,6 +14,7 @@ * limitations under the License. */ import type { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; +import { Logger } from 'winston'; export type PreparerBase = { /** @@ -21,7 +22,10 @@ export type PreparerBase = { * with contents from the remote location in temporary storage and return the path * @param template The template entity from the Service Catalog */ - prepare(template: TemplateEntityV1alpha1): Promise; + prepare( + template: TemplateEntityV1alpha1, + opts: { logger: Logger }, + ): Promise; }; export type PreparerBuilder = { diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.ts b/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.ts index cb11bf04af..cef8a64b60 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.ts @@ -51,8 +51,15 @@ export class CookieCutter implements TemplaterBase { const resultDir = await fs.promises.mkdtemp(`${options.directory}-result`); await runDockerContainer({ - imageName: 'backstage/cookiecutter', - args: ['cookiecutter', '--no-input', '-o', '/result', '/template'], + imageName: 'spotify/backstage-cookiecutter', + args: [ + 'cookiecutter', + '--no-input', + '-o', + '/result', + '/template', + '--verbose', + ], templateDir, resultDir, logStream: options.logStream, diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index b340190b41..0b4ec2a5b8 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -49,9 +49,16 @@ export async function createRouter( res.send({ id: job.id, - metadata: job.metadata, + metadata: { + ...job.context, + logger: undefined, + logStream: undefined, + }, status: job.status, - log: job.log, + stages: job.stages.map(stage => ({ + ...stage, + handler: undefined, + })), error: job.error, }); }) @@ -66,7 +73,7 @@ export async function createRouter( metadata: { annotations: { 'backstage.io/managed-by-location': - 'github:https://github.com/benjdlambert/backstage-graphql-template/blob/master/template.yaml', + 'github:https://github.com/benjdlambert/backstage-graphqsl-template/blob/master/template.yaml', }, name: 'graphql-starter', title: 'GraphQL Service', @@ -91,7 +98,9 @@ export async function createRouter( name: 'Prepare the skeleton', handler: async ctx => { const preparer = preparers.get(ctx.entity); - const skeletonDir = await preparer.prepare(ctx.entity); + const skeletonDir = await preparer.prepare(ctx.entity, { + logger: ctx.logger, + }); return { skeletonDir }; }, }, @@ -122,9 +131,10 @@ export async function createRouter( }, ], }); + res.status(201).json({ jobId: job.id }); - jobProcessor.process(job); + jobProcessor.run(job); }); const app = express(); From 9e6ed5fb5239f48cc9b8dc90adbdd9fdf739071e Mon Sep 17 00:00:00 2001 From: blam Date: Sat, 27 Jun 2020 22:10:55 +0200 Subject: [PATCH 26/35] feat(scaffolder): added some more tests for the new job processor --- .../src/scaffolder/jobs/processor.test.ts | 201 +++++++++++------- .../src/scaffolder/jobs/processor.ts | 2 +- 2 files changed, 131 insertions(+), 72 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts index 367d140f7e..6b80298926 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts @@ -15,10 +15,7 @@ */ import { JobProcessor } from './processor'; import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; -import Docker from 'dockerode'; -import { CookieCutter } from '../templater/cookiecutter'; -import { Preparers } from '../'; -import { Job } from './types'; +import { StageInput } from './types'; describe('JobProcessor', () => { const mockEntity: TemplateEntityV1alpha1 = { @@ -47,98 +44,160 @@ describe('JobProcessor', () => { const mockValues = { component_id: 'bob' }; describe('create', () => { - const templater = new CookieCutter(); - const preparers = new Preparers(); - const mockDocker = {} as jest.Mocked; - it('creates a new job', async () => { - const processor = new JobProcessor({ - dockerClient: mockDocker, - preparers, - templater, - }); + it('creates should create a new job with a unique id', async () => { + const processor = new JobProcessor(); - const job = processor.create(mockEntity, mockValues); + const job = processor.create({ + entity: mockEntity, + values: mockValues, + stages: [], + }); expect(job.id).toMatch( /^[0-9A-F]{8}-[0-9A-F]{4}-4[0-9A-F]{3}-[89AB][0-9A-F]{3}-[0-9A-F]{12}$/i, ); + }); + + it('should setup the correct context for the job', async () => { + const processor = new JobProcessor(); + + const job = processor.create({ + entity: mockEntity, + values: mockValues, + stages: [], + }); + + console.warn(job.context); + expect(job.context.entity).toBe(mockEntity); + expect(job.context.values).toBe(mockValues); + }); + + it('should set the status as pending', async () => { + const processor = new JobProcessor(); + + const job = processor.create({ + entity: mockEntity, + values: mockValues, + stages: [], + }); - expect(job.log).toEqual([]); expect(job.status).toBe('PENDING'); - expect(job.metadata.entity).toBe(mockEntity); - expect(job.metadata.values).toBe(mockValues); + }); + + it('should create the correct stages', async () => { + const stages: StageInput[] = [ + { + name: 'Do something cool step 1', + handler: jest.fn(), + }, + { + name: 'Do something cool step 2', + handler: jest.fn(), + }, + ]; + + const processor = new JobProcessor(); + + const job = processor.create({ + entity: mockEntity, + values: mockValues, + stages, + }); + + expect(job.stages).toHaveLength(stages.length); + + for (let i = 0; i < job.stages.length; i++) { + expect(job.stages[i].name).toBe(stages[i].name); + expect(job.stages[i].status).toBe('PENDING'); + } }); }); - describe('process', () => { - const preparers = new Preparers(); - const mockDocker = {} as jest.Mocked; - const mockPreparer = { prepare: jest.fn() }; - const templater = { run: jest.fn() }; - - const createJob = (): { job: Job; processor: JobProcessor } => { - preparers.register('github', mockPreparer); - - new JobProcessor(1); - const processor = new JobProcessor({ - preparers, - dockerClient: mockDocker, - templater, - }); - - return { job: processor.create(mockEntity, mockValues), processor }; - }; - - // TODO(blam): make this better. - // Wait 10ms for processor to finish. - const waitForProcessor = () => - new Promise(resolve => setTimeout(resolve, 10)); - - beforeEach(() => { - jest.clearAllMocks(); + describe('get', () => { + it('return undefined for when the job does not exist', () => { + const processor = new JobProcessor(); + expect(processor.get('123')).not.toBeDefined(); }); - it('fails when the job is not in a pending state', async () => { - const { job, processor } = createJob(); - job.status = 'TEMPLATING'; + it('should return the exact same instance of the job when one is created', async () => { + const processor = new JobProcessor(); + const job = processor.create({ + entity: mockEntity, + values: mockValues, + stages: [], + }); - await expect(processor.process(job)).rejects.toThrow( + expect(processor.get(job.id)).toBe(job); + }); + }); + describe('process', () => { + it('throws an error when the status of the job is not in pending state', async () => { + const processor = new JobProcessor(); + const job = processor.create({ + entity: mockEntity, + values: mockValues, + stages: [], + }); + + job.status = 'STARTED'; + + await expect(processor.run(job)).rejects.toThrow( /Job is not in a 'PENDING' state/, ); }); - it('calls the preparer with the entity', async () => { - const { job, processor } = createJob(); + it('will call each of the handlers in the stages', async () => { + const stages: StageInput[] = [ + { + name: 'c/o', + handler: jest.fn(), + }, + { + name: 'g/p', + handler: jest.fn(), + }, + ]; - // Create a promise to hold it at this step so we can test - mockPreparer.prepare.mockImplementationOnce(() => new Promise(() => {})); + const processor = new JobProcessor(); + const job = processor.create({ + entity: mockEntity, + values: mockValues, + stages, + }); - processor.process(job); + await processor.run(job); - await waitForProcessor(); - - expect(mockPreparer.prepare).toHaveBeenCalledWith(mockEntity); - expect(job.status).toBe('PREPARING'); + for (const stage of stages) { + expect(stage.handler).toHaveBeenCalled(); + } }); - it('calls the templater with the correct directory', async () => { - const { job, processor } = createJob(); - const mockDirectory = '/test/blam/bo'; - mockPreparer.prepare.mockResolvedValueOnce(mockDirectory); + it('should set all stages to complete and the job to complete when finishes without errors', async () => { + const stages: StageInput[] = [ + { + name: 'c/o', + handler: jest.fn(), + }, + { + name: 'g/p', + handler: jest.fn(), + }, + ]; - // Create a promise to hold it at this step so we can test - templater.run.mockImplementationOnce(() => new Promise(() => {})); - - processor.process(job); - - await waitForProcessor(); - - expect(templater.run).toHaveBeenCalledWith({ - directory: mockDirectory, + const processor = new JobProcessor(); + const job = processor.create({ + entity: mockEntity, values: mockValues, - dockerClient: mockDocker, - logStream: job.logStream, + stages, }); + + await processor.run(job); + + for (const stage of job.stages) { + expect(stage.status).toBe('COMPLETED'); + } + + expect(job.status).toBe('COMPLETED'); }); }); }); diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts index 795e07291d..89f2e89d97 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { Processor, Job, Stage, StageContext, StageInput } from './types'; +import { Processor, Job, StageContext, StageInput } from './types'; import { JsonValue } from '@backstage/config'; import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; import * as uuid from 'uuid'; From 837a182f68856d1bafbcc9b8509a4c7313b38784 Mon Sep 17 00:00:00 2001 From: blam Date: Sat, 27 Jun 2020 22:35:48 +0200 Subject: [PATCH 27/35] chore(scaffolder): finished off tests for the processor. It now works pretty well --- .../src/scaffolder/jobs/processor.test.ts | 77 ++++++++++++++++++- 1 file changed, 76 insertions(+), 1 deletion(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts index 6b80298926..501e72c078 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.test.ts @@ -67,7 +67,6 @@ describe('JobProcessor', () => { stages: [], }); - console.warn(job.context); expect(job.context.entity).toBe(mockEntity); expect(job.context.values).toBe(mockValues); }); @@ -199,5 +198,81 @@ describe('JobProcessor', () => { expect(job.status).toBe('COMPLETED'); }); + + it('should merge the return value from previous steps into the context of the next step', async () => { + const stages: StageInput[] = [ + { + name: 'c/o', + handler: jest + .fn() + .mockResolvedValue({ first: 'ben', second: 'lambert' }), + }, + { + name: 'g/p', + handler: jest + .fn() + .mockResolvedValue({ second: 'linus', third: 'lambert' }), + }, + { + name: 'go', + handler: jest.fn(), + }, + ]; + + const processor = new JobProcessor(); + const job = processor.create({ + entity: mockEntity, + values: mockValues, + stages, + }); + + await processor.run(job); + + expect(stages[1].handler).toHaveBeenCalledWith( + expect.objectContaining({ first: 'ben', second: 'lambert' }), + ); + + expect(stages[2].handler).toHaveBeenCalledWith( + expect.objectContaining({ + first: 'ben', + second: 'linus', + third: 'lambert', + }), + ); + }); + + it('should fail the job and the step if one of them fails', async () => { + const fail = new Error('something went wrong here'); + const stages: StageInput[] = [ + { + name: 'c/o', + handler: jest.fn(), + }, + { + name: 'g/p', + handler: jest.fn().mockRejectedValue(fail), + }, + { + name: 'go', + handler: jest.fn(), + }, + ]; + + const processor = new JobProcessor(); + const job = processor.create({ + entity: mockEntity, + values: mockValues, + stages, + }); + + await processor.run(job); + + expect(job.status).toBe('FAILED'); + expect(job.stages[0].status).toBe('COMPLETED'); + expect(job.stages[1].status).toBe('FAILED'); + expect(job.stages[2].status).toBe('PENDING'); + expect(job.error?.message).toBe('something went wrong here'); + expect(job.stages[1].log.join()).toContain('something went wrong here'); + }); }); }); From c96075ce8c94158f9616e8b05e5ac3a68193987a Mon Sep 17 00:00:00 2001 From: blam Date: Sat, 27 Jun 2020 23:18:15 +0200 Subject: [PATCH 28/35] feat(scaffolder): added an endpoint for getting each stage logs as plaintext in case that's what is needed. --- .../src/scaffolder/index.ts | 2 +- .../src/scaffolder/jobs/index.ts | 1 + .../src/scaffolder/jobs/processor.ts | 48 ++++++++++++------- .../src/scaffolder/jobs/types.ts | 1 - .../scaffolder/stages/prepare/github.test.ts | 4 +- .../src/scaffolder/stages/prepare/github.ts | 8 +--- .../scaffolder-backend/src/service/router.ts | 16 +++++-- 7 files changed, 49 insertions(+), 31 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/index.ts b/plugins/scaffolder-backend/src/scaffolder/index.ts index 6010d31e98..a1ee172184 100644 --- a/plugins/scaffolder-backend/src/scaffolder/index.ts +++ b/plugins/scaffolder-backend/src/scaffolder/index.ts @@ -14,6 +14,6 @@ * limitations under the License. */ export * from './stages/templater'; -export * from './stages/prepare'; export * from './stages/templater/cookiecutter'; +export * from './stages/prepare'; export * from './jobs'; diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/index.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/index.ts index 303987c5b1..683d9c2750 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/index.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/index.ts @@ -14,3 +14,4 @@ * limitations under the License. */ export * from './processor'; +export * from './types'; diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts index 89f2e89d97..1c76ae252d 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/processor.ts @@ -57,7 +57,6 @@ export class JobProcessor implements Processor { const job: Job = { id, - logStream: stream, context, stages: stages.map(stage => ({ handler: stage.handler, @@ -85,41 +84,56 @@ export class JobProcessor implements Processor { job.status = 'STARTED'; try { - for (const entry of job.stages) { + for (const stage of job.stages) { + // Create a logger for each stage so we can create seperate + // Streams for each step. const { logger, log, stream } = useLogStream({ id: job.id, - stage: entry.name, + stage: stage.name, }); + // Attach the logger to the stage, and setup some timestamps. + stage.log = log; + stage.startedAt = Date.now(); + try { - entry.startedAt = Date.now(); - - entry.log = log; - - const handler = await entry.handler({ + // Run the handler with the context created for the Job and some + // Additional logging helpers. + const handlerResponse = await stage.handler({ ...job.context, logger, logStream: stream, }); - job.context = { - ...job.context, - ...handler, - }; + // If the handler returns something, then let's merge this onto the ontext + // For the next stage to use as it might be relevant. + if (handlerResponse) { + job.context = { + ...job.context, + ...handlerResponse, + }; + } - entry.status = 'COMPLETED'; + // Complete the current stage + stage.status = 'COMPLETED'; } catch (error) { - logger.error(error); - entry.status = 'FAILED'; + // Log to the current stage the error that occured and fail the stage. + logger.error(`Stage failed with error: ${error.message}`); + stage.status = 'FAILED'; + + // Throw the error so the job can be failed too. throw error; } finally { - entry.endedAt = Date.now(); + // Always set the stage end timestamp. + stage.endedAt = Date.now(); } } + + // If all went to plan, complete the job. job.status = 'COMPLETED'; } catch (error) { + // If something went wrong, fail the job, and set the error property on the job. job.error = { name: error.name, message: error.message }; job.status = 'FAILED'; - job.context.logger.error(`Job failed with error ${error.message}`); } } } diff --git a/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts b/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts index 92e4e1cbf4..fd554e2033 100644 --- a/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/jobs/types.ts @@ -48,7 +48,6 @@ export type Job = { context: StageContext; status: ProcessorStatus; stages: Stage[]; - logStream: Writable; error?: Error; }; diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/github.test.ts b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/github.test.ts index 44d53248d0..86377be6d0 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/github.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/github.test.ts @@ -60,7 +60,7 @@ describe('GitHubPreparer', () => { 1, 'https://github.com/benjdlambert/backstage-graphql-template', expect.any(String), - { checkoutOpts: { paths: ['template'] } }, + {}, ); }); it('calls the clone command with the correct arguments for a repository when no path is provided', async () => { @@ -71,7 +71,7 @@ describe('GitHubPreparer', () => { 1, 'https://github.com/benjdlambert/backstage-graphql-template', expect.any(String), - { checkoutOpts: {} }, + {}, ); }); diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/github.ts b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/github.ts index b0ced7db2b..f53151e55d 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/prepare/github.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/prepare/github.ts @@ -21,7 +21,7 @@ import { parseLocationAnnotation } from './helpers'; import { InputError } from '@backstage/backend-common'; import { PreparerBase } from './types'; import GitUriParser from 'git-url-parse'; -import { Clone, CheckoutOptions } from 'nodegit'; +import { Clone } from 'nodegit'; export class GithubPreparer implements PreparerBase { async prepare(template: TemplateEntityV1alpha1): Promise { @@ -45,13 +45,7 @@ export class GithubPreparer implements PreparerBase { template.spec.path ?? '.', ); - const checkoutOptions = new CheckoutOptions(); - if (template.spec.path) { - checkoutOptions.paths = [templateDirectory]; - } - await Clone.clone(repositoryCheckoutUrl, tempDir, { - checkoutOpts: checkoutOptions, // TODO(blam): Maybe need some auth here? }); diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 0b4ec2a5b8..0f43c3e0de 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -21,7 +21,7 @@ import { PreparerBuilder, TemplaterBase, JobProcessor } from '../scaffolder'; import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; import Docker from 'dockerode'; import {} from '@backstage/backend-common'; -import { StageContext, Stage } from '../scaffolder/jobs/types'; +import { StageContext } from '../scaffolder/jobs/types'; export interface RouterOptions { preparers: PreparerBuilder; templater: TemplaterBase; @@ -39,6 +39,16 @@ export async function createRouter( const jobProcessor = new JobProcessor(); router + .get('/v1/job/:jobId/stage/:index/log', ({ params }, res) => { + const job = jobProcessor.get(params.jobId); + + if (!job) { + res.status(404).send({ error: 'job not found' }); + return; + } + + res.send(job.stages[Number(params.index)].log.join('')); + }) .get('/v1/job/:jobId', ({ params }, res) => { const job = jobProcessor.get(params.jobId); @@ -73,7 +83,7 @@ export async function createRouter( metadata: { annotations: { 'backstage.io/managed-by-location': - 'github:https://github.com/benjdlambert/backstage-graphqsl-template/blob/master/template.yaml', + 'github:https://github.com/benjdlambert/backstage-graphql-template/blob/master/template.yaml', }, name: 'graphql-starter', title: 'GraphQL Service', @@ -132,7 +142,7 @@ export async function createRouter( ], }); - res.status(201).json({ jobId: job.id }); + res.status(201).json({ id: job.id }); jobProcessor.run(job); }); From 066ced06ac1101ce47f60a7f6583b3e950533dba Mon Sep 17 00:00:00 2001 From: blam Date: Sat, 27 Jun 2020 23:53:41 +0200 Subject: [PATCH 29/35] chore(scaffolder): fixing some tests with it creating bad tmp dirs --- .../stages/templater/cookiecutter.test.ts | 42 +++++++++++++------ 1 file changed, 30 insertions(+), 12 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.test.ts b/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.test.ts index 82e84bc1d3..a24f1ed64f 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.test.ts @@ -33,12 +33,15 @@ describe('CookieCutter Templater', () => { beforeEach(async () => { jest.clearAllMocks(); - - await fs.remove(`${os.tmpdir()}/cookiecutter.json`); }); + const mkTemp = async () => { + const tempDir = os.tmpdir(); + return await fs.promises.mkdtemp(tempDir); + }; + it('should write a cookiecutter.json file with the values from the entitiy', async () => { - const tempdir = os.tmpdir(); + const tempdir = await mkTemp(); const values = { component_id: 'test', @@ -53,10 +56,11 @@ describe('CookieCutter Templater', () => { }); it('should merge any value that is in the cookiecutter.json path already', async () => { - const tempdir = os.tmpdir(); + const tempdir = await mkTemp(); const existingJson = { _copy_without_render: ['./github/workflows/*'], }; + await fs.writeJSON(`${tempdir}/cookiecutter.json`, existingJson); const values = { @@ -72,7 +76,7 @@ describe('CookieCutter Templater', () => { }); it('should throw an error if the cookiecutter json is malformed and not missing', async () => { - const tempdir = os.tmpdir(); + const tempdir = await mkTemp(); await fs.writeFile(`${tempdir}/cookiecutter.json`, "{'"); @@ -87,7 +91,7 @@ describe('CookieCutter Templater', () => { }); it('should run the correct docker container with the correct bindings for the volumes', async () => { - const tempdir = os.tmpdir(); + const tempdir = await mkTemp(); const values = { component_id: 'test', @@ -97,8 +101,15 @@ describe('CookieCutter Templater', () => { await cookie.run({ directory: tempdir, values, dockerClient: mockDocker }); expect(runDockerContainer).toHaveBeenCalledWith({ - imageName: 'backstage/cookiecutter', - args: ['cookiecutter', '--no-input', '-o', '/result', '/template'], + imageName: 'spotify/backstage-cookiecutter', + args: [ + 'cookiecutter', + '--no-input', + '-o', + '/result', + '/template', + '--verbose', + ], templateDir: tempdir, resultDir: expect.stringContaining(`${tempdir}-result`), logStream: undefined, @@ -107,7 +118,7 @@ describe('CookieCutter Templater', () => { }); it('should return the result path to the end templated folder', async () => { - const tempdir = os.tmpdir(); + const tempdir = await mkTemp(); const values = { component_id: 'test', @@ -126,7 +137,7 @@ describe('CookieCutter Templater', () => { it('should pass through the streamer to the run docker helper', async () => { const stream = new PassThrough(); - const tempdir = os.tmpdir(); + const tempdir = await mkTemp(); const values = { component_id: 'test', @@ -141,8 +152,15 @@ describe('CookieCutter Templater', () => { }); expect(runDockerContainer).toHaveBeenCalledWith({ - imageName: 'backstage/cookiecutter', - args: ['cookiecutter', '--no-input', '-o', '/result', '/template'], + imageName: 'spotify/backstage-cookiecutter', + args: [ + 'cookiecutter', + '--no-input', + '-o', + '/result', + '/template', + '--verbose', + ], templateDir: tempdir, resultDir: expect.stringContaining(`${tempdir}-result`), logStream: stream, From f07b794987db8a6ad785804145b90b75fdd43484 Mon Sep 17 00:00:00 2001 From: blam Date: Sun, 28 Jun 2020 00:05:45 +0200 Subject: [PATCH 30/35] chore(scaffolder): trying to fix some tests on CI, they pass locall --- .../src/scaffolder/stages/templater/cookiecutter.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.test.ts b/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.test.ts index a24f1ed64f..8c0cd5d6d7 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.test.ts @@ -18,6 +18,7 @@ jest.mock('./helpers', () => ({ runDockerContainer: jest.fn() })); import { CookieCutter } from './cookiecutter'; import fs from 'fs-extra'; import os from 'os'; +import path from 'path'; import { RunDockerContainerOptions } from './helpers'; import { PassThrough } from 'stream'; import Docker from 'dockerode'; @@ -37,7 +38,7 @@ describe('CookieCutter Templater', () => { const mkTemp = async () => { const tempDir = os.tmpdir(); - return await fs.promises.mkdtemp(tempDir); + return await fs.promises.mkdtemp(path.join(tempDir, 'temp')); }; it('should write a cookiecutter.json file with the values from the entitiy', async () => { From 9d8709d5b78c4d8b1a992cedd8fdaf826216cf8a Mon Sep 17 00:00:00 2001 From: blam Date: Sun, 28 Jun 2020 00:09:44 +0200 Subject: [PATCH 31/35] chore(scaffolder): fixing linting issues --- .../src/scaffolder/stages/templater/cookiecutter.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.test.ts b/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.test.ts index 8c0cd5d6d7..994307e8b7 100644 --- a/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/stages/templater/cookiecutter.test.ts @@ -126,13 +126,13 @@ describe('CookieCutter Templater', () => { description: 'description', }; - const path = await cookie.run({ + const returnPath = await cookie.run({ directory: tempdir, values, dockerClient: mockDocker, }); - expect(path.startsWith(`${tempdir}-result`)).toBeTruthy(); + expect(returnPath.startsWith(`${tempdir}-result`)).toBeTruthy(); }); it('should pass through the streamer to the run docker helper', async () => { From 85bb1a3e83a7f8717243f394f45818afb19b7526 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 29 Jun 2020 13:09:05 +0200 Subject: [PATCH 32/35] core: pin material-table to 1.62.x --- packages/core/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/package.json b/packages/core/package.json index 10a6c5d045..e49e4616ae 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -40,7 +40,7 @@ "classnames": "^2.2.6", "clsx": "^1.1.0", "lodash": "^4.17.15", - "material-table": "^1.58.0", + "material-table": "1.62.x", "prop-types": "^15.7.2", "rc-progress": "^3.0.0", "react": "^16.12.0", From 580569e40350ab7a796a9c323050f83ce2181d71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Mon, 29 Jun 2020 14:42:52 +0200 Subject: [PATCH 33/35] yarn.lock again --- yarn.lock | 63 ++++++++++++++++++++++++------------------------------- 1 file changed, 27 insertions(+), 36 deletions(-) diff --git a/yarn.lock b/yarn.lock index 4ee33d5b1d..8a8fd29dcd 100644 --- a/yarn.lock +++ b/yarn.lock @@ -868,14 +868,6 @@ "@babel/plugin-transform-react-jsx-self" "^7.9.0" "@babel/plugin-transform-react-jsx-source" "^7.9.0" -"@babel/runtime-corejs2@^7.4.4": - version "7.9.2" - resolved "https://registry.npmjs.org/@babel/runtime-corejs2/-/runtime-corejs2-7.9.2.tgz#f11d074ff99b9b4319b5ecf0501f12202bf2bf4d" - integrity sha512-ayjSOxuK2GaSDJFCtLgHnYjuMyIpViNujWrZo8GUpN60/n7juzJKK5yOo6RFVb0zdU9ACJFK+MsZrUnj3OmXMw== - dependencies: - core-js "^2.6.5" - regenerator-runtime "^0.13.4" - "@babel/runtime-corejs3@^7.10.2": version "7.10.3" resolved "https://registry.npmjs.org/@babel/runtime-corejs3/-/runtime-corejs3-7.10.3.tgz#931ed6941d3954924a7aa967ee440e60c507b91a" @@ -6544,7 +6536,7 @@ core-js-pure@^3.0.0, core-js-pure@^3.0.1: resolved "https://registry.npmjs.org/core-js-pure/-/core-js-pure-3.6.4.tgz#4bf1ba866e25814f149d4e9aaa08c36173506e3a" integrity sha512-epIhRLkXdgv32xIUFaaAry2wdxZYBi6bgM7cB136dzzXXa+dFyRLTZeLUJxnd8ShrmyVXBub63n2NHo2JAt8Cw== -core-js@^2.4.0, core-js@^2.6.5: +core-js@^2.4.0: version "2.6.11" resolved "https://registry.npmjs.org/core-js/-/core-js-2.6.11.tgz#38831469f9922bded8ee21c9dc46985e0399308c" integrity sha512-5wjnpaT/3dV+XB4borEsnAYQchn00XSgTAWKDkEqv+K8KevjbzmofK6hfJ9TZIlpj2N0xQpazy7PiRQiWHqzWg== @@ -6701,10 +6693,10 @@ crypto-random-string@^2.0.0: resolved "https://registry.npmjs.org/crypto-random-string/-/crypto-random-string-2.0.0.tgz#ef2a7a966ec11083388369baa02ebead229b30d5" integrity sha512-v1plID3y9r/lPhviJ1wrXpLeyUIGAZ2SHNYTEapm7/8A9nLPoyvVp3RK/EPFqn5kEznyWgYZNsRtYYIWbuG8KA== -css-box-model@^1.1.2: - version "1.2.0" - resolved "https://registry.npmjs.org/css-box-model/-/css-box-model-1.2.0.tgz#3a26377b4162b3200d2ede4b064ec5b6a75186d0" - integrity sha512-lri0br+jSNV0kkkiGEp9y9y3Njq2PmpqbeGWRFQJuZteZzY9iC9GZhQ8Y4WpPwM/2YocjHePxy14igJY7YKzkA== +css-box-model@^1.2.0: + version "1.2.1" + resolved "https://registry.npmjs.org/css-box-model/-/css-box-model-1.2.1.tgz#59951d3b81fd6b2074a62d49444415b0d2b4d7c1" + integrity sha512-a7Vr4Q/kd/aw96bnJG332W9V9LkJO69JRcaCYDUqjp6/z0w6VcZjgAcTbgFxEPfBgdnAwlh3iwu+hLopa+flJw== dependencies: tiny-invariant "^1.0.6" @@ -12427,10 +12419,10 @@ markdown-to-jsx@^6.11.4: prop-types "^15.6.2" unquote "^1.1.0" -material-table@^1.58.0: - version "1.58.2" - resolved "https://registry.npmjs.org/material-table/-/material-table-1.58.2.tgz#dc0d19652848e6bb92f747d122bd7d4681cca6dc" - integrity sha512-s/m6ebyXFXmg07zxv1Fl6qPySKaiQhASXaOB3ubRKUFA1DkryUy3PGSEVWTjUYnRyi63kGq+N6b5nsokLR6m5A== +material-table@1.62.x: + version "1.62.0" + resolved "https://registry.npmjs.org/material-table/-/material-table-1.62.0.tgz#117793ebf16ab0fccbb6f8a670d849a7be0b5995" + integrity sha512-+3tnk32lXtkXeKM7k/hZ82jpSzlXU5CsWXqJHq4Tl0Un7ycjK2Kef6EMPqeE3i58vKNqbIvbrFf/ESH0D/Qwig== dependencies: "@date-io/date-fns" "^1.1.0" "@material-ui/pickers" "^3.2.2" @@ -12440,7 +12432,7 @@ material-table@^1.58.0: fast-deep-equal "2.0.1" filefy "0.1.10" prop-types "^15.6.2" - react-beautiful-dnd "11.0.3" + react-beautiful-dnd "^13.0.0" react-double-scrollbar "0.0.15" md5.js@^1.3.4: @@ -12479,7 +12471,7 @@ media-typer@0.3.0: resolved "https://registry.npmjs.org/media-typer/-/media-typer-0.3.0.tgz#8710d7af0aa626f8fffa1ce00168545263255748" integrity sha1-hxDXrwqmJvj/+hzgAWhUUmMlV0g= -memoize-one@^5.0.4: +memoize-one@^5.1.1: version "5.1.1" resolved "https://registry.npmjs.org/memoize-one/-/memoize-one-5.1.1.tgz#047b6e3199b508eaec03504de71229b8eb1d75c0" integrity sha512-HKeeBpWvqiVJD57ZUAsJNm71eHTykffzcLZVYWiVfQeI1rJtuEaS7hQiEpWfVVk18donPwJEcFKIkCmPJNOhHA== @@ -15128,7 +15120,7 @@ quick-lru@^1.0.0: resolved "https://registry.npmjs.org/quick-lru/-/quick-lru-1.1.0.tgz#4360b17c61136ad38078397ff11416e186dcfbb8" integrity sha1-Q2CxfGETatOAeDl/8RQW4Ybc+7g= -raf-schd@^4.0.0: +raf-schd@^4.0.2: version "4.0.2" resolved "https://registry.npmjs.org/raf-schd/-/raf-schd-4.0.2.tgz#bd44c708188f2e84c810bf55fcea9231bcaed8a0" integrity sha512-VhlMZmGy6A6hrkJWHLNTGl5gtgMUm+xfGza6wbwnE914yeQ5Ybm18vgM734RZhMgfw4tacUrWseGZlpUrrakEQ== @@ -15211,19 +15203,18 @@ rc@^1.2.7, rc@^1.2.8: minimist "^1.2.0" strip-json-comments "~2.0.1" -react-beautiful-dnd@11.0.3: - version "11.0.3" - resolved "https://registry.npmjs.org/react-beautiful-dnd/-/react-beautiful-dnd-11.0.3.tgz#5678bb3e725d8b56cb7cf57f56e952105fc4f2af" - integrity sha512-2FX2SnOlKMmfn90xUHCav7cxRWXwY7FeRa6TzdxWeX7DdP5JTvVQcsWgiOkdbJSj+J+1q1nA9QO4/HQ52D0DAA== +react-beautiful-dnd@^13.0.0: + version "13.0.0" + resolved "https://registry.npmjs.org/react-beautiful-dnd/-/react-beautiful-dnd-13.0.0.tgz#f70cc8ff82b84bc718f8af157c9f95757a6c3b40" + integrity sha512-87It8sN0ineoC3nBW0SbQuTFXM6bUqM62uJGY4BtTf0yzPl8/3+bHMWkgIe0Z6m8e+gJgjWxefGRVfpE3VcdEg== dependencies: - "@babel/runtime-corejs2" "^7.4.4" - css-box-model "^1.1.2" - memoize-one "^5.0.4" - raf-schd "^4.0.0" - react-redux "^7.0.3" - redux "^4.0.1" - tiny-invariant "^1.0.4" - use-memo-one "^1.1.0" + "@babel/runtime" "^7.8.4" + css-box-model "^1.2.0" + memoize-one "^5.1.1" + raf-schd "^4.0.2" + react-redux "^7.1.1" + redux "^4.0.4" + use-memo-one "^1.1.1" react-clientside-effect@^1.2.2: version "1.2.2" @@ -15473,7 +15464,7 @@ react-popper@^1.3.6: typed-styles "^0.0.7" warning "^4.0.2" -react-redux@^7.0.3: +react-redux@^7.1.1: version "7.2.0" resolved "https://registry.npmjs.org/react-redux/-/react-redux-7.2.0.tgz#f970f62192b3981642fec46fd0db18a074fe879d" integrity sha512-EvCAZYGfOLqwV7gh849xy9/pt55rJXPwmYvI4lilPM5rUT/1NxuuN59ipdBksRVSvz0KInbPnp4IfoXJXCqiDA== @@ -15849,7 +15840,7 @@ redent@^3.0.0: indent-string "^4.0.0" strip-indent "^3.0.0" -redux@^4.0.1: +redux@^4.0.4: version "4.0.5" resolved "https://registry.npmjs.org/redux/-/redux-4.0.5.tgz#4db5de5816e17891de8a80c424232d06f051d93f" integrity sha512-VSz1uMAH24DM6MF72vcojpYPtrTUu3ByVWfPL1nPfVRb5mZVTve5GnNCUV53QM/BZ66xfWrm0CTWoM+Xlz8V1w== @@ -17948,7 +17939,7 @@ tiny-emitter@^2.0.0: resolved "https://registry.npmjs.org/tiny-emitter/-/tiny-emitter-2.1.0.tgz#1d1a56edfc51c43e863cbb5382a72330e3555423" integrity sha512-NB6Dk1A9xgQPMoGqC5CVXn123gWyte215ONT5Pp5a0yt4nlEoO1ZWeCwpncaekPHXO60i47ihFnZPiRPjRMq4Q== -tiny-invariant@^1.0.2, tiny-invariant@^1.0.4, tiny-invariant@^1.0.6: +tiny-invariant@^1.0.2, tiny-invariant@^1.0.6: version "1.1.0" resolved "https://registry.npmjs.org/tiny-invariant/-/tiny-invariant-1.1.0.tgz#634c5f8efdc27714b7f386c35e6760991d230875" integrity sha512-ytxQvrb1cPc9WBEI/HSeYYoGD0kWnGEOR8RY6KomWLBVhqz0RgTwVO9dLrGz7dC+nN9llyI7OKAgRq8Vq4ZBSw== @@ -18587,7 +18578,7 @@ use-callback-ref@^1.2.1: resolved "https://registry.npmjs.org/use-callback-ref/-/use-callback-ref-1.2.1.tgz#898759ccb9e14be6c7a860abafa3ffbd826c89bb" integrity sha512-C3nvxh0ZpaOxs9RCnWwAJ+7bJPwQI8LHF71LzbQ3BvzH5XkdtlkMadqElGevg5bYBDFip4sAnD4m06zAKebg1w== -use-memo-one@^1.1.0: +use-memo-one@^1.1.1: version "1.1.1" resolved "https://registry.npmjs.org/use-memo-one/-/use-memo-one-1.1.1.tgz#39e6f08fe27e422a7d7b234b5f9056af313bd22c" integrity sha512-oFfsyun+bP7RX8X2AskHNTxu+R3QdE/RC5IefMbqptmACAA/gfol1KDD5KRzPsGMa62sWxGZw+Ui43u6x4ddoQ== From fc7b0a0aa55d7a2acadbceeadf4d1243a13e6087 Mon Sep 17 00:00:00 2001 From: Emma Indal Date: Mon, 29 Jun 2020 15:23:09 +0200 Subject: [PATCH 34/35] fix(techdocs): move card component into techdocs plugin from backstage core --- .../src/layout/ItemCard/InfoCard.stories.tsx | 35 ------------------- packages/core/src/layout/ItemCard/index.ts | 17 --------- packages/core/src/layout/index.ts | 1 - .../src/reader/components/DocsCard.tsx | 4 +-- .../techdocs/src/reader/components/Reader.tsx | 13 ++++--- 5 files changed, 10 insertions(+), 60 deletions(-) delete mode 100644 packages/core/src/layout/ItemCard/InfoCard.stories.tsx delete mode 100644 packages/core/src/layout/ItemCard/index.ts rename packages/core/src/layout/ItemCard/ItemCard.tsx => plugins/techdocs/src/reader/components/DocsCard.tsx (96%) diff --git a/packages/core/src/layout/ItemCard/InfoCard.stories.tsx b/packages/core/src/layout/ItemCard/InfoCard.stories.tsx deleted file mode 100644 index 87c5b0684e..0000000000 --- a/packages/core/src/layout/ItemCard/InfoCard.stories.tsx +++ /dev/null @@ -1,35 +0,0 @@ -/* - * Copyright 2020 Spotify AB - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -import React, { FC } from 'react'; -import { ItemCard } from '.'; -import { Grid } from '@material-ui/core'; - -export default { - title: 'Item Card', - component: ItemCard, -}; - -const Wrapper: FC<{}> = ({ children }) => ( - - {children} - -); - -export const Default = () => ( - - - -); diff --git a/packages/core/src/layout/ItemCard/index.ts b/packages/core/src/layout/ItemCard/index.ts deleted file mode 100644 index b38dd2acc2..0000000000 --- a/packages/core/src/layout/ItemCard/index.ts +++ /dev/null @@ -1,17 +0,0 @@ -/* - * Copyright 2020 Spotify AB - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -export { ItemCard } from './ItemCard'; diff --git a/packages/core/src/layout/index.ts b/packages/core/src/layout/index.ts index e8245119f3..e2de159ec6 100644 --- a/packages/core/src/layout/index.ts +++ b/packages/core/src/layout/index.ts @@ -26,4 +26,3 @@ export * from './Sidebar'; export * from './SignInPage'; export * from './TabbedCard'; export * from './HeaderTabs'; -export * from './ItemCard'; diff --git a/packages/core/src/layout/ItemCard/ItemCard.tsx b/plugins/techdocs/src/reader/components/DocsCard.tsx similarity index 96% rename from packages/core/src/layout/ItemCard/ItemCard.tsx rename to plugins/techdocs/src/reader/components/DocsCard.tsx index 86ad53e8dd..68f42bf84a 100644 --- a/packages/core/src/layout/ItemCard/ItemCard.tsx +++ b/plugins/techdocs/src/reader/components/DocsCard.tsx @@ -37,7 +37,7 @@ const useStyles = makeStyles(theme => ({ }, })); -type ItemCardProps = { +type DocsCardProps = { description: string; tags?: string[]; title: string; @@ -45,7 +45,7 @@ type ItemCardProps = { label: string; onClick?: () => void; }; -export const ItemCard: FC = ({ +export const DocsCard: FC = ({ description, tags, title, diff --git a/plugins/techdocs/src/reader/components/Reader.tsx b/plugins/techdocs/src/reader/components/Reader.tsx index 474457ac5c..a4ff03af47 100644 --- a/plugins/techdocs/src/reader/components/Reader.tsx +++ b/plugins/techdocs/src/reader/components/Reader.tsx @@ -17,16 +17,19 @@ import React from 'react'; import { useShadowDom } from '..'; import { useAsync } from 'react-use'; +import { useLocation, useParams, useNavigate } from 'react-router-dom'; + +import { Grid } from '@material-ui/core'; +import { Header, Content } from '@backstage/core'; + import transformer, { addBaseUrl, rewriteDocLinks, addEventListener, } from '../transformers'; import { docStorageURL } from '../../config'; -import { Grid } from '@material-ui/core'; -import { Header, Content, ItemCard } from '@backstage/core'; -import { useLocation, useParams, useNavigate } from 'react-router-dom'; import URLParser from '../urlParser'; +import { DocsCard } from './DocsCard'; const useFetch = (url: string) => { const state = useAsync(async () => { @@ -102,7 +105,7 @@ export const Reader = () => { - navigate('/docs/mkdocs')} tags={['Developer Tool']} title="MkDocs" @@ -111,7 +114,7 @@ export const Reader = () => { /> - navigate('/docs/backstage-microsite')} tags={['Service']} title="Backstage" From 7ca78c23e78b6b95d72015225e14586807b9070d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Mon, 29 Jun 2020 16:16:00 +0200 Subject: [PATCH 35/35] nitpick: newlines between mock entity outputs --- plugins/catalog-backend/scripts/mock-data | 1 + plugins/scaffolder-backend/scripts/mock-data | 1 + 2 files changed, 2 insertions(+) diff --git a/plugins/catalog-backend/scripts/mock-data b/plugins/catalog-backend/scripts/mock-data index 01675db363..76f97ccf3a 100755 --- a/plugins/catalog-backend/scripts/mock-data +++ b/plugins/catalog-backend/scripts/mock-data @@ -15,4 +15,5 @@ for URL in \ --request POST 'localhost:7000/catalog/locations' \ --header 'Content-Type: application/json' \ --data-raw "{\"type\": \"github\", \"target\": \"https://github.com/spotify/backstage/blob/master/packages/catalog-model/examples/${URL}\"}" + echo done diff --git a/plugins/scaffolder-backend/scripts/mock-data b/plugins/scaffolder-backend/scripts/mock-data index 594bd76607..2424d15094 100755 --- a/plugins/scaffolder-backend/scripts/mock-data +++ b/plugins/scaffolder-backend/scripts/mock-data @@ -9,4 +9,5 @@ for URL in \ --request POST 'localhost:7000/catalog/locations' \ --header 'Content-Type: application/json' \ --data-raw "{\"type\": \"file\", \"target\": \"$(pwd)/sample-templates/${URL}/template.yaml\"}" + echo done