Commit e407c2d1 authored by Jacob Vosmaer (GitLab)'s avatar Jacob Vosmaer (GitLab) Committed by Robert Speicher

Don't allow Repository#log with limit zero

parent 501d81c5
...@@ -468,9 +468,13 @@ module Gitlab ...@@ -468,9 +468,13 @@ module Gitlab
} }
options = default_options.merge(options) options = default_options.merge(options)
options[:limit] ||= 0
options[:offset] ||= 0 options[:offset] ||= 0
limit = options[:limit]
if limit == 0 || !limit.is_a?(Integer)
raise ArgumentError.new("invalid Repository#log limit: #{limit.inspect}")
end
gitaly_migrate(:find_commits) do |is_enabled| gitaly_migrate(:find_commits) do |is_enabled|
if is_enabled if is_enabled
gitaly_commit_client.find_commits(options) gitaly_commit_client.find_commits(options)
......
...@@ -194,7 +194,7 @@ describe 'Commits' do ...@@ -194,7 +194,7 @@ describe 'Commits' do
end end
it 'includes the committed_date for each commit' do it 'includes the committed_date for each commit' do
commits = project.repository.commits(branch_name) commits = project.repository.commits(branch_name, limit: 40)
commits.each do |commit| commits.each do |commit|
expect(page).to have_content("authored #{commit.authored_date.strftime("%b %d, %Y")}") expect(page).to have_content("authored #{commit.authored_date.strftime("%b %d, %Y")}")
......
...@@ -981,6 +981,16 @@ describe Gitlab::Git::Repository, seed_helper: true do ...@@ -981,6 +981,16 @@ describe Gitlab::Git::Repository, seed_helper: true do
end end
end end
end end
context 'limit validation' do
where(:limit) do
[0, nil, '', 'foo']
end
with_them do
it { expect { repository.log(limit: limit) }.to raise_error(ArgumentError) }
end
end
end end
describe "#rugged_commits_between" do describe "#rugged_commits_between" do
......
...@@ -222,20 +222,20 @@ describe Repository do ...@@ -222,20 +222,20 @@ describe Repository do
it 'sets follow when path is a single path' do it 'sets follow when path is a single path' do
expect(Gitlab::Git::Commit).to receive(:where).with(a_hash_including(follow: true)).and_call_original.twice expect(Gitlab::Git::Commit).to receive(:where).with(a_hash_including(follow: true)).and_call_original.twice
repository.commits('master', path: 'README.md') repository.commits('master', limit: 1, path: 'README.md')
repository.commits('master', path: ['README.md']) repository.commits('master', limit: 1, path: ['README.md'])
end end
it 'does not set follow when path is multiple paths' do it 'does not set follow when path is multiple paths' do
expect(Gitlab::Git::Commit).to receive(:where).with(a_hash_including(follow: false)).and_call_original expect(Gitlab::Git::Commit).to receive(:where).with(a_hash_including(follow: false)).and_call_original
repository.commits('master', path: ['README.md', 'CHANGELOG']) repository.commits('master', limit: 1, path: ['README.md', 'CHANGELOG'])
end end
it 'does not set follow when there are no paths' do it 'does not set follow when there are no paths' do
expect(Gitlab::Git::Commit).to receive(:where).with(a_hash_including(follow: false)).and_call_original expect(Gitlab::Git::Commit).to receive(:where).with(a_hash_including(follow: false)).and_call_original
repository.commits('master') repository.commits('master', limit: 1)
end end
end end
...@@ -455,7 +455,7 @@ describe Repository do ...@@ -455,7 +455,7 @@ describe Repository do
expect do expect do
repository.create_dir(user, 'newdir', repository.create_dir(user, 'newdir',
message: 'Create newdir', branch_name: 'master') message: 'Create newdir', branch_name: 'master')
end.to change { repository.commits('master').count }.by(1) end.to change { repository.count_commits(ref: 'master') }.by(1)
newdir = repository.tree('master', 'newdir') newdir = repository.tree('master', 'newdir')
expect(newdir.path).to eq('newdir') expect(newdir.path).to eq('newdir')
...@@ -469,7 +469,7 @@ describe Repository do ...@@ -469,7 +469,7 @@ describe Repository do
repository.create_dir(user, 'newdir', repository.create_dir(user, 'newdir',
message: 'Create newdir', branch_name: 'patch', message: 'Create newdir', branch_name: 'patch',
start_branch_name: 'master', start_project: forked_project) start_branch_name: 'master', start_project: forked_project)
end.to change { repository.commits('master').count }.by(0) end.to change { repository.count_commits(ref: 'master') }.by(0)
expect(repository.branch_exists?('patch')).to be_truthy expect(repository.branch_exists?('patch')).to be_truthy
expect(forked_project.repository.branch_exists?('patch')).to be_falsy expect(forked_project.repository.branch_exists?('patch')).to be_falsy
...@@ -486,7 +486,7 @@ describe Repository do ...@@ -486,7 +486,7 @@ describe Repository do
message: 'Add newdir', message: 'Add newdir',
branch_name: 'master', branch_name: 'master',
author_email: author_email, author_name: author_name) author_email: author_email, author_name: author_name)
end.to change { repository.commits('master').count }.by(1) end.to change { repository.count_commits(ref: 'master') }.by(1)
last_commit = repository.commit last_commit = repository.commit
...@@ -502,7 +502,7 @@ describe Repository do ...@@ -502,7 +502,7 @@ describe Repository do
repository.create_file(user, 'NEWCHANGELOG', 'Changelog!', repository.create_file(user, 'NEWCHANGELOG', 'Changelog!',
message: 'Create changelog', message: 'Create changelog',
branch_name: 'master') branch_name: 'master')
end.to change { repository.commits('master').count }.by(1) end.to change { repository.count_commits(ref: 'master') }.by(1)
blob = repository.blob_at('master', 'NEWCHANGELOG') blob = repository.blob_at('master', 'NEWCHANGELOG')
...@@ -514,7 +514,7 @@ describe Repository do ...@@ -514,7 +514,7 @@ describe Repository do
repository.create_file(user, 'new_dir/new_file.txt', 'File!', repository.create_file(user, 'new_dir/new_file.txt', 'File!',
message: 'Create new_file with new_dir', message: 'Create new_file with new_dir',
branch_name: 'master') branch_name: 'master')
end.to change { repository.commits('master').count }.by(1) end.to change { repository.count_commits(ref: 'master') }.by(1)
expect(repository.tree('master', 'new_dir').path).to eq('new_dir') expect(repository.tree('master', 'new_dir').path).to eq('new_dir')
expect(repository.blob_at('master', 'new_dir/new_file.txt').data).to eq('File!') expect(repository.blob_at('master', 'new_dir/new_file.txt').data).to eq('File!')
...@@ -538,7 +538,7 @@ describe Repository do ...@@ -538,7 +538,7 @@ describe Repository do
branch_name: 'master', branch_name: 'master',
author_email: author_email, author_email: author_email,
author_name: author_name) author_name: author_name)
end.to change { repository.commits('master').count }.by(1) end.to change { repository.count_commits(ref: 'master') }.by(1)
last_commit = repository.commit last_commit = repository.commit
...@@ -554,7 +554,7 @@ describe Repository do ...@@ -554,7 +554,7 @@ describe Repository do
repository.update_file(user, 'CHANGELOG', 'Changelog!', repository.update_file(user, 'CHANGELOG', 'Changelog!',
message: 'Update changelog', message: 'Update changelog',
branch_name: 'master') branch_name: 'master')
end.to change { repository.commits('master').count }.by(1) end.to change { repository.count_commits(ref: 'master') }.by(1)
blob = repository.blob_at('master', 'CHANGELOG') blob = repository.blob_at('master', 'CHANGELOG')
...@@ -567,7 +567,7 @@ describe Repository do ...@@ -567,7 +567,7 @@ describe Repository do
branch_name: 'master', branch_name: 'master',
previous_path: 'LICENSE', previous_path: 'LICENSE',
message: 'Changes filename') message: 'Changes filename')
end.to change { repository.commits('master').count }.by(1) end.to change { repository.count_commits(ref: 'master') }.by(1)
files = repository.ls_files('master') files = repository.ls_files('master')
...@@ -584,7 +584,7 @@ describe Repository do ...@@ -584,7 +584,7 @@ describe Repository do
message: 'Update README', message: 'Update README',
author_email: author_email, author_email: author_email,
author_name: author_name) author_name: author_name)
end.to change { repository.commits('master').count }.by(1) end.to change { repository.count_commits(ref: 'master') }.by(1)
last_commit = repository.commit last_commit = repository.commit
...@@ -599,7 +599,7 @@ describe Repository do ...@@ -599,7 +599,7 @@ describe Repository do
expect do expect do
repository.delete_file(user, 'README', repository.delete_file(user, 'README',
message: 'Remove README', branch_name: 'master') message: 'Remove README', branch_name: 'master')
end.to change { repository.commits('master').count }.by(1) end.to change { repository.count_commits(ref: 'master') }.by(1)
expect(repository.blob_at('master', 'README')).to be_nil expect(repository.blob_at('master', 'README')).to be_nil
end end
...@@ -610,7 +610,7 @@ describe Repository do ...@@ -610,7 +610,7 @@ describe Repository do
repository.delete_file(user, 'README', repository.delete_file(user, 'README',
message: 'Remove README', branch_name: 'master', message: 'Remove README', branch_name: 'master',
author_email: author_email, author_name: author_name) author_email: author_email, author_name: author_name)
end.to change { repository.commits('master').count }.by(1) end.to change { repository.count_commits(ref: 'master') }.by(1)
last_commit = repository.commit last_commit = repository.commit
......
...@@ -62,7 +62,7 @@ describe API::Commits do ...@@ -62,7 +62,7 @@ describe API::Commits do
context "since optional parameter" do context "since optional parameter" do
it "returns project commits since provided parameter" do it "returns project commits since provided parameter" do
commits = project.repository.commits("master") commits = project.repository.commits("master", limit: 2)
after = commits.second.created_at after = commits.second.created_at
get api("/projects/#{project_id}/repository/commits?since=#{after.utc.iso8601}", user) get api("/projects/#{project_id}/repository/commits?since=#{after.utc.iso8601}", user)
...@@ -73,7 +73,7 @@ describe API::Commits do ...@@ -73,7 +73,7 @@ describe API::Commits do
end end
it 'include correct pagination headers' do it 'include correct pagination headers' do
commits = project.repository.commits("master") commits = project.repository.commits("master", limit: 2)
after = commits.second.created_at after = commits.second.created_at
commit_count = project.repository.count_commits(ref: 'master', after: after).to_s commit_count = project.repository.count_commits(ref: 'master', after: after).to_s
...@@ -87,12 +87,12 @@ describe API::Commits do ...@@ -87,12 +87,12 @@ describe API::Commits do
context "until optional parameter" do context "until optional parameter" do
it "returns project commits until provided parameter" do it "returns project commits until provided parameter" do
commits = project.repository.commits("master") commits = project.repository.commits("master", limit: 20)
before = commits.second.created_at before = commits.second.created_at
get api("/projects/#{project_id}/repository/commits?until=#{before.utc.iso8601}", user) get api("/projects/#{project_id}/repository/commits?until=#{before.utc.iso8601}", user)
if commits.size >= 20 if commits.size == 20
expect(json_response.size).to eq(20) expect(json_response.size).to eq(20)
else else
expect(json_response.size).to eq(commits.size - 1) expect(json_response.size).to eq(commits.size - 1)
...@@ -103,7 +103,7 @@ describe API::Commits do ...@@ -103,7 +103,7 @@ describe API::Commits do
end end
it 'include correct pagination headers' do it 'include correct pagination headers' do
commits = project.repository.commits("master") commits = project.repository.commits("master", limit: 2)
before = commits.second.created_at before = commits.second.created_at
commit_count = project.repository.count_commits(ref: 'master', before: before).to_s commit_count = project.repository.count_commits(ref: 'master', before: before).to_s
...@@ -181,7 +181,7 @@ describe API::Commits do ...@@ -181,7 +181,7 @@ describe API::Commits do
let(:page) { 3 } let(:page) { 3 }
it 'returns the third 5 commits' do it 'returns the third 5 commits' do
commit = project.repository.commits('HEAD', offset: (page - 1) * per_page).first commit = project.repository.commits('HEAD', limit: per_page, offset: (page - 1) * per_page).first
expect(json_response.size).to eq(per_page) expect(json_response.size).to eq(per_page)
expect(json_response.first['id']).to eq(commit.id) expect(json_response.first['id']).to eq(commit.id)
......
...@@ -36,7 +36,7 @@ describe API::V3::Commits do ...@@ -36,7 +36,7 @@ describe API::V3::Commits do
context "since optional parameter" do context "since optional parameter" do
it "returns project commits since provided parameter" do it "returns project commits since provided parameter" do
commits = project.repository.commits("master") commits = project.repository.commits("master", limit: 2)
since = commits.second.created_at since = commits.second.created_at
get v3_api("/projects/#{project.id}/repository/commits?since=#{since.utc.iso8601}", user) get v3_api("/projects/#{project.id}/repository/commits?since=#{since.utc.iso8601}", user)
...@@ -49,12 +49,12 @@ describe API::V3::Commits do ...@@ -49,12 +49,12 @@ describe API::V3::Commits do
context "until optional parameter" do context "until optional parameter" do
it "returns project commits until provided parameter" do it "returns project commits until provided parameter" do
commits = project.repository.commits("master") commits = project.repository.commits("master", limit: 20)
before = commits.second.created_at before = commits.second.created_at
get v3_api("/projects/#{project.id}/repository/commits?until=#{before.utc.iso8601}", user) get v3_api("/projects/#{project.id}/repository/commits?until=#{before.utc.iso8601}", user)
if commits.size >= 20 if commits.size == 20
expect(json_response.size).to eq(20) expect(json_response.size).to eq(20)
else else
expect(json_response.size).to eq(commits.size - 1) expect(json_response.size).to eq(commits.size - 1)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment