diff --git a/.vscode/extensions.json b/.vscode/extensions.json
deleted file mode 100644
index 2f1e4f7..0000000
--- a/.vscode/extensions.json
+++ /dev/null
@@ -1,6 +0,0 @@
-{
-  "recommendations": [
-    "puppet.puppet-vscode",
-    "rebornix.Ruby"
-  ]
-}
diff --git a/lib/puppet/type/node_group.rb b/lib/puppet/type/node_group.rb
index 9b84d36..cd6f5c9 100644
--- a/lib/puppet/type/node_group.rb
+++ b/lib/puppet/type/node_group.rb
@@ -35,75 +35,81 @@
   end
   newproperty(:rule, array_matching: :all) do
     desc 'Match conditions for this group'
+    # rubocop:disable Metrics/CyclomaticComplexity
+    # rubocop:disable Metrics/PerceivedComplexity
     def should
       case @resource[:purge_behavior]
-      when :rule, :all
-        super
-      else
+      # only do something special when we are asked to merge, otherwise, fall back to the default
+      when :none
+        # rule grammar
+        #    condition : [ {bool} {condition}+ ] | [ "not" {condition} ] | {operation}
+        #    bool : "and" | "or"
+        #    operation : [ {operator} {fact-path} {value} ]
+        #    operator : "=" | "~" | ">" | ">=" | "<" | "<=" | "<:"
+        #    fact-path : {field-name} | [ {path-type} {field-name} {path-component}+ ]
+        #    path-type : "trusted" | "fact"
+        #    path-component : field-name | number
+        #    field-name : string
+        #    value : string
+
         a = @resource.property(:rule).retrieve || {}
         b = shouldorig
-        aorig = a.map(&:clone)
-        atmp = a.map(&:clone)
-        # check if the node classifer has any rules defined before attempting merge.
-        if a.length >= 2
-          if (a[0] == 'or') && (a[1][0] == 'or') || (a[1][0] == 'and')
-            # Merging both rules and pinned nodes
-            if (b[0] == 'or') && (b[1][0] == 'or') || (b[1][0] == 'and')
-              # b has rules to merge
-              rules = (atmp[1] + b[1].drop(1)).uniq
-              atmp[1] = rules
-              pinned = (b[2, b.length] + atmp[2, atmp.length]).uniq
-              merged = (atmp + pinned).uniq
-            elsif ((b[0] == 'and') || (b[0] == 'or')) && PuppetX::Node_manager::Common.factcheck(b)
-              # b only has rules to merge
-              rules = (atmp[1] + b.drop(1)).uniq
-              atmp[1] = rules
-              merged = atmp
-            else
-              pinned = (b[1, b.length] + atmp[2, atmp.length]).uniq
-              merged = (atmp + pinned).uniq
-            end
-          elsif (b[0] == 'or') && (b[1][0] == 'or') || (b[1][0] == 'and')
-            # Merging both rules and pinned nodes
-            rules = b[1] # no rules to merge on a side
-            pinned = (b[2, b.length] + a[1, a.length]).uniq
-            merged = (b + pinned).uniq
-          elsif ((a[0] == 'and') || (a[0] == 'or')) && PuppetX::Node_manager::Common.factcheck(a)
-            # a only has fact rules
-            if (b[0] == 'or') && !PuppetX::Node_manager::Common.factcheck(b)
-              # b only has pinned nodes
-              rules = atmp
-              temp = ['or']
-              temp[1] = atmp
-              merged = (temp + b[1, b.length]).uniq
-            else
-              # b only has rules
-              merged = (a + b.drop(1)).uniq
-            end
-          elsif (a[0] == 'or') && (a[1][1] == 'name')
-            # a only has pinned nodes
-            if (b[0] == 'or') && !PuppetX::Node_manager::Common.factcheck(b)
-              # b only has pinned nodes
-              merged = (b + a.drop(1)).uniq
-            else
-              # b only has rules
-              temp = ['or']
-              temp[1] = b
-              merged = (temp + atmp[1, atmp.length]).uniq
-            end
-          else
-            # default fall back.
-            merged = (b + a.drop(1)).uniq
-          end
-          if merged == aorig
-            # values are the same, returning orginal value"
-            aorig
-          else
-            merged
-          end
-        else
-          b
+
+        # extract all pinned nodes if any
+        # pinned nodes are in the form ['=', 'name', <hostname>]
+        apinned = []
+        a_without_pinned = a
+        if a[0] == 'or'
+          apinned = a.select { |item| (item[0] == '=') && (item[1] == 'name') }
+          a_without_pinned = a.select { |item| (item[0] != '=') || (item[1] != 'name') }
+        end
+        bpinned = []
+        b_without_pinned = b
+        merged = []
+
+        (return b.uniq.select { |item| (item != ['or'] && item != ['and']) } if a == [''])
+        (return a.uniq.select { |item| (item != ['or'] && item != ['and']) } if b == [''])
+
+        if b[0] == 'or'
+          bpinned = b.select { |item| (item[0] == '=') && (item[1] == 'name') }
+          b_without_pinned = b.select { |item| (item[0] != '=') || (item[1] != 'name') }
         end
+
+        merged = if ((a[0] == 'and') || (a[0] == 'or')) && a[0] == b[0]
+                   # if a and b start with the same 'and' or 'or' clause, we can just combine them
+                   if a[0] == 'or'
+                     (['or'] + a_without_pinned.drop(1) + b_without_pinned.drop(1) + apinned + bpinned).uniq
+                   elsif apinned.length.positive? || bpinned.length.positive?
+                     # must both be 'and' clauses
+                     (['or'] + [a_without_pinned + b_without_pinned.drop(1)] + apinned + bpinned).uniq
+                   # we have pinned nodes
+                   else
+                     # no pinned nodes and one top level 'and' clause, just combine them.
+                     a_without_pinned + b_without_pinned.drop(1)
+                   end
+                 elsif a_without_pinned[0] == 'and' && b_without_pinned[0] == 'or'
+                   # first clause of a and b aren't equal
+                   # a first clause is one of and/or/not/operator
+                   # b first clause is one of and/or/not/operator
+                   # if a starts with `and` and b starts with `or`, create a top level `or` clause, nest a under it and append the rest of b
+                   if a_without_pinned.length == 2
+                     (['or'] + a_without_pinned[1] + b_without_pinned.drop(1) + apinned + bpinned)
+                   else
+                     (['or'] + [a_without_pinned] + b_without_pinned.drop(1) + apinned + bpinned)
+                   end
+                 # special case of a only having one subclause
+                 elsif a_without_pinned[0] == 'or'
+                   (a_without_pinned + [b_without_pinned] + apinned + bpinned).uniq
+                 elsif b_without_pinned[0] == 'or'
+                   # if b starts with 'or', we want to be sure to drop that.
+                   (['or'] + [a_without_pinned] + b_without_pinned.drop(1) + apinned + bpinned)
+                 else
+                   (['or'] + [a_without_pinned] + [b_without_pinned] + apinned + bpinned)
+                 end
+        # ensure rules are unique at the top level and remove any empty rule sets
+        merged.uniq.select { |item| (item != ['or'] && item != ['and']) }
+      else
+        super
       end
     end
 
@@ -111,6 +117,8 @@ def insync?(is)
       is == should
     end
   end
+  # rubocop:enable Metrics/CyclomaticComplexity
+  # rubocop:enable Metrics/PerceivedComplexity
   newproperty(:environment) do
     desc 'Environment for this group'
     defaultto :production
diff --git a/spec/unit/puppet/type/node_group_spec.rb b/spec/unit/puppet/type/node_group_spec.rb
index 486cb2a..90c17ad 100644
--- a/spec/unit/puppet/type/node_group_spec.rb
+++ b/spec/unit/puppet/type/node_group_spec.rb
@@ -1,5 +1,6 @@
 require 'spec_helper'
 
+# rubocop:disable Metrics/BlockLength
 describe Puppet::Type.type(:node_group) do
   it 'allows agent-specified environment' do
     expect {
@@ -73,83 +74,296 @@
   end
 
   describe 'purge_behavior' do
-    let(:resource_hash) do
-      {
-        name: 'test_group',
-        environment: 'test_env',
-        data: {
-          'data::class1' => { 'param1' => 'resource',
-                              'param2' => 'resource' },
-          'data::class2' => { 'param1' => 'resource',
-                              'param2' => 'resource' },
-        },
-        classes: {
-          'classes::class1' => { 'param1' => 'resource',
-                                 'param2' => 'resource' },
-        },
-      }
-    end
-
-    let(:existing_data) do
-      { 'data::class1' => { 'param1' => 'existing',
-                            'param3' => 'existing' },
-        'data::class3' => { 'param1' => 'existing',
-                            'param2' => 'existing' } }
+    let(:and_rule) do
+      ['and',
+       ['~', ['fact', 'fact1'], 'value1'],
+       ['~', ['fact', 'fact2'], 'value2']]
     end
-    let(:merged_data) do
-      { 'data::class1' => { 'param1' => 'resource',
-                            'param2' => 'resource',
-                            'param3' => 'existing' },
-        'data::class2' => { 'param1' => 'resource',
-                            'param2' => 'resource' },
-        'data::class3' => { 'param1' => 'existing',
-                            'param2' => 'existing' } }
+    let(:or_rule) do
+      ['or',
+       ['~', ['fact', 'fact1'], 'value1'],
+       ['~', ['fact', 'fact2'], 'value2']]
     end
 
-    let(:existing_classes) do
-      { 'classes::class1' => { 'param1' => 'existing',
-                               'param3' => 'existing' },
-        'classes::class3' => { 'param1' => 'existing',
-                               'param2' => 'existing' } }
-    end
-    let(:merged_classes) do
-      { 'classes::class1' => { 'param1' => 'resource',
-                               'param2' => 'resource',
-                               'param3' => 'existing' },
-        'classes::class3' => { 'param1' => 'existing',
-                               'param2' => 'existing' } }
-    end
+    describe 'group_with_rule' do
+      let(:group_with_rule) do
+        {
+          name: 'test_group',
+          environment: 'test_env',
+          classes: {
+            'classes::class1' => { 'param1' => 'resource',
+                                 'param2' => 'resource',
+                                 'param3' => 'existing' },
+            'classes::class3' => { 'param1' => 'existing',
+                                  'param2' => 'existing' }
+          },
+          rule:
+           ['or',
+            ['~', ['fact', 'fact1'], 'value1'],
+            ['~', ['fact', 'fact2'], 'value2']]
+        }
+      end
+      let(:group_with_merged_and_rule) do
+        ['or',
+         ['and',
+          ['~', ['fact', 'fact1'], 'value1'],
+          ['~', ['fact', 'fact2'], 'value2']],
+         ['~', ['fact', 'fact1'], 'value1'],
+         ['~', ['fact', 'fact2'], 'value2']]
+      end
 
-    it 'matches classes and data exactly by default' do
-      rsrc = described_class.new(resource_hash)
-      allow(rsrc.property(:data)).to receive(:retrieve).and_return(existing_data)
-      allow(rsrc.property(:classes)).to receive(:retrieve).and_return(existing_classes)
-      expect(rsrc.property(:data).should).to eq resource_hash[:data]
-      expect(rsrc.property(:classes).should).to eq resource_hash[:classes]
+      it 'replaces by default' do
+        rsrc = described_class.new(group_with_rule)
+        # this is simulated data from the classifier service
+        allow(rsrc.property(:rule)).to receive(:retrieve).and_return(and_rule)
+        expect(rsrc.property(:rule).should).to eq group_with_rule[:rule]
+      end
+
+      it 'merges when purge behaviour set to :none' do
+        rsrc = described_class.new(group_with_rule.merge(purge_behavior: 'none'))
+        allow(rsrc.property(:rule)).to receive(:retrieve).and_return(and_rule)
+        expect(rsrc.property(:rule).should).to eq group_with_merged_and_rule
+      end
+
+      it 'replaces rule when purge behaviour set to :all' do
+        rsrc = described_class.new(group_with_rule.merge(purge_behavior: 'all'))
+        allow(rsrc.property(:rule)).to receive(:retrieve).and_return(and_rule)
+        expect(rsrc.property(:rule).should).to eq group_with_rule[:rule]
+      end
+
+      it 'updates rule when purge behaviour set to :rule' do
+        rsrc = described_class.new(group_with_rule.merge(purge_behavior: 'rule'))
+        allow(rsrc.property(:rule)).to receive(:retrieve).and_return(and_rule)
+        expect(rsrc.property(:rule).should).to eq group_with_rule[:rule]
+      end
     end
 
-    it 'merges in classes and data when set to :none' do
-      rsrc = described_class.new(resource_hash.merge(purge_behavior: 'none'))
-      allow(rsrc.property(:data)).to receive(:retrieve).and_return(existing_data)
-      allow(rsrc.property(:classes)).to receive(:retrieve).and_return(existing_classes)
-      expect(rsrc.property(:data).should).to eq(merged_data)
-      expect(rsrc.property(:classes).should).to eq(merged_classes)
+    describe 'group with pinned nodes' do
+      let(:group_with_pinned_nodes) do
+        {
+          name: 'test_group',
+          environment: 'test_env',
+          classes: {
+            'classes::class1' => { 'param1' => 'resource',
+                                 'param2' => 'resource',
+                                 'param3' => 'existing' },
+            'classes::class3' => { 'param1' => 'existing',
+                                  'param2' => 'existing' }
+          },
+          rule:
+           ['or',
+            ['=', 'name', 'value1'],
+            ['=', 'name', 'value2'],
+            ['=', 'name', 'value2']]
+        }
+      end
+
+      # when merging the pinned nodes and the and rule, the top level "or" clause should be maintained, and the pinned nodes added
+      # to the end
+      # also the merging optimizes the pinned nodes to remove redundants.
+      let(:merged_pinned_nodes_and_rule) do
+        ['or',
+         ['and',
+          ['~', ['fact', 'fact1'], 'value1'],
+          ['~', ['fact', 'fact2'], 'value2']],
+         ['=', 'name', 'value1'],
+         ['=', 'name', 'value2']]
+      end
+
+      let(:merged_pinned_nodes_or_rule) do
+        ['or',
+         ['~', ['fact', 'fact1'], 'value1'],
+         ['~', ['fact', 'fact2'], 'value2'],
+         ['=', 'name', 'value1'],
+         ['=', 'name', 'value2']]
+      end
+
+      it 'matches rule exactly by default' do
+        rsrc = described_class.new(group_with_pinned_nodes)
+        allow(rsrc.property(:rule)).to receive(:retrieve).and_return(and_rule)
+        expect(rsrc.property(:rule).should).to eq group_with_pinned_nodes[:rule]
+      end
+
+      it 'merges and rule correctly when purge behaviour set to :none' do
+        rsrc = described_class.new(group_with_pinned_nodes.merge(purge_behavior: 'none'))
+        allow(rsrc.property(:rule)).to receive(:retrieve).and_return(and_rule)
+        expect(rsrc.property(:rule).should).to eq merged_pinned_nodes_and_rule
+      end
+
+      it 'merges or rule correctly when purge behaviour set to :none' do
+        rsrc = described_class.new(group_with_pinned_nodes.merge(purge_behavior: 'none'))
+        allow(rsrc.property(:rule)).to receive(:retrieve).and_return(or_rule)
+        expect(rsrc.property(:rule).should).to eq merged_pinned_nodes_or_rule
+      end
+
+      it 'merges pinned node correctly when purge behaviour set to :none' do
+        rsrc = described_class.new(group_with_pinned_nodes.merge(purge_behavior: 'none'))
+        allow(rsrc.property(:rule)).to receive(:retrieve).and_return(['=', 'name', 'value2'])
+        # redundant node pinning is removed
+        expect(rsrc.property(:rule).should).to eq ['or', ['=', 'name', 'value2'], ['=', 'name', 'value1']]
+      end
+
+      it 'replaces rule when purge behaviour set to :all' do
+        rsrc = described_class.new(group_with_pinned_nodes.merge(purge_behavior: 'all'))
+        allow(rsrc.property(:rule)).to receive(:retrieve).and_return(and_rule)
+        expect(rsrc.property(:rule).should).to eq group_with_pinned_nodes[:rule]
+      end
+
+      it 'replaces rule when purge behaviour set to :rule' do
+        rsrc = described_class.new(group_with_pinned_nodes.merge(purge_behavior: 'rule'))
+        allow(rsrc.property(:rule)).to receive(:retrieve).and_return(and_rule)
+        expect(rsrc.property(:rule).should).to eq group_with_pinned_nodes[:rule]
+      end
     end
 
-    it 'merges in classes and match data exactly when set to :data' do
-      rsrc = described_class.new(resource_hash.merge(purge_behavior: 'data'))
-      allow(rsrc.property(:data)).to receive(:retrieve).and_return(existing_data)
-      allow(rsrc.property(:classes)).to receive(:retrieve).and_return(existing_classes)
-      expect(rsrc.property(:data).should).to eq(resource_hash[:data])
-      expect(rsrc.property(:classes).should).to eq(merged_classes)
+    describe 'group with top-level and clause' do
+      let(:group_with_and_clause) do
+        {
+          name: 'test_group',
+          environment: 'test_env',
+          classes: {
+            'classes::class1' => { 'param1' => 'resource',
+                                 'param2' => 'resource',
+                                 'param3' => 'existing' },
+            'classes::class3' => { 'param1' => 'existing',
+                                  'param2' => 'existing' }
+          },
+          rule:
+           ['and', ['~', ['fact', 'pe_server_version'], '.+']]
+        }
+      end
+
+      let(:rule_combined_and_clauses) do
+        ['and',
+         ['~', ['fact', 'fact1'], 'value1'],
+         ['~', ['fact', 'fact2'], 'value2'],
+         ['~', ['fact', 'pe_server_version'], '.+']]
+      end
+
+      let(:rule_combined_or_clauses) do
+        ['or',
+         ['~', ['fact', 'fact1'], 'value1'],
+         ['~', ['fact', 'fact2'], 'value2'],
+         ['and', ['~', ['fact', 'pe_server_version'], '.+']]]
+      end
+
+      it 'replaces rule exactly by default' do
+        rsrc = described_class.new(group_with_and_clause)
+        allow(rsrc.property(:rule)).to receive(:retrieve).and_return(and_rule)
+        expect(rsrc.property(:rule).should).to eq group_with_and_clause[:rule]
+      end
+
+      it 'merges and rule correctly when purge behaviour set to :none' do
+        rsrc = described_class.new(group_with_and_clause.merge(purge_behavior: 'none'))
+        allow(rsrc.property(:rule)).to receive(:retrieve).and_return(and_rule)
+        expect(rsrc.property(:rule).should).to eq rule_combined_and_clauses
+      end
+
+      it 'merges or rule correctly when purge behaviour set to :none' do
+        rsrc = described_class.new(group_with_and_clause.merge(purge_behavior: 'none'))
+        allow(rsrc.property(:rule)).to receive(:retrieve).and_return(or_rule)
+        expect(rsrc.property(:rule).should).to eq rule_combined_or_clauses
+      end
+
+      it 'merges pinned node correctly when purge behaviour set to :none' do
+        rsrc = described_class.new(group_with_and_clause.merge(purge_behavior: 'none'))
+        allow(rsrc.property(:rule)).to receive(:retrieve).and_return(['=', 'name', 'value2'])
+        # redundant node pinning is removed
+        expect(rsrc.property(:rule).should).to eq ['or', ['=', 'name', 'value2'], ['and', ['~', ['fact', 'pe_server_version'], '.+']]]
+      end
+
+      it 'replaces rule when purge behaviour set to :all' do
+        rsrc = described_class.new(group_with_and_clause.merge(purge_behavior: 'all'))
+        allow(rsrc.property(:rule)).to receive(:retrieve).and_return(and_rule)
+        expect(rsrc.property(:rule).should).to eq group_with_and_clause[:rule]
+      end
+
+      it 'replaces rule when purge behaviour set to :rule' do
+        rsrc = described_class.new(group_with_and_clause.merge(purge_behavior: 'rule'))
+        allow(rsrc.property(:rule)).to receive(:retrieve).and_return(and_rule)
+        expect(rsrc.property(:rule).should).to eq group_with_and_clause[:rule]
+      end
     end
 
-    it 'merges in data and match classes exactly when set to :classes' do
-      rsrc = described_class.new(resource_hash.merge(purge_behavior: 'classes'))
-      allow(rsrc.property(:data)).to receive(:retrieve).and_return(existing_data)
-      allow(rsrc.property(:classes)).to receive(:retrieve).and_return(existing_classes)
-      expect(rsrc.property(:data).should).to eq(merged_data)
-      expect(rsrc.property(:classes).should).to eq(resource_hash[:classes])
+    describe 'resource hash' do
+      let(:resource_hash) do
+        {
+          name: 'test_group',
+          environment: 'test_env',
+          data: {
+            'data::class1' => { 'param1' => 'resource',
+                                'param2' => 'resource' },
+            'data::class2' => { 'param1' => 'resource',
+                                'param2' => 'resource' },
+          },
+          classes: {
+            'classes::class1' => { 'param1' => 'resource',
+                                   'param2' => 'resource' },
+          },
+        }
+      end
+
+      let(:existing_data) do
+        { 'data::class1' => { 'param1' => 'existing',
+                              'param3' => 'existing' },
+          'data::class3' => { 'param1' => 'existing',
+                              'param2' => 'existing' } }
+      end
+      let(:merged_data) do
+        { 'data::class1' => { 'param1' => 'resource',
+                              'param2' => 'resource',
+                              'param3' => 'existing' },
+          'data::class2' => { 'param1' => 'resource',
+                              'param2' => 'resource' },
+          'data::class3' => { 'param1' => 'existing',
+                              'param2' => 'existing' } }
+      end
+
+      let(:existing_classes) do
+        { 'classes::class1' => { 'param1' => 'existing',
+                                 'param3' => 'existing' },
+          'classes::class3' => { 'param1' => 'existing',
+                                 'param2' => 'existing' } }
+      end
+      let(:merged_classes) do
+        { 'classes::class1' => { 'param1' => 'resource',
+                                 'param2' => 'resource',
+                                 'param3' => 'existing' },
+          'classes::class3' => { 'param1' => 'existing',
+                                 'param2' => 'existing' } }
+      end
+
+      it 'matches classes and data exactly by default' do
+        rsrc = described_class.new(resource_hash)
+        allow(rsrc.property(:data)).to receive(:retrieve).and_return(existing_data)
+        allow(rsrc.property(:classes)).to receive(:retrieve).and_return(existing_classes)
+        expect(rsrc.property(:data).should).to eq resource_hash[:data]
+        expect(rsrc.property(:classes).should).to eq resource_hash[:classes]
+      end
+
+      it 'merges in classes and data when set to :none' do
+        rsrc = described_class.new(resource_hash.merge(purge_behavior: 'none'))
+        allow(rsrc.property(:data)).to receive(:retrieve).and_return(existing_data)
+        allow(rsrc.property(:classes)).to receive(:retrieve).and_return(existing_classes)
+        expect(rsrc.property(:data).should).to eq(merged_data)
+        expect(rsrc.property(:classes).should).to eq(merged_classes)
+      end
+
+      it 'merges in classes and match data exactly when set to :data' do
+        rsrc = described_class.new(resource_hash.merge(purge_behavior: 'data'))
+        allow(rsrc.property(:data)).to receive(:retrieve).and_return(existing_data)
+        allow(rsrc.property(:classes)).to receive(:retrieve).and_return(existing_classes)
+        expect(rsrc.property(:data).should).to eq(resource_hash[:data])
+        expect(rsrc.property(:classes).should).to eq(merged_classes)
+      end
+
+      it 'merges in data and match classes exactly when set to :classes' do
+        rsrc = described_class.new(resource_hash.merge(purge_behavior: 'classes'))
+        allow(rsrc.property(:data)).to receive(:retrieve).and_return(existing_data)
+        allow(rsrc.property(:classes)).to receive(:retrieve).and_return(existing_classes)
+        expect(rsrc.property(:data).should).to eq(merged_data)
+        expect(rsrc.property(:classes).should).to eq(resource_hash[:classes])
+      end
     end
   end
 
@@ -196,3 +410,4 @@
     end
   end
 end
+# rubocop:enable Metrics/BlockLength