From ff02d8a74c06bbb310101ccf4c52cf1f22d8c502 Mon Sep 17 00:00:00 2001 From: Lance Ball Date: Thu, 29 Oct 2020 18:28:35 -0400 Subject: [PATCH 1/2] feat: handle CloudEvent and Message responses from function invocation We already **kind of** handled them, but these changes make it more intuitive for the user. For example, the function author can respond with `new CloudEvent(...)` or `HTTP.binary(event)` and the framework should handle the response correctly. Signed-off-by: Lance Ball --- lib/context.js | 17 +++---- lib/invoker.js | 32 ++++++++++-- lib/request-handler.js | 3 +- package-lock.json | 20 ++++---- package.json | 2 +- test/fixtures/query-params/index.js | 2 +- test/test-context.js | 4 +- test/test.js | 77 +++++++++++++++++++++-------- 8 files changed, 108 insertions(+), 49 deletions(-) diff --git a/lib/context.js b/lib/context.js index 70ced9a..9e10ce4 100644 --- a/lib/context.js +++ b/lib/context.js @@ -1,4 +1,4 @@ -const Spec = require('../lib/ce-constants.js').Spec; +const { CloudEvent } = require('cloudevents'); class Context { constructor(request) { @@ -24,34 +24,31 @@ class CloudEventResponse { #response; constructor(response) { - this.#response = response; - if (!this.#response.headers) { - this.#response.headers = []; - } + this.#response = { data: response }; } version(version) { - this.#response.headers[Spec.version] = version; + this.#response.specversion = version; return this; } id(id) { - this.#response.headers[Spec.id] = id; + this.#response.id = id; return this; } type(type) { - this.#response.headers[Spec.type] = type; + this.#response.type = type; return this; } source(source) { - this.#response.headers[Spec.source] = source; + this.#response.source = source; return this; } response() { - return this.#response; + return new CloudEvent(this.#response); } } diff --git a/lib/invoker.js b/lib/invoker.js index 8baeb9b..38f0923 100644 --- a/lib/invoker.js +++ b/lib/invoker.js @@ -1,4 +1,5 @@ 'use strict'; +const { CloudEvent, HTTP } = require('cloudevents'); module.exports = function invoker(func) { return async function invokeFunction(context, log) { @@ -7,10 +8,10 @@ module.exports = function invoker(func) { code: 200, response: undefined, headers: { - 'Content-Type': 'application/json; charset=utf8', - 'Access-Control-Allow-Methods': + 'content-type': 'application/json; charset=utf8', + 'access-control-allow-methods': 'OPTIONS, GET, DELETE, POST, PUT, HEAD, PATCH', - 'Access-Control-Allow-Origin': '*' + 'access-control-allow-origin': '*' } }; @@ -47,9 +48,32 @@ module.exports = function invoker(func) { // Check for user defined headers if (typeof payload.response.headers === 'object') { - Object.assign(payload.headers, payload.response.headers); + const headers = {}; + // normalize the headers as lowercase + for (const header in payload.response.headers) { + headers[header.toLocaleLowerCase()] = payload.response.headers[header]; + } + payload.headers = { ...payload.headers, ...headers }; delete payload.response.headers; } + + // If the response is a CloudEvent, we need to convert it + // to a Message first and respond with the headers/body + if (payload.response instanceof CloudEvent) { + try { + const message = HTTP.binary(payload.response); + payload.headers = {...payload.headers, ...message.headers}; + payload.response = message.body; + } catch (e) { + console.error(e); + } + } + + // Check for user supplied body + if (payload.response.body !== undefined) { + payload.response = payload.response.body; + delete payload.response.body; + } return payload; }; }; diff --git a/lib/request-handler.js b/lib/request-handler.js index 0ecd2f4..3848f6a 100644 --- a/lib/request-handler.js +++ b/lib/request-handler.js @@ -22,7 +22,8 @@ module.exports = function(fastify, opts, done) { }; function sendReply(reply, payload) { - if (payload.headers['Content-Type'].startsWith('text/plain')) { + const contentType = payload.headers['content-type']; + if (contentType.startsWith('text/plain') && (typeof payload.response !== 'string')) { payload.response = JSON.stringify(payload.response); } return reply diff --git a/package-lock.json b/package-lock.json index 6a6b5c3..0b0b13a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -547,19 +547,19 @@ } }, "cloudevents": { - "version": "3.1.0", - "resolved": "https://registry.npmjs.org/cloudevents/-/cloudevents-3.1.0.tgz", - "integrity": "sha512-98t6+Qs/r2PiYflNFztUcPSDfaaRU8KKMzaMR4dn9MPpijZj3A1W+L307t00D6xRzXdkDDiMcB2THS3dCp+kcw==", + "version": "3.2.0", + "resolved": "https://registry.npmjs.org/cloudevents/-/cloudevents-3.2.0.tgz", + "integrity": "sha512-D5QVEJtREXxM0QGmla0FKs0cctcIUQIAJpIEYx7R11PFFh9O7Bykos/gZCYJgzTieDrnEesJ+6pD03P48ZRrGw==", "requires": { "ajv": "~6.12.3", "axios": "~0.19.2", - "uuid": "~8.2.0" + "uuid": "~8.3.0" }, "dependencies": { "ajv": { - "version": "6.12.3", - "resolved": "https://registry.npmjs.org/ajv/-/ajv-6.12.3.tgz", - "integrity": "sha512-4K0cK3L1hsqk9xIb2z9vs/XU+PGJZ9PNpJRDS9YLzmNdX6jmVPfamLvTJr0aDAusnHyCHO6MjzlkAsgtqp9teA==", + "version": "6.12.6", + "resolved": "https://registry.npmjs.org/ajv/-/ajv-6.12.6.tgz", + "integrity": "sha512-j3fVLgvTo527anyYyJOGTYJbG+vnnQYvE0m5mmkc1TK+nxAppkCLMIL0aZ4dblVCNoGShhm+kzE4ZUykBoMg4g==", "requires": { "fast-deep-equal": "^3.1.1", "fast-json-stable-stringify": "^2.0.0", @@ -573,9 +573,9 @@ "integrity": "sha512-f3qQ9oQy9j2AhBe/H9VC91wLmKBCCU/gDOnKNAYG5hswO7BLKj09Hc5HYNz9cGI++xlpDCIgDaitVs03ATR84Q==" }, "uuid": { - "version": "8.2.0", - "resolved": "https://registry.npmjs.org/uuid/-/uuid-8.2.0.tgz", - "integrity": "sha512-CYpGiFTUrmI6OBMkAdjSDM0k5h8SkkiTP4WAjQgDgNB1S3Ou9VBEvr6q0Kv2H1mMk7IWfxYGpMH5sd5AvcIV2Q==" + "version": "8.3.1", + "resolved": "https://registry.npmjs.org/uuid/-/uuid-8.3.1.tgz", + "integrity": "sha512-FOmRr+FmWEIG8uhZv6C2bTgEVXsHk08kE7mPlrBbEe+c3r9pjceVPgupIfNIhc4yx55H69OXANrUaSuu9eInKg==" } } }, diff --git a/package.json b/package.json index f50578d..bec7572 100644 --- a/package.json +++ b/package.json @@ -21,7 +21,7 @@ "bin": "./bin/cli.js", "dependencies": { "chalk": "^4.1.0", - "cloudevents": "^3.1.0", + "cloudevents": "^3.2.0", "commander": "^6.1.0", "death": "^1.1.0", "fastify": "^3.3.0", diff --git a/test/fixtures/query-params/index.js b/test/fixtures/query-params/index.js index a220e7a..13fb4ff 100644 --- a/test/fixtures/query-params/index.js +++ b/test/fixtures/query-params/index.js @@ -1,3 +1,3 @@ module.exports = function testFunc(context) { - return context; + return { ...context.query }; }; diff --git a/test/test-context.js b/test/test-context.js index 983179c..5701bca 100644 --- a/test/test-context.js +++ b/test/test-context.js @@ -43,8 +43,8 @@ test('Provides HTTP request query parameters with the context parameter', t => { .expect('Content-Type', /json/) .end((err, res) => { t.error(err, 'No error'); - t.equal(res.body.query.lunch, 'tacos'); - t.equal(res.body.query.supper, 'burgers'); + t.equal(res.body.lunch, 'tacos'); + t.equal(res.body.supper, 'burgers'); t.end(); server.close(); }); diff --git a/test/test.js b/test/test.js index 901ae51..53d17b1 100644 --- a/test/test.js +++ b/test/test.js @@ -9,7 +9,7 @@ const Spec = require('../lib/ce-constants.js').Spec; const { existsSync, readdirSync } = require('fs'); const { execSync } = require('child_process'); const path = require('path'); -const { CloudEvent } = require('cloudevents'); +const { CloudEvent, HTTP } = require('cloudevents'); // Ensure fixture dependencies are installed const fixtureDir = path.join(__dirname, 'fixtures'); @@ -173,6 +173,62 @@ test('Responds to 1.0 structured cloud events', t => { }, { log: false }); }); +test('Handles 1.0 CloudEvent responses', t => { + framework(_ => { + return new CloudEvent({ + source: 'test', + type: 'test-type', + data: 'some data', + datacontenttype: 'text/plain' + }); + }, server => { + request(server) + .post('/') + .send({ message: 'hello' }) + .set(Spec.id, '1') + .set(Spec.source, 'integration-test') + .set(Spec.type, 'dev.knative.example') + .set(Spec.version, '1.0') + .expect(200) + .expect('Content-Type', /text/) + .end((err, res) => { + t.error(err, 'No error'); + t.equal(res.text, 'some data'); + t.end(); + server.close(); + }); + }, + { log: false }); +}); + +test('Handles 1.0 CloudEvent Message responses', t => { + framework(_ => { + return HTTP.binary(new CloudEvent({ + source: 'test', + type: 'test-type', + data: 'some data', + datacontenttype: 'text/plain' + })); + }, server => { + request(server) + .post('/') + .send({ message: 'hello' }) + .set(Spec.id, '1') + .set(Spec.source, 'integration-test') + .set(Spec.type, 'dev.knative.example') + .set(Spec.version, '1.0') + .expect(200) + .expect('Content-Type', /text/) + .end((err, res) => { + t.error(err, 'No error'); + t.equal(res.text, 'some data'); + t.end(); + server.close(); + }); + }, + { log: false }); +}); + test('Extracts event data as the first parameter to a function', t => { const data = { lunch: "tacos" @@ -226,25 +282,6 @@ test('Successfully handles events with no data', t => { }); }); -test('Responds with error code (4xx or 5xx) to malformed cloud events', t => { - const func = require(`${__dirname}/fixtures/cloud-event/`); - framework(func, server => { - request(server) - .post('/') - .send({ message: 'hello' }) - .set(Spec.id, '1') - .set(Spec.source, 'integration-test') - .set(Spec.version, '0.3') - .set('ce-datacontenttype', 'application/json') - .expect('Content-Type', /json/) - .end((err, res) => { - t.assert(res.statusCode >= 400 && res.statusCode <= 599, 'Error code 4xx or 5xx expected.'); - t.end(); - server.close(); - }); - }, { log: false }); -}); - test('Responds with 406 Not Acceptable to unknown cloud event versions', t => { const func = require(`${__dirname}/fixtures/cloud-event/`); framework(func, server => { From f480cfa7f559817e58d3edcf9bf9ec277a2dcac4 Mon Sep 17 00:00:00 2001 From: Lance Ball Date: Fri, 30 Oct 2020 14:12:40 -0400 Subject: [PATCH 2/2] squash: incorporate PR feedback Signed-off-by: Lance Ball --- lib/invoker.js | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/lib/invoker.js b/lib/invoker.js index 38f0923..4c9d4ff 100644 --- a/lib/invoker.js +++ b/lib/invoker.js @@ -26,11 +26,7 @@ module.exports = function invoker(func) { payload.response = await func(context); } } catch (err) { - log.error(err); - payload.response = { - statusCode: err.code ? err.code : 500, - statusMessage: err.message - }; + payload.response = handleError(err, log); } // Return 204 No Content if the function returns @@ -40,12 +36,6 @@ module.exports = function invoker(func) { return payload; } - // Check for user defined status code - if (payload.response.statusCode) { - payload.code = payload.response.statusCode; - delete payload.response.statusCode; - } - // Check for user defined headers if (typeof payload.response.headers === 'object') { const headers = {}; @@ -64,11 +54,18 @@ module.exports = function invoker(func) { const message = HTTP.binary(payload.response); payload.headers = {...payload.headers, ...message.headers}; payload.response = message.body; - } catch (e) { - console.error(e); + } catch (err) { + payload.response = handleError(err, log); + return payload; } } + // Check for user defined status code + if (payload.response.statusCode) { + payload.code = payload.response.statusCode; + delete payload.response.statusCode; + } + // Check for user supplied body if (payload.response.body !== undefined) { payload.response = payload.response.body; @@ -77,3 +74,11 @@ module.exports = function invoker(func) { return payload; }; }; + +function handleError(err, log) { + log.error(err); + return { + statusCode: err.code ? err.code : 500, + statusMessage: err.message + }; +}