#603: BugFix : Handle root path '.' correctly in JSON.OBJLEN command

[iamskp11]

Fixes #603 . Adds differentiation between Legacy and JSON Path syntax.

Basically, every path that doesn't starts with "$" is Legacy Path syntax. And, it resolves to first path in JSON, irrespective of multiple matching paths. Have handled that accordingly in the OBJLEN flow. Added tests to validate it.

Above are snippets from Redis JSONPath Documentation.

Below is the response similarity between DiceDB(left) and Redis on same commands.

[iamskp11]

Hey @JyotinderSingh @lucifercr07 , request you to review this.
Idea is to differentiate between Legacy syntax and JSONPath syntax, and return array response only in case of JSONPath Syntax.
Thanks

[surya0180]

Hi @iamskp11 . I would love to know how you got this in your output in DiceCLI.
(integer) 2.

Currently in DiceCLI, when we execute JSON.OBJLEN, we are getting something like this
1) "3"

So I see that you have made some changes in the code which corrected the output. I want to know how you got it worked. Thanks in advance

[iamskp11]

Hi @iamskp11 . I would love to know how you got this in your output in DiceCLI. (integer) 2.

Currently in DiceCLI, when we execute JSON.OBJLEN, we are getting something like this 1) "3"

So I see that you have made some changes in the code which corrected the output. I want to know how you got it worked. Thanks in advance

Hey @surya0180 , if we get something like 1) "3", it means we are printing an array to console. In this case, it is an array of integer of length 1, with first value being 3. This piece of code would do that.

objectLen := make([]interface{}, 0, 1)
objectLen = append(objectLen, 3)
return clientio.Encode(objectLen, false)

(integer) 2 means we need to return a single integer to print on console. That can be achieved with this line of code.
return clientio.Encode(2, false)
Hope this clarifies.

[surya0180]

@iamskp11. I have tried doing clientio.encode(1, false), but unlike other commands, most of the JSON commands are printing strings instead of integers. I even tried hardcoding the numbers instead of app logic, even then its still printing a string instead of (integer) 5.

What might be this issue?

[iamskp11]

@iamskp11. I have tried doing clientio.encode(1, false), but unlike other commands, most of the JSON commands are printing strings instead of integers. I even tried hardcoding the numbers instead of app logic, even then its still printing a string instead of (integer) 5.

What might be this issue?

@surya0180 Not sure why this happens. Saw your inconsistency reporting on JSON.ARRLEN, and I verified it myself. I can see correct integer results for Dice-CLI.

[surya0180]

What!!!!!!!! Let me try to update my codebase and try again

[surya0180]

@iamskp11 , BTW can you tell me in which environment you are running your app?

[iamskp11]

@iamskp11 , BTW can you tell me in which environment you are running your app?

Go -> 1.23.1
Mac -> Intel

[surya0180]

@iamskp11 I updated my repo and still seeing the same output. But this is not the case in redis-cli. I can see the right output in redis unlike dice-cli. Are you sure you haven't changed anything which might be the reason you are seeing the right output? Just askng because even one of my friend is also seeing the string instead of integer

[iamskp11]

@iamskp11 I updated my repo and still seeing the same output. But this is not the case in redis-cli. I can see the right output in redis unlike dice-cli. Are you sure you haven't changed anything which might be the reason you are seeing the right output? Just askng because even one of my friend is also seeing the string instead of integer

Yes, haven't changed/done anything different.
Can you do these once?

1. Run tests on some JSON command to check if all pass -> TEST_FUNC=TestJsonObjLen make test-one , TEST_FUNC=TestEval make unittest-one. They shouldn't in your case, since output check is set to int64 strictly.

2. Try to set a breakpoint at internal/clientio/resp.go:Encode to check what's the value datatype is returned for numbers.

[surya0180]

@iamskp11 Will do these

[surya0180]

Hi @iamskp11 it looks like an issue in how the terminal is printing the buffer array. The test cases are passing successfully for JSON commands. But this is still an issue because these commands are working fine in redis-cli.

Thanks for the help. I will take it from here 👍

[JyotinderSingh]

Apologies for the late review. But it looks like the changes in this PR are out of date now given the recent changes in the codebase. Could you please update it?
We have migrated the OBJLEN implementation to the resp server, therefore the tests will also need to be migrated accordingly. I was taking a look at the modified parse method but it looks like it is no longer being used anywhere.

The tests look good overall, we can merge this once this comment is addressed.

[iamskp11]

Apologies for the late review. But it looks like the changes in this PR are out of date now given the recent changes in the codebase. Could you please update it? We have migrated the OBJLEN implementation to the resp server, therefore the tests will also need to be migrated accordingly. I was taking a look at the modified parse method but it looks like it is no longer being used anywhere.

The tests look good overall, we can merge this once this comment is addressed.

@JyotinderSingh , will be adding commits soon.

[iamskp11]

@JyotinderSingh the changes are done, in line with migrated code, and also all tests are added.
It's ready for review now.

[JyotinderSingh]

Thanks for this fix, the changes look good and well tested. LGTM.