Address comments from AI code review

[nmgaston]

Suggested PR Title - Enhance source parsers, improve utility functions, and refactor source management

_The PR review is to check for sustainability and correctness. Sustainability is actually more business critical as correctness is largely tested into the code over time. Its useful to keep in mind that SW often outlives the HW it was written for and engineers move from job to job so it is critical that code developed for Intel be supportable across many years. It is up to the submitter and reviewer to look at the code from a perspective of what if we have to debug this 3 years from now after the author is no longer available and defect databases have been lost. Yes, that happens all the time when we are working with time scales of more than 2 years. When reviewing your code it is important to look at it from this perspective._

Author Mandatory (to be filled by PR Author/Submitter)

• Developer who submits the Pull Request for merge is required to mark the checklist below as applicable for the PR changes submitted.
• Those checklist items which are not marked are considered as not applicable for the PR change.
• Items marked with an asterisk suffix are mandatory items to check and if not marked will be treated as non-compliant pull requests by the developers for Inner Source Development Model (ISDM) compliance

PULL DESCRIPTION

Address some of the AI recommendations from a previous PR.

REFERENCES

Reference URL for issue tracking (JIRA/HSD/Github): <URL to be filled>

• Added label to the Pull Request following the template: ISDM_<Complexity>*
  Note-1: Depending on complexity of code changes, use the suitable word for complexity: Low/Medium/High
  Example: PR for Slim boot loader project with medium complexity can have the label as: ISDM_Medium
• Added label to the Pull Request for easier discoverability and search
• RTC or HSD number will be included in final merge. HSD must always be included if available.
• Changelogs are updated (or N/A if not customer visible)
• inbm/log_changes.txt and inbm-vision/log_changes.txt are updated for potentially Validation-breaking log changes (or N/A if none)

CODE MAINTAINABILITY

• Commit Message meets guidelines as indicated in the URL*
  • (https://github.com/edgexfoundry/edgex-go/blob/main/.github/Contributing.md)
• Every commit is a single defect fix and does not mix feature addition or changes*
• Added required new tests relevant to the changes
  • PR contains URL links to functional tests executed with the new tests
• Updated Documentation as relevant to the changes
• Updated Build steps/commands changes as relevant
• PR change contains code related to security
• PR introduces changes that breaks compatibility with other modules (If YES, please provide description)
• Specific instructions or information for code reviewers (If any):
• Run 'go fmt' or format-python.sh as applicable.
• New/modified methods and functions should have type annotations on signatures as applicable
• New/modified methods must have appropriate doc strings (language dependent)

APPLICATION SPECIFIC

• Does PR change default config files under /etc? If so, will application still work after an upgrade that leaves /etc alone, like a Mender upgrade?
• Is cloud UI changed? If so, are cloud definition files updated?

Maintainer Mandatory (to be filled by PR Reviewer/Approving Maintainer)

• Maintainer who approves the Pull Request for merge is required to mark the checklist below as appropriate for the PR change reviewed as key proof of attestation indicating reasons for merge.
• Those checklist items which are not marked are considered as not applicable for the PR change.
• Items marked with an asterisk suffix are mandatory items to check and if not marked will be treated as non-compliant pull requests by the maintainers for ISDM compliance.

QUALITY CHECKS

• Architectural and Design Fit
• Quality of code (At least one should be checked as applicable)*
  • Commit Message meets guidelines
  • PR changes adhere to industry practices and standards
  • Error and exception code paths implemented correctly
  • Code reviewed for domain or language specific anti-patterns
  • Code is adequately commented
  • Code copyright is correct
  • Confusing logic is explained in comments
  • Commit comment can be used to design a new test case for the changes
• Test coverage shows adequate coverage with required CI functional tests pass on all supported platforms*
• Static code scan report shows zero critical issues*
• Integration tests are passing

CODE REVIEW IMPACT

• Summary of Defects Detected in Code Review: <%P1xx,P2xx,P3xx,P4xx%>
  Note P1/P2/P3/P4 denotes severity of defects found (Showstopper/High/Medium/Low) and xx denotes number of defects found

SECURITY CHECKS

Please check if your PR fulfills the following requirements:

• Follow best practices when handling primitive data types
• Configure minimal permissions when opening pipes and ports
• Check contents within input structures are valid before use
• All forms of input validated
• Avoid inter-process race conditions
• Error and exception handling implemented
• Defend against Canonical Representation Issues - Any paths utilized?
• Follow 'secure by default' - Any configuration values added
• Fail safe - Any failure scenarios?
• Clean up temporary files - Any temporary files being used?

Code must act as a teacher for future developers

${\color{red}DO \space NOT \space EDIT \space ANYTHING}$

Start of GenAI Co-Pilot generated PR Summary

Types of major changes in the pull request

enhancement, bug_fix

---

Pull request change summary

• Refactor the logic for checking if both gpgKeyUri and gpgKeyName are provided or neither of them in source_app_parser.py.
• Sanitize the source input using clean_input from inbm_common_lib.utility in source_app_parser.py and source_os_parser.py.
• Improve the canonicalize_uri function and add a new safe_extract function to avoid path traversal when extracting tarballs in utility.py.
• Refactor data classes to use List from typing and rename file_name to source_list_file_name for clarity in constants.py.
• Remove the remove_gpg_key_if_exists function and update the add_gpg_key function to handle exceptions more accurately in linux_gpg_key.py.
• Update the UbuntuApplicationSourceManager class to use the new source_list_file_name field and handle GPG key removal internally in ubuntu_source_manager.py.
• Improve the XML handler functions' documentation and fix minor issues with the return values and exception handling in xmlhandler.py.
• Update unit tests to reflect the changes in the application source parser logic, the source command handling logic, and the UbuntuApplicationSourceManager class.

---

Details of Pull Request changes by File

	Relevant files
Enhancement	7 files source_app_parser.py                                                                                inbc-program/inbc/parser/source_app_parser.py Refactor the logic for checking if both gpgKeyUri and gpgKeyName are provided or neither of them. Also, sanitize the source input using clean_input from inbm_common_lib.utility. +5/-4 source_os_parser.py                                                                                  inbc-program/inbc/parser/source_os_parser.py Sanitize the source input using clean_input from inbm_common_lib.utility. +4/-3 utility.py                                                                                                    inbm-lib/inbm_common_lib/utility.py Improve the canonicalize_uri function and add a new safe_extract function to avoid path traversal when extracting tarballs. +5/-3 constants.py                                                                                                inbm/dispatcher-agent/dispatcher/source/constants.py Refactor data classes to use List from typing and rename file_name to source_list_file_name for clarity. +7/-7 linux_gpg_key.py                                                                                        inbm/dispatcher-agent/dispatcher/source/linux_gpg_key.py Remove the remove_gpg_key_if_exists function and update the add_gpg_key function to handle exceptions more accurately. +1/-16 source_command.py                                                                                      inbm/dispatcher-agent/dispatcher/source/source_command.py Update the _handle_app_source_command function to use the new source_list_file_name field from the data classes. +4/-4 ubuntu_source_manager.py                                                                        inbm/dispatcher-agent/dispatcher/source/ubuntu_source_manager.py Update the UbuntuApplicationSourceManager class to use the new source_list_file_name field and handle GPG key removal internally. +14/-11
Tests	4 files test_source_app_parser.py                                                                      inbc-program/tests/unit/test_source_app_parser.py Update unit tests to reflect the changes in the application source parser logic and the use of clean_input for sanitization. +44/-44 test_linux_gpg_key.py                                                                              inbm/dispatcher-agent/tests/unit/source/test_linux_gpg_key.py Update unit tests to reflect the removal of the remove_gpg_key_if_exists function. +1/-22 test_source_command.py                                                                            inbm/dispatcher-agent/tests/unit/source/test_source_command.py Update unit tests to reflect the changes in the source command handling logic. +3/-3 test_ubuntu_source_cmd.py                                                                      inbm/dispatcher-agent/tests/unit/source/test_ubuntu_source_cmd.py Update unit tests to reflect the changes in the UbuntuApplicationSourceManager class and the removal of the remove_gpg_key_if_exists function. +12/-30
Documentation	1 files xmlhandler.py                                                                                              inbm/dispatcher-agent/dispatcher/xmlhandler.py Improve the XML handler functions' documentation and fix minor issues with the return values and exception handling. +6/-6

End of Co-Pilot generated PR Summary

[nmgaston]

/review

[nex-maximus]

Code Review Analysis

• 🎯 Main theme: The PR aims to address comments from a previous AI code review, focusing on sustainability and correctness of the code.
• 📝 PR summary: This PR includes changes to improve code readability, maintainability, and correctness. It refactors conditionals, list comprehensions, and string formatting, and also updates variable names for clarity. Additionally, it includes changes to exception messages and function signatures for better code practices.
• 📌 Type of PR: Enhancement
• 🏅 Score: 85
• 🧪 Relevant tests added: No
• ⏱️ Estimated effort to review [1-5]: 2, because the PR contains straightforward enhancements and refactoring with no complex logic changes.

Code Review Feedback

💡 General suggestions: Overall, the PR makes several good improvements to the codebase. It enhances readability and maintainability by using clearer variable names and more Pythonic constructs. However, there are a few additional improvements that could be made to further improve the quality of the code. It's also important to ensure that all changes are covered by unit tests to maintain code reliability.

• 🔒 Security concerns: No

✨ Usage tips:

---

Tag me in a comment '@nex-maximus' and add one of the following commands:
/review [-i]: Request a review of your Pull Request. For an incremental review, which only considers changes since the last review, include the '-i' option.
/describe: Modify the PR title and description based on the contents of the PR.
For example: /review --pr_reviewer.extra_instructions="focus on the file: ..."

[gblewis1]

/describe

[nex-maximus]

PR Description updated to latest commit (1ef60a1)

[nex-maximus]

Please provide the PR description rating here on a scale of 1 to 5 (1 being the lowest and 5 being the highest) - 4

Summary Report of Code Review Evaluation metrics for the Repository:

Code defects Acceptance ratio: 4/5 (80.0%)

Code improvements Acceptance ratio: 16/32 (50.0%)

Code fixes Acceptance ratio: 19/32 (59.38%)

14 Approvals	16 Dispprovals	7 Neutral	0 Response Pending
#461 (comment) #461 (comment) #461 (comment) #461 (comment) #461 (comment) #461 (comment) #461 (comment) #461 (comment) #461 (comment) #461 (comment) #461 (comment) #461 (comment) #461 (comment) #461 (comment)	#461 (comment) #461 (comment) #461 (comment) #461 (comment) #461 (comment) #461 (comment) #461 (comment) #461 (comment) #461 (comment) #461 (comment) #461 (comment) #461 (comment) #461 (comment) #461 (comment) #461 (comment) #461 (comment)	#461 (comment) #461 (comment) #461 (comment) #461 (comment) #461 (comment) #461 (comment) #461 (comment)

[nex-maximus]

Code Review Cost Analysis

Code Review Items	Code Review Cost(in $)
CODE_REVIEW_COMMENTS AND CODE_SUGGESTIONS(Number Of Runs - 1)	3.59
TOTAL_COST(Number Of Runs - 1)	3.59