Skip to content

Comments

JEXL expression in subject.condition #4555#4738

Merged
fgalan merged 21 commits intomasterfrom
feature/4555_jexlexpression_in_subject_condition
Jan 7, 2026
Merged

JEXL expression in subject.condition #4555#4738
fgalan merged 21 commits intomasterfrom
feature/4555_jexlexpression_in_subject_condition

Conversation

@orianar
Copy link
Collaborator

@orianar orianar commented Dec 16, 2025

Issue #4555

@fgalan
So far, the JexlExpression field does not use macros. Should we add support for them?

@orianar orianar force-pushed the feature/4555_jexlexpression_in_subject_condition branch from 4a1a886 to 018862c Compare December 16, 2025 03:54
@fgalan
Copy link
Member

fgalan commented Dec 16, 2025

So far, the JexlExpression field does not use macros. Should we add support for them?

Do you mean

"jexlExpression": "${(A|isNaN) && (B|isNaN)}"

instead of

"jexlExpression": "(A|isNaN) && (B|isNaN)"

?

I think we don't need ${...} in this case, as this is not the same case than the ngsi notification payload (in which case, the string replaced by ${...} may be included in a larger string, eg: "this is my expresion: ${...}". In this case we only need to a value of truth (either true or false in a boolean way).

@orianar orianar changed the title [WIP] JEXL expression in subject.condition #4555 JEXL expression in subject.condition #4555 Dec 17, 2025
@orianar orianar requested a review from fgalan December 17, 2025 04:22
bool basic = false;
#endif

// Build JEXL evaluation contexts using entity attributes and their metadata
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest to do the JEXL evaluation after string filters (which is less expensive) and before than geo-evaluation (which is more expensive, as it hits de MongoDB).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in #1c20aeb

Comment on lines 2087 to 2091
if (!result.boolValue)
{
continue;
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the case of the expression that evaluations to a number or string (e.g. 'a|logor 's|uppercase), isresult.boolValueset tofalse`?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -302,6 +302,8 @@ Fields:
- **expression**: an expression used to evaluate if notifications has
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CHANGES_NEXT_RELEASE entry regarding the changes in this PR should be included

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in #bf39cba

@@ -5033,6 +5033,8 @@ A `condition` contains the following subfields:
| `expression` | ✓ | object| An expression composed of `q`, `mq`, `georel`, `geometry` and `coords` (see [List Entities](#list-entities-get-v2entities) operation above about this field). `expression` and sub elements (i.e. `q`) must have content, i.e. `{}` or `""` is not allowed. `georel`, `geometry` and `coords` have to be used together (i.e. "all or nothing"). Check the example using geoquery as expression [below](#create-subscription-post-v2subscriptions).|
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a section "JEXL Support" https://github.com/telefonicaid/fiware-orion/blob/master/doc/manuals/orion-api.md#jexl-support but what it is actually describin in "JEXL Support in Custom Notifications"

I'd suggest to change this name, in the section title, in the TOC and in links (search for #jexl-support literal among directory files)

(Up to this PR, the diference was not meaninful, but now whe have JEXL support in a different place, i.e. subscription conditions, so I think is better changing the same)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in #1c20aeb

Comment on lines 33 to 38
# 01. Create httpCustom sub with subject.condition.jexlExpression
# 02. Get sub, see sub with subject.condition.jexlExpression
# 03. Update sub description
# 04. Get sub, see subject.condition.jexlExpression
# 05. Update sub subject.condition.jexlExpression
# 06. see subject.condition.jexlExpression
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand this .test is testing the basic CRUD of the new field. I'd suggest to add the following steps

# 07. Update subject.condition removing jexlExpression
# 08. Get sub, see now jexlExpression in subject.condition
# 09. Update subject.condition adding jexlExpression again
# 10. Get sub, see subject.condition.jexlExpression

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in#1c20aeb

# 01. Create custom sub with condition.JexlExpression (A|isNaN) && (B|isNaN)
# 02. Create entity E1 with A=1 and B=2
# 03. Update entity E1 with A=foo and B=bar
# 04. Update entity E1 with A=2.1 and B=-3.8
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe better

Suggested change
# 04. Update entity E1 with A=2.1 and B=-3.8
# 04. Update entity E1 with A=zzzz and B=-3.8

To have a case in which the values of both operands in && are different

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in#1c20aeb

Comment on lines 38 to 40
# 04. Update entity E1 with A=2.1 and B=-3.8
# 05. Dump accumulator and see ONE notification (only for step 02: A=1, B=2)
# (the updates in steps 03 and 04 must NOT trigger notifications)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest to add an additional step:

Suggested change
# 04. Update entity E1 with A=2.1 and B=-3.8
# 05. Dump accumulator and see ONE notification (only for step 02: A=1, B=2)
# (the updates in steps 03 and 04 must NOT trigger notifications)
# 04. Update entity E1 with A=2.1 and B=-3.8
# 05. Update entity E1 with A=6.8 and B=-3.8
# 06. Dump accumulator and see TWO notification (for steps 02: A=1, B=2 and 05: A=6.8 and B=-3.8)
# (the updates in steps 03 and 04 must NOT trigger notifications)

Copy link
Collaborator Author

@orianar orianar Dec 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@@ -0,0 +1,203 @@
# Copyright 2025 Telefonica Investigacion y Desarrollo, S.A.U
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest to add a variant of this test in a new .test in which only one of the attributes comes in the notification (e.g. A) and the other is an existing one in the entity (e.g. B).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in #1c20aeb

@@ -0,0 +1,203 @@
# Copyright 2025 Telefonica Investigacion y Desarrollo, S.A.U
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest to add a variant of this test in a new .test in which only one of the attributes comes in the notification (e.g. A) and the other is an existing one in the entity (e.g. B).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in #1c20aeb

@orianar orianar requested a review from fgalan December 21, 2025 23:27
fgalan added 2 commits January 7, 2026 16:39
Clarified notification triggering rules and expressions in the documentation.
Copy link
Collaborator Author

@orianar orianar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

fgalan and others added 2 commits January 7, 2026 17:26
Co-authored-by: Fermín Galán Márquez <fgalan@users.noreply.github.com>
Copy link
Member

@fgalan fgalan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (except for a small typo in one of the .test files)

orianar and others added 3 commits January 7, 2026 10:51
…tion/jexl_expression_in_condition_sum_eq3_mixed_update_existing_attrs.test

Co-authored-by: Fermín Galán Márquez <fgalan@users.noreply.github.com>
…tion/jexl_expression_in_condition_sum_eq3_mixed_update_existing_attrs.test

Co-authored-by: Fermín Galán Márquez <fgalan@users.noreply.github.com>
…tion/jexl_expression_in_condition_sum_eq3_mixed_update_existing_attrs.test

Co-authored-by: Fermín Galán Márquez <fgalan@users.noreply.github.com>
@fgalan fgalan merged commit bd3b361 into master Jan 7, 2026
17 checks passed
@fgalan
Copy link
Member

fgalan commented Jan 7, 2026

@fisuda this PR does some modifications to .md English files. It would be great if you could sync the Japanese translation, please.

@fgalan fgalan deleted the feature/4555_jexlexpression_in_subject_condition branch January 7, 2026 18:16
fisuda added a commit to fisuda/fiware-orion that referenced this pull request Jan 8, 2026
@fisuda
Copy link
Contributor

fisuda commented Jan 8, 2026

I sent the PR #4739.
Thanks.

fgalan added a commit that referenced this pull request Jan 8, 2026
(JP) JEXL expression in subject.condition (#4738)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants