clean up updating_home?lcg/cleanup-user

Signed-off-by: Lamont Granquist <lamont@scriptkiddie.org>

5 files changed, 16 insertions, 26 deletions

diff --git a/lib/chef/provider/user.rb b/lib/chef/provider/user.rb
index b27839bbcf..7145045785 100644
--- a/lib/chef/provider/user.rb
+++ b/lib/chef/provider/user.rb
@@ -200,10 +200,22 @@ class Chef
         raise NotImplementedError
       end
 
-      # helper used by e.g. linux, aix, solaris providers
+      private
+
+      #
+      # helpers for subclasses
+      #
+
       def should_set?(sym)
         current_resource.send(sym).to_s != new_resource.send(sym).to_s && new_resource.send(sym)
       end
+
+      def updating_home?
+        return false if new_resource.home.nil?
+        return true if current_resource.home.nil?
+        # Pathname#cleanpath matches more edge conditions than File.expand_path()
+        new_resource.home && Pathname.new(current_resource.home).cleanpath != Pathname.new(new_resource.home).cleanpath
+      end
     end
   end
 end
diff --git a/lib/chef/provider/user/aix.rb b/lib/chef/provider/user/aix.rb
index e89e56d90b..be6ff9d750 100644
--- a/lib/chef/provider/user/aix.rb
+++ b/lib/chef/provider/user/aix.rb
@@ -103,12 +103,6 @@ class Chef
           opts
         end
 
-        # FIXME: move to superclass, and one of these implements must be buggy?
-        def updating_home?
-          return true if current_resource.home.nil? && new_resource.home
-          new_resource.home && Pathname.new(current_resource.home).cleanpath != Pathname.new(new_resource.home).cleanpath
-        end
-
         private
 
         def add_password
diff --git a/lib/chef/provider/user/linux.rb b/lib/chef/provider/user/linux.rb
index 3aac717242..a846d2657a 100644
--- a/lib/chef/provider/user/linux.rb
+++ b/lib/chef/provider/user/linux.rb
@@ -85,13 +85,6 @@ class Chef
           opts
         end
 
-        # FIXME: move to superclass, and one of these implementations must be buggy?
-        def updating_home?
-          return false unless new_resource.home
-          return true unless current_resource.home
-          new_resource.home && Pathname.new(current_resource.home).cleanpath != Pathname.new(new_resource.home).cleanpath
-        end
-
         def check_lock
           # there's an old bug in rhel (https://bugzilla.redhat.com/show_bug.cgi?id=578534)
           # which means that both 0 and 1 can be success.
diff --git a/lib/chef/provider/user/solaris.rb b/lib/chef/provider/user/solaris.rb
index a635899505..1abe660cfd 100644
--- a/lib/chef/provider/user/solaris.rb
+++ b/lib/chef/provider/user/solaris.rb
@@ -110,15 +110,6 @@ class Chef
           opts
         end
 
-        def updating_home?
-          # will return false if paths are equivalent
-          # Pathname#cleanpath does a better job than ::File::expand_path (on both unix and windows)
-          # ::File.expand_path("///tmp") == ::File.expand_path("/tmp") => false
-          # ::File.expand_path("\\tmp") => "C:/tmp"
-          return true if current_resource.home.nil? && new_resource.home
-          new_resource.home && Pathname.new(current_resource.home).cleanpath != Pathname.new(new_resource.home).cleanpath
-        end
-
         def manage_password
           return unless current_resource.password != new_resource.password && new_resource.password
           logger.trace("#{new_resource} setting password to #{new_resource.password}")
diff --git a/spec/support/shared/unit/provider/useradd_based_user_provider.rb b/spec/support/shared/unit/provider/useradd_based_user_provider.rb
index cc2d22f64f..e123e8e0e2 100644
--- a/spec/support/shared/unit/provider/useradd_based_user_provider.rb
+++ b/spec/support/shared/unit/provider/useradd_based_user_provider.rb
@@ -1,7 +1,7 @@
 #
 # Author:: Adam Jacob (<adam@chef.io>)
 # Author:: Daniel DeLeo (<dan@chef.io>)
-# Copyright:: Copyright 2008-2016, Chef Software Inc.
+# Copyright:: Copyright 2008-2018, Chef Software Inc.
 #
 # License:: Apache License, Version 2.0
 #
@@ -385,7 +385,7 @@ shared_examples_for "a useradd-based user provider" do |supported_useradd_option
         expect(Pathname).to receive(:new).with(@new_resource.home).and_return(@new_home_mock)
         expect(@new_home_mock).to receive(:cleanpath).and_return(home_check["new_resource_home"].last)
 
-        expect(provider.updating_home?).to eq(home_check["expected_result"])
+        expect(provider.send(:updating_home?)).to eq(home_check["expected_result"])
       end
     end
     it "should return true if the current home does not exist but a home is specified by the new resource" do
@@ -396,7 +396,7 @@ shared_examples_for "a useradd-based user provider" do |supported_useradd_option
       @current_resource.home nil
       @new_resource.home "/home/kitten"
 
-      expect(provider.updating_home?).to eq(true)
+      expect(provider.send(:updating_home?)).to eq(true)
     end
   end
 end