Commit c11eb0c3 authored by Nick Thomas's avatar Nick Thomas 💃

Merge branch 'id-submodule-url-graphql' into 'master'

Reduce the number of Gitaly calls for submodules

See merge request gitlab-org/gitlab-ce!30735
parents b62c049f 0f468868
......@@ -137,6 +137,7 @@ export default {
:path="entry.flatPath"
:type="entry.type"
:url="entry.webUrl"
:submodule-tree-url="entry.treeUrl"
:lfs-oid="entry.lfsOid"
/>
</template>
......
......@@ -62,6 +62,11 @@ export default {
required: false,
default: null,
},
submoduleTreeUrl: {
type: String,
required: false,
default: null,
},
},
data() {
return {
......@@ -112,7 +117,7 @@ export default {
</component>
<gl-badge v-if="lfsOid" variant="default" class="label-lfs ml-1">LFS</gl-badge>
<template v-if="isSubmodule">
@ <gl-link href="#" class="commit-sha">{{ shortSha }}</gl-link>
@ <gl-link :href="submoduleTreeUrl" class="commit-sha">{{ shortSha }}</gl-link>
</template>
</td>
<td class="d-none d-sm-table-cell tree-commit">
......
......@@ -35,6 +35,8 @@ query getFiles(
edges {
node {
...TreeEntry
webUrl
treeUrl
}
}
pageInfo {
......
......@@ -7,6 +7,9 @@ module Types
implements Types::Tree::EntryType
graphql_name 'Submodule'
field :web_url, type: GraphQL::STRING_TYPE, null: true
field :tree_url, type: GraphQL::STRING_TYPE, null: true
end
# rubocop: enable Graphql/AuthorizeTypes
end
......
......@@ -15,7 +15,9 @@ module Types
Gitlab::Graphql::Representation::TreeEntry.decorate(obj.trees, obj.repository)
end
field :submodules, Types::Tree::SubmoduleType.connection_type, null: false, calls_gitaly: true
field :submodules, Types::Tree::SubmoduleType.connection_type, null: false, calls_gitaly: true, resolve: -> (obj, args, ctx) do
Gitlab::Graphql::Representation::SubmoduleTreeEntry.decorate(obj.submodules, obj)
end
field :blobs, Types::Tree::BlobType.connection_type, null: false, calls_gitaly: true, resolve: -> (obj, args, ctx) do
Gitlab::Graphql::Representation::TreeEntry.decorate(obj.blobs, obj.repository)
......
......@@ -9,6 +9,10 @@ module SubmoduleHelper
def submodule_links(submodule_item, ref = nil, repository = @repository)
url = repository.submodule_url_for(ref, submodule_item.path)
submodule_links_for_url(submodule_item.id, url, repository)
end
def submodule_links_for_url(submodule_item_id, url, repository)
if url == '.' || url == './'
url = File.join(Gitlab.config.gitlab.url, repository.project.full_path)
end
......@@ -31,13 +35,13 @@ module SubmoduleHelper
if self_url?(url, namespace, project)
[namespace_project_path(namespace, project),
namespace_project_tree_path(namespace, project, submodule_item.id)]
namespace_project_tree_path(namespace, project, submodule_item_id)]
elsif relative_self_url?(url)
relative_self_links(url, submodule_item.id, repository.project)
relative_self_links(url, submodule_item_id, repository.project)
elsif github_dot_com_url?(url)
standard_links('github.com', namespace, project, submodule_item.id)
standard_links('github.com', namespace, project, submodule_item_id)
elsif gitlab_dot_com_url?(url)
standard_links('gitlab.com', namespace, project, submodule_item.id)
standard_links('gitlab.com', namespace, project, submodule_item_id)
else
[sanitize_submodule_url(url), nil]
end
......
......@@ -3,7 +3,6 @@
class DiffFileBaseEntity < Grape::Entity
include RequestAwareEntity
include BlobHelper
include SubmoduleHelper
include DiffHelper
include TreeHelper
include ChecksCollaboration
......@@ -12,12 +11,12 @@ class DiffFileBaseEntity < Grape::Entity
expose :content_sha
expose :submodule?, as: :submodule
expose :submodule_link do |diff_file|
memoized_submodule_links(diff_file).first
expose :submodule_link do |diff_file, options|
memoized_submodule_links(diff_file, options).first
end
expose :submodule_tree_url do |diff_file|
memoized_submodule_links(diff_file).last
memoized_submodule_links(diff_file, options).last
end
expose :edit_path, if: -> (_, options) { options[:merge_request] } do |diff_file|
......@@ -92,10 +91,10 @@ class DiffFileBaseEntity < Grape::Entity
private
def memoized_submodule_links(diff_file)
def memoized_submodule_links(diff_file, options)
strong_memoize(:submodule_links) do
if diff_file.submodule?
submodule_links(diff_file.blob, diff_file.content_sha, diff_file.repository)
options[:submodule_links].for(diff_file.blob, diff_file.content_sha)
else
[]
end
......
......@@ -64,7 +64,10 @@ class DiffsEntity < Grape::Entity
merge_request_path(merge_request, format: :diff)
end
expose :diff_files, using: DiffFileEntity
expose :diff_files do |diffs, options|
submodule_links = Gitlab::SubmoduleLinks.new(merge_request.project.repository)
DiffFileEntity.represent(diffs.diff_files, options.merge(submodule_links: submodule_links))
end
expose :merge_request_diffs, using: MergeRequestDiffEntity, if: -> (_, options) { options[:merge_request_diffs]&.any? } do |diffs|
options[:merge_request_diffs]
......
......@@ -2,4 +2,18 @@
class DiscussionSerializer < BaseSerializer
entity DiscussionEntity
def represent(resource, opts = {}, entity_class = nil)
super(resource, with_additional_opts(opts), entity_class)
end
private
def with_additional_opts(opts)
additional_opts = {
submodule_links: Gitlab::SubmoduleLinks.new(@request.project.repository)
}
opts.merge(additional_opts)
end
end
# frozen_string_literal: true
class SubmoduleEntity < Grape::Entity
include RequestAwareEntity
expose :id, :path, :name, :mode
expose :icon do |blob|
'archive'
end
expose :url do |blob|
submodule_links(blob, request).first
end
expose :tree_url do |blob|
submodule_links(blob, request).last
end
private
def submodule_links(blob, request)
@submodule_links ||= SubmoduleHelper.submodule_links(blob, request.ref, request.repository)
end
end
......@@ -464,6 +464,18 @@ module Gitlab
end
end
# Returns path to url mappings for submodules
#
# Ex.
# @repository.submodule_urls_for('master')
# # => { 'rack' => 'git@localhost:rack.git' }
#
def submodule_urls_for(ref)
wrapped_gitaly_errors do
gitaly_submodule_urls_for(ref)
end
end
# Return total commits count accessible from passed ref
def commit_count(ref)
wrapped_gitaly_errors do
......@@ -1059,12 +1071,16 @@ module Gitlab
return unless commit_object && commit_object.type == :COMMIT
urls = gitaly_submodule_urls_for(ref)
urls && urls[path]
end
def gitaly_submodule_urls_for(ref)
gitmodules = gitaly_commit_client.tree_entry(ref, '.gitmodules', Gitlab::Git::Blob::MAX_DATA_DISPLAY_SIZE)
return unless gitmodules
found_module = GitmodulesParser.new(gitmodules.data).parse[path]
found_module && found_module['url']
submodules = GitmodulesParser.new(gitmodules.data).parse
submodules.transform_values { |submodule| submodule['url'] }
end
# Returns true if the given ref name exists
......
# frozen_string_literal: true
module Gitlab
module Graphql
module Representation
class SubmoduleTreeEntry < SimpleDelegator
class << self
def decorate(submodules, tree)
repository = tree.repository
submodule_links = Gitlab::SubmoduleLinks.new(repository)
submodules.map do |submodule|
self.new(submodule, submodule_links.for(submodule, tree.sha))
end
end
end
def initialize(submodule, submodule_links)
@submodule_links = submodule_links
super(submodule)
end
def web_url
@submodule_links.first
end
def tree_url
@submodule_links.last
end
end
end
end
end
# frozen_string_literal: true
module Gitlab
class SubmoduleLinks
include Gitlab::Utils::StrongMemoize
def initialize(repository)
@repository = repository
end
def for(submodule, sha)
submodule_url = submodule_url_for(sha)[submodule.path]
SubmoduleHelper.submodule_links_for_url(submodule.id, submodule_url, repository)
end
private
attr_reader :repository
def submodule_url_for(sha)
strong_memoize(:"submodule_links_for_#{sha}") do
repository.submodule_urls_for(sha)
end
end
end
end
import { shallowMount, RouterLinkStub } from '@vue/test-utils';
import { GlBadge } from '@gitlab/ui';
import { GlBadge, GlLink } from '@gitlab/ui';
import { visitUrl } from '~/lib/utils/url_utility';
import TableRow from '~/repository/components/table/row.vue';
......@@ -142,4 +142,18 @@ describe('Repository table row component', () => {
expect(vm.find(GlBadge).exists()).toBe(true);
});
it('renders commit and web links with href for submodule', () => {
factory({
id: '1',
path: 'test',
type: 'commit',
url: 'https://test.com',
submoduleTreeUrl: 'https://test.com/commit',
currentPath: '/',
});
expect(vm.find('a').attributes('href')).toEqual('https://test.com');
expect(vm.find(GlLink).attributes('href')).toEqual('https://test.com/commit');
});
});
......@@ -5,5 +5,5 @@ require 'spec_helper'
describe Types::Tree::SubmoduleType do
it { expect(described_class.graphql_name).to eq('Submodule') }
it { expect(described_class).to have_graphql_fields(:id, :name, :type, :path, :flat_path) }
it { expect(described_class).to have_graphql_fields(:id, :name, :type, :path, :flat_path, :web_url, :tree_url) }
end
......@@ -256,6 +256,22 @@ describe Gitlab::Git::Repository, :seed_helper do
end
end
describe '#submodule_urls_for' do
let(:ref) { 'master' }
it 'returns url mappings for submodules' do
urls = repository.submodule_urls_for(ref)
expect(urls).to eq({
"deeper/nested/six" => "git://github.com/randx/six.git",
"gitlab-grack" => "https://gitlab.com/gitlab-org/gitlab-grack.git",
"gitlab-shell" => "https://github.com/gitlabhq/gitlab-shell.git",
"nested/six" => "git://github.com/randx/six.git",
"six" => "git://github.com/randx/six.git"
})
end
end
describe '#commit_count' do
it { expect(repository.commit_count("master")).to eq(25) }
it { expect(repository.commit_count("feature")).to eq(9) }
......
# frozen_string_literal: true
require 'spec_helper'
describe Gitlab::Graphql::Representation::SubmoduleTreeEntry do
let(:project) { create(:project, :repository) }
let(:repository) { project.repository }
describe '.decorate' do
let(:submodules) { repository.tree.submodules }
it 'returns array of SubmoduleTreeEntry' do
entries = described_class.decorate(submodules, repository.tree)
expect(entries.first).to be_a(described_class)
expect(entries.map(&:web_url)).to contain_exactly(
"https://gitlab.com/gitlab-org/gitlab-grack",
"https://github.com/gitlabhq/gitlab-shell",
"https://github.com/randx/six"
)
expect(entries.map(&:tree_url)).to contain_exactly(
"https://gitlab.com/gitlab-org/gitlab-grack/tree/645f6c4c82fd3f5e06f67134450a570b795e55a6",
"https://github.com/gitlabhq/gitlab-shell/tree/79bceae69cb5750d6567b223597999bfa91cb3b9",
"https://github.com/randx/six/tree/409f37c4f05865e4fb208c771485f211a22c4c2d"
)
end
end
end
# frozen_string_literal: true
require 'spec_helper'
describe DiffFileBaseEntity do
let(:project) { create(:project, :repository) }
let(:repository) { project.repository }
context 'diff for a changed submodule' do
let(:commit_sha_with_changed_submodule) do
"cfe32cf61b73a0d5e9f13e774abde7ff789b1660"
end
let(:commit) { project.commit(commit_sha_with_changed_submodule) }
let(:diff_file) { commit.diffs.diff_files.to_a.last }
let(:options) { { request: {}, submodule_links: Gitlab::SubmoduleLinks.new(repository) } }
let(:entity) { described_class.new(diff_file, options).as_json }
it do
expect(entity[:submodule]).to eq(true)
expect(entity[:submodule_link]).to eq("https://github.com/randx/six")
expect(entity[:submodule_tree_url]).to eq(
"https://github.com/randx/six/tree/409f37c4f05865e4fb208c771485f211a22c4c2d"
)
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