#502 Added support for JSON.STRLEN

[apoorvyadav1111]

PR addresses #502

The changes that are included in the PR.

Added support for JSON.STRLEN. added unit tests and integration tests

All tests passed.

[apoorvyadav1111]

Hi @JyotinderSingh, PR is ready for review. Please share your feedback.

[jujiale]

thanks for your pr. I have a tiny comment. in redis. if the rootpath is not provided, the return msg should like below:
`

127.0.0.1:6379> JSON.SET doc $ '{"a":"foo", "nested": {"a": "hello"}, "nested2": {"a": 31}}'
OK
127.0.0.1:6379> JSON.STRLEN doc $
1) (nil)
127.0.0.1:6379> JSON.STRLEN doc
(error) WRONGTYPE wrong type of path value - expected string but found object

`

so I think there is difference between if the path is provided

[apoorvyadav1111]

Hi @jujiale , The parser we use returns different results from what is expected for paths not starting with $. I have been trying to get the same response as redis for different cases but found out that we need to change the parser if we have to be consistent. I checked the JSON.ARRLEN for the same behaviour as follows:

in dice:

in redis:

In dice, the result is an array hence we return array of results. In redis, if there is only one result, the logic only and only then checks for type of the path and throws the WRONGTYPE error. If the type error happens during an array it returns a nil.

Hey @JyotinderSingh, Would like to get your feedback too on this.

[apoorvyadav1111]

Hi, I have tried to keep the behavior in sync with arrlen and pushed new changes with new tests

[jujiale]

yes. In redis. different path have different returns. I think we should consider it if use the same logic for return.

[apoorvyadav1111]

For now, I have handled the case just like arrlen.

[JyotinderSingh]

Thanks for adding this command @apoorvyadav1111. Please address @jujiale's comments.

[jujiale]

a tiny question. in this logic. if path == defaultRootPath . should the jsonData not be a string? because we have made judge in AssertTypeAndEncoding

[apoorvyadav1111]

In AssertTypeAndEncoding, we get no error if the root value is json, which can be {a:1} or '"hello"'. If its the first case, then the defaultpath must return nil, but if its the latter it should return the length. Hence this is the reason I am doing a check here

[jujiale]

in which scenario. the rootpath value could be {a:1} or '"hello"
`

  127.0.0.1:6379> JSON.SET user3 $ '{a:1}'
  (error) Existing key has wrong Redis type
  127.0.0.1:6379> SET user3 '{a:1}'
  OK
  127.0.0.1:6379> JSON.STRLEN user3
  (error) Existing key has wrong Redis type
  127.0.0.1:6379> JSON.STRLEN user3 $
  (error) Existing key has wrong Redis type
  127.0.0.1:6379> JSON.SET user3 $ "hello"
  (error) Existing key has wrong Redis type
  127.0.0.1:6379> set user3 "hello"
  OK
  127.0.0.1:6379> JSON.STRLEN user3
  (error) Existing key has wrong Redis type
  127.0.0.1:6379> JSON.STRLEN user3 $
  (error) Existing key has wrong Redis type

`

[apoorvyadav1111]

Hi, please check these scenarios. My bad, I did not format the cases correctly in the comment.

[jujiale]

got it. thanks

[JyotinderSingh]

Thanks @apoorvyadav1111 for adding support for JSON.STRLEN!
Thanks @jujiale for the comprehensive reviews.
Merging.