Merge pull request #8931 from backstage/rugvip/entityrr

catalog-react: deprecate catalogRouteRef and deduplicate entityRouteRef
This commit is contained in:
Patrik Oldsberg
2022-01-20 11:11:21 +01:00
committed by GitHub
23 changed files with 389 additions and 112 deletions
+4
View File
@@ -53,6 +53,7 @@ import {
CostInsightsPage,
CostInsightsProjectGrowthInstructionsPage,
} from '@backstage/plugin-cost-insights';
import { orgPlugin } from '@backstage/plugin-org';
import { ExplorePage, explorePlugin } from '@backstage/plugin-explore';
import { GcpProjectsPage } from '@backstage/plugin-gcp-projects';
import { GraphiQLPage } from '@backstage/plugin-graphiql';
@@ -125,6 +126,9 @@ const app = createApp({
bind(scaffolderPlugin.externalRoutes, {
registerComponent: catalogImportPlugin.routes.importPage,
});
bind(orgPlugin.externalRoutes, {
catalogIndex: catalogPlugin.routes.catalogIndex,
});
},
});
@@ -35,31 +35,9 @@ import {
createRoutableExtension,
analyticsApiRef,
} from '@backstage/core-plugin-api';
import { generateBoundRoutes, AppManager } from './AppManager';
import { AppManager } from './AppManager';
import { AppComponents, AppIcons } from './types';
describe('generateBoundRoutes', () => {
it('runs happy path', () => {
const external = { myRoute: createExternalRouteRef({ id: '1' }) };
const ref = createRouteRef({ id: 'ref-1' });
const result = generateBoundRoutes(({ bind }) => {
bind(external, { myRoute: ref });
});
expect(result.get(external.myRoute)).toBe(ref);
});
it('throws on unknown keys', () => {
const external = { myRoute: createExternalRouteRef({ id: '2' }) };
const ref = createRouteRef({ id: 'ref-2' });
expect(() =>
generateBoundRoutes(({ bind }) => {
bind(external, { someOtherRoute: ref } as any);
}),
).toThrow('Key someOtherRoute is not an existing external route');
});
});
describe('Integration Test', () => {
const noOpAnalyticsApi = createApiFactory(
analyticsApiRef,
@@ -545,8 +523,47 @@ describe('Integration Test', () => {
});
expect(errorLogs).toEqual([
expect.stringContaining(
'Parameter :thing is duplicated in path /test/:thing/some/:thing',
'The above error occurred in the <Provider> component',
),
]);
});
it('should throw an error when required external plugin routes are not bound', () => {
const app = new AppManager({
apis: [],
defaultApis: [],
themes: [
{
id: 'light',
title: 'Light Theme',
variant: 'light',
Provider: ({ children }) => <>{children}</>,
},
],
icons,
plugins: [],
components,
configLoader: async () => [],
});
const Provider = app.getProvider();
const Router = app.getRouter();
const { error: errorLogs } = withLogCollector(() => {
expect(() =>
render(
<Provider>
<Router>
<Routes>
<Route path="/test/:thing" element={<ExposedComponent />} />
</Routes>
</Router>
</Provider>,
),
).toThrow(
/^External route 'extRouteRef1' of the 'blob' plugin must be bound to a target route/,
);
});
expect(errorLogs).toEqual([
expect.stringContaining(
'The above error occurred in the <Provider> component',
),
+48 -59
View File
@@ -45,9 +45,6 @@ import {
IdentityApi,
identityApiRef,
BackstagePlugin,
RouteRef,
SubRouteRef,
ExternalRouteRef,
} from '@backstage/core-plugin-api';
import { ApiFactoryRegistry, ApiResolver } from '../apis/system';
import {
@@ -64,7 +61,10 @@ import {
} from '../routing/collectors';
import { RoutingProvider } from '../routing/RoutingProvider';
import { RouteTracker } from '../routing/RouteTracker';
import { validateRoutes } from '../routing/validation';
import {
validateRouteParameters,
validateRouteBindings,
} from '../routing/validation';
import { AppContextProvider } from './AppContext';
import { AppIdentityProxy } from '../apis/implementations/IdentityApi/AppIdentityProxy';
import {
@@ -72,13 +72,13 @@ import {
AppConfigLoader,
AppContext,
AppOptions,
AppRouteBinder,
BackstageApp,
SignInPageProps,
} from './types';
import { AppThemeProvider } from './AppThemeProvider';
import { defaultConfigLoader } from './defaultConfigLoader';
import { ApiRegistry } from '../apis/system/ApiRegistry';
import { resolveRouteBindings } from './resolveRouteBindings';
type CompatiblePlugin =
| BackstagePlugin<any, any>
@@ -86,35 +86,6 @@ type CompatiblePlugin =
output(): Array<{ type: 'feature-flag'; name: string }>;
});
export function generateBoundRoutes(bindRoutes: AppOptions['bindRoutes']) {
const result = new Map<ExternalRouteRef, RouteRef | SubRouteRef>();
if (bindRoutes) {
const bind: AppRouteBinder = (
externalRoutes,
targetRoutes: { [name: string]: RouteRef | SubRouteRef },
) => {
for (const [key, value] of Object.entries(targetRoutes)) {
const externalRoute = externalRoutes[key];
if (!externalRoute) {
throw new Error(`Key ${key} is not an existing external route`);
}
if (!value && !externalRoute.optional) {
throw new Error(
`External route ${key} is required but was undefined`,
);
}
if (value) {
result.set(externalRoute, value);
}
}
};
bindRoutes({ bind });
}
return result;
}
/**
* Get the app base path from the configured app baseUrl.
*
@@ -225,39 +196,57 @@ export class AppManager implements BackstageApp {
getProvider(): ComponentType<{}> {
const appContext = new AppContextImpl(this);
// We only validate routes once
let routesHaveBeenValidated = false;
const Provider = ({ children }: PropsWithChildren<{}>) => {
const appThemeApi = useMemo(
() => AppThemeSelector.createWithStorage(this.themes),
[],
);
const { routePaths, routeParents, routeObjects, featureFlags } =
useMemo(() => {
const result = traverseElementTree({
root: children,
discoverers: [childDiscoverer, routeElementDiscoverer],
collectors: {
routePaths: routePathCollector,
routeParents: routeParentCollector,
routeObjects: routeObjectCollector,
collectedPlugins: pluginCollector,
featureFlags: featureFlagCollector,
},
});
const {
routePaths,
routeParents,
routeObjects,
featureFlags,
routeBindings,
} = useMemo(() => {
const result = traverseElementTree({
root: children,
discoverers: [childDiscoverer, routeElementDiscoverer],
collectors: {
routePaths: routePathCollector,
routeParents: routeParentCollector,
routeObjects: routeObjectCollector,
collectedPlugins: pluginCollector,
featureFlags: featureFlagCollector,
},
});
validateRoutes(result.routePaths, result.routeParents);
// TODO(Rugvip): Restructure the public API so that we can get an immediate view of
// the app, rather than having to wait for the provider to render.
// For now we need to push the additional plugins we find during
// collection and then make sure we initialize things afterwards.
result.collectedPlugins.forEach(plugin => this.plugins.add(plugin));
this.verifyPlugins(this.plugins);
// TODO(Rugvip): Restructure the public API so that we can get an immediate view of
// the app, rather than having to wait for the provider to render.
// For now we need to push the additional plugins we find during
// collection and then make sure we initialize things afterwards.
result.collectedPlugins.forEach(plugin => this.plugins.add(plugin));
this.verifyPlugins(this.plugins);
// Initialize APIs once all plugins are available
this.getApiHolder();
return {
...result,
routeBindings: resolveRouteBindings(this.bindRoutes),
};
}, [children]);
// Initialize APIs once all plugins are available
this.getApiHolder();
return result;
}, [children]);
if (!routesHaveBeenValidated) {
routesHaveBeenValidated = true;
validateRouteParameters(routePaths, routeParents);
validateRouteBindings(
routeBindings,
this.plugins as Iterable<BackstagePlugin<any, any>>,
);
}
const loadedConfig = useConfigLoader(
this.configLoader,
@@ -318,7 +307,7 @@ export class AppManager implements BackstageApp {
routePaths={routePaths}
routeParents={routeParents}
routeObjects={routeObjects}
routeBindings={generateBoundRoutes(this.bindRoutes)}
routeBindings={routeBindings}
basePath={getBasePath(loadedConfig.api)}
>
{children}
@@ -0,0 +1,43 @@
/*
* Copyright 2020 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 {
createExternalRouteRef,
createRouteRef,
} from '@backstage/core-plugin-api';
import { resolveRouteBindings } from './resolveRouteBindings';
describe('resolveRouteBindings', () => {
it('runs happy path', () => {
const external = { myRoute: createExternalRouteRef({ id: '1' }) };
const ref = createRouteRef({ id: 'ref-1' });
const result = resolveRouteBindings(({ bind }) => {
bind(external, { myRoute: ref });
});
expect(result.get(external.myRoute)).toBe(ref);
});
it('throws on unknown keys', () => {
const external = { myRoute: createExternalRouteRef({ id: '2' }) };
const ref = createRouteRef({ id: 'ref-2' });
expect(() =>
resolveRouteBindings(({ bind }) => {
bind(external, { someOtherRoute: ref } as any);
}),
).toThrow('Key someOtherRoute is not an existing external route');
});
});
@@ -0,0 +1,51 @@
/*
* Copyright 2020 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 {
RouteRef,
SubRouteRef,
ExternalRouteRef,
} from '@backstage/core-plugin-api';
import { AppOptions, AppRouteBinder } from './types';
export function resolveRouteBindings(bindRoutes: AppOptions['bindRoutes']) {
const result = new Map<ExternalRouteRef, RouteRef | SubRouteRef>();
if (bindRoutes) {
const bind: AppRouteBinder = (
externalRoutes,
targetRoutes: { [name: string]: RouteRef | SubRouteRef },
) => {
for (const [key, value] of Object.entries(targetRoutes)) {
const externalRoute = externalRoutes[key];
if (!externalRoute) {
throw new Error(`Key ${key} is not an existing external route`);
}
if (!value && !externalRoute.optional) {
throw new Error(
`External route ${key} is required but was undefined`,
);
}
if (value) {
result.set(externalRoute, value);
}
}
};
bindRoutes({ bind });
}
return result;
}
@@ -39,7 +39,7 @@ import {
routeParentCollector,
routeObjectCollector,
} from './collectors';
import { validateRoutes } from './validation';
import { validateRouteParameters } from './validation';
import { RouteResolver } from './RouteResolver';
import { AnyRouteRef, RouteFunc } from './types';
import { AppContextProvider } from '../app/AppContext';
@@ -323,7 +323,7 @@ describe('discovery', () => {
},
});
expect(() => validateRoutes(routePaths, routeParents)).toThrow(
expect(() => validateRouteParameters(routePaths, routeParents)).toThrow(
'Parameter :id is duplicated in path /foo/:id/bar/:id',
);
});
@@ -14,9 +14,16 @@
* limitations under the License.
*/
import {
BackstagePlugin,
ExternalRouteRef,
RouteRef,
SubRouteRef,
} from '@backstage/core-plugin-api';
import { AnyRouteRef } from './types';
export function validateRoutes(
// Validates that there is no duplication of route parameter names
export function validateRouteParameters(
routePaths: Map<AnyRouteRef, string>,
routeParents: Map<AnyRouteRef, AnyRouteRef | undefined>,
) {
@@ -54,3 +61,30 @@ export function validateRoutes(
}
}
}
// Validates that all non-optional external routes have been bound
export function validateRouteBindings(
routeBindings: Map<ExternalRouteRef, RouteRef | SubRouteRef>,
plugins: Iterable<BackstagePlugin<{}, Record<string, ExternalRouteRef>>>,
) {
for (const plugin of plugins) {
if (!plugin.externalRoutes) {
continue;
}
for (const [name, externalRouteRef] of Object.entries(
plugin.externalRoutes,
)) {
if (externalRouteRef.optional) {
continue;
}
if (!routeBindings.has(externalRouteRef)) {
throw new Error(
`External route '${name}' of the '${plugin.getId()}' plugin must be bound to a target route. ` +
'See https://backstage.io/link?bind-routes for details.',
);
}
}
}
}
@@ -11,6 +11,7 @@ import {
catalogImportPlugin,
} from '@backstage/plugin-catalog-import';
import { ScaffolderPage, scaffolderPlugin } from '@backstage/plugin-scaffolder';
import { orgPlugin } from '@backstage/plugin-org';
import { SearchPage } from '@backstage/plugin-search';
import { TechRadarPage } from '@backstage/plugin-tech-radar';
import {
@@ -42,6 +43,9 @@ const app = createApp({
bind(scaffolderPlugin.externalRoutes, {
registerComponent: catalogImportPlugin.routes.importPage,
});
bind(orgPlugin.externalRoutes, {
catalogIndex: catalogPlugin.routes.catalogIndex,
});
},
});