(PDK-626) Templatedir can now handle multiple directories

lib/pdk/generate/module.rb

@@ -70,7 +70,7 @@ def self.invoke(opts = {})
 
         template_url = opts.fetch(:'template-url', default_template_url)
 
-        PDK::Module::TemplateDir.new(template_url, metadata.data) do |templates|
+        PDK::Module::TemplateDir.new(template_url, metadata.data, true) do |templates|
           templates.render do |file_path, file_content|
             file = Pathname.new(temp_target_dir) + file_path
             file.dirname.mkpath

lib/pdk/module/templatedir.rb

@@ -36,7 +36,7 @@ class TemplateDir
       # @raise [ArgumentError] (see #validate_module_template!)
       #
       # @api public
-      def initialize(path_or_url, module_metadata = {})
+      def initialize(path_or_url, module_metadata = {}, init = false)
         if File.directory?(path_or_url)
           @path = path_or_url
         else
@@ -48,18 +48,21 @@ def initialize(path_or_url, module_metadata = {})
           # use.
           temp_dir = PDK::Util.make_tmpdir_name('pdk-module-template')
 
-          clone_result = PDK::Util::Git.git('clone', path_or_url, temp_dir)
+          clone_result = PDK::Util::Git.git('clone', path_or_url, '--branch', 'convert', temp_dir)
           unless clone_result[:exit_code].zero?
             PDK.logger.error clone_result[:stdout]
             PDK.logger.error clone_result[:stderr]
             raise PDK::CLI::FatalError, _("Unable to clone git repository '%{repo}' to '%{dest}'.") % { repo: path_or_url, dest: temp_dir }
           end
-
           @path = PDK::Util.canonical_path(temp_dir)
           @repo = path_or_url
         end
 
+        @init = init
         @moduleroot_dir = File.join(@path, 'moduleroot')
+        @moduleroot_init = File.join(@path, 'moduleroot_init')
+        @dirs = [@moduleroot_dir]
+        @dirs << @moduleroot_init if @init
         @object_dir = File.join(@path, 'object_templates')
         validate_module_template!
 
@@ -107,20 +110,19 @@ def metadata
       #
       # @api public
       def render
-        files_in_template.each do |template_file|
+        PDK::Module::TemplateDir.files_in_template(@dirs).each do |template_file, template_loc|
+          template_file = template_file.to_s
           PDK.logger.debug(_("Rendering '%{template}'...") % { template: template_file })
           dest_path = template_file.sub(%r{\.erb\Z}, '')
-
           begin
-            dest_content = PDK::TemplateFile.new(File.join(@moduleroot_dir, template_file), configs: config_for(dest_path)).render
+            dest_content = PDK::TemplateFile.new(File.join(template_loc, template_file), configs: config_for(dest_path)).render
           rescue => e
             error_msg = _(
               "Failed to render template '%{template}'\n" \
               '%{exception}: %{message}',
             ) % { template: template_file, exception: e.class, message: e.message }
             raise PDK::CLI::FatalError, error_msg
           end
-
           yield dest_path, dest_content
         end
       end
@@ -165,8 +167,6 @@ def object_config
         config_for(nil)
       end
 
-      private
-
       # Validate the content of the template directory.
       #
       # @raise [ArgumentError] If the specified path is not a directory.
@@ -181,27 +181,44 @@ def validate_module_template!
           raise ArgumentError, _("The specified template '%{path}' is not a directory.") % { path: @path }
         end
 
-        unless File.directory?(@moduleroot_dir) # rubocop:disable Style/GuardClause
+        unless File.directory?(@moduleroot_dir)
           raise ArgumentError, _("The template at '%{path}' does not contain a 'moduleroot/' directory.") % { path: @path }
         end
+
+        unless File.directory?(@moduleroot_init) # rubocop:disable Style/GuardClause
+          # rubocop:disable Metrics/LineLength
+          raise ArgumentError, _("The template at '%{path}' does not contain a 'moduleroot_init/' directory, which indicates you are using an older style of template. Before continuing please use the --template_url flag when running the pdk new or convert commands to pass a new style template.") % { path: @path } unless @init
+
+          PDK.logger.warn(_("The template at '%{path}' doesn't seem to have a 'moduleroot_init' directory, this could indicate that you are using an older style of template.")) % { path: @path } # rubocop:disable Lint/Void
+          PDK.logger.warn(_('To pass in a new template you can use the --template_url flag when running the pdk new or convert commands.'))
+          # rubocop:enable Metrics/LineLength
+        end
       end
 
       # Get a list of template files in the template directory.
       #
-      # @return [Array[String]] An array of file names, relative to the
-      # `moduleroot` directory.
+      # @return [Hash{String=>String}] A hash of key file names and
+      # value locations.
       #
-      # @api private
-      def files_in_template
-        @files ||= begin
-          template_paths = Dir.glob(File.join(@moduleroot_dir, '**', '*'), File::FNM_DOTMATCH).select do |template_path|
+      # @api public
+      def self.files_in_template(dirs)
+        temp_paths = []
+        dirlocs = []
+        dirs.each do |dir|
+          raise ArgumentError, _("The directory '%{dir}' doesn't exist") unless Dir.exist?(dir)
+          temp_paths += Dir.glob(File.join(dir, '**', '*'), File::FNM_DOTMATCH).select do |template_path|
             File.file?(template_path) && !File.symlink?(template_path)
+            dirlocs << dir
           end
-
-          template_paths.map do |template_path|
-            template_path.sub(%r{\A#{Regexp.escape(@moduleroot_dir)}#{Regexp.escape(File::SEPARATOR)}}, '')
+          temp_paths.map do |template_path|
+            template_path.sub!(%r{\A#{Regexp.escape(dir)}#{Regexp.escape(File::SEPARATOR)}}, '')
           end
         end
+        template_paths = Hash[temp_paths.zip dirlocs]
+        template_paths.delete('.')
+        template_paths.delete('spec')
+        template_paths.delete('spec/.')
+        template_paths
       end
 
       # Generate a hash of data to be used when rendering the specified

spec/acceptance/new_module_spec.rb

@@ -22,7 +22,7 @@
       it { is_expected.to be_file }
       its(:content_as_json) do
         is_expected.to include('name' => match(%r{-foo}),
-                               'template-ref' => match(%r{master-}),
+                               'template-ref' => match(%r{convert-}),
                                'operatingsystem_support' => include('operatingsystem' => 'Debian',
                                                                     'operatingsystemrelease' => ['8']))
       end

spec/unit/pdk/generate/module_spec.rb

@@ -25,11 +25,11 @@
 end
 
 shared_context 'mock template dir' do
-  let(:test_template_dir) { instance_double(PDK::Module::TemplateDir, render: true, metadata: {}) }
+  let(:test_template_dir) { instance_double(PDK::Module::TemplateDir, metadata: {}) }
   let(:test_template_file) { StringIO.new }
 
   before(:each) do
-    allow(PDK::Module::TemplateDir).to receive(:new).with(anything, anything).and_yield(test_template_dir)
+    allow(PDK::Module::TemplateDir).to receive(:new).with(anything, anything, anything).and_yield(test_template_dir)
 
     dir_double = instance_double(Pathname, mkpath: true)
     allow(dir_double).to receive(:+).with(anything).and_return(test_template_file)
@@ -89,6 +89,7 @@
         allow(described_class).to receive(:prepare_module_directory).with(temp_target_dir)
         allow(File).to receive(:open).with(%r{pdk-test-writable}, anything) { raise Errno::EACCES unless target_parent_writeable }
         allow(FileUtils).to receive(:rm_f).with(%r{pdk-test-writable})
+        allow(test_template_dir).to receive(:render).and_yield('test_file_path', 'test_file_content')
       end
 
       context 'when the parent directory of the target is not writable' do
@@ -168,7 +169,7 @@
         end
 
         it 'uses that template to generate the module' do
-          expect(PDK::Module::TemplateDir).to receive(:new).with('cli-template', anything).and_yield(test_template_dir)
+          expect(PDK::Module::TemplateDir).to receive(:new).with('cli-template', anything, anything).and_yield(test_template_dir)
           expect(logger).to receive(:info).with(a_string_matching(%r{generated at path}i))
           expect(logger).to receive(:info).with(a_string_matching(%r{In your module directory, add classes with the 'pdk new class' command}i))
 
@@ -177,7 +178,7 @@
 
         it 'takes precedence over the template-url answer' do
           PDK.answers.update!('template-url' => 'answer-template')
-          expect(PDK::Module::TemplateDir).to receive(:new).with('cli-template', anything).and_yield(test_template_dir)
+          expect(PDK::Module::TemplateDir).to receive(:new).with('cli-template', anything, anything).and_yield(test_template_dir)
           described_class.invoke(invoke_opts.merge(:'template-url' => 'cli-template'))
         end
 
@@ -205,7 +206,7 @@
         context 'and a template-url answer exists' do
           it 'uses the template-url from the answer file to generate the module' do
             PDK.answers.update!('template-url' => 'answer-template')
-            expect(PDK::Module::TemplateDir).to receive(:new).with('answer-template', anything).and_yield(test_template_dir)
+            expect(PDK::Module::TemplateDir).to receive(:new).with('answer-template', anything, anything).and_yield(test_template_dir)
             expect(logger).to receive(:info).with(a_string_matching(%r{generated at path}i))
             expect(logger).to receive(:info).with(a_string_matching(%r{In your module directory, add classes with the 'pdk new class' command}i))
 
@@ -221,7 +222,7 @@
             end
 
             it 'uses the vendored template url' do
-              expect(PDK::Module::TemplateDir).to receive(:new).with('file:///tmp/package/cache/pdk-module-template.git', anything).and_yield(test_template_dir)
+              expect(PDK::Module::TemplateDir).to receive(:new).with('file:///tmp/package/cache/pdk-module-template.git', anything, anything).and_yield(test_template_dir)
               expect(PDK.answers).not_to receive(:update!).with(:'template-url' => anything)
 
               described_class.invoke(invoke_opts)
@@ -230,7 +231,7 @@
 
           context 'and pdk is not installed from packages' do
             it 'uses the default template to generate the module' do
-              expect(PDK::Module::TemplateDir).to receive(:new).with(described_class.default_template_url, anything).and_yield(test_template_dir)
+              expect(PDK::Module::TemplateDir).to receive(:new).with(described_class.default_template_url, anything, anything).and_yield(test_template_dir)
               expect(PDK.answers).not_to receive(:update!).with(:'template-url' => anything)
 
               described_class.invoke(invoke_opts)

spec/unit/pdk/module/template_dir_spec.rb

@@ -2,7 +2,7 @@
 
 describe PDK::Module::TemplateDir do
   subject(:template_dir) do
-    described_class.new(path_or_url, module_metadata) do |foo|
+    described_class.new(path_or_url, module_metadata, true) do |foo|
       # block does nothing
     end
   end
@@ -42,4 +42,136 @@
   it 'has a metadata method' do
     expect(described_class.instance_methods(false)).to include(:metadata)
   end
+
+  describe '.files_in_template(dirs)' do
+    context 'when passing in an empty directory' do
+      let(:dirs) { ['/the/file/is/here'] }
+
+      before(:each) do
+        allow(Dir).to receive(:exist?).with('/the/file/is/here').and_return true
+      end
+      it 'returns an empty list' do
+        expect(described_class.files_in_template(dirs)).to eq({})
+      end
+    end
+
+    context 'when passing in a non-existant directory' do
+      let(:dirs) { ['/the/file/is/nothere'] }
+
+      before(:each) do
+        allow(Dir).to receive(:exists?).with('/the/file/is/nothere').and_return false
+      end
+      it 'raises an error' do
+        expect { described_class.files_in_template(dirs) }.to raise_error(ArgumentError, %r{The directory '%\{dir\}' doesn't exist})
+      end
+    end
+
+    context 'when passing in a directory with a single file' do
+      let(:dirs) { ['/here/moduleroot'] }
+
+      before(:each) do
+        allow(Dir).to receive(:exist?).with('/here/moduleroot').and_return true
+        allow(File).to receive(:file?).with('/here/moduleroot/filename').and_return true
+        allow(Dir).to receive(:glob).with('/here/moduleroot/**/*', File::FNM_DOTMATCH).and_return ['/here/moduleroot/filename']
+      end
+      it 'returns the file name' do
+        expect(described_class.files_in_template(dirs)).to eq('filename' => '/here/moduleroot')
+      end
+    end
+
+    context 'when passing in a directory with more than one file' do
+      let(:dirs) { ['/here/moduleroot'] }
+
+      before(:each) do
+        allow(Dir).to receive(:exist?).with('/here/moduleroot').and_return true
+        allow(File).to receive(:file?).with('/here/moduleroot/filename').and_return true
+        allow(File).to receive(:file?).with('/here/moduleroot/filename2').and_return true
+        allow(Dir).to receive(:glob).with('/here/moduleroot/**/*', File::FNM_DOTMATCH).and_return ['/here/moduleroot/filename', '/here/moduleroot/filename2']
+      end
+      it 'returns both the file names' do
+        expect(described_class.files_in_template(dirs)).to eq('filename' => '/here/moduleroot', 'filename2' => '/here/moduleroot')
+      end
+    end
+
+    context 'when passing in more than one directory with a file' do
+      let(:path_or_url) { '/path/to/templates' }
+      let(:dirs) { ['/path/to/templates/moduleroot', '/path/to/templates/moduleroot_init'] }
+
+      before(:each) do
+        allow(Dir).to receive(:exist?).with('/path/to/templates').and_return true
+        allow(Dir).to receive(:exist?).with('/path/to/templates/moduleroot').and_return true
+        allow(Dir).to receive(:exist?).with('/path/to/templates/moduleroot_init').and_return true
+        allow(File).to receive(:file?).with('/path/to/templates/moduleroot/.').and_return true
+        allow(File).to receive(:file?).with('/path/to/templates/moduleroot/filename').and_return true
+        allow(File).to receive(:file?).with('/path/to/templates/moduleroot_init/filename2').and_return true
+        allow(Dir).to receive(:glob).with('/path/to/templates/moduleroot/**/*', File::FNM_DOTMATCH).and_return ['/path/to/templates/moduleroot/.', '/path/to/templates/moduleroot/filename']
+        allow(Dir).to receive(:glob).with('/path/to/templates/moduleroot_init/**/*', File::FNM_DOTMATCH).and_return ['/path/to/templates/moduleroot_init/filename2']
+      end
+      it 'returns the file names from both directories' do
+        expect(described_class.files_in_template(dirs)).to eq('filename' => '/path/to/templates/moduleroot',
+                                                              'filename2' => '/path/to/templates/moduleroot_init')
+      end
+    end
+  end
+
+  describe '.render(template_files)' do
+    before(:each) do
+      allow(File).to receive(:directory?).with(anything).and_return(true)
+      allow(PDK::Util).to receive(:make_tmpdir_name).with('pdk-module-template').and_return('/tmp/path')
+      allow(PDK::CLI::Exec).to receive(:git).with('clone', path_or_url, '/tmp/path').and_return(exit_code: 0)
+    end
+
+    context 'when passing in a template file' do
+      let(:template_file) { instance_double('PDK::TemplateFile', 'filename.erb') }
+      let(:template_files) { { 'filename.erb' => 'file/is/here/' } }
+
+      before(:each) do
+        allow(described_class).to receive(:config_for).with('filename').and_return true
+        allow(PDK::TemplateFile).to receive(:new).with('file/is/here/filename.erb', configs: true).and_return template_file
+        allow(template_file).to receive(:render).and_return template_file
+        allow(described_class).to receive(:render) { { 'filename.erb' => 'file/is/here/' } }
+      end
+      it 'renders the template file and returns relevant values' do
+        expect(described_class.render(template_files)).to eq('filename.erb' => 'file/is/here/')
+      end
+    end
+
+    context 'when passing in two template files in the same location' do
+      let(:template_file) { instance_double('PDK::TemplateFile', 'filename.erb') }
+      let(:template_file2) { instance_double('PDK::TemplateFile', 'filename2.erb') }
+      let(:template_files) { { 'filename.erb' => 'file/is/here/', 'filename2.erb' => 'file/is/here/' } }
+
+      before(:each) do
+        allow(described_class).to receive(:config_for).with('filename').and_return true
+        allow(PDK::TemplateFile).to receive(:new).with('file/is/here/filename.erb', configs: true).and_return template_file
+        allow(template_file).to receive(:render).and_return template_file
+        allow(described_class).to receive(:config_for).with('filename2').and_return true
+        allow(PDK::TemplateFile).to receive(:new).with('file/is/here/filename2.erb', configs: true).and_return template_file
+        allow(template_file).to receive(:render).and_return template_file2
+        allow(described_class).to receive(:render) { { 'filename.erb' => 'file/is/here/', 'filename2.erb' => 'file/is/here/' } }
+      end
+      it 'renders the template file and returns relevant values' do
+        expect(described_class.render(template_files)).to eq('filename.erb' => 'file/is/here/', 'filename2.erb' => 'file/is/here/')
+      end
+    end
+
+    context 'when passing in two template files in different directories' do
+      let(:template_file) { instance_double('PDK::TemplateFile', 'filename.erb') }
+      let(:template_file2) { instance_double('PDK::TemplateFile', 'filename2.erb') }
+      let(:template_files) { { 'filename.erb' => 'file/is/here/', 'filename2.erb' => 'file/is/here/' } }
+
+      before(:each) do
+        allow(described_class).to receive(:config_for).with('filename').and_return true
+        allow(PDK::TemplateFile).to receive(:new).with('file/is/here/filename.erb', configs: true).and_return template_file
+        allow(template_file).to receive(:render).and_return template_file
+        allow(described_class).to receive(:config_for).with('filename2').and_return true
+        allow(PDK::TemplateFile).to receive(:new).with('file/is/where/filename2.erb', configs: true).and_return template_file
+        allow(template_file2).to receive(:render).and_return template_file2
+        allow(described_class).to receive(:render) { { 'filename.erb' => 'file/is/here/', 'filename2.erb' => 'file/is/where/' } }
+      end
+      it 'renders the template file and returns relevant values' do
+        expect(described_class.render(template_files)).to eq('filename.erb' => 'file/is/here/', 'filename2.erb' => 'file/is/where/')
+      end
+    end
+  end
 end