Various fixes to Positioned. Tests no longer test AdjacencyList.
[dbsrgits/DBIx-Class.git] / lib / DBIx / Class / Positioned.pm
index 35db710..8fe2d36 100644 (file)
@@ -56,6 +56,12 @@ Thats it, now you can change the position of your objects.
 This module provides a simple interface for modifying the position 
 of DBIx::Class objects.
 
+=head1 AUTO UPDATE
+
+All of the move_* methods automatically update the rows involved in 
+the query.  This is not configurable and is due to the fact that if you 
+move a record it always causes other records in the list to be updated.
+
 =head1 METHODS
 
 =head2 position_column
@@ -69,6 +75,18 @@ positional value of each record.  Default to "position".
 
 __PACKAGE__->mk_classdata( 'position_column' => 'position' );
 
+=head2 collection_column
+
+  __PACKAGE__->collection_column('thing_id');
+
+This method specified a column to limit all queries in 
+this module by.  This effectively allows you to have multiple 
+positioned lists within the same table.
+
+=cut
+
+__PACKAGE__->mk_classdata( 'collection_column' );
+
 =head2 siblings
 
   my $rs = $employee->siblings();
@@ -83,11 +101,14 @@ sub siblings {
     my( $self ) = @_;
     my $position_column = $self->position_column;
     my $rs = $self->search(
-        { $position_column => { '!=' => $self->get_column($position_column) } },
+        {
+            $position_column => { '!=' => $self->get_column($position_column) },
+            $self->_collection_clause(),
+        },
         { order_by => $self->position_column },
     );
-    if (wantarray()) { return $rs->all(); }
-    else { return $rs; }
+    return $rs->all() if (wantarray());
+    return $rs;
 }
 
 =head2 first_sibling
@@ -101,7 +122,7 @@ Returns the first sibling object.
 sub first_sibling {
     my( $self ) = @_;
     return ($self->search(
-        {},
+        { $self->_collection_clause() },
         { rows=>1, order_by => $self->position_column },
     )->all())[0];
 }
@@ -117,7 +138,7 @@ Return the last sibling.
 sub last_sibling {
     my( $self ) = @_;
     return ($self->search(
-        {},
+        { $self->_collection_clause() },
         { rows=>1, order_by => $self->position_column.' DESC' },
     )->all())[0];
 }
@@ -135,7 +156,10 @@ sub previous_sibling {
     my( $self ) = @_;
     my $position_column = $self->position_column;
     return ($self->search(
-        { $position_column => { '<' => $self->get_column($position_column) } },
+        {
+            $position_column => { '<' => $self->get_column($position_column) },
+            $self->_collection_clause(),
+        },
         { rows=>1, order_by => $position_column.' DESC' },
     )->all())[0];
 }
@@ -152,8 +176,11 @@ is returned if the current object is the last one.
 sub next_sibling {
     my( $self ) = @_;
     my $position_column = $self->position_column;
-    return ($self->search(
-        { $position_column => { '>' => $self->get_column($position_column) } },
+    return ($self->result_source->resultset->search(
+        {
+            $position_column => { '>' => $self->get_column($position_column) },
+            $self->_collection_clause(),
+        },
         { rows=>1, order_by => $position_column },
     )->all())[0];
 }
@@ -170,15 +197,8 @@ the first one.
 
 sub move_previous {
     my( $self ) = @_;
-    my $previous = $self->previous_sibling();
-    return undef if (!$previous);
-    my $position_column = $self->position_column;
-    my $self_position = $self->get_column( $position_column );
-    $self->set_column( $position_column, $previous->get_column($position_column) );
-    $previous->set_column( $position_column, $self_position );
-    $self->update();
-    $previous->update();
-    return 1;
+    my $position = $self->get_column( $self->position_column() );
+    return $self->move_to( $position - 1 );
 }
 
 =head2 move_next
@@ -192,15 +212,10 @@ success, and 0 is returned if the object is already the last in the list.
 
 sub move_next {
     my( $self ) = @_;
-    my $next = $self->next_sibling();
-    return undef if (!$next);
-    my $position_column = $self->position_column;
-    my $self_position = $self->get_column( $position_column );
-    $self->set_column( $position_column, $next->get_column($position_column) );
-    $next->set_column( $position_column, $self_position );
-    $self->update();
-    $next->update();
-    return 1;
+    my $position = $self->get_column( $self->position_column() );
+    my $count = $self->result_source->resultset->search({$self->_collection_clause()})->count();
+    return 0 if ($position==$count);
+    return $self->move_to( $position + 1 );
 }
 
 =head2 move_first
@@ -228,7 +243,7 @@ success, and 0 is returned if the object is already the last one.
 
 sub move_last {
     my( $self ) = @_;
-    my $count = $self->search()->count();
+    my $count = $self->result_source->resultset->search({$self->_collection_clause()})->count();
     return $self->move_to( $count );
 }
 
@@ -246,12 +261,14 @@ sub move_to {
     my( $self, $to_position ) = @_;
     my $position_column = $self->position_column;
     my $from_position = $self->get_column( $position_column );
-    return undef if ( $from_position==$to_position );
-    my $rs = $self->search({
+    return 0 if ( $to_position < 1 );
+    return 0 if ( $from_position==$to_position );
+    my $rs = $self->result_source->resultset->search({
         -and => [
             $position_column => { ($from_position>$to_position?'<':'>') => $from_position },
             $position_column => { ($from_position>$to_position?'>=':'<=') => $to_position },
-        ]
+        ],
+        $self->_collection_clause(),
     });
     my $op = ($from_position>$to_position) ? '+' : '-';
     $rs->update({
@@ -273,7 +290,7 @@ the table +1, thus positioning the new record at the last position.
 sub insert {
     my $self = shift;
     my $position_column = $self->position_column;
-    $self->set_column( $position_column => $self->count()+1 ) 
+    $self->set_column( $position_column => $self->result_source->resultset->search( {$self->_collection_clause()} )->count()+1 ) 
         if (!$self->get_column($position_column));
     $self->next::method( @_ );
 }
@@ -292,16 +309,34 @@ sub delete {
     $self->next::method( @_ );
 }
 
-1;
-__END__
+=head1 PRIVATE METHODS
+
+These methods are used internally.  You should never have the 
+need to use them.
+
+=head2 _collection_clause
 
-=head1 TODO
+This method returns a name=>value pare for limiting a search 
+by the collection column.  If the collection column is not 
+defined then this will return an empty list.
 
-Support foreign keys that cause rows to be members of mini 
-positionable sets.
+=cut
+
+sub _collection_clause {
+    my $self = shift;
+    if ($self->collection_column()) {
+        return ( $self->collection_column() => $self->get_column($self->collection_column()) );
+    }
+    return ();
+}
+
+1;
+__END__
 
 =head1 BUGS
 
+=head2 Race Condition on Insert
+
 If a position is not specified for an insert than a position 
 will be chosen based on COUNT(*)+1.  But, it first selects the 
 count then inserts the record.  The space of time between select 
@@ -309,6 +344,20 @@ and insert introduces a race condition.  To fix this we need the
 ability to lock tables in DBIC.  I've added an entry in the TODO 
 about this.
 
+=head2 Multiple Moves
+
+Be careful when issueing move_* methods to multiple objects.  If 
+you've pre-loaded the objects then when you move one of the objects 
+the position of the other object will not reflect their new value 
+until you reload them from the database.
+
+The are times when you will want to move objects as groups, such 
+as changeing the parent of several objects at once - this directly 
+conflicts with this problem.  One solution is for us to write a 
+ResultSet class that supports a parent() method, for example.  Another 
+solution is to somehow automagically modify the objects that exist 
+in the current object's result set to have the new position value.
+
 =head1 AUTHOR
 
 Aran Deltac <bluefeet@cpan.org>