Skip to content

Use the container API of network-modification-server - #1117

Merged
flomillot merged 4 commits into
mainfrom
refactor/unify-container-api
Oct 2, 2026
Merged

flomillot merged 4 commits into
mainfrom
refactor/unify-container-api

Conversation

@flomillot

@flomillot flomillot commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Follows gridsuite/network-modification-server#908, which reads groups and composites through the same /containers/... API.

  • The modifications, count, export, verification and references of a group, and the content of composites, are read through the /containers/... endpoints.
  • errorOnGroupNotFound is no longer sent: network-modification-server now always tolerates a missing group.

The API exposed to the front-end is unchanged.

Must be deployed with gridsuite/network-modification-server#908 and gridsuite/explore-server#226.

Read the modifications, count, export, verification and references of a group
through the /containers/... endpoints, and stop sending errorOnGroupNotFound.

Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>
@coderabbitai

coderabbitai Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

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

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 4 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: f6bb9bfe-e881-4ac6-acc7-685d6c9f6c33

📥 Commits

Reviewing files that changed from the base of the PR and between 86247a7 and 8baa5d2.

📒 Files selected for processing (8)
  • src/main/java/org/gridsuite/study/server/StudyConstants.java
  • src/main/java/org/gridsuite/study/server/service/NetworkModificationService.java
  • src/test/java/org/gridsuite/study/server/NetworkModificationTest.java
  • src/test/java/org/gridsuite/study/server/NetworkModificationTreeTest.java
  • src/test/java/org/gridsuite/study/server/VoltageInitTest.java
  • src/test/java/org/gridsuite/study/server/service/NetworkModificationServiceTest.java
  • src/test/java/org/gridsuite/study/server/studycontroller/StudyTest.java
  • src/test/java/org/gridsuite/study/server/utils/wiremock/WireMockStubs.java

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: af37d800-470d-44be-91c1-c0b60c8943b0

📥 Commits

Reviewing files that changed from the base of the PR and between 1205efe and 86247a7.

📒 Files selected for processing (1)
  • src/main/java/org/gridsuite/study/server/service/NetworkModificationService.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

Network-modification retrieval and related requests now use container endpoints. Modification deletion requests no longer include the errorOnGroupNotFound query parameter. Tests and request stubs reflect these changes.

Changes

Network modification container API

Layer / File(s) Summary
Container endpoint requests
src/main/java/org/gridsuite/study/server/service/NetworkModificationService.java, src/test/java/org/gridsuite/study/server/NetworkModificationTest.java, src/test/java/org/gridsuite/study/server/NetworkModificationTreeTest.java, src/test/java/org/gridsuite/study/server/VoltageInitTest.java, src/test/java/org/gridsuite/study/server/service/NetworkModificationServiceTest.java, src/test/java/org/gridsuite/study/server/utils/wiremock/WireMockStubs.java
Retrieval, export, count, reference, verification, and composite-modification requests use container endpoints. Corresponding test stubs and assertions use the updated routes.
Deletion query parameters
src/main/java/org/gridsuite/study/server/StudyConstants.java, src/main/java/org/gridsuite/study/server/service/NetworkModificationService.java, src/test/java/org/gridsuite/study/server/service/NetworkModificationServiceTest.java, src/test/java/org/gridsuite/study/server/studycontroller/StudyTest.java, src/test/java/org/gridsuite/study/server/utils/wiremock/WireMockStubs.java
Deletion requests no longer include errorOnGroupNotFound. The constant and test helper argument for that parameter are removed, and deletion tests are updated.

Suggested reviewers: souissimai

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 86247

The route migration is supported by the available evidence, with no concrete incompatible request found. Ship this change with the companion network-modification-server update as specified.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 86247

The client-side identity propagation and checks before mutation are preserved, and no newly introduced security bypass was established. Risk remains in compatibility between service versions: the receiving server’s container scope, missing-group behavior, and rollback support could not be confirmed.

Retained concerns

  • Low · architecture · inferred: The new client depends on container endpoints and server-side missing-group tolerance. An incompatible mixed-version deployment or downstream rollback could block verification and lifecycle cleanup. The deployment dependency is explicit, but exact compatibility and recovery behavior remain unverified; no actual mismatch was established.
Security review details

Security Blast Radius

  • inferred — The visible scope includes caller-selected modification containers and shared modifications whose applicability can affect multiple referencing studies. No newly added caller input is shown, but maximum asset or tenant exposure cannot be bounded without the receiving container API’s scope and authorization rules.

Trust Boundaries and Controls

  • observed — Writable-reference authorization preserves the userId header and propagates downstream errors rather than bypassing them. Root-network tag changes perform that check before updates; applicability mutations retain node ownership and shared-element WRITE checks. These local controls are counterevidence to an introduced fail-open mutation path, not proof of downstream policy equivalence.

Hardening Proposals

  • proposed — Validate the paired service contract for unauthorized and wrong-container-type UUIDs, missing groups, repeated and partially completed deletion, and supported rollout/rollback combinations. This would resolve the remaining scope and lifecycle uncertainties beyond mocked request-shape tests.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 5.41% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 37 functions across 7 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 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 and concisely describes the main change: switching network-modification-server access to the container API.
Description check ✅ Passed The description directly explains the endpoint changes, removal of errorOnGroupNotFound, unchanged front-end API, and deployment dependencies.
✨ Finishing Touches 💡 1
⚔️ Resolve merge conflicts 💡
  • Resolve merge conflict in branch refactor/unify-container-api

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.

@Meklo

Meklo commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

NetworkModificationService::hasModificationReferences and NetworkModificationService::assertReferencedModificationsAreWritable could also use the newly added CONTAINERS constant

Signed-off-by: Florent MILLOT <75525996+flomillot@users.noreply.github.com>
…ner-api

Signed-off-by: Florent MILLOT <75525996+flomillot@users.noreply.github.com>

# Conflicts:
#	src/test/java/org/gridsuite/study/server/NetworkModificationTest.java
#	src/test/java/org/gridsuite/study/server/NetworkModificationTreeTest.java
…ner-api

Signed-off-by: Florent MILLOT <75525996+flomillot@users.noreply.github.com>
@sonarqubecloud

sonarqubecloud Bot commented Oct 2, 2026

Copy link
Copy Markdown

@flomillot
flomillot merged commit 6dea5fe into main Oct 2, 2026
5 checks passed
@flomillot
flomillot deleted the refactor/unify-container-api branch October 2, 2026 15:28
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