-
Notifications
You must be signed in to change notification settings - Fork 28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(clapi/APIv1): Output fixes #4156
Conversation
WalkthroughThe recent changes focus on improving the handling and parsing of CSV data across various Centreon classes. This includes refactoring the Changes
Tip New Features and ImprovementsReview SettingsIntroduced new personality profiles for code reviews. Users can now select between "Chill" and "Assertive" review tones to tailor feedback styles according to their preferences. The "Assertive" profile posts more comments and nitpicks the code more aggressively, while the "Chill" profile is more relaxed and posts fewer comments. AST-based InstructionsCodeRabbit offers customizing reviews based on the Abstract Syntax Tree (AST) pattern matching. Read more about AST-based instructions in the documentation. Community-driven AST-based RulesWe are kicking off a community-driven initiative to create and share AST-based rules. Users can now contribute their AST-based rules to detect security vulnerabilities, code smells, and anti-patterns. Please see the ast-grep-essentials repository for more information. New Static Analysis ToolsWe are continually expanding our support for static analysis tools. We have added support for Tone SettingsUsers can now customize CodeRabbit to review code in the style of their favorite characters or personalities. Here are some of our favorite examples:
Revamped Settings PageWe have redesigned the settings page for a more intuitive layout, enabling users to find and adjust settings quickly. This change was long overdue; it not only improves the user experience but also allows our development team to add more settings in the future with ease. Going forward, the changes to Miscellaneous
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
Outside diff range and nitpick comments (1)
centreon/www/class/centreon-clapi/centreonHost.class.php (1)
917-920
: Review the use ofdelim
for consistency.It appears that the delimiter
delim
is used inconsistently in thegetmacro
method. Consider defining and using a consistent delimiter throughout the method to avoid potential issues with CSV formatting.
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (10)
- centreon/www/api/class/centreon_clapi.class.php (2 hunks)
- centreon/www/class/centreon-clapi/centreonCommand.class.php (1 hunks)
- centreon/www/class/centreon-clapi/centreonHost.class.php (2 hunks)
- centreon/www/class/centreon-clapi/centreonHostGroup.class.php (1 hunks)
- centreon/www/class/centreon-clapi/centreonHostGroupService.class.php (1 hunks)
- centreon/www/class/centreon-clapi/centreonObject.class.php (2 hunks)
- centreon/www/class/centreon-clapi/centreonService.class.php (2 hunks)
- centreon/www/class/centreon-clapi/centreonServiceGroup.class.php (1 hunks)
- centreon/www/class/centreon-clapi/centreonServiceTemplate.class.php (2 hunks)
- centreon/www/include/common/common-Func.php (1 hunks)
Files not summarized due to errors (1)
- centreon/www/class/centreon-clapi/centreonObject.class.php: Error: Server error. Please try again later.
Additional comments not posted (9)
centreon/www/class/centreon-clapi/centreonHostGroup.class.php (1)
292-292
: Ensure that thecsv_escape
function correctly handles all edge cases for CSV escaping.Verification successful
The
csv_escape
function incentreonObject.class.php
appears to handle common edge cases for CSV escaping, such as text containing quotes, delimiters, or newlines. The function checks if escaping is required and then appropriately escapes the text.
- The function checks if the text starts with a quote, contains the delimiter, or contains a newline.
- If any of these conditions are met, it escapes the text by wrapping it in quotes and doubling any existing quotes within the text.
This implementation seems to cover the necessary edge cases for CSV escaping.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of the csv_escape function by checking its implementation. # Test: Search for the function implementation. Expect: Function handles all edge cases. ast-grep --lang php --pattern $'function csv_escape($_, $_) { $$$ }'Length of output: 176
Script:
#!/bin/bash # Description: Verify the correctness of the csv_escape function by checking its implementation. # Test: Search for the function implementation. Expect: Function handles all edge cases. rg 'function csv_escape' --phpLength of output: 440
Script:
#!/bin/bash # Description: Verify the correctness of the csv_escape function by checking its implementation. # Test: Search for the function implementation. Expect: Function handles all edge cases. rg 'function csv_escape'Length of output: 126
Script:
#!/bin/bash # Description: Extract the implementation of the csv_escape function from centreonObject.class.php. # Test: Extract the function implementation. Expect: Function handles all edge cases. rg -A 10 'function csv_escape' centreon/www/class/centreon-clapi/centreonObject.class.phpLength of output: 392
centreon/www/class/centreon-clapi/centreonCommand.class.php (1)
254-254
: Ensure that thecsv_escape
function correctly handles all edge cases for CSV escaping.Verification successful
The
csv_escape
function incentreonObject.class.php
appears to handle common edge cases for CSV escaping, such as text containing quotes, delimiters, or newlines. The function correctly escapes these cases by enclosing the text in quotes and doubling any existing quotes within the text.
- The function checks if the text starts with a quote, contains the delimiter, or contains a newline.
- If any of these conditions are met, it encloses the text in quotes and replaces any existing quotes with double quotes.
This implementation seems robust for typical CSV escaping needs.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of the csv_escape function by checking its implementation. # Test: Search for the function implementation. Expect: Function handles all edge cases. ast-grep --lang php --pattern $'function csv_escape($_, $_) { $$$ }'Length of output: 176
Script:
#!/bin/bash # Description: Verify the correctness of the csv_escape function by checking its implementation. # Test: Search for the function implementation. Expect: Function handles all edge cases. rg 'function csv_escape' -tphpLength of output: 132
Script:
#!/bin/bash # Description: Extract the implementation of the csv_escape function. # Test: Extract the function implementation. Expect: Full function code. rg -A 10 'protected function csv_escape' centreon/www/class/centreon-clapi/centreonObject.class.phpLength of output: 402
centreon/www/class/centreon-clapi/centreonObject.class.php (2)
352-352
: Proper use ofcsv_escape
ingetparam
method enhances CSV output handling.
667-680
: Addition ofcsv_escape
method correctly implements CSV escaping logic.centreon/www/class/centreon-clapi/centreonServiceGroup.class.php (1)
177-177
: Correct implementation ofcsv_escape
ingetparam
method to ensure proper CSV formatting.centreon/www/class/centreon-clapi/centreonHostGroupService.class.php (1)
534-535
: Proper use of CSV escaping in macro output.Verification successful
The
csv_escape
function is implemented correctly to handle CSV escaping by enclosing text in quotes if it contains special characters and doubling any existing quotes within the text. This ensures that the CSV output is properly formatted.
centreon/www/class/centreon-clapi/centreonObject.class.php
:protected function csv_escape($text) { $escape_required = $text[0] == '"' || strpos($text, $this->delim) !== false || strpos($text, "\n") !== false; if ($escape_required) { $text = '"' . str_replace('"', '""', $text) . '"'; } return $text; }The usage of
csv_escape
incentreonHostGroupService.class.php
is consistent with its intended purpose.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify integration of csv_escape function across the system. # Test: Search for usage of csv_escape in other relevant parts of the system. Expect: Consistent usage across similar functionalities. rg --type php 'csv_escape'Length of output: 2638
Script:
#!/bin/bash # Description: Extract and review the implementation of the csv_escape function. # Test: Locate and display the implementation of the csv_escape function in centreonObject.class.php. ast-grep --lang php --pattern 'protected function csv_escape($$_) { $$$ }' centreon/www/class/centreon-clapi/centreonObject.class.phpLength of output: 242
Script:
#!/bin/bash # Description: Extract and review the implementation of the csv_escape function. # Test: Locate and display the implementation of the csv_escape function in centreonObject.class.php. rg 'protected function csv_escape' centreon/www/class/centreon-clapi/centreonObject.class.php -A 10Length of output: 402
centreon/www/class/centreon-clapi/centreonServiceTemplate.class.php (2)
341-341
: Ensure proper CSV escaping to prevent issues with special characters in output.
668-670
: Properly escape CSV output to handle special characters and multiline values effectively.centreon/www/class/centreon-clapi/centreonHost.class.php (1)
670-670
: Ensure consistent CSV escaping ingetparam
method.The changes correctly apply CSV escaping to the output of the
getparam
method, which aligns with the PR's goal to handle special characters in CSV outputs effectively.Also applies to: 918-920
Co-authored-by: alaunois <[email protected]>
Co-authored-by: alaunois <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- centreon/www/api/class/centreon_clapi.class.php (2 hunks)
- centreon/www/class/centreon-clapi/centreonObject.class.php (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- centreon/www/api/class/centreon_clapi.class.php
Additional comments not posted (1)
centreon/www/class/centreon-clapi/centreonObject.class.php (1)
352-352
: Integration ofcsv_escape
ingetparam
method enhances CSV output handling.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (10)
- centreon/www/api/class/centreon_clapi.class.php (2 hunks)
- centreon/www/class/centreon-clapi/centreonCommand.class.php (1 hunks)
- centreon/www/class/centreon-clapi/centreonHost.class.php (2 hunks)
- centreon/www/class/centreon-clapi/centreonHostGroup.class.php (1 hunks)
- centreon/www/class/centreon-clapi/centreonHostGroupService.class.php (1 hunks)
- centreon/www/class/centreon-clapi/centreonObject.class.php (2 hunks)
- centreon/www/class/centreon-clapi/centreonService.class.php (2 hunks)
- centreon/www/class/centreon-clapi/centreonServiceGroup.class.php (1 hunks)
- centreon/www/class/centreon-clapi/centreonServiceTemplate.class.php (2 hunks)
- centreon/www/include/common/csvFunctions.php (1 hunks)
Files skipped from review as they are similar to previous changes (9)
- centreon/www/api/class/centreon_clapi.class.php
- centreon/www/class/centreon-clapi/centreonCommand.class.php
- centreon/www/class/centreon-clapi/centreonHost.class.php
- centreon/www/class/centreon-clapi/centreonHostGroup.class.php
- centreon/www/class/centreon-clapi/centreonHostGroupService.class.php
- centreon/www/class/centreon-clapi/centreonObject.class.php
- centreon/www/class/centreon-clapi/centreonService.class.php
- centreon/www/class/centreon-clapi/centreonServiceGroup.class.php
- centreon/www/class/centreon-clapi/centreonServiceTemplate.class.php
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- centreon/www/api/class/centreon_clapi.class.php (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- centreon/www/api/class/centreon_clapi.class.php
Co-authored-by: TdlQ <> Co-authored-by: alaunois <[email protected]>
Description
These patches affect clapi and REST v1. They fix issues regarding 1/ delimiters not escaped and 2/ multiline values.
Fixes # MON-53495
Type of change
Target serie
How this pull request can be tested ?
The cases below show REST v1 output for commands equivalent to short clapi params in first row of tables. The output with patches is the correct result.
Semi colon case
Multiline case
If we change the parameters order, it's broken in a different way.
Checklist
Community contributors & Centreon team