Add the ability to update properties on printers

This follows the same pattern used in windows_printer_port

Signed-off-by: Tim Smith <tsmith@chef.io>

2 files changed, 62 insertions, 36 deletions

diff --git a/kitchen-tests/cookbooks/end_to_end/recipes/_windows_printer.rb b/kitchen-tests/cookbooks/end_to_end/recipes/_windows_printer.rb
index 64a05c05c9..e860860309 100644
--- a/kitchen-tests/cookbooks/end_to_end/recipes/_windows_printer.rb
+++ b/kitchen-tests/cookbooks/end_to_end/recipes/_windows_printer.rb
@@ -36,3 +36,14 @@ windows_printer "HP LaserJet 5th Floor" do
   port_name "My awesome port"
   create_port false
 end
+
+# make sure we can update a printer
+windows_printer "Update the HP LaserJet 5th Floor printer" do
+  device_id "HP LaserJet 5th Floor"
+  ipv4_address "10.4.64.41"
+  driver_name "Generic / Text Only"
+  port_name "My awesome port"
+  comment "This is a comment on a printer"
+  default true
+  create_port false
+end
diff --git a/lib/chef/resource/windows_printer.rb b/lib/chef/resource/windows_printer.rb
index 24d746b648..676e6544ef 100644
--- a/lib/chef/resource/windows_printer.rb
+++ b/lib/chef/resource/windows_printer.rb
@@ -1,5 +1,6 @@
 #
 # Author:: Doug Ireton (<doug@1strategy.com>)
+# Author:: Tim Smith (<tsmith@chef.io>)
 # Copyright:: 2012-2018, Nordstrom, Inc.
 # Copyright:: Chef Software, Inc.
 #
@@ -23,7 +24,6 @@ require_relative "../resource"
 class Chef
   class Resource
     # @todo
-    # 1. Allow updating the printer properties
     # 2. Fail with a warning if the port can't be found and create_port is false
     # 3. Fail with helpful messaging if the printer driver can't be installed
     class WindowsPrinter < Chef::Resource
@@ -33,7 +33,7 @@ class Chef
 
       provides(:windows_printer) { true }
 
-      description "Use the **windows_printer** resource to setup Windows printers. This resource will automatically install the driver specified in the `driver_name` property and will automatically create a printer port using either the `ipv4_address` property or the `port_name property."
+      description "Use the **windows_printer** resource to setup Windows printers. This resource will automatically install the driver specified in the `driver_name` property and will automatically create a printer port using either the `ipv4_address` property or the `port_name` property."
       introduced "14.0"
       examples <<~DOC
       **Create a printer**:
@@ -116,62 +116,62 @@ class Chef
         introduced: "17.3",
         default_description: "The resource block name or the ipv4_address prepended with IP_."
 
+      def ps_bool_to_ruby(bool)
+        bool.to_s.downcase == "true" ? true : false
+      end
+
       load_current_value do |new_resource|
-        printer_data = powershell_exec(%Q{Get-WmiObject -Class Win32_Printer -Filter "Name='#{new_resource.device_id}'"}).result
+        # this is why so we can fetch the default printer on the system
+        printer_data = powershell_exec(%Q{Get-WmiObject -Class Win32_Printer -Filter "DeviceID='#{new_resource.device_id}'"}).result
 
         if printer_data.empty?
           current_value_does_not_exist!
         else
           device_id new_resource.device_id
           comment printer_data["Comment"]
-          default printer_data["Default"]
+          default ps_bool_to_ruby(printer_data["Default"])
           location printer_data["Location"]
-          shared printer_data["Shared"]
+          shared ps_bool_to_ruby(printer_data["Shared"])
           share_name printer_data["ShareName"]
           port_name printer_data["PortName"]
 
           driver_data = powershell_exec(%Q{Get-PrinterDriver -Name="#{new_resource.driver_name}"}).result
-          unless driver_data.empty?
-            driver_name new_resource.driver_name
-          end
+          driver_name driver_data.empty? ? "unset" : new_resource.driver_name # we have to set this to a value or required validation fails
         end
       end
 
-      action :create, description: "Create a new printer and printer port, if one doesn't already." do
-        if current_resource
-          Chef::Log.info "#{@new_resource} already exists - nothing to do."
-        else
-          # Create the printer port first unless the property is set to false
-          if new_resource.create_port
-            windows_printer_port new_resource.port_name do
-              ipv4_address new_resource.ipv4_address
-              port_name new_resource.port_name
-            end
+      action :create, description: "Create or update a printer." do
+        # Create the printer port first unless the property is set to false
+        if new_resource.create_port
+          windows_printer_port new_resource.port_name do
+            ipv4_address new_resource.ipv4_address
+            port_name new_resource.port_name
           end
+        end
 
-          converge_by("install driver #{new_resource.driver_name}") do
-            powershell_exec!("Add-PrinterDriver -Name '#{new_resource.driver_name}'")
-          end
+        converge_if_changed(:driver_name) do
+          powershell_exec!("Add-PrinterDriver -Name '#{new_resource.driver_name}'")
+        end
 
-          converge_by("create #{@new_resource.device_id}") do
-            powershell_exec! <<-EOH
-            Set-WmiInstance -class Win32_Printer `
-              -EnableAllPrivileges `
-              -Argument @{ DeviceID   = "#{new_resource.device_id}";
-                          Comment    = "#{new_resource.comment}";
-                          Default    = "$#{new_resource.default}";
-                          DriverName = "#{new_resource.driver_name}";
-                          Location   = "#{new_resource.location}";
-                          PortName   = "#{new_resource.port_name}";
-                          Shared     = "$#{new_resource.shared}";
-                          ShareName  = "#{new_resource.share_name}";
-                        }
-            EOH
+        if current_resource
+          converge_if_changed(:comment, :driver_name, :location, :port_name, :shared, :share_name) do
+            # update the existing printer using PowerShell
+            printer_shellout!("Set")
+          end
+        else
+          converge_if_changed(:comment, :driver_name, :location, :port_name, :shared, :share_name) do
+            # create a whole new printer using PowerShell
+            printer_shellout!("Add")
           end
         end
+
+        # Set-Printer does let you set the printer as the default
+        converge_if_changed(:default) do
+          powershell_exec!("(New-Object -ComObject WScript.Network).SetDefaultPrinter('#{new_resource.device_id}')")
+        end
       end
 
-      action :delete, description: "Delete an existing printer. Note that this resource does not delete the associated printer port." do
+      action :delete, description: "Delete an existing printer. Note that this resource does not delete the associated printer port or remove the driver." do
         if current_resource
           converge_by("Delete #{new_resource.device_id}") do
             powershell_exec!("Remove-Printer -Name '#{new_resource.device_id}'")
@@ -180,6 +180,21 @@ class Chef
           Chef::Log.info "#{new_resource.device_id} doesn't exist - can't delete."
         end
       end
+
+      action_class do
+        def printer_shellout!(action)
+          update_cmd = "#{action}-Printer -Name '#{new_resource.device_id}'"\
+          " -DriverName '#{new_resource.driver_name}'"\
+          " -PortName '#{new_resource.port_name}'"
+
+          update_cmd << " -Shared" if new_resource.shared
+          update_cmd << " -Comment '#{new_resource.comment}'" if new_resource.comment
+          update_cmd << " -Location '#{new_resource.location}'" if new_resource.location
+          update_cmd << " -ShareName '#{new_resource.share_name}" if new_resource.share_name
+
+          powershell_exec!(update_cmd)
+        end
+      end
     end
   end
 end