Merge pull request #2972 from DataDog/option-unset

lib/datadog/core/configuration/option.rb

@@ -8,17 +8,33 @@ module Configuration
       class Option
         attr_reader :definition
 
-        # Option setting precedence. Higher number means higher precedence.
+        # Option setting precedence.
         module Precedence
+          # Represents an Option precedence level.
+          # Each precedence has a `numeric` value; higher values means higher precedence.
+          # `name` is for inspection purposes only.
+          Value = Struct.new(:numeric, :name) do
+            include Comparable
+
+            def <=>(other)
+              return nil unless other.is_a?(Value)
+
+              numeric <=> other.numeric
+            end
+          end
+
           # Remote configuration provided through the Datadog app.
-          REMOTE_CONFIGURATION = [2, :remote_configuration].freeze
+          REMOTE_CONFIGURATION = Value.new(2, :remote_configuration).freeze
 
           # Configuration provided in Ruby code, in this same process
           # or via Environment variable
-          PROGRAMMATIC = [1, :programmatic].freeze
+          PROGRAMMATIC = Value.new(1, :programmatic).freeze
 
           # Configuration that comes from default values
-          DEFAULT = [0, :default].freeze
+          DEFAULT = Value.new(0, :default).freeze
+
+          # All precedences, sorted from highest to lowest
+          LIST = [REMOTE_CONFIGURATION, PROGRAMMATIC, DEFAULT].sort.reverse.freeze
         end
 
         def initialize(definition, context)
@@ -27,6 +43,10 @@ def initialize(definition, context)
           @value = nil
           @is_set = false
 
+          # One value is stored per precedence, to allow unsetting a higher
+          # precedence value and falling back to a lower precedence one.
+          @value_per_precedence = Hash.new(UNSET)
+
           # Lowest precedence, to allow for `#set` to always succeed for a brand new `Option` instance.
           @precedence_set = Precedence::DEFAULT
         end
@@ -38,22 +58,54 @@ def initialize(definition, context)
         # @param value [Object] the new value to be associated with this option
         # @param precedence [Precedence] from what precedence order this new value comes from
         def set(value, precedence: Precedence::PROGRAMMATIC)
-          # Cannot override higher precedence value
-          if precedence[0] < @precedence_set[0]
+          # Is there a higher precedence value set?
+          if @precedence_set > precedence
+            # This should be uncommon, as higher precedence values tend to
+            # happen later in the application lifecycle.
             Datadog.logger.info do
-              "Option '#{definition.name}' not changed to '#{value}' (precedence: #{precedence[1]}) because the higher " \
-                "precedence value '#{@value}' (precedence: #{@precedence_set[1]}) was already set."
+              "Option '#{definition.name}' not changed to '#{value}' (precedence: #{precedence.name}) because the higher " \
+                "precedence value '#{@value}' (precedence: #{@precedence_set.name}) was already set."
             end
 
+            # But if it happens, we have to store the lower precedence value `value`
+            # because it's possible to revert to it by `#unset`ting
+            # the existing, higher-precedence value.
+            # Effectively, we always store one value pre precedence.
+            @value_per_precedence[precedence] = value
+
             return @value
           end
 
-          old_value = @value
-          (@value = context_exec(validate_type(value), old_value, &definition.setter)).tap do |v|
-            @is_set = true
-            @precedence_set = precedence
-            context_exec(v, old_value, &definition.on_set) if definition.on_set
+          internal_set(value, precedence)
+        end
+
+        def unset(precedence)
+          @value_per_precedence[precedence] = UNSET
+
+          # If we are unsetting the currently active value, we have to restore
+          # a lower precedence one...
+          if precedence == @precedence_set
+            # Find a lower precedence value that is already set.
+            Precedence::LIST.each do |p|
+              # DEV: This search can be optimized, but the list is small, and unset is
+              # DEV: only called from direct user interaction in the Datadog UI.
+              next unless p < precedence
+
+              # Look for value that is set.
+              # The hash `@value_per_precedence` has a custom default value of `UNSET`.
+              if (value = @value_per_precedence[p]) != UNSET
+                internal_set(value, p)
+                return nil
+              end
+            end
+
+            # If no value is left to fall back on, reset this option
+            reset
           end
+
+          # ... otherwise, we are either unsetting a higher precedence value that is not
+          # yet set, thus there's nothing to do; or we are unsetting a lower precedence
+          # value, which also does not change the current value.
         end
 
         def get
@@ -77,6 +129,7 @@ def reset
                      nil
                    end
 
+          # Reset back to the lowest precedence, to allow all `set`s to succeed right after a reset.
           @precedence_set = Precedence::DEFAULT
         end
 
@@ -185,6 +238,17 @@ def validate(type, value)
           end
         end
 
+        # Directly manipulates the current value and currently set precedence.
+        def internal_set(value, precedence)
+          old_value = @value
+          (@value = context_exec(validate_type(value), old_value, &definition.setter)).tap do |v|
+            @is_set = true
+            @precedence_set = precedence
+            @value_per_precedence[precedence] = v
+            context_exec(v, old_value, &definition.on_set) if definition.on_set
+          end
+        end
+
         def context_exec(*args, &block)
           @context.instance_exec(*args, &block)
         end
@@ -223,6 +287,11 @@ def skip_validation?
         # Used for testing
         attr_reader :precedence_set
         private :precedence_set
+
+        # Anchor object that represents a value that is not set.
+        # This is necessary because `nil` is a valid value to be set.
+        UNSET = Object.new
+        private_constant :UNSET
       end
     end
   end

lib/datadog/core/configuration/options.rb

@@ -72,6 +72,11 @@ def set_option(name, value, precedence: Configuration::Option::Precedence::PROGR
             options[name].set(value, precedence: precedence)
           end
 
+          def unset_option(name, precedence: Configuration::Option::Precedence::PROGRAMMATIC)
+            add_option(name) unless options.key?(name)
+            options[name].unset(precedence)
+          end
+
           def get_option(name)
             add_option(name) unless options.key?(name)
             options[name].get

spec/datadog/core/configuration/option_spec.rb

@@ -151,6 +151,7 @@
 
         before do
           option.set(:original_value, precedence: Datadog::Core::Configuration::Option::Precedence::REMOTE_CONFIGURATION)
+          allow(Datadog.logger).to receive(:info)
         end
 
         it 'overrides with value with the same precedence' do
@@ -188,6 +189,7 @@
 
         before do
           option.set(:original_value, precedence: Datadog::Core::Configuration::Option::Precedence::PROGRAMMATIC)
+          allow(Datadog.logger).to receive(:info)
         end
 
         it 'overrides with value with precedence REMOTE_CONFIGURATION' do
@@ -465,6 +467,131 @@
     end
   end
 
+  describe '#unset' do
+    before { allow(Datadog.logger).to receive(:info) }
+
+    # Sanity check for the combinatorial test setup that follows
+    it 'expect precedence list to not be empty' do
+      expect(Datadog::Core::Configuration::Option::Precedence::LIST).to_not be_empty
+    end
+
+    # Test all combinations of precedences to seed the Option object with all possible values set.
+    # For each combination, try to `unset` on every precedence.
+    #
+    # For example, if we have 2 precedences, `default` and `rc`,
+    # for an existing Option:
+    #
+    # | With these precedences set | `#unset` precedence | Assert that     |
+    # |----------------------------|---------------------|-----------------|
+    # | (empty)                    | rc                  | no change       |
+    # | (empty)                    | default             | no change       |
+    # | rc                         | rc                  | Option is reset |
+    # | rc                         | default             | no change       |
+    # | default                    | rc                  | no change       |
+    # | default                    | default             | Option is reset |
+    # | rc, default                | rc                  | default         |
+    # | rc, default                | default             | rc              |
+    {
+      no_precedence: [],
+      remote_configuration: [Datadog::Core::Configuration::Option::Precedence::REMOTE_CONFIGURATION],
+      programmatic: [Datadog::Core::Configuration::Option::Precedence::PROGRAMMATIC],
+      default: [Datadog::Core::Configuration::Option::Precedence::DEFAULT],
+      remote_and_programmatic: [
+        Datadog::Core::Configuration::Option::Precedence::REMOTE_CONFIGURATION,
+        Datadog::Core::Configuration::Option::Precedence::PROGRAMMATIC
+      ],
+      remote_and_default: [
+        Datadog::Core::Configuration::Option::Precedence::REMOTE_CONFIGURATION,
+        Datadog::Core::Configuration::Option::Precedence::DEFAULT
+      ],
+      programmatic_and_default: [
+        Datadog::Core::Configuration::Option::Precedence::PROGRAMMATIC,
+        Datadog::Core::Configuration::Option::Precedence::DEFAULT
+      ],
+      all: [
+        Datadog::Core::Configuration::Option::Precedence::REMOTE_CONFIGURATION,
+        Datadog::Core::Configuration::Option::Precedence::PROGRAMMATIC,
+        Datadog::Core::Configuration::Option::Precedence::DEFAULT
+      ]
+    }.each do |name, precedences|
+      context "for #{name} set" do
+        before do
+          allow(context).to(receive(:instance_exec)) { |value, _| value }
+
+          # See this Option with many values set a different precedences.
+          precedences.each do |precedence|
+            # For convenience, the option value is set to the same object as the precedence.
+            value = precedence
+
+            @highest_value ||= value
+            option.set(value, precedence: precedence)
+          end
+        end
+
+        # Far all scenarios, try to remove each precedence and assert the correct behavior.
+        Datadog::Core::Configuration::Option::Precedence::LIST.each do |precedence|
+          context "unsetting '#{precedence[1]}'" do
+            subject!(:unset) { option.unset(precedence) }
+            let(:precedence) { precedence }
+            let(:get) { option.get }
+
+            if precedences.empty?
+              context 'when no value is set' do
+                it 'resets the option' do
+                  expect(get).to eq(default)
+                  expect(option.send(:precedence_set)).to eq(Datadog::Core::Configuration::Option::Precedence::DEFAULT)
+                end
+              end
+            elsif precedence < precedences[0]
+              context 'when a value with lower precedence is unset' do
+                it 'does not modify the option value' do
+                  expect(get).to eq(@highest_value)
+                  expect(option.send(:precedence_set)).to eq(precedences[0])
+                end
+              end
+            elsif precedence == precedences[0]
+              context 'the highest precedence value is unset' do
+                if precedences.size == 1
+                  context 'removing the only value set' do
+                    it 'resets the option' do
+                      expect(get).to eq(default)
+                      expect(option.send(:precedence_set)).to eq(Datadog::Core::Configuration::Option::Precedence::DEFAULT)
+                    end
+                  end
+                else
+                  it 'falls back to lower precedence value' do
+                    expect(get).to eq(precedences[1])
+                    expect(option.send(:precedence_set)).to eq(precedences[1])
+                  end
+                end
+              end
+            elsif precedence > precedences[0]
+              context 'when a nonexistent value with higher precedence is unset' do
+                it 'does not modify the option value' do
+                  expect(get).to eq(@highest_value)
+                  expect(option.send(:precedence_set)).to eq(precedences[0])
+                end
+              end
+            end
+          end
+        end
+      end
+    end
+
+    context 'with a custom setter' do
+      let(:setter) { ->(value, _) { value + '+setter' } }
+
+      it 'invokes the setter only once when restoring a value' do
+        option.set('prog', precedence: Datadog::Core::Configuration::Option::Precedence::PROGRAMMATIC)
+        option.set('default', precedence: Datadog::Core::Configuration::Option::Precedence::DEFAULT)
+
+        option.unset(Datadog::Core::Configuration::Option::Precedence::PROGRAMMATIC)
+
+        expect(option.get).to eq('default+setter')
+      end
+    end
+  end
+
   describe '#get' do
     subject(:get) { option.get }
 
@@ -604,6 +731,7 @@
 
     context 'when deprecated_env is defined' do
       before do
+        allow(Datadog.logger).to receive(:warn) # For deprecation warnings
         allow(context).to receive(:instance_exec) do |*args|
           args[0]
         end
@@ -646,6 +774,7 @@
 
     context 'when env and deprecated_env are defined' do
       before do
+        allow(Datadog.logger).to receive(:warn) # For deprecation warnings
         allow(context).to receive(:instance_exec) do |*args|
           args[0]
         end

spec/datadog/core/configuration/options_spec.rb

@@ -138,6 +138,64 @@
         end
       end
 
+      describe '#unset_option' do
+        subject(:unset_option) { options_object.unset_option(name) }
+
+        let(:name) { :foo }
+
+        context 'when the option is defined' do
+          before { options_class.send(:option, name) { |o| o.default :test_default } }
+
+          context 'and value is not set' do
+            it 'does not change default value' do
+              expect { unset_option }.to_not change { options_object.send(name) }.from(:test_default)
+            end
+          end
+
+          context 'and value is set' do
+            before do
+              options_object.set_option(
+                name,
+                :new_value,
+                precedence: Datadog::Core::Configuration::Option::Precedence::PROGRAMMATIC
+              )
+            end
+
+            it 'defaults to PROGRAMMATIC precedence' do
+              unset_option
+              expect(options_object.get_option(name)).to eq(:test_default)
+            end
+
+            context 'with precedence' do
+              subject(:unset_option) { options_object.unset_option(name, precedence: precedence) }
+              let(:precedence) { Datadog::Core::Configuration::Option::Precedence::REMOTE_CONFIGURATION }
+
+              it 'removes the option with matching precedence' do
+                options_object.set_option(
+                  name,
+                  :should_stay,
+                  precedence: Datadog::Core::Configuration::Option::Precedence::PROGRAMMATIC
+                )
+
+                options_object.set_option(
+                  name,
+                  :go_away,
+                  precedence: Datadog::Core::Configuration::Option::Precedence::REMOTE_CONFIGURATION
+                )
+
+                unset_option
+
+                expect(options_object.get_option(name)).to eq(:should_stay)
+              end
+            end
+          end
+        end
+
+        context 'when the option is not defined' do
+          it { expect { unset_option }.to raise_error(described_class::InvalidOptionError) }
+        end
+      end
+
       describe '#get_option' do
         subject(:get_option) { options_object.get_option(name) }