From 2f2e183aca788b5f739802bc3939fdc3369337a3 Mon Sep 17 00:00:00 2001 From: Roy Clarkson Date: Wed, 13 May 2020 13:04:07 -0400 Subject: [PATCH] Refactor imperative code to be more reactive --- ...ploymentCreateServiceInstanceWorkflow.java | 14 ++--- ...ploymentDeleteServiceInstanceWorkflow.java | 13 ++-- .../AppDeploymentInstanceWorkflow.java | 62 ++++++------------- ...ploymentUpdateServiceInstanceWorkflow.java | 12 ++-- ...mentDeleteServiceInstanceWorkflowTest.java | 2 - 5 files changed, 39 insertions(+), 64 deletions(-) diff --git a/spring-cloud-app-broker-core/src/main/java/org/springframework/cloud/appbroker/workflow/instance/AppDeploymentCreateServiceInstanceWorkflow.java b/spring-cloud-app-broker-core/src/main/java/org/springframework/cloud/appbroker/workflow/instance/AppDeploymentCreateServiceInstanceWorkflow.java index 5520fbb..d8af115 100644 --- a/spring-cloud-app-broker-core/src/main/java/org/springframework/cloud/appbroker/workflow/instance/AppDeploymentCreateServiceInstanceWorkflow.java +++ b/spring-cloud-app-broker-core/src/main/java/org/springframework/cloud/appbroker/workflow/instance/AppDeploymentCreateServiceInstanceWorkflow.java @@ -78,10 +78,10 @@ public class AppDeploymentCreateServiceInstanceWorkflow private Flux createBackingServices(CreateServiceInstanceRequest request) { return getBackingServicesForService(request.getServiceDefinition(), request.getPlan()) - .flatMap(backingServices -> - targetService.addToBackingServices(backingServices, - getTargetForService(request.getServiceDefinition(), request.getPlan()), + .flatMap(backingServices -> getTargetForService(request.getServiceDefinition(), request.getPlan()) + .flatMap(targetSpec -> targetService.addToBackingServices(backingServices, targetSpec, request.getServiceInstanceId())) + .defaultIfEmpty(backingServices)) .flatMap(backingServices -> servicesParametersTransformationService.transformParameters(backingServices, request.getParameters())) @@ -97,10 +97,10 @@ public class AppDeploymentCreateServiceInstanceWorkflow private Flux deployBackingApplications(CreateServiceInstanceRequest request) { return getBackingApplicationsForService(request.getServiceDefinition(), request.getPlan()) - .flatMap(backingApps -> - targetService.addToBackingApplications(backingApps, - getTargetForService(request.getServiceDefinition(), - request.getPlan()), request.getServiceInstanceId())) + .flatMap(backingApps -> getTargetForService(request.getServiceDefinition(), request.getPlan()) + .flatMap(targetSpec -> targetService.addToBackingApplications(backingApps, targetSpec, + request.getServiceInstanceId())) + .defaultIfEmpty(backingApps)) .flatMap(backingApps -> appsParametersTransformationService.transformParameters(backingApps, request.getParameters())) diff --git a/spring-cloud-app-broker-core/src/main/java/org/springframework/cloud/appbroker/workflow/instance/AppDeploymentDeleteServiceInstanceWorkflow.java b/spring-cloud-app-broker-core/src/main/java/org/springframework/cloud/appbroker/workflow/instance/AppDeploymentDeleteServiceInstanceWorkflow.java index c0fafc4..c95354c 100644 --- a/spring-cloud-app-broker-core/src/main/java/org/springframework/cloud/appbroker/workflow/instance/AppDeploymentDeleteServiceInstanceWorkflow.java +++ b/spring-cloud-app-broker-core/src/main/java/org/springframework/cloud/appbroker/workflow/instance/AppDeploymentDeleteServiceInstanceWorkflow.java @@ -89,9 +89,10 @@ public class AppDeploymentDeleteServiceInstanceWorkflow private Flux> collectConfiguredBackingServices(DeleteServiceInstanceRequest request) { return getBackingServicesForService(request.getServiceDefinition(), request.getPlan()) - .flatMapMany(backingServices -> targetService.addToBackingServices(backingServices, - getTargetForService(request.getServiceDefinition(), request.getPlan()), - request.getServiceInstanceId())); + .flatMapMany(backingServices -> getTargetForService(request.getServiceDefinition(), request.getPlan()) + .flatMap(targetSpec -> targetService.addToBackingServices(backingServices, targetSpec, + request.getServiceInstanceId())) + .defaultIfEmpty(backingServices)); } private Flux> collectBoundBackingServices(DeleteServiceInstanceRequest request) { @@ -109,10 +110,10 @@ public class AppDeploymentDeleteServiceInstanceWorkflow .flatMap(backingApps -> credentialProviderService.deleteCredentials(backingApps, request.getServiceInstanceId())) - .flatMap(backingApps -> - targetService.addToBackingApplications(backingApps, - getTargetForService(request.getServiceDefinition(), request.getPlan()), + .flatMap(backingApps -> getTargetForService(request.getServiceDefinition(), request.getPlan()) + .flatMap(targetSpec -> targetService.addToBackingApplications(backingApps, targetSpec, request.getServiceInstanceId())) + .defaultIfEmpty(backingApps)) .flatMapMany(deploymentService::undeploy) .doOnRequest(l -> log.debug("Undeploying backing applications for {}/{}", request.getServiceDefinition().getName(), request.getPlan().getName())) diff --git a/spring-cloud-app-broker-core/src/main/java/org/springframework/cloud/appbroker/workflow/instance/AppDeploymentInstanceWorkflow.java b/spring-cloud-app-broker-core/src/main/java/org/springframework/cloud/appbroker/workflow/instance/AppDeploymentInstanceWorkflow.java index e55d6c3..a2b75c7 100644 --- a/spring-cloud-app-broker-core/src/main/java/org/springframework/cloud/appbroker/workflow/instance/AppDeploymentInstanceWorkflow.java +++ b/spring-cloud-app-broker-core/src/main/java/org/springframework/cloud/appbroker/workflow/instance/AppDeploymentInstanceWorkflow.java @@ -18,6 +18,7 @@ package org.springframework.cloud.appbroker.workflow.instance; import java.util.List; +import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import org.springframework.cloud.appbroker.deployer.BackingApplication; @@ -29,7 +30,6 @@ import org.springframework.cloud.appbroker.deployer.BrokeredServices; import org.springframework.cloud.appbroker.deployer.TargetSpec; import org.springframework.cloud.servicebroker.model.catalog.Plan; import org.springframework.cloud.servicebroker.model.catalog.ServiceDefinition; -import org.springframework.util.CollectionUtils; public class AppDeploymentInstanceWorkflow { @@ -50,57 +50,33 @@ public class AppDeploymentInstanceWorkflow { ); } - protected TargetSpec getTargetForService(ServiceDefinition serviceDefinition, Plan plan) { - BrokeredService brokeredService = findBrokeredService(serviceDefinition, plan); - return brokeredService == null ? null : brokeredService.getTarget(); + protected Mono getTargetForService(ServiceDefinition serviceDefinition, Plan plan) { + return findBrokeredService(serviceDefinition, plan) + .flatMap(brokeredService -> Mono.justOrEmpty(brokeredService.getTarget())); } protected Mono> getBackingApplicationsForService(ServiceDefinition serviceDefinition, Plan plan) { - return Mono.defer(() -> - Mono.justOrEmpty(findBackingApplications(serviceDefinition, plan))); + return findBrokeredService(serviceDefinition, plan) + .flatMap(brokeredService -> Mono.justOrEmpty(brokeredService.getApps())) + .map(backingApplications -> BackingApplications.builder() + .backingApplications(backingApplications) + .build()); } protected Mono> getBackingServicesForService(ServiceDefinition serviceDefinition, Plan plan) { - return Mono.defer(() -> - Mono.justOrEmpty(findBackingServices(serviceDefinition, plan))); + return findBrokeredService(serviceDefinition, plan) + .flatMap(brokeredService -> Mono.justOrEmpty(brokeredService.getServices())) + .map(backingServices -> BackingServices.builder() + .backingServices(backingServices) + .build()); } - private BackingApplications findBackingApplications(ServiceDefinition serviceDefinition, - Plan plan) { - BrokeredService brokeredService = findBrokeredService(serviceDefinition, plan); - BackingApplications backingApplications = null; - if (brokeredService != null) { - backingApplications = BackingApplications.builder() - .backingApplications(brokeredService.getApps()) - .build(); - } - return backingApplications; - } - - private BackingServices findBackingServices(ServiceDefinition serviceDefinition, - Plan plan) { - BrokeredService brokeredService = findBrokeredService(serviceDefinition, plan); - BackingServices backingServices = null; - if (brokeredService != null && !CollectionUtils.isEmpty(brokeredService.getServices())) { - backingServices = BackingServices.builder() - .backingServices(brokeredService.getServices()) - .build(); - } - return backingServices; - } - - private BrokeredService findBrokeredService(ServiceDefinition serviceDefinition, - Plan plan) { - String serviceName = serviceDefinition.getName(); - String planName = plan.getName(); - - return brokeredServices.stream() - .filter(brokeredService -> - brokeredService.getServiceName().equals(serviceName) - && brokeredService.getPlanName().equals(planName)) - .findFirst() - .orElse(null); + private Mono findBrokeredService(ServiceDefinition serviceDefinition, Plan plan) { + return Flux.fromIterable(brokeredServices) + .filter(brokeredService -> brokeredService.getServiceName().equals(serviceDefinition.getName()) + && brokeredService.getPlanName().equals(plan.getName())) + .singleOrEmpty(); } } diff --git a/spring-cloud-app-broker-core/src/main/java/org/springframework/cloud/appbroker/workflow/instance/AppDeploymentUpdateServiceInstanceWorkflow.java b/spring-cloud-app-broker-core/src/main/java/org/springframework/cloud/appbroker/workflow/instance/AppDeploymentUpdateServiceInstanceWorkflow.java index 177a11f..867cdc4 100644 --- a/spring-cloud-app-broker-core/src/main/java/org/springframework/cloud/appbroker/workflow/instance/AppDeploymentUpdateServiceInstanceWorkflow.java +++ b/spring-cloud-app-broker-core/src/main/java/org/springframework/cloud/appbroker/workflow/instance/AppDeploymentUpdateServiceInstanceWorkflow.java @@ -87,10 +87,10 @@ public class AppDeploymentUpdateServiceInstanceWorkflow extends AppDeploymentIns private Flux updateBackingServices(UpdateServiceInstanceRequest request) { return getBackingServicesForService(request.getServiceDefinition(), request.getPlan()) - .flatMap(backingServices -> - targetService.addToBackingServices(backingServices, - getTargetForService(request.getServiceDefinition(), request.getPlan()), + .flatMap(backingServices -> getTargetForService(request.getServiceDefinition(), request.getPlan()) + .flatMap(targetSpec -> targetService.addToBackingServices(backingServices, targetSpec, request.getServiceInstanceId())) + .defaultIfEmpty(backingServices)) .flatMap(backingServices -> servicesParametersTransformationService.transformParameters(backingServices, request.getParameters())) @@ -163,10 +163,10 @@ public class AppDeploymentUpdateServiceInstanceWorkflow extends AppDeploymentIns private Flux updateBackingApplications(UpdateServiceInstanceRequest request) { return getBackingApplicationsForService(request.getServiceDefinition(), request.getPlan()) - .flatMap(backingApps -> - targetService.addToBackingApplications(backingApps, - getTargetForService(request.getServiceDefinition(), request.getPlan()), + .flatMap(backingApps -> getTargetForService(request.getServiceDefinition(), request.getPlan()) + .flatMap(targetSpec -> targetService.addToBackingApplications(backingApps, targetSpec, request.getServiceInstanceId())) + .defaultIfEmpty(backingApps)) .flatMap(backingApps -> appsParametersTransformationService.transformParameters(backingApps, request.getParameters())) .flatMapMany(backingApps -> deploymentService.update(backingApps, request.getServiceInstanceId())) diff --git a/spring-cloud-app-broker-core/src/test/java/org/springframework/cloud/appbroker/workflow/instance/AppDeploymentDeleteServiceInstanceWorkflowTest.java b/spring-cloud-app-broker-core/src/test/java/org/springframework/cloud/appbroker/workflow/instance/AppDeploymentDeleteServiceInstanceWorkflowTest.java index d707625..8b3c646 100644 --- a/spring-cloud-app-broker-core/src/test/java/org/springframework/cloud/appbroker/workflow/instance/AppDeploymentDeleteServiceInstanceWorkflowTest.java +++ b/spring-cloud-app-broker-core/src/test/java/org/springframework/cloud/appbroker/workflow/instance/AppDeploymentDeleteServiceInstanceWorkflowTest.java @@ -234,8 +234,6 @@ class AppDeploymentDeleteServiceInstanceWorkflowTest { // no backing apps given(this.backingAppManagementService.getDeployedBackingApplications(eq(request.getServiceInstanceId()))) .willReturn(Mono.empty()); - given(this.credentialProviderService.deleteCredentials(any(), eq(request.getServiceInstanceId()))) - .willReturn(Mono.empty()); given(this.backingServicesProvisionService.deleteServiceInstance(argThat(backingServices -> { boolean nameMatch = "my-service-instance".equals(backingServices.get(0).getServiceInstanceName());