Skip to content

remove cast by type after apply JEXL expression #1034

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.

Sign up for GitHub

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

Merged
fgalan merged 13 commits into from
May 13, 2021
Merged

remove cast by type after apply JEXL expression #1034

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
76e641d
remove cast by type after apply expression
AlvaroVega May 6, 2021
363fb92
update tests
AlvaroVega May 10, 2021
e9c32a8
split updatecontext for each expresion plugin
AlvaroVega May 10, 2021
f6d7537
keep original values
AlvaroVega May 10, 2021
35b19e1
update test
AlvaroVega May 11, 2021
d28e640
remove non possible tests
AlvaroVega May 11, 2021
85f0af1
remove comment block
AlvaroVega May 11, 2021
afacaa4
update CNR
AlvaroVega May 11, 2021
c6a1f70
Update CHANGES_NEXT_RELEASE
AlvaroVega May 12, 2021
086a53f
removed non valid tests
AlvaroVega May 12, 2021
8f3ea37
rename json example files
AlvaroVega May 12, 2021
297602a
fix mock file name
AlvaroVega May 12, 2021
dfdfbcb
Merge branch 'master' into task/remove_casts_by_type_after_apply_expr…
fgalan May 13, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES_NEXT_RELEASE
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
- Fix: Do not transform attribute value using attribute type after apply expression plugin JEXL (#1036)
- Fix: check access to data in fillService facility
- Fix: use expressionLanguage defined in group is not defined at device level (#1027)
- Fix: ensure service of groups, device and commands is stored in mongo in lowercase (#1023)
Expand Down
20 changes: 0 additions & 20 deletions lib/plugins/jexlParser.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,26 +125,6 @@ function expressionApplier(context, typeInformation) {
}

newAttribute.value = applyExpression(attribute.expression, context, typeInformation);

Copy link
Member

@fgalan fgalan May 12, 2021

Choose a reason for hiding this comment

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

Is there any piece of documentation about this autocast feature that should be also removed/modified?

Copy link
Member Author

@AlvaroVega AlvaroVega May 12, 2021

Choose a reason for hiding this comment

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

There is nothing about this autocast in https://github.com/telefonicaid/iotagent-node-lib/blob/master/doc/expressionLanguage.md#jexl-based-transformations

Copy link
Member

@fgalan fgalan May 13, 2021

Choose a reason for hiding this comment

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

NTC

if (attribute.type === 'Number' && isFloat(newAttribute.value)) {
newAttribute.value = Number.parseFloat(newAttribute.value);
} else if (attribute.type === 'Number' && !Number.isNaN(Number.parseInt(newAttribute.value))) {
newAttribute.value = Number.parseInt(newAttribute.value);
} else if (attribute.type === 'Boolean') {
newAttribute.value = newAttribute.value === 'true' || newAttribute.value === '1';
} else if (attribute.type === 'None') {
newAttribute.value = null;
} else if (attribute.type === 'Text' || attribute.type === 'String') {
newAttribute.value = String(newAttribute.value);
} else if (
attribute.type.toLowerCase().includes('structuredvalue') ||
attribute.type.toLowerCase().startsWith('geo') ||
attribute.type.toLowerCase().includes('json')
) {
newAttribute.value = newAttribute.value;
} else {
newAttribute.value = String(newAttribute.value);
}
return newAttribute;
};
}
Expand Down
17 changes: 17 additions & 0 deletions test/unit/examples/contextRequests/updateContextExpressionPlugin1b.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"contextElements": [
{
"type": "Light",
"isPattern": "false",
"id": "light1",
"attributes": [
{
"name": "pressure",
"type": "Hgmm",
"value": 1040
}
]
}
],
"updateAction": "UPDATE"
}
27 changes: 27 additions & 0 deletions test/unit/examples/contextRequests/updateContextExpressionPlugin2b.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
{
"contextElements": [
{
"type": "WeatherStation",
"isPattern": "false",
"id": "ws1",
"attributes": [
{
"name": "pressure",
"type": "Hgmm",
"value": 1040
},
{
"name": "humidity",
"type": "Percentage",
"value": "12"
},
{
"name": "weather",
"type": "Summary",
"value": "Humidity 6 and pressure 1040"
}
]
}
],
"updateAction": "UPDATE"
}
17 changes: 17 additions & 0 deletions test/unit/examples/contextRequests/updateContextExpressionPlugin3b.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"contextElements": [
{
"type": "WeatherStation",
"isPattern": "false",
"id": "ws1",
"attributes": [
{
"name": "pressure",
"type": "Hgmm",
"value": 1040
}
]
}
],
"updateAction": "UPDATE"
}
4 changes: 2 additions & 2 deletions test/unit/expressions/expressionCombinedTransformations-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ describe('Combine Jexl and legacy expressions (default JEXL)', function () {
.matchHeader('fiware-servicepath', 'gardens')
Copy link
Member

@fgalan fgalan May 12, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

We have also a variant of this tests for ngsiv2

test/unit/ngsiv2/expressions/expressionCombinedTransformations-test.js

That file doesn't need modifications? (looking to CI results, it seems it doesn't, but I wonder why...)

Copy link
Member Author

@AlvaroVega AlvaroVega May 12, 2021

Choose a reason for hiding this comment

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

expressionCombinedTransformations-test.js includes tests which legacy and jexl expression, but not any of now not allowed cases:

Update for an integer attribute with string expression
Update for a Float attribute with string expression
Update for a Null attribute with arithmetic expression
Update for a Null attribute with string expression
Update for a Boolean attribute with arithmetic expression
Update for a Boolean attribute with string expression

Copy link
Member

@fgalan fgalan May 13, 2021

Choose a reason for hiding this comment

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

I have had a look into this with more detail.

In test/unit/expressions/expressionCombinedTransformations-test.js (NGSIv1 variant) the only difference between old files (e.g. test/unit/examples/contextRequests/updateContextExpressionPlugin3.json) and new files (test/unit/examples/contextRequests/updateContextExpressionPlugin8.json) is that the old uses "value": "1040" ("stringfied") and the new "value": 1040 ("native").

On the other hand test/unit/ngsiv2/expressions/expressionCombinedTransformations-test.js (NGSIv2 variant) already uses the native way, e.g. see test/unit/ngsiv2/examples/contextRequests/updateContextExpressionPlugin1.json.

Thus, I think that explains that only test/unit/expressions/expressionCombinedTransformations-test.js is changed in this PR but not test/unit/ngsiv2/expressions/expressionCombinedTransformations-test.js

NTC

.post(
'/v1/updateContext',
utils.readExampleFile('./test/unit/examples/contextRequests/updateContextExpressionPlugin3.json')
utils.readExampleFile('./test/unit/examples/contextRequests/updateContextExpressionPlugin3b.json')
Copy link
Member

@fgalan fgalan May 12, 2021

Choose a reason for hiding this comment

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

Instead of creating a new file (with the b suffix) I think it would be getter to edit the current ones. This way, the removal done in PR #995 would be easy (as there files are currently taken into account for the removal).

Copy link
Member Author

@AlvaroVega AlvaroVega May 12, 2021

Choose a reason for hiding this comment

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

IMHO is not possible edit current file, and should be splited to use in different tests.
Renamed files done into 8f3ea37

Copy link
Member

@fgalan fgalan May 13, 2021

Choose a reason for hiding this comment

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

Ok. I see (for instance https://github.com/telefonicaid/iotagent-node-lib/search?q=updateContextExpressionPlugin3).

I'll comment about the new files added in this PR on #995

Copy link
Member

@fgalan fgalan May 13, 2021

Choose a reason for hiding this comment

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

I'll comment about the new files added in this PR on #995

Here: #995 (comment)

NTC

)
.reply(
200,
Expand Down Expand Up @@ -285,7 +285,7 @@ describe('Combine Jexl and legacy expressions (default Legacy)', function () {
.matchHeader('fiware-servicepath', 'gardens')
.post(
'/v1/updateContext',
utils.readExampleFile('./test/unit/examples/contextRequests/updateContextExpressionPlugin3.json')
utils.readExampleFile('./test/unit/examples/contextRequests/updateContextExpressionPlugin3b.json')
)
.reply(
200,
Expand Down
6 changes: 3 additions & 3 deletions test/unit/expressions/jexlBasedTransformations-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ describe('Javascript Expression Language (JEXL) based transformations plugin ',
.matchHeader('fiware-servicepath', 'gardens')
.post(
'/v1/updateContext',
utils.readExampleFile('./test/unit/examples/contextRequests/updateContextExpressionPlugin1.json')
utils.readExampleFile('./test/unit/examples/contextRequests/updateContextExpressionPlugin1b.json')
)
.reply(
200,
Expand Down Expand Up @@ -236,7 +236,7 @@ describe('Javascript Expression Language (JEXL) based transformations plugin ',
.matchHeader('fiware-servicepath', 'gardens')
.post(
'/v1/updateContext',
utils.readExampleFile('./test/unit/examples/contextRequests/updateContextExpressionPlugin2.json')
utils.readExampleFile('./test/unit/examples/contextRequests/updateContextExpressionPlugin2b.json')
)
.reply(
200,
Expand Down Expand Up @@ -313,7 +313,7 @@ describe('Javascript Expression Language (JEXL) based transformations plugin ',
.matchHeader('fiware-servicepath', 'gardens')
.post(
'/v1/updateContext',
utils.readExampleFile('./test/unit/examples/contextRequests/updateContextExpressionPlugin3.json')
utils.readExampleFile('./test/unit/examples/contextRequests/updateContextExpressionPlugin3b.json')
)
.reply(
200,
Expand Down
214 changes: 0 additions & 214 deletions test/unit/ngsiv2/expressions/jexlBasedTransformations-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -385,41 +385,6 @@ describe('Java expression language (JEXL) based transformations plugin', functio
});
});

describe('When an update comes for attributes with string expression and type integer', function () {
// Case: Update for an integer attribute with string expression
const values = [
{
name: 'e',
type: 'Number',
value: 52
}
];

beforeEach(function () {
nock.cleanAll();

contextBrokerMock = nock('http://192.168.1.1:1026')
.matchHeader('fiware-service', 'smartgondor')
.matchHeader('fiware-servicepath', 'gardens')
.post(
'/v2/entities/ws1/attrs',
utils.readExampleFile(
'./test/unit/ngsiv2/examples/contextRequests/updateContextExpressionPlugin11.json'
)
)
.query({ type: 'WeatherStation' })
.reply(204);
});

it('should apply the expression before sending the values', function (done) {
iotAgentLib.update('ws1', 'WeatherStationMultiple', '', values, function (error) {
should.not.exist(error);
contextBrokerMock.done();
done();
});
});
});

describe('When an update comes for attributes without expressions and type float', function () {
// Case: Update for a Float attribute without expressions

Expand Down Expand Up @@ -492,42 +457,6 @@ describe('Java expression language (JEXL) based transformations plugin', functio
});
});

describe('When an update comes for attributes with string expressions and type float', function () {
// Case: Update for a Float attribute with string expression

const values = [
{
name: 'e',
type: 'Number',
value: 0.44
}
];

beforeEach(function () {
nock.cleanAll();

contextBrokerMock = nock('http://192.168.1.1:1026')
.matchHeader('fiware-service', 'smartgondor')
.matchHeader('fiware-servicepath', 'gardens')
.post(
'/v2/entities/ws1/attrs',
utils.readExampleFile(
'./test/unit/ngsiv2/examples/contextRequests/updateContextExpressionPlugin3.json'
)
)
.query({ type: 'WeatherStation' })
.reply(204);
});

it('should apply the expression before sending the values', function (done) {
iotAgentLib.update('ws1', 'WeatherStationMultiple', '', values, function (error) {
should.not.exist(error);
contextBrokerMock.done();
done();
});
});
});

describe('When an update comes for attributes without expressions and NULL type', function () {
// Case: Update for a Null attribute without expression

Expand Down Expand Up @@ -564,78 +493,6 @@ describe('Java expression language (JEXL) based transformations plugin', functio
});
});

describe('When an update comes for attributes with numeric expressions and NULL type', function () {
// Case: Update for a Null attribute with arithmetic expression

const values = [
{
name: 'a',
type: 'None',
value: null
}
];

beforeEach(function () {
nock.cleanAll();

contextBrokerMock = nock('http://192.168.1.1:1026')
.matchHeader('fiware-service', 'smartgondor')
.matchHeader('fiware-servicepath', 'gardens')
.post(
'/v2/entities/ws1/attrs',
utils.readExampleFile(
'./test/unit/ngsiv2/examples/contextRequests/updateContextExpressionPlugin5.json'
)
)
.query({ type: 'WeatherStation' })
.reply(204);
});

it('should apply the expression before sending the values', function (done) {
iotAgentLib.update('ws1', 'WeatherStation', '', values, function (error) {
should.not.exist(error);
contextBrokerMock.done();
done();
});
});
});

describe('When an update comes for attributes with string expressions and NULL type', function () {
// Case: Update for a Null attribute with string expression

const values = [
{
name: 'a',
type: 'None',
value: null
}
];

beforeEach(function () {
nock.cleanAll();

contextBrokerMock = nock('http://192.168.1.1:1026')
.matchHeader('fiware-service', 'smartgondor')
.matchHeader('fiware-servicepath', 'gardens')
.post(
'/v2/entities/ws1/attrs',
utils.readExampleFile(
'./test/unit/ngsiv2/examples/contextRequests/updateContextExpressionPlugin5.json'
)
)
.query({ type: 'WeatherStation' })
.reply(204);
});

it('should apply the expression before sending the values', function (done) {
iotAgentLib.update('ws1', 'WeatherStationMultiple', '', values, function (error) {
should.not.exist(error);
contextBrokerMock.done();
done();
});
});
});

describe('When an update comes for attributes without expressions and Boolean type', function () {
// Case: Update for a Boolean attribute without expression

Expand Down Expand Up @@ -672,77 +529,6 @@ describe('Java expression language (JEXL) based transformations plugin', functio
});
});

describe('When an update comes for attributes with numeric expressions and Boolean type', function () {
// Case: Update for a Boolean attribute with arithmetic expression

const values = [
{
name: 'u',
type: 'Boolean',
value: true
}
];

beforeEach(function () {
nock.cleanAll();

contextBrokerMock = nock('http://192.168.1.1:1026')
.matchHeader('fiware-service', 'smartgondor')
.matchHeader('fiware-servicepath', 'gardens')
.post(
'/v2/entities/ws1/attrs',
utils.readExampleFile(
'./test/unit/ngsiv2/examples/contextRequests/updateContextExpressionPlugin10.json'
)
)
.query({ type: 'WeatherStation' })
.reply(204);
});

it('should apply the expression before sending the values', function (done) {
iotAgentLib.update('ws1', 'WeatherStation', '', values, function (error) {
should.not.exist(error);
contextBrokerMock.done();
done();
});
});
});

describe('When an update comes for attributes with string expressions and Boolean type', function () {
// Case: Update for a Boolean attribute with string expression
const values = [
{
name: 'u',
type: 'Boolean',
value: true
}
];

beforeEach(function () {
nock.cleanAll();

contextBrokerMock = nock('http://192.168.1.1:1026')
.matchHeader('fiware-service', 'smartgondor')
.matchHeader('fiware-servicepath', 'gardens')
.post(
'/v2/entities/ws1/attrs',
utils.readExampleFile(
'./test/unit/ngsiv2/examples/contextRequests/updateContextExpressionPlugin9.json'
)
)
.query({ type: 'WeatherStation' })
.reply(204);
});

it('should apply the expression before sending the values', function (done) {
iotAgentLib.update('ws1', 'WeatherStationMultiple', '', values, function (error) {
should.not.exist(error);
contextBrokerMock.done();
done();
});
});
});

describe('When an update comes for attributes without expressions and Object type', function () {
// Case: Update for a JSON document attribute without expression
const values = [
Expand Down