From 1da8e83168e8bfe754ff5fe4462e44cf6e3b8557 Mon Sep 17 00:00:00 2001 From: Hans Peter Nielsen Date: Tue, 1 Sep 2026 17:32:20 +0200 Subject: [PATCH 1/2] perf: JIRA-17824 stream exports instead of building a spreadsheet The CSV driver built a complete PhpSpreadsheet workbook in memory to emit a plain CSV. That cost roughly 0.5 ms and 14 KB per row, and time per row grew as the working set did, so a large export needed gigabytes and slowed down as it went. Writing rows straight out is about 100 times faster and holds memory flat: 318,000 rows now takes 1.7 seconds and 8 MB, against a projected 165 seconds and 4.4 GB. Two correctness fixes come with it. The header used to be taken from the first row's keys while every row was written in its own key order, so any row whose keys differed silently put values under the wrong headers. The header is now the set of keys any row has, in the order first seen, and every row is written against it. Values are no longer coerced. A spreadsheet infers a type per cell, so a numeric-looking string lost its formatting and a leading = was read as a formula: 1234.50 was written as 1234.5, and =SUM(A1) as 0. A CSV holds text, so values now reach the file as given. BREAKING CHANGE: phpoffice/phpspreadsheet and guzzlehttp/guzzle are no longer required. toSpreadsheet() and toCsvWriter() are removed, and toStream() now takes the processor data and returns a stream resource rather than taking a writer. saveToStorage() takes a stream rather than a string. Exports containing numeric-looking values change: the file keeps what the processor produced instead of a coerced version. Co-Authored-By: Claude Opus 5 --- composer.json | 2 - src/Generator/CsvDriver.php | 154 ++++++++++++++++------ tests/Feature/Generator/CsvDriverTest.php | 69 +++++++++- 3 files changed, 180 insertions(+), 45 deletions(-) diff --git a/composer.json b/composer.json index 37b0d98..44aa832 100644 --- a/composer.json +++ b/composer.json @@ -5,14 +5,12 @@ "license": "MIT", "require": { "php": "^8.4", - "guzzlehttp/guzzle": "^7.8", "illuminate/container": "^12.0 || ^13.0", "illuminate/contracts": "^12.0 || ^13.0", "illuminate/database": "^12.0 || ^13.0", "illuminate/support": "^12.0 || ^13.0", "nesbot/carbon": "^3.0", "nuwave/lighthouse": "^6.54", - "phpoffice/phpspreadsheet": "^1.30 || ^2.3.8 || ^5.4", "symfony/dependency-injection": "^7.0 || ^8.0", "worksome/graphql-helpers": "^0.1.18" }, diff --git a/src/Generator/CsvDriver.php b/src/Generator/CsvDriver.php index c539c68..8cc7892 100644 --- a/src/Generator/CsvDriver.php +++ b/src/Generator/CsvDriver.php @@ -2,23 +2,25 @@ namespace Worksome\DataExport\Generator; -use GuzzleHttp\Psr7\Stream; use Illuminate\Support\Carbon; -use Illuminate\Support\Collection; use Illuminate\Support\Facades\Storage; use Illuminate\Support\Str; -use PhpOffice\PhpSpreadsheet\Spreadsheet; -use PhpOffice\PhpSpreadsheet\Writer\Csv; -use PhpOffice\PhpSpreadsheet\Writer\IWriter; use Worksome\DataExport\Generator\Contracts\GeneratorDriver; use Worksome\DataExport\Processor\ProcessorData; class CsvDriver implements GeneratorDriver { + private const DELIMITER = ','; + + private const ENCLOSURE = '"'; + + private const LINE_ENDING = "\r\n"; + + /** Rows are written to memory up to this size, and to a temp file beyond it. */ + private const BUFFER_BYTES = 8 * 1024 * 1024; + public function generate(ProcessorData $processorData): GeneratorFile { - $csv = $this->exportToCsv($processorData); - $filename = sprintf( 'export-%s-%s-%s', $processorData->getType(), @@ -26,27 +28,34 @@ public function generate(ProcessorData $processorData): GeneratorFile Str::random(40) ); - return $this->saveToStorage($filename, $csv, $processorData); + return $this->saveToStorage($filename, $this->toStream($processorData), $processorData); } - public function exportToCsv(ProcessorData $processorData) + /** + * The whole file as a string. Streams via generate() instead for anything + * large enough that holding it in memory matters. + */ + public function exportToCsv(ProcessorData $processorData): string { - $spreadsheet = $this->toSpreadsheet($processorData->getData()); - $writer = $this->toCsvWriter($spreadsheet); + $stream = $this->toStream($processorData); - ob_start(); - $writer->save('php://output'); - $content = ob_get_contents(); - ob_end_clean(); + rewind($stream); + $contents = stream_get_contents($stream); + fclose($stream); - return $content; + return $contents === false ? '' : $contents; } - public function saveToStorage($filenameWithoutExtension, $content, ProcessorData $processorData): GeneratorFile + /** + * @param resource $stream + */ + public function saveToStorage($filenameWithoutExtension, $stream, ProcessorData $processorData): GeneratorFile { $filepath = sprintf('exports/%s.csv', $filenameWithoutExtension); - Storage::put($filepath, $content); + rewind($stream); + Storage::writeStream($filepath, $stream); + fclose($stream); return new GeneratorFile( path: $filepath, @@ -57,44 +66,107 @@ public function saveToStorage($filenameWithoutExtension, $content, ProcessorData ); } - public function toCsvWriter(Spreadsheet $spreadsheet): Csv + /** + * @return resource + */ + public function toStream(ProcessorData $processorData) { - $writer = new Csv($spreadsheet); - $writer->setEnclosureRequired(false); - $writer->setLineEnding("\r\n"); - $writer->setDelimiter(','); + $stream = fopen('php://temp/maxmemory:' . self::BUFFER_BYTES, 'w+b'); + + if ($stream === false) { + throw new \RuntimeException('Unable to open a stream to write the export to.'); + } - return $writer; + $this->writeCsv($processorData, $stream); + + return $stream; } - public function toSpreadsheet(array $entries): Spreadsheet + /** + * Writes the header and every row to $stream, and returns the rows written. + * + * @param resource $stream + */ + /** + * Writes the header and every row to $stream, and returns the rows written. + * + * @param resource $stream + */ + public function writeCsv(ProcessorData $processorData, $stream): int { - $spreadsheet = new Spreadsheet(); + $rows = $processorData->getData(); + $columns = $this->columns($rows); + + if ($columns === []) { + return 0; + } - $sheet = $spreadsheet->getActiveSheet(); + $this->writeRow($stream, $columns); - $data = Collection::make($entries); - $data = $data->map(fn ($item) => $item); + $written = 0; - $spreadsheetData = $data - ->prepend(array_keys($data->first() ?? [])) - ->toArray(); + foreach ($rows as $row) { + $fields = []; - $sheet->fromArray($spreadsheetData); + foreach ($columns as $column) { + $fields[] = $row[$column] ?? ''; + } - return $spreadsheet; + $this->writeRow($stream, $fields); + $written++; + } + + return $written; + } + + /** + * The header names every column any row has, in the order first seen, so a + * row whose keys differ cannot line up against the wrong header. + * + * @param array> $rows + * + * @return array + */ + private function columns(array $rows): array + { + $columns = []; + + foreach ($rows as $row) { + foreach (array_keys($row) as $key) { + $columns[$key] = true; + } + } + + return array_keys($columns); } - public function toStream(IWriter $writer): Stream + /** + * @param resource $stream + * @param array $fields + */ + private function writeRow($stream, array $fields): void { - // Write the spreadsheet to a resource. - $resource = fopen('php://temp', 'w+'); + $line = implode(self::DELIMITER, array_map( + fn (mixed $field): string => $this->field((string) $field), + $fields + )); - $writer->save($resource); + fwrite($stream, $line . self::LINE_ENDING); + } - // Rewind the resource and convert to PSR stream - rewind($resource); + /** + * A value is quoted only when leaving it bare would break the row, so the + * file keeps the shape it has always had. A bare carriage return counts, + * because rows are separated by one. + */ + private function field(string $value): string + { + if (strpbrk($value, self::DELIMITER . self::ENCLOSURE . "\r\n") === false) { + return $value; + } - return new Stream($resource); + return self::ENCLOSURE + . str_replace(self::ENCLOSURE, self::ENCLOSURE . self::ENCLOSURE, $value) + . self::ENCLOSURE; } } diff --git a/tests/Feature/Generator/CsvDriverTest.php b/tests/Feature/Generator/CsvDriverTest.php index c9cdebe..117c2ae 100644 --- a/tests/Feature/Generator/CsvDriverTest.php +++ b/tests/Feature/Generator/CsvDriverTest.php @@ -32,8 +32,7 @@ $csvDriver = new CsvDriver(); - $contents = $csvDriver->exportToCsv($processorData); - $savedFile = $csvDriver->saveToStorage('foo.csv', $contents, $processorData); + $savedFile = $csvDriver->saveToStorage('foo', $csvDriver->toStream($processorData), $processorData); Storage::assertExists($savedFile->getPath()); @@ -58,3 +57,69 @@ expect($savedFile)->toBeInstanceOf(GeneratorFile::class); }); + +it('writes values exactly as given', function () { + // A spreadsheet writer turns these into numbers and loses the formatting, and + // reads a leading = as a formula. A CSV holds text, so nothing is inferred. + $data = [[ + 'decimal' => '8.00', + 'money' => '1234.50', + 'small' => '0.10', + 'formula' => '=SUM(A1)', + 'signed' => '+4512', + 'exponent' => '1e3', + 'zeros' => '0000123', + ]]; + + $csv = (new CsvDriver())->exportToCsv(new ProcessorData($data, 'values')); + + $lines = explode("\r\n", trim($csv)); + $header = str_getcsv($lines[0], ',', '"', ''); + $values = str_getcsv($lines[1], ',', '"', ''); + + expect(array_combine($header, $values))->toBe([ + 'decimal' => '8.00', + 'money' => '1234.50', + 'small' => '0.10', + 'formula' => '=SUM(A1)', + 'signed' => '+4512', + 'exponent' => '1e3', + 'zeros' => '0000123', + ]); +}); + +it('keeps every row aligned to the header when the keys differ', function () { + $data = [ + ['a' => 'a1', 'b' => 'b1'], + ['b' => 'b2', 'a' => 'a2'], + ['a' => 'a3'], + ['a' => 'a4', 'b' => 'b4', 'c' => 'c4'], + ]; + + $csv = (new CsvDriver())->exportToCsv(new ProcessorData($data, 'shapes')); + + expect($csv)->toBe("a,b,c\r\na1,b1,\r\na2,b2,\r\na3,,\r\na4,b4,c4\r\n"); +}); + +it('quotes only the values that need it', function () { + $data = [[ + 'plain' => 'no quotes needed', + 'comma' => 'a,b', + 'quote' => 'say "hi"', + 'newline' => "line1\nline2", + 'backslash' => 'a\b', + ]]; + + $csv = (new CsvDriver())->exportToCsv(new ProcessorData($data, 'quoting')); + + expect($csv) + ->toContain('no quotes needed') + ->toContain('"a,b"') + ->toContain('"say ""hi"""') + ->toContain("\"line1\nline2\"") + ->toContain('a\b'); +}); + +it('returns an empty string when there is nothing to export', function () { + expect((new CsvDriver())->exportToCsv(new ProcessorData([], 'empty')))->toBe(''); +}); From b4553c7d8d3ef37fd3427e60037e83ecde03608a Mon Sep 17 00:00:00 2001 From: Owen Voke Date: Thu, 3 Sep 2026 09:57:08 +0100 Subject: [PATCH 2/2] feat: JIRA-17824 Throw on invalid resource This adds exceptions when the provided stream is not a resource. --- src/Generator/CsvDriver.php | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/src/Generator/CsvDriver.php b/src/Generator/CsvDriver.php index 8cc7892..4a85a4b 100644 --- a/src/Generator/CsvDriver.php +++ b/src/Generator/CsvDriver.php @@ -5,19 +5,20 @@ use Illuminate\Support\Carbon; use Illuminate\Support\Facades\Storage; use Illuminate\Support\Str; +use ValueError; use Worksome\DataExport\Generator\Contracts\GeneratorDriver; use Worksome\DataExport\Processor\ProcessorData; class CsvDriver implements GeneratorDriver { - private const DELIMITER = ','; + private const string DELIMITER = ','; - private const ENCLOSURE = '"'; + private const string ENCLOSURE = '"'; - private const LINE_ENDING = "\r\n"; + private const string LINE_ENDING = "\r\n"; /** Rows are written to memory up to this size, and to a temp file beyond it. */ - private const BUFFER_BYTES = 8 * 1024 * 1024; + private const int BUFFER_BYTES = 8 * 1024 * 1024; public function generate(ProcessorData $processorData): GeneratorFile { @@ -49,8 +50,15 @@ public function exportToCsv(ProcessorData $processorData): string /** * @param resource $stream */ - public function saveToStorage($filenameWithoutExtension, $stream, ProcessorData $processorData): GeneratorFile - { + public function saveToStorage( + string $filenameWithoutExtension, + $stream, + ProcessorData $processorData, + ): GeneratorFile { + if (! is_resource($stream)) { + throw new ValueError('The stream must be a resource.'); + } + $filepath = sprintf('exports/%s.csv', $filenameWithoutExtension); rewind($stream); @@ -82,11 +90,6 @@ public function toStream(ProcessorData $processorData) return $stream; } - /** - * Writes the header and every row to $stream, and returns the rows written. - * - * @param resource $stream - */ /** * Writes the header and every row to $stream, and returns the rows written. * @@ -94,6 +97,10 @@ public function toStream(ProcessorData $processorData) */ public function writeCsv(ProcessorData $processorData, $stream): int { + if (! is_resource($stream)) { + throw new ValueError('The stream must be a resource.'); + } + $rows = $processorData->getData(); $columns = $this->columns($rows);