Skip to content
This repository was archived by the owner on Jun 26, 2026. It is now read-only.

Commit ba7ea1a

Browse files
twoGiantsclaude
andcommitted
refactor: decouple auth from GithubService
UserAvatar called GithubService.validateToken() directly, coupling it to a specific forge implementation. The createLazyAuth strategy used Object.assign with untyped hook plumbing to support lazy token resolution. Replace the static validateToken with fetchUserInfo on the SourceControlService interface. UserAvatar now calls the interface method via useSourceControlService, making it forge-agnostic. Replace createLazyAuth with a getter that caches the Octokit instance and re-creates it only when the token changes. Rename GitHubUser to ForgeUser with a simplified shape. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Stanislav Jakuschevskij <sjakusch@redhat.com>
1 parent 1df2c65 commit ba7ea1a

6 files changed

Lines changed: 49 additions & 67 deletions

File tree

src/components/UserAvatar.test.tsx

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import { render, screen, waitFor } from '@testing-library/react';
22
import userEvent from '@testing-library/user-event';
33
import { UserAvatar } from './UserAvatar';
4-
import { GithubService } from '../services/source-control/GithubService';
54
import { PAT_KEY, USER_KEY } from '../services/types';
65
import { ForgeConnectionContext } from '../context/ForgeConnectionProvider';
76
import { ReactNode } from 'react';
@@ -10,13 +9,14 @@ jest.mock('react-i18next', () => ({
109
useTranslation: () => ({ t: (key: string) => key }),
1110
}));
1211

13-
jest.mock('../services/source-control/GithubService', () => ({
14-
GithubService: { validateToken: jest.fn() },
12+
const mockFetchUserInfo = jest.fn();
13+
jest.mock('../services/source-control/useSourceControlService', () => ({
14+
useSourceControlService: () => ({
15+
fetchUserInfo: mockFetchUserInfo,
16+
}),
1517
}));
1618

17-
const mockValidateToken = GithubService.validateToken as jest.Mock;
18-
19-
const testUser = { login: 'twoGiants', avatarUrl: 'https://example.com/avatar.png' };
19+
const testUser = { name: 'twoGiants' };
2020

2121
function renderWithContext(
2222
ui: ReactNode,
@@ -39,6 +39,7 @@ describe('UserAvatar', () => {
3939
afterAll(() => {
4040
sessionStorage.clear();
4141
});
42+
4243
describe('rendering', () => {
4344
it('renders "Connect to GitHub" when no user is stored', () => {
4445
renderWithContext(<UserAvatar enableReconnect={false} />);
@@ -111,18 +112,18 @@ describe('UserAvatar', () => {
111112
expect(screen.getByRole('button', { name: 'Connect' })).toBeDisabled();
112113
});
113114

114-
it('calls validateToken with PAT and updates UI on successful connect', async () => {
115+
it('calls fetchUserInfo with PAT and updates UI on successful connect', async () => {
115116
const user = userEvent.setup();
116117
const connectToForge = jest.fn();
117-
mockValidateToken.mockResolvedValue(testUser);
118+
mockFetchUserInfo.mockResolvedValue(testUser);
118119

119120
renderWithContext(<UserAvatar enableReconnect />, { isActive: false, connectToForge });
120121

121122
await user.type(screen.getByLabelText('Personal Access Token'), 'ghp_valid');
122123
await user.click(screen.getByRole('button', { name: 'Connect' }));
123124

124125
await waitFor(() => {
125-
expect(mockValidateToken).toHaveBeenCalledWith('ghp_valid');
126+
expect(mockFetchUserInfo).toHaveBeenCalledWith('ghp_valid');
126127
});
127128

128129
expect(screen.getByText('twoGiants')).toBeInTheDocument();
@@ -131,9 +132,9 @@ describe('UserAvatar', () => {
131132
expect(connectToForge).toHaveBeenCalled();
132133
});
133134

134-
it('shows error alert when validateToken rejects', async () => {
135+
it('shows error alert when fetchUserInfo rejects', async () => {
135136
const user = userEvent.setup();
136-
mockValidateToken.mockRejectedValue(new Error('Bad credentials'));
137+
mockFetchUserInfo.mockRejectedValue(new Error('Bad credentials'));
137138

138139
renderWithContext(<UserAvatar enableReconnect />);
139140

src/components/UserAvatar.tsx

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,10 @@ import {
1111
} from '@patternfly/react-core';
1212
import { KeyIcon, UserIcon } from '@patternfly/react-icons';
1313
import { useTranslation } from 'react-i18next';
14-
import { GitHubUser, PAT_KEY, USER_KEY } from '../services/types';
14+
import { ForgeUser, PAT_KEY, USER_KEY } from '../services/types';
1515
import { useCallback, useContext, useState } from 'react';
16-
import { GithubService } from '../services/source-control/GithubService';
1716
import { ForgeConnectionContext } from '../context/ForgeConnectionProvider';
17+
import { useSourceControlService } from '../services/source-control/useSourceControlService';
1818

1919
interface UserAvatarProps {
2020
enableReconnect: boolean;
@@ -25,7 +25,7 @@ export function UserAvatar({ enableReconnect }: UserAvatarProps) {
2525
const { user, isModalOpen, openModal, closeModal, login } = useUserAvatar(enableReconnect);
2626

2727
const icon = user ? <UserIcon /> : <KeyIcon />;
28-
const label = user ? user.login : t('Connect to GitHub');
28+
const label = user ? user.name : t('Connect to GitHub');
2929

3030
return (
3131
<>
@@ -44,19 +44,18 @@ export function UserAvatar({ enableReconnect }: UserAvatarProps) {
4444
}
4545

4646
function useUserAvatar(enableReconnect: boolean) {
47+
const sourceControlService = useSourceControlService();
4748
const connectToForge = useContext(ForgeConnectionContext).connectToForge;
48-
const [user, setUser] = useState<GitHubUser | null>(() => readStoredUser());
49+
const [user, setUser] = useState<ForgeUser | null>(() => readStoredUser());
4950
const [isModalOpen, setIsModalOpen] = useState(
5051
() => enableReconnect && !sessionStorage.getItem(PAT_KEY),
5152
);
5253

53-
// TODO(twoGiants): remove useCallback here and below => it has no effect
5454
const login = useCallback(async (pat: string) => {
55-
// TODO(twoGiants): use useSourceControlService hook => don't use GithubService directly
56-
const githubUser = await GithubService.validateToken(pat);
55+
const forgeUser = await sourceControlService.fetchUserInfo(pat);
5756
sessionStorage.setItem(PAT_KEY, pat);
58-
sessionStorage.setItem(USER_KEY, JSON.stringify(githubUser));
59-
setUser(githubUser);
57+
sessionStorage.setItem(USER_KEY, JSON.stringify(forgeUser));
58+
setUser(forgeUser);
6059
setIsModalOpen(false);
6160
connectToForge();
6261
}, []);
@@ -74,12 +73,12 @@ function useUserAvatar(enableReconnect: boolean) {
7473
return { user, isModalOpen, openModal, closeModal, login, logout };
7574
}
7675

77-
function readStoredUser(): GitHubUser | null {
76+
function readStoredUser(): ForgeUser | null {
7877
const pat = sessionStorage.getItem(PAT_KEY);
7978
const userJson = sessionStorage.getItem(USER_KEY);
8079
if (pat && userJson) {
8180
try {
82-
return JSON.parse(userJson) as GitHubUser;
81+
return JSON.parse(userJson) as ForgeUser;
8382
} catch {
8483
return null;
8584
}

src/services/source-control/GithubService.ts

Lines changed: 16 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,46 +1,29 @@
11
import { Octokit } from '@octokit/rest';
2-
import { FileEntry, GitHubUser, RepoInfo, SourceRepo } from '../types';
2+
import { FileEntry, ForgeUser, RepoInfo, SourceRepo } from '../types';
33
import { SourceControlService } from './SourceControlService';
44

5-
/**
6-
* Custom Octokit auth strategy that reads the token lazily via a callback.
7-
* The callback is invoked on every request, so the token can change at runtime
8-
* (e.g. after the user enters a PAT in sessionStorage).
9-
*
10-
* Follows the same shape as @octokit/auth-token's createTokenAuth, but defers
11-
* token resolution to call-time instead of binding a static string.
12-
*/
13-
function createLazyAuth(getToken: () => string) {
14-
return Object.assign(
15-
async () => {
16-
const token = getToken();
17-
return { type: 'token' as const, tokenType: 'oauth' as const, token };
18-
},
19-
{
20-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
21-
hook: async (request: any, route: any, parameters?: any) => {
22-
const token = getToken();
23-
const endpoint = request.endpoint.merge(route, parameters);
24-
if (token) {
25-
endpoint.headers.authorization = `token ${token}`;
26-
}
27-
return request(endpoint);
28-
},
29-
},
30-
);
31-
}
32-
335
export class GithubService implements SourceControlService {
34-
private octokit: Octokit;
6+
private getToken: () => string;
7+
private cachedOctokit: Octokit | null = null;
8+
private cachedToken: string = '';
359

3610
constructor(getToken: () => string) {
37-
this.octokit = new Octokit({ authStrategy: () => createLazyAuth(getToken) });
11+
this.getToken = getToken;
12+
}
13+
14+
private get octokit(): Octokit {
15+
const token = this.getToken();
16+
if (token !== this.cachedToken) {
17+
this.cachedToken = token;
18+
this.cachedOctokit = new Octokit({ auth: token });
19+
}
20+
return this.cachedOctokit!;
3821
}
3922

40-
static async validateToken(pat: string): Promise<GitHubUser> {
23+
async fetchUserInfo(pat: string): Promise<ForgeUser> {
4124
const octokit = new Octokit({ auth: pat });
4225
const { data } = await octokit.users.getAuthenticated();
43-
return { login: data.login, avatarUrl: data.avatar_url };
26+
return { name: data.login };
4427
}
4528

4629
async listFunctionRepos(): Promise<SourceRepo[]> {

src/services/source-control/SourceControlService.test.ts

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -139,20 +139,19 @@ describe('GithubService', () => {
139139
});
140140
});
141141

142-
describe('validateToken', () => {
143-
it('returns GitHubUser on valid token', async () => {
144-
const user = await GithubService.validateToken('valid-token');
142+
describe('fetchUserInfo', () => {
143+
it('returns ForgeUser on valid token', async () => {
144+
const svc = new GithubService(() => '');
145+
const user = await svc.fetchUserInfo('valid-token');
145146

146-
expect(user).toEqual({
147-
login: 'twoGiants',
148-
avatarUrl: 'https://avatars.githubusercontent.com/u/123',
149-
});
147+
expect(user).toEqual({ name: 'twoGiants' });
150148
});
151149

152150
it('throws on invalid token', async () => {
153151
mockGetAuthenticated.mockRejectedValue(new Error('Bad credentials'));
152+
const svc = new GithubService(() => '');
154153

155-
await expect(GithubService.validateToken('bad-token')).rejects.toThrow('Bad credentials');
154+
await expect(svc.fetchUserInfo('bad-token')).rejects.toThrow('Bad credentials');
156155
});
157156
});
158157
});
Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
1-
import { FileEntry, RepoInfo, SourceRepo } from '../types';
1+
import { FileEntry, ForgeUser as ForgeUserInfo, RepoInfo, SourceRepo } from '../types';
22

33
export interface SourceControlService {
44
listFunctionRepos(): Promise<SourceRepo[]>;
55
fetchFileContent(repo: SourceRepo, path: string): Promise<string>;
66
push(repo: RepoInfo, files: FileEntry[], message: string): Promise<void>;
7+
fetchUserInfo(pat: string): Promise<ForgeUserInfo>;
78
}

src/services/types.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ export interface SourceRepo {
3131
defaultBranch: string;
3232
}
3333

34-
export interface GitHubUser {
35-
login: string;
36-
avatarUrl: string;
34+
export interface ForgeUser {
35+
name: string;
3736
}

0 commit comments

Comments
 (0)