diff options
author | Sean McGivern <sean@gitlab.com> | 2017-11-07 17:00:21 +0300 |
---|---|---|
committer | Sean McGivern <sean@gitlab.com> | 2017-11-07 19:26:55 +0300 |
commit | 4d4ddb6004e6f7f56b337a49c6eedaad70d70862 (patch) | |
tree | c211bad025f8250f224f395a2f5687d71791cb13 /spec/lib/gitlab/issuable_metadata_spec.rb | |
parent | dc1e6b436268c00bd1fdf3d15597a4656e029b95 (diff) |
Fail when issuable_meta_data is called on an unlimited collection
This method can be called with an array, or a relation:
1. Arrays always have a limited amount of values, so that's fine.
2. If the relation does not have a limit value applied, then we will load every
single object in that collection, and prevent N+1 queries for the metadata
for that. But that's wrong, because we should never call this without an
explicit limit set. So we raise in that case, and this commit will see which
specs fail.
The only failing specs here were the issues API specs, and the specs for
IssuableMetadata itself, and both have been addressed.
Diffstat (limited to 'spec/lib/gitlab/issuable_metadata_spec.rb')
-rw-r--r-- | spec/lib/gitlab/issuable_metadata_spec.rb | 12 |
1 files changed, 8 insertions, 4 deletions
diff --git a/spec/lib/gitlab/issuable_metadata_spec.rb b/spec/lib/gitlab/issuable_metadata_spec.rb index 2455969a183..42635a68ee1 100644 --- a/spec/lib/gitlab/issuable_metadata_spec.rb +++ b/spec/lib/gitlab/issuable_metadata_spec.rb @@ -1,8 +1,8 @@ require 'spec_helper' describe Gitlab::IssuableMetadata do - let(:user) { create(:user) } - let!(:project) { create(:project, :public, :repository, creator: user, namespace: user.namespace) } + let(:user) { create(:user) } + let!(:project) { create(:project, :public, :repository, creator: user, namespace: user.namespace) } subject { Class.new { include Gitlab::IssuableMetadata }.new } @@ -10,6 +10,10 @@ describe Gitlab::IssuableMetadata do expect(subject.issuable_meta_data(Issue.none, 'Issue')).to eq({}) end + it 'raises an error when given a collection with no limit' do + expect { subject.issuable_meta_data(Issue.all, 'Issue') }.to raise_error(/must have a limit/) + end + context 'issues' do let!(:issue) { create(:issue, author: user, project: project) } let!(:closed_issue) { create(:issue, state: :closed, author: user, project: project) } @@ -19,7 +23,7 @@ describe Gitlab::IssuableMetadata do let!(:closing_issues) { create(:merge_requests_closing_issues, issue: issue, merge_request: merge_request) } it 'aggregates stats on issues' do - data = subject.issuable_meta_data(Issue.all, 'Issue') + data = subject.issuable_meta_data(Issue.all.limit(10), 'Issue') expect(data.count).to eq(2) expect(data[issue.id].upvotes).to eq(1) @@ -42,7 +46,7 @@ describe Gitlab::IssuableMetadata do let!(:note) { create(:note_on_merge_request, author: user, project: project, noteable: merge_request, note: "a comment on a MR") } it 'aggregates stats on merge requests' do - data = subject.issuable_meta_data(MergeRequest.all, 'MergeRequest') + data = subject.issuable_meta_data(MergeRequest.all.limit(10), 'MergeRequest') expect(data.count).to eq(2) expect(data[merge_request.id].upvotes).to eq(1) |