Commit 747a1396 authored by Sean McGivern's avatar Sean McGivern

Merge branch '321862_fix_long_commit_messages_error' into 'master'

Fix 500 error for long commit messages

See merge request gitlab-org/gitlab!55320
parents 52fdd36a a7ef6385
---
title: Fix 500 error for long commit messages
merge_request: 55320
author:
type: fixed
...@@ -104,12 +104,12 @@ module Gitlab ...@@ -104,12 +104,12 @@ module Gitlab
end end
def fetch_last_cached_commits_list def fetch_last_cached_commits_list
cache_key = ['projects', project.id, 'last_commits_list', commit.id, ensured_path, offset, limit] cache_key = ['projects', project.id, 'last_commits', commit.id, ensured_path, offset, limit]
commits = Rails.cache.fetch(cache_key, expires_in: CACHE_EXPIRE_IN) do commits = Rails.cache.fetch(cache_key, expires_in: CACHE_EXPIRE_IN) do
repository repository
.list_last_commits_for_tree(commit.id, ensured_path, offset: offset, limit: limit, literal_pathspec: true) .list_last_commits_for_tree(commit.id, ensured_path, offset: offset, limit: limit, literal_pathspec: true)
.transform_values!(&:to_hash) .transform_values! { |commit| commit_to_hash(commit) }
end end
commits.transform_values! { |value| Commit.from_hash(value, project) } commits.transform_values! { |value| Commit.from_hash(value, project) }
...@@ -121,6 +121,12 @@ module Gitlab ...@@ -121,6 +121,12 @@ module Gitlab
resolved_commits[commit.id] ||= commit resolved_commits[commit.id] ||= commit
end end
def commit_to_hash(commit)
commit.to_hash.tap do |hash|
hash[:message] = hash[:message].to_s.truncate_bytes(1.kilobyte, omission: '...')
end
end
def commit_path(commit) def commit_path(commit)
Gitlab::Routing.url_helpers.project_commit_path(project, commit) Gitlab::Routing.url_helpers.project_commit_path(project, commit)
end end
......
...@@ -57,14 +57,12 @@ RSpec.describe Gitlab::TreeSummary do ...@@ -57,14 +57,12 @@ RSpec.describe Gitlab::TreeSummary do
context 'with caching', :use_clean_rails_memory_store_caching do context 'with caching', :use_clean_rails_memory_store_caching do
subject { Rails.cache.fetch(key) } subject { Rails.cache.fetch(key) }
before do
summarized
end
context 'Repository tree cache' do context 'Repository tree cache' do
let(:key) { ['projects', project.id, 'content', commit.id, path] } let(:key) { ['projects', project.id, 'content', commit.id, path] }
it 'creates a cache for repository content' do it 'creates a cache for repository content' do
summarized
is_expected.to eq([{ file_name: 'a.txt', type: :blob }]) is_expected.to eq([{ file_name: 'a.txt', type: :blob }])
end end
end end
...@@ -72,11 +70,34 @@ RSpec.describe Gitlab::TreeSummary do ...@@ -72,11 +70,34 @@ RSpec.describe Gitlab::TreeSummary do
context 'Commits list cache' do context 'Commits list cache' do
let(:offset) { 0 } let(:offset) { 0 }
let(:limit) { 25 } let(:limit) { 25 }
let(:key) { ['projects', project.id, 'last_commits_list', commit.id, path, offset, limit] } let(:key) { ['projects', project.id, 'last_commits', commit.id, path, offset, limit] }
it 'creates a cache for commits list' do it 'creates a cache for commits list' do
summarized
is_expected.to eq('a.txt' => commit.to_hash) is_expected.to eq('a.txt' => commit.to_hash)
end end
context 'when commit has a very long message' do
before do
repo.create_file(
project.creator,
'long.txt',
'',
message: message,
branch_name: project.default_branch_or_master
)
end
let(:message) { 'a' * 1025 }
let(:expected_message) { message[0...1021] + '...' }
it 'truncates commit message to 1 kilobyte' do
summarized
is_expected.to include('long.txt' => a_hash_including(message: expected_message))
end
end
end end
end end
end end
......
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