From b1f0319ceb5ae3b7a6ad12b94c984005dd316391 Mon Sep 17 00:00:00 2001 From: Lahiru Maramba Date: Fri, 13 Mar 2020 12:26:49 -0400 Subject: [PATCH 1/5] Add publish template operation --- src/remote-config/remote-config-api-client.ts | 37 +++++++++++++++++++ src/remote-config/remote-config.ts | 15 ++++++++ 2 files changed, 52 insertions(+) diff --git a/src/remote-config/remote-config-api-client.ts b/src/remote-config/remote-config-api-client.ts index 89c52c36f9..dc5dcd02c8 100644 --- a/src/remote-config/remote-config-api-client.ts +++ b/src/remote-config/remote-config-api-client.ts @@ -160,6 +160,43 @@ export class RemoteConfigApiClient { }); } + public publishTemplate(template: RemoteConfigTemplate, options?: { force: boolean }): Promise { + let ifMatch: string = template.etag; + if (options.force == true) { + // setting `If-Match: *` forces the Remote Config template to be updated + // and circumvent the ETag, and the protection from that it provides. + ifMatch = '*'; + } + return this.getUrl() + .then((url) => { + const request: HttpRequestConfig = { + method: 'PUT', + url: `${url}/remoteConfig`, + headers: { ...FIREBASE_REMOTE_CONFIG_HEADERS, 'If-Match': ifMatch }, + data: { + conditions: template.conditions, + parameters: template.parameters, + } + }; + return this.httpClient.send(request); + }) + .then((resp) => { + if (!validator.isNonEmptyString(resp.headers['etag'])) { + throw new FirebaseRemoteConfigError( + 'invalid-argument', + 'ETag header is not present in the server response.'); + } + return { + conditions: resp.data.conditions, + parameters: resp.data.parameters, + etag: resp.headers['etag'], + }; + }) + .catch((err) => { + throw this.toFirebaseError(err); + }); + } + private getUrl(): Promise { return this.getProjectIdPrefix() .then((projectIdPrefix) => { diff --git a/src/remote-config/remote-config.ts b/src/remote-config/remote-config.ts index 0ac9b71068..853d6d757e 100644 --- a/src/remote-config/remote-config.ts +++ b/src/remote-config/remote-config.ts @@ -81,6 +81,21 @@ export class RemoteConfig implements FirebaseServiceInterface { return new RemoteConfigTemplateImpl(templateResponse); }); } + + /** + * Publishes a Remote Config template. + * + * @param {RemoteConfigTemplate} template The Remote Config template to be validated. + * @param {any=} options Optional options object when publishing a Remote Config template. + * + * @return {Promise} A Promise that fulfills when a template is published. + */ + public publishTemplate(template: RemoteConfigTemplate, options?: { force: boolean }): Promise { + return this.client.publishTemplate(template, options) + .then((templateResponse) => { + return new RemoteConfigTemplateImpl(templateResponse); + }); + } } /** From 9f7a19b44bc438789a7dae44e0517afc555dbe56 Mon Sep 17 00:00:00 2001 From: Lahiru Maramba Date: Fri, 13 Mar 2020 13:29:30 -0400 Subject: [PATCH 2/5] Add unit tests for publishTemplate --- src/remote-config/remote-config-api-client.ts | 2 +- .../remote-config-api-client.spec.ts | 104 ++++++++++++++++++ test/unit/remote-config/remote-config.spec.ts | 90 +++++++++++++++ 3 files changed, 195 insertions(+), 1 deletion(-) diff --git a/src/remote-config/remote-config-api-client.ts b/src/remote-config/remote-config-api-client.ts index dc5dcd02c8..ced28e8fb0 100644 --- a/src/remote-config/remote-config-api-client.ts +++ b/src/remote-config/remote-config-api-client.ts @@ -162,7 +162,7 @@ export class RemoteConfigApiClient { public publishTemplate(template: RemoteConfigTemplate, options?: { force: boolean }): Promise { let ifMatch: string = template.etag; - if (options.force == true) { + if (options && options.force == true) { // setting `If-Match: *` forces the Remote Config template to be updated // and circumvent the ETag, and the protection from that it provides. ifMatch = '*'; 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 d010260c10..65382ba65f 100644 --- a/test/unit/remote-config/remote-config-api-client.spec.ts +++ b/test/unit/remote-config/remote-config-api-client.spec.ts @@ -280,4 +280,108 @@ describe('RemoteConfigApiClient', () => { }); }); }); + + describe('publishTemplate', () => { + const testResponse = { + conditions: [{ name: 'ios', expression: 'exp' }], + parameters: { param: { defaultValue: { value: 'true' } } }, + version: {}, + }; + + it(`should reject when project id is not available`, () => { + return clientWithoutProjectId.publishTemplate(REMOTE_CONFIG_TEMPLATE) + .should.eventually.be.rejectedWith(noProjectId); + }); + + const testOptions = [ + { options: undefined, etag: 'etag-123456789012-6' }, + { options: { force: true }, etag: '*' } + ]; + testOptions.forEach((option) => { + it('should resolve with the requested template on success', () => { + const stub = sinon + .stub(HttpClient.prototype, 'send') + .resolves(utils.responseFrom(testResponse, 200, { etag: 'etag-123456789012-6' })); + stubs.push(stub); + return apiClient.publishTemplate(REMOTE_CONFIG_TEMPLATE, option.options) + .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-6'); + expect(stub).to.have.been.calledOnce.and.calledWith({ + method: 'PUT', + url: 'https://firebaseremoteconfig.googleapis.com/v1/projects/test-project/remoteConfig', + headers: { ...EXPECTED_HEADERS, 'If-Match': option.etag }, + data: { + conditions: REMOTE_CONFIG_TEMPLATE.conditions, + parameters: REMOTE_CONFIG_TEMPLATE.parameters, + } + }); + }); + }); + }); + + it('should reject when the etag is not present', () => { + const stub = sinon + .stub(HttpClient.prototype, 'send') + .resolves(utils.responseFrom(testResponse)); + stubs.push(stub); + const expected = new FirebaseRemoteConfigError('invalid-argument', 'ETag header is not present in the server response.'); + return apiClient.publishTemplate(REMOTE_CONFIG_TEMPLATE) + .should.eventually.be.rejected.and.deep.equal(expected); + }); + + it('should reject 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.publishTemplate(REMOTE_CONFIG_TEMPLATE) + .should.eventually.be.rejected.and.deep.equal(expected); + }); + + it('should reject with 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.publishTemplate(REMOTE_CONFIG_TEMPLATE) + .should.eventually.be.rejected.and.deep.equal(expected); + }); + + it('should reject with 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.publishTemplate(REMOTE_CONFIG_TEMPLATE) + .should.eventually.be.rejected.and.deep.equal(expected); + }); + + const errorMessages = [ + "[VALIDATION_ERROR]: [foo] are not valid condition names. All keys in all conditional value maps must be valid condition names.", + "[VERSION_MISMATCH]: Expected version 6, found 8 for project: 123456789012" + ]; + errorMessages.forEach((message) => { + it('should reject with failed-precondition when a validation error occurres', () => { + const stub = sinon + .stub(HttpClient.prototype, 'send') + .rejects(utils.errorFrom({ + error: { + code: 400, + message: message, + status: "FAILED_PRECONDITION" + } + }, 400)); + stubs.push(stub); + const expected = new FirebaseRemoteConfigError('failed-precondition', message); + return apiClient.publishTemplate(REMOTE_CONFIG_TEMPLATE) + .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 index 81563bdd28..3464db9b3d 100644 --- a/test/unit/remote-config/remote-config.spec.ts +++ b/test/unit/remote-config/remote-config.spec.ts @@ -329,4 +329,94 @@ describe('RemoteConfig', () => { }); }); }); + + describe('publishTemplate', () => { + it('should propagate API errors', () => { + const stub = sinon + .stub(RemoteConfigApiClient.prototype, 'publishTemplate') + .rejects(INTERNAL_ERROR); + stubs.push(stub); + return remoteConfig.publishTemplate(REMOTE_CONFIG_TEMPLATE) + .should.eventually.be.rejected.and.deep.equal(INTERNAL_ERROR); + }); + + it('should reject when API response is invalid', () => { + const stub = sinon + .stub(RemoteConfigApiClient.prototype, 'publishTemplate') + .resolves(null); + stubs.push(stub); + return remoteConfig.publishTemplate(REMOTE_CONFIG_TEMPLATE) + .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, 'publishTemplate') + .resolves(response); + stubs.push(stub); + return remoteConfig.publishTemplate(REMOTE_CONFIG_TEMPLATE) + .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, 'publishTemplate') + .resolves(response); + stubs.push(stub); + return remoteConfig.publishTemplate(REMOTE_CONFIG_TEMPLATE) + .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, 'publishTemplate') + .resolves(response); + stubs.push(stub); + return remoteConfig.publishTemplate(REMOTE_CONFIG_TEMPLATE) + .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, 'publishTemplate') + .resolves(REMOTE_CONFIG_RESPONSE); + stubs.push(stub); + + return remoteConfig.publishTemplate(REMOTE_CONFIG_TEMPLATE) + .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\''); + expect(template.conditions[0].tagColor).to.equal('BLUE'); + expect(template.etag).to.equal('etag-123456789012-5'); + // verify that the 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]; + expect(p1.defaultValue).deep.equals({ value: 'true' }); + expect(p1.conditionalValues).deep.equals({ ios: { useInAppDefault: true } }); + expect(p1.description).equals('this is a promo'); + + const c = template.conditions.find((c) => c.name === 'ios'); + expect(c).to.be.not.undefined; + const cond = c as RemoteConfigCondition; + expect(cond.name).to.equal('ios'); + expect(cond.expression).to.equal('device.os == \'ios\''); + expect(cond.tagColor).to.equal('BLUE'); + }); + }); + }); }); From 3577082f60a72efab51c40fbfe119219a172068d Mon Sep 17 00:00:00 2001 From: Lahiru Maramba Date: Mon, 16 Mar 2020 19:15:52 -0400 Subject: [PATCH 3/5] Check for null or undefined etag and move the request logic to its own function --- src/remote-config/remote-config-api-client.ts | 51 ++++++------ .../remote-config-api-client.spec.ts | 81 ++++++++++--------- 2 files changed, 65 insertions(+), 67 deletions(-) diff --git a/src/remote-config/remote-config-api-client.ts b/src/remote-config/remote-config-api-client.ts index ced28e8fb0..966e311042 100644 --- a/src/remote-config/remote-config-api-client.ts +++ b/src/remote-config/remote-config-api-client.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { HttpRequestConfig, HttpClient, HttpError, AuthorizedHttpClient } from '../utils/api-request'; +import { HttpRequestConfig, HttpClient, HttpError, AuthorizedHttpClient, HttpResponse } from '../utils/api-request'; import { PrefixedFirebaseError } from '../utils/error'; import { FirebaseRemoteConfigError, RemoteConfigErrorCode } from './remote-config-utils'; import { FirebaseApp } from '../firebase-app'; @@ -127,19 +127,7 @@ export class RemoteConfigApiClient { } public validateTemplate(template: RemoteConfigTemplate): Promise { - return this.getUrl() - .then((url) => { - const request: HttpRequestConfig = { - method: 'PUT', - url: `${url}/remoteConfig?validate_only=true`, - headers: { ...FIREBASE_REMOTE_CONFIG_HEADERS, 'If-Match': template.etag }, - data: { - conditions: template.conditions, - parameters: template.parameters, - } - }; - return this.httpClient.send(request); - }) + return this.sendPutRequest(template, template.etag, 'remoteConfig?validate_only=true') .then((resp) => { if (!validator.isNonEmptyString(resp.headers['etag'])) { throw new FirebaseRemoteConfigError( @@ -167,19 +155,7 @@ export class RemoteConfigApiClient { // and circumvent the ETag, and the protection from that it provides. ifMatch = '*'; } - return this.getUrl() - .then((url) => { - const request: HttpRequestConfig = { - method: 'PUT', - url: `${url}/remoteConfig`, - headers: { ...FIREBASE_REMOTE_CONFIG_HEADERS, 'If-Match': ifMatch }, - data: { - conditions: template.conditions, - parameters: template.parameters, - } - }; - return this.httpClient.send(request); - }) + return this.sendPutRequest(template, ifMatch, 'remoteConfig') .then((resp) => { if (!validator.isNonEmptyString(resp.headers['etag'])) { throw new FirebaseRemoteConfigError( @@ -197,6 +173,27 @@ export class RemoteConfigApiClient { }); } + private sendPutRequest(template: RemoteConfigTemplate, etag: string, path: string): Promise { + if (!validator.isNonEmptyString(etag)) { + throw new FirebaseRemoteConfigError( + 'invalid-argument', + 'ETag must be a non-empty string.'); + } + return this.getUrl() + .then((url) => { + const request: HttpRequestConfig = { + method: 'PUT', + url: `${url}/${path}`, + headers: { ...FIREBASE_REMOTE_CONFIG_HEADERS, 'If-Match': etag }, + data: { + conditions: template.conditions, + parameters: template.parameters, + } + }; + return this.httpClient.send(request); + }); + } + private getUrl(): Promise { return this.getProjectIdPrefix() .then((projectIdPrefix) => { 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 65382ba65f..b583752600 100644 --- a/test/unit/remote-config/remote-config-api-client.spec.ts +++ b/test/unit/remote-config/remote-config-api-client.spec.ts @@ -42,11 +42,24 @@ describe('RemoteConfigApiClient', () => { status: 'NOT_FOUND', }, }; + + const VALIDATION_ERROR_MESSAGES = [ + "[VALIDATION_ERROR]: [foo] are not valid condition names. All keys in all conditional value maps must be valid condition names.", + "[VERSION_MISMATCH]: Expected version 6, found 8 for project: 123456789012" + ]; + const EXPECTED_HEADERS = { 'Authorization': 'Bearer mock-token', 'X-Firebase-Client': 'fire-admin-node/', 'Accept-Encoding': 'gzip', }; + + const TEST_RESPONSE = { + conditions: [{ name: 'ios', expression: 'exp' }], + parameters: { param: { defaultValue: { value: 'true' } } }, + version: {}, + }; + 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.'; @@ -100,12 +113,6 @@ describe('RemoteConfigApiClient', () => { }); 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); @@ -114,12 +121,12 @@ describe('RemoteConfigApiClient', () => { it('should resolve with the requested template on success', () => { const stub = sinon .stub(HttpClient.prototype, 'send') - .resolves(utils.responseFrom(testResponse, 200, { etag: 'etag-123456789012-1' })); + .resolves(utils.responseFrom(TEST_RESPONSE, 200, { etag: 'etag-123456789012-1' })); stubs.push(stub); return apiClient.getTemplate() .then((resp) => { - expect(resp.conditions).to.deep.equal(testResponse.conditions); - expect(resp.parameters).to.deep.equal(testResponse.parameters); + 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(stub).to.have.been.calledOnce.and.calledWith({ method: 'GET', @@ -132,7 +139,7 @@ describe('RemoteConfigApiClient', () => { it('should reject when the etag is not present', () => { const stub = sinon .stub(HttpClient.prototype, 'send') - .resolves(utils.responseFrom(testResponse)); + .resolves(utils.responseFrom(TEST_RESPONSE)); stubs.push(stub); const expected = new FirebaseRemoteConfigError('invalid-argument', 'ETag header is not present in the server response.'); return apiClient.getTemplate() @@ -182,12 +189,6 @@ describe('RemoteConfigApiClient', () => { }); describe('validateTemplate', () => { - const testResponse = { - conditions: [{ name: 'ios', expression: 'exp' }], - parameters: { param: { defaultValue: { value: 'true' } } }, - version: {}, - }; - it(`should reject when project id is not available`, () => { return clientWithoutProjectId.validateTemplate(REMOTE_CONFIG_TEMPLATE) .should.eventually.be.rejectedWith(noProjectId); @@ -196,12 +197,12 @@ describe('RemoteConfigApiClient', () => { it('should resolve with the requested template on success', () => { const stub = sinon .stub(HttpClient.prototype, 'send') - .resolves(utils.responseFrom(testResponse, 200, { etag: 'etag-123456789012-0' })); + .resolves(utils.responseFrom(TEST_RESPONSE, 200, { etag: 'etag-123456789012-0' })); stubs.push(stub); return apiClient.validateTemplate(REMOTE_CONFIG_TEMPLATE) .then((resp) => { - expect(resp.conditions).to.deep.equal(testResponse.conditions); - expect(resp.parameters).to.deep.equal(testResponse.parameters); + expect(resp.conditions).to.deep.equal(TEST_RESPONSE.conditions); + expect(resp.parameters).to.deep.equal(TEST_RESPONSE.parameters); // validate template returns an etag with the suffix -0 when successful. // verify that the etag matches the original template etag. expect(resp.etag).to.equal('etag-123456789012-6'); @@ -220,13 +221,20 @@ describe('RemoteConfigApiClient', () => { it('should reject when the etag is not present', () => { const stub = sinon .stub(HttpClient.prototype, 'send') - .resolves(utils.responseFrom(testResponse)); + .resolves(utils.responseFrom(TEST_RESPONSE)); stubs.push(stub); const expected = new FirebaseRemoteConfigError('invalid-argument', 'ETag header is not present in the server response.'); return apiClient.validateTemplate(REMOTE_CONFIG_TEMPLATE) .should.eventually.be.rejected.and.deep.equal(expected); }); + [null, undefined, ''].forEach((etag) => { + it('should reject when the etag in template is null, undefined, or an empty string', () => { + expect(() => apiClient.validateTemplate({ conditions: [], parameters: {}, etag: etag as any })) + .to.throw('ETag must be a non-empty string.'); + }); + }); + it('should reject when a full platform error response is received', () => { const stub = sinon .stub(HttpClient.prototype, 'send') @@ -258,11 +266,7 @@ describe('RemoteConfigApiClient', () => { .should.eventually.be.rejected.and.deep.equal(expected); }); - const errorMessages = [ - "[VALIDATION_ERROR]: [foo] are not valid condition names. All keys in all conditional value maps must be valid condition names.", - "[VERSION_MISMATCH]: Expected version 6, found 8 for project: 123456789012" - ]; - errorMessages.forEach((message) => { + VALIDATION_ERROR_MESSAGES.forEach((message) => { it('should reject with failed-precondition when a validation error occurres', () => { const stub = sinon .stub(HttpClient.prototype, 'send') @@ -282,12 +286,6 @@ describe('RemoteConfigApiClient', () => { }); describe('publishTemplate', () => { - const testResponse = { - conditions: [{ name: 'ios', expression: 'exp' }], - parameters: { param: { defaultValue: { value: 'true' } } }, - version: {}, - }; - it(`should reject when project id is not available`, () => { return clientWithoutProjectId.publishTemplate(REMOTE_CONFIG_TEMPLATE) .should.eventually.be.rejectedWith(noProjectId); @@ -301,12 +299,12 @@ describe('RemoteConfigApiClient', () => { it('should resolve with the requested template on success', () => { const stub = sinon .stub(HttpClient.prototype, 'send') - .resolves(utils.responseFrom(testResponse, 200, { etag: 'etag-123456789012-6' })); + .resolves(utils.responseFrom(TEST_RESPONSE, 200, { etag: 'etag-123456789012-6' })); stubs.push(stub); return apiClient.publishTemplate(REMOTE_CONFIG_TEMPLATE, option.options) .then((resp) => { - expect(resp.conditions).to.deep.equal(testResponse.conditions); - expect(resp.parameters).to.deep.equal(testResponse.parameters); + 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-6'); expect(stub).to.have.been.calledOnce.and.calledWith({ method: 'PUT', @@ -324,13 +322,20 @@ describe('RemoteConfigApiClient', () => { it('should reject when the etag is not present', () => { const stub = sinon .stub(HttpClient.prototype, 'send') - .resolves(utils.responseFrom(testResponse)); + .resolves(utils.responseFrom(TEST_RESPONSE)); stubs.push(stub); const expected = new FirebaseRemoteConfigError('invalid-argument', 'ETag header is not present in the server response.'); return apiClient.publishTemplate(REMOTE_CONFIG_TEMPLATE) .should.eventually.be.rejected.and.deep.equal(expected); }); + [null, undefined, ''].forEach((etag) => { + it('should reject when the etag in template is null, undefined, or an empty string', () => { + expect(() => apiClient.publishTemplate({ conditions: [], parameters: {}, etag: etag as any })) + .to.throw('ETag must be a non-empty string.'); + }); + }); + it('should reject when a full platform error response is received', () => { const stub = sinon .stub(HttpClient.prototype, 'send') @@ -362,11 +367,7 @@ describe('RemoteConfigApiClient', () => { .should.eventually.be.rejected.and.deep.equal(expected); }); - const errorMessages = [ - "[VALIDATION_ERROR]: [foo] are not valid condition names. All keys in all conditional value maps must be valid condition names.", - "[VERSION_MISMATCH]: Expected version 6, found 8 for project: 123456789012" - ]; - errorMessages.forEach((message) => { + VALIDATION_ERROR_MESSAGES.forEach((message) => { it('should reject with failed-precondition when a validation error occurres', () => { const stub = sinon .stub(HttpClient.prototype, 'send') From 1bcda94e965a41b8aee86f0a62b27369bd81b68d Mon Sep 17 00:00:00 2001 From: Lahiru Maramba Date: Tue, 17 Mar 2020 14:21:55 -0400 Subject: [PATCH 4/5] Replace path with validateOnly boolean --- src/remote-config/remote-config-api-client.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/remote-config/remote-config-api-client.ts b/src/remote-config/remote-config-api-client.ts index 966e311042..faf0b4eb82 100644 --- a/src/remote-config/remote-config-api-client.ts +++ b/src/remote-config/remote-config-api-client.ts @@ -127,7 +127,7 @@ export class RemoteConfigApiClient { } public validateTemplate(template: RemoteConfigTemplate): Promise { - return this.sendPutRequest(template, template.etag, 'remoteConfig?validate_only=true') + return this.sendPutRequest(template, template.etag, true) .then((resp) => { if (!validator.isNonEmptyString(resp.headers['etag'])) { throw new FirebaseRemoteConfigError( @@ -155,7 +155,7 @@ export class RemoteConfigApiClient { // and circumvent the ETag, and the protection from that it provides. ifMatch = '*'; } - return this.sendPutRequest(template, ifMatch, 'remoteConfig') + return this.sendPutRequest(template, ifMatch) .then((resp) => { if (!validator.isNonEmptyString(resp.headers['etag'])) { throw new FirebaseRemoteConfigError( @@ -173,12 +173,13 @@ export class RemoteConfigApiClient { }); } - private sendPutRequest(template: RemoteConfigTemplate, etag: string, path: string): Promise { + private sendPutRequest(template: RemoteConfigTemplate, etag: string, validateOnly?: boolean): Promise { if (!validator.isNonEmptyString(etag)) { throw new FirebaseRemoteConfigError( 'invalid-argument', 'ETag must be a non-empty string.'); } + const path = validateOnly ? 'remoteConfig?validate_only=true' : 'remoteConfig'; return this.getUrl() .then((url) => { const request: HttpRequestConfig = { From 7b60f93dcd99a782554399b2acf824b3a6d79e48 Mon Sep 17 00:00:00 2001 From: Lahiru Maramba Date: Tue, 17 Mar 2020 14:41:31 -0400 Subject: [PATCH 5/5] PR Fix --- src/remote-config/remote-config-api-client.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/remote-config/remote-config-api-client.ts b/src/remote-config/remote-config-api-client.ts index faf0b4eb82..7243c0e43a 100644 --- a/src/remote-config/remote-config-api-client.ts +++ b/src/remote-config/remote-config-api-client.ts @@ -179,7 +179,10 @@ export class RemoteConfigApiClient { 'invalid-argument', 'ETag must be a non-empty string.'); } - const path = validateOnly ? 'remoteConfig?validate_only=true' : 'remoteConfig'; + let path = 'remoteConfig'; + if (validateOnly) { + path += '?validate_only=true'; + } return this.getUrl() .then((url) => { const request: HttpRequestConfig = {