From dba1ba60165eff82739d327e9c4d189e3b671b12 Mon Sep 17 00:00:00 2001 From: Lahiru Maramba Date: Fri, 21 Feb 2020 14:27:58 -0500 Subject: [PATCH 1/7] Implement RC get and validate tempalte methods --- src/remote-config/remote-config.ts | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/remote-config/remote-config.ts b/src/remote-config/remote-config.ts index a73014a9d9..a87e82f2c0 100644 --- a/src/remote-config/remote-config.ts +++ b/src/remote-config/remote-config.ts @@ -16,6 +16,9 @@ import { FirebaseServiceInterface, FirebaseServiceInternalsInterface } from '../firebase-service'; import { FirebaseApp } from '../firebase-app'; +import { + RemoteConfigApiClient +} from './remote-config-api-client'; /** Interface representing a Remote Config parameter. */ export interface RemoteConfigParameter { @@ -56,11 +59,24 @@ class RemoteConfigInternals implements FirebaseServiceInternalsInterface { export class RemoteConfig implements FirebaseServiceInterface { public readonly INTERNAL: RemoteConfigInternals = new RemoteConfigInternals(); + private readonly client: RemoteConfigApiClient; + /** * @param {FirebaseApp} app The app for this RemoteConfig service. * @constructor */ - constructor(readonly app: FirebaseApp) { } + constructor(readonly app: FirebaseApp) { + this.client = new RemoteConfigApiClient(app); + } + + /** + * Gets the current active version of the Remote Config template of the project. + * + * @return {Promise} A Promise that fulfills when the template is available. + */ + public getTemplate(): Promise { + return Promise.resolve(new RemoteConfigTemplate()); + } } /** From e719d9b0d5c4c694653d37731464d13bcc4aa74c Mon Sep 17 00:00:00 2001 From: Lahiru Maramba Date: Fri, 28 Feb 2020 20:14:11 -0500 Subject: [PATCH 2/7] Add getTemplate() --- src/remote-config/remote-config-api-client.ts | 169 ++++++++++++- src/remote-config/remote-config-utils.ts | 2 + src/remote-config/remote-config.ts | 65 +++-- test/unit/index.spec.ts | 4 + .../remote-config-api-client.spec.ts | 161 ++++++++++++ test/unit/remote-config/remote-config.spec.ts | 231 ++++++++++++++++++ 6 files changed, 613 insertions(+), 19 deletions(-) create mode 100644 test/unit/remote-config/remote-config-api-client.spec.ts create mode 100644 test/unit/remote-config/remote-config.spec.ts diff --git a/src/remote-config/remote-config-api-client.ts b/src/remote-config/remote-config-api-client.ts index 53f69435f5..3152284d8e 100644 --- a/src/remote-config/remote-config-api-client.ts +++ b/src/remote-config/remote-config-api-client.ts @@ -14,10 +14,75 @@ * limitations under the License. */ -import { FirebaseRemoteConfigError } from './remote-config-utils'; +import { HttpRequestConfig, HttpClient, HttpError, AuthorizedHttpClient } from '../utils/api-request'; +import { PrefixedFirebaseError } from '../utils/error'; +import { FirebaseRemoteConfigError, RemoteConfigErrorCode } from './remote-config-utils'; import { FirebaseApp } from '../firebase-app'; +import * as utils from '../utils/index'; import * as validator from '../utils/validator'; +const REMOTECONFIG_V1_API = 'https://firebaseremoteconfig.googleapis.com/v1'; +const FIREBASE_REMOTE_CONFIG_HEADERS = { + 'X-Firebase-Client': 'fire-admin-node/', + // There is a known issue in which the ETag is not properly returned in cases where the request + // does not specify a compression type. Currently, it is required to include the header + // `Accept-Encoding: gzip` or equivalent in all requests. + // https://firebase.google.com/docs/remote-config/use-config-rest#etag_usage_and_forced_updates + 'Accept-Encoding': 'gzip', +}; + +enum ConditionDisplayColor { + CONDITION_DISPLAY_COLOR_UNSPECIFIED = "Unspecified", + BLUE = "Blue", + BROWN = "Brown", + CYAN = "Cyan", + DEEP_ORANGE = "Red Orange", + GREEN = "Green", + INDIGO = "Indigo", + LIME = "Lime", + ORANGE = "Orange", + PINK = "Pink", + PURPLE = "Purple", + TEAL = "Teal", +} + +interface Version { + readonly versionNumber: string; + readonly updateTime: string; + readonly updateUser: { readonly name?: string; readonly email?: string; readonly imageUrl?: string }; + readonly updateOrigin: string; + readonly updateType: string; + readonly description?: string; + readonly rollbackSource?: string; + readonly isLegacy?: boolean; +} + +/** Interface representing a Remote Config parameter value. */ +export interface RemoteConfigParameterValue { + readonly value?: string; + readonly useInAppDefault?: boolean; +} + +/** Interface representing a Remote Config parameter. */ +export interface RemoteConfigParameter { + readonly defaultValue?: RemoteConfigParameterValue; + readonly conditionalValues?: { [key: string]: RemoteConfigParameterValue }; + readonly description?: string; +} + +interface RemoteConfigCondition { + name: string; + expression: string; + tagColor?: ConditionDisplayColor; +} + +export interface RemoteConfigResponse { + readonly conditions?: RemoteConfigCondition[]; + readonly parameters?: { [key: string]: RemoteConfigParameter }; + readonly version?: Version; //only undefined when there is no active template in the project + readonly eTag: string; +} + /** * Class that facilitates sending requests to the Firebase Remote Config backend API. * @@ -25,12 +90,108 @@ import * as validator from '../utils/validator'; */ export class RemoteConfigApiClient { - constructor(app: FirebaseApp) { + private readonly httpClient: HttpClient; + private projectIdPrefix?: string; + + constructor(private readonly app: FirebaseApp) { if (!validator.isNonNullObject(app) || !('options' in app)) { throw new FirebaseRemoteConfigError( 'invalid-argument', - 'First argument passed to admin.RemoteConfig() must be a valid Firebase app ' - + 'instance.'); + 'First argument passed to admin.remoteConfig() must be a valid Firebase app instance.'); + } + + this.httpClient = new AuthorizedHttpClient(app); + } + + public getTemplate(): Promise { + return this.getUrl() + .then((url) => { + const request: HttpRequestConfig = { + method: 'GET', + url: `${url}/remoteConfig`, + headers: FIREBASE_REMOTE_CONFIG_HEADERS + }; + return this.httpClient.send(request); + }) + .then((resp) => { + if (!Object.prototype.hasOwnProperty.call(resp.headers, 'etag')) { + throw new FirebaseRemoteConfigError( + 'invalid-argument', + 'ETag is unavailable'); + } + const remoteConfigResponse = resp.data as RemoteConfigResponse; + // Include the eTag in RemoteConfigResponse + (remoteConfigResponse as any).eTag = resp.headers['etag']; + return remoteConfigResponse; + }) + .catch((err) => { + throw this.toFirebaseError(err); + }); + } + + private getUrl(): Promise { + return this.getProjectIdPrefix() + .then((projectIdPrefix) => { + return `${REMOTECONFIG_V1_API}/${projectIdPrefix}`; + }); + } + + private getProjectIdPrefix(): Promise { + if (this.projectIdPrefix) { + return Promise.resolve(this.projectIdPrefix); + } + + return utils.findProjectId(this.app) + .then((projectId) => { + if (!validator.isNonEmptyString(projectId)) { + throw new FirebaseRemoteConfigError( + 'invalid-argument', + 'Failed to determine project ID. Initialize the SDK with service account credentials, or ' + + 'set project ID as an app option. Alternatively, set the GOOGLE_CLOUD_PROJECT ' + + 'environment variable.'); + } + + this.projectIdPrefix = `projects/${projectId}`; + return this.projectIdPrefix; + }); + } + + private toFirebaseError(err: HttpError): PrefixedFirebaseError { + if (err instanceof PrefixedFirebaseError) { + return err; } + + const response = err.response; + if (!response.isJson()) { + return new FirebaseRemoteConfigError( + 'unknown-error', + `Unexpected response with status: ${response.status} and body: ${response.text}`); + } + + const error: Error = (response.data as ErrorResponse).error || {}; + let code: RemoteConfigErrorCode = 'unknown-error'; + if (error.status && error.status in ERROR_CODE_MAPPING) { + code = ERROR_CODE_MAPPING[error.status]; + } + const message = error.message || `Unknown server error: ${response.text}`; + return new FirebaseRemoteConfigError(code, message); } } + +interface ErrorResponse { + error?: Error; +} + +interface Error { + code?: number; + message?: string; + status?: string; +} + +const ERROR_CODE_MAPPING: { [key: string]: RemoteConfigErrorCode } = { + INVALID_ARGUMENT: 'invalid-argument', + NOT_FOUND: 'not-found', + RESOURCE_EXHAUSTED: 'resource-exhausted', + UNAUTHENTICATED: 'authentication-error', + UNKNOWN: 'unknown-error', +}; diff --git a/src/remote-config/remote-config-utils.ts b/src/remote-config/remote-config-utils.ts index 3debfd5a37..bf8b808e25 100644 --- a/src/remote-config/remote-config-utils.ts +++ b/src/remote-config/remote-config-utils.ts @@ -23,8 +23,10 @@ export type RemoteConfigErrorCode = | 'invalid-argument' | 'invalid-etag' | 'invalid-template' + | 'not-found' | 'obsolete-etag' | 'permission-denied' + | 'resource-exhausted' | 'unauthenticated' | 'unknown-error'; diff --git a/src/remote-config/remote-config.ts b/src/remote-config/remote-config.ts index a87e82f2c0..7277296dd1 100644 --- a/src/remote-config/remote-config.ts +++ b/src/remote-config/remote-config.ts @@ -16,21 +16,14 @@ import { FirebaseServiceInterface, FirebaseServiceInternalsInterface } from '../firebase-service'; import { FirebaseApp } from '../firebase-app'; +import * as validator from '../utils/validator'; +import { FirebaseRemoteConfigError } from './remote-config-utils'; import { - RemoteConfigApiClient + RemoteConfigApiClient, + RemoteConfigResponse, + RemoteConfigParameter } from './remote-config-api-client'; -/** Interface representing a Remote Config parameter. */ -export interface RemoteConfigParameter { - key: string; - defaultValue?: string; // If `undefined`, the parameter uses the in-app default value - description?: string; - - // A dictionary of {conditionName: value} - // `undefined` value sets `useInAppDefault` to `true` (equivalent to `No Value`) - conditionalValues?: { [name: string]: string | undefined }; -} - /** Interface representing a Remote Config condition. */ export interface RemoteConfigCondition { name: string; @@ -75,7 +68,10 @@ export class RemoteConfig implements FirebaseServiceInterface { * @return {Promise} A Promise that fulfills when the template is available. */ public getTemplate(): Promise { - return Promise.resolve(new RemoteConfigTemplate()); + return this.client.getTemplate() + .then((templateResponse) => { + return new RemoteConfigTemplate(templateResponse); + }); } } @@ -84,10 +80,41 @@ export class RemoteConfig implements FirebaseServiceInterface { */ export class RemoteConfigTemplate { - public parameters: RemoteConfigParameter[]; + public parameters: { [key: string]: RemoteConfigParameter }; public conditions: RemoteConfigCondition[]; private readonly eTagInternal: string; + constructor(config: RemoteConfigResponse) { + if (!validator.isNonNullObject(config) || + !validator.isNonEmptyString(config.eTag)) { + throw new FirebaseRemoteConfigError( + 'invalid-argument', + `Invalid Remote Config template response: ${JSON.stringify(config)}`); + } + + this.parameters = {}; + this.conditions = []; + this.eTagInternal = config.eTag; + + if (typeof config.parameters !== 'undefined') { + if (!validator.isNonNullObject(config.parameters)) { + throw new FirebaseRemoteConfigError( + 'invalid-argument', + `Remote Config parameters must be a non-null object`); + } + this.parameters = config.parameters; + } + + if (typeof config.conditions !== 'undefined') { + if (!validator.isArray(config.conditions)) { + throw new FirebaseRemoteConfigError( + 'invalid-argument', + `Remote Config conditions must be an array`); + } + this.conditions = this.parseConditions(config.conditions); + } + } + /** * Gets the ETag of the template. * @@ -105,7 +132,7 @@ export class RemoteConfigTemplate { * @return {RemoteConfigParameter} The Remote Config parameter with the provided key. */ public getParameter(key: string): RemoteConfigParameter | undefined { - return this.parameters.find((p) => p.key === key); + return this.parameters[key]; } /** @@ -127,4 +154,12 @@ export class RemoteConfigTemplate { eTag: this.eTag, }; } + + private parseConditions(conditions: any[]): RemoteConfigCondition[] { + return conditions.map(p => ({ + name: p.name, + expression: p.expression, + color: p.tagColor + })); + } } diff --git a/test/unit/index.spec.ts b/test/unit/index.spec.ts index 6bd00c36d4..d58e06efef 100755 --- a/test/unit/index.spec.ts +++ b/test/unit/index.spec.ts @@ -64,3 +64,7 @@ import './project-management/ios-app.spec'; // SecurityRules import './security-rules/security-rules.spec'; import './security-rules/security-rules-api-client.spec'; + +// RemoteConfig +import './remote-config/remote-config.spec'; +import './remote-config/remote-config-api-client.spec'; diff --git a/test/unit/remote-config/remote-config-api-client.spec.ts b/test/unit/remote-config/remote-config-api-client.spec.ts new file mode 100644 index 0000000000..d1b439743f --- /dev/null +++ b/test/unit/remote-config/remote-config-api-client.spec.ts @@ -0,0 +1,161 @@ +/*! + * Copyright 2020 Google Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +'use strict'; + +import * as _ from 'lodash'; +import * as chai from 'chai'; +import * as sinon from 'sinon'; +import { RemoteConfigApiClient } from '../../../src/remote-config/remote-config-api-client'; +import { FirebaseRemoteConfigError } from '../../../src/remote-config/remote-config-utils'; +import { HttpClient } from '../../../src/utils/api-request'; +import * as utils from '../utils'; +import * as mocks from '../../resources/mocks'; +import { FirebaseAppError } from '../../../src/utils/error'; +import { FirebaseApp } from '../../../src/firebase-app'; + +const expect = chai.expect; + +describe('RemoteConfigApiClient', () => { + + const ERROR_RESPONSE = { + error: { + code: 404, + message: 'Requested entity not found', + status: 'NOT_FOUND', + }, + }; + const EXPECTED_HEADERS = { + 'Authorization': 'Bearer mock-token', + 'X-Firebase-Client': 'fire-admin-node/', + 'Accept-Encoding': 'gzip', + }; + const noProjectId = 'Failed to determine project ID. Initialize the SDK with service ' + + 'account credentials, or set project ID as an app option. Alternatively, set the ' + + 'GOOGLE_CLOUD_PROJECT environment variable.'; + + const mockOptions = { + credential: new mocks.MockCredential(), + projectId: 'test-project', + }; + + const clientWithoutProjectId = new RemoteConfigApiClient( + mocks.mockCredentialApp()); + + // Stubs used to simulate underlying api calls. + let stubs: sinon.SinonStub[] = []; + let app: FirebaseApp; + let apiClient: RemoteConfigApiClient; + + beforeEach(() => { + app = mocks.appWithOptions(mockOptions); + apiClient = new RemoteConfigApiClient(app); + }); + + afterEach(() => { + _.forEach(stubs, (stub) => stub.restore()); + stubs = []; + return app.delete(); + }); + + describe('Constructor', () => { + it('should throw when the app is null', () => { + expect(() => new RemoteConfigApiClient(null as unknown as FirebaseApp)) + .to.throw('First argument passed to admin.remoteConfig() must be a valid Firebase app instance.'); + }); + }); + + describe('getTemplate', () => { + it(`should reject when project id is not available`, () => { + return clientWithoutProjectId.getTemplate() + .should.eventually.be.rejectedWith(noProjectId); + }); + + it('should resolve with the requested template on success', () => { + const stub = sinon + .stub(HttpClient.prototype, 'send') + .resolves(utils.responseFrom({ + conditions: [{ name: 'ios', expression: 'exp' }], + parameters: { param: { defaultValue: { value: 'true' } } }, + version: {}, + }, 200, { etag: 'etag-123456789012-1' })); + stubs.push(stub); + return apiClient.getTemplate() + .then((resp) => { + expect(resp.conditions).to.deep.equal([{ name: 'ios', expression: 'exp' }]); + expect(resp.parameters).to.deep.equal({ param: { defaultValue: { value: 'true' } } }); + expect(stub).to.have.been.calledOnce.and.calledWith({ + method: 'GET', + url: 'https://firebaseremoteconfig.googleapis.com/v1/projects/test-project/remoteConfig', + headers: EXPECTED_HEADERS, + }); + }); + }); + + it('should throw when the etag does not present in the headers', () => { + const stub = sinon + .stub(HttpClient.prototype, 'send') + .resolves(utils.responseFrom({ + conditions: [], parameters: {}, version: {}, + })); + stubs.push(stub); + const expected = new FirebaseRemoteConfigError('invalid-argument', 'ETag is unavailable'); + return apiClient.getTemplate() + .should.eventually.be.rejected.and.deep.equal(expected); + }); + + it('should throw when a full platform error response is received', () => { + const stub = sinon + .stub(HttpClient.prototype, 'send') + .rejects(utils.errorFrom(ERROR_RESPONSE, 404)); + stubs.push(stub); + const expected = new FirebaseRemoteConfigError('not-found', 'Requested entity not found'); + return apiClient.getTemplate() + .should.eventually.be.rejected.and.deep.equal(expected); + }); + + it('should throw unknown-error when error code is not present', () => { + const stub = sinon + .stub(HttpClient.prototype, 'send') + .rejects(utils.errorFrom({}, 404)); + stubs.push(stub); + const expected = new FirebaseRemoteConfigError('unknown-error', 'Unknown server error: {}'); + return apiClient.getTemplate() + .should.eventually.be.rejected.and.deep.equal(expected); + }); + + it('should throw unknown-error for non-json response', () => { + const stub = sinon + .stub(HttpClient.prototype, 'send') + .rejects(utils.errorFrom('not json', 404)); + stubs.push(stub); + const expected = new FirebaseRemoteConfigError( + 'unknown-error', 'Unexpected response with status: 404 and body: not json'); + return apiClient.getTemplate() + .should.eventually.be.rejected.and.deep.equal(expected); + }); + + it('should throw when rejected with a FirebaseAppError', () => { + const expected = new FirebaseAppError('network-error', 'socket hang up'); + const stub = sinon + .stub(HttpClient.prototype, 'send') + .rejects(expected); + stubs.push(stub); + return apiClient.getTemplate() + .should.eventually.be.rejected.and.deep.equal(expected); + }); + }); +}); diff --git a/test/unit/remote-config/remote-config.spec.ts b/test/unit/remote-config/remote-config.spec.ts new file mode 100644 index 0000000000..ac700a8d6e --- /dev/null +++ b/test/unit/remote-config/remote-config.spec.ts @@ -0,0 +1,231 @@ +/*! + * Copyright 2020 Google Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +'use strict'; + +import * as _ from 'lodash'; +import * as chai from 'chai'; +import * as sinon from 'sinon'; +import { RemoteConfig } from '../../../src/remote-config/remote-config'; +import { FirebaseApp } from '../../../src/firebase-app'; +import * as mocks from '../../resources/mocks'; +import { RemoteConfigApiClient } from '../../../src/remote-config/remote-config-api-client'; +import { FirebaseRemoteConfigError } from '../../../src/remote-config/remote-config-utils'; +import { deepCopy } from '../../../src/utils/deep-copy'; + +const expect = chai.expect; + +describe('RemoteConfig', () => { + + const INTERNAL_ERROR = new FirebaseRemoteConfigError('internal-error', 'message'); + const REMOTE_CONFIG_RESPONSE: { + // This type is effectively a RemoteConfigResponse, but with non-readonly fields + // to allow easier use from within the tests. An improvement would be to + // alter this into a helper that creates customized RemoteConfigResponse based + // on the needs of the test, as that would ensure type-safety. + conditions?: Array<{ name: string; expression: string; tagColor: string }>; + parameters?: object | null; + version?: object; + eTag: string; + } = { + conditions: [{ + name: 'ios', + expression: 'device.os == \'ios\'', + tagColor: 'BLUE', + }], + parameters: { + // eslint-disable-next-line @typescript-eslint/camelcase + holiday_promo_enabled: { + defaultValue: { value: 'true' }, + conditionalValues: { ios: { useInAppDefault: true } }, + description: 'this is a promo', + }, + }, + version: { + versionNumber: '5', + updateTime: '2020-02-26T20:33:43.300Z', + updateUser: { email: 'user@user.com' }, + updateOrigin: 'CONSOLE', + updateType: 'INCREMENTAL_UPDATE', + }, + eTag: 'etag-123456789012-5', + }; + + let remoteConfig: RemoteConfig; + let mockApp: FirebaseApp; + let mockCredentialApp: FirebaseApp; + + // Stubs used to simulate underlying api calls. + let stubs: sinon.SinonStub[] = []; + + before(() => { + mockApp = mocks.app(); + mockCredentialApp = mocks.mockCredentialApp(); + remoteConfig = new RemoteConfig(mockApp); + }); + + after(() => { + return mockApp.delete(); + }); + + afterEach(() => { + _.forEach(stubs, (stub) => stub.restore()); + stubs = []; + }); + + describe('Constructor', () => { + const invalidApps = [null, NaN, 0, 1, true, false, '', 'a', [], [1, 'a'], {}, { a: 1 }, _.noop]; + invalidApps.forEach((invalidApp) => { + it('should throw given invalid app: ' + JSON.stringify(invalidApp), () => { + expect(() => { + const remoteConfigAny: any = RemoteConfig; + return new remoteConfigAny(invalidApp); + }).to.throw( + 'First argument passed to admin.remoteConfig() must be a valid Firebase app ' + + 'instance.'); + }); + }); + + it('should throw given no app', () => { + expect(() => { + const remoteConfigAny: any = RemoteConfig; + return new remoteConfigAny(); + }).to.throw( + 'First argument passed to admin.remoteConfig() must be a valid Firebase app ' + + 'instance.'); + }); + + it('should reject when initialized without project ID', () => { + // Project ID not set in the environment. + delete process.env.GOOGLE_CLOUD_PROJECT; + delete process.env.GCLOUD_PROJECT; + const noProjectId = 'Failed to determine project ID. Initialize the SDK with service ' + + 'account credentials, or set project ID as an app option. Alternatively, set the ' + + 'GOOGLE_CLOUD_PROJECT environment variable.'; + const remoteConfigWithoutProjectId = new RemoteConfig(mockCredentialApp); + return remoteConfigWithoutProjectId.getTemplate() + .should.eventually.rejectedWith(noProjectId); + }); + + it('should not throw given a valid app', () => { + expect(() => { + return new RemoteConfig(mockApp); + }).not.to.throw(); + }); + }); + + describe('app', () => { + it('returns the app from the constructor', () => { + // We expect referential equality here + expect(remoteConfig.app).to.equal(mockApp); + }); + }); + + describe('getTemplate', () => { + it('should propagate API errors', () => { + const stub = sinon + .stub(RemoteConfigApiClient.prototype, 'getTemplate') + .rejects(INTERNAL_ERROR); + stubs.push(stub); + return remoteConfig.getTemplate() + .should.eventually.be.rejected.and.deep.equal(INTERNAL_ERROR); + }); + + it('should reject when API response is invalid', () => { + const stub = sinon + .stub(RemoteConfigApiClient.prototype, 'getTemplate') + .resolves(null); + stubs.push(stub); + return remoteConfig.getTemplate() + .should.eventually.be.rejected.and.have.property( + 'message', 'Invalid Remote Config template response: null'); + }); + + it('should reject when API response does not contain an ETag', () => { + const response = deepCopy(REMOTE_CONFIG_RESPONSE); + response.eTag = ''; + const stub = sinon + .stub(RemoteConfigApiClient.prototype, 'getTemplate') + .resolves(response); + stubs.push(stub); + return remoteConfig.getTemplate() + .should.eventually.be.rejected.and.have.property( + 'message', `Invalid Remote Config template response: ${JSON.stringify(response)}`); + }); + + it('should reject when API response does not contain valid parameters', () => { + const response = deepCopy(REMOTE_CONFIG_RESPONSE); + response.parameters = null; + const stub = sinon + .stub(RemoteConfigApiClient.prototype, 'getTemplate') + .resolves(response); + stubs.push(stub); + return remoteConfig.getTemplate() + .should.eventually.be.rejected.and.have.property( + 'message', `Remote Config parameters must be a non-null object`); + }); + + it('should reject when API response does not contain valid conditions', () => { + const response = deepCopy(REMOTE_CONFIG_RESPONSE); + response.conditions = Object(); + const stub = sinon + .stub(RemoteConfigApiClient.prototype, 'getTemplate') + .resolves(response); + stubs.push(stub); + return remoteConfig.getTemplate() + .should.eventually.be.rejected.and.have.property( + 'message', `Remote Config conditions must be an array`); + }); + + it('should resolve with Remote Config template on success', () => { + const stub = sinon + .stub(RemoteConfigApiClient.prototype, 'getTemplate') + .resolves(REMOTE_CONFIG_RESPONSE); + stubs.push(stub); + + return remoteConfig.getTemplate() + .then((template) => { + expect(template.conditions.length).to.equal(1); + expect(template.conditions[0].name).to.equal('ios'); + expect(template.conditions[0].expression).to.equal('device.os == \'ios\''); + // verify the property mapping in RemoteConfigCondition from `tagColor` to `color` + expect(template.conditions[0].color).to.equal('BLUE'); + + expect(template.eTag).to.equal('etag-123456789012-5'); + + const key = 'holiday_promo_enabled'; + const p1 = template.parameters[key]; + expect(p1.defaultValue).deep.equals({ value: 'true' }); + expect(p1.conditionalValues).deep.equals({ ios: { useInAppDefault: true } }); + expect(p1.description).equals('this is a promo'); + + const p2 = template.getParameter(key); + if (p2) { + expect(p2.defaultValue).deep.equals({ value: 'true' }); + expect(p2.conditionalValues).deep.equals({ ios: { useInAppDefault: true } }); + expect(p2.description).equals('this is a promo'); + } + + const c = template.getCondition('ios'); + if (c) { + expect(c.name).to.equal('ios'); + expect(c.expression).to.equal('device.os == \'ios\''); + expect(c.color).to.equal('BLUE'); + } + }); + }); + }); +}); From 3132370b106a89688a884e6920594f2d2e09ef79 Mon Sep 17 00:00:00 2001 From: Lahiru Maramba Date: Wed, 4 Mar 2020 11:22:07 -0500 Subject: [PATCH 3/7] PR Fixes --- src/remote-config/remote-config-api-client.ts | 40 +++++++++---------- src/remote-config/remote-config.ts | 25 +++--------- .../remote-config-api-client.spec.ts | 35 ++++++++-------- test/unit/remote-config/remote-config.spec.ts | 15 +------ 4 files changed, 42 insertions(+), 73 deletions(-) diff --git a/src/remote-config/remote-config-api-client.ts b/src/remote-config/remote-config-api-client.ts index 3152284d8e..60e302b607 100644 --- a/src/remote-config/remote-config-api-client.ts +++ b/src/remote-config/remote-config-api-client.ts @@ -21,7 +21,7 @@ import { FirebaseApp } from '../firebase-app'; import * as utils from '../utils/index'; import * as validator from '../utils/validator'; -const REMOTECONFIG_V1_API = 'https://firebaseremoteconfig.googleapis.com/v1'; +const REMOTE_CONFIG_V1_API = 'https://firebaseremoteconfig.googleapis.com/v1'; const FIREBASE_REMOTE_CONFIG_HEADERS = { 'X-Firebase-Client': 'fire-admin-node/', // There is a known issue in which the ETag is not properly returned in cases where the request @@ -32,7 +32,7 @@ const FIREBASE_REMOTE_CONFIG_HEADERS = { }; enum ConditionDisplayColor { - CONDITION_DISPLAY_COLOR_UNSPECIFIED = "Unspecified", + UNSPECIFIED = "Unspecified", BLUE = "Blue", BROWN = "Brown", CYAN = "Cyan", @@ -46,23 +46,18 @@ enum ConditionDisplayColor { TEAL = "Teal", } -interface Version { - readonly versionNumber: string; - readonly updateTime: string; - readonly updateUser: { readonly name?: string; readonly email?: string; readonly imageUrl?: string }; - readonly updateOrigin: string; - readonly updateType: string; - readonly description?: string; - readonly rollbackSource?: string; - readonly isLegacy?: boolean; +/** Interface representing a Remote Config parameter `value` in value options. */ +export interface ExplicitParameterValue { + readonly value: string; } -/** Interface representing a Remote Config parameter value. */ -export interface RemoteConfigParameterValue { - readonly value?: string; - readonly useInAppDefault?: boolean; +/** Interface representing a Remote Config parameter `useInAppDefault` in value options. */ +export interface InAppDefaultValue { + readonly useInAppDefault: boolean; } +export type RemoteConfigParameterValue = ExplicitParameterValue | InAppDefaultValue; + /** Interface representing a Remote Config parameter. */ export interface RemoteConfigParameter { readonly defaultValue?: RemoteConfigParameterValue; @@ -79,7 +74,6 @@ interface RemoteConfigCondition { export interface RemoteConfigResponse { readonly conditions?: RemoteConfigCondition[]; readonly parameters?: { [key: string]: RemoteConfigParameter }; - readonly version?: Version; //only undefined when there is no active template in the project readonly eTag: string; } @@ -117,11 +111,13 @@ export class RemoteConfigApiClient { if (!Object.prototype.hasOwnProperty.call(resp.headers, 'etag')) { throw new FirebaseRemoteConfigError( 'invalid-argument', - 'ETag is unavailable'); + 'ETag header is not present in the server response.'); + } + const remoteConfigResponse: RemoteConfigResponse = { + conditions: resp.data.conditions, + parameters: resp.data.parameters, + eTag: resp.headers['etag'], } - const remoteConfigResponse = resp.data as RemoteConfigResponse; - // Include the eTag in RemoteConfigResponse - (remoteConfigResponse as any).eTag = resp.headers['etag']; return remoteConfigResponse; }) .catch((err) => { @@ -132,7 +128,7 @@ export class RemoteConfigApiClient { private getUrl(): Promise { return this.getProjectIdPrefix() .then((projectIdPrefix) => { - return `${REMOTECONFIG_V1_API}/${projectIdPrefix}`; + return `${REMOTE_CONFIG_V1_API}/${projectIdPrefix}`; }); } @@ -145,7 +141,7 @@ export class RemoteConfigApiClient { .then((projectId) => { if (!validator.isNonEmptyString(projectId)) { throw new FirebaseRemoteConfigError( - 'invalid-argument', + 'unknown-error', 'Failed to determine project ID. Initialize the SDK with service account credentials, or ' + 'set project ID as an app option. Alternatively, set the GOOGLE_CLOUD_PROJECT ' + 'environment variable.'); diff --git a/src/remote-config/remote-config.ts b/src/remote-config/remote-config.ts index 7277296dd1..fe3be69a01 100644 --- a/src/remote-config/remote-config.ts +++ b/src/remote-config/remote-config.ts @@ -111,7 +111,11 @@ export class RemoteConfigTemplate { 'invalid-argument', `Remote Config conditions must be an array`); } - this.conditions = this.parseConditions(config.conditions); + this.conditions = config.conditions.map(p => ({ + name: p.name, + expression: p.expression, + color: p.tagColor + })); } } @@ -124,17 +128,6 @@ export class RemoteConfigTemplate { return this.eTagInternal; } - /** - * Find an existing Remote Config parameter by key. - * - * @param {string} key The key of the Remote Config parameter. - * - * @return {RemoteConfigParameter} The Remote Config parameter with the provided key. - */ - public getParameter(key: string): RemoteConfigParameter | undefined { - return this.parameters[key]; - } - /** * Find an existing Remote Config condition by name. * @@ -154,12 +147,4 @@ export class RemoteConfigTemplate { eTag: this.eTag, }; } - - private parseConditions(conditions: any[]): RemoteConfigCondition[] { - return conditions.map(p => ({ - name: p.name, - expression: p.expression, - color: p.tagColor - })); - } } diff --git a/test/unit/remote-config/remote-config-api-client.spec.ts b/test/unit/remote-config/remote-config-api-client.spec.ts index d1b439743f..78b8efa80e 100644 --- a/test/unit/remote-config/remote-config-api-client.spec.ts +++ b/test/unit/remote-config/remote-config-api-client.spec.ts @@ -72,13 +72,19 @@ describe('RemoteConfigApiClient', () => { }); describe('Constructor', () => { - it('should throw when the app is null', () => { + it('should reject when the app is null', () => { expect(() => new RemoteConfigApiClient(null as unknown as FirebaseApp)) .to.throw('First argument passed to admin.remoteConfig() must be a valid Firebase app instance.'); }); }); describe('getTemplate', () => { + const testResponse = { + conditions: [{ name: 'ios', expression: 'exp' }], + parameters: { param: { defaultValue: { value: 'true' } } }, + version: {}, + }; + it(`should reject when project id is not available`, () => { return clientWithoutProjectId.getTemplate() .should.eventually.be.rejectedWith(noProjectId); @@ -87,16 +93,13 @@ describe('RemoteConfigApiClient', () => { it('should resolve with the requested template on success', () => { const stub = sinon .stub(HttpClient.prototype, 'send') - .resolves(utils.responseFrom({ - conditions: [{ name: 'ios', expression: 'exp' }], - parameters: { param: { defaultValue: { value: 'true' } } }, - version: {}, - }, 200, { etag: 'etag-123456789012-1' })); + .resolves(utils.responseFrom(testResponse, 200, { etag: 'etag-123456789012-1' })); stubs.push(stub); return apiClient.getTemplate() .then((resp) => { - expect(resp.conditions).to.deep.equal([{ name: 'ios', expression: 'exp' }]); - expect(resp.parameters).to.deep.equal({ param: { defaultValue: { value: 'true' } } }); + expect(resp.conditions).to.deep.equal(testResponse.conditions); + expect(resp.parameters).to.deep.equal(testResponse.parameters); + expect(resp.eTag).to.equal('etag-123456789012-1'); expect(stub).to.have.been.calledOnce.and.calledWith({ method: 'GET', url: 'https://firebaseremoteconfig.googleapis.com/v1/projects/test-project/remoteConfig', @@ -105,19 +108,17 @@ describe('RemoteConfigApiClient', () => { }); }); - it('should throw when the etag does not present in the headers', () => { + it('should reject when the etag is not present', () => { const stub = sinon .stub(HttpClient.prototype, 'send') - .resolves(utils.responseFrom({ - conditions: [], parameters: {}, version: {}, - })); + .resolves(utils.responseFrom(testResponse)); stubs.push(stub); - const expected = new FirebaseRemoteConfigError('invalid-argument', 'ETag is unavailable'); + const expected = new FirebaseRemoteConfigError('invalid-argument', 'ETag header is not present in the server response.'); return apiClient.getTemplate() .should.eventually.be.rejected.and.deep.equal(expected); }); - it('should throw when a full platform error response is received', () => { + it('should reject when a full platform error response is received', () => { const stub = sinon .stub(HttpClient.prototype, 'send') .rejects(utils.errorFrom(ERROR_RESPONSE, 404)); @@ -127,7 +128,7 @@ describe('RemoteConfigApiClient', () => { .should.eventually.be.rejected.and.deep.equal(expected); }); - it('should throw unknown-error when error code is not present', () => { + it('should reject unknown-error when error code is not present', () => { const stub = sinon .stub(HttpClient.prototype, 'send') .rejects(utils.errorFrom({}, 404)); @@ -137,7 +138,7 @@ describe('RemoteConfigApiClient', () => { .should.eventually.be.rejected.and.deep.equal(expected); }); - it('should throw unknown-error for non-json response', () => { + it('should reject unknown-error for non-json response', () => { const stub = sinon .stub(HttpClient.prototype, 'send') .rejects(utils.errorFrom('not json', 404)); @@ -148,7 +149,7 @@ describe('RemoteConfigApiClient', () => { .should.eventually.be.rejected.and.deep.equal(expected); }); - it('should throw when rejected with a FirebaseAppError', () => { + it('should reject when rejected with a FirebaseAppError', () => { const expected = new FirebaseAppError('network-error', 'socket hang up'); const stub = sinon .stub(HttpClient.prototype, 'send') diff --git a/test/unit/remote-config/remote-config.spec.ts b/test/unit/remote-config/remote-config.spec.ts index ac700a8d6e..94d91ef6c5 100644 --- a/test/unit/remote-config/remote-config.spec.ts +++ b/test/unit/remote-config/remote-config.spec.ts @@ -54,13 +54,6 @@ describe('RemoteConfig', () => { description: 'this is a promo', }, }, - version: { - versionNumber: '5', - updateTime: '2020-02-26T20:33:43.300Z', - updateUser: { email: 'user@user.com' }, - updateOrigin: 'CONSOLE', - updateType: 'INCREMENTAL_UPDATE', - }, eTag: 'etag-123456789012-5', }; @@ -212,14 +205,8 @@ describe('RemoteConfig', () => { expect(p1.conditionalValues).deep.equals({ ios: { useInAppDefault: true } }); expect(p1.description).equals('this is a promo'); - const p2 = template.getParameter(key); - if (p2) { - expect(p2.defaultValue).deep.equals({ value: 'true' }); - expect(p2.conditionalValues).deep.equals({ ios: { useInAppDefault: true } }); - expect(p2.description).equals('this is a promo'); - } - const c = template.getCondition('ios'); + expect(c).to.be.not.undefined; if (c) { expect(c.name).to.equal('ios'); expect(c.expression).to.equal('device.os == \'ios\''); From 51ccdc91ba180380cb0e7b580b0c88ab816aa4b5 Mon Sep 17 00:00:00 2001 From: Lahiru Maramba Date: Wed, 4 Mar 2020 11:25:04 -0500 Subject: [PATCH 4/7] PR Fixes --- src/remote-config/remote-config-api-client.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/remote-config/remote-config-api-client.ts b/src/remote-config/remote-config-api-client.ts index 60e302b607..0420fc895c 100644 --- a/src/remote-config/remote-config-api-client.ts +++ b/src/remote-config/remote-config-api-client.ts @@ -21,7 +21,8 @@ import { FirebaseApp } from '../firebase-app'; import * as utils from '../utils/index'; import * as validator from '../utils/validator'; -const REMOTE_CONFIG_V1_API = 'https://firebaseremoteconfig.googleapis.com/v1'; +// Remote Config backend constants +const FIREBASE_REMOTE_CONFIG_V1_API = 'https://firebaseremoteconfig.googleapis.com/v1'; const FIREBASE_REMOTE_CONFIG_HEADERS = { 'X-Firebase-Client': 'fire-admin-node/', // There is a known issue in which the ETag is not properly returned in cases where the request @@ -128,7 +129,7 @@ export class RemoteConfigApiClient { private getUrl(): Promise { return this.getProjectIdPrefix() .then((projectIdPrefix) => { - return `${REMOTE_CONFIG_V1_API}/${projectIdPrefix}`; + return `${FIREBASE_REMOTE_CONFIG_V1_API}/${projectIdPrefix}`; }); } From 95107beefc54146b8dd6cca10fa62860249ee10c Mon Sep 17 00:00:00 2001 From: Lahiru Maramba Date: Wed, 4 Mar 2020 20:11:30 -0500 Subject: [PATCH 5/7] Rename eTag to etag and add assertions to check etag is read-only --- src/remote-config/remote-config-api-client.ts | 4 ++-- src/remote-config/remote-config.ts | 12 +++++----- .../remote-config-api-client.spec.ts | 2 +- test/unit/remote-config/remote-config.spec.ts | 23 +++++++++++-------- 4 files changed, 22 insertions(+), 19 deletions(-) diff --git a/src/remote-config/remote-config-api-client.ts b/src/remote-config/remote-config-api-client.ts index 0420fc895c..9baf505b46 100644 --- a/src/remote-config/remote-config-api-client.ts +++ b/src/remote-config/remote-config-api-client.ts @@ -75,7 +75,7 @@ interface RemoteConfigCondition { export interface RemoteConfigResponse { readonly conditions?: RemoteConfigCondition[]; readonly parameters?: { [key: string]: RemoteConfigParameter }; - readonly eTag: string; + readonly etag: string; } /** @@ -117,7 +117,7 @@ export class RemoteConfigApiClient { const remoteConfigResponse: RemoteConfigResponse = { conditions: resp.data.conditions, parameters: resp.data.parameters, - eTag: resp.headers['etag'], + etag: resp.headers['etag'], } return remoteConfigResponse; }) diff --git a/src/remote-config/remote-config.ts b/src/remote-config/remote-config.ts index fe3be69a01..e445da7a9f 100644 --- a/src/remote-config/remote-config.ts +++ b/src/remote-config/remote-config.ts @@ -82,11 +82,11 @@ export class RemoteConfigTemplate { public parameters: { [key: string]: RemoteConfigParameter }; public conditions: RemoteConfigCondition[]; - private readonly eTagInternal: string; + private readonly etagInternal: string; constructor(config: RemoteConfigResponse) { if (!validator.isNonNullObject(config) || - !validator.isNonEmptyString(config.eTag)) { + !validator.isNonEmptyString(config.etag)) { throw new FirebaseRemoteConfigError( 'invalid-argument', `Invalid Remote Config template response: ${JSON.stringify(config)}`); @@ -94,7 +94,7 @@ export class RemoteConfigTemplate { this.parameters = {}; this.conditions = []; - this.eTagInternal = config.eTag; + this.etagInternal = config.etag; if (typeof config.parameters !== 'undefined') { if (!validator.isNonNullObject(config.parameters)) { @@ -124,8 +124,8 @@ export class RemoteConfigTemplate { * * @return {string} The ETag of the Remote Config template. */ - get eTag(): string { - return this.eTagInternal; + get etag(): string { + return this.etagInternal; } /** @@ -144,7 +144,7 @@ export class RemoteConfigTemplate { return { parameters: this.parameters, conditions: this.conditions, - eTag: this.eTag, + etag: this.etag, }; } } diff --git a/test/unit/remote-config/remote-config-api-client.spec.ts b/test/unit/remote-config/remote-config-api-client.spec.ts index 78b8efa80e..fed41f3bf7 100644 --- a/test/unit/remote-config/remote-config-api-client.spec.ts +++ b/test/unit/remote-config/remote-config-api-client.spec.ts @@ -99,7 +99,7 @@ describe('RemoteConfigApiClient', () => { .then((resp) => { expect(resp.conditions).to.deep.equal(testResponse.conditions); expect(resp.parameters).to.deep.equal(testResponse.parameters); - expect(resp.eTag).to.equal('etag-123456789012-1'); + expect(resp.etag).to.equal('etag-123456789012-1'); expect(stub).to.have.been.calledOnce.and.calledWith({ method: 'GET', url: 'https://firebaseremoteconfig.googleapis.com/v1/projects/test-project/remoteConfig', diff --git a/test/unit/remote-config/remote-config.spec.ts b/test/unit/remote-config/remote-config.spec.ts index 94d91ef6c5..e9ad0a1bc0 100644 --- a/test/unit/remote-config/remote-config.spec.ts +++ b/test/unit/remote-config/remote-config.spec.ts @@ -19,7 +19,7 @@ import * as _ from 'lodash'; import * as chai from 'chai'; import * as sinon from 'sinon'; -import { RemoteConfig } from '../../../src/remote-config/remote-config'; +import { RemoteConfig, RemoteConfigCondition } from '../../../src/remote-config/remote-config'; import { FirebaseApp } from '../../../src/firebase-app'; import * as mocks from '../../resources/mocks'; import { RemoteConfigApiClient } from '../../../src/remote-config/remote-config-api-client'; @@ -39,7 +39,7 @@ describe('RemoteConfig', () => { conditions?: Array<{ name: string; expression: string; tagColor: string }>; parameters?: object | null; version?: object; - eTag: string; + etag: string; } = { conditions: [{ name: 'ios', @@ -54,7 +54,7 @@ describe('RemoteConfig', () => { description: 'this is a promo', }, }, - eTag: 'etag-123456789012-5', + etag: 'etag-123456789012-5', }; let remoteConfig: RemoteConfig; @@ -149,7 +149,7 @@ describe('RemoteConfig', () => { it('should reject when API response does not contain an ETag', () => { const response = deepCopy(REMOTE_CONFIG_RESPONSE); - response.eTag = ''; + response.etag = ''; const stub = sinon .stub(RemoteConfigApiClient.prototype, 'getTemplate') .resolves(response); @@ -197,7 +197,11 @@ describe('RemoteConfig', () => { // verify the property mapping in RemoteConfigCondition from `tagColor` to `color` expect(template.conditions[0].color).to.equal('BLUE'); - expect(template.eTag).to.equal('etag-123456789012-5'); + expect(template.etag).to.equal('etag-123456789012-5'); + // verify that etag is read-only + expect(() => { + (template as any).etag = "new-etag"; + }).to.throw('Cannot set property etag of # which has only a getter'); const key = 'holiday_promo_enabled'; const p1 = template.parameters[key]; @@ -207,11 +211,10 @@ describe('RemoteConfig', () => { const c = template.getCondition('ios'); expect(c).to.be.not.undefined; - if (c) { - expect(c.name).to.equal('ios'); - expect(c.expression).to.equal('device.os == \'ios\''); - expect(c.color).to.equal('BLUE'); - } + const cond = c as RemoteConfigCondition; + expect(cond.name).to.equal('ios'); + expect(cond.expression).to.equal('device.os == \'ios\''); + expect(cond.color).to.equal('BLUE'); }); }); }); From 9122356abac0f1bef057a80ae927e72305de2f45 Mon Sep 17 00:00:00 2001 From: Lahiru Maramba Date: Wed, 4 Mar 2020 20:17:08 -0500 Subject: [PATCH 6/7] PR fixes --- src/remote-config/remote-config.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/remote-config/remote-config.ts b/src/remote-config/remote-config.ts index e445da7a9f..88234d1e3d 100644 --- a/src/remote-config/remote-config.ts +++ b/src/remote-config/remote-config.ts @@ -92,8 +92,6 @@ export class RemoteConfigTemplate { `Invalid Remote Config template response: ${JSON.stringify(config)}`); } - this.parameters = {}; - this.conditions = []; this.etagInternal = config.etag; if (typeof config.parameters !== 'undefined') { @@ -103,6 +101,8 @@ export class RemoteConfigTemplate { `Remote Config parameters must be a non-null object`); } this.parameters = config.parameters; + } else { + this.parameters = {}; } if (typeof config.conditions !== 'undefined') { @@ -116,6 +116,8 @@ export class RemoteConfigTemplate { expression: p.expression, color: p.tagColor })); + } else { + this.conditions = []; } } From 2b2afbe8d0d549a9f09fa4ea4e6418cd50d2bb7c Mon Sep 17 00:00:00 2001 From: Lahiru Maramba Date: Thu, 5 Mar 2020 16:50:34 -0500 Subject: [PATCH 7/7] PR fixes --- src/remote-config/remote-config-api-client.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/remote-config/remote-config-api-client.ts b/src/remote-config/remote-config-api-client.ts index 9baf505b46..0c8c99521a 100644 --- a/src/remote-config/remote-config-api-client.ts +++ b/src/remote-config/remote-config-api-client.ts @@ -114,12 +114,11 @@ export class RemoteConfigApiClient { 'invalid-argument', 'ETag header is not present in the server response.'); } - const remoteConfigResponse: RemoteConfigResponse = { + return { conditions: resp.data.conditions, parameters: resp.data.parameters, etag: resp.headers['etag'], - } - return remoteConfigResponse; + }; }) .catch((err) => { throw this.toFirebaseError(err);