pwall567 / json-kotlin-schema-codegen

Code generation for JSON Schema (Draft 07)
MIT License
79 stars 14 forks source link

Configurable language type for number fields? #3

Closed wem closed 3 years ago

wem commented 3 years ago

Hi, first thanks for this project!

As BigDecimal is heavy weight, could it be an idea to make this configurable? So it's possible to use Int or Long.

Regards.

pwall567 commented 3 years ago

Hi Michel, I know what you mean, but the generator has to choose a class that will hold the range of values possible according to the schema. For a numeric field, the generator will choose the narrowest form it can to hold the value, based on what it knows about the constraints on the value.

If the number is an integer ("type": "integer") it will use Long, or if it has further information (minimum and maximum values in the 32-bit range, or enum values that are all 32-bit) constraining the values to 32 bits, it will use Int.

Is it possible for you to add integer type or the relevant validations to your schema to limit the range of values?

I realise that in some cases you may be working with an externally-provided schema so modification is not possible, but I would prefer to use the information in the schema where possible.

Regards, Peter

wem commented 3 years ago

Hi Peter, sorry my bad. I created this issue based on the README - phrase "fields of type number are implemented as BigDecimal (there is insufficient information in the schema in this case to allow the field to be considered an Int or Long)"

Therefore i did expect BigDecimal, but now on my tests Long type was generated, all fine.

Sorry for your wasted time ;)

Regards Michel

pwall567 commented 3 years ago

No worries, thanks Michel. I've made a note to myself to clarify that section of the README.

volkert-fastned commented 1 year ago

@pwall567 Can I maybe reopen this ticket? I understand that BigDecimal is indeed the only "safe" general mapping for "number" types in JSON Schema, but java.math.BigDecimal is Java-specific and therefore available in Kotlin/JVM only, and thus doesn't work in Kotlin Multiplatform projects (or single-platform Kotlin/Native and Kotlin/JS projects for that matter).

I did some searching for a Multiplatform replacement library for java.math.BigDecimal, and I found this one: https://github.com/ionspin/kotlin-multiplatform-bignum

As a temporary workaround in my current project, I wrote a Gradle task that replaces each occurrence of java.math.BigDecimal with com.ionspin.kotlin.bignum.decimal.BigDecimal right after the json-kotlin-gradle finishes generating the source. Of course I also had to had to add this common library dependency in my Gradle project. But this solution does seem to work for me so far.

So it would indeed be nice if the default mapping for number types in JSON Schemas could be overridden, even if just for the Multiplatform use case. Of course with the caveat in the documentation that it may be dangerous when people use this functionality to map these types to less precise types in Kotlin, such as Int, Double, etc.

Unfortunately, Kotlin does not have an "official" equivalent for BigDecimal in its standard library. So unless you'd want to add a library like kotlin-multiplatform-bignum as a dependency to json-kotlin-schema-codegen, it's probably not a good idea to use com.ionspin.kotlin.bignum.decimal.BigDecimal for everyone by default. I think just offering the option to override the default mapping for the number type would be the simplest and most flexible solution here.

Thanks for considering this!