From 0aedeb5637932fa827e42be7441e9c967049dd1d Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Tue, 21 Jun 2016 13:55:33 +0200 Subject: Add predefined CI variables to GitLab for container registry, pipelines, project name, etc. --- app/models/ci/build.rb | 32 ++++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) (limited to 'app/models') diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index d618c84e983..d823d67e3ba 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -431,10 +431,34 @@ module Ci def predefined_variables variables = [] - variables << { key: :CI_BUILD_TAG, value: ref, public: true } if tag? - variables << { key: :CI_BUILD_NAME, value: name, public: true } - variables << { key: :CI_BUILD_STAGE, value: stage, public: true } - variables << { key: :CI_BUILD_TRIGGERED, value: 'true', public: true } if trigger_request + variables << { key: 'CI', value: 'true', public: true } + variables << { key: 'GITLAB_CI', value: 'true', public: true } + + variables << { key: 'CI_BUILD_ID', value: id, public: true } + variables << { key: 'CI_BUILD_TOKEN', value: token, public: false } + variables << { key: 'CI_BUILD_REF', value: sha, public: true } + variables << { key: 'CI_BUILD_BEFORE_SHA', value: before_sha, public: true } + variables << { key: 'CI_BUILD_REF_NAME', value: ref, public: true } + variables << { key: 'CI_BUILD_TAG', value: ref, public: true } if tag? + variables << { key: 'CI_BUILD_NAME', value: name, public: true } + variables << { key: 'CI_BUILD_STAGE', value: stage, public: true } + variables << { key: 'CI_BUILD_TRIGGERED', value: 'true', public: true } if trigger_request + + variables << { key: 'CI_PIPELINE_ID', value: pipeline.id, public: true } + + variables << { key: 'CI_PROJECT_ID', value: project_id, public: true } + variables << { key: 'CI_PROJECT_NAME', value: project.path, public: true } + variables << { key: 'CI_PROJECT_PATH', value: project.path_with_namespace, public: true } + variables << { key: 'CI_PROJECT_NAMESPACE', value: project.namespace.path, public: true } + variables << { key: 'CI_PROJECT_URL', value: project.web_url, public: true } + + variables << { key: 'CI_REGISTRY', value: Gitlab.config.registry.host_port, public: true } if Gitlab.config.registry.enabled + variables << { key: 'CI_REGISTRY_IMAGE', value: project.container_registry_repository_url, public: true } if project.container_registry_repository_url + + variables << { key: 'CI_SERVER_NAME', value: 'GitLab', public: true } + variables << { key: 'CI_SERVER_VERSION', value: Gitlab::VERSION, public: true } + variables << { key: 'CI_SERVER_REVISION', value: Gitlab::REVISION, public: true } + variables << { key: 'CI_SERVER_URL', value: Gitlab::REVISION, public: true } variables end end -- cgit v1.2.1 From defb8660c08a904a385b584280f72fc6a5a94c6e Mon Sep 17 00:00:00 2001 From: Patricio Cano Date: Thu, 14 Jul 2016 13:19:40 -0500 Subject: Added the ability to block sign ups using a domain blacklist. --- app/models/application_setting.rb | 35 ++++++++++++++++++++++++++++------ app/models/user.rb | 40 ++++++++++++++++++++++++++------------- 2 files changed, 56 insertions(+), 19 deletions(-) (limited to 'app/models') diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index c6f77cc055f..84b1b54eeae 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -9,7 +9,9 @@ class ApplicationSetting < ActiveRecord::Base serialize :import_sources serialize :disabled_oauth_sign_in_sources, Array serialize :restricted_signup_domains, Array - attr_accessor :restricted_signup_domains_raw + serialize :domain_blacklist, Array + + attr_accessor :restricted_signup_domains_raw, :domain_blacklist_raw validates :session_expire_delay, presence: true, @@ -62,6 +64,10 @@ class ApplicationSetting < ActiveRecord::Base validates :enabled_git_access_protocol, inclusion: { in: %w(ssh http), allow_blank: true, allow_nil: true } + validates :domain_blacklist, + presence: true, + if: :domain_blacklist_enabled? + validates_each :restricted_visibility_levels do |record, attr, value| unless value.nil? value.each do |level| @@ -154,18 +160,35 @@ class ApplicationSetting < ActiveRecord::Base self.restricted_signup_domains.join("\n") unless self.restricted_signup_domains.nil? end - def restricted_signup_domains_raw=(values) - self.restricted_signup_domains = [] - self.restricted_signup_domains = values.split( - /\s*[,;]\s* # comma or semicolon, optionally surrounded by whitespace + def domain_blacklist_raw + self.domain_blacklist.join("\n") unless self.domain_blacklist.nil? + end + + def splitter + /\s*[,;]\s* # comma or semicolon, optionally surrounded by whitespace | # or \s # any whitespace character | # or [\r\n] # any number of newline characters - /x) + /x + end + + def restricted_signup_domains_raw=(values) + self.restricted_signup_domains = [] + self.restricted_signup_domains = values.split(splitter) self.restricted_signup_domains.reject! { |d| d.empty? } end + def domain_blacklist_raw=(values) + self.domain_blacklist = [] + self.domain_blacklist = values.split(splitter) + self.domain_blacklist.reject! { |d| d.empty? } + end + + def domain_blacklist_file=(file) + self.domain_blacklist_raw = file.read + end + def runners_registration_token ensure_runners_registration_token! end diff --git a/app/models/user.rb b/app/models/user.rb index 3d0a033785c..b0c5d84fc40 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -111,7 +111,7 @@ class User < ActiveRecord::Base validates :avatar, file_size: { maximum: 200.kilobytes.to_i } before_validation :generate_password, on: :create - before_validation :restricted_signup_domains, on: :create + before_validation :signup_domain_valid?, on: :create before_validation :sanitize_attrs before_validation :set_notification_email, if: ->(user) { user.email_changed? } before_validation :set_public_email, if: ->(user) { user.public_email_changed? } @@ -760,27 +760,41 @@ class User < ActiveRecord::Base Project.where(id: events) end - def restricted_signup_domains - email_domains = current_application_settings.restricted_signup_domains + def match_domain(email_domains) + email_domains.any? do |domain| + escaped = Regexp.escape(domain).gsub('\*', '.*?') + regexp = Regexp.new "^#{escaped}$", Regexp::IGNORECASE + email_domain = Mail::Address.new(self.email).domain + email_domain =~ regexp + end + end + + def signup_domain_valid? + valid = true - unless email_domains.blank? - match_found = email_domains.any? do |domain| - escaped = Regexp.escape(domain).gsub('\*', '.*?') - regexp = Regexp.new "^#{escaped}$", Regexp::IGNORECASE - email_domain = Mail::Address.new(self.email).domain - email_domain =~ regexp + if current_application_settings.domain_blacklist_enabled? + blocked_domains = current_application_settings.domain_blacklist + if match_domain(blocked_domains) + self.errors.add :email, 'is not from an allowed domain.' + valid = false end + end - unless match_found + allowed_domains = current_application_settings.restricted_signup_domains + unless allowed_domains.blank? + if match_domain(allowed_domains) + self.errors.clear + valid = true + else self.errors.add :email, 'is not whitelisted. ' + 'Email domains valid for registration are: ' + - email_domains.join(', ') - return false + allowed_domains.join(', ') + valid = false end end - true + return valid end def can_be_removed? -- cgit v1.2.1 From ce58437cfad3c82371b1790e47f97bc5e1d9a889 Mon Sep 17 00:00:00 2001 From: Patricio Cano Date: Thu, 14 Jul 2016 22:47:36 -0500 Subject: Fixed `signup_domain_valid?` flow and added documentation. --- app/models/user.rb | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) (limited to 'app/models') diff --git a/app/models/user.rb b/app/models/user.rb index b0c5d84fc40..d27e2374f18 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -760,41 +760,31 @@ class User < ActiveRecord::Base Project.where(id: events) end - def match_domain(email_domains) - email_domains.any? do |domain| - escaped = Regexp.escape(domain).gsub('\*', '.*?') - regexp = Regexp.new "^#{escaped}$", Regexp::IGNORECASE - email_domain = Mail::Address.new(self.email).domain - email_domain =~ regexp - end - end - def signup_domain_valid? valid = true + error = nil if current_application_settings.domain_blacklist_enabled? blocked_domains = current_application_settings.domain_blacklist - if match_domain(blocked_domains) - self.errors.add :email, 'is not from an allowed domain.' + if match_domain(blocked_domains, self.email) + error = 'is not from an allowed domain.' valid = false end end allowed_domains = current_application_settings.restricted_signup_domains unless allowed_domains.blank? - if match_domain(allowed_domains) - self.errors.clear + if match_domain(allowed_domains, self.email) valid = true else - self.errors.add :email, - 'is not whitelisted. ' + - 'Email domains valid for registration are: ' + - allowed_domains.join(', ') + error = "is not whitelisted. Email domains valid for registration are: #{allowed_domains.join(', ')}" valid = false end end - return valid + self.errors.add(:email, error) unless valid + + valid end def can_be_removed? @@ -895,4 +885,15 @@ class User < ActiveRecord::Base self.can_create_group = false self.projects_limit = 0 end + + private + + def match_domain(email_domains, email) + signup_domain = Mail::Address.new(email).domain + email_domains.any? do |domain| + escaped = Regexp.escape(domain).gsub('\*', '.*?') + regexp = Regexp.new "^#{escaped}$", Regexp::IGNORECASE + signup_domain =~ regexp + end + end end -- cgit v1.2.1 From e15fa67c9894ccb52aeb1f0116e083c9dbba24f5 Mon Sep 17 00:00:00 2001 From: Patricio Cano Date: Fri, 15 Jul 2016 09:54:04 -0500 Subject: Make sure email domain validation method is private. --- app/models/user.rb | 54 ++++++++++++++++++++++++++---------------------------- 1 file changed, 26 insertions(+), 28 deletions(-) (limited to 'app/models') diff --git a/app/models/user.rb b/app/models/user.rb index d27e2374f18..0168008355b 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -760,33 +760,6 @@ class User < ActiveRecord::Base Project.where(id: events) end - def signup_domain_valid? - valid = true - error = nil - - if current_application_settings.domain_blacklist_enabled? - blocked_domains = current_application_settings.domain_blacklist - if match_domain(blocked_domains, self.email) - error = 'is not from an allowed domain.' - valid = false - end - end - - allowed_domains = current_application_settings.restricted_signup_domains - unless allowed_domains.blank? - if match_domain(allowed_domains, self.email) - valid = true - else - error = "is not whitelisted. Email domains valid for registration are: #{allowed_domains.join(', ')}" - valid = false - end - end - - self.errors.add(:email, error) unless valid - - valid - end - def can_be_removed? !solo_owned_groups.present? end @@ -886,7 +859,32 @@ class User < ActiveRecord::Base self.projects_limit = 0 end - private + def signup_domain_valid? + valid = true + error = nil + + if current_application_settings.domain_blacklist_enabled? + blocked_domains = current_application_settings.domain_blacklist + if match_domain(blocked_domains, self.email) + error = 'is not from an allowed domain.' + valid = false + end + end + + allowed_domains = current_application_settings.restricted_signup_domains + unless allowed_domains.blank? + if match_domain(allowed_domains, self.email) + valid = true + else + error = "is not whitelisted. Email domains valid for registration are: #{allowed_domains.join(', ')}" + valid = false + end + end + + self.errors.add(:email, error) unless valid + + valid + end def match_domain(email_domains, email) signup_domain = Mail::Address.new(email).domain -- cgit v1.2.1 From 7943767267873423acb1eddbf00b6c5684f7849f Mon Sep 17 00:00:00 2001 From: Patricio Cano Date: Fri, 15 Jul 2016 13:20:45 -0500 Subject: Refactored the domain separator regex, plus syntax and grammar fixes. --- app/models/application_setting.rb | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) (limited to 'app/models') diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 84b1b54eeae..03c2bc0b57d 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -4,6 +4,12 @@ class ApplicationSetting < ActiveRecord::Base add_authentication_token_field :health_check_access_token CACHE_KEY = 'application_setting.last' + DOMAIN_LIST_SEPARATOR = %r{\s*[,;]\s* # comma or semicolon, optionally surrounded by whitespace + | # or + \s # any whitespace character + | # or + [\r\n] # any number of newline characters + }x serialize :restricted_visibility_levels serialize :import_sources @@ -164,25 +170,18 @@ class ApplicationSetting < ActiveRecord::Base self.domain_blacklist.join("\n") unless self.domain_blacklist.nil? end - def splitter - /\s*[,;]\s* # comma or semicolon, optionally surrounded by whitespace - | # or - \s # any whitespace character - | # or - [\r\n] # any number of newline characters - /x - end - def restricted_signup_domains_raw=(values) self.restricted_signup_domains = [] - self.restricted_signup_domains = values.split(splitter) + self.restricted_signup_domains = values.split(DOMAIN_LIST_SEPARATOR) self.restricted_signup_domains.reject! { |d| d.empty? } + self.restricted_signup_domains end def domain_blacklist_raw=(values) self.domain_blacklist = [] - self.domain_blacklist = values.split(splitter) + self.domain_blacklist = values.split(DOMAIN_LIST_SEPARATOR) self.domain_blacklist.reject! { |d| d.empty? } + self.domain_blacklist end def domain_blacklist_file=(file) -- cgit v1.2.1 From c71e658ccac85f111517e04b79d915c10867c7e3 Mon Sep 17 00:00:00 2001 From: Patricio Cano Date: Fri, 15 Jul 2016 18:30:38 -0500 Subject: Refactor and rename `restricted_signup_domains` to `domain_whitelist` to better conform to its behavior and newly introduced behavior. --- app/models/application_setting.rb | 20 ++++++++++---------- app/models/user.rb | 2 +- 2 files changed, 11 insertions(+), 11 deletions(-) (limited to 'app/models') diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 03c2bc0b57d..d923b4d5235 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -14,10 +14,10 @@ class ApplicationSetting < ActiveRecord::Base serialize :restricted_visibility_levels serialize :import_sources serialize :disabled_oauth_sign_in_sources, Array - serialize :restricted_signup_domains, Array + serialize :domain_whitelist, Array serialize :domain_blacklist, Array - attr_accessor :restricted_signup_domains_raw, :domain_blacklist_raw + attr_accessor :domain_whitelist_raw, :domain_blacklist_raw validates :session_expire_delay, presence: true, @@ -141,7 +141,7 @@ class ApplicationSetting < ActiveRecord::Base session_expire_delay: Settings.gitlab['session_expire_delay'], default_project_visibility: Settings.gitlab.default_projects_features['visibility_level'], default_snippet_visibility: Settings.gitlab.default_projects_features['visibility_level'], - restricted_signup_domains: Settings.gitlab['restricted_signup_domains'], + domain_whitelist: Settings.gitlab['domain_whitelist'], import_sources: %w[github bitbucket gitlab gitorious google_code fogbugz git gitlab_project], shared_runners_enabled: Settings.gitlab_ci['shared_runners_enabled'], max_artifacts_size: Settings.artifacts['max_size'], @@ -162,19 +162,19 @@ class ApplicationSetting < ActiveRecord::Base ActiveRecord::Base.connection.column_exists?(:application_settings, :home_page_url) end - def restricted_signup_domains_raw - self.restricted_signup_domains.join("\n") unless self.restricted_signup_domains.nil? + def domain_whitelist_raw + self.domain_whitelist.join("\n") unless self.domain_whitelist.nil? end def domain_blacklist_raw self.domain_blacklist.join("\n") unless self.domain_blacklist.nil? end - def restricted_signup_domains_raw=(values) - self.restricted_signup_domains = [] - self.restricted_signup_domains = values.split(DOMAIN_LIST_SEPARATOR) - self.restricted_signup_domains.reject! { |d| d.empty? } - self.restricted_signup_domains + def domain_whitelist_raw=(values) + self.domain_whitelist = [] + self.domain_whitelist = values.split(DOMAIN_LIST_SEPARATOR) + self.domain_whitelist.reject! { |d| d.empty? } + self.domain_whitelist end def domain_blacklist_raw=(values) diff --git a/app/models/user.rb b/app/models/user.rb index 0168008355b..6932e750fe0 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -871,7 +871,7 @@ class User < ActiveRecord::Base end end - allowed_domains = current_application_settings.restricted_signup_domains + allowed_domains = current_application_settings.domain_whitelist unless allowed_domains.blank? if match_domain(allowed_domains, self.email) valid = true -- cgit v1.2.1 From 23afb02aaa957dd1a5ce35a141e4e8ecd80052ca Mon Sep 17 00:00:00 2001 From: Patricio Cano Date: Sat, 16 Jul 2016 11:44:50 -0500 Subject: Refactor `match_domain` to a predicate: `domain_matches?` --- app/models/user.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'app/models') diff --git a/app/models/user.rb b/app/models/user.rb index 6932e750fe0..516934c295c 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -865,7 +865,7 @@ class User < ActiveRecord::Base if current_application_settings.domain_blacklist_enabled? blocked_domains = current_application_settings.domain_blacklist - if match_domain(blocked_domains, self.email) + if domain_matches?(blocked_domains, self.email) error = 'is not from an allowed domain.' valid = false end @@ -873,7 +873,7 @@ class User < ActiveRecord::Base allowed_domains = current_application_settings.domain_whitelist unless allowed_domains.blank? - if match_domain(allowed_domains, self.email) + if domain_matches?(allowed_domains, self.email) valid = true else error = "is not whitelisted. Email domains valid for registration are: #{allowed_domains.join(', ')}" @@ -886,7 +886,7 @@ class User < ActiveRecord::Base valid end - def match_domain(email_domains, email) + def domain_matches?(email_domains, email) signup_domain = Mail::Address.new(email).domain email_domains.any? do |domain| escaped = Regexp.escape(domain).gsub('\*', '.*?') -- cgit v1.2.1 From 6b8eceda395ae25b7ea189627b04da1f223c57d7 Mon Sep 17 00:00:00 2001 From: Patricio Cano Date: Mon, 18 Jul 2016 17:49:33 -0500 Subject: Default to manual input for `domain_whitelist`, syntax fixes and added new tests. --- app/models/application_setting.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app/models') diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index d923b4d5235..8c19d9dc9c8 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -71,7 +71,7 @@ class ApplicationSetting < ActiveRecord::Base inclusion: { in: %w(ssh http), allow_blank: true, allow_nil: true } validates :domain_blacklist, - presence: true, + presence: { message: 'Domain blacklist cannot be empty if Blacklist is enabled.' }, if: :domain_blacklist_enabled? validates_each :restricted_visibility_levels do |record, attr, value| -- cgit v1.2.1 From 08b9532e376eae369cd04d9f86ea560acfd19ed0 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 19 Jul 2016 20:59:38 +0800 Subject: API for downloading latest successful build: This was extracted from !5142 and implementing part of #4255. We split it from !5142 because we want to ship it in 8.10 while !5142 was not ready yet. --- app/models/ci/build.rb | 3 +++ app/models/ci/pipeline.rb | 10 +++++++++- app/models/project.rb | 7 +++++++ 3 files changed, 19 insertions(+), 1 deletion(-) (limited to 'app/models') diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index ffac3a22efc..65dfe4f0190 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -15,6 +15,9 @@ module Ci scope :with_artifacts, ->() { where.not(artifacts_file: nil) } scope :with_expired_artifacts, ->() { with_artifacts.where('artifacts_expire_at < ?', Time.now) } scope :last_month, ->() { where('created_at > ?', Date.today - 1.month) } + scope :latest_successful_with_artifacts, ->() do + with_artifacts.success.order(id: :desc) + end mount_uploader :artifacts_file, ArtifactUploader mount_uploader :artifacts_metadata, ArtifactUploader diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index a65a826536d..f5b4124d1ee 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -20,6 +20,14 @@ module Ci after_touch :update_state after_save :keep_around_commits + # ref can't be HEAD, can only be branch/tag name or SHA + scope :latest_successful_for, ->(ref) do + table = quoted_table_name + # TODO: Use `where(ref: ref).or(sha: ref)` in Rails 5 + where("#{table}.ref = ? OR #{table}.sha = ?", ref, ref). + success.order(id: :desc) + end + def self.truncate_sha(sha) sha[0...8] end @@ -222,7 +230,7 @@ module Ci def keep_around_commits return unless project - + project.repository.keep_around(self.sha) project.repository.keep_around(self.before_sha) end diff --git a/app/models/project.rb b/app/models/project.rb index a805f5d97bc..29aaaf5117f 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -429,6 +429,13 @@ class Project < ActiveRecord::Base repository.commit(ref) end + # ref can't be HEAD, can only be branch/tag name or SHA + def latest_successful_builds_for(ref = 'master') + Ci::Build.joins(:pipeline). + merge(pipelines.latest_successful_for(ref)). + latest_successful_with_artifacts + end + def merge_base_commit(first_commit_id, second_commit_id) sha = repository.merge_base(first_commit_id, second_commit_id) repository.commit(sha) if sha -- cgit v1.2.1 From 13e74543f9d0f91b7b3ef14279fd78d83f442750 Mon Sep 17 00:00:00 2001 From: Eugene Howe Date: Sun, 17 Jul 2016 10:32:11 -0400 Subject: speed up ExternalWikiService#get_project_wiki_path * This method previously iterated over all services in a project. Now it will directly query the ExternalWikiService for the project and filter by active state. * The presence of an external wiki is also cached * When an external wiki is added or removed, the cached value is updated --- app/models/project.rb | 16 ++++++++++++++++ app/models/service.rb | 8 ++++++++ 2 files changed, 24 insertions(+) (limited to 'app/models') diff --git a/app/models/project.rb b/app/models/project.rb index a805f5d97bc..d08b737e813 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -650,6 +650,22 @@ class Project < ActiveRecord::Base update_column(:has_external_issue_tracker, services.external_issue_trackers.any?) end + def external_wiki + if has_external_wiki.nil? + cache_has_external_wiki # Populate + end + + if has_external_wiki + @external_wiki ||= services.external_wikis.first + else + nil + end + end + + def cache_has_external_wiki + update_column(:has_external_wiki, services.external_wikis.any?) + end + def build_missing_services services_templates = Service.where(template: true) diff --git a/app/models/service.rb b/app/models/service.rb index 5432f8c7ab4..4821096379c 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -17,6 +17,7 @@ class Service < ActiveRecord::Base after_commit :reset_updated_properties after_commit :cache_project_has_external_issue_tracker + after_commit :cache_project_has_external_wiki belongs_to :project, inverse_of: :services has_one :service_hook @@ -25,6 +26,7 @@ class Service < ActiveRecord::Base scope :visible, -> { where.not(type: ['GitlabIssueTrackerService', 'GitlabCiService']) } scope :issue_trackers, -> { where(category: 'issue_tracker') } + scope :external_wikis, -> { where(type: 'ExternalWikiService') } scope :active, -> { where(active: true) } scope :without_defaults, -> { where(default: false) } @@ -212,4 +214,10 @@ class Service < ActiveRecord::Base project.cache_has_external_issue_tracker end end + + def cache_project_has_external_wiki + if project && !project.destroyed? + project.cache_has_external_wiki + end + end end -- cgit v1.2.1 From 6054c855a082fd822611d358914ac20695608e5b Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 19 Jul 2016 22:08:16 +0800 Subject: Only allow branches/tags, disallow SHA: Feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5347#note_13170854 --- app/models/ci/pipeline.rb | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) (limited to 'app/models') diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index fab91bdae5a..7efa67466c1 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -20,12 +20,9 @@ module Ci after_touch :update_state after_save :keep_around_commits - # ref can't be HEAD, can only be branch/tag name or SHA + # ref can't be HEAD or SHA, can only be branch/tag name scope :latest_successful_for, ->(ref) do - table = quoted_table_name - # TODO: Use `where(ref: ref).or(sha: ref)` in Rails 5 - where("#{table}.ref = ? OR #{table}.sha = ?", ref, ref). - success.order(id: :desc) + where(ref: ref).success.order(id: :desc) end def self.truncate_sha(sha) -- cgit v1.2.1 From 0465876197b58bee0223b5ca5cfd2da971ea60b9 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 20 Jul 2016 00:52:16 +0800 Subject: Use default_branch rather than master, feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5347#note_13173871 --- app/models/ci/pipeline.rb | 2 +- app/models/project.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'app/models') diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 7efa67466c1..3646baea88e 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -21,7 +21,7 @@ module Ci after_save :keep_around_commits # ref can't be HEAD or SHA, can only be branch/tag name - scope :latest_successful_for, ->(ref) do + scope :latest_successful_for, ->(ref = default_branch) do where(ref: ref).success.order(id: :desc) end diff --git a/app/models/project.rb b/app/models/project.rb index 29aaaf5117f..4ba3881d30e 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -430,7 +430,7 @@ class Project < ActiveRecord::Base end # ref can't be HEAD, can only be branch/tag name or SHA - def latest_successful_builds_for(ref = 'master') + def latest_successful_builds_for(ref = default_branch) Ci::Build.joins(:pipeline). merge(pipelines.latest_successful_for(ref)). latest_successful_with_artifacts -- cgit v1.2.1 From 091142118ee9555544acf6d05f61fd109e400ff2 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 20 Jul 2016 01:08:16 +0800 Subject: Now we could use normal relation, feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5347#note_13173842 --- app/models/project.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'app/models') diff --git a/app/models/project.rb b/app/models/project.rb index 4ba3881d30e..026fff0da0c 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -431,8 +431,7 @@ class Project < ActiveRecord::Base # ref can't be HEAD, can only be branch/tag name or SHA def latest_successful_builds_for(ref = default_branch) - Ci::Build.joins(:pipeline). - merge(pipelines.latest_successful_for(ref)). + builds.where(pipeline: pipelines.latest_successful_for(ref)). latest_successful_with_artifacts end -- cgit v1.2.1 From b306a52114bb155538dcee3b0eab815ac8d2655d Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Tue, 12 Jul 2016 11:05:48 -0600 Subject: Add with_warnings? method to Pipelines and add tests. --- app/models/ci/pipeline.rb | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'app/models') diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index aca8607f4e8..e862978d7b4 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -146,6 +146,12 @@ module Ci end end + def with_warnings? + builds.latest.any? do |build| + build.failed? && build.allow_failure + end + end + def config_processor return nil unless ci_yaml_file return @config_processor if defined?(@config_processor) -- cgit v1.2.1 From b2a79554c339a583b6cc0d471b8224a24f11c96d Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Thu, 14 Jul 2016 08:58:05 -0600 Subject: Address feedback. --- app/models/ci/pipeline.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'app/models') diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index e862978d7b4..90ed57e9aa0 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -146,10 +146,8 @@ module Ci end end - def with_warnings? - builds.latest.any? do |build| - build.failed? && build.allow_failure - end + def has_warnings? + builds.latest.ignored.any? end def config_processor -- cgit v1.2.1 From 17bac49154a399d34e7b884551d2fb78dff3cea3 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 19 Jul 2016 13:19:04 -0600 Subject: Ensure Owners are included in the scope for authorized_projects Prior, when providing a `min_access_level` parameter to this method, we called `Gitlab::Access.values` instead of `all_values`, mistakenly omitting the `OWNER` level. Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/19878 --- app/models/user.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app/models') diff --git a/app/models/user.rb b/app/models/user.rb index 3d0a033785c..975e935fa20 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -854,7 +854,7 @@ class User < ActiveRecord::Base groups.joins(:shared_projects).select(:project_id)] if min_access_level - scope = { access_level: Gitlab::Access.values.select { |access| access >= min_access_level } } + scope = { access_level: Gitlab::Access.all_values.select { |access| access >= min_access_level } } relations = [relations.shift] + relations.map { |relation| relation.where(members: scope) } end -- cgit v1.2.1 From e6cbd626845ee3436fdcf36b087fd1de894804a0 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Wed, 20 Jul 2016 00:08:54 +0200 Subject: Update all exposed variables to CI builds --- app/models/ci/build.rb | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) (limited to 'app/models') diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index cc4809b3abe..6115ffd87c2 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -451,7 +451,7 @@ module Ci variables << { key: 'CI', value: 'true', public: true } variables << { key: 'GITLAB_CI', value: 'true', public: true } - variables << { key: 'CI_BUILD_ID', value: id, public: true } + variables << { key: 'CI_BUILD_ID', value: id.to_s, public: true } variables << { key: 'CI_BUILD_TOKEN', value: token, public: false } variables << { key: 'CI_BUILD_REF', value: sha, public: true } variables << { key: 'CI_BUILD_BEFORE_SHA', value: before_sha, public: true } @@ -461,21 +461,32 @@ module Ci variables << { key: 'CI_BUILD_STAGE', value: stage, public: true } variables << { key: 'CI_BUILD_TRIGGERED', value: 'true', public: true } if trigger_request - variables << { key: 'CI_PIPELINE_ID', value: pipeline.id, public: true } + variables << { key: 'CI_PIPELINE_ID', value: pipeline.id.to_s, public: true } - variables << { key: 'CI_PROJECT_ID', value: project_id, public: true } + variables << { key: 'CI_PROJECT_ID', value: project.id.to_s, public: true } variables << { key: 'CI_PROJECT_NAME', value: project.path, public: true } variables << { key: 'CI_PROJECT_PATH', value: project.path_with_namespace, public: true } variables << { key: 'CI_PROJECT_NAMESPACE', value: project.namespace.path, public: true } variables << { key: 'CI_PROJECT_URL', value: project.web_url, public: true } - variables << { key: 'CI_REGISTRY', value: Gitlab.config.registry.host_port, public: true } if Gitlab.config.registry.enabled - variables << { key: 'CI_REGISTRY_IMAGE', value: project.container_registry_repository_url, public: true } if project.container_registry_repository_url + if Gitlab.config.registry.enabled + variables << { key: 'CI_REGISTRY', value: Gitlab.config.registry.host_port, public: true } + + if project.container_registry_enabled? + variables << { key: 'CI_REGISTRY_IMAGE', value: project.container_registry_repository_url, public: true } + end + end variables << { key: 'CI_SERVER_NAME', value: 'GitLab', public: true } variables << { key: 'CI_SERVER_VERSION', value: Gitlab::VERSION, public: true } variables << { key: 'CI_SERVER_REVISION', value: Gitlab::REVISION, public: true } - variables << { key: 'CI_SERVER_URL', value: Gitlab::REVISION, public: true } + + if runner + variables << { key: 'CI_RUNNER_ID', value: runner.id.to_s, public: true } + variables << { key: 'CI_RUNNER_DESCRIPTION', value: runner.description, public: true } + variables << { key: 'CI_RUNNER_TAGS', value: runner.tag_list.to_s, public: true } + end + variables end -- cgit v1.2.1 From 8bd520d70e035cd67d19b7962911ae9c31d1ff3d Mon Sep 17 00:00:00 2001 From: Felipe Artur Date: Wed, 6 Jul 2016 17:52:00 -0300 Subject: Allow slack service to send messages on different channels --- app/models/project_services/slack_service.rb | 48 +++++++++++++++++++++++----- 1 file changed, 40 insertions(+), 8 deletions(-) (limited to 'app/models') diff --git a/app/models/project_services/slack_service.rb b/app/models/project_services/slack_service.rb index cf9e4d5a8b6..a1f146ac05a 100644 --- a/app/models/project_services/slack_service.rb +++ b/app/models/project_services/slack_service.rb @@ -4,6 +4,9 @@ class SlackService < Service validates :webhook, presence: true, url: true, if: :activated? def initialize_properties + # Custom serialized properties initialization + self.supported_events.each { |event| self.class.prop_accessor event_channel_name(event) } + if properties.nil? self.properties = {} self.notify_only_broken_builds = true @@ -29,13 +32,15 @@ class SlackService < Service end def fields - [ - { type: 'text', name: 'webhook', - placeholder: 'https://hooks.slack.com/services/...' }, - { type: 'text', name: 'username', placeholder: 'username' }, - { type: 'text', name: 'channel', placeholder: '#channel' }, - { type: 'checkbox', name: 'notify_only_broken_builds' }, - ] + default_fields = + [ + { type: 'text', name: 'webhook', placeholder: 'https://hooks.slack.com/services/...' }, + { type: 'text', name: 'username', placeholder: 'username' }, + { type: 'text', name: 'channel', placeholder: "#General" }, + { type: 'checkbox', name: 'notify_only_broken_builds' }, + ] + + default_fields + build_event_channels end def supported_events @@ -74,7 +79,10 @@ class SlackService < Service end opt = {} - opt[:channel] = channel if channel + + event_channel = get_channel_field(object_kind) || channel + + opt[:channel] = event_channel if event_channel opt[:username] = username if username if message @@ -83,8 +91,32 @@ class SlackService < Service end end + def event_channel_names + supported_events.map { |event| event_channel_name(event) } + end + private + def get_channel_field(event) + field_name = event_channel_name(event) + self.send(field_name) + end + + def build_event_channels + channels = [] + + supported_events.each do |event| + channel_name = event_channel_name(event) + channels << { type: 'text', name: channel_name, placeholder: "#General" } + end + + channels + end + + def event_channel_name(event) + "#{event}_channel" + end + def project_name project.name_with_namespace.gsub(/\s/, '') end -- cgit v1.2.1 From ede048b930b2ceb89013793d878524eb20248d1f Mon Sep 17 00:00:00 2001 From: Felipe Artur Date: Tue, 12 Jul 2016 17:00:49 -0300 Subject: Add project service documentation and update integration documentation --- app/models/project_services/slack_service.rb | 15 +++++---------- app/models/service.rb | 4 ++++ 2 files changed, 9 insertions(+), 10 deletions(-) (limited to 'app/models') diff --git a/app/models/project_services/slack_service.rb b/app/models/project_services/slack_service.rb index a1f146ac05a..647188cc2ab 100644 --- a/app/models/project_services/slack_service.rb +++ b/app/models/project_services/slack_service.rb @@ -5,7 +5,7 @@ class SlackService < Service def initialize_properties # Custom serialized properties initialization - self.supported_events.each { |event| self.class.prop_accessor event_channel_name(event) } + self.supported_events.each { |event| self.class.prop_accessor(event_channel_name(event)) } if properties.nil? self.properties = {} @@ -36,7 +36,7 @@ class SlackService < Service [ { type: 'text', name: 'webhook', placeholder: 'https://hooks.slack.com/services/...' }, { type: 'text', name: 'username', placeholder: 'username' }, - { type: 'text', name: 'channel', placeholder: "#General" }, + { type: 'text', name: 'channel', placeholder: "#general" }, { type: 'checkbox', name: 'notify_only_broken_builds' }, ] @@ -99,18 +99,13 @@ class SlackService < Service def get_channel_field(event) field_name = event_channel_name(event) - self.send(field_name) + self.public_send(field_name) end def build_event_channels - channels = [] - - supported_events.each do |event| - channel_name = event_channel_name(event) - channels << { type: 'text', name: channel_name, placeholder: "#General" } + supported_events.reduce([]) do |channels, event| + channels << { type: 'text', name: event_channel_name(event), placeholder: "#general" } end - - channels end def event_channel_name(event) diff --git a/app/models/service.rb b/app/models/service.rb index 5432f8c7ab4..5e80b5e65d7 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -80,6 +80,10 @@ class Service < ActiveRecord::Base Gitlab::PushDataBuilder.build_sample(project, user) end + def event_channel_names + [] + end + def supported_events %w(push tag_push issue merge_request wiki_page) end -- cgit v1.2.1 From eb122e40a2ba8d1fe79d746deb4c7c803f8164a0 Mon Sep 17 00:00:00 2001 From: Paco Guzman Date: Wed, 20 Jul 2016 09:31:56 +0200 Subject: Expire tags/branches repo cache before delete a repository --- app/models/repository.rb | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'app/models') diff --git a/app/models/repository.rb b/app/models/repository.rb index 1a2ac90da51..511df2d67c6 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -392,6 +392,11 @@ class Repository expire_cache if exists? + # expire cache that don't depend on repository data (when expiring) + expire_tags_cache + expire_tag_count_cache + expire_branches_cache + expire_branch_count_cache expire_root_ref_cache expire_emptiness_caches expire_exists_cache -- cgit v1.2.1 From 2fe8ebc143737390ea8ba952ad1b8ba4c82dae84 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 20 Jul 2016 18:06:35 +0800 Subject: We need INNER JOIN to get the right pipeline, also added a test for checking this. --- app/models/project.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app/models') diff --git a/app/models/project.rb b/app/models/project.rb index 026fff0da0c..f2e9d607967 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -431,7 +431,7 @@ class Project < ActiveRecord::Base # ref can't be HEAD, can only be branch/tag name or SHA def latest_successful_builds_for(ref = default_branch) - builds.where(pipeline: pipelines.latest_successful_for(ref)). + Ci::Build.joins(:pipeline).merge(pipelines.latest_successful_for(ref)). latest_successful_with_artifacts end -- cgit v1.2.1 From c3882eb4eeba0792ffdf4773fa9283ea0a852270 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Wed, 20 Jul 2016 13:17:21 +0200 Subject: Improve implementation of variables --- app/models/ci/build.rb | 78 ++++++++++++---------------------------- app/models/ci/pipeline.rb | 6 ++++ app/models/ci/runner.rb | 8 +++++ app/models/ci/trigger_request.rb | 8 +++++ app/models/project.rb | 27 ++++++++++++++ 5 files changed, 72 insertions(+), 55 deletions(-) (limited to 'app/models') diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 6115ffd87c2..21c0e128b98 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -145,7 +145,15 @@ module Ci end def variables - predefined_variables + yaml_variables + project_variables + trigger_variables + variables = predefined_variables + variables += project.predefined_variables + variables += pipeline.predefined_variables + variables += runner.predefined_variables if runner + variables += project.container_registry_variables + variables += yaml_variables + variables += project.secret_variables + variables += trigger_request.user_variables if trigger_request + variables end def merge_request @@ -430,63 +438,23 @@ module Ci self.update(erased_by: user, erased_at: Time.now, artifacts_expire_at: nil) end - def project_variables - project.variables.map do |variable| - { key: variable.key, value: variable.value, public: false } - end - end - - def trigger_variables - if trigger_request && trigger_request.variables - trigger_request.variables.map do |key, value| - { key: key, value: value, public: false } - end - else - [] - end - end - def predefined_variables - variables = [] - variables << { key: 'CI', value: 'true', public: true } - variables << { key: 'GITLAB_CI', value: 'true', public: true } - - variables << { key: 'CI_BUILD_ID', value: id.to_s, public: true } - variables << { key: 'CI_BUILD_TOKEN', value: token, public: false } - variables << { key: 'CI_BUILD_REF', value: sha, public: true } - variables << { key: 'CI_BUILD_BEFORE_SHA', value: before_sha, public: true } - variables << { key: 'CI_BUILD_REF_NAME', value: ref, public: true } + variables = [ + { key: 'CI', value: 'true', public: true }, + { key: 'GITLAB_CI', value: 'true', public: true }, + { key: 'CI_BUILD_ID', value: id.to_s, public: true }, + { key: 'CI_BUILD_TOKEN', value: token, public: false }, + { key: 'CI_BUILD_REF', value: sha, public: true }, + { key: 'CI_BUILD_BEFORE_SHA', value: before_sha, public: true }, + { key: 'CI_BUILD_REF_NAME', value: ref, public: true }, + { key: 'CI_BUILD_NAME', value: name, public: true }, + { key: 'CI_BUILD_STAGE', value: stage, public: true }, + { key: 'CI_SERVER_NAME', value: 'GitLab', public: true }, + { key: 'CI_SERVER_VERSION', value: Gitlab::VERSION, public: true }, + { key: 'CI_SERVER_REVISION', value: Gitlab::REVISION, public: true } + ] variables << { key: 'CI_BUILD_TAG', value: ref, public: true } if tag? - variables << { key: 'CI_BUILD_NAME', value: name, public: true } - variables << { key: 'CI_BUILD_STAGE', value: stage, public: true } variables << { key: 'CI_BUILD_TRIGGERED', value: 'true', public: true } if trigger_request - - variables << { key: 'CI_PIPELINE_ID', value: pipeline.id.to_s, public: true } - - variables << { key: 'CI_PROJECT_ID', value: project.id.to_s, public: true } - variables << { key: 'CI_PROJECT_NAME', value: project.path, public: true } - variables << { key: 'CI_PROJECT_PATH', value: project.path_with_namespace, public: true } - variables << { key: 'CI_PROJECT_NAMESPACE', value: project.namespace.path, public: true } - variables << { key: 'CI_PROJECT_URL', value: project.web_url, public: true } - - if Gitlab.config.registry.enabled - variables << { key: 'CI_REGISTRY', value: Gitlab.config.registry.host_port, public: true } - - if project.container_registry_enabled? - variables << { key: 'CI_REGISTRY_IMAGE', value: project.container_registry_repository_url, public: true } - end - end - - variables << { key: 'CI_SERVER_NAME', value: 'GitLab', public: true } - variables << { key: 'CI_SERVER_VERSION', value: Gitlab::VERSION, public: true } - variables << { key: 'CI_SERVER_REVISION', value: Gitlab::REVISION, public: true } - - if runner - variables << { key: 'CI_RUNNER_ID', value: runner.id.to_s, public: true } - variables << { key: 'CI_RUNNER_DESCRIPTION', value: runner.description, public: true } - variables << { key: 'CI_RUNNER_TAGS', value: runner.tag_list.to_s, public: true } - end - variables end diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index aca8607f4e8..ad117e3950d 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -198,6 +198,12 @@ module Ci Note.for_commit_id(sha) end + def predefined_variables + [ + { key: 'CI_PIPELINE_ID', value: id.to_s, public: true } + ] + end + private def build_builds_for_stages(stages, user, status, trigger_request) diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb index b64ec79ec2b..49f05f881a2 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -114,6 +114,14 @@ module Ci tag_list.any? end + def predefined_variables + [ + { key: 'CI_RUNNER_ID', value: id.to_s, public: true }, + { key: 'CI_RUNNER_DESCRIPTION', value: description, public: true }, + { key: 'CI_RUNNER_TAGS', value: tag_list.to_s, public: true } + ] + end + private def tag_constraints diff --git a/app/models/ci/trigger_request.rb b/app/models/ci/trigger_request.rb index fcf2b6dc5e2..fc674871743 100644 --- a/app/models/ci/trigger_request.rb +++ b/app/models/ci/trigger_request.rb @@ -7,5 +7,13 @@ module Ci has_many :builds, class_name: 'Ci::Build' serialize :variables + + def user_variables + return [] unless variables + + variables.map do |key, value| + { key: key, value: value, public: false } + end + end end end diff --git a/app/models/project.rb b/app/models/project.rb index a805f5d97bc..71e22087187 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1164,4 +1164,31 @@ class Project < ActiveRecord::Base def ensure_dir_exist gitlab_shell.add_namespace(repository_storage_path, namespace.path) end + + def predefined_variables + [ + { key: 'CI_PROJECT_ID', value: id.to_s, public: true }, + { key: 'CI_PROJECT_NAME', value: path, public: true }, + { key: 'CI_PROJECT_PATH', value: path_with_namespace, public: true }, + { key: 'CI_PROJECT_NAMESPACE', value: namespace.path, public: true }, + { key: 'CI_PROJECT_URL', value: web_url, public: true } + ] + end + + def container_registry_variables + return [] unless Gitlab.config.registry.enabled + + variables = [ + { key: 'CI_REGISTRY', value: Gitlab.config.registry.host_port, public: true } + ] + + variables << { key: 'CI_REGISTRY_IMAGE', value: container_registry_repository_url, public: true } if container_registry_enabled? + variables + end + + def secret_variables + variables.map do |variable| + { key: variable.key, value: variable.value, public: false } + end + end end -- cgit v1.2.1 From a563f3311399d526bdfbef067c5a4c571acaaed5 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 20 Jul 2016 19:27:53 +0800 Subject: Join on association --- app/models/project.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app/models') diff --git a/app/models/project.rb b/app/models/project.rb index f2e9d607967..5cfc1d407e4 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -431,7 +431,7 @@ class Project < ActiveRecord::Base # ref can't be HEAD, can only be branch/tag name or SHA def latest_successful_builds_for(ref = default_branch) - Ci::Build.joins(:pipeline).merge(pipelines.latest_successful_for(ref)). + builds.joins(:pipeline).merge(pipelines.latest_successful_for(ref)). latest_successful_with_artifacts end -- cgit v1.2.1 From 8eafdafdda25071dcad4ce143d5c65c1ca278730 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Wed, 20 Jul 2016 14:00:25 +0200 Subject: Fix review comments --- app/models/project.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'app/models') diff --git a/app/models/project.rb b/app/models/project.rb index 71e22087187..a2a91888a54 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1182,7 +1182,10 @@ class Project < ActiveRecord::Base { key: 'CI_REGISTRY', value: Gitlab.config.registry.host_port, public: true } ] - variables << { key: 'CI_REGISTRY_IMAGE', value: container_registry_repository_url, public: true } if container_registry_enabled? + if container_registry_enabled? + variables << { key: 'CI_REGISTRY_IMAGE', value: container_registry_repository_url, public: true } + end + variables end -- cgit v1.2.1 From 4d69cb9d9460f9805bfc1f34ca3a600f54804167 Mon Sep 17 00:00:00 2001 From: Felipe Artur Date: Fri, 15 Jul 2016 18:46:29 -0300 Subject: Allow to disable user request access to groups/projects --- app/models/ability.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'app/models') diff --git a/app/models/ability.rb b/app/models/ability.rb index 6fd18f2ee24..e6c186c6910 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -172,7 +172,7 @@ class Ability rules << :read_build if project.public_builds? unless owner || project.team.member?(user) || project_group_member?(project, user) - rules << :request_access + rules << :request_access if project.request_access_enabled end end @@ -372,7 +372,7 @@ class Ability ] end - if group.public? || (group.internal? && !user.external?) + if (group.public? || (group.internal? && !user.external?)) && group.request_access_enabled rules << :request_access unless group.users.include?(user) end -- cgit v1.2.1 From 5fb436aaa4b9b597a1c9e995ecd13ee2a76aaedf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Wed, 20 Jul 2016 12:34:29 +0200 Subject: Fix a few nitpicks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- app/models/ability.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'app/models') diff --git a/app/models/ability.rb b/app/models/ability.rb index e6c186c6910..f33c8d61d3f 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -372,8 +372,8 @@ class Ability ] end - if (group.public? || (group.internal? && !user.external?)) && group.request_access_enabled - rules << :request_access unless group.users.include?(user) + if group.public? || (group.internal? && !user.external?) + rules << :request_access if group.request_access_enabled && group.users.exclude?(user) end rules.flatten -- cgit v1.2.1 From 9f70abf185c5d55afc392f2ed39246594c62886d Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 20 Jul 2016 21:46:46 +0800 Subject: Avoid mixing builds from different pipelines: So we no longer join anything, just find the latest pipeline and load builds from there to avoid mixing builds. Thanks Kamil for the help and tests. --- app/models/ci/build.rb | 3 --- app/models/ci/pipeline.rb | 2 +- app/models/project.rb | 9 +++++++-- 3 files changed, 8 insertions(+), 6 deletions(-) (limited to 'app/models') diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 20492c54729..49a123d488b 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -15,9 +15,6 @@ module Ci scope :with_artifacts, ->() { where.not(artifacts_file: nil) } scope :with_expired_artifacts, ->() { with_artifacts.where('artifacts_expire_at < ?', Time.now) } scope :last_month, ->() { where('created_at > ?', Date.today - 1.month) } - scope :latest_successful_with_artifacts, ->() do - with_artifacts.success.order(id: :desc) - end scope :manual_actions, ->() { where(when: :manual) } mount_uploader :artifacts_file, ArtifactUploader diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 3646baea88e..63246de4692 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -22,7 +22,7 @@ module Ci # ref can't be HEAD or SHA, can only be branch/tag name scope :latest_successful_for, ->(ref = default_branch) do - where(ref: ref).success.order(id: :desc) + where(ref: ref).success.order(id: :desc).limit(1) end def self.truncate_sha(sha) diff --git a/app/models/project.rb b/app/models/project.rb index 5cfc1d407e4..4fd635abbb3 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -431,8 +431,13 @@ class Project < ActiveRecord::Base # ref can't be HEAD, can only be branch/tag name or SHA def latest_successful_builds_for(ref = default_branch) - builds.joins(:pipeline).merge(pipelines.latest_successful_for(ref)). - latest_successful_with_artifacts + latest_successful_pipeline = pipelines.latest_successful_for(ref).first + + if latest_successful_pipeline + latest_successful_pipeline.builds.with_artifacts.latest + else + builds.none + end end def merge_base_commit(first_commit_id, second_commit_id) -- cgit v1.2.1 From 4a1edae3ac321241e2168b98df695e11048f7724 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 20 Jul 2016 22:00:34 +0800 Subject: Use one query. Feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5347#note_13199055 --- app/models/project.rb | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) (limited to 'app/models') diff --git a/app/models/project.rb b/app/models/project.rb index 4fd635abbb3..80860f142d4 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -431,13 +431,8 @@ class Project < ActiveRecord::Base # ref can't be HEAD, can only be branch/tag name or SHA def latest_successful_builds_for(ref = default_branch) - latest_successful_pipeline = pipelines.latest_successful_for(ref).first - - if latest_successful_pipeline - latest_successful_pipeline.builds.with_artifacts.latest - else - builds.none - end + pipeline = pipelines.latest_successful_for(ref) + builds.where(pipeline: pipeline).latest.with_artifacts end def merge_base_commit(first_commit_id, second_commit_id) -- cgit v1.2.1 From a7713232ea723a7215ab0aff4c6fed87343df1fd Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 20 Jul 2016 22:12:17 +0800 Subject: Also exclude artifacts_file with empty string, feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5347#note_13198105 --- app/models/ci/build.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'app/models') diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 49a123d488b..f87c1206e91 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -12,7 +12,9 @@ module Ci scope :unstarted, ->() { where(runner_id: nil) } scope :ignore_failures, ->() { where(allow_failure: false) } - scope :with_artifacts, ->() { where.not(artifacts_file: nil) } + scope :with_artifacts, ->() do + where.not(artifacts_file: [nil, '']) + end scope :with_expired_artifacts, ->() { with_artifacts.where('artifacts_expire_at < ?', Time.now) } scope :last_month, ->() { where('created_at > ?', Date.today - 1.month) } scope :manual_actions, ->() { where(when: :manual) } -- cgit v1.2.1 From 30f375cf5f5b5ca1d708c6f9558308a1d57cbd71 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Wed, 20 Jul 2016 16:38:38 +0200 Subject: Don't show other actions of the same name --- app/models/ci/build.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app/models') diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 21c0e128b98..58f25b20acf 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -97,7 +97,7 @@ module Ci end def other_actions - pipeline.manual_actions.where.not(id: self) + pipeline.manual_actions.where.not(name: name) end def playable? -- cgit v1.2.1 From 6c339026a39eabb8593d2851d21f8eef7b595378 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Wed, 20 Jul 2016 16:39:15 +0200 Subject: Fix a problem with processing a pipeline where stage only has manual actions --- app/models/ci/pipeline.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'app/models') diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index ad117e3950d..88fa01c896d 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -212,8 +212,9 @@ module Ci # build builds only for the first stage that has builds available. # stages.any? do |stage| - CreateBuildsService.new(self) - .execute(stage, user, status, trigger_request).present? + CreateBuildsService.new(self). + execute(stage, user, status, trigger_request). + any?(&:active?) end end -- cgit v1.2.1 From 323d796a0e7b5f1ef5a170f9918897f6a2d4121e Mon Sep 17 00:00:00 2001 From: Felipe Artur Date: Wed, 13 Jul 2016 16:49:47 -0300 Subject: Refactor service settings view --- app/models/project_services/slack_service.rb | 8 ++++++++ app/models/service.rb | 8 ++++++++ 2 files changed, 16 insertions(+) (limited to 'app/models') diff --git a/app/models/project_services/slack_service.rb b/app/models/project_services/slack_service.rb index 647188cc2ab..abbc780dc1a 100644 --- a/app/models/project_services/slack_service.rb +++ b/app/models/project_services/slack_service.rb @@ -95,6 +95,14 @@ class SlackService < Service supported_events.map { |event| event_channel_name(event) } end + def event_field(event) + fields.find { |field| field[:name] == event_channel_name(event) } + end + + def global_fields + fields.reject { |field| field[:name].end_with?('channel') } + end + private def get_channel_field(event) diff --git a/app/models/service.rb b/app/models/service.rb index 5e80b5e65d7..5bdbde97500 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -84,6 +84,14 @@ class Service < ActiveRecord::Base [] end + def event_field(event) + nil + end + + def global_fields + fields + end + def supported_events %w(push tag_push issue merge_request wiki_page) end -- cgit v1.2.1 From caf6438a2495c09a10f047d18a67b7071d05f7a2 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 20 Jul 2016 23:42:07 +0800 Subject: It's not longer than 80 chars --- app/models/ci/build.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'app/models') diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index f87c1206e91..a24665eed72 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -12,9 +12,7 @@ module Ci scope :unstarted, ->() { where(runner_id: nil) } scope :ignore_failures, ->() { where(allow_failure: false) } - scope :with_artifacts, ->() do - where.not(artifacts_file: [nil, '']) - end + scope :with_artifacts, ->() { where.not(artifacts_file: [nil, '']) } scope :with_expired_artifacts, ->() { with_artifacts.where('artifacts_expire_at < ?', Time.now) } scope :last_month, ->() { where('created_at > ?', Date.today - 1.month) } scope :manual_actions, ->() { where(when: :manual) } -- cgit v1.2.1 From 122b046b92573f3e1db579b5ff73fa3bdfffc124 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Thu, 21 Jul 2016 01:10:08 +0800 Subject: Workaround MySQL with INNER JOIN: This version of MySQL doesn't yet support 'LIMIT & IN/ALL/ANY/SOME subquery' Oh well. --- app/models/commit_status.rb | 6 +++++- app/models/project.rb | 9 +++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) (limited to 'app/models') diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index 535db26240a..2d185c28809 100644 --- a/app/models/commit_status.rb +++ b/app/models/commit_status.rb @@ -16,7 +16,11 @@ class CommitStatus < ActiveRecord::Base alias_attribute :author, :user - scope :latest, -> { where(id: unscope(:select).select('max(id)').group(:name, :commit_id)) } + scope :latest, -> do + max_id = unscope(:select).select("max(#{quoted_table_name}.id)") + + where(id: max_id.group(:name, :commit_id)) + end scope :retried, -> { where.not(id: latest) } scope :ordered, -> { order(:name) } scope :ignored, -> { where(allow_failure: true, status: [:failed, :canceled]) } diff --git a/app/models/project.rb b/app/models/project.rb index 80860f142d4..9f0dcb9a212 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -431,8 +431,13 @@ class Project < ActiveRecord::Base # ref can't be HEAD, can only be branch/tag name or SHA def latest_successful_builds_for(ref = default_branch) - pipeline = pipelines.latest_successful_for(ref) - builds.where(pipeline: pipeline).latest.with_artifacts + pipeline = pipelines.latest_successful_for(ref).to_sql + join_sql = "INNER JOIN (#{pipeline}) pipelines" + + " ON pipelines.id = #{Ci::Build.quoted_table_name}.commit_id" + builds.joins(join_sql).latest.with_artifacts + # TODO: Whenever we dropped support for MySQL, we could change to: + # pipeline = pipelines.latest_successful_for(ref) + # builds.where(pipeline: pipeline).latest.with_artifacts end def merge_base_commit(first_commit_id, second_commit_id) -- cgit v1.2.1 From ea63346df5a420cebbc44491eef8e2d2a0fb5ad7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Thu, 7 Jul 2016 16:08:06 -0400 Subject: Refactor user authorization check for a single project to avoid querying all user projects Currently, even when searching for all authorized issues of *one* project, we run the `Users#authorized_projects` query (which can be rather slow). This update checks if we are handling issues of just one project and does the authorization check locally. It does have the downside of basically repeating the logic of `Users#authorized_projects` on `Project#authorized_for_user`. --- app/models/issue.rb | 40 ++++++++++++++++++++++++++++++++++++++++ app/models/project.rb | 40 ++++++++++++++++++++++++++++++++++++++++ app/models/user.rb | 2 ++ 3 files changed, 82 insertions(+) (limited to 'app/models') diff --git a/app/models/issue.rb b/app/models/issue.rb index 60abd47409e..60af8c15340 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -52,10 +52,50 @@ class Issue < ActiveRecord::Base attributes end + class << self + private + + # Returns the project that the current scope belongs to if any, nil otherwise. + # + # Examples: + # - my_project.issues.without_due_date.owner_project => my_project + # - Issue.all.owner_project => nil + def owner_project + # No owner if we're not being called from an association + return unless all.respond_to?(:proxy_association) + + owner = all.proxy_association.owner + + # Check if the association is or belongs to a project + if owner.is_a?(Project) + owner + else + begin + owner.association(:project).target + rescue ActiveRecord::AssociationNotFoundError + nil + end + end + end + end + def self.visible_to_user(user) return where('issues.confidential IS NULL OR issues.confidential IS FALSE') if user.blank? return all if user.admin? + # Check if we are scoped to a specific project's issues + if owner_project + if owner_project.authorized_for_user?(user, Gitlab::Access::REPORTER) + # If the project is authorized for the user, they can see all issues in the project + return all + else + # else only non confidential and authored/assigned to them + return where('issues.confidential IS NULL OR issues.confidential IS FALSE + OR issues.author_id = :user_id OR issues.assignee_id = :user_id', + user_id: user.id) + end + end + where(' issues.confidential IS NULL OR issues.confidential IS FALSE diff --git a/app/models/project.rb b/app/models/project.rb index c96de0f6c72..d6191e80e62 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1210,4 +1210,44 @@ class Project < ActiveRecord::Base { key: variable.key, value: variable.value, public: false } end end + + # Checks if `user` is authorized for this project, with at least the + # `min_access_level` (if given). + # + # If you change the logic of this method, please also update `User#authorized_projects` + def authorized_for_user?(user, min_access_level = nil) + return false unless user + + return true if personal? && namespace_id == user.namespace_id + + authorized_for_user_by_group?(user, min_access_level) || + authorized_for_user_by_members?(user, min_access_level) || + authorized_for_user_by_shared_projects?(user, min_access_level) + end + + private + + def authorized_for_user_by_group?(user, min_access_level) + member = user.group_members.find_by(source_id: group) + + member && (!min_access_level || member.access_level >= min_access_level) + end + + def authorized_for_user_by_members?(user, min_access_level) + member = members.find_by(user_id: user) + + member && (!min_access_level || member.access_level >= min_access_level) + end + + def authorized_for_user_by_shared_projects?(user, min_access_level) + shared_projects = user.group_members.joins(group: :shared_projects). + where(project_group_links: { project_id: self }) + + if min_access_level + members_scope = { access_level: Gitlab::Access.values.select { |access| access >= min_access_level } } + shared_projects = shared_projects.where(members: members_scope) + end + + shared_projects.any? + end end diff --git a/app/models/user.rb b/app/models/user.rb index 975e935fa20..8dc10becd69 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -412,6 +412,8 @@ class User < ActiveRecord::Base end # Returns projects user is authorized to access. + # + # If you change the logic of this method, please also update `Project#authorized_for_user` def authorized_projects(min_access_level = nil) Project.where("projects.id IN (#{projects_union(min_access_level).to_sql})") end -- cgit v1.2.1 From 79214be727aaa0704a1be5b50aa6dd3011629bc2 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Wed, 20 Jul 2016 16:18:18 -0600 Subject: Add Discussion model to represent MR/diff discussion --- app/models/concerns/note_on_diff.rb | 25 ---------- app/models/discussion.rb | 91 +++++++++++++++++++++++++++++++++++++ app/models/note.rb | 7 +-- 3 files changed, 95 insertions(+), 28 deletions(-) create mode 100644 app/models/discussion.rb (limited to 'app/models') diff --git a/app/models/concerns/note_on_diff.rb b/app/models/concerns/note_on_diff.rb index 2785fbb21c9..4be6a2f621b 100644 --- a/app/models/concerns/note_on_diff.rb +++ b/app/models/concerns/note_on_diff.rb @@ -1,12 +1,6 @@ module NoteOnDiff extend ActiveSupport::Concern - NUMBER_OF_TRUNCATED_DIFF_LINES = 16 - - included do - delegate :blob, :highlighted_diff_lines, to: :diff_file, allow_nil: true - end - def diff_note? true end @@ -30,23 +24,4 @@ module NoteOnDiff def can_be_award_emoji? false end - - # Returns an array of at most 16 highlighted lines above a diff note - def truncated_diff_lines - prev_lines = [] - - highlighted_diff_lines.each do |line| - if line.meta? - prev_lines.clear - else - prev_lines << line - - break if for_line?(line) - - prev_lines.shift if prev_lines.length >= NUMBER_OF_TRUNCATED_DIFF_LINES - end - end - - prev_lines - end end diff --git a/app/models/discussion.rb b/app/models/discussion.rb new file mode 100644 index 00000000000..74facfd1c9c --- /dev/null +++ b/app/models/discussion.rb @@ -0,0 +1,91 @@ +class Discussion + NUMBER_OF_TRUNCATED_DIFF_LINES = 16 + + attr_reader :first_note, :notes + + delegate :created_at, + :project, + :author, + + :noteable, + :for_commit?, + :for_merge_request?, + + :line_code, + :diff_file, + :for_line?, + :active?, + + to: :first_note + + delegate :blob, :highlighted_diff_lines, to: :diff_file, allow_nil: true + + def self.for_notes(notes) + notes.group_by(&:discussion_id).values.map { |notes| new(notes) } + end + + def self.for_diff_notes(notes) + notes.group_by(&:line_code).values.map { |notes| new(notes) } + end + + def initialize(notes) + @first_note = notes.first + @notes = notes + end + + def id + first_note.discussion_id + end + + def diff_discussion? + first_note.diff_note? + end + + def legacy_diff_discussion? + notes.any?(&:legacy_diff_note?) + end + + def for_target?(target) + self.noteable == target && !diff_discussion? + end + + def expanded? + !diff_discussion? || active? + end + + def reply_attributes + data = { + noteable_type: first_note.noteable_type, + noteable_id: first_note.noteable_id, + commit_id: first_note.commit_id, + discussion_id: self.id, + } + + if diff_discussion? + data[:note_type] = first_note.type + + data.merge!(first_note.diff_attributes) + end + + data + end + + # Returns an array of at most 16 highlighted lines above a diff note + def truncated_diff_lines + prev_lines = [] + + highlighted_diff_lines.each do |line| + if line.meta? + prev_lines.clear + else + prev_lines << line + + break if for_line?(line) + + prev_lines.shift if prev_lines.length >= NUMBER_OF_TRUNCATED_DIFF_LINES + end + end + + prev_lines + end +end diff --git a/app/models/note.rb b/app/models/note.rb index 0ce10c77de9..9b0a7211b4e 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -82,11 +82,12 @@ class Note < ActiveRecord::Base end def discussions - all.group_by(&:discussion_id).values + Discussion.for_notes(all) end - def grouped_diff_notes - diff_notes.select(&:active?).sort_by(&:created_at).group_by(&:line_code) + def grouped_diff_discussions + notes = diff_notes.fresh.select(&:active?) + Discussion.for_diff_notes(notes).map { |d| [d.line_code, d] }.to_h end # Searches for notes matching the given query. -- cgit v1.2.1 From f8afe47a87ca880c95163aa7e731638a730ed14b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Thu, 21 Jul 2016 10:36:02 +0200 Subject: Make Service.external_wikis return only active external wikis MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- app/models/service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app/models') diff --git a/app/models/service.rb b/app/models/service.rb index a8e1cc2f422..40cd9b861f0 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -26,7 +26,7 @@ class Service < ActiveRecord::Base scope :visible, -> { where.not(type: ['GitlabIssueTrackerService', 'GitlabCiService']) } scope :issue_trackers, -> { where(category: 'issue_tracker') } - scope :external_wikis, -> { where(type: 'ExternalWikiService') } + scope :external_wikis, -> { where(type: 'ExternalWikiService').active } scope :active, -> { where(active: true) } scope :without_defaults, -> { where(default: false) } -- cgit v1.2.1 From 5e0669e0eb0fe466bab64f45249b16073e314c99 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Thu, 21 Jul 2016 17:02:28 +0800 Subject: Since it's too hard to use JOIN with Rails... feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5347#note_13204564 --- app/models/project.rb | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'app/models') diff --git a/app/models/project.rb b/app/models/project.rb index 9b9d497fbd8..d14d15fdc5c 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -431,13 +431,13 @@ class Project < ActiveRecord::Base # ref can't be HEAD, can only be branch/tag name or SHA def latest_successful_builds_for(ref = default_branch) - pipeline = pipelines.latest_successful_for(ref).to_sql - join_sql = "INNER JOIN (#{pipeline}) pipelines" + - " ON pipelines.id = #{Ci::Build.quoted_table_name}.commit_id" - builds.joins(join_sql).latest.with_artifacts - # TODO: Whenever we dropped support for MySQL, we could change to: - # pipeline = pipelines.latest_successful_for(ref) - # builds.where(pipeline: pipeline).latest.with_artifacts + latest_pipeline = pipelines.latest_successful_for(ref).first + + if latest_pipeline + latest_pipeline.builds.latest.with_artifacts + else + builds.none + end end def merge_base_commit(first_commit_id, second_commit_id) -- cgit v1.2.1 From a6de806498c405355380be4b80f63d134658b779 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Mon, 18 Jul 2016 16:38:36 -0300 Subject: Add service to clean up repository archive cache Replace invocation of `find` with Ruby code that matches old cached files in a better, and safe way to avoid data-integrity issues. --- app/models/repository.rb | 10 ---------- 1 file changed, 10 deletions(-) (limited to 'app/models') diff --git a/app/models/repository.rb b/app/models/repository.rb index 511df2d67c6..a6580e85498 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -11,16 +11,6 @@ class Repository attr_accessor :path_with_namespace, :project - def self.clean_old_archives - Gitlab::Metrics.measure(:clean_old_archives) do - repository_downloads_path = Gitlab.config.gitlab.repository_downloads_path - - return unless File.directory?(repository_downloads_path) - - Gitlab::Popen.popen(%W(find #{repository_downloads_path} -not -path #{repository_downloads_path} -mmin +120 -delete)) - end - end - def initialize(path_with_namespace, project) @path_with_namespace = path_with_namespace @project = project -- cgit v1.2.1 From 065a65adfe3508581664358779821b802476ee0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Thu, 21 Jul 2016 16:49:43 -0400 Subject: Update to gitlab_git 10.4.1 and take advantage of preserved Ref objects --- app/models/repository.rb | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) (limited to 'app/models') diff --git a/app/models/repository.rb b/app/models/repository.rb index a6580e85498..46a04eb80cd 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -70,7 +70,12 @@ class Repository def commit(ref = 'HEAD') return nil unless exists? - commit = Gitlab::Git::Commit.find(raw_repository, ref) + commit = + if ref.is_a?(Gitlab::Git::Commit) + ref + else + Gitlab::Git::Commit.find(raw_repository, ref) + end commit = ::Commit.new(commit, @project) if commit commit rescue Rugged::OdbError @@ -247,10 +252,10 @@ class Repository # Rugged seems to throw a `ReferenceError` when given branch_names rather # than SHA-1 hashes number_commits_behind = raw_repository. - count_commits_between(branch.target, root_ref_hash) + count_commits_between(branch.target.sha, root_ref_hash) number_commits_ahead = raw_repository. - count_commits_between(root_ref_hash, branch.target) + count_commits_between(root_ref_hash, branch.target.sha) { behind: number_commits_behind, ahead: number_commits_ahead } end @@ -674,9 +679,7 @@ class Repository end def local_branches - @local_branches ||= rugged.branches.each(:local).map do |branch| - Gitlab::Git::Branch.new(branch.name, branch.target) - end + @local_branches ||= raw_repository.local_branches end alias_method :branches, :local_branches @@ -817,7 +820,7 @@ class Repository end def revert(user, commit, base_branch, revert_tree_id = nil) - source_sha = find_branch(base_branch).target + source_sha = find_branch(base_branch).target.sha revert_tree_id ||= check_revert_content(commit, base_branch) return false unless revert_tree_id @@ -834,7 +837,7 @@ class Repository end def cherry_pick(user, commit, base_branch, cherry_pick_tree_id = nil) - source_sha = find_branch(base_branch).target + source_sha = find_branch(base_branch).target.sha cherry_pick_tree_id ||= check_cherry_pick_content(commit, base_branch) return false unless cherry_pick_tree_id @@ -855,7 +858,7 @@ class Repository end def check_revert_content(commit, base_branch) - source_sha = find_branch(base_branch).target + source_sha = find_branch(base_branch).target.sha args = [commit.id, source_sha] args << { mainline: 1 } if commit.merge_commit? @@ -869,7 +872,7 @@ class Repository end def check_cherry_pick_content(commit, base_branch) - source_sha = find_branch(base_branch).target + source_sha = find_branch(base_branch).target.sha args = [commit.id, source_sha] args << 1 if commit.merge_commit? @@ -1034,7 +1037,7 @@ class Repository end def tags_sorted_by_committed_date - tags.sort_by { |tag| commit(tag.target).committed_date } + tags.sort_by { |tag| tag.target.committed_date } end def keep_around_ref_name(sha) -- cgit v1.2.1 From 391064830d82c4b009ab7f698cba38141f449f76 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Fri, 22 Jul 2016 06:38:02 -0700 Subject: Gracefully handle case when keep-around references are corrupted or exist already We were seeing a number of error messages when attempting to create a keep-around ref: 1. Failed to create locked file `refs/keep-around/XYZ`: File exists 2. Failed to write reference `refs/keep-around/XYZ`: a reference with that name already exists. I'm not sure how these happen, but I suspect when multiple workers attempt to write the same file we may have an issue. The force parameter should help ensure the file gets created, as well as the rescues to prevent 500 Errors. Rugged/libgit2 unfortunately do not allow you to delete or re-create a reference that has been corrupted, even with the force parameter. Closes #20109 --- app/models/repository.rb | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) (limited to 'app/models') diff --git a/app/models/repository.rb b/app/models/repository.rb index 46a04eb80cd..1d3df6f9eaf 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -211,11 +211,20 @@ class Repository return if kept_around?(sha) - rugged.references.create(keep_around_ref_name(sha), sha) + # This will still fail if the file is corrupted (e.g. 0 bytes) + begin + rugged.references.create(keep_around_ref_name(sha), sha, force: true) + rescue Rugged::ReferenceError => ex + Rails.logger.error "Unable to create keep-around reference for repository #{path}: #{ex}" + end end def kept_around?(sha) - ref_exists?(keep_around_ref_name(sha)) + begin + ref_exists?(keep_around_ref_name(sha)) + rescue Rugged::ReferenceError + false + end end def tag_names -- cgit v1.2.1 From df541e510e5da0949286aa73a24ad748bc2d05c6 Mon Sep 17 00:00:00 2001 From: Ahmad Sherif Date: Fri, 22 Jul 2016 18:42:54 +0200 Subject: Load project invited groups and members eagerly in ProjectTeam#fetch_members --- app/models/project_team.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app/models') diff --git a/app/models/project_team.rb b/app/models/project_team.rb index 0b700930641..9d312a53790 100644 --- a/app/models/project_team.rb +++ b/app/models/project_team.rb @@ -173,7 +173,7 @@ class ProjectTeam invited_members = [] if project.invited_groups.any? && project.allowed_to_share_with_group? - project.project_group_links.each do |group_link| + project.project_group_links.includes(group: [:group_members]).each do |group_link| invited_group = group_link.group im = invited_group.members -- cgit v1.2.1