refactor(engine): Fix Tests for null & Empty createTimeConfig

camunda · Nov 8, 2023 · 5d7a359 · 5d7a359
1 parent ccf8675
commit 5d7a359
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 22 deletions.
diff --git a/...est/src/main/java/org/camunda/bpm/engine/rest/dto/externaltask/FetchExternalTasksDto.java b/...est/src/main/java/org/camunda/bpm/engine/rest/dto/externaltask/FetchExternalTasksDto.java
@@ -225,9 +225,7 @@ public void setIncludeExtensionProperties(boolean includeExtensionProperties) {
   }
 
   public ExternalTaskQueryBuilder buildQuery(ProcessEngine processEngine) {
-
-    var configDto = getCreateTimeConfig();
-    var config = getConfigFromDto(configDto);
+    var config = getConfigFromDto();
 
     ExternalTaskQueryBuilder fetchBuilder = processEngine
       .getExternalTaskService()
@@ -297,7 +295,9 @@ public ExternalTaskQueryBuilder buildQuery(ProcessEngine processEngine) {
     return fetchBuilder;
   }
 
-  protected CreateTimeConfig getConfigFromDto(CreateTimeConfigDto dto) {
+  protected CreateTimeConfig getConfigFromDto() {
+    var dto = getCreateTimeConfig();
+
     if (dto == null || !dto.isUseCreateTime()) {
       return EMPTY;
     }

diff --git a/engine/src/main/java/org/camunda/bpm/engine/externaltask/CreateTimeConfig.java b/engine/src/main/java/org/camunda/bpm/engine/externaltask/CreateTimeConfig.java
@@ -23,5 +23,9 @@
  * EMPTY configuration means ignoring the field at all.
  */
 public enum CreateTimeConfig {
-  ASC, DESC, EMPTY
+  ASC, DESC, EMPTY;
+
+  public boolean isEmpty() {
+    return this == EMPTY;
+  }
 }
diff --git a/engine/src/main/java/org/camunda/bpm/engine/impl/ExternalTaskServiceImpl.java b/engine/src/main/java/org/camunda/bpm/engine/impl/ExternalTaskServiceImpl.java
@@ -53,9 +53,10 @@ public ExternalTaskQueryBuilder fetchAndLock(int maxTasks, String workerId) {
 
   @Override
   public ExternalTaskQueryBuilder fetchAndLock(int maxTasks, String workerId, boolean usePriority, CreateTimeConfig createTimeConfig) {
-    var direction = getDirection(createTimeConfig);
+    var useCreateTime = useCreateTime(createTimeConfig);
+    var direction = useCreateTime ? getDirection(createTimeConfig) : null;
 
-    return new ExternalTaskQueryTopicBuilderImpl(commandExecutor, workerId, maxTasks, usePriority, true, direction);
+    return new ExternalTaskQueryTopicBuilderImpl(commandExecutor, workerId, maxTasks, usePriority, useCreateTime, direction);
   }
 
   @Override
@@ -171,13 +172,17 @@ public void extendLock(String externalTaskId, String workerId, long lockDuration
   }
 
   protected Direction getDirection(CreateTimeConfig config) {
-    if (config == null || config == EMPTY) {
+    if (config == null || config.isEmpty()) {
       return Direction.DESCENDING; // default order in case of null or empty config
     }
 
     return Direction.findByNameIgnoreCase(config.name());
   }
 
+  protected boolean useCreateTime(CreateTimeConfig config) {
+    return config != null && !config.isEmpty();
+  }
+
 
 
 }
diff --git a/...e/src/test/java/org/camunda/bpm/engine/test/api/externaltask/ExternalTaskServiceTest.java b/...e/src/test/java/org/camunda/bpm/engine/test/api/externaltask/ExternalTaskServiceTest.java
@@ -325,7 +325,7 @@ public void shouldFetchWithCreateTimeDESCWithoutPriority() {
       "org/camunda/bpm/engine/test/api/externaltask/twoExternalTaskWithPriorityProcess.bpmn20.xml"
   })
   @Test
-  public void shouldFetchWithDefaultDESCOrderWhenCreateTimeOrderIsEmpty() {
+  public void shouldIgnoreCreateTimeConfigWhenOrderIsEmpty() {
     // given
     runtimeService.startProcessInstanceByKey("twoExternalTaskWithPriorityProcess"); // priority 7 & null
     ClockTestUtil.incrementClock(60_000);
@@ -334,26 +334,26 @@ public void shouldFetchWithDefaultDESCOrderWhenCreateTimeOrderIsEmpty() {
     runtimeService.startProcessInstanceByKey("oneExternalTaskProcess"); // null priority
 
     // when
-    var result = externalTaskService.fetchAndLock(6, WORKER_ID, false, EMPTY)
+    var result = externalTaskService.fetchAndLock(6, WORKER_ID, true, EMPTY)
         .topic(TOPIC_NAME, LOCK_TIME)
         .execute();
 
     assertThat(result.size()).isEqualTo(5);
 
     // then
-    // DESC order will be used as a 'sensible' default order
-    assertThat(result.get(0).getCreateTime()).isAfterOrEqualsTo(result.get(1).getCreateTime());
-    assertThat(result.get(1).getCreateTime()).isAfterOrEqualsTo(result.get(2).getCreateTime());
-    assertThat(result.get(2).getCreateTime()).isAfterOrEqualsTo(result.get(3).getCreateTime());
-    assertThat(result.get(3).getCreateTime()).isAfterOrEqualsTo(result.get(4).getCreateTime());
+    // create time ordering will be ignored, only priority will be used
+    assertThat(result.get(0).getPriority()).isGreaterThanOrEqualTo(result.get(1).getPriority());
+    assertThat(result.get(1).getPriority()).isGreaterThanOrEqualTo(result.get(2).getPriority());
+    assertThat(result.get(2).getPriority()).isGreaterThanOrEqualTo(result.get(3).getPriority());
+    assertThat(result.get(3).getPriority()).isGreaterThanOrEqualTo(result.get(4).getPriority());
   }
 
   @Deployment(resources = {
       "org/camunda/bpm/engine/test/api/externaltask/oneExternalTaskProcess.bpmn20.xml",
       "org/camunda/bpm/engine/test/api/externaltask/twoExternalTaskWithPriorityProcess.bpmn20.xml"
   })
   @Test
-  public void shouldFetchWithDefaultDESCOrderWhenCreateTimeOrderIsNull() {
+  public void shouldIgnoreCreateTimeConfigWhenOrderIsNull() {
     // given
     runtimeService.startProcessInstanceByKey("twoExternalTaskWithPriorityProcess"); // priority 7 & null
     ClockTestUtil.incrementClock(60_000);
@@ -362,18 +362,18 @@ public void shouldFetchWithDefaultDESCOrderWhenCreateTimeOrderIsNull() {
     runtimeService.startProcessInstanceByKey("oneExternalTaskProcess"); // null priority
 
     // when
-    var result = externalTaskService.fetchAndLock(6, WORKER_ID, false, null)
+    var result = externalTaskService.fetchAndLock(6, WORKER_ID, true, null)
         .topic(TOPIC_NAME, LOCK_TIME)
         .execute();
 
     assertThat(result.size()).isEqualTo(5);
 
     // then
-    // DESC order will be used as a 'sensible' default order
-    assertThat(result.get(0).getCreateTime()).isAfterOrEqualsTo(result.get(1).getCreateTime());
-    assertThat(result.get(1).getCreateTime()).isAfterOrEqualsTo(result.get(2).getCreateTime());
-    assertThat(result.get(2).getCreateTime()).isAfterOrEqualsTo(result.get(3).getCreateTime());
-    assertThat(result.get(3).getCreateTime()).isAfterOrEqualsTo(result.get(4).getCreateTime());
+    // create time ordering will be ignored, only priority will be used
+    assertThat(result.get(0).getPriority()).isGreaterThanOrEqualTo(result.get(1).getPriority());
+    assertThat(result.get(1).getPriority()).isGreaterThanOrEqualTo(result.get(2).getPriority());
+    assertThat(result.get(2).getPriority()).isGreaterThanOrEqualTo(result.get(3).getPriority());
+    assertThat(result.get(3).getPriority()).isGreaterThanOrEqualTo(result.get(4).getPriority());
   }
 
   @Deployment(resources = "org/camunda/bpm/engine/test/api/externaltask/externalTaskPriorityProcess.bpmn20.xml")