Merge branch '215315-repository-identifier' into 'master'

Make RepoPath group wiki compatible

See merge request gitlab-org/gitlab!30279

app/models/concerns/route_model_query.rb

+# frozen_string_literal: true
+
+# Shared scope between Route and RedirectRoute
+module RouteModelQuery
+  extend ActiveSupport::Concern
+
+  class_methods do
+    def find_source_of_path(path, case_sensitive: true)
+      scope =
+        if case_sensitive
+          where(path: path)
+        else
+          where('LOWER(path) = LOWER(?)', path)
+        end
+
+      scope.first&.source
+    end
+  end
+end

app/models/redirect_route.rb

 # frozen_string_literal: true
 
 class RedirectRoute < ApplicationRecord
+  include RouteModelQuery
+
   belongs_to :source, polymorphic: true # rubocop:disable Cop/PolymorphicAssociations
 
   validates :source, presence: true

app/models/route.rb

@@ -3,6 +3,7 @@
 class Route < ApplicationRecord
   include CaseSensitivity
   include Gitlab::SQL::Pattern
+  include RouteModelQuery
 
   belongs_to :source, polymorphic: true # rubocop:disable Cop/PolymorphicAssociations
   validates :source, presence: true

app/models/snippet.rb

@@ -167,7 +167,11 @@ class Snippet < ApplicationRecord
   end
 
   def self.find_by_id_and_project(id:, project:)
-    Snippet.find_by(id: id, project: project)
+    if project.is_a?(Project)
+      ProjectSnippet.find_by(id: id, project: project)
+    elsif project.nil?
+      PersonalSnippet.find_by(id: id)
+    end
   end
 
   def self.max_file_limit(user)

ee/lib/ee/gitlab/repo_path.rb

@@ -4,10 +4,13 @@ module EE
   module Gitlab
     module RepoPath
       module ClassMethods
-        def find_project(project_path)
+        extend ::Gitlab::Utils::Override
+
+        override :find_routes_source
+        def find_routes_source(path, *args)
           return super unless License.feature_available?(:project_aliases)
 
-          if project_alias = ProjectAlias.find_by_name(project_path)
+          if project_alias = ProjectAlias.find_by_name(path)
             [project_alias.project, nil]
           else
             super

ee/spec/lib/ee/gitlab/repo_path_spec.rb

@@ -3,7 +3,17 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::RepoPath do
-  describe '.find_project' do
+  let_it_be(:group) { create(:group) }
+  let_it_be(:group_redirect_route) { 'foo/bar/baz' }
+  let_it_be(:group_redirect) { group.route.create_redirect(group_redirect_route) }
+
+  describe '.parse' do
+    it 'parses a full path of group wiki' do
+      expect(described_class.parse(group.wiki.repository.full_path)).to eq([group, nil, Gitlab::GlRepository::WIKI, nil])
+    end
+  end
+
+  describe '.find_routes_source' do
     let(:project) { create(:project) }
 
     context 'without premium license' do
@@ -11,7 +21,7 @@ RSpec.describe Gitlab::RepoPath do
         let(:project_alias) { create(:project_alias, project: project) }
 
         it 'does not return a project' do
-          expect(described_class.find_project(project_alias.name)).to eq([nil, nil])
+          expect(described_class.find_routes_source(project_alias.name)).to eq([nil, nil])
         end
       end
     end
@@ -25,23 +35,37 @@ RSpec.describe Gitlab::RepoPath do
         let(:project_alias) { create(:project_alias, project: project) }
 
         it 'returns the project' do
-          expect(described_class.find_project(project_alias.name)).to eq([project, nil])
+          expect(described_class.find_routes_source(project_alias.name)).to eq([project, nil])
         end
       end
 
       context 'project_path does not match a project alias' do
         context 'project path matches project full path' do
           it 'returns the project' do
-            expect(described_class.find_project(project.full_path)).to eq([project, nil])
+            expect(described_class.find_routes_source(project.full_path)).to eq([project, nil])
           end
         end
 
         context 'project path does not match an existing project full path' do
           it 'returns nil' do
-            expect(described_class.find_project('some-project')).to eq([nil, nil])
+            expect(described_class.find_routes_source('some-project')).to eq([nil, nil])
           end
         end
       end
     end
+
+    context 'when target is a group' do
+      context 'when finding by canonical path' do
+        it 'returns the group and nil' do
+          expect(described_class.find_routes_source(group.full_path)).to eq([group, nil])
+        end
+      end
+
+      context 'when finding via a redirect' do
+        it 'returns the group and redirect path' do
+          expect(described_class.find_routes_source(group_redirect.path)).to eq([group, group_redirect_route])
+        end
+      end
+    end
   end
 end

ee/spec/requests/api/internal/base_spec.rb

@@ -256,7 +256,7 @@ RSpec.describe API::Internal::Base do
       end
 
       it 'returns the repository_http_path at the primary node' do
-        expect(Project).to receive(:find_by_full_path).and_return(project)
+        expect(Route).to receive(:find_source_of_path).and_return(project)
 
         lfs_auth_user(user.id, project)
 

lib/gitlab/repo_path.rb

@@ -4,6 +4,11 @@ module Gitlab
   module RepoPath
     NotFoundError = Class.new(StandardError)
 
+    # @return [Array]
+    #   1. container (ActiveRecord which holds repository)
+    #   2. project (Project)
+    #   3. repo_type
+    #   4. redirected_path
     def self.parse(path)
       repo_path = path.sub(/\.git\z/, '').sub(%r{\A/}, '')
       redirected_path = nil
@@ -17,7 +22,7 @@ module Gitlab
         # `Gitlab::GlRepository::PROJECT` type.
         next unless type.valid?(repo_path)
 
-        # Removing the suffix (.wiki, .design, ...) from the project path
+        # Removing the suffix (.wiki, .design, ...) from path
         full_path = repo_path.chomp(type.path_suffix)
         container, project, redirected_path = find_container(type, full_path)
 
@@ -36,23 +41,31 @@ module Gitlab
 
         [snippet, snippet&.project, redirected_path]
       else
-        project, redirected_path = find_project(full_path)
+        container, redirected_path = find_routes_source(full_path)
 
-        [project, project, redirected_path]
+        if container.is_a?(Project)
+          [container, container, redirected_path]
+        else
+          [container, nil, redirected_path]
+        end
       end
     end
 
-    def self.find_project(project_path)
-      return [nil, nil] if project_path.blank?
+    def self.find_routes_source(path)
+      return [nil, nil] if path.blank?
 
-      project = Project.find_by_full_path(project_path, follow_redirects: true)
-      redirected_path = redirected?(project, project_path) ? project_path : nil
+      source =
+        Route.find_source_of_path(path) ||
+        Route.find_source_of_path(path, case_sensitive: false) ||
+        RedirectRoute.find_source_of_path(path, case_sensitive: false)
 
-      [project, redirected_path]
+      redirected_path = redirected?(source, path) ? path : nil
+
+      [source, redirected_path]
     end
 
-    def self.redirected?(project, project_path)
-      project && project.full_path.casecmp(project_path) != 0
+    def self.redirected?(container, container_path)
+      container && container.full_path.casecmp(container_path) != 0
     end
 
     # Snippet_path can be either:
@@ -62,7 +75,7 @@ module Gitlab
       return [nil, nil] if snippet_path.blank?
 
       snippet_id, project_path = extract_snippet_info(snippet_path)
-      project, redirected_path = find_project(project_path)
+      project, redirected_path = find_routes_source(project_path)
 
       [Snippet.find_by_id_and_project(id: snippet_id, project: project), redirected_path]
     end

spec/lib/gitlab/repo_path_spec.rb

@@ -67,11 +67,11 @@ describe ::Gitlab::RepoPath do
     end
   end
 
-  describe '.find_project' do
+  describe '.find_routes_source' do
     context 'when finding a project by its canonical path' do
       context 'when the cases match' do
         it 'returns the project and nil' do
-          expect(described_class.find_project(project.full_path)).to eq([project, nil])
+          expect(described_class.find_routes_source(project.full_path)).to eq([project, nil])
         end
       end
 
@@ -81,14 +81,14 @@ describe ::Gitlab::RepoPath do
         # requests, we should accept wrongly-cased URLs because it is a pain to
         # block people's git operations and force them to update remote URLs.
         it 'returns the project and nil' do
-          expect(described_class.find_project(project.full_path.upcase)).to eq([project, nil])
+          expect(described_class.find_routes_source(project.full_path.upcase)).to eq([project, nil])
         end
       end
     end
 
     context 'when finding a project via a redirect' do
       it 'returns the project and nil' do
-        expect(described_class.find_project(redirect.path)).to eq([project, redirect.path])
+        expect(described_class.find_routes_source(redirect.path)).to eq([project, redirect.path])
       end
     end
   end
@@ -110,6 +110,16 @@ describe ::Gitlab::RepoPath do
       end
     end
 
+    context 'when path is namespace path, but has same id as project' do
+      let(:namespace) { build_stubbed(:namespace, id: project.id) }
+
+      it 'returns nil if path is referring to namespace' do
+        allow(described_class).to receive(:find_route_source).and_return(namespace)
+
+        expect(described_class.find_snippet("#{namespace.full_path}/snippets/#{project_snippet.id}")).to eq([nil, nil])
+      end
+    end
+
     it 'returns nil for snippets not associated with the project' do
       snippet = create(:project_snippet)
 

spec/models/concerns/route_model_query_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Route, 'RouteModelQuery', :aggregate_failures do
+  let_it_be(:group1) { create(:group, path: 'Group1') }
+  let_it_be(:group2) { create(:group, path: 'Group2') }
+  let_it_be(:project1) { create(:project, path: 'Project1', group: group1) }
+  let_it_be(:project2) { create(:project, path: 'Project2', group: group2) }
+
+  describe '.find_source_of_path' do
+    it 'finds exact match' do
+      expect(described_class.find_source_of_path('Group1')).to eq(group1)
+      expect(described_class.find_source_of_path('Group2/Project2')).to eq(project2)
+
+      expect(described_class.find_source_of_path('GROUP1')).to be_nil
+      expect(described_class.find_source_of_path('GROUP2/PROJECT2')).to be_nil
+    end
+
+    it 'finds case insensitive match' do
+      expect(described_class.find_source_of_path('Group1', case_sensitive: false)).to eq(group1)
+      expect(described_class.find_source_of_path('Group2/Project2', case_sensitive: false)).to eq(project2)
+
+      expect(described_class.find_source_of_path('GROUP1', case_sensitive: false)).to eq(group1)
+      expect(described_class.find_source_of_path('GROUP2/PROJECT2', case_sensitive: false)).to eq(project2)
+    end
+  end
+end

spec/models/snippet_spec.rb

@@ -206,6 +206,32 @@ describe Snippet do
     end
   end
 
+  describe '.find_by_id_and_project' do
+    let_it_be(:project) { create(:project) }
+    let_it_be(:project_snippet) { create(:project_snippet, project: project) }
+    let_it_be(:personal_snippet) { create(:personal_snippet) }
+
+    context 'when project is provided' do
+      it 'returns ProjectSnippet' do
+        expect(described_class.find_by_id_and_project(id: project_snippet.id, project: project)).to eq(project_snippet)
+      end
+    end
+
+    context 'when project is nil' do
+      it 'returns PersonalSnippet' do
+        expect(described_class.find_by_id_and_project(id: personal_snippet.id, project: nil)).to eq(personal_snippet)
+      end
+    end
+
+    context 'when project variable is not a Project' do
+      let(:namespace) { build_stubbed(:namespace, id: project.id) }
+
+      it 'returns nil' do
+        expect(described_class.find_by_id_and_project(id: project_snippet.id, project: namespace)).to be_nil
+      end
+    end
+  end
+
   describe '.with_optional_visibility' do
     context 'when a visibility level is provided' do
       it 'returns snippets with the given visibility' do