Merge pull request #18 from rtyler/types-denormalization-14

Introduce the types table for types denormalization

Makefile

@@ -30,7 +30,9 @@ build: depends
 check: build depends migrate
 	# Running with docker-compose since our tests require a database to be
 	# present
-	$(COMPOSE) run --rm node \
+	$(COMPOSE) run --rm \
+		-e NODE_ENV=test \
+		node \
 		/usr/local/bin/node $(JEST) $(JEST_ARGS)
 
 clean:
@@ -60,8 +62,10 @@ migrate: depends
 watch: migrate
 	# Running with docker-compose since our tests require a database to be
 	# present
-	$(COMPOSE) run --rm node \
-		/usr/local/bin/node $(JEST) $(JEST_ARGS) --watch
+	$(COMPOSE) run --rm \
+		-e NODE_ENV=test \
+		node \
+		/usr/local/bin/node $(JEST) $(JEST_ARGS) --watch --coverage=false
 
 watch-compile:
 	$(TSC) -w

migrations/20181023174942-types-table.js

@@ -0,0 +1,33 @@
+'use strict';
+
+module.exports = {
+  up: (queryInterface, Sequelize) => {
+    return queryInterface.createTable('types', {
+      id: {
+        allowNull: false,
+        autoIncrement: true,
+        primaryKey: true,
+        type: Sequelize.INTEGER
+      },
+      type: {
+        unique: true,
+        allowNull: false,
+        type: Sequelize.STRING,
+      },
+      createdAt: {
+        allowNull: false,
+        defaultValue: Sequelize.literal('NOW()'),
+        type: Sequelize.DATE
+      },
+      updatedAt: {
+        allowNull: false,
+        defaultValue: Sequelize.literal('NOW()'),
+        type: Sequelize.DATE
+      }
+    });
+  },
+
+  down: (queryInterface, Sequelize) => {
+    return queryInterface.dropTable('types');
+  }
+};

package.json

@@ -73,7 +73,10 @@
     "transform": {
       "^.+\\.tsx?$": "ts-jest"
     },
-    "testRegex": "(/test/.*|(\\.|/)(test|spec))\\.(jsx?|tsx?)$",
+    "testMatch": [
+        "**/__tests__/**/*.ts?(x)",
+        "**/?(*.)+(spec|test).ts?(x)"
+    ],
     "moduleFileExtensions": [
       "ts",
       "tsx",

src/controllers/dashboard.ts

@@ -19,7 +19,7 @@ export default (app) => {
       const user = (req as any).user;
       const name : string = user.github.profile.username;
       const grants : Array<string> = await app.service('grants').find({ query: { name: name }});
-      const types : Array<string> = await app.service('types').find();
+      const types : Array<Object> = await app.service('types').find();
       const isAdmin : boolean = (grants.filter(g => g != '*').length > 0);
 
       app.service('events')

src/hooks/authorize.ts

@@ -4,16 +4,41 @@ import { SKIP } from '@feathersjs/feathers';
 
 import logger from '../logger';
 
-export default () => {
+export interface AuthorizeOptions {
+  allowInternal?: boolean,
+};
+
+export default (options : AuthorizeOptions = {}) => {
   return async context => {
+
+    /*
+     * Allow internal API calls to skip the entire authorization process
+     */
+    if ((options.allowInternal) &&
+        (!context.params.provider)) {
+      return SKIP;
+    }
+
+    if ((process.env.NODE_ENV == 'test') &&
+        (context.params.query.testing_access_token)) {
+      // Remove the property to make sure it's not used in the DB query
+      delete context.params.query.testing_access_token;
+      return SKIP;
+    }
+
     context = await authentication.hooks.authenticate(['jwt'])(context);
 
     if (context == SKIP) {
       return SKIP;
     }
 
+    if (!context.params.user) {
+      throw new Forbidden('No GitHub information, sorry');
+    }
+
     const name : string = context.params.user.github.profile.username;
     const type : string = context.params.query.type;
+
     return context.app.service('grants').find({
       query: {
         name: name,

src/models/type.ts

@@ -0,0 +1,10 @@
+'use strict';
+
+export default (sequelize, DataTypes) => {
+  const Type = sequelize.define('types', {
+    type: DataTypes.STRING,
+  }, {});
+  Type.associate = function(models) {
+  };
+  return Type;
+};

src/services/events.ts

@@ -40,7 +40,23 @@ export const eventsHooks : HooksObject = {
       authorize(),
     ],
   },
-  after: {},
+  after: {
+    create: [
+      (context) => {
+        return context.app.service('types')
+          .create({
+            type: context.data.type,
+          })
+        .then(() => { return context; })
+        .catch((err) => {
+          // hitting the UNIQUE constraint is an acceptable error
+          if (!err.errors.filter(e => e.type == 'unique violation')) {
+            throw err;
+          }
+        });
+      },
+    ],
+  },
   error: {},
 };
 

src/services/types.ts

@@ -3,28 +3,32 @@
  */
 
 import { Params, HooksObject } from '@feathersjs/feathers';
+import service from 'feathers-sequelize';
 
+import authorize from '../hooks/authorize';
+import applyGrant from '../hooks/apply-grant';
 import db from '../models';
-import Event from '../models/event';
+import Type from '../models/type';
 
 const typesHooks : HooksObject = {
-  before: {},
+  before: {
+    all: [
+      authorize({ allowInternal: true, }),
+      applyGrant(),
+    ],
+  },
   after: {},
   error: {},
 };
 
-export class TypesService {
-  async find(params : Params) : Promise<any> {
-    return db.sequelize.query('SELECT DISTINCT(type) FROM events', { type: db.sequelize.QueryTypes.SELECT }).then((types) => {
-      if (types.length > 0) {
-        return types.map(t => t.type);
-      }
-      return [];
-    });
-  }
-}
-
 export default (app) => {
-  app.use('/types', new TypesService);
+  const Model : any = Type(db.sequelize, db.sequelize.Sequelize);
+  const typesService = service({ Model: Model });
+
+  delete typesService.update;
+  delete typesService.remove;
+  delete typesService.patch;
+
+  app.use('/types', typesService);
   app.service('types').hooks(typesHooks);
 }

test/events.test.ts

@@ -31,20 +31,31 @@ describe('Acceptance tests for /events', () => {
     );
   });
 
-  it('POST /events should allow creating a valid event', () => {
-    return request(getUrl('/events'), {
-      method: 'POST',
-      json: true,
-      resolveWithFullResponse: true,
-      body: {
-        type: 'jest-example',
-        payload: {
-          generatedAt: Date.now(),
+  describe('POST to /events', () => {
+    it('should create a valid event', () => {
+      return request.post(getUrl('/events'), {
+        json: true,
+        resolveWithFullResponse: true,
+        body: {
+          type: 'jest-example',
+          payload: {
+            generatedAt: Date.now(),
+          },
+          correlator: '0xdeadbeef',
         },
-        correlator: '0xdeadbeef',
-      },
-    }).then(response =>
-      expect(response.statusCode).toEqual(201)
-    );
+      }).then((response) => {
+        expect(response.statusCode).toEqual(201)
+
+        // ensure that a type has been created in the types table
+        return request.get(getUrl('/types'), {
+          json: true,
+          resolveWithFullResponse: true,
+          qs: { testing_access_token: true },
+        }).then((response) => {
+          expect(response.statusCode).toEqual(200);
+          expect(response.body.length).toEqual(1);
+        });
+      });
+    });
   });
 });

test/hooks/authorize.test.ts

@@ -0,0 +1,55 @@
+import { SKIP } from '@feathersjs/feathers';
+import { Forbidden } from '@feathersjs/errors';
+
+import authorize from '../../src/hooks/authorize';
+
+describe('The `authorize` hook', () => {
+  let context = null;
+  let mockServices = {};
+  const mockApp = {
+    authenticate: () => {
+      return () => { return Promise.resolve({}); };
+    },
+    passport: {
+      _strategy: () => { return ['jwt'] },
+      options: () => { },
+    },
+    service: (name) => { return mockServices; },
+  };
+
+  beforeEach(() => {
+    context = {
+      type: 'before',
+      app: mockApp,
+      params: {
+        query: {},
+      },
+      data: {
+      },
+    };
+  })
+
+  describe('in testing mode', () => {
+    it('should not skip when a `testing_access_token` is omitted', () => {
+      return expect(authorize()(context)).rejects.toThrow(Forbidden);
+    });
+
+    it('should SKIP when a `testing_access_token` is provided', () => {
+      context.params.query.testing_access_token = true;
+      return expect(authorize()(context)).resolves.toEqual(SKIP);
+    });
+  });
+
+  describe('with the allowInternal option', () => {
+    it('should not skip for external API calls', () => {
+      context.params.provider = 'rest';
+      return expect(authorize({ allowInternal: true })(context))
+        .rejects.toThrow(Forbidden);
+    });
+
+    it('should SKIP for internal API calls', () => {
+      return expect(authorize({ allowInternal: true })(context))
+        .resolves.toEqual(SKIP);
+    });
+  });
+});

test/types.test.ts

@@ -1,12 +1,50 @@
-import { TypesService } from '../src/services/types';
+import url from 'url';
+import request from 'request-promise';
 
-describe('Unit tests for /types', () => {
-  describe('find', () => {
-    let service = new TypesService();
+import app from '../src/app';
+import types from '../src/service/types';
 
-    it('should return an Array', async () => {
-      const result = await service.find();
-      expect(result.length).toBeGreaterThan(0);
+// Offsetting a bit to ensure that we can watch and run at the same time
+const port = (app.get('port') || 3030) + 10;
+const getUrl = pathname => url.format({
+  hostname: app.get('host') || 'localhost',
+  protocol: 'http',
+  port,
+  pathname
+});
+
+describe('Acceptance tests for /types', () => {
+  beforeEach((done) => {
+    this.server = app.listen(port);
+    this.server.once('listening', () => done());
+  });
+  afterEach(done => this.server.close(done));
+
+  describe('with unauthenticated requests', () => {
+    it('responds to GET /types', () => {
+      return request(getUrl('/types'), {
+        json: true,
+        resolveWithFullResponse: true,
+      }).then(response =>
+        expect(response.statusCode).toEqual(401)
+      ).catch(err =>
+        expect(err.statusCode).toEqual(401)
+      );
+    });
+  });
+
+  describe('with authenticated requests', () => {
+    it('responds to GET /types with an Array of types', () => {
+      return request(getUrl('/types'), {
+        json: true,
+        resolveWithFullResponse: true,
+        qs: {
+          testing_access_token: true,
+        },
+      }).then((response) => {
+        expect(response.statusCode).toEqual(200);
+        expect(response.body).toBeInstanceOf(Array);
+      });
     });
   });
 });

views/dashboard.pug

@@ -45,7 +45,7 @@ html(lang="en")
               if types.length > 0
                 select(id='type-nav', name='types', onchange='navigateToType()')
                   each t in types
-                    option(value=t) #{t}
+                    option(value=t.type) #{t.type}
             th(scope='col').text-center Payload
             th(scope='col').text-center Actions
           each e in events.data