From 159399107fb8ba4d0e0544ef1603b1a6220d9a61 Mon Sep 17 00:00:00 2001 From: Alex Eftimie Date: Wed, 22 May 2024 09:31:11 +0200 Subject: [PATCH 01/10] Try to switch away from polyfill Signed-off-by: Alex Eftimie --- plugins/techdocs/package.json | 1 + plugins/techdocs/src/alpha.tsx | 5 +-- plugins/techdocs/src/client.ts | 67 +++++++++++----------------------- yarn.lock | 8 ++++ 4 files changed, 31 insertions(+), 50 deletions(-) diff --git a/plugins/techdocs/package.json b/plugins/techdocs/package.json index 845b65cd8f..22f3074bea 100644 --- a/plugins/techdocs/package.json +++ b/plugins/techdocs/package.json @@ -70,6 +70,7 @@ "@material-ui/icons": "^4.9.1", "@material-ui/lab": "4.0.0-alpha.61", "@material-ui/styles": "^4.10.0", + "@microsoft/fetch-event-source": "^2.0.1", "@types/react": "^16.13.1 || ^17.0.0 || ^18.0.0", "dompurify": "^3.0.0", "event-source-polyfill": "1.0.25", diff --git a/plugins/techdocs/src/alpha.tsx b/plugins/techdocs/src/alpha.tsx index 88dce3b48e..07fd7667ff 100644 --- a/plugins/techdocs/src/alpha.tsx +++ b/plugins/techdocs/src/alpha.tsx @@ -29,7 +29,6 @@ import { createApiFactory, discoveryApiRef, fetchApiRef, - identityApiRef, } from '@backstage/core-plugin-api'; import { compatWrapper, @@ -55,14 +54,12 @@ const techDocsStorageApi = createApiExtension({ deps: { configApi: configApiRef, discoveryApi: discoveryApiRef, - identityApi: identityApiRef, fetchApi: fetchApiRef, }, - factory: ({ configApi, discoveryApi, identityApi, fetchApi }) => + factory: ({ configApi, discoveryApi, fetchApi }) => new TechDocsStorageClient({ configApi, discoveryApi, - identityApi, fetchApi, }), }), diff --git a/plugins/techdocs/src/client.ts b/plugins/techdocs/src/client.ts index 0f11eb4068..76338a8f1e 100644 --- a/plugins/techdocs/src/client.ts +++ b/plugins/techdocs/src/client.ts @@ -16,11 +16,7 @@ import { CompoundEntityRef } from '@backstage/catalog-model'; import { Config } from '@backstage/config'; -import { - DiscoveryApi, - FetchApi, - IdentityApi, -} from '@backstage/core-plugin-api'; +import { DiscoveryApi, FetchApi } from '@backstage/core-plugin-api'; import { NotFoundError, ResponseError } from '@backstage/errors'; import { SyncResult, @@ -29,7 +25,7 @@ import { TechDocsMetadata, TechDocsStorageApi, } from '@backstage/plugin-techdocs-react'; -import { EventSourcePolyfill } from 'event-source-polyfill'; +import { fetchEventSource } from '@microsoft/fetch-event-source'; /** * API to talk to `techdocs-backend`. @@ -124,18 +120,15 @@ export class TechDocsClient implements TechDocsApi { export class TechDocsStorageClient implements TechDocsStorageApi { public configApi: Config; public discoveryApi: DiscoveryApi; - public identityApi: IdentityApi; private fetchApi: FetchApi; constructor(options: { configApi: Config; discoveryApi: DiscoveryApi; - identityApi: IdentityApi; fetchApi: FetchApi; }) { this.configApi = options.configApi; this.discoveryApi = options.discoveryApi; - this.identityApi = options.identityApi; this.fetchApi = options.fetchApi; } @@ -213,47 +206,29 @@ export class TechDocsStorageClient implements TechDocsStorageApi { const apiOrigin = await this.getApiOrigin(); const url = `${apiOrigin}/sync/${namespace}/${kind}/${name}`; - const { token } = await this.identityApi.getCredentials(); return new Promise((resolve, reject) => { - // Polyfill is used to add support for custom headers and auth - const source = new EventSourcePolyfill(url, { + fetchEventSource(url, { withCredentials: true, - headers: token ? { Authorization: `Bearer ${token}` } : {}, + fetch: this.fetchApi.fetch, + onmessage(e: any) { + if (e.data) { + logHandler(JSON.parse(e.data)); + } + }, + onclose(e: any) { + let updated: boolean = false; + + if (e.data) { + ({ updated } = JSON.parse(e.data)); + } + + resolve(updated ? 'updated' : 'cached'); + }, + onerror(e: any) { + reject(e); + }, }); - - source.addEventListener('log', (e: any) => { - if (e.data) { - logHandler(JSON.parse(e.data)); - } - }); - - source.addEventListener('finish', (e: any) => { - let updated: boolean = false; - - if (e.data) { - ({ updated } = JSON.parse(e.data)); - } - - resolve(updated ? 'updated' : 'cached'); - }); - - source.onerror = (e: any) => { - source.close(); - - switch (e.status) { - // the endpoint returned a 404 status - case 404: - reject(new NotFoundError(e.message)); - return; - - // also handles the event-stream close. the reject is ignored if the Promise was already - // resolved by a finish event. - default: - reject(new Error(e.data)); - return; - } - }; }); } diff --git a/yarn.lock b/yarn.lock index d006daa23c..b70da3e894 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7535,6 +7535,7 @@ __metadata: "@material-ui/icons": ^4.9.1 "@material-ui/lab": 4.0.0-alpha.61 "@material-ui/styles": ^4.10.0 + "@microsoft/fetch-event-source": ^2.0.1 "@testing-library/dom": ^10.0.0 "@testing-library/jest-dom": ^6.0.0 "@testing-library/react": ^15.0.0 @@ -10507,6 +10508,13 @@ __metadata: languageName: node linkType: hard +"@microsoft/fetch-event-source@npm:^2.0.1": + version: 2.0.1 + resolution: "@microsoft/fetch-event-source@npm:2.0.1" + checksum: a50e1c0f33220206967266d0a4bbba0703e2793b079d9f6e6bfd48f71b2115964a803e14cf6e902c6fab321edc084f26022334f5eaacc2cec87f174715d41852 + languageName: node + linkType: hard + "@microsoft/microsoft-graph-types@npm:^2.6.0": version: 2.40.0 resolution: "@microsoft/microsoft-graph-types@npm:2.40.0" From f1c280b3a27e64ad0e9702d0846871f47f88dd8e Mon Sep 17 00:00:00 2001 From: Alex Eftimie Date: Wed, 22 May 2024 11:37:18 +0200 Subject: [PATCH 02/10] Update unit tests Signed-off-by: Alex Eftimie --- plugins/techdocs/src/client.test.ts | 150 ++++++++++------------------ plugins/techdocs/src/client.ts | 24 ++--- 2 files changed, 64 insertions(+), 110 deletions(-) diff --git a/plugins/techdocs/src/client.test.ts b/plugins/techdocs/src/client.test.ts index dde52cda49..788d7e3fed 100644 --- a/plugins/techdocs/src/client.test.ts +++ b/plugins/techdocs/src/client.test.ts @@ -17,16 +17,15 @@ import { UrlPatternDiscovery } from '@backstage/core-app-api'; import { IdentityApi } from '@backstage/core-plugin-api'; import { NotFoundError } from '@backstage/errors'; -import { EventSourcePolyfill } from 'event-source-polyfill'; +import { fetchEventSource } from '@microsoft/fetch-event-source'; import { MockConfigApi, MockFetchApi } from '@backstage/test-utils'; import { TechDocsStorageClient } from './client'; -const MockedEventSource = EventSourcePolyfill as jest.MockedClass< - typeof EventSourcePolyfill +jest.mock('@microsoft/fetch-event-source'); +const mockFetchEventSource = fetchEventSource as jest.MockedFunction< + typeof fetchEventSource >; -jest.mock('event-source-polyfill'); - const mockEntity = { kind: 'Component', namespace: 'default', @@ -51,7 +50,6 @@ describe('TechDocsStorageClient', () => { const storageApi = new TechDocsStorageClient({ configApi, discoveryApi, - identityApi, fetchApi, }); @@ -72,7 +70,6 @@ describe('TechDocsStorageClient', () => { const storageApi = new TechDocsStorageClient({ configApi, discoveryApi, - identityApi, fetchApi, }); @@ -84,71 +81,49 @@ describe('TechDocsStorageClient', () => { }); describe('syncEntityDocs', () => { - it('should create eventsource without headers', async () => { + it('should create eventsource with fetch', async () => { const storageApi = new TechDocsStorageClient({ configApi, discoveryApi, - identityApi, fetchApi, }); - MockedEventSource.prototype.addEventListener.mockImplementation( - (type, fn) => { - if (type === 'finish' && typeof fn === 'function') { - fn({ data: '{"updated": false}' } as any); - } - }, - ); - + mockFetchEventSource.mockImplementation(async (_url, options) => { + const { onopen, onmessage } = options; + await Promise.resolve(); + await onopen?.({ ok: true } as Response); + await Promise.resolve(); + onmessage?.({ id: '', event: 'finish', data: '{"updated": false}' }); + }); identityApi.getCredentials.mockResolvedValue({}); await storageApi.syncEntityDocs(mockEntity); - expect(MockedEventSource).toHaveBeenCalledWith( + expect(mockFetchEventSource).toHaveBeenCalledWith( 'http://backstage:9191/api/techdocs/sync/default/Component/test-component', - { withCredentials: true, headers: {} }, - ); - }); - - it('should create eventsource with headers', async () => { - const storageApi = new TechDocsStorageClient({ - configApi, - discoveryApi, - identityApi, - fetchApi, - }); - - MockedEventSource.prototype.addEventListener.mockImplementation( - (type, fn) => { - if (type === 'finish' && typeof fn === 'function') { - fn({ data: '{"updated": false}' } as any); - } + { + fetch: fetchApi.fetch, + onerror: expect.any(Function), + onmessage: expect.any(Function), }, ); - - identityApi.getCredentials.mockResolvedValue({ token: 'token' }); - await storageApi.syncEntityDocs(mockEntity); - - expect(MockedEventSource).toHaveBeenCalledWith( - 'http://backstage:9191/api/techdocs/sync/default/Component/test-component', - { withCredentials: true, headers: { Authorization: 'Bearer token' } }, - ); }); it('should resolve to cached', async () => { + const myFetchApi = new MockFetchApi({ + injectIdentityAuth: { identityApi }, + }); const storageApi = new TechDocsStorageClient({ configApi, discoveryApi, - identityApi, - fetchApi, + fetchApi: myFetchApi, + }); + mockFetchEventSource.mockImplementation(async (_url, options) => { + const { onopen, onmessage } = options; + await Promise.resolve(); + await onopen?.({ ok: true } as Response); + await Promise.resolve(); + onmessage?.({ id: '', event: 'finish', data: '{"updated": false}' }); }); - - MockedEventSource.prototype.addEventListener.mockImplementation( - (type, fn) => { - if (type === 'finish' && typeof fn === 'function') { - fn({ data: '{"updated": false}' } as any); - } - }, - ); identityApi.getCredentials.mockResolvedValue({}); await expect(storageApi.syncEntityDocs(mockEntity)).resolves.toEqual( @@ -160,17 +135,15 @@ describe('TechDocsStorageClient', () => { const storageApi = new TechDocsStorageClient({ configApi, discoveryApi, - identityApi, fetchApi, }); - - MockedEventSource.prototype.addEventListener.mockImplementation( - (type, fn) => { - if (type === 'finish' && typeof fn === 'function') { - fn({ data: '{"updated": true}' } as any); - } - }, - ); + mockFetchEventSource.mockImplementation(async (_url, options) => { + const { onopen, onmessage } = options; + await Promise.resolve(); + await onopen?.({ ok: true } as Response); + await Promise.resolve(); + onmessage?.({ id: '', event: 'finish', data: '{"updated": true}' }); + }); identityApi.getCredentials.mockResolvedValue({}); await expect(storageApi.syncEntityDocs(mockEntity)).resolves.toEqual( @@ -182,21 +155,18 @@ describe('TechDocsStorageClient', () => { const storageApi = new TechDocsStorageClient({ configApi, discoveryApi, - identityApi, fetchApi, }); - MockedEventSource.prototype.addEventListener.mockImplementation( - (type, fn) => { - if (type === 'log' && typeof fn === 'function') { - fn({ data: '"A log message"' } as any); - } - - if (type === 'finish' && typeof fn === 'function') { - fn({ data: '{"updated": false}' } as any); - } - }, - ); + mockFetchEventSource.mockImplementation(async (_url, options) => { + const { onopen, onmessage } = options; + await Promise.resolve(); + await onopen?.({ ok: true } as Response); + await Promise.resolve(); + onmessage?.({ id: '', event: 'log', data: '"A log message"' }); + await Promise.resolve(); + onmessage?.({ id: '', event: 'finish', data: '{"updated": false}' }); + }); identityApi.getCredentials.mockResolvedValue({}); const logHandler = jest.fn(); @@ -212,25 +182,18 @@ describe('TechDocsStorageClient', () => { const storageApi = new TechDocsStorageClient({ configApi, discoveryApi, - identityApi, fetchApi, }); + mockFetchEventSource.mockImplementation(async (_url, options) => { + const { onerror } = options; + onerror?.(new NotFoundError('Some not found warning')); + }); + // we await later after we emitted the error identityApi.getCredentials.mockResolvedValue({}); const promise = storageApi.syncEntityDocs(mockEntity).then(); - // flush the event loop - await new Promise(r => setTimeout(r)); - - const instance = MockedEventSource.mock - .instances[0] as jest.Mocked; - - instance.onerror?.({ - status: 404, - message: 'Some not found warning', - } as any); - await expect(promise).rejects.toThrow(NotFoundError); await expect(promise).rejects.toThrow('Some not found warning'); }); @@ -239,7 +202,6 @@ describe('TechDocsStorageClient', () => { const storageApi = new TechDocsStorageClient({ configApi, discoveryApi, - identityApi, fetchApi, }); @@ -247,16 +209,10 @@ describe('TechDocsStorageClient', () => { identityApi.getCredentials.mockResolvedValue({}); const promise = storageApi.syncEntityDocs(mockEntity).then(); - // flush the event loop - await new Promise(r => setTimeout(r)); - - const instance = MockedEventSource.mock - .instances[0] as jest.Mocked; - - instance.onerror?.({ - type: 'error', - data: 'Some other error', - } as any); + mockFetchEventSource.mockImplementation(async (_url, options) => { + const { onerror } = options; + onerror?.(new Error('Some other error')); + }); await expect(promise).rejects.toThrow(Error); await expect(promise).rejects.toThrow('Some other error'); diff --git a/plugins/techdocs/src/client.ts b/plugins/techdocs/src/client.ts index 76338a8f1e..b79c140750 100644 --- a/plugins/techdocs/src/client.ts +++ b/plugins/techdocs/src/client.ts @@ -209,24 +209,22 @@ export class TechDocsStorageClient implements TechDocsStorageApi { return new Promise((resolve, reject) => { fetchEventSource(url, { - withCredentials: true, fetch: this.fetchApi.fetch, onmessage(e: any) { - if (e.data) { + if (e.event === 'log') { logHandler(JSON.parse(e.data)); + } else if (e.event === 'finish') { + let updated: boolean = false; + if (e.data) { + ({ updated } = JSON.parse(e.data)); + } + resolve(updated ? 'updated' : 'cached'); + } else if (e.event === 'error') { + reject(new Error(e.data)); } }, - onclose(e: any) { - let updated: boolean = false; - - if (e.data) { - ({ updated } = JSON.parse(e.data)); - } - - resolve(updated ? 'updated' : 'cached'); - }, - onerror(e: any) { - reject(e); + onerror(err) { + reject(err); }, }); }); From db76476e6215d0c812b0ecbf077ed1e71cb2733a Mon Sep 17 00:00:00 2001 From: Alex Eftimie Date: Wed, 22 May 2024 11:45:06 +0200 Subject: [PATCH 03/10] Cleanup and fix build Signed-off-by: Alex Eftimie --- plugins/scaffolder/package.json | 1 + plugins/techdocs/dev/index.tsx | 7 +---- plugins/techdocs/package.json | 2 -- .../techdocs/src/event-source-polyfill.d.ts | 27 ------------------- plugins/techdocs/src/plugin.ts | 5 +--- yarn.lock | 10 +------ 6 files changed, 4 insertions(+), 48 deletions(-) delete mode 100644 plugins/techdocs/src/event-source-polyfill.d.ts diff --git a/plugins/scaffolder/package.json b/plugins/scaffolder/package.json index 73e231644a..e4ba300d27 100644 --- a/plugins/scaffolder/package.json +++ b/plugins/scaffolder/package.json @@ -103,6 +103,7 @@ "@testing-library/jest-dom": "^6.0.0", "@testing-library/react": "^15.0.0", "@testing-library/user-event": "^14.0.0", + "@types/event-source-polyfill": "^1.0.0", "@types/humanize-duration": "^3.18.1", "@types/json-schema": "^7.0.9", "msw": "^1.0.0" diff --git a/plugins/techdocs/dev/index.tsx b/plugins/techdocs/dev/index.tsx index 9cf0364c63..57ed36c4e3 100644 --- a/plugins/techdocs/dev/index.tsx +++ b/plugins/techdocs/dev/index.tsx @@ -25,11 +25,7 @@ import { techdocsStorageApiRef, } from '../src'; -import { - configApiRef, - discoveryApiRef, - identityApiRef, -} from '@backstage/core-plugin-api'; +import { configApiRef, discoveryApiRef } from '@backstage/core-plugin-api'; import { TechDocsReaderPageProvider } from '@backstage/plugin-techdocs-react'; import { Header, Page, TabbedLayout } from '@backstage/core-components'; @@ -135,7 +131,6 @@ createDevApp() deps: { configApi: configApiRef, discoveryApi: discoveryApiRef, - identityApi: identityApiRef, }, factory: () => apiBridge, }) diff --git a/plugins/techdocs/package.json b/plugins/techdocs/package.json index 22f3074bea..c0113c0c43 100644 --- a/plugins/techdocs/package.json +++ b/plugins/techdocs/package.json @@ -73,7 +73,6 @@ "@microsoft/fetch-event-source": "^2.0.1", "@types/react": "^16.13.1 || ^17.0.0 || ^18.0.0", "dompurify": "^3.0.0", - "event-source-polyfill": "1.0.25", "git-url-parse": "^14.0.0", "jss": "~10.10.0", "lodash": "^4.17.21", @@ -91,7 +90,6 @@ "@testing-library/react": "^15.0.0", "@testing-library/user-event": "^14.0.0", "@types/dompurify": "^3.0.0", - "@types/event-source-polyfill": "^1.0.0", "canvas": "^2.10.2" }, "peerDependencies": { diff --git a/plugins/techdocs/src/event-source-polyfill.d.ts b/plugins/techdocs/src/event-source-polyfill.d.ts deleted file mode 100644 index 8e298bde38..0000000000 --- a/plugins/techdocs/src/event-source-polyfill.d.ts +++ /dev/null @@ -1,27 +0,0 @@ -/* - * Copyright 2021 The Backstage Authors - * - * 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. - */ - -declare module 'event-source-polyfill' { - export class EventSourcePolyfill extends EventSource { - constructor( - url: string, - options?: { - withCredentials?: boolean; - headers?: HeadersInit; - }, - ); - } -} diff --git a/plugins/techdocs/src/plugin.ts b/plugins/techdocs/src/plugin.ts index bdd4814898..efa6bdfc1f 100644 --- a/plugins/techdocs/src/plugin.ts +++ b/plugins/techdocs/src/plugin.ts @@ -31,7 +31,6 @@ import { createRoutableExtension, discoveryApiRef, fetchApiRef, - identityApiRef, } from '@backstage/core-plugin-api'; import { createSearchResultListItemExtension, @@ -52,14 +51,12 @@ export const techdocsPlugin = createPlugin({ deps: { configApi: configApiRef, discoveryApi: discoveryApiRef, - identityApi: identityApiRef, fetchApi: fetchApiRef, }, - factory: ({ configApi, discoveryApi, identityApi, fetchApi }) => + factory: ({ configApi, discoveryApi, fetchApi }) => new TechDocsStorageClient({ configApi, discoveryApi, - identityApi, fetchApi, }), }), diff --git a/yarn.lock b/yarn.lock index b70da3e894..052268febb 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6967,6 +6967,7 @@ __metadata: "@testing-library/jest-dom": ^6.0.0 "@testing-library/react": ^15.0.0 "@testing-library/user-event": ^14.0.0 + "@types/event-source-polyfill": ^1.0.0 "@types/humanize-duration": ^3.18.1 "@types/json-schema": ^7.0.9 "@types/react": ^16.13.1 || ^17.0.0 || ^18.0.0 @@ -7541,11 +7542,9 @@ __metadata: "@testing-library/react": ^15.0.0 "@testing-library/user-event": ^14.0.0 "@types/dompurify": ^3.0.0 - "@types/event-source-polyfill": ^1.0.0 "@types/react": ^16.13.1 || ^17.0.0 || ^18.0.0 canvas: ^2.10.2 dompurify: ^3.0.0 - event-source-polyfill: 1.0.25 git-url-parse: ^14.0.0 jss: ~10.10.0 lodash: ^4.17.21 @@ -25499,13 +25498,6 @@ __metadata: languageName: node linkType: hard -"event-source-polyfill@npm:1.0.25": - version: 1.0.25 - resolution: "event-source-polyfill@npm:1.0.25" - checksum: ed30428cc80eadfd693d267ba4a72dceaae938174cd116081ce38ad62bfd95f199430be7e8341e6f8f1e29489bbd5cfd4b3f6c8d6d463435623f7f91ae5f71b1 - languageName: node - linkType: hard - "event-source-polyfill@npm:^1.0.31": version: 1.0.31 resolution: "event-source-polyfill@npm:1.0.31" From 147f11f08db08608a7439549198c64791ad7c44c Mon Sep 17 00:00:00 2001 From: Alex Eftimie Date: Wed, 22 May 2024 14:50:07 +0200 Subject: [PATCH 04/10] Fix build. Move scaffolder away from polyfill Signed-off-by: Alex Eftimie --- plugins/scaffolder/package.json | 3 +- plugins/scaffolder/src/api.test.ts | 54 +++++++++++++--------------- plugins/scaffolder/src/api.ts | 41 ++++++++++----------- plugins/scaffolder/src/setupTests.ts | 3 -- plugins/techdocs/api-report.md | 4 --- yarn.lock | 17 +-------- 6 files changed, 46 insertions(+), 76 deletions(-) diff --git a/plugins/scaffolder/package.json b/plugins/scaffolder/package.json index e4ba300d27..b8f69cebe1 100644 --- a/plugins/scaffolder/package.json +++ b/plugins/scaffolder/package.json @@ -70,6 +70,7 @@ "@material-ui/core": "^4.12.2", "@material-ui/icons": "^4.9.1", "@material-ui/lab": "4.0.0-alpha.61", + "@microsoft/fetch-event-source": "^2.0.1", "@react-hookz/web": "^24.0.0", "@rjsf/core": "5.18.2", "@rjsf/material-ui": "5.18.2", @@ -78,7 +79,6 @@ "@types/react": "^16.13.1 || ^17.0.0 || ^18.0.0", "@uiw/react-codemirror": "^4.9.3", "classnames": "^2.2.6", - "event-source-polyfill": "^1.0.31", "git-url-parse": "^14.0.0", "humanize-duration": "^3.25.1", "json-schema": "^0.4.0", @@ -103,7 +103,6 @@ "@testing-library/jest-dom": "^6.0.0", "@testing-library/react": "^15.0.0", "@testing-library/user-event": "^14.0.0", - "@types/event-source-polyfill": "^1.0.0", "@types/humanize-duration": "^3.18.1", "@types/json-schema": "^7.0.9", "msw": "^1.0.0" diff --git a/plugins/scaffolder/src/api.test.ts b/plugins/scaffolder/src/api.test.ts index b9b1881066..cf482e52f7 100644 --- a/plugins/scaffolder/src/api.test.ts +++ b/plugins/scaffolder/src/api.test.ts @@ -20,14 +20,13 @@ import { MockFetchApi, setupRequestMockHandlers } from '@backstage/test-utils'; import { rest } from 'msw'; import { setupServer } from 'msw/node'; import { ScaffolderClient } from './api'; -import { EventSourcePolyfill } from 'event-source-polyfill'; +import { fetchEventSource } from '@microsoft/fetch-event-source'; -const MockedEventSource = EventSourcePolyfill as jest.MockedClass< - typeof EventSourcePolyfill +jest.mock('@microsoft/fetch-event-source'); +const mockFetchEventSource = fetchEventSource as jest.MockedFunction< + typeof fetchEventSource >; -jest.mock('event-source-polyfill'); - const server = setupServer(); describe('api', () => { @@ -87,26 +86,23 @@ describe('api', () => { describe('streamEvents', () => { describe('eventsource', () => { it('should work', async () => { - MockedEventSource.prototype.addEventListener.mockImplementation( - (type, fn) => { - if (typeof fn !== 'function') { - return; - } - - if (type === 'log') { - fn({ - data: '{"id":1,"taskId":"a-random-id","type":"log","createdAt":"","body":{"message":"My log message"}}', - } as any); - } else if (type === 'completion') { - fn({ - data: '{"id":2,"taskId":"a-random-id","type":"completion","createdAt":"","body":{"message":"Finished!"}}', - } as any); - } - }, - ); - - const token = 'fake-token'; - identityApi.getCredentials.mockResolvedValue({ token: token }); + mockFetchEventSource.mockImplementation(async (_url, options) => { + const { onopen, onmessage } = options; + await Promise.resolve(); + await onopen?.({ ok: true } as Response); + await Promise.resolve(); + onmessage?.({ + id: '', + event: 'log', + data: '{"id":1,"taskId":"a-random-id","type":"log","createdAt":"","body":{"message":"My log message"}}', + }); + await Promise.resolve(); + onmessage?.({ + id: '', + event: 'completion', + data: '{"id":2,"taskId":"a-random-id","type":"completion","createdAt":"","body":{"message":"Finished!"}}', + }); + }); const next = jest.fn(); @@ -116,14 +112,14 @@ describe('api', () => { .subscribe({ next, complete }); }); - expect(MockedEventSource).toHaveBeenCalledWith( + expect(mockFetchEventSource).toHaveBeenCalledWith( 'http://backstage/api/v2/tasks/a-random-task-id/eventstream', { - withCredentials: true, - headers: { Authorization: `Bearer ${token}` }, + fetch: fetchApi.fetch, + onmessage: expect.any(Function), + onerror: expect.any(Function), }, ); - expect(MockedEventSource.prototype.close).toHaveBeenCalled(); expect(next).toHaveBeenCalledTimes(2); expect(next).toHaveBeenCalledWith({ diff --git a/plugins/scaffolder/src/api.ts b/plugins/scaffolder/src/api.ts index fe0d7c11d0..8dc11ef296 100644 --- a/plugins/scaffolder/src/api.ts +++ b/plugins/scaffolder/src/api.ts @@ -41,7 +41,7 @@ import { } from '@backstage/plugin-scaffolder-react'; import queryString from 'qs'; -import { EventSourcePolyfill } from 'event-source-polyfill'; +import { fetchEventSource } from '@microsoft/fetch-event-source'; /** * An API to interact with the scaffolder backend. @@ -226,11 +226,8 @@ export class ScaffolderClient implements ScaffolderApi { params.set('after', String(Number(after))); } - Promise.all([ - this.discoveryApi.getBaseUrl('scaffolder'), - this.identityApi?.getCredentials(), - ]).then( - ([baseUrl, credentials]) => { + this.discoveryApi.getBaseUrl('scaffolder').then( + baseUrl => { const url = `${baseUrl}/v2/tasks/${encodeURIComponent( taskId, )}/eventstream`; @@ -245,22 +242,22 @@ export class ScaffolderClient implements ScaffolderApi { } }; - const eventSource = new EventSourcePolyfill(url, { - withCredentials: true, - headers: credentials?.token - ? { Authorization: `Bearer ${credentials.token}` } - : {}, - }); - eventSource.addEventListener('log', processEvent); - eventSource.addEventListener('recovered', processEvent); - eventSource.addEventListener('cancelled', processEvent); - eventSource.addEventListener('completion', (event: any) => { - processEvent(event); - eventSource.close(); - subscriber.complete(); - }); - eventSource.addEventListener('error', event => { - subscriber.error(event); + fetchEventSource(url, { + fetch: this.fetchApi.fetch, + onmessage(e: any) { + if (e.event === 'log') { + processEvent(e); + return; + } else if (e.event === 'completion') { + processEvent(e); + subscriber.complete(); + return; + } + processEvent(e); + }, + onerror(err) { + subscriber.error(err); + }, }); }, error => { diff --git a/plugins/scaffolder/src/setupTests.ts b/plugins/scaffolder/src/setupTests.ts index 8608e5b98e..7b25d3706a 100644 --- a/plugins/scaffolder/src/setupTests.ts +++ b/plugins/scaffolder/src/setupTests.ts @@ -16,9 +16,6 @@ import '@testing-library/jest-dom'; -const { EventSourcePolyfill } = jest.requireMock('event-source-polyfill'); -global.EventSource = EventSourcePolyfill; - // Patch jsdom to add feature used by CodeMirror document.createRange = () => { const range = new Range(); diff --git a/plugins/techdocs/api-report.md b/plugins/techdocs/api-report.md index ff43f4309b..582e941507 100644 --- a/plugins/techdocs/api-report.md +++ b/plugins/techdocs/api-report.md @@ -14,7 +14,6 @@ import { DiscoveryApi } from '@backstage/core-plugin-api'; import { Entity } from '@backstage/catalog-model'; import { EntityOwnerPickerProps } from '@backstage/plugin-catalog-react'; import { FetchApi } from '@backstage/core-plugin-api'; -import { IdentityApi } from '@backstage/core-plugin-api'; import { JSX as JSX_2 } from 'react'; import { PropsWithChildren } from 'react'; import { default as React_2 } from 'react'; @@ -465,7 +464,6 @@ export class TechDocsStorageClient implements TechDocsStorageApi_2 { constructor(options: { configApi: Config; discoveryApi: DiscoveryApi; - identityApi: IdentityApi; fetchApi: FetchApi; }); // (undocumented) @@ -485,8 +483,6 @@ export class TechDocsStorageClient implements TechDocsStorageApi_2 { getEntityDocs(entityId: CompoundEntityRef, path: string): Promise; // (undocumented) getStorageUrl(): Promise; - // (undocumented) - identityApi: IdentityApi; syncEntityDocs( entityId: CompoundEntityRef, logHandler?: (line: string) => void, diff --git a/yarn.lock b/yarn.lock index 052268febb..3ac0d88613 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6958,6 +6958,7 @@ __metadata: "@material-ui/core": ^4.12.2 "@material-ui/icons": ^4.9.1 "@material-ui/lab": 4.0.0-alpha.61 + "@microsoft/fetch-event-source": ^2.0.1 "@react-hookz/web": ^24.0.0 "@rjsf/core": 5.18.2 "@rjsf/material-ui": 5.18.2 @@ -6967,13 +6968,11 @@ __metadata: "@testing-library/jest-dom": ^6.0.0 "@testing-library/react": ^15.0.0 "@testing-library/user-event": ^14.0.0 - "@types/event-source-polyfill": ^1.0.0 "@types/humanize-duration": ^3.18.1 "@types/json-schema": ^7.0.9 "@types/react": ^16.13.1 || ^17.0.0 || ^18.0.0 "@uiw/react-codemirror": ^4.9.3 classnames: ^2.2.6 - event-source-polyfill: ^1.0.31 git-url-parse: ^14.0.0 humanize-duration: ^3.25.1 json-schema: ^0.4.0 @@ -17134,13 +17133,6 @@ __metadata: languageName: node linkType: hard -"@types/event-source-polyfill@npm:^1.0.0": - version: 1.0.5 - resolution: "@types/event-source-polyfill@npm:1.0.5" - checksum: f506b68710162f2ade1bccbc5691b8c67e5a703e565df2bc0b7b5be2637ba838ef81ec6c10b03248fe4d054386d95a6e827c7aace6e924986c2b9985f77b55de - languageName: node - linkType: hard - "@types/expect@npm:^1.20.4": version: 1.20.4 resolution: "@types/expect@npm:1.20.4" @@ -25498,13 +25490,6 @@ __metadata: languageName: node linkType: hard -"event-source-polyfill@npm:^1.0.31": - version: 1.0.31 - resolution: "event-source-polyfill@npm:1.0.31" - checksum: 973f226404e2a1b14ed7ef15c718b89e213b41d7cfeeb1c10937fd09229f13904f3d7c3075ab28ccf858c213007559908eecdd577577330352f53a351383dd75 - languageName: node - linkType: hard - "event-target-shim@npm:^5.0.0": version: 5.0.1 resolution: "event-target-shim@npm:5.0.1" From cbebad1483dda205bef36d8533905d1d6ef80315 Mon Sep 17 00:00:00 2001 From: Alex Eftimie Date: Wed, 22 May 2024 14:53:42 +0200 Subject: [PATCH 05/10] Add changeset Signed-off-by: Alex Eftimie --- .changeset/empty-avocados-tease.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/empty-avocados-tease.md diff --git a/.changeset/empty-avocados-tease.md b/.changeset/empty-avocados-tease.md new file mode 100644 index 0000000000..da3aaba0e6 --- /dev/null +++ b/.changeset/empty-avocados-tease.md @@ -0,0 +1,6 @@ +--- +'@backstage/plugin-scaffolder': minor +'@backstage/plugin-techdocs': minor +--- + +Use fetch-event-source instead of event-source-polify in order to stop hardcoding the Authorization header and reuse the fetchApi configuration. From 79605f824ea5746657dcd363a49630905858732e Mon Sep 17 00:00:00 2001 From: Alex Eftimie Date: Wed, 22 May 2024 19:23:54 +0200 Subject: [PATCH 06/10] Update .changeset/empty-avocados-tease.md Co-authored-by: Aramis Sennyey <159921952+aramissennyeydd@users.noreply.github.com> Signed-off-by: Alex Eftimie --- .changeset/empty-avocados-tease.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/empty-avocados-tease.md b/.changeset/empty-avocados-tease.md index da3aaba0e6..08f191becd 100644 --- a/.changeset/empty-avocados-tease.md +++ b/.changeset/empty-avocados-tease.md @@ -3,4 +3,4 @@ '@backstage/plugin-techdocs': minor --- -Use fetch-event-source instead of event-source-polify in order to stop hardcoding the Authorization header and reuse the fetchApi configuration. +Internal updates to allow reusing Backstage's `fetchApi` implementation for event source requests. This allows you to for example, override the `Authorization` header. From a15948009f11f9b29ebc0b708735db33ae8b29c0 Mon Sep 17 00:00:00 2001 From: Alex Eftimie Date: Mon, 27 May 2024 16:09:26 +0200 Subject: [PATCH 07/10] address PR comments Signed-off-by: Alex Eftimie --- plugins/scaffolder/src/api.ts | 7 +++++-- plugins/techdocs/api-report.md | 1 + plugins/techdocs/src/client.ts | 6 +++++- plugins/techdocs/src/plugin.ts | 8 +++++++- 4 files changed, 18 insertions(+), 4 deletions(-) diff --git a/plugins/scaffolder/src/api.ts b/plugins/scaffolder/src/api.ts index 8dc11ef296..25f76c029a 100644 --- a/plugins/scaffolder/src/api.ts +++ b/plugins/scaffolder/src/api.ts @@ -41,7 +41,10 @@ import { } from '@backstage/plugin-scaffolder-react'; import queryString from 'qs'; -import { fetchEventSource } from '@microsoft/fetch-event-source'; +import { + EventSourceMessage, + fetchEventSource, +} from '@microsoft/fetch-event-source'; /** * An API to interact with the scaffolder backend. @@ -244,7 +247,7 @@ export class ScaffolderClient implements ScaffolderApi { fetchEventSource(url, { fetch: this.fetchApi.fetch, - onmessage(e: any) { + onmessage(e: EventSourceMessage) { if (e.event === 'log') { processEvent(e); return; diff --git a/plugins/techdocs/api-report.md b/plugins/techdocs/api-report.md index 582e941507..b756bc0ab0 100644 --- a/plugins/techdocs/api-report.md +++ b/plugins/techdocs/api-report.md @@ -465,6 +465,7 @@ export class TechDocsStorageClient implements TechDocsStorageApi_2 { configApi: Config; discoveryApi: DiscoveryApi; fetchApi: FetchApi; + identityApi?: any; }); // (undocumented) configApi: Config; diff --git a/plugins/techdocs/src/client.ts b/plugins/techdocs/src/client.ts index b79c140750..06362825a1 100644 --- a/plugins/techdocs/src/client.ts +++ b/plugins/techdocs/src/client.ts @@ -126,6 +126,8 @@ export class TechDocsStorageClient implements TechDocsStorageApi { configApi: Config; discoveryApi: DiscoveryApi; fetchApi: FetchApi; + // @deprecated Remove identityApiRef + identityApi?: any; }) { this.configApi = options.configApi; this.discoveryApi = options.discoveryApi; @@ -212,7 +214,9 @@ export class TechDocsStorageClient implements TechDocsStorageApi { fetch: this.fetchApi.fetch, onmessage(e: any) { if (e.event === 'log') { - logHandler(JSON.parse(e.data)); + if (e.data) { + logHandler(JSON.parse(e.data)); + } } else if (e.event === 'finish') { let updated: boolean = false; if (e.data) { diff --git a/plugins/techdocs/src/plugin.ts b/plugins/techdocs/src/plugin.ts index efa6bdfc1f..9d4157ba9e 100644 --- a/plugins/techdocs/src/plugin.ts +++ b/plugins/techdocs/src/plugin.ts @@ -31,6 +31,7 @@ import { createRoutableExtension, discoveryApiRef, fetchApiRef, + identityApiRef, } from '@backstage/core-plugin-api'; import { createSearchResultListItemExtension, @@ -51,13 +52,18 @@ export const techdocsPlugin = createPlugin({ deps: { configApi: configApiRef, discoveryApi: discoveryApiRef, + /** + * @deprecated fetchApi is enough + */ + identityApi: identityApiRef, fetchApi: fetchApiRef, }, - factory: ({ configApi, discoveryApi, fetchApi }) => + factory: ({ configApi, discoveryApi, fetchApi, identityApi = null }) => new TechDocsStorageClient({ configApi, discoveryApi, fetchApi, + identityApi, }), }), createApiFactory({ From b7db436113774855800cae92b424a827edaedece Mon Sep 17 00:00:00 2001 From: Alex Eftimie Date: Tue, 28 May 2024 18:49:35 +0200 Subject: [PATCH 08/10] update pr comments Signed-off-by: Alex Eftimie --- plugins/scaffolder/src/api.ts | 3 +++ plugins/techdocs/api-report.md | 3 ++- plugins/techdocs/src/client.ts | 13 ++++++++++--- plugins/techdocs/src/plugin.ts | 8 +------- 4 files changed, 16 insertions(+), 11 deletions(-) diff --git a/plugins/scaffolder/src/api.ts b/plugins/scaffolder/src/api.ts index 25f76c029a..6ef5fff588 100644 --- a/plugins/scaffolder/src/api.ts +++ b/plugins/scaffolder/src/api.ts @@ -245,8 +245,10 @@ export class ScaffolderClient implements ScaffolderApi { } }; + const ctrl = new AbortController(); fetchEventSource(url, { fetch: this.fetchApi.fetch, + signal: ctrl.signal, onmessage(e: EventSourceMessage) { if (e.event === 'log') { processEvent(e); @@ -254,6 +256,7 @@ export class ScaffolderClient implements ScaffolderApi { } else if (e.event === 'completion') { processEvent(e); subscriber.complete(); + ctrl.abort(); return; } processEvent(e); diff --git a/plugins/techdocs/api-report.md b/plugins/techdocs/api-report.md index b756bc0ab0..ebd6adcd3c 100644 --- a/plugins/techdocs/api-report.md +++ b/plugins/techdocs/api-report.md @@ -14,6 +14,7 @@ import { DiscoveryApi } from '@backstage/core-plugin-api'; import { Entity } from '@backstage/catalog-model'; import { EntityOwnerPickerProps } from '@backstage/plugin-catalog-react'; import { FetchApi } from '@backstage/core-plugin-api'; +import { IdentityApi } from '@backstage/core-plugin-api'; import { JSX as JSX_2 } from 'react'; import { PropsWithChildren } from 'react'; import { default as React_2 } from 'react'; @@ -465,7 +466,7 @@ export class TechDocsStorageClient implements TechDocsStorageApi_2 { configApi: Config; discoveryApi: DiscoveryApi; fetchApi: FetchApi; - identityApi?: any; + identityApi?: IdentityApi; }); // (undocumented) configApi: Config; diff --git a/plugins/techdocs/src/client.ts b/plugins/techdocs/src/client.ts index 06362825a1..d00be2a01f 100644 --- a/plugins/techdocs/src/client.ts +++ b/plugins/techdocs/src/client.ts @@ -16,7 +16,11 @@ import { CompoundEntityRef } from '@backstage/catalog-model'; import { Config } from '@backstage/config'; -import { DiscoveryApi, FetchApi } from '@backstage/core-plugin-api'; +import { + DiscoveryApi, + FetchApi, + IdentityApi, +} from '@backstage/core-plugin-api'; import { NotFoundError, ResponseError } from '@backstage/errors'; import { SyncResult, @@ -126,8 +130,8 @@ export class TechDocsStorageClient implements TechDocsStorageApi { configApi: Config; discoveryApi: DiscoveryApi; fetchApi: FetchApi; - // @deprecated Remove identityApiRef - identityApi?: any; + /** @deprecated identityApi is not needed any more */ + identityApi?: IdentityApi; }) { this.configApi = options.configApi; this.discoveryApi = options.discoveryApi; @@ -210,8 +214,10 @@ export class TechDocsStorageClient implements TechDocsStorageApi { const url = `${apiOrigin}/sync/${namespace}/${kind}/${name}`; return new Promise((resolve, reject) => { + const ctrl = new AbortController(); fetchEventSource(url, { fetch: this.fetchApi.fetch, + signal: ctrl.signal, onmessage(e: any) { if (e.event === 'log') { if (e.data) { @@ -228,6 +234,7 @@ export class TechDocsStorageClient implements TechDocsStorageApi { } }, onerror(err) { + ctrl.abort(); reject(err); }, }); diff --git a/plugins/techdocs/src/plugin.ts b/plugins/techdocs/src/plugin.ts index 9d4157ba9e..efa6bdfc1f 100644 --- a/plugins/techdocs/src/plugin.ts +++ b/plugins/techdocs/src/plugin.ts @@ -31,7 +31,6 @@ import { createRoutableExtension, discoveryApiRef, fetchApiRef, - identityApiRef, } from '@backstage/core-plugin-api'; import { createSearchResultListItemExtension, @@ -52,18 +51,13 @@ export const techdocsPlugin = createPlugin({ deps: { configApi: configApiRef, discoveryApi: discoveryApiRef, - /** - * @deprecated fetchApi is enough - */ - identityApi: identityApiRef, fetchApi: fetchApiRef, }, - factory: ({ configApi, discoveryApi, fetchApi, identityApi = null }) => + factory: ({ configApi, discoveryApi, fetchApi }) => new TechDocsStorageClient({ configApi, discoveryApi, fetchApi, - identityApi, }), }), createApiFactory({ From edc180129e23d62ccf219dd87c329216b2484a85 Mon Sep 17 00:00:00 2001 From: Alex Eftimie Date: Thu, 30 May 2024 02:23:35 +0200 Subject: [PATCH 09/10] Update .changeset/empty-avocados-tease.md Co-authored-by: Vincenzo Scamporlino Signed-off-by: Alex Eftimie --- .changeset/empty-avocados-tease.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.changeset/empty-avocados-tease.md b/.changeset/empty-avocados-tease.md index 08f191becd..f202af2b82 100644 --- a/.changeset/empty-avocados-tease.md +++ b/.changeset/empty-avocados-tease.md @@ -1,6 +1,6 @@ --- -'@backstage/plugin-scaffolder': minor -'@backstage/plugin-techdocs': minor +'@backstage/plugin-scaffolder': patch +'@backstage/plugin-techdocs': patch --- Internal updates to allow reusing Backstage's `fetchApi` implementation for event source requests. This allows you to for example, override the `Authorization` header. From 9d54ebfb3f2396499f301b4bfd80c4dde955dd4c Mon Sep 17 00:00:00 2001 From: Alex Eftimie Date: Thu, 30 May 2024 08:19:02 +0200 Subject: [PATCH 10/10] Fix unittests Signed-off-by: Alex Eftimie --- plugins/scaffolder/src/api.test.ts | 1 + plugins/techdocs/src/client.test.ts | 1 + 2 files changed, 2 insertions(+) diff --git a/plugins/scaffolder/src/api.test.ts b/plugins/scaffolder/src/api.test.ts index cf482e52f7..488169719d 100644 --- a/plugins/scaffolder/src/api.test.ts +++ b/plugins/scaffolder/src/api.test.ts @@ -118,6 +118,7 @@ describe('api', () => { fetch: fetchApi.fetch, onmessage: expect.any(Function), onerror: expect.any(Function), + signal: expect.any(AbortSignal), }, ); diff --git a/plugins/techdocs/src/client.test.ts b/plugins/techdocs/src/client.test.ts index 788d7e3fed..62d497d790 100644 --- a/plugins/techdocs/src/client.test.ts +++ b/plugins/techdocs/src/client.test.ts @@ -104,6 +104,7 @@ describe('TechDocsStorageClient', () => { fetch: fetchApi.fetch, onerror: expect.any(Function), onmessage: expect.any(Function), + signal: expect.any(AbortSignal), }, ); });