Fixing review comments in volumes

lib/oneview-sdk/resource/api200/volume.rb

@@ -39,6 +39,14 @@ def create
         self
       end
 
+      # Update resource attributes
+      # @param [Hash] attributes attributes to be updated
+      # @return [OneviewSDK::Volume] self
+      def update(attributes = {})
+        @data.delete('provisioningParameters')
+        super
+      end
+
       # Deletes the resource from OneView or from Oneview and storage system
       # @param [Symbol] flag Delete storage system from Oneview only or in storage system as well
       # @return [true] if resource was deleted successfully
@@ -59,16 +67,19 @@ def delete(flag = :all)
 
       # Sets the storage system to the volume
       # @param [OneviewSDK::StorageSystem] storage_system Storage System
+      # @note The storageSystemUri attribute should not be set in the updated. Once created, this attribute is read only.
       def set_storage_system(storage_system)
         assure_uri(storage_system)
         set('storageSystemUri', storage_system['uri'])
       end
 
       # Sets the storage pool to the volume
+      # @note The storagePoolUri attribute should not be set in the updated. Once created, this attribute is read only.
       # @param [OneviewSDK::StoragePool] storage_pool Storage pool
       def set_storage_pool(storage_pool)
         assure_uri(storage_pool)
-        self['provisioningParameters']['storagePoolUri'] = storage_pool['uri'] if self['provisioningParameters']
+        self['provisioningParameters'] ||= {}
+        self['provisioningParameters']['storagePoolUri'] = storage_pool['uri']
       end
 
       # Adds the storage volume template to the volume

lib/oneview-sdk/resource/api500/c7000/volume.rb

@@ -43,6 +43,14 @@ def create
           self
         end
 
+        # Update resource attributes
+        # @param [Hash] attributes attributes to be updated
+        # @return [OneviewSDK::Volume] self
+        def update(attributes = {})
+          @data.delete('properties')
+          OneviewSDK::Resource.instance_method(:update).bind(self).call(attributes)
+        end
+
         # Deletes the resource from OneView or from Oneview and storage system
         # @param [Symbol] flag Delete storage system from Oneview only or in storage system as well.
         #   Flags: :all = removes the volume from oneview and storage system. :oneview = removes from oneview only.
@@ -60,20 +68,23 @@ def delete(flag = :all)
 
         # Sets the storage pool to the volume
         # @param [OneviewSDK::StoragePool] storage_pool Storage pool.
+        # @note The storagePoolUri attribute should not be set in the updated. Once created, this attribute is read only.
         def set_storage_pool(storage_pool)
           assure_uri(storage_pool)
-          @data['properties']['storagePool'] = storage_pool['uri'] if @data['properties']
+          @data['properties'] ||= {}
+          @data['properties']['storagePool'] = storage_pool['uri']
         end
 
         # Sets the snapshot pool to the volume
         # @param [OneviewSDK::StoragePool] storage_pool Storage Pool to use for snapshots.
         def set_snapshot_pool(storage_pool)
           assure_uri(storage_pool)
-          if @data['properties']
-            @data['properties']['snapshotPool'] = storage_pool['uri']
-          else
+          if @data['uri']
             @data['deviceSpecificAttributes'] ||= {}
             @data['deviceSpecificAttributes']['snapshotPoolUri'] = storage_pool['uri']
+          else
+            @data['properties'] ||= {}
+            @data['properties']['snapshotPool'] = storage_pool['uri']
           end
         end
 

spec/integration/shared_examples/volume/api200/update_snapshot_pool.rb

@@ -12,21 +12,21 @@
 RSpec.shared_examples 'VolumeSnapshotPoolUpdateExample' do |context_name|
   include_context context_name
 
-  let(:item) { described_class.find_by(current_client, name: VOLUME2_NAME).first }
-
   describe '#update snapshot pool' do
     it 'updating the snapshot pool' do
-      old_snapshot_pool = resource_class_of('StoragePool').find_by(current_client, uri: item['snapshotPoolUri']).first
-      new_snapshot_pool = resource_class_of('StoragePool').get_all(current_client).first
-      item.set_snapshot_pool(new_snapshot_pool)
-      item.update
-      item.retrieve!
-      expect(item['snapshotPoolUri']).to eq(new_snapshot_pool['uri'])
+      volume = described_class.find_by(current_client, name: VOLUME4_NAME).first
+      storage_pools = resource_class_of('StoragePool').find_by(current_client, storageSystemUri: volume['storageSystemUri'])
+      old_snapshot_pool = resource_class_of('StoragePool').find_by(current_client, uri: volume['snapshotPoolUri']).first
+      new_snapshot_pool = storage_pools.select { |pool| pool['uri'] != old_snapshot_pool['uri'] }.first
+      volume.set_snapshot_pool(new_snapshot_pool)
+      volume.update
+      volume.retrieve!
+      expect(volume['snapshotPoolUri']).to eq(new_snapshot_pool['uri'])
       # Returning to original snapshot pool
-      item.set_snapshot_pool(old_snapshot_pool)
-      item.update
-      item.retrieve!
-      expect(item['snapshotPoolUri']).to eq(old_snapshot_pool['uri'])
+      volume.set_snapshot_pool(old_snapshot_pool)
+      volume.update
+      volume.retrieve!
+      expect(volume['snapshotPoolUri']).to eq(old_snapshot_pool['uri'])
     end
   end
 end

spec/integration/shared_examples/volume/api500/create.rb

@@ -231,30 +231,24 @@
   end
 
   describe '#set_storage_pool' do
-    it 'set_storage_pool in properties' do
-      item = described_class.new(current_client, name: VOLUME_NAME, properties: {})
-      item.set_storage_pool(storage_pool)
-      expect(item['properties']['storagePool']).to eq(storage_pool['uri'])
-    end
-
     it 'set_storage_pool' do
       item = described_class.new(current_client, name: VOLUME_NAME)
       item.set_storage_pool(storage_pool)
-      expect(item['storagePoolUri']).to eq(storage_pool['uri'])
+      expect(item['properties']['storagePool']).to eq(storage_pool['uri'])
     end
   end
 
   describe '#set_snapshot_pool' do
     it 'set_snapshot_pool in properties' do
-      item = described_class.new(current_client, name: VOLUME_NAME, properties: {})
+      item = described_class.new(current_client, name: VOLUME_NAME)
       item.set_snapshot_pool(storage_pool)
       expect(item['properties']['snapshotPool']).to eq(storage_pool['uri'])
     end
 
     it 'set_snapshot_pool' do
-      item = described_class.new(current_client, name: VOLUME_NAME)
+      item = described_class.get_all(current_client).first
       item.set_snapshot_pool(storage_pool)
-      expect(item['snapshotPoolUri']).to eq(storage_pool['uri'])
+      expect(item['deviceSpecificAttributes']['snapshotPoolUri']).to eq(storage_pool['uri'])
     end
   end
 

spec/integration/shared_examples/volume/api500/update_snapshot_pool.rb

@@ -12,22 +12,21 @@
 RSpec.shared_examples 'VolumeSnapshotPoolUpdateExample API500' do |context_name|
   include_context context_name
 
-  let(:item) { described_class.find_by(current_client, name: VOLUME2_NAME).first }
-
   describe '#update snapshot pool' do
     it 'updating the snapshot pool' do
-      old_snapshot_pool = resource_class_of('StoragePool').find_by(current_client, uri: item['deviceSpecificAttributes']['snapshotPoolUri']).first
+      volume = described_class.find_by(current_client, name: VOLUME5_NAME).first
+      old_snapshot_pool = resource_class_of('StoragePool').find_by(current_client, uri: volume['deviceSpecificAttributes']['snapshotPoolUri']).first
       new_snapshot_pool = resource_class_of('StoragePool').find_by(current_client, isManaged: false).first
       new_snapshot_pool.manage(true)
-      item.set_snapshot_pool(new_snapshot_pool)
-      item.update
-      item.retrieve!
-      expect(item['deviceSpecificAttributes']['snapshotPoolUri']).to eq(new_snapshot_pool['uri'])
+      volume.set_snapshot_pool(new_snapshot_pool)
+      volume.update
+      volume.retrieve!
+      expect(volume['deviceSpecificAttributes']['snapshotPoolUri']).to eq(new_snapshot_pool['uri'])
       # Returning to original snapshot pool
-      item.set_snapshot_pool(old_snapshot_pool)
-      item.update
-      item.retrieve!
-      expect(item['deviceSpecificAttributes']['snapshotPoolUri']).to eq(old_snapshot_pool['uri'])
+      volume.set_snapshot_pool(old_snapshot_pool)
+      volume.update
+      volume.retrieve!
+      expect(volume['deviceSpecificAttributes']['snapshotPoolUri']).to eq(old_snapshot_pool['uri'])
       new_snapshot_pool.manage(false)
     end
   end

spec/integration/shared_examples/volume/create.rb

@@ -71,6 +71,7 @@
       item2 = described_class.new(current_client, options.merge(name: VOLUME4_NAME, snapshotUri: "#{item[:uri]}/snapshots/#{snap['uri']}"))
       item2.set_storage_system(storage_system)
       item2.set_storage_pool(storage_pool)
+      item2.set_snapshot_pool(storage_pool)
       expect { item2.create }.to_not raise_error
       expect(item2.retrieve!).to eq(true)
       expect(item2['name']).to eq(VOLUME4_NAME)
@@ -112,16 +113,10 @@
   end
 
   describe '#set_storage_pool' do
-    it 'set_storage_pool in provisioningParameters' do
-      item = described_class.new(current_client, name: VOLUME_NAME, provisioningParameters: {})
-      item.set_storage_pool(storage_pool)
-      expect(item['provisioningParameters']['storagePoolUri']).to eq(storage_pool['uri'])
-    end
-
     it 'set_storage_pool' do
       item = described_class.new(current_client, name: VOLUME_NAME)
       item.set_storage_pool(storage_pool)
-      expect(item['storagePoolUri']).to eq(storage_pool['uri'])
+      expect(item['provisioningParameters']['storagePoolUri']).to eq(storage_pool['uri'])
     end
   end
 

spec/unit/resource/api200/volume_spec.rb

@@ -35,6 +35,21 @@
     end
   end
 
+  describe '#update' do
+    it 'updating a volume' do
+      fake_response = FakeResponse.new
+      item = described_class.new(@client_200, uri: 'rest/fake', name: 'Volume')
+      item.set_storage_pool(OneviewSDK::StoragePool.new(@client_200, uri: '/rest/fake2'))
+      allow_any_instance_of(OneviewSDK::Client).to receive(:rest_put).and_return(fake_response)
+      allow_any_instance_of(OneviewSDK::Client).to receive(:response_handler)
+        .with(fake_response).and_return('name' => 'Volume2')
+      data = { 'uri' => item['uri'], 'name' => 'Volume2' }
+      expect(@client_200).to receive(:rest_put).with('rest/fake', { 'body' => data }, 200)
+      item.update(name: 'Volume2')
+      expect(item['name']).to eq('Volume2')
+    end
+  end
+
   describe '#delete' do
     it 'raises an exception when is passed as invalid flag' do
       allow_any_instance_of(OneviewSDK::Client).to receive(:response_handler).and_return(true)
@@ -70,16 +85,10 @@
     end
 
     describe '#set_storage_pool' do
-      it 'sets the storagePoolUri in provisioningParameters' do
-        @item['provisioningParameters'] = {}
+      it 'sets the storagePoolUri' do
         @item.set_storage_pool(OneviewSDK::StoragePool.new(@client_200, uri: '/rest/fake'))
         expect(@item['provisioningParameters']['storagePoolUri']).to eq('/rest/fake')
       end
-
-      it 'should not sets the storagePoolUri without provisioningParameters' do
-        @item.set_storage_pool(OneviewSDK::StoragePool.new(@client_200, uri: '/rest/fake'))
-        expect(@item['storagePoolUri']).to_not be
-      end
     end
 
     describe '#set_storage_volume_template' do

spec/unit/resource/api500/c7000/volume_spec.rb

@@ -64,6 +64,20 @@
     end
   end
 
+  describe '#update' do
+    it 'updating a volume' do
+      item = described_class.new(@client_500, uri: 'rest/fake', name: 'Volume')
+      item.set_storage_pool(OneviewSDK::API500::C7000::StoragePool.new(@client_500, uri: '/rest/fake2'))
+      allow_any_instance_of(OneviewSDK::Client).to receive(:rest_put).and_return(fake_response)
+      allow_any_instance_of(OneviewSDK::Client).to receive(:response_handler)
+        .with(fake_response).and_return('name' => 'Volume2')
+      data = { 'uri' => item['uri'], 'name' => 'Volume2' }
+      expect(@client_500).to receive(:rest_put).with('rest/fake', { 'body' => data }, 500)
+      item.update(name: 'Volume2')
+      expect(item['name']).to eq('Volume2')
+    end
+  end
+
   describe '#delete' do
     it 'raises an exception when is passed as invalid flag' do
       allow_any_instance_of(OneviewSDK::Client).to receive(:response_handler).and_return(true)
@@ -87,30 +101,24 @@
   end
 
   describe '#set_storage_pool' do
-    it 'sets the storagePool attribute in properties' do
-      item = described_class.new(@client_500, properties: {})
-      item.set_storage_pool(OneviewSDK::StoragePool.new(@client_500, uri: '/rest/fake'))
-      expect(item['properties']['storagePool']).to eq('/rest/fake')
-    end
-
-    it 'should not sets the storagePoolUri without properties' do
+    it 'sets the storagePool attribute' do
       item = described_class.new(@client_500)
       item.set_storage_pool(OneviewSDK::StoragePool.new(@client_500, uri: '/rest/fake'))
-      expect(item['storagePoolUri']).to_not be
+      expect(item['properties']['storagePool']).to eq('/rest/fake')
     end
   end
 
   describe '#set_snapshot_pool' do
     it 'sets the snapshotPool attribute in properties' do
-      item = described_class.new(@client_500, properties: {})
+      item = described_class.new(@client_500)
       item.set_snapshot_pool(OneviewSDK::StoragePool.new(@client_500, uri: '/rest/fake'))
       expect(item['properties']['snapshotPool']).to eq('/rest/fake')
     end
 
     it 'sets the snapshotPoolUri attribute' do
-      item = described_class.new(@client_500, deviceSpecificAttributes: {})
-      item.set_snapshot_pool(OneviewSDK::StoragePool.new(@client_500, uri: '/rest/fake'))
-      expect(item['deviceSpecificAttributes']['snapshotPoolUri']).to eq('/rest/fake')
+      item = described_class.new(@client_500, uri: 'rest/fake')
+      item.set_snapshot_pool(OneviewSDK::StoragePool.new(@client_500, uri: '/rest/fake2'))
+      expect(item['deviceSpecificAttributes']['snapshotPoolUri']).to eq('/rest/fake2')
     end
   end
 
@@ -322,10 +330,11 @@
   end
 
   describe '#get_volume_template_uri' do
-    let(:instance_item) { described_class.new(@client_500, storagePoolUri: '/storage-pools/1') }
+    let(:instance_item) { described_class.new(@client_500) }
 
     before do
       storage_pool = OneviewSDK::API500::C7000::StoragePool.new(@client_500, storageSystemUri: '/storage-systems/1', uri: '/storage-pools/1')
+      instance_item.set_storage_pool(storage_pool)
       expect(storage_pool).to receive(:retrieve!).and_return(true)
       expect(OneviewSDK::API500::C7000::StoragePool).to receive(:new).and_return(storage_pool)
       storage_system = OneviewSDK::API500::C7000::StorageSystem.new(@client_500)