Skip to content

Fixed #2215 - Support BigInteger and BigDecimal in StdValueInstantiator #2793

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

Merged
merged 1 commit into from
Jul 23, 2020

Conversation

upsidedownsmile
Copy link
Contributor

@upsidedownsmile upsidedownsmile commented Jul 11, 2020

(for #2215)

  • Add BigInteger creator to StdValueInstantiator#createFromInt, StdValueInstantiator#createFromLong StdValueInstantiator#createFromDouble for the case that number type is INT, LONG and DOUBLE respectively.
  • Created StdValueInstantiator#createFromBigInteger and StdValueInstantiator#createFromBigDecimal and auxiliary methods(canCreateFrom..., configureFrom...) for the remaining cases, when number type is BIG_INTEGER and BIG_DECIMAL.
  • Get the single argument creator inside BasicDeserealizerFactory#_handleSingleArgumentCreator and add C_BIG_INTEGER and C_BIG_DECIMAL types to CreatorCollector to represent each one.

@cowtowncoder
Copy link
Member

Sounds good -- thank you very much for contributing this!

I need to re-read it once to see if I missed anything, but one thing I do need before merging is CLA from:

https://github.com/FasterXML/jackson/blob/master/MANUAL.md

and the common way is to print it, fill & sign, scan/photo, email to info at fasterxml dot com.
This only needs to be done once, before the first contribution.

Looking forward to merging this PR, thank you again!

@upsidedownsmile
Copy link
Contributor Author

I'm glad to hear that.
I just sent the CLA to info@fasterxml.com.

@cowtowncoder
Copy link
Member

CLA received, will do one last review, merge. Thank you very much for contributing this!

@cowtowncoder cowtowncoder merged commit 942aa25 into FasterXML:2.12 Jul 23, 2020
@Override
public Object createFromBigInteger(DeserializationContext ctxt, BigInteger value) throws IOException
{
if (_fromBigDecimalCreator != null) {
Copy link
Member

Choose a reason for hiding this comment

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

... and here we have a bug.

Which is not caught by test cases since numbers used do not extend beyond value range of long (for BigInteger), or, for BigDecimal as that is never really reported by JSON parser. Would be exposed by Smile codec, maybe some other binary codecs.
But I'll try fixing BigInteger case at least.

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