From: Peter Rabbitson <ribasushi@cpan.org>
Date: Wed, 25 Feb 2009 08:14:16 +0000 (+0000)
Subject: Wrap dangerous Ordered operations in transactions (still needs optimisations wrt... 
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=8f5357074109c0b803f1565921cb13f85449d8d1;p=dbsrgits%2FDBIx-Class-Historic.git

Wrap dangerous Ordered operations in transactions (still needs optimisations wrt sibling shifting)
---

diff --git a/lib/DBIx/Class/Ordered.pm b/lib/DBIx/Class/Ordered.pm
index e380fd2..dc51856 100644
--- a/lib/DBIx/Class/Ordered.pm
+++ b/lib/DBIx/Class/Ordered.pm
@@ -349,8 +349,9 @@ sub move_to {
 
     my $position_column = $self->position_column;
 
-    # FIXME this needs to be wrapped in a transaction
     {
+        my $guard = $self->result_source->schema->txn_scope_guard;
+
         my ($direction, @between);
         if ( $from_position < $to_position ) {
             $direction = -1;
@@ -371,6 +372,8 @@ sub move_to {
         $self->_shift_siblings ($direction, @between);
         $self->_ordered_internal_update({ $position_column => $new_pos_val });
 
+        $guard->commit;
+
         return 1;
     }
 }
@@ -412,8 +415,9 @@ sub move_to_group {
         return $self->move_to ($to_position);
     }
 
-    # FIXME this needs to be wrapped in a transaction
     {
+        my $guard = $self->result_source->schema->txn_scope_guard;
+
         # Move to end of current group to adjust siblings
         $self->move_last;
 
@@ -436,6 +440,8 @@ sub move_to_group {
 
         $self->_ordered_internal_update;
 
+        $guard->commit;
+
         return 1;
     }
 }
@@ -493,8 +499,9 @@ sub update {
         return $self->next::method( \%changes, @_ );
     }
 
-    # FIXME this needs to be wrapped in a transaction
     {
+        my $guard = $self->result_source->schema->txn_scope_guard;
+
         # if any of our grouping columns have been changed
         if (grep { exists $changes{$_} } ($self->_grouping_columns) ) {
 
@@ -522,7 +529,20 @@ sub update {
             $self->move_to(delete $changes{$position_column});
         }
 
-        return $self->next::method( \%changes, @_ );
+        my @res;
+        my $want = wantarray();
+        if (not defined $want) {
+            $self->next::method( \%changes, @_ );
+        }
+        elsif ($want) {
+            @res = $self->next::method( \%changes, @_ );
+        }
+        else {
+            $res[0] = $self->next::method( \%changes, @_ );
+        }
+
+        $guard->commit;
+        return $want ? @res : $res[0];
     }
 }
 
@@ -536,11 +556,25 @@ integrity of the positions.
 
 sub delete {
     my $self = shift;
-    # FIXME this needs to be wrapped in a transaction
-    {
-        $self->move_last;
-        return $self->next::method( @_ );
+
+    my $guard = $self->result_source->schema->txn_scope_guard;
+
+    $self->move_last;
+
+    my @res;
+    my $want = wantarray();
+    if (not defined $want) {
+        $self->next::method( @_ );
     }
+    elsif ($want) {
+        @res = $self->next::method( @_ );
+    }
+    else {
+        $res[0] = $self->next::method( @_ );
+    }
+
+    $guard->commit;
+    return $want ? @res : $res[0];
 }
 
 =head1 METHODS FOR EXTENDING ORDERED
@@ -818,7 +852,11 @@ could result in the position not being assigned correctly.
 
 =head1 AUTHOR
 
-Aran Deltac <bluefeet@cpan.org>
+ Original code framework
+   Aran Deltac <bluefeet@cpan.org>
+
+ Constraints support and code generalisation
+   Peter Rabbitson <ribasushi@cpan.org>
 
 =head1 LICENSE
 
diff --git a/t/87ordered.t b/t/87ordered.t
index 67033db..1d9f17a 100644
--- a/t/87ordered.t
+++ b/t/87ordered.t
@@ -42,11 +42,15 @@ foreach my $group_id (1..4) {
 my $group_3 = $employees->search({group_id=>3});
 my $to_group = 1;
 my $to_pos = undef;
-while (my $employee = $group_3->next) {
-	$employee->discard_changes;     # since we are effective shift()ing the $rs
-	$employee->move_to_group($to_group, $to_pos);
-	$to_pos++;
-	$to_group = $to_group==1 ? 2 : 1;
+# now that we have transactions we need to work around stupid sqlite
+{
+  my @empl = $group_3->all;
+  while (my $employee = shift @empl) {
+    $employee->discard_changes;     # since we are effective shift()ing the $rs while doing this
+    $employee->move_to_group($to_group, $to_pos);
+    $to_pos++;
+    $to_group = $to_group==1 ? 2 : 1;
+  }
 }
 foreach my $group_id (1..4) {
     my $group_employees = $employees->search({group_id=>$group_id});
@@ -124,12 +128,17 @@ $to_group = 1;
 my $to_group_2_base = 7;
 my $to_group_2 = 1;
 $to_pos = undef;
-while (my $employee = $group_4->next) {
-	$employee->move_to_group({group_id_2=>$to_group, group_id_3=>$to_group_2}, $to_pos);
-	$to_pos++;
+
+# now that we have transactions we need to work around stupid sqlite
+{
+  my @empl = $group_3->all;
+  while (my $employee = shift @empl) {
+    $employee->move_to_group({group_id_2=>$to_group, group_id_3=>$to_group_2}, $to_pos);
+    $to_pos++;
     $to_group = ($to_group % 3) + 1;
     $to_group_2_base++;
     $to_group_2 = (ceil($to_group_2_base/3.0) %3) +1
+  }
 }
 foreach my $group_id_2 (1..4) {
     foreach my $group_id_3 (1..4) {