WsdlToPhp / PackageGenerator

Generates a PHP SDK based on a WSDL, simple and powerful, WSDL to PHP
https://providr.io
MIT License
422 stars 73 forks source link

Generated classes/files enhancements #233

Open mikaelcom opened 3 years ago

mikaelcom commented 3 years ago

Tasks:

sprankhub commented 3 years ago

In the generated ClassMap, it would be much nicer to use MyType::class instead of '\\MyType'.

sprankhub commented 3 years ago

WSDL:

<wsdl:operation name="SomePing">
    <soap:operation soapAction="urn:WebService-Shop#SomePing" style="rpc"/>
    <wsdl:input>
        <soap:body use="literal" encodingStyle="http://schemas.xmlsoap.org/soap/encoding/" namespace="urn:WebService-Shop"/>
    </wsdl:input>
    <wsdl:output>
        <soap:body use="literal" encodingStyle="http://schemas.xmlsoap.org/soap/encoding/" namespace="urn:WebService-Shop"/>
    </wsdl:output>
</wsdl:operation>

This leads to broken code:

$this->setResult($resultSomePing = $this->getSoapClient()->__soapCall('SomePing', ], [], [], $this->outputHeaders));

Not sure if this only happens in the new version or also in the old one.

sprankhub commented 3 years ago

Constructors of child classes miss parent constructor calls. Given the following hierarchy:

class ShopRequest extends AbstractStructBase

class SomeRequestType extends ShopRequest

The generated constructor of SomeRequestType does not call the constructor of ShopRequest and also does not include the parent properties.

Not sure if this only happens in the new version or also in the old one.

sprankhub commented 3 years ago

Currently, service classes return either a type or a bool, which does not let us use a return type. I suggest throwing an exception in case of any errors or at least return null, so that we can use return type hints.

Sorry for just posting this here, but I am currently unsure if these issues belong to the old or the new or both versions and I do not know how you would like to structure your work. Tell me if I should better create new issues :)

mikaelcom commented 3 years ago

No worries, this issue is perfectly suited to gather future enhancements, wether they are old issues or not.

mikaelcom commented 3 years ago

WSDL:

<wsdl:operation name="SomePing">
    <soap:operation soapAction="urn:WebService-Shop#SomePing" style="rpc"/>
    <wsdl:input>
        <soap:body use="literal" encodingStyle="http://schemas.xmlsoap.org/soap/encoding/" namespace="urn:WebService-Shop"/>
    </wsdl:input>
    <wsdl:output>
        <soap:body use="literal" encodingStyle="http://schemas.xmlsoap.org/soap/encoding/" namespace="urn:WebService-Shop"/>
    </wsdl:output>
</wsdl:operation>

This leads to broken code:

$this->setResult($resultSomePing = $this->getSoapClient()->__soapCall('SomePing', ], [], [], $this->outputHeaders));

Not sure if this only happens in the new version or also in the old one.

It has to be fixed within https://github.com/WsdlToPhp/PackageGenerator/pull/234. If I understand well, this is the case when the Soap operation does not require any parameter, isn't?

mikaelcom commented 3 years ago

Constructors of child classes miss parent constructor calls. Given the following hierarchy:

class ShopRequest extends AbstractStructBase

class SomeRequestType extends ShopRequest

The generated constructor of SomeRequestType does not call the constructor of ShopRequest and also does not include the parent properties.

Not sure if this only happens in the new version or also in the old one.

This is the default behaviour.

In my point of view, adding the inherited constructor arguments to the child classes constructor is not necessarily useful because:

Most of the time, arguments are optional, so I usually use the fluent way:

$struct = (new Struct())
    ->setValue($value)
    ->setAny($any)
;

Or even the __set_state method:

$struct = Struct::__set_state([
    'value' => $value,
    'any'   => $any,
]);

Nevertheless, calling the parent constructor without any argument could be added in order to handle removable properties. This is feasible only if there is no required arguments unless they are also added to the child constructor arguments at first position with its own required arguments.

Let me know your thoughts about this.

mikaelcom commented 3 years ago

In the generated ClassMap, it would be much nicer to use MyType::class instead of '\\MyType'.

To be tested as there is a time classmap required fully qualified class name with the first \.

sprankhub commented 3 years ago

Thanks @mikaelcom!

It has to be fixed within #234. If I understand well, this is the case when the Soap operation does not require any parameter, isn't?

Most probably yes :)

Constructors of child classes miss parent constructor calls.

If most of the properties are optional, I agree. However, in my case, nearly all are required. And if this is the case, it is easy to miss required parameters if the child constructor does not also include the parent properties. However, this of course depends on the use case and is completely opinionated, so feel free to ignore that suggestion :)

To be tested as there is a time classmap required fully qualified class name with the first .

Sounds strange. I can only imagine this happened in really old PHP versions. However, since we now target PHP >= 7.4, I do not think that this leads to issues.