From 47a0276e53de4635df43124607ac1a101d6f1b70 Mon Sep 17 00:00:00 2001 From: "Z.J. van de Weg" Date: Tue, 23 May 2017 17:10:07 +0200 Subject: [PATCH] Initial implementation for real time job view Added the needed keys and paths to a new entity, BuildDetailsEntity. Not renaming BuildEntity to BuildBasicEntity on explicit request. Most code now has test coverage, but not all. This will be added on later commits on this branch. Resolves gitlab-org/gitlab-ce#31397 --- app/controllers/projects/jobs_controller.rb | 11 +++++ app/models/ci/build.rb | 17 ++++--- app/serializers/build_details_entity.rb | 34 ++++++++++++++ app/serializers/build_serializer.rb | 4 +- app/serializers/runner_entity.rb | 3 ++ .../unreleased/zj-job-view-goes-real-time.yml | 4 ++ lib/gitlab/etag_caching/router.rb | 8 +++- .../projects/jobs_controller_spec.rb | 44 ++++++++++++++----- spec/lib/gitlab/etag_caching/router_spec.rb | 11 +++++ spec/serializers/build_details_entity_spec.rb | 39 ++++++++++++++++ spec/serializers/runner_entity_spec.rb | 14 ++++++ 11 files changed, 169 insertions(+), 20 deletions(-) create mode 100644 app/serializers/build_details_entity.rb create mode 100644 app/serializers/runner_entity.rb create mode 100644 changelogs/unreleased/zj-job-view-goes-real-time.yml create mode 100644 spec/serializers/build_details_entity_spec.rb create mode 100644 spec/serializers/runner_entity_spec.rb diff --git a/app/controllers/projects/jobs_controller.rb b/app/controllers/projects/jobs_controller.rb index d2cd1cfdab8..9b8a9eedfef 100644 --- a/app/controllers/projects/jobs_controller.rb +++ b/app/controllers/projects/jobs_controller.rb @@ -45,6 +45,17 @@ class Projects::JobsController < Projects::ApplicationController @builds = @project.pipelines.find_by_sha(@build.sha).builds.order('id DESC') @builds = @builds.where("id not in (?)", @build.id) @pipeline = @build.pipeline + + respond_to do |format| + format.html + format.json do + Gitlab::PollingInterval.set_header(response, interval: 10_000) + + render json: BuildSerializer + .new(project: @project, current_user: @current_user) + .represent_status(@build, {}, BuildDetailsEntity) + end + end end def trace diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 60b71ff0d93..ff6860bc181 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -204,14 +204,17 @@ module Ci end def merge_request - merge_requests = MergeRequest.includes(:merge_request_diff) - .where(source_branch: ref, - source_project: pipeline.project) - .reorder(iid: :asc) + @merge_request ||= + begin + merge_requests = MergeRequest.includes(:merge_request_diff) + .where(source_branch: ref, + source_project: pipeline.project) + .reorder(iid: :asc) - merge_requests.find do |merge_request| - merge_request.commits_sha.include?(pipeline.sha) - end + merge_requests.find do |merge_request| + merge_request.commits_sha.include?(pipeline.sha) + end + end end def repo_url diff --git a/app/serializers/build_details_entity.rb b/app/serializers/build_details_entity.rb new file mode 100644 index 00000000000..08c5e69180c --- /dev/null +++ b/app/serializers/build_details_entity.rb @@ -0,0 +1,34 @@ +class BuildDetailsEntity < BuildEntity + expose :coverage, :erased_at, :duration + expose :tag_list, as: :tags + + expose :artifacts, using: BuildArtifactEntity + expose :runner, using: RunnerEntity + expose :pipeline, using: PipelineEntity + + expose :merge_request_path do |build| + merge_request = build.merge_request + project = build.project + + if merge_request.nil? || !can?(request.current_user, :read_merge_request, project) + nil + else + namespace_project_merge_request_path(project.namespace, project, merge_request) + end + end + + expose :new_issue_path do |build| + project = build.project + + unless build.failed? && can?(request.current_user, :create_issue, project) + nil + else + new_namespace_project_issue_path(project.namespace, project) + end + end + + expose :raw_path do |build| + project = build.project + raw_namespace_project_build_path(project.namespace, project, build) + end +end diff --git a/app/serializers/build_serializer.rb b/app/serializers/build_serializer.rb index 79b67001199..7b501a52604 100644 --- a/app/serializers/build_serializer.rb +++ b/app/serializers/build_serializer.rb @@ -1,8 +1,10 @@ class BuildSerializer < BaseSerializer entity BuildEntity - def represent_status(resource) + def represent_status(resource, opts = {}, entity_class = nil) data = represent(resource, { only: [:status] }) data.fetch(:status, {}) + + represent(resource, opts, entity_class) end end diff --git a/app/serializers/runner_entity.rb b/app/serializers/runner_entity.rb new file mode 100644 index 00000000000..77d6de6e84c --- /dev/null +++ b/app/serializers/runner_entity.rb @@ -0,0 +1,3 @@ +class RunnerEntity < Grape::Entity + expose :id, :name, :description +end diff --git a/changelogs/unreleased/zj-job-view-goes-real-time.yml b/changelogs/unreleased/zj-job-view-goes-real-time.yml new file mode 100644 index 00000000000..376c9dfa65f --- /dev/null +++ b/changelogs/unreleased/zj-job-view-goes-real-time.yml @@ -0,0 +1,4 @@ +--- +title: Job details page update real time +merge_request: 11651 +author: diff --git a/lib/gitlab/etag_caching/router.rb b/lib/gitlab/etag_caching/router.rb index d137cc1bae6..53f3f442bc3 100644 --- a/lib/gitlab/etag_caching/router.rb +++ b/lib/gitlab/etag_caching/router.rb @@ -9,7 +9,9 @@ module Gitlab # - Ending in `noteable/issue//notes` for the `issue_notes` route # - Ending in `issues/id`/realtime_changes` for the `issue_title` route USED_IN_ROUTES = %w[noteable issue notes issues realtime_changes - commit pipelines merge_requests new].freeze + commit pipelines merge_requests builds + new].freeze + RESERVED_WORDS = Gitlab::PathRegex::ILLEGAL_PROJECT_PATH_WORDS - USED_IN_ROUTES RESERVED_WORDS_REGEX = Regexp.union(*RESERVED_WORDS.map(&Regexp.method(:escape))) ROUTES = [ @@ -40,6 +42,10 @@ module Gitlab Gitlab::EtagCaching::Router::Route.new( %r(^(?!.*(#{RESERVED_WORDS_REGEX})).*/pipelines/\d+\.json\z), 'project_pipeline' + ), + Gitlab::EtagCaching::Router::Route.new( + %r(^(?!.*(#{RESERVED_WORDS_REGEX})).*/builds/\d+\.json\z), + 'project_build' ) ].freeze diff --git a/spec/controllers/projects/jobs_controller_spec.rb b/spec/controllers/projects/jobs_controller_spec.rb index 838bdae1445..7109310b6df 100644 --- a/spec/controllers/projects/jobs_controller_spec.rb +++ b/spec/controllers/projects/jobs_controller_spec.rb @@ -101,26 +101,48 @@ describe Projects::JobsController do end describe 'GET show' do - context 'when build exists' do - let!(:build) { create(:ci_build, pipeline: pipeline) } + let!(:build) { create(:ci_build, :failed, pipeline: pipeline) } - before do - get_show(id: build.id) + context 'when requesting HTML' do + context 'when build exists' do + before do + get_show(id: build.id) + end + + it 'has a build' do + expect(response).to have_http_status(:ok) + expect(assigns(:build).id).to eq(build.id) + end end - it 'has a build' do - expect(response).to have_http_status(:ok) - expect(assigns(:build).id).to eq(build.id) + context 'when build does not exist' do + before do + get_show(id: 1234) + end + + it 'renders not_found' do + expect(response).to have_http_status(:not_found) + end end end - context 'when build does not exist' do + context 'when requesting JSON' do + let(:merge_request) { create(:merge_request, source_project: project) } + before do - get_show(id: 1234) + project.add_developer(user) + sign_in(user) + + allow_any_instance_of(Ci::Build).to receive(:merge_request).and_return(merge_request) + + get_show(id: build.id, format: :json) end - it 'renders not_found' do - expect(response).to have_http_status(:not_found) + it 'exposes needed information' do + expect(response).to have_http_status(:ok) + expect(json_response['new_issue_path']).to end_with('/issues/new') + expect(json_response['raw_path']).to match(/builds\/\d+\/raw\z/) + expect(json_response['merge_request_path']).to match(/merge_requests\/\d+\z/) end end diff --git a/spec/lib/gitlab/etag_caching/router_spec.rb b/spec/lib/gitlab/etag_caching/router_spec.rb index 46a238b17f4..22494f4dcf5 100644 --- a/spec/lib/gitlab/etag_caching/router_spec.rb +++ b/spec/lib/gitlab/etag_caching/router_spec.rb @@ -67,6 +67,17 @@ describe Gitlab::EtagCaching::Router do expect(result.name).to eq 'merge_request_pipelines' end + it 'matches build endpoint' do + env = build_env( + '/my-group/my-project/builds/234.json' + ) + + result = described_class.match(env) + + expect(result).to be_present + expect(result.name).to eq 'project_build' + end + it 'does not match blob with confusing name' do env = build_env( '/my-group/my-project/blob/master/pipelines.json' diff --git a/spec/serializers/build_details_entity_spec.rb b/spec/serializers/build_details_entity_spec.rb new file mode 100644 index 00000000000..4b827a0994c --- /dev/null +++ b/spec/serializers/build_details_entity_spec.rb @@ -0,0 +1,39 @@ +require 'spec_helper' + +describe BuildDetailsEntity do + it 'inherits from BuildEntity' do + expect(described_class).to be < BuildEntity + end + + describe '#as_json' do + let(:project) { create(:project, :repository) } + let(:user) { create(:user) } + let!(:build) { create(:ci_build, :failed, project: project) } + let(:request) { double('request') } + let(:entity) { described_class.new(build, request: request, current_user: user, project: project) } + subject { entity.as_json } + + before do + allow(request).to receive(:current_user).and_return(user) + + project.add_master(user) + end + + context 'when the user has access to issues and merge requests' do + let!(:merge_request) { create(:merge_request, source_project: project) } + + it 'contains the needed key value pairs' do + expect(subject).to include(:coverage, :erased_at, :duration) + expect(subject).to include(:artifacts, :runner, :pipeline) + expect(subject).to include(:raw_path, :merge_request_path, :new_issue_path) + end + end + + context 'when the user can only read the build' do + it "won't display the paths to issues and merge requests" do + expect(subject['new_issue_path']).to be_nil + expect(subject['merge_request_path']).to be_nil + end + end + end +end diff --git a/spec/serializers/runner_entity_spec.rb b/spec/serializers/runner_entity_spec.rb new file mode 100644 index 00000000000..eb662a10b7e --- /dev/null +++ b/spec/serializers/runner_entity_spec.rb @@ -0,0 +1,14 @@ +require 'spec_helper' + +describe RunnerEntity do + let(:runner) { build(:ci_runner) } + let(:entity) { described_class.represent(runner) } + + describe '#as_json' do + subject { entity.as_json } + + it 'contains required fields' do + expect(subject).to include(:id, :name, :description) + end + end +end