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

Support oneOfs & anyOfs in operation request & response schema #701

Open
wants to merge 10 commits into
base: dev
Choose a base branch
from

Conversation

lgarber-akamai
Copy link
Contributor

@lgarber-akamai lgarber-akamai commented Dec 12, 2024

📝 Description

This pull request adds support for parsing OpenAPI oneOfs and anyOfs in both the request and response schema for operations.

Additionally, this PR adds support for rendering each root-level request oneOf separate section in the generated command help pages.

✔️ How to Test

The following test steps assume you have pulled down this pull request locally and run the following command:

make SPEC='https://gist.githubusercontent.com/lgarber-akamai/07485a643e8329a5c16bec775e8066c4/raw/56f4491ba6e239c9cad736f05ff2d1ab0608098c/openapi.json' install

NOTE: The overridden specification is necessary because the currently published OpenAPI specification does not yet make use of oneOfs.

Unit Testing

make testunit

Integration Testing

make testint

Manual Testing

  1. Run the following command the view the help page of the nodebalancers config-create command:
linode-cli nodebalancers config-create --help
  1. Ensure the output contains separate categories for each configuration protocol.
  2. Create a NodeBalancer using the following command:
linode-cli nodebalancers create \
  --region us-mia \
  --label my-nodebalancer
  1. Create a NodeBalancer configuration using the following command, replacing the MYIDHERE with the ID of the newly created NodeBalancer:
linode-cli nodebalancers config-create MYIDHERE \
  --port 440 \
  --protocol http \
  --algorithm roundrobin \
  --stickiness http_cookie \
  --check http_body \
  --check_interval 90 \
  --check_timeout 10 \
  --check_attempts 3 \
  --check_path "/test" \
  --check_body "it works"
  1. Ensure the NodeBalancer configuration has been created successfully.

📷 Preview

Improved Help Page

Improved Help Page

@lgarber-akamai lgarber-akamai added the new-feature for new features in the changelog. label Dec 12, 2024
@lgarber-akamai lgarber-akamai changed the title Support root-level oneOfs in operation request schema Support root-level oneOfs in operation request & response schema Dec 12, 2024
Comment on lines 142 to 204
v,
prefix=pref,
parent=parent,
# NOTE: We do not increment the depth because dicts do not have
# parent arguments.
depth=depth,
)
elif (
v.type == "array"
and v.items
and v.items.type == "object"
and v.extensions.get("linode-cli-format") != "json"
):
# handle lists of objects as a special case, where each property
# of the object in the list is its own argument
pref = prefix + "." + k if prefix else k

# Support specifying this list as JSON
args.append(
OpenAPIRequestArg(
k,
v.items,
False,
prefix=prefix,
is_parent=True,
parent=parent,
# NOTE: We do not increment the depth because dicts do not have
# parent arguments.
depth=depth,
)
elif (
v.type == "array"
and v.items
and v.items.type == "object"
and v.extensions.get("linode-cli-format") != "json"
):
# handle lists of objects as a special case, where each property
# of the object in the list is its own argument
pref = prefix + "." + k if prefix else k

# Support specifying this list as JSON
args.append(
OpenAPIRequestArg(
k,
v.items,
False,
prefix=prefix,
is_parent=True,
parent=parent,
depth=depth,
)
)
)

args += _parse_request_model(
v.items,
prefix=pref,
parent=pref,
depth=depth + 1,
)
else:
# required fields are defined in the schema above the property, so
# we have to check here if required fields are defined/if this key
# is among them and pass it into the OpenAPIRequestArg class.
required = False
if schema.required:
required = k in schema.required
args.append(
OpenAPIRequestArg(
k,
v,
required,
prefix=prefix,
parent=parent,
depth=depth,
)
args += _parse_request_model(
v.items,
prefix=pref,
parent=pref,
depth=depth + 1,
)
else:
# required fields are defined in the schema above the property, so
# we have to check here if required fields are defined/if this key
# is among them and pass it into the OpenAPIRequestArg class.
required = False
if schema.required:
required = k in schema.required
args.append(
OpenAPIRequestArg(
k,
v,
required,
prefix=prefix,
parent=parent,
depth=depth,
)
)

Copy link
Contributor Author

@lgarber-akamai lgarber-akamai Dec 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic is all the same, I just replaced the wrapping if statement with a guard clause

@lgarber-akamai lgarber-akamai changed the title Support root-level oneOfs in operation request & response schema Support root-level oneOfs & anyOfs in operation request & response schema Dec 12, 2024
@lgarber-akamai lgarber-akamai marked this pull request as ready for review December 12, 2024 22:13
@lgarber-akamai lgarber-akamai requested a review from a team as a code owner December 12, 2024 22:13
@lgarber-akamai lgarber-akamai requested review from jriddle-linode and zliang-akamai and removed request for a team December 12, 2024 22:13
if (
v.type == "object"
and not v.readOnly
and len(_aggregate_schema_properties(v)[0]) > 0
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is necessary to account for nested anyOfs and oneOfs.

@lgarber-akamai lgarber-akamai changed the title Support root-level oneOfs & anyOfs in operation request & response schema Support oneOfs & anyOfs in operation request & response schema Dec 13, 2024
Copy link
Collaborator

@jriddle-linode jriddle-linode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Working locally, looks good to me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-feature for new features in the changelog.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants