[NAE-2122] Implement Structured and Efficient Pagination in gRPC #323
Conversation
- Refactor repositories and services to support pagination - Updated repository and service layer methods to utilize pagination where applicable, improving scalability and performance. - Replaced unpaged results with pageable implementations across multiple data queries and ensured method signatures reflect these changes.
- Refactor pagination support across repositories and services - Updated methods to support `Pageable` for paginated data retrieval and processing. - Replaced bulk-loading calls with paginated queries to improve scalability and performance.
- Refactor co-member search logic for better pagination support - Replaced outdated co-member retrieval methods with a more efficient approach using predicates for filtering. - Removed unused methods and redundant code from GroupService and GroupServiceImpl, simplifying the logic.
- Fixed pagination response for WorkflowController where pageSize and pageNumber were flipped
- Fix serialization issue with Case Icon object
- Add comprehensive documentation and pagination to repositories - Enhanced Javadoc documentation across repositories and services for clarity. - Added methods supporting pagination in key repositories like UserRepository, AuthorityRepository, ProcessRoleRepository, and GroupRepository. - Removed unused methods in ElasticPetriNetService and related interfaces for cleaner implementation.
## Walkthrough
This change introduces comprehensive pagination support across user, group, authority, PetriNet, and process role services and repositories. Many methods that previously returned lists or sets now return paginated `Page` results and accept `Pageable` parameters. New configuration properties for pagination are added. Several repository and service interfaces are refactored, and related tests and usages are updated accordingly. Some obsolete or redundant methods are removed, and documentation is improved throughout.
## Changes
| Files/Groups | Change Summary |
|-----------------------------------------------------------------------------------------------------|----------------|
| `.gitignore` | Added `/storage/` to ignore root-level storage directory. |
| `application-engine/.../auth/service/RegistrationService.java`, `.../UserController.java`, `.../impersonation/service/ImpersonationAuthorizationService.java` | Updated user/authority retrieval methods to use paged queries (`Pageable`), replacing list-based logic. |
| `application-engine/.../elastic/domain/ElasticPetriNetRepository.java`, `.../service/ElasticPetriNetService.java`, `.../service/interfaces/IElasticPetriNetService.java` | Removed methods related to URI node ID; added Javadoc; removed `findAllByUriNodeId`. |
| `application-engine/.../importer/service/Importer.java` | Simplified role fetching: replaced list-based retrieval with single-object fetch by import ID. |
| `application-engine/.../petrinet/config/ProcessBeansConfiguration.java` | ProcessRoleService bean now injects `RealmService` and `PaginationProperties`. |
| `application-engine/.../petrinet/domain/repositories/PetriNetRepository.java` | Removed/added methods for paginated queries; added Javadoc; removed unused queries. |
| `application-engine/.../petrinet/domain/roles/ProcessRoleRepository.java` | Major refactor: added pagination, new query methods, Javadoc, and improved composite ID handling. |
| `application-engine/.../petrinet/service/PetriNetService.java` | Methods now support pagination (`Page`/`Pageable`), especially for references and identifier-based queries. |
| `application-engine/.../petrinet/service/ProcessRoleService.java` | Methods now accept/return paged results; deletion logic paginates over realms/users; constructor updated. |
| `application-engine/.../petrinet/service/interfaces/IPetriNetService.java` | Added static transform methods, overloads, pagination, cache control, and documentation. |
| `application-engine/.../petrinet/web/PetriNetController.java` | Controller now returns paged results directly from service, preserving pagination metadata. |
| `application-engine/.../startup/runner/AnonymousRoleRunner.java`, `.../DefaultRoleRunner.java` | Simplified existence checks for roles: switched from set-based to single-object queries. |
| `application-engine/.../startup/runner/DefaultFiltersRunner.java` | Replaced `Map.of` with mutable `HashMap` for filter metadata. |
| `application-engine/.../startup/runner/FunctionsCacheRunner.java` | Explicitly uses `Pageable.unpaged()` for PetriNet retrieval. |
| `application-engine/.../startup/runner/SuperCreatorRunner.java` | Loads all process roles/authorities via paginated loops; injects `PaginationProperties`. |
| `application-engine/.../workflow/service/CaseSearchService.java` | Uses `Pageable.unpaged()` for PetriNet retrieval; added TODO comment. |
| `application-engine/.../workflow/service/ConfigurableMenuService.java` | Uses paginated call for global roles retrieval. |
| `application-engine/.../workflow/service/WorkflowService.java`, `.../service/interfaces/IWorkflowService.java` | Removed `decrypt(Case)` method from service and interface. |
| `application-engine/.../workflow/web/WorkflowController.java` | Updated OpenAPI summaries to clarify pagination; fixed metadata construction order. |
| `application-engine/src/main/resources/petriNets/engine-processes/preference_item.xml` | Commented out action code for updating a field based on URI node ID. |
| `application-engine/src/test/groovy/com/netgrif/application/engine/action/AssignActionTest.groovy`, `.../auth/UserServiceTest.groovy`, `.../petrinet/domain/FunctionsTest.groovy`, `.../petrinet/domain/PetriNetTest.groovy`, `.../service/ProcessRoleServiceTest.java` | Updated tests to use paginated service/repository methods and assertions. |
| `nae-object-library/.../domain/Icon.java` | Made `Icon` class `Serializable` and added `serialVersionUID`. |
| `nae-spring-core-adapter/.../service/ProcessRoleService.java` | Updated interface methods to use paginated results (`Page`, `Pageable`). |
| `nae-spring-core-adapter/.../utils/PaginationProperties.java` | Added new configuration class for backend/frontend page sizes. |
| `nae-user-ce/.../auth/service/AuthorityServiceImpl.java` | Methods now support pagination; removed redundant permission/role methods. |
| `nae-user-ce/.../auth/service/GroupServiceImpl.java` | Added pagination to group queries; updated logic for default user group; removed unused methods. |
| `nae-user-ce/.../auth/service/UserServiceImpl.java` | All user queries now paginated and realm-scoped; removed deprecated imports; updated predicate logic. |
| `nae-user-common/.../auth/repository/AuthorityRepository.java` | Methods now return `Optional` and `Page`, removed list-based and prefix search methods, added Javadoc. |
| `nae-user-common/.../auth/repository/GroupRepository.java` | Methods now return paginated results; added Javadoc; updated parameters. |
| `nae-user-common/.../auth/repository/UserRepository.java` | Replaced default methods with explicit paginated, collection-scoped queries; added Javadoc. |
| `nae-user-common/.../auth/service/AuthorityService.java` | Interface now supports pagination, removed permission/role-specific methods, added Javadoc. |
| `nae-user-common/.../auth/service/GroupService.java` | Methods updated for pagination; removed co-member retrieval; added Javadoc. |
| `nae-user-common/.../auth/service/UserService.java` | Methods updated for pagination and realm scoping; removed overloaded/legacy methods. |
| `nae-user-common/.../auth/web/requestbodies/UserSearchRequestBody.java` | Added `realmId` field and JavaDoc comments for all fields. |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant Client
participant Controller
participant Service
participant Repository
Client->>Controller: Request paginated resource (e.g., users, roles)
Controller->>Service: getAll(Pageable pageable)
Service->>Repository: findAll(Pageable pageable)
Repository-->>Service: Page<Entity>
Service-->>Controller: Page<Entity>
Controller-->>Client: Page<Entity> (with pagination metadata)Possibly related PRs
Suggested labels
|
![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/main/java/com/netgrif/application/engine/startup/runner/DefaultFiltersRunner.java (1)
458-462:filterMetadatais still stored as a raw object graph – serialise it before persistingThe inline TODO highlights that the map should be converted to a string. Persisting a complex object into a text field may later break JSON querying or export/import.
One minimal fix insidecreateFilterCase:- setDataMap.put(FILTER_FIELD_ID, Map.of( - "type", "filter", - "value", filterQuery, - "allowedNets", allowedNets, - "filterMetadata", filterMetadata // TODO this is a map of <String, Object> that needs to be converted to string - )); + ObjectMapper mapper = new ObjectMapper(); + setDataMap.put(FILTER_FIELD_ID, Map.of( + "type", "filter", + "value", filterQuery, + "allowedNets", allowedNets, + "filterMetadata", mapper.writeValueAsString(filterMetadata) + ));Inject a singleton
ObjectMapperto avoid per-call instantiation and handle the possibleJsonProcessingException.nae-user-ce/src/main/java/com/netgrif/application/engine/auth/service/GroupServiceImpl.java (1)
473-475: Address the TODO: Implement proper pagination for group owner emailsUsing
Pageable.unpaged()loads all results at once, which could cause performance issues with large datasets. This TODO should be addressed before merging.Consider implementing a paginated approach or limiting the number of groups that can be searched at once. Would you like me to help implement a solution or open an issue to track this?
🧹 Nitpick comments (10)
application-engine/src/main/java/com/netgrif/application/engine/startup/runner/DefaultFiltersRunner.java (1)
60-113: Factor-out repetitiveHashMap<>(Map.of(...))boilerplateThe same 3-line pattern for building
filterMetadataappears four times. A tiny factory helper or a static method (e.g.private static Map<String,Object> meta(Object… kvPairs)) keepsrun()concise and makes later edits less error-prone.application-engine/src/main/java/com/netgrif/application/engine/startup/runner/FunctionsCacheRunner.java (1)
25-26: Address the TODO comment about unpaged usage.Using
Pageable.unpaged()is appropriate in this startup runner context since you need to cache functions from all PetriNets without pagination limitations. Consider removing the TODO comment if this is the intended behavior.- // TODO JOFO: unpaged necessary? - petriNetService.getAll(Pageable.unpaged()).forEach(cacheService::cachePetriNetFunctions); + petriNetService.getAll(Pageable.unpaged()).forEach(cacheService::cachePetriNetFunctions);application-engine/src/main/java/com/netgrif/application/engine/workflow/service/CaseSearchService.java (2)
271-272: Properly adapted to paginated interface with valid TODO consideration.The change correctly adapts to the new paginated
PetriNetServiceinterface while maintaining the existing functionality. The TODO comment raises a valid point about whether unpaged retrieval is necessary in this full-text search context. Consider evaluating if pagination would be beneficial here, especially for systems with large numbers of PetriNets, though unpaged may be justified for comprehensive search scenarios.
271-272: Consider pagination benefits for full-text search performance.The current implementation using
Pageable.unpaged().getContent()maintains existing behavior while adapting to the new paginated API. However, the TODO comment raises a valid concern.For full-text search operations, especially when dealing with large numbers of PetriNets, implementing proper pagination could significantly improve performance and memory usage. Consider:
- Performance: Loading all PetriNets at once may cause memory pressure in large deployments
- Scalability: Pagination would allow the search to scale better as the number of nets grows
- User Experience: Could enable progressive loading/searching
Consider evaluating whether pagination would be beneficial here:
-// TODO JOFO: unpaged necessary? -petriNets = petriNetService.getAll(Pageable.unpaged()).getContent(); +// Consider implementing pagination for better performance with large datasets +petriNets = petriNetService.getAll(PageRequest.of(0, maxPetriNetsForSearch)).getContent();application-engine/src/main/java/com/netgrif/application/engine/auth/service/RegistrationService.java (1)
64-76: Good pagination adaptation with minor optimization opportunityThe changes correctly adapt to the paginated user service API. However, consider optimizing the stream operation on line 74:
-userService.saveUsers(users.stream().map(u -> (IUser) u).collect(Collectors.toList())); +userService.saveUsers(users.getContent().stream().map(u -> (IUser) u).collect(Collectors.toList()));This avoids potential redundant iteration and makes the intent clearer by working directly with the page content.
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/utils/PaginationProperties.java (1)
11-13: Consider separating @configuration and @ConfigurationProperties concerns.While this works, it's generally better practice to keep
@ConfigurationPropertiesclasses separate from@Configurationclasses for better separation of concerns and testability.Consider this approach:
-@Data -@Configuration -@ConfigurationProperties(prefix = "netgrif.pagination") -public class PaginationProperties { +@Data +@ConfigurationProperties(prefix = "netgrif.pagination") +public class PaginationProperties {Then create a separate configuration class or enable this via
@EnableConfigurationProperties(PaginationProperties.class)in an existing configuration class.nae-user-common/src/main/java/com/netgrif/application/engine/auth/repository/GroupRepository.java (1)
13-26: Fix Javadoc inconsistency in return type description.The Javadoc comment describes the return type as "a set of Group entities" but the actual return type is
Page<Group>.- * @return a set of {@link Group} entities associated with the specified owner ID + * @return a {@link Page} of {@link Group} entities associated with the specified owner IDapplication-engine/src/main/java/com/netgrif/application/engine/startup/runner/SuperCreatorRunner.java (1)
116-116: Consider performance implications of the collection conversion chain.The current implementation creates multiple intermediate collections:
Page→Stream→List→Set. This is inefficient for large datasets.- superUser.setAuthorities(Set.copyOf(authorityService.findAll(Pageable.unpaged()).stream().toList())); + superUser.setAuthorities(new HashSet<>(authorityService.findAll(Pageable.unpaged()).getContent()));application-engine/src/main/java/com/netgrif/application/engine/petrinet/service/PetriNetService.java (1)
357-399: Consider simplifying the aggregation pagination logicThe current implementation manually handles pagination in MongoDB aggregation. While functional, this could be simplified using Spring Data MongoDB's aggregation framework features.
Consider extracting this complex aggregation logic into a custom repository implementation or using Spring Data MongoDB's
AggregationOperationwith built-in pagination support. This would improve maintainability and reduce the complexity of this method.nae-user-common/src/main/java/com/netgrif/application/engine/auth/service/UserService.java (1)
113-113: Fix parameter name to reflect single realm usage.The parameter name
realmIdsshould be singularrealmIdto maintain consistency with other methods in this interface that accept a single realm identifier.- Page<User> findAllByStateAndExpirationDateBefore(UserState state, LocalDateTime expirationDate, String realmIds, Pageable pageable); + Page<User> findAllByStateAndExpirationDateBefore(UserState state, LocalDateTime expirationDate, String realmId, Pageable pageable);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (44)
.gitignore(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/auth/service/RegistrationService.java(3 hunks)application-engine/src/main/java/com/netgrif/application/engine/auth/web/UserController.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/elastic/domain/ElasticPetriNetRepository.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticPetriNetService.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/elastic/service/interfaces/IElasticPetriNetService.java(0 hunks)application-engine/src/main/java/com/netgrif/application/engine/impersonation/service/ImpersonationAuthorizationService.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/importer/service/Importer.java(2 hunks)application-engine/src/main/java/com/netgrif/application/engine/petrinet/config/ProcessBeansConfiguration.java(3 hunks)application-engine/src/main/java/com/netgrif/application/engine/petrinet/domain/repositories/PetriNetRepository.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/petrinet/domain/roles/ProcessRoleRepository.java(3 hunks)application-engine/src/main/java/com/netgrif/application/engine/petrinet/service/PetriNetService.java(4 hunks)application-engine/src/main/java/com/netgrif/application/engine/petrinet/service/ProcessRoleService.java(14 hunks)application-engine/src/main/java/com/netgrif/application/engine/petrinet/service/interfaces/IPetriNetService.java(1 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/startup/runner/AnonymousRoleRunner.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/startup/runner/DefaultFiltersRunner.java(3 hunks)application-engine/src/main/java/com/netgrif/application/engine/startup/runner/DefaultRoleRunner.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/startup/runner/FunctionsCacheRunner.java(2 hunks)application-engine/src/main/java/com/netgrif/application/engine/startup/runner/SuperCreatorRunner.java(5 hunks)application-engine/src/main/java/com/netgrif/application/engine/workflow/service/CaseSearchService.java(2 hunks)application-engine/src/main/java/com/netgrif/application/engine/workflow/service/ConfigurableMenuService.java(2 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/java/com/netgrif/application/engine/workflow/web/WorkflowController.java(3 hunks)application-engine/src/main/resources/petriNets/engine-processes/preference_item.xml(1 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/action/AssignActionTest.groovy(2 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/auth/UserServiceTest.groovy(2 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/FunctionsTest.groovy(3 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/PetriNetTest.groovy(2 hunks)application-engine/src/test/java/com/netgrif/application/engine/petrinet/service/ProcessRoleServiceTest.java(3 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/Icon.java(1 hunks)nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/petrinet/service/ProcessRoleService.java(2 hunks)nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/utils/PaginationProperties.java(1 hunks)nae-user-ce/src/main/java/com/netgrif/application/engine/auth/service/AuthorityServiceImpl.java(3 hunks)nae-user-ce/src/main/java/com/netgrif/application/engine/auth/service/GroupServiceImpl.java(7 hunks)nae-user-ce/src/main/java/com/netgrif/application/engine/auth/service/UserServiceImpl.java(11 hunks)nae-user-common/src/main/java/com/netgrif/application/engine/auth/repository/AuthorityRepository.java(1 hunks)nae-user-common/src/main/java/com/netgrif/application/engine/auth/repository/GroupRepository.java(1 hunks)nae-user-common/src/main/java/com/netgrif/application/engine/auth/repository/UserRepository.java(1 hunks)nae-user-common/src/main/java/com/netgrif/application/engine/auth/service/AuthorityService.java(1 hunks)nae-user-common/src/main/java/com/netgrif/application/engine/auth/service/GroupService.java(2 hunks)nae-user-common/src/main/java/com/netgrif/application/engine/auth/service/UserService.java(2 hunks)nae-user-common/src/main/java/com/netgrif/application/engine/auth/web/requestbodies/UserSearchRequestBody.java(1 hunks)
💤 Files with no reviewable changes (3)
- application-engine/src/main/java/com/netgrif/application/engine/workflow/service/WorkflowService.java
- application-engine/src/main/java/com/netgrif/application/engine/workflow/service/interfaces/IWorkflowService.java
- application-engine/src/main/java/com/netgrif/application/engine/elastic/service/interfaces/IElasticPetriNetService.java
🧰 Additional context used
🧠 Learnings (2)
application-engine/src/main/java/com/netgrif/application/engine/workflow/service/ConfigurableMenuService.java (1)
Learnt from: renczesstefan
PR: netgrif/application-engine#318
File: nae-object-library/src/main/java/com/netgrif/application/engine/objects/workflow/domain/menu/MenuItemConstants.java:60-62
Timestamp: 2025-06-23T13:30:13.096Z
Learning: In MenuItemConstants enum in nae-object-library/src/main/java/com/netgrif/application/engine/objects/workflow/domain/menu/MenuItemConstants.java, the field `attributeId` will be renamed to `value` to make it more generic and appropriate for both dataset attribute identifiers and technical constants like PATH_SEPARATOR.
application-engine/src/main/resources/petriNets/engine-processes/preference_item.xml (1)
Learnt from: renczesstefan
PR: netgrif/application-engine#318
File: nae-object-library/src/main/java/com/netgrif/application/engine/objects/workflow/domain/menu/MenuItemConstants.java:60-62
Timestamp: 2025-06-23T13:30:13.096Z
Learning: In MenuItemConstants enum in nae-object-library/src/main/java/com/netgrif/application/engine/objects/workflow/domain/menu/MenuItemConstants.java, the field `attributeId` will be renamed to `value` to make it more generic and appropriate for both dataset attribute identifiers and technical constants like PATH_SEPARATOR.
🧬 Code Graph Analysis (1)
application-engine/src/main/java/com/netgrif/application/engine/startup/runner/SuperCreatorRunner.java (1)
application-engine/src/main/java/com/netgrif/application/engine/petrinet/service/ProcessRoleService.java (1)
ProcessRoleService(38-492)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: Build
🔇 Additional comments (62)
application-engine/src/main/java/com/netgrif/application/engine/startup/runner/DefaultFiltersRunner.java (1)
60-67: Good call switching to a mutableHashMap
createFiltermutatesfilterMetadataviaput, which would blow up with anUnsupportedOperationExceptionwhen fed an immutableMap.of(...). Wrapping the literal innew HashMap<>(…)fixes that runtime trap.Also applies to: 70-88
.gitignore (1)
211-211: Good addition to prevent tracking storage artifacts.Adding
/storage/to .gitignore is appropriate for the pagination implementation which may involve local storage or caching mechanisms.application-engine/src/main/java/com/netgrif/application/engine/startup/runner/FunctionsCacheRunner.java (1)
10-10: LGTM - Import added for pagination support.application-engine/src/main/java/com/netgrif/application/engine/workflow/service/ConfigurableMenuService.java (2)
21-21: LGTM - Import added for pagination support.
98-98: Correct usage of unpaged pagination for menu options.Using
Pageable.unpaged()is appropriate here since the method needs to retrieve all global roles to filter them for menu options.application-engine/src/main/java/com/netgrif/application/engine/startup/runner/AnonymousRoleRunner.java (1)
30-31: Improved efficiency by using single entity lookup.The change from
findAllByImportId()tofindByImportId()with null check is more efficient for checking the existence of a single anonymous role rather than retrieving a collection.application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/PetriNetTest.groovy (2)
21-21: LGTM - Import added for pagination support in tests.
92-92: Test correctly updated for new paginated API.The test properly adapts to the new API signature by adding
Pageable.unpaged()parameter, ensuring all references are retrieved for verification.application-engine/src/test/groovy/com/netgrif/application/engine/action/AssignActionTest.groovy (2)
25-25: LGTM! Import added for pagination support.The
Pageableimport is correctly added to support the updated repository method signatures.
152-153: LGTM! Pagination parameter correctly added to repository calls.The addition of
Pageable.ofSize(1)is appropriate since the test only needs the first matching role. This maintains the existing test logic while adapting to the new paginated repository API.nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/Icon.java (2)
6-7: LGTM! Serialization imports added.The imports for
SerialandSerializableare correctly added to support serialization.
11-14: LGTM! Proper serialization implementation.The class correctly implements
Serializablewith a properserialVersionUIDfield annotated with@Serial. This ensures version compatibility for serialization operations.application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/FunctionsTest.groovy (3)
22-22: LGTM! Import added for pagination support.The
Pageableimport is correctly added to support the updated service method signatures.
127-127: LGTM! Pagination parameter correctly added to service call.The addition of
Pageable.unpaged()is appropriate for this test scenario where all matching nets should be retrieved without pagination limits.
145-145: LGTM! Pagination parameter correctly added to service call.The addition of
Pageable.unpaged()is appropriate for this test scenario where all matching nets should be retrieved without pagination limits.application-engine/src/main/java/com/netgrif/application/engine/impersonation/service/ImpersonationAuthorizationService.java (1)
99-99: LGTM! Pagination parameter correctly added to service call.The addition of
Pageable.unpaged()is appropriate since this method needs to retrieve all authorities matching the given IDs without pagination limits. This maintains the existing behavior while adapting to the new paginated API.application-engine/src/test/groovy/com/netgrif/application/engine/auth/UserServiceTest.groovy (2)
106-106: LGTM! Pagination parameter correctly added to service call.The addition of
Pageable.unpaged()is appropriate for this test scenario where all matching users should be retrieved without pagination limits.
117-117: LGTM! Pagination parameter correctly added to service call.The addition of
Pageable.unpaged()is appropriate for this test scenario where all matching users should be retrieved without pagination limits.application-engine/src/main/java/com/netgrif/application/engine/startup/runner/DefaultRoleRunner.java (2)
32-33: LGTM! Improved efficiency with direct entity lookup.The change from collection-based existence checking to direct single-entity lookup is more efficient and aligns well with the broader pagination refactoring. Using
findByImportId()with a null check is cleaner than fetching all matching entities and checking for empty collections.
32-33: LGTM: Improved semantic clarity with single-entity lookup.The change from
findAllByImportId()tofindByImportId()and null checking is semantically more appropriate since import IDs should be unique. This aligns well with the broader pagination refactoring effort and eliminates unnecessary collection handling for what should be a single entity lookup.application-engine/src/main/java/com/netgrif/application/engine/workflow/service/CaseSearchService.java (2)
26-26: Correct import addition for pagination support.The import is appropriately added to support the pagination changes in the service method below.
26-26: LGTM: Added necessary import for pagination support.The import is correctly added to support the new paginated API.
application-engine/src/main/java/com/netgrif/application/engine/auth/web/UserController.java (1)
263-263: LGTM: Clean adaptation to paginated service APIThe change correctly adapts to the new paginated
AuthorityService.findAll()method while maintaining the original return type and behavior. UsingPageable.unpaged()preserves the "fetch all" semantics, and the stream conversion is efficient.application-engine/src/main/java/com/netgrif/application/engine/importer/service/Importer.java (1)
1064-1064: Ignore Optional handling suggestion:findByImportIdreturnsProcessRoleManual verification shows that
ProcessRoleService.findByImportId(String)is declared in the adapter interface to returnProcessRoleand its implementation delegates to the repository’sOptional<ProcessRole>via.orElse(null). There is no change to anOptional<ProcessRole>return type at the service level, so the existing calls inImporter.javacompile as written.Likely an incorrect or invalid review comment.
application-engine/src/test/java/com/netgrif/application/engine/petrinet/service/ProcessRoleServiceTest.java (1)
16-17: LGTM: Comprehensive test adaptation for paginationThe test methods are properly updated to work with the new paginated service API:
- Correct use of
Pageable.unpaged()to maintain existing test behavior- Proper adaptation of assertions using
getTotalElements()- Stream operations correctly adapted for
Pageobjects- Import statements appropriately added
The test logic and verification intent remain intact while working with the new API.
Also applies to: 58-66, 98-103, 108-113
nae-user-common/src/main/java/com/netgrif/application/engine/auth/web/requestbodies/UserSearchRequestBody.java (1)
7-36: Excellent documentation and multi-tenancy enhancementThe additions are well-implemented:
- New
realmIdfield: Properly supports multi-tenancy by allowing realm-scoped user searches- Comprehensive JavaDoc: Clear, professional documentation that explains the purpose and usage of each field
- Consistent documentation style: All fields are properly documented with their intended behavior
This enhances the API usability and supports the broader pagination and multi-tenancy improvements in the system.
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/utils/PaginationProperties.java (1)
21-28: LGTM! Well-documented configuration properties.The default values are reasonable and the documentation clearly explains the purpose of each property. The implementation follows Spring Boot configuration properties patterns correctly.
application-engine/src/main/java/com/netgrif/application/engine/elastic/domain/ElasticPetriNetRepository.java (1)
9-21: Excellent documentation improvements.The added Javadoc comments significantly improve code readability and maintainability. The documentation clearly explains the purpose and behavior of each method.
application-engine/src/main/java/com/netgrif/application/engine/petrinet/config/ProcessBeansConfiguration.java (2)
4-6: LGTM! Proper dependency imports added.The new imports for
PaginationPropertiesandRealmServiceare correctly added to support the updated bean configuration.
32-47: Excellent dependency injection implementation.The new dependencies are properly injected with consistent
@Lazyannotations and correctly passed to the constructor. This follows Spring best practices and maintains consistency with the existing code pattern.application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/PetriNetController.java (1)
127-136: Excellent pagination implementation improvements.The changes correctly improve pagination handling by:
- Passing
Pageableparameters directly to service methods (lines 128, 130, 135)- Returning
Pageobjects directly instead of wrapping inPageImpl- Properly handling the specific case where both identifier and version are provided (lines 132-133)
This approach preserves pagination metadata and is more efficient than the previous implementation.
nae-user-common/src/main/java/com/netgrif/application/engine/auth/repository/AuthorityRepository.java (4)
5-11: LGTM! Proper imports added for pagination support.The new imports for
Page,Pageable, andOptionalcorrectly support the enhanced method signatures and pagination functionality.
13-16: Excellent documentation addition.The Javadoc comments provide clear context about the repository's purpose and its relationship to the MongoDB database.
20-26: Great improvement using Optional return type.Changing
findByNameto returnOptional<Authority>instead ofAuthorityis a best practice that makes the API more robust by explicitly handling the case where no matching entity is found.
28-35: Excellent pagination support implementation.The new
findAllBy_idInmethod properly implements pagination by:
- Accepting a
Pageableparameter for pagination configuration- Returning
Page<Authority>to preserve pagination metadata- Providing clear documentation about its purpose and parameters
This follows Spring Data pagination best practices perfectly.
nae-user-common/src/main/java/com/netgrif/application/engine/auth/repository/GroupRepository.java (3)
10-10: LGTM! Collection import supports more flexible parameter types.The change from
SettoCollectionimport aligns with the method signature updates and provides more flexibility for callers.
27-27: Method signature change looks correct.The addition of
Pageableparameter and change toPage<Group>return type is consistent with the pagination implementation pattern.
37-44: Good improvement in method signature flexibility.The change from
Set<String>toCollection<String>parameter type provides more flexibility while maintaining type safety. The pagination support is properly implemented.application-engine/src/main/java/com/netgrif/application/engine/startup/runner/SuperCreatorRunner.java (1)
74-81: Fix potential concurrent modification and inefficient collection operations.The current implementation has several issues:
- Direct modification of
user.getProcessRoles()collection while iterating- Inefficient repeated
addAll()operations- No initial capacity estimation for the collection
- Pageable pageable = PageRequest.of(0, paginationProperties.getBackendPageSize()); - Page<ProcessRole> processRoles; - do { - processRoles = processRoleService.findAll(pageable); - user.getProcessRoles().addAll(processRoles.getContent()); - pageable = pageable.next(); - } while (processRoles.hasNext()); + Set<ProcessRole> allProcessRoles = new HashSet<>(); + Pageable pageable = PageRequest.of(0, paginationProperties.getBackendPageSize()); + Page<ProcessRole> processRoles; + do { + processRoles = processRoleService.findAll(pageable); + allProcessRoles.addAll(processRoles.getContent()); + pageable = pageable.next(); + } while (processRoles.hasNext()); + user.setProcessRoles(allProcessRoles);Likely an incorrect or invalid review comment.
application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticPetriNetService.java (1)
6-6: LGTM! Service properly decoupled from PetriNet domain logic.The removal of IPetriNetService dependency and related methods simplifies the service to focus solely on Elasticsearch operations. This is a good architectural decision that reduces coupling.
nae-user-common/src/main/java/com/netgrif/application/engine/auth/service/GroupService.java (2)
29-29: LGTM! Method signatures properly updated for pagination support.The changes from
Set<String>toCollection<String>parameter types and return types toPage<Group>are consistent with the pagination implementation pattern and provide better flexibility.Also applies to: 31-31
55-55: Method signature change is consistent with pagination pattern.The addition of
Pageableparameter and change toPage<String>return type follows the established pagination pattern in the codebase.nae-user-ce/src/main/java/com/netgrif/application/engine/auth/service/AuthorityServiceImpl.java (4)
9-10: LGTM! Proper imports for pagination support.The imports for
PageandPageableare necessary for the pagination implementation.
28-30: Method signature properly updated for pagination.The change to accept
Pageableparameter and returnPage<Authority>is consistent with the pagination pattern implemented across the codebase.
34-37: Good simplification of the getOrCreate method.The method is now cleaner and more focused on its primary purpose of finding or creating authorities by name.
45-47: Pagination implementation looks correct.The method properly accepts
Pageableparameter and the ObjectId conversion logic is appropriate for MongoDB operations.nae-user-common/src/main/java/com/netgrif/application/engine/auth/service/AuthorityService.java (1)
4-46: Well-structured pagination implementationThe refactoring to support pagination is clean and follows Spring Data best practices. The added Javadoc documentation is comprehensive and helpful. The removal of permission and role-specific methods suggests a good simplification of the interface.
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/petrinet/service/ProcessRoleService.java (1)
9-39: Consistent pagination support across service methodsThe pagination implementation is uniformly applied to all relevant query methods. The interface maintains a good balance between paginated and non-paginated methods based on their use cases.
application-engine/src/main/java/com/netgrif/application/engine/petrinet/domain/repositories/PetriNetRepository.java (1)
11-59: Clean repository refactoring with proper documentationThe repository interface has been effectively streamlined by removing unused methods and adding pagination support where needed. The comprehensive Javadoc documentation and use of Optional return types follow Spring Data best practices.
application-engine/src/main/java/com/netgrif/application/engine/workflow/web/WorkflowController.java (1)
105-105: Good catch on fixing the PageMetadata constructor parameter order!The parameter order correction from
(pageNumber, pageSize)to(pageSize, pageNumber)is an important bug fix that ensures pagination metadata is correctly represented in API responses.Also applies to: 116-116, 129-129, 140-140, 168-168
application-engine/src/main/java/com/netgrif/application/engine/petrinet/domain/roles/ProcessRoleRepository.java (1)
16-22: Excellent addition of comprehensive JavaDoc documentation!The detailed documentation for the repository interface and its methods greatly improves code maintainability and developer experience.
application-engine/src/main/java/com/netgrif/application/engine/petrinet/service/ProcessRoleService.java (1)
442-472: Well-implemented nested pagination for scalable role deletion!The refactored
deleteRolesOfNetmethod properly handles large datasets by using nested pagination loops for realms and users, preventing memory issues that could occur with the previous approach.nae-user-common/src/main/java/com/netgrif/application/engine/auth/repository/UserRepository.java (1)
41-71: Good defensive programming with clear exception messages!Throwing exceptions for unsupported multi-collection operations with helpful messages directing users to the correct methods prevents misuse of the repository interface.
application-engine/src/main/java/com/netgrif/application/engine/petrinet/service/interfaces/IPetriNetService.java (4)
27-337: Excellent documentation improvements!The comprehensive Javadoc additions significantly enhance the interface clarity and maintainability. The documentation follows standard conventions and provides clear descriptions for all methods, parameters, return values, and exceptions.
98-111: Good addition of parameterized import method.The new overloaded
importPetriNetmethod with additionalparamsparameter provides flexibility for passing extra configuration during import while maintaining backward compatibility.
270-337: Well-designed new methods enhance the interface.The newly added methods provide valuable functionality:
- Cache management for performance optimization
- Multiple retrieval options by ObjectId
- User-context aware deletion for audit trails
- Dynamic action execution capability
- Input validation for identifiers
- Workflow integration through case-to-PetriNet mapping
145-145: Ensure Pagination Changes Are Fully Applied and Consumers Updated
- No implementations of
IPetriNetServicewere detected viaclass … implements IPetriNetService—please confirm that all concrete service classes (e.g.PetriNetServiceImpl) have had their method signatures updated to:
Page<PetriNet> getByIdentifier(String identifier, Pageable pageable);Page<PetriNet> getAll(Pageable pageable);Page<PetriNetReference> getReferences(…, Pageable pageable);Page<PetriNetReference> getReferencesByIdentifier(String identifier, …, Pageable pageable);Page<PetriNetReference> getReferencesByVersion(Version version, …, Pageable pageable);- Controller and assembler calls to
getAll,getReferences*are already passing aPageableand handlingPage<T>. There were no direct calls togetByIdentifierfound—ensure any internal or downstream consumers of that method are updated to includePageableand processPage<PetriNet>.- Provide a migration guide for API consumers, detailing the signature changes and how to handle
Page<T>results.- Consider your API versioning strategy if backward compatibility is required.
nae-user-ce/src/main/java/com/netgrif/application/engine/auth/service/UserServiceImpl.java (5)
5-5: Good integration of centralized pagination configuration.The
PaginationPropertiesprovides a clean way to manage pagination settings. The@Lazyannotation on the setter suggests potential circular dependency - ensure this is documented if it's a known constraint.Also applies to: 60-61, 109-114
149-155: Clean pagination implementation for user retrieval.The method correctly implements pagination with proper type conversion using the
changeTypehelper. The single realm parameter simplifies the query scope appropriately.
254-262: Excellent implementation of paginated role loading.The do-while loop pattern for loading all process roles in pages prevents memory issues with large datasets. This approach is consistently applied in both
addAllRolesToAdminByUsernameandgetSystemmethods.Also applies to: 415-423
490-503: Robust implementation of role removal with pagination.The nested iteration pattern correctly handles multiple realms while preventing memory issues through pagination. The do-while loop ensures all affected users are processed.
623-639: Well-structured predicate building for realm-scoped queries.The updated
buildPredicatemethod correctly:
- Filters users by shared group membership for realm scoping
- Excludes the system user from results
- Handles optional query parameter gracefully
- Maintains case-insensitive search across multiple fields
| // String uriNodeId = elasticCaseService.findUriNodeId(useCase) | ||
| // def node = uriService.findById(uriNodeId) | ||
| // updateMultichoiceWithCurrentNode(moveDestUri, node) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Remove commented code or confirm feature deprecation.
The commented code disables dynamic URI node ID functionality that appears to be systematically deprecated across the codebase. Rather than leaving this dead code commented out, consider either:
- Completely removing these lines if the feature is permanently deprecated
- Implementing alternative functionality if this capability is still required
Please confirm whether the dynamic URI node updating feature is intentionally deprecated and whether these lines can be safely removed:
🏁 Script executed:
#!/bin/bash
# Description: Verify if URI node ID methods are still used anywhere in the codebase
# Expected: No usage should be found if this feature is fully deprecated
# Search for any remaining usage of URI node ID methods
rg -A 3 "findUriNodeId|findAllByUriNodeId|elasticCaseService\.findUriNodeId"
# Search for any URI service usage in XML files
rg -A 3 "uriService\.findById" --type xml
# Check for any updateMultichoiceWithCurrentNode usage
rg -A 3 "updateMultichoiceWithCurrentNode"Length of output: 300338
Remove deprecated URI node update snippet
The three commented lines in application-engine/src/main/resources/petriNets/engine-processes/preference_item.xml (around lines 150–152) reference elasticCaseService.findUriNodeId, uriService.findById and updateMultichoiceWithCurrentNode but no active code calls these methods elsewhere. They can be safely removed to eliminate dead code.
• File: application-engine/src/main/resources/petriNets/engine-processes/preference_item.xml
Remove lines:
// String uriNodeId = elasticCaseService.findUriNodeId(useCase)
// def node = uriService.findById(uriNodeId)
// updateMultichoiceWithCurrentNode(moveDestUri, node)🤖 Prompt for AI Agents
In
application-engine/src/main/resources/petriNets/engine-processes/preference_item.xml
around lines 150 to 152, remove the three commented lines referencing
elasticCaseService.findUriNodeId, uriService.findById, and
updateMultichoiceWithCurrentNode as they are deprecated and not called
elsewhere, eliminating dead code.
🛠️ Refactor suggestion
Clean up commented code or document deprecation timeline.
The commented-out code disables the dynamic update of the move_dest_uri field based on URI node IDs, which aligns with the systematic removal of URI node ID-based methods mentioned in the broader refactoring.
However, leaving commented code in production files is typically not recommended. Consider:
- If permanently deprecated: Remove the commented lines entirely
- If temporarily disabled: Add a clear comment explaining the timeline and reasoning
- If pending migration: Document the migration plan and target completion
-// String uriNodeId = elasticCaseService.findUriNodeId(useCase)
-// def node = uriService.findById(uriNodeId)
-// updateMultichoiceWithCurrentNode(moveDestUri, node)
+// URI node ID functionality deprecated as part of NAE-2122 pagination refactoring
+// Dynamic node updates temporarily disabled pending alternative implementationOr if permanently removed:
-// String uriNodeId = elasticCaseService.findUriNodeId(useCase)
-// def node = uriService.findById(uriNodeId)
-// updateMultichoiceWithCurrentNode(moveDestUri, node)📝 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.
| // String uriNodeId = elasticCaseService.findUriNodeId(useCase) | |
| // def node = uriService.findById(uriNodeId) | |
| // updateMultichoiceWithCurrentNode(moveDestUri, node) | |
| // URI node ID functionality deprecated as part of NAE-2122 pagination refactoring | |
| // Dynamic node updates temporarily disabled pending alternative implementation |
🤖 Prompt for AI Agents
In
application-engine/src/main/resources/petriNets/engine-processes/preference_item.xml
around lines 150 to 152, the commented-out code related to updating
move_dest_uri based on URI node IDs should be either fully removed if this
functionality is permanently deprecated, or if it is temporarily disabled, add a
clear comment explaining why it is disabled and when or under what conditions it
will be re-enabled or removed. If this is part of a migration, document the
migration plan and expected timeline in the comments to maintain clarity.
| Pageable pageable = PageRequest.of(0, paginationProperties.getBackendPageSize()); | ||
| Page<ProcessRole> processRoles; | ||
| do { | ||
| processRoles = processRoleService.findAll(pageable); | ||
| superUser.getProcessRoles().addAll(processRoles.getContent()); | ||
| pageable = pageable.next(); | ||
| } while (processRoles.hasNext()); | ||
|
|
There was a problem hiding this comment.
Apply the same fix for concurrent modification issues.
The same issues exist in the setAllProcessRoles method.
- Pageable pageable = PageRequest.of(0, paginationProperties.getBackendPageSize());
- Page<ProcessRole> processRoles;
- do {
- processRoles = processRoleService.findAll(pageable);
- superUser.getProcessRoles().addAll(processRoles.getContent());
- pageable = pageable.next();
- } while (processRoles.hasNext());
+ Set<ProcessRole> allProcessRoles = new HashSet<>();
+ Pageable pageable = PageRequest.of(0, paginationProperties.getBackendPageSize());
+ Page<ProcessRole> processRoles;
+ do {
+ processRoles = processRoleService.findAll(pageable);
+ allProcessRoles.addAll(processRoles.getContent());
+ pageable = pageable.next();
+ } while (processRoles.hasNext());
+ superUser.getProcessRoles().clear();
+ superUser.getProcessRoles().addAll(allProcessRoles);🤖 Prompt for AI Agents
In
application-engine/src/main/java/com/netgrif/application/engine/startup/runner/SuperCreatorRunner.java
around lines 104 to 111, the code adds process roles directly to the superUser's
collection inside a loop, which can cause concurrent modification issues. To fix
this, first collect all process roles in a temporary list during pagination,
then add them to superUser's process roles after the loop completes, avoiding
modification during iteration.
| Pageable pageable = PageRequest.of(0, paginationProperties.getBackendPageSize()); | ||
| Page<Group> userGroups; | ||
| do { | ||
| userGroups = groupRepository.findByOwnerId(user.getStringId(), pageable); | ||
|
|
||
| Optional<Group> group = userGroups.stream().filter(g -> g.getIdentifier().equals(user.getUsername())).findFirst(); | ||
| if (group.isPresent()) { | ||
| return group.get(); | ||
| } | ||
|
|
||
| pageable = pageable.next(); | ||
| } while (userGroups.hasNext()); | ||
| throw new ResourceNotFoundException(ResourceNotFoundExceptionCode.DEFAULT_USER_GROUP_NOT_FOUND, errorMessage); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider optimizing the pagination loop for better performance
The current implementation iterates through all pages to find a matching group. For users with many groups, this could be inefficient. Consider using a query that directly filters by both owner ID and identifier to avoid pagination altogether.
Replace the pagination loop with a direct query:
-Pageable pageable = PageRequest.of(0, paginationProperties.getBackendPageSize());
-Page<Group> userGroups;
-do {
- userGroups = groupRepository.findByOwnerId(user.getStringId(), pageable);
-
- Optional<Group> group = userGroups.stream().filter(g -> g.getIdentifier().equals(user.getUsername())).findFirst();
- if (group.isPresent()) {
- return group.get();
- }
-
- pageable = pageable.next();
-} while (userGroups.hasNext());
-throw new ResourceNotFoundException(ResourceNotFoundExceptionCode.DEFAULT_USER_GROUP_NOT_FOUND, errorMessage);
+Optional<Group> group = groupRepository.findByOwnerIdAndIdentifier(user.getStringId(), user.getUsername());
+return group.orElseThrow(() -> new ResourceNotFoundException(ResourceNotFoundExceptionCode.DEFAULT_USER_GROUP_NOT_FOUND, errorMessage));Note: This would require adding a new method findByOwnerIdAndIdentifier to the GroupRepository.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
nae-user-ce/src/main/java/com/netgrif/application/engine/auth/service/GroupServiceImpl.java
between lines 135 and 147, the current code uses a pagination loop to find a
group by owner ID and identifier, which is inefficient. To fix this, add a new
method findByOwnerIdAndIdentifier in GroupRepository that directly queries for a
group matching both criteria. Replace the pagination loop with a single call to
this new method and handle the optional result accordingly, throwing
ResourceNotFoundException if not found.
...on-engine/src/main/java/com/netgrif/application/engine/petrinet/service/PetriNetService.java
Show resolved
Hide resolved
...engine/src/main/java/com/netgrif/application/engine/petrinet/service/ProcessRoleService.java
Outdated
Show resolved
Hide resolved
| }) | ||
| public ResponseEntity<List<Authority>> getAllAuthorities() { | ||
| return ResponseEntity.ok(authorityService.findAll()); | ||
| return ResponseEntity.ok(authorityService.findAll(Pageable.unpaged()).stream().toList()); |
There was a problem hiding this comment.
Here may would be good idea to provide pageable property as request param.
There was a problem hiding this comment.
I agree, but currently it is not needed, because authority set will never be large enough. This would need more rework through all repos.
| public List<ProcessRole> saveAll(Iterable<ProcessRole> entities) { | ||
| return StreamSupport.stream(entities.spliterator(), false).map(processRole -> { | ||
| if (!processRole.isGlobal() || processRoleRepository.findAllByImportId(processRole.getImportId()).isEmpty()) { | ||
| if (!processRole.isGlobal() || processRoleRepository.findByImportId(processRole.getImportId()) == null) { |
There was a problem hiding this comment.
I think this returns Optional and then it will never be null. Please consider to replace it with Optional.isEmpty().
There was a problem hiding this comment.
Ah, sorry 😄 I just noticed while scrolling down that coderabbitai had the same comment.
| // All cases | ||
| createCaseFilter("All cases", "assignment", FILTER_VISIBILITY_PUBLIC, "", new ArrayList<String>(), | ||
| Map.of( | ||
| new HashMap<>(Map.of( |
There was a problem hiding this comment.
In this case we can use
new HashMap<>(){{
put("key1", "value1");
put("key2", "value2");
}};Map.of return immutable map, that cannot be modified later. But I think it is OK here, because you are not using this instance later, and its key value pair is copied to HashMap.
|
|
||
| public void setAllAuthorities() { | ||
| superUser.setAuthorities(Set.copyOf(authorityService.findAll())); | ||
| superUser.setAuthorities(Set.copyOf(authorityService.findAll(Pageable.unpaged()).stream().toList())); |
There was a problem hiding this comment.
I think this can be an issue, since Set.copyOf returns immutable set, so later when a super user is accessed through this runner, it will return object with authority set, that cannot be updated with new authorities, please consider use of new HashSet
There was a problem hiding this comment.
In this case, the object is immediately saved to DB and nothing works with authorities outside this one assign. When queried from DB, the object is returned with mutable set.
|
|
||
| // @Deprecated | ||
| // List<Field> getData(String caseId); | ||
|
|
There was a problem hiding this comment.
Kindly please, remove unnecessary comments.
There was a problem hiding this comment.
This is deletion of that :)
| */ | ||
| @Data | ||
| @Configuration | ||
| @ConfigurationProperties(prefix = "netgrif.pagination") |
There was a problem hiding this comment.
There is other PR opened to refactor configuration properties naming scheme. The new naming scheme is the following: netgrif.<app>.<property-domain>.<property-name>, so in this case it would be netgrif.engine.pagination. Please update it to use new naming convention.
| Pageable pageable = PageRequest.of(0, paginationProperties.getBackendPageSize()); | ||
| Page<ProcessRole> processRoles; | ||
| do { | ||
| processRoles = processRoleService.findAll(pageable); |
There was a problem hiding this comment.
Why are we using here do-while with restricted page size instead of unpaged and one call? Could it be a performance issue to use unpaged? If yes, then what is the situation in case when user holds too much roles and then it is saved to database?
- changes/fixes based on PR comments.
- Updated tests in `PetriNetTest.groovy` by introducing the `Page<PetriNetReference>` type for the `nets` variable to ensure type safety and clarity. - Imported `Page` and `PetriNetReference` and adjusted relevant usages in `PetriNetTest.groovy`. - Refined total result calculation logic in `PetriNetService` by transitioning the `total` value handling to use a `Number` type, avoiding potential null pointer exceptions. - Adopted safer numeric type conversions ensuring robust and error-free handling of MongoDB aggregation results. - Maintained backward compatibility while improving code readability and reliability.
Description
Refactor services and repositories to work with pagination and return pages.
Implements NAE-2122
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?
Tested manually in application.
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores