OAI / Arazzo-Specification

The Arazzo Specification - A Tapestry for Deterministic API Workflows
https://spec.openapis.org/arazzo/latest.html
Apache License 2.0
214 stars 43 forks source link

Review `operationRef` templating verbiage #130

Closed frankkilcommins closed 7 months ago

frankkilcommins commented 9 months ago

From @handrews feedback review:

Regarding operationRef:

A relative or absolute URI reference to an OAS operation. This field is mutually exclusive of the operationId and workflowId fields respectively. A complete URI Template SHOULD be used. The operation being referenced MUST be described within one of the source descriptions.

Why is this a URI Template? Why is it different from OAS templating? I philosophically approve of using standard URI Templates, but you're using them with a Parameter Object including style which is designed for OAS templating (the Parameter Object explicitly references OAS templating, and makes no mention of RFC 6570 at all). And it is unclear how other features of URI Templates are intended to interact with the Parameter Object. I realize this was probably discussed somewhere in this repository, but it's surprising enough that I think it needs addressing in the specification. I would not understand how to implement this as it is.

ndenny commented 9 months ago

From the meeting - suggest we enforce use of $sourceDescriptions - so: operationRef: $sourceDescriptions.<name>#/path/to/operation [or is it operationRef: $sourceDescriptions.<name>.url#/path/to/operation ]

which would expand for tooling to be a URI as ($sourceDescriptions.<name>.url) + #path/to/operation

handrews commented 9 months ago

Adding a note from my call with @frankkilcommins : we're fairly sure that the requirement to use a URI Template (RFC 6570) instead of a URI-reference (RFC 3986) is some sort of editing error or is left over from a different idea for how this works. The sentence should just be removed as the "relative or absolute URI" language is sufficient.

gcatanese commented 9 months ago

From the meeting - suggest we enforce use of $sourceDescriptions - so: operationRef: $sourceDescriptions.<name>#/path/to/operation [or is it operationRef: $sourceDescriptions.<name>.url#/path/to/operation ]

which would expand for tooling to be a URI as ($sourceDescriptions.<name>.url) + #path/to/operation

Would it make sense to enforce it for operationId too (i.e. $sourceDescriptions.<name>.<operationId>)?

handrews commented 9 months ago

operationRef: $sourceDescriptions.<name>#/path/to/operation [or is it operationRef: $sourceDescriptions.<name>.url#/path/to/operation ]

OK so just to be clear, this operationRef is intentionally going to behave differently from the OAS Link Object's operationRef?

It looks to me like the objective is to avoid using URIs directly and instead always use imported sourceDescription identifiers. I think this is a very good idea. Moonwalk is also moving towards named imports.

Might I suggest something like operationPath instead? This captures the fact that you need a JSON Pointer path (presumably because the operation does not have an operationId), and avoids the cognitive dissonance of operationRef meaning something slightly different here than in OAS.

Since you are using plain text JSON Pointers elsewhere, it would make sense to use them for operationPath as well, instead of using the URI fragment syntax with its extra layer of escaping:

  operationPath: $sourceDescriptions.<name>./plain/text/json/pointer/to/operation
ndenny commented 9 months ago

Gave it shot in #144

ndenny commented 9 months ago

Maybe I'm opening a can of worms here, but I was paying enough in some past TSC meetings to know this could be a problem:

As operationPath is for referencing a operation that does not have an operationId defined, but OpenAPI descriptions can span multiple documents (or is that files?) - this does mean the sourceDescription has to point to the document with the operation in it? It's not like the JSON Pointer has any de-referencing ability.

frankkilcommins commented 7 months ago

I believe this issue can now be closed