Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate invalid custom attribute names #18538

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions app/models/mixins/custom_attribute_mixin.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ module CustomAttributeMixin
SECTION_SEPARATOR = ":SECTION:".freeze
DEFAULT_SECTION_NAME = 'Custom Attribute'.freeze

CUSTOM_ATTRIBUTE_INVALID_NAME_WARNING = "A custom attribute name must begin with a letter (a-z, but also letters with diacritical marks and non-Latin letters) or an underscore (_). Subsequent characters can be letters, underscores, digits (0-9), or dollar signs ($)".freeze
CUSTOM_ATTRIBUTE_VALID_NAME_REGEXP = /\A[\p{Alpha}_][\p{Alpha}_\d\$]*\z/

included do
has_many :custom_attributes, :as => :resource, :dependent => :destroy
has_many :miq_custom_attributes, -> { where(:source => 'EVM') }, :as => :resource, :dependent => :destroy, :class_name => "CustomAttribute"
Expand Down Expand Up @@ -38,8 +41,13 @@ def self.load_custom_attributes_for(cols)
custom_attributes.each { |custom_attribute| add_custom_attribute(custom_attribute) }
end

def self.invalid_custom_attribute_message(attribute)
"Invalid custom attribute: '#{attribute}'. #{CUSTOM_ATTRIBUTE_INVALID_NAME_WARNING}"
end

def self.add_custom_attribute(custom_attribute)
return if respond_to?(custom_attribute)
ActiveSupport::Deprecation.warn(invalid_custom_attribute_message(custom_attribute)) unless custom_attribute.to_s =~ CUSTOM_ATTRIBUTE_VALID_NAME_REGEXP

ca_sym = custom_attribute.to_sym
without_prefix = custom_attribute.sub(CUSTOM_ATTRIBUTES_PREFIX, "")
Expand Down Expand Up @@ -102,6 +110,7 @@ def miq_custom_get(key)

def miq_custom_set(key, value)
return miq_custom_delete(key) if value.blank?
ActiveSupport::Deprecation.warn(self.class.invalid_custom_attribute_message(key)) unless key.to_s =~ self.class::CUSTOM_ATTRIBUTE_VALID_NAME_REGEXP

record = miq_custom_attributes.find_by(:name => key.to_s)
if record.nil?
Expand Down
34 changes: 34 additions & 0 deletions spec/models/mixins/custom_attribute_mixin_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,40 @@ def self.name; "TestClass"; end
end
end

context ".add_custom_attribute" do
it "regular key" do
test_class.add_custom_attribute("foo")
expect(test_class.new).to respond_to(:foo)
expect(test_class.new).to respond_to(:foo=)
end

it "key with a letter followed by a number" do
test_class.add_custom_attribute("fun4all")
expect(test_class.new).to respond_to(:"fun4all")
expect(test_class.new).to respond_to(:"fun4all=")
end

it "key with a space(deprecated)" do
ActiveSupport::Deprecation.silence { test_class.add_custom_attribute("exit message") }
expect(test_class.new).to respond_to(:"exit message")
expect(test_class.new).to respond_to(:"exit message=")
end

it "key with leading number(deprecated)" do
ActiveSupport::Deprecation.silence { test_class.add_custom_attribute("4fun") }
expect(test_class.new).to respond_to(:"4fun")
expect(test_class.new).to respond_to(:"4fun=")
end
end

it "#miq_custom_set with a space(deprecated)" do
object = test_class.create!
ActiveSupport::Deprecation.silence { object.miq_custom_set("hello world", "baz") }
ca = CustomAttribute.find_by(:resource_type => test_class.name, :resource_id => object.id)
expect(ca.name).to eq("hello world")
expect(ca.value).to eq("baz")
end

it "#miq_custom_set" do
supported_factories.each do |factory_name|
object = FactoryBot.create(factory_name)
Expand Down