From 51de3cf238070419da34f3bfb9ebb06f9bb55f83 Mon Sep 17 00:00:00 2001 From: Semyon Pupkov Date: Thu, 14 Sep 2023 16:23:10 +0500 Subject: [PATCH 1/4] Pass class name to Sequel::MassAssignmentRestriction error If the instance has a class name, pass this name to the error message to better understand the error in monitoring systems such as Sentry. --- lib/sequel/model/base.rb | 11 ++++++++--- spec/extensions/csv_serializer_spec.rb | 3 ++- spec/model/base_spec.rb | 16 +++++++++++++++- 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/lib/sequel/model/base.rb b/lib/sequel/model/base.rb index 1c1a6f1f35..7a8df9637e 100644 --- a/lib/sequel/model/base.rb +++ b/lib/sequel/model/base.rb @@ -2035,15 +2035,20 @@ def set_restricted(hash, type) if meths.include?(m) set_column_value(m, v) elsif strict + err_msg = -> (msg) do + msg += " for class #{self.class.name}" if self.class.name + msg + end + # Avoid using respond_to? or creating symbols from user input if public_methods.map(&:to_s).include?(m) if Array(model.primary_key).map(&:to_s).member?(k.to_s) && model.restrict_primary_key? - raise MassAssignmentRestriction, "#{k} is a restricted primary key" + raise MassAssignmentRestriction, err_msg.call("#{k} is a restricted primary key") else - raise MassAssignmentRestriction, "#{k} is a restricted column" + raise MassAssignmentRestriction, err_msg.call("#{k} is a restricted column") end else - raise MassAssignmentRestriction, "method #{m} doesn't exist" + raise MassAssignmentRestriction, err_msg.call("method #{m} doesn't exist") end end end diff --git a/spec/extensions/csv_serializer_spec.rb b/spec/extensions/csv_serializer_spec.rb index 968eaf3bcb..08bccc74dc 100644 --- a/spec/extensions/csv_serializer_spec.rb +++ b/spec/extensions/csv_serializer_spec.rb @@ -163,7 +163,8 @@ def self.name; 'Album' end it "should raise an error if attempting to set a restricted column and :all_columns is not used" do @Artist.restrict_primary_key - proc{@Artist.from_csv(@artist.to_csv)}.must_raise(Sequel::MassAssignmentRestriction) + err = proc{@Artist.from_csv(@artist.to_csv)}.must_raise(Sequel::MassAssignmentRestriction) + err.message.must_equal("id is a restricted primary key for class Artist") end it "should use a dataset's selected columns" do diff --git a/spec/model/base_spec.rb b/spec/model/base_spec.rb index 78c29bd498..9eec636c15 100644 --- a/spec/model/base_spec.rb +++ b/spec/model/base_spec.rb @@ -664,13 +664,27 @@ class ModelTest2 < Sequel::Model(@db[:foo]) end it "should raise an error if a missing/restricted column/method is accessed" do - proc{@c.new(:a=>1)}.must_raise(Sequel::MassAssignmentRestriction) + err = proc{@c.new(:a=>1)}.must_raise(Sequel::MassAssignmentRestriction) + err.message.must_equal("method a= doesn't exist") proc{@c.create(:a=>1)}.must_raise(Sequel::MassAssignmentRestriction) c = @c.new proc{c.set(:a=>1)}.must_raise(Sequel::MassAssignmentRestriction) proc{c.update(:a=>1)}.must_raise(Sequel::MassAssignmentRestriction) end + it "should add class name to error message" do + item = Class.new(Sequel::Model(:items)) do + columns :id + end + + item.class_eval do + def self.name; 'Item' end + end + + err = proc{item.new(:a=>1)}.must_raise(Sequel::MassAssignmentRestriction) + err.message.must_equal("method a= doesn't exist for class Item") + end + it "should be disabled by strict_param_setting = false" do @c.strict_param_setting = false @c.strict_param_setting.must_equal false From 5ab8426e79fe31e80978ad090aa0fe07974a0252 Mon Sep 17 00:00:00 2001 From: Semyon Pupkov Date: Fri, 15 Sep 2023 13:00:42 +0500 Subject: [PATCH 2/4] build msg inside exception --- lib/sequel/model/base.rb | 11 +++-------- lib/sequel/model/exceptions.rb | 13 ++++++++++--- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/lib/sequel/model/base.rb b/lib/sequel/model/base.rb index 7a8df9637e..71a9a228e7 100644 --- a/lib/sequel/model/base.rb +++ b/lib/sequel/model/base.rb @@ -2035,20 +2035,15 @@ def set_restricted(hash, type) if meths.include?(m) set_column_value(m, v) elsif strict - err_msg = -> (msg) do - msg += " for class #{self.class.name}" if self.class.name - msg - end - # Avoid using respond_to? or creating symbols from user input if public_methods.map(&:to_s).include?(m) if Array(model.primary_key).map(&:to_s).member?(k.to_s) && model.restrict_primary_key? - raise MassAssignmentRestriction, err_msg.call("#{k} is a restricted primary key") + raise MassAssignmentRestriction.new("#{k} is a restricted primary key", self) else - raise MassAssignmentRestriction, err_msg.call("#{k} is a restricted column") + raise MassAssignmentRestriction.new("#{k} is a restricted column", self) end else - raise MassAssignmentRestriction, err_msg.call("method #{m} doesn't exist") + raise MassAssignmentRestriction.new("method #{m} doesn't exist", self) end end end diff --git a/lib/sequel/model/exceptions.rb b/lib/sequel/model/exceptions.rb index 81aabacb28..a9a0a18004 100644 --- a/lib/sequel/model/exceptions.rb +++ b/lib/sequel/model/exceptions.rb @@ -24,11 +24,18 @@ def initialize(message=nil, model=nil) UndefinedAssociation = Class.new(Error) ).name - ( # Raised when a mass assignment method is called in strict mode with either a restricted column # or a column without a setter method. - MassAssignmentRestriction = Class.new(Error) - ).name + class MassAssignmentRestriction < Error + # The Sequel::Model object related to this exception. + attr_reader :model + + def initialize(msg, model) + @model = model + msg += " for class #{model.class.name}" if model.class.name + super(msg) + end + end # Exception class raised when +raise_on_save_failure+ is set and validation fails class ValidationFailed < Error From 4a3dbdbd195c8f27c3a68fc9299c31a910bf6c1e Mon Sep 17 00:00:00 2001 From: Semyon Pupkov Date: Sat, 16 Sep 2023 19:13:55 +0500 Subject: [PATCH 3/4] use create method --- lib/sequel/model/base.rb | 6 +++--- lib/sequel/model/exceptions.rb | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/sequel/model/base.rb b/lib/sequel/model/base.rb index 71a9a228e7..1d5507589d 100644 --- a/lib/sequel/model/base.rb +++ b/lib/sequel/model/base.rb @@ -2038,12 +2038,12 @@ def set_restricted(hash, type) # Avoid using respond_to? or creating symbols from user input if public_methods.map(&:to_s).include?(m) if Array(model.primary_key).map(&:to_s).member?(k.to_s) && model.restrict_primary_key? - raise MassAssignmentRestriction.new("#{k} is a restricted primary key", self) + raise MassAssignmentRestriction.create("#{k} is a restricted primary key", self) else - raise MassAssignmentRestriction.new("#{k} is a restricted column", self) + raise MassAssignmentRestriction.create("#{k} is a restricted column", self) end else - raise MassAssignmentRestriction.new("method #{m} doesn't exist", self) + raise MassAssignmentRestriction.create("method #{m} doesn't exist", self) end end end diff --git a/lib/sequel/model/exceptions.rb b/lib/sequel/model/exceptions.rb index a9a0a18004..30e98edd99 100644 --- a/lib/sequel/model/exceptions.rb +++ b/lib/sequel/model/exceptions.rb @@ -30,10 +30,10 @@ class MassAssignmentRestriction < Error # The Sequel::Model object related to this exception. attr_reader :model - def initialize(msg, model) + def self.create(msg, model) @model = model msg += " for class #{model.class.name}" if model.class.name - super(msg) + new(msg) end end From fbad090e267b5e3525334a6c51d7331883a8effb Mon Sep 17 00:00:00 2001 From: Semyon Pupkov Date: Sun, 17 Sep 2023 02:21:07 +0500 Subject: [PATCH 4/4] instance variable --- lib/sequel/model/exceptions.rb | 5 +++-- spec/model/base_spec.rb | 1 + spec/model/exceptions_spec.rb | 15 +++++++++++++++ 3 files changed, 19 insertions(+), 2 deletions(-) create mode 100644 spec/model/exceptions_spec.rb diff --git a/lib/sequel/model/exceptions.rb b/lib/sequel/model/exceptions.rb index 30e98edd99..72a5a98f55 100644 --- a/lib/sequel/model/exceptions.rb +++ b/lib/sequel/model/exceptions.rb @@ -31,9 +31,10 @@ class MassAssignmentRestriction < Error attr_reader :model def self.create(msg, model) - @model = model msg += " for class #{model.class.name}" if model.class.name - new(msg) + new(msg).tap do |err| + err.instance_variable_set(:@model, model) + end end end diff --git a/spec/model/base_spec.rb b/spec/model/base_spec.rb index 9eec636c15..811c9a9916 100644 --- a/spec/model/base_spec.rb +++ b/spec/model/base_spec.rb @@ -683,6 +683,7 @@ def self.name; 'Item' end err = proc{item.new(:a=>1)}.must_raise(Sequel::MassAssignmentRestriction) err.message.must_equal("method a= doesn't exist for class Item") + err.model.class.must_equal(item) end it "should be disabled by strict_param_setting = false" do diff --git a/spec/model/exceptions_spec.rb b/spec/model/exceptions_spec.rb new file mode 100644 index 0000000000..cc0f2ee025 --- /dev/null +++ b/spec/model/exceptions_spec.rb @@ -0,0 +1,15 @@ +require_relative "spec_helper" + +describe Sequel::MassAssignmentRestriction, "#create" do + it "should set model attr" do + model_cls = Class.new + model_cls.class_eval do + def self.name; 'TestModel' end + end + + model = model_cls.new + err = Sequel::MassAssignmentRestriction.create("method foo doesn't exist", model) + err.message.must_equal("method foo doesn't exist for class TestModel") + err.model.must_equal(model) + end +end