feat: add BigDecimal default value handling #720
Conversation
| private fun checkAndGetLocaleValue(value: StringValue, type: JavaTypeName): String? { | ||
| if (type.toString() == "java.util.Locale") return "Locale.forLanguageTag(\"${value.value}\")" | ||
| return null | ||
| private fun checkAndGetLocaleCodeBlock(value: Value<out Value<*>>, type: JavaTypeName): CodeBlock? { |
There was a problem hiding this comment.
This change seems mostly unrelated. I assume it prevents a crash in the event a non-string is passed in? Would be good to add a test case for.
There was a problem hiding this comment.
Also, assuming it does prevent a crash, perhaps rather than returning null it'd be better to throw some sort of exception with a friendly error message. If someone has set someLocale: Locale = 123 in their schema, it's probably best we let them know rather than just silently not do anything.
There was a problem hiding this comment.
I think you're correct. Will do the changes.
| is StringValue -> CodeBlock.of("new java.math.BigDecimal(\$S)", value.value) | ||
| is IntValue -> CodeBlock.of("new java.math.BigDecimal(\$L)", value.value) | ||
| is FloatValue -> CodeBlock.of("new java.math.BigDecimal(\$L)", value.value) | ||
| else -> null |
There was a problem hiding this comment.
Same thought here around throwing an exception rather than silently defaulting to null.
| is IntValue -> CodeBlock.of("%L", value.value) | ||
| is StringValue -> { | ||
| val localeValueOverride = checkAndGetLocaleCodeBlock(value, type) | ||
| if (localeValueOverride != null) CodeBlock.of("%L", localeValueOverride) |
There was a problem hiding this comment.
I would think this would need to change, as you changed the above method to return a CodeBlock.
There was a problem hiding this comment.
Oh actually looks like you probably can get rid of most of this and just simplify to
is StringValue -> CodeBlock.of("%S", value.value)
There was a problem hiding this comment.
Yup, missed that one. 👍
| private fun checkAndGetLocaleCodeBlock(value: Value<Value<*>>, type: TypeName): CodeBlock? { | ||
| return if (value is StringValue && type.className.canonicalName == "java.util.Locale") { | ||
| CodeBlock.of("%L", "Locale.forLanguageTag(\"${value.value}\")") | ||
| } else null |
There was a problem hiding this comment.
Same comment as the java version around throwing exception.
| is StringValue -> CodeBlock.of("%L", "java.math.BigDecimal(\"${value.value}\")") | ||
| is IntValue -> CodeBlock.of("%L", "java.math.BigDecimal(${value.value})") | ||
| is FloatValue -> CodeBlock.of("%L", "java.math.BigDecimal(${value.value})") | ||
| else -> null |
There was a problem hiding this comment.
Same comment as the java version around throwing exception.
| @@ -4832,4 +4832,36 @@ It takes a title and such. | |||
| } | |||
| assertThat(exception.message).isEqualTo("Property \"firstname\" does not exist in input type \"Person\"") | |||
| } | |||
|
|
|||
| @Test | |||
| fun `The default value for BigDecimal should be overridden`() { | |||
There was a problem hiding this comment.
Thank you for adding tests! Would you mind adding a case that would exercise the null (or exception if you make that change) route?
There was a problem hiding this comment.
Yes, I'll add tests to cover handling of incompatible values 😉
…cale or BigDecimal
Closes #712