Support java-class stringable property#724
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #724 +/- ##
==========================================
- Coverage 70.74% 70.61% -0.13%
==========================================
Files 44 44
Lines 1839 1865 +26
Branches 317 317
==========================================
+ Hits 1301 1317 +16
- Misses 538 548 +10
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
@RustedBones I'll take a look on Monday, thanks for opening! |
|
@RustedBones the tests make sense, although I don't fully understand all of the details. Maybe you could explain a little more in detail about what your change fixes? Other than that, I don't have a problem just merging. |
| .name("javaStringField").`type`().stringBuilder().prop(GenericData.STRING_PROP, "String").endString().noDefault() | ||
| .name("charSequenceField").`type`().stringBuilder().prop(GenericData.STRING_PROP, "CharSequence").endString().noDefault() | ||
| .name("utf8Field").`type`().stringBuilder().prop(GenericData.STRING_PROP, "CharSequence").endString().noDefault() | ||
| .name("stringableBigDecimalField").`type`().stringBuilder().prop(SpecificData.CLASS_PROP, classOf[java.math.BigDecimal].getName).endString().noDefault() |
There was a problem hiding this comment.
avsc equivalent for
{"type": "string", "java-class": "java.math.BigDecimal"}| .name("charSequenceField").`type`().stringBuilder().prop(GenericData.STRING_PROP, "CharSequence").endString().noDefault() | ||
| .name("utf8Field").`type`().stringBuilder().prop(GenericData.STRING_PROP, "CharSequence").endString().noDefault() | ||
| .name("stringableBigDecimalField").`type`().stringBuilder().prop(SpecificData.CLASS_PROP, classOf[java.math.BigDecimal].getName).endString().noDefault() | ||
| .name("stringableDefaultField").`type`().stringBuilder().prop(SpecificData.CLASS_PROP, classOf[StringableCustom].getName).endString().noDefault() |
There was a problem hiding this comment.
avsc equivalent for
{"type": "string", "java-class": "com.spotify.ratatool.scalacheck.StringableCustom"}Called default because no custom stringableGen passed for this class
| .name("stringableDefaultField").`type`().stringBuilder().prop(SpecificData.CLASS_PROP, classOf[StringableCustom].getName).endString().noDefault() | ||
| .name("mapDefaultKeyField").`type`().map().values().longType().noDefault() | ||
| .name("mapJavaStringKeyField").`type`().map().prop(GenericData.STRING_PROP, "String").values().longType().noDefault() | ||
| .name("mapStringableBigIntegerKeyField").`type`().map().prop(SpecificData.KEY_CLASS_PROP, classOf[java.math.BigInteger].getName).values().longType().noDefault() |
There was a problem hiding this comment.
avsc equivalent for
{"type": "string", "java-class": "java.math.BigInteger"}| val stringableGens = Map[Class[_], Gen[_]]( | ||
| classOf[java.math.BigDecimal] -> Gen.choose(BigDecimal(-1), BigDecimal(1)).map(_.bigDecimal), | ||
| classOf[java.math.BigInteger] -> Gen | ||
| .choose(BigInt(0), BigInt(Long.MaxValue)) | ||
| .map(_.bigInteger) | ||
| ) | ||
| val gen = avroOf(schema, stringableGens) |
There was a problem hiding this comment.
without those custom stringableGens, random strings would be generated to create BigDecimal and BigInteger that most likely creates a parse exception.
Here we give custom generators for those types and bypass the conversion from string.
The StringableCustom type not being part of the map will be constructed with random string
benkonz
left a comment
benkonz
left a comment
There was a problem hiding this comment.
thanks for helping me understand these changes, LGTM!
2 participants
Attempt to support
java-classandjava-key-classas defined in the avro specificationjava-element-classis only used in the reflect API