Commit 8f8e692b authored by Heinrich Lee Yu's avatar Heinrich Lee Yu

Make sure Projects#show is matched last

EE-routes should override CE ones so it should come first. This
also makes fallback routes in CE work efficiently.

For example: `/namespace/project/insights` matches
ProjectsController#show first and then fails due to
ProjectUrlConstrainer (when it was checking for existence)
then it tries to match the route defined in EE.

This results in unnecessary SQL queries when we had existence
checks and broken routes when we removed the existence check.

We also needed to move the definition of legacy routes so that
they are matched earlier.
parent d3a6c5bd
......@@ -245,12 +245,6 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
post :validate_query, on: :collection
end
end
Gitlab.ee do
resources :alerts, constraints: { id: /\d+/ }, only: [:index, :create, :show, :update, :destroy] do
post :notify, on: :collection
end
end
end
resources :merge_requests, concerns: :awardable, except: [:new, :create, :show], constraints: { id: /\d+/ } do
......@@ -353,17 +347,6 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
end
end
Gitlab.ee do
resources :path_locks, only: [:index, :destroy] do
collection do
post :toggle
end
end
get '/service_desk' => 'service_desk#show', as: :service_desk
put '/service_desk' => 'service_desk#update', as: :service_desk_refresh
end
resource :variables, only: [:show, :update]
resources :triggers, only: [:index, :create, :edit, :update, :destroy]
......@@ -397,11 +380,6 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
get :failures
get :status
get :test_report
Gitlab.ee do
get :security
get :licenses
end
end
member do
......@@ -536,24 +514,11 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
get :realtime_changes
post :create_merge_request
get :discussions, format: :json
Gitlab.ee do
get 'designs(/*vueroute)', to: 'issues#designs', as: :designs, format: false
end
end
collection do
post :bulk_update
post :import_csv
Gitlab.ee do
post :export_csv
get :service_desk
end
end
Gitlab.ee do
resources :issue_links, only: [:index, :create, :destroy], as: 'links', path: 'links'
end
end
......@@ -629,6 +594,15 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
Gitlab.ee do
resources :managed_licenses, only: [:index, :show, :new, :create, :edit, :update, :destroy]
end
# Legacy routes.
# Introduced in 12.0.
# Should be removed after 12.1
Gitlab::Routing.redirect_legacy_paths(self, :settings, :branches, :tags,
:network, :graphs, :autocomplete_sources,
:project_members, :deploy_keys, :deploy_tokens,
:labels, :milestones, :services, :boards, :releases,
:forks, :group_links, :import, :avatar)
end
resources(:projects,
......@@ -653,22 +627,4 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
end
end
end
# Legacy routes.
# Introduced in 12.0.
# Should be removed after 12.1
scope(path: '*namespace_id',
as: :namespace,
namespace_id: Gitlab::PathRegex.full_namespace_route_regex) do
scope(path: ':project_id',
constraints: { project_id: Gitlab::PathRegex.project_route_regex },
module: :projects,
as: :project) do
Gitlab::Routing.redirect_legacy_paths(self, :settings, :branches, :tags,
:network, :graphs, :autocomplete_sources,
:project_members, :deploy_keys, :deploy_tokens,
:labels, :milestones, :services, :boards, :releases,
:forks, :group_links, :import, :avatar)
end
end
end
# frozen_string_literal: true
namespace :admin do
resources :users, constraints: { id: %r{[a-zA-Z./0-9_\-]+} } do
resources :users, only: [], constraints: { id: %r{[a-zA-Z./0-9_\-]+} } do
member do
post :reset_runners_minutes
end
......
......@@ -144,14 +144,6 @@ constraints(::Constraints::GroupUrlConstrainer.new) do
resource :roadmap, only: [:show], controller: 'roadmap'
legacy_ee_group_boards_redirect = redirect do |params, request|
path = "/groups/#{params[:group_id]}/-/boards"
path << "/#{params[:extra_params]}" if params[:extra_params].present?
path << "?#{request.query_string}" if request.query_string.present?
path
end
get 'boards(/*extra_params)', as: :legacy_ee_group_boards_redirect, to: legacy_ee_group_boards_redirect
resource :dependency_proxy, only: [:show, :update]
resources :packages, only: [:index]
end
......
......@@ -52,6 +52,18 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
end
# End of the /-/ scope.
resources :path_locks, only: [:index, :destroy] do
collection do
post :toggle
end
end
namespace :prometheus do
resources :alerts, constraints: { id: /\d+/ }, only: [:index, :create, :show, :update, :destroy] do
post :notify, on: :collection
end
end
post 'alerts/notify', to: 'alerting/notifications#create'
resource :tracing, only: [:show]
......@@ -67,6 +79,22 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
end
end
resources :issues, only: [], constraints: { id: /\d+/ } do
member do
get 'designs(/*vueroute)', to: 'issues#designs', as: :designs, format: false
end
collection do
post :export_csv
get :service_desk
end
resources :issue_links, only: [:index, :create, :destroy], as: 'links', path: 'links'
end
get '/service_desk' => 'service_desk#show', as: :service_desk
put '/service_desk' => 'service_desk#update', as: :service_desk_refresh
resources :merge_requests, only: [], constraints: { id: /\d+/ } do
member do
get :metrics_reports
......@@ -77,6 +105,13 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
end
end
resources :pipelines, only: [] do
member do
get :security
get :licenses
end
end
resource :insights, only: [:show], trailing_slash: true do
collection do
post :query
......
......@@ -41,7 +41,7 @@ describe Projects::ManagedLicensesController do
describe 'GET #index' do
subject do
allow(controller).to receive(:current_user).and_return(user)
sign_in(user) if user
get :index, params: { namespace_id: project.namespace.to_param, project_id: project }, format: :json
end
......@@ -98,7 +98,7 @@ describe Projects::ManagedLicensesController do
describe 'GET #show' do
subject do
allow(controller).to receive(:current_user).and_return(user)
sign_in(user) if user
get :show,
params: {
......@@ -151,7 +151,7 @@ describe Projects::ManagedLicensesController do
let(:user) { dev_user }
subject do
allow(controller).to receive(:current_user).and_return(user)
sign_in(user) if user
get :show,
params: {
......@@ -189,7 +189,7 @@ describe Projects::ManagedLicensesController do
end
subject do
allow(controller).to receive(:current_user).and_return(user)
sign_in(user) if user
post :create,
params: {
......@@ -300,7 +300,7 @@ describe Projects::ManagedLicensesController do
end
subject do
allow(controller).to receive(:current_user).and_return(user)
sign_in(user) if user
patch :update,
params: {
......@@ -406,7 +406,7 @@ describe Projects::ManagedLicensesController do
let(:id_to_destroy) { software_license_policy.id }
subject do
allow(controller).to receive(:current_user).and_return(user)
sign_in(user) if user
delete :destroy,
params: {
......
......@@ -10,7 +10,7 @@ module Gitlab
RoutesNotFound = Class.new(StandardError)
def draw(routes_name)
drawn_any = draw_ce(routes_name) | draw_ee(routes_name)
drawn_any = draw_ee(routes_name) | draw_ce(routes_name)
drawn_any || raise(RoutesNotFound.new("Cannot find #{routes_name}"))
end
......
......@@ -776,10 +776,6 @@ describe 'project routing' do
it 'routes when :template_type is `issue`' do
expect(get(show_with_template_type('issue'))).to route_to('projects/templates#show', namespace_id: 'gitlab', project_id: 'gitlabhq', template_type: 'issue', key: 'template_name', format: 'json')
end
it 'routes to application#route_not_found when :template_type is unknown' do
expect(get(show_with_template_type('invalid'))).to route_to('application#route_not_found', unmatched_route: 'gitlab/gitlabhq/templates/invalid/template_name')
end
end
end
......
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