Commit 99f64064 authored by fjsanpedro's avatar fjsanpedro

Generate sitemap routes statically

In this MR, in order to improve the request time,
we generate the Sitemap routes statically instead of
dynamically.
parent 2e3ed169
---
title: Generate Sitemap routes statically
merge_request: 47174
author:
type: changed
# frozen_string_literal: true # frozen_string_literal: true
# Generating the urls for the project and groups is the most # Routes have been hardcoded in order to improve performance.
# expensive part of the sitemap generation because we need
# to call the Rails route helpers.
#
# We could hardcode them but if a route changes the sitemap
# urls will be invalid.
module Gitlab module Gitlab
module Sitemaps module Sitemaps
class UrlExtractor class UrlExtractor
...@@ -24,25 +19,36 @@ module Gitlab ...@@ -24,25 +19,36 @@ module Gitlab
end end
def extract_from_group(group) def extract_from_group(group)
full_path = group.full_path
[ [
group_url(group), "#{base_url}#{full_path}",
issues_group_url(group), "#{base_url}groups/#{full_path}/-/issues",
merge_requests_group_url(group), "#{base_url}groups/#{full_path}/-/merge_requests",
group_packages_url(group), "#{base_url}groups/#{full_path}/-/packages"
group_epics_url(group) ].tap do |urls|
] urls << "#{base_url}groups/#{full_path}/-/epics" if group.feature_available?(:epics)
end
end end
def extract_from_project(project) def extract_from_project(project)
full_path = project.full_path
[ [
project_url(project), "#{base_url}#{full_path}"
project_issues_url(project),
project_merge_requests_url(project)
].tap do |urls| ].tap do |urls|
urls << project_snippets_url(project) if project.snippets_enabled? urls << "#{base_url}#{full_path}/-/merge_requests" if project.feature_available?(:merge_requests, nil)
urls << project_wiki_url(project, Wiki::HOMEPAGE) if project.wiki_enabled? urls << "#{base_url}#{full_path}/-/issues" if project.feature_available?(:issues, nil)
urls << "#{base_url}#{full_path}/-/snippets" if project.feature_available?(:snippets, nil)
urls << "#{base_url}#{full_path}/-/wikis/home" if project.feature_available?(:wiki, nil)
end end
end end
private
def base_url
@base_url ||= root_url
end
end end
end end
end end
......
...@@ -3,6 +3,8 @@ ...@@ -3,6 +3,8 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Gitlab::Sitemaps::UrlExtractor do RSpec.describe Gitlab::Sitemaps::UrlExtractor do
include Gitlab::Routing
before do before do
stub_default_url_options(host: 'localhost') stub_default_url_options(host: 'localhost')
end end
...@@ -53,48 +55,115 @@ RSpec.describe Gitlab::Sitemaps::UrlExtractor do ...@@ -53,48 +55,115 @@ RSpec.describe Gitlab::Sitemaps::UrlExtractor do
subject { described_class.extract_from_group(group) } subject { described_class.extract_from_group(group) }
it 'returns several group urls' do it 'returns several group urls' do
stub_licensed_features(epics: true)
expected_urls = [ expected_urls = [
"http://localhost/#{group.full_path}", group_url(group),
"http://localhost/groups/#{group.full_path}/-/issues", issues_group_url(group),
"http://localhost/groups/#{group.full_path}/-/merge_requests", merge_requests_group_url(group),
"http://localhost/groups/#{group.full_path}/-/packages", group_packages_url(group),
"http://localhost/groups/#{group.full_path}/-/epics" group_epics_url(group)
] ]
expect(subject).to match_array(expected_urls) expect(subject).to match_array(expected_urls)
end end
context 'when epics are not enabled' do
it 'does nor include epics url' do
stub_licensed_features(epics: false)
expect(subject).not_to include(group_epics_url(group))
end
end
end end
describe '.extract_from_project' do describe '.extract_from_project' do
let(:project) { build(:project) } let_it_be_with_reload(:project) { create(:project) }
let(:project_feature) { project.project_feature }
subject { described_class.extract_from_project(project) } subject { described_class.extract_from_project(project) }
it 'returns several project urls' do it 'returns several project urls' do
expected_urls = [ expected_urls = [
"http://localhost/#{project.full_path}", project_url(project),
"http://localhost/#{project.full_path}/-/issues", project_issues_url(project),
"http://localhost/#{project.full_path}/-/merge_requests", project_merge_requests_url(project),
"http://localhost/#{project.full_path}/-/snippets", project_snippets_url(project),
"http://localhost/#{project.full_path}/-/wikis/home" project_wiki_url(project, Wiki::HOMEPAGE)
] ]
expect(subject).to match_array(expected_urls) expect(subject).to match_array(expected_urls)
end end
context 'when wiki is disabled' do context 'when wiki access level is' do
let(:project) { build(:project, :wiki_disabled) } context 'disabled' do
it 'does not include wiki url' do
project_feature.update!(wiki_access_level: ProjectFeature::DISABLED)
it 'does not include wiki url' do expect(subject).not_to include(project_wiki_url(project, Wiki::HOMEPAGE))
expect(subject).not_to include("http://localhost/#{project.full_path}/-/wiki_home") end
end
context 'private' do
it 'does not include wiki url' do
project_feature.update!(wiki_access_level: ProjectFeature::PRIVATE)
expect(subject).not_to include(project_wiki_url(project, Wiki::HOMEPAGE))
end
end end
end end
context 'when snippets are disabled' do context 'when snippets are disabled' do
let(:project) { build(:project, :snippets_disabled) } context 'disabled' do
it 'does not include snippets url' do
project_feature.update!(snippets_access_level: ProjectFeature::DISABLED)
expect(subject).not_to include(project_snippets_url(project))
end
end
context 'private' do
it 'does not include snippets url' do
project_feature.update!(snippets_access_level: ProjectFeature::PRIVATE)
expect(subject).not_to include(project_snippets_url(project))
end
end
end
context 'when issues are disabled' do
context 'disabled' do
it 'does not include issues url' do
project_feature.update!(issues_access_level: ProjectFeature::DISABLED)
expect(subject).not_to include(project_issues_url(project))
end
end
context 'private' do
it 'does not include issues url' do
project_feature.update!(issues_access_level: ProjectFeature::PRIVATE)
expect(subject).not_to include(project_issues_url(project))
end
end
end
context 'when merge requests are disabled' do
context 'disabled' do
it 'does not include merge requests url' do
project_feature.update!(merge_requests_access_level: ProjectFeature::DISABLED)
expect(subject).not_to include(project_merge_requests_url(project))
end
end
context 'private' do
it 'does not include merge requests url' do
project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE)
it 'does not include snippets url' do expect(subject).not_to include(project_merge_requests_url(project))
expect(subject).not_to include("http://localhost/#{project.full_path}/-/wiki_home") end
end end
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