Merge pull request #16169 from bdunne/delete_settings_changes

Add a method to easily delete settings_changes for a resource

app/models/mixins/configuration_management_mixin.rb

@@ -11,17 +11,26 @@ def settings_for_resource
 
   def add_settings_for_resource(settings)
     Vmdb::Settings.save!(self, settings)
-    # Reload the settings immediately for this worker. This is typically a UI
-    # worker making the change, who will need to see the changes right away.
-    Vmdb::Settings.reload!
+    immediately_reload_settings
+  end
 
-    # Reload the settings for all workers on the servers whether local or remote.
-    reload_all_server_settings
+  def remove_settings_path_for_resource(*keys)
+    Vmdb::Settings.destroy!(self, keys)
+    immediately_reload_settings
   end
 
   def reload_all_server_settings
     servers_for_settings_reload.each do |server|
       server.enqueue_for_server("reload_settings") if server.started?
     end
   end
+
+  private def immediately_reload_settings
+    # Reload the settings immediately for this worker. This is typically a UI
+    # worker making the change, who will need to see the changes right away.
+    Vmdb::Settings.reload!
+
+    # Reload the settings for all workers on the servers whether local or remote.
+    reload_all_server_settings
+  end
 end

lib/vmdb/settings.rb

@@ -55,6 +55,12 @@ def self.save!(resource, hash)
       apply_settings_changes(resource, deltas)
     end
 
+    def self.destroy!(resource, keys)
+      return if keys.blank?
+      settings_path = File.join("/", keys.collect(&:to_s))
+      resource.settings_changes.where("key LIKE ?", "#{settings_path}%").destroy_all
+    end
+
     def self.for_resource(resource)
       build(resource).load!
     end

spec/models/miq_region_spec.rb

@@ -160,42 +160,6 @@
     end
   end
 
-  context "ConfigurationManagementMixin" do
-    describe "#settings_for_resource" do
-      it "returns the resource's settings" do
-        settings = {:some_thing => [1, 2, 3]}
-        stub_settings(settings)
-        expect(region.settings_for_resource.to_hash).to eq(settings)
-      end
-    end
-
-    describe "#add_settings_for_resource" do
-      it "sets the specified settings" do
-        settings = {:some_test_setting => {:setting => 1}}
-        expect(region).to receive(:reload_all_server_settings)
-
-        region.add_settings_for_resource(settings)
-
-        expect(Vmdb::Settings.for_resource(region).some_test_setting.setting).to eq(1)
-      end
-    end
-
-    describe "#reload_all_server_settings" do
-      it "queues #reload_settings for the started servers" do
-        started_server = FactoryGirl.create(:miq_server, :status => "started")
-        FactoryGirl.create(:miq_server, :status => "started", :id => external_region_id)
-        FactoryGirl.create(:miq_server, :status => "stopped")
-
-        region.reload_all_server_settings
-
-        expect(MiqQueue.count).to eq(1)
-        message = MiqQueue.first
-        expect(message.instance_id).to eq(started_server.id)
-        expect(message.method_name).to eq("reload_settings")
-      end
-    end
-  end
-
   describe "#vms" do
     it "brings them back" do
       FactoryGirl.create(:vm_vmware, :id => external_region_id)

spec/models/miq_server/configuration_management_spec.rb

@@ -51,39 +51,6 @@
 
   context "ConfigurationManagementMixin" do
     let(:miq_server) { FactoryGirl.create(:miq_server) }
-
-    describe "#settings_for_resource" do
-      it "returns the resource's settings" do
-        settings = {:some_thing => [1, 2, 3]}
-        stub_settings(settings)
-        expect(miq_server.settings_for_resource.to_hash).to eq(settings)
-      end
-    end
-
-    describe "#add_settings_for_resource" do
-      it "sets the specified settings" do
-        settings = {:some_test_setting => {:setting => 1}}
-        expect(miq_server).to receive(:reload_all_server_settings)
-
-        miq_server.add_settings_for_resource(settings)
-
-        expect(Vmdb::Settings.for_resource(miq_server).some_test_setting.setting).to eq(1)
-      end
-    end
-
-    describe "#reload_all_server_settings" do
-      it "queues #reload_settings for the started servers" do
-        FactoryGirl.create(:miq_server, :status => "started")
-
-        miq_server.reload_all_server_settings
-
-        expect(MiqQueue.count).to eq(1)
-        message = MiqQueue.first
-        expect(message.instance_id).to eq(miq_server.id)
-        expect(message.method_name).to eq("reload_settings")
-      end
-    end
-
     describe "#config_activated" do
       let(:zone) { FactoryGirl.create(:zone, :name => "My Zone") }
       let(:zone_other_region) do

spec/models/mixins/configuration_management_mixin_spec.rb

@@ -0,0 +1,63 @@
+describe ConfigurationManagementMixin do
+  let(:miq_server) { FactoryGirl.create(:miq_server, :zone => zone, :status => "started") }
+  let(:region)     { FactoryGirl.create(:miq_region, :region => ApplicationRecord.my_region_number) }
+  let(:settings)   { {:some_test_setting => {:setting => {:deeper => 1}, :other => 2}} }
+  let(:zone)       { FactoryGirl.create(:zone) }
+
+  [:miq_server, :region, :zone].each do |i|
+    context "On a #{i.capitalize}" do
+      subject { send(i) }
+
+      describe "#settings_for_resource" do
+        it "returns the resource's settings" do
+          stub_settings(settings)
+          expect(subject.settings_for_resource.to_hash).to eq(settings)
+        end
+      end
+
+      describe "#add_settings_for_resource" do
+        it "sets the specified settings" do
+          expect(subject).to receive(:reload_all_server_settings)
+
+          subject.add_settings_for_resource(settings)
+
+          expect(Vmdb::Settings.for_resource(subject).some_test_setting.setting.deeper).to eq(1)
+        end
+      end
+
+      describe "#remove_settings_path_for_resource" do
+        it "removes the specified setting record and all children" do
+          expect(subject).to receive(:reload_all_server_settings).twice
+
+          subject.add_settings_for_resource(settings)
+
+          expect(Vmdb::Settings.for_resource(subject).some_test_setting.setting.deeper).to eq(1)
+
+          subject.remove_settings_path_for_resource(:some_test_setting, :setting)
+
+          expect(Vmdb::Settings.for_resource(subject).some_test_setting.to_h).to eq(:other => 2)
+        end
+      end
+
+      describe "#reload_all_server_settings" do
+        it "queues #reload_settings for the started servers" do
+          started_server = miq_server
+
+          # the first id from a region other than ours
+          remote_region_number = ApplicationRecord.my_region_number + 1
+          external_region_id   = ApplicationRecord.region_to_range(remote_region_number).first
+
+          FactoryGirl.create(:miq_server, :status => "started", :id => external_region_id)
+          FactoryGirl.create(:miq_server, :status => "stopped")
+
+          subject.reload_all_server_settings
+
+          expect(MiqQueue.count).to eq(1)
+          message = MiqQueue.first
+          expect(message.instance_id).to eq(started_server.id)
+          expect(message.method_name).to eq("reload_settings")
+        end
+      end
+    end
+  end
+end

spec/models/zone_spec.rb

@@ -184,45 +184,6 @@
     expect(described_class.new.settings).to be_kind_of(Hash)
   end
 
-  context "ConfigurationManagementMixin" do
-    let(:zone) { FactoryGirl.create(:zone) }
-
-    describe "#settings_for_resource" do
-      it "returns the resource's settings" do
-        settings = {:some_thing => [1, 2, 3]}
-        stub_settings(settings)
-        expect(zone.settings_for_resource.to_hash).to eq(settings)
-      end
-    end
-
-    describe "#add_settings_for_resource" do
-      it "sets the specified settings" do
-        settings = {:some_test_setting => {:setting => 1}}
-        expect(zone).to receive(:reload_all_server_settings)
-
-        zone.add_settings_for_resource(settings)
-
-        expect(Vmdb::Settings.for_resource(zone).some_test_setting.setting).to eq(1)
-      end
-    end
-
-    describe "#reload_all_server_settings" do
-      it "queues #reload_settings for the started servers" do
-        some_other_zone = FactoryGirl.create(:zone)
-        started_server = FactoryGirl.create(:miq_server, :status => "started", :zone => zone)
-        FactoryGirl.create(:miq_server, :status => "started", :zone => some_other_zone)
-        FactoryGirl.create(:miq_server, :status => "stopped", :zone => zone)
-
-        zone.reload_all_server_settings
-
-        expect(MiqQueue.count).to eq(1)
-        message = MiqQueue.first
-        expect(message.instance_id).to eq(started_server.id)
-        expect(message.method_name).to eq("reload_settings")
-      end
-    end
-  end
-
   context "ConfigurationManagementMixin" do
     describe "#remote_cockpit_ws_miq_server" do
       before(:each) do