From 3e65aab33041f258c8adbe599854158bf080aabf Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 18 Jun 2020 20:00:07 +0300 Subject: [PATCH] Redirect some deprecated repo routes Move more repository routes to repository_scoped file and add legacy redirects. But keep serving old routes for raw, blame, blob and tree. Signed-off-by: Dmytro Zaporozhets --- .../dz-redirect-old-repo-routes.yml | 5 ++ config/routes/project.rb | 2 +- config/routes/repository.rb | 90 +++++-------------- config/routes/repository_scoped.rb | 74 +++++++++++++++ spec/routing/project_routing_spec.rb | 28 ++++-- 5 files changed, 123 insertions(+), 76 deletions(-) create mode 100644 changelogs/unreleased/dz-redirect-old-repo-routes.yml diff --git a/changelogs/unreleased/dz-redirect-old-repo-routes.yml b/changelogs/unreleased/dz-redirect-old-repo-routes.yml new file mode 100644 index 00000000000000..a6c72e43433945 --- /dev/null +++ b/changelogs/unreleased/dz-redirect-old-repo-routes.yml @@ -0,0 +1,5 @@ +--- +title: Redirect some of deprecated repository routes +merge_request: 34867 +author: +type: removed diff --git a/config/routes/project.rb b/config/routes/project.rb index d62e2f1b2f2d39..935e816f73c02f 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -386,7 +386,6 @@ # The wiki and repository routing contains wildcard characters so # its preferable to keep it below all other project routes draw :repository_scoped - draw :repository draw :wiki namespace :import do @@ -584,6 +583,7 @@ # Introduced in 12.0. # Should be removed with https://gitlab.com/gitlab-org/gitlab/issues/28848. Gitlab::Routing.redirect_legacy_paths(self, :mirror, :tags, :hooks, + :commits, :commit, :find_file, :files, :compare, :cycle_analytics, :mattermost, :variables, :triggers, :environments, :protected_environments, :error_tracking, :alert_management, :tracing, diff --git a/config/routes/repository.rb b/config/routes/repository.rb index 58de3d29bb00fd..854c6517cd55eb 100644 --- a/config/routes/repository.rb +++ b/config/routes/repository.rb @@ -1,84 +1,36 @@ # frozen_string_literal: true # All routing related to repository browsing +# +# NOTE: Add new routes to repository_scoped.rb instead (see +# https://docs.gitlab.com/ee/development/routing.html#project-routes). resource :repository, only: [:create] -resources :commit, only: [:show], constraints: { id: /\h{7,40}/ } do - member do - get :branches - get :pipelines - post :revert - post :cherry_pick - get :diff_for_path - get :diff_files - get :merge_requests - end -end - -# NOTE: Add new routes to repository_scoped.rb instead (see -# https://docs.gitlab.com/ee/development/routing.html#project-routes). -# # Don't use format parameter as file extension (old 3.0.x behavior) # See http://guides.rubyonrails.org/routing.html#route-globbing-and-wildcard-segments scope format: false do - get '/compare/:from...:to', to: 'compare#show', as: 'compare', constraints: { from: /.+/, to: /.+/ } + get '/refs/switch', + to: redirect('%{namespace_id}/%{project_id}/-/refs/switch') - resources :compare, only: [:index, :create] do - collection do - get :diff_for_path - get :signatures - end - end - - resources :refs, only: [] do - collection do - get 'switch' - end + get '/refs/:id/logs_tree', + to: redirect('%{namespace_id}/%{project_id}/-/refs/%{id}/logs_tree'), + constraints: { id: Gitlab::PathRegex.git_reference_regex } - member do - # tree viewer logs - get 'logs_tree', constraints: { id: Gitlab::PathRegex.git_reference_regex } - # Directories with leading dots erroneously get rejected if git - # ref regex used in constraints. Regex verification now done in controller. - get 'logs_tree/*path', action: :logs_tree, as: :logs_file, format: false, constraints: { - id: /.*/, - path: /[^\0]*/ - } - end - end + get '/refs/:id/logs_tree/*path', + to: redirect('%{namespace_id}/%{project_id}/-/refs/%{id}/logs_tree/%{path}'), + constraints: { id: /.*/, path: /[^\0]*/ } scope constraints: { id: /[^\0]+/ } do - scope controller: :blob do - get '/new/*id', action: :new, as: :new_blob - post '/create/*id', action: :create, as: :create_blob - get '/edit/*id', action: :edit, as: :edit_blob - put '/update/*id', action: :update, as: :update_blob - post '/preview/*id', action: :preview, as: :preview_blob - - scope path: '/blob/*id', as: :blob do - get :diff - get '/', action: :show - delete '/', action: :destroy - post '/', action: :create - put '/', action: :update - end - end - - get '/tree/*id', to: 'tree#show', as: :tree - get '/raw/*id', to: 'raw#show', as: :raw - get '/blame/*id', to: 'blame#show', as: :blame - - get '/commits', to: 'commits#commits_root', as: :commits_root - get '/commits/*id/signatures', to: 'commits#signatures', as: :signatures - get '/commits/*id', to: 'commits#show', as: :commits - - post '/create_dir/*id', to: 'tree#create_dir', as: :create_dir - - scope controller: :find_file do - get '/find_file/*id', action: :show, as: :find_file - - get '/files/*id', action: :list, as: :files - end + # Deprecated. Keep for compatibility. + # Issue https://gitlab.com/gitlab-org/gitlab/-/issues/118849 + get '/tree/*id', to: 'tree#show', as: :deprecated_tree + get '/blob/*id', to: 'blob#show', as: :deprecated_blob + get '/raw/*id', to: 'raw#show', as: :deprecated_raw + get '/blame/*id', to: 'blame#show', as: :deprecated_blame + + # Redirect those explicitly since `redirect_legacy_paths` conflicts with project new/edit actions + get '/new/*id', to: redirect('%{namespace_id}/%{project_id}/-/new/%{id}') + get '/edit/*id', to: redirect('%{namespace_id}/%{project_id}/-/edit/%{id}') end end diff --git a/config/routes/repository_scoped.rb b/config/routes/repository_scoped.rb index 7fabf3ff895b51..d2be18c62f9dd8 100644 --- a/config/routes/repository_scoped.rb +++ b/config/routes/repository_scoped.rb @@ -6,6 +6,33 @@ # Don't use format parameter as file extension (old 3.0.x behavior) # See http://guides.rubyonrails.org/routing.html#route-globbing-and-wildcard-segments scope format: false do + get '/compare/:from...:to', to: 'compare#show', as: 'compare', constraints: { from: /.+/, to: /.+/ } + + resources :compare, only: [:index, :create] do + collection do + get :diff_for_path + get :signatures + end + end + + resources :refs, only: [] do + collection do + get 'switch' + end + + member do + # tree viewer logs + get 'logs_tree', constraints: { id: Gitlab::PathRegex.git_reference_regex } + + # Directories with leading dots erroneously get rejected if git + # ref regex used in constraints. Regex verification now done in controller. + get 'logs_tree/*path', action: :logs_tree, as: :logs_file, format: false, constraints: { + id: /.*/, + path: /[^\0]*/ + } + end + end + scope constraints: { id: Gitlab::PathRegex.git_reference_regex } do resources :network, only: [:show] @@ -38,4 +65,51 @@ end end end + + scope constraints: { id: /[^\0]+/ } do + scope controller: :blob do + get '/new/*id', action: :new, as: :new_blob + post '/create/*id', action: :create, as: :create_blob + get '/edit/*id', action: :edit, as: :edit_blob + put '/update/*id', action: :update, as: :update_blob + post '/preview/*id', action: :preview, as: :preview_blob + + scope path: '/blob/*id', as: :blob do + get :diff + get '/', action: :show + delete '/', action: :destroy + post '/', action: :create + put '/', action: :update + end + end + + get '/tree/*id', to: 'tree#show', as: :tree + get '/raw/*id', to: 'raw#show', as: :raw + get '/blame/*id', to: 'blame#show', as: :blame + + get '/commits', to: 'commits#commits_root', as: :commits_root + get '/commits/*id/signatures', to: 'commits#signatures', as: :signatures + get '/commits/*id', to: 'commits#show', as: :commits + + post '/create_dir/*id', to: 'tree#create_dir', as: :create_dir + + scope controller: :find_file do + get '/find_file/*id', action: :show, as: :find_file + get '/files/*id', action: :list, as: :files + end + end +end + +resources :commit, only: [:show], constraints: { id: /\h{7,40}/ } do + member do + get :branches + get :pipelines + post :revert + post :cherry_pick + get :diff_for_path + get :diff_files + get :merge_requests + end end + +resource :repository, only: [:create] diff --git a/spec/routing/project_routing_spec.rb b/spec/routing/project_routing_spec.rb index 056f4d30ea5eee..43d7c31d87f693 100644 --- a/spec/routing/project_routing_spec.rb +++ b/spec/routing/project_routing_spec.rb @@ -202,6 +202,16 @@ namespace_id: 'gitlab', project_id: 'gitlabhq', id: "stable", path: "new\n\nline.txt" }) end + + it_behaves_like 'redirecting a legacy path', '/gitlab/gitlabhq/refs/switch', '/gitlab/gitlabhq/-/refs/switch' + + it_behaves_like 'redirecting a legacy path', + '/gitlab/gitlabhq/refs/feature%2345/logs_tree', + '/gitlab/gitlabhq/-/refs/feature%2345/logs_tree' + + it_behaves_like 'redirecting a legacy path', + '/gitlab/gitlabhq/refs/stable/logs_tree/new%0A%0Aline.txt', + '/gitlab/gitlabhq/-/refs/stable/logs_tree/new%0A%0Aline.txt' end describe Projects::MergeRequestsController, 'routing' do @@ -357,9 +367,7 @@ expect(get('/gitlab/gitlabhq/-/commit/4246fbd13872934f72a8fd0d6fb1317b47b59cb5')).to route_to('projects/commit#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: '4246fbd13872934f72a8fd0d6fb1317b47b59cb5') end - it 'to #show unscoped routing' do - expect(get('/gitlab/gitlabhq/commit/4246fbd')).to route_to('projects/commit#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: '4246fbd') - end + it_behaves_like 'redirecting a legacy path', "/gitlab/gitlabhq/commit/4246fbd", "/gitlab/gitlabhq/-/commit/4246fbd" end # patch_project_commit GET /:project_id/commits/:id/patch(.:format) commits#patch @@ -376,9 +384,7 @@ expect(get('/gitlab/gitlabhq/-/commits/master.atom')).to route_to('projects/commits#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'master.atom') end - it 'to #show unscoped routing' do - expect(get('/gitlab/gitlabhq/commits/master.atom')).to route_to('projects/commits#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'master.atom') - end + it_behaves_like 'redirecting a legacy path', "/gitlab/gitlabhq/commits/master", "/gitlab/gitlabhq/-/commits/master" end # project_project_members GET /:project_id/project_members(.:format) project_members#index @@ -517,6 +523,7 @@ end it 'to #show from unscoped routing' do + expect(get('/gitlab/gitlabhq/tree/master')).to route_to('projects/tree#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'master') expect(get('/gitlab/gitlabhq/tree/master/app/models/project.rb')).to route_to('projects/tree#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'master/app/models/project.rb') end end @@ -545,6 +552,9 @@ namespace_id: 'gitlab', project_id: 'gitlabhq', id: "#{newline_file}" }) end + + it_behaves_like 'redirecting a legacy path', "/gitlab/gitlabhq/find_file", "/gitlab/gitlabhq/-/find_file" + it_behaves_like 'redirecting a legacy path', "/gitlab/gitlabhq/files/master", "/gitlab/gitlabhq/-/files/master" end describe Projects::BlobController, 'routing' do @@ -575,6 +585,9 @@ namespace_id: 'gitlab', project_id: 'gitlabhq', id: "master/docs/#{newline_file}" }) end + + it_behaves_like 'redirecting a legacy path', "/gitlab/gitlabhq/new/master", "/gitlab/gitlabhq/-/new/master" + it_behaves_like 'redirecting a legacy path', "/gitlab/gitlabhq/edit/master/README", "/gitlab/gitlabhq/-/edit/master/README" end # project_raw GET /:project_id/-/raw/:id(.:format) raw#show {id: /[^\0]+/, project_id: /[^\/]+/} @@ -610,6 +623,9 @@ expect(get('/gitlab/gitlabhq/-/compare/master...stable')).to route_to('projects/compare#show', namespace_id: 'gitlab', project_id: 'gitlabhq', from: 'master', to: 'stable') expect(get('/gitlab/gitlabhq/-/compare/issue/1234...stable')).to route_to('projects/compare#show', namespace_id: 'gitlab', project_id: 'gitlabhq', from: 'issue/1234', to: 'stable') end + + it_behaves_like 'redirecting a legacy path', '/gitlab/gitlabhq/compare', '/gitlab/gitlabhq/-/compare' + it_behaves_like 'redirecting a legacy path', '/gitlab/gitlabhq/compare/master...stable', '/gitlab/gitlabhq/-/compare/master...stable' end describe Projects::NetworkController, 'routing' do -- GitLab