Skip to content

Commit dd2dc38

Browse files
authored
refactor: reorganize API logic and create class/hook for simplifying proxy logic (#124)
* wip: commit progress on UrlSync class/hook * refactor: consolidate emoji-testing logic * docs: update comments for clarity * refactor: rename helpers to renderHelpers * wip: finish initial implementation of UrlSync * chore: finish tests for UrlSync class * chore: add mock DiscoveryApi helper * chore: finish tests for useUrlSync * refactor: consolidate mock URL logic for useUrlSync * fix: update test helper to use API list * fix: remove unneeded imports * fix: get tests for all current code passing * fix: remove typo * fix: update useUrlSync to expose underlying api * refactor: increase data hiding for hook * fix: make useUrlSync tests less dependent on implementation details * refactor: remove reliance on baseUrl argument for fetch calls * refactor: split Backstage error type into separate file * refactor: clean up imports for api file * refactor: split main query options into separate file * consolidate how mock endpoints are defined * fix: remove base URL from auth calls * refactor: consolidate almost all auth logic into CoderAuthProvider * move api file into api directory * fix: revert prop that was changed for debugging * fix: revert prop definition * refactor: extract token-checking logic into middleware for server * refactor: move shared auth key to queryOptions file * docs: add reminder about arrow functions * fix: remove configApi from embedded class properties * fix: update query logic to remove any whitespace
1 parent fc86b8c commit dd2dc38

23 files changed

+748
-310
lines changed

plugins/backstage-plugin-coder/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
"@material-ui/icons": "^4.9.1",
4242
"@material-ui/lab": "4.0.0-alpha.61",
4343
"@tanstack/react-query": "4.36.1",
44+
"use-sync-external-store": "^1.2.1",
4445
"valibot": "^0.28.1"
4546
},
4647
"peerDependencies": {
Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
import { type UrlSyncSnapshot, UrlSync } from './UrlSync';
2+
import { type DiscoveryApi } from '@backstage/core-plugin-api';
3+
import {
4+
getMockConfigApi,
5+
getMockDiscoveryApi,
6+
mockBackstageAssetsEndpoint,
7+
mockBackstageProxyEndpoint,
8+
mockBackstageUrlRoot,
9+
} from '../testHelpers/mockBackstageData';
10+
11+
// Tests have to assume that DiscoveryApi and ConfigApi will always be in sync,
12+
// and can be trusted as being equivalent-ish ways of getting at the same source
13+
// of truth. If they're ever not, that's a bug with Backstage itself
14+
describe(`${UrlSync.name}`, () => {
15+
it('Has cached URLs ready to go when instantiated', () => {
16+
const urlSync = new UrlSync({
17+
apis: {
18+
configApi: getMockConfigApi(),
19+
discoveryApi: getMockDiscoveryApi(),
20+
},
21+
});
22+
23+
const cachedUrls = urlSync.getCachedUrls();
24+
expect(cachedUrls).toEqual<UrlSyncSnapshot>({
25+
baseUrl: mockBackstageUrlRoot,
26+
apiRoute: mockBackstageProxyEndpoint,
27+
assetsRoute: mockBackstageAssetsEndpoint,
28+
});
29+
});
30+
31+
it('Will update cached URLs if getApiEndpoint starts returning new values (for any reason)', async () => {
32+
let baseUrl = mockBackstageUrlRoot;
33+
const mockDiscoveryApi: DiscoveryApi = {
34+
getBaseUrl: async () => baseUrl,
35+
};
36+
37+
const urlSync = new UrlSync({
38+
apis: {
39+
configApi: getMockConfigApi(),
40+
discoveryApi: mockDiscoveryApi,
41+
},
42+
});
43+
44+
const initialSnapshot = urlSync.getCachedUrls();
45+
baseUrl = 'blah';
46+
47+
await urlSync.getApiEndpoint();
48+
const newSnapshot = urlSync.getCachedUrls();
49+
expect(initialSnapshot).not.toEqual(newSnapshot);
50+
51+
expect(newSnapshot).toEqual<UrlSyncSnapshot>({
52+
baseUrl: 'blah',
53+
apiRoute: 'blah/coder/api/v2',
54+
assetsRoute: 'blah/coder',
55+
});
56+
});
57+
58+
it('Lets external systems subscribe and unsubscribe to cached URL changes', async () => {
59+
let baseUrl = mockBackstageUrlRoot;
60+
const mockDiscoveryApi: DiscoveryApi = {
61+
getBaseUrl: async () => baseUrl,
62+
};
63+
64+
const urlSync = new UrlSync({
65+
apis: {
66+
configApi: getMockConfigApi(),
67+
discoveryApi: mockDiscoveryApi,
68+
},
69+
});
70+
71+
const onChange = jest.fn();
72+
urlSync.subscribe(onChange);
73+
74+
baseUrl = 'blah';
75+
await urlSync.getApiEndpoint();
76+
77+
expect(onChange).toHaveBeenCalledWith({
78+
baseUrl: 'blah',
79+
apiRoute: 'blah/coder/api/v2',
80+
assetsRoute: 'blah/coder',
81+
} satisfies UrlSyncSnapshot);
82+
83+
urlSync.unsubscribe(onChange);
84+
onChange.mockClear();
85+
baseUrl = mockBackstageUrlRoot;
86+
87+
await urlSync.getApiEndpoint();
88+
expect(onChange).not.toHaveBeenCalled();
89+
});
90+
});
Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
/**
2+
* @file This is basically a fancier version of Backstage's built-in
3+
* DiscoveryApi that is designed to work much better with React. Its hook
4+
* counterpart is useUrlSync.
5+
*
6+
* The class helps with synchronizing URLs between Backstage classes and React
7+
* UI components. It will:
8+
* 1. Make sure URLs are cached so that they can be accessed directly and
9+
* synchronously from the UI
10+
* 2. Make sure that there are mechanisms for binding value changes to React
11+
* state, so that if the URLs change over time, React components can
12+
* re-render correctly
13+
*
14+
* As of April 2024, there are two main built-in ways of getting URLs from
15+
* Backstage config values:
16+
* 1. ConfigApi (offers synchronous methods, but does not have direct access to
17+
* the proxy config - you have to stitch together the full path yourself)
18+
* 2. DiscoveryApi (has access to proxy config, but all methods are async)
19+
*
20+
* Both of these work fine inside event handlers and effects, but are never safe
21+
* to put directly inside render logic. They're not pure functions, so they
22+
* can't be used as derived values, and they don't go through React state, so
23+
* they're completely disconnected from React's render cycles.
24+
*/
25+
import {
26+
type DiscoveryApi,
27+
type ConfigApi,
28+
createApiRef,
29+
} from '@backstage/core-plugin-api';
30+
import {
31+
type Subscribable,
32+
type SubscriptionCallback,
33+
CODER_API_REF_ID_PREFIX,
34+
} from '../typesConstants';
35+
import { StateSnapshotManager } from '../utils/StateSnapshotManager';
36+
37+
// This is the value we tell people to use inside app-config.yaml
38+
export const CODER_PROXY_PREFIX = '/coder';
39+
40+
const BASE_URL_KEY_FOR_CONFIG_API = 'backend.baseUrl';
41+
const PROXY_URL_KEY_FOR_DISCOVERY_API = 'proxy';
42+
43+
type UrlPrefixes = Readonly<{
44+
proxyPrefix: string;
45+
apiRoutePrefix: string;
46+
assetsRoutePrefix: string;
47+
}>;
48+
49+
export const defaultUrlPrefixes = {
50+
proxyPrefix: `/api/proxy`,
51+
apiRoutePrefix: '/api/v2',
52+
assetsRoutePrefix: '', // Deliberately left as empty string
53+
} as const satisfies UrlPrefixes;
54+
55+
export type UrlSyncSnapshot = Readonly<{
56+
baseUrl: string;
57+
apiRoute: string;
58+
assetsRoute: string;
59+
}>;
60+
61+
type Subscriber = SubscriptionCallback<UrlSyncSnapshot>;
62+
63+
type ConstructorInputs = Readonly<{
64+
urlPrefixes?: Partial<UrlPrefixes>;
65+
apis: Readonly<{
66+
discoveryApi: DiscoveryApi;
67+
configApi: ConfigApi;
68+
}>;
69+
}>;
70+
71+
const proxyRouteReplacer = /\/api\/proxy.*?$/;
72+
73+
type UrlSyncApi = Subscribable<UrlSyncSnapshot> &
74+
Readonly<{
75+
getApiEndpoint: () => Promise<string>;
76+
getAssetsEndpoint: () => Promise<string>;
77+
getCachedUrls: () => UrlSyncSnapshot;
78+
}>;
79+
80+
export class UrlSync implements UrlSyncApi {
81+
private readonly discoveryApi: DiscoveryApi;
82+
private readonly urlCache: StateSnapshotManager<UrlSyncSnapshot>;
83+
private urlPrefixes: UrlPrefixes;
84+
85+
constructor(setup: ConstructorInputs) {
86+
const { apis, urlPrefixes = {} } = setup;
87+
const { discoveryApi, configApi } = apis;
88+
89+
this.discoveryApi = discoveryApi;
90+
this.urlPrefixes = { ...defaultUrlPrefixes, ...urlPrefixes };
91+
92+
const proxyRoot = this.getProxyRootFromConfigApi(configApi);
93+
this.urlCache = new StateSnapshotManager<UrlSyncSnapshot>({
94+
initialSnapshot: this.prepareNewSnapshot(proxyRoot),
95+
});
96+
}
97+
98+
// ConfigApi is literally only used because it offers a synchronous way to
99+
// get an initial URL to use from inside the constructor. Should not be used
100+
// beyond initial constructor call, so it's not being embedded in the class
101+
private getProxyRootFromConfigApi(configApi: ConfigApi): string {
102+
const baseUrl = configApi.getString(BASE_URL_KEY_FOR_CONFIG_API);
103+
return `${baseUrl}${this.urlPrefixes.proxyPrefix}`;
104+
}
105+
106+
private prepareNewSnapshot(newProxyUrl: string): UrlSyncSnapshot {
107+
const { assetsRoutePrefix, apiRoutePrefix } = this.urlPrefixes;
108+
109+
return {
110+
baseUrl: newProxyUrl.replace(proxyRouteReplacer, ''),
111+
assetsRoute: `${newProxyUrl}${CODER_PROXY_PREFIX}${assetsRoutePrefix}`,
112+
apiRoute: `${newProxyUrl}${CODER_PROXY_PREFIX}${apiRoutePrefix}`,
113+
};
114+
}
115+
116+
/* ***************************************************************************
117+
* All public functions should be defined as arrow functions to ensure they
118+
* can be passed around React without risk of losing their `this` context
119+
****************************************************************************/
120+
121+
getApiEndpoint = async (): Promise<string> => {
122+
const proxyRoot = await this.discoveryApi.getBaseUrl(
123+
PROXY_URL_KEY_FOR_DISCOVERY_API,
124+
);
125+
126+
const newSnapshot = this.prepareNewSnapshot(proxyRoot);
127+
this.urlCache.updateSnapshot(newSnapshot);
128+
return newSnapshot.apiRoute;
129+
};
130+
131+
getAssetsEndpoint = async (): Promise<string> => {
132+
const proxyRoot = await this.discoveryApi.getBaseUrl(
133+
PROXY_URL_KEY_FOR_DISCOVERY_API,
134+
);
135+
136+
const newSnapshot = this.prepareNewSnapshot(proxyRoot);
137+
this.urlCache.updateSnapshot(newSnapshot);
138+
return newSnapshot.assetsRoute;
139+
};
140+
141+
getCachedUrls = (): UrlSyncSnapshot => {
142+
return this.urlCache.getSnapshot();
143+
};
144+
145+
unsubscribe = (callback: Subscriber): void => {
146+
this.urlCache.unsubscribe(callback);
147+
};
148+
149+
subscribe = (callback: Subscriber): (() => void) => {
150+
this.urlCache.subscribe(callback);
151+
return () => this.unsubscribe(callback);
152+
};
153+
}
154+
155+
export const urlSyncApiRef = createApiRef<UrlSync>({
156+
id: `${CODER_API_REF_ID_PREFIX}.url-sync`,
157+
});

0 commit comments

Comments
 (0)