gitlab-org--gitlab-foss/app/models/project_import_data.rb

38 lines
1 KiB
Ruby
Raw Normal View History

# frozen_string_literal: true
require 'carrierwave/orm/activerecord'
class ProjectImportData < ActiveRecord::Base
belongs_to :project, inverse_of: :import_data
2016-04-11 05:13:51 -04:00
attr_encrypted :credentials,
key: Settings.attr_encrypted_db_key_base,
2016-04-11 05:13:51 -04:00
marshal: true,
encode: true,
mode: :per_attribute_iv_and_salt,
insecure_mode: true,
algorithm: 'aes-256-cbc'
serialize :data, JSON # rubocop:disable Cop/ActiveRecordSerialize
validates :project, presence: true
2016-04-01 06:04:41 -04:00
before_validation :symbolize_credentials
def symbolize_credentials
2016-04-11 05:13:51 -04:00
# bang doesn't work here - attr_encrypted makes it not to work
2016-04-06 04:36:30 -04:00
self.credentials = self.credentials.deep_symbolize_keys unless self.credentials.blank?
2016-04-01 06:04:41 -04:00
end
Refactor Project#create_or_update_import_data In https://gitlab.com/gitlab-org/release/framework/issues/28 we found that this method was changed a lot over the years: 43 times if our calculations were correct. Looking at the method, it had quite a few branches going on: def create_or_update_import_data(data: nil, credentials: nil) return if data.nil? && credentials.nil? project_import_data = import_data || build_import_data if data project_import_data.data ||= {} project_import_data.data = project_import_data.data.merge(data) end if credentials project_import_data.credentials ||= {} project_import_data.credentials = project_import_data.credentials.merge(credentials) end project_import_data end If we turn the || and ||= operators into regular if statements, we can see a bit more clearly that this method has quite a lot of branches in it: def create_or_update_import_data(data: nil, credentials: nil) if data.nil? && credentials.nil? return else project_import_data = if import_data import_data else build_import_data end if data if project_import_data.data # nothing else project_import_data.data = {} end project_import_data.data = project_import_data.data.merge(data) end if credentials if project_import_data.credentials # nothing else project_import_data.credentials = {} end project_import_data.credentials = project_import_data.credentials.merge(credentials) end project_import_data end end The number of if statements and branches here makes it easy to make mistakes. To resolve this, we refactor this code in such a way that we can get rid of all but the first `if data.nil? && credentials.nil?` statement. We can do this by simply sending `to_h` to `nil` in the right places, which removes the need for statements such as `if data`. Since this data gets written to a database, in ProjectImportData we do make sure to not write empty Hash values. This requires an `unless` (which is really a `if !`), but the resulting code is still very easy to read.
2018-12-10 12:09:33 -05:00
def merge_data(hash)
self.data = data.to_h.merge(hash) unless hash.empty?
end
def merge_credentials(hash)
self.credentials = credentials.to_h.merge(hash) unless hash.empty?
end
def clear_credentials
self.credentials = {}
end
end