From f24702ddce596fa854cedc100a89c17b6c35166a Mon Sep 17 00:00:00 2001 From: Jeremy Evans Date: Mon, 18 Sep 2023 12:32:17 -0700 Subject: [PATCH] Add MassAssignmentRestriction#column Update MassAssignmentRestriction.create to take the column. Also, have it always include the class in the error message, even if the class is anonymous. For consistency, the column is always saved as string, since the mass assignment methods are generally used with string-keyed hashes provided by the user, and the comparison to allowed methods is done using strings. Simplify some of the related test code. --- CHANGELOG | 2 ++ lib/sequel/model/base.rb | 9 +++++---- lib/sequel/model/exceptions.rb | 14 +++++++++----- spec/extensions/csv_serializer_spec.rb | 4 +++- spec/model/base_spec.rb | 14 ++++++-------- spec/model/exceptions_spec.rb | 13 +++++++++---- 6 files changed, 34 insertions(+), 22 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index d3294ee1a..3093c732a 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,5 +1,7 @@ === master +* Add MassAssignmentRestriction#model and #column for getting the model instance and related column for mass assignment errors (artofhuman, jeremyevans) (#2079) + * Stop using base64 library in column_encryption plugin (jeremyevans) === 5.72.0 (2023-09-01) diff --git a/lib/sequel/model/base.rb b/lib/sequel/model/base.rb index 1d5507589..f23050260 100644 --- a/lib/sequel/model/base.rb +++ b/lib/sequel/model/base.rb @@ -2031,19 +2031,20 @@ def set_restricted(hash, type) meths = setter_methods(type) strict = strict_param_setting hash.each do |k,v| + k = k.to_s m = "#{k}=" if meths.include?(m) set_column_value(m, v) elsif strict # 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.create("#{k} is a restricted primary key", self) + if Array(model.primary_key).map(&:to_s).member?(k) && model.restrict_primary_key? + raise MassAssignmentRestriction.create("#{k} is a restricted primary key", self, k) else - raise MassAssignmentRestriction.create("#{k} is a restricted column", self) + raise MassAssignmentRestriction.create("#{k} is a restricted column", self, k) end else - raise MassAssignmentRestriction.create("method #{m} doesn't exist", self) + raise MassAssignmentRestriction.create("method #{m} doesn't exist", self, k) end end end diff --git a/lib/sequel/model/exceptions.rb b/lib/sequel/model/exceptions.rb index 72a5a98f5..588157fd5 100644 --- a/lib/sequel/model/exceptions.rb +++ b/lib/sequel/model/exceptions.rb @@ -30,11 +30,15 @@ class MassAssignmentRestriction < Error # The Sequel::Model object related to this exception. attr_reader :model - def self.create(msg, model) - msg += " for class #{model.class.name}" if model.class.name - new(msg).tap do |err| - err.instance_variable_set(:@model, model) - end + # The column related to this exception, as a string. + attr_reader :column + + # Create an instance of this class with the model and column set. + def self.create(msg, model, column) + e = new("#{msg} for class #{model.class.inspect}") + e.instance_variable_set(:@model, model) + e.instance_variable_set(:@column, column) + e end end diff --git a/spec/extensions/csv_serializer_spec.rb b/spec/extensions/csv_serializer_spec.rb index 08bccc74d..e64e37223 100644 --- a/spec/extensions/csv_serializer_spec.rb +++ b/spec/extensions/csv_serializer_spec.rb @@ -164,7 +164,9 @@ 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 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") + err.message.must_include("id is a restricted primary key for class ") + err.model.class.must_equal @Artist + err.column.must_equal "id" 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 811c9a991..386ba2bf3 100644 --- a/spec/model/base_spec.rb +++ b/spec/model/base_spec.rb @@ -665,7 +665,8 @@ class ModelTest2 < Sequel::Model(@db[:foo]) it "should raise an error if a missing/restricted column/method is accessed" do err = proc{@c.new(:a=>1)}.must_raise(Sequel::MassAssignmentRestriction) - err.message.must_equal("method a= doesn't exist") + err.message.must_include("method a= doesn't exist for class ") + err.column.must_equal("a") proc{@c.create(:a=>1)}.must_raise(Sequel::MassAssignmentRestriction) c = @c.new proc{c.set(:a=>1)}.must_raise(Sequel::MassAssignmentRestriction) @@ -673,17 +674,14 @@ class ModelTest2 < Sequel::Model(@db[:foo]) 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 + item = Class.new(Sequel::Model(:items)) + item.columns :id + def item.inspect; '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) + err.column.must_equal("a") 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 index cc0f2ee02..815b99f63 100644 --- a/spec/model/exceptions_spec.rb +++ b/spec/model/exceptions_spec.rb @@ -3,13 +3,18 @@ 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 = Sequel::MassAssignmentRestriction.create("method foo doesn't exist", model, "foo") + err.message.must_include("method foo doesn't exist for class ") + err.model.must_equal(model) + err.column.must_equal("foo") + + def model_cls.inspect; 'TestModel' end + model = model_cls.new + err = Sequel::MassAssignmentRestriction.create("method foo doesn't exist", model, "foo") err.message.must_equal("method foo doesn't exist for class TestModel") err.model.must_equal(model) + err.column.must_equal("foo") end end