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

Task/add attribute metatadas and static attribute metadatas to jexl ctxt #1631

Merged
merged 14 commits into from
Aug 7, 2024

Conversation

AlvaroVega
Copy link
Member

@AlvaroVega AlvaroVega commented Aug 5, 2024

Issue #1630

Implemented having into account:
telefonicaid/fiware-orion#4600

  • Add test about metadata in jexl context

@AlvaroVega AlvaroVega marked this pull request as ready for review August 5, 2024 09:15
@AlvaroVega AlvaroVega changed the title Task/add metatada attributes to jexl ctxt Task/add attribute metatadas and static attribute metadatas to jexl ctxt Aug 5, 2024
@mapedraza
Copy link
Collaborator

Should context also need to be updated while calculating metadata expressions here?

// Expand metadata value expression
if (currentAttr.metadata) {
for (var metaKey in currentAttr.metadata) {
if (currentAttr.metadata[metaKey].expression && metaKey !== constants.TIMESTAMP_ATTRIBUTE) {
let newAttrMeta = {};
if (currentAttr.metadata[metaKey].type) {
newAttrMeta['type'] = currentAttr.metadata[metaKey].type;
}
let metaValueExpression;
try {
metaValueExpression = jexlParser.applyExpression(
currentAttr.metadata[metaKey].expression,
jexlctxt,
typeInformation
);
//we fallback to null if anything unexpecte happend
if (
metaValueExpression === null ||
metaValueExpression === undefined ||
Number.isNaN(metaValueExpression)
) {
metaValueExpression = null;
}
} catch (e) {
metaValueExpression = null;
}
newAttrMeta['value'] = metaValueExpression;
currentAttr.metadata[metaKey] = newAttrMeta;
}
}
}
}

@mapedraza
Copy link
Collaborator

mapedraza commented Aug 5, 2024

Would be great to add a test under JEXL section here:

// 0100 - JEXL TESTS
{
describeName: '0100 - Simple group with active attribute + JEXL expression boolean (!)',
provision: {
url: 'http://localhost:' + config.iota.server.port + '/iot/services',
method: 'POST',
json: {
services: [
{
resource: '/iot/json',
apikey: globalEnv.apikey,

@fgalan
Copy link
Member

fgalan commented Aug 5, 2024

Should context also need to be updated while calculating metadata expressions here?

// Expand metadata value expression
if (currentAttr.metadata) {
for (var metaKey in currentAttr.metadata) {
if (currentAttr.metadata[metaKey].expression && metaKey !== constants.TIMESTAMP_ATTRIBUTE) {
let newAttrMeta = {};
if (currentAttr.metadata[metaKey].type) {
newAttrMeta['type'] = currentAttr.metadata[metaKey].type;
}
let metaValueExpression;
try {
metaValueExpression = jexlParser.applyExpression(
currentAttr.metadata[metaKey].expression,
jexlctxt,
typeInformation
);
//we fallback to null if anything unexpecte happend
if (
metaValueExpression === null ||
metaValueExpression === undefined ||
Number.isNaN(metaValueExpression)
) {
metaValueExpression = null;
}
} catch (e) {
metaValueExpression = null;
}
newAttrMeta['value'] = metaValueExpression;
currentAttr.metadata[metaKey] = newAttrMeta;
}
}
}
}

Differently from attribute calculation (where there is an order based in array index) in the case of metadata we don't have such array, so we have to be careful if the calculation of one metadata depends on another calculated metadata in the same attribute. Maybe this such be warned at documentation.

doc/api.md Outdated
Comment on lines 595 to 597
Attribute metadata and Static Attribute metadata are available in the **context** under the following convention:
`metadata.AttributeName.MetadataName` or `metadata.StaticAttributeName.MetadataName` in a similar way of defined for
[Context Broker](https://github.com/telefonicaid/fiware-orion/blob/master/doc/manuals/orion-api.md#metadata-support)
Copy link
Member

Choose a reason for hiding this comment

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

It seems this paragraph is the only new thing in documentation, all the rest seems to be re-formatting.

@AlvaroVega could you confirm, pls?

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Attribute metadata and Static Attribute metadata are available in the **context** under the following convention:
`metadata.AttributeName.MetadataName` or `metadata.StaticAttributeName.MetadataName` in a similar way of defined for
[Context Broker](https://github.com/telefonicaid/fiware-orion/blob/master/doc/manuals/orion-api.md#metadata-support)
Attribute metadata and Static Attribute metadata are available in the **context** under the following convention:
`metadata.<AttributeName>.<MetadataName>` or `metadata.<StaticAttributeName>.<MetadataName>` in a similar way of defined for
[Context Broker](https://github.com/telefonicaid/fiware-orion/blob/master/doc/manuals/orion-api.md#metadata-support)

Using the <...> syntax would make more explicit that they refer to actual names, different to metadata which is a literal token.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it is

@AlvaroVega
Copy link
Member Author

Would be great to add a test under JEXL section here:

// 0100 - JEXL TESTS
{
describeName: '0100 - Simple group with active attribute + JEXL expression boolean (!)',
provision: {
url: 'http://localhost:' + config.iota.server.port + '/iot/services',
method: 'POST',
json: {
services: [
{
resource: '/iot/json',
apikey: globalEnv.apikey,

Fixed in c8778c7

@AlvaroVega
Copy link
Member Author

Should context also need to be updated while calculating metadata expressions here?

// Expand metadata value expression
if (currentAttr.metadata) {
for (var metaKey in currentAttr.metadata) {
if (currentAttr.metadata[metaKey].expression && metaKey !== constants.TIMESTAMP_ATTRIBUTE) {
let newAttrMeta = {};
if (currentAttr.metadata[metaKey].type) {
newAttrMeta['type'] = currentAttr.metadata[metaKey].type;
}
let metaValueExpression;
try {
metaValueExpression = jexlParser.applyExpression(
currentAttr.metadata[metaKey].expression,
jexlctxt,
typeInformation
);
//we fallback to null if anything unexpecte happend
if (
metaValueExpression === null ||
metaValueExpression === undefined ||
Number.isNaN(metaValueExpression)
) {
metaValueExpression = null;
}
} catch (e) {
metaValueExpression = null;
}
newAttrMeta['value'] = metaValueExpression;
currentAttr.metadata[metaKey] = newAttrMeta;
}
}
}
}

Done in 344cb5c

@AlvaroVega
Copy link
Member Author

Differently from attribute calculation (where there is an order based in array index) in the case of metadata we don't have such array, so we have to be careful if the calculation of one metadata depends on another calculated metadata in the same attribute. Maybe this such be warned at documentation.

Done in a103f39

@@ -511,6 +535,11 @@ function sendUpdateValueNgsi2(entityName, originMeasures, originTypeInformation,
}
newAttrMeta['value'] = metaValueExpression;
currentAttr.metadata[metaKey] = newAttrMeta;

//RE-Populate de JEXLcontext
if (jexlctxt.metadata && jexlctxt.metadata[currentAttr.name]) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This if clause checks if the metadata exits into the original jexl context but, in the case of new metadatas generated as a result of expressions, it would not be included. I think It should be appended to the jexlctxt even if it was not part previously of it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in 5da8640

Copy link
Collaborator

@mapedraza mapedraza left a comment

Choose a reason for hiding this comment

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

LGTM

doc/api.md Outdated Show resolved Hide resolved
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

@fgalan fgalan merged commit 2bfa8ea into master Aug 7, 2024
8 checks passed
@fgalan fgalan deleted the task/add_metatada_attributes_to_jexl_ctxt branch August 7, 2024 11:20
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