From 34362de09969ae96d6404cf6ab93a18126ac1805 Mon Sep 17 00:00:00 2001
From: Denise Yu <dyu@pivotal.io>
Date: Fri, 17 Nov 2017 17:17:38 +0100
Subject: [PATCH 1/8] Add space_guids query parameter to GET
 /v3/service_instances

[#152922808]

Signed-off-by: Jen Spinney <jennifer.spinney@suse.com>
---
 .../v3/service_instances_controller.rb        |  2 +-
 app/fetchers/service_instance_list_fetcher.rb | 11 +++-
 .../service_instances_list_message.rb         |  4 +-
 .../service_instances/_list.md.erb            |  1 +
 ...b => service_instances_controller_spec.rb} | 30 ++++++++--
 .../service_instances_list_message_spec.rb    | 26 ++++++++-
 .../service_instance_list_fetcher_spec.rb     | 58 ++++++++++++++-----
 7 files changed, 107 insertions(+), 25 deletions(-)
 rename spec/unit/controllers/v3/{service_instance_controller_spec.rb => service_instances_controller_spec.rb} (93%)

diff --git a/app/controllers/v3/service_instances_controller.rb b/app/controllers/v3/service_instances_controller.rb
index a3dd03dc0e9..0bdf8c2cf39 100644
--- a/app/controllers/v3/service_instances_controller.rb
+++ b/app/controllers/v3/service_instances_controller.rb
@@ -16,7 +16,7 @@ def index
     dataset = if can_read_globally?
                 ServiceInstanceListFetcher.new.fetch_all(message: message)
               else
-                ServiceInstanceListFetcher.new.fetch(message: message, space_guids: readable_space_guids)
+                ServiceInstanceListFetcher.new.fetch(message: message, readable_space_guids: readable_space_guids)
               end
 
     render status: :ok, json: Presenters::V3::PaginatedListPresenter.new(
diff --git a/app/fetchers/service_instance_list_fetcher.rb b/app/fetchers/service_instance_list_fetcher.rb
index 01212a3f3b1..ee0e0aa5159 100644
--- a/app/fetchers/service_instance_list_fetcher.rb
+++ b/app/fetchers/service_instance_list_fetcher.rb
@@ -1,11 +1,11 @@
 module VCAP::CloudController
   class ServiceInstanceListFetcher
-    def fetch(message:, space_guids:)
+    def fetch(message:, readable_space_guids:)
       source_space_instance_dataset = ServiceInstance.select_all(ServiceInstance.table_name).
-                                      join(Space.table_name, id: :space_id, guid: space_guids)
+                                      join(Space.table_name, id: :space_id, guid: readable_space_guids)
 
       shared_instance_dataset = ServiceInstance.select_all(ServiceInstance.table_name).
-                                join(:service_instance_shares, service_instance_guid: :guid, target_space_guid: space_guids)
+                                join(:service_instance_shares, service_instance_guid: :guid, target_space_guid: readable_space_guids)
 
       dataset = source_space_instance_dataset.union(shared_instance_dataset, alias: :service_instances)
 
@@ -24,6 +24,11 @@ def filter(dataset, message)
         dataset = dataset.where(service_instances__name: message.names)
       end
 
+      if message.requested?(:space_guids)
+        dataset = dataset.select_all(ServiceInstance.table_name).
+                  join_table(:inner, Space.table_name, { id: Sequel.qualify(:service_instances, :space_id), guid: message.space_guids })
+      end
+
       dataset
     end
   end
diff --git a/app/messages/service_instances/service_instances_list_message.rb b/app/messages/service_instances/service_instances_list_message.rb
index f368e921fe4..1f1a4478670 100644
--- a/app/messages/service_instances/service_instances_list_message.rb
+++ b/app/messages/service_instances/service_instances_list_message.rb
@@ -2,12 +2,13 @@
 
 module VCAP::CloudController
   class ServiceInstancesListMessage < ListMessage
-    ALLOWED_KEYS = [:page, :per_page, :order_by, :names].freeze
+    ALLOWED_KEYS = [:page, :per_page, :order_by, :names, :space_guids].freeze
 
     attr_accessor(*ALLOWED_KEYS)
 
     validates_with NoAdditionalParamsValidator
     validates :names, array: true, allow_nil: true
+    validates :space_guids, array: true, allow_nil: true
 
     def initialize(params={})
       super(params.symbolize_keys)
@@ -16,6 +17,7 @@ def initialize(params={})
     def self.from_params(params)
       opts = params.dup
       to_array! opts, 'names'
+      to_array! opts, 'space_guids'
       new(opts.symbolize_keys)
     end
 
diff --git a/docs/v3/source/includes/experimental_resources/service_instances/_list.md.erb b/docs/v3/source/includes/experimental_resources/service_instances/_list.md.erb
index 90822bb3bb5..47be9f9a5cb 100644
--- a/docs/v3/source/includes/experimental_resources/service_instances/_list.md.erb
+++ b/docs/v3/source/includes/experimental_resources/service_instances/_list.md.erb
@@ -32,5 +32,6 @@ This includes access granted by service instance sharing.
 Name | Type | Description
 ---- | ---- | ------------
 **names** | _list of strings_ | Comma-delimited list of service instance names to filter by.
+**space_guids** | _list of strings_ | Comma-delimited list of space guids to filter by.
 **page** | _integer_ | Page to display. Valid values are integers >= 1.
 **per_page** | _integer_ | Number of results per page. <br>Valid values are 1 through 5000.
diff --git a/spec/unit/controllers/v3/service_instance_controller_spec.rb b/spec/unit/controllers/v3/service_instances_controller_spec.rb
similarity index 93%
rename from spec/unit/controllers/v3/service_instance_controller_spec.rb
rename to spec/unit/controllers/v3/service_instances_controller_spec.rb
index cb08319882d..f59bfd58f1b 100644
--- a/spec/unit/controllers/v3/service_instance_controller_spec.rb
+++ b/spec/unit/controllers/v3/service_instances_controller_spec.rb
@@ -2,13 +2,13 @@
 
 RSpec.describe ServiceInstancesV3Controller, type: :controller do
   let(:user) { set_current_user(VCAP::CloudController::User.make) }
-  let(:space) { VCAP::CloudController::Space.make }
-  let!(:service_instance) { VCAP::CloudController::ManagedServiceInstance.make(space: space) }
+  let(:space) { VCAP::CloudController::Space.make(guid: 'space-1-guid') }
+  let!(:service_instance) { VCAP::CloudController::ManagedServiceInstance.make(space: space, name: 'service-instance-1') }
 
   describe '#index' do
     context 'when there are multiple service instances' do
-      let!(:service_instance2) { VCAP::CloudController::ManagedServiceInstance.make }
-      let!(:service_instance3) { VCAP::CloudController::ManagedServiceInstance.make }
+      let!(:service_instance2) { VCAP::CloudController::ManagedServiceInstance.make(name: 'service-instance-2', space: VCAP::CloudController::Space.make(guid: 'space-2-guid')) }
+      let!(:service_instance3) { VCAP::CloudController::ManagedServiceInstance.make(name: 'service-instance-3', space: VCAP::CloudController::Space.make(guid: 'space-3-guid')) }
 
       context 'as an admin' do
         before do
@@ -23,6 +23,28 @@
           response_names = parsed_body['resources'].map { |resource| resource['name'] }
           expect(response_names).to include(service_instance.name, service_instance2.name, service_instance3.name)
         end
+
+        context 'when service instances are filtered by name' do
+          it 'returns only the matching service instances' do
+            get :index, names: 'service-instance-1,service-instance-2'
+            expect(response.status).to eq(200), response.body
+            expect(parsed_body['resources'].length).to eq 2
+
+            response_names = parsed_body['resources'].map { |resource| resource['name'] }
+            expect(response_names).to include(service_instance.name, service_instance2.name)
+          end
+        end
+
+        context 'when service instances are filtered by space guid' do
+          it 'returns only the matching service instances' do
+            get :index, space_guids: 'space-2-guid, space-3-guid'
+            expect(response.status).to eq(200), response.body
+            expect(parsed_body['resources'].length).to eq 2
+
+            response_names = parsed_body['resources'].map { |resource| resource['name'] }
+            expect(response_names).to include(service_instance2.name, service_instance3.name)
+          end
+        end
       end
 
       context 'as a user who only has limited access' do
diff --git a/spec/unit/messages/service_instances_list_message_spec.rb b/spec/unit/messages/service_instances_list_message_spec.rb
index 1d935b1d602..b5fac8eeccf 100644
--- a/spec/unit/messages/service_instances_list_message_spec.rb
+++ b/spec/unit/messages/service_instances_list_message_spec.rb
@@ -9,7 +9,8 @@ module VCAP::CloudController
           'page'      => 1,
           'per_page'  => 5,
           'order_by'  => 'name',
-          'names' => 'rabbitmq, redis,mysql'
+          'names' => 'rabbitmq, redis,mysql',
+          'space_guids' => 'space-1, space-2, space-3',
         }
       end
 
@@ -21,6 +22,7 @@ module VCAP::CloudController
         expect(message.per_page).to eq(5)
         expect(message.order_by).to eq('name')
         expect(message.names).to match_array(['mysql', 'rabbitmq', 'redis'])
+        expect(message.space_guids).to match_array(['space-1', 'space-2', 'space-3'])
       end
 
       it 'converts requested keys to symbols' do
@@ -30,6 +32,7 @@ module VCAP::CloudController
         expect(message.requested?(:per_page)).to be_truthy
         expect(message.requested?(:order_by)).to be_truthy
         expect(message.requested?(:names)).to be_truthy
+        expect(message.requested?(:space_guids)).to be_truthy
       end
     end
 
@@ -39,7 +42,8 @@ module VCAP::CloudController
             page: 1,
             per_page: 5,
             order_by: 'created_at',
-            names: ['rabbitmq', 'redis']
+            names: ['rabbitmq', 'redis'],
+            space_guids: ['space-1', 'space-2'],
           })
         expect(message).to be_valid
       end
@@ -56,5 +60,23 @@ module VCAP::CloudController
         expect(message.errors[:base]).to include("Unknown query parameter(s): 'foobar'")
       end
     end
+
+    describe 'validations' do
+      context 'names' do
+        it 'validates names is an array' do
+          message = ServiceInstancesListMessage.new names: 'tricked you, not an array'
+          expect(message).to be_invalid
+          expect(message.errors[:names]).to include('must be an array')
+        end
+      end
+
+      context 'space guids' do
+        it 'validates app_guids is an array' do
+          message = ServiceInstancesListMessage.new space_guids: 'tricked you, not an array'
+          expect(message).to be_invalid
+          expect(message.errors[:space_guids]).to include('must be an array')
+        end
+      end
+    end
   end
 end
diff --git a/spec/unit/queries/service_instance_list_fetcher_spec.rb b/spec/unit/queries/service_instance_list_fetcher_spec.rb
index fa07743ce8d..193ef08ac33 100644
--- a/spec/unit/queries/service_instance_list_fetcher_spec.rb
+++ b/spec/unit/queries/service_instance_list_fetcher_spec.rb
@@ -9,8 +9,9 @@ module VCAP::CloudController
     let(:fetcher) { ServiceInstanceListFetcher.new }
 
     describe '#fetch_all' do
-      let!(:service_instance_1) { ManagedServiceInstance.make(name: 'rabbitmq') }
-      let!(:service_instance_2) { ManagedServiceInstance.make(name: 'redis') }
+      let!(:service_instance_1) { ManagedServiceInstance.make(name: 'rabbitmq', space: Space.make(guid: 'space-1')) }
+      let!(:service_instance_2) { ManagedServiceInstance.make(name: 'redis', space: Space.make(guid: 'space-2')) }
+      let!(:service_instance_3) { ManagedServiceInstance.make(name: 'mysql', space: Space.make(guid: 'space-3')) }
 
       it 'returns a Sequel::Dataset' do
         results = fetcher.fetch_all(message: message)
@@ -19,18 +20,28 @@ module VCAP::CloudController
 
       it 'includes all the V3 Service Instances' do
         results = fetcher.fetch_all(message: message).all
-        expect(results.length).to eq 2
-        expect(results).to include(service_instance_1, service_instance_2)
+        expect(results.length).to eq 3
+        expect(results).to include(service_instance_1, service_instance_2, service_instance_3)
       end
 
       context 'filter' do
         context 'by service instance name' do
-          let(:filters) { { names: ['rabbitmq'] } }
+          let(:filters) { { names: ['rabbitmq', 'redis'] } }
 
           it 'only returns matching service instances' do
             results = fetcher.fetch_all(message: message).all
-            expect(results).to match_array([service_instance_1])
-            expect(results).not_to include(service_instance_2)
+            expect(results).to match_array([service_instance_1, service_instance_2])
+            expect(results).not_to include(service_instance_3)
+          end
+        end
+
+        context 'by space guid' do
+          let(:filters) { { space_guids: ['space-2', 'space-3'] } }
+
+          it 'only returns matching service instances' do
+            results = fetcher.fetch_all(message: message).all
+            expect(results).to match_array([service_instance_2, service_instance_3])
+            expect(results).not_to include(service_instance_1)
           end
         end
       end
@@ -41,11 +52,11 @@ module VCAP::CloudController
       let!(:service_instance_2) { ManagedServiceInstance.make(name: 'redis', space: space_1) }
       let!(:service_instance_3) { ManagedServiceInstance.make(name: 'mysql', space: space_2) }
 
-      let(:space_1) { Space.make }
-      let(:space_2) { Space.make }
+      let(:space_1) { Space.make(guid: 'space-1') }
+      let(:space_2) { Space.make(guid: 'space-2') }
 
       it 'returns all of the service instances in the specified space' do
-        results = fetcher.fetch(message: message, space_guids: [space_1.guid]).all
+        results = fetcher.fetch(message: message, readable_space_guids: [space_1.guid]).all
 
         expect(results).to match_array([service_instance_1, service_instance_2])
       end
@@ -55,16 +66,35 @@ module VCAP::CloudController
           let(:filters) { { names: ['rabbitmq'] } }
 
           it 'only returns matching service instances' do
-            results = fetcher.fetch(message: message, space_guids: [space_1.guid]).all
+            results = fetcher.fetch(message: message, readable_space_guids: [space_1.guid]).all
             expect(results).to match_array([service_instance_1])
           end
         end
 
+        context 'by space guid' do
+          let(:filters) { { space_guids: ['space-1'] } }
+
+          it 'only returns matching service instances' do
+            results = fetcher.fetch_all(message: message).all
+            expect(results).to match_array([service_instance_1, service_instance_2])
+            expect(results).not_to include(service_instance_3)
+          end
+        end
+
         context 'by non-existent service instance name' do
           let(:filters) { { names: ['made-up-name'] } }
 
           it 'returns no matching service instances' do
-            results = fetcher.fetch(message: message, space_guids: [space_1.guid]).all
+            results = fetcher.fetch(message: message, readable_space_guids: [space_1.guid]).all
+            expect(results).to be_empty
+          end
+        end
+
+        context 'by non-existent space guid' do
+          let(:filters) { { space_guids: ['made-up-name'] } }
+
+          it 'returns no matching service instances' do
+            results = fetcher.fetch(message: message, readable_space_guids: [space_1.guid]).all
             expect(results).to be_empty
           end
         end
@@ -79,7 +109,7 @@ module VCAP::CloudController
         end
 
         it 'returns all of the service instances shared into the specified space' do
-          results = fetcher.fetch(message: message, space_guids: [shared_to_space.guid]).all
+          results = fetcher.fetch(message: message, readable_space_guids: [shared_to_space.guid]).all
           expect(results).to match_array([service_instance_1, service_instance_2])
         end
       end
@@ -94,7 +124,7 @@ module VCAP::CloudController
         end
 
         it 'returns all of the service instances shared into the specified space' do
-          results = fetcher.fetch(message: message, space_guids: [shared_to_space.guid]).all
+          results = fetcher.fetch(message: message, readable_space_guids: [shared_to_space.guid]).all
           expect(results).to match_array([service_instance_1, service_instance_2, service_instance_4])
         end
       end

From 28d141f2c6465467a61e8bd8df78ab36623e985b Mon Sep 17 00:00:00 2001
From: Denise Yu <dyu@pivotal.io>
Date: Mon, 20 Nov 2017 14:46:09 +0000
Subject: [PATCH 2/8] Add space relationship to service instance presentation

[#152922808]

Signed-off-by: Sam Gunaratne <sgunaratne@pivotal.io>
---
 .../v3/service_instance_presenter.rb          |  18 ++-
 spec/request/service_instances_spec.rb        | 114 +++++++++++++++++-
 .../v3/service_instance_presenter_spec.rb     |   5 +
 3 files changed, 135 insertions(+), 2 deletions(-)

diff --git a/app/presenters/v3/service_instance_presenter.rb b/app/presenters/v3/service_instance_presenter.rb
index 537d1969e8a..b218ab488b0 100644
--- a/app/presenters/v3/service_instance_presenter.rb
+++ b/app/presenters/v3/service_instance_presenter.rb
@@ -9,12 +9,28 @@ def to_hash
             guid:       service_instance.guid,
             created_at: service_instance.created_at,
             updated_at: service_instance.updated_at,
-            name:      service_instance.name
+            name:      service_instance.name,
+            relationships: {
+              space: {
+                data: {
+                  guid: service_instance.space.guid
+                }
+              }
+            },
+            links: {
+              space: {
+                href: url_builder.build_url(path: "/v3/spaces/#{service_instance.space.guid}")
+              }
+            }
           }
         end
 
         private
 
+        def url_builder
+          VCAP::CloudController::Presenters::ApiUrlBuilder.new
+        end
+
         def service_instance
           @resource
         end
diff --git a/spec/request/service_instances_spec.rb b/spec/request/service_instances_spec.rb
index 5d98657fb46..4b34c57a4e7 100644
--- a/spec/request/service_instances_spec.rb
+++ b/spec/request/service_instances_spec.rb
@@ -7,14 +7,16 @@
   let(:user_header) { headers_for(user) }
   let(:admin_header) { admin_headers_for(user, email: user_email, user_name: user_name) }
   let(:space) { VCAP::CloudController::Space.make }
+  let(:another_space) { VCAP::CloudController::Space.make }
   let(:target_space) { VCAP::CloudController::Space.make }
   let!(:service_instance1) { VCAP::CloudController::ManagedServiceInstance.make(space: space, name: 'rabbitmq') }
   let!(:service_instance2) { VCAP::CloudController::ManagedServiceInstance.make(space: space, name: 'redis') }
-  let!(:service_instance3) { VCAP::CloudController::ManagedServiceInstance.make(space: space, name: 'mysql') }
+  let!(:service_instance3) { VCAP::CloudController::ManagedServiceInstance.make(space: another_space, name: 'mysql') }
 
   describe 'GET /v3/service_instances' do
     it 'returns a paginated list of service instances the user has access to' do
       set_current_user_as_role(role: 'space_developer', org: space.organization, space: space, user: user)
+      set_current_user_as_role(role: 'space_developer', org: another_space.organization, space: another_space, user: user)
       get '/v3/service_instances?per_page=2&order_by=name', nil, user_header
       expect(last_response.status).to eq(200)
 
@@ -41,12 +43,36 @@
               'name' => service_instance3.name,
               'created_at' => iso8601,
               'updated_at' => iso8601,
+              'relationships' => {
+                'space' => {
+                  'data' => {
+                    'guid' => service_instance3.space.guid
+                  }
+                }
+              },
+              'links' => {
+                'space' => {
+                  'href' => "#{link_prefix}/v3/spaces/#{service_instance3.space.guid}"
+                }
+              }
             },
             {
               'guid' => service_instance1.guid,
               'name' => service_instance1.name,
               'created_at' => iso8601,
               'updated_at' => iso8601,
+              'relationships' => {
+                'space' => {
+                  'data' => {
+                    'guid' => service_instance1.space.guid
+                  }
+                }
+              },
+              'links' => {
+                'space' => {
+                  'href' => "#{link_prefix}/v3/spaces/#{service_instance1.space.guid}"
+                }
+              }
             }
           ]
         }
@@ -79,6 +105,80 @@
               'name' => service_instance2.name,
               'created_at' => iso8601,
               'updated_at' => iso8601,
+              'relationships' => {
+                'space' => {
+                  'data' => {
+                    'guid' => service_instance2.space.guid
+                  }
+                }
+              },
+              'links' => {
+                'space' => {
+                  'href' => "#{link_prefix}/v3/spaces/#{service_instance2.space.guid}"
+                }
+              }
+            }
+          ]
+        }
+      )
+    end
+
+    it 'returns a paginated list of service instances filtered by space guid' do
+      set_current_user_as_role(role: 'space_developer', org: space.organization, space: space, user: user)
+      get "/v3/service_instances?per_page=2&space_guids=#{space.guid}", nil, user_header
+      expect(last_response.status).to eq(200)
+
+      parsed_response = MultiJson.load(last_response.body)
+      expect(parsed_response).to be_a_response_like(
+        {
+          'pagination' => {
+            'total_results' => 2,
+            'total_pages' => 1,
+            'first' => {
+              'href' => "#{link_prefix}/v3/service_instances?page=1&per_page=2&space_guids=#{space.guid}"
+            },
+            'last' => {
+              'href' => "#{link_prefix}/v3/service_instances?page=1&per_page=2&space_guids=#{space.guid}"
+            },
+            'next' => nil,
+            'previous' => nil
+          },
+          'resources' => [
+            {
+              'guid' => service_instance1.guid,
+              'name' => service_instance1.name,
+              'created_at' => iso8601,
+              'updated_at' => iso8601,
+              'relationships' => {
+                'space' => {
+                  'data' => {
+                   'guid' => service_instance1.space.guid
+                  }
+                }
+              },
+              'links' => {
+                'space' => {
+                  'href' => "#{link_prefix}/v3/spaces/#{service_instance1.space.guid}"
+                }
+              }
+            },
+            {
+              'guid' => service_instance2.guid,
+              'name' => service_instance2.name,
+              'created_at' => iso8601,
+              'updated_at' => iso8601,
+              'relationships' => {
+                'space' => {
+                  'data' => {
+                    'guid' => service_instance2.space.guid
+                  }
+                }
+              },
+              'links' => {
+                'space' => {
+                  'href' => "#{link_prefix}/v3/spaces/#{service_instance2.space.guid}"
+                }
+              }
             }
           ]
         }
@@ -116,6 +216,18 @@
                 'name' => service_instance1.name,
                 'created_at' => iso8601,
                 'updated_at' => iso8601,
+                'relationships' => {
+                  'space' => {
+                    'data' => {
+                      'guid' => service_instance1.space.guid
+                    }
+                  }
+                },
+                'links' => {
+                  'space' => {
+                    'href' => "#{link_prefix}/v3/spaces/#{service_instance1.space.guid}"
+                  }
+                }
               }
             ]
           }
diff --git a/spec/unit/presenters/v3/service_instance_presenter_spec.rb b/spec/unit/presenters/v3/service_instance_presenter_spec.rb
index 58c595f90a4..292faa22691 100644
--- a/spec/unit/presenters/v3/service_instance_presenter_spec.rb
+++ b/spec/unit/presenters/v3/service_instance_presenter_spec.rb
@@ -14,6 +14,11 @@ module VCAP::CloudController::Presenters::V3
         expect(result[:created_at]).to eq(service_instance.created_at)
         expect(result[:updated_at]).to eq(service_instance.updated_at)
         expect(result[:name]).to eq('denise-db')
+        expect(result[:relationships][:space][:data][:guid]).to equal(service_instance.space.guid)
+      end
+
+      it 'has a links hash with a space url' do
+        expect(result[:links][:space][:href]).to eq "#{link_prefix}/v3/spaces/#{service_instance.space.guid}"
       end
     end
   end

From afa2108b324ae486274991ff69e6ffc75a14855c Mon Sep 17 00:00:00 2001
From: Sam Gunaratne <sgunaratne@pivotal.io>
Date: Mon, 20 Nov 2017 15:44:06 +0000
Subject: [PATCH 3/8] Add docs for service instance space relationship

[#152922808]

Signed-off-by: Denise Yu <dyu@pivotal.io>
---
 .../api_resources/_service_instances.erb      | 20 +++++++++++++++----
 .../service_instances/_object.md.erb          | 10 ++++++++++
 docs/v3/source/index.md                       |  1 +
 3 files changed, 27 insertions(+), 4 deletions(-)
 create mode 100644 docs/v3/source/includes/experimental_resources/service_instances/_object.md.erb

diff --git a/docs/v3/source/includes/api_resources/_service_instances.erb b/docs/v3/source/includes/api_resources/_service_instances.erb
index a9a15f49183..81c52eae9b9 100644
--- a/docs/v3/source/includes/api_resources/_service_instances.erb
+++ b/docs/v3/source/includes/api_resources/_service_instances.erb
@@ -35,10 +35,22 @@
   },
   "resources": [
     {
-      "guid": "d4c91047-7b29-4fda-b7f9-04033e5c9c9f",
-      "created_at": "2017-02-02T00:14:30Z",
-      "updated_at": "2017-02-02T00:14:30Z",
-      "name": "my_service_instance"
+      "guid": "85ccdcad-d725-4109-bca4-fd6ba062b5c8",
+      "created_at": "2017-11-17T13:54:21Z",
+      "updated_at": "2017-11-17T13:54:21Z",
+      "name": "my_service_instance",
+      "relationships": {
+        "space": {
+          "data": {
+            "guid": "ae0031f9-dd49-461c-a945-df40e77c39cb"
+          }
+        }
+      },
+      "links": {
+        "space": {
+          "href": "https://api.example.org/v3/spaces/ae0031f9-dd49-461c-a945-df40e77c39cb"
+        }
+      }
     }
   ]
 }
diff --git a/docs/v3/source/includes/experimental_resources/service_instances/_object.md.erb b/docs/v3/source/includes/experimental_resources/service_instances/_object.md.erb
new file mode 100644
index 00000000000..fdb3ed1c886
--- /dev/null
+++ b/docs/v3/source/includes/experimental_resources/service_instances/_object.md.erb
@@ -0,0 +1,10 @@
+### The service_instance object
+
+Name | Type | Description
+---- | ---- | -----------
+**name** | _string_ | Name of the service instance.
+**guid** | _uuid_ | Unique identifier for the service instance.
+**created_at** | _datetime_ | The time with zone when the object was created.
+**updated_at** | _datetime_ | The time with zone when the object was last updated.
+**space** | [_to-one relationship_](#to-one-relationships) | The space the service instance is contained in.
+**links** | [_links object_](#links) | Links to related resources.
diff --git a/docs/v3/source/index.md b/docs/v3/source/index.md
index 9028e760027..1c48ded85a1 100644
--- a/docs/v3/source/index.md
+++ b/docs/v3/source/index.md
@@ -137,6 +137,7 @@ includes:
   - experimental_resources/service_bindings/delete
   - experimental_resources/service_bindings/list
   - experimental_resources/service_instances/header
+  - experimental_resources/service_instances/object
   - experimental_resources/service_instances/list
   - experimental_resources/service_instances/share_to_space
   - experimental_resources/service_instances/unshare_from_space

From 87dbe9f5cb2959fef471ab6157dab4ce4d513d76 Mon Sep 17 00:00:00 2001
From: Sam Gunaratne <sgunaratne@pivotal.io>
Date: Tue, 21 Nov 2017 14:35:53 +0000
Subject: [PATCH 4/8] Additional test for combining query params on v3 service
 instances

[#152922808]

Signed-off-by: Denise Yu <dyu@pivotal.io>
---
 .../service_instance_list_fetcher_spec.rb     | 29 +++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/spec/unit/queries/service_instance_list_fetcher_spec.rb b/spec/unit/queries/service_instance_list_fetcher_spec.rb
index 193ef08ac33..c9005b3291a 100644
--- a/spec/unit/queries/service_instance_list_fetcher_spec.rb
+++ b/spec/unit/queries/service_instance_list_fetcher_spec.rb
@@ -44,6 +44,21 @@ module VCAP::CloudController
             expect(results).not_to include(service_instance_1)
           end
         end
+
+        context 'by all query params' do
+          let!(:service_instance_4) { ManagedServiceInstance.make(name: 'couchdb', space: service_instance_3.space) }
+          let(:filters) {
+            {
+              space_guids: ['space-3'],
+              names: ['couchdb'],
+            }
+          }
+
+          it 'only returns matching service instances' do
+            results = fetcher.fetch_all(message: message).all
+            expect(results).to match_array([service_instance_4])
+          end
+        end
       end
     end
 
@@ -81,6 +96,20 @@ module VCAP::CloudController
           end
         end
 
+        context 'by all query params' do
+          let(:filters) {
+            {
+              space_guids: ['space-1'],
+              names: ['rabbitmq'],
+            }
+          }
+
+          it 'only returns matching service instances' do
+            results = fetcher.fetch_all(message: message).all
+            expect(results).to match_array([service_instance_1])
+          end
+        end
+
         context 'by non-existent service instance name' do
           let(:filters) { { names: ['made-up-name'] } }
 

From 3eb64144c78125c9cec37a4223d510d0a5090823 Mon Sep 17 00:00:00 2001
From: Jen Spinney <jennifer.spinney@suse.com>
Date: Mon, 27 Nov 2017 11:42:36 +0000
Subject: [PATCH 5/8] /v3/service_instances does not return user provided
 services

* Add additional tests for user provided services and service instance sharing

Signed-off-by: Sam Gunaratne <sgunaratne@pivotal.io>
---
 .../v3/service_instances_controller.rb        |  6 +++---
 ... managed_service_instance_list_fetcher.rb} |  8 ++++----
 .../v3/service_instances_controller_spec.rb   | 15 ++++++++++++--
 ...ged_service_instance_list_fetcher_spec.rb} | 20 ++++++++++---------
 4 files changed, 31 insertions(+), 18 deletions(-)
 rename app/fetchers/{service_instance_list_fetcher.rb => managed_service_instance_list_fetcher.rb} (77%)
 rename spec/unit/queries/{service_instance_list_fetcher_spec.rb => managed_service_instance_list_fetcher_spec.rb} (87%)

diff --git a/app/controllers/v3/service_instances_controller.rb b/app/controllers/v3/service_instances_controller.rb
index 0bdf8c2cf39..5a44bd809e8 100644
--- a/app/controllers/v3/service_instances_controller.rb
+++ b/app/controllers/v3/service_instances_controller.rb
@@ -6,7 +6,7 @@
 require 'presenters/v3/paginated_list_presenter'
 require 'actions/service_instance_share'
 require 'actions/service_instance_unshare'
-require 'fetchers/service_instance_list_fetcher'
+require 'fetchers/managed_service_instance_list_fetcher'
 
 class ServiceInstancesV3Controller < ApplicationController
   def index
@@ -14,9 +14,9 @@ def index
     invalid_param!(message.errors.full_messages) unless message.valid?
 
     dataset = if can_read_globally?
-                ServiceInstanceListFetcher.new.fetch_all(message: message)
+                ManagedServiceInstanceListFetcher.new.fetch_all(message: message)
               else
-                ServiceInstanceListFetcher.new.fetch(message: message, readable_space_guids: readable_space_guids)
+                ManagedServiceInstanceListFetcher.new.fetch(message: message, readable_space_guids: readable_space_guids)
               end
 
     render status: :ok, json: Presenters::V3::PaginatedListPresenter.new(
diff --git a/app/fetchers/service_instance_list_fetcher.rb b/app/fetchers/managed_service_instance_list_fetcher.rb
similarity index 77%
rename from app/fetchers/service_instance_list_fetcher.rb
rename to app/fetchers/managed_service_instance_list_fetcher.rb
index ee0e0aa5159..adab35171fd 100644
--- a/app/fetchers/service_instance_list_fetcher.rb
+++ b/app/fetchers/managed_service_instance_list_fetcher.rb
@@ -1,10 +1,10 @@
 module VCAP::CloudController
-  class ServiceInstanceListFetcher
+  class ManagedServiceInstanceListFetcher
     def fetch(message:, readable_space_guids:)
-      source_space_instance_dataset = ServiceInstance.select_all(ServiceInstance.table_name).
+      source_space_instance_dataset = ManagedServiceInstance.select_all(ServiceInstance.table_name).
                                       join(Space.table_name, id: :space_id, guid: readable_space_guids)
 
-      shared_instance_dataset = ServiceInstance.select_all(ServiceInstance.table_name).
+      shared_instance_dataset = ManagedServiceInstance.select_all(ServiceInstance.table_name).
                                 join(:service_instance_shares, service_instance_guid: :guid, target_space_guid: readable_space_guids)
 
       dataset = source_space_instance_dataset.union(shared_instance_dataset, alias: :service_instances)
@@ -13,7 +13,7 @@ def fetch(message:, readable_space_guids:)
     end
 
     def fetch_all(message:)
-      dataset = ServiceInstance.dataset
+      dataset = ManagedServiceInstance.dataset
       filter(dataset, message)
     end
 
diff --git a/spec/unit/controllers/v3/service_instances_controller_spec.rb b/spec/unit/controllers/v3/service_instances_controller_spec.rb
index f59bfd58f1b..106c79c8b06 100644
--- a/spec/unit/controllers/v3/service_instances_controller_spec.rb
+++ b/spec/unit/controllers/v3/service_instances_controller_spec.rb
@@ -9,13 +9,14 @@
     context 'when there are multiple service instances' do
       let!(:service_instance2) { VCAP::CloudController::ManagedServiceInstance.make(name: 'service-instance-2', space: VCAP::CloudController::Space.make(guid: 'space-2-guid')) }
       let!(:service_instance3) { VCAP::CloudController::ManagedServiceInstance.make(name: 'service-instance-3', space: VCAP::CloudController::Space.make(guid: 'space-3-guid')) }
+      let!(:user_provided_service_instance) { VCAP::CloudController::UserProvidedServiceInstance.make }
 
       context 'as an admin' do
         before do
           set_current_user_as_admin
         end
 
-        it 'returns all service instances' do
+        it 'returns all managed service instances' do
           get :index
           expect(response.status).to eq(200), response.body
           expect(parsed_body['resources'].length).to eq 3
@@ -52,7 +53,7 @@
           set_current_user_as_role(role: 'space_developer', org: space.organization, space: space, user: user)
         end
 
-        it 'returns a subset of service instances' do
+        it 'returns a subset of managed service instances' do
           get :index
           expect(response.status).to eq(200), response.body
           expect(parsed_body['resources'].length).to eq 1
@@ -207,6 +208,16 @@
       end
     end
 
+    context 'when the service instance is user provided' do
+      let(:service_instance) { VCAP::CloudController::UserProvidedServiceInstance.make }
+
+      it 'returns a 400' do
+        post :share_service_instance, service_instance_guid: service_instance.guid, body: req_body
+        expect(response.status).to eq 400
+        expect(response.body).to include('User-provided services cannot be shared')
+      end
+    end
+
     context 'when the target space does not exist' do
       before do
         req_body[:data] = [{ guid: 'nonexistant-space-guid' }]
diff --git a/spec/unit/queries/service_instance_list_fetcher_spec.rb b/spec/unit/queries/managed_service_instance_list_fetcher_spec.rb
similarity index 87%
rename from spec/unit/queries/service_instance_list_fetcher_spec.rb
rename to spec/unit/queries/managed_service_instance_list_fetcher_spec.rb
index c9005b3291a..b08104293f3 100644
--- a/spec/unit/queries/service_instance_list_fetcher_spec.rb
+++ b/spec/unit/queries/managed_service_instance_list_fetcher_spec.rb
@@ -1,24 +1,25 @@
 require 'spec_helper'
-require 'fetchers/service_instance_list_fetcher'
+require 'fetchers/managed_service_instance_list_fetcher'
 require 'messages/service_instances/service_instances_list_message'
 
 module VCAP::CloudController
-  RSpec.describe ServiceInstanceListFetcher do
+  RSpec.describe ManagedServiceInstanceListFetcher do
     let(:filters) { {} }
     let(:message) { ServiceInstancesListMessage.new(filters) }
-    let(:fetcher) { ServiceInstanceListFetcher.new }
+    let(:fetcher) { ManagedServiceInstanceListFetcher.new }
 
     describe '#fetch_all' do
       let!(:service_instance_1) { ManagedServiceInstance.make(name: 'rabbitmq', space: Space.make(guid: 'space-1')) }
       let!(:service_instance_2) { ManagedServiceInstance.make(name: 'redis', space: Space.make(guid: 'space-2')) }
       let!(:service_instance_3) { ManagedServiceInstance.make(name: 'mysql', space: Space.make(guid: 'space-3')) }
+      let!(:user_provided_service_instance) { UserProvidedServiceInstance.make(name: 'my-thing') }
 
       it 'returns a Sequel::Dataset' do
         results = fetcher.fetch_all(message: message)
         expect(results).to be_a(Sequel::Dataset)
       end
 
-      it 'includes all the V3 Service Instances' do
+      it 'includes all the managed service instances' do
         results = fetcher.fetch_all(message: message).all
         expect(results.length).to eq 3
         expect(results).to include(service_instance_1, service_instance_2, service_instance_3)
@@ -28,7 +29,7 @@ module VCAP::CloudController
         context 'by service instance name' do
           let(:filters) { { names: ['rabbitmq', 'redis'] } }
 
-          it 'only returns matching service instances' do
+          it 'only returns matching managed service instances' do
             results = fetcher.fetch_all(message: message).all
             expect(results).to match_array([service_instance_1, service_instance_2])
             expect(results).not_to include(service_instance_3)
@@ -66,11 +67,12 @@ module VCAP::CloudController
       let!(:service_instance_1) { ManagedServiceInstance.make(name: 'rabbitmq', space: space_1) }
       let!(:service_instance_2) { ManagedServiceInstance.make(name: 'redis', space: space_1) }
       let!(:service_instance_3) { ManagedServiceInstance.make(name: 'mysql', space: space_2) }
+      let!(:user_provided_service_instance) { UserProvidedServiceInstance.make(name: 'my-thing', space: space_1) }
 
       let(:space_1) { Space.make(guid: 'space-1') }
       let(:space_2) { Space.make(guid: 'space-2') }
 
-      it 'returns all of the service instances in the specified space' do
+      it 'returns all of the managed service instances in the specified space' do
         results = fetcher.fetch(message: message, readable_space_guids: [space_1.guid]).all
 
         expect(results).to match_array([service_instance_1, service_instance_2])
@@ -104,8 +106,8 @@ module VCAP::CloudController
             }
           }
 
-          it 'only returns matching service instances' do
-            results = fetcher.fetch_all(message: message).all
+          it 'only returns matching managed service instances' do
+            results = fetcher.fetch(message: message, readable_space_guids: [space_1.guid]).all
             expect(results).to match_array([service_instance_1])
           end
         end
@@ -129,7 +131,7 @@ module VCAP::CloudController
         end
       end
 
-      context 'when service instances are shared' do
+      context 'when managed service instances are shared' do
         let(:shared_to_space) { Space.make }
 
         before do

From f219ad5992377970feaf06a410177a6375234057 Mon Sep 17 00:00:00 2001
From: Sam Gunaratne <sgunaratne@pivotal.io>
Date: Tue, 28 Nov 2017 15:24:40 +0000
Subject: [PATCH 6/8] Well-formed but invalid share requests should return 422

422 Unprocessable is a more appropriate error message as
the syntax of the request is correct, however the semantics of the
request are wrong.

[#153176525]

Signed-off-by: Jen Spinney <jennifer.spinney@suse.com>
---
 spec/request/service_instances_spec.rb        | 22 ----------
 spec/request/v2/service_instances_spec.rb     |  2 +-
 .../service_instances_controller_spec.rb      |  4 +-
 .../v3/service_instances_controller_spec.rb   | 40 ++++++++++++++++++-
 vendor/errors/v2.yml                          | 10 ++---
 5 files changed, 46 insertions(+), 32 deletions(-)

diff --git a/spec/request/service_instances_spec.rb b/spec/request/service_instances_spec.rb
index 4b34c57a4e7..4f49a28dfe1 100644
--- a/spec/request/service_instances_spec.rb
+++ b/spec/request/service_instances_spec.rb
@@ -280,28 +280,6 @@
       })
       expect(event.metadata['target_space_guids']).to eq([target_space.guid])
     end
-
-    context 'when the service offering has shareable false' do
-      before do
-        service_instance1.service.extra = { shareable: false }.to_json
-        service_instance1.service.save
-      end
-
-      it 'fails to share' do
-        share_request = {
-          'data' => [
-            { 'guid' => target_space.guid }
-          ]
-        }
-
-        post "/v3/service_instances/#{service_instance1.guid}/relationships/shared_spaces", share_request.to_json, admin_header
-
-        expect(last_response.status).to eq(400)
-        parsed_response = MultiJson.load(last_response.body)
-        expect(parsed_response['errors'].first['code']).to eq(390003)
-        expect(parsed_response['errors'].first['title']).to eq('CF-ServiceShareIsDisabled')
-      end
-    end
   end
 
   describe 'DELETE /v3/service_instances/:guid/relationships/shared_spaces/:space-guid' do
diff --git a/spec/request/v2/service_instances_spec.rb b/spec/request/v2/service_instances_spec.rb
index cb4ba70c566..d03b3a8bc0c 100644
--- a/spec/request/v2/service_instances_spec.rb
+++ b/spec/request/v2/service_instances_spec.rb
@@ -262,7 +262,7 @@
       it 'fails with an appropriate response' do
         delete "v2/service_instances/#{service_instance.guid}", nil, admin_headers
 
-        expect(last_response.status).to eq(400)
+        expect(last_response.status).to eq(422)
 
         parsed_response = MultiJson.load(last_response.body)
         expect(parsed_response['description']).to eq 'Service instances must be unshared before they can be deleted. ' \
diff --git a/spec/unit/controllers/services/service_instances_controller_spec.rb b/spec/unit/controllers/services/service_instances_controller_spec.rb
index b49a9c50bc0..328123300c3 100644
--- a/spec/unit/controllers/services/service_instances_controller_spec.rb
+++ b/spec/unit/controllers/services/service_instances_controller_spec.rb
@@ -2574,7 +2574,7 @@ def stub_delete_and_return(status, body)
             it 'should give the user an error' do
               delete "/v2/service_instances/#{service_instance.guid}"
 
-              expect(last_response).to have_status_code 400
+              expect(last_response).to have_status_code 422
               expect(last_response.body).to include 'ServiceInstanceDeletionSharesExists'
               expect(last_response.body).to include(
                 'Service instances must be unshared before they can be deleted. ' \
@@ -2600,7 +2600,7 @@ def stub_delete_and_return(status, body)
               it 'should give the user an error' do
                 delete "/v2/service_instances/#{service_instance.guid}"
 
-                expect(last_response).to have_status_code 400
+                expect(last_response).to have_status_code 422
                 expect(last_response.body).to include 'ServiceInstanceDeletionSharesExists'
                 expect(last_response.body).to include(
                   'Service instances must be unshared before they can be deleted. ' \
diff --git a/spec/unit/controllers/v3/service_instances_controller_spec.rb b/spec/unit/controllers/v3/service_instances_controller_spec.rb
index 106c79c8b06..a49195b5e79 100644
--- a/spec/unit/controllers/v3/service_instances_controller_spec.rb
+++ b/spec/unit/controllers/v3/service_instances_controller_spec.rb
@@ -211,13 +211,49 @@
     context 'when the service instance is user provided' do
       let(:service_instance) { VCAP::CloudController::UserProvidedServiceInstance.make }
 
-      it 'returns a 400' do
+      it 'returns a 422' do
         post :share_service_instance, service_instance_guid: service_instance.guid, body: req_body
-        expect(response.status).to eq 400
+        expect(response.status).to eq 422
         expect(response.body).to include('User-provided services cannot be shared')
       end
     end
 
+    context 'when the service instance is a route service' do
+      let(:service) { VCAP::CloudController::Service.make(:routing) }
+      let(:service_plan) { VCAP::CloudController::ServicePlan.make(service: service) }
+      let(:service_instance) { VCAP::CloudController::ManagedServiceInstance.make(service_plan: service_plan) }
+
+      it 'returns a 422' do
+        post :share_service_instance, service_instance_guid: service_instance.guid, body: req_body
+        expect(response.status).to eq 422
+        expect(response.body).to include('Route services cannot be shared')
+      end
+    end
+
+    context 'when the target space has a service instance with the same name' do
+      before do
+        target_space.add_service_instance(VCAP::CloudController::ManagedServiceInstance.make(name: service_instance.name))
+      end
+
+      it 'returns a 422' do
+        post :share_service_instance, service_instance_guid: service_instance.guid, body: req_body
+        expect(response.status).to eq 422
+        expect(response.body).to include("A service instance called #{service_instance.name} already exists in #{target_space.name}")
+      end
+    end
+
+    context 'when the service is not shareable' do
+      let(:service) { VCAP::CloudController::Service.make(extra: { shareable: false }.to_json) }
+      let(:service_plan) { VCAP::CloudController::ServicePlan.make(service: service) }
+      let(:service_instance) { VCAP::CloudController::ManagedServiceInstance.make(service_plan: service_plan) }
+
+      it 'returns a 422' do
+        post :share_service_instance, service_instance_guid: service_instance.guid, body: req_body
+        expect(response.status).to eq 422
+        expect(response.body).to include("The #{service.label} service does not support service instance sharing.")
+      end
+    end
+
     context 'when the target space does not exist' do
       before do
         req_body[:data] = [{ guid: 'nonexistant-space-guid' }]
diff --git a/vendor/errors/v2.yml b/vendor/errors/v2.yml
index b35cf9bc8bc..75445319ae3 100644
--- a/vendor/errors/v2.yml
+++ b/vendor/errors/v2.yml
@@ -1111,27 +1111,27 @@
 
 390002:
   name: ServiceInstanceDeletionSharesExists
-  http_code: 400
+  http_code: 422
   message: "Service instances must be unshared before they can be deleted. Unsharing %s will automatically delete any bindings that have been made to applications in other spaces."
 
 390003:
   name: ServiceShareIsDisabled
-  http_code: 400
+  http_code: 422
   message: "The %s service does not support service instance sharing."
 
 390004:
   name: UserProvidedServiceInstanceSharingNotSupported
-  http_code: 400
+  http_code: 422
   message: "User-provided services cannot be shared"
 
 390005:
   name: RouteServiceInstanceSharingNotSupported
-  http_code: 400
+  http_code: 422
   message: "Route services cannot be shared"
 
 390006:
   name: SharedServiceInstanceNameTaken
-  http_code: 400
+  http_code: 422
   message: "A service instance called %s already exists in %s"
 
 390007:

From b4010d69abde4a3fb24d64c9fa229db4d2c36075 Mon Sep 17 00:00:00 2001
From: Denise Yu <dyu@pivotal.io>
Date: Wed, 29 Nov 2017 10:51:15 +0000
Subject: [PATCH 7/8] Shared service instance information includes space_guid

* This allows developers who does not have cross space access to
discover the space_guid when using service instance sharing.

[#153244755]

Signed-off-by: Sam Gunaratne <sgunaratne@pivotal.io>
---
 .../v2/service_instance_shared_from_presenter.rb      |  1 +
 spec/request/v2/service_instances_spec.rb             |  4 ++++
 .../services/service_instances_controller_spec.rb     | 11 ++++++++---
 .../v2/service_instance_shared_from_presenter_spec.rb |  1 +
 .../v2/service_instance_shared_to_presenter_spec.rb   |  1 +
 5 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/app/presenters/v2/service_instance_shared_from_presenter.rb b/app/presenters/v2/service_instance_shared_from_presenter.rb
index 405ac03c3bd..1de2def9d0b 100644
--- a/app/presenters/v2/service_instance_shared_from_presenter.rb
+++ b/app/presenters/v2/service_instance_shared_from_presenter.rb
@@ -4,6 +4,7 @@ module V2
       class ServiceInstanceSharedFromPresenter
         def to_hash(space)
           {
+            'space_guid' => space.guid,
             'space_name' => space.name,
             'organization_name' => space.organization.name
           }
diff --git a/spec/request/v2/service_instances_spec.rb b/spec/request/v2/service_instances_spec.rb
index d03b3a8bc0c..4a8a289fd8e 100644
--- a/spec/request/v2/service_instances_spec.rb
+++ b/spec/request/v2/service_instances_spec.rb
@@ -166,6 +166,7 @@
 
       parsed_response = MultiJson.load(last_response.body)
       expect(parsed_response).to be_a_response_like({
+        'space_guid' => space.guid,
         'space_name' => space.name,
         'organization_name' => space.organization.name
       })
@@ -199,6 +200,7 @@
 
         parsed_response = MultiJson.load(last_response.body)
         expect(parsed_response).to be_a_response_like({
+          'space_guid' => space.guid,
           'space_name' => space.name,
           'organization_name' => space.organization.name
         })
@@ -230,11 +232,13 @@
           'next_url' => nil,
           'resources' => [
             {
+              'space_guid' => space1.guid,
               'space_name' => space1.name,
               'organization_name' => space1.organization.name,
               'bound_app_count' => 0
             },
             {
+              'space_guid' => space2.guid,
               'space_name' => space2.name,
               'organization_name' => space2.organization.name,
               'bound_app_count' => 0
diff --git a/spec/unit/controllers/services/service_instances_controller_spec.rb b/spec/unit/controllers/services/service_instances_controller_spec.rb
index 328123300c3..cc684cae503 100644
--- a/spec/unit/controllers/services/service_instances_controller_spec.rb
+++ b/spec/unit/controllers/services/service_instances_controller_spec.rb
@@ -3848,9 +3848,11 @@ def verify_forbidden(user)
           get "/v2/service_instances/#{instance.guid}/shared_from"
           expect(last_response.status).to eql(200), last_response.body
           parsed_response = JSON.parse(last_response.body)
+
+          expect(parsed_response.keys).to match_array(['space_name', 'space_guid', 'organization_name'])
           expect(parsed_response['space_name']).to eq(space.name)
+          expect(parsed_response['space_guid']).to eq(space.guid)
           expect(parsed_response['organization_name']).to eq(space.organization.name)
-          expect(parsed_response.keys).to match_array(['space_name', 'organization_name'])
         end
 
         describe 'permissions' do
@@ -3961,12 +3963,15 @@ def verify_forbidden(user)
           space1_resource = resources.find { |resource| resource['space_name'] == space1.name }
           space2_resource = resources.find { |resource| resource['space_name'] == space2.name }
 
-          expect(space1_resource.keys).to match_array(['space_name', 'organization_name', 'bound_app_count'])
-          expect(space2_resource.keys).to match_array(['space_name', 'organization_name', 'bound_app_count'])
+          expect(space1_resource.keys).to match_array(['space_name', 'space_guid', 'organization_name', 'bound_app_count'])
+          expect(space2_resource.keys).to match_array(['space_name', 'space_guid', 'organization_name', 'bound_app_count'])
 
           expect(space1_resource.fetch('space_name')).to eq(space1.name)
           expect(space2_resource.fetch('space_name')).to eq(space2.name)
 
+          expect(space1_resource.fetch('space_guid')).to eq(space1.guid)
+          expect(space2_resource.fetch('space_guid')).to eq(space2.guid)
+
           expect(space1_resource.fetch('organization_name')).to eq(space1.organization.name)
           expect(space2_resource.fetch('organization_name')).to eq(space2.organization.name)
 
diff --git a/spec/unit/presenters/v2/service_instance_shared_from_presenter_spec.rb b/spec/unit/presenters/v2/service_instance_shared_from_presenter_spec.rb
index 8c18d57602e..3476f2688e4 100644
--- a/spec/unit/presenters/v2/service_instance_shared_from_presenter_spec.rb
+++ b/spec/unit/presenters/v2/service_instance_shared_from_presenter_spec.rb
@@ -8,6 +8,7 @@ module CloudController::Presenters::V2
         presenter = ServiceInstanceSharedFromPresenter.new
         expect(presenter.to_hash(space)).to eq(
           {
+            'space_guid' => space.guid,
             'space_name' => space.name,
             'organization_name' => space.organization.name,
           }
diff --git a/spec/unit/presenters/v2/service_instance_shared_to_presenter_spec.rb b/spec/unit/presenters/v2/service_instance_shared_to_presenter_spec.rb
index 8119cfd3c69..fbd6d6e7fce 100644
--- a/spec/unit/presenters/v2/service_instance_shared_to_presenter_spec.rb
+++ b/spec/unit/presenters/v2/service_instance_shared_to_presenter_spec.rb
@@ -8,6 +8,7 @@ module CloudController::Presenters::V2
         presenter = ServiceInstanceSharedToPresenter.new
         expect(presenter.to_hash(space, 42)).to eq(
           {
+            'space_guid' => space.guid,
             'space_name' => space.name,
             'organization_name' => space.organization.name,
             'bound_app_count' => 42

From dd181779f5efbae61f5309e7b0cf8727b32093d9 Mon Sep 17 00:00:00 2001
From: Denise Yu <dyu@pivotal.io>
Date: Thu, 30 Nov 2017 13:58:44 +0000
Subject: [PATCH 8/8] Disable renaming a shared service instance

* This could introduce a name clash in spaces receiving a shared
instance

[#152109574]
---
 .../services/service_instances_controller.rb  |  9 +++++++++
 .../service_instances_controller_spec.rb      | 19 ++++++++++++++++++-
 vendor/errors/v2.yml                          |  5 +++++
 3 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/app/controllers/services/service_instances_controller.rb b/app/controllers/services/service_instances_controller.rb
index 7810b621e32..95e31e8d853 100644
--- a/app/controllers/services/service_instances_controller.rb
+++ b/app/controllers/services/service_instances_controller.rb
@@ -122,6 +122,7 @@ def update(guid)
       validate_access(:read_for_update, service_instance)
       validate_access(:update, projected_service_instance(service_instance))
 
+      validate_name_update(service_instance)
       validate_space_update(related_objects[:space])
       validate_plan_update(related_objects[:plan], related_objects[:service], service_instance)
 
@@ -432,6 +433,14 @@ def validate_space_update(space)
       space_change_not_allowed! if space_change_requested?(request_attrs['space_guid'], space)
     end
 
+    def validate_name_update(service_instance)
+      return unless request_attrs['name'] && service_instance.shared?
+
+      if request_attrs['name'] != service_instance.name
+        raise CloudController::Errors::ApiError.new_from_details('SharedServiceInstanceCannotBeRenamed')
+      end
+    end
+
     def invalid_service_instance!(service_instance)
       raise Sequel::ValidationFailed.new(service_instance)
     end
diff --git a/spec/unit/controllers/services/service_instances_controller_spec.rb b/spec/unit/controllers/services/service_instances_controller_spec.rb
index cc684cae503..90d8f4c4ee7 100644
--- a/spec/unit/controllers/services/service_instances_controller_spec.rb
+++ b/spec/unit/controllers/services/service_instances_controller_spec.rb
@@ -1742,13 +1742,30 @@ def stub_delete_and_return(status, body)
             service_instance.add_shared_space(shared_to_space)
           end
 
-          context 'and a developer in the originating space tries to update the instance' do
+          context 'and a developer in the originating space tries to update the instance without renaming' do
             it 'updates successfully' do
               put "/v2/service_instances/#{service_instance.guid}", body
               expect(last_response).to have_status_code 201
             end
           end
 
+          context 'and a developer in the originating space tries to rename the instance' do
+            let(:body) do
+              {
+                name: 'dont-rename-me',
+                tags: []
+              }.to_json
+            end
+
+            it 'fails and returns error that service instance cannot be renamed after sharing' do
+              put "/v2/service_instances/#{service_instance.guid}", body
+
+              expect(last_response).to have_status_code(422)
+              expect(decoded_response['code']).to eq(390008)
+              expect(decoded_response['error_code']).to eq('CF-SharedServiceInstanceCannotBeRenamed')
+            end
+          end
+
           context 'and a developer in the shared to space tries to update the instance' do
             before do
               set_current_user(shared_to_user)
diff --git a/vendor/errors/v2.yml b/vendor/errors/v2.yml
index 75445319ae3..47f0498e0e0 100644
--- a/vendor/errors/v2.yml
+++ b/vendor/errors/v2.yml
@@ -1138,3 +1138,8 @@
   name: InvalidServiceInstanceSharingTargetSpace
   http_code: 422
   message: 'Service instances cannot be shared into the space where they were created'
+
+390008:
+  name: SharedServiceInstanceCannotBeRenamed
+  http_code: 422
+  message: 'Service instances that have been shared cannot be renamed'