diff --git a/.github/workflows/throws.yaml b/.github/workflows/throws.yaml new file mode 100644 index 0000000..4609137 --- /dev/null +++ b/.github/workflows/throws.yaml @@ -0,0 +1,39 @@ +name: Throws + +on: [push, pull_request] +jobs: + run: + runs-on: ${{ matrix.operating-system }} + strategy: + matrix: + operating-system: [ubuntu-latest] + php-versions: [ '8.4' ] + fail-fast: false + name: Intermediate throws on PHP ${{ matrix.php-versions }} @ ${{ matrix.operating-system }} + steps: + - name: Checkout + uses: actions/checkout@master + - name: Install PHP + uses: shivammathur/setup-php@master + with: + php-version: ${{ matrix.php-versions }} + tools: 'composer:v2' + extensions: mbstring, posix, dom, soap + coverage: xdebug + - name: Install dependencies + run: composer update --prefer-dist --no-progress --no-suggest + - name: Run the throw counting tests + run: composer test:throws + - name: Report intermediate throws over the whole suite + run: | + status=0 + composer throws:report > throws-report.txt 2>&1 || status=$? + cat throws-report.txt + { + echo '## Intermediate throws' + echo + echo '```' + sed -n '/^Intermediate throws:/,$p' throws-report.txt + echo '```' + } >> "$GITHUB_STEP_SUMMARY" + exit $status diff --git a/benchmarks/ComplexTypeBenchTrait.php b/benchmarks/ComplexTypeBenchTrait.php index fac22d7..5b9acc3 100644 --- a/benchmarks/ComplexTypeBenchTrait.php +++ b/benchmarks/ComplexTypeBenchTrait.php @@ -23,6 +23,7 @@ trait ComplexTypeBenchTrait /** @var list */ protected array $items; protected SoapResponse $literalResponse; + protected SoapResponse $literalSparseResponse; protected SoapResponse $encodedResponse; protected BenchSoapClient $extSoapLiteral; @@ -77,6 +78,13 @@ public function setUp(): void $this->encodedResponse = new SoapResponse( $this->encodedDriver->encode('test', [$this->singleItem])->getRequest() ); + // Real-world responses often leave most elements out: only 2 of the 12 elements are present here. + $this->literalSparseResponse = new SoapResponse( + '' + . '' + . '42
Springfield
' + . '' + ); // ext-soap $this->extSoapLiteral = $this->buildExtSoapClient('literal'); diff --git a/benchmarks/DecodeBench.php b/benchmarks/DecodeBench.php index 1ace581..cfd113b 100644 --- a/benchmarks/DecodeBench.php +++ b/benchmarks/DecodeBench.php @@ -41,6 +41,22 @@ public function benchLiteral_x500(): void } } + #[Revs(100)] + #[Groups(['literal', 'php-soap', 'x1', 'sparse'])] + public function benchLiteralSparse_x1(): void + { + $this->literalDriver->decode('test', $this->literalSparseResponse); + } + + #[Groups(['literal', 'php-soap', 'x500', 'sparse'])] + public function benchLiteralSparse_x500(): void + { + $response = $this->literalSparseResponse; + foreach ($this->items as $item) { + $this->literalDriver->decode('test', $response); + } + } + // -- php-soap/encoding: encoded -- #[Revs(100)] diff --git a/composer.json b/composer.json index a1e258e..bc4c53e 100644 --- a/composer.json +++ b/composer.json @@ -52,6 +52,10 @@ "guzzlehttp/guzzle": "^7.8", "phpbench/phpbench": "^1.6" }, + "scripts": { + "test:throws": "@php -d xdebug.mode=develop vendor/bin/phpunit --group no-throws --fail-on-skipped", + "throws:report": "@php -d xdebug.mode=develop vendor/bin/phpunit --no-progress --no-results --do-not-fail-on-risky --extension 'Soap\\Encoding\\Test\\Support\\Throws\\ThrowAttributionExtension'" + }, "config": { "allow-plugins": { "php-http/discovery": true diff --git a/tests/Fixture/Model/Address.php b/tests/Fixture/Model/Address.php new file mode 100644 index 0000000..ac42d33 --- /dev/null +++ b/tests/Fixture/Model/Address.php @@ -0,0 +1,11 @@ +|null */ + public ?array $tags; + public ?string $code; + public ?int $rank; +} diff --git a/tests/Integration/IntermediateThrowScenariosTest.php b/tests/Integration/IntermediateThrowScenariosTest.php new file mode 100644 index 0000000..6f1b0d2 --- /dev/null +++ b/tests/Integration/IntermediateThrowScenariosTest.php @@ -0,0 +1,240 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + XSD; + + /** + * @param Closure(): mixed $operation + */ + #[DataProvider('provideScenarios')] + public function test_it_encodes_and_decodes_the_scenario(Closure $operation, mixed $expected): void + { + static::assertEquals($expected, $operation()); + } + + /** + * @return iterable + */ + public static function provideScenarios(): iterable + { + $literal = self::driver('tns:itemType', 'literal'); + $typed = self::driver( + 'tns:itemType', + 'literal', + EncoderRegistry::default() + ->addClassMap('http://test-uri/', 'itemType', Item::class) + ->addClassMap('http://test-uri/', 'addressType', Address::class), + ); + + yield 'decode sparse response into stdClass' => [ + 'operation' => static fn (): mixed => $literal->decode( + 'test', + self::response('42
C
'), + ), + 'expected' => (object) [ + 'code' => null, + 'rank' => null, + 'id' => 42, + 'name' => null, + 'price' => null, + 'active' => null, + 'address' => (object) ['street' => null, 'city' => 'C', 'zip' => null], + 'tags' => [], + ], + ]; + + yield 'decode nil element into stdClass' => [ + 'operation' => static fn (): mixed => $literal->decode( + 'test', + self::response('42
'), + ), + 'expected' => (object) [ + 'code' => null, + 'rank' => null, + 'id' => 42, + 'name' => null, + 'price' => null, + 'active' => null, + 'address' => null, + 'tags' => [], + ], + ]; + + yield 'decode single occurrence of a repeating element' => [ + 'operation' => static fn (): mixed => $literal->decode( + 'test', + self::response('1a'), + ), + 'expected' => (object) [ + 'code' => null, + 'rank' => null, + 'id' => 1, + 'name' => null, + 'price' => null, + 'active' => null, + 'address' => null, + 'tags' => ['a'], + ], + ]; + + yield 'encode stdClass with absent properties' => [ + 'operation' => static fn (): string => $literal->encode('test', [(object) ['id' => 1]])->getRequest(), + 'expected' => self::literalRequest( + '1
' + ), + ]; + + yield 'encode sparse array input' => [ + 'operation' => static fn (): string => $literal->encode('test', [['id' => 1, 'address' => ['city' => 'C']]])->getRequest(), + 'expected' => self::literalRequest( + '1
C
' + ), + ]; + + yield 'encode uninitialised typed properties' => [ + 'operation' => static function () use ($typed): string { + $item = new Item(); + $item->id = 42; + + return $typed->encode('test', [$item])->getRequest(); + }, + 'expected' => self::literalRequest( + '42
' + ), + ]; + + $untyped = self::driver('tns:untypedValues', 'literal'); + yield 'decode untyped values' => [ + 'operation' => static fn (): mixed => $untyped->decode( + 'test', + self::response('51.5trueword'), + ), + 'expected' => (object) ['integer' => 5, 'float' => 1.5, 'boolean' => true, 'string' => 'word'], + ]; + + $soapEncArray = self::driver('tns:intArray', 'encoded'); + yield 'decode soap-enc array of a builtin type' => [ + 'operation' => static fn (): mixed => $soapEncArray->decode( + 'test', + self::response('12'), + ), + 'expected' => [1, 2], + ]; + } + + private static function driver(string $partType, string $use, ?EncoderRegistry $registry = null): Driver + { + $schema = self::ITEM_SCHEMA; + $encodingStyle = $use === 'encoded' ? ' encodingStyle="http://schemas.xmlsoap.org/soap/encoding/"' : ''; + $wsdl = << + + + + + {$schema} + + + + + + + + + + + + + + + + + + + WSDL; + + $wsdlObject = (new Wsdl1Reader(new CallbackLoader(static fn (): string => $wsdl)))('file.wsdl'); + + return Driver::createFromMetadata( + (new Wsdl1MetadataProvider($wsdlObject))->getMetadata(), + $wsdlObject->namespaces, + $registry ?? EncoderRegistry::default(), + ); + } + + private static function response(string $testParam): SoapResponse + { + return new SoapResponse( + '' + .''.$testParam.'' + .'' + ); + } + + private static function literalRequest(string $testParam): string + { + return '' + .'' + .''.$testParam.'' + .''."\n"; + } +} diff --git a/tests/Support/Throws/AssertsNoThrows.php b/tests/Support/Throws/AssertsNoThrows.php new file mode 100644 index 0000000..452f8d2 --- /dev/null +++ b/tests/Support/Throws/AssertsNoThrows.php @@ -0,0 +1,39 @@ +describe(); + $grouped[$key] = ($grouped[$key] ?? 0) + 1; + } + arsort($grouped); + + $lines = []; + foreach ($grouped as $description => $count) { + $lines[] = sprintf('%5dx %s', $count, $description); + } + + static::assertCount( + 0, + $events, + sprintf("Expected no exceptions to be thrown, got %d:\n%s", count($events), implode("\n", $lines)), + ); + } +} diff --git a/tests/Support/Throws/ThrowAttribution.php b/tests/Support/Throws/ThrowAttribution.php new file mode 100644 index 0000000..6921047 --- /dev/null +++ b/tests/Support/Throws/ThrowAttribution.php @@ -0,0 +1,146 @@ +|null */ + private ?array $previousIni = null; + private string $currentTest = ''; + private ?string $currentExclusion = null; + private int $all = 0; + private int $outsideSrc = 0; + /** @var array{expectException: int, catch: int} */ + private array $excluded = ['expectException' => 0, 'catch' => 0]; + /** @var array> */ + private array $fileLines = []; + /** @var array}> */ + private array $sites = []; + /** @var array */ + private array $tests = []; + /** @var array */ + private array $unsuccessfulTests = []; + + public function testStarted(Test $test): void + { + $this->currentTest = $test->id(); + $this->currentExclusion = $test instanceof TestMethod ? $this->exclusion($test) : null; + $this->previousIni ??= ThrowCounter::enable(); + } + + public function testPrinted(string $output): void + { + foreach (XdebugExceptionTraceParser::parse($output) as $event) { + $this->all++; + if ($this->currentExclusion !== null) { + $this->excluded[$this->currentExclusion]++; + continue; + } + if ($event->srcCaller() === null) { + $this->outsideSrc++; + continue; + } + + $site = $event->describe(); + $this->sites[$site] ??= ['throws' => 0, 'tests' => []]; + $this->sites[$site]['throws']++; + $this->sites[$site]['tests'][$this->currentTest] = true; + $this->tests[$this->currentTest] = true; + } + } + + public function testUnsuccessful(Test $test): void + { + $this->unsuccessfulTests[$test->id()] = true; + $this->stopTracing(); + } + + /** + * Called as soon as the outcome of a test is known: PHPUnit can throw again while it handles a skipped or + * failed test, after the output buffer of the test is closed, which would print the trace to the console. + */ + public function stopTracing(): void + { + if ($this->previousIni !== null) { + ThrowCounter::restore($this->previousIni); + $this->previousIni = null; + } + } + + public function render(): string + { + $sites = $this->sites; + uasort($sites, static fn (array $a, array $b): int => $b['throws'] <=> $a['throws']); + + $lines = [ + sprintf( + 'Intermediate throws: %d in %d tests', + $this->all - $this->outsideSrc - $this->excluded['expectException'] - $this->excluded['catch'], + count($this->tests), + ), + sprintf( + 'Not counted: %d outside src/, %d in tests using expectException(), %d in tests catching an exception themselves (%d throws in total)', + $this->outsideSrc, + $this->excluded['expectException'], + $this->excluded['catch'], + $this->all, + ), + ]; + if ($this->unsuccessfulTests !== []) { + $lines[] = sprintf('Warning: %d tests failed or errored, run composer test to see them', count($this->unsuccessfulTests)); + } + $lines[] = ''; + $lines[] = sprintf('%7s %6s %s', 'throws', 'tests', 'site'); + foreach ($sites as $site => $count) { + $lines[] = sprintf('%7d %6d %s', $count['throws'], count($count['tests']), $site); + } + + return implode("\n", $lines)."\n"; + } + + /** + * PHPUnit does not report expectException() as an event, so the test method body is checked instead. + * A test that catches one of the exceptions this package reports errors with is left out as well, + * and counted separately in the report so the effect of that heuristic stays visible. + * + * @return 'expectException'|'catch'|null + */ + private function exclusion(TestMethod $test): ?string + { + $method = new ReflectionMethod($test->className(), $test->methodName()); + $file = $method->getFileName(); + $start = $method->getStartLine(); + $end = $method->getEndLine(); + if ($file === false || $start === false || $end === false) { + return null; + } + + $this->fileLines[$file] ??= file($file) ?: []; + $body = implode('', array_slice($this->fileLines[$file], $start - 1, $end - $start + 1)); + + return match (true) { + preg_match('/\bexpectException\w*\s*\(/', $body) === 1 => 'expectException', + preg_match( + '/\bcatch\s*\(\s*\\\\?(?:[\w\\\\]*\\\\)?(?:EncodingException|RestrictionException|SoapFaultException|Throwable|Exception)(?![\w\\\\])/', + $body, + ) === 1 => 'catch', + default => null, + }; + } +} diff --git a/tests/Support/Throws/ThrowAttributionExtension.php b/tests/Support/Throws/ThrowAttributionExtension.php new file mode 100644 index 0000000..cd63ecd --- /dev/null +++ b/tests/Support/Throws/ThrowAttributionExtension.php @@ -0,0 +1,117 @@ +registerSubscribers( + new class($attribution) implements PreparationStartedSubscriber { + public function __construct(private readonly ThrowAttribution $attribution) + { + } + + public function notify(PreparationStarted $event): void + { + $this->attribution->testStarted($event->test()); + } + }, + new class($attribution) implements PrintedUnexpectedOutputSubscriber { + public function __construct(private readonly ThrowAttribution $attribution) + { + } + + public function notify(PrintedUnexpectedOutput $event): void + { + $this->attribution->testPrinted($event->output()); + } + }, + new class($attribution) implements FailedSubscriber { + public function __construct(private readonly ThrowAttribution $attribution) + { + } + + public function notify(Failed $event): void + { + $this->attribution->testUnsuccessful($event->test()); + } + }, + new class($attribution) implements ErroredSubscriber { + public function __construct(private readonly ThrowAttribution $attribution) + { + } + + public function notify(Errored $event): void + { + $this->attribution->testUnsuccessful($event->test()); + } + }, + new class($attribution) implements SkippedSubscriber { + public function __construct(private readonly ThrowAttribution $attribution) + { + } + + public function notify(Skipped $event): void + { + $this->attribution->stopTracing(); + } + }, + new class($attribution) implements FinishedSubscriber { + public function __construct(private readonly ThrowAttribution $attribution) + { + } + + public function notify(Finished $event): void + { + $this->attribution->stopTracing(); + } + }, + new class($attribution) implements ExecutionFinishedSubscriber { + public function __construct(private readonly ThrowAttribution $attribution) + { + } + + public function notify(ExecutionFinished $event): void + { + $this->attribution->stopTracing(); + print "\n".$this->attribution->render(); + } + }, + ); + } +} diff --git a/tests/Support/Throws/ThrowCounter.php b/tests/Support/Throws/ThrowCounter.php new file mode 100644 index 0000000..0a87776 --- /dev/null +++ b/tests/Support/Throws/ThrowCounter.php @@ -0,0 +1,83 @@ + + */ + public static function enable(): array + { + if (!self::isAvailable()) { + throw new RuntimeException('Counting throws needs Xdebug with xdebug.mode=develop.'); + } + + $previous = []; + foreach ([ + 'display_errors' => '1', + 'log_errors' => '0', + 'html_errors' => '0', + 'xdebug.var_display_max_children' => '0', + 'xdebug.var_display_max_data' => '0', + 'xdebug.var_display_max_depth' => '0', + 'xdebug.show_exception_trace' => '1', + ] as $key => $value) { + $old = ini_set($key, $value); + if ($old !== false) { + $previous[$key] = $old; + } + } + + return $previous; + } + + /** + * @param array $previous + */ + public static function restore(array $previous): void + { + foreach ($previous as $key => $value) { + ini_set($key, $value); + } + } + + /** + * @return list + */ + public static function record(callable $operation): array + { + $previous = self::enable(); + ob_start(); + try { + $operation(); + } finally { + $output = (string) ob_get_clean(); + self::restore($previous); + } + + return XdebugExceptionTraceParser::parse($output); + } +} diff --git a/tests/Support/Throws/ThrowEvent.php b/tests/Support/Throws/ThrowEvent.php new file mode 100644 index 0000000..5ed6a8a --- /dev/null +++ b/tests/Support/Throws/ThrowEvent.php @@ -0,0 +1,75 @@ + $class + * @param list $frames Outermost call first, as Xdebug prints them. + */ + public function __construct( + public readonly string $class, + public readonly string $message, + public readonly string $file, + public readonly int $line, + public readonly array $frames, + ) { + } + + public function site(): string + { + return self::relative($this->file).':'.$this->line; + } + + /** + * The innermost place in src/ that led to this throw, or null when the stack never passes through src/. + */ + public function srcCaller(): ?string + { + if (self::isSrc($this->file)) { + return $this->site(); + } + + foreach (array_reverse($this->frames) as $frame) { + if (self::isSrc($frame->file)) { + return self::relative($frame->file).':'.$frame->line.' -> '.$frame->function; + } + } + + return null; + } + + public function describe(): string + { + $caller = $this->srcCaller(); + + return sprintf('%s at %s%s', $this->class, $this->site(), $caller !== null ? ' via '.$caller : ''); + } + + private static function isSrc(string $file): bool + { + return str_starts_with($file, self::root().'/src/'); + } + + private static function relative(string $file): string + { + $root = self::root().'/'; + + return str_starts_with($file, $root) ? substr($file, strlen($root)) : $file; + } + + private static function root(): string + { + return dirname(__DIR__, 3); + } +} diff --git a/tests/Support/Throws/ThrowFrame.php b/tests/Support/Throws/ThrowFrame.php new file mode 100644 index 0000000..08d87f1 --- /dev/null +++ b/tests/Support/Throws/ThrowFrame.php @@ -0,0 +1,14 @@ + + */ + public static function parse(string $output): array + { + $events = []; + $header = null; + $event = null; + + foreach (explode("\n", $output) as $line) { + if ($event !== null) { + if (preg_match('/^\s+[\d.]+\s+\d+\s+\d+\. (.+?)\(.*\) (.+):(\d+)$/', $line, $frame)) { + $event['frames'][] = new ThrowFrame($frame[1], $frame[2], (int) $frame[3]); + continue; + } + + $events[] = self::toEvent($event); + $event = null; + } + + $startsTrace = preg_match('/^([A-Za-z_\\\\][\w\\\\]*): /', $line, $match) === 1 + && is_a($match[1], Throwable::class, true); + + if ($header !== null && !$startsTrace) { + if ($line === 'Call Stack:') { + $event = self::parseHeader($header); + $header = null; + } else { + $header .= "\n".$line; + } + continue; + } + + if ($startsTrace) { + $header = $line; + } + } + + if ($event !== null) { + $events[] = self::toEvent($event); + } + + return $events; + } + + /** + * @param array{class: class-string, message: string, file: string, line: int, frames: list} $event + */ + private static function toEvent(array $event): ThrowEvent + { + return new ThrowEvent($event['class'], $event['message'], $event['file'], $event['line'], $event['frames']); + } + + /** + * @return array{class: class-string, message: string, file: string, line: int, frames: list}|null + */ + private static function parseHeader(string $header): ?array + { + if (!preg_match('/^([A-Za-z_\\\\][\w\\\\]*): (.*) in (.+) on line (\d+)\s*$/s', $header, $match) + || !is_a($match[1], Throwable::class, true) + ) { + return null; + } + + return [ + 'class' => $match[1], + 'message' => trim($match[2]), + 'file' => $match[3], + 'line' => (int) $match[4], + 'frames' => [], + ]; + } +} diff --git a/tests/Unit/Encoder/ObjectEncoderTest.php b/tests/Unit/Encoder/ObjectEncoderTest.php index b6f9fc3..dcfab6f 100644 --- a/tests/Unit/Encoder/ObjectEncoderTest.php +++ b/tests/Unit/Encoder/ObjectEncoderTest.php @@ -212,6 +212,50 @@ public function test_it_can_decode_from_xml_item(): void static::assertEquals($expected, $actual); } + public function test_it_decodes_absent_elements_as_null(): void + { + $encoder = new ObjectEncoder(stdClass::class); + $xsdType = XsdType::create('user') + ->withXmlNamespace("https://test") + ->withXmlNamespaceName('test') + ->withXmlTargetNodeName('user') + ->withMeta(static fn (TypeMeta $meta): TypeMeta => $meta->withIsQualified(true)); + $context = self::createContext($xsdType, self::buildTypes()); + + $actual = $encoder->iso($context)->from(Element::fromString('')); + + static::assertEquals( + (object)[ + 'active' => null, + 'hat' => (object)[ + 'color' => null, + ], + ], + $actual + ); + } + + public function test_it_decodes_absent_attributes_as_null(): void + { + $encoder = new ObjectEncoder(stdClass::class); + $xsdType = XsdType::create('user') + ->withXmlNamespace("https://test") + ->withXmlNamespaceName('test') + ->withXmlTargetNodeName('user') + ->withMeta(static fn (TypeMeta $meta): TypeMeta => $meta->withIsQualified(true)); + $context = self::createContext($xsdType, self::buildTypes(activeAsAttribute: true)); + + $actual = $encoder->iso($context)->from(Element::fromString('')); + + static::assertEquals( + (object)[ + 'active' => null, + 'hat' => null, + ], + $actual + ); + } + public function test_it_can_encode_from_array(): void { $encoder = new ObjectEncoder(stdClass::class); diff --git a/tests/Unit/Support/Throws/ThrowCounterTest.php b/tests/Unit/Support/Throws/ThrowCounterTest.php new file mode 100644 index 0000000..064993c --- /dev/null +++ b/tests/Unit/Support/Throws/ThrowCounterTest.php @@ -0,0 +1,73 @@ +class); + static::assertSame("canary\nspanning two lines", $events[0]->message); + static::assertSame(__FILE__, $events[0]->file); + static::assertSame($line, $events[0]->line); + static::assertNotEmpty($events[0]->frames); + } + + public function test_it_passes_when_nothing_is_thrown(): void + { + static::assertNoThrows(static fn (): int => 1 + 1); + } + + public function test_it_fails_with_the_throws_grouped_by_site(): void + { + try { + static::assertNoThrows(static function (): void { + for ($i = 0; $i < 2; $i++) { + try { + throw new RuntimeException('expected'); + } catch (RuntimeException) { + } + } + }); + } catch (AssertionFailedError $failure) { + static::assertStringContainsString('got 2', $failure->getMessage()); + static::assertStringContainsString( + '2x RuntimeException at tests/Unit/Support/Throws/ThrowCounterTest.php:', + $failure->getMessage() + ); + + return; + } + + static::fail('assertNoThrows did not fail on thrown exceptions.'); + } +}