From 4ee2ae14f5bb4349ac4a42388321f49786501484 Mon Sep 17 00:00:00 2001 From: Steve Azzopardi Date: Tue, 21 Aug 2018 14:47:55 +0200 Subject: [PATCH 1/2] Add artifact information for job controller gitlab-org/gitlab-ce#50101 --- app/serializers/build_details_entity.rb | 22 ++++++++ ...01-add-artifact-information-to-job-api.yml | 5 ++ .../projects/jobs_controller_spec.rb | 53 +++++++++++++++++-- 3 files changed, 77 insertions(+), 3 deletions(-) create mode 100644 changelogs/unreleased/50101-add-artifact-information-to-job-api.yml diff --git a/app/serializers/build_details_entity.rb b/app/serializers/build_details_entity.rb index b887b99d31c..271ff668eda 100644 --- a/app/serializers/build_details_entity.rb +++ b/app/serializers/build_details_entity.rb @@ -9,6 +9,28 @@ class BuildDetailsEntity < JobEntity expose :metadata, using: BuildMetadataEntity + expose :artifact, if: -> (*) { can?(current_user, :read_build, build) } do + expose :download_path, if: -> (*) { build.artifacts? } do |build| + download_project_job_artifacts_path(project, build) + end + + expose :browse_path, if: -> (*) { build.browsable_artifacts? } do |build| + browse_project_job_artifacts_path(project, build) + end + + expose :keep_path, if: -> (*) { build.has_expiring_artifacts? && can?(current_user, :update_build, build) } do |build| + keep_project_job_artifacts_path(project, build) + end + + expose :expire_at, if: -> (*) { build.artifacts_expire_at.present? } do |build| + build.artifacts_expire_at + end + + expose :expired, if: -> (*) { build.artifacts_expire_at.present? } do |build| + build.artifacts_expired? + end + end + expose :erased_by, if: -> (*) { build.erased? }, using: UserEntity expose :erase_path, if: -> (*) { build.erasable? && can?(current_user, :erase_build, build) } do |build| erase_project_job_path(project, build) diff --git a/changelogs/unreleased/50101-add-artifact-information-to-job-api.yml b/changelogs/unreleased/50101-add-artifact-information-to-job-api.yml new file mode 100644 index 00000000000..f98d111a337 --- /dev/null +++ b/changelogs/unreleased/50101-add-artifact-information-to-job-api.yml @@ -0,0 +1,5 @@ +--- +title: Send artifact information in job API +merge_request: 50460 +author: +type: other diff --git a/spec/controllers/projects/jobs_controller_spec.rb b/spec/controllers/projects/jobs_controller_spec.rb index 1aca44c6e74..a0676ab7fda 100644 --- a/spec/controllers/projects/jobs_controller_spec.rb +++ b/spec/controllers/projects/jobs_controller_spec.rb @@ -135,7 +135,7 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do end end - context 'when requesting JSON' do + context 'when requesting JSON with failed job' do let(:merge_request) { create(:merge_request, source_project: project) } before do @@ -150,9 +150,56 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do it 'exposes needed information' do expect(response).to have_gitlab_http_status(:ok) expect(json_response['raw_path']).to match(%r{jobs/\d+/raw\z}) + expect(json_response['merge_request']['path']).to match(%r{merge_requests/\d+\z}) + expect(json_response['new_issue_path']).to include('/issues/new') + end + end + + context 'when request JSON for successful job' do + let(:merge_request) { create(:merge_request, source_project: project) } + let(:job) { create(:ci_build, :success, :artifacts, pipeline: pipeline) } + + before do + project.add_developer(user) + sign_in(user) + + allow_any_instance_of(Ci::Build).to receive(:merge_request).and_return(merge_request) + + get_show(id: job.id, format: :json) + end + + it 'exposes needed information' do + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['artifact']['download_path']).to match(%r{artifacts/download}) + expect(json_response['artifact']['browse_path']).to match(%r{artifacts/browse}) + expect(json_response['artifact']).not_to have_key(:expired) + expect(json_response['artifact']).not_to have_key(:expired_at) + expect(json_response['raw_path']).to match(%r{jobs/\d+/raw\z}) expect(json_response.dig('merge_request', 'path')).to match(%r{merge_requests/\d+\z}) - expect(json_response['new_issue_path']) - .to include('/issues/new') + end + + context 'when request JSON for successful job and expired artifacts' do + let(:merge_request) { create(:merge_request, source_project: project) } + let(:job) { create(:ci_build, :success, :artifacts, :expired, pipeline: pipeline) } + + before do + project.add_developer(user) + sign_in(user) + + allow_any_instance_of(Ci::Build).to receive(:merge_request).and_return(merge_request) + + get_show(id: job.id, format: :json) + end + + it 'exposes needed information' do + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['artifact']).not_to have_key(:download_path) + expect(json_response['artifact']).not_to have_key(:browse_path) + expect(json_response['artifact']['expired']).to eq(true) + expect(json_response['artifact']['expire_at']).not_to be_empty + expect(json_response['raw_path']).to match(%r{jobs/\d+/raw\z}) + expect(json_response.dig('merge_request', 'path')).to match(%r{merge_requests/\d+\z}) + end end end From 71433c5525c68bbf25f171e760e3817af7f217d4 Mon Sep 17 00:00:00 2001 From: Steve Azzopardi Date: Thu, 30 Aug 2018 19:27:04 +0200 Subject: [PATCH 2/2] Add JSON schema validation for job API Start validating against the exciting JSON schema for the job controller JSON response. The JSON schema is not complete since there is quite a lot missing, this will be done in a separate issue. --- .../projects/jobs_controller_spec.rb | 5 +++- .../api/schemas/ci_detailed_status.json | 24 ++++++++++++++++++- spec/fixtures/api/schemas/http_method.json | 5 ++++ spec/fixtures/api/schemas/job/artifact.json | 11 +++++++++ spec/fixtures/api/schemas/{ => job}/job.json | 13 +++++++--- .../fixtures/api/schemas/job/job_details.json | 7 ++++++ spec/fixtures/api/schemas/pipeline_stage.json | 2 +- 7 files changed, 61 insertions(+), 6 deletions(-) create mode 100644 spec/fixtures/api/schemas/http_method.json create mode 100644 spec/fixtures/api/schemas/job/artifact.json rename spec/fixtures/api/schemas/{ => job}/job.json (57%) create mode 100644 spec/fixtures/api/schemas/job/job_details.json diff --git a/spec/controllers/projects/jobs_controller_spec.rb b/spec/controllers/projects/jobs_controller_spec.rb index a0676ab7fda..d9499d7e207 100644 --- a/spec/controllers/projects/jobs_controller_spec.rb +++ b/spec/controllers/projects/jobs_controller_spec.rb @@ -149,6 +149,7 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do it 'exposes needed information' do expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('job/job_details') expect(json_response['raw_path']).to match(%r{jobs/\d+/raw\z}) expect(json_response['merge_request']['path']).to match(%r{merge_requests/\d+\z}) expect(json_response['new_issue_path']).to include('/issues/new') @@ -170,6 +171,7 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do it 'exposes needed information' do expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('job/job_details') expect(json_response['artifact']['download_path']).to match(%r{artifacts/download}) expect(json_response['artifact']['browse_path']).to match(%r{artifacts/browse}) expect(json_response['artifact']).not_to have_key(:expired) @@ -178,7 +180,7 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do expect(json_response.dig('merge_request', 'path')).to match(%r{merge_requests/\d+\z}) end - context 'when request JSON for successful job and expired artifacts' do + context 'when request JSON for successful job with expired artifacts' do let(:merge_request) { create(:merge_request, source_project: project) } let(:job) { create(:ci_build, :success, :artifacts, :expired, pipeline: pipeline) } @@ -193,6 +195,7 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do it 'exposes needed information' do expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('job/job_details') expect(json_response['artifact']).not_to have_key(:download_path) expect(json_response['artifact']).not_to have_key(:browse_path) expect(json_response['artifact']['expired']).to eq(true) diff --git a/spec/fixtures/api/schemas/ci_detailed_status.json b/spec/fixtures/api/schemas/ci_detailed_status.json index 01e34249bf1..d74248eabef 100644 --- a/spec/fixtures/api/schemas/ci_detailed_status.json +++ b/spec/fixtures/api/schemas/ci_detailed_status.json @@ -18,7 +18,29 @@ "tooltip": { "type": "string" }, "has_details": { "type": "boolean" }, "details_path": { "type": "string" }, - "favicon": { "type": "string" } + "favicon": { "type": "string" }, + "action": { + "type": "object", + "required": [ + "icon", + "title", + "path", + "method" + ], + "properties": { + "icon": { + "type": "string", + "enum": [ + "retry", + "play", + "cancel" + ] + }, + "title": { "type": "string" }, + "path": { "type": "string" }, + "method": { "$ref": "http_method.json" } + } + } }, "additionalProperties": false } diff --git a/spec/fixtures/api/schemas/http_method.json b/spec/fixtures/api/schemas/http_method.json new file mode 100644 index 00000000000..c0f8acc5781 --- /dev/null +++ b/spec/fixtures/api/schemas/http_method.json @@ -0,0 +1,5 @@ +{ + "type": "string", + "description": "HTTP methods that the API can specify to send.", + "enum": [ "post", "get", "put", "patch" ] +} diff --git a/spec/fixtures/api/schemas/job/artifact.json b/spec/fixtures/api/schemas/job/artifact.json new file mode 100644 index 00000000000..1812e69fbd6 --- /dev/null +++ b/spec/fixtures/api/schemas/job/artifact.json @@ -0,0 +1,11 @@ +{ + "type": "object", + "properties": { + "download_path": { "type": "string"}, + "browse_path": { "type": "string"}, + "keep_path": { "type": "string"}, + "expired": { "type": "boolean" }, + "expire_at": { "type": "string", "format": "date-time" } + }, + "additionalProperties": false +} diff --git a/spec/fixtures/api/schemas/job.json b/spec/fixtures/api/schemas/job/job.json similarity index 57% rename from spec/fixtures/api/schemas/job.json rename to spec/fixtures/api/schemas/job/job.json index 7b92ab25bc1..c793d93c0f6 100644 --- a/spec/fixtures/api/schemas/job.json +++ b/spec/fixtures/api/schemas/job/job.json @@ -1,4 +1,5 @@ { + "description": "Basic job information", "type": "object", "required": [ "id", @@ -13,12 +14,18 @@ "properties": { "id": { "type": "integer" }, "name": { "type": "string" }, - "started": { "type": "boolean" } , + "started": { + "oneOf": [ + { "type": "string", "format": "date-time" }, + { "type": "boolean" } + ] + }, "build_path": { "type": "string" }, + "retry_path": { "type": "string" }, "playable": { "type": "boolean" }, "created_at": { "type": "string" }, "updated_at": { "type": "string" }, - "status": { "$ref": "ci_detailed_status.json" } + "status": { "$ref": "../ci_detailed_status.json" } }, - "additionalProperties": false + "additionalProperties": true } diff --git a/spec/fixtures/api/schemas/job/job_details.json b/spec/fixtures/api/schemas/job/job_details.json new file mode 100644 index 00000000000..73eca83d788 --- /dev/null +++ b/spec/fixtures/api/schemas/job/job_details.json @@ -0,0 +1,7 @@ +{ + "allOf": [{ "$ref": "job.json" }], + "description": "An extension of job.json with more detailed information", + "properties": { + "artifact": { "$ref": "artifact.json" } + } +} diff --git a/spec/fixtures/api/schemas/pipeline_stage.json b/spec/fixtures/api/schemas/pipeline_stage.json index 55454200bb3..eb2667295f0 100644 --- a/spec/fixtures/api/schemas/pipeline_stage.json +++ b/spec/fixtures/api/schemas/pipeline_stage.json @@ -13,7 +13,7 @@ "groups": { "optional": true }, "latest_statuses": { "type": "array", - "items": { "$ref": "job.json" }, + "items": { "$ref": "job/job.json" }, "optional": true }, "status": { "$ref": "ci_detailed_status.json" },