Cutted _version_ field

[jaceksocha]

Currently search method returns data from SOLR which is parsed by JSON.parse(). Result of this parsing is truncated "version" field, because "version" occurs in solr as long type, which is parsing to int.
The solution of this bug may be an alternative parser to JSON.parse() (for example: https://npmjs.org/package/json-bigint)
or switch to return raw data (without parsing).

Is there any possibility to patch this issue?

[lbdremy]

Hi @githubjs,

Could you provide me the JSON response from Solr that JSON.parse is not able to parse.

Thanks

[marc-portier]

I just ran into this myself, and reported in solr land as https://issues.apache.org/jira/browse/SOLR-6364 - and was just coming over here to suggest what @githubjs has already done in this issue :)

You can see this at work by adding the option { "versions" : "true" } during a solrclient.add()
The response will then hold a section "adds" which is an array holding

• [0] the id of the created document and
• [1] the assigned version --> current json parsing will always round that number to a multiple of 100 (ie loosing the last two digits of precision)

(almost funny: the solr4 web-ui uses javascript in the browser and suffers from the same issue, you really need curl cli commands - or anything not passing javascript Number limitations to see the actual not rounded numeric value)

The point is that large numbers (e.g. in version field of solr4) are "acceptable" for the json spec, but are truncated/rounded in javascript (the issue mentioned holds all the references)

Replacing the classic JSON.parse and stringify with the one from the mentioned json-bigint package should do the trick.

I'm volunteering to squeeze this into the v0.3.x since I need this anyway - my other option is to switch to XML messaging towards SOLR which is way less elegant IMHO

Even if the mentioned solr-issue (for now) suggests some future where this is fixed through a " "json.long" parameter that can force string responses, it is beneficial for our client library to be able to work without it and thus support deployments with existing SOLR 4.x out there.

I'm not sure yet what the json-bigint effect will be on numbers that are not too big, and if there is any danger in breaking our own api/contract to users of this library. If that is the case I suggest we introduce some init-creation-time option indicating to deliberately use this json-big-int.

Any other thoughts or suggestions?

[marc-portier]

hm, this is a bumpy road:

introducing the json-bigint I run into this: sidorares/json-bigint@4fd9752#commitcomment-7370451

this makes the ping-test fail because for no apparent reason the admin/ping response holds the key 'echoParams' twice: see also https://issues.apache.org/jira/browse/SOLR-6369

I'm waiting some to see how json-bigint and solr communities are going to respond.
Without their help the only (fast) way out I see is to force using classic json-parse in the ping context

While waiting for external reaction I'm adding some tests to really force this problem when not parsing the version correctly

[marc-portier]

Quicky: adding &omitHeader=true in ping context avoids the responseHeader section holding the duplicate keys... -- will use this to get past tests for now.

[marc-portier]

Some additional notes:

1. just realized my branch for this fix https://github.com/marc-portier/solr-node-client/tree/v0.3.x-patch71 also holds the fix for Support the indexing of unstructured (extracted) content from files (in solr known content-types) #90 - @lbdremy: let me know if you want those in separate pull-reqs versus the main v0.3.x
2. Just received a reaction from solr land: refering to http://wiki.apache.org/solr/SolJSON#JSON_specific_parameters - seems they have a json.nl=arrarr option to serialize json as an array-of-arrays. That avoids duplicate-keys, something which can also pop-up at other places in json-replies apparently (not only in ping-response-header). Unsure if we want to go down that path? Would surely change the response-layout our clients are producing, so I guess we should not do that by default, but only by specifying an option do to so during client-instantiation.

In the follow up comments it also states we could avoid the duplicate-key in the ping response-header by changing the config (duplication seems to come from defaults mentioned in our test/materials/solrconfig.xml)

1086     <lst name="defaults">
1087       <str name="echoParams">all</str>
1088     </lst>

Removing that is another quick out - but could lead to more unsatisfied users of our client not understanding why the client.ping() doesn't work versus their fairly standard solrconfig.xml

My current hope remains that json-bigint will want to make their parse() more lenient so it can (better) function as a drop-in replacement for classic JSON.parse

Basically I'm (currently) fine with this patch as it is. I see some alternative routes to proceed:

• accept as is and merge into v0.3.x - give or take some formal things I've maybe overlooked
• possibly additionally follow-up with some reminder-issues to (somewhere in the future) consider initialisation options to allow configuring behavioral settings like json-parser to use, arrarr json options to use and whatnot more of that?
• possibly accompanied with some hope on a reasonably quick fix from the json-bigint we can include
• possibly combined with the attitude/agreement not to release v0.3.x until some/most/all of those new related issues would be solved too

OR

• not accept until all is resolved and all dust has landed, but that will require some more upfront discussion and planning: IMHO that might not be the fastest route to pragmatically resolving issues as we go allong (just my 2c)

[lbdremy]

Wow ok that's a lot of stuff happening.

So, I think the right approach is to use json-bigint module as a drop-in replacement for JSON.parse like you said, the parse method of the json-bigint module should behave like the standard JSON lib concerning duplicate keys. (i.e the last value is used for the duplicate key)

We should "advertise" this problem regarding duplicate keys where we know it is relevant and suggest to the user to use the option you mentionned in the query json.nl=arrarr,in case they are interested by the two or more values.

[marc-portier]

Yep.
I have submitted an issue and fix in the json-bigint project - no idea when it is going to land, hoping on "soon" - See sidorares/json-bigint#6

As suggested I would like us to advance with the current state of things and put down some new issues to keep watching out for the cleanup/outcome in other regions later on. Those issues could be about:

• adding documentation as you suggest about json.nl=arrarr and stuff.
• upgrading to new json-bigint when it is there + revert the omitHeader trick on ping and tests (rather then include our corrected fork of json-bigint)
• ... and/or any other follow-up-issues you think off

Am already running the fixed libs in my project, to great satisfaction :)
Anyway, I'm eager to follow up on all sides getting it right eventually so I'm not off on a solitary fork of myself - so you easily assign some of those tasks to me.

[lbdremy]

Great, we could use your fork of json-bigint until your patch is landed into the original project.

I think on Saturday I'm gonna write some more tests and publish 0.3.0 with what is there (there is already plenty of nice stuff now), new features and bug fixes will be published as patch version on 0.3, like that you and other users can start using 0.3 from npm repo.

[sidorares]

@lbdremy published as json-bigint@v0.1.0

[lbdremy]

thanks @sidorares