src(receiver): don't default specversion on receiving if there isn't one.

[lholmquist]

• When receiving an event, we should not default the specversion to 1.0 if no specverion field is detected.

• If no specversion is detected or the specversion does not conform to a currently supported version, we will now throw a ValidationError

• This is a BREAKING_CHANGE

fixes #332, #333

Signed-off-by: Lucas Holmquist [email protected]

[lholmquist]

Did we need to id also?

[lance]

@lholmquist thinking about this... when we were discussing validation not too long ago, the point was to have loose validation on receipt of a cloud event (since you could be receiving an event from a "faulty" system). But we could give the user to explicitly validate afterwards with the validate() function. I think probably a misspoke when I said we should throw (sorry!).

The goal would be to create an event without the specversion (or ID) set, and allow for user validation to deal with errors. The only problem is how to do this without changing the existing behavior for users who just want to create an event. Ideally they really shouldn't be required to specify a version - it should just default to 1.0.

[lholmquist]

let me read that discussion, #328 (comment), and get back to you

[lholmquist]

Just want to the workflow straight in my head. A new event/message comes in and is received. The specversion is not something this sdk know about or is missing. No error happens and a new cloudevent is created. The user then calls the CE validate function and receives an error because of the non-valid specversion field, they handle it.

I think the only thing we should change is that we don't default the value, since with the loose validation, it is working as intended. And if a user needs to handle the error, I think they can just use the CE.cloneWith method?

[lance]

I think the only thing we should change is that we don't default the value, since with the loose validation, it is working as intended. And if a user needs to handle the error, I think they can just use the CE.cloneWith method?

I'm kind of torn on this... Requiring a user to specify the spec version when creating a new event seems a little unfortunate, since it will pretty much always be Version.V1.

[lholmquist]

I'm kind of torn on this... Requiring a user to specify the spec version when creating a new event seems a little unfortunate, since it will pretty much always be Version.V1.

I think we should still default when using the new CloudEvent() constructor. There error would only happen if a user runs validate when receiving an Event from somewhere, at least that is how i'm understanding the issue. So, is there really an issue with us defaulting to v1?

[lholmquist]

@lance i wondering what the next steps are here. This PR is probably not correct the way it is written after reading some of our discussions, i wonder if there really is an issue at all?

[lance]

@lholmquist sorry - I have been heads down on an upcoming release and have not had much time to think about this issue. Honestly, I think the way it is now is ok but not great. However, I'm not opposed to just letting this sit and stew for a while.

[lholmquist]

Honestly, I think the way it is now is ok but not great. However, I'm not opposed to just letting this sit and stew for a while.

Ok, i'm going to close this PR, since it is not the correct way of doing it, if we thought about doing it. Are we still blocked by this for the 4.0 release then?

[lance]

Are we still blocked by this for the 4.0 release then?

I think we are probably good to release next week.