Skip to content
Projects
Groups
Snippets
Help
Loading...
Help
Support
Keyboard shortcuts
?
Submit feedback
Contribute to GitLab
Sign in / Register
Toggle navigation
G
gitlab-ce
Project overview
Project overview
Details
Activity
Releases
Repository
Repository
Files
Commits
Branches
Tags
Contributors
Graph
Compare
Issues
0
Issues
0
List
Boards
Labels
Milestones
Merge Requests
0
Merge Requests
0
Analytics
Analytics
Repository
Value Stream
Wiki
Wiki
Snippets
Snippets
Members
Members
Collapse sidebar
Close sidebar
Activity
Graph
Create a new issue
Commits
Issue Boards
Open sidebar
Boxiang Sun
gitlab-ce
Commits
f4be2d4e
Commit
f4be2d4e
authored
Feb 26, 2019
by
Heinrich Lee Yu
Browse files
Options
Browse Files
Download
Email Patches
Plain Diff
Revert "Merge branch '56726-fix-n+1-in-issues-and-merge-requests-api' into 'master'"
This reverts merge request !25042
parent
f5201a81
Changes
7
Hide whitespace changes
Inline
Side-by-side
Showing
7 changed files
with
58 additions
and
51 deletions
+58
-51
app/models/concerns/issuable.rb
app/models/concerns/issuable.rb
+3
-3
app/models/issue.rb
app/models/issue.rb
+0
-4
app/views/shared/_issuable_meta_data.html.haml
app/views/shared/_issuable_meta_data.html.haml
+1
-1
changelogs/unreleased/56726-fix-n-1-in-issues-and-merge-requests-api.yml
...leased/56726-fix-n-1-in-issues-and-merge-requests-api.yml
+0
-5
lib/api/entities.rb
lib/api/entities.rb
+50
-22
spec/lib/gitlab/issuable_metadata_spec.rb
spec/lib/gitlab/issuable_metadata_spec.rb
+4
-4
spec/requests/api/merge_requests_spec.rb
spec/requests/api/merge_requests_spec.rb
+0
-12
No files found.
app/models/concerns/issuable.rb
View file @
f4be2d4e
...
...
@@ -28,7 +28,7 @@ module Issuable
# This object is used to gather issuable meta data for displaying
# upvotes, downvotes, notes and closing merge requests count for issues and merge requests
# lists avoiding n+1 queries and improving performance.
IssuableMeta
=
Struct
.
new
(
:upvotes
,
:downvotes
,
:
user_
notes_count
,
:merge_requests_count
)
IssuableMeta
=
Struct
.
new
(
:upvotes
,
:downvotes
,
:notes_count
,
:merge_requests_count
)
included
do
cache_markdown_field
:title
,
pipeline: :single_line
...
...
@@ -36,8 +36,8 @@ module Issuable
redact_field
:description
belongs_to
:author
,
class_name:
'User'
belongs_to
:updated_by
,
class_name:
'User'
belongs_to
:author
,
class_name:
"User"
belongs_to
:updated_by
,
class_name:
"User"
belongs_to
:last_edited_by
,
class_name:
'User'
belongs_to
:milestone
...
...
app/models/issue.rb
View file @
f4be2d4e
...
...
@@ -263,10 +263,6 @@ class Issue < ActiveRecord::Base
end
# rubocop: enable CodeReuse/ServiceClass
def
merge_requests_count
merge_requests_closing_issues
.
count
end
private
def
ensure_metrics
...
...
app/views/shared/_issuable_meta_data.html.haml
View file @
f4be2d4e
-
note_count
=
@issuable_meta_data
[
issuable
.
id
].
user_
notes_count
-
note_count
=
@issuable_meta_data
[
issuable
.
id
].
notes_count
-
issue_votes
=
@issuable_meta_data
[
issuable
.
id
]
-
upvotes
,
downvotes
=
issue_votes
.
upvotes
,
issue_votes
.
downvotes
-
issuable_url
=
@collection_type
==
"Issue"
?
issue_path
(
issuable
,
anchor:
'notes'
)
:
merge_request_path
(
issuable
,
anchor:
'notes'
)
...
...
changelogs/unreleased/56726-fix-n-1-in-issues-and-merge-requests-api.yml
deleted
100644 → 0
View file @
f5201a81
---
title
:
Fix N+1 query in Issues and MergeRequest API when issuable_metadata is present
merge_request
:
25042
author
:
Alex Koval
type
:
other
lib/api/entities.rb
View file @
f4be2d4e
...
...
@@ -468,17 +468,11 @@ module API
end
end
class
Issueable
Entity
<
Grape
::
Entity
class
Project
Entity
<
Grape
::
Entity
expose
:id
,
:iid
expose
(
:project_id
)
{
|
entity
|
entity
&
.
project
.
try
(
:id
)
}
expose
:title
,
:description
expose
:state
,
:created_at
,
:updated_at
# Avoids an N+1 query when metadata is included
def
issuable_metadata
(
subject
,
options
,
method
)
cached_subject
=
options
.
dig
(
:issuable_metadata
,
subject
.
id
)
(
cached_subject
||
subject
).
public_send
(
method
)
# rubocop: disable GitlabSecurity/PublicSend
end
end
class
Diff
<
Grape
::
Entity
...
...
@@ -521,35 +515,57 @@ module API
end
end
class
IssueBasic
<
Issueable
Entity
class
IssueBasic
<
Project
Entity
expose
:closed_at
expose
:closed_by
,
using:
Entities
::
UserBasic
expose
:labels
do
|
issue
|
expose
:labels
do
|
issue
,
options
|
# Avoids an N+1 query since labels are preloaded
issue
.
labels
.
map
(
&
:title
).
sort
end
expose
:milestone
,
using:
Entities
::
Milestone
expose
:assignees
,
:author
,
using:
Entities
::
UserBasic
expose
:assignee
,
using:
::
API
::
Entities
::
UserBasic
do
|
issue
|
expose
:assignee
,
using:
::
API
::
Entities
::
UserBasic
do
|
issue
,
options
|
issue
.
assignees
.
first
end
expose
(
:user_notes_count
)
{
|
issue
,
options
|
issuable_metadata
(
issue
,
options
,
:user_notes_count
)
}
expose
(
:merge_requests_count
)
{
|
issue
,
options
|
issuable_metadata
(
issue
,
options
,
:merge_requests_count
)
}
expose
(
:upvotes
)
{
|
issue
,
options
|
issuable_metadata
(
issue
,
options
,
:upvotes
)
}
expose
(
:downvotes
)
{
|
issue
,
options
|
issuable_metadata
(
issue
,
options
,
:downvotes
)
}
expose
:user_notes_count
expose
:upvotes
do
|
issue
,
options
|
if
options
[
:issuable_metadata
]
# Avoids an N+1 query when metadata is included
options
[
:issuable_metadata
][
issue
.
id
].
upvotes
else
issue
.
upvotes
end
end
expose
:downvotes
do
|
issue
,
options
|
if
options
[
:issuable_metadata
]
# Avoids an N+1 query when metadata is included
options
[
:issuable_metadata
][
issue
.
id
].
downvotes
else
issue
.
downvotes
end
end
expose
:due_date
expose
:confidential
expose
:discussion_locked
expose
:web_url
do
|
issue
|
expose
:web_url
do
|
issue
,
options
|
Gitlab
::
UrlBuilder
.
build
(
issue
)
end
expose
:time_stats
,
using:
'API::Entities::IssuableTimeStats'
do
|
issue
|
issue
end
expose
:merge_requests_count
do
|
issue
,
options
|
if
options
[
:issuable_metadata
]
# Avoids an N+1 query when metadata is included
options
[
:issuable_metadata
][
issue
.
id
].
merge_requests_count
else
issue
.
merge_requests_closing_issues
.
count
end
end
end
class
Issue
<
IssueBasic
...
...
@@ -612,14 +628,14 @@ module API
end
end
class
MergeRequestSimple
<
Issueable
Entity
class
MergeRequestSimple
<
Project
Entity
expose
:title
expose
:web_url
do
|
merge_request
,
options
|
Gitlab
::
UrlBuilder
.
build
(
merge_request
)
end
end
class
MergeRequestBasic
<
Issueable
Entity
class
MergeRequestBasic
<
Project
Entity
expose
:merged_by
,
using:
Entities
::
UserBasic
do
|
merge_request
,
_options
|
merge_request
.
metrics
&
.
merged_by
end
...
...
@@ -643,12 +659,23 @@ module API
MarkupHelper
.
markdown_field
(
entity
,
:description
)
end
expose
:target_branch
,
:source_branch
expose
(
:user_notes_count
)
{
|
merge_request
,
options
|
issuable_metadata
(
merge_request
,
options
,
:user_notes_count
)
}
expose
(
:upvotes
)
{
|
merge_request
,
options
|
issuable_metadata
(
merge_request
,
options
,
:upvotes
)
}
expose
(
:downvotes
)
{
|
merge_request
,
options
|
issuable_metadata
(
merge_request
,
options
,
:downvotes
)
}
expose
:upvotes
do
|
merge_request
,
options
|
if
options
[
:issuable_metadata
]
options
[
:issuable_metadata
][
merge_request
.
id
].
upvotes
else
merge_request
.
upvotes
end
end
expose
:downvotes
do
|
merge_request
,
options
|
if
options
[
:issuable_metadata
]
options
[
:issuable_metadata
][
merge_request
.
id
].
downvotes
else
merge_request
.
downvotes
end
end
expose
:author
,
:assignee
,
using:
Entities
::
UserBasic
expose
:source_project_id
,
:target_project_id
expose
:labels
do
|
merge_request
|
expose
:labels
do
|
merge_request
,
options
|
# Avoids an N+1 query since labels are preloaded
merge_request
.
labels
.
map
(
&
:title
).
sort
end
...
...
@@ -666,6 +693,7 @@ module API
end
expose
:diff_head_sha
,
as: :sha
expose
:merge_commit_sha
expose
:user_notes_count
expose
:discussion_locked
expose
:should_remove_source_branch?
,
as: :should_remove_source_branch
expose
:force_remove_source_branch?
,
as: :force_remove_source_branch
...
...
@@ -673,7 +701,7 @@ module API
# Deprecated
expose
:allow_collaboration
,
as: :allow_maintainer_to_push
,
if:
->
(
merge_request
,
_
)
{
merge_request
.
for_fork?
}
expose
:web_url
do
|
merge_request
|
expose
:web_url
do
|
merge_request
,
options
|
Gitlab
::
UrlBuilder
.
build
(
merge_request
)
end
...
...
spec/lib/gitlab/issuable_metadata_spec.rb
View file @
f4be2d4e
...
...
@@ -28,12 +28,12 @@ describe Gitlab::IssuableMetadata do
expect
(
data
.
count
).
to
eq
(
2
)
expect
(
data
[
issue
.
id
].
upvotes
).
to
eq
(
1
)
expect
(
data
[
issue
.
id
].
downvotes
).
to
eq
(
0
)
expect
(
data
[
issue
.
id
].
user_
notes_count
).
to
eq
(
0
)
expect
(
data
[
issue
.
id
].
notes_count
).
to
eq
(
0
)
expect
(
data
[
issue
.
id
].
merge_requests_count
).
to
eq
(
1
)
expect
(
data
[
closed_issue
.
id
].
upvotes
).
to
eq
(
0
)
expect
(
data
[
closed_issue
.
id
].
downvotes
).
to
eq
(
1
)
expect
(
data
[
closed_issue
.
id
].
user_
notes_count
).
to
eq
(
0
)
expect
(
data
[
closed_issue
.
id
].
notes_count
).
to
eq
(
0
)
expect
(
data
[
closed_issue
.
id
].
merge_requests_count
).
to
eq
(
0
)
end
end
...
...
@@ -51,12 +51,12 @@ describe Gitlab::IssuableMetadata do
expect
(
data
.
count
).
to
eq
(
2
)
expect
(
data
[
merge_request
.
id
].
upvotes
).
to
eq
(
1
)
expect
(
data
[
merge_request
.
id
].
downvotes
).
to
eq
(
1
)
expect
(
data
[
merge_request
.
id
].
user_
notes_count
).
to
eq
(
1
)
expect
(
data
[
merge_request
.
id
].
notes_count
).
to
eq
(
1
)
expect
(
data
[
merge_request
.
id
].
merge_requests_count
).
to
eq
(
0
)
expect
(
data
[
merge_request_closed
.
id
].
upvotes
).
to
eq
(
0
)
expect
(
data
[
merge_request_closed
.
id
].
downvotes
).
to
eq
(
0
)
expect
(
data
[
merge_request_closed
.
id
].
user_
notes_count
).
to
eq
(
0
)
expect
(
data
[
merge_request_closed
.
id
].
notes_count
).
to
eq
(
0
)
expect
(
data
[
merge_request_closed
.
id
].
merge_requests_count
).
to
eq
(
0
)
end
end
...
...
spec/requests/api/merge_requests_spec.rb
View file @
f4be2d4e
...
...
@@ -320,18 +320,6 @@ describe API::MergeRequests do
expect
(
json_response
.
first
[
'title'
]).
to
eq
merge_request_closed
.
title
expect
(
json_response
.
first
[
'id'
]).
to
eq
merge_request_closed
.
id
end
it
'avoids N+1 queries'
do
control
=
ActiveRecord
::
QueryRecorder
.
new
do
get
api
(
"/projects/
#{
project
.
id
}
/merge_requests"
,
user
)
end
.
count
create
(
:merge_request
,
author:
user
,
assignee:
user
,
source_project:
project
,
target_project:
project
,
created_at:
base_time
)
expect
do
get
api
(
"/projects/
#{
project
.
id
}
/merge_requests"
,
user
)
end
.
not_to
exceed_query_limit
(
control
)
end
end
describe
"GET /groups/:id/merge_requests"
do
...
...
Write
Preview
Markdown
is supported
0%
Try again
or
attach a new file
Attach a file
Cancel
You are about to add
0
people
to the discussion. Proceed with caution.
Finish editing this message first!
Cancel
Please
register
or
sign in
to comment