Moving #create_server logic from request to model

The attribute #template_name doesn't exist anymore since it value was
making the save method to have 2 different behaviors.

Now, to create a vm from scratch, you can simple call save on a new vm record.
And to create from a template, it is necessary to call all the methods (clone, provision, start)
on the template object. This change make it a bit more verbose but gives more control
over the flow, like use a cdrom as install-repository without the need to have another template.

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

@@ -65,7 +65,6 @@ class Server < Fog::Compute::Server
           attribute :snapshot_time
           attribute :start_delay
           attribute :tags
-          attribute :template_name
           attribute :transportable_snapshot_id
           attribute :user_version,                                                    :default => '0'
           attribute :uuid
@@ -146,6 +145,16 @@ def clean_shutdown
             true
           end
 
+          def can_be_cloned?
+            allowed_operations.include?('clone')
+          end
+
+          def clone(name)
+            raise 'Clone Operation not Allowed' unless can_be_cloned?
+            reference = service.clone_vm(name, reference)
+            reload
+          end
+
           def revert(snapshot_ref)
             Fog::Logger.deprecation 'This method is DEPRECATED. Use #snapshot_revert instead.'
             snapshot_revert(snapshot_ref)

lib/fog/compute/xen_server/requests/clone_server.rb

@@ -6,6 +6,8 @@ def clone_server(server_name, template_ref)
           @connection.request({ :parser => Fog::Parsers::XenServer::Base.new, :method => 'VM.clone' },
                               template_ref, server_name)
         end
+
+        alias_method :clone_vm, :clone_server
       end
     end
   end

lib/fog/compute/xen_server/requests/create_server.rb

@@ -2,49 +2,12 @@ module Fog
   module Compute
     class XenServer
       class Real
-        def create_server_raw(config = {})
-          @connection.request({:parser => Fog::Parsers::XenServer::Base.new, :method => 'VM.create' }, config)
-        end
-
         def create_server(config = {}, extra_params = {})
-          networks = extra_params.fetch(:networks, [])
-          auto_start = extra_params.fetch(:auto_start, true)
-          name_label = config.fetch(:name, '')
-          template = config.fetch(:template_name, nil)
-
-          return create_server_raw(config) if template.nil?
-
-          raise "Invalid networks argument" unless networks.kind_of? Array
-
-          if template.kind_of? String
-            template_string = template
-            # try template by UUID
-            template = servers.templates.find { |s| s.uuid == template_string }
-            if template.nil?
-              # Try with the template name just in case
-              template = servers.get get_by_name(template_string, 'VM')
-            end
-          end
-
-          raise "Template #{template_string} does not exist" if template.allowed_operations.nil?
-          raise 'Clone Operation not Allowed' unless template.allowed_operations.include?('clone')
-
-          # Clone the VM template
-          ref = clone_server name_label, template.reference
-          # Add additional NICs
-          networks.each do |n|
-            create_vif ref, n.reference
-          end
-          if auto_start
-            @connection.request({:parser => Fog::Parsers::XenServer::Base.new, :method => 'VM.provision'}, ref)
-            start_vm( ref )
-          end
-
-          ref
+          @connection.request({:parser => Fog::Parsers::XenServer::Base.new, :method => 'VM.create' }, config)
         end
 
         alias_method :create_vm, :create_server
       end
     end
   end
-end
+end

spec/fog/compute/xen_server/models/server_spec.rb

@@ -27,7 +27,7 @@ def self.read_identity
     server_class.read_identity.must_equal(:reference)
   end
 
-  it 'should have 58 attributes' do
+  it 'should have 57 attributes' do
     server_class.attributes.must_equal([ :reference,
                                          :actions_after_crash,
                                          :actions_after_reboot,
@@ -77,7 +77,6 @@ def self.read_identity
                                          :snapshot_time,
                                          :start_delay,
                                          :tags,
-                                         :template_name,
                                          :transportable_snapshot_id,
                                          :user_version,
                                          :uuid,
@@ -110,7 +109,7 @@ def self.read_identity
                                          :vtpms => :vtpms)
   end
 
-  it 'should have 77 masks' do
+  it 'should have 76 masks' do
     server_class.masks.must_equal(:reference => :reference,
                                   :actions_after_crash => :actions_after_crash,
                                   :actions_after_reboot => :actions_after_reboot,
@@ -160,7 +159,6 @@ def self.read_identity
                                   :snapshot_time => :snapshot_time,
                                   :start_delay => :start_delay,
                                   :tags => :tags,
-                                  :template_name => :template_name,
                                   :transportable_snapshot_id => :transportable_snapshot_id,
                                   :user_version => :user_version,
                                   :uuid => :uuid,
@@ -482,4 +480,57 @@ def server.wait_for(&block); instance_eval(&block); @halted = true end
       end
     end
   end
+
+  describe '#can_be_cloned?' do
+    describe "when it can be cloned" do
+      before :each do
+        server.allowed_operations = %w(clone)
+      end
+
+      it 'should return true' do
+        server.can_be_cloned?.must_equal(true)
+      end
+    end
+
+    describe 'when can not be cloned' do
+      before :each do
+        server.allowed_operations = []
+      end
+
+      it 'should return false' do
+        server.can_be_cloned?.must_equal(false)
+      end
+    end
+  end
+
+  describe '#clone' do
+    describe "when it can be cloned" do
+      before :each do
+        def server.can_be_cloned?; true end
+        def service.clone_vm(name, reference); @cloned = true end
+        def server.reload; @reloaded = true end
+        server.stub(:service, service) do
+          server.clone('')
+        end
+      end
+
+      it 'should clone the vm' do
+        service.instance_variable_get(:@cloned).must_equal(true)
+      end
+
+      it 'should reload the vm' do
+        server.instance_variable_get(:@reloaded).must_equal(true)
+      end
+    end
+
+    describe 'when can not be cloned' do
+      before :each do
+        def server.can_be_cloned?; false end
+      end
+
+      it 'should raise an exception' do
+        lambda { server.clone('') }.must_raise RuntimeError, 'Clone Operation not Allowed'
+      end
+    end
+  end
 end