You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kie.apache.org by "Copilot (via GitHub)" <gi...@apache.org> on 2026/10/13 17:39:44 UTC

Re: [PR] [kie-7138] Allow WS-Human Task lifecycle transition parameters to be … [incubator-kie]

Copilot commented on code in PR #7139:
URL: https://github.com/apache/incubator-kie/pull/7139#discussion_r4261995977


##########
kogito-jbpm/jbpm-usertask/src/main/java/org/kie/kogito/usertask/impl/lifecycle/WsHumanTaskLifeCycle.java:
##########
@@ -244,7 +245,25 @@ public UserTaskTransitionToken newTransitionToken(String transitionId, UserTaskS
                 .findAny()
                 .orElseThrow(() -> new UserTaskTransitionException("Invalid transition " + transitionId + " from " + state));
 
-        return new DefaultUserTaskTransitionToken(transition.id(), transition.source(), transition.target(), data);
+        Map<String, Object> normalizedData = new HashMap<>();
+        data.forEach((k, v) -> {
+            if (PARAMETER_USER.equalsIgnoreCase(k)) {
+                normalizedData.put(PARAMETER_USER, v);
+            } else if (PARAMETER_NOTIFY.equalsIgnoreCase(k)) {
+                normalizedData.put(PARAMETER_NOTIFY, v);
+            } else if (PARAMETER_DELEGATED_USER.equalsIgnoreCase(k)) {
+                normalizedData.put(PARAMETER_DELEGATED_USER, v);
+            } else if (PARAMETER_FORWARDED_USERS.equalsIgnoreCase(k)) {
+                normalizedData.put(PARAMETER_FORWARDED_USERS, v);
+            } else if (PARAMETER_NOMINATED_USERS.equalsIgnoreCase(k)) {
+                normalizedData.put(PARAMETER_NOMINATED_USERS, v);
+            } else if (PARAMETER_SUSPEND_UNTIL.equalsIgnoreCase(k)) {
+                normalizedData.put(PARAMETER_SUSPEND_UNTIL, v);
+            } else {
+                normalizedData.put(k, v);
+            }
+        });

Review Comment:
   If `data` contains multiple entries that differ only by case (e.g., `user` and `USER`), normalization will collapse them into the same canonical key and silently overwrite one value. Since iteration order depends on the concrete `Map` implementation, the resulting value can be non-deterministic. Consider defining deterministic behavior (e.g., prefer the exact canonical key if present, otherwise first-wins/last-wins explicitly) or rejecting ambiguous input by throwing a `UserTaskTransitionException` when duplicates are detected.



##########
kogito-jbpm/jbpm-usertask/src/main/java/org/kie/kogito/usertask/impl/lifecycle/WsHumanTaskLifeCycle.java:
##########
@@ -244,7 +245,25 @@ public UserTaskTransitionToken newTransitionToken(String transitionId, UserTaskS
                 .findAny()
                 .orElseThrow(() -> new UserTaskTransitionException("Invalid transition " + transitionId + " from " + state));
 
-        return new DefaultUserTaskTransitionToken(transition.id(), transition.source(), transition.target(), data);
+        Map<String, Object> normalizedData = new HashMap<>();
+        data.forEach((k, v) -> {
+            if (PARAMETER_USER.equalsIgnoreCase(k)) {
+                normalizedData.put(PARAMETER_USER, v);
+            } else if (PARAMETER_NOTIFY.equalsIgnoreCase(k)) {
+                normalizedData.put(PARAMETER_NOTIFY, v);
+            } else if (PARAMETER_DELEGATED_USER.equalsIgnoreCase(k)) {
+                normalizedData.put(PARAMETER_DELEGATED_USER, v);
+            } else if (PARAMETER_FORWARDED_USERS.equalsIgnoreCase(k)) {
+                normalizedData.put(PARAMETER_FORWARDED_USERS, v);
+            } else if (PARAMETER_NOMINATED_USERS.equalsIgnoreCase(k)) {
+                normalizedData.put(PARAMETER_NOMINATED_USERS, v);
+            } else if (PARAMETER_SUSPEND_UNTIL.equalsIgnoreCase(k)) {
+                normalizedData.put(PARAMETER_SUSPEND_UNTIL, v);
+            } else {
+                normalizedData.put(k, v);
+            }
+        });

Review Comment:
   The long `if/else` chain makes adding/removing normalized parameters error-prone. Consider using a lookup map (e.g., mapping lowercase keys to canonical constants) and normalize via a single lookup; this centralizes the list of canonical parameters and reduces branching.



##########
kogito-jbpm/jbpm-usertask/src/main/java/org/kie/kogito/usertask/impl/lifecycle/WsHumanTaskLifeCycle.java:
##########
@@ -244,7 +245,25 @@ public UserTaskTransitionToken newTransitionToken(String transitionId, UserTaskS
                 .findAny()
                 .orElseThrow(() -> new UserTaskTransitionException("Invalid transition " + transitionId + " from " + state));
 
-        return new DefaultUserTaskTransitionToken(transition.id(), transition.source(), transition.target(), data);
+        Map<String, Object> normalizedData = new HashMap<>();

Review Comment:
   Since `normalizedData` will typically receive ~`data.size()` entries, pre-sizing the `HashMap` (e.g., with `data.size()`) can avoid rehash/resizes for larger parameter maps.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org