From b99011af62935de0b15e8a314ffb7df1f8a3f303 Mon Sep 17 00:00:00 2001 From: Igor Date: Fri, 9 Aug 2019 21:01:55 +0000 Subject: Split MR widget into cached and non-cached serializers Splits auto-refreshing of MR widget into 2 requests: - the one which uses etag-caching and invalidates the fields on change - the one without caching The idea is to gradually move all the fields to etag-cached endpoint --- .../vue_merge_request_widget/mr_widget_options.vue | 4 +- .../services/mr_widget_service.js | 11 +- .../stores/mr_widget_store.js | 92 +++---- .../projects/merge_requests/content_controller.rb | 23 +- app/models/issue.rb | 2 +- app/models/merge_request.rb | 16 ++ app/presenters/merge_request_presenter.rb | 8 - .../merge_request_poll_cached_widget_entity.rb | 103 ++++++++ .../merge_request_poll_widget_entity.rb | 142 +++++++++++ app/serializers/merge_request_serializer.rb | 4 +- app/serializers/merge_request_widget_entity.rb | 282 +++------------------ .../unreleased/id-mr-widget-etag-caching.yml | 5 + config/routes/project.rb | 1 + lib/gitlab/etag_caching/router.rb | 4 + .../merge_requests/content_controller_spec.rb | 54 ++-- .../entities/merge_request_poll_cached_widget.json | 46 ++++ .../entities/merge_request_poll_widget.json | 55 ++++ .../api/schemas/entities/merge_request_widget.json | 159 +++--------- spec/models/merge_request_spec.rb | 2 + 19 files changed, 560 insertions(+), 453 deletions(-) create mode 100644 app/serializers/merge_request_poll_cached_widget_entity.rb create mode 100644 app/serializers/merge_request_poll_widget_entity.rb create mode 100644 changelogs/unreleased/id-mr-widget-etag-caching.yml create mode 100644 spec/fixtures/api/schemas/entities/merge_request_poll_cached_widget.json create mode 100644 spec/fixtures/api/schemas/entities/merge_request_poll_widget.json diff --git a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue index f7848a5fced..edd21a81f8b 100644 --- a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue +++ b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue @@ -166,6 +166,7 @@ export default { ciEnvironmentsStatusPath: store.ciEnvironmentsStatusPath, mergeRequestBasicPath: store.mergeRequestBasicPath, mergeRequestWidgetPath: store.mergeRequestWidgetPath, + mergeRequestCachedWidgetPath: store.mergeRequestCachedWidgetPath, mergeActionsContentPath: store.mergeActionsContentPath, rebasePath: store.rebasePath, }; @@ -176,8 +177,7 @@ export default { checkStatus(cb, isRebased) { return this.service .checkStatus() - .then(res => res.data) - .then(data => { + .then(({ data }) => { this.handleNotification(data); this.mr.setData(data, isRebased); this.setFaviconHelper(); diff --git a/app/assets/javascripts/vue_merge_request_widget/services/mr_widget_service.js b/app/assets/javascripts/vue_merge_request_widget/services/mr_widget_service.js index 1dae53039d5..f637a44bf2d 100644 --- a/app/assets/javascripts/vue_merge_request_widget/services/mr_widget_service.js +++ b/app/assets/javascripts/vue_merge_request_widget/services/mr_widget_service.js @@ -34,7 +34,16 @@ export default class MRWidgetService { } checkStatus() { - return axios.get(this.endpoints.mergeRequestWidgetPath); + // two endpoints are requested in order to get MR info: + // one which is etag-cached and invalidated and another one which is not cached + // the idea is to move all the fields to etag-cached endpoint and then perform only one request + // https://gitlab.com/gitlab-org/gitlab-ce/issues/61559#note_188801390 + const getData = axios.get(this.endpoints.mergeRequestWidgetPath); + const getCachedData = axios.get(this.endpoints.mergeRequestCachedWidgetPath); + + return axios + .all([getData, getCachedData]) + .then(axios.spread((res, cachedRes) => ({ data: Object.assign(res.data, cachedRes.data) }))); } fetchMergeActionsContent() { diff --git a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js index 581fee7477f..3eab8e6fc0b 100644 --- a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js +++ b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js @@ -10,6 +10,8 @@ export default class MergeRequestStore { this.sha = data.diff_head_sha; this.gitlabLogo = data.gitlabLogo; + this.setPaths(data); + this.setData(data); } @@ -18,13 +20,9 @@ export default class MergeRequestStore { this.sha = data.diff_head_sha; } - const currentUser = data.current_user; const pipelineStatus = data.pipeline ? data.pipeline.details.status : null; this.squash = data.squash; - this.squashBeforeMergeHelpPath = - this.squashBeforeMergeHelpPath || data.squash_before_merge_help_path; - this.troubleshootingDocsPath = this.troubleshootingDocsPath || data.troubleshooting_docs_path; this.enableSquashBeforeMerge = this.enableSquashBeforeMerge || true; this.iid = data.iid; @@ -33,8 +31,6 @@ export default class MergeRequestStore { this.targetBranchSha = data.target_branch_sha; this.sourceBranch = data.source_branch; this.sourceBranchProtected = data.source_branch_protected; - this.conflictsDocsPath = data.conflicts_docs_path; - this.mergeRequestPipelinesHelpPath = data.merge_request_pipelines_docs_path; this.mergeStatus = data.merge_status; this.commitMessage = data.default_merge_commit_message; this.shortMergeCommitSha = data.short_merge_commit_sha; @@ -48,7 +44,7 @@ export default class MergeRequestStore { this.postMergeDeployments = this.postMergeDeployments || []; this.commits = data.commits_without_merge_commits || []; this.squashCommitMessage = data.default_squash_commit_message; - this.initRebase(data); + this.rebaseInProgress = data.rebase_in_progress; if (data.issues_links) { const links = data.issues_links; @@ -66,14 +62,7 @@ export default class MergeRequestStore { this.setToAutoMergeBy = MergeRequestStore.formatUserObject(data.merge_user || {}); this.mergeUserId = data.merge_user_id; this.currentUserId = gon.current_user_id; - this.sourceBranchPath = data.source_branch_path; - this.sourceBranchLink = data.source_branch_with_namespace_link; this.mergeError = data.merge_error; - this.targetBranchPath = data.target_branch_commits_path; - this.targetBranchTreePath = data.target_branch_tree_path; - this.conflictResolutionPath = data.conflict_resolution_path; - this.cancelAutoMergePath = data.cancel_auto_merge_path; - this.removeWIPPath = data.remove_wip_path; this.sourceBranchRemoved = !data.source_branch_exists; this.shouldRemoveSourceBranch = data.remove_source_branch || false; this.onlyAllowMergeIfPipelineSucceeds = data.only_allow_merge_if_pipeline_succeeds || false; @@ -83,46 +72,23 @@ export default class MergeRequestStore { this.preferredAutoMergeStrategy = MergeRequestStore.getPreferredAutoMergeStrategy( this.availableAutoMergeStrategies, ); - this.mergePath = data.merge_path; this.ffOnlyEnabled = data.ff_only_enabled; this.shouldBeRebased = Boolean(data.should_be_rebased); - this.mergeRequestBasicPath = data.merge_request_basic_path; - this.mergeRequestWidgetPath = data.merge_request_widget_path; - this.emailPatchesPath = data.email_patches_path; - this.plainDiffPath = data.plain_diff_path; - this.newBlobPath = data.new_blob_path; - this.createIssueToResolveDiscussionsPath = data.create_issue_to_resolve_discussions_path; - this.mergeCheckPath = data.merge_check_path; - this.mergeActionsContentPath = data.commit_change_content_path; - this.mergeCommitPath = data.merge_commit_path; this.isRemovingSourceBranch = this.isRemovingSourceBranch || false; this.isOpen = data.state === 'opened'; this.hasMergeableDiscussionsState = data.mergeable_discussions_state === false; - this.canRemoveSourceBranch = currentUser.can_remove_source_branch || false; - this.canMerge = Boolean(data.merge_path); - this.canCreateIssue = currentUser.can_create_issue || false; - this.canCancelAutomaticMerge = Boolean(data.cancel_auto_merge_path); this.isSHAMismatch = this.sha !== data.diff_head_sha; this.canBeMerged = data.can_be_merged || false; this.isMergeAllowed = data.mergeable || false; this.mergeOngoing = data.merge_ongoing; this.allowCollaboration = data.allow_collaboration; - this.targetProjectFullPath = data.target_project_full_path; - this.sourceProjectFullPath = data.source_project_full_path; this.sourceProjectId = data.source_project_id; this.targetProjectId = data.target_project_id; this.mergePipelinesEnabled = Boolean(data.merge_pipelines_enabled); this.mergeTrainsCount = data.merge_trains_count || 0; this.mergeTrainIndex = data.merge_train_index; - // Cherry-pick and Revert actions related - this.canCherryPickInCurrentMR = currentUser.can_cherry_pick_on_current_merge_request || false; - this.canRevertInCurrentMR = currentUser.can_revert_on_current_merge_request || false; - this.cherryPickInForkPath = currentUser.cherry_pick_in_fork_path; - this.revertInForkPath = currentUser.revert_in_fork_path; - // CI related - this.ciEnvironmentsStatusPath = data.ci_environments_status_path; this.hasCI = data.has_ci; this.ciStatus = data.ci_status; this.isPipelineFailed = this.ciStatus === 'failed' || this.ciStatus === 'canceled'; @@ -133,8 +99,33 @@ export default class MergeRequestStore { this.isPipelineActive = data.pipeline ? data.pipeline.active : false; this.isPipelineBlocked = pipelineStatus ? pipelineStatus.group === 'manual' : false; this.ciStatusFaviconPath = pipelineStatus ? pipelineStatus.favicon : null; - this.testResultsPath = data.test_reports_path; + this.cancelAutoMergePath = data.cancel_auto_merge_path; + this.canCancelAutomaticMerge = Boolean(data.cancel_auto_merge_path); + + this.newBlobPath = data.new_blob_path; + this.sourceBranchPath = data.source_branch_path; + this.sourceBranchLink = data.source_branch_with_namespace_link; + this.rebasePath = data.rebase_path; + this.targetBranchPath = data.target_branch_commits_path; + this.targetBranchTreePath = data.target_branch_tree_path; + this.conflictResolutionPath = data.conflict_resolution_path; + this.removeWIPPath = data.remove_wip_path; + this.createIssueToResolveDiscussionsPath = data.create_issue_to_resolve_discussions_path; + this.mergePath = data.merge_path; + this.canMerge = Boolean(data.merge_path); + this.mergeCommitPath = data.merge_commit_path; + this.canPushToSourceBranch = data.can_push_to_source_branch; + + const currentUser = data.current_user; + + this.cherryPickInForkPath = currentUser.cherry_pick_in_fork_path; + this.revertInForkPath = currentUser.revert_in_fork_path; + + this.canRemoveSourceBranch = currentUser.can_remove_source_branch || false; + this.canCreateIssue = currentUser.can_create_issue || false; + this.canCherryPickInCurrentMR = currentUser.can_cherry_pick_on_current_merge_request || false; + this.canRevertInCurrentMR = currentUser.can_revert_on_current_merge_request || false; this.setState(data); } @@ -161,6 +152,24 @@ export default class MergeRequestStore { } } + setPaths(data) { + // Paths are set on the first load of the page and not auto-refreshed + this.squashBeforeMergeHelpPath = data.squash_before_merge_help_path; + this.troubleshootingDocsPath = data.troubleshooting_docs_path; + this.mergeRequestBasicPath = data.merge_request_basic_path; + this.mergeRequestWidgetPath = data.merge_request_widget_path; + this.mergeRequestCachedWidgetPath = data.merge_request_cached_widget_path; + this.emailPatchesPath = data.email_patches_path; + this.plainDiffPath = data.plain_diff_path; + this.mergeCheckPath = data.merge_check_path; + this.mergeActionsContentPath = data.commit_change_content_path; + this.targetProjectFullPath = data.target_project_full_path; + this.sourceProjectFullPath = data.source_project_full_path; + this.mergeRequestPipelinesHelpPath = data.merge_request_pipelines_docs_path; + this.conflictsDocsPath = data.conflicts_docs_path; + this.ciEnvironmentsStatusPath = data.ci_environments_status_path; + } + get isNothingToMergeState() { return this.state === stateKey.nothingToMerge; } @@ -169,13 +178,6 @@ export default class MergeRequestStore { return this.state === stateKey.merged; } - initRebase(data) { - this.canPushToSourceBranch = data.can_push_to_source_branch; - this.rebaseInProgress = data.rebase_in_progress; - this.approvalsLeft = !data.approved; - this.rebasePath = data.rebase_path; - } - static buildMetrics(metrics) { if (!metrics) { return {}; diff --git a/app/controllers/projects/merge_requests/content_controller.rb b/app/controllers/projects/merge_requests/content_controller.rb index 6e026b83ee3..eec5c1a4355 100644 --- a/app/controllers/projects/merge_requests/content_controller.rb +++ b/app/controllers/projects/merge_requests/content_controller.rb @@ -7,16 +7,33 @@ class Projects::MergeRequests::ContentController < Projects::MergeRequests::Appl # for other types of serialization before_action :close_merge_request_if_no_source_project + before_action :set_polling_header around_action :allow_gitaly_ref_name_caching def widget respond_to do |format| format.json do - Gitlab::PollingInterval.set_header(response, interval: 10_000) + render json: serializer(MergeRequestPollWidgetEntity) + end + end + end - serializer = MergeRequestSerializer.new(current_user: current_user, project: merge_request.project) - render json: serializer.represent(merge_request, serializer: 'widget') + def cached_widget + respond_to do |format| + format.json do + render json: serializer(MergeRequestPollCachedWidgetEntity) end end end + + private + + def set_polling_header + Gitlab::PollingInterval.set_header(response, interval: 10_000) + end + + def serializer(entity) + serializer = MergeRequestSerializer.new(current_user: current_user, project: merge_request.project) + serializer.represent(merge_request, {}, entity) + end end diff --git a/app/models/issue.rb b/app/models/issue.rb index bc5ec94081b..c5a18f0af0f 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -64,7 +64,7 @@ class Issue < ApplicationRecord scope :public_only, -> { where(confidential: false) } scope :confidential_only, -> { where(confidential: true) } - after_save :expire_etag_cache + after_commit :expire_etag_cache after_save :ensure_metrics, unless: :imported? attr_spammable :title, spam_title: true diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 4c4883fc022..4306dd9266f 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -73,6 +73,7 @@ class MergeRequest < ApplicationRecord after_update :clear_memoized_shas after_update :reload_diff_if_branch_changed after_save :ensure_metrics + after_commit :expire_etag_cache # When this attribute is true some MR validation is ignored # It allows us to close or modify broken merge requests @@ -389,6 +390,10 @@ class MergeRequest < ApplicationRecord def merge_async(user_id, params) jid = MergeWorker.perform_async(id, user_id, params.to_h) update_column(:merge_jid, jid) + + # merge_ongoing? depends on merge_jid + # expire etag cache since the attribute is changed without triggering callbacks + expire_etag_cache end # Set off a rebase asynchronously, atomically updating the `rebase_jid` of @@ -409,6 +414,10 @@ class MergeRequest < ApplicationRecord update_column(:rebase_jid, jid) end + + # rebase_in_progress? depends on rebase_jid + # expire etag cache since the attribute is changed without triggering callbacks + expire_etag_cache end def merge_participants @@ -1429,4 +1438,11 @@ class MergeRequest < ApplicationRecord variables.append(key: 'CI_MERGE_REQUEST_SOURCE_BRANCH_NAME', value: source_branch.to_s) end end + + def expire_etag_cache + return unless project.namespace + + key = Gitlab::Routing.url_helpers.cached_widget_project_json_merge_request_path(project, self, format: :json) + Gitlab::EtagCaching::Store.new.touch(key) + end end diff --git a/app/presenters/merge_request_presenter.rb b/app/presenters/merge_request_presenter.rb index 9c44ed711a6..919d2653ec8 100644 --- a/app/presenters/merge_request_presenter.rb +++ b/app/presenters/merge_request_presenter.rb @@ -208,14 +208,6 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated merge_request.subscribed?(current_user, merge_request.target_project) end - def conflicts_docs_path - help_page_path('user/project/merge_requests/resolve_conflicts.md') - end - - def merge_request_pipelines_docs_path - help_page_path('ci/merge_request_pipelines/index.md') - end - def source_branch_link if source_branch_exists? link_to(source_branch, source_branch_commits_path, class: 'ref-name') diff --git a/app/serializers/merge_request_poll_cached_widget_entity.rb b/app/serializers/merge_request_poll_cached_widget_entity.rb new file mode 100644 index 00000000000..005a3e47bbb --- /dev/null +++ b/app/serializers/merge_request_poll_cached_widget_entity.rb @@ -0,0 +1,103 @@ +# frozen_string_literal: true + +class MergeRequestPollCachedWidgetEntity < IssuableEntity + expose :auto_merge_enabled + expose :state + expose :merge_commit_sha + expose :short_merge_commit_sha + expose :merge_error + expose :merge_status + expose :merge_user_id + expose :source_branch + expose :source_project_id + expose :target_branch + expose :target_branch_sha + expose :target_project_id + expose :squash + expose :rebase_in_progress?, as: :rebase_in_progress + expose :default_squash_commit_message + expose :commits_count + expose :merge_ongoing?, as: :merge_ongoing + expose :work_in_progress?, as: :work_in_progress + expose :cannot_be_merged?, as: :has_conflicts + expose :can_be_merged?, as: :can_be_merged + expose :remove_source_branch?, as: :remove_source_branch + expose :source_branch_exists?, as: :source_branch_exists + expose :branch_missing?, as: :branch_missing + + expose :commits_without_merge_commits, using: MergeRequestWidgetCommitEntity do |merge_request| + merge_request.commits.without_merge_commits + end + expose :diff_head_sha do |merge_request| + merge_request.diff_head_sha.presence + end + expose :metrics do |merge_request| + metrics = build_metrics(merge_request) + + MergeRequestMetricsEntity.new(metrics).as_json + end + + expose :diverged_commits_count do |merge_request| + if merge_request.open? && merge_request.diverged_from_target_branch? + merge_request.diverged_commits_count + else + 0 + end + end + + # Paths + # + expose :target_branch_commits_path do |merge_request| + presenter(merge_request).target_branch_commits_path + end + + expose :target_branch_tree_path do |merge_request| + presenter(merge_request).target_branch_tree_path + end + + expose :merge_commit_path do |merge_request| + if merge_request.merge_commit_sha + project_commit_path(merge_request.project, merge_request.merge_commit_sha) + end + end + + expose :source_branch_path do |merge_request| + presenter(merge_request).source_branch_path + end + + expose :source_branch_with_namespace_link do |merge_request| + presenter(merge_request).source_branch_with_namespace_link + end + + private + + delegate :current_user, to: :request + + def presenter(merge_request) + @presenters ||= {} + @presenters[merge_request] ||= MergeRequestPresenter.new(merge_request, current_user: current_user) # rubocop: disable CodeReuse/Presenter + end + + # Once SchedulePopulateMergeRequestMetricsWithEventsData fully runs, + # we can remove this method and just serialize MergeRequest#metrics + # instead. See https://gitlab.com/gitlab-org/gitlab-ce/issues/41587 + def build_metrics(merge_request) + # There's no need to query and serialize metrics data for merge requests that are not + # merged or closed. + return unless merge_request.merged? || merge_request.closed? + return merge_request.metrics if merge_request.merged? && merge_request.metrics&.merged_by_id + return merge_request.metrics if merge_request.closed? && merge_request.metrics&.latest_closed_by_id + + build_metrics_from_events(merge_request) + end + + def build_metrics_from_events(merge_request) + closed_event = merge_request.closed_event + merge_event = merge_request.merge_event + + MergeRequest::Metrics.new(latest_closed_at: closed_event&.updated_at, + latest_closed_by: closed_event&.author, + merged_at: merge_event&.updated_at, + merged_by: merge_event&.author) + end +end diff --git a/app/serializers/merge_request_poll_widget_entity.rb b/app/serializers/merge_request_poll_widget_entity.rb new file mode 100644 index 00000000000..65132b4b215 --- /dev/null +++ b/app/serializers/merge_request_poll_widget_entity.rb @@ -0,0 +1,142 @@ +# frozen_string_literal: true + +class MergeRequestPollWidgetEntity < IssuableEntity + expose :auto_merge_strategy + expose :available_auto_merge_strategies do |merge_request| + AutoMergeService.new(merge_request.project, current_user).available_strategies(merge_request) # rubocop: disable CodeReuse/ServiceClass + end + expose :source_branch_protected do |merge_request| + merge_request.source_project.present? && ProtectedBranch.protected?(merge_request.source_project, merge_request.source_branch) + end + expose :allow_collaboration + expose :should_be_rebased?, as: :should_be_rebased + expose :ff_only_enabled do |merge_request| + merge_request.project.merge_requests_ff_only_enabled + end + + # User entities + expose :merge_user, using: UserEntity + + expose :actual_head_pipeline, with: PipelineDetailsEntity, as: :pipeline, if: -> (mr, _) { presenter(mr).can_read_pipeline? } + + expose :merge_pipeline, with: PipelineDetailsEntity, if: ->(mr, _) { mr.merged? && can?(request.current_user, :read_pipeline, mr.target_project)} + + expose :default_merge_commit_message + + expose :mergeable?, as: :mergeable + + expose :default_merge_commit_message_with_description do |merge_request| + merge_request.default_merge_commit_message(include_description: true) + end + + # Booleans + expose :mergeable_discussions_state?, as: :mergeable_discussions_state do |merge_request| + # This avoids calling MergeRequest#mergeable_discussions_state without + # considering the state of the MR first. If a MR isn't mergeable, we can + # safely short-circuit it. + if merge_request.mergeable_state?(skip_ci_check: true, skip_discussions_check: true) + merge_request.mergeable_discussions_state? + else + false + end + end + + expose :project_archived do |merge_request| + merge_request.project.archived? + end + + expose :only_allow_merge_if_pipeline_succeeds do |merge_request| + merge_request.project.only_allow_merge_if_pipeline_succeeds? + end + + # CI related + expose :has_ci?, as: :has_ci + expose :ci_status do |merge_request| + presenter(merge_request).ci_status + end + + expose :cancel_auto_merge_path do |merge_request| + presenter(merge_request).cancel_auto_merge_path + end + + expose :test_reports_path do |merge_request| + if merge_request.has_test_reports? + test_reports_project_merge_request_path(merge_request.project, merge_request, format: :json) + end + end + + expose :supports_suggestion?, as: :can_receive_suggestion + + expose :create_issue_to_resolve_discussions_path do |merge_request| + presenter(merge_request).create_issue_to_resolve_discussions_path + end + + expose :current_user do + expose :can_remove_source_branch do |merge_request| + presenter(merge_request).can_remove_source_branch? + end + + expose :can_revert_on_current_merge_request do |merge_request| + presenter(merge_request).can_revert_on_current_merge_request? + end + + expose :can_cherry_pick_on_current_merge_request do |merge_request| + presenter(merge_request).can_cherry_pick_on_current_merge_request? + end + + expose :can_create_note do |merge_request| + can?(current_user, :create_note, merge_request) + end + + expose :can_create_issue do |merge_request| + can?(current_user, :create_issue, merge_request.project) + end + + expose :can_update do |merge_request| + can?(current_user, :update_merge_request, merge_request) + end + end + + expose :can_push_to_source_branch do |merge_request| + presenter(merge_request).can_push_to_source_branch? + end + + expose :new_blob_path do |merge_request| + if presenter(merge_request).can_push_to_source_branch? + project_new_blob_path(merge_request.source_project, merge_request.source_branch) + end + end + + expose :rebase_path do |merge_request| + presenter(merge_request).rebase_path + end + + expose :conflict_resolution_path do |merge_request| + presenter(merge_request).conflict_resolution_path + end + + expose :remove_wip_path do |merge_request| + presenter(merge_request).remove_wip_path + end + + expose :merge_path do |merge_request| + presenter(merge_request).merge_path + end + + expose :cherry_pick_in_fork_path do |merge_request| + presenter(merge_request).cherry_pick_in_fork_path + end + + expose :revert_in_fork_path do |merge_request| + presenter(merge_request).revert_in_fork_path + end + + private + + delegate :current_user, to: :request + + def presenter(merge_request) + @presenters ||= {} + @presenters[merge_request] ||= MergeRequestPresenter.new(merge_request, current_user: current_user) # rubocop: disable CodeReuse/Presenter + end +end diff --git a/app/serializers/merge_request_serializer.rb b/app/serializers/merge_request_serializer.rb index 6f589351670..8ad1df5dfe0 100644 --- a/app/serializers/merge_request_serializer.rb +++ b/app/serializers/merge_request_serializer.rb @@ -4,8 +4,8 @@ class MergeRequestSerializer < BaseSerializer # This overrided method takes care of which entity should be used # to serialize the `merge_request` based on `serializer` key in `opts` param. # Hence, `entity` doesn't need to be declared on the class scope. - def represent(merge_request, opts = {}) - entity = + def represent(merge_request, opts = {}, entity = nil) + entity ||= case opts[:serializer] when 'sidebar' IssuableSidebarBasicEntity diff --git a/app/serializers/merge_request_widget_entity.rb b/app/serializers/merge_request_widget_entity.rb index fd2673fa0cc..554b307d4f8 100644 --- a/app/serializers/merge_request_widget_entity.rb +++ b/app/serializers/merge_request_widget_entity.rb @@ -1,117 +1,62 @@ # frozen_string_literal: true -class MergeRequestWidgetEntity < IssuableEntity - expose :state - expose :in_progress_merge_commit_sha - expose :merge_commit_sha - expose :short_merge_commit_sha - expose :merge_error +class MergeRequestWidgetEntity < Grape::Entity + include RequestAwareEntity + + # Currently this attr is exposed to be used in app/assets/javascripts/notes/stores/getters.js + # in order to determine whether a noteable is an issue or an MR expose :merge_params - expose :merge_status - expose :merge_user_id - expose :auto_merge_enabled - expose :auto_merge_strategy - expose :available_auto_merge_strategies do |merge_request| - AutoMergeService.new(merge_request.project, current_user).available_strategies(merge_request) # rubocop: disable CodeReuse/ServiceClass - end - expose :source_branch - expose :source_branch_protected do |merge_request| - merge_request.source_project.present? && ProtectedBranch.protected?(merge_request.source_project, merge_request.source_branch) - end - expose :source_project_id + expose :source_project_full_path do |merge_request| merge_request.source_project&.full_path end - expose :squash - expose :target_branch - expose :target_branch_sha - expose :target_project_id + expose :target_project_full_path do |merge_request| merge_request.project&.full_path end - expose :allow_collaboration - expose :should_be_rebased?, as: :should_be_rebased - expose :ff_only_enabled do |merge_request| - merge_request.project.merge_requests_ff_only_enabled + expose :email_patches_path do |merge_request| + project_merge_request_path(merge_request.project, merge_request, format: :patch) end - expose :metrics do |merge_request| - metrics = build_metrics(merge_request) - - MergeRequestMetricsEntity.new(metrics).as_json + expose :plain_diff_path do |merge_request| + project_merge_request_path(merge_request.project, merge_request, format: :diff) end - expose :rebase_commit_sha - expose :rebase_in_progress?, as: :rebase_in_progress - - expose :can_push_to_source_branch do |merge_request| - presenter(merge_request).can_push_to_source_branch? + expose :merge_request_basic_path do |merge_request| + project_merge_request_path(merge_request.target_project, merge_request, serializer: :basic, format: :json) end - expose :rebase_path do |merge_request| - presenter(merge_request).rebase_path + expose :merge_request_widget_path do |merge_request| + widget_project_json_merge_request_path(merge_request.target_project, merge_request, format: :json) end - # User entities - expose :merge_user, using: UserEntity - - # Diff sha's - expose :diff_head_sha do |merge_request| - merge_request.diff_head_sha.presence + expose :merge_request_cached_widget_path do |merge_request| + cached_widget_project_json_merge_request_path(merge_request.target_project, merge_request, format: :json) end - expose :actual_head_pipeline, with: PipelineDetailsEntity, as: :pipeline, if: -> (mr, _) { presenter(mr).can_read_pipeline? } - - expose :merge_pipeline, with: PipelineDetailsEntity, if: ->(mr, _) { mr.merged? && can?(request.current_user, :read_pipeline, mr.target_project)} - - expose :default_squash_commit_message - expose :default_merge_commit_message - - expose :default_merge_commit_message_with_description do |merge_request| - merge_request.default_merge_commit_message(include_description: true) + expose :create_note_path do |merge_request| + project_notes_path(merge_request.project, target_type: 'merge_request', target_id: merge_request.id) end - expose :commits_without_merge_commits, using: MergeRequestWidgetCommitEntity do |merge_request| - merge_request.commits.without_merge_commits + expose :commit_change_content_path do |merge_request| + commit_change_content_project_merge_request_path(merge_request.project, merge_request) end - expose :commits_count - - # Booleans - expose :merge_ongoing?, as: :merge_ongoing - expose :work_in_progress?, as: :work_in_progress - expose :source_branch_exists?, as: :source_branch_exists - - expose :mergeable_discussions_state?, as: :mergeable_discussions_state do |merge_request| - # This avoids calling MergeRequest#mergeable_discussions_state without - # considering the state of the MR first. If a MR isn't mergeable, we can - # safely short-circuit it. - if merge_request.mergeable_state?(skip_ci_check: true, skip_discussions_check: true) - merge_request.mergeable_discussions_state? - else - false - end + expose :preview_note_path do |merge_request| + preview_markdown_path(merge_request.project, target_type: 'MergeRequest', target_id: merge_request.iid) end - expose :branch_missing?, as: :branch_missing - expose :cannot_be_merged?, as: :has_conflicts - expose :can_be_merged?, as: :can_be_merged - expose :mergeable?, as: :mergeable - expose :remove_source_branch?, as: :remove_source_branch - - expose :project_archived do |merge_request| - merge_request.project.archived? + expose :conflicts_docs_path do |merge_request| + help_page_path('user/project/merge_requests/resolve_conflicts.md') end - expose :only_allow_merge_if_pipeline_succeeds do |merge_request| - merge_request.project.only_allow_merge_if_pipeline_succeeds? + expose :merge_request_pipelines_docs_path do |merge_request| + help_page_path('ci/merge_request_pipelines/index.md') end - # CI related - expose :has_ci?, as: :has_ci - expose :ci_status do |merge_request| - presenter(merge_request).ci_status + expose :ci_environments_status_path do |merge_request| + ci_environments_status_project_merge_request_path(merge_request.project, merge_request) end # Rendering and redacting Markdown can be expensive. These links are @@ -131,175 +76,16 @@ class MergeRequestWidgetEntity < IssuableEntity end end - expose :source_branch_with_namespace_link do |merge_request| - presenter(merge_request).source_branch_with_namespace_link - end - - expose :source_branch_path do |merge_request| - presenter(merge_request).source_branch_path - end - - expose :current_user do - expose :can_remove_source_branch do |merge_request| - presenter(merge_request).can_remove_source_branch? - end - - expose :can_revert_on_current_merge_request do |merge_request| - presenter(merge_request).can_revert_on_current_merge_request? - end - - expose :can_cherry_pick_on_current_merge_request do |merge_request| - presenter(merge_request).can_cherry_pick_on_current_merge_request? - end - - expose :can_create_note do |merge_request| - can?(request.current_user, :create_note, merge_request) - end - - expose :can_create_issue do |merge_request| - can?(current_user, :create_issue, merge_request.project) - end - - expose :can_update do |merge_request| - can?(request.current_user, :update_merge_request, merge_request) - end - end - - # Paths - # - expose :target_branch_commits_path do |merge_request| - presenter(merge_request).target_branch_commits_path - end - - expose :target_branch_tree_path do |merge_request| - presenter(merge_request).target_branch_tree_path - end - - expose :new_blob_path do |merge_request| - if presenter(merge_request).can_push_to_source_branch? - project_new_blob_path(merge_request.source_project, merge_request.source_branch) - end - end - - expose :conflict_resolution_path do |merge_request| - presenter(merge_request).conflict_resolution_path - end - - expose :remove_wip_path do |merge_request| - presenter(merge_request).remove_wip_path - end - - expose :cancel_auto_merge_path do |merge_request| - presenter(merge_request).cancel_auto_merge_path - end - - expose :create_issue_to_resolve_discussions_path do |merge_request| - presenter(merge_request).create_issue_to_resolve_discussions_path - end - - expose :merge_path do |merge_request| - presenter(merge_request).merge_path - end - - expose :cherry_pick_in_fork_path do |merge_request| - presenter(merge_request).cherry_pick_in_fork_path - end - - expose :revert_in_fork_path do |merge_request| - presenter(merge_request).revert_in_fork_path - end - - expose :email_patches_path do |merge_request| - project_merge_request_path(merge_request.project, merge_request, format: :patch) - end - - expose :plain_diff_path do |merge_request| - project_merge_request_path(merge_request.project, merge_request, format: :diff) - end - - expose :merge_request_basic_path do |merge_request| - project_merge_request_path(merge_request.target_project, merge_request, serializer: :basic, format: :json) - end - - expose :merge_request_widget_path do |merge_request| - widget_project_json_merge_request_path(merge_request.target_project, merge_request, format: :json) - end - - expose :ci_environments_status_path do |merge_request| - ci_environments_status_project_merge_request_path(merge_request.project, merge_request) - end - - expose :diverged_commits_count do |merge_request| - if merge_request.open? && merge_request.diverged_from_target_branch? - merge_request.diverged_commits_count - else - 0 - end - end - - expose :create_note_path do |merge_request| - project_notes_path(merge_request.project, target_type: 'merge_request', target_id: merge_request.id) - end - - expose :commit_change_content_path do |merge_request| - commit_change_content_project_merge_request_path(merge_request.project, merge_request) - end - - expose :preview_note_path do |merge_request| - preview_markdown_path(merge_request.project, target_type: 'MergeRequest', target_id: merge_request.iid) - end - - expose :merge_commit_path do |merge_request| - if merge_request.merge_commit_sha - project_commit_path(merge_request.project, merge_request.merge_commit_sha) - end - end - - expose :test_reports_path do |merge_request| - if merge_request.has_test_reports? - test_reports_project_merge_request_path(merge_request.project, merge_request, format: :json) - end - end - - expose :supports_suggestion?, as: :can_receive_suggestion - - expose :conflicts_docs_path do |merge_request| - presenter(merge_request).conflicts_docs_path - end - - expose :merge_request_pipelines_docs_path do |merge_request| - presenter(merge_request).merge_request_pipelines_docs_path + def as_json(options = {}) + super(options) + .merge(MergeRequestPollCachedWidgetEntity.new(object, **@options.opts_hash).as_json(options)) + .merge(MergeRequestPollWidgetEntity.new(object, **@options.opts_hash).as_json(options)) end private - delegate :current_user, to: :request - def presenter(merge_request) @presenters ||= {} - @presenters[merge_request] ||= MergeRequestPresenter.new(merge_request, current_user: current_user) # rubocop: disable CodeReuse/Presenter - end - - # Once SchedulePopulateMergeRequestMetricsWithEventsData fully runs, - # we can remove this method and just serialize MergeRequest#metrics - # instead. See https://gitlab.com/gitlab-org/gitlab-ce/issues/41587 - def build_metrics(merge_request) - # There's no need to query and serialize metrics data for merge requests that are not - # merged or closed. - return unless merge_request.merged? || merge_request.closed? - return merge_request.metrics if merge_request.merged? && merge_request.metrics&.merged_by_id - return merge_request.metrics if merge_request.closed? && merge_request.metrics&.latest_closed_by_id - - build_metrics_from_events(merge_request) - end - - def build_metrics_from_events(merge_request) - closed_event = merge_request.closed_event - merge_event = merge_request.merge_event - - MergeRequest::Metrics.new(latest_closed_at: closed_event&.updated_at, - latest_closed_by: closed_event&.author, - merged_at: merge_event&.updated_at, - merged_by: merge_event&.author) + @presenters[merge_request] ||= MergeRequestPresenter.new(merge_request, current_user: request.current_user) # rubocop: disable CodeReuse/Presenter end end diff --git a/changelogs/unreleased/id-mr-widget-etag-caching.yml b/changelogs/unreleased/id-mr-widget-etag-caching.yml new file mode 100644 index 00000000000..69bf89991d4 --- /dev/null +++ b/changelogs/unreleased/id-mr-widget-etag-caching.yml @@ -0,0 +1,5 @@ +--- +title: Split MR widget into etag-cached and non-cached serializers +merge_request: 31354 +author: +type: performance diff --git a/config/routes/project.rb b/config/routes/project.rb index 380ecad001d..b9258a35f0c 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -267,6 +267,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do get :pipelines get :diffs, to: 'merge_requests/diffs#show' get :widget, to: 'merge_requests/content#widget' + get :cached_widget, to: 'merge_requests/content#cached_widget' end get :diff_for_path, controller: 'merge_requests/diffs' diff --git a/lib/gitlab/etag_caching/router.rb b/lib/gitlab/etag_caching/router.rb index 17fbecbd097..d09dcdbb337 100644 --- a/lib/gitlab/etag_caching/router.rb +++ b/lib/gitlab/etag_caching/router.rb @@ -61,6 +61,10 @@ module Gitlab Gitlab::EtagCaching::Router::Route.new( %r(#{RESERVED_WORDS_PREFIX}/import/gitea/realtime_changes\.json\z), 'realtime_changes_import_gitea' + ), + Gitlab::EtagCaching::Router::Route.new( + %r(#{RESERVED_WORDS_PREFIX}/merge_requests/\d+/cached_widget\.json\z), + 'merge_request_widget' ) ].freeze diff --git a/spec/controllers/projects/merge_requests/content_controller_spec.rb b/spec/controllers/projects/merge_requests/content_controller_spec.rb index 2879e06aee4..818cf794ec6 100644 --- a/spec/controllers/projects/merge_requests/content_controller_spec.rb +++ b/spec/controllers/projects/merge_requests/content_controller_spec.rb @@ -11,8 +11,8 @@ describe Projects::MergeRequests::ContentController do sign_in(user) end - def do_request - get :widget, params: { + def do_request(action = :cached_widget) + get action, params: { namespace_id: project.namespace.to_param, project_id: project, id: merge_request.iid, @@ -20,41 +20,65 @@ describe Projects::MergeRequests::ContentController do } end - describe 'GET widget' do - context 'user has access to the project' do - before do - expect(::Gitlab::GitalyClient).to receive(:allow_ref_name_caching).and_call_original + context 'user has access to the project' do + before do + expect(::Gitlab::GitalyClient).to receive(:allow_ref_name_caching).and_call_original - project.add_maintainer(user) - end + project.add_maintainer(user) + end + describe 'GET cached_widget' do it 'renders widget MR entity as json' do do_request - expect(response).to match_response_schema('entities/merge_request_widget') + expect(response).to match_response_schema('entities/merge_request_poll_cached_widget') end + it 'closes an MR with moved source project' do + merge_request.update_column(:source_project_id, nil) + + expect { do_request }.to change { merge_request.reload.open? }.from(true).to(false) + end + end + + describe 'GET widget' do it 'checks whether the MR can be merged' do controller.instance_variable_set(:@merge_request, merge_request) expect(merge_request).to receive(:check_mergeability) - do_request + do_request(:widget) end - it 'closes an MR with moved source project' do - merge_request.update_column(:source_project_id, nil) + context 'merged merge request' do + let(:merge_request) do + create(:merged_merge_request, :with_test_reports, target_project: project, source_project: project) + end - expect { do_request }.to change { merge_request.reload.open? }.from(true).to(false) + it 'renders widget MR entity as json' do + do_request(:widget) + + expect(response).to match_response_schema('entities/merge_request_poll_widget') + end end end + end - context 'user does not have access to the project' do - it 'renders widget MR entity as json' do + context 'user does not have access to the project' do + describe 'GET cached_widget' do + it 'returns 404' do do_request expect(response).to have_http_status(:not_found) end end + + describe 'GET widget' do + it 'returns 404' do + do_request(:widget) + + expect(response).to have_http_status(:not_found) + end + end end end diff --git a/spec/fixtures/api/schemas/entities/merge_request_poll_cached_widget.json b/spec/fixtures/api/schemas/entities/merge_request_poll_cached_widget.json new file mode 100644 index 00000000000..e8959307767 --- /dev/null +++ b/spec/fixtures/api/schemas/entities/merge_request_poll_cached_widget.json @@ -0,0 +1,46 @@ +{ + "type": "object", + "properties" : { + "id": { "type": "integer" }, + "iid": { "type": "integer" }, + "description": { "type": ["string", "null"] }, + "title": { "type": "string" }, + "auto_merge_enabled": { "type": "boolean" }, + "state": { "type": "string" }, + "merge_commit_sha": { "type": ["string", "null"] }, + "short_merge_commit_sha": { "type": ["string", "null"] }, + "merge_error": { "type": ["string", "null"] }, + "merge_status": { "type": "string" }, + "merge_user_id": { "type": ["integer", "null"] }, + "source_branch": { "type": "string" }, + "source_project_id": { "type": "integer" }, + "target_branch": { "type": "string" }, + "target_branch_sha": { "type": "string" }, + "target_project_id": { "type": "integer" }, + "squash": { "type": "boolean" }, + "rebase_in_progress": { "type": "boolean" }, + "default_squash_commit_message": { "type": "string" }, + "commits_count": { "type": ["integer", "null"] }, + "merge_ongoing": { "type": "boolean" }, + "work_in_progress": { "type": "boolean" }, + "has_conflicts": { "type": "boolean" }, + "can_be_merged": { "type": "boolean" }, + "remove_source_branch": { "type": ["boolean", "null"] }, + "source_branch_exists": { "type": "boolean" }, + "branch_missing": { "type": "boolean" }, + "commits_without_merge_commits": { "type": "array" }, + "diff_head_sha": { "type": ["string", "null"] }, + "metrics": { + "oneOf": [ + { "type": "null" }, + { "$ref": "merge_request_metrics.json" } + ] + }, + "diverged_commits_count": { "type": "integer" }, + "target_branch_commits_path": { "type": "string" }, + "target_branch_tree_path": { "type": "string" }, + "merge_commit_path": { "type": ["string", "null"] }, + "source_branch_with_namespace_link": { "type": "string" }, + "source_branch_path": { "type": "string" } + } +} diff --git a/spec/fixtures/api/schemas/entities/merge_request_poll_widget.json b/spec/fixtures/api/schemas/entities/merge_request_poll_widget.json new file mode 100644 index 00000000000..2052892dfa3 --- /dev/null +++ b/spec/fixtures/api/schemas/entities/merge_request_poll_widget.json @@ -0,0 +1,55 @@ +{ + "type": "object", + "properties" : { + "id": { "type": "integer" }, + "iid": { "type": "integer" }, + "description": { "type": ["string", "null"] }, + "title": { "type": "string" }, + "auto_merge_strategy": { "type": ["string", "null"] }, + "available_auto_merge_strategies": { "type": "array" }, + "source_branch_protected": { "type": "boolean" }, + "allow_collaboration": { "type": "boolean"}, + "should_be_rebased": { "type": "boolean" }, + "ff_only_enabled": { "type": ["boolean", false] }, + "merge_user": { "type": ["object", "null"] }, + "pipeline": { "type": ["object", "null"] }, + "merge_pipeline": { "type": ["object", "null"] }, + "default_merge_commit_message": { "type": ["string", "null"] }, + "mergeable": { "type": "boolean" }, + "default_merge_commit_message_with_description": { "type": "string" }, + "mergeable_discussions_state": { "type": "boolean" }, + "project_archived": { "type": "boolean" }, + "only_allow_merge_if_pipeline_succeeds": { "type": "boolean" }, + "has_ci": { "type": "boolean" }, + "ci_status": { "type": ["string", "null"] }, + "cancel_auto_merge_path": { "type": ["string", "null"] }, + "test_reports_path": { "type": ["string", "null"] }, + "can_receive_suggestion": { "type": "boolean" }, + "create_issue_to_resolve_discussions_path": { "type": ["string", "null"] }, + "current_user": { + "type": "object", + "required": [ + "can_remove_source_branch", + "can_revert_on_current_merge_request", + "can_cherry_pick_on_current_merge_request" + ], + "properties": { + "can_remove_source_branch": { "type": "boolean" }, + "can_revert_on_current_merge_request": { "type": ["boolean", "null"] }, + "can_cherry_pick_on_current_merge_request": { "type": ["boolean", "null"] }, + "can_create_note": { "type": "boolean" }, + "can_create_issue": { "type": "boolean" }, + "can_update": { "type": "boolean" } + }, + "additionalProperties": false + }, + "can_push_to_source_branch": { "type": "boolean" }, + "new_blob_path": { "type": ["string", "null"] }, + "rebase_path": { "type": ["string", "null"] }, + "conflict_resolution_path": { "type": ["string", "null"] }, + "remove_wip_path": { "type": ["string", "null"] }, + "merge_path": { "type": ["string", "null"] }, + "cherry_pick_in_fork_path": { "type": ["string", "null"] }, + "revert_in_fork_path": { "type": ["string", "null"] } + } +} diff --git a/spec/fixtures/api/schemas/entities/merge_request_widget.json b/spec/fixtures/api/schemas/entities/merge_request_widget.json index eac1dbc6474..779a47222b7 100644 --- a/spec/fixtures/api/schemas/entities/merge_request_widget.json +++ b/spec/fixtures/api/schemas/entities/merge_request_widget.json @@ -1,132 +1,35 @@ { "type": "object", - "properties" : { - "id": { "type": "integer" }, - "iid": { "type": "integer" }, - "author_id": { "type": "integer" }, - "description": { "type": ["string", "null"] }, - "lock_version": { "type": ["string", "null"] }, - "milestone_id": { "type": ["string", "null"] }, - "position": { "type": "integer" }, - "state": { "type": "string" }, - "title": { "type": "string" }, - "updated_by_id": { "type": ["string", "null"] }, - "created_at": { "type": "string" }, - "updated_at": { "type": "string" }, - "time_estimate": { "type": "integer" }, - "total_time_spent": { "type": "integer" }, - "human_time_estimate": { "type": ["integer", "null"] }, - "human_total_time_spent": { "type": ["integer", "null"] }, - "milestone": { "type": ["object", "null"] }, - "labels": { "type": ["array", "null"] }, - "in_progress_merge_commit_sha": { "type": ["string", "null"] }, - "merge_error": { "type": ["string", "null"] }, - "merge_commit_sha": { "type": ["string", "null"] }, - "short_merge_commit_sha": { "type": ["string", "null"] }, - "merge_params": { "type": ["object", "null"] }, - "merge_status": { "type": "string" }, - "merge_user_id": { "type": ["integer", "null"] }, - "merge_when_pipeline_succeeds": { "type": "boolean" }, - "source_branch": { "type": "string" }, - "source_project_id": { "type": "integer" }, - "source_project_full_path": { "type": ["string", "null"]}, - "target_branch": { "type": "string" }, - "target_project_id": { "type": "integer" }, - "target_project_full_path": { "type": ["string", "null"]}, - "allow_collaboration": { "type": "boolean"}, - "metrics": { - "oneOf": [ - { "type": "null" }, - { "$ref": "merge_request_metrics.json" } - ] - }, - "author": { "type": ["object", "null"] }, - "merge_user": { "type": ["object", "null"] }, - "diff_head_sha": { "type": ["string", "null"] }, - "diff_head_commit_short_id": { "type": ["string", "null"] }, - "default_merge_commit_message": { "type": ["string", "null"] }, - "pipeline": { "type": ["object", "null"] }, - "merge_pipeline": { "type": ["object", "null"] }, - "work_in_progress": { "type": "boolean" }, - "source_branch_exists": { "type": "boolean" }, - "mergeable_discussions_state": { "type": "boolean" }, - "conflicts_can_be_resolved_in_ui": { "type": "boolean" }, - "branch_missing": { "type": "boolean" }, - "commits_count": { "type": ["integer", "null"] }, - "has_conflicts": { "type": "boolean" }, - "can_be_merged": { "type": "boolean" }, - "mergeable": { "type": "boolean" }, - "project_archived": { "type": "boolean" }, - "only_allow_merge_if_pipeline_succeeds": { "type": "boolean" }, - "has_ci": { "type": "boolean" }, - "ci_status": { "type": ["string", "null"] }, - "issues_links": { - "type": "object", - "required": ["closing", "mentioned_but_not_closing", "assign_to_closing"], + "allOf": [ + { "$ref": "merge_request_poll_cached_widget.json" }, + { "$ref": "merge_request_poll_widget.json" }, + { "properties" : { - "closing": { "type": "string" }, - "mentioned_but_not_closing": { "type": "string" }, - "assign_to_closing": { "type": ["string", "null"] } - }, - "additionalProperties": false - }, - "source_branch_with_namespace_link": { "type": "string" }, - "current_user": { - "type": "object", - "required": [ - "can_remove_source_branch", - "can_revert_on_current_merge_request", - "can_cherry_pick_on_current_merge_request" - ], - "properties": { - "can_remove_source_branch": { "type": "boolean" }, - "can_revert_on_current_merge_request": { "type": ["boolean", "null"] }, - "can_cherry_pick_on_current_merge_request": { "type": ["boolean", "null"] }, - "can_create_note": { "type": "boolean" }, - "can_create_issue": { "type": "boolean" }, - "can_update": { "type": "boolean" } - }, - "additionalProperties": false - }, - "target_branch_commits_path": { "type": "string" }, - "target_branch_tree_path": { "type": "string" }, - "source_branch_path": { "type": "string" }, - "conflict_resolution_path": { "type": ["string", "null"] }, - "cancel_merge_when_pipeline_succeeds_path": { "type": ["string", "null"] }, - "create_issue_to_resolve_discussions_path": { "type": ["string", "null"] }, - "merge_path": { "type": ["string", "null"] }, - "cherry_pick_in_fork_path": { "type": ["string", "null"] }, - "revert_in_fork_path": { "type": ["string", "null"] }, - "email_patches_path": { "type": "string" }, - "plain_diff_path": { "type": "string" }, - "merge_request_basic_path": { "type": "string" }, - "merge_request_widget_path": { "type": "string" }, - "new_blob_path": { "type": ["string", "null"] }, - "merge_check_path": { "type": "string" }, - "ci_environments_status_path": { "type": "string" }, - "default_merge_commit_message_with_description": { "type": "string" }, - "default_squash_commit_message": { "type": "string" }, - "commits_without_merge_commits": { "type": "array" }, - "diverged_commits_count": { "type": "integer" }, - "commit_change_content_path": { "type": "string" }, - "merge_commit_path": { "type": ["string", "null"] }, - "remove_wip_path": { "type": ["string", "null"] }, - "commits_count": { "type": "integer" }, - "remove_source_branch": { "type": ["boolean", "null"] }, - "merge_ongoing": { "type": "boolean" }, - "ff_only_enabled": { "type": ["boolean", false] }, - "should_be_rebased": { "type": "boolean" }, - "create_note_path": { "type": ["string", "null"] }, - "preview_note_path": { "type": ["string", "null"] }, - "rebase_commit_sha": { "type": ["string", "null"] }, - "rebase_in_progress": { "type": "boolean" }, - "can_push_to_source_branch": { "type": "boolean" }, - "rebase_path": { "type": ["string", "null"] }, - "squash": { "type": "boolean" }, - "test_reports_path": { "type": ["string", "null"] }, - "can_receive_suggestion": { "type": "boolean" }, - "source_branch_protected": { "type": "boolean" }, - "conflicts_docs_path": { "type": ["string", "null"] }, - "merge_request_pipelines_docs_path": { "type": ["string", "null"] } - } + "merge_params": { "type": ["object", "null"] }, + "source_project_full_path": { "type": ["string", "null"]}, + "target_project_full_path": { "type": ["string", "null"]}, + "email_patches_path": { "type": "string" }, + "plain_diff_path": { "type": "string" }, + "merge_request_basic_path": { "type": "string" }, + "merge_request_widget_path": { "type": "string" }, + "merge_request_cached_widget_path": { "type": "string" }, + "create_note_path": { "type": ["string", "null"] }, + "commit_change_content_path": { "type": "string" }, + "preview_note_path": { "type": ["string", "null"] }, + "conflicts_docs_path": { "type": ["string", "null"] }, + "merge_request_pipelines_docs_path": { "type": ["string", "null"] }, + "ci_environments_status_path": { "type": "string" }, + "issues_links": { + "type": "object", + "required": ["closing", "mentioned_but_not_closing", "assign_to_closing"], + "properties" : { + "closing": { "type": "string" }, + "mentioned_but_not_closing": { "type": "string" }, + "assign_to_closing": { "type": ["string", "null"] } + }, + "additionalProperties": false + } + } + } + ] } diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 6a5bd276233..53424204db7 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -1988,6 +1988,7 @@ describe MergeRequest do params = {} merge_jid = 'hash-123' + expect(merge_request).to receive(:expire_etag_cache) expect(MergeWorker).to receive(:perform_async).with(merge_request.id, user_id, params) do merge_jid end @@ -2011,6 +2012,7 @@ describe MergeRequest do .with(merge_request.id, user_id) .and_return(rebase_jid) + expect(merge_request).to receive(:expire_etag_cache) expect(merge_request).to receive(:lock!).and_call_original execute -- cgit v1.2.1