Merge branch 'ce-to-ee-2018-04-11' into 'master'

CE upstream - 2018-04-11 15:29 UTC

See merge request gitlab-org/gitlab-ee!5328

app/assets/javascripts/pipelines/pipeline_details_mediator.js

@@ -40,10 +40,8 @@ export default class pipelinesMediator {
   }
 
   successCallback(response) {
-    return response.json().then((data) => {
-      this.state.isLoading = false;
-      this.store.storePipeline(data);
-    });
+    this.state.isLoading = false;
+    this.store.storePipeline(response.data);
   }
 
   errorCallback() {

app/assets/javascripts/pipelines/services/pipeline_service.js

-import Vue from 'vue';
-import VueResource from 'vue-resource';
-
-Vue.use(VueResource);
+import axios from '../../lib/utils/axios_utils';
 
 export default class PipelineService {
   constructor(endpoint) {
-    this.pipeline = Vue.resource(endpoint);
+    this.pipeline = endpoint;
   }
 
   getPipeline() {
-    return this.pipeline.get();
+    return axios.get(this.pipeline);
   }
 
-  // eslint-disable-next-line
+  // eslint-disable-next-line class-methods-use-this
   postAction(endpoint) {
-    return Vue.http.post(`${endpoint}.json`);
+    return axios.post(`${endpoint}.json`);
   }
 
   static getSecurityReport(endpoint) {
-    return Vue.http.get(endpoint);
+    return axios.get(endpoint);
   }
 }

app/controllers/projects/commit_controller.rb

@@ -34,6 +34,7 @@ class Projects::CommitController < Projects::ApplicationController
 
   def pipelines
     @pipelines = @commit.pipelines.order(id: :desc)
+    @pipelines = @pipelines.where(ref: params[:ref]) if params[:ref]
 
     respond_to do |format|
       format.html

app/helpers/ci_status_helper.rb

@@ -94,7 +94,7 @@ module CiStatusHelper
 
   def render_project_pipeline_status(pipeline_status, tooltip_placement: 'auto left')
     project = pipeline_status.project
-    path = pipelines_project_commit_path(project, pipeline_status.sha)
+    path = pipelines_project_commit_path(project, pipeline_status.sha, ref: pipeline_status.ref)
 
     render_status_with_link(
       'commit',
@@ -105,7 +105,7 @@ module CiStatusHelper
 
   def render_commit_status(commit, ref: nil, tooltip_placement: 'auto left')
     project = commit.project
-    path = pipelines_project_commit_path(project, commit)
+    path = pipelines_project_commit_path(project, commit, ref: ref)
 
     render_status_with_link(
       'commit',

app/models/event.rb

@@ -116,7 +116,10 @@ class Event < ActiveRecord::Base
       end
     end
 
+    # Remove this method when removing Gitlab.rails5? code.
     def subclass_from_attributes(attrs)
+      return super if Gitlab.rails5?
+
       # Without this Rails will keep calling this method on the returned class,
       # resulting in an infinite loop.
       return unless self == Event

app/views/projects/commits/_commit.html.haml

@@ -5,6 +5,7 @@
 
 - link = commit_path(project, commit, merge_request: merge_request)
 - cache_key = [project.full_path,
+               ref,
                commit.id,
                Gitlab::CurrentSettings.current_application_settings,
                @path.presence,
@@ -61,7 +62,7 @@
         - if commit.status(ref)
           = render_commit_status(commit, ref: ref)
 
-        .js-commit-pipeline-status{ data: { endpoint: pipelines_project_commit_path(project, commit.id) } }
+        .js-commit-pipeline-status{ data: { endpoint: pipelines_project_commit_path(project, commit.id, ref: ref) } }
 
         .commit-sha-group
           .label.label-monospace

changelogs/unreleased/44541-fix-file-tree-commit-status-cache.yml

+---
+title: Fix pipeline status in branch/tag tree page
+merge_request: 17995
+author:
+type: fixed

changelogs/unreleased/45159-fix-illustration.yml

+---
+title: Adds illustration for when job log was erased
+merge_request:
+author:
+type: fixed

changelogs/unreleased/fl-pipelines-details-axios.yml

+---
+title: Replace vue resource with axios for pipelines details page
+merge_request:
+author:
+type: other

lib/gitlab/ci/status/build/erased.rb

@@ -5,7 +5,7 @@ module Gitlab
         class Erased < Status::Extended
           def illustration
             {
-              image: 'illustrations/skipped-job_empty.svg',
+              image: 'illustrations/erased-log_empty.svg',
               size: 'svg-430',
               title: _('Job has been erased')
             }

lib/gitlab/git/attributes_parser.rb

@@ -3,12 +3,8 @@ module Gitlab
     # Class for parsing Git attribute files and extracting the attributes for
     # file patterns.
     class AttributesParser
-      def initialize(attributes_data)
+      def initialize(attributes_data = "")
         @data = attributes_data || ""
-
-        if @data.is_a?(File)
-          @patterns = parse_file
-        end
       end
 
       # Returns all the Git attributes for the given path.
@@ -28,7 +24,7 @@ module Gitlab
 
       # Returns a Hash containing the file patterns and their attributes.
       def patterns
-        @patterns ||= parse_file
+        @patterns ||= parse_data
       end
 
       # Parses an attribute string.
@@ -91,8 +87,8 @@ module Gitlab
 
       private
 
-      # Parses the Git attributes file.
-      def parse_file
+      # Parses the Git attributes file contents.
+      def parse_data
         pairs = []
         comment = '#'
 

lib/gitlab/git/info_attributes.rb

-# Gitaly note: JV: not sure what to make of this class. Why does it use
-# the full disk path of the repository to look up attributes This is
-# problematic in Gitaly, because Gitaly hides the full disk path to the
-# repository from gitlab-ce.
-
-module Gitlab
-  module Git
-    # Parses gitattributes at `$GIT_DIR/info/attributes`
-    #
-    # Unlike Rugged this parser only needs a single IO call (a call to `open`),
-    # vastly reducing the time spent in extracting attributes.
-    #
-    # This class _only_ supports parsing the attributes file located at
-    # `$GIT_DIR/info/attributes` as GitLab doesn't use any other files
-    # (`.gitattributes` is copied to this particular path).
-    #
-    # Basic usage:
-    #
-    #     attributes = Gitlab::Git::InfoAttributes.new(some_repo.path)
-    #
-    #     attributes.attributes('README.md') # => { "eol" => "lf }
-    class InfoAttributes
-      delegate :attributes, :patterns, to: :parser
-
-      # path - The path to the Git repository.
-      def initialize(path)
-        @repo_path = File.expand_path(path)
-      end
-
-      def parser
-        @parser ||= begin
-          if File.exist?(attributes_path)
-            File.open(attributes_path, 'r') do |file_handle|
-              AttributesParser.new(file_handle)
-            end
-          else
-            AttributesParser.new("")
-          end
-        end
-      end
-
-      private
-
-      def attributes_path
-        @attributes_path ||= File.join(@repo_path, 'info/attributes')
-      end
-    end
-  end
-end

lib/gitlab/git/repository.rb

@@ -105,7 +105,6 @@ module Gitlab
         )
         @path = File.join(storage_path, @relative_path)
         @name = @relative_path.split("/").last
-        @attributes = Gitlab::Git::InfoAttributes.new(path)
       end
 
       def ==(other)
@@ -997,11 +996,32 @@ module Gitlab
         raise InvalidRef
       end
 
+      def info_attributes
+        return @info_attributes if @info_attributes
+
+        content =
+          gitaly_migrate(:get_info_attributes) do |is_enabled|
+            if is_enabled
+              gitaly_repository_client.info_attributes
+            else
+              attributes_path = File.join(File.expand_path(@path), 'info', 'attributes')
+
+              if File.exist?(attributes_path)
+                File.read(attributes_path)
+              else
+                ""
+              end
+            end
+          end
+
+        @info_attributes = AttributesParser.new(content)
+      end
+
       # Returns the Git attributes for the given file path.
       #
       # See `Gitlab::Git::Attributes` for more information.
       def attributes(path)
-        @attributes.attributes(path)
+        info_attributes.attributes(path)
       end
 
       def gitattribute(path, name)

lib/gitlab/gitaly_client/repository_service.rb

@@ -50,6 +50,15 @@ module Gitlab
         GitalyClient.call(@storage, :repository_service, :apply_gitattributes, request)
       end
 
+      def info_attributes
+        request = Gitaly::GetInfoAttributesRequest.new(repository: @gitaly_repo)
+
+        response = GitalyClient.call(@storage, :repository_service, :get_info_attributes, request)
+        response.each_with_object("") do |message, attributes|
+          attributes << message.attributes
+        end
+      end
+
       def fetch_remote(remote, ssh_auth:, forced:, no_tags:, timeout:, prune: true)
         request = Gitaly::FetchRemoteRequest.new(
           repository: @gitaly_repo, remote: remote, force: forced,

spec/features/dashboard/projects_spec.rb

@@ -89,7 +89,7 @@ feature 'Dashboard Projects' do
   end
 
   describe 'with a pipeline', :clean_gitlab_redis_shared_state do
-    let(:pipeline) { create(:ci_pipeline, project: project, sha: project.commit.sha) }
+    let(:pipeline) { create(:ci_pipeline, project: project, sha: project.commit.sha, ref: project.default_branch) }
 
     before do
       # Since the cache isn't updated when a new pipeline is created
@@ -102,7 +102,7 @@ feature 'Dashboard Projects' do
       visit dashboard_projects_path
 
       page.within('.controls') do
-        expect(page).to have_xpath("//a[@href='#{pipelines_project_commit_path(project, project.commit)}']")
+        expect(page).to have_xpath("//a[@href='#{pipelines_project_commit_path(project, project.commit, ref: pipeline.ref)}']")
         expect(page).to have_css('.ci-status-link')
         expect(page).to have_css('.ci-status-icon-success')
         expect(page).to have_link('Commit: passed')

spec/features/projects/files/user_reads_pipeline_status_spec.rb

+require 'spec_helper'
+
+describe 'user reads pipeline status', :js do
+  let(:project) { create(:project, :repository) }
+  let(:user) { create(:user) }
+  let(:v110_pipeline) { create_pipeline('v1.1.0', 'success') }
+  let(:x110_pipeline) { create_pipeline('x1.1.0', 'failed') }
+
+  before do
+    project.add_master(user)
+
+    project.repository.add_tag(user, 'x1.1.0', 'v1.1.0')
+    v110_pipeline
+    x110_pipeline
+
+    sign_in(user)
+  end
+
+  shared_examples 'visiting project tree' do
+    scenario 'sees the correct pipeline status' do
+      visit project_tree_path(project, expected_pipeline.ref)
+      wait_for_requests
+
+      page.within('.blob-commit-info') do
+        expect(page).to have_link('', href: project_pipeline_path(project, expected_pipeline))
+        expect(page).to have_selector(".ci-status-icon-#{expected_pipeline.status}")
+      end
+    end
+  end
+
+  it_behaves_like 'visiting project tree' do
+    let(:expected_pipeline) { v110_pipeline }
+  end
+
+  it_behaves_like 'visiting project tree' do
+    let(:expected_pipeline) { x110_pipeline }
+  end
+
+  def create_pipeline(ref, status)
+    create(:ci_pipeline,
+      project: project,
+      ref: ref,
+      sha: project.commit(ref).sha,
+      status: status)
+  end
+end

spec/javascripts/pipelines/pipeline_details_mediator_spec.js

-import _ from 'underscore';
-import Vue from 'vue';
+import MockAdapter from 'axios-mock-adapter';
+import axios from '~/lib/utils/axios_utils';
 import PipelineMediator from '~/pipelines/pipeline_details_mediator';
 
 describe('PipelineMdediator', () => {
   let mediator;
+  let mock;
+
   beforeEach(() => {
-    mediator = new PipelineMediator({ endpoint: 'foo' });
+    mock = new MockAdapter(axios);
+    mediator = new PipelineMediator({ endpoint: 'foo.json' });
+  });
+
+  afterEach(() => {
+    mock.restore();
   });
 
   it('should set defaults', () => {
-    expect(mediator.options).toEqual({ endpoint: 'foo' });
+    expect(mediator.options).toEqual({ endpoint: 'foo.json' });
     expect(mediator.state.isLoading).toEqual(false);
     expect(mediator.store).toBeDefined();
     expect(mediator.service).toBeDefined();
   });
 
   describe('request and store data', () => {
-    const interceptor = (request, next) => {
-      next(request.respondWith(JSON.stringify({ foo: 'bar' }), {
-        status: 200,
-      }));
-    };
-
-    beforeEach(() => {
-      Vue.http.interceptors.push(interceptor);
-    });
-
-    afterEach(() => {
-      Vue.http.interceptors = _.without(Vue.http.interceptor, interceptor);
-    });
-
-    it('should store received data', (done) => {
+    it('should store received data', done => {
+      mock.onGet('foo.json').reply(200, { id: '121123' });
       mediator.fetchPipeline();
 
       setTimeout(() => {
-        expect(mediator.store.state.pipeline).toEqual({ foo: 'bar' });
+        expect(mediator.store.state.pipeline).toEqual({ id: '121123' });
         done();
-      });
+      }, 0);
     });
   });
 });

spec/lib/gitlab/git/attributes_parser_spec.rb

@@ -66,18 +66,6 @@ describe Gitlab::Git::AttributesParser, seed_helper: true do
       end
     end
 
-    context 'when attributes data is a file handle' do
-      subject do
-        File.open(attributes_path, 'r') do |file_handle|
-          described_class.new(file_handle)
-        end
-      end
-
-      it 'returns the attributes as a Hash' do
-        expect(subject.attributes('test.txt')).to eq({ 'text' => true })
-      end
-    end
-
     context 'when attributes data is nil' do
       let(:data) { nil }
 

spec/lib/gitlab/git/info_attributes_spec.rb

-require 'spec_helper'
-
-describe Gitlab::Git::InfoAttributes, seed_helper: true do
-  let(:path) do
-    File.join(SEED_STORAGE_PATH, 'with-git-attributes.git')
-  end
-
-  subject { described_class.new(path) }
-
-  describe '#attributes' do
-    context 'using a path with attributes' do
-      it 'returns the attributes as a Hash' do
-        expect(subject.attributes('test.txt')).to eq({ 'text' => true })
-      end
-
-      it 'returns an empty Hash for a defined path without attributes' do
-        expect(subject.attributes('bla/bla.txt')).to eq({})
-      end
-    end
-  end
-
-  describe '#parser' do
-    it 'parses a file with entries' do
-      expect(subject.patterns).to be_an_instance_of(Hash)
-      expect(subject.patterns["/*.txt"]).to eq({ 'text' => true })
-    end
-
-    it 'does not parse anything when the attributes file does not exist' do
-      expect(File).to receive(:exist?)
-        .with(File.join(path, 'info/attributes'))
-        .and_return(false)
-
-      expect(subject.patterns).to eq({})
-    end
-
-    it 'does not parse attributes files with unsupported encoding' do
-      path = File.join(SEED_STORAGE_PATH, 'with-invalid-git-attributes.git')
-      subject = described_class.new(path)
-
-      expect(subject.patterns).to eq({})
-    end
-  end
-end

spec/lib/gitlab/gitaly_client/repository_service_spec.rb

@@ -84,6 +84,17 @@ describe Gitlab::GitalyClient::RepositoryService do
     end
   end
 
+  describe '#info_attributes' do
+    it 'reads the info attributes' do
+      expect_any_instance_of(Gitaly::RepositoryService::Stub)
+        .to receive(:get_info_attributes)
+        .with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash))
+        .and_return([])
+
+      client.info_attributes
+    end
+  end
+
   describe '#has_local_branches?' do
     it 'sends a has_local_branches message' do
       expect_any_instance_of(Gitaly::RepositoryService::Stub)