Merge branch 'osw-diffs-metadata-endpoint' into 'master'

Introduce a lightweight diffs_metadata endpoint

See merge request gitlab-org/gitlab!18104

app/controllers/projects/merge_requests/diffs_controller.rb

@@ -7,7 +7,7 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic
   before_action :apply_diff_view_cookie!
   before_action :commit, except: :diffs_batch
   before_action :define_diff_vars, except: :diffs_batch
-  before_action :define_diff_comment_vars, except: :diffs_batch
+  before_action :define_diff_comment_vars, except: [:diffs_batch, :diffs_metadata]
 
   def show
     render_diffs
@@ -37,6 +37,11 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic
     render json: PaginatedDiffSerializer.new(current_user: current_user).represent(diffs, options)
   end
 
+  def diffs_metadata
+    render json: DiffsMetadataSerializer.new(project: @merge_request.project)
+                   .represent(@diffs, additional_attributes)
+  end
+
   private
 
   def preloadable_mr_relations

app/helpers/diff_helper.rb

@@ -203,8 +203,8 @@ module DiffHelper
     link_to "#{hide_whitespace? ? 'Show' : 'Hide'} whitespace changes", url, class: options[:class]
   end
 
-  def render_overflow_warning?(diff_files)
-    diffs = @merge_request_diff.presence || diff_files
+  def render_overflow_warning?(diffs_collection)
+    diffs = @merge_request_diff.presence || diffs_collection.diff_files
 
     diffs.overflow?
   end

app/serializers/diff_file_metadata_entity.rb

+# frozen_string_literal: true
+
+class DiffFileMetadataEntity < Grape::Entity
+  expose :added_lines
+  expose :removed_lines
+  expose :new_path
+  expose :old_path
+  expose :new_file?, as: :new_file
+  expose :deleted_file?, as: :deleted_file
+end

app/serializers/diffs_entity.rb

@@ -53,7 +53,7 @@ class DiffsEntity < Grape::Entity
   # rubocop: enable CodeReuse/ActiveRecord
 
   expose :render_overflow_warning do |diffs|
-    render_overflow_warning?(diffs.diff_files)
+    render_overflow_warning?(diffs)
   end
 
   expose :email_patch_path, if: -> (*) { merge_request } do |diffs|

app/serializers/diffs_metadata_entity.rb

+# frozen_string_literal: true
+
+class DiffsMetadataEntity < DiffsEntity
+  unexpose :diff_files
+  expose :diff_files, using: DiffFileMetadataEntity
+end

app/serializers/diffs_metadata_serializer.rb

+# frozen_string_literal: true
+
+class DiffsMetadataSerializer < BaseSerializer
+  entity DiffsMetadataEntity
+end

app/views/projects/diffs/_diffs.html.haml

@@ -21,7 +21,7 @@
         = parallel_diff_btn
     = render 'projects/diffs/stats', diff_files: diff_files
 
-- if render_overflow_warning?(diff_files)
+- if render_overflow_warning?(diffs)
   = render 'projects/diffs/warning', diff_files: diffs
 
 .files{ data: { can_create_note: can_create_note } }

changelogs/unreleased/osw-diffs-metadata-endpoint.yml

+---
+title: Introduce a lightweight diffs_metadata endpoint
+merge_request: 18104
+author:
+type: added

config/routes/project.rb

@@ -282,6 +282,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
             get :pipelines
             get :diffs, to: 'merge_requests/diffs#show'
             get :diffs_batch, to: 'merge_requests/diffs#diffs_batch'
+            get :diffs_metadata, to: 'merge_requests/diffs#diffs_metadata'
             get :widget, to: 'merge_requests/content#widget'
             get :cached_widget, to: 'merge_requests/content#cached_widget'
           end

spec/controllers/projects/merge_requests/diffs_controller_spec.rb

@@ -100,6 +100,136 @@ describe Projects::MergeRequests::DiffsController do
     it_behaves_like 'persisted preferred diff view cookie'
   end
 
+  describe 'GET diffs_metadata' do
+    def go(extra_params = {})
+      params = {
+        namespace_id: project.namespace.to_param,
+        project_id: project,
+        id: merge_request.iid,
+        format: 'json'
+      }
+
+      get :diffs_metadata, params: params.merge(extra_params)
+    end
+
+    context 'when not authorized' do
+      let(:another_user) { create(:user) }
+
+      before do
+        sign_in(another_user)
+      end
+
+      it 'returns 404 when not a member' do
+        go
+
+        expect(response).to have_gitlab_http_status(404)
+      end
+
+      it 'returns 404 when visibility level is not enough' do
+        project.add_guest(another_user)
+
+        go
+
+        expect(response).to have_gitlab_http_status(404)
+      end
+    end
+
+    context 'when diffable does not exists' do
+      it 'returns 404' do
+        go(diff_id: 9999)
+
+        expect(response).to have_gitlab_http_status(404)
+      end
+    end
+
+    context 'with valid diff_id' do
+      it 'returns success' do
+        go(diff_id: merge_request.merge_request_diff.id)
+
+        expect(response).to have_gitlab_http_status(200)
+      end
+
+      it 'serializes diffs metadata with expected arguments' do
+        expected_options = {
+          environment: nil,
+          merge_request: merge_request,
+          merge_request_diff: merge_request.merge_request_diff,
+          merge_request_diffs: merge_request.merge_request_diffs,
+          start_version: nil,
+          start_sha: nil,
+          commit: nil,
+          latest_diff: true
+        }
+
+        expect_next_instance_of(DiffsMetadataSerializer) do |instance|
+          expect(instance).to receive(:represent)
+            .with(an_instance_of(Gitlab::Diff::FileCollection::MergeRequestDiff), expected_options)
+            .and_call_original
+        end
+
+        go(diff_id: merge_request.merge_request_diff.id)
+      end
+    end
+
+    context 'with MR regular diff params' do
+      it 'returns success' do
+        go
+
+        expect(response).to have_gitlab_http_status(200)
+      end
+
+      it 'serializes diffs metadata with expected arguments' do
+        expected_options = {
+          environment: nil,
+          merge_request: merge_request,
+          merge_request_diff: merge_request.merge_request_diff,
+          merge_request_diffs: merge_request.merge_request_diffs,
+          start_version: nil,
+          start_sha: nil,
+          commit: nil,
+          latest_diff: true
+        }
+
+        expect_next_instance_of(DiffsMetadataSerializer) do |instance|
+          expect(instance).to receive(:represent)
+            .with(an_instance_of(Gitlab::Diff::FileCollection::MergeRequestDiff), expected_options)
+            .and_call_original
+        end
+
+        go
+      end
+    end
+
+    context 'with commit param' do
+      it 'returns success' do
+        go(commit_id: merge_request.diff_head_sha)
+
+        expect(response).to have_gitlab_http_status(200)
+      end
+
+      it 'serializes diffs metadata with expected arguments' do
+        expected_options = {
+          environment: nil,
+          merge_request: merge_request,
+          merge_request_diff: nil,
+          merge_request_diffs: merge_request.merge_request_diffs,
+          start_version: nil,
+          start_sha: nil,
+          commit: merge_request.diff_head_commit,
+          latest_diff: nil
+        }
+
+        expect_next_instance_of(DiffsMetadataSerializer) do |instance|
+          expect(instance).to receive(:represent)
+            .with(an_instance_of(Gitlab::Diff::FileCollection::Commit), expected_options)
+            .and_call_original
+        end
+
+        go(commit_id: merge_request.diff_head_sha)
+      end
+    end
+  end
+
   describe 'GET diff_for_path' do
     def diff_for_path(extra_params = {})
       params = {

spec/serializers/diffs_metadata_entity_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe DiffsMetadataEntity do
+  let(:user) { create(:user) }
+  let(:project) { create(:project, :repository) }
+  let(:request) { EntityRequest.new(project: project, current_user: user) }
+  let(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: project) }
+  let(:merge_request_diffs) { merge_request.merge_request_diffs }
+  let(:merge_request_diff) { merge_request_diffs.last }
+
+  let(:entity) do
+    described_class.new(merge_request_diff.diffs,
+                        request: request,
+                        merge_request: merge_request,
+                        merge_request_diffs: merge_request_diffs)
+  end
+
+  context 'as json' do
+    subject { entity.as_json }
+
+    it 'contain only required attributes' do
+      expect(subject.keys).to contain_exactly(
+        # Inherited attributes
+        :real_size, :size, :branch_name,
+        :target_branch_name, :commit, :merge_request_diff,
+        :start_version, :latest_diff, :latest_version_path,
+        :added_lines, :removed_lines, :render_overflow_warning,
+        :email_patch_path, :plain_diff_path,
+        :merge_request_diffs,
+        # Attributes
+        :diff_files
+      )
+    end
+
+    describe 'diff_files' do
+      it 'returns diff files metadata' do
+        payload =
+          DiffFileMetadataEntity.represent(merge_request_diff.diffs.diff_files).as_json
+
+        expect(subject[:diff_files]).to eq(payload)
+      end
+    end
+  end
+end