Use AR attribute APIpl-service-ar-attributes

3 files changed, 24 insertions, 51 deletions

diff --git a/app/models/project_services/jira_service.rb b/app/models/project_services/jira_service.rb
index d08fcd8954d..ce37f6d5151 100644
--- a/app/models/project_services/jira_service.rb
+++ b/app/models/project_services/jira_service.rb
@@ -21,7 +21,7 @@ class JiraService < IssueTrackerService
 
   before_update :reset_password
 
-  alias_method :project_url, :url
+  alias_attribute :project_url, :url
 
   # When these are false GitLab does not create cross reference
   # comments on Jira except when an issue gets transitioned.
@@ -103,10 +103,8 @@ class JiraService < IssueTrackerService
     "#{url}/secure/CreateIssue.jspa"
   end
 
-  alias_method :original_url, :url
-
   def url
-    original_url&.chomp('/')
+    super&.chomp('/')
   end
 
   def execute(push)
diff --git a/app/models/service.rb b/app/models/service.rb
index f6d8fb1fb46..4e1498d3fe0 100644
--- a/app/models/service.rb
+++ b/app/models/service.rb
@@ -23,7 +23,9 @@ class Service < ApplicationRecord
   default_value_for :pipeline_events, true
   default_value_for :wiki_page_events, true
 
-  after_initialize :initialize_properties
+  after_initialize do |service|
+    service.assign_attributes(service.properties)
+  end
 
   after_commit :reset_updated_properties
   after_commit :cache_project_has_external_issue_tracker
@@ -77,10 +79,6 @@ class Service < ApplicationRecord
     read_attribute(:category).to_sym
   end
 
-  def initialize_properties
-    self.properties = {} if properties.nil?
-  end
-
   def title
     # implement inside child
   end
@@ -174,31 +172,7 @@ class Service < ApplicationRecord
   # Also keep track of updated properties in a similar way as ActiveModel::Dirty
   def self.prop_accessor(*args)
     args.each do |arg|
-      class_eval %{
-        unless method_defined?(arg)
-          def #{arg}
-            properties['#{arg}']
-          end
-        end
-
-        def #{arg}=(value)
-          self.properties ||= {}
-          updated_properties['#{arg}'] = #{arg} unless #{arg}_changed?
-          self.properties['#{arg}'] = value
-        end
-
-        def #{arg}_changed?
-          #{arg}_touched? && #{arg} != #{arg}_was
-        end
-
-        def #{arg}_touched?
-          updated_properties.include?('#{arg}')
-        end
-
-        def #{arg}_was
-          updated_properties['#{arg}']
-        end
-      }
+      define_prop_accessor(arg, ActiveModel::Type::Value.new)
     end
   end
 
@@ -206,16 +180,25 @@ class Service < ApplicationRecord
   # for each serialized property.
   # Also keep track of updated properties in a similar way as ActiveModel::Dirty
   def self.boolean_accessor(*args)
-    self.prop_accessor(*args)
-
     args.each do |arg|
-      class_eval %{
-        def #{arg}?
-          # '!!' is used because nil or empty string is converted to nil
-          !!ActiveRecord::Type::Boolean.new.cast(#{arg})
-        end
-      }
+      define_prop_accessor(arg, :boolean, default: false)
+      alias_attribute "#{arg}?", arg
+    end
+  end
+
+  def self.define_prop_accessor(arg, type, **options)
+    define_method("#{arg}=") do |value|
+      self.properties ||= {}
+      updated_properties[arg.to_s] = value
+      properties[arg.to_s] = value
+      super(value)
     end
+
+    define_method("#{arg}_touched?") do
+      updated_properties.include?(arg.to_s)
+    end
+
+    attribute(arg, type, **options)
   end
 
   # Returns a hash of the properties that have been assigned a new value since last save,
diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb
index 0797b9a9d83..b5b17ae194f 100644
--- a/spec/models/service_spec.rb
+++ b/spec/models/service_spec.rb
@@ -339,15 +339,7 @@ describe Service do
     end
 
     let(:service) do
-      fake_service.new(properties: [
-        { token: 'token-value' },
-        { api_token: 'api_token-value' },
-        { key: 'key-value' },
-        { api_key: 'api_key-value' },
-        { password: 'password-value' },
-        { password_field: 'password_field-value' },
-        { safe_field: 'safe_field-value' }
-      ])
+      fake_service.new
     end
 
     it 'filters out sensitive fields' do