From 250ae34981228afdf28e667ba3b0f43ff489b5ea Mon Sep 17 00:00:00 2001 From: Abe Haskins Date: Wed, 28 Nov 2018 10:44:54 -0800 Subject: [PATCH 1/8] Add ability to overwrite DataSnapshot path when template + params are specified. Fixes #10 --- spec/main.spec.ts | 26 +++++++++++++++++++++++--- src/main.ts | 15 ++++++++++++--- 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/spec/main.spec.ts b/spec/main.spec.ts index 4db6f9b..67a6a4b 100644 --- a/spec/main.spec.ts +++ b/spec/main.spec.ts @@ -24,7 +24,9 @@ import { expect } from 'chai'; import * as functions from 'firebase-functions'; import { set } from 'lodash'; -import { mockConfig, makeChange, _makeResourceName, wrap } from '../src/main'; +import { mockConfig, makeChange, _compiledWildcardPath, wrap } from '../src/main'; +import { makeDataSnapshot } from '../src/providers/database'; +import { FirebaseFunctionsTest } from '../src/lifecycle'; describe('main', () => { describe('#wrap', () => { @@ -98,11 +100,29 @@ describe('main', () => { expect(context.params).to.deep.equal(params); expect(context.resource.name).to.equal('ref/a/nested/b'); }); + + it('should set DataSnapshot path based on params', () => { + const fft = new FirebaseFunctionsTest(); + fft.init(); + + const snap = makeDataSnapshot( + 'hello world', + '/ref/{wildcard}/nested/{anotherWildcard}', + ); + const params = { + wildcard: 'at', + anotherWildcard: 'bat', + }; + const wrapped = wrap(constructCF('google.firebase.database.ref.write')); + const result = wrapped(snap, { params }); + expect(result.data._path).to.equal('ref/at/nested/bat'); + expect(result.context.resource.name).to.equal('ref/at/nested/bat'); + }); }); - describe('#_makeResourceName', () => { + describe('#_compiledWildcardPath', () => { it('constructs the right resource name from params', () => { - const resource = _makeResourceName('companies/{company}/users/{user}', { + const resource = _compiledWildcardPath('companies/{company}/users/{user}', { company: 'Google', user: 'Lauren', }); diff --git a/src/main.ts b/src/main.ts index 6bfba38..bcab633 100644 --- a/src/main.ts +++ b/src/main.ts @@ -23,6 +23,7 @@ import { has, merge, random, get } from 'lodash'; import { CloudFunction, EventContext, Resource, Change } from 'firebase-functions'; +import {makeDataSnapshot} from './providers/database'; /** Fields of the event context that can be overridden/customized. */ export type EventContextOptions = { @@ -65,7 +66,7 @@ export function wrap(cloudFunction: CloudFunction): WrappedFunction { eventId: _makeEventId(), resource: { service: cloudFunction.__trigger.eventTrigger.service, - name: _makeResourceName(cloudFunction.__trigger.eventTrigger.resource, options? options.params: null), + name: _compiledWildcardPath(cloudFunction.__trigger.eventTrigger.resource, options? options.params: null), }, eventType: cloudFunction.__trigger.eventTrigger.eventType, timestamp: (new Date()).toISOString(), @@ -74,7 +75,14 @@ export function wrap(cloudFunction: CloudFunction): WrappedFunction { if (defaultContext.eventType.match(/firebase.database/)) { defaultContext.authType = 'UNAUTHENTICATED'; defaultContext.auth = null; + + if (typeof data === 'object') { + const unsafeData = data as any; + const formattedSnapshotPath = _compiledWildcardPath(unsafeData._path, options ? options.params : null); + data = makeDataSnapshot(unsafeData._data, formattedSnapshotPath) as any; + } } + let context = merge({}, defaultContext, options); return cloudFunction.run( data, @@ -85,14 +93,15 @@ export function wrap(cloudFunction: CloudFunction): WrappedFunction { } /** @internal */ -export function _makeResourceName(triggerResource: string, params = {}): string { +export function _compiledWildcardPath(triggerResource: string, params = {}): string { const wildcardRegex = new RegExp('{[^/{}]*}', 'g'); let resourceName = triggerResource.replace(wildcardRegex, (wildcard) => { let wildcardNoBraces = wildcard.slice(1, -1); // .slice removes '{' and '}' from wildcard let sub = get(params, wildcardNoBraces); return sub || wildcardNoBraces + random(1, 9); }); - return resourceName; + + return resourceName.split('/').filter((c) => c).join('/'); } function _makeEventId(): string { From c4f4f44b6625e103e770a26eca8397e460cf25f3 Mon Sep 17 00:00:00 2001 From: Abe Haskins Date: Wed, 28 Nov 2018 11:30:41 -0800 Subject: [PATCH 2/8] Adds additional errors for when data path doesn't match trigger path --- package.json | 4 ++-- spec/main.spec.ts | 48 ++++++++++++++++++++++++++++++++++++++++++++++- src/main.ts | 37 +++++++++++++++++++++++++++++++----- 3 files changed, 81 insertions(+), 8 deletions(-) diff --git a/package.json b/package.json index 4eb8ebb..35f9e07 100644 --- a/package.json +++ b/package.json @@ -37,12 +37,12 @@ "@types/chai": "^4.1.2", "@types/mocha": "^5.0.0", "chai": "^4.1.2", - "firebase-admin": "~6.1.0", + "firebase-admin": "^6.2.0", "firebase-functions": "^2.1.0", "mocha": "^5.0.5", "sinon": "^4.4.9", "tslint": "^5.8.0", - "typescript": "^2.4.1" + "typescript": "^2.9.2" }, "peerDependencies": { "firebase-functions": ">1.0.1", diff --git a/spec/main.spec.ts b/spec/main.spec.ts index 67a6a4b..ab35d55 100644 --- a/spec/main.spec.ts +++ b/spec/main.spec.ts @@ -24,7 +24,7 @@ import { expect } from 'chai'; import * as functions from 'firebase-functions'; import { set } from 'lodash'; -import { mockConfig, makeChange, _compiledWildcardPath, wrap } from '../src/main'; +import {mockConfig, makeChange, _compiledWildcardPath, wrap, _isValidWildcardMatch} from '../src/main'; import { makeDataSnapshot } from '../src/providers/database'; import { FirebaseFunctionsTest } from '../src/lifecycle'; @@ -118,6 +118,52 @@ describe('main', () => { expect(result.data._path).to.equal('ref/at/nested/bat'); expect(result.context.resource.name).to.equal('ref/at/nested/bat'); }); + + it('should error when DataSnapshot wildcard path does not match resource path', () => { + const fft = new FirebaseFunctionsTest(); + fft.init(); + + const snap = makeDataSnapshot( + 'hello world', + '/different/{wildcard}/nested/{anotherWildcard}', + ); + const params = { + wildcard: 'at', + anotherWildcard: 'bat', + }; + const wrapped = wrap(constructCF('google.firebase.database.ref.write')); + expect(() => wrapped(snap, { params })).to.throw(); + }); + }); + + describe('#_isValidWildcardMatch', () => { + it('should match a path which fits a wildcard template', () => { + const valid = _isValidWildcardMatch( + 'companies/{company}/users/{user}', + '/companies/firebase/users/abe'); + expect(valid).to.equal(true); + }); + + it('should not match a path which is too long', () => { + const tooLong = _isValidWildcardMatch( + 'companies/{company}/users/{user}', + 'companies/firebase/users/abe/boots'); + expect(tooLong).to.equal(false); + }); + + it('should not match a path which is too short', () => { + const tooShort = _isValidWildcardMatch( + 'companies/{company}/users/{user}', + 'companies/firebase/users/'); + expect(tooShort).to.equal(false); + }); + + it('should not match a path which has different chunks', () => { + const differentChunk = _isValidWildcardMatch( + 'locations/{company}/users/{user}', + 'companies/firebase/users/{user}'); + expect(differentChunk).to.equal(false); + }); }); describe('#_compiledWildcardPath', () => { diff --git a/src/main.ts b/src/main.ts index bcab633..b058201 100644 --- a/src/main.ts +++ b/src/main.ts @@ -72,17 +72,24 @@ export function wrap(cloudFunction: CloudFunction): WrappedFunction { timestamp: (new Date()).toISOString(), params: {}, }; + + const unsafeData = data as any; if (defaultContext.eventType.match(/firebase.database/)) { defaultContext.authType = 'UNAUTHENTICATED'; defaultContext.auth = null; - if (typeof data === 'object') { - const unsafeData = data as any; - const formattedSnapshotPath = _compiledWildcardPath(unsafeData._path, options ? options.params : null); - data = makeDataSnapshot(unsafeData._data, formattedSnapshotPath) as any; + if (typeof unsafeData === 'object' && unsafeData._path) { + const formattedSnapshotPath = _compiledWildcardPath(unsafeData._path, options ? options.params : null); + data = makeDataSnapshot(unsafeData._data, formattedSnapshotPath, unsafeData.app) as any; } } + if (unsafeData._path && + !_isValidWildcardMatch(cloudFunction.__trigger.eventTrigger.resource, unsafeData._path)) { + throw new Error(`Provided path ${_trimSlashes(unsafeData._path)} \ +does not match trigger template ${_trimSlashes(cloudFunction.__trigger.eventTrigger.resource)}`); + } + let context = merge({}, defaultContext, options); return cloudFunction.run( data, @@ -101,7 +108,27 @@ export function _compiledWildcardPath(triggerResource: string, params = {}): str return sub || wildcardNoBraces + random(1, 9); }); - return resourceName.split('/').filter((c) => c).join('/'); + return _trimSlashes(resourceName); +} + +export function _isValidWildcardMatch(wildcardPath, snapshotPath) { + const wildcardRegex = /{.+}/; + const wildcardChunks = _trimSlashes(wildcardPath).split('/'); + const snapshotChucks = _trimSlashes(snapshotPath).split('/'); + + if (snapshotChucks.length > wildcardChunks.length) { + return false; + } + + const mismatchedChunks = wildcardChunks.slice(-snapshotChucks.length).filter((chunk, index) => { + return !(wildcardRegex.exec(chunk) || chunk === snapshotChucks[index]); + }); + + return !mismatchedChunks.length; +} + +export function _trimSlashes(path: string) { + return path.split('/').filter((c) => c).join('/'); } function _makeEventId(): string { From bad2e403effb89ed4541badd373a271e2cc12d37 Mon Sep 17 00:00:00 2001 From: Abe Haskins Date: Wed, 28 Nov 2018 11:41:02 -0800 Subject: [PATCH 3/8] Adds `DataSnapshot.key` check when setting path based on params --- spec/main.spec.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/main.spec.ts b/spec/main.spec.ts index ab35d55..d3291ca 100644 --- a/spec/main.spec.ts +++ b/spec/main.spec.ts @@ -116,6 +116,7 @@ describe('main', () => { const wrapped = wrap(constructCF('google.firebase.database.ref.write')); const result = wrapped(snap, { params }); expect(result.data._path).to.equal('ref/at/nested/bat'); + expect(result.data.key).to.equal('bat'); expect(result.context.resource.name).to.equal('ref/at/nested/bat'); }); From 6dbfb469fdb26cb4a66c0eadc6aa059a18f98bd0 Mon Sep 17 00:00:00 2001 From: Abe Haskins Date: Wed, 28 Nov 2018 12:22:42 -0800 Subject: [PATCH 4/8] Enable mixed passing of wildcard values from snapshot path and params --- spec/main.spec.ts | 131 +++++++++++++++++++++++++++++++++----- src/main.ts | 40 ++++++++++-- src/providers/database.ts | 2 +- 3 files changed, 150 insertions(+), 23 deletions(-) diff --git a/spec/main.spec.ts b/spec/main.spec.ts index d3291ca..d787d67 100644 --- a/spec/main.spec.ts +++ b/spec/main.spec.ts @@ -24,7 +24,14 @@ import { expect } from 'chai'; import * as functions from 'firebase-functions'; import { set } from 'lodash'; -import {mockConfig, makeChange, _compiledWildcardPath, wrap, _isValidWildcardMatch} from '../src/main'; +import { + mockConfig, + makeChange, + _compiledWildcardPath, + wrap, + _isValidWildcardMatch, + _extractParamsFromPath, +} from '../src/main'; import { makeDataSnapshot } from '../src/providers/database'; import { FirebaseFunctionsTest } from '../src/lifecycle'; @@ -56,7 +63,7 @@ describe('main', () => { expect(typeof context.eventId).to.equal('string'); expect(context.resource.service).to.equal('service'); expect(/ref\/wildcard[1-9]\/nested\/anotherWildcard[1-9]/ - .test(context.resource.name)).to.be.true; + .test(context.resource.name)).to.be.true; expect(context.eventType).to.equal('event'); expect(Date.parse(context.timestamp)).to.be.greaterThan(0); expect(context.params).to.deep.equal({}); @@ -101,7 +108,7 @@ describe('main', () => { expect(context.resource.name).to.equal('ref/a/nested/b'); }); - it('should set DataSnapshot path based on params', () => { + it('should set auto-fill DataSnapshot path based on params', () => { const fft = new FirebaseFunctionsTest(); fft.init(); @@ -120,20 +127,112 @@ describe('main', () => { expect(result.context.resource.name).to.equal('ref/at/nested/bat'); }); + it('should set auto-fill DataSnapshot path based on params', () => { + const fft = new FirebaseFunctionsTest(); + fft.init(); + + const snap = makeDataSnapshot( + 'hello world', + '/ref/{wildcard}/nested/{anotherWildcard}', + ); + const params = { + wildcard: 'at', + anotherWildcard: 'bat', + }; + const wrapped = wrap(constructCF('google.firebase.database.ref.write')); + const result = wrapped(snap, { params }); + expect(result.data._path).to.equal('ref/at/nested/bat'); + expect(result.data.key).to.equal('bat'); + expect(result.context.resource.name).to.equal('ref/at/nested/bat'); + }); + + it('should set auto-fill params based on DataSnapshot path', () => { + const fft = new FirebaseFunctionsTest(); + fft.init(); + + const snap = makeDataSnapshot( + 'hello world', + '/ref/cat/nested/that', + ); + + const params = {}; + const wrapped = wrap(constructCF('google.firebase.database.ref.write')); + const result = wrapped(snap, { params }); + + expect(result.data._path).to.equal('ref/cat/nested/that'); + expect(result.data.key).to.equal('that'); + expect(result.context.resource.name).to.equal('ref/cat/nested/that'); + }); + + it('should allow mixed wildcards based on DataSnapshot path and params', () => { + const fft = new FirebaseFunctionsTest(); + fft.init(); + + const snap = makeDataSnapshot( + 'hello world', + '/ref/cat/nested/{anotherWildcard}', + ); + + const params = {anotherWildcard: 'where'}; + const wrapped = wrap(constructCF('google.firebase.database.ref.write')); + const result = wrapped(snap, { params }); + + expect(result.data._path).to.equal('ref/cat/nested/where'); + expect(result.data.key).to.equal('where'); + expect(result.context.resource.name).to.equal('ref/cat/nested/where'); + }); + it('should error when DataSnapshot wildcard path does not match resource path', () => { - const fft = new FirebaseFunctionsTest(); - fft.init(); - - const snap = makeDataSnapshot( - 'hello world', - '/different/{wildcard}/nested/{anotherWildcard}', - ); - const params = { - wildcard: 'at', - anotherWildcard: 'bat', - }; - const wrapped = wrap(constructCF('google.firebase.database.ref.write')); - expect(() => wrapped(snap, { params })).to.throw(); + const fft = new FirebaseFunctionsTest(); + fft.init(); + + const snap = makeDataSnapshot( + 'hello world', + '/different/{wildcard}/nested/{anotherWildcard}', + ); + const params = { + wildcard: 'at', + anotherWildcard: 'bat', + }; + const wrapped = wrap(constructCF('google.firebase.database.ref.write')); + expect(() => wrapped(snap, { params })).to.throw(); + }); + }); + + describe('#_extractParamsFromPath', () => { + it('should match a path which fits a wildcard template', () => { + const params = _extractParamsFromPath( + 'companies/{company}/users/{user}', + '/companies/firebase/users/abe'); + expect(params).to.deep.equal({company: 'firebase', user: 'abe'}); + }); + + it('should not match unfilled wildcards which is too long', () => { + const params = _extractParamsFromPath( + 'companies/{company}/users/{user}', + 'companies/{still_wild}/users/abe'); + expect(params).to.deep.equal({user: 'abe'}); + }); + + it('should not match a path which is too long', () => { + const params = _extractParamsFromPath( + 'companies/{company}/users/{user}', + 'companies/firebase/users/abe/boots'); + expect(params).to.deep.equal({}); + }); + + it('should not match a path which is too short', () => { + const params = _extractParamsFromPath( + 'companies/{company}/users/{user}', + 'companies/firebase/users/'); + expect(params).to.deep.equal({}); + }); + + it('should not match a path which has different chunks', () => { + const params = _extractParamsFromPath( + 'locations/{company}/users/{user}', + 'companies/firebase/users/{user}'); + expect(params).to.deep.equal({}); }); }); diff --git a/src/main.ts b/src/main.ts index b058201..ebac88b 100644 --- a/src/main.ts +++ b/src/main.ts @@ -62,6 +62,19 @@ export function wrap(cloudFunction: CloudFunction): WrappedFunction { throw new Error('This library can only be used with functions written with firebase-functions v1.0.0 and above'); } let wrapped: WrappedFunction = (data: T, options: EventContextOptions) => { + // Although in Typescript we require `options` some of our JS samples do not pass it. + options = options || {}; + + const unsafeData = data as any; + if (typeof unsafeData === 'object' && unsafeData._path) { + options.params = { + ...options.params, + ..._extractParamsFromPath(cloudFunction.__trigger.eventTrigger.resource, unsafeData._path), + }; + const formattedSnapshotPath = _compiledWildcardPath(unsafeData._path, options ? options.params : null); + data = makeDataSnapshot(unsafeData._data, formattedSnapshotPath, unsafeData.app) as any; + } + const defaultContext: EventContext = { eventId: _makeEventId(), resource: { @@ -73,15 +86,9 @@ export function wrap(cloudFunction: CloudFunction): WrappedFunction { params: {}, }; - const unsafeData = data as any; if (defaultContext.eventType.match(/firebase.database/)) { defaultContext.authType = 'UNAUTHENTICATED'; defaultContext.auth = null; - - if (typeof unsafeData === 'object' && unsafeData._path) { - const formattedSnapshotPath = _compiledWildcardPath(unsafeData._path, options ? options.params : null); - data = makeDataSnapshot(unsafeData._data, formattedSnapshotPath, unsafeData.app) as any; - } } if (unsafeData._path && @@ -111,6 +118,27 @@ export function _compiledWildcardPath(triggerResource: string, params = {}): str return _trimSlashes(resourceName); } +export function _extractParamsFromPath(wildcardPath, snapshotPath) { + if (!_isValidWildcardMatch(wildcardPath, snapshotPath)) { + return {}; + } + + const wildcardKeyRegex = /{(.+)}/; + const wildcardChunks = _trimSlashes(wildcardPath).split('/'); + const snapshotChucks = _trimSlashes(snapshotPath).split('/'); + return wildcardChunks.slice(-snapshotChucks.length).reduce((params, chunk, index) => { + let match = wildcardKeyRegex.exec(chunk); + if (match) { + const wildcardKey = match[1]; + const potentialWildcardValue = snapshotChucks[index]; + if (!wildcardKeyRegex.exec(potentialWildcardValue)) { + params[wildcardKey] = potentialWildcardValue; + } + } + return params; + }, {}); +} + export function _isValidWildcardMatch(wildcardPath, snapshotPath) { const wildcardRegex = /{.+}/; const wildcardChunks = _trimSlashes(wildcardPath).split('/'); diff --git a/src/providers/database.ts b/src/providers/database.ts index 28708f8..8a23d83 100644 --- a/src/providers/database.ts +++ b/src/providers/database.ts @@ -30,7 +30,7 @@ export function makeDataSnapshot( /** Value of data for the snapshot. */ val: string | number | boolean | any[] | object, /** Full path of the reference (e.g. 'users/alovelace'). */ - refPath: string, + refPath?: string, /** The Firebase app that the database belongs to. * The databaseURL supplied when initializing the app will be used for creating this snapshot. * You do not need to supply this parameter if you supplied Firebase config values when initializing From e8050ffd3cb1d5ddc4c3a14626adab87a2c05bff Mon Sep 17 00:00:00 2001 From: Abe Haskins Date: Wed, 28 Nov 2018 12:27:15 -0800 Subject: [PATCH 5/8] Adds tests for params from DataSnapshot path --- spec/main.spec.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/spec/main.spec.ts b/spec/main.spec.ts index d787d67..a266905 100644 --- a/spec/main.spec.ts +++ b/spec/main.spec.ts @@ -161,6 +161,7 @@ describe('main', () => { expect(result.data._path).to.equal('ref/cat/nested/that'); expect(result.data.key).to.equal('that'); + expect(result.context.params.wildcard).to.equal('cat'); expect(result.context.resource.name).to.equal('ref/cat/nested/that'); }); @@ -179,6 +180,8 @@ describe('main', () => { expect(result.data._path).to.equal('ref/cat/nested/where'); expect(result.data.key).to.equal('where'); + expect(result.context.params.wildcard).to.equal('cat'); + expect(result.context.params.anotherWildcard).to.equal('where'); expect(result.context.resource.name).to.equal('ref/cat/nested/where'); }); From b67803d76a8549d5adf0351f1bbc6be25e038e88 Mon Sep 17 00:00:00 2001 From: Abe Haskins Date: Wed, 28 Nov 2018 12:34:21 -0800 Subject: [PATCH 6/8] Remove accidental duplicate test --- spec/main.spec.ts | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/spec/main.spec.ts b/spec/main.spec.ts index a266905..3e328ee 100644 --- a/spec/main.spec.ts +++ b/spec/main.spec.ts @@ -127,25 +127,6 @@ describe('main', () => { expect(result.context.resource.name).to.equal('ref/at/nested/bat'); }); - it('should set auto-fill DataSnapshot path based on params', () => { - const fft = new FirebaseFunctionsTest(); - fft.init(); - - const snap = makeDataSnapshot( - 'hello world', - '/ref/{wildcard}/nested/{anotherWildcard}', - ); - const params = { - wildcard: 'at', - anotherWildcard: 'bat', - }; - const wrapped = wrap(constructCF('google.firebase.database.ref.write')); - const result = wrapped(snap, { params }); - expect(result.data._path).to.equal('ref/at/nested/bat'); - expect(result.data.key).to.equal('bat'); - expect(result.context.resource.name).to.equal('ref/at/nested/bat'); - }); - it('should set auto-fill params based on DataSnapshot path', () => { const fft = new FirebaseFunctionsTest(); fft.init(); From 1616980d51675ba0cab40aac3c0f8d4bd7cd360d Mon Sep 17 00:00:00 2001 From: Abe Haskins Date: Wed, 5 Dec 2018 13:17:55 -0800 Subject: [PATCH 7/8] Makes path/params relationship unidirectional --- spec/main.spec.ts | 27 ++------------------------- src/main.ts | 32 ++++++++++++++++++++++++++------ 2 files changed, 28 insertions(+), 31 deletions(-) diff --git a/spec/main.spec.ts b/spec/main.spec.ts index 3e328ee..79dfd00 100644 --- a/spec/main.spec.ts +++ b/spec/main.spec.ts @@ -108,7 +108,7 @@ describe('main', () => { expect(context.resource.name).to.equal('ref/a/nested/b'); }); - it('should set auto-fill DataSnapshot path based on params', () => { + it('should throw when snapshot path includes unspecified wildcards', () => { const fft = new FirebaseFunctionsTest(); fft.init(); @@ -121,10 +121,7 @@ describe('main', () => { anotherWildcard: 'bat', }; const wrapped = wrap(constructCF('google.firebase.database.ref.write')); - const result = wrapped(snap, { params }); - expect(result.data._path).to.equal('ref/at/nested/bat'); - expect(result.data.key).to.equal('bat'); - expect(result.context.resource.name).to.equal('ref/at/nested/bat'); + expect(() => wrapped(snap, { params })).to.throw(); }); it('should set auto-fill params based on DataSnapshot path', () => { @@ -146,26 +143,6 @@ describe('main', () => { expect(result.context.resource.name).to.equal('ref/cat/nested/that'); }); - it('should allow mixed wildcards based on DataSnapshot path and params', () => { - const fft = new FirebaseFunctionsTest(); - fft.init(); - - const snap = makeDataSnapshot( - 'hello world', - '/ref/cat/nested/{anotherWildcard}', - ); - - const params = {anotherWildcard: 'where'}; - const wrapped = wrap(constructCF('google.firebase.database.ref.write')); - const result = wrapped(snap, { params }); - - expect(result.data._path).to.equal('ref/cat/nested/where'); - expect(result.data.key).to.equal('where'); - expect(result.context.params.wildcard).to.equal('cat'); - expect(result.context.params.anotherWildcard).to.equal('where'); - expect(result.context.resource.name).to.equal('ref/cat/nested/where'); - }); - it('should error when DataSnapshot wildcard path does not match resource path', () => { const fft = new FirebaseFunctionsTest(); fft.init(); diff --git a/src/main.ts b/src/main.ts index ebac88b..6768dbc 100644 --- a/src/main.ts +++ b/src/main.ts @@ -20,10 +20,11 @@ // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE // SOFTWARE. -import { has, merge, random, get } from 'lodash'; +import { has, merge, random, get, differenceWith } from 'lodash'; import { CloudFunction, EventContext, Resource, Change } from 'firebase-functions'; import {makeDataSnapshot} from './providers/database'; +const wildcardRegex = new RegExp('{[^/{}]*}', 'g'); /** Fields of the event context that can be overridden/customized. */ export type EventContextOptions = { @@ -67,13 +68,34 @@ export function wrap(cloudFunction: CloudFunction): WrappedFunction { const unsafeData = data as any; if (typeof unsafeData === 'object' && unsafeData._path) { + // Remake the snapshot to standardize the path and remove extra slashes + const formattedSnapshotPath = _compiledWildcardPath(unsafeData._path, options ? options.params : null); + data = makeDataSnapshot(unsafeData._data, formattedSnapshotPath, unsafeData.app) as any; + + // Ensure the path has no wildcards + if (unsafeData._path.match(wildcardRegex)) { + throw new Error(`Data path ("${unsafeData._path}") should not contain wildcards.`); + } + + // Ensure that the same param wasn't specified by the path and the options.params bundle + const pathParams = _extractParamsFromPath(cloudFunction.__trigger.eventTrigger.resource, unsafeData._path); + const overlappingWildcards = differenceWith( + Object.keys(options.params), + Object.keys(pathParams), + ); + + if (overlappingWildcards.length) { + throw new Error(`The same context parameter (${overlappingWildcards.join(', ')}) \ +was supplied by the data path "${unsafeData._path}" and the options bundle "${JSON.stringify(options)}".`); + } + + // Build final params bundle options.params = { ...options.params, - ..._extractParamsFromPath(cloudFunction.__trigger.eventTrigger.resource, unsafeData._path), + ...pathParams, }; - const formattedSnapshotPath = _compiledWildcardPath(unsafeData._path, options ? options.params : null); - data = makeDataSnapshot(unsafeData._data, formattedSnapshotPath, unsafeData.app) as any; } + /// const defaultContext: EventContext = { eventId: _makeEventId(), @@ -108,7 +130,6 @@ does not match trigger template ${_trimSlashes(cloudFunction.__trigger.eventTrig /** @internal */ export function _compiledWildcardPath(triggerResource: string, params = {}): string { - const wildcardRegex = new RegExp('{[^/{}]*}', 'g'); let resourceName = triggerResource.replace(wildcardRegex, (wildcard) => { let wildcardNoBraces = wildcard.slice(1, -1); // .slice removes '{' and '}' from wildcard let sub = get(params, wildcardNoBraces); @@ -140,7 +161,6 @@ export function _extractParamsFromPath(wildcardPath, snapshotPath) { } export function _isValidWildcardMatch(wildcardPath, snapshotPath) { - const wildcardRegex = /{.+}/; const wildcardChunks = _trimSlashes(wildcardPath).split('/'); const snapshotChucks = _trimSlashes(snapshotPath).split('/'); From 6635ed4344c9bf1acc4ec06e8a5077a20863949f Mon Sep 17 00:00:00 2001 From: Abe Haskins Date: Wed, 5 Dec 2018 16:03:57 -0800 Subject: [PATCH 8/8] Begin addressing review feedback --- spec/main.spec.ts | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/spec/main.spec.ts b/spec/main.spec.ts index adfa9ff..9fb0695 100644 --- a/spec/main.spec.ts +++ b/spec/main.spec.ts @@ -116,7 +116,7 @@ describe('main', () => { expect(context.resource.name).to.equal('ref/a/nested/b'); }); - it('should throw when snapshot path includes unspecified wildcards', () => { + it('should throw when snapshot path includes wildcards', () => { const fft = new FirebaseFunctionsTest(); fft.init(); @@ -141,9 +141,8 @@ describe('main', () => { '/ref/cat/nested/that', ); - const params = {}; const wrapped = wrap(constructCF('google.firebase.database.ref.write')); - const result = wrapped(snap, { params }); + const result = wrapped(snap, {}); expect(result.data._path).to.equal('ref/cat/nested/that'); expect(result.data.key).to.equal('that'); @@ -151,18 +150,14 @@ describe('main', () => { expect(result.context.resource.name).to.equal('ref/cat/nested/that'); }); - it('should error when DataSnapshot wildcard path does not match resource path', () => { + it('should error when DataSnapshot path does not match trigger template', () => { const fft = new FirebaseFunctionsTest(); fft.init(); const snap = makeDataSnapshot( 'hello world', - '/different/{wildcard}/nested/{anotherWildcard}', + '/different/at/nested/bat', ); - const params = { - wildcard: 'at', - anotherWildcard: 'bat', - }; const wrapped = wrap(constructCF('google.firebase.database.ref.write')); expect(() => wrapped(snap, { params })).to.throw(); }); @@ -176,7 +171,7 @@ describe('main', () => { expect(params).to.deep.equal({company: 'firebase', user: 'abe'}); }); - it('should not match unfilled wildcards which is too long', () => { + it('should not match unfilled wildcards', () => { const params = _extractParamsFromPath( 'companies/{company}/users/{user}', 'companies/{still_wild}/users/abe');