diff options
Diffstat (limited to 'lib')
52 files changed, 718 insertions, 352 deletions
diff --git a/lib/api/api.rb b/lib/api/api.rb index e2ad3c5f4e3..c000666d992 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -99,12 +99,13 @@ module API mount ::API::Features mount ::API::Files mount ::API::GroupBoards - mount ::API::Groups mount ::API::GroupMilestones + mount ::API::Groups + mount ::API::GroupVariables mount ::API::Internal mount ::API::Issues - mount ::API::Jobs mount ::API::JobArtifacts + mount ::API::Jobs mount ::API::Keys mount ::API::Labels mount ::API::Lint @@ -122,11 +123,12 @@ module API mount ::API::ProjectExport mount ::API::ProjectImport mount ::API::ProjectHooks - mount ::API::Projects mount ::API::ProjectMilestones + mount ::API::Projects mount ::API::ProjectSnapshots mount ::API::ProjectSnippets mount ::API::ProtectedBranches + mount ::API::ProtectedTags mount ::API::Repositories mount ::API::Runner mount ::API::Runners @@ -143,7 +145,6 @@ module API mount ::API::Triggers mount ::API::Users mount ::API::Variables - mount ::API::GroupVariables mount ::API::Version mount ::API::Wikis diff --git a/lib/api/award_emoji.rb b/lib/api/award_emoji.rb index c3d93996816..bde4b3ff4f6 100644 --- a/lib/api/award_emoji.rb +++ b/lib/api/award_emoji.rb @@ -100,7 +100,7 @@ module API end def can_award_awardable? - awardable.user_can_award?(current_user, params[:name]) + awardable.user_can_award?(current_user) end def awardable diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 27f28e1df93..95b25d7351a 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -91,6 +91,7 @@ module API group_link.group.name end expose :group_access, as: :group_access_level + expose :expires_at end class ProjectIdentity < Grape::Entity @@ -428,6 +429,11 @@ module API expose :merge_access_levels, using: Entities::ProtectedRefAccess end + class ProtectedTag < Grape::Entity + expose :name + expose :create_access_levels, using: Entities::ProtectedRefAccess + end + class Milestone < Grape::Entity expose :id, :iid expose :project_id, if: -> (entity, options) { entity&.project_id } @@ -856,7 +862,7 @@ module API class NotificationSetting < Grape::Entity expose :level expose :events, if: ->(notification_setting, _) { notification_setting.custom? } do - ::NotificationSetting::EMAIL_EVENTS.each do |event| + ::NotificationSetting.email_events.each do |event| expose event end end @@ -1080,6 +1086,10 @@ module API expose :filename, :size end + class JobArtifact < Grape::Entity + expose :file_type, :size, :filename, :file_format + end + class JobBasic < Grape::Entity expose :id, :status, :stage, :name, :ref, :tag, :coverage expose :created_at, :started_at, :finished_at @@ -1094,7 +1104,9 @@ module API end class Job < JobBasic + # artifacts_file is included in job_artifacts, but kept for backward compatibility (remove in api/v5) expose :artifacts_file, using: JobArtifactFile, if: -> (job, opts) { job.artifacts? } + expose :job_artifacts, as: :artifacts, using: JobArtifact expose :runner, with: Runner expose :artifacts_expire_at end @@ -1153,7 +1165,7 @@ module API class License < Grape::Entity expose :key, :name, :nickname - expose :featured, as: :popular + expose :popular?, as: :popular expose :url, as: :html_url expose(:source_url) { |license| license.meta['source'] } expose(:description) { |license| license.meta['description'] } diff --git a/lib/api/events.rb b/lib/api/events.rb index fc4ba5a3188..a415508a632 100644 --- a/lib/api/events.rb +++ b/lib/api/events.rb @@ -1,6 +1,7 @@ module API class Events < Grape::API include PaginationParams + include APIGuard helpers do params :event_filter_params do @@ -24,6 +25,8 @@ module API end resource :events do + allow_access_with_scope :read_user, if: -> (request) { request.get? } + desc "List currently authenticated user's events" do detail 'This feature was introduced in GitLab 9.3.' success Entities::Event @@ -46,6 +49,8 @@ module API requires :id, type: String, desc: 'The ID or Username of the user' end resource :users do + allow_access_with_scope :read_user, if: -> (request) { request.get? } + desc 'Get the contribution events of a specified user' do detail 'This feature was introduced in GitLab 8.13.' success Entities::Event diff --git a/lib/api/files.rb b/lib/api/files.rb index 29d7489bd7c..ff4f75c12df 100644 --- a/lib/api/files.rb +++ b/lib/api/files.rb @@ -59,7 +59,7 @@ module API params :simple_file_params do requires :file_path, type: String, desc: 'The url encoded path to the file. Ex. lib%2Fclass%2Erb' requires :branch, type: String, desc: 'Name of the branch to commit into. To create a new branch, also provide `start_branch`.' - requires :commit_message, type: String, desc: 'Commit message' + requires :commit_message, type: String, allow_blank: false, desc: 'Commit message' optional :start_branch, type: String, desc: 'Name of the branch to start the new commit from' optional :author_email, type: String, desc: 'The email of the author' optional :author_name, type: String, desc: 'The name of the author' diff --git a/lib/api/jobs.rb b/lib/api/jobs.rb index 10c6e565f09..fc8c52085ab 100644 --- a/lib/api/jobs.rb +++ b/lib/api/jobs.rb @@ -38,7 +38,7 @@ module API builds = user_project.builds.order('id DESC') builds = filter_builds(builds, params[:scope]) - builds = builds.preload(:user, :job_artifacts_archive, :runner, pipeline: :project) + builds = builds.preload(:user, :job_artifacts_archive, :job_artifacts, :runner, pipeline: :project) present paginate(builds), with: Entities::Job end @@ -54,7 +54,7 @@ module API pipeline = user_project.pipelines.find(params[:pipeline_id]) builds = pipeline.builds builds = filter_builds(builds, params[:scope]) - builds = builds.preload(:job_artifacts_archive, project: [:namespace]) + builds = builds.preload(:job_artifacts_archive, :job_artifacts, project: [:namespace]) present paginate(builds), with: Entities::Job end diff --git a/lib/api/notification_settings.rb b/lib/api/notification_settings.rb index 0266bf2f717..bf0d6b9e434 100644 --- a/lib/api/notification_settings.rb +++ b/lib/api/notification_settings.rb @@ -23,7 +23,7 @@ module API params do optional :level, type: String, desc: 'The global notification level' optional :notification_email, type: String, desc: 'The email address to send notifications' - NotificationSetting::EMAIL_EVENTS.each do |event| + NotificationSetting.email_events.each do |event| optional event, type: Boolean, desc: 'Enable/disable this notification' end end @@ -50,7 +50,9 @@ module API end end - %w[group project].each do |source_type| + [Group, Project].each do |source_class| + source_type = source_class.name.underscore + params do requires :id, type: String, desc: "The #{source_type} ID" end @@ -73,7 +75,7 @@ module API end params do optional :level, type: String, desc: "The #{source_type} notification level" - NotificationSetting::EMAIL_EVENTS.each do |event| + NotificationSetting.email_events(source_class).each do |event| optional event, type: Boolean, desc: 'Enable/disable this notification' end end diff --git a/lib/api/project_snippets.rb b/lib/api/project_snippets.rb index 1de5551fee9..0ada0ef4708 100644 --- a/lib/api/project_snippets.rb +++ b/lib/api/project_snippets.rb @@ -49,7 +49,7 @@ module API params do requires :title, type: String, desc: 'The title of the snippet' requires :file_name, type: String, desc: 'The file name of the snippet' - requires :code, type: String, desc: 'The content of the snippet' + requires :code, type: String, allow_blank: false, desc: 'The content of the snippet' optional :description, type: String, desc: 'The description of a snippet' requires :visibility, type: String, values: Gitlab::VisibilityLevel.string_values, @@ -78,7 +78,7 @@ module API requires :snippet_id, type: Integer, desc: 'The ID of a project snippet' optional :title, type: String, desc: 'The title of the snippet' optional :file_name, type: String, desc: 'The file name of the snippet' - optional :code, type: String, desc: 'The content of the snippet' + optional :code, type: String, allow_blank: false, desc: 'The content of the snippet' optional :description, type: String, desc: 'The description of a snippet' optional :visibility, type: String, values: Gitlab::VisibilityLevel.string_values, diff --git a/lib/api/projects.rb b/lib/api/projects.rb index 5738bf220c6..2801ae918c6 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -386,7 +386,7 @@ module API requires :forked_from_id, type: String, desc: 'The ID of the project it was forked from' end post ":id/fork/:forked_from_id" do - authenticated_as_admin! + authorize! :admin_project, user_project fork_from_project = find_project!(params[:forked_from_id]) diff --git a/lib/api/protected_tags.rb b/lib/api/protected_tags.rb new file mode 100644 index 00000000000..bf0a7184e1c --- /dev/null +++ b/lib/api/protected_tags.rb @@ -0,0 +1,79 @@ +module API + class ProtectedTags < Grape::API + include PaginationParams + + TAG_ENDPOINT_REQUIREMENTS = API::PROJECT_ENDPOINT_REQUIREMENTS.merge(name: API::NO_SLASH_URL_PART_REGEX) + + before { authorize_admin_project } + + params do + requires :id, type: String, desc: 'The ID of a project' + end + resource :projects, requirements: API::PROJECT_ENDPOINT_REQUIREMENTS do + desc "Get a project's protected tags" do + detail 'This feature was introduced in GitLab 11.3.' + success Entities::ProtectedTag + end + params do + use :pagination + end + get ':id/protected_tags' do + protected_tags = user_project.protected_tags.preload(:create_access_levels) + + present paginate(protected_tags), with: Entities::ProtectedTag, project: user_project + end + + desc 'Get a single protected tag' do + detail 'This feature was introduced in GitLab 11.3.' + success Entities::ProtectedTag + end + params do + requires :name, type: String, desc: 'The name of the tag or wildcard' + end + get ':id/protected_tags/:name', requirements: TAG_ENDPOINT_REQUIREMENTS do + protected_tag = user_project.protected_tags.find_by!(name: params[:name]) + + present protected_tag, with: Entities::ProtectedTag, project: user_project + end + + desc 'Protect a single tag or wildcard' do + detail 'This feature was introduced in GitLab 11.3.' + success Entities::ProtectedTag + end + params do + requires :name, type: String, desc: 'The name of the protected tag' + optional :create_access_level, type: Integer, default: Gitlab::Access::MAINTAINER, + values: ProtectedRefAccess::ALLOWED_ACCESS_LEVELS, + desc: 'Access levels allowed to create (defaults: `40`, maintainer access level)' + end + post ':id/protected_tags' do + protected_tags_params = { + name: params[:name], + create_access_levels_attributes: [{ access_level: params[:create_access_level] }] + } + + protected_tag = ::ProtectedTags::CreateService.new(user_project, + current_user, + protected_tags_params).execute + + if protected_tag.persisted? + present protected_tag, with: Entities::ProtectedTag, project: user_project + else + render_api_error!(protected_tag.errors.full_messages, 422) + end + end + + desc 'Unprotect a single tag' do + detail 'This feature was introduced in GitLab 11.3.' + end + params do + requires :name, type: String, desc: 'The name of the protected tag' + end + delete ':id/protected_tags/:name', requirements: TAG_ENDPOINT_REQUIREMENTS do + protected_tag = user_project.protected_tags.find_by!(name: params[:name]) + + destroy_conditionally!(protected_tag) + end + end + end +end diff --git a/lib/api/repositories.rb b/lib/api/repositories.rb index 33a9646ac3b..79736107bbb 100644 --- a/lib/api/repositories.rb +++ b/lib/api/repositories.rb @@ -123,6 +123,39 @@ module API not_found! end end + + desc 'Get the common ancestor between commits' do + success Entities::Commit + end + params do + # For now we just support 2 refs passed, but `merge-base` supports + # multiple defining this as an Array instead of 2 separate params will + # make sure we don't need to deprecate this API in favor of one + # supporting multiple commits when this functionality gets added to + # Gitaly + requires :refs, type: Array[String] + end + get ':id/repository/merge_base' do + refs = params[:refs] + + unless refs.size == 2 + render_api_error!('Provide exactly 2 refs', 400) + end + + merge_base = Gitlab::Git::MergeBase.new(user_project.repository, refs) + + if merge_base.unknown_refs.any? + ref_noun = 'ref'.pluralize(merge_base.unknown_refs.size) + message = "Could not find #{ref_noun}: #{merge_base.unknown_refs.join(', ')}" + render_api_error!(message, 400) + end + + if merge_base.commit + present merge_base.commit, with: Entities::Commit + else + not_found!("Merge Base") + end + end end end end diff --git a/lib/api/services.rb b/lib/api/services.rb index 1f2bf546cd7..d1a5ee7db35 100644 --- a/lib/api/services.rb +++ b/lib/api/services.rb @@ -354,20 +354,6 @@ module API desc: 'Flowdock token' } ], - 'gemnasium' => [ - { - required: true, - name: :api_key, - type: String, - desc: 'Your personal API key on gemnasium.com' - }, - { - required: true, - name: :token, - type: String, - desc: "The project's slug on gemnasium.com" - } - ], 'hangouts-chat' => [ { required: true, @@ -695,7 +681,6 @@ module API EmailsOnPushService, ExternalWikiService, FlowdockService, - GemnasiumService, HangoutsChatService, HipchatService, IrkerService, diff --git a/lib/api/templates.rb b/lib/api/templates.rb index 41862768a3f..927baaea652 100644 --- a/lib/api/templates.rb +++ b/lib/api/templates.rb @@ -16,31 +16,8 @@ module API gitlab_version: 8.15 } }.freeze - PROJECT_TEMPLATE_REGEX = - %r{[\<\{\[] - (project|description| - one\sline\s.+\swhat\sit\sdoes\.) # matching the start and end is enough here - [\>\}\]]}xi.freeze - YEAR_TEMPLATE_REGEX = /[<{\[](year|yyyy)[>}\]]/i.freeze - FULLNAME_TEMPLATE_REGEX = - %r{[\<\{\[] - (fullname|name\sof\s(author|copyright\sowner)) - [\>\}\]]}xi.freeze helpers do - def parsed_license_template - # We create a fresh Licensee::License object since we'll modify its - # content in place below. - template = Licensee::License.new(params[:name]) - - template.content.gsub!(YEAR_TEMPLATE_REGEX, Time.now.year.to_s) - template.content.gsub!(PROJECT_TEMPLATE_REGEX, params[:project]) if params[:project].present? - - fullname = params[:fullname].presence || current_user.try(:name) - template.content.gsub!(FULLNAME_TEMPLATE_REGEX, fullname) if fullname - template - end - def render_response(template_type, template) not_found!(template_type.to_s.singularize) unless template present template, with: Entities::Template @@ -56,11 +33,12 @@ module API use :pagination end get "templates/licenses" do - options = { - featured: declared(params)[:popular].present? ? true : nil - } - licences = ::Kaminari.paginate_array(Licensee::License.all(options)) - present paginate(licences), with: Entities::License + popular = declared(params)[:popular] + popular = to_boolean(popular) if popular.present? + + templates = LicenseTemplateFinder.new(popular: popular).execute + + present paginate(::Kaminari.paginate_array(templates)), with: ::API::Entities::License end desc 'Get the text for a specific license' do @@ -71,9 +49,15 @@ module API requires :name, type: String, desc: 'The name of the template' end get "templates/licenses/:name", requirements: { name: /[\w\.-]+/ } do - not_found!('License') unless Licensee::License.find(declared(params)[:name]) + templates = LicenseTemplateFinder.new.execute + template = templates.find { |template| template.key == params[:name] } + + not_found!('License') unless template.present? - template = parsed_license_template + template.resolve!( + project_name: params[:project].presence, + fullname: params[:fullname].presence || current_user&.name + ) present template, with: ::API::Entities::License end diff --git a/lib/banzai/filter/spaced_link_filter.rb b/lib/banzai/filter/spaced_link_filter.rb new file mode 100644 index 00000000000..574a8a6c7a5 --- /dev/null +++ b/lib/banzai/filter/spaced_link_filter.rb @@ -0,0 +1,77 @@ +# frozen_string_literal: true + +require 'uri' + +module Banzai + module Filter + # HTML Filter for markdown links with spaces in the URLs + # + # Based on Banzai::Filter::AutolinkFilter + # + # CommonMark does not allow spaces in the url portion of a link. + # For example, `[example](page slug)` is not valid. However, + # in our wikis, we support (via RedCarpet) this type of link, allowing + # wiki pages to be easily linked by their title. This filter adds that functionality. + # The intent is for this to only be used in Wikis - in general, we want + # to adhere to CommonMark's spec. + # + class SpacedLinkFilter < HTML::Pipeline::Filter + include ActionView::Helpers::TagHelper + + # Pattern to match a standard markdown link + # + # Rubular: http://rubular.com/r/z9EAHxYmKI + LINK_PATTERN = /\[([^\]]+)\]\(([^)"]+)(?: \"([^\"]+)\")?\)/ + + # Text matching LINK_PATTERN inside these elements will not be linked + IGNORE_PARENTS = %w(a code kbd pre script style).to_set + + # The XPath query to use for finding text nodes to parse. + TEXT_QUERY = %Q(descendant-or-self::text()[ + not(#{IGNORE_PARENTS.map { |p| "ancestor::#{p}" }.join(' or ')}) + and contains(., ']\(') + ]).freeze + + def call + return doc if context[:markdown_engine] == :redcarpet + + doc.xpath(TEXT_QUERY).each do |node| + content = node.to_html + + next unless content.match(LINK_PATTERN) + + html = spaced_link_filter(content) + + next if html == content + + node.replace(html) + end + + doc + end + + private + + def spaced_link_match(link) + match = LINK_PATTERN.match(link) + return link unless match && match[1] && match[2] + + # escape the spaces in the url so that it's a valid markdown link, + # then run it through the markdown processor again, let it do its magic + text = match[1] + new_link = match[2].gsub(' ', '%20') + title = match[3] ? " \"#{match[3]}\"" : '' + html = Banzai::Filter::MarkdownFilter.call("[#{text}](#{new_link}#{title})", context) + + # link is wrapped in a <p>, so strip that off + html.sub('<p>', '').chomp('</p>') + end + + def spaced_link_filter(text) + Gitlab::StringRegexMarker.new(CGI.unescapeHTML(text), text.html_safe).mark(LINK_PATTERN) do |link, left:, right:| + spaced_link_match(link) + end + end + end + end +end diff --git a/lib/banzai/pipeline/wiki_pipeline.rb b/lib/banzai/pipeline/wiki_pipeline.rb index c37b8e71cb0..737ff0cc818 100644 --- a/lib/banzai/pipeline/wiki_pipeline.rb +++ b/lib/banzai/pipeline/wiki_pipeline.rb @@ -5,6 +5,7 @@ module Banzai @filters ||= begin super.insert_after(Filter::TableOfContentsFilter, Filter::GollumTagsFilter) .insert_before(Filter::TaskListFilter, Filter::WikiLinkFilter) + .insert_before(Filter::WikiLinkFilter, Filter::SpacedLinkFilter) end end end diff --git a/lib/banzai/reference_parser/merge_request_parser.rb b/lib/banzai/reference_parser/merge_request_parser.rb index a370ff5b5b3..9e5d55f72bc 100644 --- a/lib/banzai/reference_parser/merge_request_parser.rb +++ b/lib/banzai/reference_parser/merge_request_parser.rb @@ -14,11 +14,12 @@ module Banzai # Eager loading these ensures we don't end up running dozens of # queries in this process. target_project: [ - { namespace: :owner }, + { namespace: [:owner, :route] }, { group: [:owners, :group_members] }, :invited_groups, :project_members, - :project_feature + :project_feature, + :route ] }), self.class.data_attribute diff --git a/lib/feature.rb b/lib/feature.rb index 09c5ef3ad94..24dbcb32fc0 100644 --- a/lib/feature.rb +++ b/lib/feature.rb @@ -47,7 +47,8 @@ class Feature end def disabled?(key, thing = nil) - !enabled?(key, thing) + # we need to make different method calls to make it easy to mock / define expectations in test mode + thing.nil? ? !enabled?(key) : !enabled?(key, thing) end def enable(key, thing = true) diff --git a/lib/gitlab/auth/ldap/access.rb b/lib/gitlab/auth/ldap/access.rb index 865185eb5db..eeab7791643 100644 --- a/lib/gitlab/auth/ldap/access.rb +++ b/lib/gitlab/auth/ldap/access.rb @@ -19,8 +19,10 @@ module Gitlab # Whether user is allowed, or not, we should update # permissions to keep things clean if access.allowed? - access.update_user - Users::UpdateService.new(user, user: user, last_credential_check_at: Time.now).execute + unless Gitlab::Database.read_only? + access.update_user + Users::UpdateService.new(user, user: user, last_credential_check_at: Time.now).execute + end true else @@ -60,6 +62,12 @@ module Gitlab false end + def update_user + # no-op in CE + end + + private + def adapter @adapter ||= Gitlab::Auth::LDAP::Adapter.new(provider) end @@ -68,16 +76,16 @@ module Gitlab Gitlab::Auth::LDAP::Config.new(provider) end - def find_ldap_user - Gitlab::Auth::LDAP::Person.find_by_dn(ldap_identity.extern_uid, adapter) - end - def ldap_user return unless provider @ldap_user ||= find_ldap_user end + def find_ldap_user + Gitlab::Auth::LDAP::Person.find_by_dn(ldap_identity.extern_uid, adapter) + end + def block_user(user, reason) user.ldap_block @@ -102,10 +110,6 @@ module Gitlab "unblocking Gitlab user \"#{user.name}\" (#{user.email})" ) end - - def update_user - # no-op in CE - end end end end diff --git a/lib/gitlab/auth/o_auth/provider.rb b/lib/gitlab/auth/o_auth/provider.rb index e73743944a9..26da9d09ccc 100644 --- a/lib/gitlab/auth/o_auth/provider.rb +++ b/lib/gitlab/auth/o_auth/provider.rb @@ -29,6 +29,7 @@ module Gitlab def self.enabled?(name) return true if name == 'database' + return true if self.ldap_provider?(name) && providers.include?(name.to_sym) Gitlab::Auth.omniauth_enabled? && providers.include?(name.to_sym) end diff --git a/lib/gitlab/bitbucket_server_import/importer.rb b/lib/gitlab/bitbucket_server_import/importer.rb index 268d21a77d1..b591d94668f 100644 --- a/lib/gitlab/bitbucket_server_import/importer.rb +++ b/lib/gitlab/bitbucket_server_import/importer.rb @@ -1,7 +1,10 @@ +# frozen_string_literal: true + module Gitlab module BitbucketServerImport class Importer include Gitlab::ShellAdapter + attr_reader :recover_missing_commits attr_reader :project, :project_key, :repository_slug, :client, :errors, :users @@ -175,21 +178,18 @@ module Gitlab description = '' description += @formatter.author_line(pull_request.author) unless find_user_id(pull_request.author_email) description += pull_request.description if pull_request.description - - source_branch_sha = pull_request.source_branch_sha - target_branch_sha = pull_request.target_branch_sha author_id = gitlab_user_id(pull_request.author_email) attributes = { iid: pull_request.iid, title: pull_request.title, description: description, - source_project: project, + source_project_id: project.id, source_branch: Gitlab::Git.ref_name(pull_request.source_branch_name), - source_branch_sha: source_branch_sha, - target_project: project, + source_branch_sha: pull_request.source_branch_sha, + target_project_id: project.id, target_branch: Gitlab::Git.ref_name(pull_request.target_branch_name), - target_branch_sha: target_branch_sha, + target_branch_sha: pull_request.target_branch_sha, state: pull_request.state, author_id: author_id, assignee_id: nil, @@ -197,7 +197,9 @@ module Gitlab updated_at: pull_request.updated_at } - merge_request = project.merge_requests.create!(attributes) + creator = Gitlab::Import::MergeRequestCreator.new(project) + merge_request = creator.execute(attributes) + import_pull_request_comments(pull_request, merge_request) if merge_request.persisted? end diff --git a/lib/gitlab/data_builder/build.rb b/lib/gitlab/data_builder/build.rb index 2f1445a050a..0b71b31a476 100644 --- a/lib/gitlab/data_builder/build.rb +++ b/lib/gitlab/data_builder/build.rb @@ -28,6 +28,7 @@ module Gitlab build_finished_at: build.finished_at, build_duration: build.duration, build_allow_failure: build.allow_failure, + build_failure_reason: build.failure_reason, # TODO: do we still need it? project_id: project.id, diff --git a/lib/gitlab/database/migration_helpers.rb b/lib/gitlab/database/migration_helpers.rb index f39b3b6eb5b..7f012312819 100644 --- a/lib/gitlab/database/migration_helpers.rb +++ b/lib/gitlab/database/migration_helpers.rb @@ -58,7 +58,6 @@ module Gitlab if Database.postgresql? options = options.merge({ algorithm: :concurrently }) - disable_statement_timeout end if index_exists?(table_name, column_name, options) @@ -66,7 +65,9 @@ module Gitlab return end - add_index(table_name, column_name, options) + disable_statement_timeout do + add_index(table_name, column_name, options) + end end # Removes an existed index, concurrently when supported @@ -87,7 +88,6 @@ module Gitlab if supports_drop_index_concurrently? options = options.merge({ algorithm: :concurrently }) - disable_statement_timeout end unless index_exists?(table_name, column_name, options) @@ -95,7 +95,9 @@ module Gitlab return end - remove_index(table_name, options.merge({ column: column_name })) + disable_statement_timeout do + remove_index(table_name, options.merge({ column: column_name })) + end end # Removes an existing index, concurrently when supported @@ -116,7 +118,6 @@ module Gitlab if supports_drop_index_concurrently? options = options.merge({ algorithm: :concurrently }) - disable_statement_timeout end unless index_exists_by_name?(table_name, index_name) @@ -124,7 +125,9 @@ module Gitlab return end - remove_index(table_name, options.merge({ name: index_name })) + disable_statement_timeout do + remove_index(table_name, options.merge({ name: index_name })) + end end # Only available on Postgresql >= 9.2 @@ -171,8 +174,6 @@ module Gitlab on_delete = 'SET NULL' if on_delete == :nullify end - disable_statement_timeout - key_name = concurrent_foreign_key_name(source, column) unless foreign_key_exists?(source, target, column: column) @@ -199,7 +200,9 @@ module Gitlab # while running. # # Note this is a no-op in case the constraint is VALID already - execute("ALTER TABLE #{source} VALIDATE CONSTRAINT #{key_name};") + disable_statement_timeout do + execute("ALTER TABLE #{source} VALIDATE CONSTRAINT #{key_name};") + end end def foreign_key_exists?(source, target = nil, column: nil) @@ -224,8 +227,48 @@ module Gitlab # Long-running migrations may take more than the timeout allowed by # the database. Disable the session's statement timeout to ensure # migrations don't get killed prematurely. (PostgreSQL only) + # + # There are two possible ways to disable the statement timeout: + # + # - Per transaction (this is the preferred and default mode) + # - Per connection (requires a cleanup after the execution) + # + # When using a per connection disable statement, code must be inside + # a block so we can automatically execute `RESET ALL` after block finishes + # otherwise the statement will still be disabled until connection is dropped + # or `RESET ALL` is executed def disable_statement_timeout - execute('SET statement_timeout TO 0') if Database.postgresql? + # bypass disabled_statement logic when not using postgres, but still execute block when one is given + unless Database.postgresql? + if block_given? + yield + end + + return + end + + if block_given? + begin + execute('SET statement_timeout TO 0') + + yield + ensure + execute('RESET ALL') + end + else + unless transaction_open? + raise <<~ERROR + Cannot call disable_statement_timeout() without a transaction open or outside of a transaction block. + If you don't want to use a transaction wrap your code in a block call: + + disable_statement_timeout { # code that requires disabled statement here } + + This will make sure statement_timeout is disabled before and reset after the block execution is finished. + ERROR + end + + execute('SET LOCAL statement_timeout TO 0') + end end def true_value @@ -367,30 +410,30 @@ module Gitlab 'in the body of your migration class' end - disable_statement_timeout - - transaction do - if limit - add_column(table, column, type, default: nil, limit: limit) - else - add_column(table, column, type, default: nil) + disable_statement_timeout do + transaction do + if limit + add_column(table, column, type, default: nil, limit: limit) + else + add_column(table, column, type, default: nil) + end + + # Changing the default before the update ensures any newly inserted + # rows already use the proper default value. + change_column_default(table, column, default) end - # Changing the default before the update ensures any newly inserted - # rows already use the proper default value. - change_column_default(table, column, default) - end - - begin - update_column_in_batches(table, column, default, &block) + begin + update_column_in_batches(table, column, default, &block) - change_column_null(table, column, false) unless allow_null - # We want to rescue _all_ exceptions here, even those that don't inherit - # from StandardError. - rescue Exception => error # rubocop: disable all - remove_column(table, column) + change_column_null(table, column, false) unless allow_null + # We want to rescue _all_ exceptions here, even those that don't inherit + # from StandardError. + rescue Exception => error # rubocop: disable all + remove_column(table, column) - raise error + raise error + end end end diff --git a/lib/gitlab/git.rb b/lib/gitlab/git.rb index 55236a1122f..2913a3e416d 100644 --- a/lib/gitlab/git.rb +++ b/lib/gitlab/git.rb @@ -10,9 +10,11 @@ module Gitlab TAG_REF_PREFIX = "refs/tags/".freeze BRANCH_REF_PREFIX = "refs/heads/".freeze - CommandError = Class.new(StandardError) - CommitError = Class.new(StandardError) - OSError = Class.new(StandardError) + BaseError = Class.new(StandardError) + CommandError = Class.new(BaseError) + CommitError = Class.new(BaseError) + OSError = Class.new(BaseError) + UnknownRef = Class.new(BaseError) class << self include Gitlab::EncodingHelper diff --git a/lib/gitlab/git/diff.rb b/lib/gitlab/git/diff.rb index b58296375ef..61ce10ca131 100644 --- a/lib/gitlab/git/diff.rb +++ b/lib/gitlab/git/diff.rb @@ -226,6 +226,7 @@ module Gitlab @new_file = diff.from_id == BLANK_SHA @renamed_file = diff.from_path != diff.to_path @deleted_file = diff.to_id == BLANK_SHA + @too_large = diff.too_large if diff.respond_to?(:too_large) collapse! if diff.respond_to?(:collapsed) && diff.collapsed end diff --git a/lib/gitlab/git/merge_base.rb b/lib/gitlab/git/merge_base.rb new file mode 100644 index 00000000000..b27f7038c26 --- /dev/null +++ b/lib/gitlab/git/merge_base.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +module Gitlab + module Git + class MergeBase + include Gitlab::Utils::StrongMemoize + + def initialize(repository, refs) + @repository, @refs = repository, refs + end + + # Returns the SHA of the first common ancestor + def sha + if unknown_refs.any? + raise UnknownRef, "Can't find merge base for unknown refs: #{unknown_refs.inspect}" + end + + strong_memoize(:sha) do + @repository.merge_base(*commits_for_refs) + end + end + + # Returns the merge base as a Gitlab::Git::Commit + def commit + return unless sha + + @commit ||= @repository.commit_by(oid: sha) + end + + # Returns the refs passed on initialization that aren't found in + # the repository, and thus cannot be used to find a merge base. + def unknown_refs + @unknown_refs ||= Hash[@refs.zip(commits_for_refs)] + .select { |ref, commit| commit.nil? }.keys + end + + private + + def commits_for_refs + @commits_for_refs ||= @repository.commits_by(oids: @refs) + end + end + end +end diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 3e11355435b..9521a2d63a0 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -366,18 +366,9 @@ module Gitlab end end - # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/1233 def new_commits(newrev) - gitaly_migrate(:new_commits) do |is_enabled| - if is_enabled - gitaly_ref_client.list_new_commits(newrev) - else - refs = Gitlab::GitalyClient::StorageSettings.allow_disk_access do - rev_list(including: newrev, excluding: :all).split("\n").map(&:strip) - end - - Gitlab::Git::Commit.batch_by_oid(self, refs) - end + wrapped_gitaly_errors do + gitaly_ref_client.list_new_commits(newrev) end end @@ -552,14 +543,8 @@ module Gitlab end def update_branch(branch_name, user:, newrev:, oldrev:) - gitaly_migrate(:operation_user_update_branch) do |is_enabled| - if is_enabled - gitaly_operation_client.user_update_branch(branch_name, user, newrev, oldrev) - else - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - OperationService.new(user, self).update_branch(branch_name, newrev, oldrev) - end - end + wrapped_gitaly_errors do + gitaly_operation_client.user_update_branch(branch_name, user, newrev, oldrev) end end diff --git a/lib/gitlab/git/repository_mirroring.rb b/lib/gitlab/git/repository_mirroring.rb index 65eb5cc18cf..752a91fbb60 100644 --- a/lib/gitlab/git/repository_mirroring.rb +++ b/lib/gitlab/git/repository_mirroring.rb @@ -2,34 +2,7 @@ module Gitlab module Git module RepositoryMirroring def remote_branches(remote_name) - gitaly_migrate(:ref_find_all_remote_branches) do |is_enabled| - if is_enabled - gitaly_ref_client.remote_branches(remote_name) - else - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - rugged_remote_branches(remote_name) - end - end - end - end - - private - - def rugged_remote_branches(remote_name) - branches = [] - - rugged.references.each("refs/remotes/#{remote_name}/*").map do |ref| - name = ref.name.sub(%r{\Arefs/remotes/#{remote_name}/}, '') - - begin - target_commit = Gitlab::Git::Commit.find(self, ref.target.oid) - branches << Gitlab::Git::Branch.new(self, name, ref.target, target_commit) - rescue Rugged::ReferenceError - # Omit invalid branch - end - end - - branches + gitaly_ref_client.remote_branches(remote_name) end end end diff --git a/lib/gitlab/gitaly_client/diff.rb b/lib/gitlab/gitaly_client/diff.rb index d98a0ce988f..af9d674535b 100644 --- a/lib/gitlab/gitaly_client/diff.rb +++ b/lib/gitlab/gitaly_client/diff.rb @@ -1,7 +1,7 @@ module Gitlab module GitalyClient class Diff - ATTRS = %i(from_path to_path old_mode new_mode from_id to_id patch overflow_marker collapsed).freeze + ATTRS = %i(from_path to_path old_mode new_mode from_id to_id patch overflow_marker collapsed too_large).freeze include AttributesBag end diff --git a/lib/gitlab/github_import.rb b/lib/gitlab/github_import.rb index 65b5e30c70f..d40b06f969f 100644 --- a/lib/gitlab/github_import.rb +++ b/lib/gitlab/github_import.rb @@ -10,24 +10,6 @@ module Gitlab Client.new(token_to_use, parallel: parallel) end - # Inserts a raw row and returns the ID of the inserted row. - # - # attributes - The attributes/columns to set. - # relation - An ActiveRecord::Relation to use for finding the ID of the row - # when using MySQL. - def self.insert_and_return_id(attributes, relation) - # We use bulk_insert here so we can bypass any queries executed by - # callbacks or validation rules, as doing this wouldn't scale when - # importing very large projects. - result = Gitlab::Database - .bulk_insert(relation.table_name, [attributes], return_ids: true) - - # MySQL doesn't support returning the IDs of a bulk insert in a way that - # is not a pain, so in this case we'll issue an extra query instead. - result.first || - relation.where(iid: attributes[:iid]).limit(1).pluck(:id).first - end - # Returns the ID of the ghost user. def self.ghost_user_id key = 'github-import/ghost-user-id' diff --git a/lib/gitlab/github_import/bulk_importing.rb b/lib/gitlab/github_import/bulk_importing.rb index 147597289cf..da2f96b5c4b 100644 --- a/lib/gitlab/github_import/bulk_importing.rb +++ b/lib/gitlab/github_import/bulk_importing.rb @@ -15,10 +15,12 @@ module Gitlab end # Bulk inserts the given rows into the database. - def bulk_insert(model, rows, batch_size: 100) + def bulk_insert(model, rows, batch_size: 100, pre_hook: nil) rows.each_slice(batch_size) do |slice| + pre_hook.call(slice) if pre_hook Gitlab::Database.bulk_insert(model.table_name, slice) end + rows end end end diff --git a/lib/gitlab/github_import/importer/diff_note_importer.rb b/lib/gitlab/github_import/importer/diff_note_importer.rb index 8274f37d358..d562958e955 100644 --- a/lib/gitlab/github_import/importer/diff_note_importer.rb +++ b/lib/gitlab/github_import/importer/diff_note_importer.rb @@ -13,7 +13,7 @@ module Gitlab @note = note @project = project @client = client - @user_finder = UserFinder.new(project, client) + @user_finder = GithubImport::UserFinder.new(project, client) end def execute diff --git a/lib/gitlab/github_import/importer/issue_importer.rb b/lib/gitlab/github_import/importer/issue_importer.rb index 31fefebf787..4226eee85cc 100644 --- a/lib/gitlab/github_import/importer/issue_importer.rb +++ b/lib/gitlab/github_import/importer/issue_importer.rb @@ -4,6 +4,8 @@ module Gitlab module GithubImport module Importer class IssueImporter + include Gitlab::Import::DatabaseHelpers + attr_reader :project, :issue, :client, :user_finder, :milestone_finder, :issuable_finder @@ -19,7 +21,7 @@ module Gitlab @issue = issue @project = project @client = client - @user_finder = UserFinder.new(project, client) + @user_finder = GithubImport::UserFinder.new(project, client) @milestone_finder = MilestoneFinder.new(project) @issuable_finder = GithubImport::IssuableFinder.new(project, issue) end @@ -55,7 +57,11 @@ module Gitlab updated_at: issue.updated_at } - GithubImport.insert_and_return_id(attributes, project.issues) + insert_and_return_id(attributes, project.issues).tap do |id| + # We use .insert_and_return_id which effectively disables all callbacks. + # Trigger iid logic here to make sure we track internal id values consistently. + project.issues.find(id).ensure_project_iid! + end rescue ActiveRecord::InvalidForeignKey # It's possible the project has been deleted since scheduling this # job. In this case we'll just skip creating the issue. diff --git a/lib/gitlab/github_import/importer/milestones_importer.rb b/lib/gitlab/github_import/importer/milestones_importer.rb index c53480e828a..94eb9136b9a 100644 --- a/lib/gitlab/github_import/importer/milestones_importer.rb +++ b/lib/gitlab/github_import/importer/milestones_importer.rb @@ -17,10 +17,20 @@ module Gitlab end def execute - bulk_insert(Milestone, build_milestones) + # We insert records in bulk, by-passing any standard model callbacks. + # The pre_hook here makes sure we track internal ids consistently. + # Note this has to be called before performing an insert of a batch + # because we're outside a transaction scope here. + bulk_insert(Milestone, build_milestones, pre_hook: method(:track_greatest_iid)) build_milestones_cache end + def track_greatest_iid(slice) + greatest_iid = slice.max { |e| e[:iid] }[:iid] + + InternalId.track_greatest(nil, { project: project }, :milestones, greatest_iid, ->(_) { project.milestones.maximum(:iid) }) + end + def build_milestones build_database_rows(each_milestone) end diff --git a/lib/gitlab/github_import/importer/note_importer.rb b/lib/gitlab/github_import/importer/note_importer.rb index c890f2df360..2b06d1b3baf 100644 --- a/lib/gitlab/github_import/importer/note_importer.rb +++ b/lib/gitlab/github_import/importer/note_importer.rb @@ -13,7 +13,7 @@ module Gitlab @note = note @project = project @client = client - @user_finder = UserFinder.new(project, client) + @user_finder = GithubImport::UserFinder.new(project, client) end def execute diff --git a/lib/gitlab/github_import/importer/pull_request_importer.rb b/lib/gitlab/github_import/importer/pull_request_importer.rb index 6b3688c4381..ae7c4cf1b38 100644 --- a/lib/gitlab/github_import/importer/pull_request_importer.rb +++ b/lib/gitlab/github_import/importer/pull_request_importer.rb @@ -4,6 +4,8 @@ module Gitlab module GithubImport module Importer class PullRequestImporter + include Gitlab::Import::MergeRequestHelpers + attr_reader :pull_request, :project, :client, :user_finder, :milestone_finder, :issuable_finder @@ -15,7 +17,7 @@ module Gitlab @pull_request = pull_request @project = project @client = client - @user_finder = UserFinder.new(project, client) + @user_finder = GithubImport::UserFinder.new(project, client) @milestone_finder = MilestoneFinder.new(project) @issuable_finder = GithubImport::IssuableFinder.new(project, pull_request) @@ -44,75 +46,27 @@ module Gitlab description = MarkdownText .format(pull_request.description, pull_request.author, author_found) - # This work must be wrapped in a transaction as otherwise we can leave - # behind incomplete data in the event of an error. This can then lead - # to duplicate key errors when jobs are retried. - MergeRequest.transaction do - attributes = { - iid: pull_request.iid, - title: pull_request.truncated_title, - description: description, - source_project_id: project.id, - target_project_id: project.id, - source_branch: pull_request.formatted_source_branch, - target_branch: pull_request.target_branch, - state: pull_request.state, - milestone_id: milestone_finder.id_for(pull_request), - author_id: author_id, - assignee_id: user_finder.assignee_id_for(pull_request), - created_at: pull_request.created_at, - updated_at: pull_request.updated_at - } - - # When creating merge requests there are a lot of hooks that may - # run, for many different reasons. Many of these hooks (e.g. the - # ones used for rendering Markdown) are completely unnecessary and - # may even lead to transaction timeouts. - # - # To ensure importing pull requests has a minimal impact and can - # complete in a reasonable time we bypass all the hooks by inserting - # the row and then retrieving it. We then only perform the - # additional work that is strictly necessary. - merge_request_id = GithubImport - .insert_and_return_id(attributes, project.merge_requests) + attributes = { + iid: pull_request.iid, + title: pull_request.truncated_title, + description: description, + source_project_id: project.id, + target_project_id: project.id, + source_branch: pull_request.formatted_source_branch, + target_branch: pull_request.target_branch, + state: pull_request.state, + milestone_id: milestone_finder.id_for(pull_request), + author_id: author_id, + assignee_id: user_finder.assignee_id_for(pull_request), + created_at: pull_request.created_at, + updated_at: pull_request.updated_at + } - [project.merge_requests.find(merge_request_id), false] - end - rescue ActiveRecord::InvalidForeignKey - # It's possible the project has been deleted since scheduling this - # job. In this case we'll just skip creating the merge request. - [] - rescue ActiveRecord::RecordNotUnique - # It's possible we previously created the MR, but failed when updating - # the Git data. In this case we'll just continue working on the - # existing row. - [project.merge_requests.find_by(iid: pull_request.iid), true] + create_merge_request_without_hooks(project, attributes, pull_request.iid) end - def insert_git_data(merge_request, already_exists = false) - # These fields are set so we can create the correct merge request - # diffs. - merge_request.source_branch_sha = pull_request.source_branch_sha - merge_request.target_branch_sha = pull_request.target_branch_sha - - merge_request.keep_around_commit - - # MR diffs normally use an "after_save" hook to pull data from Git. - # All of this happens in the transaction started by calling - # create/save/etc. This in turn can lead to these transactions being - # held open for much longer than necessary. To work around this we - # first save the diff, then populate it. - diff = - if already_exists - merge_request.merge_request_diffs.take || - merge_request.merge_request_diffs.build - else - merge_request.merge_request_diffs.build - end - - diff.importing = true - diff.save - diff.save_git_content + def insert_git_data(merge_request, already_exists) + insert_or_replace_git_data(merge_request, pull_request.source_branch_sha, pull_request.target_branch_sha, already_exists) end end end diff --git a/lib/gitlab/hook_data/issuable_builder.rb b/lib/gitlab/hook_data/issuable_builder.rb index f2eda398b8f..65389835ad7 100644 --- a/lib/gitlab/hook_data/issuable_builder.rb +++ b/lib/gitlab/hook_data/issuable_builder.rb @@ -28,7 +28,7 @@ module Gitlab end def safe_keys - issuable_builder::SAFE_HOOK_ATTRIBUTES + issuable_builder::SAFE_HOOK_RELATIONS + issuable_builder.safe_hook_attributes + issuable_builder::SAFE_HOOK_RELATIONS end private diff --git a/lib/gitlab/hook_data/issue_builder.rb b/lib/gitlab/hook_data/issue_builder.rb index 0d71c748dc6..dd63db969f6 100644 --- a/lib/gitlab/hook_data/issue_builder.rb +++ b/lib/gitlab/hook_data/issue_builder.rb @@ -1,50 +1,52 @@ module Gitlab module HookData class IssueBuilder < BaseBuilder - SAFE_HOOK_ATTRIBUTES = %i[ - assignee_id - author_id - closed_at - confidential - created_at - description - due_date - id - iid - last_edited_at - last_edited_by_id - milestone_id - moved_to_id - project_id - relative_position - state - time_estimate - title - updated_at - updated_by_id - ].freeze - SAFE_HOOK_RELATIONS = %i[ assignees labels total_time_spent ].freeze + def self.safe_hook_attributes + %i[ + assignee_id + author_id + closed_at + confidential + created_at + description + due_date + id + iid + last_edited_at + last_edited_by_id + milestone_id + moved_to_id + project_id + relative_position + state + time_estimate + title + updated_at + updated_by_id + ].freeze + end + alias_method :issue, :object def build attrs = { - description: absolute_image_urls(issue.description), - url: Gitlab::UrlBuilder.build(issue), - total_time_spent: issue.total_time_spent, - human_total_time_spent: issue.human_total_time_spent, - human_time_estimate: issue.human_time_estimate, - assignee_ids: issue.assignee_ids, - assignee_id: issue.assignee_ids.first # This key is deprecated + description: absolute_image_urls(issue.description), + url: Gitlab::UrlBuilder.build(issue), + total_time_spent: issue.total_time_spent, + human_total_time_spent: issue.human_total_time_spent, + human_time_estimate: issue.human_time_estimate, + assignee_ids: issue.assignee_ids, + assignee_id: issue.assignee_ids.first # This key is deprecated } - issue.attributes.with_indifferent_access.slice(*SAFE_HOOK_ATTRIBUTES) - .merge!(attrs) + issue.attributes.with_indifferent_access.slice(*self.class.safe_hook_attributes) + .merge!(attrs) end end end diff --git a/lib/gitlab/hook_data/merge_request_builder.rb b/lib/gitlab/hook_data/merge_request_builder.rb index dfbed0597ed..3aa6a4f3767 100644 --- a/lib/gitlab/hook_data/merge_request_builder.rb +++ b/lib/gitlab/hook_data/merge_request_builder.rb @@ -1,33 +1,35 @@ module Gitlab module HookData class MergeRequestBuilder < BaseBuilder - SAFE_HOOK_ATTRIBUTES = %i[ - assignee_id - author_id - created_at - description - head_pipeline_id - id - iid - last_edited_at - last_edited_by_id - merge_commit_sha - merge_error - merge_params - merge_status - merge_user_id - merge_when_pipeline_succeeds - milestone_id - source_branch - source_project_id - state - target_branch - target_project_id - time_estimate - title - updated_at - updated_by_id - ].freeze + def self.safe_hook_attributes + %i[ + assignee_id + author_id + created_at + description + head_pipeline_id + id + iid + last_edited_at + last_edited_by_id + merge_commit_sha + merge_error + merge_params + merge_status + merge_user_id + merge_when_pipeline_succeeds + milestone_id + source_branch + source_project_id + state + target_branch + target_project_id + time_estimate + title + updated_at + updated_by_id + ].freeze + end SAFE_HOOK_RELATIONS = %i[ assignee @@ -50,8 +52,8 @@ module Gitlab human_time_estimate: merge_request.human_time_estimate } - merge_request.attributes.with_indifferent_access.slice(*SAFE_HOOK_ATTRIBUTES) - .merge!(attrs) + merge_request.attributes.with_indifferent_access.slice(*self.class.safe_hook_attributes) + .merge!(attrs) end end end diff --git a/lib/gitlab/i18n.rb b/lib/gitlab/i18n.rb index f67e92d3363..7346eab9e76 100644 --- a/lib/gitlab/i18n.rb +++ b/lib/gitlab/i18n.rb @@ -23,7 +23,8 @@ module Gitlab 'tr_TR' => 'Türkçe', 'id_ID' => 'Bahasa Indonesia', 'fil_PH' => 'Filipino', - 'pl_PL' => 'Polski' + 'pl_PL' => 'Polski', + 'cs_CZ' => 'Čeština' }.freeze def available_locales diff --git a/lib/gitlab/import/database_helpers.rb b/lib/gitlab/import/database_helpers.rb new file mode 100644 index 00000000000..80857061933 --- /dev/null +++ b/lib/gitlab/import/database_helpers.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +module Gitlab + module Import + module DatabaseHelpers + # Inserts a raw row and returns the ID of the inserted row. + # + # attributes - The attributes/columns to set. + # relation - An ActiveRecord::Relation to use for finding the ID of the row + # when using MySQL. + def insert_and_return_id(attributes, relation) + # We use bulk_insert here so we can bypass any queries executed by + # callbacks or validation rules, as doing this wouldn't scale when + # importing very large projects. + result = Gitlab::Database + .bulk_insert(relation.table_name, [attributes], return_ids: true) + + # MySQL doesn't support returning the IDs of a bulk insert in a way that + # is not a pain, so in this case we'll issue an extra query instead. + result.first || + relation.where(iid: attributes[:iid]).limit(1).pluck(:id).first + end + end + end +end diff --git a/lib/gitlab/import/merge_request_creator.rb b/lib/gitlab/import/merge_request_creator.rb new file mode 100644 index 00000000000..a01951b0762 --- /dev/null +++ b/lib/gitlab/import/merge_request_creator.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +# This module is designed for importers that need to create many merge +# requests quickly. When creating merge requests there are a lot of hooks +# that may run, for many different reasons. Many of these hooks (e.g. the ones +# used for rendering Markdown) are completely unnecessary and may even lead to +# transaction timeouts. +# +# To ensure importing merge requests requests has a minimal impact and can +# complete in a reasonable time we bypass all the hooks by inserting the row +# and then retrieving it. We then only perform the additional work that is +# strictly necessary. +module Gitlab + module Import + class MergeRequestCreator + include ::Gitlab::Import::DatabaseHelpers + include ::Gitlab::Import::MergeRequestHelpers + + attr_accessor :project + + def initialize(project) + @project = project + end + + def execute(attributes) + source_branch_sha = attributes.delete(:source_branch_sha) + target_branch_sha = attributes.delete(:target_branch_sha) + iid = attributes[:iid] + + merge_request, already_exists = create_merge_request_without_hooks(project, attributes, iid) + + if merge_request + insert_or_replace_git_data(merge_request, source_branch_sha, target_branch_sha, already_exists) + end + + merge_request + end + end + end +end diff --git a/lib/gitlab/import/merge_request_helpers.rb b/lib/gitlab/import/merge_request_helpers.rb new file mode 100644 index 00000000000..8ba70700dc1 --- /dev/null +++ b/lib/gitlab/import/merge_request_helpers.rb @@ -0,0 +1,70 @@ +# frozen_string_literal: true + +module Gitlab + module Import + module MergeRequestHelpers + include DatabaseHelpers + + def create_merge_request_without_hooks(project, attributes, iid) + # This work must be wrapped in a transaction as otherwise we can leave + # behind incomplete data in the event of an error. This can then lead + # to duplicate key errors when jobs are retried. + MergeRequest.transaction do + # When creating merge requests there are a lot of hooks that may + # run, for many different reasons. Many of these hooks (e.g. the + # ones used for rendering Markdown) are completely unnecessary and + # may even lead to transaction timeouts. + # + # To ensure importing pull requests has a minimal impact and can + # complete in a reasonable time we bypass all the hooks by inserting + # the row and then retrieving it. We then only perform the + # additional work that is strictly necessary. + merge_request_id = insert_and_return_id(attributes, project.merge_requests) + + merge_request = project.merge_requests.find(merge_request_id) + + # We use .insert_and_return_id which effectively disables all callbacks. + # Trigger iid logic here to make sure we track internal id values consistently. + merge_request.ensure_target_project_iid! + + [merge_request, false] + end + rescue ActiveRecord::InvalidForeignKey + # It's possible the project has been deleted since scheduling this + # job. In this case we'll just skip creating the merge request. + [] + rescue ActiveRecord::RecordNotUnique + # It's possible we previously created the MR, but failed when updating + # the Git data. In this case we'll just continue working on the + # existing row. + [project.merge_requests.find_by(iid: iid), true] + end + + def insert_or_replace_git_data(merge_request, source_branch_sha, target_branch_sha, already_exists = false) + # These fields are set so we can create the correct merge request + # diffs. + merge_request.source_branch_sha = source_branch_sha + merge_request.target_branch_sha = target_branch_sha + + merge_request.keep_around_commit + + # MR diffs normally use an "after_save" hook to pull data from Git. + # All of this happens in the transaction started by calling + # create/save/etc. This in turn can lead to these transactions being + # held open for much longer than necessary. To work around this we + # first save the diff, then populate it. + diff = + if already_exists + merge_request.merge_request_diffs.take || + merge_request.merge_request_diffs.build + else + merge_request.merge_request_diffs.build + end + + diff.importing = true + diff.save + diff.save_git_content + end + end + end +end diff --git a/lib/gitlab/import_export/members_mapper.rb b/lib/gitlab/import_export/members_mapper.rb index ac827cbe1ca..bcbaf00e11b 100644 --- a/lib/gitlab/import_export/members_mapper.rb +++ b/lib/gitlab/import_export/members_mapper.rb @@ -45,7 +45,7 @@ module Gitlab end def ensure_default_member! - @project.project_members.destroy_all + @project.project_members.destroy_all # rubocop: disable DestroyAll ProjectMember.create!(user: @user, access_level: ProjectMember::MAINTAINER, source_id: @project.id, importing: true) end diff --git a/lib/gitlab/middleware/multipart.rb b/lib/gitlab/middleware/multipart.rb index 18f91db98fc..3d588918adf 100644 --- a/lib/gitlab/middleware/multipart.rb +++ b/lib/gitlab/middleware/multipart.rb @@ -82,9 +82,13 @@ module Gitlab end def open_file(params, key) - ::UploadedFile.from_params( - params, key, - [FileUploader.root, Gitlab.config.uploads.storage_path]) + allowed_paths = [ + FileUploader.root, + Gitlab.config.uploads.storage_path, + File.join(Rails.root, 'public/uploads/tmp') + ] + + ::UploadedFile.from_params(params, key, allowed_paths) end end diff --git a/lib/gitlab/path_regex.rb b/lib/gitlab/path_regex.rb index 61653044433..359dd2bcbc7 100644 --- a/lib/gitlab/path_regex.rb +++ b/lib/gitlab/path_regex.rb @@ -40,6 +40,7 @@ module Gitlab invites jwt koding + login notification_settings oauth profile diff --git a/lib/gitlab/setup_helper.rb b/lib/gitlab/setup_helper.rb index b2d75aac1d0..5b68e4470cd 100644 --- a/lib/gitlab/setup_helper.rb +++ b/lib/gitlab/setup_helper.rb @@ -1,3 +1,5 @@ +require 'toml-rb' + module Gitlab module SetupHelper class << self @@ -9,7 +11,7 @@ module Gitlab # because it uses a Unix socket. # For development and testing purposes, an extra storage is added to gitaly, # which is not known to Rails, but must be explicitly stubbed. - def gitaly_configuration_toml(gitaly_dir, gitaly_ruby: true) + def gitaly_configuration_toml(gitaly_dir, storage_paths, gitaly_ruby: true) storages = [] address = nil @@ -24,10 +26,7 @@ module Gitlab address = val['gitaly_address'] end - # https://gitlab.com/gitlab-org/gitaly/issues/1238 - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - storages << { name: key, path: val.legacy_disk_path } - end + storages << { name: key, path: storage_paths[key] } end if Rails.env.test? @@ -44,12 +43,12 @@ module Gitlab end # rubocop:disable Rails/Output - def create_gitaly_configuration(dir, force: false) + def create_gitaly_configuration(dir, storage_paths, force: false) config_path = File.join(dir, 'config.toml') FileUtils.rm_f(config_path) if force File.open(config_path, File::WRONLY | File::CREAT | File::EXCL) do |f| - f.puts gitaly_configuration_toml(dir) + f.puts gitaly_configuration_toml(dir, storage_paths) end rescue Errno::EEXIST puts "Skipping config.toml generation:" diff --git a/lib/gitlab/template/finders/base_template_finder.rb b/lib/gitlab/template/finders/base_template_finder.rb index 473b05257c6..a5105439b12 100644 --- a/lib/gitlab/template/finders/base_template_finder.rb +++ b/lib/gitlab/template/finders/base_template_finder.rb @@ -21,7 +21,7 @@ module Gitlab def category_directory(category) return @base_dir unless category.present? - @base_dir + @categories[category] + File.join(@base_dir, @categories[category]) end class << self diff --git a/lib/gitlab/template/finders/repo_template_finder.rb b/lib/gitlab/template/finders/repo_template_finder.rb index 33f07fa0120..29bc2393ff9 100644 --- a/lib/gitlab/template/finders/repo_template_finder.rb +++ b/lib/gitlab/template/finders/repo_template_finder.rb @@ -27,7 +27,7 @@ module Gitlab directory = select_directory(file_name) raise FileNotFoundError if directory.nil? - category_directory(directory) + file_name + File.join(category_directory(directory), file_name) end def list_files_for(dir) @@ -37,8 +37,8 @@ module Gitlab entries = @repository.tree(:head, dir).entries - names = entries.map(&:name) - names.select { |f| f =~ self.class.filter_regex(@extension) } + paths = entries.map(&:path) + paths.select { |f| f =~ self.class.filter_regex(@extension) } end private @@ -47,10 +47,10 @@ module Gitlab return [] unless @commit # Insert root as directory - directories = ["", @categories.keys] + directories = ["", *@categories.keys] directories.find do |category| - path = category_directory(category) + file_name + path = File.join(category_directory(category), file_name) @repository.blob_at(@commit.id, path) end end diff --git a/lib/gitlab/usage_data.rb b/lib/gitlab/usage_data.rb index 22c9638ecc0..7797bd5fab2 100644 --- a/lib/gitlab/usage_data.rb +++ b/lib/gitlab/usage_data.rb @@ -34,6 +34,7 @@ module Gitlab def system_usage_data { counts: { + assignee_lists: List.assignee.count, boards: Board.count, ci_builds: ::Ci::Build.count, ci_internal_pipelines: ::Ci::Pipeline.internal.count, @@ -61,9 +62,11 @@ module Gitlab groups: Group.count, issues: Issue.count, keys: Key.count, + label_lists: List.label.count, labels: Label.count, lfs_objects: LfsObject.count, merge_requests: MergeRequest.count, + milestone_lists: List.milestone.count, milestones: Milestone.count, notes: Note.count, pages_domains: PagesDomain.count, diff --git a/lib/tasks/gitlab/gitaly.rake b/lib/tasks/gitlab/gitaly.rake index e9ca6404fe8..80de3d2ef51 100644 --- a/lib/tasks/gitlab/gitaly.rake +++ b/lib/tasks/gitlab/gitaly.rake @@ -1,13 +1,12 @@ namespace :gitlab do namespace :gitaly do desc "GitLab | Install or upgrade gitaly" - task :install, [:dir, :repo] => :gitlab_environment do |t, args| - require 'toml-rb' - + task :install, [:dir, :storage_path, :repo] => :gitlab_environment do |t, args| warn_user_is_not_gitlab - unless args.dir.present? - abort %(Please specify the directory where you want to install gitaly:\n rake "gitlab:gitaly:install[/home/git/gitaly]") + unless args.dir.present? && args.storage_path.present? + abort %(Please specify the directory where you want to install gitaly and the path for the default storage +Usage: rake "gitlab:gitaly:install[/installation/dir,/storage/path]") end args.with_defaults(repo: 'https://gitlab.com/gitlab-org/gitaly.git') @@ -27,7 +26,8 @@ namespace :gitlab do "BUNDLE_PATH=#{Bundler.bundle_path}") end - Gitlab::SetupHelper.create_gitaly_configuration(args.dir) + storage_paths = { 'default' => args.storage_path } + Gitlab::SetupHelper.create_gitaly_configuration(args.dir, storage_paths) Dir.chdir(args.dir) do # In CI we run scripts/gitaly-test-build instead of this command unless ENV['CI'].present? @@ -35,17 +35,5 @@ namespace :gitlab do end end end - - desc "GitLab | Print storage configuration in TOML format" - task storage_config: :environment do - require 'toml-rb' - - puts "# Gitaly storage configuration generated from #{Gitlab.config.source} on #{Time.current.to_s(:long)}" - puts "# This is in TOML format suitable for use in Gitaly's config.toml file." - - # Exclude gitaly-ruby configuration because that depends on the gitaly - # installation directory. - puts Gitlab::SetupHelper.gitaly_configuration_toml('', gitaly_ruby: false) - end end end diff --git a/lib/tasks/gitlab/site_statistics.rake b/lib/tasks/gitlab/site_statistics.rake new file mode 100644 index 00000000000..7d24ec72a9d --- /dev/null +++ b/lib/tasks/gitlab/site_statistics.rake @@ -0,0 +1,23 @@ +namespace :gitlab do + desc "GitLab | Refresh Site Statistics counters" + task refresh_site_statistics: :environment do + puts 'Updating Site Statistics counters: ' + + print '* Repositories... ' + SiteStatistic.transaction do + # see https://gitlab.com/gitlab-org/gitlab-ce/issues/48967 + ActiveRecord::Base.connection.execute('SET LOCAL statement_timeout TO 0') if Gitlab::Database.postgresql? + SiteStatistic.update_all('repositories_count = (SELECT COUNT(*) FROM projects)') + end + puts 'OK!'.color(:green) + + print '* Wikis... ' + SiteStatistic.transaction do + # see https://gitlab.com/gitlab-org/gitlab-ce/issues/48967 + ActiveRecord::Base.connection.execute('SET LOCAL statement_timeout TO 0') if Gitlab::Database.postgresql? + SiteStatistic.update_all('wikis_count = (SELECT COUNT(*) FROM project_features WHERE wiki_access_level != 0)') + end + puts 'OK!'.color(:green) + puts + end +end diff --git a/lib/tasks/gitlab/traces.rake b/lib/tasks/gitlab/traces.rake index ddcca69711f..5a232091a7e 100644 --- a/lib/tasks/gitlab/traces.rake +++ b/lib/tasks/gitlab/traces.rake @@ -18,5 +18,22 @@ namespace :gitlab do logger.info("Scheduled #{job_ids.count} jobs. From #{job_ids.min} to #{job_ids.max}") end end + + task migrate: :environment do + logger = Logger.new(STDOUT) + logger.info('Starting transfer of job traces') + + Ci::Build.joins(:project) + .with_archived_trace_stored_locally + .find_each(batch_size: 10) do |build| + begin + build.job_artifacts_trace.file.migrate!(ObjectStorage::Store::REMOTE) + + logger.info("Transferred job trace of #{build.id} to object storage") + rescue => e + logger.error("Failed to transfer artifacts of #{build.id} with error: #{e.message}") + end + end + end end end |