validate() only checks for null in the core parameters array

[judgej]

Something I noticed doing some testing.

The request validate() function is used to check whether required parameters have been set, to prevent an unnecessary call to the remote gateway. If a parameter has not been set at all, then an exception is thrown.

Now, default parameters are generally given a default value as a helper to show just what kind of data type it accepts. It may be a an empty string when a string is expected, a 0 when an integer is expected, an array of values when one of a set of enumerated values is expected.

My problem is with the empty strings - the empty string means the validate() function will always pass the test even if no parameter has been supplied. So a default value of merchantId of "" will mean the driver assumes the merchantId has been set, even if the merchant app has not explicitly set it.

I tried mapping an empty string to a null in the getter:

// authorizeRequest
public function getMerchantId()
{
    return $this->getParameter('merchantId') ?: null;
}

That does return a null if the parameter was not set, but the validate() function just goes to the underlying parameters array and by-passes the getter in the driver.

So possible approaches to solve this:

• Require all default parameters in the driver to be null. Kind of defeats the object of default parameters.
• Update validate() so it uses the request message getter if it is available. This puts control back into the hands of the driver developer (e.g. if the getter has not been changed from the known default, then return a null). The standard Omnipay tests, however, imply that getters should always return exactly what the setters are given, so this would be a little bit invalidated (put an empty string in; get a null out).
• Keep track of which parameters have been changed at any point from their default value. Any that have not been changed, treat them as "not set" when doing a validation check. This may trigger some false positive validation checks though, if the default value of a mandatory parameter just happens to be what you want.
• Just keep it as it is, so a driver developer - if setting default values on required parameters - needs to do their own explicit check and throw their own validation exception. validate('merchantId'); if ($this->getMerchantId() === '') throw new WhateverException(...); This feels like it is duplicating something that Omnipay is trying to offer out of the box already.

I think I like the second option. Any other approaches? Any other views on whether this really is a problem (are people already following the forth option)?

[judgej]

On that second option, there is a precedent for the getter NOT returning what the setter sets. I had currency as a default parameter on a new gateway, and it kept failing the tests because it was being transformed to upper case by the getter:

// Omnipay\Common\AbstractGateway

    /**
     * @return string
     */
    public function getCurrency()
    {
        return strtoupper($this->getParameter('currency'));
    }

So 5b56ea7ae2828 was going in and 5B56EA7AE2828 was coming out.

[judgej]

I'm just going to close this and use null throughout the default parameter values. Specific validation can be added on an ad-hoc basis in drivers where used, and the example application is just going to be have to be happy with not knowing the data type of a default parameter.