Merge branch 'kpaulisse-release-1-0' into kpaulisse-release-1-0-doc

doc/dev/api/v1/objects/diff.md

@@ -44,6 +44,10 @@ Note that this is a pass-through of information provided in the Puppet catalog,
 
 Note also that if the diff represents removal of a resource, this will return `nil`, because the resource does not exist in the new catalog.
 
+#### `#new_location` (Hash)
+
+Returns a hash containing `:file` (equal to `#new_file`) and `:line` (equal to `#new_line`) when either is defined. Returns `nil` if both are undefined.
+
 #### `#new_value` (Object)
 
 Returns the value of the resource from the new catalog.
@@ -111,6 +115,10 @@ Note that this is a pass-through of information provided in the Puppet catalog,
 
 Note also that if the diff represents addition of a resource, this will return `nil`, because the resource does not exist in the old catalog.
 
+#### `#old_location` (Hash)
+
+Returns a hash containing `:file` (equal to `#old_file`) and `:line` (equal to `#old_line`) when either is defined. Returns `nil` if both are undefined.
+
 #### `#old_value` (Object)
 
 Returns the value of the resource from the old catalog.
@@ -249,4 +257,5 @@ These methods are available for debugging or development purposes but are not gu
 - `#inspect` (String): Returns inspection of object
 - `#raw` (Array): Returns internal array data structure of the "diff"
 - `#to_h` (Hash): Returns object as a hash, where keys are above described methods
+- `#to_h_with_string_keys` (Hash): Returns object as a hash, where keys are above described methods; keys are strings, not symbols
 - `#[]` (Object): Retrieve indexed array elements from raw internal array object

doc/optionsref.md

@@ -27,7 +27,7 @@ Usage: octocatalog-diff [command line options]
         --bootstrap-then-exit        Bootstrap from-dir and/or to-dir and then exit
         --[no-]color                 Enable/disable colors in output
     -o, --output-file FILENAME       Output results into FILENAME
-        --output-format FORMAT       Output format: text,json
+        --output-format FORMAT       Output format: text,json,legacy_json
     -d, --[no-]debug                 Print debugging messages to STDERR
     -q, --[no-]quiet                 Quiet (no status messages except errors)
         --ignore "Type1[Title1],Type2[Title2],..."
@@ -708,10 +708,12 @@ to ignore any changes for any defined type where this tag is set. (<a href="../l
       <pre><code>--output-format FORMAT</code></pre>
     </td>
     <td valign=top>
-      Output format: text,json
+      Output format: text,json,legacy_json
     </td>
     <td valign=top>
-      Output format option (<a href="../lib/octocatalog-diff/cli/options/output_format.rb">output_format.rb</a>)
+      Output format option. 'text' is human readable text, 'json' is an array of differences
+identified by human readable keys (the preferred octocatalog-diff 1.x format), and 'legacy_json' is an
+array of differences, where each difference is an array (the octocatalog-diff 0.x format). (<a href="../lib/octocatalog-diff/cli/options/output_format.rb">output_format.rb</a>)
     </td>
   </tr>
 

lib/octocatalog-diff/api/v1/diff.rb

@@ -22,8 +22,13 @@ class Diff
         # Constructor: Accepts a diff in the traditional array format and stores it.
         # @param raw [Array] Diff in the traditional format
         def initialize(raw)
+          if raw.is_a?(OctocatalogDiff::API::V1::Diff)
+            @raw = raw.raw
+            return
+          end
+
           unless raw.is_a?(Array)
-            raise ArgumentError, 'OctocatalogDiff::API::V1::Diff#initialize expects Array argument'
+            raise ArgumentError, "OctocatalogDiff::API::V1::Diff#initialize expects Array argument (got #{raw.class})"
           end
           @raw = raw
         end
@@ -119,6 +124,22 @@ def new_line
           x.nil? ? nil : x['line']
         end
 
+        # Public: Get the "old" location, i.e. location in the "from" catalog
+        # @return [Hash] <file:, line:> of resource
+        def old_location
+          return nil if addition?
+          return @raw[3] if removal?
+          @raw[4]
+        end
+
+        # Public: Get the "new" location, i.e. location in the "to" catalog
+        # @return [Hash] <file:, line:> of resource
+        def new_location
+          return @raw[3] if addition?
+          return nil if removal?
+          @raw[5]
+        end
+
         # Public: Convert this object to a hash
         # @return [Hash] Hash with keys set by these methods
         def to_h
@@ -132,10 +153,20 @@ def to_h
             old_file: old_file,
             old_line: old_line,
             new_file: new_file,
-            new_line: new_line
+            new_line: new_line,
+            old_location: old_location,
+            new_location: new_location
           }
         end
 
+        # Public: Convert this object to a hash with string keys
+        # @return [Hash] Hash with keys set by these methods, with string keys
+        def to_h_with_string_keys
+          result = {}
+          to_h.each { |key, val| result[key.to_s] = val }
+          result
+        end
+
         # Public: String inspection
         # @return [String] String for inspection
         def inspect
@@ -147,24 +178,6 @@ def inspect
         def to_s
           raw.inspect
         end
-
-        private
-
-        # Private: Get the "old" location, i.e. location in the "from" catalog
-        # @return [Hash] <file:, line:> of resource
-        def old_location
-          return nil if addition?
-          return @raw[3] if removal?
-          @raw[4]
-        end
-
-        # Private: Get the "new" location, i.e. location in the "to" catalog
-        # @return [Hash] <file:, line:> of resource
-        def new_location
-          return @raw[3] if addition?
-          return nil if removal?
-          @raw[5]
-        end
       end
     end
   end

lib/octocatalog-diff/catalog-diff/display.rb

@@ -1,7 +1,9 @@
 # frozen_string_literal: true
 
+require_relative '../api/v1/diff'
 require_relative 'differ'
 require_relative 'display/json'
+require_relative 'display/legacy_json'
 require_relative 'display/text'
 
 module OctocatalogDiff
@@ -21,8 +23,9 @@ class Display
       # @param logger [Logger] Logger object
       # @return [String] Text output for provided diff
       def self.output(diff_in, options = {}, logger = nil)
-        diff = diff_in.is_a?(OctocatalogDiff::CatalogDiff::Differ) ? diff_in.diff : diff_in
-        raise ArgumentError, "text_output requires Array<Diff results>; passed in #{diff_in.class}" unless diff.is_a?(Array)
+        diff_x = diff_in.is_a?(OctocatalogDiff::CatalogDiff::Differ) ? diff_in.diff : diff_in
+        raise ArgumentError, "text_output requires Array<Diff results>; passed in #{diff_in.class}" unless diff_x.is_a?(Array)
+        diff = diff_x.map { |x| OctocatalogDiff::API::V1::Diff.new(x) }
 
         # req_format means 'requested format' because 'format' has a built-in meaning to Ruby
         req_format = options.fetch(:format, :color_text)
@@ -41,6 +44,9 @@ def self.output(diff_in, options = {}, logger = nil)
         when :json
           logger.debug 'Generating JSON output' if logger
           OctocatalogDiff::CatalogDiff::Display::Json.generate(diff, opts, logger)
+        when :legacy_json
+          logger.debug 'Generating Legacy JSON output' if logger
+          OctocatalogDiff::CatalogDiff::Display::LegacyJson.generate(diff, opts, logger)
         when :text
           logger.debug 'Generating non-colored text output' if logger
           OctocatalogDiff::CatalogDiff::Display::Text.generate(diff, opts.merge(color: false), logger)

lib/octocatalog-diff/catalog-diff/display/json.rb

@@ -7,7 +7,8 @@
 module OctocatalogDiff
   module CatalogDiff
     class Display
-      # Display the output from a diff in JSON format.
+      # Display the output from a diff in JSON format. This is the new format, used in octocatalog-diff
+      # 1.x, where each diff is represented by an hash with named keys.
       class Json < OctocatalogDiff::CatalogDiff::Display
         # Generate JSON representation of the 'diff' suitable for further analysis.
         # @param diff [Array<Diff results>] The diff which *must* be in this format
@@ -16,7 +17,7 @@ class Json < OctocatalogDiff::CatalogDiff::Display
         # @param _logger [Logger] Not used here
         def self.generate(diff, options = {}, _logger = nil)
           result = {
-            'diff' => diff
+            'diff' => diff.map(&:to_h_with_string_keys)
           }
           result['header'] = options[:header] unless options[:header].nil?
           result.to_json

lib/octocatalog-diff/catalog-diff/display/legacy_json.rb

@@ -0,0 +1,28 @@
+# frozen_string_literal: true
+
+require_relative '../display'
+
+require 'json'
+
+module OctocatalogDiff
+  module CatalogDiff
+    class Display
+      # Display the output from a diff in JSON format. This is the legacy format, used in octocatalog-diff
+      # 0.x, where each diff is represented by an array.
+      class LegacyJson < OctocatalogDiff::CatalogDiff::Display
+        # Generate JSON representation of the 'diff' suitable for further analysis.
+        # @param diff [Array<Diff results>] The diff which *must* be in this format
+        # @param options [Hash] Options which are:
+        #           - :header => [String] Header to print; no header is printed if not specified
+        # @param _logger [Logger] Not used here
+        def self.generate(diff, options = {}, _logger = nil)
+          result = {
+            'diff' => diff.map(&:raw)
+          }
+          result['header'] = options[:header] unless options[:header].nil?
+          result.to_json
+        end
+      end
+    end
+  end
+end

lib/octocatalog-diff/cli/options/output_format.rb

@@ -1,13 +1,15 @@
 # frozen_string_literal: true
 
-# Output format option
+# Output format option. 'text' is human readable text, 'json' is an array of differences
+# identified by human readable keys (the preferred octocatalog-diff 1.x format), and 'legacy_json' is an
+# array of differences, where each difference is an array (the octocatalog-diff 0.x format).
 # @param parser [OptionParser object] The OptionParser argument
 # @param options [Hash] Options hash being constructed; this is modified in this method.
 OctocatalogDiff::Cli::Options::Option.newoption(:output_format) do
   has_weight 100
 
   def parse(parser, options)
-    valid = %w(text json)
+    valid = %w(text json legacy_json)
     parser.on('--output-format FORMAT', "Output format: #{valid.join(',')}") do |fmt|
       raise ArgumentError, "Invalid format. Must be one of: #{valid.join(',')}" unless valid.include?(fmt)
       options[:format] = fmt.to_sym

lib/octocatalog-diff/cli/printer.rb

@@ -19,10 +19,13 @@ def initialize(options, logger)
       # The method to call externally, passing in diffs. This takes the appropriate action
       # based on options, which is either to write the result into an output file, or print
       # the result on STDOUT. Does not return anything.
-      # @param diffs [OctocatalogDiff::CatalogDiff::Differ] Difference array
+      # @param diffs [Array<Diffs>] Array of differences
       # @param from_dir [String] Directory in which "from" catalog was compiled
       # @param to_dir [String] Directory in which "to" catalog was compiled
       def printer(diffs, from_dir = nil, to_dir = nil)
+        unless diffs.is_a?(Array)
+          raise ArgumentError, "printer() expects an array, not #{diffs.class}"
+        end
         display_opts = @options.merge(compilation_from_dir: from_dir, compilation_to_dir: to_dir)
         diff_text = OctocatalogDiff::CatalogDiff::Display.output(diffs, display_opts, @logger)
         if @options[:output_file].nil?

spec/octocatalog-diff/integration/outputs_spec.rb

@@ -51,7 +51,7 @@
     expect(result[:output]).to match(pattern)
   end
 
-  it 'should write JSON output to a specified file' do
+  it 'should write 1.x JSON output to a specified file' do
     output_file = File.join(@tmpdir, 'octo-output.json')
     argv = default_argv.concat ['-o', output_file, '--output-format', 'json']
     result = OctocatalogDiff::Integration.integration(argv: argv)
@@ -63,6 +63,46 @@
     expect(data).to be_a_kind_of(Hash)
     expect(data['header']).to eq(nil)
     expect(data['diff']).to be_a_kind_of(Array)
+
+    answer = {
+      'diff_type' => '~',
+      'type' => 'File',
+      'title' => '/usr/bin/node-waf',
+      'structure' => %w(parameters ensure),
+      'old_value' => '/usr/share/nvm/0.8.11/bin/node-waf',
+      'new_value' => 'link',
+      'old_file' => '/environments/production/modules/nodejs/manifests/init.pp',
+      'old_line' => 55,
+      'new_file' => '/environments/production/modules/nodejs/manifests/init.pp',
+      'new_line' => 55,
+      'old_location' => { 'file' => '/environments/production/modules/nodejs/manifests/init.pp', 'line' => 55 },
+      'new_location' => { 'file' => '/environments/production/modules/nodejs/manifests/init.pp', 'line' => 55 }
+    }
+    expect(data['diff']).to include(answer)
+  end
+
+  it 'should write 0.x JSON output to a specified file' do
+    output_file = File.join(@tmpdir, 'octo-output.json')
+    argv = default_argv.concat ['-o', output_file, '--output-format', 'legacy_json']
+    result = OctocatalogDiff::Integration.integration(argv: argv)
+    expect(result[:exitcode]).to eq(2), OctocatalogDiff::Integration.format_exception(result)
+    expect(result[:logs]).to match(/Wrote diff to #{Regexp.escape(output_file)}/)
+    expect(File.file?(output_file)).to eq(true)
+    content = File.read(output_file)
+    data = JSON.parse(content)
+    expect(data).to be_a_kind_of(Hash)
+    expect(data['header']).to eq(nil)
+    expect(data['diff']).to be_a_kind_of(Array)
+
+    answer = [
+      '!',
+      "File\f/usr/bin/npm\fparameters\ftarget",
+      '/usr/share/nvm/0.8.11/bin/npm',
+      nil,
+      { 'file' => '/environments/production/modules/nodejs/manifests/init.pp', 'line' => 46 },
+      { 'file' => '/environments/production/modules/nodejs/manifests/init.pp', 'line' => 46 }
+    ]
+    expect(data['diff']).to include(answer)
   end
 
   it 'should write JSON output to the screen' do

spec/octocatalog-diff/tests/api/v1/diff_spec.rb

@@ -273,9 +273,44 @@
     end
   end
 
+  describe '#old_location' do
+    it 'should return nil for addition' do
+      testobj = described_class.new(add_2)
+      expect(testobj.old_location).to be_nil
+    end
+
+    it 'should return location for removal' do
+      testobj = described_class.new(del_2)
+      expect(testobj.old_location).to eq(loc_1)
+    end
+
+    it 'should return location for change' do
+      testobj = described_class.new(chg_2)
+      expect(testobj.old_location).to eq(loc_1)
+    end
+  end
+
+  describe '#new_location' do
+    it 'should return location for addition' do
+      testobj = described_class.new(add_2)
+      expect(testobj.new_location).to eq(loc_2)
+    end
+
+    it 'should return nil for removal' do
+      testobj = described_class.new(del_2)
+      expect(testobj.new_location).to be_nil
+    end
+
+    it 'should return location for change' do
+      testobj = described_class.new(chg_2)
+      expect(testobj.new_location).to eq(loc_2)
+    end
+  end
+
   describe '#to_h' do
     it 'should return a hash with the expected keys and values' do
-      methods = %w(diff_type type title structure old_value new_value old_line new_line old_file new_file)
+      methods = %w(diff_type type title structure old_value new_value)
+                .concat %w(old_line new_line old_file new_file old_location new_location)
       testobj = described_class.new(chg_2)
       result = testobj.to_h
       methods.each do |method_name|
@@ -286,6 +321,20 @@
     end
   end
 
+  describe '#to_h_with_string_keys' do
+    it 'should return a hash with the expected keys and values' do
+      methods = %w(diff_type type title structure old_value new_value)
+                .concat %w(old_line new_line old_file new_file old_location new_location)
+      testobj = described_class.new(chg_2)
+      result = testobj.to_h_with_string_keys
+      methods.each do |method_name|
+        method = method_name.to_sym
+        expect(result.key?(method.to_s)).to eq(true)
+        expect(result[method.to_s]).to eq(testobj.send(method))
+      end
+    end
+  end
+
   describe '#inspect' do
     it 'should return a string' do
       testobj = described_class.new(chg_2)
@@ -301,6 +350,12 @@
   end
 
   describe '#initialize' do
+    it 'should set up raw object when called with an instance of itself' do
+      obj1 = described_class.new(chg_2)
+      testobj = described_class.new(obj1)
+      expect(testobj.raw).to eq(chg_2)
+    end
+
     it 'should raise ArgumentError if called with a non-array' do
       expect { described_class.new('foo') }.to raise_error(ArgumentError)
     end