Skip to content

Commit 7903538

Browse files
committed
sea-auth-u2m: address round-1 M2M review parity — shared fakeBinding helper, doc + loader cleanup
Ports the M2M-side round-1 review fixes (commit 88d7d21 on sea-auth-m2m) onto the U2M worktree so the two branches stay aligned in review quality. The U2M-specific work in 5eba37f is unchanged; this commit is pure cleanup applied across all three SEA-auth test files (PAT / M2M / U2M). - Extracted `makeFakeBinding()` to `tests/unit/sea/_helpers/fakeBinding.ts` and refactored all three auth-*.test.ts files to import it. The U2M-worktree commit had THREE copies of the closure (the third was the cause for the bloat reviewer's "rule of three" call-out that the M2M-worktree fixup was meant to forestall). - Dropped the unused `SeaAuthMode` type alias from `SeaAuth.ts` — zero callers; inlined literals already power the discriminated union. - Tightened `SeaNativeBinding.openSession` parameter type to consume the discriminated `SeaNativeConnectionOptions` union from `SeaAuth.ts`, restoring compile-time per-mode field enforcement at the FFI seam. - Augmented the Rust `AuthMode` doc-comment with the napi-emission explanation (PascalCase verbatim, not kebab-case) plus the cross-reference reminder to extend `open_session()`'s match on every new variant. - Added the const-enum hazard note to `SeaNativeConnectionOptions`' doc-comment, locking in the duplicated-literal pattern as deliberate (importing the napi `const enum AuthMode` breaks `isolatedModules`). - Cleaned up the conditional-type-cast lobotomy in `auth-pat.test.ts` on the token-provider fixture; plain `as any` + eslint-disable. Skipped findings (same justification as M2M-worktree commit): F-3 borderline error-class taxonomy, F-4 cosmetic arg-order, F-5 redundant comment-anchor (compiler already enforces), F-8 null vs undefined paranoia, F-9 mocha named-function style. Tests: - Unit: 55/55 pass (same count as 5eba37f — pure restructure). - Native build: clean (1m04s release profile). - Type-check: clean (tsc --noEmit). Co-authored-by: Isaac Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>
1 parent f2d7b3b commit 7903538

7 files changed

Lines changed: 102 additions & 95 deletions

File tree

lib/sea/SeaAuth.ts

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,6 @@ import { ConnectionOptions } from '../contracts/IDBSQLClient';
1616
import AuthenticationError from '../errors/AuthenticationError';
1717
import HiveDriverError from '../errors/HiveDriverError';
1818

19-
/**
20-
* Auth-mode discriminant value crossing the napi boundary. The string
21-
* literals are what napi-rs emits from the `#[napi(string_enum)] AuthMode`
22-
* enum at `native/sea/src/database.rs` — they MUST match the variant
23-
* names verbatim (`'Pat'`, `'OAuthM2m'`, `'OAuthU2m'`).
24-
*/
25-
export type SeaAuthMode = 'Pat' | 'OAuthM2m' | 'OAuthU2m';
26-
2719
/**
2820
* Default local listener port for the U2M authorization-code callback.
2921
* Hardcoded here so the override of the kernel default (8020) to the
@@ -47,6 +39,15 @@ const U2M_DEFAULT_REDIRECT_PORT = 8030;
4739
* - `'OAuthU2m'` → `oauthRedirectPort` overrides the kernel default;
4840
* everything else (client_id, scopes, callback timeout,
4941
* token_url_override) uses kernel defaults.
42+
*
43+
* The `authMode` string literals MUST match the napi-emitted `AuthMode`
44+
* variant names verbatim (`'Pat'`, `'OAuthM2m'`, `'OAuthU2m'` — napi-rs's
45+
* `#[napi(string_enum)]` without an explicit case option emits the
46+
* Rust variant identifier as-is). We duplicate the values here instead
47+
* of importing `AuthMode` from `native/sea/index.d.ts` because that
48+
* file declares `AuthMode` as `export const enum`, which is
49+
* incompatible with `isolatedModules` and a runtime-coupling hazard.
50+
* The Rust source of truth lives at `native/sea/src/database.rs`.
5051
*/
5152
export type SeaNativeConnectionOptions =
5253
| {

lib/sea/SeaNativeLoader.ts

Lines changed: 13 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,13 @@
2626
import type {
2727
Connection as NativeConnection,
2828
Statement as NativeStatement,
29-
ConnectionOptions,
3029
ExecuteOptions,
3130
ArrowBatch,
3231
ArrowSchema,
3332
} from '@sea-native';
33+
import type { SeaNativeConnectionOptions } from './SeaAuth';
3434

35-
export type { ConnectionOptions, ExecuteOptions, ArrowBatch, ArrowSchema };
35+
export type { ExecuteOptions, ArrowBatch, ArrowSchema, SeaNativeConnectionOptions };
3636
export type Connection = NativeConnection;
3737
export type Statement = NativeStatement;
3838

@@ -44,28 +44,18 @@ export type SeaExecuteOptions = ExecuteOptions;
4444
export type SeaArrowBatch = ArrowBatch;
4545
export type SeaArrowSchema = ArrowSchema;
4646

47-
/**
48-
* Discriminated session-open options. Mirrors the auto-generated
49-
* `ConnectionOptions` in `native/sea/index.d.ts` as it lands across
50-
* the auth PRs (PAT today; OAuth M2M + U2M after the kernel-side
51-
* OAuth surface ships). Kept permissive (all OAuth fields optional)
52-
* so the JS adapter's auth-union narrows correctly without three
53-
* overloads on this method.
54-
*/
55-
export interface SeaOpenSessionOptions {
56-
hostName: string;
57-
httpPath: string;
58-
authMode: 'Pat' | 'OAuthM2m' | 'OAuthU2m';
59-
token?: string;
60-
oauthClientId?: string;
61-
oauthClientSecret?: string;
62-
oauthRedirectPort?: number;
63-
}
64-
6547
export interface SeaNativeBinding {
6648
version(): string;
67-
/** Open a session over PAT, OAuth M2M, or OAuth U2M auth. */
68-
openSession(opts: SeaOpenSessionOptions): Promise<NativeConnection>;
49+
/**
50+
* Open a session over PAT, OAuth M2M, or OAuth U2M auth. Returns an
51+
* opaque Connection. The discriminated `SeaNativeConnectionOptions`
52+
* union from `SeaAuth` is the source of truth for the per-mode
53+
* required fields, so the loader-seam enforces the same compile-time
54+
* check the adapter applies — a caller can't bypass
55+
* `buildSeaConnectionOptions` and pass, say, `{ authMode: 'Pat' }`
56+
* with no token.
57+
*/
58+
openSession(opts: SeaNativeConnectionOptions): Promise<NativeConnection>;
6959
Connection: typeof NativeConnection;
7060
Statement: typeof NativeStatement;
7161
}
@@ -115,6 +105,7 @@ function tryLoad(): SeaNativeBinding | undefined {
115105
cachedError = new Error(`SEA native binding failed to load with non-standard error: ${String(err)}`);
116106
return undefined;
117107
}
108+
}
118109

119110
/**
120111
* Returns the loaded native binding. Throws a structured error if

native/sea/index.d.ts

Lines changed: 16 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
// Copyright (c) 2026 Databricks, Inc.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
import { SeaNativeBinding } from '../../../../lib/sea/SeaNativeLoader';
16+
17+
export interface RecordedCall {
18+
method: string;
19+
args: unknown[];
20+
}
21+
22+
export interface FakeBinding {
23+
binding: SeaNativeBinding;
24+
calls: RecordedCall[];
25+
}
26+
27+
/**
28+
* Build a fake `SeaNativeBinding` that records every `openSession` call
29+
* and returns a `Connection` whose `close()` is also recorded. Shared
30+
* across the SEA auth unit-test files (PAT / M2M / U2M / future flows)
31+
* so the closure shape lives in exactly one place.
32+
*
33+
* No real native code runs — the fake is structural-typing-only.
34+
*/
35+
export function makeFakeBinding(): FakeBinding {
36+
const calls: RecordedCall[] = [];
37+
38+
const fakeConnection = {
39+
async executeStatement() {
40+
throw new Error('not used in this test');
41+
},
42+
async close() {
43+
calls.push({ method: 'connection.close', args: [] });
44+
},
45+
};
46+
47+
const binding: SeaNativeBinding = {
48+
version() {
49+
return 'fake-binding';
50+
},
51+
async openSession(opts: Parameters<SeaNativeBinding['openSession']>[0]) {
52+
calls.push({ method: 'openSession', args: [opts] });
53+
return fakeConnection as unknown;
54+
},
55+
Connection: function FakeConnection() {} as unknown as Function,
56+
Statement: function FakeStatement() {} as unknown as Function,
57+
};
58+
59+
return { binding, calls };
60+
}

tests/unit/sea/auth-m2m.test.ts

Lines changed: 1 addition & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -15,37 +15,10 @@
1515
import { expect } from 'chai';
1616
import SeaBackend from '../../../lib/sea/SeaBackend';
1717
import { buildSeaConnectionOptions } from '../../../lib/sea/SeaAuth';
18-
import { SeaNativeBinding } from '../../../lib/sea/SeaNativeLoader';
1918
import { ConnectionOptions } from '../../../lib/contracts/IDBSQLClient';
2019
import AuthenticationError from '../../../lib/errors/AuthenticationError';
2120
import HiveDriverError from '../../../lib/errors/HiveDriverError';
22-
23-
function makeFakeBinding() {
24-
const calls: Array<{ method: string; args: unknown[] }> = [];
25-
26-
const fakeConnection = {
27-
async executeStatement() {
28-
throw new Error('not used in this test');
29-
},
30-
async close() {
31-
calls.push({ method: 'connection.close', args: [] });
32-
},
33-
};
34-
35-
const binding: SeaNativeBinding = {
36-
version() {
37-
return 'fake-binding';
38-
},
39-
async openSession(opts: Parameters<SeaNativeBinding['openSession']>[0]) {
40-
calls.push({ method: 'openSession', args: [opts] });
41-
return fakeConnection as unknown;
42-
},
43-
Connection: function FakeConnection() {} as unknown as Function,
44-
Statement: function FakeStatement() {} as unknown as Function,
45-
};
46-
47-
return { binding, calls };
48-
}
21+
import { makeFakeBinding } from './_helpers/fakeBinding';
4922

5023
describe('SeaAuth + SeaBackend — OAuth M2M auth flow', () => {
5124
describe('buildSeaConnectionOptions', () => {

tests/unit/sea/auth-pat.test.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -99,10 +99,8 @@ describe('SeaAuth — PAT auth options builder', () => {
9999
host: 'example.cloud.databricks.com',
100100
path: '/sql/1.0/warehouses/abc',
101101
authType: 'token-provider',
102-
tokenProvider: { getToken: async () => 'tok' } as unknown as ConnectionOptions extends infer T
103-
? // eslint-disable-next-line @typescript-eslint/no-explicit-any
104-
any
105-
: never,
102+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
103+
tokenProvider: { getToken: async () => 'tok' } as any,
106104
};
107105

108106
expect(() => buildSeaConnectionOptions(opts)).to.throw(

tests/unit/sea/auth-u2m.test.ts

Lines changed: 1 addition & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -15,36 +15,9 @@
1515
import { expect } from 'chai';
1616
import SeaBackend from '../../../lib/sea/SeaBackend';
1717
import { buildSeaConnectionOptions } from '../../../lib/sea/SeaAuth';
18-
import { SeaNativeBinding } from '../../../lib/sea/SeaNativeLoader';
1918
import { ConnectionOptions } from '../../../lib/contracts/IDBSQLClient';
2019
import HiveDriverError from '../../../lib/errors/HiveDriverError';
21-
22-
function makeFakeBinding() {
23-
const calls: Array<{ method: string; args: unknown[] }> = [];
24-
25-
const fakeConnection = {
26-
async executeStatement() {
27-
throw new Error('not used in this test');
28-
},
29-
async close() {
30-
calls.push({ method: 'connection.close', args: [] });
31-
},
32-
};
33-
34-
const binding: SeaNativeBinding = {
35-
version() {
36-
return 'fake-binding';
37-
},
38-
async openSession(opts: Parameters<SeaNativeBinding['openSession']>[0]) {
39-
calls.push({ method: 'openSession', args: [opts] });
40-
return fakeConnection as unknown;
41-
},
42-
Connection: function FakeConnection() {} as unknown as Function,
43-
Statement: function FakeStatement() {} as unknown as Function,
44-
};
45-
46-
return { binding, calls };
47-
}
20+
import { makeFakeBinding } from './_helpers/fakeBinding';
4821

4922
describe('SeaAuth + SeaBackend — OAuth U2M auth flow', () => {
5023
describe('buildSeaConnectionOptions', () => {

0 commit comments

Comments
 (0)