firebase / firebase-functions-test

MIT License
232 stars 48 forks source link

Update CloudEvent generation to replace `data` instead of deep merge #144

Closed TheIronDev closed 2 years ago

TheIronDev commented 2 years ago

Description

CloudEvent.data is defined as an arbitrary payload. For some CloudEvents it may be a string, for others it may be an object.

Theres a few special cases that need to be handled with data:

Code sample

const data = {
  message: {
    json: { firebase: 'test' },
    data: 'eyJmaXJlYmFzZSI6InRlc3QifQ==',
  },
  subscription: 'subscription',
};
const cloudFn = pubsub.onMessagePublished('topic', handler);
const cloudFnWrap = wrapV2(cloudFn);
const cloudEventPartial = { data };

expect(cloudFnWrap(cloudEventPartial).cloudEvent.data.message).to.not.include({
  data: 'eyJoZWxsbyI6IndvcmxkIn0=', // Note: would be a mismatch from the json
});
expect(cloudFnWrap(cloudEventPartial).cloudEvent.data.message.json)
  .to.equal(data.message.json);
TheIronDev commented 2 years ago

Let's keep the behavior as is except for when we know the schema is a map. In cases where something is logically a map of string to string we can say that overriding anything in that map overrides the whole map. So for example, setting content type on a storage object still leaves content size present, but overriding a pubsub message's json attribute overrides the whole json.

Would that be something as simple as:

const combined = merge(generated, partial);

if (combined?.data?.message?.json && partial?.data?.message?.json) {
  combined.data.message.json = partial.data.message.json;
}

Or is there something bigger I am overlooking?

inlined commented 2 years ago
/**
  * Create a new T given an initial set of values and a value to be merged in deeply.
  * any field mask in opaqueKeys mean that any value in delta would overwrite base
  * starting at that field.
  * @example
  * const original = {
  *   nested: { key1: "val1", key2: "val2"},
  *   data: { json: { key: "value" } }
  * };
  * const delta = {
  *   nested: { key1: "otherval1" },
  *   data: { json: { ohterKey: "otherValue" } },
  * };
  * const result = deepMerge(original, delta, "data.json");
  * // result is {
  * //   nested: { key1: "otherval1", key2: "val2" },
  * //   data: { json: { otherKey: "otherValue" }},
  * // }
  */
deepMerge<T>(base: T, delta: RecursivePartial<T>, opaqueKeys... RecursiveKeysOf<T>[]): T;

At least that's how I'd do it if I wanted to solve it generically. Otherwise you can do piecemeal overrides everywhere. This would solve json + data for Pub/Sub:

const combined = merge(generated, partial);

if (partial.data?.message?.json) {
  combined.data.message.json = partial.data!.message!.json;
  if (!patial.data?.message?.data) {
    combined.data.message.data = Buffer.from(JSON.stringify(partial.data!.message!.data)).toString("base64");
  }
} else if (partial.data?.message?.data) {
  Object.addProperty(combined.data.message, "json", {
    get: () => JSON.parse(Buffer.from(partial.data!.message!.data, "base64").toString())
  });
}
TheIronDev commented 2 years ago

Closing this PR in favor of https://github.com/firebase/firebase-functions-test/pull/146