Commit 10afe1b0 authored by Markus Koller's avatar Markus Koller

Support more types in UrlBuilder

- Add support for projects and groups.
- Add support for building path-only URLs.
- Reuse it in `HasRepository#web_url`.
- Remove state and turn the class into a singleton.
- Provide a default instance in presenters.
- Rewrite specs with parameterized table syntax.
parent cf76d567
...@@ -110,7 +110,7 @@ module HasRepository ...@@ -110,7 +110,7 @@ module HasRepository
end end
def web_url(only_path: nil) def web_url(only_path: nil)
raise NotImplementedError Gitlab::UrlBuilder.build(self, only_path: only_path)
end end
def repository_size_checker def repository_size_checker
......
...@@ -172,8 +172,8 @@ class Group < Namespace ...@@ -172,8 +172,8 @@ class Group < Namespace
"#{self.class.reference_prefix}#{full_path}" "#{self.class.reference_prefix}#{full_path}"
end end
def web_url def web_url(only_path: nil)
Gitlab::Routing.url_helpers.group_canonical_url(self) Gitlab::UrlBuilder.build(self, only_path: only_path)
end end
def human_name def human_name
......
...@@ -2,8 +2,4 @@ ...@@ -2,8 +2,4 @@
class PersonalSnippet < Snippet class PersonalSnippet < Snippet
include WithUploads include WithUploads
def web_url(only_path: nil)
Gitlab::Routing.url_helpers.snippet_url(self, only_path: only_path)
end
end end
...@@ -1121,10 +1121,6 @@ class Project < ApplicationRecord ...@@ -1121,10 +1121,6 @@ class Project < ApplicationRecord
end end
end end
def web_url(only_path: nil)
Gitlab::Routing.url_helpers.project_url(self, only_path: only_path)
end
def readme_url def readme_url
readme_path = repository.readme_path readme_path = repository.readme_path
if readme_path if readme_path
......
...@@ -5,8 +5,4 @@ class ProjectSnippet < Snippet ...@@ -5,8 +5,4 @@ class ProjectSnippet < Snippet
validates :project, presence: true validates :project, presence: true
validates :secret, inclusion: { in: [false] } validates :secret, inclusion: { in: [false] }
def web_url(only_path: nil)
Gitlab::Routing.url_helpers.project_snippet_url(project, self, only_path: only_path)
end
end end
...@@ -44,8 +44,8 @@ class ProjectWiki ...@@ -44,8 +44,8 @@ class ProjectWiki
# @deprecated use full_path when you need it for an URL route or disk_path when you want to point to the filesystem # @deprecated use full_path when you need it for an URL route or disk_path when you want to point to the filesystem
alias_method :path_with_namespace, :full_path alias_method :path_with_namespace, :full_path
def web_url def web_url(only_path: nil)
Gitlab::Routing.url_helpers.project_wiki_url(@project, :home) Gitlab::UrlBuilder.build(self, only_path: only_path)
end end
def url_to_repo def url_to_repo
......
...@@ -18,7 +18,7 @@ class CommitPresenter < Gitlab::View::Presenter::Delegated ...@@ -18,7 +18,7 @@ class CommitPresenter < Gitlab::View::Presenter::Delegated
end end
def web_url def web_url
Gitlab::UrlBuilder.new(commit).url url_builder.build(commit)
end end
def signature_html def signature_html
......
...@@ -4,20 +4,14 @@ class IssuePresenter < Gitlab::View::Presenter::Delegated ...@@ -4,20 +4,14 @@ class IssuePresenter < Gitlab::View::Presenter::Delegated
presents :issue presents :issue
def web_url def web_url
url_builder.url url_builder.build(issue)
end end
def issue_path def issue_path
url_builder.issue_path(issue) url_builder.build(issue, only_path: true)
end end
def subscribed? def subscribed?
issue.subscribed?(current_user, issue.project) issue.subscribed?(current_user, issue.project)
end end
private
def url_builder
@url_builder ||= Gitlab::UrlBuilder.new(issue)
end
end end
...@@ -4,12 +4,6 @@ class MilestonePresenter < Gitlab::View::Presenter::Delegated ...@@ -4,12 +4,6 @@ class MilestonePresenter < Gitlab::View::Presenter::Delegated
presents :milestone presents :milestone
def milestone_path def milestone_path
url_builder.milestone_path(milestone) url_builder.build(milestone, only_path: true)
end
private
def url_builder
@url_builder ||= Gitlab::UrlBuilder.new(milestone)
end end
end end
...@@ -11,10 +11,6 @@ class EpicIssuePresenter < Gitlab::View::Presenter::Delegated ...@@ -11,10 +11,6 @@ class EpicIssuePresenter < Gitlab::View::Presenter::Delegated
private private
def url_builder
@url_builder ||= Gitlab::UrlBuilder.new(issue)
end
def can_admin_issue_link?(current_user) def can_admin_issue_link?(current_user)
Ability.allowed?(current_user, :admin_epic_issue, issue) && Ability.allowed?(current_user, :admin_epic, issue.epic) Ability.allowed?(current_user, :admin_epic_issue, issue) && Ability.allowed?(current_user, :admin_epic, issue.epic)
end end
......
...@@ -14,11 +14,11 @@ class EpicPresenter < Gitlab::View::Presenter::Delegated ...@@ -14,11 +14,11 @@ class EpicPresenter < Gitlab::View::Presenter::Delegated
end end
def group_epic_path def group_epic_path
url_builder.group_epic_path(epic.group, epic) url_builder.build(epic, only_path: true)
end end
def group_epic_url def group_epic_url
url_builder.group_epic_url(epic.group, epic) url_builder.build(epic)
end end
def group_epic_link_path def group_epic_link_path
...@@ -146,9 +146,4 @@ class EpicPresenter < Gitlab::View::Presenter::Delegated ...@@ -146,9 +146,4 @@ class EpicPresenter < Gitlab::View::Presenter::Delegated
} }
end end
end end
# important for using routing helpers in GraphQL
def url_builder
@url_builder ||= Gitlab::UrlBuilder.new(epic)
end
end end
...@@ -3,45 +3,47 @@ ...@@ -3,45 +3,47 @@
module EE module EE
module Gitlab module Gitlab
module UrlBuilder module UrlBuilder
extend ::Gitlab::Utils::Override extend ActiveSupport::Concern
override :url class_methods do
def url extend ::Gitlab::Utils::Override
case object
when ::DesignManagement::Design override :build
design_url def build(object, **options)
when Epic case object.itself
group_epic_url(object.group, object) when ::DesignManagement::Design
when Vulnerability design_url(object, **options)
project_security_vulnerability_url(object.project, object) when Epic
else instance.group_epic_url(object.group, object, **options)
super when Vulnerability
instance.project_security_vulnerability_url(object.project, object, **options)
else
super
end
end end
end
override :note_url override :note_url
def note_url def note_url(note, **options)
noteable = object.noteable noteable = note.noteable
if object.for_epic? if note.for_epic?
group_epic_url(noteable.group, noteable, anchor: dom_id(object)) instance.group_epic_url(noteable.group, noteable, anchor: dom_id(note), **options)
elsif object.for_vulnerability? elsif note.for_vulnerability?
project_security_vulnerability_url(noteable.project, noteable, anchor: dom_id(object)) instance.project_security_vulnerability_url(noteable.project, noteable, anchor: dom_id(note), **options)
else else
super super
end
end end
end
private
def design_url def design_url(design, **options)
size, ref = opts.values_at(:size, :ref) size, ref = options.values_at(:size, :ref)
design = object options.except!(:size, :ref)
if size if size
project_design_management_designs_resized_image_url(design.project, design, ref, size) instance.project_design_management_designs_resized_image_url(design.project, design, ref, size, **options)
else else
project_design_management_designs_raw_image_url(design.project, design, ref) instance.project_design_management_designs_raw_image_url(design.project, design, ref, **options)
end
end end
end end
end end
......
...@@ -3,63 +3,40 @@ ...@@ -3,63 +3,40 @@
require 'spec_helper' require 'spec_helper'
describe Gitlab::UrlBuilder do describe Gitlab::UrlBuilder do
describe '.build' do subject { described_class }
context 'when passing a DesignManagement::Design' do
it 'returns a proper URL to the raw (unresized) image' do
design = build_stubbed(:design)
url = described_class.build(design, ref: 'master')
expect(url).to eq "#{Settings.gitlab['url']}/#{design.project.full_path}/-/design_management/designs/#{design.id}/master/raw_image"
end
it 'returns a proper URL to the resized image' do describe '.build' do
design = build_stubbed(:design) using RSpec::Parameterized::TableSyntax
url = described_class.build(design, ref: 'master', size: 'small') where(:factory, :path_generator) do
:design | ->(design) { "/#{design.project.full_path}/-/design_management/designs/#{design.id}/raw_image" }
:epic | ->(epic) { "/groups/#{epic.group.full_path}/-/epics/#{epic.iid}" }
:vulnerability | ->(vulnerability) { "/#{vulnerability.project.full_path}/-/security/vulnerabilities/#{vulnerability.id}" }
expect(url).to eq "#{Settings.gitlab['url']}/#{design.project.full_path}/-/design_management/designs/#{design.id}/master/resized_image/small" :note_on_epic | ->(note) { "/groups/#{note.noteable.group.full_path}/-/epics/#{note.noteable.iid}#note_#{note.id}" }
end :note_on_vulnerability | ->(note) { "/#{note.project.full_path}/-/security/vulnerabilities/#{note.noteable.id}#note_#{note.id}" }
end end
context 'when passing an epic' do with_them do
it 'returns a proper URL' do let(:object) { build_stubbed(factory) }
epic = build_stubbed(:epic, iid: 42) let(:path) { path_generator.call(object) }
url = described_class.build(epic)
expect(url).to eq "#{Settings.gitlab['url']}/groups/#{epic.group.full_path}/-/epics/#{epic.iid}" it 'returns the full URL' do
expect(subject.build(object)).to eq("#{Settings.gitlab['url']}#{path}")
end end
end
context 'when passing an epic note' do
it 'returns a proper URL' do
epic = create(:epic)
note = build_stubbed(:note_on_epic, noteable: epic)
url = described_class.build(note) it 'returns only the path if only_path is set' do
expect(subject.build(object, only_path: true)).to eq(path)
expect(url).to eq "#{Settings.gitlab['url']}/groups/#{epic.group.full_path}/-/epics/#{epic.iid}#note_#{note.id}"
end end
end end
context 'when passing a vulnerability' do context 'when passing a DesignManagement::Design' do
it 'returns a proper URL' do let(:design) { build_stubbed(:design) }
vulnerability = build_stubbed(:vulnerability, id: 42)
url = described_class.build(vulnerability)
expect(url).to eq "#{Settings.gitlab['url']}/#{vulnerability.project.full_path}/-/security/vulnerabilities/#{vulnerability.id}"
end
end
context 'when passing a vulnerability note' do it 'uses the given ref and size in the URL' do
it 'returns a proper URL' do url = subject.build(design, ref: 'feature', size: 'small')
vulnerability = create(:vulnerability)
note = build_stubbed(:note_on_vulnerability, noteable: vulnerability)
url = described_class.build(note)
expect(url).to eq "#{Settings.gitlab['url']}/#{vulnerability.project.full_path}/-/security/vulnerabilities/#{vulnerability.id}#note_#{note.id}" expect(url).to eq "#{Settings.gitlab['url']}/#{design.project.full_path}/-/design_management/designs/#{design.id}/feature/resized_image/small"
end end
end end
end end
......
...@@ -2,76 +2,74 @@ ...@@ -2,76 +2,74 @@
module Gitlab module Gitlab
class UrlBuilder class UrlBuilder
include Singleton
include Gitlab::Routing include Gitlab::Routing
include GitlabRoutingHelper include GitlabRoutingHelper
include ActionView::RecordIdentifier
attr_reader :object, :opts delegate :build, to: :class
def self.build(object, opts = {}) class << self
new(object, opts).url include ActionView::RecordIdentifier
end
def url def build(object, **options)
# Objects are sometimes wrapped in a BatchLoader instance # Objects are sometimes wrapped in a BatchLoader instance
case object.itself case object.itself
when Commit when ::Ci::Build
commit_url instance.project_job_url(object.project, object, **options)
when Issue when Commit
issue_url(object) commit_url(object, **options)
when MergeRequest when Group
merge_request_url(object) instance.group_canonical_url(object, **options)
when Note when Issue
note_url instance.issue_url(object, **options)
when WikiPage when MergeRequest
wiki_page_url instance.merge_request_url(object, **options)
when Snippet when Milestone
opts[:raw].present? ? gitlab_raw_snippet_url(object) : gitlab_snippet_url(object) instance.milestone_url(object, **options)
when Milestone when Note
milestone_url(object) note_url(object, **options)
when ::Ci::Build when Project
project_job_url(object.project, object) instance.project_url(object, **options)
when User when Snippet
user_url(object) snippet_url(object, **options)
else when User
raise NotImplementedError.new("No URL builder defined for #{object.inspect}") instance.user_url(object, **options)
when ProjectWiki
instance.project_wiki_url(object.project, :home, **options)
when WikiPage
instance.project_wiki_url(object.wiki.project, object.slug, **options)
else
raise NotImplementedError.new("No URL builder defined for #{object.inspect}")
end
end end
end
private
def initialize(object, opts = {}) def commit_url(commit, **options)
@object = object return '' unless commit.project
@opts = opts
end
def commit_url(opts = {})
return '' if object.project.nil?
namespace_project_commit_url({ instance.commit_url(commit, **options)
namespace_id: object.project.namespace, end
project_id: object.project,
id: object.id
}.merge!(opts))
end
def note_url
if object.for_commit?
commit_url(id: object.commit_id, anchor: dom_id(object))
elsif object.for_issue?
issue_url(object.noteable, anchor: dom_id(object))
elsif object.for_merge_request? def note_url(note, **options)
merge_request_url(object.noteable, anchor: dom_id(object)) if note.for_commit?
return '' unless note.project
elsif object.for_snippet? instance.project_commit_url(note.project, note.commit_id, anchor: dom_id(note), **options)
gitlab_snippet_url(object.noteable, anchor: dom_id(object)) elsif note.for_issue?
instance.issue_url(note.noteable, anchor: dom_id(note), **options)
elsif note.for_merge_request?
instance.merge_request_url(note.noteable, anchor: dom_id(note), **options)
elsif note.for_snippet?
instance.gitlab_snippet_url(note.noteable, anchor: dom_id(note), **options)
end
end end
end
def wiki_page_url def snippet_url(snippet, **options)
project_wiki_url(object.wiki.project, object.slug) if options.delete(:raw).present?
instance.gitlab_raw_snippet_url(snippet, **options)
else
instance.gitlab_snippet_url(snippet, **options)
end
end
end end
end end
end end
......
...@@ -26,6 +26,10 @@ module Gitlab ...@@ -26,6 +26,10 @@ module Gitlab
self self
end end
def url_builder
Gitlab::UrlBuilder.instance
end
class_methods do class_methods do
def presenter? def presenter?
true true
......
...@@ -25,6 +25,14 @@ FactoryBot.define do ...@@ -25,6 +25,14 @@ FactoryBot.define do
due_date { Date.new(2000, 1, 30) } due_date { Date.new(2000, 1, 30) }
end end
trait :on_project do
project
end
trait :on_group do
group
end
after(:build, :stub) do |milestone, evaluator| after(:build, :stub) do |milestone, evaluator|
if evaluator.group if evaluator.group
milestone.group = evaluator.group milestone.group = evaluator.group
...@@ -44,5 +52,7 @@ FactoryBot.define do ...@@ -44,5 +52,7 @@ FactoryBot.define do
factory :active_milestone, traits: [:active] factory :active_milestone, traits: [:active]
factory :closed_milestone, traits: [:closed] factory :closed_milestone, traits: [:closed]
factory :project_milestone, traits: [:on_project]
factory :group_milestone, traits: [:on_group]
end end
end end
This diff is collapsed.
...@@ -21,8 +21,6 @@ describe PersonalSnippet do ...@@ -21,8 +21,6 @@ describe PersonalSnippet do
let_it_be(:container) { create(:personal_snippet, :repository) } let_it_be(:container) { create(:personal_snippet, :repository) }
let(:stubbed_container) { build_stubbed(:personal_snippet) } let(:stubbed_container) { build_stubbed(:personal_snippet) }
let(:expected_full_path) { "@snippets/#{container.id}" } let(:expected_full_path) { "@snippets/#{container.id}" }
let(:expected_repository_klass) { Repository }
let(:expected_storage_klass) { Storage::Hashed }
let(:expected_web_url_path) { "snippets/#{container.id}" } let(:expected_web_url_path) { "snippets/#{container.id}" }
end end
end end
...@@ -37,8 +37,6 @@ describe ProjectSnippet do ...@@ -37,8 +37,6 @@ describe ProjectSnippet do
let_it_be(:container) { create(:project_snippet, :repository) } let_it_be(:container) { create(:project_snippet, :repository) }
let(:stubbed_container) { build_stubbed(:project_snippet) } let(:stubbed_container) { build_stubbed(:project_snippet) }
let(:expected_full_path) { "#{container.project.full_path}/@snippets/#{container.id}" } let(:expected_full_path) { "#{container.project.full_path}/@snippets/#{container.id}" }
let(:expected_repository_klass) { Repository }
let(:expected_storage_klass) { Storage::Hashed }
let(:expected_web_url_path) { "#{container.project.full_path}/snippets/#{container.id}" } let(:expected_web_url_path) { "#{container.project.full_path}/snippets/#{container.id}" }
end end
end end
...@@ -116,9 +116,6 @@ describe Project do ...@@ -116,9 +116,6 @@ describe Project do
let_it_be(:container) { create(:project, :repository, path: 'somewhere') } let_it_be(:container) { create(:project, :repository, path: 'somewhere') }
let(:stubbed_container) { build_stubbed(:project) } let(:stubbed_container) { build_stubbed(:project) }
let(:expected_full_path) { "#{container.namespace.full_path}/somewhere" } let(:expected_full_path) { "#{container.namespace.full_path}/somewhere" }
let(:expected_repository_klass) { Repository }
let(:expected_storage_klass) { Storage::Hashed }
let(:expected_web_url_path) { "#{container.namespace.full_path}/somewhere" }
end end
it 'has an inverse relationship with merge requests' do it 'has an inverse relationship with merge requests' do
......
...@@ -28,7 +28,7 @@ describe ProjectWiki do ...@@ -28,7 +28,7 @@ describe ProjectWiki do
describe '#web_url' do describe '#web_url' do
it 'returns the full web URL to the wiki' do it 'returns the full web URL to the wiki' do
expect(subject.web_url).to eq("#{Gitlab.config.gitlab.url}/#{project.full_path}/-/wikis/home") expect(subject.web_url).to eq(Gitlab::UrlBuilder.build(subject))
end end
end end
......
# frozen_string_literal: true # frozen_string_literal: true
RSpec.shared_examples 'model with repository' do RSpec.shared_examples 'model with repository' do
let(:container) { raise NotImplementedError }
let(:stubbed_container) { raise NotImplementedError }
let(:expected_full_path) { raise NotImplementedError }
let(:expected_web_url_path) { expected_full_path }
describe '#commits_by' do describe '#commits_by' do
let(:commits) { container.repository.commits('HEAD', limit: 3).commits } let(:commits) { container.repository.commits('HEAD', limit: 3).commits }
let(:commit_shas) { commits.map(&:id) } let(:commit_shas) { commits.map(&:id) }
...@@ -46,74 +51,33 @@ RSpec.shared_examples 'model with repository' do ...@@ -46,74 +51,33 @@ RSpec.shared_examples 'model with repository' do
end end
end end
describe '#ssh_url_to_repo' do describe '#url_to_repo' do
it 'returns container ssh address' do it 'returns the SSH URL to the repository' do
expect(container.ssh_url_to_repo).to eq container.url_to_repo expect(container.url_to_repo).to eq("#{Gitlab.config.gitlab_shell.ssh_path_prefix}#{expected_web_url_path}.git")
end end
end end
describe '#http_url_to_repo' do describe '#ssh_url_to_repo' do
subject { container.http_url_to_repo } it 'returns the SSH URL to the repository' do
expect(container.ssh_url_to_repo).to eq(container.url_to_repo)
context 'when a custom HTTP clone URL root is not set' do
it 'returns the url to the repo without a username' do
expect(subject).to eq("#{container.web_url}.git")
expect(subject).not_to include('@')
end
end end
end
context 'when a custom HTTP clone URL root is set' do describe '#http_url_to_repo' do
before do it 'returns the HTTP URL to the repository' do
stub_application_setting(custom_http_clone_url_root: custom_http_clone_url_root) expect(container.http_url_to_repo).to eq("#{Gitlab.config.gitlab.url}/#{expected_web_url_path}.git")
end
context 'when custom HTTP clone URL root has a relative URL root' do
context 'when custom HTTP clone URL root ends with a slash' do
let(:custom_http_clone_url_root) { 'https://git.example.com:51234/mygitlab/' }
it 'returns the url to the repo, with the root replaced with the custom one' do
expect(subject).to eq("#{custom_http_clone_url_root}#{expected_web_url_path}.git")
end
end
context 'when custom HTTP clone URL root does not end with a slash' do
let(:custom_http_clone_url_root) { 'https://git.example.com:51234/mygitlab' }
it 'returns the url to the repo, with the root replaced with the custom one' do
expect(subject).to eq("#{custom_http_clone_url_root}/#{expected_web_url_path}.git")
end
end
end
context 'when custom HTTP clone URL root does not have a relative URL root' do
context 'when custom HTTP clone URL root ends with a slash' do
let(:custom_http_clone_url_root) { 'https://git.example.com:51234/' }
it 'returns the url to the repo, with the root replaced with the custom one' do
expect(subject).to eq("#{custom_http_clone_url_root}#{expected_web_url_path}.git")
end
end
context 'when custom HTTP clone URL root does not end with a slash' do
let(:custom_http_clone_url_root) { 'https://git.example.com:51234' }
it 'returns the url to the repo, with the root replaced with the custom one' do
expect(subject).to eq("#{custom_http_clone_url_root}/#{expected_web_url_path}.git")
end
end
end
end end
end end
describe '#repository' do describe '#repository' do
it 'returns valid repo' do it 'returns valid repo' do
expect(container.repository).to be_kind_of(expected_repository_klass) expect(container.repository).to be_kind_of(Repository)
end end
end end
describe '#storage' do describe '#storage' do
it 'returns valid storage' do it 'returns valid storage' do
expect(container.storage).to be_kind_of(expected_storage_klass) expect(container.storage).to be_kind_of(Storage::Hashed)
end end
end end
......
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