Skip to content

Conversation

@martin-georgiev
Copy link
Owner

@martin-georgiev martin-georgiev commented Apr 13, 2025

Summary by CodeRabbit

  • New Features
    • Updated JSON conversion functions now support flexible, variable argument counts with robust validation.
  • Documentation
    • Improved user guidance with clearer usage examples and detailed parameter descriptions.
  • Tests
    • Expanded test coverage ensures that improper usage is reliably caught and clearly communicated.

@coveralls
Copy link

Coverage Status

coverage: 94.87% (+0.06%) from 94.808%
when pulling 471c3fe on new-funcs2
into 0cda902 on main.

@coderabbitai
Copy link

coderabbitai bot commented Apr 13, 2025

Walkthrough

The pull request updates several Doctrine ORM query functions to extend a variadic base class and integrate boolean argument validation. Each function now includes a new method to enforce argument count and type rules, with modifications to the function prototype and documentation. The changes improve error handling by throwing exceptions when invalid arguments are provided. Parallel updates in the test suite extend coverage with new assertions and expected SQL/DQL statements for various error cases.

Changes

File(s) Change Summary
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/(ArrayToJson, JsonbInsert, JsonbSet, RowToJson).php Changed class inheritance from BaseFunction to BaseVariadicFunction; integrated BooleanValidationTrait; added validateArguments method with argument count and boolean validations; updated customizeFunction and documentation comments.
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/(ArrayToJsonTest, JsonbInsertTest, JsonbSetTest, RowToJsonTest).php Enhanced test coverage by adding new exception tests (e.g. test_invalid_boolean_throws_exception, test_too_many_arguments_throws_exception, and test_too_few_arguments_throws_exception); updated expected SQL/DQL statements with descriptive keys.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Function
    participant Validator as BooleanValidationTrait

    Client->>Function: Call function with arguments
    Function->>Function: validateArguments(...)
    alt Valid argument count
        alt Boolean argument provided
            Function->>Validator: validateBoolean(arg)
            alt Boolean valid
                Function->>Client: Execute function and return result
            else Boolean invalid
                Function->>Client: Throw InvalidBooleanException
            end
        else
            Function->>Client: Execute function and return result
        end
    else Invalid argument count
        Function->>Client: Throw InvalidArgumentForVariadicFunctionException
    end
Loading

Possibly related PRs

Poem

I'm just a bunny, leaping with glee,
Coding carrots and JSON merrily.
With new validations, my hops are precise,
Boolean checks ensure errors think twice.
In a garden of functions, I twirl and play,
Celebrating each change in a bunny-like way!
🥕🐰 Happy coding, hooray!

Tip

⚡💬 Agentic Chat (Pro Plan, General Availability)
  • We're introducing multi-step agentic chat in review comments and issue comments, within and outside of PR's. This feature enhances review and issue discussions with the CodeRabbit agentic chat by enabling advanced interactions, including the ability to create pull requests directly from comments and add commits to existing pull requests.

📜 Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0cda902 and 471c3fe.

📒 Files selected for processing (8)
  • src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ArrayToJson.php (1 hunks)
  • src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbInsert.php (1 hunks)
  • src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbSet.php (1 hunks)
  • src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RowToJson.php (1 hunks)
  • tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ArrayToJsonTest.php (2 hunks)
  • tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbInsertTest.php (2 hunks)
  • tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbSetTest.php (2 hunks)
  • tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RowToJsonTest.php (3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/**/*.php`: Use the PostgreSQL official documentation to verify that tests include comprehensive use cases and example SQL que...

tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/**/*.php: Use the PostgreSQL official documentation to verify that tests include comprehensive use cases and example SQL queries for the tested SQL functions and operators.

  • tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ArrayToJsonTest.php
  • tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RowToJsonTest.php
  • tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbSetTest.php
  • tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbInsertTest.php
🧬 Code Graph Analysis (7)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ArrayToJson.php (3)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidArgumentForVariadicFunctionException.php (2)
  • InvalidArgumentForVariadicFunctionException (7-46)
  • between (29-37)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php (1)
  • BaseVariadicFunction (20-73)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Traits/BooleanValidationTrait.php (1)
  • validateBoolean (25-37)
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ArrayToJsonTest.php (2)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidArgumentForVariadicFunctionException.php (1)
  • InvalidArgumentForVariadicFunctionException (7-46)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidBooleanException.php (1)
  • InvalidBooleanException (14-33)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RowToJson.php (3)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidArgumentForVariadicFunctionException.php (2)
  • InvalidArgumentForVariadicFunctionException (7-46)
  • between (29-37)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php (1)
  • BaseVariadicFunction (20-73)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Traits/BooleanValidationTrait.php (1)
  • validateBoolean (25-37)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbSet.php (3)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidArgumentForVariadicFunctionException.php (2)
  • InvalidArgumentForVariadicFunctionException (7-46)
  • between (29-37)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php (1)
  • BaseVariadicFunction (20-73)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Traits/BooleanValidationTrait.php (1)
  • validateBoolean (25-37)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbInsert.php (3)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidArgumentForVariadicFunctionException.php (2)
  • InvalidArgumentForVariadicFunctionException (7-46)
  • between (29-37)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php (1)
  • BaseVariadicFunction (20-73)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Traits/BooleanValidationTrait.php (1)
  • validateBoolean (25-37)
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbSetTest.php (3)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidArgumentForVariadicFunctionException.php (1)
  • InvalidArgumentForVariadicFunctionException (7-46)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidBooleanException.php (1)
  • InvalidBooleanException (14-33)
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbInsertTest.php (3)
  • test_invalid_boolean_throws_exception (37-44)
  • test_too_few_arguments_throws_exception (46-53)
  • test_too_many_arguments_throws_exception (55-62)
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbInsertTest.php (2)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidArgumentForVariadicFunctionException.php (1)
  • InvalidArgumentForVariadicFunctionException (7-46)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidBooleanException.php (1)
  • InvalidBooleanException (14-33)
⏰ Context from checks skipped due to timeout of 90000ms (20)
  • GitHub Check: sloth
  • GitHub Check: PHP 8.4 + Doctrine ORM 3.0 + Doctrine Lexer latest
  • GitHub Check: PHP 8.3 + Doctrine ORM 2.18 + Doctrine Lexer 3.0
  • GitHub Check: PHP 8.4 + Doctrine ORM 2.14 + Doctrine Lexer 2.1
  • GitHub Check: PHP 8.3 + Doctrine ORM 3.0 + Doctrine Lexer latest
  • GitHub Check: PHP 8.3 + Doctrine ORM latest + Doctrine Lexer latest
  • GitHub Check: PHP 8.4 + Doctrine ORM 2.18 + Doctrine Lexer 2.1
  • GitHub Check: PHP 8.4 + Doctrine ORM latest + Doctrine Lexer latest
  • GitHub Check: PHP 8.3 + Doctrine ORM 2.14 + Doctrine Lexer 2.1
  • GitHub Check: PHP 8.4 + Doctrine ORM 2.18 + Doctrine Lexer 3.0
  • GitHub Check: PHP 8.4 + Doctrine ORM latest + Doctrine Lexer 3.0
  • GitHub Check: PHP 8.3 + Doctrine ORM latest + Doctrine Lexer 3.0
  • GitHub Check: PHP 8.3 + Doctrine ORM latest + Doctrine Lexer 2.1
  • GitHub Check: PHP 8.3 + Doctrine ORM 3.0 + Doctrine Lexer 3.0
  • GitHub Check: PHP 8.2 + Doctrine ORM 2.14 + Doctrine Lexer latest
  • GitHub Check: PHP 8.2 + Doctrine ORM 2.18 + Doctrine Lexer latest
  • GitHub Check: PHP 8.2 + Doctrine ORM latest + Doctrine Lexer 3.0
  • GitHub Check: PHP 8.1 + Doctrine ORM 2.18 + Doctrine Lexer latest
  • GitHub Check: PHP 8.1 + Doctrine ORM 3.0 + Doctrine Lexer latest
  • GitHub Check: PHP 8.2 + Doctrine ORM 3.0 + Doctrine Lexer 3.0
🔇 Additional comments (50)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ArrayToJson.php (8)

7-9: Use statements added for Node, InvalidArgumentForVariadicFunctionException, and BooleanValidationTrait
These lines incorporate necessary imports for the new variadic function and boolean validation logic. The dependencies are well-structured and relevant.


14-17: Updated documentation references
The docblock now correctly references PostgreSQL 16 and clarifies usage with a pretty_bool argument. The explanations for how a multidimensional array becomes JSON are clear.


21-23: Examples demonstrating optional boolean parameter
Providing realistic usage examples in the docblock improves clarity. This helps developers understand how to invoke ARRAY_TO_JSON with or without the boolean argument.


25-25: Class now extends BaseVariadicFunction
Switching to BaseVariadicFunction aligns with the new variadic logic for argument handling. This is a suitable approach for functions accepting one or two parameters.


27-27: Trait inclusion for boolean validation
Using BooleanValidationTrait is an elegant solution for reusing the logic that checks whether the second argument is a valid boolean string.


32-32: Customized function prototype
Preserving the function prototype string as array_to_json(%s) maintains clarity and consistency with PostgreSQL function naming.


34-39: Argument count validation
Enforcing between 1 and 2 arguments is correct for array_to_json. Throwing InvalidArgumentForVariadicFunctionException::between ensures descriptive error messages.


41-43: Boolean validation for second argument
This correctly invokes the validateBoolean method to confirm that the second argument is a valid 'true' or 'false' string literal. The implementation is concise and effective.

src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RowToJson.php (8)

7-9: Added imports for Node, InvalidArgumentForVariadicFunctionException, and BooleanValidationTrait
This mirrors the structure in ArrayToJson for handling variadic arguments and boolean validation—ensuring consistency across similar functions.


14-16: Documentation improvements
Clarifying the function’s behavior, especially the new pretty_bool parameter, makes the class easier to use. Referencing PostgreSQL 16 is helpful for the latest features and syntax.


20-22: Additional usage examples
Showing how to call ROW_TO_JSON with or without 'true' highlights the optional boolean parameter. Demonstrations in the docblock are greatly beneficial for usage reference.


24-24: Class now extends BaseVariadicFunction
Adopting BaseVariadicFunction matches the updated architecture for variable argument counts, enabling more flexible function definitions.


26-26: Trait usage for boolean validation
Like in ArrayToJson, BooleanValidationTrait handles the logic for checking valid boolean strings. This maintains uniform validation across similar custom PostgreSQL functions.


31-31: Customized function prototype
Keeping row_to_json(%s) clearly associates the class with the underlying PostgreSQL function name, maintaining standard naming conventions.


33-39: Validate argument count
Restricting arguments to one or two is appropriate for row_to_json, throwing an exception with a clear error message if the range is not respected.


40-43: Ensure boolean correctness on second parameter
Applying validateBoolean for the second argument ensures alignment with PostgreSQL’s pretty_bool usage for ROW_TO_JSON.

tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ArrayToJsonTest.php (5)

9-10: Imported exceptions for test coverage
Importing InvalidArgumentForVariadicFunctionException and InvalidBooleanException indicates robust negative test coverage for the new validations.


25-25: Added SQL statement with pretty print
Including 'true' in the SQL confirms the function can handle the optional boolean parameter. Ensures real-case coverage in tests.


33-33: Added DQL statement with pretty print
Mirroring the SQL test in DQL ensures integration correctness from the ORM perspective. This is good coverage for the new parameter.


37-44: Test invalid boolean
The test verifies that providing an unsupported boolean string triggers an InvalidBooleanException, confirming correct enforcement of 'true' or 'false'. This negative test is crucial to maintain data integrity.


46-53: Test too many arguments
Checks that InvalidArgumentForVariadicFunctionException is thrown when the function receives more than two arguments. This ensures argument count rules are respected.

tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RowToJsonTest.php (5)

8-9: Imports for new exceptions are consistent.
These imports align well with the newly introduced argument validation logic.


26-26: New SQL statement for pretty print flag is valid.
This updated query correctly demonstrates how to pass a boolean argument as 'true'.


35-35: New DQL statement for pretty print matches the updated SQL.
Ensures that the optional boolean parameter 'true' is passed consistently in the test.


38-46: Test coverage for invalid boolean values is commendable.
This solidifies confidence in the exception-handling path for unexpected boolean strings.


48-55: Test coverage for excessive arguments ensures proper validation enforcement.
The function correctly rejects more than two arguments, and the test verifies this behavior.

src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbSet.php (6)

14-21: Documentation improvements reflect accurate PostgreSQL behavior.
The explanation of the create_missing default is clear, aligning with official docs.


25-27: Examples showcase proper usage of the function in DQL.
These illustrations help clarify how to pass the path and the create_missing flag.


29-29: Extending BaseVariadicFunction matches the new variadic approach.
Switching from a fixed-argument base class to a variadic base is consistent with the PR objectives.


31-32: Use of BooleanValidationTrait is appropriate.
This trait cleanly consolidates logic for validating boolean arguments.


35-36: Function prototype set to 'jsonb_set(%s)' is correct.
This matches PostgreSQL syntax for jsonb_set and ensures arguments are processed variadically.


38-49: Argument validation ensures correct parameter count and boolean check.
The logic accurately requires 3-4 arguments and validates the fourth as a boolean, consistent with PostgreSQL's create_missing parameter.

src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbInsert.php (6)

14-17: Documentation clarifies the insertion logic at a specified path.
Noting that the existing value remains unchanged unless the last parameter is true. Well-aligned with PostgreSQL docs.


21-23: Usage examples illustrate how to handle path and boolean flags.
These DQL samples are helpful to demonstrate correct invocation of JSONB_INSERT.


25-25: Extending BaseVariadicFunction aligns with optional boolean support.
This change allows flexible argument counts for JSONB_INSERT, as per PostgreSQL capabilities.


27-28: Use of BooleanValidationTrait is necessary for validating create_if_missing.
Keeps the logic consistent across other JSONB-based functions.


31-32: Function prototype updated to jsonb_insert(%s).
Ensures arguments are handled variadically without rigid placeholders.


34-45: Validation method correctly enforces argument counts and boolean checks.
It matches PostgreSQL expectations by allowing 3 or 4 parameters and verifying the last parameter if present.

tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbInsertTest.php (6)

8-9: Nice addition of exception imports for better error handling.

These imports are appropriate for the new validation tests that check both argument counts and boolean parameter validation.


24-25: Improved test organization with descriptive SQL statement keys.

Using descriptive keys like 'basic usage' and 'with create-if-missing parameter' enhances readability and makes the tests more maintainable than using numeric indices.


32-33: Improved test organization with descriptive DQL statement keys.

The consistent naming between SQL and DQL statements makes it easier to understand the test cases and their expected outcomes.


37-44: Good test coverage for invalid boolean parameter.

This test correctly validates that the function properly rejects non-boolean values in the optional fourth parameter. According to PostgreSQL documentation, jsonb_insert's fourth parameter must be a boolean.


46-53: Proper validation of minimum argument count.

This test ensures the function enforces the PostgreSQL constraint that jsonb_insert requires at least 3 arguments.


55-62: Proper validation of maximum argument count.

This test verifies that the function correctly rejects calls with more than the 4 arguments allowed by PostgreSQL's jsonb_insert function.

tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/JsonbSetTest.php (6)

8-9: Good addition of exception imports for function validation.

These imports support the new tests that validate both argument counts and boolean parameter types.


24-25: Improved test organization with descriptive SQL keys.

Using descriptive keys like 'basic usage' and 'with create-if-missing parameter' improves readability and test maintenance.


32-33: Consistent naming approach for DQL statements.

The DQL statement keys match the SQL statement keys, creating a consistent pattern across the test suite.


37-44: Good validation of boolean parameter requirements.

This test correctly validates that the function rejects non-boolean values in the optional fourth parameter. According to PostgreSQL documentation, jsonb_set's fourth parameter must be a boolean.


46-53: Proper validation of minimum argument count.

This test ensures the function enforces the PostgreSQL constraint that jsonb_set requires at least 3 arguments.


55-62: Proper validation of maximum argument count.

This test verifies that the function correctly rejects calls with more than the 4 arguments allowed by PostgreSQL's jsonb_set function.

✨ Finishing Touches
  • 📝 Generate Docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai plan to trigger planning for file edits and PR creation.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@martin-georgiev martin-georgiev merged commit 67265cc into main Apr 13, 2025
47 of 50 checks passed
@martin-georgiev martin-georgiev deleted the new-funcs2 branch April 13, 2025 07:49
@github-actions github-actions bot mentioned this pull request Apr 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants