From aa8bc75bc069b4c9780824a58587cffb3b8b6b59 Mon Sep 17 00:00:00 2001 From: Mehmet Emin INAC Date: Thu, 14 Jan 2021 03:56:57 +0100 Subject: [PATCH 1/2] Do not reload the routes multiple times on application boot --- .../create_incident_label_service.rb | 20 +++++++------ config/application.rb | 4 +++ config/initializers/8_devise.rb | 7 ++--- ...ate_incident_sla_exceeded_label_service.rb | 28 +++++++++++-------- lib/gitlab/usage_data.rb | 2 +- spec/factories/labels.rb | 2 +- ..._settings_to_all_existing_projects_spec.rb | 2 +- ...e_incident_issues_to_incident_type_spec.rb | 2 +- .../services/incident_shared_examples.rb | 2 +- 9 files changed, 40 insertions(+), 29 deletions(-) diff --git a/app/services/incident_management/create_incident_label_service.rb b/app/services/incident_management/create_incident_label_service.rb index 595f5df184fe5a..31dca29c13e823 100644 --- a/app/services/incident_management/create_incident_label_service.rb +++ b/app/services/incident_management/create_incident_label_service.rb @@ -2,18 +2,20 @@ module IncidentManagement class CreateIncidentLabelService < BaseService - LABEL_PROPERTIES = { - title: 'incident', - color: '#CC0033', - description: <<~DESCRIPTION.chomp - Denotes a disruption to IT services and \ - the associated issues require immediate attention - DESCRIPTION - }.freeze + def self.label_properties + @label_properties ||= { + title: 'incident', + color: '#CC0033', + description: <<~DESCRIPTION.chomp + Denotes a disruption to IT services and \ + the associated issues require immediate attention + DESCRIPTION + }.freeze + end def execute label = Labels::FindOrCreateService - .new(current_user, project, **LABEL_PROPERTIES) + .new(current_user, project, **self.class.label_properties) .execute(skip_authorization: true) ServiceResponse.success(payload: { label: label }) diff --git a/config/application.rb b/config/application.rb index 341cc36cbdb647..371f6f22d53a48 100644 --- a/config/application.rb +++ b/config/application.rb @@ -368,5 +368,9 @@ class Application < Rails::Application end end end + + config.before_eager_load do |config| + ::API::API + end end end diff --git a/config/initializers/8_devise.rb b/config/initializers/8_devise.rb index a4841a11a0049f..00f50c5161a692 100644 --- a/config/initializers/8_devise.rb +++ b/config/initializers/8_devise.rb @@ -6,10 +6,9 @@ manager.default_strategies(scope: :user).unshift :two_factor_backupable end - # This is the default. This makes it explicit that Devise loads routes - # before eager loading. Disabling this seems to cause an error loading - # grape-entity `expose` for some reason. - config.reload_routes = true + # Rails applications are already reloading the routes with `set_routes_reloader_hook` + # so there is no need to reload the routes before the eager_load! hook. + config.reload_routes = false # ==> Mailer Configuration # Configure the class responsible to send e-mails. diff --git a/ee/app/services/incident_management/create_incident_sla_exceeded_label_service.rb b/ee/app/services/incident_management/create_incident_sla_exceeded_label_service.rb index 1dfec055e5933a..17384931d02d39 100644 --- a/ee/app/services/incident_management/create_incident_sla_exceeded_label_service.rb +++ b/ee/app/services/incident_management/create_incident_sla_exceeded_label_service.rb @@ -2,21 +2,27 @@ module IncidentManagement class CreateIncidentSlaExceededLabelService < BaseService - def self.doc_url - Rails.application.routes.url_helpers.help_page_url('operations/incident_management/incidents', anchor: 'service-level-agreement-countdown-timer') - end + class << self + def label_properties + @label_properties ||= { + title: 'missed::SLA', + color: '#D9534F', + description: <<~DESCRIPTION.chomp + Incidents that have missed the targeted SLA (Service Level Agreement). #{doc_url} + DESCRIPTION + }.freeze + end + + private - LABEL_PROPERTIES = { - title: 'missed::SLA', - color: '#D9534F', - description: <<~DESCRIPTION.chomp - Incidents that have missed the targeted SLA (Service Level Agreement). #{doc_url} - DESCRIPTION - }.freeze + def doc_url + Rails.application.routes.url_helpers.help_page_url('operations/incident_management/incidents', anchor: 'service-level-agreement-countdown-timer') + end + end def execute label = Labels::FindOrCreateService - .new(current_user, project, **LABEL_PROPERTIES) + .new(current_user, project, **self.class.label_properties) .execute(skip_authorization: true) ServiceResponse.success(payload: { label: label }) diff --git a/lib/gitlab/usage_data.rb b/lib/gitlab/usage_data.rb index f935c677930805..9d9da254220663 100644 --- a/lib/gitlab/usage_data.rb +++ b/lib/gitlab/usage_data.rb @@ -142,7 +142,7 @@ def system_usage_data issues_created_manually_from_alerts: issues_created_manually_from_alerts, incident_issues: count(::Issue.incident, start: issue_minimum_id, finish: issue_maximum_id), alert_bot_incident_issues: count(::Issue.authored(::User.alert_bot), start: issue_minimum_id, finish: issue_maximum_id), - incident_labeled_issues: count(::Issue.with_label_attributes(::IncidentManagement::CreateIncidentLabelService::LABEL_PROPERTIES), start: issue_minimum_id, finish: issue_maximum_id), + incident_labeled_issues: count(::Issue.with_label_attributes(::IncidentManagement::CreateIncidentLabelService.label_properties), start: issue_minimum_id, finish: issue_maximum_id), keys: count(Key), label_lists: count(List.label), lfs_objects: count(LfsObject), diff --git a/spec/factories/labels.rb b/spec/factories/labels.rb index a9a9416c48bb7b..58975767867734 100644 --- a/spec/factories/labels.rb +++ b/spec/factories/labels.rb @@ -19,7 +19,7 @@ end trait :incident do - properties = IncidentManagement::CreateIncidentLabelService::LABEL_PROPERTIES + properties = IncidentManagement::CreateIncidentLabelService.label_properties title { properties.fetch(:title) } description { properties.fetch(:description) } color { properties.fetch(:color) } diff --git a/spec/migrations/add_incident_settings_to_all_existing_projects_spec.rb b/spec/migrations/add_incident_settings_to_all_existing_projects_spec.rb index a62fc43df02717..185f5f923be905 100644 --- a/spec/migrations/add_incident_settings_to_all_existing_projects_spec.rb +++ b/spec/migrations/add_incident_settings_to_all_existing_projects_spec.rb @@ -67,7 +67,7 @@ context 'when project has incident labels' do before do issue = issues.create!(project_id: project.id) - incident_label_attrs = IncidentManagement::CreateIncidentLabelService::LABEL_PROPERTIES + incident_label_attrs = IncidentManagement::CreateIncidentLabelService.label_properties incident_label = labels.create!(project_id: project.id, **incident_label_attrs) label_links.create!(target_id: issue.id, label_id: incident_label.id, target_type: 'Issue') end diff --git a/spec/migrations/migrate_incident_issues_to_incident_type_spec.rb b/spec/migrations/migrate_incident_issues_to_incident_type_spec.rb index dc38695c7fe6fe..1ce02e7bdec5a0 100644 --- a/spec/migrations/migrate_incident_issues_to_incident_type_spec.rb +++ b/spec/migrations/migrate_incident_issues_to_incident_type_spec.rb @@ -11,7 +11,7 @@ let(:labels) { table(:labels) } let(:issues) { table(:issues) } let(:label_links) { table(:label_links) } - let(:label_props) { IncidentManagement::CreateIncidentLabelService::LABEL_PROPERTIES } + let(:label_props) { IncidentManagement::CreateIncidentLabelService.label_properties } let(:namespace) { namespaces.create!(name: 'foo', path: 'foo') } let!(:project) { projects.create!(namespace_id: namespace.id) } diff --git a/spec/support/shared_examples/services/incident_shared_examples.rb b/spec/support/shared_examples/services/incident_shared_examples.rb index 9fced12b543824..d204b7f6c42129 100644 --- a/spec/support/shared_examples/services/incident_shared_examples.rb +++ b/spec/support/shared_examples/services/incident_shared_examples.rb @@ -63,7 +63,7 @@ subject(:execute) { service.execute } describe 'execute' do - let(:incident_label_attributes) { described_class::LABEL_PROPERTIES } + let(:incident_label_attributes) { described_class.label_properties } let(:title) { incident_label_attributes[:title] } let(:color) { incident_label_attributes[:color] } let(:description) { incident_label_attributes[:description] } -- GitLab From c99e87c044c0e258baa830e07de16bfbbe766c73 Mon Sep 17 00:00:00 2001 From: Mehmet Emin INAC Date: Mon, 25 Jan 2021 13:58:05 +0100 Subject: [PATCH 2/2] Hack to see if it works --- app/helpers/gitlab_routing_helper.rb | 6 ++++++ config/routes.rb | 13 +++++++++++++ 2 files changed, 19 insertions(+) diff --git a/app/helpers/gitlab_routing_helper.rb b/app/helpers/gitlab_routing_helper.rb index 7ae00a70671eda..882ae300796da7 100644 --- a/app/helpers/gitlab_routing_helper.rb +++ b/app/helpers/gitlab_routing_helper.rb @@ -9,6 +9,12 @@ module GitlabRoutingHelper Gitlab::Routing.includes_helpers(self) end + def self.unbound_methods + instance_methods(false).map do |method_name| + instance_method(method_name) + end + end + # Project def project_tree_path(project, ref = nil, *args) namespace_project_tree_path(project.namespace, project, ref || @ref || project.repository.root_ref, *args) # rubocop:disable Cop/ProjectPathHelper diff --git a/config/routes.rb b/config/routes.rb index 673719abb00001..0c3fcf8cd623a0 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -307,9 +307,22 @@ end end + GitlabRoutingHelper.unbound_methods.each do |method| + bound_method = method.bind(Gitlab::Routing.url_helpers) + + # direct(method.name) do |*args| + # args.pop if args.last == {} + + # bound_method.call(*args) + # end + end + root to: "root#index" get '*unmatched_route', to: 'application#route_not_found' end Gitlab::Routing.add_helpers(TimeboxesRoutingHelper) + +# HACK! +ActionDispatch::Routing::RouteSet::MountedHelpers.prepend(GitlabRoutingHelper) -- GitLab