Faster in-memory writes and libxml issue detection

infection.json.dist

@@ -23,6 +23,11 @@
             "ignore": [
                 "VeeWee\\Xml\\*Exception::__construct"
             ]
+        },
+        "TrueValue": {
+            "ignore": [
+                "VeeWee\\Xml\\Writer\\Writer::inMemory"
+            ]
         }
     }
 }

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;
 }

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;
+        };
 }

src/Xml/Writer/Writer.php

@@ -8,18 +8,18 @@
 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;
 
 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;
     }

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('<notvalidxml');
+
+                    return 'ok';
+                }
+            );
+            static::fail('Expected an exception');
+        } catch (XmlRuntimeException $exception) {
+            static::assertStringStartsWith('XML issues detected: ' . PHP_EOL . '[FATAL] ', $exception->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('<notvalidxml');
+
+        $result = ErrorHandling\disallow_issues(static fn (): 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('<notvalidxml');
+
+                    return 'ok';
+                }
+            );
+            static::fail('Expected an exception');
+        } catch (XmlRuntimeException) {
+        }
+
+        static::assertFalse(libxml_use_internal_errors());
+        static::assertSame([], libxml_get_errors());
+    }
+
+    public function test_it_restores_the_libxml_state_on_failure(): void
+    {
+        libxml_use_internal_errors(false);
+
+        try {
+            ErrorHandling\disallow_issues(
+                static function (): string {
+                    simplexml_load_string('<notvalidxml');
+
+                    throw new Exception('nonono');
+                }
+            );
+            static::fail('Expected an exception');
+        } catch (XmlRuntimeException $exception) {
+            static::assertStringStartsWith('nonono' . PHP_EOL . '[FATAL] ', $exception->getMessage());
+        }
+
+        static::assertFalse(libxml_use_internal_errors());
+        static::assertSame([], libxml_get_errors());
+    }
 }