Add support for schema_uri validation

[afrittoli]

Custom schemas can be loaded into the SDK.
If a consumed event includes a custom schema, it can be loaded from the SDK local DB and the event can be validated accordingly.

Added an example about parsing an event with a custom schema.

[codecov]

Codecov Report

Attention: Patch coverage is 94.73684% with 22 lines in your changes missing coverage. Please review.

Project coverage is 82.94%. Comparing base (080d904) to head (a5e116d).
Report is 4 commits behind head on main.

Files	Patch %	Lines
pkg/api/zz_ztest_foosubjectbarpredicate_1_2_3.go	0.00%	8 Missing ⚠️
pkg/api/schemas.go	62.50%	3 Missing and 3 partials ⚠️
pkg/api/bindings.go	66.66%	2 Missing and 2 partials ⚠️
pkg/api/zz_ztest_foosubjectbarpredicate_2_2_3.go	50.00%	2 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #87      +/-   ##
==========================================
+ Coverage   82.29%   82.94%   +0.64%     
==========================================
  Files         101      101              
  Lines        8028     8435     +407     
==========================================
+ Hits         6607     6996     +389     
- Misses       1196     1209      +13     
- Partials      225      230       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[xibz]

Overall looks good, I think I had one comment that is necessary for me to approve this which was the silently failing one.

[xibz]

We really should look into optional parameters at some point. Makes using things a little easier.

event, err := cdeventsv04.NewCustomTypeEvent(cdevents.CustomTypeEventOptions.WithSchemaURI("my-schema"))

This adds some flexibility on where we can add validation as well, as in we could choose to do it upon creating the struct with a cdevents.WithQuickValidate() or something as an option

[afrittoli]

I was debating between explicit setters/getters and providing some kind of context object where users could stash fields to be set:
event.WithSubjectId("foo").WithSource("bar")

But that leads to as many WithX methods as SetX methods to be defined, and I didn't really see an advantage. For the subject content, I introduced SetSubjectContent which allows setting the whole content from a Struct directly rather than field by field.

I'd like to better understand what you are proposing here, maybe something for the next WG.

[xibz]

Why don't we wrap the cloudevent library to hide all this from the user? This example would then just be something like

client, err := cdevents.NewHTTPClient()
if err != nil {
    panic(err)
}

result, err := client.Send(event)

This becomes much simpler I feel like

[afrittoli]

This is on purpose - the CloudEvents API is rich, as it supports multiple transport options each with its own configuration capabilities. I don't think we should replicate that on CDEvents side, transport is not CDEvent's concern.

[xibz]

Wrapping it does not mean a user cannot pass their own cloudevent client in. That can be done via optional params. This is standard in a lot of go sdks

[afrittoli]

I'm not convinced that:

client, err := cdevents.NewClientCloudEventsHTTP()

is going to provide a better user experience than:

client, err := cloudevents.NewClientHTTP()

The only difference is the import of cloudevents, which is required anyway unless we wrap the IsUndelivered method and others that may be required.

[xibz]

This is where my experience in SDKs come in. If you wrap it, you have full control on what it looks like to the user. You can add hooks, etc. With your current implementation you cannot without fully understanding two SDKs. Two! That's a bad experience

[xibz]

So this just silently fails if it is not a CDEventReaderV04? We already return errors, we should just return an error?

Unless this event reader can be something else, we should make it an interface in that case if it already isn't

[afrittoli]

Uhm, it doesn't fail. It checks if it's a CDEventReaderV04, and if it is it runs extra validation. If it isn't there's nothing else to be done in terms of validation.

[xibz]

Should we allowing skipping? That seems really bad. Especially if they expect it to validate and it doesnt if it isnt that type

[xibz]

Okay, after looking at this code some more. I wonder if this is the best approach. If we have multiple versions, would we keep appending if statements? I personally like the handler pattern for things like this. I'll add a github issue for that

[xibz]

I think most of my comments is re-architecturing/refactoring, so I think with that said it isn't in scope for this PR. I'll go ahead and approve and follow up with the appropriate GH issues