Give the template finders a uniform interface

Two changes in this commit:

* Template finders now take a `project` (currently unused)
* LicenseTemplateFinder now respects the `name` parameter

app/finders/license_template_finder.rb

@@ -5,35 +5,49 @@
 # Used to find license templates, which may come from a variety of external
 # sources
 #
-# Arguments:
+# Params can be any of the following:
 #   popular: boolean. When set to true, only "popular" licenses are shown. When
 #            false, all licenses except popular ones are shown. When nil (the
 #            default), *all* licenses will be shown.
+#   name:    string. If set, return a single license matching that name (or nil)
 class LicenseTemplateFinder
+  include Gitlab::Utils::StrongMemoize
+
   prepend ::EE::LicenseTemplateFinder
 
-  attr_reader :params
+  attr_reader :project, :params
 
-  def initialize(params = {})
+  def initialize(project, params = {})
+    @project = project
     @params = params
   end
 
   def execute
-    Licensee::License.all(featured: popular_only?).map do |license|
-      LicenseTemplate.new(
-        id: license.key,
-        name: license.name,
-        nickname: license.nickname,
-        category: (license.featured? ? :Popular : :Other),
-        content: license.content,
-        url: license.url,
-        meta: license.meta
-      )
+    if params[:name]
+      vendored_licenses.find { |template| template.key == params[:name] }
+    else
+      vendored_licenses
     end
   end
 
   private
 
+  def vendored_licenses
+    strong_memoize(:vendored_licenses) do
+      Licensee::License.all(featured: popular_only?).map do |license|
+        LicenseTemplate.new(
+          key: license.key,
+          name: license.name,
+          nickname: license.nickname,
+          category: (license.featured? ? :Popular : :Other),
+          content: license.content,
+          url: license.url,
+          meta: license.meta
+        )
+      end
+    end
+  end
+
   def popular_only?
     params.fetch(:popular, nil)
   end

app/finders/template_finder.rb

 # frozen_string_literal: true
 
 class TemplateFinder
+  include Gitlab::Utils::StrongMemoize
+
   prepend ::EE::TemplateFinder
 
-  VENDORED_TEMPLATES = {
+  VENDORED_TEMPLATES = HashWithIndifferentAccess.new(
     dockerfiles: ::Gitlab::Template::DockerfileTemplate,
     gitignores: ::Gitlab::Template::GitignoreTemplate,
     gitlab_ci_ymls: ::Gitlab::Template::GitlabCiYmlTemplate
-  }.freeze
+  ).freeze
 
   class << self
-    def build(type, params = {})
-      if type == :licenses
-        LicenseTemplateFinder.new(params) # rubocop: disable CodeReuse/Finder
+    def build(type, project, params = {})
+      if type.to_s == 'licenses'
+        LicenseTemplateFinder.new(project, params) # rubocop: disable CodeReuse/Finder
       else
-        new(type, params)
+        new(type, project, params)
       end
     end
   end
 
-  attr_reader :type, :params
+  attr_reader :type, :project, :params
 
   attr_reader :vendored_templates
   private :vendored_templates
 
-  def initialize(type, params = {})
+  def initialize(type, project, params = {})
     @type = type
+    @project = project
     @params = params
 
     @vendored_templates = VENDORED_TEMPLATES.fetch(type)

app/helpers/blob_helper.rb

@@ -159,10 +159,6 @@ module BlobHelper
     end
   end
 
-  def licenses_for_select
-    @licenses_for_select ||= template_dropdown_names(TemplateFinder.build(:licenses).execute)
-  end
-
   def ref_project
     @ref_project ||= @target_project || @project
   end
@@ -173,22 +169,26 @@ module BlobHelper
 
     categories.each_with_object({}) do |category, hash|
       hash[category] = grouped[category].map do |item|
-        { name: item.name, id: item.id }
+        { name: item.name, id: item.key }
       end
     end
   end
   private :template_dropdown_names
 
-  def gitignore_names
-    @gitignore_names ||= template_dropdown_names(TemplateFinder.build(:gitignores).execute)
+  def licenses_for_select(project = @project)
+    @licenses_for_select ||= template_dropdown_names(TemplateFinder.build(:licenses, project).execute)
+  end
+
+  def gitignore_names(project = @project)
+    @gitignore_names ||= template_dropdown_names(TemplateFinder.build(:gitignores, project).execute)
   end
 
-  def gitlab_ci_ymls
-    @gitlab_ci_ymls ||= template_dropdown_names(TemplateFinder.build(:gitlab_ci_ymls).execute)
+  def gitlab_ci_ymls(project = @project)
+    @gitlab_ci_ymls ||= template_dropdown_names(TemplateFinder.build(:gitlab_ci_ymls, project).execute)
   end
 
-  def dockerfile_names
-    @dockerfile_names ||= template_dropdown_names(TemplateFinder.build(:dockerfiles).execute)
+  def dockerfile_names(project = @project)
+    @dockerfile_names ||= template_dropdown_names(TemplateFinder.build(:dockerfiles, project).execute)
   end
 
   def blob_editor_paths

app/models/license_template.rb

@@ -12,12 +12,10 @@ class LicenseTemplate
       (fullname|name\sof\s(author|copyright\sowner))
     [\>\}\]]}xi.freeze
 
-  attr_reader :id, :name, :category, :nickname, :url, :meta
+  attr_reader :key, :name, :category, :nickname, :url, :meta
 
-  alias_method :key, :id
-
-  def initialize(id:, name:, category:, content:, nickname: nil, url: nil, meta: {})
-    @id = id
+  def initialize(key:, name:, category:, content:, nickname: nil, url: nil, meta: {})
+    @key = key
     @name = name
     @category = category
     @content = content

ee/app/finders/ee/license_template_finder.rb

 module EE
   module LicenseTemplateFinder
-    include ::Gitlab::Utils::StrongMemoize
     extend ::Gitlab::Utils::Override
 
     override :execute
     def execute
       return super unless custom_templates?
 
-      extra = custom_licenses.map do |template|
-        LicenseTemplate.new(
-          id: template.name,
-          name: template.name,
-          nickname: template.name,
-          category: :Custom,
-          content: -> { template.content }
-        )
+      if params[:name]
+        custom_template || super
+      else
+        custom_templates + super
       end
-
-      extra.push(*super)
     end
 
     private
 
+    def custom_templates
+      templates_for(template_project).map do |template|
+        translate(template, category: :Custom)
+      end
+    end
+
+    def custom_template
+      template = template_for(template_project, params[:name])
+
+      translate(template, category: :Custom)
+    rescue ::Gitlab::Template::Finders::RepoTemplateFinder::FileNotFoundError
+      nil
+    end
+
     def custom_templates?
       !popular_only? &&
         ::License.feature_available?(:custom_file_templates) &&
         template_project.present?
     end
 
-    def custom_licenses
-      ::Gitlab::Template::CustomLicenseTemplate.all(template_project)
-    end
-
     def template_project
       strong_memoize(:template_project) { ::Gitlab::CurrentSettings.file_template_project }
     end
+
+    def templates_for(project)
+      return [] unless project
+
+      ::Gitlab::Template::CustomLicenseTemplate.all(project)
+    end
+
+    def template_for(project, name)
+      return unless project
+
+      ::Gitlab::Template::CustomLicenseTemplate.find(name, project)
+    end
+
+    def translate(template, category:)
+      return unless template
+
+      LicenseTemplate.new(
+        key: template.key,
+        name: template.name,
+        nickname: template.name,
+        category: category,
+        content: -> { template.content }
+      )
+    end
   end
 end

ee/app/finders/ee/template_finder.rb

 module EE
   module TemplateFinder
-    include ::Gitlab::Utils::StrongMemoize
     extend ::Gitlab::Utils::Override
 
-    CUSTOM_TEMPLATES = {
+    CUSTOM_TEMPLATES = HashWithIndifferentAccess.new(
       dockerfiles: ::Gitlab::Template::CustomDockerfileTemplate,
       gitignores: ::Gitlab::Template::CustomGitignoreTemplate,
       gitlab_ci_ymls: ::Gitlab::Template::CustomGitlabCiYmlTemplate
-    }.freeze
+    ).freeze
 
     attr_reader :custom_templates
     private :custom_templates

ee/spec/finders/license_template_finder_spec.rb

@@ -2,7 +2,7 @@ require 'spec_helper'
 
 describe LicenseTemplateFinder do
   describe '#execute' do
-    subject(:result) { described_class.new(params).execute }
+    subject(:result) { described_class.new(nil, params).execute }
 
     let(:params) { {} }
 

ee/spec/finders/template_finder_spec.rb

@@ -24,7 +24,7 @@ describe TemplateFinder do
     end
 
     with_them do
-      subject(:result) { described_class.new(type, params).execute }
+      subject(:result) { described_class.new(type, nil, params).execute }
 
       context 'specifying name' do
         let(:params) { { name: custom_name } }

ee/spec/helpers/ee/blob_helper_spec.rb

@@ -20,10 +20,10 @@ describe BlobHelper do
       expect(Gitlab::Template::CustomLicenseTemplate)
         .to receive(:all)
         .with(project)
-        .and_return([OpenStruct.new(name: "name")])
+        .and_return([OpenStruct.new(key: 'name', name: 'Name')])
 
       expect(categories).to contain_exactly(:Popular, :Other, :Custom)
-      expect(custom).to contain_exactly({ name: "name", id: "name" })
+      expect(custom).to contain_exactly({ id: 'name', name: 'Name' })
       expect(popular).to be_present
       expect(other).to be_present
     end

lib/api/templates.rb

@@ -35,7 +35,7 @@ module API
       popular = declared(params)[:popular]
       popular = to_boolean(popular) if popular.present?
 
-      templates = TemplateFinder.build(:licenses, popular: popular).execute
+      templates = TemplateFinder.build(:licenses, nil, popular: popular).execute
 
       present paginate(::Kaminari.paginate_array(templates)), with: ::API::Entities::License
     end
@@ -48,8 +48,7 @@ module API
       requires :name, type: String, desc: 'The name of the template'
     end
     get "templates/licenses/:name", requirements: { name: /[\w\.-]+/ } do
-      templates = TemplateFinder.build(:licenses).execute
-      template = templates.find { |template| template.key == params[:name] }
+      template = TemplateFinder.build(:licenses, nil, name: params[:name]).execute
 
       not_found!('License') unless template.present?
 
@@ -72,7 +71,7 @@ module API
         use :pagination
       end
       get "templates/#{template_type}" do
-        templates = ::Kaminari.paginate_array(TemplateFinder.new(template_type).execute)
+        templates = ::Kaminari.paginate_array(TemplateFinder.build(template_type, nil).execute)
         present paginate(templates), with: Entities::TemplatesList
       end
 
@@ -84,7 +83,7 @@ module API
         requires :name, type: String, desc: 'The name of the template'
       end
       get "templates/#{template_type}/:name" do
-        finder = TemplateFinder.build(template_type, name: declared(params)[:name])
+        finder = TemplateFinder.build(template_type, nil, name: declared(params)[:name])
         new_template = finder.execute
 
         render_response(template_type, new_template)

lib/gitlab/template/base_template.rb

@@ -12,14 +12,21 @@ module Gitlab
       def name
         File.basename(@path, self.class.extension)
       end
-      alias_method :id, :name
+      alias_method :key, :name
 
       def content
         @finder.read(@path)
       end
 
+      # Present for compatibility with license templates, which can replace text
+      # like `[fullname]` with a user-specified string. This is a no-op for
+      # other templates
+      def resolve!(_placeholders = {})
+        self
+      end
+
       def to_json
-        { name: name, content: content }
+        { key: key, name: name, content: content }
       end
 
       def <=>(other)

spec/finders/license_template_finder_spec.rb

@@ -2,7 +2,7 @@ require 'spec_helper'
 
 describe LicenseTemplateFinder do
   describe '#execute' do
-    subject(:result) { described_class.new(params).execute }
+    subject(:result) { described_class.new(nil, params).execute }
 
     let(:categories) { categorised_licenses.keys }
     let(:categorised_licenses) { result.group_by(&:category) }
@@ -31,7 +31,7 @@ describe LicenseTemplateFinder do
       it 'returns all licenses known by the Licensee gem' do
         from_licensee = Licensee::License.all.map { |l| l.key }
 
-        expect(result.map(&:id)).to match_array(from_licensee)
+        expect(result.map(&:key)).to match_array(from_licensee)
       end
 
       it 'correctly copies all attributes' do

spec/finders/template_finder_spec.rb

@@ -4,6 +4,8 @@ describe TemplateFinder do
   using RSpec::Parameterized::TableSyntax
 
   describe '#build' do
+    let(:project) { build_stubbed(:project) }
+
     where(:type, :expected_class) do
       :dockerfiles    | described_class
       :gitignores     | described_class
@@ -12,9 +14,10 @@ describe TemplateFinder do
     end
 
     with_them do
-      subject { described_class.build(type) }
+      subject(:finder) { described_class.build(type, project) }
 
       it { is_expected.to be_a(expected_class) }
+      it { expect(finder.project).to eq(project) }
     end
   end
 
@@ -27,19 +30,19 @@ describe TemplateFinder do
 
     with_them do
       it 'returns all vendored templates when no name is specified' do
-        result = described_class.new(type).execute
+        result = described_class.new(type, nil).execute
 
         expect(result).to include(have_attributes(name: vendored_name))
       end
 
       it 'returns only the specified vendored template when a name is specified' do
-        result = described_class.new(type, name: vendored_name).execute
+        result = described_class.new(type, nil, name: vendored_name).execute
 
         expect(result).to have_attributes(name: vendored_name)
       end
 
       it 'returns nil when an unknown name is specified' do
-        result = described_class.new(type, name: 'unknown').execute
+        result = described_class.new(type, nil, name: 'unknown').execute
 
         expect(result).to be_nil
       end

spec/models/license_template_spec.rb

@@ -54,6 +54,6 @@ describe LicenseTemplate do
   end
 
   def build_template(content)
-    described_class.new(id: 'foo', name: 'foo', category: :Other, content: content)
+    described_class.new(key: 'foo', name: 'foo', category: :Other, content: content)
   end
 end