diff options
| author | Shinya Maeda <shinya@gitlab.com> | 2018-08-03 08:17:22 +0300 |
|---|---|---|
| committer | Shinya Maeda <shinya@gitlab.com> | 2018-08-03 08:17:22 +0300 |
| commit | 1f53cf7cf0cb53b5d69ab141fa9020356e62027e (patch) | |
| tree | fac87bd3fcf2900116bbc2777eb6f347f50922f6 | |
| parent | d867081df185b873667d9eec1184ac92efc8973e (diff) | |
| parent | 5f664759b57ef1c0fcfb7e95dfbff6c080a7a72f (diff) | |
Merge branch 'master-ce' into artifact-format-v2-with-parser
227 files changed, 1505 insertions, 656 deletions
diff --git a/GITLAB_SHELL_VERSION b/GITLAB_SHELL_VERSION index ae9a76b9249..8104cabd36f 100644 --- a/GITLAB_SHELL_VERSION +++ b/GITLAB_SHELL_VERSION @@ -1 +1 @@ -8.0.0 +8.1.0 diff --git a/Gemfile.lock b/Gemfile.lock index 77c255a7464..d8f878875f3 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -745,7 +745,7 @@ GEM retriable (3.1.1) rinku (2.0.0) rotp (2.1.2) - rouge (3.1.1) + rouge (3.2.0) rqrcode (0.7.0) chunky_png rqrcode-rails3 (0.1.7) diff --git a/app/assets/javascripts/ide/components/new_dropdown/index.vue b/app/assets/javascripts/ide/components/new_dropdown/index.vue index 440e480d596..8eddfa5e455 100644 --- a/app/assets/javascripts/ide/components/new_dropdown/index.vue +++ b/app/assets/javascripts/ide/components/new_dropdown/index.vue @@ -35,7 +35,9 @@ export default { watch: { dropdownOpen() { this.$nextTick(() => { - this.$refs.dropdownMenu.scrollIntoView(); + this.$refs.dropdownMenu.scrollIntoView({ + block: 'nearest', + }); }); }, mouseOver() { diff --git a/app/assets/javascripts/search_autocomplete.js b/app/assets/javascripts/search_autocomplete.js index 5b2e0468784..72a2c7ca101 100644 --- a/app/assets/javascripts/search_autocomplete.js +++ b/app/assets/javascripts/search_autocomplete.js @@ -137,7 +137,11 @@ export default class SearchAutocomplete { if (!term) { const contents = this.getCategoryContents(); if (contents) { - this.searchInput.data('glDropdown').filter.options.callback(contents); + const glDropdownInstance = this.searchInput.data('glDropdown'); + + if (glDropdownInstance) { + glDropdownInstance.filter.options.callback(contents); + } this.enableAutocomplete(); } return; diff --git a/app/controllers/admin/services_controller.rb b/app/controllers/admin/services_controller.rb index e70aa549140..91a36af34f3 100644 --- a/app/controllers/admin/services_controller.rb +++ b/app/controllers/admin/services_controller.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class Admin::ServicesController < Admin::ApplicationController include ServiceParams @@ -30,7 +32,7 @@ class Admin::ServicesController < Admin::ApplicationController def services_templates Service.available_services_names.map do |service_name| - service_template = service_name.concat("_service").camelize.constantize + service_template = "#{service_name}_service".camelize.constantize service_template.where(template: true).first_or_create end end diff --git a/app/controllers/projects/pipelines_controller.rb b/app/controllers/projects/pipelines_controller.rb index 1d5d1ee5d5a..b5db646bf57 100644 --- a/app/controllers/projects/pipelines_controller.rb +++ b/app/controllers/projects/pipelines_controller.rb @@ -1,7 +1,6 @@ class Projects::PipelinesController < Projects::ApplicationController before_action :whitelist_query_limiting, only: [:create, :retry] before_action :pipeline, except: [:index, :new, :create, :charts] - before_action :commit, only: [:show, :builds, :failures] before_action :authorize_read_pipeline! before_action :authorize_create_pipeline!, only: [:new, :create] before_action :authorize_update_pipeline!, only: [:retry, :cancel] @@ -168,10 +167,6 @@ class Projects::PipelinesController < Projects::ApplicationController .present(current_user: current_user) end - def commit - @commit ||= @pipeline.commit - end - def whitelist_query_limiting # Also see https://gitlab.com/gitlab-org/gitlab-ce/issues/42343 Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/42339') diff --git a/app/models/ability.rb b/app/models/ability.rb index bb600eaccba..a853106e5bd 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require_dependency 'declarative_policy' class Ability diff --git a/app/models/abuse_report.rb b/app/models/abuse_report.rb index 4cbd90c5817..1b78fd04ebb 100644 --- a/app/models/abuse_report.rb +++ b/app/models/abuse_report.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class AbuseReport < ActiveRecord::Base include CacheMarkdownField diff --git a/app/models/active_session.rb b/app/models/active_session.rb index b4a86dbb331..0d9c6a4a1f0 100644 --- a/app/models/active_session.rb +++ b/app/models/active_session.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class ActiveSession include ActiveModel::Model diff --git a/app/models/appearance.rb b/app/models/appearance.rb index b770aadef0e..bffba3e13fa 100644 --- a/app/models/appearance.rb +++ b/app/models/appearance.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class Appearance < ActiveRecord::Base include CacheableAttributes include CacheMarkdownField diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 17297769e7e..bbe7811841a 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class ApplicationSetting < ActiveRecord::Base include CacheableAttributes include CacheMarkdownField diff --git a/app/models/audit_event.rb b/app/models/audit_event.rb index 112a8778b4e..8508c88d406 100644 --- a/app/models/audit_event.rb +++ b/app/models/audit_event.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class AuditEvent < ActiveRecord::Base serialize :details, Hash # rubocop:disable Cop/ActiveRecordSerialize diff --git a/app/models/award_emoji.rb b/app/models/award_emoji.rb index 4d1a15c53aa..99c7866d636 100644 --- a/app/models/award_emoji.rb +++ b/app/models/award_emoji.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class AwardEmoji < ActiveRecord::Base DOWNVOTE_NAME = "thumbsdown".freeze UPVOTE_NAME = "thumbsup".freeze diff --git a/app/models/badge.rb b/app/models/badge.rb index 265c5d872d4..7e3b6b659e4 100644 --- a/app/models/badge.rb +++ b/app/models/badge.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class Badge < ActiveRecord::Base # This structure sets the placeholders that the urls # can have. This hash also sets which action to ask when diff --git a/app/models/blob.rb b/app/models/blob.rb index 71c974b4c09..acc64ffca67 100644 --- a/app/models/blob.rb +++ b/app/models/blob.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # Blob is a Rails-specific wrapper around Gitlab::Git::Blob objects class Blob < SimpleDelegator CACHE_TIME = 60 # Cache raw blobs referred to by a (mutable) ref for 1 minute diff --git a/app/models/board.rb b/app/models/board.rb index bb6bb753daf..a137863456c 100644 --- a/app/models/board.rb +++ b/app/models/board.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class Board < ActiveRecord::Base belongs_to :group belongs_to :project diff --git a/app/models/broadcast_message.rb b/app/models/broadcast_message.rb index 4aa236555cb..baf8adb318b 100644 --- a/app/models/broadcast_message.rb +++ b/app/models/broadcast_message.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class BroadcastMessage < ActiveRecord::Base include CacheMarkdownField include Sortable diff --git a/app/models/chat_name.rb b/app/models/chat_name.rb index fbd0f123341..03b0af53046 100644 --- a/app/models/chat_name.rb +++ b/app/models/chat_name.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class ChatName < ActiveRecord::Base LAST_USED_AT_INTERVAL = 1.hour diff --git a/app/models/chat_team.rb b/app/models/chat_team.rb index 25ecf2d5937..4e724f9adf7 100644 --- a/app/models/chat_team.rb +++ b/app/models/chat_team.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class ChatTeam < ActiveRecord::Base validates :team_id, presence: true validates :namespace, uniqueness: true diff --git a/app/models/clusters/applications/ingress.rb b/app/models/clusters/applications/ingress.rb index 27fc3b85465..4a8fd9a0b8c 100644 --- a/app/models/clusters/applications/ingress.rb +++ b/app/models/clusters/applications/ingress.rb @@ -1,15 +1,18 @@ module Clusters module Applications class Ingress < ActiveRecord::Base + VERSION = '0.23.0'.freeze + self.table_name = 'clusters_applications_ingress' include ::Clusters::Concerns::ApplicationCore include ::Clusters::Concerns::ApplicationStatus + include ::Clusters::Concerns::ApplicationVersion include ::Clusters::Concerns::ApplicationData include AfterCommitQueue default_value_for :ingress_type, :nginx - default_value_for :version, :nginx + default_value_for :version, VERSION enum ingress_type: { nginx: 1 @@ -33,6 +36,7 @@ module Clusters def install_command Gitlab::Kubernetes::Helm::InstallCommand.new( name, + version: VERSION, chart: chart, values: values ) diff --git a/app/models/clusters/applications/jupyter.rb b/app/models/clusters/applications/jupyter.rb index 975d434e1a4..72dd734246b 100644 --- a/app/models/clusters/applications/jupyter.rb +++ b/app/models/clusters/applications/jupyter.rb @@ -1,12 +1,13 @@ module Clusters module Applications class Jupyter < ActiveRecord::Base - VERSION = '0.0.1'.freeze + VERSION = 'v0.6'.freeze self.table_name = 'clusters_applications_jupyter' include ::Clusters::Concerns::ApplicationCore include ::Clusters::Concerns::ApplicationStatus + include ::Clusters::Concerns::ApplicationVersion include ::Clusters::Concerns::ApplicationData belongs_to :oauth_application, class_name: 'Doorkeeper::Application' @@ -36,6 +37,7 @@ module Clusters def install_command Gitlab::Kubernetes::Helm::InstallCommand.new( name, + version: VERSION, chart: chart, values: values, repository: repository diff --git a/app/models/clusters/applications/prometheus.rb b/app/models/clusters/applications/prometheus.rb index ea6ec4d6b03..53ac9199ae2 100644 --- a/app/models/clusters/applications/prometheus.rb +++ b/app/models/clusters/applications/prometheus.rb @@ -9,6 +9,7 @@ module Clusters include ::Clusters::Concerns::ApplicationCore include ::Clusters::Concerns::ApplicationStatus + include ::Clusters::Concerns::ApplicationVersion include ::Clusters::Concerns::ApplicationData default_value_for :version, VERSION @@ -44,8 +45,8 @@ module Clusters def install_command Gitlab::Kubernetes::Helm::InstallCommand.new( name, + version: VERSION, chart: chart, - version: version, values: values ) end diff --git a/app/models/clusters/applications/runner.rb b/app/models/clusters/applications/runner.rb index e6f795f3e0b..6d97dd1448a 100644 --- a/app/models/clusters/applications/runner.rb +++ b/app/models/clusters/applications/runner.rb @@ -1,12 +1,13 @@ module Clusters module Applications class Runner < ActiveRecord::Base - VERSION = '0.1.13'.freeze + VERSION = '0.1.31'.freeze self.table_name = 'clusters_applications_runners' include ::Clusters::Concerns::ApplicationCore include ::Clusters::Concerns::ApplicationStatus + include ::Clusters::Concerns::ApplicationVersion include ::Clusters::Concerns::ApplicationData belongs_to :runner, class_name: 'Ci::Runner', foreign_key: :runner_id @@ -29,6 +30,7 @@ module Clusters def install_command Gitlab::Kubernetes::Helm::InstallCommand.new( name, + version: VERSION, chart: chart, values: values, repository: repository diff --git a/app/models/clusters/concerns/application_version.rb b/app/models/clusters/concerns/application_version.rb new file mode 100644 index 00000000000..ccad74dc35a --- /dev/null +++ b/app/models/clusters/concerns/application_version.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module Clusters + module Concerns + module ApplicationVersion + extend ActiveSupport::Concern + + included do + state_machine :status do + after_transition any => [:installing] do |application| + application.update(version: application.class.const_get(:VERSION)) + end + end + end + end + end +end diff --git a/app/models/commit.rb b/app/models/commit.rb index 56d4c86774e..8b9f4490ffa 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -1,4 +1,6 @@ # coding: utf-8 +# frozen_string_literal: true + class Commit extend ActiveModel::Naming extend Gitlab::Cache::RequestCache @@ -339,21 +341,21 @@ class Commit end def cherry_pick_description(user) - message_body = "(cherry picked from commit #{sha})" + message_body = ["(cherry picked from commit #{sha})"] if merged_merge_request?(user) commits_in_merge_request = merged_merge_request(user).commits if commits_in_merge_request.present? - message_body << "\n" + message_body << "" commits_in_merge_request.reverse.each do |commit_in_merge| - message_body << "\n#{commit_in_merge.short_id} #{commit_in_merge.title}" + message_body << "#{commit_in_merge.short_id} #{commit_in_merge.title}" end end end - message_body + message_body.join("\n") end def cherry_pick_message(user) diff --git a/app/models/commit_range.rb b/app/models/commit_range.rb index b93c111dabc..094747ee48d 100644 --- a/app/models/commit_range.rb +++ b/app/models/commit_range.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # CommitRange makes it easier to work with commit ranges # # Examples: diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index 8b1093655b7..b65d7672973 100644 --- a/app/models/commit_status.rb +++ b/app/models/commit_status.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class CommitStatus < ActiveRecord::Base include HasStatus include Importable diff --git a/app/models/compare.rb b/app/models/compare.rb index feb4b89c781..b2d46ada831 100644 --- a/app/models/compare.rb +++ b/app/models/compare.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class Compare include Gitlab::Utils::StrongMemoize diff --git a/app/models/container_repository.rb b/app/models/container_repository.rb index d0c94d3b694..41413854d5c 100644 --- a/app/models/container_repository.rb +++ b/app/models/container_repository.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class ContainerRepository < ActiveRecord::Base belongs_to :project diff --git a/app/models/cycle_analytics.rb b/app/models/cycle_analytics.rb index b34d1382d43..d0f5b6970b1 100644 --- a/app/models/cycle_analytics.rb +++ b/app/models/cycle_analytics.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class CycleAnalytics STAGES = %i[issue plan code test review staging production].freeze diff --git a/app/models/dashboard_milestone.rb b/app/models/dashboard_milestone.rb index 86eb4ec76fc..96bc8090b81 100644 --- a/app/models/dashboard_milestone.rb +++ b/app/models/dashboard_milestone.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class DashboardMilestone < GlobalMilestone def issues_finder_params { authorized_only: true } diff --git a/app/models/deploy_key.rb b/app/models/deploy_key.rb index 89a74b7dcb1..fd5d7726fb6 100644 --- a/app/models/deploy_key.rb +++ b/app/models/deploy_key.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class DeployKey < Key include IgnorableColumn diff --git a/app/models/deploy_keys_project.rb b/app/models/deploy_keys_project.rb index 6eef12c4373..71fd02fac86 100644 --- a/app/models/deploy_keys_project.rb +++ b/app/models/deploy_keys_project.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class DeployKeysProject < ActiveRecord::Base belongs_to :project belongs_to :deploy_key, inverse_of: :deploy_keys_projects diff --git a/app/models/deploy_token.rb b/app/models/deploy_token.rb index fdbe95059e5..0b2eedf3631 100644 --- a/app/models/deploy_token.rb +++ b/app/models/deploy_token.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class DeployToken < ActiveRecord::Base include Expirable include TokenAuthenticatable @@ -28,7 +30,7 @@ class DeployToken < ActiveRecord::Base end def active? - !revoked && expires_at > Date.today + !revoked && !expired? end def scopes @@ -61,6 +63,12 @@ class DeployToken < ActiveRecord::Base private + def expired? + return false unless expires_at + + expires_at < Date.today + end + def ensure_at_least_one_scope errors.add(:base, "Scopes can't be blank") unless read_repository || read_registry end diff --git a/app/models/deployment.rb b/app/models/deployment.rb index 687246b47b2..6962b54441b 100644 --- a/app/models/deployment.rb +++ b/app/models/deployment.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class Deployment < ActiveRecord::Base include AtomicInternalId include IidRoutes diff --git a/app/models/diff_discussion.rb b/app/models/diff_discussion.rb index bd6af622bfb..93e3ebf7896 100644 --- a/app/models/diff_discussion.rb +++ b/app/models/diff_discussion.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # A discussion on merge request or commit diffs consisting of `DiffNote` notes. # # A discussion of this type can be resolvable. diff --git a/app/models/diff_note.rb b/app/models/diff_note.rb index d752d5bcdee..58d949315e0 100644 --- a/app/models/diff_note.rb +++ b/app/models/diff_note.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # A note on merge request or commit diffs # # A note of this type can be resolvable. diff --git a/app/models/directly_addressed_user.rb b/app/models/directly_addressed_user.rb index 0d519c6ac22..06df2d6c012 100644 --- a/app/models/directly_addressed_user.rb +++ b/app/models/directly_addressed_user.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class DirectlyAddressedUser class << self def reference_pattern diff --git a/app/models/discussion.rb b/app/models/discussion.rb index 35a0ef00856..dbc7b6e67be 100644 --- a/app/models/discussion.rb +++ b/app/models/discussion.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # A non-diff discussion on an issue, merge request, commit, or snippet, consisting of `DiscussionNote` notes. # # A discussion of this type can be resolvable. diff --git a/app/models/discussion_note.rb b/app/models/discussion_note.rb index e660b024083..89d86aaed66 100644 --- a/app/models/discussion_note.rb +++ b/app/models/discussion_note.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # A note in a non-diff discussion on an issue, merge request, commit, or snippet. # # A note of this type can be resolvable. diff --git a/app/models/email.rb b/app/models/email.rb index 15bdedeac33..b6a977dfa22 100644 --- a/app/models/email.rb +++ b/app/models/email.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class Email < ActiveRecord::Base include Sortable include Gitlab::SQL::Pattern diff --git a/app/models/environment.rb b/app/models/environment.rb index 4856d313318..c8d1d378ae0 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class Environment < ActiveRecord::Base # Used to generate random suffixes for the slug LETTERS = 'a'..'z' @@ -173,7 +175,7 @@ class Environment < ActiveRecord::Base # * cannot end with `-` def generate_slug # Lowercase letters and numbers only - slugified = name.to_s.downcase.gsub(/[^a-z0-9]/, '-') + slugified = +name.to_s.downcase.gsub(/[^a-z0-9]/, '-') # Must start with a letter slugified = 'env-' + slugified unless LETTERS.cover?(slugified[0]) diff --git a/app/models/epic.rb b/app/models/epic.rb index 286b855de3f..f027993376c 100644 --- a/app/models/epic.rb +++ b/app/models/epic.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # Placeholder class for model that is implemented in EE # It reserves '&' as a reference prefix, but the table does not exists in CE class Epic < ActiveRecord::Base diff --git a/app/models/event.rb b/app/models/event.rb index ac0b1c7b27c..ba28866e8e6 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class Event < ActiveRecord::Base include Sortable include IgnorableColumn diff --git a/app/models/event_collection.rb b/app/models/event_collection.rb index 8b8244314af..a4c69b11781 100644 --- a/app/models/event_collection.rb +++ b/app/models/event_collection.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # A collection of events to display in an event list. # # An EventCollection is meant to be used for displaying events to a user (e.g. diff --git a/app/models/external_issue.rb b/app/models/external_issue.rb index 282fd7edcb7..4f73beaafc5 100644 --- a/app/models/external_issue.rb +++ b/app/models/external_issue.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class ExternalIssue include Referable diff --git a/app/models/fork_network.rb b/app/models/fork_network.rb index 7f1728e8c77..1b9bf93cbbc 100644 --- a/app/models/fork_network.rb +++ b/app/models/fork_network.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class ForkNetwork < ActiveRecord::Base belongs_to :root_project, class_name: 'Project' has_many :fork_network_members diff --git a/app/models/fork_network_member.rb b/app/models/fork_network_member.rb index eb9417dc34f..36c66f21b0b 100644 --- a/app/models/fork_network_member.rb +++ b/app/models/fork_network_member.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class ForkNetworkMember < ActiveRecord::Base belongs_to :fork_network belongs_to :project diff --git a/app/models/forked_project_link.rb b/app/models/forked_project_link.rb index 8d35864eff6..0f7067238cd 100644 --- a/app/models/forked_project_link.rb +++ b/app/models/forked_project_link.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class ForkedProjectLink < ActiveRecord::Base belongs_to :forked_to_project, -> { where.not(pending_delete: true) }, class_name: 'Project' belongs_to :forked_from_project, -> { where.not(pending_delete: true) }, class_name: 'Project' diff --git a/app/models/generic_commit_status.rb b/app/models/generic_commit_status.rb index 5ac8bde44cd..3028bf21301 100644 --- a/app/models/generic_commit_status.rb +++ b/app/models/generic_commit_status.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class GenericCommitStatus < CommitStatus before_validation :set_default_values diff --git a/app/models/global_label.rb b/app/models/global_label.rb index 2a1b7564962..c5b2492bbf6 100644 --- a/app/models/global_label.rb +++ b/app/models/global_label.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class GlobalLabel attr_accessor :title, :labels alias_attribute :name, :title diff --git a/app/models/global_milestone.rb b/app/models/global_milestone.rb index dc2f6817190..2ddad9b6b0b 100644 --- a/app/models/global_milestone.rb +++ b/app/models/global_milestone.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class GlobalMilestone include Milestoneish diff --git a/app/models/gpg_key.rb b/app/models/gpg_key.rb index 44eda741679..077afffd358 100644 --- a/app/models/gpg_key.rb +++ b/app/models/gpg_key.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class GpgKey < ActiveRecord::Base KEY_PREFIX = '-----BEGIN PGP PUBLIC KEY BLOCK-----'.freeze KEY_SUFFIX = '-----END PGP PUBLIC KEY BLOCK-----'.freeze diff --git a/app/models/gpg_key_subkey.rb b/app/models/gpg_key_subkey.rb index b57922aba30..440b588bc78 100644 --- a/app/models/gpg_key_subkey.rb +++ b/app/models/gpg_key_subkey.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class GpgKeySubkey < ActiveRecord::Base include ShaAttribute diff --git a/app/models/gpg_signature.rb b/app/models/gpg_signature.rb index bf88d75246f..0816778deae 100644 --- a/app/models/gpg_signature.rb +++ b/app/models/gpg_signature.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class GpgSignature < ActiveRecord::Base include ShaAttribute diff --git a/app/models/group.rb b/app/models/group.rb index ddebaff50b0..cd548fc0061 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'carrierwave/orm/activerecord' class Group < Namespace diff --git a/app/models/group_custom_attribute.rb b/app/models/group_custom_attribute.rb index 8157d602d67..22f14885657 100644 --- a/app/models/group_custom_attribute.rb +++ b/app/models/group_custom_attribute.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class GroupCustomAttribute < ActiveRecord::Base belongs_to :group diff --git a/app/models/group_label.rb b/app/models/group_label.rb index 92c83b54861..ff14529c6e6 100644 --- a/app/models/group_label.rb +++ b/app/models/group_label.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class GroupLabel < Label belongs_to :group diff --git a/app/models/group_milestone.rb b/app/models/group_milestone.rb index 98135ee3c8b..d6ab32ea7c8 100644 --- a/app/models/group_milestone.rb +++ b/app/models/group_milestone.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class GroupMilestone < GlobalMilestone attr_accessor :group diff --git a/app/models/guest.rb b/app/models/guest.rb index df287c277a7..9c8097e1ac8 100644 --- a/app/models/guest.rb +++ b/app/models/guest.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class Guest class << self def can?(action, subject = :global) diff --git a/app/models/identity.rb b/app/models/identity.rb index 3fd0c5e751d..f5a13dbd6f2 100644 --- a/app/models/identity.rb +++ b/app/models/identity.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class Identity < ActiveRecord::Base def self.uniqueness_scope :provider diff --git a/app/models/import_export_upload.rb b/app/models/import_export_upload.rb index 60d53d6c2c8..f0cc5aafcd4 100644 --- a/app/models/import_export_upload.rb +++ b/app/models/import_export_upload.rb @@ -1,9 +1,12 @@ +# frozen_string_literal: true + class ImportExportUpload < ActiveRecord::Base include WithUploads include ObjectStorage::BackgroundMove belongs_to :project + # These hold the project Import/Export archives (.tar.gz files) mount_uploader :import_file, ImportExportUploader mount_uploader :export_file, ImportExportUploader diff --git a/app/models/individual_note_discussion.rb b/app/models/individual_note_discussion.rb index 6be8ca45739..07ee7470ea2 100644 --- a/app/models/individual_note_discussion.rb +++ b/app/models/individual_note_discussion.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # A discussion to wrap a single `Note` note on the root of an issue, merge request, # commit, or snippet, that is not displayed as a discussion. # diff --git a/app/models/instance_configuration.rb b/app/models/instance_configuration.rb index b30b707e5fe..7d8ce0bbd05 100644 --- a/app/models/instance_configuration.rb +++ b/app/models/instance_configuration.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'resolv' class InstanceConfiguration diff --git a/app/models/internal_id.rb b/app/models/internal_id.rb index e5d0f94073c..4eb211eff61 100644 --- a/app/models/internal_id.rb +++ b/app/models/internal_id.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # An InternalId is a strictly monotone sequence of integers # generated for a given scope and usage. # diff --git a/app/models/issue.rb b/app/models/issue.rb index e4ed06f9a69..0d135f54038 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'carrierwave/orm/activerecord' class Issue < ActiveRecord::Base diff --git a/app/models/issue_assignee.rb b/app/models/issue_assignee.rb index 326b9eb7ad5..400c0256945 100644 --- a/app/models/issue_assignee.rb +++ b/app/models/issue_assignee.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class IssueAssignee < ActiveRecord::Base belongs_to :issue belongs_to :assignee, class_name: "User", foreign_key: :user_id diff --git a/app/models/issue_collection.rb b/app/models/issue_collection.rb index 49f011c113f..05607fc3a08 100644 --- a/app/models/issue_collection.rb +++ b/app/models/issue_collection.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # IssueCollection can be used to reduce a list of issues down to a subset. # # IssueCollection is not meant to be some sort of Enumerable, instead it's meant diff --git a/app/models/key.rb b/app/models/key.rb index ae5769c0627..3bb0d2f6f9c 100644 --- a/app/models/key.rb +++ b/app/models/key.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'digest/md5' class Key < ActiveRecord::Base diff --git a/app/models/label.rb b/app/models/label.rb index 7b08547fa6e..96c1515b41a 100644 --- a/app/models/label.rb +++ b/app/models/label.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class Label < ActiveRecord::Base include CacheMarkdownField include Referable diff --git a/app/models/label_link.rb b/app/models/label_link.rb index d68e1f54317..779657b25d5 100644 --- a/app/models/label_link.rb +++ b/app/models/label_link.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class LabelLink < ActiveRecord::Base include Importable diff --git a/app/models/label_priority.rb b/app/models/label_priority.rb index 5b85e0b6533..8ed8bb7577f 100644 --- a/app/models/label_priority.rb +++ b/app/models/label_priority.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class LabelPriority < ActiveRecord::Base belongs_to :project belongs_to :label diff --git a/app/models/legacy_diff_discussion.rb b/app/models/legacy_diff_discussion.rb index 80fc6304fd4..7d78c580fa2 100644 --- a/app/models/legacy_diff_discussion.rb +++ b/app/models/legacy_diff_discussion.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # A discussion on merge request or commit diffs consisting of `LegacyDiffNote` notes. # # All new diff discussions are of the type `DiffDiscussion`, but any diff discussions created diff --git a/app/models/legacy_diff_note.rb b/app/models/legacy_diff_note.rb index d90cafd14b4..20f9b18e4ca 100644 --- a/app/models/legacy_diff_note.rb +++ b/app/models/legacy_diff_note.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # A note on merge request or commit diffs, using the legacy implementation. # # All new diff notes are of the type `DiffNote`, but any diff notes created diff --git a/app/models/lfs_file_lock.rb b/app/models/lfs_file_lock.rb index 50bb6ca382d..431d37e12e9 100644 --- a/app/models/lfs_file_lock.rb +++ b/app/models/lfs_file_lock.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class LfsFileLock < ActiveRecord::Base belongs_to :project belongs_to :user diff --git a/app/models/lfs_object.rb b/app/models/lfs_object.rb index 84487031ee5..2a1a4ef48b7 100644 --- a/app/models/lfs_object.rb +++ b/app/models/lfs_object.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class LfsObject < ActiveRecord::Base include AfterCommitQueue include ObjectStorage::BackgroundMove diff --git a/app/models/lfs_objects_project.rb b/app/models/lfs_objects_project.rb index b0625c52b62..353602800d7 100644 --- a/app/models/lfs_objects_project.rb +++ b/app/models/lfs_objects_project.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class LfsObjectsProject < ActiveRecord::Base belongs_to :project belongs_to :lfs_object diff --git a/app/models/list.rb b/app/models/list.rb index 4edcfa78835..eabe3ffccbb 100644 --- a/app/models/list.rb +++ b/app/models/list.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class List < ActiveRecord::Base belongs_to :board belongs_to :label diff --git a/app/models/member.rb b/app/models/member.rb index 00a13a279a9..05c0bc8cb97 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class Member < ActiveRecord::Base include AfterCommitQueue include Sortable diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index a99b197af66..9f8ebd9b4ca 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class MergeRequest < ActiveRecord::Base include AtomicInternalId include IidRoutes diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index a073bbfad20..d9393b4e545 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class MergeRequestDiff < ActiveRecord::Base include Sortable include Importable @@ -249,15 +251,13 @@ class MergeRequestDiff < ActiveRecord::Base end def load_diffs(options) - raw = merge_request_diff_files.map(&:to_hash) + collection = merge_request_diff_files if paths = options[:paths] - raw = raw.select do |diff| - paths.include?(diff[:old_path]) || paths.include?(diff[:new_path]) - end + collection = collection.where('old_path IN (?) OR new_path IN (?)', paths, paths) end - Gitlab::Git::DiffCollection.new(raw, options) + Gitlab::Git::DiffCollection.new(collection.map(&:to_hash), options) end def load_commits diff --git a/app/models/merge_request_diff_commit.rb b/app/models/merge_request_diff_commit.rb index 1c2e57bb01f..4ad3690512d 100644 --- a/app/models/merge_request_diff_commit.rb +++ b/app/models/merge_request_diff_commit.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class MergeRequestDiffCommit < ActiveRecord::Base include ShaAttribute diff --git a/app/models/merge_request_diff_file.rb b/app/models/merge_request_diff_file.rb index cd8ba6b904d..a9f110bec5c 100644 --- a/app/models/merge_request_diff_file.rb +++ b/app/models/merge_request_diff_file.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class MergeRequestDiffFile < ActiveRecord::Base include Gitlab::EncodingHelper include DiffFile diff --git a/app/models/merge_requests_closing_issues.rb b/app/models/merge_requests_closing_issues.rb index 7f7c114803d..242b65bedc0 100644 --- a/app/models/merge_requests_closing_issues.rb +++ b/app/models/merge_requests_closing_issues.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class MergeRequestsClosingIssues < ActiveRecord::Base belongs_to :merge_request belongs_to :issue diff --git a/app/models/milestone.rb b/app/models/milestone.rb index d0f7f9c3593..f2b2d291da9 100644 --- a/app/models/milestone.rb +++ b/app/models/milestone.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class Milestone < ActiveRecord::Base # Represents a "No Milestone" state used for filtering Issues and Merge # Requests that have no milestone assigned. diff --git a/app/models/namespace.rb b/app/models/namespace.rb index de06e080a7d..b974309aeb6 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class Namespace < ActiveRecord::Base include CacheMarkdownField include Sortable diff --git a/app/models/note.rb b/app/models/note.rb index 87cf7c7b2fa..969d34ae09a 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # A note on the root of an issue, merge request, commit, or snippet. # # A note of this type is never resolvable. diff --git a/app/models/note_diff_file.rb b/app/models/note_diff_file.rb index e688018a6d9..27aef7adc48 100644 --- a/app/models/note_diff_file.rb +++ b/app/models/note_diff_file.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class NoteDiffFile < ActiveRecord::Base include DiffFile diff --git a/app/models/notification_reason.rb b/app/models/notification_reason.rb index c3965565022..0a13487574f 100644 --- a/app/models/notification_reason.rb +++ b/app/models/notification_reason.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # Holds reasons for a notification to have been sent as well as a priority list to select which reason to use # above the rest class NotificationReason diff --git a/app/models/notification_recipient.rb b/app/models/notification_recipient.rb index 1a03dd9df56..9f16eefe074 100644 --- a/app/models/notification_recipient.rb +++ b/app/models/notification_recipient.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class NotificationRecipient include Gitlab::Utils::StrongMemoize diff --git a/app/models/notification_setting.rb b/app/models/notification_setting.rb index 1933c46ee44..1df3a51a7fc 100644 --- a/app/models/notification_setting.rb +++ b/app/models/notification_setting.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class NotificationSetting < ActiveRecord::Base include IgnorableColumn diff --git a/app/models/oauth_access_grant.rb b/app/models/oauth_access_grant.rb index 3a997406565..d5a8a1a25b6 100644 --- a/app/models/oauth_access_grant.rb +++ b/app/models/oauth_access_grant.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class OauthAccessGrant < Doorkeeper::AccessGrant belongs_to :resource_owner, class_name: 'User' belongs_to :application, class_name: 'Doorkeeper::Application' diff --git a/app/models/oauth_access_token.rb b/app/models/oauth_access_token.rb index e8595b13d6d..0aa920fa828 100644 --- a/app/models/oauth_access_token.rb +++ b/app/models/oauth_access_token.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class OauthAccessToken < Doorkeeper::AccessToken belongs_to :resource_owner, class_name: 'User' belongs_to :application, class_name: 'Doorkeeper::Application' diff --git a/app/models/out_of_context_discussion.rb b/app/models/out_of_context_discussion.rb index 4227c40b69a..4de717e2c51 100644 --- a/app/models/out_of_context_discussion.rb +++ b/app/models/out_of_context_discussion.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # When notes on a commit are displayed in the context of a merge request that # contains that commit, they are displayed as if they were a discussion. # diff --git a/app/models/pages_domain.rb b/app/models/pages_domain.rb index bfea64c3759..7739a3894d3 100644 --- a/app/models/pages_domain.rb +++ b/app/models/pages_domain.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class PagesDomain < ActiveRecord::Base VERIFICATION_KEY = 'gitlab-pages-verification-code'.freeze VERIFICATION_THRESHOLD = 3.days.freeze diff --git a/app/models/personal_access_token.rb b/app/models/personal_access_token.rb index 063dc521324..207146479c0 100644 --- a/app/models/personal_access_token.rb +++ b/app/models/personal_access_token.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class PersonalAccessToken < ActiveRecord::Base include Expirable include TokenAuthenticatable diff --git a/app/models/personal_snippet.rb b/app/models/personal_snippet.rb index 355624fd552..1b5be8698b1 100644 --- a/app/models/personal_snippet.rb +++ b/app/models/personal_snippet.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class PersonalSnippet < Snippet include WithUploads end diff --git a/app/models/project.rb b/app/models/project.rb index af32afc08e2..16d63639141 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'carrierwave/orm/activerecord' class Project < ActiveRecord::Base diff --git a/app/models/project_authorization.rb b/app/models/project_authorization.rb index 73302207e6b..746bb4584c9 100644 --- a/app/models/project_authorization.rb +++ b/app/models/project_authorization.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class ProjectAuthorization < ActiveRecord::Base belongs_to :user belongs_to :project diff --git a/app/models/project_auto_devops.rb b/app/models/project_auto_devops.rb index faa831b1949..155400d1a43 100644 --- a/app/models/project_auto_devops.rb +++ b/app/models/project_auto_devops.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class ProjectAutoDevops < ActiveRecord::Base belongs_to :project diff --git a/app/models/project_ci_cd_setting.rb b/app/models/project_ci_cd_setting.rb index 588cced5781..1dad235cc2b 100644 --- a/app/models/project_ci_cd_setting.rb +++ b/app/models/project_ci_cd_setting.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class ProjectCiCdSetting < ActiveRecord::Base belongs_to :project, inverse_of: :ci_cd_settings diff --git a/app/models/project_custom_attribute.rb b/app/models/project_custom_attribute.rb index 3f1a7b86a82..4e767cb3b26 100644 --- a/app/models/project_custom_attribute.rb +++ b/app/models/project_custom_attribute.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class ProjectCustomAttribute < ActiveRecord::Base belongs_to :project diff --git a/app/models/project_deploy_token.rb b/app/models/project_deploy_token.rb index ab4482f0c0b..719c492a1ff 100644 --- a/app/models/project_deploy_token.rb +++ b/app/models/project_deploy_token.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class ProjectDeployToken < ActiveRecord::Base belongs_to :project belongs_to :deploy_token, inverse_of: :project_deploy_tokens diff --git a/app/models/project_feature.rb b/app/models/project_feature.rb index 9c768b13f78..d74cb2506ba 100644 --- a/app/models/project_feature.rb +++ b/app/models/project_feature.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class ProjectFeature < ActiveRecord::Base # == Project features permissions # diff --git a/app/models/project_group_link.rb b/app/models/project_group_link.rb index cf8fc41e870..bc3759142ae 100644 --- a/app/models/project_group_link.rb +++ b/app/models/project_group_link.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class ProjectGroupLink < ActiveRecord::Base include Expirable diff --git a/app/models/project_import_data.rb b/app/models/project_import_data.rb index 1d7089ccfc7..2c3080c6d8d 100644 --- a/app/models/project_import_data.rb +++ b/app/models/project_import_data.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'carrierwave/orm/activerecord' class ProjectImportData < ActiveRecord::Base diff --git a/app/models/project_import_state.rb b/app/models/project_import_state.rb index 1605317ae14..89ed09af96a 100644 --- a/app/models/project_import_state.rb +++ b/app/models/project_import_state.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class ProjectImportState < ActiveRecord::Base include AfterCommitQueue diff --git a/app/models/project_label.rb b/app/models/project_label.rb index 313815e5869..d0b16cc98b4 100644 --- a/app/models/project_label.rb +++ b/app/models/project_label.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class ProjectLabel < Label MAX_NUMBER_OF_PRIORITIES = 1 diff --git a/app/models/project_snippet.rb b/app/models/project_snippet.rb index 25b5d777641..b3585c4cf4c 100644 --- a/app/models/project_snippet.rb +++ b/app/models/project_snippet.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class ProjectSnippet < Snippet belongs_to :project belongs_to :author, class_name: "User" diff --git a/app/models/project_statistics.rb b/app/models/project_statistics.rb index ef4a26f0e7f..781a197d56f 100644 --- a/app/models/project_statistics.rb +++ b/app/models/project_statistics.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class ProjectStatistics < ActiveRecord::Base belongs_to :project belongs_to :namespace diff --git a/app/models/project_team.rb b/app/models/project_team.rb index c7d0f49d837..33bc6a561f9 100644 --- a/app/models/project_team.rb +++ b/app/models/project_team.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class ProjectTeam include BulkMemberAccessLoad diff --git a/app/models/protectable_dropdown.rb b/app/models/protectable_dropdown.rb index c96edc5a259..25e70ab406c 100644 --- a/app/models/protectable_dropdown.rb +++ b/app/models/protectable_dropdown.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class ProtectableDropdown REF_TYPES = %i[branches tags].freeze diff --git a/app/models/protected_branch.rb b/app/models/protected_branch.rb index dff99cfca35..6c1073265a1 100644 --- a/app/models/protected_branch.rb +++ b/app/models/protected_branch.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class ProtectedBranch < ActiveRecord::Base include Gitlab::ShellAdapter include ProtectedRef diff --git a/app/models/protected_ref_matcher.rb b/app/models/protected_ref_matcher.rb index d970f2b01fc..bfa9180ac93 100644 --- a/app/models/protected_ref_matcher.rb +++ b/app/models/protected_ref_matcher.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class ProtectedRefMatcher def initialize(protected_ref) @protected_ref = protected_ref diff --git a/app/models/protected_tag.rb b/app/models/protected_tag.rb index 42a9bcf7723..a36f0d36262 100644 --- a/app/models/protected_tag.rb +++ b/app/models/protected_tag.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class ProtectedTag < ActiveRecord::Base include Gitlab::ShellAdapter include ProtectedRef diff --git a/app/models/push_event.rb b/app/models/push_event.rb index 90c085c888e..9c0267c3140 100644 --- a/app/models/push_event.rb +++ b/app/models/push_event.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class PushEvent < Event # This validation exists so we can't accidentally use PushEvent with a # different "action" value. diff --git a/app/models/push_event_payload.rb b/app/models/push_event_payload.rb index 6cdb1cd4fe9..c7769edf055 100644 --- a/app/models/push_event_payload.rb +++ b/app/models/push_event_payload.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class PushEventPayload < ActiveRecord::Base include ShaAttribute diff --git a/app/models/readme_blob.rb b/app/models/readme_blob.rb index 1863a08f1de..7b49fa632f6 100644 --- a/app/models/readme_blob.rb +++ b/app/models/readme_blob.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class ReadmeBlob < SimpleDelegator attr_reader :repository diff --git a/app/models/redirect_route.rb b/app/models/redirect_route.rb index 31de204d824..c6bd4bb6dfa 100644 --- a/app/models/redirect_route.rb +++ b/app/models/redirect_route.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class RedirectRoute < ActiveRecord::Base belongs_to :source, polymorphic: true # rubocop:disable Cop/PolymorphicAssociations diff --git a/app/models/release.rb b/app/models/release.rb index c936899799e..cba80ad30ca 100644 --- a/app/models/release.rb +++ b/app/models/release.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class Release < ActiveRecord::Base include CacheMarkdownField diff --git a/app/models/remote_mirror.rb b/app/models/remote_mirror.rb index 6172bb38881..833faf3bc82 100644 --- a/app/models/remote_mirror.rb +++ b/app/models/remote_mirror.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class RemoteMirror < ActiveRecord::Base include AfterCommitQueue diff --git a/app/models/repository.rb b/app/models/repository.rb index 9a6281bc1f7..69f375dc6f3 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'securerandom' class Repository diff --git a/app/models/route.rb b/app/models/route.rb index 2d609920051..4b23dfa5778 100644 --- a/app/models/route.rb +++ b/app/models/route.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class Route < ActiveRecord::Base include CaseSensitivity diff --git a/app/models/security_event.rb b/app/models/security_event.rb index d131c11cb6c..3fe4cc99c9b 100644 --- a/app/models/security_event.rb +++ b/app/models/security_event.rb @@ -1,2 +1,4 @@ +# frozen_string_literal: true + class SecurityEvent < AuditEvent end diff --git a/app/models/sent_notification.rb b/app/models/sent_notification.rb index 3da7c301d28..e65b3df0fb6 100644 --- a/app/models/sent_notification.rb +++ b/app/models/sent_notification.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class SentNotification < ActiveRecord::Base serialize :position, Gitlab::Diff::Position # rubocop:disable Cop/ActiveRecordSerialize diff --git a/app/models/service.rb b/app/models/service.rb index cbfe0c6eedd..140058771ee 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # To add new service you should build a class inherited from Service # and implement a set of methods class Service < ActiveRecord::Base diff --git a/app/models/snippet.rb b/app/models/snippet.rb index 390bdbf838a..5b394e3fa79 100644 --- a/app/models/snippet.rb +++ b/app/models/snippet.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class Snippet < ActiveRecord::Base include Gitlab::VisibilityLevel include CacheMarkdownField diff --git a/app/models/snippet_blob.rb b/app/models/snippet_blob.rb index fa5fa151607..cf1ab089829 100644 --- a/app/models/snippet_blob.rb +++ b/app/models/snippet_blob.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class SnippetBlob include BlobLike diff --git a/app/models/spam_log.rb b/app/models/spam_log.rb index 56a115d1db4..ef3f974b959 100644 --- a/app/models/spam_log.rb +++ b/app/models/spam_log.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class SpamLog < ActiveRecord::Base belongs_to :user diff --git a/app/models/subscription.rb b/app/models/subscription.rb index 2f0c9640744..0f6ee0ddf7e 100644 --- a/app/models/subscription.rb +++ b/app/models/subscription.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class Subscription < ActiveRecord::Base belongs_to :user belongs_to :project diff --git a/app/models/system_note_metadata.rb b/app/models/system_note_metadata.rb index 1c2161accc4..c5c77bc8333 100644 --- a/app/models/system_note_metadata.rb +++ b/app/models/system_note_metadata.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class SystemNoteMetadata < ActiveRecord::Base # These notes's action text might contain a reference that is external. # We should always force a deep validation upon references that are found diff --git a/app/models/term_agreement.rb b/app/models/term_agreement.rb index c317bd0c90b..9b3c8ac68bd 100644 --- a/app/models/term_agreement.rb +++ b/app/models/term_agreement.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class TermAgreement < ActiveRecord::Base belongs_to :term, class_name: 'ApplicationSetting::Term' belongs_to :user diff --git a/app/models/timelog.rb b/app/models/timelog.rb index 659146f43e4..e04c644a53a 100644 --- a/app/models/timelog.rb +++ b/app/models/timelog.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class Timelog < ActiveRecord::Base validates :time_spent, :user, presence: true validate :issuable_id_is_present diff --git a/app/models/todo.rb b/app/models/todo.rb index a2ab405fdbe..5f5c2f9073d 100644 --- a/app/models/todo.rb +++ b/app/models/todo.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class Todo < ActiveRecord::Base include Sortable diff --git a/app/models/tree.rb b/app/models/tree.rb index 4c1856b67a8..3641c33254c 100644 --- a/app/models/tree.rb +++ b/app/models/tree.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class Tree include Gitlab::MarkupHelper diff --git a/app/models/trending_project.rb b/app/models/trending_project.rb index 27e3732da17..7b22e8cb760 100644 --- a/app/models/trending_project.rb +++ b/app/models/trending_project.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class TrendingProject < ActiveRecord::Base belongs_to :project diff --git a/app/models/u2f_registration.rb b/app/models/u2f_registration.rb index 808acec098f..37598173fd1 100644 --- a/app/models/u2f_registration.rb +++ b/app/models/u2f_registration.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # Registration information for U2F (universal 2nd factor) devices, like Yubikeys class U2fRegistration < ActiveRecord::Base diff --git a/app/models/upload.rb b/app/models/upload.rb index cf71a7b76fc..23bc9ca42fc 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class Upload < ActiveRecord::Base # Upper limit for foreground checksum processing CHECKSUM_THRESHOLD = 100.megabytes diff --git a/app/models/user.rb b/app/models/user.rb index 31b49b9bf2a..37f2e8b680e 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'carrierwave/orm/activerecord' class User < ActiveRecord::Base @@ -128,7 +130,7 @@ class User < ActiveRecord::Base has_many :builds, dependent: :nullify, class_name: 'Ci::Build' # rubocop:disable Cop/ActiveRecordDependent has_many :pipelines, dependent: :nullify, class_name: 'Ci::Pipeline' # rubocop:disable Cop/ActiveRecordDependent has_many :todos - has_many :notification_settings, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent + has_many :notification_settings has_many :award_emoji, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent has_many :triggers, dependent: :destroy, class_name: 'Ci::Trigger', foreign_key: :owner_id # rubocop:disable Cop/ActiveRecordDependent diff --git a/app/models/user_agent_detail.rb b/app/models/user_agent_detail.rb index 2d05fdd3e54..e2b2e7f1df9 100644 --- a/app/models/user_agent_detail.rb +++ b/app/models/user_agent_detail.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class UserAgentDetail < ActiveRecord::Base belongs_to :subject, polymorphic: true # rubocop:disable Cop/PolymorphicAssociations diff --git a/app/models/user_callout.rb b/app/models/user_callout.rb index 9d461c6750a..97e955ace36 100644 --- a/app/models/user_callout.rb +++ b/app/models/user_callout.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class UserCallout < ActiveRecord::Base belongs_to :user diff --git a/app/models/user_custom_attribute.rb b/app/models/user_custom_attribute.rb index eff25b31f9b..e0ffe8ebbfd 100644 --- a/app/models/user_custom_attribute.rb +++ b/app/models/user_custom_attribute.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class UserCustomAttribute < ActiveRecord::Base belongs_to :user diff --git a/app/models/user_interacted_project.rb b/app/models/user_interacted_project.rb index dd55a6acb79..ae6778e49be 100644 --- a/app/models/user_interacted_project.rb +++ b/app/models/user_interacted_project.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class UserInteractedProject < ActiveRecord::Base belongs_to :user belongs_to :project diff --git a/app/models/user_synced_attributes_metadata.rb b/app/models/user_synced_attributes_metadata.rb index 688432a9d67..7115262942d 100644 --- a/app/models/user_synced_attributes_metadata.rb +++ b/app/models/user_synced_attributes_metadata.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class UserSyncedAttributesMetadata < ActiveRecord::Base belongs_to :user diff --git a/app/models/users_star_project.rb b/app/models/users_star_project.rb index 0dfe597317e..bdaf58ae1c1 100644 --- a/app/models/users_star_project.rb +++ b/app/models/users_star_project.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class UsersStarProject < ActiveRecord::Base belongs_to :project, counter_cache: :star_count, touch: true belongs_to :user diff --git a/app/models/wiki_directory.rb b/app/models/wiki_directory.rb index 9340fc2dbbe..712ba79bbd2 100644 --- a/app/models/wiki_directory.rb +++ b/app/models/wiki_directory.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class WikiDirectory include ActiveModel::Validations diff --git a/app/models/wiki_page.rb b/app/models/wiki_page.rb index 55243136140..33790afc35e 100644 --- a/app/models/wiki_page.rb +++ b/app/models/wiki_page.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # rubocop:disable Rails/ActiveRecordAliases class WikiPage PageChangedError = Class.new(StandardError) diff --git a/app/services/projects/gitlab_projects_import_service.rb b/app/services/projects/gitlab_projects_import_service.rb index 615dccc4685..044afa1d5e1 100644 --- a/app/services/projects/gitlab_projects_import_service.rb +++ b/app/services/projects/gitlab_projects_import_service.rb @@ -15,7 +15,7 @@ module Projects end def execute - prepare_template_environment(template_file&.path) + prepare_template_environment(template_file) prepare_import_params @@ -61,7 +61,6 @@ module Projects if template_file params[:import_type] = 'gitlab_project' - params[:import_source] = import_upload_path end params[:import_data] = { data: data } if data.present? diff --git a/app/services/users/activity_service.rb b/app/services/users/activity_service.rb index 822df6c646a..db03ba8756f 100644 --- a/app/services/users/activity_service.rb +++ b/app/services/users/activity_service.rb @@ -11,6 +11,7 @@ module Users author.user end + @user = nil unless @user.is_a?(User) @activity = activity end diff --git a/app/uploaders/import_export_uploader.rb b/app/uploaders/import_export_uploader.rb index 7c45ba5ca95..716922bc017 100644 --- a/app/uploaders/import_export_uploader.rb +++ b/app/uploaders/import_export_uploader.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class ImportExportUploader < AttachmentUploader - EXTENSION_WHITELIST = %w[tar.gz].freeze + EXTENSION_WHITELIST = %w[tar.gz gz].freeze def extension_whitelist EXTENSION_WHITELIST diff --git a/app/views/projects/jobs/show.html.haml b/app/views/projects/jobs/show.html.haml index 1f33bb3a129..078f40c4477 100644 --- a/app/views/projects/jobs/show.html.haml +++ b/app/views/projects/jobs/show.html.haml @@ -22,7 +22,7 @@ %br Go to - = link_to project_runners_path(@build.project) do + = link_to project_runners_path(@build.project, anchor: 'js-runners-settings') do Runners page - if @build.starts_environment? diff --git a/app/views/projects/pipelines/_info.html.haml b/app/views/projects/pipelines/_info.html.haml index bc247460d28..78e3b209686 100644 --- a/app/views/projects/pipelines/_info.html.haml +++ b/app/views/projects/pipelines/_info.html.haml @@ -1,37 +1,34 @@ -#js-pipeline-header-vue.pipeline-header-container +.commit-box + %h3.commit-title + = markdown(commit.title, pipeline: :single_line) + - if commit.description.present? + .commit-description< + = preserve(markdown(commit.description, pipeline: :single_line)) -- if @commit.present? - .commit-box - %h3.commit-title - = markdown(@commit.title, pipeline: :single_line) - - if @commit.description.present? - .commit-description< - = preserve(markdown(@commit.description, pipeline: :single_line)) +.info-well + - if commit.status + .well-segment.pipeline-info + .icon-container + = icon('clock-o') + = pluralize @pipeline.total_size, "job" + - if @pipeline.ref + from + = link_to @pipeline.ref, project_ref_path(@project, @pipeline.ref), class: "ref-name" + - if @pipeline.duration + in + = time_interval_in_words(@pipeline.duration) + - if @pipeline.queued_duration + = "(queued for #{time_interval_in_words(@pipeline.queued_duration)})" - .info-well - - if @commit.status - .well-segment.pipeline-info - .icon-container - = icon('clock-o') - = pluralize @pipeline.total_size, "job" - - if @pipeline.ref - from - = link_to @pipeline.ref, project_ref_path(@project, @pipeline.ref), class: "ref-name" - - if @pipeline.duration - in - = time_interval_in_words(@pipeline.duration) - - if @pipeline.queued_duration - = "(queued for #{time_interval_in_words(@pipeline.queued_duration)})" + .well-segment.branch-info + .icon-container.commit-icon + = custom_icon("icon_commit") + = link_to commit.short_id, project_commit_path(@project, @pipeline.sha), class: "commit-sha js-details-short" + = link_to("#", class: "js-details-expand d-none d-sm-none d-md-inline") do + %span.text-expander + = sprite_icon('ellipsis_h', size: 12) + %span.js-details-content.hide + = link_to @pipeline.sha, project_commit_path(@project, @pipeline.sha), class: "commit-sha commit-hash-full" + = clipboard_button(text: @pipeline.sha, title: "Copy commit SHA to clipboard") - .well-segment.branch-info - .icon-container.commit-icon - = custom_icon("icon_commit") - = link_to @commit.short_id, project_commit_path(@project, @pipeline.sha), class: "commit-sha js-details-short" - = link_to("#", class: "js-details-expand d-none d-sm-none d-md-inline") do - %span.text-expander - = sprite_icon('ellipsis_h', size: 12) - %span.js-details-content.hide - = link_to @pipeline.sha, project_commit_path(@project, @pipeline.sha), class: "commit-sha commit-hash-full" - = clipboard_button(text: @pipeline.sha, title: "Copy commit SHA to clipboard") - - = render_if_exists "projects/pipelines/info_extension", pipeline: @pipeline + = render_if_exists "projects/pipelines/info_extension", pipeline: @pipeline diff --git a/app/views/projects/pipelines/show.html.haml b/app/views/projects/pipelines/show.html.haml index a7d7c923957..ff0ed3ed30d 100644 --- a/app/views/projects/pipelines/show.html.haml +++ b/app/views/projects/pipelines/show.html.haml @@ -4,8 +4,10 @@ - page_title "Pipeline" .js-pipeline-container{ class: container_class, data: { controller_action: "#{controller.action_name}" } } - - if @commit - = render "projects/pipelines/info" + #js-pipeline-header-vue.pipeline-header-container + + - if @pipeline.commit.present? + = render "projects/pipelines/info", commit: @pipeline.commit = render "projects/pipelines/with_tabs", pipeline: @pipeline diff --git a/app/views/projects/settings/ci_cd/show.html.haml b/app/views/projects/settings/ci_cd/show.html.haml index be22bbd7a9b..e9f5e62f5e7 100644 --- a/app/views/projects/settings/ci_cd/show.html.haml +++ b/app/views/projects/settings/ci_cd/show.html.haml @@ -28,7 +28,7 @@ .settings-content = render 'autodevops_form' -%section.qa-runners-settings.settings.no-animate{ class: ('expanded' if expanded) } +%section.qa-runners-settings.settings.no-animate#js-runners-settings{ class: ('expanded' if expanded) } .settings-header %h4 = _("Runners") diff --git a/app/workers/create_gpg_signature_worker.rb b/app/workers/create_gpg_signature_worker.rb index 91833941070..a1aeeb7c4fc 100644 --- a/app/workers/create_gpg_signature_worker.rb +++ b/app/workers/create_gpg_signature_worker.rb @@ -4,6 +4,10 @@ class CreateGpgSignatureWorker include ApplicationWorker def perform(commit_shas, project_id) + # Older versions of GitPushService may push a single commit ID on the stack. + # We need this to be backwards compatible. + commit_shas = Array(commit_shas) + return if commit_shas.empty? project = Project.find_by(id: project_id) diff --git a/changelogs/unreleased/44824-remove-ghost-notification-settings-for-group-and-project.yml b/changelogs/unreleased/44824-remove-ghost-notification-settings-for-group-and-project.yml new file mode 100644 index 00000000000..ddc878ee710 --- /dev/null +++ b/changelogs/unreleased/44824-remove-ghost-notification-settings-for-group-and-project.yml @@ -0,0 +1,5 @@ +--- +title: Adds foreign key to notification_settings.user_id +merge_request: 20567 +author: Jacopo Beschi @jacopo-beschi +type: added diff --git a/changelogs/unreleased/48246-osw-load-diffs-improvement.yml b/changelogs/unreleased/48246-osw-load-diffs-improvement.yml new file mode 100644 index 00000000000..c4292ab0d29 --- /dev/null +++ b/changelogs/unreleased/48246-osw-load-diffs-improvement.yml @@ -0,0 +1,5 @@ +--- +title: Improve performance when fetching collapsed diffs and commenting in merge requests +merge_request: 20940 +author: +type: performance diff --git a/changelogs/unreleased/48773-gitlab-project-import-should-use-object-storage.yml b/changelogs/unreleased/48773-gitlab-project-import-should-use-object-storage.yml new file mode 100644 index 00000000000..f298380b920 --- /dev/null +++ b/changelogs/unreleased/48773-gitlab-project-import-should-use-object-storage.yml @@ -0,0 +1,5 @@ +--- +title: Add object storage logic to project import +merge_request: 20773 +author: +type: added diff --git a/changelogs/unreleased/48834-chart-versions-for-applications-installed-by-one-click-install-buttons-should-be-version-locked.yml b/changelogs/unreleased/48834-chart-versions-for-applications-installed-by-one-click-install-buttons-should-be-version-locked.yml new file mode 100644 index 00000000000..d79b95411aa --- /dev/null +++ b/changelogs/unreleased/48834-chart-versions-for-applications-installed-by-one-click-install-buttons-should-be-version-locked.yml @@ -0,0 +1,6 @@ +--- +title: Chart versions for applications installed by one click install buttons should + be version locked +merge_request: 20765 +author: +type: fixed diff --git a/changelogs/unreleased/49830-use-helm-272.yml b/changelogs/unreleased/49830-use-helm-272.yml new file mode 100644 index 00000000000..f6ecc12dbfa --- /dev/null +++ b/changelogs/unreleased/49830-use-helm-272.yml @@ -0,0 +1,5 @@ +--- +title: Use Helm 2.7.2 for GitLab Managed Apps +merge_request: 20956 +author: +type: changed diff --git a/changelogs/unreleased/49851-link-to-runners.yml b/changelogs/unreleased/49851-link-to-runners.yml new file mode 100644 index 00000000000..89fd6853bc8 --- /dev/null +++ b/changelogs/unreleased/49851-link-to-runners.yml @@ -0,0 +1,6 @@ +--- +title: Automatically expand runner's settings block when linking to the runner's settings + page +merge_request: +author: +type: other diff --git a/changelogs/unreleased/49861-top-nav-search-bar-produces-console-error-when-unauthenticated.yml b/changelogs/unreleased/49861-top-nav-search-bar-produces-console-error-when-unauthenticated.yml new file mode 100644 index 00000000000..30f5002c5b5 --- /dev/null +++ b/changelogs/unreleased/49861-top-nav-search-bar-produces-console-error-when-unauthenticated.yml @@ -0,0 +1,5 @@ +--- +title: fix error caused when using the search bar while unauthenticated +merge_request: 20970 +author: +type: fixed diff --git a/changelogs/unreleased/frozen-string-enable-app-models.yml b/changelogs/unreleased/frozen-string-enable-app-models.yml new file mode 100644 index 00000000000..4c149ea55ef --- /dev/null +++ b/changelogs/unreleased/frozen-string-enable-app-models.yml @@ -0,0 +1,5 @@ +--- +title: Enable frozen string in app/models/*.rb +merge_request: 20851 +author: gfyoung +type: performance diff --git a/changelogs/unreleased/rouge-3-2-0.yml b/changelogs/unreleased/rouge-3-2-0.yml new file mode 100644 index 00000000000..15ac4cc1e76 --- /dev/null +++ b/changelogs/unreleased/rouge-3-2-0.yml @@ -0,0 +1,5 @@ +--- +title: Update to Rouge 3.2.0, including Terraform and Crystal lexer and bug fixes +merge_request: 20991 +author: +type: changed diff --git a/db/migrate/20180710162338_add_foreign_key_from_notification_settings_to_users.rb b/db/migrate/20180710162338_add_foreign_key_from_notification_settings_to_users.rb new file mode 100644 index 00000000000..91656f194e5 --- /dev/null +++ b/db/migrate/20180710162338_add_foreign_key_from_notification_settings_to_users.rb @@ -0,0 +1,30 @@ +class AddForeignKeyFromNotificationSettingsToUsers < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + class NotificationSetting < ActiveRecord::Base + self.table_name = 'notification_settings' + + include EachBatch + end + + class User < ActiveRecord::Base + self.table_name = 'users' + end + + DOWNTIME = false + + disable_ddl_transaction! + + def up + NotificationSetting.each_batch(of: 1000) do |batch| + batch.where('NOT EXISTS (?)', User.select(1).where('users.id = notification_settings.user_id')) + .delete_all + end + + add_concurrent_foreign_key(:notification_settings, :users, column: :user_id, on_delete: :cascade) + end + + def down + remove_foreign_key(:notification_settings, column: :user_id) + end +end diff --git a/db/schema.rb b/db/schema.rb index 769baa825a5..2bef2971f29 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -2350,6 +2350,7 @@ ActiveRecord::Schema.define(version: 20180726172057) do add_foreign_key "milestones", "projects", name: "fk_9bd0a0c791", on_delete: :cascade add_foreign_key "note_diff_files", "notes", column: "diff_note_id", on_delete: :cascade add_foreign_key "notes", "projects", name: "fk_99e097b079", on_delete: :cascade + add_foreign_key "notification_settings", "users", name: "fk_0c95e91db7", on_delete: :cascade add_foreign_key "oauth_openid_requests", "oauth_access_grants", column: "access_grant_id", name: "fk_oauth_openid_requests_oauth_access_grants_access_grant_id" add_foreign_key "pages_domains", "projects", name: "fk_ea2f6dfc6f", on_delete: :cascade add_foreign_key "personal_access_tokens", "users" diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index d95f8c7c8cc..95d705d3a3d 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -962,8 +962,8 @@ the binaries directory: ```yaml job: - artifacts: - name: "$CI_COMMIT_REF_NAME" + artifacts: + name: "$CI_COMMIT_REF_NAME" paths: - binaries/ ``` diff --git a/doc/development/performance.md b/doc/development/performance.md index c4162a05b77..6b4cb6d72d1 100644 --- a/doc/development/performance.md +++ b/doc/development/performance.md @@ -347,13 +347,7 @@ def expire_first_branch_cache end ``` -## Anti-Patterns - -This is a collection of [anti-patterns][anti-pattern] that should be avoided -unless these changes have a measurable, significant and positive impact on -production environments. - -### String Freezing +## String Freezing In recent Ruby versions calling `freeze` on a String leads to it being allocated only once and re-used. For example, on Ruby 2.3 this will only allocate the @@ -365,17 +359,38 @@ only once and re-used. For example, on Ruby 2.3 this will only allocate the end ``` -Blindly adding a `.freeze` call to every String is an anti-pattern that should -be avoided unless one can prove (using production data) the call actually has a -positive impact on performance. +Depending on the size of the String and how frequently it would be allocated +(before the `.freeze` call was added), this _may_ make things faster, but +there's no guarantee it will. + +Strings will be frozen by default in Ruby 3.0. To prepare our code base for +this eventuality, it's a good practice to add the following header to all +Ruby files: + +```ruby +# frozen_string_literal: true +``` + +This may cause test failures in the code that expects to be able to manipulate +strings. Instead of using `dup`, use the unary plus to get an unfrozen string: + +```ruby +test = +"hello" +test += " world" +``` + +## Anti-Patterns -This feature of Ruby wasn't really meant to make things faster directly, instead -it was meant to reduce the number of allocations. Depending on the size of the -String and how frequently it would be allocated (before the `.freeze` call was -added), this _may_ make things faster, but there's no guarantee it will. +This is a collection of [anti-patterns][anti-pattern] that should be avoided +unless these changes have a measurable, significant and positive impact on +production environments. -Another common flavour of this is to not only freeze a String, but also assign -it to a constant, for example: +### Moving Allocations to Constants + +Storing an object as a constant so you only allocate it once _may_ improve +performance, but there's no guarantee this will. Looking up constants has an +impact on runtime performance, and as such, using a constant instead of +referencing an object directly may even slow code down. For example: ```ruby SOME_CONSTANT = 'foo'.freeze @@ -393,13 +408,6 @@ there's nothing stopping somebody from doing this elsewhere in the code: SOME_CONSTANT = 'bar' ``` -### Moving Allocations to Constants - -Storing an object as a constant so you only allocate it once _may_ improve -performance, but there's no guarantee this will. Looking up constants has an -impact on runtime performance, and as such, using a constant instead of -referencing an object directly may even slow code down. - [#15607]: https://gitlab.com/gitlab-org/gitlab-ce/issues/15607 [yorickpeterse]: https://gitlab.com/yorickpeterse [anti-pattern]: https://en.wikipedia.org/wiki/Anti-pattern diff --git a/doc/user/profile/account/two_factor_authentication.md b/doc/user/profile/account/two_factor_authentication.md index d3a2a7dcd14..e25e1e19b13 100644 --- a/doc/user/profile/account/two_factor_authentication.md +++ b/doc/user/profile/account/two_factor_authentication.md @@ -88,7 +88,7 @@ storage in a safe place. **Each code can be used only once** to log in to your account. If you lose the recovery codes or just want to generate new ones, you can do so -from the **Profile settings âž” Account** page where you first enabled 2FA. +[using SSH](#generate-new-recovery-codes-using-ssh). ## Logging in with 2FA Enabled diff --git a/lib/backup/repository.rb b/lib/backup/repository.rb index af762db517c..906ed498026 100644 --- a/lib/backup/repository.rb +++ b/lib/backup/repository.rb @@ -1,10 +1,7 @@ require 'yaml' -require_relative 'helper' module Backup class Repository - include Backup::Helper - attr_reader :progress def initialize(progress) @@ -42,131 +39,36 @@ module Backup end def prepare_directories - Gitlab.config.repositories.storages.each do |name, repository_storage| - delete_all_repositories(name, repository_storage) + Gitlab.config.repositories.storages.each do |name, _repository_storage| + Gitlab::GitalyClient::StorageService.new(name).delete_all_repositories end end def backup_project(project) - gitaly_migrate(:repository_backup, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| - if is_enabled - backup_project_gitaly(project) - else - backup_project_local(project) - end - end - - backup_custom_hooks(project) - rescue => e - progress_warn(project, e, 'Failed to backup repo') - end - - def backup_project_gitaly(project) path_to_project_bundle = path_to_bundle(project) Gitlab::GitalyClient::RepositoryService.new(project.repository) .create_bundle(path_to_project_bundle) - end - - def backup_project_local(project) - path_to_project_repo = Gitlab::GitalyClient::StorageSettings.allow_disk_access do - path_to_repo(project) - end - - path_to_project_bundle = path_to_bundle(project) - - cmd = %W(#{Gitlab.config.git.bin_path} --git-dir=#{path_to_project_repo} bundle create #{path_to_project_bundle} --all) - output, status = Gitlab::Popen.popen(cmd) - progress_warn(project, cmd.join(' '), output) unless status.zero? - end - - def delete_all_repositories(name, repository_storage) - gitaly_migrate(:delete_all_repositories, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| - if is_enabled - Gitlab::GitalyClient::StorageService.new(name).delete_all_repositories - else - local_delete_all_repositories(name, repository_storage) - end - end - end - - def local_delete_all_repositories(name, repository_storage) - path = repository_storage.legacy_disk_path - return unless File.exist?(path) - - bk_repos_path = File.join(Gitlab.config.backup.path, "tmp", "#{name}-repositories.old." + Time.now.to_i.to_s) - FileUtils.mkdir_p(bk_repos_path, mode: 0700) - files = Dir.glob(File.join(path, "*"), File::FNM_DOTMATCH) - [File.join(path, "."), File.join(path, "..")] - - begin - FileUtils.mv(files, bk_repos_path) - rescue Errno::EACCES - access_denied_error(path) - rescue Errno::EBUSY - resource_busy_error(path) - end - end - def local_restore_custom_hooks(project, dir) - path_to_project_repo = Gitlab::GitalyClient::StorageSettings.allow_disk_access do - path_to_repo(project) - end - cmd = %W(tar -xf #{path_to_tars(project, dir)} -C #{path_to_project_repo} #{dir}) - output, status = Gitlab::Popen.popen(cmd) - unless status.zero? - progress_warn(project, cmd.join(' '), output) - end - end - - def gitaly_restore_custom_hooks(project, dir) - custom_hooks_path = path_to_tars(project, dir) - Gitlab::GitalyClient::RepositoryService.new(project.repository) - .restore_custom_hooks(custom_hooks_path) + backup_custom_hooks(project) + rescue => e + progress_warn(project, e, 'Failed to backup repo') end - def local_backup_custom_hooks(project) - in_path(path_to_tars(project)) do |dir| - path_to_project_repo = Gitlab::GitalyClient::StorageSettings.allow_disk_access do - path_to_repo(project) - end - break unless File.exist?(File.join(path_to_project_repo, dir)) - - FileUtils.mkdir_p(path_to_tars(project)) - cmd = %W(tar -cf #{path_to_tars(project, dir)} -c #{path_to_project_repo} #{dir}) - output, status = Gitlab::Popen.popen(cmd) - - unless status.zero? - progress_warn(project, cmd.join(' '), output) - end - end - end + def backup_custom_hooks(project) + FileUtils.mkdir_p(project_backup_path(project)) - def gitaly_backup_custom_hooks(project) - FileUtils.mkdir_p(path_to_tars(project)) - custom_hooks_path = path_to_tars(project, 'custom_hooks') + custom_hooks_path = custom_hooks_tar(project) Gitlab::GitalyClient::RepositoryService.new(project.repository) .backup_custom_hooks(custom_hooks_path) end - def backup_custom_hooks(project) - gitaly_migrate(:backup_custom_hooks, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| - if is_enabled - gitaly_backup_custom_hooks(project) - else - local_backup_custom_hooks(project) - end - end - end - def restore_custom_hooks(project) - in_path(path_to_tars(project)) do |dir| - gitaly_migrate(:restore_custom_hooks, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| - if is_enabled - gitaly_restore_custom_hooks(project, dir) - else - local_restore_custom_hooks(project, dir) - end - end - end + return unless Dir.exist?(project_backup_path(project)) + return if Dir.glob("#{project_backup_path(project)}/custom_hooks*").none? + + custom_hooks_path = custom_hooks_tar(project) + Gitlab::GitalyClient::RepositoryService.new(project.repository) + .restore_custom_hooks(custom_hooks_path) end def restore @@ -181,7 +83,8 @@ module Backup restore_repo_success = nil if File.exist?(path_to_project_bundle) begin - project.repository.create_from_bundle path_to_project_bundle + project.repository.create_from_bundle(path_to_project_bundle) + restore_custom_hooks(project) restore_repo_success = true rescue => e restore_repo_success = false @@ -197,8 +100,6 @@ module Backup progress.puts "[Failed] restoring #{project.full_path} repository".color(:red) end - restore_custom_hooks(project) - wiki = ProjectWiki.new(project) path_to_wiki_bundle = path_to_bundle(wiki) @@ -219,48 +120,28 @@ module Backup protected - def path_to_repo(project) - project.repository.path_to_repo - end - def path_to_bundle(project) File.join(backup_repos_path, project.disk_path + '.bundle') end - def path_to_tars(project, dir = nil) - path = File.join(backup_repos_path, project.disk_path) + def project_backup_path(project) + File.join(backup_repos_path, project.disk_path) + end - if dir - File.join(path, "#{dir}.tar") - else - path - end + def custom_hooks_tar(project) + File.join(project_backup_path(project), "custom_hooks.tar") end def backup_repos_path File.join(Gitlab.config.backup.path, 'repositories') end - def in_path(path) - return unless Dir.exist?(path) - - dir_entries = Dir.entries(path) - - if dir_entries.include?('custom_hooks') || dir_entries.include?('custom_hooks.tar') - yield('custom_hooks') - end - end - def prepare FileUtils.rm_rf(backup_repos_path) FileUtils.mkdir_p(Gitlab.config.backup.path) FileUtils.mkdir(backup_repos_path, mode: 0700) end - def silent - { err: '/dev/null', out: '/dev/null' } - end - private def progress_warn(project, cmd, output) @@ -273,18 +154,8 @@ module Backup project_or_wiki.repository.empty? end - def repository_storage_paths_args - Gitlab.config.repositories.storages.values.map { |rs| rs.legacy_disk_path } - end - def display_repo_path(project) project.hashed_storage?(:repository) ? "#{project.full_path} (#{project.disk_path})" : project.full_path end - - def gitaly_migrate(method, status: Gitlab::GitalyClient::MigrationStatus::OPT_IN, &block) - Gitlab::GitalyClient.migrate(method, status: status, &block) - rescue GRPC::NotFound, GRPC::BadStatus => e - raise Error, e - end end end diff --git a/lib/gitlab/bare_repository_import/importer.rb b/lib/gitlab/bare_repository_import/importer.rb index 4ca5a78e068..04aa6aab771 100644 --- a/lib/gitlab/bare_repository_import/importer.rb +++ b/lib/gitlab/bare_repository_import/importer.rb @@ -26,6 +26,12 @@ module Gitlab end end + # This is called from within a rake task only used by Admins, so allow writing + # to STDOUT + def self.log(message) + puts message # rubocop:disable Rails/Output + end + attr_reader :user, :project_name, :bare_repo delegate :log, to: :class @@ -59,11 +65,10 @@ module Gitlab import_type: 'bare_repository', namespace_id: group&.id).execute - if project.persisted? && mv_repo(project) + if project.persisted? && mv_repositories(project) log " * Created #{project.name} (#{project_full_path})".color(:green) project.write_repository_config - Gitlab::Git::Repository.create_hooks(project.repository.path_to_repo, Gitlab.config.gitlab_shell.hooks_path) ProjectCacheWorker.perform_async(project.id) else @@ -74,12 +79,11 @@ module Gitlab project end - def mv_repo(project) - storage_path = storage_path_for_shard(project.repository_storage) - FileUtils.mv(repo_path, project.repository.path_to_repo) + def mv_repositories(project) + mv_repo(bare_repo.repo_path, project.repository) if bare_repo.wiki_exists? - FileUtils.mv(wiki_path, File.join(storage_path, project.disk_path + '.wiki.git')) + mv_repo(bare_repo.wiki_path, project.wiki.repository) end true @@ -89,6 +93,11 @@ module Gitlab false end + def mv_repo(path, repository) + repository.create_from_bundle(bundle(path)) + FileUtils.rm_rf(path) + end + def storage_path_for_shard(shard) Gitlab.config.repositories.storages[shard].legacy_disk_path end @@ -101,10 +110,17 @@ module Gitlab Groups::NestedCreateService.new(user, group_path: group_path).execute end - # This is called from within a rake task only used by Admins, so allow writing - # to STDOUT - def self.log(message) - puts message # rubocop:disable Rails/Output + def bundle(repo_path) + # TODO: we could save some time and disk space by using + # `git bundle create - --all` and streaming the bundle directly to + # Gitaly, rather than writing it on disk first + bundle_path = "#{repo_path}.bundle" + cmd = %W(#{Gitlab.config.git.bin_path} --git-dir=#{repo_path} bundle create #{bundle_path} --all) + output, status = Gitlab::Popen.popen(cmd) + + raise output unless status.zero? + + bundle_path end end end diff --git a/lib/gitlab/bare_repository_import/repository.rb b/lib/gitlab/bare_repository_import/repository.rb index fe267248275..c0c666dfb7b 100644 --- a/lib/gitlab/bare_repository_import/repository.rb +++ b/lib/gitlab/bare_repository_import/repository.rb @@ -1,5 +1,3 @@ -# Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/953 -# module Gitlab module BareRepositoryImport class Repository diff --git a/lib/gitlab/cleanup/project_uploads.rb b/lib/gitlab/cleanup/project_uploads.rb index b88e00311d5..f55ab535efe 100644 --- a/lib/gitlab/cleanup/project_uploads.rb +++ b/lib/gitlab/cleanup/project_uploads.rb @@ -40,7 +40,7 @@ module Gitlab # Accepts a path in the form of "#{hex_secret}/#{filename}" def find_correct_path(upload_path) upload = Upload.find_by(uploader: 'FileUploader', path: upload_path) - return unless upload && upload.local? + return unless upload && upload.local? && upload.model upload.absolute_path rescue => e diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index eb02c7ac8e1..73151e4a4c5 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -39,19 +39,6 @@ module Gitlab ChecksumError = Class.new(StandardError) class << self - # Unlike `new`, `create` takes the repository path - def create(repo_path, bare: true, symlink_hooks_to: nil) - FileUtils.mkdir_p(repo_path, mode: 0770) - - # Equivalent to `git --git-path=#{repo_path} init [--bare]` - repo = Rugged::Repository.init_at(repo_path, bare) - repo.close - - create_hooks(repo_path, symlink_hooks_to) if symlink_hooks_to.present? - - true - end - def create_hooks(repo_path, global_hooks_path) local_hooks_path = File.join(repo_path, 'hooks') real_local_hooks_path = :not_found diff --git a/lib/gitlab/import_export/command_line_util.rb b/lib/gitlab/import_export/command_line_util.rb index 2f163db936b..3adc44f8044 100644 --- a/lib/gitlab/import_export/command_line_util.rb +++ b/lib/gitlab/import_export/command_line_util.rb @@ -18,6 +18,21 @@ module Gitlab private + def download_or_copy_upload(uploader, upload_path) + if uploader.upload.local? + copy_files(uploader.path, upload_path) + else + download(uploader.url, upload_path) + end + end + + def download(url, upload_path) + File.open(upload_path, 'w') do |file| + # Download (stream) file from the uploader's location + IO.copy_stream(URI.parse(url).open, file) + end + end + def tar_with_options(archive:, dir:, options:) execute(%W(tar -#{options} #{archive} -C #{dir} .)) end diff --git a/lib/gitlab/import_export/file_importer.rb b/lib/gitlab/import_export/file_importer.rb index 4c411f4847e..7fd66b4e244 100644 --- a/lib/gitlab/import_export/file_importer.rb +++ b/lib/gitlab/import_export/file_importer.rb @@ -10,15 +10,18 @@ module Gitlab new(*args).import end - def initialize(archive_file:, shared:) + def initialize(project:, archive_file:, shared:) + @project = project @archive_file = archive_file @shared = shared end def import mkdir_p(@shared.export_path) + mkdir_p(@shared.archive_path) - remove_symlinks! + remove_symlinks + copy_archive wait_for_archived_file do decompress_archive @@ -27,7 +30,8 @@ module Gitlab @shared.error(e) false ensure - remove_symlinks! + remove_import_file + remove_symlinks end private @@ -51,7 +55,15 @@ module Gitlab result end - def remove_symlinks! + def copy_archive + return if @archive_file + + @archive_file = File.join(@shared.archive_path, Gitlab::ImportExport.export_filename(project: @project)) + + download_or_copy_upload(@project.import_export_upload.import_file, @archive_file) + end + + def remove_symlinks extracted_files.each do |path| FileUtils.rm(path) if File.lstat(path).symlink? end @@ -59,6 +71,10 @@ module Gitlab true end + def remove_import_file + FileUtils.rm_rf(@archive_file) + end + def extracted_files Dir.glob("#{@shared.export_path}/**/*", File::FNM_DOTMATCH).reject { |f| IGNORED_FILENAMES.include?(File.basename(f)) } end diff --git a/lib/gitlab/import_export/importer.rb b/lib/gitlab/import_export/importer.rb index 63cab07324a..4e179f63d8c 100644 --- a/lib/gitlab/import_export/importer.rb +++ b/lib/gitlab/import_export/importer.rb @@ -35,7 +35,8 @@ module Gitlab end def import_file - Gitlab::ImportExport::FileImporter.import(archive_file: @archive_file, + Gitlab::ImportExport::FileImporter.import(project: @project, + archive_file: @archive_file, shared: @shared) end @@ -91,7 +92,14 @@ module Gitlab end def remove_import_file - FileUtils.rm_rf(@archive_file) + return unless Gitlab::ImportExport.object_storage? + + upload = @project.import_export_upload + + return unless upload&.import_file&.file + + upload.remove_import_file! + upload.save! end def overwrite_project diff --git a/lib/gitlab/import_export/uploads_manager.rb b/lib/gitlab/import_export/uploads_manager.rb index 1110149712d..07875ebb56a 100644 --- a/lib/gitlab/import_export/uploads_manager.rb +++ b/lib/gitlab/import_export/uploads_manager.rb @@ -91,10 +91,7 @@ module Gitlab mkdir_p(File.join(uploads_export_path, secret)) - File.open(upload_path, 'w') do |file| - # Download (stream) file from the uploader's location - IO.copy_stream(URI.parse(upload.file.url).open, file) - end + download_or_copy_upload(upload, upload_path) end end end diff --git a/lib/gitlab/kubernetes/helm.rb b/lib/gitlab/kubernetes/helm.rb index 0f0588b8b23..530ccf88053 100644 --- a/lib/gitlab/kubernetes/helm.rb +++ b/lib/gitlab/kubernetes/helm.rb @@ -1,7 +1,7 @@ module Gitlab module Kubernetes module Helm - HELM_VERSION = '2.7.0'.freeze + HELM_VERSION = '2.7.2'.freeze NAMESPACE = 'gitlab-managed-apps'.freeze end end diff --git a/lib/gitlab/template_helper.rb b/lib/gitlab/template_helper.rb index 3b8e45e0688..3918abaab45 100644 --- a/lib/gitlab/template_helper.rb +++ b/lib/gitlab/template_helper.rb @@ -2,11 +2,17 @@ module Gitlab module TemplateHelper include Gitlab::Utils::StrongMemoize - def prepare_template_environment(file_path) - return unless file_path.present? + def prepare_template_environment(file) + return unless file&.path.present? - FileUtils.mkdir_p(File.dirname(import_upload_path)) - FileUtils.copy_entry(file_path, import_upload_path) + if Gitlab::ImportExport.object_storage? + params[:import_export_upload] = ImportExportUpload.new(import_file: file) + else + FileUtils.mkdir_p(File.dirname(import_upload_path)) + FileUtils.copy_entry(file.path, import_upload_path) + + params[:import_source] = import_upload_path + end end def import_upload_path diff --git a/spec/controllers/admin/services_controller_spec.rb b/spec/controllers/admin/services_controller_spec.rb index 701211c2586..4439ea4a533 100644 --- a/spec/controllers/admin/services_controller_spec.rb +++ b/spec/controllers/admin/services_controller_spec.rb @@ -13,7 +13,7 @@ describe Admin::ServicesController do Service.available_services_names.each do |service_name| context "#{service_name}" do let!(:service) do - service_template = service_name.concat("_service").camelize.constantize + service_template = "#{service_name}_service".camelize.constantize service_template.where(template: true).first_or_create end diff --git a/spec/features/projects/import_export/import_file_object_storage_spec.rb b/spec/features/projects/import_export/import_file_object_storage_spec.rb new file mode 100644 index 00000000000..0d364543916 --- /dev/null +++ b/spec/features/projects/import_export/import_file_object_storage_spec.rb @@ -0,0 +1,103 @@ +require 'spec_helper' + +describe 'Import/Export - project import integration test', :js do + include Select2Helper + + let(:user) { create(:user) } + let(:file) { File.join(Rails.root, 'spec', 'features', 'projects', 'import_export', 'test_project_export.tar.gz') } + let(:export_path) { "#{Dir.tmpdir}/import_file_spec" } + + before do + stub_feature_flags(import_export_object_storage: true) + stub_uploads_object_storage(FileUploader) + allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path) + gitlab_sign_in(user) + end + + after do + FileUtils.rm_rf(export_path, secure: true) + end + + context 'when selecting the namespace' do + let(:user) { create(:admin) } + let!(:namespace) { user.namespace } + let(:project_path) { 'test-project-path' + SecureRandom.hex } + + context 'prefilled the path' do + it 'user imports an exported project successfully' do + visit new_project_path + + select2(namespace.id, from: '#project_namespace_id') + fill_in :project_path, with: project_path, visible: true + click_import_project_tab + click_link 'GitLab export' + + expect(page).to have_content('Import an exported GitLab project') + expect(URI.parse(current_url).query).to eq("namespace_id=#{namespace.id}&path=#{project_path}") + + attach_file('file', file) + click_on 'Import project' + + expect(Project.count).to eq(1) + + project = Project.last + expect(project).not_to be_nil + expect(project.description).to eq("Foo Bar") + expect(project.issues).not_to be_empty + expect(project.merge_requests).not_to be_empty + expect(project_hook_exists?(project)).to be true + expect(wiki_exists?(project)).to be true + expect(project.import_state.status).to eq('finished') + end + end + + context 'path is not prefilled' do + it 'user imports an exported project successfully' do + visit new_project_path + click_import_project_tab + click_link 'GitLab export' + + fill_in :path, with: 'test-project-path', visible: true + attach_file('file', file) + + expect { click_on 'Import project' }.to change { Project.count }.by(1) + + project = Project.last + expect(project).not_to be_nil + expect(page).to have_content("Project 'test-project-path' is being imported") + end + end + end + + it 'invalid project' do + project = create(:project, namespace: user.namespace) + + visit new_project_path + + select2(user.namespace.id, from: '#project_namespace_id') + fill_in :project_path, with: project.name, visible: true + click_import_project_tab + click_link 'GitLab export' + attach_file('file', file) + click_on 'Import project' + + page.within('.flash-container') do + expect(page).to have_content('Project could not be imported') + end + end + + def wiki_exists?(project) + wiki = ProjectWiki.new(project) + wiki.repository.exists? && !wiki.repository.empty? + end + + def project_hook_exists?(project) + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + Gitlab::Git::Hook.new('post-receive', project.repository.raw_repository).exists? + end + end + + def click_import_project_tab + find('#import-project-tab').click + end +end diff --git a/spec/features/projects/import_export/import_file_spec.rb b/spec/features/projects/import_export/import_file_spec.rb index 9cbfb62d872..2d86115de12 100644 --- a/spec/features/projects/import_export/import_file_spec.rb +++ b/spec/features/projects/import_export/import_file_spec.rb @@ -8,6 +8,7 @@ describe 'Import/Export - project import integration test', :js do let(:export_path) { "#{Dir.tmpdir}/import_file_spec" } before do + stub_feature_flags(import_export_object_storage: false) allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path) gitlab_sign_in(user) end diff --git a/spec/javascripts/ide/components/new_dropdown/index_spec.js b/spec/javascripts/ide/components/new_dropdown/index_spec.js index 092c405a70b..5cb8b177fc9 100644 --- a/spec/javascripts/ide/components/new_dropdown/index_spec.js +++ b/spec/javascripts/ide/components/new_dropdown/index_spec.js @@ -62,7 +62,9 @@ describe('new dropdown component', () => { vm.dropdownOpen = true; setTimeout(() => { - expect(vm.$refs.dropdownMenu.scrollIntoView).toHaveBeenCalled(); + expect(vm.$refs.dropdownMenu.scrollIntoView).toHaveBeenCalledWith({ + block: 'nearest', + }); done(); }); diff --git a/spec/lib/backup/repository_spec.rb b/spec/lib/backup/repository_spec.rb index 92a27e308d2..c5a854b5660 100644 --- a/spec/lib/backup/repository_spec.rb +++ b/spec/lib/backup/repository_spec.rb @@ -73,37 +73,27 @@ describe Backup::Repository do end end - describe '#delete_all_repositories', :seed_helper do - shared_examples('delete_all_repositories') do - before do - allow(FileUtils).to receive(:mkdir_p).and_call_original - allow(FileUtils).to receive(:mv).and_call_original - end - - after(:all) do - ensure_seeds - end - - it 'removes all repositories' do - # Sanity check: there should be something for us to delete - expect(list_repositories).to include(File.join(SEED_STORAGE_PATH, TEST_REPO_PATH)) + describe '#prepare_directories', :seed_helper do + before do + allow(FileUtils).to receive(:mkdir_p).and_call_original + allow(FileUtils).to receive(:mv).and_call_original + end - subject.delete_all_repositories('default', Gitlab.config.repositories.storages['default']) + after(:all) do + ensure_seeds + end - expect(list_repositories).to be_empty - end + it' removes all repositories' do + # Sanity check: there should be something for us to delete + expect(list_repositories).to include(File.join(SEED_STORAGE_PATH, TEST_REPO_PATH)) - def list_repositories - Dir[File.join(SEED_STORAGE_PATH, '*.git')] - end - end + subject.prepare_directories - context 'with gitaly' do - it_behaves_like 'delete_all_repositories' + expect(list_repositories).to be_empty end - context 'without gitaly', :skip_gitaly_mock do - it_behaves_like 'delete_all_repositories' + def list_repositories + Dir[File.join(SEED_STORAGE_PATH, '*.git')] end end diff --git a/spec/lib/gitlab/bare_repository_import/importer_spec.rb b/spec/lib/gitlab/bare_repository_import/importer_spec.rb index 468f6ff6d24..6e21c846c0a 100644 --- a/spec/lib/gitlab/bare_repository_import/importer_spec.rb +++ b/spec/lib/gitlab/bare_repository_import/importer_spec.rb @@ -1,10 +1,11 @@ require 'spec_helper' -describe Gitlab::BareRepositoryImport::Importer, repository: true do +describe Gitlab::BareRepositoryImport::Importer, :seed_helper do let!(:admin) { create(:admin) } let!(:base_dir) { Dir.mktmpdir + '/' } let(:bare_repository) { Gitlab::BareRepositoryImport::Repository.new(base_dir, File.join(base_dir, "#{project_path}.git")) } let(:gitlab_shell) { Gitlab::Shell.new } + let(:source_project) { TEST_REPO_PATH } subject(:importer) { described_class.new(admin, bare_repository) } @@ -17,16 +18,11 @@ describe Gitlab::BareRepositoryImport::Importer, repository: true do after do FileUtils.rm_rf(base_dir) + TestEnv.clean_test_path + ensure_seeds Rainbow.enabled = @rainbow end - around do |example| - # TODO migrate BareRepositoryImport https://gitlab.com/gitlab-org/gitaly/issues/953 - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - example.run - end - end - shared_examples 'importing a repository' do describe '.execute' do it 'creates a project for a repository in storage' do @@ -86,8 +82,8 @@ describe Gitlab::BareRepositoryImport::Importer, repository: true do importer.create_project_if_needed end - it 'creates the Git repo on disk with the proper symlink for hooks' do - create_bare_repository("#{project_path}.git") + it 'creates the Git repo on disk' do + prepare_repository("#{project_path}.git", source_project) importer.create_project_if_needed @@ -97,9 +93,6 @@ describe Gitlab::BareRepositoryImport::Importer, repository: true do expect(gitlab_shell.exists?(project.repository_storage, repo_path)).to be(true) expect(gitlab_shell.exists?(project.repository_storage, hook_path)).to be(true) - - full_hook_path = File.join(project.repository.path_to_repo, 'hooks') - expect(File.readlink(full_hook_path)).to eq(Gitlab.config.gitlab_shell.hooks_path) end context 'hashed storage enabled' do @@ -148,7 +141,7 @@ describe Gitlab::BareRepositoryImport::Importer, repository: true do end it 'creates the Git repo in disk' do - create_bare_repository("#{project_path}.git") + prepare_repository("#{project_path}.git", source_project) importer.create_project_if_needed @@ -158,23 +151,23 @@ describe Gitlab::BareRepositoryImport::Importer, repository: true do expect(gitlab_shell.exists?(project.repository_storage, project.disk_path + '.wiki.git')).to be(true) end - it 'moves an existing project to the correct path' do + context 'with a repository already on disk' do + let!(:base_dir) { TestEnv.repos_path } # This is a quick way to get a valid repository instead of copying an # existing one. Since it's not persisted, the importer will try to # create the project. - project = build(:project, :legacy_storage, :repository) - original_commit_count = project.repository.commit_count - - legacy_path = Gitlab.config.repositories.storages[project.repository_storage].legacy_disk_path + let(:project) { build(:project, :legacy_storage, :repository) } + let(:project_path) { project.full_path } - bare_repo = Gitlab::BareRepositoryImport::Repository.new(legacy_path, project.repository.path) - gitlab_importer = described_class.new(admin, bare_repo) + it 'moves an existing project to the correct path' do + original_commit_count = project.repository.commit_count - expect(gitlab_importer).to receive(:create_project).and_call_original + expect(importer).to receive(:create_project).and_call_original - new_project = gitlab_importer.create_project_if_needed + new_project = importer.create_project_if_needed - expect(new_project.repository.commit_count).to eq(original_commit_count) + expect(new_project.repository.commit_count).to eq(original_commit_count) + end end end @@ -185,8 +178,8 @@ describe Gitlab::BareRepositoryImport::Importer, repository: true do it_behaves_like 'importing a repository' it 'creates the Wiki git repo in disk' do - create_bare_repository("#{project_path}.git") - create_bare_repository("#{project_path}.wiki.git") + prepare_repository("#{project_path}.git", source_project) + prepare_repository("#{project_path}.wiki.git", source_project) expect(Projects::CreateService).to receive(:new).with(admin, hash_including(skip_wiki: true, import_type: 'bare_repository')).and_call_original @@ -213,8 +206,13 @@ describe Gitlab::BareRepositoryImport::Importer, repository: true do end end - def create_bare_repository(project_path) + def prepare_repository(project_path, source_project) repo_path = File.join(base_dir, project_path) - Gitlab::Git::Repository.create(repo_path, bare: true) + + return create_bare_repository(repo_path) unless source_project + + cmd = %W(#{Gitlab.config.git.bin_path} clone --bare #{source_project} #{repo_path}) + + system(git_env, *cmd, chdir: SEED_STORAGE_PATH, out: '/dev/null', err: '/dev/null') end end diff --git a/spec/lib/gitlab/cleanup/project_uploads_spec.rb b/spec/lib/gitlab/cleanup/project_uploads_spec.rb new file mode 100644 index 00000000000..37b38776775 --- /dev/null +++ b/spec/lib/gitlab/cleanup/project_uploads_spec.rb @@ -0,0 +1,278 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Cleanup::ProjectUploads do + subject { described_class.new(logger: logger) } + let(:logger) { double(:logger) } + + before do + allow(logger).to receive(:info).at_least(1).times + allow(logger).to receive(:debug).at_least(1).times + end + + describe '#run!' do + shared_examples_for 'moves the file' do + shared_examples_for 'a real run' do + let(:args) { [dry_run: false] } + + it 'moves the file to its proper location' do + subject.run!(*args) + + expect(File.exist?(path)).to be_falsey + expect(File.exist?(new_path)).to be_truthy + end + + it 'logs action as done' do + expect(logger).to receive(:info).with("Looking for orphaned project uploads to clean up...") + expect(logger).to receive(:info).with("Did #{action}") + + subject.run!(*args) + end + end + + shared_examples_for 'a dry run' do + it 'does not move the file' do + subject.run!(*args) + + expect(File.exist?(path)).to be_truthy + expect(File.exist?(new_path)).to be_falsey + end + + it 'logs action as able to be done' do + expect(logger).to receive(:info).with("Looking for orphaned project uploads to clean up. Dry run...") + expect(logger).to receive(:info).with("Can #{action}") + + subject.run!(*args) + end + end + + context 'when dry_run is false' do + let(:args) { [dry_run: false] } + + it_behaves_like 'a real run' + end + + context 'when dry_run is nil' do + let(:args) { [dry_run: nil] } + + it_behaves_like 'a real run' + end + + context 'when dry_run is true' do + let(:args) { [dry_run: true] } + + it_behaves_like 'a dry run' + end + + context 'with dry_run not specified' do + let(:args) { [] } + + it_behaves_like 'a dry run' + end + end + + shared_examples_for 'moves the file to lost and found' do + let(:action) { "move to lost and found #{path} -> #{new_path}" } + + it_behaves_like 'moves the file' + end + + shared_examples_for 'fixes the file' do + let(:action) { "fix #{path} -> #{new_path}" } + + it_behaves_like 'moves the file' + end + + context 'orphaned project upload file' do + context 'when an upload record matching the secret and filename is found' do + context 'when the project is still in legacy storage' do + let(:orphaned) { create(:upload, :issuable_upload, :with_file, model: create(:project, :legacy_storage)) } + let(:new_path) { orphaned.absolute_path } + let(:path) { File.join(FileUploader.root, 'some', 'wrong', 'location', orphaned.path) } + + before do + FileUtils.mkdir_p(File.dirname(path)) + FileUtils.mv(new_path, path) + end + + it_behaves_like 'fixes the file' + end + + context 'when the project was moved to hashed storage' do + let(:orphaned) { create(:upload, :issuable_upload, :with_file) } + let(:new_path) { orphaned.absolute_path } + let(:path) { File.join(FileUploader.root, 'some', 'wrong', 'location', orphaned.path) } + + before do + FileUtils.mkdir_p(File.dirname(path)) + FileUtils.mv(new_path, path) + end + + it_behaves_like 'fixes the file' + end + + context 'when the project is missing (the upload *record* is an orphan)' do + let(:orphaned) { create(:upload, :issuable_upload, :with_file, model: build(:project, :legacy_storage)) } + let!(:path) { orphaned.absolute_path } + let!(:new_path) { File.join(FileUploader.root, '-', 'project-lost-found', orphaned.model.full_path, orphaned.path) } + + before do + orphaned.model.delete + end + + it_behaves_like 'moves the file to lost and found' + end + + # We will probably want to add logic (Reschedule background upload) to + # cover Case 2 in https://gitlab.com/gitlab-org/gitlab-ce/issues/46535#note_75355104 + context 'when the file should be in object storage' do + context 'when the file otherwise has the correct local path' do + let!(:orphaned) { create(:upload, :issuable_upload, :object_storage, model: build(:project, :legacy_storage)) } + let!(:path) { File.join(FileUploader.root, orphaned.model.full_path, orphaned.path) } + + before do + stub_feature_flags(import_export_object_storage: true) + stub_uploads_object_storage(FileUploader) + + FileUtils.mkdir_p(File.dirname(path)) + FileUtils.touch(path) + end + + it 'does not move the file' do + expect(File.exist?(path)).to be_truthy + + subject.run!(dry_run: false) + + expect(File.exist?(path)).to be_truthy + end + end + + # E.g. the upload file was orphaned, and then uploads were migrated to + # object storage + context 'when the file has the wrong local path' do + let!(:orphaned) { create(:upload, :issuable_upload, :object_storage, model: build(:project, :legacy_storage)) } + let!(:path) { File.join(FileUploader.root, 'wrong', orphaned.path) } + let!(:new_path) { File.join(FileUploader.root, '-', 'project-lost-found', 'wrong', orphaned.path) } + + before do + stub_feature_flags(import_export_object_storage: true) + stub_uploads_object_storage(FileUploader) + + FileUtils.mkdir_p(File.dirname(path)) + FileUtils.touch(path) + end + + it_behaves_like 'moves the file to lost and found' + end + end + end + + context 'when a matching upload record can not be found' do + context 'when the file path fits the known pattern' do + let!(:orphaned) { create(:upload, :issuable_upload, :with_file, model: build(:project, :legacy_storage)) } + let!(:path) { orphaned.absolute_path } + let!(:new_path) { File.join(FileUploader.root, '-', 'project-lost-found', orphaned.model.full_path, orphaned.path) } + + before do + orphaned.delete + end + + it_behaves_like 'moves the file to lost and found' + end + + context 'when the file path does not fit the known pattern' do + let!(:invalid_path) { File.join('group', 'file.jpg') } + let!(:path) { File.join(FileUploader.root, invalid_path) } + let!(:new_path) { File.join(FileUploader.root, '-', 'project-lost-found', invalid_path) } + + before do + FileUtils.mkdir_p(File.dirname(path)) + FileUtils.touch(path) + end + + after do + File.delete(path) if File.exist?(path) + end + + it_behaves_like 'moves the file to lost and found' + end + end + end + + context 'non-orphaned project upload file' do + it 'does not move the file' do + tracked = create(:upload, :issuable_upload, :with_file, model: build(:project, :legacy_storage)) + tracked_path = tracked.absolute_path + + expect(logger).not_to receive(:info).with(/move|fix/i) + expect(File.exist?(tracked_path)).to be_truthy + + subject.run!(dry_run: false) + + expect(File.exist?(tracked_path)).to be_truthy + end + end + + context 'ignorable cases' do + # Because we aren't concerned about these, and can save a lot of + # processing time by ignoring them. If we wish to cleanup hashed storage + # directories, it should simply require removing this test and modifying + # the find command. + context 'when the file is already in hashed storage' do + let(:project) { create(:project) } + + before do + expect(logger).not_to receive(:info).with(/move|fix/i) + end + + it 'does not move even an orphan file' do + orphaned = create(:upload, :issuable_upload, :with_file, model: project) + path = orphaned.absolute_path + orphaned.delete + + expect(File.exist?(path)).to be_truthy + + subject.run!(dry_run: false) + + expect(File.exist?(path)).to be_truthy + end + end + + it 'does not move any non-project (FileUploader) uploads' do + paths = [] + orphaned1 = create(:upload, :personal_snippet_upload, :with_file) + orphaned2 = create(:upload, :namespace_upload, :with_file) + orphaned3 = create(:upload, :attachment_upload, :with_file) + paths << orphaned1.absolute_path + paths << orphaned2.absolute_path + paths << orphaned3.absolute_path + Upload.delete_all + + expect(logger).not_to receive(:info).with(/move|fix/i) + paths.each do |path| + expect(File.exist?(path)).to be_truthy + end + + subject.run!(dry_run: false) + + paths.each do |path| + expect(File.exist?(path)).to be_truthy + end + end + + it 'does not move any uploads in tmp (which would interfere with ongoing upload activity)' do + path = File.join(FileUploader.root, 'tmp', 'foo.jpg') + FileUtils.mkdir_p(File.dirname(path)) + FileUtils.touch(path) + + expect(logger).not_to receive(:info).with(/move|fix/i) + expect(File.exist?(path)).to be_truthy + + subject.run!(dry_run: false) + + expect(File.exist?(path)).to be_truthy + end + end + end +end diff --git a/spec/lib/gitlab/git/attributes_at_ref_parser_spec.rb b/spec/lib/gitlab/git/attributes_at_ref_parser_spec.rb index 5d22dcfb508..ca067a29174 100644 --- a/spec/lib/gitlab/git/attributes_at_ref_parser_spec.rb +++ b/spec/lib/gitlab/git/attributes_at_ref_parser_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::Git::AttributesAtRefParser, seed_helper: true do +describe Gitlab::Git::AttributesAtRefParser, :seed_helper do let(:project) { create(:project, :repository) } let(:repository) { project.repository } diff --git a/spec/lib/gitlab/git/attributes_parser_spec.rb b/spec/lib/gitlab/git/attributes_parser_spec.rb index 2d103123998..18ebfef38f0 100644 --- a/spec/lib/gitlab/git/attributes_parser_spec.rb +++ b/spec/lib/gitlab/git/attributes_parser_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::Git::AttributesParser, seed_helper: true do +describe Gitlab::Git::AttributesParser, :seed_helper do let(:attributes_path) { File.join(SEED_STORAGE_PATH, 'with-git-attributes.git', 'info', 'attributes') } let(:data) { File.read(attributes_path) } diff --git a/spec/lib/gitlab/git/blame_spec.rb b/spec/lib/gitlab/git/blame_spec.rb index ba790b717ae..e704d1c673c 100644 --- a/spec/lib/gitlab/git/blame_spec.rb +++ b/spec/lib/gitlab/git/blame_spec.rb @@ -1,7 +1,7 @@ # coding: utf-8 require "spec_helper" -describe Gitlab::Git::Blame, seed_helper: true do +describe Gitlab::Git::Blame, :seed_helper do let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } let(:blame) do Gitlab::Git::Blame.new(repository, SeedRepo::Commit::ID, "CONTRIBUTING.md") diff --git a/spec/lib/gitlab/git/blob_snippet_spec.rb b/spec/lib/gitlab/git/blob_snippet_spec.rb index d6d365f6492..6effec8295c 100644 --- a/spec/lib/gitlab/git/blob_snippet_spec.rb +++ b/spec/lib/gitlab/git/blob_snippet_spec.rb @@ -2,7 +2,7 @@ require "spec_helper" -describe Gitlab::Git::BlobSnippet, seed_helper: true do +describe Gitlab::Git::BlobSnippet, :seed_helper do describe '#data' do context 'empty lines' do let(:snippet) { Gitlab::Git::BlobSnippet.new('master', nil, nil, nil) } diff --git a/spec/lib/gitlab/git/blob_spec.rb b/spec/lib/gitlab/git/blob_spec.rb index 034b89a46fa..ea49502ae2e 100644 --- a/spec/lib/gitlab/git/blob_spec.rb +++ b/spec/lib/gitlab/git/blob_spec.rb @@ -2,7 +2,7 @@ require "spec_helper" -describe Gitlab::Git::Blob, seed_helper: true do +describe Gitlab::Git::Blob, :seed_helper do let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } describe 'initialize' do diff --git a/spec/lib/gitlab/git/branch_spec.rb b/spec/lib/gitlab/git/branch_spec.rb index a8c5627e678..79ccbb79966 100644 --- a/spec/lib/gitlab/git/branch_spec.rb +++ b/spec/lib/gitlab/git/branch_spec.rb @@ -1,6 +1,6 @@ require "spec_helper" -describe Gitlab::Git::Branch, seed_helper: true do +describe Gitlab::Git::Branch, :seed_helper do let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } let(:rugged) do Gitlab::GitalyClient::StorageSettings.allow_disk_access do diff --git a/spec/lib/gitlab/git/commit_spec.rb b/spec/lib/gitlab/git/commit_spec.rb index 0adb684765d..2718a3c5e49 100644 --- a/spec/lib/gitlab/git/commit_spec.rb +++ b/spec/lib/gitlab/git/commit_spec.rb @@ -1,6 +1,6 @@ require "spec_helper" -describe Gitlab::Git::Commit, seed_helper: true do +describe Gitlab::Git::Commit, :seed_helper do let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } let(:commit) { described_class.find(repository, SeedRepo::Commit::ID) } let(:rugged_commit) do diff --git a/spec/lib/gitlab/git/committer_with_hooks_spec.rb b/spec/lib/gitlab/git/committer_with_hooks_spec.rb index 2100690f873..c7626058acd 100644 --- a/spec/lib/gitlab/git/committer_with_hooks_spec.rb +++ b/spec/lib/gitlab/git/committer_with_hooks_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::Git::CommitterWithHooks, seed_helper: true do +describe Gitlab::Git::CommitterWithHooks, :seed_helper do # TODO https://gitlab.com/gitlab-org/gitaly/issues/1234 skip 'needs to be moved to gitaly-ruby test suite' do shared_examples 'calling wiki hooks' do diff --git a/spec/lib/gitlab/git/compare_spec.rb b/spec/lib/gitlab/git/compare_spec.rb index b6a42e422b5..7cc6f52f8ee 100644 --- a/spec/lib/gitlab/git/compare_spec.rb +++ b/spec/lib/gitlab/git/compare_spec.rb @@ -1,6 +1,6 @@ require "spec_helper" -describe Gitlab::Git::Compare, seed_helper: true do +describe Gitlab::Git::Compare, :seed_helper do let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } let(:compare) { Gitlab::Git::Compare.new(repository, SeedRepo::BigCommit::ID, SeedRepo::Commit::ID, straight: false) } let(:compare_straight) { Gitlab::Git::Compare.new(repository, SeedRepo::BigCommit::ID, SeedRepo::Commit::ID, straight: true) } diff --git a/spec/lib/gitlab/git/diff_collection_spec.rb b/spec/lib/gitlab/git/diff_collection_spec.rb index 65edc750f39..81658874be7 100644 --- a/spec/lib/gitlab/git/diff_collection_spec.rb +++ b/spec/lib/gitlab/git/diff_collection_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::Git::DiffCollection, seed_helper: true do +describe Gitlab::Git::DiffCollection, :seed_helper do subject do Gitlab::Git::DiffCollection.new( iterator, diff --git a/spec/lib/gitlab/git/diff_spec.rb b/spec/lib/gitlab/git/diff_spec.rb index 11ab376ab8f..87d9fcee39e 100644 --- a/spec/lib/gitlab/git/diff_spec.rb +++ b/spec/lib/gitlab/git/diff_spec.rb @@ -1,6 +1,6 @@ require "spec_helper" -describe Gitlab::Git::Diff, seed_helper: true do +describe Gitlab::Git::Diff, :seed_helper do let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } before do diff --git a/spec/lib/gitlab/git/hooks_service_spec.rb b/spec/lib/gitlab/git/hooks_service_spec.rb index 9337aa39e13..55ffced36ac 100644 --- a/spec/lib/gitlab/git/hooks_service_spec.rb +++ b/spec/lib/gitlab/git/hooks_service_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::Git::HooksService, seed_helper: true do +describe Gitlab::Git::HooksService, :seed_helper do let(:gl_id) { 'user-456' } let(:gl_username) { 'janedoe' } let(:user) { Gitlab::Git::User.new(gl_username, 'Jane Doe', 'janedoe@example.com', gl_id) } diff --git a/spec/lib/gitlab/git/index_spec.rb b/spec/lib/gitlab/git/index_spec.rb index e51b875be11..c4edd6961e1 100644 --- a/spec/lib/gitlab/git/index_spec.rb +++ b/spec/lib/gitlab/git/index_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::Git::Index, seed_helper: true do +describe Gitlab::Git::Index, :seed_helper do let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } let(:index) { described_class.new(repository) } diff --git a/spec/lib/gitlab/git/remote_repository_spec.rb b/spec/lib/gitlab/git/remote_repository_spec.rb index eb148cc3804..53ed7c5a13a 100644 --- a/spec/lib/gitlab/git/remote_repository_spec.rb +++ b/spec/lib/gitlab/git/remote_repository_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::Git::RemoteRepository, seed_helper: true do +describe Gitlab::Git::RemoteRepository, :seed_helper do let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } subject { described_class.new(repository) } diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 62396af1ebe..35a6fc94753 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -1,7 +1,7 @@ # coding: utf-8 require "spec_helper" -describe Gitlab::Git::Repository, seed_helper: true do +describe Gitlab::Git::Repository, :seed_helper do include Gitlab::EncodingHelper using RSpec::Parameterized::TableSyntax diff --git a/spec/lib/gitlab/git/tag_spec.rb b/spec/lib/gitlab/git/tag_spec.rb index be2f5bfb819..2d9db576a6c 100644 --- a/spec/lib/gitlab/git/tag_spec.rb +++ b/spec/lib/gitlab/git/tag_spec.rb @@ -1,6 +1,6 @@ require "spec_helper" -describe Gitlab::Git::Tag, seed_helper: true do +describe Gitlab::Git::Tag, :seed_helper do let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } shared_examples 'Gitlab::Git::Repository#tags' do diff --git a/spec/lib/gitlab/git/tree_spec.rb b/spec/lib/gitlab/git/tree_spec.rb index 001e406a930..3792d6bf67b 100644 --- a/spec/lib/gitlab/git/tree_spec.rb +++ b/spec/lib/gitlab/git/tree_spec.rb @@ -1,6 +1,6 @@ require "spec_helper" -describe Gitlab::Git::Tree, seed_helper: true do +describe Gitlab::Git::Tree, :seed_helper do let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } context :repo do diff --git a/spec/lib/gitlab/gitaly_client/storage_service_spec.rb b/spec/lib/gitlab/gitaly_client/storage_service_spec.rb new file mode 100644 index 00000000000..6c25e2d6ebd --- /dev/null +++ b/spec/lib/gitlab/gitaly_client/storage_service_spec.rb @@ -0,0 +1,13 @@ +require 'spec_helper' + +describe Gitlab::GitalyClient::StorageService do + describe '#delete_all_repositories' do + let!(:project) { create(:project, :repository) } + + it 'removes all repositories' do + described_class.new(project.repository_storage).delete_all_repositories + + expect(project.repository.exists?).to be(false) + end + end +end diff --git a/spec/lib/gitlab/import_export/file_importer_object_storage_spec.rb b/spec/lib/gitlab/import_export/file_importer_object_storage_spec.rb new file mode 100644 index 00000000000..287745eb40e --- /dev/null +++ b/spec/lib/gitlab/import_export/file_importer_object_storage_spec.rb @@ -0,0 +1,89 @@ +require 'spec_helper' + +describe Gitlab::ImportExport::FileImporter do + let(:shared) { Gitlab::ImportExport::Shared.new(nil) } + let(:storage_path) { "#{Dir.tmpdir}/file_importer_spec" } + let(:valid_file) { "#{shared.export_path}/valid.json" } + let(:symlink_file) { "#{shared.export_path}/invalid.json" } + let(:hidden_symlink_file) { "#{shared.export_path}/.hidden" } + let(:subfolder_symlink_file) { "#{shared.export_path}/subfolder/invalid.json" } + let(:evil_symlink_file) { "#{shared.export_path}/.\nevil" } + + before do + stub_const('Gitlab::ImportExport::FileImporter::MAX_RETRIES', 0) + stub_feature_flags(import_export_object_storage: true) + stub_uploads_object_storage(FileUploader) + + allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(storage_path) + allow_any_instance_of(Gitlab::ImportExport::CommandLineUtil).to receive(:untar_zxf).and_return(true) + allow_any_instance_of(Gitlab::ImportExport::Shared).to receive(:relative_archive_path).and_return('test') + allow(SecureRandom).to receive(:hex).and_return('abcd') + setup_files + end + + after do + FileUtils.rm_rf(storage_path) + end + + context 'normal run' do + before do + described_class.import(project: build(:project), archive_file: '', shared: shared) + end + + it 'removes symlinks in root folder' do + expect(File.exist?(symlink_file)).to be false + end + + it 'removes hidden symlinks in root folder' do + expect(File.exist?(hidden_symlink_file)).to be false + end + + it 'removes evil symlinks in root folder' do + expect(File.exist?(evil_symlink_file)).to be false + end + + it 'removes symlinks in subfolders' do + expect(File.exist?(subfolder_symlink_file)).to be false + end + + it 'does not remove a valid file' do + expect(File.exist?(valid_file)).to be true + end + + it 'creates the file in the right subfolder' do + expect(shared.export_path).to include('test/abcd') + end + end + + context 'error' do + before do + allow_any_instance_of(described_class).to receive(:wait_for_archived_file).and_raise(StandardError) + described_class.import(project: build(:project), archive_file: '', shared: shared) + end + + it 'removes symlinks in root folder' do + expect(File.exist?(symlink_file)).to be false + end + + it 'removes hidden symlinks in root folder' do + expect(File.exist?(hidden_symlink_file)).to be false + end + + it 'removes symlinks in subfolders' do + expect(File.exist?(subfolder_symlink_file)).to be false + end + + it 'does not remove a valid file' do + expect(File.exist?(valid_file)).to be true + end + end + + def setup_files + FileUtils.mkdir_p("#{shared.export_path}/subfolder/") + FileUtils.touch(valid_file) + FileUtils.ln_s(valid_file, symlink_file) + FileUtils.ln_s(valid_file, subfolder_symlink_file) + FileUtils.ln_s(valid_file, hidden_symlink_file) + FileUtils.ln_s(valid_file, evil_symlink_file) + end +end diff --git a/spec/lib/gitlab/import_export/file_importer_spec.rb b/spec/lib/gitlab/import_export/file_importer_spec.rb index 265937f899e..78fccdf1dfc 100644 --- a/spec/lib/gitlab/import_export/file_importer_spec.rb +++ b/spec/lib/gitlab/import_export/file_importer_spec.rb @@ -24,7 +24,7 @@ describe Gitlab::ImportExport::FileImporter do context 'normal run' do before do - described_class.import(archive_file: '', shared: shared) + described_class.import(project: nil, archive_file: '', shared: shared) end it 'removes symlinks in root folder' do @@ -55,7 +55,7 @@ describe Gitlab::ImportExport::FileImporter do context 'error' do before do allow_any_instance_of(described_class).to receive(:wait_for_archived_file).and_raise(StandardError) - described_class.import(archive_file: '', shared: shared) + described_class.import(project: nil, archive_file: '', shared: shared) end it 'removes symlinks in root folder' do diff --git a/spec/lib/gitlab/import_export/importer_object_storage_spec.rb b/spec/lib/gitlab/import_export/importer_object_storage_spec.rb new file mode 100644 index 00000000000..24a994b3611 --- /dev/null +++ b/spec/lib/gitlab/import_export/importer_object_storage_spec.rb @@ -0,0 +1,115 @@ +require 'spec_helper' + +describe Gitlab::ImportExport::Importer do + let(:user) { create(:user) } + let(:test_path) { "#{Dir.tmpdir}/importer_spec" } + let(:shared) { project.import_export_shared } + let(:project) { create(:project) } + let(:import_file) { fixture_file_upload('spec/features/projects/import_export/test_project_export.tar.gz') } + + subject(:importer) { described_class.new(project) } + + before do + allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(test_path) + allow_any_instance_of(Gitlab::ImportExport::FileImporter).to receive(:remove_import_file) + stub_feature_flags(import_export_object_storage: true) + stub_uploads_object_storage(FileUploader) + + FileUtils.mkdir_p(shared.export_path) + ImportExportUpload.create(project: project, import_file: import_file) + end + + after do + FileUtils.rm_rf(test_path) + end + + describe '#execute' do + it 'succeeds' do + importer.execute + + expect(shared.errors).to be_empty + end + + it 'extracts the archive' do + expect(Gitlab::ImportExport::FileImporter).to receive(:import).and_call_original + + importer.execute + end + + it 'checks the version' do + expect(Gitlab::ImportExport::VersionChecker).to receive(:check!).and_call_original + + importer.execute + end + + context 'all restores are executed' do + [ + Gitlab::ImportExport::AvatarRestorer, + Gitlab::ImportExport::RepoRestorer, + Gitlab::ImportExport::WikiRestorer, + Gitlab::ImportExport::UploadsRestorer, + Gitlab::ImportExport::LfsRestorer, + Gitlab::ImportExport::StatisticsRestorer + ].each do |restorer| + it "calls the #{restorer}" do + fake_restorer = double(restorer.to_s) + + expect(fake_restorer).to receive(:restore).and_return(true).at_least(1) + expect(restorer).to receive(:new).and_return(fake_restorer).at_least(1) + + importer.execute + end + end + + it 'restores the ProjectTree' do + expect(Gitlab::ImportExport::ProjectTreeRestorer).to receive(:new).and_call_original + + importer.execute + end + + it 'removes the import file' do + expect(importer).to receive(:remove_import_file).and_call_original + + importer.execute + + expect(project.import_export_upload.import_file&.file).to be_nil + end + end + + context 'when project successfully restored' do + let!(:existing_project) { create(:project, namespace: user.namespace) } + let(:project) { create(:project, namespace: user.namespace, name: 'whatever', path: 'whatever') } + + before do + restorers = double(:restorers, all?: true) + + allow(subject).to receive(:import_file).and_return(true) + allow(subject).to receive(:check_version!).and_return(true) + allow(subject).to receive(:restorers).and_return(restorers) + allow(project).to receive(:import_data).and_return(double(data: { 'original_path' => existing_project.path })) + end + + context 'when import_data' do + context 'has original_path' do + it 'overwrites existing project' do + expect_any_instance_of(::Projects::OverwriteProjectService).to receive(:execute).with(existing_project) + + subject.execute + end + end + + context 'has not original_path' do + before do + allow(project).to receive(:import_data).and_return(double(data: {})) + end + + it 'does not call the overwrite service' do + expect_any_instance_of(::Projects::OverwriteProjectService).not_to receive(:execute).with(existing_project) + + subject.execute + end + end + end + end + end +end diff --git a/spec/lib/gitlab/import_export/importer_spec.rb b/spec/lib/gitlab/import_export/importer_spec.rb index c074e61da26..f07946824c4 100644 --- a/spec/lib/gitlab/import_export/importer_spec.rb +++ b/spec/lib/gitlab/import_export/importer_spec.rb @@ -10,9 +10,10 @@ describe Gitlab::ImportExport::Importer do before do allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(test_path) + allow_any_instance_of(Gitlab::ImportExport::FileImporter).to receive(:remove_import_file) + FileUtils.mkdir_p(shared.export_path) FileUtils.cp(Rails.root.join('spec/features/projects/import_export/test_project_export.tar.gz'), test_path) - allow(subject).to receive(:remove_import_file) end after do @@ -69,7 +70,7 @@ describe Gitlab::ImportExport::Importer do let(:project) { create(:project, namespace: user.namespace, name: 'whatever', path: 'whatever') } before do - restorers = double + restorers = double(:restorers, all?: true) allow(subject).to receive(:import_file).and_return(true) allow(subject).to receive(:check_version!).and_return(true) diff --git a/spec/lib/gitlab/kubernetes/helm/install_command_spec.rb b/spec/lib/gitlab/kubernetes/helm/install_command_spec.rb index 25c6fa3b9a3..cd456a45287 100644 --- a/spec/lib/gitlab/kubernetes/helm/install_command_spec.rb +++ b/spec/lib/gitlab/kubernetes/helm/install_command_spec.rb @@ -14,7 +14,7 @@ describe Gitlab::Kubernetes::Helm::InstallCommand do let(:commands) do <<~EOS helm init --client-only >/dev/null - helm install #{application.chart} --name #{application.name} --namespace #{namespace} -f /data/helm/#{application.name}/config/values.yaml >/dev/null + helm install #{application.chart} --name #{application.name} --version #{application.version} --namespace #{namespace} -f /data/helm/#{application.name}/config/values.yaml >/dev/null EOS end end @@ -42,7 +42,7 @@ describe Gitlab::Kubernetes::Helm::InstallCommand do <<~EOS helm init --client-only >/dev/null helm repo add #{application.name} #{application.repository} - helm install #{application.chart} --name #{application.name} --namespace #{namespace} -f /data/helm/#{application.name}/config/values.yaml >/dev/null + helm install #{application.chart} --name #{application.name} --version #{application.version} --namespace #{namespace} -f /data/helm/#{application.name}/config/values.yaml >/dev/null EOS end end @@ -56,7 +56,7 @@ describe Gitlab::Kubernetes::Helm::InstallCommand do <<~EOS helm init --client-only >/dev/null helm repo add #{application.name} #{application.repository} - helm install #{application.chart} --name #{application.name} --namespace #{namespace} -f /data/helm/#{application.name}/config/values.yaml >/dev/null + helm install #{application.chart} --name #{application.name} --version #{application.version} --namespace #{namespace} -f /data/helm/#{application.name}/config/values.yaml >/dev/null EOS end end diff --git a/spec/migrations/add_foreign_key_from_notification_settings_to_users_spec.rb b/spec/migrations/add_foreign_key_from_notification_settings_to_users_spec.rb new file mode 100644 index 00000000000..656d4f75e3b --- /dev/null +++ b/spec/migrations/add_foreign_key_from_notification_settings_to_users_spec.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require 'spec_helper' +require Rails.root.join('db', 'migrate', '20180710162338_add_foreign_key_from_notification_settings_to_users.rb') + +describe AddForeignKeyFromNotificationSettingsToUsers, :migration do + let(:notification_settings) { table(:notification_settings) } + let(:users) { table(:users) } + let(:projects) { table(:projects) } + + before do + users.create!(email: 'email@email.com', name: 'foo', username: 'foo', projects_limit: 0) + projects.create!(name: 'gitlab', path: 'gitlab-org/gitlab-ce', namespace_id: 1) + end + + describe 'removal of orphans without user' do + let!(:notification_setting_without_user) { create_notification_settings!(user_id: 123) } + let!(:notification_setting_with_user) { create_notification_settings!(user_id: users.last.id) } + + it 'removes orphaned notification_settings without user' do + expect { migrate! }.to change { notification_settings.count }.by(-1) + end + + it "doesn't remove notification_settings with valid user" do + expect { migrate! }.not_to change { notification_setting_with_user.reload } + end + end + + def create_notification_settings!(**opts) + notification_settings.create!( + source_id: projects.last.id, + source_type: 'Project', + user_id: users.last.id, + **opts) + end +end diff --git a/spec/models/clusters/applications/ingress_spec.rb b/spec/models/clusters/applications/ingress_spec.rb index bb5b2ef3a47..d378248d5d6 100644 --- a/spec/models/clusters/applications/ingress_spec.rb +++ b/spec/models/clusters/applications/ingress_spec.rb @@ -23,6 +23,20 @@ describe Clusters::Applications::Ingress do it { is_expected.to contain_exactly(cluster) } end + describe '#make_installing!' do + before do + application.make_installing! + end + + context 'application install previously errored with older version' do + let(:application) { create(:clusters_applications_ingress, :scheduled, version: '0.22.0') } + + it 'updates the application version' do + expect(application.reload.version).to eq('0.23.0') + end + end + end + describe '#make_installed!' do before do application.make_installed! @@ -73,9 +87,17 @@ describe Clusters::Applications::Ingress do it 'should be initialized with ingress arguments' do expect(subject.name).to eq('ingress') expect(subject.chart).to eq('stable/nginx-ingress') - expect(subject.version).to be_nil + expect(subject.version).to eq('0.23.0') expect(subject.values).to eq(ingress.values) end + + context 'application failed to install previously' do + let(:ingress) { create(:clusters_applications_ingress, :errored, version: 'nginx') } + + it 'should be initialized with the locked version' do + expect(subject.version).to eq('0.23.0') + end + end end describe '#values' do diff --git a/spec/models/clusters/applications/jupyter_spec.rb b/spec/models/clusters/applications/jupyter_spec.rb index 65750141e65..e0d57ac65f7 100644 --- a/spec/models/clusters/applications/jupyter_spec.rb +++ b/spec/models/clusters/applications/jupyter_spec.rb @@ -25,6 +25,20 @@ describe Clusters::Applications::Jupyter do end end + describe '#make_installing!' do + before do + application.make_installing! + end + + context 'application install previously errored with older version' do + let(:application) { create(:clusters_applications_jupyter, :scheduled, version: 'v0.5') } + + it 'updates the application version' do + expect(application.reload.version).to eq('v0.6') + end + end + end + describe '#install_command' do let!(:ingress) { create(:clusters_applications_ingress, :installed, external_ip: '127.0.0.1') } let!(:jupyter) { create(:clusters_applications_jupyter, cluster: ingress.cluster) } @@ -36,10 +50,18 @@ describe Clusters::Applications::Jupyter do it 'should be initialized with 4 arguments' do expect(subject.name).to eq('jupyter') expect(subject.chart).to eq('jupyter/jupyterhub') - expect(subject.version).to be_nil + expect(subject.version).to eq('v0.6') expect(subject.repository).to eq('https://jupyterhub.github.io/helm-chart/') expect(subject.values).to eq(jupyter.values) end + + context 'application failed to install previously' do + let(:jupyter) { create(:clusters_applications_jupyter, :errored, version: '0.0.1') } + + it 'should be initialized with the locked version' do + expect(subject.version).to eq('v0.6') + end + end end describe '#values' do diff --git a/spec/models/clusters/applications/prometheus_spec.rb b/spec/models/clusters/applications/prometheus_spec.rb index e4b61552033..3812c65b3b6 100644 --- a/spec/models/clusters/applications/prometheus_spec.rb +++ b/spec/models/clusters/applications/prometheus_spec.rb @@ -16,6 +16,20 @@ describe Clusters::Applications::Prometheus do it { is_expected.to contain_exactly(cluster) } end + describe '#make_installing!' do + before do + application.make_installing! + end + + context 'application install previously errored with older version' do + let(:application) { create(:clusters_applications_prometheus, :scheduled, version: '6.7.2') } + + it 'updates the application version' do + expect(application.reload.version).to eq('6.7.3') + end + end + end + describe 'transition to installed' do let(:project) { create(:project) } let(:cluster) { create(:cluster, projects: [project]) } @@ -155,6 +169,14 @@ describe Clusters::Applications::Prometheus do expect(command.version).to eq('6.7.3') expect(command.values).to eq(prometheus.values) end + + context 'application failed to install previously' do + let(:prometheus) { create(:clusters_applications_prometheus, :errored, version: '2.0.0') } + + it 'should be initialized with the locked version' do + expect(subject.version).to eq('6.7.3') + end + end end describe '#values' do diff --git a/spec/models/clusters/applications/runner_spec.rb b/spec/models/clusters/applications/runner_spec.rb index b12500d0acd..526300755b5 100644 --- a/spec/models/clusters/applications/runner_spec.rb +++ b/spec/models/clusters/applications/runner_spec.rb @@ -8,6 +8,20 @@ describe Clusters::Applications::Runner do it { is_expected.to belong_to(:runner) } + describe '#make_installing!' do + before do + application.make_installing! + end + + context 'application install previously errored with older version' do + let(:application) { create(:clusters_applications_runner, :scheduled, version: '0.1.30') } + + it 'updates the application version' do + expect(application.reload.version).to eq('0.1.31') + end + end + end + describe '.installed' do subject { described_class.installed } @@ -31,10 +45,18 @@ describe Clusters::Applications::Runner do it 'should be initialized with 4 arguments' do expect(subject.name).to eq('runner') expect(subject.chart).to eq('runner/gitlab-runner') - expect(subject.version).to be_nil + expect(subject.version).to eq('0.1.31') expect(subject.repository).to eq('https://charts.gitlab.io') expect(subject.values).to eq(gitlab_runner.values) end + + context 'application failed to install previously' do + let(:gitlab_runner) { create(:clusters_applications_runner, :errored, runner: ci_runner, version: '0.1.13') } + + it 'should be initialized with the locked version' do + expect(subject.version).to eq('0.1.31') + end + end end describe '#values' do diff --git a/spec/models/deploy_token_spec.rb b/spec/models/deploy_token_spec.rb index cd84a684fec..3435f93c999 100644 --- a/spec/models/deploy_token_spec.rb +++ b/spec/models/deploy_token_spec.rb @@ -74,6 +74,14 @@ describe DeployToken do expect(deploy_token.active?).to be_falsy end end + + context "when it hasn't been revoked and has no expiry" do + let(:deploy_token) { create(:deploy_token, expires_at: nil) } + + it 'should return true' do + expect(deploy_token.active?).to be_truthy + end + end end describe '#username' do diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb index 0aee78ac12d..90cce826b6c 100644 --- a/spec/models/merge_request_diff_spec.rb +++ b/spec/models/merge_request_diff_spec.rb @@ -127,6 +127,13 @@ describe MergeRequestDiff do expect(diffs.map(&:new_path)).to contain_exactly('files/ruby/popen.rb') end + it 'only serializes diff files found by query' do + expect(diff_with_commits.merge_request_diff_files.count).to be > 10 + expect_any_instance_of(MergeRequestDiffFile).to receive(:to_hash).once + + diffs + end + it 'uses the diffs from the DB' do expect(diff_with_commits).to receive(:load_diffs) diff --git a/spec/requests/api/project_import_spec.rb b/spec/requests/api/project_import_spec.rb index 55332f56508..e3fb6cecce9 100644 --- a/spec/requests/api/project_import_spec.rb +++ b/spec/requests/api/project_import_spec.rb @@ -7,6 +7,8 @@ describe API::ProjectImport do let(:namespace) { create(:group) } before do allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path) + stub_feature_flags(import_export_object_storage: true) + stub_uploads_object_storage(FileUploader) namespace.add_owner(user) end diff --git a/spec/services/projects/gitlab_projects_import_service_spec.rb b/spec/services/projects/gitlab_projects_import_service_spec.rb index a2061127698..b5f2c826c97 100644 --- a/spec/services/projects/gitlab_projects_import_service_spec.rb +++ b/spec/services/projects/gitlab_projects_import_service_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe Projects::GitlabProjectsImportService do set(:namespace) { create(:namespace) } let(:path) { 'test-path' } - let(:file) { fixture_file_upload('spec/fixtures/doc_sample.txt', 'text/plain') } + let(:file) { fixture_file_upload('spec/fixtures/project_export.tar.gz') } let(:overwrite) { false } let(:import_params) { { namespace_id: namespace.id, path: path, file: file, overwrite: overwrite } } diff --git a/spec/services/users/activity_service_spec.rb b/spec/services/users/activity_service_spec.rb index f20849e6924..719b4adf212 100644 --- a/spec/services/users/activity_service_spec.rb +++ b/spec/services/users/activity_service_spec.rb @@ -28,6 +28,18 @@ describe Users::ActivityService do end end + context 'when a bad object is passed' do + let(:fake_object) { double(username: 'hello') } + + it 'does not record activity' do + service = described_class.new(fake_object, 'pull') + + expect(service).not_to receive(:record_activity) + + service.execute + end + end + context 'when last activity is today' do let(:last_activity_on) { Date.today } diff --git a/spec/support/helpers/seed_helper.rb b/spec/support/helpers/seed_helper.rb index 8fd107260cc..25781f5e679 100644 --- a/spec/support/helpers/seed_helper.rb +++ b/spec/support/helpers/seed_helper.rb @@ -101,10 +101,4 @@ bla/bla.txt handle.write('# hello'.encode(enc)) end end - - # Prevent developer git configurations from being persisted to test - # repositories - def git_env - { 'GIT_TEMPLATE_DIR' => '' } - end end diff --git a/spec/support/helpers/test_env.rb b/spec/support/helpers/test_env.rb index e531495d917..8e1d4cfe269 100644 --- a/spec/support/helpers/test_env.rb +++ b/spec/support/helpers/test_env.rb @@ -243,6 +243,14 @@ module TestEnv set_repo_refs(target_repo_path, refs) end + def create_bare_repository(path) + FileUtils.mkdir_p(path) + + system(git_env, *%W(#{Gitlab.config.git.bin_path} -C #{path} init --bare), + out: '/dev/null', + err: '/dev/null') + end + def repos_path @repos_path ||= Gitlab.config.repositories.storages[REPOS_STORAGE].legacy_disk_path end diff --git a/spec/support/shared_examples/helm_generated_script.rb b/spec/support/shared_examples/helm_generated_script.rb index 05d53a13fd3..ef9bb7f5533 100644 --- a/spec/support/shared_examples/helm_generated_script.rb +++ b/spec/support/shared_examples/helm_generated_script.rb @@ -7,7 +7,7 @@ shared_examples 'helm commands' do echo http://mirror.clarkson.edu/alpine/v$ALPINE_VERSION/main >> /etc/apk/repositories echo http://mirror1.hs-esslingen.de/pub/Mirrors/alpine/v$ALPINE_VERSION/main >> /etc/apk/repositories apk add -U wget ca-certificates openssl >/dev/null - wget -q -O - https://kubernetes-helm.storage.googleapis.com/helm-v2.7.0-linux-amd64.tar.gz | tar zxC /tmp >/dev/null + wget -q -O - https://kubernetes-helm.storage.googleapis.com/helm-v2.7.2-linux-amd64.tar.gz | tar zxC /tmp >/dev/null mv /tmp/linux-amd64/helm /usr/bin/ EOS end diff --git a/spec/support/stored_repositories.rb b/spec/support/stored_repositories.rb index 21995c89a6e..26f823cb6ef 100644 --- a/spec/support/stored_repositories.rb +++ b/spec/support/stored_repositories.rb @@ -1,8 +1,4 @@ RSpec.configure do |config| - config.before(:each, :repository) do - TestEnv.clean_test_path - end - config.before(:all, :broken_storage) do FileUtils.rm_rf Gitlab.config.repositories.storages.broken.legacy_disk_path end diff --git a/spec/tasks/gitlab/backup_rake_spec.rb b/spec/tasks/gitlab/backup_rake_spec.rb index 93a436cb2b5..3ba6caf1337 100644 --- a/spec/tasks/gitlab/backup_rake_spec.rb +++ b/spec/tasks/gitlab/backup_rake_spec.rb @@ -87,6 +87,27 @@ describe 'gitlab:app namespace rake task' do expect { run_rake_task('gitlab:backup:restore') }.to output.to_stdout end end + + context 'when the restore directory is not empty' do + before do + # We only need a backup of the repositories for this test + stub_env('SKIP', 'db,uploads,builds,artifacts,lfs,registry') + end + + it 'removes stale data' do + expect { run_rake_task('gitlab:backup:create') }.to output.to_stdout + + excluded_project = create(:project, :repository, name: 'mepmep') + + expect { run_rake_task('gitlab:backup:restore') }.to output.to_stdout + + raw_repo = excluded_project.repository.raw + + # The restore will not find the repository in the backup, but will create + # an empty one in its place + expect(raw_repo.empty?).to be(true) + end + end end # backup_restore task describe 'backup' do diff --git a/spec/tasks/gitlab/cleanup_rake_spec.rb b/spec/tasks/gitlab/cleanup_rake_spec.rb index ba08ece1b4b..cc2cca10f58 100644 --- a/spec/tasks/gitlab/cleanup_rake_spec.rb +++ b/spec/tasks/gitlab/cleanup_rake_spec.rb @@ -68,317 +68,86 @@ describe 'gitlab:cleanup rake tasks' do end end + # A single integration test that is redundant with one part of the + # Gitlab::Cleanup::ProjectUploads spec. + # + # Additionally, this tests DRY_RUN env var values, and the extra line of + # output that says you can disable DRY_RUN if it's enabled. describe 'cleanup:project_uploads' do - context 'orphaned project upload file' do - context 'when an upload record matching the secret and filename is found' do - context 'when the project is still in legacy storage' do - let!(:orphaned) { create(:upload, :issuable_upload, :with_file, model: build(:project, :legacy_storage)) } - let!(:correct_path) { orphaned.absolute_path } - let!(:other_project) { create(:project, :legacy_storage) } - let!(:orphaned_path) { correct_path.sub(/#{orphaned.model.full_path}/, other_project.full_path) } + let!(:logger) { double(:logger) } - before do - FileUtils.mkdir_p(File.dirname(orphaned_path)) - FileUtils.mv(correct_path, orphaned_path) - end - - it 'moves the file to its proper location' do - expect(Rails.logger).to receive(:info).twice - expect(Rails.logger).to receive(:info).with("Did fix #{orphaned_path} -> #{correct_path}") - - expect(File.exist?(orphaned_path)).to be_truthy - expect(File.exist?(correct_path)).to be_falsey - - stub_env('DRY_RUN', 'false') - run_rake_task('gitlab:cleanup:project_uploads') - - expect(File.exist?(orphaned_path)).to be_falsey - expect(File.exist?(correct_path)).to be_truthy - end - - it 'a dry run does not move the file' do - expect(Rails.logger).to receive(:info).twice - expect(Rails.logger).to receive(:info).with("Can fix #{orphaned_path} -> #{correct_path}") - expect(Rails.logger).to receive(:info) - - expect(File.exist?(orphaned_path)).to be_truthy - expect(File.exist?(correct_path)).to be_falsey - - run_rake_task('gitlab:cleanup:project_uploads') - - expect(File.exist?(orphaned_path)).to be_truthy - expect(File.exist?(correct_path)).to be_falsey - end - - context 'when the project record is missing (Upload#absolute_path raises error)' do - let!(:lost_and_found_path) { File.join(FileUploader.root, '-', 'project-lost-found', other_project.full_path, orphaned.path) } - - before do - orphaned.model.delete - end - - it 'moves the file to lost and found' do - expect(Rails.logger).to receive(:info).twice - expect(Rails.logger).to receive(:info).with("Did move to lost and found #{orphaned_path} -> #{lost_and_found_path}") - - expect(File.exist?(orphaned_path)).to be_truthy - expect(File.exist?(lost_and_found_path)).to be_falsey - - stub_env('DRY_RUN', 'false') - run_rake_task('gitlab:cleanup:project_uploads') - - expect(File.exist?(orphaned_path)).to be_falsey - expect(File.exist?(lost_and_found_path)).to be_truthy - end - - it 'a dry run does not move the file' do - expect(Rails.logger).to receive(:info).twice - expect(Rails.logger).to receive(:info).with("Can move to lost and found #{orphaned_path} -> #{lost_and_found_path}") - expect(Rails.logger).to receive(:info) - - expect(File.exist?(orphaned_path)).to be_truthy - expect(File.exist?(lost_and_found_path)).to be_falsey - - run_rake_task('gitlab:cleanup:project_uploads') - - expect(File.exist?(orphaned_path)).to be_truthy - expect(File.exist?(lost_and_found_path)).to be_falsey - end - end - end - - context 'when the project was moved to hashed storage' do - let!(:orphaned) { create(:upload, :issuable_upload, :with_file) } - let!(:correct_path) { orphaned.absolute_path } - let!(:orphaned_path) { File.join(FileUploader.root, 'foo', 'bar', orphaned.path) } - - before do - FileUtils.mkdir_p(File.dirname(orphaned_path)) - FileUtils.mv(correct_path, orphaned_path) - end - - it 'moves the file to its proper location' do - expect(Rails.logger).to receive(:info).twice - expect(Rails.logger).to receive(:info).with("Did fix #{orphaned_path} -> #{correct_path}") - - expect(File.exist?(orphaned_path)).to be_truthy - expect(File.exist?(correct_path)).to be_falsey - - stub_env('DRY_RUN', 'false') - run_rake_task('gitlab:cleanup:project_uploads') - - expect(File.exist?(orphaned_path)).to be_falsey - expect(File.exist?(correct_path)).to be_truthy - end - - it 'a dry run does not move the file' do - expect(Rails.logger).to receive(:info).twice - expect(Rails.logger).to receive(:info).with("Can fix #{orphaned_path} -> #{correct_path}") - expect(Rails.logger).to receive(:info) + before do + expect(main_object).to receive(:logger).and_return(logger).at_least(1).times - expect(File.exist?(orphaned_path)).to be_truthy - expect(File.exist?(correct_path)).to be_falsey + allow(logger).to receive(:info).at_least(1).times + allow(logger).to receive(:debug).at_least(1).times + end - run_rake_task('gitlab:cleanup:project_uploads') + context 'with a fixable orphaned project upload file' do + let(:orphaned) { create(:upload, :issuable_upload, :with_file, model: build(:project, :legacy_storage)) } + let(:new_path) { orphaned.absolute_path } + let(:path) { File.join(FileUploader.root, 'some', 'wrong', 'location', orphaned.path) } - expect(File.exist?(orphaned_path)).to be_truthy - expect(File.exist?(correct_path)).to be_falsey - end - end + before do + FileUtils.mkdir_p(File.dirname(path)) + FileUtils.mv(new_path, path) end - context 'when a matching upload record can not be found' do - context 'when the file path fits the known pattern' do - let!(:orphaned) { create(:upload, :issuable_upload, :with_file, model: build(:project, :legacy_storage)) } - let!(:orphaned_path) { orphaned.absolute_path } - let!(:lost_and_found_path) { File.join(FileUploader.root, '-', 'project-lost-found', orphaned.model.full_path, orphaned.path) } - - before do - orphaned.delete - end - - it 'moves the file to lost and found' do - expect(Rails.logger).to receive(:info).twice - expect(Rails.logger).to receive(:info).with("Did move to lost and found #{orphaned_path} -> #{lost_and_found_path}") - - expect(File.exist?(orphaned_path)).to be_truthy - expect(File.exist?(lost_and_found_path)).to be_falsey - - stub_env('DRY_RUN', 'false') - run_rake_task('gitlab:cleanup:project_uploads') - - expect(File.exist?(orphaned_path)).to be_falsey - expect(File.exist?(lost_and_found_path)).to be_truthy - end - - it 'a dry run does not move the file' do - expect(Rails.logger).to receive(:info).twice - expect(Rails.logger).to receive(:info).with("Can move to lost and found #{orphaned_path} -> #{lost_and_found_path}") - expect(Rails.logger).to receive(:info) - - expect(File.exist?(orphaned_path)).to be_truthy - expect(File.exist?(lost_and_found_path)).to be_falsey - - run_rake_task('gitlab:cleanup:project_uploads') - - expect(File.exist?(orphaned_path)).to be_truthy - expect(File.exist?(lost_and_found_path)).to be_falsey - end + context 'with DRY_RUN disabled' do + before do + stub_env('DRY_RUN', 'false') end - context 'when the file path does not fit the known pattern' do - let!(:invalid_path) { File.join('group', 'file.jpg') } - let!(:orphaned_path) { File.join(FileUploader.root, invalid_path) } - let!(:lost_and_found_path) { File.join(FileUploader.root, '-', 'project-lost-found', invalid_path) } - - before do - FileUtils.mkdir_p(File.dirname(orphaned_path)) - FileUtils.touch(orphaned_path) - end - - after do - File.delete(orphaned_path) if File.exist?(orphaned_path) - end - - it 'moves the file to lost and found' do - expect(Rails.logger).to receive(:info).twice - expect(Rails.logger).to receive(:info).with("Did move to lost and found #{orphaned_path} -> #{lost_and_found_path}") - - expect(File.exist?(orphaned_path)).to be_truthy - expect(File.exist?(lost_and_found_path)).to be_falsey - - stub_env('DRY_RUN', 'false') - run_rake_task('gitlab:cleanup:project_uploads') - - expect(File.exist?(orphaned_path)).to be_falsey - expect(File.exist?(lost_and_found_path)).to be_truthy - end - - it 'a dry run does not move the file' do - expect(Rails.logger).to receive(:info).twice - expect(Rails.logger).to receive(:info).with("Can move to lost and found #{orphaned_path} -> #{lost_and_found_path}") - expect(Rails.logger).to receive(:info) - - expect(File.exist?(orphaned_path)).to be_truthy - expect(File.exist?(lost_and_found_path)).to be_falsey - - run_rake_task('gitlab:cleanup:project_uploads') + it 'moves the file to its proper location' do + run_rake_task('gitlab:cleanup:project_uploads') - expect(File.exist?(orphaned_path)).to be_truthy - expect(File.exist?(lost_and_found_path)).to be_falsey - end + expect(File.exist?(path)).to be_falsey + expect(File.exist?(new_path)).to be_truthy end - end - end - - context 'non-orphaned project upload file' do - it 'does not move the file' do - tracked = create(:upload, :issuable_upload, :with_file, model: build(:project, :legacy_storage)) - tracked_path = tracked.absolute_path - expect(Rails.logger).not_to receive(:info).with(/move|fix/i) - expect(File.exist?(tracked_path)).to be_truthy - - stub_env('DRY_RUN', 'false') - run_rake_task('gitlab:cleanup:project_uploads') - - expect(File.exist?(tracked_path)).to be_truthy - end - end - - context 'ignorable cases' do - shared_examples_for 'does not move anything' do - it 'does not move even an orphan file' do - orphaned = create(:upload, :issuable_upload, :with_file, model: project) - orphaned_path = orphaned.absolute_path - orphaned.delete - - expect(File.exist?(orphaned_path)).to be_truthy + it 'logs action as done' do + expect(logger).to receive(:info).with("Looking for orphaned project uploads to clean up...") + expect(logger).to receive(:info).with("Did fix #{path} -> #{new_path}") run_rake_task('gitlab:cleanup:project_uploads') - - expect(File.exist?(orphaned_path)).to be_truthy end end - # Because we aren't concerned about these, and can save a lot of - # processing time by ignoring them. If we wish to cleanup hashed storage - # directories, it should simply require removing this test and modifying - # the find command. - context 'when the file is already in hashed storage' do - let(:project) { create(:project) } + shared_examples_for 'does not move the file' do + it 'does not move the file' do + run_rake_task('gitlab:cleanup:project_uploads') - before do - stub_env('DRY_RUN', 'false') - expect(Rails.logger).not_to receive(:info).with(/move|fix/i) + expect(File.exist?(path)).to be_truthy + expect(File.exist?(new_path)).to be_falsey end - it_behaves_like 'does not move anything' - end - - context 'when DRY_RUN env var is unset' do - let(:project) { create(:project, :legacy_storage) } + it 'logs action as able to be done' do + expect(logger).to receive(:info).with("Looking for orphaned project uploads to clean up. Dry run...") + expect(logger).to receive(:info).with("Can fix #{path} -> #{new_path}") + expect(logger).to receive(:info).with(/To clean up these files run this command with DRY_RUN=false/) - it_behaves_like 'does not move anything' + run_rake_task('gitlab:cleanup:project_uploads') + end end - context 'when DRY_RUN env var is true' do - let(:project) { create(:project, :legacy_storage) } - + context 'with DRY_RUN explicitly enabled' do before do stub_env('DRY_RUN', 'true') end - it_behaves_like 'does not move anything' + it_behaves_like 'does not move the file' end - context 'when DRY_RUN env var is foo' do - let(:project) { create(:project, :legacy_storage) } - + context 'with DRY_RUN set to an unknown value' do before do stub_env('DRY_RUN', 'foo') end - it_behaves_like 'does not move anything' + it_behaves_like 'does not move the file' end - it 'does not move any non-project (FileUploader) uploads' do - stub_env('DRY_RUN', 'false') - - paths = [] - orphaned1 = create(:upload, :personal_snippet_upload, :with_file) - orphaned2 = create(:upload, :namespace_upload, :with_file) - orphaned3 = create(:upload, :attachment_upload, :with_file) - paths << orphaned1.absolute_path - paths << orphaned2.absolute_path - paths << orphaned3.absolute_path - Upload.delete_all - - expect(Rails.logger).not_to receive(:info).with(/move|fix/i) - paths.each do |path| - expect(File.exist?(path)).to be_truthy - end - - run_rake_task('gitlab:cleanup:project_uploads') - - paths.each do |path| - expect(File.exist?(path)).to be_truthy - end - end - - it 'does not move any uploads in tmp (which would interfere with ongoing upload activity)' do - stub_env('DRY_RUN', 'false') - - path = File.join(FileUploader.root, 'tmp', 'foo.jpg') - FileUtils.mkdir_p(File.dirname(path)) - FileUtils.touch(path) - - expect(Rails.logger).not_to receive(:info).with(/move|fix/i) - expect(File.exist?(path)).to be_truthy - - run_rake_task('gitlab:cleanup:project_uploads') - - expect(File.exist?(path)).to be_truthy + context 'with DRY_RUN unset' do + it_behaves_like 'does not move the file' end end end diff --git a/spec/workers/create_gpg_signature_worker_spec.rb b/spec/workers/create_gpg_signature_worker_spec.rb index 502765e9786..f5479e57260 100644 --- a/spec/workers/create_gpg_signature_worker_spec.rb +++ b/spec/workers/create_gpg_signature_worker_spec.rb @@ -4,10 +4,9 @@ describe CreateGpgSignatureWorker do let(:project) { create(:project, :repository) } let(:commits) { project.repository.commits('HEAD', limit: 3).commits } let(:commit_shas) { commits.map(&:id) } + let(:gpg_commit) { instance_double(Gitlab::Gpg::Commit) } context 'when GpgKey is found' do - let(:gpg_commit) { instance_double(Gitlab::Gpg::Commit) } - before do allow(Project).to receive(:find_by).with(id: project.id).and_return(project) allow(project).to receive(:commits_by).with(oids: commit_shas).and_return(commits) @@ -36,6 +35,16 @@ describe CreateGpgSignatureWorker do end end + context 'handles when a string is passed in for the commit SHA' do + it 'creates a signature once' do + allow(Gitlab::Gpg::Commit).to receive(:new).with(commits.first).and_return(gpg_commit) + + expect(gpg_commit).to receive(:signature).once + + described_class.new.perform(commit_shas.first, project.id) + end + end + context 'when Commit is not found' do let(:nonexisting_commit_sha) { '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a34' } |