Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions spec/repo_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,7 @@ describe Crecto do
user = Repo.get(User, id)
user.is_a?(User).should eq(true)
user.not_nil!.id.should eq(id)
user.not_nil!.some_date.as(Time).to_local.epoch_ms.should be_close(now.epoch_ms, 2000)
user.not_nil!.some_date.as(Time).to_local.to_unix_ms.should be_close(now.to_unix_ms, 2000)
end

it "should return nil if not in db" do
Expand Down Expand Up @@ -782,7 +782,7 @@ describe Crecto do
u.created_at.should eq(created_at)
changeset.instance.name.should eq("new name")
changeset.valid?.should eq(true)
changeset.instance.updated_at.as(Time).to_local.epoch_ms.should be_close(Time.now.epoch_ms, 2000)
changeset.instance.updated_at.as(Time).to_local.to_unix_ms.should be_close(Time.now.to_unix_ms, 2000)
end

it "should return a changeset and set the changeset action" do
Expand Down Expand Up @@ -822,7 +822,7 @@ describe Crecto do
u.created_at.should eq(created_at)
changeset.instance.name.should eq("new name")
changeset.valid?.should eq(true)
changeset.instance.updated_at.as(Time).to_local.epoch_ms.should be_close(Time.now.epoch_ms, 2000)
changeset.instance.updated_at.as(Time).to_local.to_unix_ms.should be_close(Time.now.to_unix_ms, 2000)
end

it "should raise if changeset is invalid (name is nil)" do
Expand Down Expand Up @@ -948,7 +948,6 @@ describe Crecto do
end
end


describe "#update_all" do
it "should update multiple records" do
user = User.new
Expand Down Expand Up @@ -1087,7 +1086,7 @@ describe Crecto do
user.user_projects.size.should eq 1
user.projects.size.should eq 1
end

it "should preload the has_many through association with a query" do
user = User.new
user.name = "tester"
Expand Down
6 changes: 3 additions & 3 deletions spec/schema_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ describe Crecto do
end
it "allows the primary key to be a string" do
user = UserUUIDCustom.new
user.name = "whatever"
user.name = "whatever"
# Need to set this because of MySQL and SQLite
# MySQL actually inserts the uuid because of the trigger,
# but the `instance` method below seems to return the object before the trigger is fired.
Expand Down Expand Up @@ -255,15 +255,15 @@ describe Crecto do
it "should set the updated at value to now" do
u = User.new
u.updated_at_to_now
u.updated_at.as(Time).epoch_ms.should be_close(Time.now.epoch_ms, 100)
u.updated_at.as(Time).to_unix_ms.should be_close(Time.now.to_unix_ms, 100)
end
end

describe "#created_at_to_now" do
it "should set the created at value to now" do
u = UserDifferentDefaults.new
u.created_at_to_now
u.xyz.as(Time).epoch_ms.should be_close(Time.now.epoch_ms, 2000)
u.xyz.as(Time).to_unix_ms.should be_close(Time.now.to_unix_ms, 2000)
end
end

Expand Down
14 changes: 7 additions & 7 deletions src/crecto/changeset/changeset.cr
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,8 @@ module Crecto
return unless REQUIRED_FORMATS.has_key?(@class_key)
REQUIRED_FORMATS[@class_key].each do |format|
next unless @instance_hash[format[:field]]?
raise Crecto::InvalidType.new("Format validator can only validate strings") unless @instance_hash.fetch(format[:field]).is_a?(String)
val = @instance_hash.fetch(format[:field]).as(String)
raise Crecto::InvalidType.new("Format validator can only validate strings") unless @instance_hash.fetch(format[:field], nil).is_a?(String)
val = @instance_hash.fetch(format[:field], nil).as(String)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think all the instances of fetch here can be replaced with the regular, non-nilable [], since we're already guarding that the hash has the key on the previous line.

Alternatively, the whole section could be re-written as

if field_value = @instance_hash[format[:field]]?
  raise Crecto::InvalidType.new("Format validator can only validate strings") unless field_value.is_a?(String)
  add_error(format[:field].to_s, "is invalid") if format[:pattern].match(field_value).nil?
end

Crystal's automatic type reduction should ensure that the typing is correct at each point without the need for an .as(String) cast.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think all the instances of fetch here can be replaced with the regular, non-nilable []

Yes, but I think fetch still looks better IMO.

the whole section could be re-written as

I will leave that exercise to someone else.

add_error(format[:field].to_s, "is invalid") if format[:pattern].match(val).nil?
end
end
Expand All @@ -122,7 +122,7 @@ module Crecto
return unless REQUIRED_ARRAY_INCLUSIONS.has_key?(@class_key)
REQUIRED_ARRAY_INCLUSIONS[@class_key].each do |inclusion|
next unless @instance_hash[inclusion[:field]]?
val = @instance_hash.fetch(inclusion[:field])
val = @instance_hash.fetch(inclusion[:field], nil)
add_error(inclusion[:field].to_s, "is invalid") unless inclusion[:in].includes?(val)
end
end
Expand All @@ -131,7 +131,7 @@ module Crecto
return unless REQUIRED_RANGE_INCLUSIONS.has_key?(@class_key)
REQUIRED_RANGE_INCLUSIONS[@class_key].each do |inclusion|
next unless @instance_hash[inclusion[:field]]?
val = @instance_hash.fetch(inclusion[:field])
val = @instance_hash.fetch(inclusion[:field], nil)
if inclusion[:in].is_a?(Range(Float64, Float64)) && val.is_a?(Float64)
add_error(inclusion[:field].to_s, "is invalid") unless inclusion[:in].as(Range(Float64, Float64)).includes?(val.as(Float64))
elsif inclusion[:in].is_a?(Range(Int32, Int32)) && val.is_a?(Int32)
Expand All @@ -148,7 +148,7 @@ module Crecto
return unless REQUIRED_ARRAY_EXCLUSIONS.has_key?(@class_key)
REQUIRED_ARRAY_EXCLUSIONS[@class_key].each do |exclusion|
next unless @instance_hash[exclusion[:field]]?
val = @instance_hash.fetch(exclusion[:field])
val = @instance_hash.fetch(exclusion[:field], nil)
add_error(exclusion[:field].to_s, "is invalid") if exclusion[:in].includes?(val)
end
end
Expand All @@ -157,7 +157,7 @@ module Crecto
return unless REQUIRED_RANGE_EXCLUSIONS.has_key?(@class_key)
REQUIRED_RANGE_EXCLUSIONS[@class_key].each do |exclusion|
next unless @instance_hash[exclusion[:field]]?
val = @instance_hash.fetch(exclusion[:field])
val = @instance_hash.fetch(exclusion[:field], nil)
if exclusion[:in].is_a?(Range(Float64, Float64)) && val.is_a?(Float64)
add_error(exclusion[:field].to_s, "is invalid") if exclusion[:in].as(Range(Float64, Float64)).includes?(val.as(Float64))
elsif exclusion[:in].is_a?(Range(Int32, Int32)) && val.is_a?(Int32)
Expand All @@ -174,7 +174,7 @@ module Crecto
return unless REQUIRED_LENGTHS.has_key?(@class_key)
REQUIRED_LENGTHS[@class_key].each do |length|
next unless @instance_hash[length[:field]]?
val = @instance_hash.fetch(length[:field]).as(String)
val = @instance_hash.fetch(length[:field], nil).as(String)
add_error(length[:field].to_s, "is invalid") if !length[:is].nil? && val.size != length[:is].as(Int32)
add_error(length[:field].to_s, "is invalid") if !length[:min].nil? && val.size < length[:min].as(Int32)
add_error(length[:field].to_s, "is invalid") if !length[:max].nil? && val.size > length[:max].as(Int32)
Expand Down