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

fix: [sc-105230] particle-api-js getEventStream does not honor setContext / X-Particle-Tool http header correctly #147

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
3 changes: 2 additions & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
module.exports = {
extends: ['eslint-config-particle'],
parserOptions: {
sourceType: 'module'
sourceType: 'module',
ecmaVersion: 8
},
env: {
browser: true,
Expand Down
2 changes: 0 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,6 @@ All essential commands are available at the root via `npm run <script name>` - e
<summary><b>How to run your tests</b></summary>
<p>

The `Agent` integration tests ([source](./test/Agent.integration.js)) depend on a real HTTP api backend and a valid Particle access token. Be sure to set relevant environment variables to avoid test failures. You can prefix commands test commands like this `PARTICLE_API_BASE_URL=<url> PARTICLE_API_TOKEN=<token> npm test`

Comment on lines -37 to -38
Copy link
Contributor

Choose a reason for hiding this comment

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

@joegoggins

Also, eliminates problem where PARTICLE_API_TOKEN had to be specified for many npm run commands by moving *.js files that require these to an e2e-tests that does NOT run in CI. This is not a step down in terms of quality; as these tests were not designed to be run in this CI context, but rather manually by developers working on the event stream.

fwiw, @deleonn updated CI over in #142 to ensure at least the node-side event e2e tests ran with their required perquisites - here's an example of them running and "passing":

https://app.circleci.com/pipelines/github/particle-iot/particle-api-js/53/workflows/cd0aa933-14fd-4836-a96b-024f13e31457/jobs/156?invite=true#step-103-42

further complicating things, the reference to "Agent integration tests" in these docs is incorrect - it's actually the test/EventStream-e2e-node.js file which is running in CI.

all that said, at this point i'd just delete all of those (e2e-tests directory and related) as they're likely to cause more confusion than help.

`npm test` runs the tests.

`npm run coverage` shows code coverage
Expand Down
206 changes: 103 additions & 103 deletions docs/api.md

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ End-to-end test program for the event stream with Node

Steps:
- npm run compile
- PARTICLE_API_TOKEN=<my-token> node test/EventStream-e2e-node.js
- PARTICLE_API_TOKEN=<my-token> node e2e-tests/EventStream-e2e-node.js
- Follow the scenarios in EventStream.feature

*/
Expand Down
File renamed without changes.
10 changes: 10 additions & 0 deletions e2e-tests/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
The tests in this directory do not run in CI.

They can be used to do deep validations on the behavior of the library against the actual Particle API

Follow the directions at in each script for how to use them.

Also, you'll need set valid values for env vars like PARTICLE_API_BASE_URL
and PARTICLE_API_TOKEN.

Note also that these e2e-tests are not actively maintained.
14 changes: 12 additions & 2 deletions src/EventStream.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,23 @@ import url from 'url';
import { EventEmitter } from 'events';

class EventStream extends EventEmitter {
constructor(uri, token) {
/**
*
* @param {String} uri Event stream URL
* @param {String} token Particle Access Token
* @param {Object} options
* @param {Particle} options.headers key/val http headers to pass along
* @hideconstructor
*/
constructor(uri, token, options={ headers:{} }) {
super();
this.uri = uri;
this.token = token;
this.reconnectInterval = 2000;
this.timeout = 13000; // keep alive can be sent up to 12 seconds after last event
this.data = '';
this.buf = '';
this.options = options;

this.parse = this.parse.bind(this);
this.end = this.end.bind(this);
Expand All @@ -32,7 +41,8 @@ class EventStream extends EventEmitter {
path: `${path}?access_token=${this.token}`,
method: 'get',
port: parseInt(port, 10) || (isSecure ? 443 : 80),
mode: 'prefer-streaming'
mode: 'prefer-streaming',
headers: this.options.headers
});

this.req = req;
Expand Down
20 changes: 19 additions & 1 deletion src/Particle.js
Original file line number Diff line number Diff line change
Expand Up @@ -892,7 +892,25 @@ class Particle {
}

auth = this._getActiveAuthToken(auth);
return new EventStream(`${this.baseUrl}${uri}`, auth).connect();
const headers = this._getDefaultHttpHeadersForContext();
return new EventStream(`${this.baseUrl}${uri}`, auth, { headers }).connect();
}

/**
* A reimplementation of Agent's _addToolContext() and _addProjectContext() methods
* that can be used by getEventStream() which doesn't use Agent for http interactions
* @private
* @returns {Object} key/value http headers
*/
_getDefaultHttpHeadersForContext() {
const returnThis = {};
if (this.context && this.context.tool) {
returnThis['X-Particle-Tool'] = `${this.context.tool.name}@${this.context.tool.version}`;
}
if (this.context && this.context.project) {
returnThis['X-Particle-Project'] = `${this.context.project.name}@${this.context.project.version}`;
}
return returnThis;
}

/**
Expand Down
9 changes: 6 additions & 3 deletions test/EventStream.spec.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { sinon, expect } from './test-setup';
import http from 'http';
import { EventEmitter } from 'events';

import EventStream from '../src/EventStream';

describe('EventStream', () => {
Expand Down Expand Up @@ -42,7 +41,10 @@ describe('EventStream', () => {
return fakeRequest;
});

const eventStream = new EventStream('http://hostname:8080/path', 'token');
const fakeToolName = 'fake';
const fakeToolVersion = '1.2.3';
const headers = { 'X-Particle-Tool': `${fakeToolName}@${fakeToolVersion}` };
const eventStream = new EventStream('http://hostname:8080/path', 'token', { headers });

return eventStream.connect().then(() => {
expect(http.request).to.have.been.calledWith({
Expand All @@ -51,7 +53,8 @@ describe('EventStream', () => {
path: '/path?access_token=token',
method: 'get',
port: 8080,
mode: 'prefer-streaming'
mode: 'prefer-streaming',
headers
});
});
});
Expand Down
40 changes: 39 additions & 1 deletion test/Particle.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -929,14 +929,52 @@ describe('ParticleAPI', () => {
describe('.getEventStream', () => {
before(() => {
sinon.stub(EventStream.prototype, 'connect').callsFake(function connect(){
return Promise.resolve({ uri: this.uri });
return Promise.resolve({ uri: this.uri, options: this.options });
});
});

after(() => {
sinon.restore();
});

describe('http headers', () => {
it('initializes options correctly with a blank headers object', async () => {
const { options } = await api.getEventStream({});
expect(options).to.be.an('object');
expect(options.headers).to.be.an('object');
});

it('determines http headers using ._getDefaultHttpHeadersForContext()', async () => {
const fakeHeaders = { 'X-Fake-Header': 'foo' };
sinon.stub(api, '_getDefaultHttpHeadersForContext').returns(fakeHeaders);
const { options } = await api.getEventStream({});
expect(options).to.be.an('object');
expect(options.headers).to.eql(fakeHeaders);
});

describe('._getDefaultHttpHeadersForContext() (a way to get http headers from context)', () => {

it('returns empty object when no context has been set', () => {
expect(api._getDefaultHttpHeadersForContext()).to.eql({});
});

it('includes X-Particle-Tool when this.context.tool is set', () => {
api.setContext('tool', { name: 'fake', version: '1.2.3' });
const r = api._getDefaultHttpHeadersForContext();
expect(r).to.be.an('object');
expect(r['X-Particle-Tool']).to.eql('fake@1.2.3');
});

it('includes X-Particle-Project when this.context.project is set', () => {
api.setContext('project', { name: 'fake', version: '1.2.3' });
const r = api._getDefaultHttpHeadersForContext();
expect(r).to.be.an('object');
expect(r['X-Particle-Project']).to.eql('fake@1.2.3');
});
});

});

it('requests public events', () => {
return api.getEventStream({ }).then(({ uri }) => {
uri.should.endWith('events');
Expand Down