-
Notifications
You must be signed in to change notification settings - Fork 59
Update Rtdb test sdk to allow json for CloudEvent #159
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
bcfeea5
1481939
eb3d89b
77b831b
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 |
---|---|---|
|
@@ -6,10 +6,46 @@ import { | |
} from '../../../providers/database'; | ||
import { getBaseCloudEvent } from '../helpers'; | ||
import { Change } from 'firebase-functions'; | ||
import { makeDataSnapshot } from '../../../providers/database'; | ||
|
||
type ChangeLike = { | ||
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. nit: This reads a little weird. Why did you create a type alias to an anonymous interface type rather than just creating a named interface (i.e. |
||
before: database.DataSnapshot | object; | ||
after: database.DataSnapshot | object; | ||
}; | ||
|
||
function getOrCreateDataSnapshot( | ||
data: database.DataSnapshot | object, | ||
TheIronDev marked this conversation as resolved.
Show resolved
Hide resolved
|
||
ref: string | ||
) { | ||
if (data instanceof database.DataSnapshot) { | ||
return data; | ||
} | ||
if (data instanceof Object) { | ||
return makeDataSnapshot(data, ref); | ||
} | ||
return exampleDataSnapshot(ref); | ||
} | ||
|
||
function getOrCreateDataSnapshotChange( | ||
data: DeepPartial<Change<database.DataSnapshot> | ChangeLike>, | ||
ref: string | ||
) { | ||
if (data instanceof Change) { | ||
return data; | ||
} | ||
if (data instanceof Object && data?.before && data?.after) { | ||
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. I wonder if we want Functionally it's the difference between one LOC in:
I could see arguments in either direction. Either we're helping them write simpler code or we could require them to be explicit to avoid accidental misuse (e.g. typo in "before" or "after"). Thoughts? |
||
const beforeDataSnapshot = getOrCreateDataSnapshot(data!.before, ref); | ||
const afterDataSnapshot = getOrCreateDataSnapshot(data!.after, ref); | ||
return new Change(beforeDataSnapshot, afterDataSnapshot); | ||
} | ||
return exampleDataSnapshotChange(ref); | ||
} | ||
|
||
export function getDatabaseSnapshotCloudEvent( | ||
cloudFunction: CloudFunction<database.DatabaseEvent<database.DataSnapshot>>, | ||
cloudEventPartial?: DeepPartial<database.DatabaseEvent<database.DataSnapshot>> | ||
cloudEventPartial?: DeepPartial< | ||
database.DatabaseEvent<database.DataSnapshot | object> | ||
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. Nit: I personally would hoist |
||
> | ||
) { | ||
const { | ||
instance, | ||
|
@@ -19,9 +55,7 @@ export function getDatabaseSnapshotCloudEvent( | |
params, | ||
} = getCommonDatabaseFields(cloudFunction, cloudEventPartial); | ||
|
||
const data = | ||
(cloudEventPartial?.data as database.DataSnapshot) || | ||
exampleDataSnapshot(ref); | ||
const data = getOrCreateDataSnapshot(cloudEventPartial?.data, ref); | ||
|
||
return { | ||
// Spread common fields | ||
|
@@ -43,7 +77,7 @@ export function getDatabaseChangeSnapshotCloudEvent( | |
database.DatabaseEvent<Change<database.DataSnapshot>> | ||
>, | ||
cloudEventPartial?: DeepPartial< | ||
database.DatabaseEvent<Change<database.DataSnapshot>> | ||
database.DatabaseEvent<Change<database.DataSnapshot> | ChangeLike> | ||
> | ||
): database.DatabaseEvent<Change<database.DataSnapshot>> { | ||
const { | ||
|
@@ -54,9 +88,7 @@ export function getDatabaseChangeSnapshotCloudEvent( | |
params, | ||
} = getCommonDatabaseFields(cloudFunction, cloudEventPartial); | ||
|
||
const data = | ||
(cloudEventPartial?.data as Change<database.DataSnapshot>) || | ||
exampleDataSnapshotChange(ref); | ||
const data = getOrCreateDataSnapshotChange(cloudEventPartial?.data, ref); | ||
|
||
return { | ||
// Spread common fields | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,7 +29,7 @@ import { DeepPartial } from './cloudevent/types'; | |
* It will subsequently invoke the cloud function it wraps with the provided {@link CloudEvent} | ||
*/ | ||
export type WrappedV2Function<T extends CloudEvent<unknown>> = ( | ||
cloudEventPartial?: DeepPartial<T> | ||
cloudEventPartial?: DeepPartial<T | object> | ||
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.
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.
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.
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. There's a big difference between |
||
) => any | Promise<any>; | ||
|
||
/** | ||
|
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.
This is such a cool problem you had to solve. Good find!