diff --git a/app/models/host.rb b/app/models/host.rb index ec79162eac2..eb1da4b6464 100644 --- a/app/models/host.rb +++ b/app/models/host.rb @@ -1432,11 +1432,10 @@ def self.display_name(number = 1) end def verbose_supports?(feature, description = nil) - supports?(feature).tap do |value| - unless value - description ||= feature.to_s.humanize(:capitalize => false) - _log.warn("Cannot #{description} because <#{unsupported_reason(feature)}>") - end + if (reason = unsupported_reason(feature)) + description ||= feature.to_s.humanize(:capitalize => false) + _log.warn("Cannot #{description} because <#{reason}>") end + !reason end end diff --git a/app/models/mixins/supports_feature_mixin.rb b/app/models/mixins/supports_feature_mixin.rb index e68a2be1fe6..b4f06de1c21 100644 --- a/app/models/mixins/supports_feature_mixin.rb +++ b/app/models/mixins/supports_feature_mixin.rb @@ -12,21 +12,17 @@ # # To make a feature conditionally supported, pass a block to the +supports+ method. # The block is evaluated in the context of the instance. -# If you call the private method +unsupported_reason_add+ with the feature -# and a reason, then the feature will be unsupported and the reason will be +# If a feature is not supported, return a string for the reason. A nil means it is supported +# Alternatively, calling the private method +unsupported_reason_add+ with the feature +# and a reason, marks the feature as unsupported, and the reason will be # accessible through # # instance.unsupported_reason(:feature) # -# The above allows you to call +supports_feature?+ or +supports?(feature) :methods -# on the Class and Instance -# -# Post.supports_publish? # => true # Post.supports?(:publish) # => true -# Post.new.supports_publish? # => true -# Post.supports_fake? # => false -# Post.supports_archive? # => true -# Post.new(featured: true).supports_archive? # => false +# Post.new.supports?(:publish) # => true +# Post.supports?(:archive) # => true +# Post.new(featured: true).supports?(:archive) # => false # # To get a reason why a feature is unsupported use the +unsupported_reason+ method # @@ -51,18 +47,16 @@ module SupportsFeatureMixin extend ActiveSupport::Concern - COMMON_FEATURES = %i[create delete destroy refresh_ems update].freeze - # Whenever this mixin is included we define all features as unsupported by default. # This way we can query for every feature included do private_class_method :unsupported private_class_method :unsupported_reason_add - private_class_method :define_supports_feature_methods + class_attribute :supports_features, :instance_writer => false, :default => {} end - def self.reason_or_default(reason) - (reason.presence || _("Feature not available/supported")) + def self.default_supports_reason + _("Feature not available/supported") end # query instance for the reason why the feature is unsupported @@ -74,22 +68,16 @@ def unsupported_reason(feature) # query the instance if the feature is supported or not def supports?(feature) - method_name = "supports_#{feature}?" - if respond_to?(method_name) - public_send(method_name) - else - unsupported_reason_add(feature) - false - end + self.class.check_supports(feature.to_sym, :instance => self) end private # used inside a +supports+ block to add a reason why the feature is not supported # just adding a reason will make the feature unsupported - def unsupported_reason_add(feature, reason = nil) + def unsupported_reason_add(feature, reason) feature = feature.to_sym - unsupported[feature] = SupportsFeatureMixin.reason_or_default(reason) + unsupported[feature] = reason end def unsupported @@ -99,24 +87,43 @@ def unsupported class_methods do # This is the DSL used a class level to define what is supported def supports(feature, &block) - define_supports_feature_methods(feature, &block) + self.supports_features = supports_features.merge(feature.to_sym => block || true) end # supports_not does not take a block, because its never supported # and not conditionally supported def supports_not(feature, reason: nil) - define_supports_feature_methods(feature, :is_supported => false, :reason => reason) + self.supports_features = supports_features.merge(feature.to_sym => reason.presence || false) end # query the class if the feature is supported or not def supports?(feature) - method_name = "supports_#{feature}?" - if respond_to?(method_name) - public_send(method_name) - else - unsupported_reason_add(feature) - false + check_supports(feature.to_sym, :instance => self) + end + + def check_supports(feature, instance:) + instance.send(:unsupported).delete(feature) + + # undeclared features are not supported + value = supports_features[feature.to_sym] + + if value.respond_to?(:call) + begin + # for class level supports, blocks are not evaluated and assumed to be true + result = instance.instance_eval(&value) unless instance.kind_of?(Class) + # if no errors yet but result was an error message + # then add the error + if !instance.send(:unsupported).key?(feature) && result.kind_of?(String) + instance.send(:unsupported_reason_add, feature, result) + end + rescue => e + _log.log_backtrace(e) + instance.send(:unsupported_reason_add, feature, "Internal Error: #{e.message}") + end + elsif value != true + instance.send(:unsupported_reason_add, feature, value || SupportsFeatureMixin.default_supports_reason) end + !instance.send(:unsupported).key?(feature) end # all subclasses that are considered for supporting features @@ -166,46 +173,8 @@ def unsupported end # use this for making a class not support a feature - def unsupported_reason_add(feature, reason = nil) - feature = feature.to_sym - unsupported[feature] = SupportsFeatureMixin.reason_or_default(reason) - end - - def define_supports_feature_methods(feature, is_supported: true, reason: nil, &block) - method_name = "supports_#{feature}?" - feature = feature.to_sym - - # silence potential redefinition warnings - silence_warnings do - # defines the method on the instance - define_method(method_name) do - unsupported.delete(feature) - if block - begin - result = instance_eval(&block) - # if no errors yet but result was an error message - # then add the error - if !unsupported.key?(feature) && result.kind_of?(String) - unsupported_reason_add(feature, result) - end - rescue => e - _log.log_backtrace(e) - unsupported_reason_add(feature, "Internal Error: #{e.message}") - end - else - unsupported_reason_add(feature, reason) unless is_supported - end - !unsupported.key?(feature) - end - - # defines the method on the class - define_singleton_method(method_name) do - unsupported.delete(feature) - # TODO: durandom - make reason evaluate in class context, to e.g. include the name of a subclass (.to_proc?) - unsupported_reason_add(feature, reason) unless is_supported - !unsupported.key?(feature) - end - end + def unsupported_reason_add(feature, reason) + unsupported[feature.to_sym] = reason end end end diff --git a/app/models/vm/operations.rb b/app/models/vm/operations.rb index b2affa33280..829d29e04f7 100644 --- a/app/models/vm/operations.rb +++ b/app/models/vm/operations.rb @@ -26,6 +26,7 @@ module Vm::Operations supports :launch_vmrc_console do begin validate_remote_console_vmrc_support + nil rescue => err _('VM VMRC Console error: %{error}') % {:error => err} end @@ -33,6 +34,7 @@ module Vm::Operations supports :launch_native_console do validate_native_console_support + nil rescue StandardError => err _('VM NATIVE Console error: %{error}') % {:error => err} end diff --git a/spec/models/mixins/supports_feature_mixin_spec.rb b/spec/models/mixins/supports_feature_mixin_spec.rb index 723a8a41af8..8c2afb4745a 100644 --- a/spec/models/mixins/supports_feature_mixin_spec.rb +++ b/spec/models/mixins/supports_feature_mixin_spec.rb @@ -33,22 +33,6 @@ def initialize(values = {}) let(:test_inst) { test_class.new } - describe "#supports_feature?" do - it "defines supports on the instance" do - expect(test_inst.supports_std_accept?).to be_truthy - expect(test_inst.supports_module_accept?).to be_truthy - expect(test_inst.supports_std_denial?).to be_falsey - end - end - - describe ".supports_feature?" do - it "defines supports on the class" do - expect(test_class.supports_std_accept?).to be_truthy - expect(test_class.supports_module_accept?).to be_truthy - expect(test_class.supports_std_denial?).to be_falsey - end - end - describe ".supports?" do it "handles base supports" do expect(test_class.supports?(:std_accept)).to be_truthy @@ -231,11 +215,37 @@ def initialize(values = {}) end it "gives reason when implicit dynamic attrs" do - test_class.supports(:implicit_feature) { "dynamically unsupported" unless attr1 } + test_class.supports(:implicit_feature) { "dynamically unsupported" unless attr1 } test_inst = test_class.new(:attr1 => false) expect(test_inst.unsupported_reason(:implicit_feature)).to eq("dynamically unsupported") end + + it "gives reason when chained to a denail with a default reason" do + test_class.supports_not :denial_no_reason + test_class.supports(:denial_chained) { unsupported_reason(:denial_no_reason) } + + expect(test_inst.unsupported_reason(:denial_chained)).to eq("Feature not available/supported") + end + + it "gives reason when chained to a denail with a default reason (checking supported)" do + test_class.supports_not :denial_no_reason + test_class.supports(:denial_chained) { unsupported_reason(:denial_no_reason) unless supports?(:denial_no_reason) } + + expect(test_inst.unsupported_reason(:denial_chained)).to eq("Feature not available/supported") + end + + it "gives no reason when chained to an attribute with success" do + test_class.supports(:std_chained) { unsupported_reason(:std_accept) } + + expect(test_inst.unsupported_reason(:std_chained)).to eq(nil) + end + + it "gives no reason when chained to an attribute with success (checking supported)" do + test_class.supports(:std_chained) { unsupported_reason(:std_accept) unless supports?(:std_accept) } + + expect(test_inst.unsupported_reason(:std_chained)).to eq(nil) + end end describe ".subclasses_supporting" do diff --git a/spec/support/supports_helper.rb b/spec/support/supports_helper.rb index 37a524d0e62..b399840766d 100644 --- a/spec/support/supports_helper.rb +++ b/spec/support/supports_helper.rb @@ -25,7 +25,7 @@ def stub_supports_not(model, feature = :update, reason = nil) stub_supports(model, feature, :supported => false) - reason ||= SupportsFeatureMixin.reason_or_default(reason) + reason ||= SupportsFeatureMixin.default_supports_reason receive_reason = receive(:unsupported_reason).with(feature).and_return(reason) allow(model).to(receive_reason) allow_any_instance_of(model).to(receive_reason)