Skip to content
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

1159 refactor authz cpp file #1161

Merged
merged 4 commits into from
Dec 10, 2024
Merged

Conversation

JoshuaSBrown
Copy link
Collaborator

@JoshuaSBrown JoshuaSBrown commented Dec 4, 2024

#DEPENDS

This PR should be merged first. https://github.com/ORNL/DataFed/pull/1158/files

Summary by Sourcery

Refactor the AuthzWorker class to enhance code clarity and maintainability by modularizing functionality into specific methods. Update the build system to support building and linking tests for the AuthzWorker class, including necessary dependencies. Implement comprehensive unit tests to ensure the correctness of the AuthzWorker's functionality and the getVersion function.

Enhancements:

  • Refactor the AuthzWorker class to improve code organization and readability by separating concerns into distinct methods for URL validation, path validation, and authorization checks.

Build:

  • Add options to the CMake configuration to build DataFed Authz tests and include necessary libraries for GSSAPI and Globus Common.

Tests:

  • Introduce unit tests for the AuthzWorker class, covering various scenarios such as path validation, URL validation, and response processing.
  • Add a test suite for the getVersion function to verify version string correctness.

@JoshuaSBrown JoshuaSBrown self-assigned this Dec 4, 2024
Copy link

sourcery-ai bot commented Dec 4, 2024

Reviewer's Guide by Sourcery

This PR refactors the AuthzWorker class by splitting it into header and implementation files, improving code organization, adding comprehensive documentation, and introducing unit tests. The changes focus on better code structure, maintainability, and testability while preserving the original functionality.

Class diagram for the refactored AuthzWorker

classDiagram
    class AuthzWorker {
        +AuthzWorker(Config *a_config, LogContext log_context)
        +~AuthzWorker()
        +int checkAuth(char *client_id, char *path, char *action)
        +bool isTestPath(const std::string &) const
        +bool isURLValid(char * full_ftp_url) const
        +bool isPathValid(const std::string & posix_path) const
        +std::string getAuthzPath(char * full_ftp_url)
        +std::string removeOrigin(char * full_ftp_url) const
        +int processResponse(ICommunicator::Response & response)
        -void initCommunicator()
        -Config *m_config
        -std::string m_test_path
        -std::string m_local_globus_path_root
        -LogContext m_log_context
        -std::unique_ptr<ICredentials> m_sec_ctx
        -std::unique_ptr<ICommunicator> m_comm
        -std::unordered_map<CredentialType, std::string> m_cred_options
    }
Loading

File-Level Changes

Change Details Files
Refactored AuthzWorker class implementation into separate header and source files
  • Split class declaration into header file with detailed documentation
  • Moved implementation details to source file
  • Added private member variables for better encapsulation
  • Introduced new helper methods for better code organization
  • Added comprehensive method documentation using Doxygen style comments
repository/gridftp/globus5/authz/source/AuthzWorker.cpp
repository/gridftp/globus5/authz/source/AuthzWorker.hpp
Added comprehensive unit test suite for AuthzWorker
  • Created test cases for path validation functionality
  • Added tests for URL processing and validation
  • Implemented tests for authorization checking
  • Created tests for response processing
  • Added test configuration setup
repository/gridftp/globus5/authz/tests/unit/test_AuthzWorker.cpp
repository/gridftp/globus5/authz/tests/unit/test_getVersion.cpp
Updated build system to support unit testing
  • Added CMake configuration for unit tests
  • Added new build option BUILD_AUTHZ_TESTS
  • Added GSSAPI and GlobusCommon dependency handling
  • Updated main CMakeLists.txt to include test dependencies
CMakeLists.txt
repository/gridftp/globus5/authz/CMakeLists.txt
repository/gridftp/globus5/authz/tests/unit/CMakeLists.txt
repository/gridftp/globus5/authz/tests/CMakeLists.txt
cmake/GSSAPI.cmake
cmake/GlobusCommon.cmake

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time. You can also use
    this command to specify where the summary should be inserted.

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@JoshuaSBrown JoshuaSBrown added the Component: GridFTP Auth Module Relates to GridFTP authorization library label Dec 4, 2024
@JoshuaSBrown JoshuaSBrown linked an issue Dec 4, 2024 that may be closed by this pull request
Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @JoshuaSBrown - I've reviewed your changes and they look great!

Here's what I looked at during the review
  • 🟢 General issues: all looks good
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@JoshuaSBrown
Copy link
Collaborator Author

JoshuaSBrown commented Dec 5, 2024

@par-hermes format

1 similar comment
@JoshuaSBrown
Copy link
Collaborator Author

@par-hermes format

@JoshuaSBrown JoshuaSBrown changed the base branch from devel to master December 10, 2024 09:59
@JoshuaSBrown JoshuaSBrown changed the base branch from master to devel December 10, 2024 09:59
@JoshuaSBrown JoshuaSBrown merged commit 5cf925f into devel Dec 10, 2024
9 checks passed
@JoshuaSBrown JoshuaSBrown deleted the 1159-refactor-authz-cpp-file branch December 10, 2024 10:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: GridFTP Auth Module Relates to GridFTP authorization library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Authz, Test] - refactor the cpp authorization class testable
2 participants