[Catalyst-commits] r9535 - in
Catalyst-Controller-DBIC-API/1.001/trunk: .
lib/Catalyst/Controller/DBIC lib/Catalyst/Controller/DBIC/API
t/rest t/rpc
timbunce at dev.catalyst.perl.org
timbunce at dev.catalyst.perl.org
Fri Mar 20 17:43:34 GMT 2009
Author: timbunce
Date: 2009-03-20 17:43:33 +0000 (Fri, 20 Mar 2009)
New Revision: 9535
Modified:
Catalyst-Controller-DBIC-API/1.001/trunk/Changes
Catalyst-Controller-DBIC-API/1.001/trunk/lib/Catalyst/Controller/DBIC/API.pm
Catalyst-Controller-DBIC-API/1.001/trunk/lib/Catalyst/Controller/DBIC/API/Base.pm
Catalyst-Controller-DBIC-API/1.001/trunk/lib/Catalyst/Controller/DBIC/API/REST.pm
Catalyst-Controller-DBIC-API/1.001/trunk/t/rest/delete.t
Catalyst-Controller-DBIC-API/1.001/trunk/t/rest/update.t
Catalyst-Controller-DBIC-API/1.001/trunk/t/rpc/list_json_search.t
Log:
Return 404 if object() finds no object
Improve behaviour of end() for more status values and error cases.
Modified: Catalyst-Controller-DBIC-API/1.001/trunk/Changes
===================================================================
--- Catalyst-Controller-DBIC-API/1.001/trunk/Changes 2009-03-20 15:47:40 UTC (rev 9534)
+++ Catalyst-Controller-DBIC-API/1.001/trunk/Changes 2009-03-20 17:43:33 UTC (rev 9535)
@@ -1,5 +1,9 @@
Revision history for Catalyst-Controller-DBIC-API
+1.003000 ???
+- Return 404 if object() finds no object
+- Improve behaviour of end() for more status values and error cases.
+
1.002000
- Better error handing when unable to parse search arg
- Added setup_dbic_args_method config option
Modified: Catalyst-Controller-DBIC-API/1.001/trunk/lib/Catalyst/Controller/DBIC/API/Base.pm
===================================================================
--- Catalyst-Controller-DBIC-API/1.001/trunk/lib/Catalyst/Controller/DBIC/API/Base.pm 2009-03-20 15:47:40 UTC (rev 9534)
+++ Catalyst-Controller-DBIC-API/1.001/trunk/lib/Catalyst/Controller/DBIC/API/Base.pm 2009-03-20 17:43:33 UTC (rev 9535)
@@ -51,12 +51,17 @@
my $req_params = (grep { ref $_ } values %{$c->req->params}) ? $c->req->params : $self->expand_hash($c->req->params);
# if expand_hash didn't do anything, try json
unless (exists $req_params->{search} && ref $req_params->{search} eq 'HASH') {
+ my $search = $c->req->params->{search};
eval {
- $req_params->{search} = exists $c->req->params->{search} ? JSON::Any->from_json($c->req->params->{search}) : undef;
+ $req_params->{search} = defined $search ? JSON::Any->from_json($search) : undef;
};
if ($@) {
# json didn't work either. looks like it's screwed.
- $self->push_error($c, { message => "can not parse search arg" });
+ # if it looks like json then try to be helpful
+ chomp $@;
+ my $hint = ($search =~ /^{/) ? ": $@" : "";
+ $hint =~ s{ at /.*?pm line \d+.*}{}; # tidy up
+ $self->push_error($c, { message => "Can't parse search arg$hint" });
return;
}
}
@@ -337,23 +342,23 @@
my ($self, $c) = @_;
# check for errors
- my $default_status;
+ my $status = $c->res->status || 200;
+ my $errors = $self->get_errors($c);
- # Check for errors caught elsewhere
- if ( $c->res->status and $c->res->status != 200 ) {
- $default_status = $c->res->status;
- $c->stash->{response}->{success} = 'false';
- } elsif ($self->get_errors($c)) {
- $c->stash->{response}->{messages} = $self->get_errors($c);
- $c->stash->{response}->{success} = 'false';
- $default_status = 400;
- } else {
- $c->stash->{response}->{success} = 'true';
- $default_status = 200;
- }
+ if ($errors) {
+ $status = 400 if $status < 400;
+ $c->stash->{response}->{messages} = $errors;
+ }
+
+ # if the user hasn't explicitly set stash->{response}->{success}
+ # then default it based on the status we're returning
+ $c->stash->{response}->{success} ||=
+ ($status >= 400) ? 'false' : 'true';
- delete $c->stash->{response}->{list} unless ($default_status == 200);
- $c->res->status( $default_status || 200 );
+ # don't return primary data if there was an error
+ delete $c->stash->{response}->{list} if $status >= 400;
+
+ $c->res->status( $status );
$c->forward('serialize');
}
Modified: Catalyst-Controller-DBIC-API/1.001/trunk/lib/Catalyst/Controller/DBIC/API/REST.pm
===================================================================
--- Catalyst-Controller-DBIC-API/1.001/trunk/lib/Catalyst/Controller/DBIC/API/REST.pm 2009-03-20 15:47:40 UTC (rev 9534)
+++ Catalyst-Controller-DBIC-API/1.001/trunk/lib/Catalyst/Controller/DBIC/API/REST.pm 2009-03-20 17:43:33 UTC (rev 9535)
@@ -94,6 +94,7 @@
my $object = $c->stash->{$self->rs_stash_key}->find( $id );
unless ($object) {
$self->push_error($c, { message => "Invalid id" });
+ $c->res->status(404); # Not Found
$c->detach; # no point continuing
}
$c->stash->{$self->object_stash_key} = $object;
Modified: Catalyst-Controller-DBIC-API/1.001/trunk/lib/Catalyst/Controller/DBIC/API.pm
===================================================================
--- Catalyst-Controller-DBIC-API/1.001/trunk/lib/Catalyst/Controller/DBIC/API.pm 2009-03-20 15:47:40 UTC (rev 9534)
+++ Catalyst-Controller-DBIC-API/1.001/trunk/lib/Catalyst/Controller/DBIC/API.pm 2009-03-20 17:43:33 UTC (rev 9535)
@@ -269,10 +269,18 @@
=head2 end
-If the request was successful then $c->stash->{response}->{success} is set to 1, if not then it is set to 0 and $c->stash->{response}->{messages} set to an arrayref containing all error messages.
+If there were errors then $c->stash->{response}->{messages} is set to an
+arrayref containing the error messages.
-Then the contents of $c->stash->{response} are serialized using L<Catalyst::Action::Serialize>.
+The $c->res->status is set to 400 if there were any errors and it's not
+already >= 400, otherwise it's set to 200.
+If $c->stash->{response}->{success} is not already set then it's set to 'false'
+if status is >= 400, otherwise it's set to 'false'.
+
+If the status is >= 400 then $c->stash->{response}->{list} is deleted so as not to return
+bulk data for a request with errors.
+
=head1 EXTENDING
By default the create, delete and update actions will not return anything apart from the success parameter set in L</end>, often this is not ideal but the required behaviour varies from application to application. So normally it's sensible to write an intermediate class which your main controller classes subclass from. For example if you wanted create to return the JSON for the newly created object you might have something like:
Modified: Catalyst-Controller-DBIC-API/1.001/trunk/t/rest/delete.t
===================================================================
--- Catalyst-Controller-DBIC-API/1.001/trunk/t/rest/delete.t 2009-03-20 15:47:40 UTC (rev 9534)
+++ Catalyst-Controller-DBIC-API/1.001/trunk/t/rest/delete.t 2009-03-20 17:43:33 UTC (rev 9535)
@@ -37,5 +37,5 @@
my $req = HTTP::Request->new( DELETE => $track_delete_url );
$req->content_type('text/x-json');
$mech->request($req);
- cmp_ok( $mech->status, '==', 400, 'Attempt to delete again caught' );
+ cmp_ok( $mech->status, '==', 404, 'Attempt to delete again caught' );
}
Modified: Catalyst-Controller-DBIC-API/1.001/trunk/t/rest/update.t
===================================================================
--- Catalyst-Controller-DBIC-API/1.001/trunk/t/rest/update.t 2009-03-20 15:47:40 UTC (rev 9534)
+++ Catalyst-Controller-DBIC-API/1.001/trunk/t/rest/update.t 2009-03-20 17:43:33 UTC (rev 9535)
@@ -32,7 +32,7 @@
$req->content_type('text/x-json');
$mech->request($req);
- cmp_ok( $mech->status, '==', 400, 'Attempt with invalid track id caught' );
+ cmp_ok( $mech->status, '==', 404, 'Attempt with invalid track id caught' );
my $response = JSON::Syck::Load( $mech->content);
is_deeply( $response->{messages}, ['Invalid id'], 'correct message returned' );
Modified: Catalyst-Controller-DBIC-API/1.001/trunk/t/rpc/list_json_search.t
===================================================================
--- Catalyst-Controller-DBIC-API/1.001/trunk/t/rpc/list_json_search.t 2009-03-20 15:47:40 UTC (rev 9534)
+++ Catalyst-Controller-DBIC-API/1.001/trunk/t/rpc/list_json_search.t 2009-03-20 17:43:33 UTC (rev 9535)
@@ -29,7 +29,10 @@
cmp_ok( $mech->status, '==', 400, 'attempt with gibberish json not okay' );
my $response = JSON::Syck::Load( $mech->content);
- is_deeply( { messages => ['can not parse search arg'], success => 'false' }, $response, 'correct data returned for gibberish in search' );
+ $response->{messages}[0] =~ s/arg: .*/arg/ # don't depend on exact $@ error text
+ if ref $response->{messages} eq 'ARRAY';
+ is_deeply( $response, { messages => ["Can't parse search arg"], success => 'false' },
+ 'correct data returned for gibberish in search' );
}
More information about the Catalyst-commits
mailing list