🎨 small webhook refactorings (#1383)

* 🧹 clean up forgotten leftover

* 🎨 reset req.params

* 🐛 Bugfix and minor change on request parameters

*  Minor improvements on request parameters

Co-authored-by: Jan Oberhauser <jan.oberhauser@gmail.com>
This commit is contained in:
Ben Hesseldieck
2021-01-28 15:44:10 +01:00
committed by GitHub
parent bc74d7ad47
commit 8764171989
3 changed files with 10 additions and 11 deletions

View File

@@ -117,6 +117,9 @@ export class ActiveWorkflowRunner {
throw new ResponseHelper.ResponseError('The "activeWorkflows" instance did not get initialized yet.', 404, 404); throw new ResponseHelper.ResponseError('The "activeWorkflows" instance did not get initialized yet.', 404, 404);
} }
// Reset request parameters
req.params = {};
let webhook = await Db.collections.Webhook?.findOne({ webhookPath: path, method: httpMethod }) as IWebhookDb; let webhook = await Db.collections.Webhook?.findOne({ webhookPath: path, method: httpMethod }) as IWebhookDb;
let webhookId: string | undefined; let webhookId: string | undefined;
@@ -126,10 +129,11 @@ export class ActiveWorkflowRunner {
const pathElements = path.split('/'); const pathElements = path.split('/');
webhookId = pathElements.shift(); webhookId = pathElements.shift();
const dynamicWebhooks = await Db.collections.Webhook?.find({ webhookId, method: httpMethod, pathLength: pathElements.length }); const dynamicWebhooks = await Db.collections.Webhook?.find({ webhookId, method: httpMethod, pathLength: pathElements.length });
if (dynamicWebhooks === undefined) { if (dynamicWebhooks === undefined || dynamicWebhooks.length === 0) {
// The requested webhook is not registered // The requested webhook is not registered
throw new ResponseHelper.ResponseError(`The requested webhook "${httpMethod} ${path}" is not registered.`, 404, 404); throw new ResponseHelper.ResponseError(`The requested webhook "${httpMethod} ${path}" is not registered.`, 404, 404);
} }
// set webhook to the first webhook result // set webhook to the first webhook result
// if more results have been returned choose the one with the most route-matches // if more results have been returned choose the one with the most route-matches
webhook = dynamicWebhooks[0]; webhook = dynamicWebhooks[0];
@@ -323,10 +327,7 @@ export class ActiveWorkflowRunner {
// TODO check if there is standard error code for duplicate key violation that works // TODO check if there is standard error code for duplicate key violation that works
// with all databases // with all databases
if (error.name === 'QueryFailedError') { if (error.name === 'QueryFailedError') {
errorMessage = error.parameters.length === 5 errorMessage = `The webhook path [${webhook.webhookPath}] and method [${webhook.method}] already exist.`;
? `Node [${webhook.node}] can't be saved, please duplicate [${webhook.node}] and delete the currently existing one.`
: `The webhook path [${webhook.webhookPath}] and method [${webhook.method}] already exist.`;
} else if (error.detail) { } else if (error.detail) {
// it's a error runnig the webhook methods (checkExists, create) // it's a error runnig the webhook methods (checkExists, create)
errorMessage = error.detail; errorMessage = error.detail;

View File

@@ -1693,7 +1693,6 @@ class App {
let response; let response;
try { try {
delete req.params[0];
response = await this.activeWorkflowRunner.executeWebhook('HEAD', requestUrl, req, res); response = await this.activeWorkflowRunner.executeWebhook('HEAD', requestUrl, req, res);
} catch (error) { } catch (error) {
ResponseHelper.sendErrorResponse(res, error); ResponseHelper.sendErrorResponse(res, error);
@@ -1735,7 +1734,6 @@ class App {
let response; let response;
try { try {
delete req.params[0];
response = await this.activeWorkflowRunner.executeWebhook('GET', requestUrl, req, res); response = await this.activeWorkflowRunner.executeWebhook('GET', requestUrl, req, res);
} catch (error) { } catch (error) {
ResponseHelper.sendErrorResponse(res, error); ResponseHelper.sendErrorResponse(res, error);
@@ -1757,7 +1755,6 @@ class App {
let response; let response;
try { try {
delete req.params[0];
response = await this.activeWorkflowRunner.executeWebhook('POST', requestUrl, req, res); response = await this.activeWorkflowRunner.executeWebhook('POST', requestUrl, req, res);
} catch (error) { } catch (error) {
ResponseHelper.sendErrorResponse(res, error); ResponseHelper.sendErrorResponse(res, error);
@@ -1779,7 +1776,6 @@ class App {
let response; let response;
try { try {
delete req.params[0];
response = await this.testWebhooks.callTestWebhook('HEAD', requestUrl, req, res); response = await this.testWebhooks.callTestWebhook('HEAD', requestUrl, req, res);
} catch (error) { } catch (error) {
ResponseHelper.sendErrorResponse(res, error); ResponseHelper.sendErrorResponse(res, error);
@@ -1821,7 +1817,6 @@ class App {
let response; let response;
try { try {
delete req.params[0];
response = await this.testWebhooks.callTestWebhook('GET', requestUrl, req, res); response = await this.testWebhooks.callTestWebhook('GET', requestUrl, req, res);
} catch (error) { } catch (error) {
ResponseHelper.sendErrorResponse(res, error); ResponseHelper.sendErrorResponse(res, error);
@@ -1843,7 +1838,6 @@ class App {
let response; let response;
try { try {
delete req.params[0];
response = await this.testWebhooks.callTestWebhook('POST', requestUrl, req, res); response = await this.testWebhooks.callTestWebhook('POST', requestUrl, req, res);
} catch (error) { } catch (error) {
ResponseHelper.sendErrorResponse(res, error); ResponseHelper.sendErrorResponse(res, error);

View File

@@ -56,6 +56,9 @@ export class TestWebhooks {
async callTestWebhook(httpMethod: WebhookHttpMethod, path: string, request: express.Request, response: express.Response): Promise<IResponseCallbackData> { async callTestWebhook(httpMethod: WebhookHttpMethod, path: string, request: express.Request, response: express.Response): Promise<IResponseCallbackData> {
let webhookData: IWebhookData | undefined = this.activeWebhooks!.get(httpMethod, path); let webhookData: IWebhookData | undefined = this.activeWebhooks!.get(httpMethod, path);
// Reset request parameters
request.params = {};
// check if path is dynamic // check if path is dynamic
if (webhookData === undefined) { if (webhookData === undefined) {
const pathElements = path.split('/'); const pathElements = path.split('/');
@@ -65,6 +68,7 @@ export class TestWebhooks {
// The requested webhook is not registered // The requested webhook is not registered
throw new ResponseHelper.ResponseError(`The requested webhook "${httpMethod} ${path}" is not registered.`, 404, 404); throw new ResponseHelper.ResponseError(`The requested webhook "${httpMethod} ${path}" is not registered.`, 404, 404);
} }
path = webhookData.path; path = webhookData.path;
// extracting params from path // extracting params from path
path.split('/').forEach((ele, index) => { path.split('/').forEach((ele, index) => {