Merge pull request #13 from ngfelixl/validate-data

feat: data validation, add missing tests

Author: ngfelixl

package.json

@@ -1,6 +1,6 @@
 {
   "name": "nodeplotlib",
-  "version": "0.5.2",
+  "version": "0.6.0",
   "description": "NodeJS frontend-less plotting lib using plotly.js inspired by matplotlib",
   "main": "dist/lib/index.js",
   "types": "dist/lib/index.d.ts",
@@ -19,7 +19,7 @@
   "scripts": {
     "build": "npm run clean && webpack && npm run build-copy-files",
     "clean": "shx rm -rf dist",
-    "test": "jest --config jest.config.json --coverage",
+    "test": "jest --config jest.config.json --coverage --maxWorkers=15",
     "format": "prettier --write \"src/**/*.ts\" \"src/**/*.js\"",
     "lint": "tslint --project ./tsconfig.json",
     "coverage": "cat coverage/lcov.info | coveralls",

src/index.ts

@@ -1,2 +1,6 @@
 export { plot, clear, stack } from './plot';
 export { Plot, Layout } from './models/index';
+
+import { plot } from './plot';
+
+plot([{x: [1, 2], y: [1, 2]}]);

src/plot.ts

@@ -22,6 +22,8 @@ export function clear(): void {
  * @param layout
  */
 export function stack(data: Plot[], layout?: Layout): void {
+  validate(data, layout);
+
   const container: IPlot = layout ? { data, layout } : { data };
   plots.push(container);
 }
@@ -34,8 +36,10 @@ export function stack(data: Plot[], layout?: Layout): void {
  */
 export function plot(data?: Plot[] | null, layout?: Layout): void {
   if (data) {
+    validate(data, layout);
     stack(data, layout);
   }
+
   const id = Object.keys(plotContainer).length;
 
   plotContainer[id] = {
@@ -47,3 +51,15 @@ export function plot(data?: Plot[] | null, layout?: Layout): void {
 
   server.spawn(plotContainer);
 }
+
+function validate(data: Plot[], layout?: Layout) {
+  if (!(data instanceof Array) || data.length === 0) {
+    throw new TypeError('Plot data must be an array with at least 1 element');
+  }
+
+  if (layout) {
+    if (!(layout instanceof Object)) {
+      throw new TypeError('Layout must be an object');
+    }
+  }
+}

src/server.ts

@@ -1,5 +1,6 @@
 import { readFile } from 'fs';
 import { createServer, IncomingMessage, Server as HttpServer, ServerResponse } from 'http';
+import { Socket } from 'net';
 import opn from 'opn';
 import { join } from 'path';
 import { IPlotsContainer } from './models';
@@ -8,10 +9,21 @@ export class Server {
   private instance: HttpServer;
   private plotsContainer: IPlotsContainer = {};
   private port: number;
+  private sockets: {[id: number]: Socket} = {};
+  private nextSocketID = 0;
 
   constructor(port: number) {
     this.port = port;
     this.instance = this.createServer();
+
+    this.instance.on('connection', (socket: Socket) => {
+      const id = this.nextSocketID++;
+      this.sockets[id] = socket;
+
+      socket.on('close', () => {
+        delete this.sockets[id];
+      });
+    });
   }
 
   /**
@@ -30,12 +42,18 @@ export class Server {
   }
 
   /**
-   * Closes the webserver and clears the plots container.
+   * Closes the webserver, destroys all connected sockets
+   * and clears the plots container.
    */
   public clean() {
     if (this.instance.address()) {
       this.instance.close();
     }
+
+    for (const socket of Object.values(this.sockets)) {
+      socket.destroy();
+    }
+
     this.plotsContainer = {};
   }
 
@@ -86,7 +104,7 @@ export class Server {
   }
 
   /**
-   * 
+   * Serves the website at http://localhost:PORT/plots/:id/index.html
    * @param req 
    * @param res 
    */
@@ -141,7 +159,7 @@ export class Server {
       .reduce((a, b) => a && b.opened, true);
 
     if (this.instance && !pending && opened) {
-      this.instance.close();
+      this.clean();
     }
   }
 }

test/clear.spec.ts

@@ -1,20 +1,20 @@
-import * as lib from '../src/plot';
+import { clear, plots } from '../src/plot';
 
 describe('clear', () => {
   beforeEach(() => {
-    lib.plots.push('fdsa' as any);
+    plots.push('test' as any);
   });
 
   it('should clear the plots array', () => {
-    lib.clear();
+    clear();
 
-    expect(lib.plots).toEqual([]);
+    expect(plots).toEqual([]);
   });
 
   it('should be clearable multiple times', () => {
-    lib.clear();
-    lib.clear();
+    clear();
+    clear();
 
-    expect(lib.plots).toEqual([]);
+    expect(plots).toEqual([]);
   });
 });

test/plot.spec.ts

@@ -1,4 +1,5 @@
-import { plot } from '../src/index';
+import { plot, Plot } from '../src/index';
+import { plots, stack } from '../src/plot';
 // import { Server } from '../src/server';
 jest.mock('../src/server');
 
@@ -7,11 +8,29 @@ describe('plot', () => {
     jest.resetAllMocks();
   });
 
-  it('should call "spawn" once', () => {
-    plot();
+  it('should throw an error if array length is 0', () => {
+    expect(() => { plot([]) })
+        .toThrow(new RegExp('Plot data must be an array with at least 1 element'))
+  });
+
+  it('should spawn the server if data is valid', () => {
+    plot([{x: [1], y: [1], type: 'line' as any}]);
   });
 
-  it('should stack data and call "spawn" once when using with data', () => {
+  /* it('should stack data and call "spawn" once when using with data', () => {
     plot([], {});
+  }); */
+
+  it('should clear the temporary plots array', () => {
+    stack([{x: [1], y: [2], type: 'line' as any}]);
+    expect(plots.length).toBe(1);
+
+    plot();
+    expect(plots.length).toBe(0);
+  });
+
+  it('should throw an error if layout is not an object', () => {
+    expect(() => { plot([{x: [1], y: [1], type: 'line' as any}], 'test' as any) })
+        .toThrow(new RegExp('Layout must be an object'));
   });
 });

test/server.spec.ts

@@ -1,15 +1,27 @@
 import opn from 'opn';
 import request from 'request';
 import { Server } from '../src/server';
+
+const port = 8080;
+const validData = {
+  opened: false,
+  pending: false,
+  plots: [{data: [{ x: [1], y: [2]}]}]
+};
+
 jest.mock('opn');
+jest.mock('fs', () => ({readFile: (path: any, options: any, callback: (err: any, data: any) => void) => {
+  callback('Error', null);
+}}));
 
 describe('Server', () => {
   let server: any;
 
   beforeEach(() => {
-    server = new Server(8080);
+    server = new Server(port);
   });
 
+
   it('should instantiate', () => {
     expect(server).toBeTruthy();
   });
@@ -25,105 +37,75 @@ describe('Server', () => {
   });
 
   it('should serve the data', (done) => {
-    server.spawn({0: {
-      opened: false,
-      pending: false,
-      plots: [{data: [{ x: [1], y: [2]}]}]
-    }});
+    server.spawn({0: validData});
 
-    request(`http://localhost:8080/data/0`, (err, response, body) => {
+    request(`http://localhost:${port}/data/0`, (err, response, body) => {
       expect(JSON.parse(body)).toEqual([{data: [{ x: [1], y: [2]}]}]);
       done();
     });
   });
 
   it('should spawn two times but listen just once', (done) => {
-    const data = {0: {
-      opened: false,
-      pending: false,
-      plots: [{data: [{ x: [1], y: [2]}]}]
-    }};
+    const data = {0: validData};
 
     server.spawn(data);
     server.spawn(data);
 
-    request(`http://localhost:8080/data/0`, (err, response, body) => {
+    request(`http://localhost:${port}/data/0`, (err, response, body) => {
       expect(JSON.parse(body)).toEqual([{data: [{ x: [1], y: [2]}]}]);
       done();
     });
   });
 
   it('should serve the website and return 404 if html file not found', (done) => {
-    server.spawn({0: {
-      opened: false,
-      pending: false,
-      plots: [{data: [{ x: [1], y: [2]}]}]
-    }});
+    server.spawn({0: validData});
 
-    request(`http://localhost:8080/plots/0/index.html`, (err, response, body) => {
+    request(`http://localhost:${port}/plots/0/index.html`, (err, response, body) => {
       expect(response.statusCode).toBe(404);
       done();
     });
   });
 
   it('should serve the nodeplotlib script and return 404 if file not found', (done) => {
-    server.spawn({0: {
-      opened: false,
-      pending: false,
-      plots: [{data: [{ x: [1], y: [2]}]}]
-    }});
+    server.spawn({0: validData});
 
-    request(`http://localhost:8080/plots/0/nodeplotlib.min.js`, (err, response, body) => {
+    request(`http://localhost:${port}/plots/0/nodeplotlib.min.js`, (err, response, body) => {
       expect(response.statusCode).toBe(404);
       done();
     });
   });
 
   it('should serve the plotly.min.js script and return 404 if file not found', (done) => {
-    server.spawn({0: {
-      opened: false,
-      pending: false,
-      plots: [{data: [{ x: [1], y: [2]}]}]
-    }});
+    server.spawn({0: validData});
 
-    request(`http://localhost:8080/plots/0/plotly.min.js`, (err, response, body) => {
+    request(`http://localhost:${port}/plots/0/plotly.min.js`, (err, response, body) => {
       expect(response.statusCode).toBe(404);
       done();
     });
   });
 
   it('should not close the webserver, if one plot hasn\'t got its data', (done) => {
-    server.spawn({0: {
-      opened: false,
-      pending: false,
-      plots: [{data: [{ x: [1], y: [2]}]}]
-    },
-    1: {
-      opened: false,
-      pending: false,
-      plots: [{data: [{ x: [1], y: [3]}]}]
-    }});
+    server.spawn({
+      0: { pending: false, opened: false, plots: [{ data: [{x: [1], y: [2]}] }]},
+      1: { pending: false, opened: false, plots: [{ data: [{x: [1], y: [3]}] }]}
+    });
 
-    request(`http://localhost:8080/data/0`, (err, response, body) => {
+    request(`http://localhost:${port}/data/0`, (err, response, body) => {
       expect(JSON.parse(body)).toEqual([{data: [{ x: [1], y: [2]}]}]);
 
-      request(`http://localhost:8080/data/1`, (err1, response1, body1) => {
+      request(`http://localhost:${port}/data/1`, (err1, response1, body1) => {
         expect(JSON.parse(body1)).toEqual([{data: [{ x: [1], y: [3]}]}]);
         done();
       });
     });
   });
 
   it('should return 404 if routes not matching', (done) => {
-    const data = {0: {
-      opened: false,
-      pending: false,
-      plots: [{data: [{ x: [1], y: [2]}]}]
-    }};
+    const data = {0: validData};
 
     server.spawn(data);
 
-    request(`http://localhost:8080/fdsaffds`, (err, response, body) => {
+    request(`http://localhost:${port}/fdsaffds`, (err, response, body) => {
       expect(response.statusCode).toBe(404);
       expect(response.body).toBe('Server address not found');
       done();
@@ -134,4 +116,9 @@ describe('Server', () => {
     server.clean();
     server = null;
   });
-});
+
+  // afterAll(() => {
+  //   console.log((process as any)._getActiveRequests());
+  //   console.log((process as any)._getActiveHandles()[0]);
+  // });
+});