From dfa543ad5f682735e475bec83a829d943eb83a01 Mon Sep 17 00:00:00 2001 From: Adam-Stomski Date: Thu, 5 Apr 2018 23:11:56 +0200 Subject: [PATCH 1/2] Change .all and has_many associations to return *Collection object --- .../granite_orm/associations/has_many_spec.cr | 58 +++++++++++++++++++ src/granite_orm/association_collection.cr | 36 ++++++++++++ src/granite_orm/associations.cr | 14 +---- src/granite_orm/base.cr | 2 + src/granite_orm/collection.cr | 22 +++++++ src/granite_orm/querying.cr | 6 +- 6 files changed, 125 insertions(+), 13 deletions(-) create mode 100644 src/granite_orm/association_collection.cr create mode 100644 src/granite_orm/collection.cr diff --git a/spec/granite_orm/associations/has_many_spec.cr b/spec/granite_orm/associations/has_many_spec.cr index d25e05f0..d4894573 100644 --- a/spec/granite_orm/associations/has_many_spec.cr +++ b/spec/granite_orm/associations/has_many_spec.cr @@ -25,6 +25,34 @@ module {{adapter.capitalize.id}} teacher.klasss.size.should eq 2 end + it "can query the association" do + teacher = Teacher.new + teacher.name = "test teacher" + teacher.save + + class1 = Klass.new + class1.name = "Test class X" + class1.teacher = teacher + class1.save + + class2 = Klass.new + class2.name = "Test class X" + class2.teacher = teacher + class2.save + + class3 = Klass.new + class3.name = "Test class with different name" + class3.teacher = teacher + class3.save + + klasses = teacher.klasss.all("AND name = ? ORDER BY id DESC", ["Test class X"]) + klasses.map(&.id).should eq [class2.id, class1.id] + + klass = teacher.klasss.find_by(:name, "Test class with different name").not_nil! + klass.id.should eq class3.id + klass.name.should eq "Test class with different name" + end + describe "#has_many, through:" do it "provides a method to retrieve associated objects through another table" do student = Student.new @@ -66,6 +94,36 @@ module {{adapter.capitalize.id}} klass2.students.map(&.id).compact.sort.should eq [student.id, unrelated_student.id].compact.sort end + + it "can query the association" do + student = Student.create(name: "test student") + + klass1 = Klass.create(name: "Test class X") + klass2 = Klass.create(name: "Test class X") + klass3 = Klass.create(name: "Test class with different name") + + enrollment1 = Enrollment.new + enrollment1.student = student + enrollment1.klass = klass1 + enrollment1.save + + enrollment2 = Enrollment.new + enrollment2.student = student + enrollment2.klass = klass2 + enrollment2.save + + enrollment3 = Enrollment.new + enrollment3.klass = klass3 + enrollment3.student = student + enrollment3.save + + klasses = student.klasss.all("AND klasss.name = ? ORDER BY klasss.id DESC", ["Test class X"]) + klasses.map(&.id).should eq [klass2.id, klass1.id] + + klass = student.klasss.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" + end end end end diff --git a/src/granite_orm/association_collection.cr b/src/granite_orm/association_collection.cr new file mode 100644 index 00000000..d8a526f3 --- /dev/null +++ b/src/granite_orm/association_collection.cr @@ -0,0 +1,36 @@ +class Granite::ORM::AssociationCollection(Owner, Target) + forward_missing_to all + + def initialize(@owner : Owner, @through : Symbol? = nil) + end + + def all(clause = "", params = [] of DB::Any) + Target.all( + [query, clause].join(" "), + [owner.id] + params + ) + end + + def find_by(field : String | Symbol, value) + Target.first( + [query, "AND #{Target.table_name}.#{field} = ?"].join(" "), + [owner.id, value] + ) + end + + private getter owner + private getter through + + private def foreign_key + "#{Target.table_name}.#{Owner.table_name[0...-1]}_id" + end + + private def query + if through.nil? + query = "WHERE #{foreign_key} = ?" + else + query = "JOIN #{through} ON #{through}.#{Target.table_name[0...-1]}_id = #{Target.table_name}.id " + query = query + "WHERE #{through}.#{Owner.table_name[0...-1]}_id = ?" + end + end +end diff --git a/src/granite_orm/associations.cr b/src/granite_orm/associations.cr index b26ac8a7..a4a52c69 100644 --- a/src/granite_orm/associations.cr +++ b/src/granite_orm/associations.cr @@ -25,12 +25,7 @@ module Granite::ORM::Associations macro has_many(children_table) def {{children_table.id}} {% children_class = children_table.id[0...-1].camelcase %} - {% name_space = @type.name.gsub(/::/, "_").downcase.id %} - {% table_name = SETTINGS[:table_name] || name_space + "s" %} - return [] of {{children_class}} unless id - foreign_key = "{{children_table.id}}.{{table_name[0...-1]}}_id" - query = "WHERE #{foreign_key} = ?" - {{children_class}}.all(query, id) + Granite::ORM::AssociationCollection(self, {{children_class}}).new(self) end end @@ -38,12 +33,7 @@ module Granite::ORM::Associations macro has_many(children_table, through) def {{children_table.id}} {% children_class = children_table.id[0...-1].camelcase %} - {% name_space = @type.name.gsub(/::/, "_").downcase.id %} - {% table_name = SETTINGS[:table_name] || name_space + "s" %} - return [] of {{children_class}} unless id - query = "JOIN {{through.id}} ON {{through.id}}.{{children_table.id[0...-1]}}_id = {{children_table.id}}.id " - query = query + "WHERE {{through.id}}.{{table_name[0...-1]}}_id = ?" - {{children_class}}.all(query, id) + Granite::ORM::AssociationCollection(self, {{children_class}}).new(self, {{through}}) end end end diff --git a/src/granite_orm/base.cr b/src/granite_orm/base.cr index 3ab00c56..045979f0 100644 --- a/src/granite_orm/base.cr +++ b/src/granite_orm/base.cr @@ -1,3 +1,5 @@ +require "./collection" +require "./association_collection" require "./associations" require "./callbacks" require "./fields" diff --git a/src/granite_orm/collection.cr b/src/granite_orm/collection.cr new file mode 100644 index 00000000..eea5c20c --- /dev/null +++ b/src/granite_orm/collection.cr @@ -0,0 +1,22 @@ +class Granite::ORM::Collection(M) + forward_missing_to collection + + def initialize(@loader : -> Array(M)) + @loaded = false + @collection = [] of M + end + + def loaded? + @loaded + end + + private getter loader + + private def collection + return @collection if loaded? + + @collection = loader.call + @loaded = true + @collection + end +end diff --git a/src/granite_orm/querying.cr b/src/granite_orm/querying.cr index 4e5a9cff..cdbaed2a 100644 --- a/src/granite_orm/querying.cr +++ b/src/granite_orm/querying.cr @@ -47,7 +47,7 @@ module Granite::ORM::Querying # your Model class. This allows you to take full advantage of the database # that you are using so you are not restricted or dummied down to support a # DSL. - def all(clause = "", params = [] of DB::Any) + def raw_all(clause = "", params = [] of DB::Any) rows = [] of self @@adapter.select(@@table_name, fields, clause, params) do |results| results.each do @@ -57,6 +57,10 @@ module Granite::ORM::Querying return rows end + def all(clause = "", params = [] of DB::Any) + Collection(self).new(-> { raw_all(clause, params) }) + 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? From 8ffc7fd497e8208cdc49b45fff4ab1eb686f9b63 Mon Sep 17 00:00:00 2001 From: Adam-Stomski Date: Sun, 8 Apr 2018 01:12:23 +0200 Subject: [PATCH 2/2] Add #find #find! #find_by! methods for querying association --- .../granite_orm/associations/has_many_spec.cr | 180 ++++++++++------ .../associations/has_many_through_spec.cr | 200 ++++++++++++++++++ src/granite_orm/association_collection.cr | 21 +- src/granite_orm/querying.cr | 14 +- 4 files changed, 337 insertions(+), 78 deletions(-) create mode 100644 spec/granite_orm/associations/has_many_through_spec.cr diff --git a/spec/granite_orm/associations/has_many_spec.cr b/spec/granite_orm/associations/has_many_spec.cr index d4894573..461000b5 100644 --- a/spec/granite_orm/associations/has_many_spec.cr +++ b/spec/granite_orm/associations/has_many_spec.cr @@ -25,104 +25,146 @@ module {{adapter.capitalize.id}} teacher.klasss.size.should eq 2 end - it "can query the association" do - teacher = Teacher.new - teacher.name = "test teacher" - teacher.save + context "querying association" do + it "#all" do + teacher = Teacher.new + teacher.name = "test teacher" + teacher.save - class1 = Klass.new - class1.name = "Test class X" - class1.teacher = teacher - class1.save + klass1 = Klass.new + klass1.name = "Test class X" + klass1.teacher = teacher + klass1.save - class2 = Klass.new - class2.name = "Test class X" - class2.teacher = teacher - class2.save + klass2 = Klass.new + klass2.name = "Test class X" + klass2.teacher = teacher + klass2.save - class3 = Klass.new - class3.name = "Test class with different name" - class3.teacher = teacher - class3.save + klass3 = Klass.new + klass3.name = "Test class with different name" + klass3.teacher = teacher + klass3.save - klasses = teacher.klasss.all("AND name = ? ORDER BY id DESC", ["Test class X"]) - klasses.map(&.id).should eq [class2.id, class1.id] + klasses = teacher.klasss.all("AND klasss.name = ? ORDER BY klasss.id DESC", ["Test class X"]) + klasses.map(&.id).should eq [klass2.id, klass1.id] + end - klass = teacher.klasss.find_by(:name, "Test class with different name").not_nil! - klass.id.should eq class3.id - klass.name.should eq "Test class with different name" - end + it "#find_by" do + teacher = Teacher.new + teacher.name = "test teacher" + teacher.save - describe "#has_many, through:" do - it "provides a method to retrieve associated objects through another table" do - student = Student.new - student.name = "test student" - student.save + klass1 = Klass.new + klass1.name = "Test class X" + klass1.teacher = teacher + klass1.save + + klass2 = Klass.new + klass2.name = "Test class X" + klass2.teacher = teacher + klass2.save - unrelated_student = Student.new - unrelated_student.name = "other student" - unrelated_student.save + klass3 = Klass.new + klass3.name = "Test class with different name" + klass3.teacher = teacher + klass3.save + + klass = teacher.klasss.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" + end + + it "#find_by!" do + teacher = Teacher.new + teacher.name = "test teacher" + teacher.save klass1 = Klass.new - klass1.name = "Test class" + klass1.name = "Test class X" + klass1.teacher = teacher klass1.save klass2 = Klass.new - klass2.name = "Test class" + klass2.name = "Test class X" + klass2.teacher = teacher klass2.save klass3 = Klass.new - klass3.name = "Test class" + klass3.name = "Test class with different name" + klass3.teacher = teacher klass3.save - enrollment1 = Enrollment.new - enrollment1.student = student - enrollment1.klass = klass1 - enrollment1.save + klass = teacher.klasss.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" - enrollment2 = Enrollment.new - enrollment2.student = student - enrollment2.klass = klass2 - enrollment2.save + expect_raises( + Granite::ORM::Querying::NotFound, + "Couldn't find #{Klass.name} with name=not_found" + ) do + klass = teacher.klasss.find_by!(:name, "not_found") + end + end + + it "#find" do + teacher = Teacher.new + teacher.name = "test teacher" + teacher.save + + klass1 = Klass.new + klass1.name = "Test class X" + klass1.teacher = teacher + klass1.save - enrollment3 = Enrollment.new - enrollment3.klass = klass2 - enrollment3.student = unrelated_student - enrollment3.save + klass2 = Klass.new + klass2.name = "Test class X" + klass2.teacher = teacher + klass2.save - student.klasss.map(&.id).compact.sort.should eq [klass1.id, klass2.id].compact.sort + klass3 = Klass.new + klass3.name = "Test class with different name" + klass3.teacher = teacher + klass3.save - klass2.students.map(&.id).compact.sort.should eq [student.id, unrelated_student.id].compact.sort + klass = teacher.klasss.find(klass1.id).not_nil! + klass.id.should eq klass1.id + klass.name.should eq "Test class X" end - it "can query the association" do - student = Student.create(name: "test student") + it "#find!" do + teacher = Teacher.new + teacher.name = "test teacher" + teacher.save - klass1 = Klass.create(name: "Test class X") - klass2 = Klass.create(name: "Test class X") - klass3 = Klass.create(name: "Test class with different name") + klass1 = Klass.new + klass1.name = "Test class X" + klass1.teacher = teacher + klass1.save - enrollment1 = Enrollment.new - enrollment1.student = student - enrollment1.klass = klass1 - enrollment1.save + klass2 = Klass.new + klass2.name = "Test class X" + klass2.teacher = teacher + klass2.save - enrollment2 = Enrollment.new - enrollment2.student = student - enrollment2.klass = klass2 - enrollment2.save + klass3 = Klass.new + klass3.name = "Test class with different name" + klass3.teacher = teacher + klass3.save - enrollment3 = Enrollment.new - enrollment3.klass = klass3 - enrollment3.student = student - enrollment3.save - klasses = student.klasss.all("AND klasss.name = ? ORDER BY klasss.id DESC", ["Test class X"]) - klasses.map(&.id).should eq [klass2.id, klass1.id] + klass = teacher.klasss.find!(klass1.id).not_nil! + klass.id.should eq klass1.id + klass.name.should eq "Test class X" - klass = student.klasss.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" + id = klass3.id.as(Int64) + 42 + + expect_raises( + Granite::ORM::Querying::NotFound, + "Couldn't find #{Klass.name} with id=#{id}" + ) do + teacher.klasss.find!(id) + end end end end diff --git a/spec/granite_orm/associations/has_many_through_spec.cr b/spec/granite_orm/associations/has_many_through_spec.cr new file mode 100644 index 00000000..3df7ab20 --- /dev/null +++ b/spec/granite_orm/associations/has_many_through_spec.cr @@ -0,0 +1,200 @@ +require "../../spec_helper" + +{% for adapter in GraniteExample::ADAPTERS %} +module {{adapter.capitalize.id}} + describe "{{ adapter.id }} has_many, through:" do + it "provides a method to retrieve associated objects through another table" do + student = Student.new + student.name = "test student" + student.save + + unrelated_student = Student.new + unrelated_student.name = "other student" + unrelated_student.save + + klass1 = Klass.new + klass1.name = "Test class" + klass1.save + + klass2 = Klass.new + klass2.name = "Test class" + klass2.save + + klass3 = Klass.new + klass3.name = "Test class" + klass3.save + + enrollment1 = Enrollment.new + enrollment1.student = student + enrollment1.klass = klass1 + enrollment1.save + + enrollment2 = Enrollment.new + enrollment2.student = student + enrollment2.klass = klass2 + enrollment2.save + + enrollment3 = Enrollment.new + enrollment3.klass = klass2 + enrollment3.student = unrelated_student + enrollment3.save + + student.klasss.map(&.id).compact.sort.should eq [klass1.id, klass2.id].compact.sort + + klass2.students.map(&.id).compact.sort.should eq [student.id, unrelated_student.id].compact.sort + end + + context "querying association" do + it "#all" do + student = Student.create(name: "test student") + + klass1 = Klass.create(name: "Test class X") + klass2 = Klass.create(name: "Test class X") + klass3 = Klass.create(name: "Test class with different name") + + enrollment1 = Enrollment.new + enrollment1.student = student + enrollment1.klass = klass1 + enrollment1.save + + enrollment2 = Enrollment.new + enrollment2.student = student + enrollment2.klass = klass2 + enrollment2.save + + enrollment3 = Enrollment.new + enrollment3.klass = klass3 + enrollment3.student = student + enrollment3.save + + klasses = student.klasss.all("AND klasss.name = ? ORDER BY klasss.id DESC", ["Test class X"]) + klasses.map(&.id).should eq [klass2.id, klass1.id] + end + + it "#find_by" do + student = Student.create(name: "test student") + + klass1 = Klass.create(name: "Test class X") + klass2 = Klass.create(name: "Test class X") + klass3 = Klass.create(name: "Test class with different name") + + enrollment1 = Enrollment.new + enrollment1.student = student + enrollment1.klass = klass1 + enrollment1.save + + enrollment2 = Enrollment.new + enrollment2.student = student + enrollment2.klass = klass2 + enrollment2.save + + enrollment3 = Enrollment.new + enrollment3.klass = klass3 + enrollment3.student = student + enrollment3.save + + klass = student.klasss.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" + end + + it "#find_by!" do + student = Student.create(name: "test student") + + klass1 = Klass.create(name: "Test class X") + klass2 = Klass.create(name: "Test class X") + klass3 = Klass.create(name: "Test class with different name") + + enrollment1 = Enrollment.new + enrollment1.student = student + enrollment1.klass = klass1 + enrollment1.save + + enrollment2 = Enrollment.new + enrollment2.student = student + enrollment2.klass = klass2 + enrollment2.save + + enrollment3 = Enrollment.new + enrollment3.klass = klass3 + enrollment3.student = student + enrollment3.save + + klass = student.klasss.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" + + expect_raises( + Granite::ORM::Querying::NotFound, + "Couldn't find #{Klass.name} with name=not_found" + ) do + klass = student.klasss.find_by!(:name, "not_found") + end + end + + it "#find" do + student = Student.create(name: "test student") + + klass1 = Klass.create(name: "Test class X") + klass2 = Klass.create(name: "Test class X") + klass3 = Klass.create(name: "Test class with different name") + + enrollment1 = Enrollment.new + enrollment1.student = student + enrollment1.klass = klass1 + enrollment1.save + + enrollment2 = Enrollment.new + enrollment2.student = student + enrollment2.klass = klass2 + enrollment2.save + + enrollment3 = Enrollment.new + enrollment3.klass = klass3 + enrollment3.student = student + enrollment3.save + + klass = student.klasss.find(klass1.id).not_nil! + klass.id.should eq klass1.id + klass.name.should eq "Test class X" + end + + it "#find!" do + student = Student.create(name: "test student") + + klass1 = Klass.create(name: "Test class X") + klass2 = Klass.create(name: "Test class X") + klass3 = Klass.create(name: "Test class with different name") + + enrollment1 = Enrollment.new + enrollment1.student = student + enrollment1.klass = klass1 + enrollment1.save + + enrollment2 = Enrollment.new + enrollment2.student = student + enrollment2.klass = klass2 + enrollment2.save + + enrollment3 = Enrollment.new + enrollment3.klass = klass3 + enrollment3.student = student + enrollment3.save + + klass = student.klasss.find!(klass1.id).not_nil! + klass.id.should eq klass1.id + klass.name.should eq "Test class X" + + id = klass3.id.as(Int64) + 42 + + expect_raises( + Granite::ORM::Querying::NotFound, + "Couldn't find #{Klass.name} with id=#{id}" + ) do + student.klasss.find!(id) + end + end + end + end +end +{% end %} diff --git a/src/granite_orm/association_collection.cr b/src/granite_orm/association_collection.cr index d8a526f3..9afca73f 100644 --- a/src/granite_orm/association_collection.cr +++ b/src/granite_orm/association_collection.cr @@ -18,6 +18,21 @@ class Granite::ORM::AssociationCollection(Owner, Target) ) end + def find_by!(field : String | Symbol, value) + find_by(field, value) || + raise Granite::ORM::Querying::NotFound.new( + "Couldn't find #{Target.name} with #{field}=#{value}" + ) + end + + def find(value) + find_by(Target.primary_name, value) + end + + def find!(value) + find_by!(Target.primary_name, value) + end + private getter owner private getter through @@ -27,10 +42,10 @@ class Granite::ORM::AssociationCollection(Owner, Target) private def query if through.nil? - query = "WHERE #{foreign_key} = ?" + "WHERE #{foreign_key} = ?" else - query = "JOIN #{through} ON #{through}.#{Target.table_name[0...-1]}_id = #{Target.table_name}.id " - query = query + "WHERE #{through}.#{Owner.table_name[0...-1]}_id = ?" + "JOIN #{through} ON #{through}.#{Target.table_name[0...-1]}_id = #{Target.table_name}.id " \ + "WHERE #{through}.#{Owner.table_name[0...-1]}_id = ?" end end end diff --git a/src/granite_orm/querying.cr b/src/granite_orm/querying.cr index cdbaed2a..f4dfba25 100644 --- a/src/granite_orm/querying.cr +++ b/src/granite_orm/querying.cr @@ -41,12 +41,6 @@ module Granite::ORM::Querying @@adapter.clear @@table_name end - # All will return all rows in the database. The clause allows you to specify - # a WHERE, JOIN, GROUP BY, ORDER BY and any other SQL92 compatible query to - # your table. The results will be an array of instantiated instances of - # your Model class. This allows you to take full advantage of the database - # that you are using so you are not restricted or dummied down to support a - # DSL. def raw_all(clause = "", params = [] of DB::Any) rows = [] of self @@adapter.select(@@table_name, fields, clause, params) do |results| @@ -57,6 +51,14 @@ module Granite::ORM::Querying return rows end + # All will return all rows in the database. The clause allows you to specify + # a WHERE, JOIN, GROUP BY, ORDER BY and any other SQL92 compatible query to + # your table. The result will be a Collection(Model) object which lazy loads + # an array of instantiated instances of your Model class. + # This allows you to take full advantage of the database + # that you are using so you are not restricted or dummied down to support a + # DSL. + # Lazy load prevent running unnecessary queries from unused variables. def all(clause = "", params = [] of DB::Any) Collection(self).new(-> { raw_all(clause, params) }) end