Fix Sonar issues #214
Fix Sonar issues #214
Conversation
WalkthroughThis pull request encompasses a series of modifications across multiple Java classes in the CSE (Central South Europe) project. The changes primarily focus on refining error handling, simplifying code structure, and updating method signatures. Key areas of modification include URL handling, exception management, collection processing, and method implementations. The updates aim to improve code readability, maintainability, and robustness by leveraging modern Java features and streamlining control flow. Changes
Poem
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
cse-cc-export-runner-app/src/main/java/com/farao_community/farao/cse/export_runner/app/services/CseExportRunner.java (1)
Line range hint
158-162: Align exception handling with BorderExchanges class
- The load flow error handling should be consistent with the
BorderExchangesclass:
- Both classes should use the same exception type
- Both should handle non-converged cases similarly
- Consider the impact of non-converged cases on PRA results
Consider these improvements:
- if (result.isFailed()) { + if (!result.isOk()) { + String status = result.isFailed() ? "failed" : "did not converge"; LOGGER.error("Loadflow computation diverged on network '{}'", network.getId()); - throw new CseInternalException(String.format("Loadflow computation diverged on network %s", network.getId())); + throw new CseComputationException(String.format("Loadflow computation %s on network %s", status, network.getId())); }Let's verify the exception handling consistency:
#!/bin/bash # Search for exception usage patterns echo "Searching for CseComputationException usage..." rg "CseComputationException" echo "Searching for CseInternalException usage..." rg "CseInternalException"
🧹 Nitpick comments (12)
cse-cc-import-runner-app/src/main/java/com/farao_community/farao/cse/import_runner/app/services/MerchantLineService.java (1)
75-75: Good use of Optional's functional methods!The refactoring to use
mapandorElseimproves readability and follows functional programming practices. Consider breaking the line for better readability:- final double mendrisioCagnoTargetFlow = reducedFlow.map(aDouble -> Math.min(defaultFlow, aDouble)).orElse(defaultFlow); + final double mendrisioCagnoTargetFlow = reducedFlow + .map(aDouble -> Math.min(defaultFlow, aDouble)) + .orElse(defaultFlow);cse-lib/data/src/test/java/com/farao_community/farao/cse/data/ntc2/Ntc2Test.java (1)
Line range hint
47-47: Consider improving test method naming and organization.While the tests are comprehensive, consider these improvements:
- Fix the typo in "testConstrutor" → "testConstructor"
- Use more descriptive BDD-style names (e.g., "should_return_correct_exchange_values_for_all_countries")
- Consider moving test data to constants for better maintainability
- void testConstrutor() { + void should_return_correct_exchange_values_for_all_countries() {cse-cc-import-runner-app/src/main/java/com/farao_community/farao/cse/import_runner/app/util/Ntc2Util.java (1)
Line range hint
53-58: Consider extracting magic numbers as constants.The values 96, 24, and 15 are used for interval calculations. Consider extracting these as named constants to improve code maintainability and readability:
private static final String IT_AREA_CODE = "10YIT-GRTN-----B"; + private static final int QUARTER_HOURLY_INTERVALS = 96; + private static final int HOURLY_INTERVALS = 24; + private static final int MINUTES_PER_QUARTER = 15; private Ntc2Util() { }Then update the usage:
- if (qtyByPositionMap.size() == 96) { - if (targetDateInCETZone.getMinute() % 15 != 0) { + if (qtyByPositionMap.size() == QUARTER_HOURLY_INTERVALS) { + if (targetDateInCETZone.getMinute() % MINUTES_PER_QUARTER != 0) { throw new CseDataException("Minutes must be a multiple of 15 for 96 intervals"); } position = 1 + (4 * targetDateInCETZone.getHour()) + (targetDateInCETZone.getMinute() / 15); - } else if (qtyByPositionMap.size() == 24) { + } else if (qtyByPositionMap.size() == HOURLY_INTERVALS) {cse-lib/network-processing/src/main/java/com/farao_community/farao/cse/network_processing/ucte_pst_change/UctePstProcessor.java (2)
31-31: LGTM! Consider enhancing the warning message.Good refactoring to centralize the warning message. The implementation follows Java best practices with proper immutability.
Consider adding the node ID to the warning message for better debugging:
- this.warnMessageNoPstAtVoltageLevel = String.format("No PST at voltage level (%s) has been found.", voltageLevel); + this.warnMessageNoPstAtVoltageLevel = String.format("No PST at voltage level (%s) and node (%s) has been found.", voltageLevel, nodeId);Also applies to: 37-37
50-50: LGTM! Consider reducing code duplication.The warning message changes are consistently applied. However, there's an opportunity to reduce code duplication across these three methods.
Consider extracting the common pattern into a private method:
+ private void processPhaseTapChanger(Network network, Consumer<PhaseTapChanger> pstConfigurator) { + TwoWindingsTransformer transformer = getTwoWindingsTransformerAtVoltageLevelWithPhaseTapChanger(network); + if (transformer != null) { + PhaseTapChanger phaseTapChanger = transformer.getPhaseTapChanger(); + pstConfigurator.accept(phaseTapChanger); + setTransformerInActivePowerRegulation(transformer, phaseTapChanger); + } else { + businessLogger.warn(warnMessageNoPstAtVoltageLevel); + } + }Then simplify the existing methods:
public void forcePhaseTapChangerInActivePowerRegulationForIdcc(Network network, double defaultRegulationValue) { - TwoWindingsTransformer transformer = getTwoWindingsTransformerAtVoltageLevelWithPhaseTapChanger(network); - if (transformer != null) { - PhaseTapChanger phaseTapChanger = transformer.getPhaseTapChanger(); - double regulationValue = phaseTapChanger.getRegulationValue(); - if (Double.isNaN(regulationValue)) { - phaseTapChanger.setRegulationValue(defaultRegulationValue); - } - setTransformerInActivePowerRegulation(transformer, phaseTapChanger); - } else { - businessLogger.warn(warnMessageNoPstAtVoltageLevel); - } + processPhaseTapChanger(network, phaseTapChanger -> { + double regulationValue = phaseTapChanger.getRegulationValue(); + if (Double.isNaN(regulationValue)) { + phaseTapChanger.setRegulationValue(defaultRegulationValue); + } + }); }Similar simplifications can be applied to the other two methods.
Also applies to: 61-61, 89-89
cse-lib/network-processing/src/main/java/com/farao_community/farao/cse/network_processing/busbar_change/NetworkModifier.java (1)
157-162: Excellent use of modern Java switch expressions!The switch expression with pattern matching improves:
- Type safety by leveraging compile-time checks
- Code readability by eliminating verbose if-else chains
- Maintainability by ensuring exhaustive handling of branch types
Consider adding a comment documenting the minimum Java version requirement, as this syntax requires Java 17 or later.
cse-lib/data/src/main/java/com/farao_community/farao/cse/data/ttc_rao/TtcRao.java (1)
198-204: Consider breaking down the stream chain for better readabilityThe current chain of stream operations is complex and could be harder to debug. Consider extracting intermediate steps into well-named variables.
- importedRemedialActionCreationContext.stream() - .filter(CsePstCreationContext.class::isInstance) - .map(CsePstCreationContext.class::cast) - .filter(csePstCreationContext -> cracResultsHelper.getCurativePstRangeActionIds(contingencyId).contains(csePstCreationContext.getCreatedObjectId())) - .forEach(csePstCreationContext -> addPstAction(curativeActions, csePstCreationContext, raId -> cracResultsHelper.getTapOfPstRangeActionInCurative(contingencyId, raId))); + var pstContexts = importedRemedialActionCreationContext.stream() + .filter(CsePstCreationContext.class::isInstance) + .map(CsePstCreationContext.class::cast); + + var eligiblePstContexts = pstContexts + .filter(csePstCreationContext -> + cracResultsHelper.getCurativePstRangeActionIds(contingencyId) + .contains(csePstCreationContext.getCreatedObjectId())); + + eligiblePstContexts.forEach(csePstCreationContext -> + addPstAction(curativeActions, csePstCreationContext, + raId -> cracResultsHelper.getTapOfPstRangeActionInCurative(contingencyId, raId)));cse-cc-import-runner-app/src/main/java/com/farao_community/farao/cse/import_runner/app/util/FileUtil.java (1)
31-34: LGTM! Improved URL handling using URI.The change to use
URIfor URL validation is a good improvement as it provides better validation and parsing of URLs.Consider adding a comment explaining why URI is preferred over direct URL parsing, for future maintainers:
public static String getFilenameFromUrl(String url) { try { + // Use URI for robust URL validation and parsing return FilenameUtils.getName(new URI(url).toURL().getPath()); } catch (MalformedURLException | URISyntaxException | IllegalArgumentException e) {cse-lib/data/src/main/java/com/farao_community/farao/cse/data/cnec/CracResultsHelper.java (1)
237-257: Consider simplifying the flow control logic.The nested if-else statements make the code harder to follow. Consider using early returns to flatten the structure:
- if (upperBound.isPresent()) { - if (lowerBound.isPresent()) { - double flowTemp = raoResult.getFlow(optimizedInstant, flowCnec, monitoredSide, Unit.AMPERE); - flow = Math.abs(flowTemp); - if (flowTemp >= 0) { - iMax = upperBound.get(); - } else { - iMax = Math.abs(lowerBound.get()); - } - } else { - flow = raoResult.getFlow(optimizedInstant, flowCnec, monitoredSide, Unit.AMPERE); - iMax = upperBound.get(); - } - } else { - if (lowerBound.isPresent()) { - // Case where it is limited in opposite direction so the flow is inverted - flow = -raoResult.getFlow(optimizedInstant, flowCnec, monitoredSide, Unit.AMPERE); - iMax = Math.abs(lowerBound.get()); - } else { - throw new CseDataException(String.format("Cnec %s is defined with no thresholds", flowCnec.getName())); - } - } + if (!upperBound.isPresent() && !lowerBound.isPresent()) { + throw new CseDataException(String.format("Cnec %s is defined with no thresholds", flowCnec.getName())); + } + + double currentFlow = raoResult.getFlow(optimizedInstant, flowCnec, monitoredSide, Unit.AMPERE); + + if (upperBound.isPresent() && lowerBound.isPresent()) { + flow = Math.abs(currentFlow); + iMax = currentFlow >= 0 ? upperBound.get() : Math.abs(lowerBound.get()); + return new FlowCnecResult(flow, iMax); + } + + if (upperBound.isPresent()) { + return new FlowCnecResult(currentFlow, upperBound.get()); + } + + // Case where it is limited in opposite direction + return new FlowCnecResult(-currentFlow, Math.abs(lowerBound.get()));cse-cc-export-runner-app/src/main/java/com/farao_community/farao/cse/export_runner/app/services/FileImporter.java (2)
73-73: Enhance NOSONAR comment documentationWhile the NOSONAR comment explains why the suppression is needed, it could be more descriptive about the specific security measures in place.
Consider expanding the comment to:
-// NOSONAR Usage of whitelist not triggered by Sonar quality assessment, even if listed as a solution to the vulnerability +// NOSONAR: URL stream opening is secured through whitelist validation in the previous lines. This is a false positive from Sonar as it doesn't recognize the whitelist check as a security measure.
74-77: Improve error message clarityThe error message could be more specific about which type of error occurred.
- throw new CseDataException(String.format("Exception occurred while retrieving file content from %s", urlString), e); + String errorType = e instanceof URISyntaxException ? "invalid URL format" : + e instanceof IllegalArgumentException ? "malformed URL" : "IO error"; + throw new CseDataException(String.format("Failed to retrieve file content from %s: %s", urlString, errorType), e);cse-lib/data/src/test/java/com/farao_community/farao/cse/data/CseReferenceExchangesTest.java (1)
48-60: Consistent error handling pattern appliedThe same robust resource management pattern is consistently applied across test methods, improving code quality.
However, consider extracting the common test setup to reduce duplication.
+ private void assertThrowsForInvalidDateTime(OffsetDateTime dateTime) throws IOException { + try (final InputStream vulcanusFile = getClass().getResourceAsStream("vulcanus_28122019_96.xls")) { + assertThrows( + CseDataException.class, + () -> cseReferenceExchanges = CseReferenceExchanges.fromVulcanusFile( + dateTime, + vulcanusFile, + "vulcanus_28122019_96.xls", + ProcessType.IDCC + ) + ); + } + } + @Test void creationThrowsExceptionWhenMinutesNotAMultipleOf15() throws IOException { - final OffsetDateTime dateTime = OffsetDateTime.parse("2019-12-28T14:05Z"); - try (final InputStream vulcanusFile = getClass().getResourceAsStream("vulcanus_28122019_96.xls")) { - assertThrows( - CseDataException.class, - () -> cseReferenceExchanges = CseReferenceExchanges.fromVulcanusFile( - dateTime, - vulcanusFile, - "vulcanus_28122019_96.xls", - ProcessType.IDCC - ) - ); - } + assertThrowsForInvalidDateTime(OffsetDateTime.parse("2019-12-28T14:05Z")); } @Test void creationThrowsExceptionWhenTargetDateTimeDifferentFromFileDate() throws IOException { - final OffsetDateTime dateTime = OffsetDateTime.parse("2019-12-29T14:30Z"); - try (final InputStream vulcanusFile = getClass().getResourceAsStream("vulcanus_28122019_96.xls")) { - assertThrows( - CseDataException.class, - () -> cseReferenceExchanges = CseReferenceExchanges.fromVulcanusFile( - dateTime, - vulcanusFile, - "vulcanus_28122019_96.xls", - ProcessType.IDCC - ) - ); - } + assertThrowsForInvalidDateTime(OffsetDateTime.parse("2019-12-29T14:30Z")); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (29)
cse-cc-export-runner-app/src/main/java/com/farao_community/farao/cse/export_runner/app/FileUtil.java(3 hunks)cse-cc-export-runner-app/src/main/java/com/farao_community/farao/cse/export_runner/app/services/CseExportRunner.java(1 hunks)cse-cc-export-runner-app/src/main/java/com/farao_community/farao/cse/export_runner/app/services/FileImporter.java(2 hunks)cse-cc-import-runner-app/src/main/java/com/farao_community/farao/cse/import_runner/app/dichotomy/MultipleDichotomyRunner.java(1 hunks)cse-cc-import-runner-app/src/main/java/com/farao_community/farao/cse/import_runner/app/dichotomy/ZonalScalableProvider.java(1 hunks)cse-cc-import-runner-app/src/main/java/com/farao_community/farao/cse/import_runner/app/services/FileImporter.java(2 hunks)cse-cc-import-runner-app/src/main/java/com/farao_community/farao/cse/import_runner/app/services/InitialShiftService.java(5 hunks)cse-cc-import-runner-app/src/main/java/com/farao_community/farao/cse/import_runner/app/services/MerchantLineService.java(2 hunks)cse-cc-import-runner-app/src/main/java/com/farao_community/farao/cse/import_runner/app/services/UrlValidationService.java(0 hunks)cse-cc-import-runner-app/src/main/java/com/farao_community/farao/cse/import_runner/app/util/FileUtil.java(2 hunks)cse-cc-import-runner-app/src/main/java/com/farao_community/farao/cse/import_runner/app/util/Ntc2Util.java(2 hunks)cse-cc-import-runner-app/src/test/java/com/farao_community/farao/cse/import_runner/app/UrlValidationServiceTest.java(0 hunks)cse-cc-import-runner-app/src/test/java/com/farao_community/farao/cse/import_runner/app/dichotomy/MultipleDichotomyRunnerTest.java(1 hunks)cse-cc-import-runner-app/src/test/java/com/farao_community/farao/cse/import_runner/app/dichotomy/ZonalScalableProviderTest.java(3 hunks)cse-cc-import-runner-app/src/test/java/com/farao_community/farao/cse/import_runner/app/services/FileImporterTest.java(3 hunks)cse-lib/computation/src/main/java/com/farao_community/farao/cse/computation/BorderExchanges.java(1 hunks)cse-lib/data/src/main/java/com/farao_community/farao/cse/data/cnec/CracResultsHelper.java(6 hunks)cse-lib/data/src/main/java/com/farao_community/farao/cse/data/ttc_rao/TtcRao.java(4 hunks)cse-lib/data/src/main/java/com/farao_community/farao/cse/data/ttc_res/TtcResult.java(1 hunks)cse-lib/data/src/test/java/com/farao_community/farao/cse/data/CseReferenceExchangesTest.java(2 hunks)cse-lib/data/src/test/java/com/farao_community/farao/cse/data/cnec/CracResultsHelperTest.java(2 hunks)cse-lib/data/src/test/java/com/farao_community/farao/cse/data/ntc2/Ntc2Test.java(1 hunks)cse-lib/data/src/test/java/com/farao_community/farao/cse/data/target_ch/TargetCHDocumentAdaptedTest.java(1 hunks)cse-lib/network-processing/src/main/java/com/farao_community/farao/cse/network_processing/CracCreationParametersService.java(0 hunks)cse-lib/network-processing/src/main/java/com/farao_community/farao/cse/network_processing/busbar_change/BusBarChangePreProcessor.java(3 hunks)cse-lib/network-processing/src/main/java/com/farao_community/farao/cse/network_processing/busbar_change/NetworkModifier.java(3 hunks)cse-lib/network-processing/src/main/java/com/farao_community/farao/cse/network_processing/pisa_change/PiSaLinkProcessor.java(4 hunks)cse-lib/network-processing/src/main/java/com/farao_community/farao/cse/network_processing/ucte_pst_change/UctePstProcessor.java(4 hunks)cse-lib/network-processing/src/test/java/com/farao_community/farao/cse/network_processing/TestUtils.java(1 hunks)
💤 Files with no reviewable changes (3)
- cse-cc-import-runner-app/src/main/java/com/farao_community/farao/cse/import_runner/app/services/UrlValidationService.java
- cse-lib/network-processing/src/main/java/com/farao_community/farao/cse/network_processing/CracCreationParametersService.java
- cse-cc-import-runner-app/src/test/java/com/farao_community/farao/cse/import_runner/app/UrlValidationServiceTest.java
✅ Files skipped from review due to trivial changes (3)
- cse-lib/network-processing/src/main/java/com/farao_community/farao/cse/network_processing/pisa_change/PiSaLinkProcessor.java
- cse-lib/data/src/main/java/com/farao_community/farao/cse/data/ttc_res/TtcResult.java
- cse-cc-import-runner-app/src/main/java/com/farao_community/farao/cse/import_runner/app/dichotomy/MultipleDichotomyRunner.java
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (35)
cse-lib/data/src/test/java/com/farao_community/farao/cse/data/cnec/CracResultsHelperTest.java (1)
73-73: LGTM! Good use of Java generics.The change from
List<BranchCnecCreationContext>toList<? extends BranchCnecCreationContext>improves type safety and flexibility by allowing the list to contain any subclass ofBranchCnecCreationContext. This aligns well with the method signature update inCracResultsHelper.cse-lib/data/src/test/java/com/farao_community/farao/cse/data/target_ch/TargetCHDocumentAdaptedTest.java (1)
19-19: LGTM! Good improvement in class visibility.Reducing the visibility from
publicto package-private aligns with the principle of least privilege and Sonar's best practices for test classes. Since test classes don't need to be accessed from outside their package, this change helps in reducing the public API surface without affecting test execution.cse-cc-import-runner-app/src/main/java/com/farao_community/farao/cse/import_runner/app/services/MerchantLineService.java (1)
43-46: Clean refactoring using modern Java switch expression!The switch expression provides a more concise and readable way to handle different process types. The use of arrow syntax (
->) and exhaustive case handling aligns well with modern Java practices.cse-lib/data/src/test/java/com/farao_community/farao/cse/data/ntc2/Ntc2Test.java (2)
16-18: LGTM! Clean import statements.Good cleanup of unused logging imports and proper organization of test assertions.
29-29: LGTM! Removed unnecessary exception declarations.Good cleanup of the
setUpmethod signature by removingthrows IOException, URISyntaxExceptionas these exceptions cannot be thrown by the method implementation.cse-cc-import-runner-app/src/main/java/com/farao_community/farao/cse/import_runner/app/util/Ntc2Util.java (2)
9-9: Formatting change looks good.
68-68: Great use of modern Java features!The change from
collect(Collectors.toList())totoList()simplifies the code while maintaining the same functionality. This is a good practice introduced in Java 16.cse-cc-import-runner-app/src/test/java/com/farao_community/farao/cse/import_runner/app/dichotomy/ZonalScalableProviderTest.java (1)
Line range hint
37-46: LGTM! Exception handling improvement.The removal of
throws IOExceptiondeclarations from test methods aligns with the implementation changes inZonalScalableProvider, where exceptions are now handled internally. This improves the code by localizing exception handling and reducing exception propagation.Also applies to: 49-58
cse-cc-import-runner-app/src/main/java/com/farao_community/farao/cse/import_runner/app/dichotomy/ZonalScalableProvider.java (1)
Line range hint
36-41: LGTM! Improved exception handling encapsulation.The removal of
throws IOExceptionfrom the method signature is a good improvement as it:
- Encapsulates exception handling within the appropriate layer
- Simplifies the API by not exposing implementation details
- Aligns with the single responsibility principle
cse-cc-import-runner-app/src/main/java/com/farao_community/farao/cse/import_runner/app/services/InitialShiftService.java (2)
Line range hint
46-71: LGTM! Exception handling improvement.The removal of
throws IOExceptionfrom the method signature aligns with the broader refactoring to improve exception handling across the codebase.
104-109: Good improvement in logging practices.Using intermediate variables for formatted messages improves code readability and maintainability. It also helps avoid potential formatting issues and makes debugging easier.
cse-cc-import-runner-app/src/test/java/com/farao_community/farao/cse/import_runner/app/dichotomy/MultipleDichotomyRunnerTest.java (1)
Line range hint
140-149: LGTM! Consistent exception handling improvement.The removal of
throws IOExceptionfrom the mock method aligns with the broader changes in exception handling across the codebase, maintaining consistency while simplifying the code.cse-lib/network-processing/src/main/java/com/farao_community/farao/cse/network_processing/busbar_change/NetworkModifier.java (3)
10-26: LGTM! Well-organized importsThe new imports are properly organized and necessary for the enhanced type-safe branch handling implementation.
32-37: LGTM! Standard collection importsThe utility imports are well-organized and necessary for the collection operations in the class.
408-408: Well-documented Sonar suppressionThe NOSONAR comment is properly justified with a clear explanation of why the warning should be suppressed due to API limitations in
network.getBranchStream().cse-lib/data/src/main/java/com/farao_community/farao/cse/data/ttc_rao/TtcRao.java (3)
159-163: Great improvements to type safety and collection handling!The changes enhance the code in two ways:
- Using
List<? extends ElementaryCreationContext>improves type safety and flexibility- Using
toList()is more concise and returns an unmodifiable list
185-189: Consistent application of improvements!The same enhancements have been correctly applied here, maintaining consistency throughout the codebase.
238-238: Consistent type safety improvementThe method signature has been correctly updated to maintain consistency with the type safety improvements.
cse-cc-export-runner-app/src/main/java/com/farao_community/farao/cse/export_runner/app/FileUtil.java (2)
31-35: LGTM! Improved URL handling using URI.The change to use
URIfor URL validation aligns with the changes in the import runner app, providing consistent URL handling across the codebase.
53-65: LGTM! Good use of switch expressions.The switch expression provides a more concise and maintainable way to handle different process types. The error messages are clear and consistent.
cse-cc-import-runner-app/src/test/java/com/farao_community/farao/cse/import_runner/app/services/FileImporterTest.java (1)
Line range hint
165-178: LGTM! Good use of parameterized tests.The change to use parameterized tests improves test maintainability by:
- Consolidating similar test cases
- Making test data more readable through
@CsvSource- Reducing code duplication
cse-cc-import-runner-app/src/main/java/com/farao_community/farao/cse/import_runner/app/services/FileImporter.java (2)
181-183: LGTM! Improved URL handling using URI.The change to use
URIfor URL validation is consistent with the changes in FileUtil, providing robust URL handling across the codebase.
191-193: LGTM! Consistent URL handling in getFileNameFromUrl.The method now uses the same URI-based URL handling approach, maintaining consistency with
openUrlStream.cse-lib/data/src/main/java/com/farao_community/farao/cse/data/cnec/CracResultsHelper.java (4)
9-14: LGTM! Import statements are properly organized.The new imports are necessary for the error handling improvements.
130-134: Good improvement in method signature and stream collection!
- The wildcard extends type
List<? extends BranchCnecCreationContext>makes the method more flexible.- Using
.toList()instead of.collect(Collectors.toList())improves readability.
322-342: Good error handling refactoring!
- Extracted common error handling logic into
getCountryFromSubstation.- Error messages are properly defined as final strings.
- The code is now more maintainable and follows DRY principles.
385-386: Good use of modern Java features!Using
.toList()instead of.collect(Collectors.toList())improves code readability.cse-cc-export-runner-app/src/main/java/com/farao_community/farao/cse/export_runner/app/services/FileImporter.java (1)
72-74: Improved URL handling with URI validationThe change to use
URI.toURL()instead of direct URL creation is a good security practice that helps prevent malformed URLs and improves input validation.cse-lib/data/src/test/java/com/farao_community/farao/cse/data/CseReferenceExchangesTest.java (1)
32-44: Well-implemented resource management with try-with-resourcesGood improvement in resource handling using try-with-resources pattern, ensuring proper cleanup of the input stream.
cse-lib/computation/src/main/java/com/farao_community/farao/cse/computation/BorderExchanges.java (1)
Line range hint
93-97: Consider handling non-converged load flow cases explicitlyThe change from
!result.isOk()toresult.isFailed()might miss handling non-converged load flow cases. In power systems, a non-converged result, while not explicitly failed, could still indicate numerical issues that might affect the accuracy of exchange calculations.Consider explicitly handling all possible states:
- if (result.isFailed()) { + if (!result.isOk()) { + String status = result.isFailed() ? "failed" : "did not converge"; LOGGER.error("Loadflow computation diverged on network '{}'", network.getId()); - throw new CseComputationException(String.format("Loadflow computation diverged on network %s", network.getId())); + throw new CseComputationException(String.format("Loadflow computation %s on network %s", status, network.getId())); }Let's verify the LoadFlow API usage across the codebase:
cse-lib/network-processing/src/test/java/com/farao_community/farao/cse/network_processing/TestUtils.java (2)
Line range hint
1-58: Well-structured test utility class!The implementation follows good practices:
- Proper resource handling with try-with-resources
- Clear separation of concerns
- Effective error handling
- Good use of utility methods
49-49: LGTM! Modern Java stream collection.The change to use
toList()instead ofcollect(Collectors.toList())makes the code more concise while maintaining the same functionality.Let's verify the project's Java version compatibility since
toList()requires Java 16+:Also applies to: 51-51
✅ Verification successful
Verified: Java 16+ compatibility confirmed
The project's dependency ecosystem (Jakarta EE 9+, JAXB 4.x) indicates modern Java compatibility. The use of
toList()(Java 16+) is appropriate for this codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check Java version requirements # Look for Maven configuration files and Gradle build files # Check Maven configuration fd pom.xml --exec grep -l "<java.version\|<maven.compiler\|<source>\|<target>" {} \; # Check Gradle configuration fd "build.gradle" --exec grep -l "sourceCompatibility\|targetCompatibility\|JavaVersion" {} \;Length of output: 187
Script:
#!/bin/bash # Try different approaches to find Java version configuration # Search for Java version and compiler settings with broader patterns rg -i "java.*(version|source|target|release|compatibility)" -g "pom.xml" -g "*.gradle" # Look for parent pom references rg "<parent>" -A 5 -g "pom.xml"Length of output: 3284
Script:
#!/bin/bash # Look for Spring Boot version and other version indicators # Search for Spring Boot version rg -i "spring-boot.*version|spring.boot.*version" -g "pom.xml" # Search for any other version properties rg "<properties>" -A 10 -g "pom.xml" # Look for dependency management section rg "<dependencyManagement>" -A 10 -g "pom.xml"Length of output: 1428
cse-lib/network-processing/src/main/java/com/farao_community/farao/cse/network_processing/busbar_change/BusBarChangePreProcessor.java (3)
12-14: LGTM! Import statements are well-organized.The new imports are properly organized and all are actively used in the code.
Also applies to: 21-22, 28-35
164-164: LGTM! Good use of toList() for immutability.The change to
toList()not only simplifies the code but also provides an unmodifiable list, which is safer as it prevents accidental modifications.
76-76: Verify Java version compatibility for List.getFirst()The change to
getFirst()improves readability, but this method was introduced in Java 21. Please ensure your project's Java version is compatible.Run this script to check the project's Java version:
Please check if the PR fulfills these requirements (please use
'[x]'to check the checkboxes, or submit the PR and then click the checkboxes)Does this PR already have an issue describing the problem ? If so, link to this issue using
'#XXX'and skip the restWhat kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
What is the current behavior? (You can also link to an open issue here)
What is the new behavior (if this is a feature change)?
Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
Other information:
(if any of the questions/checkboxes don't apply, please delete them entirely)
Summary by CodeRabbit
Based on the comprehensive summary of changes across multiple files, here are the release notes:
New Features
Bug Fixes
Refactoring
if-elsestatements with more conciseswitchexpressionstoList()Code Quality
Test Improvements
These changes primarily focus on improving code quality, error handling, and maintainability across the CSE (Core-South Europe) network processing and import/export runner applications.