v3(service instance): allow metadata updates while create is in progress

Solves #1847
Solves #1848

[#174825693](https://www.pivotaltracker.com/story/show/174825693)

app/actions/service_instance_update_managed.rb

@@ -19,6 +19,8 @@ def update(service_instance, message)
       updater.raise_if_cannot_proceed!
 
       begin
+        return updater.metadata_sync, nil unless is_deleting?(service_instance) || !updater.only_metadata?
+
         lock = UpdaterLock.new(service_instance)
         lock.lock!
 
@@ -35,12 +37,16 @@ def update(service_instance, message)
           return si, nil
         end
       ensure
-        lock.unlock_and_fail! if lock.needs_unlock?
+        lock.unlock_and_fail! if lock.present? && lock.needs_unlock?
       end
     end
 
     private
 
+    def is_deleting?(service_instance)
+      service_instance.operation_in_progress? && service_instance.last_operation[:type] == 'delete'
+    end
+
     attr_reader :service_event_repository
 
     class Updater
@@ -71,12 +77,21 @@ def update_broker_needed?
         service_name_changed || parameters_changed || service_plan_changed || maintenance_info_changed
       end
 
+      def only_metadata?
+        message.requested_keys.one? && message.requested?(:metadata)
+      end
+
+      def metadata_sync
+        MetadataUpdate.update(service_instance, message)
+        service_instance
+      end
+
       def update_sync
         logger = Steno.logger('cc.action.service_instance_update')
 
         service_instance.db.transaction do
           service_instance.update(message.updates) if message.updates.any?
-          MetadataUpdate.update(service_instance, message)
+          metadata_sync
         end
 
         logger.info("Finished updating service_instance #{service_instance.guid}")

spec/request/service_instances_spec.rb

@@ -1583,7 +1583,7 @@ def check_filtered_instances(*instances)
           api_call.call(space_dev_headers)
 
           service_instance.reload
-          expect(service_instance.reload.tags).to eq(%w(baz quz))
+          expect(service_instance.tags).to eq(%w(baz quz))
 
           expect_metadata(
             service_instance,
@@ -1683,7 +1683,7 @@ def check_filtered_instances(*instances)
           expect(service_instance.last_operation.state).to eq('in progress')
         end
 
-        it 'does not update the service instance' do
+        it 'does not immediately update the service instance' do
           api_call.call(space_dev_headers)
 
           service_instance.reload
@@ -2499,6 +2499,55 @@ def check_filtered_instances(*instances)
         end
       end
     end
+
+    context 'when an operation is in progress' do
+      let(:service_instance) {
+        si = VCAP::CloudController::ManagedServiceInstance.make(
+          space: space
+        )
+        si.save_with_new_operation({}, { type: 'create', state: 'in progress', description: 'almost there, I promise' })
+        si
+      }
+      let(:guid) { service_instance.guid }
+      let(:request_body) {
+        {
+            metadata: {
+                labels: { unit: 'metre', distance: '1003' },
+                annotations: { location: 'london' }
+            }
+        }
+      }
+
+      context 'and the update contains metadata only' do
+        it 'updates the metadata' do
+          api_call.call(admin_headers)
+          expect(last_response).to have_status_code(200)
+          expect(parsed_response.dig('metadata', 'labels')).to eq({ 'unit' => 'metre', 'distance' => '1003' })
+          expect(parsed_response.dig('metadata', 'annotations')).to eq({ 'location' => 'london' })
+        end
+
+        it 'does not update the service instance last operation' do
+          api_call.call(admin_headers)
+          expect(last_response).to have_status_code(200)
+          expect(parsed_response['last_operation']).to include({
+                                                                   'type' => 'create',
+                                                                   'state' => 'in progress',
+                                                                   'description' => 'almost there, I promise'
+                                                               })
+        end
+      end
+
+      context 'and the update contains more than just metadata' do
+        it 'returns an error' do
+          request_body[:name] = 'new-name'
+          api_call.call(admin_headers)
+          expect(last_response).to have_status_code(409)
+          response = parsed_response['errors'].first
+          expect(response).to include('title' => 'CF-AsyncServiceInstanceOperationInProgress')
+          expect(response).to include('detail' => include("An operation for service instance #{service_instance.name} is in progress"))
+        end
+      end
+    end
   end
 
   describe 'DELETE /v3/service_instances/:guid' do

spec/unit/actions/service_instance_update_managed_spec.rb

@@ -372,13 +372,33 @@ module VCAP::CloudController
         end
       end
 
-      context 'when an operation is in progress' do
+      context 'when a delete is in progress' do
         let(:body) { {} }
 
         before do
           service_instance.save_with_new_operation({}, { type: 'delete', state: 'in progress' })
         end
 
+        it 'raises with metadata only updates' do
+          msg = ServiceInstanceUpdateManagedMessage.new({
+                                                            metadata: {
+                                                                labels: {
+                                                                    foo: 'bar',
+                                                                    'pre.fix/to_delete': nil,
+                                                                },
+                                                                annotations: {
+                                                                    alpha: 'beta',
+                                                                    'pre.fix/to_delete': nil,
+                                                                }
+                                                            }
+                                                        })
+          expect {
+            action.update(service_instance, msg)
+          }.to raise_error CloudController::Errors::ApiError do |err|
+            expect(err.name).to eq('AsyncServiceInstanceOperationInProgress')
+          end
+        end
+
         it 'raises' do
           expect {
             action.update(service_instance, message)
@@ -388,6 +408,90 @@ module VCAP::CloudController
         end
       end
 
+      context 'when an update is in progress' do
+        before do
+          service_instance.save_with_new_operation({}, { type: 'update', state: 'in progress' })
+        end
+
+        it 'allows metadata updates' do
+          msg = ServiceInstanceUpdateManagedMessage.new({
+                                                            metadata: {
+                                                                labels: {
+                                                                    foo: 'bar',
+                                                                    'pre.fix/to_delete': nil,
+                                                                },
+                                                                annotations: {
+                                                                    alpha: 'beta',
+                                                                    'pre.fix/to_delete': nil,
+                                                                }
+                                                            }
+                                                        })
+          expect { action.update(service_instance, msg) }.not_to raise_error
+
+          service_instance.reload
+
+          expect(service_instance.labels.map { |l| { prefix: l.key_prefix, key: l.key_name, value: l.value } }).to match_array([
+            { prefix: nil, key: 'foo', value: 'bar' },
+            { prefix: 'pre.fix', key: 'tail', value: 'fluffy' },
+          ])
+          expect(service_instance.annotations.map { |a| { prefix: a.key_prefix, key: a.key, value: a.value } }).to match_array([
+            { prefix: nil, key: 'alpha', value: 'beta' },
+            { prefix: 'pre.fix', key: 'fox', value: 'bushy' },
+          ])
+        end
+
+        it 'raises with non metadata updates' do
+          msg = ServiceInstanceUpdateManagedMessage.new({ name: 'new-name' })
+          expect {
+            action.update(service_instance, msg)
+          }.to raise_error CloudController::Errors::ApiError do |err|
+            expect(err.name).to eq('AsyncServiceInstanceOperationInProgress')
+          end
+        end
+      end
+
+      context 'when a create is in progress' do
+        before do
+          service_instance.save_with_new_operation({}, { type: 'create', state: 'in progress' })
+        end
+
+        it 'allows metadata updates' do
+          msg = ServiceInstanceUpdateManagedMessage.new({
+                                                            metadata: {
+                                                                labels: {
+                                                                    foo: 'bar',
+                                                                    'pre.fix/to_delete': nil,
+                                                                },
+                                                                annotations: {
+                                                                    alpha: 'beta',
+                                                                    'pre.fix/to_delete': nil,
+                                                                }
+                                                            }
+                                                        })
+          expect { action.update(service_instance, msg) }.not_to raise_error
+
+          service_instance.reload
+
+          expect(service_instance.labels.map { |l| { prefix: l.key_prefix, key: l.key_name, value: l.value } }).to match_array([
+            { prefix: nil, key: 'foo', value: 'bar' },
+            { prefix: 'pre.fix', key: 'tail', value: 'fluffy' },
+          ])
+          expect(service_instance.annotations.map { |a| { prefix: a.key_prefix, key: a.key, value: a.value } }).to match_array([
+            { prefix: nil, key: 'alpha', value: 'beta' },
+            { prefix: 'pre.fix', key: 'fox', value: 'bushy' },
+          ])
+        end
+
+        it 'raises with non metadata updates' do
+          msg = ServiceInstanceUpdateManagedMessage.new({ name: 'new-name' })
+          expect {
+            action.update(service_instance, msg)
+          }.to raise_error CloudController::Errors::ApiError do |err|
+            expect(err.name).to eq('AsyncServiceInstanceOperationInProgress')
+          end
+        end
+      end
+
       describe 'invalid name updates' do
         context 'when the new name is already taken' do
           let(:instance_in_same_space) { ServiceInstance.make(space: service_instance.space) }