Skip to content

G
gitlab-ce
Commit 702dceb1 authored by nmilojevic1's avatar nmilojevic1
Browse files
Options

Remove feature flag for streaming serializer 

- Add chagelog for FF removal
- Remove unused specs
- Clean up Legacy code
parent 7fc6a442
Hide whitespace changes
Inline Side-by-side
Showing with 14 additions and 467 deletions
app/services/projects/import_export/export_service.rb
View file @ 702dceb1
......@@ -58,12 +58,7 @@ module Projects
end
def tree_saver_class
if ::Feature.enabled?(:streaming_serializer, project, default_enabled: true)
Gitlab::ImportExport::Project::TreeSaver
else
# Once we remove :streaming_serializer feature flag, Project::LegacyTreeSaver should be removed as well
Gitlab::ImportExport::Project::LegacyTreeSaver
end
Gitlab::ImportExport::Project::TreeSaver
end
def uploads_saver
......
changelogs/unreleased/211816-remove-streaming-serializer-feature-flag.yml 0 → 100644
View file @ 702dceb1
---
title: Use faster streaming serializer for project exports
merge_request: 28925
author:
type: performance
lib/gitlab/import_export/project/legacy_tree_saver.rb deleted 100644 → 0
View file @ 7fc6a442
# frozen_string_literal: true
module Gitlab
module ImportExport
module Project
class LegacyTreeSaver
attr_reader :full_path
def initialize(project:, current_user:, shared:, params: {})
@params = params
@project = project
@current_user = current_user
@shared = shared
@full_path = File.join(@shared.export_path, ImportExport.project_filename)
end
def save
project_tree = tree_saver.serialize(@project, reader.project_tree)
fix_project_tree(project_tree)
tree_saver.save(project_tree, @shared.export_path, ImportExport.project_filename)
true
rescue => e
@shared.error(e)
false
end
private
# Aware that the resulting hash needs to be pure-hash and
# does not include any AR objects anymore, only objects that run `.to_json`
def fix_project_tree(project_tree)
if @params[:description].present?
project_tree['description'] = @params[:description]
end
project_tree['project_members'] += group_members_array
end
def reader
@reader ||= Gitlab::ImportExport::Reader.new(shared: @shared)
end
def group_members_array
group_members.as_json(reader.group_members_tree).each do |group_member|
group_member['source_type'] = 'Project' # Make group members project members of the future import
end
end
def group_members
return [] unless @current_user.can?(:admin_group, @project.group)
# We need `.where.not(user_id: nil)` here otherwise when a group has an
# invitee, it would make the following query return 0 rows since a NULL
# user_id would be present in the subquery
# See http://stackoverflow.com/questions/129077/not-in-clause-and-null-values
non_null_user_ids = @project.project_members.where.not(user_id: nil).select(:user_id)
GroupMembersFinder.new(@project.group).execute.where.not(user_id: non_null_user_ids)
end
def tree_saver
@tree_saver ||= Gitlab::ImportExport::LegacyRelationTreeSaver.new
end
end
end
end
end
spec/features/projects/import_export/export_file_spec.rb
View file @ 702dceb1
......@@ -38,7 +38,11 @@ describe 'Import/Export - project export integration test', :js do
sign_in(user)
end
shared_examples 'export file without sensitive words' do
context "with streaming serializer" do
before do
stub_feature_flags(project_export_as_ndjson: false)
end
it 'exports a project successfully', :sidekiq_inline do
export_project_and_download_file(page, project)
......@@ -59,27 +63,8 @@ describe 'Import/Export - project export integration test', :js do
end
end
context "with legacy export" do
before do
stub_feature_flags(streaming_serializer: false)
stub_feature_flags(project_export_as_ndjson: false)
end
it_behaves_like "export file without sensitive words"
end
context "with streaming serializer" do
before do
stub_feature_flags(streaming_serializer: true)
stub_feature_flags(project_export_as_ndjson: false)
end
it_behaves_like "export file without sensitive words"
end
context "with ndjson" do
before do
stub_feature_flags(streaming_serializer: true)
stub_feature_flags(project_export_as_ndjson: true)
end
......
spec/lib/gitlab/import_export/project/legacy_tree_saver_spec.rb deleted 100644 → 0
View file @ 7fc6a442
# frozen_string_literal: true
require 'spec_helper'
describe Gitlab::ImportExport::Project::LegacyTreeSaver do
describe 'saves the project tree into a json object' do
let(:shared) { project.import_export_shared }
let(:project_tree_saver) { described_class.new(project: project, current_user: user, shared: shared) }
let(:export_path) { "#{Dir.tmpdir}/project_tree_saver_spec" }
let(:user) { create(:user) }
let!(:project) { setup_project }
before do
project.add_maintainer(user)
allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path)
allow_any_instance_of(MergeRequest).to receive(:source_branch_sha).and_return('ABCD')
allow_any_instance_of(MergeRequest).to receive(:target_branch_sha).and_return('DCBA')
end
after do
FileUtils.rm_rf(export_path)
end
it 'saves project successfully' do
expect(project_tree_saver.save).to be true
end
# It is mostly duplicated in
# `spec/lib/gitlab/import_export/fast_hash_serializer_spec.rb`
# except:
# context 'with description override' do
# context 'group members' do
# ^ These are specific for the Project::TreeSaver
context 'JSON' do
let(:saved_project_json) do
project_tree_saver.save
project_json(project_tree_saver.full_path)
end
# It is not duplicated in
# `spec/lib/gitlab/import_export/fast_hash_serializer_spec.rb`
context 'with description override' do
let(:params) { { description: 'Foo Bar' } }
let(:project_tree_saver) { described_class.new(project: project, current_user: user, shared: shared, params: params) }
it 'overrides the project description' do
expect(saved_project_json).to include({ 'description' => params[:description] })
end
end
it 'saves the correct json' do
expect(saved_project_json).to include({ 'description' => 'description', 'visibility_level' => 20 })
end
it 'has approvals_before_merge set' do
expect(saved_project_json['approvals_before_merge']).to eq(1)
end
it 'has milestones' do
expect(saved_project_json['milestones']).not_to be_empty
end
it 'has merge requests' do
expect(saved_project_json['merge_requests']).not_to be_empty
end
it 'has merge request\'s milestones' do
expect(saved_project_json['merge_requests'].first['milestone']).not_to be_empty
end
it 'has merge request\'s source branch SHA' do
expect(saved_project_json['merge_requests'].first['source_branch_sha']).to eq('ABCD')
end
it 'has merge request\'s target branch SHA' do
expect(saved_project_json['merge_requests'].first['target_branch_sha']).to eq('DCBA')
end
it 'has events' do
expect(saved_project_json['merge_requests'].first['milestone']['events']).not_to be_empty
end
it 'has snippets' do
expect(saved_project_json['snippets']).not_to be_empty
end
it 'has snippet notes' do
expect(saved_project_json['snippets'].first['notes']).not_to be_empty
end
it 'has releases' do
expect(saved_project_json['releases']).not_to be_empty
end
it 'has no author on releases' do
expect(saved_project_json['releases'].first['author']).to be_nil
end
it 'has the author ID on releases' do
expect(saved_project_json['releases'].first['author_id']).not_to be_nil
end
it 'has issues' do
expect(saved_project_json['issues']).not_to be_empty
end
it 'has issue comments' do
notes = saved_project_json['issues'].first['notes']
expect(notes).not_to be_empty
expect(notes.first['type']).to eq('DiscussionNote')
end
it 'has issue assignees' do
expect(saved_project_json['issues'].first['issue_assignees']).not_to be_empty
end
it 'has author on issue comments' do
expect(saved_project_json['issues'].first['notes'].first['author']).not_to be_empty
end
it 'has project members' do
expect(saved_project_json['project_members']).not_to be_empty
end
it 'has merge requests diffs' do
expect(saved_project_json['merge_requests'].first['merge_request_diff']).not_to be_empty
end
it 'has merge request diff files' do
expect(saved_project_json['merge_requests'].first['merge_request_diff']['merge_request_diff_files']).not_to be_empty
end
it 'has merge request diff commits' do
expect(saved_project_json['merge_requests'].first['merge_request_diff']['merge_request_diff_commits']).not_to be_empty
end
it 'has merge requests comments' do
expect(saved_project_json['merge_requests'].first['notes']).not_to be_empty
end
it 'has author on merge requests comments' do
expect(saved_project_json['merge_requests'].first['notes'].first['author']).not_to be_empty
end
it 'has pipeline stages' do
expect(saved_project_json.dig('ci_pipelines', 0, 'stages')).not_to be_empty
end
it 'has pipeline statuses' do
expect(saved_project_json.dig('ci_pipelines', 0, 'stages', 0, 'statuses')).not_to be_empty
end
it 'has pipeline builds' do
builds_count = saved_project_json
.dig('ci_pipelines', 0, 'stages', 0, 'statuses')
.count { |hash| hash['type'] == 'Ci::Build' }
expect(builds_count).to eq(1)
end
it 'has no when YML attributes but only the DB column' do
expect_any_instance_of(Gitlab::Ci::YamlProcessor).not_to receive(:build_attributes)
saved_project_json
end
it 'has pipeline commits' do
expect(saved_project_json['ci_pipelines']).not_to be_empty
end
it 'has ci pipeline notes' do
expect(saved_project_json['ci_pipelines'].first['notes']).not_to be_empty
end
it 'has labels with no associations' do
expect(saved_project_json['labels']).not_to be_empty
end
it 'has labels associated to records' do
expect(saved_project_json['issues'].first['label_links'].first['label']).not_to be_empty
end
it 'has project and group labels' do
label_types = saved_project_json['issues'].first['label_links'].map { |link| link['label']['type'] }
expect(label_types).to match_array(%w(ProjectLabel GroupLabel))
end
it 'has priorities associated to labels' do
priorities = saved_project_json['issues'].first['label_links'].flat_map { |link| link['label']['priorities'] }
expect(priorities).not_to be_empty
end
it 'has issue resource label events' do
expect(saved_project_json['issues'].first['resource_label_events']).not_to be_empty
end
it 'has merge request resource label events' do
expect(saved_project_json['merge_requests'].first['resource_label_events']).not_to be_empty
end
it 'saves the correct service type' do
expect(saved_project_json['services'].first['type']).to eq('CustomIssueTrackerService')
end
it 'saves the properties for a service' do
expect(saved_project_json['services'].first['properties']).to eq('one' => 'value')
end
it 'has project feature' do
project_feature = saved_project_json['project_feature']
expect(project_feature).not_to be_empty
expect(project_feature["issues_access_level"]).to eq(ProjectFeature::DISABLED)
expect(project_feature["wiki_access_level"]).to eq(ProjectFeature::ENABLED)
expect(project_feature["builds_access_level"]).to eq(ProjectFeature::PRIVATE)
end
it 'has custom attributes' do
expect(saved_project_json['custom_attributes'].count).to eq(2)
end
it 'has badges' do
expect(saved_project_json['project_badges'].count).to eq(2)
end
it 'does not complain about non UTF-8 characters in MR diff files' do
ActiveRecord::Base.connection.execute("UPDATE merge_request_diff_files SET diff = '---\n- :diff: !binary |-\n LS0tIC9kZXYvbnVsbAorKysgYi9pbWFnZXMvbnVjb3IucGRmCkBAIC0wLDAg\n KzEsMTY3OSBAQAorJVBERi0xLjUNJeLjz9MNCisxIDAgb2JqDTw8L01ldGFk\n YXR'")
expect(project_tree_saver.save).to be true
end
context 'group members' do
let(:user2) { create(:user, email: 'group@member.com') }
let(:member_emails) do
saved_project_json['project_members'].map do |pm|
pm['user']['email']
end
end
before do
Group.first.add_developer(user2)
end
it 'does not export group members if it has no permission' do
Group.first.add_developer(user)
expect(member_emails).not_to include('group@member.com')
end
it 'does not export group members as maintainer' do
Group.first.add_maintainer(user)
expect(member_emails).not_to include('group@member.com')
end
it 'exports group members as group owner' do
Group.first.add_owner(user)
expect(member_emails).to include('group@member.com')
end
context 'as admin' do
let(:user) { create(:admin) }
it 'exports group members as admin' do
expect(member_emails).to include('group@member.com')
end
it 'exports group members as project members' do
member_types = saved_project_json['project_members'].map { |pm| pm['source_type'] }
expect(member_types).to all(eq('Project'))
end
end
end
context 'project attributes' do
it 'does not contain the runners token' do
expect(saved_project_json).not_to include("runners_token" => 'token')
end
end
it 'has a board and a list' do
expect(saved_project_json['boards'].first['lists']).not_to be_empty
end
end
end
def setup_project
release = create(:release)
group = create(:group)
project = create(:project,
:public,
:repository,
:issues_disabled,
:wiki_enabled,
:builds_private,
description: 'description',
releases: [release],
group: group,
approvals_before_merge: 1
)
allow(project).to receive(:commit).and_return(Commit.new(RepoHelpers.sample_commit, project))
issue = create(:issue, assignees: [user], project: project)
snippet = create(:project_snippet, project: project)
project_label = create(:label, project: project)
group_label = create(:group_label, group: group)
create(:label_link, label: project_label, target: issue)
create(:label_link, label: group_label, target: issue)
create(:label_priority, label: group_label, priority: 1)
milestone = create(:milestone, project: project)
merge_request = create(:merge_request, source_project: project, milestone: milestone)
ci_build = create(:ci_build, project: project, when: nil)
ci_build.pipeline.update(project: project)
create(:commit_status, project: project, pipeline: ci_build.pipeline)
create(:milestone, project: project)
create(:discussion_note, noteable: issue, project: project)
create(:note, noteable: merge_request, project: project)
create(:note, noteable: snippet, project: project)
create(:note_on_commit,
author: user,
project: project,
commit_id: ci_build.pipeline.sha)
create(:resource_label_event, label: project_label, issue: issue)
create(:resource_label_event, label: group_label, merge_request: merge_request)
create(:event, :created, target: milestone, project: project, author: user)
create(:service, project: project, type: 'CustomIssueTrackerService', category: 'issue_tracker', properties: { one: 'value' })
create(:project_custom_attribute, project: project)
create(:project_custom_attribute, project: project)
create(:project_badge, project: project)
create(:project_badge, project: project)
board = create(:board, project: project, name: 'TestBoard')
create(:list, board: board, position: 0, label: project_label)
project
end
def project_json(filename)
::JSON.parse(IO.read(filename))
end
end
spec/services/projects/import_export/export_service_spec.rb
View file @ 702dceb1
......@@ -26,28 +26,10 @@ describe Projects::ImportExport::ExportService do
service.execute
end
context 'when :streaming_serializer feature is enabled' do
before do
stub_feature_flags(streaming_serializer: true)
end
it 'saves the models' do
expect(Gitlab::ImportExport::Project::TreeSaver).to receive(:new).and_call_original
service.execute
end
end
it 'saves the models' do
expect(Gitlab::ImportExport::Project::TreeSaver).to receive(:new).and_call_original
context 'when :streaming_serializer feature is disabled' do
before do
stub_feature_flags(streaming_serializer: false)
end
it 'saves the models' do
expect(Gitlab::ImportExport::Project::LegacyTreeSaver).to receive(:new).and_call_original
service.execute
end
service.execute
end
it 'saves the uploads' do
......
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7