allow creating multiple related columns when updating or creating new records
Mike Wisener [Fri, 24 Jan 2014 03:09:17 +0000 (03:09 +0000)]
fixes RT#65168

Changes
dist.ini
lib/Catalyst/Controller/DBIC/API.pm
t/lib/RestTest/Controller/API/REST/Artist.pm
t/lib/RestTest/Controller/API/RPC/Artist.pm
t/rest/create.t
t/rest/update.t
t/rpc/create.t
t/rpc/update.t

diff --git a/Changes b/Changes
index 3840398..454929e 100644 (file)
--- a/Changes
+++ b/Changes
@@ -1,6 +1,8 @@
 Revision history for Catalyst-Controller-DBIC-API: {{ $dist->version }}
 
 {{ $NEXT }}
+ - Allow creating multiple related objects when updating or creating new
+   records (RT#65168)
 
 2.005001  2014-01-13 21:43:59+01:00 Europe/Vienna
  - Fixed test failures with JSON 2.90 (RT#90188, thanks Samuel Kaufman!)
index ba1412d..fb33f8f 100644 (file)
--- a/dist.ini
+++ b/dist.ini
@@ -6,6 +6,7 @@ author = Alexander Hartmaier <abraxxa@cpan.org>
 author = Florian Ragwitz <rafl@debian.org>
 author = Oleg Kostyuk <cub.uanic@gmail.com>
 author = Samuel Kaufman <sam@socialflow.com>
+author = Mike Wisener <xmikew@32ths.com>
 license = Perl_5
 copyright_holder = Luke Saunders, Nicholas Perez, Alexander Hartmaier, et al.
 
index 7cfb6e6..a60ef81 100644 (file)
@@ -686,22 +686,28 @@ sub validate_object {
 
         if ( ref $allowed_fields ) {
             my $related_source = $object->result_source->related_source($key);
-            my $related_params = $params->{$key};
+
+            # Could be an arrayref of hashrefs, or just a hashref.
+            my $related_rows = ref($params->{$key}) eq "ARRAY" ? $params->{$key} : [ $params->{$key} ];
             my %allowed_related_map = map { $_ => 1 } @$allowed_fields;
             my $allowed_related_cols =
                   ( $allowed_related_map{'*'} )
                 ? [ $related_source->columns ]
                 : $allowed_fields;
 
-            foreach my $related_col ( @{$allowed_related_cols} ) {
-                if (defined(
-                        my $related_col_value =
-                            $related_params->{$related_col}
-                    )
-                    )
-                {
-                    $values{$key}{$related_col} = $related_col_value;
+            foreach my $related_row ( @$related_rows ) {
+                my %valid_row;
+                foreach my $related_col ( @{$allowed_related_cols} ) {
+                    if (defined(
+                            my $related_col_value =
+                                $related_row->{$related_col}
+                        )
+                       )
+                    {
+                        $valid_row{$related_col} = $related_col_value;
+                    }
                 }
+                push(@{$values{$key}}, \%valid_row) if (keys %valid_row);
             }
         }
         else {
@@ -843,34 +849,43 @@ with the specified parameters.
 =cut
 
 sub update_object_relation {
-    my ( $self, $c, $object, $related_params, $relation ) = @_;
-    my $row = $object->find_related( $relation, {}, {} );
+    my ( $self, $c, $object, $related_records, $relation ) = @_;
+    my $row_count = scalar(@$related_records);
 
-    if ($row) {
-        foreach my $key ( keys %$related_params ) {
-            my $value = $related_params->{$key};
-            if ( ref($value) && !( reftype($value) eq reftype(JSON::true) ) )
-            {
-                $self->update_object_relation( $c, $row,
-                    delete $related_params->{$key}, $key );
-            }
+    # validate_object should always wrap single related records in [ ]
+    while (my $related_params = pop @$related_records) {
 
-            # accessor = colname
-            elsif ( $row->can($key) ) {
-                $row->$key($value);
-            }
+        # if we only have one row, don't need to worry about introspecting
+        my $row = $row_count > 1
+            ? $self->find_related_row( $object, $relation, $related_params )
+            : $object->find_related($relation, {}, {} );
 
-            # accessor != colname
-            else {
-                my $accessor =
-                    $row->result_source->column_info($key)->{accessor};
-                $row->$accessor($value);
+        if ($row) {
+            foreach my $key ( keys %$related_params ) {
+                my $value = $related_params->{$key};
+                if ( ref($value) && !( reftype($value) eq reftype(JSON::true) ) )
+                {
+                    $self->update_object_relation( $c, $row,
+                        delete $related_params->{$key}, $key );
+                }
+
+                # accessor = colname
+                elsif ( $row->can($key) ) {
+                    $row->$key($value);
+                }
+
+                # accessor != colname
+                else {
+                    my $accessor =
+                        $row->result_source->column_info($key)->{accessor};
+                    $row->$accessor($value);
+                }
             }
+            $row->update();
+        }
+        else {
+            $object->create_related( $relation, $related_params );
         }
-        $row->update();
-    }
-    else {
-        $object->create_related( $relation, $related_params );
     }
 }
 
@@ -907,10 +922,45 @@ sub insert_object_from_params {
     $object->insert;
 
     while ( my ( $k, $v ) = each %rels ) {
-        $object->create_related( $k, $v );
+        foreach my $row (@$v) {
+            $object->create_related( $k, $row );
+        }
     }
 }
 
+=method_protected find_related_row
+
+Attempts to find the related row by introspecting the result source and determining
+if we have enough data to properly update.
+
+=cut
+
+sub find_related_row {
+    my ($self, $object, $relation, $related_params) = @_;
+
+    # make a shallow copy, grep + hash slicing and autovivication creates undef
+    # values in the hash we operate on.
+    my $search_params = { %$related_params };
+
+    my @pri = $object->result_source->related_source($relation)->primary_columns;
+    my @have = grep { defined($_) } @{$search_params}{ @pri };
+    if (@have && @pri == @have) {
+        return $object->find_related($relation, @have, { key => 'primary' })
+    }
+
+    # if we were not passed a pri key, see if we meet any unique_constaints
+    my %constraints = $object->result_source->related_source($relation)->unique_constraints;
+    while ( my ($constraint_name,$constraints) = each %constraints ) {
+        my @needed = @$constraints;
+        my @have = grep { defined($_) } @{$search_params}{ @needed };
+        return $object->find_related($relation, @have, { key => $constraint_name })
+            if (@have && @have == @needed);
+    }
+
+    # didn't find anything
+    return undef;
+}
+
 =method_protected delete_objects
 
 Iterates through each object calling L</delete_object>.
index d2ceada..ea5a4bc 100644 (file)
@@ -7,8 +7,8 @@ __PACKAGE__->config
     ( action => { setup => { PathPart => 'artist', Chained => '/api/rest/rest_base' } },
       class => 'RestTestDB::Artist',
       create_requires => ['name'],
-      create_allows => ['name'],
-      update_allows => ['name'],
+      create_allows => ['name', { cds => ['*'] }],
+      update_allows => ['name', { cds => ['*'] }],
       prefetch_allows => [[qw/ cds /],{ 'cds' => 'tracks'}],
       );
 
index 2d0367d..e9a23cd 100644 (file)
@@ -8,9 +8,9 @@ __PACKAGE__->config
     ( action => { setup => { PathPart => 'artist', Chained => '/api/rpc/rpc_base' } },
       class => 'RestTestDB::Artist',
       create_requires => ['name'],
-      create_allows => ['name'],
-      update_allows => ['name'],
+      create_allows => ['name', { cds => ['*'] }],
+      update_allows => ['name', { cds => ['*'] }],
       prefetch_allows => [[qw/ cds /],{ 'cds' => 'tracks'}],
-      );
+    );
 
 1;
index ba850c1..17b3d34 100644 (file)
@@ -105,4 +105,57 @@ my $producer_create_url = "$base/api/rest/producer";
         'json for bulk create returned' );
 }
 
+# test create of single related row
+{
+    my $test_data = $json->encode(
+        { name => 'Futuristic Artist', cds => { 'title' => 'snarky cd name', year => '3030' } }
+    );
+    my $req = PUT($artist_create_url);
+    $req->content_type('text/x-json');
+    $req->content_length(
+        do { use bytes; length($test_data) }
+    );
+    $req->content($test_data);
+    $mech->request($req);
+    cmp_ok( $mech->status, '==', 200, 'request with single related row okay' );
+    my $count = $schema->resultset('Artist')
+        ->search({ name => 'Futuristic Artist' })
+        ->count;
+    ok( $count, 'record with related object created' );
+    $count = $schema->resultset('Artist')
+        ->search_related('cds', { title => 'snarky cd name' })
+        ->count;
+    ok( $count, "record's related object created" );
+}
+
+# test create of multiple related rows
+{
+    my $test_data = $json->encode(
+        { name => 'Futuristic Artist 2',
+          cds => [
+            { 'title' => 'snarky cd name 2', year => '3030' },
+            { 'title' => 'snarky cd name 3', year => '3030' },
+          ]
+        }
+    );
+
+    my $req = PUT($artist_create_url);
+    $req->content_type('text/x-json');
+    $req->content_length(
+        do { use bytes; length($test_data) }
+    );
+    $req->content($test_data);
+    $mech->request($req);
+    cmp_ok( $mech->status, '==', 200, 'request with multiple related rows okay' );
+    my $count = $schema->resultset('Artist')
+        ->search({ name => 'Futuristic Artist 2' })
+        ->count;
+    ok( $count, 'record with related object created' );
+    $count = $schema->resultset('Artist')
+        ->search_related('cds', { title => ['snarky cd name 2','snarky cd name 3'] })
+        ->count;
+    ok( $count == 2, "record's related objects created" ) or explain diag $count;
+
+}
+
 done_testing();
index 7cd6ce5..cfcd2db 100644 (file)
@@ -25,6 +25,9 @@ my $track_url         = "$base/api/rest/track/";
 my $track_update_url  = $track_url . $track->id;
 my $tracks_update_url = $track_url;
 
+my $artist_url        = "$base/api/rest/artist/";
+my $artist_update_url = $artist_url . "1";
+
 # test invalid track id caught
 {
     diag 'DBIx::Class warns about a non-numeric id which is ok because we test for that too';
@@ -101,6 +104,79 @@ my $tracks_update_url = $track_url;
     is( $track->cd->year, 2009, 'related row updated' );
 }
 
+{
+    my $test_data = $json->encode(
+        { name => 'Caterwauler B. McCrae',
+          cds => [
+            {
+                cdid => 1,
+                title => 'All the bees are gone',
+                year => 3030,
+            },
+            {
+                cdid => 2,
+                title => 'All the crops are gone',
+                year => 3031
+            }
+          ]
+        }
+    );
+
+    my $req = POST( $artist_update_url, Content => $test_data );
+    $req->content_type('text/x-json');
+    $mech->request($req);
+
+    cmp_ok( $mech->status, '==', 200, 'Multi-row update returned 200 OK' );
+
+    my $artist = $schema->resultset('Artist')->search({ artistid => 1 });
+    ok ($artist->next->name eq "Caterwauler B. McCrae", "mutliple related row parent record update");
+
+    # make sure the old cds don't exist, it's possible we just inserted the new rows instead of updating them
+    my $old_cds = $artist->search_related('cds', { title => ['Spoonful of bees', 'Forkful of bees'] } )->count;
+    ok ($old_cds == 0, 'update performed update and not create on related rows');
+
+    my @cds = $artist->search_related('cds', { year => ['3030', '3031'] }, { order_by => 'year' })->all;
+    ok (@cds == 2, 'update modified proper number of related rows');
+    ok ($cds[0]->title eq 'All the bees are gone', 'update modified column on related row');
+    ok ($cds[1]->title eq 'All the crops are gone', 'update modified column on second related row');
+}
+
+# update related rows using only unique_constraint on CD vs. primary key
+# update the year on constraint match
+{
+    my $test_data = $json->encode(
+        { name => 'Caterwauler McCrae',
+          cds => [
+            {
+                artist => 1,
+                title => 'All the bees are gone',
+                year => 3032,
+            },
+            {
+                artist => 1,
+                title => 'All the crops are gone',
+                year => 3032
+            }
+          ]
+        }
+    );
+
+    my $req = POST( $artist_update_url, Content => $test_data );
+    $req->content_type('text/x-json');
+    $mech->request($req);
+
+    cmp_ok( $mech->status, '==', 200, 'Multi-row unique constraint update returned 200 OK' );
+
+    my $artist = $schema->resultset('Artist')->search({ artistid => 1 });
+    ok ($artist->next->name eq "Caterwauler McCrae", "multi-row unique constraint related row parent record updated");
+
+    my $old_cds = $artist->search_related('cds', { year => ['3030', '3031'] }, { order_by => 'year' })->count;
+    ok ( $old_cds == 0, 'multi-row update with unique constraint updated year' );
+
+    my $cds = $artist->search_related('cds', { 'year' => 3032 } )->count;
+    ok ( $cds == 2, 'multi-row update with unique constraint okay' );
+}
+
 # bulk_update existing objects
 {
 
index f2757a3..a12e766 100644 (file)
@@ -78,6 +78,59 @@ my $producer_create_url   = "$base/api/rpc/producer/create";
     );
 }
 
+# test create of single related row
+{
+    my $test_data = $json->encode(
+        { name => 'Futuristic Artist', cds => { 'title' => 'snarky cd name', year => '3030' } }
+    );
+    my $req = PUT($artist_create_url);
+    $req->content_type('text/x-json');
+    $req->content_length(
+        do { use bytes; length($test_data) }
+    );
+    $req->content($test_data);
+    $mech->request($req);
+    cmp_ok( $mech->status, '==', 200, 'request with single related row okay' );
+    my $count = $schema->resultset('Artist')
+        ->search({ name => 'Futuristic Artist' })
+        ->count;
+    ok( $count, 'record with related object created' );
+    $count = $schema->resultset('Artist')
+        ->search_related('cds', { title => 'snarky cd name' })
+        ->count;
+    ok( $count, "record's related object created" );
+}
+
+# test create of multiple related rows
+{
+    my $test_data = $json->encode(
+        { name => 'Futuristic Artist 2',
+          cds => [
+            { 'title' => 'snarky cd name 2', year => '3030' },
+            { 'title' => 'snarky cd name 3', year => '3030' },
+          ]
+        }
+    );
+
+    my $req = PUT($artist_create_url);
+    $req->content_type('text/x-json');
+    $req->content_length(
+        do { use bytes; length($test_data) }
+    );
+    $req->content($test_data);
+    $mech->request($req);
+    cmp_ok( $mech->status, '==', 200, 'request with multiple related rows okay' );
+    my $count = $schema->resultset('Artist')
+        ->search({ name => 'Futuristic Artist 2' })
+        ->count;
+    ok( $count, 'record with related object created' );
+    $count = $schema->resultset('Artist')
+        ->search_related('cds', { title => ['snarky cd name 2','snarky cd name 3'] })
+        ->count;
+    ok( $count == 2, "record's related objects created" ) or explain diag $count;
+
+}
+
 # test stash config handling
 {
     my $req = POST(
@@ -125,4 +178,6 @@ my $producer_create_url   = "$base/api/rpc/producer/create";
     cmp_ok( $mech->status, '==', 400, 'invalid param value produces error' );
 }
 
+# test creating record with multiple related-rows
+
 done_testing();
index c0fcc77..a877759 100644 (file)
@@ -26,6 +26,8 @@ my $any_track_update_url =
     "$base/api/rpc/any/track/id/" . $track->id . "/update";
 my $tracks_update_url = "$base/api/rpc/track/update";
 
+my $artist_update_url = "$base/api/rpc/artist/id/1/update";
+
 # test invalid track id caught
 {
     diag 'DBIx::Class warns about a non-numeric id which is ok because we test for that too';
@@ -124,6 +126,79 @@ my $tracks_update_url = "$base/api/rpc/track/update";
 }
 
 {
+    my $test_data = $json->encode(
+        { name => 'Caterwauler B. McCrae',
+          cds => [
+            {
+                cdid => 1,
+                title => 'All the bees are gone',
+                year => 3030,
+            },
+            {
+                cdid => 2,
+                title => 'All the crops are gone',
+                year => 3031
+            }
+          ]
+        }
+    );
+
+    my $req = POST( $artist_update_url, Content => $test_data );
+    $req->content_type('text/x-json');
+    $mech->request($req);
+
+    cmp_ok( $mech->status, '==', 200, 'Multi-row update returned 200 OK' );
+
+    my $artist = $schema->resultset('Artist')->search({ artistid => 1 });
+    ok ($artist->next->name eq "Caterwauler B. McCrae", "mutliple related row parent record update");
+
+    # make sure the old cds don't exist, it's possible we just inserted the new rows instead of updating them
+    my $old_cds = $artist->search_related('cds', { title => ['Spoonful of bees', 'Forkful of bees'] } )->count;
+    ok ($old_cds == 0, 'update performed update and not create on related rows');
+
+    my @cds = $artist->search_related('cds', { year => ['3030', '3031'] }, { order_by => 'year' })->all;
+    ok (@cds == 2, 'update modified proper number of related rows');
+    ok ($cds[0]->title eq 'All the bees are gone', 'update modified column on related row');
+    ok ($cds[1]->title eq 'All the crops are gone', 'update modified column on second related row');
+}
+
+# update related rows using only unique_constraint on CD vs. primary key
+# update the year on constraint match
+{
+    my $test_data = $json->encode(
+        { name => 'Caterwauler McCrae',
+          cds => [
+            {
+                artist => 1,
+                title => 'All the bees are gone',
+                year => 3032,
+            },
+            {
+                artist => 1,
+                title => 'All the crops are gone',
+                year => 3032
+            }
+          ]
+        }
+    );
+
+    my $req = POST( $artist_update_url, Content => $test_data );
+    $req->content_type('text/x-json');
+    $mech->request($req);
+
+    cmp_ok( $mech->status, '==', 200, 'Multi-row unique constraint update returned 200 OK' );
+
+    my $artist = $schema->resultset('Artist')->search({ artistid => 1 });
+    ok ($artist->next->name eq "Caterwauler McCrae", "multi-row unique constraint related row parent record updated");
+
+    my $old_cds = $artist->search_related('cds', { year => ['3030', '3031'] }, { order_by => 'year' })->count;
+    ok ( $old_cds == 0, 'multi-row update with unique constraint updated year' );
+
+    my $cds = $artist->search_related('cds', { 'year' => 3032 } )->count;
+    ok ( $cds == 2, 'multi-row update with unique constraint okay' );
+}
+
+{
     my $req = POST( $any_track_update_url, { title => 'baa' } );
     $mech->request( $req, $content_type );
     cmp_ok( $mech->status, '==', 200, 'Stash update okay' );
@@ -141,6 +216,7 @@ my $tracks_update_url = "$base/api/rpc/track/update";
     is( $track->get_column('position'), '14', 'Position changed' );
 }
 
+
 # bulk_update existing objects
 {