test-fullautomation / python-jsonpreprocessor

A preprocessor for json files
Apache License 2.0
2 stars 2 forks source link

Concept clarification wanted #104

Closed HolQue closed 1 month ago

HolQue commented 10 months ago

I wanted to know what happens, when I apply an index to a string parameter:

"param1" : "value",
"param2" : ${param1}[0]

And - it's so much cool - the JsonPreprocessor acts accordingly to the Python syntax rules: He returned the corresponding character:

[DICT] (2/1) > {param1} [STR]  :  'value'
[DICT] (2/2) > {param2} [STR]  :  'v'

This is really nice and we should mention this possibility in the documentation.

But when we mention this, it's obvious, that users will assume, that also slicing is possible.

I tried:

"param1" : "value",
"param2" : ${param1}[0:-1]

And I get the usual

Expecting ',' delimiter

error message, most probably because of the colon is interpreted as part of the JSON syntax, and not as part of the Python slicing syntax.

Here we need a more strict separation.

@Thomas: Please decide if the JsonPreprocessor shall support this feature.

If not, expressions like "param2" : ${param1}[0:] must cause a better error message.

HolQue commented 10 months ago

Addendum:

In case of slicing is not supported AND in case of the content of the square brackets is not encapsulated in single quotes, then the content of the square brackets has to be interpreted as index, referencing a list element (or a single string character). Therefore a value of type int is required. Burt something like 0:-1 is not an integer.

Expected in this case are the usual Python error messages:

TypeError: string indices must be integers

or

TypeError: list indices must be integers or slices, not str

The current error message

Expecting ',' delimiter

is an indicator for a parsing problem of the JsonPreprocessor, who does not detect, that the colon is part of a dollar operator expression and must therefore not be interpreted as part of the JSON syntax.

test-fullautomation commented 10 months ago

Hi Son,

can you please check if slicing would be a problem. If we need to only allow the colon as character in the parsing expressions, then maybe we should allow it. Can be a nice feature to be able to use sub-strings.

Thank you, Thomas

test-fullautomation commented 8 months ago

@namsonx : What is the status of this issue?

test-fullautomation commented 8 months ago

Hi @namsonx , what is the status of this ticket? Thank you, Thomas

HolQue commented 8 months ago

Currently the JSON code

"param1" : "value",
"param2" : ${param1}[0:-1]

causes

'Invalid nested parameter format: ${param1}[0'

It doesn't matter here that that slicing is not supported, but like I already mentioned in https://github.com/test-fullautomation/python-jsonpreprocessor/issues/104#issuecomment-1657890797 the error message must be improved.

If square brackets do not contain a string (encapsulated in quotes) and do not contain a dollar operator expression, the content is assumed to be an index - and therefore has to be of type int. "0:-1" is not of type int. And therefore the error message has to tell exactly this, but currently tells something completely different. And in the error message the expression is cut after the colon. I am really worried about the fact, that JSON delimiters like colon still can harm the parser.

squarebracketcontent = "0:-1"
try:
   index = int(squarebracketcontent)
   print(f"index : {index}")
except Exception as reason:
   print(f"exception: {reason}")

>> exception: invalid literal for int() with base 10: '0:-1'
test-fullautomation commented 7 months ago

Error message need to be improved as only a $-Expression, integer or string can be inside square-brackets. Slicing shall be not part of this issue.

namsonx commented 6 months ago

Hello Holger,

I was not aware of the case of index like ${param1}[0:-1] I pushed new commit 7ca65cd5e008 to stabi branch for this ticket.

Thank you! Son

test-fullautomation commented 6 months ago

Hi Holger, this requires a lot of new test cases... Thank you, Thomas

HolQue commented 4 months ago

Retest causes following result:

"teststring" : "ABC",
"param01" : ${teststring}[0:],
"param02" : ${teststring}[:1],
"param03" : ${teststring}[0:1],
"param04" : ${teststring}[:],

"testlist" : ["A", "B"],
"param05" : ${testlist}[0:],
"param06" : ${testlist}[:1],
"param07" : ${testlist}[0:1],
"param08" : ${testlist}[:]

{'param01': 'ABC',
 'param02': 'A',
 'param03': 'A',
 'param04': 'ABC',
 'param05': ['A', 'B'],
 'param06': ['A'],
 'param07': ['A'],
 'param08': ['A', 'B'],
 'testlist': ['A', 'B'],
 'teststring': 'ABC'}

Slicing applied to a string and to a list works (with hard coded integers). But because of the slicing feature is not supported, an error message is expected.

The same with a dictionary:

// "testdict" : {"A" : 1, "B" : 2},
// "param09" : ${testdict}[0:], // Error: 'The variable '${testdict}[0:]' is not available!'!
// "param10" : ${testdict}[:1], // Error: 'The variable '${testdict}[:1]' is not available!'!
// "param11" : ${testdict}[0:1], // Error: 'The variable '${testdict}[0:1]' is not available!'!
// "param12" : ${testdict}[:] // Error: 'The variable '${testdict}[:]' is not available!'!

Like expected, an error is thrown. But the error message is misleading.

The expectation and proposal how to solve is already communicated in https://github.com/test-fullautomation/python-jsonpreprocessor/issues/104#issuecomment-1740777467 https://github.com/test-fullautomation/python-jsonpreprocessor/issues/104#issuecomment-1752707971

namsonx commented 4 months ago

Hello Thomas,

I will update the error message as Holger mentioned. Can we move this error message update to next release?

Thank you, Son

test-fullautomation commented 4 months ago

shifted to 0.11.0

HolQue commented 2 months ago

Concept clarification done. Decision: Slicing has to be blocked (tracked in another issue). Observation: It's possible to apply indices to strings. Therefore we should handle this as an officially confirmed feature. Remaining aspects that might not be OK (e.g. error messages), will be tracked in other issues.

This issue can be closed.

test-fullautomation commented 1 month ago

integrated in RobotFramework AIO 0.11.0