Skip to content
This repository has been archived by the owner. It is now read-only.

JsonPath transformation service comply with the contract #4379

Merged
merged 1 commit into from Oct 10, 2017

Conversation

@sjsf
Copy link
Contributor

@sjsf sjsf commented Oct 5, 2017

...as it was returning null in case the path pointed to an element which did not exist.
Now it throws a TransformationException.

I'm not 100% sure though if this really is always the right way of fixing it. A missing element in JSON is somewhat similar to "intentionally null", i.e. UndefType.UNDEF might also be a consideration. Any deeper insights anybody?

fixes #4376
Signed-off-by: Simon Kaufmann simon.kfm@googlemail.com

...as it was returning null in case the path pointed to an element which did not exist.

fixes eclipse-archived#4376
Signed-off-by: Simon Kaufmann <simon.kfm@googlemail.com>
@sjsf sjsf force-pushed the fixJsonTransformation branch from 5cd0969 to f90852c Oct 6, 2017
@kaikreuzer
Copy link
Contributor

@kaikreuzer kaikreuzer commented Oct 6, 2017

Any deeper insights anybody?

Not from my end.
Shall we wait for insights from others or "just merge"...?

@sjsf
Copy link
Contributor Author

@sjsf sjsf commented Oct 6, 2017

Then suggest using merging it because it now fits to the javadoc contract. If it turns out this is not appropriate, then we still can adapt it afterwards.

@martinvw
Copy link
Contributor

@martinvw martinvw commented Oct 6, 2017

Hmm, reading your suggestion UndefType.UNDEF or UndefType.NULL might make more sense, it is after all an element which may be only missing this time. And it's not uncommon in JSON the leave out any null values, see for example some NPE's I got in the Nest binding because I do not posses all Nest devices.

htreu
htreu approved these changes Oct 9, 2017
Copy link
Contributor

@htreu htreu left a comment

lgtm, should be merged.

@htreu
Copy link
Contributor

@htreu htreu commented Oct 9, 2017

@martinvw imho the transformation service is right about throwing a TransformationException. Its in the responsibility of the caller to act appropriate. And from the code I saw the exception is handled well by all callers.

@kaikreuzer kaikreuzer merged commit f1c63ba into eclipse-archived:master Oct 10, 2017
2 checks passed
@sjsf sjsf deleted the fixJsonTransformation branch Nov 14, 2017
@kaikreuzer kaikreuzer added this to the 0.9.0 milestone Nov 30, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

4 participants