Refactor getAmount() - pull out the float conversion.

Author: judgej

src/Omnipay/Common/Message/AbstractRequest.php

@@ -37,6 +37,16 @@ abstract class AbstractRequest implements RequestInterface
      */
     protected $response;
 
+    /**
+     * @var bool
+     */
+    protected $zeroAmountAllowed = true;
+
+    /**
+     * @var bool
+     */
+    protected $negativeAmountAllowed = false;
+
     /**
      * Create a new Request
      *
@@ -158,6 +168,30 @@ public function setCardReference($value)
         return $this->setParameter('cardReference', $value);
     }
 
+    /**
+     * Convert an amount into a float.
+     *
+     * @var string|int|float $value The value to convert.
+     * @throws InvalidRequestException on any validation failure.
+     * @return float The amount converted to a float.
+     */
+
+    public function toFloat($value)
+    {
+        if ( ! is_string($value) && ! is_int($value) && ! is_float($value)) {
+            throw new InvalidRequestException('Data type is not a valid decimal number.');
+        }
+
+        if (is_string($value)) {
+            // Validate generic number, with optional sign and decimals.
+            if ( ! preg_match('/^[-]?[0-9]+(\.[0-9]*)?$/', $value)) {
+                throw new InvalidRequestException('String is not a valid decimal number.');
+            }
+        }
+
+        return (float)$value;
+    }
+
     /**
      * Validates and returns the formated amount.
      *
@@ -168,41 +202,31 @@ public function setCardReference($value)
     public function getAmount()
     {
         $amount = $this->getParameter('amount');
-        $message = 'Please specify amount as a string or float, '
-            . 'with decimal places (e.g. \'10.00\' to represent $10.00).';
 
         if ($amount !== null) {
             // Don't allow integers for currencies that support decimals.
             // This is for legacy reasons - upgrades from v0.9
-            if (is_int($amount) && $this->getCurrencyDecimalPlaces() > 0) {
-                throw new InvalidRequestException($message);
-            }
-
-            if (is_string($amount)) {
-                // Negative amounts are valid numbers at this stage.
-                if (preg_match('/[^0-9\.-]/', $amount)) {
-                    throw new InvalidRequestException('Invalid character in amount.');
-                }
-
-                // Generic number, with optional sign and decimals.
-                if (!preg_match('/^[-]?[0-9]+(\.[0-9]*)?$/', $amount)) {
-                    throw new InvalidRequestException('Amount string is not a valid decimal number.');
-                }
-
-                // Don't allow integers for currencies that support decimals (legacy v0.9).
-                if ($this->getCurrencyDecimalPlaces() > 0 && false === strpos((string) $amount, '.')) {
-                    throw new InvalidRequestException($message);
-                }
+            if ($this->getCurrencyDecimalPlaces() > 0) {
+                if (is_int($amount) || (is_string($amount) && false === strpos((string) $amount, '.'))) {
+                    throw new InvalidRequestException(
+                        'Please specify amount as a string or float, '
+                            . 'with decimal places (e.g. \'10.00\' to represent $10.00).'
+                    );
+                };
             }
 
-            // The number_format() used later requires a float.
-            $amount = (float)$amount;
+            $amount = $this->toFloat($amount);
 
             // Check for a negative amount.
-            if ($amount < 0) {
+            if ( ! $this->negativeAmountAllowed && $amount < 0) {
                 throw new InvalidRequestException('A negative amount is not allowed.');
             }
 
+            // Check for a zero amount.
+            if ( ! $this->zeroAmountAllowed && $amount === 0.0) {
+                throw new InvalidRequestException('A zero amount is not allowed.');
+            }
+
             // Check for rounding that may occur if too many significant decimal digits are supplied.
             $decimal_count = strlen(substr(strrchr((string)$amount, '.'), 1));
             if ($decimal_count > $this->getCurrencyDecimalPlaces()) {

tests/Omnipay/Common/Message/AbstractRequestTest.php

@@ -95,7 +95,7 @@ public function testAmountZeroFloat()
 
     public function testAmountZeroString()
     {
-        $this->assertSame($this->request, $this->request->setAmount('0.0'));
+        $this->assertSame($this->request, $this->request->setAmount('0.000000'));
         $this->assertSame('0.00', $this->request->getAmount());
     }
 
@@ -169,6 +169,15 @@ public function testAmountInvalidFormatThrowsException()
         $this->request->getAmount();
     }
 
+    /**
+     * @expectedException Omnipay\Common\Exception\InvalidRequestException
+     */
+    public function testAmountInvalidTypeThrowsException()
+    {
+        $this->assertSame($this->request, $this->request->setAmount(true));
+        $this->request->getAmount();
+    }
+
     /**
      * @expectedException Omnipay\Common\Exception\InvalidRequestException
      */