Skip to content

Endpoints with hyphens in parameter names trigger Incorrect path templating warning, are not generated #976

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

Closed
harabat opened this issue Feb 22, 2024 · 0 comments · Fixed by #986

Comments

@harabat
Copy link
Contributor

harabat commented Feb 22, 2024

Describe the bug

openapi-python-client fails to generate endpoints and throws incorrect path templating warnings when the path has a parameter with a hyphen.

Renaming the parameters fixes the issue. Hyphens in parameter names don't seem to go against Swagger/OpenAPI naming conventions, which would indicate that the issue is with openapi-python-client, though I haven't found any existing issues around this.

OpenAPI Spec File

This is from an OpenAPI spec generated from a Swagger spec with swagger-converter.

  "paths": {
    "/activitypub/user-id/{user-id}":
      "get": {
        "parameters": [
          {
            "name": "user-id",
            ...
          }
        ],
        ...
      }
    },
    "/activitypub/user-id/{user-id}/inbox": {
      "post": {
        "parameters": [
          {
            "name": "user-id",
            ...
          }
        ],
        ...
      }
    },
  ...
  }

Desktop (please complete the following information):

  • OS: Linux Manjaro
  • Python Version: 3.12.2
  • openapi-python-client version: 0.17.3

Additional context

Warnings:

WARNING parsing GET /activitypub/user-id/{user-id} within activitypub. Endpoint will not be generated.
Incorrect path templating for /activitypub/user-id/{user-id} (Path parameters do not match with path)

WARNING parsing POST /activitypub/user-id/{user-id}/inbox within activitypub. Endpoint will not be generated.
Incorrect path templating for /activitypub/user-id/{user-id}/inbox (Path parameters do not match with path)

To diagnose, I added the following print statements to parser/openapi.py:

    def sort_parameters(*, endpoint: "Endpoint") -> Union["Endpoint", ParseError]:
        # ...
        print(endpoint.name, endpoint.path)                                                   # <- new
        print(f'\tparameters_from_path: {parameters_from_path}')                              # <- new
        print(f'\tendpoint {endpoint.name}.path_parameters: {endpoint.path_parameters}')      # <- new

        if parameters_from_path != [param.name for param in endpoint.path_parameters]:
            return ParseError(
                detail=f"Incorrect path templating for {endpoint.path} (Path parameters do not match with path)",
            )
        return endpoint

For comparison, replacing one of the user-id occurrences with userid and generating a client yields this:

('activitypubPerson /activitypub/user-id/{userid}'
 "      parameters_from_path: ['userid']"
 "      endpoint activitypubPerson.path_parameters: [IntProperty(name='userid', "
 "required=True, default=None, python_name='userid', description=None, "
 'example=None)]'
 'activitypubPersonInbox /activitypub/user-id/{user-id}/inbox'
 '      parameters_from_path: []'
 '      endpoint activitypubPersonInbox.path_parameters: '
 "[IntProperty(name='user-id', required=True, default=None, "
 "python_name='user_id', description=None, example=None)]"
 ...
)
github-merge-queue bot pushed a commit that referenced this issue Mar 6, 2024
Fixes #976 and #578, and replaces #978.

@dbanty please choose your preferred approach between this and PR #987.

The original issue is that `openapi-python-client` throws `incorrect
path templating` warnings when the path has a parameter with a hyphen
and consequently fails to generate the endpoints.

---

The first commit ensures that hyphens are recognised as allowed
delimiters in parameter path names. This allows the endpoints to be
generated.

However, this generates lines like these:

```python
def _get_kwargs(
    user_id: int,
) -> Dict[str, Any]:
    _kwargs: Dict[str, Any] = {
        "method": "post",
        "url": "/activitypub/user-id/{user-id}/inbox".format(user-id=user_id,),
    }
    return _kwargs
```

Since Python variable names cannot contain hyphens, the `user-id`
parameter name here will trigger errors (starting with `ruff`).

---

The second commit replaces parameter names with their `python_name` in
`__init__.py` and passes the modified path to
`templates/endpoint_module.py.jinja`.

This fixes the issue and allows endpoints to be generated correctly. 

---

#987 is a different option for the second commit which instead creates a
custom Jinja filter in `utils.py` and so that the parameter names in
`endpoint.path` can be converted to their python names directly in
`templates/endpoint_module.py.jinja`.

Both approaches are equivalent and have been tested with different
parameter names (snake case, camel case, kebab case, mixed).

---------

Co-authored-by: harabat <[email protected]>
Co-authored-by: Dylan Anthony <[email protected]>
Co-authored-by: Dylan Anthony <[email protected]>
dbanty added a commit that referenced this issue Mar 6, 2024
This PR was created by Knope. Merging it will create a new release

### Breaking Changes

#### Update PDM metadata syntax

Metadata generated for PDM will now use the new `distribution = true`
syntax instead of `package-type = "library"`.
New packages generated with `--meta pdm` will require PDM `2.12.0` or
later to build.

### Features

#### Add response content to `UnexpectedStatus` exception

The error message for `UnexpectedStatus` exceptions will now include the
UTF-8 decoded (ignoring errors) body of the response.

PR #989 implements #840. Thanks @harabat!

### Fixes

#### Allow hyphens in path parameters

Before now, path parameters which were invalid Python identifiers were
not allowed, and would fail generation with an
"Incorrect path templating" error. In particular, this meant that path
parameters with hyphens were not allowed.
This has now been fixed!

PR #986 fixed issue #976. Thanks @harabat!

> [!WARNING]
> This change may break custom templates, see [this
diff](https://github.com/openapi-generators/openapi-python-client/pull/986/files#diff-0de8437b26075d8fe8454cf47d8d95d4835c7f827fa87328e03f690412be803e)
> if you have trouble upgrading.

Co-authored-by: GitHub <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant