Bug 65681 - JSON Extractor returns empty string for null value instead of returning Default value
Summary: JSON Extractor returns empty string for null value instead of returning Defau...
Status: RESOLVED FIXED
Alias: None
Product: JMeter - Now in Github
Classification: Unclassified
Component: Main (show other bugs)
Version: 5.4.1
Hardware: All All
: P2 normal (vote)
Target Milestone: JMETER_5.5
Assignee: JMeter issues mailing list
URL:
Keywords: FixedInTrunk, PatchAvailable
Depends on:
Blocks:
 
Reported: 2021-11-13 13:50 UTC by Philippe Mouawad
Modified: 2021-11-21 14:00 UTC (History)
1 user (show)



Attachments
Test plan reproducer (11.16 KB, application/xml)
2021-11-13 13:50 UTC, Philippe Mouawad
Details
Set default value when a null value is found by the JSON path expression (5.17 KB, patch)
2021-11-13 14:54 UTC, Felix Schumacher
Details | Diff
Set default value when a null value is found by the JSON path expression (12.11 KB, patch)
2021-11-14 11:38 UTC, Felix Schumacher
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Philippe Mouawad 2021-11-13 13:50:35 UTC
Created attachment 38085 [details]
Test plan reproducer

In attached test plan "$.totoman_gap_code" is null but second extractor extracts empty string.

In JSON we have:
"totoman_gap_code":null

But you can see in Debug Sampler that we end up with:

productGapCode=
productGapCode_matchNr=1


It seems issue appears when combining extractors ie, JSON Extractor - productGapCode uses a variable "productRetailerOffer" extracted by JSON Extractor - productRetailerOffer.


Thanks
Comment 1 Felix Schumacher 2021-11-13 14:16:28 UTC
That probably comes from JSONManager#stringifyObject, that converts null to empty strings. (will look a bit deeper in a minute)
Comment 2 Felix Schumacher 2021-11-13 14:30:30 UTC
On the other hand, the default value currently seems to be used, when
 - a list of values is extracted and the index to return is larger than the list
 - no value could be extracted from the list
It doesn't look for empty string or null.

We could probably change the implementation that it does not convert null values to empty strings and convert null values to default values. Is that, what you want?
Comment 3 Philippe Mouawad 2021-11-13 14:35:57 UTC
Hello Felix,
Yes I think that would be expected behaviour

Thanks for your rapid feed-back 
Regards
Comment 4 Felix Schumacher 2021-11-13 14:54:49 UTC
Created attachment 38086 [details]
Set default value when a null value is found by the JSON path expression

This will change the current behaviour of empty string for null values to default value.
Comment 5 Felix Schumacher 2021-11-14 11:38:35 UTC
Created attachment 38091 [details]
Set default value when a null value is found by the JSON path expression

I added test cases for the new behaviour.

I am not sure, whether the handling of multiple results is correct or intended.

Consider the structure '[{"c": null}, {"c": "abc"}]' and the path '$[*].c' with a default value of "NONE". JSON Path will lead to a result of '[null, "abc"]' which we will enhance to '["NONE", "abc"]'. (That sounds valid)

But, what if the user intended to give a default value for a complete absent result? Like using an expression '$.listOfValues' on the above structure and an expected default value '[1, 2, 3]'? In that case the new implementation would yield unexpected results.

What are your opinions on this?
Comment 6 Philippe Mouawad 2021-11-14 19:55:41 UTC
(In reply to Felix Schumacher from comment #5)
> Created attachment 38091 [details]
> Set default value when a null value is found by the JSON path expression
> 
> I added test cases for the new behaviour.
Thanks for investigation and fix.
> 
> I am not sure, whether the handling of multiple results is correct or
> intended.
> 
> Consider the structure '[{"c": null}, {"c": "abc"}]' and the path '$[*].c'
> with a default value of "NONE". JSON Path will lead to a result of '[null,
> "abc"]' which we will enhance to '["NONE", "abc"]'. (That sounds valid)
> 
Agreed

> But, what if the user intended to give a default value for a complete absent
> result? Like using an expression '$.listOfValues' on the above structure and
> an expected default value '[1, 2, 3]'? In that case the new implementation
> would yield unexpected results.

I don't get the case.
What will be the result vs the previous behaviour ? Was it more consistent before ? 



What 
> 
> What are your opinions on this?
Comment 7 Felix Schumacher 2021-11-15 11:01:28 UTC
(In reply to Philippe Mouawad from comment #6)
> (In reply to Felix Schumacher from comment #5)
> > Created attachment 38091 [details]
> > Set default value when a null value is found by the JSON path expression
> > 
> > I added test cases for the new behaviour.
> Thanks for investigation and fix.
> > 
> > I am not sure, whether the handling of multiple results is correct or
> > intended.
> > 
> > Consider the structure '[{"c": null}, {"c": "abc"}]' and the path '$[*].c'
> > with a default value of "NONE". JSON Path will lead to a result of '[null,
> > "abc"]' which we will enhance to '["NONE", "abc"]'. (That sounds valid)
> > 
> Agreed
> 
> > But, what if the user intended to give a default value for a complete absent
> > result? Like using an expression '$.listOfValues' on the above structure and
> > an expected default value '[1, 2, 3]'? In that case the new implementation
> > would yield unexpected results.
> 
> I don't get the case.
> What will be the result vs the previous behaviour ? Was it more consistent
> before ? 

Well, when no result is found, we get the default value, when we get a list with non null values, we get the (probably) intended result, but when we get a list with null and non null values, it might be unexpected:

$.listOfValues    |   result
------------------------------
null              |  [1, 2, 3]
""                |  ""
[4, 5]            |  [4, 5]
[1, null]         |  [1, [1, 2, 3]]

But while writing this, I wonder, whether the default value is always a string and therefore the results would be "[1, 2, 3]"; ""; [4, 5]; [1, "[1, 2, 3]"]

I hope that makes my concerns clearer (note, I haven't checked the old behaviour)
Comment 8 Philippe Mouawad 2021-11-15 12:19:11 UTC
Hello Felix,
Nothing hurts me in the results.
Which one disturbs you ?
Thanks
Comment 9 Felix Schumacher 2021-11-15 14:56:20 UTC
The last one seems surprising to me, but if you think they are all OK, I will check in the patch.
Comment 10 Felix Schumacher 2021-11-15 15:07:31 UTC
@Philippe, could you test next build from trunk, or nightly and report back, if it fixes your problem?

commit b06f9def2a5c7007a6ee804487fc259c0fbd747b
AuthorDate: Sat Nov 13 15:46:37 2021 +0100

    Use default values for null values when extracting with JSONPostProcessor
    
    When we extract a null value for a given expression, we now replace that
    null with the given default value for the expression.
    
    That leads to slight change of the meaning of a missing value.
    
    Consider the JSON structure: {"context": null} and the JSON Path: "$.context"
    
    The new implementation will replace the `null` with the default value, while
    the old would have stringified it to the empty string.
    
    Cleanup of parameter handling of default values. We should use the default
    value for the current handled expression and not pass all default values
    for all expression. That leads to confusion, only. (Well, I was confused)
    
    Bugzilla Id: 65681
---
 .../extractor/json/jsonpath/JSONManager.java       |  2 +-
 .../extractor/json/jsonpath/JSONPostProcessor.java | 51 +++++++++++-----------
 .../jmeter/extractor/TestJSONPostProcessor.java    | 25 ++++++++---
 xdocs/changes.xml                                  |  1 +
 4 files changed, 45 insertions(+), 34 deletions(-)
Comment 11 Felix Schumacher 2021-11-20 11:14:00 UTC
Nikola pointed out, that now - to be more consistent - the JMESPathExtractor should behave the same way. I tend to agree. What do you think?
Comment 12 Felix Schumacher 2021-11-20 11:52:19 UTC
Though checking an example he sent to me with our current implementation and an online JMESPath checker, it seems, that JMESPath does not return null values (at least not in his example).

The example is

  { "values": [null, "", {}, [], 1] }

and the JMESPath expression would be values[*] (JSON Path would be $.values[*])
The results are
JMESPath: ["", {}, [], 1]
JSONPath: [null, "", {}, [], 1] (or with the new impl and a default value of NF ["NF", "", {}, {], 1])

I haven't found a way to configure our used JMESPath engine to return null values, so currently we can't do anything but document that behaviour.
Comment 13 eR@SeR 2021-11-21 12:52:09 UTC
Hello,

I did the testing of the new implementation and it seems to work for cases that I can think of. Others are welcome to do the same :)

Note: If the Default value is left empty, null and empty values will be identical i.e. null will be empty too, but that should be intended behavior, I guess.

The question is whether the same should be used for JSON JMESPath Extractor and all other extractors where possible, to have unique behavior?

I used the following example to compare the behavior of JSON and JSON JMESPath Extractor:

{"category_ids":[null,"",9615,9617]}

json expr: $.category_ids[*] ---> match no: -1 ----> Default values: NF

we have:

JSONCategory_ids_1=NF
JSONCategory_ids_2=
JSONCategory_ids_3=9615
JSONCategory_ids_4=9617
JSONCategory_ids_matchNr=4

======================================
but in JSON JMESPath Extractor:

json jmes expr: category_ids[*] ---> match no: -1 ----> Default values: NF

we have:

JMESPathCategory_ids_1=
JMESPathCategory_ids_2=9615
JMESPathCategory_ids_3=9617
JMESPathCategory_ids_matchNr=3

IMO, they should behave the same in JMeter, to have consistency among JSON extractors. Not only for this example, but in general. If that is not possible, as Felix pointed out, then provide the examples which produce different outputs in JSON extractors in the corresponding documentation.
Comment 14 Philippe Mouawad 2021-11-21 14:00:46 UTC
Hello Felix,
I tested nightly build ,it works for me.

Thanks
Comment 15 The ASF infrastructure team 2022-09-24 20:38:22 UTC
This issue has been migrated to GitHub: https://github.com/apache/jmeter/issues/5590