Skip to content
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

Update SSRC API client #2457

Merged
merged 6 commits into from
Feb 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
51 changes: 48 additions & 3 deletions src/remote-config/remote-config-api-client-internal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,19 @@ import { PrefixedFirebaseError } from '../utils/error';
import * as utils from '../utils/index';
import * as validator from '../utils/validator';
import { deepCopy } from '../utils/deep-copy';
import { ListVersionsOptions, ListVersionsResult, RemoteConfigTemplate } from './remote-config-api';
import {
ListVersionsOptions,
ListVersionsResult,
RemoteConfigTemplate,
RemoteConfigServerTemplateData
} from './remote-config-api';

// Remote Config backend constants
const FIREBASE_REMOTE_CONFIG_V1_API = 'https://firebaseremoteconfig.googleapis.com/v1';
/**
* Allows the `FIREBASE_REMOTE_CONFIG_URL_BASE` environment
* variable to override the default API endpoint URL.
*/
const FIREBASE_REMOTE_CONFIG_URL_BASE = process.env.FIREBASE_REMOTE_CONFIG_URL_BASE || 'https://firebaseremoteconfig.googleapis.com';
Copy link
Contributor Author

@erikeldridge erikeldridge Feb 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lahirumaramba I've found it helpful here and in the Web SDK to be able to point the SDK at a local server for testing while in development, and issue reproduction after release. I don't see other references to process.env in this SDK, though. Do you have a preferred way to reference an env var?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! This makes sense to me. We use process.env to handle other env variables

if (!process.env.STORAGE_EMULATOR_HOST && process.env.FIREBASE_STORAGE_EMULATOR_HOST) {

Let's make a note to document this new env var in our public docs

Copy link
Contributor Author

@erikeldridge erikeldridge Feb 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a row to our burndown to document this env var.

const FIREBASE_REMOTE_CONFIG_HEADERS = {
'X-Firebase-Client': `fire-admin-node/${utils.getSdkVersion()}`,
// There is a known issue in which the ETag is not properly returned in cases where the request
Expand Down Expand Up @@ -166,6 +175,24 @@ export class RemoteConfigApiClient {
});
}

public getServerTemplate(): Promise<RemoteConfigServerTemplateData> {
return this.getUrl()
.then((url) => {
const request: HttpRequestConfig = {
method: 'GET',
url: `${url}/namespaces/firebase-server/serverRemoteConfig`,
headers: FIREBASE_REMOTE_CONFIG_HEADERS
};
return this.httpClient.send(request);
})
.then((resp) => {
return this.toRemoteConfigServerTemplate(resp);
})
.catch((err) => {
throw this.toFirebaseError(err);
});
}

private sendPutRequest(template: RemoteConfigTemplate, etag: string, validateOnly?: boolean): Promise<HttpResponse> {
let path = 'remoteConfig';
if (validateOnly) {
Expand All @@ -191,7 +218,7 @@ export class RemoteConfigApiClient {
private getUrl(): Promise<string> {
return this.getProjectIdPrefix()
.then((projectIdPrefix) => {
return `${FIREBASE_REMOTE_CONFIG_V1_API}/${projectIdPrefix}`;
return `${FIREBASE_REMOTE_CONFIG_URL_BASE}/v1/${projectIdPrefix}`;
});
}

Expand Down Expand Up @@ -255,6 +282,24 @@ export class RemoteConfigApiClient {
};
}

/**
* Creates a RemoteConfigServerTemplate from the API response.
* If provided, customEtag is used instead of the etag returned in the API response.
*
* @param {HttpResponse} resp API response object.
* @param {string} customEtag A custom etag to replace the etag fom the API response (Optional).
*/
private toRemoteConfigServerTemplate(resp: HttpResponse, customEtag?: string): RemoteConfigServerTemplateData {
const etag = (typeof customEtag === 'undefined') ? resp.headers['etag'] : customEtag;
this.validateEtag(etag);
return {
conditions: resp.data.conditions,
parameters: resp.data.parameters,
etag,
version: resp.data.version,
};
}

/**
* Checks if the given RemoteConfigTemplate object is valid.
* The object must have valid parameters, parameter groups, conditions, and an etag.
Expand Down
20 changes: 10 additions & 10 deletions src/remote-config/remote-config-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ export interface RemoteConfigCondition {
}

/**
* Interface representing a Remote Config condition in the data-plane.
* Represents a Remote Config condition in the dataplane.
* A condition targets a specific group of users. A list of these conditions make up
* part of a Remote Config template.
*/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's get TW reviews on these. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah. Good point. I was focused on this being an internal client and overlooked that it updated the API. I've added @jenh as a reviewer.

Expand Down Expand Up @@ -156,7 +156,7 @@ export interface RemoteConfigParameterGroup {
}

/**
* Interface representing a Remote Config client template.
* Represents a Remote Config client template.
*/
export interface RemoteConfigTemplate {
/**
Expand Down Expand Up @@ -189,7 +189,7 @@ export interface RemoteConfigTemplate {
}

/**
* Interface representing the data in a Remote Config server template.
* Represents the data in a Remote Config server template.
*/
export interface RemoteConfigServerTemplateData {
/**
Expand All @@ -203,7 +203,7 @@ export interface RemoteConfigServerTemplateData {
parameters: { [key: string]: RemoteConfigParameter };

/**
* ETag of the current Remote Config template (readonly).
* Current Remote Config template ETag (read-only).
*/
readonly etag: string;

Expand All @@ -214,28 +214,28 @@ export interface RemoteConfigServerTemplateData {
}

/**
* Interface representing a stateful abstraction for a Remote Config server template.
* Represents a stateful abstraction for a Remote Config server template.
*/
export interface RemoteConfigServerTemplate {

/**
* Cached {@link RemoteConfigServerTemplateData}
* Cached {@link RemoteConfigServerTemplateData}.
*/
cache: RemoteConfigServerTemplateData;

/**
* A {@link RemoteConfigServerConfig} containing default values for Config
* A {@link RemoteConfigServerConfig} that contains default Config values.
*/
defaultConfig: RemoteConfigServerConfig;

/**
* Evaluates the current template to produce a {@link RemoteConfigServerConfig}
* Evaluates the current template to produce a {@link RemoteConfigServerConfig}.
*/
evaluate(): RemoteConfigServerConfig;

/**
* Fetches and caches the current active version of the
* {@link RemoteConfigServerTemplate} of the project.
* project's {@link RemoteConfigServerTemplate}.
*/
load(): Promise<void>;
}
Expand Down Expand Up @@ -364,6 +364,6 @@ export interface ListVersionsOptions {
}

/**
* Type representing the configuration produced by evaluating a server template.
* Represents the configuration produced by evaluating a server template.
*/
export type RemoteConfigServerConfig = { [key: string]: string | boolean | number }
36 changes: 34 additions & 2 deletions test/unit/remote-config/remote-config-api-client.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import { getSdkVersion } from '../../../src/utils/index';
import {
RemoteConfigTemplate, Version, ListVersionsResult,
} from '../../../src/remote-config/index';
import { RemoteConfigServerTemplateData } from '../../../src/remote-config/remote-config-api';

const expect = chai.expect;

Expand Down Expand Up @@ -661,6 +662,36 @@ describe('RemoteConfigApiClient', () => {
});
});

describe('getServerTemplate', () => {
it('should reject when project id is not available', () => {
return clientWithoutProjectId.getServerTemplate()
.should.eventually.be.rejectedWith(noProjectId);
});

// tests for api response validations
runEtagHeaderTests(() => apiClient.getServerTemplate());
runErrorResponseTests(() => apiClient.getServerTemplate());

it('should resolve with the latest template on success', () => {
const stub = sinon
.stub(HttpClient.prototype, 'send')
.resolves(utils.responseFrom(TEST_RESPONSE, 200, { etag: 'etag-123456789012-1' }));
stubs.push(stub);
return apiClient.getServerTemplate()
.then((resp) => {
expect(resp.conditions).to.deep.equal(TEST_RESPONSE.conditions);
expect(resp.parameters).to.deep.equal(TEST_RESPONSE.parameters);
expect(resp.etag).to.equal('etag-123456789012-1');
expect(resp.version).to.deep.equal(TEST_RESPONSE.version);
expect(stub).to.have.been.calledOnce.and.calledWith({
method: 'GET',
url: 'https://firebaseremoteconfig.googleapis.com/v1/projects/test-project/namespaces/firebase-server/serverRemoteConfig',
headers: EXPECTED_HEADERS,
});
});
});
});

function runTemplateVersionNumberTests(rcOperation: (v: string | number) => any): void {
['', null, NaN, true, [], {}].forEach((invalidVersion) => {
it(`should reject if the versionNumber is: ${invalidVersion}`, () => {
Expand All @@ -677,7 +708,7 @@ describe('RemoteConfigApiClient', () => {
});
}

function runEtagHeaderTests(rcOperation: () => Promise<RemoteConfigTemplate>): void {
function runEtagHeaderTests(rcOperation: () => Promise<RemoteConfigTemplate | RemoteConfigServerTemplateData>): void {
it('should reject when the etag is not present in the response', () => {
const stub = sinon
.stub(HttpClient.prototype, 'send')
Expand All @@ -690,7 +721,8 @@ describe('RemoteConfigApiClient', () => {
});
}

function runErrorResponseTests(rcOperation: () => Promise<RemoteConfigTemplate | ListVersionsResult>): void {
function runErrorResponseTests(
rcOperation: () => Promise<RemoteConfigTemplate | RemoteConfigServerTemplateData | ListVersionsResult>): void {
it('should reject when a full platform error response is received', () => {
const stub = sinon
.stub(HttpClient.prototype, 'send')
Expand Down