Skip to content

Commit 0f1dba9

Browse files
Copilotneilime
andcommitted
Optimize stream-to-string conversion in DiffRendererAdapter
Co-authored-by: neilime <314088+neilime@users.noreply.github.com>
1 parent d83aee5 commit 0f1dba9

3 files changed

Lines changed: 52 additions & 23 deletions

File tree

packages/core/src/reader/reader.adapter.integration.spec.ts

Lines changed: 24 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,15 @@ import { FileRendererAdapter } from '../renderer/file-renderer.adapter.js';
55
import { MarkdownFormatterAdapter } from '../formatter/markdown/markdown-formatter.adapter.js';
66
import { MarkdownTableGenerator } from '../formatter/markdown/markdown-table.generator.js';
77
import { SectionIdentifier } from '../generator/section-generator.adapter.js';
8-
import { readableToBuffer } from './reader.adapter.js';
9-
import { existsSync } from 'node:fs';
8+
import { readableToBuffer, readableToString } from './reader.adapter.js';
109

1110
describe('ReaderAdapter Integration', () => {
1211
let readerAdapter: FileReaderAdapter;
13-
let rendererService: RendererService;
1412
let rendererAdapter: FileRendererAdapter;
1513
let formatterAdapter: MarkdownFormatterAdapter;
1614

1715
beforeEach(() => {
1816
readerAdapter = new FileReaderAdapter();
19-
rendererService = new RendererService(readerAdapter);
2017
rendererAdapter = new FileRendererAdapter();
2118
formatterAdapter = new MarkdownFormatterAdapter(new MarkdownTableGenerator());
2219
});
@@ -42,10 +39,6 @@ describe('ReaderAdapter Integration', () => {
4239
// Verify reading works
4340
expect(readBuffer.toString()).toBe(existingContent);
4441

45-
// Act: Use RendererService to read content (the new way migrations work)
46-
const serviceBuffer = await rendererService.readExistingContent('/test/document.md');
47-
expect(serviceBuffer.toString()).toBe(existingContent);
48-
4942
// Act: Write new content using RendererAdapter (existing pattern)
5043
await rendererAdapter.initialize('/test/document.md', formatterAdapter);
5144
const formattedSection = formatterAdapter.section(SectionIdentifier.Overview, Buffer.from(newSectionContent));
@@ -66,33 +59,45 @@ describe('ReaderAdapter Integration', () => {
6659
// Arrange
6760
mockFs({});
6861

69-
// Act & Assert: RendererService should return empty buffer for non-existent files
70-
const buffer = await rendererService.readExistingContent('/test/nonexistent.md');
71-
expect(buffer.length).toBe(0);
72-
7362
// Act & Assert: ReaderAdapter should let the stream handle the error
7463
const stream = await readerAdapter.getContent('/test/nonexistent.md');
7564
await expect(readableToBuffer(stream)).rejects.toThrow();
7665
});
7766

78-
it('should maintain consistency between old Buffer-based processing and new stream-based reading', async () => {
67+
it('should read content consistently using ReaderAdapter', async () => {
7968
// Arrange
8069
const testContent = '# Test Document\n\nSome content here.\n';
8170
mockFs({
8271
'/test/document.md': testContent,
8372
});
8473

85-
// Act: Read using new ReaderAdapter pattern
74+
// Act: Read using ReaderAdapter pattern
8675
const streamContent = await readerAdapter.getContent('/test/document.md');
8776
const streamBuffer = await readableToBuffer(streamContent);
8877

89-
// Act: Read using RendererService (bridge pattern)
90-
const serviceBuffer = await rendererService.readExistingContent('/test/document.md');
78+
// Assert: Content should match what was written
79+
expect(streamBuffer.toString()).toBe(testContent);
80+
});
81+
82+
it('should convert stream directly to string efficiently', async () => {
83+
// Arrange
84+
const testContent = '# Test Document\n\nSome content here.\n';
85+
mockFs({
86+
'/test/document.md': testContent,
87+
});
88+
89+
// Act: Read using readableToString for direct string conversion
90+
const stream1 = await readerAdapter.getContent('/test/document.md');
91+
const directString = await readableToString(stream1, 'utf-8');
92+
93+
// Act: Read using readableToBuffer then convert to string
94+
const stream2 = await readerAdapter.getContent('/test/document.md');
95+
const bufferThenString = (await readableToBuffer(stream2)).toString('utf-8');
9196

9297
// Assert: Both methods should return identical content
93-
expect(streamBuffer.toString()).toBe(testContent);
94-
expect(serviceBuffer.toString()).toBe(testContent);
95-
expect(streamBuffer.equals(serviceBuffer)).toBe(true);
98+
expect(directString).toBe(testContent);
99+
expect(bufferThenString).toBe(testContent);
100+
expect(directString).toBe(bufferThenString);
96101
});
97102
});
98103
});

packages/core/src/reader/reader.adapter.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,30 @@ export async function readableToBuffer(readable: ReadableContent): Promise<Buffe
2424
resolve(Buffer.concat(chunks));
2525
});
2626

27+
readable.on('error', (err) => {
28+
reject(err);
29+
});
30+
});
31+
}
32+
33+
/**
34+
* Utility function to convert ReadableContent directly to string
35+
* This is more efficient than converting to buffer first when only string is needed
36+
*/
37+
export async function readableToString(readable: ReadableContent, encoding: BufferEncoding = 'utf-8'): Promise<string> {
38+
return new Promise((resolve, reject) => {
39+
const chunks: string[] = [];
40+
41+
readable.setEncoding(encoding);
42+
43+
readable.on('data', (chunk: string) => {
44+
chunks.push(chunk);
45+
});
46+
47+
readable.on('end', () => {
48+
resolve(chunks.join(''));
49+
});
50+
2751
readable.on('error', (err) => {
2852
reject(err);
2953
});

packages/core/src/renderer/diff-renderer.adapter.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { FileRendererAdapter } from './file-renderer.adapter.js';
77
import { existsSync, readFileSync, unlinkSync } from 'node:fs';
88
import { FormatterAdapter } from '../formatter/formatter.adapter.js';
99
import { SectionIdentifier } from '../generator/section-generator.adapter.js';
10-
import { READER_ADAPTER_IDENTIFIER, readableToBuffer } from '../reader/reader.adapter.js';
10+
import { READER_ADAPTER_IDENTIFIER, readableToString } from '../reader/reader.adapter.js';
1111
import type { ReaderAdapter } from '../reader/reader.adapter.js';
1212

1313
@injectable()
@@ -45,14 +45,14 @@ export class DiffRendererAdapter extends AbstractRendererAdapter {
4545
// Produce patch between destination and temp
4646
const destinationContent = existsSync(destination) ? readFileSync(destination, 'utf-8') : '';
4747

48-
// Use ReaderAdapter to read temp file content
48+
// Use ReaderAdapter to read temp file content directly as string
4949
const tempStream = await this.readerAdapter.getContent(temp);
50-
const tempBuffer = await readableToBuffer(tempStream);
50+
const tempContent = await readableToString(tempStream, 'utf-8');
5151

5252
const diff = createPatch(
5353
destination,
5454
destinationContent,
55-
tempBuffer.toString('utf-8'),
55+
tempContent,
5656
);
5757

5858
// Reset initialized parameters

0 commit comments

Comments
 (0)