Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Not evaluable expression should be treated as null when using skipValue #1440

Closed
mapedraza opened this issue Sep 11, 2023 · 10 comments
Closed

Comments

@mapedraza
Copy link
Collaborator

mapedraza commented Sep 11, 2023

IoT Agent Node Lib version the issue has been seen with

Latest (3.3.0-next)

Bound or port used (API interaction)

Other

NGSI version

NGSIv2

Are you running a container?

Yes, I am using a contaner (Docker, Kubernetes...)

Image type

None

Expected behaviour you didn't see

When an active attribute expression involving a measurement ("expression":"t*2")is evaluated, if missing parameter (I.E: when a device sends {"m":3} the resulting entity attribute is null. This is the expected.

When defining active attribute expression as constant null ("expression":null) and "skipValue":null, when sending any measurement, no matter the parameters, the attribute is not updated by the IoTA since the expression is constant and the result always match with skipValue. This is the expected value

When, for a given active attribute, with "skipValue":null and an expression "expression":"t*2", a measurement without that parameter ({"m":3}) arrives, the attribute is written with a null value. This is not the expected result. It is expected that IoTA should not update the value.

There is a workaround to make this working, by defining the expression like t?t*2:null, which will force to null the value. It is also possible to define a value different that null as skipValue, and it will work too. I.E: "expression":"t?t*2:\"invalid\"" and "skipValue":"invalid"

Probably, this is because internally, the result of an expression that does not have all the parameters available is an error that, in a subsequent stage, is transformed to a null. And the comparison with skipValue, in order to avoid persisting it into the CB is done in between.

Unexpected behaviour you saw

No response

Steps to reproduce the problem

No response

Configs

No response

Log output

No response

CC: @manucarrace

@AlvaroVega
Copy link
Member

AlvaroVega commented Sep 11, 2023

https://github.com/telefonicaid/iotagent-node-lib/blob/master/doc/api.md
skipValue: indicates that if the result of applying expression to a measure is equal to the value of skipValue then the attribute corresponding to the measure is not sent to CB. In other words, this field is not an expression, it is a value that is compared with the result of applying expression to a measure. By default if skipValue is not defined then is considered as null (i.e. if the result of apply expression results in null then corresponding attribute is not sent to CB). It is only used if expression is provided (otherwise is ignored).

So this case

When an active attribute expression involving a measurement ("expression":"t*2")is evaluated, if missing parameter (I.E: when a device sends {"m":3} the resulting entity attribute is null. This is the expected.

is the same that
When, for a given active attribute, with "skipValue":null and an expression "expression":"t*2", a measurement without that parameter ({"m":3}) arrives, the attribute is written with a null value.

@mrutid
Copy link
Member

mrutid commented Sep 11, 2023

"When, for a given active attribute, with "skipValue":null and an expression "expression":"t*2", a measurement without that parameter ({"m":3}) arrives, the attribute is written with a null value. This is not the expected result. It is expected that IoTA should not update the value."

In JEXL null*2 == null no exception or error. are we sure about this behaviour?

It's not the same than "nojson"|jsonparse (Error: SyntaxError: Unexpected end of JSON input) in which case we convey to return null value also.

@mapedraza
Copy link
Collaborator Author

mapedraza commented Sep 11, 2023

After some research in JEXL playground, when different expressions applied:

a?"existo":null -> returns undefined (IoTA does not persists the measure)
a*10 -> returns null (IoTA persist the measure)

Checking the code, at this part:

if (expressionPlugin.contextAvailable(attr.expression, ctxt, typeInformation)) {
res = expressionPlugin.applyExpression(attr.expression, ctxt, typeInformation);
if (
// By default undefined is equivalent to null: should not progress
(attr.skipValue === undefined && res === null) ||
(attr.skipValue !== undefined && res === attr.skipValue)
) {
logger.debug(
context,
'sendUpdateValueNgsi2 skip value=%j for res=%j with expression=%j',
attr.skipValue,
res,
attr.expression
);
delete payload.entities[0][j]; // remove measure attr
attr = undefined; // stop process attr
}
} else {
logger.info(
context,
'sendUpdateValueNgsi2 no context available for apply expression=%j',
attr.expression
);
res = newAttr.value; // keep newAttr value

Probably something like this could help:

 if (expressionPlugin.contextAvailable(attr.expression, ctxt, typeInformation)) { 
     res = expressionPlugin.applyExpression(attr.expression, ctxt, typeInformation); 
     if ( 
         // By default undefined is equivalent to null: should not progress 
         (attr.skipValue === undefined && (res === null || res === undefined || isNaN(res))) || 
         (attr.skipValue !== undefined && res === attr.skipValue) 

@AlvaroVega
Copy link
Member

AlvaroVega commented Sep 11, 2023

in this case:

res === attr.skipValue

computation result is false since res is a number (null *2 -> 0) and attrSkipValue is null

 js
Welcome to Node.js v18.13.0.
Type ".help" for more information.
> null * 2
0

it seems that in TypeScript/JS/ES, an arithmetic operation involving a number and null will cause null to be implicit converted to a number. And null gets converted to 0.

@fgalan
Copy link
Member

fgalan commented Sep 12, 2023

PR #1441 has been merged.

@mapedraza maybe it would be a good idea checking the fix before closing this issue (I'm relaunching IOTA-JSON, IOTA-UL and IOTA Manager builds at dockerhub right now, to get the latest from IOTA library code).

@AlvaroVega
Copy link
Member

The following cases should be tested with iotagent after #1441

{
a:v,
b:v*3,
c:v==null,
d:v?"soy null":"no soy null",
e:v==null?"soy null":"no soy null",
f:(v*3)?"soy null":"no soy null",
g:v == undefined
}

@mapedraza
Copy link
Collaborator Author

I am going to open a new PR adding a test for cases proposed by @AlvaroVega. After that, if everything works well, we can close the issue as proposed by @fgalan

@AlvaroVega
Copy link
Member

The following cases should be tested with iotagent after #1441

{
a:v,
b:v*3,
c:v==null,
d:v?"soy null":"no soy null",
e:v==null?"soy null":"no soy null",
f:(v*3)?"soy null":"no soy null",
g:v == undefined
}

I believe that these cases are currently covered by https://github.com/telefonicaid/iotagent-node-lib/blob/master/test/unit/ngsiv2/expressions/jexlBasedTransformations-test.js

@mapedraza
Copy link
Collaborator Author

@fgalan
Copy link
Member

fgalan commented Oct 3, 2023

Fixed by PR #1474

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

No branches or pull requests

4 participants