Merge branch 'id-lsif-for-multiple-paths' into 'master'

Accept paths for LSIF info endpoint

See merge request gitlab-org/gitlab!25915

app/assets/javascripts/api.js

@@ -476,12 +476,12 @@ const Api = {
     return axios.get(url);
   },
 
-  lsifData(projectPath, commitId, path) {
+  lsifData(projectPath, commitId, paths) {
     const url = Api.buildUrl(this.lsifPath)
       .replace(':id', encodeURIComponent(projectPath))
       .replace(':commit_id', commitId);
 
-    return axios.get(url, { params: { path } });
+    return axios.get(url, { params: { paths } });
   },
 
   environments(id) {

app/assets/javascripts/code_navigation/store/actions.js

@@ -13,9 +13,10 @@ export default {
     commit(types.REQUEST_DATA);
 
     api
-      .lsifData(state.projectPath, state.commitId, state.blobPath)
+      .lsifData(state.projectPath, state.commitId, [state.blobPath])
       .then(({ data }) => {
-        const normalizedData = data.reduce((acc, d) => {
+        const dataForPath = data[state.blobPath];
+        const normalizedData = dataForPath.reduce((acc, d) => {
           if (d.hover) {
             acc[`${d.start_line}:${d.start_char}`] = d;
             addInteractionClass(d);

app/services/projects/lsif_data_service.rb

@@ -2,20 +2,22 @@
 
 module Projects
   class LsifDataService
-    attr_reader :file, :project, :path, :commit_id,
-      :docs, :doc_ranges, :ranges, :def_refs, :hover_refs
+    attr_reader :file, :project, :commit_id, :docs,
+      :doc_ranges, :ranges, :def_refs, :hover_refs
 
     CACHE_EXPIRE_IN = 1.hour
 
-    def initialize(file, project, params)
+    def initialize(file, project, commit_id)
       @file = file
       @project = project
-      @path = params[:path]
-      @commit_id = params[:commit_id]
-    end
+      @commit_id = commit_id
 
-    def execute
       fetch_data!
+    end
+
+    def execute(path)
+      doc_id = find_doc_id(docs, path)
+      dir_absolute_path = docs[doc_id]&.delete_suffix(path)
 
       doc_ranges[doc_id]&.map do |range_id|
         location, ref_id = ranges[range_id].values_at('loc', 'ref_id')
@@ -26,7 +28,7 @@ module Projects
           end_line: line_data.last,
           start_char: column_data.first,
           end_char: column_data.last,
-          definition_url: definition_url_for(def_refs[ref_id]),
+          definition_url: definition_url_for(def_refs[ref_id], dir_absolute_path),
           hover: highlighted_hover(hover_refs[ref_id])
         }
       end
@@ -58,8 +60,8 @@ module Projects
       @hover_refs = data['hover_refs']
     end
 
-    def doc_id
-      @doc_id ||= docs.reduce(nil) do |doc_id, (id, doc_path)|
+    def find_doc_id(docs, path)
+      docs.reduce(nil) do |doc_id, (id, doc_path)|
         next doc_id unless doc_path =~ /#{path}$/
 
         if doc_id.nil? || docs[doc_id].size > doc_path.size
@@ -70,11 +72,7 @@ module Projects
       end
     end
 
-    def dir_absolute_path
-      @dir_absolute_path ||= docs[doc_id]&.delete_suffix(path)
-    end
-
-    def definition_url_for(ref_id)
+    def definition_url_for(ref_id, dir_absolute_path)
       return unless range = ranges[ref_id]
 
       def_doc_id, location = range.values_at('doc_id', 'loc')

lib/api/lsif_data.rb

@@ -15,7 +15,7 @@ module API
     resource :projects, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do
       segment ':id/commits/:commit_id' do
         params do
-          requires :path, type: String, desc: 'The path of a file'
+          requires :paths, type: Array, desc: 'The paths of the files'
         end
         get 'lsif/info' do
           authorize! :download_code, user_project
@@ -30,7 +30,9 @@ module API
           authorize! :read_pipeline, artifact.job.pipeline
           file_too_large! if artifact.file.cached_size > MAX_FILE_SIZE
 
-          ::Projects::LsifDataService.new(artifact.file, @project, params).execute
+          service = ::Projects::LsifDataService.new(artifact.file, @project, params[:commit_id])
+
+          params[:paths].to_h { |path| [path, service.execute(path)] }
         end
       end
     end

spec/frontend/code_navigation/store/actions_spec.js

@@ -45,18 +45,20 @@ describe('Code navigation actions', () => {
 
     describe('success', () => {
       beforeEach(() => {
-        mock.onGet(apiUrl).replyOnce(200, [
-          {
-            start_line: 0,
-            start_char: 0,
-            hover: { value: '123' },
-          },
-          {
-            start_line: 1,
-            start_char: 0,
-            hover: null,
-          },
-        ]);
+        mock.onGet(apiUrl).replyOnce(200, {
+          index: [
+            {
+              start_line: 0,
+              start_char: 0,
+              hover: { value: '123' },
+            },
+            {
+              start_line: 1,
+              start_char: 0,
+              hover: null,
+            },
+          ],
+        });
       });
 
       it('commits REQUEST_DATA_SUCCESS with normalized data', done => {

spec/requests/api/lsif_data_spec.rb

@@ -9,18 +9,20 @@ describe API::LsifData do
   let(:commit) { project.commit }
 
   describe 'GET lsif/info' do
-    let(:endpoint_path) { "/projects/#{project.id}/commits/#{commit.id}/lsif/info" }
+    subject do
+      endpoint_path = "/projects/#{project.id}/commits/#{commit.id}/lsif/info"
+
+      get api(endpoint_path, user), params: { paths: ['main.go', 'morestrings/reverse.go'] }
+
+      response
+    end
 
     context 'user does not have access to the project' do
       before do
         project.add_guest(user)
       end
 
-      it 'returns 403' do
-        get api(endpoint_path, user), params: { path: 'main.go' }
-
-        expect(response).to have_gitlab_http_status(:forbidden)
-      end
+      it { is_expected.to have_gitlab_http_status(:forbidden) }
     end
 
     context 'user has access to the project' do
@@ -28,35 +30,27 @@ describe API::LsifData do
         project.add_reporter(user)
       end
 
-      context 'code_navigation feature is disabled' do
-        before do
-          stub_feature_flags(code_navigation: false)
-        end
-
-        it 'returns 404' do
-          get api(endpoint_path, user)
-
-          expect(response).to have_gitlab_http_status(:not_found)
-        end
-      end
-
       context 'there is no job artifact for the passed commit' do
-        it 'returns 404' do
-          get api(endpoint_path, user), params: { path: 'main.go' }
-
-          expect(response).to have_gitlab_http_status(:not_found)
-        end
+        it { is_expected.to have_gitlab_http_status(:not_found) }
       end
 
       context 'lsif data is stored as a job artifact' do
         let!(:pipeline) { create(:ci_pipeline, project: project, sha: commit.id) }
         let!(:artifact) { create(:ci_job_artifact, :lsif, job: create(:ci_build, pipeline: pipeline)) }
 
-        it 'returns code navigation info for a given path' do
-          get api(endpoint_path, user), params: { path: 'main.go' }
+        context 'code_navigation feature is disabled' do
+          before do
+            stub_feature_flags(code_navigation: false)
+          end
 
-          expect(response).to have_gitlab_http_status(:ok)
-          expect(response.parsed_body.last).to eq({
+          it { is_expected.to have_gitlab_http_status(:not_found) }
+        end
+
+        it 'returns code navigation info for a given path', :aggregate_failures do
+          expect(subject).to have_gitlab_http_status(:ok)
+
+          data_for_main = response.parsed_body['main.go']
+          expect(data_for_main.last).to eq({
             'end_char' => 18,
             'end_line' => 8,
             'start_char' => 13,
@@ -67,26 +61,33 @@ describe API::LsifData do
               'value' => Gitlab::Highlight.highlight(nil, 'func Func2(i int) string', language: 'go')
             }]
           })
+
+          data_for_reverse = response.parsed_body['morestrings/reverse.go']
+          expect(data_for_reverse.last).to eq({
+            'end_char' => 9,
+            'end_line' => 7,
+            'start_char' => 8,
+            'start_line' => 7,
+            'definition_url' => project_blob_path(project, "#{commit.id}/morestrings/reverse.go", anchor: 'L6'),
+            'hover' => [{
+              'language' => 'go',
+              'value' => Gitlab::Highlight.highlight(nil, 'var b string', language: 'go')
+            }]
+          })
         end
 
         context 'the stored file is too large' do
-          it 'returns 413' do
+          before do
             allow_any_instance_of(JobArtifactUploader).to receive(:cached_size).and_return(20.megabytes)
-
-            get api(endpoint_path, user), params: { path: 'main.go' }
-
-            expect(response).to have_gitlab_http_status(:payload_too_large)
           end
+
+          it { is_expected.to have_gitlab_http_status(:payload_too_large) }
         end
 
         context 'the user does not have access to the pipeline' do
           let(:project) { create(:project, :repository, builds_access_level: ProjectFeature::DISABLED) }
 
-          it 'returns 403' do
-            get api(endpoint_path, user), params: { path: 'main.go' }
-
-            expect(response).to have_gitlab_http_status(:forbidden)
-          end
+          it { is_expected.to have_gitlab_http_status(:forbidden) }
         end
       end
     end

spec/services/projects/lsif_data_service_spec.rb

@@ -7,9 +7,8 @@ describe Projects::LsifDataService do
   let(:project) { build_stubbed(:project) }
   let(:path) { 'main.go' }
   let(:commit_id) { Digest::SHA1.hexdigest(SecureRandom.hex) }
-  let(:params) { { path: path, commit_id: commit_id } }
 
-  let(:service) { described_class.new(artifact.file, project, params) }
+  let(:service) { described_class.new(artifact.file, project, commit_id) }
 
   describe '#execute' do
     def highlighted_value(value)
@@ -18,7 +17,7 @@ describe Projects::LsifDataService do
 
     context 'fetched lsif file', :use_clean_rails_memory_store_caching do
       it 'is cached' do
-        service.execute
+        service.execute(path)
 
         cached_data = Rails.cache.fetch("project:#{project.id}:lsif:#{commit_id}")
 
@@ -30,7 +29,7 @@ describe Projects::LsifDataService do
       let(:path_prefix) { "/#{project.full_path}/-/blob/#{commit_id}" }
 
       it 'returns lsif ranges for the file' do
-        expect(service.execute).to eq([
+        expect(service.execute(path)).to eq([
           {
             end_char: 9,
             end_line: 6,
@@ -87,7 +86,7 @@ describe Projects::LsifDataService do
       let(:path) { 'morestrings/reverse.go' }
 
       it 'returns lsif ranges for the file' do
-        expect(service.execute.first).to eq({
+        expect(service.execute(path).first).to eq({
           end_char: 2,
           end_line: 11,
           start_char: 1,
@@ -102,7 +101,7 @@ describe Projects::LsifDataService do
       let(:path) { 'unknown.go' }
 
       it 'returns nil' do
-        expect(service.execute).to eq(nil)
+        expect(service.execute(path)).to eq(nil)
       end
     end
   end
@@ -120,9 +119,7 @@ describe Projects::LsifDataService do
       end
 
       it 'fetches the document with the shortest absolute path' do
-        service.instance_variable_set(:@docs, docs)
-
-        expect(service.__send__(:doc_id)).to eq(3)
+        expect(service.__send__(:find_doc_id, docs, path)).to eq(3)
       end
     end
   end