From a17dc23b3890a3b8c73dd7f0b689ce966ac40f1f Mon Sep 17 00:00:00 2001
From: Keenan Brock <keenan@thebrocks.net>
Date: Wed, 29 Nov 2017 16:24:35 -0500
Subject: [PATCH] Handle deprecated classes in arel

Attribute deprecation is based upon aliases and virtual attributes.
Ensure both concepts are working correctly together.

Bug: The virtual attribute was generating bogus arel, when it should
have been deferring to the attribute_alias mechanism built into rails

So the new code prioritizes attribute_alias over virtual_attribute

https://bugzilla.redhat.com/show_bug.cgi?id=1507047
---
 lib/extensions/ar_virtual.rb                 |  2 +-
 spec/lib/extensions/ar_virtual_spec.rb       | 51 ++++++++++++++++++--
 spec/models/mixins/deprecation_mixin_spec.rb |  8 +++
 3 files changed, 57 insertions(+), 4 deletions(-)
 create mode 100644 spec/models/mixins/deprecation_mixin_spec.rb

diff --git a/lib/extensions/ar_virtual.rb b/lib/extensions/ar_virtual.rb
index 80c25a4669a8..416720a0cd2b 100644
--- a/lib/extensions/ar_virtual.rb
+++ b/lib/extensions/ar_virtual.rb
@@ -31,7 +31,7 @@ module VirtualArel
   module ClassMethods
     def arel_attribute(column_name, arel_table = self.arel_table)
       load_schema
-      if virtual_attribute?(column_name)
+      if virtual_attribute?(column_name) && !attribute_alias?(column_name)
         col = _virtual_arel[column_name.to_s]
         col.call(arel_table) if col
       else
diff --git a/spec/lib/extensions/ar_virtual_spec.rb b/spec/lib/extensions/ar_virtual_spec.rb
index 5f0a46567da5..f9e3013549f0 100644
--- a/spec/lib/extensions/ar_virtual_spec.rb
+++ b/spec/lib/extensions/ar_virtual_spec.rb
@@ -103,7 +103,9 @@ def self.connection
     end
 
     context ".virtual_columns=" do
-      it "" do
+      it "can have multiple virtual columns" do
+        TestClass.virtual_column :existing_vcol, :type => :string
+
         {
           :vcol1  => {:type => :string},
           "vcol2" => {:type => :string},
@@ -111,10 +113,12 @@ def self.connection
           TestClass.virtual_column name, options
         end
 
-        expect(TestClass.virtual_attribute_names).to match_array(["vcol1", "vcol2"])
+        expect(TestClass.virtual_attribute_names).to match_array(%w(existing_vcol vcol1 vcol2))
       end
+    end
 
-      it "with existing virtual columns" do
+    context ".virtual_attribute_names" do
+      it "has virtual attributes" do
         TestClass.virtual_column :existing_vcol, :type => :string
 
         {
@@ -126,6 +130,20 @@ def self.connection
 
         expect(TestClass.virtual_attribute_names).to match_array(["existing_vcol", "vcol1", "vcol2"])
       end
+
+      it "does not have aliases" do
+        TestClass.virtual_attribute :existing_vcol, :string
+        TestClass.alias_attribute :col2, :col1
+        expect(TestClass.virtual_attribute_names).to match_array(["existing_vcol"])
+      end
+
+      it "supports virtual_column aliases" do
+        TestClass.virtual_attribute :existing_vcol, :string
+        TestClass.alias_attribute :col2, :col1
+        TestClass.virtual_attribute :col2, :integer
+
+        expect(TestClass.virtual_attribute_names).to match_array(%w(existing_vcol col2))
+      end
     end
 
     shared_examples_for "TestClass with virtual columns" do
@@ -148,6 +166,12 @@ def self.connection
             it("as string") { expect(TestClass.virtual_attribute?("col1")).not_to be_truthy }
             it("as symbol") { expect(TestClass.virtual_attribute?(:col1)).not_to  be_truthy }
           end
+
+          context "with alias" do
+            before { TestClass.alias_attribute :col2, :col1 }
+            it("as string") { expect(TestClass.virtual_attribute?("col1")).not_to be_truthy }
+            it("as symbol") { expect(TestClass.virtual_attribute?(:col1)).not_to  be_truthy }
+          end
         end
 
         it ".remove_virtual_fields" do
@@ -661,6 +685,27 @@ def col2
       end
     end
 
+    describe ".arel_attribute" do
+      it "supports aliases" do
+        TestClass.alias_attribute :col2, :col1
+
+        arel_attr = TestClass.arel_attribute(:col2)
+        expect(arel_attr).to_not be_nil
+        expect(arel_attr.name).to eq("col1") # typically this is a symbol. not perfect but it works
+      end
+
+      # NOTE: should not need to add a virtual attribute to an alias
+      # TODO: change code for reports and automate to expose aliases like it does with attributes/virtual attributes.
+      it "supports aliases marked as a virtual_attribute" do
+        TestClass.alias_attribute :col2, :col1
+        TestClass.virtual_attribute :col2, :integer
+
+        arel_attr = TestClass.arel_attribute(:col2)
+        expect(arel_attr).to_not be_nil
+        expect(arel_attr.name).to eq("col1") # typically this is a symbol. not perfect but it works
+      end
+    end
+
     describe "#select" do
       it "supports virtual attributes" do
         class TestClass
diff --git a/spec/models/mixins/deprecation_mixin_spec.rb b/spec/models/mixins/deprecation_mixin_spec.rb
new file mode 100644
index 000000000000..07dd30c1fe08
--- /dev/null
+++ b/spec/models/mixins/deprecation_mixin_spec.rb
@@ -0,0 +1,8 @@
+describe DeprecationMixin do
+  # Host.deprecate_attribute :address, :hostname
+  it "works with arel attributes" do
+    expect(Host.attribute_supported_by_sql?(:address)).to eq(true)
+    expect(Host.arel_attribute(:address)).to_not be_nil
+    expect(Host.arel_attribute(:address).name).to eq("hostname") # typically this is a symbol. not perfect but it works
+  end
+end