Commit 606aab9e authored by Mark Chao's avatar Mark Chao

Allow RepoPath to query group container

Determine source from any path,
when a path can either be a group or project.
parent ac4de842
# 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
# frozen_string_literal: true # frozen_string_literal: true
class RedirectRoute < ApplicationRecord class RedirectRoute < ApplicationRecord
include RouteModelQuery
belongs_to :source, polymorphic: true # rubocop:disable Cop/PolymorphicAssociations belongs_to :source, polymorphic: true # rubocop:disable Cop/PolymorphicAssociations
validates :source, presence: true validates :source, presence: true
......
...@@ -3,6 +3,7 @@ ...@@ -3,6 +3,7 @@
class Route < ApplicationRecord class Route < ApplicationRecord
include CaseSensitivity include CaseSensitivity
include Gitlab::SQL::Pattern include Gitlab::SQL::Pattern
include RouteModelQuery
belongs_to :source, polymorphic: true # rubocop:disable Cop/PolymorphicAssociations belongs_to :source, polymorphic: true # rubocop:disable Cop/PolymorphicAssociations
validates :source, presence: true validates :source, presence: true
......
...@@ -167,7 +167,11 @@ class Snippet < ApplicationRecord ...@@ -167,7 +167,11 @@ class Snippet < ApplicationRecord
end end
def self.find_by_id_and_project(id:, project:) 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 end
def self.max_file_limit(user) def self.max_file_limit(user)
......
...@@ -4,10 +4,13 @@ module EE ...@@ -4,10 +4,13 @@ module EE
module Gitlab module Gitlab
module RepoPath module RepoPath
module ClassMethods 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) 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] [project_alias.project, nil]
else else
super super
......
...@@ -3,7 +3,17 @@ ...@@ -3,7 +3,17 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Gitlab::RepoPath do 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) } let(:project) { create(:project) }
context 'without premium license' do context 'without premium license' do
...@@ -11,7 +21,7 @@ RSpec.describe Gitlab::RepoPath do ...@@ -11,7 +21,7 @@ RSpec.describe Gitlab::RepoPath do
let(:project_alias) { create(:project_alias, project: project) } let(:project_alias) { create(:project_alias, project: project) }
it 'does not return a project' do 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 end
end end
...@@ -25,22 +35,36 @@ RSpec.describe Gitlab::RepoPath do ...@@ -25,22 +35,36 @@ RSpec.describe Gitlab::RepoPath do
let(:project_alias) { create(:project_alias, project: project) } let(:project_alias) { create(:project_alias, project: project) }
it 'returns the project' do 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
end end
context 'project_path does not match a project alias' do context 'project_path does not match a project alias' do
context 'project path matches project full path' do context 'project path matches project full path' do
it 'returns the project' 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
end end
context 'project path does not match an existing project full path' do context 'project path does not match an existing project full path' do
it 'returns nil' 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
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
end end
......
...@@ -256,7 +256,7 @@ RSpec.describe API::Internal::Base do ...@@ -256,7 +256,7 @@ RSpec.describe API::Internal::Base do
end end
it 'returns the repository_http_path at the primary node' do 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) lfs_auth_user(user.id, project)
......
...@@ -4,6 +4,11 @@ module Gitlab ...@@ -4,6 +4,11 @@ module Gitlab
module RepoPath module RepoPath
NotFoundError = Class.new(StandardError) 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) def self.parse(path)
repo_path = path.sub(/\.git\z/, '').sub(%r{\A/}, '') repo_path = path.sub(/\.git\z/, '').sub(%r{\A/}, '')
redirected_path = nil redirected_path = nil
...@@ -17,7 +22,7 @@ module Gitlab ...@@ -17,7 +22,7 @@ module Gitlab
# `Gitlab::GlRepository::PROJECT` type. # `Gitlab::GlRepository::PROJECT` type.
next unless type.valid?(repo_path) 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) full_path = repo_path.chomp(type.path_suffix)
container, project, redirected_path = find_container(type, full_path) container, project, redirected_path = find_container(type, full_path)
...@@ -36,23 +41,31 @@ module Gitlab ...@@ -36,23 +41,31 @@ module Gitlab
[snippet, snippet&.project, redirected_path] [snippet, snippet&.project, redirected_path]
else 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
end end
def self.find_project(project_path) def self.find_routes_source(path)
return [nil, nil] if project_path.blank? return [nil, nil] if path.blank?
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 = Project.find_by_full_path(project_path, follow_redirects: true) redirected_path = redirected?(source, path) ? path : nil
redirected_path = redirected?(project, project_path) ? project_path : nil
[project, redirected_path] [source, redirected_path]
end end
def self.redirected?(project, project_path) def self.redirected?(container, container_path)
project && project.full_path.casecmp(project_path) != 0 container && container.full_path.casecmp(container_path) != 0
end end
# Snippet_path can be either: # Snippet_path can be either:
...@@ -62,7 +75,7 @@ module Gitlab ...@@ -62,7 +75,7 @@ module Gitlab
return [nil, nil] if snippet_path.blank? return [nil, nil] if snippet_path.blank?
snippet_id, project_path = extract_snippet_info(snippet_path) 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] [Snippet.find_by_id_and_project(id: snippet_id, project: project), redirected_path]
end end
......
...@@ -67,11 +67,11 @@ describe ::Gitlab::RepoPath do ...@@ -67,11 +67,11 @@ describe ::Gitlab::RepoPath do
end end
end end
describe '.find_project' do describe '.find_routes_source' do
context 'when finding a project by its canonical path' do context 'when finding a project by its canonical path' do
context 'when the cases match' do context 'when the cases match' do
it 'returns the project and nil' 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
end end
...@@ -81,14 +81,14 @@ describe ::Gitlab::RepoPath do ...@@ -81,14 +81,14 @@ describe ::Gitlab::RepoPath do
# requests, we should accept wrongly-cased URLs because it is a pain to # 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. # block people's git operations and force them to update remote URLs.
it 'returns the project and nil' do 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 end
end end
context 'when finding a project via a redirect' do context 'when finding a project via a redirect' do
it 'returns the project and nil' 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 end
end end
...@@ -110,6 +110,16 @@ describe ::Gitlab::RepoPath do ...@@ -110,6 +110,16 @@ describe ::Gitlab::RepoPath do
end end
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 it 'returns nil for snippets not associated with the project' do
snippet = create(:project_snippet) snippet = create(:project_snippet)
......
# 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
...@@ -206,6 +206,32 @@ describe Snippet do ...@@ -206,6 +206,32 @@ describe Snippet do
end end
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 describe '.with_optional_visibility' do
context 'when a visibility level is provided' do context 'when a visibility level is provided' do
it 'returns snippets with the given visibility' do it 'returns snippets with the given visibility' do
......
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