Merge pull request #421 from rodjek/pdk-772

(PDK-772) Refactor PDK::Module::Convert for re-use in PDK::Module::Update

lib/pdk/cli/update.rb

@@ -23,7 +23,15 @@ module PDK::CLI
         raise PDK::CLI::ExitWithError, _('You can not specify --noop and --force when updating a module')
       end
 
-      PDK::Module::Update.invoke(opts)
+      updater = PDK::Module::Update.new(opts)
+
+      if updater.current_version == updater.new_version
+        PDK.logger.info _('This module is already up to date with version %{version} of the template.') % {
+          version: updater.new_version,
+        }
+      else
+        updater.run
+      end
     end
   end
 end

lib/pdk/module/convert.rb

@@ -7,44 +7,30 @@ module PDK
   module Module
     class Convert
       def self.invoke(options)
-        update_manager = PDK::Module::UpdateManager.new
-        template_url = options.fetch(:'template-url', PDK::Util.default_template_url)
+        new(options).run
+      end
 
-        PDK::Module::TemplateDir.new(template_url, nil, false) do |templates|
-          new_metadata = update_metadata('metadata.json', templates.metadata, options)
+      attr_reader :options
 
-          if options[:noop] && new_metadata.nil?
-            update_manager.add_file('metadata.json', '')
-          elsif File.file?('metadata.json')
-            update_manager.modify_file('metadata.json', new_metadata)
-          else
-            update_manager.add_file('metadata.json', new_metadata)
-          end
+      def initialize(options = {})
+        @options = options
+      end
 
-          templates.render do |file_path, file_content|
-            if File.exist? file_path
-              update_manager.modify_file(file_path, file_content)
-            else
-              update_manager.add_file(file_path, file_content)
-            end
-          end
-        end
+      def run
+        stage_changes!
 
         unless update_manager.changes?
           PDK::Report.default_target.puts(_('No changes required.'))
           return
         end
 
-        # Print the summary to the default target of reports
-        summary = get_summary(update_manager)
-        print_summary(summary)
+        print_summary
 
-        # Generates the full convert report
-        full_report(update_manager) unless update_manager.changes[:modified].empty?
+        full_report('convert_report.txt') unless update_manager.changes[:modified].empty?
 
-        return if options[:noop]
+        return if noop?
 
-        unless options[:force]
+        unless force?
           PDK.logger.info _(
             'Module conversion is a potentially destructive action. ' \
             'Ensure that you have committed your module to a version control ' \
@@ -56,19 +42,61 @@ def self.invoke(options)
 
         # Mark these files for removal after generating the report as these
         # changes are not something that the user needs to review.
-        if update_manager.changed?('Gemfile')
+        if needs_bundle_update?
           update_manager.remove_file('Gemfile.lock')
           update_manager.remove_file(File.join('.bundle', 'config'))
         end
 
         update_manager.sync_changes!
 
-        PDK::Util::Bundler.ensure_bundle! if update_manager.changed?('Gemfile')
+        PDK::Util::Bundler.ensure_bundle! if needs_bundle_update?
+
+        print_result 'Convert completed'
+      end
+
+      def noop?
+        options[:noop]
+      end
+
+      def force?
+        options[:force]
+      end
+
+      def needs_bundle_update?
+        update_manager.changed?('Gemfile')
+      end
+
+      def stage_changes!
+        PDK::Module::TemplateDir.new(template_url, nil, false) do |templates|
+          new_metadata = update_metadata('metadata.json', templates.metadata)
+
+          if options[:noop] && new_metadata.nil?
+            update_manager.add_file('metadata.json', '')
+          elsif File.file?('metadata.json')
+            update_manager.modify_file('metadata.json', new_metadata)
+          else
+            update_manager.add_file('metadata.json', new_metadata)
+          end
+
+          templates.render do |file_path, file_content|
+            if File.exist? file_path
+              update_manager.modify_file(file_path, file_content)
+            else
+              update_manager.add_file(file_path, file_content)
+            end
+          end
+        end
+      end
+
+      def update_manager
+        @update_manager ||= PDK::Module::UpdateManager.new
+      end
 
-        print_result(summary)
+      def template_url
+        @template_url ||= options.fetch(:'template-url', PDK::Util.default_template_url)
       end
 
-      def self.update_metadata(metadata_path, template_metadata, options = {})
+      def update_metadata(metadata_path, template_metadata)
         if File.file?(metadata_path)
           if File.readable?(metadata_path)
             begin
@@ -79,12 +107,12 @@ def self.update_metadata(metadata_path, template_metadata, options = {})
               metadata = PDK::Generate::Module.prepare_metadata(options) unless options[:noop] # rubocop:disable Metrics/BlockNesting
             end
           else
-            raise PDK::CLI::ExitWithError, _('Unable to convert module metadata; %{path} exists but it is not readable.') % {
+            raise PDK::CLI::ExitWithError, _('Unable to update module metadata; %{path} exists but it is not readable.') % {
               path: metadata_path,
             }
           end
         elsif File.exist?(metadata_path)
-          raise PDK::CLI::ExitWithError, _('Unable to convert module metadata; %{path} exists but it is not a file.') % {
+          raise PDK::CLI::ExitWithError, _('Unable to update module metadata; %{path} exists but it is not a file.') % {
             path: metadata_path,
           }
         else
@@ -102,7 +130,7 @@ def self.update_metadata(metadata_path, template_metadata, options = {})
         metadata.to_json
       end
 
-      def self.get_summary(update_manager)
+      def summary
         summary = {}
         update_manager.changes.each do |category, update_category|
           updated_files = if update_category.respond_to?(:keys)
@@ -117,7 +145,7 @@ def self.get_summary(update_manager)
         summary
       end
 
-      def self.print_summary(summary)
+      def print_summary
         footer = false
 
         summary.keys.each do |category|
@@ -131,23 +159,23 @@ def self.print_summary(summary)
         PDK::Report.default_target.puts(_("\n%{banner}") % { banner: generate_banner('', 40) }) if footer
       end
 
-      def self.print_result(summary)
-        PDK::Report.default_target.puts(_("\n%{banner}") % { banner: generate_banner('Convert completed', 40) })
+      def print_result(banner_text)
+        PDK::Report.default_target.puts(_("\n%{banner}") % { banner: generate_banner(banner_text, 40) })
         summary_to_print = summary.map { |k, v| "#{v.length} files #{k}" unless v.empty? }.compact
         PDK::Report.default_target.puts(_("\n%{summary}\n\n") % { summary: "#{summary_to_print.join(', ')}." })
       end
 
-      def self.full_report(update_manager)
-        File.open('convert_report.txt', 'w') do |f|
-          f.write("/* Convert Report generated by PDK at #{Time.now} */")
+      def full_report(path)
+        File.open(path, 'w') do |f|
+          f.write("/* Report generated by PDK at #{Time.now} */")
           update_manager.changes[:modified].each do |_, diff|
             f.write("\n\n\n" + diff)
           end
         end
-        PDK::Report.default_target.puts(_("\nYou can find a report of differences in convert_report.txt.\n\n"))
+        PDK::Report.default_target.puts(_("\nYou can find a report of differences in %{path}.\n\n") % { path: path })
       end
 
-      def self.generate_banner(text, width = 80)
+      def generate_banner(text, width = 80)
         padding = width - text.length
         banner = ''
         padding_char = '-'

lib/pdk/module/update.rb

@@ -1,8 +1,98 @@
+require 'pdk/module/convert'
+
 module PDK
   module Module
-    class Update
-      def self.invoke(_opts = {})
-        # TODO: do some updatey things
+    class Update < Convert
+      GIT_DESCRIBE_PATTERN = %r{\A(?<base>.+?)-(?<additional_commits>\d+)-g(?<sha>.+)\Z}
+
+      def run
+        stage_changes!
+
+        PDK.logger.info(update_message)
+
+        print_summary
+        full_report('update_report.txt') unless update_manager.changes[:modified].empty?
+
+        return if noop?
+
+        unless force?
+          message = _('Do you want to continue and make these changes to your module?')
+          return unless PDK::CLI::Util.prompt_for_yes(message)
+        end
+
+        if needs_bundle_update?
+          update_manager.remove_file('Gemfile.lock')
+          update_manager.remove_file(File.join('.bundle', 'config'))
+        end
+
+        update_manager.sync_changes!
+
+        PDK::Util::Bundler.ensure_bundle! if needs_bundle_update?
+
+        print_result 'Update completed'
+      end
+
+      def module_metadata
+        @module_metadata ||= PDK::Module::Metadata.from_file('metadata.json')
+      rescue ArgumentError => e
+        raise PDK::CLI::ExitWithError, e.message
+      end
+
+      def template_url
+        @template_url ||= module_metadata.data['template-url']
+      end
+
+      def current_version
+        @current_version ||= describe_ref_to_s(current_template_version)
+      end
+
+      def new_version
+        @new_version ||= fetch_remote_version(new_template_version)
+      end
+
+      private
+
+      def current_template_version
+        @current_template_version ||= module_metadata.data['template-ref']
+      end
+
+      def describe_ref_to_s(describe_ref)
+        data = GIT_DESCRIBE_PATTERN.match(describe_ref)
+
+        return data if data.nil?
+
+        if data[:base].start_with?('heads/')
+          "#{data[:base].gsub(%r{^heads/}, '')}@#{data[:sha]}"
+        else
+          data[:base]
+        end
+      end
+
+      def new_template_version
+        PDK::Util.default_template_ref
+      end
+
+      def fetch_remote_version(version)
+        return version unless version.include?('/')
+
+        branch = version.partition('/').last
+        sha_length = GIT_DESCRIBE_PATTERN.match(current_template_version)[:sha].length - 1
+        "#{branch}@#{PDK::Util::Git.ls_remote(template_url, "refs/heads/#{branch}")[0..sha_length]}"
+      end
+
+      def update_message
+        format_string = if template_url == PDK::Util.puppetlabs_template_url
+                          _('Updating %{module_name} using the default template, from %{current_version} to %{new_version}')
+                        else
+                          _('Updating %{module_name} using the template at %{template_url}, from %{current_version} to %{new_version}')
+                        end
+
+        format_string % {
+          module_name:     module_metadata.data['name'],
+          template_url:    template_url,
+          current_version: current_version,
+          new_version:     new_version,
+        }
       end
     end
   end

lib/pdk/util/git.rb

@@ -37,6 +37,21 @@ def self.repo_exists?(repo, ref = nil)
 
         git(*args)[:exit_code].zero?
       end
+
+      def self.ls_remote(repo, ref)
+        output = git('ls-remote', '--refs', repo, ref)
+
+        unless output[:exit_code].zero?
+          PDK.logger.error output[:stdout]
+          PDK.logger.error output[:stderr]
+          raise PDK::CLI::ExitWithError, _('Unable to access the template repository "%{repository}"') % {
+            repository: repo,
+          }
+        end
+
+        matching_refs = output[:stdout].split("\n").map { |r| r.split("\t") }
+        matching_refs.find { |_sha, remote_ref| remote_ref == ref }.first
+      end
     end
   end
 end