diff --git a/infection.json.dist b/infection.json.dist index 3aa41b8f..f189099d 100644 --- a/infection.json.dist +++ b/infection.json.dist @@ -23,6 +23,11 @@ "ignore": [ "VeeWee\\Xml\\*Exception::__construct" ] + }, + "TrueValue": { + "ignore": [ + "VeeWee\\Xml\\Writer\\Writer::inMemory" + ] } } } \ No newline at end of file diff --git a/src/Xml/ErrorHandling/disallow_issues.php b/src/Xml/ErrorHandling/disallow_issues.php index 407305cc..0a5ed828 100644 --- a/src/Xml/ErrorHandling/disallow_issues.php +++ b/src/Xml/ErrorHandling/disallow_issues.php @@ -6,8 +6,15 @@ use Throwable; use VeeWee\Xml\Exception\RuntimeException; +use function count; +use function libxml_clear_errors; +use function libxml_get_errors; +use function libxml_use_internal_errors; /** + * Does the same as detect_issues(), without building a Result or an IssueCollection when libxml reported nothing. + * Every Writer::write() and every DOM load runs through here, so the happy path has to stay cheap. + * * @template T * * @param callable(): T $run @@ -17,27 +24,29 @@ */ function disallow_issues(callable $run) { - [$result, $issues] = detect_issues($run); - - /** @psalm-suppress ArgumentTypeCoercion - Psalm does not support generic closures... */ - return $result->proceed( - /** - * @param T $value - * @return T - */ - static function ($value) use ($issues) { - if (count($issues)) { - throw RuntimeException::fromIssues('XML issues detected: ', $issues); - } - - return $value; - }, - /** - * @throws RuntimeException - * @return never - */ - static function (Throwable $exception) use ($issues) { - throw RuntimeException::combineExceptionWithIssues($exception, $issues); + $previousErrorReporting = libxml_use_internal_errors(true); + libxml_clear_errors(); + + try { + $value = $run(); + } catch (Throwable $exception) { + $errors = libxml_get_errors(); + libxml_clear_errors(); + libxml_use_internal_errors($previousErrorReporting); + + throw RuntimeException::combineExceptionWithIssues($exception, issue_collection_from_xml_errors($errors)); + } + + $errors = libxml_get_errors(); + libxml_clear_errors(); + libxml_use_internal_errors($previousErrorReporting); + + if ($errors !== []) { + $issues = issue_collection_from_xml_errors($errors); + if (count($issues)) { + throw RuntimeException::fromIssues('XML issues detected: ', $issues); } - ); + } + + return $value; } diff --git a/src/Xml/Internal/configure.php b/src/Xml/Internal/configure.php index 230e938b..3c3986f4 100644 --- a/src/Xml/Internal/configure.php +++ b/src/Xml/Internal/configure.php @@ -3,7 +3,6 @@ namespace VeeWee\Xml\Internal; use Closure; -use Psl\Iter; /** * @psalm-internal \Veewee\Xml @@ -23,15 +22,16 @@ */ function configure(callable ...$stages): Closure { - return static fn ($input) => Iter\reduce( - $stages, + return /** * @param T $input - * @param (callable(T): T) $next - * * @return T */ - static fn (mixed $input, callable $next): mixed => $next($input), - $input - ); + static function (mixed $input) use ($stages): mixed { + foreach ($stages as $stage) { + $input = $stage($input); + } + + return $input; + }; } diff --git a/src/Xml/Writer/Writer.php b/src/Xml/Writer/Writer.php index 5f96cd67..dfbddcbf 100644 --- a/src/Xml/Writer/Writer.php +++ b/src/Xml/Writer/Writer.php @@ -8,9 +8,7 @@ use VeeWee\Xml\Exception\RuntimeException; use XMLWriter; use function VeeWee\Xml\ErrorHandling\disallow_issues; -use function VeeWee\Xml\ErrorHandling\disallow_libxml_false_returns; use function VeeWee\Xml\Internal\configure; -use function VeeWee\Xml\Writer\Opener\memory_opener; use function VeeWee\Xml\Writer\Opener\xml_file_opener; use function VeeWee\Xml\Writer\Opener\xml_stream_opener; @@ -18,8 +16,10 @@ final class Writer { private XMLWriter $writer; - private function __construct(XMLWriter $writer) - { + private function __construct( + XMLWriter $writer, + private readonly bool $inMemory = false, + ) { $this->writer = $writer; } @@ -37,7 +37,7 @@ public static function configure(callable $opener, callable ... $configurators): */ public static function fromUnsafeWriter(XMLWriter $writer, callable ... $configurators): self { - return new self(configure(...$configurators)($writer)); + return new self($configurators !== [] ? configure(...$configurators)($writer) : $writer); } /** @@ -69,10 +69,12 @@ public static function forStream(mixed $stream, callable ... $configurators): se */ public static function inMemory(callable ... $configurators): self { - return self::configure( - memory_opener(), - ...$configurators - ); + if ($configurators !== []) { + // A configurator can replace or reopen the writer, so only an unconfigured one is known to stay in memory. + return self::fromUnsafeWriter(XMLWriter::toMemory(), ...$configurators); + } + + return new self(XMLWriter::toMemory(), true); } /** @@ -87,17 +89,18 @@ public function write(callable $writer): self disallow_issues( static function () use ($cursor) : void { foreach ($cursor as $written) { - disallow_libxml_false_returns( - $written, - 'Could not write the provided XML to the stream.' - ); + if ($written === false) { + throw RuntimeException::withMessage('Could not write the provided XML to the stream.'); + } } } ); - // Flush the content to the file. - // Make sure to keep the buffer in case you are using an in-memory writer. - $this->writer->flush(false); + // Flush the content to the file or stream, and keep the buffer: a memory writer built with configurators also gets here. + // A plain in-memory writer is only read through flush() or outputMemory(), which both flush on their own. + if (!$this->inMemory) { + $this->writer->flush(false); + } return $this; } diff --git a/tests/Xml/ErrorHandling/DisallowIssuesTest.php b/tests/Xml/ErrorHandling/DisallowIssuesTest.php index a155eb65..df5f5feb 100644 --- a/tests/Xml/ErrorHandling/DisallowIssuesTest.php +++ b/tests/Xml/ErrorHandling/DisallowIssuesTest.php @@ -4,10 +4,13 @@ namespace VeeWee\Tests\Xml\ErrorHandling; +use Error; use Exception; use PHPUnit\Framework\TestCase; use RuntimeException; use VeeWee\Xml\ErrorHandling; +use VeeWee\Xml\Exception\RuntimeException as XmlRuntimeException; +use function libxml_get_errors; use function libxml_use_internal_errors; use function simplexml_load_string; @@ -54,4 +57,117 @@ static function () use ($exception) { } ); } + + public function test_it_adds_the_detected_issues_to_the_message(): void + { + try { + ErrorHandling\disallow_issues( + static function (): string { + simplexml_load_string('getMessage()); + static::assertNull($exception->getPrevious()); + } + } + + public function test_it_wraps_a_failure_with_its_code_and_previous_exception(): void + { + $exception = new Exception('nonono', 42); + + try { + ErrorHandling\disallow_issues( + static function () use ($exception) { + throw $exception; + } + ); + static::fail('Expected an exception'); + } catch (XmlRuntimeException $actual) { + static::assertSame('nonono' . PHP_EOL, $actual->getMessage()); + static::assertSame(42, $actual->getCode()); + static::assertSame($exception, $actual->getPrevious()); + } + } + + public function test_it_wraps_errors_that_are_not_exceptions(): void + { + $error = new Error('broken'); + + try { + ErrorHandling\disallow_issues( + static function () use ($error) { + throw $error; + } + ); + static::fail('Expected an exception'); + } catch (XmlRuntimeException $actual) { + static::assertSame('broken' . PHP_EOL, $actual->getMessage()); + static::assertSame($error, $actual->getPrevious()); + } + } + + public function test_it_does_not_use_previously_occured_errors(): void + { + libxml_use_internal_errors(true); + simplexml_load_string(' 'ok'); + + static::assertSame('ok', $result); + } + + public function test_it_restores_the_libxml_state_on_success(): void + { + libxml_use_internal_errors(false); + + ErrorHandling\disallow_issues(static fn (): string => 'ok'); + + static::assertFalse(libxml_use_internal_errors()); + static::assertSame([], libxml_get_errors()); + } + + public function test_it_restores_the_libxml_state_on_detected_issues(): void + { + libxml_use_internal_errors(false); + + try { + ErrorHandling\disallow_issues( + static function (): string { + simplexml_load_string('getMessage()); + } + + static::assertFalse(libxml_use_internal_errors()); + static::assertSame([], libxml_get_errors()); + } } diff --git a/tests/Xml/Writer/WriterTest.php b/tests/Xml/Writer/WriterTest.php index 97cafdbe..fa878ad7 100644 --- a/tests/Xml/Writer/WriterTest.php +++ b/tests/Xml/Writer/WriterTest.php @@ -4,6 +4,7 @@ namespace VeeWee\Tests\Xml\Writer; +use LogicException; use PHPUnit\Framework\TestCase; use VeeWee\Tests\Xml\Helper\FillFileTrait; use VeeWee\Tests\Xml\Helper\TmpFileTrait; @@ -51,6 +52,50 @@ public function test_it_can_open_a_stream(): void static::assertSame('hello', stream_get_contents($handle)); } + public function test_it_flushes_a_stream_after_every_write(): void + { + [$_, $handle] = $this->fillFile(''); + $writer = Writer::forStream($handle); + + $writer->write(raw('hello')); + rewind($handle); + static::assertSame('hello', stream_get_contents($handle)); + + $writer->write(raw(' world')); + rewind($handle); + static::assertSame('hello world', stream_get_contents($handle)); + } + + public function test_it_flushes_a_stream_returned_by_an_in_memory_configurator(): void + { + [$_, $handle] = $this->fillFile(''); + + $writer = Writer::inMemory( + static fn (XMLWriter $writer): XMLWriter => XMLWriter::toStream($handle), + ); + + $writer->write(raw('hello')); + rewind($handle); + + static::assertSame('hello', stream_get_contents($handle)); + } + + public function test_it_flushes_an_in_memory_writer_that_a_configurator_reopened(): void + { + $this->createTmpFile(static function (string $path): void { + $writer = Writer::inMemory( + static function (XMLWriter $writer) use ($path): XMLWriter { + $writer->openUri($path); + return $writer; + }, + ); + + $writer->write(element('root')); + + self::assertXmlStringEqualsXmlFile($path, ''); + }); + } + public function test_it_can_configure_a_writer(): void { $this->createTmpFile(static function (string $path): void { @@ -62,6 +107,34 @@ public function test_it_can_configure_a_writer(): void } + public function test_it_applies_configurators_in_order(): void + { + $actual = Writer::inMemory( + static function (XMLWriter $writer): XMLWriter { + $writer->writeRaw('a'); + return $writer; + }, + static function (XMLWriter $writer): XMLWriter { + $writer->writeRaw('b'); + return $writer; + }, + )->write(raw('c'))->map(memory_output()); + + static::assertSame('abc', $actual); + } + + public function test_it_uses_the_writer_returned_by_a_configurator(): void + { + $replacement = XMLWriter::toMemory(); + $replacement->writeRaw('replaced-'); + + $actual = Writer::inMemory( + static fn (XMLWriter $writer): XMLWriter => $replacement, + )->write(raw('hello'))->map(memory_output()); + + static::assertSame('replaced-hello', $actual); + } + public function test_it_can_fluently_write_multiple_times(): void { $this->createTmpFile(static function (string $path): void { @@ -88,6 +161,19 @@ public function test_it_can_use_an_unsafe_writer(): void static::assertXmlStringEqualsXmlString($result, ''); } + public function test_it_can_configure_an_unsafe_writer(): void + { + $actual = Writer::fromUnsafeWriter( + XMLWriter::toMemory(), + static function (XMLWriter $writer): XMLWriter { + $writer->writeRaw('configured-'); + return $writer; + }, + )->write(raw('hello'))->map(memory_output()); + + static::assertSame('configured-hello', $actual); + } + public function test_it_throws_error_on_not_initialized(): void { $this->expectExceptionMessage('Invalid or uninitialized XMLWriter object'); @@ -111,6 +197,28 @@ static function () { }); } + public function test_it_wraps_the_invalid_write_exception_and_stops_writing(): void + { + $writer = Writer::inMemory(); + + try { + $writer->write( + static function () { + yield true; + yield false; + throw new LogicException('The writer should stop after the first false.'); + } + ); + static::fail('Expected an exception'); + } catch (RuntimeException $exception) { + static::assertSame('Could not write the provided XML to the stream.' . PHP_EOL, $exception->getMessage()); + $previous = $exception->getPrevious(); + static::assertInstanceOf(RuntimeException::class, $previous); + static::assertSame('Could not write the provided XML to the stream.', $previous->getMessage()); + static::assertNull($previous->getPrevious()); + } + } + public function test_it_can_map_writer_to_something_else(): void {