From 590d61a2de453b2359c81f7070caae09df6e788d Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 8 Nov 2016 02:33:04 +0800 Subject: Also show latest pipeline for ImageForBuildService --- app/services/ci/image_for_build_service.rb | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) (limited to 'app/services') diff --git a/app/services/ci/image_for_build_service.rb b/app/services/ci/image_for_build_service.rb index 75d847d5bee..17ce95073cb 100644 --- a/app/services/ci/image_for_build_service.rb +++ b/app/services/ci/image_for_build_service.rb @@ -1,11 +1,18 @@ module Ci class ImageForBuildService def execute(project, opts) - sha = opts[:sha] || ref_sha(project, opts[:ref]) + ref = opts[:ref] + sha = opts[:sha] || ref_sha(project, ref) pipelines = project.pipelines.where(sha: sha) - pipelines = pipelines.where(ref: opts[:ref]) if opts[:ref] - image_name = image_for_status(pipelines.status) + + latest_pipeline = if ref + pipelines.latest_for(ref) + else + pipelines.latest + end.first + + image_name = image_for_status(latest_pipeline.status) image_path = Rails.root.join('public/ci', image_name) OpenStruct.new(path: image_path, name: image_name) -- cgit v1.2.3 From f593abbc70ab02823cd99d2db11598b629cbb3a0 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 8 Nov 2016 03:44:25 +0800 Subject: There's not always a pipeline --- app/services/ci/image_for_build_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app/services') diff --git a/app/services/ci/image_for_build_service.rb b/app/services/ci/image_for_build_service.rb index 17ce95073cb..026a727a8f9 100644 --- a/app/services/ci/image_for_build_service.rb +++ b/app/services/ci/image_for_build_service.rb @@ -12,7 +12,7 @@ module Ci pipelines.latest end.first - image_name = image_for_status(latest_pipeline.status) + image_name = image_for_status(latest_pipeline.try(:status)) image_path = Rails.root.join('public/ci', image_name) OpenStruct.new(path: image_path, name: image_name) -- cgit v1.2.3 From 721f2d3788ae5e8374f357014bd9e20d62de0a81 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 21 Nov 2016 22:19:16 +0800 Subject: Still use compound pipeline status, but group by ref and sha so that it would show latest pipeline if ref and sha are both specified, otherwise still the same as before. --- app/services/ci/image_for_build_service.rb | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) (limited to 'app/services') diff --git a/app/services/ci/image_for_build_service.rb b/app/services/ci/image_for_build_service.rb index 026a727a8f9..d5a07ef630b 100644 --- a/app/services/ci/image_for_build_service.rb +++ b/app/services/ci/image_for_build_service.rb @@ -3,18 +3,11 @@ module Ci def execute(project, opts) ref = opts[:ref] sha = opts[:sha] || ref_sha(project, ref) - pipelines = project.pipelines.where(sha: sha) - latest_pipeline = if ref - pipelines.latest_for(ref) - else - pipelines.latest - end.first - - image_name = image_for_status(latest_pipeline.try(:status)) - + image_name = image_for_status(pipelines.latest_for(ref).status) image_path = Rails.root.join('public/ci', image_name) + OpenStruct.new(path: image_path, name: image_name) end -- cgit v1.2.3 From 6192ea53fad0ea04e356e5a79a5a0e5359ba46ce Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Thu, 24 Nov 2016 16:50:37 +0800 Subject: Rename latest_for to latest, feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7333/diffs#note_18819292 --- app/services/ci/image_for_build_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app/services') diff --git a/app/services/ci/image_for_build_service.rb b/app/services/ci/image_for_build_service.rb index d5a07ef630b..1eeb0e2363a 100644 --- a/app/services/ci/image_for_build_service.rb +++ b/app/services/ci/image_for_build_service.rb @@ -5,7 +5,7 @@ module Ci sha = opts[:sha] || ref_sha(project, ref) pipelines = project.pipelines.where(sha: sha) - image_name = image_for_status(pipelines.latest_for(ref).status) + image_name = image_for_status(pipelines.latest(ref).status) image_path = Rails.root.join('public/ci', image_name) OpenStruct.new(path: image_path, name: image_name) -- cgit v1.2.3 From 79aad815272fdebca30080cb33dd3fa77ef72350 Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Wed, 5 Oct 2016 10:12:46 -0500 Subject: fix awkward verb conjugation in cherry-pick and revert errors --- app/services/commits/change_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app/services') diff --git a/app/services/commits/change_service.rb b/app/services/commits/change_service.rb index db5f2bf9b2e..4d410f66c55 100644 --- a/app/services/commits/change_service.rb +++ b/app/services/commits/change_service.rb @@ -35,7 +35,7 @@ module Commits success else error_msg = "Sorry, we cannot #{action.to_s.dasherize} this #{@commit.change_type_title(current_user)} automatically. - It may have already been #{action.to_s.dasherize}, or a more recent commit may have updated some of its content." + A #{action.to_s.dasherize} may have already been performed with this #{@commit.change_type_title(current_user)}, or a more recent commit may have updated some of its content." raise ChangeError, error_msg end end -- cgit v1.2.3 From 7cced60069c248156decf6ceabc4d1f447e47ff7 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 14 Dec 2016 21:00:06 +0800 Subject: Introduce latest_status and add a few tests Feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7333#note_20003268 --- app/services/ci/image_for_build_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app/services') diff --git a/app/services/ci/image_for_build_service.rb b/app/services/ci/image_for_build_service.rb index 1eeb0e2363a..240ddabec36 100644 --- a/app/services/ci/image_for_build_service.rb +++ b/app/services/ci/image_for_build_service.rb @@ -5,7 +5,7 @@ module Ci sha = opts[:sha] || ref_sha(project, ref) pipelines = project.pipelines.where(sha: sha) - image_name = image_for_status(pipelines.latest(ref).status) + image_name = image_for_status(pipelines.latest_status(ref)) image_path = Rails.root.join('public/ci', image_name) OpenStruct.new(path: image_path, name: image_name) -- cgit v1.2.3 From 58486918fc12bbcc5139b6ca32461ad5e037497b Mon Sep 17 00:00:00 2001 From: Nick Thomas Date: Thu, 8 Dec 2016 15:37:41 +0000 Subject: Create environments when the build referencing them is created --- app/services/ci/create_pipeline_builds_service.rb | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) (limited to 'app/services') diff --git a/app/services/ci/create_pipeline_builds_service.rb b/app/services/ci/create_pipeline_builds_service.rb index 005014fa1de..b7da3f8e7eb 100644 --- a/app/services/ci/create_pipeline_builds_service.rb +++ b/app/services/ci/create_pipeline_builds_service.rb @@ -10,18 +10,29 @@ module Ci end end + def project + pipeline.project + end + private def create_build(build_attributes) build_attributes = build_attributes.merge( pipeline: pipeline, - project: pipeline.project, + project: project, ref: pipeline.ref, tag: pipeline.tag, user: current_user, trigger_request: trigger_request ) - pipeline.builds.create(build_attributes) + build = pipeline.builds.create(build_attributes) + + # Create the environment before the build starts. This sets its slug and + # makes it available as an environment variable + project.environments.find_or_create_by(name: build.expanded_environment_name) if + build.has_environment? + + build end def new_builds -- cgit v1.2.3 From 9f97fa4d9f4e86e8f1ff1db4621bcf81390936da Mon Sep 17 00:00:00 2001 From: Mark Fletcher Date: Wed, 14 Dec 2016 20:45:39 +0000 Subject: Ensure issuable state changes only fire webhooks once * Webhooks for close and reopen events now fired in respective services only * Prevents generic 'update' webhooks firing too --- app/services/issuable_base_service.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'app/services') diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index b5f63cc5a1a..ab3d2a9a0cd 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -184,7 +184,8 @@ class IssuableBaseService < BaseService old_labels = issuable.labels.to_a old_mentioned_users = issuable.mentioned_users.to_a - params[:label_ids] = process_label_ids(params, existing_label_ids: issuable.label_ids) + label_ids = process_label_ids(params, existing_label_ids: issuable.label_ids) + params[:label_ids] = label_ids if labels_changing?(issuable.label_ids, label_ids) if params.present? && update_issuable(issuable, params) # We do not touch as it will affect a update on updated_at field @@ -201,6 +202,10 @@ class IssuableBaseService < BaseService issuable end + def labels_changing?(old_label_ids, new_label_ids) + old_label_ids.sort != new_label_ids.sort + end + def change_state(issuable) case params.delete(:state_event) when 'reopen' -- cgit v1.2.3 From 7fa06ed55d18af4d055041eb27d38fecf9b5548f Mon Sep 17 00:00:00 2001 From: Timothy Andrew Date: Tue, 22 Nov 2016 14:34:23 +0530 Subject: Calls to the API are checked for scope. - Move the `Oauth2::AccessTokenValidationService` class to `AccessTokenValidationService`, since it is now being used for personal access token validation as well. - Each API endpoint declares the scopes it accepts (if any). Currently, the top level API module declares the `api` scope, and the `Users` API module declares the `read_user` scope (for GET requests). - Move the `find_user_by_private_token` from the API `Helpers` module to the `APIGuard` module, to avoid littering `Helpers` with more auth-related methods to support `find_user_by_private_token` --- app/services/access_token_validation_service.rb | 34 ++++++++++++++++++ .../oauth2/access_token_validation_service.rb | 42 ---------------------- 2 files changed, 34 insertions(+), 42 deletions(-) create mode 100644 app/services/access_token_validation_service.rb delete mode 100644 app/services/oauth2/access_token_validation_service.rb (limited to 'app/services') diff --git a/app/services/access_token_validation_service.rb b/app/services/access_token_validation_service.rb new file mode 100644 index 00000000000..69449f3a445 --- /dev/null +++ b/app/services/access_token_validation_service.rb @@ -0,0 +1,34 @@ +module AccessTokenValidationService + # Results: + VALID = :valid + EXPIRED = :expired + REVOKED = :revoked + INSUFFICIENT_SCOPE = :insufficient_scope + + class << self + def validate(token, scopes: []) + if token.expired? + return EXPIRED + + elsif token.revoked? + return REVOKED + + elsif !self.sufficient_scope?(token, scopes) + return INSUFFICIENT_SCOPE + + else + return VALID + end + end + + # True if the token's scope contains any of the required scopes. + def sufficient_scope?(token, required_scopes) + if required_scopes.blank? + true + else + # Check whether the token is allowed access to any of the required scopes. + Set.new(required_scopes).intersection(Set.new(token.scopes)).present? + end + end + end +end diff --git a/app/services/oauth2/access_token_validation_service.rb b/app/services/oauth2/access_token_validation_service.rb deleted file mode 100644 index 264fdccde8f..00000000000 --- a/app/services/oauth2/access_token_validation_service.rb +++ /dev/null @@ -1,42 +0,0 @@ -module Oauth2::AccessTokenValidationService - # Results: - VALID = :valid - EXPIRED = :expired - REVOKED = :revoked - INSUFFICIENT_SCOPE = :insufficient_scope - - class << self - def validate(token, scopes: []) - if token.expired? - return EXPIRED - - elsif token.revoked? - return REVOKED - - elsif !self.sufficient_scope?(token, scopes) - return INSUFFICIENT_SCOPE - - else - return VALID - end - end - - protected - - # True if the token's scope is a superset of required scopes, - # or the required scopes is empty. - def sufficient_scope?(token, scopes) - if scopes.blank? - # if no any scopes required, the scopes of token is sufficient. - return true - else - # If there are scopes required, then check whether - # the set of authorized scopes is a superset of the set of required scopes - required_scopes = Set.new(scopes) - authorized_scopes = Set.new(token.scopes) - - return authorized_scopes >= required_scopes - end - end - end -end -- cgit v1.2.3 From b303948ff549ce57d3b6985c2c366dfcdc5a2ca3 Mon Sep 17 00:00:00 2001 From: Timothy Andrew Date: Mon, 5 Dec 2016 22:55:53 +0530 Subject: Convert AccessTokenValidationService into a class. - Previously, AccessTokenValidationService was a module, and all its public methods accepted a token. It makes sense to convert it to a class which accepts a token during initialization. - Also rename the `sufficient_scope?` method to `include_any_scope?` - Based on feedback from @rymai --- app/services/access_token_validation_service.rb | 38 ++++++++++++------------- 1 file changed, 18 insertions(+), 20 deletions(-) (limited to 'app/services') diff --git a/app/services/access_token_validation_service.rb b/app/services/access_token_validation_service.rb index 69449f3a445..ddaaed90e5b 100644 --- a/app/services/access_token_validation_service.rb +++ b/app/services/access_token_validation_service.rb @@ -1,34 +1,32 @@ -module AccessTokenValidationService +AccessTokenValidationService = Struct.new(:token) do # Results: VALID = :valid EXPIRED = :expired REVOKED = :revoked INSUFFICIENT_SCOPE = :insufficient_scope - class << self - def validate(token, scopes: []) - if token.expired? - return EXPIRED + def validate(scopes: []) + if token.expired? + return EXPIRED - elsif token.revoked? - return REVOKED + elsif token.revoked? + return REVOKED - elsif !self.sufficient_scope?(token, scopes) - return INSUFFICIENT_SCOPE + elsif !self.include_any_scope?(scopes) + return INSUFFICIENT_SCOPE - else - return VALID - end + else + return VALID end + end - # True if the token's scope contains any of the required scopes. - def sufficient_scope?(token, required_scopes) - if required_scopes.blank? - true - else - # Check whether the token is allowed access to any of the required scopes. - Set.new(required_scopes).intersection(Set.new(token.scopes)).present? - end + # True if the token's scope contains any of the passed scopes. + def include_any_scope?(scopes) + if scopes.blank? + true + else + # Check whether the token is allowed access to any of the required scopes. + Set.new(scopes).intersection(Set.new(token.scopes)).present? end end end -- cgit v1.2.3 From 170efaaba273792ddffc2806ef1501f33d87a5a2 Mon Sep 17 00:00:00 2001 From: Rydkin Maxim Date: Fri, 16 Dec 2016 01:14:20 +0300 Subject: Enable Style/MultilineOperationIndentation in Rubocop, fixes #25741 --- app/services/groups/update_service.rb | 2 +- app/services/issues/update_service.rb | 2 +- app/services/merge_requests/build_service.rb | 2 +- app/services/merge_requests/update_service.rb | 2 +- app/services/projects/update_service.rb | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) (limited to 'app/services') diff --git a/app/services/groups/update_service.rb b/app/services/groups/update_service.rb index 99ad12b1003..fff2273f402 100644 --- a/app/services/groups/update_service.rb +++ b/app/services/groups/update_service.rb @@ -5,7 +5,7 @@ module Groups new_visibility = params[:visibility_level] if new_visibility && new_visibility.to_i != group.visibility_level unless can?(current_user, :change_visibility_level, group) && - Gitlab::VisibilityLevel.allowed_for?(current_user, new_visibility) + Gitlab::VisibilityLevel.allowed_for?(current_user, new_visibility) deny_visibility_level(group, new_visibility) return group diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb index a2111b3806b..78cbf94ec69 100644 --- a/app/services/issues/update_service.rb +++ b/app/services/issues/update_service.rb @@ -10,7 +10,7 @@ module Issues end if issue.previous_changes.include?('title') || - issue.previous_changes.include?('description') + issue.previous_changes.include?('description') todo_service.update_issue(issue, current_user) end diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb index bebfca7537b..6a7393a9921 100644 --- a/app/services/merge_requests/build_service.rb +++ b/app/services/merge_requests/build_service.rb @@ -42,7 +42,7 @@ module MergeRequests end if merge_request.source_project == merge_request.target_project && - merge_request.target_branch == merge_request.source_branch + merge_request.target_branch == merge_request.source_branch messages << 'You must select different branches' end diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb index fda0da19d87..ad16ef8c70f 100644 --- a/app/services/merge_requests/update_service.rb +++ b/app/services/merge_requests/update_service.rb @@ -25,7 +25,7 @@ module MergeRequests end if merge_request.previous_changes.include?('title') || - merge_request.previous_changes.include?('description') + merge_request.previous_changes.include?('description') todo_service.update_merge_request(merge_request, current_user) end diff --git a/app/services/projects/update_service.rb b/app/services/projects/update_service.rb index 921ca6748d3..8a6af8d8ada 100644 --- a/app/services/projects/update_service.rb +++ b/app/services/projects/update_service.rb @@ -6,7 +6,7 @@ module Projects if new_visibility && new_visibility.to_i != project.visibility_level unless can?(current_user, :change_visibility_level, project) && - Gitlab::VisibilityLevel.allowed_for?(current_user, new_visibility) + Gitlab::VisibilityLevel.allowed_for?(current_user, new_visibility) deny_visibility_level(project, new_visibility) return project -- cgit v1.2.3 From f73193c328b871a9a3af803012c10d9bc1bd0904 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Tue, 6 Dec 2016 17:31:58 +0100 Subject: Smarter refreshing of authorized projects Prior to this commit the refreshing of authorized projects was done in two steps: 1. Remove existing authorizations 2. Insert a new list of all authorizations This can lead to a high amount of dead tuples as every time all rows are being replaced. For example, if a user with 100 authorizations is given access to a new project this would lead to: * 100 rows being removed * 101 new rows being inserted This commit changes the way this system works so it only removes/inserts what is necessary. Using the above example this would lead to only 1 new row being inserted, with the initial 100 being left untouched. Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/25257 --- .../users/refresh_authorized_projects_service.rb | 128 +++++++++++++++++++++ 1 file changed, 128 insertions(+) create mode 100644 app/services/users/refresh_authorized_projects_service.rb (limited to 'app/services') diff --git a/app/services/users/refresh_authorized_projects_service.rb b/app/services/users/refresh_authorized_projects_service.rb new file mode 100644 index 00000000000..7d38ac3a374 --- /dev/null +++ b/app/services/users/refresh_authorized_projects_service.rb @@ -0,0 +1,128 @@ +module Users + # Service for refreshing the authorized projects of a user. + # + # This particular service class can not be used to update data for the same + # user concurrently. Doing so could lead to an incorrect state. To ensure this + # doesn't happen a caller must synchronize access (e.g. using + # `Gitlab::ExclusiveLease`). + # + # Usage: + # + # user = User.find_by(username: 'alice') + # service = Users::RefreshAuthorizedProjectsService.new(some_user) + # service.execute + class RefreshAuthorizedProjectsService + attr_reader :user + + LEASE_TIMEOUT = 1.minute.to_i + + # user - The User for which to refresh the authorized projects. + def initialize(user) + @user = user + + # We need an up to date User object that has access to all relations that + # may have been created earlier. The only way to ensure this is to reload + # the User object. + user.reload + end + + # This method returns the updated User object. + def execute + current = current_authorizations_per_project + fresh = fresh_access_levels_per_project + + remove = current.each_with_object([]) do |(project_id, row), array| + # rows not in the new list or with a different access level should be + # removed. + if !fresh[project_id] || fresh[project_id] != row.access_level + array << row.id + end + end + + add = fresh.each_with_object([]) do |(project_id, level), array| + # rows not in the old list or with a different access level should be + # added. + if !current[project_id] || current[project_id].access_level != level + array << [user.id, project_id, level] + end + end + + update_with_lease(remove, add) + end + + # Updates the list of authorizations using an exclusive lease. + def update_with_lease(remove = [], add = []) + lease_key = "refresh_authorized_projects:#{user.id}" + lease = Gitlab::ExclusiveLease.new(lease_key, timeout: LEASE_TIMEOUT) + + until uuid = lease.try_obtain + # Keep trying until we obtain the lease. If we don't do so we may end up + # not updating the list of authorized projects properly. To prevent + # hammering Redis too much we'll wait for a bit between retries. + sleep(1) + end + + begin + update_authorizations(remove, add) + ensure + Gitlab::ExclusiveLease.cancel(lease_key, uuid) + end + end + + # Updates the list of authorizations for the current user. + # + # remove - The IDs of the authorization rows to remove. + # add - Rows to insert in the form `[user id, project id, access level]` + def update_authorizations(remove = [], add = []) + return if remove.empty? && add.empty? + + User.transaction do + user.remove_project_authorizations(remove) unless remove.empty? + ProjectAuthorization.insert_authorizations(add) unless add.empty? + user.set_authorized_projects_column + end + + # Since we batch insert authorization rows, Rails' associations may get + # out of sync. As such we force a reload of the User object. + user.reload + end + + def fresh_access_levels_per_project + fresh_authorizations.each_with_object({}) do |row, hash| + hash[row.project_id] = row.access_level + end + end + + def current_authorizations_per_project + current_authorizations.each_with_object({}) do |row, hash| + hash[row.project_id] = row + end + end + + def current_authorizations + user.project_authorizations.select(:id, :project_id, :access_level) + end + + def fresh_authorizations + ProjectAuthorization. + unscoped. + select('project_id, MAX(access_level) AS access_level'). + from("(#{project_authorizations_union.to_sql}) #{ProjectAuthorization.table_name}"). + group(:project_id) + end + + private + + # Returns a union query of projects that the user is authorized to access + def project_authorizations_union + relations = [ + user.personal_projects.select("#{user.id} AS user_id, projects.id AS project_id, #{Gitlab::Access::MASTER} AS access_level"), + user.groups_projects.select_for_project_authorization, + user.projects.select_for_project_authorization, + user.groups.joins(:shared_projects).select_for_project_authorization + ] + + Gitlab::SQL::Union.new(relations) + end + end +end -- cgit v1.2.3 From 5d4531db2555d3051fc47e9268728a670ece95f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kim=20=22BKC=22=20Carlb=C3=A4cker?= Date: Mon, 17 Oct 2016 17:58:57 +0200 Subject: Gogs Importer --- app/services/projects/import_service.rb | 1 + 1 file changed, 1 insertion(+) (limited to 'app/services') diff --git a/app/services/projects/import_service.rb b/app/services/projects/import_service.rb index d7221fe993c..8cac0b01881 100644 --- a/app/services/projects/import_service.rb +++ b/app/services/projects/import_service.rb @@ -5,6 +5,7 @@ module Projects class Error < StandardError; end ALLOWED_TYPES = [ + 'gogs', 'bitbucket', 'fogbugz', 'gitlab', -- cgit v1.2.3 From 103114e3d73819f76bed9d8ad1bbdb8964875579 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Thu, 15 Dec 2016 17:31:14 +0100 Subject: Rename Gogs to Gitea, DRY the controller and improve views MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- app/services/projects/import_service.rb | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) (limited to 'app/services') diff --git a/app/services/projects/import_service.rb b/app/services/projects/import_service.rb index 8cac0b01881..287c0a4257f 100644 --- a/app/services/projects/import_service.rb +++ b/app/services/projects/import_service.rb @@ -5,13 +5,13 @@ module Projects class Error < StandardError; end ALLOWED_TYPES = [ - 'gogs', + 'github', 'bitbucket', - 'fogbugz', 'gitlab', - 'github', 'google_code', - 'gitlab_project' + 'fogbugz', + 'gitlab_project', + 'gitea' ] def execute @@ -71,8 +71,23 @@ module Projects def importer return Gitlab::ImportExport::Importer.new(project) if @project.gitlab_project_import? - class_name = "Gitlab::#{project.import_type.camelize}Import::Importer" - class_name.constantize.new(project) + class_name = + case project.import_type + when 'github', 'gitea' + Gitlab::GithubImport::Importer + when 'bitbucket' + Gitlab::BitbucketImport::Importer + when 'gitlab' + Gitlab::GitlabImport::Importer + when 'google_code' + Gitlab::GoogleCodeImport::Importer + when 'fogbugz' + Gitlab::FogbugzImport::Importer + else + raise 'Unknown importer type!' + end + + class_name.new(project) end def unknown_url? -- cgit v1.2.3 From 8fc63d1f648fa38eac9e5422dd42667d8e7f1b49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Fri, 16 Dec 2016 09:15:30 +0100 Subject: Improve Gitlab::ImportSources MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- app/services/projects/import_service.rb | 32 ++------------------------------ 1 file changed, 2 insertions(+), 30 deletions(-) (limited to 'app/services') diff --git a/app/services/projects/import_service.rb b/app/services/projects/import_service.rb index 287c0a4257f..cd230528743 100644 --- a/app/services/projects/import_service.rb +++ b/app/services/projects/import_service.rb @@ -4,16 +4,6 @@ module Projects class Error < StandardError; end - ALLOWED_TYPES = [ - 'github', - 'bitbucket', - 'gitlab', - 'google_code', - 'fogbugz', - 'gitlab_project', - 'gitea' - ] - def execute add_repository_to_project unless project.gitlab_project_import? @@ -65,29 +55,11 @@ module Projects end def has_importer? - ALLOWED_TYPES.include?(project.import_type) + Gitlab::ImportSources.importer_names.include?(project.import_type) end def importer - return Gitlab::ImportExport::Importer.new(project) if @project.gitlab_project_import? - - class_name = - case project.import_type - when 'github', 'gitea' - Gitlab::GithubImport::Importer - when 'bitbucket' - Gitlab::BitbucketImport::Importer - when 'gitlab' - Gitlab::GitlabImport::Importer - when 'google_code' - Gitlab::GoogleCodeImport::Importer - when 'fogbugz' - Gitlab::FogbugzImport::Importer - else - raise 'Unknown importer type!' - end - - class_name.new(project) + Gitlab::ImportSources.importer(project.import_type).new(project) end def unknown_url? -- cgit v1.2.3 From 33564b113386b3b9c1606dcc2ff7251c178667e5 Mon Sep 17 00:00:00 2001 From: jurre Date: Tue, 20 Dec 2016 15:23:20 +0100 Subject: Make 'unmarked as WIP' message more consistent --- app/services/system_note_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app/services') diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb index 8b48d90f60b..7613ecd5021 100644 --- a/app/services/system_note_service.rb +++ b/app/services/system_note_service.rb @@ -146,7 +146,7 @@ module SystemNoteService end def remove_merge_request_wip(noteable, project, author) - body = 'unmarked as a Work In Progress' + body = 'unmarked as a **Work In Progress**' create_note(noteable: noteable, project: project, author: author, note: body) end -- cgit v1.2.3 From b82fdf6257255b720526ccef716759892e88de09 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Tue, 20 Dec 2016 17:52:27 +0100 Subject: Fix error 500 renaming group. Also added specs and changelog. --- app/services/groups/update_service.rb | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'app/services') diff --git a/app/services/groups/update_service.rb b/app/services/groups/update_service.rb index fff2273f402..4e878ec556a 100644 --- a/app/services/groups/update_service.rb +++ b/app/services/groups/update_service.rb @@ -14,7 +14,13 @@ module Groups group.assign_attributes(params) - group.save + begin + group.save + rescue Gitlab::UpdatePathError => e + group.errors.add(:base, e.message) + + false + end end end end -- cgit v1.2.3 From 3ef4f74b1acc9399db320b53dffc592542de0126 Mon Sep 17 00:00:00 2001 From: Markus Koller Date: Tue, 22 Nov 2016 17:58:10 +0100 Subject: Add more storage statistics This adds counters for build artifacts and LFS objects, and moves the preexisting repository_size and commit_count from the projects table into a new project_statistics table. The counters are displayed in the administration area for projects and groups, and also available through the API for admins (on */all) and normal users (on */owned) The statistics are updated through ProjectCacheWorker, which can now do more granular updates with the new :statistics argument. --- app/services/git_push_service.rb | 2 +- app/services/git_tag_push_service.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'app/services') diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb index 185556c12cc..f603036cf03 100644 --- a/app/services/git_push_service.rb +++ b/app/services/git_push_service.rb @@ -74,7 +74,7 @@ class GitPushService < BaseService types = [] end - ProjectCacheWorker.perform_async(@project.id, types) + ProjectCacheWorker.perform_async(@project.id, types, [:commit_count, :repository_size]) end protected diff --git a/app/services/git_tag_push_service.rb b/app/services/git_tag_push_service.rb index 20a4445bddf..96432837481 100644 --- a/app/services/git_tag_push_service.rb +++ b/app/services/git_tag_push_service.rb @@ -12,7 +12,7 @@ class GitTagPushService < BaseService project.execute_hooks(@push_data.dup, :tag_push_hooks) project.execute_services(@push_data.dup, :tag_push_hooks) Ci::CreatePipelineService.new(project, current_user, @push_data).execute - ProjectCacheWorker.perform_async(project.id) + ProjectCacheWorker.perform_async(project.id, [], [:commit_count, :repository_size]) true end -- cgit v1.2.3 From 89d3ef38ccc7be722a0906d08b51a48b1c8ff681 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Wed, 21 Dec 2016 16:26:35 +0100 Subject: Schedule at most 100 commits When processing push payloads we now schedule at most the 100 most recent commits, instead of all commits that were in a payload. This prevents one from overloading the system by pushing thousands if not millions of commits in a single go. Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/25827 --- app/services/git_push_service.rb | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) (limited to 'app/services') diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb index 185556c12cc..6bbc3a9d9ff 100644 --- a/app/services/git_push_service.rb +++ b/app/services/git_push_service.rb @@ -3,6 +3,9 @@ class GitPushService < BaseService include Gitlab::CurrentSettings include Gitlab::Access + # The N most recent commits to process in a single push payload. + PROCESS_COMMIT_LIMIT = 100 + # This method will be called after each git update # and only if the provided user and project are present in GitLab. # @@ -77,6 +80,16 @@ class GitPushService < BaseService ProjectCacheWorker.perform_async(@project.id, types) end + # Schedules processing of commit messages. + def process_commit_messages + default = is_default_branch? + + push_commits.last(PROCESS_COMMIT_LIMIT).each do |commit| + ProcessCommitWorker. + perform_async(project.id, current_user.id, commit.to_hash, default) + end + end + protected def execute_related_hooks @@ -128,17 +141,6 @@ class GitPushService < BaseService end end - # Extract any GFM references from the pushed commit messages. If the configured issue-closing regex is matched, - # close the referenced Issue. Create cross-reference Notes corresponding to any other referenced Mentionables. - def process_commit_messages - default = is_default_branch? - - @push_commits.each do |commit| - ProcessCommitWorker. - perform_async(project.id, current_user.id, commit.to_hash, default) - end - end - def build_push_data @push_data ||= Gitlab::DataBuilder::Push.build( @project, -- cgit v1.2.3 From 9410f215eab0ba49051d2a00f0b4174f5dc13a6f Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 26 Dec 2016 11:56:19 +0200 Subject: Add nested groups support to the Groups::CreateService Signed-off-by: Dmitriy Zaporozhets --- app/services/groups/create_service.rb | 13 +++++++++++++ 1 file changed, 13 insertions(+) (limited to 'app/services') diff --git a/app/services/groups/create_service.rb b/app/services/groups/create_service.rb index 2bccd584dde..9a630aee626 100644 --- a/app/services/groups/create_service.rb +++ b/app/services/groups/create_service.rb @@ -12,6 +12,19 @@ module Groups return @group end + parent_id = params[:parent_id] + + if parent_id + parent = Group.find(parent_id) + + unless can?(current_user, :admin_group, parent) + @group.parent_id = nil + @group.errors.add(:parent_id, 'manage access required to create subgroup') + + return @group + end + end + @group.name ||= @group.path.dup @group.save @group.add_owner(current_user) -- cgit v1.2.3 From 8cc5333420230d6a6f0a2c9844af9a1f1dc955ec Mon Sep 17 00:00:00 2001 From: victorwu Date: Thu, 15 Dec 2016 13:54:43 -0600 Subject: Replace wording for slash command confirmation message --- app/services/notes/create_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app/services') diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb index d75592e31f3..1beca9f4109 100644 --- a/app/services/notes/create_service.rb +++ b/app/services/notes/create_service.rb @@ -41,7 +41,7 @@ module Notes # We must add the error after we call #save because errors are reset # when #save is called if only_commands - note.errors.add(:commands_only, 'Your commands have been executed!') + note.errors.add(:commands_only, 'Commands applied') end note.commands_changes = command_params.keys -- cgit v1.2.3 From 1b082a4c338d7575e15d7450906801db59873441 Mon Sep 17 00:00:00 2001 From: Felipe Artur Date: Wed, 14 Dec 2016 19:39:53 -0200 Subject: Check if user can read issue before being assigned --- app/services/issuable_base_service.rb | 41 ++++++++++++++++++++--------- app/services/issues/base_service.rb | 4 --- app/services/merge_requests/base_service.rb | 4 --- 3 files changed, 29 insertions(+), 20 deletions(-) (limited to 'app/services') diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index ab3d2a9a0cd..4ce5fd993d9 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -36,14 +36,10 @@ class IssuableBaseService < BaseService end end - def filter_params(issuable_ability_name = :issue) - filter_assignee - filter_milestone - filter_labels + def filter_params(issuable) + ability_name = :"admin_#{issuable.to_ability_name}" - ability = :"admin_#{issuable_ability_name}" - - unless can?(current_user, ability, project) + unless can?(current_user, ability_name, project) params.delete(:milestone_id) params.delete(:labels) params.delete(:add_label_ids) @@ -52,14 +48,35 @@ class IssuableBaseService < BaseService params.delete(:assignee_id) params.delete(:due_date) end + + filter_assignee(issuable) + filter_milestone + filter_labels end - def filter_assignee - if params[:assignee_id] == IssuableFinder::NONE - params[:assignee_id] = '' + def filter_assignee(issuable) + return unless params[:assignee_id].present? + + assignee_id = params[:assignee_id] + + if assignee_id.to_s == IssuableFinder::NONE + params[:assignee_id] = "" + else + params.delete(:assignee_id) unless assignee_can_read?(issuable, assignee_id) end end + def assignee_can_read?(issuable, assignee_id) + new_assignee = User.find_by_id(assignee_id) + + return false unless new_assignee.present? + + ability_name = :"read_#{issuable.to_ability_name}" + resource = issuable.persisted? ? issuable : project + + can?(new_assignee, ability_name, resource) + end + def filter_milestone milestone_id = params[:milestone_id] return unless milestone_id @@ -138,7 +155,7 @@ class IssuableBaseService < BaseService def create(issuable) merge_slash_commands_into_params!(issuable) - filter_params + filter_params(issuable) params.delete(:state_event) params[:author] ||= current_user @@ -180,7 +197,7 @@ class IssuableBaseService < BaseService change_state(issuable) change_subscription(issuable) change_todo(issuable) - filter_params + filter_params(issuable) old_labels = issuable.labels.to_a old_mentioned_users = issuable.mentioned_users.to_a diff --git a/app/services/issues/base_service.rb b/app/services/issues/base_service.rb index 742e834df97..35af867a098 100644 --- a/app/services/issues/base_service.rb +++ b/app/services/issues/base_service.rb @@ -17,10 +17,6 @@ module Issues private - def filter_params - super(:issue) - end - def execute_hooks(issue, action = 'open') issue_data = hook_data(issue, action) hooks_scope = issue.confidential? ? :confidential_issue_hooks : :issue_hooks diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index 800fd39c424..70e25956dc7 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -38,10 +38,6 @@ module MergeRequests private - def filter_params - super(:merge_request) - end - def merge_requests_for(branch) origin_merge_requests = @project.origin_merge_requests .opened.where(source_branch: branch).to_a -- cgit v1.2.3 From f0ba001877e074eb8046f476c9972a47fe108f1b Mon Sep 17 00:00:00 2001 From: Adam Niedzielski Date: Wed, 28 Dec 2016 14:41:30 +0100 Subject: Cache project authorizations even when user has access to zero projects --- app/services/users/refresh_authorized_projects_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app/services') diff --git a/app/services/users/refresh_authorized_projects_service.rb b/app/services/users/refresh_authorized_projects_service.rb index 7d38ac3a374..8559908e0c3 100644 --- a/app/services/users/refresh_authorized_projects_service.rb +++ b/app/services/users/refresh_authorized_projects_service.rb @@ -74,7 +74,7 @@ module Users # remove - The IDs of the authorization rows to remove. # add - Rows to insert in the form `[user id, project id, access level]` def update_authorizations(remove = [], add = []) - return if remove.empty? && add.empty? + return if remove.empty? && add.empty? && user.authorized_projects_populated User.transaction do user.remove_project_authorizations(remove) unless remove.empty? -- cgit v1.2.3 From 283e868ef523185b0ee314b9e2164599780d888b Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 29 Dec 2016 19:18:05 +0200 Subject: Refactor nested group related code * Simplify code around group parent access check * Rename 'Nested groups' to 'Subgroups' tab at group#show page Signed-off-by: Dmitriy Zaporozhets --- app/services/groups/create_service.rb | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) (limited to 'app/services') diff --git a/app/services/groups/create_service.rb b/app/services/groups/create_service.rb index 9a630aee626..febeb661fb5 100644 --- a/app/services/groups/create_service.rb +++ b/app/services/groups/create_service.rb @@ -12,17 +12,11 @@ module Groups return @group end - parent_id = params[:parent_id] + if @group.parent && !can?(current_user, :admin_group, @group.parent) + @group.parent = nil + @group.errors.add(:parent_id, 'manage access required to create subgroup') - if parent_id - parent = Group.find(parent_id) - - unless can?(current_user, :admin_group, parent) - @group.parent_id = nil - @group.errors.add(:parent_id, 'manage access required to create subgroup') - - return @group - end + return @group end @group.name ||= @group.path.dup -- cgit v1.2.3