webhook events should be presented as a list of strings

packages/server/modules/webhooks/graph/schemas/webhooks.graphql

@@ -38,7 +38,7 @@ type Webhook {
   streamId: String!
   url: String!
   description: String
-  events: JSONObject!
+  events: [String]!
   enabled: Boolean
   history(limit: Int! = 25): WebhookEventCollection
 }
@@ -47,7 +47,7 @@ input WebhookCreateInput {
   streamId: String!
   url: String!
   description: String
-  events: JSONObject!
+  events: [String]!
   secret: String
   enabled: Boolean
 }
@@ -58,7 +58,7 @@ input WebhookUpdateInput {
   description: String
   secret: String
   enabled: Boolean
-  events: JSONObject
+  events: [String]
 }
 
 type WebhookEventCollection{

packages/server/modules/webhooks/services/webhooks.js

@@ -7,10 +7,17 @@ const crs = require( 'crypto-random-string' )
 const WebhooksConfig = ( ) => knex( 'webhooks_config' )
 const WebhooksEvents = ( ) => knex( 'webhooks_events' )
 
+let MAX_STREAM_WEBHOOKS = 100
+
 module.exports = {
 
   async createWebhook( { streamId, url, description, secret, enabled, events } ) {
-    // TODO: limit max number of webhooks for a stream to 100 (github has a 20 limit per event)
+    let streamWebhookCount = await module.exports.getStreamWebhooksCount( { streamId } )
+    if ( streamWebhookCount >= MAX_STREAM_WEBHOOKS ) {
+      throw new Error( `Maximum number of webhooks for a stream reached (${MAX_STREAM_WEBHOOKS})` )
+    }
+
+    let eventsObj = Object.assign( {}, ...events.map( ( x ) => ( { [x]: true } ) ) )
 
     let [ id ] = await WebhooksConfig( ).returning( 'id' ).insert( {
       id: crs( { length: 10 } ),
@@ -19,14 +26,17 @@ module.exports = {
       description,
       secret,
       enabled,
-      events: events
+      events: eventsObj
     } )
     return id
   },
 
   async getWebhook( { id } ) {
-    // TODO: get webhook object + summary of event history (last event status, etc)
-    return await WebhooksConfig().select( '*' ).where( { id } ).first()
+    let webhook = await WebhooksConfig().select( '*' ).where( { id } ).first()
+    if ( webhook ) {
+      webhook.events = Object.keys( webhook.events )
+    }
+    return webhook
   },
 
   async updateWebhook( { id, url, description, secret, enabled, events } ) {
@@ -35,7 +45,10 @@ module.exports = {
     if ( description !== undefined ) fieldsToUpdate.description = description
     if ( secret !== undefined ) fieldsToUpdate.secret = secret
     if ( enabled !== undefined ) fieldsToUpdate.enabled = enabled
-    if ( events !== undefined ) fieldsToUpdate.events = events
+    if ( events !== undefined ) {
+      let eventsObj = Object.assign( {}, ...events.map( ( x ) => ( { [x]: true } ) ) )
+      fieldsToUpdate.events = eventsObj
+    }
 
     let [ res ] = await WebhooksConfig( )
       .returning( 'id' )
@@ -49,8 +62,16 @@ module.exports = {
   },
 
   async getStreamWebhooks( { streamId } ) {
-    // TODO: also get summary of event history for each webhook (last event status, etc)
-    return await WebhooksConfig( ).select( '*' ).where( { streamId } )
+    let webhooks = await WebhooksConfig( ).select( '*' ).where( { streamId } )
+    for ( let webhook of webhooks ) {
+      webhook.events = Object.keys( webhook.events )
+    }
+    return webhooks
+  },
+
+  async getStreamWebhooksCount( { streamId } ) {
+    let [ res ] = await WebhooksConfig( ).count().where( { streamId } )
+    return parseInt( res.count )
   },
 
   async dispatchStreamEvent( { streamId, event, eventPayload } ) {

packages/server/modules/webhooks/tests/webhooks.spec.js

@@ -34,10 +34,7 @@ describe( 'Webhooks', ( ) => {
     description: 'test wh',
     secret: 'secret',
     enabled: true,
-    events: {
-      'commit_create': true,
-      'commit_update': true
-    }
+    events: [ 'commit_create', 'commit_update' ]
   }
 
   before( async ( ) => {
@@ -101,5 +98,32 @@ describe( 'Webhooks', ( ) => {
       expect( lastEvents ).to.have.lengthOf( 1 )
       expect( lastEvents[0].payload ).to.equal( 'payload123' )
     } )
+
+    it( 'Should have a webhook limit for streams', async ( ) => {
+      let limit = 100
+      for ( let i = 0; i < limit - 1; i++ ) {
+        await createWebhook( webhookOne )
+      }
+      try {
+        await createWebhook( webhookOne )
+      } catch ( err ) {
+        if ( err.toString().indexOf( 'Maximum' ) > -1 ) return
+      }
+      assert.fail( 'Configured more webhooks than the limit' )
+    } )
+
+    it( 'Should cleanup stream webhooks', async ( ) => {
+      // just cleanup the 99 extra webhooks added before (not a real test)
+      let streamWebhooks = await getStreamWebhooks( { streamId: streamOne.id } )
+      for ( let webhook of streamWebhooks ) {
+        if ( webhook.id != webhookOne.id ) {
+          await deleteWebhook( { id: webhook.id } )
+        }
+      }
+      streamWebhooks = await getStreamWebhooks( { streamId: streamOne.id } )
+      expect( streamWebhooks ).to.have.lengthOf( 1 )
+      expect( streamWebhooks[0] ).to.have.property( 'id' )
+      expect( streamWebhooks[0].id ).to.equal( webhookOne.id )
+    } )
   } )
 } )

packages/webhook-service/package.json

@@ -12,7 +12,8 @@
   },
   "homepage": "https://github.com/specklesystems/speckle-server#readme",
   "scripts": {
-    "test": "echo \"Error: no test specified\" && exit 1"
+    "test": "echo \"Error: no test specified\" && exit 1",
+    "dev": "node src/main.js"
   },
   "dependencies": {
     "knex": "^0.95.7",