diff options
Diffstat (limited to 'lib')
| -rw-r--r-- | lib/api/entities.rb | 5 | ||||
| -rw-r--r-- | lib/api/labels.rb | 41 | ||||
| -rw-r--r-- | lib/api/system_hooks.rb | 2 | ||||
| -rw-r--r-- | lib/banzai/renderer.rb | 6 | ||||
| -rw-r--r-- | lib/extracts_path.rb | 16 | ||||
| -rw-r--r-- | lib/gitlab/backend/shell.rb | 13 | ||||
| -rw-r--r-- | lib/gitlab/exclusive_lease.rb | 66 |
7 files changed, 70 insertions, 79 deletions
diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 01e31f6f7d1..1942aeea656 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -433,11 +433,14 @@ module API end class LabelBasic < Grape::Entity - expose :name, :color, :description + expose :id, :name, :color, :description end class Label < LabelBasic expose :open_issues_count, :closed_issues_count, :open_merge_requests_count + expose :priority do |label, options| + label.priority(options[:project]) + end expose :subscribed do |label, options| label.subscribed?(options[:current_user]) diff --git a/lib/api/labels.rb b/lib/api/labels.rb index 238cea00fba..97218054f37 100644 --- a/lib/api/labels.rb +++ b/lib/api/labels.rb @@ -11,7 +11,7 @@ module API success Entities::Label end get ':id/labels' do - present available_labels, with: Entities::Label, current_user: current_user + present available_labels, with: Entities::Label, current_user: current_user, project: user_project end desc 'Create a new label' do @@ -21,6 +21,7 @@ module API requires :name, type: String, desc: 'The name of the label to be created' requires :color, type: String, desc: "The color of the label given in 6-digit hex notation with leading '#' sign (e.g. #FFAABB)" optional :description, type: String, desc: 'The description of label to be created' + optional :priority, type: Integer, desc: 'The priority of the label', allow_blank: true end post ':id/labels' do authorize! :admin_label, user_project @@ -28,10 +29,15 @@ module API label = available_labels.find_by(title: params[:name]) conflict!('Label already exists') if label - label = user_project.labels.create(declared(params, include_parent_namespaces: false).to_h) + priority = params.delete(:priority) + label_params = declared(params, + include_parent_namespaces: false, + include_missing: false).to_h + label = user_project.labels.create(label_params) if label.valid? - present label, with: Entities::Label, current_user: current_user + label.prioritize!(user_project, priority) if priority + present label, with: Entities::Label, current_user: current_user, project: user_project else render_validation_error!(label) end @@ -49,7 +55,7 @@ module API label = user_project.labels.find_by(title: params[:name]) not_found!('Label') unless label - present label.destroy, with: Entities::Label, current_user: current_user + present label.destroy, with: Entities::Label, current_user: current_user, project: user_project end desc 'Update an existing label. At least one optional parameter is required.' do @@ -60,7 +66,8 @@ module API optional :new_name, type: String, desc: 'The new name of the label' optional :color, type: String, desc: "The new color of the label given in 6-digit hex notation with leading '#' sign (e.g. #FFAABB)" optional :description, type: String, desc: 'The new description of label' - at_least_one_of :new_name, :color, :description + optional :priority, type: Integer, desc: 'The priority of the label', allow_blank: true + at_least_one_of :new_name, :color, :description, :priority end put ':id/labels' do authorize! :admin_label, user_project @@ -68,17 +75,25 @@ module API label = user_project.labels.find_by(title: params[:name]) not_found!('Label not found') unless label - update_params = declared(params, - include_parent_namespaces: false, - include_missing: false).to_h + update_priority = params.key?(:priority) + priority = params.delete(:priority) + label_params = declared(params, + include_parent_namespaces: false, + include_missing: false).to_h # Rename new name to the actual label attribute name - update_params['name'] = update_params.delete('new_name') if update_params.key?('new_name') + label_params[:name] = label_params.delete('new_name') if label_params.key?('new_name') - if label.update(update_params) - present label, with: Entities::Label, current_user: current_user - else - render_validation_error!(label) + render_validation_error!(label) unless label.update(label_params) + + if update_priority + if priority.nil? + label.unprioritize!(user_project) + else + label.prioritize!(user_project, priority) + end end + + present label, with: Entities::Label, current_user: current_user, project: user_project end end end diff --git a/lib/api/system_hooks.rb b/lib/api/system_hooks.rb index 32f731c5652..b6bfff9f20f 100644 --- a/lib/api/system_hooks.rb +++ b/lib/api/system_hooks.rb @@ -32,7 +32,7 @@ module API if hook.save present hook, with: Entities::Hook else - not_found! + render_validation_error!(hook) end end diff --git a/lib/banzai/renderer.rb b/lib/banzai/renderer.rb index ce048a36fa0..f31fb6c3f71 100644 --- a/lib/banzai/renderer.rb +++ b/lib/banzai/renderer.rb @@ -46,7 +46,7 @@ module Banzai return html if html.present? html = cacheless_render_field(object, field) - object.update_column(html_field, html) unless object.new_record? || object.destroyed? + update_object(object, html_field, html) unless object.new_record? || object.destroyed? html end @@ -166,5 +166,9 @@ module Banzai return unless cache_key Rails.cache.send(:expanded_key, full_cache_key(cache_key, pipeline_name)) end + + def update_object(object, html_field, html) + object.update_column(html_field, html) + end end end diff --git a/lib/extracts_path.rb b/lib/extracts_path.rb index 9b74364849e..82551f1f222 100644 --- a/lib/extracts_path.rb +++ b/lib/extracts_path.rb @@ -106,7 +106,7 @@ module ExtractsPath # resolved (e.g., when a user inserts an invalid path or ref). def assign_ref_vars # assign allowed options - allowed_options = ["filter_ref", "extended_sha1"] + allowed_options = ["filter_ref"] @options = params.select {|key, value| allowed_options.include?(key) && !value.blank? } @options = HashWithIndifferentAccess.new(@options) @@ -114,17 +114,13 @@ module ExtractsPath @ref, @path = extract_ref(@id) @repo = @project.repository - if @options[:extended_sha1].present? - @commit = @repo.commit(@options[:extended_sha1]) - else - @commit = @repo.commit(@ref) + @commit = @repo.commit(@ref) - if @path.empty? && !@commit && @id.ends_with?('.atom') - @id = @ref = extract_ref_without_atom(@id) - @commit = @repo.commit(@ref) + if @path.empty? && !@commit && @id.ends_with?('.atom') + @id = @ref = extract_ref_without_atom(@id) + @commit = @repo.commit(@ref) - request.format = :atom if @commit - end + request.format = :atom if @commit end raise InvalidPathError unless @commit diff --git a/lib/gitlab/backend/shell.rb b/lib/gitlab/backend/shell.rb index 9cec71a3222..82e194c1af1 100644 --- a/lib/gitlab/backend/shell.rb +++ b/lib/gitlab/backend/shell.rb @@ -127,19 +127,6 @@ module Gitlab 'rm-project', storage, "#{name}.git"]) end - # Gc repository - # - # storage - project storage path - # path - project path with namespace - # - # Ex. - # gc("/path/to/storage", "gitlab/gitlab-ci") - # - def gc(storage, path) - Gitlab::Utils.system_silent([gitlab_shell_projects_path, 'gc', - storage, "#{path}.git"]) - end - # Add new key to gitlab-shell # # Ex. diff --git a/lib/gitlab/exclusive_lease.rb b/lib/gitlab/exclusive_lease.rb index 7e8f35e9298..2dd42704396 100644 --- a/lib/gitlab/exclusive_lease.rb +++ b/lib/gitlab/exclusive_lease.rb @@ -1,66 +1,52 @@ +require 'securerandom' + module Gitlab # This class implements an 'exclusive lease'. We call it a 'lease' # because it has a set expiry time. We call it 'exclusive' because only # one caller may obtain a lease for a given key at a time. The # implementation is intended to work across GitLab processes and across - # servers. It is a 'cheap' alternative to using SQL queries and updates: + # servers. It is a cheap alternative to using SQL queries and updates: # you do not need to change the SQL schema to start using # ExclusiveLease. # - # It is important to choose the timeout wisely. If the timeout is very - # high (1 hour) then the throughput of your operation gets very low (at - # most once an hour). If the timeout is lower than how long your - # operation may take then you cannot count on exclusivity. For example, - # if the timeout is 10 seconds and you do an operation which may take 20 - # seconds then two overlapping operations may hold a lease for the same - # key at the same time. - # - # This class has no 'cancel' method. I originally decided against adding - # it because it would add complexity and a false sense of security. The - # complexity: instead of setting '1' we would have to set a UUID, and to - # delete it we would have to execute Lua on the Redis server to only - # delete the key if the value was our own UUID. Otherwise there is a - # chance that when you intend to cancel your lease you actually delete - # someone else's. The false sense of security: you cannot design your - # system to rely too much on the lease being cancelled after use because - # the calling (Ruby) process may crash or be killed. You _cannot_ count - # on begin/ensure blocks to cancel a lease, because the 'ensure' does - # not always run. Think of 'kill -9' from the Unicorn master for - # instance. - # - # If you find that leases are getting in your way, ask yourself: would - # it be enough to lower the lease timeout? Another thing that might be - # appropriate is to only use a lease for bulk/automated operations, and - # to ignore the lease when you get a single 'manual' user request (a - # button click). - # class ExclusiveLease + LUA_CANCEL_SCRIPT = <<-EOS + local key, uuid = KEYS[1], ARGV[1] + if redis.call("get", key) == uuid then + redis.call("del", key) + end + EOS + + def self.cancel(key, uuid) + Gitlab::Redis.with do |redis| + redis.eval(LUA_CANCEL_SCRIPT, keys: [redis_key(key)], argv: [uuid]) + end + end + + def self.redis_key(key) + "gitlab:exclusive_lease:#{key}" + end + def initialize(key, timeout:) - @key, @timeout = key, timeout + @redis_key = self.class.redis_key(key) + @timeout = timeout + @uuid = SecureRandom.uuid end - # Try to obtain the lease. Return true on success, + # Try to obtain the lease. Return lease UUID on success, # false if the lease is already taken. def try_obtain # Performing a single SET is atomic Gitlab::Redis.with do |redis| - !!redis.set(redis_key, '1', nx: true, ex: @timeout) + redis.set(@redis_key, @uuid, nx: true, ex: @timeout) && @uuid end end # Returns true if the key for this lease is set. def exists? Gitlab::Redis.with do |redis| - redis.exists(redis_key) + redis.exists(@redis_key) end end - - # No #cancel method. See comments above! - - private - - def redis_key - "gitlab:exclusive_lease:#{@key}" - end end end |