Add nodeActivity#1016
Open
ayolab wants to merge 1 commit into
Open
Conversation
Signed-off-by: Ayoub LABIDI <ayoub.labidi@protonmail.com>
📝 WalkthroughWalkthroughThis PR replaces a boolean "blockedNode" model with a new ChangesNode activity status migration
Sequence Diagram(s)sequenceDiagram
participant StudyController
participant StudyService
participant RootNetworkNodeInfoService
participant NotificationService
StudyController->>StudyService: assertTreeIsIdle(nodeUuid, rootNetworkUuid)
StudyService->>RootNetworkNodeInfoService: setNodeActivity(COMPUTATION_RUNNING)
RootNetworkNodeInfoService->>NotificationService: emitNodeActivityStatusUpdated
StudyService->>StudyService: runComputation(action)
StudyService->>RootNetworkNodeInfoService: clearNodeActivity
RootNetworkNodeInfoService->>NotificationService: emitNodeActivityStatusUpdated(IDLE)
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.44.1)src/main/java/org/gridsuite/study/server/controller/StudyController.javaast-grep timed out on this file src/main/java/org/gridsuite/study/server/service/ConsumerService.javaast-grep retry budget exhausted before isolating this batch src/main/java/org/gridsuite/study/server/service/NetworkModificationTreeService.javaast-grep retry budget exhausted before isolating this batch
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/main/java/org/gridsuite/study/server/service/StudyService.java (1)
1770-1782: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
getBuildInfos/setModificationReportsrun afterBUILDINGis set but outside thetry.If
getBuildInfosorsetModificationReportsthrows,clearNodeActivityis not invoked. DB state reverts via the caller's transaction, but theBUILDINGactivity-status notification emitted bysetNodeActivityis not rolled back, so clients can be left seeing a stuckBUILDINGstate. Consider extending thetryto cover these steps.♻️ Suggested scope widening
setNodeActivity(studyUuid, rootNetworkUuid, nodeUuid, NodeActivityStatus.BUILDING); - BuildInfos buildInfos = networkModificationTreeService.getBuildInfos(nodeUuid, rootNetworkUuid); - - // Store all reports (inherited + new) for this node - networkModificationTreeService.setModificationReports(nodeUuid, rootNetworkUuid, buildInfos.getAllReportsAsMap()); try { + BuildInfos buildInfos = networkModificationTreeService.getBuildInfos(nodeUuid, rootNetworkUuid); + // Store all reports (inherited + new) for this node + networkModificationTreeService.setModificationReports(nodeUuid, rootNetworkUuid, buildInfos.getAllReportsAsMap()); networkModificationService.buildNode(nodeUuid, rootNetworkUuid, buildInfos, workflowInfos); } catch (Exception e) { clearNodeActivity(studyUuid, rootNetworkUuid, nodeUuid); throw e; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/org/gridsuite/study/server/service/StudyService.java` around lines 1770 - 1782, The node activity can get stuck in BUILDING because getBuildInfos and setModificationReports run after setNodeActivity but outside the guarded failure path. Move the build-info retrieval and modification-report update into the same try/catch used around networkModificationService.buildNode in StudyService so any exception triggers clearNodeActivity. Keep the existing cleanup logic in the catch and make sure the BUILDING status is only emitted once the whole build preparation and execution flow is safely covered.src/main/java/org/gridsuite/study/server/dto/InvalidateNodeTreeParameters.java (1)
21-25: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDeclare these shared presets as
final.These are public shared constant instances but are only
static, so they can be accidentally reassigned by any caller, silently corrupting invalidation behavior everywhere. Mark themfinal.♻️ Proposed change
- public static InvalidateNodeTreeParameters ALL = new InvalidateNodeTreeParameters(InvalidationMode.ALL, ComputationsInvalidationMode.ALL); + public static final InvalidateNodeTreeParameters ALL = new InvalidateNodeTreeParameters(InvalidationMode.ALL, ComputationsInvalidationMode.ALL); `@SuppressWarnings`("checkstyle:AbbreviationAsWordInName") - public static InvalidateNodeTreeParameters ONLY_CHILDREN = new InvalidateNodeTreeParameters(InvalidationMode.ONLY_CHILDREN, ComputationsInvalidationMode.ALL); + public static final InvalidateNodeTreeParameters ONLY_CHILDREN = new InvalidateNodeTreeParameters(InvalidationMode.ONLY_CHILDREN, ComputationsInvalidationMode.ALL); `@SuppressWarnings`("checkstyle:AbbreviationAsWordInName") - public static InvalidateNodeTreeParameters ONLY_CHILDREN_BUILD_STATUS = new InvalidateNodeTreeParameters(InvalidationMode.ONLY_CHILDREN_BUILD_STATUS, ComputationsInvalidationMode.ALL); + public static final InvalidateNodeTreeParameters ONLY_CHILDREN_BUILD_STATUS = new InvalidateNodeTreeParameters(InvalidationMode.ONLY_CHILDREN_BUILD_STATUS, ComputationsInvalidationMode.ALL);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/org/gridsuite/study/server/dto/InvalidateNodeTreeParameters.java` around lines 21 - 25, The shared preset instances in InvalidateNodeTreeParameters are mutable static fields, so they can be reassigned by callers; make ALL, ONLY_CHILDREN, and ONLY_CHILDREN_BUILD_STATUS final to treat them as true constants. Update the field declarations in InvalidateNodeTreeParameters so these public presets cannot be reassigned, keeping the existing initialization with InvalidationMode and ComputationsInvalidationMode unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/java/org/gridsuite/study/server/service/StudyService.java`:
- Around line 3244-3246: The modification notification flow in StudyService is
unbalanced because setNodeActivity(...) can throw after
emitStartModificationEquipmentNotification() but before the try/finally block
starts, leaving the UI stuck in MODIFICATIONS_UPDATING_IN_PROGRESS. Reorder the
logic in the same method so the UPDATING state is set before emitting the start
notification, then keep the existing try/finally cleanup with
emitEndModificationEquipmentNotification() and clearNodeActivity() to guarantee
the start/end pair always matches.
---
Nitpick comments:
In
`@src/main/java/org/gridsuite/study/server/dto/InvalidateNodeTreeParameters.java`:
- Around line 21-25: The shared preset instances in InvalidateNodeTreeParameters
are mutable static fields, so they can be reassigned by callers; make ALL,
ONLY_CHILDREN, and ONLY_CHILDREN_BUILD_STATUS final to treat them as true
constants. Update the field declarations in InvalidateNodeTreeParameters so
these public presets cannot be reassigned, keeping the existing initialization
with InvalidationMode and ComputationsInvalidationMode unchanged.
In `@src/main/java/org/gridsuite/study/server/service/StudyService.java`:
- Around line 1770-1782: The node activity can get stuck in BUILDING because
getBuildInfos and setModificationReports run after setNodeActivity but outside
the guarded failure path. Move the build-info retrieval and modification-report
update into the same try/catch used around networkModificationService.buildNode
in StudyService so any exception triggers clearNodeActivity. Keep the existing
cleanup logic in the catch and make sure the BUILDING status is only emitted
once the whole build preparation and execution flow is safely covered.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e65c8fcc-dcc3-45b9-b0e4-0dbf2f83a195
📒 Files selected for processing (18)
src/main/java/org/gridsuite/study/server/controller/StudyController.javasrc/main/java/org/gridsuite/study/server/dto/InvalidateNodeTreeParameters.javasrc/main/java/org/gridsuite/study/server/dto/RootNetworkNodeInfo.javasrc/main/java/org/gridsuite/study/server/networkmodificationtree/dto/BuildStatus.javasrc/main/java/org/gridsuite/study/server/networkmodificationtree/dto/NetworkModificationNode.javasrc/main/java/org/gridsuite/study/server/networkmodificationtree/dto/NodeActivityStatus.javasrc/main/java/org/gridsuite/study/server/networkmodificationtree/dto/NodeBuildStatus.javasrc/main/java/org/gridsuite/study/server/networkmodificationtree/entities/RootNetworkNodeInfoEntity.javasrc/main/java/org/gridsuite/study/server/notification/NotificationService.javasrc/main/java/org/gridsuite/study/server/repository/rootnetwork/RootNetworkNodeInfoRepository.javasrc/main/java/org/gridsuite/study/server/service/ConsumerService.javasrc/main/java/org/gridsuite/study/server/service/NetworkModificationTreeService.javasrc/main/java/org/gridsuite/study/server/service/RebuildNodeService.javasrc/main/java/org/gridsuite/study/server/service/RootNetworkNodeInfoService.javasrc/main/java/org/gridsuite/study/server/service/StudyService.javasrc/main/java/org/gridsuite/study/server/service/SupervisionService.javasrc/main/resources/db/changelog/changesets/changelog_20260703T120000Z.xmlsrc/main/resources/db/changelog/db.changelog-master.yaml
💤 Files with no reviewable changes (2)
- src/main/java/org/gridsuite/study/server/networkmodificationtree/dto/NodeBuildStatus.java
- src/main/java/org/gridsuite/study/server/networkmodificationtree/dto/BuildStatus.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.