[NAE-2229] Action API #373
Conversation
Introduced `ActionApi` interface and its implementation `ActionApiImpl` to standardize and handle various workflow operations like task assignment, case creation, and data
Updated `Map` to `HashMap` for parameters to ensure consistent type usage. Added `processIdentifier` parameter to `searchCases` and `searchTasks` methods. Replaced `userId` with `username` in task-related methods to better align with modern practices. Removed unused `createCase` method to eliminate redundancy.
Replaced username and realmId parameters with AuthPrincipalDto to standardize user context handling across Action API methods. Added a helper method to resolve AbstractUser from AuthPrincipalDto, ensuring consistent and concise user lookup logic. This change improves maintainability and aligns with updated authentication patterns.
Introduced new methods in ActionApi and implemented them in ActionApiImpl, enabling user, case, and task operations. Updated related services and repositories to support user search by realm with pagination and predicate filtering.
renczesstefan
added
improvement
WalkthroughAdds a new ActionApi surface (interface, impl, configuration) with case/task/user/data operations, introduces AuthPrincipalDto and ActionApiMethods, adds method-resolution utilities and AmbiguousMethodCallException, extends UserService with realm-scoped search, adjusts QueryDSL/maven config, and small POJO/parameter renames. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant ActionApi
participant DataService
participant WorkflowService
participant TaskService
participant ElasticService
participant UserService
Note over Client,ActionApi: Data read/write
Client->>ActionApi: getData(taskId, params)
ActionApi->>DataService: getData(taskId)
DataService-->>ActionApi: GetDataEventOutcome
ActionApi-->>Client: outcome
Client->>ActionApi: setData(taskId, dataSet, params)
ActionApi->>ActionApi: convert nested map → ObjectNode
ActionApi->>DataService: setData(taskId, objectNode)
DataService-->>ActionApi: SetDataEventOutcome
ActionApi-->>Client: outcome
Note over Client,ActionApi: Search (predicate or elastic)
Client->>ActionApi: searchCases(..., authPrincipal?, pageable, isIntersection?)
ActionApi->>ActionApi: resolve AuthPrincipalDto → LoggedUser (locale)
alt elastic queries
ActionApi->>ElasticService: execute elastic query list
ElasticService-->>ActionApi: results
else predicate
ActionApi->>WorkflowService: searchCases(predicate, pageable)
WorkflowService-->>ActionApi: results
end
ActionApi-->>Client: Page<Case>
Note over Client,ActionApi: Task lifecycle
Client->>ActionApi: assignTask(taskId, authPrincipal, params)
ActionApi->>ActionApi: resolve user by username+realm
ActionApi->>TaskService: assign(taskId, user, params)
TaskService-->>ActionApi: AssignTaskEventOutcome
ActionApi-->>Client: outcome
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Files/areas to inspect closely:
Possibly related PRs
Pre-merge checks✅ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (9)
application-engine/src/main/java/com/netgrif/application/engine/actions/ActionApiConfiguration.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/actions/ActionApiImpl.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/elastic/web/requestbodies/ElasticTaskSearchRequest.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/petrinet/service/ProcessRoleService.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/auth/dto/AuthPrincipalDto.java(1 hunks)nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/actions/ActionApi.java(1 hunks)nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/actions/ActionApiMethods.java(1 hunks)nae-user-ce/src/main/java/com/netgrif/application/engine/auth/service/UserServiceImpl.java(2 hunks)nae-user-common/src/main/java/com/netgrif/application/engine/auth/service/UserService.java(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
application-engine/src/main/java/com/netgrif/application/engine/actions/ActionApiImpl.java (1)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/auth/domain/ActorTransformer.java (1)
ActorTransformer(10-131)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: Build
🔇 Additional comments (2)
application-engine/src/main/java/com/netgrif/application/engine/petrinet/service/ProcessRoleService.java (1)
103-105: Improved parameter clarityThanks for renaming the argument to
roleId—the call flow reads much clearer now with the composite ID intent explicit. ✅nae-user-common/src/main/java/com/netgrif/application/engine/auth/service/UserService.java (1)
429-437: Unify return type with existing API and coordinate related API boundariesThe suggestion to use
Page<AbstractUser>is valid—most UserService pagination methods returnAbstractUserfor abstraction. However, verification reveals a coordination requirement: ActionApi interface (nae-spring-core-adapter) explicitly definesPage<User> searchUsers(...), and ActionApiImpl delegates directly touserService.search(). Changing the UserService return type also requires updating both ActionApi and ActionApiImpl for consistency.Two other UserService methods already return concrete
Page<User>(line 52, line 398), so precedent exists but the majority pattern favors abstraction. Before implementing this refactor:
- Confirm whether ActionApi is part of the public API contract with external consumers
- Coordinate changes across UserService, ActionApi interface, and ActionApiImpl if proceeding
| @Bean | ||
| @ConditionalOnMissingBean | ||
| public ActionApi actionApi() { | ||
| return new ActionApiImpl(); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Be explicit in conditional and prefer constructor wiring
Use explicit type in @ConditionalOnMissingBean to avoid surprises, and consider constructor injection for ActionApiImpl dependencies.
@Bean
- @ConditionalOnMissingBean
+ @ConditionalOnMissingBean(ActionApi.class)
public ActionApi actionApi() {
return new ActionApiImpl();
}📝 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.
| @Bean | |
| @ConditionalOnMissingBean | |
| public ActionApi actionApi() { | |
| return new ActionApiImpl(); | |
| } | |
| @Bean | |
| @ConditionalOnMissingBean(ActionApi.class) | |
| public ActionApi actionApi() { | |
| return new ActionApiImpl(); | |
| } |
🤖 Prompt for AI Agents
In
application-engine/src/main/java/com/netgrif/application/engine/actions/ActionApiConfiguration.java
around lines 11 to 15, update the conditional and wiring: replace the bare
@ConditionalOnMissingBean with @ConditionalOnMissingBean(ActionApi.class) to be
explicit about the type, and prefer constructor injection for ActionApiImpl by
either registering ActionApiImpl as a Spring-managed bean with its dependencies
injected via its constructor or by changing the @Bean method to accept the
required dependencies as parameters and return new ActionApiImpl(...); ensure
the bean creation uses the concrete dependencies via constructor rather than
instantiating with a no-arg constructor.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
application-engine/src/main/java/com/netgrif/application/engine/actions/ActionApiImpl.java
Show resolved
Hide resolved
| import lombok.Builder; | ||
| import lombok.NoArgsConstructor; | ||
|
|
||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.stream.Collectors; | ||
|
|
||
| @Builder |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
Verify builder scope: @builder only covers local fields, not inherited ones.
Adding @Builder to a class that extends another class generates a builder that only includes fields declared in ElasticTaskSearchRequest (i.e., just the query field). Inherited fields from TaskSearchRequest (role, useCase, title, user, process, transitionId, fullText, group, users) referenced in the custom constructor will not be part of the generated builder.
If the intention is to provide builder support for all fields including inherited ones, both this class and TaskSearchRequest must use @SuperBuilder instead of @Builder.
Run the following script to verify the parent class structure and existing builder usage:
🏁 Script executed:
#!/bin/bash
# Check parent class TaskSearchRequest for annotations and fields
echo "=== Checking parent class TaskSearchRequest ==="
fd -e java -x grep -l "class TaskSearchRequest" | head -1 | xargs cat
echo -e "\n=== Searching for builder usage on ElasticTaskSearchRequest ==="
rg -n --type java -C3 'ElasticTaskSearchRequest\.builder\(\)'
echo -e "\n=== Searching for TaskSearchRequest superbuilder patterns ==="
rg -n --type java -C3 '@SuperBuilder.*class.*TaskSearchRequest'Length of output: 3171
🏁 Script executed:
#!/bin/bash
# Check for other instantiation patterns of ElasticTaskSearchRequest
echo "=== All instantiation patterns of ElasticTaskSearchRequest ==="
rg -n --type java 'new ElasticTaskSearchRequest|ElasticTaskSearchRequest\.builder|ElasticTaskSearchRequest\(' -C2
# Check if parent fields are ever set via setter after build()
echo -e "\n=== Check for setter patterns after builder build ==="
rg -n --type java -A5 'ElasticTaskSearchRequest\.builder.*build\(\)'
# Check the custom constructor usage if any
echo -e "\n=== Check for Map-based constructor usage ==="
rg -n --type java 'new ElasticTaskSearchRequest\(.*Map' -C2Length of output: 3002
Switch to @SuperBuilder to include inherited fields
The current @Builder on ElasticTaskSearchRequest only exposes its own query field; fields from TaskSearchRequest aren’t part of the generated builder. If you need to set parent properties via builder, replace @Builder/@AllArgsConstructor with Lombok’s @SuperBuilder on both classes.
🤖 Prompt for AI Agents
In
application-engine/src/main/java/com/netgrif/application/engine/elastic/web/requestbodies/ElasticTaskSearchRequest.java
around lines 7 to 14, the class uses Lombok's @Builder which only builds its own
fields and does not include inherited fields from TaskSearchRequest; replace
@Builder with Lombok's @SuperBuilder on this class and ensure the parent
TaskSearchRequest is also annotated with @SuperBuilder (replace any
@Builder/@AllArgsConstructor there as well), keep or add a no-args constructor
annotation if needed (e.g., @NoArgsConstructor) and remove/conflict any
duplicate builder annotations so the generated builder includes inherited
properties.
There was a problem hiding this comment.
It is OK for us to us Builder and AllArgsConstructor.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
It is OK for us to us Builder and AllArgsConstructor.
...-library/src/main/java/com/netgrif/application/engine/objects/auth/dto/AuthPrincipalDto.java
Show resolved
Hide resolved
...e-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/actions/ActionApi.java
Outdated
Show resolved
Hide resolved
| SEARCH_USER("searchUsers"); | ||
|
|
||
| private String methodName; | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Fix naming and immutability
- Constant name should match the method: SEARCH_USERS.
- Make methodName final.
- SEARCH_USER("searchUsers");
+ SEARCH_USERS("searchUsers");
@@
- private String methodName;
+ private final String methodName;Please update any usages of SEARCH_USER accordingly.
Also applies to: 20-21
🤖 Prompt for AI Agents
In
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/actions/ActionApiMethods.java
around lines 18 to 21, rename the enum constant SEARCH_USER to SEARCH_USERS to
match the method name, change the instance field methodName to be final, and
update the enum constructor/assignments accordingly; also update any usages
across the codebase that reference SEARCH_USER to use SEARCH_USERS; apply the
same renaming and immutability fix for the related declaration at lines 20-21.
There was a problem hiding this comment.
@renczesstefan, thank you for implementing the changes! 🎉
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
nae-user-ce/src/main/java/com/netgrif/application/engine/auth/service/UserServiceImpl.java
Show resolved
Hide resolved
Moved Querydsl dependencies and plugin configuration from `nae-user-common` to `nae-spring-core-adapter`. Updated package structure to align with the new module structure, improving separation of concerns and maintainability.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
nae-spring-core-adapter/pom.xml(2 hunks)nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/package-info.java(1 hunks)nae-user-common/pom.xml(0 hunks)
💤 Files with no reviewable changes (1)
- nae-user-common/pom.xml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Test
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
🔇 Additional comments (3)
nae-spring-core-adapter/pom.xml (2)
115-119: LGTM.The
querydsl-aptdependency is the correct annotation processor for QueryDSL code generation.
144-167: LGTM.The
apt-maven-pluginconfiguration properly integrates QueryDSL annotation processing into the Maven build lifecycle. The processor output directory, exclusions, and options are appropriately configured for this project.nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/package-info.java (1)
13-13: LGTM! Package declaration now correctly matches the file location.The package name correction from
com.netgrif.application.engine.authtocom.netgrif.application.engine.adapter.springis valid. Verification confirms no Java source code incorrectly references the old package name, and the imports (lines 15–20) are consistent with the corrected package. The file's directory structure now aligns with its package declaration.
Replaced HashMap with Map in ActionApi and ActionApiImpl for flexibility and cleaner API design. Introduced method resolution with superclass parameter types in NaeReflectionUtils, along with custom exception handling for ambiguous method calls.
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/utils/NaeReflectionUtils.java (1)
27-31: resolveClass may return null but downstream assumes non-null.Either document that findMethod rejects null beans (preferred) or make resolveClass throw on null to avoid surprising NPEs later. Current comment advertises null returns; findMethod now enforces non-null. Align docs.
♻️ Duplicate comments (6)
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/actions/ActionApi.java (4)
78-79: Prefer primitive boolean for isIntersection.Avoid nullable Boolean to prevent accidental NPEs on autounboxing and to signal required input.
- Page<Case> searchCases(List<String> elasticStringQueries, AuthPrincipalDto authPrincipalDto, Pageable pageable, Boolean isIntersection); + Page<Case> searchCases(List<String> elasticStringQueries, AuthPrincipalDto authPrincipalDto, Pageable pageable, boolean isIntersection);
128-129: Prefer primitive boolean for isIntersection.Same rationale as cases search.
- Page<Task> searchTasks(List<String> elasticStringQueries, AuthPrincipalDto authPrincipalDto, Pageable pageable, Boolean isIntersection); + Page<Task> searchTasks(List<String> elasticStringQueries, AuthPrincipalDto authPrincipalDto, Pageable pageable, boolean isIntersection);
49-50: Harden mutating API: include AuthPrincipalDto in setData.For consistent authorization and auditing on writes.
- SetDataEventOutcome setData(String taskId, Map<String, Map<String, String>> dataSet, Map<String, String> params) throws JsonProcessingException; + SetDataEventOutcome setData(String taskId, Map<String, Map<String, String>> dataSet, Map<String, String> params, AuthPrincipalDto authPrincipalDto) throws JsonProcessingException;
92-100: Harden mutating API: require AuthPrincipalDto in deleteCase.Aligns with other mutating methods (create, assign, cancel, finish) and prevents silent reliance on ambient SecurityContext.
- DeleteCaseEventOutcome deleteCase(String caseId, Map<String, String> params); + DeleteCaseEventOutcome deleteCase(String caseId, Map<String, String> params, AuthPrincipalDto authPrincipalDto);application-engine/src/main/java/com/netgrif/application/engine/actions/ActionApiImpl.java (2)
52-77: elasticTaskService is never injected → NPE on searchTasks.Wire it like other services.
private IElasticTaskService elasticTaskService; @@ public void setUserService(UserService userService) { this.userService = userService; } + + @Autowired + public void setElasticTaskService(IElasticTaskService elasticTaskService) { + this.elasticTaskService = elasticTaskService; + }
112-115: deleteCase lacks principal/locale; authorization/auditing gap.Align with createCase/elastic search methods: resolve user and propagate.
- public DeleteCaseEventOutcome deleteCase(String caseId, Map<String, String> params) { - return workflowService.deleteCase(caseId, params); - } + public DeleteCaseEventOutcome deleteCase(String caseId, Map<String, String> params, AuthPrincipalDto authPrincipalDto) { + LoggedUser loggedUser = ActorTransformer.toLoggedUser(resolveAbstractUser(authPrincipalDto)); + Locale locale = LocaleContextHolder.getLocale(); + return workflowService.deleteCase(caseId, loggedUser, locale, params); + }If no such overload exists, please expose one in IWorkflowService, or document reliance on SecurityContext and ensure it is set here.
#!/bin/bash rg -n "deleteCase\\(" --type java application-engine/src/main/java | head -20
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
application-engine/src/main/java/com/netgrif/application/engine/actions/ActionApiImpl.java(1 hunks)nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/actions/ActionApi.java(1 hunks)nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/utils/NaeReflectionUtils.java(2 hunks)nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/utils/exceptions/AmbiguousMethodCallException.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
application-engine/src/main/java/com/netgrif/application/engine/actions/ActionApiImpl.java (1)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/auth/domain/ActorTransformer.java (1)
ActorTransformer(10-131)
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/utils/NaeReflectionUtils.java (1)
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/utils/exceptions/AmbiguousMethodCallException.java (1)
AmbiguousMethodCallException(3-12)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: Build
application-engine/src/main/java/com/netgrif/application/engine/actions/ActionApiImpl.java
Show resolved
Hide resolved
application-engine/src/main/java/com/netgrif/application/engine/actions/ActionApiImpl.java
Show resolved
Hide resolved
application-engine/src/main/java/com/netgrif/application/engine/actions/ActionApiImpl.java
Show resolved
Hide resolved
application-engine/src/main/java/com/netgrif/application/engine/actions/ActionApiImpl.java
Show resolved
Hide resolved
application-engine/src/main/java/com/netgrif/application/engine/actions/ActionApiImpl.java
Show resolved
Hide resolved
application-engine/src/main/java/com/netgrif/application/engine/actions/ActionApiImpl.java
Show resolved
Hide resolved
...e-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/actions/ActionApi.java
Show resolved
Hide resolved
...netgrif/application/engine/adapter/spring/utils/exceptions/AmbiguousMethodCallException.java
Outdated
Show resolved
Hide resolved
...er/src/main/java/com/netgrif/application/engine/adapter/spring/utils/NaeReflectionUtils.java
Show resolved
Hide resolved
...er/src/main/java/com/netgrif/application/engine/adapter/spring/utils/NaeReflectionUtils.java
Show resolved
Hide resolved
Refactored `ActionApiImpl` to include `ObjectMapper` and handle `isIntersection` using a safer boolean check. Updated `AmbiguousMethodCallException` to implement `Serializable` and added `serialVersionUID` for better compatibility. These changes improve code clarity, safety, and functionality.
Specify ActionApi class in the ConditionalOnMissingBean annotation to ensure proper bean initialization. This prevents unintended behavior when similar beans are present in the context.
The @ToString.Exclude and @EqualsAndHashCode.Exclude annotations were added to the sessionId field in AuthPrincipalDto. This ensures that the sessionId is not included in toString or equals/hashCode operations, improving security and avoiding unintended leaks of sensitive data.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
application-engine/src/main/java/com/netgrif/application/engine/actions/ActionApiImpl.java (4)
97-102: Use injected ObjectMapper; avoid JSON round-trip.This issue was flagged in a previous review but remains unresolved. Line 98 creates a new
ObjectMapper()instead of using the injectedobjectMapperfield (lines 86-89), and lines 99-100 perform an unnecessary serialize→parse round-trip.Apply this diff to use the injected mapper and direct conversion:
public SetDataEventOutcome setData(String taskId, Map<String, Map<String, String>> dataSet, Map<String, String> params) throws JsonProcessingException { - ObjectMapper mapper = new ObjectMapper(); - String json = mapper.writeValueAsString(dataSet); - ObjectNode values = (ObjectNode) mapper.readTree(json); + ObjectNode values = objectMapper.valueToTree(dataSet); return dataService.setData(taskId, values, params); }
105-107: Apply processIdentifier filter or remove unused parameter.This issue was flagged in a previous review but remains unresolved. The
processIdentifierparameter is accepted but never used in line 106. The search will return cases from all processes instead of filtering by the specifiedprocessIdentifier.Either compose the predicate to include the processIdentifier filter, or remove the parameter if it's not needed. If filtering is required, you'll need to combine predicates using QueryDSL, for example:
public Page<Case> searchCases(String processIdentifier, Predicate predicate, Pageable pageable) { Predicate combined = predicate; if (processIdentifier != null && !processIdentifier.isEmpty()) { Predicate processFilter = QCase.case$.processIdentifier.eq(processIdentifier); combined = predicate != null ? ExpressionUtils.and(predicate, processFilter) : processFilter; } return workflowService.search(combined, pageable); }
131-133: Apply processIdentifier filter or remove unused parameter.Same issue as
searchCasesat lines 105-107: theprocessIdentifierparameter is accepted but never used. Tasks from all processes will be returned instead of filtering by the specified identifier.Apply the same fix pattern as suggested for
searchCases, using the appropriate QueryDSL Q-type for Task filtering.
110-116: Consider primitive boolean parameter for clarity.The nullable
Boolean isIntersectionparameter at line 110 is safely handled withBoolean.TRUE.equals()at line 111. However, a previous review suggested using primitivebooleanthroughout the API to eliminate null ambiguity and make the contract explicit.If external callers can pass null (meaning "use default behavior"), the current defensive pattern is acceptable. Otherwise, consider changing to primitive
booleanin the interface and all implementations.Should the API require explicit boolean values? If so, update the signature:
-public Page<Case> searchCases(List<String> elasticStringQueries, AuthPrincipalDto authPrincipalDto, Pageable pageable, Boolean isIntersection) { - boolean intersect = Boolean.TRUE.equals(isIntersection); +public Page<Case> searchCases(List<String> elasticStringQueries, AuthPrincipalDto authPrincipalDto, Pageable pageable, boolean isIntersection) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
application-engine/src/main/java/com/netgrif/application/engine/actions/ActionApiConfiguration.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/actions/ActionApiImpl.java(1 hunks)nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/utils/exceptions/AmbiguousMethodCallException.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
application-engine/src/main/java/com/netgrif/application/engine/actions/ActionApiImpl.java (1)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/auth/domain/ActorTransformer.java (1)
ActorTransformer(10-131)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (22)
- GitHub Check: task-list-completed
- GitHub Check: Test
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
🔇 Additional comments (3)
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/utils/exceptions/AmbiguousMethodCallException.java (1)
11-17: Implementation looks good.The constructors follow standard exception patterns and cover the primary use cases. Previous review concerns regarding
serialVersionUIDand thefinalmodifier have been properly addressed.application-engine/src/main/java/com/netgrif/application/engine/actions/ActionApiConfiguration.java (1)
8-16: LGTM! Configuration is correct.The explicit type in
@ConditionalOnMissingBean(ActionApi.class)and the bean definition are appropriate for the setter-based injection pattern used inActionApiImpl.application-engine/src/main/java/com/netgrif/application/engine/actions/ActionApiImpl.java (1)
180-186: LGTM! Robust user resolution.The null guard at lines 181-183 and the clear exception message at line 185 ensure fail-fast behavior with actionable error details. This addresses the concerns from previous reviews.
...netgrif/application/engine/adapter/spring/utils/exceptions/AmbiguousMethodCallException.java
Outdated
Show resolved
Hide resolved
...netgrif/application/engine/adapter/spring/utils/exceptions/AmbiguousMethodCallException.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/auth/dto/AuthPrincipalDto.java(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: Test
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
🔇 Additional comments (1)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/auth/dto/AuthPrincipalDto.java (1)
19-21: Previous concern properly addressed.The sessionId field is now correctly excluded from toString and equals/hashCode, preventing accidental leakage in logs and avoiding equality issues with ephemeral session data.
...-library/src/main/java/com/netgrif/application/engine/objects/auth/dto/AuthPrincipalDto.java
Show resolved
Hide resolved
Corrected the enum constant to accurately reflect the plural form, ensuring consistency with the corresponding method name. This improves code clarity and prevents potential mismatches or confusion.
The `Serializable` interface was unnecessary in `AmbiguousMethodCallException` since `RuntimeException` already implements it. This change simplifies the class and avoids redundant code.
The `Serializable` interface was unnecessary in `AmbiguousMethodCallException` since `RuntimeException` already implements it. This change simplifies the class and avoids redundant code.
Added `@NonNull` annotations to critical fields in `AuthPrincipalDto` to ensure required values are always provided. Refined method retrieval in `NaeReflectionUtils` to filter only bridge and synthetic methods, improving specificity. Expanded `AmbiguousMethodCallException` with detailed Javadoc for better clarity and usage documentation.
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (3)
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/utils/NaeReflectionUtils.java (1)
72-79: Add null guards and normalize param array to avoid NPEs and clarify API.
Guard bean/method name and treat null requestParamTypes as no-args; pass normalized array to both paths.- public static Method findMethod(Object bean, String methodToExecute, Class<?>[] requestParamTypes) - throws NoSuchMethodException, AmbiguousMethodCallException { - try { - return NaeReflectionUtils.resolveClass(bean).getMethod(methodToExecute, requestParamTypes); - } catch (NoSuchMethodException e) { - return findMethodWithSuperClassParams(bean, methodToExecute, requestParamTypes, e); - } - } + public static Method findMethod(Object bean, String methodToExecute, Class<?>[] requestParamTypes) + throws NoSuchMethodException, AmbiguousMethodCallException { + Objects.requireNonNull(bean, "bean must not be null"); + Objects.requireNonNull(methodToExecute, "methodToExecute must not be null"); + final Class<?>[] paramTypes = (requestParamTypes == null) ? new Class<?>[0] : requestParamTypes; + try { + return NaeReflectionUtils.resolveClass(bean).getMethod(methodToExecute, paramTypes); + } catch (NoSuchMethodException e) { + return findMethodWithSuperClassParams(bean, methodToExecute, paramTypes, e); + } + }nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/actions/ActionApiMethods.java (1)
20-28: Make methodName immutable.Declare methodName final to enforce enum immutability.
- private String methodName; + private final String methodName;nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/actions/ActionApi.java (1)
77-77: Use primitive boolean for isIntersection.Prevents accidental nulls and simplifies callers.
- Page<Case> searchCases(List<String> elasticStringQueries, AuthPrincipalDto authPrincipalDto, Pageable pageable, Boolean isIntersection); + Page<Case> searchCases(List<String> elasticStringQueries, AuthPrincipalDto authPrincipalDto, Pageable pageable, boolean isIntersection); @@ - Page<Task> searchTasks(List<String> elasticStringQueries, AuthPrincipalDto authPrincipalDto, Pageable pageable, Boolean isIntersection); + Page<Task> searchTasks(List<String> elasticStringQueries, AuthPrincipalDto authPrincipalDto, Pageable pageable, boolean isIntersection);Also applies to: 127-127
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/auth/dto/AuthPrincipalDto.java(1 hunks)nae-spring-core-adapter/pom.xml(2 hunks)nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/actions/ActionApi.java(1 hunks)nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/actions/ActionApiMethods.java(1 hunks)nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/utils/NaeReflectionUtils.java(2 hunks)nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/utils/exceptions/AmbiguousMethodCallException.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/utils/NaeReflectionUtils.java (1)
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/utils/exceptions/AmbiguousMethodCallException.java (1)
AmbiguousMethodCallException(9-32)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: Build
- GitHub Check: task-list-completed
🔇 Additional comments (4)
nae-spring-core-adapter/pom.xml (2)
101-114: Dependencies correctly added; past duplicate issue resolved. ✅The
commons-lang3dependency (lines 101–104) lacks an explicit version, but this is correct—Spring Boot's BOM (imported at line 60–64) manages it. Thequerydsl-aptartifact (lines 110–114) is properly added with a matching version placeholder. The previous duplicatequerydsl-coreissue flagged in past reviews has been resolved.
139-162: Querydsl annotation processor configuration is correct. ✅The
apt-maven-pluginconfiguration properly runsQuerydslAnnotationProcessorwith output to Maven's standard generated-sources directory. The exclusion ofgroovy.lang.MetaClassand theunknownAsEmbeddable=trueoption follow Querydsl best practices. Generated query types will support the newActionApipredicates.One note:
apt-maven-pluginversion 1.1.3 is stable but dates from 2012. Verify this aligns with your project's Querydsl and Maven versions.nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/utils/exceptions/AmbiguousMethodCallException.java (1)
5-31: LGTM — well-scoped runtime exception with proper serialization and docs.
No issues found.nae-object-library/src/main/java/com/netgrif/application/engine/objects/auth/dto/AuthPrincipalDto.java (1)
11-25: DTO looks solid; previous concerns addressed.Fields, Lombok usage, and sessionId exclusions are correct.
...-library/src/main/java/com/netgrif/application/engine/objects/auth/dto/AuthPrincipalDto.java
Show resolved
Hide resolved
| Case findCase(String caseId); | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Clarify not-found semantics in Javadoc.
Document whether findCase/findTask return null or throw (and which exception). Reduces ambiguity for API consumers.
Also applies to: 106-107
🤖 Prompt for AI Agents
In
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/actions/ActionApi.java
around lines 56-57 and 106-107, add/extend the Javadoc for the findCase and
findTask methods to explicitly state their not-found semantics: specify whether
the method returns null or throws an exception, and if it throws, name the exact
exception type (or a custom exception) and the conditions under which it is
thrown; include example usage notes for callers (e.g., "may return null" or
"throws NotFoundException when no entity exists with the given id") and keep the
wording concise and consistent for both methods.
...e-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/actions/ActionApi.java
Show resolved
Hide resolved
...er/src/main/java/com/netgrif/application/engine/adapter/spring/actions/ActionApiMethods.java
Show resolved
Hide resolved
...er/src/main/java/com/netgrif/application/engine/adapter/spring/utils/NaeReflectionUtils.java
Outdated
Show resolved
Hide resolved
The @nonnull annotations were removed from the `username` and `realmId` fields in `AuthPrincipalDto` to relax validation constraints. This allows greater flexibility in handling null values for these fields where necessary.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/auth/dto/AuthPrincipalDto.java (1)
21-23: Add @JsonIgnore to prevent sessionId exposure in JSON payloads.The
sessionIdfield is properly excluded fromtoStringandequals/hashCode, but it's still included in JSON serialization. A previous review comment recommended adding@JsonIgnoreto prevent leaking this ephemeral session token in API responses.Given that
AuthPrincipalDtois used inActionApiand related authorization contexts (as shown in the AI summary), it's likely serialized in controller responses or inter-service communication. ExposingsessionIdcould create a security risk.Apply this diff to exclude
sessionIdfrom JSON serialization:package com.netgrif.application.engine.objects.auth.dto; +import com.fasterxml.jackson.annotation.JsonIgnore; import lombok.Data; import lombok.EqualsAndHashCode; import lombok.NonNull; import lombok.ToString; @@ @ToString.Exclude @EqualsAndHashCode.Exclude + @JsonIgnore private String sessionId;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/auth/dto/AuthPrincipalDto.java(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: Build
🔇 Additional comments (2)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/auth/dto/AuthPrincipalDto.java (2)
11-15: LGTM: Proper Serializable implementation.The class correctly implements
Serializablewith an explicitserialVersionUIDand uses the modern@Serialannotation. The@Dataannotation appropriately generates standard boilerplate.
17-19: Clarify nullability requirements for authentication identifiers.A previous review comment recommended adding
@NonNullannotations tousernameandrealmIdto enforce null-safety for required authentication context, and it was marked as addressed. However, the current code shows these fields without the annotations (though@NonNullis imported).If these fields are required for valid authentication (as suggested by the ActionApi usage context), please reapply the
@NonNullannotations:@Data public class AuthPrincipalDto implements Serializable { @Serial private static final long serialVersionUID = 6725518942728316525L; + @NonNull private String username; + @NonNull private String realmId;If they can legitimately be null in certain scenarios, remove the unused
@NonNullimport (see previous comment).
...-library/src/main/java/com/netgrif/application/engine/objects/auth/dto/AuthPrincipalDto.java
Outdated
Show resolved
Hide resolved
| <groupId>org.apache.commons</groupId> | ||
| <artifactId>commons-lang3</artifactId> | ||
| <groupId>com.querydsl</groupId> | ||
| <artifactId>querydsl-apt</artifactId> |
There was a problem hiding this comment.
Please correct the PR description
There was a problem hiding this comment.
These are not new, only moved to another module, I will mark it in dependencies section.
| * @param pageable the pagination information | ||
| * @return a page of cases matching the criteria | ||
| */ | ||
| Page<Case> searchCases(String processIdentifier, Predicate predicate, Pageable pageable); |
There was a problem hiding this comment.
Let's keep it consistent. You mix search and find words.
Either searchCase, searchCases
or findCase, findCases
Applies same for the tasks and users
There was a problem hiding this comment.
Search keyword refers to search multiple records using some query. Find refers to get one record by its ID. I want to keep it as is.
...er/src/main/java/com/netgrif/application/engine/adapter/spring/utils/NaeReflectionUtils.java
Outdated
Show resolved
Hide resolved
...er/src/main/java/com/netgrif/application/engine/adapter/spring/utils/NaeReflectionUtils.java
Show resolved
Hide resolved
Added `@JsonIgnore` to prevent `sessionId` serialization. Enhanced null-checking and parameter handling in `NaeReflectionUtils`. Optimized `ActionApiMethods` lookup with a prebuilt map for better performance.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/actions/ActionApi.java (1)
100-107: Clarify not-found semantics forfindTask(mirrorfindCase)
findCaseexplicitly documents “otherwise returns null” when the case is not found, butfindTask’s Javadoc only says “@return the found task” and omits what happens when no task exists. Please state whetherfindTaskreturns null or throws, and align the Javadoc with the actual implementation, similar tofindCase.nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/utils/NaeReflectionUtils.java (1)
72-81: NullrequestParamTypesstill cause NPE in fallback path
findMethodcorrectly normalizes a nullrequestParamTypesto an emptyparamTypesarray, but the fallback passes the original (possibly null) reference intofindMethodWithSuperClassParams, which then callsrequestParamTypes.lengthand will throwNullPointerException.Use the normalized array in the fallback to make the null-handling effective:
public static Method findMethod(Object bean, String methodToExecute, Class<?>[] requestParamTypes) throws NoSuchMethodException, AmbiguousMethodCallException { Objects.requireNonNull(bean, "bean must not be null"); Objects.requireNonNull(methodToExecute, "methodToExecute must not be null"); final Class<?>[] paramTypes = (requestParamTypes == null) ? new Class<?>[0] : requestParamTypes; try { - return NaeReflectionUtils.resolveClass(bean).getMethod(methodToExecute, paramTypes); + return NaeReflectionUtils.resolveClass(bean).getMethod(methodToExecute, paramTypes); } catch (NoSuchMethodException e) { - return findMethodWithSuperClassParams(bean, methodToExecute, requestParamTypes, e); + return findMethodWithSuperClassParams(bean, methodToExecute, paramTypes, e); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/auth/dto/AuthPrincipalDto.java(1 hunks)nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/actions/ActionApi.java(1 hunks)nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/actions/ActionApiMethods.java(1 hunks)nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/utils/NaeReflectionUtils.java(2 hunks)
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2025-11-14T09:31:48.850Z
Learnt from: Retoocs
Repo: netgrif/application-engine PR: 383
File: application-engine/src/main/java/com/netgrif/application/engine/configuration/security/ImpersonationRequestFilter.java:87-87
Timestamp: 2025-11-14T09:31:48.850Z
Learning: In the ImpersonationRequestFilter.java file within the security configuration, logging the full principal object (including its toString() representation) in the warn statement when a non-LoggedUser principal is encountered is acceptable, even though it may include PII. The team prefers the additional debugging context over restricting the log to just the class name.
Applied to files:
nae-object-library/src/main/java/com/netgrif/application/engine/objects/auth/dto/AuthPrincipalDto.java
📚 Learning: 2025-11-14T10:22:01.634Z
Learnt from: Retoocs
Repo: netgrif/application-engine PR: 383
File: application-engine/src/main/java/com/netgrif/application/engine/startup/ApplicationRunnerOrderResolver.java:43-43
Timestamp: 2025-11-14T10:22:01.634Z
Learning: For the netgrif/application-engine repository, avoid flagging trivial or nitpick-level issues such as redundant null checks, minor code style improvements, or obvious simplifications that don't affect functionality or introduce bugs. Focus review comments on substantive issues like logic errors, security concerns, performance problems, or breaking changes.
Applied to files:
nae-object-library/src/main/java/com/netgrif/application/engine/objects/auth/dto/AuthPrincipalDto.javanae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/actions/ActionApi.java
📚 Learning: 2025-09-23T10:56:18.499Z
Learnt from: renczesstefan
Repo: netgrif/application-engine PR: 359
File: nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/utils/NaeReflectionUtils.java:41-54
Timestamp: 2025-09-23T10:56:18.499Z
Learning: In NaeReflectionUtils.indexOfClass method, exact class equality using Objects.equals is intentional and should not be changed to inheritance-based matching using isAssignableFrom, as the method is designed for precise class identification.
Applied to files:
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/utils/NaeReflectionUtils.java
📚 Learning: 2025-09-23T10:56:18.499Z
Learnt from: renczesstefan
Repo: netgrif/application-engine PR: 359
File: nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/utils/NaeReflectionUtils.java:41-54
Timestamp: 2025-09-23T10:56:18.499Z
Learning: The indexOfClass method in NaeReflectionUtils is designed for exact class matching using Objects.equals, not inheritance-based matching. It's used in ApplicationRunnerOrderResolver for finding specific runner classes by their exact Class object for ordering purposes (BeforeRunner, AfterRunner annotations). Using isAssignableFrom would break the runner ordering logic.
Applied to files:
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/utils/NaeReflectionUtils.java
📚 Learning: 2025-10-20T11:46:37.958Z
Learnt from: renczesstefan
Repo: netgrif/application-engine PR: 373
File: application-engine/src/main/java/com/netgrif/application/engine/actions/ActionApiImpl.java:105-107
Timestamp: 2025-10-20T11:46:37.958Z
Learning: In ActionApiImpl, the processIdentifier parameter in searchCases() is intentionally unused because it's required by the ActionApi interface for plugin implementations. This implementation returns cases from all processes without filtering by processIdentifier.
Applied to files:
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/actions/ActionApi.java
📚 Learning: 2025-12-02T14:35:05.277Z
Learnt from: renczesstefan
Repo: netgrif/application-engine PR: 373
File: nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/actions/ActionApi.java:98-98
Timestamp: 2025-12-02T14:35:05.277Z
Learning: In ActionApi, the deleteCase method intentionally does not include an AuthPrincipalDto parameter because it does not require author information, unlike createCase which needs the author via authPrincipalDto.
Applied to files:
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/actions/ActionApi.java
🧬 Code graph analysis (1)
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/utils/NaeReflectionUtils.java (1)
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/utils/exceptions/AmbiguousMethodCallException.java (1)
AmbiguousMethodCallException(9-32)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test
🔇 Additional comments (2)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/auth/dto/AuthPrincipalDto.java (1)
11-24: AuthPrincipalDto structure and sessionId handling look soundSerializable DTO with Lombok plus explicit exclusion/ignoring of
sessionId(toString/equals/hashCode/JSON) is appropriate for avoiding leakage of ephemeral session data; no further issues from this file.nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/actions/ActionApiMethods.java (1)
7-37: Enum mapping and precomputed lookup map are appropriateCentralizing Action API method names in an enum and backing
fromMethodNamewith a precomputed unmodifiableMap<String, ActionApiMethods>gives clear semantics and efficient O(1) lookups; this design looks correct and robust.
| /** | ||
| * Searches for cases matching the given process identifier and predicate. | ||
| * | ||
| * @param processIdentifier the identifier of the process | ||
| * @param predicate the criteria for filtering cases | ||
| * @param pageable the pagination information | ||
| * @return a page of cases matching the criteria | ||
| */ | ||
| Page<Case> searchCases(String processIdentifier, Predicate predicate, Pageable pageable); | ||
|
|
There was a problem hiding this comment.
Align searchCases Javadoc with current implementation behavior
The Javadoc states that searchCases(String processIdentifier, Predicate predicate, Pageable pageable) searches “matching the given process identifier”, but the current ActionApiImpl intentionally ignores processIdentifier and returns cases from all processes.
To avoid surprising API consumers, either:
- update the Javadoc to reflect the actual behavior (e.g., note that the default implementation does not filter by
processIdentifier), or - adjust
ActionApiImplto honor the documented contract and filter byprocessIdentifier.
Based on learnings, the current implementation choice is intentional, so clarifying it in the Javadoc is likely the safest option.
🤖 Prompt for AI Agents
In
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/actions/ActionApi.java
around lines 58 to 67, the Javadoc for searchCases claims it filters by
processIdentifier but the implementation intentionally ignores that parameter;
update the Javadoc to state that this overload does not filter by
processIdentifier (returns cases across all processes) and mention that callers
should use another overload or pass a predicate that enforces process filtering
if they require it; keep the method signature and behavior unchanged, but make
the comment explicit about the implementation choice and any recommended
alternatives.
Description
Implements abstraction for Action API.
Implements NAE-2229
Dependencies
Moved to
nae-spring-core-adapterThird 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
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.