Skip to content

Commit 1ac1f5c

Browse files
sherrardbsherrardb
committed
fix post_charge() arguments:
 * updated Kavorka signature to remove non-functional argument types * removed Net::Stripe::Customer and HashRef for customer, as neither form was being serialized correctly for passing to the API call * removed Net::Stripe::Card and Net::Stripe::Token for card, as neither form was being serialized correctly for passing to the API call * added in-method validation and unit tests for the different combinations of the allowed argument types * updated and reorganized failing unit tests * closes L<#94> and L<#137>
1 parent b3ffb02 commit 1ac1f5c

File tree

4 files changed

+226
-31
lines changed

4 files changed

+226
-31
lines changed

‎lib/Net/Stripe.pm

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use HTTP::Request::Common qw/GET POST DELETE/;
88
use MIME::Base64 qw/encode_base64/;
99
use URI::Escape qw/uri_escape/;
1010
use JSON qw/decode_json/;
11+
use Net::Stripe::TypeConstraints;
1112
use Net::Stripe::Token;
1213
use Net::Stripe::Invoiceitem;
1314
use Net::Stripe::Invoice;
@@ -88,6 +89,19 @@ which is the format that C<Net::Stripe::List> expects. This makes the SDK
8889
compatible with the Stripe API back to the earliest documented API version
8990
<https://stripe.com/docs/upgrades#2011-06-21>.
9091
92+
=item fix post_charge() arguments
93+
94+
Some argument types for `customer` and `card` were non-functional in the
95+
current code and have been removed from the Kavorka method signature. We
96+
removed `Net::Stripe::Customer` and `HashRef` for `customer` and we removed
97+
`Net::Stripe::Card` and `Net::Stripe::Token` for `card`, as none of these
98+
forms were being serialized correctly for passing to the API call. We must
99+
retain `Net::Stripe::Card` for the `card` attribute in `Net::Stripe::Charge`
100+
because the `card` value returned from the API call is objectified into
101+
a card object. We have also added TypeConstraints for the string arguments
102+
passed and added in-method validation to ensure that the passed argument
103+
values make sense in the context of one another.
104+
91105
=back
92106
93107
=method new PARAMHASH
@@ -215,15 +229,40 @@ Returns a list of L<Net::Stripe::Charge> objects.
215229
Charges: {
216230
method post_charge(Int :$amount,
217231
Str :$currency,
218-
Net::Stripe::Customer|HashRef|Str :$customer?,
219-
Net::Stripe::Card|Net::Stripe::Token|Str|HashRef :$card?,
232+
StripeCustomerId :$customer?,
233+
StripeTokenId|StripeCardId|HashRef :$card?,
220234
Str :$description?,
221235
HashRef :$metadata?,
222236
Bool :$capture?,
223237
Str :$statement_descriptor?,
224238
Int :$application_fee?,
225239
Str :$receipt_email?
226240
) {
241+
242+
if ( defined( $card ) ) {
243+
my $card_id_type = Moose::Util::TypeConstraints::find_type_constraint( 'StripeCardId' );
244+
if ( defined( $customer ) && ! $card_id_type->check( $card ) ) {
245+
die Net::Stripe::Error->new(
246+
type => "post_charge error",
247+
message => sprintf(
248+
"Invalid value '%s' passed for parameter 'card'. Charges for an existing customer can only accept a card id.",
249+
$card,
250+
),
251+
);
252+
}
253+
254+
my $token_id_type = Moose::Util::TypeConstraints::find_type_constraint( 'StripeTokenId' );
255+
if ( ! defined( $customer ) && ! $token_id_type->check( $card ) && ref( $card ) ne 'HASH' ) {
256+
die Net::Stripe::Error->new(
257+
type => "post_charge error",
258+
message => sprintf(
259+
"Invalid value '%s' passed for parameter 'card'. Charges without an existing customer can only accept a token id or card hashref.",
260+
$card,
261+
),
262+
);
263+
}
264+
}
265+
227266
my $charge = Net::Stripe::Charge->new(amount => $amount,
228267
currency => $currency,
229268
customer => $customer,

‎lib/Net/Stripe/Charge.pm

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ has 'id' => (is => 'ro', isa => 'Maybe[Str]');
1010
has 'created' => (is => 'ro', isa => 'Maybe[Int]');
1111
has 'amount' => (is => 'ro', isa => 'Maybe[Int]', required => 1);
1212
has 'currency' => (is => 'ro', isa => 'Maybe[Str]', required => 1);
13-
has 'customer' => (is => 'ro', isa => 'Maybe[Str]');
14-
has 'card' => (is => 'ro', isa => 'Maybe[Net::Stripe::Token|Net::Stripe::Card|Str]');
13+
has 'customer' => (is => 'ro', isa => 'Maybe[StripeCustomerId]');
14+
has 'card' => (is => 'ro', isa => 'Maybe[Net::Stripe::Card|StripeTokenId|StripeCardId]');
1515
has 'description' => (is => 'ro', isa => 'Maybe[Str]');
1616
has 'livemode' => (is => 'ro', isa => 'Maybe[Bool|Object]');
1717
has 'paid' => (is => 'ro', isa => 'Maybe[Bool|Object]');

‎lib/Net/Stripe/TypeConstraints.pm

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
package Net::Stripe::TypeConstraints;
2+
3+
use strict;
4+
use Moose::Util::TypeConstraints qw/subtype as where message/;
5+
6+
# ABSTRACT: Custom Moose TypeConstraints for Net::Stripe object attributes and parameters
7+
8+
subtype 'StripeTokenId',
9+
as 'Str',
10+
where {
11+
/^tok_.+/
12+
},
13+
message {
14+
sprintf( "Value '%s' must be a token id string of the form tok_.+", $_ );
15+
};
16+
17+
subtype 'StripeCardId',
18+
as 'Str',
19+
where {
20+
/^card_.+/
21+
},
22+
message {
23+
sprintf( "Value '%s' must be a card id string of the form card_.+", $_ );
24+
};
25+
26+
subtype 'StripeCustomerId',
27+
as 'Str',
28+
where {
29+
/^cus_.+/
30+
},
31+
message {
32+
sprintf( "Value '%s' must be a customer id string of the form cus_.+", $_ );
33+
};
34+
35+
1;

‎t/live.t

Lines changed: 148 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,7 @@ Charges: {
267267
is $card->name, $fake_card->{name}, 'retrieve the card name';
268268
}
269269

270-
Post_charge_using_token: {
270+
Post_charge_using_token_id: {
271271
my $token = $stripe->post_token( card => $fake_card );
272272
my $charge = $stripe->post_charge(
273273
amount => 100,
@@ -277,48 +277,171 @@ Charges: {
277277
isa_ok $charge, 'Net::Stripe::Charge';
278278
ok $charge->paid, 'charge was paid';
279279
is $charge->status, 'paid', 'charge status is paid';
280+
is $charge->card->id, $token->card->id, 'charge card id matches';
280281
}
281282

282-
Rainy_day: {
283-
# swallow the expected warning rather than have it print out durring tests.
284-
close STDERR;
285-
open(STDERR, ">", "/dev/null");
283+
Post_charge_using_card_id: {
284+
my $token = $stripe->post_token( card => $fake_card );
285+
eval {
286+
$stripe->post_charge(
287+
amount => 100,
288+
currency => 'usd',
289+
card => $token->card->id,
290+
);
291+
};
292+
if ($@) {
293+
my $e = $@;
294+
isa_ok $e, 'Net::Stripe::Error', 'error raised is an object';
295+
is $e->type, 'post_charge error', 'error type';
296+
like $e->message, qr/^Invalid value 'card_.+' passed for parameter 'card'\. Charges without an existing customer can only accept a token id or card hashref\.$/, 'error message';
297+
} else {
298+
fail 'post charge with card id';
299+
}
300+
}
301+
302+
Post_charge_using_card_hash: {
303+
my $charge = $stripe->post_charge(
304+
amount => 100,
305+
currency => 'usd',
306+
card => $fake_card,
307+
);
308+
isa_ok $charge, 'Net::Stripe::Charge';
309+
ok $charge->paid, 'charge was paid';
310+
is $charge->status, 'paid', 'charge status is paid';
311+
is $charge->card->last4, substr( $fake_card->{number}, -4 ), 'charge card last4 matches';
312+
}
313+
314+
Post_charge_for_customer_id_with_attached_card: {
315+
my $customer = $stripe->post_customer(
316+
card => $fake_card,
317+
);
318+
my $charge = $stripe->post_charge(
319+
amount => 100,
320+
currency => 'usd',
321+
customer => $customer->id,
322+
);
323+
isa_ok $charge, 'Net::Stripe::Charge';
324+
ok $charge->paid, 'charge was paid';
325+
is $charge->status, 'paid', 'charge status is paid';
326+
is $charge->card->id, $customer->default_card, 'charged default card';
327+
}
286328

287-
throws_ok {
329+
Post_charge_for_customer_id_without_attached_card: {
330+
my $customer = $stripe->post_customer();
331+
eval {
332+
# swallow the expected warning rather than have it print out during tests.
333+
local $SIG{__WARN__} = sub {};
288334
$stripe->post_charge(
289-
amount => 3300,
335+
amount => 100,
290336
currency => 'usd',
291-
description => 'Wikileaks donation',
337+
customer => $customer->id,
338+
);
339+
};
340+
if ($@) {
341+
my $e = $@;
342+
isa_ok $e, 'Net::Stripe::Error', 'error raised is an object';
343+
is $e->type, 'card_error', 'error type';
344+
is $e->message, 'Cannot charge a customer that has no active card', 'error message';
345+
} else {
346+
fail 'post charge for customer with token id';
347+
}
348+
}
349+
350+
Post_charge_for_customer_id_using_token_id: {
351+
my $token = $stripe->post_token( card => $fake_card );
352+
my $customer = $stripe->post_customer();
353+
eval {
354+
$stripe->post_charge(
355+
amount => 100,
356+
currency => 'usd',
357+
customer => $customer->id,
358+
card => $token->id,
359+
);
360+
};
361+
if ($@) {
362+
my $e = $@;
363+
isa_ok $e, 'Net::Stripe::Error', 'error raised is an object';
364+
is $e->type, 'post_charge error', 'error type';
365+
like $e->message, qr/^Invalid value 'tok_.+' passed for parameter 'card'\. Charges for an existing customer can only accept a card id\.$/, 'error message';
366+
} else {
367+
fail 'post charge for customer with token id';
368+
}
369+
}
370+
371+
Post_charge_for_customer_id_using_card_id: {
372+
# customer may have multiple cards. allow ability to select a specific
373+
# card for a given charge.
374+
my $customer = $stripe->post_customer();
375+
my $card = $stripe->post_card(
376+
customer => $customer,
377+
card => $fake_card,
378+
);
379+
for ( 1..3 ) {
380+
my $other_card = $stripe->post_card(
381+
customer => $customer,
382+
card => $fake_card,
292383
);
293-
} qr/invalid_request_error/, 'missing card and customer';
384+
isnt $card->id, $other_card->id, 'different card id';
385+
}
386+
my $charge = $stripe->post_charge(
387+
amount => 100,
388+
currency => 'usd',
389+
customer => $customer->id,
390+
card => $card->id,
391+
);
392+
isa_ok $charge, 'Net::Stripe::Charge';
393+
ok $charge->paid, 'charge was paid';
394+
is $charge->status, 'paid', 'charge status is paid';
395+
is $charge->card->id, $card->id, 'charge card id matches';
396+
}
397+
398+
Post_charge_for_customer_id_using_card_hash: {
399+
my $customer = $stripe->post_customer();
400+
eval {
401+
$stripe->post_charge(
402+
amount => 100,
403+
currency => 'usd',
404+
customer => $customer->id,
405+
card => $fake_card,
406+
);
407+
};
408+
if ($@) {
409+
my $e = $@;
410+
isa_ok $e, 'Net::Stripe::Error', 'error raised is an object';
411+
is $e->type, 'post_charge error', 'error type';
412+
like $e->message, qr/^Invalid value 'HASH\(0x[0-9a-f]+\)' passed for parameter 'card'. Charges for an existing customer can only accept a card id\.$/, 'error message';
413+
} else {
414+
fail 'post charge for customer with card hashref';
415+
}
416+
}
294417

295-
throws_ok {
418+
Rainy_day: {
419+
# swallow the expected warning rather than have it print out during tests.
420+
local $SIG{__WARN__} = sub {};
421+
# Test a charge with no source or customer
422+
eval {
296423
$stripe->post_charge(
297424
amount => 3300,
298425
currency => 'usd',
299426
description => 'Wikileaks donation',
300-
customer => 'fake-customer-id',
301-
card => {
302-
number => '4242-4242-4242-4242',
303-
exp_month => $future->month,
304-
exp_year => $future->year,
305-
cvc => 123,
306-
name => 'Anonymous',
307-
},
308427
);
309-
} qr/invalid_request_error/, 'missing card and customer';
428+
429+
};
430+
if ($@) {
431+
my $e = $@;
432+
isa_ok $e, 'Net::Stripe::Error', 'error raised is an object';
433+
is $e->type, 'invalid_request_error', 'error type';
434+
is $e->message, 'Must provide source or customer.', 'error message';
435+
} else {
436+
fail 'missing card and customer';
437+
}
310438

311439
# Test an invalid currency
312440
eval {
313441
$stripe->post_charge(
314442
amount => 3300,
315443
currency => 'zzz',
316-
card => {
317-
number => '4242-4242-4242-4242',
318-
exp_month => $future->month,
319-
exp_year => $future->year,
320-
cvc => 123,
321-
},
444+
card => $fake_card,
322445
);
323446
};
324447
if ($@) {
@@ -330,8 +453,6 @@ Charges: {
330453
} else {
331454
fail 'report invalid currency';
332455
}
333-
close STDERR;
334-
open(STDERR, ">&", STDOUT);
335456
}
336457

337458
Charge_with_receipt_email: {

0 commit comments

Comments
 (0)