Filter Plan objects provided in Request objects

Resolves spring-cloud#141
royclarkson · Dec 18, 2018 · 2e8c60e · 2e8c60e
1 parent 6b2ede4
commit 2e8c60e
Show file tree

Hide file tree

Showing 9 changed files with 159 additions and 20 deletions.
diff --git a/...oud/servicebroker/autoconfigure/web/AbstractServiceInstanceControllerIntegrationTest.java b/...oud/servicebroker/autoconfigure/web/AbstractServiceInstanceControllerIntegrationTest.java
@@ -57,15 +57,25 @@ public abstract class AbstractServiceInstanceControllerIntegrationTest extends C
 
 	protected String updateRequestBody;
 
+	protected String updateRequestBodyWithPlan;
+
 	@Before
 	public void setUpCommonFixtures() {
-		this.createRequestBody = JsonUtils.toJson(CreateServiceInstanceRequest.builder()
+		this.createRequestBody = JsonUtils.toJson(CreateServiceInstanceRequest
+				.builder()
+				.serviceDefinitionId(serviceDefinition.getId())
+				.planId("plan-one-id")
+				.build());
+
+		this.updateRequestBody = JsonUtils.toJson(UpdateServiceInstanceRequest
+				.builder()
 				.serviceDefinitionId(serviceDefinition.getId())
-				.planId("standard")
 				.build());
 
-		this.updateRequestBody = JsonUtils.toJson(UpdateServiceInstanceRequest.builder()
+		this.updateRequestBodyWithPlan = JsonUtils.toJson(UpdateServiceInstanceRequest
+				.builder()
 				.serviceDefinitionId(serviceDefinition.getId())
+				.planId("plan-three-id")
 				.build());
 	}
 
@@ -132,7 +142,7 @@ protected String buildDeleteUrl(String platformInstanceId, boolean asyncAccepted
 		return buildBaseUrl(platformInstanceId)
 				.path(SERVICE_INSTANCE_ID)
 				.queryParam("service_id", serviceDefinition.getId())
-				.queryParam("plan_id", "standard")
+				.queryParam("plan_id", "plan-three-id")
 				.queryParam("accepts_incomplete", asyncAccepted)
 				.toUriString();
 	}

diff --git a/...ringframework/cloud/servicebroker/autoconfigure/web/CatalogControllerIntegrationTest.java b/...ringframework/cloud/servicebroker/autoconfigure/web/CatalogControllerIntegrationTest.java
@@ -116,14 +116,14 @@ private void assertResult(ResultActions result) throws Exception {
 						SERVICE_REQUIRES_SYSLOG_DRAIN.toString(),
 						SERVICE_REQUIRES_ROUTE_FORWARDING.toString())
 				))
-				.andExpect(jsonPath("$.services[*].plans[*]", hasSize(2)))
-				.andExpect(jsonPath("$.services[*].plans[*].id", containsInAnyOrder(plans.get(0).getId(), plans.get(1).getId())))
-				.andExpect(jsonPath("$.services[*].plans[*].name", containsInAnyOrder(plans.get(0).getName(), plans.get(1).getName())))
-				.andExpect(jsonPath("$.services[*].plans[*].description", containsInAnyOrder(plans.get(0).getDescription(), plans.get(1).getDescription())))
+				.andExpect(jsonPath("$.services[*].plans[*]", hasSize(3)))
+				.andExpect(jsonPath("$.services[*].plans[*].id", containsInAnyOrder(plans.get(0).getId(), plans.get(1).getId(), plans.get(2).getId())))
+				.andExpect(jsonPath("$.services[*].plans[*].name", containsInAnyOrder(plans.get(0).getName(), plans.get(1).getName(), plans.get(2).getName())))
+				.andExpect(jsonPath("$.services[*].plans[*].description", containsInAnyOrder(plans.get(0).getDescription(), plans.get(1).getDescription(), plans.get(2).getDescription())))
 				.andExpect(jsonPath("$.services[*].plans[*].metadata", contains(plans.get(1).getMetadata())))
 				.andExpect(jsonPath("$.services[*].plans[*].bindable", hasSize(1)))
 				.andExpect(jsonPath("$.services[*].plans[*].bindable", contains(plans.get(1).isBindable())))
-				.andExpect(jsonPath("$.services[*].plans[*].free", containsInAnyOrder(plans.get(0).isFree(), plans.get(1).isFree())))
+				.andExpect(jsonPath("$.services[*].plans[*].free", containsInAnyOrder(plans.get(0).isFree(), plans.get(1).isFree(), plans.get(2).isFree())))
 				.andExpect(jsonPath("$.services[*].plans[*].schemas.service_instance.create.parameters", contains(createServiceInstanceSchema)))
 				.andExpect(jsonPath("$.services[*].plans[*].schemas.service_instance.update.parameters", contains(updateServiceInstanceSchema)))
 				.andExpect(jsonPath("$.services[*].plans[*].schemas.service_binding.create.parameters", contains(createServiceBindingSchema)));

diff --git a/...loud/servicebroker/autoconfigure/web/ServiceInstanceBindingControllerIntegrationTest.java b/...loud/servicebroker/autoconfigure/web/ServiceInstanceBindingControllerIntegrationTest.java
@@ -92,6 +92,30 @@ public void createBindingToAppWithoutAsyncAndHeadersSucceeds() throws Exception
 		assertHeaderValuesSet(actualRequest);
 	}
 
+	@Test
+	public void createBindingToAppFiltersPlansSucceeds() throws Exception {
+		setupCatalogService();
+
+		setupServiceInstanceBindingService(CreateServiceInstanceAppBindingResponse
+				.builder()
+				.bindingExisted(false)
+				.build());
+
+		mockMvc.perform(put(buildCreateUrl(PLATFORM_INSTANCE_ID, false))
+				.content(createRequestBody)
+				.header(API_INFO_LOCATION_HEADER, API_INFO_LOCATION)
+				.header(ORIGINATING_IDENTITY_HEADER, buildOriginatingIdentityHeader())
+				.accept(MediaType.APPLICATION_JSON)
+				.contentType(MediaType.APPLICATION_JSON))
+			   .andExpect(status().isCreated());
+
+		CreateServiceInstanceBindingRequest actualRequest = verifyCreateBinding();
+		assertThat(actualRequest.isAsyncAccepted()).isEqualTo(false);
+		assertThat(actualRequest.getServiceDefinition().getPlans().size()).isEqualTo(1);
+		assertThat(actualRequest.getServiceDefinition().getPlans().get(0).getId()).isEqualTo(actualRequest.getPlanId());
+		assertHeaderValuesSet(actualRequest);
+	}
+
 	@Test
 	public void createBindingToAppWithAsyncAndHeadersSucceeds() throws Exception {
 		setupCatalogService();
@@ -344,6 +368,30 @@ public void deleteBindingWithoutAsyncAndHeadersSucceeds() throws Exception {
 		assertHeaderValuesSet(actualRequest);
 	}
 
+	@Test
+	public void deleteBindingFiltersPlansSucceeds() throws Exception {
+		setupCatalogService();
+
+		setupServiceInstanceBindingService(DeleteServiceInstanceBindingResponse
+				.builder()
+				.build());
+
+		mockMvc.perform(delete(buildDeleteUrl(PLATFORM_INSTANCE_ID, false))
+				.header(API_INFO_LOCATION_HEADER, API_INFO_LOCATION)
+				.header(ORIGINATING_IDENTITY_HEADER, buildOriginatingIdentityHeader())
+				.contentType(MediaType.APPLICATION_JSON))
+			   .andExpect(status().isOk())
+			   .andExpect(content().string("{}"));
+
+		verify(serviceInstanceBindingService).deleteServiceInstanceBinding(any(DeleteServiceInstanceBindingRequest.class));
+
+		DeleteServiceInstanceBindingRequest actualRequest = verifyDeleteBinding();
+		assertThat(actualRequest.isAsyncAccepted()).isEqualTo(false);
+		assertThat(actualRequest.getServiceDefinition().getPlans().size()).isEqualTo(1);
+		assertThat(actualRequest.getServiceDefinition().getPlans().get(0).getId()).isEqualTo(actualRequest.getPlanId());
+		assertHeaderValuesSet(actualRequest);
+	}
+
 	@Test
 	public void deleteBindingWithUnknownInstanceIdFails() throws Exception {
 		setupCatalogService();

diff --git a/...ramework/cloud/servicebroker/autoconfigure/web/ServiceInstanceBindingIntegrationTest.java b/...ramework/cloud/servicebroker/autoconfigure/web/ServiceInstanceBindingIntegrationTest.java
@@ -33,7 +33,7 @@ public abstract class ServiceInstanceBindingIntegrationTest extends ControllerIn
 	public void setupBase() {
 		CreateServiceInstanceBindingRequest request = CreateServiceInstanceBindingRequest.builder()
 				.serviceDefinitionId(serviceDefinition.getId())
-				.planId("standard")
+				.planId("plan-one-id")
 				.build();
 
 		createRequestBody = JsonUtils.toJson(request);

diff --git a/...ework/cloud/servicebroker/autoconfigure/web/ServiceInstanceControllerIntegrationTest.java b/...ework/cloud/servicebroker/autoconfigure/web/ServiceInstanceControllerIntegrationTest.java
@@ -185,6 +185,30 @@ public void createServiceInstanceWithExistingInstanceSucceeds() throws Exception
 				.andExpect(status().isOk());
 	}
 
+	@Test
+	public void createServiceInstanceFiltersPlansSucceeds() throws Exception {
+		setupCatalogService();
+
+		setupServiceInstanceService(CreateServiceInstanceResponse
+				.builder()
+				.async(true)
+				.build());
+
+		mockMvc.perform(put(buildCreateUpdateUrl(PLATFORM_INSTANCE_ID, true))
+				.content(createRequestBody)
+				.header(API_INFO_LOCATION_HEADER, API_INFO_LOCATION)
+				.header(ORIGINATING_IDENTITY_HEADER, buildOriginatingIdentityHeader())
+				.contentType(MediaType.APPLICATION_JSON)
+				.accept(MediaType.APPLICATION_JSON))
+			   .andExpect(status().isAccepted());
+
+		CreateServiceInstanceRequest actualRequest = verifyCreateServiceInstance();
+		assertThat(actualRequest.isAsyncAccepted()).isEqualTo(true);
+		assertThat(actualRequest.getServiceDefinition().getPlans().size()).isEqualTo(1);
+		assertThat(actualRequest.getServiceDefinition().getPlans().get(0).getId()).isEqualTo(actualRequest.getPlanId());
+		assertHeaderValuesSet(actualRequest);
+	}
+
 	@Test
 	public void createServiceInstanceWithUnknownServiceDefinitionIdFails() throws Exception {
 		setupCatalogService(null);
@@ -350,6 +374,26 @@ public void deleteServiceInstanceWithoutAsyncAndHeadersSucceeds() throws Excepti
 		assertHeaderValuesNotSet(actualRequest);
 	}
 
+	@Test
+	public void deleteServiceInstanceFiltersPlansSucceeds() throws Exception {
+		setupCatalogService();
+
+		setupServiceInstanceService(DeleteServiceInstanceResponse
+				.builder()
+				.build());
+
+		mockMvc.perform(delete(buildDeleteUrl())
+				.accept(MediaType.APPLICATION_JSON))
+			   .andExpect(status().isOk())
+			   .andExpect(content().string("{}"));
+
+		DeleteServiceInstanceRequest actualRequest = verifyDeleteServiceInstance();
+		assertThat(actualRequest.isAsyncAccepted()).isEqualTo(false);
+		assertThat(actualRequest.getServiceDefinition().getPlans().size()).isEqualTo(1);
+		assertThat(actualRequest.getServiceDefinition().getPlans().get(0).getId()).isEqualTo(actualRequest.getPlanId());
+		assertHeaderValuesNotSet(actualRequest);
+	}
+
 	@Test
 	public void deleteServiceInstanceWithUnknownIdFails() throws Exception {
 		setupCatalogService();
@@ -413,6 +457,29 @@ public void updateServiceInstanceWithoutSyncAndHeadersSucceeds() throws Exceptio
 
 		UpdateServiceInstanceRequest actualRequest = verifyUpdateServiceInstance();
 		assertThat(actualRequest.isAsyncAccepted()).isEqualTo(false);
+		assertThat(actualRequest.getServiceDefinition().getPlans().size()).isEqualTo(3);
+		assertHeaderValuesNotSet(actualRequest);
+	}
+
+	@Test
+	public void updateServiceInstanceFiltersPlansSucceeds() throws Exception {
+		setupCatalogService();
+
+		setupServiceInstanceService(UpdateServiceInstanceResponse
+				.builder()
+				.build());
+
+		mockMvc.perform(patch(buildCreateUpdateUrl())
+				.content(updateRequestBodyWithPlan)
+				.contentType(MediaType.APPLICATION_JSON)
+				.accept(MediaType.APPLICATION_JSON))
+			   .andExpect(status().isOk())
+			   .andExpect(content().string("{}"));
+
+		UpdateServiceInstanceRequest actualRequest = verifyUpdateServiceInstance();
+		assertThat(actualRequest.isAsyncAccepted()).isEqualTo(false);
+		assertThat(actualRequest.getServiceDefinition().getPlans().size()).isEqualTo(1);
+		assertThat(actualRequest.getServiceDefinition().getPlans().get(0).getId()).isEqualTo(actualRequest.getPlanId());
 		assertHeaderValuesNotSet(actualRequest);
 	}
 

diff --git a/...ava/org/springframework/cloud/servicebroker/autoconfigure/web/fixture/ServiceFixture.java b/...ava/org/springframework/cloud/servicebroker/autoconfigure/web/fixture/ServiceFixture.java
@@ -35,7 +35,7 @@ public static ServiceDefinition getSimpleService() {
 				.name("Service One")
 				.description("Description for Service One")
 				.bindable(true)
-				.plans(getPlanOne(), getPlanTwo())
+				.plans(getPlanOne(), getPlanTwo(), getPlanThree())
 				.requires(ServiceDefinitionRequires.SERVICE_REQUIRES_SYSLOG_DRAIN.toString(),
 						ServiceDefinitionRequires.SERVICE_REQUIRES_ROUTE_FORWARDING.toString())
 				.build();
@@ -80,4 +80,12 @@ private static Plan getPlanTwo() {
 				.schemas(schemas)
 				.build();
 	}
+
+	private static Plan getPlanThree() {
+		return Plan.builder()
+				   .id("plan-three-id")
+				   .name("Plan Three")
+				   .description("Description for Plan Three")
+				   .build();
+	}
 }
diff --git a/...core/src/main/java/org/springframework/cloud/servicebroker/controller/BaseController.java b/...core/src/main/java/org/springframework/cloud/servicebroker/controller/BaseController.java
@@ -36,6 +36,7 @@
 import org.springframework.http.HttpStatus;
 import org.springframework.http.converter.json.Jackson2ObjectMapperBuilder;
 import org.springframework.util.Base64Utils;
+import org.springframework.util.StringUtils;
 
 /**
  * Base functionality shared by controllers.
@@ -65,16 +66,20 @@ protected void setCommonRequestFields(AsyncServiceBrokerRequest request, String
 		request.setAsyncAccepted(asyncAccepted);
 	}
 
-	protected ServiceDefinition getRequiredServiceDefinition(String serviceDefinitionId) {
-		ServiceDefinition serviceDefinition = getServiceDefinition(serviceDefinitionId);
+	protected ServiceDefinition getRequiredServiceDefinition(String serviceDefinitionId, String planId) {
+		ServiceDefinition serviceDefinition = getServiceDefinition(serviceDefinitionId, planId);
 		if (serviceDefinition == null) {
 			throw new ServiceDefinitionDoesNotExistException(serviceDefinitionId);
 		}
 		return serviceDefinition;
 	}
 
-	protected ServiceDefinition getServiceDefinition(String serviceDefinitionId) {
-		return catalogService.getServiceDefinition(serviceDefinitionId);
+	protected ServiceDefinition getServiceDefinition(String serviceDefinitionId, String planId) {
+		ServiceDefinition serviceDefinition = catalogService.getServiceDefinition(serviceDefinitionId);
+		if (StringUtils.hasText(planId) && serviceDefinition != null) {
+			serviceDefinition.getPlans().removeIf(plan -> !plan.getId().equals(planId));
+		}
+		return serviceDefinition;
 	}
 
 	protected Context parseOriginatingIdentity(String originatingIdentityString) {

diff --git a/.../org/springframework/cloud/servicebroker/controller/ServiceInstanceBindingController.java b/.../org/springframework/cloud/servicebroker/controller/ServiceInstanceBindingController.java
@@ -55,6 +55,7 @@
  *
  * @author sgreenberg@pivotal.io
  * @author Scott Frederick
+ * @author Roy Clarkson
  */
 @ServiceBrokerRestController
 public class ServiceInstanceBindingController extends BaseController {
@@ -84,7 +85,7 @@ public ResponseEntity<CreateServiceInstanceBindingResponse> createServiceInstanc
 			@Valid @RequestBody CreateServiceInstanceBindingRequest request) {
 		request.setServiceInstanceId(serviceInstanceId);
 		request.setBindingId(bindingId);
-		request.setServiceDefinition(getRequiredServiceDefinition(request.getServiceDefinitionId()));
+		request.setServiceDefinition(getRequiredServiceDefinition(request.getServiceDefinitionId(), request.getPlanId()));
 		setCommonRequestFields(request, pathVariables.get(ServiceBrokerRequest.PLATFORM_INSTANCE_ID_VARIABLE), apiInfoLocation,
 				originatingIdentityString, acceptsIncomplete);
 
@@ -190,7 +191,7 @@ public ResponseEntity<DeleteServiceInstanceBindingResponse> deleteServiceInstanc
 				.bindingId(bindingId)
 				.serviceDefinitionId(serviceDefinitionId)
 				.planId(planId)
-				.serviceDefinition(getServiceDefinition(serviceDefinitionId))
+				.serviceDefinition(getServiceDefinition(serviceDefinitionId, planId))
 				.asyncAccepted(acceptsIncomplete)
 				.platformInstanceId(pathVariables.get(ServiceBrokerRequest.PLATFORM_INSTANCE_ID_VARIABLE))
 				.apiInfoLocation(apiInfoLocation)

diff --git a/...in/java/org/springframework/cloud/servicebroker/controller/ServiceInstanceController.java b/...in/java/org/springframework/cloud/servicebroker/controller/ServiceInstanceController.java
@@ -85,7 +85,7 @@ public ResponseEntity<CreateServiceInstanceResponse> createServiceInstance(
 			@RequestHeader(value = ServiceBrokerRequest.API_INFO_LOCATION_HEADER, required = false) String apiInfoLocation,
 			@RequestHeader(value = ServiceBrokerRequest.ORIGINATING_IDENTITY_HEADER, required = false) String originatingIdentityString,
 			@Valid @RequestBody CreateServiceInstanceRequest request) {
-		ServiceDefinition serviceDefinition = getRequiredServiceDefinition(request.getServiceDefinitionId());
+		ServiceDefinition serviceDefinition = getRequiredServiceDefinition(request.getServiceDefinitionId(), request.getPlanId());
 
 		request.setServiceInstanceId(serviceInstanceId);
 		request.setServiceDefinition(serviceDefinition);
@@ -185,7 +185,7 @@ public ResponseEntity<DeleteServiceInstanceResponse> deleteServiceInstance(
 			@RequestParam(value = AsyncServiceBrokerRequest.ASYNC_REQUEST_PARAMETER, required = false) boolean acceptsIncomplete,
 			@RequestHeader(value = ServiceBrokerRequest.API_INFO_LOCATION_HEADER, required = false) String apiInfoLocation,
 			@RequestHeader(value = ServiceBrokerRequest.ORIGINATING_IDENTITY_HEADER, required = false) String originatingIdentityString) {
-		ServiceDefinition serviceDefinition = getRequiredServiceDefinition(serviceDefinitionId);
+		ServiceDefinition serviceDefinition = getRequiredServiceDefinition(serviceDefinitionId, planId);
 
 		DeleteServiceInstanceRequest request = DeleteServiceInstanceRequest.builder()
 				.serviceInstanceId(serviceInstanceId)
@@ -224,7 +224,7 @@ public ResponseEntity<UpdateServiceInstanceResponse> updateServiceInstance(
 			@RequestHeader(value = ServiceBrokerRequest.API_INFO_LOCATION_HEADER, required = false) String apiInfoLocation,
 			@RequestHeader(value = ServiceBrokerRequest.ORIGINATING_IDENTITY_HEADER, required = false) String originatingIdentityString,
 			@Valid @RequestBody UpdateServiceInstanceRequest request) {
-		ServiceDefinition serviceDefinition = getRequiredServiceDefinition(request.getServiceDefinitionId());
+		ServiceDefinition serviceDefinition = getRequiredServiceDefinition(request.getServiceDefinitionId(), request.getPlanId());
 
 		request.setServiceInstanceId(serviceInstanceId);
 		request.setServiceDefinition(serviceDefinition);