Add latest changes from gitlab-org/gitlab@master

lib/gitlab/ci/trace/stream.rb

@@ -63,10 +63,6 @@ module Gitlab
           end.force_encoding(Encoding.default_external)
         end
-        def html_with_state(state = nil)
-          ::Gitlab::Ci::Ansi2html.convert(stream, state)
-        end
-
         def html(last_lines: nil)
           text = raw(last_lines: last_lines)
           buffer = StringIO.new(text)

lib/gitlab/gitaly_client/commit_service.rb

@@ -298,18 +298,6 @@ module Gitlab
         Gitlab::SafeRequestStore[key] = commit
       end
-      # rubocop: disable CodeReuse/ActiveRecord
-      def patch(revision)
-        request = Gitaly::CommitPatchRequest.new(
-          repository: @gitaly_repo,
-          revision: encode_binary(revision)
-        )
-        response = GitalyClient.call(@repository.storage, :diff_service, :commit_patch, request, timeout: GitalyClient.medium_timeout)
-
-        response.sum(&:data)
-      end
-      # rubocop: enable CodeReuse/ActiveRecord
-
       def commit_stats(revision)
         request = Gitaly::CommitStatsRequest.new(
           repository: @gitaly_repo,

spec/controllers/projects/jobs_controller_spec.rb

@@ -527,6 +527,7 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do
   describe 'GET trace.json' do
     before do
+      stub_feature_flags(job_log_json: true)
       get_trace
     end
@@ -535,8 +536,119 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do
       it 'returns a trace' do
         expect(response).to have_gitlab_http_status(:ok)
+        expect(response).to match_response_schema('job/build_trace')
         expect(json_response['id']).to eq job.id
         expect(json_response['status']).to eq job.status
+        expect(json_response['state']).to be_present
+        expect(json_response['append']).not_to be_nil
+        expect(json_response['truncated']).not_to be_nil
+        expect(json_response['size']).to be_present
+        expect(json_response['total']).to be_present
+        expect(json_response['lines'].count).to be_positive
+      end
+    end
+
+    context 'when job has a trace' do
+      let(:job) { create(:ci_build, :trace_live, pipeline: pipeline) }
+
+      it 'returns a trace' do
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(response).to match_response_schema('job/build_trace')
+        expect(json_response['id']).to eq job.id
+        expect(json_response['status']).to eq job.status
+        expect(json_response['lines']).to eq [{ 'content' => [{ 'text' => 'BUILD TRACE' }], 'offset' => 0 }]
+      end
+    end
+
+    context 'when job has no traces' do
+      let(:job) { create(:ci_build, pipeline: pipeline) }
+
+      it 'returns no traces' do
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(response).to match_response_schema('job/build_trace')
+        expect(json_response['id']).to eq job.id
+        expect(json_response['status']).to eq job.status
+        expect(json_response['lines']).to be_nil
+      end
+    end
+
+    context 'when job has a trace with ANSI sequence and Unicode' do
+      let(:job) { create(:ci_build, :unicode_trace_live, pipeline: pipeline) }
+
+      it 'returns a trace with Unicode' do
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(response).to match_response_schema('job/build_trace')
+        expect(json_response['id']).to eq job.id
+        expect(json_response['status']).to eq job.status
+        expect(json_response['lines'].flat_map {|l| l['content'].map { |c| c['text'] } }).to include("ヾ(´༎ຶД༎ຶ`)ﾉ")
+      end
+    end
+
+    context 'when trace artifact is in ObjectStorage' do
+      let(:url) { 'http://object-storage/trace' }
+      let(:file_path) { expand_fixture_path('trace/sample_trace') }
+      let!(:job) { create(:ci_build, :success, :trace_artifact, pipeline: pipeline) }
+
+      before do
+        allow_any_instance_of(JobArtifactUploader).to receive(:file_storage?) { false }
+        allow_any_instance_of(JobArtifactUploader).to receive(:url) { url }
+        allow_any_instance_of(JobArtifactUploader).to receive(:size) { File.size(file_path) }
+      end
+
+      context 'when there are no network issues' do
+        before do
+          stub_remote_url_206(url, file_path)
+
+          get_trace
+        end
+
+        it 'returns a trace' do
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response['id']).to eq job.id
+          expect(json_response['status']).to eq job.status
+          expect(json_response['lines'].count).to be_positive
+        end
+      end
+
+      context 'when there is a network issue' do
+        before do
+          stub_remote_url_500(url)
+        end
+
+        it 'returns a trace' do
+          expect { get_trace }.to raise_error(Gitlab::HttpIO::FailedToGetChunkError)
+        end
+      end
+    end
+
+    def get_trace
+      get :trace,
+        params: {
+          namespace_id: project.namespace,
+          project_id: project,
+          id: job.id
+        },
+        format: :json
+    end
+  end
+
+  describe 'GET legacy trace.json' do
+    before do
+      get_trace
+    end
+
+    context 'when job has a trace artifact' do
+      let(:job) { create(:ci_build, :trace_artifact, pipeline: pipeline) }
+
+      it 'returns a trace' do
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(json_response['id']).to eq job.id
+        expect(json_response['status']).to eq job.status
+        expect(json_response['state']).to be_present
+        expect(json_response['append']).not_to be_nil
+        expect(json_response['truncated']).not_to be_nil
+        expect(json_response['size']).to be_present
+        expect(json_response['total']).to be_present
         expect(json_response['html']).to eq(job.trace.html)
       end
     end
@@ -612,12 +724,13 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do
     end
     def get_trace
-      get :trace, params: {
-                    namespace_id: project.namespace,
-                    project_id: project,
-                    id: job.id
-                  },
-                  format: :json
+      get :trace,
+        params: {
+          namespace_id: project.namespace,
+          project_id: project,
+          id: job.id
+        },
+        format: :json
     end
   end

spec/factories/evidences.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :evidence do
+    release
+  end
+end

spec/features/projects/jobs/permissions_spec.rb

@@ -10,6 +10,7 @@ describe 'Project Jobs Permissions' do
   let!(:job) { create(:ci_build, :running, :coverage, :trace_artifact, pipeline: pipeline) }
   before do
+    stub_feature_flags(job_log_json: true)
     sign_in(user)
     project.enable_ci
@@ -69,7 +70,7 @@ describe 'Project Jobs Permissions' do
         it_behaves_like 'recent job page details responds with status', 200 do
           it 'renders job details', :js do
             expect(page).to have_content "Job ##{job.id}"
-            expect(page).to have_css '.js-build-trace'
+            expect(page).to have_css '.log-line'
           end
         end

spec/finders/todos_finder_spec.rb

@@ -16,6 +16,10 @@ describe TodosFinder do
     end
     describe '#execute' do
+      it 'returns no todos if user is nil' do
+        expect(described_class.new(nil, {}).execute).to be_empty
+      end
+
       context 'filtering' do
         let!(:todo1) { create(:todo, user: user, project: project, target: issue) }
         let!(:todo2) { create(:todo, user: user, group: group, target: merge_request) }
@@ -97,14 +101,39 @@ describe TodosFinder do
           end
         end
-        context 'with subgroups' do
-          let(:subgroup) { create(:group, parent: group) }
-          let!(:todo3) { create(:todo, user: user, group: subgroup, target: issue) }
-          it 'returns todos from subgroups when filtered by a group' do
-            todos = finder.new(user, { group_id: group.id }).execute
-            expect(todos).to match_array([todo1, todo2, todo3])
+        context 'by groups' do
+          context 'with subgroups' do
+            let(:subgroup) { create(:group, parent: group) }
+            let!(:todo3) { create(:todo, user: user, group: subgroup, target: issue) }
+
+            it 'returns todos from subgroups when filtered by a group' do
+              todos = finder.new(user, { group_id: group.id }).execute
+
+              expect(todos).to match_array([todo1, todo2, todo3])
+            end
+          end
+
+          context 'filtering for multiple groups' do
+            let_it_be(:group2) { create(:group) }
+            let_it_be(:group3) { create(:group) }
+
+            let!(:todo1) { create(:todo, user: user, project: project, target: issue) }
+            let!(:todo2) { create(:todo, user: user, group: group, target: merge_request) }
+            let!(:todo3) { create(:todo, user: user, group: group2, target: merge_request) }
+
+            let(:subgroup1) { create(:group, parent: group) }
+            let!(:todo4) { create(:todo, user: user, group: subgroup1, target: issue) }
+            let(:subgroup2) { create(:group, parent: group2) }
+            let!(:todo5) { create(:todo, user: user, group: subgroup2, target: issue) }
+            let!(:todo6) { create(:todo, user: user, group: group3, target: issue) }
+
+            it 'returns the expected groups' do
+              todos = finder.new(user, { group_id: [group.id, group2.id] }).execute
+
+              expect(todos).to match_array([todo1, todo2, todo3, todo4, todo5])
+            end
           end
         end
       end

spec/fixtures/api/schemas/evidences/author.json → spec/fixtures/api/schemas/evidences/evidence.json

 {
   "type": "object",
   "required": [
-    "id",
-    "name",
-    "email"
+    "release"
   ],
   "properties": {
-    "id": { "type": "integer" },
-    "name": { "type": "string" },
-    "email": { "type": "string" }
+    "release": { "$ref": "release.json" }
   },
   "additionalProperties": false
 }
+

spec/fixtures/api/schemas/evidences/issue.json

@@ -14,13 +14,12 @@
   "properties": {
     "id": { "type": "integer" },
     "title": { "type": "string" },
-    "description": { "type": "string" },
-    "author": { "$ref": "author.json" },
+    "description": { "type": ["string", "null"] },
     "state": { "type": "string" },
     "iid": { "type": "integer" },
     "confidential": { "type": "boolean" },
     "created_at": { "type": "date" },
-    "due_date": { "type": "date" }
+    "due_date": { "type": ["date", "null"] }
   },
   "additionalProperties": false
 }

spec/fixtures/api/schemas/evidences/milestone.json

@@ -13,11 +13,11 @@
   "properties": {
     "id": { "type": "integer" },
     "title": { "type": "string" },
-    "description": { "type": "string" },
+    "description": { "type": ["string", "null"] },
     "state": { "type": "string" },
     "iid": { "type": "integer" },
     "created_at": { "type": "date" },
-    "due_date": { "type": "date" },
+    "due_date": { "type": ["date", "null"] },
     "issues": {
       "type": "array",
       "items": { "$ref": "issue.json" }

spec/fixtures/api/schemas/evidences/project.json

@@ -9,7 +9,7 @@
   "properties": {
     "id": { "type": "integer" },
     "name": { "type": "string" },
-    "description": { "type": "string" },
+    "description": { "type": ["string", "null"] },
     "created_at": { "type": "date" }
   },
   "additionalProperties": false

spec/fixtures/api/schemas/evidences/release.json

@@ -2,7 +2,7 @@
   "type": "object",
   "required": [
     "id",
-    "tag",
+    "tag_name",
     "name",
     "description",
     "created_at",
@@ -11,8 +11,8 @@
   ],
   "properties": {
     "id": { "type": "integer" },
-    "tag": { "type": "string" },
-    "name": { "type": "string" },
+    "tag_name": { "type": "string" },
+    "name": { "type": ["string", "null"] },
     "description": { "type": "string" },
     "created_at": { "type": "date" },
     "project": { "$ref": "project.json" },

spec/fixtures/api/schemas/job/build_trace.json

+{
+  "description": "Build trace",
+  "type": "object",
+  "required": [
+    "id",
+    "status",
+    "complete",
+    "state",
+    "append",
+    "truncated",
+    "offset",
+    "size",
+    "total"
+  ],
+  "properties": {
+    "id": { "type": "integer" },
+    "status": { "type": "string" },
+    "complete": { "type": "boolean" },
+    "state": { "type": ["string", "null"] },
+    "append": { "type": ["boolean", "null"] },
+    "truncated": { "type": ["boolean", "null"] },
+    "offset": { "type": ["integer", "null"] },
+    "size": { "type": ["integer", "null"] },
+    "total": { "type": ["integer", "null"] },
+    "html": { "type": ["string", "null"] },
+    "lines": {
+      "type": ["array", "null"],
+      "items": { "$ref": "./build_trace_line.json" }
+    }
+  }
+}

spec/fixtures/api/schemas/job/build_trace_line.json

+{
+  "description": "Build trace line",
+  "type": "object",
+  "required": [
+    "offset",
+    "content"
+  ],
+  "properties": {
+    "offset": { "type": "integer" },
+    "content": {
+      "type": "array",
+      "items": { "$ref": "./build_trace_line_content.json" }
+    },
+    "section": "string",
+    "section_header": "boolean",
+    "section_duration": "string"
+  }
+}

spec/fixtures/api/schemas/job/build_trace_line_content.json

+{
+  "description": "Build trace line content",
+  "type": "object",
+  "required": [
+    "text"
+  ],
+  "properties": {
+    "text": { "type": "string" },
+    "style": { "type": "string" }
+  }
+}

spec/graphql/mutations/concerns/mutations/resolves_group_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Mutations::ResolvesGroup do
+  let(:mutation_class) do
+    Class.new(Mutations::BaseMutation) do
+      include Mutations::ResolvesGroup
+    end
+  end
+
+  let(:context) { double }
+  subject(:mutation) { mutation_class.new(object: nil, context: context) }
+
+  it 'uses the GroupsResolver to resolve groups by path' do
+    group = create(:group)
+
+    expect(Resolvers::GroupResolver).to receive(:new).with(object: nil, context: context).and_call_original
+    expect(mutation.resolve_group(full_path: group.full_path).sync).to eq(group)
+  end
+end

spec/graphql/resolvers/todo_resolver_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Resolvers::TodoResolver do
+  include GraphqlHelpers
+
+  describe '#resolve' do
+    let_it_be(:current_user) { create(:user) }
+    let_it_be(:user) { create(:user) }
+    let_it_be(:author1) { create(:user) }
+    let_it_be(:author2) { create(:user) }
+
+    let_it_be(:todo1) { create(:todo, user: user, target_type: 'MergeRequest', state: :pending, action: Todo::MENTIONED, author: author1) }
+    let_it_be(:todo2) { create(:todo, user: user, state: :done, action: Todo::ASSIGNED, author: author2) }
+    let_it_be(:todo3) { create(:todo, user: user, state: :pending, action: Todo::ASSIGNED, author: author1) }
+
+    it 'calls TodosFinder' do
+      expect_next_instance_of(TodosFinder) do |finder|
+        expect(finder).to receive(:execute)
+      end
+
+      resolve_todos
+    end
+
+    context 'when using no filter' do
+      it 'returns expected todos' do
+        todos = resolve(described_class, obj: user, args: {}, ctx: { current_user: user })
+
+        expect(todos).to contain_exactly(todo1, todo3)
+      end
+    end
+
+    context 'when using filters' do
+      # TODO These can be removed as soon as we support filtering for multiple field contents for todos
+
+      it 'just uses the first state' do
+        todos = resolve(described_class, obj: user, args: { state: [:done, :pending] }, ctx: { current_user: user })
+
+        expect(todos).to contain_exactly(todo2)
+      end
+
+      it 'just uses the first action' do
+        todos = resolve(described_class, obj: user, args: { action: [Todo::MENTIONED, Todo::ASSIGNED] }, ctx: { current_user: user })
+
+        expect(todos).to contain_exactly(todo1)
+      end
+
+      it 'just uses the first author id' do
+        # We need a pending todo for now because of TodosFinder's state query
+        todo4 = create(:todo, user: user, state: :pending, action: Todo::ASSIGNED, author: author2)
+
+        todos = resolve(described_class, obj: user, args: { author_id: [author2.id, author1.id] }, ctx: { current_user: user })
+
+        expect(todos).to contain_exactly(todo4)
+      end
+
+      it 'just uses the first project id' do
+        project1 = create(:project)
+        project2 = create(:project)
+
+        create(:todo, project: project1, user: user, state: :pending, action: Todo::ASSIGNED, author: author1)
+        todo5 = create(:todo, project: project2, user: user, state: :pending, action: Todo::ASSIGNED, author: author1)
+
+        todos = resolve(described_class, obj: user, args: { project_id: [project2.id, project1.id] }, ctx: { current_user: user })
+
+        expect(todos).to contain_exactly(todo5)
+      end
+
+      it 'just uses the first group id' do
+        group1 = create(:group)
+        group2 = create(:group)
+
+        group1.add_developer(user)
+        group2.add_developer(user)
+
+        create(:todo, group: group1, user: user, state: :pending, action: Todo::ASSIGNED, author: author1)
+        todo5 = create(:todo, group: group2, user: user, state: :pending, action: Todo::ASSIGNED, author: author1)
+
+        todos = resolve(described_class, obj: user, args: { group_id: [group2.id, group1.id] }, ctx: { current_user: user })
+
+        expect(todos).to contain_exactly(todo5)
+      end
+
+      it 'just uses the first target' do
+        todos = resolve(described_class, obj: user, args: { type: %w[Issue MergeRequest] }, ctx: { current_user: user })
+
+        # Just todo3 because todo2 is in state "done"
+        expect(todos).to contain_exactly(todo3)
+      end
+    end
+
+    context 'when no user is provided' do
+      it 'returns no todos' do
+        todos = resolve(described_class, obj: nil, args: {}, ctx: { current_user: current_user })
+
+        expect(todos).to be_empty
+      end
+    end
+
+    context 'when provided user is not current user' do
+      it 'returns no todos' do
+        todos = resolve(described_class, obj: user, args: {}, ctx: { current_user: current_user })
+
+        expect(todos).to be_empty
+      end
+    end
+  end
+
+  def resolve_todos(args = {}, context = { current_user: current_user })
+    resolve(described_class, obj: current_user, args: args, ctx: context)
+  end
+end

spec/graphql/types/query_type_spec.rb

@@ -7,7 +7,7 @@ describe GitlabSchema.types['Query'] do
     expect(described_class.graphql_name).to eq('Query')
   end
-  it { is_expected.to have_graphql_fields(:project, :namespace, :group, :echo, :metadata) }
+  it { is_expected.to have_graphql_fields(:project, :namespace, :group, :echo, :metadata, :current_user) }
   describe 'namespace field' do
     subject { described_class.fields['namespace'] }

spec/graphql/types/todo_type_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe GitlabSchema.types['Todo'] do
+  it 'has the correct fields' do
+    expected_fields = [:id, :project, :group, :author, :action, :target_type, :body, :state, :created_at]
+
+    is_expected.to have_graphql_fields(*expected_fields)
+  end
+
+  it { expect(described_class).to require_graphql_authorizations(:read_todo) }
+end

spec/lib/gitlab/ci/trace/stream_spec.rb

@@ -248,60 +248,6 @@ describe Gitlab::Ci::Trace::Stream, :clean_gitlab_redis_cache do
     end
   end
-  describe '#html_with_state' do
-    shared_examples_for 'html_with_states' do
-      it 'returns html content with state' do
-        result = stream.html_with_state
-
-        expect(result.html).to eq("<span>1234</span>")
-      end
-
-      context 'follow-up state' do
-        let!(:last_result) { stream.html_with_state }
-
-        before do
-          data_stream.seek(4, IO::SEEK_SET)
-          data_stream.write("5678")
-          stream.seek(0)
-        end
-
-        it "returns appended trace" do
-          result = stream.html_with_state(last_result.state)
-
-          expect(result.append).to be_truthy
-          expect(result.html).to eq("<span>5678</span>")
-        end
-      end
-    end
-
-    context 'when stream is StringIO' do
-      let(:data_stream) do
-        StringIO.new("1234")
-      end
-
-      let(:stream) do
-        described_class.new { data_stream }
-      end
-
-      it_behaves_like 'html_with_states'
-    end
-
-    context 'when stream is ChunkedIO' do
-      let(:data_stream) do
-        Gitlab::Ci::Trace::ChunkedIO.new(build).tap do |chunked_io|
-          chunked_io.write("1234")
-          chunked_io.seek(0, IO::SEEK_SET)
-        end
-      end
-
-      let(:stream) do
-        described_class.new { data_stream }
-      end
-
-      it_behaves_like 'html_with_states'
-    end
-  end
-
   describe '#html' do
     shared_examples_for 'htmls' do
       it "returns html" do

spec/lib/gitlab/gitaly_client/commit_service_spec.rb

@@ -252,31 +252,6 @@ describe Gitlab::GitalyClient::CommitService do
     end
   end
-  describe '#patch' do
-    let(:request) do
-      Gitaly::CommitPatchRequest.new(
-        repository: repository_message, revision: revision
-      )
-    end
-    let(:response) { [double(data: "my "), double(data: "diff")] }
-
-    subject { described_class.new(repository).patch(revision) }
-
-    it 'sends an RPC request' do
-      expect_any_instance_of(Gitaly::DiffService::Stub).to receive(:commit_patch)
-        .with(request, kind_of(Hash)).and_return([])
-
-      subject
-    end
-
-    it 'concatenates the responses data' do
-      allow_any_instance_of(Gitaly::DiffService::Stub).to receive(:commit_patch)
-        .with(request, kind_of(Hash)).and_return(response)
-
-      expect(subject).to eq("my diff")
-    end
-  end
-
   describe '#commit_stats' do
     let(:request) do
       Gitaly::CommitStatsRequest.new(