Skip to content

Primitive data types should not be optionals #250

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
danielkaneider opened this issue Aug 2, 2018 · 5 comments
Closed

Primitive data types should not be optionals #250

danielkaneider opened this issue Aug 2, 2018 · 5 comments

Comments

@danielkaneider
Copy link
Contributor

Hi,
we're currently using OptionalProperties.all which isn't ideal for all cases. To be more specific:

  • primitives such as int getNumber() should not be optional
  • all other objects eg Integer getNumber() should be by default optional
  • eventually a NonNull annotation should be used to mark some returned Object as non-optional

If you want I can make a PR for that. I'd add an additional annotation OptionalProperties.default, which could easily cover the first two cases. For the last case we could either use the optionalAnnotations setting for the non-optional annotations, or create a new one nonOptionalAnnotations.

@vojtechhabarta
Copy link
Owner

Hi,

you can also use OptionalProperties.useLibraryDefinition which determines properties "optionality" from configured library (I guess you are using Jackson2). By default Jackson2 considers properties as optional unless they are annotated with @JsonProperty(required = true).

I think this is close to your needs. It satisfies point (2) - properties are optional by default, for point (3) you can use @JsonProperty annotation, for point (1) you also need to use @JsonProperty annotation in this case.

So only point (1) doesn't work automatically as you wanted but on the other hand it is consistent with Jackson2 library which also considers properties of primitive types as optional by default.

#183 contains more information about this parameter.

What do you think?

@danielkaneider
Copy link
Contributor Author

You're mostly right. Jackson2 is a good starting point. However Jackson(2) can be configured globally, while the typescript-generator only considers the annotations of the current class. Jackson has a Include annotation which can be set among others to NON_NULL or NON_EMPTY. In the first case a int type would be non-nullable, while in the second case it is not. Sure, you could set the annotations on every primitive method, but generally you don't do that. Therefore it might be good to have some option in the generator for primitives.

@vojtechhabarta
Copy link
Owner

I didn't think about global configuration. That's true that currently Jackson2Parser only allows to add Jackson modules and no other global configuration.

To achieve this functionality (primitive types as required property) I would use some existing mechanism in Jackson rather than adding new option. I tried to use @JsonInclude annotation but without success. Jackson2Parser uses BeanPropertyWriter.isRequired() to determine if property is optional, and it seems that this is not affected by @JsonInclude annotation, this annotation only affects how property is serialized.

@danielkaneider
Copy link
Contributor Author

Nevermind, I found a working solution. Using a extension using BeforeEnums I could do a workaround for those primitives. During implementing that I found, that it would be nice to have an extension point before the processModel method is called (TransformationPhase.BeforeModels) I.e. allowing an extension to make modifications to the model, before they are converted to the internal TS model. Besides that, feel free to close this issue.

@vojtechhabarta
Copy link
Owner

Your PR is now merged and released, closing the issue.

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

2 participants