Address review comments

Author: hoffmaen

app/actions/route_update.rb

@@ -2,16 +2,15 @@ module VCAP::CloudController
   class RouteUpdate
     def update(route:, message:)
       Route.db.transaction do
-        route.options = route.options.symbolize_keys.merge(message.options).compact if message.requested?(:options)
-
+        if message.requested?(:options)
+          route.options = route.options.symbolize_keys.merge(message.options).compact
+          route.apps.each do |process|
+            ProcessRouteHandler.new(process).notify_backend_of_route_update
+          end
+        end
         route.save
         MetadataUpdate.update(route, message)
-
-        route.apps.each do |process|
-          ProcessRouteHandler.new(process).notify_backend_of_route_update
-        end
       end
-
       route
     end
   end

spec/unit/actions/route_update_spec.rb

@@ -41,11 +41,16 @@ module VCAP::CloudController
     end
 
     let(:message) { RouteUpdateMessage.new(body) }
-    let(:space) { Space.make }
-    let(:route) { Route.make(space:) }
+    let(:process) { ProcessModel.make }
+    let(:route_mapping) { RouteMappingModel.make(app: process.app) }
+    let(:route) { route_mapping.route }
 
     subject { RouteUpdate.new }
     describe '#update metadata' do
+      before do
+        expect(ProcessRouteHandler).not_to receive(:new)
+      end
+
       context 'when the route has no existing metadata' do
         context 'when no metadata is specified' do
           let(:body) do
@@ -136,6 +141,13 @@ module VCAP::CloudController
     end
 
     describe '#update options' do
+      let(:fake_route_handler) { instance_double(ProcessRouteHandler) }
+
+      before do
+        allow(ProcessRouteHandler).to receive(:new).with(process).and_return(fake_route_handler)
+        allow(fake_route_handler).to receive(:notify_backend_of_route_update)
+      end
+
       context 'when the route has no existing options' do
         context 'when no options are specified' do
           let(:body) do
@@ -144,17 +156,15 @@ module VCAP::CloudController
 
           it 'adds no options' do
             expect(message).to be_valid
-
-            fake_route_handler = instance_double(ProcessRouteHandler)
-            process = ProcessModelFactory.make(space: space, state: 'STARTED', diego: false)
-            RouteMappingModel.make(app: process.app, route: route, process_type: process.type)
-            allow(ProcessRouteHandler).to receive(:new).with(process).and_return(fake_route_handler)
-            expect(fake_route_handler).to receive(:notify_backend_of_route_update)
-
             subject.update(route:, message:)
             route.reload
             expect(route.options).to eq({})
           end
+
+          it 'does not notifies the backend' do
+            expect(fake_route_handler).not_to receive(:notify_backend_of_route_update)
+            subject.update(route:, message:)
+          end
         end
 
         context 'when an option is specified' do
@@ -168,17 +178,15 @@ module VCAP::CloudController
 
           it 'adds the route option' do
             expect(message).to be_valid
-
-            fake_route_handler = instance_double(ProcessRouteHandler)
-            process = ProcessModelFactory.make(space: space, state: 'STARTED', diego: false)
-            RouteMappingModel.make(app: process.app, route: route, process_type: process.type)
-            allow(ProcessRouteHandler).to receive(:new).with(process).and_return(fake_route_handler)
-            expect(fake_route_handler).to receive(:notify_backend_of_route_update)
-
             subject.update(route:, message:)
             route.reload
             expect(route[:options]).to eq('{"loadbalancing":"round-robin"}')
           end
+
+          it 'notifies the backend' do
+            expect(fake_route_handler).to receive(:notify_backend_of_route_update)
+            subject.update(route:, message:)
+          end
         end
       end
 
@@ -194,17 +202,15 @@ module VCAP::CloudController
 
           it 'modifies nothing' do
             expect(message).to be_valid
-
-            fake_route_handler = instance_double(ProcessRouteHandler)
-            process = ProcessModelFactory.make(space: space, state: 'STARTED', diego: false)
-            RouteMappingModel.make(app: process.app, route: route, process_type: process.type)
-            allow(ProcessRouteHandler).to receive(:new).with(process).and_return(fake_route_handler)
-            expect(fake_route_handler).to receive(:notify_backend_of_route_update)
-
             subject.update(route:, message:)
             route.reload
             expect(route.options).to include({ 'loadbalancing' => 'round-robin' })
           end
+
+          it 'does not notifies the backend' do
+            expect(fake_route_handler).not_to receive(:notify_backend_of_route_update)
+            subject.update(route:, message:)
+          end
         end
 
         context 'when an option is specified' do
@@ -218,18 +224,15 @@ module VCAP::CloudController
 
           it 'updates the option' do
             expect(message).to be_valid
-
-            fake_route_handler = instance_double(ProcessRouteHandler)
-            process = ProcessModelFactory.make(space: space, state: 'STARTED', diego: false)
-            RouteMappingModel.make(app: process.app, route: route, process_type: process.type)
-            allow(ProcessRouteHandler).to receive(:new).with(process).and_return(fake_route_handler)
-            expect(fake_route_handler).to receive(:notify_backend_of_route_update)
-
             subject.update(route:, message:)
             route.reload
-
             expect(route.options).to include({ 'loadbalancing' => 'least-connection' })
           end
+
+          it 'notifies the backend' do
+            expect(fake_route_handler).to receive(:notify_backend_of_route_update)
+            subject.update(route:, message:)
+          end
         end
 
         context 'when the option value is set to null' do
@@ -243,17 +246,15 @@ module VCAP::CloudController
 
           it 'removes this option' do
             expect(message).to be_valid
-
-            fake_route_handler = instance_double(ProcessRouteHandler)
-            process = ProcessModelFactory.make(space: space, state: 'STARTED', diego: false)
-            RouteMappingModel.make(app: process.app, route: route, process_type: process.type)
-            allow(ProcessRouteHandler).to receive(:new).with(process).and_return(fake_route_handler)
-            expect(fake_route_handler).to receive(:notify_backend_of_route_update)
-
             subject.update(route:, message:)
             route.reload
             expect(route.options).to eq({})
           end
+
+          it 'notifies the backend' do
+            expect(fake_route_handler).to receive(:notify_backend_of_route_update)
+            subject.update(route:, message:)
+          end
         end
       end
     end