Revert "find,find_by,first now ensures T, call with trailing …

…`?` for `T?` (amberframework#146)" This reverts commit 719d2e5.
maiha · Mar 31, 2018 · 5f34347 · 5f34347
1 parent 00ae2eb
commit 5f34347
Show file tree

Hide file tree

Showing 10 changed files with 40 additions and 100 deletions.
diff --git a/README.md b/README.md
@@ -138,34 +138,28 @@ end
 #### Find First
 
 ```crystal
-post = Post.first?
+post = Post.first
 if post
   puts post.name
 end
-
-post = Post.first # raises when no records exist
 ```
 
 #### Find
 
 ```crystal
-post = Post.find? 1
+post = Post.find 1
 if post
   puts post.name
 end
-
-post = Post.find 1 # raises when no records found
 ```
 
 #### Find By
 
 ```crystal
-post = Post.find_by? :slug, "example_slug"
+post = Post.find_by :slug, "example_slug"
 if post
   puts post.name
 end
-
-post = Post.find_by :slug, "foo" # raises when no records found
 ```
 
 #### Insert

diff --git a/spec/granite_orm/callbacks/callbacks_spec.cr b/spec/granite_orm/callbacks/callbacks_spec.cr
@@ -20,7 +20,7 @@ module {{adapter.capitalize.id}}
     describe "#save" do
       it "runs before_save, before_update, after_update, after_save" do
         Callback.new(name: "foo").save
-        callback = Callback.first
+        callback = Callback.first.not_nil!
         callback.save
 
         callback.history.to_s.strip.should eq <<-EOF
@@ -35,7 +35,7 @@ module {{adapter.capitalize.id}}
     describe "#destroy" do
       it "runs before_destroy, after_destroy" do
         Callback.new(name: "foo").save
-        callback = Callback.first
+        callback = Callback.first.not_nil!
         callback.destroy
 
         callback.history.to_s.strip.should eq <<-EOF

diff --git a/spec/granite_orm/fields/timestamps_spec.cr b/spec/granite_orm/fields/timestamps_spec.cr
@@ -17,7 +17,7 @@ module {{adapter.capitalize.id}}
   describe "{{ adapter.id }} timestamps" do
     it "consistently uses UTC for created_at" do
       parent = Parent.new(name: "parent").tap(&.save)
-      found_parent = Parent.find(parent.id)
+      found_parent = Parent.find(parent.id).not_nil!
 
       original_timestamp = parent.created_at.not_nil!
       read_timestamp = found_parent.created_at.not_nil!
@@ -28,7 +28,7 @@ module {{adapter.capitalize.id}}
 
     it "consistently uses UTC for updated_at" do
       parent = Parent.new(name: "parent").tap(&.save)
-      found_parent = Parent.find(parent.id)
+      found_parent = Parent.find(parent.id).not_nil!
 
       original_timestamp = parent.updated_at.not_nil!
       read_timestamp = found_parent.updated_at.not_nil!
@@ -39,7 +39,7 @@ module {{adapter.capitalize.id}}
 
     it "truncates the subsecond parts of created_at" do
       parent = Parent.new(name: "parent").tap(&.save)
-      found_parent = Parent.find(parent.id)
+      found_parent = Parent.find(parent.id).not_nil!
 
       original_timestamp = parent.created_at.not_nil!
       read_timestamp = found_parent.created_at.not_nil!
@@ -49,7 +49,7 @@ module {{adapter.capitalize.id}}
 
     it "truncates the subsecond parts of updated_at" do
       parent = Parent.new(name: "parent").tap(&.save)
-      found_parent = Parent.find(parent.id)
+      found_parent = Parent.find(parent.id).not_nil!
 
       original_timestamp = parent.updated_at.not_nil!
       read_timestamp = found_parent.updated_at.not_nil!

diff --git a/spec/granite_orm/querying/find_by_spec.cr b/spec/granite_orm/querying/find_by_spec.cr
@@ -2,7 +2,7 @@ require "../../spec_helper"
 
 {% for adapter in GraniteExample::ADAPTERS %}
 module {{adapter.capitalize.id}}
-  describe "{{ adapter.id }} #find_by?, #find_by" do
+  describe "{{ adapter.id }} #find_by" do
     it "finds an object with a string field" do
       Parent.clear
       name = "robinson"
@@ -11,11 +11,8 @@ module {{adapter.capitalize.id}}
       model.name = name
       model.save
 
-      found = Parent.find_by?("name", name)
-      found.not_nil!.id.should eq model.id
-
       found = Parent.find_by("name", name)
-      found.should be_a(Parent)
+      found.not_nil!.id.should eq model.id
     end
 
     it "finds an object with a symbol field" do
@@ -26,11 +23,8 @@ module {{adapter.capitalize.id}}
       model.name = name
       model.save
 
-      found = Parent.find_by?(:name, name)
-      found.not_nil!.id.should eq model.id
-
       found = Parent.find_by(:name, name)
-      found.id.should eq model.id
+      found.not_nil!.id.should eq model.id
     end
 
     it "also works with reserved words" do
@@ -41,21 +35,11 @@ module {{adapter.capitalize.id}}
       model.all = value
       model.save
 
-      found = ReservedWord.find_by?("all", value)
+      found = ReservedWord.find_by("all", value)
       found.not_nil!.id.should eq model.id
 
       found = ReservedWord.find_by(:all, value)
-      found.id.should eq model.id
-    end
-
-    it "returns nil or raises if no result" do
-      Parent.clear
-      found = Parent.find_by?("name", "xxx")
-      found.should be_nil
-
-      expect_raises(Granite::ORM::Querying::NotFound, /Couldn't find .*Parent.* with name=xxx/) do
-        Parent.find_by("name", "xxx")
-      end
+      found.not_nil!.id.should eq model.id
     end
   end
 end

diff --git a/spec/granite_orm/querying/find_spec.cr b/spec/granite_orm/querying/find_spec.cr
@@ -2,18 +2,15 @@ require "../../spec_helper"
 
 {% for adapter in GraniteExample::ADAPTERS %}
 module {{adapter.capitalize.id}}
-  describe "{{ adapter.id }} #find?, #find" do
+  describe "{{ adapter.id }} #find" do
     it "finds an object by id" do
       model = Parent.new
       model.name = "Test Comment"
       model.save
 
-      found = Parent.find? model.id
+      found = Parent.find model.id
       found.should_not be_nil
       found.not_nil!.id.should eq model.id
-
-      found = Parent.find model.id
-      found.id.should eq model.id
     end
 
     it "updates states of new_record and persisted" do
@@ -22,7 +19,7 @@ module {{adapter.capitalize.id}}
       model.save
       model_id = model.id
 
-      model = Parent.find(model_id)
+      model = Parent.find(model_id).not_nil!
       model.new_record?.should be_false
       model.persisted?.should be_true
     end
@@ -34,11 +31,8 @@ module {{adapter.capitalize.id}}
         school.save
         primary_key = school.custom_id
 
-        found_school = School.find? primary_key
-        found_school.should_not be_nil
-
         found_school = School.find primary_key
-        found_school.should be_a(School)
+        found_school.should_not be_nil
       end
     end
 
@@ -49,20 +43,8 @@ module {{adapter.capitalize.id}}
         county.save
         primary_key = county.id
 
-        found_county = Nation::County.find? primary_key
-        found_county.should_not be_nil
-
         found_county = Nation::County.find primary_key
-        found_county.should be_a(Nation::County)
-      end
-    end
-
-    it "returns nil or raises if no result" do
-      found = Parent.find? 0
-      found.should be_nil
-
-      expect_raises(Granite::ORM::Querying::NotFound, /Couldn't find .*Parent.* with id=0/) do
-        Parent.find 0
+        found_county.should_not be_nil
       end
     end
   end

diff --git a/spec/granite_orm/querying/first_spec.cr b/spec/granite_orm/querying/first_spec.cr
@@ -2,7 +2,7 @@ require "../../spec_helper"
 
 {% for adapter in GraniteExample::ADAPTERS %}
 module {{adapter.capitalize.id}}
-  describe "{{ adapter.id }} #first?, #first" do
+  describe "{{ adapter.id }} #first" do
     it "finds the first object" do
       Parent.clear
       first = Parent.new.tap do |model|
@@ -15,11 +15,8 @@ module {{adapter.capitalize.id}}
         model.save
       end
 
-      found = Parent.first?
-      found.not_nil!.id.should eq first.id
-
       found = Parent.first
-      found.id.should eq first.id
+      found.not_nil!.id.should eq first.id
     end
 
     it "supports a SQL clause" do
@@ -34,26 +31,19 @@ module {{adapter.capitalize.id}}
         model.save
       end
 
-      found = Parent.first?("ORDER BY id DESC")
-      found.not_nil!.id.should eq second.id
-
       found = Parent.first("ORDER BY id DESC")
-      found.id.should eq second.id
+      found.not_nil!.id.should eq second.id
     end
 
-    it "returns nil or raises if no result" do
+    it "returns nil if no result" do
       Parent.clear
       first = Parent.new.tap do |model|
         model.name = "Test 1"
         model.save
       end
 
-      found = Parent.first?("WHERE name = 'Test 2'")
+      found = Parent.first("WHERE name = 'Test 2'")
       found.should be nil
-
-      expect_raises(Granite::ORM::Querying::NotFound, /Couldn't find .*Parent.* with first\(WHERE name = 'Test 2'\)/) do
-        Parent.first("WHERE name = 'Test 2'")
-      end
     end
   end
 end

diff --git a/spec/granite_orm/transactions/destroy_spec.cr b/spec/granite_orm/transactions/destroy_spec.cr
@@ -10,7 +10,7 @@ module {{adapter.capitalize.id}}
 
       id = parent.id
       parent.destroy
-      found = Parent.find? id
+      found = Parent.find id
       found.should be_nil
     end
 
@@ -37,7 +37,7 @@ module {{adapter.capitalize.id}}
         primary_key = school.custom_id
         school.destroy
 
-        found_school = School.find? primary_key
+        found_school = School.find primary_key
         found_school.should be_nil
       end
     end
@@ -50,7 +50,7 @@ module {{adapter.capitalize.id}}
         primary_key = county.id
         county.destroy
 
-        found_county = Nation::County.find? primary_key
+        found_county = Nation::County.find primary_key
         found_county.should be_nil
       end
     end

diff --git a/spec/granite_orm/transactions/save_natural_key_spec.cr b/spec/granite_orm/transactions/save_natural_key_spec.cr
@@ -43,7 +43,7 @@ module {{adapter.capitalize.id}}
       Kvs.count.should eq(1)
 
       ## Read
-      port = Kvs.find("mysql_port")
+      port = Kvs.find("mysql_port").not_nil!
       port.v.should eq("3306")
       port.new_record?.should be_false
 

diff --git a/spec/granite_orm/transactions/save_spec.cr b/spec/granite_orm/transactions/save_spec.cr
@@ -29,7 +29,7 @@ module {{adapter.capitalize.id}}
       parents.size.should eq 1
 
       found = Parent.first
-      found.name.should eq parent.name
+      found.not_nil!.name.should eq parent.name
     end
 
     it "does not update an invalid object" do
@@ -39,7 +39,7 @@ module {{adapter.capitalize.id}}
       parent.name = ""
       parent.save
       parent = Parent.find parent.id
-      parent.name.should eq "Test Parent"
+      parent.not_nil!.name.should eq "Test Parent"
     end
 
     it "does not update when the conflicted primary key is given to the new record" do
@@ -75,8 +75,8 @@ module {{adapter.capitalize.id}}
         school.save
 
         found_school = School.find primary_key
-        found_school.custom_id.should eq primary_key
-        found_school.name.should eq new_name
+        found_school.not_nil!.custom_id.should eq primary_key
+        found_school.not_nil!.name.should eq new_name
       end
 
       it "updates states of new_record and persisted" do
@@ -113,7 +113,7 @@ module {{adapter.capitalize.id}}
         county.save
 
         found_county = Nation::County.find primary_key
-        found_county.name.should eq new_name
+        found_county.not_nil!.name.should eq new_name
       end
     end
 

diff --git a/src/granite_orm/querying.cr b/src/granite_orm/querying.cr
@@ -1,7 +1,4 @@
 module Granite::ORM::Querying
-  class NotFound < Exception
-  end  
-
   macro extended
     macro __process_querying
       \{% primary_name = PRIMARY[:name] %}
@@ -58,38 +55,31 @@ module Granite::ORM::Querying
   end
 
   # First adds a `LIMIT 1` clause to the query and returns the first result
-  def first?(clause = "", params = [] of DB::Any)
-    all([clause.strip, "LIMIT 1"].join(" "), params).first?
-  end
-
   def first(clause = "", params = [] of DB::Any)
-    first?(clause, params) || raise NotFound.new("Couldn't find " + {{@type.name.stringify}} + " with first(#{clause})")
+    all([clause.strip, "LIMIT 1"].join(" "), params).first?
   end
 
   # find returns the row with the primary key specified.
   # it checks by primary by default, but one can pass
   # another field for comparison
-  def find?(value)
-    return find_by?(@@primary_name, value)
-  end
-
   def find(value)
     return find_by(@@primary_name, value)
   end
 
+  # find_by using symbol for field name.
+  def find_by(field : Symbol, value)
+    find_by(field.to_s, value)
+  end
+
   # find_by returns the first row found where the field maches the value
-  def find_by?(field : String | Symbol, value)
+  def find_by(field : String, value)
     row = nil
     @@adapter.select_one(@@table_name, fields, field.to_s, value) do |result|
       row = from_sql(result) if result
     end
     return row
   end
 
-  def find_by(field : String | Symbol, value)
-    find_by?(field, value) || raise NotFound.new("Couldn't find " + {{@type.name.stringify}} + " with #{field}=#{value}")
-  end
-
   def find_each(clause = "", params = [] of DB::Any, batch_size limit = 100, offset = 0)
     find_in_batches(clause, params, batch_size: limit, offset: offset) do |batch|
       batch.each do |record|