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

Add 'javaOptional' extension rule to allow individual fields to use Java Optional on getter #913

Conversation

mbramwell1
Copy link

Change the Use Optionals for Getters functionality to allow for specifying to use optionals on a field level aswell as for the whole schema. Not a breaking change.

…fying to use optionals on a field level aswell as for the whole schema.
@mbramwell1
Copy link
Author

At the moment you can only specify using optionals on a global level, which is fine I guess but does lead to extra code where it sometimes isnt needed. This cange allows for both use on a global level, or specifying to use optional on a field level by providing "java_optional": true

Copy link
Owner

@joelittlejohn joelittlejohn left a comment

Choose a reason for hiding this comment

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

Great. Thank you! I've added some review comments.

@@ -129,6 +129,24 @@ private boolean isRequired(String nodeName, JsonNode node, Schema schema) {
return false;
}

private boolean usesOptional(String nodeName, JsonNode node, Schema schema) {
if (node.has("java_optional")) {
Copy link
Owner

Choose a reason for hiding this comment

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

Could you rename these to javaOptional, as this is the naming convention for our extension properties.

return requiredNode.asBoolean();
}

JsonNode requiredArray = schema.getContent().get("java_optional");
Copy link
Owner

Choose a reason for hiding this comment

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

I think you mean javaOptionalArray here? requiredArray is a name which I assume was taken from another method.

@@ -77,7 +77,7 @@ public JDefinedClass apply(String nodeName, JsonNode node, JDefinedClass jclass,
ruleFactory.getAnnotator().propertyField(field, jclass, nodeName, node);

if (isIncludeGetters) {
JMethod getter = addGetter(jclass, field, nodeName, node, isRequired(nodeName, node, schema));
JMethod getter = addGetter(jclass, field, nodeName, node, isRequired(nodeName, node, schema), usesOptional(nodeName, node, schema));
Copy link
Owner

Choose a reason for hiding this comment

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

Could you name usesOptional to useOptional throughout? I think this scans better.

@mbramwell1
Copy link
Author

No probs. Review comments addressed.

@joelittlejohn joelittlejohn merged commit 2424bcd into joelittlejohn:master Sep 12, 2018
@joelittlejohn joelittlejohn added this to the 1.0.0-beta1 milestone Sep 12, 2018
@mbramwell1 mbramwell1 deleted the feature/improve-optionals-for-getters branch September 13, 2018 07:28
@joelittlejohn joelittlejohn changed the title Improve Optionals for Getters code to allow for specifying on a field level Add 'javaOptional' extension rule to allow individual fields to use Java Optional on getter Sep 21, 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

Successfully merging this pull request may close these issues.

2 participants