Merge branch 'id-lsif-multiple-files-in-zip' into 'master'

Load preprocessed code-nav-data directly from object storage

See merge request gitlab-org/gitlab!27688

app/assets/javascripts/api.js

@@ -44,7 +44,6 @@ const Api = {
   mergeRequestsPipeline: '/api/:version/projects/:id/merge_requests/:merge_request_iid/pipelines',
   adminStatisticsPath: '/api/:version/application/statistics',
   pipelineSinglePath: '/api/:version/projects/:id/pipelines/:pipeline_id',
-  lsifPath: '/api/:version/projects/:id/commits/:commit_id/lsif/info',
   environmentsPath: '/api/:version/projects/:id/environments',
 
   group(groupId, callback) {
@@ -474,14 +473,6 @@ const Api = {
     return axios.get(url);
   },
 
-  lsifData(projectPath, commitId, paths) {
-    const url = Api.buildUrl(this.lsifPath)
-      .replace(':id', encodeURIComponent(projectPath))
-      .replace(':commit_id', commitId);
-
-    return axios.get(url, { params: { paths } });
-  },
-
   environments(id) {
     const url = Api.buildUrl(this.environmentsPath).replace(':id', encodeURIComponent(id));
     return axios.get(url);

app/assets/javascripts/code_navigation/components/app.vue

@@ -7,7 +7,7 @@ export default {
     Popover,
   },
   computed: {
-    ...mapState(['currentDefinition', 'currentDefinitionPosition']),
+    ...mapState(['currentDefinition', 'currentDefinitionPosition', 'definitionPathPrefix']),
   },
   mounted() {
     this.blobViewer = document.querySelector('.blob-viewer');
@@ -39,5 +39,6 @@ export default {
     v-if="currentDefinition"
     :position="currentDefinitionPosition"
     :data="currentDefinition"
+    :definition-path-prefix="definitionPathPrefix"
   />
 </template>

app/assets/javascripts/code_navigation/components/popover.vue

@@ -14,6 +14,10 @@ export default {
       type: Object,
       required: true,
     },
+    definitionPathPrefix: {
+      type: String,
+      required: true,
+    },
   },
   data() {
     return {
@@ -27,6 +31,11 @@ export default {
         top: `${this.position.y + this.position.height}px`,
       };
     },
+    definitionPath() {
+      return (
+        this.data.definition_path && `${this.definitionPathPrefix}/${this.data.definition_path}`
+      );
+    },
   },
   watch: {
     position: {
@@ -67,8 +76,8 @@ export default {
         {{ hover.value }}
       </p>
     </div>
-    <div v-if="data.definition_url" class="popover-body">
-      <gl-button :href="data.definition_url" target="_blank" class="w-100" variant="default">
+    <div v-if="definitionPath" class="popover-body">
+      <gl-button :href="definitionPath" target="_blank" class="w-100" variant="default">
         {{ __('Go to definition') }}
       </gl-button>
     </div>

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

-import api from '~/api';
+import axios from '~/lib/utils/axios_utils';
 import * as types from './mutation_types';
 import { getCurrentHoverElement, setCurrentHoverElement, addInteractionClass } from '../utils';
 
@@ -12,11 +12,10 @@ export default {
   fetchData({ commit, dispatch, state }) {
     commit(types.REQUEST_DATA);
 
-    api
-      .lsifData(state.projectPath, state.commitId, [state.blobPath])
+    axios
+      .get(state.codeNavUrl)
       .then(({ data }) => {
-        const dataForPath = data[state.blobPath];
-        const normalizedData = dataForPath.reduce((acc, d) => {
+        const normalizedData = data.reduce((acc, d) => {
           if (d.hover) {
             acc[`${d.start_line}:${d.start_char}`] = d;
             addInteractionClass(d);

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

 import * as types from './mutation_types';
 
 export default {
-  [types.SET_INITIAL_DATA](state, { projectPath, commitId, blobPath }) {
-    state.projectPath = projectPath;
-    state.commitId = commitId;
-    state.blobPath = blobPath;
+  [types.SET_INITIAL_DATA](state, { codeNavUrl, definitionPathPrefix }) {
+    state.codeNavUrl = codeNavUrl;
+    state.definitionPathPrefix = definitionPathPrefix;
   },
   [types.REQUEST_DATA](state) {
     state.loading = true;

app/controllers/projects/blob_controller.rb

@@ -208,11 +208,24 @@ class Projects::BlobController < Projects::ApplicationController
       .last_for_path(@repository, @ref, @path).sha
   end
 
+  def set_code_navigation_build
+    return if Feature.disabled?(:code_navigation, @project)
+
+    artifact =
+      Ci::JobArtifact
+        .for_sha(@blob.commit_id, @project.id)
+        .for_job_name(Ci::Build::CODE_NAVIGATION_JOB_NAME)
+        .last
+
+    @code_navigation_build = artifact&.job
+  end
+
   def show_html
     environment_params = @repository.branch_exists?(@ref) ? { ref: @ref } : { commit: @commit }
     environment_params[:find_latest] = true
     @environment = EnvironmentsFinder.new(@project, current_user, environment_params).execute.last
     @last_commit = @repository.last_commit_for_path(@commit.id, @blob.path)
+    set_code_navigation_build
 
     render 'show'
   end

app/models/ci/build.rb

@@ -33,6 +33,8 @@ module Ci
       scheduler_failure: 2
     }.freeze
 
+    CODE_NAVIGATION_JOB_NAME = 'code_navigation'
+
     has_one :deployment, as: :deployable, class_name: 'Deployment'
     has_one :resource, class_name: 'Ci::Resource', inverse_of: :build
     has_many :trace_sections, class_name: 'Ci::BuildTraceSection'

app/models/ci/job_artifact.rb

@@ -82,6 +82,7 @@ module Ci
     scope :with_files_stored_locally, -> { where(file_store: [nil, ::JobArtifactUploader::Store::LOCAL]) }
     scope :with_files_stored_remotely, -> { where(file_store: ::JobArtifactUploader::Store::REMOTE) }
     scope :for_sha, ->(sha, project_id) { joins(job: :pipeline).where(ci_pipelines: { sha: sha, project_id: project_id }) }
+    scope :for_job_name, ->(name) { joins(:job).where(ci_builds: { name: name }) }
 
     scope :with_file_types, -> (file_types) do
       types = self.file_types.select { |file_type| file_types.include?(file_type) }.values

app/views/projects/blob/_blob.html.haml

@@ -9,8 +9,9 @@
   = render "projects/blob/auxiliary_viewer", blob: blob
 
 #blob-content-holder.blob-content-holder
-  - if native_code_navigation_enabled?(@project)
-    #js-code-navigation{ data: { commit_id: blob.commit_id, blob_path: blob.path, project_path: @project.full_path } }
+  - if @code_navigation_build
+    - code_nav_url = raw_project_job_artifacts_url(@project, @code_navigation_build, path: "lsif/#{blob.path}")
+    #js-code-navigation{ data: { code_nav_url: "#{code_nav_url}.json", definition_path_prefix: project_blob_path(@project, @ref) } }
   %article.file-holder
     = render 'projects/blob/header', blob: blob
     = render 'projects/blob/content', blob: blob

spec/controllers/projects/blob_controller_spec.rb

@@ -8,18 +8,17 @@ describe Projects::BlobController do
   let(:project) { create(:project, :public, :repository) }
 
   describe "GET show" do
+    def request
+      get(:show, params: { namespace_id: project.namespace, project_id: project, id: id })
+    end
+
     render_views
 
     context 'with file path' do
       before do
         expect(::Gitlab::GitalyClient).to receive(:allow_ref_name_caching).and_call_original
 
-        get(:show,
-            params: {
-              namespace_id: project.namespace,
-              project_id: project,
-              id: id
-            })
+        request
       end
 
       context "valid branch, valid file" do
@@ -119,6 +118,32 @@ describe Projects::BlobController do
         end
       end
     end
+
+    context 'when there is an artifact with code navigation data' do
+      let!(:pipeline) { create(:ci_pipeline, project: project, sha: project.commit.id) }
+      let!(:job) { create(:ci_build, pipeline: pipeline, name: Ci::Build::CODE_NAVIGATION_JOB_NAME) }
+      let!(:artifact) { create(:ci_job_artifact, :lsif, job: job) }
+
+      let(:id) { 'master/README.md' }
+
+      it 'assigns code_navigation_build variable' do
+        request
+
+        expect(assigns[:code_navigation_build]).to eq(job)
+      end
+
+      context 'when code_navigation feature is disabled' do
+        before do
+          stub_feature_flags(code_navigation: false)
+        end
+
+        it 'does not assign code_navigation_build variable' do
+          request
+
+          expect(assigns[:code_navigation_build]).to be_nil
+        end
+      end
+    end
   end
 
   describe 'GET diff' do

spec/frontend/code_navigation/components/app_spec.js

@@ -16,6 +16,7 @@ function factory(initialState = {}) {
     state: {
       ...createState(),
       ...initialState,
+      definitionPathPrefix: 'https://test.com/blob/master',
     },
     actions: {
       fetchData,

spec/frontend/code_navigation/components/popover_spec.js

 import { shallowMount } from '@vue/test-utils';
 import Popover from '~/code_navigation/components/popover.vue';
 
+const DEFINITION_PATH_PREFIX = 'http:/';
+
 const MOCK_CODE_DATA = Object.freeze({
   hover: [
     {
@@ -8,7 +10,7 @@ const MOCK_CODE_DATA = Object.freeze({
       value: 'console.log',
     },
   ],
-  definition_url: 'http://test.com',
+  definition_path: 'test.com',
 });
 
 const MOCK_DOCS_DATA = Object.freeze({
@@ -18,13 +20,13 @@ const MOCK_DOCS_DATA = Object.freeze({
       value: 'console.log',
     },
   ],
-  definition_url: 'http://test.com',
+  definition_path: 'test.com',
 });
 
 let wrapper;
 
-function factory(position, data) {
-  wrapper = shallowMount(Popover, { propsData: { position, data } });
+function factory(position, data, definitionPathPrefix) {
+  wrapper = shallowMount(Popover, { propsData: { position, data, definitionPathPrefix } });
 }
 
 describe('Code navigation popover component', () => {
@@ -33,14 +35,14 @@ describe('Code navigation popover component', () => {
   });
 
   it('renders popover', () => {
-    factory({ x: 0, y: 0, height: 0 }, MOCK_CODE_DATA);
+    factory({ x: 0, y: 0, height: 0 }, MOCK_CODE_DATA, DEFINITION_PATH_PREFIX);
 
     expect(wrapper.element).toMatchSnapshot();
   });
 
   describe('code output', () => {
     it('renders code output', () => {
-      factory({ x: 0, y: 0, height: 0 }, MOCK_CODE_DATA);
+      factory({ x: 0, y: 0, height: 0 }, MOCK_CODE_DATA, DEFINITION_PATH_PREFIX);
 
       expect(wrapper.find({ ref: 'code-output' }).exists()).toBe(true);
       expect(wrapper.find({ ref: 'doc-output' }).exists()).toBe(false);
@@ -49,7 +51,7 @@ describe('Code navigation popover component', () => {
 
   describe('documentation output', () => {
     it('renders code output', () => {
-      factory({ x: 0, y: 0, height: 0 }, MOCK_DOCS_DATA);
+      factory({ x: 0, y: 0, height: 0 }, MOCK_DOCS_DATA, DEFINITION_PATH_PREFIX);
 
       expect(wrapper.find({ ref: 'code-output' }).exists()).toBe(false);
       expect(wrapper.find({ ref: 'doc-output' }).exists()).toBe(true);

spec/frontend/code_navigation/store/actions_spec.js

@@ -27,12 +27,10 @@ describe('Code navigation actions', () => {
 
   describe('fetchData', () => {
     let mock;
-    const state = {
-      projectPath: 'gitlab-org/gitlab',
-      commitId: '123',
-      blobPath: 'index',
-    };
-    const apiUrl = '/api/1/projects/gitlab-org%2Fgitlab/commits/123/lsif/info';
+
+    const codeNavUrl =
+      'gitlab-org/gitlab-shell/-/jobs/1114/artifacts/raw/lsif/cmd/check/main.go.json';
+    const state = { codeNavUrl };
 
     beforeEach(() => {
       window.gon = { api_version: '1' };
@@ -45,20 +43,18 @@ describe('Code navigation actions', () => {
 
     describe('success', () => {
       beforeEach(() => {
-        mock.onGet(apiUrl).replyOnce(200, {
-          index: [
-            {
-              start_line: 0,
-              start_char: 0,
-              hover: { value: '123' },
-            },
-            {
-              start_line: 1,
-              start_char: 0,
-              hover: null,
-            },
-          ],
-        });
+        mock.onGet(codeNavUrl).replyOnce(200, [
+          {
+            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 => {
@@ -106,7 +102,7 @@ describe('Code navigation actions', () => {
 
     describe('error', () => {
       beforeEach(() => {
-        mock.onGet(apiUrl).replyOnce(500);
+        mock.onGet(codeNavUrl).replyOnce(500);
       });
 
       it('dispatches requestDataError', done => {

spec/frontend/code_navigation/store/mutations_spec.js

@@ -11,14 +11,12 @@ describe('Code navigation mutations', () => {
   describe('SET_INITIAL_DATA', () => {
     it('sets initial data', () => {
       mutations.SET_INITIAL_DATA(state, {
-        projectPath: 'test',
-        commitId: '123',
-        blobPath: 'index.js',
+        codeNavUrl: 'https://test.com/builds/1005',
+        definitionPathPrefix: 'https://test.com/blob/master',
       });
 
-      expect(state.projectPath).toBe('test');
-      expect(state.commitId).toBe('123');
-      expect(state.blobPath).toBe('index.js');
+      expect(state.codeNavUrl).toBe('https://test.com/builds/1005');
+      expect(state.definitionPathPrefix).toBe('https://test.com/blob/master');
     });
   });
 

spec/models/ci/job_artifact_spec.rb

@@ -140,6 +140,18 @@ describe Ci::JobArtifact do
     end
   end
 
+  describe '.for_job_name' do
+    it 'returns job artifacts for a given job name' do
+      first_job = create(:ci_build, name: 'first')
+      second_job = create(:ci_build, name: 'second')
+      first_artifact = create(:ci_job_artifact, job: first_job)
+      second_artifact = create(:ci_job_artifact, job: second_job)
+
+      expect(described_class.for_job_name(first_job.name)).to eq([first_artifact])
+      expect(described_class.for_job_name(second_job.name)).to eq([second_artifact])
+    end
+  end
+
   describe 'callbacks' do
     subject { create(:ci_job_artifact, :archive) }