fix: Store duration values in a normalized format #27161
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
korthout
approved these changes
Jan 23, 2025
Comment on lines
250
to
251
| public void shouldReturnNormalizedPeriod() { | ||
| final var evaluationResult = evaluateExpression("=duration(\"P2Y\")"); |
There was a problem hiding this comment.
❓ Should we also add a test case for period and duration with two separate units?
e.g.
P2Y3MP5DT3H2M
Comment on lines
235
to
237
| @Test | ||
| public void shouldReturnNormalizedDuration() { | ||
| final var evaluationResult = evaluateExpression("=duration(\"P5D\")"); |
There was a problem hiding this comment.
🔧 We could show the normalization by starting with a non-normalized format:
PT120H->P5D
.../test/java/io/camunda/zeebe/engine/processing/variable/mapping/ActivityInputMappingTest.java
Show resolved
Hide resolved
Add a test case to verify that a duration value is stored in a normalized format.
Use toString() of FEEL duration to serialize the value into a JSON variable. Compared to the Java's duration value, the FEEL value returns the duration in a normalized format (i.e. in the biggest available unit).
Adjust the duration value of existing test cases to expect a normalized format.
saig0
force-pushed
the
27098-feel-duration
branch
from
e56f6ff to
10cda94
Compare
saig0
added
backport stable/8.6
Update the JUnit Maven dependency and the test annotations from JUnit 4 to JUnit 5.
Add more cases to verify the normalized duration formation using JUnit's parameterized tests.
saig0
force-pushed
the
27098-feel-duration
branch
from
10cda94 to
71aa2d3
Compare
github-merge-queue bot
pushed a commit
that referenced
this pull request
Jan 23, 2025
## Description Use `toString()` of FEEL duration to serialize the value into a JSON variable. Compared to the Java's duration value, the FEEL value returns the duration in a normalized format (i.e. in the biggest available unit). This fix aligns the engine with the [docs](https://docs.camunda.io/docs/components/modeler/feel/language-guide/feel-temporal-expressions/#subtraction) and avoids the workaround using the [string()](https://docs.camunda.io/docs/components/modeler/feel/builtin-functions/feel-built-in-functions-conversion/#stringfrom) function. ``` // before "PT120H" // after "P5D" ``` ## Related issues closes #27098
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
|
Created backport PR for
Please cherry-pick the changes locally and resolve any conflicts. git fetch origin backport-27161-to-stable/8.3
git worktree add --checkout .worktree/backport-27161-to-stable/8.3 backport-27161-to-stable/8.3
cd .worktree/backport-27161-to-stable/8.3
git reset --hard HEAD^
git cherry-pick -x e7aed5e34b8b24f3dc5078a45a4351db5def4d85 8fb4232e6a65d65c34218a7498fe2942ea2a8ea0 8ee6f31d622d76755dffa2376510ecabde259005 71aa2d3eef0b6d7d553db5b054c6711d354e4664
git push --force-with-lease |
|
Created backport PR for
Please cherry-pick the changes locally and resolve any conflicts. git fetch origin backport-27161-to-stable/8.4
git worktree add --checkout .worktree/backport-27161-to-stable/8.4 backport-27161-to-stable/8.4
cd .worktree/backport-27161-to-stable/8.4
git reset --hard HEAD^
git cherry-pick -x e7aed5e34b8b24f3dc5078a45a4351db5def4d85 8fb4232e6a65d65c34218a7498fe2942ea2a8ea0 8ee6f31d622d76755dffa2376510ecabde259005 71aa2d3eef0b6d7d553db5b054c6711d354e4664
git push --force-with-lease |
|
Created backport PR for
Please cherry-pick the changes locally and resolve any conflicts. git fetch origin backport-27161-to-stable/8.5
git worktree add --checkout .worktree/backport-27161-to-stable/8.5 backport-27161-to-stable/8.5
cd .worktree/backport-27161-to-stable/8.5
git reset --hard HEAD^
git cherry-pick -x e7aed5e34b8b24f3dc5078a45a4351db5def4d85 8fb4232e6a65d65c34218a7498fe2942ea2a8ea0 8ee6f31d622d76755dffa2376510ecabde259005 71aa2d3eef0b6d7d553db5b054c6711d354e4664
git push --force-with-lease |
|
Successfully created backport PR for |
|
Successfully created backport PR for |
github-actions
bot
added
the
version:8.5.14
rodrigo-lourenco-lopes
added
the
version:8.7.0-alpha4
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Use
toString()of FEEL duration to serialize the value into a JSON variable. Compared to the Java's duration value, theFEEL value returns the duration in a normalized format (i.e. in the biggest available unit).
This fix aligns the engine with the docs and avoids the workaround using the string() function.
Related issues
closes #27098