Return HTTP 400 Bad Request for unknown instance in last operation

v2.15 of the OSB API spec is a little unclear about how to handle the
scenario where a request is made for the last operation of an instance id
that does not exist and never existed. It does specify a 400 Bad Request
for "malformed or missing mandatory data". In this case an unknown instance
id is provided, so it's the best option for response code.

The spec is improved in v2.16. There, it adds a 404 Not Found for this
scenario. We'll change this when we move to the newer spec.

Resolves #306

spring-cloud-open-service-broker-autoconfigure/src/test/java/org/springframework/cloud/servicebroker/autoconfigure/web/AbstractServiceInstanceControllerIntegrationTest.java

@@ -125,6 +125,11 @@ protected void setupServiceInstanceService(ServiceInstanceDoesNotExistException
 				.willReturn(Mono.error(exception));
 	}
 
+	protected void setupServiceInstanceServiceLastOperation(ServiceInstanceDoesNotExistException exception) {
+		given(serviceInstanceService.getLastOperation(any(GetLastServiceOperationRequest.class)))
+				.willReturn(Mono.error(exception));
+	}
+
 	protected void setupServiceInstanceService(UpdateServiceInstanceResponse response) {
 		given(serviceInstanceService.updateServiceInstance(any(UpdateServiceInstanceRequest.class)))
 				.willReturn(Mono.just(response));

spring-cloud-open-service-broker-autoconfigure/src/test/java/org/springframework/cloud/servicebroker/autoconfigure/web/reactive/ServiceInstanceControllerIntegrationTest.java

@@ -53,6 +53,7 @@
 import org.springframework.test.web.reactive.server.WebTestClient;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.hamcrest.core.StringContains.containsString;
 import static org.springframework.cloud.servicebroker.exception.ServiceBrokerAsyncRequiredException.ASYNC_REQUIRED_ERROR;
 import static org.springframework.cloud.servicebroker.exception.ServiceBrokerMaintenanceInfoConflictException.MAINTENANCE_INFO_CONFLICT_ERROR;
 import static org.springframework.cloud.servicebroker.exception.ServiceBrokerMaintenanceInfoConflictException.MAINTENANCE_INFO_CONFLICT_MESSAGE;
@@ -787,6 +788,20 @@ void lastOperationHasSucceededStatusWithDeletionComplete() {
 				.jsonPath("$.description").isEqualTo("all gone");
 	}
 
+	@Test
+	void lastOperationWithUnknownInstanceBadRequest() {
+		setupServiceInstanceServiceLastOperation(new ServiceInstanceDoesNotExistException("nonexistent-instance-id"));
+
+		client.get().uri(buildLastOperationUrl())
+				.accept(MediaType.APPLICATION_JSON)
+				.exchange()
+				.expectStatus().is4xxClientError()
+				.expectStatus().isEqualTo(HttpStatus.BAD_REQUEST)
+				.expectBody()
+				.jsonPath("$.state").doesNotExist()
+				.jsonPath("$.description").value(containsString("The requested Service Instance does not exist"));
+	}
+
 	@Test
 	void lastOperationHasFailedStatus() {
 		setupServiceInstanceService(GetLastServiceOperationResponse.builder()

spring-cloud-open-service-broker-autoconfigure/src/test/java/org/springframework/cloud/servicebroker/autoconfigure/web/servlet/ServiceInstanceControllerIntegrationTest.java

@@ -861,6 +861,21 @@ void lastOperationHasSucceededStatusWithDeletionComplete() throws Exception {
 				.andExpect(jsonPath("$.description", is("all gone")));
 	}
 
+	@Test
+	void lastOperationWithUnknownInstanceBadRequest() throws Exception {
+		setupServiceInstanceServiceLastOperation(new ServiceInstanceDoesNotExistException("nonexistent-instance-id"));
+
+		MvcResult mvcResult = mockMvc.perform(get(buildLastOperationUrl()))
+				.andExpect(request().asyncStarted())
+				.andReturn();
+
+		mockMvc.perform(asyncDispatch(mvcResult))
+				.andExpect(status().isBadRequest())
+				.andExpect(jsonPath("$.state").doesNotExist())
+				.andExpect(jsonPath("$.description",
+						containsString("The requested Service Instance does not exist")));
+	}
+
 	@Test
 	void lastOperationHasFailedStatus() throws Exception {
 		setupServiceInstanceService(GetLastServiceOperationResponse.builder()

spring-cloud-open-service-broker-core/src/main/java/org/springframework/cloud/servicebroker/controller/ServiceInstanceController.java

@@ -241,9 +241,20 @@ public Mono<ResponseEntity<GetLastServiceOperationResponse>> getServiceInstanceL
 						.doOnError(e -> LOG.error("Error getting service instance last operation. error=" +
 								e.getMessage(), e)))
 				.map(response -> {
-					boolean isSuccessfulDelete = response.getState().equals(OperationState.SUCCEEDED) && response
+					boolean isSuccessfulDelete = OperationState.SUCCEEDED.equals(response.getState()) && response
 							.isDeleteOperation();
 					return new ResponseEntity<>(response, isSuccessfulDelete ? HttpStatus.GONE : HttpStatus.OK);
+				})
+				.onErrorResume(e -> {
+					if (e instanceof ServiceInstanceDoesNotExistException) {
+						// TODO: v2.16 of the OSB API spec changes this to an HTTP 404
+						return Mono.just(new ResponseEntity<>(GetLastServiceOperationResponse.builder()
+								.description("The requested Service Instance does not exist")
+								.build(), HttpStatus.BAD_REQUEST));
+					}
+					else {
+						return Mono.error(e);
+					}
 				});
 	}
 

spring-cloud-open-service-broker-core/src/test/java/org/springframework/cloud/servicebroker/controller/ServiceInstanceControllerResponseCodeTest.java

@@ -313,6 +313,20 @@ void getLastOperationWithDeleteSucceededResponseGivesExpectedStatus() {
 				.build(), HttpStatus.GONE);
 	}
 
+	@Test
+	void getLastOperationWithUnknownInstanceBadRequest() {
+		given(serviceInstanceService.getLastOperation(any(GetLastServiceOperationRequest.class)))
+				.willReturn(Mono.error(new ServiceInstanceDoesNotExistException("instance never existed")));
+
+		ResponseEntity<GetLastServiceOperationResponse> responseEntity = controller
+				.getServiceInstanceLastOperation(pathVariables, null, "service-definition-id",
+						"service-definition-plan-id", null, null, null, null)
+				.block();
+
+		assertThat(responseEntity).isNotNull();
+		assertThat(responseEntity.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST);
+	}
+
 	@Test
 	void getLastOperationWithFailedResponseGivesExpectedStatus() {
 		validateGetLastOperationWithResponseStatus(GetLastServiceOperationResponse.builder()