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

Handle references to path-items (3.1.1, 3.2) #71

Open
karenetheridge opened this issue Feb 28, 2024 · 8 comments
Open

Handle references to path-items (3.1.1, 3.2) #71

karenetheridge opened this issue Feb 28, 2024 · 8 comments
Labels
2024 Things I intend to do in 2024 blocked blocked on something else spec support something that the spec says should be supported v3.1.1 support v3.2.0 support

Comments

@karenetheridge
Copy link
Owner

karenetheridge commented Feb 28, 2024

Because /paths might contain references to path-items (this is allowed by the spec but not by the 3.1.0 schema, but may be fixed in 3.1.1), meaning the path-item object itself is not always at /paths/$path_template and the operation may not be at /paths/$path_template/$method:

  • a reference to the path-item object (after resolving references) should be returned in the $options hash populated by find_path, for use by callers
  • find_path should to include in the $options hash the full uri of the path-item found (this could be useful for logging, as well as returning in a response along with the validation result)
@karenetheridge karenetheridge added the spec support something that the spec says should be supported label Feb 29, 2024
@karenetheridge
Copy link
Owner Author

partial duplicate of #53

@karenetheridge
Copy link
Owner Author

relevant changes to the spec and spec schema:

@karenetheridge karenetheridge added the blocked blocked on something else label Mar 23, 2024
@karenetheridge karenetheridge changed the title Allow references to path-items Allow references to path-items (3.1.1, 3.2) Mar 23, 2024
@karenetheridge
Copy link
Owner Author

When traversing the document:

  • we already record the entity location of path-items (whether under /paths or /components/pathItems), so no changes are needed here.

When evaluating at runtime:

  • paths only live under /paths. so, find_path only needs to look in one place when matching URIs (at first).

  • but once it's found a path match, now it needs to find an http method match, and that could be on the other side of a $ref. so now we need to look for $ref under /paths/$path, and then follow the $ref (possibly recursively) and see if there is a matching http method there. Note that we cannot just follow the $ref chain until the end as we would for other entities; there could be other patn-item fields in between. so we need to do some special logic to collapse all the $ref chains together. Since the spec says "n case a Path Item Object field appears both in the defined object and the referenced object, the behavior is undefined." we can just die if we see a duplicated field when collapsing this data (this has to be done at runtime because $refs can cross document boundaries).

  • once we have a match, we'll have the location of the matched operation, which could be on the far side of a $ref. Although an operation can't be split across multiple locations, the relevant parameters for the matched operation could be at the path-item level or buried in an intermediary $ref, so we need to save that location separately for passing back to the caller of find_path. So perhaps instead we should construct a collapsed version of the path-item for storing in the $options hash.

  • when we follow the $ref uri inside a path-item definition, we'll need to verify that its target is a path-item object (via the entity registry). since we already need to write custom $ref-following code here (see above) we can add the entity validation here.

Maybe for 3.1.x we can just opt to not support $refs inside path-items at all (which is now allowed via OAI/OpenAPI-Specification#3355) - to avoid having to implement all the above logic. In 3.2.0 the logic is changing (OAI/OpenAPI-Specification#2657) -- I think it should change to a path-item entry either being a normal definition, OR just a $ref (but not a combination of both), so we can treat ref resolution here in the normal way when in find_path - when we find a matching path, resolve the entire $ref chain down to the end to find the final path-item object, where we'll find both the path-item-level parameters and the operation-level parameters, all together (because we don't use $refs at the operation level (yet!!)).

@karenetheridge karenetheridge changed the title Allow references to path-items (3.1.1, 3.2) Check for, and disallow, references to path-items (3.1.1) Mar 24, 2024
@karenetheridge karenetheridge changed the title Check for, and disallow, references to path-items (3.1.1) Handle references to path-items (3.1.1, 3.2) Mar 24, 2024
@karenetheridge
Copy link
Owner Author

karenetheridge commented Mar 24, 2024

For 3.1.1, we can check for and disallow $refs, even in 3.1.0, because the published schema allows for path-item-or-reference (albeit in a convoluted way that allows stacking, which is the bit we don't want to support).

For 3.2 we need to be serious about proper support, which also means we can't assume that path_item_path is under /paths, and also $state->{initial_schema_uri} may change after calling find_path (the path-item could be in another document), which means we need to pass the state object around a bit more. Note that any subsequent $ref keywords must be resolved against the new document, not the original.

We also aren't doing as many cross-checks between values in the options hash as we should be.

@karenetheridge
Copy link
Owner Author

karenetheridge commented Mar 24, 2024

If operation_id is provided to find_path, and the operation lives somewhere else and would be reached by a $ref, the calculated path_template is incorrect. We don't even need to use a $ref to /components/pathItems:

paths:
  /hello:
    $ref: '#/paths/~1goodbye'  
  /goodbye:
    get:
      operationId: greeting

If provided a request like GET /hello and operationId "greeting", we will use the operation_path of /paths/~1goodbye/get and then unjsonp it to ('', 'paths', '/goodbye', 'get') which will give us the correct path_item_path (after following $refs from /paths/~1hello) but the path_template for this, /goodbye, is not correct for this request.

If the path-item lives in /components, the unjsonp operation will fail entirely.
Also if the path-item lives in another document, the current mechanism for lookup up operation_paths will fail (see #73).

This also now means it would be no longer possible to call find_path with just operation_path and path_captures -- we cannot infer the path_template from operation_path as the operation might be shared between multiple paths. However, validate_request will always have the request available, so we can perform the path_template and path-item lookup from that, and validate_response doesn't care about anything in the path-item level at all: it doesn't use path_captures and only needs the operation-level data. Therefore, we should rework find_path to not error if it fails to calculate the path-item path or is not provided with the path_captures, and let validate_request bomb out instead. Or, alternatively, we should split up the functionality of find_path into separate methods, to allow performing only the core needs for validate_response.

@karenetheridge
Copy link
Owner Author

karenetheridge commented Mar 24, 2024

splitting up find_path:

for validate_response:

  • we only care about the operation location. This can be derived from path_template and method, or operation_id. path_template can be derived from request, and method can be defined from request.
  • we should perhaps expose operation_path_uri to the user, in addition to or instead of path_item_uri, because at the scope of the response we may not even know what the HTTP method is anymore (so we can't necessarily calculate it from path_item_uri), if the user only told us the operationId. However, we don't need to return it explicitly, nor force the user to calculate it from path_item_uri + method, because if the user already has the operationId they can just use a (not yet written) public method to find the operation id uri from the operationId (see fixes for multiple documents #73).

for validate_request:

  • we care about everything: the operation location, the path-item location, the path template and path capture variables, and the http method.

@karenetheridge
Copy link
Owner Author

Note that the semantics for $ref in path-item will be changing from what we initially thought -- https://github.com/OAI/OpenAPI-Specification/pull/3860/files

The published schema for 3.1.0 will be changing to allow sibling $ref, with all the complications of multiple overlapping definitions; we may want to simply not support this, or disallow adding multiple operation and parameter definitions at each sub-level of $ref (and only respect the definitions in the final object).

@karenetheridge karenetheridge added the 2024 Things I intend to do in 2024 label Jul 18, 2024
@karenetheridge
Copy link
Owner Author

see also OAI/OpenAPI-Specification#3734 - this won't be changing in 3.1.1 nor 3.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2024 Things I intend to do in 2024 blocked blocked on something else spec support something that the spec says should be supported v3.1.1 support v3.2.0 support
Projects
None yet
Development

No branches or pull requests

1 participant