phpro / soap-client

A general purpose SOAP client for PHP
MIT License
862 stars 175 forks source link

Two properties (one with an underscore _ one without) results in having only one getter method #502

Closed sippsolutions closed 9 months ago

sippsolutions commented 9 months ago

Bug Report

Q A
BC Break yes
Version 3.1.1

Summary

Two properties having the same name but one is with underscores results in one of them having no getter method.

Current behavior

When generating type classes the \Phpro\SoapClient\CodeGenerator\Util\Normalizer::normalizeMethodName removes underscores. \Phpro\SoapClient\CodeGenerator\Assembler\GetterAssembler::assemble then removes the generated (generatePropertyMethod) method (via removeMethod). When you have two properties named TEST_KEY and TESTKEY both of them are transformed into a class property, but only one of them will get a "getter" method as the underscores get removed and the previously generated method for TEST_KEY (getTESTKEY) will be removed as TESTKEY gets assigned the same getter name (getTESTKEY). Unfortunately I cannot change the WSDL as it comes from a third party API.

\Phpro\SoapClient\CodeGenerator\Util\Normalizer::generatePropertyMethod should be changed from regex ^a-z0-9]+ to ^a-z0-9]_+. OR, maybe a workaround: If two properties have the same getter method name, the first one should not be removed with removeMethod but the second one should get a new getter name (like first one having getTESTKEY, second one getting getTESTKEY__2) - this would result in a non-breaking change as existing method names are not affected.

How to reproduce

Generate type classes with two properties named the same but one property having an additional underscore inside of the name.

Expected behavior

Both properties have their corresponding getter method.


Working example for the workaround (should also be applied for Setters, ImmutableSetters, ...):

--- src/Phpro/SoapClient/CodeGenerator/Assembler/GetterAssembler.php    2024-02-14 15:53:30
+++ src/Phpro/SoapClient/CodeGenerator/Assembler/GetterAssembler.php    2024-02-14 15:54:33
@@ -2,13 +2,13 @@

 namespace Phpro\SoapClient\CodeGenerator\Assembler;

+use Laminas\Code\Generator\MethodGenerator;
 use Phpro\SoapClient\CodeGenerator\Context\ContextInterface;
 use Phpro\SoapClient\CodeGenerator\Context\PropertyContext;
+use Phpro\SoapClient\CodeGenerator\LaminasCodeFactory\DocBlockGeneratorFactory;
 use Phpro\SoapClient\CodeGenerator\Model\Property;
 use Phpro\SoapClient\CodeGenerator\Util\Normalizer;
-use Phpro\SoapClient\CodeGenerator\LaminasCodeFactory\DocBlockGeneratorFactory;
 use Phpro\SoapClient\Exception\AssemblerException;
-use Laminas\Code\Generator\MethodGenerator;
 use Soap\Engine\Metadata\Model\TypeMeta;

 /**
@@ -59,6 +59,13 @@
         try {
             $prefix = $this->getPrefix($property);
             $methodName = Normalizer::generatePropertyMethod($prefix, $property->getName());
+            if ($class->hasMethod($methodName)) {
+                $i = 2;
+                while ($class->hasMethod($methodName . '__' . $i)) {
+                    $i++;
+                }
+                $methodName = $methodName . '__' . $i;
+            }
             $class->removeMethod($methodName);

             $methodGenerator = new MethodGenerator($methodName);
veewee commented 9 months ago

Hello,

Thanks for reporting! Having 2 fields with the same name only variated by a underscore sounds - besides very confusing from an implementor side - also very uncommon.

If we want to fix this, I'dd say allowing the generatePropertyMethod to contain underscores sounds like the way to go. This might be a subtle change in this library, but will result in broken code in packages that use this library : the public API of the generated types changes which will result in broken code. This is probably something we want to do in next major version which won't be ready anyday soon.

I'll put it to the list of tasks to take into consideration. You could solve this with a custom assembler from your end ATM.

veewee commented 4 months ago

Hello there @sippsolutions,

I wanted to get back to this issue: We are finishing up v4 of the soap-client package which uses a new encoding system internally. With this new system, you should be able to get around this issue more easily.

We are eager to receive some early feedback on this new release. If you got some time, feel free to play around with the alpha version: