Skip to content
This repository has been archived by the owner on Jun 20, 2023. It is now read-only.

@JestId annotated variable not set if id field is not in the source data #142

Closed
jsaimani opened this issue Aug 22, 2014 · 9 comments
Closed
Labels

Comments

@jsaimani
Copy link

For a POJO with one of the fields which annotated with @JestId, the getSourceAsObject throws a NullPointerException.

Unhandled exception occurred while converting source to the object . While saving the document the field annotated with @JestId is not set.

Looks related to ES_METADATA_ID which is not being set on the source.

@tomsen-san
Copy link
Contributor

Found the same problem. Most likely caused by the 'special' treatment of singular objects as opposed to list of objects in JestResult.extractSource().
The code starting line 144 should do the same copy of the '_id' as the code from line 152.

@kramer
Copy link
Member

kramer commented Jan 14, 2015

Reproduction steps please.
@tomsen-san In which case exactly do you end up on line 144 with a document that has both source and _id (in the JSON)? If I'm not mistaken Search response always has the array and Get response is handled on line 160 instead.

@tomsen-san
Copy link
Contributor

Sorry, currently very short on time and my environment is gone.
I did an index of a doc and retrieval with a Get by id.
Will reproduce steps Next week and report back.
Thomas
Am 14.01.2015 22:45 schrieb Cihat Keser [email protected]:Reproduction steps please.
@tomsen-san In which case exactly do you end up in line 144 with document that has both source and _id (in the JSON)? If I'm not mistaken Search response always has the array and Get response is handled on line 160 instead.

—Reply to this email directly or view it on GitHub.

@tomsen-san
Copy link
Contributor

@kramer Ok, debugged the setup again and what can I say - You are absolutely and completely correct. The Get-response is handled on line 160 and there the _id is not added to the source object. How should we proceed? I can do another fix + pull request.
I also looked at the tests for JestResult and it seems that the Get case is tested in method extractGetResourceWithLongId(). The Comment-class used in the test uses the annotation JestId - but in the test the annotated field is explicitly set in the _source json (line 57). I suspect that this is wrong since the code should supply the value for the field?

kramer pushed a commit that referenced this issue Jan 20, 2015
@kramer
Copy link
Member

kramer commented Jan 20, 2015

Just to clear things: Check out the test case getAsClass I just added, this (I think) is the intended use case: a field in the source is selected to be used as id so it is present on both metadata and source. Then there is your case where id exists solely in the metadata, the bug exists for this case. The test case you mentioned JestResultTest.extractGetResourceWithLongId is also testing the first/intended case and that's the reason id is set on both metadata and source there.

@tomsen-san As a fix we need to duplicate line 154 right after line 160 (using variable jsonObject instead of currentObj). If you could spare the time for the fix it'd be great (you can also duplicate and modify the test case extractGetResourceWithLongId to test this). Regardless thanks a lot for your feedback.

@kramer kramer changed the title @JestId issue @JestId annotated variable not set if id field is not in the source data Jan 20, 2015
@kramer kramer added the bug label Jan 20, 2015
@tomsen-san
Copy link
Contributor

I updated my fork, did the changes and did a pull request. The build fails though :-( Couldn't figure out from the log what is wrong. @kramer Maybe you could have a look and suggest fixes?

@kramer
Copy link
Member

kramer commented Jan 22, 2015

@tomsen-san It's ok there is a sporadically failing test case that doesn't like Travis' build environment and it passes after a few tries, need to take a look into that sometime... Will merge your PR soon.

@tomsen-san
Copy link
Contributor

:-)

kramer pushed a commit that referenced this issue Jan 22, 2015
@kramer
Copy link
Member

kramer commented Jan 22, 2015

Closing as fixed by #174

@kramer kramer closed this as completed Jan 22, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants