Skip to content

extract step execution to commons#48

Closed
benrejebmoh wants to merge 40 commits intomainfrom
extract-step-execution-to-commons
Closed

extract step execution to commons#48
benrejebmoh wants to merge 40 commits intomainfrom
extract-step-execution-to-commons

Conversation

@benrejebmoh
Copy link
Contributor

PR Summary

We need to use the step execution service logic in other services, so common behaviour is extracted into the lib monitor-commons

Signed-off-by: benrejebmoh <mohamed.ben-rejeb@rte-france.com>
@coderabbitai
Copy link

coderabbitai bot commented Feb 24, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds publisher-based step lifecycle support: two new publisher interfaces, a new AbstractStepExecutor centralizing RUNNING/COMPLETED/FAILED/SKIPPED transitions, refactors StepExecutionService to use those publishers, and moves ReportInfos to the commons package.

Changes

Cohort / File(s) Summary
New Publisher Interfaces
monitor-commons/src/main/java/org/gridsuite/monitor/commons/steps/ReportPublisher.java, monitor-commons/src/main/java/org/gridsuite/monitor/commons/steps/StepStatusPublisher.java
Adds two functional interfaces: ReportPublisher.sendReport(ReportInfos) and StepStatusPublisher.updateStepStatus(UUID, ProcessExecutionStep).
Abstract Step Executor
monitor-commons/src/main/java/org/gridsuite/monitor/commons/steps/AbstractStepExecutor.java
New abstract class centralizing step lifecycle transitions and invoking the publishers for status updates and reports; includes execute/skip/update logic and injected publishers.
StepExecutionService Refactor
monitor-worker-server/src/main/java/org/gridsuite/monitor/worker/server/services/StepExecutionService.java
Now extends AbstractStepExecutor; replaces direct NotificationService/ReportService usage with publisher wiring, adds publisher accessors, and delegates skip/execute logic to base class.
ReportInfos Package Move
monitor-commons/src/main/java/org/gridsuite/monitor/commons/ReportInfos.java
Moves ReportInfos from org.gridsuite.monitor.worker.server.dto to org.gridsuite.monitor.commons (package-only change).
Import Updates (prod & tests)
monitor-worker-server/src/main/java/.../ProcessStepExecutionContext.java, .../services/ReportService.java, monitor-worker-server/src/test/java/.../*Test.java
Updated imports to reference relocated ReportInfos from the commons package across production and test code.
Maven Dependency
monitor-commons/pom.xml
Adds com.powsybl:powsybl-commons dependency.

Sequence Diagram

sequenceDiagram
    participant Service as StepExecutionService
    participant Executor as AbstractStepExecutor
    participant StatusPub as StepStatusPublisher
    participant ReportPub as ReportPublisher

    Service->>Executor: executeStep(processId, stepId, name, order, startedAt, reportUuid, reportInfos, resultInfos, stepExecution)
    activate Executor

    Executor->>StatusPub: updateStepStatus(..., RUNNING)
    activate StatusPub
    StatusPub-->>Executor: ack
    deactivate StatusPub

    Executor->>Executor: run stepExecution
    alt Success
        Executor->>ReportPub: sendReport(reportInfos)
        activate ReportPub
        ReportPub-->>Executor: ack
        deactivate ReportPub

        Executor->>StatusPub: updateStepStatus(..., COMPLETED)
    else Failure
        Executor->>StatusPub: updateStepStatus(..., FAILED + error)
    end
    activate StatusPub
    StatusPub-->>Executor: ack
    deactivate StatusPub

    deactivate Executor
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Suggested reviewers

  • thangqp

Poem

🐰
I nibble on lines of code so bright,
Publishers hop, set statuses right.
Steps run, skip, complete with cheer,
Reports delivered — carrots near.
A little hop for every build.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'extract step execution to commons' directly and clearly summarizes the main change: moving step execution logic to the shared monitor-commons library for reuse across services.
Description check ✅ Passed The description explains that step execution service logic is being extracted into monitor-commons to enable reuse by other services, which is directly related to the changeset showing new interfaces, abstract executor, and service refactoring.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch extract-step-execution-to-commons

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@benrejebmoh benrejebmoh marked this pull request as draft February 24, 2026 15:49
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (2)
monitor-worker-server/src/main/java/org/gridsuite/monitor/worker/server/services/StepExecutionService.java (1)

68-69: context.getReportInfos() called twice — cache in a local variable.

On Line 68, Objects.requireNonNull(context.getReportInfos()) validates and discards the reference; Line 69 calls it again. Cache the result so the null guard and the value use the same object reference.

♻️ Proposed fix
 public void executeStep(ProcessStepExecutionContext<C> context, ProcessStep<C> step) {
     setExecutionId(context.getProcessExecutionId());
+    ReportInfos reportInfos = Objects.requireNonNull(context.getReportInfos());
     executeStep(
             context.getStepExecutionId(),
             step.getType().getName(),
             context.getStepOrder(),
             context.getStartedAt(),
-            Objects.requireNonNull(context.getReportInfos()).reportUuid(),
-            context.getReportInfos(),
+            reportInfos.reportUuid(),
+            reportInfos,
             context.getResultInfos(),
             () -> step.execute(context)
     );
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@monitor-worker-server/src/main/java/org/gridsuite/monitor/worker/server/services/StepExecutionService.java`
around lines 68 - 69, The code calls context.getReportInfos() twice—once inside
Objects.requireNonNull(...) and again on the next line—so cache the value in a
local variable (e.g., ReportInfos reportInfos =
Objects.requireNonNull(context.getReportInfos())) and then use
reportInfos.reportUuid() and reportInfos for the subsequent arguments; update
the block in StepExecutionService where
Objects.requireNonNull(context.getReportInfos()) and context.getReportInfos()
are used to reference the single cached variable.
monitor-commons/src/main/java/org/gridsuite/monitor/commons/steps/StepExecutionInterface.java (1)

65-68: sendReport inside the try block couples report-delivery failure to step failure.

If stepExecution.run() succeeds but getReportPublisher().sendReport(reportInfos) throws on Line 67, the catch on Line 69 fires and the step is marked FAILED — even though the computation completed successfully. Consider whether report-delivery failure should be treated as a step failure or handled separately (e.g., its own retry/dead-letter flow).

♻️ Proposed separation of concerns
 try {
     stepExecution.run();
-    getReportPublisher().sendReport(reportInfos);
     updateStepStatus(stepExecutionId, stepTypeName, stepOrder, startedAt, reportUuid, resultInfos, StepStatus.COMPLETED);
 } catch (Exception e) {
     try {
         updateStepStatus(stepExecutionId, stepTypeName, stepOrder, startedAt, reportUuid, resultInfos, StepStatus.FAILED);
     } catch (Exception statusEx) {
         e.addSuppressed(statusEx);
     }
     throw e;
 }
+// Report delivery is a separate concern; handle its failure independently.
+getReportPublisher().sendReport(reportInfos);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@monitor-commons/src/main/java/org/gridsuite/monitor/commons/steps/StepExecutionInterface.java`
around lines 65 - 68, The call to getReportPublisher().sendReport(reportInfos)
is inside the try that surrounds stepExecution.run(), so a report-delivery
failure will incorrectly mark the step as FAILED; move or isolate sendReport so
it cannot change step outcome: keep stepExecution.run() and
updateStepStatus(..., StepStatus.COMPLETED) inside the primary try for the
computation, then perform getReportPublisher().sendReport(reportInfos) in a
separate try/catch (or a retry/dead-letter flow) that logs or records
report-delivery errors without calling updateStepStatus with a failure status;
reference stepExecution.run(), getReportPublisher().sendReport(reportInfos),
updateStepStatus(...) and StepStatus.COMPLETED when making this change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@monitor-commons/src/main/java/org/gridsuite/monitor/commons/steps/StepExecutionInterface.java`:
- Around line 65-73: When handling failures in the try/catch around
stepExecution.run(), ensure the original exception from stepExecution.run() is
not lost if updateStepStatus(...) throws: inside the catch(Exception e) block
catch and suppress any exception thrown by updateStepStatus(stepExecutionId,
stepTypeName, stepOrder, startedAt, reportUuid, resultInfos, StepStatus.FAILED)
(optionally logging it via the same logger/reporting mechanism) and then rethrow
the original exception `e`; i.e., wrap the call to updateStepStatus in its own
try/catch so updateStepStatus failures cannot overwrite the original exception
thrown by stepExecution.run().

In
`@monitor-worker-server/src/main/java/org/gridsuite/monitor/worker/server/services/StepExecutionService.java`:
- Around line 27-31: StepExecutionService stores executionId as instance state
causing a race between setExecutionId(...) and the interface default methods
(skipStep/executeStep) that call getExecutionId(); fix by removing shared
mutable state: either (A) replace the executionId field with a ThreadLocal<UUID>
(e.g., executionIdHolder) and ensure you call executionIdHolder.remove() in a
finally block after each step, updating setExecutionId/getExecutionId to use the
ThreadLocal, or (B, preferred) change the StepExecutionInterface API to remove
getExecutionId()/setExecutionId() and add UUID executionId as the first
parameter to skipStep and executeStep and propagate that parameter through all
implementing classes (including StepExecutionService) so no singleton holds
per-thread state.

---

Nitpick comments:
In
`@monitor-commons/src/main/java/org/gridsuite/monitor/commons/steps/StepExecutionInterface.java`:
- Around line 65-68: The call to getReportPublisher().sendReport(reportInfos) is
inside the try that surrounds stepExecution.run(), so a report-delivery failure
will incorrectly mark the step as FAILED; move or isolate sendReport so it
cannot change step outcome: keep stepExecution.run() and updateStepStatus(...,
StepStatus.COMPLETED) inside the primary try for the computation, then perform
getReportPublisher().sendReport(reportInfos) in a separate try/catch (or a
retry/dead-letter flow) that logs or records report-delivery errors without
calling updateStepStatus with a failure status; reference stepExecution.run(),
getReportPublisher().sendReport(reportInfos), updateStepStatus(...) and
StepStatus.COMPLETED when making this change.

In
`@monitor-worker-server/src/main/java/org/gridsuite/monitor/worker/server/services/StepExecutionService.java`:
- Around line 68-69: The code calls context.getReportInfos() twice—once inside
Objects.requireNonNull(...) and again on the next line—so cache the value in a
local variable (e.g., ReportInfos reportInfos =
Objects.requireNonNull(context.getReportInfos())) and then use
reportInfos.reportUuid() and reportInfos for the subsequent arguments; update
the block in StepExecutionService where
Objects.requireNonNull(context.getReportInfos()) and context.getReportInfos()
are used to reference the single cached variable.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2e3181e and e948dcf.

📒 Files selected for processing (4)
  • monitor-commons/src/main/java/org/gridsuite/monitor/commons/steps/ReportPublisher.java
  • monitor-commons/src/main/java/org/gridsuite/monitor/commons/steps/StepExecutionInterface.java
  • monitor-commons/src/main/java/org/gridsuite/monitor/commons/steps/StepStatusPublisher.java
  • monitor-worker-server/src/main/java/org/gridsuite/monitor/worker/server/services/StepExecutionService.java

Signed-off-by: benrejebmoh <mohamed.ben-rejeb@rte-france.com>
…s' into extract-step-execution-to-commons

# Conflicts:
#	monitor-commons/src/main/java/org/gridsuite/monitor/commons/steps/ReportPublisher.java
#	monitor-worker-server/src/main/java/org/gridsuite/monitor/worker/server/services/StepExecutionService.java
…s' into extract-step-execution-to-commons

Signed-off-by: benrejebmoh <mohamed.ben-rejeb@rte-france.com>

# Conflicts:
#	monitor-commons/src/main/java/org/gridsuite/monitor/commons/steps/ReportPublisher.java
#	monitor-worker-server/src/main/java/org/gridsuite/monitor/worker/server/services/StepExecutionService.java
@benrejebmoh benrejebmoh marked this pull request as ready for review February 25, 2026 14:23
@coderabbitai
Copy link

coderabbitai bot commented Feb 25, 2026

Caution

Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted.

Error details
{"name":"HttpError","status":500,"request":{"method":"PATCH","url":"https://api.github.com/repos/gridsuite/monitor-core/issues/comments/3953049296","headers":{"accept":"application/vnd.github.v3+json","user-agent":"octokit.js/0.0.0-development octokit-core.js/7.0.6 Node.js/24","authorization":"token [REDACTED]","content-type":"application/json; charset=utf-8"},"body":{"body":"<!-- This is an auto-generated comment: summarize by coderabbit.ai -->\n<!-- walkthrough_start -->\n\n<details>\n<summary>📝 Walkthrough</summary>\n\n## Walkthrough\n\nThis pull request introduces a publisher-based pattern for step execution monitoring by extracting common interfaces and data transfer objects to the monitor-commons module. The ReportInfos class is relocated from worker DTO to commons, and new functional interfaces (ReportPublisher, StepStatusPublisher) and an orchestration interface (StepExecution) are added. StepExecutionService is refactored to implement StepExecution and use publishers instead of direct service field injection.\n\n## Changes\n\n|Cohort / File(s)|Summary|\n|---|---|\n|**New Publisher Interfaces** <br> `monitor-commons/src/main/java/org/gridsuite/monitor/commons/steps/ReportPublisher.java`, `monitor-commons/src/main/java/org/gridsuite/monitor/commons/steps/StepStatusPublisher.java`|Added two new functional interfaces with single abstract methods for publishing reports and updating step status respectively.|\n|**Step Execution Orchestration** <br> `monitor-commons/src/main/java/org/gridsuite/monitor/commons/steps/StepExecution.java`|New interface defining abstract accessors for publishers and three default methods (skipStep, executeStep, updateStepStatus) that orchestrate step execution workflows using the publisher pattern.|\n|**ReportInfos Relocation** <br> `monitor-commons/src/main/java/org/gridsuite/monitor/commons/ReportInfos.java`|ReportInfos class package changed from org.gridsuite.monitor.worker.server.dto to org.gridsuite.monitor.commons to support commons-level access.|\n|**StepExecutionService Refactoring** <br> `monitor-worker-server/src/main/java/org/gridsuite/monitor/worker/server/services/StepExecutionService.java`|Refactored to implement StepExecution interface; replaced direct NotificationService/ReportService fields with StepStatusPublisher and ReportPublisher; added publisher accessors; updated skipStep and executeStep methods to use context-driven parameters and publisher integration.|\n|**Import Updates—Production Code** <br> `monitor-worker-server/src/main/java/org/gridsuite/monitor/worker/server/core/ProcessStepExecutionContext.java`, `monitor-worker-server/src/main/java/org/gridsuite/monitor/worker/server/services/ReportService.java`|Updated ReportInfos imports from worker DTO to commons package.|\n|**Import Updates—Test Code** <br> `monitor-worker-server/src/test/java/org/gridsuite/monitor/worker/server/processes/commons/steps/ApplyModificationsStepTest.java`, `monitor-worker-server/src/test/java/org/gridsuite/monitor/worker/server/processes/commons/steps/LoadNetworkStepTest.java`, `monitor-worker-server/src/test/java/org/gridsuite/monitor/worker/server/processes/securityanalysis/steps/SecurityAnalysisRunComputationStepTest.java`, `monitor-worker-server/src/test/java/org/gridsuite/monitor/worker/server/services/ReportServiceTest.java`, `monitor-worker-server/src/test/java/org/gridsuite/monitor/worker/server/services/StepExecutionServiceTest.java`|Updated ReportInfos imports from worker DTO to commons package across multiple test files.|\n|**Dependency Management** <br> `monitor-commons/pom.xml`|Added com.powsybl:powsybl-commons dependency to monitor-commons module.|\n\n## Sequence Diagram\n\n```mermaid\nsequenceDiagram\n    actor Client\n    participant StepExecution as StepExecution<br/>(Interface)\n    participant StepExecutionService as StepExecutionService<br/>(Impl)\n    participant StepStatusPublisher as StepStatusPublisher<br/>(Publisher)\n    participant ReportPublisher as ReportPublisher<br/>(Publisher)\n    participant ProcessStep as ProcessStep<br/>(Execution)\n\n    Client->>StepExecution: executeStep(context, step)\n    StepExecution->>StepExecutionService: getStepStatusPublisher()\n    StepExecutionService-->>StepExecution: StepStatusPublisher\n    StepExecution->>StepStatusPublisher: updateStepStatus(RUNNING)\n    StepStatusPublisher-->>StepExecution: status updated\n    StepExecution->>ProcessStep: run(Runnable)\n    ProcessStep-->>StepExecution: execution complete\n    StepExecution->>StepExecutionService: getReportPublisher()\n    StepExecutionService-->>StepExecution: ReportPublisher\n    StepExecution->>ReportPublisher: sendReport(reportInfos)\n    ReportPublisher-->>StepExecution: report sent\n    StepExecution->>StepStatusPublisher: updateStepStatus(COMPLETED)\n    StepStatusPublisher-->>StepExecution: final status updated\n    StepExecution-->>Client: execution finished\n```\n\n## Estimated code review effort\n\n🎯 4 (Complex) | ⏱️ ~45 minutes\n\n## Suggested reviewers\n\n- thangqp\n\n## Poem\n\n> 🐰 *Hops with glee through commons ground,*  \n> *Publishers now all around!*  \n> *Step by step, the status flows,*  \n> *Reports sent where data goes.* 🎉\n\n</details>\n\n<!-- walkthrough_end -->\n\n\n<!-- pre_merge_checks_walkthrough_start -->\n\n<details>\n<summary>🚥 Pre-merge checks | ✅ 2 | ❌ 1</summary>\n\n### ❌ Failed checks (1 warning)\n\n|     Check name     | Status     | Explanation                                                                          | Resolution                                                                         |\n| :----------------: | :--------- | :----------------------------------------------------------------------------------- | :--------------------------------------------------------------------------------- |\n| Docstring Coverage | ⚠️ Warning | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |\n\n<details>\n<summary>✅ Passed checks (2 passed)</summary>\n\n|     Check name    | Status   | Explanation                                                                                                                                                           |\n| :---------------: | :------- | :-------------------------------------------------------------------------------------------------------------------------------------------------------------------- |\n|    Title check    | ✅ Passed | The title 'extract step execution to commons' clearly and concisely summarizes the main change: moving step execution logic to the shared monitor-commons library.    |\n| Description check | ✅ Passed | The description is directly related to the changeset, explaining that step execution service logic is being extracted to monitor-commons for reuse by other services. |\n\n</details>\n\n<sub>✏️ Tip: You can configure your own custom pre-merge checks in the settings.</sub>\n\n</details>\n\n<!-- pre_merge_checks_walkthrough_end -->\n\n<!-- finishing_touch_checkbox_start -->\n\n<details>\n<summary>✨ Finishing Touches</summary>\n\n- [ ] <!-- {\"checkboxId\": \"7962f53c-55bc-4827-bfbf-6a18da830691\"} --> 📝 Generate docstrings (stacked PR)\n- [ ] <!-- {\"checkboxId\": \"3e1879ae-f29b-4d0d-8e06-d12b7ba33d98\"} --> 📝 Generate docstrings (commit on current branch)\n<details>\n<summary>🧪 Generate unit tests (beta)</summary>\n\n- [ ] <!-- {\"checkboxId\": \"f47ac10b-58cc-4372-a567-0e02b2c3d479\", \"radioGroupId\": \"utg-output-choice-group-unknown_comment_id\"} -->   Create PR with unit tests\n- [ ] <!-- {\"checkboxId\": \"07f1e7d6-8a8e-4e23-9900-8731c2c87f58\", \"radioGroupId\": \"utg-output-choice-group-unknown_comment_id\"} -->   Post copyable unit tests in a comment\n- [ ] <!-- {\"checkboxId\": \"6ba7b810-9dad-11d1-80b4-00c04fd430c8\", \"radioGroupId\": \"utg-output-choice-group-unknown_comment_id\"} -->   Commit unit tests in branch `extract-step-execution-to-commons`\n\n</details>\n\n</details>\n\n<!-- finishing_touch_checkbox_end -->\n\n<!-- tips_start -->\n\n---\n\nThanks for using [CodeRabbit](https://coderabbit.ai?utm_source=oss&utm_medium=github&utm_campaign=gridsuite/monitor-core&utm_content=48)! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.\n\n<details>\n<summary>❤️ Share</summary>\n\n- [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai)\n- [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai)\n- [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai)\n- [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code)\n\n</details>\n\n<sub>Comment `@coderabbitai help` to get the list of available commands and usage tips.</sub>\n\n<!-- tips_end -->\n\n<!-- internal state start -->\n\n\n<!-- DwQgtGAEAqAWCWBnSTIEMB26CuAXA9mAOYCmGJATmriQCaQDG+Ats2bgFyQAOFk+AIwBWJBrngA3EsgEBPRvlqU0AgfFwA6NPEgQAfACgjoCEYDEZyAAUASpETZWaCrKPR1AGxJcSAD1xUYvY03JB+onjw+FgECqzRyJAGAHKOApRcACwAHJBJAKo2ADJcsLi43IgcAPTVROqw2AIaTMx1FPC0Duok1czR6vgUYEwUvdzYHh7VOXkG+YgZkOkYYyIC/bBzAMr42BQMJMtUGAywPv6BuGCIIWDhDJHRYAQjLP0YiQbQzqS4x5gzlxmNosEltrhqNgqvxuGQ5gBhMbUOjoTiQABMAAYMQA2MA4sAYzLQACMAFYOJkAOwcUkAZgAWkYACLSBgdbjiaIcb6wI4TKaQMYAR2w0n+fgCaDEyFw/OCJFCDyeWEWFAk8EOkA8+HqDBQGFiaHssGcqI88AEVBckAwaDY9A+g2GrQ+yEQ+Eg8uo3rN/11+sYmGWRzG0NRcn48so9komsOiA0MAVZ0wpEg/SkyDd0VDZs1Q0gADMi7clWFfBFuVhixQWH6jo8KGMjQolAI0ItDbEYzqrZmBgRXe8EhojPpjOAoGR6PhizhXqRyFQaPQ3ewuLx+MJROJs8t5EwlFRVOotDpJyYoHBUKgQ2g8IRl8o13E2EauFQAO72Rwg20o2PZQz00bRdDAQwp1MAxnWHN54k+apEAOPpQWqIQ0AkNBqiGIh2k6boaD6IchmqXMkPLSpqhsJUhlwKwmktRB+QoDRMOw3kACIeIMCxIAAQQASWIMhX1RBwnFtedGDNDBSEQCdBNoLp0DtEhfwmARLQYAAaEtsFOGs0A8HtKGLGUjlo7h6MY7SkFYw1BwwF0EPdZDUJBeAMAwrCcLwgiumwHoSJc4dyNHSiQkQGi6IoBimIcyh2L88cbwVbyaAoCztXvDAMHwSE3wAAQAMUMsQontDwhKNczLPQDB6CUBgPHNZATUQbyiC8TMSHlRRIAkfBOjjJrrPogAKCb4tq0tkDGGzZoweaAEpxwMIpvOkWT0zoLgAGpSWyaoCSMABRW54BBN9gOFEhNQ0sJi1LeKuAAWToeBHCMLbyBzOTSFoQ7jtOrELqum7UTusZHt/EgXvoj6vp+niuInGC4KGNyEg8hg0O83zsNwih8KIDogpCrGKAixCYqomKISVCEoUQOzmNYlLOIMNG+MsYTRJXFF6EkgD5BktN5OkIxaoCRRsETNTyE0xKDWLCrjNMzL6u1JnuBZ3BoXZpK+G8xseBlABrNAMzwjRycI4KaA0amWkipMGeTOAjm17KGpatqxg6+xut6lRbiuPqBvobBuFoFE9YN6FIG/BAznQBhDi5YP8nyISWUrasqqE+hMFL6x60TRBzqrR4az1nhK+kava9VPWvYVYt4F67zWuwJQPUhJrnHofk0BPd92A6pqUGngqitRb8GkgMqNaqkzZZ1khkyE/5rqW3BkA4tANEiDwNFz/PGrnUn7Ypojt9diikysJvEBbovonbyBki9cfaHUOvUygYtT8D4JQesfA5K0EtPJWePZ6y0AVnQAA3ObSWGZPReA8PIf+wcGAmTatpJs0RpRBFejwRKLFurVFjvHcQcCqLBFZinBoZsTTU26inIYltiy6m/BtJSm8/ban/vtSAAADRORUjZUNYhIpyEjqY40op5dCx8SZk3vk7Xo1NabuQZtUaRrNjYsWSsfCRBgoDGwNGwaOIciD2kNmMdAKlxESOGqNOhCcQhJ0QJNS+BcVQ1hLvpV++Aq410/hgBuvAInNyiXXKqetVqWM2ttAGe1gaQAOtiMGENxBQ3XIoMMD14BPQRq9dEn0AGo14lYzGpFhjfh4ZQG48ZKB4wJj5DRAUHaU2InolpFBLadPVFIGm4ytTSCMSERJbd4zTK5mgbivF+ICxfKuCS/5nDiwXBg6WVjIDvTQKM5Aet5n10WYcYACJKw0CasgcJVcETRC7kQPQKBmDcC8B+f4FzW41n0tQ4sDCiAlnrMwNSRQWACHwJbMAtAOhSAwPpLuJAPC0DAJ2RY9Al4dDgbEPwPytTqEofZMxwwcWom1uTagVU0qQGsm1RWACxhBF/uILuBCrkammbFA+2xrlHHRZi5AS95SQGMYbNmcjYxlyZXFBKFLWL6XxaiTUJoTTKwUJ8AICthyQEmnWBsnL4DcvpV/YV19FWCuFetI5AkVLIC0jpDOVchgwj+NK2RKrKCTVWjav4M1lUc39YGolvgbLdj7Dq11JskxHNlog5BOYEj6rENjaleL4AEvBT6PeYV4AmXgAALx2vGylyATVQriZqJQIthWJqgLRHKw4PSW3gPrEI3ovQRjUkwOq/hEXIvhAQqY+kxhZi4WyvchowB2NgINW4LCuqOJLXAhVYwkEMC4dwZwDp+qUDlF6Zu7Bi2mUHTQfw2ATKQHoSaSaV6pSQHzogYFPb7RsH0kME8H7nBrgErgB1LaEYynbYXOuJAG6xCUF4IgKJe1qU2aA4J0GQiTQ0FhwN46PD6QKr+T0+xd1wIfRChsfZn3+CNXEqu1RglVVfSyd9ipQhfpID+igf7mHxToEBydSrICBMDQq6EXCqOaGDUqua+B/EiZnhJ+2/VaIOA8LgGTcnGW0SzDtbyC7+pLvoLVFdbZoQ2yOAq3gkhEPeJoD6/xWGNCBv5B4OEFA0EYuut5FEyBZ1BHnuarUlqsC2d07WLCRYZKVtYtirsqJ93ym/GgWQzbbUsorR0IsvtqoV3ie/S5ySe2DojgaxjEqthSkoDl6LsZfbgaqogNBxX8CWnoTtdIBYoh8HlPWbARAti+xykcRdihp70Bq1A/A8LUtOqENCQ+FsqB2MoGW4LSGxg4P4FgFDDBkCTWec3AF0TXlDtwLcvQYS36ID1ud+T9AyAOCDvdA+Qm84F1QCeSQqIa26uvZJ5T0mVqyYDYy/IccfNfIPiew0fclC2tsnKvgCr7OmNYmgqd+ADyGQjPQGFzA4WWxtX5/4AWLU1mQta0VXQNrrLU6+BrSG+wBwPTWZAMliX0VREWeNBpz3iEOdY1WjA2rvwcU4/YRxQvZKkXMwFyThW3PubOJ5V2TvvL0AogjkPflT0kUdpJ0QFFmyUU0sAwzRnDHGWMtRhNemk0Cg/UKLpqjm7GR0yZTbZlKgK1avlhxlmWMF/ZNW5TMXuubp61xDbtektwJtzV6kVZ+ooFUAweQoA12jRq4tkA/hZSjoZ1PeR0+SO9b4mRsrk8BoUWMZxnw9fl5MYjyxxfdCl4BwfVH4aa/9X2PXiRIau8UEDz/J6PPdUlczRQXkJeJHj/1wsv3JBJpmrJ/LpfdpCqBZ5evhMHH4fxSFRvxa9Ej979SUc5IGlqjj5G6pNd4ug4z7b3PoXniRadu7UqfbV2F81hO39udr9lKJdnltdiEEAVRBfrPuPu/pBngOht/gdu/H/lVAAVKEARJqAVXDdgiJ8lASPjYjfsiohmwPjrGAckXi/lZthDQCWN5qZCjojqxn4kPigi3tQSQXQV3DloPswSfvFGwWkvxCci5MWBKJAKVN3EcAJNVLIOWhQEYEcickIEMAAPqhaKTpL/S7RSzZIHT0iZCnTkikg8z1IQBgBGDKLPw34sAaC+DMAeCrLozrIiSbLCx/hSR7K6EKRKROrR7apj5C5KBwhNRkAMDyCxDWHuy2HMD2GOFcDkx7DcAlxxAaA2TfiICyDaT6QAbmrgapEZFZHaQqIsYmTRBEBdRw59h+BIBgqQCYQMCWyegYBgCYDzzBa+ZKizjhGyAdx3hGgpqKwPgqSALRB3rSifCAJSD3rdFhGnDyAUJ9j9BIJeCCLKEGaDRiLS5sihFKALFG5YAm5hTYw2E2RxEOEeAKJJBQC7E9ELFR7uKtDpH4CZHZFOFFHvGlGHGSLRF0yxHxFXG/QZI+HiIHRGFnQGCXSFIeEwxlIVKIxvTHIozMBmHowNKWGwSm42EoT4xeQ9J+SaIO46JO7hQ2EhoabLLOF8yCRuFiRbIiw7LST7KAwC5Cbg5vh9gABSfkFsTR5msxrULOjGZsFJQOSYx85GUKds/SD8LsTSGgrubEVubEtAvYXoMp2iPQ8pJxbEz8+klSe4X2m2GCXCfY6sUwsgYAYoJa6K9A7G/AC4fY4+uAsgcIsxPBNKMQCA1a0hG0UAv8oJ0Od+YuUIQc+kQpoumoXUaglorpnGkO9EHULivA0g7AG0f0O0By+hpIp0phUJkMsJJS90cMz0VSyMtSqJvMGJVh2JMRuJ3SRM/k9uspJJeiOJ0UXu3APuGAVJaJNJGy9JHhosuyjpQZfhzqSsQRwekAPJ2EZkIiRwqBeYZsfxBiNuBJxMfSWpgyTS+iuMhiy5vZx8jKbIQpT2uALS6AAgEc4G4e78nqiR/UTBVed2ueHeCOr5jKya8sisPWJARwSgFkkw/wd+0OQwZwEoWyrG8BNYNqK6MqkAmhz+UAiAn+esk0tGCScu0QoSrGPZeFVE0AbpJAyQh6H6SoAA8lxpQP+rxrQEBqtFwAIMFGKmpMgR/AbjEj2uVlKgANJCRWBWDnQFwIXJyiYck7QJ5l7MwV5D4g5p5t5oYYVYX5Y4UYCEWy7RKaVKjEVwhkXfqsbUXcYrr0X8bPb0T5DBS0ACYHwaYCaqbqbikUXdnqVMWZjODNEwU2D5DJDJBCTJAADik6hkcoCodanQqINghk9oxCwKyuakAh/w0lH5ghiOAauRM8mhzCiFsQCIlF70VgRQ500AIlYCkhwkxVBcIRs4XCeYewuArQ28ilUAoW9mmFV2BFNl+F6lOl3AelpF5FRlNFFAdFgGuAtlll1lDlIF9lOV0I7lHIJAEOIYUuuWkS6lDcvFfY9QKK81Y2PAsmC2ZKPWew/W5sL5YabERyCwO0Kms14pSGn2MxQcIFgSNqr1amA1Kc/IIWHJXCYlqWlFAkeAsAYAlooy5VtKq4jG4cZCrORql1Js+kfByeImLiYw4hrYhwpcXRex4R5SYVvW51cNkcMoHqKeG0AA2pCEQFwAALrmD8x04w0JCM4KjM6s317s5Rqc43zkpup86E1CJ1SLmPHS5Hk/HHGuQ4kblNlEmtlUx7kdlKiMxaVcUB4tXWBC4hnbFcAy6yVN7J7vm4BI2UrV5a02IF5bFuLS6o1XUm320mwW0l5W1AWPhqbW2ly2361oVdoYXvWqWcWqh4XvVURdX6QQh5qsYDUGX76ZTDXcbGZDz/CmXjXQFt5u1gYgVe3i363KUYaB2dW9XdVh3q0h3dVR0A0hCx1DUJ1UTGW0Wvp6qYCp2QhmUTWvZXxJVWWdAo2A7zQWXLTzT92OUab3Rj3OVMoxUqC9Th1uUcFB5uru05260+2SJtWN4yoBJvaNxgER1d2iXl0hKV0BDV26UkVx36T10hCN2jXN0mZt0AZ8ad3vU93TWKqT2D2fVOUj1Spb3JyA0X5KFGDJCUXJDnTAk6HZkgykjUj5IFkwm3TFmwzlLwyInVIon9k1lYm6lm6tKW7u5dL4ny3bmOxK26ku4EMU4aidKjC9AcVHnoH+B9m8yuGCziSMleFjmUFKTMqWRhU+zfJ80H6/2ybC5djVqQrmxKmQAsjQCUV8nWwZjbUKhimD2RlSMUapjuxKPmb9GZJSyCPegkX3RY3hGS64olhFh9hJXj1U5OSMPH1oGkIgHoAvRGmEoKhEYHBHAyR9jqPiOununlYNXoAs1cIUCGSFJHAdZYRdblVXr1imR8KvGhiyDRD0BM5zH7EREWzyjrG05ZSdHs2AWiCBwlM80Hxc58Dj5C1smBmUGlOCkVMI1VN8035C71N7bj4CRWBCTITwDrrOI7RTqghIWnCsm0AOo0miHmoSFSG9SyEmTyGUBQNZlTMgx5kFLXRFlw6oMInlnImVnYMWG1l4NKntK0OTJy125aLkO7mUNKk0MTIvPTIxQhpn5LLHzUnsPuFvgjnMnjlHL8M43mzj77z0RjmBNaPSm3yK3OyuxKkaAqkaBqn4AaAwtIaakPOPwKnPyYsD2ybJiBmFSOQgIGhFhJMtYlj8JBloJma2wLhYtjPeRygkVvBTHlhGhqrLzRCbZ9g+Paj7r8lMvgtC7BOAXmpY3dSFPM3FMI29gc3lPCls3tP0XlV1NGiAINNegP5hlNisnQ4c70WdMzndOzE0BiB0AZkgkwM5K5lgD5nQm7PIP7PwnoNHM1LfRVnmGNIXMENXOvMNk0C3CkMtk7m6J7nPMqnEFgEzIq3UQCTcA/KyDvSKDb6dF6zQASisNrL8x0lCwAtMneG8OOq22JnxRgJEB3y4s6kuhux0yEt2Xino4kA6b0CQtVs4sDJ4u6mKkEMovu5osEDNv0QaZoKBnqxGRAJBk7hW6dsxASEUu2vQObMOvbOIOuvQwoMetllIyQAwrfinP+uuSXOxshsSjhv3O9uklkQxtENB0Jv1mdkwrjxX6Xk8LZu5s/P9l/NDnFvcMSxGtKRg5tadvCPxTIBmxvu0Afvm7fu3BfjtuY40qQf/AyQ9tylIuDuovotjvD3iMKrbGVsYcLhYc6L1vDiNvugEdiMsamleNWREvsvuk45OR9ihup297cCrsbNZJbNOs7NFLtilKlmVIHvet1LolnO4NnuBsXuoRcfXvEkUPO4PvXNxtVwzKLDNjqCyCYArNdT0ydlCp6eunLM4LGfRUYCvLfJ4DBaIeaC/tsMFscMMmeFiw8Mgc3WSUQcvaMsipFgwvIc/KWT+dQuYfwuRtUdDADsjLJR4ejtYu8UUfalPzux0eUk/wakxh8B3SUFoKccSFXRCjFYZoLZYtmyCuHopzJa5EeOVRwKpxahbCaPwKY2UAWOlz/CtDcDSHejXTNVKFFP05s1KtlPnmVMLgmtvjc5dPav85aEBlehNOwYqtc1s4ze80avzfmuLeE18eGNAyCfOuFlutidoP7tIlSe+syenvwTntEOXtht3OqePPqfUMXue6fPCo5u3B5suFuf/PbJAcsl7TLeQAIhTPmxdv/AJZbAyRYs/ZpeIsKnItJcYtYuxCo99sNsEswsdylK3Bjl9iBf3T4nGOIC1eTRssp3FpwW8U2DnRWCUU2DQBqHvVbpKq/xKDrQ5fRiOTTuVTjGXpGvzsdK0ByuCQs0lMTctOqvc3bfVP81aviBLdKS/zkDrPHdgmOtndIM7vuvicYMVk+snuYnKJPeacve4AqcItRtPNffPee5HlfOHD/fOd+S/PA8Aeg9efAcQ8TnR5w9cC4+xd6mZcwsgsodSARdInh84cJfKnDv4fR8rdzuxAUuJOkLJO0uvFoL8vyBw9/g3nq+qhHegk5mbsusidwkm9HNHsntQTXhhAzwySPhLh+/FKsCbjChoCEYluHiienhqBgSXiQQGCt9ujqBqGERqEHPfh0BqFp0QQt/ThhAACcOQtADA4hDA5I9IWIm/WI2QeIKgpIWI1ImQmQogBC1IaA1Im/pI8cGIm/u/C4V4G/JA9I9IP/DA9IGULQDgYMBcQtAQwmgAZCkgMQAgUkLALQBoB6Q5ILEAICxAoCD+aAzfmv2ghQAZ+uAOfl0AX4etl+s4bAa31TJqE2ApMEgGoUgpNFEAK/duv8EnAGAAA3opS4hIBbAAAIV1BNE6AdnP5FYCOp0AuIXACyB4EWC6QOBLEPYJil4ERJLYtgMQSWBMhSCOBSASihMgpj7EVBEg9QXkC4gAJaANnFkBEirryREA0PUQJbBUH6oOMHA4wTZ3cBx4SA1gponYKiYODDBTgwyGyEQAcgu0/+fkB4K4D2DpBhg2BKMloBCR344ocAhQBUE8QIhkALiCLlwDuDLY91NTIgBUFU1FKeQdga3kMF0DLYcdJIS4N6ilCuIKQ1vFxEBqeDxQtQ4vFxGJRtQnEVUCoQqHV69QAA5FKEjhMIGMeYWIM/F6HC5lqFATbAqkHS7pFgm2QFmWh2jLFxmByYEJjnPrKh1KOoPUKAnl4sRzQToOsnTH7DWhdkGgGoQUJaErESASQpLBQBcjyRLhxQ1IeyjeRDMJcegtQd4OKFcQhgQzBgpkPKFcAuIPQ24VcIAC+zQoob8NKHAjUh/gwIVyEYzVDmhhghoWEK8FojUhbQwzjWC6FlMAhnIOCh9lzR7hNsG2DwvL0oL9QDSUaNqN5DNL+gYKwwtUNamz6oB0gXCAYeBlRBRFjh7oaxnwHDDdgowZLWMFMkTAXDsRXEG4XcOcCPCiAzw34W8JWgfCxgXwyQT8LqH/D6g1UIEYeiSEDwkR+IyEdCKuElCQhZQw0SCLMG7Yz6cCV5BMnMzKi6hGI1QVqJlG4iOh0QJIXaIjjiZUOVADMKgCxAaA0BAAUh+qtd4EbLbAC9FJTpkUwpSMUGSKyawAg4S6MPKgGyBhjIx0oi0a8OkAtZVQSQgAOodA6CtACJAGMsFCjzYwvBGswCQBdQmOUKWINT3ECIBiwkRHRs6IzD/lZBmKAsS8NlElJ5RDw7qK6JaG6jARVo+EUYJrEOjKi6MVvBCMUr00UhaQrsAxBsCIjiRnQkEcWHJC4gBAtATIKeMP5H96Qm/TfjAJyAWRN+xYTfmgGyACBbxAgbEOPD/5Yh6QxYakNkEAnUhyQJhWgCYWLC4g/xGIbIM/1JA0hsgmQckMqO3G3BbAlQ24SCM/HACkB2QYsNkFoBYgTxEE8kLQCAkYgSAmQUkLiDxDUhT+f/AQJkHQHFgMQ5IJ8dSAED0hYBGIDEHhN/4qABAEEghAyHJDFhkJ1Y+0XmidHKBSAwiaqAbAwmQAYRhgtcgeVuaEkyGt7dsi+1VoCpPyV1ZZCoKUktCCAkIDwOVBnYJAVBpIGUY2IaxliGg/o5cbkK4Dgw1xzQscXg1lp4l1E6kiNrizvY0xE2atb3L1VFpDYDJXAIyYYJMkmRzJIvT4CoOpA2S14CQeyfKEcl5pnJkAVycXihFXCPJMtesmpK3J+TNJytbSdRDNqcxj4hkwsaCMKixSUpCUrgNZLqm2TUpDkpcZlJUE5S8geUuoVbwU7PdipzZG9o7iGTO8bervZxr7j3wRTFJdUmKWZKalZTyQyUiyZ8DSmwAMp3ULKb1MgD9TrhAo1Sd5Nty+SxpbZcqf8UPIzTjyfkWqaOKWlxTWcKgtaW1JWlbSdplgnqZCMUrrj1xU/DfhQKoGkBaBVohgaQK/4QBFw+ANQvugjCMCUQiMqtiwNYEoSGIj4XFEBlohwwBB7wdQK8miZiCsQAM1vp31hnwzFgiMmgGoUhmQQgAA=== -->\n\n<!-- internal state end -->"},"request":{"retryCount":3,"retries":3,"retryAfter":16}}}

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@monitor-commons/src/main/java/org/gridsuite/monitor/commons/steps/StepExecution.java`:
- Around line 46-55: The code captures ResultInfos too early in executeStep (it
passes the resultInfos object before calling stepExecution.run()), causing
stale/null resultUUID/resultType in status updates; modify executeStep to defer
reading/result retrieval of ResultInfos until after stepExecution.run()
completes (or when building the COMPLETED/FAILED status payload), e.g., replace
passing the pre-captured ResultInfos into the status publisher with a fresh read
or Supplier/closure that calls the ResultInfos getters post-run; apply the same
change pattern in StepExecutionService methods that build status updates between
lines ~72-90 so resultUUID/resultType are read after stepExecution.run().

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e948dcf and e33e3c3.

📒 Files selected for processing (12)
  • monitor-commons/pom.xml
  • monitor-commons/src/main/java/org/gridsuite/monitor/commons/ReportInfos.java
  • monitor-commons/src/main/java/org/gridsuite/monitor/commons/steps/ReportPublisher.java
  • monitor-commons/src/main/java/org/gridsuite/monitor/commons/steps/StepExecution.java
  • monitor-worker-server/src/main/java/org/gridsuite/monitor/worker/server/core/ProcessStepExecutionContext.java
  • monitor-worker-server/src/main/java/org/gridsuite/monitor/worker/server/services/ReportService.java
  • monitor-worker-server/src/main/java/org/gridsuite/monitor/worker/server/services/StepExecutionService.java
  • monitor-worker-server/src/test/java/org/gridsuite/monitor/worker/server/processes/commons/steps/ApplyModificationsStepTest.java
  • monitor-worker-server/src/test/java/org/gridsuite/monitor/worker/server/processes/commons/steps/LoadNetworkStepTest.java
  • monitor-worker-server/src/test/java/org/gridsuite/monitor/worker/server/processes/securityanalysis/steps/SecurityAnalysisRunComputationStepTest.java
  • monitor-worker-server/src/test/java/org/gridsuite/monitor/worker/server/services/ReportServiceTest.java
  • monitor-worker-server/src/test/java/org/gridsuite/monitor/worker/server/services/StepExecutionServiceTest.java
✅ Files skipped from review due to trivial changes (2)
  • monitor-worker-server/src/test/java/org/gridsuite/monitor/worker/server/processes/commons/steps/ApplyModificationsStepTest.java
  • monitor-worker-server/src/test/java/org/gridsuite/monitor/worker/server/services/StepExecutionServiceTest.java

Copy link

@TheMaskedTurtle TheMaskedTurtle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small comments on how the StepExecution is designed but the overall design is good to me !

Copy link
Contributor

@antoinebhs antoinebhs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thangqp is working on a big reorganization of our files to be able to share code through interfaces (#46). It kind of get out of control though and we're both on vacation next week. So I would suggest that if you want to move forward you can merge this! Sorry I forgot to synchronize with you on this...
Anyway we'll need to publish the lib too

Signed-off-by: benrejebmoh <mohamed.ben-rejeb@rte-france.com>

Signed-off-by: benrejebmoh <mohamed.ben-rejeb@rte-france.com>
…n-to-commons

# Conflicts:
#	monitor-worker-server/src/test/java/org/gridsuite/monitor/worker/server/processes/commons/steps/ApplyModificationsStepTest.java
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
monitor-worker-server/src/main/java/org/gridsuite/monitor/worker/server/services/StepExecutionService.java (1)

33-40: Narrow overridden accessor visibility to protected.

These overrides don’t need to be part of the public service API; keeping them protected better matches the abstraction boundary.

♻️ Proposed refactor
-    public StepStatusPublisher getStepStatusPublisher() {
+    protected StepStatusPublisher getStepStatusPublisher() {
         return stepStatusPublisher;
     }
@@
-    public ReportPublisher getReportPublisher() {
+    protected ReportPublisher getReportPublisher() {
         return reportPublisher;
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@monitor-worker-server/src/main/java/org/gridsuite/monitor/worker/server/services/StepExecutionService.java`
around lines 33 - 40, The overrides getStepStatusPublisher and
getReportPublisher in StepExecutionService are public but should be narrowed to
protected; change their method visibility from public to protected for both
getStepStatusPublisher() (returning stepStatusPublisher) and
getReportPublisher() (returning reportPublisher) so they are not part of the
public service API, and adjust any internal callers or tests in the codebase
that rely on these being public to use package/protected access or refactor them
to use the service's public API.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@monitor-commons/src/main/java/org/gridsuite/monitor/commons/steps/AbstractStepExecutor.java`:
- Around line 46-55: The code in executeStep currently captures/snapshots the
ResultInfos before invoking stepExecution.run(), causing fields set during
execution to be missing from the COMPLETED/FAILED status updates; change the
logic in executeStep (and the other analogous block referenced at lines 72-90)
to defer reading or serializing ResultInfos until after stepExecution.run()
completes (or after catching exceptions) so the status update uses the latest
ResultInfos values, e.g., call getters/serialize ResultInfos only when updating
to COMPLETED or FAILED rather than before calling stepExecution.run().

---

Nitpick comments:
In
`@monitor-worker-server/src/main/java/org/gridsuite/monitor/worker/server/services/StepExecutionService.java`:
- Around line 33-40: The overrides getStepStatusPublisher and getReportPublisher
in StepExecutionService are public but should be narrowed to protected; change
their method visibility from public to protected for both
getStepStatusPublisher() (returning stepStatusPublisher) and
getReportPublisher() (returning reportPublisher) so they are not part of the
public service API, and adjust any internal callers or tests in the codebase
that rely on these being public to use package/protected access or refactor them
to use the service's public API.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e948dcf and b9ad2dd.

📒 Files selected for processing (12)
  • monitor-commons/pom.xml
  • monitor-commons/src/main/java/org/gridsuite/monitor/commons/ReportInfos.java
  • monitor-commons/src/main/java/org/gridsuite/monitor/commons/steps/AbstractStepExecutor.java
  • monitor-commons/src/main/java/org/gridsuite/monitor/commons/steps/ReportPublisher.java
  • monitor-worker-server/src/main/java/org/gridsuite/monitor/worker/server/core/ProcessStepExecutionContext.java
  • monitor-worker-server/src/main/java/org/gridsuite/monitor/worker/server/services/ReportService.java
  • monitor-worker-server/src/main/java/org/gridsuite/monitor/worker/server/services/StepExecutionService.java
  • monitor-worker-server/src/test/java/org/gridsuite/monitor/worker/server/processes/commons/steps/ApplyModificationsStepTest.java
  • monitor-worker-server/src/test/java/org/gridsuite/monitor/worker/server/processes/commons/steps/LoadNetworkStepTest.java
  • monitor-worker-server/src/test/java/org/gridsuite/monitor/worker/server/processes/securityanalysis/steps/SecurityAnalysisRunComputationStepTest.java
  • monitor-worker-server/src/test/java/org/gridsuite/monitor/worker/server/services/ReportServiceTest.java
  • monitor-worker-server/src/test/java/org/gridsuite/monitor/worker/server/services/StepExecutionServiceTest.java
✅ Files skipped from review due to trivial changes (2)
  • monitor-worker-server/src/test/java/org/gridsuite/monitor/worker/server/services/StepExecutionServiceTest.java
  • monitor-worker-server/src/test/java/org/gridsuite/monitor/worker/server/processes/commons/steps/ApplyModificationsStepTest.java

Signed-off-by: benrejebmoh <mohamed.ben-rejeb@rte-france.com>

Signed-off-by: benrejebmoh <mohamed.ben-rejeb@rte-france.com>
Signed-off-by: benrejebmoh <mohamed.ben-rejeb@rte-france.com>

Signed-off-by: benrejebmoh <mohamed.ben-rejeb@rte-france.com>
# Conflicts:
#	monitor-worker-server/src/main/java/org/gridsuite/monitor/worker/server/services/StepExecutionService.java
#	monitor-worker-server/src/test/java/org/gridsuite/monitor/worker/server/processes/commons/steps/ApplyModificationsStepTest.java
Signed-off-by: benrejebmoh <mohamed.ben-rejeb@rte-france.com>

Signed-off-by: benrejebmoh <mohamed.ben-rejeb@rte-france.com>
Signed-off-by: benrejebmoh <mohamed.ben-rejeb@rte-france.com>

Signed-off-by: benrejebmoh <mohamed.ben-rejeb@rte-france.com>
Signed-off-by: benrejebmoh <mohamed.ben-rejeb@rte-france.com>

Signed-off-by: benrejebmoh <mohamed.ben-rejeb@rte-france.com>
Signed-off-by: benrejebmoh <mohamed.ben-rejeb@rte-france.com>
benrejebmoh and others added 26 commits March 4, 2026 12:21
Signed-off-by: benrejebmoh <mohamed.ben-rejeb@rte-france.com>

Signed-off-by: benrejebmoh <mohamed.ben-rejeb@rte-france.com>
Signed-off-by: benrejebmoh <mohamed.ben-rejeb@rte-france.com>
Signed-off-by: benrejebmoh <mohamed.ben-rejeb@rte-france.com>
Signed-off-by: benrejebmoh <mohamed.ben-rejeb@rte-france.com>

Signed-off-by: benrejebmoh <mohamed.ben-rejeb@rte-france.com>
Signed-off-by: benrejebmoh <mohamed.ben-rejeb@rte-france.com>
Signed-off-by: benrejebmoh <mohamed.ben-rejeb@rte-france.com>
Signed-off-by: benrejebmoh <mohamed.ben-rejeb@rte-france.com>

Signed-off-by: benrejebmoh <mohamed.ben-rejeb@rte-france.com>
Signed-off-by: LE SAULNIER Kevin <kevin.lesaulnier.pro@gmail.com>
Signed-off-by: achour94 <berrahmaachour@gmail.com>
Signed-off-by: benrejebmoh <mohamed.ben-rejeb@rte-france.com>

Signed-off-by: benrejebmoh <mohamed.ben-rejeb@rte-france.com>
Signed-off-by: benrejebmoh <mohamed.ben-rejeb@rte-france.com>

Signed-off-by: benrejebmoh <mohamed.ben-rejeb@rte-france.com>
Signed-off-by: Franck LECUYER <franck.lecuyer@rte-france.com>
…ssConfig interface (#43)

Moved modificationUuids from ProcessConfig to ModifyingProcessConfig interface in order to make the ProcessConfig interface more generic. It is required for further developments related to SnapshotRefinerServer's connection to monitor-server

Signed-off-by: Kamil MARUT <kamil.marut@rte-france.com>
Signed-off-by: LE SAULNIER Kevin <kevin.lesaulnier.pro@gmail.com>
Signed-off-by: benrejebmoh <mohamed.ben-rejeb@rte-france.com>

Signed-off-by: benrejebmoh <mohamed.ben-rejeb@rte-france.com>
Signed-off-by: benrejebmoh <mohamed.ben-rejeb@rte-france.com>

Signed-off-by: benrejebmoh <mohamed.ben-rejeb@rte-france.com>
Signed-off-by: benrejebmoh <mohamed.ben-rejeb@rte-france.com>

Signed-off-by: benrejebmoh <mohamed.ben-rejeb@rte-france.com>
Signed-off-by: benrejebmoh <mohamed.ben-rejeb@rte-france.com>
Signed-off-by: benrejebmoh <mohamed.ben-rejeb@rte-france.com>

Signed-off-by: benrejebmoh <mohamed.ben-rejeb@rte-france.com>
Signed-off-by: benrejebmoh <mohamed.ben-rejeb@rte-france.com>
Signed-off-by: benrejebmoh <mohamed.ben-rejeb@rte-france.com>
Signed-off-by: benrejebmoh <mohamed.ben-rejeb@rte-france.com>

Signed-off-by: benrejebmoh <mohamed.ben-rejeb@rte-france.com>
Signed-off-by: benrejebmoh <mohamed.ben-rejeb@rte-france.com>
@sonarqubecloud
Copy link

sonarqubecloud bot commented Mar 4, 2026

@benrejebmoh
Copy link
Contributor Author

closed and replaced by #60 du to gpg verification problems

@benrejebmoh benrejebmoh closed this Mar 4, 2026
@benrejebmoh benrejebmoh deleted the extract-step-execution-to-commons branch March 4, 2026 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants