diff options
Diffstat (limited to 'app/models/concerns')
| -rw-r--r-- | app/models/concerns/avatarable.rb | 5 | ||||
| -rw-r--r-- | app/models/concerns/fast_destroy_all.rb | 91 | ||||
| -rw-r--r-- | app/models/concerns/participable.rb | 4 | ||||
| -rw-r--r-- | app/models/concerns/protected_ref.rb | 2 | ||||
| -rw-r--r-- | app/models/concerns/routable.rb | 7 | ||||
| -rw-r--r-- | app/models/concerns/storage/legacy_namespace.rb | 30 |
6 files changed, 120 insertions, 19 deletions
diff --git a/app/models/concerns/avatarable.rb b/app/models/concerns/avatarable.rb index 7677891b9ce..13246a774e3 100644 --- a/app/models/concerns/avatarable.rb +++ b/app/models/concerns/avatarable.rb @@ -31,12 +31,13 @@ module Avatarable asset_host = ActionController::Base.asset_host use_asset_host = asset_host.present? + use_authentication = respond_to?(:public?) && !public? # Avatars for private and internal groups and projects require authentication to be viewed, # which means they can only be served by Rails, on the regular GitLab host. # If an asset host is configured, we need to return the fully qualified URL # instead of only the avatar path, so that Rails doesn't prefix it with the asset host. - if use_asset_host && respond_to?(:public?) && !public? + if use_asset_host && use_authentication use_asset_host = false only_path = false end @@ -49,6 +50,6 @@ module Avatarable url_base << gitlab_config.relative_url_root end - url_base + avatar.url + url_base + avatar.local_url end end diff --git a/app/models/concerns/fast_destroy_all.rb b/app/models/concerns/fast_destroy_all.rb new file mode 100644 index 00000000000..7ea042c6742 --- /dev/null +++ b/app/models/concerns/fast_destroy_all.rb @@ -0,0 +1,91 @@ +## +# This module is for replacing `dependent: :destroy` and `before_destroy` hooks. +# +# In general, `destroy_all` is inefficient because it calls each callback with `DELETE` queries i.e. O(n), whereas, +# `delete_all` is efficient as it deletes all rows with a single `DELETE` query. +# +# It's better to use `delete_all` as our best practice, however, +# if external data (e.g. ObjectStorage, FileStorage or Redis) are assosiated with database records, +# it is difficult to accomplish it. +# +# This module defines a format to use `delete_all` and delete associated external data. +# Here is an exmaple +# +# Situation +# - `Project` has many `Ci::BuildTraceChunk` through `Ci::Build` +# - `Ci::BuildTraceChunk` stores associated data in Redis, so it relies on `dependent: :destroy` and `before_destroy` for the deletion +# +# How to use +# - Define `use_fast_destroy :build_trace_chunks` in `Project` model. +# - Define `begin_fast_destroy` and `finalize_fast_destroy(params)` in `Ci::BuildTraceChunk` model. +# - Use `fast_destroy_all` instead of `destroy` and `destroy_all` +# - Remove `dependent: :destroy` and `before_destroy` as it's no longer need +# +# Expectation +# - When a project is `destroy`ed, the associated trace_chunks will be deleted by `delete_all`, +# and the associated data will be removed, too. +# - When `fast_destroy_all` is called, it also performns as same. +module FastDestroyAll + extend ActiveSupport::Concern + + ForbiddenActionError = Class.new(StandardError) + + included do + before_destroy do + raise ForbiddenActionError, '`destroy` and `destroy_all` are forbbiden. Please use `fast_destroy_all`' + end + end + + class_methods do + ## + # This method delete rows and associated external data efficiently + # + # This method can replace `destroy` and `destroy_all` without having `after_destroy` hook + def fast_destroy_all + params = begin_fast_destroy + + delete_all + + finalize_fast_destroy(params) + end + + ## + # This method returns identifiers to delete associated external data (e.g. file paths, redis keys) + # + # This method must be defined in fast destroyable model + def begin_fast_destroy + raise NotImplementedError + end + + ## + # This method deletes associated external data with the identifiers returned by `begin_fast_destroy` + # + # This method must be defined in fast destroyable model + def finalize_fast_destroy(params) + raise NotImplementedError + end + end + + module Helpers + extend ActiveSupport::Concern + + class_methods do + ## + # This method is to be defined on models which have fast destroyable models as children, + # and let us avoid to use `dependent: :destroy` hook + def use_fast_destroy(relation) + before_destroy(prepend: true) do + perform_fast_destroy(public_send(relation)) # rubocop:disable GitlabSecurity/PublicSend + end + end + end + + def perform_fast_destroy(subject) + params = subject.begin_fast_destroy + + run_after_commit do + subject.finalize_fast_destroy(params) + end + end + end +end diff --git a/app/models/concerns/participable.rb b/app/models/concerns/participable.rb index e48bc0be410..01b1ef9f82c 100644 --- a/app/models/concerns/participable.rb +++ b/app/models/concerns/participable.rb @@ -98,6 +98,10 @@ module Participable participants.merge(ext.users) + filter_by_ability(participants) + end + + def filter_by_ability(participants) case self when PersonalSnippet Ability.users_that_can_read_personal_snippet(participants.to_a, self) diff --git a/app/models/concerns/protected_ref.rb b/app/models/concerns/protected_ref.rb index 454374121f3..94eef4ff7cd 100644 --- a/app/models/concerns/protected_ref.rb +++ b/app/models/concerns/protected_ref.rb @@ -31,7 +31,7 @@ module ProtectedRef end end - def protected_ref_accessible_to?(ref, user, action:, protected_refs: nil) + def protected_ref_accessible_to?(ref, user, project:, action:, protected_refs: nil) access_levels_for_ref(ref, action: action, protected_refs: protected_refs).any? do |access_level| access_level.check_access(user) end diff --git a/app/models/concerns/routable.rb b/app/models/concerns/routable.rb index dfd7d94450b..915ad6959be 100644 --- a/app/models/concerns/routable.rb +++ b/app/models/concerns/routable.rb @@ -102,7 +102,7 @@ module Routable # the route. Caching this per request ensures that even if we have multiple instances, # we will not have to duplicate work, avoiding N+1 queries in some cases. def full_path - return uncached_full_path unless RequestStore.active? + return uncached_full_path unless RequestStore.active? && persisted? RequestStore[full_path_key] ||= uncached_full_path end @@ -124,6 +124,11 @@ module Routable end end + # Group would override this to check from association + def owned_by?(user) + owner == user + end + private def set_path_errors diff --git a/app/models/concerns/storage/legacy_namespace.rb b/app/models/concerns/storage/legacy_namespace.rb index f05e606995d..f66bdd529f1 100644 --- a/app/models/concerns/storage/legacy_namespace.rb +++ b/app/models/concerns/storage/legacy_namespace.rb @@ -45,25 +45,25 @@ module Storage # Hooks - # Save the storage paths before the projects are destroyed to use them on after destroy + # Save the storages before the projects are destroyed to use them on after destroy def prepare_for_destroy - old_repository_storage_paths + old_repository_storages end private def move_repositories - # Move the namespace directory in all storage paths used by member projects - repository_storage_paths.each do |repository_storage_path| + # Move the namespace directory in all storages used by member projects + repository_storages.each do |repository_storage| # Ensure old directory exists before moving it - gitlab_shell.add_namespace(repository_storage_path, full_path_was) + gitlab_shell.add_namespace(repository_storage, full_path_was) # Ensure new directory exists before moving it (if there's a parent) - gitlab_shell.add_namespace(repository_storage_path, parent.full_path) if parent + gitlab_shell.add_namespace(repository_storage, parent.full_path) if parent - unless gitlab_shell.mv_namespace(repository_storage_path, full_path_was, full_path) + unless gitlab_shell.mv_namespace(repository_storage, full_path_was, full_path) - Rails.logger.error "Exception moving path #{repository_storage_path} from #{full_path_was} to #{full_path}" + Rails.logger.error "Exception moving path #{repository_storage} from #{full_path_was} to #{full_path}" # if we cannot move namespace directory we should rollback # db changes in order to prevent out of sync between db and fs @@ -72,33 +72,33 @@ module Storage end end - def old_repository_storage_paths - @old_repository_storage_paths ||= repository_storage_paths + def old_repository_storages + @old_repository_storage_paths ||= repository_storages end - def repository_storage_paths + def repository_storages # We need to get the storage paths for all the projects, even the ones that are # pending delete. Unscoping also get rids of the default order, which causes # problems with SELECT DISTINCT. Project.unscoped do - all_projects.select('distinct(repository_storage)').to_a.map(&:repository_storage_path) + all_projects.select('distinct(repository_storage)').to_a.map(&:repository_storage) end end def rm_dir # Remove the namespace directory in all storages paths used by member projects - old_repository_storage_paths.each do |repository_storage_path| + old_repository_storages.each do |repository_storage| # Move namespace directory into trash. # We will remove it later async new_path = "#{full_path}+#{id}+deleted" - if gitlab_shell.mv_namespace(repository_storage_path, full_path, new_path) + if gitlab_shell.mv_namespace(repository_storage, full_path, new_path) Gitlab::AppLogger.info %Q(Namespace directory "#{full_path}" moved to "#{new_path}") # Remove namespace directroy async with delay so # GitLab has time to remove all projects first run_after_commit do - GitlabShellWorker.perform_in(5.minutes, :rm_namespace, repository_storage_path, new_path) + GitlabShellWorker.perform_in(5.minutes, :rm_namespace, repository_storage, new_path) end end end |