issue #285 - Cast value passed to preg_match to a string

[tbreuss]

I'll test your branch tomorrow at work by generating PHP classes for a large WSDL SOAP service.

I'll leave a comment here if the test was successful or not.

[tbreuss]

@mikaelcom I tested your branch with a large WSDL and have noticed the following behavior.

With Version 4.1.7:

/**
 * This class stands for ReducedWorkingStateType StructType
 * @subpackage Structs
 */
class ReducedWorkingStateType extends AbstractStructBase
{
    /**
     * The Success
     * Meta information extracted from the WSDL
     * - documentation: Successo Notifica elaborata con successo | Succès Le traitement a terminé avec succès | Erfolgreich Verarbeitung war erfolgreich
     * - choice: Success
     * - choiceMaxOccurs: 1
     * - choiceMinOccurs: 1
     * @var \Transmitter\PhpSdk\StructType\SuccessResponseType|null
     */
    protected ?\Transmitter\PhpSdk\StructType\SuccessResponseType $Success = null;
    /**
     * Constructor method for ReducedWorkingStateType
     * @uses ReducedWorkingStateType::setSuccess()
     * @param \Transmitter\PhpSdk\StructType\SuccessResponseType $success
     */
    public function __construct(?\Transmitter\PhpSdk\StructType\SuccessResponseType $success = null)
    {
        $this
            ->setSuccess($success);
    }
    /**
     * Get Success value
     * @return \Transmitter\PhpSdk\StructType\SuccessResponseType|null
     */
    public function getSuccess(): ?\Transmitter\PhpSdk\StructType\SuccessResponseType
    {
        return $this->Success ?? null;
    }
    /**
     * This method is responsible for validating the value(s) passed to the setSuccess method
     * This method is willingly generated in order to preserve the one-line inline validation within the setSuccess method
     * This has to validate that the property which is being set is the only one among the given choices
     * @param mixed $value
     * @return string A non-empty message if the values does not match the validation rules
     */
    public function validateSuccessForChoiceConstraintFromSetSuccess($value): string
    {
        $message = '';
        if (is_null($value)) {
            return $message;
        }
        $properties = [
        ];
        try {
            foreach ($properties as $property) {
                if (isset($this->{$property})) {
                    throw new InvalidArgumentException(sprintf('The property Success can\'t be set as the property %s is already set. Only one property must be set among these properties: Success, %s.', $property, implode(', ', $properties)), __LINE__);
                }
            }
        } catch (InvalidArgumentException $e) {
            $message = $e->getMessage();
        }

        return $message;
    }
    /**
     * Set Success value
     * This property belongs to a choice that allows only one property to exist. It is
     * therefore removable from the request, consequently if the value assigned to this
     * property is null, the property is removed from this object
     * @throws InvalidArgumentException
     * @param \Transmitter\PhpSdk\StructType\SuccessResponseType $success
     * @return \Transmitter\PhpSdk\StructType\ReducedWorkingStateType
     */
    public function setSuccess(?\Transmitter\PhpSdk\StructType\SuccessResponseType $success = null): self
    {
        // validation for constraint: choice(Success)
        if ('' !== ($successChoiceErrorMessage = self::validateSuccessForChoiceConstraintFromSetSuccess($success))) {
            throw new InvalidArgumentException($successChoiceErrorMessage, __LINE__);
        }
        if (is_null($success) || (is_array($success) && empty($success))) {
            unset($this->Success);
        } else {
            $this->Success = $success;
        }

        return $this;
    }
}

With version dev-feature/issue-285:

/**
 * This class stands for ReducedWorkingStateType StructType
 * Meta information extracted from the WSDL
 * - base: sdc:WorkingStateType
 * @subpackage Structs
 */
class ReducedWorkingStateType extends WorkingStateType
{
}

Is this the expected behavior?

[tbreuss]

And I additionally noticed the following:

In case of a boolean value, the generated code seems to be invalid.

    public function setProcessByDistributor(?bool $processByDistributor = null): self
    {
        // validation for constraint: boolean
        if (!is_null($processByDistributor) && !is_bool($processByDistributor)) {
            throw new InvalidArgumentException(sprintf('Invalid value %s, please provide a bool, %s given', var_export($processByDistributor, true), gettype($processByDistributor)), __LINE__);
        }
        // validation for constraint: pattern(true, false)
        if (!is_null($processByDistributor) && !preg_match('/true|false/', (string) $processByDistributor)) {
            throw new InvalidArgumentException(sprintf('Invalid value %s, please provide a literal that is among the set of character sequences denoted by the regular expression /true|false/', var_export($processByDistributor, true)), __LINE__);
        }
        $this->ProcessByDistributor = $processByDistributor;

        return $this;
    }

The issue here is, that casting of a boolen leads to an int 1/0 and not to a string true/false.

And I think, both of the two if statements are unnecessary, anyway.

[mikaelcom]

Is this the expected behavior?

The WorkingStateType class should contain the properties previously contained by the ReducedWorkingStateType class. It's certainly to recent updates.

[mikaelcom]

And I additionally noticed the following: In case of a boolean value, the generated code seems to be invalid. The issue here is, that casting of a boolen leads to an int 1/0 and not to a string true/false.

Why is there a pattern on a boolean? Is the pattern containing true and false as valid values to be matched by the pattern? Could you paste the XSD element definition?

[tbreuss]

The WorkingStateType class should contain the properties previously contained by the ReducedWorkingStateType class. It's certainly to recent updates.

Ah, okay. I assumed that the feature branch would only solve issue #285.

[mikaelcom]

The WorkingStateType class should contain the properties previously contained by the ReducedWorkingStateType class. It's certainly to recent updates.

Ah, okay. I assumed that the feature branch would only solve issue #285.

I merged improvements recently :)

[tbreuss]

And I additionally noticed the following: In case of a boolean value, the generated code seems to be invalid. The issue here is, that casting of a boolen leads to an int 1/0 and not to a string true/false.

Why is there a pattern on a boolean? Is the pattern containing true and false as valid values to be matched by the pattern? Could you paste the XSD element definition?

This is the relevant code:

    <xs:simpleType name="SimpleBooleanType">
        <xs:restriction base="xs:boolean">
            <xs:pattern value="true"/>
            <xs:pattern value="false"/>
        </xs:restriction>
    </xs:simpleType>

Which is used by:

    <xs:complexType name="ControlsType">
        <xs:sequence>
            <xs:element name="ProcessByDistributor" type="SimpleBooleanType">
            </xs:element>
        </xs:sequence>
    </xs:complexType>

As you have suspected, we have a boolean here, which is restricted by a true/false pattern.

I'm not an expert in this field, and therefore don't know if this is common.

[tbreuss]

@mikaelcom

I'm not an expert in this field, and therefore don't know if this is common.

I'm coming back to this topic.

If we have a boolean restriction like this:

    <xs:simpleType name="SimpleBooleanType">
        <xs:restriction base="xs:boolean">
            <xs:pattern value="true"/>
            <xs:pattern value="false"/>
        </xs:restriction>
    </xs:simpleType>

Couldn't we specify that the code looks more or less like this?

    public function setSimpleBoolean(?bool $someBooleanValue = null): self
    {
        $this->someBooleanValue = $someBooleanValue;

        return $this;
    }

Meaning: In the generated PHP class method if the param is of type boolean no further checks are necessary. What do you think?

[mikaelcom]

Couldn't we specify that the code looks more or less like this?

It's not implemented this way currently as it's a generic ruler that generates each rule per encountered "meta" tag such as pattern, I'll look into it ASAP

[mikaelcom]

@tbreuss I fixed the pattern rule validation generation in order to ignore boolean values, a little bit empiric but it's easy to implement and to make it evolve if necessary :smile:

[github-actions[bot]]

Coverage Report

Totals
Statements:	96.5% ( 4077 / 4225 )
Methods:	93.64% ( 707 / 755 )
Lines:	97.12% ( 3370 / 3470 )

[sonarcloud[bot]]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

100.0% Coverage
0.0% Duplication

[tbreuss]

@tbreuss I fixed the pattern rule validation generation in order to ignore boolean values, a little bit empiric but it's easy to implement and to make it evolve if necessary 😄

This is good news. I will test your changes against my large WSDL. I'll keep you informed.

[mikaelcom]

@tbreuss I fixed the pattern rule validation generation in order to ignore boolean values, a little bit empiric but it's easy to implement and to make it evolve if necessary smile

This is good news. I will test your changes against my large WSDL. I'll keep you informed.

Good to merge after all?