Commit a306f0f4 authored by Eduardo Bonet's avatar Eduardo Bonet

Don't transform empty .ipynb diffs

Prevents displaying the transformed ipynb file diffs when we see that
after clean up no changes are left to be displayed.

Changelog: fixed
parent c3f0b38f
...@@ -125,7 +125,7 @@ class BlobPresenter < Gitlab::View::Presenter::Delegated ...@@ -125,7 +125,7 @@ class BlobPresenter < Gitlab::View::Presenter::Delegated
end end
def transformed_blob_data def transformed_blob_data
@transformed_blob ||= if blob.path.ends_with?('.ipynb') @transformed_blob ||= if blob.path.ends_with?('.ipynb') && blob.transformed_for_diff
IpynbDiff.transform(blob.data, IpynbDiff.transform(blob.data,
raise_errors: true, raise_errors: true,
options: { include_metadata: false, cell_decorator: :percent }) options: { include_metadata: false, cell_decorator: :percent })
......
...@@ -456,19 +456,27 @@ module Gitlab ...@@ -456,19 +456,27 @@ module Gitlab
from = old_blob_lazy&.data from = old_blob_lazy&.data
to = new_blob_lazy&.data to = new_blob_lazy&.data
new_diff = IpynbDiff.diff(from, to, transformed_diff = IpynbDiff.diff(from, to,
diff_opts: { context: 5, include_diff_info: true }, diff_opts: { context: 5, include_diff_info: true },
transform_options: { cell_decorator: :percent }, transform_options: { cell_decorator: :percent },
raise_if_invalid_notebook: true) raise_if_invalid_notebook: true)
new_diff = strip_diff_frontmatter(transformed_diff)
diff.diff = new_diff.scan(/.*\n/)[2..-1].join('') if new_diff
if new_diff
diff.diff = new_diff
new_blob_lazy.transformed_for_diff = true if new_blob_lazy
old_blob_lazy.transformed_for_diff = true if old_blob_lazy
end
Gitlab::AppLogger.info({ message: new_diff ? 'IPYNB_DIFF_GENERATED' : 'IPYNB_DIFF_NIL' }) Gitlab::AppLogger.info({ message: new_diff ? 'IPYNB_DIFF_GENERATED' : 'IPYNB_DIFF_NIL' })
rescue IpynbDiff::InvalidNotebookError => e rescue IpynbDiff::InvalidNotebookError => e
Gitlab::ErrorTracking.track_exception(e, issue_url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/344676') Gitlab::ErrorTracking.track_exception(e, issue_url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/344676')
end end
def strip_diff_frontmatter(diff_content)
diff_content.scan(/.*\n/)[2..-1]&.join('') if diff_content.present?
end
def alternate_viewer_class def alternate_viewer_class
return unless viewer.instance_of?(DiffViewer::Renamed) return unless viewer.instance_of?(DiffViewer::Renamed)
......
...@@ -24,7 +24,7 @@ module Gitlab ...@@ -24,7 +24,7 @@ module Gitlab
LFS_POINTER_MIN_SIZE = 120.bytes LFS_POINTER_MIN_SIZE = 120.bytes
LFS_POINTER_MAX_SIZE = 200.bytes LFS_POINTER_MAX_SIZE = 200.bytes
attr_accessor :size, :mode, :id, :commit_id, :loaded_size, :binary attr_accessor :size, :mode, :id, :commit_id, :loaded_size, :binary, :transformed_for_diff
attr_writer :name, :path, :data attr_writer :name, :path, :data
def self.gitlab_blob_truncated_true def self.gitlab_blob_truncated_true
...@@ -127,6 +127,7 @@ module Gitlab ...@@ -127,6 +127,7 @@ module Gitlab
# Retain the actual size before it is encoded # Retain the actual size before it is encoded
@loaded_size = @data.bytesize if @data @loaded_size = @data.bytesize if @data
@loaded_all_data = @loaded_size == size @loaded_all_data = @loaded_size == size
@transformed_for_diff = false
record_metric_blob_size record_metric_blob_size
record_metric_truncated(truncated?) record_metric_truncated(truncated?)
......
...@@ -52,7 +52,7 @@ RSpec.describe Gitlab::Diff::File do ...@@ -52,7 +52,7 @@ RSpec.describe Gitlab::Diff::File do
end end
describe 'initialize' do describe 'initialize' do
context 'when file is ipynb' do context 'when file is ipynb with a change after transformation' do
let(:commit) { project.commit("f6b7a707") } let(:commit) { project.commit("f6b7a707") }
let(:diff) { commit.raw_diffs.first } let(:diff) { commit.raw_diffs.first }
let(:diff_file) { described_class.new(diff, diff_refs: commit.diff_refs, repository: project.repository) } let(:diff_file) { described_class.new(diff, diff_refs: commit.diff_refs, repository: project.repository) }
...@@ -77,6 +77,20 @@ RSpec.describe Gitlab::Diff::File do ...@@ -77,6 +77,20 @@ RSpec.describe Gitlab::Diff::File do
end end
end end
end end
context 'when file is ipynb, but there only changes that are removed' do
let(:commit) { project.commit("2b5ef814") }
let(:diff) { commit.raw_diffs.first }
let(:diff_file) { described_class.new(diff, diff_refs: commit.diff_refs, repository: project.repository) }
before do
stub_feature_flags(jupyter_clean_diffs: true)
end
it 'does not recreate the diff' do
expect(diff_file.diff.diff).to include('execution_count')
end
end
end end
describe '#diff_lines' do describe '#diff_lines' do
......
...@@ -126,6 +126,10 @@ RSpec.describe BlobPresenter do ...@@ -126,6 +126,10 @@ RSpec.describe BlobPresenter do
let(:blob) { repository.blob_at('f6b7a707', 'files/ipython/markdown-table.ipynb') } let(:blob) { repository.blob_at('f6b7a707', 'files/ipython/markdown-table.ipynb') }
let(:git_blob) { blob.__getobj__ } let(:git_blob) { blob.__getobj__ }
before do
allow(git_blob).to receive(:transformed_for_diff).and_return(true)
end
it 'uses md as the transformed language' do it 'uses md as the transformed language' do
expect(Gitlab::Highlight).to receive(:highlight).with('files/ipython/markdown-table.ipynb', anything, plain: nil, language: 'md') expect(Gitlab::Highlight).to receive(:highlight).with('files/ipython/markdown-table.ipynb', anything, plain: nil, language: 'md')
......
...@@ -53,7 +53,7 @@ module TestEnv ...@@ -53,7 +53,7 @@ module TestEnv
'wip' => 'b9238ee', 'wip' => 'b9238ee',
'csv' => '3dd0896', 'csv' => '3dd0896',
'v1.1.0' => 'b83d6e3', 'v1.1.0' => 'b83d6e3',
'add-ipython-files' => 'c10c411', 'add-ipython-files' => '2b5ef814',
'add-pdf-file' => 'e774ebd', 'add-pdf-file' => 'e774ebd',
'squash-large-files' => '54cec52', 'squash-large-files' => '54cec52',
'add-pdf-text-binary' => '79faa7b', 'add-pdf-text-binary' => '79faa7b',
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment