Skip to content

Commit

Permalink
fix: handle results as streams
Browse files Browse the repository at this point in the history
- Large result sets crash JSON.parse and JSON.stringify calls - invalid string length
- Use read & write streams to handle results
- Use JSONStream to handle reading and writing JSON caches
  • Loading branch information
AriPerkkio committed Feb 7, 2021
1 parent 894a2d3 commit 562538e
Show file tree
Hide file tree
Showing 8 changed files with 110 additions and 59 deletions.
5 changes: 5 additions & 0 deletions lib/JSONStream.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// dominictarr/JSONStream#139
declare module 'JSONStream' {
export function stringify(): NodeJS.ReadWriteStream;
export function parse(pattern: any): NodeJS.ReadWriteStream;
}
98 changes: 67 additions & 31 deletions lib/file-client/result-comparator.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import fs from 'fs';
import * as JSONStream from 'JSONStream';

import {
RESULTS_COMPARISON_CACHE_LOCATION,
Expand All @@ -22,8 +23,10 @@ const EXTENSION = RESULT_PARSER_TO_EXTENSION[config.resultParser];
* - `added`: results which were present in previous scan but not in the current
* - `removed`: results which are present in current scan but not in the previous
*/
export function compareResults(current: Result[]): ComparisonResults {
const previous = readCache();
export async function compareResults(
current: Result[]
): Promise<ComparisonResults> {
const previous = await readCache();

if (previous.length === 0) {
// All results are new
Expand All @@ -49,61 +52,94 @@ export function compareResults(current: Result[]): ComparisonResults {
/**
* Write comparison results to file system and update cache with current results
*/
export function writeComparisonResults(
export async function writeComparisonResults(
comparisonResults: ComparisonResults,
currentScanResults: Result[]
): void {
writeComparisons(comparisonResults);
): Promise<void> {
await writeComparisons(comparisonResults);

if (config.updateComparisonReference) {
updateCache(currentScanResults);
await updateCache(currentScanResults);
}
}

function readCache(): Result[] {
async function readCache(): Promise<Result[]> {
if (!fs.existsSync(RESULTS_COMPARISON_CACHE_LOCATION)) {
return [];
}

const cache = fs.readFileSync(RESULTS_COMPARISON_CACHE_LOCATION, 'utf8');
return JSON.parse(cache);
const results: Result[] = [];

await new Promise((resolve, reject) => {
fs.createReadStream(RESULTS_COMPARISON_CACHE_LOCATION, {
encoding: 'utf8',
})
.pipe(JSONStream.parse('*'))
.on('data', result => results.push(result))
.on('end', resolve)
.on('error', reject);
});

return results;
}

function updateCache(currentScanResults: Result[]): void {
async function updateCache(currentScanResults: Result[]): Promise<void> {
if (fs.existsSync(RESULTS_COMPARISON_CACHE_LOCATION)) {
fs.unlinkSync(RESULTS_COMPARISON_CACHE_LOCATION);
}

fs.writeFileSync(
RESULTS_COMPARISON_CACHE_LOCATION,
JSON.stringify(currentScanResults),
'utf8'
);
return new Promise((resolve, reject) => {
const stream = fs.createWriteStream(RESULTS_COMPARISON_CACHE_LOCATION);
const jstream = JSONStream.stringify()
.pipe(stream)
.on('finish', resolve)
.on('error', reject);

const copy = [...currentScanResults];
while (copy.length > 0) {
const chunk = copy.splice(0, 200);
jstream.write(JSON.stringify(chunk));
}
jstream.end();
stream.end();
});
}

function writeComparisons(comparisonResults: ComparisonResults): void {
async function writeComparisons(
comparisonResults: ComparisonResults
): Promise<void> {
// Directory should always be available but let's handle condition where
// user intentionally removes it during scan.
if (!fs.existsSync(RESULTS_COMPARE_LOCATION)) {
fs.mkdirSync(RESULTS_COMPARE_LOCATION, { recursive: true });
}

for (const type of ComparisonTypes) {
const results = comparisonResults[type];

if (results.length) {
const filename = `${type}${EXTENSION}`;
const content = [
RESULT_COMPARISON_TEMPLATE.header(type),
...results.map(RESULT_COMPARISON_TEMPLATE.results),
].join('\n');

fs.writeFileSync(
`${RESULTS_COMPARE_LOCATION}/${filename}`,
content,
'utf8'
);
}
const results = [...comparisonResults[type]];
if (!results.length) continue;

await new Promise((resolve, reject) => {
const stream = fs
.createWriteStream(
`${RESULTS_COMPARE_LOCATION}/${type}${EXTENSION}`,
{ encoding: 'utf8' }
)
.on('finish', resolve)
.on('error', reject);

stream.write(RESULT_COMPARISON_TEMPLATE.header(type));
stream.write('\n');

while (results.length > 0) {
const chunk = results
.splice(0, 200)
.map(RESULT_COMPARISON_TEMPLATE.results)
.join('\n');
stream.write(chunk);
}

stream.end();
});
}
}

Expand Down
25 changes: 17 additions & 8 deletions lib/file-client/results-writer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,10 @@ function parseMessages(messages: LintMessage[]): Result[] {
/**
* Write results to file at `./eslint-remote-tester-results`
*/
export function writeResults(
export async function writeResults(
messages: LintMessage[],
repository: string
): void {
): Promise<void> {
// Don't write empty files for completely valid results
if (!messages.length) {
return;
Expand All @@ -87,12 +87,21 @@ export function writeResults(
// Construct result file name, e.g. mui-org_material-ui.md
const repositoryOwnerAndName = repository.split('/').join('_');
const fileName = `${repositoryOwnerAndName}${RESULT_EXTENSION}`;
const formattedResults = results.map(RESULT_TEMPLATE).join('\n');

fs.writeFileSync(
`${RESULTS_LOCATION}/${fileName}`,
formattedResults,
'utf8'
);
await new Promise((resolve, reject) => {
const stream = fs
.createWriteStream(`${RESULTS_LOCATION}/${fileName}`, {
encoding: 'utf8',
})
.on('finish', resolve)
.on('error', reject);

while (results.length > 0) {
const chunk = results.splice(0, 200);
const formattedResults = chunk.map(RESULT_TEMPLATE).join('\n');
stream.write(formattedResults);
}
stream.end();
});
}
}
2 changes: 1 addition & 1 deletion lib/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ async function scanRepo(repository: string) {
);

try {
writeResults(results, repository);
await writeResults(results, repository);
} catch (e) {
logger.onWriteFailure(repository, e);
}
Expand Down
4 changes: 2 additions & 2 deletions lib/progress-logger/exit-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export default async function onExit(): Promise<LogMessage[]> {

if (config.compare) {
try {
comparisonResults = compareResults(results);
comparisonResults = await compareResults(results);
ResultsStore.setComparisonResults(comparisonResults);

messages.push({
Expand All @@ -32,7 +32,7 @@ export default async function onExit(): Promise<LogMessage[]> {
level: 'verbose',
});

writeComparisonResults(comparisonResults, results);
await writeComparisonResults(comparisonResults, results);
} catch (e) {
errors.push('Error occured while generating comparison results');
errors.push(e.stack);
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
"bugs": "https://github.com/AriPerkkio/eslint-remote-tester",
"dependencies": {
"@babel/code-frame": "^7.12.13",
"JSONStream": "^1.3.5",
"chalk": "^4.1.0",
"ink": "^3.0.8",
"object-hash": "^2.1.1",
Expand Down
32 changes: 16 additions & 16 deletions test/unit/file-client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,55 +138,55 @@ describe('file-client', () => {
});

describe('compareResults', () => {
test('marks new results as "added"', () => {
test('marks new results as "added"', async () => {
const one = generateResult('1');
const two = generateResult('2');
const three = generateResult('3');
createComparisonCache(three);

const results = compareResults([one, two, three]);
const results = await compareResults([one, two, three]);

expect(results.added).toEqual([one, two]);
expect(results.removed).toHaveLength(0);
});

test('marks disappeared results as "removed"', () => {
test('marks disappeared results as "removed"', async () => {
const one = generateResult('1');
const two = generateResult('2');
const three = generateResult('3');
createComparisonCache(one, two, three);

const results = compareResults([three]);
const results = await compareResults([three]);

expect(results.removed).toEqual([one, two]);
expect(results.added).toHaveLength(0);
});

test('identifies changes in result.__internalHash', () => {
test('identifies changes in result.__internalHash', async () => {
const result = generateResult();
createComparisonCache({ ...result, __internalHash: '1' as any });

const results = compareResults([
const results = await compareResults([
{ ...result, __internalHash: '2' as any },
]);

expect(results.added).toEqual([{ ...result, __internalHash: '2' }]);
});

test('marks all results as "added" when previous results are not found', () => {
test('marks all results as "added" when previous results are not found', async () => {
removeComparisonCache();

const one = generateResult('1');
const two = generateResult('2');
const results = compareResults([one, two]);
const results = await compareResults([one, two]);

expect(results.added).toEqual([one, two]);
expect(results.removed).toHaveLength(0);
});
});

describe('writeComparisonResults', () => {
test('writes comparison results to file system', () => {
test('writes comparison results to file system', async () => {
const template = RESULT_PARSER_TO_COMPARE_TEMPLATE.markdown;

const results = [generateResult('1'), generateResult('2')];
Expand All @@ -195,7 +195,7 @@ describe('file-client', () => {
removed: [generateResult('4')],
};

writeComparisonResults(comparisonResults, results);
await writeComparisonResults(comparisonResults, results);
const { added, removed } = getComparisonResults();

expect(added).toBe(
Expand All @@ -210,31 +210,31 @@ describe('file-client', () => {
);
});

test('updates comparison cache on file system when updateComparisonReference is enabled', () => {
test('updates comparison cache on file system when updateComparisonReference is enabled', async () => {
mockConfig.mockReturnValue({ updateComparisonReference: true });
removeComparisonCache();
const results = [generateResult('1'), generateResult('2')];

writeComparisonResults({ added: [], removed: [] }, results);
await writeComparisonResults({ added: [], removed: [] }, results);

expect(comparisonCacheExists()).toBe(true);
expect(readComparisonCache()).toEqual(results);
});

test('does not update comparison cache when updateComparisonReference is disabled', () => {
test('does not update comparison cache when updateComparisonReference is disabled', async () => {
mockConfig.mockReturnValue({ updateComparisonReference: false });
removeComparisonCache();
const results = [generateResult('1'), generateResult('2')];

writeComparisonResults({ added: [], removed: [] }, results);
await writeComparisonResults({ added: [], removed: [] }, results);

expect(comparisonCacheExists()).toBe(false);
});

test('does not crash if results compare location is not found', () => {
test('does not crash if results compare location is not found', async () => {
removeResultsDirectory();

writeComparisonResults({ added: [], removed: [] }, [
await writeComparisonResults({ added: [], removed: [] }, [
generateResult(),
]);
});
Expand Down
2 changes: 1 addition & 1 deletion yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -949,7 +949,7 @@
"@typescript-eslint/types" "4.6.0"
eslint-visitor-keys "^2.0.0"

JSONStream@^1.0.4:
JSONStream@^1.0.4, JSONStream@^1.3.5:
version "1.3.5"
resolved "https://registry.yarnpkg.com/JSONStream/-/JSONStream-1.3.5.tgz#3208c1f08d3a4d99261ab64f92302bc15e111ca0"
integrity sha512-E+iruNOY8VV9s4JEbe1aNEm6MiszPRr/UfcHMz0TQh1BXSxHK+ASV1R6W4HpjBhSeS+54PIsAMCBmwD06LLsqQ==
Expand Down

0 comments on commit 562538e

Please sign in to comment.