From c962c0b57acfb321194541fac65872e8f6de4007 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Wed, 19 Apr 2023 13:49:26 +0200 Subject: [PATCH 01/11] deprecate LocalStoredShortcuts, add DefaultShortcutsApi MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw Signed-off-by: zcurrent <84637714+zcurrent@users.noreply.github.com> --- .changeset/six-mails-shout.md | 5 ++++ plugins/shortcuts/api-report.md | 5 +++- plugins/shortcuts/src/AddShortcut.test.tsx | 4 ++-- plugins/shortcuts/src/EditShortcut.test.tsx | 4 ++-- plugins/shortcuts/src/ShortcutItem.test.tsx | 4 ++-- plugins/shortcuts/src/Shortcuts.test.tsx | 7 ++---- ...ts.test.ts => DefaultShortcutsApi.test.ts} | 14 +++++------ ...redShortcuts.ts => DefaultShortcutsApi.ts} | 11 +++++++-- plugins/shortcuts/src/api/index.ts | 2 +- plugins/shortcuts/src/deprecated.ts | 23 +++++++++++++++++++ plugins/shortcuts/src/index.ts | 1 + plugins/shortcuts/src/plugin.ts | 4 ++-- plugins/shortcuts/src/setupTests.ts | 1 + 13 files changed, 61 insertions(+), 24 deletions(-) create mode 100644 .changeset/six-mails-shout.md rename plugins/shortcuts/src/api/{LocalStoredShortcuts.test.ts => DefaultShortcutsApi.test.ts} (85%) rename plugins/shortcuts/src/api/{LocalStoredShortcuts.ts => DefaultShortcutsApi.ts} (87%) create mode 100644 plugins/shortcuts/src/deprecated.ts diff --git a/.changeset/six-mails-shout.md b/.changeset/six-mails-shout.md new file mode 100644 index 0000000000..f803e0858e --- /dev/null +++ b/.changeset/six-mails-shout.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-shortcuts': patch +--- + +Marked `LocalStoredShortcuts` as deprecated, replacing it with `DefaultShortcutsApi` whose naming more clearly suggests that the shortcuts aren't necessarily stored locally (it depends on the storage implementation). diff --git a/plugins/shortcuts/api-report.md b/plugins/shortcuts/api-report.md index 0dd42f5344..471af42374 100644 --- a/plugins/shortcuts/api-report.md +++ b/plugins/shortcuts/api-report.md @@ -14,7 +14,7 @@ import { ShortcutApi as ShortcutApi_2 } from '@backstage/plugin-shortcuts'; import type { StorageApi } from '@backstage/core-plugin-api'; // @public -export class LocalStoredShortcuts implements ShortcutApi_2 { +export class DefaultShortcutsApi implements ShortcutApi_2 { constructor(storageApi: StorageApi); // (undocumented) add(shortcut: Omit): Promise; @@ -30,6 +30,9 @@ export class LocalStoredShortcuts implements ShortcutApi_2 { update(shortcut: Shortcut_2): Promise; } +// @public @deprecated (undocumented) +export const LocalStoredShortcuts: typeof DefaultShortcutsApi; + // @public (undocumented) export type Shortcut = { id: string; diff --git a/plugins/shortcuts/src/AddShortcut.test.tsx b/plugins/shortcuts/src/AddShortcut.test.tsx index a697e02e99..f8786e2b24 100644 --- a/plugins/shortcuts/src/AddShortcut.test.tsx +++ b/plugins/shortcuts/src/AddShortcut.test.tsx @@ -17,7 +17,7 @@ import React from 'react'; import { screen, fireEvent, waitFor } from '@testing-library/react'; import { AddShortcut } from './AddShortcut'; -import { LocalStoredShortcuts } from './api'; +import { DefaultShortcutsApi } from './api'; import { MockAnalyticsApi, MockStorageApi, @@ -28,7 +28,7 @@ import { TestApiProvider } from '@backstage/test-utils'; import { analyticsApiRef } from '@backstage/core-plugin-api'; describe('AddShortcut', () => { - const api = new LocalStoredShortcuts(MockStorageApi.create()); + const api = new DefaultShortcutsApi(MockStorageApi.create()); const props = { onClose: jest.fn(), diff --git a/plugins/shortcuts/src/EditShortcut.test.tsx b/plugins/shortcuts/src/EditShortcut.test.tsx index df2ed112ba..72a89c13b6 100644 --- a/plugins/shortcuts/src/EditShortcut.test.tsx +++ b/plugins/shortcuts/src/EditShortcut.test.tsx @@ -18,7 +18,7 @@ import React from 'react'; import { screen, fireEvent, waitFor } from '@testing-library/react'; import { EditShortcut } from './EditShortcut'; import { Shortcut } from './types'; -import { LocalStoredShortcuts } from './api'; +import { DefaultShortcutsApi } from './api'; import { MockAnalyticsApi, MockStorageApi, @@ -34,7 +34,7 @@ describe('EditShortcut', () => { url: '/some-url', title: 'some title', }; - const api = new LocalStoredShortcuts(MockStorageApi.create()); + const api = new DefaultShortcutsApi(MockStorageApi.create()); const props = { onClose: jest.fn(), diff --git a/plugins/shortcuts/src/ShortcutItem.test.tsx b/plugins/shortcuts/src/ShortcutItem.test.tsx index b98dd2dc0b..b78b72e9cb 100644 --- a/plugins/shortcuts/src/ShortcutItem.test.tsx +++ b/plugins/shortcuts/src/ShortcutItem.test.tsx @@ -18,7 +18,7 @@ import React from 'react'; import { screen, waitFor } from '@testing-library/react'; import { ShortcutItem } from './ShortcutItem'; import { Shortcut } from './types'; -import { LocalStoredShortcuts } from './api'; +import { DefaultShortcutsApi } from './api'; import { MockStorageApi, renderInTestApp } from '@backstage/test-utils'; import { SidebarOpenStateProvider } from '@backstage/core-components'; @@ -28,7 +28,7 @@ describe('ShortcutItem', () => { url: '/some-url', title: 'some title', }; - const api = new LocalStoredShortcuts(MockStorageApi.create()); + const api = new DefaultShortcutsApi(MockStorageApi.create()); it('displays the shortcut', async () => { await renderInTestApp( diff --git a/plugins/shortcuts/src/Shortcuts.test.tsx b/plugins/shortcuts/src/Shortcuts.test.tsx index 02b351d3a4..a1339083b6 100644 --- a/plugins/shortcuts/src/Shortcuts.test.tsx +++ b/plugins/shortcuts/src/Shortcuts.test.tsx @@ -22,7 +22,7 @@ import { } from '@backstage/test-utils'; import { screen, waitFor } from '@testing-library/react'; import { Shortcuts } from './Shortcuts'; -import { LocalStoredShortcuts, shortcutsApiRef } from './api'; +import { DefaultShortcutsApi, shortcutsApiRef } from './api'; import { SidebarOpenStateProvider } from '@backstage/core-components'; @@ -32,10 +32,7 @@ describe('Shortcuts', () => { {} }}> diff --git a/plugins/shortcuts/src/api/LocalStoredShortcuts.test.ts b/plugins/shortcuts/src/api/DefaultShortcutsApi.test.ts similarity index 85% rename from plugins/shortcuts/src/api/LocalStoredShortcuts.test.ts rename to plugins/shortcuts/src/api/DefaultShortcutsApi.test.ts index 86890fc75b..44bc99fb98 100644 --- a/plugins/shortcuts/src/api/LocalStoredShortcuts.test.ts +++ b/plugins/shortcuts/src/api/DefaultShortcutsApi.test.ts @@ -17,12 +17,12 @@ import { MockStorageApi } from '@backstage/test-utils'; import { pageTheme } from '@backstage/theme'; import { Shortcut } from '../types'; -import { LocalStoredShortcuts } from './LocalStoredShortcuts'; +import { DefaultShortcutsApi } from './DefaultShortcutsApi'; import { ShortcutApi } from './ShortcutApi'; -describe('LocalStoredShortcuts', () => { +describe('DefaultShortcutsApi', () => { it('should observe shortcuts', async () => { - const shortcutApi: ShortcutApi = new LocalStoredShortcuts( + const shortcutApi: ShortcutApi = new DefaultShortcutsApi( MockStorageApi.create(), ); const shortcut: Shortcut = { id: 'id', title: 'title', url: '/url' }; @@ -48,7 +48,7 @@ describe('LocalStoredShortcuts', () => { it('should add shortcuts with ids', async () => { const storageApi = MockStorageApi.create(); - const shortcutApi: ShortcutApi = new LocalStoredShortcuts(storageApi); + const shortcutApi: ShortcutApi = new DefaultShortcutsApi(storageApi); const shortcut: Omit = { title: 'title', url: '/url' }; const spy = jest.spyOn(storageApi, 'set'); @@ -61,7 +61,7 @@ describe('LocalStoredShortcuts', () => { it('should update shortcuts', async () => { const storageApi = MockStorageApi.create(); - const shortcutApi: ShortcutApi = new LocalStoredShortcuts(storageApi); + const shortcutApi: ShortcutApi = new DefaultShortcutsApi(storageApi); const shortcut: Shortcut = { id: 'someid', title: 'title', url: '/url' }; const spy = jest.spyOn(storageApi, 'set'); @@ -74,7 +74,7 @@ describe('LocalStoredShortcuts', () => { it('should remove shortcuts', async () => { const storageApi = MockStorageApi.create(); - const shortcutApi: ShortcutApi = new LocalStoredShortcuts(storageApi); + const shortcutApi: ShortcutApi = new DefaultShortcutsApi(storageApi); const shortcut: Shortcut = { id: 'someid', title: 'title', url: '/url' }; const spy = jest.spyOn(storageApi, 'set'); @@ -84,7 +84,7 @@ describe('LocalStoredShortcuts', () => { it('should get a color', () => { const storageApi = MockStorageApi.create(); - const shortcutApi: ShortcutApi = new LocalStoredShortcuts(storageApi); + const shortcutApi: ShortcutApi = new DefaultShortcutsApi(storageApi); expect(shortcutApi.getColor('/catalog')).toEqual(pageTheme.home.colors[0]); }); diff --git a/plugins/shortcuts/src/api/LocalStoredShortcuts.ts b/plugins/shortcuts/src/api/DefaultShortcutsApi.ts similarity index 87% rename from plugins/shortcuts/src/api/LocalStoredShortcuts.ts rename to plugins/shortcuts/src/api/DefaultShortcutsApi.ts index 44f7364f69..8e5fd99540 100644 --- a/plugins/shortcuts/src/api/LocalStoredShortcuts.ts +++ b/plugins/shortcuts/src/api/DefaultShortcutsApi.ts @@ -22,11 +22,18 @@ import type { Observable } from '@backstage/types'; import ObservableImpl from 'zen-observable'; /** - * Implementation of the ShortcutApi that uses the StorageApi to store shortcuts. + * Default implementation of the {@link ShortcutApi} that uses the + * {@link @backstage/core-plugin-api#StorageApi} to store shortcuts. + * + * @remarks + * + * Note that the storage API given is used directly, as in, this implementation + * does not itself dive into a sub-bucket. So you may want to provide a bucket + * that is scoped to the plugin (default: 'shortcuts'). * * @public */ -export class LocalStoredShortcuts implements ShortcutApi { +export class DefaultShortcutsApi implements ShortcutApi { private shortcuts: Shortcut[]; private readonly subscribers = new Set< ZenObservable.SubscriptionObserver diff --git a/plugins/shortcuts/src/api/index.ts b/plugins/shortcuts/src/api/index.ts index eb24bed1d3..ae975208a0 100644 --- a/plugins/shortcuts/src/api/index.ts +++ b/plugins/shortcuts/src/api/index.ts @@ -14,6 +14,6 @@ * limitations under the License. */ -export { LocalStoredShortcuts } from './LocalStoredShortcuts'; +export { DefaultShortcutsApi } from './DefaultShortcutsApi'; export { shortcutsApiRef } from './ShortcutApi'; export type { ShortcutApi } from './ShortcutApi'; diff --git a/plugins/shortcuts/src/deprecated.ts b/plugins/shortcuts/src/deprecated.ts new file mode 100644 index 0000000000..8d5a2463fd --- /dev/null +++ b/plugins/shortcuts/src/deprecated.ts @@ -0,0 +1,23 @@ +/* + * Copyright 2023 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. + */ + +import { DefaultShortcutsApi } from './api'; + +/** + * @public + * @deprecated Use {@link DefaultShortcutsApi} instead. + */ +export const LocalStoredShortcuts = DefaultShortcutsApi; diff --git a/plugins/shortcuts/src/index.ts b/plugins/shortcuts/src/index.ts index b47c94b7b1..1d35cc2721 100644 --- a/plugins/shortcuts/src/index.ts +++ b/plugins/shortcuts/src/index.ts @@ -22,5 +22,6 @@ export { shortcutsPlugin, Shortcuts } from './plugin'; export * from './api'; +export * from './deprecated'; export type { Shortcut } from './types'; export type { ShortcutsProps } from './Shortcuts'; diff --git a/plugins/shortcuts/src/plugin.ts b/plugins/shortcuts/src/plugin.ts index 2b0d0559e0..8288c4fb87 100644 --- a/plugins/shortcuts/src/plugin.ts +++ b/plugins/shortcuts/src/plugin.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { LocalStoredShortcuts, shortcutsApiRef } from './api'; +import { DefaultShortcutsApi, shortcutsApiRef } from './api'; import { createApiFactory, createComponentExtension, @@ -30,7 +30,7 @@ export const shortcutsPlugin = createPlugin({ api: shortcutsApiRef, deps: { storageApi: storageApiRef }, factory: ({ storageApi }) => - new LocalStoredShortcuts(storageApi.forBucket('shortcuts')), + new DefaultShortcutsApi(storageApi.forBucket('shortcuts')), }), ], }); diff --git a/plugins/shortcuts/src/setupTests.ts b/plugins/shortcuts/src/setupTests.ts index fc6dbd98f8..427556fe26 100644 --- a/plugins/shortcuts/src/setupTests.ts +++ b/plugins/shortcuts/src/setupTests.ts @@ -13,5 +13,6 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + import '@testing-library/jest-dom'; import 'cross-fetch/polyfill'; From 352bd28ca22139ac1a8cb51d656241ba247f6f7f Mon Sep 17 00:00:00 2001 From: Zach Hammer Date: Tue, 18 Apr 2023 16:21:43 -0400 Subject: [PATCH 02/11] Match implementation of other search result items for stack overflow Allow for empty result Signed-off-by: Zach Hammer Signed-off-by: zcurrent <84637714+zcurrent@users.noreply.github.com> --- plugins/stack-overflow/src/plugin.ts | 6 ++++- .../StackOverflowSearchResultListItem.tsx | 27 ++++++++++--------- 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/plugins/stack-overflow/src/plugin.ts b/plugins/stack-overflow/src/plugin.ts index a837846708..5be4c89cbe 100644 --- a/plugins/stack-overflow/src/plugin.ts +++ b/plugins/stack-overflow/src/plugin.ts @@ -23,6 +23,8 @@ import { import { createCardExtension } from '@backstage/plugin-home'; import { StackOverflowQuestionsContentProps } from './types'; import { stackOverflowApiRef, StackOverflowClient } from './api'; +import { SearchResultListItemExtensionProps } from '@backstage/plugin-search-react'; +import { StackOverflowSearchResultListItemProps } from './search/StackOverflowSearchResultListItem/StackOverflowSearchResultListItem'; /** * The Backstage plugin that holds stack overflow specific components @@ -45,7 +47,9 @@ export const stackOverflowPlugin = createPlugin({ * * @public */ -export const StackOverflowSearchResultListItem = stackOverflowPlugin.provide( +export const StackOverflowSearchResultListItem: ( + props: SearchResultListItemExtensionProps, +) => JSX.Element | null = stackOverflowPlugin.provide( createComponentExtension({ name: 'StackOverflowResultListItem', component: { diff --git a/plugins/stack-overflow/src/search/StackOverflowSearchResultListItem/StackOverflowSearchResultListItem.tsx b/plugins/stack-overflow/src/search/StackOverflowSearchResultListItem/StackOverflowSearchResultListItem.tsx index 9971b0e7e5..de015145e1 100644 --- a/plugins/stack-overflow/src/search/StackOverflowSearchResultListItem/StackOverflowSearchResultListItem.tsx +++ b/plugins/stack-overflow/src/search/StackOverflowSearchResultListItem/StackOverflowSearchResultListItem.tsx @@ -29,8 +29,8 @@ import { useAnalytics } from '@backstage/core-plugin-api'; import { ResultHighlight } from '@backstage/plugin-search-common'; import { HighlightedSearchResultText } from '@backstage/plugin-search-react'; -type StackOverflowSearchResultListItemProps = { - result: any; // TODO(emmaindal): type to StackOverflowDocument. +export type StackOverflowSearchResultListItemProps = { + result?: any; // TODO(emmaindal): type to StackOverflowDocument. icon?: React.ReactNode; rank?: number; highlight?: ResultHighlight; @@ -39,17 +39,20 @@ type StackOverflowSearchResultListItemProps = { export const StackOverflowSearchResultListItem = ( props: StackOverflowSearchResultListItemProps, ) => { - const { location, title, text, answers, tags } = props.result; - const { highlight } = props; + const { result, highlight } = props; const analytics = useAnalytics(); const handleClick = () => { - analytics.captureEvent('discover', title, { - attributes: { to: location }, + analytics.captureEvent('discover', result.title, { + attributes: { to: result.location }, value: props.rank, }); }; + if (!result) { + return null; + } + return ( <> @@ -58,7 +61,7 @@ export const StackOverflowSearchResultListItem = ( + {highlight?.fields?.title ? ( ) : ( - _unescape(title) + _unescape(result.title) )} } @@ -81,13 +84,13 @@ export const StackOverflowSearchResultListItem = ( /> ) : ( - `Author: ${text}` + `Author: ${result.text}` ) } /> - - {tags && - tags.map((tag: string) => ( + + {result.tags && + result.tags.map((tag: string) => ( ))} From 579665cd07c18124c5a5ab14aab888d87f71c1fb Mon Sep 17 00:00:00 2001 From: Zach Hammer Date: Tue, 18 Apr 2023 16:25:44 -0400 Subject: [PATCH 03/11] Add changeset Signed-off-by: Zach Hammer Signed-off-by: zcurrent <84637714+zcurrent@users.noreply.github.com> --- .changeset/five-tigers-whisper.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/five-tigers-whisper.md diff --git a/.changeset/five-tigers-whisper.md b/.changeset/five-tigers-whisper.md new file mode 100644 index 0000000000..aa9758c3b5 --- /dev/null +++ b/.changeset/five-tigers-whisper.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-stack-overflow': patch +--- + +StackOverflowSearchResultListItem can now accept an empty result prop so that it can be rendered in the suggested SearchResultListItem pattern. From 9aa10551c585704d3212ecbd62b66a0182c30dc9 Mon Sep 17 00:00:00 2001 From: Zach Hammer Date: Tue, 18 Apr 2023 17:26:38 -0400 Subject: [PATCH 04/11] Fix ae-forgotten-export // Warning: (ae-forgotten-export) The symbol "StackOverflowSearchResultListItemProps" needs to be exported by the entry point index.d.ts Signed-off-by: Zach Hammer Signed-off-by: zcurrent <84637714+zcurrent@users.noreply.github.com> --- plugins/stack-overflow/src/index.ts | 1 + .../StackOverflowSearchResultListItem.tsx | 5 +++++ .../src/search/StackOverflowSearchResultListItem/index.ts | 1 + 3 files changed, 7 insertions(+) diff --git a/plugins/stack-overflow/src/index.ts b/plugins/stack-overflow/src/index.ts index a796f563eb..11af38f7ab 100644 --- a/plugins/stack-overflow/src/index.ts +++ b/plugins/stack-overflow/src/index.ts @@ -30,5 +30,6 @@ export type { StackOverflowQuestionsContentProps, StackOverflowQuestionsRequestParams, } from './types'; +export type { StackOverflowSearchResultListItemProps } from './search/StackOverflowSearchResultListItem'; export { stackOverflowApiRef } from './api'; export type { StackOverflowApi } from './api'; diff --git a/plugins/stack-overflow/src/search/StackOverflowSearchResultListItem/StackOverflowSearchResultListItem.tsx b/plugins/stack-overflow/src/search/StackOverflowSearchResultListItem/StackOverflowSearchResultListItem.tsx index de015145e1..245666deeb 100644 --- a/plugins/stack-overflow/src/search/StackOverflowSearchResultListItem/StackOverflowSearchResultListItem.tsx +++ b/plugins/stack-overflow/src/search/StackOverflowSearchResultListItem/StackOverflowSearchResultListItem.tsx @@ -29,6 +29,11 @@ import { useAnalytics } from '@backstage/core-plugin-api'; import { ResultHighlight } from '@backstage/plugin-search-common'; import { HighlightedSearchResultText } from '@backstage/plugin-search-react'; +/** + * Props for {@link StackOverflowSearchResultListItem} + * + * @public + */ export type StackOverflowSearchResultListItemProps = { result?: any; // TODO(emmaindal): type to StackOverflowDocument. icon?: React.ReactNode; diff --git a/plugins/stack-overflow/src/search/StackOverflowSearchResultListItem/index.ts b/plugins/stack-overflow/src/search/StackOverflowSearchResultListItem/index.ts index 499f7273aa..7aeec9e55a 100644 --- a/plugins/stack-overflow/src/search/StackOverflowSearchResultListItem/index.ts +++ b/plugins/stack-overflow/src/search/StackOverflowSearchResultListItem/index.ts @@ -15,3 +15,4 @@ */ export { StackOverflowSearchResultListItem } from './StackOverflowSearchResultListItem'; +export type { StackOverflowSearchResultListItemProps } from './StackOverflowSearchResultListItem'; From 278450d5be1246578ef51c3db2a8002972b2f55b Mon Sep 17 00:00:00 2001 From: Zach Hammer Date: Wed, 19 Apr 2023 09:10:23 -0400 Subject: [PATCH 05/11] Build api report Signed-off-by: Zach Hammer Signed-off-by: zcurrent <84637714+zcurrent@users.noreply.github.com> --- plugins/stack-overflow/api-report.md | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/plugins/stack-overflow/api-report.md b/plugins/stack-overflow/api-report.md index 598bd06a04..b006f263ed 100644 --- a/plugins/stack-overflow/api-report.md +++ b/plugins/stack-overflow/api-report.md @@ -8,8 +8,9 @@ import { ApiRef } from '@backstage/core-plugin-api'; import { BackstagePlugin } from '@backstage/core-plugin-api'; import { CardExtensionProps } from '@backstage/plugin-home'; -import { ReactNode } from 'react'; +import { default as React_2 } from 'react'; import { ResultHighlight } from '@backstage/plugin-search-common'; +import { SearchResultListItemExtensionProps } from '@backstage/plugin-search-react'; // @public export const HomePageStackOverflowQuestions: ( @@ -53,10 +54,15 @@ export type StackOverflowQuestionsRequestParams = { }; // @public -export const StackOverflowSearchResultListItem: (props: { - result: any; - icon?: ReactNode; - rank?: number | undefined; - highlight?: ResultHighlight | undefined; -}) => JSX.Element; +export const StackOverflowSearchResultListItem: ( + props: SearchResultListItemExtensionProps, +) => JSX.Element | null; + +// @public +export type StackOverflowSearchResultListItemProps = { + result?: any; + icon?: React_2.ReactNode; + rank?: number; + highlight?: ResultHighlight; +}; ``` From 6a6f70ea1703225f4e516dd27f7e49aa09691657 Mon Sep 17 00:00:00 2001 From: Brian Forbis Date: Tue, 18 Apr 2023 13:25:16 -0400 Subject: [PATCH 06/11] Default knex connection.application_name Signed-off-by: Brian Forbis Signed-off-by: zcurrent <84637714+zcurrent@users.noreply.github.com> --- packages/backend-common/src/database/DatabaseManager.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/backend-common/src/database/DatabaseManager.ts b/packages/backend-common/src/database/DatabaseManager.ts index 02f2c6b172..036cabde9f 100644 --- a/packages/backend-common/src/database/DatabaseManager.ts +++ b/packages/backend-common/src/database/DatabaseManager.ts @@ -268,6 +268,12 @@ export class DatabaseManager { client, ); + if (client === 'pg') { + ( + connection as Knex.PgConnectionConfig + ).application_name ||= `backstage_plugin_${pluginId}`; + } + return { // include base connection if client type has not been overridden ...(overridden ? {} : baseConnection), From cf352863f9652db7184f93d7e30cfacc0fb39d1c Mon Sep 17 00:00:00 2001 From: Brian Forbis Date: Tue, 18 Apr 2023 13:35:42 -0400 Subject: [PATCH 07/11] add changeset Signed-off-by: Brian Forbis Signed-off-by: zcurrent <84637714+zcurrent@users.noreply.github.com> --- .changeset/eighty-olives-live.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/eighty-olives-live.md diff --git a/.changeset/eighty-olives-live.md b/.changeset/eighty-olives-live.md new file mode 100644 index 0000000000..88dd768103 --- /dev/null +++ b/.changeset/eighty-olives-live.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-common': minor +--- + +Defaults the connection.application_name of the database connections created for each plugin. From 518d67e7b9f7b57bba3aa1c1171a429ce9c63669 Mon Sep 17 00:00:00 2001 From: bforbis Date: Wed, 19 Apr 2023 10:15:06 -0400 Subject: [PATCH 08/11] Apply suggestions from code review Co-authored-by: Johan Haals Signed-off-by: bforbis Signed-off-by: zcurrent <84637714+zcurrent@users.noreply.github.com> --- .changeset/eighty-olives-live.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.changeset/eighty-olives-live.md b/.changeset/eighty-olives-live.md index 88dd768103..16898701f4 100644 --- a/.changeset/eighty-olives-live.md +++ b/.changeset/eighty-olives-live.md @@ -1,5 +1,5 @@ --- -'@backstage/backend-common': minor +'@backstage/backend-common': patch --- -Defaults the connection.application_name of the database connections created for each plugin. +Updated the `DatabaseManager` to include the plugin id in the Postgres application name of the database connections created for each plugin. From 986d8ce07fc85c5fc74285d512c296984ce0fcf9 Mon Sep 17 00:00:00 2001 From: Brian Forbis Date: Wed, 19 Apr 2023 12:01:26 -0400 Subject: [PATCH 09/11] add tests Signed-off-by: Brian Forbis Signed-off-by: zcurrent <84637714+zcurrent@users.noreply.github.com> --- .../src/database/DatabaseManager.test.ts | 79 +++++++++++++++++++ .../src/database/DatabaseManager.ts | 2 +- 2 files changed, 80 insertions(+), 1 deletion(-) diff --git a/packages/backend-common/src/database/DatabaseManager.test.ts b/packages/backend-common/src/database/DatabaseManager.test.ts index 336e5d499c..69386d77c5 100644 --- a/packages/backend-common/src/database/DatabaseManager.test.ts +++ b/packages/backend-common/src/database/DatabaseManager.test.ts @@ -165,6 +165,7 @@ describe('DatabaseManager', () => { user: 'foo', password: 'bar', port: '5432', + application_name: 'backstage_plugin_pluginwithoutconfig', }, }); @@ -741,5 +742,83 @@ describe('DatabaseManager', () => { expect(baseConfig.data.role).toEqual('backstage-plugin'); }); + + it('Defaults the application_name for postgres clients', async () => { + const testManager = DatabaseManager.fromConfig( + new ConfigReader({ + backend: { + database: { + client: 'pg', + connection: {}, + }, + }, + }), + ); + + await testManager.forPlugin('testplugin').getClient(); + const mockCalls = mocked(createDatabaseClient).mock.calls.splice(-1); + const [baseConfig, _] = mockCalls[0]; + expect(baseConfig.get()).toMatchObject({ + connection: { + application_name: 'backstage_plugin_testplugin', + }, + }); + }); + + it('Allows manually setting the application_name for postgres clients', async () => { + const testManager = DatabaseManager.fromConfig( + new ConfigReader({ + backend: { + database: { + client: 'pg', + connection: { + application_name: 'backstage_custom_app_name', + }, + }, + }, + }), + ); + + await testManager.forPlugin('testplugin').getClient(); + const mockCalls = mocked(createDatabaseClient).mock.calls.splice(-1); + const [baseConfig, overrides] = mockCalls[0]; + expect(baseConfig.get().connection.application_name).toBe( + 'backstage_custom_app_name', + ); + expect(overrides.connection.application_name).toBeUndefined(); + }); + + it('Allows manually setting the application_name for individual plugin client', async () => { + const testManager = DatabaseManager.fromConfig( + new ConfigReader({ + backend: { + database: { + client: 'pg', + connection: { + host: 'localhost', + user: 'foo', + password: 'bar', + database: 'foodb', + application_name: 'backstage_custom_app_name', + }, + plugin: { + overrideplugin: { + connection: { + application_name: 'custom_plugin', + }, + }, + }, + }, + }, + }), + ); + + await testManager.forPlugin('overrideplugin').getClient(); + const mockCalls = mocked(createDatabaseClient).mock.calls.splice(-1); + const [baseConfig, _] = mockCalls[0]; + expect(baseConfig.get().connection.application_name).toBe( + 'custom_plugin', + ); + }); }); }); diff --git a/packages/backend-common/src/database/DatabaseManager.ts b/packages/backend-common/src/database/DatabaseManager.ts index 036cabde9f..db5d36afad 100644 --- a/packages/backend-common/src/database/DatabaseManager.ts +++ b/packages/backend-common/src/database/DatabaseManager.ts @@ -270,7 +270,7 @@ export class DatabaseManager { if (client === 'pg') { ( - connection as Knex.PgConnectionConfig + baseConnection as Knex.PgConnectionConfig ).application_name ||= `backstage_plugin_${pluginId}`; } From 76c5acd67e2c4b6dbf976dd1d396343764f74b65 Mon Sep 17 00:00:00 2001 From: Guillermo Manzo Date: Thu, 20 Apr 2023 09:55:58 -0700 Subject: [PATCH 10/11] Update ADOPTERS.md Signed-off-by: Guillermo Manzo Signed-off-by: zcurrent <84637714+zcurrent@users.noreply.github.com> --- ADOPTERS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ADOPTERS.md b/ADOPTERS.md index 24c889f518..d6a86316a9 100644 --- a/ADOPTERS.md +++ b/ADOPTERS.md @@ -21,7 +21,7 @@ _You can do this by using the [Adopter form](https://info.backstage.spotify.com/ | [Fiverr](https://www.fiverr.com) | [@nirga](https://github.com/nirga) | Unifying separate tools that developers are using today (i.e. monitoring, dead letter queues management, etc.) into a single platform. | | [Zalando SE](https://www.zalando.de) | [@leviferreira](https://github.com/leviferreira) | Building V2 of the Internal Development Portal. | | [LegalZoom](https://legalzoom.com) | [@backjo](https://github.com/backjo) | Developer portal - hub for all engineering projects and metadata. | -| [Expedia Group](https://www.expediagroup.com) | [Guillermo Manzo](mailto:gmanzo@expediagroup.com), [Sheena Sharma](mailto:shesharma@expediagroup.com), [Alekhya Karuturi](mailto:akaruturi@expediagroup.com) | EG Developer Front Door | +| [Expedia Group](https://www.expediagroup.com) | [Guillermo Manzo](mailto:gmanzo@expediagroup.com) | EG Developer Portal | | [Paddle.com](https://paddle.com) | [Ioannis Georgoulas](https://github.com/geototti21) | Developer portal (Tech Docs, Service Catalog, Internal Tooling), we use vanilla Backstage FE and custom BE implementation in Go | | [Acast.com](https://acast.com) | [Olle Lundberg](https://github.com/lndbrg) | Developer portal with tech docs, service catalog and a bunch of other internal tooling | | [Lunar](https://lunar.app) | [Bjørn Hald Sørensen](https://github.com/crevil) | Internal developer portal for service overview and insights, API documentation, technical guides, onboarding guides and RFC's. | From 78bb674a71334bc6c26bcfa4acd8caf7d9d6f785 Mon Sep 17 00:00:00 2001 From: zcurrent <84637714+zcurrent@users.noreply.github.com> Date: Thu, 20 Apr 2023 22:01:53 +0000 Subject: [PATCH 11/11] - Updated queryWithPaging to make one request per second to avoid concurrency issues.\n- Created changeset Signed-off-by: zcurrent Signed-off-by: zcurrent <84637714+zcurrent@users.noreply.github.com> --- .changeset/olive-turkeys-switch.md | 5 +++++ plugins/catalog-backend-module-github/src/lib/github.ts | 2 ++ 2 files changed, 7 insertions(+) create mode 100644 .changeset/olive-turkeys-switch.md diff --git a/.changeset/olive-turkeys-switch.md b/.changeset/olive-turkeys-switch.md new file mode 100644 index 0000000000..d208f3b197 --- /dev/null +++ b/.changeset/olive-turkeys-switch.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog-backend-module-github': patch +--- + +Fixed bug in queryWithPaging that caused secondary rate limit errors in GitHub with organizations having more than 1000 repositories. This change makes one request per second to avoid concurrency issues. diff --git a/plugins/catalog-backend-module-github/src/lib/github.ts b/plugins/catalog-backend-module-github/src/lib/github.ts index 39501e89bc..d0b629092a 100644 --- a/plugins/catalog-backend-module-github/src/lib/github.ts +++ b/plugins/catalog-backend-module-github/src/lib/github.ts @@ -545,6 +545,7 @@ export async function queryWithPaging< variables: Variables, ): Promise { const result: OutputType[] = []; + const sleep = (ms: number) => new Promise(r => setTimeout(r, ms)); let cursor: string | undefined = undefined; for (let j = 0; j < 1000 /* just for sanity */; ++j) { @@ -573,6 +574,7 @@ export async function queryWithPaging< if (!conn.pageInfo.hasNextPage) { break; } else { + await sleep(1000); cursor = conn.pageInfo.endCursor; } }