Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix up regex, fails test for #79(!) #94

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Fix up regex, fails test for #79(!) #94

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ce84616
fix up regex, fails test for #79
wadtech Jan 25, 2016
36427bc
small refactor and add some test coverage for customHeaders
wadtech Jan 25, 2016
66cc077
remove repetition from specs
wadtech Jan 25, 2016
18f5039
going way too far, start to pull out parsing code into modules.
wadtech Jan 25, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 23 additions & 42 deletions lib/canned.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,13 @@ var querystring = require('querystring')
var url = require('url')
var cannedUtils = require('./utils')
var lookup = require('./lookup')
var ResponseParser = require('./response-parser')
var RequestParser = require('./request-parser')
var _ = require('lodash')

var responseParser = new ResponseParser();
var requestParser = new RequestParser();

function Canned(dir, options) {
this.logger = options.logger
this.wildcard = options.wildcard || 'any'
Expand Down Expand Up @@ -90,16 +95,6 @@ function getContentType(mimetype){
return Response.content_types[mimetype]
}

function stringifyValues(object) {
_.each(object, function(value, key) {
if (typeof value === "object") {
stringifyValues(value);
} else {
object[key] = String(value)
}
})
}

function isContentTypeJson(request) {
var isJson = false;
if (request.headers && request.headers['content-type']) {
Expand All @@ -108,42 +103,28 @@ function isContentTypeJson(request) {
return isJson;
}


Canned.prototype.parseMetaData = function(response) {
var metaData = {}
var metaData = {request: {}},
lines;

// convert CR+LF => LF+LF, CR => LF, fixes line breaks causing issues in windows
response = response.replace("\r", "\n");
var lines = response.split("\n")
var that = this

var optionsMatch = new RegExp(/\/\/!.*[statusCode|contentType|customHeaders]/g)
var requestMatch = new RegExp(/\/\/! [body|params|header]+: ([\w {}":\[\]\-\+\%,@.\/]*)/g)
// we only care about special comment lines
lines = response.split("\n").filter(function(line) {
return line.indexOf("//!") === 0;
})

lines.forEach(function(line) {
if(line.indexOf("//!") === 0) { // special comment line
var matchedRequest = requestMatch.exec(line)
if(matchedRequest) {
metaData.request = JSON.parse(matchedRequest[1])
stringifyValues(metaData.request);
return
}
var matchedOptions = optionsMatch.exec(line)
if(matchedOptions) {
try {
line = line.replace("//!", '')
var content = line.split(',').map(function (s) {
var parts = s.split(':');
parts[0] = '"' + parts[0].trim() + '"'
return parts.join(':')
}).join(',')
var opts = JSON.parse('{' + content + '}')
cannedUtils.extend(metaData, opts)
} catch(e) {
that._log('Invalid file header format try //! statusCode: 201')
}
return
}
// extract any request attributes
var requestAttrs = requestParser.parse(line);
if (Object.keys(requestAttrs).length > 0) {
cannedUtils.extend(metaData.request, requestAttrs);
}

var responseAttrs = responseParser.parse(line);
// extract any response attributes
cannedUtils.extend(metaData, responseAttrs)
})

return metaData
Expand All @@ -160,7 +141,7 @@ Canned.prototype.getSelectedResponse = function(responses, content, headers) {
customHeaders: metaData.customHeaders
}

stringifyValues(content);
cannedUtils.stringifyValues(content);

responses.forEach(function(response) {
var metaData = that.parseMetaData(response)
Expand Down Expand Up @@ -314,7 +295,7 @@ Canned.prototype.responder = function(body, req, res) {
}
}
}

var paths = lookup(httpObj.pathname.join('/'), that.wildcard);
paths.splice(0,1); // The first path is the default
responseHandler = function (err, resp) {
Expand Down Expand Up @@ -397,4 +378,4 @@ Canned.prototype.responseFilter = function (req, res) {
}
}

module.exports = Canned;
module.exports = Canned;
33 changes: 33 additions & 0 deletions lib/request-parser.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
var cannedUtils = require('./utils');
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please use underscore _ in the filename - is a pain sometimes :(

var requestMatch = /\/\/!.*(?:body|params|header):\s+([\w {}":\[\]\-\+\%,@.\/]*)/g;

var RequestParser = function() {}

function parseRequestOptions(line) {
var match,
requestItems = {};

while (match = requestMatch.exec(line)) {
try {
cannedUtils.recursiveMerge(requestItems, JSON.parse(match[1]));
} catch (e) {
console.log(e);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so far the coding style is (largely) without ; so for consistency reasons I would stick with that.

//@todo some logging
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you want todo logging pass the logger down from canned, so it stays configurable.

}
}

return requestItems;
}

function parseEntry(lines) {
var result = {};
lines.split('\n').forEach(function(line) {
cannedUtils.recursiveMerge(result, parseRequestOptions(line));
});
return result;
}

RequestParser.prototype.parse = parseRequestOptions;
RequestParser.prototype.parseEntry = parseEntry;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need to export parseEntry? Shouldn't everybody use parse as the public api? Also than you don't need to have a constructor, just module.exports = parse is fine.


module.exports = RequestParser;
53 changes: 53 additions & 0 deletions lib/response-parser.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
var cannedUtils = require('./utils');

var responseMatch = /\/\/!.*(?:statusCode|contentType|customHeaders)/g;

/**
* the ResponseParser is responsible for collecting the intended return
* status code, content type and header declarations.
*/
function ResponseParser() {}

/**
* _parseresponse takes a single line from a file and extracts
* JSON data if possible. Returns an object.
*/
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comments are not really needed I think it is fairly obvious :)

function parseLine(line) {
var match,
response = {};

while (responseMatch.exec(line)) {
try {
// drop the magix comment
line = line.replace("//!", '').trim();

var content = line.split(',').map(function (s) {
var parts = s.split(':');
if (parts[0].trim()[-1] !== '"') {
parts[0] = '"' + parts[0].trim() + '"'
}
return parts.join(':')
}).join(',')

response = cannedUtils.recursiveMerge(response, JSON.parse('{' + content + '}'))
} catch(e) {
console.log(e);
//@todo pass in log and get cracking
}
}

return response;
}

function parseEntry(lines) {
var result = {};
lines.split('\n').forEach(function(line) {
cannedUtils.recursiveMerge(result, parseLine(line));
});
return result;
}

ResponseParser.prototype.parse = parseLine;
ResponseParser.prototype.parseEntry = parseEntry;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as request parser, no need to export the single version just the one that parses all the lines is fine, also no constructor needed.


module.exports = ResponseParser;
37 changes: 37 additions & 0 deletions lib/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
"use strict";

var _ = require('lodash')

var utils = module.exports = {}

utils.escapeRegexSpecialChars = function (text) {
Expand All @@ -18,6 +20,31 @@ utils.extend = function (target) {
return target
}

/**
* recursively merge an object onto target, preserving existing keys.
* Modified from http://stackoverflow.com/a/383245/771564
*/
utils.recursiveMerge = function(target, other) {
if (!other) {
return target;
}

for (var prop in other) {
try {
// Property in destination targetect set; update its value.
if ( other[prop].constructor == Object ) {
target[prop] = utils.recursiveMerge(target[prop], other[prop]);
} else {
target[prop] = other[prop];
}
} catch(e) {
// Property in destination targetect not set; create it and set its value.
target[prop] = other[prop];
}
}
return target;
}

utils.removeJSLikeComments = function (text) {
return text.replace(/\/\*.+?\*\/|\/\/\s.*(?=[\n\r])/g, '')
}
Expand All @@ -27,3 +54,13 @@ utils.removeSpecialComments = function (data) {
return line.indexOf("//!") !== 0
}).join("\n").trim()
}

utils.stringifyValues = function(object) {
_.each(object, function(value, key) {
if (typeof value === "object") {
utils.stringifyValues(value);
} else {
object[key] = String(value)
}
})
}
95 changes: 92 additions & 3 deletions spec/canned.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -556,15 +556,104 @@ describe('canned', function () {
expect(parsedMeta).toEqual({
request: {
serialkey: 'abc'
},
params: {
serialkey: '12121'
}
});
done();
})
})

describe("parsing metadata", function() {
var Canned, can;

beforeEach(function() {
Canned = require('../lib/canned')
can = new Canned('./spec/test_responses', {});
})

it("Should accept statusCode", function(done) {
var mock_text = '//! statusCode: 418';
var parsedMeta = can.parseMetaData(mock_text);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the tests, should we maybe create a ResponseFileParser which contains all thoses methods? This also makes the tests much easier as you would just need to require and use the ResponseFileParser here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had intended to move the parsing code out into a module in future, I think it'll help hide away some of the gnarly stuff and also let us test it in isolation more neatly.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes that is what I was thinking, what I think is that all the fileParsing canned does kind of belongs together that is why I was thinking of a ResponseFileParser... but feel free to experiment.


expect(parsedMeta).toEqual({
statusCode: 418
});
done();
})

it("Should accept customHeaders", function(done) {
var mock_text = '//! statusCode: 418\n' +
'//! customHeaders: {"Authorization": "Bearer xyz"}';
var parsedMeta = can.parseMetaData(mock_text);

expect(parsedMeta).toEqual({
statusCode: 418,
customHeaders: {
Authorization: 'Bearer xyz'
}
});
done();
})

it("Should accept request body", function(done) {
var mock_text = '//! statusCode: 418\n' +
'//! customHeaders: {"Authorization": "Bearer xyz"}\n' +
'//! customHeaders: {"Location": "Wimbledon Common"}\n' +
'//! body: {"colour": "green"}';
var parsedMeta = can.parseMetaData(mock_text);

expect(parsedMeta).toEqual({
statusCode: 418,
customHeaders: {
Authorization: 'Bearer xyz',
Location: 'Wimbledon Common'
},
request: {
colour: 'green'
}
});
done();
})

it("Should accept request body", function(done) {
var Canned = require('../lib/canned')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to require in the function, just require once and use it.

var can = new Canned('./spec/test_responses', {});
var mock_text = '//! statusCode: 418\n' +
'//! customHeaders: {"Authorization": "Bearer xyz"}\n' +
'//! body: {"colour": "green"}';
var parsedMeta = can.parseMetaData(mock_text);

expect(parsedMeta).toEqual({
statusCode: 418,
customHeaders: {
Authorization: 'Bearer xyz'
},
request: {
colour: 'green'
}
});
done();
})

it("Should apply the latest request params, body or header specified", function(done) {
var mock_text = '//! statusCode: 418\n' +
'//! customHeaders: {"Authorization": "Bearer xyz"}\n' +
'//! body: {"colour": "green"}\n' +
'//! params: {"count": 126}';
var parsedMeta = can.parseMetaData(mock_text);

expect(parsedMeta).toEqual({
statusCode: 418,
customHeaders: {
Authorization: 'Bearer xyz'
},
request: {
count: '126',
}
});
done();
})
})

describe("variable POST responses", function() {
var req, data
beforeEach(function() {
Expand Down
Loading