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

NullPointerException fix #61

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

schurik
Copy link

@schurik schurik commented Nov 16, 2016

in com.fasterxml.jackson.databind.ext.CoreXMLSerializers the visitor is passed with a null as JavaType. Because of this a NPE could be thrown in TSJsonFormatVisotorWrapper.expectStringFormat(null)

@atsu85
Copy link
Collaborator

atsu85 commented Nov 18, 2016

@schurik, both Your commits contain lots of formatting changes that every other developer might want to reformat according to their preferences. Could undo the formatting changes?

I'm also wondering in which situation that NPE reveals itslef. There are lots of tests for jackson module, probably I've missed smth.

@schurik
Copy link
Author

schurik commented Nov 18, 2016

@atsu85 I will undo the formatting changes.
I am using XMLGregorianCalender in my model. Inside the CoreXMLSerialie.acceptJsonFormatVisitor method the visitor and null is passed to the delegate. And in case TSJsonFormatVisitorWrapper.expectStringFormat() the type is null. Here the NPE occurs

@atsu85
Copy link
Collaborator

atsu85 commented Nov 21, 2016

@schurik, thanks for undoing the formatting changes.

I tried to quickly reproduce the issue this PR should address, but i probably tried smth different, as everything seemed to work well, when I added

public XMLGregorianCalendar _XMLGregorianCalendar;

to DefinitionGeneratorTest.TestClass, the output included the field with number type that seems to be expected:


export interface TestClass {
    _XMLGregorianCalendar: number;
   // other fields and methods
}

What did You do differently? Perhaps You could even create/update the test for this issue Yourself?

@schurik
Copy link
Author

schurik commented Nov 21, 2016

In my case my model needs to be serialized as JSON and/or XML. My property has an addition @JsonFormat anntotation.

This is my setup:
@JsonFormat(shape = JsonFormat.Shape.STRING, pattern = "yyyy-MM-dd'T'HH:mm:ss.sss'Z'", timezone = "GMT") public XMLGregorianCalendar _someDate;

and the output is:
export interface TestClass { _XMLGregorianCalendar: string; }

and while serializing _someDate I get NPE without the null check

@atsu85
Copy link
Collaborator

atsu85 commented Nov 21, 2016

@schurik, thanks, I'll add the test myself.
Could You also squash the commits into single commit?

atsu85 pushed a commit to atsu85/java2typescript that referenced this pull request Nov 21, 2016
atsu85 pushed a commit to atsu85/java2typescript that referenced this pull request Nov 21, 2016
atsu85 pushed a commit to atsu85/java2typescript that referenced this pull request Nov 21, 2016
@atsu85
Copy link
Collaborator

atsu85 commented Feb 18, 2017

Hi @schurik, I had pretty-much forgotten this after failing to reproduce the NPE You reported and tried to fix with this PR. Please see my comments from the commit that tries to create the test for reproducing the NPE.

Please help me to figure out how to reproduce this issue, so the test could be updated and i could merge the fix.

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