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

[JAXRS] use contextPath variable for RestApplication templates #825

Closed
wants to merge 16 commits into from
Closed

[JAXRS] use contextPath variable for RestApplication templates #825

wants to merge 16 commits into from

Conversation

michbeck100
Copy link
Contributor

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: master, 3.3.x, 4.0.x. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

The mustache templates for the JAXRS application define / in the @ApplicationPath annotation. This PR replaces this with {{contextPath}}, which contains the path of the server url. If no additional path is given and the server url contains no trailing / the value is an empty string. This will be replaced again with / by JAXRS.

Michael Kotten and others added 4 commits August 16, 2018 08:31
…k in order to make name clashing less probable. (#731)
* add option to generate all models

* update doc

* rename option to skipFormModel

* update petstore samples

* Update doc

* Update samples
@wing328
Copy link
Member

wing328 commented Aug 17, 2018

Thanks for the PR but your commit (as shown in the Commits tab) is not linked to your Github account, which means this PR won't count as your contribution in https://github.com/OpenAPITools/openapi-generator/graphs/contributors.

Let me know if you need help fixing it.

Ref: https://github.com/OpenAPITools/openapi-generator/wiki/FAQ#how-can-i-update-commits-that-are-not-linked-to-my-github-account

@michbeck100
Copy link
Contributor Author

@wing328 I've added my work email to my account, so it should be ok now.

viclovsky and others added 3 commits August 17, 2018 18:15
… improvement (#831)

Added "@Api" swagger annotation, "nickname" for ApiOperation. 
Fixed javadock for rest-assured client.
* Added server variable support

* Replaced tabs with 4 spaces
@@ -3,7 +3,7 @@ package {{invokerPackage}};
import javax.ws.rs.ApplicationPath;
import javax.ws.rs.core.Application;

@ApplicationPath("/")
@ApplicationPath("{{contextPath}}")
Copy link
Member

@wing328 wing328 Aug 19, 2018

Choose a reason for hiding this comment

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

@michbeck100 I would suggest using {{{contextPath}}} instead to avoid special characters (e.g. @) being escaped.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think we need a new variable contentPath as just some characters are escaped which make no sense in an url anyway (see here)

Copy link
Member

Choose a reason for hiding this comment

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

I mean {{{contextPath}}}. Sorry for the typo. (fixed above)

jimschubert and others added 9 commits August 19, 2018 16:17
A user asked about how one would approach generating code from multiple
specifications. Adding this information to the README, as it seems
others would find the information helpful. Also explaining how to use
tasks from this plugin as this may not be commonly known or intuitive
beahvior.
It makes sense that error messages should be written to STDERR and
all others should be written to STDOUT (as shown in #207). However, it
would be convenient to parse the debugging output when the relevant
flags are set.

This change will disable logging to STDOUT and redirect all log messages
to STDERR when any of the debug flags are set. (Resolves #473)
Adds new --log-to-stderr as well as a missed option for
--skip-validate-spec.
* [PHP] Remove PHP related rules from root gitignore

After conversation with @ackintosh we've agreed that every PHP codegen
should create it's own `.gitignore`. Client libraries(SDKs) should ignore
`composer.lock` file while server stubs better do opposite.

* [PHP] Set .gitignore as default supporting file

* [PHP] Add default gitignore to Client SDK

* [PHP] Add default gitignore to Laravel

* [PHP] Add default gitignore to Lumen

* [PHP] Add default gitignore to Silex

Seems like issue #663 and pull request #681 missed this security script,
I've changed output path in bin/security/silex-petstore-server.sh.

* [PHP] Update Slim

* [PHP] Add default gitignore to Symfony

I've copied few old rules from root gitignore to local one. These rules
should be reviewed by original SymfonyCodegen authors.

* [PHP] Add default gitignore to Zend

* [PHP] Refresh Openapi3 client samples

* [PHP] Add refs to .gitignore templates collection
@wing328
Copy link
Member

wing328 commented Aug 20, 2018

Seems like this PR has been incorrectly rebased and therefore it contains commits from other authors. Do you mind submitting a new one by cherry-picking the commits you created?

@michbeck100
Copy link
Contributor Author

Sorry for that, #850 contains the changes now.

@michbeck100 michbeck100 deleted the jaxrs_context_path branch August 20, 2018 06:03
@jmini
Copy link
Member

jmini commented Aug 20, 2018

NOTE: You could also have forced pushed your local branch containing the correct commits (context_path) to your remote jaxrs_context_path attached to this PR
This would have cleaned up this PR...

But closing this one and opening a new one is also possible. We will continue the review in #850.

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.

10 participants