Skip to content

Stephenarosaj/headerz #8749

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 20 commits into from
Feb 5, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
cdf7b39
add custom headers to track web frameworks usage
stephenarosaj Jan 28, 2025
fb27687
add transport methods to update framework usage flags after transport…
stephenarosaj Jan 30, 2025
6f27cbd
add transport methods to update framework usage flags after transport…
stephenarosaj Jan 30, 2025
40a8c0b
redesign framework/gen flags into CallerSdkType enum
stephenarosaj Jan 31, 2025
5e397d0
update custom headers assignment logic and values; add gen flag funct…
stephenarosaj Jan 31, 2025
1d504bf
unfreeze CallerSdkTypeEnum
stephenarosaj Jan 31, 2025
f45b282
add checking of _isUsingGen back to custom headers logic
stephenarosaj Jan 31, 2025
b307ae5
add call to _setCallerSdkType() on transport class
stephenarosaj Feb 4, 2025
112a605
add tests for fetch with custom headers based on dcFetch _callerSdkTy…
stephenarosaj Feb 4, 2025
7755617
add changeset for PR
stephenarosaj Feb 4, 2025
aa56521
flip header assignment logic to only use js/gen if _callerSdkType not…
stephenarosaj Feb 4, 2025
3339fde
add test for custom header values when _isUsingGen is true
stephenarosaj Feb 4, 2025
b77ad49
add firebase minor version bump to changeset
stephenarosaj Feb 4, 2025
2619098
fix formatting issues
stephenarosaj Feb 4, 2025
1e3db01
update changelog
stephenarosaj Feb 4, 2025
08accc6
only call transport's _setCallerSdkType() if DataConnect is initialized
stephenarosaj Feb 4, 2025
784597c
simplify tests
stephenarosaj Feb 4, 2025
9efd040
add comments explaining funky eslint logic
stephenarosaj Feb 5, 2025
595fee3
remove new, useless @internal comments
stephenarosaj Feb 5, 2025
99cced0
convert regex strings to regex literals
stephenarosaj Feb 5, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/little-news-sniff.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@firebase/data-connect': minor
'firebase': minor
---

Add custom request headers based on the type of SDK (JS/TS, React, Angular, etc) that's invoking Data Connect requests. This will help us understand how users interact with Data Connect when using the Web SDK.
13 changes: 13 additions & 0 deletions common/api-review/data-connect.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,19 @@ import { FirebaseError } from '@firebase/util';
import { LogLevelString } from '@firebase/logger';
import { Provider } from '@firebase/component';

// @public
export type CallerSdkType = 'Base' | 'Generated' | 'TanstackReactCore' | 'GeneratedReact' | 'TanstackAngularCore' | 'GeneratedAngular';

// @public (undocumented)
export const CallerSdkTypeEnum: {
readonly Base: "Base";
readonly Generated: "Generated";
readonly TanstackReactCore: "TanstackReactCore";
readonly GeneratedReact: "GeneratedReact";
readonly TanstackAngularCore: "TanstackAngularCore";
readonly GeneratedAngular: "GeneratedAngular";
};

// @public
export function connectDataConnectEmulator(dc: DataConnect, host: string, port?: number, sslEnabled?: boolean): void;

Expand Down
17 changes: 15 additions & 2 deletions packages/data-connect/src/api/DataConnect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,12 @@ import {
} from '../core/FirebaseAuthProvider';
import { QueryManager } from '../core/QueryManager';
import { logDebug, logError } from '../logger';
import { DataConnectTransport, TransportClass } from '../network';
import {
CallerSdkType,
CallerSdkTypeEnum,
DataConnectTransport,
TransportClass
} from '../network';
import { RESTTransport } from '../network/transport/rest';

import { MutationManager } from './Mutation';
Expand Down Expand Up @@ -92,6 +97,7 @@ export class DataConnect {
private _transportOptions?: TransportOptions;
private _authTokenProvider?: AuthTokenProvider;
_isUsingGeneratedSdk: boolean = false;
_callerSdkType: CallerSdkType = CallerSdkTypeEnum.Base;
private _appCheckTokenProvider?: AppCheckTokenProvider;
// @internal
constructor(
Expand All @@ -116,6 +122,12 @@ export class DataConnect {
this._isUsingGeneratedSdk = true;
}
}
_setCallerSdkType(callerSdkType: CallerSdkType): void {
this._callerSdkType = callerSdkType;
if (this._initialized) {
this._transport._setCallerSdkType(callerSdkType);
}
}
_delete(): Promise<void> {
_removeServiceInstance(
this.app,
Expand Down Expand Up @@ -164,7 +176,8 @@ export class DataConnect {
this._authTokenProvider,
this._appCheckTokenProvider,
undefined,
this._isUsingGeneratedSdk
this._isUsingGeneratedSdk,
this._callerSdkType
);
if (this._transportOptions) {
this._transport.useEmulator(
Expand Down
19 changes: 15 additions & 4 deletions packages/data-connect/src/network/fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,23 @@ import { Code, DataConnectError } from '../core/error';
import { SDK_VERSION } from '../core/version';
import { logDebug, logError } from '../logger';

import { CallerSdkType, CallerSdkTypeEnum } from './transport';

let connectFetch: typeof fetch | null = globalThis.fetch;
export function initializeFetch(fetchImpl: typeof fetch): void {
connectFetch = fetchImpl;
}
function getGoogApiClientValue(_isUsingGen: boolean): string {
function getGoogApiClientValue(
_isUsingGen: boolean,
_callerSdkType: CallerSdkType
): string {
let str = 'gl-js/ fire/' + SDK_VERSION;
if (_isUsingGen) {
if (
_callerSdkType !== CallerSdkTypeEnum.Base &&
_callerSdkType !== CallerSdkTypeEnum.Generated
) {
str += ' js/' + _callerSdkType.toLowerCase();
} else if (_isUsingGen || _callerSdkType === CallerSdkTypeEnum.Generated) {
str += ' js/gen';
}
return str;
Expand All @@ -42,14 +52,15 @@ export function dcFetch<T, U>(
appId: string | null,
accessToken: string | null,
appCheckToken: string | null,
_isUsingGen: boolean
_isUsingGen: boolean,
_callerSdkType: CallerSdkType
): Promise<{ data: T; errors: Error[] }> {
if (!connectFetch) {
throw new DataConnectError(Code.OTHER, 'No Fetch Implementation detected!');
}
const headers: HeadersInit = {
'Content-Type': 'application/json',
'X-Goog-Api-Client': getGoogApiClientValue(_isUsingGen)
'X-Goog-Api-Client': getGoogApiClientValue(_isUsingGen, _callerSdkType)
};
if (accessToken) {
headers['X-Firebase-Auth-Token'] = accessToken;
Expand Down
24 changes: 23 additions & 1 deletion packages/data-connect/src/network/transport/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,26 @@ import { DataConnectOptions, TransportOptions } from '../../api/DataConnect';
import { AppCheckTokenProvider } from '../../core/AppCheckTokenProvider';
import { AuthTokenProvider } from '../../core/FirebaseAuthProvider';

/**
* enum representing different flavors of the SDK used by developers
* use the CallerSdkType for type-checking, and the CallerSdkTypeEnum for value-checking/assigning
*/
export type CallerSdkType =
| 'Base' // Core JS SDK
| 'Generated' // Generated JS SDK
| 'TanstackReactCore' // Tanstack non-generated React SDK
| 'GeneratedReact' // Generated React SDK
| 'TanstackAngularCore' // Tanstack non-generated Angular SDK
| 'GeneratedAngular'; // Generated Angular SDK
export const CallerSdkTypeEnum = {
Base: 'Base', // Core JS SDK
Generated: 'Generated', // Generated JS SDK
TanstackReactCore: 'TanstackReactCore', // Tanstack non-generated React SDK
GeneratedReact: 'GeneratedReact', // Tanstack non-generated Angular SDK
TanstackAngularCore: 'TanstackAngularCore', // Tanstack non-generated Angular SDK
GeneratedAngular: 'GeneratedAngular' // Generated Angular SDK
} as const;

/**
* @internal
*/
Expand All @@ -33,6 +53,7 @@ export interface DataConnectTransport {
): Promise<{ data: T; errors: Error[] }>;
useEmulator(host: string, port?: number, sslEnabled?: boolean): void;
onTokenChanged: (token: string | null) => void;
_setCallerSdkType(callerSdkType: CallerSdkType): void;
}

/**
Expand All @@ -45,5 +66,6 @@ export type TransportClass = new (
authProvider?: AuthTokenProvider,
appCheckProvider?: AppCheckTokenProvider,
transportOptions?: TransportOptions,
_isUsingGen?: boolean
_isUsingGen?: boolean,
_callerSdkType?: CallerSdkType
) => DataConnectTransport;
15 changes: 11 additions & 4 deletions packages/data-connect/src/network/transport/rest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import { logDebug } from '../../logger';
import { addToken, urlBuilder } from '../../util/url';
import { dcFetch } from '../fetch';

import { DataConnectTransport } from '.';
import { CallerSdkType, CallerSdkTypeEnum, DataConnectTransport } from '.';

export class RESTTransport implements DataConnectTransport {
private _host = '';
Expand All @@ -43,7 +43,8 @@ export class RESTTransport implements DataConnectTransport {
private authProvider?: AuthTokenProvider | undefined,
private appCheckProvider?: AppCheckTokenProvider | undefined,
transportOptions?: TransportOptions | undefined,
private _isUsingGen = false
private _isUsingGen = false,
private _callerSdkType: CallerSdkType = CallerSdkTypeEnum.Base
) {
if (transportOptions) {
if (typeof transportOptions.port === 'number') {
Expand Down Expand Up @@ -180,7 +181,8 @@ export class RESTTransport implements DataConnectTransport {
this.appId,
this._accessToken,
this._appCheckToken,
this._isUsingGen
this._isUsingGen,
this._callerSdkType
)
);
return withAuth;
Expand All @@ -205,9 +207,14 @@ export class RESTTransport implements DataConnectTransport {
this.appId,
this._accessToken,
this._appCheckToken,
this._isUsingGen
this._isUsingGen,
this._callerSdkType
);
});
return taskResult;
};

_setCallerSdkType(callerSdkType: CallerSdkType): void {
this._callerSdkType = callerSdkType;
}
}
121 changes: 112 additions & 9 deletions packages/data-connect/test/unit/fetch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,25 +20,30 @@ import chaiAsPromised from 'chai-as-promised';
import * as sinon from 'sinon';

import { dcFetch, initializeFetch } from '../../src/network/fetch';
import { CallerSdkType, CallerSdkTypeEnum } from '../../src/network/transport';
use(chaiAsPromised);
function mockFetch(json: object): void {
function mockFetch(json: object, reject: boolean): sinon.SinonStub {
const fakeFetchImpl = sinon.stub().returns(
Promise.resolve({
json: () => {
return Promise.resolve(json);
},
status: 401
status: reject ? 401 : 200
} as Response)
);
initializeFetch(fakeFetchImpl);
return fakeFetchImpl;
}
describe('fetch', () => {
it('should throw an error with just the message when the server responds with an error with a message property in the body', async () => {
const message = 'Failed to connect to Postgres instance';
mockFetch({
code: 401,
message
});
mockFetch(
{
code: 401,
message
},
true
);
await expect(
dcFetch(
'http://localhost',
Expand All @@ -51,7 +56,8 @@ describe('fetch', () => {
null,
null,
null,
false
false,
CallerSdkTypeEnum.Base
)
).to.eventually.be.rejectedWith(message);
});
Expand All @@ -61,7 +67,7 @@ describe('fetch', () => {
code: 401,
message1: message
};
mockFetch(json);
mockFetch(json, true);
await expect(
dcFetch(
'http://localhost',
Expand All @@ -74,8 +80,105 @@ describe('fetch', () => {
null,
null,
null,
false
false,
CallerSdkTypeEnum.Base
)
).to.eventually.be.rejectedWith(JSON.stringify(json));
});
it('should assign different values to custom headers based on the _callerSdkType argument (_isUsingGen is false)', async () => {
const json = {
code: 200,
message1: 'success'
};
const fakeFetchImpl = mockFetch(json, false);

for (const callerSdkType in CallerSdkTypeEnum) {
// this check is done to follow the best practices outlined by the "guard-for-in" eslint rule
if (
Object.prototype.hasOwnProperty.call(CallerSdkTypeEnum, callerSdkType)
) {
await dcFetch(
'http://localhost',
{
name: 'n',
operationName: 'n',
variables: {}
},
{} as AbortController,
null,
null,
null,
false, // _isUsingGen is false
callerSdkType as CallerSdkType
);

let expectedHeaderRegex: RegExp;
if (callerSdkType === CallerSdkTypeEnum.Base) {
// should not contain any "js/xxx" substring
expectedHeaderRegex = RegExp(/^((?!js\/\w).)*$/);
} else if (callerSdkType === CallerSdkTypeEnum.Generated) {
expectedHeaderRegex = RegExp(/js\/gen/);
} else {
expectedHeaderRegex = RegExp(`js\/${callerSdkType.toLowerCase()}`);
}
expect(
fakeFetchImpl.calledWithMatch(
'http://localhost',
sinon.match.hasNested(
'headers.X-Goog-Api-Client',
sinon.match(expectedHeaderRegex)
)
)
).to.be.true;
}
}
});
it('should assign custom headers based on _callerSdkType before checking to-be-deprecated _isUsingGen', async () => {
const json = {
code: 200,
message1: 'success'
};
const fakeFetchImpl = mockFetch(json, false);

for (const callerSdkType in CallerSdkTypeEnum) {
// this check is done to follow the best practices outlined by the "guard-for-in" eslint rule
if (
Object.prototype.hasOwnProperty.call(CallerSdkTypeEnum, callerSdkType)
) {
await dcFetch(
'http://localhost',
{
name: 'n',
operationName: 'n',
variables: {}
},
{} as AbortController,
null,
null,
null,
true, // _isUsingGen is true
callerSdkType as CallerSdkType
);

let expectedHeaderRegex: RegExp;
if (
callerSdkType === CallerSdkTypeEnum.Generated ||
callerSdkType === CallerSdkTypeEnum.Base
) {
expectedHeaderRegex = RegExp(`js\/gen`);
} else {
expectedHeaderRegex = RegExp(`js\/${callerSdkType.toLowerCase()}`);
}
expect(
fakeFetchImpl.calledWithMatch(
'http://localhost',
sinon.match.hasNested(
'headers.X-Goog-Api-Client',
sinon.match(expectedHeaderRegex)
)
)
).to.be.true;
}
}
});
});
Loading