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.

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(yieldable-parser): parseAsync sometimes blocks for a *long* time #36 #37

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 12 commits
Commits
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
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@
"index.js",
"yieldable-stringify.js",
"yieldable-parser.js",
"test/"
"test/",
"utils/"
],
"devDependencies": {
"eslint": "^6.6.0",
Expand Down
1 change: 1 addition & 0 deletions test/big-json-with-string.txt
Copy link
Author

Choose a reason for hiding this comment

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

This one may require attention into the JSON info. I copied and pasted from the shared file in the issue

Large diffs are not rendered by default.

54 changes: 54 additions & 0 deletions test/test-index-of-generator.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
const tap = require('tap');
const { indexOfGenerator } = require('../utils/index-of-generator');

function promisifyGenerator(generator){
return (...args) => {
const gen = generator(...args);

function handleNext(resolvedValue){
if(resolvedValue.done){
return Promise.resolve(resolvedValue.value);
}
return Promise.resolve(gen.next(resolvedValue.value)).then(handleNext);
}


return Promise.resolve(gen.next())
.then((resolvedValue) => handleNext(resolvedValue), (error) => Promise.reject(gen.throw(error)));
}
}

const asyncIndexOf = promisifyGenerator(indexOfGenerator);

const stringAsArray = ['s', 'o', 'm', 'e'];
const text = stringAsArray.join('');

tap.test('should return -1 if not found due to initialIndex', async () => {
const index = await asyncIndexOf(text, 's', 1);
const syncIndexOf = text.indexOf('s', 1);
tap.equal(index, syncIndexOf);
})

tap.test('should return proper index', async () => {
const index = await asyncIndexOf(text, 'e');
const syncIndexOf = text.indexOf('e');
tap.equal(index, syncIndexOf);
})

tap.test('should return proper test with initialIndex', async () => {
const index = await asyncIndexOf(text, 'e', 2);
const syncIndexOf = text.indexOf('e', 2);
tap.equal(index, syncIndexOf);
})

tap.test('should return correct indexOf if initialIndex is same', async () => {
const index = await asyncIndexOf(text, 'e', 3);
const syncIndexOf = text.indexOf('e', 3);
tap.equal(index, syncIndexOf);
})

tap.test('should return -1 if not found', async () => {
const index = await asyncIndexOf(text, 'x');
const syncIndexOf = text.indexOf('x');
tap.equal(index, syncIndexOf);
})
15 changes: 15 additions & 0 deletions test/test-parse-big-escaped-string.js
lukas8219 marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
'use strict';

const yj = require('../index');
const tap = require('tap');
const { readFileSync } = require('fs');
const str = readFileSync('./test/big-json-with-string.txt');
const nodeJSNativeParse = JSON.parse(str);

// Make sure the API works well without the optional parameters.
yj.parseAsync(str, (err, obj) => {
if (!err)
tap.equal(obj.text, nodeJSNativeParse.text, 'Failed to parse big string message - omitting logs', { diagnostic: false });
else
tap.fail(err);
});
26 changes: 26 additions & 0 deletions utils/index-of-generator.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
//write more performant option for indexOf
//if more than 50k characters, use BHM Algorithm
//consider a big string to be 10k characters
const MAX_STRING_SIZE = 10000;
lukas8219 marked this conversation as resolved.
Show resolved Hide resolved

module.exports.indexOfGenerator = function* indexOfGenerator(stringInput, searchInput, initialIndex = 0) {
if (!stringInput || !stringInput.length) {
return -1;
}
stringInput = stringInput.slice(initialIndex);
let innerAt = 0;
let index = -1;
while (true) {
const at = Math.max(innerAt - 1, 0);
yield chunk = stringInput.slice(at, innerAt + MAX_STRING_SIZE);
if (!chunk.length) {
return index;
}
const indexOf = chunk.indexOf(searchInput);
if (indexOf !== -1) {
yield index = indexOf + at + initialIndex;
return index;
}
innerAt += chunk.length + 1;
}
}
11 changes: 6 additions & 5 deletions yieldable-parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
* Multiple authors (IBM Corp.) - initial implementation and documentation
***************************************************************************/
'use strict';
const { indexOfGenerator } = require('./utils/index-of-generator');

/**
* This method parses a JSON text to produce an object or array.
Expand Down Expand Up @@ -64,14 +65,14 @@ let parseWrapper = (text, reviver, intensity, cb) => {
};

// Process strings specially.
let normalizeUnicodedString = () => {
let normalizeUnicodedString = function*() {
let inQuotes = ' ';
let tempIndex = at;
let index = 0;
let slash = 0;
let c = '"';
while (c) {
index = parseStr.indexOf('"', tempIndex + 1);
index = yield * indexOfGenerator(parseStr, '"', tempIndex + 1);
tempIndex = index;
ch = parseStr.charAt(tempIndex - 1);
while (ch === '\\') {
Expand All @@ -88,7 +89,7 @@ let parseWrapper = (text, reviver, intensity, cb) => {
}

// When parsing string values, look for " and \ characters.
index = inQuotes.indexOf('\\');
index = yield * indexOfGenerator(inQuotes, '\\');
while (index >= 0) {
let escapee = {
'"': '"',
Expand Down Expand Up @@ -124,7 +125,7 @@ let parseWrapper = (text, reviver, intensity, cb) => {
at = index + 1;
} else
break;
index = inQuotes.indexOf('\\', at);
index = yield * indexOfGenerator(inQuotes, '\\', at);
}
at = 0;
return inQuotes;
Expand Down Expand Up @@ -225,7 +226,7 @@ let parseWrapper = (text, reviver, intensity, cb) => {
return inQuotes;
} else {
seek();
return normalizeUnicodedString();
return yield * normalizeUnicodedString();
}
case '0':
case '1':
Expand Down