From c6cc54d4805209a373e845d32771d83c209190c7 Mon Sep 17 00:00:00 2001
From: Keenan Brock <keenan@thebrocks.net>
Date: Fri, 2 Oct 2020 16:33:38 -0400
Subject: [PATCH] Hide deprecated and duplicate attributes

Some virtual_attributes are not needed in reporting nor the api.
Some deprecated attributes should not be advertised for customers to start using.

Now, the attributes are still accessible. So nothing will break. But
these attributes are no longer advertised on the public apis (i.e.: report builder and rest api).

This allows us to control which parts of our models are exposed.

This fixes https://github.com/ManageIQ/manageiq/issues/18130
This enables https://github.com/ManageIQ/manageiq/issues/20532
---
 app/models/ext_management_system.rb          |  3 +++
 app/models/mixins/deprecation_mixin.rb       | 19 +++++++++++++++++++
 lib/miq_expression.rb                        |  2 +-
 spec/models/mixins/deprecation_mixin_spec.rb | 18 ++++++++++++++++++
 4 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/app/models/ext_management_system.rb b/app/models/ext_management_system.rb
index a66d5358e748..ad76adda8548 100644
--- a/app/models/ext_management_system.rb
+++ b/app/models/ext_management_system.rb
@@ -3,6 +3,9 @@ class ExtManagementSystem < ApplicationRecord
   include SupportsFeatureMixin
   include ExternalUrlMixin
   include VerifyCredentialsMixin
+  include DeprecationMixin
+
+  hide_attribute "aggregate_memory" # better to use total_memory (coin toss - they're similar)
 
   def self.with_tenant(tenant_id)
     tenant = Tenant.find(tenant_id)
diff --git a/app/models/mixins/deprecation_mixin.rb b/app/models/mixins/deprecation_mixin.rb
index 4f1f254bf465..26faa273c216 100644
--- a/app/models/mixins/deprecation_mixin.rb
+++ b/app/models/mixins/deprecation_mixin.rb
@@ -1,7 +1,25 @@
 module DeprecationMixin
   extend ActiveSupport::Concern
 
+  included do
+    cattr_accessor :hidden_attribute_names
+    self.hidden_attribute_names = []
+  end
+
   module ClassMethods
+    # @param [String|Symbol] attribute name of attribute to be hidden from the api and reporting
+    # this attribute is accessible to all ruby methods. But it is not advertised.
+    # we do this when deprecating an attribute or when introducing an internal attribute
+    def hide_attribute(attribute)
+      hidden_attribute_names << attribute.to_s
+    end
+
+    # @return Array[String] attribute names that can be advertised in the api and reporting
+    # Other attributes are accessible, they are just no longer in our public api (or never were)
+    def visible_attribute_names
+      attribute_names - hidden_attribute_names
+    end
+
     private
 
     def deprecate_belongs_to(old_belongs_to, new_belongs_to)
@@ -18,6 +36,7 @@ def deprecate_belongs_to(old_belongs_to, new_belongs_to)
     def deprecate_attribute(old_attribute, new_attribute)
       deprecate_attribute_methods(old_attribute, new_attribute)
       virtual_attribute(old_attribute, -> { type_for_attribute(new_attribute.to_s) })
+      hide_attribute(old_attribute)
     end
 
     def deprecate_attribute_methods(old_attribute, new_attribute)
diff --git a/lib/miq_expression.rb b/lib/miq_expression.rb
index 02851f896267..164dd57ed45f 100644
--- a/lib/miq_expression.rb
+++ b/lib/miq_expression.rb
@@ -909,7 +909,7 @@ def self.build_relats(model, parent = {}, seen = [])
     parent[:class_path] ||= model.name
     parent[:assoc_path] ||= model.name
     parent[:root] ||= model.name
-    result = {:columns => model.attribute_names, :parent => parent}
+    result = {:columns => (model.try(:visible_attribute_names) || model.attribute_names), :parent => parent}
     result[:reflections] = {}
 
     model.reflections_with_virtual.each do |assoc, ref|
diff --git a/spec/models/mixins/deprecation_mixin_spec.rb b/spec/models/mixins/deprecation_mixin_spec.rb
index 7e059ef1c762..7c3984df9fa4 100644
--- a/spec/models/mixins/deprecation_mixin_spec.rb
+++ b/spec/models/mixins/deprecation_mixin_spec.rb
@@ -7,4 +7,22 @@
       expect(Host.arel_attribute(:address).name).to eq("hostname") # typically this is a symbol. not perfect but it works
     end
   end
+
+  context "visible_attribute_names" do
+    let(:klass) do
+      Class.new(ApplicationRecord) do
+        include DeprecationMixin
+        self.table_name = "vms"
+      end
+    end
+
+    it "shows regular attributes" do
+      expect(klass.visible_attribute_names).to include("name")
+    end
+
+    it "hides hidden attributes" do
+      klass.hide_attribute :name
+      expect(klass.visible_attribute_names).not_to include("name")
+    end
+  end
 end