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

Getter and Setter for hideGenerationTimestamp #7997

Closed
jmini opened this issue Apr 10, 2018 · 3 comments
Closed

Getter and Setter for hideGenerationTimestamp #7997

jmini opened this issue Apr 10, 2018 · 3 comments

Comments

@jmini
Copy link
Contributor

jmini commented Apr 10, 2018

This is inspired by swagger-api/swagger-codegen-generators#55 for swagger v3.


Codegen classes can be used directly (no maven plugin, no cli tool) like this:

JavaClientCodegen config = new io.swagger.codegen.languages.JavaClientCodegen();
config.setJava8Mode(true);
config.setOutputDir(outputDir);

final SwaggerParser swaggerParser = new SwaggerParser();
Swagger swagger = swaggerParser.read(folder + "/" + inputSpecName + ".json", null, true);

final ClientOptInput opts = new ClientOptInput();
opts.setConfig(config);
opts.setSwagger(swagger);
opts.setOpts(new ClientOpts());
new DefaultGenerator().opts(opts).generate();

For some values, it is possible to use a setter, for some values the only way is to use the additionalProperties map.


Currently hideGenerationTimestamp is can only be modified like this:

JavaClientCodegen config = new JavaClientCodegen();
codegen.additionalProperties().put(CodegenConstants.HIDE_GENERATION_TIMESTAMP, false);

This issue is about providing a setter setHideGenerationTimestamp(boolean) in order to be able to do something like this:

JavaClientCodegen config = new JavaClientCodegen();
codegen.setHideGenerationTimestamp(false);

There are already setters in the DefaultCodegen class:

  • setSkipOverwrite(..)
  • setRemoveOperationIdPrefix(..)

The new setter should work in a similar manner.

The value need to be available in the templates (using {{hideGenerationTimestamp}})
The Codegen classes need to conciliate both way (value set with the new setter, value set in the additionalProperties map) and ensure that everything is consistent. This is done in processOpts().


hideGenerationTimestamp is defined multiple times. In the DefaultCodegen class and in some child classes.
This is bad design. “fields declaration hiding another field or variable” is a bad practice in java. Name shadowing can cause subtle errors and should be avoided.

With this pull request, the code should be clean up for hideGenerationTimestamp (otherwise the setters need to be defined at each level where hideGenerationTimestamp is declared).

When this change is made, the code that deals with hideGenerationTimestamp in the child processOpts() methods can be removed. It is handled at DefaultCodegen level.

The default value set in each subclass needs to be kept (initial value and custom initial value).


A getter isHideGenerationTimestamp() should be added in DefaultCodegen.

@jmini
Copy link
Contributor Author

jmini commented Apr 10, 2018

Related issue: #8004 :

It should be possible to set a value with the setter:

JavaClientCodegen config = new io.swagger.codegen.languages.JavaClientCodegen();
config.setApiPackage("com.company.api");
config.setModelPackage("com.company.model");

It should work in a similar way than setters that are already working:

JavaClientCodegen config = new io.swagger.codegen.languages.JavaClientCodegen();
config.setInvokerPackage("com.company.invk");

@jmini
Copy link
Contributor Author

jmini commented Apr 12, 2018

Second PR for this issue: #8009

@jmini
Copy link
Contributor Author

jmini commented Apr 12, 2018

Thank you, I think this issue can now be closed.

@jmini jmini closed this as completed Apr 12, 2018
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

No branches or pull requests

1 participant