8000 chore(Coder plugin): update all Backstage code to use preview SDK (#131) · coder/backstage-plugins@c245950 · GitHub
[go: up one dir, main page]
Skip to content

Commit c245950

Browse files
ParkreinerParkreiner
authored
chore(Coder plugin): update all Backstage code to use preview SDK (#131)
* chore: add vendored version of experimental Coder SDK * chore: update CoderClient class to use new SDK * chore: delete mock SDK * fix: improve data hiding for CoderSdk * docs: update typo * wip: commit progress on updating Coder client * wip: commit more progress on updating types * chore: remove valibot type definitions from global constants file * chore: rename mocks file * fix: update type mismatches * wip: commit more update progress * wip: commit progress on updating client/SDK integration * fix: get all tests passing for CoderClient * fix: update UrlSync updates * fix: get all tests passing * chore: update all mock data to use Coder core entity mocks * fix: add extra helpers to useCoderSdk * fix: add additional properties to hide from SDK * fix: shrink down the API of useCoderSdk * update method name for clarity * chore: removal vestigal endpoint properties
1 parent 06d24da commit c245950

26 files changed

+487
-309
lines changed

‎plugins/backstage-plugin-coder/src/api/CoderClient.test.ts

Lines changed: 6 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,4 @@
1-
import {
2-
CODER_AUTH_HEADER_KEY,
3-
CoderClient,
4-
disabledClientError,
5-
} from './CoderClient';
1+
import { CODER_AUTH_HEADER_KEY, CoderClient } from './CoderClient';
62
import type { IdentityApi } from '@backstage/core-plugin-api';
73
import { UrlSync } from './UrlSync';
84
import { rest } from 'msw';
@@ -12,8 +8,8 @@ import { delay } from '../utils/time';
128
import {
139
mockWorkspacesList,
1410
mockWorkspacesListForRepoSearch,
15-
} from '../testHelpers/mockCoderAppData';
16-
import type { Workspace, WorkspacesResponse } from '../typesConstants';
11+
} from '../testHelpers/mockCoderPluginData';
12+
import type { Workspace, WorkspacesResponse } from './vendoredSdk';
1713
import {
1814
getMockConfigApi,
1915
getMockDiscoveryApi,
@@ -100,50 +96,6 @@ describe(`${CoderClient.name}`, () => {
10096
});
10197
});
10298

103-
describe('cleanupClient functionality', () => {
104-
it('Will prevent any new SDK requests from going through', async () => {
105-
const client = new CoderClient({ apis: getConstructorApis() });
106-
client.cleanupClient();
107-
108-
// Request should fail, even though token is valid
109-
await expect(() => {
110-
return client.syncToken(mockCoderAuthToken);
111-
}).rejects.toThrow(disabledClientError);
112-
113-
await expect(() => {
114-
return client.sdk.getWorkspaces({
115-
q: 'owner:me',
116-
limit: 0,
117-
});
118-
}).rejects.toThrow(disabledClientError);
119-
});
120-
121-
it('Will abort any pending requests', async () => {
122-
const client = new CoderClient({
123-
initialToken: mockCoderAuthToken,
124-
apis: getConstructorApis(),
125-
});
126-
127-
// Sanity check to ensure that request can still go through normally
128-
const workspacesPromise1 = client.sdk.getWorkspaces({
129-
q: 'owner:me',
130-
limit: 0,
131-
});
132-
133-
await expect(workspacesPromise1).resolves.toEqual<WorkspacesResponse>({
134-
workspaces: mockWorkspacesList,
135-
count: mockWorkspacesList.length,
136-
});
137-
138-
const workspacesPromise2 = client.sdk.getWorkspaces({
139-
q: 'owner:me',
140-
limit: 0,
141-
});
142-
client.cleanupClient();
143-
await expect(() => workspacesPromise2).rejects.toThrow();
144-
});
145-
});
146-
14799
// Eventually the Coder SDK is going to get too big to test every single
148100
// function. Focus tests on the functionality specifically being patched in
149101
// for Backstage
@@ -180,10 +132,10 @@ describe(`${CoderClient.name}`, () => {
180132
});
181133

182134
const { urlSync } = apis;
183-
const apiEndpoint = await urlSync.getApiEndpoint();
135+
const assetsEndpoint = await urlSync.getAssetsEndpoint();
184136

185-
const allWorkspacesAreRemapped = !workspaces.some(ws =>
186-
ws.template_icon.startsWith(apiEndpoint),
137+
const allWorkspacesAreRemapped = workspaces.every(ws =>
138+
ws.template_icon.startsWith(assetsEndpoint),
187139
);
188140

189141
expect(allWorkspacesAreRemapped).toBe(true);

‎plugins/backstage-plugin-coder/src/api/CoderClient.ts

Lines changed: 32 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,19 @@
1-
import globalAxios, {
1+
import {
22
AxiosError,
3-
type AxiosInstance,
43
type InternalAxiosRequestConfig as RequestConfig,
54
} from 'axios';
65
import { type IdentityApi, createApiRef } from '@backstage/core-plugin-api';
7-
import {
8-
type Workspace,
9-
CODER_API_REF_ID_PREFIX,
10-
WorkspacesRequest,
11-
WorkspacesResponse,
12-
User,
13-
} from '../typesConstants';
6+
import { CODER_API_REF_ID_PREFIX } from '../typesConstants';
147
import type { UrlSync } from './UrlSync';
158
import type { CoderWorkspacesConfig } from '../hooks/useCoderWorkspacesConfig';
16-
import { CoderSdk } from './MockCoderSdk';
9+
import {
10+
type CoderSdk,
11+
type User,
12+
type Workspace,
13+
type WorkspacesRequest,
14+
type WorkspacesResponse,
15+
makeCoderSdk,
16+
} from './vendoredSdk';
1717

1818
export const CODER_AUTH_HEADER_KEY = 'Coder-Session-Token';
1919
const DEFAULT_REQUEST_TIMEOUT_MS = 20_000;
@@ -39,11 +39,6 @@ type CoderClientApi = Readonly<{
3939
* Return value indicates whether the token is valid.
4040
*/
4141
syncToken: (newToken: string) => Promise<boolean>;
42-
43-
/**
44-
* Cleans up a client instance, removing its links to all external systems.
45-
*/
46-
cleanupClient: () => void;
4742
}>;
4843

4944
const sharedCleanupAbortReason = new DOMException(
@@ -59,19 +54,30 @@ export const disabledClientError = new Error(
5954
);
6055

6156
type ConstructorInputs = Readonly<{
57+
/**
58+
* initialToken is strictly for testing, and is basically limited to making it
59+
* easier to test API logic.
60+
*
61+
* If trying to test UI logic that depends on CoderClient, it's probably
62+
* better to interact with CoderClient indirectly through the auth components,
63+
* so that React state is aware of everything.
64+
*/
6265
initialToken?: string;
63-
requestTimeoutMs?: number;
6466

67+
requestTimeoutMs?: number;
6568
apis: Readonly<{
6669
urlSync: UrlSync;
6770
identityApi: IdentityApi;
6871
}>;
6972
}>;
7073

74+
type RequestInterceptor = (
75+
config: RequestConfig,
76+
) => RequestConfig | Promise<RequestConfig>;
77+
7178
export class CoderClient implements CoderClientApi {
7279
private readonly urlSync: UrlSync;
7380
private readonly identityApi: IdentityApi;
74-
private readonly axios: AxiosInstance;
7581

7682
private readonly requestTimeoutMs: number;
7783
private readonly cleanupController: AbortController;
@@ -82,33 +88,28 @@ export class CoderClient implements CoderClientApi {
8288

8389
constructor(inputs: ConstructorInputs) {
8490
const {
85-
apis,
8691
initialToken,
92+
2851 apis: { urlSync, identityApi },
8793
requestTimeoutMs = DEFAULT_REQUEST_TIMEOUT_MS,
8894
} = inputs;
89-
const { urlSync, identityApi } = apis;
9095

9196
this.urlSync = urlSync;
9297
this.identityApi = identityApi;
93-
this.axios = globalAxios.create();
94-
9598
this.loadedSessionToken = initialToken;
9699
this.requestTimeoutMs = requestTimeoutMs;
97-
98100
this.cleanupController = new AbortController();
99101
this.trackedEjectionIds = new Set();
100102

101-
this.sdk = this.getBackstageCoderSdk(this.axios);
103+
this.sdk = this.createBackstageCoderSdk();
102104
this.addBaseRequestInterceptors();
103105
}
104106

105107
private addRequestInterceptor(
106-
requestInterceptor: (
107-
config: RequestConfig,
108-
) => RequestConfig | Promise<RequestConfig>,
108+
requestInterceptor: RequestInterceptor,
109109
errorInterceptor?: (error: unknown) => unknown,
110110
): number {
111-
const ejectionId = this.axios.interceptors.request.use(
111+
const axios = this.sdk.getAxiosInstance();
112+
const ejectionId = axios.interceptors.request.use(
112113
requestInterceptor,
113114
errorInterceptor,
114115
);
@@ -120,7 +121,8 @@ export class CoderClient implements CoderClientApi {
120121
private removeRequestInterceptorById(ejectionId: number): boolean {
121122
// Even if we somehow pass in an ID that hasn't been associated with the
122123
// Axios instance, that's a noop. No harm in calling method no matter what
123-
this.axios.interceptors.request.eject(ejectionId);
124+
const axios = this.sdk.getAxiosInstance();
125+
axios.interceptors.request.eject(ejectionId);
124126

125127
if (!this.trackedEjectionIds.has(ejectionId)) {
126128
return false;
@@ -179,10 +181,8 @@ export class CoderClient implements CoderClientApi {
179181
this.addRequestInterceptor(baseRequestInterceptor, baseErrorInterceptor);
180182
}
181183

182-
private getBackstageCoderSdk(
183-
axiosInstance: AxiosInstance,
184-
): BackstageCoderSdk {
185-
const baseSdk = new CoderSdk(axiosInstance);
184+
private createBackstageCoderSdk(): BackstageCoderSdk {
185+
const baseSdk = makeCoderSdk();
186186

187187
const getWorkspaces: (typeof baseSdk)['getWorkspaces'] = async request => {
188188
const workspacesRes = await baseSdk.getWorkspaces(request);
@@ -335,23 +335,6 @@ export class CoderClient implements CoderClientApi {
335335
this.removeRequestInterceptorById(validationId);
336336
}
337337
};
338-
339-
cleanupClient = (): void => {
340-
this.trackedEjectionIds.forEach(id => {
341-
this.axios.interceptors.request.eject(id);
342-
});
343-
344-
this.trackedEjectionIds.clear();
345-
this.cleanupController.abort(sharedCleanupAbortReason);
346-
this.loadedSessionToken = undefined;
347-
348-
// Not using this.addRequestInterceptor, because we don't want to track this
349-
// interceptor at all. It should never be ejected once the client has been
350-
// disabled
351-
this.axios.interceptors.request.use(() => {
352-
throw disabledClientError;
353-
});
354-
};
355338
}
356339

357340
function appendParamToQuery(

‎plugins/backstage-plugin-coder/src/api/MockCoderSdk.ts

Lines changed: 0 additions & 48 deletions
This file was deleted.

‎plugins/backstage-plugin-coder/src/api/UrlSync.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ import {
44
getMockConfigApi,
55
getMockDiscoveryApi,
66
mockBackstageAssetsEndpoint,
7-
mockBackstageApiEndpoint,
87
mockBackstageUrlRoot,
8+
mockBackstageApiEndpointWithoutSdkPath,
99
} from '../testHelpers/mockBackstageData';
1010

1111
// Tests have to assume that DiscoveryApi and ConfigApi will always be in sync,
@@ -23,7 +23,7 @@ describe(`${UrlSync.name}`, () => {
2323
const cachedUrls = urlSync.getCachedUrls();
2424
expect(cachedUrls).toEqual<UrlSyncSnapshot>({
2525
baseUrl: mockBackstageUrlRoot,
26-
apiRoute: mockBackstageApiEndpoint,
26+
apiRoute: mockBackstageApiEndpointWithoutSdkPath,
2727
assetsRoute: mockBackstageAssetsEndpoint,
2828
});
2929
});
@@ -50,7 +50,7 @@ describe(`${UrlSync.name}`, () => {
5050

5151
expect(newSnapshot).toEqual<UrlSyncSnapshot>({
5252
baseUrl: 'blah',
53-
apiRoute: 'blah/coder/api/v2',
53+
apiRoute: 'blah/coder',
5454
assetsRoute: 'blah/coder',
5555
});
5656
});
@@ -76,7 +76,7 @@ describe(`${UrlSync.name}`, () => {
7676

7777
expect(onChange).toHaveBeenCalledWith({
7878
baseUrl: 'blah',
79-
apiRoute: 'blah/coder/api/v2',
79+
apiRoute: 'blah/coder',
8080
assetsRoute: 'blah/coder',
8181
} satisfies UrlSyncSnapshot);
8282

‎plugins/backstage-plugin-coder/src/api/UrlSync.ts

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -42,14 +42,10 @@ const PROXY_URL_KEY_FOR_DISCOVERY_API = 'proxy';
4242

4343
type UrlPrefixes = Readonly<{
4444
proxyPrefix: string;
45-
apiRoutePrefix: string;
46-
assetsRoutePrefix: string;
4745
}>;
4846

4947
export const defaultUrlPrefixes = {
5048
proxyPrefix: `/api/proxy`,
51-
apiRoutePrefix: '/api/v2',
52-
assetsRoutePrefix: '', // Deliberately left as empty string
5349
} as const satisfies UrlPrefixes;
5450

5551
export type UrlSyncSnapshot = Readonly<{
@@ -104,12 +100,10 @@ export class UrlSync implements UrlSyncApi {
104100
}
105101

106102
private prepareNewSnapshot(newProxyUrl: string): UrlSyncSnapshot {
107-
const { assetsRoutePrefix, apiRoutePrefix } = this.urlPrefixes;
108-
109103
return {
110104
baseUrl: newProxyUrl.replace(proxyRouteReplacer, ''),
111-
assetsRoute: `${newProxyUrl}${CODER_PROXY_PREFIX}${assetsRoutePrefix}`,
112-
apiRoute: `${newProxyUrl}${CODER_PROXY_PREFIX}${apiRoutePrefix}`,
105+
assetsRoute: `${newProxyUrl}${CODER_PROXY_PREFIX}`,
106+
apiRoute: `${newProxyUrl}${CODER_PROXY_PREFIX}`,
113107
};
114108
}
115109

0 commit comments

Comments
 (0)
0