Implement GET /v3/service_instances/:guid/relationships/shared_spaces

app/actions/service_binding_create.rb

@@ -63,8 +63,7 @@ def mitigate_orphan(binding)
     end
 
     def bindable_in_space?(service_instance, app_space)
-      service_instance.space == app_space ||
-        (FeatureFlag.enabled?(:service_instance_sharing) && service_instance.shared_spaces.include?(app_space))
+      service_instance.space == app_space || service_instance.shared_spaces.include?(app_space)
     end
 
     def logger

app/actions/service_instance_share.rb

@@ -3,9 +3,9 @@
 module VCAP::CloudController
   class ServiceInstanceShare
     def create(service_instance, target_spaces, user_audit_info)
-      supported_service_type!(service_instance)
-      service_instance_shareable!(service_instance)
-      valid_target_spaces!(service_instance, target_spaces)
+      validate_supported_service_type!(service_instance)
+      validate_service_instance_is_shareable!(service_instance)
+      validate_target_spaces!(service_instance, target_spaces)
 
       ServiceInstance.db.transaction do
         target_spaces.each do |space|
@@ -21,37 +21,43 @@ def create(service_instance, target_spaces, user_audit_info)
 
     private
 
-    def valid_target_spaces!(service_instance, target_spaces)
-      no_sharing_to_self!(service_instance, target_spaces)
+    def validate_target_spaces!(service_instance, target_spaces)
+      validate_not_sharing_to_self!(service_instance, target_spaces)
+      validate_plan_is_active!(service_instance)
 
       target_spaces.each do |space|
-        plan_visibility!(service_instance, space)
-        name_uniqueness!(service_instance, space)
+        validate_plan_visibility!(service_instance, space)
+        validate_name_uniqueness!(service_instance, space)
       end
     end
 
-    def plan_visibility!(service_instance, space)
-      visible_plans = ServicePlan.organization_visible(space.organization)
+    def validate_plan_is_active!(service_instance)
+      if !service_instance.service_plan.active?
+        error_msg = "The service instance could not be shared as the #{service_instance.service_plan.name} plan is inactive."
+        raise CloudController::Errors::ApiError.new_from_details('UnprocessableEntity', error_msg)
+      end
+    end
 
-      if !visible_plans.include?(service_instance.service_plan)
+    def validate_plan_visibility!(service_instance, space)
+      unless service_instance.service_plan.visible_in_space?(space)
         error_msg = "Access to service #{service_instance.service.label} and plan #{service_instance.service_plan.name} is not enabled in #{space.organization.name}/#{space.name}"
         raise CloudController::Errors::ApiError.new_from_details('UnprocessableEntity', error_msg)
       end
     end
 
-    def name_uniqueness!(service_instance, space)
+    def validate_name_uniqueness!(service_instance, space)
       if space.service_instances.map(&:name).include?(service_instance.name)
         raise CloudController::Errors::ApiError.new_from_details('SharedServiceInstanceNameTaken', service_instance.name, space.name)
       end
     end
 
-    def no_sharing_to_self!(service_instance, spaces)
+    def validate_not_sharing_to_self!(service_instance, spaces)
       if spaces.include?(service_instance.space)
         raise CloudController::Errors::ApiError.new_from_details('InvalidServiceInstanceSharingTargetSpace')
       end
     end
 
-    def supported_service_type!(service_instance)
+    def validate_supported_service_type!(service_instance)
       if service_instance.route_service?
         raise CloudController::Errors::ApiError.new_from_details('RouteServiceInstanceSharingNotSupported')
       end
@@ -61,7 +67,7 @@ def supported_service_type!(service_instance)
       end
     end
 
-    def service_instance_shareable!(service_instance)
+    def validate_service_instance_is_shareable!(service_instance)
       unless service_instance.shareable?
         raise CloudController::Errors::ApiError.new_from_details('ServiceShareIsDisabled', service_instance.service.label)
       end

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)
 
@@ -358,7 +359,12 @@ def unbind_route(route_guid, instance_guid)
       raise CloudController::Errors::ApiError.new_from_details('ServiceInstanceNotFound', instance_guid)
     end
 
-    private
+    class ServiceInstanceSharedToEagerLoader
+      def eager_load_dataset(spaces, _, _, _, _)
+        spaces.eager(:organization)
+      end
+    end
+    private_constant :ServiceInstanceSharedToEagerLoader
 
     class ServiceInstanceSharedToSerializer
       def initialize(service_instance)
@@ -370,10 +376,13 @@ def serialize(controller, space, opts, orphans=nil)
         CloudController::Presenters::V2::ServiceInstanceSharedToPresenter.new.to_hash(space, bound_app_count)
       end
     end
+    private_constant :ServiceInstanceSharedToSerializer
+
+    private
 
     def create_paginated_collection_renderer(service_instance)
       VCAP::CloudController::RestController::PaginatedCollectionRenderer.new(
-        VCAP::CloudController::RestController::SecureEagerLoader.new,
+        ServiceInstanceSharedToEagerLoader.new,
         ServiceInstanceSharedToSerializer.new(service_instance),
         {
           max_results_per_page: config.get(:renderer, :max_results_per_page),
@@ -432,6 +441,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

app/controllers/v3/service_instances_controller.rb

@@ -6,17 +6,17 @@
 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
     message = ServiceInstancesListMessage.from_params(query_params)
     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, 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(
@@ -44,12 +44,10 @@ def share_service_instance
     share.create(service_instance, spaces, user_audit_info)
 
     render status: :ok, json: Presenters::V3::ToManyRelationshipPresenter.new(
-      "service_instances/#{service_instance.guid}", service_instance.shared_spaces, 'shared_spaces')
+      "service_instances/#{service_instance.guid}", service_instance.shared_spaces, 'shared_spaces', build_related: false)
   end
 
   def unshare_service_instance
-    FeatureFlag.raise_unless_enabled!(:service_instance_sharing)
-
     service_instance = ServiceInstance.first(guid: params[:service_instance_guid])
 
     resource_not_found!(:service_instance) unless service_instance && can_read_service_instance?(service_instance)
@@ -68,6 +66,14 @@ def unshare_service_instance
     head :no_content
   end
 
+  def relationships_shared_spaces
+    service_instance = ServiceInstance.first(guid: params[:service_instance_guid])
+    resource_not_found!(:service_instance) unless service_instance && can_read_space?(service_instance.space)
+
+    render status: :ok, json: Presenters::V3::ToManyRelationshipPresenter.new(
+      "service_instances/#{service_instance.guid}", service_instance.shared_spaces, 'shared_spaces', build_related: false)
+  end
+
   private
 
   def check_spaces_are_writeable!(spaces)

app/fetchers/managed_service_instance_list_fetcher.rb

@@ -0,0 +1,35 @@
+module VCAP::CloudController
+  class ManagedServiceInstanceListFetcher
+    def fetch(message:, readable_space_guids:)
+      source_space_instance_dataset = ManagedServiceInstance.select_all(ServiceInstance.table_name).
+                                      join(Space.table_name, id: :space_id, guid: readable_space_guids)
+
+      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)
+
+      filter(dataset, message)
+    end
+
+    def fetch_all(message:)
+      dataset = ManagedServiceInstance.dataset
+      filter(dataset, message)
+    end
+
+    private
+
+    def filter(dataset, message)
+      if message.requested?(:names)
+        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
+end

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
 

app/models/services/service_binding.rb

@@ -50,7 +50,7 @@ def validate_space_match
       return unless service_instance && app
       return if service_instance.space == app.space
 
-      if !FeatureFlag.enabled?(:service_instance_sharing) || service_instance.shared_spaces.exclude?(app.space)
+      if service_instance.shared_spaces.exclude?(app.space)
         errors.add(:service_instance, :space_mismatch)
       end
     end
@@ -83,7 +83,7 @@ def after_initialize
     end
 
     def self.user_visibility_filter(user)
-      { service_instance: ServiceInstance.user_visible(user) }
+      { app: AppModel.user_visible(user) }
     end
 
     def required_parameters

app/models/services/service_plan.rb

@@ -132,6 +132,11 @@ def broker_private?
       service_broker.private? if service_broker
     end
 
+    def visible_in_space?(space)
+      visible_plans = ServicePlan.space_visible(space)
+      visible_plans.include?(self)
+    end
+
     private
 
     def before_validation

app/models/v3/persistence/app_model.rb

@@ -80,6 +80,16 @@ def stopped?
       desired_state == ProcessModel::STOPPED
     end
 
+    def self.user_visibility_filter(user)
+      space_guids = Space.join(:spaces_developers, space_id: :id, user_id: user.id).select(:spaces__guid).
+                    union(Space.join(:spaces_managers, space_id: :id, user_id: user.id).select(:spaces__guid)).
+                    union(Space.join(:spaces_auditors, space_id: :id, user_id: user.id).select(:spaces__guid)).
+                    union(Space.join(:organizations_managers, organization_id: :organization_id, user_id: user.id).select(:spaces__guid))
+      {
+        apps__guid: AppModel.where(space: space_guids.all).select(:guid)
+      }
+    end
+
     private
 
     def update_enable_ssh

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
           }

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

config/routes.rb

@@ -120,6 +120,7 @@
 
   # service_instances
   get '/service_instances', to: 'service_instances_v3#index'
+  get '/service_instances/:service_instance_guid/relationships/shared_spaces', to: 'service_instances_v3#relationships_shared_spaces'
   post '/service_instances/:service_instance_guid/relationships/shared_spaces', to: 'service_instances_v3#share_service_instance'
   delete '/service_instances/:service_instance_guid/relationships/shared_spaces/:space_guid', to: 'service_instances_v3#unshare_service_instance'
 end