diff --git a/Dockerfile b/Dockerfile index c7cbcfd5..88a94650 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM crystallang/crystal:1.0.0 +FROM crystallang/crystal:1.6.0 ARG sqlite_version=3110000 ARG sqlite_version_year=2016 diff --git a/shard.yml b/shard.yml index b7dfb8c7..619383ca 100644 --- a/shard.yml +++ b/shard.yml @@ -30,4 +30,4 @@ development_dependencies: ameba: github: crystal-ameba/ameba - version: ~> 0.13.0 + version: ~> 1.3.1 diff --git a/spec/adapter/adapters_spec.cr b/spec/adapter/adapters_spec.cr index 3b4cc5c2..41c70290 100644 --- a/spec/adapter/adapters_spec.cr +++ b/spec/adapter/adapters_spec.cr @@ -15,9 +15,21 @@ describe Granite::Connections do it "should allow connections to be be saved and looked up" do Granite::Connections.registered_connections.size.should eq 3 - Granite::Connections["mysql"].not_nil!.url.should eq ENV["MYSQL_DATABASE_URL"] - Granite::Connections["pg"].not_nil!.url.should eq ENV["PG_DATABASE_URL"] - Granite::Connections["sqlite"].not_nil!.url.should eq ENV["SQLITE_DATABASE_URL"] + if connection = Granite::Connections["mysql"] + connection.url.should eq ENV["MYSQL_DATABASE_URL"] + else + connection.should_not be_falsey + end + if connection = Granite::Connections["pg"] + connection.url.should eq ENV["PG_DATABASE_URL"] + else + connection.should_not be_falsey + end + if connection = Granite::Connections["sqlite"] + connection.url.should eq ENV["SQLITE_DATABASE_URL"] + else + connection.should_not be_falsey + end end it "should disallow multiple connections with the same name" do diff --git a/spec/granite/associations/has_many_spec.cr b/spec/granite/associations/has_many_spec.cr index 161f30af..3fb7ebe9 100644 --- a/spec/granite/associations/has_many_spec.cr +++ b/spec/granite/associations/has_many_spec.cr @@ -68,9 +68,13 @@ describe "has_many" do klass3.teacher = teacher klass3.save - klass = teacher.klasses.find_by(name: "Test class with different name").not_nil! - klass.id.should eq klass3.id - klass.name.should eq "Test class with different name" + klass = teacher.klasses.find_by(name: "Test class with different name") + if klass + klass.id.should eq klass3.id + klass.name.should eq "Test class with different name" + else + klass.should_not be_nil + end end it "#find_by!" do @@ -93,7 +97,7 @@ describe "has_many" do klass3.teacher = teacher klass3.save - klass = teacher.klasses.find_by!(name: "Test class with different name").not_nil! + klass = teacher.klasses.find_by!(name: "Test class with different name") klass.id.should eq klass3.id klass.name.should eq "Test class with different name" @@ -125,9 +129,13 @@ describe "has_many" do klass3.teacher = teacher klass3.save - klass = teacher.klasses.find(klass1.id).not_nil! - klass.id.should eq klass1.id - klass.name.should eq "Test class X" + klass = teacher.klasses.find(klass1.id) + if klass + klass.id.should eq klass1.id + klass.name.should eq "Test class X" + else + klass.should_not be_nil + end end it "#find!" do @@ -150,7 +158,7 @@ describe "has_many" do klass3.teacher = teacher klass3.save - klass = teacher.klasses.find!(klass1.id).not_nil! + klass = teacher.klasses.find!(klass1.id) klass.id.should eq klass1.id klass.name.should eq "Test class X" diff --git a/spec/granite/associations/has_many_through_spec.cr b/spec/granite/associations/has_many_through_spec.cr index 36bca32f..fda28ea0 100644 --- a/spec/granite/associations/has_many_through_spec.cr +++ b/spec/granite/associations/has_many_through_spec.cr @@ -37,9 +37,9 @@ describe "has_many, through:" do enrollment3.student = unrelated_student enrollment3.save - student.klasses.map(&.id).compact.sort.should eq [klass1.id, klass2.id].compact.sort + student.klasses.compact_map(&.id).sort!.should eq [klass1.id, klass2.id].compact.sort! - klass2.students.map(&.id).compact.sort.should eq [student.id, unrelated_student.id].compact.sort + klass2.students.compact_map(&.id).sort!.should eq [student.id, unrelated_student.id].compact.sort! end context "querying association" do @@ -91,9 +91,13 @@ describe "has_many, through:" do enrollment3.student = student enrollment3.save - klass = student.klasses.find_by(name: "Test class with different name").not_nil! - klass.id.should eq klass3.id - klass.name.should eq "Test class with different name" + klass = student.klasses.find_by(name: "Test class with different name") + if klass + klass.id.should eq klass3.id + klass.name.should eq "Test class with different name" + else + klass.should_not be_nil + end end it "#find_by!" do @@ -118,7 +122,7 @@ describe "has_many, through:" do enrollment3.student = student enrollment3.save - klass = student.klasses.find_by!(name: "Test class with different name").not_nil! + klass = student.klasses.find_by!(name: "Test class with different name") klass.id.should eq klass3.id klass.name.should eq "Test class with different name" @@ -152,9 +156,13 @@ describe "has_many, through:" do enrollment3.student = student enrollment3.save - klass = student.klasses.find(klass1.id).not_nil! - klass.id.should eq klass1.id - klass.name.should eq "Test class X" + klass = student.klasses.find(klass1.id) + if klass + klass.id.should eq klass1.id + klass.name.should eq "Test class X" + else + klass.should_not be_nil + end end it "#find!" do @@ -179,7 +187,7 @@ describe "has_many, through:" do enrollment3.student = student enrollment3.save - klass = student.klasses.find!(klass1.id).not_nil! + klass = student.klasses.find!(klass1.id) klass.id.should eq klass1.id klass.name.should eq "Test class X" diff --git a/spec/granite/querying/all_spec.cr b/spec/granite/querying/all_spec.cr index c8176708..71c58086 100644 --- a/spec/granite/querying/all_spec.cr +++ b/spec/granite/querying/all_spec.cr @@ -9,7 +9,7 @@ describe "#all" do all = Parent.all all.size.should eq model_ids.size - all.map(&.id).compact.sort.should eq model_ids.compact + all.compact_map(&.id).sort!.should eq model_ids.compact end # TODO Fails under MySQL diff --git a/spec/granite/querying/find_by_spec.cr b/spec/granite/querying/find_by_spec.cr index e4f10a16..c77e6d64 100644 --- a/spec/granite/querying/find_by_spec.cr +++ b/spec/granite/querying/find_by_spec.cr @@ -9,7 +9,11 @@ describe "#find_by, #find_by!" do model.save.should be_true found = Parent.find_by(name: name) - found.not_nil!.id.should eq model.id + if pa = found + pa.id.should eq model.id + else + pa.should_not be_nil + end found = Parent.find_by!(name: name) found.should be_a(Parent) @@ -23,7 +27,12 @@ describe "#find_by, #find_by!" do expected = Review.create(name: "review3", upvotes: 10.to_i64) - Review.find_by(name: "review3", upvotes: 10).not_nil!.id.should eq expected.id + r = Review.find_by(name: "review3", upvotes: 10) + if r + r.id.should eq expected.id + else + r.should_not be_nil + end expect_raises(Granite::Querying::NotFound, /No .*Review.* found where name = review1 and upvotes = 20/) do Review.find_by!(name: "review1", upvotes: 20) @@ -37,7 +46,11 @@ describe "#find_by, #find_by!" do model.save.should be_true found = Student.find_by(name: nil) - found.not_nil!.id.should eq model.id + if stu = found + stu.id.should eq model.id + else + stu.should_not be_nil + end found = Student.find_by!(name: nil) found.should be_a(Student) @@ -52,7 +65,11 @@ describe "#find_by, #find_by!" do model.save.should be_true found = ReservedWord.find_by(all: value) - found.not_nil!.id.should eq model.id + if rw = found + rw.id.should eq model.id + else + rw.should_not be_nil + end found = ReservedWord.find_by!(all: value) found.id.should eq model.id @@ -66,7 +83,11 @@ describe "#find_by, #find_by!" do model.save.should be_true found = Parent.find_by({"name" => name}) - found.not_nil!.id.should eq model.id + if pa = found + pa.id.should eq model.id + else + pa.should_not be_nil + end found = Parent.find_by!({"name" => name}) found.should be_a(Parent) diff --git a/spec/granite/querying/find_each_spec.cr b/spec/granite/querying/find_each_spec.cr index 5c6c5c9d..28f1e9b6 100644 --- a/spec/granite/querying/find_each_spec.cr +++ b/spec/granite/querying/find_each_spec.cr @@ -4,7 +4,7 @@ describe "#find_each" do it "finds all the records" do Parent.clear model_ids = (0...100).map do |i| - Parent.new(name: "role_#{i}").tap { |r| r.save } + Parent.new(name: "role_#{i}").tap(&.save) end.map(&.id) found_roles = [] of Int64 | Nil @@ -12,7 +12,7 @@ describe "#find_each" do found_roles << model.id end - found_roles.compact.sort.should eq model_ids.compact + found_roles.compact.sort!.should eq model_ids.compact end it "doesnt yield when no records are found" do @@ -38,7 +38,7 @@ describe "#find_each" do found_models << model.id end - found_models.compact.sort.should eq created_models.compact + found_models.compact.sort!.should eq created_models.compact end it "doesnt obliterate a parameterized query" do diff --git a/spec/granite/querying/find_in_batches.cr b/spec/granite/querying/find_in_batches.cr index ac487975..895339e5 100644 --- a/spec/granite/querying/find_in_batches.cr +++ b/spec/granite/querying/find_in_batches.cr @@ -12,7 +12,7 @@ describe "#find_in_batches" do batch.size.should eq 10 end - found_models.compact.sort.should eq model_ids.compact + found_models.compact.sort!.should eq model_ids.compact end it "doesnt yield when no records are found" do @@ -56,7 +56,7 @@ describe "#find_in_batches" do end end - found_models.compact.sort.should eq created_models.compact + found_models.compact.sort!.should eq created_models.compact end it "doesnt obliterate a parameterized query" do @@ -67,7 +67,7 @@ describe "#find_in_batches" do looking_for_ids = created_models[0...5] Parent.find_in_batches("WHERE id IN(#{looking_for_ids.join(",")})") do |batch| - batch.map(&.id).compact.should eq looking_for_ids + batch.compact_map(&.id).should eq looking_for_ids end end end diff --git a/spec/granite/querying/find_spec.cr b/spec/granite/querying/find_spec.cr index 6d94f5f3..fd70dc58 100644 --- a/spec/granite/querying/find_spec.cr +++ b/spec/granite/querying/find_spec.cr @@ -8,7 +8,7 @@ describe "#find, #find!" do found = Parent.find model.id found.should_not be_nil - found.not_nil!.id.should eq model.id + found && (found.id.should eq model.id) found = Parent.find! model.id found.id.should eq model.id diff --git a/spec/granite/querying/first_spec.cr b/spec/granite/querying/first_spec.cr index 2549e181..b9251889 100644 --- a/spec/granite/querying/first_spec.cr +++ b/spec/granite/querying/first_spec.cr @@ -14,7 +14,11 @@ describe "#first, #first!" do end found = Parent.first - found.not_nil!.id.should eq first.id + if pa = found + pa.id.should eq first.id + else + pa.should_not be_nil + end found = Parent.first! found.id.should eq first.id @@ -33,7 +37,11 @@ describe "#first, #first!" do end found = Parent.first("ORDER BY id DESC") - found.not_nil!.id.should eq second.id + if pa = found + found.id.should eq second.id + else + pa.should_not be_nil + end found = Parent.first!("ORDER BY id DESC") found.id.should eq second.id diff --git a/spec/granite/transactions/create_spec.cr b/spec/granite/transactions/create_spec.cr index 832daaca..975e1bda 100644 --- a/spec/granite/transactions/create_spec.cr +++ b/spec/granite/transactions/create_spec.cr @@ -13,20 +13,19 @@ describe "#create" do end it "doesn't have a race condition on IDs" do + n = 1000 + ids = Array(Int64).new(n) channel = Channel(Int64).new - 2.times do + n.times do spawn do parent = Parent.new(name: "Test Parent") parent.save - channel.send(parent.id.not_nil!) + (id = parent.id) && channel.send(id) end end - - id1 = channel.receive - id2 = channel.receive - - id1.should_not eq id2 + n.times { ids << channel.receive } + ids.uniq!.size.should eq n end describe "with a custom primary key" do diff --git a/spec/granite/transactions/import_spec.cr b/spec/granite/transactions/import_spec.cr index 55e44e62..81271104 100644 --- a/spec/granite/transactions/import_spec.cr +++ b/spec/granite/transactions/import_spec.cr @@ -38,7 +38,7 @@ describe "#import" do reviews = Review.all("WHERE name LIKE ?", ["ImportReview%"]) reviews.size.should eq 4 - reviews.none? { |r| r.published }.should be_true + reviews.none?(&.published).should be_true reviews.all? { |r| r.upvotes == 0 }.should be_true reviews.each { |r| r.published = true; r.upvotes = 1.to_i64 } @@ -48,7 +48,7 @@ describe "#import" do reviews = Review.all("WHERE name LIKE ?", ["ImportReview%"]) reviews.size.should eq 4 - reviews.all? { |r| r.published }.should be_true + reviews.all?(&.published).should be_true reviews.all? { |r| r.upvotes == 1 }.should be_true end end @@ -132,7 +132,7 @@ describe "#import" do schools.size.should eq 4 schools.all? { |s| s.name == "ImportExistingSchool" }.should be_true - schools.each { |s| s.name = "ImportExistingSchoolEdited" } + schools.each(&.name=("ImportExistingSchoolEdited")) School.import(schools, update_on_duplicate: true, columns: ["name"]) diff --git a/src/adapter/pg.cr b/src/adapter/pg.cr index 5d017d7d..5624d77a 100644 --- a/src/adapter/pg.cr +++ b/src/adapter/pg.cr @@ -44,7 +44,7 @@ class Granite::Adapter::Pg < Granite::Adapter::Base stmt << "INSERT INTO #{quote(table_name)} (" stmt << fields.map { |name| "#{quote(name)}" }.join(", ") stmt << ") VALUES (" - stmt << fields.map { |name| "$#{fields.index(name).not_nil! + 1}" }.join(", ") + stmt << position_str(fields.size) stmt << ")" stmt << " RETURNING #{quote(lastval)}" if lastval @@ -116,7 +116,7 @@ class Granite::Adapter::Pg < Granite::Adapter::Base def update(table_name : String, primary_name : String, fields, params) statement = String.build do |stmt| stmt << "UPDATE #{quote(table_name)} SET " - stmt << fields.map { |name| "#{quote(name)}=$#{fields.index(name).not_nil! + 1}" }.join(", ") + stmt << fields.map_with_index { |name, i| "#{quote(name)}=$#{i + 1}" }.join(", ") stmt << " WHERE #{quote(primary_name)}=$#{fields.size + 1}" end @@ -153,4 +153,15 @@ class Granite::Adapter::Pg < Granite::Adapter::Base clause end + + private def position_str(n : Int32) : String + i = 1 + String.build do |str| + while i <= n + str << "$" << i + i += 1 + str << ", " if i <= n + end + end + end end diff --git a/src/granite/columns.cr b/src/granite/columns.cr index 348a08ac..b299e171 100644 --- a/src/granite/columns.cr +++ b/src/granite/columns.cr @@ -9,7 +9,7 @@ module Granite::Columns # All fields def fields : Array(String) {% begin %} - {% columns = @type.instance_vars.select { |ivar| ivar.annotation(Granite::Column) }.map(&.name.stringify) %} + {% columns = @type.instance_vars.select(&.annotation(Granite::Column)).map(&.name.stringify) %} {{columns.empty? ? "[] of String".id : columns}} {% end %} end @@ -37,7 +37,7 @@ module Granite::Columns {% begin %} result.column_names.each do |col| case col - {% for column in @type.instance_vars.select { |ivar| ivar.annotation(Granite::Column) } %} + {% for column in @type.instance_vars.select(&.annotation(Granite::Column)) %} {% ann = column.annotation(Granite::Column) %} when {{column.name.stringify}} @{{column.id}} = {% if ann[:converter] %} @@ -109,9 +109,9 @@ module Granite::Columns end def to_h - fields = {{"Hash(String, Union(#{@type.instance_vars.select { |ivar| ivar.annotation(Granite::Column) }.map(&.type.id).splat})).new".id}} + fields = {{"Hash(String, Union(#{@type.instance_vars.select(&.annotation(Granite::Column)).map(&.type.id).splat})).new".id}} - {% for column in @type.instance_vars.select { |ivar| ivar.annotation(Granite::Column) } %} + {% for column in @type.instance_vars.select(&.annotation(Granite::Column)) %} {% if column.type.id == Time.id %} fields["{{column.name}}"] = {{column.name.id}}.try(&.in(Granite.settings.default_timezone).to_s(Granite::DATETIME_FORMAT)) {% elsif column.type.id == Slice.id %} @@ -148,7 +148,7 @@ module Granite::Columns def read_attribute(attribute_name : Symbol | String) : Type {% begin %} case attribute_name.to_s - {% for column in @type.instance_vars.select { |ivar| ivar.annotation(Granite::Column) } %} + {% for column in @type.instance_vars.select(&.annotation(Granite::Column)) %} when "{{ column.name }}" then @{{ column.name.id }} {% end %} else diff --git a/src/granite/query/executors/value.cr b/src/granite/query/executors/value.cr index 501f0524..2e03f1df 100644 --- a/src/granite/query/executors/value.cr +++ b/src/granite/query/executors/value.cr @@ -10,10 +10,12 @@ module Granite::Query::Executor # db.scalar raises when a query returns 0 results, so I'm using query_one? # https://github.com/crystal-lang/crystal-db/blob/7d30e9f50e478cb6404d16d2ce91e639b6f9c476/src/db/statement.cr#L18 - raise "No default provided" if @default.nil? - - Model.adapter.open do |db| - db.query_one?(@sql, args: @args, as: Scalar) || @default.not_nil! + if @default.nil? + raise "No default provided" + else + Model.adapter.open do |db| + db.query_one?(@sql, args: @args, as: Scalar) || @default + end end end diff --git a/src/granite/querying.cr b/src/granite/querying.cr index 113a2e37..f019d5a5 100644 --- a/src/granite/querying.cr +++ b/src/granite/querying.cr @@ -87,7 +87,7 @@ module Granite::Querying loop do results = all "#{clause} LIMIT ? OFFSET ?", params + [limit, offset] - break unless results.any? + break if results.empty? yield results offset += limit end @@ -115,7 +115,7 @@ module Granite::Querying end def exec(clause = "") - adapter.open { |db| db.exec(clause) } + adapter.open(&.exec(clause)) end def query(clause = "", params = [] of Granite::Columns::Type, &block) diff --git a/src/granite/transactions.cr b/src/granite/transactions.cr index 57945359..e882a8ea 100644 --- a/src/granite/transactions.cr +++ b/src/granite/transactions.cr @@ -24,7 +24,7 @@ module Granite::Transactions def create!(args : Granite::ModelArgs) instance = create(args) - if instance.errors.any? + if !instance.errors.empty? raise Granite::RecordNotSaved.new(self.name, instance) end @@ -78,13 +78,13 @@ module Granite::Transactions end def set_timestamps(*, to time = Time.local(Granite.settings.default_timezone), mode = :create) - {% if @type.instance_vars.select { |ivar| ivar.annotation(Granite::Column) }.map(&.name.stringify).includes? "created_at" %} + {% if @type.instance_vars.select(&.annotation(Granite::Column)).map(&.name.stringify).includes? "created_at" %} if mode == :create @created_at = time.at_beginning_of_second end {% end %} - {% if @type.instance_vars.select { |ivar| ivar.annotation(Granite::Column) }.map(&.name.stringify).includes? "updated_at" %} + {% if @type.instance_vars.select(&.annotation(Granite::Column)).map(&.name.stringify).includes? "updated_at" %} @updated_at = time.at_beginning_of_second {% end %} end