From 42456365769596811ba5f52cd762f764bbcbdaf1 Mon Sep 17 00:00:00 2001 From: "Juan Picado @jotadeveloper" Date: Thu, 14 Jun 2018 07:25:09 +0200 Subject: [PATCH] feat: add published package support to template The package published can be printed into the template Improve error handling Improve notification documentation --- docs/notifications.md | 97 +++++++++++++++++++++++-- src/api/endpoint/api/publish.js | 13 +++- src/lib/notify.js | 43 +++++++---- test/functional/lib/simple_server.js | 2 +- test/functional/notifications/notify.js | 16 ++-- 5 files changed, 140 insertions(+), 31 deletions(-) diff --git a/docs/notifications.md b/docs/notifications.md index 494c18d5a..b286004ba 100644 --- a/docs/notifications.md +++ b/docs/notifications.md @@ -5,12 +5,14 @@ title: "Notifications" Notify was built primarily to use with Slack's Incoming webhooks, but will also deliver a simple payload to -any endpoint. Currently only active for `publish` / `create` -commands. +any endpoint. Currently only active for `npm publish` +command. ## Usage -An example with a **HipChat** and **Google Hangouts Chat** hook: +An example with a **HipChat**, **Stride** and **Google Hangouts Chat** hook: + +> Verdaccio supports any API, feel free to ad more examples. #### Single notification @@ -43,7 +45,84 @@ notify: content: '{"body": {"version": 1,"type": "doc","content": [{"type": "paragraph","content": [{"type": "text","text": "New package published: * {{ name }}* Publisher name: * {{ publisher.name }}"}]}]}}' ``` -### Publisher information +## Template + +We use [Handlebars](https://handlebarsjs.com/) as main template engine. + +### Format Examples + +``` +# iterate all versions +{{ name }}{{#each versions}} v{{version}}{{/each}}`"} + +# publisher and `dist-tag` package published +{{ publisher.name }} has published {{publishedPackage}}"} +``` + +### Properties + +List of properties accesible via template + +* Metadata +* Publisher (who is publishing) +* Package Published (package@1.0.0) + +### Metadata + +Package metadata that the template has access + +``` +{ + "_id": "@test/pkg1", + "name": "@test/pkg1", + "description": "", + "dist-tags": { + "beta": "1.0.54" + }, + "versions": { + "1.0.54": { + "name": "@test/pkg1", + "version": "1.0.54", + "description": "some description", + "main": "index.js", + "scripts": { + "test": "echo \"Error: no test specified\" && exit 1" + }, + "keywords": [], + "author": { + "name": "Author Name", + "email": "author@domain.com" + }, + "license": "MIT", + "dependencies": { + "webpack": "4.12.0" + }, + "readmeFilename": "README.md", + "_id": "@ test/pkg1@1.0.54", + "_npmVersion": "6.1.0", + "_nodeVersion": "9.9.0", + "_npmUser": {}, + "dist": { + "integrity": "sha512-JlXWpLtMUBAqvVZBvH7UVLhXkGE1ctmXbDjbH/l0zMuG7wVzQ7GshTYvD/b5C+G2vOL2oiIS1RtayA/kKkTwKw==", + "shasum": "29c55c52c1e76e966e706165e5b9f22e32aa9f22", + "tarball": "http://localhost:4873/@test/pkg1/-/@test/pkg1-1.0.54.tgz" + } + } + }, + "readme": "# test", + "_attachments": { + "@test/pkg1-1.0.54.tgz": { + "content_type": "application/octet-stream", + "data": "H4sIAAAAAAAAE+y9Z5PjyJIgOJ ...", + "length": 33112 + } + }, + "time": {} +} +``` + + +### Publisher You can access to the package publisher information in the `content` of a webhook using the `publisher` object. @@ -69,6 +148,14 @@ notify: **Note:** it's not possible to get the publisher information if the `package.json` file already has the `publisher` property. +### Package Published + +You can acces to the package is being published with the keyword `{{publishedPackage}}` as follows. + +``` +{{ publisher.name }} has published {{publishedPackage}}"} +``` + ## Configuration Property | Type | Required | Support | Default | Description @@ -78,4 +165,4 @@ packagePattern| string | No | all | | Only run this notification if the package packagePatternFlags| string | No | all | | Any flags to be used with the regular expression headers| array/object | Yes | all | | If this endpoint requires specific headers, set them here as an array of key: value objects. endpoint| string | Yes | all | | set the URL endpoint for this call -content| string | Yes | all | | any [handlebar](https://handlebarsjs.com/) expressions +content| string | Yes | all | | any [Handlebar](https://handlebarsjs.com/) expressions diff --git a/src/api/endpoint/api/publish.js b/src/api/endpoint/api/publish.js index f4ebadb24..5661a6ab9 100644 --- a/src/api/endpoint/api/publish.js +++ b/src/api/endpoint/api/publish.js @@ -12,6 +12,7 @@ import {notify} from '../../../lib/notify'; import type {Router} from 'express'; import type {Config, Callback} from '@verdaccio/types'; import type {IAuth, $ResponseExtend, $RequestExtend, $NextFunctionVer, IStorageHandler} from '../../../../types'; +import logger from '../../../lib/logger'; export default function(router: Router, auth: IAuth, storage: IStorageHandler, config: Config) { const can = allow(auth); @@ -80,9 +81,9 @@ export default function(router: Router, auth: IAuth, storage: IStorageHandler, c return next(err); } - const t2 = Object.keys(metadata.versions)[0]; - metadata.versions[t2].readme = _.isNil(metadata.readme) === false ? String(metadata.readme) : ''; - create_version(t2, metadata.versions[t2], function(err) { + const versionToPublish = Object.keys(metadata.versions)[0]; + metadata.versions[versionToPublish].readme = _.isNil(metadata.readme) === false ? String(metadata.readme) : ''; + create_version(versionToPublish, metadata.versions[versionToPublish], function(err) { if (err) { return next(err); } @@ -91,7 +92,11 @@ export default function(router: Router, auth: IAuth, storage: IStorageHandler, c if (err) { return next(err); } - notify(metadata, config, req.remote_user); + + notify(metadata, config, req.remote_user, `${metadata.name}@${versionToPublish}`).then(() =>{}, (err) => { + logger.logger.error({err}, 'notify batch service has failed: @{err}'); + }); + res.status(201); return next({ok: ok_message, success: true}); }); diff --git a/src/lib/notify.js b/src/lib/notify.js index 695065b4f..f817fa19d 100644 --- a/src/lib/notify.js +++ b/src/lib/notify.js @@ -1,9 +1,9 @@ -const Handlebars = require('handlebars'); -const request = require('request'); -const _ = require('lodash'); -const logger = require('./logger'); +import Handlebars from 'handlebars'; +import request from 'request'; +import _ from 'lodash'; +import logger from './logger'; -const handleNotify = function(metadata, notifyEntry, publisherInfo) { +const handleNotify = function(metadata, notifyEntry, publisherInfo, publishedPackage) { let regex; if (metadata.name && notifyEntry.packagePattern) { // FUTURE: comment out due https://github.com/verdaccio/verdaccio/pull/108#issuecomment-312421052 @@ -18,7 +18,7 @@ const handleNotify = function(metadata, notifyEntry, publisherInfo) { // don't override 'publisher' if package.json already has that if (!metadata.publisher) { - metadata = {...metadata, publisher: publisherInfo}; + metadata = {...metadata, publishedPackage, publisher: publisherInfo}; } const content = template(metadata); @@ -52,29 +52,40 @@ const handleNotify = function(metadata, notifyEntry, publisherInfo) { return new Promise((resolve, reject) => { request(options, function(err, response, body) { if (err || response.statusCode >= 400) { - const errorMessage = _.isNil(err) ? response.statusMessage : err; - logger.logger.error({err: errorMessage}, ' notify error: @{err.message}'); + const errorMessage = _.isNil(err) ? response.body : err.message; + logger.logger.error({errorMessage}, 'notify service has thrown an error: @{errorMessage}'); + reject(errorMessage); } else { - logger.logger.info({content: content}, 'A notification has been shipped: @{content}'); - if (body) { - logger.logger.debug({body: body}, ' body: @{body}'); + logger.logger.info({content}, 'A notification has been shipped: @{content}'); + if (_.isNil(body) === false) { + const bodyResolved = _.isNil(body) === false ? body : null; + + logger.logger.debug({body}, ' body: @{body}'); + return resolve(bodyResolved); } - resolve(_.isNil(body) === false ? body : null); + + reject(Error('body is missing')); } }); }); }; -const notify = function(metadata, config, publisherInfo) { +function sendNotification(metadata, key, ...moreMedatata) { + return handleNotify(metadata, key, ...moreMedatata); +} + +const notify = function(metadata, config, ...moreMedatata) { if (config.notify) { if (config.notify.content) { - return handleNotify(metadata, config.notify, publisherInfo); + return sendNotification(metadata, config.notify, ...moreMedatata); } else { // multiple notifications endpoints PR #108 - return Promise.all(_.map(config.notify, (key) => handleNotify(metadata, key, publisherInfo))); + return Promise.all(_.map(config.notify, (key) => sendNotification(metadata, key, ...moreMedatata))); } } + + return Promise.resolve(); }; -module.exports.notify = notify; +export {notify}; diff --git a/test/functional/lib/simple_server.js b/test/functional/lib/simple_server.js index 129d44c4b..77c605ebb 100644 --- a/test/functional/lib/simple_server.js +++ b/test/functional/lib/simple_server.js @@ -12,7 +12,7 @@ export default class ExpressServer { } start(port: number): Promise { - return new Promise((resolve, reject) => { + return new Promise((resolve) => { this.app.use(bodyParser.json()); this.app.use(bodyParser.urlencoded({ extended: true diff --git a/test/functional/notifications/notify.js b/test/functional/notifications/notify.js index 53841c167..cc3e05433 100644 --- a/test/functional/notifications/notify.js +++ b/test/functional/notifications/notify.js @@ -22,6 +22,12 @@ export default function(express) { describe('notifications', () => { + function parseBody(notification) { + const jsonBody = JSON.parse(notification); + + return jsonBody; + } + beforeAll(function () { express.post('/api/notify', function (req, res) { res.send(req.body); @@ -38,7 +44,7 @@ export default function(express) { }; notify(metadata, config, publisherInfo).then(function (body) { - const jsonBody = JSON.parse(body); + const jsonBody = parseBody(body); assert.ok( `New package published: * ${metadata.name}*. Publisher name: * ${ publisherInfo.name @@ -63,7 +69,7 @@ export default function(express) { }; notify(metadata, configMultipleHeader, publisherInfo).then(function (body) { - const jsonBody = JSON.parse(body); + const jsonBody = parseBody(body); assert.ok( `New package published: * ${metadata.name}*. Publisher name: * ${ publisherInfo.name @@ -99,7 +105,7 @@ export default function(express) { notify(metadata, multipleNotificationsEndpoint, publisherInfo).then(function (body) { body.forEach(function(notification) { - const jsonBody = JSON.parse(notification); + const jsonBody = parseBody(notification); assert.ok( `New package published: * ${metadata.name}*. Publisher name: * ${ publisherInfo.name @@ -125,7 +131,7 @@ export default function(express) { assert.equal(false, 'This service should fails with status code 400'); done(); }, function (err) { - assert.ok('Bad Request' === err, 'The error message should be "Bad Request'); + expect(err).toEqual('bad response'); done(); }); }); @@ -140,7 +146,7 @@ export default function(express) { notify(metadata, config, publisherInfo).then( function(body) { - const jsonBody = JSON.parse(body); + const jsonBody = parseBody(body); assert.ok( `New package published: * ${metadata.name}*. Publisher name: * ${ metadata.publisher.name