Merge pull request #109 from meshcloud/feature/fix-nullable-planid-in-updates

fix: Nullable planId in ServiceInstance updates

Author: meshkodiak[bot]

src/main/kotlin/io/meshcloud/dockerosb/model/ServiceInstance.kt

@@ -34,14 +34,17 @@ data class ServiceInstance(
       serviceDefinitionId = request.serviceDefinitionId
   )
 
-    constructor(request: UpdateServiceInstanceRequest) : this(
-        serviceInstanceId = request.serviceInstanceId,
+  fun update(request: UpdateServiceInstanceRequest): ServiceInstance {
+    require(serviceInstanceId == request.serviceInstanceId)
+
+    return copy(
         asyncAccepted = request.isAsyncAccepted,
-        context = request.context,
-        originatingIdentity = request.originatingIdentity,
-        parameters = request.parameters ?: mutableMapOf(),
-        planId = request.planId,
-        serviceDefinition = ServiceDefinition(request.serviceDefinition),
-        serviceDefinitionId = request.serviceDefinitionId
+        context = request.context ?: context,
+        originatingIdentity = request.originatingIdentity ?: originatingIdentity,
+        parameters = request.parameters ?: request.parameters,
+        planId = request.planId ?: planId,
+        serviceDefinition = request.serviceDefinition?.let { ServiceDefinition(it) } ?: serviceDefinition,
+        serviceDefinitionId = request.serviceDefinitionId ?: serviceDefinitionId
     )
+  }
 }

src/main/kotlin/io/meshcloud/dockerosb/service/GenericServiceInstanceService.kt

@@ -42,10 +42,12 @@ class GenericServiceInstanceService(
     }
 
     gitContextFactory.acquireContext().use { context ->
-      val serviceInstance = ServiceInstance(request)
-
       val repository = context.buildServiceInstanceRepository()
-      repository.updateServiceInstance(serviceInstance)
+      val existingInstance = repository.tryGetServiceInstance(request.serviceInstanceId)
+          ?: throw ServiceInstanceDoesNotExistException(request.serviceInstanceId)
+
+      val updatedInstance = existingInstance.update(request)
+      repository.updateServiceInstance(updatedInstance)
 
       return Mono.just(
           UpdateServiceInstanceResponse.builder()

src/test/kotlin/io/meshcloud/dockerosb/DockerOsbApplicationTests.kt

@@ -0,0 +1,39 @@
+package io.meshcloud.dockerosb.scenario
+
+import io.meshcloud.dockerosb.ServiceBrokerFixture
+import io.meshcloud.dockerosb.persistence.GitOperationContextFactory
+import io.meshcloud.dockerosb.persistence.ServiceInstanceRepository
+import org.junit.After
+import org.junit.Before
+import org.junit.runner.RunWith
+import org.springframework.beans.factory.annotation.Autowired
+import org.springframework.boot.test.context.SpringBootTest
+import org.springframework.test.context.ActiveProfiles
+import org.springframework.test.context.junit4.SpringRunner
+
+@RunWith(SpringRunner::class)
+@SpringBootTest
+@ActiveProfiles("test")
+abstract class DockerOsbApplicationTests {
+
+  protected lateinit var fixture: ServiceBrokerFixture
+
+  @Autowired
+  private lateinit var gitContextFactory: GitOperationContextFactory
+
+  @Before
+  fun before() {
+    fixture = ServiceBrokerFixture("src/test/resources/catalog.yml")
+  }
+
+  @After
+  fun cleanUp() {
+    fixture.close()
+  }
+
+  protected fun useServiceInstanceRepository(fn: (repository: ServiceInstanceRepository) -> Unit) {
+    gitContextFactory.acquireContext().use {
+      fn(it.buildServiceInstanceRepository())
+    }
+  }
+}

src/test/kotlin/io/meshcloud/dockerosb/scenario/DockerOsbApplicationTests.kt

@@ -0,0 +1,41 @@
+package io.meshcloud.dockerosb.scenario
+
+import io.meshcloud.dockerosb.service.GenericServiceInstanceService
+import org.junit.Assert.*
+import org.junit.Test
+import org.springframework.beans.factory.annotation.Autowired
+
+class UpdateServiceInstanceScenario : DockerOsbApplicationTests() {
+
+  @Autowired
+  private lateinit var serviceInstanceService: GenericServiceInstanceService
+
+  @Test
+  fun `update request with null plan id does update the service instance without the plan`() {
+    val instanceId = "e4bd6a78-7e05-4d5a-97b8-f8c5d1c710da"
+    val createRequest = fixture.builder.createServiceInstanceRequest(instanceId)
+    serviceInstanceService.createServiceInstance(createRequest)
+
+    val createdPlanId = useServiceInstanceRepository { repository ->
+      val instance = repository.tryGetServiceInstance(instanceId)
+      instance!!.planId
+    }
+
+    val updateRequestWithNullPlanId = fixture.builder.updateServiceInstanceRequest(instanceId) {
+      planId(null)
+    }
+
+    val response = serviceInstanceService.updateServiceInstance(updateRequestWithNullPlanId)
+
+    response.subscribe {
+      assertEquals("updating service", it.operation)
+    }
+
+    val updatedPlanId = useServiceInstanceRepository { repository ->
+      val instance = repository.tryGetServiceInstance(instanceId)
+      instance!!.planId
+    }
+    
+    assertEquals(createdPlanId, updatedPlanId)
+  }
+}

src/test/kotlin/io/meshcloud/dockerosb/scenario/UpdateServiceInstanceScenario.kt

@@ -3,15 +3,12 @@ package io.meshcloud.dockerosb.service
 import io.meshcloud.dockerosb.ServiceBrokerFixture
 import io.meshcloud.dockerosb.model.ServiceBinding
 import io.meshcloud.dockerosb.model.Status
-import org.apache.commons.io.FileUtils
 import org.junit.After
 import org.junit.Assert
 import org.junit.Before
 import org.junit.Test
 import org.springframework.cloud.servicebroker.exception.ServiceBrokerAsyncRequiredException
 import org.springframework.cloud.servicebroker.model.binding.GetLastServiceBindingOperationRequest
-import org.springframework.cloud.servicebroker.model.instance.DeleteServiceInstanceRequest
-import org.springframework.cloud.servicebroker.model.instance.GetLastServiceOperationRequest
 import org.springframework.cloud.servicebroker.model.instance.OperationState
 import java.io.File