WIP : Separate set and reset Parameters for PCCMin

[basseche]

PR Summary

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 9f67eb8a-73e1-4520-b3a8-fedb96c8b108

📥 Commits

Reviewing files that changed from the base of the PR and between 04c2f61 and 85421b8.

📒 Files selected for processing (4)

• src/main/java/org/gridsuite/study/server/controller/pccmin/PccMinStudyParametersController.java
• src/main/java/org/gridsuite/study/server/service/pccmin/PccMinService.java
• src/test/java/org/gridsuite/study/server/PccMinTest.java
• src/test/java/org/gridsuite/study/server/controller/pccmin/PccMinStudyParametersControllerTest.java

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

PCC-min parameter setting now has a required request body and always returns 200 OK. A separate reset endpoint applies profile-based or default parameters and returns a status based on the service result.

Changes

PCC-min parameter reset

Layer / File(s)	Summary
Service parameter set and reset src/main/java/org/gridsuite/study/server/service/pccmin/PccMinService.java	Parameter setting no longer returns a profile-issue flag. Reset handling attempts profile parameter duplication, applies defaults when duplication is unavailable or fails, and returns the profile-issue result. Both operations invalidate status and emit existing notifications.
Reset endpoint and coverage src/main/java/org/gridsuite/study/server/controller/pccmin/PccMinStudyParametersController.java, src/test/java/org/gridsuite/study/server/PccMinTest.java, src/test/java/org/gridsuite/study/server/controller/pccmin/PccMinStudyParametersControllerTest.java	Parameter setting requires a request body and returns 200 OK. The reset endpoint returns 204 when the service reports a profile issue and 200 otherwise. Tests exercise the endpoint and both response cases.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant Controller as PccMinStudyParametersController
  participant Service as PccMinService
  Client->>Controller: POST /parameters/reset
  Controller->>Service: resetPccMinParameters(studyUuid, userId)
  Service-->>Controller: Return profile-issue result
  Controller-->>Client: Return 204 if true, otherwise 200

Loading

Suggested reviewers: klesaulnier

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 85421

The separate reset endpoint is mergeable after normal checks. The investigated fallback concern predates this change.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 85421

The reset capability and its authority appear unchanged; the main change is how callers invoke it. No introduced security regression was established. Authentication, study-access enforcement for the new route, and downstream recovery guarantees remain unverified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — One successful invocation affects the selected study's parameter reference, associated external parameter resources, and PCC-min statuses across that study's nodes. Maximum unauthorized cross-study or cross-user exposure cannot be determined without effective ingress and authorization policy.

Security Findings and Attack Paths

• inferred — The endpoint split alone does not establish newly gained authority: profile lookup, parameter replacement, cleanup, and invalidation were already reachable through bodyless setting requests with the same study and user inputs. No introduced or worsened attack path was established by the inspected comparison.

Trust Boundaries and Controls

• observed — The local study lookup checks existence rather than caller ownership. The inspected controller and service do not establish authenticated binding of the user-ID header or study-access authorization. These local conditions predate the split; upstream enforcement and policy equivalence for the new route remain coverage gaps, not verified bypasses.

Resilience and Maintainability Implications

• inferred — Database UUID persistence and external duplication/deletion remain separate lifecycle operations. Atomic recovery, retry idempotency, and concurrent-reset ownership are not established. The extraction preserves these existing uncertainties rather than demonstrating stronger or weaker containment.

Hardening Proposals

• proposed — Verify that both routes receive equivalent authenticated user-ID binding and study-access enforcement before rollout, including deployments with path-specific gateway rules.

🚥 Pre-merge checks | ✅ 3 | ❌ 1 | ❓ 1

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 21 functions across 4 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	❓ Inconclusive	The description contains only a template and does not explain the PCC-min parameter changes. It is too vague to confirm that it adequately describes the changeset.	Add a brief summary that explains the separate set and reset endpoints, the service behavior changes, and the related test updates.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly identifies the main change: separating PCC-min parameter setting and resetting. The “WIP” prefix adds minor noise but does not make the title unclear.

• Fix all pre-merge checks with AI

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud