Skip to content
This repository has been archived by the owner on May 7, 2020. It is now read-only.

JSONPath transformation fails on indefinite paths #4862

Closed
ThomDietrich opened this issue Jan 6, 2018 · 8 comments · Fixed by #4951
Closed

JSONPath transformation fails on indefinite paths #4862

ThomDietrich opened this issue Jan 6, 2018 · 8 comments · Fixed by #4951

Comments

@ThomDietrich
Copy link
Contributor

ThomDietrich commented Jan 6, 2018

I am not able to filter an MQTT json response with the JSONPath transformation even though my expression is correct. My guess (Edit: assessment, proof further down) is that the transformation doesn't handle indefinite path expressions correctly. According to https://github.com/json-path/JsonPath#what-is-returned-when the jayway jsonpath implementation returns results as lists as soon as the path is indefinite, e.g. when a filter expression is used.

I did my testing with the following data:

  {
    "Time": "2018-01-06T12:56:59",
    "DS18B20-1": {
      "Id": "000005FB25D0",
      "Temperature": 3.2
    },
    "DS18B20-2": {
      "Id": "031680634EFF",
      "Temperature": -18.9
    },
    "TempUnit": "C"
  }

My jayway jsonpath expression (I want to select one specific sensor and the index "DS18B20-1" could change at some point):

$.*[?(@.Id=='000005FB25D0')].Temperature

According to https://jsonpath.herokuapp.com the path returns a list rather than a value:

[
   3.2
]

Finally here is a snippet from the DEBUG log confirming my hunch:

13:07:08.921 [DEBUG] [nternal.JSonPathTransformationService] - about to transform '{"Time":"2018-01-06T13:07:08","Uptime":1,"Vcc":3.187,"POWER":"OFF","Wifi":{"AP":1,"SSId":"HotelZurBirke","RSSI":70,"APMac":"24:65:11:BF:12:D8"}}' by the function '$.Wifi.RSSI'
13:07:08.928 [DEBUG] [nternal.JSonPathTransformationService] - transformation resulted in '70'

13:07:08.997 [DEBUG] [nternal.JSonPathTransformationService] - about to transform '{"Time":"2018-01-06T13:07:08","DS18B20-1":{"Id":"000005FB25D0","Temperature":3.2},"DS18B20-2":{"Id":"031680634EFF","Temperature":-18.9},"TempUnit":"C"}' by the function '$.*[?(@.Id=='000005FB25D0')].Temperature'
13:07:09.002 [DEBUG] [nternal.JSonPathTransformationService] - transformation resulted in '[3.2]'
13:07:09.007 [WARN ] [ab.core.events.EventPublisherDelegate] - given new state is NULL, couldn't post update for 'FK_KuehlschrankTemp_Kuehl'

@clinique @SJKA would you be so kind to have a look? Thanks!

Suggestion: The JSONPath transformation should be able to handle indefinite paths / lists as JSONPath result. This means that resulting lists have to be processed. How about:

  • Yield NULL when an empty list is returned
  • Yield NULL and an log message on WARN level when a list with multiple elements is returned
  • Yield the element when the list consists of exactly one element

?

@ThomDietrich
Copy link
Contributor Author

ThomDietrich commented Jan 16, 2018

One bonus issue to consider: https://community.openhab.org/t/problem-with-spaces-in-json-keys/38847 --> the transformation could remove newlines from the source string.

Besides that I hope this counts as a friendly reminder for the above.

sjsf added a commit to sjsf/smarthome that referenced this issue Jan 19, 2018
...as JsonPath may return a List if multiple elements matched,
even if they were correctly filtered down to a single element.

fixes eclipse-archived#4862
Signed-off-by: Simon Kaufmann <simon.kfm@googlemail.com>
@sjsf
Copy link
Contributor

sjsf commented Jan 19, 2018

Makes sense to me, I quickly created #4951 to implement this.

One could argue that the first element should be chosen in case there are multiple elements returned - but I agree that your proposal is cleaner.

@ThomDietrich
Copy link
Contributor Author

Great @SJKA thanks again!!

I am not sure if that argument would be valid. What is true however is: With the right jsonpath selector you should be able to always select exactly the one element you are after. ✌️

maggu2810 pushed a commit that referenced this issue Jan 23, 2018
...as JsonPath may return a List if multiple elements matched,
even if they were correctly filtered down to a single element.

fixes #4862
Signed-off-by: Simon Kaufmann <simon.kfm@googlemail.com>
dgajic pushed a commit to dgajic/smarthome that referenced this issue Jan 27, 2018
…ed#4951)

...as JsonPath may return a List if multiple elements matched,
even if they were correctly filtered down to a single element.

fixes eclipse-archived#4862
Signed-off-by: Simon Kaufmann <simon.kfm@googlemail.com>
@5iver
Copy link
Contributor

5iver commented Feb 11, 2018

I recently updated to a recent OH snapshot, and found that the PR for this was the cause of several of my rules failing. I use the JSONPATH transform in rules, where I am expecting it to return a List because I used an indefinite path, but it looks like this case was not accounted for in the PR. Using it this way is slightly awkward though, since the transform would return List.toString, and required removing the square brackets, quotes, and splitting to convert back to a List to get a result like what would be expected from JSONPATH. After the PR, this functionality is no longer available, and the transform behaves even less like JSONPATH. So, the PR solved one thing but broke another. ☹️

Suggestions:
When an indefinite path is used, the JSONPATH transform should return List, not List.toString. This will make the transform much more straightforward to use in rules.

When transforming an item or label, providing the first element like in the PR makes sense, but I don't see how this could be done in the transform itself without breaking rule functionality. I don't know the inner workings well enough to know which file(s) this could be done in, or if it is even feasible. If not, JSONPATH transforms with indefinite paths should not be allowed for items/labels. In this case, the JS transform could be used to parse the JSON.

@5iver
Copy link
Contributor

5iver commented Feb 12, 2018

@ThomDietrich, @SJKA?

@jboeddeker
Copy link

jboeddeker commented Feb 21, 2018

If the result is a list, why should NULL be returned?

Sorry, but i don't understand this. I do not see anything wrong with returning a List.

If a consumer of a basic service (jsonpath) needs just one result of a resultset, it is wrong to restrict the basic service to a single element.

With this change, how should a consumer use the service who needs the full resultset (the list)?

I support the suggestion from @openhab-5iver , just return the list(*) instead of a converted list. With this list, every consumer is able to chose what to do by himself. Some prefer to take just the first element, some will process all and maybe some will convert it to string as before.

Why not use this Regex to parse the number out of the list: \d+\.\d+.

(*) This will break my functionality as well, but at least, i am able to work around this.

@5iver
Copy link
Contributor

5iver commented Feb 26, 2018

@ThomDietrich , @SJKA , or @kaikreuzer , is any developer available to look into this? There have been some discussions here. IMO, this is the most promising solution.

@sjsf
Copy link
Contributor

sjsf commented Feb 28, 2018

-> #5132

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants