godfat (owner)

Revisions

gist: 43742 Download_button fork
public
Description:
Collection#destroy! and OFFSET/LIMIT
Public Clone URL: git://gist.github.com/43742.git
Embed All Files: show embed
00000 use case.rb #
raw 
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22

# 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 #
raw 
1
2
3
4
5
6
7

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 #
raw 
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30

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 #
raw 
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41

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
 
 
sam/dm-core.patch #
raw 
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48

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