Fix conflicting redirect searchmk-fix-redirect-matching

3 files changed, 61 insertions, 27 deletions

diff --git a/app/models/redirect_route.rb b/app/models/redirect_route.rb
index 964175ddab8..a034aacb23c 100644
--- a/app/models/redirect_route.rb
+++ b/app/models/redirect_route.rb
@@ -8,5 +8,5 @@ class RedirectRoute < ActiveRecord::Base
     presence: true,
     uniqueness: { case_sensitive: false }
 
-  scope :matching_path_and_descendants, -> (path) { where('redirect_routes.path = ? OR redirect_routes.path LIKE ?', path, "#{sanitize_sql_like(path)}/%") }
+  scope :matching_path_and_descendants, -> (path) { where('LOWER(redirect_routes.path) = LOWER(?) OR LOWER(redirect_routes.path) LIKE LOWER(?)', path, "#{sanitize_sql_like(path)}/%") }
 end
diff --git a/spec/models/redirect_route_spec.rb b/spec/models/redirect_route_spec.rb
index 80943877095..106ae59af29 100644
--- a/spec/models/redirect_route_spec.rb
+++ b/spec/models/redirect_route_spec.rb
@@ -20,8 +20,16 @@ describe RedirectRoute do
     let!(:redirect4) { group.redirect_routes.create(path: 'gitlabb/test/foo/bar') }
     let!(:redirect5) { group.redirect_routes.create(path: 'gitlabb/test/baz') }
 
-    it 'returns correct routes' do
-      expect(described_class.matching_path_and_descendants('gitlabb/test')).to match_array([redirect2, redirect3, redirect4, redirect5])
+    context 'when the redirect route matches with same casing' do
+      it 'returns correct routes' do
+        expect(described_class.matching_path_and_descendants('gitlabb/test')).to match_array([redirect2, redirect3, redirect4, redirect5])
+      end
+    end
+
+    context 'when the redirect route matches with different casing' do
+      it 'returns correct routes' do
+        expect(described_class.matching_path_and_descendants('GitLABB/test')).to match_array([redirect2, redirect3, redirect4, redirect5])
+      end
     end
   end
 end
diff --git a/spec/models/route_spec.rb b/spec/models/route_spec.rb
index bdacc60fb53..42c2104ceb8 100644
--- a/spec/models/route_spec.rb
+++ b/spec/models/route_spec.rb
@@ -145,45 +145,71 @@ describe Route do
 
   describe '#delete_conflicting_redirects' do
     context 'when a redirect route with the same path exists' do
-      let!(:redirect1) { route.create_redirect(route.path) }
+      context 'when the redirect route has matching case' do
+        let!(:redirect1) { route.create_redirect(route.path) }
 
-      it 'deletes the redirect' do
-        route.delete_conflicting_redirects
-        expect(route.conflicting_redirects).to be_empty
+        it 'deletes the redirect' do
+          expect do
+            route.delete_conflicting_redirects
+          end.to change{RedirectRoute.count}.by(-1)
+        end
+
+        context 'when redirect routes with paths descending from the route path exists' do
+          let!(:redirect2) { route.create_redirect("#{route.path}/foo") }
+          let!(:redirect3) { route.create_redirect("#{route.path}/foo/bar") }
+          let!(:redirect4) { route.create_redirect("#{route.path}/baz/quz") }
+          let!(:other_redirect) { route.create_redirect("other") }
+
+          it 'deletes all redirects with paths that descend from the route path' do
+            expect do
+              route.delete_conflicting_redirects
+            end.to change{RedirectRoute.count}.by(-4)
+          end
+        end
       end
 
-      context 'when redirect routes with paths descending from the route path exists' do
-        let!(:redirect2) { route.create_redirect("#{route.path}/foo") }
-        let!(:redirect3) { route.create_redirect("#{route.path}/foo/bar") }
-        let!(:redirect4) { route.create_redirect("#{route.path}/baz/quz") }
-        let!(:other_redirect) { route.create_redirect("other") }
+      context 'when the redirect route is differently cased' do
+        let!(:redirect1) { route.create_redirect(route.path.upcase) }
 
-        it 'deletes all redirects with paths that descend from the route path' do
-          route.delete_conflicting_redirects
-          expect(route.conflicting_redirects).to be_empty
+        it 'deletes the redirect' do
+          expect do
+            route.delete_conflicting_redirects
+          end.to change{RedirectRoute.count}.by(-1)
         end
       end
     end
   end
 
   describe '#conflicting_redirects' do
+    it 'returns an ActiveRecord::Relation' do
+      expect(route.conflicting_redirects).to be_an(ActiveRecord::Relation)
+    end
+
     context 'when a redirect route with the same path exists' do
-      let!(:redirect1) { route.create_redirect(route.path) }
+      context 'when the redirect route has matching case' do
+        let!(:redirect1) { route.create_redirect(route.path) }
 
-      it 'returns the redirect route' do
-        expect(route.conflicting_redirects).to be_an(ActiveRecord::Relation)
-        expect(route.conflicting_redirects).to match_array([redirect1])
+        it 'returns the redirect route' do
+          expect(route.conflicting_redirects).to match_array([redirect1])
+        end
+
+        context 'when redirect routes with paths descending from the route path exists' do
+          let!(:redirect2) { route.create_redirect("#{route.path}/foo") }
+          let!(:redirect3) { route.create_redirect("#{route.path}/foo/bar") }
+          let!(:redirect4) { route.create_redirect("#{route.path}/baz/quz") }
+          let!(:other_redirect) { route.create_redirect("other") }
+
+          it 'returns the redirect routes' do
+            expect(route.conflicting_redirects).to match_array([redirect1, redirect2, redirect3, redirect4])
+          end
+        end
       end
 
-      context 'when redirect routes with paths descending from the route path exists' do
-        let!(:redirect2) { route.create_redirect("#{route.path}/foo") }
-        let!(:redirect3) { route.create_redirect("#{route.path}/foo/bar") }
-        let!(:redirect4) { route.create_redirect("#{route.path}/baz/quz") }
-        let!(:other_redirect) { route.create_redirect("other") }
+      context 'when the redirect route is differently cased' do
+        let!(:redirect1) { route.create_redirect(route.path.upcase) }
 
-        it 'returns the redirect routes' do
-          expect(route.conflicting_redirects).to be_an(ActiveRecord::Relation)
-          expect(route.conflicting_redirects).to match_array([redirect1, redirect2, redirect3, redirect4])
+        it 'returns the redirect route' do
+          expect(route.conflicting_redirects).to match_array([redirect1])
         end
       end
     end