[NAE-2125] Remove URI service usage from admin and menu items #318
Conversation
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
This commit removes the entire UriService, its related interfaces, controllers, configuration, tests, and associated functionality. The changes streamline the codebase by eliminating unused URI Node management logic and legacy features.
Introduce a protected evaluateRules method that publishes events using the publisher. This allows for better modular handling of event-driven logic within PetriNetService.
WalkthroughThis change removes the entire URI node infrastructure from the application engine. All code, interfaces, services, controllers, and tests related to Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ActionDelegate
participant DataSet
Client->>ActionDelegate: createMenuItem(body)
ActionDelegate->>ActionDelegate: Construct path string using PATH_SEPARATOR
ActionDelegate->>DataSet: Store path as string in dataset
Note over ActionDelegate,DataSet: No UriNode or IUriService involved
Suggested labels
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (4)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
🔭 Outside diff range comments (2)
application-engine/src/test/groovy/com/netgrif/application/engine/action/MenuItemApiTest.groovy (2)
34-38: Whole test class is@Disabled– risk of silently rotting codeWith the removal of
UriNodethe test was disabled instead of being adapted.
Leaving a key integration test muted will hide regressions. Either:
- Update the test to the new
pathsemantics and re-enable it, or- Delete the file entirely if the scenarios are covered elsewhere.
Retaining a disabled test adds maintenance cost without benefit.
114-121: Field name still uses legacyuriThe data-set map key is
"uri", but the codebase migrated to"path".
Keeping the old key undermines the refactor and will break ActionDelegate once the test
is re-enabled.- "uri": "/netgrif/test_new", + "path": "/netgrif/test_new",
🧹 Nitpick comments (8)
application-engine/src/test/groovy/com/netgrif/application/engine/action/MenuItemApiTest.groovy (2)
94-99: Hard-coded path separator – use the newly introduced constant
"/netgrif/test"is built with a magic"/". Prefer the canonical separator:- "/netgrif/test" + "${MenuItemConstants.PATH_SEPARATOR.attributeId}netgrif${MenuItemConstants.PATH_SEPARATOR.attributeId}test"This ensures future changes to the separator propagate automatically.
79-88: RepeatedThread.sleep– replace with polling
Thread.sleepintroduces flaky tests and inflates the execution time. Switch to Awaitility or a simple poll loop with timeout.Awaitility.await() .atMost(Duration.ofSeconds(5)) .until(() -> condition());nae-object-library/src/main/java/com/netgrif/application/engine/objects/workflow/domain/menu/MenuItemBody.java (1)
79-108: Constructor explosion – prefer builder over 6-arg ctorsSix overloaded constructors make the API error-prone. Lombok’s
@Builder(or a manual builder) would convey intent better and allow optional parameters:@Builder public class MenuItemBody { ... }This will also keep binary compatibility when further attributes are added.
application-engine/src/main/resources/petriNets/engine-processes/preference_item.xml (2)
158-168: Avoid silent use of hard-coded “/”; reuse the central separator constant.The surrounding refactor introduced
MenuItemConstants.PATH_SEPARATOR(value “/”).
For consistency and future-proofing, buildnewUriwith that constant instead of the literal slash.-String newUri = moveDestUri.value.join("/") +String newUri = moveDestUri.value.join(MenuItemConstants.PATH_SEPARATOR)
251-253: Micro-optimisation: avoid repeatedlastIndexOfon immutable string.
nodePath.valueis evaluated twice. Cache it to a local variable for clarity and a tiny perf win.-def idx = nodePath.value.lastIndexOf("/") -return nodePath.value.substring(idx + 1) +def path = nodePath.value +def idx = path.lastIndexOf(MenuItemConstants.PATH_SEPARATOR) +return path.substring(idx + 1)application-engine/src/main/java/com/netgrif/application/engine/petrinet/service/PetriNetService.java (1)
275-278: Minor: unnecessary list copy.
repository.findAllById(ids)already returns a mutableList; wrapping it in a newArrayListis redundant.-return new ArrayList<>(repository.findAllById(ids)); +return repository.findAllById(ids);application-engine/src/main/groovy/com/netgrif/application/engine/petrinet/domain/dataset/logic/action/ActionDelegate.groovy (2)
2164-2171:nameFromPathreturns the slash-prefixed segment
substring(lastIndexOf('/'))keeps the leading slash (e.g."/folder").
Downstream code (option lists, display names) probably expects just"folder".
Consider+1to drop the separator to avoid awkward labels.- return path.substring(path.lastIndexOf(MenuItemConstants.PATH_SEPARATOR.attributeId)) + return path.substring(path.lastIndexOf(MenuItemConstants.PATH_SEPARATOR.attributeId) + 1)
2794-2809: Edge cases insplitUriPath
path.split("/")returns["", "folder"]for"/folder", so replacing index0works, but multiple consecutive slashes or trailing slash ("/folder/") yield empty tokens you don’t normalise.- Throwing
IllegalArgumentExceptionfor any non-root path that splits to length 0 is unreachable; consider removing.Recommend using
Paths.get(...).normalize()or manual filtering of empty elements for robustness.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (29)
application-engine/src/main/groovy/com/netgrif/application/engine/petrinet/domain/dataset/logic/action/ActionDelegate.groovy(11 hunks)application-engine/src/main/groovy/com/netgrif/application/engine/startup/ImportHelper.groovy(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/configuration/UriServiceConfiguration.java(0 hunks)application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseService.java(0 hunks)application-engine/src/main/java/com/netgrif/application/engine/elastic/service/interfaces/IElasticCaseService.java(0 hunks)application-engine/src/main/java/com/netgrif/application/engine/petrinet/service/PetriNetService.java(3 hunks)application-engine/src/main/java/com/netgrif/application/engine/petrinet/service/UriService.java(0 hunks)application-engine/src/main/java/com/netgrif/application/engine/petrinet/service/interfaces/IPetriNetService.java(0 hunks)application-engine/src/main/java/com/netgrif/application/engine/petrinet/service/interfaces/IUriService.java(0 hunks)application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/PetriNetController.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/UriController.java(0 hunks)application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/responsebodies/UriNodeResource.java(0 hunks)application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/responsebodies/UriNodeResources.java(0 hunks)application-engine/src/main/java/com/netgrif/application/engine/startup/runner/UriRunner.java(0 hunks)application-engine/src/main/java/com/netgrif/application/engine/workflow/domain/repositories/CaseRepository.java(0 hunks)application-engine/src/main/java/com/netgrif/application/engine/workflow/service/WorkflowService.java(0 hunks)application-engine/src/main/java/com/netgrif/application/engine/workflow/service/interfaces/IWorkflowService.java(0 hunks)application-engine/src/main/resources/petriNets/engine-processes/preference_item.xml(3 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/TestHelper.groovy(0 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/action/MenuItemApiTest.groovy(2 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/service/PetriNetServiceTest.groovy(0 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/service/UriServiceTest.groovy(0 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/workflow/WorkflowServiceTest.groovy(0 hunks)application-engine/src/test/java/com/netgrif/application/engine/workflow/service/TaskServiceTest.java(0 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/ElasticCase.java(0 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/workflow/domain/Case.java(0 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/workflow/domain/menu/MenuItemBody.java(2 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/workflow/domain/menu/MenuItemConstants.java(1 hunks)nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/ElasticCase.java(0 hunks)
💤 Files with no reviewable changes (21)
- application-engine/src/main/java/com/netgrif/application/engine/workflow/domain/repositories/CaseRepository.java
- application-engine/src/main/java/com/netgrif/application/engine/workflow/service/interfaces/IWorkflowService.java
- application-engine/src/test/groovy/com/netgrif/application/engine/TestHelper.groovy
- nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/ElasticCase.java
- application-engine/src/test/java/com/netgrif/application/engine/workflow/service/TaskServiceTest.java
- application-engine/src/main/java/com/netgrif/application/engine/elastic/service/interfaces/IElasticCaseService.java
- application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/responsebodies/UriNodeResources.java
- application-engine/src/test/groovy/com/netgrif/application/engine/workflow/WorkflowServiceTest.groovy
- application-engine/src/main/java/com/netgrif/application/engine/configuration/UriServiceConfiguration.java
- application-engine/src/main/java/com/netgrif/application/engine/workflow/service/WorkflowService.java
- application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseService.java
- nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/ElasticCase.java
- application-engine/src/main/java/com/netgrif/application/engine/petrinet/service/interfaces/IPetriNetService.java
- nae-object-library/src/main/java/com/netgrif/application/engine/objects/workflow/domain/Case.java
- application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/service/PetriNetServiceTest.groovy
- application-engine/src/main/java/com/netgrif/application/engine/startup/runner/UriRunner.java
- application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/service/UriServiceTest.groovy
- application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/responsebodies/UriNodeResource.java
- application-engine/src/main/java/com/netgrif/application/engine/petrinet/service/interfaces/IUriService.java
- application-engine/src/main/java/com/netgrif/application/engine/petrinet/service/UriService.java
- application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/UriController.java
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: task-list-completed
- GitHub Check: Build
- GitHub Check: task-list-completed
🔇 Additional comments (3)
application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/PetriNetController.java (1)
111-113: Resource leak – input stream should be closed
multipartFile.getInputStream()is never closed. Wrap with try-with-resources, or pass theMultipartFiledirectly to the service to let it take ownership.application-engine/src/main/resources/petriNets/engine-processes/preference_item.xml (1)
190-196: ```shell
#!/usr/bin/env bash
set -eecho "Searching for lingering
uriServicereferences in process definitions..."
rg "uriService" -n application-engine/src/main/resources</details> <details> <summary>application-engine/src/main/java/com/netgrif/application/engine/petrinet/service/PetriNetService.java (1)</summary> `283-288`: **Java 21 `List#getFirst()` is OK – verify toolchain.** `getFirst()` requires JDK 21. The build matrix states Java 21, but CI or downstream consumers must also compile with 21+. Double-check before merge. </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
...main/java/com/netgrif/application/engine/objects/workflow/domain/menu/MenuItemConstants.java
Show resolved
Hide resolved
Comment on lines
109
to
113
| Authentication auth, Locale locale) throws MissingPetriNetMetaDataException, MissingIconKeyException { | ||
| try { | ||
| String decodedUriNodeId = new String(Base64.decodeBase64(uriNodeId)); | ||
| VersionType release = releaseType == null ? VersionType.MAJOR : VersionType.valueOf(releaseType.trim().toUpperCase()); | ||
| ImportPetriNetEventOutcome importPetriNetOutcome = service.importPetriNet(multipartFile.getInputStream(), release, (LoggedUser) auth.getPrincipal(), decodedUriNodeId); | ||
| ImportPetriNetEventOutcome importPetriNetOutcome = service.importPetriNet(multipartFile.getInputStream(), release, (LoggedUser) auth.getPrincipal()); | ||
| return EventOutcomeWithMessageResource.successMessage("Petri net " + multipartFile.getOriginalFilename() + " imported successfully", |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Possible IllegalArgumentException on invalid meta value
VersionType.valueOf(releaseType.trim().toUpperCase()) will throw IllegalArgumentException
for unexpected input, resulting in a 500 error. Convert it to a controlled 400 response:
- VersionType release = releaseType == null ? VersionType.MAJOR : VersionType.valueOf(releaseType.trim().toUpperCase());
+ VersionType release;
+ try {
+ release = releaseType == null
+ ? VersionType.MAJOR
+ : VersionType.valueOf(releaseType.trim().toUpperCase());
+ } catch (IllegalArgumentException ex) {
+ return EventOutcomeWithMessageResource.errorMessage("Unknown meta version type: " + releaseType);
+ }📝 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
| Authentication auth, Locale locale) throws MissingPetriNetMetaDataException, MissingIconKeyException { | |
| try { | |
| String decodedUriNodeId = new String(Base64.decodeBase64(uriNodeId)); | |
| VersionType release = releaseType == null ? VersionType.MAJOR : VersionType.valueOf(releaseType.trim().toUpperCase()); | |
| ImportPetriNetEventOutcome importPetriNetOutcome = service.importPetriNet(multipartFile.getInputStream(), release, (LoggedUser) auth.getPrincipal(), decodedUriNodeId); | |
| ImportPetriNetEventOutcome importPetriNetOutcome = service.importPetriNet(multipartFile.getInputStream(), release, (LoggedUser) auth.getPrincipal()); | |
| return EventOutcomeWithMessageResource.successMessage("Petri net " + multipartFile.getOriginalFilename() + " imported successfully", | |
| Authentication auth, Locale locale) throws MissingPetriNetMetaDataException, MissingIconKeyException { | |
| try { | |
| VersionType release; | |
| try { | |
| release = releaseType == null | |
| ? VersionType.MAJOR | |
| : VersionType.valueOf(releaseType.trim().toUpperCase()); | |
| } catch (IllegalArgumentException ex) { | |
| return EventOutcomeWithMessageResource.errorMessage("Unknown meta version type: " + releaseType); | |
| } | |
| ImportPetriNetEventOutcome importPetriNetOutcome = service.importPetriNet( | |
| multipartFile.getInputStream(), | |
| release, | |
| (LoggedUser) auth.getPrincipal() | |
| ); | |
| return EventOutcomeWithMessageResource.successMessage( | |
| "Petri net " + multipartFile.getOriginalFilename() + " imported successfully", |
🤖 Prompt for AI Agents
In
application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/PetriNetController.java
around lines 109 to 113, the call to VersionType.valueOf with releaseType can
throw an IllegalArgumentException if the input is invalid, causing a 500 error.
To fix this, catch the IllegalArgumentException when parsing releaseType, and
instead return a controlled 400 Bad Request response indicating the invalid
input. This prevents server errors and provides clearer feedback to the client.
Member
Author
There was a problem hiding this comment.
IllegalArgumentException will be added to enclosing try catch block.
.../src/main/java/com/netgrif/application/engine/objects/workflow/domain/menu/MenuItemBody.java
Show resolved
Hide resolved
application-engine/src/main/groovy/com/netgrif/application/engine/startup/ImportHelper.groovy
Outdated
Show resolved
Hide resolved
...vy/com/netgrif/application/engine/petrinet/domain/dataset/logic/action/ActionDelegate.groovy
Show resolved
Hide resolved
...vy/com/netgrif/application/engine/petrinet/domain/dataset/logic/action/ActionDelegate.groovy
Show resolved
Hide resolved
Corrected a misspelled variable `pathNUame` to `pathName` in the `getOrCreateFolderItem` method. This fixes an issue where folder creation logic could break due to the typo.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
application-engine/src/main/groovy/com/netgrif/application/engine/petrinet/domain/dataset/logic/action/ActionDelegate.groovy (1)
2117-2122:createNodePathstill produces “//” for already-sanitised pathsThis was reported in a previous review and the risky branch is unchanged.
Ifpathis"/foo/"(ends with separator but is not the root) theelse
appends another/, yielding"/foo//bar".(See past review around lines 2117-2123.)
Please normalise the incoming path (or trim the trailing separator) before concatenation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
application-engine/src/main/groovy/com/netgrif/application/engine/petrinet/domain/dataset/logic/action/ActionDelegate.groovy(11 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: Build
- GitHub Check: task-list-completed
Comment on lines
2089
to
2103
| Case parentItemCase = getOrCreateFolderItem(body.path) | ||
| I18nString newName = body.menuName ?: (body.filter?.dataSet[FILTER_FIELD_I18N_FILTER_NAME].value as I18nString) | ||
|
|
||
| Case menuItemCase = createCase(FilterRunner.PREFERRED_ITEM_NET_IDENTIFIER, newName?.defaultValue) | ||
| menuItemCase.setUriNodeId(uriService.findByUri(body.uri).stringId) | ||
| menuItemCase.dataSet[MenuItemConstants.PREFERENCE_ITEM_FIELD_ALLOWED_ROLES.attributeId].options = body.allowedRoles | ||
| menuItemCase.dataSet[MenuItemConstants.PREFERENCE_ITEM_FIELD_BANNED_ROLES.attributeId].options = body.bannedRoles | ||
| if (parentItemCase != null) { | ||
| parentItemCase = appendChildCaseIdAndSave(parentItemCase, menuItemCase.stringId) | ||
| } | ||
| menuItemCase = workflowService.save(menuItemCase) | ||
| Task newItemTask = taskService.findOne(menuItemCase.tasks.find { it.transition == MenuItemConstants.PREFERENCE_ITEM_FIELD_INIT_TRANS_ID.attributeId }.task) | ||
| String nodePath = createNodePath(body.uri, sanitizedIdentifier) | ||
| uriService.getOrCreate(nodePath, UriContentType.CASE) | ||
| String nodePath = createNodePath(body.path, sanitizedIdentifier) | ||
|
|
||
| newItemTask = assignTask(newItemTask) | ||
| setData(newItemTask, body.toDataSet(parentItemCase.stringId, nodePath)) |
There was a problem hiding this comment.
Possible NullPointerException when body.path == "/"
getOrCreateFolderItem("/") legitimately returns null (root has no folder case).
parentItemCase is dereferenced unconditionally a few lines later:
setData(newItemTask, body.toDataSet(parentItemCase.stringId, nodePath))If the path is root, the application crashes before the item is stored.
- setData(newItemTask, body.toDataSet(parentItemCase.stringId, nodePath))
+ // root has no parent – pass null instead of exploding
+ setData(newItemTask, body.toDataSet(parentItemCase?.stringId, nodePath))🤖 Prompt for AI Agents
In
application-engine/src/main/groovy/com/netgrif/application/engine/petrinet/domain/dataset/logic/action/ActionDelegate.groovy
around lines 2089 to 2103, the variable parentItemCase can be null when
body.path is "/", causing a NullPointerException when accessing
parentItemCase.stringId. To fix this, add a null check before dereferencing
parentItemCase.stringId in the call to body.toDataSet, and handle the root path
case appropriately by passing a safe value or adjusting the logic to avoid using
parentItemCase.stringId when parentItemCase is null.
Comment on lines
2163
to
2171
| protected String nameFromPath(String path) { | ||
| if (path == null || path == MenuItemConstants.PATH_SEPARATOR.attributeId || path.length() == 0) { | ||
| return "" | ||
| } | ||
| if (path.lastIndexOf(MenuItemConstants.PATH_SEPARATOR.attributeId) == 0) { | ||
| return path.replace(MenuItemConstants.PATH_SEPARATOR.attributeId, "") | ||
| } | ||
| return path.substring(path.lastIndexOf(MenuItemConstants.PATH_SEPARATOR.attributeId)) | ||
| } |
There was a problem hiding this comment.
nameFromPath returns the separator ("/b" instead of "b").
substring(lastIndex) keeps the leading slash.
Down-stream calls treat the value as folder name – the extra / breaks look-ups
and produces malformed titles.
- return path.substring(path.lastIndexOf(MenuItemConstants.PATH_SEPARATOR.attributeId))
+ return path.substring(path.lastIndexOf(MenuItemConstants.PATH_SEPARATOR.attributeId) + 1)📝 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
| protected String nameFromPath(String path) { | |
| if (path == null || path == MenuItemConstants.PATH_SEPARATOR.attributeId || path.length() == 0) { | |
| return "" | |
| } | |
| if (path.lastIndexOf(MenuItemConstants.PATH_SEPARATOR.attributeId) == 0) { | |
| return path.replace(MenuItemConstants.PATH_SEPARATOR.attributeId, "") | |
| } | |
| return path.substring(path.lastIndexOf(MenuItemConstants.PATH_SEPARATOR.attributeId)) | |
| } | |
| protected String nameFromPath(String path) { | |
| if (path == null || path == MenuItemConstants.PATH_SEPARATOR.attributeId || path.length() == 0) { | |
| return "" | |
| } | |
| if (path.lastIndexOf(MenuItemConstants.PATH_SEPARATOR.attributeId) == 0) { | |
| return path.replace(MenuItemConstants.PATH_SEPARATOR.attributeId, "") | |
| } | |
| return path.substring(path.lastIndexOf(MenuItemConstants.PATH_SEPARATOR.attributeId) + 1) | |
| } |
🤖 Prompt for AI Agents
In
application-engine/src/main/groovy/com/netgrif/application/engine/petrinet/domain/dataset/logic/action/ActionDelegate.groovy
around lines 2163 to 2171, the method nameFromPath returns a string starting
with the path separator because substring is called with lastIndexOf only, which
includes the separator. To fix this, adjust the substring call to start from
lastIndexOf plus one, so the returned name excludes the leading separator and
downstream folder name look-ups work correctly.
Comment on lines
2822
to
2828
| Case caseByPath = findCaseElastic("processIdentifier:$FilterRunner.PREFERRED_ITEM_NET_IDENTIFIER AND dataSet.nodePath.textValue.keyword:\"$path\"") | ||
| Set<String> childrenIds = caseByPath.dataSet[MenuItemConstants.PREFERENCE_ITEM_FIELD_CHILD_ITEM_IDS.attributeId].value as Set | ||
| if (!childrenIds.isEmpty()) { | ||
| for (String id : childrenIds) { | ||
| UriNode childNode = uriService.findById(id) | ||
| options.put(childNode.name, new I18nString(childNode.name)) | ||
| Case childFolderCase = workflowService.findOne(id) | ||
| options.put(childFolderCase.dataSet[MenuItemConstants.PREFERENCE_ITEM_FIELD_NODE_NAME.attributeId].value as String, new I18nString(childFolderCase.dataSet[MenuItemConstants.PREFERENCE_ITEM_FIELD_NODE_NAME.attributeId].value as String)) | ||
| } |
There was a problem hiding this comment.
Null-safety when collecting children options
findCaseElastic(..) may return null – e.g. an empty tree or wrong path.
Current code dereferences caseByPath unguarded, causing NPE during option
generation.
- Case caseByPath = findCaseElastic("processIdentifier:$FilterRunner.PREFERRED_ITEM_NET_IDENTIFIER AND dataSet.nodePath.textValue.keyword:\"$path\"")
- Set<String> childrenIds = caseByPath.dataSet[MenuItemConstants.PREFERENCE_ITEM_FIELD_CHILD_ITEM_IDS.attributeId].value as Set
- if (!childrenIds.isEmpty()) {
+ Case caseByPath = findCaseElastic("processIdentifier:$FilterRunner.PREFERRED_ITEM_NET_IDENTIFIER AND dataSet.nodePath.textValue.keyword:\"$path\"")
+ if (caseByPath) {
+ Set<String> childrenIds = caseByPath.dataSet[MenuItemConstants.PREFERENCE_ITEM_FIELD_CHILD_ITEM_IDS.attributeId].value as Set
+ if (!childrenIds.isEmpty()) {
…
}
}📝 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
| Case caseByPath = findCaseElastic("processIdentifier:$FilterRunner.PREFERRED_ITEM_NET_IDENTIFIER AND dataSet.nodePath.textValue.keyword:\"$path\"") | |
| Set<String> childrenIds = caseByPath.dataSet[MenuItemConstants.PREFERENCE_ITEM_FIELD_CHILD_ITEM_IDS.attributeId].value as Set | |
| if (!childrenIds.isEmpty()) { | |
| for (String id : childrenIds) { | |
| UriNode childNode = uriService.findById(id) | |
| options.put(childNode.name, new I18nString(childNode.name)) | |
| Case childFolderCase = workflowService.findOne(id) | |
| options.put(childFolderCase.dataSet[MenuItemConstants.PREFERENCE_ITEM_FIELD_NODE_NAME.attributeId].value as String, new I18nString(childFolderCase.dataSet[MenuItemConstants.PREFERENCE_ITEM_FIELD_NODE_NAME.attributeId].value as String)) | |
| } | |
| Case caseByPath = findCaseElastic("processIdentifier:$FilterRunner.PREFERRED_ITEM_NET_IDENTIFIER AND dataSet.nodePath.textValue.keyword:\"$path\"") | |
| if (caseByPath) { | |
| Set<String> childrenIds = caseByPath.dataSet[MenuItemConstants.PREFERENCE_ITEM_FIELD_CHILD_ITEM_IDS.attributeId].value as Set | |
| if (!childrenIds.isEmpty()) { | |
| for (String id : childrenIds) { | |
| Case childFolderCase = workflowService.findOne(id) | |
| options.put( | |
| childFolderCase.dataSet[MenuItemConstants.PREFERENCE_ITEM_FIELD_NODE_NAME.attributeId].value as String, | |
| new I18nString(childFolderCase.dataSet[MenuItemConstants.PREFERENCE_ITEM_FIELD_NODE_NAME.attributeId].value as String) | |
| ) | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In
application-engine/src/main/groovy/com/netgrif/application/engine/petrinet/domain/dataset/logic/action/ActionDelegate.groovy
around lines 2822 to 2828, the variable caseByPath can be null if
findCaseElastic returns no result, leading to a NullPointerException when
accessing its dataSet. Add a null check after assigning caseByPath to ensure it
is not null before accessing its properties and processing childrenIds. If
caseByPath is null, skip the children options collection to prevent the NPE.
renczesstefan
changed the title
Introduced annotations (`EntryPoint` and `EntryPointMethod`) and corresponding domain models (`Plugin`, `EntryPoint`, `Method`, and `ListenerFilter`) to support a plugin architecture. These enable defining and managing plugins, their methods, and listener filters for remote invocation.
Introduced a new interface to define plugin registration configuration. It includes methods for retrieving the plugin name, version, and entry points. This provides a foundation for standardizing plugin integration.
Introduced core classes and services to enable a plugin architecture. This includes plugin injection, entry point management, and dynamic meta-class updates for ActionDelegate. The changes set the foundation for extensible plugin support within the system.
Introduced core classes and services to enable a plugin architecture. This includes plugin injection, entry point management, and dynamic meta-class updates for ActionDelegate. The changes set the foundation for extensible plugin support within the system.
The EntryPointLoaderService interface was unused and redundant, so it has been deleted to simplify the codebase. This helps maintain cleaner and more maintainable code.
Replaced the use of EntryPointLoaderService with ApplicationContext to retrieve PluginRegistrationConfiguration beans. This simplifies the process of loading plugins and their entry points, consolidating configuration management and improving maintainability.
Updated the PluginRunner to assign the plugin version from the configuration file instead of using a hardcoded value. This ensures better flexibility and easier version management for plugins.
Introduced PluginHolder as a new field in ActionDelegate and initialized it in the constructor. This change supports enhanced plugin management within the ActionDelegate logic.
- Enhanced PluginInjector with improved meta-class injection logic and documentation. - Updated PluginRegistrationConfiguration to include metadata retrieval.
- Introduced @target and @retention annotations for ListenerFilter definition. - Ensured compatibility with method-level usage and set retention policy at runtime.
[NAE-2129] Inject Plugin JAR into Community Edition
- Deleted the unused `IUriService` import from `ActionDelegate.groovy` to improve code cleanliness. - No changes to functionality or behavior, ensuring adherence to best practices.
Replaced all instances of `MenuItemConstants.attributeId` with `value` for improved clarity and consistency in referencing constants. Updated related logic in workflow and dataset handling across multiple files to align with the new structure.
Introduced a @Getter annotation for the 'path' field to expose it externally and implemented a setter method to ensure the value is trimmed before assignment. These changes improve data encapsulation and ensure the 'path' field is managed consistently.
If the import service does not return a PetriNet object, a warning is logged, and an empty Optional is returned. This prevents potential null pointer issues and improves logging for debugging.
Simplified and optimized the `createNodePath` method by normalizing the input path before concatenation. This reduces redundancy and ensures consistent handling of path separators.
tuplle
previously approved these changes
Jun 24, 2025
machacjozef
changed the base branch from
release/7.0.0-rev2
to
release/7.0.0-rev3
martinkranec
reviewed
Jul 1, 2025
| def newUri = uriService.getOrCreate("/netgrif/test_new", UriContentType.DEFAULT) | ||
| caze = setData(caze, [ | ||
| "uri": newUri.path, | ||
| "uri": "/netgrif/test_new", |
18 tasks
This was referenced Jul 28, 2025
This was referenced Aug 14, 2025
18 tasks
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
Removal of UriNode concept.
Implements NAE-2125
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
Refactor
Bug Fixes
Tests
Chores