eiffel-community / eiffel-remrem-publish

eiffel-remrem-publish
https://eiffel-community.github.io/eiffel-remrem-publish
Apache License 2.0
8 stars 77 forks source link

/producer/msg accepts events with missing required fields. #202

Closed Isacholm closed 2 years ago

Isacholm commented 4 years ago

Description

When trying to send events with missing required fields, remrem should respond to the user that the event is invalid. Invalid events should not be published to rabbitmq.

When sending version 3.0.0 of EiffelAnnouncementPublishedEvent with the data.severity field missing (this field is described in the protocol as required), The response from remrem-publish is that the event was published fine (and it was).

response from /version:

{
  "serviceVersion": {
    "version": "2.0.8"
  },
  "endpointVersions": {
    "semanticsVersion": "2.0.6"
  }
}

Expected response from /producer/msg:

{"status_code":503,"result":"FAIL","message":"Message protocol is invalid"}

Actual response from /producer/msg:

{"events":[{"id":"2466ba4a-4540-4d22-b9e5-4565ac8de696","status_code":200,"result":"SUCCESS","message":"Event sent successfully"}]}

Motivation

This protects users from sending and reading invalid events on the message broker.

Exemplification

Benefits

Possible Drawbacks

zburswa commented 2 years ago

Hi,

REMReM publish doesnot perform any vadilation on the events. It takes the event and publishes it to the configured exchange in messagebus.

We use REMReM generate to generate events and these events are published using publish.

REMReM generate uses eiffel-semantics protocol for generating the events. In eiffel-semantics, schema is defined for each event and generates the event. We always use generate for events generation and if we try to generate EiffelAnnouncementPublishedEvent with missing data.severity field, it will throw an exception.

magnusbaeck commented 2 years ago

I'm not sure what you're saying here. Doesn't the /producer/msg endpoint pass the event to REMReM Generate (or the semantics library) for validation? Or are you saying that the bug report is incorrect and that AnnP events with the data.severity field missing actually are rejected?

m-linner-ericsson commented 2 years ago

So the /producer/msg does not do any validation. Have you experienced the same problem when using "generateAndPublish" endpoint?

magnusbaeck commented 2 years ago

Oh, that would certainly explain things. This difference isn't clear from the documentation.

m-linner-ericsson commented 2 years ago

Would it be enough to update the documentation for the /producer/msg endpoint and clarify that it doesn't validate the message?

magnusbaeck commented 2 years ago

If the documentation is clear on what the endpoint does I'm happy.