summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorStan Hu <stanhu@gmail.com>2019-05-09 07:53:25 -0500
committerStan Hu <stanhu@gmail.com>2019-05-09 18:04:40 -0500
commite67481e079023e1319e91fabfb90b88c9a2b2655 (patch)
tree1b8536d97905bec416e18336e5dc439bc589919d
parent712dccd4c0f37b9fce12f5a945a551f8dca07cc4 (diff)
downloadgitlab-ce-e67481e079023e1319e91fabfb90b88c9a2b2655.tar.gz
Properly handle LFS Batch API response in project importsh-fix-lfs-download-errors
Project imports were failing with `undefined method each_with_object for String` because the import was attempting to parse the LFS Batch API and failing due to the fact that the Content-Type wasn't a supported format (e.g. application/vnd.git-lfs+json instead of application/json). We now parse the body as JSON. Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/61624
-rw-r--r--app/services/projects/lfs_pointers/lfs_download_link_list_service.rb12
-rw-r--r--changelogs/unreleased/sh-fix-lfs-download-errors.yml5
-rw-r--r--spec/services/projects/lfs_pointers/lfs_download_link_list_service_spec.rb20
3 files changed, 35 insertions, 2 deletions
diff --git a/app/services/projects/lfs_pointers/lfs_download_link_list_service.rb b/app/services/projects/lfs_pointers/lfs_download_link_list_service.rb
index 05974948505..9b72480d18b 100644
--- a/app/services/projects/lfs_pointers/lfs_download_link_list_service.rb
+++ b/app/services/projects/lfs_pointers/lfs_download_link_list_service.rb
@@ -37,7 +37,17 @@ module Projects
raise DownloadLinksError, response.message unless response.success?
- parse_response_links(response['objects'])
+ # Since the LFS Batch API may return a Content-Ttpe of
+ # application/vnd.git-lfs+json
+ # (https://github.com/git-lfs/git-lfs/blob/master/docs/api/batch.md#requests),
+ # HTTParty does not know this is actually JSON.
+ data = JSON.parse(response.body)
+
+ raise DownloadLinksError, "LFS Batch API did return any objects" unless data.is_a?(Hash) && data.key?('objects')
+
+ parse_response_links(data['objects'])
+ rescue JSON::ParserError
+ raise DownloadLinksError, "LFS Batch API response is not JSON"
end
def parse_response_links(objects_response)
diff --git a/changelogs/unreleased/sh-fix-lfs-download-errors.yml b/changelogs/unreleased/sh-fix-lfs-download-errors.yml
new file mode 100644
index 00000000000..ad67df6bb06
--- /dev/null
+++ b/changelogs/unreleased/sh-fix-lfs-download-errors.yml
@@ -0,0 +1,5 @@
+---
+title: Properly handle LFS Batch API response in project import
+merge_request: 28223
+author:
+type: fixed
diff --git a/spec/services/projects/lfs_pointers/lfs_download_link_list_service_spec.rb b/spec/services/projects/lfs_pointers/lfs_download_link_list_service_spec.rb
index d8427d0bf78..80debcd3a7a 100644
--- a/spec/services/projects/lfs_pointers/lfs_download_link_list_service_spec.rb
+++ b/spec/services/projects/lfs_pointers/lfs_download_link_list_service_spec.rb
@@ -33,7 +33,10 @@ describe Projects::LfsPointers::LfsDownloadLinkListService do
before do
allow(project).to receive(:lfs_enabled?).and_return(true)
- allow(Gitlab::HTTP).to receive(:post).and_return(objects_response)
+ response = instance_double(HTTParty::Response)
+ allow(response).to receive(:body).and_return(objects_response.to_json)
+ allow(response).to receive(:success?).and_return(true)
+ allow(Gitlab::HTTP).to receive(:post).and_return(response)
end
describe '#execute' do
@@ -89,6 +92,21 @@ describe Projects::LfsPointers::LfsDownloadLinkListService do
expect { subject.send(:get_download_links, new_oids) }.to raise_error(described_class::DownloadLinksError)
end
+
+ shared_examples 'JSON parse errors' do |body|
+ it 'raises error' do
+ response = instance_double(HTTParty::Response)
+ allow(response).to receive(:body).and_return(body)
+ allow(response).to receive(:success?).and_return(true)
+ allow(Gitlab::HTTP).to receive(:post).and_return(response)
+
+ expect { subject.send(:get_download_links, new_oids) }.to raise_error(described_class::DownloadLinksError)
+ end
+ end
+
+ it_behaves_like 'JSON parse errors', '{'
+ it_behaves_like 'JSON parse errors', '{}'
+ it_behaves_like 'JSON parse errors', '{ foo: 123 }'
end
describe '#parse_response_links' do