Merge branch 'master' of https://github.com/cloudfoundry/cloud_controller_ng

Author: Harsha Nandiwada

app/actions/service_route_binding_create.rb

@@ -1,3 +1,5 @@
+require 'services/service_brokers/service_client_provider'
+
 module VCAP::CloudController
   module V3
     class ServiceRouteBindingCreate
@@ -6,12 +8,7 @@ def initialize(service_event_repository)
       end
 
       def precursor(service_instance, route)
-        not_supported! unless service_instance.route_service?
-        not_bindable! unless service_instance.bindable?
-        route_is_internal! if route.try(:internal?)
-        space_mismatch! unless route.space == service_instance.space
-        already_exists! if route.service_instance == service_instance
-        already_bound! if route.service_instance
+        validate!(service_instance, route)
 
         RouteBinding.new.save_with_new_operation(
           {
@@ -98,6 +95,16 @@ class BindingNotRetrievable < StandardError; end
 
       private
 
+      def validate!(service_instance, route)
+        not_supported! unless service_instance.route_service?
+        not_bindable! unless service_instance.bindable?
+        route_is_internal! if route.try(:internal?)
+        space_mismatch! unless route.space == service_instance.space
+        already_exists! if route.service_instance == service_instance
+        already_bound! if route.service_instance
+        operation_in_progress! if service_instance.operation_in_progress?
+      end
+
       attr_reader :service_event_repository
 
       def fetch_last_operation(client, binding)
@@ -154,6 +161,10 @@ def bindings_retrievable?(binding)
         binding.service_instance.service.bindings_retrievable
       end
 
+      def operation_in_progress!
+        raise UnprocessableCreate.new('There is an operation in progress for the service instance')
+      end
+
       def route_is_internal!
         raise UnprocessableCreate.new('Route services cannot be bound to internal routes')
       end

app/models/services/route_binding.rb

@@ -1,3 +1,5 @@
+require 'cloud_controller/process_route_handler'
+
 module VCAP::CloudController
   class RouteBinding < Sequel::Model
     plugin :after_initialize

app/presenters/mixins/last_operation_helper.rb

@@ -0,0 +1,21 @@
+module VCAP::CloudController::Presenters::Mixins
+  module LastOperationHelper
+    def last_operation(resource)
+      last_operation = resource.try(:last_operation)
+
+      if last_operation
+        last_operation.to_hash
+      else
+        # Bindings created using V2 may not have a last operation, so we make
+        # a best attempt so that the output JSON can be parsed consistently
+        {
+          type: 'create',
+          state: 'succeeded',
+          description: '',
+          created_at: resource.try(:created_at),
+          updated_at: resource.try(:updated_at),
+        }
+      end
+    end
+  end
+end

app/presenters/v3/service_credential_binding_presenter.rb

@@ -1,32 +1,40 @@
 require_relative 'base_presenter'
+require 'presenters/mixins/last_operation_helper'
 
 module VCAP
   module CloudController
     module Presenters
       module V3
         class ServiceCredentialBindingPresenter < BasePresenter
+          include VCAP::CloudController::Presenters::Mixins::LastOperationHelper
+
           def to_hash
             base_hash.merge(extra).merge(decorations)
           end
 
           private
 
+          def binding
+            @resource
+          end
+
           def base_hash
             {
-              guid: @resource.guid,
-              created_at: @resource.created_at,
-              updated_at: @resource.updated_at,
-              name: @resource.name,
-              type: type
+              guid: binding.guid,
+              created_at: binding.created_at,
+              updated_at: binding.updated_at,
+              name: binding.name,
+              type: type,
+              last_operation: last_operation(binding),
             }
           end
 
           def decorations
-            @decorators.reduce({}) { |memo, d| d.decorate(memo, [@resource]) }
+            @decorators.reduce({}) { |memo, d| d.decorate(memo, [binding]) }
           end
 
           def type
-            case @resource
+            case binding
             when VCAP::CloudController::ServiceKey
               'key'
             when VCAP::CloudController::ServiceBinding
@@ -38,65 +46,49 @@ def extra
             case type
             when 'app'
               {
-                last_operation: last_operation,
                 relationships: base_relationships.merge(app_relationship),
                 links: base_links.merge(app_link)
               }
             when 'key'
               {
-                last_operation: nil,
                 relationships: base_relationships,
                 links: base_links
               }
             end
           end
 
-          def last_operation
-            return nil if @resource.last_operation.blank?
-
-            last_operation = @resource.last_operation
-
-            {
-              type: last_operation.type,
-              state: last_operation.state,
-              description: last_operation.description,
-              created_at: last_operation.created_at,
-              updated_at: last_operation.updated_at
-            }
-          end
-
           def base_links
-            parameters = { parameters: "#{path_to_self}/parameters" } unless @resource.service_instance.user_provided_instance?
+            parameters = { parameters: "#{path_to_self}/parameters" } unless binding.service_instance.user_provided_instance?
 
             {
               self: path_to_self,
               details: "#{path_to_self}/details",
-              service_instance: "/v3/service_instances/#{@resource.service_instance_guid}"
+              service_instance: "/v3/service_instances/#{binding.service_instance_guid}"
             }.merge(parameters || {}).transform_values do |path|
               hrefify(path)
             end
           end
 
           def path_to_self
-            "/v3/service_credential_bindings/#{@resource.guid}"
+            "/v3/service_credential_bindings/#{binding.guid}"
           end
 
           def base_relationships
             {
-              service_instance: { data: { guid: @resource.service_instance_guid } }
+              service_instance: { data: { guid: binding.service_instance_guid } }
             }
           end
 
           def app_relationship
-            return {} if @resource.app_guid.blank?
+            return {} if binding.app_guid.blank?
 
-            { app: { data: { guid: @resource.app_guid } } }
+            { app: { data: { guid: binding.app_guid } } }
           end
 
           def app_link
-            return {} if @resource.app_guid.blank?
+            return {} if binding.app_guid.blank?
 
-            { app: hrefify("/v3/apps/#{@resource.app_guid}") }
+            { app: hrefify("/v3/apps/#{binding.app_guid}") }
           end
 
           def hrefify(path)

app/presenters/v3/service_route_binding_presenter.rb

@@ -1,10 +1,13 @@
 require_relative 'base_presenter'
+require 'presenters/mixins/last_operation_helper'
 
 module VCAP
   module CloudController
     module Presenters
       module V3
         class ServiceRouteBindingPresenter < BasePresenter
+          include VCAP::CloudController::Presenters::Mixins::LastOperationHelper
+
           def to_hash
             base.merge(decorations)
           end
@@ -17,7 +20,7 @@ def base
               route_service_url: binding.route_service_url,
               created_at: binding.created_at,
               updated_at: binding.updated_at,
-              last_operation: last_operation,
+              last_operation: last_operation(binding),
               relationships: relationships,
               links: links
             }
@@ -31,20 +34,6 @@ def binding
             @resource
           end
 
-          def last_operation
-            return nil if binding.last_operation.blank?
-
-            last_operation = binding.last_operation
-
-            {
-              type: last_operation.type,
-              state: last_operation.state,
-              description: last_operation.description,
-              created_at: last_operation.created_at,
-              updated_at: last_operation.updated_at
-            }
-          end
-
           def links
             {
               self: {

app/repositories/app_usage_event_repository.rb

@@ -1,4 +1,5 @@
 require 'database/old_record_cleanup'
+require 'cloud_controller/url_secret_obfuscator'
 
 module VCAP::CloudController
   module Repositories

spec/request/service_credential_bindings_spec.rb

@@ -1296,7 +1296,7 @@ def expected_json(binding)
       created_at: iso8601,
       updated_at: iso8601,
       name: binding.name,
-      last_operation: nil,
+      last_operation: last_operation(binding),
       relationships: {
         service_instance: {
           data: {
@@ -1350,14 +1350,22 @@ def extra(binding)
   end
 
   def last_operation(binding)
-    if binding.last_operation.present?
+    if binding.try(:last_operation)
       {
         type: binding.last_operation.type,
         state: binding.last_operation.state,
         description: binding.last_operation.description,
         created_at: iso8601,
         updated_at: iso8601
       }
+    else
+      {
+        type: 'create',
+        state: 'succeeded',
+        description: '',
+        created_at: iso8601,
+        updated_at: iso8601
+      }
     end
   end
 

spec/request/service_route_bindings_spec.rb

@@ -504,6 +504,21 @@
           expect(VCAP::CloudController::RouteBinding.all).to be_empty
         end
       end
+
+      context 'there is an operation in progress for the service instance' do
+        it 'responds with 422' do
+          service_instance.save_with_new_operation({}, { type: 'guacamole', state: 'in progress' })
+
+          post '/v3/service_route_bindings', request.to_json, space_dev_headers
+
+          expect(last_response).to have_status_code(422)
+          expect(parsed_response['errors']).to include(include({
+            'detail' => include('There is an operation in progress for the service instance'),
+            'title' => 'CF-UnprocessableEntity',
+            'code' => 10008,
+          }))
+        end
+      end
     end
 
     context 'user-provided service instance' do

spec/unit/actions/service_route_binding_create_spec.rb

@@ -1,4 +1,4 @@
-require 'spec_helper'
+require 'db_spec_helper'
 require 'actions/service_route_binding_create'
 
 module VCAP::CloudController
@@ -120,6 +120,19 @@ module V3
               )
             end
           end
+
+          context 'when there is an operation in progress for the service instance' do
+            it 'raises an error' do
+              service_instance.save_with_new_operation({}, { type: 'tacos', state: 'in progress' })
+
+              expect {
+                action.precursor(service_instance, route)
+              }.to raise_error(
+                ServiceRouteBindingCreate::UnprocessableCreate,
+                'There is an operation in progress for the service instance'
+              )
+            end
+          end
         end
 
         context 'user-provided service instance' do