aas/additionals
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

fix some n+1 problems, use bulk for migration and refactored Gemfile for testing

Browse Source
This commit is contained in:
Alexander Meindl 2022-01-13 18:03:29 +01:00
parent 02814beb0f
commit 2ed2716944
16 changed files with 157 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
.github/workflows/linters.yml vendored
View File

@ -12,9 +12,13 @@ jobs:
steps:
- uses: actions/checkout@v1
- name: Install package dependencies
run: |
sudo apt-get install --yes --quiet pandoc
- name: Setup Gemfile
run: |
touch .enable_dev
touch .enable_linters
sed -i "3isource 'https://rubygems.org'" Gemfile
- name: Setup Ruby

5
.github/workflows/tests.yml vendored
View File

@ -81,6 +81,10 @@ jobs:
libpq-dev
libmysqlclient-dev
- name: Setup Gemfile
run: |
touch .enable_test
- name: Setup Ruby
uses: ruby/setup-ruby@v1
with:
@ -92,6 +96,7 @@ jobs:
run: |
cp plugins/additionals/test/support/database-${{ matrix.db }}.yml config/database.yml
cp plugins/additionals/test/support/configuration.yml config/configuration.yml
cp plugins/additionals/test/support/additional_environment.rb config/additional_environment.rb
- name: Install Ruby dependencies
working-directory: redmine

2
.gitignore vendored
View File

@ -11,6 +11,6 @@ Gemfile.lock
docs/_build
docs/_static
docs/_templates
.enable_dev
.enable_*
/node_modules
/yarn.lock

6
.rubocop.yml
View File

@ -94,6 +94,12 @@ Style/OptionHash:
Exclude:
- lib/additionals/patches/*.rb
# postgresql and mysql are supported
# autodetect does not work without database configuration
Rails/BulkChangeTable:
Enabled: true
Database: postgresql
Style/ReturnNil:
Enabled: true

48
Gemfile
View File

@ -3,13 +3,32 @@
# Specify your gem's dependencies in additionals.gemspec
gemspec
group :development do
# this is only used for development.
# if you want to use it, do:
# - create .enable_dev file in additionals directory
# - remove rubocop entries from REDMINE/Gemfile
# - remove REDMINE/.rubocop* files
if File.file? File.expand_path './.enable_dev', __dir__
# this is only used for local development.
# if you want to use it, do:
# - create .enable_dev file in additionals directory
# (do not use for production!)
# (this is used to not create conflicts with other plugins)
if File.file? File.expand_path './.enable_dev', __dir__
group :development, :test do
gem 'awesome_print', require: 'ap'
gem 'better_errors'
gem 'binding_of_caller'
gem 'debug'
gem 'marginalia'
gem 'memory_profiler'
gem 'solargraph'
end
end
# if you want to use it for linters, do:
# - create .enable_test file in additionals directory
# - remove rubocop entries from REDMINE/Gemfile
# - remove REDMINE/.rubocop* files
# - create .enable_linters file in additionals directory
# (do not use for production!)
# (this is used to not create conflicts with other plugins)
if File.file? File.expand_path './.enable_linters', __dir__
group :development, :test do
gem 'brakeman', require: false
gem 'pandoc-ruby', require: false
gem 'rubocop', require: false
@ -18,3 +37,18 @@ group :development do
gem 'slim_lint', require: false
end
end
# if you want to use it for tests, do:
# - create .enable_test file in additionals directory
# (this is used to not create conflicts with other plugins)
if File.file? File.expand_path './.enable_test', __dir__
group :development, :test do
gem 'active_record_doctor', require: false
gem 'bullet'
end
group :test do
gem 'ci_reporter_minitest'
gem 'simplecov-cobertura' if ENV['COVERAGE_COBERTURA']
gem 'timecop'
end
end

2
app/models/dashboard.rb
View File

@ -115,7 +115,7 @@ class Dashboard < ActiveRecord::Base
if user.admin?
scope.where.not(visibility: VISIBILITY_PRIVATE).or(scope.where(author_id: user.id))
elsif user.memberships.any?
elsif user.memberships.includes([:memberships]).any?
scope.where("#{table_name}.visibility = ?" \
" OR (#{table_name}.visibility = ? AND #{table_name}.id IN (" \
"SELECT DISTINCT d.id FROM #{table_name} d" \

6
db/migrate/006_add_new_ticket_message_to_projects.rb
View File

@ -2,7 +2,9 @@
class AddNewTicketMessageToProjects < ActiveRecord::Migration[5.2]
def change
add_column :projects, :new_ticket_message, :text
add_column :projects, :enable_new_ticket_message, :integer, default: 1, null: false
change_table :projects, bulk: true do |t|
t.text :new_ticket_message
t.integer :enable_new_ticket_message, default: 1, null: false
end
end
end

4
lib/additionals/patches/principal_patch.rb
View File

@ -28,7 +28,9 @@ module Additionals
all
else
view_all_active = if user.memberships.to_a.any?
user.memberships.any? { |m| m.roles.any? { |r| r.users_visibility == 'all' } }
user.memberships
.includes([:roles])
.any? { |m| m.roles.any? { |r| r.users_visibility == 'all' } }
else
user.builtin_role.users_visibility == 'all'
end

1
lib/additionals/wiki_macros/member_macro.rb
View File

@ -61,6 +61,7 @@ module Additionals
users = User.visible
.where(type: 'User')
.active
.includes([:email_address])
.sorted
end

75
test/global_test_helper.rb
View File

@ -2,6 +2,81 @@
module Additionals
module GlobalTestHelper
def after_setup
return super unless defined?(Bullet) && Bullet.enable?
Bullet.unused_eager_loading_enable = false
Bullet.raise = true
# @see https://github.com/flyerhzm/bullet#safe-list
# ignore missing n+1 problems in redmine core
Bullet.add_safelist type: :n_plus_one_query, class_name: 'Attachment', association: :author
Bullet.add_safelist type: :n_plus_one_query, class_name: 'Attachment', association: :container
Bullet.add_safelist type: :n_plus_one_query, class_name: 'Board', association: :messages
Bullet.add_safelist type: :n_plus_one_query, class_name: 'Dashboard', association: :author
Bullet.add_safelist type: :n_plus_one_query, class_name: 'Dashboard', association: :dashboard_roles
Bullet.add_safelist type: :n_plus_one_query, class_name: 'Dashboard', association: :project
Bullet.add_safelist type: :n_plus_one_query, class_name: 'Issue', association: :assigned_to
Bullet.add_safelist type: :n_plus_one_query, class_name: 'Issue', association: :category
Bullet.add_safelist type: :n_plus_one_query, class_name: 'Issue', association: :custom_values
Bullet.add_safelist type: :n_plus_one_query, class_name: 'Issue', association: :fixed_version
Bullet.add_safelist type: :n_plus_one_query, class_name: 'Issue', association: :parent
Bullet.add_safelist type: :n_plus_one_query, class_name: 'Issue', association: :tracker
Bullet.add_safelist type: :n_plus_one_query, class_name: 'Journal', association: :details
Bullet.add_safelist type: :n_plus_one_query, class_name: 'Journal', association: :journal_message
Bullet.add_safelist type: :n_plus_one_query, class_name: 'Journal', association: :journalized
Bullet.add_safelist type: :n_plus_one_query, class_name: 'Member', association: :member_roles
Bullet.add_safelist type: :n_plus_one_query, class_name: 'Member', association: :project
Bullet.add_safelist type: :n_plus_one_query, class_name: 'Message', association: :attachments
Bullet.add_safelist type: :n_plus_one_query, class_name: 'Message', association: :children
Bullet.add_safelist type: :n_plus_one_query, class_name: 'Message', association: :parent
Bullet.add_safelist type: :n_plus_one_query, class_name: 'News', association: :attachments
Bullet.add_safelist type: :n_plus_one_query, class_name: 'Project', association: :attachments
Bullet.add_safelist type: :n_plus_one_query, class_name: 'Project', association: :boards
Bullet.add_safelist type: :n_plus_one_query, class_name: 'Project', association: :contacts
Bullet.add_safelist type: :n_plus_one_query, class_name: 'Project', association: :dashboards
Bullet.add_safelist type: :n_plus_one_query, class_name: 'Project', association: :db_entries
Bullet.add_safelist type: :n_plus_one_query, class_name: 'Project', association: :documents
Bullet.add_safelist type: :n_plus_one_query, class_name: 'Project', association: :helpdesk_setting
Bullet.add_safelist type: :n_plus_one_query, class_name: 'Project', association: :invoice_setting
Bullet.add_safelist type: :n_plus_one_query, class_name: 'Project', association: :invoices
Bullet.add_safelist type: :n_plus_one_query, class_name: 'Project', association: :issues
Bullet.add_safelist type: :n_plus_one_query, class_name: 'Project', association: :messenger_setting
Bullet.add_safelist type: :n_plus_one_query, class_name: 'Project', association: :news
Bullet.add_safelist type: :n_plus_one_query, class_name: 'Project', association: :parent
Bullet.add_safelist type: :n_plus_one_query, class_name: 'Project', association: :passwords
Bullet.add_safelist type: :n_plus_one_query, class_name: 'Project', association: :repositories
Bullet.add_safelist type: :n_plus_one_query, class_name: 'Project', association: :tag_taggings
Bullet.add_safelist type: :n_plus_one_query, class_name: 'Project', association: :taggings
Bullet.add_safelist type: :n_plus_one_query, class_name: 'Project', association: :time_entries
Bullet.add_safelist type: :n_plus_one_query, class_name: 'Project', association: :versions
Bullet.add_safelist type: :n_plus_one_query, class_name: 'Project', association: :wiki
Bullet.add_safelist type: :n_plus_one_query, class_name: 'TimeEntry', association: :automation_schedule
Bullet.add_safelist type: :n_plus_one_query, class_name: 'TimeEntry', association: :reporting_cost
Bullet.add_safelist type: :n_plus_one_query, class_name: 'Tracker', association: :default_status
Bullet.add_safelist type: :n_plus_one_query, class_name: 'User', association: :local_avatar
Bullet.add_safelist type: :n_plus_one_query, class_name: 'User', association: :preference
Bullet.add_safelist type: :n_plus_one_query, class_name: 'Version', association: :attachments
Bullet.add_safelist type: :n_plus_one_query, class_name: 'WikiContent', association: :page
Bullet.add_safelist type: :n_plus_one_query, class_name: 'WikiPage', association: :attachments
Bullet.add_safelist type: :n_plus_one_query, class_name: 'WikiPage', association: :content
Bullet.add_safelist type: :n_plus_one_query, class_name: 'WikiPage', association: :links_from
Bullet.add_safelist type: :n_plus_one_query, class_name: 'WikiPage', association: :tag_taggings
Bullet.add_safelist type: :n_plus_one_query, class_name: 'WikiPage', association: :taggings
Bullet.add_safelist type: :n_plus_one_query, class_name: 'WikiPage', association: :wiki
Bullet.add_safelist type: :n_plus_one_query, class_name: 'WikiPage', association: :wiki_page_votes
Bullet.start_request
super
end
def before_teardown
super
return unless defined?(Bullet) && Bullet.enable?
Bullet.perform_out_of_channel_notifications if Bullet.notification?
Bullet.end_request
Bullet.raise = false
end
def assert_select_td_column(column_name, colspan: nil)
c = column_name.to_s
.gsub('issue.cf', 'issue_cf')

4
test/integration/api_test/issues_test.rb
View File

@ -3,7 +3,7 @@
require File.expand_path '../../../test_helper', __FILE__
module ApiTest
class IssuesTest < Redmine::ApiTest::Base
class IssuesTest < Additionals::ApiTest
fixtures :projects,
:users,
:roles,
@ -25,8 +25,6 @@ module ApiTest
:journal_details,
:queries
include Additionals::TestHelper
test 'GET /issues.xml should contain metadata' do
get '/issues.xml'
assert_select 'issues[type=array][total_count][limit="25"][offset="0"]'

6
test/integration/api_test/projects_test.rb
View File

@ -3,7 +3,7 @@
require File.expand_path '../../../test_helper', __FILE__
module ApiTest
class ProjectsTest < Redmine::ApiTest::Base
class ProjectsTest < Additionals::ApiTest
fixtures :users, :email_addresses, :roles,
:enumerations,
:projects, :projects_trackers, :enabled_modules,
@ -14,9 +14,7 @@ module ApiTest
:attachments,
:custom_fields, :custom_values,
:time_entries,
:dashboards
include Additionals::TestHelper
:dashboards, :dashboard_roles
test 'GET /projects.xml should return projects' do
get '/projects.xml'

2
test/integration/common_views_test.rb
View File

@ -18,8 +18,6 @@ class CommonViewsTest < Additionals::IntegrationTest
:custom_values,
:custom_fields_trackers
include Additionals::TestHelper
def setup
prepare_tests
end

5
test/support/additional_environment.rb Normal file
View File

@ -0,0 +1,5 @@
# frozen_string_literal: true
# Activate n+1 tests, if true
# TODO: this is disabled at the moment
Bullet.enable = false if defined? Bullet

6
test/test_helper.rb
View File

@ -55,6 +55,12 @@ module Additionals
end
class IntegrationTest < Redmine::IntegrationTest
include Additionals::TestHelper
extend PluginFixturesLoader
end
class ApiTest < Redmine::ApiTest::Base
include Additionals::TestHelper
extend PluginFixturesLoader
end
end

1
test/unit/dashboard_test.rb
View File

@ -165,6 +165,7 @@ class DashboardTest < Additionals::TestCase
assert_raise Exception do
Dashboard.project_only
.where(system_default: true)
.includes([:dashboard_roles])
.destroy_all
end
end