From 2deb7f070b39110b9a6c26086ff033583c65ed98 Mon Sep 17 00:00:00 2001 From: Mehmet Emin INAC Date: Tue, 19 Jan 2021 18:30:07 +0100 Subject: [PATCH 1/6] Move project shorthand url helper generation from initializers Initializers run only once while booting the application therefore on development and test environments, if we add a new route, the shorthand url helpers for `projects` resource will not be available until we reboot the project unless we don't move the logic to correct place. --- config/application.rb | 25 ------------------------- config/routes.rb | 22 ++++++++++++++++++++++ 2 files changed, 22 insertions(+), 25 deletions(-) diff --git a/config/application.rb b/config/application.rb index a8b6bc937cfb91..341cc36cbdb647 100644 --- a/config/application.rb +++ b/config/application.rb @@ -368,30 +368,5 @@ class Application < Rails::Application end end end - - config.after_initialize do - # Devise (see initializers/8_devise.rb) already reloads routes if - # eager loading is enabled, so don't do this twice since it's - # expensive. - Rails.application.reload_routes! unless config.eager_load - - project_url_helpers = Module.new do - extend ActiveSupport::Concern - - Gitlab::Application.routes.named_routes.helper_names.each do |name| - next unless name.include?('namespace_project') - - define_method(name.sub('namespace_project', 'project')) do |project, *args| - send(name, project&.namespace, project, *args) - end - end - end - - # We add the MilestonesRoutingHelper because we know that this does not - # conflict with the methods defined in `project_url_helpers`, and we want - # these methods available in the same places. - Gitlab::Routing.add_helpers(project_url_helpers) - Gitlab::Routing.add_helpers(TimeboxesRoutingHelper) - end end end diff --git a/config/routes.rb b/config/routes.rb index 91d1a817175d9a..5258b29e9db154 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -287,7 +287,29 @@ get '/sitemap' => 'sitemap#show', format: :xml end + # Creates shorthand helper methods for project resources. + # For example; for the `namespace_project_path` this also creates `project_path`. + # + # TODO: We don't need the `Gitlab::Routing` module at all as we can use + # the `direct` DSL method of Rails to define url helpers. Move all the + # custom url helpers to use the `direct` DSL method and remove the `Gitlab::Routing`. + Gitlab::Application.routes.set.map(&:name).compact.select { |name| name.include?('namespace_project') }.each do |name| + new_name = name.sub('namespace_project', 'project') + + direct(new_name) do |project, *args| + # This is due to a bug I've found in Rails + # There is already a PR for it but it can take quite long + # to be merged and we will need to update our Rails version as well. + # https://github.com/rails/rails/pull/41184 + args.pop if args.last == {} + + send("#{name}_url", project&.namespace, project, *args) + end + end + root to: "root#index" get '*unmatched_route', to: 'application#route_not_found' end + +Gitlab::Routing.add_helpers(TimeboxesRoutingHelper) -- GitLab From d500524cc86546b7914f50274aa5178ddde7515c Mon Sep 17 00:00:00 2001 From: Mehmet Emin INAC Date: Wed, 20 Jan 2021 06:09:16 +0100 Subject: [PATCH 2/6] Fix Grape::Entity expose method issue --- config/initializers/zz_metrics.rb | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/config/initializers/zz_metrics.rb b/config/initializers/zz_metrics.rb index 430e4d60d611d1..0beb7739085b7a 100644 --- a/config/initializers/zz_metrics.rb +++ b/config/initializers/zz_metrics.rb @@ -154,9 +154,14 @@ def instrument_classes(instrumentation) # of the ActiveRecord methods. This has to take place _after_ initializing as # for some unknown reason calling eager_load! earlier breaks Devise. Gitlab::Application.config.after_initialize do - Rails.application.eager_load! + # We should move all the logic of this file to somewhere else + # and require it after `Rails.application.initialize!` in `environment.rb` file. + models_path = Rails.root.join('app', 'models').to_s + relname_range = (models_path.length + 1)...-3 - models = Rails.root.join('app', 'models').to_s + Dir.glob("#{models_path}/**/*.rb").sort.each do |file| + require_dependency file[relname_range] + end regex = Regexp.union( ActiveRecord::Querying.public_instance_methods(false).map(&:to_s) @@ -172,7 +177,7 @@ def instrument_classes(instrumentation) else loc = method.source_location - loc && loc[0].start_with?(models) && method.source =~ regex + loc && loc[0].start_with?(models_path) && method.source =~ regex end end -- GitLab From 392124767437a488ca260f84750e47a17db6152a Mon Sep 17 00:00:00 2001 From: Mehmet Emin INAC Date: Wed, 20 Jan 2021 05:21:43 +0000 Subject: [PATCH 3/6] Apply 1 suggestion(s) to 1 file(s) --- config/routes.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/routes.rb b/config/routes.rb index 5258b29e9db154..fe0d282c2d8d92 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -293,7 +293,7 @@ # TODO: We don't need the `Gitlab::Routing` module at all as we can use # the `direct` DSL method of Rails to define url helpers. Move all the # custom url helpers to use the `direct` DSL method and remove the `Gitlab::Routing`. - Gitlab::Application.routes.set.map(&:name).compact.select { |name| name.include?('namespace_project') }.each do |name| + Gitlab::Application.routes.set.filter_map { |route| route.name if route.name&.include?('namespace_project') }.each do |name| new_name = name.sub('namespace_project', 'project') direct(new_name) do |project, *args| -- GitLab From f8bbbf2f71c94ea0e6ad15228748abb56a2d4c4d Mon Sep 17 00:00:00 2001 From: Mehmet Emin INAC Date: Fri, 22 Jan 2021 20:41:14 +0100 Subject: [PATCH 4/6] Add reference to related issue --- config/routes.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/config/routes.rb b/config/routes.rb index fe0d282c2d8d92..f32e094d34a911 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -293,6 +293,7 @@ # TODO: We don't need the `Gitlab::Routing` module at all as we can use # the `direct` DSL method of Rails to define url helpers. Move all the # custom url helpers to use the `direct` DSL method and remove the `Gitlab::Routing`. + # For more information: https://gitlab.com/gitlab-org/gitlab/-/issues/299583 Gitlab::Application.routes.set.filter_map { |route| route.name if route.name&.include?('namespace_project') }.each do |name| new_name = name.sub('namespace_project', 'project') -- GitLab From eeefa0f84e6e1eafefd37cb7844d16dec436537e Mon Sep 17 00:00:00 2001 From: Mehmet Emin INAC Date: Fri, 22 Jan 2021 21:14:32 +0100 Subject: [PATCH 5/6] Do not remove the file extension --- config/initializers/zz_metrics.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/config/initializers/zz_metrics.rb b/config/initializers/zz_metrics.rb index 0beb7739085b7a..a3771975950bb5 100644 --- a/config/initializers/zz_metrics.rb +++ b/config/initializers/zz_metrics.rb @@ -157,10 +157,9 @@ def instrument_classes(instrumentation) # We should move all the logic of this file to somewhere else # and require it after `Rails.application.initialize!` in `environment.rb` file. models_path = Rails.root.join('app', 'models').to_s - relname_range = (models_path.length + 1)...-3 - Dir.glob("#{models_path}/**/*.rb").sort.each do |file| - require_dependency file[relname_range] + Dir.glob("**/*.rb", base: models_path).sort.each do |file| + require_dependency file end regex = Regexp.union( -- GitLab From d0888be5b5b3fa99fd58e18670d7980f44fdde03 Mon Sep 17 00:00:00 2001 From: Mehmet Emin INAC Date: Fri, 22 Jan 2021 21:29:06 +0100 Subject: [PATCH 6/6] Add reference to related issue about Rails bug --- config/routes.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/config/routes.rb b/config/routes.rb index f32e094d34a911..eee31730f8a3d3 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -298,10 +298,8 @@ new_name = name.sub('namespace_project', 'project') direct(new_name) do |project, *args| - # This is due to a bug I've found in Rails - # There is already a PR for it but it can take quite long - # to be merged and we will need to update our Rails version as well. - # https://github.com/rails/rails/pull/41184 + # This is due to a bug I've found in Rails. + # For more information: https://gitlab.com/gitlab-org/gitlab/-/issues/299591 args.pop if args.last == {} send("#{name}_url", project&.namespace, project, *args) -- GitLab