Task/default attribute value null instead blank #946

AlvaroVega · 2020-11-26T07:48:15Z

(revert from a revert)
this branch contais PR changes from original branch task/default_attribute_value_null_instead_blank

Do not merge until passes:

travis/unit tests
E2E tests (sanity and full)

AlvaroVega · 2020-11-26T14:13:18Z

No faults where reported about iotas:

Failing scenarios:
tests/features/TS03_cbflows/001_cb2cep.feature:142 cb001005 Entity update in CB matches a basic attribute VALUE rule with action UPDATECB -- @1.1
tests/features/TS03_cbflows/001_cb2cep.feature:143 cb001005 Entity update in CB matches a basic attribute VALUE rule with action UPDATECB -- @1.2
tests/features/TS03_cbflows/001_cb2cep.feature:144 cb001005 Entity update in CB matches a basic attribute VALUE rule with action UPDATECB -- @1.3
tests/features/TS03_cbflows/001_cb2cep.feature:145 cb001005 Entity update in CB matches a basic attribute VALUE rule with action UPDATECB -- @1.4
tests/features/TS03_cbflows/001_cb2cep.feature:146 cb001005 Entity update in CB matches a basic attribute VALUE rule with action UPDATECB -- @1.5
tests/features/TS03_cbflows/001_cb2cep.feature:147 cb001005 Entity update in CB matches a basic attribute VALUE rule with action UPDATECB -- @1.6
tests/features/TS03_cbflows/005_cb2sth.feature:34 cb00501 Entities updated in CB should be stored in STH by raw data -- @1.2
tests/features/TS03_cbflows/005_cb2sth.feature:35 cb00501 Entities updated in CB should be stored in STH by raw data -- @1.3
tests/features/TS03_cbflows/005_cb2sth.feature:36 cb00501 Entities updated in CB should be stored in STH by raw data -- @1.4
tests/features/TS03_cbflows/005_cb2sth.feature:59 cb00502 Entities updated in CB should be stored in STH by aggregated -- @1.1
tests/features/TS03_cbflows/005_cb2sth.feature:60 cb00502 Entities updated in CB should be stored in STH by aggregated -- @1.2
tests/features/TS03_cbflows/005_cb2sth.feature:61 cb00502 Entities updated in CB should be stored in STH by aggregated -- @1.3
tests/features/TS04_pepflows/001_pep2cb.feature:178 pep00109 SC_9 standard entity delete -- @1.1

6 features passed, 3 failed, 62 skipped
51 scenarios passed, 13 failed, 970 skipped
530 steps passed, 13 failed, 14195 skipped, 0 undefined
Took 77m45.746s

…te_value_null_instead_blank

fgalan

LGTM

But let's wait a bit before merging, in order not adding more entropy to master until we have reports from CI e2e testing tonight after the merging of the big bunch of code in PR #849

fgalan · 2020-11-26T16:13:07Z

From travis:

/home/travis/build/telefonicaid/iotagent-node-lib/lib/constants.js
  92:5  error  Duplicate key 'ATTRIBUTE_DEFAULT'  no-dupe-keys
  93:5  error  Duplicate key 'DATETIME_DEFAULT'   no-dupe-keys

…te_value_null_instead_blank

fgalan · 2020-12-02T12:08:08Z

It seems that some test related with NGSI-LD are failing:

  891 passing (30s)
  14 pending
  5 failing

  1) NGSI-LD - Secured access to the Context Broker with OAuth2 provider
       When subscriptions are used on a protected Context Broker
         subscribe requests use auth header:
     TypeError: Cannot read property 'service' of undefined
      at Object.subscribeNgsiLD [as subscribe] (lib/services/ngsi/subscription-NGSI-LD.js:41:175)
      at Function.subscribe (lib/services/ngsi/subscriptionService.js:36:126)
      at Domain.expandedFunction (lib/services/common/domain.js:42:323)
      at Domain.run (domain.js:349:14)
      at ensureSouthboundTransaction (lib/services/common/domain.js:35:1580)
      at Object.subscribe (lib/services/common/domain.js:42:383)
      at /home/runner/work/iotagent-node-lib/iotagent-node-lib/test/unit/ngsi-ld/general/contextBrokerOAuthSecurityAccess-test.js:321:29
      at Object.getDevice (lib/services/devices/deviceRegistryMemory.js:46:1893)
      at getDevice (lib/services/devices/deviceService.js:102:133)
      at Object.getDevice (lib/services/devices/deviceService.js:128:369)
      at Context.<anonymous> (test/unit/ngsi-ld/general/contextBrokerOAuthSecurityAccess-test.js:320:25)
      at Request._callback (test/unit/ngsi-ld/general/contextBrokerOAuthSecurityAccess-test.js:313:25)
      at Request.self.callback (node_modules/request/request.js:185:22)
      at Request.EventEmitter.emit (domain.js:466:23)
      at Request.<anonymous> (node_modules/request/request.js:1161:10)
      at Request.EventEmitter.emit (domain.js:466:23)
      at IncomingMessage.<anonymous> (node_modules/request/request.js:1083:12)
      at IncomingMessage.EventEmitter.emit (domain.js:466:23)
      at endReadableNT (_stream_readable.js:1145:12)
      at process._tickCallback (internal/process/next_tick.js:63:19)

  2) NGSI-LD - Secured access to the Context Broker with OAuth2 provider
       When subscriptions are used on a protected Context Broker
         unsubscribe requests use auth header:
     TypeError: Cannot read property 'service' of undefined
      at Object.subscribeNgsiLD [as subscribe] (lib/services/ngsi/subscription-NGSI-LD.js:41:175)
      at Function.subscribe (lib/services/ngsi/subscriptionService.js:36:126)
      at Domain.expandedFunction (lib/services/common/domain.js:42:323)
      at Domain.run (domain.js:349:14)
      at ensureSouthboundTransaction (lib/services/common/domain.js:35:1580)
      at Object.subscribe (lib/services/common/domain.js:42:383)
      at /home/runner/work/iotagent-node-lib/iotagent-node-lib/test/unit/ngsi-ld/general/contextBrokerOAuthSecurityAccess-test.js:342:29
      at Object.getDevice (lib/services/devices/deviceRegistryMemory.js:46:1893)
      at getDevice (lib/services/devices/deviceService.js:102:133)
      at Object.getDevice (lib/services/devices/deviceService.js:128:369)
      at Context.<anonymous> (test/unit/ngsi-ld/general/contextBrokerOAuthSecurityAccess-test.js:341:25)
      at Request._callback (test/unit/ngsi-ld/general/contextBrokerOAuthSecurityAccess-test.js:313:25)
      at Request.self.callback (node_modules/request/request.js:185:22)
      at Request.EventEmitter.emit (domain.js:466:23)
      at Request.<anonymous> (node_modules/request/request.js:1161:10)
      at Request.EventEmitter.emit (domain.js:466:23)
      at IncomingMessage.<anonymous> (node_modules/request/request.js:1083:12)
      at IncomingMessage.EventEmitter.emit (domain.js:466:23)
      at endReadableNT (_stream_readable.js:1145:12)
      at process._tickCallback (internal/process/next_tick.js:63:19)

  3) NGSI-LD - Secured access to the Context Broker with OAuth2 provider (FIWARE Keyrock IDM)configured through group provisioning
       When a device is provisioned for a configuration contains an OAuth2 trust token
         should not raise any error:
     Error: Timeout of 5000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/home/runner/work/iotagent-node-lib/iotagent-node-lib/test/unit/ngsi-ld/general/contextBrokerOAuthSecurityAccess-test.js)
  

  4) NGSI-LD - Provisioning API: Single service mode
       When a device is provisioned for a configuration
         should not raise any error:

      Uncaught AssertionError: expected 500 to be 201
      + expected - actual

      -500
      +201
      
      at Assertion.fail (node_modules/should/cjs/should.js:275:17)
      at Assertion.value (node_modules/should/cjs/should.js:356:19)
      at Request._callback (test/unit/ngsi-ld/provisioning/singleConfigurationMode-test.js:300:44)
      at Request.self.callback (node_modules/request/request.js:185:22)
      at Request.EventEmitter.emit (domain.js:466:23)
      at Request.<anonymous> (node_modules/request/request.js:1161:10)
      at Request.EventEmitter.emit (domain.js:466:23)
      at IncomingMessage.<anonymous> (node_modules/request/request.js:1083:12)
      at IncomingMessage.EventEmitter.emit (domain.js:466:23)
      at endReadableNT (_stream_readable.js:1145:12)
      at process._tickCallback (internal/process/next_tick.js:63:19)

  5) NGSI-LD - Provisioning API: Single service mode
       When a device is provisioned for a configuration
         should send the mixed data to the Context Broker:
     Error: Timeout of 5000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/home/runner/work/iotagent-node-lib/iotagent-node-lib/test/unit/ngsi-ld/provisioning/singleConfigurationMode-test.js)

@jason-fox could you have a look an provide feedback on how to fix them, please?

jason-fox · 2020-12-02T13:45:12Z

From the latest ETSI specification:

5.2 Data Types

5.2.1 Introduction

Implementations shall support the data types defined by the clauses below. For each member defined by each data
type (including nested ones) a term shall be added to the Core @context, as mandated by clause 4.5.
None of the members described admit a null value, except when they are used in the context of an update operation
(see clause 5.5.8) and implementations shall raise an error of type BadRequestData if a null value is encountered.

The tests in question are failing with 500 BadRequestData because the input payload to the context broker is invalid.

fgalan · 2020-12-02T14:52:22Z

So maybe the problem is this kind of changes in test/unit/ngs-ld payload files?

What should be used in that case instead of null? In other words, what is the way of NGSI-LD of representing null-ity?

jason-fox · 2020-12-02T16:02:04Z

When sending an undefined value, you need to insert a JSON object which is interpreted as null rather than a null directly. This is defined in entities-NGSI-LD.js here

const NGSI_LD_NULL = { '@type': 'Intangible', '@value': null };

Where Intangible can be defined in the supplied @context as https://schema.org/Intangible if required (it defaults to core of course). This was discussed by @vraybaud in the review of the NGSI-LD code. #849

test/unit/ngsi-ld/examples/contextRequests/createMinimumProvisionedDevice.json

…ionedDevice.json Co-authored-by: Fermín Galán Márquez <[email protected]>

…te_value_null_instead_blank

AlvaroVega · 2020-12-14T10:44:19Z

IMHO current fails #946 are not related with #946 or at least changed proposed by #946 is not working

fgalan · 2020-12-14T12:37:29Z

At 54ec0b9, using "value": { "@type": "Intangible", "@value": null } test results show:

  874 passing (50s)
  14 pending
  20 failing

At 2db114b, using "value": null test results show:

  890 passing (27s)
  14 pending
  6 failing

So it seems that using the "NGSI-LD null way" test restuls are worse than with just null...

Definitely, we need help from NGSI-LD experts to fix this. @jason-fox could you provide feedback on how to fix this (or even better: a PR with the fix itself using revert-945-revert-942-task/default_attribute_value_null_instead_blank as base branch), please?

…ault_attribute_value_null_instead_blank Fixing requests

jason-fox · 2020-12-14T16:55:38Z

The unexpected side-effect from this change is a change in default truthy-ness of undefined boolean values.

alert(Boolean(true));  // returns true
alert(Boolean(false));  // returns false
alert(Boolean(null)); // returns false
alert(Boolean(' '));  // true

Previously if I had the following

{
  "id": "light1",
  "type": "Light",
  "state": {
    "type": "Boolean",
    "value": " "
  }
}

This is implicitly true

Now I have the following:

{
  "id": "light1",
  "type": "Light",
  "state": {
    "type": "Boolean",
    "value": null
  }
}

But for NGSI-LD as explained, you can't have a value:null and you can only have type=Property or type=Relationship, therefore the provisioned Boolean type must be cast properly before being sent to the context broker. Therefore changing the default value from ' ' to null has changed the calculated boolean values from true to false.

fgalan · 2020-12-16T08:22:50Z

Thanks for the feedback @jason-fox

We think that side-effect would not have any impact on the existing e2e usages of the issue. We are going to merge this PR and we will confirm it after the CI e2e after some hours.

fgalan

LGTM

Revert "Revert "Task/default attribute value null instead blank""

66580ae

AlvaroVega changed the title ~~[WIP] Task/default attribute value null instead blank~~ Task/default attribute value null instead blank Nov 26, 2020

Merge branch 'master' into revert-945-revert-942-task/default_attribu…

4dad2e8

…te_value_null_instead_blank

AlvaroVega requested a review from fgalan November 26, 2020 15:11

fgalan approved these changes Nov 26, 2020

View reviewed changes

AlvaroVega mentioned this pull request Nov 30, 2020

IOTA creates attribute with blank value when there is a mapping #938

Closed

AlvaroVega added 4 commits December 2, 2020 08:51

Update constants.js

ddf0428

Update constants.js

f044f3f

update examples

2de34bc

Merge branch 'master' into revert-945-revert-942-task/default_attribu…

aab0258

…te_value_null_instead_blank

fgalan reviewed Dec 3, 2020

View reviewed changes

test/unit/ngsi-ld/examples/contextRequests/createMinimumProvisionedDevice.json Show resolved Hide resolved

AlvaroVega and others added 3 commits December 14, 2020 10:45

Update test/unit/ngsi-ld/examples/contextRequests/createMinimumProvis…

54ec0b9

…ionedDevice.json Co-authored-by: Fermín Galán Márquez <[email protected]>

fix null value

c399225

Merge branch 'master' into revert-945-revert-942-task/default_attribu…

2db114b

…te_value_null_instead_blank

Fixing requests

4da1d61

jason-fox mentioned this pull request Dec 14, 2020

Fixing requests #964

Merged

Merge pull request #964 from jason-fox/revert-945-revert-942-task/def…

cd40d08

…ault_attribute_value_null_instead_blank Fixing requests

fgalan approved these changes Dec 16, 2020

View reviewed changes

fgalan merged commit c07889b into master Dec 16, 2020

fgalan deleted the revert-945-revert-942-task/default_attribute_value_null_instead_blank branch December 16, 2020 08:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Task/default attribute value null instead blank #946

Task/default attribute value null instead blank #946

Uh oh!

AlvaroVega commented Nov 26, 2020 •

edited

Loading

Uh oh!

AlvaroVega commented Nov 26, 2020

Uh oh!

fgalan left a comment

Uh oh!

fgalan commented Nov 26, 2020

Uh oh!

fgalan commented Dec 2, 2020

Uh oh!

jason-fox commented Dec 2, 2020 •

edited

Loading

5.2 Data Types

5.2.1 Introduction

Uh oh!

fgalan commented Dec 2, 2020 •

edited

Loading

Uh oh!

jason-fox commented Dec 2, 2020

Uh oh!

Uh oh!

AlvaroVega commented Dec 14, 2020

Uh oh!

fgalan commented Dec 14, 2020 •

edited

Loading

Uh oh!

jason-fox commented Dec 14, 2020

Uh oh!

fgalan commented Dec 16, 2020

Uh oh!

fgalan left a comment

Uh oh!

Uh oh!

Task/default attribute value null instead blank #946

Task/default attribute value null instead blank #946

Uh oh!

Conversation

AlvaroVega commented Nov 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AlvaroVega commented Nov 26, 2020

Uh oh!

fgalan left a comment

Choose a reason for hiding this comment

Uh oh!

fgalan commented Nov 26, 2020

Uh oh!

fgalan commented Dec 2, 2020

Uh oh!

jason-fox commented Dec 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

5.2 Data Types

5.2.1 Introduction

Uh oh!

fgalan commented Dec 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jason-fox commented Dec 2, 2020

Uh oh!

Uh oh!

AlvaroVega commented Dec 14, 2020

Uh oh!

fgalan commented Dec 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jason-fox commented Dec 14, 2020

Uh oh!

fgalan commented Dec 16, 2020

Uh oh!

fgalan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

AlvaroVega commented Nov 26, 2020 •

edited

Loading

jason-fox commented Dec 2, 2020 •

edited

Loading

fgalan commented Dec 2, 2020 •

edited

Loading

fgalan commented Dec 14, 2020 •

edited

Loading