Merge branch '5983-realtime-pipelines-table' into 'master'

app/assets/javascripts/commit/pipelines/pipelines_bundle.js

@@ -8,10 +8,8 @@ Vue.use(VueResource);
 
 /**
  * Commits View > Pipelines Tab > Pipelines Table.
- * Merge Request View > Pipelines Tab > Pipelines Table.
  *
  * Renders Pipelines table in pipelines tab in the commits show view.
- * Renders Pipelines table in pipelines tab in the merge request show view.
  */
 
 $(() => {
@@ -20,13 +18,14 @@ $(() => {
   gl.commits.pipelines = gl.commits.pipelines || {};
 
   if (gl.commits.PipelinesTableBundle) {
+    document.querySelector('#commit-pipeline-table-view').removeChild(this.pipelinesTableBundle.$el);
     gl.commits.PipelinesTableBundle.$destroy(true);
   }
 
   const pipelineTableViewEl = document.querySelector('#commit-pipeline-table-view');
-  gl.commits.pipelines.PipelinesTableBundle = new CommitPipelinesTable();
 
   if (pipelineTableViewEl && pipelineTableViewEl.dataset.disableInitialization === undefined) {
-    gl.commits.pipelines.PipelinesTableBundle.$mount(pipelineTableViewEl);
+    gl.commits.pipelines.PipelinesTableBundle = new CommitPipelinesTable().$mount();
+    document.querySelector('#commit-pipeline-table-view').appendChild(gl.commits.pipelines.PipelinesTableBundle.$el);
   }
 });

app/assets/javascripts/commit/pipelines/pipelines_table.js

 import Vue from 'vue';
+import Visibility from 'visibilityjs';
 import PipelinesTableComponent from '../../vue_shared/components/pipelines_table';
 import PipelinesService from '../../vue_pipelines_index/services/pipelines_service';
 import PipelineStore from '../../vue_pipelines_index/stores/pipelines_store';
@@ -7,6 +8,7 @@ import EmptyState from '../../vue_pipelines_index/components/empty_state';
 import ErrorState from '../../vue_pipelines_index/components/error_state';
 import '../../lib/utils/common_utils';
 import '../../vue_shared/vue_resource_interceptor';
+import Poll from '../../lib/utils/poll';
 
 /**
  *
@@ -20,6 +22,7 @@ import '../../vue_shared/vue_resource_interceptor';
  */
 
 export default Vue.component('pipelines-table', {
+
   components: {
     'pipelines-table-component': PipelinesTableComponent,
     'error-state': ErrorState,
@@ -42,6 +45,7 @@ export default Vue.component('pipelines-table', {
       state: store.state,
       isLoading: false,
       hasError: false,
+      isMakingRequest: false,
     };
   },
 
@@ -64,17 +68,41 @@ export default Vue.component('pipelines-table', {
    *
    */
   beforeMount() {
-    this.endpoint = this.$el.dataset.endpoint;
-    this.helpPagePath = this.$el.dataset.helpPagePath;
+    const element = document.querySelector('#commit-pipeline-table-view');
+
+    this.endpoint = element.dataset.endpoint;
+    this.helpPagePath = element.dataset.helpPagePath;
     this.service = new PipelinesService(this.endpoint);
 
-    this.fetchPipelines();
+    this.poll = new Poll({
+      resource: this.service,
+      method: 'getPipelines',
+      successCallback: this.successCallback,
+      errorCallback: this.errorCallback,
+      notificationCallback: this.setIsMakingRequest,
+    });
+
+    if (!Visibility.hidden()) {
+      this.isLoading = true;
+      this.poll.makeRequest();
+    }
+
+    Visibility.change(() => {
+      if (!Visibility.hidden()) {
+        this.poll.restart();
+      } else {
+        this.poll.stop();
+      }
+    });
 
     eventHub.$on('refreshPipelines', this.fetchPipelines);
   },
 
   beforeUpdate() {
-    if (this.state.pipelines.length && this.$children) {
+    if (this.state.pipelines.length &&
+        this.$children &&
+        !this.isMakingRequest &&
+        !this.isLoading) {
       this.store.startTimeAgoLoops.call(this, Vue);
     }
   },
@@ -83,21 +111,35 @@ export default Vue.component('pipelines-table', {
     eventHub.$off('refreshPipelines');
   },
 
+  destroyed() {
+    this.poll.stop();
+  },
+
   methods: {
     fetchPipelines() {
       this.isLoading = true;
+
       return this.service.getPipelines()
-        .then(response => response.json())
-        .then((json) => {
-          // depending of the endpoint the response can either bring a `pipelines` key or not.
-          const pipelines = json.pipelines || json;
-          this.store.storePipelines(pipelines);
-          this.isLoading = false;
-        })
-        .catch(() => {
-          this.hasError = true;
-          this.isLoading = false;
-        });
+        .then(response => this.successCallback(response))
+        .catch(() => this.errorCallback());
+    },
+
+    successCallback(resp) {
+      const response = resp.json();
+
+      // depending of the endpoint the response can either bring a `pipelines` key or not.
+      const pipelines = response.pipelines || response;
+      this.store.storePipelines(pipelines);
+      this.isLoading = false;
+    },
+
+    errorCallback() {
+      this.hasError = true;
+      this.isLoading = false;
+    },
+
+    setIsMakingRequest(isMakingRequest) {
+      this.isMakingRequest = isMakingRequest;
     },
   },
 

app/assets/javascripts/lib/utils/poll.js

@@ -65,7 +65,6 @@ export default class Poll {
         this.makeRequest();
       }, pollInterval);
     }
-
     this.options.successCallback(response);
   }
 
@@ -76,8 +75,14 @@ export default class Poll {
     notificationCallback(true);
 
     return resource[method](data)
-      .then(response => this.checkConditions(response))
-      .catch(error => errorCallback(error));
+      .then((response) => {
+        this.checkConditions(response);
+        notificationCallback(false);
+      })
+      .catch((error) => {
+        notificationCallback(false);
+        errorCallback(error);
+      });
   }
 
   /**

app/assets/javascripts/merge_request_tabs.js

@@ -90,6 +90,7 @@ import './flash';
         .on('click', this.clickTab);
     }
 
+    // Used in tests
     unbindEvents() {
       $(document)
         .off('shown.bs.tab', '.merge-request-tabs a[data-toggle="tab"]', this.tabShown)
@@ -99,9 +100,11 @@ import './flash';
         .off('click', this.clickTab);
     }
 
-    destroy() {
-      this.unbindEvents();
+    destroyPipelinesView() {
       if (this.commitPipelinesTable) {
+        document.querySelector('#commit-pipeline-table-view')
+          .removeChild(this.commitPipelinesTable.$el);
+
         this.commitPipelinesTable.$destroy();
       }
     }
@@ -128,6 +131,7 @@ import './flash';
         this.loadCommits($target.attr('href'));
         this.expandView();
         this.resetViewContainer();
+        this.destroyPipelinesView();
       } else if (this.isDiffAction(action)) {
         this.loadDiff($target.attr('href'));
         if (Breakpoints.get().getBreakpointSize() !== 'lg') {
@@ -136,12 +140,14 @@ import './flash';
         if (this.diffViewType() === 'parallel') {
           this.expandViewContainer();
         }
+        this.destroyPipelinesView();
       } else if (action === 'pipelines') {
         this.resetViewContainer();
-        this.loadPipelines();
+        this.mountPipelinesView();
       } else {
         this.expandView();
         this.resetViewContainer();
+        this.destroyPipelinesView();
       }
       if (this.setUrl) {
         this.setCurrentAction(action);
@@ -227,16 +233,12 @@ import './flash';
       });
     }
 
-    loadPipelines() {
-      if (this.pipelinesLoaded) {
-        return;
-      }
-      const pipelineTableViewEl = document.querySelector('#commit-pipeline-table-view');
-      // Could already be mounted from the `pipelines_bundle`
-      if (pipelineTableViewEl) {
-        this.commitPipelinesTable = new CommitPipelinesTable().$mount(pipelineTableViewEl);
-      }
-      this.pipelinesLoaded = true;
+    mountPipelinesView() {
+      this.commitPipelinesTable = new CommitPipelinesTable().$mount();
+      // $mount(el) replaces the el with the new rendered component. We need it in order to mount
+      // it everytime this tab is clicked - https://vuejs.org/v2/api/#vm-mount
+      document.querySelector('#commit-pipeline-table-view')
+        .appendChild(this.commitPipelinesTable.$el);
     }
 
     loadDiff(source) {

app/assets/javascripts/vue_pipelines_index/pipelines.js

 import Vue from 'vue';
+import Visibility from 'visibilityjs';
 import PipelinesService from './services/pipelines_service';
 import eventHub from './event_hub';
 import PipelinesTableComponent from '../vue_shared/components/pipelines_table';
@@ -7,6 +8,7 @@ import EmptyState from './components/empty_state';
 import ErrorState from './components/error_state';
 import NavigationTabs from './components/navigation_tabs';
 import NavigationControls from './components/nav_controls';
+import Poll from '../lib/utils/poll';
 
 export default {
   props: {
@@ -47,6 +49,7 @@ export default {
       pagenum: 1,
       isLoading: false,
       hasError: false,
+      isMakingRequest: false,
     };
   },
 
@@ -120,18 +123,49 @@ export default {
         tagsPath: this.tagsPath,
       };
     },
+
+    pageParameter() {
+      return gl.utils.getParameterByName('page') || this.pagenum;
+    },
+
+    scopeParameter() {
+      return gl.utils.getParameterByName('scope') || this.apiScope;
+    },
   },
 
   created() {
     this.service = new PipelinesService(this.endpoint);
 
-    this.fetchPipelines();
+    const poll = new Poll({
+      resource: this.service,
+      method: 'getPipelines',
+      data: { page: this.pageParameter, scope: this.scopeParameter },
+      successCallback: this.successCallback,
+      errorCallback: this.errorCallback,
+      notificationCallback: this.setIsMakingRequest,
+    });
+
+    if (!Visibility.hidden()) {
+      this.isLoading = true;
+      poll.makeRequest();
+    }
+
+    Visibility.change(() => {
+      if (!Visibility.hidden()) {
+        poll.restart();
+      } else {
+        poll.stop();
+      }
+    });
 
     eventHub.$on('refreshPipelines', this.fetchPipelines);
   },
 
   beforeUpdate() {
-    if (this.state.pipelines.length && this.$children) {
+    if (this.state.pipelines.length &&
+        this.$children &&
+        !this.isMakingRequest &&
+        !this.isLoading) {
       this.store.startTimeAgoLoops.call(this, Vue);
     }
   },
@@ -154,27 +188,35 @@ export default {
     },
 
     fetchPipelines() {
-      const pageNumber = gl.utils.getParameterByName('page') || this.pagenum;
-      const scope = gl.utils.getParameterByName('scope') || this.apiScope;
+      if (!this.isMakingRequest) {
+        this.isLoading = true;
 
-      this.isLoading = true;
-      return this.service.getPipelines(scope, pageNumber)
-        .then(resp => ({
-          headers: resp.headers,
-          body: resp.json(),
-        }))
-        .then((response) => {
-          this.store.storeCount(response.body.count);
-          this.store.storePipelines(response.body.pipelines);
-          this.store.storePagination(response.headers);
-        })
-        .then(() => {
-          this.isLoading = false;
-        })
-        .catch(() => {
-          this.hasError = true;
-          this.isLoading = false;
-        });
+        this.service.getPipelines({ scope: this.scopeParameter, page: this.pageParameter })
+          .then(response => this.successCallback(response))
+          .catch(() => this.errorCallback());
+      }
+    },
+
+    successCallback(resp) {
+      const response = {
+        headers: resp.headers,
+        body: resp.json(),
+      };
+
+      this.store.storeCount(response.body.count);
+      this.store.storePipelines(response.body.pipelines);
+      this.store.storePagination(response.headers);
+
+      this.isLoading = false;
+    },
+
+    errorCallback() {
+      this.hasError = true;
+      this.isLoading = false;
+    },
+
+    setIsMakingRequest(isMakingRequest) {
+      this.isMakingRequest = isMakingRequest;
     },
   },
 

app/assets/javascripts/vue_pipelines_index/services/pipelines_service.js

@@ -26,7 +26,8 @@ export default class PipelinesService {
     this.pipelines = Vue.resource(endpoint);
   }
 
-  getPipelines(scope, page) {
+  getPipelines(data = {}) {
+    const { scope, page } = data;
     return this.pipelines.get({ scope, page });
   }
 

app/controllers/projects/commit_controller.rb

@@ -35,6 +35,8 @@ class Projects::CommitController < Projects::ApplicationController
     respond_to do |format|
       format.html
       format.json do
+        Gitlab::PollingInterval.set_header(response, interval: 10_000)
+
         render json: PipelineSerializer
           .new(project: @project, user: @current_user)
           .represent(@pipelines)

app/controllers/projects/merge_requests_controller.rb

@@ -233,6 +233,8 @@ class Projects::MergeRequestsController < Projects::ApplicationController
       end
 
       format.json do
+        Gitlab::PollingInterval.set_header(response, interval: 10_000)
+
         render json: PipelineSerializer
           .new(project: @project, user: @current_user)
           .represent(@pipelines)
@@ -246,6 +248,8 @@ class Projects::MergeRequestsController < Projects::ApplicationController
       format.json do
         define_pipelines_vars
 
+        Gitlab::PollingInterval.set_header(response, interval: 10_000)
+
         render json: {
           pipelines: PipelineSerializer
           .new(project: @project, user: @current_user)

app/controllers/projects/pipelines_controller.rb

@@ -29,6 +29,8 @@ class Projects::PipelinesController < Projects::ApplicationController
     respond_to do |format|
       format.html
       format.json do
+        Gitlab::PollingInterval.set_header(response, interval: 10_000)
+
         render json: {
           pipelines: PipelineSerializer
             .new(project: @project, user: @current_user)

app/models/ci/pipeline.rb

@@ -88,6 +88,8 @@ module Ci
 
         pipeline.run_after_commit do
           PipelineHooksWorker.perform_async(id)
+          Ci::ExpirePipelineCacheService.new(project, nil)
+            .execute(pipeline)
         end
       end
 

app/services/ci/expire_pipeline_cache_service.rb

+module Ci
+  class ExpirePipelineCacheService < BaseService
+    attr_reader :pipeline
+
+    def execute(pipeline)
+      @pipeline = pipeline
+      store = Gitlab::EtagCaching::Store.new
+
+      store.touch(project_pipelines_path)
+      store.touch(commit_pipelines_path) if pipeline.commit
+      store.touch(new_merge_request_pipelines_path)
+      merge_requests_pipelines_paths.each { |path| store.touch(path) }
+    end
+
+    private
+
+    def project_pipelines_path
+      Gitlab::Routing.url_helpers.namespace_project_pipelines_path(
+        project.namespace,
+        project,
+        format: :json)
+    end
+
+    def commit_pipelines_path
+      Gitlab::Routing.url_helpers.pipelines_namespace_project_commit_path(
+        project.namespace,
+        project,
+        pipeline.commit.id,
+        format: :json)
+    end
+
+    def new_merge_request_pipelines_path
+      Gitlab::Routing.url_helpers.new_namespace_project_merge_request_path(
+        project.namespace,
+        project,
+        format: :json)
+    end
+
+    def merge_requests_pipelines_paths
+      pipeline.merge_requests.collect do |merge_request|
+        Gitlab::Routing.url_helpers.pipelines_namespace_project_merge_request_path(
+          project.namespace,
+          project,
+          merge_request,
+          format: :json)
+      end
+    end
+  end
+end

lib/gitlab/etag_caching/middleware.rb

@@ -10,6 +10,22 @@ module Gitlab
         {
           regexp: %r(^(?!.*(#{RESERVED_WORDS})).*/issues/\d+/rendered_title\z),
           name: 'issue_title'
+        },
+        {
+          regexp: %r(^(?!.*(#{RESERVED_WORDS})).*/pipelines\.json\z),
+          name: 'project_pipelines'
+        },
+        {
+          regexp: %r(^(?!.*(#{RESERVED_WORDS})).*/commit/\s+/pipelines\.json\z),
+          name: 'commit_pipelines'
+        },
+        {
+          regexp: %r(^(?!.*(#{RESERVED_WORDS})).*/merge_requests/new\.json\z),
+          name: 'new_merge_request_pipelines'
+        },
+        {
+          regexp: %r(^(?!.*(#{RESERVED_WORDS})).*/merge_requests/\d+/pipelines\.json\z),
+          name: 'merge_request_pipelines'
         }
       ].freeze
 
@@ -65,7 +81,7 @@ module Gitlab
 
         status_code = Gitlab::PollingInterval.polling_enabled? ? 304 : 429
 
-        [status_code, { 'ETag' => etag }, ['']]
+        [status_code, { 'ETag' => etag }, []]
       end
 
       def track_cache_miss(if_none_match, cached_value_present, route)

spec/javascripts/merge_request_tabs_spec.js

@@ -42,7 +42,8 @@ require('vendor/jquery.scrollTo');
     });
 
     afterEach(function () {
-      this.class.destroy();
+      this.class.unbindEvents();
+      this.class.destroyPipelinesView();
     });
 
     describe('#activateTab', function () {
@@ -68,6 +69,7 @@ require('vendor/jquery.scrollTo');
         expect($('#diffs')).toHaveClass('active');
       });
     });
+
     describe('#opensInNewTab', function () {
       var tabUrl;
       var windowTarget = '_blank';
@@ -119,6 +121,7 @@ require('vendor/jquery.scrollTo');
           stopImmediatePropagation: function () {}
         });
       });
+
       it('opens page tab in a new browser tab with Cmd+Click - Mac', function () {
         spyOn(window, 'open').and.callFake(function (url, name) {
           expect(url).toEqual(tabUrl);
@@ -132,6 +135,7 @@ require('vendor/jquery.scrollTo');
           stopImmediatePropagation: function () {}
         });
       });
+
       it('opens page tab in a new browser tab with Middle-click - Mac/PC', function () {
         spyOn(window, 'open').and.callFake(function (url, name) {
           expect(url).toEqual(tabUrl);
@@ -152,6 +156,7 @@ require('vendor/jquery.scrollTo');
         spyOn($, 'ajax').and.callFake(function () {});
         this.subject = this.class.setCurrentAction;
       });
+
       it('changes from commits', function () {
         setLocation({
           pathname: '/foo/bar/merge_requests/1/commits'
@@ -159,13 +164,16 @@ require('vendor/jquery.scrollTo');
         expect(this.subject('notes')).toBe('/foo/bar/merge_requests/1');
         expect(this.subject('diffs')).toBe('/foo/bar/merge_requests/1/diffs');
       });
+
       it('changes from diffs', function () {
         setLocation({
           pathname: '/foo/bar/merge_requests/1/diffs'
         });
+
         expect(this.subject('notes')).toBe('/foo/bar/merge_requests/1');
         expect(this.subject('commits')).toBe('/foo/bar/merge_requests/1/commits');
       });
+
       it('changes from diffs.html', function () {
         setLocation({
           pathname: '/foo/bar/merge_requests/1/diffs.html'
@@ -173,6 +181,7 @@ require('vendor/jquery.scrollTo');
         expect(this.subject('notes')).toBe('/foo/bar/merge_requests/1');
         expect(this.subject('commits')).toBe('/foo/bar/merge_requests/1/commits');
       });
+
       it('changes from notes', function () {
         setLocation({
           pathname: '/foo/bar/merge_requests/1'
@@ -180,6 +189,7 @@ require('vendor/jquery.scrollTo');
         expect(this.subject('diffs')).toBe('/foo/bar/merge_requests/1/diffs');
         expect(this.subject('commits')).toBe('/foo/bar/merge_requests/1/commits');
       });
+
       it('includes search parameters and hash string', function () {
         setLocation({
           pathname: '/foo/bar/merge_requests/1/diffs',
@@ -188,6 +198,7 @@ require('vendor/jquery.scrollTo');
         });
         expect(this.subject('show')).toBe('/foo/bar/merge_requests/1?view=parallel#L15-35');
       });
+
       it('replaces the current history state', function () {
         var newState;
         setLocation({
@@ -200,6 +211,7 @@ require('vendor/jquery.scrollTo');
           }, document.title, newState);
         }
       });
+
       it('treats "show" like "notes"', function () {
         setLocation({
           pathname: '/foo/bar/merge_requests/1/commits'
@@ -210,6 +222,7 @@ require('vendor/jquery.scrollTo');
 
     describe('#tabShown', () => {
       beforeEach(function () {
+        spyOn($, 'ajax').and.callFake(function () {});
         loadFixtures('merge_requests/merge_request_with_task_list.html.raw');
       });
 

spec/lib/gitlab/etag_caching/middleware_spec.rb

@@ -91,6 +91,12 @@ describe Gitlab::EtagCaching::Middleware do
       expect(status).to eq 304
     end
 
+    it 'returns empty body' do
+      _, _, body = middleware.call(build_env(path, if_none_match))
+
+      expect(body).to be_empty
+    end
+
     it 'tracks "etag_caching_cache_hit" event' do
       expect(Gitlab::Metrics).to receive(:add_event)
         .with(:etag_caching_middleware_used, endpoint: 'issue_notes')

spec/models/ci/pipeline_spec.rb

@@ -335,6 +335,14 @@ describe Ci::Pipeline, models: true do
       end
     end
 
+    describe 'pipeline ETag caching' do
+      it 'executes ExpirePipelinesCacheService' do
+        expect_any_instance_of(Ci::ExpirePipelineCacheService).to receive(:execute).with(pipeline)
+
+        pipeline.cancel
+      end
+    end
+
     def create_build(name, queued_at = current, started_from = 0)
       create(:ci_build,
              name: name,

spec/services/ci/expire_pipeline_cache_service_spec.rb

+require 'spec_helper'
+
+describe Ci::ExpirePipelineCacheService, services: true do
+  let(:user) { create(:user) }
+  let(:project) { create(:empty_project) }
+  let(:pipeline) { create(:ci_pipeline, project: project) }
+  subject { described_class.new(project, user) }
+
+  describe '#execute' do
+    it 'invalidate Etag caching for project pipelines path' do
+      pipelines_path = "/#{project.full_path}/pipelines.json"
+      new_mr_pipelines_path = "/#{project.full_path}/merge_requests/new.json"
+
+      expect_any_instance_of(Gitlab::EtagCaching::Store).to receive(:touch).with(pipelines_path)
+      expect_any_instance_of(Gitlab::EtagCaching::Store).to receive(:touch).with(new_mr_pipelines_path)
+
+      subject.execute(pipeline)
+    end
+  end
+end