root
/
gitlab-ce
mirror of https://gitlab.com/free-releases/gitlab-ce.git
1
0
Fork 0
Code Issues Actions Packages Projects Releases Wiki Activity

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
This commit is contained in:
Igor 2019-08-09 21:01:55 +00:00 committed by Douwe Maan
parent 43b9be9d6c
commit b99011af62
19 changed files with 565 additions and 458 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue
View File

@ -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();

11
app/assets/javascripts/vue_merge_request_widget/services/mr_widget_service.js
View File

@ -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() {

92
app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js
View File

@ -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 {};

25
app/controllers/projects/merge_requests/content_controller.rb
View File

@ -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)
serializer = MergeRequestSerializer.new(current_user: current_user, project: merge_request.project)
render json: serializer.represent(merge_request, serializer: 'widget')
render json: serializer(MergeRequestPollWidgetEntity)
end
end
end
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

2
app/models/issue.rb
View File

@ -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

16
app/models/merge_request.rb
View File

@ -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

8
app/presenters/merge_request_presenter.rb
View File

@ -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')

103
app/serializers/merge_request_poll_cached_widget_entity.rb Normal file
View File

@ -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

142
app/serializers/merge_request_poll_widget_entity.rb Normal file
View File

@ -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

4
app/serializers/merge_request_serializer.rb
View File

@ -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

282
app/serializers/merge_request_widget_entity.rb
View File

@ -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

5
changelogs/unreleased/id-mr-widget-etag-caching.yml Normal file
View File

@ -0,0 +1,5 @@
---
title: Split MR widget into etag-cached and non-cached serializers
merge_request: 31354
author:
type: performance

1
config/routes/project.rb
View File

@ -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'

4
lib/gitlab/etag_caching/router.rb
View File

@ -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

62
spec/controllers/projects/merge_requests/content_controller_spec.rb
View File

@ -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,26 +20,18 @@ 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')
end
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
expect(response).to match_response_schema('entities/merge_request_poll_cached_widget')
end
it 'closes an MR with moved source project' do
@ -49,12 +41,44 @@ describe Projects::MergeRequests::ContentController do
end
end
context 'user does not have access to the project' do
it 'renders widget MR entity as json' do
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(:widget)
end
context 'merged merge request' do
let(:merge_request) do
create(:merged_merge_request, :with_test_reports, target_project: project, source_project: project)
end
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
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

46
spec/fixtures/api/schemas/entities/merge_request_poll_cached_widget.json vendored Normal file
View File

@ -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" }
}
}

55
spec/fixtures/api/schemas/entities/merge_request_poll_widget.json vendored Normal file
View File

@ -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"] }
}
}

159
spec/fixtures/api/schemas/entities/merge_request_widget.json vendored
View File

@ -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
}
}
}
]
}

2
spec/models/merge_request_spec.rb
View File

@ -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