[NAE-2072] Workspaces #309
Conversation
- add workspace id to import - add workspace to tests
- change WorkspaceConstants to get workspace from user
# Conflicts: # application-engine/src/main/groovy/com/netgrif/application/engine/DevConsole.groovy # application-engine/src/main/groovy/com/netgrif/application/engine/migration/ActionMigration.groovy # application-engine/src/main/groovy/com/netgrif/application/engine/startup/ImportHelper.groovy # application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticPetriNetService.java # application-engine/src/main/java/com/netgrif/application/engine/startup/runner/AnonymousRoleRunner.java # application-engine/src/main/java/com/netgrif/application/engine/startup/runner/DefaultRoleRunner.java # application-engine/src/main/java/com/netgrif/application/engine/startup/runner/SuperCreatorRunner.java # application-engine/src/main/java/com/netgrif/application/engine/workflow/service/TaskSearchService.java # application-engine/src/test/groovy/com/netgrif/application/engine/action/AssignActionTest.groovy # application-engine/src/test/groovy/com/netgrif/application/engine/action/AssignRemoveTest.groovy # application-engine/src/test/groovy/com/netgrif/application/engine/action/RemoveActionTest.groovy # application-engine/src/test/groovy/com/netgrif/application/engine/auth/AuthenticationControllerTest.groovy # application-engine/src/test/groovy/com/netgrif/application/engine/auth/SecurityContextTest.groovy # application-engine/src/test/groovy/com/netgrif/application/engine/auth/TaskAuthorizationServiceTest.groovy # application-engine/src/test/groovy/com/netgrif/application/engine/auth/WorkflowAuthorizationServiceTest.groovy # application-engine/src/test/groovy/com/netgrif/application/engine/elastic/DataSearchRequestTest.groovy # application-engine/src/test/groovy/com/netgrif/application/engine/elastic/ElasticSearchTest.groovy # application-engine/src/test/groovy/com/netgrif/application/engine/elastic/ElasticTaskTest.groovy # application-engine/src/test/groovy/com/netgrif/application/engine/elastic/ReindexTest.groovy # application-engine/src/test/groovy/com/netgrif/application/engine/importer/PredefinedRolesPermissionsTest.groovy # application-engine/src/test/groovy/com/netgrif/application/engine/importer/UserListTest.groovy # application-engine/src/test/groovy/com/netgrif/application/engine/ipc/AssignCancelFinishWithCaseTest.groovy # application-engine/src/test/groovy/com/netgrif/application/engine/ipc/CaseApiTest.groovy # application-engine/src/test/groovy/com/netgrif/application/engine/ipc/TaskApiTest.groovy # application-engine/src/test/groovy/com/netgrif/application/engine/pdf/service/PdfGeneratorTest.groovy # application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/ActionRefTest.groovy # application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/ArcOrderTest.groovy # application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/EventTest.groovy # application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/FunctionsTest.groovy # application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/ImporterTest.groovy # application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/PetriNetTest.groovy # application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/dataset/CaseFieldTest.groovy # application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/dataset/ChangeCasePropertyTest.groovy # application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/dataset/ChangeFieldValueInitTest.groovy # application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/dataset/ChoiceFieldTest.groovy # application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/dataset/DynamicCaseNameTest.groovy # application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/dataset/DynamicChoicesTest.groovy # application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/dataset/DynamicDefaultValueTest.groovy # application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/dataset/DynamicEnumerationTest.groovy # application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/dataset/DynamicValidationPerformanceTest.groovy # application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/dataset/DynamicValidationTest.groovy # application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/dataset/FileFieldTest.groovy # application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/dataset/FileListFieldTest.groovy # application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/dataset/MapFieldTest.groovy # application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/roles/ProcessRoleTest.groovy # application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/web/PetriNetControllerTest.groovy # application-engine/src/test/java/com/netgrif/application/engine/importer/ImporterTest.java # application-engine/src/test/java/com/netgrif/application/engine/workflow/service/TaskServiceTest.java # application-engine/src/test/java/com/netgrif/application/engine/workflow/web/VariableArcsTest.java
- add changes from netgrif-modules
- add context holder - add functions for default roles
# Conflicts: # application-engine/src/main/groovy/com/netgrif/application/engine/petrinet/domain/dataset/logic/action/ActionDelegate.groovy # application-engine/src/main/groovy/com/netgrif/application/engine/petrinet/domain/dataset/logic/action/FieldActionsRunner.groovy # application-engine/src/main/java/com/netgrif/application/engine/auth/web/UserController.java # application-engine/src/main/java/com/netgrif/application/engine/auth/web/responsebodies/User.java # application-engine/src/main/java/com/netgrif/application/engine/startup/runner/AnonymousRoleRunner.java # application-engine/src/main/java/com/netgrif/application/engine/startup/runner/DefaultRoleRunner.java # nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/auth/domain/LoggedUserImpl.java # nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/petrinet/service/ProcessRoleService.java # nae-user-ce/src/main/java/com/netgrif/application/engine/auth/service/UserServiceImpl.java
- some rework
# Conflicts: # application-engine/src/main/java/com/netgrif/application/engine/auth/web/UserController.java # application-engine/src/main/java/com/netgrif/application/engine/petrinet/service/PetriNetService.java # nae-user-common/src/main/java/com/netgrif/application/engine/auth/web/responsebodies/User.java
- deleted commented files
- rweork of import default processes
# Conflicts: # application-engine/src/main/java/com/netgrif/application/engine/auth/web/UserController.java
- after merge
- remove function
- add workspaceId to actions as parameter to funcions
Kovy95
added
breaking change
WalkthroughThis change introduces workspace isolation throughout the application engine. All major domain entities (users, Petri nets, cases, tasks, roles) and their repositories are updated to include a Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Controller
participant Service
participant Repository
User->>Controller: Import Petri Net (workspaceId)
Controller->>Service: importPetriNet(xml, ..., workspaceId)
Service->>Repository: save PetriNet (workspaceId)
Repository-->>Service: PetriNet persisted
Service-->>Controller: Import result
Controller-->>User: Response
sequenceDiagram
participant User
participant AuthProvider
participant WorkspaceService
User->>AuthProvider: Authenticate (credentials)
AuthProvider->>WorkspaceService: getDefaultWorkspace()
WorkspaceService-->>AuthProvider: Workspace (workspaceId)
AuthProvider->>User: Authenticated user (workspaceId assigned)
Suggested labels
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
- fix problem with cases and tasks
# Conflicts: # application-engine/src/main/resources/application.properties
- rework fo async runner - add all default roles to new users - fix predicates in task and workflow services
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 38
🔭 Outside diff range comments (3)
application-engine/src/main/java/com/netgrif/application/engine/petrinet/domain/repositories/PetriNetRepository.java (1)
15-17: 💡 Verification agent🧩 Analysis chain
Verify workspace isolation for legacy query methods.
These methods (
findByTitle_DefaultValue,findByImportId,deleteBy_id) lack workspace scoping, which could potentially allow cross-workspace data access.Also applies to: 33-33
🏁 Script executed:
#!/bin/bash # Description: Check if these legacy methods are used in workspace-aware contexts # Search for usage of non-workspace methods rg -A 5 -B 2 "findByTitle_DefaultValue|findByImportId|deleteBy_id"Length of output: 76041
🏁 Script executed:
#!/bin/bash set -e # Locate the PetriNetRepository file file=$(rg -l "interface PetriNetRepository" | head -n1) echo "Inspecting $file" echo "---- File content (1-200) ----" sed -n '1,200p' "$file" echo "---- Checking for workspace references ----" grep -R "workspace" -n "$file" || echo "No workspace-related fields in repository interface."Length of output: 3010
Enforce workspace isolation on legacy PetriNetRepository methods
Legacy repository methods
findByTitle_DefaultValue(String),findByImportId(String), anddeleteBy_id(ObjectId)are not scoped byworkspaceId, which may allow cross-workspace data access. Please deprecate or remove them and introduce workspace-aware replacements:• Deprecate with
@Deprecatedand add JavaDoc pointing to the new API.
• Add workspace-scoped variants, e.g.:
List<PetriNet> findByTitle_DefaultValueAndWorkspaceId(String title, String workspaceId)PetriNet findByImportIdAndWorkspaceId(String id, String workspaceId)void deleteBy_idAndWorkspaceId(ObjectId id, String workspaceId)
• Update any service or controller calls to use the new methods.Locations needing attention:
- application-engine/src/main/java/com/netgrif/application/engine/petrinet/domain/repositories/PetriNetRepository.java
- Lines 15–17:
findByTitle_DefaultValue,findByImportId- Line 33:
deleteBy_idapplication-engine/src/main/java/com/netgrif/application/engine/petrinet/service/ProcessRoleService.java (1)
49-50:⚠️ Potential issueCritical: Cache fields are not workspace-aware and may cause cross-workspace data leakage.
The
defaultRoleandanonymousRoleinstance fields are cached globally but the methods now retrieve workspace-specific roles. This will cause the first workspace's roles to be returned for all subsequent workspaces.Apply this diff to make the cache workspace-aware:
- private ProcessRole defaultRole; - private ProcessRole anonymousRole; + private final Map<String, ProcessRole> defaultRoleCache = new ConcurrentHashMap<>(); + private final Map<String, ProcessRole> anonymousRoleCache = new ConcurrentHashMap<>();And update the getter methods:
public ProcessRole defaultRole(String workspaceId) { - if (defaultRole == null) { + return defaultRoleCache.computeIfAbsent(workspaceId, (wsId) -> { Set<ProcessRole> roles = processRoleRepository.findAllByImportIdAndWorkspaceId(ProcessRole.DEFAULT_ROLE, workspaceId); if (roles.isEmpty()) throw new IllegalStateException("No default process role has been found!"); if (roles.size() > 1) throw new IllegalStateException("More than 1 default process role exists!"); - defaultRole = roles.stream().findFirst().orElse(null); - } - return defaultRole; + return roles.stream().findFirst().orElse(null); + }); }Similarly for
anonymousRole(String workspaceId).Also update the
clearCache()method:public void clearCache() { - this.defaultRole = null; - this.anonymousRole = null; + this.defaultRoleCache.clear(); + this.anonymousRoleCache.clear(); }Don't forget to add the import:
+import java.util.concurrent.ConcurrentHashMap;Also applies to: 378-399
application-engine/src/main/java/com/netgrif/application/engine/petrinet/service/PetriNetService.java (1)
368-378:⚠️ Potential issueCache key should include workspace ID.
The
@Cacheableannotation on line 371 doesn't include workspace ID in the cache key, which could lead to incorrect data being returned across workspaces.Update the cache key:
-@Cacheable(value = "petriNetNewest", unless = "#result == null") +@Cacheable(value = "petriNetNewest", key = "#identifier + '_' + #workspaceId", unless = "#result == null") public PetriNet getNewestVersionByIdentifier(String identifier, String workspaceId) {
🧹 Nitpick comments (22)
application-engine/src/main/java/com/netgrif/application/engine/petrinet/config/DefaultProcessResource.java (2)
11-16: Remove redundant @propertysource and reconsider bean scopeSpring Boot auto-loads
application.properties, so@PropertySource("classpath:application.properties")is unnecessary. Configuration-properties beans are also typically singletons; using prototype scope may lead to unexpected lifecycle behaviors. Consider:
- Removing
@PropertySource- Omitting
@Scope(default singleton) or explicitly usingSCOPE_SINGLETON
18-18: Prefer List over Resource[]Using
List<Resource>provides greater flexibility (validation, transformations, immutability). For example:private List<Resource> defaultProcesses;Adjust getter/setter accordingly and consider adding
@NotEmptyor similar validation.application-engine/src/main/java/com/netgrif/application/engine/auth/service/UserResourceHelperService.java (1)
34-34: Good workspace context propagation in user resources.Setting the workspace ID on the user result ensures proper workspace isolation in user resource responses. This change correctly propagates the workspace context from the authentication layer to the resource layer.
Consider whether this workspace ID assignment should be moved to the
UserFactory.getUser()method to ensure consistency across all user object creation:// In UserFactory public User getUser(IUser user, Locale locale) { User result = // ... existing user creation logic if (WorkspaceContextHolder.getWorkspaceId() != null) { result.setWorkspaceId(WorkspaceContextHolder.getWorkspaceId()); } return result; }This would ensure workspace ID is set consistently wherever User objects are created, not just in this specific service method.
application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/ActionRefTest.groovy (1)
4-4: Remove redundant import.The import of
PetriNetServiceappears to be redundant sinceIPetriNetServiceis already imported (line 46) and the field is declared using the interface type.-import com.netgrif.application.engine.petrinet.service.PetriNetServiceapplication-engine/src/main/java/com/netgrif/application/engine/startup/runner/SuperCreatorRunner.java (1)
78-78: Consider refactoring to reduce workspace ID assignment duplication.The workspace ID assignment is correct but occurs in multiple places (lines 78 and 104), which could lead to maintenance issues.
Consider extracting workspace ID assignment to a private method:
+ private void ensureWorkspaceId() { + this.superUser.setWorkspaceId(defaultWorkspaceService.getDefaultWorkspace().getId()); + } + private IUser createSuperUser() { // ... existing code ... - this.superUser.setWorkspaceId(defaultWorkspaceService.getDefaultWorkspace().getId()); + ensureWorkspaceId(); return this.superUser; } public LoggedUser getLoggedSuper() { - this.superUser.setWorkspaceId(defaultWorkspaceService.getDefaultWorkspace().getId()); + ensureWorkspaceId(); return userService.transformToLoggedUser(superUser); }nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/ElasticCase.java (1)
90-90: Consider adding null safety checks for workspace ID.The workspace ID initialization and updates are correctly implemented. However, consider adding null checks to handle cases where the workspace ID might be null.
- workspaceId = useCase.getWorkspaceId(); + workspaceId = useCase.getWorkspaceId() != null ? useCase.getWorkspaceId() : "";And similarly for the update method:
- workspaceId = useCase.getWorkspaceId(); + workspaceId = useCase.getWorkspaceId() != null ? useCase.getWorkspaceId() : "";Also applies to: 121-121
application-engine/src/main/java/com/netgrif/application/engine/configuration/security/jwt/JwtService.java (1)
86-86: Add null safety check for workspace service call.The workspace ID assignment integrates workspace context into the JWT authentication flow correctly. However, consider adding a null check to handle potential null responses from the workspace service.
- user.setWorkspaceId(defaultWorkspaceService.getDefaultWorkspace().getId()); + var defaultWorkspace = defaultWorkspaceService.getDefaultWorkspace(); + if (defaultWorkspace != null) { + user.setWorkspaceId(defaultWorkspace.getId()); + }application-engine/src/main/java/com/netgrif/application/engine/configuration/authentication/providers/ldap/NetgrifLdapAuthenticationProvider.java (1)
44-45: Consider using constructor injection instead of field injection.Field injection makes testing more difficult and is generally discouraged in favor of constructor injection.
Consider moving this to constructor injection:
- @Autowired - protected DefaultWorkspaceService defaultWorkspaceService;And add it to the constructor parameters:
- public NetgrifLdapAuthenticationProvider(NaeLdapProperties properties) { + public NetgrifLdapAuthenticationProvider(NaeLdapProperties properties, DefaultWorkspaceService defaultWorkspaceService) { this.ldapProperties = properties; + this.defaultWorkspaceService = defaultWorkspaceService; // ... rest of constructor }application-engine/src/main/java/com/netgrif/application/engine/startup/runner/DefaultNetImportRunner.java (1)
22-24: Consider adding exception handling for startup reliability.If
importDefaultProcessesfails, it could prevent the application from starting up properly. Consider adding try-catch to log errors and continue startup.@Override public void run(ApplicationArguments args) throws Exception { - petriNetService.importDefaultProcesses(defaultWorkspaceService.getDefaultWorkspace().getId()); + try { + petriNetService.importDefaultProcesses(defaultWorkspaceService.getDefaultWorkspace().getId()); + log.info("Default processes imported successfully"); + } catch (Exception e) { + log.error("Failed to import default processes", e); + // Depending on requirements, you might want to rethrow or continue + throw e; + } }application-engine/src/main/java/com/netgrif/application/engine/auth/service/RegistrationService.java (1)
139-143: Approve workspace-aware default role assignment with performance consideration.The logic correctly assigns default roles for all workspaces instead of a single default role, which aligns with the workspace isolation feature. The implementation properly retrieves existing roles, adds workspace-specific defaults, and updates the user.
Consider the performance impact if there are many workspaces, as this will create more role assignments per user.
application-engine/src/test/java/com/netgrif/application/engine/importer/ImporterTest.java (1)
65-65: Consider extracting the workspace ID to reduce repetition.The workspace ID is retrieved multiple times using
superCreator.getLoggedSuper().getWorkspaceId(). Consider extracting this to a variable in the@BeforeEachmethod or within each test method to improve readability and potentially avoid multiple method calls.@BeforeEach public void before() { testHelper.truncateDbs(); + // Could add workspace ID extraction here if needed across multiple tests } @Test public void importPetriNet() throws MissingPetriNetMetaDataException, IOException, MissingIconKeyException { + String workspaceId = superCreator.getLoggedSuper().getWorkspaceId(); - petriNetService.importPetriNet(new FileInputStream("src/test/resources/prikladFM_test.xml"), VersionType.MAJOR, superCreator.getLoggedSuper(), superCreator.getLoggedSuper().getWorkspaceId()); + petriNetService.importPetriNet(new FileInputStream("src/test/resources/prikladFM_test.xml"), VersionType.MAJOR, superCreator.getLoggedSuper(), workspaceId); assertNetProperlyImported(); }Also applies to: 71-71, 81-81, 88-88, 93-93, 105-105
application-engine/src/main/groovy/com/netgrif/application/engine/DevConsole.groovy (1)
26-26: Consider security implications for dev profile endpoints.Since this controller is only active in the "dev" profile, ensure that these endpoints are properly secured and not exposed in production environments. The workspace filtering adds an additional layer of data isolation which is good.
application-engine/src/main/java/com/netgrif/application/engine/petrinet/service/workspace/DefaultWorkspaceServiceImpl.java (4)
5-5: Remove unused logging annotation.The
@Slf4jannotation is present but no logging is used in this class, making it unnecessary overhead.-@Slf4j public class DefaultWorkspaceServiceImpl implements DefaultWorkspaceService {
9-10: Add class-level documentation.The class lacks JavaDoc documentation explaining its purpose, usage, and relationship to the workspace isolation feature mentioned in the PR objectives.
+/** + * Default implementation of DefaultWorkspaceService that provides a singleton + * default workspace instance for workspace isolation throughout the application. + * This service is used when no specific workspace context is available. + */ @Slf4j public class DefaultWorkspaceServiceImpl implements DefaultWorkspaceService {
14-18: Consider defensive programming practices.The constructor directly modifies a newly created Workspace object. Consider adding null checks and error handling to make the implementation more robust.
public DefaultWorkspaceServiceImpl() { - this.defaultWorkspace = new Workspace(); - this.defaultWorkspace.setId(DefaultWorkspaceService.DEFAULT_WORKSPACE_ID); - this.defaultWorkspace.setDefaultWorkspace(true); + try { + this.defaultWorkspace = new Workspace(); + this.defaultWorkspace.setId(DefaultWorkspaceService.DEFAULT_WORKSPACE_ID); + this.defaultWorkspace.setDefaultWorkspace(true); + log.debug("Default workspace initialized with ID: {}", DefaultWorkspaceService.DEFAULT_WORKSPACE_ID); + } catch (Exception e) { + log.error("Failed to initialize default workspace", e); + throw new IllegalStateException("Cannot initialize default workspace", e); + } }
25-28: Consider workspace expansion capability.The method name
getAllWorkspaces()suggests it might return multiple workspaces in the future, but currently only returns the default workspace. Consider documenting this limitation or designing for future extensibility.+/** + * Returns all available workspaces. Currently only returns the default workspace. + * This method is designed to be extended when multiple workspace support is added. + * + * @return immutable list containing all workspaces + */ @Override public List<Workspace> getAllWorkspaces() { return List.of(defaultWorkspace); }application-engine/src/main/groovy/com/netgrif/application/engine/petrinet/domain/dataset/logic/action/ActionDelegate.groovy (1)
64-64: Remove unused import.The
BooleanBuilderimport doesn't appear to be used anywhere in this file and should be removed to keep the imports clean.-import com.querydsl.core.BooleanBuilderapplication-engine/src/main/java/com/netgrif/application/engine/workflow/domain/repositories/TaskRepository.java (1)
6-6: Remove unused import.The
Predicateimport is not directly used in this interface. The interface already has access toPredicatethrough theQuerydslPredicateExecutorinheritance.Apply this diff to remove the unused import:
-import com.querydsl.core.types.Predicate;application-engine/src/main/java/com/netgrif/application/engine/petrinet/service/ProcessRoleService.java (1)
423-441: Use batch operations for better performance.The method updates users one by one in a loop, which can be inefficient for many users.
Consider batching user updates for better performance:
List<IUser> usersWithRemovedRoles = this.userService.findAllByProcessRoles(new HashSet<>(roleToDelete), null); +List<IUser> usersToUpdate = new ArrayList<>(); for (IUser user : usersWithRemovedRoles) { if (user.getProcessRoles().isEmpty()) continue; Set<ProcessResourceId> newRoles = user.getProcessRoles().stream() .filter(role -> !deletedRoleStringId.contains(role.getStringId())) .map(ProcessRole::get_id) .collect(Collectors.toSet()); - this.assignRolesToUser(user, newRoles, userService.getLoggedOrSystem().transformToLoggedUser()); + user.setProcessRoles(processRoleRepository.findAllByCompositeId( + newRoles.stream().map(ProcessResourceId::getStringId).collect(Collectors.toList()) + ).stream().collect(Collectors.toSet())); + usersToUpdate.add(user); } + +// Batch save all users +if (!usersToUpdate.isEmpty()) { + userService.saveAll(usersToUpdate); + // Reload security context for logged user if affected + LoggedUser loggedUser = userService.getLoggedOrSystem().transformToLoggedUser(); + if (usersToUpdate.stream().anyMatch(u -> u.getStringId().equals(loggedUser.getId()))) { + securityContextService.reloadSecurityContext(loggedUser); + } +} this.processRoleRepository.deleteAllBy_idIn(roleToDelete);Note: This assumes
userService.saveAll()method exists. If not, consider implementing it.application-engine/src/main/java/com/netgrif/application/engine/petrinet/service/interfaces/IPetriNetService.java (2)
58-60: Add JavaDoc documentation for new workspace methods.The new methods lack documentation explaining their purpose and behavior.
Add documentation:
+ /** + * Imports default processes into the specified workspace. + * @param workspaceId the workspace to import processes into + * @return list of imported PetriNet instances + */ List<PetriNet> importDefaultProcesses(String workspaceId); + /** + * Asynchronously deletes all processes associated with the specified workspace. + * @param workspaceId the workspace whose processes should be deleted + */ void deleteWorkspaceProcesses(String workspaceId);
45-56: Consider using a builder pattern or parameter object for import methods.The import methods have many parameters which makes them hard to use and maintain. Consider refactoring to use a parameter object.
Create an import parameter object:
public class PetriNetImportParams { private final InputStream xmlFile; private final VersionType releaseType; private final LoggedUser user; private final String workspaceId; private final String uriNodeId; private final Map<String, String> params; // Builder pattern implementation... }Then simplify the interface:
ImportPetriNetEventOutcome importPetriNet(PetriNetImportParams params) throws IOException, MissingPetriNetMetaDataException, MissingIconKeyException;application-engine/src/main/java/com/netgrif/application/engine/petrinet/service/PetriNetService.java (1)
584-585: Fix inconsistent use of isPresent().The code uses deprecated
!isPresent()instead ofisEmpty().-if (!petriNetOptional.isPresent()) { +if (petriNetOptional.isEmpty()) { throw new IllegalArgumentException("Could not find process with id [" + processId + "]"); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (107)
application-engine/src/main/groovy/com/netgrif/application/engine/AsyncRunnerWrapper.java(1 hunks)application-engine/src/main/groovy/com/netgrif/application/engine/DevConsole.groovy(2 hunks)application-engine/src/main/groovy/com/netgrif/application/engine/migration/ActionMigration.groovy(1 hunks)application-engine/src/main/groovy/com/netgrif/application/engine/petrinet/domain/dataset/logic/action/ActionDelegate.groovy(4 hunks)application-engine/src/main/groovy/com/netgrif/application/engine/petrinet/domain/dataset/logic/action/FieldActionsRunner.groovy(3 hunks)application-engine/src/main/groovy/com/netgrif/application/engine/petrinet/domain/dataset/logic/action/runner/CaseFieldsExpressionRunner.groovy(2 hunks)application-engine/src/main/groovy/com/netgrif/application/engine/petrinet/domain/dataset/logic/action/runner/RoleActionsRunner.groovy(2 hunks)application-engine/src/main/groovy/com/netgrif/application/engine/startup/ImportHelper.groovy(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/service/UserResourceHelperService.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/auth/web/UserController.java(4 hunks)application-engine/src/main/java/com/netgrif/application/engine/auth/web/requestbodies/UpdateUserRequest.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/auth/web/responsebodies/AuthoritiesResources.java(0 hunks)application-engine/src/main/java/com/netgrif/application/engine/auth/web/responsebodies/IProcessRoleFactory.java(0 hunks)application-engine/src/main/java/com/netgrif/application/engine/auth/web/responsebodies/IUserFactory.java(0 hunks)application-engine/src/main/java/com/netgrif/application/engine/auth/web/responsebodies/ProcessRole.java(0 hunks)application-engine/src/main/java/com/netgrif/application/engine/auth/web/responsebodies/User.java(0 hunks)application-engine/src/main/java/com/netgrif/application/engine/auth/web/responsebodies/UserFactory.java(0 hunks)application-engine/src/main/java/com/netgrif/application/engine/auth/web/responsebodies/WorkspaceResponse.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/configuration/authentication/providers/basic/NetgrifBasicAuthenticationProvider.java(3 hunks)application-engine/src/main/java/com/netgrif/application/engine/configuration/authentication/providers/ldap/NetgrifLdapAuthenticationProvider.java(3 hunks)application-engine/src/main/java/com/netgrif/application/engine/configuration/security/jwt/JwtService.java(3 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/ElasticCaseService.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticPetriNetService.java(3 hunks)application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticTaskService.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticViewPermissionService.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/export/service/ExportService.java(3 hunks)application-engine/src/main/java/com/netgrif/application/engine/impersonation/service/ImpersonationAuthorizationService.java(3 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/DefaultProcessResource.java(1 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(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/petrinet/service/PetriNetService.java(13 hunks)application-engine/src/main/java/com/netgrif/application/engine/petrinet/service/ProcessRoleService.java(6 hunks)application-engine/src/main/java/com/netgrif/application/engine/petrinet/service/interfaces/IPetriNetService.java(2 hunks)application-engine/src/main/java/com/netgrif/application/engine/petrinet/service/workspace/DefaultWorkspaceConfiguration.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/petrinet/service/workspace/DefaultWorkspaceServiceImpl.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/PetriNetController.java(5 hunks)application-engine/src/main/java/com/netgrif/application/engine/security/service/SecurityContextService.java(4 hunks)application-engine/src/main/java/com/netgrif/application/engine/startup/runner/AnonymousRoleRunner.java(2 hunks)application-engine/src/main/java/com/netgrif/application/engine/startup/runner/DashboardRunner.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/startup/runner/DefaultNetImportRunner.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/startup/runner/DefaultRoleRunner.java(2 hunks)application-engine/src/main/java/com/netgrif/application/engine/startup/runner/FilterRunner.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/startup/runner/ImpersonationRunner.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/startup/runner/SuperCreatorRunner.java(4 hunks)application-engine/src/main/java/com/netgrif/application/engine/workflow/domain/repositories/CaseRepository.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/workflow/domain/repositories/TaskRepository.java(2 hunks)application-engine/src/main/java/com/netgrif/application/engine/workflow/service/CaseSearchService.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/workflow/service/DataService.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/workflow/service/EventService.java(3 hunks)application-engine/src/main/java/com/netgrif/application/engine/workflow/service/TaskSearchService.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/workflow/service/TaskService.java(4 hunks)application-engine/src/main/java/com/netgrif/application/engine/workflow/service/WorkflowService.java(5 hunks)application-engine/src/main/java/com/netgrif/application/engine/workflow/service/interfaces/IEventService.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/workflow/web/responsebodies/Task.java(3 hunks)application-engine/src/main/resources/application.properties(1 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/action/AssignActionTest.groovy(1 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/action/AssignRemoveTest.groovy(1 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/action/RemoveActionTest.groovy(1 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/auth/AuthenticationControllerTest.groovy(1 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/auth/SecurityContextTest.groovy(1 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/auth/TaskAuthorizationServiceTest.groovy(1 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/auth/WorkflowAuthorizationServiceTest.groovy(1 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/elastic/DataSearchRequestTest.groovy(1 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/elastic/ElasticSearchTest.groovy(1 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/elastic/ElasticTaskTest.groovy(1 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/elastic/ReindexTest.groovy(1 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/importer/PredefinedRolesPermissionsTest.groovy(1 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/importer/UserListTest.groovy(1 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/ipc/AssignCancelFinishWithCaseTest.groovy(1 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/ipc/CaseApiTest.groovy(2 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/ipc/TaskApiTest.groovy(6 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/pdf/service/PdfGeneratorTest.groovy(13 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/ActionRefTest.groovy(2 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/ArcOrderTest.groovy(1 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/ArcReferenceTest.groovy(2 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/EventTest.groovy(1 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/FunctionsTest.groovy(10 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/ImporterTest.groovy(7 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/PetriNetTest.groovy(2 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/dataset/CaseFieldTest.groovy(2 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/dataset/ChangeCasePropertyTest.groovy(1 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/dataset/ChangeFieldValueInitTest.groovy(1 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/dataset/ChoiceFieldTest.groovy(1 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/dataset/DynamicCaseNameTest.groovy(1 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/dataset/DynamicChoicesTest.groovy(1 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/dataset/DynamicDefaultValueTest.groovy(1 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/dataset/DynamicEnumerationTest.groovy(1 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/dataset/DynamicValidationPerformanceTest.groovy(1 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/dataset/DynamicValidationTest.groovy(1 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/dataset/FieldTest.groovy(2 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/dataset/FileFieldTest.groovy(1 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/dataset/FileListFieldTest.groovy(1 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/dataset/MapFieldTest.groovy(3 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/roles/ProcessRoleTest.groovy(1 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/service/PetriNetServiceTest.groovy(1 hunks)application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/web/PetriNetControllerTest.groovy(1 hunks)application-engine/src/test/java/com/netgrif/application/engine/importer/ConstructorAndDestructorTest.java(1 hunks)application-engine/src/test/java/com/netgrif/application/engine/importer/ImporterTest.java(4 hunks)application-engine/src/test/java/com/netgrif/application/engine/petrinet/service/ProcessRoleServiceTest.java(4 hunks)application-engine/src/test/java/com/netgrif/application/engine/workflow/service/TaskServiceTest.java(1 hunks)application-engine/src/test/java/com/netgrif/application/engine/workflow/web/VariableArcsTest.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/auth/domain/IUser.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/auth/domain/LoggedUser.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/ElasticCase.java(3 hunks)
⛔ Files not processed due to max files limit (26)
- nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/ElasticPetriNet.java
- nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/ElasticTask.java
- nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/PetriNet.java
- nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/Transition.java
- nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/roles/PredefinedProcessRole.java
- nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/roles/ProcessRole.java
- nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/workspace/DefaultWorkspaceService.java
- nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/workspace/Workspace.java
- nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/workspace/WorkspaceRequest.java
- nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/workspace/WorkspaceService.java
- nae-object-library/src/main/java/com/netgrif/application/engine/objects/workflow/domain/Case.java
- nae-object-library/src/main/java/com/netgrif/application/engine/objects/workflow/domain/Task.java
- nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/auth/domain/AnonymousUser.java
- nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/auth/domain/LoggedUserImpl.java
- nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/auth/domain/User.java
- nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/auth/domain/mapper/LoggedUserMapper.java
- nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/ElasticCase.java
- nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/ElasticPetriNet.java
- nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/ElasticTask.java
- nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/petrinet/service/ProcessRoleService.java
- nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/workflow/domain/Task.java
- nae-user-ce/src/main/java/com/netgrif/application/engine/auth/config/WorkspaceContextHolder.java
- nae-user-ce/src/main/java/com/netgrif/application/engine/auth/service/UserFactoryImpl.java
- nae-user-ce/src/main/java/com/netgrif/application/engine/auth/service/UserServiceImpl.java
- nae-user-common/src/main/java/com/netgrif/application/engine/auth/service/UserService.java
- nae-user-common/src/main/java/com/netgrif/application/engine/auth/web/responsebodies/User.java
💤 Files with no reviewable changes (6)
- application-engine/src/main/java/com/netgrif/application/engine/auth/web/responsebodies/IUserFactory.java
- application-engine/src/main/java/com/netgrif/application/engine/auth/web/responsebodies/AuthoritiesResources.java
- application-engine/src/main/java/com/netgrif/application/engine/auth/web/responsebodies/UserFactory.java
- application-engine/src/main/java/com/netgrif/application/engine/auth/web/responsebodies/IProcessRoleFactory.java
- application-engine/src/main/java/com/netgrif/application/engine/auth/web/responsebodies/ProcessRole.java
- application-engine/src/main/java/com/netgrif/application/engine/auth/web/responsebodies/User.java
🧰 Additional context used
🧬 Code Graph Analysis (2)
application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/PetriNetController.java (1)
application-engine/src/main/java/com/netgrif/application/engine/petrinet/service/ProcessRoleService.java (1)
ProcessRoleService(36-498)
application-engine/src/main/java/com/netgrif/application/engine/auth/web/UserController.java (1)
application-engine/src/main/java/com/netgrif/application/engine/workflow/web/responsebodies/MessageResource.java (1)
MessageResource(10-31)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
🔇 Additional comments (108)
application-engine/src/main/java/com/netgrif/application/engine/startup/runner/DashboardRunner.java (1)
35-37: Confirm disabling of dashboard importsThe calls to
createDashboardNet()andcreateDashboardTileNet()are commented out, so dashboard Petri nets won’t import at startup. Ensure an alternative initialization path exists under the new workspace-aware flows or document why these imports are deferred.application-engine/src/main/java/com/netgrif/application/engine/auth/web/requestbodies/UpdateUserRequest.java (1)
30-30: Ensure workspaceId is consumed in update flowsYou’ve exposed
public String workspaceId;with Lombok getters/setters, but downstream code (transformToLoggedUser(),transformToAuthor(), service handlers) currently returnnull. Verify that theworkspaceIdis propagated through the user-update logic and applied in the service layer.application-engine/src/main/java/com/netgrif/application/engine/startup/runner/ImpersonationRunner.java (1)
33-33: Confirm handling of impersonation config netsCommenting out
createConfigNets()prevents the import of impersonation-related Petri nets at startup. Ensure that a workspace-scoped runner now handles these, or explicitly document why this is deferred.application-engine/src/main/groovy/com/netgrif/application/engine/petrinet/domain/dataset/logic/action/runner/RoleActionsRunner.groovy (1)
3-3: Good addition of workspace context import.The import of
WorkspaceContextHolderis correctly added to support workspace-aware action execution.application-engine/src/main/java/com/netgrif/application/engine/startup/runner/FilterRunner.java (1)
38-41:❓ Verification inconclusive
Remove commented code or provide clear documentation.
All filter net creation calls have been commented out without explanation. This appears to be part of centralizing net imports to
DefaultNetImportRunner, but the approach raises concerns:
- Dead code: If these methods are no longer needed, consider removing them entirely
- Documentation: Add comments explaining why this functionality was moved
- Consistency: Ensure the same nets are being created in the new centralized location
Consider either:
- Removing the unused methods entirely, or
- Adding clear documentation explaining the migration:
@Override public void run(ApplicationArguments args) throws Exception { + // Filter net creation moved to DefaultNetImportRunner for workspace-aware imports + // See DefaultNetImportRunner.java for centralized default process imports -// createFilterNet(); -// createPreferenceItemNet(); -// createImportFiltersNet(); -// createExportFiltersNet(); }
🏁 Script executed:
#!/bin/bash # Verify that DefaultNetImportRunner handles these same net imports echo "Checking if DefaultNetImportRunner imports the same nets..." if [ -f "application-engine/src/main/java/com/netgrif/application/engine/startup/runner/DefaultNetImportRunner.java" ]; then echo "Found DefaultNetImportRunner, checking for filter net imports:" rg -A 3 -B 3 "filter\.xml|preference_item\.xml|import_filters\.xml|export_filters\.xml" application-engine/src/main/java/com/netgrif/application/engine/startup/runner/DefaultNetImportRunner.java else echo "DefaultNetImportRunner not found - need to verify where these nets are now imported" fiLength of output: 580
🏁 Script executed:
#!/bin/bash # List create*Net method definitions in FilterRunner.java rg -n 'void create.*Net' application-engine/src/main/java/com/netgrif/application/engine/startup/runner/FilterRunner.java # Search for any .xml references in DefaultNetImportRunner.java rg -n '\.xml' application-engine/src/main/java/com/netgrif/application/engine/startup/runner/DefaultNetImportRunner.java || trueLength of output: 253
Clarify Disabled Filter Net Creation
The four filter‐net creation calls in FilterRunner.run(...) (lines 38–41) are commented out with no explanation:
@Override public void run(ApplicationArguments args) throws Exception { + // DefaultNetImportRunner now handles filter net imports - // createFilterNet(); - // createPreferenceItemNet(); - // createImportFiltersNet(); - // createExportFiltersNet(); }I could not find matching
.xmlimports orcreate*Netinvocations in DefaultNetImportRunner.java. Please verify that:
- These nets are indeed initialized in DefaultNetImportRunner (or another central runner).
- If so, add a clear comment in FilterRunner referencing the new location.
- Otherwise, either re-enable these calls here or remove the unused methods to prevent dead code.
application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseService.java (1)
218-218:✅ Verification successful
LGTM! Workspace filtering properly integrated.
The addition of
buildWorkspaceQuery(query, user)correctly integrates workspace filtering into the case search pipeline. The placement between tags and group queries is logical and maintains consistency with other query builders.Please verify that the
buildWorkspaceQuerymethod is properly implemented in the parent class:
🏁 Script executed:
#!/bin/bash # Description: Verify buildWorkspaceQuery method exists in ElasticViewPermissionService # Expected: Method should be found with proper signature ast-grep --pattern $'class ElasticViewPermissionService { $$$ buildWorkspaceQuery($_, $_) { $$$ } $$$ }'Length of output: 117
🏁 Script executed:
#!/bin/bash # Search for any definition or references of buildWorkspaceQuery in the codebase # Find the parent class file echo "Locating ElasticViewPermissionService class:" rg "class ElasticViewPermissionService" -n # Search for buildWorkspaceQuery occurrences echo "Searching for buildWorkspaceQuery usages and definition:" rg "buildWorkspaceQuery" -nLength of output: 964
buildWorkspaceQuery verified in parent class
The
buildWorkspaceQuery(BoolQuery.Builder, LoggedUser)method is defined inElasticViewPermissionService.java(line 96), so its invocation inElasticCaseService.javais valid. LGTM.application-engine/src/test/java/com/netgrif/application/engine/importer/ConstructorAndDestructorTest.java (1)
49-49: LGTM! Test correctly updated for workspace-aware imports.The addition of the workspace ID parameter using
superCreator.getLoggedSuper().getWorkspaceId()properly adapts the test to the new workspace-aware Petri net import API. The use of the super user's workspace ID is appropriate for this test context.application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/dataset/FieldTest.groovy (2)
8-8: LGTM! Appropriate import for workspace functionality.The import of
DefaultWorkspaceServiceis necessary for accessing theDEFAULT_WORKSPACE_IDconstant used in the test.
79-79: LGTM! Test correctly updated with default workspace ID.The addition of
DefaultWorkspaceService.DEFAULT_WORKSPACE_IDas the workspace parameter correctly adapts the test to the workspace-aware import API. Using the default workspace constant is appropriate for test scenarios and ensures consistency.application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/ActionRefTest.groovy (1)
63-63: LGTM! Test correctly updated for workspace-aware imports.The addition of the workspace ID parameter using
userService.getLoggedOrSystem().getWorkspaceId()properly adapts the test to the new workspace-aware Petri net import API. The approach to get the workspace ID from the logged user or system user is appropriate.application-engine/src/main/java/com/netgrif/application/engine/workflow/service/TaskSearchService.java (1)
116-116: LGTM! Critical security enhancement for workspace isolation.The workspace filtering correctly ensures that task searches are scoped to the user's workspace, preventing cross-workspace data leakage. The placement within the query building logic is appropriate.
application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/ArcReferenceTest.groovy (2)
5-5: LGTM! Proper import addition for workspace support.The import of
DefaultWorkspaceServiceis necessary for accessing the default workspace ID in test scenarios.
44-44: LGTM! Test methods properly updated for workspace-aware API.The addition of
DefaultWorkspaceService.DEFAULT_WORKSPACE_IDparameter toimportPetriNetcalls correctly aligns with the new workspace-aware method signature. Using the default workspace ID is appropriate for test scenarios.Also applies to: 52-52
application-engine/src/test/java/com/netgrif/application/engine/workflow/service/TaskServiceTest.java (2)
94-94: LGTM! Test setup properly updated for workspace-aware imports.The addition of
userService.getLoggedOrSystem().getWorkspaceId()as the fourth parameter correctly maintains workspace context during Petri net imports in test setup.
101-101: LGTM! Test method properly updated for workspace context.The workspace ID parameter addition ensures that the Petri net import in the test method maintains proper workspace scoping, consistent with the production implementation.
application-engine/src/main/java/com/netgrif/application/engine/workflow/service/CaseSearchService.java (1)
97-97: LGTM! Consistent workspace isolation implementation.The workspace filtering correctly ensures that case searches are scoped to the user's workspace, consistent with the approach used in
TaskSearchService. This maintains data isolation and prevents cross-workspace access to cases.application-engine/src/test/groovy/com/netgrif/application/engine/elastic/ElasticTaskTest.groovy (1)
82-82:❓ Verification inconclusive
Verify userService import and null safety.
The code references
userService.getLoggedOrSystem().getWorkspaceId()but I don't see an import foruserServicein the provided code. Additionally, consider adding null safety checks.
🏁 Script executed:
#!/bin/bash # Check if userService is imported or injected in this test class ast-grep --pattern 'import $$$userService$$$' echo "--- Checking for userService injection ---" ast-grep --pattern '@Autowired $_userService' echo "--- Checking for userService field declaration ---" ast-grep --pattern '$_ userService'Length of output: 339
We still can’t find any definition or injection of
userServiceinElasticTaskTest.groovy, nor imports for it or its type. It looks like this needs to be added explicitly (or inherited via a shared base class).Please add something like:
import com.netgrif.application.engine.service.UserService @Autowired UserService userServicebefore using
userService.getLoggedOrSystem(). Also, ensure you handle the case wheregetLoggedOrSystem()orgetWorkspaceId()might return null (or wrap the call in an Optional assertion) to avoid NPEs in your test.— fix_required
application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticTaskService.java (1)
207-207:✅ Verification successful
Verify buildWorkspaceQuery method implementation.
The addition of workspace filtering to task queries is appropriate for the workspace isolation feature. Please ensure the
buildWorkspaceQuerymethod is properly implemented.
🏁 Script executed:
#!/bin/bash # Check if buildWorkspaceQuery method is implemented in this class or parent classes echo "--- Checking for buildWorkspaceQuery method in current file ---" ast-grep --pattern 'buildWorkspaceQuery($$$) { $$$ }' echo "--- Checking for buildWorkspaceQuery in parent class ElasticViewPermissionService ---" fd -e java ElasticViewPermissionService | xargs cat | grep -A 10 "buildWorkspaceQuery"Length of output: 767
buildWorkspaceQuery method confirmed and correctly implemented
- The
buildWorkspaceQuerymethod is defined inElasticViewPermissionServiceand filters onworkspaceIdusing the user’s workspace.- The addition of
buildWorkspaceQuery(query, user)inElasticTaskServiceproperly integrates workspace isolation into task searches.No further changes required.
application-engine/src/test/groovy/com/netgrif/application/engine/pdf/service/PdfGeneratorTest.groovy (1)
92-92: LGTM! Consistent workspace ID parameter addition across all test methods.The systematic addition of
userService.getLoggedOrSystem().getWorkspaceId()as the fourth parameter to allpetriNetService.importPetriNetcalls correctly implements workspace isolation in the test suite. This ensures that Petri net imports in tests are properly scoped to the appropriate workspace context.Also applies to: 104-104, 118-118, 132-132, 151-151, 171-171, 185-185, 212-212, 227-227, 241-241, 254-254, 266-266, 285-285
application-engine/src/main/groovy/com/netgrif/application/engine/petrinet/domain/dataset/logic/action/runner/CaseFieldsExpressionRunner.groovy (2)
3-3: LGTM! Proper import for workspace context handling.The import of
WorkspaceContextHolderis necessary for setting workspace context during expression execution.
46-46: LGTM! Correct workspace context propagation before expression execution.Setting the workspace context using
WorkspaceContextHolder.setWorkspaceId(useCase.getWorkspaceId(), true)before executing the expression ensures that any workspace-scoped operations within the expression code run in the correct workspace context. The placement beforeinitCode()andcode()execution is appropriate.application-engine/src/test/groovy/com/netgrif/application/engine/elastic/ElasticSearchTest.groovy (1)
103-104: LGTM! Consistent workspace parameter addition in test setup.The addition of
userService.getLoggedOrSystem().getWorkspaceId()as the fourth parameter to bothpetriNetService.importPetriNetcalls ensures that the imported Petri nets in the test setup are properly scoped to the appropriate workspace context, consistent with the workspace isolation implementation.application-engine/src/main/groovy/com/netgrif/application/engine/migration/ActionMigration.groovy (1)
27-27: LGTM! Proper workspace scoping for migration operations.The addition of
userService.getLoggedOrSystem().getWorkspaceId()as the fourth parameter topetriNetService.importPetriNetensures that Petri nets imported during migration operations are properly scoped to the appropriate workspace. This is crucial for maintaining workspace isolation and data integrity during migration processes.application-engine/src/test/groovy/com/netgrif/application/engine/action/AssignActionTest.groovy (1)
110-113: LGTM! Consistent workspace scoping implementation.The addition of workspace ID parameters to both Petri net imports ensures proper workspace isolation in the test environment. The usage of
userService.getLoggedOrSystem().getWorkspaceId()maintains consistency with the workspace context.application-engine/src/test/groovy/com/netgrif/application/engine/auth/WorkflowAuthorizationServiceTest.groovy (1)
121-125: LGTM! Proper workspace context in test setup.The workspace ID parameter additions ensure that both test Petri nets are properly scoped to the workspace context, maintaining consistency with the workspace isolation feature being implemented across the application.
application-engine/src/main/java/com/netgrif/application/engine/workflow/web/responsebodies/Task.java (3)
79-79: LGTM! Workspace field addition for API context.The
workspaceIdfield addition supports workspace context propagation in the Task response body, ensuring API clients receive proper workspace information.
109-109: LGTM! Proper workspace extraction from domain object.The workspace ID is correctly extracted from the domain Task object, maintaining consistency with other field mappings in the constructor.
119-119: LGTM! Consistent workspace handling for ElasticTask.The workspace ID extraction from ElasticTask entity follows the same pattern as other fields, ensuring consistent workspace context across different data sources.
application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/web/PetriNetControllerTest.groovy (1)
84-84: LGTM! Workspace-aware test setup.The workspace ID parameter addition ensures the test Petri net import is properly scoped to workspace context, maintaining consistency with the workspace isolation implementation across the test suite.
application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/service/PetriNetServiceTest.groovy (1)
111-111: LGTM! Workspace context properly added to Petri net import.The addition of the workspace ID parameter aligns with the workspace isolation feature. The workspace ID is correctly obtained from the logged or system user context.
application-engine/src/test/groovy/com/netgrif/application/engine/auth/SecurityContextTest.groovy (1)
74-74: LGTM! Workspace context properly added to security test setup.The addition of the workspace ID parameter ensures that the Petri net import in the security context test respects workspace boundaries.
application-engine/src/test/java/com/netgrif/application/engine/petrinet/service/ProcessRoleServiceTest.java (1)
58-59: LGTM! Consistent workspace context implementation across all test methods.All Petri net import calls have been properly updated to include the workspace ID parameter. The consistent use of
superCreator.getLoggedSuper().getWorkspaceId()ensures proper workspace isolation in all test scenarios.Also applies to: 68-68, 95-95, 105-105
application-engine/src/test/groovy/com/netgrif/application/engine/action/AssignRemoveTest.groovy (1)
68-68: LGTM: Workspace context properly added.The workspace ID parameter is correctly added to the
importPetriNetcall, andUserServiceis properly autowired in this class (line 50), so this change will work as expected.application-engine/src/test/groovy/com/netgrif/application/engine/auth/AuthenticationControllerTest.groovy (1)
87-87: LGTM! Workspace ID integration is correctly implemented.The workspace ID parameter addition is correctly implemented. Unlike other test files, this one properly has the
userServiceimported and injected, so the reference touserService.getLoggedOrSystem().getWorkspaceId()will work correctly.application-engine/src/test/groovy/com/netgrif/application/engine/ipc/TaskApiTest.groovy (1)
71-71: LGTM! Workspace ID parameter properly integrated.All
petriNetService.importPetriNet()calls have been consistently updated to include the workspace ID parameter, anduserServiceis properly autowired in this class. The implementation correctly propagates workspace context across all test methods.Also applies to: 102-102, 124-125, 199-199, 224-224, 259-259
application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/EventTest.groovy (1)
90-90: LGTM! Workspace context properly propagated.The workspace ID is correctly obtained from the logged user and passed to the import method, ensuring proper workspace isolation during Petri net import.
application-engine/src/main/java/com/netgrif/application/engine/workflow/service/DataService.java (1)
523-524: Excellent workspace context propagation in action execution.The addition of
useCase.getWorkspaceId()as a parameter toactionsRunner.run()ensures that file field actions execute within the correct workspace context, maintaining proper isolation between workspaces.This change is crucial for the workspace isolation feature as it ensures that all action executions are workspace-aware and prevents cross-workspace data contamination during file field operations.
application-engine/src/test/groovy/com/netgrif/application/engine/auth/TaskAuthorizationServiceTest.groovy (1)
160-160: LGTM! Workspace context properly added to Petri net imports.The addition of workspace ID parameters to both
petriNetService.importPetriNetcalls correctly implements workspace isolation for the test setup. The consistent use ofuserService.getLoggedOrSystem().getWorkspaceId()ensures that test Petri nets are imported within the appropriate workspace context.Also applies to: 164-164
application-engine/src/test/groovy/com/netgrif/application/engine/action/RemoveActionTest.groovy (1)
96-96: LGTM! Consistent workspace context implementation.The addition of the workspace ID parameter maintains consistency with the workspace isolation pattern implemented across the codebase. The test setup now correctly imports the Petri net within the appropriate workspace context.
application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticViewPermissionService.java (1)
96-100: LGTM! Proper workspace filtering implementation for Elasticsearch.The new
buildWorkspaceQuerymethod correctly implements workspace-based filtering for Elasticsearch queries. The implementation:
- Uses appropriate term query to match the
workspaceIdfield- Properly adds the workspace constraint as a filter clause
- Follows consistent patterns with other query building methods in the class
- Is appropriately scoped as
protectedfor use by subclassesThis supports the broader workspace isolation functionality being implemented across the application.
application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/dataset/ChangeCasePropertyTest.groovy (1)
59-59: LGTM! Workspace context correctly implemented.The addition of the workspace ID parameter to the
petriNetService.importPetriNetcall maintains consistency with the workspace isolation implementation across the test suite. The test now properly imports the Petri net within the appropriate workspace context.application-engine/src/test/groovy/com/netgrif/application/engine/elastic/DataSearchRequestTest.groovy (1)
99-99: LGTM! Workspace context properly added to Petri net import.The addition of the workspace ID parameter aligns with the broader workspace isolation implementation. The
UserServicedependency is properly autowired, ensuring the workspace ID can be retrieved correctly.application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/dataset/FileFieldTest.groovy (1)
102-102: LGTM! Workspace context properly added to Petri net import.The addition of the workspace ID parameter is correctly implemented. The
UserServicedependency is properly autowired on line 76, ensuring the workspace ID can be retrieved without issues.application-engine/src/main/java/com/netgrif/application/engine/elastic/domain/ElasticPetriNetRepository.java (1)
14-14:✅ Verification successful
LGTM! Verify all method usages are updated.
The method signature change correctly adds workspace filtering to Petri net retrieval. This follows Spring Data naming conventions and aligns with the workspace isolation feature.
Please verify that all existing usages of the old
findAllByUriNodeIdmethod have been updated to include the workspace ID parameter:
🏁 Script executed:
#!/bin/bash # Description: Search for any remaining usages of the old method signature # Expected: No occurrences of the old method name without workspace parameter rg -A 3 "findAllByUriNodeId\(" --type javaLength of output: 2625
Workspace isolation applied correctly
- No direct calls to the old
findAllByUriNodeIdrepository method remain—every repository invocation now usesfindAllByUriNodeIdAndWorkspaceId.- The service layer intentionally continues to expose
findAllByUriNodeId(String)at the interface/API level, injecting the workspace ID from the current user context internally.No further updates are required.
application-engine/src/test/java/com/netgrif/application/engine/workflow/web/VariableArcsTest.java (1)
111-111: LGTM! Test updated correctly for workspace context.The test method call has been properly updated to include the workspace ID parameter, ensuring the Petri net import is scoped to the correct workspace context.
application-engine/src/main/java/com/netgrif/application/engine/export/service/ExportService.java (1)
15-16: LGTM! Appropriate imports for workspace filtering.The imports for
QCase,QTask, andBooleanBuilderare correctly added to support the workspace filtering implementation.Also applies to: 22-22
application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/dataset/FileListFieldTest.groovy (1)
166-166: LGTM! Workspace integration properly implemented.The workspace ID parameter addition is consistent with the broader workspace integration changes and
userServiceis properly autowired.application-engine/src/main/java/com/netgrif/application/engine/startup/runner/SuperCreatorRunner.java (3)
8-8: LGTM! Proper workspace service integration.The import addition aligns with the workspace integration requirements.
42-42: LGTM! Workspace service dependency properly added.The dependency injection follows proper Spring patterns and integrates with the workspace architecture.
104-104: LGTM! Proper workspace context before user transformation.The workspace ID assignment ensures correct context is maintained when transforming to LoggedUser.
application-engine/src/main/java/com/netgrif/application/engine/auth/web/responsebodies/WorkspaceResponse.java (1)
1-16: LGTM! Well-structured workspace response DTO.The class follows standard DTO patterns with proper Lombok annotations and constructor. The implementation supports the new workspace API endpoints effectively.
application-engine/src/test/groovy/com/netgrif/application/engine/importer/UserListTest.groovy (1)
57-57: Workspace context integration looks correct.The addition of the workspace ID parameter to the
importPetriNetmethod call is consistent with the broader workspace context refactoring. The workspace ID is properly obtained from the logged/system user.application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/FunctionsTest.groovy (1)
90-91: Consistent workspace context integration across all test methods.All
importPetriNetmethod calls have been consistently updated to include the workspace ID parameter obtained fromuserService.getLoggedOrSystem().getWorkspaceId(). The pattern is applied uniformly across all test methods.Also applies to: 109-109, 133-133, 151-151, 157-157, 167-167, 177-178, 189-189, 201-201, 207-208, 226-226, 236-236
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/ElasticCase.java (1)
36-36: Workspace field addition is correct.The addition of the
workspaceIdfield properly extends the ElasticCase domain object to support workspace context, aligning with the broader workspace integration effort.application-engine/src/main/java/com/netgrif/application/engine/configuration/security/jwt/JwtService.java (1)
8-8: Proper dependency injection for workspace service.The
DefaultWorkspaceServiceis correctly injected as a final field, following proper Spring dependency injection patterns with the@RequiredArgsConstructorannotation.Also applies to: 37-37
application-engine/src/main/java/com/netgrif/application/engine/petrinet/service/workspace/DefaultWorkspaceConfiguration.java (2)
11-15: Well-structured configuration with proper conditional bean creation.The use of
@ConditionalOnMissingBeanallows for flexible customization while providing a sensible default implementation.
14-14:⚠️ Potential issueMissing import for DefaultWorkspaceServiceImpl.
The class
DefaultWorkspaceServiceImplis referenced but not imported.Add the missing import:
import com.netgrif.application.engine.objects.petrinet.domain.workspace.DefaultWorkspaceService; +import com.netgrif.application.engine.petrinet.service.workspace.DefaultWorkspaceServiceImpl; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;Likely an incorrect or invalid review comment.
application-engine/src/main/java/com/netgrif/application/engine/startup/runner/DefaultNetImportRunner.java (1)
15-19: Excellent use of constructor injection and proper annotations.The class follows Spring best practices with constructor injection via
@RequiredArgsConstructorand proper execution ordering with@RunnerOrder.application-engine/src/main/java/com/netgrif/application/engine/auth/service/RegistrationService.java (1)
14-15: LGTM: Proper workspace service integration.The import and injection of
DefaultWorkspaceServicecorrectly sets up the dependency needed for workspace-aware role assignment.Also applies to: 50-51
application-engine/src/main/java/com/netgrif/application/engine/configuration/authentication/providers/basic/NetgrifBasicAuthenticationProvider.java (2)
4-4: LGTM: Proper workspace service integration.The imports and injection of
DefaultWorkspaceServiceare correctly implemented to support workspace context setting in authentication.Also applies to: 9-9, 33-34
69-70:✅ Verification successful
Verify the safety of the cast to LoggedUserImpl.
The cast from
UserDetailstoLoggedUserImplcould be unsafe ifuserService.transformToLoggedUser()doesn't guarantee returning aLoggedUserImplinstance. This could cause aClassCastExceptionat runtime.Please verify that
userService.transformToLoggedUser()always returns aLoggedUserImplinstance:
🏁 Script executed:
#!/bin/bash # Description: Verify the return type of transformToLoggedUser method # Expected: Method should return LoggedUserImpl or its subtype ast-grep --pattern $'transformToLoggedUser($_) { $$$ }'Length of output: 58
To locate the actual declaration and return type of
transformToLoggedUser, let’s search all Java sources with context:#!/bin/bash # Search for the method declaration and its return type rg -n "transformToLoggedUser" -C 3 --glob '*.java'
Let’s locate the actual method signature to confirm its return type:
🏁 Script executed:
#!/bin/bash # Search for transformToLoggedUser declaration with its return type rg -n "transformToLoggedUser" -C 3 --glob '*.java'Length of output: 74735
No casting risk:
transformToLoggedUserreturnsLoggedUserImplThe
UserServiceinterface declaresLoggedUserImpl transformToLoggedUser(IUser user);and its implementation in
UserServiceImplreturns aLoggedUserImplinstance. Assigning directly toLoggedUserImpl userDetailsis therefore safe and cannot cause aClassCastException.application-engine/src/main/java/com/netgrif/application/engine/impersonation/service/ImpersonationAuthorizationService.java (2)
15-15: LGTM: Consistent workspace service integration.The import and injection of
DefaultWorkspaceServicefollows the established pattern for workspace-aware functionality.Also applies to: 55-56
111-113: Approve consistent workspace-aware role assignment.The implementation correctly adds default roles for all workspaces, ensuring impersonated users have appropriate access across workspace boundaries. This is consistent with the same pattern implemented in
RegistrationService.application-engine/src/main/java/com/netgrif/application/engine/startup/runner/DefaultRoleRunner.java (2)
9-10: LGTM: Proper workspace-aware dependencies.The new imports and injection of workspace-related services correctly set up the dependencies needed for workspace-aware default role creation.
Also applies to: 27-27
31-31: Excellent refactoring to centralize role creation logic.The simplification from manual role creation to service delegation is a great improvement. This centralizes the role creation logic in the service layer and ensures workspace-aware role creation, following good separation of concerns principles.
application-engine/src/test/java/com/netgrif/application/engine/importer/ImporterTest.java (1)
136-136:✅ Verification successful
Verify repository interface change is consistent.
The change from
findByIdentifiertofindByIdentifierAndWorkspaceIdindicates that the repository interface has been updated to include workspace scoping. Ensure that this change is consistently applied across all repository usages in the codebase.
🏁 Script executed:
#!/bin/bash # Description: Verify that all repository calls have been updated to include workspace filtering # Search for any remaining calls to the old findByIdentifier method without workspace rg "findByIdentifier\(" --type java -A 2 -B 2Length of output: 8708
🏁 Script executed:
#!/bin/bash # Locate the PetriNetRepository interface and inspect its methods rg -n "interface.*PetriNetRepository" --type java -A5 rg -n "findByIdentifierAndWorkspaceId" --type java -A2 -B2Length of output: 5732
PetriNetRepository usages are correctly updated
All calls tofindByIdentifieronPetriNetRepositoryhave been replaced withfindByIdentifierAndWorkspaceId, and no remaining direct references to the old method exist in the application-engine module. Other repositories (e.g. GroupRepository, RuleRepository) still define and usefindByIdentifierby design and were not intended to change.application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticPetriNetService.java (2)
25-25: LGTM! Proper dependency injection implementation.The UserService dependency is correctly added via constructor injection, following best practices for dependency injection. The constructor properly initializes the new field.
Also applies to: 31-35
93-93:✅ Verification successful
Verify the repository interface change impacts.
The change from
findAllByUriNodeId(uriNodeId)tofindAllByUriNodeIdAndWorkspaceId(uriNodeId, userService.getLoggedOrSystem().getWorkspaceId())is a breaking change that requires the corresponding repository interface to be updated. This correctly implements workspace isolation.
🏁 Script executed:
#!/bin/bash # Description: Verify that the ElasticPetriNetRepository interface has been updated accordingly # Search for the old method signature in repository interfaces rg "findAllByUriNodeId\(" --type java -A 2 -B 2 # Search for the new method signature to confirm it exists rg "findAllByUriNodeIdAndWorkspaceId\(" --type java -A 2 -B 2Length of output: 7549
Verified: ElasticPetriNetRepository updated correctly
Confirmed that
ElasticPetriNetRepositorydeclaresList<ElasticPetriNet> findAllByUriNodeIdAndWorkspaceId(String uriNodeId, String workspaceId);and the service still exposes
findAllByUriNodeId(String uriNodeId)unchanged. No further interface or implementation updates are required.application-engine/src/main/groovy/com/netgrif/application/engine/DevConsole.groovy (2)
38-40: LGTM! Correct workspace-aware filtering implementation.The addition of the
Authenticationparameter and extraction ofLoggedUserto filter cases by workspace ID correctly implements workspace isolation for the dev console.
45-48: LGTM! Consistent workspace filtering pattern.The
netSnapshotmethod follows the same pattern asdataset, correctly extracting the workspace ID from the authenticated user and filtering cases accordingly.application-engine/src/main/java/com/netgrif/application/engine/auth/web/UserController.java (2)
7-16: LGTM! Proper dependency injection for workspace functionality.The new imports and service dependencies are correctly added to support workspace operations. The constructor injection follows proper DI patterns.
Also applies to: 36-36, 65-67
182-182: LGTM! Correct workspace ID propagation.Setting the workspace ID from the actual user ensures that the returned user object has the correct workspace context, which is important for maintaining workspace isolation.
application-engine/src/main/java/com/netgrif/application/engine/petrinet/service/workspace/DefaultWorkspaceServiceImpl.java (1)
10-29:✅ Verification successful
Verify thread safety for singleton usage.
Based on the AI summary, this service is used extensively across authentication, authorization, and service layers. Ensure thread safety since the workspace object might be accessed concurrently.
🏁 Script executed:
#!/bin/bash # Description: Check how DefaultWorkspaceService is configured and used across the codebase # Expected: Should find Spring bean configuration and usage patterns # Search for Spring configuration of this service rg -A 5 -B 5 "DefaultWorkspaceServiceImpl|DefaultWorkspaceService" --type java # Search for @Bean or @Component annotations related to workspace service ast-grep --pattern $'@Bean $$$ DefaultWorkspaceService$_($$$) { $$$ }' # Search for usage patterns to understand concurrency requirements rg -A 3 "getDefaultWorkspace|getAllWorkspaces" --type javaLength of output: 66145
Thread safety confirmed
The
DefaultWorkspaceServiceImplis registered as a singleton Spring bean (seeDefaultWorkspaceConfiguration) and creates its onlyWorkspaceinstance exactly once in the constructor. BothgetDefaultWorkspace()andgetAllWorkspaces()return that pre-configured object for read-only access, with no further mutations after construction. Concurrent callers only read its ID and default flag, so under current usage this bean is thread-safe.application-engine/src/main/groovy/com/netgrif/application/engine/petrinet/domain/dataset/logic/action/ActionDelegate.groovy (3)
4-4: LGTM: Import aligns with workspace context enhancement.The new
AsyncRunnerWrapperimport is correctly added to support the field type change and workspace context propagation functionality.
124-124: LGTM: Field type change supports workspace context.The change from
AsyncRunnertoAsyncRunnerWrapperis consistent with the workspace isolation enhancement and properly used in the code.
1983-1985: LGTM: Proper workspace context propagation.The modification correctly passes the workspace ID to the async runner, ensuring that the delete operation maintains workspace isolation. This is a proper implementation of the workspace context enhancement.
application-engine/src/main/java/com/netgrif/application/engine/startup/runner/AnonymousRoleRunner.java (2)
7-34: Clean refactoring to support workspace isolation.The refactoring properly delegates anonymous role creation to the service layer and integrates workspace context, which aligns well with the broader workspace isolation feature.
33-33:✅ Verification successful
Add null check for default workspace.
The call to
defaultWorkspaceService.getDefaultWorkspace()could potentially return null. Consider adding a null check to preventNullPointerException.Run the following script to verify if
getDefaultWorkspace()can return null:
🏁 Script executed:
#!/bin/bash # Description: Check if DefaultWorkspaceService.getDefaultWorkspace() can return null # Search for the implementation of getDefaultWorkspace ast-grep --pattern 'getDefaultWorkspace() { $$$ }' # Also check for any null checks in other usages rg -A 5 'getDefaultWorkspace\(\)' --type javaLength of output: 10710
🏁 Script executed:
#!/bin/bash # Inspect DefaultWorkspaceServiceImpl to see how defaultWorkspace is initialized and whether it can be null sed -n '1,200p' application-engine/src/main/java/com/netgrif/application/engine/petrinet/service/workspace/DefaultWorkspaceServiceImpl.javaLength of output: 1029
No null check required for
defaultWorkspaceService.getDefaultWorkspace()The
DefaultWorkspaceServiceImplconstructor always initializes and returns a non-nullWorkspaceinstance, sogetDefaultWorkspace()cannot return null. The refactoring delegation is safe—no additional null guard is needed. 🎉application-engine/src/main/java/com/netgrif/application/engine/workflow/service/TaskService.java (3)
731-751: Consistent workspace filtering implementation.The workspace filtering is consistently applied across all search methods using the same pattern. This ensures proper workspace isolation for task queries.
850-851: Proper workspace-scoped role retrieval.The default and anonymous role lookups now correctly pass the workspace ID, ensuring roles are retrieved within the correct workspace context.
818-818: Ensure workspace ID is not null during task creation.The
transition.getWorkspaceId()should be validated to ensure it's not null before creating the task.Consider adding a null check:
.transitionId(transition.getImportId()) -.workspaceId(transition.getWorkspaceId()) +.workspaceId(Objects.requireNonNull(transition.getWorkspaceId(), "Transition must have a workspace ID")) .layout(transition.getLayout())application-engine/src/main/groovy/com/netgrif/application/engine/AsyncRunnerWrapper.java (1)
15-17:❓ Verification inconclusive
Add null safety check for workspace ID.
The method should validate that the workspace ID is not null before proceeding.
Consider adding validation:
public void run(Closure closure) { - run(closure, userService.getLoggedOrSystem().getWorkspaceId()); + String workspaceId = userService.getLoggedOrSystem().getWorkspaceId(); + if (workspaceId == null) { + throw new IllegalStateException("Workspace ID cannot be null for async operations"); + } + run(closure, workspaceId); }
🏁 Script executed:
#!/bin/bash # Description: Check if getWorkspaceId() can return null in UserService # Search for getWorkspaceId implementation ast-grep --pattern 'getWorkspaceId() { $$$ }' # Check for null returns or checks rg -A 5 'getWorkspaceId' --type java | grep -E '(return null|== null|!= null)'Length of output: 4112
🏁 Script executed:
#!/bin/bash # Print AsyncRunnerWrapper.java to inspect imports and userService type sed -n '1,200p' application-engine/src/main/groovy/com/netgrif/application/engine/AsyncRunnerWrapper.javaLength of output: 1139
🏁 Script executed:
#!/bin/bash # Inspect WorkspaceContextHolder sed -n '1,200p' application-engine/src/main/java/com/netgrif/application/engine/auth/config/WorkspaceContextHolder.java # Inspect UserService interface sed -n '1,200p' application-engine/src/main/java/com/netgrif/application/engine/auth/service/UserService.java # Inspect UserServiceImpl to see getLoggedOrSystem logic sed -n '1,200p' nae-user-ce/src/main/java/com/netgrif/application/engine/auth/service/UserServiceImpl.javaLength of output: 16247
🏁 Script executed:
#!/bin/bash # Locate getLoggedOrSystem in codebase rg -n "getLoggedOrSystem" -t java -A 3 -B 3Length of output: 57680
🏁 Script executed:
#!/bin/bash # Locate and inspect WorkspaceContextHolder implementation ws_file=$(rg -l "class WorkspaceContextHolder" | head -n1) echo "Found WorkspaceContextHolder at $ws_file" sed -n '1,50p' "$ws_file" rg -n "setWorkspaceId" -H "$ws_file" -A5 -B5 # Inspect getLoggedOrSystem and getSystem in UserServiceImpl svc=nae-user-ce/src/main/java/com/netgrif/application/engine/auth/service/UserServiceImpl.java echo "Inspecting getLoggedOrSystem in $svc" sed -n '415,460p' "$svc" echo "Inspecting getSystem in $svc" rg -n "getSystem(" -H "$svc" -A5 -B5Length of output: 10124
🏁 Script executed:
#!/bin/bash # Search for mockLoggedUser definition rg -n "mockLoggedUser" -H application-engine/src/test/javaLength of output: 933
Verify null safety for workspace ID in AsyncRunnerWrapper
File: application-engine/src/main/groovy/com/netgrif/application/engine/AsyncRunnerWrapper.java
Lines: 15–17, 25–27Snippet:
public void run(Closure closure) { run(closure, userService.getLoggedOrSystem().getWorkspaceId()); } public void execute(Runnable runnable) { execute(runnable, userService.getLoggedOrSystem().getWorkspaceId()); }While
getSystem()will always fall back to a default workspace,getLoggedOrSystem().getWorkspaceId()can still benullif the security principal’s workspaceId isn’t set. Please verify that a null workspace ID cannot occur here; if it can, add a guard in both overloads:public void run(Closure closure) { - run(closure, userService.getLoggedOrSystem().getWorkspaceId()); + String workspaceId = userService.getLoggedOrSystem().getWorkspaceId(); + if (workspaceId == null) { + throw new IllegalStateException("Workspace ID cannot be null for async operations"); + } + run(closure, workspaceId); } public void execute(Runnable runnable) { - execute(runnable, userService.getLoggedOrSystem().getWorkspaceId()); + String workspaceId = userService.getLoggedOrSystem().getWorkspaceId(); + if (workspaceId == null) { + throw new IllegalStateException("Workspace ID cannot be null for async operations"); + } + execute(runnable, workspaceId); }application-engine/src/main/java/com/netgrif/application/engine/petrinet/web/PetriNetController.java (2)
80-80: LGTM: AsyncRunnerWrapper integration.Good integration of workspace-aware asynchronous execution. The change from
AsyncRunnertoAsyncRunnerWrapperensures workspace context is properly maintained during async operations.
215-215: LGTM: Workspace context in async deletion.The workspace ID is correctly passed to the async runner to ensure the deletion operation executes within the proper workspace context.
application-engine/src/main/java/com/netgrif/application/engine/importer/service/Importer.java (2)
130-132: LGTM: Workspace parameter propagation.The workspace ID parameter is correctly added to both
importPetriNetmethod overloads and properly propagated to theinitializemethod. This ensures workspace context is maintained throughout the import process.Also applies to: 142-144
151-151: LGTM: Workspace-scoped role initialization.The
initializemethod correctly accepts the workspace ID and uses it to retrieve workspace-specific default and anonymous roles. This ensures roles are properly scoped to the correct workspace during Petri net import.Also applies to: 157-158
application-engine/src/main/java/com/netgrif/application/engine/workflow/domain/repositories/CaseRepository.java (1)
22-22: LGTM: Workspace-scoped repository methods.The repository method updates correctly implement workspace filtering:
findAllByProcessIdentifierAndWorkspaceIdreplaces the previous method with workspace scoping- New
findAllByWorkspaceIdmethods provide both list and paged variantsfindAllByUriNodeIdAndWorkspaceIdadds workspace filtering to URI-based queriesThe method naming follows Spring Data conventions and ensures all case queries are properly workspace-scoped.
Also applies to: 26-28, 33-33
application-engine/src/main/groovy/com/netgrif/application/engine/petrinet/domain/dataset/logic/action/FieldActionsRunner.groovy (1)
3-3: LGTM: Workspace context management in action execution.The implementation correctly manages workspace context during field action execution:
- Added
WorkspaceContextHolderimport for workspace context management- Updated
runmethod signatures to acceptworkspaceIdparameter consistently- Workspace context is properly set before action execution, prioritizing the use case's workspace ID with fallback to the parameter
- Uses
force=trueflag to ensure workspace context is establishedThis ensures all field actions execute within the correct workspace boundary, maintaining workspace isolation.
Also applies to: 47-48, 51-51, 60-60
application-engine/src/main/groovy/com/netgrif/application/engine/startup/ImportHelper.groovy (1)
114-122: LGTM! Workspace context properly propagated.The new overloaded
createNetmethod correctly extracts the workspace ID from the authenticated user and passes it to the import service. The implementation maintains backward compatibility while adding workspace isolation support.application-engine/src/main/java/com/netgrif/application/engine/workflow/service/interfaces/IEventService.java (1)
21-27:✅ Verification successful
Verify all implementations are updated.
The interface changes require all implementing classes to be updated with the new
workspaceIdparameter. Ensure that all implementations ofIEventServicehave been updated accordingly.Run the following script to find all implementations and verify they're updated:
🏁 Script executed:
#!/bin/bash # Description: Find all IEventService implementations and check if they're updated # Find all classes implementing IEventService ast-grep --pattern 'class $_ implements $$$IEventService$$$' # Search for EventService implementation to verify the methods are updated rg -A 10 "class.*EventService.*implements.*IEventService" # Check for the updated method signatures in implementations rg "runActions.*workspaceId|runEventActions.*workspaceId" --type javaLength of output: 11074
All IEventService implementations are up-to-date.
The only class implementing IEventService (
EventService) already includes all three updated method signatures with the new workspaceId parameter. No further changes are needed.application-engine/src/main/java/com/netgrif/application/engine/petrinet/domain/roles/ProcessRoleRepository.java (3)
19-19: LGTM: Workspace scoping added for role retrieval.The new method correctly enables filtering roles by workspace ID, supporting proper data isolation.
25-29: LGTM: Existing query methods updated with workspace scoping.The workspace parameter additions to these query methods ensure proper data isolation for role operations based on name, import ID, and global flags.
34-36:❓ Verification inconclusive
Verify workspace isolation for ID-based operations.
These methods (
findByIdObjectIdanddeleteAllBy_idIn) don't include workspace scoping, which could potentially allow access to roles across workspace boundaries.
🏁 Script executed:
#!/bin/bash # Description: Check if these methods are used in workspace-aware contexts or need workspace scoping # Search for usage of findByIdObjectId rg -A 5 -B 2 "findByIdObjectId" # Search for usage of deleteAllBy_idIn rg -A 5 -B 2 "deleteAllBy_idIn"Length of output: 79855
🏁 Script executed:
#!/bin/bash # Locate and display key parts of ProcessRoleRepository FILE="application-engine/src/main/java/com/netgrif/application/engine/petrinet/domain/roles/ProcessRoleRepository.java" echo "---- File Head ----" sed -n '1,200p' "$FILE" echo "---- Search for networkId methods ----" rg "findByNetworkIdAndObjectId" -n "$FILE" rg "deleteAllByNetworkIdAndObjectId" -n "$FILE"Length of output: 3352
🏁 Script executed:
#!/bin/bash # Display ProcessRole class to check fields and workspace context FILE="application-engine/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/roles/ProcessRole.java" echo "---- Existence ----" [ -f "$FILE" ] && echo "Found: $FILE" || { echo "Not found"; exit 1; } echo "---- Class Definition ----" sed -n '1,200p' "$FILE" echo "---- Fields for workspace, id ----" rg "workspaceId" -n "$FILE" rg "shortProcessId" -n "$FILE" rg "objectId" -n "$FILE"Length of output: 328
Verify workspace scoping on ProcessRoleRepository ID‐based methods
The methods
Optional<ProcessRole> findByIdObjectId(ObjectId objectId); void deleteAllBy_idIn(Collection<ProcessResourceId> ids);do not include any
workspaceIdfilter, which could allow cross-workspace reads or deletions. Please confirm and, if necessary, add workspace scoping:• Verify that
ProcessRolehas aworkspaceIdfield (or that it’s embedded in the document’s_id).
• If workspace is tracked separately, introduce scoped queries such as:@Query("{ 'workspaceId': ?1, '_id.objectId': ?0 }") Optional<ProcessRole> findByWorkspaceIdAndObjectId(ObjectId objectId, String workspaceId); @Query("{ 'workspaceId': ?1, '_id.objectId': { $in: ?0 } }") void deleteAllByWorkspaceIdAnd_idIn(Collection<ObjectId> ids, String workspaceId);• Update callers (e.g., service layer) to pass the current
workspaceIdwhen invoking these methods.application-engine/src/main/java/com/netgrif/application/engine/petrinet/domain/repositories/PetriNetRepository.java (1)
19-31: LGTM: Comprehensive workspace scoping for PetriNet queries.The workspace parameter additions provide thorough data isolation for identifier, version, URI, and general workspace-based PetriNet retrieval operations.
application-engine/src/main/java/com/netgrif/application/engine/workflow/service/WorkflowService.java (5)
37-37: LGTM: Import added to support workspace query building.The BooleanBuilder import is correctly added to support the workspace predicate construction in the search method.
192-192: LGTM: Workspace filtering for case retrieval.The method correctly filters cases by the current user's workspace ID, ensuring proper data isolation.
200-200: LGTM: Workspace filtering for URI-based case retrieval.The method correctly scopes case retrieval by both URI and workspace ID, maintaining proper data isolation.
208-209: LGTM: Workspace predicate building for search.The implementation correctly adds workspace filtering to the search predicate using BooleanBuilder, ensuring case search results are scoped to the user's workspace.
344-344: LGTM: Workspace context propagation to event service.The workspace ID is correctly passed to event service calls for both case creation and deletion workflows, ensuring workspace context is maintained during event processing.
Also applies to: 356-356, 405-405, 413-413
application-engine/src/main/java/com/netgrif/application/engine/workflow/service/EventService.java (5)
40-40: LGTM: Workspace context propagation from case.The method correctly extracts and passes the workspace ID from the case when delegating to the overloaded method.
44-46: LGTM: New workspace-aware method signature.The overloaded method signature correctly accepts workspace ID parameter for scenarios where no case context is available.
49-49: LGTM: Workspace context propagation to actions runner.The method signature includes workspace ID parameter and correctly propagates it to the actionsRunner.run call with appropriate conditional logic for workspace resolution.
Also applies to: 55-55
70-70: LGTM: Consistent workspace handling in event actions.The method signature and implementation consistently handle workspace context propagation to the actions runner with the same conditional logic pattern.
Also applies to: 76-76
104-104: LGTM: Workspace propagation in data event processing.The method correctly passes the case's workspace ID when delegating to runEventActions, maintaining workspace context throughout data event processing.
application-engine/src/main/java/com/netgrif/application/engine/petrinet/service/ProcessRoleService.java (1)
216-216:❓ Verification inconclusive
Verify workspace ID retrieval for system operations.
The method uses
userService.getLoggedOrSystem().getWorkspaceId()which might not have a workspace ID when running as system user during startup or background processes.Run this script to verify how the system user's workspace ID is handled:
🏁 Script executed:
#!/bin/bash # Description: Check how system user's workspace ID is handled in the codebase # Search for getLoggedOrSystem usage and workspace ID handling rg -A 5 "getLoggedOrSystem\(\)\.getWorkspaceId\(\)" # Check if system user has workspace ID initialization ast-grep --pattern 'class $_ { $$$ getLoggedOrSystem() { $$$ } $$$ }'Length of output: 93354
I’ll locate the
UserServicedefinition and thegetLoggedOrSystemmethod to see how the system user’s workspace ID is set.
🏁 Script executed:
#!/bin/bash # Find where UserService is declared rg -n "interface UserService" -g "application-engine/src/main/java/**/*.java" rg -n "class UserService" -g "application-engine/src/main/java/**/*.java" # Locate the getLoggedOrSystem method implementation rg -n "getLoggedOrSystem" -g "application-engine/src/main/java/**/*.java"Length of output: 11622
I’ll inspect the
IUserServiceinterface and theUserServicearound thegetSystemandgetLoggedOrSystemstubs to see how the system user’s workspace ID is initialized.
🏁 Script executed:
#!/bin/bash # 1. Show the interface definition for IUserService sed -n '1,100p' application-engine/src/main/java/com/netgrif/application/engine/auth/service/interfaces/IUserService.java # 2. Find getSystem in the UserService implementation rg -n "getSystem" -g "application-engine/src/main/java/com/netgrif/application/engine/auth/service/UserService.java" # 3. Show context around the commented getLoggedOrSystem stub sed -n '240,300p' application-engine/src/main/java/com/netgrif/application/engine/auth/service/UserService.javaLength of output: 6205
Gathering how the system user’s workspace is initialized:
🏁 Script executed:
#!/bin/bash # Locate the SystemUserRunner class rg -n "class SystemUserRunner" -g "application-engine/src/main/java/**/*.java" # Show its contents to see how SYSTEM_USER_EMAIL user and workspace are set sed -n '1,200p' application-engine/src/main/java/com/netgrif/application/engine/startup/runner/SystemUserRunner.javaLength of output: 1998
Inspecting how workspace ID is propagated in
transformToLoggedUser:
🏁 Script executed:
#!/bin/bash # Locate transformToLoggedUser in UserService rg -n "transformToLoggedUser" -g "application-engine/src/main/java/com/netgrif/application/engine/auth/service/UserService.java" # Show its implementation context sed -n '200,260p' application-engine/src/main/java/com/netgrif/application/engine/auth/service/UserService.javaLength of output: 3325
To confirm how
getWorkspaceId()is provided on the returned user, let’s inspect theIUserinterface and theLoggedUserclass:
🏁 Script executed:
#!/bin/bash # 1. Show IUser definition (where getWorkspaceId should be declared) sed -n '1,200p' application-engine/src/main/java/com/netgrif/application/engine/objects/auth/domain/IUser.java # 2. Show LoggedUser (to see how workspaceId is stored/transformed) sed -n '1,200p' application-engine/src/main/java/com/netgrif/application/engine/objects/auth/domain/LoggedUser.javaLength of output: 507
Inspecting how the system user is created and its workspace ID is set:
🏁 Script executed:
#!/bin/bash # Find createSystemUser implementation in UserService rg -n "createSystemUser" -g "application-engine/src/main/java/com/netgrif/application/engine/auth/service/UserService.java" # Show the method body around createSystemUser sed -n '300,360p' application-engine/src/main/java/com/netgrif/application/engine/auth/service/UserService.javaLength of output: 2202
I’ll locate the
createSystemUserimplementation and theIUser/LoggedUserclasses to confirm how the workspace ID is set:
🏁 Script executed:
#!/bin/bash # Find createSystemUser definition rg -n "createSystemUser" -g "*.java" # Locate IUser interface rg -n "interface IUser" -g "application-engine/src/main/java/**/*.java" # Locate LoggedUser class rg -n "class LoggedUser" -g "application-engine/src/main/java/**/*.java"Length of output: 2091
Verify system user’s workspace ID initialization
The call to
userService.getLoggedOrSystem().getWorkspaceId()falls back togetSystem()when no user is authenticated, which loads the system user by email. Please confirm that the system user created byUserServiceImpl.createSystemUser()has itsworkspaceIdfield initialized (and persisted) before any background or startup processes invoke workspace-scoped operations.• Check
UserServiceImpl.createSystemUser()innae-user-ce/src/.../UserServiceImpl.javato ensure it sets a validworkspaceId.
• If it does not, initialize the default workspace on the system user before saving.application-engine/src/main/java/com/netgrif/application/engine/petrinet/service/PetriNetService.java (3)
234-239: Good workspace validation implementation!The validation ensures that workspace ID is provided and properly constructs the URI with workspace context.
388-390: Good use of aggregation with workspace filtering!The aggregation query properly includes workspace ID in the match criteria, ensuring workspace isolation.
291-295:✅ Verification successful
Verify AsyncRunnerWrapper properly propagates workspace context.
The async deletion uses workspace ID parameter, but we need to ensure the context is properly maintained in async execution.
Run this script to verify AsyncRunnerWrapper implementation:
🏁 Script executed:
#!/bin/bash # Description: Verify AsyncRunnerWrapper properly handles workspace context # Find AsyncRunnerWrapper implementation fd -e java "AsyncRunnerWrapper" # Check how it handles workspace context ast-grep --pattern 'class AsyncRunnerWrapper { $$$ execute($$$, String workspaceId) { $$$ } $$$ }' # Check if it sets SecurityContext or MDC with workspace rg -A 10 "class AsyncRunnerWrapper"Length of output: 1648
No action needed: AsyncRunnerWrapper propagates workspace context
The
run(Closure, String)method in AsyncRunnerWrapper invokes
WorkspaceContextHolder.setWorkspaceId(workspaceId, true)
before callingasync.run(…), ensuring the workspace ID is correctly set in the async execution.
| @@ -23,6 +23,8 @@ public abstract class LoggedUser implements Serializable { | |||
|
|
|||
| private String realmId; | |||
|
|
|||
| private String workspaceId; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Missing initialization of workspaceId in explicit constructors
You’ve added private String workspaceId; but the two custom constructors don’t set it, and toString() omits it. This will leave the field null for most instances. Please update both constructors to accept and assign workspaceId, and include it in your toString().
🤖 Prompt for AI Agents
In
nae-object-library/src/main/java/com/netgrif/application/engine/objects/auth/domain/LoggedUser.java
at line 26, the new field workspaceId is declared but not initialized in the two
explicit constructors, and it is missing from the toString() method. Update both
constructors to add a parameter for workspaceId and assign it to the field.
Also, modify the toString() method to include workspaceId in its output.
| # Default petrinets import | ||
| nae.petrinet.resources.default-processes=file:src/main/resources/petriNets/engine-processes/filter.xml,file:src/main/resources/petriNets/engine-processes/preference_item.xml,file:src/main/resources/petriNets/engine-processes/import_filters.xml,file:src/main/resources/petriNets/engine-processes/export_filters.xml,file:src/main/resources/petriNets/engine-processes/impersonation_config.xml,file:src/main/resources/petriNets/engine-processes/impersonation_users_select.xml,file:src/main/resources/petriNets/engine-processes/dashboard.xml,file:src/main/resources/petriNets/engine-processes/dashboard_tile.xml |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify file paths and consider externalizing configuration.
The new configuration property for default processes looks good for workspace-aware imports. However, consider the following:
- File path validation: Ensure all referenced XML files exist at the specified locations
- Environment flexibility: The hardcoded "file:src/main/resources/" paths may not work in all deployment environments (e.g., when deployed as JAR)
- Configuration externalization: Consider making this configurable per environment
Consider using classpath references instead of file paths for better portability:
-nae.petrinet.resources.default-processes=file:src/main/resources/petriNets/engine-processes/filter.xml,file:src/main/resources/petriNets/engine-processes/preference_item.xml,file:src/main/resources/petriNets/engine-processes/import_filters.xml,file:src/main/resources/petriNets/engine-processes/export_filters.xml,file:src/main/resources/petriNets/engine-processes/impersonation_config.xml,file:src/main/resources/petriNets/engine-processes/impersonation_users_select.xml,file:src/main/resources/petriNets/engine-processes/dashboard.xml,file:src/main/resources/petriNets/engine-processes/dashboard_tile.xml
+nae.petrinet.resources.default-processes=classpath:petriNets/engine-processes/filter.xml,classpath:petriNets/engine-processes/preference_item.xml,classpath:petriNets/engine-processes/import_filters.xml,classpath:petriNets/engine-processes/export_filters.xml,classpath:petriNets/engine-processes/impersonation_config.xml,classpath:petriNets/engine-processes/impersonation_users_select.xml,classpath:petriNets/engine-processes/dashboard.xml,classpath:petriNets/engine-processes/dashboard_tile.xml🏁 Script executed:
#!/bin/bash
# Verify that all referenced XML files exist
echo "Checking if default process files exist..."
files=(
"application-engine/src/main/resources/petriNets/engine-processes/filter.xml"
"application-engine/src/main/resources/petriNets/engine-processes/preference_item.xml"
"application-engine/src/main/resources/petriNets/engine-processes/import_filters.xml"
"application-engine/src/main/resources/petriNets/engine-processes/export_filters.xml"
"application-engine/src/main/resources/petriNets/engine-processes/impersonation_config.xml"
"application-engine/src/main/resources/petriNets/engine-processes/impersonation_users_select.xml"
"application-engine/src/main/resources/petriNets/engine-processes/dashboard.xml"
"application-engine/src/main/resources/petriNets/engine-processes/dashboard_tile.xml"
)
for file in "${files[@]}"; do
if [ -f "$file" ]; then
echo "✓ Found: $file"
else
echo "✗ Missing: $file"
fi
doneLength of output: 3376
Use classpath-based Spring Resource references for default Petri net processes
- Verified that all 8 XML files exist under
application‐engine/src/main/resources/petriNets/engine-processes/. - Loading via
file:src/...will work in the IDE but fail once packaged as a JAR/WAR. - Switch to
classpath:prefix so Spring can load them from the classpath at runtime. - Consider externalizing this property per environment (profiles, env vars, etc.) for greater flexibility.
Proposed change:
- nae.petrinet.resources.default-processes=
- file:src/main/resources/petriNets/engine-processes/filter.xml,
- file:src/main/resources/petriNets/engine-processes/preference_item.xml,
- file:src/main/resources/petriNets/engine-processes/import_filters.xml,
- file:src/main/resources/petriNets/engine-processes/export_filters.xml,
- file:src/main/resources/petriNets/engine-processes/impersonation_config.xml,
- file:src/main/resources/petriNets/engine-processes/impersonation_users_select.xml,
- file:src/main/resources/petriNets/engine-processes/dashboard.xml,
- file:src/main/resources/petriNets/engine-processes/dashboard_tile.xml
+ nae.petrinet.resources.default-processes=
+ classpath:petriNets/engine-processes/filter.xml,
+ classpath:petriNets/engine-processes/preference_item.xml,
+ classpath:petriNets/engine-processes/import_filters.xml,
+ classpath:petriNets/engine-processes/export_filters.xml,
+ classpath:petriNets/engine-processes/impersonation_config.xml,
+ classpath:petriNets/engine-processes/impersonation_users_select.xml,
+ classpath:petriNets/engine-processes/dashboard.xml,
+ classpath:petriNets/engine-processes/dashboard_tile.xmlCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In application-engine/src/main/resources/application.properties around lines 203
to 204, the property nae.petrinet.resources.default-processes uses hardcoded
file system paths with the "file:" prefix, which will fail when running from a
packaged JAR/WAR. Change these paths to use the "classpath:" prefix instead to
enable Spring to load the XML resources from the classpath at runtime.
Additionally, consider externalizing this property using Spring profiles or
environment variables to allow configuration per deployment environment.
| @@ -34,6 +33,7 @@ abstract class RoleActionsRunner { | |||
| log.debug("Action: $action") | |||
| def code = getActionCode(action) | |||
| try { | |||
| WorkspaceContextHolder.setWorkspaceId(roleContext.petriNet.getWorkspaceId(), true); | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure workspace context cleanup after action execution.
The workspace context is properly set before action execution, but there's no visible cleanup mechanism. Consider wrapping the action execution in a try-finally block to ensure the workspace context is cleared.
try {
WorkspaceContextHolder.setWorkspaceId(roleContext.petriNet.getWorkspaceId(), true);
code.init(action, roleContext, params)
code()
+} finally {
+ WorkspaceContextHolder.clear()
}Also verify the true parameter usage - ensure this aligns with the intended workspace context behavior across the application.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
application-engine/src/main/groovy/com/netgrif/application/engine/petrinet/domain/dataset/logic/action/runner/RoleActionsRunner.groovy
at line 36, the workspace context is set but not cleared after action execution.
Wrap the action execution code in a try-finally block where you set the
workspace context before the try, execute the action inside the try, and clear
the workspace context in the finally block to ensure cleanup. Also, review the
usage of the 'true' parameter in setWorkspaceId to confirm it matches the
intended behavior for workspace context handling in the application.
| @@ -40,7 +40,7 @@ class DynamicDefaultValueTest { | |||
|
|
|||
| @Test | |||
| void testInitValues() { | |||
| ImportPetriNetEventOutcome optNet = petriNetService.importPetriNet(new FileInputStream("src/test/resources/petriNets/dynamic_init.xml"), VersionType.MAJOR, superCreator.getLoggedSuper()); | |||
| ImportPetriNetEventOutcome optNet = petriNetService.importPetriNet(new FileInputStream("src/test/resources/petriNets/dynamic_init.xml"), VersionType.MAJOR, superCreator.getLoggedSuper(), userService.getLoggedOrSystem().getWorkspaceId()); | |||
There was a problem hiding this comment.
Missing userService import and potential null safety issue.
Similar to other test files, this code references userService.getLoggedOrSystem().getWorkspaceId() without a visible import. Ensure the import is present and consider null safety.
The missing import will cause compilation failure. Add the import:
+import com.netgrif.application.engine.auth.service.interfaces.IUserServiceAnd inject the service:
+ @Autowired
+ private IUserService userServiceCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
application-engine/src/test/groovy/com/netgrif/application/engine/petrinet/domain/dataset/DynamicDefaultValueTest.groovy
at line 43, the code uses userService without importing it, causing compilation
errors. Add the appropriate import statement for userService at the top of the
file and inject the userService bean into the test class to ensure it is
available. Also, add null checks or safe navigation operators when calling
getLoggedOrSystem() and getWorkspaceId() to prevent potential null pointer
exceptions.
| @@ -50,7 +50,7 @@ class ReindexTest { | |||
|
|
|||
| @Test | |||
| void reindexTest() { | |||
| ImportPetriNetEventOutcome net = petriNetService.importPetriNet(new FileInputStream("src/test/resources/all_data.xml"), VersionType.MAJOR, superCreator.getLoggedSuper()) | |||
| ImportPetriNetEventOutcome net = petriNetService.importPetriNet(new FileInputStream("src/test/resources/all_data.xml"), VersionType.MAJOR, superCreator.getLoggedSuper(), userService.getLoggedOrSystem().getWorkspaceId()) | |||
There was a problem hiding this comment.
Consistent workspace integration but missing userService dependency.
The workspace ID parameter addition is consistent with other test files in this PR. However, like the other test files, this appears to be missing the userService import and injection.
Add the missing import and service injection:
+import com.netgrif.application.engine.auth.service.interfaces.IUserService
@ActiveProfiles(["test"])
@SpringBootTest
@ExtendWith(SpringExtension.class)
class ReindexTest {
+
+ @Autowired
+ private IUserService userService🤖 Prompt for AI Agents
In
application-engine/src/test/groovy/com/netgrif/application/engine/elastic/ReindexTest.groovy
at line 53, the code uses userService to get the workspace ID but the
userService dependency is not imported or injected. To fix this, add the
appropriate import statement for userService at the top of the file and inject
the userService bean into the test class, ensuring it is available for use in
the method.
| Page<Task> findByTransitionIdInAndWorkspaceId(Pageable pageable, Collection<String> ids, String workspaceId); | ||
|
|
||
| List<Task> findAllByTransitionIdIn(Collection<String> ids); | ||
| List<Task> findAllByTransitionIdInAndWorkspaceId(Collection<String> ids, String workspaceId); | ||
|
|
||
| Page<Task> findByUserId(Pageable pageable, String userId); | ||
|
|
||
| List<Task> findByUserIdAndFinishDateNotNull(Long userId); | ||
| List<Task> findByUserIdAndWorkspaceIdAndFinishDateNotNull(Long userId, String workspaceId); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify data migration for workspace-aware queries.
The repository methods now filter by workspaceId, which could return empty results if existing data doesn't have this field populated. Ensure that a data migration strategy is in place to populate workspace IDs for existing tasks.
Run the following script to check if there are tasks without workspace IDs:
🏁 Script executed:
#!/bin/bash
# Description: Check for tasks that might not have workspaceId populated
# Search for MongoDB migration scripts that handle workspaceId
fd -e js -e json -e yaml -e yml | xargs rg -l "workspaceId.*migration|migrate.*workspace" -A 5
# Check if there's a default workspace assignment in Task entity
ast-grep --pattern 'class Task {
$$$
workspaceId $$$
$$$
}'Length of output: 164
🏁 Script executed:
#!/bin/bash
# Search for Java-based migrations annotated with @ChangeLog/@ChangeSet
echo "==== Migration classes ===="
rg -n "@ChangeLog" -G "*.java" || echo "No @ChangeLog annotations found"
rg -n "@ChangeSet" -G "*.java" || echo "No @ChangeSet annotations found"
# If any migrations are found, check if they reference workspaceId
echo "==== workspaceId in migrations ===="
rg -l "@ChangeLog" -G "*.java" | xargs -r rg -n "workspaceId" || echo "No workspaceId in @ChangeLog classes"
rg -l "@ChangeSet" -G "*.java" | xargs -r rg -n "workspaceId" || echo "No workspaceId in @ChangeSet classes"Length of output: 2093
🏁 Script executed:
#!/bin/bash
# Search for Mongock/Java-based migrations
echo "=== Checking Java migrations for @ChangeLog/@ChangeSet ==="
migration_files=$(rg -l "@ChangeLog|@ChangeSet" -g "*.java" || true)
if [ -z "$migration_files" ]; then
echo "No Java migrations found (@ChangeLog/@ChangeSet)."
else
for file in $migration_files; do
echo "File: $file"
rg -n "workspaceId" "$file" || echo " → No workspaceId references in this file."
done
fi
# Search for Liquibase XML changeLogs
echo "=== Checking Liquibase XML changeLog files ==="
xml_files=$(rg -l "<databaseChangeLog" -g "*.xml" || true)
if [ -z "$xml_files" ]; then
echo "No Liquibase XML changelogs found."
else
for xf in $xml_files; do
echo "File: $xf"
rg -n "workspaceId" "$xf" || echo " → No workspaceId references in this file."
done
fiLength of output: 583
🏁 Script executed:
#!/bin/bash
# Locate the Task entity class (excluding repositories)
task_file=$(rg -l "class Task" -g "*.java" | grep -v "TaskRepository" | head -n1)
if [ -z "$task_file" ]; then
echo "❌ Task entity class not found."
exit 1
fi
echo "Found Task entity: $task_file"
echo "---- workspaceId field and surrounding context ----"
rg -n "workspaceId" -C3 "$task_file"Length of output: 2315
Add migration to backfill workspaceId for existing Tasks
The new repository queries filter on workspaceId, but no existing migration was found to populate this field for pre‐existing Task records. Without a backfill, queries may return empty results for legacy data.
Please:
- Introduce a data migration (e.g., Mongock/ChangeLog, Liquibase/Flyway) that sets
workspaceIdon all existing Task entities/documents. - Consider adding a default or fallback strategy in the Task entity or repository layer to handle any null or missing
workspaceIdvalues. - Verify the migration runs before these new queries execute in production.
Relevant locations:
- Task entity:
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/workflow/domain/Task.java - TaskRepository:
application-engine/src/main/java/com/netgrif/application/engine/workflow/domain/repositories/TaskRepository.java
🤖 Prompt for AI Agents
In
application-engine/src/main/java/com/netgrif/application/engine/workflow/domain/repositories/TaskRepository.java
around lines 26 to 32, the repository methods now filter by workspaceId but no
migration exists to backfill this field for existing Task records. To fix this,
create a data migration script (using Mongock/ChangeLog or Liquibase/Flyway)
that updates all existing Task entities/documents to set their workspaceId
appropriately. Additionally, implement a default or fallback mechanism in the
Task entity or repository layer to handle cases where workspaceId might still be
null. Ensure this migration runs before deploying the new queries to production.
| private HttpServletRequest getServletRequest() { | ||
| return ((ServletRequestAttributes) Objects.requireNonNull(RequestContextHolder.getRequestAttributes())).getRequest(); | ||
| } | ||
|
|
||
| private HttpServletResponse getServletResponse() { | ||
| return ((ServletRequestAttributes) Objects.requireNonNull(RequestContextHolder.getRequestAttributes())).getResponse(); | ||
| } |
There was a problem hiding this comment.
Handle cases where RequestContextHolder might not be available.
The current implementation will throw NullPointerException in async contexts or background jobs where RequestContextHolder doesn't have request attributes. Consider adding graceful error handling.
Add null checks and fallback behavior:
private HttpServletRequest getServletRequest() {
- return ((ServletRequestAttributes) Objects.requireNonNull(RequestContextHolder.getRequestAttributes())).getRequest();
+ ServletRequestAttributes attributes = (ServletRequestAttributes) RequestContextHolder.getRequestAttributes();
+ if (attributes == null) {
+ log.warn("No request attributes available in current context");
+ return null;
+ }
+ return attributes.getRequest();
}
private HttpServletResponse getServletResponse() {
- return ((ServletRequestAttributes) Objects.requireNonNull(RequestContextHolder.getRequestAttributes())).getResponse();
+ ServletRequestAttributes attributes = (ServletRequestAttributes) RequestContextHolder.getRequestAttributes();
+ if (attributes == null) {
+ log.warn("No request attributes available in current context");
+ return null;
+ }
+ return attributes.getResponse();
}Then update the calling code to handle null values:
- securityContextRepository.saveContext(context, getServletRequest(), getServletResponse());
+ HttpServletRequest request = getServletRequest();
+ HttpServletResponse response = getServletResponse();
+ if (request != null && response != null) {
+ securityContextRepository.saveContext(context, request, response);
+ } else {
+ log.debug("Skipping security context save - no HTTP request/response available");
+ }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
application-engine/src/main/java/com/netgrif/application/engine/security/service/SecurityContextService.java
around lines 106 to 112, the methods getServletRequest and getServletResponse
assume RequestContextHolder always has request attributes, which can cause
NullPointerException in async or background contexts. Modify these methods to
check if RequestContextHolder.getRequestAttributes() returns null and return
null or an appropriate fallback instead of throwing an exception. Then update
any calling code to handle possible null values safely.
| if (loggedUser.getWorkspaceId() == null) { | ||
| loggedUser.setWorkspaceId(defaultWorkspaceService.getDefaultWorkspace().getId()); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Move workspace initialization outside the cached token check.
The workspace ID initialization should happen for all logged users, not just those with cached tokens. This ensures consistent workspace assignment.
Consider moving the workspace initialization before the if statement:
private void reloadSecurityContext(LoggedUser loggedUser, boolean forceRefresh) {
+ if (loggedUser.getWorkspaceId() == null) {
+ loggedUser.setWorkspaceId(defaultWorkspaceService.getDefaultWorkspace().getId());
+ }
if (isUserLogged(loggedUser) && cachedTokens.contains(loggedUser.getId())) {
- if (loggedUser.getWorkspaceId() == null) {
- loggedUser.setWorkspaceId(defaultWorkspaceService.getDefaultWorkspace().getId());
- }📝 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.
| if (loggedUser.getWorkspaceId() == null) { | |
| loggedUser.setWorkspaceId(defaultWorkspaceService.getDefaultWorkspace().getId()); | |
| } | |
| private void reloadSecurityContext(LoggedUser loggedUser, boolean forceRefresh) { | |
| // Always ensure workspaceId is set, before any token-based logic | |
| if (loggedUser.getWorkspaceId() == null) { | |
| loggedUser.setWorkspaceId(defaultWorkspaceService.getDefaultWorkspace().getId()); | |
| } | |
| if (isUserLogged(loggedUser) && cachedTokens.contains(loggedUser.getId())) { | |
| // Removed redundant workspaceId check here | |
| // ... existing logic for refreshing security context ... | |
| } | |
| // ... remainder of method ... | |
| } |
🤖 Prompt for AI Agents
In
application-engine/src/main/java/com/netgrif/application/engine/security/service/SecurityContextService.java
around lines 82 to 84, the workspace ID initialization is currently inside a
conditional block that checks for cached tokens. To ensure all logged users have
their workspace ID initialized consistently, move the block that sets the
workspace ID from inside the cached token check to before it. This way, the
workspace ID is assigned regardless of token caching.
| log.info("Creating " + predefinedRole.getName() + " process role in workspace " + workspaceId); | ||
|
|
||
| Set<ProcessRole> role = processRoleRepository.findAllByImportIdAndWorkspaceId(predefinedRole.getName(), workspaceId); | ||
| if (role != null && !role.isEmpty()) { | ||
| log.info("Role already exists"); | ||
| return role.stream().findFirst().orElse(null); | ||
| } | ||
|
|
||
| ProcessRole newRole = new com.netgrif.application.engine.adapter.spring.petrinet.domain.roles.ProcessRole(); | ||
| newRole.setImportId(predefinedRole.getName()); | ||
| newRole.setName(new I18nString(predefinedRole.getName())); | ||
| newRole.setDescription(predefinedRole.getDescription()); | ||
| newRole.setEvents(new LinkedHashMap<EventType, Event>()); | ||
| newRole.setWorkspaceId(workspaceId); | ||
| newRole = processRoleRepository.save(newRole); | ||
|
|
||
| return newRole; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add null checks and improve error handling.
The method should validate input parameters and handle edge cases more robustly.
Apply this diff to add validation and improve error handling:
public ProcessRole createDefaultOrAnonymousRole(PredefinedProcessRole predefinedRole, String workspaceId) {
+ if (predefinedRole == null) {
+ throw new IllegalArgumentException("PredefinedProcessRole cannot be null");
+ }
+ if (workspaceId == null || workspaceId.isEmpty()) {
+ throw new IllegalArgumentException("WorkspaceId cannot be null or empty");
+ }
+
log.info("Creating " + predefinedRole.getName() + " process role in workspace " + workspaceId);
Set<ProcessRole> role = processRoleRepository.findAllByImportIdAndWorkspaceId(predefinedRole.getName(), workspaceId);
if (role != null && !role.isEmpty()) {
log.info("Role already exists");
return role.stream().findFirst().orElse(null);
}
ProcessRole newRole = new com.netgrif.application.engine.adapter.spring.petrinet.domain.roles.ProcessRole();
newRole.setImportId(predefinedRole.getName());
newRole.setName(new I18nString(predefinedRole.getName()));
newRole.setDescription(predefinedRole.getDescription());
- newRole.setEvents(new LinkedHashMap<EventType, Event>());
+ newRole.setEvents(new LinkedHashMap<>());
newRole.setWorkspaceId(workspaceId);
- newRole = processRoleRepository.save(newRole);
-
- return newRole;
+
+ try {
+ return processRoleRepository.save(newRole);
+ } catch (Exception e) {
+ log.error("Failed to create " + predefinedRole.getName() + " role in workspace " + workspaceId, e);
+ throw new RuntimeException("Failed to create predefined role", e);
+ }
}📝 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.
| log.info("Creating " + predefinedRole.getName() + " process role in workspace " + workspaceId); | |
| Set<ProcessRole> role = processRoleRepository.findAllByImportIdAndWorkspaceId(predefinedRole.getName(), workspaceId); | |
| if (role != null && !role.isEmpty()) { | |
| log.info("Role already exists"); | |
| return role.stream().findFirst().orElse(null); | |
| } | |
| ProcessRole newRole = new com.netgrif.application.engine.adapter.spring.petrinet.domain.roles.ProcessRole(); | |
| newRole.setImportId(predefinedRole.getName()); | |
| newRole.setName(new I18nString(predefinedRole.getName())); | |
| newRole.setDescription(predefinedRole.getDescription()); | |
| newRole.setEvents(new LinkedHashMap<EventType, Event>()); | |
| newRole.setWorkspaceId(workspaceId); | |
| newRole = processRoleRepository.save(newRole); | |
| return newRole; | |
| } | |
| public ProcessRole createDefaultOrAnonymousRole(PredefinedProcessRole predefinedRole, String workspaceId) { | |
| if (predefinedRole == null) { | |
| throw new IllegalArgumentException("PredefinedProcessRole cannot be null"); | |
| } | |
| if (workspaceId == null || workspaceId.isEmpty()) { | |
| throw new IllegalArgumentException("WorkspaceId cannot be null or empty"); | |
| } | |
| log.info("Creating " + predefinedRole.getName() + " process role in workspace " + workspaceId); | |
| Set<ProcessRole> role = processRoleRepository.findAllByImportIdAndWorkspaceId(predefinedRole.getName(), workspaceId); | |
| if (role != null && !role.isEmpty()) { | |
| log.info("Role already exists"); | |
| return role.stream().findFirst().orElse(null); | |
| } | |
| ProcessRole newRole = new com.netgrif.application.engine.adapter.spring.petrinet.domain.roles.ProcessRole(); | |
| newRole.setImportId(predefinedRole.getName()); | |
| newRole.setName(new I18nString(predefinedRole.getName())); | |
| newRole.setDescription(predefinedRole.getDescription()); | |
| newRole.setEvents(new LinkedHashMap<>()); | |
| newRole.setWorkspaceId(workspaceId); | |
| try { | |
| return processRoleRepository.save(newRole); | |
| } catch (Exception e) { | |
| log.error("Failed to create " + predefinedRole.getName() + " role in workspace " + workspaceId, e); | |
| throw new RuntimeException("Failed to create predefined role", e); | |
| } | |
| } |
🤖 Prompt for AI Agents
In
application-engine/src/main/java/com/netgrif/application/engine/petrinet/service/ProcessRoleService.java
around lines 403 to 420, the method lacks validation for input parameters and
robust error handling. Add null checks for predefinedRole and workspaceId at the
start of the method, throwing appropriate exceptions if they are null. Also,
ensure that the processRoleRepository call handles potential null or empty
results gracefully. This will prevent NullPointerExceptions and improve the
method's reliability.
| nets.add(existingNets.getFirst()); | ||
| } else { | ||
| VersionType release = VersionType.MAJOR; | ||
| LoggedUser author = userService.getLoggedOrSystem().transformToLoggedUser(); | ||
| String uriNodeId = uriService.getDefault().getStringId(); | ||
| nets.add(importPetriNet(netStream, release, author, uriNodeId, workspaceId).getNet()); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Handle resource loading exceptions more gracefully.
The current implementation logs errors but continues processing, which might hide critical issues.
Improve error handling:
} catch (IOException e) {
- log.info("Default Petri net import failed, file {} doesn't exists", resource.getFilename());
+ log.error("Failed to load default Petri net from file: {}", resource.getFilename(), e);
+ // Consider collecting failed imports and returning them or throwing an exception
+ continue;
} catch (MissingPetriNetMetaDataException e) {
- throw new RuntimeException(e);
+ log.error("Invalid Petri net metadata in file: {}", resource.getFilename(), e);
+ throw new RuntimeException("Failed to import default process: " + resource.getFilename(), e);
}📝 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.
| nets.add(existingNets.getFirst()); | |
| } else { | |
| VersionType release = VersionType.MAJOR; | |
| LoggedUser author = userService.getLoggedOrSystem().transformToLoggedUser(); | |
| String uriNodeId = uriService.getDefault().getStringId(); | |
| nets.add(importPetriNet(netStream, release, author, uriNodeId, workspaceId).getNet()); | |
| } | |
| } catch (IOException e) { | |
| - log.info("Default Petri net import failed, file {} doesn't exists", resource.getFilename()); | |
| + log.error("Failed to load default Petri net from file: {}", resource.getFilename(), e); | |
| + // Consider collecting failed imports and returning them or throwing an exception | |
| + continue; | |
| } catch (MissingPetriNetMetaDataException e) { | |
| - throw new RuntimeException(e); | |
| + log.error("Invalid Petri net metadata in file: {}", resource.getFilename(), e); | |
| + throw new RuntimeException("Failed to import default process: " + resource.getFilename(), e); | |
| } |
🤖 Prompt for AI Agents
In
application-engine/src/main/java/com/netgrif/application/engine/petrinet/service/PetriNetService.java
around lines 273 to 279, the current code logs errors during resource loading
but continues processing, potentially hiding critical issues. Modify the error
handling to catch exceptions explicitly, log the error with detailed context,
and then rethrow the exception or handle it in a way that prevents further
processing with invalid data. This ensures that critical failures are not
silently ignored and are properly propagated or managed.
# Conflicts: # nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/workflow/domain/Task.java
Description
Implements NAE-2072
Dependencies
Third party dependencies
No new dependencies were introduced
Blocking Pull requests
<Depends on #(PR id)>/
How Has Been This Tested?
manually
Test Configuration
<Please describe configuration for tests to run if applicable, like program parameters, host OS, VM configuration etc.>
Checklist:
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores
Documentation