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

added currently failing version of issue#1351 test #1416

Merged
merged 1 commit into from
Oct 16, 2016

Conversation

arifogel
Copy link
Contributor

Issue #1351 is not fixed for me. I've added a failing version of the test, with slight modifications from the original test committed for the initial fix. Note that if you change just about any feature of the modified test, the problem goes away. However, I do not have the option in my project to make such modifications, so I really would appreciate if my failing version gets fixed.

@cowtowncoder cowtowncoder merged commit 847fd82 into FasterXML:master Oct 16, 2016
@cowtowncoder
Copy link
Member

Ok, will try to see what gives wrt fail and hopefully fix an edge case.

@cowtowncoder
Copy link
Member

Filed #1417 for follow up; I can reproduce this with latest from master and 2.8(.4)

@cowtowncoder
Copy link
Member

@arifogel Ok, so the problem here is that there is no way to create a default instance of Issue1351NonBean, as there is no 0-argument ("default") constructor. Because of this, it is not possibly to (try to) figure out default values for properties. There is nothing I can do about that.
Earlier this would have thrown an exception, but since allowing global default usage, this has not been done with 2.7 or later.

But it is possible that the fallback handling here is wrong: it would seem like null should be considered default for String; as is, looks like "" is assumed enough, which I don't think makes sense.

So I may be able to fix the specific case here, regardless.

@arifogel
Copy link
Contributor Author

Can this be averted in specific cases by writing a default constructor that
passes "default" values to the jsonconstructor?

On Oct 15, 2016 6:43 PM, "Tatu Saloranta" [email protected] wrote:

@arifogel https://github.com/arifogel Ok, so the problem here is that
there is no way to create a default instance of Issue1351NonBean, as
there is no 0-argument ("default") constructor. Because of this, it is not
possibly to (try to) figure out default values for properties. There is
nothing I can do about that.
Earlier this would have thrown an exception, but since allowing global
default usage, this has not been done with 2.7 or later.

But it is possible that the fallback handling here is wrong: it would seem
like null should be considered default for String; as is, looks like ""
is assumed enough, which I don't think makes sense.

So I may be able to fix the specific case here, regardless.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1416 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AHYSEokb2OlAuLYTjrOaicXJbbUw7s0yks5q0YE1gaJpZM4KXhEJ
.

@cowtowncoder
Copy link
Member

@arifogel yes. There just has to be 0-arg constructor (can have any visibility, even private), which is called to create an instance; and then property values are accessed using normal rules (getter, field).

@cowtowncoder
Copy link
Member

Ok. So what happens here is that the default value for String is "", which is why comparison is made.
It gets bit tricky...

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