Update package versions and add CLI option validations#4151
Conversation
Updated project files to use Ginger.ExecuterService.Contracts v25.2.1. Added System.IO namespace and temp folder logic in CLIDynamicFile.cs. Introduced HandleDynamicOptions method in CLIProcessor.cs. Refactored HandleFileOptions method in CLIProcessor.cs. Implemented IOptionValidation in DoOptions and DynamicOptions. Added UseTempFolder and Url options with validation logic. Introduced IOptionValidation interface for CLI options validation.
WalkthroughThe changes include upgrading the version of the Changes
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🪧 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: 6
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (10)
Ginger/Ginger/Ginger.csproj(1 hunks)Ginger/GingerCoreNET/GingerCoreNET.csproj(1 hunks)Ginger/GingerCoreNET/RunLib/CLILib/CLIDynamicFile.cs(2 hunks)Ginger/GingerCoreNET/RunLib/CLILib/CLIProcessor.cs(6 hunks)Ginger/GingerCoreNET/RunLib/CLILib/DoOptions.cs(4 hunks)Ginger/GingerCoreNET/RunLib/CLILib/DoOptionsHanlder.cs(2 hunks)Ginger/GingerCoreNET/RunLib/CLILib/DynamicOptions.cs(1 hunks)Ginger/GingerCoreNET/RunLib/CLILib/Interfaces/IOptionValidation.cs(1 hunks)Ginger/GingerCoreNETUnitTest/GingerCoreNETUnitTest.csproj(1 hunks)Ginger/GingerRuntime/GingerRuntime.csproj(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
Ginger/GingerRuntime/GingerRuntime.csproj (1)
Learnt from: sudarshan0212
PR: Ginger-Automation/Ginger#4024
File: Ginger/GingerRuntime/GingerRuntime.csproj:43-43
Timestamp: 2025-03-25T06:06:50.023Z
Learning: Version 24.6.1 of Ginger.ExecuterService.Contracts is unlisted and should not be suggested for updates.
Ginger/Ginger/Ginger.csproj (1)
Learnt from: sudarshan0212
PR: Ginger-Automation/Ginger#4024
File: Ginger/GingerRuntime/GingerRuntime.csproj:43-43
Timestamp: 2025-03-25T06:06:50.023Z
Learning: Version 24.6.1 of Ginger.ExecuterService.Contracts is unlisted and should not be suggested for updates.
🧬 Code Definitions (3)
Ginger/GingerCoreNET/RunLib/CLILib/DoOptions.cs (3)
Ginger/Ginger/App.xaml.cs (1)
DoOptions(349-356)Ginger/GingerCoreNET/RunLib/CLILib/DynamicOptions.cs (1)
Validate(39-49)Ginger/GingerCoreNET/RunLib/CLILib/Interfaces/IOptionValidation.cs (1)
Validate(12-12)
Ginger/GingerCoreNET/RunLib/CLILib/DynamicOptions.cs (2)
Ginger/GingerCoreNET/RunLib/CLILib/DoOptions.cs (2)
Verb(28-107)Validate(97-106)Ginger/GingerCoreNET/RunLib/CLILib/Interfaces/IOptionValidation.cs (1)
Validate(12-12)
Ginger/GingerCoreNET/RunLib/CLILib/CLIProcessor.cs (3)
Ginger/GingerCoreNET/RunLib/CLILib/DoOptions.cs (1)
Validate(97-106)Ginger/GingerCoreNET/RunLib/CLILib/DynamicOptions.cs (1)
Validate(39-49)Ginger/GingerCoreNET/RunLib/CLILib/Interfaces/IOptionValidation.cs (1)
Validate(12-12)
🔇 Additional comments (21)
Ginger/GingerRuntime/GingerRuntime.csproj (1)
44-44: Update PackageReference for Ginger.ExecuterService.Contracts
The package version has been updated to 25.2.1 in this project file, which aligns with the overall dependency upgrade objective of the PR. Please verify that all features and integrations relying on this package are compatible with the new version.Ginger/Ginger/Ginger.csproj (1)
724-724: Consistent PackageReference Upgrade Across Projects
The PackageReference for Ginger.ExecuterService.Contracts has been updated to version 25.2.1 here as well. This change is consistent with similar updates in other project files, ensuring uniform dependency management. Confirm that no breaking changes arise from this upgrade by running the appropriate integration tests.Ginger/GingerCoreNETUnitTest/GingerCoreNETUnitTest.csproj (1)
92-92: Updated PackageReference to Ginger.ExecuterService.Contracts (v25.2.1)The package reference has been updated to version 25.2.1 in this unit test project. This change is consistent with the overall package upgrade mandate. Please ensure that the new version is fully compatible with the existing unit tests and that there are no unexpected breaking changes.
Ginger/GingerCoreNET/GingerCoreNET.csproj (1)
262-262: Updated PackageReference to Ginger.ExecuterService.Contracts (v25.2.1)The package reference in this project has been updated to version 25.2.1 in line with the broader upgrade across the solution. Verify that all dependent modules and compilation targets remain compatible with the new version.
Ginger/GingerCoreNET/RunLib/CLILib/Interfaces/IOptionValidation.cs (1)
1-14: Well-structured interface for CLI option validation.This new interface is properly designed with a single responsibility (validation) and is well-documented with appropriate XML comments. The interface correctly defines a validation method that returns a boolean to indicate success or failure.
Ginger/GingerCoreNET/RunLib/CLILib/CLIDynamicFile.cs (1)
32-32: Correctly added System.IO namespace.Adding the System.IO namespace is appropriate for the new functionality that uses Path methods.
Ginger/GingerCoreNET/RunLib/CLILib/DoOptionsHanlder.cs (1)
47-50: Properly handling the UseTempFolder flag.The code correctly sets the solution path to a temporary folder when the UseTempFolder flag is true.
Ginger/GingerCoreNET/RunLib/CLILib/DoOptions.cs (5)
19-20: Correctly added required namespaces.The added namespaces properly support the new functionality involving the IOptionValidation interface and logging.
29-29: Good implementation of IOptionValidation interface.The class now correctly implements the IOptionValidation interface, allowing for validation of CLI options.
44-44: Appropriately grouped option for mutual exclusivity.Adding the "solution path" group to the Solution option properly indicates that it's mutually exclusive with the UseTempFolder option.
91-92: Well-defined UseTempFolder option.The UseTempFolder option is correctly defined with the same "solution path" group as the Solution option, indicating mutual exclusivity. The help text clearly explains its purpose.
97-106: Well-implemented validation for mutually exclusive options.The Validate method correctly enforces that Solution and UseTempFolder cannot be used simultaneously, providing a clear error message.
Ginger/GingerCoreNET/RunLib/CLILib/DynamicOptions.cs (4)
19-20: No issues with the new using statements.
They correctly reference the relevant namespaces needed for the extended functionalities.
22-22: Importing 'System' namespace.
No functional issues here; it aligns with the usage ofUriand other system-level classes.
28-29: Verb name and interface implementation look consistent.
Updating the help text to "Run dynamic xml/json" clarifies supported file types, and implementingIOptionValidationis aligned with the project's CLI validation approach.
36-37: New property 'Url' introduced.
Defining it as aUriis a good choice for type safety.Ginger/GingerCoreNET/RunLib/CLILib/CLIProcessor.cs (5)
31-32: Added references to System.Net and System.Net.Http.
This is necessary for making HTTP requests in the new URL-handling logic.
98-98: Routing 'DynamicOptions' to the dedicated handler.
Usingawait HandleDynamicOptions(opts)ensures asynchronous processing of dynamic files or URLs.
114-121: Validate 'DoOptions' before execution.
The newly introduced validation call (opts.Validate()) helps ensure that mutually exclusive fields are not both set. The error handling logic is consistent with other CLI paths.
313-354: New 'HandleUrlOptions' method for dynamic execution via URL.
This is a well-structured workflow:
- Sets the CLI mode to dynamic.
- Uses an
HttpClientHandlerwith system proxy by default.- Logs success/failure appropriately.
403-420: Centralized 'ProcessExecConfigsAndExecute' method.
Loading configurations and proceeding toCLILoadAndPreparefollowed byExecuteRunSetstreamlines the code for dynamic or config-based runs. The error handling approach (settingEnvironment.ExitCode) is consistent with the rest of the CLI code.
…per method in CLIHelper t
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
Ginger/GingerCoreNET/RunLib/CLILib/CLIDynamicFile.cs(2 hunks)Ginger/GingerCoreNET/RunLib/CLILib/CLIHelper.cs(2 hunks)Ginger/GingerCoreNET/RunLib/CLILib/DoOptionsHanlder.cs(2 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
Ginger/GingerCoreNET/RunLib/CLILib/CLIDynamicFile.cs (1)
Ginger/GingerCoreNET/RunLib/CLILib/CLIHelper.cs (1)
GetTempFolderPathForRepo(1029-1034)
Ginger/GingerCoreNET/RunLib/CLILib/DoOptionsHanlder.cs (1)
Ginger/GingerCoreNET/RunLib/CLILib/CLIHelper.cs (1)
GetTempFolderPathForRepo(1029-1034)
🔇 Additional comments (4)
Ginger/GingerCoreNET/RunLib/CLILib/CLIDynamicFile.cs (1)
127-130: Support for temporary folder logic looks good.The conditional check for
UseTempFolderis implemented correctly, using the helper method to get the temporary folder path.Ginger/GingerCoreNET/RunLib/CLILib/DoOptionsHanlder.cs (2)
47-50: Temporary folder handling logic looks good.The implementation correctly checks the
UseTempFolderflag and sets the solution path accordingly.
409-412: Good reuse of helper method.The method correctly leverages the centralized
GetTempFolderPathForRepoimplementation fromCLIHelper.Ginger/GingerCoreNET/RunLib/CLILib/CLIHelper.cs (1)
39-39: LGTM! Addition of System.IO namespace.This import is necessary for the new method that uses Path functions.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
Ginger/GingerCoreNET/RunLib/CLILib/CLIHelper.cs (1)
1020-1043: 🛠️ Refactor suggestionURL parsing improved but directory existence check still missing.
The method constructs a temporary folder path by extracting the repository name from the source control URL. While you've improved the URL parsing by trimming trailing slashes and using Path.GetFileNameWithoutExtension, the method still doesn't ensure the directory exists.
public string GetTempFolderPathForRepo(string sourceControlUrl = null) { string urlToUse = sourceControlUrl ?? SourceControlURL; string repoName = string.Empty; if (!string.IsNullOrEmpty(urlToUse)) { // Remove trailing slash if present urlToUse = urlToUse.TrimEnd('/'); repoName = Path.GetFileNameWithoutExtension(urlToUse.Split('/').Last()); } - return Path.Combine(Path.GetTempPath(), repoName); + string tempPath = Path.Combine(Path.GetTempPath(), repoName); + + // Ensure directory exists + if (!Directory.Exists(tempPath)) + { + Directory.CreateDirectory(tempPath); + } + + return tempPath; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
Ginger/GingerCoreNET/RunLib/CLILib/CLIHelper.cs(2 hunks)
🔇 Additional comments (1)
Ginger/GingerCoreNET/RunLib/CLILib/CLIHelper.cs (1)
39-39: LGTM! System.IO namespace is correctly imported.The System.IO namespace is properly added to support the new file and directory operations in the GetTempFolderPathForRepo method.
Renamed `UseTempFolder` to `UseTempSolutionFolder` in `DoOptions.cs` and `DoOptionsHanlder.cs`, affecting the command-line option attribute, property name, and validation logic. Renamed `executionConfigSourceUrl` to `execConfigSourceUrl` in `DoOptions.cs` and `DynamicOptions.cs`, affecting the command-line option attribute and validation logic. Updated validation error messages to reflect the new property names.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
Ginger/GingerCoreNET/RunLib/CLILib/DoOptions.cs (1)
94-95: 🧹 Nitpick (assertive)Introduce optional
execConfigSourceUrl
DefiningExecutionConfigurationSourceUrlas aUriis helpful for clarity. Consider adding a small validation for absolute URIs in theValidate()method.A possible adjustment:
public bool Validate() { if (UseTempSolutionFolder && !string.IsNullOrEmpty(Solution)) { Reporter.ToLog(eLogLevel.ERROR, $"'solution' & 'useTempSolutionFolder' are mutually exclusive."); return false; } + if (ExecutionConfigurationSourceUrl is not null && !ExecutionConfigurationSourceUrl.IsAbsoluteUri) + { + Reporter.ToLog(eLogLevel.ERROR, "Invalid URL scheme or format in 'execConfigSourceUrl'"); + return false; + } return true; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
Ginger/GingerCoreNET/RunLib/CLILib/DoOptions.cs(4 hunks)Ginger/GingerCoreNET/RunLib/CLILib/DoOptionsHanlder.cs(2 hunks)Ginger/GingerCoreNET/RunLib/CLILib/DynamicOptions.cs(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
Ginger/GingerCoreNET/RunLib/CLILib/DynamicOptions.cs (2)
Learnt from: amitamir
PR: Ginger-Automation/Ginger#4151
File: Ginger/GingerCoreNET/RunLib/CLILib/CLIProcessor.cs:292-0
Timestamp: 2025-03-30T14:00:25.054Z
Learning: The CommandLineParser library automatically handles validation to ensure required parameters are provided when options are in the same group. Custom Validate() methods (from IOptionValidation interface) are used for more complex validations like ensuring options are mutually exclusive.
Learnt from: amitamir
PR: Ginger-Automation/Ginger#4151
File: Ginger/GingerCoreNET/RunLib/CLILib/DynamicOptions.cs:39-0
Timestamp: 2025-03-30T14:01:53.677Z
Learning: In the CommandLineParser library, options in the same group (using the Group attribute) ensure that at least one option from that group must be provided. For example, in DynamicOptions class, both 'FileName' and 'Url' are in the "source" group, ensuring at least one must be specified. Custom Validate() methods are then used to enforce additional constraints like mutual exclusivity.
🧬 Code Definitions (2)
Ginger/GingerCoreNET/RunLib/CLILib/DoOptionsHanlder.cs (1)
Ginger/GingerCoreNET/RunLib/CLILib/CLIHelper.cs (1)
GetTempFolderPathForRepo(1029-1043)
Ginger/GingerCoreNET/RunLib/CLILib/DoOptions.cs (3)
Ginger/Ginger/App.xaml.cs (1)
DoOptions(349-356)Ginger/GingerCoreNET/RunLib/CLILib/DynamicOptions.cs (1)
Validate(39-49)Ginger/GingerCoreNET/RunLib/CLILib/Interfaces/IOptionValidation.cs (1)
Validate(12-12)
🔇 Additional comments (10)
Ginger/GingerCoreNET/RunLib/CLILib/DoOptionsHanlder.cs (2)
47-50: Conditional logic for setting solution path
The new check forUseTempSolutionFolderis appropriate. It cleanly prevents accidental overlap with a user-specified folder by overwritingmOpts.Solutionunder a single condition.
409-412: Centralize all path computations inCLIHelper
Returning the path from the helper is consistent with the recommended approach from previous code reviews. The logic appears correct and avoids duplication.Ginger/GingerCoreNET/RunLib/CLILib/DoOptions.cs (4)
19-20: Implementation of IOptionValidation
Importing the interface and updating the class signature to implementIOptionValidationis a good step toward organizing validations.Also applies to: 29-29
44-44: Grouping'solution'with'solution path'
This attribute clarifies thatsolutionbelongs to the same group asuseTempSolutionFolder. Good usage of the CommandLine grouping feature.
91-92: New propertyUseTempSolutionFolder
This property nicely integrates into the solution path group for toggling a temp folder destination in the CLI.
97-106:Validate()method
The mutual exclusivity check forUseTempSolutionFolderandSolutionis accurate and prevents conflicting CLI arguments.Ginger/GingerCoreNET/RunLib/CLILib/DynamicOptions.cs (4)
19-20: Extended references
Added references toAmdocs.Ginger.Common,Amdocs.Ginger.CoreNET.RunLib.CLILib.Interfaces, andSystem. These imports look correct for URL handling and interface usage.Also applies to: 22-22
28-29: Class now supportsIOptionValidation
Declaring[Verb("dynamic", HelpText = "Run dynamic xml/json")]clarifies command usage. ImplementingIOptionValidationkeeps validation consistent with the rest of the CLI code.
33-33:FileNameoption
Allowing dynamic file input (XML/JSON) is flexible. No issues found.
39-49:Validate()method
The mutual exclusivity check betweenFileNameandUrlis correct, preventing conflicting inputs.
2 participants
Updated project files to use Ginger.ExecuterService.Contracts v25.2.1. Added System.IO namespace and temp folder logic in CLIDynamicFile.cs. Introduced HandleDynamicOptions method in CLIProcessor.cs. Refactored HandleFileOptions method in CLIProcessor.cs. Implemented IOptionValidation in DoOptions and DynamicOptions. Added UseTempFolder and Url options with validation logic. Introduced IOptionValidation interface for CLI options validation.
Thank you for your contribution.
Before submitting this PR, please make sure:
Summary by CodeRabbit
New Features
Chores