diff options
author | Grzegorz Bizon <grzegorz@gitlab.com> | 2018-06-05 10:39:59 +0300 |
---|---|---|
committer | Grzegorz Bizon <grzegorz@gitlab.com> | 2018-06-05 10:39:59 +0300 |
commit | 809a50fcbf5ecfbf5ec02671f1ca2710a96f58d3 (patch) | |
tree | 0b77ad7a705f6477b03d0f83634a73e3cf36f7d1 /app/models/concerns | |
parent | 114c26ccf0f10788271c6108774e72809a7f93e1 (diff) | |
parent | 04236363bce399fbde36f396fdcf51d61735e1b0 (diff) |
Merge branch 'master' into 'backstage/gb/use-persisted-stages-to-improve-pipelines-table'
Conflicts:
app/models/ci/pipeline.rb
Diffstat (limited to 'app/models/concerns')
-rw-r--r-- | app/models/concerns/atomic_internal_id.rb | 16 | ||||
-rw-r--r-- | app/models/concerns/batch_destroy_dependent_associations.rb | 28 | ||||
-rw-r--r-- | app/models/concerns/cacheable_attributes.rb | 38 | ||||
-rw-r--r-- | app/models/concerns/has_variable.rb | 2 | ||||
-rw-r--r-- | app/models/concerns/iid_routes.rb | 9 | ||||
-rw-r--r-- | app/models/concerns/issuable.rb | 2 | ||||
-rw-r--r-- | app/models/concerns/project_features_compatibility.rb | 2 | ||||
-rw-r--r-- | app/models/concerns/reactive_caching.rb | 1 | ||||
-rw-r--r-- | app/models/concerns/time_trackable.rb | 6 |
9 files changed, 77 insertions, 27 deletions
diff --git a/app/models/concerns/atomic_internal_id.rb b/app/models/concerns/atomic_internal_id.rb index 22f516a172f..164c704260e 100644 --- a/app/models/concerns/atomic_internal_id.rb +++ b/app/models/concerns/atomic_internal_id.rb @@ -25,9 +25,13 @@ module AtomicInternalId extend ActiveSupport::Concern module ClassMethods - def has_internal_id(column, scope:, init:) # rubocop:disable Naming/PredicateName - before_validation(on: :create) do + def has_internal_id(column, scope:, init:, presence: true) # rubocop:disable Naming/PredicateName + before_validation :"ensure_#{scope}_#{column}!", on: :create + validates column, presence: presence + + define_method("ensure_#{scope}_#{column}!") do scope_value = association(scope).reader + if read_attribute(column).blank? && scope_value scope_attrs = { scope_value.class.table_name.singularize.to_sym => scope_value } usage = self.class.table_name.to_sym @@ -35,13 +39,9 @@ module AtomicInternalId new_iid = InternalId.generate_next(self, scope_attrs, usage, init) write_attribute(column, new_iid) end - end - validates column, presence: true, numericality: true + read_attribute(column) + end end end - - def to_param - iid.to_s - end end diff --git a/app/models/concerns/batch_destroy_dependent_associations.rb b/app/models/concerns/batch_destroy_dependent_associations.rb new file mode 100644 index 00000000000..353ee2e73d0 --- /dev/null +++ b/app/models/concerns/batch_destroy_dependent_associations.rb @@ -0,0 +1,28 @@ +# Provides a way to work around Rails issue where dependent objects are all +# loaded into memory before destroyed: https://github.com/rails/rails/issues/22510. +# +# This concern allows an ActiveRecord module to destroy all its dependent +# associations in batches. The idea is borrowed from https://github.com/thisismydesign/batch_dependent_associations. +# +# The differences here with that gem: +# +# 1. We allow excluding certain associations. +# 2. We don't need to support delete_all since we can use the EachBatch concern. +module BatchDestroyDependentAssociations + extend ActiveSupport::Concern + + DEPENDENT_ASSOCIATIONS_BATCH_SIZE = 1000 + + def dependent_associations_to_destroy + self.class.reflect_on_all_associations(:has_many).select { |assoc| assoc.options[:dependent] == :destroy } + end + + def destroy_dependent_associations_in_batches(exclude: []) + dependent_associations_to_destroy.each do |association| + next if exclude.include?(association.name) + + # rubocop:disable GitlabSecurity/PublicSend + public_send(association.name).find_each(batch_size: DEPENDENT_ASSOCIATIONS_BATCH_SIZE, &:destroy) + end + end +end diff --git a/app/models/concerns/cacheable_attributes.rb b/app/models/concerns/cacheable_attributes.rb index b32459fdabf..d58d7165969 100644 --- a/app/models/concerns/cacheable_attributes.rb +++ b/app/models/concerns/cacheable_attributes.rb @@ -6,15 +6,16 @@ module CacheableAttributes end class_methods do + def cache_key + "#{name}:#{Gitlab::VERSION}:#{Gitlab.migrations_hash}:#{Rails.version}".freeze + end + # Can be overriden def current_without_cache last end - def cache_key - "#{name}:#{Gitlab::VERSION}:#{Gitlab.migrations_hash}:json".freeze - end - + # Can be overriden def defaults {} end @@ -24,10 +25,18 @@ module CacheableAttributes end def cached - json_attributes = Rails.cache.read(cache_key) - return nil unless json_attributes.present? + if RequestStore.active? + RequestStore[:"#{name}_cached_attributes"] ||= retrieve_from_cache + else + retrieve_from_cache + end + end + + def retrieve_from_cache + record = Rails.cache.read(cache_key) + ensure_cache_setup if record.present? - build_from_defaults(JSON.parse(json_attributes)) + record end def current @@ -35,7 +44,12 @@ module CacheableAttributes return cached_record if cached_record.present? current_without_cache.tap { |current_record| current_record&.cache! } - rescue + rescue => e + if Rails.env.production? + Rails.logger.warn("Cached record for #{name} couldn't be loaded, falling back to uncached record: #{e}") + else + raise e + end # Fall back to an uncached value if there are any problems (e.g. Redis down) current_without_cache end @@ -46,9 +60,15 @@ module CacheableAttributes # Gracefully handle when Redis is not available. For example, # omnibus may fail here during gitlab:assets:compile. end + + def ensure_cache_setup + # This is a workaround for a Rails bug that causes attribute methods not + # to be loaded when read from cache: https://github.com/rails/rails/issues/27348 + define_attribute_methods + end end def cache! - Rails.cache.write(self.class.cache_key, attributes.to_json) + Rails.cache.write(self.class.cache_key, self) end end diff --git a/app/models/concerns/has_variable.rb b/app/models/concerns/has_variable.rb index 8a241e4374a..c8e20c0ab81 100644 --- a/app/models/concerns/has_variable.rb +++ b/app/models/concerns/has_variable.rb @@ -13,7 +13,7 @@ module HasVariable attr_encrypted :value, mode: :per_attribute_iv_and_salt, insecure_mode: true, - key: Gitlab::Application.secrets.db_key_base, + key: Settings.attr_encrypted_db_key_base, algorithm: 'aes-256-cbc' def key=(new_key) diff --git a/app/models/concerns/iid_routes.rb b/app/models/concerns/iid_routes.rb new file mode 100644 index 00000000000..246748cf52c --- /dev/null +++ b/app/models/concerns/iid_routes.rb @@ -0,0 +1,9 @@ +module IidRoutes + ## + # This automagically enforces all related routes to use `iid` instead of `id` + # If you want to use `iid` for some routes and `id` for other routes, this module should not to be included, + # instead you should define `iid` or `id` explictly at each route generators. e.g. pipeline_path(project.id, pipeline.iid) + def to_param + iid.to_s + end +end diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index b45395343cc..44150b37708 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -97,8 +97,6 @@ module Issuable strip_attributes :title - after_save :ensure_metrics, unless: :imported? - # We want to use optimistic lock for cases when only title or description are involved # http://api.rubyonrails.org/classes/ActiveRecord/Locking/Optimistic.html def locking_enabled? diff --git a/app/models/concerns/project_features_compatibility.rb b/app/models/concerns/project_features_compatibility.rb index b3fec99c816..1f7d78a2efe 100644 --- a/app/models/concerns/project_features_compatibility.rb +++ b/app/models/concerns/project_features_compatibility.rb @@ -1,4 +1,4 @@ -# Makes api V3 compatible with old project features permissions methods +# Makes api V4 compatible with old project features permissions methods # # After migrating issues_enabled merge_requests_enabled builds_enabled snippets_enabled and wiki_enabled # fields to a new table "project_features", support for the old fields is still needed in the API. diff --git a/app/models/concerns/reactive_caching.rb b/app/models/concerns/reactive_caching.rb index eef9caf1c8e..be0a5b49012 100644 --- a/app/models/concerns/reactive_caching.rb +++ b/app/models/concerns/reactive_caching.rb @@ -74,6 +74,7 @@ module ReactiveCaching def clear_reactive_cache!(*args) Rails.cache.delete(full_reactive_cache_key(*args)) + Rails.cache.delete(alive_reactive_cache_key(*args)) end def exclusively_update_reactive_cache!(*args) diff --git a/app/models/concerns/time_trackable.rb b/app/models/concerns/time_trackable.rb index 1caf47072bc..0fc321c52bc 100644 --- a/app/models/concerns/time_trackable.rb +++ b/app/models/concerns/time_trackable.rb @@ -30,8 +30,6 @@ module TimeTrackable return if @time_spent == 0 - touch if touchable? - if @time_spent == :reset reset_spent_time else @@ -59,10 +57,6 @@ module TimeTrackable private - def touchable? - valid? && persisted? - end - def reset_spent_time timelogs.new(time_spent: total_time_spent * -1, user: @time_spent_user) # rubocop:disable Gitlab/ModuleWithInstanceVariables end |