From f949f6fbb847badc8a10a7e35f43bff6e1d16b19 Mon Sep 17 00:00:00 2001 From: Stefan Zilske Date: Wed, 15 Jun 2022 13:22:31 +0200 Subject: [PATCH 1/2] fix: Delegate to VariableMapAdapter, fixes #254 --- .../basic/ReadAdapterLockedExternalTask.java | 17 ++-- .../data/factory/BasicVariableFactory.java | 2 +- .../reader/LockedExternalTaskReaderTest.java | 9 ++ .../ReadAdapterLockedExternalTaskTest.kt | 97 +++++++++++++++++++ 4 files changed, 114 insertions(+), 11 deletions(-) create mode 100644 extension/core/src/test/kotlin/io/holunda/camunda/bpm/data/adapter/basic/ReadAdapterLockedExternalTaskTest.kt diff --git a/extension/core/src/main/java/io/holunda/camunda/bpm/data/adapter/basic/ReadAdapterLockedExternalTask.java b/extension/core/src/main/java/io/holunda/camunda/bpm/data/adapter/basic/ReadAdapterLockedExternalTask.java index ccab48c2..e69200c4 100644 --- a/extension/core/src/main/java/io/holunda/camunda/bpm/data/adapter/basic/ReadAdapterLockedExternalTask.java +++ b/extension/core/src/main/java/io/holunda/camunda/bpm/data/adapter/basic/ReadAdapterLockedExternalTask.java @@ -14,24 +14,21 @@ @SuppressWarnings("java:S1192") public class ReadAdapterLockedExternalTask implements ReadAdapter { - private final LockedExternalTask lockedExternalTask; - private final String variableName; + private final ReadAdapter readAdapter; - public ReadAdapterLockedExternalTask(LockedExternalTask lockedExternalTask, String variableName) { - this.lockedExternalTask = lockedExternalTask; - this.variableName = variableName; + public ReadAdapterLockedExternalTask(LockedExternalTask lockedExternalTask, String variableName, Class clazz) { + readAdapter = new ReadWriteAdapterVariableMap<>(lockedExternalTask.getVariables(), variableName, clazz); } @Override public T get() { - return getOptional().orElseThrow(() -> new VariableNotFoundException("Couldn't find required variable '" + variableName + "'")); + return readAdapter.get(); } @Override @SuppressWarnings("unchecked") public Optional getOptional() { - return (Optional) Optional.ofNullable(lockedExternalTask.getVariables()) - .map(it -> it.get(variableName)); + return readAdapter.getOptional(); } @Override @@ -46,7 +43,7 @@ public Optional getLocalOptional() { @Override public T getOrDefault(T defaultValue) { - return getOptional().orElse(defaultValue); + return readAdapter.getOrDefault(defaultValue); } @Override @@ -56,7 +53,7 @@ public T getLocalOrDefault(T defaultValue) { @Override public T getOrNull() { - return getOptional().orElse(null); + return readAdapter.getOrNull(); } @Override diff --git a/extension/core/src/main/java/io/holunda/camunda/bpm/data/factory/BasicVariableFactory.java b/extension/core/src/main/java/io/holunda/camunda/bpm/data/factory/BasicVariableFactory.java index 2ba9538d..e5be4858 100644 --- a/extension/core/src/main/java/io/holunda/camunda/bpm/data/factory/BasicVariableFactory.java +++ b/extension/core/src/main/java/io/holunda/camunda/bpm/data/factory/BasicVariableFactory.java @@ -94,7 +94,7 @@ public ReadAdapter from(CaseService caseService, String caseExecutionId) { @Override public ReadAdapter from(LockedExternalTask lockedExternalTask) { - return new ReadAdapterLockedExternalTask<>(lockedExternalTask, name); + return new ReadAdapterLockedExternalTask<>(lockedExternalTask, name, clazz); } /** diff --git a/extension/core/src/test/java/io/holunda/camunda/bpm/data/reader/LockedExternalTaskReaderTest.java b/extension/core/src/test/java/io/holunda/camunda/bpm/data/reader/LockedExternalTaskReaderTest.java index e9d1b039..01646f55 100644 --- a/extension/core/src/test/java/io/holunda/camunda/bpm/data/reader/LockedExternalTaskReaderTest.java +++ b/extension/core/src/test/java/io/holunda/camunda/bpm/data/reader/LockedExternalTaskReaderTest.java @@ -10,11 +10,13 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.UUID; import static io.holunda.camunda.bpm.data.CamundaBpmData.listVariable; import static io.holunda.camunda.bpm.data.CamundaBpmData.mapVariable; import static io.holunda.camunda.bpm.data.CamundaBpmData.setVariable; import static io.holunda.camunda.bpm.data.CamundaBpmData.stringVariable; +import static io.holunda.camunda.bpm.data.CamundaBpmData.uuidVariable; import static java.util.Arrays.asList; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; @@ -25,11 +27,13 @@ public class LockedExternalTaskReaderTest { private static final VariableFactory STRING = stringVariable("myString"); + private static final VariableFactory UUID = uuidVariable("myUuid"); private static final VariableFactory> LIST = listVariable("myList", String.class); private static final VariableFactory> SET = setVariable("mySet", String.class); private static final VariableFactory> MAP = mapVariable("myMap", String.class, String.class); private final String stringValue = "value"; + private final UUID uuidValue = java.util.UUID.randomUUID(); private final List listValue = asList("foo", "bar"); private final Set setValue = asHashSet("foo", "bar"); private final Map mapValue = Map.of("a", "b", "c", "d"); @@ -46,6 +50,7 @@ public void setUp() { .putValue(LIST.getName(), listValue) .putValue(SET.getName(), setValue) .putValue(MAP.getName(), mapValue) + .putValue(UUID.getName(), uuidValue) ); } @@ -55,6 +60,8 @@ public void shouldDelegateGet() { assertThat(reader.get(LIST)).isEqualTo(listValue); assertThat(reader.get(SET)).isEqualTo(setValue); assertThat(reader.get(MAP)).isEqualTo(mapValue); + assertThat(reader.get(UUID)).isEqualTo(uuidValue); + assertThat(reader.get(UUID)).isInstanceOf(java.util.UUID.class); } @Test @@ -63,6 +70,7 @@ public void shouldDelegateGetOptional() { assertThat(reader.getOptional(LIST)).hasValue(listValue); assertThat(reader.getOptional(SET)).hasValue(setValue); assertThat(reader.getOptional(MAP)).hasValue(mapValue); + assertThat(reader.getOptional(UUID)).hasValue(uuidValue); assertThat(reader.getOptional(stringVariable("xxx"))).isEmpty(); } @@ -86,6 +94,7 @@ public void shouldDelegateGetOrNull() { assertThat(reader.getOrNull(LIST)).isEqualTo(listValue); assertThat(reader.getOrNull(SET)).isEqualTo(setValue); assertThat(reader.getOrNull(MAP)).isEqualTo(mapValue); + assertThat(reader.getOrNull(UUID)).isEqualTo(uuidValue); assertThat(reader.getOrNull(stringVariable("xxx"))).isNull(); } diff --git a/extension/core/src/test/kotlin/io/holunda/camunda/bpm/data/adapter/basic/ReadAdapterLockedExternalTaskTest.kt b/extension/core/src/test/kotlin/io/holunda/camunda/bpm/data/adapter/basic/ReadAdapterLockedExternalTaskTest.kt new file mode 100644 index 00000000..7fa3a259 --- /dev/null +++ b/extension/core/src/test/kotlin/io/holunda/camunda/bpm/data/adapter/basic/ReadAdapterLockedExternalTaskTest.kt @@ -0,0 +1,97 @@ +package io.holunda.camunda.bpm.data.adapter.basic + +import io.holunda.camunda.bpm.data.CamundaBpmData +import org.assertj.core.api.Assertions.assertThat +import org.camunda.bpm.engine.externaltask.LockedExternalTask +import org.camunda.bpm.engine.variable.VariableMap +import org.junit.Test +import java.util.* + +class ReadAdapterLockedExternalTaskTest { + + private val uuidVar = CamundaBpmData.uuidVariable("uuidVar") + + @Test + fun `get UUID type from variableMap directly`() { + val map = CamundaBpmData.builder() + .set(uuidVar, UUID.randomUUID()) + .build() + + assertThat(uuidVar.from(map).get()).isInstanceOf(UUID::class.java) + } + + @Test + fun `get UUID type from ExternalTask`() { + val variables = CamundaBpmData.builder() + .set(uuidVar, UUID.randomUUID()) + .build() + + val task = LockedExternalTaskFake( + id = UUID.randomUUID().toString(), + variables = variables + ) + + assertThat(task.variables).isSameAs(variables) + assertThat(uuidVar.from(task).get()).isInstanceOf(UUID::class.java) + } +} + +data class LockedExternalTaskFake( + private var id: String? = null, + private var topicName: String? = null, + private var workerId: String? = null, + private var lockExpirationTime: Date? = null, + private var processInstanceId: String? = null, + private var executionId: String? = null, + private var activityId: String? = null, + private var activityInstanceId: String? = null, + private var processDefinitionId: String? = null, + private var processDefinitionKey: String? = null, + private var processDefinitionVersionTag: String? = null, + private var retries: Int? = null, + private var errorMessage: String? = null, + private var errorDetails: String? = null, + private var variables: VariableMap? = null, + private var tenantId: String? = null, + private var priority: Long = 0, + private var businessKey: String? = null, + private var extensionProperties: Map? = null, +) : LockedExternalTask { + override fun getId() = id + + override fun getTopicName() = topicName + + override fun getWorkerId() = workerId + + override fun getLockExpirationTime() = lockExpirationTime + + override fun getProcessInstanceId() = processInstanceId + + override fun getExecutionId() = executionId + + override fun getActivityId() = activityId + + override fun getActivityInstanceId() = activityInstanceId + + override fun getProcessDefinitionId() = processDefinitionId + + override fun getProcessDefinitionKey() = processDefinitionKey + + override fun getProcessDefinitionVersionTag() = processDefinitionVersionTag + + override fun getRetries() = retries + + override fun getErrorMessage() = errorMessage + + override fun getErrorDetails() = errorDetails + + override fun getVariables() = variables + + override fun getTenantId() = tenantId + + override fun getPriority() = priority + + override fun getBusinessKey() = businessKey + + override fun getExtensionProperties() = extensionProperties +} From 265f88dfae81c0a5026d5a3477890c10c29f1631 Mon Sep 17 00:00:00 2001 From: Stefan Zilske Date: Wed, 15 Jun 2022 13:52:15 +0200 Subject: [PATCH 2/2] fix: Code smells --- .../basic/ReadAdapterLockedExternalTask.java | 1 - .../reader/LockedExternalTaskReaderTest.java | 70 +++++++++---------- 2 files changed, 35 insertions(+), 36 deletions(-) diff --git a/extension/core/src/main/java/io/holunda/camunda/bpm/data/adapter/basic/ReadAdapterLockedExternalTask.java b/extension/core/src/main/java/io/holunda/camunda/bpm/data/adapter/basic/ReadAdapterLockedExternalTask.java index e69200c4..1455864e 100644 --- a/extension/core/src/main/java/io/holunda/camunda/bpm/data/adapter/basic/ReadAdapterLockedExternalTask.java +++ b/extension/core/src/main/java/io/holunda/camunda/bpm/data/adapter/basic/ReadAdapterLockedExternalTask.java @@ -1,7 +1,6 @@ package io.holunda.camunda.bpm.data.adapter.basic; import io.holunda.camunda.bpm.data.adapter.ReadAdapter; -import io.holunda.camunda.bpm.data.adapter.VariableNotFoundException; import org.camunda.bpm.engine.externaltask.LockedExternalTask; import java.util.Optional; diff --git a/extension/core/src/test/java/io/holunda/camunda/bpm/data/reader/LockedExternalTaskReaderTest.java b/extension/core/src/test/java/io/holunda/camunda/bpm/data/reader/LockedExternalTaskReaderTest.java index 01646f55..46b41472 100644 --- a/extension/core/src/test/java/io/holunda/camunda/bpm/data/reader/LockedExternalTaskReaderTest.java +++ b/extension/core/src/test/java/io/holunda/camunda/bpm/data/reader/LockedExternalTaskReaderTest.java @@ -26,14 +26,14 @@ public class LockedExternalTaskReaderTest { - private static final VariableFactory STRING = stringVariable("myString"); - private static final VariableFactory UUID = uuidVariable("myUuid"); - private static final VariableFactory> LIST = listVariable("myList", String.class); - private static final VariableFactory> SET = setVariable("mySet", String.class); - private static final VariableFactory> MAP = mapVariable("myMap", String.class, String.class); + private static final VariableFactory STRING_VAR = stringVariable("myString"); + private static final VariableFactory UUID_VAR = uuidVariable("myUuid"); + private static final VariableFactory> LIST_VAR = listVariable("myList", String.class); + private static final VariableFactory> SET_VAR = setVariable("mySet", String.class); + private static final VariableFactory> MAP_VAR = mapVariable("myMap", String.class, String.class); private final String stringValue = "value"; - private final UUID uuidValue = java.util.UUID.randomUUID(); + private final UUID uuidValue = UUID.randomUUID(); private final List listValue = asList("foo", "bar"); private final Set setValue = asHashSet("foo", "bar"); private final Map mapValue = Map.of("a", "b", "c", "d"); @@ -46,71 +46,71 @@ public class LockedExternalTaskReaderTest { public void setUp() { when(externalTask.getVariables()).thenReturn( Variables - .putValue(STRING.getName(), stringValue) - .putValue(LIST.getName(), listValue) - .putValue(SET.getName(), setValue) - .putValue(MAP.getName(), mapValue) - .putValue(UUID.getName(), uuidValue) + .putValue(STRING_VAR.getName(), stringValue) + .putValue(LIST_VAR.getName(), listValue) + .putValue(SET_VAR.getName(), setValue) + .putValue(MAP_VAR.getName(), mapValue) + .putValue(UUID_VAR.getName(), uuidValue) ); } @Test public void shouldDelegateGet() { - assertThat(reader.get(STRING)).isEqualTo(stringValue); - assertThat(reader.get(LIST)).isEqualTo(listValue); - assertThat(reader.get(SET)).isEqualTo(setValue); - assertThat(reader.get(MAP)).isEqualTo(mapValue); - assertThat(reader.get(UUID)).isEqualTo(uuidValue); - assertThat(reader.get(UUID)).isInstanceOf(java.util.UUID.class); + assertThat(reader.get(STRING_VAR)).isEqualTo(stringValue); + assertThat(reader.get(LIST_VAR)).isEqualTo(listValue); + assertThat(reader.get(SET_VAR)).isEqualTo(setValue); + assertThat(reader.get(MAP_VAR)).isEqualTo(mapValue); + assertThat(reader.get(UUID_VAR)).isEqualTo(uuidValue); + assertThat(reader.get(UUID_VAR)).isInstanceOf(UUID.class); } @Test public void shouldDelegateGetOptional() { - assertThat(reader.getOptional(STRING)).hasValue(stringValue); - assertThat(reader.getOptional(LIST)).hasValue(listValue); - assertThat(reader.getOptional(SET)).hasValue(setValue); - assertThat(reader.getOptional(MAP)).hasValue(mapValue); - assertThat(reader.getOptional(UUID)).hasValue(uuidValue); + assertThat(reader.getOptional(STRING_VAR)).hasValue(stringValue); + assertThat(reader.getOptional(LIST_VAR)).hasValue(listValue); + assertThat(reader.getOptional(SET_VAR)).hasValue(setValue); + assertThat(reader.getOptional(MAP_VAR)).hasValue(mapValue); + assertThat(reader.getOptional(UUID_VAR)).hasValue(uuidValue); assertThat(reader.getOptional(stringVariable("xxx"))).isEmpty(); } @Test public void shouldDelegateGetLocalOptional() { - assertThatThrownBy(() -> reader.getLocalOptional(STRING)) + assertThatThrownBy(() -> reader.getLocalOptional(STRING_VAR)) .isInstanceOf(UnsupportedOperationException.class) .hasMessage("Can't get a local variable on an external task"); } @Test public void shouldDelegateGetLocal() { - assertThatThrownBy(() -> reader.getLocal(STRING)) + assertThatThrownBy(() -> reader.getLocal(STRING_VAR)) .isInstanceOf(UnsupportedOperationException.class) .hasMessage("Can't get a local variable on an external task"); } @Test public void shouldDelegateGetOrNull() { - assertThat(reader.getOrNull(STRING)).isEqualTo(stringValue); - assertThat(reader.getOrNull(LIST)).isEqualTo(listValue); - assertThat(reader.getOrNull(SET)).isEqualTo(setValue); - assertThat(reader.getOrNull(MAP)).isEqualTo(mapValue); - assertThat(reader.getOrNull(UUID)).isEqualTo(uuidValue); + assertThat(reader.getOrNull(STRING_VAR)).isEqualTo(stringValue); + assertThat(reader.getOrNull(LIST_VAR)).isEqualTo(listValue); + assertThat(reader.getOrNull(SET_VAR)).isEqualTo(setValue); + assertThat(reader.getOrNull(MAP_VAR)).isEqualTo(mapValue); + assertThat(reader.getOrNull(UUID_VAR)).isEqualTo(uuidValue); assertThat(reader.getOrNull(stringVariable("xxx"))).isNull(); } @Test public void shouldDelegateGetLocalOrNull() { - assertThatThrownBy(() -> reader.getLocalOrNull(STRING)) + assertThatThrownBy(() -> reader.getLocalOrNull(STRING_VAR)) .isInstanceOf(UnsupportedOperationException.class) .hasMessage("Can't get a local variable on an external task"); } @Test public void shouldDelegateGetOrDefault() { - assertThat(reader.getOrDefault(STRING, "default")).isEqualTo(stringValue); - assertThat(reader.getOrDefault(LIST, asList("a", "b"))).isEqualTo(listValue); - assertThat(reader.getOrDefault(SET, asHashSet("a", "b"))).isEqualTo(setValue); - assertThat(reader.getOrDefault(MAP, Map.of("a", "b", "c", "d"))).isEqualTo(mapValue); + assertThat(reader.getOrDefault(STRING_VAR, "default")).isEqualTo(stringValue); + assertThat(reader.getOrDefault(LIST_VAR, asList("a", "b"))).isEqualTo(listValue); + assertThat(reader.getOrDefault(SET_VAR, asHashSet("a", "b"))).isEqualTo(setValue); + assertThat(reader.getOrDefault(MAP_VAR, Map.of("a", "b", "c", "d"))).isEqualTo(mapValue); assertThat(reader.getOrDefault(stringVariable("xxx"), "default")).isEqualTo("default"); assertThat(reader.getOrDefault(listVariable("xxx", String.class), asList("a", "b"))).isEqualTo(asList("a", "b")); @@ -120,7 +120,7 @@ public void shouldDelegateGetOrDefault() { @Test public void shouldDelegateGetLocalOrDefault() { - assertThatThrownBy(() -> reader.getLocalOrDefault(STRING, stringValue)) + assertThatThrownBy(() -> reader.getLocalOrDefault(STRING_VAR, stringValue)) .isInstanceOf(UnsupportedOperationException.class) .hasMessage("Can't get a local variable on an external task"); }