Skip to content

Commit ea11b51

Browse files
turtledreamsturtledreams
authored
broader (#65)
* broader * added tests
1 parent c57f79e commit ea11b51

4 files changed

Lines changed: 111 additions & 12 deletions

File tree

‎lib/countly-bulk.js‎

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -415,7 +415,7 @@ function CountlyBulk(conf) {
415415
}
416416
storeSet("cly_bulk_queue", bulkQueue);
417417
readyToProcess = true;
418-
});
418+
}, "heartBeat", false);
419419
}
420420

421421
if (isEmpty) {
@@ -488,10 +488,14 @@ function CountlyBulk(conf) {
488488
* Making HTTP request
489489
* @param {Object} params - key value object with URL params
490490
* @param {Function} callback - callback when request finished or failed
491+
* @param {string} info - function name or any other information for the logs
492+
* @param {Boolean} isBroad - if true a broader (more generous) response check would be implemented
491493
*/
492-
function makeRequest(params, callback) {
494+
function makeRequest(params, callback, info, isBroad) {
493495
try {
494-
cc.log(cc.logLevelEnums.DEBUG, "CountlyBulk makeRequest, Sending HTTP request.");
496+
info = info || "general";
497+
isBroad = isBroad || false;
498+
cc.log(cc.logLevelEnums.DEBUG, `CountlyBulk makeRequest, Sending ${info} HTTP request`);
495499
var serverOptions = parseUrl(conf.url);
496500
var data = prepareParams(params);
497501
var method = "GET";
@@ -531,8 +535,17 @@ function CountlyBulk(conf) {
531535
});
532536

533537
res.on("end", () => {
534-
// checks result field, JSON format and status code
535-
if (cc.isResponseValid(res.statusCode, str)) {
538+
// response validation function will be selected to also accept JSON arrays if isBroad is true
539+
var isResponseValidated;
540+
if (isBroad) {
541+
// JSON array/object both can pass
542+
isResponseValidated = cc.isResponseValidBroad(res.statusCode, str);
543+
}
544+
else {
545+
// only JSON object with result can pass
546+
isResponseValidated = cc.isResponseValid(res.statusCode, str);
547+
}
548+
if (isResponseValidated) {
536549
callback(false, params);
537550
}
538551
else {

‎lib/countly-common.js‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,38 @@ var cc = {
229229
return false;
230230
}
231231
},
232+
/**
233+
* Check if the http response fits the bill of:
234+
* 1. The HTTP response code was successful (which is any 2xx code or code between 200 <= x < 300)
235+
* 2. The returned request is a JSON object OR JSON array
236+
* @param {Number} statusCode - http incoming statusCode
237+
* @param {String} str - response from server, ideally must be: {"result":"Success"} or should contain at least result field
238+
* @returns {Boolean} - returns true if response passes the tests
239+
*/
240+
isResponseValidBroad: function isResponseValidBroad(statusCode, str) {
241+
// status code and response format check
242+
if (!(statusCode >= 200 && statusCode < 300)) {
243+
this.log(this.logLevelEnums.ERROR, `isResponseValidBroad, The server status code is not within the expected range: [${statusCode}]`);
244+
return false;
245+
}
246+
247+
// Try to parse JSON
248+
try {
249+
var response = JSON.parse(str);
250+
// check if parsed response is a JSON object or JSON array, if not it is not valid
251+
if ((Object.prototype.toString.call(response) !== "[object Object]") && (!Array.isArray(response))) {
252+
this.log(this.logLevelEnums.ERROR, `isResponseValidBroad, Http response is not JSON Object nor JSON Array.`);
253+
return false;
254+
}
255+
256+
// request should be accepted even if does not have result field
257+
return true;
258+
}
259+
catch (e) {
260+
this.log(this.logLevelEnums.ERROR, `isResponseValidBroad, Http response is in the wrong format. Error: `, e);
261+
return false;
262+
}
263+
},
232264
};
233265

234266
module.exports = cc;

‎lib/countly.js‎

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1178,7 +1178,7 @@ Countly.Bulk = Bulk;
11781178
if (typeof callback === "function") {
11791179
callback(err, remoteConfigs);
11801180
}
1181-
});
1181+
}, "fetch_remote_config", true);
11821182
}
11831183
else {
11841184
cc.log(cc.logLevelEnums.ERROR, "fetch_remote_config, Remote config consent not given.");
@@ -1487,7 +1487,7 @@ Countly.Bulk = Bulk;
14871487
}
14881488
storeSet("cly_queue", requestQueue);
14891489
readyToProcess = true;
1490-
});
1490+
}, "heartBeat", false);
14911491
}
14921492

14931493
setTimeout(heartBeat, beatInterval);
@@ -1606,10 +1606,14 @@ Countly.Bulk = Bulk;
16061606
* @param {String} api - API endpoint
16071607
* @param {Object} params - key value object with URL params
16081608
* @param {Function} callback - callback when request finished or failed
1609+
* @param {string} info - function name or any other information for the logs
1610+
* @param {Boolean} isBroad - if true a broader (more generous) response check would be implemented
16091611
*/
1610-
function makeRequest(url, api, params, callback) {
1612+
function makeRequest(url, api, params, callback, info, isBroad) {
16111613
try {
1612-
cc.log(cc.logLevelEnums.INFO, "makeRequest, Triggering the HTTP request.");
1614+
info = info || "general";
1615+
isBroad = isBroad || false;
1616+
cc.log(cc.logLevelEnums.INFO, `makeRequest, Sending ${info} HTTP request`);
16131617
var serverOptions = parseUrl(url);
16141618
var data = prepareParams(params);
16151619
var method = "GET";
@@ -1646,9 +1650,18 @@ Countly.Bulk = Bulk;
16461650
str += chunk;
16471651
});
16481652
res.on("end", () => {
1649-
cc.log(cc.logLevelEnums.DEBUG, `makeRequest, Response status code: [${res.statusCode}], response: [${str}].`);
1650-
// checks result field, JSON format and status code
1651-
if (cc.isResponseValid(res.statusCode, str)) {
1653+
cc.log(cc.logLevelEnums.DEBUG, `makeRequest, Response status code: [${res.statusCode}], response: [${str}]`);
1654+
// response validation function will be selected to also accept JSON arrays if isBroad is true
1655+
var isResponseValidated;
1656+
if (isBroad) {
1657+
// JSON array/object both can pass
1658+
isResponseValidated = cc.isResponseValidBroad(res.statusCode, str);
1659+
}
1660+
else {
1661+
// only JSON object with result can pass
1662+
isResponseValidated = cc.isResponseValid(res.statusCode, str);
1663+
}
1664+
if (isResponseValidated) {
16521665
callback(false, params, str);
16531666
}
16541667
else {

‎test/tests_http_response_success.js‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,52 +6,93 @@ describe("Response success suite", () => {
66
it("Check if correct response parameters returns true", () => {
77
var str = '{"result": "Success"}';
88
var result = cc.isResponseValid(200, str);
9+
var resultB = cc.isResponseValidBroad(200, str);
910
assert.ok(result);
11+
assert.ok(resultB);
1012
});
1113
it("Check if wrong response that includes result in it returns false", () => {
1214
var str = '{"endResult": "Success"}';
1315
var result = cc.isResponseValid(200, str);
16+
var resultB = cc.isResponseValidBroad(200, str);
1417
assert.equal(result, false);
18+
assert.equal(resultB, true);
1519
});
1620
it("Check if wrong response that does not include result in it returns false", () => {
1721
var str = '{"end": "Success"}';
1822
var result = cc.isResponseValid(200, str);
23+
var resultB = cc.isResponseValidBroad(200, str);
1924
assert.equal(result, false);
25+
assert.equal(resultB, true);
2026
});
2127
it("Check if wrong statusCode greater than 300 returns false", () => {
2228
var str = '{"result": "Success"}';
2329
var result = cc.isResponseValid(400, str);
30+
var resultB = cc.isResponseValidBroad(400, str);
2431
assert.equal(result, false);
32+
assert.equal(resultB, false);
2533
});
2634
it("Check if wrong statusCode less than 200 returns false", () => {
2735
var str = '{"result": "Success"}';
2836
var result = cc.isResponseValid(100, str);
37+
var resultB = cc.isResponseValidBroad(100, str);
2938
assert.equal(result, false);
39+
assert.equal(resultB, false);
3040
});
3141
it("Check if wrong statusCode 300 returns false", () => {
3242
var str = '{"result": "Success"}';
3343
var result = cc.isResponseValid(300, str);
44+
var resultB = cc.isResponseValidBroad(300, str);
3445
assert.equal(result, false);
46+
assert.equal(resultB, false);
3547
});
3648
it("Check if non Success value at result field returns true", () => {
3749
var str = '{"result": "Sth"}';
3850
var result = cc.isResponseValid(200, str);
51+
var resultB = cc.isResponseValidBroad(200, str);
3952
assert.equal(result, true);
53+
assert.equal(resultB, true);
4054
});
4155
it("Check if there is no statusCode it returns false", () => {
4256
var str = '{"result": "Success"}';
4357
var result = cc.isResponseValid({}.a, str);
58+
var resultB = cc.isResponseValidBroad({}.a, str);
4459
assert.equal(result, false);
60+
assert.equal(resultB, false);
4561
});
4662
it("Check if just string/non-object returns false", () => {
4763
var str = "RESULT";
4864
var result = cc.isResponseValid(200, str);
65+
var resultB = cc.isResponseValidBroad(200, str);
4966
assert.equal(result, false);
67+
assert.equal(resultB, false);
5068
});
5169
it("Check if empty response returns false", () => {
5270
var res = {};
5371
var str = "";
5472
var result = cc.isResponseValid(res, str);
73+
var resultB = cc.isResponseValidBroad(res, str);
5574
assert.equal(result, false);
75+
assert.equal(resultB, false);
76+
});
77+
it("Check if JSON array returns true", () => {
78+
var str = '["result", "Success"]';
79+
var result = cc.isResponseValid(200, str);
80+
var resultB = cc.isResponseValidBroad(200, str);
81+
assert.equal(result, false);
82+
assert.equal(resultB, true);
83+
});
84+
it("Check if empty JSON arrays returns true", () => {
85+
var str = '[]';
86+
var result = cc.isResponseValid(200, str);
87+
var resultB = cc.isResponseValidBroad(200, str);
88+
assert.equal(result, false);
89+
assert.equal(resultB, true);
90+
});
91+
it("Check if just an array returns false", () => {
92+
var str = [];
93+
var result = cc.isResponseValid(200, str);
94+
var resultB = cc.isResponseValidBroad(200, str);
95+
assert.equal(result, false);
96+
assert.equal(resultB, false);
5697
});
5798
});

0 commit comments

Comments
 (0)