Import export study#194
Open
ghazwarhili wants to merge 4 commits into
Open
Conversation
📝 WalkthroughWalkthroughModification DTOs gain support for extracting referenced filter and load-flow-parameter UUIDs and embedding their resolved content for export. ChangesReferenced data embedding
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 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 |
|
ghazwarhili
requested review from
EtienneLt,
SlimaneAmar and
etiennehomer
as code owners
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/main/java/org/gridsuite/modification/dto/ByFormulaModificationInfos.java (1)
74-87: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winSame NPE risk on
filtersmap as flagged on the base contract.
filters.get(ref.getId())(Line 84) assumesfiltersis non-null.🤖 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/modification/dto/ByFormulaModificationInfos.java` around lines 74 - 87, The embedReferencedData method in ByFormulaModificationInfos still assumes the filters map is non-null when resolving references. Update the stream/forEach logic to guard against a null filters argument before calling filters.get(ref.getId()), or skip setting filterContent when the map is absent, while keeping the existing formulaInfosList and getFilters() null checks intact.src/main/java/org/gridsuite/modification/dto/BalancesAdjustmentModificationInfos.java (1)
95-106: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winNPE risk if
lfParamsis null.
lfParams.get(loadFlowParametersId)(Line 104) will throwNullPointerExceptionif the caller passes a null map, since only the presence ofloadFlowParametersIdis checked, notlfParamsitself. This mirrors the same pattern flagged inModificationInfos.embedReferencedData— see that comment for the base-contract concern.🤖 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/modification/dto/BalancesAdjustmentModificationInfos.java` around lines 95 - 106, The embedReferencedData method in BalancesAdjustmentModificationInfos can throw a NullPointerException because it dereferences lfParams without checking it first. Update embedReferencedData to guard both loadFlowParametersId and lfParams before calling lfParams.get(...), and only assign loadFlowParameters when the map is present. Keep the behavior consistent with the base embedReferencedData contract and the null-safe pattern used elsewhere in ModificationInfos.
🧹 Nitpick comments (2)
src/main/java/org/gridsuite/modification/dto/FilterInfos.java (1)
35-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider excluding
filterContentfromtoString().
FilterInfoscarries@ToString(class-level), andfilterContentcan hold a full nestedAbstractFilterpayload. SinceByFormulaModificationInfos/ModificationByAssignmentInfosuse@ToString(callSuper = true)and nestFilterInfosinside lists, any incidentaltoString()call (e.g., in exception messages or debug logs) could produce very large/verbose output once this field is populated during export.♻️ Suggested exclusion
`@JsonInclude`(JsonInclude.Include.NON_NULL) `@Schema`(description = "embedded filter content (populated only on export)") + `@ToString.Exclude` private AbstractFilter filterContent;🤖 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/modification/dto/FilterInfos.java` around lines 35 - 37, `FilterInfos` currently includes `filterContent` in the Lombok-generated `toString()`, which can make logs and exception messages extremely verbose when nested under `ByFormulaModificationInfos` or `ModificationByAssignmentInfos`. Update the class-level `@ToString` handling on `FilterInfos` so `filterContent` is excluded from the generated string output, while keeping the field itself intact for export serialization.src/main/java/org/gridsuite/modification/dto/ByFormulaModificationInfos.java (1)
64-87: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate traversal logic with
ModificationByAssignmentInfos.
getReferencedFilterUuids()/embedReferencedData()here are structurally identical to the overrides inModificationByAssignmentInfos(stream → flatMap over a null-safe filter list → extract/set onFilterInfos). Consider extracting a shared static helper (e.g., inModificationInfosor a new utility) parameterized by aFunctionthat maps an element to itsList<FilterInfos>, to avoid maintaining two copies of the same logic.♻️ Example shared helper sketch
// e.g. in ModificationInfos or a shared utility class protected static <T> List<UUID> extractFilterUuids(List<T> items, Function<T, List<FilterInfos>> filtersGetter) { return items == null ? List.of() : items.stream() .flatMap(i -> filtersGetter.apply(i) == null ? Stream.empty() : filtersGetter.apply(i).stream()) .map(FilterInfos::getId) .filter(Objects::nonNull) .toList(); } protected static <T> void embedFilterContent(List<T> items, Function<T, List<FilterInfos>> filtersGetter, Map<UUID, AbstractFilter> filters) { if (items == null) { return; } items.stream() .flatMap(i -> filtersGetter.apply(i) == null ? Stream.empty() : filtersGetter.apply(i).stream()) .forEach(ref -> { if (ref.getId() != null) { ref.setFilterContent(filters.get(ref.getId())); } }); }🤖 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/modification/dto/ByFormulaModificationInfos.java` around lines 64 - 87, Both getReferencedFilterUuids() and embedReferencedData() in ByFormulaModificationInfos duplicate the same null-safe filter traversal logic already present in ModificationByAssignmentInfos. Extract that stream/flatMap/map/embed behavior into a shared helper in ModificationInfos or a small utility, parameterized by a Function that returns each item’s List<FilterInfos>, and update both overrides to delegate to it so the two classes stay consistent and only one copy of the traversal needs maintenance.
🤖 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.
Duplicate comments:
In
`@src/main/java/org/gridsuite/modification/dto/BalancesAdjustmentModificationInfos.java`:
- Around line 95-106: The embedReferencedData method in
BalancesAdjustmentModificationInfos can throw a NullPointerException because it
dereferences lfParams without checking it first. Update embedReferencedData to
guard both loadFlowParametersId and lfParams before calling lfParams.get(...),
and only assign loadFlowParameters when the map is present. Keep the behavior
consistent with the base embedReferencedData contract and the null-safe pattern
used elsewhere in ModificationInfos.
In
`@src/main/java/org/gridsuite/modification/dto/ByFormulaModificationInfos.java`:
- Around line 74-87: The embedReferencedData method in
ByFormulaModificationInfos still assumes the filters map is non-null when
resolving references. Update the stream/forEach logic to guard against a null
filters argument before calling filters.get(ref.getId()), or skip setting
filterContent when the map is absent, while keeping the existing
formulaInfosList and getFilters() null checks intact.
---
Nitpick comments:
In
`@src/main/java/org/gridsuite/modification/dto/ByFormulaModificationInfos.java`:
- Around line 64-87: Both getReferencedFilterUuids() and embedReferencedData()
in ByFormulaModificationInfos duplicate the same null-safe filter traversal
logic already present in ModificationByAssignmentInfos. Extract that
stream/flatMap/map/embed behavior into a shared helper in ModificationInfos or a
small utility, parameterized by a Function that returns each item’s
List<FilterInfos>, and update both overrides to delegate to it so the two
classes stay consistent and only one copy of the traversal needs maintenance.
In `@src/main/java/org/gridsuite/modification/dto/FilterInfos.java`:
- Around line 35-37: `FilterInfos` currently includes `filterContent` in the
Lombok-generated `toString()`, which can make logs and exception messages
extremely verbose when nested under `ByFormulaModificationInfos` or
`ModificationByAssignmentInfos`. Update the class-level `@ToString` handling on
`FilterInfos` so `filterContent` is excluded from the generated string output,
while keeping the field itself intact for export serialization.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 183fb5bf-a8ab-4db1-894e-0032520c121e
📒 Files selected for processing (8)
src/main/java/org/gridsuite/modification/dto/BalancesAdjustmentModificationInfos.javasrc/main/java/org/gridsuite/modification/dto/ByFormulaModificationInfos.javasrc/main/java/org/gridsuite/modification/dto/FilterInfos.javasrc/main/java/org/gridsuite/modification/dto/ModificationByAssignmentInfos.javasrc/main/java/org/gridsuite/modification/dto/ModificationInfos.javasrc/test/java/org/gridsuite/modification/modifications/byfilter/assignment/AbstractModificationByAssignmentTest.javasrc/test/java/org/gridsuite/modification/modifications/byfilter/assignment/GeneratorModificationByAssignmentTest.javasrc/test/java/org/gridsuite/modification/modifications/byfilter/formula/AbstractByFormulaModificationTest.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Summary