Skip to content
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

Whats the suggested way to declare strongly typed events? (And why is CloudEvent sealed?) #308

Open
JoanComasFdz opened this issue Aug 17, 2024 · 3 comments

Comments

@JoanComasFdz
Copy link

JoanComasFdz commented Aug 17, 2024

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):

public record MyEvent(string topic, ..., MyPayload payload) : BaseEvent { ... }

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:

public record BaseEvent<T> : CloudEvent
{
   // constructor...
   
   // json attribute
   public TPayload Payload {get:}
}

But CloudEvent is sealed :(

My first alternative is to declare my BaseEvent with operators to cast to and from CloudEvent:

using CloudNative.CloudEvents;
using System.Text.Json;

public abstract record BaseEvent<TPayload>
{
    public string Id { get; init; } = Guid.NewGuid().ToString();
    public string Type { get; init; }
    public Uri Source { get; init; }
    public string Subject { get; init; }
    public DateTimeOffset Time { get; init; } = DateTimeOffset.UtcNow;
    public TPayload Data { get; init; }

    protected BaseEvent(TPayload data, string source, string type, string subject = null)
    {
        Data = data;
        Source = new Uri(source);
        Type = type;
        Subject = subject;
    }

    public static implicit operator CloudEvent(BaseEvent<TPayload> baseEvent)
    {
        return new CloudEvent
        {
            Id = baseEvent.Id,
            Type = baseEvent.Type,
            Source = baseEvent.Source,
            Subject = baseEvent.Subject,
            Time = baseEvent.Time,
            Data = JsonSerializer.Serialize(baseEvent.Data),
            DataContentType = "application/json"
        };
    }

    public static implicit operator BaseEvent<TPayload>(CloudEvent cloudEvent)
    {
        var data = JsonSerializer.Deserialize<TPayload>(cloudEvent.Data.ToString());
        var baseEvent = Activator.CreateInstance(typeof(BaseEvent<TPayload>), data, cloudEvent.Source.ToString(), cloudEvent.Type, cloudEvent.Subject) as BaseEvent<TPayload>;
        baseEvent.Id = cloudEvent.Id;
        baseEvent.Time = cloudEvent.Time.Value;
        return baseEvent;
    }
}

But before I move forward, I am double checking:

  • What would be the proposed way to declare cloud events strongly typed?

An additionally:

  • Why is CloudEvent sealed?
  • Why is it a class and not a record?

Thanks

@jskeet
Copy link
Contributor

jskeet commented Aug 19, 2024

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#...)

@JoanComasFdz
Copy link
Author

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:

  • id
  • specversion
  • time

Identification properties:

  • type
  • source

Payload:

  • data
  • datacontenttype
  • subject

(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!

@jskeet
Copy link
Contributor

jskeet commented Aug 21, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants