Skip to content

Commit c88e82d

Browse files
Copilothotlong
andcommitted
refactor: extract shared hook capture helper in auth-plugin tests
Address code review feedback by extracting duplicate hook capture logic into a shared createHookCapture() utility at the test suite level. Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
1 parent bac5f29 commit c88e82d

2 files changed

Lines changed: 24 additions & 25 deletions

File tree

ROADMAP.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,7 @@ business/custom objects, aligning with industry best practices (e.g., ServiceNow
330330
- v3.x: The `SystemObjectName` constants now emit `sys_`-prefixed names. Implementations using `StorageNameMapping.resolveTableName()` can set `tableName` to preserve legacy physical table names during the transition.
331331
- v3.x: The `@objectstack/plugin-auth` ObjectQL adapter now includes `AUTH_MODEL_TO_PROTOCOL` mapping to translate better-auth's hardcoded model names (`user`, `session`, `account`, `verification`) to protocol names (`sys_user`, `sys_session`, `sys_account`, `sys_verification`). Custom adapters must adopt the same mapping.
332332
- v3.x: **Bug fix**`AuthManager.createDatabaseConfig()` now wraps the ObjectQL adapter as a `DBAdapterInstance` factory function (`(options) => DBAdapter`). Previously the raw adapter object was passed, which fell through to the Kysely adapter path and failed silently. `AuthManager.handleRequest()` and `AuthPlugin.registerAuthRoutes()` now inspect `response.status >= 500` and log the error body, since better-auth catches internal errors and returns 500 Responses without throwing.
333+
- v3.x: **Bug fix**`AuthPlugin` now defers HTTP route registration to a `kernel:ready` hook instead of doing it synchronously in `start()`. This makes the plugin resilient to plugin loading order — the `http-server` service is guaranteed to be available after all plugins complete their init/start phases. The CLI `serve` command also registers `HonoServerPlugin` before config plugins (with duplicate detection) for the same reason.
333334
- v4.0: Legacy un-prefixed aliases will be fully removed.
334335

335336
---

packages/plugins/plugin-auth/src/auth-plugin.test.ts

Lines changed: 23 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,19 @@ describe('AuthPlugin', () => {
88
let mockContext: PluginContext;
99
let authPlugin: AuthPlugin;
1010

11+
/** Shared hook capture utilities for tests that need kernel:ready simulation */
12+
const createHookCapture = () => {
13+
const handlers = new Map<string, Array<(...args: any[]) => Promise<void>>>();
14+
const hookFn = vi.fn((name: string, handler: (...args: any[]) => Promise<void>) => {
15+
if (!handlers.has(name)) handlers.set(name, []);
16+
handlers.get(name)!.push(handler);
17+
});
18+
const trigger = async (name: string) => {
19+
for (const h of handlers.get(name) || []) await h();
20+
};
21+
return { handlers, hookFn, trigger };
22+
};
23+
1124
beforeEach(() => {
1225
mockContext = {
1326
registerService: vi.fn(),
@@ -98,29 +111,19 @@ describe('AuthPlugin', () => {
98111
});
99112

100113
describe('Start Phase', () => {
101-
let hookHandlers: Map<string, Array<(...args: any[]) => Promise<void>>>;
114+
let hookCapture: ReturnType<typeof createHookCapture>;
102115

103116
beforeEach(async () => {
104-
hookHandlers = new Map();
117+
hookCapture = createHookCapture();
105118
authPlugin = new AuthPlugin({
106119
secret: 'test-secret-at-least-32-chars-long',
107120
baseUrl: 'http://localhost:3000',
108121
});
109122
// Capture hook registrations so we can trigger them in tests
110-
mockContext.hook = vi.fn((name: string, handler: (...args: any[]) => Promise<void>) => {
111-
if (!hookHandlers.has(name)) hookHandlers.set(name, []);
112-
hookHandlers.get(name)!.push(handler);
113-
});
123+
mockContext.hook = hookCapture.hookFn;
114124
await authPlugin.init(mockContext);
115125
});
116126

117-
/** Helper: invoke all handlers registered for a given hook */
118-
const triggerHook = async (name: string) => {
119-
for (const handler of hookHandlers.get(name) || []) {
120-
await handler();
121-
}
122-
};
123-
124127
it('should register a kernel:ready hook for route registration', async () => {
125128
await authPlugin.start(mockContext);
126129

@@ -153,7 +156,7 @@ describe('AuthPlugin', () => {
153156
expect(mockRawApp.all).not.toHaveBeenCalled();
154157

155158
// Simulate kernel:ready
156-
await triggerHook('kernel:ready');
159+
await hookCapture.trigger('kernel:ready');
157160

158161
expect(mockContext.getService).toHaveBeenCalledWith('http-server');
159162
expect(mockHttpServer.getRawApp).toHaveBeenCalled();
@@ -184,7 +187,7 @@ describe('AuthPlugin', () => {
184187
});
185188

186189
await authPlugin.start(mockContext);
187-
await triggerHook('kernel:ready');
190+
await hookCapture.trigger('kernel:ready');
188191

189192
// Extract the registered route handler
190193
const routeHandler = mockRawApp.all.mock.calls[0][1];
@@ -237,7 +240,7 @@ describe('AuthPlugin', () => {
237240
mockContext.getService = vi.fn(() => null);
238241

239242
await authPlugin.start(mockContext);
240-
await triggerHook('kernel:ready');
243+
await hookCapture.trigger('kernel:ready');
241244

242245
expect(mockContext.getService).toHaveBeenCalledWith('http-server');
243246
expect(mockContext.logger.warn).toHaveBeenCalledWith(
@@ -252,7 +255,7 @@ describe('AuthPlugin', () => {
252255
});
253256

254257
await authPlugin.start(mockContext);
255-
await triggerHook('kernel:ready');
258+
await hookCapture.trigger('kernel:ready');
256259

257260
expect(mockContext.logger.warn).toHaveBeenCalledWith(
258261
expect.stringContaining('No HTTP server available')
@@ -289,11 +292,8 @@ describe('AuthPlugin', () => {
289292

290293
describe('Configuration Options', () => {
291294
it('should use custom base path', async () => {
292-
const hookHandlers: Map<string, Array<(...args: any[]) => Promise<void>>> = new Map();
293-
mockContext.hook = vi.fn((name: string, handler: (...args: any[]) => Promise<void>) => {
294-
if (!hookHandlers.has(name)) hookHandlers.set(name, []);
295-
hookHandlers.get(name)!.push(handler);
296-
});
295+
const { hookFn, trigger } = createHookCapture();
296+
mockContext.hook = hookFn;
297297

298298
authPlugin = new AuthPlugin({
299299
secret: 'test-secret-at-least-32-chars-long',
@@ -322,9 +322,7 @@ describe('AuthPlugin', () => {
322322
await authPlugin.start(mockContext);
323323

324324
// Trigger kernel:ready to actually register routes
325-
for (const handler of hookHandlers.get('kernel:ready') || []) {
326-
await handler();
327-
}
325+
await trigger('kernel:ready');
328326

329327
expect(mockRawApp.all).toHaveBeenCalledWith(
330328
'/custom/auth/*',

0 commit comments

Comments
 (0)