Make tests pass for new definitive chaining
nperez [Mon, 1 Mar 2010 06:55:58 +0000 (00:55 -0600)]
lib/Catalyst/Controller/DBIC/API.pm
lib/Catalyst/Controller/DBIC/API/REST.pm
lib/Catalyst/Controller/DBIC/API/RPC.pm
lib/Catalyst/Controller/DBIC/API/RequestArguments.pm
t/lib/RestTest/Model/RestTestDB.pm
t/rest/create.t
t/rest/update.t

index ef19f08..e257ed1 100644 (file)
@@ -56,6 +56,23 @@ __PACKAGE__->config();
   # /api/rpc/artist/id/[id]/update
 =cut
 
+=method_private begin
+
+ :Private
+
+begin is provided in the base class to setup the Catalyst Request object, by applying the DBIC::API::Request role.
+
+=cut
+
+sub begin :Private
+{
+    my ($self, $c) = @_;
+
+    Catalyst::Controller::DBIC::API::Request->meta->apply($c->req)
+        unless Moose::Util::does_role($c->req, 'Catalyst::Controller::DBIC::API::Request');
+}
+
+
 =method_protected setup
 
  :Chained('specify.in.subclass.config') :CaptureArgs(0) :PathPart('specify.in.subclass.config')
@@ -63,7 +80,8 @@ __PACKAGE__->config();
 This action is the chain root of the controller. It must either be overridden or configured to provide a base pathpart to the action and also a parent action. For example, for class MyAppDB::Track you might have
 
   package MyApp::Controller::API::RPC::Track;
-  use base qw/Catalyst::Controller::DBIC::API::RPC/;
+  use Moose; 
+  BEGIN { extends 'Catalyst::Controller::DBIC::API::RPC'; }
 
   __PACKAGE__->config
     ( action => { setup => { PathPart => 'track', Chained => '/api/rpc/rpc_base' } }, 
@@ -78,20 +96,16 @@ This action is the chain root of the controller. It must either be overridden or
     $self->next::method($c);
   }
 
-This action will populate $c->req->current_result_set with $self->stored_result_source->resultset for other actions in the chain to use.
+This action does nothing by default.
 
 =cut
 
-sub setup :Chained('specify.in.subclass.config') :CaptureArgs(0) :PathPart('specify.in.subclass.config')
-{
-    my ($self, $c) = @_;
-
-    Catalyst::Controller::DBIC::API::Request->meta->apply($c->req)
-        unless Moose::Util::does_role($c->req, 'Catalyst::Controller::DBIC::API::Request');
-}
+sub setup :Chained('specify.in.subclass.config') :CaptureArgs(0) :PathPart('specify.in.subclass.config') {}
 
 =method_protected deserialize
 
+ :Chained('setup') :CaptureArgs(0) :PathPart('') :ActionClass('Deserialize')
+
 deserialize absorbs the request data and transforms it into useful bits by using CGI::Expand->expand_hash and a smattering of JSON::Any->from_json for a handful of arguments. Current only the following arguments are capable of being expressed as JSON:
 
     search_arg
@@ -101,7 +115,7 @@ deserialize absorbs the request data and transforms it into useful bits by using
     grouped_by_arg
     prefetch_arg
 
-It should be noted that arguments can used mixed modes in with some caveats. Each top level arg can be expressed as CGI::Expand with their immediate child keys expressed as JSON.
+It should be noted that arguments can used mixed modes in with some caveats. Each top level arg can be expressed as CGI::Expand with their immediate child keys expressed as JSON when sending the data application/x-www-form-urlencoded. Otherwise, you can send content as raw json and it will be deserialized as is with no CGI::Expand expasion.
 
 =cut
 
@@ -232,37 +246,54 @@ This action is the chain root for object level actions (such as create, update,
 sub objects_no_id :Chained('deserialize') :CaptureArgs(0) :PathPart('')
 {
     my ($self, $c) = @_;
-
-    my $vals = $c->req->request_data->{$self->data_root};
-    unless(defined($vals))
-    {
-        # no data root, assume the request_data itself is the payload
-        $vals = [$c->req->request_data];
-    }
-    unless(reftype($vals) eq 'ARRAY')
-    {
-        $c->log->error('Invalid request data');
-        $self->push_error($c, { message => 'Invalid request data' });
-        $c->detach();
-    }
-
-    foreach my $val (@$vals)
+    
+    if($c->req->has_request_data)
     {
-        unless(exists($val->{id}))
+        my $data = $c->req->request_data;
+        my $vals;
+        
+        if(exists($data->{$self->data_root}) && defined($data->{$self->data_root}))
         {
-            $c->req->add_object([$c->req->current_result_set->new_result({}), $val]);
-            next;
+            my $root = $data->{$self->data_root};
+            if(reftype($root) eq 'ARRAY')
+            {
+                $vals = $root;
+            }
+            elsif(reftype($root) eq 'HASH')
+            {
+                $vals = [$root];
+            }
+            else
+            {
+                $c->log->error('Invalid request data');
+                $self->push_error($c, { message => 'Invalid request data' });
+                $c->detach();
+            }
         }
-
-        try
+        else
         {
-            $c->req->add_object([$self->object_lookup($c, $val->{id}), $val]);
+            # no data root, assume the request_data itself is the payload
+            $vals = [$c->req->request_data];
         }
-        catch
+
+        foreach my $val (@$vals)
         {
-            $c->log->error($_);
-            $self->push_error($c, { message => $_ });
-            $c->detach();
+            unless(exists($val->{id}))
+            {
+                $c->req->add_object([$c->req->current_result_set->new_result({}), $val]);
+                next;
+            }
+
+            try
+            {
+                $c->req->add_object([$self->object_lookup($c, $val->{id}), $val]);
+            }
+            catch
+            {
+                $c->log->error($_);
+                $self->push_error($c, { message => $_ });
+                $c->detach();
+            }
         }
     }
 }
@@ -326,6 +357,9 @@ sub list :Private
     $self->list_munge_parameters($c);
     $self->list_perform_search($c);
     $self->list_format_output($c);
+
+    # make sure there are no objects lingering
+    $c->req->clear_objects(); 
 }
 
 =method_protected list_munge_parameters
@@ -441,7 +475,7 @@ sub item :Private
     }
     else
     {
-        $c->stash->{response}->{$self->item_root} = $self->each_object_inflate($c, $c->req->get_object(0));
+        $c->stash->{response}->{$self->item_root} = $self->each_object_inflate($c, $c->req->get_object(0)->[0]);
     }
 }
 
@@ -1011,8 +1045,6 @@ For example if you wanted create to return the JSON for the newly created object
 
 It should be noted that the return_object attribute will produce the above result for you, free of charge.
 
-For REST the only difference besides the class names would be that create should be :Private rather than an endpoint.
-
 Similarly you might want create, update and delete to all forward to the list action once they are done so you can refresh your view. This should also be simple enough.
 
 If more extensive customization is required, it is recommened to peer into the roles that comprise the system and make use 
index b40b5de..23655e2 100644 (file)
@@ -38,7 +38,7 @@ As described in L<Catalyst::Controller::DBIC::API/setup>, this action is the cha
 
 =method_protected no_id
 
-Chained: L</object_no_id>
+Chained: L</objects_no_id>
 PathPart: none
 CaptureArgs: 0
 
@@ -50,7 +50,7 @@ GET: forwards to L<Catalyst::Controller::DBIC::API/list>
 
 =cut
 
-sub no_id : Chained('object_no_id') PathPart('') ActionClass('REST') :CaptureArgs(0) {}
+sub no_id : Chained('objects_no_id') PathPart('') ActionClass('REST') {}
 
 sub no_id_PUT
 {
@@ -90,7 +90,7 @@ GET: forwards to L<Catalyst::Controller::DBIC::API/item>
 
 =cut
 
-sub with_id :Chained('object_with_id') :PathPart('') :ActionClass('REST') :CaptureArgs(0) {}
+sub with_id :Chained('object_with_id') :PathPart('') :ActionClass('REST') {}
 
 sub with_id_PUT
 {
index 9655497..abcf404 100644 (file)
@@ -5,7 +5,7 @@ use Moose;
 BEGIN { extends 'Catalyst::Controller::DBIC::API'; }
 
 __PACKAGE__->config(
-    'action'    => { object => { PathPart => 'id' } }, 
+    'action'    => { object_with_id => { PathPart => 'id' } }, 
     'default'   => 'application/json',
     'stash_key' => 'response',
     'map'       => {
@@ -52,7 +52,7 @@ sub index : Chained('setup') PathPart('') Args(0) {
 
 =method_protected create
 
-Chained: L</object_no_id>
+Chained: L</objects_no_id>
 PathPart: create
 CaptureArgs: 0
 
@@ -60,7 +60,7 @@ Provides an endpoint to the functionality described in L<Catalyst::Controller::D
 
 =cut
 
-sub create :Chained('object_no_id') :PathPart('create') :Args(0)
+sub create :Chained('objects_no_id') :PathPart('create')
 {
        my ($self, $c) = @_;
     $c->forward('update_or_create');
@@ -76,10 +76,10 @@ Provides an endpoint to the functionality described in L<Catalyst::Controller::D
 
 =cut
 
-sub list :Chained('deserialize') :PathPart('list') :Args(0) {
+sub list :Chained('deserialize') :PathPart('list')
+{
        my ($self, $c) = @_;
-
-        $self->next::method($c);
+    $self->next::method($c);
 }
 
 =method_protected item
@@ -92,10 +92,10 @@ Provides an endpoint to the functionality described in L<Catalyst::Controller::D
 
 =cut
 
-sub item :Chained('object_with_id') :PathPart('') :Args(0) {
+sub item :Chained('object_with_id') :PathPart('')
+{
     my ($self, $c) = @_;
-
-    $c->forward('view');
+    $self->next::method($c);
 }
 
 =method_protected update
@@ -108,9 +108,9 @@ Provides an endpoint to the functionality described in L<Catalyst::Controller::D
 
 =cut
 
-sub update :Chained('object_with_id') :PathPart('update') :Args(0) {
+sub update :Chained('object_with_id') :PathPart('update')
+{
     my ($self, $c) = @_;
-
     $c->forward('update_or_create');
 }
 
@@ -124,7 +124,7 @@ Provides an endpoint to the functionality described in L<Catalyst::Controller::D
 
 =cut
 
-sub delete :Chained('object_with_id') :PathPart('delete') :Args(0)
+sub delete :Chained('object_with_id') :PathPart('delete')
 {
     my ($self, $c) = @_;
     $self->next::method($c);
@@ -132,7 +132,7 @@ sub delete :Chained('object_with_id') :PathPart('delete') :Args(0)
 
 =method_protected update_bulk
 
-Chained: L</object_no_id>
+Chained: L</objects_no_id>
 PathPart: update
 Args: 0
 
@@ -140,7 +140,7 @@ Provides an endpoint to the functionality described in L<Catalyst::Controller::D
 
 =cut
 
-sub update_bulk :Chained('object_no_id') :PathPart('update') :Args(0)
+sub update_bulk :Chained('objects_no_id') :PathPart('update')
 {
     my ($self, $c) = @_;
     $c->forward('update_or_create');
@@ -148,7 +148,7 @@ sub update_bulk :Chained('object_no_id') :PathPart('update') :Args(0)
 
 =method_protected delete_bulk
 
-Chained: L</object_no_id>
+Chained: L</objects_no_id>
 PathPart: delete
 Args: 0
 
@@ -156,7 +156,7 @@ Provides an endpoint to the functionality described in L<Catalyst::Controller::D
 
 =cut
 
-sub delete_bulk :Chained('object_no_id') :PathPart('delete') :Args(0)
+sub delete_bulk :Chained('objects_no_id') :PathPart('delete')
 {
     my ($self, $c) = @_;
     $self->next::method($c);
index ad9280e..7b961b4 100644 (file)
@@ -462,10 +462,12 @@ request_data holds the raw (but deserialized) data for ths request
         is => 'ro',
         isa => HashRef,
         writer => '_set_request_data',
+        predicate => 'has_request_data',
         trigger => sub
         {
             my ($self, $new) = @_;
             my $controller = $self->_controller;
+            return unless defined($new) && keys %$new;
             $self->_set_prefetch($new->{$controller->prefetch_arg}) if exists $new->{$controller->prefetch_arg};
             $self->_set_select($new->{$controller->select_arg}) if exists $new->{$controller->select_arg};
             $self->_set_as($new->{$controller->as_arg}) if exists $new->{$controller->as_arg};
index 7d89f5c..4e1fb5b 100644 (file)
@@ -9,7 +9,7 @@ use Catalyst::Utils;
 __PACKAGE__->config(
                                        schema_class => 'RestTest::Schema',
                                        connect_info => [
-                        "DBI:SQLite:t/var/DBIxClass.db",
+                        "dbi:SQLite:t/var/DBIxClass.db",
                         "",
                         "",
                                           {AutoCommit => 1}
index 4fdbfb5..58a6ea8 100644 (file)
@@ -30,7 +30,7 @@ my $producer_create_url = "$base/api/rest/producer";
                                                 );
        $req->content( $test_data );
        $mech->request($req);
-
+    
        cmp_ok( $mech->status, '==', 400, 'attempt without required params caught' );
        my $response = JSON::Any->Load( $mech->content);
        like($response->{messages}->[0], qr/No value supplied for name and no default/, 'correct message returned' );
index ad4c025..49f0051 100644 (file)
@@ -71,7 +71,7 @@ my $track_update_url = "$base/api/rest/track/" . $track->id;
 }
 
 {
-       my $test_data = JSON::Any->Dump({ title => 'monkey monkey', 'cd.year' => 2009 });
+       my $test_data = JSON::Any->Dump({ title => 'monkey monkey', 'cd' => { year => 2009 } });
        my $req = POST( $track_update_url, Content => $test_data );
        $req->content_type('text/x-json');
        $mech->request($req);