Skip to content

Prevent error url parameter #762

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
wants to merge 1 commit into from

Conversation

juanber84
Copy link

Scenario: GET endpoint with url parameter in your openapi.json.

The original code set the base url of request in url named var before that the lib set parameters config, so the value of url param will be wrong.

@juanber84 juanber84 changed the title Prevent error with url paremeter Prevent error with url parameter May 23, 2023
@juanber84 juanber84 changed the title Prevent error with url parameter Prevent error url parameter May 23, 2023
@dbanty
Copy link
Collaborator

dbanty commented May 24, 2023

I'm confused by when this would happen. Can you add a test to end-to-end tests which illustrates the problem (and will ensure it doesn't pop back up)?

@juanber84
Copy link
Author

juanber84 commented May 25, 2023

Hi, of course. I try explain the problem.

For example we have a openapi.json file with the following content.

{
    "openapi": "3.0.2",
    "info": {
        "title": "FastAPI",
        "version": "0.1.0"
    },
    "paths": {
        "/": {
            "get": {
                "summary": "Root",
                "operationId": "root__get",
                "parameters": [
                    {
                        "required": false,
                        "schema": {
                            "title": "Url",
                            "type": "string"
                        },
                        "name": "url",
                        "in": "query"
                    }
                ],
                "responses": {
                    "200": {
                        "description": "Successful Response",
                        "content": {
                            "application/json": {
                                "schema": {}
                            }
                        }
                    },
                    "422": {
                        "description": "Validation Error",
                        "content": {
                            "application/json": {
                                "schema": {
                                    "$ref": "#/components/schemas/HTTPValidationError"
                                }
                            }
                        }
                    }
                }
            }
        }
    },
    "components": {
        "schemas": {
            "HTTPValidationError": {
                "title": "HTTPValidationError",
                "type": "object",
                "properties": {
                    "detail": {
                        "title": "Detail",
                        "type": "array",
                        "items": {
                            "$ref": "#/components/schemas/ValidationError"
                        }
                    }
                }
            },
            "ValidationError": {
                "title": "ValidationError",
                "required": [
                    "loc",
                    "msg",
                    "type"
                ],
                "type": "object",
                "properties": {
                    "loc": {
                        "title": "Location",
                        "type": "array",
                        "items": {
                            "anyOf": [
                                {
                                    "type": "string"
                                },
                                {
                                    "type": "integer"
                                }
                            ]
                        }
                    },
                    "msg": {
                        "title": "Message",
                        "type": "string"
                    },
                    "type": {
                        "title": "Error Type",
                        "type": "string"
                    }
                }
            }
        }
    }
}

You can see that we have a endpoint / which recived one parameter named "url".

Next, we can use your tool to generate the client.

openapi-python-client generate --path ./openapi.json

The tool generate the client us.

fast-api-client
├── README.md
├── fast_api_client
│   ├── __init__.py
│   ├── api
│   │   ├── __init__.py
│   │   └── default
│   │       ├── __init__.py
│   │       └── root_get.py
│   ├── client.py
│   ├── errors.py
│   ├── models
│   │   ├── __init__.py
│   │   ├── http_validation_error.py
│   │   └── validation_error.py
│   ├── py.typed
│   └── types.py
└── pyproject.toml

And finally, the code generated by the tool in root_get.py is.

from http import HTTPStatus
from typing import Any, Dict, Optional, Union, cast

import httpx

from ... import errors
from ...client import Client
from ...models.http_validation_error import HTTPValidationError
from ...types import UNSET, Response, Unset


def _get_kwargs(
    *,
    client: Client,
    url: Union[Unset, None, str] = UNSET,
) -> Dict[str, Any]:
    url = "{}/".format(client.base_url)

    headers: Dict[str, str] = client.get_headers()
    cookies: Dict[str, Any] = client.get_cookies()

    params: Dict[str, Any] = {}
    params["url"] = url

    params = {k: v for k, v in params.items() if v is not UNSET and v is not None}

    return {
        "method": "get",
        "url": url,
        "headers": headers,
        "cookies": cookies,
        "timeout": client.get_timeout(),
        "follow_redirects": client.follow_redirects,
        "params": params,
    }
...
...
...
...

Bug

def _get_kwargs(
    *,
    client: Client,
    url: Union[Unset, None, str] = UNSET, <====================================
) -> Dict[str, Any]:
    url = "{}/".format(client.base_url) <======================================

    headers: Dict[str, str] = client.get_headers()
    cookies: Dict[str, Any] = client.get_cookies()

    params: Dict[str, Any] = {}
    params["url"] = url <======================================================

    params = {k: v for k, v in params.items() if v is not UNSET and v is not None}

The tool is overwriting my url parameter with the value of api endpoint

Solution

If you move the url endpoint var setting after params generation you can fix the problem

@dbanty
Copy link
Collaborator

dbanty commented May 27, 2023

Thank you for that, I get it now! I think this is very similar to #758 actually and I'd like to handle it the same way (treat url as a reserved word) since I think relying on the position in the code could end up with more bugs in the future if someone ends up moving it or adding more param logic later.

So I'm going to close this PR and instead solve both of these problems together. Thank you for reporting!

@dbanty dbanty closed this May 27, 2023
dbanty added a commit that referenced this pull request May 27, 2023
…anks @truenicoco & @juanber84!

* fix: Allow parameters named "client" and "url"

* chore: Cleanup unused import

* chore: Hold mypy's hand

---------

Co-authored-by: Dylan Anthony <[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
Development

Successfully merging this pull request may close these issues.

2 participants