Skip to content

Comments

Implement state estimation result provider and service#44

Open
achour94 wants to merge 1 commit intomainfrom
results/implement-state-estimation-result-provider
Open

Implement state estimation result provider and service#44
achour94 wants to merge 1 commit intomainfrom
results/implement-state-estimation-result-provider

Conversation

@achour94
Copy link

PR Summary

This pull request adds support for handling state estimation results in the monitoring server. The changes introduce new service and provider classes for state estimation.

Signed-off-by: achour94 <berrahmaachour@gmail.com>
@achour94 achour94 self-assigned this Feb 24, 2026
@coderabbitai
Copy link

coderabbitai bot commented Feb 24, 2026

📝 Walkthrough

Walkthrough

A new state estimation result type is introduced to the monitoring commons enum, paired with two Spring services (StateEstimationService and StateEstimationResultProvider) that handle fetching and deleting state estimation results via REST. Configuration and comprehensive unit tests are included.

Changes

Cohort / File(s) Summary
Result Type Extension
monitor-commons/src/main/java/org/gridsuite/monitor/commons/ResultType.java
Added STATE_ESTIMATION enum constant to the ResultType public enum.
State Estimation Services
monitor-server/src/main/java/org/gridsuite/monitor/server/services/StateEstimationService.java, StateEstimationResultProvider.java
New Spring services: StateEstimationService communicates with external state estimation server via REST (GET/DELETE operations); StateEstimationResultProvider implements ResultProvider interface delegating to StateEstimationService.
Service Configuration
monitor-server/src/main/resources/application-local.yaml
Added state-estimation-server configuration entry with base URI pointing to localhost:6040.
Test Suite
monitor-server/src/test/java/org/gridsuite/monitor/server/services/StateEstimationServiceTest.java, StateEstimationResultProviderTest.java
Unit tests validating service delegation, REST communication (success/failure paths), and proper method invocations using mocks and RestClientTest framework.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant ResultProvider as StateEstimationResultProvider
    participant Service as StateEstimationService
    participant REST as RestTemplate
    participant Server as State Estimation Server

    Client->>ResultProvider: getResult(resultId)
    ResultProvider->>Service: getResult(resultId)
    Service->>REST: GET /v1/results/{resultId}
    REST->>Server: HTTP GET Request
    Server-->>REST: 200 OK + JSON Response
    REST-->>Service: Response Body (String)
    Service-->>ResultProvider: Result String
    ResultProvider-->>Client: Result String

    Client->>ResultProvider: deleteResult(resultId)
    ResultProvider->>Service: deleteResult(resultId)
    Service->>REST: DELETE /v1/results?resultsUuids={resultId}
    REST->>Server: HTTP DELETE Request
    Server-->>REST: 200 OK
    REST-->>Service: Void
    Service-->>ResultProvider: Void
    ResultProvider-->>Client: Void
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A new service hops into place,
StateEstimation joins the race,
REST calls leap through the digital space,
Results fetched with graceful pace,
Tests verify each delegation trace! ✨

🚥 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 accurately describes the main change: implementing state estimation result provider and service classes, which aligns with the primary objective of adding state estimation result handling.
Description check ✅ Passed The description is directly related to the changeset, explaining that the PR adds support for handling state estimation results through new service and provider classes.

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

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch results/implement-state-estimation-result-provider

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.

@sonarqubecloud
Copy link

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-server/src/main/java/org/gridsuite/monitor/server/services/StateEstimationService.java`:
- Around line 36-38: The helper getStateEstimationServerBaseUri() currently
concatenates stateEstimationServerBaseUri, DELIMITER and SE_API_VERSION which
can produce double slashes when stateEstimationServerBaseUri already ends with a
slash; change it to normalize the base by trimming any trailing '/' from
stateEstimationServerBaseUri (or conditionally avoid adding DELIMITER when it
already ends with one) before appending DELIMITER + SE_API_VERSION + DELIMITER
so constructed URLs like //v1//results are eliminated; update any callers
relying on getStateEstimationServerBaseUri() accordingly.

ℹ️ 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 8e717e9.

📒 Files selected for processing (6)
  • monitor-commons/src/main/java/org/gridsuite/monitor/commons/ResultType.java
  • monitor-server/src/main/java/org/gridsuite/monitor/server/services/StateEstimationResultProvider.java
  • monitor-server/src/main/java/org/gridsuite/monitor/server/services/StateEstimationService.java
  • monitor-server/src/main/resources/application-local.yaml
  • monitor-server/src/test/java/org/gridsuite/monitor/server/services/StateEstimationResultProviderTest.java
  • monitor-server/src/test/java/org/gridsuite/monitor/server/services/StateEstimationServiceTest.java

Comment on lines +36 to +38
private String getStateEstimationServerBaseUri() {
return this.stateEstimationServerBaseUri + DELIMITER + SE_API_VERSION + DELIMITER;
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Normalize the base URI to avoid //v1//results requests.

With a trailing slash in the default base URI and leading slashes in paths, the constructed URLs contain double slashes, which can break strict path matching and won’t match the test expectations.

🔧 Proposed fix
-    private String getStateEstimationServerBaseUri() {
-        return this.stateEstimationServerBaseUri + DELIMITER + SE_API_VERSION + DELIMITER;
-    }
+    private String getStateEstimationServerBaseUri() {
+        String baseUri = this.stateEstimationServerBaseUri;
+        if (baseUri.endsWith(DELIMITER)) {
+            baseUri = baseUri.substring(0, baseUri.length() - 1);
+        }
+        return baseUri + DELIMITER + SE_API_VERSION;
+    }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@monitor-server/src/main/java/org/gridsuite/monitor/server/services/StateEstimationService.java`
around lines 36 - 38, The helper getStateEstimationServerBaseUri() currently
concatenates stateEstimationServerBaseUri, DELIMITER and SE_API_VERSION which
can produce double slashes when stateEstimationServerBaseUri already ends with a
slash; change it to normalize the base by trimming any trailing '/' from
stateEstimationServerBaseUri (or conditionally avoid adding DELIMITER when it
already ends with one) before appending DELIMITER + SE_API_VERSION + DELIMITER
so constructed URLs like //v1//results are eliminated; update any callers
relying on getStateEstimationServerBaseUri() accordingly.

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.

2 participants