Entry endpoints - #201
Conversation
… through entry servers from gateway Signed-off-by: LE SAULNIER Kevin <kevin.lesaulnier.pro@gmail.com>
|
Warning Review limit reached
Next review available in: 50 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughChangesThe PR adds versioned explore REST controllers and downstream service methods for case, contingency, filter, monitoring, network conversion, study configuration, study, workspace, and group operations. Services return Explore API
Sequence Diagram(s)sequenceDiagram
participant ExploreClient
participant NetworkConversionController
participant NetworkConversionService
participant NetworkConversionServer
ExploreClient->>NetworkConversionController: submit case conversion request
NetworkConversionController->>NetworkConversionService: convertCase(caseUuid, format, fileName, formatParameters, userId)
NetworkConversionService->>NetworkConversionServer: POST conversion request
NetworkConversionServer-->>NetworkConversionService: conversion UUID or HTTP error
NetworkConversionService-->>NetworkConversionController: ResponseEntity<UUID>
NetworkConversionController-->>ExploreClient: conversion response
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
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 |
Signed-off-by: LE SAULNIER Kevin <kevin.lesaulnier.pro@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/test/java/org/gridsuite/explore/server/NetworkConversionServiceTest.java (1)
49-102: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover downstream error forwarding.
Add error-response cases for these methods. The PR’s pass-through contract includes preserving upstream status and headers, plus bodies for text endpoints, but these tests only exercise successful responses.
🤖 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/test/java/org/gridsuite/explore/server/NetworkConversionServiceTest.java` around lines 49 - 102, Extend the tests for getCaseImportParameters, convertCase, downloadFile, and getExportFormats to cover downstream non-2xx responses. Configure MockRestServiceServer responses with representative error statuses, headers, and bodies where applicable, then assert each service method preserves the upstream status, headers, and text body according to its pass-through contract.
🤖 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/explore/server/services/CaseService.java`:
- Around line 62-76: Update importCaseWithoutDirectoryElementCreation to catch
HttpStatusCodeException from RestTemplate.exchange and return a response
preserving the upstream status, headers, and response body, matching the
handling used by the other proxy methods.
In
`@src/main/java/org/gridsuite/explore/server/services/NetworkConversionService.java`:
- Around line 57-72: Preserve upstream error response bodies in all three proxy
methods: in NetworkConversionService.convertCase, return a body-capable
ResponseEntity and forward e.getResponseBodyAsString(); in
NetworkConversionService’s download method, retain failed-download payload bytes
using a suitable resource such as ByteArrayResource instead of returning an
empty body; and in SpreadsheetConfigCollectionService’s replace-all method,
return a body-capable response and forward the upstream error payload. Apply the
corresponding changes at
src/main/java/org/gridsuite/explore/server/services/NetworkConversionService.java
lines 57-72 and 75-85, and
src/main/java/org/gridsuite/explore/server/services/SpreadsheetConfigCollectionService.java
lines 146-168.
In `@src/test/java/org/gridsuite/explore/server/ActionsControllerTest.java`:
- Around line 52-65: Update ActionsControllerTest.tearDown() to restore
ContingencyListService’s original actions-server base URI after stopping
WireMockServer, or mark the Spring context dirty so the singleton is recreated;
ensure later tests cannot retain the temporary WireMock URI.
---
Nitpick comments:
In
`@src/test/java/org/gridsuite/explore/server/NetworkConversionServiceTest.java`:
- Around line 49-102: Extend the tests for getCaseImportParameters, convertCase,
downloadFile, and getExportFormats to cover downstream non-2xx responses.
Configure MockRestServiceServer responses with representative error statuses,
headers, and bodies where applicable, then assert each service method preserves
the upstream status, headers, and text body according to its pass-through
contract.
🪄 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: fb27d33a-45e3-4c64-ace8-da4bd14cbc39
📒 Files selected for processing (22)
src/main/java/org/gridsuite/explore/server/ActionsController.javasrc/main/java/org/gridsuite/explore/server/CaseController.javasrc/main/java/org/gridsuite/explore/server/FilterController.javasrc/main/java/org/gridsuite/explore/server/MonitorController.javasrc/main/java/org/gridsuite/explore/server/NetworkConversionController.javasrc/main/java/org/gridsuite/explore/server/StudyConfigController.javasrc/main/java/org/gridsuite/explore/server/StudyController.javasrc/main/java/org/gridsuite/explore/server/UserAdminController.javasrc/main/java/org/gridsuite/explore/server/services/CaseService.javasrc/main/java/org/gridsuite/explore/server/services/ContingencyListService.javasrc/main/java/org/gridsuite/explore/server/services/FilterService.javasrc/main/java/org/gridsuite/explore/server/services/MonitorService.javasrc/main/java/org/gridsuite/explore/server/services/NetworkConversionService.javasrc/main/java/org/gridsuite/explore/server/services/SpreadsheetConfigCollectionService.javasrc/main/java/org/gridsuite/explore/server/services/SpreadsheetConfigService.javasrc/main/java/org/gridsuite/explore/server/services/StudyService.javasrc/main/java/org/gridsuite/explore/server/services/UserAdminService.javasrc/main/java/org/gridsuite/explore/server/services/WorkspaceService.javasrc/main/resources/application-local.ymlsrc/test/java/org/gridsuite/explore/server/ActionsControllerTest.javasrc/test/java/org/gridsuite/explore/server/ExplorePassThroughControllersTest.javasrc/test/java/org/gridsuite/explore/server/NetworkConversionServiceTest.java
| public ResponseEntity<UUID> importCaseWithoutDirectoryElementCreation(MultipartFile multipartFile, boolean withExpiration) { | ||
| MultiValueMap<String, Object> body = new LinkedMultiValueMap<>(); | ||
| HttpHeaders headers = new HttpHeaders(); | ||
| headers.setContentType(MediaType.MULTIPART_FORM_DATA); | ||
| if (multipartFile != null) { | ||
| Objects.requireNonNull(multipartFile.getOriginalFilename()); | ||
| body.add("file", multipartFile.getResource()); | ||
| } | ||
| body.add("withExpiration", withExpiration); | ||
| HttpEntity<MultiValueMap<String, Object>> request = new HttpEntity<>(body, headers); | ||
|
|
||
| String path = UriComponentsBuilder.fromPath(DELIMITER + CASE_SERVER_API_VERSION + "/cases") | ||
| .buildAndExpand() | ||
| .toUriString(); | ||
| return restTemplate.exchange(caseServerBaseUri + path, HttpMethod.POST, request, UUID.class); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the service and related proxy methods
git ls-files | rg '^src/main/java/org/gridsuite/explore/server/services/CaseService\.java$|^src/main/java/.*CaseService.*\.java$'
# Show an outline of the file if ast-grep is available
if command -v ast-grep >/dev/null 2>&1; then
ast-grep outline src/main/java/org/gridsuite/explore/server/services/CaseService.java --view expanded
else
echo "ast-grep not available"
fi
# Read the relevant section with line numbers
sed -n '1,220p' src/main/java/org/gridsuite/explore/server/services/CaseService.java | cat -nRepository: gridsuite/explore-server
Length of output: 9183
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for HttpStatusCodeException handling in the codebase
rg -n "HttpStatusCodeException|ResponseEntity\.status\(e\.getStatusCode\(\)\)" src/main/java
# Search for the specific importCaseWithoutDirectoryElementCreation method usages and nearby methods
rg -n "importCaseWithoutDirectoryElementCreation|downloadCase|deleteCase|basename" src/main/java/org/gridsuite/explore/server/services/CaseService.javaRepository: gridsuite/explore-server
Length of output: 6397
Preserve upstream POST failures.
importCaseWithoutDirectoryElementCreation still lets RestTemplate.exchange throw on downstream 4xx/5xx, so case-server errors become entry-server errors instead of forwarding the upstream status and headers. Catch HttpStatusCodeException here like the other proxy methods.
🤖 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/explore/server/services/CaseService.java` around
lines 62 - 76, Update importCaseWithoutDirectoryElementCreation to catch
HttpStatusCodeException from RestTemplate.exchange and return a response
preserving the upstream status, headers, and response body, matching the
handling used by the other proxy methods.
| public ResponseEntity<UUID> convertCase(UUID caseUuid, String format, String fileName, String formatParameters, String userId) { | ||
| String path = UriComponentsBuilder.fromPath(DELIMITER + NETWORK_CONVERSION_API_VERSION + "/cases/{caseUuid}/convert/{format}") | ||
| .queryParam("fileName", fileName) | ||
| .buildAndExpand(caseUuid, format) | ||
| .toUriString(); | ||
|
|
||
| HttpHeaders headers = new HttpHeaders(); | ||
| headers.setContentType(MediaType.APPLICATION_JSON); | ||
| headers.set(HEADER_USER_ID, userId); | ||
| try { | ||
| return restTemplate.exchange(networkConversionServerBaseUri + path, HttpMethod.POST, new HttpEntity<>(formatParameters, headers), UUID.class); | ||
| } catch (HttpStatusCodeException e) { | ||
| return ResponseEntity.status(e.getStatusCode()) | ||
| .headers(Objects.requireNonNullElseGet(e.getResponseHeaders(), HttpHeaders::new)) | ||
| .build(); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve upstream error bodies across typed proxy responses. These success-specific ResponseEntity types force HTTP-error responses to be rebuilt without their payloads, contradicting the pass-through contract.
src/main/java/org/gridsuite/explore/server/services/NetworkConversionService.java#L57-L72: return a body-capable response and forwarde.getResponseBodyAsString()for failed conversions.src/main/java/org/gridsuite/explore/server/services/NetworkConversionService.java#L75-L85: retain failed download payload bytes, such as throughByteArrayResource, instead of returning an empty body.src/main/java/org/gridsuite/explore/server/services/SpreadsheetConfigCollectionService.java#L146-L168: return a body-capable response and forward the replace-all error payload.
📍 Affects 2 files
src/main/java/org/gridsuite/explore/server/services/NetworkConversionService.java#L57-L72(this comment)src/main/java/org/gridsuite/explore/server/services/NetworkConversionService.java#L75-L85src/main/java/org/gridsuite/explore/server/services/SpreadsheetConfigCollectionService.java#L146-L168
🤖 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/explore/server/services/NetworkConversionService.java`
around lines 57 - 72, Preserve upstream error response bodies in all three proxy
methods: in NetworkConversionService.convertCase, return a body-capable
ResponseEntity and forward e.getResponseBodyAsString(); in
NetworkConversionService’s download method, retain failed-download payload bytes
using a suitable resource such as ByteArrayResource instead of returning an
empty body; and in SpreadsheetConfigCollectionService’s replace-all method,
return a body-capable response and forward the upstream error payload. Apply the
corresponding changes at
src/main/java/org/gridsuite/explore/server/services/NetworkConversionService.java
lines 57-72 and 75-85, and
src/main/java/org/gridsuite/explore/server/services/SpreadsheetConfigCollectionService.java
lines 146-168.
| @BeforeEach | ||
| void setUp() { | ||
| wireMockServer = new WireMockServer(wireMockConfig().dynamicPort()); | ||
| wireMockUtils = new WireMockUtils(wireMockServer); | ||
| wireMockServer.start(); | ||
| contingencyListService.setActionsServerBaseUri(wireMockServer.baseUrl()); | ||
| } | ||
|
|
||
| @AfterEach | ||
| void tearDown() { | ||
| if (wireMockServer != null) { | ||
| wireMockServer.stop(); | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the relevant test and service files.
git ls-files | rg '(^|/)ActionsControllerTest\.java$|(^|/)ContingencyListService\.java$|(^|/)WireMockUtils\.java$|(^|/)Abstract.*Test.*\.java$|(^|/)Test.*Config.*\.java$'
# Show concise structure of the test file.
ast-grep outline src/test/java/org/gridsuite/explore/server/ActionsControllerTest.java --view expanded || true
# Show concise structure of the service file if present.
if [ -f src/main/java/org/gridsuite/explore/server/services/ContingencyListService.java ]; then
ast-grep outline src/main/java/org/gridsuite/explore/server/services/ContingencyListService.java --view expanded || true
fi
# Show the specific test section with line numbers.
sed -n '1,220p' src/test/java/org/gridsuite/explore/server/ActionsControllerTest.java
# Search for test context isolation/reset usage around this service.
rg -n "setActionsServerBaseUri|ActionsControllerTest|DirtiesContext|DynamicPropertySource|TestPropertySource|ContextConfiguration|WireMockServer" src/test/java src/main/javaRepository: gridsuite/explore-server
Length of output: 10913
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the service implementation and any configuration that owns its base URI.
if [ -f src/main/java/org/gridsuite/explore/server/services/ContingencyListService.java ]; then
sed -n '1,220p' src/main/java/org/gridsuite/explore/server/services/ContingencyListService.java
fi
# Inspect other tests that use the same WireMock pattern.
rg -n -A3 -B3 "new WireMockServer|setActionsServerBaseUri\(|wireMockConfig\(\)\.dynamicPort\(\)" src/test/javaRepository: gridsuite/explore-server
Length of output: 13413
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the application context / test class hierarchy for ActionsControllerTest.
rg -n -A3 -B3 "`@SpringBootTest`|`@WebMvcTest`|`@ExtendWith`|`@ContextConfiguration`|`@DirtiesContext`|abstract class|class ActionsControllerTest" src/test/java/org/gridsuite/explore/server/ActionsControllerTest.java src/test/java
# If present, inspect test properties or bean definitions that might make the service bean prototype-scoped or reset between tests.
rg -n -A2 -B2 "ContingencyListService|actionsServerBaseUri|baseUri" src/main/java src/test/javaRepository: gridsuite/explore-server
Length of output: 29739
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the exact test annotations and any context-reset annotations.
sed -n '1,140p' src/test/java/org/gridsuite/explore/server/ActionsControllerTest.java
# Locate the service bean definition and scope.
rg -n -A4 -B4 "class ContingencyListService|`@Service`|`@Scope`|setActionsServerBaseUri|actionsServerBaseUri" src/main/java src/test/java
# Find other tests that use the same Spring test context pattern.
rg -n -A2 -B2 "`@SpringBootTest`|`@WebMvcTest`|`@DirtiesContext`" src/test/java/org/gridsuite/explore/serverRepository: gridsuite/explore-server
Length of output: 47988
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# List every test that autowires or references ContingencyListService.
rg -n -A4 -B4 "ContingencyListService|setActionsServerBaseUri\(" src/test/java
# Inspect the setup method in ExploreTest, which also mutates this service.
sed -n '160,215p' src/test/java/org/gridsuite/explore/server/ExploreTest.java
# Find test classes using the same Spring Boot test context as ActionsControllerTest.
rg -n -A2 -B2 "`@SpringBootTest`\(classes = \{ExploreApplication\.class, TestChannelBinderConfiguration\.class\}\)|`@ContextConfiguration`\(classes = \{ExploreApplication\.class, TestChannelBinderConfiguration\.class\}\)" src/test/java/org/gridsuite/explore/serverRepository: gridsuite/explore-server
Length of output: 14690
Restore the actions-server base URI after each test. ContingencyListService is a singleton, so stopping wireMockServer leaves the bean pointing at a dead port. Reset it in tearDown() or dirty the Spring context to avoid leaking the test URI into later tests.
🤖 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/test/java/org/gridsuite/explore/server/ActionsControllerTest.java` around
lines 52 - 65, Update ActionsControllerTest.tearDown() to restore
ContingencyListService’s original actions-server base URI after stopping
WireMockServer, or mark the Spring context dirty so the singleton is recreated;
ensure later tests cannot retain the temporary WireMock URI.
|
|
||
| @RestController | ||
| @RequestMapping(value = "/" + ExploreApi.API_VERSION + "/explore") | ||
| public class MonitorController { |
PR Summary
Implements all entry endpoints to make all API calls from front pass through entry server rather than directly to target server