Add MockCloudEvents for RTDB-V2 #156
Conversation
This commit is a follow-up to firebase-function's addition of rtdb-v2. This commit must wait for the RTDB-V2 updates to be published on npm.
| (cloudEventPartial?.firebaseDatabaseHost as string) || | ||
| 'firebaseDatabaseHost'; | ||
| const ref = (cloudEventPartial?.ref as string) || '/foo/bar'; | ||
| const location = (cloudEventPartial?.location as string) || 'us-central1'; |
There was a problem hiding this comment.
I'm unclear where on the function I should grab the location as a string. I'll follow up with you on this one.
|
Is this ready for re-review? |
The branch is unblocked, but this is not ready for re-review. I'll take a look this weekend to clean things up, but I'll need to check and see how V4 mucks with what I wrote. |
|
It is ready for re-review now (sorry for the loooooooong delay) |
| @@ -41,8 +41,11 @@ export namespace testApp { | |||
|
|
|||
| getApp(): firebase.app.App { | |||
| if (typeof this.appSingleton === 'undefined') { | |||
| const config = process.env.FIREBASE_CONFIG | |||
There was a problem hiding this comment.
Do we know what happens when firebase.initializeApp() is initialized w/ an empty config?
I imagine some things won't work as intended in some strange way. Blowing up JSON.parse(undefined) isn't a great experience either, but curious what problem we are fixing here.
There was a problem hiding this comment.
Yeah this one was interesting.
Tossing an empty config didn't seem to do much of anything. I'm down for any other alternative.
There was a problem hiding this comment.
With the note about ref and params, this will have parity with the test SDK for the v1 API.
Totally out of left field, but working on a Python RTDB interface makes me wonder if we want a break from the past here (instead of? as well?) In v1 we required people to call the makeSnapshot method because the ref was a derived property of the snapshot. But in the v2 event, the ref is redundant with the "ref" field of the raw cloud event. I wonder if we should support literally passing JSON for before and after? And in that case we'd just create a DataShapshot with that JSON value and the ref location in event.ref. Thoughts @TheIronDev and @taeold ?
| @@ -0,0 +1,15 @@ | |||
| import { MockCloudEventAbstractFactory } from '../../types'; | |||
There was a problem hiding this comment.
nit: I started this with storage the last time I edited the codebase, but I think that this habit of having a file for every event type is probably overkill. In the CLI at least we have a map of event type to service and then the strategy object is per service.
There was a problem hiding this comment.
Thats fair. Theres definitely DRYness that could get applied here, but I also wanted to push to make the code as easy-to-read and easy to add.
Adding a new event is a matter of creating a new AbstractFactory with generateMock and match implemented.
TBH I could go either way.
There was a problem hiding this comment.
I'll follow up in a future cl.
This commit is a follow-up to firebase-function's addition of rtdb-v2.
This commit must wait for the RTDB-V2 updates to be published on npm.