From 2af46d8d16adfc196b2cbf2c3de5c5fda8da84fb Mon Sep 17 00:00:00 2001
From: Eugene Kalenkovich <rubify@softover.com>
Date: Thu, 24 May 2012 17:17:04 -0400
Subject: [PATCH] Fix for Collection#slice offset adjustment behavior not
 accounting for reversed query (for correct slice with negative offset
 chaining)

---
 .gitignore                                    |  3 +
 lib/dm-core/collection.rb                     | 20 ++++--
 spec/public/shared/collection_shared_spec.rb  | 72 +++++++++++++++++++
 spec/semipublic/shared/subject_shared_spec.rb |  8 ++-
 4 files changed, 97 insertions(+), 6 deletions(-)

diff --git a/.gitignore b/.gitignore
index 0285842f3..56786cc44 100644
--- a/.gitignore
+++ b/.gitignore
@@ -31,5 +31,8 @@ measurements
 .bundle
 Gemfile.*
 
+## RVM
+.rvmrc
+
 ## PROJECT::SPECIFIC
 spec/db/
diff --git a/lib/dm-core/collection.rb b/lib/dm-core/collection.rb
index 46d3f0ad7..bb7bc17c9 100644
--- a/lib/dm-core/collection.rb
+++ b/lib/dm-core/collection.rb
@@ -1412,12 +1412,24 @@ def sliced_query(offset, limit)
       query = self.query
 
       if offset >= 0
-        query.slice(offset, limit)
+        if query.add_reversed?
+          query.slice(query.limit - limit - offset, limit)
+        else
+          query.slice(offset, limit)
+        end
       else
-        query = query.slice((limit + offset).abs, limit).reverse!
+        if query.limit
+          if query.add_reversed?
+            query.slice(-offset - limit, limit)
+          else  
+            query.slice(query.limit + offset, limit)
+          end
+        else
+          query = query.slice((limit + offset).abs, limit).reverse!
 
-        # tell the Query to prepend each result from the adapter
-        query.update(:add_reversed => !query.add_reversed?)
+          # tell the Query to prepend each result from the adapter
+          query.update(:add_reversed => !query.add_reversed?)
+        end
       end
     end
 
diff --git a/spec/public/shared/collection_shared_spec.rb b/spec/public/shared/collection_shared_spec.rb
index 3772c4418..4410445d7 100644
--- a/spec/public/shared/collection_shared_spec.rb
+++ b/spec/public/shared/collection_shared_spec.rb
@@ -1182,6 +1182,78 @@
       end
     end
 
+    describe 'with chained negative offset after positive' do
+      before :all do
+        unless @skip
+          @return = @resources = @articles.slice!(1, 6).slice!(-4, 3)
+        end
+      end
+
+      it 'should return a Collection' do
+        @return.should be_kind_of(DataMapper::Collection)
+      end
+
+      it 'should return the expected Resources' do
+        @return.should == @copy.entries.slice!(1, 6).slice!(-4, 3)
+      end
+
+      it 'should remove the Resources from the Collection' do
+        @resources.each { |resource| @articles.should_not be_include(resource) }
+      end
+
+      it 'should scope the Collection' do
+        @resources.reload.should == @copy.entries.slice!(1, 6).slice!(-4, 3)
+      end
+    end
+
+    describe 'with chained positive offset after negative' do
+      before :all do
+        unless @skip
+          @return = @resources = @articles.slice!(-6, 5).slice!(1, 2)
+        end
+      end
+
+      it 'should return a Collection' do
+        @return.should be_kind_of(DataMapper::Collection)
+      end
+
+      it 'should return the expected Resources' do
+        @return.should == @copy.entries.slice!(-6, 5).slice!(1, 2)
+      end
+
+      it 'should remove the Resources from the Collection' do
+        @resources.each { |resource| @articles.should_not be_include(resource) }
+      end
+
+      it 'should scope the Collection' do
+        @resources.reload.should == @copy.entries.slice!(-6, 5).slice!(1, 2)
+      end
+    end
+    
+    describe 'with chained negative offset after negative' do
+      before :all do
+        unless @skip
+          @return = @resources = @articles.slice!(-7, 6).slice!(-4, 2)
+        end
+      end
+
+      it 'should return a Collection' do
+        @return.should be_kind_of(DataMapper::Collection)
+      end
+
+      it 'should return the expected Resources' do
+        @return.should == @copy.entries.slice!(-7, 6).slice!(-4, 2)
+      end
+
+      it 'should remove the Resources from the Collection' do
+        @resources.each { |resource| @articles.should_not be_include(resource) }
+      end
+
+      it 'should scope the Collection' do
+        @resources.reload.should == @copy.entries.slice!(-7, 6).slice!(-4, 2)
+      end
+    end
+
     describe 'with an offset not within the Collection' do
       before :all do
         unless @skip
diff --git a/spec/semipublic/shared/subject_shared_spec.rb b/spec/semipublic/shared/subject_shared_spec.rb
index 351ba5d22..c747b3656 100644
--- a/spec/semipublic/shared/subject_shared_spec.rb
+++ b/spec/semipublic/shared/subject_shared_spec.rb
@@ -3,13 +3,17 @@
     describe 'with a default' do
       subject { @subject_with_default.default? }
 
-      it { should be(true) }
+      it "default?" do 
+        should be(true)
+      end
     end
 
     describe 'without a default' do
       subject { @subject_without_default.default? }
 
-      it { should be(false) }
+      it "default?" do
+        should be(false)
+      end
     end
   end