-
Notifications
You must be signed in to change notification settings - Fork 59
Improvement for DataSnapshot paths (#10, #26) #32
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
250ae34
c4f4f44
bad2e40
6dbfb46
e8050ff
b67803d
1616980
15aa31f
6635ed4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,7 +24,16 @@ 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, | ||
_isValidWildcardMatch, | ||
_extractParamsFromPath, | ||
} from '../src/main'; | ||
import { makeDataSnapshot } from '../src/providers/database'; | ||
import { FirebaseFunctionsTest } from '../src/lifecycle'; | ||
|
||
describe('main', () => { | ||
describe('#wrap', () => { | ||
|
@@ -54,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({}); | ||
|
@@ -106,11 +115,124 @@ describe('main', () => { | |
expect(context.params).to.deep.equal(params); | ||
expect(context.resource.name).to.equal('ref/a/nested/b'); | ||
}); | ||
|
||
it('should throw when snapshot path includes wildcards', () => { | ||
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')); | ||
expect(() => wrapped(snap, { params })).to.throw(); | ||
}); | ||
|
||
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 wrapped = wrap(constructCF('google.firebase.database.ref.write')); | ||
const result = wrapped(snap, {}); | ||
|
||
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'); | ||
}); | ||
|
||
it('should error when DataSnapshot path does not match trigger template', () => { | ||
const fft = new FirebaseFunctionsTest(); | ||
fft.init(); | ||
|
||
const snap = makeDataSnapshot( | ||
'hello world', | ||
'/different/at/nested/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', () => { | ||
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}'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
expect(params).to.deep.equal({}); | ||
}); | ||
}); | ||
|
||
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}'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
expect(differentChunk).to.equal(false); | ||
}); | ||
}); | ||
|
||
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', | ||
}); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,9 +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, 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 = { | ||
|
@@ -97,14 +99,47 @@ export function wrap<T>(cloudFunction: CloudFunction<T>): WrappedFunction { | |
} else { | ||
_checkOptionValidity(['eventId', 'timestamp', 'params', 'auth', 'authType'], options); | ||
let eventContextOptions = options as EventContextOptions; | ||
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, | ||
eventContextOptions ? eventContextOptions.params : null); | ||
data = makeDataSnapshot(unsafeData._data, formattedSnapshotPath, unsafeData.app) as any; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remaking the data snapshot isn't necessary if you are not populating the snapshot's path from params. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's what I thought, It is necessary for standardizing the path to remove extra |
||
|
||
// 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( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is also not necessary if you do a check within makeSnapshot to ensure the snapshot path doesn't have wildcards. |
||
Object.keys(eventContextOptions.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 | ||
eventContextOptions.params = { | ||
...eventContextOptions.params, | ||
...pathParams, | ||
}; | ||
} | ||
|
||
const defaultContext: EventContext = { | ||
eventId: _makeEventId(), | ||
resource: cloudFunction.__trigger.eventTrigger && { | ||
service: cloudFunction.__trigger.eventTrigger.service, | ||
name: _makeResourceName( | ||
cloudFunction.__trigger.eventTrigger.resource, | ||
has(eventContextOptions, 'params') && eventContextOptions.params, | ||
), | ||
eventId: _makeEventId(), | ||
resource: cloudFunction.__trigger.eventTrigger && { | ||
service: cloudFunction.__trigger.eventTrigger.service, | ||
name: _compiledWildcardPath( | ||
cloudFunction.__trigger.eventTrigger.resource, | ||
has(eventContextOptions, 'params') && eventContextOptions.params, | ||
), | ||
}, | ||
eventType: get(cloudFunction, '__trigger.eventTrigger.eventType'), | ||
timestamp: (new Date()).toISOString(), | ||
|
@@ -116,6 +151,13 @@ export function wrap<T>(cloudFunction: CloudFunction<T>): WrappedFunction { | |
defaultContext.authType = 'UNAUTHENTICATED'; | ||
defaultContext.auth = null; | ||
} | ||
|
||
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)}`); | ||
} | ||
|
||
context = merge({}, defaultContext, eventContextOptions); | ||
} | ||
|
||
|
@@ -129,14 +171,54 @@ export function wrap<T>(cloudFunction: CloudFunction<T>): WrappedFunction { | |
} | ||
|
||
/** @internal */ | ||
export function _makeResourceName(triggerResource: string, params = {}): string { | ||
const wildcardRegex = new RegExp('{[^/{}]*}', 'g'); | ||
export function _compiledWildcardPath(triggerResource: string, params = {}): string { | ||
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 _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 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 { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you make makeDataSnapshot throw, instead of only catching it later when wrapped is called.