Merge branch 'dz-redirect-old-repo-routes' into 'master'

Redirect some deprecated repository routes

See merge request gitlab-org/gitlab!34867

changelogs/unreleased/dz-redirect-old-repo-routes.yml

+---
+title: Redirect some of deprecated repository routes
+merge_request: 34867
+author:
+type: removed

config/routes/project.rb

@@ -386,7 +386,6 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
         # 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 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
       # 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,

config/routes/repository.rb

 # 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

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 @@ scope format: false do
       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]

spec/routing/project_routing_spec.rb

@@ -202,6 +202,16 @@ RSpec.describe 'project routing' do
                        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 @@ RSpec.describe 'project routing' do
       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 @@ RSpec.describe 'project 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 '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 @@ RSpec.describe 'project routing' do
     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 @@ RSpec.describe 'project routing' do
                       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 @@ RSpec.describe 'project routing' do
                         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 @@ RSpec.describe 'project routing' do
       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