From 13fe10589dd6b608e797f41b33b8653e215d2ca8 Mon Sep 17 00:00:00 2001 From: Swapnil Bhosale Date: Wed, 13 Feb 2019 18:16:56 +0530 Subject: [PATCH 1/4] FIX-ISSUE-27. 1) Do not add explicit log binding 2)update test cases for not to test on log method --- src/IncomingMessage.js | 11 +++--- test/ExpressAdapter.test.js | 28 +++++++--------- test/IncomingMessage.test.js | 59 +++++++++++++++------------------ test/expressIntegration.test.js | 26 +++++++-------- 4 files changed, 56 insertions(+), 68 deletions(-) diff --git a/src/IncomingMessage.js b/src/IncomingMessage.js index 2078216..6573801 100644 --- a/src/IncomingMessage.js +++ b/src/IncomingMessage.js @@ -1,7 +1,7 @@ /* eslint-disable no-underscore-dangle */ import EventEmitter from "events"; -const NOOP = () => {}; +const NOOP = () => { }; function removePortFromAddress(address) { return address @@ -20,8 +20,8 @@ function createConnectionObject(context) { const xForwardedFor = req.headers ? req.headers["x-forwarded-for"] : undefined; return { - encrypted : req.originalUrl && req.originalUrl.toLowerCase().startsWith("https"), - remoteAddress : removePortFromAddress(xForwardedFor) + encrypted: req.originalUrl && req.originalUrl.toLowerCase().startsWith("https"), + remoteAddress: removePortFromAddress(xForwardedFor) }; } @@ -37,14 +37,12 @@ function createConnectionObject(context) { */ function sanitizeContext(context) { const sanitizedContext = { - ...context, - log : context.log.bind(context) + ...context }; // We don't want the developper to mess up express flow // See https://github.com/yvele/azure-function-express/pull/12#issuecomment-336733540 delete sanitizedContext.done; - return sanitizedContext; } @@ -74,6 +72,7 @@ export default class IncomingMessage extends EventEmitter { this.connection = createConnectionObject(context); this.context = sanitizeContext(context); // Specific to Azure Function + //this.context = Object.assign(context) } } diff --git a/test/ExpressAdapter.test.js b/test/ExpressAdapter.test.js index 8e9f299..674c537 100644 --- a/test/ExpressAdapter.test.js +++ b/test/ExpressAdapter.test.js @@ -1,7 +1,5 @@ import { ExpressAdapter } from "../src"; -const NOOP = () => {}; - describe("ExpressAdapter", () => { it("Should work", done => { @@ -16,17 +14,16 @@ describe("ExpressAdapter", () => { }); const context = { - log : NOOP, - bindings : { req: { originalUrl: "http://foo.com/bar" } }, - done : () => { + bindings: { req: { originalUrl: "http://foo.com/bar" } }, + done: () => { expect(listenerCalled).toBe(true); // Response that will be sent to Azure Function runtime expect(context.res).toEqual({ - body : "body", - headers : {}, - isRaw : true, - status : 200 + body: "body", + headers: {}, + isRaw: true, + status: 200 }); done(); } @@ -48,17 +45,16 @@ describe("ExpressAdapter", () => { }); const context = { - log : NOOP, - bindings : { req: { originalUrl: "http://foo.com/bar" } }, - done : () => { + bindings: { req: { originalUrl: "http://foo.com/bar" } }, + done: () => { expect(listenerCalled).toBe(true); // Response that will be sent to Azure Function runtime expect(context.res).toEqual({ - body : "body", - headers : {}, - isRaw : true, - status : 200 + body: "body", + headers: {}, + isRaw: true, + status: 200 }); done(); } diff --git a/test/IncomingMessage.test.js b/test/IncomingMessage.test.js index 8c14ad5..4fd03a3 100644 --- a/test/IncomingMessage.test.js +++ b/test/IncomingMessage.test.js @@ -5,13 +5,12 @@ describe("IncomingMessage", () => { it("Should work", () => { const context = { - bindings : { - req : { - originalUrl : "https://foo.com/bar", - headers : { "x-forwarded-for": "192.168.0.1:57996" } + bindings: { + req: { + originalUrl: "https://foo.com/bar", + headers: { "x-forwarded-for": "192.168.0.1:57996" } } - }, - log : () => {} + } }; const req = new IncomingMessage(context); @@ -19,10 +18,10 @@ describe("IncomingMessage", () => { req.socket.destroy(); expect(req).toMatchObject({ - url : "https://foo.com/bar", - connection : { - encrypted : true, - remoteAddress : "192.168.0.1" + url: "https://foo.com/bar", + connection: { + encrypted: true, + remoteAddress: "192.168.0.1" } }); }); @@ -30,12 +29,11 @@ describe("IncomingMessage", () => { it("Should work with no headers", () => { const context = { - bindings : { - req : { - originalUrl : "http://foo.com/bar" + bindings: { + req: { + originalUrl: "http://foo.com/bar" } - }, - log : () => {} + } }; const req = new IncomingMessage(context); @@ -43,10 +41,10 @@ describe("IncomingMessage", () => { req.socket.destroy(); expect(req).toMatchObject({ - url : "http://foo.com/bar", - connection : { - encrypted : false, - remoteAddress : undefined + url: "http://foo.com/bar", + connection: { + encrypted: false, + remoteAddress: undefined } }); }); @@ -54,15 +52,14 @@ describe("IncomingMessage", () => { it("Should work with a full native context object", () => { const context = { - invocationId : "f0f6e586-0b79-4407-aa53-97919f45eba5", - bindingData : { foo: "bar" }, - bindings : { - req : { - originalUrl : "http://foo.com/bar" + invocationId: "f0f6e586-0b79-4407-aa53-97919f45eba5", + bindingData: { foo: "bar" }, + bindings: { + req: { + originalUrl: "http://foo.com/bar" } }, - log : () => {}, - done : () => {} + done: () => { } }; const req = new IncomingMessage(context); @@ -70,10 +67,10 @@ describe("IncomingMessage", () => { req.socket.destroy(); expect(req).toMatchObject({ - url : "http://foo.com/bar", - connection : { - encrypted : false, - remoteAddress : undefined + url: "http://foo.com/bar", + connection: { + encrypted: false, + remoteAddress: undefined } }); @@ -82,8 +79,6 @@ describe("IncomingMessage", () => { expect(req.context.invocationId).toBe(context.invocationId); expect(req.context.bindingData).toBe(context.bindingData); expect(req.context.bindings).toBe(context.bindings); - expect(req.context.log).not.toBe(context.log); - expect(req.context.log).toBeInstanceOf(Function); expect(req.context.done).toBeUndefined(); // We don't want to pass done }); diff --git a/test/expressIntegration.test.js b/test/expressIntegration.test.js index b4da748..8837463 100644 --- a/test/expressIntegration.test.js +++ b/test/expressIntegration.test.js @@ -18,18 +18,17 @@ describe("express integration", () => { // 3. Mock Azure Function context var context = { - bindings : { req: { method: "GET", originalUrl: "https://lol.com/api/foo/bar" } }, - log : () => { throw new Error("Log should not be called"); }, - done : (error) => { + bindings: { req: { method: "GET", originalUrl: "https://lol.com/api/foo/bar" } }, + done: (error) => { expect(error).toBeUndefined(); expect(context.res.status).toBe(200); expect(context.res.body).toBe('{"foo":"foo","bar":"bar"}'); expect(context.res.headers).toEqual({ - "X-Powered-By" : "Express", - "Cache-Control" : "max-age=600", - "Content-Type" : "application/json; charset=utf-8", - "Content-Length" : "25", - ETag : 'W/"19-0CKEGOfZ5AYCM4LPaa4gzWL6olU"' + "X-Powered-By": "Express", + "Cache-Control": "max-age=600", + "Content-Type": "application/json; charset=utf-8", + "Content-Length": "25", + ETag: 'W/"19-0CKEGOfZ5AYCM4LPaa4gzWL6olU"' }); done(); @@ -55,16 +54,15 @@ describe("express integration", () => { // 3. Mock Azure Function context var context = { - bindings : { req: { method: "GET", originalUrl: "https://lol.com/api/foo/bar" } }, - log : () => { throw new Error("Log should not be called"); }, - done : (error) => { + bindings: { req: { method: "GET", originalUrl: "https://lol.com/api/foo/bar" } }, + done: (error) => { expect(error).toBeUndefined(); expect(context.res.status).toBe(200); expect(context.res.body).toBe('{"foo":"foo","bar":"bar"}'); expect(context.res.headers).toEqual({ - "Content-Type" : "application/json; charset=utf-8", - "Content-Length" : "25", - ETag : 'W/"19-0CKEGOfZ5AYCM4LPaa4gzWL6olU"' + "Content-Type": "application/json; charset=utf-8", + "Content-Length": "25", + ETag: 'W/"19-0CKEGOfZ5AYCM4LPaa4gzWL6olU"' }); done(); From c0d3c06d2241ef7a896c4365e2acc5f1606d3d6b Mon Sep 17 00:00:00 2001 From: Swapnil Bhosale Date: Wed, 13 Feb 2019 18:18:01 +0530 Subject: [PATCH 2/4] remove commented code --- src/IncomingMessage.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/IncomingMessage.js b/src/IncomingMessage.js index 6573801..7799c00 100644 --- a/src/IncomingMessage.js +++ b/src/IncomingMessage.js @@ -72,7 +72,6 @@ export default class IncomingMessage extends EventEmitter { this.connection = createConnectionObject(context); this.context = sanitizeContext(context); // Specific to Azure Function - //this.context = Object.assign(context) } } From db4564f3f44c84cd870dd097c7c17c790823a29e Mon Sep 17 00:00:00 2001 From: Swapnil Bhosale Date: Wed, 13 Feb 2019 18:25:59 +0530 Subject: [PATCH 3/4] remove space as suggested by travis ci --- src/IncomingMessage.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/IncomingMessage.js b/src/IncomingMessage.js index 7799c00..90e4e81 100644 --- a/src/IncomingMessage.js +++ b/src/IncomingMessage.js @@ -20,8 +20,8 @@ function createConnectionObject(context) { const xForwardedFor = req.headers ? req.headers["x-forwarded-for"] : undefined; return { - encrypted: req.originalUrl && req.originalUrl.toLowerCase().startsWith("https"), - remoteAddress: removePortFromAddress(xForwardedFor) + encrypted : req.originalUrl && req.originalUrl.toLowerCase().startsWith("https"), + remoteAddress : removePortFromAddress(xForwardedFor) }; } From 76a8b64cf201c306ab66b2d90543d61b502cab22 Mon Sep 17 00:00:00 2001 From: Swapnil Bhosale Date: Thu, 14 Feb 2019 11:50:19 +0530 Subject: [PATCH 4/4] 1)update formatting --- src/IncomingMessage.js | 5 ++-- test/ExpressAdapter.test.js | 24 ++++++++-------- test/IncomingMessage.test.js | 50 ++++++++++++++++----------------- test/expressIntegration.test.js | 25 +++++++++-------- 4 files changed, 53 insertions(+), 51 deletions(-) diff --git a/src/IncomingMessage.js b/src/IncomingMessage.js index 90e4e81..dfaea45 100644 --- a/src/IncomingMessage.js +++ b/src/IncomingMessage.js @@ -1,7 +1,7 @@ /* eslint-disable no-underscore-dangle */ import EventEmitter from "events"; -const NOOP = () => { }; +const NOOP = () => {}; function removePortFromAddress(address) { return address @@ -20,7 +20,7 @@ function createConnectionObject(context) { const xForwardedFor = req.headers ? req.headers["x-forwarded-for"] : undefined; return { - encrypted : req.originalUrl && req.originalUrl.toLowerCase().startsWith("https"), + encrypted : req.originalUrl && req.originalUrl.toLowerCase().startsWith("https"), remoteAddress : removePortFromAddress(xForwardedFor) }; } @@ -43,6 +43,7 @@ function sanitizeContext(context) { // We don't want the developper to mess up express flow // See https://github.com/yvele/azure-function-express/pull/12#issuecomment-336733540 delete sanitizedContext.done; + return sanitizedContext; } diff --git a/test/ExpressAdapter.test.js b/test/ExpressAdapter.test.js index 674c537..cdb89ff 100644 --- a/test/ExpressAdapter.test.js +++ b/test/ExpressAdapter.test.js @@ -14,16 +14,16 @@ describe("ExpressAdapter", () => { }); const context = { - bindings: { req: { originalUrl: "http://foo.com/bar" } }, - done: () => { + bindings : { req: { originalUrl: "http://foo.com/bar" } }, + done : () => { expect(listenerCalled).toBe(true); // Response that will be sent to Azure Function runtime expect(context.res).toEqual({ - body: "body", - headers: {}, - isRaw: true, - status: 200 + body : "body", + headers : {}, + isRaw : true, + status : 200 }); done(); } @@ -45,16 +45,16 @@ describe("ExpressAdapter", () => { }); const context = { - bindings: { req: { originalUrl: "http://foo.com/bar" } }, - done: () => { + bindings : { req: { originalUrl: "http://foo.com/bar" } }, + done : () => { expect(listenerCalled).toBe(true); // Response that will be sent to Azure Function runtime expect(context.res).toEqual({ - body: "body", - headers: {}, - isRaw: true, - status: 200 + body : "body", + headers : {}, + isRaw : true, + status : 200 }); done(); } diff --git a/test/IncomingMessage.test.js b/test/IncomingMessage.test.js index 4fd03a3..4326cc3 100644 --- a/test/IncomingMessage.test.js +++ b/test/IncomingMessage.test.js @@ -5,10 +5,10 @@ describe("IncomingMessage", () => { it("Should work", () => { const context = { - bindings: { - req: { - originalUrl: "https://foo.com/bar", - headers: { "x-forwarded-for": "192.168.0.1:57996" } + bindings : { + req : { + originalUrl : "https://foo.com/bar", + headers : { "x-forwarded-for": "192.168.0.1:57996" } } } }; @@ -18,10 +18,10 @@ describe("IncomingMessage", () => { req.socket.destroy(); expect(req).toMatchObject({ - url: "https://foo.com/bar", - connection: { - encrypted: true, - remoteAddress: "192.168.0.1" + url : "https://foo.com/bar", + connection : { + encrypted : true, + remoteAddress : "192.168.0.1" } }); }); @@ -29,9 +29,9 @@ describe("IncomingMessage", () => { it("Should work with no headers", () => { const context = { - bindings: { - req: { - originalUrl: "http://foo.com/bar" + bindings : { + req : { + originalUrl : "http://foo.com/bar" } } }; @@ -41,10 +41,10 @@ describe("IncomingMessage", () => { req.socket.destroy(); expect(req).toMatchObject({ - url: "http://foo.com/bar", - connection: { - encrypted: false, - remoteAddress: undefined + url : "http://foo.com/bar", + connection : { + encrypted : false, + remoteAddress : undefined } }); }); @@ -52,14 +52,14 @@ describe("IncomingMessage", () => { it("Should work with a full native context object", () => { const context = { - invocationId: "f0f6e586-0b79-4407-aa53-97919f45eba5", - bindingData: { foo: "bar" }, - bindings: { - req: { - originalUrl: "http://foo.com/bar" + invocationId : "f0f6e586-0b79-4407-aa53-97919f45eba5", + bindingData : { foo: "bar" }, + bindings : { + req : { + originalUrl : "http://foo.com/bar" } }, - done: () => { } + done : () => {} }; const req = new IncomingMessage(context); @@ -67,10 +67,10 @@ describe("IncomingMessage", () => { req.socket.destroy(); expect(req).toMatchObject({ - url: "http://foo.com/bar", - connection: { - encrypted: false, - remoteAddress: undefined + url : "http://foo.com/bar", + connection : { + encrypted : false, + remoteAddress : undefined } }); diff --git a/test/expressIntegration.test.js b/test/expressIntegration.test.js index 8837463..1cbce29 100644 --- a/test/expressIntegration.test.js +++ b/test/expressIntegration.test.js @@ -18,17 +18,17 @@ describe("express integration", () => { // 3. Mock Azure Function context var context = { - bindings: { req: { method: "GET", originalUrl: "https://lol.com/api/foo/bar" } }, - done: (error) => { + bindings : { req: { method: "GET", originalUrl: "https://lol.com/api/foo/bar" } }, + done : (error) => { expect(error).toBeUndefined(); expect(context.res.status).toBe(200); expect(context.res.body).toBe('{"foo":"foo","bar":"bar"}'); expect(context.res.headers).toEqual({ - "X-Powered-By": "Express", - "Cache-Control": "max-age=600", - "Content-Type": "application/json; charset=utf-8", - "Content-Length": "25", - ETag: 'W/"19-0CKEGOfZ5AYCM4LPaa4gzWL6olU"' + "X-Powered-By" : "Express", + "Cache-Control" : "max-age=600", + "Content-Type" : "application/json; charset=utf-8", + "Content-Length" : "25", + ETag : 'W/"19-0CKEGOfZ5AYCM4LPaa4gzWL6olU"' }); done(); @@ -54,15 +54,16 @@ describe("express integration", () => { // 3. Mock Azure Function context var context = { - bindings: { req: { method: "GET", originalUrl: "https://lol.com/api/foo/bar" } }, - done: (error) => { + bindings : { req: { method: "GET", originalUrl: "https://lol.com/api/foo/bar" } }, + log : () => { throw new Error("Log should not be called"); }, + done : (error) => { expect(error).toBeUndefined(); expect(context.res.status).toBe(200); expect(context.res.body).toBe('{"foo":"foo","bar":"bar"}'); expect(context.res.headers).toEqual({ - "Content-Type": "application/json; charset=utf-8", - "Content-Length": "25", - ETag: 'W/"19-0CKEGOfZ5AYCM4LPaa4gzWL6olU"' + "Content-Type" : "application/json; charset=utf-8", + "Content-Length" : "25", + ETag : 'W/"19-0CKEGOfZ5AYCM4LPaa4gzWL6olU"' }); done();