php-soap / wsdl-reader

A pure PHP wsdl metadata provider
18 stars 3 forks source link

Support nested sequences inside of choices #23

Closed lstrojny closed 7 months ago

lstrojny commented 7 months ago

Bug Report

Q A
BC Break yes/no
Version x.y.z

Summary

Current behaviour

Given this type:

                <xs:complexType>
                    <xs:choice minOccurs="0">
                        <xs:element name="flag" type="xs:boolean" />
                        <xs:sequence>
                            <xs:element name="mandatory" type="xs:string" />
                            <xs:element minOccurs="0" name="optional" type="xs:boolean" />
                        </xs:sequence>
                    </xs:choice>
                </xs:complexType>

Code generation will fail with non-empty-string assertion:

  at vendor/azjezz/psl/src/Psl/Type/Exception/AssertException.php:32
 Psl\Type\Exception\AssertException::withValue() at vendor/azjezz/psl/src/Psl/Type/Internal/NonEmptyStringType.php:71
 Psl\Type\Internal\NonEmptyStringType->assert() at vendor/phpro/soap-client/src/Phpro/SoapClient/CodeGenerator/Model/Property.php:66
 Phpro\SoapClient\CodeGenerator\Model\Property::fromMetaData() at vendor/phpro/soap-client/src/Phpro/SoapClient/CodeGenerator/Model/Type.php:68
 Phpro\SoapClient\CodeGenerator\Model\Type::Phpro\SoapClient\CodeGenerator\Model\{closure}() at n/a:n/a
 array_map() at vendor/phpro/soap-client/src/Phpro/SoapClient/CodeGenerator/Model/Type.php:72
 Phpro\SoapClient\CodeGenerator\Model\Type::fromMetadata() at vendor/phpro/soap-client/src/Phpro/SoapClient/CodeGenerator/Model/TypeMap.php:47
 Phpro\SoapClient\CodeGenerator\Model\TypeMap::Phpro\SoapClient\CodeGenerator\Model\{closure}() at n/a:n/a
 array_map() at vendor/php-soap/engine/src/Metadata/Collection/TypeCollection.php:51
 Soap\Engine\Metadata\Collection\TypeCollection->map() at vendor/phpro/soap-client/src/Phpro/SoapClient/CodeGenerator/Model/TypeMap.php:48
 Phpro\SoapClient\CodeGenerator\Model\TypeMap::fromMetadata() at vendor/phpro/soap-client/src/Phpro/SoapClient/Console/Command/GenerateTypesCommand.php:77
 Phpro\SoapClient\Console\Command\GenerateTypesCommand->execute() at vendor/symfony/console/Command/Command.php:326
 Symfony\Component\Console\Command\Command->run() at vendor/symfony/console/Application.php:1078
 Symfony\Component\Console\Application->doRunCommand() at vendor/symfony/console/Application.php:324
 Symfony\Component\Console\Application->doRun() at vendor/symfony/console/Application.php:175
 Symfony\Component\Console\Application->run() at vendor/phpro/soap-client/bin/soap-client:26
 include() at vendor/bin/soap-client:119

Fix

Not 100% certain, but it looks like the culprit is in \Soap\WsdlReader\Metadata\Converter\Types\Visitor\ElementContainerVisitor::parseElementItem. It handles choice and group but not sequence. Adding an instanceof check for sequence here, fixes the issue.

veewee commented 7 months ago

Hello,

Thanks for reporting the issue. That seems like a proper fix to me:

Before:

 > http://test-uri/:a {
+    ?boolean $flag
+     $
   }

After:

 > http://test-uri/:a {
    ?boolean $flag
    ?string $mandatory
    ?boolean $optional
 }

If created a PR with your suggestion. Can you check it to make sure this is what you had in mind out and see if it works for you? https://github.com/php-soap/wsdl-reader/pull/24

lstrojny commented 7 months ago

Thanks! Fix looks good and works fine

lstrojny commented 7 months ago

Just to make sure it's a broad enough fix: checking for ElementContainer would be too broad?

veewee commented 7 months ago

Not sure ATM:

As far as I understand that interface, it is implemented by Group, Sequence and Choice (and references to them) which we cover. However I can also see ComplexType and I'm not sure how that will behave. Maybe we can find an example for that specific case and see how it goes?

Screenshot 2024-03-31 at 10 25 23

If I change the if statement to

        if ($element instanceof ElementContainer) {
            return $this->__invoke($element, $context);
        }

All tests seem to pass:

................................................................. 65 / 95 ( 68%)
..............................                                    95 / 95 (100%)

Time: 00:11.202, Memory: 8.00 MB

OK (95 tests, 95 assertions)

However, I'dd like to be sure and test it out on having an anonymous complex type in there. Will try to find an example when I find more time. Feel free to play around yourself as well :)

veewee commented 7 months ago

I've tried some versions with complexTypes and it seems to compile those. I'm gonna just merge it as-is and see if someone comes up with a failing scenario for nested anonymous complex types. Currently this has been reported a second time, so a fix is more urgent.