From 890184b15b424c9c722ddb6bea166a36acac87f6 Mon Sep 17 00:00:00 2001
From: Bernard Pietraga <bernard.pietraga@gmail.com>
Date: Mon, 12 Nov 2018 10:58:04 +0100
Subject: [PATCH 01/14] (maint) move parameter and property logic to classes

---
 lib/puppet/resource_api.rb                    |  99 +++++---------
 lib/puppet/resource_api/parameter.rb          |  42 ++++++
 lib/puppet/resource_api/property.rb           |  64 +++++++++
 .../resource_api/read_only_parameter.rb       |  37 +++++
 spec/puppet/resource_api/parameter_spec.rb    |  95 +++++++++++++
 spec/puppet/resource_api/property_spec.rb     | 128 ++++++++++++++++++
 .../resource_api/read_only_parameter_spec.rb  |  85 ++++++++++++
 7 files changed, 487 insertions(+), 63 deletions(-)
 create mode 100644 lib/puppet/resource_api/parameter.rb
 create mode 100644 lib/puppet/resource_api/property.rb
 create mode 100644 lib/puppet/resource_api/read_only_parameter.rb
 create mode 100644 spec/puppet/resource_api/parameter_spec.rb
 create mode 100644 spec/puppet/resource_api/property_spec.rb
 create mode 100644 spec/puppet/resource_api/read_only_parameter_spec.rb

diff --git a/lib/puppet/resource_api.rb b/lib/puppet/resource_api.rb
index 3f692b94..d150a1fd 100644
--- a/lib/puppet/resource_api.rb
+++ b/lib/puppet/resource_api.rb
@@ -1,7 +1,10 @@
 require 'pathname'
 require 'puppet/resource_api/data_type_handling'
 require 'puppet/resource_api/glue'
+require 'puppet/resource_api/parameter'
+require 'puppet/resource_api/property'
 require 'puppet/resource_api/puppet_context' unless RUBY_PLATFORM == 'java'
+require 'puppet/resource_api/read_only_parameter'
 require 'puppet/resource_api/type_definition'
 require 'puppet/resource_api/version'
 require 'puppet/type'
@@ -180,7 +183,30 @@ def to_resource
                               :newproperty
                             end
 
-        send(param_or_property, name.to_sym) do
+        parent = if param_or_property == :newproperty
+                   Puppet::ResourceApi::Property
+                 elsif options[:behaviour] == :read_only
+                   Puppet::ResourceApi::ReadOnlyParameter
+                 else
+                   Puppet::ResourceApi::Parameter
+                 end
+
+        # This part fo code creates new parameter or property. It takes the
+        # param_and_property which is one of methods and then translates it to
+        # the i.ex.
+        # newparam(:param_name, parent: Puppet::ResourceApi::Parameter) do; end;
+        send(param_or_property, name.to_sym, parent: parent) do
+          # The initialize takes passed hash with resource.
+          # The method pass needed arguments, depending on parent to:
+          # - Puppet::ResourceApi::Property
+          # - Puppet::ResourceApi::Parameter
+          # - Puppet::ResourceApi::ReadOnlyParameter
+          # @param resource_hash [{ resource: #<Puppet::Type::TypeName> }] is
+          # one element hash referencing the resource object.
+          def initialize(resource_hash)
+            super(name, data_type, data_definition, resource_hash[:resource])
+          end
+
           unless options[:type]
             raise Puppet::DevError, "#{definition[:name]}.#{name} has no type"
           end
@@ -226,69 +252,16 @@ def insync?(is)
             options[:type],
           )
 
-          if param_or_property == :newproperty
-            define_method(:should) do
-              if name == :ensure && rs_value.is_a?(String)
-                rs_value.to_sym
-              elsif rs_value == false
-                # work around https://tickets.puppetlabs.com/browse/PUP-2368
-                :false # rubocop:disable Lint/BooleanSymbol
-              elsif rs_value == true
-                # work around https://tickets.puppetlabs.com/browse/PUP-2368
-                :true # rubocop:disable Lint/BooleanSymbol
-              else
-                rs_value
-              end
-            end
-
-            define_method(:should=) do |value|
-              @shouldorig = value
-
-              if name == :ensure
-                value = value.to_s
-              end
-
-              # Puppet requires the @should value to always be stored as an array. We do not use this
-              # for anything else
-              # @see Puppet::Property.should=(value)
-              @should = [
-                Puppet::ResourceApi::DataTypeHandling.mungify(
-                  type,
-                  value,
-                  "#{definition[:name]}.#{name}",
-                  Puppet::ResourceApi.caller_is_resource_app?,
-                ),
-              ]
-            end
-
-            # used internally
-            # @returns the final mungified value of this property
-            define_method(:rs_value) do
-              @should ? @should.first : @should
-            end
-          else
-            define_method(:value) do
-              @value
-            end
-
-            define_method(:value=) do |value|
-              if options[:behaviour] == :read_only
-                raise Puppet::ResourceError, "Attempting to set `#{name}` read_only attribute value to `#{value}`"
-              end
-
-              @value = Puppet::ResourceApi::DataTypeHandling.mungify(
-                type,
-                value,
-                "#{definition[:name]}.#{name}",
-                Puppet::ResourceApi.caller_is_resource_app?,
-              )
-            end
+          # inside of parameter or property provide method to check and return
+          # data type.
+          define_method(:data_type) do
+            type
+          end
 
-            # used internally
-            # @returns the final mungified value of this parameter
-            define_method(:rs_value) do
-              @value
-            end
+          # inside of parameter or property provide method to check and return
+          # the definition hash object.
+          define_method(:data_definition) do
+            definition
           end
 
           # puppet symbolizes some values through puppet/parameter/value.rb (see .convert()), but (especially) Enums
diff --git a/lib/puppet/resource_api/parameter.rb b/lib/puppet/resource_api/parameter.rb
new file mode 100644
index 00000000..e18ddc6a
--- /dev/null
+++ b/lib/puppet/resource_api/parameter.rb
@@ -0,0 +1,42 @@
+require 'puppet/parameter'
+
+module Puppet; module ResourceApi; end; end # predeclare the main module # rubocop:disable Style/Documentation,Style/ClassAndModuleChildren
+
+# Class containing parameter functionality for ResourceApi.
+class Puppet::ResourceApi::Parameter < Puppet::Parameter
+  # This initialize takes arguments and sets up new parameter.
+  # @param name the name used for reporting errors
+  # @param type the type instance
+  # @param definition the definition of the property
+  # @param resource the resource instance which is passed to the parent class.
+  def initialize(name, type, definiton, resource)
+    @name = name
+    @type = type
+    @definiton = definiton
+    super(resource: resource) # Pass resource to parent Puppet class.
+  end
+
+  # This method handles return of assigned value from parameter.
+  # @return [type] the type value
+  def value
+    @value
+  end
+
+  # This method assigns value to the parameter and cleans value.
+  # @param value the value to be set and clean
+  # @return [type] the cleaned value
+  def value=(value)
+    @value = Puppet::ResourceApi::DataTypeHandling.mungify(
+      @type,
+      value,
+      "#{@definiton[:name]}.#{name}",
+      Puppet::ResourceApi.caller_is_resource_app?,
+    )
+  end
+
+  # used internally
+  # @returns the final mungified value of this parameter
+  def rs_value
+    @value
+  end
+end
diff --git a/lib/puppet/resource_api/property.rb b/lib/puppet/resource_api/property.rb
new file mode 100644
index 00000000..8dfd5b38
--- /dev/null
+++ b/lib/puppet/resource_api/property.rb
@@ -0,0 +1,64 @@
+require 'puppet/property'
+
+module Puppet; module ResourceApi; end; end # predeclare the main module # rubocop:disable Style/Documentation,Style/ClassAndModuleChildren
+
+# Class containing property functionality for ResourceApi.
+class Puppet::ResourceApi::Property < Puppet::Property
+  # This initialize takes arguments and sets up new property.
+  # @param name the name used for reporting errors
+  # @param type the type instance
+  # @param definition the definition of the property
+  # @param resource the resource instance which is passed to the parent class.
+  def initialize(name, type, definition, resource)
+    @name = name
+    @type = type
+    @definition = definition
+    # Pass resource and should to parent Puppet class.
+    super(resource: resource, should: should)
+  end
+
+  # This method returns value of the property.
+  # @return [type] the property value
+  def should
+    if @name == :ensure && rs_value.is_a?(String)
+      rs_value.to_sym
+    elsif rs_value == false
+      # work around https://tickets.puppetlabs.com/browse/PUP-2368
+      :false # rubocop:disable Lint/BooleanSymbol
+    elsif rs_value == true
+      # work around https://tickets.puppetlabs.com/browse/PUP-2368
+      :true # rubocop:disable Lint/BooleanSymbol
+    else
+      rs_value
+    end
+  end
+
+  # This method sets and returns value of the property and sets @shouldorig.
+  # @param value the value to be set and clean
+  # @return [type] the property value
+  def should=(value)
+    @shouldorig = value
+
+    if @name == :ensure
+      value = value.to_s
+    end
+
+    # Puppet requires the @should value to always be stored as an array. We do not use this
+    # for anything else
+    # @see Puppet::Property.should=(value)
+    @should = [
+      Puppet::ResourceApi::DataTypeHandling.mungify(
+        @type,
+        value,
+        "#{@definition[:name]}.#{@name}",
+        Puppet::ResourceApi.caller_is_resource_app?,
+      ),
+    ]
+  end
+
+  # used internally
+  # @returns the final mungified value of this property
+  def rs_value
+    @should ? @should.first : @should
+  end
+end
diff --git a/lib/puppet/resource_api/read_only_parameter.rb b/lib/puppet/resource_api/read_only_parameter.rb
new file mode 100644
index 00000000..dd00ddd8
--- /dev/null
+++ b/lib/puppet/resource_api/read_only_parameter.rb
@@ -0,0 +1,37 @@
+require 'puppet/parameter'
+
+module Puppet; module ResourceApi; end; end # predeclare the main module # rubocop:disable Style/Documentation,Style/ClassAndModuleChildren
+
+# Class containing read only parameter functionality for ResourceApi.
+class Puppet::ResourceApi::ReadOnlyParameter < Puppet::Property
+  # This initialize takes arguments and sets up new parameter.
+  # @param name the name used for reporting errors
+  # @param type the type instance
+  # @param definition the definition of the property
+  # @param resource the resource instance which is passed to the parent class.
+  def initialize(name, type, definition, resource)
+    @name = name
+    @type = type
+    @definition = definition
+    super(resource: resource) # Pass resource to parent Puppet class.
+  end
+
+  # This method handles return of assigned value from parameter.
+  # @return [type] the type value
+  def value # rubocop:disable Style/TrivialAccessors
+    @value
+  end
+
+  # This method raises error if the there is attempt to set value in parameter.
+  # @return [Puppet::ResourceError] the error with information.
+  def value=(value)
+    raise Puppet::ResourceError,
+          "Attempting to set `#{@name}` read_only attribute value to `#{value}`"
+  end
+
+  # used internally
+  # @returns the final mungified value of this parameter
+  def rs_value
+    @value
+  end
+end
diff --git a/spec/puppet/resource_api/parameter_spec.rb b/spec/puppet/resource_api/parameter_spec.rb
new file mode 100644
index 00000000..a86e7672
--- /dev/null
+++ b/spec/puppet/resource_api/parameter_spec.rb
@@ -0,0 +1,95 @@
+require 'spec_helper'
+
+RSpec.describe Puppet::ResourceApi::Parameter do
+  subject(:parameter) do
+    described_class.new(name, type, definition, resource)
+  end
+
+  let(:definition) { {} }
+  let(:log_sink) { [] }
+  let(:name) { 'some_parameter' }
+  let(:resource) { {} }
+  let(:result) { 'value' }
+  let(:strict_level) { :error }
+  let(:type) { Puppet::Pops::Types::PStringType.new(nil) }
+  let(:value) { 'value' }
+
+  before(:each) do
+    # set default to strictest setting
+    # by default Puppet runs at warning level
+    Puppet.settings[:strict] = strict_level
+    # Enable debug logging
+    Puppet.debug = true
+
+    Puppet::Util::Log.newdestination(Puppet::Test::LogCollector.new(log_sink))
+  end
+
+  after(:each) do
+    Puppet::Util::Log.close_all
+  end
+
+  it { expect { described_class.new(nil) }.to raise_error ArgumentError, %r{wrong number of arguments} }
+  it { expect { described_class.new(name, type, definition, resource) }.not_to raise_error }
+
+  describe '#value=(value)' do
+    context 'when value is string' do
+      context 'when the value is set with string value' do
+        it('value is returned') do
+          expect(parameter.value=(value)).to eq result
+        end
+
+        it('value=(value) is called') do
+          allow(described_class).to receive(:value=).with(value).and_return(result)
+          described_class.value=(value) # rubocop:disable Style/RedundantParentheses, Layout/SpaceAroundOperators
+          expect(described_class).to have_received(:value=).once
+        end
+      end
+    end
+  end
+
+  describe '#value' do
+    context 'when the value is not set' do
+      it('nil is returned') do
+        expect(parameter.value).to eq nil
+      end
+    end
+
+    context 'when value is string' do
+      context 'when the value is set' do
+        it('value is called') do
+          allow(described_class).to receive(:value).and_return(result)
+          described_class.value
+          expect(described_class).to have_received(:value).once
+        end
+
+        it('value is returned') do
+          parameter.instance_variable_set(:@value, value)
+          expect(parameter.value).to eq result
+        end
+      end
+    end
+  end
+
+  describe '#rs_value' do
+    context 'when the value is not set' do
+      it('nil is returned') do
+        expect(parameter.value).to eq nil
+      end
+    end
+
+    context 'when value is string' do
+      context 'when the value is set' do
+        it('value is called') do
+          allow(described_class).to receive(:rs_value).and_return(result)
+          described_class.rs_value
+          expect(described_class).to have_received(:rs_value).once
+        end
+
+        it('value is returned') do
+          parameter.instance_variable_set(:@value, value)
+          expect(parameter.rs_value).to eq result
+        end
+      end
+    end
+  end
+end
diff --git a/spec/puppet/resource_api/property_spec.rb b/spec/puppet/resource_api/property_spec.rb
new file mode 100644
index 00000000..5c0b1fb5
--- /dev/null
+++ b/spec/puppet/resource_api/property_spec.rb
@@ -0,0 +1,128 @@
+require 'spec_helper'
+
+RSpec.describe Puppet::ResourceApi::Property do
+  subject(:property) do
+    described_class.new(name, type, definition, resource)
+  end
+
+  let(:definition) { {} }
+  let(:log_sink) { [] }
+  let(:name) { 'some_property' }
+  let(:resource) { {} }
+  let(:result) { 'value' }
+  let(:strict_level) { :error }
+  let(:type) { Puppet::Pops::Types::PStringType.new(nil) }
+
+  before(:each) do
+    # set default to strictest setting
+    # by default Puppet runs at warning level
+    Puppet.settings[:strict] = strict_level
+    # Enable debug logging
+    Puppet.debug = true
+
+    Puppet::Util::Log.newdestination(Puppet::Test::LogCollector.new(log_sink))
+  end
+
+  after(:each) do
+    Puppet::Util::Log.close_all
+  end
+
+  it { expect { described_class.new(nil) }.to raise_error ArgumentError, %r{wrong number of arguments} }
+  it { expect { described_class.new(name, type, definition, resource) }.not_to raise_error }
+
+  describe '#should=(value)' do
+    context 'when value is string' do
+      context 'when the should is set with string value' do
+        let(:value) { 'value' }
+
+        it('value is called') do
+          allow(described_class).to receive(:should=).with(value).and_return(result)
+          allow(described_class).to receive(:rs_value).and_return(result)
+          described_class.should=(value) # rubocop:disable Style/RedundantParentheses, Layout/SpaceAroundOperators
+          expect(described_class).to have_received(:should=).once
+        end
+
+        it('value is returned') do
+          expect(property.should=(value)).to eq result
+        end
+      end
+    end
+  end
+
+  describe '#should' do
+    context 'when value is boolean' do
+      context 'when the @should is set' do
+        let(:should_true_value) { [true] } # rs_value takes value in array
+        let(:true_result) { :true }
+        let(:should_false_value) { [false] } # rs_value takes value in array
+        let(:false_result) { :false }
+
+        it('true symbol value is returned') do
+          property.instance_variable_set(:@should, should_true_value)
+          expect(property.should).to eq true_result
+        end
+
+        it('false symbol value is returned') do
+          property.instance_variable_set(:@should, should_false_value)
+          expect(property.should).to eq false_result
+        end
+      end
+    end
+
+    context 'when value is string' do
+      context 'when the name is :ensure' do
+        context 'when the @should is set' do
+          let(:should_value) { ['present'] } # rs_value takes value in array
+          let(:name) { :ensure }
+          let(:result) { :present }
+
+          it('symbol value is returned') do
+            property.instance_variable_set(:@should, should_value)
+            property.instance_variable_set(:@name, name)
+            expect(property.should).to eq result
+          end
+        end
+      end
+
+      context 'when the should is set' do
+        let(:should_value) { ['value'] } # rs_value takes value in array
+
+        it('value is called') do
+          allow(described_class).to receive(:should).and_return(result)
+          described_class.should
+          expect(described_class).to have_received(:should).once
+        end
+
+        it('value is returned') do
+          property.instance_variable_set(:@should, should_value)
+          expect(property.should).to eq result
+        end
+      end
+    end
+  end
+
+  describe '#rs_value' do
+    context 'when the value is not set' do
+      it('nil is returned') do
+        expect(property.value).to eq nil
+      end
+    end
+
+    context 'when value is string' do
+      let(:should_value) { ['value'] }
+
+      context 'when the value is set' do
+        it('value is called') do
+          allow(described_class).to receive(:rs_value).and_return(result)
+          described_class.rs_value
+          expect(described_class).to have_received(:rs_value).once
+        end
+
+        it('value is returned') do
+          property.instance_variable_set(:@should, should_value)
+          expect(property.rs_value).to eq result
+        end
+      end
+    end
+  end
+end
diff --git a/spec/puppet/resource_api/read_only_parameter_spec.rb b/spec/puppet/resource_api/read_only_parameter_spec.rb
new file mode 100644
index 00000000..c166ad90
--- /dev/null
+++ b/spec/puppet/resource_api/read_only_parameter_spec.rb
@@ -0,0 +1,85 @@
+require 'spec_helper'
+
+RSpec.describe Puppet::ResourceApi::ReadOnlyParameter do
+  subject(:read_only_parameter) do
+    described_class.new(name, type, definition, resource)
+  end
+
+  let(:definition) { {} }
+  let(:log_sink) { [] }
+  let(:name) { 'some_parameter' }
+  let(:resource) { {} }
+  let(:result) { 'value' }
+  let(:strict_level) { :error }
+  let(:type) { Puppet::Pops::Types::PStringType }
+  let(:value) { 'value' }
+
+  before(:each) do
+    # set default to strictest setting
+    # by default Puppet runs at warning level
+    Puppet.settings[:strict] = strict_level
+    # Enable debug logging
+    Puppet.debug = true
+
+    Puppet::Util::Log.newdestination(Puppet::Test::LogCollector.new(log_sink))
+  end
+
+  after(:each) do
+    Puppet::Util::Log.close_all
+  end
+
+  it { expect { described_class.new(nil) }.to raise_error ArgumentError, %r{wrong number of arguments} }
+  it { expect { described_class.new(name, type, definition, resource) }.not_to raise_error }
+
+  describe '#value=(value)' do
+    context 'when called from `puppet resource`' do
+      context 'when the value set attempt is performed' do
+        it 'value set fails' do
+          expect { read_only_parameter.value=(value) }.to raise_error Puppet::ResourceError, %r{Attempting to set `some_parameter` read_only attribute value to `value`} # rubocop:disable Style/RedundantParentheses, Layout/SpaceAroundOperators
+        end
+      end
+    end
+  end
+
+  describe '#value' do
+    context 'when value is string' do
+      context 'when the value is set' do
+        before(:each) do
+          allow(described_class).to receive(:value).and_return(result)
+        end
+
+        it('value is called') do
+          described_class.value
+          expect(described_class).to have_received(:value).once
+        end
+
+        it('value is returned') do
+          expect(described_class.value).to eq result
+        end
+      end
+    end
+  end
+
+  describe '#rs_value' do
+    context 'when the value is not set' do
+      it('nil is returned') do
+        expect(read_only_parameter.value).to eq nil
+      end
+    end
+
+    context 'when value is string' do
+      context 'when the value is set' do
+        it('value is called') do
+          allow(described_class).to receive(:rs_value).and_return(result)
+          described_class.rs_value
+          expect(described_class).to have_received(:rs_value).once
+        end
+
+        it('value is returned') do
+          read_only_parameter.instance_variable_set(:@value, value)
+          expect(read_only_parameter.rs_value).to eq result
+        end
+      end
+    end
+  end
+end

From 02867d11d333afa1e3f8c9bad019528056296835 Mon Sep 17 00:00:00 2001
From: Bernard Pietraga <bernard.pietraga@gmail.com>
Date: Thu, 22 Nov 2018 13:00:53 +0100
Subject: [PATCH 02/14] (maint) move value handling logic to ValueCreator

---
 lib/puppet/resource_api.rb                    |  91 +------
 lib/puppet/resource_api/parameter.rb          |   6 +
 lib/puppet/resource_api/property.rb           |  14 ++
 .../resource_api/read_only_parameter.rb       |   6 +
 lib/puppet/resource_api/value_creator.rb      |  81 +++++++
 .../puppet/resource_api/value_creator_spec.rb | 224 ++++++++++++++++++
 6 files changed, 344 insertions(+), 78 deletions(-)
 create mode 100644 lib/puppet/resource_api/value_creator.rb
 create mode 100644 spec/puppet/resource_api/value_creator_spec.rb

diff --git a/lib/puppet/resource_api.rb b/lib/puppet/resource_api.rb
index d150a1fd..48f5049e 100644
--- a/lib/puppet/resource_api.rb
+++ b/lib/puppet/resource_api.rb
@@ -6,6 +6,7 @@
 require 'puppet/resource_api/puppet_context' unless RUBY_PLATFORM == 'java'
 require 'puppet/resource_api/read_only_parameter'
 require 'puppet/resource_api/type_definition'
+require 'puppet/resource_api/value_creator'
 require 'puppet/resource_api/version'
 require 'puppet/type'
 require 'puppet/util/network_device'
@@ -196,6 +197,10 @@ def to_resource
         # the i.ex.
         # newparam(:param_name, parent: Puppet::ResourceApi::Parameter) do; end;
         send(param_or_property, name.to_sym, parent: parent) do
+          unless options[:type]
+            raise Puppet::DevError, "#{definition[:name]}.#{name} has no type"
+          end
+
           # The initialize takes passed hash with resource.
           # The method pass needed arguments, depending on parent to:
           # - Puppet::ResourceApi::Property
@@ -207,46 +212,12 @@ def initialize(resource_hash)
             super(name, data_type, data_definition, resource_hash[:resource])
           end
 
-          unless options[:type]
-            raise Puppet::DevError, "#{definition[:name]}.#{name} has no type"
-          end
-
           if options[:desc]
             desc "#{options[:desc]} (a #{options[:type]})"
           else
             warn("#{definition[:name]}.#{name} has no docs")
           end
 
-          if options[:behaviour] == :namevar
-            isnamevar
-          end
-
-          # read-only values do not need type checking, but can have default values
-          if options[:behaviour] != :read_only && options.key?(:default)
-            if options.key? :default
-              if options[:default] == false
-                # work around https://tickets.puppetlabs.com/browse/PUP-2368
-                defaultto :false # rubocop:disable Lint/BooleanSymbol
-              elsif options[:default] == true
-                # work around https://tickets.puppetlabs.com/browse/PUP-2368
-                defaultto :true # rubocop:disable Lint/BooleanSymbol
-              else
-                # marshal the default option to decouple that from the actual value.
-                # we cache the dumped value in `marshalled`, but use a block to unmarshal
-                # everytime the value is requested. Objects that can't be marshalled
-                # See https://stackoverflow.com/a/8206537/4918
-                marshalled = Marshal.dump(options[:default])
-                defaultto { Marshal.load(marshalled) } # rubocop:disable Security/MarshalLoad
-              end
-            end
-          end
-
-          if name == :ensure
-            def insync?(is)
-              rs_value.to_s == is.to_s
-            end
-          end
-
           type = Puppet::ResourceApi::DataTypeHandling.parse_puppet_type(
             name,
             options[:type],
@@ -264,44 +235,19 @@ def insync?(is)
             definition
           end
 
-          # puppet symbolizes some values through puppet/parameter/value.rb (see .convert()), but (especially) Enums
-          # are strings. specifying a munge block here skips the value_collection fallback in puppet/parameter.rb's
-          # default .unsafe_munge() implementation.
-          munge { |v| v }
-
           # provide hints to `puppet type generate` for better parsing
           if type.instance_of? Puppet::Pops::Types::POptionalType
             type = type.type
           end
 
-          case type
-          when Puppet::Pops::Types::PStringType
-            # require any string value
-            Puppet::ResourceApi.def_newvalues(self, param_or_property, %r{})
-          when Puppet::Pops::Types::PBooleanType
-            Puppet::ResourceApi.def_newvalues(self, param_or_property, 'true', 'false')
-            aliasvalue true, 'true'
-            aliasvalue false, 'false'
-            aliasvalue :true, 'true' # rubocop:disable Lint/BooleanSymbol
-            aliasvalue :false, 'false' # rubocop:disable Lint/BooleanSymbol
-
-          when Puppet::Pops::Types::PIntegerType
-            Puppet::ResourceApi.def_newvalues(self, param_or_property, %r{^-?\d+$})
-          when Puppet::Pops::Types::PFloatType, Puppet::Pops::Types::PNumericType
-            Puppet::ResourceApi.def_newvalues(self, param_or_property, Puppet::Pops::Patterns::NUMERIC)
-          end
-
-          if param_or_property == :newproperty
-            # stop puppet from trying to call into the provider when
-            # no pre-defined values have been specified
-            # "This is not the provider you are looking for." -- Obi-Wan Kaniesobi.
-            def call_provider(value); end
-          end
-
-          case options[:type]
-          when 'Enum[present, absent]'
-            Puppet::ResourceApi.def_newvalues(self, param_or_property, 'absent', 'present')
-          end
+          # Initialize ValueCreator and call create_values which creates alias
+          # values and default values for properties and params.
+          Puppet::ResourceApi::ValueCreator.new(
+            self,
+            type,
+            param_or_property,
+            options,
+          ).create_values
         end
       end
 
@@ -555,17 +501,6 @@ def self.class_name_from_type_name(type_name)
     type_name.to_s.split('_').map(&:capitalize).join
   end
 
-  # Add the value to `this` property or param, depending on whether param_or_property is `:newparam`, or `:newproperty`
-  def self.def_newvalues(this, param_or_property, *values)
-    if param_or_property == :newparam
-      this.newvalues(*values)
-    else
-      values.each do |v|
-        this.newvalue(v) {}
-      end
-    end
-  end
-
   def self.caller_is_resource_app?
     caller.any? { |c| c.match(%r{application/resource.rb:}) }
   end
diff --git a/lib/puppet/resource_api/parameter.rb b/lib/puppet/resource_api/parameter.rb
index e18ddc6a..e1f5c315 100644
--- a/lib/puppet/resource_api/parameter.rb
+++ b/lib/puppet/resource_api/parameter.rb
@@ -39,4 +39,10 @@ def value=(value)
   def rs_value
     @value
   end
+
+  # puppet symbolizes some values through puppet/parameter/value.rb
+  # (see .convert()), but (especially) Enums are strings. specifying a
+  # munge block here skips the value_collection fallback in
+  # puppet/parameter.rb's default .unsafe_munge() implementation.
+  munge { |v| v }
 end
diff --git a/lib/puppet/resource_api/property.rb b/lib/puppet/resource_api/property.rb
index 8dfd5b38..cef9c940 100644
--- a/lib/puppet/resource_api/property.rb
+++ b/lib/puppet/resource_api/property.rb
@@ -13,6 +13,8 @@ def initialize(name, type, definition, resource)
     @name = name
     @type = type
     @definition = definition
+    # Define class method insync?(is) if the name is :ensure
+    def_insync? if @name == :ensure
     # Pass resource and should to parent Puppet class.
     super(resource: resource, should: should)
   end
@@ -61,4 +63,16 @@ def should=(value)
   def rs_value
     @should ? @should.first : @should
   end
+
+  # method added only for the :ensure property, add option to check if the
+  # rs_value matches is
+  def def_insync?
+    self.class.send(:define_method, :insync?) { |is| rs_value.to_s == is.to_s }
+  end
+
+  # puppet symbolizes some values through puppet/parameter/value.rb
+  # (see .convert()), but (especially) Enums are strings. specifying a
+  # munge block here skips the value_collection fallback in
+  # puppet/parameter.rb's default .unsafe_munge() implementation.
+  munge { |v| v }
 end
diff --git a/lib/puppet/resource_api/read_only_parameter.rb b/lib/puppet/resource_api/read_only_parameter.rb
index dd00ddd8..5091f7ec 100644
--- a/lib/puppet/resource_api/read_only_parameter.rb
+++ b/lib/puppet/resource_api/read_only_parameter.rb
@@ -34,4 +34,10 @@ def value=(value)
   def rs_value
     @value
   end
+
+  # puppet symbolizes some values through puppet/parameter/value.rb
+  # (see .convert()), but (especially) Enums are strings. specifying a
+  # munge block here skips the value_collection fallback in
+  # puppet/parameter.rb's default .unsafe_munge() implementation.
+  munge { |v| v }
 end
diff --git a/lib/puppet/resource_api/value_creator.rb b/lib/puppet/resource_api/value_creator.rb
new file mode 100644
index 00000000..dadb1c55
--- /dev/null
+++ b/lib/puppet/resource_api/value_creator.rb
@@ -0,0 +1,81 @@
+module Puppet; module ResourceApi; end; end # predeclare the main module # rubocop:disable Style/Documentation,Style/ClassAndModuleChildren
+
+# This class is responsible for setting default and alias values for the
+# resource class.
+class Puppet::ResourceApi::ValueCreator
+  def initialize(resource_class, data_type, param_or_property, options = {})
+    @resource_class = resource_class
+    @data_type = data_type
+    @param_or_property = param_or_property
+    @options = options
+  end
+
+  # This method is responsible for all setup of the value mapping for desired
+  # resource class.
+  def create_values
+    @resource_class.isnamevar if @options[:behaviour] == :namevar
+
+    # read-only values do not need type checking, but can have default values
+    if @options[:behaviour] != :read_only && @options.key?(:default)
+      if @options.key? :default
+        if @options[:default] == false
+          # work around https://tickets.puppetlabs.com/browse/PUP-2368
+          @resource_class.defaultto :false # rubocop:disable Lint/BooleanSymbol
+        elsif @options[:default] == true
+          # work around https://tickets.puppetlabs.com/browse/PUP-2368
+          @resource_class.defaultto :true # rubocop:disable Lint/BooleanSymbol
+        else
+          # marshal the default option to decouple that from the actual value.
+          # we cache the dumped value in `marshalled`, but use a block to
+          # unmarshal everytime the value is requested. Objects that can't be
+          # marshalled
+          # See https://stackoverflow.com/a/8206537/4918
+          marshalled = Marshal.dump(@options[:default])
+          @resource_class.defaultto { Marshal.load(marshalled) } # rubocop:disable Security/MarshalLoad
+        end
+      end
+    end
+
+    case @data_type
+    when Puppet::Pops::Types::PStringType
+      # require any string value
+      def_newvalues(@resource_class, %r{})
+    when Puppet::Pops::Types::PBooleanType
+      def_newvalues(@resource_class, 'true', 'false')
+      @resource_class.aliasvalue true, 'true'
+      @resource_class.aliasvalue false, 'false'
+      @resource_class.aliasvalue :true, 'true' # rubocop:disable Lint/BooleanSymbol
+      @resource_class.aliasvalue :false, 'false' # rubocop:disable Lint/BooleanSymbol
+    when Puppet::Pops::Types::PIntegerType
+      def_newvalues(@resource_class, %r{^-?\d+$})
+    when Puppet::Pops::Types::PFloatType, Puppet::Pops::Types::PNumericType
+      def_newvalues(@resource_class, Puppet::Pops::Patterns::NUMERIC)
+    end
+
+    def_call_provider() if @param_or_property == :newproperty
+
+    case @options[:type]
+    when 'Enum[present, absent]'
+      def_newvalues(@resource_class, 'absent', 'present')
+    end
+  end
+
+  # Add the value to `this` property or param, depending on whether
+  # param_or_property is `:newparam`, or `:newproperty`.
+  def def_newvalues(this, *values)
+    if @param_or_property == :newparam
+      this.newvalues(*values)
+    else
+      values.each do |v|
+        this.newvalue(v) {}
+      end
+    end
+  end
+
+  # stop puppet from trying to call into the provider when
+  # no pre-defined values have been specified
+  # "This is not the provider you are looking for." -- Obi-Wan Kaniesobi.
+  def def_call_provider
+    @resource_class.send(:define_method, :call_provider) { |value| }
+  end
+end
diff --git a/spec/puppet/resource_api/value_creator_spec.rb b/spec/puppet/resource_api/value_creator_spec.rb
new file mode 100644
index 00000000..b486eae7
--- /dev/null
+++ b/spec/puppet/resource_api/value_creator_spec.rb
@@ -0,0 +1,224 @@
+require 'spec_helper'
+
+RSpec.describe Puppet::ResourceApi::ValueCreator do
+  subject(:instance) do
+    described_class.new(resource_class, data_type, param_or_property, options)
+  end
+
+  let(:resource_class) { Puppet::ResourceApi::Property }
+  let(:data_type) { Puppet::Pops::Types::PEnumType.new(['absent', 'present']) } # rubocop:disable Style/WordArray
+  let(:param_or_property) { :newproperty }
+  let(:options) do
+    {
+      type: 'Enum[present, absent]',
+      desc: 'Whether this resource should be present or absent on the target system.',
+      default: 'present',
+    }
+  end
+
+  before(:each) do
+    allow(resource_class).to receive(:newvalue)
+    allow(resource_class).to receive(:newvalues)
+    allow(resource_class).to receive(:aliasvalue)
+    allow(resource_class).to receive(:defaultto)
+    allow(resource_class).to receive(:isnamevar)
+  end
+
+  it { expect { described_class.new(nil) }.to raise_error ArgumentError, %r{wrong number of arguments} }
+  it { expect { instance }.not_to raise_error }
+
+  describe '#create_values' do
+    before(:each) do
+      instance.create_values
+    end
+
+    context 'when resource class is property' do
+      it 'resource_class has #call_provider defined' do
+        expect(resource_class.method_defined?(:call_provider)).to eq(true)
+      end
+
+      context 'when property is :ensure' do
+        it 'calls #newvalue twice' do
+          expect(Puppet::ResourceApi::Property).to have_received(:newvalue).twice
+        end
+      end
+
+      context 'when default is set' do
+        it 'calls #defaultto once' do
+          expect(resource_class).to have_received(:defaultto)
+        end
+      end
+
+      context 'when property has Boolean type' do
+        let(:data_type) { Puppet::Pops::Types::PBooleanType.new() } # rubocop:disable Style/WordArray
+        let(:options) do
+          {
+            type: 'Boolean',
+            desc: 'Boolean test.',
+          }
+        end
+
+        it 'calls #newvalue twice' do
+          expect(resource_class).to have_received(:newvalue).twice
+        end
+
+        it 'calls #aliasvalue four times' do
+          expect(resource_class).to have_received(:aliasvalue).at_least(4).times
+        end
+      end
+
+      context 'when property has String type' do
+        let(:data_type) { Puppet::Pops::Types::PStringType.new('s') } # rubocop:disable Style/WordArray
+        let(:options) do
+          {
+            type: 'String',
+            desc: 'String test.',
+          }
+        end
+
+        it 'calls #newvalue twice' do
+          expect(resource_class).to have_received(:newvalue).once
+        end
+      end
+
+      context 'when property has Integer type' do
+        let(:data_type) { Puppet::Pops::Types::PIntegerType.new(1) } # rubocop:disable Style/WordArray
+        let(:options) do
+          {
+            type: 'Integer',
+            desc: 'Integer test.',
+          }
+        end
+
+        it 'calls #newvalue twice' do
+          expect(resource_class).to have_received(:newvalue).once
+        end
+      end
+
+      context 'when property has Float type' do
+        let(:data_type) { Puppet::Pops::Types::PFloatType.new(1.0) } # rubocop:disable Style/WordArray
+        let(:options) do
+          {
+            type: 'Float',
+            desc: 'Float test.',
+          }
+        end
+
+        it 'calls #newvalue twice' do
+          expect(resource_class).to have_received(:newvalue).once
+        end
+      end
+    end
+
+    context 'when resource class is parameter' do
+      let(:resource_class) { Puppet::ResourceApi::Parameter }
+      let(:data_type) { Puppet::Pops::Types::PBooleanType.new() } # rubocop:disable Style/WordArray
+      let(:param_or_property) { :newparam }
+
+      it 'resource_class has no #call_provider method' do
+        expect(resource_class.method_defined?(:call_provider)).to eq(false)
+      end
+
+      context 'when behaviour is set to :namevar' do
+        let(:options) do
+          {
+            type: 'String',
+            desc: 'Namevar test',
+            behaviour: :namevar,
+          }
+        end
+
+        it 'calls #isnamevar once' do
+          expect(resource_class).to have_received(:isnamevar).once
+        end
+      end
+
+      context 'when default value is set' do
+        let(:options) do
+          {
+            type: 'Boolean',
+            desc: 'Default value test',
+            default: true,
+          }
+        end
+
+        it 'calls #defaultto once' do
+          expect(resource_class).to have_received(:defaultto).once
+        end
+      end
+
+      context 'when there is no default value' do
+        let(:options) do
+          {
+            type: 'Boolean',
+            desc: 'No default value test',
+          }
+        end
+
+        it 'does not call #defaultto' do
+          expect(resource_class).not_to receive(:defaultto)
+        end
+      end
+
+      context 'when parameter has Boolean type' do
+        let(:data_type) { Puppet::Pops::Types::PBooleanType.new() } # rubocop:disable Style/WordArray
+        let(:options) do
+          {
+            type: 'Boolean',
+            desc: 'Boolean test.',
+          }
+        end
+
+        it 'calls #newvalues twice' do
+          expect(resource_class).to have_received(:newvalues).once
+        end
+
+        it 'calls #aliasvalue four times' do
+          expect(resource_class).to have_received(:aliasvalue).at_least(4).times
+        end
+      end
+
+      context 'when parameter has String type' do
+        let(:data_type) { Puppet::Pops::Types::PStringType.new('s') } # rubocop:disable Style/WordArray
+        let(:options) do
+          {
+            type: 'String',
+            desc: 'String test.',
+          }
+        end
+
+        it 'calls #newvalues twice' do
+          expect(resource_class).to have_received(:newvalues).once
+        end
+      end
+
+      context 'when parameter has Integer type' do
+        let(:data_type) { Puppet::Pops::Types::PIntegerType.new(1) } # rubocop:disable Style/WordArray
+        let(:options) do
+          {
+            type: 'Integer',
+            desc: 'Integer test.',
+          }
+        end
+
+        it 'calls #newvalues twice' do
+          expect(resource_class).to have_received(:newvalues).once
+        end
+      end
+
+      context 'when parameter has Float type' do
+        let(:data_type) { Puppet::Pops::Types::PFloatType.new(1.0) } # rubocop:disable Style/WordArray
+        let(:options) do
+          {
+            type: 'Float',
+            desc: 'Float test.',
+          }
+        end
+
+        it 'calls #newvalues twice' do
+          expect(resource_class).to have_received(:newvalues).once
+        end
+      end
+    end
+  end
+end

From ee55a77cfca33ec12c1edc9fbe0475ff4c58a170 Mon Sep 17 00:00:00 2001
From: Bernard Pietraga <bernard.pietraga@gmail.com>
Date: Sat, 24 Nov 2018 13:25:55 +0100
Subject: [PATCH 03/14] (maint) remove duplicated :default key check in
 ValueCreator

In original logic the @options.key?(:default) is duplicated.
And the upper confitional covers the needed code flow.
---
 lib/puppet/resource_api/value_creator.rb | 30 +++++++++++-------------
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/lib/puppet/resource_api/value_creator.rb b/lib/puppet/resource_api/value_creator.rb
index dadb1c55..9c3503ac 100644
--- a/lib/puppet/resource_api/value_creator.rb
+++ b/lib/puppet/resource_api/value_creator.rb
@@ -17,22 +17,20 @@ def create_values
 
     # read-only values do not need type checking, but can have default values
     if @options[:behaviour] != :read_only && @options.key?(:default)
-      if @options.key? :default
-        if @options[:default] == false
-          # work around https://tickets.puppetlabs.com/browse/PUP-2368
-          @resource_class.defaultto :false # rubocop:disable Lint/BooleanSymbol
-        elsif @options[:default] == true
-          # work around https://tickets.puppetlabs.com/browse/PUP-2368
-          @resource_class.defaultto :true # rubocop:disable Lint/BooleanSymbol
-        else
-          # marshal the default option to decouple that from the actual value.
-          # we cache the dumped value in `marshalled`, but use a block to
-          # unmarshal everytime the value is requested. Objects that can't be
-          # marshalled
-          # See https://stackoverflow.com/a/8206537/4918
-          marshalled = Marshal.dump(@options[:default])
-          @resource_class.defaultto { Marshal.load(marshalled) } # rubocop:disable Security/MarshalLoad
-        end
+      if @options[:default] == false
+        # work around https://tickets.puppetlabs.com/browse/PUP-2368
+        @resource_class.defaultto :false # rubocop:disable Lint/BooleanSymbol
+      elsif @options[:default] == true
+        # work around https://tickets.puppetlabs.com/browse/PUP-2368
+        @resource_class.defaultto :true # rubocop:disable Lint/BooleanSymbol
+      else
+        # marshal the default option to decouple that from the actual value.
+        # we cache the dumped value in `marshalled`, but use a block to
+        # unmarshal everytime the value is requested. Objects that can't be
+        # marshalled
+        # See https://stackoverflow.com/a/8206537/4918
+        marshalled = Marshal.dump(@options[:default])
+        @resource_class.defaultto { Marshal.load(marshalled) } # rubocop:disable Security/MarshalLoad
       end
     end
 

From ad47c17b332e6e7c8e854e2a8383110d51e0ff2b Mon Sep 17 00:00:00 2001
From: Bernard Pietraga <bernard.pietraga@gmail.com>
Date: Mon, 26 Nov 2018 08:54:45 +0100
Subject: [PATCH 04/14] (maint) add ValueCreator docs and small restructure in
 ResourceApi

---
 lib/puppet/resource_api.rb               | 31 ++++++++++++------------
 lib/puppet/resource_api/value_creator.rb | 12 +++++++--
 2 files changed, 26 insertions(+), 17 deletions(-)

diff --git a/lib/puppet/resource_api.rb b/lib/puppet/resource_api.rb
index 48f5049e..05a9dad2 100644
--- a/lib/puppet/resource_api.rb
+++ b/lib/puppet/resource_api.rb
@@ -201,6 +201,12 @@ def to_resource
             raise Puppet::DevError, "#{definition[:name]}.#{name} has no type"
           end
 
+          if options[:desc]
+            desc "#{options[:desc]} (a #{options[:type]})"
+          else
+            warn("#{definition[:name]}.#{name} has no docs")
+          end
+
           # The initialize takes passed hash with resource.
           # The method pass needed arguments, depending on parent to:
           # - Puppet::ResourceApi::Property
@@ -212,36 +218,31 @@ def initialize(resource_hash)
             super(name, data_type, data_definition, resource_hash[:resource])
           end
 
-          if options[:desc]
-            desc "#{options[:desc]} (a #{options[:type]})"
-          else
-            warn("#{definition[:name]}.#{name} has no docs")
-          end
-
+          # get type class object for the parameter or property
           type = Puppet::ResourceApi::DataTypeHandling.parse_puppet_type(
             name,
             options[:type],
           )
 
+          # provide hints to `puppet type generate` for better parsing
+          if type.instance_of? Puppet::Pops::Types::POptionalType
+            type = type.type
+          end
+
           # inside of parameter or property provide method to check and return
-          # data type.
+          # data type
           define_method(:data_type) do
             type
           end
 
           # inside of parameter or property provide method to check and return
-          # the definition hash object.
+          # the definition hash object
           define_method(:data_definition) do
             definition
           end
 
-          # provide hints to `puppet type generate` for better parsing
-          if type.instance_of? Puppet::Pops::Types::POptionalType
-            type = type.type
-          end
-
-          # Initialize ValueCreator and call create_values which creates alias
-          # values and default values for properties and params.
+          # initialize ValueCreator and call create_values which makes alias
+          # values and default values for properties and params
           Puppet::ResourceApi::ValueCreator.new(
             self,
             type,
diff --git a/lib/puppet/resource_api/value_creator.rb b/lib/puppet/resource_api/value_creator.rb
index 9c3503ac..ca7fb21b 100644
--- a/lib/puppet/resource_api/value_creator.rb
+++ b/lib/puppet/resource_api/value_creator.rb
@@ -3,6 +3,14 @@ module Puppet; module ResourceApi; end; end # predeclare the main module # ruboc
 # This class is responsible for setting default and alias values for the
 # resource class.
 class Puppet::ResourceApi::ValueCreator
+  # This initialize takes arguments and sets up new value creator for given
+  # resource class which can be Puppet::ResourceApi::Parameter,
+  # Puppet::ResourceApi::ReadOnlyParameter, Puppet::ResourceApi::Property.
+  # @param resource_class the class of selected resource to be extended
+  # @param data_type the type instance
+  # @param definition the definition of the property
+  # @param options the ResourceAPI options hash containing setup information for
+  # selected parameter or property
   def initialize(resource_class, data_type, param_or_property, options = {})
     @resource_class = resource_class
     @data_type = data_type
@@ -58,8 +66,8 @@ def create_values
     end
   end
 
-  # Add the value to `this` property or param, depending on whether
-  # param_or_property is `:newparam`, or `:newproperty`.
+  # add the value to `this` property or param, depending on whether
+  # param_or_property is `:newparam`, or `:newproperty`
   def def_newvalues(this, *values)
     if @param_or_property == :newparam
       this.newvalues(*values)

From b7c636d08cc548ea06a0c6dddae7d058dd2a357c Mon Sep 17 00:00:00 2001
From: Bernard Pietraga <bernard.pietraga@gmail.com>
Date: Mon, 26 Nov 2018 09:46:57 +0100
Subject: [PATCH 05/14] (maint) change parameter and properties class structure

Add Puppet::Util to paramater and property for support to older
Puppet versions.
Change the ValueCreator class to module.
Provide test changes.
Fix typos.
---
 lib/puppet/resource_api.rb                    | 88 ++++++++-----------
 lib/puppet/resource_api/parameter.rb          | 32 ++++---
 lib/puppet/resource_api/property.rb           | 32 +++----
 .../resource_api/read_only_parameter.rb       | 34 ++-----
 lib/puppet/resource_api/value_creator.rb      | 71 +++++++--------
 spec/puppet/resource_api/parameter_spec.rb    | 30 ++-----
 spec/puppet/resource_api/property_spec.rb     | 36 +++-----
 .../resource_api/read_only_parameter_spec.rb  | 33 +++----
 .../puppet/resource_api/value_creator_spec.rb | 28 +++---
 9 files changed, 148 insertions(+), 236 deletions(-)

diff --git a/lib/puppet/resource_api.rb b/lib/puppet/resource_api.rb
index 05a9dad2..b0dd0ab9 100644
--- a/lib/puppet/resource_api.rb
+++ b/lib/puppet/resource_api.rb
@@ -10,6 +10,8 @@
 require 'puppet/resource_api/version'
 require 'puppet/type'
 require 'puppet/util/network_device'
+require 'puppet/property'
+require 'puppet/parameter'
 
 module Puppet::ResourceApi
   @warning_count = 0
@@ -178,24 +180,22 @@ def to_resource
         # TODO: using newparam everywhere would suppress change reporting
         #       that would allow more fine-grained reporting through context,
         #       but require more invest in hooking up the infrastructure to emulate existing data
-        param_or_property = if [:read_only, :parameter, :namevar].include? options[:behaviour]
-                              :newparam
-                            else
-                              :newproperty
-                            end
+        if [:parameter, :namevar].include? options[:behaviour]
+          param_or_property = :newparam
+          parent = Puppet::ResourceApi::Parameter
+        elsif options[:behaviour] == :read_only
+          param_or_property = :newparam
+          parent = Puppet::ResourceApi::ReadOnlyParameter
+        else
+          param_or_property = :newproperty
+          parent = Puppet::ResourceApi::Property
+        end
 
-        parent = if param_or_property == :newproperty
-                   Puppet::ResourceApi::Property
-                 elsif options[:behaviour] == :read_only
-                   Puppet::ResourceApi::ReadOnlyParameter
-                 else
-                   Puppet::ResourceApi::Parameter
-                 end
-
-        # This part fo code creates new parameter or property. It takes the
-        # param_and_property which is one of methods and then translates it to
-        # the i.ex.
-        # newparam(:param_name, parent: Puppet::ResourceApi::Parameter) do; end;
+        # This call creates a new parameter or property with all work-arounds or
+        # customizations required by the Resource API applied. Under the hood,
+        # this maps to the relevant DSL methods in Puppet::Type. See
+        # https://puppet.com/docs/puppet/6.0/custom_types.html#reference-5883
+        # for details.
         send(param_or_property, name.to_sym, parent: parent) do
           unless options[:type]
             raise Puppet::DevError, "#{definition[:name]}.#{name} has no type"
@@ -207,48 +207,32 @@ def to_resource
             warn("#{definition[:name]}.#{name} has no docs")
           end
 
-          # The initialize takes passed hash with resource.
-          # The method pass needed arguments, depending on parent to:
-          # - Puppet::ResourceApi::Property
-          # - Puppet::ResourceApi::Parameter
-          # - Puppet::ResourceApi::ReadOnlyParameter
-          # @param resource_hash [{ resource: #<Puppet::Type::TypeName> }] is
-          # one element hash referencing the resource object.
-          def initialize(resource_hash)
-            super(name, data_type, data_definition, resource_hash[:resource])
+          # The initialize method is called when puppet core starts building up
+          # type objects. The core passes in a hash of shape { resource:
+          # #<Puppet::Type::TypeName> }. We use this to pass through the
+          # required configuration data to the parent (see
+          # Puppet::ResourceApi::Property, Puppet::ResourceApi::Parameter and
+          # Puppet::ResourceApi::ReadOnlyParameter).
+          define_method(:initialize) do |resource_hash|
+            super(definition[:name], self.class.data_type, name, resource_hash)
           end
 
-          # get type class object for the parameter or property
-          type = Puppet::ResourceApi::DataTypeHandling.parse_puppet_type(
-            name,
-            options[:type],
-          )
-
-          # provide hints to `puppet type generate` for better parsing
-          if type.instance_of? Puppet::Pops::Types::POptionalType
-            type = type.type
-          end
-
-          # inside of parameter or property provide method to check and return
-          # data type
-          define_method(:data_type) do
-            type
+          # get pops data type object for this parameter or property
+          define_singleton_method(:data_type) do
+            @rsapi_data_type ||= Puppet::ResourceApi::DataTypeHandling.parse_puppet_type(
+              name,
+              options[:type],
+            )
           end
 
-          # inside of parameter or property provide method to check and return
-          # the definition hash object
-          define_method(:data_definition) do
-            definition
-          end
-
-          # initialize ValueCreator and call create_values which makes alias
-          # values and default values for properties and params
-          Puppet::ResourceApi::ValueCreator.new(
+          # from ValueCreator call create_values which makes alias values and
+          # default values for properties and params
+          Puppet::ResourceApi::ValueCreator.create_values(
             self,
-            type,
+            data_type,
             param_or_property,
             options,
-          ).create_values
+          )
         end
       end
 
diff --git a/lib/puppet/resource_api/parameter.rb b/lib/puppet/resource_api/parameter.rb
index e1f5c315..ef749405 100644
--- a/lib/puppet/resource_api/parameter.rb
+++ b/lib/puppet/resource_api/parameter.rb
@@ -1,25 +1,23 @@
+require 'puppet/util'
 require 'puppet/parameter'
 
 module Puppet; module ResourceApi; end; end # predeclare the main module # rubocop:disable Style/Documentation,Style/ClassAndModuleChildren
 
 # Class containing parameter functionality for ResourceApi.
 class Puppet::ResourceApi::Parameter < Puppet::Parameter
-  # This initialize takes arguments and sets up new parameter.
-  # @param name the name used for reporting errors
-  # @param type the type instance
-  # @param definition the definition of the property
-  # @param resource the resource instance which is passed to the parent class.
-  def initialize(name, type, definiton, resource)
-    @name = name
-    @type = type
-    @definiton = definiton
-    super(resource: resource) # Pass resource to parent Puppet class.
-  end
+  attr_reader :value
 
-  # This method handles return of assigned value from parameter.
-  # @return [type] the type value
-  def value
-    @value
+  # This initialize takes arguments and sets up new parameter.
+  # @param type_name the name of the Puppet Type
+  # @param data_type the data type of parameter instance
+  # @param attribute_name the name of attribue of the parameter
+  # @param resource_hash the resource hash instance which is passed to the
+  # parent class.
+  def initialize(type_name, data_type, attribute_name, resource_hash)
+    @type_name = type_name
+    @data_type = data_type
+    @attribute_name = attribute_name
+    super(resource_hash) # Pass resource to parent Puppet class.
   end
 
   # This method assigns value to the parameter and cleans value.
@@ -27,9 +25,9 @@ def value
   # @return [type] the cleaned value
   def value=(value)
     @value = Puppet::ResourceApi::DataTypeHandling.mungify(
-      @type,
+      @data_type,
       value,
-      "#{@definiton[:name]}.#{name}",
+      "#{@type_name}.#{@attribute_name}",
       Puppet::ResourceApi.caller_is_resource_app?,
     )
   end
diff --git a/lib/puppet/resource_api/property.rb b/lib/puppet/resource_api/property.rb
index cef9c940..59f0b4eb 100644
--- a/lib/puppet/resource_api/property.rb
+++ b/lib/puppet/resource_api/property.rb
@@ -1,3 +1,4 @@
+require 'puppet/util'
 require 'puppet/property'
 
 module Puppet; module ResourceApi; end; end # predeclare the main module # rubocop:disable Style/Documentation,Style/ClassAndModuleChildren
@@ -5,24 +6,25 @@ module Puppet; module ResourceApi; end; end # predeclare the main module # ruboc
 # Class containing property functionality for ResourceApi.
 class Puppet::ResourceApi::Property < Puppet::Property
   # This initialize takes arguments and sets up new property.
-  # @param name the name used for reporting errors
-  # @param type the type instance
-  # @param definition the definition of the property
-  # @param resource the resource instance which is passed to the parent class.
-  def initialize(name, type, definition, resource)
-    @name = name
-    @type = type
-    @definition = definition
+  # @param type_name the name of the Puppet Type
+  # @param data_type the data type of property instance
+  # @param attribute_name the name of attribue of the property
+  # @param resource_hash the resource hash instance which is passed to the
+  # parent class.
+  def initialize(type_name, data_type, attribute_name, resource_hash)
+    @type_name = type_name
+    @data_type = data_type
+    @attribute_name = attribute_name
     # Define class method insync?(is) if the name is :ensure
-    def_insync? if @name == :ensure
-    # Pass resource and should to parent Puppet class.
-    super(resource: resource, should: should)
+    def_insync? if @attribute_name == :ensure
+    # Pass resource to parent Puppet class.
+    super(resource_hash)
   end
 
   # This method returns value of the property.
   # @return [type] the property value
   def should
-    if @name == :ensure && rs_value.is_a?(String)
+    if @attribute_name == :ensure && rs_value.is_a?(String)
       rs_value.to_sym
     elsif rs_value == false
       # work around https://tickets.puppetlabs.com/browse/PUP-2368
@@ -41,7 +43,7 @@ def should
   def should=(value)
     @shouldorig = value
 
-    if @name == :ensure
+    if @attribute_name == :ensure
       value = value.to_s
     end
 
@@ -50,9 +52,9 @@ def should=(value)
     # @see Puppet::Property.should=(value)
     @should = [
       Puppet::ResourceApi::DataTypeHandling.mungify(
-        @type,
+        @data_type,
         value,
-        "#{@definition[:name]}.#{@name}",
+        "#{@type_name}.#{@attribute_name}",
         Puppet::ResourceApi.caller_is_resource_app?,
       ),
     ]
diff --git a/lib/puppet/resource_api/read_only_parameter.rb b/lib/puppet/resource_api/read_only_parameter.rb
index 5091f7ec..97458e48 100644
--- a/lib/puppet/resource_api/read_only_parameter.rb
+++ b/lib/puppet/resource_api/read_only_parameter.rb
@@ -1,32 +1,14 @@
-require 'puppet/parameter'
-
-module Puppet; module ResourceApi; end; end # predeclare the main module # rubocop:disable Style/Documentation,Style/ClassAndModuleChildren
+require 'puppet/util'
+require 'puppet/resource_api/parameter'
 
 # Class containing read only parameter functionality for ResourceApi.
-class Puppet::ResourceApi::ReadOnlyParameter < Puppet::Property
-  # This initialize takes arguments and sets up new parameter.
-  # @param name the name used for reporting errors
-  # @param type the type instance
-  # @param definition the definition of the property
-  # @param resource the resource instance which is passed to the parent class.
-  def initialize(name, type, definition, resource)
-    @name = name
-    @type = type
-    @definition = definition
-    super(resource: resource) # Pass resource to parent Puppet class.
-  end
-
-  # This method handles return of assigned value from parameter.
-  # @return [type] the type value
-  def value # rubocop:disable Style/TrivialAccessors
-    @value
-  end
-
+class Puppet::ResourceApi::ReadOnlyParameter < Puppet::ResourceApi::Parameter
   # This method raises error if the there is attempt to set value in parameter.
   # @return [Puppet::ResourceError] the error with information.
   def value=(value)
     raise Puppet::ResourceError,
-          "Attempting to set `#{@name}` read_only attribute value to `#{value}`"
+          "Attempting to set `#{@attribute_name}` read_only attribute value " \
+          "to `#{value}`"
   end
 
   # used internally
@@ -34,10 +16,4 @@ def value=(value)
   def rs_value
     @value
   end
-
-  # puppet symbolizes some values through puppet/parameter/value.rb
-  # (see .convert()), but (especially) Enums are strings. specifying a
-  # munge block here skips the value_collection fallback in
-  # puppet/parameter.rb's default .unsafe_munge() implementation.
-  munge { |v| v }
 end
diff --git a/lib/puppet/resource_api/value_creator.rb b/lib/puppet/resource_api/value_creator.rb
index ca7fb21b..6b2a9c6d 100644
--- a/lib/puppet/resource_api/value_creator.rb
+++ b/lib/puppet/resource_api/value_creator.rb
@@ -1,9 +1,9 @@
 module Puppet; module ResourceApi; end; end # predeclare the main module # rubocop:disable Style/Documentation,Style/ClassAndModuleChildren
 
-# This class is responsible for setting default and alias values for the
+# This module is responsible for setting default and alias values for the
 # resource class.
-class Puppet::ResourceApi::ValueCreator
-  # This initialize takes arguments and sets up new value creator for given
+module Puppet::ResourceApi::ValueCreator
+  # This method is responsible for all setup of the value mapping for desired
   # resource class which can be Puppet::ResourceApi::Parameter,
   # Puppet::ResourceApi::ReadOnlyParameter, Puppet::ResourceApi::Property.
   # @param resource_class the class of selected resource to be extended
@@ -11,65 +11,62 @@ class Puppet::ResourceApi::ValueCreator
   # @param definition the definition of the property
   # @param options the ResourceAPI options hash containing setup information for
   # selected parameter or property
-  def initialize(resource_class, data_type, param_or_property, options = {})
-    @resource_class = resource_class
-    @data_type = data_type
-    @param_or_property = param_or_property
-    @options = options
-  end
-
-  # This method is responsible for all setup of the value mapping for desired
-  # resource class.
-  def create_values
-    @resource_class.isnamevar if @options[:behaviour] == :namevar
+  def self.create_values(resource_class, data_type, param_or_property, options = {})
+    resource_class.isnamevar if options[:behaviour] == :namevar
 
     # read-only values do not need type checking, but can have default values
-    if @options[:behaviour] != :read_only && @options.key?(:default)
-      if @options[:default] == false
+    if options[:behaviour] != :read_only && options.key?(:default)
+      if options[:default] == false
         # work around https://tickets.puppetlabs.com/browse/PUP-2368
-        @resource_class.defaultto :false # rubocop:disable Lint/BooleanSymbol
-      elsif @options[:default] == true
+        resource_class.defaultto :false # rubocop:disable Lint/BooleanSymbol
+      elsif options[:default] == true
         # work around https://tickets.puppetlabs.com/browse/PUP-2368
-        @resource_class.defaultto :true # rubocop:disable Lint/BooleanSymbol
+        resource_class.defaultto :true # rubocop:disable Lint/BooleanSymbol
       else
         # marshal the default option to decouple that from the actual value.
         # we cache the dumped value in `marshalled`, but use a block to
         # unmarshal everytime the value is requested. Objects that can't be
         # marshalled
         # See https://stackoverflow.com/a/8206537/4918
-        marshalled = Marshal.dump(@options[:default])
-        @resource_class.defaultto { Marshal.load(marshalled) } # rubocop:disable Security/MarshalLoad
+        marshalled = Marshal.dump(options[:default])
+        resource_class.defaultto { Marshal.load(marshalled) } # rubocop:disable Security/MarshalLoad
       end
     end
 
-    case @data_type
+    # provide hints to `puppet type generate` for better parsing
+    if data_type.instance_of? Puppet::Pops::Types::POptionalType
+      data_type = data_type.type
+    end
+
+    case data_type
     when Puppet::Pops::Types::PStringType
       # require any string value
-      def_newvalues(@resource_class, %r{})
+      def_newvalues(resource_class, param_or_property, %r{})
     when Puppet::Pops::Types::PBooleanType
-      def_newvalues(@resource_class, 'true', 'false')
-      @resource_class.aliasvalue true, 'true'
-      @resource_class.aliasvalue false, 'false'
-      @resource_class.aliasvalue :true, 'true' # rubocop:disable Lint/BooleanSymbol
-      @resource_class.aliasvalue :false, 'false' # rubocop:disable Lint/BooleanSymbol
+      def_newvalues(resource_class, param_or_property, 'true', 'false')
+      resource_class.aliasvalue true, 'true'
+      resource_class.aliasvalue false, 'false'
+      resource_class.aliasvalue :true, 'true' # rubocop:disable Lint/BooleanSymbol
+      resource_class.aliasvalue :false, 'false' # rubocop:disable Lint/BooleanSymbol
     when Puppet::Pops::Types::PIntegerType
-      def_newvalues(@resource_class, %r{^-?\d+$})
+      def_newvalues(resource_class, param_or_property, %r{^-?\d+$})
     when Puppet::Pops::Types::PFloatType, Puppet::Pops::Types::PNumericType
-      def_newvalues(@resource_class, Puppet::Pops::Patterns::NUMERIC)
+      def_newvalues(resource_class, param_or_property, Puppet::Pops::Patterns::NUMERIC)
     end
 
-    def_call_provider() if @param_or_property == :newproperty
+    def_call_provider(resource_class) if param_or_property == :newproperty
 
-    case @options[:type]
+    case options[:type]
     when 'Enum[present, absent]'
-      def_newvalues(@resource_class, 'absent', 'present')
+      def_newvalues(resource_class, param_or_property, 'absent', 'present')
     end
+    resource_class
   end
 
   # add the value to `this` property or param, depending on whether
   # param_or_property is `:newparam`, or `:newproperty`
-  def def_newvalues(this, *values)
-    if @param_or_property == :newparam
+  def self.def_newvalues(this, param_or_property, *values)
+    if param_or_property == :newparam
       this.newvalues(*values)
     else
       values.each do |v|
@@ -81,7 +78,7 @@ def def_newvalues(this, *values)
   # stop puppet from trying to call into the provider when
   # no pre-defined values have been specified
   # "This is not the provider you are looking for." -- Obi-Wan Kaniesobi.
-  def def_call_provider
-    @resource_class.send(:define_method, :call_provider) { |value| }
+  def self.def_call_provider(resource_class)
+    resource_class.send(:define_method, :call_provider) { |value| }
   end
 end
diff --git a/spec/puppet/resource_api/parameter_spec.rb b/spec/puppet/resource_api/parameter_spec.rb
index a86e7672..8eab61e1 100644
--- a/spec/puppet/resource_api/parameter_spec.rb
+++ b/spec/puppet/resource_api/parameter_spec.rb
@@ -2,40 +2,24 @@
 
 RSpec.describe Puppet::ResourceApi::Parameter do
   subject(:parameter) do
-    described_class.new(name, type, definition, resource)
+    described_class.new(type_name, data_type, attribute_name, resource_hash)
   end
 
-  let(:definition) { {} }
-  let(:log_sink) { [] }
-  let(:name) { 'some_parameter' }
-  let(:resource) { {} }
+  let(:type_name) { 'test_name' }
+  let(:attribute_name) { 'some_parameter' }
+  let(:data_type) { Puppet::Pops::Types::PStringType.new(nil) }
+  let(:resource_hash) { { resource: {} } }
   let(:result) { 'value' }
-  let(:strict_level) { :error }
-  let(:type) { Puppet::Pops::Types::PStringType.new(nil) }
   let(:value) { 'value' }
 
-  before(:each) do
-    # set default to strictest setting
-    # by default Puppet runs at warning level
-    Puppet.settings[:strict] = strict_level
-    # Enable debug logging
-    Puppet.debug = true
-
-    Puppet::Util::Log.newdestination(Puppet::Test::LogCollector.new(log_sink))
-  end
-
-  after(:each) do
-    Puppet::Util::Log.close_all
-  end
-
   it { expect { described_class.new(nil) }.to raise_error ArgumentError, %r{wrong number of arguments} }
-  it { expect { described_class.new(name, type, definition, resource) }.not_to raise_error }
+  it { expect { described_class.new(type_name, data_type, attribute_name, resource_hash) }.not_to raise_error }
 
   describe '#value=(value)' do
     context 'when value is string' do
       context 'when the value is set with string value' do
         it('value is returned') do
-          expect(parameter.value=(value)).to eq result
+          expect(parameter.value=(value)).to eq result # rubocop:disable Style/RedundantParentheses, Layout/SpaceAroundOperators
         end
 
         it('value=(value) is called') do
diff --git a/spec/puppet/resource_api/property_spec.rb b/spec/puppet/resource_api/property_spec.rb
index 5c0b1fb5..e3cfb654 100644
--- a/spec/puppet/resource_api/property_spec.rb
+++ b/spec/puppet/resource_api/property_spec.rb
@@ -2,33 +2,17 @@
 
 RSpec.describe Puppet::ResourceApi::Property do
   subject(:property) do
-    described_class.new(name, type, definition, resource)
+    described_class.new(type_name, data_type, attribute_name, resource_hash)
   end
 
-  let(:definition) { {} }
-  let(:log_sink) { [] }
-  let(:name) { 'some_property' }
-  let(:resource) { {} }
+  let(:type_name) { 'test_name' }
+  let(:attribute_name) { 'some_property' }
+  let(:data_type) { Puppet::Pops::Types::PStringType.new(nil) }
+  let(:resource_hash) { { resource: {} } }
   let(:result) { 'value' }
-  let(:strict_level) { :error }
-  let(:type) { Puppet::Pops::Types::PStringType.new(nil) }
-
-  before(:each) do
-    # set default to strictest setting
-    # by default Puppet runs at warning level
-    Puppet.settings[:strict] = strict_level
-    # Enable debug logging
-    Puppet.debug = true
-
-    Puppet::Util::Log.newdestination(Puppet::Test::LogCollector.new(log_sink))
-  end
-
-  after(:each) do
-    Puppet::Util::Log.close_all
-  end
 
   it { expect { described_class.new(nil) }.to raise_error ArgumentError, %r{wrong number of arguments} }
-  it { expect { described_class.new(name, type, definition, resource) }.not_to raise_error }
+  it { expect { described_class.new(type_name, data_type, attribute_name, resource_hash) }.not_to raise_error }
 
   describe '#should=(value)' do
     context 'when value is string' do
@@ -43,7 +27,7 @@
         end
 
         it('value is returned') do
-          expect(property.should=(value)).to eq result
+          expect(property.should=(value)).to eq result # rubocop:disable Style/RedundantParentheses, Layout/SpaceAroundOperators
         end
       end
     end
@@ -53,9 +37,9 @@
     context 'when value is boolean' do
       context 'when the @should is set' do
         let(:should_true_value) { [true] } # rs_value takes value in array
-        let(:true_result) { :true }
+        let(:true_result) { :true } # rubocop:disable Lint/BooleanSymbol
         let(:should_false_value) { [false] } # rs_value takes value in array
-        let(:false_result) { :false }
+        let(:false_result) { :false } # rubocop:disable Lint/BooleanSymbol
 
         it('true symbol value is returned') do
           property.instance_variable_set(:@should, should_true_value)
@@ -74,7 +58,7 @@
         context 'when the @should is set' do
           let(:should_value) { ['present'] } # rs_value takes value in array
           let(:name) { :ensure }
-          let(:result) { :present }
+          let(:result) { 'present' }
 
           it('symbol value is returned') do
             property.instance_variable_set(:@should, should_value)
diff --git a/spec/puppet/resource_api/read_only_parameter_spec.rb b/spec/puppet/resource_api/read_only_parameter_spec.rb
index c166ad90..610177b9 100644
--- a/spec/puppet/resource_api/read_only_parameter_spec.rb
+++ b/spec/puppet/resource_api/read_only_parameter_spec.rb
@@ -2,40 +2,27 @@
 
 RSpec.describe Puppet::ResourceApi::ReadOnlyParameter do
   subject(:read_only_parameter) do
-    described_class.new(name, type, definition, resource)
+    described_class.new(type_name, data_type, attribute_name, resource_hash)
   end
 
-  let(:definition) { {} }
-  let(:log_sink) { [] }
-  let(:name) { 'some_parameter' }
-  let(:resource) { {} }
+  let(:type_name) { 'test_name' }
+  let(:attribute_name) { 'some_parameter' }
+  let(:data_type) { Puppet::Pops::Types::PStringType }
+  let(:resource_hash) { { resource: {} } }
   let(:result) { 'value' }
-  let(:strict_level) { :error }
-  let(:type) { Puppet::Pops::Types::PStringType }
   let(:value) { 'value' }
 
-  before(:each) do
-    # set default to strictest setting
-    # by default Puppet runs at warning level
-    Puppet.settings[:strict] = strict_level
-    # Enable debug logging
-    Puppet.debug = true
-
-    Puppet::Util::Log.newdestination(Puppet::Test::LogCollector.new(log_sink))
-  end
-
-  after(:each) do
-    Puppet::Util::Log.close_all
-  end
-
   it { expect { described_class.new(nil) }.to raise_error ArgumentError, %r{wrong number of arguments} }
-  it { expect { described_class.new(name, type, definition, resource) }.not_to raise_error }
+  it { expect { described_class.new(type_name, data_type, attribute_name, resource_hash) }.not_to raise_error }
 
   describe '#value=(value)' do
     context 'when called from `puppet resource`' do
       context 'when the value set attempt is performed' do
         it 'value set fails' do
-          expect { read_only_parameter.value=(value) }.to raise_error Puppet::ResourceError, %r{Attempting to set `some_parameter` read_only attribute value to `value`} # rubocop:disable Style/RedundantParentheses, Layout/SpaceAroundOperators
+          expect { read_only_parameter.value=(value) }.to raise_error( # rubocop:disable Style/RedundantParentheses, Layout/SpaceAroundOperators
+            Puppet::ResourceError,
+            %r{Attempting to set `some_parameter` read_only attribute value to `value`},
+          )
         end
       end
     end
diff --git a/spec/puppet/resource_api/value_creator_spec.rb b/spec/puppet/resource_api/value_creator_spec.rb
index b486eae7..8dc1f384 100644
--- a/spec/puppet/resource_api/value_creator_spec.rb
+++ b/spec/puppet/resource_api/value_creator_spec.rb
@@ -1,8 +1,8 @@
 require 'spec_helper'
 
 RSpec.describe Puppet::ResourceApi::ValueCreator do
-  subject(:instance) do
-    described_class.new(resource_class, data_type, param_or_property, options)
+  subject(:value_creator) do
+    described_class
   end
 
   let(:resource_class) { Puppet::ResourceApi::Property }
@@ -24,12 +24,12 @@
     allow(resource_class).to receive(:isnamevar)
   end
 
-  it { expect { described_class.new(nil) }.to raise_error ArgumentError, %r{wrong number of arguments} }
-  it { expect { instance }.not_to raise_error }
+  it { expect { described_class.create_values(nil) }.to raise_error ArgumentError, %r{wrong number of arguments} }
+  it { expect { value_creator }.not_to raise_error }
 
   describe '#create_values' do
     before(:each) do
-      instance.create_values
+      value_creator.create_values(resource_class, data_type, param_or_property, options)
     end
 
     context 'when resource class is property' do
@@ -50,7 +50,7 @@
       end
 
       context 'when property has Boolean type' do
-        let(:data_type) { Puppet::Pops::Types::PBooleanType.new() } # rubocop:disable Style/WordArray
+        let(:data_type) { Puppet::Pops::Types::PBooleanType.new }
         let(:options) do
           {
             type: 'Boolean',
@@ -68,7 +68,7 @@
       end
 
       context 'when property has String type' do
-        let(:data_type) { Puppet::Pops::Types::PStringType.new('s') } # rubocop:disable Style/WordArray
+        let(:data_type) { Puppet::Pops::Types::PStringType.new('s') }
         let(:options) do
           {
             type: 'String',
@@ -82,7 +82,7 @@
       end
 
       context 'when property has Integer type' do
-        let(:data_type) { Puppet::Pops::Types::PIntegerType.new(1) } # rubocop:disable Style/WordArray
+        let(:data_type) { Puppet::Pops::Types::PIntegerType.new(1) }
         let(:options) do
           {
             type: 'Integer',
@@ -96,7 +96,7 @@
       end
 
       context 'when property has Float type' do
-        let(:data_type) { Puppet::Pops::Types::PFloatType.new(1.0) } # rubocop:disable Style/WordArray
+        let(:data_type) { Puppet::Pops::Types::PFloatType.new(1.0) }
         let(:options) do
           {
             type: 'Float',
@@ -112,7 +112,7 @@
 
     context 'when resource class is parameter' do
       let(:resource_class) { Puppet::ResourceApi::Parameter }
-      let(:data_type) { Puppet::Pops::Types::PBooleanType.new() } # rubocop:disable Style/WordArray
+      let(:data_type) { Puppet::Pops::Types::PBooleanType.new }
       let(:param_or_property) { :newparam }
 
       it 'resource_class has no #call_provider method' do
@@ -161,7 +161,7 @@
       end
 
       context 'when parameter has Boolean type' do
-        let(:data_type) { Puppet::Pops::Types::PBooleanType.new() } # rubocop:disable Style/WordArray
+        let(:data_type) { Puppet::Pops::Types::PBooleanType.new }
         let(:options) do
           {
             type: 'Boolean',
@@ -179,7 +179,7 @@
       end
 
       context 'when parameter has String type' do
-        let(:data_type) { Puppet::Pops::Types::PStringType.new('s') } # rubocop:disable Style/WordArray
+        let(:data_type) { Puppet::Pops::Types::PStringType.new('s') }
         let(:options) do
           {
             type: 'String',
@@ -193,7 +193,7 @@
       end
 
       context 'when parameter has Integer type' do
-        let(:data_type) { Puppet::Pops::Types::PIntegerType.new(1) } # rubocop:disable Style/WordArray
+        let(:data_type) { Puppet::Pops::Types::PIntegerType.new(1) }
         let(:options) do
           {
             type: 'Integer',
@@ -207,7 +207,7 @@
       end
 
       context 'when parameter has Float type' do
-        let(:data_type) { Puppet::Pops::Types::PFloatType.new(1.0) } # rubocop:disable Style/WordArray
+        let(:data_type) { Puppet::Pops::Types::PFloatType.new(1.0) }
         let(:options) do
           {
             type: 'Float',

From 8ace289c5fe7d066dd943fd5d7aa540004d57468 Mon Sep 17 00:00:00 2001
From: Bernard Pietraga <bernard.pietraga@gmail.com>
Date: Thu, 29 Nov 2018 08:31:58 +0100
Subject: [PATCH 06/14] (maint) move the call_provider method to property

---
 lib/puppet/resource_api/property.rb      | 5 +++++
 lib/puppet/resource_api/value_creator.rb | 9 ---------
 2 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/lib/puppet/resource_api/property.rb b/lib/puppet/resource_api/property.rb
index 59f0b4eb..7860f9b6 100644
--- a/lib/puppet/resource_api/property.rb
+++ b/lib/puppet/resource_api/property.rb
@@ -77,4 +77,9 @@ def def_insync?
   # munge block here skips the value_collection fallback in
   # puppet/parameter.rb's default .unsafe_munge() implementation.
   munge { |v| v }
+
+  # stop puppet from trying to call into the provider when
+  # no pre-defined values have been specified
+  # "This is not the provider you are looking for." -- Obi-Wan Kaniesobi.
+  def call_provider(_value); end
 end
diff --git a/lib/puppet/resource_api/value_creator.rb b/lib/puppet/resource_api/value_creator.rb
index 6b2a9c6d..93e91ea8 100644
--- a/lib/puppet/resource_api/value_creator.rb
+++ b/lib/puppet/resource_api/value_creator.rb
@@ -54,8 +54,6 @@ def self.create_values(resource_class, data_type, param_or_property, options = {
       def_newvalues(resource_class, param_or_property, Puppet::Pops::Patterns::NUMERIC)
     end
 
-    def_call_provider(resource_class) if param_or_property == :newproperty
-
     case options[:type]
     when 'Enum[present, absent]'
       def_newvalues(resource_class, param_or_property, 'absent', 'present')
@@ -74,11 +72,4 @@ def self.def_newvalues(this, param_or_property, *values)
       end
     end
   end
-
-  # stop puppet from trying to call into the provider when
-  # no pre-defined values have been specified
-  # "This is not the provider you are looking for." -- Obi-Wan Kaniesobi.
-  def self.def_call_provider(resource_class)
-    resource_class.send(:define_method, :call_provider) { |value| }
-  end
 end

From a0d1cbbc0bab797f164b23a457ba67767da3b670 Mon Sep 17 00:00:00 2001
From: David Schmitt <david@black.co.at>
Date: Thu, 29 Nov 2018 12:05:45 +0000
Subject: [PATCH 07/14] (maint) update parameter tests

---
 spec/puppet/resource_api/parameter_spec.rb | 73 +++++++++-------------
 1 file changed, 31 insertions(+), 42 deletions(-)

diff --git a/spec/puppet/resource_api/parameter_spec.rb b/spec/puppet/resource_api/parameter_spec.rb
index 8eab61e1..f5fac01f 100644
--- a/spec/puppet/resource_api/parameter_spec.rb
+++ b/spec/puppet/resource_api/parameter_spec.rb
@@ -9,70 +9,59 @@
   let(:attribute_name) { 'some_parameter' }
   let(:data_type) { Puppet::Pops::Types::PStringType.new(nil) }
   let(:resource_hash) { { resource: {} } }
-  let(:result) { 'value' }
-  let(:value) { 'value' }
 
-  it { expect { described_class.new(nil) }.to raise_error ArgumentError, %r{wrong number of arguments} }
-  it { expect { described_class.new(type_name, data_type, attribute_name, resource_hash) }.not_to raise_error }
+  describe '#new(type_name, data_type, attribute_name, resource_hash)' do
+    it { expect { described_class.new(nil) }.to raise_error ArgumentError, %r{wrong number of arguments} }
+    it { expect { described_class.new(type_name, data_type, attribute_name, resource_hash) }.not_to raise_error }
+  end
 
   describe '#value=(value)' do
-    context 'when value is string' do
-      context 'when the value is set with string value' do
-        it('value is returned') do
-          expect(parameter.value=(value)).to eq result # rubocop:disable Style/RedundantParentheses, Layout/SpaceAroundOperators
-        end
+    it 'calls mungify and stores the munged value' do
+      expect(Puppet::ResourceApi::DataTypeHandling).to receive(:mungify)
+        .with(data_type, 'value', 'test_name.some_parameter', false)
+        .and_return('munged value')
 
-        it('value=(value) is called') do
-          allow(described_class).to receive(:value=).with(value).and_return(result)
-          described_class.value=(value) # rubocop:disable Style/RedundantParentheses, Layout/SpaceAroundOperators
-          expect(described_class).to have_received(:value=).once
-        end
-      end
+      parameter.value = 'value'
+
+      expect(parameter.value).to eq 'munged value'
+    end
+
+    it 'calls mungify and reports its error' do
+      expect(Puppet::ResourceApi::DataTypeHandling).to receive(:mungify)
+        .and_raise Exception, 'error'
+
+      expect { parameter.value = 'value' }.to raise_error Exception, 'error'
+
+      expect(parameter.value).to eq nil
     end
   end
 
   describe '#value' do
     context 'when the value is not set' do
-      it('nil is returned') do
+      it 'nil is returned' do
         expect(parameter.value).to eq nil
       end
     end
 
-    context 'when value is string' do
-      context 'when the value is set' do
-        it('value is called') do
-          allow(described_class).to receive(:value).and_return(result)
-          described_class.value
-          expect(described_class).to have_received(:value).once
-        end
-
-        it('value is returned') do
-          parameter.instance_variable_set(:@value, value)
-          expect(parameter.value).to eq result
-        end
+    context 'when the value is set' do
+      it 'value is returned' do
+        parameter.instance_variable_set(:@value, 'value')
+        expect(parameter.value).to eq 'value'
       end
     end
   end
 
   describe '#rs_value' do
     context 'when the value is not set' do
-      it('nil is returned') do
-        expect(parameter.value).to eq nil
+      it 'nil is returned' do
+        expect(parameter.rs_value).to eq nil
       end
     end
 
-    context 'when value is string' do
-      context 'when the value is set' do
-        it('value is called') do
-          allow(described_class).to receive(:rs_value).and_return(result)
-          described_class.rs_value
-          expect(described_class).to have_received(:rs_value).once
-        end
-
-        it('value is returned') do
-          parameter.instance_variable_set(:@value, value)
-          expect(parameter.rs_value).to eq result
-        end
+    context 'when the value is set' do
+      it 'value is returned' do
+        parameter.instance_variable_set(:@value, 'value')
+        expect(parameter.rs_value).to eq 'value'
       end
     end
   end

From 430f1627d646e86241c4d927195553ef7a7ed34c Mon Sep 17 00:00:00 2001
From: Bernard Pietraga <bernard.pietraga@gmail.com>
Date: Thu, 29 Nov 2018 17:48:36 +0100
Subject: [PATCH 08/14] (maint) update property and read only parameter tests

---
 spec/puppet/resource_api/property_spec.rb     | 152 ++++++++++--------
 .../resource_api/read_only_parameter_spec.rb  |  54 +++----
 2 files changed, 104 insertions(+), 102 deletions(-)

diff --git a/spec/puppet/resource_api/property_spec.rb b/spec/puppet/resource_api/property_spec.rb
index e3cfb654..98c6ba9e 100644
--- a/spec/puppet/resource_api/property_spec.rb
+++ b/spec/puppet/resource_api/property_spec.rb
@@ -9,77 +9,101 @@
   let(:attribute_name) { 'some_property' }
   let(:data_type) { Puppet::Pops::Types::PStringType.new(nil) }
   let(:resource_hash) { { resource: {} } }
-  let(:result) { 'value' }
 
-  it { expect { described_class.new(nil) }.to raise_error ArgumentError, %r{wrong number of arguments} }
-  it { expect { described_class.new(type_name, data_type, attribute_name, resource_hash) }.not_to raise_error }
+  describe '#new(type_name, data_type, attribute_name, resource_hash)' do
+    it { expect { described_class.new(nil) }.to raise_error ArgumentError, %r{wrong number of arguments} }
+    it { expect { described_class.new(type_name, data_type, attribute_name, resource_hash) }.not_to raise_error }
+
+    context 'when name is :ensure' do
+      let(:attribute_name) { :ensure }
+
+      it 'the #insync? method is avaliable' do
+        expect(property.methods.include?(:insync?)).to eq true
+      end
+    end
+  end
 
   describe '#should=(value)' do
-    context 'when value is string' do
-      context 'when the should is set with string value' do
-        let(:value) { 'value' }
-
-        it('value is called') do
-          allow(described_class).to receive(:should=).with(value).and_return(result)
-          allow(described_class).to receive(:rs_value).and_return(result)
-          described_class.should=(value) # rubocop:disable Style/RedundantParentheses, Layout/SpaceAroundOperators
-          expect(described_class).to have_received(:should=).once
-        end
+    context 'when value is string type' do
+      it 'calls mungify and stores the munged value' do
+        expect(Puppet::ResourceApi::DataTypeHandling).to receive(:mungify)
+          .with(data_type, 'value', 'test_name.some_property', false)
+          .and_return('munged value')
 
-        it('value is returned') do
-          expect(property.should=(value)).to eq result # rubocop:disable Style/RedundantParentheses, Layout/SpaceAroundOperators
-        end
+        property.should = 'value'
+
+        expect(property.value).to eq 'munged value'
       end
     end
+
+    context 'when attribute name is :ensure' do
+      let(:attribute_name) { :ensure }
+      let(:data_type) { Puppet::Pops::Types::PEnumType.new(%w[absent present]) }
+
+      it 'calls mungify and stores the munged symbol value' do
+        expect(Puppet::ResourceApi::DataTypeHandling).to receive(:mungify)
+          .with(data_type, 'absent', 'test_name.ensure', false)
+          .and_return('absent')
+
+        property.should = :absent
+
+        expect(property.should).to eq :absent
+      end
+    end
+
+    it 'calls mungify and stores the munged value' do
+      expect(Puppet::ResourceApi::DataTypeHandling).to receive(:mungify)
+        .with(data_type, 'value', 'test_name.some_property', false)
+        .and_return('munged value')
+
+      property.should = 'value'
+
+      expect(property.value).to eq 'munged value'
+    end
+
+    it 'calls mungify and reports its error' do
+      expect(Puppet::ResourceApi::DataTypeHandling).to receive(:mungify)
+        .and_raise Exception, 'error'
+
+      expect { property.should = 'value' }.to raise_error Exception, 'error'
+
+      expect(property.should).to eq nil
+    end
   end
 
   describe '#should' do
-    context 'when value is boolean' do
-      context 'when the @should is set' do
-        let(:should_true_value) { [true] } # rs_value takes value in array
-        let(:true_result) { :true } # rubocop:disable Lint/BooleanSymbol
-        let(:should_false_value) { [false] } # rs_value takes value in array
-        let(:false_result) { :false } # rubocop:disable Lint/BooleanSymbol
-
-        it('true symbol value is returned') do
-          property.instance_variable_set(:@should, should_true_value)
-          expect(property.should).to eq true_result
-        end
-
-        it('false symbol value is returned') do
-          property.instance_variable_set(:@should, should_false_value)
-          expect(property.should).to eq false_result
-        end
+    context 'when the should is not set' do
+      it 'nil is returned' do
+        expect(property.should).to eq nil
       end
     end
 
-    context 'when value is string' do
-      context 'when the name is :ensure' do
-        context 'when the @should is set' do
-          let(:should_value) { ['present'] } # rs_value takes value in array
-          let(:name) { :ensure }
-          let(:result) { 'present' }
-
-          it('symbol value is returned') do
-            property.instance_variable_set(:@should, should_value)
-            property.instance_variable_set(:@name, name)
-            expect(property.should).to eq result
-          end
+    context 'when the should is set' do
+      context 'when should is string' do
+        it 'value is returned' do
+          property.instance_variable_set(:@should, ['value'])
+          expect(property.should).to eq 'value'
         end
       end
 
-      context 'when the should is set' do
-        let(:should_value) { ['value'] } # rs_value takes value in array
+      context 'when should is boolean' do
+        it ':false symbol value is returned' do
+          property.instance_variable_set(:@should, [false])
+          expect(property.should).to eq :false # rubocop:disable Lint/BooleanSymbol
+        end
 
-        it('value is called') do
-          allow(described_class).to receive(:should).and_return(result)
-          described_class.should
-          expect(described_class).to have_received(:should).once
+        it ':true symbol value is returned' do
+          property.instance_variable_set(:@should, [true])
+          expect(property.should).to eq :true # rubocop:disable Lint/BooleanSymbol
         end
+      end
+
+      context 'when atribute name is :ensure' do
+        let(:attribute_name) { :ensure }
 
-        it('value is returned') do
-          property.instance_variable_set(:@should, should_value)
-          expect(property.should).to eq result
+        it 'symbol value is returned' do
+          property.instance_variable_set(:@should, ['present'])
+          expect(property.should).to eq :present
         end
       end
     end
@@ -87,25 +111,15 @@
 
   describe '#rs_value' do
     context 'when the value is not set' do
-      it('nil is returned') do
-        expect(property.value).to eq nil
+      it 'nil is returned' do
+        expect(property.rs_value).to eq nil
       end
     end
 
-    context 'when value is string' do
-      let(:should_value) { ['value'] }
-
-      context 'when the value is set' do
-        it('value is called') do
-          allow(described_class).to receive(:rs_value).and_return(result)
-          described_class.rs_value
-          expect(described_class).to have_received(:rs_value).once
-        end
-
-        it('value is returned') do
-          property.instance_variable_set(:@should, should_value)
-          expect(property.rs_value).to eq result
-        end
+    context 'when the value is set' do
+      it 'value is returned' do
+        property.instance_variable_set(:@should, ['value'])
+        expect(property.rs_value).to eq 'value'
       end
     end
   end
diff --git a/spec/puppet/resource_api/read_only_parameter_spec.rb b/spec/puppet/resource_api/read_only_parameter_spec.rb
index 610177b9..f464e60b 100644
--- a/spec/puppet/resource_api/read_only_parameter_spec.rb
+++ b/spec/puppet/resource_api/read_only_parameter_spec.rb
@@ -7,19 +7,19 @@
 
   let(:type_name) { 'test_name' }
   let(:attribute_name) { 'some_parameter' }
-  let(:data_type) { Puppet::Pops::Types::PStringType }
+  let(:data_type) { Puppet::Pops::Types::PStringType.new(nil) }
   let(:resource_hash) { { resource: {} } }
-  let(:result) { 'value' }
-  let(:value) { 'value' }
 
-  it { expect { described_class.new(nil) }.to raise_error ArgumentError, %r{wrong number of arguments} }
-  it { expect { described_class.new(type_name, data_type, attribute_name, resource_hash) }.not_to raise_error }
+  describe '#new(type_name, data_type, attribute_name, resource_hash)' do
+    it { expect { described_class.new(nil) }.to raise_error ArgumentError, %r{wrong number of arguments} }
+    it { expect { described_class.new(type_name, data_type, attribute_name, resource_hash) }.not_to raise_error }
+  end
 
   describe '#value=(value)' do
     context 'when called from `puppet resource`' do
       context 'when the value set attempt is performed' do
         it 'value set fails' do
-          expect { read_only_parameter.value=(value) }.to raise_error( # rubocop:disable Style/RedundantParentheses, Layout/SpaceAroundOperators
+          expect { read_only_parameter.value = 'value' }.to raise_error(
             Puppet::ResourceError,
             %r{Attempting to set `some_parameter` read_only attribute value to `value`},
           )
@@ -29,43 +29,31 @@
   end
 
   describe '#value' do
-    context 'when value is string' do
-      context 'when the value is set' do
-        before(:each) do
-          allow(described_class).to receive(:value).and_return(result)
-        end
-
-        it('value is called') do
-          described_class.value
-          expect(described_class).to have_received(:value).once
-        end
+    context 'when the value is not set' do
+      it 'nil is returned' do
+        expect(read_only_parameter.value).to eq nil
+      end
+    end
 
-        it('value is returned') do
-          expect(described_class.value).to eq result
-        end
+    context 'when the value is set' do
+      it 'value is returned' do
+        read_only_parameter.instance_variable_set(:@value, 'value')
+        expect(read_only_parameter.value).to eq 'value'
       end
     end
   end
 
   describe '#rs_value' do
     context 'when the value is not set' do
-      it('nil is returned') do
-        expect(read_only_parameter.value).to eq nil
+      it 'nil is returned' do
+        expect(read_only_parameter.rs_value).to eq nil
       end
     end
 
-    context 'when value is string' do
-      context 'when the value is set' do
-        it('value is called') do
-          allow(described_class).to receive(:rs_value).and_return(result)
-          described_class.rs_value
-          expect(described_class).to have_received(:rs_value).once
-        end
-
-        it('value is returned') do
-          read_only_parameter.instance_variable_set(:@value, value)
-          expect(read_only_parameter.rs_value).to eq result
-        end
+    context 'when the value is set' do
+      it 'value is returned' do
+        read_only_parameter.instance_variable_set(:@value, 'value')
+        expect(read_only_parameter.rs_value).to eq 'value'
       end
     end
   end

From 48957a80e92767fd3b43e79ce2aaa193ceb972ae Mon Sep 17 00:00:00 2001
From: Bernard Pietraga <bernard.pietraga@gmail.com>
Date: Fri, 30 Nov 2018 09:57:06 +0100
Subject: [PATCH 09/14] (maint) Delete unnecessary requires in resource_api.rb

---
 lib/puppet/resource_api.rb | 2 --
 1 file changed, 2 deletions(-)

diff --git a/lib/puppet/resource_api.rb b/lib/puppet/resource_api.rb
index b0dd0ab9..0f9ba788 100644
--- a/lib/puppet/resource_api.rb
+++ b/lib/puppet/resource_api.rb
@@ -10,8 +10,6 @@
 require 'puppet/resource_api/version'
 require 'puppet/type'
 require 'puppet/util/network_device'
-require 'puppet/property'
-require 'puppet/parameter'
 
 module Puppet::ResourceApi
   @warning_count = 0

From a96abac5d4dad8bf1b0ac951089a072d816c4d07 Mon Sep 17 00:00:00 2001
From: Bernard Pietraga <bernard.pietraga@gmail.com>
Date: Mon, 3 Dec 2018 10:54:24 +0100
Subject: [PATCH 10/14] (maint) Update resource tests

---
 spec/puppet/resource_api/parameter_spec.rb    |  53 ++++-----
 spec/puppet/resource_api/property_spec.rb     | 111 ++++--------------
 .../resource_api/read_only_parameter_spec.rb  |  46 +++-----
 3 files changed, 55 insertions(+), 155 deletions(-)

diff --git a/spec/puppet/resource_api/parameter_spec.rb b/spec/puppet/resource_api/parameter_spec.rb
index f5fac01f..2c930f4a 100644
--- a/spec/puppet/resource_api/parameter_spec.rb
+++ b/spec/puppet/resource_api/parameter_spec.rb
@@ -15,17 +15,7 @@
     it { expect { described_class.new(type_name, data_type, attribute_name, resource_hash) }.not_to raise_error }
   end
 
-  describe '#value=(value)' do
-    it 'calls mungify and stores the munged value' do
-      expect(Puppet::ResourceApi::DataTypeHandling).to receive(:mungify)
-        .with(data_type, 'value', 'test_name.some_parameter', false)
-        .and_return('munged value')
-
-      parameter.value = 'value'
-
-      expect(parameter.value).to eq 'munged value'
-    end
-
+  describe 'value error handling' do
     it 'calls mungify and reports its error' do
       expect(Puppet::ResourceApi::DataTypeHandling).to receive(:mungify)
         .and_raise Exception, 'error'
@@ -36,33 +26,30 @@
     end
   end
 
-  describe '#value' do
-    context 'when the value is not set' do
-      it 'nil is returned' do
-        expect(parameter.value).to eq nil
-      end
-    end
+  describe 'value munging and storage' do
+    before(:each) do
+      allow(Puppet::ResourceApi::DataTypeHandling).to receive(:mungify)
+        .with(data_type, value, 'test_name.some_parameter', false)
+        .and_return(munged_value)
 
-    context 'when the value is set' do
-      it 'value is returned' do
-        parameter.instance_variable_set(:@value, 'value')
-        expect(parameter.value).to eq 'value'
-      end
+      parameter.value = value
     end
-  end
 
-  describe '#rs_value' do
-    context 'when the value is not set' do
-      it 'nil is returned' do
-        expect(parameter.rs_value).to eq nil
-      end
+    context 'when handling strings' do
+      let(:value) { 'value' }
+      let(:munged_value) { 'munged value' }
+
+      it { expect(parameter.rs_value).to eq 'munged value' }
+      it { expect(parameter.value).to eq 'munged value' }
     end
 
-    context 'when the value is set' do
-      it 'value is returned' do
-        parameter.instance_variable_set(:@value, 'value')
-        expect(parameter.rs_value).to eq 'value'
-      end
+    context 'when handling boolean true' do
+      let(:value) { true }
+      let(:munged_value) { true }
+      let(:data_type) { Puppet::Pops::Types::PBooleanType.new }
+
+      it { expect(parameter.rs_value).to eq true }
+      it { expect(parameter.value).to eq true }
     end
   end
 end
diff --git a/spec/puppet/resource_api/property_spec.rb b/spec/puppet/resource_api/property_spec.rb
index 98c6ba9e..edbde76b 100644
--- a/spec/puppet/resource_api/property_spec.rb
+++ b/spec/puppet/resource_api/property_spec.rb
@@ -13,54 +13,9 @@
   describe '#new(type_name, data_type, attribute_name, resource_hash)' do
     it { expect { described_class.new(nil) }.to raise_error ArgumentError, %r{wrong number of arguments} }
     it { expect { described_class.new(type_name, data_type, attribute_name, resource_hash) }.not_to raise_error }
-
-    context 'when name is :ensure' do
-      let(:attribute_name) { :ensure }
-
-      it 'the #insync? method is avaliable' do
-        expect(property.methods.include?(:insync?)).to eq true
-      end
-    end
   end
 
-  describe '#should=(value)' do
-    context 'when value is string type' do
-      it 'calls mungify and stores the munged value' do
-        expect(Puppet::ResourceApi::DataTypeHandling).to receive(:mungify)
-          .with(data_type, 'value', 'test_name.some_property', false)
-          .and_return('munged value')
-
-        property.should = 'value'
-
-        expect(property.value).to eq 'munged value'
-      end
-    end
-
-    context 'when attribute name is :ensure' do
-      let(:attribute_name) { :ensure }
-      let(:data_type) { Puppet::Pops::Types::PEnumType.new(%w[absent present]) }
-
-      it 'calls mungify and stores the munged symbol value' do
-        expect(Puppet::ResourceApi::DataTypeHandling).to receive(:mungify)
-          .with(data_type, 'absent', 'test_name.ensure', false)
-          .and_return('absent')
-
-        property.should = :absent
-
-        expect(property.should).to eq :absent
-      end
-    end
-
-    it 'calls mungify and stores the munged value' do
-      expect(Puppet::ResourceApi::DataTypeHandling).to receive(:mungify)
-        .with(data_type, 'value', 'test_name.some_property', false)
-        .and_return('munged value')
-
-      property.should = 'value'
-
-      expect(property.value).to eq 'munged value'
-    end
-
+  describe 'should error handling' do
     it 'calls mungify and reports its error' do
       expect(Puppet::ResourceApi::DataTypeHandling).to receive(:mungify)
         .and_raise Exception, 'error'
@@ -71,56 +26,32 @@
     end
   end
 
-  describe '#should' do
-    context 'when the should is not set' do
-      it 'nil is returned' do
-        expect(property.should).to eq nil
-      end
-    end
+  describe 'value munging and storage' do
+    before(:each) do
+      allow(Puppet::ResourceApi::DataTypeHandling).to receive(:mungify)
+        .with(data_type, value, 'test_name.some_property', false)
+        .and_return(munged_value)
 
-    context 'when the should is set' do
-      context 'when should is string' do
-        it 'value is returned' do
-          property.instance_variable_set(:@should, ['value'])
-          expect(property.should).to eq 'value'
-        end
-      end
-
-      context 'when should is boolean' do
-        it ':false symbol value is returned' do
-          property.instance_variable_set(:@should, [false])
-          expect(property.should).to eq :false # rubocop:disable Lint/BooleanSymbol
-        end
-
-        it ':true symbol value is returned' do
-          property.instance_variable_set(:@should, [true])
-          expect(property.should).to eq :true # rubocop:disable Lint/BooleanSymbol
-        end
-      end
+      property.should = value
+    end
 
-      context 'when atribute name is :ensure' do
-        let(:attribute_name) { :ensure }
+    context 'when handling strings' do
+      let(:value) { 'value' }
+      let(:munged_value) { 'munged value' }
 
-        it 'symbol value is returned' do
-          property.instance_variable_set(:@should, ['present'])
-          expect(property.should).to eq :present
-        end
-      end
+      it { expect(property.should).to eq 'munged value' }
+      it { expect(property.rs_value).to eq 'munged value' }
+      it { expect(property.value).to eq 'munged value' }
     end
-  end
 
-  describe '#rs_value' do
-    context 'when the value is not set' do
-      it 'nil is returned' do
-        expect(property.rs_value).to eq nil
-      end
-    end
+    context 'when handling boolean true' do
+      let(:value) { true }
+      let(:munged_value) { true }
+      let(:data_type) { Puppet::Pops::Types::PBooleanType.new }
 
-    context 'when the value is set' do
-      it 'value is returned' do
-        property.instance_variable_set(:@should, ['value'])
-        expect(property.rs_value).to eq 'value'
-      end
+      it { expect(property.should).to eq :true } # rubocop:disable Lint/BooleanSymbol
+      it { expect(property.rs_value).to eq true }
+      it { expect(property.value).to eq :true } # rubocop:disable Lint/BooleanSymbol
     end
   end
 end
diff --git a/spec/puppet/resource_api/read_only_parameter_spec.rb b/spec/puppet/resource_api/read_only_parameter_spec.rb
index f464e60b..226b18e7 100644
--- a/spec/puppet/resource_api/read_only_parameter_spec.rb
+++ b/spec/puppet/resource_api/read_only_parameter_spec.rb
@@ -15,46 +15,28 @@
     it { expect { described_class.new(type_name, data_type, attribute_name, resource_hash) }.not_to raise_error }
   end
 
-  describe '#value=(value)' do
-    context 'when called from `puppet resource`' do
-      context 'when the value set attempt is performed' do
-        it 'value set fails' do
-          expect { read_only_parameter.value = 'value' }.to raise_error(
-            Puppet::ResourceError,
-            %r{Attempting to set `some_parameter` read_only attribute value to `value`},
-          )
-        end
+  describe 'value munging and storage' do
+    context 'when the value set attempt is performed' do
+      it 'value set fails' do
+        expect { read_only_parameter.value = 'value' }.to raise_error(
+          Puppet::ResourceError,
+          %r{Attempting to set `some_parameter` read_only attribute value to `value`},
+        )
       end
     end
-  end
 
-  describe '#value' do
-    context 'when the value is not set' do
-      it 'nil is returned' do
-        expect(read_only_parameter.value).to eq nil
-      end
+    context 'when value is not set' do
+      it { expect(read_only_parameter.rs_value).to eq nil }
+      it { expect(read_only_parameter.value).to eq nil }
     end
 
-    context 'when the value is set' do
-      it 'value is returned' do
+    context 'when value is already set' do
+      before(:each) do
         read_only_parameter.instance_variable_set(:@value, 'value')
-        expect(read_only_parameter.value).to eq 'value'
       end
-    end
-  end
 
-  describe '#rs_value' do
-    context 'when the value is not set' do
-      it 'nil is returned' do
-        expect(read_only_parameter.rs_value).to eq nil
-      end
-    end
-
-    context 'when the value is set' do
-      it 'value is returned' do
-        read_only_parameter.instance_variable_set(:@value, 'value')
-        expect(read_only_parameter.rs_value).to eq 'value'
-      end
+      it { expect(read_only_parameter.rs_value).to eq 'value' }
+      it { expect(read_only_parameter.value).to eq 'value' }
     end
   end
 end

From 003726f71728c4a5bb0897ebc02deceab4af267e Mon Sep 17 00:00:00 2001
From: Bernard Pietraga <bernard.pietraga@gmail.com>
Date: Mon, 3 Dec 2018 13:45:31 +0100
Subject: [PATCH 11/14] (maint) Add class check for property class #insync?
 overload

---
 lib/puppet/resource_api/property.rb | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/puppet/resource_api/property.rb b/lib/puppet/resource_api/property.rb
index 7860f9b6..182fa4f5 100644
--- a/lib/puppet/resource_api/property.rb
+++ b/lib/puppet/resource_api/property.rb
@@ -66,10 +66,11 @@ def rs_value
     @should ? @should.first : @should
   end
 
-  # method added only for the :ensure property, add option to check if the
-  # rs_value matches is
+  # method overloaded only for the :ensure property, add option to check if the
+  # rs_value matches is. Only if the class is child of
+  # Puppet::ResourceApi::Property.
   def def_insync?
-    self.class.send(:define_method, :insync?) { |is| rs_value.to_s == is.to_s }
+    self.class.send(:define_method, :insync?) { |is| rs_value.to_s == is.to_s } unless self.class == Puppet::ResourceApi::Property
   end
 
   # puppet symbolizes some values through puppet/parameter/value.rb

From f91bb120bcd73cf09bff18104fa74f885d341168 Mon Sep 17 00:00:00 2001
From: Bernard Pietraga <bernard.pietraga@gmail.com>
Date: Tue, 4 Dec 2018 15:47:29 +0100
Subject: [PATCH 12/14] (maint) Add rename in ValueCreator and update tests

Rename resource_class to attribute_class to avoid confusion
in Puppet::ResourceApi::ValueCreator.

Update `value_creator_spec.rb` to check methods call arguments.
---
 lib/puppet/resource_api/value_creator.rb      | 30 +++----
 .../puppet/resource_api/value_creator_spec.rb | 82 ++++++++++---------
 2 files changed, 59 insertions(+), 53 deletions(-)

diff --git a/lib/puppet/resource_api/value_creator.rb b/lib/puppet/resource_api/value_creator.rb
index 93e91ea8..cb7c119b 100644
--- a/lib/puppet/resource_api/value_creator.rb
+++ b/lib/puppet/resource_api/value_creator.rb
@@ -11,17 +11,17 @@ module Puppet::ResourceApi::ValueCreator
   # @param definition the definition of the property
   # @param options the ResourceAPI options hash containing setup information for
   # selected parameter or property
-  def self.create_values(resource_class, data_type, param_or_property, options = {})
-    resource_class.isnamevar if options[:behaviour] == :namevar
+  def self.create_values(attribute_class, data_type, param_or_property, options = {})
+    attribute_class.isnamevar if options[:behaviour] == :namevar
 
     # read-only values do not need type checking, but can have default values
     if options[:behaviour] != :read_only && options.key?(:default)
       if options[:default] == false
         # work around https://tickets.puppetlabs.com/browse/PUP-2368
-        resource_class.defaultto :false # rubocop:disable Lint/BooleanSymbol
+        attribute_class.defaultto :false # rubocop:disable Lint/BooleanSymbol
       elsif options[:default] == true
         # work around https://tickets.puppetlabs.com/browse/PUP-2368
-        resource_class.defaultto :true # rubocop:disable Lint/BooleanSymbol
+        attribute_class.defaultto :true # rubocop:disable Lint/BooleanSymbol
       else
         # marshal the default option to decouple that from the actual value.
         # we cache the dumped value in `marshalled`, but use a block to
@@ -29,7 +29,7 @@ def self.create_values(resource_class, data_type, param_or_property, options = {
         # marshalled
         # See https://stackoverflow.com/a/8206537/4918
         marshalled = Marshal.dump(options[:default])
-        resource_class.defaultto { Marshal.load(marshalled) } # rubocop:disable Security/MarshalLoad
+        attribute_class.defaultto { Marshal.load(marshalled) } # rubocop:disable Security/MarshalLoad
       end
     end
 
@@ -41,24 +41,24 @@ def self.create_values(resource_class, data_type, param_or_property, options = {
     case data_type
     when Puppet::Pops::Types::PStringType
       # require any string value
-      def_newvalues(resource_class, param_or_property, %r{})
+      def_newvalues(attribute_class, param_or_property, %r{})
     when Puppet::Pops::Types::PBooleanType
-      def_newvalues(resource_class, param_or_property, 'true', 'false')
-      resource_class.aliasvalue true, 'true'
-      resource_class.aliasvalue false, 'false'
-      resource_class.aliasvalue :true, 'true' # rubocop:disable Lint/BooleanSymbol
-      resource_class.aliasvalue :false, 'false' # rubocop:disable Lint/BooleanSymbol
+      def_newvalues(attribute_class, param_or_property, 'true', 'false')
+      attribute_class.aliasvalue true, 'true'
+      attribute_class.aliasvalue false, 'false'
+      attribute_class.aliasvalue :true, 'true' # rubocop:disable Lint/BooleanSymbol
+      attribute_class.aliasvalue :false, 'false' # rubocop:disable Lint/BooleanSymbol
     when Puppet::Pops::Types::PIntegerType
-      def_newvalues(resource_class, param_or_property, %r{^-?\d+$})
+      def_newvalues(attribute_class, param_or_property, %r{^-?\d+$})
     when Puppet::Pops::Types::PFloatType, Puppet::Pops::Types::PNumericType
-      def_newvalues(resource_class, param_or_property, Puppet::Pops::Patterns::NUMERIC)
+      def_newvalues(attribute_class, param_or_property, Puppet::Pops::Patterns::NUMERIC)
     end
 
     case options[:type]
     when 'Enum[present, absent]'
-      def_newvalues(resource_class, param_or_property, 'absent', 'present')
+      def_newvalues(attribute_class, param_or_property, 'absent', 'present')
     end
-    resource_class
+    attribute_class
   end
 
   # add the value to `this` property or param, depending on whether
diff --git a/spec/puppet/resource_api/value_creator_spec.rb b/spec/puppet/resource_api/value_creator_spec.rb
index 8dc1f384..feb0e6fb 100644
--- a/spec/puppet/resource_api/value_creator_spec.rb
+++ b/spec/puppet/resource_api/value_creator_spec.rb
@@ -5,7 +5,7 @@
     described_class
   end
 
-  let(:resource_class) { Puppet::ResourceApi::Property }
+  let(:attribute_class) { Puppet::ResourceApi::Property }
   let(:data_type) { Puppet::Pops::Types::PEnumType.new(['absent', 'present']) } # rubocop:disable Style/WordArray
   let(:param_or_property) { :newproperty }
   let(:options) do
@@ -17,11 +17,11 @@
   end
 
   before(:each) do
-    allow(resource_class).to receive(:newvalue)
-    allow(resource_class).to receive(:newvalues)
-    allow(resource_class).to receive(:aliasvalue)
-    allow(resource_class).to receive(:defaultto)
-    allow(resource_class).to receive(:isnamevar)
+    allow(attribute_class).to receive(:newvalue)
+    allow(attribute_class).to receive(:newvalues)
+    allow(attribute_class).to receive(:aliasvalue)
+    allow(attribute_class).to receive(:defaultto)
+    allow(attribute_class).to receive(:isnamevar)
   end
 
   it { expect { described_class.create_values(nil) }.to raise_error ArgumentError, %r{wrong number of arguments} }
@@ -29,23 +29,20 @@
 
   describe '#create_values' do
     before(:each) do
-      value_creator.create_values(resource_class, data_type, param_or_property, options)
+      value_creator.create_values(attribute_class, data_type, param_or_property, options)
     end
 
-    context 'when resource class is property' do
-      it 'resource_class has #call_provider defined' do
-        expect(resource_class.method_defined?(:call_provider)).to eq(true)
-      end
-
+    context 'when attribute class is property' do
       context 'when property is :ensure' do
         it 'calls #newvalue twice' do
-          expect(Puppet::ResourceApi::Property).to have_received(:newvalue).twice
+          expect(Puppet::ResourceApi::Property).to have_received(:newvalue).with('absent')
+          expect(Puppet::ResourceApi::Property).to have_received(:newvalue).with('present')
         end
       end
 
       context 'when default is set' do
         it 'calls #defaultto once' do
-          expect(resource_class).to have_received(:defaultto)
+          expect(attribute_class).to have_received(:defaultto) { |&block| expect(block[0]).to eq('present') }
         end
       end
 
@@ -59,11 +56,15 @@
         end
 
         it 'calls #newvalue twice' do
-          expect(resource_class).to have_received(:newvalue).twice
+          expect(attribute_class).to have_received(:newvalue).with('true')
+          expect(attribute_class).to have_received(:newvalue).with('false')
         end
 
         it 'calls #aliasvalue four times' do
-          expect(resource_class).to have_received(:aliasvalue).at_least(4).times
+          expect(attribute_class).to have_received(:aliasvalue).with(true, 'true')
+          expect(attribute_class).to have_received(:aliasvalue).with(false, 'false')
+          expect(attribute_class).to have_received(:aliasvalue).with(:true, 'true') # rubocop:disable Lint/BooleanSymbol
+          expect(attribute_class).to have_received(:aliasvalue).with(:false, 'false') # rubocop:disable Lint/BooleanSymbol
         end
       end
 
@@ -76,8 +77,8 @@
           }
         end
 
-        it 'calls #newvalue twice' do
-          expect(resource_class).to have_received(:newvalue).once
+        it 'calls #newvalue once' do
+          expect(attribute_class).to have_received(:newvalue).with(%r{})
         end
       end
 
@@ -90,8 +91,8 @@
           }
         end
 
-        it 'calls #newvalue twice' do
-          expect(resource_class).to have_received(:newvalue).once
+        it 'calls #newvalue once' do
+          expect(attribute_class).to have_received(:newvalue).with(%r{^-?\d+$})
         end
       end
 
@@ -104,19 +105,19 @@
           }
         end
 
-        it 'calls #newvalue twice' do
-          expect(resource_class).to have_received(:newvalue).once
+        it 'calls #newvalue once' do
+          expect(attribute_class).to have_received(:newvalue)
         end
       end
     end
 
-    context 'when resource class is parameter' do
-      let(:resource_class) { Puppet::ResourceApi::Parameter }
+    context 'when attribute class is parameter' do
+      let(:attribute_class) { Puppet::ResourceApi::Parameter }
       let(:data_type) { Puppet::Pops::Types::PBooleanType.new }
       let(:param_or_property) { :newparam }
 
-      it 'resource_class has no #call_provider method' do
-        expect(resource_class.method_defined?(:call_provider)).to eq(false)
+      it 'attribute_class has no #call_provider method' do
+        expect(attribute_class.method_defined?(:call_provider)).to eq(false)
       end
 
       context 'when behaviour is set to :namevar' do
@@ -129,7 +130,7 @@
         end
 
         it 'calls #isnamevar once' do
-          expect(resource_class).to have_received(:isnamevar).once
+          expect(attribute_class).to have_received(:isnamevar)
         end
       end
 
@@ -143,7 +144,7 @@
         end
 
         it 'calls #defaultto once' do
-          expect(resource_class).to have_received(:defaultto).once
+          expect(attribute_class).to have_received(:defaultto).with(:true) # rubocop:disable Lint/BooleanSymbol
         end
       end
 
@@ -156,7 +157,7 @@
         end
 
         it 'does not call #defaultto' do
-          expect(resource_class).not_to receive(:defaultto)
+          expect(attribute_class).not_to receive(:defaultto).with(nil)
         end
       end
 
@@ -169,12 +170,15 @@
           }
         end
 
-        it 'calls #newvalues twice' do
-          expect(resource_class).to have_received(:newvalues).once
+        it 'calls #newvalues once' do
+          expect(attribute_class).to have_received(:newvalues).with('true', 'false')
         end
 
         it 'calls #aliasvalue four times' do
-          expect(resource_class).to have_received(:aliasvalue).at_least(4).times
+          expect(attribute_class).to have_received(:aliasvalue).with(true, 'true')
+          expect(attribute_class).to have_received(:aliasvalue).with(false, 'false')
+          expect(attribute_class).to have_received(:aliasvalue).with(:true, 'true') # rubocop:disable Lint/BooleanSymbol
+          expect(attribute_class).to have_received(:aliasvalue).with(:false, 'false') # rubocop:disable Lint/BooleanSymbol
         end
       end
 
@@ -187,8 +191,8 @@
           }
         end
 
-        it 'calls #newvalues twice' do
-          expect(resource_class).to have_received(:newvalues).once
+        it 'calls #newvalues once' do
+          expect(attribute_class).to have_received(:newvalues).with(%r{})
         end
       end
 
@@ -201,8 +205,8 @@
           }
         end
 
-        it 'calls #newvalues twice' do
-          expect(resource_class).to have_received(:newvalues).once
+        it 'calls #newvalues once' do
+          expect(attribute_class).to have_received(:newvalues).with(%r{^-?\d+$})
         end
       end
 
@@ -215,8 +219,10 @@
           }
         end
 
-        it 'calls #newvalues twice' do
-          expect(resource_class).to have_received(:newvalues).once
+        it 'calls #newvalues once' do
+          expect(attribute_class).to have_received(:newvalues).with(
+            %r{\A[[:blank:]]*([-+]?)[[:blank:]]*((0[xX][0-9A-Fa-f]+)|(0?\d+)((?:\.\d+)?(?:[eE]-?\d+)?))[[:blank:]]*\z},
+          )
         end
       end
     end

From 87d8b0fc8ce37a73438352fbf85cea8e77e02646 Mon Sep 17 00:00:00 2001
From: Bernard Pietraga <bernard.pietraga@gmail.com>
Date: Tue, 4 Dec 2018 17:30:45 +0100
Subject: [PATCH 13/14] (maint) Change the definition of #insync?

---
 lib/puppet/resource_api/property.rb       |  4 +--
 spec/puppet/resource_api/property_spec.rb | 34 +++++++++++++++++++++++
 2 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/lib/puppet/resource_api/property.rb b/lib/puppet/resource_api/property.rb
index 182fa4f5..34609f8c 100644
--- a/lib/puppet/resource_api/property.rb
+++ b/lib/puppet/resource_api/property.rb
@@ -16,7 +16,7 @@ def initialize(type_name, data_type, attribute_name, resource_hash)
     @data_type = data_type
     @attribute_name = attribute_name
     # Define class method insync?(is) if the name is :ensure
-    def_insync? if @attribute_name == :ensure
+    def_insync? if @attribute_name == :ensure && self.class != Puppet::ResourceApi::Property
     # Pass resource to parent Puppet class.
     super(resource_hash)
   end
@@ -70,7 +70,7 @@ def rs_value
   # rs_value matches is. Only if the class is child of
   # Puppet::ResourceApi::Property.
   def def_insync?
-    self.class.send(:define_method, :insync?) { |is| rs_value.to_s == is.to_s } unless self.class == Puppet::ResourceApi::Property
+    define_singleton_method(:insync?) { |is| rs_value.to_s == is.to_s }
   end
 
   # puppet symbolizes some values through puppet/parameter/value.rb
diff --git a/spec/puppet/resource_api/property_spec.rb b/spec/puppet/resource_api/property_spec.rb
index edbde76b..8c285f31 100644
--- a/spec/puppet/resource_api/property_spec.rb
+++ b/spec/puppet/resource_api/property_spec.rb
@@ -15,6 +15,40 @@
     it { expect { described_class.new(type_name, data_type, attribute_name, resource_hash) }.not_to raise_error }
   end
 
+  describe 'the special :ensure behaviour' do
+    let(:ensure_property_class) do
+      Class.new(described_class) do
+        define_method(:initialize) do
+          super('test_name',
+            Puppet::Pops::Types::PEnumType.new(%w[absent present]),
+            :ensure,
+            { resource: {} })
+        end
+      end
+    end
+    let(:ensure_property) { ensure_property_class.new }
+
+    it 'has a #insync? method' do
+      expect(ensure_property.public_methods(false)).to include(:insync?)
+    end
+
+    describe '#insync?' do
+      let(:data_type) { Puppet::Pops::Types::PEnumType.new(%w[absent present]) }
+
+      before(:each) do
+        allow(Puppet::ResourceApi::DataTypeHandling).to receive(:mungify)
+          .with(data_type, 'present', 'test_name.ensure', false)
+          .and_return('present')
+
+        ensure_property.should = 'present'
+      end
+
+      it 'compares using symbols' do
+        expect(ensure_property.insync?(:present)).to eq(true)
+      end
+    end
+  end
+
   describe 'should error handling' do
     it 'calls mungify and reports its error' do
       expect(Puppet::ResourceApi::DataTypeHandling).to receive(:mungify)

From 83f99dbfc524597d7f2708387811c166c3c52ae5 Mon Sep 17 00:00:00 2001
From: Bernard Pietraga <bernard.pietraga@gmail.com>
Date: Tue, 4 Dec 2018 17:49:41 +0100
Subject: [PATCH 14/14] (maint) Update :ensure property tests

---
 spec/puppet/resource_api/property_spec.rb | 24 +++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/spec/puppet/resource_api/property_spec.rb b/spec/puppet/resource_api/property_spec.rb
index 8c285f31..50d6c589 100644
--- a/spec/puppet/resource_api/property_spec.rb
+++ b/spec/puppet/resource_api/property_spec.rb
@@ -28,25 +28,29 @@
     end
     let(:ensure_property) { ensure_property_class.new }
 
+    before(:each) do
+      allow(Puppet::ResourceApi::DataTypeHandling).to receive(:mungify)
+        .with(Puppet::Pops::Types::PEnumType.new(%w[absent present]), 'present', 'test_name.ensure', false)
+        .and_return('present')
+
+      ensure_property.should = 'present'
+    end
+
     it 'has a #insync? method' do
       expect(ensure_property.public_methods(false)).to include(:insync?)
     end
 
     describe '#insync?' do
-      let(:data_type) { Puppet::Pops::Types::PEnumType.new(%w[absent present]) }
-
-      before(:each) do
-        allow(Puppet::ResourceApi::DataTypeHandling).to receive(:mungify)
-          .with(data_type, 'present', 'test_name.ensure', false)
-          .and_return('present')
-
-        ensure_property.should = 'present'
-      end
-
       it 'compares using symbols' do
         expect(ensure_property.insync?(:present)).to eq(true)
       end
     end
+
+    context "when handling 'present' string" do
+      it { expect(ensure_property.should).to eq :present }
+      it { expect(ensure_property.rs_value).to eq 'present' }
+      it { expect(ensure_property.value).to eq :present }
+    end
   end
 
   describe 'should error handling' do