From dd0489004f59aa5996139c606b1d7fc37d04f897 Mon Sep 17 00:00:00 2001 From: wellwelwel <46850407+wellwelwel@users.noreply.github.com> Date: Wed, 31 Jan 2024 07:00:52 -0300 Subject: [PATCH 1/4] fix(cache): improve cache key formation --- lib/helpers.js | 29 +++++++++++++++++++++++++++++ lib/parsers/parser_cache.js | 31 ++++++++++++++++++++----------- 2 files changed, 49 insertions(+), 11 deletions(-) diff --git a/lib/helpers.js b/lib/helpers.js index 2a8f80367f..54afc8f979 100644 --- a/lib/helpers.js +++ b/lib/helpers.js @@ -63,3 +63,32 @@ function typeMatch(type, list, Types) { } exports.typeMatch = typeMatch; + +/** + * Basic local performance check for 15,000 unique keys generated by using: + * + * - **SHA256:** ~221ms + * - **SHA1:** ~209ms + * - **Base64:** ~60ms + * - **Regex:** ~54ms + * - **Loop each char:** ~36ms (this) + * - **No sanitize:** ~20ms + */ +function sanitizeKey(value, delimiter = ':') { + const str = String(value); + if (str === 'undefined') return '_'; + let res = ''; + for (let i = 0, len = str.length; i < len; i++) { + if (str[i] === '_') { + // Ensuring that `some_key` is diffenret from `some{delimiter}key` + res += '__'; + } else if (str[i] === delimiter) { + res += '_'; + } else { + res += str[i]; + } + } + return res; +} + +exports.sanitizeKey = sanitizeKey; diff --git a/lib/parsers/parser_cache.js b/lib/parsers/parser_cache.js index 2660da9136..8d7366f856 100644 --- a/lib/parsers/parser_cache.js +++ b/lib/parsers/parser_cache.js @@ -1,27 +1,36 @@ 'use strict'; const LRU = require('lru-cache').default; +const helpers = require('../helpers'); const parserCache = new LRU({ - max: 15000 + max: 15000, }); function keyFromFields(type, fields, options, config) { let res = - `${type}` + + `${helpers.sanitizeKey(type, '/')}` + `/${typeof options.nestTables}` + - `/${options.nestTables}` + - `/${options.rowsAsArray}` + - `/${options.supportBigNumbers || config.supportBigNumbers}` + - `/${options.bigNumberStrings || config.bigNumberStrings}` + + `/${helpers.sanitizeKey(options.nestTables, '/')}` + + `/${Boolean(options.rowsAsArray)}` + + `/${Boolean(options.supportBigNumbers || config.supportBigNumbers)}` + + `/${Boolean(options.bigNumberStrings || config.bigNumberStrings)}` + `/${typeof options.typeCast}` + - `/${options.timezone || config.timezone}` + - `/${options.decimalNumbers}` + - `/${options.dateStrings}`; + `/${helpers.sanitizeKey(options.timezone || config.timezone, '/')}` + + `/${Boolean(options.decimalNumbers)}` + + `/${helpers.sanitizeKey(options.dateStrings, '/')}`; + for (let i = 0; i < fields.length; ++i) { const field = fields[i]; - res += `/${field.name}:${field.columnType}:${field.length}:${field.schema}:${field.table}:${field.flags}:${field.characterSet}`; + res += `/${helpers.sanitizeKey(field.name)}:${helpers.sanitizeKey( + field.columnType, + )}:${helpers.sanitizeKey(field.length)}:${helpers.sanitizeKey( + field.schema, + )}:${helpers.sanitizeKey(field.table)}:${helpers.sanitizeKey( + field.flags, + )}:${helpers.sanitizeKey(field.characterSet)}`; } + return res; } @@ -49,5 +58,5 @@ function clearCache() { module.exports = { getParser: getParser, setMaxCache: setMaxCache, - clearCache: clearCache + clearCache: clearCache, }; From 4c3b0c0c048aa76bc153a03a12fa23ed9aaff5ff Mon Sep 17 00:00:00 2001 From: wellwelwel <46850407+wellwelwel@users.noreply.github.com> Date: Wed, 31 Jan 2024 07:01:10 -0300 Subject: [PATCH 2/4] ci: add tests to sanitize cache key helper --- test/unit/helpers/sanitize-cache-key.js | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 test/unit/helpers/sanitize-cache-key.js diff --git a/test/unit/helpers/sanitize-cache-key.js b/test/unit/helpers/sanitize-cache-key.js new file mode 100644 index 0000000000..72b1fcdbb5 --- /dev/null +++ b/test/unit/helpers/sanitize-cache-key.js @@ -0,0 +1,21 @@ +'use strict'; + +const assert = require('assert'); +const helpers = require('../../../lib/helpers.js'); + +assert.deepStrictEqual(helpers.sanitizeKey(), '_'); +assert.deepStrictEqual(helpers.sanitizeKey(undefined), '_'); +assert.deepStrictEqual(helpers.sanitizeKey(null), 'null'); +assert.deepStrictEqual(helpers.sanitizeKey(false), 'false'); +assert.deepStrictEqual(helpers.sanitizeKey(true), 'true'); +assert.deepStrictEqual(helpers.sanitizeKey('text'), 'text'); +assert.deepStrictEqual(helpers.sanitizeKey('_'), '__'); +assert.deepStrictEqual(helpers.sanitizeKey('__'), '____'); +assert.deepStrictEqual(helpers.sanitizeKey(":", ':'), '_'); +assert.deepStrictEqual(helpers.sanitizeKey("/", '/'), '_'); +assert.deepStrictEqual(helpers.sanitizeKey(":", '/'), ':'); +assert.deepStrictEqual(helpers.sanitizeKey("/", ':'), '/'); +assert.deepStrictEqual(helpers.sanitizeKey(16), '16'); +assert.deepStrictEqual(helpers.sanitizeKey(16_000), '16000'); +assert.deepStrictEqual(helpers.sanitizeKey(5e52), '5e+52'); +assert.deepStrictEqual(helpers.sanitizeKey(98756123165498765321654987n), '98756123165498765321654987'); From 8dafb2b3595dbadf0794a529f55cbcabef7d36c8 Mon Sep 17 00:00:00 2001 From: wellwelwel <46850407+wellwelwel@users.noreply.github.com> Date: Wed, 31 Jan 2024 20:18:58 -0300 Subject: [PATCH 3/4] refactor: change serialization to `JSON.stringify` --- lib/helpers.js | 29 ------------------- lib/parsers/parser_cache.js | 37 +++++++++++++------------ test/unit/helpers/sanitize-cache-key.js | 21 -------------- 3 files changed, 20 insertions(+), 67 deletions(-) delete mode 100644 test/unit/helpers/sanitize-cache-key.js diff --git a/lib/helpers.js b/lib/helpers.js index 54afc8f979..2a8f80367f 100644 --- a/lib/helpers.js +++ b/lib/helpers.js @@ -63,32 +63,3 @@ function typeMatch(type, list, Types) { } exports.typeMatch = typeMatch; - -/** - * Basic local performance check for 15,000 unique keys generated by using: - * - * - **SHA256:** ~221ms - * - **SHA1:** ~209ms - * - **Base64:** ~60ms - * - **Regex:** ~54ms - * - **Loop each char:** ~36ms (this) - * - **No sanitize:** ~20ms - */ -function sanitizeKey(value, delimiter = ':') { - const str = String(value); - if (str === 'undefined') return '_'; - let res = ''; - for (let i = 0, len = str.length; i < len; i++) { - if (str[i] === '_') { - // Ensuring that `some_key` is diffenret from `some{delimiter}key` - res += '__'; - } else if (str[i] === delimiter) { - res += '_'; - } else { - res += str[i]; - } - } - return res; -} - -exports.sanitizeKey = sanitizeKey; diff --git a/lib/parsers/parser_cache.js b/lib/parsers/parser_cache.js index 8d7366f856..f963f7c9e8 100644 --- a/lib/parsers/parser_cache.js +++ b/lib/parsers/parser_cache.js @@ -1,37 +1,40 @@ 'use strict'; const LRU = require('lru-cache').default; -const helpers = require('../helpers'); const parserCache = new LRU({ max: 15000, }); function keyFromFields(type, fields, options, config) { - let res = - `${helpers.sanitizeKey(type, '/')}` + - `/${typeof options.nestTables}` + - `/${helpers.sanitizeKey(options.nestTables, '/')}` + - `/${Boolean(options.rowsAsArray)}` + - `/${Boolean(options.supportBigNumbers || config.supportBigNumbers)}` + - `/${Boolean(options.bigNumberStrings || config.bigNumberStrings)}` + - `/${typeof options.typeCast}` + - `/${helpers.sanitizeKey(options.timezone || config.timezone, '/')}` + - `/${Boolean(options.decimalNumbers)}` + - `/${helpers.sanitizeKey(options.dateStrings, '/')}`; + const res = [ + type, + typeof options.nestTables, + options.nestTables, + Boolean(options.rowsAsArray), + Boolean(options.supportBigNumbers || config.supportBigNumbers), + Boolean(options.bigNumberStrings || config.bigNumberStrings), + typeof options.typeCast, + options.timezone || config.timezone, + Boolean(options.decimalNumbers), + options.dateStrings, + ]; for (let i = 0; i < fields.length; ++i) { const field = fields[i]; - res += `/${helpers.sanitizeKey(field.name)}:${helpers.sanitizeKey( + + res.push([ + field.name, field.columnType, - )}:${helpers.sanitizeKey(field.length)}:${helpers.sanitizeKey( + field.length, field.schema, - )}:${helpers.sanitizeKey(field.table)}:${helpers.sanitizeKey( + field.table, field.flags, - )}:${helpers.sanitizeKey(field.characterSet)}`; + field.characterSet, + ]); } - return res; + return JSON.stringify(res); } function getParser(type, fields, options, config, compiler) { diff --git a/test/unit/helpers/sanitize-cache-key.js b/test/unit/helpers/sanitize-cache-key.js deleted file mode 100644 index 72b1fcdbb5..0000000000 --- a/test/unit/helpers/sanitize-cache-key.js +++ /dev/null @@ -1,21 +0,0 @@ -'use strict'; - -const assert = require('assert'); -const helpers = require('../../../lib/helpers.js'); - -assert.deepStrictEqual(helpers.sanitizeKey(), '_'); -assert.deepStrictEqual(helpers.sanitizeKey(undefined), '_'); -assert.deepStrictEqual(helpers.sanitizeKey(null), 'null'); -assert.deepStrictEqual(helpers.sanitizeKey(false), 'false'); -assert.deepStrictEqual(helpers.sanitizeKey(true), 'true'); -assert.deepStrictEqual(helpers.sanitizeKey('text'), 'text'); -assert.deepStrictEqual(helpers.sanitizeKey('_'), '__'); -assert.deepStrictEqual(helpers.sanitizeKey('__'), '____'); -assert.deepStrictEqual(helpers.sanitizeKey(":", ':'), '_'); -assert.deepStrictEqual(helpers.sanitizeKey("/", '/'), '_'); -assert.deepStrictEqual(helpers.sanitizeKey(":", '/'), ':'); -assert.deepStrictEqual(helpers.sanitizeKey("/", ':'), '/'); -assert.deepStrictEqual(helpers.sanitizeKey(16), '16'); -assert.deepStrictEqual(helpers.sanitizeKey(16_000), '16000'); -assert.deepStrictEqual(helpers.sanitizeKey(5e52), '5e+52'); -assert.deepStrictEqual(helpers.sanitizeKey(98756123165498765321654987n), '98756123165498765321654987'); From 400991185336c6a8bf661adb792a10ec980de5c6 Mon Sep 17 00:00:00 2001 From: wellwelwel <46850407+wellwelwel@users.noreply.github.com> Date: Thu, 1 Feb 2024 06:43:25 -0300 Subject: [PATCH 4/4] ci: add tests to cache key serialization --- lib/parsers/parser_cache.js | 3 +- test/unit/parsers/cache-key-serialization.js | 490 +++++++++++++++++++ 2 files changed, 492 insertions(+), 1 deletion(-) create mode 100644 test/unit/parsers/cache-key-serialization.js diff --git a/lib/parsers/parser_cache.js b/lib/parsers/parser_cache.js index f963f7c9e8..0eb577703c 100644 --- a/lib/parsers/parser_cache.js +++ b/lib/parsers/parser_cache.js @@ -34,7 +34,7 @@ function keyFromFields(type, fields, options, config) { ]); } - return JSON.stringify(res); + return JSON.stringify(res, null, 0); } function getParser(type, fields, options, config, compiler) { @@ -62,4 +62,5 @@ module.exports = { getParser: getParser, setMaxCache: setMaxCache, clearCache: clearCache, + _keyFromFields: keyFromFields, }; diff --git a/test/unit/parsers/cache-key-serialization.js b/test/unit/parsers/cache-key-serialization.js new file mode 100644 index 0000000000..e32c0270ee --- /dev/null +++ b/test/unit/parsers/cache-key-serialization.js @@ -0,0 +1,490 @@ +'use strict'; + +const assert = require('assert'); +const _keyFromFields = + require('../../../lib/parsers/parser_cache.js')._keyFromFields; + +// Invalid +const test1 = { + type: undefined, + fields: [ + { + name: undefined, + columnType: undefined, + length: undefined, + schema: undefined, + table: undefined, + flags: undefined, + characterSet: undefined, + }, + ], + options: { + nestTables: undefined, + rowsAsArray: undefined, + supportBigNumbers: undefined, + bigNumberStrings: undefined, + typeCast: undefined, + timezone: undefined, + decimalNumbers: undefined, + dateStrings: undefined, + }, + config: { + supportBigNumbers: undefined, + bigNumberStrings: undefined, + timezone: undefined, + }, +}; + +// Invalid, except for `config` (global overwriting) +const test2 = { + type: undefined, + fields: [ + { + name: undefined, + columnType: undefined, + length: undefined, + schema: undefined, + table: undefined, + flags: undefined, + characterSet: undefined, + }, + ], + options: { + nestTables: undefined, + rowsAsArray: undefined, + supportBigNumbers: undefined, + bigNumberStrings: undefined, + typeCast: undefined, + timezone: undefined, + decimalNumbers: undefined, + dateStrings: undefined, + }, + config: { + supportBigNumbers: false, + bigNumberStrings: false, + timezone: 'local', + }, +}; + +// Invalid, except for options +const test3 = { + type: undefined, + fields: [ + { + name: undefined, + columnType: undefined, + length: undefined, + schema: undefined, + table: undefined, + flags: undefined, + characterSet: undefined, + }, + ], + options: { + nestTables: '', + rowsAsArray: false, + supportBigNumbers: false, + bigNumberStrings: false, + typeCast: true, + timezone: 'local', + decimalNumbers: false, + dateStrings: false, + }, + config: { + supportBigNumbers: undefined, + bigNumberStrings: undefined, + timezone: undefined, + }, +}; + +// Based on results of `SELECT * FROM test WHERE value = ?` +const test4 = { + type: 'binary', + fields: [ + { + name: 'id', + columnType: '3', + length: undefined, + schema: 'test', + table: 'test', + flags: '16899', + characterSet: '63', + }, + { + name: 'value', + columnType: '246', + length: undefined, + schema: 'test', + table: 'test', + flags: '0', + characterSet: '63', + }, + ], + options: { + nestTables: false, + rowsAsArray: false, + supportBigNumbers: false, + bigNumberStrings: false, + typeCast: true, + timezone: 'local', + decimalNumbers: false, + dateStrings: 'DATETIME', + }, + config: { + supportBigNumbers: undefined, + bigNumberStrings: undefined, + timezone: undefined, + }, +}; + +// Same from test4, but with invalid booleans need to reach out the same key +const test5 = { + type: 'binary', + fields: [ + { + name: 'id', + columnType: '3', + length: undefined, + schema: 'test', + table: 'test', + flags: '16899', + characterSet: '63', + }, + { + name: 'value', + columnType: '246', + length: undefined, + schema: 'test', + table: 'test', + flags: '0', + characterSet: '63', + }, + ], + options: { + nestTables: false, + rowsAsArray: undefined, + supportBigNumbers: undefined, + bigNumberStrings: undefined, + typeCast: true, + timezone: 'local', + decimalNumbers: undefined, + dateStrings: 'DATETIME', + }, + config: { + supportBigNumbers: undefined, + bigNumberStrings: undefined, + timezone: undefined, + }, +}; + +// Forcing delimiters on strings fields +// Checking for quotes escape +const test6 = { + type: 'binary', + fields: [ + { + name: ':', + columnType: '©', + length: undefined, + schema: '/', + table: ',', + flags: '_', + characterSet: '❌', + }, + ], + options: { + nestTables: false, + rowsAsArray: true, + supportBigNumbers: true, + bigNumberStrings: true, + typeCast: true, + timezone: '""`\'', + decimalNumbers: true, + dateStrings: '#', + }, + config: { + supportBigNumbers: undefined, + bigNumberStrings: undefined, + timezone: undefined, + }, +}; + +// valid with `true` on booleans +const test7 = { + type: 'binary', + fields: [ + { + name: 'id', + columnType: '3', + length: undefined, + schema: 'test', + table: 'test', + flags: '16899', + characterSet: '63', + }, + { + name: 'value', + columnType: '246', + length: undefined, + schema: 'test', + table: 'test', + flags: '0', + characterSet: '63', + }, + ], + options: { + nestTables: true, + rowsAsArray: true, + supportBigNumbers: true, + bigNumberStrings: true, + typeCast: true, + timezone: 'local', + decimalNumbers: true, + dateStrings: 'DATETIME', + }, + config: { + supportBigNumbers: true, + bigNumberStrings: true, + timezone: true, + }, +}; + +// Expects the same result from test7, but using valid values instead of `true` on booleans fields +const test8 = { + type: 'binary', + fields: [ + { + name: 'id', + columnType: '3', + length: undefined, + schema: 'test', + table: 'test', + flags: '16899', + characterSet: '63', + }, + { + name: 'value', + columnType: '246', + length: undefined, + schema: 'test', + table: 'test', + flags: '0', + characterSet: '63', + }, + ], + options: { + nestTables: true, + rowsAsArray: 2, + supportBigNumbers: 'yes', + bigNumberStrings: [], + typeCast: true, + timezone: 'local', + decimalNumbers: { + a: null, + }, + dateStrings: 'DATETIME', + }, + config: { + supportBigNumbers: true, + bigNumberStrings: true, + timezone: true, + }, +}; + +// Invalid: checking function parser in wrong fields, expecting to be `null` +const test9 = { + type: 'binary', + fields: [ + { + name: 'id', + columnType: '3', + length: undefined, + schema: 'test', + table: 'test', + flags: '16899', + characterSet: '63', + }, + ], + options: { + nestTables: false, + rowsAsArray: false, + supportBigNumbers: false, + // Expected: true + bigNumberStrings: (_, next) => next(), + // Expected: "function" + typeCast: (_, next) => next(), + timezone: 'local', + decimalNumbers: false, + // Expected: null + dateStrings: (_, next) => next(), + }, + config: { + supportBigNumbers: undefined, + bigNumberStrings: undefined, + timezone: undefined, + }, +}; + +const result1 = _keyFromFields( + test1.type, + test1.fields, + test1.options, + test1.config, +); +const result2 = _keyFromFields( + test2.type, + test2.fields, + test2.options, + test2.config, +); +const result3 = _keyFromFields( + test3.type, + test3.fields, + test3.options, + test3.config, +); +const result4 = _keyFromFields( + test4.type, + test4.fields, + test4.options, + test4.config, +); +const result5 = _keyFromFields( + test5.type, + test5.fields, + test5.options, + test5.config, +); +const result6 = _keyFromFields( + test6.type, + test6.fields, + test6.options, + test6.config, +); +const result7 = _keyFromFields( + test7.type, + test7.fields, + test7.options, + test7.config, +); +const result8 = _keyFromFields( + test8.type, + test8.fields, + test8.options, + test8.config, +); +const result9 = _keyFromFields( + test9.type, + test9.fields, + test9.options, + test9.config, +); + +assert.deepStrictEqual( + result1, + '[null,"undefined",null,false,false,false,"undefined",null,false,null,[null,null,null,null,null,null,null]]', +); +assert(JSON.parse(result1)); + +assert.deepStrictEqual( + result2, + '[null,"undefined",null,false,false,false,"undefined","local",false,null,[null,null,null,null,null,null,null]]', +); +assert(JSON.parse(result2)); + +assert.deepStrictEqual( + result3, + '[null,"string","",false,false,false,"boolean","local",false,false,[null,null,null,null,null,null,null]]', +); +assert(JSON.parse(result3)); + +assert.deepStrictEqual( + result4, + '["binary","boolean",false,false,false,false,"boolean","local",false,"DATETIME",["id","3",null,"test","test","16899","63"],["value","246",null,"test","test","0","63"]]', +); +assert(JSON.parse(result4)); + +assert.deepStrictEqual(result4, result5); +assert(JSON.parse(result5)); + +assert.deepStrictEqual( + result6, + '["binary","boolean",false,true,true,true,"boolean","\\"\\"`\'",true,"#",[":","©",null,"/",",","_","❌"]]', +); +// Ensuring that JSON is valid with invalid delimiters +assert(JSON.parse(result6)); + +assert.deepStrictEqual( + result7, + '["binary","boolean",true,true,true,true,"boolean","local",true,"DATETIME",["id","3",null,"test","test","16899","63"],["value","246",null,"test","test","0","63"]]', +); +assert(JSON.parse(result7)); + +assert.deepStrictEqual(result7, result8); +assert(JSON.parse(result8)); + +assert.deepStrictEqual( + result9, + '["binary","boolean",false,false,false,true,"function","local",false,null,["id","3",null,"test","test","16899","63"]]', +); +assert(JSON.parse(result9)); +assert(JSON.parse(result9)[5] === true); +assert(JSON.parse(result9)[6] === 'function'); +assert(JSON.parse(result9)[9] === null); + +// Testing twice all existent tests needs to return 7 keys, since two of them expects to be the same +assert( + Array.from( + new Set([ + _keyFromFields(test1.type, test1.fields, test1.options, test1.config), + _keyFromFields(test1.type, test1.fields, test1.options, test1.config), + _keyFromFields(test2.type, test2.fields, test2.options, test2.config), + _keyFromFields(test2.type, test2.fields, test2.options, test2.config), + _keyFromFields(test3.type, test3.fields, test3.options, test3.config), + _keyFromFields(test3.type, test3.fields, test3.options, test3.config), + _keyFromFields(test4.type, test4.fields, test4.options, test4.config), + _keyFromFields(test4.type, test4.fields, test4.options, test4.config), + _keyFromFields(test5.type, test5.fields, test5.options, test5.config), + _keyFromFields(test5.type, test5.fields, test5.options, test5.config), + _keyFromFields(test6.type, test6.fields, test6.options, test6.config), + _keyFromFields(test6.type, test6.fields, test6.options, test6.config), + _keyFromFields(test7.type, test7.fields, test7.options, test7.config), + _keyFromFields(test7.type, test7.fields, test7.options, test7.config), + _keyFromFields(test8.type, test8.fields, test8.options, test8.config), + _keyFromFields(test8.type, test8.fields, test8.options, test8.config), + _keyFromFields(test9.type, test9.fields, test9.options, test9.config), + _keyFromFields(test9.type, test9.fields, test9.options, test9.config), + ]), + ).length === 7, +); + +const stringify = JSON.stringify; + +// Overwriting the native `JSON.stringify` +JSON.stringify = (value, replacer, space = 8) => stringify(value, replacer, space); + +// Testing twice all existent tests needs to return 7 keys, since two of them expects to be the same +assert( + Array.from( + new Set([ + result1, + _keyFromFields(test1.type, test1.fields, test1.options, test1.config), + result2, + _keyFromFields(test2.type, test2.fields, test2.options, test2.config), + result3, + _keyFromFields(test3.type, test3.fields, test3.options, test3.config), + result4, + _keyFromFields(test4.type, test4.fields, test4.options, test4.config), + result5, + _keyFromFields(test5.type, test5.fields, test5.options, test5.config), + result6, + _keyFromFields(test6.type, test6.fields, test6.options, test6.config), + result7, + _keyFromFields(test7.type, test7.fields, test7.options, test7.config), + result8, + _keyFromFields(test8.type, test8.fields, test8.options, test8.config), + result9, + _keyFromFields(test9.type, test9.fields, test9.options, test9.config), + ]), + ).length === 7, +);