From 34c551711425e935bcfd5a48f69c8bc883248998 Mon Sep 17 00:00:00 2001 From: Morgan Aubert Date: Sun, 21 Jul 2024 21:51:40 -0400 Subject: [PATCH] Ensure custom template objects always raise UnknownVariable exceptions when attributes cannot be resolved --- .../can_define_template_attributes_spec.cr | 8 +++- .../ext/marten/db/fields/file/file_spec.cr | 9 ++++ .../template/ext/marten/db/query/page_spec.cr | 9 ++++ .../ext/marten/db/query/paginator_spec.cr | 8 ++++ .../template/ext/marten/db/query/set_spec.cr | 6 +++ .../ext/marten/http/flash_store_spec.cr | 6 ++- .../ext/marten/schema/bound_field_spec.cr | 6 ++- .../marten/template/ext/marten/schema_spec.cr | 6 ++- spec/marten/template/object/auto_spec.cr | 33 ++++++++++---- .../can_define_template_attributes.cr | 2 + .../template/ext/marten/db/field/file/file.cr | 14 +----- .../template/ext/marten/db/query/page.cr | 32 +------------- .../template/ext/marten/db/query/paginator.cr | 9 +--- .../template/ext/marten/db/query/set.cr | 40 +---------------- .../template/ext/marten/http/flash_store.cr | 4 +- src/marten/template/ext/marten/schema.cr | 4 +- .../template/ext/marten/schema/bound_field.cr | 2 + src/marten/template/object/auto.cr | 44 ++++++++++++------- 18 files changed, 119 insertions(+), 123 deletions(-) diff --git a/spec/marten/template/concerns/can_define_template_attributes_spec.cr b/spec/marten/template/concerns/can_define_template_attributes_spec.cr index 6c2887540..78e59eba4 100644 --- a/spec/marten/template/concerns/can_define_template_attributes_spec.cr +++ b/spec/marten/template/concerns/can_define_template_attributes_spec.cr @@ -6,7 +6,13 @@ describe Marten::Template::CanDefineTemplateAttributes do test = Marten::Template::CanDefineTemplateAttributesSpec::Test.new test.resolve_template_attribute("foo").should eq "foo" test.resolve_template_attribute("bar").should eq "bar" - test.resolve_template_attribute("xyz").should be_nil + end + + it "raises a Marten::Template::Errors::UnknownVariable exception when the attribute is not defined" do + test = Marten::Template::CanDefineTemplateAttributesSpec::Test.new + expect_raises(Marten::Template::Errors::UnknownVariable) do + test.resolve_template_attribute("xyz") + end end end end diff --git a/spec/marten/template/ext/marten/db/fields/file/file_spec.cr b/spec/marten/template/ext/marten/db/fields/file/file_spec.cr index 1b4dbfafa..998704322 100644 --- a/spec/marten/template/ext/marten/db/fields/file/file_spec.cr +++ b/spec/marten/template/ext/marten/db/fields/file/file_spec.cr @@ -39,5 +39,14 @@ describe Marten::DB::Field::File::File do file.resolve_template_attribute("url").should eq field.storage.url("css/app.css") end + + it "raises as expected if the specified attribute is not supported" do + field = Marten::DB::Field::File.new("my_field") + file = Marten::DB::Field::File::File.new(field, "path/to/file.txt") + + expect_raises(Marten::Template::Errors::UnknownVariable) do + file.resolve_template_attribute("unknown_attribute") + end + end end end diff --git a/spec/marten/template/ext/marten/db/query/page_spec.cr b/spec/marten/template/ext/marten/db/query/page_spec.cr index de5f055f5..876e8e1c3 100644 --- a/spec/marten/template/ext/marten/db/query/page_spec.cr +++ b/spec/marten/template/ext/marten/db/query/page_spec.cr @@ -145,5 +145,14 @@ describe Marten::DB::Query::Page do Marten::DB::Query::Page(Tag).new([tag], 2, paginator).resolve_template_attribute("size").should eq 1 Marten::DB::Query::Page(Tag).new([] of Tag, 2, paginator).resolve_template_attribute("size").should eq 0 end + + it "raises as expected if the specified attribute is not supported" do + tag = Tag.create!(name: "a_tag", is_active: true) + paginator = Tag.all.order(:name).paginator(2) + + expect_raises(Marten::Template::Errors::UnknownVariable) do + Marten::DB::Query::Page(Tag).new([tag], 2, paginator).resolve_template_attribute("unknown") + end + end end end diff --git a/spec/marten/template/ext/marten/db/query/paginator_spec.cr b/spec/marten/template/ext/marten/db/query/paginator_spec.cr index fbdf7411e..a16304f7d 100644 --- a/spec/marten/template/ext/marten/db/query/paginator_spec.cr +++ b/spec/marten/template/ext/marten/db/query/paginator_spec.cr @@ -15,5 +15,13 @@ describe Marten::DB::Query::Paginator do paginator.resolve_template_attribute("pages_count").should eq 3 end + + it "raises as expected if the specified attribute is not supported" do + paginator = Tag.all.order(:name).paginator(2) + + expect_raises(Marten::Template::Errors::UnknownVariable) do + paginator.resolve_template_attribute("unknown_attribute") + end + end end end diff --git a/spec/marten/template/ext/marten/db/query/set_spec.cr b/spec/marten/template/ext/marten/db/query/set_spec.cr index cbf8f03f4..da97eaef6 100644 --- a/spec/marten/template/ext/marten/db/query/set_spec.cr +++ b/spec/marten/template/ext/marten/db/query/set_spec.cr @@ -145,5 +145,11 @@ describe Marten::DB::Query::Set do Tag.all.resolve_template_attribute("size").should eq 2 end + + it "raises as expected if the specified attribute is not supported" do + expect_raises(Marten::Template::Errors::UnknownVariable) do + Tag.all.resolve_template_attribute("unknown") + end + end end end diff --git a/spec/marten/template/ext/marten/http/flash_store_spec.cr b/spec/marten/template/ext/marten/http/flash_store_spec.cr index cb07c4862..fcde2a8e1 100644 --- a/spec/marten/template/ext/marten/http/flash_store_spec.cr +++ b/spec/marten/template/ext/marten/http/flash_store_spec.cr @@ -27,10 +27,12 @@ describe Marten::HTTP::FlashStore do flash_store.resolve_template_attribute("alert").should eq "bad" end - it "returns nil if the flash message is not found" do + it "raises as expected if the specified attribute is not supported" do flash_store = Marten::HTTP::FlashStore.new(flashes: {"foo" => "bar", "alert" => "bad"}, discard: [] of String) - flash_store.resolve_template_attribute("unknown").should be_nil + expect_raises(Marten::Template::Errors::UnknownVariable) do + flash_store.resolve_template_attribute("unknown") + end end end end diff --git a/spec/marten/template/ext/marten/schema/bound_field_spec.cr b/spec/marten/template/ext/marten/schema/bound_field_spec.cr index bd307bb36..3a6324b21 100644 --- a/spec/marten/template/ext/marten/schema/bound_field_spec.cr +++ b/spec/marten/template/ext/marten/schema/bound_field_spec.cr @@ -48,11 +48,13 @@ describe Marten::Schema::BoundField do bound_field_2.resolve_template_attribute("value").should be_nil end - it "returns nil if the passed attribute name is not found" do + it "raises as expected if the passed attribute is not supported" do schema = Marten::Schema::BoundFieldExtSpec::TestSchema.new(Marten::HTTP::Params::Data{"foo" => ["hello"]}) bound_field = Marten::Schema::BoundField.new(schema, schema.class.get_field("foo")) - bound_field.resolve_template_attribute("unknown").should be_nil + expect_raises(Marten::Template::Errors::UnknownVariable) do + bound_field.resolve_template_attribute("unknown") + end end end end diff --git a/spec/marten/template/ext/marten/schema_spec.cr b/spec/marten/template/ext/marten/schema_spec.cr index 2ed180847..78f8e88b7 100644 --- a/spec/marten/template/ext/marten/schema_spec.cr +++ b/spec/marten/template/ext/marten/schema_spec.cr @@ -17,10 +17,12 @@ describe Marten::Schema do errors.should eq schema.errors end - it "returns nil if the passed attribute name is not found" do + it "raises as expected if the passed attribute name is not found" do schema = Marten::SchemaExtSpec::SimpleSchema.new(Marten::HTTP::Params::Data{"foo" => ["hello"]}) - schema.resolve_template_attribute("unknown").should be_nil + expect_raises(Marten::Template::Errors::UnknownVariable) do + schema.resolve_template_attribute("unknown") + end end end end diff --git a/spec/marten/template/object/auto_spec.cr b/spec/marten/template/object/auto_spec.cr index 8311afc0b..f0163a837 100644 --- a/spec/marten/template/object/auto_spec.cr +++ b/spec/marten/template/object/auto_spec.cr @@ -7,34 +7,49 @@ describe Marten::Template::Object::Auto do obj.resolve_template_attribute("attr").should eq "hello" end - it "allows to resolve a key using the #[]? method as a fallback" do + it "allows to resolve a key using the #[] method as a fallback" do obj = Marten::Template::Object::AutoSpec::Test.new({"foo" => "bar"}) obj.resolve_template_attribute("foo").should eq "bar" end - it "does not resolve the value using the #[]? method if a public method is resolved first" do + it "resolves values #[] with priority over a method with the same key" do obj = Marten::Template::Object::AutoSpec::Test.new({"attr" => "bar"}) - obj.resolve_template_attribute("attr").should eq "hello" + obj.resolve_template_attribute("attr").should eq "bar" end it "does not allow to resolve a method that takes arguments" do obj = Marten::Template::Object::AutoSpec::Test.new - obj.resolve_template_attribute("with_args").should be_nil + expect_raises(Marten::Template::Errors::UnknownVariable) do + obj.resolve_template_attribute("with_args") + end end it "does not allow to resolve a method that takes a block" do obj = Marten::Template::Object::AutoSpec::Test.new - obj.resolve_template_attribute("with_block").should be_nil + expect_raises(Marten::Template::Errors::UnknownVariable) do + obj.resolve_template_attribute("with_block") + end end it "does not allow to resolve a protected method" do obj = Marten::Template::Object::AutoSpec::Test.new - obj.resolve_template_attribute("protected_attr").should be_nil + expect_raises(Marten::Template::Errors::UnknownVariable) do + obj.resolve_template_attribute("protected_attr") + end end it "does not allow to resolve a private method" do obj = Marten::Template::Object::AutoSpec::Test.new - obj.resolve_template_attribute("private_attr").should be_nil + expect_raises(Marten::Template::Errors::UnknownVariable) do + obj.resolve_template_attribute("private_attr") + end + end + + it "raises a Marten::Template::Errors::UnknownVariable exception if the attribute is not supported" do + obj = Marten::Template::Object::AutoSpec::Test.new + expect_raises(Marten::Template::Errors::UnknownVariable) do + obj.resolve_template_attribute("unknown") + end end end end @@ -46,8 +61,8 @@ module Marten::Template::Object::AutoSpec def initialize(@data = {} of String => String) end - def []?(key : String) - @data[key]? + def [](key : String) + @data[key] end def attr diff --git a/src/marten/template/concerns/can_define_template_attributes.cr b/src/marten/template/concerns/can_define_template_attributes.cr index dcd47e93b..ce0595b54 100644 --- a/src/marten/template/concerns/can_define_template_attributes.cr +++ b/src/marten/template/concerns/can_define_template_attributes.cr @@ -12,6 +12,8 @@ module Marten when {% if name.is_a?(StringLiteral) %}{{ name }}{% else %}{{ name.id.stringify }}{% end %} {{ name.id }} {% end %} + else + raise Marten::Template::Errors::UnknownVariable.new end end end diff --git a/src/marten/template/ext/marten/db/field/file/file.cr b/src/marten/template/ext/marten/db/field/file/file.cr index f98f581e3..67564ae28 100644 --- a/src/marten/template/ext/marten/db/field/file/file.cr +++ b/src/marten/template/ext/marten/db/field/file/file.cr @@ -1,17 +1,5 @@ class Marten::DB::Field::File::File include Marten::Template::Object - # :nodoc: - def resolve_template_attribute(key : ::String) - case key - when "attached?" - attached? - when "name" - name - when "size" - size - when "url" - url - end - end + template_attributes :attached?, :name, :size, :url end diff --git a/src/marten/template/ext/marten/db/query/page.cr b/src/marten/template/ext/marten/db/query/page.cr index c6517b1a1..9a0b02a0c 100644 --- a/src/marten/template/ext/marten/db/query/page.cr +++ b/src/marten/template/ext/marten/db/query/page.cr @@ -1,34 +1,6 @@ class Marten::DB::Query::Page(M) include Marten::Template::Object - def resolve_template_attribute(key : String) - case key - when "all?" - all? - when "any?" - any? - when "count" - count - when "empty?" - empty? - when "first?" - first? - when "next_page?" - next_page? - when "next_page_number" - next_page_number - when "none?" - none? - when "number" - number - when "one?" - one? - when "previous_page?" - previous_page? - when "previous_page_number" - previous_page_number - when "size" - size - end - end + template_attributes :all?, :any?, :count, :empty?, :first?, :next_page?, :next_page_number, :none?, :number, :one?, + :previous_page?, :previous_page_number, :size end diff --git a/src/marten/template/ext/marten/db/query/paginator.cr b/src/marten/template/ext/marten/db/query/paginator.cr index 912feb540..697816a13 100644 --- a/src/marten/template/ext/marten/db/query/paginator.cr +++ b/src/marten/template/ext/marten/db/query/paginator.cr @@ -1,12 +1,5 @@ class Marten::DB::Query::Paginator(M) include Marten::Template::Object - def resolve_template_attribute(key : String) - case key - when "page_size" - page_size - when "pages_count" - pages_count - end - end + template_attributes :page_size, :pages_count end diff --git a/src/marten/template/ext/marten/db/query/set.cr b/src/marten/template/ext/marten/db/query/set.cr index f70537001..d1c8d93d2 100644 --- a/src/marten/template/ext/marten/db/query/set.cr +++ b/src/marten/template/ext/marten/db/query/set.cr @@ -1,42 +1,6 @@ class Marten::DB::Query::Set(M) include Marten::Template::Object - def resolve_template_attribute(key : String) - case key - when "all" - all - when "all?" - all? - when "any?" - any? - when "count" - count - when "distinct" - distinct - when "empty?" - empty? - when "exists?" - exists? - when "first" - first - when "first!" - first! - when "first?" - first? - when "last" - last - when "last!" - last! - when "none" - none - when "none?" - none? - when "one?" - one? - when "reverse" - reverse - when "size" - size - end - end + template_attributes :all, :all?, :any?, :count, :distinct, :empty?, :exists?, :first, :first!, :first?, :last, :last!, + :none, :none?, :one?, :reverse, :size end diff --git a/src/marten/template/ext/marten/http/flash_store.cr b/src/marten/template/ext/marten/http/flash_store.cr index 35db12b11..22409ee95 100644 --- a/src/marten/template/ext/marten/http/flash_store.cr +++ b/src/marten/template/ext/marten/http/flash_store.cr @@ -9,7 +9,9 @@ class Marten::HTTP::FlashStore when "size" size else - self[key]? + self[key] end + rescue KeyError + raise Marten::Template::Errors::UnknownVariable.new end end diff --git a/src/marten/template/ext/marten/schema.cr b/src/marten/template/ext/marten/schema.cr index f1a3a6a7c..26843d1a5 100644 --- a/src/marten/template/ext/marten/schema.cr +++ b/src/marten/template/ext/marten/schema.cr @@ -7,7 +7,9 @@ abstract class Marten::Schema when "errors" errors else - self[key]? + self[key] end + rescue Marten::Schema::Errors::UnknownField + raise Marten::Template::Errors::UnknownVariable.new end end diff --git a/src/marten/template/ext/marten/schema/bound_field.cr b/src/marten/template/ext/marten/schema/bound_field.cr index be77ab178..11f77f15d 100644 --- a/src/marten/template/ext/marten/schema/bound_field.cr +++ b/src/marten/template/ext/marten/schema/bound_field.cr @@ -14,6 +14,8 @@ class Marten::Schema::BoundField id when "value" value + else + raise Marten::Template::Errors::UnknownVariable.new end end end diff --git a/src/marten/template/object/auto.cr b/src/marten/template/object/auto.cr index fcab9b8b3..1ab3f120b 100644 --- a/src/marten/template/object/auto.cr +++ b/src/marten/template/object/auto.cr @@ -12,27 +12,39 @@ module Marten # :nodoc: def resolve_template_attribute(key : String) {% begin %} - value = case key - {% if !@type.abstract? && !@type.type_vars.any?(&.abstract?) %} - {% already_processed = [] of String %} - {% for type in [@type] + @type.ancestors %} - {% if type.name != "Object" && type.name != "Reference" %} - {% for method in type.methods %} - {% if !already_processed.includes?(method.name.id.stringify) %} - {% if method.visibility == :public && !method.accepts_block? && method.args.empty? %} - when {{ method.name.id.stringify }} - self.{{ method.name.id }} - {% already_processed << method.name.id.stringify %} + value = nil + looked_up_value = false + + if responds_to?(:[]) + begin + value = self[key] + looked_up_value = true + rescue KeyError + # Do nothing + end + end + + if !looked_up_value + value = case key + {% if !@type.abstract? && !@type.type_vars.any?(&.abstract?) %} + {% already_processed = [] of String %} + {% for type in [@type] + @type.ancestors %} + {% if type.name != "Object" && type.name != "Reference" %} + {% for method in type.methods %} + {% if !already_processed.includes?(method.name.id.stringify) %} + {% if method.visibility == :public && !method.accepts_block? && method.args.empty? %} + when {{ method.name.id.stringify }} + self.{{ method.name.id }} + {% already_processed << method.name.id.stringify %} + {% end %} {% end %} {% end %} {% end %} {% end %} {% end %} - {% end %} - end - - if value.nil? && responds_to?(:[]?) - value = self[key]? + else + raise Marten::Template::Errors::UnknownVariable.new + end end value