Skip to content

MODWRKFLOW-85: Provide common/sane defaults during pre-process to act as appropriate initialization. - #174

Merged
kaladay merged 2 commits into
sprint18-stagingfrom
MODWRKFLOW-85
Oct 23, 2026
Merged

kaladay merged 2 commits into
sprint18-stagingfrom
MODWRKFLOW-85

Conversation

@kaladay

@kaladay kaladay commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Purpose

Resolves MODWRKFLOW-85.

Details

Remove all of the @NotNull annotations.
We shouldn't have the JSON handler be more opinionated. We can better use Java logic to parse and handle the situation. This better allows for providing defaults.
This reduces some of the problems with Jackson's highly opinionated, fickle, and very subject to breaking change designs.

Add @ColumnDefault to provide the default where possible. In any case where there is a columnDefinition is use, the @ColumnDefault cannot be used. Instead, directly specify the default within the columnDefinition. Use constant variables where possible, however, many cannot be used because Java complains that they are too dynamic to be used. In those cases, unfortunately, static values must be hard coded into the string itself. This is less maintainable, but the situation is forced by Java's inflexible design in this case. This can be looked at in the future where we could redefine some global statics or other behavioral change to reduce this problem. Such changes are largely out of scope for this issue.

The ScriptTask is udpated to use a SCRIPT_FORMAT_DEFAULT at a package level so that the unit test can better utilize this. The whole scriptFormat vs extension logic should be revisited in the future. We really shouldn't have these extra steps and fields to swap between.

Many of the text fields should instead be TEXT rather than VARCHAR. Under the hood, VARCHAR is essentially a TEXT with some restrictions. However, TEXT operates far better and cleaner than VARCHAR. Only in places where it is easy and obvious, change VARCHAR (the implicit default) explicitly to TEXT. This should prevent problems where much of the text data is not a safe or sane limit.

Get rid of any @Size limitations that clearly should not exist. Any that is questionable or uncertain is left as-is.

Set nullable to falee where possible that things have defaults being added. There are some fields that have comments noting that they cannot have this set to false when it otherwise should. These will have to be re-reviewed at a later date to confirm/deny if the problem they are working around is resolved or not.

The default for includeAttachment is not a boolean but instead a string of either true or false. This is because of the use of Freemarker on that field. Fixing this to utilize Camunda's/Operaton's own expression handler would require too much out of scope changes to handle here. Explicitly add a comment where needed to document this behavior.

The Workflow is no longer completely missing the initialization of Setup.

Update unit tests accordingly.

For unit tests that happened to be updated, change the arguments stream logic to the more recent practices. The new practices have shown to have less odd problems. We should migrate to the new approach over time or via a separate issue when we have the chance (outside of this issue).

… as appropriate initialization.

Remove all of the `@NotNull` annotations.
We shouldn't have the **JSON** handler be more opinionated.
We can better use **Java** logic to parse and handle the situation.
This better allows for providing defaults.
This reduces some of the problems with **Jackson's** highly opinionated, fickle, and very subject to breaking change designs.

Add `@ColumnDefault` to provide the default where possible.
In any case where there is a `columnDefinition` is use, the `@ColumnDefault` cannot be used.
Instead, directly specify the default within the `columnDefinition`.
Use constant variables where possible, however, many cannot be used because **Java** complains that they are too dynamic to be used.
In those cases, unfortunately, static values must be hard coded into the string itself.
This is less maintainable, but the situation is forced by **Java's** inflexible design in this case.
This can be looked at in the future where we could redefine some global statics or other behavioral change to reduce this problem.
Such changes are largely out of scope for this issue.

The `ScriptTask` is udpated to use a `SCRIPT_FORMAT_DEFAULT` at a package level so that the unit test can better utilize this.
The whole `scriptFormat` vs extension logic should be revisited in the future.
We really shouldn't have these extra steps and fields to swap between.

Many of the text fields should instead be `TEXT` rather than `VARCHAR`.
Under the hood, `VARCHAR` is essentially a `TEXT` with some restrictions.
However, `TEXT` operates far better and cleaner than `VARCHAR`.
Only in places where it is easy and obvious, change `VARCHAR` (the implicit default) explicitly to `TEXT`.
This should prevent problems where much of the text data is not a safe or sane limit.

Get rid of any `@Size` limitations that clearly should not exist.
Any that is questionable or uncertain is left as-is.

Set `nullable` to `falee` where possible that things have defaults being added.
There are some fields that have comments noting that they cannot have this set to `false` when it otherwise should.
These will have to be re-reviewed at a later date to confirm/deny if the problem they are working around is resolved or not.

The default for `includeAttachment` is not a boolean but instead a string of either `true` or `false`.
This is because of the use of **Freemarker** on that field.
Fixing this to utilize **Camunda's/Operaton's** own expression handler would require too much out of scope changes to handle here.
Explicitly add a comment where needed to document this behavior.

The `Workflow` is no longer completely missing the initialization of `Setup`.

Update unit tests accordingly.

For unit tests that happened to be updated, change the arguments stream logic to the more recent practices.
The new practices have shown to have less odd problems.
We should migrate to the new approach over time or via a separate issue when we have the chance (outside of this issue).
@kaladay
kaladay requested a review from a team October 21, 2026 18:21
@kaladay
kaladay changed the base branch from master to sprint18-staging October 21, 2026 18:22
Add class level static constants to make changes easier and more maintainable.
This will allow for improving the unit tests.

Only update the unit tests that needed to be updated to pass the current coverage requirements.
@sonarqubecloud

Copy link
Copy Markdown

@kaladay
kaladay merged commit 3fcdba8 into sprint18-staging Oct 23, 2026
15 checks passed
@kaladay
kaladay deleted the MODWRKFLOW-85 branch October 28, 2026 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants