Skip to content

Commit eacd66d

Browse files
greydnlsgreydnls
committed
Merge pull request #24 from judgej/master
Issue #17 and Issue #13
2 parents fcd5a60 + 5280d9f commit eacd66d

File tree

4 files changed

+240
-10
lines changed

4 files changed

+240
-10
lines changed

‎src/Omnipay/Common/Helper.php

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55

66
namespace Omnipay\Common;
77

8+
use InvalidArgumentException;
9+
810
/**
911
* Helper class
1012
*
@@ -115,4 +117,30 @@ public static function getGatewayClassName($shortName)
115117

116118
return '\\Omnipay\\'.$shortName.'Gateway';
117119
}
120+
121+
/**
122+
* Convert an amount into a float.
123+
* The float datatype can then be converted into the string
124+
* format that the remote gateway requies.
125+
*
126+
* @var string|int|float $value The value to convert.
127+
* @throws InvalidArgumentException on a validation failure.
128+
* @return float The amount converted to a float.
129+
*/
130+
131+
public static function toFloat($value)
132+
{
133+
if (!is_string($value) && !is_int($value) && !is_float($value)) {
134+
throw new InvalidArgumentException('Data type is not a valid decimal number.');
135+
}
136+
137+
if (is_string($value)) {
138+
// Validate generic number, with optional sign and decimals.
139+
if (!preg_match('/^[-]?[0-9]+(\.[0-9]*)?$/', $value)) {
140+
throw new InvalidArgumentException('String is not a valid decimal number.');
141+
}
142+
}
143+
144+
return (float)$value;
145+
}
118146
}

‎src/Omnipay/Common/Message/AbstractRequest.php

Lines changed: 60 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use Omnipay\Common\ItemBag;
1515
use Symfony\Component\HttpFoundation\ParameterBag;
1616
use Symfony\Component\HttpFoundation\Request as HttpRequest;
17+
use InvalidArgumentException;
1718

1819
/**
1920
* Abstract Request
@@ -88,6 +89,16 @@ abstract class AbstractRequest implements RequestInterface
8889
*/
8990
protected $response;
9091

92+
/**
93+
* @var bool
94+
*/
95+
protected $zeroAmountAllowed = true;
96+
97+
/**
98+
* @var bool
99+
*/
100+
protected $negativeAmountAllowed = false;
101+
91102
/**
92103
* Create a new Request
93104
*
@@ -271,21 +282,61 @@ public function setCardReference($value)
271282
}
272283

273284
/**
274-
* Get the payment amount.
285+
* Convert an amount into a float.
275286
*
276-
* @return string
287+
* @var string|int|float $value The value to convert.
288+
* @throws InvalidRequestException on any validation failure.
289+
* @return float The amount converted to a float.
290+
*/
291+
292+
public function toFloat($value)
293+
{
294+
try {
295+
return Helper::toFloat($value);
296+
} catch (InvalidArgumentException $e) {
297+
// Throw old exception for legacy implementations.
298+
throw new InvalidRequestException($e->getMessage(), $e->getCode(), $e);
299+
}
300+
}
301+
302+
/**
303+
* Validates and returns the formated amount.
304+
*
305+
* @throws InvalidRequestException on any validation failure.
306+
* @return string The amount formatted to the correct number of decimal places for the selected currency.
277307
*/
278308
public function getAmount()
279309
{
280310
$amount = $this->getParameter('amount');
311+
281312
if ($amount !== null) {
282-
if (!is_float($amount) &&
283-
$this->getCurrencyDecimalPlaces() > 0 &&
284-
false === strpos((string) $amount, '.')) {
285-
throw new InvalidRequestException(
286-
'Please specify amount as a string or float, ' .
287-
'with decimal places (e.g. \'10.00\' to represent $10.00).'
288-
);
313+
// Don't allow integers for currencies that support decimals.
314+
// This is for legacy reasons - upgrades from v0.9
315+
if ($this->getCurrencyDecimalPlaces() > 0) {
316+
if (is_int($amount) || (is_string($amount) && false === strpos((string) $amount, '.'))) {
317+
throw new InvalidRequestException(
318+
'Please specify amount as a string or float, '
319+
. 'with decimal places (e.g. \'10.00\' to represent $10.00).'
320+
);
321+
};
322+
}
323+
324+
$amount = $this->toFloat($amount);
325+
326+
// Check for a negative amount.
327+
if (!$this->negativeAmountAllowed && $amount < 0) {
328+
throw new InvalidRequestException('A negative amount is not allowed.');
329+
}
330+
331+
// Check for a zero amount.
332+
if (!$this->zeroAmountAllowed && $amount === 0.0) {
333+
throw new InvalidRequestException('A zero amount is not allowed.');
334+
}
335+
336+
// Check for rounding that may occur if too many significant decimal digits are supplied.
337+
$decimal_count = strlen(substr(strrchr((string)$amount, '.'), 1));
338+
if ($decimal_count > $this->getCurrencyDecimalPlaces()) {
339+
throw new InvalidRequestException('Amount precision is too high for currency.');
289340
}
290341

291342
return $this->formatCurrency($amount);

‎tests/Omnipay/Common/HelperTest.php

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,4 +134,93 @@ public function testGetGatewayClassNameUnderscoreNamespace()
134134
$class = Helper::getGatewayClassName('PayPal_Express');
135135
$this->assertEquals('\\Omnipay\\PayPal\\ExpressGateway', $class);
136136
}
137+
138+
/**
139+
* Some valid toFloat() inputs.
140+
*/
141+
public function testToFloatFromFloat()
142+
{
143+
$shortName = Helper::toFloat(1.99);
144+
$this->assertSame(1.99, $shortName);
145+
}
146+
147+
public function testToFloatFromInt()
148+
{
149+
$shortName = Helper::toFloat(199);
150+
$this->assertSame(199.0, $shortName);
151+
}
152+
153+
public function testToFloatFromStringDecimal()
154+
{
155+
$shortName = Helper::toFloat("1.99");
156+
$this->assertSame(1.99, $shortName);
157+
}
158+
159+
public function testToFloatFromStringRedunantZeroes()
160+
{
161+
$shortName = Helper::toFloat("000009.99900000000");
162+
$this->assertSame(9.999, $shortName);
163+
}
164+
165+
public function testToFloatFromStringEmptyDecimal()
166+
{
167+
$shortName = Helper::toFloat("1.");
168+
$this->assertSame(1.0, $shortName);
169+
}
170+
171+
public function testToFloatFromStringInt()
172+
{
173+
$shortName = Helper::toFloat("199");
174+
$this->assertSame(199.0, $shortName);
175+
}
176+
177+
public function testToFloatFromStringIntNegative()
178+
{
179+
$shortName = Helper::toFloat("-199");
180+
$this->assertSame(-199.0, $shortName);
181+
}
182+
183+
/**
184+
* Some invalid toFloat() inputs.
185+
*/
186+
187+
/**
188+
* The number MUST always start with a digit.
189+
* This is arguably an arbitrary rule that perhaps does not need
190+
* to be enforced.
191+
*
192+
* @expectedException \InvalidArgumentException
193+
* @expectedExceptionMessage String is not a valid decimal number
194+
*/
195+
public function testToFloatFromStringEmptyIntegerPart()
196+
{
197+
$shortName = Helper::toFloat(".99");
198+
}
199+
200+
/**
201+
* @expectedException \InvalidArgumentException
202+
* @expectedExceptionMessage String is not a valid decimal number
203+
*/
204+
public function testToFloatFromStringTwoDecimalPoints()
205+
{
206+
$shortName = Helper::toFloat("1.99.");
207+
}
208+
209+
/**
210+
* @expectedException \InvalidArgumentException
211+
* @expectedExceptionMessage String is not a valid decimal number
212+
*/
213+
public function testToFloatFromStringWrongDecimalPoints()
214+
{
215+
$shortName = Helper::toFloat("1,99");
216+
}
217+
218+
/**
219+
* @expectedException \InvalidArgumentException
220+
* @expectedExceptionMessage Data type is not a valid decimal number
221+
*/
222+
public function testToFloatFromBoolean()
223+
{
224+
$shortName = Helper::toFloat(false);
225+
}
137226
}

‎tests/Omnipay/Common/Message/AbstractRequestTest.php

Lines changed: 63 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,18 +87,34 @@ public function testAmountWithEmpty()
8787
$this->assertSame(null, $this->request->getAmount());
8888
}
8989

90+
public function testAmountZeroFloat()
91+
{
92+
$this->assertSame($this->request, $this->request->setAmount(0.0));
93+
$this->assertSame('0.00', $this->request->getAmount());
94+
}
95+
96+
public function testAmountZeroString()
97+
{
98+
$this->assertSame($this->request, $this->request->setAmount('0.000000'));
99+
$this->assertSame('0.00', $this->request->getAmount());
100+
}
101+
90102
public function testGetAmountNoDecimals()
91103
{
92104
$this->assertSame($this->request, $this->request->setCurrency('JPY'));
93105
$this->assertSame($this->request, $this->request->setAmount('1366'));
94106
$this->assertSame('1366', $this->request->getAmount());
95107
}
96108

109+
/**
110+
* @expectedException Omnipay\Common\Exception\InvalidRequestException
111+
*/
97112
public function testGetAmountNoDecimalsRounding()
98113
{
114+
// There will not be any rounding; the amount is sent as requested or not at all.
99115
$this->assertSame($this->request, $this->request->setAmount('136.5'));
100116
$this->assertSame($this->request, $this->request->setCurrency('JPY'));
101-
$this->assertSame('137', $this->request->getAmount());
117+
$this->request->getAmount();
102118
}
103119

104120
/**
@@ -117,6 +133,7 @@ public function testAmountWithIntThrowsException()
117133
public function testAmountWithIntStringThrowsException()
118134
{
119135
// ambiguous value, avoid errors upgrading from v0.9
136+
// Some currencies only take integers, so an integer (in a string) should be valid.
120137
$this->assertSame($this->request, $this->request->setAmount('10'));
121138
$this->request->getAmount();
122139
}
@@ -134,6 +151,51 @@ public function testGetAmountIntegerNoDecimals()
134151
$this->assertSame(1366, $this->request->getAmountInteger());
135152
}
136153

154+
/**
155+
* @expectedException Omnipay\Common\Exception\InvalidRequestException
156+
*/
157+
public function testAmountThousandsSepThrowsException()
158+
{
159+
$this->assertSame($this->request, $this->request->setAmount('1,234.00'));
160+
$this->request->getAmount();
161+
}
162+
163+
/**
164+
* @expectedException Omnipay\Common\Exception\InvalidRequestException
165+
*/
166+
public function testAmountInvalidFormatThrowsException()
167+
{
168+
$this->assertSame($this->request, $this->request->setAmount('1.234.00'));
169+
$this->request->getAmount();
170+
}
171+
172+
/**
173+
* @expectedException Omnipay\Common\Exception\InvalidRequestException
174+
*/
175+
public function testAmountInvalidTypeThrowsException()
176+
{
177+
$this->assertSame($this->request, $this->request->setAmount(true));
178+
$this->request->getAmount();
179+
}
180+
181+
/**
182+
* @expectedException Omnipay\Common\Exception\InvalidRequestException
183+
*/
184+
public function testAmountNegativeStringThrowsException()
185+
{
186+
$this->assertSame($this->request, $this->request->setAmount('-123.00'));
187+
$this->request->getAmount();
188+
}
189+
190+
/**
191+
* @expectedException Omnipay\Common\Exception\InvalidRequestException
192+
*/
193+
public function testAmountNegativeFloatThrowsException()
194+
{
195+
$this->assertSame($this->request, $this->request->setAmount(-123.00));
196+
$this->request->getAmount();
197+
}
198+
137199
public function testCurrency()
138200
{
139201
$this->assertSame($this->request, $this->request->setCurrency('USD'));

0 commit comments

Comments
 (0)