From 3c176b58fe39e9f97329a5a42d79eaca255a3a97 Mon Sep 17 00:00:00 2001 From: Igor Drozdov <idrozdov@gitlab.com> Date: Wed, 31 Jul 2024 09:14:54 +0000 Subject: [PATCH] Fix omniauth callbacks controller tests - Fix double render for omniauth callbacks controller test - Specify keys as symbols into InheritableOptions --- Gemfile.lock | 2 +- .../active_record_schema_ignore_tables.rb | 2 +- ...agement_project_id_to_security_policies.rb | 1 - ...ent_project_id_to_approval_policy_rules.rb | 1 - .../queue_non_billable_to_billable_service.rb | 2 +- .../vulnerability_exports/export_service.rb | 4 +- .../ee/omniauth_callbacks_controller_spec.rb | 40 ++++++++++--------- .../api/entities/group_approval_rule_spec.rb | 2 +- gems/gitlab-backup-cli/Gemfile.lock | 2 +- .../gitlab-backup-cli.gemspec | 2 +- lib/tasks/gitlab/db.rake | 11 +++++ .../autocomplete_sources_controller_spec.rb | 14 +++++-- 12 files changed, 51 insertions(+), 32 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 2c04890aa1c3e..8eeb247ea1d65 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -27,7 +27,7 @@ PATH remote: gems/gitlab-backup-cli specs: gitlab-backup-cli (0.0.1) - activesupport (~> 7.0.8) + activesupport (< 7.2) rainbow (~> 3.0) thor (~> 1.3) diff --git a/config/initializers/active_record_schema_ignore_tables.rb b/config/initializers/active_record_schema_ignore_tables.rb index 55c44e00c4015..e4b55937404e6 100644 --- a/config/initializers/active_record_schema_ignore_tables.rb +++ b/config/initializers/active_record_schema_ignore_tables.rb @@ -1,4 +1,4 @@ # frozen_string_literal: true # Ignore dynamically managed partitions in static application schema -ActiveRecord::SchemaDumper.ignore_tables += ["#{Gitlab::Database::DYNAMIC_PARTITIONS_SCHEMA}.*"] +ActiveRecord::Tasks::DatabaseTasks.structure_dump_flags = ["-T", "#{Gitlab::Database::DYNAMIC_PARTITIONS_SCHEMA}.*"] diff --git a/db/migrate/20240422000001_add_security_policy_management_project_id_to_security_policies.rb b/db/migrate/20240422000001_add_security_policy_management_project_id_to_security_policies.rb index d3fb81985f568..74dc1403c38dc 100644 --- a/db/migrate/20240422000001_add_security_policy_management_project_id_to_security_policies.rb +++ b/db/migrate/20240422000001_add_security_policy_management_project_id_to_security_policies.rb @@ -12,7 +12,6 @@ def up :security_policy_management_project, index: false, null: false, - unique: false, foreign_key: { on_delete: :cascade, to_table: :projects } # rubocop:enable Migration/AddReference # rubocop:enable Rails/NotNullColumn diff --git a/db/migrate/20240422000005_add_security_policy_management_project_id_to_approval_policy_rules.rb b/db/migrate/20240422000005_add_security_policy_management_project_id_to_approval_policy_rules.rb index 18b5d506b563c..5282765853ef0 100644 --- a/db/migrate/20240422000005_add_security_policy_management_project_id_to_approval_policy_rules.rb +++ b/db/migrate/20240422000005_add_security_policy_management_project_id_to_approval_policy_rules.rb @@ -12,7 +12,6 @@ def up :security_policy_management_project, index: false, null: false, - unique: false, foreign_key: { on_delete: :cascade, to_table: :projects } # rubocop:enable Migration/AddReference # rubocop:enable Rails/NotNullColumn diff --git a/ee/app/services/gitlab_subscriptions/member_management/queue_non_billable_to_billable_service.rb b/ee/app/services/gitlab_subscriptions/member_management/queue_non_billable_to_billable_service.rb index 550c116ffb147..ad7a51e7300fc 100644 --- a/ee/app/services/gitlab_subscriptions/member_management/queue_non_billable_to_billable_service.rb +++ b/ee/app/services/gitlab_subscriptions/member_management/queue_non_billable_to_billable_service.rb @@ -72,7 +72,7 @@ def assign_users_members_and_source(users, members, source, params) end def sanitized_params - sanitized_params = params.slice(:access_level, :expires_at, :member_role_id) + sanitized_params = params.slice(:expires_at, :member_role_id).to_h sanitized_params[:access_level] = new_access_level sanitized_params[:existing_members_hash] = existing_members_hash sanitized_params[:source_namespace] = source_namespace diff --git a/ee/app/services/vulnerability_exports/export_service.rb b/ee/app/services/vulnerability_exports/export_service.rb index ea835664d3791..c7b5c124d6278 100644 --- a/ee/app/services/vulnerability_exports/export_service.rb +++ b/ee/app/services/vulnerability_exports/export_service.rb @@ -69,9 +69,9 @@ def finalise_segmented_export semaphore = Async::Semaphore.new(NUMBER_OF_CONCURRENT_TASKS) write_semaphore = Async::Semaphore.new - tasks = vulnerability_export.export_parts.map do |part| + tasks = vulnerability_export.export_parts.map(&:file).map do |file| semaphore.async do - part.file.open do |stream| + file.open do |stream| stream.readline if export_header.present? # Moves the cursor to next line stream.each_line do |line| diff --git a/ee/spec/controllers/ee/omniauth_callbacks_controller_spec.rb b/ee/spec/controllers/ee/omniauth_callbacks_controller_spec.rb index 9d11d5b3e0a75..a9816c38fb2cd 100644 --- a/ee/spec/controllers/ee/omniauth_callbacks_controller_spec.rb +++ b/ee/spec/controllers/ee/omniauth_callbacks_controller_spec.rb @@ -81,18 +81,20 @@ context 'when auth hash is missing required groups' do let(:connect_config) do - ActiveSupport::InheritableOptions.new({ - 'name' => provider, - 'args' => { - 'name' => provider, - 'client_options' => { - 'identifier' => 'gitlab-test-client', - 'gitlab' => { - 'required_groups' => ['Owls'] + ActiveSupport::InheritableOptions.new( + HashWithIndifferentAccess.new({ + name: provider, + args: { + name: provider, + client_options: { + identifier: 'gitlab-test-client', + gitlab: { + required_groups: ['Owls'] + } } } - } - }) + }) + ) end before do @@ -123,15 +125,17 @@ context 'when linking to existing profile' do let(:user) { create(:user) } let(:connect_config) do - ActiveSupport::InheritableOptions.new({ - 'name' => provider, - 'args' => { - 'name' => provider, - 'client_options' => { - 'identifier' => 'gitlab-test-client' + ActiveSupport::InheritableOptions.new( + HashWithIndifferentAccess.new({ + name: provider, + args: { + name: provider, + client_options: { + identifier: 'gitlab-test-client' + } } - } - }) + }) + ) end before do diff --git a/ee/spec/lib/ee/api/entities/group_approval_rule_spec.rb b/ee/spec/lib/ee/api/entities/group_approval_rule_spec.rb index ed7079e705673..e943a4b4eee57 100644 --- a/ee/spec/lib/ee/api/entities/group_approval_rule_spec.rb +++ b/ee/spec/lib/ee/api/entities/group_approval_rule_spec.rb @@ -5,7 +5,7 @@ RSpec.describe EE::API::Entities::GroupApprovalRule, feature_category: :source_code_management do subject(:hash) { described_class.new(approval_rule).as_json } - let(:approval_rule) { build(:approval_group_rule) } + let_it_be(:approval_rule) { create(:approval_group_rule) } it 'exposes attributes' do expect(hash.keys).to match_array(%i[ diff --git a/gems/gitlab-backup-cli/Gemfile.lock b/gems/gitlab-backup-cli/Gemfile.lock index 1395a84d354fc..c53a05343adf7 100644 --- a/gems/gitlab-backup-cli/Gemfile.lock +++ b/gems/gitlab-backup-cli/Gemfile.lock @@ -10,7 +10,7 @@ PATH remote: . specs: gitlab-backup-cli (0.0.1) - activesupport (~> 7.0.8) + activesupport (< 7.2) rainbow (~> 3.0) thor (~> 1.3) diff --git a/gems/gitlab-backup-cli/gitlab-backup-cli.gemspec b/gems/gitlab-backup-cli/gitlab-backup-cli.gemspec index 2debae549a9df..b847665f64352 100644 --- a/gems/gitlab-backup-cli/gitlab-backup-cli.gemspec +++ b/gems/gitlab-backup-cli/gitlab-backup-cli.gemspec @@ -24,7 +24,7 @@ Gem::Specification.new do |spec| spec.executables = spec.files.grep(%r{\Aexe/}) { |f| File.basename(f) } spec.require_paths = ["lib"] - spec.add_dependency "activesupport", "~> 7.0.8" + spec.add_dependency "activesupport", "< 7.2" spec.add_dependency "rainbow", "~> 3.0" spec.add_dependency "thor", "~> 1.3" diff --git a/lib/tasks/gitlab/db.rake b/lib/tasks/gitlab/db.rake index f371b430594bd..accd67159d7de 100644 --- a/lib/tasks/gitlab/db.rake +++ b/lib/tasks/gitlab/db.rake @@ -127,6 +127,8 @@ namespace :gitlab do database_name = ":#{database_name}" if database_name load_database = connection.tables.count <= 1 + ActiveRecord::Base.connection_handler.clear_all_connections!(:all) + if load_database puts "Running db:schema:load#{database_name} rake task" Gitlab::Database.add_post_migrate_path_to_rails(force: true) @@ -139,6 +141,15 @@ namespace :gitlab do load_database end + desc "Clear all connections" + task :clear_all_connections do + ActiveRecord::Base.connection_handler.clear_all_connections!(:all) + end + + ActiveRecord::Tasks::DatabaseTasks.for_each(databases) do |name| + Rake::Task["db:test:purge:#{name}"].enhance(['gitlab:db:clear_all_connections']) + end + desc 'GitLab | DB | Run database migrations and print `unattended_migrations_completed` if action taken' task unattended: :environment do no_database = !ActiveRecord::Base.connection.schema_migration.table_exists? diff --git a/spec/controllers/projects/autocomplete_sources_controller_spec.rb b/spec/controllers/projects/autocomplete_sources_controller_spec.rb index 153ff139b0e06..b33169da6808e 100644 --- a/spec/controllers/projects/autocomplete_sources_controller_spec.rb +++ b/spec/controllers/projects/autocomplete_sources_controller_spec.rb @@ -145,7 +145,7 @@ def members_by_username(username) end it 'returns an array of member object' do - get :members, format: :json, params: { namespace_id: group.path, project_id: public_project.path, type: issuable_type } + get :members, format: :json, params: { namespace_id: group.path, project_id: public_project.path, type: issuable_type, type_id: issuable_iid } expect(members_by_username('all').symbolize_keys).to include( username: 'all', @@ -175,7 +175,7 @@ def members_by_username(username) end it 'does not return the all mention user' do - get :members, format: :json, params: { namespace_id: group.path, project_id: public_project.path, type: issuable_type } + get :members, format: :json, params: { namespace_id: group.path, project_id: public_project.path, type: issuable_type, type_id: issuable_iid } expect(json_response).not_to include(a_hash_including( { username: 'all', name: 'All Project and Group Members' })) @@ -185,12 +185,14 @@ def members_by_username(username) context 'with issue' do let(:issuable_type) { issue.class.name } + let(:issuable_iid) { issue.iid } it_behaves_like 'all members are returned' end context 'with work item' do let(:issuable_type) { work_item.class.name } + let(:issuable_iid) { work_item.iid } it_behaves_like 'all members are returned' end @@ -211,7 +213,7 @@ def members_by_username(username) end it 'returns members including those from invited private groups' do - get :members, format: :json, params: { namespace_id: group.path, project_id: public_project.path, type: issuable_type } + get :members, format: :json, params: { namespace_id: group.path, project_id: public_project.path, type: issuable_type, type_id: issuable_iid } expect(members_by_username('all').symbolize_keys).to include( username: 'all', @@ -235,7 +237,7 @@ def members_by_username(username) end it 'does not return the all mention user' do - get :members, format: :json, params: { namespace_id: group.path, project_id: public_project.path, type: issuable_type } + get :members, format: :json, params: { namespace_id: group.path, project_id: public_project.path, type: issuable_type, type_id: issuable_iid } expect(json_response).not_to include(a_hash_including( { username: 'all', name: 'All Project and Group Members' })) @@ -246,20 +248,24 @@ def members_by_username(username) context 'with issue' do it_behaves_like 'private project is inaccessible' do let(:issuable_type) { private_issue.class.name } + let(:issuable_iid) { private_issue.iid } end it_behaves_like 'returns all members of public project' do let(:issuable_type) { issue.class.name } + let(:issuable_iid) { issue.iid } end end context 'with work item' do it_behaves_like 'private project is inaccessible' do let(:issuable_type) { private_work_item.class.name } + let(:issuable_iid) { private_work_item.iid } end it_behaves_like 'returns all members of public project' do let(:issuable_type) { work_item.class.name } + let(:issuable_iid) { work_item.iid } end end end -- GitLab