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

Jump to bottom

Add SeAsSecurityMessage interface and unit tests #633

Merged

anthonyvercolano merged 5 commits into Azure:master from kfbehar:SetAsSecurityMessage

Sep 24, 2019

Contributor

kfbehar commented Sep 23, 2019 •

edited

Loading

Checklist

I have read the [contribution guidelines] (https://github.com/Azure/azure-iot-sdk-node/blob/master/.github/CONTRIBUTING.md).
I added or modified the existing tests to cover the change (we do not allow our test coverage to go down).
If this is a modification that impacts the behavior of a public API
- I edited the corresponding document in the devdoc folder and added or modified requirements.

Reference/Link to the issue solved with this PR (if any)

Description of the problem

Description of the solution


          Add SeAsSecurityMessage interface and unit tests

732e146

Contributor

anthonyvercolano commented Sep 23, 2019 •

edited

Loading

/azp run node-canary
#Closed

azure-pipelines bot commented Sep 23, 2019 •

edited by anthonyvercolano

Loading

Azure Pipelines successfully started running 1 pipeline(s). #Closed


          fix linter error

04bc719

Contributor Author

kfbehar commented Sep 23, 2019 •

edited by anthonyvercolano

Loading

/azp run node-canary #Closed

azure-pipelines bot commented Sep 23, 2019 •

edited by anthonyvercolano

Loading

Commenter does not have sufficient privileges for PR 633 in repo Azure/azure-iot-sdk-node #Closed

Contributor

anthonyvercolano commented Sep 23, 2019 •

edited

Loading

/azp run node-canary
#Closed

azure-pipelines bot commented Sep 23, 2019 •

edited by anthonyvercolano

Loading

Azure Pipelines successfully started running 1 pipeline(s). #Closed

Contributor Author

kfbehar commented Sep 23, 2019 •

edited by anthonyvercolano

Loading

/azp run node-canary #Closed

azure-pipelines bot commented Sep 23, 2019 •

edited by anthonyvercolano

Loading

Azure Pipelines successfully started running 1 pipeline(s). #Closed

Contributor

anthonyvercolano commented Sep 23, 2019 •

edited

Loading

/azp run horton-node-gate
#Closed

anthonyvercolano reviewed

View reviewed changes

common/transport/amqp/test/_amqp_test.js Outdated Show resolved Hide resolved

anthonyvercolano reviewed

View reviewed changes

device/transport/http/test/_http_test.js Outdated Show resolved Hide resolved

anthonyvercolano reviewed

View reviewed changes

device/transport/mqtt/src/mqtt.ts

                   if (message.contentType) systemProperties['$.ct'] = <string>message.contentType;
                   /*Codes_SRS_NODE_DEVICE_MQTT_16_083: [The `sendEvent` method shall serialize the `contentEncoding` property of the message as a key-value pair on the topic with the key `$.ce`.]*/
                   if (message.contentEncoding) systemProperties['$.ce'] = <string>message.contentEncoding;
+                  if (message.interfaceId) systemProperties['$.ifid'] = message.interfaceId;

Contributor

anthonyvercolano Sep 23, 2019 •

edited

Loading

['$.ifid'] [](start = 45, length = 10)

Does this clash with digitaltwins? #ByDesign

Contributor

anthonyvercolano Sep 23, 2019

There is a property sent in digitaltwins registration (which uses the telemetry path) that uses this as a property name.

In reply to: 327269387 [](ancestors = 327269387)

Contributor Author

kfbehar Sep 23, 2019 •

edited by anthonyvercolano

Loading

I was not aware of that, but we are already using that property in other SDKs, so I think it should be fine.
I think it should only be a problem if they use the same property AND the same value, only then is the message routed as a security message. #ByDesign


          fix quotes

1cd4120

anthonyvercolano approved these changes

View reviewed changes

Contributor

anthonyvercolano left a comment

azure-pipelines bot commented Sep 24, 2019 •

edited by anthonyvercolano

Loading

No pipelines are associated with this pull request. #Closed

4 similar comments

azure-pipelines bot commented Sep 24, 2019 •

edited by anthonyvercolano

Loading

No pipelines are associated with this pull request. #Closed

azure-pipelines bot commented Sep 24, 2019 •

edited by anthonyvercolano

Loading

No pipelines are associated with this pull request. #Closed

azure-pipelines bot commented Sep 24, 2019 •

edited by anthonyvercolano

Loading

No pipelines are associated with this pull request. #Closed

azure-pipelines bot commented Sep 24, 2019 •

edited by anthonyvercolano

Loading

No pipelines are associated with this pull request. #Closed

anthonyvercolano and others added 2 commits

September 24, 2019 12:11


          Merge branch 'master' into SetAsSecurityMessage

140c909


          Merge branch 'master' into SetAsSecurityMessage

782b167

anthonyvercolano merged commit 70b60fa into Azure:master

anthonyvercolano mentioned this pull request

Bump package versions for release on 2019-09-25 #637

Merged

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

Labels

None yet