[NAE-2464] Post release fixes - #464
Open
renczesstefan wants to merge 18 commits into
Open
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
WalkthroughThe PR adds executor-backed asynchronous action execution, defers delegate cleanup across async work, improves Elastic full-text queries and Groovy import discovery, updates MongoDB identifier queries and migration wiring, narrows management endpoint exposure, and updates release version references. ChangesAsynchronous action execution
Migration dependency wiring
Elastic full-text search
Groovy action import discovery
Repository identifier migration
Management and release configuration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ActionDelegate
participant AsyncRunner
participant ActionsExecutor
ActionDelegate->>AsyncRunner: retainForAsyncExecution()
AsyncRunner->>ActionsExecutor: submit wrapped action
ActionsExecutor->>AsyncRunner: execute closure
AsyncRunner->>ActionDelegate: releaseAfterAsyncExecution()
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…elpers to use a dedicated MongoTemplate bean (`migrationMongoTemplate`) with fallback logic in multi-database setups.
…Id` and deprecate `findByNetworkIdAndObjectId`.
…te management and enhanced executor configuration. - Added methods to `ActionDelegate` to manage asynchronous execution lifecycle (`retainForAsyncExecution`, `releaseAfterAsyncExecution`, `clearAfterExecution`). - Enhanced `AsyncRunner` with delegate state tracking and custom `actionsExecutor`. - Added `async_run.xml` test Petri net and corresponding test cases to validate asynchronous action handling.
…te management and enhanced executor configuration. - Added methods to `ActionDelegate` to manage asynchronous execution lifecycle (`retainForAsyncExecution`, `releaseAfterAsyncExecution`, `clearAfterExecution`). - Enhanced `AsyncRunner` with delegate state tracking and custom `actionsExecutor`. - Added `async_run.xml` test Petri net and corresponding test cases to validate asynchronous action handling.
- Update `server-patterns` to replace `/manage/**` with `/manage/health` - Configure `management.endpoint.shutdown.enabled` as `false` across properties - Expand `management.endpoints.web.exposure.include` for additional actuator endpoints
…logic; consolidate MongoTemplate usage to default bean.
# Conflicts: # application-engine/src/test/groovy/com/netgrif/application/engine/TestHelper.groovy # application-engine/src/test/groovy/com/netgrif/application/engine/action/ActionDelegateTest.groovy
… and `TestHelper` for consistency and clarity.
…d` and deprecate `findByNetworkIdAndObjectId`; update tests and enums accordingly.
…ariable naming consistency.
…tringQuery` with `BoolQuery`, introduce `FullTextField` model, and enhance wildcard handling.
…ce static imports with dynamic discovery, introduce `ACTION_IMPORT_PACKAGES`, and optimize class loading.
[NAE-2464] Release 1.0.1 Bugfixes
machacjozef
marked this pull request as ready for review
July 30, 2026 08:21
coderabbitai
Bot
added
breaking change
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@application-engine/src/main/java/com/netgrif/application/engine/configuration/properties/MigrationProperties.java`:
- Around line 24-33: Use MigrationProperties.mongoTemplateBeanName when wiring
the migration helpers instead of hard-coded `@Qualifier`("mongoTemplate")
injection. Update CaseMigrationHelper, PetriNetMigrationHelper, and
TaskMigrationHelper to resolve the configured bean, falling back to the default
mongoTemplate bean when unavailable; update MigrationProperties only as needed
to support this wiring.
In
`@application-engine/src/main/java/com/netgrif/application/engine/configuration/TaskExecutionConfiguration.java`:
- Around line 20-26: Update the actionsExecutor() ThreadPoolTaskExecutor
configuration to set a finite queue capacity, define a maxPoolSize, and
explicitly configure a RejectedExecutionHandler for submissions beyond available
capacity. Preserve the existing core pool size and thread name prefix while
ensuring overload behavior is intentional.
In
`@application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseService.java`:
- Around line 579-585: Update boost parsing in the fulltext field definition
handling to accept the parsed value only when Float.isFinite(boost) and boost >
0. Treat NaN, infinities, zero, and negative values like NumberFormatException,
log the invalid input, and preserve the default 1.0f fallback.
- Around line 559-566: Update the full-text term normalization stream to
preserve backslashes through splitting and trimming, removing the pre-escaping
backslash deletion and the removeDanglingEscapeCharacters mapping. Ensure terms
flow unchanged into escapeWildcardValue so literal backslashes in paths such as
C:\temp are retained.
In
`@application-engine/src/main/java/com/netgrif/application/engine/event/GroovyShellFactory.java`:
- Around line 57-91: Update selectActionImport to emit a warning when multiple
most-specific candidates remain tied, including the conflicting class names in
the diagnostic, before returning Optional.empty(). Preserve the existing
behavior for unique winners and unresolved conflicts, and add focused tests
covering both tie and non-tie specificity resolution.
In `@application-engine/src/test/resources/petriNets/async_run.xml`:
- Around line 16-19: Make the async fixture in
application-engine/src/test/resources/petriNets/async_run.xml lines 16-19
persist a test-observable side effect instead of only printing useCase.stringId.
Update the test in
application-engine/src/test/groovy/com/netgrif/application/engine/action/ActionDelegateTest.groovy
lines 183-197 to await that persisted side effect with a bounded timeout and
assert it occurred, covering submission, execution, and delegate release.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e64b8de1-0138-4bec-ae5b-11a5a2934a26
📒 Files selected for processing (23)
application-engine/src/main/groovy/com/netgrif/application/engine/AsyncRunner.groovyapplication-engine/src/main/groovy/com/netgrif/application/engine/migration/helpers/CaseMigrationHelper.groovyapplication-engine/src/main/groovy/com/netgrif/application/engine/migration/helpers/PetriNetMigrationHelper.groovyapplication-engine/src/main/groovy/com/netgrif/application/engine/migration/helpers/TaskMigrationHelper.groovyapplication-engine/src/main/groovy/com/netgrif/application/engine/petrinet/domain/dataset/logic/action/ActionDelegate.groovyapplication-engine/src/main/java/com/netgrif/application/engine/configuration/MongoClientConfiguration.javaapplication-engine/src/main/java/com/netgrif/application/engine/configuration/TaskExecutionConfiguration.javaapplication-engine/src/main/java/com/netgrif/application/engine/configuration/properties/MigrationProperties.javaapplication-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseService.javaapplication-engine/src/main/java/com/netgrif/application/engine/elastic/service/model/FullTextField.javaapplication-engine/src/main/java/com/netgrif/application/engine/event/GroovyShellFactory.javaapplication-engine/src/main/java/com/netgrif/application/engine/petrinet/domain/roles/ProcessRoleRepository.javaapplication-engine/src/main/java/com/netgrif/application/engine/workflow/domain/repositories/CaseRepository.javaapplication-engine/src/main/java/com/netgrif/application/engine/workflow/service/CaseEventHandler.javaapplication-engine/src/main/resources/application-old-dev.propertiesapplication-engine/src/main/resources/application-old.propertiesapplication-engine/src/main/resources/application.yamlapplication-engine/src/test/groovy/com/netgrif/application/engine/TestHelper.groovyapplication-engine/src/test/groovy/com/netgrif/application/engine/action/ActionDelegateTest.groovyapplication-engine/src/test/groovy/com/netgrif/application/engine/action/AssignRemoveTest.groovyapplication-engine/src/test/resources/application-test.yamlapplication-engine/src/test/resources/petriNets/async_run.xmldocker-compose.yml
Comment on lines
+20
to
+26
| @Bean("actionsExecutor") | ||
| TaskExecutor actionsExecutor() { | ||
| ThreadPoolTaskExecutor executor = new ThreadPoolTaskExecutor(); | ||
| executor.setCorePoolSize(5); | ||
| executor.setThreadNamePrefix("ActionsExecutor-"); | ||
| return executor; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C3 \
'actionsExecutor|setQueueCapacity|setMaxPoolSize|setRejectedExecutionHandler|waitForTasksToCompleteOnShutdown' \
application-engine
fd -a 'pom.xml' . -x grep -nH -E 'spring-boot|spring-context' {}Repository: netgrif/application-engine
Length of output: 9649
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== TaskExecutionConfiguration.java =="
cat -n application-engine/src/main/java/com/netgrif/application/engine/configuration/TaskExecutionConfiguration.java
echo
echo "== AsyncRunner.groovy =="
cat -n application-engine/src/main/groovy/com/netgrif/application/engine/AsyncRunner.groovy
echo
echo "== pom dependency versions around spring context/core =="
python3 - <<'PY'
from pathlib import Path
p=Path('application-engine/pom.xml')
text=p.read_text()
for needle in ['spring-boot.version','spring-boot-dependencies','spring-boot-parent','spring-context','spring-core','spring-boot-starter']:
idx=text.find(needle)
if idx!=-1:
print(text[max(0,idx-250):idx+len(needle)+250])
PY
echo
echo "== All async executor usage =="
rg -n -C2 'AsyncRunner|actionsExecutor|execute\(\(|TaskExecutor' application-engine/src/main/java application-engine/src/main/groovy | head -200Repository: netgrif/application-engine
Length of output: 26959
🌐 Web query:
Spring ThreadPoolTaskExecutor default queue capacity infinity unbounded queue rejection policy
💡 Result:
The Spring ThreadPoolTaskExecutor is designed with specific default behaviors regarding task queuing and rejection. Default Queue Capacity The default queue capacity for a ThreadPoolTaskExecutor is Integer.MAX_VALUE [1][2][3]. In practice, this is treated as an unbounded queue [4][5]. Because the queue is effectively infinite, tasks will continue to be added to the queue when the core pool size is reached, preventing the thread pool from ever scaling beyond the core pool size (unless the queue capacity is set to a finite value) [4][5]. Using an unbounded queue is generally discouraged for production applications, as it can lead to OutOfMemoryErrors if task submission outpaces processing [4]. Default Rejection Policy The default rejection policy is the AbortPolicy [6][7]. When the thread pool and its queue are both full (which only occurs if a finite queue capacity is configured), this policy throws a TaskRejectedException [6][8]. Configurable Options You can change these defaults to control how the executor behaves under load: 1. Queue Capacity: You can use setQueueCapacity(int) to define a specific limit [1][3]. Setting this to a positive value uses a LinkedBlockingQueue, while setting it to any other value (like 0) uses a SynchronousQueue [1][3]. A capacity of 0, for instance, forces the pool to create new threads immediately rather than queuing [1][5]. 2. Rejection Policy: You can configure alternatives to the default AbortPolicy, such as: - CallerRunsPolicy: Forces the thread that submitted the task to execute it, providing a natural throttling mechanism [6][7]. - DiscardPolicy: Silently drops the task [6][8]. - DiscardOldestPolicy: Drops the oldest unhandled task to make room for the new one [6][8].
Citations:
- 1: https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/scheduling/concurrent/ThreadPoolTaskExecutor.html
- 2: https://github.com/spring-projects/spring-framework/blob/main/spring-context/src/main/java/org/springframework/scheduling/concurrent/ThreadPoolTaskExecutor.java
- 3: https://docs.spring.io/spring-framework/docs/7.0.3/javadoc-api/org/springframework/scheduling/concurrent/ThreadPoolTaskExecutor.html
- 4: https://docs.enterprise.spring.io/spring-framework/reference/6.0/integration/scheduling.html
- 5: https://stackoverflow.com/questions/44638056/whats-springs-default-queue-size-with-a-threadpooltaskexecutor
- 6: https://docs.spring.io/spring-framework/reference/integration/scheduling.html
- 7: https://docs.spring.io/spring-framework/reference/6.2/integration/scheduling.html
- 8: https://docs.enterprise.spring.io/spring-framework/reference/integration/scheduling.html
Bound the action queue and define overload behavior.
ThreadPoolTaskExecutor defaults to an unbounded queue with CorePoolSize, so async action submissions can accumulate indefinitely behind slow tasks before backpressure occurs. Configure a finite queue, a maxPoolSize, and an explicit rejection policy if that capacity can be reached.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@application-engine/src/main/java/com/netgrif/application/engine/configuration/TaskExecutionConfiguration.java`
around lines 20 - 26, Update the actionsExecutor() ThreadPoolTaskExecutor
configuration to set a finite queue capacity, define a maxPoolSize, and
explicitly configure a RejectedExecutionHandler for submissions beyond available
capacity. Preserve the existing core pool size and thread name prefix while
ensuring overload behavior is intentional.
Comment on lines
+559
to
+566
| return Arrays.stream(fullText | ||
| .replaceAll("\\\\", "") | ||
| .replaceAll("\\s+", " ") | ||
| .trim() | ||
| .split("\\s+")) | ||
| .map(String::trim) | ||
| .map(this::removeDanglingEscapeCharacters) | ||
| .filter(term -> !term.isBlank()) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file exists and line count =="
wc -l application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseService.java
echo "== relevant slices =="
sed -n '520,585p' application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseService.java
echo "----"
sed -n '568,578p' application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseService.java
echo "== usages of removeDanglingEscapeCharacters and escapeWildcardValue =="
rg -n "removeDanglingEscapeCharacters|escapeWildcardValue|build.*Search|Query|fullText" application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseService.javaRepository: netgrif/application-engine
Length of output: 11663
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== escapeWildcardValue implementation =="
sed -n '590,610p' application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseService.java
echo "== deterministic Java-like normalization behavior for representative inputs =="
python3 - <<'PY'
import re
def normalize_fulltext_search(full_text):
return [
term for term in [
re.sub(r"[\\\s]+", " ", full_text) # original .replaceAll("\\\\", "").replaceAll("\\s+", " ")
.strip()
.split()
] if term.strip()
]
def remove_dangling_escape_characters(term):
return re.sub(r"\\+$", "", term)
def escape_wildcard_value(value):
return value.replace("[", "\\[").replace(":", "\\:")
for raw in ["C:\\temp", "C:temp", "file:c:\\temp"]:
terms = normalize_fulltext_search(raw)
escaped = "*" + escape_wildcard_value(remove_dangling_escape_characters(terms[0] if terms else "" if True else terms[0])) + "*"
print(f"{raw!r} -> {terms!r}", "-> after removeDangling:", remove_dangling_escape_characters(terms[0]) if terms else "", "-> wildcard:", escaped)
print("\nIf backslashes are kept until wildcard escape:")
for raw in ["C:\\temp", "file:c:\\temp"]:
parts = re.sub(r"\\s+", " ", raw).strip().split()
escaped = "*" + escape_wildcard_value(parts[0]) + "*"
print(f"{raw!r} -> {parts!r} -> wildcard:", escaped)
PYRepository: netgrif/application-engine
Length of output: 706
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== behavioral probe for normalization flow vs keep-backslash flow =="
python3 - <<'PY'
import re
def normalize_flow(full_text):
# Model: .replaceAll("\\\\", "") .replaceAll("\\s+", " ") .trim().split("\\s+")
term = re.sub(r"\\", "", full_text)
term = re.sub(r"\s+", " ", term)
term = term.strip()
terms = term.split()
return [t for t in terms if t]
def kept_backslash_flow(full_text):
# Model: .replaceAll("\\s+", " ") .trim().split("\\s+") without removing backslashes
term = re.sub(r"\s+", " ", full_text)
term = term.strip()
terms = term.split()
return [t for t in terms if t]
def escape_wildcard_value(value):
return value.replace("\\", "\\\\").replace("*", "\\*").replace("?", "\\?")
raws = ["C:\\temp", "file:c:\\temp", "C:temp"]
print("normalized -> escaped wildcard:")
for raw in raws:
for term in normalize_flow(raw) or [""]:
print(repr(raw), "=>", repr(term), "=>", repr("*" + escape_wildcard_value(term) + "*"))
print("\nkept backslashes -> escaped wildcard:")
for raw in raws:
for term in kept_backslash_flow(raw) or [""]:
print(repr(raw), "=>", repr(term), "=>", repr("*" + escape_wildcard_value(term) + "*"))
PYRepository: netgrif/application-engine
Length of output: 564
Preserve literal backslashes in full-text search terms.
C:\\temp is normalized to C:temp before escapeWildcardValue runs, so wildcard searches for Windows paths lose the backslash. Keep backslashes until wildcard escaping, then no longer need removeDanglingEscapeCharacters.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseService.java`
around lines 559 - 566, Update the full-text term normalization stream to
preserve backslashes through splitting and trimming, removing the pre-escaping
backslash deletion and the removeDanglingEscapeCharacters mapping. Ensure terms
flow unchanged into escapeWildcardValue so literal backslashes in paths such as
C:\temp are retained.
Comment on lines
+579
to
+585
| if (parts.length == 2 && !parts[1].isBlank()) { | ||
| try { | ||
| boost = Float.parseFloat(parts[1].trim()); | ||
| } catch (NumberFormatException e) { | ||
| log.warn("Invalid boost [{}] in fulltext field definition [{}]. Using default boost 1.0.", parts[1], fieldDefinition); | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant service method and nearby boost usage, without running repository code.
file="application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseService.java"
if [ -f "$file" ]; then
wc -l "$file"
sed -n '540,610p' "$file"
else
fd -a 'ElasticCaseService.java' .
fi
# Locate nearby Elasticsearch search source construction involving boosts and fields.
rg -n "boost|Float\.parseFloat|SearchSourceBuilder|fields" application-engine/src/main/java/com/netgrif/application/engine/elastic -S || true
# Behavioral probe: Java parser accepts non-finite/non-positive boosts and ElasticSearch source values are positive-only?
python3 - <<'PY'
values = ['inf', '-inf', 'nan', '-1', '0', '1', '1.5']
for v in values:
f = float(v)
print(f"{v!r:8} -> parsed={f}, finite={f==f and abs(f)!=float('inf')}, positive={f>0}")
PYRepository: netgrif/application-engine
Length of output: 6130
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the call site where parsed boosts are attached to Elasticsearch queries.
sed -n '420,450p' application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseService.java
# Check Java project version/constraints to ground Java 21 parser semantics.
find . -maxdepth 3 \( -name 'pom.xml' -o -name 'build.gradle' -o -name 'gradle.properties' \) -print -exec sh -c 'echo "### $1"; sed -n "/<java.version\\|<java>/,/<\\/java>/p; /targetCompatibility\\|sourceCompatibility/p" "$1" 2>/dev/null || true' sh {} \;Repository: netgrif/application-engine
Length of output: 11087
Reject non-finite and non-positive boosts.
Float.parseFloat can parse NaN, Infinity, and negative/zero values here, and the parsed boost is applied directly to Elasticsearch wildcard queries. Keep the parsed value only when Float.isFinite(boost) && boost > 0, logging otherwise so the fallback to 1.0f is used consistently for invalid input.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseService.java`
around lines 579 - 585, Update boost parsing in the fulltext field definition
handling to accept the parsed value only when Float.isFinite(boost) and boost >
0. Treat NaN, infinities, zero, and negative values like NumberFormatException,
log the invalid input, and preserve the default 1.0f fallback.
Comment on lines
+57
to
+91
| private Set<String> findAllActionImportClasses() { | ||
| Set<Class<?>> classes = ACTION_IMPORT_PACKAGES.stream() | ||
| .flatMap(packageName -> findAllClassesUsingClassLoader(packageName).stream()) | ||
| .map(this::loadClass) | ||
| .collect(Collectors.toCollection(HashSet::new)); | ||
|
|
||
| return classes.stream() | ||
| .collect(Collectors.groupingBy(Class::getSimpleName)) | ||
| .values().stream() | ||
| .map(this::selectActionImport) | ||
| .flatMap(Optional::stream) | ||
| .map(Class::getName) | ||
| .collect(Collectors.toSet()); | ||
| } | ||
|
|
||
| private Optional<Class<?>> selectActionImport(List<Class<?>> candidates) { | ||
| int highestSpecificity = candidates.stream() | ||
| .mapToInt(candidate -> importSpecificity(candidate, candidates)) | ||
| .max() | ||
| .orElseThrow(); | ||
|
|
||
| List<Class<?>> mostSpecificCandidates = candidates.stream() | ||
| .filter(candidate -> importSpecificity(candidate, candidates) == highestSpecificity) | ||
| .toList(); | ||
|
|
||
| return mostSpecificCandidates.size() == 1 | ||
| ? Optional.of(mostSpecificCandidates.getFirst()) | ||
| : Optional.empty(); | ||
| } | ||
|
|
||
| private int importSpecificity(Class<?> candidate, List<Class<?>> candidates) { | ||
| return (int) candidates.stream() | ||
| .filter(other -> other != candidate && other.isAssignableFrom(candidate)) | ||
| .count(); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
Ambiguous class-name conflicts are silently dropped with no diagnostic signal.
When two unrelated classes across the scanned packages share a simple name (selectActionImport returns Optional.empty() on a specificity tie), both are silently excluded from the import customizer. Since the scan now spans three broad packages instead of the previous single package, name collisions are more likely than before, and a script referencing the dropped short name will fail with an opaque "unable to resolve class" error with no trace back to this resolution step.
Consider logging a warning identifying the colliding candidates when a tie occurs, so operators can diagnose missing imports without reverse-engineering this algorithm.
🪵 Suggested logging addition
private Optional<Class<?>> selectActionImport(List<Class<?>> candidates) {
int highestSpecificity = candidates.stream()
.mapToInt(candidate -> importSpecificity(candidate, candidates))
.max()
.orElseThrow();
List<Class<?>> mostSpecificCandidates = candidates.stream()
.filter(candidate -> importSpecificity(candidate, candidates) == highestSpecificity)
.toList();
- return mostSpecificCandidates.size() == 1
- ? Optional.of(mostSpecificCandidates.getFirst())
- : Optional.empty();
+ if (mostSpecificCandidates.size() == 1) {
+ return Optional.of(mostSpecificCandidates.getFirst());
+ }
+ log.warn("Ambiguous action-import simple name '{}' resolved among candidates {}; skipping import.",
+ candidates.getFirst().getSimpleName(), mostSpecificCandidates);
+ return Optional.empty();
}Separately, this new specificity-based conflict resolution has no dedicated unit test in the provided context (existing GroovyShellFactoryTest only exercises broader role/field action flows). A focused test for selectActionImport/importSpecificity covering the tie/no-tie cases would guard this logic against regressions.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| private Set<String> findAllActionImportClasses() { | |
| Set<Class<?>> classes = ACTION_IMPORT_PACKAGES.stream() | |
| .flatMap(packageName -> findAllClassesUsingClassLoader(packageName).stream()) | |
| .map(this::loadClass) | |
| .collect(Collectors.toCollection(HashSet::new)); | |
| return classes.stream() | |
| .collect(Collectors.groupingBy(Class::getSimpleName)) | |
| .values().stream() | |
| .map(this::selectActionImport) | |
| .flatMap(Optional::stream) | |
| .map(Class::getName) | |
| .collect(Collectors.toSet()); | |
| } | |
| private Optional<Class<?>> selectActionImport(List<Class<?>> candidates) { | |
| int highestSpecificity = candidates.stream() | |
| .mapToInt(candidate -> importSpecificity(candidate, candidates)) | |
| .max() | |
| .orElseThrow(); | |
| List<Class<?>> mostSpecificCandidates = candidates.stream() | |
| .filter(candidate -> importSpecificity(candidate, candidates) == highestSpecificity) | |
| .toList(); | |
| return mostSpecificCandidates.size() == 1 | |
| ? Optional.of(mostSpecificCandidates.getFirst()) | |
| : Optional.empty(); | |
| } | |
| private int importSpecificity(Class<?> candidate, List<Class<?>> candidates) { | |
| return (int) candidates.stream() | |
| .filter(other -> other != candidate && other.isAssignableFrom(candidate)) | |
| .count(); | |
| } | |
| private Set<String> findAllActionImportClasses() { | |
| Set<Class<?>> classes = ACTION_IMPORT_PACKAGES.stream() | |
| .flatMap(packageName -> findAllClassesUsingClassLoader(packageName).stream()) | |
| .map(this::loadClass) | |
| .collect(Collectors.toCollection(HashSet::new)); | |
| return classes.stream() | |
| .collect(Collectors.groupingBy(Class::getSimpleName)) | |
| .values().stream() | |
| .map(this::selectActionImport) | |
| .flatMap(Optional::stream) | |
| .map(Class::getName) | |
| .collect(Collectors.toSet()); | |
| } | |
| private Optional<Class<?>> selectActionImport(List<Class<?>> candidates) { | |
| int highestSpecificity = candidates.stream() | |
| .mapToInt(candidate -> importSpecificity(candidate, candidates)) | |
| .max() | |
| .orElseThrow(); | |
| List<Class<?>> mostSpecificCandidates = candidates.stream() | |
| .filter(candidate -> importSpecificity(candidate, candidates) == highestSpecificity) | |
| .toList(); | |
| if (mostSpecificCandidates.size() == 1) { | |
| return Optional.of(mostSpecificCandidates.getFirst()); | |
| } | |
| log.warn("Ambiguous action-import simple name '{}' resolved among candidates {}; skipping import.", | |
| candidates.getFirst().getSimpleName(), mostSpecificCandidates); | |
| return Optional.empty(); | |
| } | |
| private int importSpecificity(Class<?> candidate, List<Class<?>> candidates) { | |
| return (int) candidates.stream() | |
| .filter(other -> other != candidate && other.isAssignableFrom(candidate)) | |
| .count(); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@application-engine/src/main/java/com/netgrif/application/engine/event/GroovyShellFactory.java`
around lines 57 - 91, Update selectActionImport to emit a warning when multiple
most-specific candidates remain tied, including the conflicting class names in
the diagnostic, before returning Optional.empty(). Preserve the existing
behavior for unique winners and unresolved conflicts, and add focused tests
covering both tie and non-tie specificity resolution.
Comment on lines
+16
to
+19
| async.run { | ||
| sleep(2000) | ||
| println useCase.stringId | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the async path observable and await it. The fixture only sleeps and prints, while the test asserts only that import and case creation succeeded; broken submission, execution, or delegate release still passes.
application-engine/src/test/resources/petriNets/async_run.xml#L16-L19: replaceprintlnwith a test-observable persisted side effect.application-engine/src/test/groovy/com/netgrif/application/engine/action/ActionDelegateTest.groovy#L183-L197: await that side effect with a bounded timeout and assert it occurred.
📍 Affects 2 files
application-engine/src/test/resources/petriNets/async_run.xml#L16-L19(this comment)application-engine/src/test/groovy/com/netgrif/application/engine/action/ActionDelegateTest.groovy#L183-L197
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@application-engine/src/test/resources/petriNets/async_run.xml` around lines
16 - 19, Make the async fixture in
application-engine/src/test/resources/petriNets/async_run.xml lines 16-19
persist a test-observable side effect instead of only printing useCase.stringId.
Update the test in
application-engine/src/test/groovy/com/netgrif/application/engine/action/ActionDelegateTest.groovy
lines 183-197 to await that persisted side effect with a bounded timeout and
assert it occurred, covering submission, execution, and delegate release.
There was a problem hiding this comment.
@machacjozef, acknowledged — I’ll skip review feedback under /src/test/*. For a persistent repository-wide exclusion, add that path to CodeRabbit’s path_filters configuration.
You are interacting with an AI system.
…ce static imports with dynamic discovery, introduce `ACTION_IMPORT_PACKAGES`, and optimize class loading.
coderabbitai
Bot
added
Medium
and removed
breaking change
- Update version to 7.0.2 in all affected `pom.xml` files - Modify `Dockerfile` and `Dockerfile.multi-stage` to use version 7.0.2 - Adjust `docker-compose.yml` to reflect the new image version
coderabbitai
Bot
added
improvement
Retoocs
requested changes
Jul 31, 2026
|
Retoocs
approved these changes
Jul 31, 2026
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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
Post release fixes after 7.0.1
Implements NAE-2464
Dependencies
No new dependencies were introduced
Third party dependencies
No new dependencies were introduced
Blocking Pull requests
There are no dependencies on other PR
How Has Been This Tested?
This was tested manually and with unit tests.
Test Configuration
Checklist:
Summary by CodeRabbit