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

[2.0.0 REVIEW] Thoughts on a selection expression for models #238

Open
jasonmac01 opened this issue Jul 17, 2019 · 10 comments
Open

[2.0.0 REVIEW] Thoughts on a selection expression for models #238

jasonmac01 opened this issue Jul 17, 2019 · 10 comments
Labels
keep-open Prevents stale bot from closing this issue or PR 👻 Needs Champion RFC Needs a champion to progress (See CONTRIBUTING.md) 💭 Strawman (RFC 0) RFC Stage 0 (See CONTRIBUTING.md)

Comments

@jasonmac01
Copy link

In regards to the oneOf support for a message within the Operation object:

Having the concept of "multiple message shapes can be received over this channel operation" is very useful - Perfect!

However, what indicates to the recipient which message should be chosen at runtime?

Within AWS API Gateway, we support a concept of a modelSelectionExpression on our Routes - I think this roughly aligns with your Runtime Expression concept at a high level. Do you think this sort of runtime expression should be added within the Operation?

-Jason
AWS API Gateway

@fmvilas
Copy link
Member

fmvilas commented Jul 18, 2019

The meaning of oneOf is that your message should match one (and only one) of those at any time, meaning that there's no way to understand which one is going to be matching. At runtime you should evaluate your message against all of them to 1) Make sure it only matches against one, and 2) understand which one is "the matching one". Does it make sense?

@jasonmac01
Copy link
Author

Sure, I suppose iterating through the collection is a possibility, but perhaps a bit inefficient. It may actually be error-prone as well when shapes have overlapping attribute names.

What would be your honest opinion on having an expression that allowed the actor (message broker, producer, or consumer) to extract a "key" that was then used to lookup the appropriate model?

@fmvilas
Copy link
Member

fmvilas commented Jul 18, 2019

Sure I would like to have something like this but don't forget the constraint that it should only validate against one of them. How are you going to guarantee that constraint if you don't evaluate all of them? And it's an honest question, I don't have an answer for this.

@jasonmac01
Copy link
Author

The selection expression is what solves this guarantee problem.

As it currently stands, just having oneOf seems ambiguous. What if the payload could in fact be serialized to multiple models? Do you just pick the first, or maybe pick the one with the most matching properties? Either way, that's expensive to accomplish and difficult to predict.

With a selection expression, the developer is able to model how the actor receiving the message should choose the appropriate model.

This would guarantee that only one model is chosen because the models would be stored in a map, whereby the keys are inherently unique. Here's an example:

subscribe:
  messageSelectionExpression: "$message.header#Content-Type"
  message:
    selection:
      application/json: 
        $ref: '#/components/messages/UserJsonMsg'
      application/xml: 
        $ref: '#/components/messages/UserXmlMsg'

I.e. Choose the message/model based on the header's Content-Type value.

@fmvilas
Copy link
Member

fmvilas commented Jul 26, 2019

Oh ok! So you're advocating for a totally different way without using oneOf. That is a different beast and looks like it makes sense to have something like this.

But let me clarify something, when you say "What if the payload could in fact be serialized to multiple models?" that's exactly what I meant before. It cannot happen. If that happens then it's an error. If you make use of oneOf, the payload MUST be defined by only one of the definitions.

This is how you'd do this right now:

subscribe:
  oneOf:
    - $ref: '#/components/messages/UserJsonMsg'
    - $ref: '#/components/messages/UserXmlMsg'

components:
  messages:
    UserJsonMsg:
      payload:
        ...
      bindings:
        http:
          headers:
            Content-Type:
              type: string
              enum: ['application/json']
    UserXmlMsg:
      payload:
        ...
      bindings:
        http:
          headers:
            Content-Type:
              type: string
              enum: ['application/xml']

But I get your point. It'd be great to have a routing mechanism at operation level. I'll label this as "Under Consideration" for now. Thanks!

@jasonmac01
Copy link
Author

I think I see your point - you're suggesting that the spec is mandating that all messages be mutually exclusive. I suppose that works, but it puts a lot of work on any implementors trying to validate a definition. It may also be constraining in some cases.

Let me just run through one more example as documentation. I'm not expecting a response.


Rather than giving an example of XML versus JSON, let's use this example which is more agnostic of serialization method:

  oneOf:
    - $ref: '#/components/messages/Human'
    - $ref: '#/components/messages/SuperHero'

In this example, a SuperHero is a superset of Human. Same properties plus a few more.

If this is invalid because of the specification enforcement, then that could be too constraining, right?

If this is valid because they are in fact different models, then it presents a problem related to deserialization. Think of this issue from the perspective of automated code generation. The underlying deserializer would be able to deserialize an inbound Human message to either a Human or a SuperHero model. Which would be chosen?

As an alternative, if the entire response had some indicator to suggest the model to be chosen, then it enables a simplified code generation and validation process (lookup the key somewhere in the entire payload, map it to a unique model).

@fmvilas
Copy link
Member

fmvilas commented Jul 30, 2019

Agree there should be another mechanism for message selection, aside from oneOf. Actually, the purpose of oneOf wasn't about message selection but to act as a restrictive constraint. Usually, there's a field like type inside your message that would indicate what type of message it is. It should serve for both, uniqueness and message routing/selection.

@ciaranodonnell
Copy link

This last idea with the type field is how OneOf works in protocol buffers.
When you specify a OneOf, the code generation (for c# at least) makes an enum field that specifies which one is present.
Heres a link to the protobuf.net docs to explain that approach.
https://developers.google.com/protocol-buffers/docs/reference/csharp-generated#oneof

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity 😴
It will be closed in 30 days if no further activity occurs. To unstale this issue, add a comment with detailed explanation.
Thank you for your contributions ❤️

@github-actions github-actions bot added the stale label Mar 12, 2020
@fmvilas fmvilas added keep-open Prevents stale bot from closing this issue or PR and removed stale labels Mar 13, 2020
@fmvilas fmvilas added this to the AsyncAPI specification 2.1.0 milestone Mar 13, 2020
@fmvilas fmvilas modified the milestone: AsyncAPI specification 2.1.0 Jan 31, 2021
@smoya smoya added 👻 Needs Champion RFC Needs a champion to progress (See CONTRIBUTING.md) 💭 Strawman (RFC 0) RFC Stage 0 (See CONTRIBUTING.md) labels Feb 5, 2024
@smoya
Copy link
Member

smoya commented Feb 5, 2024

I just added the Strawman and Needs Champion label because I consider this a nice feature someone could be willing to champion.
I didn't read any request to this besides this issue anyway but let's keep it opened for some more time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
keep-open Prevents stale bot from closing this issue or PR 👻 Needs Champion RFC Needs a champion to progress (See CONTRIBUTING.md) 💭 Strawman (RFC 0) RFC Stage 0 (See CONTRIBUTING.md)
Projects
None yet
Development

No branches or pull requests

4 participants