Callback key can be a string with embedded RuntimeExpression

[PerthCharern]

@darrelmiller

Issue from @epet, @Shwetap05

Currently we only allow RuntimeExpression in a key in a Callback object.

        /// <summary>
        /// A Path Item Object used to define a callback request and expected responses.
        /// </summary>
        public Dictionary<RuntimeExpression, OpenApiPathItem> PathItems { get; set; }
            = new Dictionary<RuntimeExpression, OpenApiPathItem>();

This is not correct given an example in the spec: https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.0.md#callback-object-example

myWebhook:
  'http://notificationServer.com?transactionId={$request.body#/id}&email={$request.body#/email}':
    post:
      requestBody:
        description: Callback payload
        content: 
          'application/json':
            schema:
              $ref: '#/components/schemas/SomePayload'
      responses:
        '200':
          description: webhook successfully processed and no retries will be performed

Note that the "expression" key above is a URL with RuntimeExpression embedded in.

Note: I think the spec is a bit ambiguous. The spec states this: "The key that identifies the Path Item Object is a runtime expression that can be evaluated in the context of a runtime HTTP request/response to identify the URL to be used for the callback request."

The runtime expression in the spec is strictly defined. There's a bit of leeway with this sentence "Runtime expressions preserve the type of the referenced value. Expressions can be embedded into string values by surrounding the expression with {} curly braces.", but this sentence is in the Examples section of runtime expression section and it contradicts with the definition of runtime expression directly above: https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.0.md#examples-1

I would say we are actually faithful to the spec with the current implementation - though probably not aligned with the spirit of the spec.

[PerthCharern]

tl;dr;

We will need to modify our RuntimeExpression class to handle cases like:

http://notificationServer.com?transactionId={$request.body#/id}&email={$request.body#/email}

where there are "expressions" in { } in a string.

[darrelmiller]

I've created a CompositeExpression class that will contain the outer template string and then create child RuntimeExpression objects for the parts inside {}. As we don't actually do the runtime resolution yet, this shouldn't be too difficult to complete.

[darrelmiller]

Fixed in v1.1.0-preview.4

[Shwetap05]

Darrel, looks like this issue is still not fixed in 1.1

[PerthCharern]

There's a bug in the expression parser. We are checking for {$ to determine if it's a composite expression. If the expression is a simple string like /abcd, then we are not parsing it correctly.

[darrelmiller]

Yeah. I just fixed it to look for Contains("$")

[darrelmiller]

Wait. It's not an issue in the expression parser. It was in the callback deserializer

[darrelmiller]

@Shwetap05 I have a fix pending for the example that Perth included above. However, it will not handle the case when the callback key does not have any runtime expression. i.e. when it is a string literal. This scenario is not one that is currently supported by the OpenAPI specification. Perhaps it should be, but I will need to bring this up with the Technical Steering Committee to see if we can get the spec updated to add support for this use case.

If the customer could add an issue on the OpenAPI repo that would help me make the case for it.

[PerthCharern]

I would argue that even in the current spec version, there's already a bit of an ambiguity in whether the constant /abcd scenario "is one that is currently supported by the OpenAPI specification."

---

From the spec:

Expressions can be embedded into string values by surrounding the expression with {} curly braces.

This makes it sound like if we are okay with treating a composite expression as a runtime expression, then we should also be okay with including a literal string as a composite expression too. The word "string values" above do not specify any restrictions on the string.

---

At the end of the day, it sounds like a string literal is a valid key for the callback - it's just a matter of allowing it in the Callback Object or in the Runtime Expression.