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

Ability to reset settings to default value and delete newly added keys #17482

Merged
merged 6 commits into from
Jun 8, 2018
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
38 changes: 37 additions & 1 deletion lib/vmdb/settings.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
module Vmdb
class Settings
extend Vmdb::SettingsWalker::ClassMethods
include Vmdb::Logging

class ConfigurationInvalid < StandardError
attr_accessor :errors
Expand All @@ -21,6 +22,11 @@ def initialize(errors)
PASSWORD_FIELDS = Vmdb::SettingsWalker::PASSWORD_FIELDS
DUMP_LOG_FILE = Rails.root.join("log/last_settings.txt").freeze

# RESET_COMMAND remove record for selected key and specific resource
# RESET_ALL_COMMAND remove all records for selected key and all resources
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these should match the name of the constant, right? DELETE_COMMAND and DELETE_ALL_COMMAND

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

MAGIC_VALUES = [RESET_COMMAND = "<<reset>>".freeze,
RESET_ALL_COMMAND = "<<reset_all>>".freeze].freeze
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The definitions inside other definitions is weird, but...


cattr_accessor :last_loaded

def self.init
Expand Down Expand Up @@ -62,8 +68,9 @@ def self.valid?

def self.save!(resource, hash)
new_settings = build_without_local(resource).load!.merge!(hash).to_hash
settings_to_validate = remove_magic_values(new_settings.deep_clone)

valid, errors = validate(new_settings)
valid, errors = validate(settings_to_validate)
raise ConfigurationInvalid.new(errors) unless valid # rubocop:disable Style/RaiseArgs

hash_for_parent = parent_settings_without_local(resource).load!.to_hash
Expand Down Expand Up @@ -188,6 +195,7 @@ def self.apply_settings_changes(resource, deltas)

deltas.each do |delta|
record = index.delete(delta[:key])
next if process_magic_value(resource, delta)
if record
record.update_attributes!(delta)
else
Expand All @@ -198,5 +206,33 @@ def self.apply_settings_changes(resource, deltas)
end
end
private_class_method :apply_settings_changes

def self.process_magic_value(resource, delta)
return false unless MAGIC_VALUES.include?(delta[:value])
case delta[:value]
when RESET_COMMAND
_log.info("resetting #{delta[:key]} on #{resource.class} id:#{resource.id}")
resource.settings_changes.where("key LIKE ?", "#{delta[:key]}%").destroy_all
when RESET_ALL_COMMAND
_log.info("resetting #{delta[:key]} to default for all resorces")
SettingsChange.where("key LIKE ?", "#{delta[:key]}%").destroy_all
end
true
end
private_class_method :process_magic_value

def self.walk_hash(hash, &block)
hash.each do |key, value|
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer using the existing walk method, which already handles this. Even so, I don't think Iike this manual thing at all, A reset is really just taking the parent's value at that level and replacing it. Then, let the differ handle the rest.

yield key, value, hash
walk_hash(value, &block) if value&.class == Hash
end
end
private_class_method :walk_hash

def self.remove_magic_values(hash)
walk_hash(hash) { |key, value, owner| owner.delete(key) if MAGIC_VALUES.include?(value) }
hash
end
private_class_method :remove_magic_values
end
end
79 changes: 79 additions & 0 deletions spec/lib/vmdb/settings_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,85 @@
miq_server.zone.reload
expect(miq_server.zone.settings_changes.count).to eq 0
end

context "deleting entries" do
let(:server_value) { 1 }
let(:zone_value) { 2 }

let(:reset) { ::Vmdb::Settings::RESET_COMMAND }
let(:reset_all) { ::Vmdb::Settings::RESET_ALL_COMMAND }

let(:second_server) { FactoryGirl.create(:miq_server, :zone => miq_server.zone) }

before do
MiqRegion.seed

described_class.save!(miq_server, :api => {:token_ttl => server_value}, :session => {:timeout => server_value})
described_class.save!(miq_server, :api => {:new_key => "new value"})
described_class.save!(second_server, :api => {:token_ttl => server_value}, :session => {:timeout => server_value})

described_class.save!(miq_server.zone, :api => {:token_ttl => zone_value}, :session => {:timeout => zone_value})
described_class.save!(miq_server, :array => [:element1 => 1, :element2 => 2])
expect(SettingsChange.count).to eq 8
end

context "magic value <<reset>>" do
it "deletes key-value for specific key for the resource if specified on leaf level" do
described_class.save!(miq_server, :api => {:token_ttl => reset})

expect(SettingsChange.count).to eq 7
expect(miq_server.settings_changes.find_by(:key => "/api/token_ttl")).to be nil
expect(Vmdb::Settings.for_resource(miq_server).api.new_key).to eq "new value"
expect(Vmdb::Settings.for_resource(second_server).api.token_ttl).to eq server_value
end

it "deletes current node and all sub-nodes for the resource if specified on node level" do
described_class.save!(miq_server, :api => reset)

expect(SettingsChange.count).to eq 6
expect(miq_server.settings_changes.where("key LIKE ?", "/api%").count).to eq 0
expect(Vmdb::Settings.for_resource(miq_server).api.new_key).to eq nil
expect(Vmdb::Settings.for_resource(second_server).api.token_ttl).to eq server_value
end

it "deletes new key-value settings not present in defaul yaml" do
described_class.save!(miq_server, :api => {:new_key => reset})
expect(Vmdb::Settings.for_resource(miq_server).api.new_key).to eq nil
end

it "deletes array" do
described_class.save!(miq_server, :array => reset)
expect(Vmdb::Settings.for_resource(miq_server).array).to be nil
end

it "passes validation" do
described_class.save!(miq_server, :session => {:timeout => reset})
expect(Vmdb::Settings.for_resource(miq_server).session.timeout).to eq zone_value
end
end

context "magic value <<reset_all>>" do
it "deletes all key-value for specific key for all resource if specified on leaf levelher" do
described_class.save!(miq_server, :api => {:token_ttl => reset_all})

expect(SettingsChange.where("key LIKE ?", "/api/token_ttl").count).to eq 0
expect(SettingsChange.where("key LIKE ?", "/api%").count).to eq 1
expect(SettingsChange.where("key LIKE ?", "/session%").count).to eq 3
end

it "deletes specific node and all sub-nodes for all resources if specified on node level" do
described_class.save!(miq_server, :api => reset_all)

expect(SettingsChange.where("key LIKE ?", "/api%").count).to eq 0
expect(SettingsChange.where("key LIKE ?", "/session%").count).to eq 3
end

it "passes validation" do
described_class.save!(miq_server, :session => {:timeout => reset_all})
expect(SettingsChange.where("key LIKE ?", "/session%").count).to eq 0
end
end
end
end

describe "save_yaml!" do
Expand Down