Collection#destroy! and OFFSET/LIMIT

00000 use case.rb

# http://datamapper.lighthouseapp.com/projects/20609-datamapper/tickets/746-patch-raise-argumenterror-if-limitoffset-was-set-in-query-in-collectiondestroy
# it's surprising that Collection#destroy! doesn't use LIMIT/OFFSET

require 'rubygems'
require 'dm-core'

class Pet
  include DataMapper::Resource
  property :id, Serial
end

DataMapper.setup(:default, 'sqlite3::memory:')
Pet.auto_migrate!

10.times{ Pet.create }

DataMapper.logger.set_log(STDOUT, :debug)

p Pet.all(:limit => 10, :offset => 5).size
p Pet.all(:limit => 10, :offset => 5).destroy!
p Pet.all.size

00001 output

Tue, 06 Jan 2009 15:53:01 GMT ~ debug ~ (0.000032) SELECT "id" FROM "pets" ORDER BY "id" LIMIT 10 OFFSET 5
5
Tue, 06 Jan 2009 15:53:01 GMT ~ debug ~ (0.000040) DELETE FROM "pets"
true
Tue, 06 Jan 2009 15:53:01 GMT ~ debug ~ (0.000032) SELECT "id" FROM "pets" ORDER BY "id"
0

0001-raise-ArgumentError-if-LIMIT-OFFSET-was-set-in-query.patch

From 2788fa24d19f53179f71220902475d7a137ff674 Mon Sep 17 00:00:00 2001
From: godfat <godfat@godfat.org>
Date: Tue, 6 Jan 2009 23:15:58 +0800
Subject: [PATCH 1/2] raise ArgumentError if LIMIT/OFFSET was set in query in Coll..#destroy!

  This would prevent from accidentally destroy something, e.g.,

    Pet.all(:limit => 10, :offset => 5).size     # => 10
    Pet.all(:limit => 10, :offset => 5).destroy! # => destroy all
---
 lib/dm-core/adapters/data_objects_adapter.rb |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/lib/dm-core/adapters/data_objects_adapter.rb b/lib/dm-core/adapters/data_objects_adapter.rb
index 727d615..6ba6940 100644
--- a/lib/dm-core/adapters/data_objects_adapter.rb
+++ b/lib/dm-core/adapters/data_objects_adapter.rb
@@ -268,6 +268,8 @@ module DataMapper
         end
 
         def delete_statement(query)
+          raise ArgumentError, "No LIMIT(=>#{query.limit}) and OFFSET(=>#{query.offset}) in DELETE!" if query.limit || query.offset.to_i != 0
+
           statement = "DELETE FROM #{quote_table_name(query.model.storage_name(query.repository.name))}"
           statement << " WHERE #{where_statement(query)}" if query.conditions.any?
           statement
-- 
1.6.1

0002-spec-for-raise-ArgumentError-in-Collection-destroy.patch

From 061cb8c4446c4a118b4520bd8e6172777535320d Mon Sep 17 00:00:00 2001
From: godfat <godfat@godfat.org>
Date: Tue, 6 Jan 2009 23:47:00 +0800
Subject: [PATCH 2/2] spec for raise ArgumentError in Collection#destroy! which intro from:

  * 2788fa24d19f53179f71220902475d7a137ff674
---
 spec/public/shared/collection_shared_spec.rb |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/spec/public/shared/collection_shared_spec.rb b/spec/public/shared/collection_shared_spec.rb
index 458463f..86324cb 100644
--- a/spec/public/shared/collection_shared_spec.rb
+++ b/spec/public/shared/collection_shared_spec.rb
@@ -507,6 +507,22 @@ share_examples_for 'A public Collection' do
     it 'should bypass validation' do
       pending 'TODO: not sure how to best spec this'
     end
+
+    [:limit, :offset].each{ |kind|
+      it "should raise ArgumentError when #{kind.to_s.upcase} was provided" do
+        # why there's nothing raise with a lambda?
+        # lambda {
+        #   @model.all(kind => 3).destroy!
+        # }.should raise_error(ArgumentError)
+        begin
+          @model.all(kind => 3).destroy!
+
+        rescue => e
+          e.should be_an_instance_of(ArgumentError)
+
+        end
+      end
+    }
   end
 
   it { @articles.should respond_to(:first) }
-- 
1.6.1

dm-core.patch

From 3a20269d0f4803d58c958f044ba61d3f17459199 Mon Sep 17 00:00:00 2001
From: godfat <godfat@godfat.org>
Date: Tue, 6 Jan 2009 23:49:48 +0800
Subject: [PATCH] raise ArgumentError if LIMIT/OFFSET was set in query in Coll..#destroy!

  This would prevent from accidentally destroy something, e.g.,

    Pet.all(:limit => 10, :offset => 5).size     # => 10
    Pet.all(:limit => 10, :offset => 5).destroy! # => destroy all

  also fixed mock.
---
 lib/dm-core/adapters/data_objects_adapter.rb    |    2 ++
 spec/unit/adapters/data_objects_adapter_spec.rb |    4 +++-
 2 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/lib/dm-core/adapters/data_objects_adapter.rb b/lib/dm-core/adapters/data_objects_adapter.rb
index 833fcd1..86b00fc 100644
--- a/lib/dm-core/adapters/data_objects_adapter.rb
+++ b/lib/dm-core/adapters/data_objects_adapter.rb
@@ -255,6 +255,8 @@ module DataMapper
         end
 
         def delete_statement(query)
+          raise ArgumentError, "No LIMIT(=>#{query.limit}) and OFFSET(=>#{query.offset}) in DELETE!" if query.limit || query.offset.to_i != 0
+
           statement = "DELETE FROM #{quote_table_name(query.model.storage_name(query.repository.name))}"
           statement << " WHERE #{conditions_statement(query)}" if query.conditions.any?
           statement
diff --git a/spec/unit/adapters/data_objects_adapter_spec.rb b/spec/unit/adapters/data_objects_adapter_spec.rb
index 4a15afd..f2a9403 100644
--- a/spec/unit/adapters/data_objects_adapter_spec.rb
+++ b/spec/unit/adapters/data_objects_adapter_spec.rb
@@ -412,7 +412,9 @@ describe DataMapper::Adapters::DataObjectsAdapter do
       @property    = mock('property', :kind_of? => true, :field => 'property')
       @bind_values = [ 'bind value' ]
       @conditions  = [ [ :eql, @property, @bind_values[0] ] ]
-      @query       = mock('query', :kind_of? => true, :model => @model, :links => [], :conditions => @conditions, :bind_values => @bind_values)
+      @query       = mock('query', :kind_of? => true, :model => @model, :links => [],
+                                   :conditions => @conditions, :bind_values => @bind_values,
+                                   :limit => nil, :offset => nil)
       @resource    = mock('resource', :to_query => @query)
       @statement   = 'DELETE FROM "models" WHERE ("property" = ?)'
     end
-- 
1.6.1