Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Improvements to online codegen #55

Merged
merged 2 commits into from
May 16, 2018

Conversation

cbornet
Copy link
Member

@cbornet cbornet commented May 15, 2018

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
  • Filed the PR against the correct branch: Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

  • Change openapi-online context path to "/"
  • Fix openAPI generated spec
  • Some minor changes

@jimschubert
Copy link
Member

@cbornet I've tested out this pull request, and I had a question. The previous jetty implementation used reflection to update enum values for all language implementations in the client and server apis, such that these options would remain up to date as generators were added/deleted/renamed. It looks like this reflection isn't done on this branch, so when I query for a scala client for example, I get an error:

query

curl -X GET "http://localhost:8080/api/gen/clients/scala" -H "accept: application/json"

response

{
  "timestamp": "2018-05-16T01:21:10.315Z",
  "status": 400,
  "error": "Bad Request",
  "message": "Unsupported target scala supplied. java.lang.RuntimeException: Can't load config class with name 'scala'\nAvailable:\nada\nada-server\nandroid\napache2\napex\naspnetcore\nbash\nclojure\ncwiki\ncpp-qt5\ncpp-pistache-server\ncpp-restbed-server\ncpp-restsdk\ncpp-tizen\ncsharp\ncsharp-dotnet2\ncsharp-nancyfx\ndart\neiffel\nelixir\nelm\nerlang-client\nerlang-server\nflash\nscala-finch\ngo\ngo-server\ngroovy\nkotlin\nkotlin-server\nhaskell-http-client\nhaskell\njava\njaxrs-cxf-client\njava-inflector\njava-msf4j\njava-pkmst\njava-play-framework\njava-undertow-server\njava-vertx\njaxrs-cxf\njaxrs-cxf-cdi\njaxrs-jersey\njaxrs-resteasy\njaxrs-resteasy-eap\njaxrs-spec\njavascript\njavascript-closure-angular\njmeter\nlua\nnodejs-server\nobjc\nopenapi\nopenapi-yaml\nperl\nphp\nphp-lumen\nphp-slim\nphp-silex\nphp-symfony\nphp-ze-ph\npowershell\npython\npython-flask\nr\nruby\nruby-on-rails\nruby-sinatra\nrust\nrust-server\nscalatra\nscala-akka\nscala-httpclient\nscala-gatling\nscala-lagom-server\nscalaz\nspring\ndynamic-html\nhtml\nhtml2\nswift2-deprecated\nswift3\nswift4\ntypescript-angular\ntypescript-angularjs\ntypescript-aurelia\ntypescript-fetch\ntypescript-inversify\ntypescript-jquery\ntypescript-node\n",
  "path": "/api/gen/clients/scala"
}

If I query for the modified scala client name, scala-httpclient

curl -X GET "http://localhost:8080/api/gen/clients/scala-httpclient" -H "accept: application/json"

I get the expected result:

{
    "sortParamsByRequiredFlag": {
        "opt": "sortParamsByRequiredFlag",
        "description": "Sort method arguments to place required parameters before optional parameters.",
        "type": "boolean",
        "default": "true",
        "enum": null
    },
    "ensureUniqueParams": {
        "opt": "ensureUniqueParams",
        "description": "Whether to ensure parameter names are unique in an operation (rename parameters that are not).",
        "type": "boolean",
        "default": "true",
        "enum": null
    },
    "allowUnicodeIdentifiers": {
        "opt": "allowUnicodeIdentifiers",
        "description": "boolean, toggles whether unicode identifiers are allowed in names or not, default is false",
        "type": "boolean",
        "default": "false",
        "enum": null
    },
    "prependFormOrBodyParameters": {
        "opt": "prependFormOrBodyParameters",
        "description": "Add form or body parameters to the beginning of the parameter list.",
        "type": "boolean",
        "default": "false",
        "enum": null
    },
    "modelPackage": {
        "opt": "modelPackage",
        "description": "package for generated models",
        "type": "string",
        "default": null,
        "enum": null
    },
    "apiPackage": {
        "opt": "apiPackage",
        "description": "package for generated api classes",
        "type": "string",
        "default": null,
        "enum": null
    },
    "sourceFolder": {
        "opt": "sourceFolder",
        "description": "source folder for generated code",
        "type": "string",
        "default": null,
        "enum": null
    },
    "modelPropertyNaming": {
        "opt": "modelPropertyNaming",
        "description": "Naming convention for the property: 'camelCase', 'PascalCase', 'snake_case' and 'original', which keeps the original name",
        "type": "string",
        "default": "camelCase",
        "enum": null
    }
}

Do you have plans to include this reflection-based update in this PR? If not, can you update the enum values for the client and server options as part of this PR and then we can create an issue to do this via reflection.

@cbornet
Copy link
Member Author

cbornet commented May 16, 2018

@jmini Indeed, that would be better. I'll check if it can be done with Springfox 🤞

@cbornet cbornet force-pushed the spring-online-gen branch from 2afe600 to 54394fc Compare May 16, 2018 07:37
WORKDIR /generator

COPY target/openapi-generator-online-3.0.0-SNAPSHOT.jar /generator/openapi-generator-online.jar
COPY target/*.jar /generator/openapi-generator-online.jar
Copy link
Member

Choose a reason for hiding this comment

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

@cbornet I tried *.jar before but I got errors and that's why I used the full filename instead.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's weird. Works on my machine 😉 . And that's also what we do in JHipster.
Did you do mvn clean before packaging ?

Copy link
Member

Choose a reason for hiding this comment

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

We can go with this. If it breaks with errors, we'll just revert the change.

Copy link
Member

Choose a reason for hiding this comment

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

(I also tested *.jar locally before and worked fine for me)

Copy link
Member

Choose a reason for hiding this comment

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

Here is the error reported by the CI (travis):

Removing intermediate container eaef9e5f5b5b
Step 3/6 : COPY target/*.jar /generator/openapi-generator-online.jar
When using COPY with more than one source file, the destination must be a directory and end with a /

Ref: https://travis-ci.org/OpenAPITools/openapi-generator/builds/379603374

I'll submit a PR to fix it.

Copy link
Member Author

Choose a reason for hiding this comment

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

So it seems there are multiple jar in the target directory. That shouldn't be the case, isn't it ?
Can you try COPY target/openapi-generator-online*.jar /generator/openapi-generator-online.jar so there's no need to modify the Dockerfile for each new version ?

Copy link
Member

Choose a reason for hiding this comment

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

Same result:

Removing intermediate container 5b368abf32be
Step 3/6 : COPY target/openapi-generator-online-*.jar /generator/openapi-generator-online.jar
When using COPY with more than one source file, the destination must be a directory and end with a /

Let's go with the hard-coded solution for the time being as it's not something we want to automate with the highest priority.

@wing328 wing328 added this to the 3.0.0 milestone May 16, 2018
@wing328 wing328 merged commit e3814f5 into OpenAPITools:master May 16, 2018
@cbornet
Copy link
Member Author

cbornet commented May 16, 2018

@jmini I have something working for the dynamic allowableValues with sprinfox. PR incoming.

@cbornet cbornet deleted the spring-online-gen branch May 16, 2018 09:08
@wing328 wing328 mentioned this pull request May 16, 2018
4 tasks
aserkes added a commit to aserkes/openapi-generator that referenced this pull request Sep 21, 2022
* Updated and new sample config files; not committing the resulting generated sample code yet

* fix samples compilation errors (OpenAPITools#56)

Signed-off-by: aserkes <andrii.serkes@oracle.com>

Signed-off-by: aserkes <andrii.serkes@oracle.com>

Signed-off-by: aserkes <andrii.serkes@oracle.com>
Co-authored-by: aserkes <andrii.serkes@oracle.com>
nilskuhn pushed a commit to nilskuhn/openapi-generator that referenced this pull request Apr 6, 2023
…norepo

chore(deps): update nrwl monorepo to v10.3.0
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants