summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGitLab Bot <gitlab-bot@gitlab.com>2021-01-27 18:09:08 +0000
committerGitLab Bot <gitlab-bot@gitlab.com>2021-01-27 18:09:08 +0000
commit874d5e8019150bfb2428cac3089c1dac1d6f9ed9 (patch)
tree3a1d9b1828222fe16a5ca9071da34598c7280d50
parent0cbb4a75699e1ab6a0cb704b551e672e09192377 (diff)
downloadgitlab-ce-874d5e8019150bfb2428cac3089c1dac1d6f9ed9.tar.gz
Add latest changes from gitlab-org/gitlab@master
-rw-r--r--.gitlab/ci/rails.gitlab-ci.yml11
-rw-r--r--.gitlab/ci/rules.gitlab-ci.yml7
-rw-r--r--GITALY_SERVER_VERSION2
-rw-r--r--Gemfile2
-rw-r--r--Gemfile.lock4
-rw-r--r--app/assets/javascripts/jobs/components/artifacts_block.vue29
-rw-r--r--app/assets/stylesheets/framework.scss1
-rw-r--r--app/assets/stylesheets/framework/carousel.scss202
-rw-r--r--app/assets/stylesheets/framework/variables.scss21
-rw-r--r--app/controllers/projects/issues_controller.rb3
-rw-r--r--app/controllers/projects/merge_requests/diffs_controller.rb5
-rw-r--r--app/experiments/application_experiment.rb9
-rw-r--r--app/models/merge_request.rb19
-rw-r--r--app/models/merge_request_diff.rb23
-rw-r--r--app/services/merge_requests/mergeability_check_service.rb6
-rw-r--r--app/services/merge_requests/reload_merge_head_diff_service.rb50
-rw-r--r--changelogs/unreleased/292507-batch-merge-ref-diff.yml5
-rw-r--r--config/application.rb1
-rw-r--r--config/feature_flags/development/gitlab_experiments.yml8
-rw-r--r--db/migrate/20210107105306_add_diff_type_to_merge_request_diffs.rb31
-rw-r--r--db/schema_migrations/202101071053061
-rw-r--r--db/structure.sql3
-rw-r--r--doc/administration/pages/index.md14
-rw-r--r--locale/gitlab.pot6
-rw-r--r--qa/qa/specs/features/browser_ui/5_package/nuget_repository_spec.rb115
-rwxr-xr-xscripts/trigger-build93
-rw-r--r--spec/controllers/projects/issues_controller_spec.rb51
-rw-r--r--spec/controllers/projects/merge_requests/diffs_controller_spec.rb8
-rw-r--r--spec/experiments/application_experiment_spec.rb42
-rw-r--r--spec/factories/merge_request_diffs.rb6
-rw-r--r--spec/lib/gitlab/import_export/all_models.yml1
-rw-r--r--spec/lib/gitlab/import_export/safe_model_attributes.yml1
-rw-r--r--spec/models/merge_request_diff_spec.rb45
-rw-r--r--spec/models/merge_request_spec.rb25
-rw-r--r--spec/services/merge_requests/delete_non_latest_diffs_service_spec.rb2
-rw-r--r--spec/services/merge_requests/mergeability_check_service_spec.rb28
-rw-r--r--spec/services/merge_requests/reload_merge_head_diff_service_spec.rb61
-rw-r--r--spec/support/gitlab_experiment.rb3
38 files changed, 588 insertions, 356 deletions
diff --git a/.gitlab/ci/rails.gitlab-ci.yml b/.gitlab/ci/rails.gitlab-ci.yml
index 6de8cdfd9b0..bc21a93f23a 100644
--- a/.gitlab/ci/rails.gitlab-ci.yml
+++ b/.gitlab/ci/rails.gitlab-ci.yml
@@ -376,6 +376,17 @@ db:rollback:
- bundle exec rake db:migrate VERSION=20181228175414
- bundle exec rake db:migrate SKIP_SCHEMA_VERSION_CHECK=true
+db:gitlabcom-database-testing:
+ extends: .rails:rules:db:gitlabcom-database-testing
+ stage: test
+ image: ruby:2.7-alpine
+ needs: []
+ allow_failure: true
+ script:
+ - source scripts/utils.sh
+ - install_gitlab_gem
+ - ./scripts/trigger-build gitlab-com-database-testing
+
gitlab:setup:
extends: .db-job-base
variables:
diff --git a/.gitlab/ci/rules.gitlab-ci.yml b/.gitlab/ci/rules.gitlab-ci.yml
index 5e8cdf0daaf..58fa1101834 100644
--- a/.gitlab/ci/rules.gitlab-ci.yml
+++ b/.gitlab/ci/rules.gitlab-ci.yml
@@ -523,6 +523,13 @@
changes: *db-patterns
- <<: *if-merge-request-title-run-all-rspec
+.rails:rules:db:gitlabcom-database-testing:
+ rules:
+ - if: '$GITLABCOM_DATABASE_TESTING_TRIGGER_TOKEN == null'
+ when: never
+ - <<: *if-merge-request
+ changes: *db-patterns
+
.rails:rules:ee-and-foss-unit:
rules:
- changes: *backend-patterns
diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION
index 262de3bbaa1..a5a1ec362aa 100644
--- a/GITALY_SERVER_VERSION
+++ b/GITALY_SERVER_VERSION
@@ -1 +1 @@
-c38393255d22bf3533ca8b8714f614411f10fc30
+86d933c0a993eabbff4c725da3cff067f371e641
diff --git a/Gemfile b/Gemfile
index b607b155d36..ecb3b77fa1d 100644
--- a/Gemfile
+++ b/Gemfile
@@ -477,7 +477,7 @@ gem 'flipper', '~> 0.17.1'
gem 'flipper-active_record', '~> 0.17.1'
gem 'flipper-active_support_cache_store', '~> 0.17.1'
gem 'unleash', '~> 0.1.5'
-gem 'gitlab-experiment', '~> 0.4.8'
+gem 'gitlab-experiment', '~> 0.4.5'
# Structured logging
gem 'lograge', '~> 0.5'
diff --git a/Gemfile.lock b/Gemfile.lock
index 61dfa7937b3..5e52a732906 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -424,7 +424,7 @@ GEM
github-markup (1.7.0)
gitlab-chronic (0.10.5)
numerizer (~> 0.2)
- gitlab-experiment (0.4.8)
+ gitlab-experiment (0.4.5)
activesupport (>= 3.0)
scientist (~> 1.5, >= 1.5.0)
gitlab-fog-azure-rm (1.0.0)
@@ -1364,7 +1364,7 @@ DEPENDENCIES
gitaly (~> 13.8.0.pre.rc3)
github-markup (~> 1.7.0)
gitlab-chronic (~> 0.10.5)
- gitlab-experiment (~> 0.4.8)
+ gitlab-experiment (~> 0.4.5)
gitlab-fog-azure-rm (~> 1.0)
gitlab-labkit (= 0.14.0)
gitlab-license (~> 1.0)
diff --git a/app/assets/javascripts/jobs/components/artifacts_block.vue b/app/assets/javascripts/jobs/components/artifacts_block.vue
index 2850a8e86fd..0f34926f689 100644
--- a/app/assets/javascripts/jobs/components/artifacts_block.vue
+++ b/app/assets/javascripts/jobs/components/artifacts_block.vue
@@ -1,13 +1,15 @@
<script>
-import { GlIcon, GlLink } from '@gitlab/ui';
+import { GlButton, GlButtonGroup, GlIcon, GlLink } from '@gitlab/ui';
import TimeagoTooltip from '~/vue_shared/components/time_ago_tooltip.vue';
import timeagoMixin from '~/vue_shared/mixins/timeago';
export default {
components: {
- TimeagoTooltip,
+ GlButton,
+ GlButtonGroup,
GlIcon,
GlLink,
+ TimeagoTooltip,
},
mixins: [timeagoMixin],
props: {
@@ -36,7 +38,7 @@ export default {
</script>
<template>
<div class="block">
- <div class="title font-weight-bold">{{ s__('Job|Job artifacts') }}</div>
+ <div class="title gl-font-weight-bold">{{ s__('Job|Job artifacts') }}</div>
<p
v-if="isExpired || willExpire"
class="build-detail-row"
@@ -61,32 +63,29 @@ export default {
)
}}</span>
</p>
- <div class="btn-group d-flex gl-mt-3" role="group">
- <gl-link
+ <gl-button-group class="gl-display-flex gl-mt-3">
+ <gl-button
v-if="artifact.keep_path"
:href="artifact.keep_path"
- class="btn btn-sm btn-default"
data-method="post"
data-testid="keep-artifacts"
- >{{ s__('Job|Keep') }}</gl-link
+ >{{ s__('Job|Keep') }}</gl-button
>
- <gl-link
+ <gl-button
v-if="artifact.download_path"
:href="artifact.download_path"
- class="btn btn-sm btn-default"
- download
rel="nofollow"
data-testid="download-artifacts"
- >{{ s__('Job|Download') }}</gl-link
+ download
+ >{{ s__('Job|Download') }}</gl-button
>
- <gl-link
+ <gl-button
v-if="artifact.browse_path"
:href="artifact.browse_path"
- class="btn btn-sm btn-default"
data-testid="browse-artifacts"
data-qa-selector="browse_artifacts_button"
- >{{ s__('Job|Browse') }}</gl-link
+ >{{ s__('Job|Browse') }}</gl-button
>
- </div>
+ </gl-button-group>
</div>
</template>
diff --git a/app/assets/stylesheets/framework.scss b/app/assets/stylesheets/framework.scss
index e40b95cdce6..7931f4deea0 100644
--- a/app/assets/stylesheets/framework.scss
+++ b/app/assets/stylesheets/framework.scss
@@ -15,7 +15,6 @@
@import 'framework/badges';
@import 'framework/calendar';
@import 'framework/callout';
-@import 'framework/carousel';
@import 'framework/common';
@import 'framework/dropdowns';
@import 'framework/files';
diff --git a/app/assets/stylesheets/framework/carousel.scss b/app/assets/stylesheets/framework/carousel.scss
deleted file mode 100644
index d51a9f9c173..00000000000
--- a/app/assets/stylesheets/framework/carousel.scss
+++ /dev/null
@@ -1,202 +0,0 @@
-// Notes on the classes:
-//
-// 1. .carousel.pointer-event should ideally be pan-y (to allow for users to scroll vertically)
-// even when their scroll action started on a carousel, but for compatibility (with Firefox)
-// we're preventing all actions instead
-// 2. The .carousel-item-left and .carousel-item-right is used to indicate where
-// the active slide is heading.
-// 3. .active.carousel-item is the current slide.
-// 4. .active.carousel-item-left and .active.carousel-item-right is the current
-// slide in its in-transition state. Only one of these occurs at a time.
-// 5. .carousel-item-next.carousel-item-left and .carousel-item-prev.carousel-item-right
-// is the upcoming slide in transition.
-
-.carousel {
- position: relative;
-
- &.pointer-event {
- touch-action: pan-y;
- }
-}
-
-
-.carousel-inner {
- position: relative;
- width: 100%;
- overflow: hidden;
- @include clearfix();
-}
-
-.carousel-item {
- position: relative;
- display: none;
- float: left;
- width: 100%;
- margin-right: -100%;
- backface-visibility: hidden;
- @include transition($carousel-transition);
-}
-
-.carousel-item.active,
-.carousel-item-next,
-.carousel-item-prev {
- display: block;
-}
-
-.carousel-item-next:not(.carousel-item-left),
-.active.carousel-item-right {
- transform: translateX(100%);
-}
-
-.carousel-item-prev:not(.carousel-item-right),
-.active.carousel-item-left {
- transform: translateX(-100%);
-}
-
-
-//
-// Alternate transitions
-//
-
-.carousel-fade {
- .carousel-item {
- opacity: 0;
- transition-property: opacity;
- transform: none;
- }
-
- .carousel-item.active,
- .carousel-item-next.carousel-item-left,
- .carousel-item-prev.carousel-item-right {
- z-index: 1;
- opacity: 1;
- }
-
- .active.carousel-item-left,
- .active.carousel-item-right {
- z-index: 0;
- opacity: 0;
- @include transition(0s $carousel-transition-duration opacity);
- }
-}
-
-
-//
-// Left/right controls for nav
-//
-
-.carousel-control-prev,
-.carousel-control-next {
- position: absolute;
- top: 0;
- bottom: 0;
- z-index: 1;
- // Use flex for alignment (1-3)
- display: flex; // 1. allow flex styles
- align-items: center; // 2. vertically center contents
- justify-content: center; // 3. horizontally center contents
- width: $carousel-control-width;
- color: $carousel-control-color;
- text-align: center;
- opacity: $carousel-control-opacity;
- @include transition($carousel-control-transition);
-
- // Hover/focus state
- @include hover-focus {
- color: $carousel-control-color;
- text-decoration: none;
- outline: 0;
- opacity: $carousel-control-hover-opacity;
- }
-}
-
-.carousel-control-prev {
- left: 0;
- @if $enable-gradients {
- background: linear-gradient(90deg, rgba($black, 0.25), rgba($black, 0.001));
- }
-}
-
-.carousel-control-next {
- right: 0;
- @if $enable-gradients {
- background: linear-gradient(270deg, rgba($black, 0.25), rgba($black, 0.001));
- }
-}
-
-// Icons for within
-.carousel-control-prev-icon,
-.carousel-control-next-icon {
- display: inline-block;
- width: $carousel-control-icon-width;
- height: $carousel-control-icon-width;
- background: no-repeat 50% / 100% 100%;
-}
-
-.carousel-control-prev-icon {
- background-image: $carousel-control-prev-icon-bg;
-}
-
-.carousel-control-next-icon {
- background-image: $carousel-control-next-icon-bg;
-}
-
-
-// Optional indicator pips
-//
-// Add an ordered list with the following class and add a list item for each
-// slide your carousel holds.
-
-.carousel-indicators {
- position: absolute;
- right: 0;
- bottom: 0;
- left: 0;
- z-index: 15;
- display: flex;
- justify-content: center;
- padding-left: 0; // override <ol> default
- // Use the .carousel-control's width as margin so we don't overlay those
- margin-right: $carousel-control-width;
- margin-left: $carousel-control-width;
- list-style: none;
-
- li {
- box-sizing: content-box;
- flex: 0 1 auto;
- width: $carousel-indicator-width;
- height: $carousel-indicator-height;
- margin-right: $carousel-indicator-spacer;
- margin-left: $carousel-indicator-spacer;
- text-indent: -999px;
- cursor: pointer;
- background-color: $carousel-indicator-active-bg;
- background-clip: padding-box;
- // Use transparent borders to increase the hit area by 10px on top and bottom.
- border-top: $carousel-indicator-hit-area-height solid transparent;
- border-bottom: $carousel-indicator-hit-area-height solid transparent;
- opacity: 0.5;
- @include transition($carousel-indicator-transition);
- }
-
- .active {
- opacity: 1;
- }
-}
-
-
-// Optional captions
-//
-//
-
-.carousel-caption {
- position: absolute;
- right: (100% - $carousel-caption-width) / 2;
- bottom: 20px;
- left: (100% - $carousel-caption-width) / 2;
- z-index: 10;
- padding-top: 20px;
- padding-bottom: 20px;
- color: $carousel-caption-color;
- text-align: center;
-}
diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss
index ee0d76b0301..7db172c55a1 100644
--- a/app/assets/stylesheets/framework/variables.scss
+++ b/app/assets/stylesheets/framework/variables.scss
@@ -872,6 +872,27 @@ $add-to-slack-well-max-width: 750px;
$add-to-slack-logo-size: 100px;
/*
+Security & Compliance Carousel
+*/
+$security-and-compliance-carousel-image-carousel-width: 1000px;
+$security-and-compliance-carousel-image-discover-button-width: 45%;
+$security-and-compliance-carousel-image-discover-buttons-max-width: 280px;
+$security-and-compliance-carousel-image-discover-footer-max-width: 500px;
+$security-and-compliance-carousel-image-discover-feedback-width: 30%;
+$security-and-compliance-carousel-image-discover-text-carousel-max-width: 650px;
+$security-and-compliance-carousel-image-discover-text-carousel-caption-height: 100%;
+$security-and-compliance-carousel-image-discover-text-carousel-caption-max-width: 500px;
+$security-and-compliance-carousel-control-icon-width: 10px;
+$security-and-compliance-carousel-control-position: -5%;
+$security-and-compliance-carousel-inner-width: 90%;
+$security-and-compliance-carousel-indicators-bottom: -20px;
+$security-and-compliance-carousel-indicators-bottom-lg: -15px;
+$security-and-compliance-carousel-indicators-dimension: 6px;
+$security-and-compliance-carousel-indicators-border-radius: 100%;
+$security-and-compliance-carousel-prev-icon-background: url("data:image/svg+xml;charset=utf8,%3Csvg xmlns='http://www.w3.org/2000/svg' fill='%23666666' viewBox='0 0 8 8'%3E%3Cpath d='M5.25 0l-4 4 4 4 1.5-1.5-2.5-2.5 2.5-2.5-1.5-1.5z'/%3E%3C/svg%3E");
+$security-and-compliance-carousel-next-icon-background: url("data:image/svg+xml;charset=utf8,%3Csvg xmlns='http://www.w3.org/2000/svg' fill='%23666666' viewBox='0 0 8 8'%3E%3Cpath d='M2.75 0l-1.5 1.5 2.5 2.5-2.5 2.5 1.5 1.5 4-4-4-4z'/%3E%3C/svg%3E");
+
+/*
Popup
*/
$popup-triangle-size: 15px;
diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index d3b3341e0b2..9418fda97e0 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -59,7 +59,8 @@ class Projects::IssuesController < Projects::ApplicationController
around_action :allow_gitaly_ref_name_caching, only: [:discussions]
before_action :run_null_hypothesis_experiment,
- only: [:index, :new, :create]
+ only: [:index, :new, :create],
+ if: -> { Feature.enabled?(:gitlab_experiments) }
respond_to :html
diff --git a/app/controllers/projects/merge_requests/diffs_controller.rb b/app/controllers/projects/merge_requests/diffs_controller.rb
index 9180b3f6b62..98ef9d918ae 100644
--- a/app/controllers/projects/merge_requests/diffs_controller.rb
+++ b/app/controllers/projects/merge_requests/diffs_controller.rb
@@ -122,10 +122,7 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic
end
end
- if render_merge_ref_head_diff?
- return CompareService.new(@project, @merge_request.merge_ref_head.sha)
- .execute(@project, @merge_request.target_branch)
- end
+ return @merge_request.merge_head_diff if render_merge_ref_head_diff?
if @start_sha
@merge_request_diff.compare_with(@start_sha)
diff --git a/app/experiments/application_experiment.rb b/app/experiments/application_experiment.rb
index 9a1a37a6b19..7a8851d11ce 100644
--- a/app/experiments/application_experiment.rb
+++ b/app/experiments/application_experiment.rb
@@ -1,20 +1,13 @@
# frozen_string_literal: true
class ApplicationExperiment < Gitlab::Experiment
- def enabled?
- return false if Feature::Definition.get(name).nil? # there has to be a feature flag yaml file
- return false unless Gitlab.dev_env_or_com? # we're in an environment that allows experiments
-
- Feature.get(name).state != :off # rubocop:disable Gitlab/AvoidFeatureGet
- end
-
def publish(_result)
track(:assignment) # track that we've assigned a variant for this context
Gon.global.push({ experiment: { name => signature } }, true) # push to client
end
def track(action, **event_args)
- return unless should_track? # no events for opted out actors or excluded subjects
+ return if excluded? # no events for opted out actors or excluded subjects
Gitlab::Tracking.event(name, action.to_s, **event_args.merge(
context: (event_args[:context] || []) << SnowplowTracker::SelfDescribingJson.new(
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 725a8edbcda..cae0f0a5c55 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -50,12 +50,15 @@ class MergeRequest < ApplicationRecord
end
end
- has_many :merge_request_diffs
+ has_many :merge_request_diffs,
+ -> { regular }, inverse_of: :merge_request
has_many :merge_request_context_commits, inverse_of: :merge_request
has_many :merge_request_context_commit_diff_files, through: :merge_request_context_commits, source: :diff_files
has_one :merge_request_diff,
- -> { order('merge_request_diffs.id DESC') }, inverse_of: :merge_request
+ -> { regular.order('merge_request_diffs.id DESC') }, inverse_of: :merge_request
+ has_one :merge_head_diff,
+ -> { merge_head }, inverse_of: :merge_request, class_name: 'MergeRequestDiff'
has_one :cleanup_schedule, inverse_of: :merge_request
belongs_to :latest_merge_request_diff, class_name: 'MergeRequestDiff'
@@ -476,13 +479,17 @@ class MergeRequest < ApplicationRecord
# This is used after project import, to reset the IDs to the correct
# values. It is not intended to be called without having already scoped the
# relation.
+ #
+ # Only set `regular` merge request diffs as latest so `merge_head` diff
+ # won't be considered as `MergeRequest#merge_request_diff`.
def self.set_latest_merge_request_diff_ids!
- update = '
+ update = "
latest_merge_request_diff_id = (
SELECT MAX(id)
FROM merge_request_diffs
WHERE merge_requests.id = merge_request_diffs.merge_request_id
- )'.squish
+ AND merge_request_diffs.diff_type = #{MergeRequestDiff.diff_types[:regular]}
+ )".squish
self.each_batch do |batch|
batch.update_all(update)
@@ -921,7 +928,7 @@ class MergeRequest < ApplicationRecord
def create_merge_request_diff
fetch_ref!
- # n+1: https://gitlab.com/gitlab-org/gitlab-foss/issues/37435
+ # n+1: https://gitlab.com/gitlab-org/gitlab/-/issues/19377
Gitlab::GitalyClient.allow_n_plus_1_calls do
merge_request_diffs.create!
reload_merge_request_diff
@@ -995,7 +1002,7 @@ class MergeRequest < ApplicationRecord
# rubocop: enable CodeReuse/ServiceClass
def diffable_merge_ref?
- open? && merge_ref_head.present? && (Feature.enabled?(:display_merge_conflicts_in_diff, project) || can_be_merged?)
+ open? && merge_head_diff.present? && (Feature.enabled?(:display_merge_conflicts_in_diff, project) || can_be_merged?)
end
# Returns boolean indicating the merge_status should be rechecked in order to
diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb
index d23e66b9697..5500ee7f74a 100644
--- a/app/models/merge_request_diff.rb
+++ b/app/models/merge_request_diff.rb
@@ -32,6 +32,7 @@ class MergeRequestDiff < ApplicationRecord
has_many :merge_request_diff_commits, -> { order(:merge_request_diff_id, :relative_order) }
validates :base_commit_sha, :head_commit_sha, :start_commit_sha, sha: true
+ validates :merge_request_id, uniqueness: { scope: :diff_type }, if: :merge_head?
state_machine :state, initial: :empty do
event :clean do
@@ -50,6 +51,11 @@ class MergeRequestDiff < ApplicationRecord
state :overflow_diff_lines_limit
end
+ enum diff_type: {
+ regular: 1,
+ merge_head: 2
+ }
+
scope :with_files, -> { without_states(:without_files, :empty) }
scope :viewable, -> { without_state(:empty) }
scope :by_commit_sha, ->(sha) do
@@ -72,6 +78,7 @@ class MergeRequestDiff < ApplicationRecord
join_condition = merge_requests[:id].eq(mr_diffs[:merge_request_id])
.and(mr_diffs[:id].not_eq(merge_requests[:latest_merge_request_diff_id]))
+ .and(mr_diffs[:diff_type].eq(diff_types[:regular]))
arel_join = mr_diffs.join(merge_requests).on(join_condition)
joins(arel_join.join_sources)
@@ -196,6 +203,10 @@ class MergeRequestDiff < ApplicationRecord
end
def set_as_latest_diff
+ # Don't set merge_head diff as latest so it won't get considered as the
+ # MergeRequest#merge_request_diff.
+ return if merge_head?
+
MergeRequest
.where('id = ? AND COALESCE(latest_merge_request_diff_id, 0) < ?', self.merge_request_id, self.id)
.update_all(latest_merge_request_diff_id: self.id)
@@ -203,8 +214,16 @@ class MergeRequestDiff < ApplicationRecord
def ensure_commit_shas
self.start_commit_sha ||= merge_request.target_branch_sha
- self.head_commit_sha ||= merge_request.source_branch_sha
- self.base_commit_sha ||= find_base_sha
+
+ if merge_head? && merge_request.merge_ref_head.present?
+ diff_refs = merge_request.merge_ref_head.diff_refs
+
+ self.head_commit_sha ||= diff_refs.head_sha
+ self.base_commit_sha ||= diff_refs.base_sha
+ else
+ self.head_commit_sha ||= merge_request.source_branch_sha
+ self.base_commit_sha ||= find_base_sha
+ end
end
# Override head_commit_sha to keep compatibility with merge request diff
diff --git a/app/services/merge_requests/mergeability_check_service.rb b/app/services/merge_requests/mergeability_check_service.rb
index 96a2322f6a0..9fecab85cc1 100644
--- a/app/services/merge_requests/mergeability_check_service.rb
+++ b/app/services/merge_requests/mergeability_check_service.rb
@@ -114,6 +114,7 @@ module MergeRequests
merge_to_ref_success = merge_to_ref
+ reload_merge_head_diff
update_diff_discussion_positions! if merge_to_ref_success
if merge_to_ref_success && can_git_merge?
@@ -123,6 +124,10 @@ module MergeRequests
end
end
+ def reload_merge_head_diff
+ MergeRequests::ReloadMergeHeadDiffService.new(merge_request).execute
+ end
+
def update_diff_discussion_positions!
Discussions::CaptureDiffNotePositionsService.new(merge_request).execute
end
@@ -153,6 +158,7 @@ module MergeRequests
def merge_to_ref
params = { allow_conflicts: Feature.enabled?(:display_merge_conflicts_in_diff, project) }
result = MergeRequests::MergeToRefService.new(project, merge_request.author, params).execute(merge_request)
+
result[:status] == :success
end
diff --git a/app/services/merge_requests/reload_merge_head_diff_service.rb b/app/services/merge_requests/reload_merge_head_diff_service.rb
new file mode 100644
index 00000000000..66fcb5c022b
--- /dev/null
+++ b/app/services/merge_requests/reload_merge_head_diff_service.rb
@@ -0,0 +1,50 @@
+# frozen_string_literal: true
+
+module MergeRequests
+ class ReloadMergeHeadDiffService
+ include BaseServiceUtility
+
+ def initialize(merge_request)
+ @merge_request = merge_request
+ end
+
+ def execute
+ return error("default_merge_ref_for_diffs feature flag is disabled") unless enabled?
+ return error("Merge request has no merge ref head.") unless merge_request.merge_ref_head.present?
+
+ error_msg = recreate_merge_head_diff
+
+ return error(error_msg) if error_msg
+
+ success
+ end
+
+ private
+
+ attr_reader :merge_request
+
+ def enabled?
+ Feature.enabled?(:default_merge_ref_for_diffs, merge_request.project)
+ end
+
+ def recreate_merge_head_diff
+ merge_request.merge_head_diff&.destroy!
+
+ # n+1: https://gitlab.com/gitlab-org/gitlab/-/issues/19377
+ Gitlab::GitalyClient.allow_n_plus_1_calls do
+ merge_request.create_merge_head_diff!
+ end
+
+ # Reset the merge request so it won't load the merge head diff as the
+ # MergeRequest#merge_request_diff.
+ merge_request.reset
+
+ nil
+ rescue StandardError => e
+ message = "Failed to recreate merge head diff: #{e.message}"
+
+ Gitlab::AppLogger.error(message: message, merge_request_id: merge_request.id)
+ message
+ end
+ end
+end
diff --git a/changelogs/unreleased/292507-batch-merge-ref-diff.yml b/changelogs/unreleased/292507-batch-merge-ref-diff.yml
new file mode 100644
index 00000000000..fb000c4f160
--- /dev/null
+++ b/changelogs/unreleased/292507-batch-merge-ref-diff.yml
@@ -0,0 +1,5 @@
+---
+title: Support batch loading of merge head diffs
+merge_request: 51078
+author:
+type: performance
diff --git a/config/application.rb b/config/application.rb
index a2823b88e98..d95ef44c497 100644
--- a/config/application.rb
+++ b/config/application.rb
@@ -184,6 +184,7 @@ module Gitlab
config.assets.precompile << "page_bundles/build.css"
config.assets.precompile << "page_bundles/ci_status.css"
config.assets.precompile << "page_bundles/cycle_analytics.css"
+ config.assets.precompile << "page_bundles/security_discover.css"
config.assets.precompile << "page_bundles/dev_ops_report.css"
config.assets.precompile << "page_bundles/environments.css"
config.assets.precompile << "page_bundles/epics.css"
diff --git a/config/feature_flags/development/gitlab_experiments.yml b/config/feature_flags/development/gitlab_experiments.yml
new file mode 100644
index 00000000000..51fa6aa4529
--- /dev/null
+++ b/config/feature_flags/development/gitlab_experiments.yml
@@ -0,0 +1,8 @@
+---
+name: gitlab_experiments
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/45840
+rollout_issue_url:
+milestone: '13.7'
+type: development
+group: group::adoption
+default_enabled: false
diff --git a/db/migrate/20210107105306_add_diff_type_to_merge_request_diffs.rb b/db/migrate/20210107105306_add_diff_type_to_merge_request_diffs.rb
new file mode 100644
index 00000000000..0b9b5e93054
--- /dev/null
+++ b/db/migrate/20210107105306_add_diff_type_to_merge_request_diffs.rb
@@ -0,0 +1,31 @@
+# frozen_string_literal: true
+
+class AddDiffTypeToMergeRequestDiffs < ActiveRecord::Migration[6.0]
+ include Gitlab::Database::MigrationHelpers
+
+ DOWNTIME = false
+
+ disable_ddl_transaction!
+
+ UNIQUE_INDEX_NAME = 'index_merge_request_diffs_on_unique_merge_request_id'
+
+ def up
+ unless column_exists?(:merge_request_diffs, :diff_type)
+ with_lock_retries do
+ add_column :merge_request_diffs, :diff_type, :integer, null: false, limit: 2, default: 1
+ end
+ end
+
+ add_concurrent_index :merge_request_diffs, :merge_request_id, unique: true, where: 'diff_type = 2', name: UNIQUE_INDEX_NAME
+ end
+
+ def down
+ remove_concurrent_index_by_name(:merge_request_diffs, UNIQUE_INDEX_NAME)
+
+ if column_exists?(:merge_request_diffs, :diff_type)
+ with_lock_retries do
+ remove_column :merge_request_diffs, :diff_type
+ end
+ end
+ end
+end
diff --git a/db/schema_migrations/20210107105306 b/db/schema_migrations/20210107105306
new file mode 100644
index 00000000000..fe66a041837
--- /dev/null
+++ b/db/schema_migrations/20210107105306
@@ -0,0 +1 @@
+f76ce27a82f4773dcda324d79cc93a044f21317dbb9fdff035879502b5752da3 \ No newline at end of file
diff --git a/db/structure.sql b/db/structure.sql
index bae7dc05122..1a451a2430c 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -14011,6 +14011,7 @@ CREATE TABLE merge_request_diffs (
stored_externally boolean,
files_count smallint,
sorted boolean DEFAULT false NOT NULL,
+ diff_type smallint DEFAULT 1 NOT NULL,
CONSTRAINT check_93ee616ac9 CHECK ((external_diff_store IS NOT NULL))
);
@@ -22221,6 +22222,8 @@ CREATE INDEX index_merge_request_diffs_on_external_diff_store ON merge_request_d
CREATE INDEX index_merge_request_diffs_on_merge_request_id_and_id ON merge_request_diffs USING btree (merge_request_id, id);
+CREATE UNIQUE INDEX index_merge_request_diffs_on_unique_merge_request_id ON merge_request_diffs USING btree (merge_request_id) WHERE (diff_type = 2);
+
CREATE INDEX index_merge_request_metrics_on_first_deployed_to_production_at ON merge_request_metrics USING btree (first_deployed_to_production_at);
CREATE INDEX index_merge_request_metrics_on_latest_closed_at ON merge_request_metrics USING btree (latest_closed_at) WHERE (latest_closed_at IS NOT NULL);
diff --git a/doc/administration/pages/index.md b/doc/administration/pages/index.md
index 37dc03a297d..de9e083f39c 100644
--- a/doc/administration/pages/index.md
+++ b/doc/administration/pages/index.md
@@ -928,6 +928,20 @@ For example, if there is a connection timeout:
error="failed to connect to internal Pages API: Get \"https://gitlab.example.com:3000/api/v4/internal/pages/status\": net/http: request canceled while waiting for connection (Client.Timeout exceeded while awaiting headers)"
```
+### Pages cannot communicate with an instance of the GitLab API
+
+If you use the default value for `domain_config_source=auto` and run multiple instances of GitLab
+Pages, you may see intermittent 502 error responses while serving Pages content. You may also see
+the following warning in the Pages logs:
+
+```plaintext
+WARN[0010] Pages cannot communicate with an instance of the GitLab API. Please sync your gitlab-secrets.json file https://gitlab.com/gitlab-org/gitlab-pages/-/issues/535#workaround. error="pages endpoint unauthorized"
+```
+
+This can happen if your `gitlab-secrets.json` file is out of date between GitLab Rails and GitLab
+Pages. Follow steps 8-10 of [Running GitLab Pages on a separate server](#running-gitlab-pages-on-a-separate-server),
+in all of your GitLab Pages instances.
+
### 500 error with `securecookie: failed to generate random iv` and `Failed to save the session`
This problem most likely results from an [out-dated operating system](https://docs.gitlab.com/omnibus/package-information/deprecated_os.html).
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 88010cebc97..870fbe08edf 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -17550,6 +17550,9 @@ msgstr ""
msgid "May"
msgstr ""
+msgid "Mean time to merge"
+msgstr ""
+
msgid "Measured in bytes of code. Excludes generated and vendored code."
msgstr ""
@@ -33593,6 +33596,9 @@ msgid_plural "days"
msgstr[0] ""
msgstr[1] ""
+msgid "days"
+msgstr ""
+
msgid "default branch"
msgstr ""
diff --git a/qa/qa/specs/features/browser_ui/5_package/nuget_repository_spec.rb b/qa/qa/specs/features/browser_ui/5_package/nuget_repository_spec.rb
index be806fcbb3e..e3bd1db297d 100644
--- a/qa/qa/specs/features/browser_ui/5_package/nuget_repository_spec.rb
+++ b/qa/qa/specs/features/browser_ui/5_package/nuget_repository_spec.rb
@@ -13,6 +13,13 @@ module QA
end
end
+ let(:another_project) do
+ Resource::Project.fabricate_via_api! do |project|
+ project.name = 'nuget-package-install-project'
+ project.template_name = 'dotnetcore'
+ end
+ end
+
let!(:runner) do
Resource::Runner.fabricate! do |runner|
runner.name = "qa-runner-#{Time.now.to_i}"
@@ -22,11 +29,21 @@ module QA
end
end
+ let!(:another_runner) do
+ Resource::Runner.fabricate! do |runner|
+ runner.name = "qa-runner-#{Time.now.to_i}"
+ runner.tags = ["runner-for-#{another_project.name}"]
+ runner.executor = :docker
+ runner.project = another_project
+ end
+ end
+
after do
runner.remove_via_api!
+ another_runner.remove_via_api!
end
- it 'publishes a nuget package and deletes it', testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/issues/1073' do
+ it 'publishes a nuget package at the project level, installs and deletes it at the group level', testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/issues/1073' do
Flow::Login.sign_in
Resource::Repository::Commit.fabricate_via_api! do |commit|
@@ -37,23 +54,23 @@ module QA
{
file_path: '.gitlab-ci.yml',
content: <<~YAML
- image: mcr.microsoft.com/dotnet/core/sdk:3.1
-
- stages:
- - deploy
-
- deploy:
- stage: deploy
- script:
- - dotnet restore -p:Configuration=Release
- - dotnet build -c Release
- - dotnet pack -c Release
- - dotnet nuget add source "$CI_SERVER_URL/api/v4/projects/$CI_PROJECT_ID/packages/nuget/index.json" --name gitlab --username gitlab-ci-token --password $CI_JOB_TOKEN --store-password-in-clear-text
- - dotnet nuget push "bin/Release/*.nupkg" --source gitlab
- only:
- - "#{project.default_branch}"
- tags:
- - "runner-for-#{project.name}"
+ image: mcr.microsoft.com/dotnet/core/sdk:3.1
+
+ stages:
+ - deploy
+
+ deploy:
+ stage: deploy
+ script:
+ - dotnet restore -p:Configuration=Release
+ - dotnet build -c Release
+ - dotnet pack -c Release
+ - dotnet nuget add source "$CI_SERVER_URL/api/v4/projects/$CI_PROJECT_ID/packages/nuget/index.json" --name gitlab --username gitlab-ci-token --password $CI_JOB_TOKEN --store-password-in-clear-text
+ - dotnet nuget push "bin/Release/*.nupkg" --source gitlab
+ only:
+ - "#{project.default_branch}"
+ tags:
+ - "runner-for-#{project.name}"
YAML
}
]
@@ -71,7 +88,67 @@ module QA
expect(job).to be_successful(timeout: 800)
end
- Page::Project::Menu.perform(&:click_packages_link)
+ another_project.visit!
+
+ Resource::Repository::Commit.fabricate_via_api! do |commit|
+ commit.project = another_project
+ commit.commit_message = 'Add new csproj file'
+ commit.add_files(
+ [
+ {
+ file_path: 'otherdotnet.csproj',
+ content: <<~EOF
+ <Project Sdk="Microsoft.NET.Sdk">
+
+ <PropertyGroup>
+ <OutputType>Exe</OutputType>
+ <TargetFramework>netcoreapp3.1</TargetFramework>
+ </PropertyGroup>
+
+ </Project>
+ EOF
+ }
+ ]
+ )
+ commit.update_files(
+ [
+ {
+ file_path: '.gitlab-ci.yml',
+ content: <<~YAML
+ image: mcr.microsoft.com/dotnet/core/sdk:3.1
+
+ stages:
+ - install
+
+ install:
+ stage: install
+ script:
+ - dotnet nuget locals all --clear
+ - dotnet nuget add source "$CI_SERVER_URL/api/v4/groups/#{another_project.group.id}/-/packages/nuget/index.json" --name gitlab --username gitlab-ci-token --password $CI_JOB_TOKEN --store-password-in-clear-text
+ - "dotnet add otherdotnet.csproj package #{package_name} --version 1.0.0"
+ only:
+ - "#{another_project.default_branch}"
+ tags:
+ - "runner-for-#{another_project.name}"
+ YAML
+ }
+ ]
+ )
+ end
+
+ Flow::Pipeline.visit_latest_pipeline
+
+ Page::Project::Pipeline::Show.perform do |pipeline|
+ pipeline.click_job('install')
+ end
+
+ Page::Project::Job::Show.perform do |job|
+ expect(job).to be_successful(timeout: 800)
+ end
+
+ project.group.sandbox.visit!
+
+ Page::Group::Menu.perform(&:go_to_group_packages)
Page::Project::Packages::Index.perform do |index|
expect(index).to have_package(package_name)
diff --git a/scripts/trigger-build b/scripts/trigger-build
index ab6dcc63e11..aeea25f9ad8 100755
--- a/scripts/trigger-build
+++ b/scripts/trigger-build
@@ -3,13 +3,6 @@
require 'gitlab'
-#
-# Configure credentials to be used with gitlab gem
-#
-Gitlab.configure do |config|
- config.endpoint = 'https://gitlab.com/api/v4'
-end
-
module Trigger
def self.ee?
# Support former project name for `dev`
@@ -34,18 +27,13 @@ module Trigger
ENV['GITLAB_BOT_MULTI_PROJECT_PIPELINE_POLLING_TOKEN']
end
- def initialize
- # gitlab-bot's token "GitLab multi-project pipeline polling"
- Gitlab.private_token = self.class.access_token
- end
-
def invoke!(post_comment: false, downstream_job_name: nil)
pipeline_variables = variables
puts "Triggering downstream pipeline on #{downstream_project_path}"
puts "with variables #{pipeline_variables}"
- pipeline = Gitlab.run_trigger(
+ pipeline = gitlab_client(:downstream).run_trigger(
downstream_project_path,
trigger_token,
ref,
@@ -54,23 +42,34 @@ module Trigger
puts "Triggered downstream pipeline: #{pipeline.web_url}\n"
puts "Waiting for downstream pipeline status"
- Trigger::CommitComment.post!(pipeline) if post_comment
+ Trigger::CommitComment.post!(pipeline, gitlab_client(:upstream)) if post_comment
downstream_job =
if downstream_job_name
- Gitlab.pipeline_jobs(downstream_project_path, pipeline.id).auto_paginate.find do |potential_job|
+ gitlab_client(:downstream).pipeline_jobs(downstream_project_path, pipeline.id).auto_paginate.find do |potential_job|
potential_job.name == downstream_job_name
end
end
if downstream_job
- Trigger::Job.new(downstream_project_path, downstream_job.id)
+ Trigger::Job.new(downstream_project_path, downstream_job.id, gitlab_client(:downstream))
else
- Trigger::Pipeline.new(downstream_project_path, pipeline.id)
+ Trigger::Pipeline.new(downstream_project_path, pipeline.id, gitlab_client(:downstream))
end
end
private
+ # Override to trigger and work with pipeline on different GitLab instance
+ # type: :downstream -> downstream build and pipeline status
+ # type: :upstream -> this project, e.g. for posting comments
+ def gitlab_client(type)
+ # By default, always use the same client
+ @gitlab_client ||= Gitlab.client(
+ endpoint: 'https://gitlab.com/api/v4',
+ private_token: self.class.access_token
+ )
+ end
+
# Must be overridden
def downstream_project_path
raise NotImplementedError
@@ -305,9 +304,53 @@ module Trigger
end
end
+ class DatabaseTesting < Base
+ def self.access_token
+ ENV['GITLABCOM_DATABASE_TESTING_ACCESS_TOKEN']
+ end
+
+ private
+
+ def gitlab_client(type)
+ @gitlab_clients ||= {
+ downstream: Gitlab.client(
+ endpoint: 'https://ops.gitlab.net/api/v4',
+ private_token: self.class.access_token
+ ),
+ upstream: Gitlab.client(
+ endpoint: 'https://gitlab.com/api/v4',
+ private_token: Base.access_token
+ )
+ }
+
+ @gitlab_clients[type]
+ end
+
+ def trigger_token
+ ENV['GITLABCOM_DATABASE_TESTING_TRIGGER_TOKEN']
+ end
+
+ def downstream_project_path
+ ENV['GITLABCOM_DATABASE_TESTING_PROJECT_PATH'] || 'gitlab-com/database-team/gitlab-com-database-testing'
+ end
+
+ def extra_variables
+ {
+ # Use CI_MERGE_REQUEST_SOURCE_BRANCH_SHA for omnibus checkouts due to pipeline for merged results
+ # and fallback to CI_COMMIT_SHA for the `detached` pipelines.
+ 'GITLAB_COMMIT_SHA' => Trigger.non_empty_variable_value('CI_MERGE_REQUEST_SOURCE_BRANCH_SHA') || ENV['CI_COMMIT_SHA'],
+ 'TRIGGERED_USER_LOGIN' => ENV['GITLAB_USER_LOGIN']
+ }
+ end
+
+ def ref
+ ENV['GITLABCOM_DATABASE_TESTING_TRIGGER_REF'] || 'master'
+ end
+ end
+
class CommitComment
- def self.post!(downstream_pipeline)
- Gitlab.create_commit_comment(
+ def self.post!(downstream_pipeline, gitlab_client)
+ gitlab_client.create_commit_comment(
ENV['CI_PROJECT_PATH'],
Trigger.non_empty_variable_value('CI_MERGE_REQUEST_SOURCE_BRANCH_SHA') || ENV['CI_COMMIT_SHA'],
"The [`#{ENV['CI_JOB_NAME']}`](#{ENV['CI_JOB_URL']}) job from pipeline #{ENV['CI_PIPELINE_URL']} triggered #{downstream_pipeline.web_url} downstream.")
@@ -329,9 +372,10 @@ module Trigger
unscoped_class_name.downcase
end
- def initialize(project, id)
+ def initialize(project, id, gitlab_client)
@project = project
@id = id
+ @gitlab_client = gitlab_client
@start_time = Time.now.to_i
end
@@ -359,7 +403,7 @@ module Trigger
end
def status
- Gitlab.public_send(self.class.gitlab_api_method_name, project, id).status.to_sym # rubocop:disable GitlabSecurity/PublicSend
+ gitlab_client.public_send(self.class.gitlab_api_method_name, project, id).status.to_sym # rubocop:disable GitlabSecurity/PublicSend
rescue Gitlab::Error::Error => error
puts "Ignoring the following error: #{error}"
# Ignore GitLab API hiccups. If GitLab is really down, we'll hit the job
@@ -369,7 +413,7 @@ module Trigger
private
- attr_reader :project, :id, :start_time
+ attr_reader :project, :id, :gitlab_client, :start_time
end
Job = Class.new(Pipeline)
@@ -380,6 +424,8 @@ when 'omnibus'
Trigger::Omnibus.new.invoke!(post_comment: true, downstream_job_name: 'Trigger:qa-test').wait!
when 'cng'
Trigger::CNG.new.invoke!.wait!
+when 'gitlab-com-database-testing'
+ Trigger::DatabaseTesting.new.invoke!
when 'docs'
docs_trigger = Trigger::Docs.new
@@ -395,5 +441,6 @@ when 'docs'
else
puts "Please provide a valid option:
omnibus - Triggers a pipeline that builds the omnibus-gitlab package
- cng - Triggers a pipeline that builds images used by the GitLab helm chart"
+ cng - Triggers a pipeline that builds images used by the GitLab helm chart
+ gitlab-com-database-testing - Triggers a pipeline that tests database changes on GitLab.com data"
end
diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb
index 81ffd2c4512..597f7007a7f 100644
--- a/spec/controllers/projects/issues_controller_spec.rb
+++ b/spec/controllers/projects/issues_controller_spec.rb
@@ -63,20 +63,53 @@ RSpec.describe Projects::IssuesController do
end
end
- describe 'the null hypothesis experiment', :experiment do
- before do
- stub_experiments(null_hypothesis: :candidate)
- end
-
+ describe 'the null hypothesis experiment', :snowplow do
it 'defines the expected before actions' do
expect(controller).to use_before_action(:run_null_hypothesis_experiment)
end
- it 'assigns the candidate experience and tracks the event' do
- expect(experiment(:null_hypothesis)).to track('index').on_any_instance.for(:candidate)
- .with_context(project: project)
+ context 'when rolled out to 100%' do
+ it 'assigns the candidate experience and tracks the event' do
+ get :index, params: { namespace_id: project.namespace, project_id: project }
+
+ expect_snowplow_event(
+ category: 'null_hypothesis',
+ action: 'index',
+ context: [{
+ schema: 'iglu:com.gitlab/gitlab_experiment/jsonschema/0-3-0',
+ data: { variant: 'candidate', experiment: 'null_hypothesis', key: anything }
+ }]
+ )
+ end
+ end
- get :index, params: { namespace_id: project.namespace, project_id: project }
+ context 'when not rolled out' do
+ before do
+ stub_feature_flags(null_hypothesis: false)
+ end
+
+ it 'assigns the control experience and tracks the event' do
+ get :index, params: { namespace_id: project.namespace, project_id: project }
+
+ expect_snowplow_event(
+ category: 'null_hypothesis',
+ action: 'index',
+ context: [{
+ schema: 'iglu:com.gitlab/gitlab_experiment/jsonschema/0-3-0',
+ data: { variant: 'control', experiment: 'null_hypothesis', key: anything }
+ }]
+ )
+ end
+ end
+
+ context 'when gitlab_experiments is disabled' do
+ it 'does not run the experiment at all' do
+ stub_feature_flags(gitlab_experiments: false)
+
+ expect(controller).not_to receive(:run_null_hypothesis_experiment)
+
+ get :index, params: { namespace_id: project.namespace, project_id: project }
+ end
end
end
end
diff --git a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb
index f54a07de853..50f8942d9d5 100644
--- a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb
+++ b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb
@@ -226,11 +226,7 @@ RSpec.describe Projects::MergeRequests::DiffsController do
let(:diffable_merge_ref) { true }
it 'compares diffs with the head' do
- MergeRequests::MergeToRefService.new(project, merge_request.author).execute(merge_request)
-
- expect(CompareService).to receive(:new).with(
- project, merge_request.merge_ref_head.sha
- ).and_call_original
+ create(:merge_request_diff, :merge_head, merge_request: merge_request)
go(diff_head: true)
@@ -242,8 +238,6 @@ RSpec.describe Projects::MergeRequests::DiffsController do
let(:diffable_merge_ref) { false }
it 'compares diffs with the base' do
- expect(CompareService).not_to receive(:new)
-
go(diff_head: true)
expect(response).to have_gitlab_http_status(:ok)
diff --git a/spec/experiments/application_experiment_spec.rb b/spec/experiments/application_experiment_spec.rb
index beefc09e591..ece52d37351 100644
--- a/spec/experiments/application_experiment_spec.rb
+++ b/spec/experiments/application_experiment_spec.rb
@@ -2,45 +2,9 @@
require 'spec_helper'
-RSpec.describe ApplicationExperiment, :experiment do
+RSpec.describe ApplicationExperiment do
subject { described_class.new(:stub) }
- before do
- allow(subject).to receive(:enabled?).and_return(true)
- end
-
- describe "enabled" do
- before do
- allow(subject).to receive(:enabled?).and_call_original
-
- allow(Feature::Definition).to receive(:get).and_return('_instance_')
- allow(Gitlab).to receive(:dev_env_or_com?).and_return(true)
- allow(Feature).to receive(:get).and_return(double(state: :on))
- end
-
- it "is enabled when all criteria are met" do
- expect(subject).to be_enabled
- end
-
- it "isn't enabled if the feature definition doesn't exist" do
- expect(Feature::Definition).to receive(:get).with('stub').and_return(nil)
-
- expect(subject).not_to be_enabled
- end
-
- it "isn't enabled if we're not in dev or dotcom environments" do
- expect(Gitlab).to receive(:dev_env_or_com?).and_return(false)
-
- expect(subject).not_to be_enabled
- end
-
- it "isn't enabled if the feature flag state is :off" do
- expect(Feature).to receive(:get).with('stub').and_return(double(state: :off))
-
- expect(subject).not_to be_enabled
- end
- end
-
describe "publishing results" do
it "tracks the assignment" do
expect(subject).to receive(:track).with(:assignment)
@@ -67,8 +31,8 @@ RSpec.describe ApplicationExperiment, :experiment do
end
describe "tracking events", :snowplow do
- it "doesn't track if we shouldn't track" do
- allow(subject).to receive(:should_track?).and_return(false)
+ it "doesn't track if excluded" do
+ subject.exclude { true }
subject.track(:action)
diff --git a/spec/factories/merge_request_diffs.rb b/spec/factories/merge_request_diffs.rb
index 481cabdae6d..f93f3f22109 100644
--- a/spec/factories/merge_request_diffs.rb
+++ b/spec/factories/merge_request_diffs.rb
@@ -10,12 +10,18 @@ FactoryBot.define do
head_commit_sha { Digest::SHA1.hexdigest(SecureRandom.hex) }
start_commit_sha { Digest::SHA1.hexdigest(SecureRandom.hex) }
+ diff_type { :regular }
+
trait :external do
external_diff { fixture_file_upload("spec/fixtures/doc_sample.txt", "plain/txt") }
stored_externally { true }
importing { true } # this avoids setting the state to 'empty'
end
+ trait :merge_head do
+ diff_type { :merge_head }
+ end
+
factory :external_merge_request_diff, traits: [:external]
end
end
diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml
index ab9b62e02f0..3f40e0d228b 100644
--- a/spec/lib/gitlab/import_export/all_models.yml
+++ b/spec/lib/gitlab/import_export/all_models.yml
@@ -146,6 +146,7 @@ merge_requests:
- merge_user
- merge_request_diffs
- merge_request_diff
+- merge_head_diff
- merge_request_context_commits
- merge_request_context_commit_diff_files
- events
diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml
index 79769f7ba7a..fa1df7acd07 100644
--- a/spec/lib/gitlab/import_export/safe_model_attributes.yml
+++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml
@@ -220,6 +220,7 @@ MergeRequestDiff:
- commits_count
- files_count
- sorted
+- diff_type
MergeRequestDiffCommit:
- merge_request_diff_id
- relative_order
diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb
index a5493d1650b..83024c587fa 100644
--- a/spec/models/merge_request_diff_spec.rb
+++ b/spec/models/merge_request_diff_spec.rb
@@ -16,6 +16,8 @@ RSpec.describe MergeRequestDiff do
describe 'validations' do
subject { diff_with_commits }
+ it { is_expected.not_to validate_uniqueness_of(:diff_type).scoped_to(:merge_request_id) }
+
it 'checks sha format of base_commit_sha, head_commit_sha and start_commit_sha' do
subject.base_commit_sha = subject.head_commit_sha = subject.start_commit_sha = 'foobar'
@@ -23,6 +25,24 @@ RSpec.describe MergeRequestDiff do
expect(subject.errors.count).to eq 3
expect(subject.errors).to all(include('is not a valid SHA'))
end
+
+ it 'does not validate uniqueness by default' do
+ expect(build(:merge_request_diff, merge_request: subject.merge_request)).to be_valid
+ end
+
+ context 'when merge request diff is a merge_head type' do
+ it 'is valid' do
+ expect(build(:merge_request_diff, :merge_head, merge_request: subject.merge_request)).to be_valid
+ end
+
+ context 'when merge_head diff exists' do
+ let(:existing_merge_head_diff) { create(:merge_request_diff, :merge_head) }
+
+ it 'validates uniqueness' do
+ expect(build(:merge_request_diff, :merge_head, merge_request: existing_merge_head_diff.merge_request)).not_to be_valid
+ end
+ end
+ end
end
describe 'create new record' do
@@ -35,6 +55,26 @@ RSpec.describe MergeRequestDiff do
it { expect(subject.head_commit_sha).to eq('b83d6e391c22777fca1ed3012fce84f633d7fed0') }
it { expect(subject.base_commit_sha).to eq('ae73cb07c9eeaf35924a10f713b364d32b2dd34f') }
it { expect(subject.start_commit_sha).to eq('0b4bc9a49b562e85de7cc9e834518ea6828729b9') }
+
+ context 'when diff_type is merge_head' do
+ let_it_be(:merge_request) { create(:merge_request) }
+
+ let_it_be(:merge_head) do
+ MergeRequests::MergeToRefService
+ .new(merge_request.project, merge_request.author)
+ .execute(merge_request)
+
+ merge_request.create_merge_head_diff
+ end
+
+ it { expect(merge_head).to be_valid }
+ it { expect(merge_head).to be_persisted }
+ it { expect(merge_head.commits.count).to eq(30) }
+ it { expect(merge_head.diffs.count).to eq(20) }
+ it { expect(merge_head.head_commit_sha).to eq(merge_request.merge_ref_head.diff_refs.head_sha) }
+ it { expect(merge_head.base_commit_sha).to eq(merge_request.merge_ref_head.diff_refs.base_sha) }
+ it { expect(merge_head.start_commit_sha).to eq(merge_request.target_branch_sha) }
+ end
end
describe '.by_commit_sha' do
@@ -63,6 +103,7 @@ RSpec.describe MergeRequestDiff do
let_it_be(:merge_request) { create(:merge_request) }
let_it_be(:outdated) { merge_request.merge_request_diff }
let_it_be(:latest) { merge_request.create_merge_request_diff }
+ let_it_be(:merge_head) { merge_request.create_merge_head_diff }
let_it_be(:closed_mr) { create(:merge_request, :closed_last_month) }
let(:closed) { closed_mr.merge_request_diff }
@@ -103,14 +144,14 @@ RSpec.describe MergeRequestDiff do
stub_external_diffs_setting(enabled: true)
end
- it { is_expected.to contain_exactly(outdated.id, latest.id, closed.id, merged.id, closed_recently.id, merged_recently.id) }
+ it { is_expected.to contain_exactly(outdated.id, latest.id, closed.id, merged.id, closed_recently.id, merged_recently.id, merge_head.id) }
it 'ignores diffs with 0 files' do
MergeRequestDiffFile.where(merge_request_diff_id: [closed_recently.id, merged_recently.id]).delete_all
closed_recently.update!(files_count: 0)
merged_recently.update!(files_count: 0)
- is_expected.to contain_exactly(outdated.id, latest.id, closed.id, merged.id)
+ is_expected.to contain_exactly(outdated.id, latest.id, closed.id, merged.id, merge_head.id)
end
end
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 094bb370100..9ba379167ee 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -491,6 +491,7 @@ RSpec.describe MergeRequest, factory_default: :keep do
create(:merge_request, params).tap do |mr|
diffs.times { mr.merge_request_diffs.create }
+ mr.create_merge_head_diff
end
end
@@ -4379,37 +4380,41 @@ RSpec.describe MergeRequest, factory_default: :keep do
end
describe '#diffable_merge_ref?' do
+ let(:merge_request) { create(:merge_request) }
+
context 'merge request can be merged' do
- context 'merge_to_ref is not calculated' do
+ context 'merge_head diff is not created' do
it 'returns true' do
- expect(subject.diffable_merge_ref?).to eq(false)
+ expect(merge_request.diffable_merge_ref?).to eq(false)
end
end
- context 'merge_to_ref is calculated' do
+ context 'merge_head diff is created' do
before do
- MergeRequests::MergeToRefService.new(subject.project, subject.author).execute(subject)
+ create(:merge_request_diff, :merge_head, merge_request: merge_request)
end
it 'returns true' do
- expect(subject.diffable_merge_ref?).to eq(true)
+ expect(merge_request.diffable_merge_ref?).to eq(true)
end
context 'merge request is merged' do
- subject { build_stubbed(:merge_request, :merged, project: project) }
+ before do
+ merge_request.mark_as_merged!
+ end
it 'returns false' do
- expect(subject.diffable_merge_ref?).to eq(false)
+ expect(merge_request.diffable_merge_ref?).to eq(false)
end
end
context 'merge request cannot be merged' do
before do
- subject.mark_as_unchecked!
+ merge_request.mark_as_unchecked!
end
it 'returns false' do
- expect(subject.diffable_merge_ref?).to eq(true)
+ expect(merge_request.diffable_merge_ref?).to eq(true)
end
context 'display_merge_conflicts_in_diff is disabled' do
@@ -4418,7 +4423,7 @@ RSpec.describe MergeRequest, factory_default: :keep do
end
it 'returns false' do
- expect(subject.diffable_merge_ref?).to eq(false)
+ expect(merge_request.diffable_merge_ref?).to eq(false)
end
end
end
diff --git a/spec/services/merge_requests/delete_non_latest_diffs_service_spec.rb b/spec/services/merge_requests/delete_non_latest_diffs_service_spec.rb
index cdaacaf5fca..d2070a466b1 100644
--- a/spec/services/merge_requests/delete_non_latest_diffs_service_spec.rb
+++ b/spec/services/merge_requests/delete_non_latest_diffs_service_spec.rb
@@ -12,6 +12,8 @@ RSpec.describe MergeRequests::DeleteNonLatestDiffsService, :clean_gitlab_redis_s
stub_const("#{described_class.name}::BATCH_SIZE", 2)
3.times { merge_request.create_merge_request_diff }
+ merge_request.create_merge_head_diff
+ merge_request.reset
end
it 'schedules non-latest merge request diffs removal' do
diff --git a/spec/services/merge_requests/mergeability_check_service_spec.rb b/spec/services/merge_requests/mergeability_check_service_spec.rb
index 17bfa9d7368..e0baf5af8b4 100644
--- a/spec/services/merge_requests/mergeability_check_service_spec.rb
+++ b/spec/services/merge_requests/mergeability_check_service_spec.rb
@@ -33,6 +33,14 @@ RSpec.describe MergeRequests::MergeabilityCheckService, :clean_gitlab_redis_shar
expect(merge_request.merge_status).to eq('can_be_merged')
end
+ it 'reloads merge head diff' do
+ expect_next_instance_of(MergeRequests::ReloadMergeHeadDiffService) do |service|
+ expect(service).to receive(:execute)
+ end
+
+ subject
+ end
+
it 'update diff discussion positions' do
expect_next_instance_of(Discussions::CaptureDiffNotePositionsService) do |service|
expect(service).to receive(:execute)
@@ -142,7 +150,11 @@ RSpec.describe MergeRequests::MergeabilityCheckService, :clean_gitlab_redis_shar
end
it 'resets one merge request upon execution' do
- expect_any_instance_of(MergeRequest).to receive(:reset).once
+ expect_next_instance_of(MergeRequests::ReloadMergeHeadDiffService) do |svc|
+ expect(svc).to receive(:execute).and_return(status: :success)
+ end
+
+ expect_any_instance_of(MergeRequest).to receive(:reset).once.and_call_original
execute_within_threads(amount: 2)
end
@@ -266,6 +278,14 @@ RSpec.describe MergeRequests::MergeabilityCheckService, :clean_gitlab_redis_shar
it_behaves_like 'unmergeable merge request'
+ it 'reloads merge head diff' do
+ expect_next_instance_of(MergeRequests::ReloadMergeHeadDiffService) do |service|
+ expect(service).to receive(:execute)
+ end
+
+ subject
+ end
+
it 'returns ServiceResponse.error' do
result = subject
@@ -329,6 +349,12 @@ RSpec.describe MergeRequests::MergeabilityCheckService, :clean_gitlab_redis_shar
subject
end
+
+ it 'does not reload merge head diff' do
+ expect(MergeRequests::ReloadMergeHeadDiffService).not_to receive(:new)
+
+ subject
+ end
end
end
diff --git a/spec/services/merge_requests/reload_merge_head_diff_service_spec.rb b/spec/services/merge_requests/reload_merge_head_diff_service_spec.rb
new file mode 100644
index 00000000000..3152a4e3861
--- /dev/null
+++ b/spec/services/merge_requests/reload_merge_head_diff_service_spec.rb
@@ -0,0 +1,61 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe MergeRequests::ReloadMergeHeadDiffService do
+ let(:merge_request) { create(:merge_request) }
+
+ subject { described_class.new(merge_request).execute }
+
+ describe '#execute' do
+ before do
+ MergeRequests::MergeToRefService
+ .new(merge_request.project, merge_request.author)
+ .execute(merge_request)
+ end
+
+ it 'creates a merge head diff' do
+ expect(subject[:status]).to eq(:success)
+ expect(merge_request.reload.merge_head_diff).to be_present
+ end
+
+ context 'when merge ref head is not present' do
+ before do
+ allow(merge_request).to receive(:merge_ref_head).and_return(nil)
+ end
+
+ it 'returns error' do
+ expect(subject[:status]).to eq(:error)
+ end
+ end
+
+ context 'when failed to create merge head diff' do
+ before do
+ allow(merge_request).to receive(:create_merge_head_diff!).and_raise('fail')
+ end
+
+ it 'returns error' do
+ expect(subject[:status]).to eq(:error)
+ end
+ end
+
+ context 'when there is existing merge head diff' do
+ let!(:existing_merge_head_diff) { create(:merge_request_diff, :merge_head, merge_request: merge_request) }
+
+ it 'recreates merge head diff' do
+ expect(subject[:status]).to eq(:success)
+ expect(merge_request.reload.merge_head_diff).not_to eq(existing_merge_head_diff)
+ end
+ end
+
+ context 'when default_merge_ref_for_diffs feature flag is disabled' do
+ before do
+ stub_feature_flags(default_merge_ref_for_diffs: false)
+ end
+
+ it 'returns error' do
+ expect(subject[:status]).to eq(:error)
+ end
+ end
+ end
+end
diff --git a/spec/support/gitlab_experiment.rb b/spec/support/gitlab_experiment.rb
index f8375a02b1e..1f283e4f06c 100644
--- a/spec/support/gitlab_experiment.rb
+++ b/spec/support/gitlab_experiment.rb
@@ -1,7 +1,4 @@
# frozen_string_literal: true
-# Require the provided spec helper and matchers.
-require 'gitlab/experiment/rspec'
-
# Disable all caching for experiments in tests.
Gitlab::Experiment::Configuration.cache = nil