Open JoanComasFdz opened 3 months ago
CloudEvent
is sealed because I agree with the adage of "design for inheritance or prohibit it". (I usually prefer classes to be sealed or abstract.) I don't expect to change that.
Even if CloudEvent
weren't sealed, I'm not sure what you'd do - you couldn't stop the Data
property of type object
from being used, so there'd always be the possibility of having an event with the "wrong" type of data. The example you've given has both Data
and Payload
, which seems odd to me - wouldn't you expect the payload to be the data and vice versa? Passing your derived class to any of the existing formatters wouldn't do the right thing - and not that CloudEvent
itself is not expected to be serialized using regular JSON serializers.
In terms of what I'd do instead: I'd avoid implicit potentially-lossy operators, preferring methods for conversions - but other than that the way of creating a CloudEvent
seems reasonable. Your way of converting from a CloudEvent doesn't look appropriate to me though - it depends on how you create the CloudEvent
in the first place, but I wouldn't normally call Data.ToString()
and expect to get back JSON. (I don't understand how you're using Activator.CreateInstance
to create an instance of an abstract type either, but maybe that's something records support somehow? If there are multiple concrete types derived from BaseEvent<TPayload>
which would you expect to be used?)
As for why it's a class and not a record: we could potentially have designed it for immutability from the start, but that certainly wasn't the case in v1, and while my changes for v2 were fairly significant, I think making them immutable would have been a much bigger change. (Note that the repo was started in 2018, two years before record types were available in C#...)
Thanks a lot for the clarifications and feedback! They all make sense to me.
It was a rough, very first attempt, not a final version. So as you said, the Activator.CreateInstance
wouldn't work.
So the way I am understanding cloudevents spec right now is that there are 3 types of properties here:
Auto generated properties:
Identification properties:
Payload:
(I guess we can argue about some of them being in the right place)
On event declaration time, devs should only specify the identification properties:
public record OrderCreatedEvent: BaseEent<OrderInfo>
{
public OrderCreatedEvent(OrderInfo payload, string subject) : base("my.event", "http://service.company.com/api", payload, subject) {}
}
On event instantiation time, devs should only specify the payload and subject:
var orderInfo= new OrderInfo(...);
var orderCreatedEvent = new OrderInfo(orderinfo, "order.created/123"); // id, spec, time are autoinitialized, datacontenttpye is extracted from payload, somehow
publisher.Publish(orderCreatedEvent );
On publishing time, infrastructure should convert it into a CloudEvent.
var ce = new CloudEvent { Type = customEvent.Type, ...};
On receving time, infrastructure should parse the body to a CloudEvent, then map it to a strongly typed event.
var cloudEvent = new JsonEventFormatter().DecodeStructuredModeMessage(body, ...);
// map the cloudEvent to a customEvent (OrderCreatedEvent) => THE problem.
The biggest issue I am facing is how to achieve the last part, without forcing to declare to many things in the MyEvent
, like a copy constructor (which will be boilerplate code) and keeping immutability. And i'd like to minimize the amount of reflection used, for performance reasons.
I am starting to lean towards a code generator to generate the boilerplate code for the copy constructor.
But any ideas are welcome!
If you put a constructor in a common base type with a CloudEvent
parameter, can't that deal with extracting the various individual aspects from a CloudEvent
? That way the only boilerplate you might need would be a constructor in each concrete event type to chain to the base constructor:
public record OrderCreatedEvent(... /* whatever you need here, if anything */) : BaseEvent<OrderInfo>
{
public OrderCreatedEvent(CloudEvent evt) : base(evt) {}
}
The deserialization of the JSON to the CloudEvent
in the first place, selecting the right concrete type for Data
is another area you may have work to do - I've previously created a CloudEventFormatter
that delegates to a specific parser based on the event type - see https://github.com/GoogleCloudPlatform/functions-framework-dotnet/pull/230/files#diff-4de2945379de39ce530bb34eeac4543817d16da00fe305f476e2cb6cbee4aed6 for a prototype.
Hi all,
I have bee working in event-driven architectures using RabbitMQ for the last years, and the approach, for relatively small products, is that the project that owns an event is declaring the event in a separate project like (oversimplifying):
A library receives the instance and serializes it all to publish it to RabbitMQ. And on the subscribing side, it uses the class definition to deserialize the string and pass it onto the subscription.
This allows all actors (publishers, subscribers, tests, etc) to use a single definition for a single event while freeing all the code from serialization, deserialization and casting.
I am looking to migrate this implementation to CloudEvents, so the first thing I thought about was:
But
CloudEvent
is sealed :(My first alternative is to declare my BaseEvent with operators to cast to and from
CloudEvent
:But before I move forward, I am double checking:
An additionally:
CloudEvent
sealed?Thanks