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 4 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
48 changes: 45 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,16 @@ 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';
// Honors env param to enable URL override independent of binary change.
erikeldridge marked this conversation as resolved.
Show resolved Hide resolved
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 +172,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 +215,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 +279,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
80 changes: 79 additions & 1 deletion src/remote-config/remote-config-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,27 @@ export interface RemoteConfigCondition {
tagColor?: TagColor;
}

/**
* Interface representing a Remote Config condition in the data-plane.
erikeldridge marked this conversation as resolved.
Show resolved Hide resolved
* 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.

export interface RemoteConfigServerCondition {

/**
* A non-empty and unique name of this condition.
*/
name: string;

/**
* The logic of this condition.
* See the documentation on
* {@link https://firebase.google.com/docs/remote-config/condition-reference | condition expressions}
* for the expected syntax of this field.
*/
expression: string;
}

/**
* Interface representing an explicit parameter value.
*/
Expand Down Expand Up @@ -135,7 +156,7 @@ export interface RemoteConfigParameterGroup {
}

/**
* Interface representing a Remote Config template.
* Interface representing a Remote Config client template.
erikeldridge marked this conversation as resolved.
Show resolved Hide resolved
*/
export interface RemoteConfigTemplate {
/**
Expand Down Expand Up @@ -167,6 +188,58 @@ export interface RemoteConfigTemplate {
version?: Version;
}

/**
* Interface representing the data in a Remote Config server template.
erikeldridge marked this conversation as resolved.
Show resolved Hide resolved
*/
export interface RemoteConfigServerTemplateData {
/**
* A list of conditions in descending order by priority.
*/
conditions: RemoteConfigServerCondition[];

/**
* Map of parameter keys to their optional default values and optional conditional values.
*/
parameters: { [key: string]: RemoteConfigParameter };

/**
* ETag of the current Remote Config template (readonly).
erikeldridge marked this conversation as resolved.
Show resolved Hide resolved
*/
readonly etag: string;

/**
* Version information for the current Remote Config template.
*/
version?: Version;
}

/**
* Interface representing a stateful abstraction for a Remote Config server template.
erikeldridge marked this conversation as resolved.
Show resolved Hide resolved
*/
export interface RemoteConfigServerTemplate {

/**
* Cached {@link RemoteConfigServerTemplateData}
erikeldridge marked this conversation as resolved.
Show resolved Hide resolved
*/
cache: RemoteConfigServerTemplateData;

/**
* A {@link RemoteConfigServerConfig} containing default values for Config
erikeldridge marked this conversation as resolved.
Show resolved Hide resolved
*/
defaultConfig: RemoteConfigServerConfig;

/**
* Evaluates the current template to produce a {@link RemoteConfigServerConfig}
erikeldridge marked this conversation as resolved.
Show resolved Hide resolved
*/
evaluate(): RemoteConfigServerConfig;

/**
* Fetches and caches the current active version of the
* {@link RemoteConfigServerTemplate} of the project.
erikeldridge marked this conversation as resolved.
Show resolved Hide resolved
*/
load(): Promise<void>;
}

/**
* Interface representing a Remote Config user.
*/
Expand Down Expand Up @@ -289,3 +362,8 @@ export interface ListVersionsOptions {
*/
endTime?: Date | string;
}

/**
* Type representing the configuration produced by evaluating a server template.
erikeldridge marked this conversation as resolved.
Show resolved Hide resolved
*/
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