Repository navigation
A CloudEvent should be readonly but provide a way to augment itself #233
Description
With recent API changes, a CloudEvent is nicely represented as a plain old JS object. For example, for most purposes, this is actually a valid CE.
const ce = {
source: '/',
type: 'example',
specversion: 'v1.0',
id: 'asd0aan3412juv'
}However, because we want to have spec compliance checks, and provide default values (e.g. a UUID or timestamp), we have a constructor and a class. The example above can be written as:
const ce = new CloudEvent({
source: '/',
type: 'example',
});Here, code in the constructor is providing default values for id and specversion.
However this introduces some tricky edge cases. For example, extensions. According to the spec, extensions must exist as siblings of the CE properties. And JS being as loosey goosey as it is, this means a user can do some things outside of the spec boundaries. For example
const ce = new CloudEvent({ source: '/', type: 'example' });
ce[extension-1] = // <- note invalid extension name
{
foo: 'bar' // <-- note invalid extension value
};A solution to this is to
- At the bottom of the
CloudEventconstructor, callthis.validate() - Make the
CloudEventobject read only just after validation withObject.freeze(this); - Provide a "builder" of some kind that enables cloning an event with new properties/extensions, e.g.
myEvent.cloneWith( { extension: 'value' });(this could be a function onCloudEventor a class unto itself.
Note that both #228 and #229 should be implemented and added to the validation step as a part of this.
Related: #29
Activity
lholmquist commentedon Jun 30, 2020
Sounds good. Going to assign to myself
In the example, you are just talking about extensions, but i'm assuming this could go for any other value, like source or type or version, etc..?
lance commentedon Jun 30, 2020
Yes. Though it's obviously not required, since using
Object.freeze()doesn't prevent the object's properties from being changed. I could, for example, change the timestamp on a "readonly" CE.freeze()just prevents adding and removing properties. We could get fancy and use TS to make the properties themselvesreadonly. I'm pretty sure under the covers, TS just uses the property descriptor to do that. Hmm... maybe this should be part of the change to really enforce the readonly aspect of the object.lholmquist commentedon Jul 1, 2020
Something i was thinking about was trying to use a Proxy for this instead of Freezing the object and having another method that would be needed to "update" the CloudEvent.
This way, whenever a property is updated, the Proxy can "trap" the set call and run the validate method.
So doing something like this:
would trigger the validate method
lholmquist commentedon Jul 1, 2020
Although, doing it this way(using a Proxy) would not satisfy issue #29
@lance @grant any thoughts on using the Proxy approach. I'm sort of leaning toward the Proxy approach if it works(testing things out now) instead of having a separate method to update the object
lholmquist commentedon Jul 1, 2020
just realized not everyone might know what this Proxy object is, https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Proxy
grant commentedon Jul 1, 2020
How about we make CloudEvents immutable like they aught to be and use
Object.preventExtensions(),and make it really easy to construct new CloudEvents?
grant commentedon Jul 1, 2020
I think #29 should be addressed first before this issue.
lholmquist commentedon Jul 1, 2020
After doing a bit of research on Proxies and a WIP PR #234 , it might be easier/more efficient to have a method an a cloudEvent that takes fields to update and merges then with the existing CloudEvents properties and then returns a new CloudEvent.
sort of what @lance suggested with
myEvent.cloneWith( { extension: 'value' });:)While i do like the proxy approach, the implementation was starting to get a bit convoluted.
grant commentedon Jul 1, 2020
I would expect CloudEvents to be lightweight and immutable.
If you want to clone or slightly modify a CloudEvent, I'd propose this radical idea:
Create a
CloudEventfrom aCloudEvent(with additional overridden attributes):https://github.com/cloudevents/sdk-javascript/blob/master/src/event/cloudevent.ts#L42
Or I could imagine a new
clone(overrides)method.lholmquist commentedon Jul 1, 2020
Yup. Doing something now. Probably sticking to the "cloneWith" method
[-]A CloudEvent should be readonly but provide a way to augment itsef[/-][+]A CloudEvent should be readonly but provide a way to augment itself[/+]