From df3ca41e62efb7ccbc1ea82d5676527e8eeca530 Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Fri, 22 Jul 2016 15:45:38 +0200 Subject: Remove obsolete code --- lib/gitlab/backend/grack_auth.rb | 163 ------------------- lib/gitlab/lfs/response.rb | 329 --------------------------------------- lib/gitlab/lfs/router.rb | 98 ------------ 3 files changed, 590 deletions(-) delete mode 100644 lib/gitlab/backend/grack_auth.rb delete mode 100644 lib/gitlab/lfs/response.rb delete mode 100644 lib/gitlab/lfs/router.rb (limited to 'lib') diff --git a/lib/gitlab/backend/grack_auth.rb b/lib/gitlab/backend/grack_auth.rb deleted file mode 100644 index ab94abeda77..00000000000 --- a/lib/gitlab/backend/grack_auth.rb +++ /dev/null @@ -1,163 +0,0 @@ -module Grack - class AuthSpawner - def self.call(env) - # Avoid issues with instance variables in Grack::Auth persisting across - # requests by creating a new instance for each request. - Auth.new({}).call(env) - end - end - - class Auth < Rack::Auth::Basic - attr_accessor :user, :project, :env - - def call(env) - @env = env - @request = Rack::Request.new(env) - @auth = Request.new(env) - - @ci = false - - # Need this patch due to the rails mount - # Need this if under RELATIVE_URL_ROOT - unless Gitlab.config.gitlab.relative_url_root.empty? - # If website is mounted using relative_url_root need to remove it first - @env['PATH_INFO'] = @request.path.sub(Gitlab.config.gitlab.relative_url_root, '') - else - @env['PATH_INFO'] = @request.path - end - - @env['SCRIPT_NAME'] = "" - - auth! - - lfs_response = Gitlab::Lfs::Router.new(project, @user, @ci, @request).try_call - return lfs_response unless lfs_response.nil? - - if @user.nil? && !@ci - unauthorized - else - render_not_found - end - end - - private - - def auth! - return unless @auth.provided? - - return bad_request unless @auth.basic? - - # Authentication with username and password - login, password = @auth.credentials - - # Allow authentication for GitLab CI service - # if valid token passed - if ci_request?(login, password) - @ci = true - return - end - - @user = authenticate_user(login, password) - end - - def ci_request?(login, password) - matched_login = /(?^[a-zA-Z]*-ci)-token$/.match(login) - - if project && matched_login.present? - underscored_service = matched_login['s'].underscore - - if underscored_service == 'gitlab_ci' - return project && project.valid_build_token?(password) - elsif Service.available_services_names.include?(underscored_service) - service_method = "#{underscored_service}_service" - service = project.send(service_method) - - return service && service.activated? && service.valid_token?(password) - end - end - - false - end - - def oauth_access_token_check(login, password) - if login == "oauth2" && git_cmd == 'git-upload-pack' && password.present? - token = Doorkeeper::AccessToken.by_token(password) - token && token.accessible? && User.find_by(id: token.resource_owner_id) - end - end - - def authenticate_user(login, password) - user = Gitlab::Auth.find_with_user_password(login, password) - - unless user - user = oauth_access_token_check(login, password) - end - - # If the user authenticated successfully, we reset the auth failure count - # from Rack::Attack for that IP. A client may attempt to authenticate - # with a username and blank password first, and only after it receives - # a 401 error does it present a password. Resetting the count prevents - # false positives from occurring. - # - # Otherwise, we let Rack::Attack know there was a failed authentication - # attempt from this IP. This information is stored in the Rails cache - # (Redis) and will be used by the Rack::Attack middleware to decide - # whether to block requests from this IP. - config = Gitlab.config.rack_attack.git_basic_auth - - if config.enabled - if user - # A successful login will reset the auth failure count from this IP - Rack::Attack::Allow2Ban.reset(@request.ip, config) - else - banned = Rack::Attack::Allow2Ban.filter(@request.ip, config) do - # Unless the IP is whitelisted, return true so that Allow2Ban - # increments the counter (stored in Rails.cache) for the IP - if config.ip_whitelist.include?(@request.ip) - false - else - true - end - end - - if banned - Rails.logger.info "IP #{@request.ip} failed to login " \ - "as #{login} but has been temporarily banned from Git auth" - end - end - end - - user - end - - def git_cmd - if @request.get? - @request.params['service'] - elsif @request.post? - File.basename(@request.path) - else - nil - end - end - - def project - return @project if defined?(@project) - - @project = project_by_path(@request.path_info) - end - - def project_by_path(path) - if m = /^([\w\.\/-]+)\.git/.match(path).to_a - path_with_namespace = m.last - path_with_namespace.gsub!(/\.wiki$/, '') - - path_with_namespace[0] = '' if path_with_namespace.start_with?('/') - Project.find_with_namespace(path_with_namespace) - end - end - - def render_not_found - [404, { "Content-Type" => "text/plain" }, ["Not Found"]] - end - end -end diff --git a/lib/gitlab/lfs/response.rb b/lib/gitlab/lfs/response.rb deleted file mode 100644 index a1ee1aa81ff..00000000000 --- a/lib/gitlab/lfs/response.rb +++ /dev/null @@ -1,329 +0,0 @@ -module Gitlab - module Lfs - class Response - def initialize(project, user, ci, request) - @origin_project = project - @project = storage_project(project) - @user = user - @ci = ci - @env = request.env - @request = request - end - - def render_download_object_response(oid) - render_response_to_download do - if check_download_sendfile_header? - render_lfs_sendfile(oid) - else - render_not_found - end - end - end - - def render_batch_operation_response - request_body = JSON.parse(@request.body.read) - case request_body["operation"] - when "download" - render_batch_download(request_body) - when "upload" - render_batch_upload(request_body) - else - render_not_found - end - end - - def render_storage_upload_authorize_response(oid, size) - render_response_to_push do - [ - 200, - { "Content-Type" => "application/json; charset=utf-8" }, - [JSON.dump({ - 'StoreLFSPath' => "#{Gitlab.config.lfs.storage_path}/tmp/upload", - 'LfsOid' => oid, - 'LfsSize' => size - })] - ] - end - end - - def render_storage_upload_store_response(oid, size, tmp_file_name) - return render_forbidden unless tmp_file_name - - render_response_to_push do - render_lfs_upload_ok(oid, size, tmp_file_name) - end - end - - def render_unsupported_deprecated_api - [ - 501, - { "Content-Type" => "application/json; charset=utf-8" }, - [JSON.dump({ - 'message' => 'Server supports batch API only, please update your Git LFS client to version 1.0.1 and up.', - 'documentation_url' => "#{Gitlab.config.gitlab.url}/help", - })] - ] - end - - private - - def render_not_enabled - [ - 501, - { - "Content-Type" => "application/json; charset=utf-8", - }, - [JSON.dump({ - 'message' => 'Git LFS is not enabled on this GitLab server, contact your admin.', - 'documentation_url' => "#{Gitlab.config.gitlab.url}/help", - })] - ] - end - - def render_unauthorized - [ - 401, - { - 'Content-Type' => 'text/plain' - }, - ['Unauthorized'] - ] - end - - def render_not_found - [ - 404, - { - "Content-Type" => "application/vnd.git-lfs+json" - }, - [JSON.dump({ - 'message' => 'Not found.', - 'documentation_url' => "#{Gitlab.config.gitlab.url}/help", - })] - ] - end - - def render_forbidden - [ - 403, - { - "Content-Type" => "application/vnd.git-lfs+json" - }, - [JSON.dump({ - 'message' => 'Access forbidden. Check your access level.', - 'documentation_url' => "#{Gitlab.config.gitlab.url}/help", - })] - ] - end - - def render_lfs_sendfile(oid) - return render_not_found unless oid.present? - - lfs_object = object_for_download(oid) - - if lfs_object && lfs_object.file.exists? - [ - 200, - { - # GitLab-workhorse will forward Content-Type header - "Content-Type" => "application/octet-stream", - "X-Sendfile" => lfs_object.file.path - }, - [] - ] - else - render_not_found - end - end - - def render_batch_upload(body) - return render_not_found if body.empty? || body['objects'].nil? - - render_response_to_push do - response = build_upload_batch_response(body['objects']) - [ - 200, - { - "Content-Type" => "application/json; charset=utf-8", - "Cache-Control" => "private", - }, - [JSON.dump(response)] - ] - end - end - - def render_batch_download(body) - return render_not_found if body.empty? || body['objects'].nil? - - render_response_to_download do - response = build_download_batch_response(body['objects']) - [ - 200, - { - "Content-Type" => "application/json; charset=utf-8", - "Cache-Control" => "private", - }, - [JSON.dump(response)] - ] - end - end - - def render_lfs_upload_ok(oid, size, tmp_file) - if store_file(oid, size, tmp_file) - [ - 200, - { - 'Content-Type' => 'text/plain', - 'Content-Length' => 0 - }, - [] - ] - else - [ - 422, - { 'Content-Type' => 'text/plain' }, - ["Unprocessable entity"] - ] - end - end - - def render_response_to_download - return render_not_enabled unless Gitlab.config.lfs.enabled - - unless @project.public? - return render_unauthorized unless @user || @ci - return render_forbidden unless user_can_fetch? - end - - yield - end - - def render_response_to_push - return render_not_enabled unless Gitlab.config.lfs.enabled - return render_unauthorized unless @user - return render_forbidden unless user_can_push? - - yield - end - - def check_download_sendfile_header? - @env['HTTP_X_SENDFILE_TYPE'].to_s == "X-Sendfile" - end - - def user_can_fetch? - # Check user access against the project they used to initiate the pull - @ci || @user.can?(:download_code, @origin_project) - end - - def user_can_push? - # Check user access against the project they used to initiate the push - @user.can?(:push_code, @origin_project) - end - - def storage_project(project) - if project.forked? - storage_project(project.forked_from_project) - else - project - end - end - - def store_file(oid, size, tmp_file) - tmp_file_path = File.join("#{Gitlab.config.lfs.storage_path}/tmp/upload", tmp_file) - - object = LfsObject.find_or_create_by(oid: oid, size: size) - if object.file.exists? - success = true - else - success = move_tmp_file_to_storage(object, tmp_file_path) - end - - if success - success = link_to_project(object) - end - - success - ensure - # Ensure that the tmp file is removed - FileUtils.rm_f(tmp_file_path) - end - - def object_for_download(oid) - @project.lfs_objects.find_by(oid: oid) - end - - def move_tmp_file_to_storage(object, path) - File.open(path) do |f| - object.file = f - end - - object.file.store! - object.save - end - - def link_to_project(object) - if object && !object.projects.exists?(@project.id) - object.projects << @project - object.save - end - end - - def select_existing_objects(objects) - objects_oids = objects.map { |o| o['oid'] } - @project.lfs_objects.where(oid: objects_oids).pluck(:oid).to_set - end - - def build_upload_batch_response(objects) - selected_objects = select_existing_objects(objects) - - upload_hypermedia_links(objects, selected_objects) - end - - def build_download_batch_response(objects) - selected_objects = select_existing_objects(objects) - - download_hypermedia_links(objects, selected_objects) - end - - def download_hypermedia_links(all_objects, existing_objects) - all_objects.each do |object| - if existing_objects.include?(object['oid']) - object['actions'] = { - 'download' => { - 'href' => "#{@origin_project.http_url_to_repo}/gitlab-lfs/objects/#{object['oid']}", - 'header' => { - 'Authorization' => @env['HTTP_AUTHORIZATION'] - }.compact - } - } - else - object['error'] = { - 'code' => 404, - 'message' => "Object does not exist on the server or you don't have permissions to access it", - } - end - end - - { 'objects' => all_objects } - end - - def upload_hypermedia_links(all_objects, existing_objects) - all_objects.each do |object| - # generate actions only for non-existing objects - next if existing_objects.include?(object['oid']) - - object['actions'] = { - 'upload' => { - 'href' => "#{@origin_project.http_url_to_repo}/gitlab-lfs/objects/#{object['oid']}/#{object['size']}", - 'header' => { - 'Authorization' => @env['HTTP_AUTHORIZATION'] - }.compact - } - } - end - - { 'objects' => all_objects } - end - end - end -end diff --git a/lib/gitlab/lfs/router.rb b/lib/gitlab/lfs/router.rb deleted file mode 100644 index f2a76a56b8f..00000000000 --- a/lib/gitlab/lfs/router.rb +++ /dev/null @@ -1,98 +0,0 @@ -module Gitlab - module Lfs - class Router - attr_reader :project, :user, :ci, :request - - def initialize(project, user, ci, request) - @project = project - @user = user - @ci = ci - @env = request.env - @request = request - end - - def try_call - return unless @request && @request.path.present? - - case @request.request_method - when 'GET' - get_response - when 'POST' - post_response - when 'PUT' - put_response - else - nil - end - end - - private - - def get_response - path_match = @request.path.match(/\/(info\/lfs|gitlab-lfs)\/objects\/([0-9a-f]{64})$/) - return nil unless path_match - - oid = path_match[2] - return nil unless oid - - case path_match[1] - when "info/lfs" - lfs.render_unsupported_deprecated_api - when "gitlab-lfs" - lfs.render_download_object_response(oid) - else - nil - end - end - - def post_response - post_path = @request.path.match(/\/info\/lfs\/objects(\/batch)?$/) - return nil unless post_path - - # Check for Batch API - if post_path[0].ends_with?("/info/lfs/objects/batch") - lfs.render_batch_operation_response - elsif post_path[0].ends_with?("/info/lfs/objects") - lfs.render_unsupported_deprecated_api - else - nil - end - end - - def put_response - object_match = @request.path.match(/\/gitlab-lfs\/objects\/([0-9a-f]{64})\/([0-9]+)(|\/authorize){1}$/) - return nil if object_match.nil? - - oid = object_match[1] - size = object_match[2].try(:to_i) - return nil if oid.nil? || size.nil? - - # GitLab-workhorse requests - # 1. Try to authorize the request - # 2. send a request with a header containing the name of the temporary file - if object_match[3] && object_match[3] == '/authorize' - lfs.render_storage_upload_authorize_response(oid, size) - else - tmp_file_name = sanitize_tmp_filename(@request.env['HTTP_X_GITLAB_LFS_TMP']) - lfs.render_storage_upload_store_response(oid, size, tmp_file_name) - end - end - - def lfs - return unless @project - - Gitlab::Lfs::Response.new(@project, @user, @ci, @request) - end - - def sanitize_tmp_filename(name) - if name.present? - name.gsub!(/^.*(\\|\/)/, '') - name = name.match(/[0-9a-f]{73}/) - name[0] if name - else - nil - end - end - end - end -end -- cgit v1.2.1 From 2e6085d8af37d77e550331595ca8a1fc8f2af49f Mon Sep 17 00:00:00 2001 From: Elliot Wiltshire Date: Mon, 25 Jul 2016 14:46:40 -0700 Subject: Fixing scope issue in GitAccess. --- lib/gitlab/git_access.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb index 8e8f39d9cb2..d321a129292 100644 --- a/lib/gitlab/git_access.rb +++ b/lib/gitlab/git_access.rb @@ -134,7 +134,7 @@ module Gitlab end def build_status_object(status, message = '') - GitAccessStatus.new(status, message) + Gitlab::GitAccessStatus.new(status, message) end end end -- cgit v1.2.1 From 9ae1ecf876e40ce9dd64c72e025f32e38c882fd6 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 28 Jul 2016 14:35:02 +0200 Subject: Extract build status badge metadata to separate class --- lib/gitlab/badge/build.rb | 25 ++++--------------------- lib/gitlab/badge/build/metadata.rb | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 21 deletions(-) create mode 100644 lib/gitlab/badge/build/metadata.rb (limited to 'lib') diff --git a/lib/gitlab/badge/build.rb b/lib/gitlab/badge/build.rb index e5e9fab3f5c..21d60854bf5 100644 --- a/lib/gitlab/badge/build.rb +++ b/lib/gitlab/badge/build.rb @@ -4,15 +4,15 @@ module Gitlab # Build badge # class Build - include Gitlab::Application.routes.url_helpers - include ActionView::Helpers::AssetTagHelper - include ActionView::Helpers::UrlHelper - def initialize(project, ref) @project, @ref = project, ref @image = ::Ci::ImageForBuildService.new.execute(project, ref: ref) end + def metadata + Build::Metadata.new(@project, @ref) + end + def type 'image/svg+xml' end @@ -24,23 +24,6 @@ module Gitlab def to_s @image[:name].sub(/\.svg$/, '') end - - def to_html - link_to(image_tag(image_url, alt: 'build status'), link_url) - end - - def to_markdown - "[![build status](#{image_url})](#{link_url})" - end - - def image_url - build_namespace_project_badges_url(@project.namespace, - @project, @ref, format: :svg) - end - - def link_url - namespace_project_commits_url(@project.namespace, @project, id: @ref) - end end end end diff --git a/lib/gitlab/badge/build/metadata.rb b/lib/gitlab/badge/build/metadata.rb new file mode 100644 index 00000000000..553ef8d7b16 --- /dev/null +++ b/lib/gitlab/badge/build/metadata.rb @@ -0,0 +1,36 @@ +module Gitlab + module Badge + class Build + ## + # Class that describes build badge metadata + # + class Metadata + include Gitlab::Application.routes.url_helpers + include ActionView::Helpers::AssetTagHelper + include ActionView::Helpers::UrlHelper + + def initialize(project, ref) + @project = project + @ref = ref + end + + def to_html + link_to(image_tag(image_url, alt: 'build status'), link_url) + end + + def to_markdown + "[![build status](#{image_url})](#{link_url})" + end + + def image_url + build_namespace_project_badges_url(@project.namespace, + @project, @ref, format: :svg) + end + + def link_url + namespace_project_commits_url(@project.namespace, @project, id: @ref) + end + end + end + end +end -- cgit v1.2.1 From 0c4fa8619ca477c0a78c825df8dd38cd2a109644 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 28 Jul 2016 14:58:53 +0200 Subject: Calculate build status only in build badge class --- lib/gitlab/badge/build.rb | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/badge/build.rb b/lib/gitlab/badge/build.rb index 21d60854bf5..7bc6f285ce1 100644 --- a/lib/gitlab/badge/build.rb +++ b/lib/gitlab/badge/build.rb @@ -5,8 +5,16 @@ module Gitlab # class Build def initialize(project, ref) - @project, @ref = project, ref - @image = ::Ci::ImageForBuildService.new.execute(project, ref: ref) + @project = project + @ref = ref + end + + def status + sha = @project.commit(@ref).try(:sha) + + @project.pipelines + .where(sha: sha, ref: @ref) + .status || 'unknown' end def metadata @@ -18,11 +26,9 @@ module Gitlab end def data - File.read(@image[:path]) - end - - def to_s - @image[:name].sub(/\.svg$/, '') + File.read( + Rails.root.join('public/ci', 'build-' + status + '.svg') + ) end end end -- cgit v1.2.1 From 503c44ee2a66e3e160a9ca9c02aba14a8aa7e310 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 28 Jul 2016 15:30:05 +0200 Subject: Add badge template class to use with SVG ERB template --- lib/gitlab/badge/build/template.rb | 63 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 lib/gitlab/badge/build/template.rb (limited to 'lib') diff --git a/lib/gitlab/badge/build/template.rb b/lib/gitlab/badge/build/template.rb new file mode 100644 index 00000000000..a7c2e176935 --- /dev/null +++ b/lib/gitlab/badge/build/template.rb @@ -0,0 +1,63 @@ +module Gitlab + module Badge + class Build + ## + # Abstract class for build badge template. + # + # Template object will be passed to badge.svg.erb template. + # + class Template + STATUS_COLOR = { + success: '#4c1', + failed: '#e05d44', + running: '#dfb317', + pending: '#dfb317', + canceled: '#9f9f9f', + skipped: '#9f9f9f', + unknown: '#9f9f9f' + } + + def initialize(status) + @status = status + end + + def key_text + 'build' + end + + def value_text + @status + end + + def key_width + 38 + end + + def value_width + 54 + end + + def key_color + '#555' + end + + def value_color + STATUS_COLOR[@status.to_sym] || + STATUS_COLOR[:unknown] + end + + def key_text_anchor + key_width / 2 + end + + def value_text_anchor + key_width + (value_width / 2) + end + + def width + key_width + value_width + end + end + end + end +end -- cgit v1.2.1 From ab0aedef5b5b41135ce28490cedfaab13095f650 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Tue, 2 Aug 2016 11:56:47 +0200 Subject: Always compare with FETCH_HEAD in downtime_check This ensures this CI step works properly even when doing a shallow clone. --- lib/tasks/downtime_check.rake | 26 ++++++-------------------- 1 file changed, 6 insertions(+), 20 deletions(-) (limited to 'lib') diff --git a/lib/tasks/downtime_check.rake b/lib/tasks/downtime_check.rake index 30a2e9be5ce..afe5d42910c 100644 --- a/lib/tasks/downtime_check.rake +++ b/lib/tasks/downtime_check.rake @@ -1,26 +1,12 @@ desc 'Checks if migrations in a branch require downtime' task downtime_check: :environment do - # First we'll want to make sure we're comparing with the right upstream - # repository/branch. - current_branch = `git rev-parse --abbrev-ref HEAD`.strip - - # Either the developer ran this task directly on the master branch, or they're - # making changes directly on the master branch. - if current_branch == 'master' - if defined?(Gitlab::License) - repo = 'gitlab-ee' - else - repo = 'gitlab-ce' - end - - `git fetch https://gitlab.com/gitlab-org/#{repo}.git --depth 1` - - compare_with = 'FETCH_HEAD' - # The developer is working on a different branch, in this case we can just - # compare with the master branch. + if defined?(Gitlab::License) + repo = 'gitlab-ee' else - compare_with = 'master' + repo = 'gitlab-ce' end - Rake::Task['gitlab:db:downtime_check'].invoke(compare_with) + `git fetch https://gitlab.com/gitlab-org/#{repo}.git --depth 1` + + Rake::Task['gitlab:db:downtime_check'].invoke('FETCH_HEAD') end -- cgit v1.2.1 From e63eccf9744de0965d4727326a4b30f1fe8165e8 Mon Sep 17 00:00:00 2001 From: winniehell Date: Mon, 1 Aug 2016 02:43:29 +0200 Subject: Do not look up commit again when it is passed to RelativeLinkFilter (!5455) --- lib/banzai/filter/relative_link_filter.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'lib') diff --git a/lib/banzai/filter/relative_link_filter.rb b/lib/banzai/filter/relative_link_filter.rb index 5b73fc8fcee..7d172ccdb8f 100644 --- a/lib/banzai/filter/relative_link_filter.rb +++ b/lib/banzai/filter/relative_link_filter.rb @@ -51,7 +51,7 @@ module Banzai relative_url_root, context[:project].path_with_namespace, uri_type(file_path), - ref || context[:project].default_branch, # if no ref exists, point to the default branch + ref, file_path ].compact.join('/').squeeze('/').chomp('/') @@ -115,7 +115,7 @@ module Banzai end def current_commit - @current_commit ||= context[:commit] || ref ? repository.commit(ref) : repository.head_commit + @current_commit ||= context[:commit] || repository.commit(ref) end def relative_url_root @@ -123,7 +123,7 @@ module Banzai end def ref - context[:ref] + context[:ref] || context[:project].default_branch end def repository -- cgit v1.2.1 From cd7c2cb6ddd4d9c9f9bdae00c887c0022c121c17 Mon Sep 17 00:00:00 2001 From: Paco Guzman Date: Wed, 20 Jul 2016 18:25:36 +0200 Subject: Cache highlighted diff lines for merge requests Introducing the concept of SafeDiffs which relates diffs with UI highlighting. --- lib/gitlab/diff/file.rb | 5 ++++- lib/gitlab/diff/highlight.rb | 7 ++++--- lib/gitlab/diff/line.rb | 14 ++++++++++++++ lib/gitlab/email/message/repository_push.rb | 2 +- 4 files changed, 23 insertions(+), 5 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/diff/file.rb b/lib/gitlab/diff/file.rb index b09ca1fb8b0..77b3798d78f 100644 --- a/lib/gitlab/diff/file.rb +++ b/lib/gitlab/diff/file.rb @@ -63,15 +63,18 @@ module Gitlab diff_refs.try(:head_sha) end + attr_writer :diff_lines, :highlighted_diff_lines + # Array of Gitlab::Diff::Line objects def diff_lines - @lines ||= Gitlab::Diff::Parser.new.parse(raw_diff.each_line).to_a + @diff_lines ||= Gitlab::Diff::Parser.new.parse(raw_diff.each_line).to_a end def highlighted_diff_lines @highlighted_diff_lines ||= Gitlab::Diff::Highlight.new(self, repository: self.repository).highlight end + # Array[] with right/left keys that contains Gitlab::Diff::Line objects which text is hightlighted def parallel_diff_lines @parallel_diff_lines ||= Gitlab::Diff::ParallelDiff.new(self).parallelize end diff --git a/lib/gitlab/diff/highlight.rb b/lib/gitlab/diff/highlight.rb index 649a265a02c..9ea976e18fa 100644 --- a/lib/gitlab/diff/highlight.rb +++ b/lib/gitlab/diff/highlight.rb @@ -40,8 +40,6 @@ module Gitlab def highlight_line(diff_line) return unless diff_file && diff_file.diff_refs - line_prefix = diff_line.text.match(/\A(.)/) ? $1 : ' ' - rich_line = if diff_line.unchanged? || diff_line.added? new_lines[diff_line.new_pos - 1] @@ -51,7 +49,10 @@ module Gitlab # Only update text if line is found. This will prevent # issues with submodules given the line only exists in diff content. - "#{line_prefix}#{rich_line}".html_safe if rich_line + if rich_line + line_prefix = diff_line.text.match(/\A(.)/) ? $1 : ' ' + "#{line_prefix}#{rich_line}".html_safe + end end def inline_diffs diff --git a/lib/gitlab/diff/line.rb b/lib/gitlab/diff/line.rb index c6189d660c2..cf097e0d0de 100644 --- a/lib/gitlab/diff/line.rb +++ b/lib/gitlab/diff/line.rb @@ -9,6 +9,20 @@ module Gitlab @old_pos, @new_pos = old_pos, new_pos end + def self.init_from_hash(hash) + new(hash[:text], hash[:type], hash[:index], hash[:old_pos], hash[:new_pos]) + end + + def serialize_keys + @serialize_keys ||= %i(text type index old_pos new_pos) + end + + def to_hash + hash = {} + serialize_keys.each { |key| hash[key] = send(key) } + hash + end + def old_line old_pos unless added? || meta? end diff --git a/lib/gitlab/email/message/repository_push.rb b/lib/gitlab/email/message/repository_push.rb index 97701b0cd42..48946ba355b 100644 --- a/lib/gitlab/email/message/repository_push.rb +++ b/lib/gitlab/email/message/repository_push.rb @@ -41,7 +41,7 @@ module Gitlab def diffs return unless compare - @diffs ||= safe_diff_files(compare.diffs(max_files: 30), diff_refs: diff_refs, repository: project.repository) + @diffs ||= SafeDiffs::Compare.new(compare, diff_options: { max_files: 30 }, project: project, diff_refs: diff_refs).diff_files end def diffs_count -- cgit v1.2.1 From 8f359ea9170b984ad43d126e17628c31ac3a1f14 Mon Sep 17 00:00:00 2001 From: Paco Guzman Date: Tue, 26 Jul 2016 09:21:42 +0200 Subject: Move to Gitlab::Diff::FileCollection Instead calling diff_collection.count use diff_collection.size which is cache on the diff_collection --- lib/gitlab/diff/file.rb | 2 +- lib/gitlab/diff/file_collection.rb | 9 +++ lib/gitlab/diff/file_collection/base.rb | 30 ++++++++ lib/gitlab/diff/file_collection/commit.rb | 14 ++++ lib/gitlab/diff/file_collection/compare.rb | 14 ++++ lib/gitlab/diff/file_collection/merge_request.rb | 88 ++++++++++++++++++++++++ lib/gitlab/email/message/repository_push.rb | 10 +-- 7 files changed, 162 insertions(+), 5 deletions(-) create mode 100644 lib/gitlab/diff/file_collection.rb create mode 100644 lib/gitlab/diff/file_collection/base.rb create mode 100644 lib/gitlab/diff/file_collection/commit.rb create mode 100644 lib/gitlab/diff/file_collection/compare.rb create mode 100644 lib/gitlab/diff/file_collection/merge_request.rb (limited to 'lib') diff --git a/lib/gitlab/diff/file.rb b/lib/gitlab/diff/file.rb index 77b3798d78f..e47df508ca2 100644 --- a/lib/gitlab/diff/file.rb +++ b/lib/gitlab/diff/file.rb @@ -63,7 +63,7 @@ module Gitlab diff_refs.try(:head_sha) end - attr_writer :diff_lines, :highlighted_diff_lines + attr_writer :highlighted_diff_lines # Array of Gitlab::Diff::Line objects def diff_lines diff --git a/lib/gitlab/diff/file_collection.rb b/lib/gitlab/diff/file_collection.rb new file mode 100644 index 00000000000..ce6717c7205 --- /dev/null +++ b/lib/gitlab/diff/file_collection.rb @@ -0,0 +1,9 @@ +module Gitlab + module Diff + module FileCollection + def self.default_options + ::Commit.max_diff_options.merge(ignore_whitespace_change: false, no_collapse: false) + end + end + end +end diff --git a/lib/gitlab/diff/file_collection/base.rb b/lib/gitlab/diff/file_collection/base.rb new file mode 100644 index 00000000000..20562773c14 --- /dev/null +++ b/lib/gitlab/diff/file_collection/base.rb @@ -0,0 +1,30 @@ +module Gitlab + module Diff + module FileCollection + + class Base + attr_reader :project, :diff_options, :diff_view, :diff_refs + + delegate :count, :size, :real_size, to: :diff_files + + def initialize(diffs, project:, diff_options:, diff_refs: nil) + @diffs = diffs + @project = project + @diff_options = diff_options + @diff_refs = diff_refs + end + + def diff_files + @diffs.decorate! { |diff| decorate_diff!(diff) } + end + + private + + def decorate_diff!(diff) + return diff if diff.is_a?(Gitlab::Diff::File) + Gitlab::Diff::File.new(diff, diff_refs: @diff_refs, repository: @project.repository) + end + end + end + end +end diff --git a/lib/gitlab/diff/file_collection/commit.rb b/lib/gitlab/diff/file_collection/commit.rb new file mode 100644 index 00000000000..2a46109ad99 --- /dev/null +++ b/lib/gitlab/diff/file_collection/commit.rb @@ -0,0 +1,14 @@ +module Gitlab + module Diff + module FileCollection + class Commit < Base + def initialize(commit, diff_options:) + super(commit.diffs(diff_options), + project: commit.project, + diff_options: diff_options, + diff_refs: commit.diff_refs) + end + end + end + end +end diff --git a/lib/gitlab/diff/file_collection/compare.rb b/lib/gitlab/diff/file_collection/compare.rb new file mode 100644 index 00000000000..1bcda145f15 --- /dev/null +++ b/lib/gitlab/diff/file_collection/compare.rb @@ -0,0 +1,14 @@ +module Gitlab + module Diff + module FileCollection + class Compare < Base + def initialize(compare, project:, diff_options:, diff_refs: nil) + super(compare.diffs(diff_options), + project: project, + diff_options: diff_options, + diff_refs: diff_refs) + end + end + end + end +end diff --git a/lib/gitlab/diff/file_collection/merge_request.rb b/lib/gitlab/diff/file_collection/merge_request.rb new file mode 100644 index 00000000000..7c40622d594 --- /dev/null +++ b/lib/gitlab/diff/file_collection/merge_request.rb @@ -0,0 +1,88 @@ +module Gitlab + module Diff + module FileCollection + class MergeRequest < Base + def initialize(merge_request, diff_options:) + @merge_request = merge_request + + super(merge_request.diffs(diff_options), + project: merge_request.project, + diff_options: diff_options, + diff_refs: merge_request.diff_refs) + end + + def diff_files + super.tap { |_| store_highlight_cache } + end + + private + + # Extracted method to highlight in the same iteration to the diff_collection. Iteration in the DiffCollections + # seems particularly slow on big diffs (event when already populated). + def decorate_diff!(diff) + highlight! super + end + + def highlight!(diff_file) + if cacheable? + cache_highlight!(diff_file) + else + highlight_diff_file!(diff_file) + end + end + + def highlight_diff_file!(diff_file) + diff_file.highlighted_diff_lines = Gitlab::Diff::Highlight.new(diff_file, repository: diff_file.repository).highlight + diff_file + end + + def highlight_diff_file_from_cache!(diff_file, cache_diff_lines) + diff_file.highlighted_diff_lines = cache_diff_lines.map do |line| + Gitlab::Diff::Line.init_from_hash(line) + end + end + + # + # If we find the highlighted diff files lines on the cache we replace existing diff_files lines (no highlighted) + # for the highlighted ones, so we just skip their execution. + # If the highlighted diff files lines are not cached we calculate and cache them. + # + # The content of the cache is a Hash where the key correspond to the file_path and the values are Arrays of + # hashes that represent serialized diff lines. + # + def cache_highlight!(diff_file) + file_path = diff_file.file_path + + if highlight_cache[file_path] + highlight_diff_file_from_cache!(diff_file, highlight_cache[file_path]) + else + highlight_diff_file!(diff_file) + highlight_cache[file_path] = diff_file.highlighted_diff_lines.map(&:to_hash) + end + + diff_file + end + + def highlight_cache + return @highlight_cache if defined?(@highlight_cache) + + @highlight_cache = Rails.cache.read(cache_key) || {} + @highlight_cache_was_empty = highlight_cache.empty? + @highlight_cache + end + + def store_highlight_cache + Rails.cache.write(cache_key, highlight_cache) if @highlight_cache_was_empty + end + + def cacheable? + @merge_request.merge_request_diff.present? + end + + def cache_key + [@merge_request.merge_request_diff, 'highlighted-diff-files', diff_options] + end + end + end + end +end diff --git a/lib/gitlab/email/message/repository_push.rb b/lib/gitlab/email/message/repository_push.rb index 48946ba355b..71213813e17 100644 --- a/lib/gitlab/email/message/repository_push.rb +++ b/lib/gitlab/email/message/repository_push.rb @@ -40,16 +40,18 @@ module Gitlab def diffs return unless compare - - @diffs ||= SafeDiffs::Compare.new(compare, diff_options: { max_files: 30 }, project: project, diff_refs: diff_refs).diff_files + + @diffs ||= compare.diff_file_collection(diff_options: { max_files: 30 }, diff_refs: diff_refs).diff_files end def diffs_count - diffs.count if diffs + diffs.size if diffs end def compare - @opts[:compare] + if @opts[:compare] + Compare.decorate(@opts[:compare], project) + end end def diff_refs -- cgit v1.2.1 From 1d0c7b74920a94e488e6a2c090abb3e525438053 Mon Sep 17 00:00:00 2001 From: Paco Guzman Date: Wed, 27 Jul 2016 13:09:52 +0200 Subject: Introduce Compare model in the codebase. This object will manage Gitlab::Git::Compare instances --- lib/gitlab/diff/file_collection/base.rb | 2 +- lib/gitlab/diff/file_collection/commit.rb | 3 +++ lib/gitlab/diff/file_collection/compare.rb | 3 +++ lib/gitlab/diff/file_collection/merge_request.rb | 10 +++------- lib/gitlab/email/message/repository_push.rb | 18 +++++++++--------- 5 files changed, 19 insertions(+), 17 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/diff/file_collection/base.rb b/lib/gitlab/diff/file_collection/base.rb index 20562773c14..a0c88265c45 100644 --- a/lib/gitlab/diff/file_collection/base.rb +++ b/lib/gitlab/diff/file_collection/base.rb @@ -7,7 +7,7 @@ module Gitlab delegate :count, :size, :real_size, to: :diff_files - def initialize(diffs, project:, diff_options:, diff_refs: nil) + def initialize(diffs, project:, diff_options: nil, diff_refs: nil) @diffs = diffs @project = project @diff_options = diff_options diff --git a/lib/gitlab/diff/file_collection/commit.rb b/lib/gitlab/diff/file_collection/commit.rb index 2a46109ad99..19def300b74 100644 --- a/lib/gitlab/diff/file_collection/commit.rb +++ b/lib/gitlab/diff/file_collection/commit.rb @@ -3,6 +3,9 @@ module Gitlab module FileCollection class Commit < Base def initialize(commit, diff_options:) + # Not merge just set defaults + diff_options = diff_options || Gitlab::Diff::FileCollection.default_options + super(commit.diffs(diff_options), project: commit.project, diff_options: diff_options, diff --git a/lib/gitlab/diff/file_collection/compare.rb b/lib/gitlab/diff/file_collection/compare.rb index 1bcda145f15..aba5a28b51f 100644 --- a/lib/gitlab/diff/file_collection/compare.rb +++ b/lib/gitlab/diff/file_collection/compare.rb @@ -3,6 +3,9 @@ module Gitlab module FileCollection class Compare < Base def initialize(compare, project:, diff_options:, diff_refs: nil) + # Not merge just set defaults + diff_options = diff_options || Gitlab::Diff::FileCollection.default_options + super(compare.diffs(diff_options), project: project, diff_options: diff_options, diff --git a/lib/gitlab/diff/file_collection/merge_request.rb b/lib/gitlab/diff/file_collection/merge_request.rb index 7c40622d594..9fde0bba183 100644 --- a/lib/gitlab/diff/file_collection/merge_request.rb +++ b/lib/gitlab/diff/file_collection/merge_request.rb @@ -4,6 +4,8 @@ module Gitlab class MergeRequest < Base def initialize(merge_request, diff_options:) @merge_request = merge_request + # Not merge just set defaults + diff_options = diff_options || Gitlab::Diff::FileCollection.default_options super(merge_request.diffs(diff_options), project: merge_request.project, @@ -27,15 +29,10 @@ module Gitlab if cacheable? cache_highlight!(diff_file) else - highlight_diff_file!(diff_file) + diff_file # Don't need to eager load highlighted diff lines end end - def highlight_diff_file!(diff_file) - diff_file.highlighted_diff_lines = Gitlab::Diff::Highlight.new(diff_file, repository: diff_file.repository).highlight - diff_file - end - def highlight_diff_file_from_cache!(diff_file, cache_diff_lines) diff_file.highlighted_diff_lines = cache_diff_lines.map do |line| Gitlab::Diff::Line.init_from_hash(line) @@ -56,7 +53,6 @@ module Gitlab if highlight_cache[file_path] highlight_diff_file_from_cache!(diff_file, highlight_cache[file_path]) else - highlight_diff_file!(diff_file) highlight_cache[file_path] = diff_file.highlighted_diff_lines.map(&:to_hash) end diff --git a/lib/gitlab/email/message/repository_push.rb b/lib/gitlab/email/message/repository_push.rb index 71213813e17..16491ede71b 100644 --- a/lib/gitlab/email/message/repository_push.rb +++ b/lib/gitlab/email/message/repository_push.rb @@ -35,13 +35,13 @@ module Gitlab def commits return unless compare - @commits ||= Commit.decorate(compare.commits, project) + @commits ||= compare.commits end def diffs return unless compare - @diffs ||= compare.diff_file_collection(diff_options: { max_files: 30 }, diff_refs: diff_refs).diff_files + @diffs ||= compare.diff_file_collection(diff_options: { max_files: 30 }).diff_files end def diffs_count @@ -49,9 +49,7 @@ module Gitlab end def compare - if @opts[:compare] - Compare.decorate(@opts[:compare], project) - end + @opts[:compare] if @opts[:compare] end def diff_refs @@ -99,16 +97,18 @@ module Gitlab if commits.length > 1 namespace_project_compare_url(project_namespace, project, - from: Commit.new(compare.base, project), - to: Commit.new(compare.head, project)) + from: compare.start_commit, + to: compare.head_commit) else namespace_project_commit_url(project_namespace, - project, commits.first) + project, + commits.first) end else unless @action == :delete namespace_project_tree_url(project_namespace, - project, ref_name) + project, + ref_name) end end end -- cgit v1.2.1 From c86c1905b5574cac234315598d8d715fcaee3ea7 Mon Sep 17 00:00:00 2001 From: Paco Guzman Date: Wed, 27 Jul 2016 19:00:34 +0200 Subject: switch from diff_file_collection to diffs So we have raw_diffs too --- lib/api/commits.rb | 4 ++-- lib/api/entities.rb | 2 +- lib/gitlab/diff/file_collection.rb | 9 --------- lib/gitlab/diff/file_collection/base.rb | 17 +++++++++++------ lib/gitlab/diff/file_collection/commit.rb | 5 +---- lib/gitlab/diff/file_collection/compare.rb | 5 +---- lib/gitlab/diff/file_collection/merge_request.rb | 23 ++++++----------------- lib/gitlab/email/message/repository_push.rb | 3 ++- 8 files changed, 24 insertions(+), 44 deletions(-) delete mode 100644 lib/gitlab/diff/file_collection.rb (limited to 'lib') diff --git a/lib/api/commits.rb b/lib/api/commits.rb index 4a11c8e3620..b4eaf1813d4 100644 --- a/lib/api/commits.rb +++ b/lib/api/commits.rb @@ -54,7 +54,7 @@ module API sha = params[:sha] commit = user_project.commit(sha) not_found! "Commit" unless commit - commit.diffs.to_a + commit.raw_diffs.to_a end # Get a commit's comments @@ -96,7 +96,7 @@ module API } if params[:path] && params[:line] && params[:line_type] - commit.diffs(all_diffs: true).each do |diff| + commit.raw_diffs(all_diffs: true).each do |diff| next unless diff.new_path == params[:path] lines = Gitlab::Diff::Parser.new.parse(diff.diff.each_line) diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 3e21b7a0b8a..e5b00dc45a5 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -224,7 +224,7 @@ module API class MergeRequestChanges < MergeRequest expose :diffs, as: :changes, using: Entities::RepoDiff do |compare, _| - compare.diffs(all_diffs: true).to_a + compare.raw_diffs(all_diffs: true).to_a end end diff --git a/lib/gitlab/diff/file_collection.rb b/lib/gitlab/diff/file_collection.rb deleted file mode 100644 index ce6717c7205..00000000000 --- a/lib/gitlab/diff/file_collection.rb +++ /dev/null @@ -1,9 +0,0 @@ -module Gitlab - module Diff - module FileCollection - def self.default_options - ::Commit.max_diff_options.merge(ignore_whitespace_change: false, no_collapse: false) - end - end - end -end diff --git a/lib/gitlab/diff/file_collection/base.rb b/lib/gitlab/diff/file_collection/base.rb index a0c88265c45..2b9fc65b985 100644 --- a/lib/gitlab/diff/file_collection/base.rb +++ b/lib/gitlab/diff/file_collection/base.rb @@ -1,28 +1,33 @@ module Gitlab module Diff module FileCollection - class Base attr_reader :project, :diff_options, :diff_view, :diff_refs delegate :count, :size, :real_size, to: :diff_files - def initialize(diffs, project:, diff_options: nil, diff_refs: nil) - @diffs = diffs + def self.default_options + ::Commit.max_diff_options.merge(ignore_whitespace_change: false, no_collapse: false) + end + + def initialize(diffable, project:, diff_options: nil, diff_refs: nil) + diff_options = self.class.default_options.merge(diff_options || {}) + + @diffable = diffable + @diffs = diffable.raw_diffs(diff_options) @project = project @diff_options = diff_options @diff_refs = diff_refs end def diff_files - @diffs.decorate! { |diff| decorate_diff!(diff) } + @diff_files ||= @diffs.decorate! { |diff| decorate_diff!(diff) } end private def decorate_diff!(diff) - return diff if diff.is_a?(Gitlab::Diff::File) - Gitlab::Diff::File.new(diff, diff_refs: @diff_refs, repository: @project.repository) + Gitlab::Diff::File.new(diff, repository: project.repository, diff_refs: diff_refs) end end end diff --git a/lib/gitlab/diff/file_collection/commit.rb b/lib/gitlab/diff/file_collection/commit.rb index 19def300b74..4dc297ec036 100644 --- a/lib/gitlab/diff/file_collection/commit.rb +++ b/lib/gitlab/diff/file_collection/commit.rb @@ -3,10 +3,7 @@ module Gitlab module FileCollection class Commit < Base def initialize(commit, diff_options:) - # Not merge just set defaults - diff_options = diff_options || Gitlab::Diff::FileCollection.default_options - - super(commit.diffs(diff_options), + super(commit, project: commit.project, diff_options: diff_options, diff_refs: commit.diff_refs) diff --git a/lib/gitlab/diff/file_collection/compare.rb b/lib/gitlab/diff/file_collection/compare.rb index aba5a28b51f..20d8f891cc3 100644 --- a/lib/gitlab/diff/file_collection/compare.rb +++ b/lib/gitlab/diff/file_collection/compare.rb @@ -3,10 +3,7 @@ module Gitlab module FileCollection class Compare < Base def initialize(compare, project:, diff_options:, diff_refs: nil) - # Not merge just set defaults - diff_options = diff_options || Gitlab::Diff::FileCollection.default_options - - super(compare.diffs(diff_options), + super(compare, project: project, diff_options: diff_options, diff_refs: diff_refs) diff --git a/lib/gitlab/diff/file_collection/merge_request.rb b/lib/gitlab/diff/file_collection/merge_request.rb index 9fde0bba183..4f946908e2f 100644 --- a/lib/gitlab/diff/file_collection/merge_request.rb +++ b/lib/gitlab/diff/file_collection/merge_request.rb @@ -4,10 +4,8 @@ module Gitlab class MergeRequest < Base def initialize(merge_request, diff_options:) @merge_request = merge_request - # Not merge just set defaults - diff_options = diff_options || Gitlab::Diff::FileCollection.default_options - super(merge_request.diffs(diff_options), + super(merge_request, project: merge_request.project, diff_options: diff_options, diff_refs: merge_request.diff_refs) @@ -19,18 +17,11 @@ module Gitlab private - # Extracted method to highlight in the same iteration to the diff_collection. Iteration in the DiffCollections - # seems particularly slow on big diffs (event when already populated). + # Extracted method to highlight in the same iteration to the diff_collection. def decorate_diff!(diff) - highlight! super - end - - def highlight!(diff_file) - if cacheable? - cache_highlight!(diff_file) - else - diff_file # Don't need to eager load highlighted diff lines - end + diff_file = super + cache_highlight!(diff_file) if cacheable? + diff_file end def highlight_diff_file_from_cache!(diff_file, cache_diff_lines) @@ -55,15 +46,13 @@ module Gitlab else highlight_cache[file_path] = diff_file.highlighted_diff_lines.map(&:to_hash) end - - diff_file end def highlight_cache return @highlight_cache if defined?(@highlight_cache) @highlight_cache = Rails.cache.read(cache_key) || {} - @highlight_cache_was_empty = highlight_cache.empty? + @highlight_cache_was_empty = @highlight_cache.empty? @highlight_cache end diff --git a/lib/gitlab/email/message/repository_push.rb b/lib/gitlab/email/message/repository_push.rb index 16491ede71b..62d29387d60 100644 --- a/lib/gitlab/email/message/repository_push.rb +++ b/lib/gitlab/email/message/repository_push.rb @@ -41,7 +41,8 @@ module Gitlab def diffs return unless compare - @diffs ||= compare.diff_file_collection(diff_options: { max_files: 30 }).diff_files + # This diff is more moderated in number of files and lines + @diffs ||= compare.diffs(diff_options: { max_files: 30, max_lines: 5000, no_collapse: true }).diff_files end def diffs_count -- cgit v1.2.1 From dd35c3ddf6dce7a69cc116fe6165dad68b8e9251 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Tue, 2 Aug 2016 17:51:17 +0200 Subject: Improve AutolinkFilter#text_parse performance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit By using clever XPath queries we can quite significantly improve the performance of this method. The actual improvement depends a bit on the amount of links used but in my tests the new implementation is usually around 8 times faster than the old one. This was measured using the following benchmark: require 'benchmark/ips' text = '

' + Note.select("string_agg(note, '') AS note").limit(50).take[:note] + '

' document = Nokogiri::HTML.fragment(text) filter = Banzai::Filter::AutolinkFilter.new(document, autolink: true) puts "Input size: #{(text.bytesize.to_f / 1024 / 1024).round(2)} MB" filter.rinku_parse Benchmark.ips(time: 15) do |bench| bench.report 'text_parse' do filter.text_parse end bench.report 'text_parse_fast' do filter.text_parse_fast end bench.compare! end Here the "text_parse_fast" method is the new implementation and "text_parse" the old one. The input size was around 180 MB. Running this benchmark outputs the following: Input size: 181.16 MB Calculating ------------------------------------- text_parse 1.000 i/100ms text_parse_fast 9.000 i/100ms ------------------------------------------------- text_parse 13.021 (±15.4%) i/s - 188.000 text_parse_fast 112.741 (± 3.5%) i/s - 1.692k Comparison: text_parse_fast: 112.7 i/s text_parse: 13.0 i/s - 8.66x slower Again the production timings may (and most likely will) vary depending on the input being processed. --- lib/banzai/filter/autolink_filter.rb | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) (limited to 'lib') diff --git a/lib/banzai/filter/autolink_filter.rb b/lib/banzai/filter/autolink_filter.rb index 9ed45707515..799b83b1069 100644 --- a/lib/banzai/filter/autolink_filter.rb +++ b/lib/banzai/filter/autolink_filter.rb @@ -31,6 +31,14 @@ module Banzai # 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(., '://') + and not(starts-with(., 'http')) + and not(starts-with(., 'ftp')) + ]) + def call return doc if context[:autolink] == false @@ -66,16 +74,11 @@ module Banzai # Autolinks any text matching LINK_PATTERN that Rinku didn't already # replace def text_parse - search_text_nodes(doc).each do |node| + doc.xpath(TEXT_QUERY).each do |node| content = node.to_html - next if has_ancestor?(node, IGNORE_PARENTS) next unless content.match(LINK_PATTERN) - # If Rinku didn't link this, there's probably a good reason, so we'll - # skip it too - next if content.start_with?(*%w(http https ftp)) - html = autolink_filter(content) next if html == content -- cgit v1.2.1 From f87eb2502023fb0c17d29aef398058c62736c9ca Mon Sep 17 00:00:00 2001 From: James Lopez Date: Wed, 3 Aug 2016 13:00:34 +0200 Subject: Fix Import/Export error checking versions --- lib/gitlab/import_export/version_checker.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/gitlab/import_export/version_checker.rb b/lib/gitlab/import_export/version_checker.rb index abfc694b879..de3fe6d822e 100644 --- a/lib/gitlab/import_export/version_checker.rb +++ b/lib/gitlab/import_export/version_checker.rb @@ -25,7 +25,7 @@ module Gitlab def verify_version!(version) if Gem::Version.new(version) > Gem::Version.new(Gitlab::ImportExport.version) - raise Gitlab::ImportExport::Error("Import version mismatch: Required <= #{Gitlab::ImportExport.version} but was #{version}") + raise Gitlab::ImportExport::Error.new("Import version mismatch: Required <= #{Gitlab::ImportExport.version} but was #{version}") else true end -- cgit v1.2.1 From b8f754dd0abdf437669e17a820a8e6c230afa73e Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Wed, 3 Aug 2016 14:54:12 +0200 Subject: Stop 'git push' over HTTP early Before this change we always let users push Git data over HTTP before deciding whether to accept to push. This was different from pushing over SSH where we terminate a 'git push' early if we already know the user is not allowed to push. This change let Git over HTTP follow the same behavior as Git over SSH. We also distinguish between HTTP 404 and 403 responses when denying Git requests, depending on whether the user is allowed to know the project exists. --- lib/gitlab/git_access.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb index 8e8f39d9cb2..69943e22353 100644 --- a/lib/gitlab/git_access.rb +++ b/lib/gitlab/git_access.rb @@ -14,7 +14,7 @@ module Gitlab @user_access = UserAccess.new(user, project: project) end - def check(cmd, changes = nil) + def check(cmd, changes) return build_status_object(false, "Git access over #{protocol.upcase} is not allowed") unless protocol_allowed? unless actor -- cgit v1.2.1 From da3d3ba89c19364ca626eb57380e1e33bd344902 Mon Sep 17 00:00:00 2001 From: "Z.J. van de Weg" Date: Wed, 3 Aug 2016 14:45:32 +0200 Subject: Endpoints to enable and disable deploy keys Resolves #20123 --- lib/api/deploy_keys.rb | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) (limited to 'lib') diff --git a/lib/api/deploy_keys.rb b/lib/api/deploy_keys.rb index 5c570b5e5ca..ab4868eca2d 100644 --- a/lib/api/deploy_keys.rb +++ b/lib/api/deploy_keys.rb @@ -74,6 +74,37 @@ module API end end + desc 'Enable a deploy key for a project' do + detail 'This feature was added in GitLab 8.11' + success Entities::SSHKey + end + params do + requires :key_id, type: Integer, desc: 'The ID of the deploy key' + end + post ":id/#{path}/:key_id/enable" do + key = DeployKey.find(params[:key_id]) + + if user_project.deploy_keys << key + present key, with: Entities::SSHKey + else + render_validation_error!(key) + end + end + + desc 'Disable a deploy key for a project' do + detail 'This feature was added in GitLab 8.11' + success Entities::SSHKey + end + params do + requires :key_id, type: Integer, desc: 'The ID of the deploy key' + end + delete ":id/#{path}/:key_id/disable" do + key = user_project.deploy_keys_projects.find_by(deploy_key_id: params[:key_id]) + key.destroy + + present key.deploy_key, with: Entities::SSHKey + end + # Delete existing deploy key of currently authenticated user # # Example Request: -- cgit v1.2.1 From 038d6febedc03280bd686d929057c02d05f2afd6 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Wed, 3 Aug 2016 16:52:08 +0200 Subject: Improve performance of SyntaxHighlightFilter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit By using Rouge::Lexer.find instead of find_fancy() and memoizing the HTML formatter we can speed up the highlighting process by between 1.7 and 1.8 times (at least when measured using synthetic benchmarks). To measure this I used the following benchmark: require 'benchmark/ips' input = '' Dir['./app/controllers/**/*.rb'].each do |controller| input << <<-EOF 
#{File.read(controller).strip}
EOF end document = Nokogiri::HTML.fragment(input) filter = Banzai::Filter::SyntaxHighlightFilter.new(document) puts "Input size: #{(input.bytesize.to_f / 1024).round(2)} KB" Benchmark.ips do |bench| bench.report 'call' do filter.call end end This benchmark produces 250 KB of input. Before these changes the timing output would be as follows: Calculating ------------------------------------- call 1.000 i/100ms ------------------------------------------------- call 22.439 (±35.7%) i/s - 93.000 After these changes the output instead is as follows: Calculating ------------------------------------- call 1.000 i/100ms ------------------------------------------------- call 41.283 (±38.8%) i/s - 148.000 Note that due to the fairly high standard deviation and this being a synthetic benchmark it's entirely possible the real-world improvements are smaller. --- lib/banzai/filter/syntax_highlight_filter.rb | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) (limited to 'lib') diff --git a/lib/banzai/filter/syntax_highlight_filter.rb b/lib/banzai/filter/syntax_highlight_filter.rb index 91f0159f9a1..fcdb496aed2 100644 --- a/lib/banzai/filter/syntax_highlight_filter.rb +++ b/lib/banzai/filter/syntax_highlight_filter.rb @@ -17,15 +17,12 @@ module Banzai def highlight_node(node) language = node.attr('class') - code = node.text - + code = node.text css_classes = "code highlight" - - lexer = Rouge::Lexer.find_fancy(language) || Rouge::Lexers::PlainText - formatter = Rouge::Formatters::HTML.new + lexer = lexer_for(language) begin - code = formatter.format(lexer.lex(code)) + code = format(lex(lexer, code)) css_classes << " js-syntax-highlight #{lexer.tag}" rescue @@ -41,14 +38,27 @@ module Banzai private + # Separate method so it can be instrumented. + def lex(lexer, code) + lexer.lex(code) + end + + def format(tokens) + rouge_formatter.format(tokens) + end + + def lexer_for(language) + (Rouge::Lexer.find(language) || Rouge::Lexers::PlainText).new + end + def replace_parent_pre_element(node, highlighted) # Replace the parent `pre` element with the entire highlighted block node.parent.replace(highlighted) end # Override Rouge::Plugins::Redcarpet#rouge_formatter - def rouge_formatter(lexer) - Rouge::Formatters::HTML.new + def rouge_formatter(lexer = nil) + @rouge_formatter ||= Rouge::Formatters::HTML.new end end end -- cgit v1.2.1 From c008a1a9674f7c01b4504e22ed414b07eff05385 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Wed, 3 Aug 2016 09:32:01 -0700 Subject: Make Compare#diffs diff_options a regular argument --- lib/gitlab/email/message/repository_push.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/gitlab/email/message/repository_push.rb b/lib/gitlab/email/message/repository_push.rb index 62d29387d60..0e3b65fceb4 100644 --- a/lib/gitlab/email/message/repository_push.rb +++ b/lib/gitlab/email/message/repository_push.rb @@ -42,7 +42,7 @@ module Gitlab return unless compare # This diff is more moderated in number of files and lines - @diffs ||= compare.diffs(diff_options: { max_files: 30, max_lines: 5000, no_collapse: true }).diff_files + @diffs ||= compare.diffs(max_files: 30, max_lines: 5000, no_collapse: true).diff_files end def diffs_count -- cgit v1.2.1 From 8a62f4e7d46908c56bedf155792323dc4218be94 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Wed, 3 Aug 2016 11:22:40 -0700 Subject: Update the gitlab-shell version in the tmp/tests directory to the right version Previously the gitlab-shell version would never be updated if the directory existed via the `gitlab:shell:install` Rake task. This could lead to incompatibility issues or random errors. --- lib/tasks/gitlab/shell.rake | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/tasks/gitlab/shell.rake b/lib/tasks/gitlab/shell.rake index c85ebdf8619..ba93945bd03 100644 --- a/lib/tasks/gitlab/shell.rake +++ b/lib/tasks/gitlab/shell.rake @@ -5,7 +5,8 @@ namespace :gitlab do warn_user_is_not_gitlab default_version = Gitlab::Shell.version_required - args.with_defaults(tag: 'v' + default_version, repo: "https://gitlab.com/gitlab-org/gitlab-shell.git") + default_version_tag = 'v' + default_version + args.with_defaults(tag: default_version_tag, repo: "https://gitlab.com/gitlab-org/gitlab-shell.git") user = Gitlab.config.gitlab.user home_dir = Rails.env.test? ? Rails.root.join('tmp/tests') : Gitlab.config.gitlab.user_home @@ -15,7 +16,12 @@ namespace :gitlab do target_dir = Gitlab.config.gitlab_shell.path # Clone if needed - unless File.directory?(target_dir) + if File.directory?(target_dir) + Dir.chdir(target_dir) do + system(*%W(Gitlab.config.git.bin_path} fetch --tags --quiet)) + system(*%W(Gitlab.config.git.bin_path} checkout --quiet #{default_version_tag})) + end + else system(*%W(#{Gitlab.config.git.bin_path} clone -- #{args.repo} #{target_dir})) end -- cgit v1.2.1 From 2b6bd6a33f765175222cdb88cd927e420864a236 Mon Sep 17 00:00:00 2001 From: "Z.J. van de Weg" Date: Wed, 3 Aug 2016 16:45:35 +0200 Subject: Use Grape DSL for deploy keys endpoints Also a minor clean up of the post endpoint --- lib/api/deploy_keys.rb | 73 +++++++++++++++++++++++--------------------------- 1 file changed, 34 insertions(+), 39 deletions(-) (limited to 'lib') diff --git a/lib/api/deploy_keys.rb b/lib/api/deploy_keys.rb index ab4868eca2d..6a0be345667 100644 --- a/lib/api/deploy_keys.rb +++ b/lib/api/deploy_keys.rb @@ -10,6 +10,9 @@ module API present keys, with: Entities::SSHKey end + params do + requires :id, type: String, desc: 'The ID of the project' + end resource :projects do before { authorize_admin_project } @@ -17,52 +20,42 @@ module API # Use "projects/:id/deploy_keys/..." instead. # %w(keys deploy_keys).each do |path| - # Get a specific project's deploy keys - # - # Example Request: - # GET /projects/:id/deploy_keys + desc "Get a specific project's deploy keys" do + success Entities::SSHKey + end get ":id/#{path}" do present user_project.deploy_keys, with: Entities::SSHKey end - # Get single deploy key owned by currently authenticated user - # - # Example Request: - # GET /projects/:id/deploy_keys/:key_id + desc 'Get single deploy key' do + success Entities::SSHKey + end + params do + requires :key_id, type: Integer, desc: 'The ID of the deploy key' + end get ":id/#{path}/:key_id" do key = user_project.deploy_keys.find params[:key_id] present key, with: Entities::SSHKey end - # Add new deploy key to currently authenticated user - # If deploy key already exists - it will be joined to project - # but only if original one was accessible by same user - # - # Parameters: - # key (required) - New deploy Key - # title (required) - New deploy Key's title - # Example Request: - # POST /projects/:id/deploy_keys + desc 'Add new deploy key to currently authenticated user' do + success Entities::SSHKey + end + params do + requires :key, type: String, desc: "The new deploy key" + requires :title, type: String, desc: 'The title to identify the key from' + end post ":id/#{path}" do - attrs = attributes_for_keys [:title, :key] - - if attrs[:key].present? - attrs[:key].strip! + attrs = declared(params, include_parent_namespaces: false).to_h - # check if key already exist in project - key = user_project.deploy_keys.find_by(key: attrs[:key]) - if key - present key, with: Entities::SSHKey - next - end + key = user_project.deploy_keys.find_by(key: attrs[:key]) + present key, with: Entities::SSHKey if key - # Check for available deploy keys in other projects - key = current_user.accessible_deploy_keys.find_by(key: attrs[:key]) - if key - user_project.deploy_keys << key - present key, with: Entities::SSHKey - next - end + # Check for available deploy keys in other projects + key = current_user.accessible_deploy_keys.find_by(key: attrs[:key]) + if key + user_project.deploy_keys << key + present key, with: Entities::SSHKey end key = DeployKey.new attrs @@ -105,12 +98,14 @@ module API present key.deploy_key, with: Entities::SSHKey end - # Delete existing deploy key of currently authenticated user - # - # Example Request: - # DELETE /projects/:id/deploy_keys/:key_id + desc 'Delete existing deploy key of currently authenticated user' do + success Key + end + params do + requires :key_id, type: Integer, desc: 'The ID of the deploy key' + end delete ":id/#{path}/:key_id" do - key = user_project.deploy_keys.find params[:key_id] + key = user_project.deploy_keys.find(params[:key_id]) key.destroy end end -- cgit v1.2.1 From fa54a8e984949227b2b56ecd20e37e0c6ba498cd Mon Sep 17 00:00:00 2001 From: Paco Guzman Date: Thu, 4 Aug 2016 11:56:58 +0200 Subject: =?UTF-8?q?Don=E2=80=99t=20close=20issues=20on=20original=20projec?= =?UTF-8?q?t=20from=20a=20fork?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Paco Guzman --- lib/gitlab/closing_issue_extractor.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/gitlab/closing_issue_extractor.rb b/lib/gitlab/closing_issue_extractor.rb index 9bef9037ad6..58f86abc5c4 100644 --- a/lib/gitlab/closing_issue_extractor.rb +++ b/lib/gitlab/closing_issue_extractor.rb @@ -22,7 +22,9 @@ module Gitlab @extractor.analyze(closing_statements.join(" ")) - @extractor.issues + @extractor.issues.reject do |issue| + @extractor.project.forked_from?(issue.project) # Don't extract issues on original project + end end end end -- cgit v1.2.1 From 6a0bbb5aa58e37a0ad8c3817c4e809143adce1be Mon Sep 17 00:00:00 2001 From: James Lopez Date: Tue, 2 Aug 2016 11:32:28 +0200 Subject: using shared path for project import uploads and refactored gitlab remove export worker --- lib/gitlab/import_export.rb | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'lib') diff --git a/lib/gitlab/import_export.rb b/lib/gitlab/import_export.rb index 48b2c43ac21..bb562bdcd2c 100644 --- a/lib/gitlab/import_export.rb +++ b/lib/gitlab/import_export.rb @@ -13,6 +13,10 @@ module Gitlab File.join(Settings.shared['path'], 'tmp/project_exports') end + def import_upload_path(filename:) + File.join(storage_path, 'uploads', filename) + end + def project_filename "project.json" end -- cgit v1.2.1 From f15ed5f0a5c298a2f0eb5aaa6d848364133532a5 Mon Sep 17 00:00:00 2001 From: Herminio Torres Date: Thu, 4 Aug 2016 01:35:17 -0300 Subject: Fix Rename `add_users_into_project` and `projects_ids` We never add things `into` projects, we just add them `to` projects. So how about we rename this to `add_users_to_project`. Rename `projects_ids` to `project_ids` by following the convention of rails. --- lib/tasks/gitlab/bulk_add_permission.rake | 12 ++++++------ lib/tasks/gitlab/web_hook.rake | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) (limited to 'lib') diff --git a/lib/tasks/gitlab/bulk_add_permission.rake b/lib/tasks/gitlab/bulk_add_permission.rake index 5dbf7d61e06..83dd870fa31 100644 --- a/lib/tasks/gitlab/bulk_add_permission.rake +++ b/lib/tasks/gitlab/bulk_add_permission.rake @@ -4,13 +4,13 @@ namespace :gitlab do task all_users_to_all_projects: :environment do |t, args| user_ids = User.where(admin: false).pluck(:id) admin_ids = User.where(admin: true).pluck(:id) - projects_ids = Project.pluck(:id) + project_ids = Project.pluck(:id) - puts "Importing #{user_ids.size} users into #{projects_ids.size} projects" - ProjectMember.add_users_into_projects(projects_ids, user_ids, ProjectMember::DEVELOPER) + puts "Importing #{user_ids.size} users into #{project_ids.size} projects" + ProjectMember.add_users_to_projects(project_ids, user_ids, ProjectMember::DEVELOPER) - puts "Importing #{admin_ids.size} admins into #{projects_ids.size} projects" - ProjectMember.add_users_into_projects(projects_ids, admin_ids, ProjectMember::MASTER) + puts "Importing #{admin_ids.size} admins into #{project_ids.size} projects" + ProjectMember.add_users_to_projects(project_ids, admin_ids, ProjectMember::MASTER) end desc "GitLab | Add a specific user to all projects (as a developer)" @@ -18,7 +18,7 @@ namespace :gitlab do user = User.find_by(email: args.email) project_ids = Project.pluck(:id) puts "Importing #{user.email} users into #{project_ids.size} projects" - ProjectMember.add_users_into_projects(project_ids, Array.wrap(user.id), ProjectMember::DEVELOPER) + ProjectMember.add_users_to_projects(project_ids, Array.wrap(user.id), ProjectMember::DEVELOPER) end desc "GitLab | Add all users to all groups (admin users are added as owners)" diff --git a/lib/tasks/gitlab/web_hook.rake b/lib/tasks/gitlab/web_hook.rake index f467cc0ee29..49530e7a372 100644 --- a/lib/tasks/gitlab/web_hook.rake +++ b/lib/tasks/gitlab/web_hook.rake @@ -26,10 +26,10 @@ namespace :gitlab do namespace_path = ENV['NAMESPACE'] projects = find_projects(namespace_path) - projects_ids = projects.pluck(:id) + project_ids = projects.pluck(:id) puts "Removing webhooks with the url '#{web_hook_url}' ... " - count = WebHook.where(url: web_hook_url, project_id: projects_ids, type: 'ProjectHook').delete_all + count = WebHook.where(url: web_hook_url, project_id: project_ids, type: 'ProjectHook').delete_all puts "#{count} webhooks were removed." end -- cgit v1.2.1 From 460065b743a5f28d92bda71b0e778b01cd369d80 Mon Sep 17 00:00:00 2001 From: "Z.J. van de Weg" Date: Thu, 4 Aug 2016 13:09:10 +0200 Subject: Move deploy_key tests to deploy_key_spec.rb Also, fix the failing test in the process --- lib/api/deploy_keys.rb | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) (limited to 'lib') diff --git a/lib/api/deploy_keys.rb b/lib/api/deploy_keys.rb index 6a0be345667..52f89373ad3 100644 --- a/lib/api/deploy_keys.rb +++ b/lib/api/deploy_keys.rb @@ -38,15 +38,16 @@ module API present key, with: Entities::SSHKey end + # TODO: for 9.0 we should check if params are there with the params block + # grape provides, at this point we'd change behaviour so we can't + # Behaviour now if you don't provide all required params: it renders a + # validation error or two. desc 'Add new deploy key to currently authenticated user' do success Entities::SSHKey end - params do - requires :key, type: String, desc: "The new deploy key" - requires :title, type: String, desc: 'The title to identify the key from' - end post ":id/#{path}" do - attrs = declared(params, include_parent_namespaces: false).to_h + attrs = attributes_for_keys [:title, :key] + attrs[:key].strip! if attrs[:key] key = user_project.deploy_keys.find_by(key: attrs[:key]) present key, with: Entities::SSHKey if key -- cgit v1.2.1 From 926cee002d701548b5344e0b93e95beb3802fd54 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Mon, 30 May 2016 18:13:42 -0300 Subject: Deduplicated resque.yml loading from several places We will trust redis configuration params loading to Gitlab::RedisConfig. --- lib/gitlab/mail_room.rb | 44 ++++++++++++++++++++++++++ lib/gitlab/redis.rb | 84 ++++++++++++++++++++++++++++++++++--------------- 2 files changed, 103 insertions(+), 25 deletions(-) create mode 100644 lib/gitlab/mail_room.rb (limited to 'lib') diff --git a/lib/gitlab/mail_room.rb b/lib/gitlab/mail_room.rb new file mode 100644 index 00000000000..745cc79a10e --- /dev/null +++ b/lib/gitlab/mail_room.rb @@ -0,0 +1,44 @@ +require 'yaml' +require 'json' +require_relative 'lib/gitlab/redis' unless defined?(Gitlab::Redis) + +module Gitlab + module MailRoom + + class << self + + def enabled? + config[:enabled] && config[:address] + end + + def config + @config ||= fetch_config + end + + private + + def fetch_config + return nil unless File.exists?(config_file) + + rails_env = ENV['RAILS_ENV'] || ENV['RACK_ENV'] || 'development' + all_config = YAML.load_file(config_file)[rails_env].deep_symbolize_keys + + config = all_config[:incoming_email] || {} + config[:enabled] = false if config[:enabled].nil? + config[:port] = 143 if config[:port].nil? + config[:ssl] = false if config[:ssl].nil? + config[:start_tls] = false if config[:start_tls].nil? + config[:mailbox] = 'inbox' if config[:mailbox].nil? + + if config[:enabled] && config[:address] + config[:redis_url] = Gitlab::Redis.new(rails_env).url + end + end + + def config_file + file = ENV['MAIL_ROOM_GITLAB_CONFIG_FILE'] || 'config/gitlab.yml' + File.expand_path("../../../#{file}", __FILE__) + end + end + end +end diff --git a/lib/gitlab/redis.rb b/lib/gitlab/redis.rb index 40766f35f77..8050b28f33a 100644 --- a/lib/gitlab/redis.rb +++ b/lib/gitlab/redis.rb @@ -1,50 +1,84 @@ +# This file should not have any direct dependency on Rails environment +# please require all dependencies below: +require 'active_support/core_ext/hash/keys' + module Gitlab class Redis CACHE_NAMESPACE = 'cache:gitlab' SESSION_NAMESPACE = 'session:gitlab' SIDEKIQ_NAMESPACE = 'resque:gitlab' - - attr_reader :url + MAILROOM_NAMESPACE = 'mail_room:gitlab' + DEFAULT_REDIS_URL = 'redis://localhost:6379' # To be thread-safe we must be careful when writing the class instance # variables @url and @pool. Because @pool depends on @url we need two # mutexes to prevent deadlock. - URL_MUTEX = Mutex.new + PARAMS_MUTEX = Mutex.new POOL_MUTEX = Mutex.new - private_constant :URL_MUTEX, :POOL_MUTEX + private_constant :PARAMS_MUTEX, :POOL_MUTEX - def self.url - @url || URL_MUTEX.synchronize { @url = new.url } - end + class << self + def params + @params || PARAMS_MUTEX.synchronize { @params = new.params } + end + + # @deprecated Use .params instead to get sentinel support + def url + raw_config_hash[:url] + end - def self.with - if @pool.nil? - POOL_MUTEX.synchronize do - @pool = ConnectionPool.new { ::Redis.new(url: url) } + def with + if @pool.nil? + POOL_MUTEX.synchronize do + @pool = ConnectionPool.new { ::Redis.new(params) } + end end + @pool.with { |redis| yield redis } end - @pool.with { |redis| yield redis } end - def self.redis_store_options - url = new.url - redis_config_hash = ::Redis::Store::Factory.extract_host_options_from_uri(url) - # Redis::Store does not handle Unix sockets well, so let's do it for them - redis_uri = URI.parse(url) + def initialize(rails_env=nil) + @rails_env = rails_env || Rails.env + end + + def params + redis_store_options + end + + private + + def redis_store_options + config = raw_config_hash + + redis_uri = URI.parse(config[:url]) if redis_uri.scheme == 'unix' - redis_config_hash[:path] = redis_uri.path + # Redis::Store does not handle Unix sockets well, so let's do it for them + config[:path] = redis_uri.path + config.delete(:url) + else + redis_hash = ::Redis::Store::Factory.extract_host_options_from_uri(redis_uri) + config.merge!(redis_hash) end - redis_config_hash + + config end - def initialize(rails_env=nil) - rails_env ||= Rails.env - config_file = File.expand_path('../../../config/resque.yml', __FILE__) + def raw_config_hash + config_data = fetch_config - @url = "redis://localhost:6379" - if File.exist?(config_file) - @url = YAML.load_file(config_file)[rails_env] + if config_data + config_data.is_a?(String) ? { url: config_data } : config_data.deep_symbolize_keys + else + { url: DEFAULT_REDIS_URL } end end + + def fetch_config + File.exists?(config_file) ? YAML.load_file(config_file)[@rails_env] : false + end + + def config_file + File.expand_path('../../../config/resque.yml', __FILE__) + end end end -- cgit v1.2.1 From ef6043880ee492e6c8fd7672d0e36ca81a62cfc9 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Fri, 27 Nov 2015 14:32:32 -0200 Subject: Specs for RedisConfig Make sure :url is not present on RedisConfig.params after parsing --- lib/gitlab/redis.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/gitlab/redis.rb b/lib/gitlab/redis.rb index 8050b28f33a..8c277d97f1a 100644 --- a/lib/gitlab/redis.rb +++ b/lib/gitlab/redis.rb @@ -54,12 +54,12 @@ module Gitlab if redis_uri.scheme == 'unix' # Redis::Store does not handle Unix sockets well, so let's do it for them config[:path] = redis_uri.path - config.delete(:url) else redis_hash = ::Redis::Store::Factory.extract_host_options_from_uri(redis_uri) config.merge!(redis_hash) end + config.delete(:url) config end -- cgit v1.2.1 From 6f318795083ca3d3726bb6bf5f4dc4081cfba8bf Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Fri, 8 Jul 2016 03:33:37 +0200 Subject: Fixed specs for Gitlab::Redis and code for Redis Sentinel support --- lib/gitlab/mail_room.rb | 10 +++++----- lib/gitlab/redis.rb | 9 +++++---- 2 files changed, 10 insertions(+), 9 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/mail_room.rb b/lib/gitlab/mail_room.rb index 745cc79a10e..781c89579b6 100644 --- a/lib/gitlab/mail_room.rb +++ b/lib/gitlab/mail_room.rb @@ -1,12 +1,11 @@ require 'yaml' require 'json' -require_relative 'lib/gitlab/redis' unless defined?(Gitlab::Redis) +require_relative 'redis' unless defined?(Gitlab::Redis) module Gitlab module MailRoom class << self - def enabled? config[:enabled] && config[:address] end @@ -18,7 +17,7 @@ module Gitlab private def fetch_config - return nil unless File.exists?(config_file) + return {} unless File.exist?(config_file) rails_env = ENV['RAILS_ENV'] || ENV['RACK_ENV'] || 'development' all_config = YAML.load_file(config_file)[rails_env].deep_symbolize_keys @@ -33,11 +32,12 @@ module Gitlab if config[:enabled] && config[:address] config[:redis_url] = Gitlab::Redis.new(rails_env).url end + + config end def config_file - file = ENV['MAIL_ROOM_GITLAB_CONFIG_FILE'] || 'config/gitlab.yml' - File.expand_path("../../../#{file}", __FILE__) + ENV['MAIL_ROOM_GITLAB_CONFIG_FILE'] || File.expand_path('../../../config/gitlab.yml', __FILE__) end end end diff --git a/lib/gitlab/redis.rb b/lib/gitlab/redis.rb index 8c277d97f1a..9c4b01bfe1a 100644 --- a/lib/gitlab/redis.rb +++ b/lib/gitlab/redis.rb @@ -19,7 +19,7 @@ module Gitlab class << self def params - @params || PARAMS_MUTEX.synchronize { @params = new.params } + PARAMS_MUTEX.synchronize { new.params } end # @deprecated Use .params instead to get sentinel support @@ -38,7 +38,7 @@ module Gitlab end def initialize(rails_env=nil) - @rails_env = rails_env || Rails.env + @rails_env = rails_env || ::Rails.env end def params @@ -55,7 +55,7 @@ module Gitlab # Redis::Store does not handle Unix sockets well, so let's do it for them config[:path] = redis_uri.path else - redis_hash = ::Redis::Store::Factory.extract_host_options_from_uri(redis_uri) + redis_hash = ::Redis::Store::Factory.extract_host_options_from_uri(config[:url]) config.merge!(redis_hash) end @@ -74,7 +74,8 @@ module Gitlab end def fetch_config - File.exists?(config_file) ? YAML.load_file(config_file)[@rails_env] : false + file = config_file + File.exist?(file) ? YAML.load_file(file)[@rails_env] : false end def config_file -- cgit v1.2.1 From 67ae8adc72c1b59c440e0bfbde31df4e0b9920ec Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Sat, 9 Jul 2016 17:13:19 +0200 Subject: Fixed MailRoom specs and make sure it works with new resque.yml format Some codestyle changes --- lib/gitlab/mail_room.rb | 4 ++++ lib/gitlab/redis.rb | 6 +++++- 2 files changed, 9 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/gitlab/mail_room.rb b/lib/gitlab/mail_room.rb index 781c89579b6..b49cf1c633b 100644 --- a/lib/gitlab/mail_room.rb +++ b/lib/gitlab/mail_room.rb @@ -14,6 +14,10 @@ module Gitlab @config ||= fetch_config end + def reload_config! + @config = fetch_config + end + private def fetch_config diff --git a/lib/gitlab/redis.rb b/lib/gitlab/redis.rb index 9c4b01bfe1a..70e333eb29f 100644 --- a/lib/gitlab/redis.rb +++ b/lib/gitlab/redis.rb @@ -24,7 +24,7 @@ module Gitlab # @deprecated Use .params instead to get sentinel support def url - raw_config_hash[:url] + new.url end def with @@ -45,6 +45,10 @@ module Gitlab redis_store_options end + def url + raw_config_hash[:url] + end + private def redis_store_options -- cgit v1.2.1 From 3a93bae25f03a2992401e1de5bfbf52c3921b1a4 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Tue, 12 Jul 2016 17:10:03 +0200 Subject: Few minor fixes to Redis params order and commented out sentinel config in resque.yml.example Codestyle changes --- lib/gitlab/mail_room.rb | 1 - lib/gitlab/redis.rb | 13 +++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/mail_room.rb b/lib/gitlab/mail_room.rb index b49cf1c633b..1f68e09fa2b 100644 --- a/lib/gitlab/mail_room.rb +++ b/lib/gitlab/mail_room.rb @@ -4,7 +4,6 @@ require_relative 'redis' unless defined?(Gitlab::Redis) module Gitlab module MailRoom - class << self def enabled? config[:enabled] && config[:address] diff --git a/lib/gitlab/redis.rb b/lib/gitlab/redis.rb index 70e333eb29f..17ac15a01dd 100644 --- a/lib/gitlab/redis.rb +++ b/lib/gitlab/redis.rb @@ -53,18 +53,19 @@ module Gitlab def redis_store_options config = raw_config_hash + redis_url = config.delete(:url) + redis_uri = URI.parse(redis_url) - redis_uri = URI.parse(config[:url]) if redis_uri.scheme == 'unix' # Redis::Store does not handle Unix sockets well, so let's do it for them config[:path] = redis_uri.path + config else - redis_hash = ::Redis::Store::Factory.extract_host_options_from_uri(config[:url]) - config.merge!(redis_hash) + redis_hash = ::Redis::Store::Factory.extract_host_options_from_uri(redis_url) + # order is important here, sentinels must be after the connection keys. + # {url: ..., port: ..., sentinels: [...]} + redis_hash.merge(config) end - - config.delete(:url) - config end def raw_config_hash -- cgit v1.2.1 From ce41b5c73f7574ec19c47f24e9450ff1b54ef1b1 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Thu, 4 Aug 2016 18:55:24 +0200 Subject: Small refactor and a few documentation fixes --- lib/gitlab/mail_room.rb | 4 ++-- lib/gitlab/redis.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/mail_room.rb b/lib/gitlab/mail_room.rb index 1f68e09fa2b..12999a90a29 100644 --- a/lib/gitlab/mail_room.rb +++ b/lib/gitlab/mail_room.rb @@ -13,8 +13,8 @@ module Gitlab @config ||= fetch_config end - def reload_config! - @config = fetch_config + def reset_config! + @config = nil end private diff --git a/lib/gitlab/redis.rb b/lib/gitlab/redis.rb index 17ac15a01dd..72fa80e61e2 100644 --- a/lib/gitlab/redis.rb +++ b/lib/gitlab/redis.rb @@ -37,7 +37,7 @@ module Gitlab end end - def initialize(rails_env=nil) + def initialize(rails_env = nil) @rails_env = rails_env || ::Rails.env end -- cgit v1.2.1 From 482d7802cc71280595cad71882bf1b438461e435 Mon Sep 17 00:00:00 2001 From: tiagonbotelho Date: Mon, 1 Aug 2016 16:48:15 +0100 Subject: changes default_branch_protection to allow devs_can_merge protection option aswell --- lib/gitlab/user_access.rb | 2 ++ 1 file changed, 2 insertions(+) (limited to 'lib') diff --git a/lib/gitlab/user_access.rb b/lib/gitlab/user_access.rb index 3a69027368f..c55a7fc4d3d 100644 --- a/lib/gitlab/user_access.rb +++ b/lib/gitlab/user_access.rb @@ -30,6 +30,8 @@ module Gitlab return false unless user if project.protected_branch?(ref) + return true if project.empty_repo? && project.user_can_push_to_empty_repo?(user) + access_levels = project.protected_branches.matching(ref).map(&:push_access_level) access_levels.any? { |access_level| access_level.check_access(user) } else -- cgit v1.2.1 From ceff0c91700b15ecf26931c23f40637dcdf7204d Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Tue, 2 Aug 2016 13:41:22 -0300 Subject: Check out locally PRs where the source/target branch were removed --- lib/gitlab/github_import/importer.rb | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/github_import/importer.rb b/lib/gitlab/github_import/importer.rb index 3932fcb1eda..3b501533ffb 100644 --- a/lib/gitlab/github_import/importer.rb +++ b/lib/gitlab/github_import/importer.rb @@ -71,11 +71,12 @@ module Gitlab pull_requests = client.pull_requests(repo, state: :all, sort: :created, direction: :asc, per_page: 100) pull_requests = pull_requests.map { |raw| PullRequestFormatter.new(project, raw) }.select(&:valid?) - source_branches_removed = pull_requests.reject(&:source_branch_exists?).map { |pr| [pr.source_branch_name, pr.source_branch_sha] } + source_branches_removed = pull_requests.reject(&:source_branch_exists?).map { |pr| [pr.source_branch_name, pr.number] } target_branches_removed = pull_requests.reject(&:target_branch_exists?).map { |pr| [pr.target_branch_name, pr.target_branch_sha] } branches_removed = source_branches_removed | target_branches_removed - restore_branches(branches_removed) + restore_source_branches(source_branches_removed) + restore_target_branches(target_branches_removed) pull_requests.each do |pull_request| merge_request = pull_request.create! @@ -120,17 +121,20 @@ module Gitlab end end - def restore_branches(branches) - branches.each do |name, sha| - client.create_ref(repo, "refs/heads/#{name}", sha) + def restore_source_branches(branches) + branches.each do |name, number| + project.repository.fetch_ref(repo_url, "pull/#{number}/head", name) end + end - project.repository.fetch_ref(repo_url, '+refs/heads/*', 'refs/heads/*') + def restore_target_branches(branches) + branches.each do |name, sha| + project.repository.create_branch(name, sha) + end end def clean_up_restored_branches(branches) branches.each do |name, _| - client.delete_ref(repo, "heads/#{name}") project.repository.delete_branch(name) rescue Rugged::ReferenceError end -- cgit v1.2.1 From 51cf14b37c64b1afab25b10dfe94e281ff58d288 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Tue, 2 Aug 2016 13:46:02 -0300 Subject: Does not need to disable GitHub webhooks since PRs are check out locally --- lib/gitlab/github_import/hook_formatter.rb | 23 ---------------------- lib/gitlab/github_import/importer.rb | 31 ------------------------------ 2 files changed, 54 deletions(-) delete mode 100644 lib/gitlab/github_import/hook_formatter.rb (limited to 'lib') diff --git a/lib/gitlab/github_import/hook_formatter.rb b/lib/gitlab/github_import/hook_formatter.rb deleted file mode 100644 index db1fabaa18a..00000000000 --- a/lib/gitlab/github_import/hook_formatter.rb +++ /dev/null @@ -1,23 +0,0 @@ -module Gitlab - module GithubImport - class HookFormatter - EVENTS = %w[* create delete pull_request push].freeze - - attr_reader :raw - - delegate :id, :name, :active, to: :raw - - def initialize(raw) - @raw = raw - end - - def config - raw.config.attrs - end - - def valid? - (EVENTS & raw.events).any? && active - end - end - end -end diff --git a/lib/gitlab/github_import/importer.rb b/lib/gitlab/github_import/importer.rb index 3b501533ffb..294e44b7124 100644 --- a/lib/gitlab/github_import/importer.rb +++ b/lib/gitlab/github_import/importer.rb @@ -66,8 +66,6 @@ module Gitlab end def import_pull_requests - disable_webhooks - pull_requests = client.pull_requests(repo, state: :all, sort: :created, direction: :asc, per_page: 100) pull_requests = pull_requests.map { |raw| PullRequestFormatter.new(project, raw) }.select(&:valid?) @@ -90,35 +88,6 @@ module Gitlab raise Projects::ImportService::Error, e.message ensure clean_up_restored_branches(branches_removed) - clean_up_disabled_webhooks - end - - def disable_webhooks - update_webhooks(hooks, active: false) - end - - def clean_up_disabled_webhooks - update_webhooks(hooks, active: true) - end - - def update_webhooks(hooks, options) - hooks.each do |hook| - client.edit_hook(repo, hook.id, hook.name, hook.config, options) - end - end - - def hooks - @hooks ||= - begin - client.hooks(repo).map { |raw| HookFormatter.new(raw) }.select(&:valid?) - - # The GitHub Repository Webhooks API returns 404 for users - # without admin access to the repository when listing hooks. - # In this case we just want to return gracefully instead of - # spitting out an error and stop the import process. - rescue Octokit::NotFound - [] - end end def restore_source_branches(branches) -- cgit v1.2.1 From 1e736fb5272b75eb363a1ef766e29d35d53babb6 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Tue, 2 Aug 2016 23:25:45 -0300 Subject: Allow users to import cross-repository pull requests from GitHub --- lib/gitlab/github_import/pull_request_formatter.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/github_import/pull_request_formatter.rb b/lib/gitlab/github_import/pull_request_formatter.rb index a4ea2210abd..f7f8a4ce984 100644 --- a/lib/gitlab/github_import/pull_request_formatter.rb +++ b/lib/gitlab/github_import/pull_request_formatter.rb @@ -33,7 +33,7 @@ module Gitlab end def valid? - source_branch.valid? && target_branch.valid? && !cross_project? + source_branch.valid? && target_branch.valid? end def source_branch @@ -68,10 +68,6 @@ module Gitlab raw_data.body || "" end - def cross_project? - source_branch_repo.id != target_branch_repo.id - end - def description formatter.author_line(author) + body end -- cgit v1.2.1 From b791dcc05b379a64c1370bc4be8d0aac60b9c31b Mon Sep 17 00:00:00 2001 From: winniehell Date: Fri, 5 Aug 2016 01:22:50 +0200 Subject: Ignore URLs starting with // (!5677) --- lib/banzai/filter/relative_link_filter.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/banzai/filter/relative_link_filter.rb b/lib/banzai/filter/relative_link_filter.rb index 5b73fc8fcee..46762d401fb 100644 --- a/lib/banzai/filter/relative_link_filter.rb +++ b/lib/banzai/filter/relative_link_filter.rb @@ -35,6 +35,7 @@ module Banzai def process_link_attr(html_attr) return if html_attr.blank? + return if html_attr.value.start_with?('//') uri = URI(html_attr.value) if uri.relative? && uri.path.present? @@ -92,7 +93,7 @@ module Banzai parts = request_path.split('/') parts.pop if uri_type(request_path) != :tree - path.sub!(%r{^\./}, '') + path.sub!(%r{\A\./}, '') while path.start_with?('../') parts.pop -- cgit v1.2.1 From 554e18ab025fcd86001faa57fab14fe3ca28a672 Mon Sep 17 00:00:00 2001 From: "Z.J. van de Weg" Date: Fri, 5 Aug 2016 11:35:44 +0200 Subject: Create service for enabling deploy keys --- lib/api/deploy_keys.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'lib') diff --git a/lib/api/deploy_keys.rb b/lib/api/deploy_keys.rb index 52f89373ad3..6dc9beb57ec 100644 --- a/lib/api/deploy_keys.rb +++ b/lib/api/deploy_keys.rb @@ -76,12 +76,12 @@ module API requires :key_id, type: Integer, desc: 'The ID of the deploy key' end post ":id/#{path}/:key_id/enable" do - key = DeployKey.find(params[:key_id]) + key = EnableDeployKeyService.new(user_project, current_user, declared(params)).execute - if user_project.deploy_keys << key + if key present key, with: Entities::SSHKey else - render_validation_error!(key) + not_found!('Deploy Key') end end -- cgit v1.2.1 From c74005e75cf29eb14d2e9f5a2c3744b6e06ded0a Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Thu, 28 Jul 2016 15:12:49 +0200 Subject: Log base64-decoded PostReceive arguments The change to base64-encoding the third argument to PostReceive in gitlab-shell made our Sidekiq ArgumentsLogger a little less useful. This change adds a log statement for the decoded data. Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/20381 --- lib/gitlab/git_post_receive.rb | 1 - 1 file changed, 1 deletion(-) (limited to 'lib') diff --git a/lib/gitlab/git_post_receive.rb b/lib/gitlab/git_post_receive.rb index a088e19d1e7..d32bdd86427 100644 --- a/lib/gitlab/git_post_receive.rb +++ b/lib/gitlab/git_post_receive.rb @@ -39,7 +39,6 @@ module Gitlab end def deserialize_changes(changes) - changes = Base64.decode64(changes) unless changes.include?(' ') changes = utf8_encode_changes(changes) changes.lines end -- cgit v1.2.1 From 2aa2f52191b746df851853cf5fe9ce7249a70739 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Sat, 6 Aug 2016 03:44:39 +0200 Subject: Enable Style/EmptyLinesAroundModuleBody cop --- lib/banzai/filter/video_link_filter.rb | 2 -- 1 file changed, 2 deletions(-) (limited to 'lib') diff --git a/lib/banzai/filter/video_link_filter.rb b/lib/banzai/filter/video_link_filter.rb index fd8b9a6f0cc..0f86ae83f72 100644 --- a/lib/banzai/filter/video_link_filter.rb +++ b/lib/banzai/filter/video_link_filter.rb @@ -1,6 +1,5 @@ module Banzai module Filter - # Find every image that isn't already wrapped in an `a` tag, and that has # a `src` attribute ending with a video extension, add a new video node and # a "Download" link in the case the video cannot be played. @@ -54,6 +53,5 @@ module Banzai container end end - end end -- cgit v1.2.1 From 5f6223cf9f285da3814991d1271e328e23be9d45 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Sat, 6 Aug 2016 03:52:24 +0200 Subject: Enable Style/EmptyLinesAroundClassBody cop --- lib/banzai/filter/video_link_filter.rb | 1 - lib/gitlab/import_export/avatar_restorer.rb | 1 - 2 files changed, 2 deletions(-) (limited to 'lib') diff --git a/lib/banzai/filter/video_link_filter.rb b/lib/banzai/filter/video_link_filter.rb index 0f86ae83f72..ac7bbcb0d10 100644 --- a/lib/banzai/filter/video_link_filter.rb +++ b/lib/banzai/filter/video_link_filter.rb @@ -4,7 +4,6 @@ module Banzai # a `src` attribute ending with a video extension, add a new video node and # a "Download" link in the case the video cannot be played. class VideoLinkFilter < HTML::Pipeline::Filter - def call doc.xpath(query).each do |el| el.replace(video_node(doc, el)) diff --git a/lib/gitlab/import_export/avatar_restorer.rb b/lib/gitlab/import_export/avatar_restorer.rb index 352539eb594..cfa595629f4 100644 --- a/lib/gitlab/import_export/avatar_restorer.rb +++ b/lib/gitlab/import_export/avatar_restorer.rb @@ -1,7 +1,6 @@ module Gitlab module ImportExport class AvatarRestorer - def initialize(project:, shared:) @project = project @shared = shared -- cgit v1.2.1 From c9aa19881cf719baaea1bbb9bb00f84145a99b8b Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Sat, 6 Aug 2016 04:03:01 +0200 Subject: Enable Style/SpaceAroundEqualsInParameterDefault cop --- lib/gitlab/ldap/access.rb | 2 +- lib/gitlab/ldap/adapter.rb | 2 +- lib/gitlab/popen.rb | 2 +- lib/gitlab/redis.rb | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/ldap/access.rb b/lib/gitlab/ldap/access.rb index f2b649e50a2..2f326d00a2f 100644 --- a/lib/gitlab/ldap/access.rb +++ b/lib/gitlab/ldap/access.rb @@ -25,7 +25,7 @@ module Gitlab end end - def initialize(user, adapter=nil) + def initialize(user, adapter = nil) @adapter = adapter @user = user @provider = user.ldap_identity.provider diff --git a/lib/gitlab/ldap/adapter.rb b/lib/gitlab/ldap/adapter.rb index df65179bfea..9a5bcfb5c9b 100644 --- a/lib/gitlab/ldap/adapter.rb +++ b/lib/gitlab/ldap/adapter.rb @@ -13,7 +13,7 @@ module Gitlab Gitlab::LDAP::Config.new(provider) end - def initialize(provider, ldap=nil) + def initialize(provider, ldap = nil) @provider = provider @ldap = ldap || Net::LDAP.new(config.adapter_options) end diff --git a/lib/gitlab/popen.rb b/lib/gitlab/popen.rb index 43e07e09160..ca23ccef25b 100644 --- a/lib/gitlab/popen.rb +++ b/lib/gitlab/popen.rb @@ -5,7 +5,7 @@ module Gitlab module Popen extend self - def popen(cmd, path=nil) + def popen(cmd, path = nil) unless cmd.is_a?(Array) raise "System commands must be given as an array of strings" end diff --git a/lib/gitlab/redis.rb b/lib/gitlab/redis.rb index 40766f35f77..1f92986ec9a 100644 --- a/lib/gitlab/redis.rb +++ b/lib/gitlab/redis.rb @@ -37,7 +37,7 @@ module Gitlab redis_config_hash end - def initialize(rails_env=nil) + def initialize(rails_env = nil) rails_env ||= Rails.env config_file = File.expand_path('../../../config/resque.yml', __FILE__) -- cgit v1.2.1 From ed0a7c254ab32130296fb7ee467f655d200d7854 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Sat, 6 Aug 2016 03:01:52 +0200 Subject: Small refactor in Redis class and improved specs --- lib/gitlab/redis.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/gitlab/redis.rb b/lib/gitlab/redis.rb index 72fa80e61e2..9376b54f43b 100644 --- a/lib/gitlab/redis.rb +++ b/lib/gitlab/redis.rb @@ -19,7 +19,7 @@ module Gitlab class << self def params - PARAMS_MUTEX.synchronize { new.params } + @params || PARAMS_MUTEX.synchronize { @params = new.params } end # @deprecated Use .params instead to get sentinel support @@ -35,6 +35,10 @@ module Gitlab end @pool.with { |redis| yield redis } end + + def reset_params! + @params = nil + end end def initialize(rails_env = nil) -- cgit v1.2.1 From 1b5e2303debf00784603d6bd9d87d613de3c8091 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 28 Jul 2016 15:39:41 +0200 Subject: Use new badge template to render build status badge --- lib/gitlab/badge/build.rb | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'lib') diff --git a/lib/gitlab/badge/build.rb b/lib/gitlab/badge/build.rb index 7bc6f285ce1..c94ef3e9678 100644 --- a/lib/gitlab/badge/build.rb +++ b/lib/gitlab/badge/build.rb @@ -21,6 +21,10 @@ module Gitlab Build::Metadata.new(@project, @ref) end + def template + Build::Template.new(status) + end + def type 'image/svg+xml' end -- cgit v1.2.1 From bc17996227117bcaf11dc41a0dd2b2f11a7329f4 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 8 Aug 2016 12:38:36 +0200 Subject: Improve builds badge specs, remove legacy methods --- lib/gitlab/badge/build.rb | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/badge/build.rb b/lib/gitlab/badge/build.rb index c94ef3e9678..1de721a2269 100644 --- a/lib/gitlab/badge/build.rb +++ b/lib/gitlab/badge/build.rb @@ -4,35 +4,26 @@ module Gitlab # Build badge # class Build + delegate :key_text, :value_text, to: :template + def initialize(project, ref) @project = project @ref = ref + @sha = @project.commit(@ref).try(:sha) end def status - sha = @project.commit(@ref).try(:sha) - @project.pipelines - .where(sha: sha, ref: @ref) + .where(sha: @sha, ref: @ref) .status || 'unknown' end def metadata - Build::Metadata.new(@project, @ref) + @metadata ||= Build::Metadata.new(@project, @ref) end def template - Build::Template.new(status) - end - - def type - 'image/svg+xml' - end - - def data - File.read( - Rails.root.join('public/ci', 'build-' + status + '.svg') - ) + @template ||= Build::Template.new(status) end end end -- cgit v1.2.1 From 427c9f0b5b5f6f0c242e75a98dca2434a27945d8 Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Mon, 8 Aug 2016 13:02:44 +0200 Subject: Revert "Defend against 'Host' header injection" This reverts commit 47b5b441395921e9f8e9982bb3f560e5db5a67bc. See https://gitlab.com/gitlab-org/gitlab-ce/issues/17877#note_13488047 --- lib/support/nginx/gitlab | 7 +------ lib/support/nginx/gitlab-ssl | 7 +------ 2 files changed, 2 insertions(+), 12 deletions(-) (limited to 'lib') diff --git a/lib/support/nginx/gitlab b/lib/support/nginx/gitlab index 4a4892a2e07..d521de28e8a 100644 --- a/lib/support/nginx/gitlab +++ b/lib/support/nginx/gitlab @@ -49,12 +49,7 @@ server { proxy_http_version 1.1; - ## By overwriting Host and clearing X-Forwarded-Host we ensure that - ## internal HTTP redirects generated by GitLab always send users to - ## YOUR_SERVER_FQDN. - proxy_set_header Host YOUR_SERVER_FQDN; - proxy_set_header X-Forwarded-Host ""; - + proxy_set_header Host $http_host; proxy_set_header X-Real-IP $remote_addr; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; proxy_set_header X-Forwarded-Proto $scheme; diff --git a/lib/support/nginx/gitlab-ssl b/lib/support/nginx/gitlab-ssl index 0b93d7f292f..bf014b56cf6 100644 --- a/lib/support/nginx/gitlab-ssl +++ b/lib/support/nginx/gitlab-ssl @@ -93,12 +93,7 @@ server { proxy_http_version 1.1; - ## By overwriting Host and clearing X-Forwarded-Host we ensure that - ## internal HTTP redirects generated by GitLab always send users to - ## YOUR_SERVER_FQDN. - proxy_set_header Host YOUR_SERVER_FQDN; - proxy_set_header X-Forwarded-Host ""; - + proxy_set_header Host $http_host; proxy_set_header X-Real-IP $remote_addr; proxy_set_header X-Forwarded-Ssl on; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; -- cgit v1.2.1 From 89f2be7d5867991c1fe964e8d9a94ff64c13ce61 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 8 Aug 2016 12:49:26 +0200 Subject: Improve indentation in svg badge template --- lib/gitlab/badge/build/template.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/gitlab/badge/build/template.rb b/lib/gitlab/badge/build/template.rb index a7c2e176935..deba3b669b3 100644 --- a/lib/gitlab/badge/build/template.rb +++ b/lib/gitlab/badge/build/template.rb @@ -2,7 +2,7 @@ module Gitlab module Badge class Build ## - # Abstract class for build badge template. + # Class that represents a build badge template. # # Template object will be passed to badge.svg.erb template. # -- cgit v1.2.1 From 74d12b6b4708164fe14c6019874384615ed3c711 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 8 Aug 2016 14:22:26 +0200 Subject: Remove legacy Ci::StaticModel we do not use anymore --- lib/ci/static_model.rb | 49 ------------------------------------------------- 1 file changed, 49 deletions(-) delete mode 100644 lib/ci/static_model.rb (limited to 'lib') diff --git a/lib/ci/static_model.rb b/lib/ci/static_model.rb deleted file mode 100644 index bb2bdbed495..00000000000 --- a/lib/ci/static_model.rb +++ /dev/null @@ -1,49 +0,0 @@ -# Provides an ActiveRecord-like interface to a model whose data is not persisted to a database. -module Ci - module StaticModel - extend ActiveSupport::Concern - - module ClassMethods - # Used by ActiveRecord's polymorphic association to set object_id - def primary_key - 'id' - end - - # Used by ActiveRecord's polymorphic association to set object_type - def base_class - self - end - end - - # Used by AR for fetching attributes - # - # Pass it along if we respond to it. - def [](key) - send(key) if respond_to?(key) - end - - def to_param - id - end - - def new_record? - false - end - - def persisted? - false - end - - def destroyed? - false - end - - def ==(other) - if other.is_a? ::Ci::StaticModel - id == other.id - else - super - end - end - end -end -- cgit v1.2.1 From f04e9dad476cb6b2ac338dd8730d79b22f3fd070 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Fri, 5 Aug 2016 15:57:47 +0200 Subject: Support pending invitation project members importing projects --- lib/gitlab/import_export/members_mapper.rb | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/import_export/members_mapper.rb b/lib/gitlab/import_export/members_mapper.rb index b459054c198..36c4cf6efa0 100644 --- a/lib/gitlab/import_export/members_mapper.rb +++ b/lib/gitlab/import_export/members_mapper.rb @@ -18,11 +18,14 @@ module Gitlab @map ||= begin @exported_members.inject(missing_keys_tracking_hash) do |hash, member| - existing_user = User.where(find_project_user_query(member)).first - old_user_id = member['user']['id'] - if existing_user && add_user_as_team_member(existing_user, member) - hash[old_user_id] = existing_user.id + if member['user'] + old_user_id = member['user']['id'] + existing_user = User.where(find_project_user_query(member)).first + hash[old_user_id] = existing_user.id if existing_user && add_team_member(member, existing_user) + else + add_team_member(member) end + hash end end @@ -45,7 +48,7 @@ module Gitlab ProjectMember.create!(user: @user, access_level: ProjectMember::MASTER, source_id: @project.id, importing: true) end - def add_user_as_team_member(existing_user, member) + def add_team_member(member, existing_user = nil) member['user'] = existing_user ProjectMember.create(member_hash(member)).persisted? -- cgit v1.2.1 From 7e47a82899bdb10d2cdc61ce237a25bfa7f8a392 Mon Sep 17 00:00:00 2001 From: "Z.J. van de Weg" Date: Mon, 8 Aug 2016 20:32:10 +0200 Subject: Namespace EnableDeployKeyService under Projects --- lib/api/deploy_keys.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/api/deploy_keys.rb b/lib/api/deploy_keys.rb index 6dc9beb57ec..825e05fbae3 100644 --- a/lib/api/deploy_keys.rb +++ b/lib/api/deploy_keys.rb @@ -76,7 +76,8 @@ module API requires :key_id, type: Integer, desc: 'The ID of the deploy key' end post ":id/#{path}/:key_id/enable" do - key = EnableDeployKeyService.new(user_project, current_user, declared(params)).execute + key = ::Projects::EnableDeployKeyService.new(user_project, + current_user, declared(params)).execute if key present key, with: Entities::SSHKey -- cgit v1.2.1 From 167a0c7f0bd88e565f580e144b59d9eebe7e24f6 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Thu, 4 Aug 2016 17:15:00 -0300 Subject: Remove SHA suffix for removed branches name when importing PR from GH --- lib/gitlab/github_import/branch_formatter.rb | 2 +- lib/gitlab/github_import/importer.rb | 54 ++++++++++++++-------------- 2 files changed, 27 insertions(+), 29 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/github_import/branch_formatter.rb b/lib/gitlab/github_import/branch_formatter.rb index 7d2d545b84e..4be4fc8fe37 100644 --- a/lib/gitlab/github_import/branch_formatter.rb +++ b/lib/gitlab/github_import/branch_formatter.rb @@ -8,7 +8,7 @@ module Gitlab end def name - @name ||= exists? ? ref : "#{ref}-#{short_id}" + ref end def valid? diff --git a/lib/gitlab/github_import/importer.rb b/lib/gitlab/github_import/importer.rb index 294e44b7124..9ddc8905bd6 100644 --- a/lib/gitlab/github_import/importer.rb +++ b/lib/gitlab/github_import/importer.rb @@ -12,7 +12,6 @@ module Gitlab if credentials @client = Client.new(credentials[:user]) - @formatter = Gitlab::ImportFormatter.new else raise Projects::ImportService::Error, "Unable to find project import data credentials for project ID: #{@project.id}" end @@ -69,43 +68,42 @@ module Gitlab pull_requests = client.pull_requests(repo, state: :all, sort: :created, direction: :asc, per_page: 100) pull_requests = pull_requests.map { |raw| PullRequestFormatter.new(project, raw) }.select(&:valid?) - source_branches_removed = pull_requests.reject(&:source_branch_exists?).map { |pr| [pr.source_branch_name, pr.number] } - target_branches_removed = pull_requests.reject(&:target_branch_exists?).map { |pr| [pr.target_branch_name, pr.target_branch_sha] } - branches_removed = source_branches_removed | target_branches_removed - - restore_source_branches(source_branches_removed) - restore_target_branches(target_branches_removed) - pull_requests.each do |pull_request| - merge_request = pull_request.create! - apply_labels(merge_request) - import_comments(merge_request) - import_comments_on_diff(merge_request) + begin + restore_source_branch(pull_request) unless pull_request.source_branch_exists? + restore_target_branch(pull_request) unless pull_request.target_branch_exists? + + merge_request = pull_request.create! + apply_labels(merge_request) + import_comments(merge_request) + import_comments_on_diff(merge_request) + rescue ActiveRecord::RecordInvalid => e + raise Projects::ImportService::Error, e.message + ensure + clean_up_restored_branches(pull_request) + end end true - rescue ActiveRecord::RecordInvalid => e - raise Projects::ImportService::Error, e.message - ensure - clean_up_restored_branches(branches_removed) end - def restore_source_branches(branches) - branches.each do |name, number| - project.repository.fetch_ref(repo_url, "pull/#{number}/head", name) - end + def restore_source_branch(pull_request) + project.repository.fetch_ref(repo_url, "pull/#{pull_request.number}/head", pull_request.source_branch_name) end - def restore_target_branches(branches) - branches.each do |name, sha| - project.repository.create_branch(name, sha) - end + def restore_target_branch(pull_request) + project.repository.create_branch(pull_request.target_branch_name, pull_request.target_branch_sha) end - def clean_up_restored_branches(branches) - branches.each do |name, _| - project.repository.delete_branch(name) rescue Rugged::ReferenceError - end + def remove_branch(name) + project.repository.delete_branch(name) + rescue Rugged::ReferenceError + nil + end + + def clean_up_restored_branches(pull_request) + remove_branch(pull_request.source_branch_name) unless pull_request.source_branch_exists? + remove_branch(pull_request.target_branch_name) unless pull_request.target_branch_exists? project.repository.after_remove_branch end -- cgit v1.2.1 From 0a1535a9f44b12accdf3d6585ff7fee53737da51 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Mon, 8 Aug 2016 20:24:40 -0300 Subject: Prefixes removed branches name with PR number when importing PR from GH --- lib/gitlab/github_import/branch_formatter.rb | 4 ---- lib/gitlab/github_import/pull_request_formatter.rb | 16 ++++++++++++++-- 2 files changed, 14 insertions(+), 6 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/github_import/branch_formatter.rb b/lib/gitlab/github_import/branch_formatter.rb index 4be4fc8fe37..4750675ae9d 100644 --- a/lib/gitlab/github_import/branch_formatter.rb +++ b/lib/gitlab/github_import/branch_formatter.rb @@ -7,10 +7,6 @@ module Gitlab branch_exists? && commit_exists? end - def name - ref - end - def valid? repo.present? end diff --git a/lib/gitlab/github_import/pull_request_formatter.rb b/lib/gitlab/github_import/pull_request_formatter.rb index f7f8a4ce984..b84538a090a 100644 --- a/lib/gitlab/github_import/pull_request_formatter.rb +++ b/lib/gitlab/github_import/pull_request_formatter.rb @@ -1,8 +1,8 @@ module Gitlab module GithubImport class PullRequestFormatter < BaseFormatter - delegate :exists?, :name, :project, :repo, :sha, to: :source_branch, prefix: true - delegate :exists?, :name, :project, :repo, :sha, to: :target_branch, prefix: true + delegate :exists?, :project, :ref, :repo, :sha, to: :source_branch, prefix: true + delegate :exists?, :project, :ref, :repo, :sha, to: :target_branch, prefix: true def attributes { @@ -40,10 +40,22 @@ module Gitlab @source_branch ||= BranchFormatter.new(project, raw_data.head) end + def source_branch_name + @source_branch_name ||= begin + source_branch_exists? ? source_branch_ref : "pull/#{number}/#{source_branch_ref}" + end + end + def target_branch @target_branch ||= BranchFormatter.new(project, raw_data.base) end + def target_branch_name + @target_branch_name ||= begin + target_branch_exists? ? target_branch_ref : "pull/#{number}/#{target_branch_ref}" + end + end + private def assigned? -- cgit v1.2.1 From ec298011f2da1b94232e601c8e857ca180a2274d Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 9 Aug 2016 06:48:23 +0000 Subject: Rails prefers require_dependency so that it won't require twice: Closes #20724 --- lib/gitlab/email/receiver.rb | 2 +- lib/gitlab/request_profiler/middleware.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/email/receiver.rb b/lib/gitlab/email/receiver.rb index 9213cfb51e8..a40c44eb1bc 100644 --- a/lib/gitlab/email/receiver.rb +++ b/lib/gitlab/email/receiver.rb @@ -1,5 +1,5 @@ -require 'gitlab/email/handler' +require_dependency 'gitlab/email/handler' # Inspired in great part by Discourse's Email::Receiver module Gitlab diff --git a/lib/gitlab/request_profiler/middleware.rb b/lib/gitlab/request_profiler/middleware.rb index 4e787dc0656..786e1d49f5e 100644 --- a/lib/gitlab/request_profiler/middleware.rb +++ b/lib/gitlab/request_profiler/middleware.rb @@ -1,5 +1,5 @@ require 'ruby-prof' -require 'gitlab/request_profiler' +require_dependency 'gitlab/request_profiler' module Gitlab module RequestProfiler -- cgit v1.2.1 From f8e854798032d71e3b3cb2bcf49cf104e5d0e611 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Fri, 5 Aug 2016 10:56:27 +0200 Subject: fix MR source project assignment --- lib/gitlab/import_export/relation_factory.rb | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/import_export/relation_factory.rb b/lib/gitlab/import_export/relation_factory.rb index 5e56b3d1aa7..b0726268ca6 100644 --- a/lib/gitlab/import_export/relation_factory.rb +++ b/lib/gitlab/import_export/relation_factory.rb @@ -102,17 +102,19 @@ module Gitlab def update_project_references project_id = @relation_hash.delete('project_id') + # If source and target are the same, populate them with the new project ID. + if @relation_hash['source_project_id'] + @relation_hash['source_project_id'] = same_source_and_target? ? project_id : -1 + end + # project_id may not be part of the export, but we always need to populate it if required. @relation_hash['project_id'] = project_id @relation_hash['gl_project_id'] = project_id if @relation_hash['gl_project_id'] @relation_hash['target_project_id'] = project_id if @relation_hash['target_project_id'] - @relation_hash['source_project_id'] = -1 if @relation_hash['source_project_id'] + end - # If source and target are the same, populate them with the new project ID. - if @relation_hash['source_project_id'] && @relation_hash['target_project_id'] && - @relation_hash['target_project_id'] == @relation_hash['source_project_id'] - @relation_hash['source_project_id'] = project_id - end + def same_source_and_target? + @relation_hash['target_project_id'] && @relation_hash['target_project_id'] == @relation_hash['source_project_id'] end def reset_ci_tokens -- cgit v1.2.1 From 57451f52cdbd527a980c0df75e1ee8bb7897d2e9 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 9 Aug 2016 11:29:32 +0200 Subject: Memoize CI config node validator to prevent leaks --- lib/gitlab/ci/config/node/validatable.rb | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/ci/config/node/validatable.rb b/lib/gitlab/ci/config/node/validatable.rb index f6e2896dfb2..085e6e988d1 100644 --- a/lib/gitlab/ci/config/node/validatable.rb +++ b/lib/gitlab/ci/config/node/validatable.rb @@ -7,13 +7,11 @@ module Gitlab class_methods do def validator - validator = Class.new(Node::Validator) - - if defined?(@validations) - @validations.each { |rules| validator.class_eval(&rules) } + @validator ||= Class.new(Node::Validator).tap do |validator| + if defined?(@validations) + @validations.each { |rules| validator.class_eval(&rules) } + end end - - validator end private -- cgit v1.2.1 From c53b599e61027e910aa0425150373cb30c67b150 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Tue, 2 Aug 2016 15:56:27 -0600 Subject: Retain old behavior --- lib/api/api.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/api/api.rb b/lib/api/api.rb index bd16806892b..6cd4a853dbe 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -7,8 +7,10 @@ module API rack_response({ 'message' => '404 Not found' }.to_json, 404) end - rescue_from Grape::Exceptions::ValidationErrors do |e| - error!({ messages: e.full_messages }, 400) + # Retain 405 error rather than a 500 error for Grape 0.15.0+. + # See: https://github.com/ruby-grape/grape/commit/252bfd27c320466ec3c0751812cf44245e97e5de + rescue_from Grape::Exceptions::Base do |e| + error! e.message, e.status, e.headers end rescue_from :all do |exception| -- cgit v1.2.1 From 12e93d6f4b3af60d8f1404aa99625f8978075bf3 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 9 Aug 2016 18:29:14 -0500 Subject: Rename `run` task helper method to prevent conflict with StateMachine This prevents the following message from appearing whenever running a Rake task: Instance method "run" is already defined in Object, use generic helper instead or set StateMachines::Machine.ignore_method_conflicts = true. --- lib/tasks/gitlab/check.rake | 8 ++++---- lib/tasks/gitlab/info.rake | 4 ++-- lib/tasks/gitlab/task_helpers.rake | 14 +++++++------- lib/tasks/spinach.rake | 6 ++---- 4 files changed, 15 insertions(+), 17 deletions(-) (limited to 'lib') diff --git a/lib/tasks/gitlab/check.rake b/lib/tasks/gitlab/check.rake index 60f4636e737..0894994200f 100644 --- a/lib/tasks/gitlab/check.rake +++ b/lib/tasks/gitlab/check.rake @@ -46,7 +46,7 @@ namespace :gitlab do } correct_options = options.map do |name, value| - run(%W(#{Gitlab.config.git.bin_path} config --global --get #{name})).try(:squish) == value + run_command(%W(#{Gitlab.config.git.bin_path} config --global --get #{name})).try(:squish) == value end if correct_options.all? @@ -316,7 +316,7 @@ namespace :gitlab do min_redis_version = "2.8.0" print "Redis version >= #{min_redis_version}? ... " - redis_version = run(%W(redis-cli --version)) + redis_version = run_command(%W(redis-cli --version)) redis_version = redis_version.try(:match, /redis-cli (\d+\.\d+\.\d+)/) if redis_version && (Gem::Version.new(redis_version[1]) > Gem::Version.new(min_redis_version)) @@ -893,7 +893,7 @@ namespace :gitlab do def check_ruby_version required_version = Gitlab::VersionInfo.new(2, 1, 0) - current_version = Gitlab::VersionInfo.parse(run(%W(ruby --version))) + current_version = Gitlab::VersionInfo.parse(run_command(%W(ruby --version))) print "Ruby version >= #{required_version} ? ... " @@ -910,7 +910,7 @@ namespace :gitlab do def check_git_version required_version = Gitlab::VersionInfo.new(2, 7, 3) - current_version = Gitlab::VersionInfo.parse(run(%W(#{Gitlab.config.git.bin_path} --version))) + current_version = Gitlab::VersionInfo.parse(run_command(%W(#{Gitlab.config.git.bin_path} --version))) puts "Your git bin path is \"#{Gitlab.config.git.bin_path}\"" print "Git version >= #{required_version} ? ... " diff --git a/lib/tasks/gitlab/info.rake b/lib/tasks/gitlab/info.rake index fe43d40e6d2..dffea8ed155 100644 --- a/lib/tasks/gitlab/info.rake +++ b/lib/tasks/gitlab/info.rake @@ -8,7 +8,7 @@ namespace :gitlab do # check Ruby version ruby_version = run_and_match(%W(ruby --version), /[\d\.p]+/).try(:to_s) # check Gem version - gem_version = run(%W(gem --version)) + gem_version = run_command(%W(gem --version)) # check Bundler version bunder_version = run_and_match(%W(bundle --version), /[\d\.]+/).try(:to_s) # check Bundler version @@ -17,7 +17,7 @@ namespace :gitlab do puts "" puts "System information".color(:yellow) puts "System:\t\t#{os_name || "unknown".color(:red)}" - puts "Current User:\t#{run(%W(whoami))}" + puts "Current User:\t#{run_command(%W(whoami))}" puts "Using RVM:\t#{rvm_version.present? ? "yes".color(:green) : "no"}" puts "RVM Version:\t#{rvm_version}" if rvm_version.present? puts "Ruby Version:\t#{ruby_version || "unknown".color(:red)}" diff --git a/lib/tasks/gitlab/task_helpers.rake b/lib/tasks/gitlab/task_helpers.rake index ab96b1d3593..74be413423a 100644 --- a/lib/tasks/gitlab/task_helpers.rake +++ b/lib/tasks/gitlab/task_helpers.rake @@ -23,7 +23,7 @@ namespace :gitlab do # It will primarily use lsb_relase to determine the OS. # It has fallbacks to Debian, SuSE, OS X and systems running systemd. def os_name - os_name = run(%W(lsb_release -irs)) + os_name = run_command(%W(lsb_release -irs)) os_name ||= if File.readable?('/etc/system-release') File.read('/etc/system-release') end @@ -34,7 +34,7 @@ namespace :gitlab do os_name ||= if File.readable?('/etc/SuSE-release') File.read('/etc/SuSE-release') end - os_name ||= if os_x_version = run(%W(sw_vers -productVersion)) + os_name ||= if os_x_version = run_command(%W(sw_vers -productVersion)) "Mac OS X #{os_x_version}" end os_name ||= if File.readable?('/etc/os-release') @@ -62,10 +62,10 @@ namespace :gitlab do # Returns nil if nothing matched # Returns the MatchData if the pattern matched # - # see also #run + # see also #run_command # see also String#match def run_and_match(command, regexp) - run(command).try(:match, regexp) + run_command(command).try(:match, regexp) end # Runs the given command @@ -74,7 +74,7 @@ namespace :gitlab do # Returns the output of the command otherwise # # see also #run_and_match - def run(command) + def run_command(command) output, _ = Gitlab::Popen.popen(command) output rescue Errno::ENOENT @@ -82,7 +82,7 @@ namespace :gitlab do end def uid_for(user_name) - run(%W(id -u #{user_name})).chomp.to_i + run_command(%W(id -u #{user_name})).chomp.to_i end def gid_for(group_name) @@ -96,7 +96,7 @@ namespace :gitlab do def warn_user_is_not_gitlab unless @warned_user_not_gitlab gitlab_user = Gitlab.config.gitlab.user - current_user = run(%W(whoami)).chomp + current_user = run_command(%W(whoami)).chomp unless current_user == gitlab_user puts " Warning ".color(:black).background(:yellow) puts " You are running as user #{current_user.color(:magenta)}, we hope you know what you are doing." diff --git a/lib/tasks/spinach.rake b/lib/tasks/spinach.rake index da255f5464b..c0f860a82d2 100644 --- a/lib/tasks/spinach.rake +++ b/lib/tasks/spinach.rake @@ -34,17 +34,15 @@ task :spinach do run_spinach_tests(nil) end -def run_command(cmd) +def run_system_command(cmd) system({'RAILS_ENV' => 'test', 'force' => 'yes'}, *cmd) end def run_spinach_command(args) - run_command(%w(spinach -r rerun) + args) + run_system_command(%w(spinach -r rerun) + args) end def run_spinach_tests(tags) - #run_command(%w(rake gitlab:setup)) or raise('gitlab:setup failed!') - success = run_spinach_command(%W(--tags #{tags})) 3.times do |_| break if success -- cgit v1.2.1 From 4955a47cb1c52168114364e45a2fccf6bc105452 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Sat, 6 Aug 2016 07:25:51 -0700 Subject: Clean up project destruction Instead of redirecting from the project service to the service and back to the model, put all destruction code in the service. Also removes a possible source of failure where run_after_commit may not destroy the project. --- lib/api/projects.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/api/projects.rb b/lib/api/projects.rb index 8fed7db8803..60cfc103afd 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -323,7 +323,7 @@ module API # DELETE /projects/:id delete ":id" do authorize! :remove_project, user_project - ::Projects::DestroyService.new(user_project, current_user, {}).pending_delete! + ::Projects::DestroyService.new(user_project, current_user, {}).async_execute end # Mark this project as forked from another -- cgit v1.2.1 From 29850364eccccc3ce7305f6706cea1d5d073de2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Thu, 23 Jun 2016 17:14:31 +0200 Subject: New AccessRequests API endpoints for Group & Project MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also, mutualize AccessRequests and Members endpoints for Group & Project. New API documentation for the AccessRequests endpoints. Signed-off-by: Rémy Coutable --- lib/api/access_requests.rb | 91 +++++++++++++++++++++++++++ lib/api/api.rb | 8 ++- lib/api/entities.rb | 24 +++---- lib/api/group_members.rb | 87 -------------------------- lib/api/helpers.rb | 25 ++------ lib/api/helpers/members_helpers.rb | 13 ++++ lib/api/members.rb | 124 +++++++++++++++++++++++++++++++++++++ lib/api/project_members.rb | 110 -------------------------------- 8 files changed, 253 insertions(+), 229 deletions(-) create mode 100644 lib/api/access_requests.rb delete mode 100644 lib/api/group_members.rb create mode 100644 lib/api/helpers/members_helpers.rb create mode 100644 lib/api/members.rb delete mode 100644 lib/api/project_members.rb (limited to 'lib') diff --git a/lib/api/access_requests.rb b/lib/api/access_requests.rb new file mode 100644 index 00000000000..9c41d8aaa3e --- /dev/null +++ b/lib/api/access_requests.rb @@ -0,0 +1,91 @@ +module API + class AccessRequests < Grape::API + before { authenticate! } + + helpers ::API::Helpers::MembersHelpers + + %w[group project].each do |source_type| + resource source_type.pluralize do + # Get a list of group/project access requests viewable by the authenticated user. + # + # Parameters: + # id (required) - The group/project ID + # + # Example Request: + # GET /groups/:id/access_requests + # GET /projects/:id/access_requests + get ":id/access_requests" do + source = find_source(source_type, params[:id]) + authorize_admin_source!(source_type, source) + + access_requesters = source.requesters + users = Kaminari.paginate_array(access_requesters.map(&:user)) + + present paginate(users), with: Entities::AccessRequester, source: source + end + + # Request access to the group/project + # + # Parameters: + # id (required) - The group/project ID + # + # Example Request: + # POST /groups/:id/access_requests + # POST /projects/:id/access_requests + post ":id/access_requests" do + source = find_source(source_type, params[:id]) + access_requester = source.request_access(current_user) + + if access_requester.persisted? + present access_requester.user, with: Entities::AccessRequester, access_requester: access_requester + else + render_validation_error!(access_requester) + end + end + + # Approve a group/project access request + # + # Parameters: + # id (required) - The group/project ID + # user_id (required) - The user ID of the access requester + # access_level (optional) - Access level + # + # Example Request: + # PUT /groups/:id/access_requests/:user_id/approve + # PUT /projects/:id/access_requests/:user_id/approve + put ':id/access_requests/:user_id/approve' do + required_attributes! [:user_id] + source = find_source(source_type, params[:id]) + authorize_admin_source!(source_type, source) + + member = source.requesters.find_by!(user_id: params[:user_id]) + if params[:access_level] + member.update(access_level: params[:access_level]) + end + member.accept_request + + status :created + present member.user, with: Entities::Member, member: member + end + + # Deny a group/project access request + # + # Parameters: + # id (required) - The group/project ID + # user_id (required) - The user ID of the access requester + # + # Example Request: + # DELETE /groups/:id/access_requests/:user_id + # DELETE /projects/:id/access_requests/:user_id + delete ":id/access_requests/:user_id" do + required_attributes! [:user_id] + source = find_source(source_type, params[:id]) + + access_requester = source.requesters.find_by!(user_id: params[:user_id]) + + ::Members::DestroyService.new(access_requester, current_user).execute + end + end + end + end +end diff --git a/lib/api/api.rb b/lib/api/api.rb index 6cd4a853dbe..d43af3f24e9 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -3,6 +3,10 @@ module API include APIGuard version 'v3', using: :path + rescue_from Gitlab::Access::AccessDeniedError do + rack_response({ 'message' => '403 Forbidden' }.to_json, 403) + end + rescue_from ActiveRecord::RecordNotFound do rack_response({ 'message' => '404 Not found' }.to_json, 404) end @@ -32,6 +36,7 @@ module API # Ensure the namespace is right, otherwise we might load Grape::API::Helpers helpers ::API::Helpers + mount ::API::AccessRequests mount ::API::AwardEmoji mount ::API::Branches mount ::API::Builds @@ -40,19 +45,18 @@ module API mount ::API::DeployKeys mount ::API::Environments mount ::API::Files - mount ::API::GroupMembers mount ::API::Groups mount ::API::Internal mount ::API::Issues mount ::API::Keys mount ::API::Labels mount ::API::LicenseTemplates + mount ::API::Members mount ::API::MergeRequests mount ::API::Milestones mount ::API::Namespaces mount ::API::Notes mount ::API::ProjectHooks - mount ::API::ProjectMembers mount ::API::ProjectSnippets mount ::API::Projects mount ::API::Repositories diff --git a/lib/api/entities.rb b/lib/api/entities.rb index e5b00dc45a5..c5ff4557b4a 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -91,9 +91,17 @@ module API end end - class ProjectMember < UserBasic + class Member < UserBasic expose :access_level do |user, options| - options[:project].project_members.find_by(user_id: user.id).access_level + member = options[:member] || options[:source].members.find_by(user_id: user.id) + member.access_level + end + end + + class AccessRequester < UserBasic + expose :requested_at do |user, options| + access_requester = options[:access_requester] || options[:source].requesters.find_by(user_id: user.id) + access_requester.requested_at end end @@ -108,12 +116,6 @@ module API expose :shared_projects, using: Entities::Project end - class GroupMember < UserBasic - expose :access_level do |user, options| - options[:group].group_members.find_by(user_id: user.id).access_level - end - end - class RepoBranch < Grape::Entity expose :name @@ -325,7 +327,7 @@ module API expose :id, :path, :kind end - class Member < Grape::Entity + class MemberAccess < Grape::Entity expose :access_level expose :notification_level do |member, options| if member.notification_setting @@ -334,10 +336,10 @@ module API end end - class ProjectAccess < Member + class ProjectAccess < MemberAccess end - class GroupAccess < Member + class GroupAccess < MemberAccess end class ProjectService < Grape::Entity diff --git a/lib/api/group_members.rb b/lib/api/group_members.rb deleted file mode 100644 index dbe5bb08d3f..00000000000 --- a/lib/api/group_members.rb +++ /dev/null @@ -1,87 +0,0 @@ -module API - class GroupMembers < Grape::API - before { authenticate! } - - resource :groups do - # Get a list of group members viewable by the authenticated user. - # - # Example Request: - # GET /groups/:id/members - get ":id/members" do - group = find_group(params[:id]) - users = group.users - present users, with: Entities::GroupMember, group: group - end - - # Add a user to the list of group members - # - # Parameters: - # id (required) - group id - # user_id (required) - the users id - # access_level (required) - Project access level - # Example Request: - # POST /groups/:id/members - post ":id/members" do - group = find_group(params[:id]) - authorize! :admin_group, group - required_attributes! [:user_id, :access_level] - - unless validate_access_level?(params[:access_level]) - render_api_error!("Wrong access level", 422) - end - - if group.group_members.find_by(user_id: params[:user_id]) - render_api_error!("Already exists", 409) - end - - group.add_users([params[:user_id]], params[:access_level], current_user) - member = group.group_members.find_by(user_id: params[:user_id]) - present member.user, with: Entities::GroupMember, group: group - end - - # Update group member - # - # Parameters: - # id (required) - The ID of a group - # user_id (required) - The ID of a group member - # access_level (required) - Project access level - # Example Request: - # PUT /groups/:id/members/:user_id - put ':id/members/:user_id' do - group = find_group(params[:id]) - authorize! :admin_group, group - required_attributes! [:access_level] - - group_member = group.group_members.find_by(user_id: params[:user_id]) - not_found!('User can not be found') if group_member.nil? - - if group_member.update_attributes(access_level: params[:access_level]) - @member = group_member.user - present @member, with: Entities::GroupMember, group: group - else - handle_member_errors group_member.errors - end - end - - # Remove member. - # - # Parameters: - # id (required) - group id - # user_id (required) - the users id - # - # Example Request: - # DELETE /groups/:id/members/:user_id - delete ":id/members/:user_id" do - group = find_group(params[:id]) - authorize! :admin_group, group - member = group.group_members.find_by(user_id: params[:user_id]) - - if member.nil? - render_api_error!("404 Not Found - user_id:#{params[:user_id]} not a member of group #{group.name}", 404) - else - member.destroy - end - end - end - end -end diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 130509cdad6..f06c262fd4c 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -28,7 +28,7 @@ module API # If the sudo is the current user do nothing if identifier && !(@current_user.id == identifier || @current_user.username == identifier) - render_api_error!('403 Forbidden: Must be admin to use sudo', 403) unless @current_user.is_admin? + forbidden!('Must be admin to use sudo') unless @current_user.is_admin? @current_user = User.by_username_or_id(identifier) not_found!("No user id or username for: #{identifier}") if @current_user.nil? end @@ -47,18 +47,18 @@ module API end end + # Deprecated def user_project @project ||= find_project(params[:id]) - @project || not_found!("Project") end def find_project(id) project = Project.find_with_namespace(id) || Project.find_by(id: id) - if project && can?(current_user, :read_project, project) + if can?(current_user, :read_project, project) project else - nil + not_found!('Project') end end @@ -89,11 +89,7 @@ module API end def find_group(id) - begin - group = Group.find(id) - rescue ActiveRecord::RecordNotFound - group = Group.find_by!(path: id) - end + group = Group.find_by(path: id) || Group.find_by(id: id) if can?(current_user, :read_group, group) group @@ -135,7 +131,7 @@ module API end def authorize!(action, subject) - forbidden! unless abilities.allowed?(current_user, action, subject) + forbidden! unless can?(current_user, action, subject) end def authorize_push_project @@ -197,10 +193,6 @@ module API errors end - def validate_access_level?(level) - Gitlab::Access.options_with_owner.values.include? level.to_i - end - # Checks the occurrences of datetime attributes, each attribute if present in the params hash must be in ISO 8601 # format (YYYY-MM-DDTHH:MM:SSZ) or a Bad Request error is invoked. # @@ -411,11 +403,6 @@ module API File.read(Gitlab.config.gitlab_shell.secret_file).chomp end - def handle_member_errors(errors) - error!(errors[:access_level], 422) if errors[:access_level].any? - not_found!(errors) - end - def send_git_blob(repository, blob) env['api.format'] = :txt content_type 'text/plain' diff --git a/lib/api/helpers/members_helpers.rb b/lib/api/helpers/members_helpers.rb new file mode 100644 index 00000000000..90114f6f667 --- /dev/null +++ b/lib/api/helpers/members_helpers.rb @@ -0,0 +1,13 @@ +module API + module Helpers + module MembersHelpers + def find_source(source_type, id) + public_send("find_#{source_type}", id) + end + + def authorize_admin_source!(source_type, source) + authorize! :"admin_#{source_type}", source + end + end + end +end diff --git a/lib/api/members.rb b/lib/api/members.rb new file mode 100644 index 00000000000..56f8b1ca391 --- /dev/null +++ b/lib/api/members.rb @@ -0,0 +1,124 @@ +module API + class Members < Grape::API + before { authenticate! } + + helpers ::API::Helpers::MembersHelpers + + %w[group project].each do |source_type| + resource source_type.pluralize do + # Get a list of group/project members viewable by the authenticated user. + # + # Parameters: + # id (required) - The group/project ID + # query - Query string + # + # Example Request: + # GET /groups/:id/members + # GET /projects/:id/members + get ":id/members" do + source = find_source(source_type, params[:id]) + + members = source.members + members = members.joins(:user).merge(User.search(params[:query])) if params[:query] + users = Kaminari.paginate_array(members.map(&:user)) + + present paginate(users), with: Entities::Member, source: source + end + + # Get a group/project member + # + # Parameters: + # id (required) - The group/project ID + # user_id (required) - The user ID of the member + # + # Example Request: + # GET /groups/:id/members/:user_id + # GET /projects/:id/members/:user_id + get ":id/members/:user_id" do + source = find_source(source_type, params[:id]) + + members = source.members + member = members.find_by!(user_id: params[:user_id]) + + present member.user, with: Entities::Member, member: member + end + + # Add a new group/project member + # + # Parameters: + # id (required) - The group/project ID + # user_id (required) - The user ID of the new member + # access_level (required) - A valid access level + # + # Example Request: + # POST /groups/:id/members + # POST /projects/:id/members + post ":id/members" do + source = find_source(source_type, params[:id]) + authorize_admin_source!(source_type, source) + required_attributes! [:user_id, :access_level] + + access_requester = source.requesters.find_by(user_id: params[:user_id]) + if access_requester + # We pass current_user = access_requester so that the requester doesn't + # receive a "access denied" email + ::Members::DestroyService.new(access_requester, access_requester.user).execute + end + + conflict!('Member already exists') if source.members.exists?(user_id: params[:user_id]) + + source.add_user(params[:user_id], params[:access_level], current_user) + member = source.members.find_by(user_id: params[:user_id]) + if member + present member.user, with: Entities::Member, member: member + else + render_api_error!('400 Bad Request', 400) + end + end + + # Update a group/project member + # + # Parameters: + # id (required) - The group/project ID + # user_id (required) - The user ID of the member + # access_level (required) - A valid access level + # + # Example Request: + # PUT /groups/:id/members/:user_id + # PUT /projects/:id/members/:user_id + put ":id/members/:user_id" do + source = find_source(source_type, params[:id]) + authorize_admin_source!(source_type, source) + required_attributes! [:user_id, :access_level] + + member = source.members.find_by!(user_id: params[:user_id]) + + if member.update_attributes(access_level: params[:access_level]) + present member.user, with: Entities::Member, member: member + else + render_validation_error!(member) + end + end + + # Remove a group/project member + # + # Parameters: + # id (required) - The group/project ID + # user_id (required) - The user ID of the member + # + # Example Request: + # DELETE /groups/:id/members/:user_id + # DELETE /projects/:id/members/:user_id + delete ":id/members/:user_id" do + source = find_source(source_type, params[:id]) + required_attributes! [:user_id] + + member = source.members.find_by!(user_id: params[:user_id]) + + ::Members::DestroyService.new(member, current_user).execute + status :no_content + end + end + end + end +end diff --git a/lib/api/project_members.rb b/lib/api/project_members.rb deleted file mode 100644 index 6a0b3e7d134..00000000000 --- a/lib/api/project_members.rb +++ /dev/null @@ -1,110 +0,0 @@ -module API - # Projects members API - class ProjectMembers < Grape::API - before { authenticate! } - - resource :projects do - # Get a project team members - # - # Parameters: - # id (required) - The ID of a project - # query - Query string - # Example Request: - # GET /projects/:id/members - get ":id/members" do - if params[:query].present? - @members = paginate user_project.users.where("username LIKE ?", "%#{params[:query]}%") - else - @members = paginate user_project.users - end - present @members, with: Entities::ProjectMember, project: user_project - end - - # Get a project team members - # - # Parameters: - # id (required) - The ID of a project - # user_id (required) - The ID of a user - # Example Request: - # GET /projects/:id/members/:user_id - get ":id/members/:user_id" do - @member = user_project.users.find params[:user_id] - present @member, with: Entities::ProjectMember, project: user_project - end - - # Add a new project team member - # - # Parameters: - # id (required) - The ID of a project - # user_id (required) - The ID of a user - # access_level (required) - Project access level - # Example Request: - # POST /projects/:id/members - post ":id/members" do - authorize! :admin_project, user_project - required_attributes! [:user_id, :access_level] - - # either the user is already a team member or a new one - project_member = user_project.project_member(params[:user_id]) - if project_member.nil? - project_member = user_project.project_members.new( - user_id: params[:user_id], - access_level: params[:access_level] - ) - end - - if project_member.save - @member = project_member.user - present @member, with: Entities::ProjectMember, project: user_project - else - handle_member_errors project_member.errors - end - end - - # Update project team member - # - # Parameters: - # id (required) - The ID of a project - # user_id (required) - The ID of a team member - # access_level (required) - Project access level - # Example Request: - # PUT /projects/:id/members/:user_id - put ":id/members/:user_id" do - authorize! :admin_project, user_project - required_attributes! [:access_level] - - project_member = user_project.project_members.find_by(user_id: params[:user_id]) - not_found!("User can not be found") if project_member.nil? - - if project_member.update_attributes(access_level: params[:access_level]) - @member = project_member.user - present @member, with: Entities::ProjectMember, project: user_project - else - handle_member_errors project_member.errors - end - end - - # Remove a team member from project - # - # Parameters: - # id (required) - The ID of a project - # user_id (required) - The ID of a team member - # Example Request: - # DELETE /projects/:id/members/:user_id - delete ":id/members/:user_id" do - project_member = user_project.project_members.find_by(user_id: params[:user_id]) - - unless current_user.can?(:admin_project, user_project) || - current_user.can?(:destroy_project_member, project_member) - forbidden! - end - - if project_member.nil? - { message: "Access revoked", id: params[:user_id].to_i } - else - project_member.destroy - end - end - end - end -end -- cgit v1.2.1 From 7c1b33b48f8c45b726a513b6809a85919d41c23c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Tue, 9 Aug 2016 12:14:11 +0200 Subject: Restore back-compatibility for current members API endpoints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- lib/api/helpers.rb | 1 - lib/api/members.rb | 45 ++++++++++++++++++++++++++++++++++++++------- 2 files changed, 38 insertions(+), 8 deletions(-) (limited to 'lib') diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index f06c262fd4c..d0469d6602d 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -47,7 +47,6 @@ module API end end - # Deprecated def user_project @project ||= find_project(params[:id]) end diff --git a/lib/api/members.rb b/lib/api/members.rb index 56f8b1ca391..ccb7618360d 100644 --- a/lib/api/members.rb +++ b/lib/api/members.rb @@ -65,14 +65,29 @@ module API ::Members::DestroyService.new(access_requester, access_requester.user).execute end - conflict!('Member already exists') if source.members.exists?(user_id: params[:user_id]) - - source.add_user(params[:user_id], params[:access_level], current_user) member = source.members.find_by(user_id: params[:user_id]) + + # This is to ensure back-compatibility but 409 behavior should be used + # for both project and group members in 9.0! + conflict!('Member already exists') if source_type == 'group' && member + + unless member + source.add_user(params[:user_id], params[:access_level], current_user) + member = source.members.find_by(user_id: params[:user_id]) + end + if member present member.user, with: Entities::Member, member: member else - render_api_error!('400 Bad Request', 400) + # Since `source.add_user` doesn't return a member object, we have to + # build a new one and populate its errors in order to render them. + member = source.members.build(attributes_for_keys([:user_id, :access_level])) + member.valid? # populate the errors + + # This is to ensure back-compatibility but 400 behavior should be used + # for all validation errors in 9.0! + render_api_error!('Access level is not known', 422) if member.errors.key?(:access_level) + render_validation_error!(member) end end @@ -96,6 +111,9 @@ module API if member.update_attributes(access_level: params[:access_level]) present member.user, with: Entities::Member, member: member else + # This is to ensure back-compatibility but 400 behavior should be used + # for all validation errors in 9.0! + render_api_error!('Access level is not known', 422) if member.errors.key?(:access_level) render_validation_error!(member) end end @@ -113,10 +131,23 @@ module API source = find_source(source_type, params[:id]) required_attributes! [:user_id] - member = source.members.find_by!(user_id: params[:user_id]) + # This is to ensure back-compatibility but find_by! should be used + # in that casse in 9.0! + member = source.members.find_by(user_id: params[:user_id]) + + # This is to ensure back-compatibility but this should be removed in + # favor of find_by! in 9.0! + not_found!("Member: user_id:#{params[:user_id]}") if source_type == 'group' && member.nil? + + # This is to ensure back-compatibility but 204 behavior should be used + # for all DELETE endpoints in 9.0! + if member.nil? + { message: "Access revoked", id: params[:user_id].to_i } + else + ::Members::DestroyService.new(member, current_user).execute - ::Members::DestroyService.new(member, current_user).execute - status :no_content + present member.user, with: Entities::Member, member: member + end end end end -- cgit v1.2.1 From 5010be7766d08a9adee51c7d16ba71c19ff7dede Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Tue, 9 Aug 2016 12:20:09 +0200 Subject: Improve the performance of the GET /:sources/:id/{access_requests,members} API endpoints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The performance was greatly improved by removing two N+1 queries issues for each endpoint. For comparison: 1. `GET /projects/:id/members` With two N+1 queries issues (one was already fxed in the following snippet): ``` ProjectMember Load (0.2ms) SELECT "members".* FROM "members" WHERE "members"."source_type" = $1 AND "members"."type" IN ('ProjectMember') AND "members"."source_id" = $2 AND "members"."source_type" = $3 AND "members"."type" IN ('ProjectMember') AND "members"."requested_at" IS NULL ORDER BY "members"."id" DESC [["source_type", "Project"], ["source_id", 1], ["source_type", "Project"]] User Load (0.5ms) SELECT "users".* FROM "users" WHERE "users"."id" IN (5, 22, 16, 13) ORDER BY "users"."id" DESC ActiveRecord::SchemaMigration Load (0.2ms) SELECT "schema_migrations".* FROM "schema_migrations" ProjectMember Load (0.3ms) SELECT "members".* FROM "members" WHERE "members"."source_type" = $1 AND "members"."type" IN ('ProjectMember') AND "members"."source_id" = $2 AND "members"."source_type" = $3 AND "members"."type" IN ('ProjectMember') AND "members"."requested_at" IS NULL AND "members"."user_id" = $4 ORDER BY "members"."id" DESC LIMIT 1 [["source_type", "Project"], ["source_id", 1], ["source_type", "Project"], ["user_id", 5]] ProjectMember Load (0.3ms) SELECT "members".* FROM "members" WHERE "members"."source_type" = $1 AND "members"."type" IN ('ProjectMember') AND "members"."source_id" = $2 AND "members"."source_type" = $3 AND "members"."type" IN ('ProjectMember') AND "members"."requested_at" IS NULL AND "members"."user_id" = $4 ORDER BY "members"."id" DESC LIMIT 1 [["source_type", "Project"], ["source_id", 1], ["source_type", "Project"], ["user_id", 22]] ProjectMember Load (0.3ms) SELECT "members".* FROM "members" WHERE "members"."source_type" = $1 AND "members"."type" IN ('ProjectMember') AND "members"."source_id" = $2 AND "members"."source_type" = $3 AND "members"."type" IN ('ProjectMember') AND "members"."requested_at" IS NULL AND "members"."user_id" = $4 ORDER BY "members"."id" DESC LIMIT 1 [["source_type", "Project"], ["source_id", 1], ["source_type", "Project"], ["user_id", 16]] ProjectMember Load (0.3ms) SELECT "members".* FROM "members" WHERE "members"."source_type" = $1 AND "members"."type" IN ('ProjectMember') AND "members"."source_id" = $2 AND "members"."source_type" = $3 AND "members"."type" IN ('ProjectMember') AND "members"."requested_at" IS NULL AND "members"."user_id" = $4 ORDER BY "members"."id" DESC LIMIT 1 [["source_type", "Project"], ["source_id", 1], ["source_type", "Project"], ["user_id", 13]] ``` Without the N+1 queries issues: ``` ProjectMember Load (0.3ms) SELECT "members".* FROM "members" WHERE "members"."source_type" = $1 AND "members"."type" IN ('ProjectMember') AND "members"."source_id" = $2 AND "members"."source_type" = $3 AND "members"."type" IN ('ProjectMember') AND "members"."requested_at" IS NULL ORDER BY "members"."id" DESC LIMIT 20 OFFSET 0 [["source_type", "Project"], ["source_id", 1], ["source_type", "Project"]] User Load (0.5ms) SELECT "users".* FROM "users" WHERE "users"."id" IN (5, 22, 16, 13) ORDER BY "users"."id" DESC ``` 2. `GET /projects/:id/access_requests` With two N+1 queries issues: ``` ProjectMember Load (0.3ms) SELECT "members".* FROM "members" WHERE "members"."source_type" = $1 AND "members"."type" IN ('ProjectMember') AND "members"."source_id" = $2 AND "members"."source_type" = $3 AND "members"."type" IN ('ProjectMember') AND ("members"."requested_at" IS NOT NULL) ORDER BY "members"."id" DESC [["source_type", "Project"], ["source_id", 8], ["source_type", "Project"]] User Load (0.1ms) SELECT "users".* FROM "users" WHERE "users"."id" = $1 ORDER BY "users"."id" DESC LIMIT 1 [["id", 24]] User Load (0.1ms) SELECT "users".* FROM "users" WHERE "users"."id" = $1 ORDER BY "users"."id" DESC LIMIT 1 [["id", 23]] ActiveRecord::SchemaMigration Load (0.2ms) SELECT "schema_migrations".* FROM "schema_migrations" ProjectMember Load (0.1ms) SELECT "members".* FROM "members" WHERE "members"."source_type" = $1 AND "members"."type" IN ('ProjectMember') AND "members"."source_id" = $2 AND "members"."source_type" = $3 AND "members"."type" IN ('ProjectMember') AND ("members"."requested_at" IS NOT NULL) AND "members"."user_id" = $4 ORDER BY "members"."id" DESC LIMIT 1 [["source_type", "Project"], ["source_id", 8], ["source_type", "Project"], ["user_id", 24]] ProjectMember Load (0.2ms) SELECT "members".* FROM "members" WHERE "members"."source_type" = $1 AND "members"."type" IN ('ProjectMember') AND "members"."source_id" = $2 AND "members"."source_type" = $3 AND "members"."type" IN ('ProjectMember') AND ("members"."requested_at" IS NOT NULL) AND "members"."user_id" = $4 ORDER BY "members"."id" DESC LIMIT 1 [["source_type", "Project"], ["source_id", 8], ["source_type", "Project"], ["user_id", 23]] ``` Without the N+1 queries issues: ``` ProjectMember Load (0.3ms) SELECT "members".* FROM "members" WHERE "members"."source_type" = $1 AND "members"."type" IN ('ProjectMember') AND "members"."source_id" = $2 AND "members"."source_type" = $3 AND "members"."type" IN ('ProjectMember') AND ("members"."requested_at" IS NOT NULL) ORDER BY "members"."id" DESC LIMIT 20 OFFSET 0 [["source_type", "Project"], ["source_id", 8], ["source_type", "Project"]] User Load (0.5ms) SELECT "users".* FROM "users" WHERE "users"."id" IN (24, 23) ORDER BY "users"."id" DESC ``` Signed-off-by: Rémy Coutable --- lib/api/access_requests.rb | 5 ++--- lib/api/entities.rb | 4 ++-- lib/api/members.rb | 6 +++--- 3 files changed, 7 insertions(+), 8 deletions(-) (limited to 'lib') diff --git a/lib/api/access_requests.rb b/lib/api/access_requests.rb index 9c41d8aaa3e..d02b469dac8 100644 --- a/lib/api/access_requests.rb +++ b/lib/api/access_requests.rb @@ -18,10 +18,9 @@ module API source = find_source(source_type, params[:id]) authorize_admin_source!(source_type, source) - access_requesters = source.requesters - users = Kaminari.paginate_array(access_requesters.map(&:user)) + access_requesters = paginate(source.requesters.includes(:user)) - present paginate(users), with: Entities::AccessRequester, source: source + present access_requesters.map(&:user), with: Entities::AccessRequester, access_requesters: access_requesters end # Request access to the group/project diff --git a/lib/api/entities.rb b/lib/api/entities.rb index c5ff4557b4a..ae74d14a4bb 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -93,14 +93,14 @@ module API class Member < UserBasic expose :access_level do |user, options| - member = options[:member] || options[:source].members.find_by(user_id: user.id) + member = options[:member] || options[:members].find { |m| m.user_id == user.id } member.access_level end end class AccessRequester < UserBasic expose :requested_at do |user, options| - access_requester = options[:access_requester] || options[:source].requesters.find_by(user_id: user.id) + access_requester = options[:access_requester] || options[:access_requesters].find { |m| m.user_id == user.id } access_requester.requested_at end end diff --git a/lib/api/members.rb b/lib/api/members.rb index ccb7618360d..2fae83f60b2 100644 --- a/lib/api/members.rb +++ b/lib/api/members.rb @@ -18,11 +18,11 @@ module API get ":id/members" do source = find_source(source_type, params[:id]) - members = source.members + members = source.members.includes(:user) members = members.joins(:user).merge(User.search(params[:query])) if params[:query] - users = Kaminari.paginate_array(members.map(&:user)) + members = paginate(members) - present paginate(users), with: Entities::Member, source: source + present members.map(&:user), with: Entities::Member, members: members end # Get a group/project member -- cgit v1.2.1 From 96ebc8c4f7223091d97c38c442d68c8058d26261 Mon Sep 17 00:00:00 2001 From: bogdanvlviv Date: Wed, 10 Aug 2016 00:23:25 +0300 Subject: Use `File::exist?` instead of `File::exists?` Since version ruby-2.2.0, method `File::exists?` is deprecated. --- lib/backup/files.rb | 2 +- lib/backup/manager.rb | 2 +- lib/backup/repository.rb | 8 ++++---- lib/tasks/gitlab/check.rake | 20 ++++++++++---------- lib/tasks/gitlab/shell.rake | 2 +- lib/tasks/spinach.rake | 2 +- 6 files changed, 18 insertions(+), 18 deletions(-) (limited to 'lib') diff --git a/lib/backup/files.rb b/lib/backup/files.rb index 654b4d1c896..cedbb289f6a 100644 --- a/lib/backup/files.rb +++ b/lib/backup/files.rb @@ -27,7 +27,7 @@ module Backup def backup_existing_files_dir timestamped_files_path = File.join(files_parent_dir, "#{name}.#{Time.now.to_i}") - if File.exists?(app_files_dir) + if File.exist?(app_files_dir) FileUtils.mv(app_files_dir, File.expand_path(timestamped_files_path)) end end diff --git a/lib/backup/manager.rb b/lib/backup/manager.rb index 2ff3e3bdfb0..0dfffaf0bc6 100644 --- a/lib/backup/manager.rb +++ b/lib/backup/manager.rb @@ -114,7 +114,7 @@ module Backup tar_file = ENV["BACKUP"].nil? ? File.join("#{file_list.first}_gitlab_backup.tar") : File.join(ENV["BACKUP"] + "_gitlab_backup.tar") - unless File.exists?(tar_file) + unless File.exist?(tar_file) puts "The specified backup doesn't exist!" exit 1 end diff --git a/lib/backup/repository.rb b/lib/backup/repository.rb index 1f5917b8127..f117fc3d37d 100644 --- a/lib/backup/repository.rb +++ b/lib/backup/repository.rb @@ -28,7 +28,7 @@ module Backup wiki = ProjectWiki.new(project) - if File.exists?(path_to_repo(wiki)) + if File.exist?(path_to_repo(wiki)) $progress.print " * #{wiki.path_with_namespace} ... " if wiki.repository.empty? $progress.puts " [SKIPPED]".color(:cyan) @@ -49,7 +49,7 @@ module Backup def restore Gitlab.config.repositories.storages.each do |name, path| - next unless File.exists?(path) + next unless File.exist?(path) # Move repos dir to 'repositories.old' dir bk_repos_path = File.join(path, '..', 'repositories.old.' + Time.now.to_i.to_s) @@ -63,7 +63,7 @@ module Backup project.ensure_dir_exist - if File.exists?(path_to_bundle(project)) + if File.exist?(path_to_bundle(project)) FileUtils.mkdir_p(path_to_repo(project)) cmd = %W(tar -xf #{path_to_bundle(project)} -C #{path_to_repo(project)}) else @@ -80,7 +80,7 @@ module Backup wiki = ProjectWiki.new(project) - if File.exists?(path_to_bundle(wiki)) + if File.exist?(path_to_bundle(wiki)) $progress.print " * #{wiki.path_with_namespace} ... " # If a wiki bundle exists, first remove the empty repo diff --git a/lib/tasks/gitlab/check.rake b/lib/tasks/gitlab/check.rake index 0894994200f..5f4a6bbfa35 100644 --- a/lib/tasks/gitlab/check.rake +++ b/lib/tasks/gitlab/check.rake @@ -64,7 +64,7 @@ namespace :gitlab do for_more_information( see_installation_guide_section "GitLab" ) - end + end end end @@ -73,7 +73,7 @@ namespace :gitlab do database_config_file = Rails.root.join("config", "database.yml") - if File.exists?(database_config_file) + if File.exist?(database_config_file) puts "yes".color(:green) else puts "no".color(:red) @@ -94,7 +94,7 @@ namespace :gitlab do gitlab_config_file = Rails.root.join("config", "gitlab.yml") - if File.exists?(gitlab_config_file) + if File.exist?(gitlab_config_file) puts "yes".color(:green) else puts "no".color(:red) @@ -113,7 +113,7 @@ namespace :gitlab do print "GitLab config outdated? ... " gitlab_config_file = Rails.root.join("config", "gitlab.yml") - unless File.exists?(gitlab_config_file) + unless File.exist?(gitlab_config_file) puts "can't check because of previous errors".color(:magenta) end @@ -144,7 +144,7 @@ namespace :gitlab do script_path = "/etc/init.d/gitlab" - if File.exists?(script_path) + if File.exist?(script_path) puts "yes".color(:green) else puts "no".color(:red) @@ -169,7 +169,7 @@ namespace :gitlab do recipe_path = Rails.root.join("lib/support/init.d/", "gitlab") script_path = "/etc/init.d/gitlab" - unless File.exists?(script_path) + unless File.exist?(script_path) puts "can't check because of previous errors".color(:magenta) return end @@ -361,7 +361,7 @@ namespace :gitlab do Gitlab.config.repositories.storages.each do |name, repo_base_path| print "#{name}... " - if File.exists?(repo_base_path) + if File.exist?(repo_base_path) puts "yes".color(:green) else puts "no".color(:red) @@ -385,7 +385,7 @@ namespace :gitlab do Gitlab.config.repositories.storages.each do |name, repo_base_path| print "#{name}... " - unless File.exists?(repo_base_path) + unless File.exist?(repo_base_path) puts "can't check because of previous errors".color(:magenta) return end @@ -408,7 +408,7 @@ namespace :gitlab do Gitlab.config.repositories.storages.each do |name, repo_base_path| print "#{name}... " - unless File.exists?(repo_base_path) + unless File.exist?(repo_base_path) puts "can't check because of previous errors".color(:magenta) return end @@ -438,7 +438,7 @@ namespace :gitlab do Gitlab.config.repositories.storages.each do |name, repo_base_path| print "#{name}... " - unless File.exists?(repo_base_path) + unless File.exist?(repo_base_path) puts "can't check because of previous errors".color(:magenta) return end diff --git a/lib/tasks/gitlab/shell.rake b/lib/tasks/gitlab/shell.rake index ba93945bd03..bb7eb852f1b 100644 --- a/lib/tasks/gitlab/shell.rake +++ b/lib/tasks/gitlab/shell.rake @@ -90,7 +90,7 @@ namespace :gitlab do task build_missing_projects: :environment do Project.find_each(batch_size: 1000) do |project| path_to_repo = project.repository.path_to_repo - if File.exists?(path_to_repo) + if File.exist?(path_to_repo) print '-' else if Gitlab::Shell.new.add_repository(project.repository_storage_path, diff --git a/lib/tasks/spinach.rake b/lib/tasks/spinach.rake index c0f860a82d2..8dbfa7751dc 100644 --- a/lib/tasks/spinach.rake +++ b/lib/tasks/spinach.rake @@ -46,7 +46,7 @@ def run_spinach_tests(tags) success = run_spinach_command(%W(--tags #{tags})) 3.times do |_| break if success - break unless File.exists?('tmp/spinach-rerun.txt') + break unless File.exist?('tmp/spinach-rerun.txt') tests = File.foreach('tmp/spinach-rerun.txt').map(&:chomp) puts '' -- cgit v1.2.1 From 68cea38e94886e69099a3faa0d1e2fbde2e71899 Mon Sep 17 00:00:00 2001 From: Elliot Date: Thu, 11 Aug 2016 13:15:46 +0200 Subject: Fix front-end for branches that happen to contain urlencoding escape characters (e.g. %) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- lib/extracts_path.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/extracts_path.rb b/lib/extracts_path.rb index 51e46da82cc..84688f6646e 100644 --- a/lib/extracts_path.rb +++ b/lib/extracts_path.rb @@ -94,7 +94,7 @@ module ExtractsPath @options = params.select {|key, value| allowed_options.include?(key) && !value.blank? } @options = HashWithIndifferentAccess.new(@options) - @id = Addressable::URI.unescape(get_id) + @id = Addressable::URI.normalize_component(get_id) @ref, @path = extract_ref(@id) @repo = @project.repository if @options[:extended_sha1].blank? -- cgit v1.2.1 From 39203f1adfc6fee3eca50f0cab99ffc597865200 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 11 Aug 2016 15:22:35 +0200 Subject: Pre-create all builds for Pipeline when a trigger is received This change simplifies a Pipeline processing by introducing a special new status: created. This status is used for all builds that are created for a pipeline. We are then processing next stages and queueing some of the builds (created -> pending) or skipping them (created -> skipped). This makes it possible to simplify and solve a few ordering problems with how previously builds were scheduled. This also allows us to visualise a full pipeline (with created builds). This also removes an after_touch used for updating a pipeline state parameters. Right now in various places we explicitly call a reload_status! on pipeline to force it to be updated and saved. --- lib/ci/gitlab_ci_yaml_processor.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb index a2e8bd22a52..47efd5bd9f2 100644 --- a/lib/ci/gitlab_ci_yaml_processor.rb +++ b/lib/ci/gitlab_ci_yaml_processor.rb @@ -62,7 +62,7 @@ module Ci # - before script should be a concatenated command commands: [job[:before_script] || @before_script, job[:script]].flatten.compact.join("\n"), tag_list: job[:tags] || [], - name: job[:name], + name: job[:name].to_s, allow_failure: job[:allow_failure] || false, when: job[:when] || 'on_success', environment: job[:environment], -- cgit v1.2.1 From 6109daf480327581b6e2dcdfffe90464be6c7796 Mon Sep 17 00:00:00 2001 From: Scott Le Date: Thu, 28 Jul 2016 11:04:57 +0700 Subject: api for generating new merge request DRY code + fix rubocop Add more test cases Append to changelog DRY changes list find_url service for merge_requests use GET for getting merge request links remove files rename to get_url_service reduce loop add test case for cross project refactor tiny thing update changelog --- lib/api/internal.rb | 4 ++++ lib/gitlab/changes_list.rb | 25 +++++++++++++++++++++++++ lib/gitlab/checks/change_access.rb | 24 +++--------------------- lib/gitlab/git.rb | 18 ++++++++++++++++++ lib/gitlab/git_access.rb | 4 ++-- 5 files changed, 52 insertions(+), 23 deletions(-) create mode 100644 lib/gitlab/changes_list.rb (limited to 'lib') diff --git a/lib/api/internal.rb b/lib/api/internal.rb index 959b700de78..d8e9ac406c4 100644 --- a/lib/api/internal.rb +++ b/lib/api/internal.rb @@ -74,6 +74,10 @@ module API response end + get "/merge_request_urls" do + ::MergeRequests::GetUrlsService.new(project).execute(params[:changes]) + end + # # Discover user by ssh key # diff --git a/lib/gitlab/changes_list.rb b/lib/gitlab/changes_list.rb new file mode 100644 index 00000000000..95308aca95f --- /dev/null +++ b/lib/gitlab/changes_list.rb @@ -0,0 +1,25 @@ +module Gitlab + class ChangesList + include Enumerable + + attr_reader :raw_changes + + def initialize(changes) + @raw_changes = changes.kind_of?(String) ? changes.lines : changes + end + + def each(&block) + changes.each(&block) + end + + def changes + @changes ||= begin + @raw_changes.map do |change| + next if change.blank? + oldrev, newrev, ref = change.strip.split(' ') + { oldrev: oldrev, newrev: newrev, ref: ref } + end.compact + end + end + end +end diff --git a/lib/gitlab/checks/change_access.rb b/lib/gitlab/checks/change_access.rb index 5551fac4b8b..52f117e963b 100644 --- a/lib/gitlab/checks/change_access.rb +++ b/lib/gitlab/checks/change_access.rb @@ -4,8 +4,8 @@ module Gitlab attr_reader :user_access, :project def initialize(change, user_access:, project:) - @oldrev, @newrev, @ref = change.split(' ') - @branch_name = branch_name(@ref) + @oldrev, @newrev, @ref = change.values_at(:oldrev, :newrev, :ref) + @branch_name = Gitlab::Git.branch_name(@ref) @user_access = user_access @project = project end @@ -47,7 +47,7 @@ module Gitlab end def tag_checks - tag_ref = tag_name(@ref) + tag_ref = Gitlab::Git.tag_name(@ref) if tag_ref && protected_tag?(tag_ref) && user_access.cannot_do_action?(:admin_project) "You are not allowed to change existing tags on this project." @@ -73,24 +73,6 @@ module Gitlab def matching_merge_request? Checks::MatchingMergeRequest.new(@newrev, @branch_name, @project).match? end - - def branch_name(ref) - ref = @ref.to_s - if Gitlab::Git.branch_ref?(ref) - Gitlab::Git.ref_name(ref) - else - nil - end - end - - def tag_name(ref) - ref = @ref.to_s - if Gitlab::Git.tag_ref?(ref) - Gitlab::Git.ref_name(ref) - else - nil - end - end end end end diff --git a/lib/gitlab/git.rb b/lib/gitlab/git.rb index 191bea86ac3..7584efe4fa8 100644 --- a/lib/gitlab/git.rb +++ b/lib/gitlab/git.rb @@ -9,6 +9,24 @@ module Gitlab ref.gsub(/\Arefs\/(tags|heads)\//, '') end + def branch_name(ref) + ref = ref.to_s + if self.branch_ref?(ref) + self.ref_name(ref) + else + nil + end + end + + def tag_name(ref) + ref = ref.to_s + if self.tag_ref?(ref) + self.ref_name(ref) + else + nil + end + end + def tag_ref?(ref) ref.start_with?(TAG_REF_PREFIX) end diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb index 5bc70f530d2..1882eb8d050 100644 --- a/lib/gitlab/git_access.rb +++ b/lib/gitlab/git_access.rb @@ -76,10 +76,10 @@ module Gitlab return build_status_object(false, "A repository for this project does not exist yet.") end - changes = changes.lines if changes.kind_of?(String) + changes_list = Gitlab::ChangesList.new(changes) # Iterate over all changes to find if user allowed all of them to be applied - changes.map(&:strip).reject(&:blank?).each do |change| + changes_list.each do |change| status = change_access_check(change) unless status.allowed? # If user does not have access to make at least one change - cancel all push -- cgit v1.2.1 From 1f2253545ba7a902212bace29f144a2246eeedab Mon Sep 17 00:00:00 2001 From: Paco Guzman Date: Fri, 8 Jul 2016 18:42:47 +0200 Subject: Use cache for todos counter calling TodoService --- lib/api/todos.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/api/todos.rb b/lib/api/todos.rb index 26c24c3baff..a90a667fafe 100644 --- a/lib/api/todos.rb +++ b/lib/api/todos.rb @@ -73,7 +73,7 @@ module API # delete do todos = find_todos - todos.each(&:done) + TodoService.new.mark_todos_as_done(todos, current_user) todos.length end -- cgit v1.2.1 From f8b53ba20b74181a46985b0c7dde742239bd54f8 Mon Sep 17 00:00:00 2001 From: Paco Guzman Date: Thu, 11 Aug 2016 18:39:50 +0200 Subject: Recover usage of Todos counter cache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We’re being kept up to date the counter data but we’re not using it. The only thing which is not real if is the number of projects that the user read changes the number of todos can be stale for some time. The counters will be sync just after the user receives a new todo or mark any as done --- lib/api/todos.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'lib') diff --git a/lib/api/todos.rb b/lib/api/todos.rb index a90a667fafe..19df13d8aac 100644 --- a/lib/api/todos.rb +++ b/lib/api/todos.rb @@ -61,9 +61,9 @@ module API # delete ':id' do todo = current_user.todos.find(params[:id]) - todo.done + TodoService.new.mark_todos_as_done([todo], current_user) - present todo, with: Entities::Todo, current_user: current_user + present todo.reload, with: Entities::Todo, current_user: current_user end # Mark all todos as done @@ -74,8 +74,6 @@ module API delete do todos = find_todos TodoService.new.mark_todos_as_done(todos, current_user) - - todos.length end end end -- cgit v1.2.1 From 2f06027dc318bd8c4e177444a3168a0129a53687 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Fri, 12 Aug 2016 18:27:42 -0400 Subject: Change the order of the access rules to check simpler first, and add specs --- lib/gitlab/checks/change_access.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/gitlab/checks/change_access.rb b/lib/gitlab/checks/change_access.rb index 52f117e963b..4b32eb966aa 100644 --- a/lib/gitlab/checks/change_access.rb +++ b/lib/gitlab/checks/change_access.rb @@ -11,7 +11,7 @@ module Gitlab end def exec - error = protected_branch_checks || tag_checks || push_checks + error = push_checks || tag_checks || protected_branch_checks if error GitAccessStatus.new(false, error) -- cgit v1.2.1 From 504a3b5e6f0b2e2957cf1e4d9d8eebbf32234bdb Mon Sep 17 00:00:00 2001 From: Ahmad Sherif Date: Sun, 14 Aug 2016 22:27:49 +0200 Subject: Fix a memory leak caused by Banzai::Filter::SanitizationFilter In Banzai::Filter::SanitizationFilter#customize_whitelist, we append three lambdas that has reference to the SanitizationFilter instance, which in turn (potentially) has a reference to the following chain: context hash -> Project instance -> Repository instance -> lookup hash -> various Rugged instances -> various mmap-ed git pack files. All of the above is not garbage collected because the array we append the lambdas to is the constant HTML::Pipeline::SanitizationFilter::WHITELIST. --- lib/banzai/filter/sanitization_filter.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/banzai/filter/sanitization_filter.rb b/lib/banzai/filter/sanitization_filter.rb index ca80aac5a08..6e13282d5f4 100644 --- a/lib/banzai/filter/sanitization_filter.rb +++ b/lib/banzai/filter/sanitization_filter.rb @@ -7,7 +7,7 @@ module Banzai UNSAFE_PROTOCOLS = %w(data javascript vbscript).freeze def whitelist - whitelist = super + whitelist = super.dup customize_whitelist(whitelist) @@ -42,6 +42,8 @@ module Banzai # Allow any protocol in `a` elements... whitelist[:protocols].delete('a') + whitelist[:transformers] = whitelist[:transformers].dup + # ...but then remove links with unsafe protocols whitelist[:transformers].push(remove_unsafe_links) -- cgit v1.2.1