Refactor requests to return simple hashes and add unit tests

This massive commit refactors all of the request methods on the
Fog::Compute[:vsphere] instance to return simple hashes.  The behavior
before this commit returned full vmware object references which was a
problem because it was difficult to unit test.

With this patch, it is much easier to add and maintain Mock
implementations of the request methods.  This makes adding behavior
tests for the server model much easier.

In addition, test coverage using Shindo has been added.  Previously
there was little test coverage of the behavior.

To run the tests:

    shindont tests/vsphere/

Author: Jeff McCune

lib/fog/vsphere/compute.rb

@@ -14,36 +14,73 @@ class Vsphere < Fog::Service
 
       request_path 'fog/vsphere/requests/compute'
       request :current_time
+      request :find_vm_by_ref
       request :list_virtual_machines
       request :vm_power_off
       request :vm_power_on
       request :vm_reboot
       request :vm_clone
       request :vm_destroy
-      request :find_all_by_uuid
-      request :find_all_by_instance_uuid
-      request :find_template_by_instance_uuid
-      request :find_vm_by_ref
+
+      module Shared
+
+        attr_reader :vsphere_is_vcenter
+        attr_reader :vsphere_rev
+
+        # Utility method to convert a VMware managed object into an attribute hash.
+        # This should only really be necessary for the real class.
+        # This method is expected to be called by the request methods
+        # in order to massage VMware Managed Object References into Attribute Hashes.
+        def convert_vm_mob_ref_to_attr_hash(vm_mob_ref)
+          return nil unless vm_mob_ref
+          # A cloning VM doesn't have a configuration yet.  Unfortuantely we just get
+          # a RunTime exception.
+          begin
+            is_ready = vm_mob_ref.config ? true : false
+          rescue RuntimeError
+            is_ready = nil
+          end
+          {
+            'id'               => is_ready ? vm_mob_ref.config.instanceUuid : vm_mob_ref._ref,
+            'mo_ref'           => vm_mob_ref._ref,
+            'name'             => vm_mob_ref.name,
+            'uuid'             => is_ready ? vm_mob_ref.config.uuid : nil,
+            'instance_uuid'    => is_ready ? vm_mob_ref.config.instanceUuid : nil,
+            'hostname'         => vm_mob_ref.summary.guest.hostName,
+            'operatingsystem'  => vm_mob_ref.summary.guest.guestFullName,
+            'ipaddress'        => vm_mob_ref.summary.guest.ipAddress,
+            'power_state'      => vm_mob_ref.runtime.powerState,
+            'connection_state' => vm_mob_ref.runtime.connectionState,
+            'hypervisor'       => vm_mob_ref.runtime.host ? vm_mob_ref.runtime.host.name : nil,
+            'tools_state'      => vm_mob_ref.summary.guest.toolsStatus,
+            'tools_version'    => vm_mob_ref.summary.guest.toolsVersionStatus,
+            'mac_addresses'    => is_ready ? vm_mob_ref.macs : nil,
+            'is_a_template'    => is_ready ? vm_mob_ref.config.template : nil
+          }
+        end
+
+      end
 
       class Mock
 
+        include Shared
+
         def initialize(options={})
           require 'mocha'
           @vsphere_username = options[:vsphere_username]
-          @vsphere_password = options[:vsphere_password]
+          @vsphere_password = 'REDACTED'
           @vsphere_server   = options[:vsphere_server]
           @vsphere_expected_pubkey_hash = options[:vsphere_expected_pubkey_hash]
+          @vsphere_is_vcenter = true
+          @vsphere_rev = '4.0'
           @connection = Mocha::Mock.new
-          # This may, or may not, be a terrible idea:
-          @connection.stub_everything
         end
 
       end
 
       class Real
 
-        attr_reader :vsphere_is_vcenter
-        attr_reader :vsphere_rev
+        include Shared
 
         def initialize(options={})
           require 'rbvmomi'
@@ -92,14 +129,6 @@ def initialize(options={})
           authenticate
         end
 
-        def reload
-          raise NotImplementedError
-        end
-
-        def request
-          raise NotImplementedError
-        end
-
         private
 
         def authenticate

lib/fog/vsphere/models/compute/server.rb

@@ -31,56 +31,29 @@ class Server < Fog::Compute::Server
         attribute :mac_addresses, :aliases => 'macs'
         attribute :hypervisor,    :aliases => 'host'
         attribute :is_a_template
+        attribute :connection_state
+        attribute :mo_ref
 
-        # Return an attribute hash suitable for the initializer given a
-        # vSphere Managed Object (mob) instance.
-        def self.attribute_hash_from_mob(vm)
-          return {} unless vm
-          # A cloning VM doesn't have a configuration yet.  Unfortuantely we just get
-          # a RunTime exception.
-          begin
-            is_ready = vm.config ? true : false
-          rescue RuntimeError
-            is_ready = nil
-          end
-          {
-            :id               => is_ready ? vm.config.instanceUuid : vm._ref,
-            :name             => vm.name,
-            :uuid             => is_ready ? vm.config.uuid : nil,
-            :instance_uuid    => is_ready ? vm.config.instanceUuid : nil,
-            :hostname         => vm.summary.guest.hostName,
-            :operatingsystem  => vm.summary.guest.guestFullName,
-            :ipaddress        => vm.summary.guest.ipAddress,
-            :power_state      => vm.runtime.powerState,
-            :connection_state => vm.runtime.connectionState,
-            :hypervisor       => vm.runtime.host ? vm.runtime.host.name : nil,
-            :tools_state      => vm.summary.guest.toolsStatus,
-            :tools_version    => vm.summary.guest.toolsVersionStatus,
-            :mac_addresses    => is_ready ? vm.macs : nil,
-            :is_a_template    => is_ready ? vm.config.template : nil
-          }
-        end
-
-        def start
+        def start(options = {})
           requires :instance_uuid
-          connection.vm_power_on(:instance_uuid => instance_uuid)
+          connection.vm_power_on('instance_uuid' => instance_uuid)
         end
 
-        def stop(params = {})
-          params = { :force => false }.merge(params)
+        def stop(options = {})
+          options = { :force => false }.merge(options)
           requires :instance_uuid
-          connection.vm_power_off(:instance_uuid => instance_uuid, :force => params[:force])
+          connection.vm_power_off('instance_uuid' => instance_uuid, 'force' => options[:force])
         end
 
-        def reboot(params = {})
-          params = { :force => false }.merge(params)
+        def reboot(options = {})
+          options = { :force => false }.merge(options)
           requires :instance_uuid
-          connection.vm_reboot(:instance_uuid => instance_uuid, :force => params[:force])
+          connection.vm_reboot('instance_uuid' => instance_uuid, 'force' => options[:force])
         end
 
-        def destroy(params = {})
+        def destroy(options = {})
           requires :instance_uuid
-          connection.vm_destroy(:instance_uuid => instance_uuid)
+          connection.vm_destroy('instance_uuid' => instance_uuid)
         end
 
       end

lib/fog/vsphere/models/compute/servers.rb

@@ -10,27 +10,22 @@ class Servers < Fog::Collection
         model Fog::Compute::Vsphere::Server
 
         def all
-          # Virtual Machine Managed Objects (vm_mobs)
-          vm_mobs = connection.list_virtual_machines
-          vm_attributes = vm_mobs.collect do |vm_mob|
-            model.attribute_hash_from_mob(vm_mob)
-          end
-
-          load(vm_attributes)
+          response = connection.list_virtual_machines
+          load(response['virtual_machines'])
         end
 
         def get(id)
           # Is the id a managed_object_reference?  This may be the case if we're reloading
           # a model of a VM in the process of being cloned, since it
           # will not have a instance_uuid yet.
           if id =~ /^vm-/
-            vm_mob = connection.find_vm_by_ref(:vm_ref => id)
+            response = connection.find_vm_by_ref('vm_ref' => id)
+            server_attributes = response['virtual_machine']
           else
-            vm_mob = connection.find_all_by_instance_uuid(id).first
-          end
-          if server_attributes = model.attribute_hash_from_mob(vm_mob)
-            new(server_attributes)
+            response = connection.list_virtual_machines('instance_uuid' => id)
+            server_attributes = response['virtual_machines'].first
           end
+          new(server_attributes)
         rescue Fog::Compute::Vsphere::NotFound
           nil
         end

lib/fog/vsphere/requests/compute/current_time.rb

@@ -4,15 +4,16 @@ class Vsphere
       class Real
 
         def current_time
-          @connection.serviceInstance.CurrentTime
+          current_time = @connection.serviceInstance.CurrentTime
+          { 'current_time' => current_time }
         end
 
       end
 
       class Mock
 
         def current_time
-          Time.now.utc
+          { 'current_time' => Time.now.utc }
         end
 
       end

lib/fog/vsphere/requests/compute/find_all_by_instance_uuid.rb

@@ -1,28 +1,41 @@
 module Fog
   module Compute
     class Vsphere
-      class Real
+
+      module Shared
 
         # REVISIT:  This is a naive implementation and not very efficient since
         # we find ALL VM's and then iterate over them looking for the managed object
         # reference id...  There should be an easier way to obtain a reference to a
         # VM using only the name or the _ref.  This request is primarily intended to
         # reload the attributes of a cloning VM which does not yet have an instance_uuid
-        def find_vm_by_ref(params = {})
-          list_virtual_machines.detect(lambda { raise Fog::Vsphere::Errors::NotFound }) do |vm|
-            vm._ref == params[:vm_ref]
+        def find_vm_by_ref(options = {})
+          raise ArgumentError, "Must pass a vm_ref option" unless options['vm_ref']
+
+          # This is the inefficient call
+          all_vm_attributes = list_virtual_machines['virtual_machines']
+          # Find the VM attributes of the reference
+          if vm_attributes = all_vm_attributes.find { |vm| vm['mo_ref'] == options['vm_ref'] }
+            response = { 'virtual_machine' => vm_attributes }
+          else
+            raise Fog::Compute::Vsphere::NotFound, "VirtualMachine with Managed Object Reference #{options['vm_ref']} could not be found."
           end
+          response
         end
 
       end
 
-      class Mock
+      # The Real and Mock classes share the same method
+      # because list_virtual_machines will be properly mocked for us
 
-        def find_vm_by_ref(params = {})
-          Fog::Mock.not_implmented
-        end
+      class Real
+        include Shared
+      end
 
+      class Mock
+        include Shared
       end
+
     end
   end
 end