diff --git a/packages/server/modules/webhooks/graph/schemas/webhooks.graphql b/packages/server/modules/webhooks/graph/schemas/webhooks.graphql index 22d013fca..163fb8522 100644 --- a/packages/server/modules/webhooks/graph/schemas/webhooks.graphql +++ b/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{ diff --git a/packages/server/modules/webhooks/services/webhooks.js b/packages/server/modules/webhooks/services/webhooks.js index 14b79e633..a6c3c0807 100644 --- a/packages/server/modules/webhooks/services/webhooks.js +++ b/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 } ) { diff --git a/packages/server/modules/webhooks/tests/webhooks.spec.js b/packages/server/modules/webhooks/tests/webhooks.spec.js index fa32d46ce..41a7f3664 100644 --- a/packages/server/modules/webhooks/tests/webhooks.spec.js +++ b/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 ) + } ) } ) } ) diff --git a/packages/webhook-service/package.json b/packages/webhook-service/package.json index 1289b2bcc..b72b20d84 100644 --- a/packages/webhook-service/package.json +++ b/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",