[BUG] '/' prepended to paths that don't start with '/'

[dkliban]

Bug Report Checklist

Description

I am currently trying to switch from swagger-codegen-cli to openapi-generator-cli, but I ran into a problem with the generated Python client code. I do not experience the same behavior when generating client code using swagger-codegen-cli.

/ is prepended to paths that don't start with a /. This is a problem because my OpenAPI schema includes paths that consist of a single path parameter. e.g.: {artifact_href}. In the generated code this is turned into /{artifact_href}. Since the path parameter being passed in looks like /pulp/api/v3/artifacts/1234/, the client makes a request to http://localhost//pulp/api/v3/artifacts/1234/ and the server is unable to service the request. Notice the // in the URL.

This problem is not unique to the Python generator. I am seeing the exact same behavior for Ruby also.

openapi-generator version

The latest docker image of openapi-generator-cli.

OpenAPI declaration file content or url

https://docs.pulpproject.org/en/3.0/nightly/api.json

Command line used for generation

docker run --rm -v ${PWD}:/local openapitools/openapi-generator-cli generate \
    -i /local/api.json \
    -g python \
    -o /local/pulpcore-client \
    -DpackageName=pulpcore.client.pulpcore \
    -DprojectName=pulpcore-client \
    -DpackageVersion=3.0.0rc1 \
    --skip-validate-spec

Steps to reproduce

Generate a Python client with the schema from above. Observe that the client that is generated has

        return self.api_client.call_api(
            '/{artifact_href}', 'GET',

while the path for this operation in the OpenAPI schema was simply {artifact_href}. I would expect the generator to not modify the path value.

Related issues/PRs

Suggest a fix

[auto-labeler]

👍 Thanks for opening this issue!
🏷 I have applied any labels matching special text in your issue.

The team will review the labels and make any necessary changes.

[jmini]

This was introduced with #1034.

Not having a leading / is not conform to the OpenAPI Spec:

A relative path to an individual endpoint. The field name MUST begin with a slash.
source https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.2.md#pathsObject
This is why we decided to add this when missing.

But you are bringing an interesting case...
And for OpenAPI-Generator an interesting question: what should we do with invalid spec? Try to fix it or not...

A question from my side:
Why is your parameter "/pulp/api/v3/artifacts/1234/" and not "pulp/api/v3/artifacts/1234/"?

[jimschubert]

@jmini I don't think we have a choice but to "fix" specs, otherwise generators may individually attempt to apply fixes to create meaningful generated code.

Paths are intended to be appended to the base URL, but it seems like a base url of / is the issue here, while /api would concatenate as expected.

I'd have to look across some generators to offer a suggestion on how we might best approach this. But I think we'll need to fix any issues in generators as they arise.

[dkliban]

@jmini, @jimschubert I don't expect the generator to correct a path specified in the schema. I expect a generator to take exactly what is in the schema and insert it into templates. If the generated code does not make sense, the schema needs to be adjusted by the user of the generator.

We use the full URI of a resource as it's identifier. So our API returns these strings and our users store strings like "/pulp/api/v3/artifacts/1234/" in the database. We chose that format over "pulp/api/v3/artifacts/1234/" because that is what is returned by Django's 'reverse' method[0]. We could definitely change it, but it seems inappropriate to modify our application so that we can use openapi-generator.

I would like to switch from swagger-codegen to openapi-generator, but this is preventing me right now.

[0] https://github.com/django/django/blob/master/django/urls/base.py#L27

[jimschubert]

@dkliban these two comments pretty accurately demonstrate our dilemma regarding fixing or ignoring invalid documents:

If the generated code does not make sense, the schema needs to be adjusted by the user of the generator.

and

We could definitely change it, but it seems inappropriate to modify our application so that we can use openapi-generator.

Even so, some users such as yourself might rely on tooling which doesn't follow the OpenAPI specification appropriately (swagger-codegen).

Take these url, path, and combinations for instance:

url	path	combined	valid input, per spec?
http://localhost/api	/dogs	http://localhost/api/dogs	true
http://localhost/api/	/dogs	http://localhost/api//dogs	true
http://localhost/api/	dogs	http://localhost/api/dogs	false
http://localhost/api	dogs	http://localhost/apidogs	false

My point in my previous comment was that, if swagger-parser doesn't treat the last two options as invalid and also doesn't normalize the spec to provide tooling a "valid" object model according to the specification, then it is up to tooling to normalize these combinations. In all of the examples above, we know what the combined output should be for a given URL. But even in the second example, we have no choice really but to modify what the user provides since it's technically valid for both inputs.

One option I see to help alleviate this problem is to introduce a --strict-spec option for the cli. This may allow for use cases such as yours. I was wondering if you'd be willing to evaluate #2783?

It's also possible to create a custom template, which may also potentially help solve the issue. This past weekend, we've added experimental support for custom templates using Handlebars. Granted, this would require rewriting any affected code in handlebars templates, but the built-in adapter supports conditions (if/eq/neq) as well as string operations (a custom startsWith, and built-ins like substring, cut, capitalize, etc).

Eventually, I'd like to include an immutable input configuration which can be passed to templates such that users who customize templates may have greater control over outputs. I intend this work to be part of project #5

[dkliban]

@jimschubert Thanks for putting together that patch! It looks great.

I was planning to work around this problem by just doing a search and replace on the generated code[0]. However, I would much rather not have to do that.

[0] https://github.com/pulp/pulp-swagger-codegen/pull/1/files#diff-ca18249b58faae4bb31643f0b73d12feR37

[dkliban]

@jimschubert Thanks for adding the extra flag! Just tried it out and it works.

#2783
#2814