-
Notifications
You must be signed in to change notification settings - Fork 37
Setting jsonpath values when jsonpath contains numeric value does set/sync properly client-side but writes proper value server-side. #84
Comments
TO BE CLEAR: This java-client ISSUE was observed, testing local server build, AFTER applying and rebuilding the server json-path.js fix attached (as .txt) in deepstreamIO/deepstream.io#651. |
Tested again with deepstream server 2.2.0 master and java client 2.0.8 master and the result is not sync'd either. The client and server side values are different, NOT sync'd. The resulting values are different than the values after applying the above mentioned server-side patch fix found in deepstreamIO/deepstream.io#651 but the client and server values are still NOT the same on the client-side as on the server-side/redis-side. |
** NOTICE ** Cause of issue identified on the java-client side here was a missing condition check for iterating an array index item which was null and not instantiating an new object to build along the path. This fix is for a specific edge case resolution. Summary: UtilJsonPath:setIterateThrough(...) : the fix is for array index items in path that are null -> add new object
FYI the website link for the Contributor License Agreement is broken, so I couldn't do a PR but below is my deepstream.io-client-java/master patch with my FIXes (.txt -> .java source) for this client=side issue related to these server-side issues #651 and #646. Source and New Tests: Coupled with the Server-Side FIXes attached to the issues #651 and #646 mentioned these together allow for a couple edge case bugfixes and more robust complex jsonpath support for array and (NEW) numeric object member name support. Much of our data is streamed, deeply nested json objs, with numerous objects members having numeric fieldnames. These are automated transforms in another ETL application, not possible to refactor easily.The numeric fieldnames also help significantly as the number of records scales due to memory usage for each fieldname's length for every obj. Smaller fieldnames = smaller objects = smaller data stores = less bandwidth/memory/storage. (AWS == $$$$). We also don't want the added padding of nulls in sparse arrays of slotted values. And, of course, arrays are much harder to sync for. |
Hey, thanks for raising and fixing. Can you raise a Pr? We'll solve cla issue on our side. Thanks! |
@yasserf, How do I raise a PR without a signed agreement? |
|
Yes, @AlexBHarley already did this. :-) |
You'll need to create a fork, we dont add outside collaborators to the org due to github seats. Signing the CLA only means you don't hold the IP to the fix made. Awesome, thanks @alex! |
Dev Note: this was discovered while testing with the java-client the two following and related deepstream.io server (master/v2.2.0) issues:
Setting An element to an array with index = 0 creates an Object deepstream.io#646
Record Set JsonPath with numeric parts that are not array indices returned different results on client vs server side deepstream.io#651
I have submitted FIXes for the above server-side json-path resolver issues. See comments and attachments in issue#651 deepstreamIO/deepstream.io#651.
Description of this java-client issue:
There is still a corner case with Java client (v2.0.8) see below (w redis cache plugin configured, all components running localhost/Mac osx, all latest versions):
e.g.
rec_setPathNum.set("aaa[0].b") does not apply update on client-side BUT does update server-side with no errs logged.
I’ve tested and VERIFIED that the JAVA client corner case I sighted above ( rec_setPathNum.set(“aaa[0].b”, newTixTextObj); ) IS AN ISSUE with the JAVA-client somewhere. I haven’t traced the java-client side issue yet, but I did verify that the back-end/server/redis value is written correctly AND the above JAVA client SNAPSHOT call returns the correct data as well, but the java record that the “.set” is called on above does NOT reflect the correct updated server-side value. The client-side and server-side are NOT SYNC’D anymore in this use case.
The text was updated successfully, but these errors were encountered: