fix: restore support for unquoted string values stored in text[]
#333
Conversation
…uring the transformation from PostgreSQL back into PHP in `TextArray`
WalkthroughThis pull request updates the PostgreSQL-to-PHP type conversions and exception handling. It updates the documentation to reflect a new version, renames and introduces exception classes for handling JSON data, and refactors transformation logic. The changes replace the legacy Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client Code
participant J as JsonTransformer
participant P as PostgresJsonToPHPArrayTransformer
C->>J: transformFromPostgresJson(postgresValue)
J->>P: transformPostgresJsonEncodedValueToPHPValue(postgresValue)
P-->>J: phpValue
J-->>C: phpValue
sequenceDiagram
participant C as Client Code
participant T as TextArray
participant P as PHPArrayToPostgresValueTransformer
C->>T: transformToPostgresTextArray(phpArray)
T->>P: transformToPostgresTextArray(phpArray)
P-->>T: postgresValue
T-->>C: postgresValue
Possibly related PRs
Suggested labels
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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
docs/UPGRADE.md (1)
3-3: Version number format change.The version number has been changed from "3.0.0" to "3.0". While this doesn't affect functionality, ensure this aligns with your project's versioning scheme.
tests/MartinGeorgiev/Utils/PostgresJsonToPHPArrayTransformerTest.php (2)
19-22: Ensure consistent type validation in test method signature.The method signature uses a union type
null|array|bool|int|string $phpValuebut the corresponding data provider return type is annotated asmixed. Consider either:
- Updating the data provider return type annotation to match the method signature
- Simplifying the method signature to use
mixed $phpValuefor consistency- public function can_transform_json_to_php_value(null|array|bool|int|string $phpValue, string $postgresValue): void + public function can_transform_json_to_php_value(mixed $phpValue, string $postgresValue): void
76-90: Test case for nested JSON structures in array elements.The test cases for PostgreSQL JSON arrays are currently limited to simple objects. Consider adding a test case with nested JSON structures to validate more complex transformations.
public static function provideValidJsonbArrayTransformations(): array { return [ 'empty array' => [ 'phpArray' => [], 'postgresArray' => '{}', ], 'array with one object' => [ 'phpArray' => ['{key:value}'], 'postgresArray' => '{{key:value}}', ], 'array with multiple objects' => [ 'phpArray' => ['{key1:value1}', '{key2:value2}'], 'postgresArray' => '{{key1:value1},{key2:value2}}', ], + 'array with nested JSON objects' => [ + 'phpArray' => ['{key1:{nested:value}}', '{key2:[1,2,3]}'], + 'postgresArray' => '{{key1:{nested:value}},{key2:[1,2,3]}}', + ], ]; }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
docs/UPGRADE.md(3 hunks)src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidJsonArrayItemForPHPException.php(2 hunks)src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidJsonItemForPHPException.php(1 hunks)src/MartinGeorgiev/Doctrine/DBAL/Types/JsonTransformer.php(2 hunks)src/MartinGeorgiev/Doctrine/DBAL/Types/JsonbArray.php(2 hunks)src/MartinGeorgiev/Doctrine/DBAL/Types/TextArray.php(3 hunks)src/MartinGeorgiev/Utils/ArrayDataTransformer.php(0 hunks)src/MartinGeorgiev/Utils/PHPArrayToPostgresValueTransformer.php(1 hunks)src/MartinGeorgiev/Utils/PostgresArrayToPHPArrayTransformer.php(1 hunks)src/MartinGeorgiev/Utils/PostgresJsonToPHPArrayTransformer.php(1 hunks)tests/MartinGeorgiev/Doctrine/DBAL/Types/JsonbArrayTest.php(2 hunks)tests/MartinGeorgiev/Doctrine/DBAL/Types/TextArrayTest.php(2 hunks)tests/MartinGeorgiev/Utils/ArrayDataTransformerTest.php(0 hunks)tests/MartinGeorgiev/Utils/PHPArrayToPostgresValueTransformerTest.php(1 hunks)tests/MartinGeorgiev/Utils/PostgresArrayToPHPArrayTransformerTest.php(1 hunks)tests/MartinGeorgiev/Utils/PostgresJsonToPHPArrayTransformerTest.php(1 hunks)
💤 Files with no reviewable changes (2)
- src/MartinGeorgiev/Utils/ArrayDataTransformer.php
- tests/MartinGeorgiev/Utils/ArrayDataTransformerTest.php
🧰 Additional context used
🧬 Code Definitions (8)
src/MartinGeorgiev/Doctrine/DBAL/Types/TextArray.php (2)
src/MartinGeorgiev/Utils/PHPArrayToPostgresValueTransformer.php (2)
PHPArrayToPostgresValueTransformer(16-101)transformToPostgresTextArray(27-44)src/MartinGeorgiev/Utils/PostgresArrayToPHPArrayTransformer.php (2)
PostgresArrayToPHPArrayTransformer(16-261)transformPostgresArrayToPHPArray(29-93)
src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidJsonItemForPHPException.php (1)
src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidJsonArrayItemForPHPException.php (3)
create(11-14)forInvalidType(16-19)forInvalidFormat(21-24)
src/MartinGeorgiev/Utils/PHPArrayToPostgresValueTransformer.php (1)
src/MartinGeorgiev/Doctrine/DBAL/Types/TextArray.php (1)
transformToPostgresTextArray(40-47)
src/MartinGeorgiev/Doctrine/DBAL/Types/JsonbArray.php (1)
src/MartinGeorgiev/Utils/PostgresJsonToPHPArrayTransformer.php (3)
PostgresJsonToPHPArrayTransformer(17-65)transformPostgresArrayToPHPArray(21-34)transformPostgresJsonEncodedValueToPHPArray(39-51)
tests/MartinGeorgiev/Doctrine/DBAL/Types/JsonbArrayTest.php (1)
src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidJsonArrayItemForPHPException.php (1)
InvalidJsonArrayItemForPHPException(9-25)
src/MartinGeorgiev/Utils/PostgresJsonToPHPArrayTransformer.php (4)
src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidJsonArrayItemForPHPException.php (3)
InvalidJsonArrayItemForPHPException(9-25)forInvalidType(16-19)forInvalidFormat(21-24)src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidJsonItemForPHPException.php (3)
InvalidJsonItemForPHPException(9-25)forInvalidType(16-19)forInvalidFormat(21-24)src/MartinGeorgiev/Doctrine/DBAL/Types/JsonbArray.php (1)
transformPostgresArrayToPHPArray(28-31)src/MartinGeorgiev/Utils/PostgresArrayToPHPArrayTransformer.php (1)
transformPostgresArrayToPHPArray(29-93)
tests/MartinGeorgiev/Utils/PHPArrayToPostgresValueTransformerTest.php (1)
src/MartinGeorgiev/Utils/PHPArrayToPostgresValueTransformer.php (2)
PHPArrayToPostgresValueTransformer(16-101)transformToPostgresTextArray(27-44)
tests/MartinGeorgiev/Utils/PostgresArrayToPHPArrayTransformerTest.php (2)
src/MartinGeorgiev/Utils/PostgresArrayToPHPArrayTransformer.php (2)
PostgresArrayToPHPArrayTransformer(16-261)transformPostgresArrayToPHPArray(29-93)src/MartinGeorgiev/Utils/PostgresJsonToPHPArrayTransformer.php (1)
transformPostgresArrayToPHPArray(21-34)
🔇 Additional comments (34)
src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidJsonArrayItemForPHPException.php (1)
9-9: Consistent renaming from JSONB to JSON.The class name has been changed from
InvalidJsonbArrayItemForPHPExceptiontoInvalidJsonArrayItemForPHPException, and the error message has been updated from 'Invalid JSONB format in array: %s' to 'Invalid JSON format in array: %s'. These changes make the exception more generic, allowing it to handle both JSONB and standard JSON formats, which aligns with the PR objective of improving support for different string value formats.Also applies to: 23-23
tests/MartinGeorgiev/Doctrine/DBAL/Types/JsonbArrayTest.php (1)
8-8: Test updates align with exception class changes.The test file has been appropriately updated to use the new
InvalidJsonArrayItemForPHPExceptionclass and its corresponding error message. This ensures the tests remain consistent with the implementation changes in the exception class.Also applies to: 100-101
src/MartinGeorgiev/Doctrine/DBAL/Types/TextArray.php (2)
8-9: Improved separation of concerns with specialized transformers.The code now uses two specialized transformer classes (
PHPArrayToPostgresValueTransformerandPostgresArrayToPHPArrayTransformer) instead of the singleArrayDataTransformerclass. This separation of concerns allows for more precise handling of different data formats, directly addressing the PR objective of restoring support for unquoted string values stored intext[]arrays.The method names have also been updated to better reflect their purpose, improving code readability and maintainability.
Also applies to: 46-46, 69-69
8-9:✅ Verification successful
Verify transformer behavior with unquoted strings.
Based on the PR objective, these changes aim to restore support for unquoted string values in
text[]. Let's verify that the new transformers correctly handle this case.Also applies to: 46-46, 69-69
🏁 Script executed:
#!/bin/bash # Checking how the new transformer handles unquoted strings in arrays echo "Looking for test cases that verify unquoted string handling:" rg -A 5 "unquoted" --glob="*.php" tests/ # Check the actual implementation in the transformers echo -e "\nChecking implementation in PostgresArrayToPHPArrayTransformer:" cat src/MartinGeorgiev/Utils/PostgresArrayToPHPArrayTransformer.php | grep -E -A 15 "transformPostgresArrayToPHPArray|parseValue"Length of output: 1666
Transformer Unquoted String Handling Verified
The unit test
can_transform_unquoted_postgres_array_to_php(intests/MartinGeorgiev/Doctrine/DBAL/Types/TextArrayTest.php) confirms that unquoted string values intext[]arrays are correctly handled. Additionally, a review of the implementation insrc/MartinGeorgiev/Utils/PostgresArrayToPHPArrayTransformer.phpshows that the methods correctly process unquoted strings. The changes at lines 8–9 (and corresponding adjustments at lines 46 and 69) are consistent with the intended behavior.docs/UPGRADE.md (2)
19-19: Updated exception handling guidance.The documentation has been appropriately updated to reflect the changes in exception classes. Instead of catching only
InvalidJsonbArrayItemForPHPException, users are now instructed to catch bothInvalidJsonItemForPHPExceptionandInvalidJsonArrayItemForPHPException. The code example has also been updated to use the new exception class.This guidance helps users migrate their code correctly when upgrading to the new version.
Also applies to: 32-32
19-19:❓ Verification inconclusive
Verify if both exceptions need to be caught.
The documentation says to catch both
InvalidJsonItemForPHPExceptionandInvalidJsonArrayItemForPHPException, but the example only shows catchingInvalidJsonArrayItemForPHPException. Let's verify if there are scenarios where both exceptions need to be caught or if the example should be updated to demonstrate catching both.Also applies to: 32-32
🏁 Script executed:
#!/bin/bash # Check where InvalidJsonItemForPHPException is thrown echo "Checking where InvalidJsonItemForPHPException is used:" rg "InvalidJsonItemForPHPException" --glob="*.php" src/ # Check if JsonbArray uses both exceptions echo -e "\nChecking JsonbArray implementation:" cat src/MartinGeorgiev/Doctrine/DBAL/Types/JsonbArray.php | grep -E "InvalidJson.*Exception"Length of output: 953
Inconsistency in Exception Handling Requirements
Based on the verification:
- The code (in src/MartinGeorgiev/Utils/PostgresJsonToPHPArrayTransformer.php) throws and documents the use of
InvalidJsonItemForPHPException.- Searches within JsonbArray.php (and the repository overall) did not reveal any usage of
InvalidJsonArrayItemForPHPException.It appears that only
InvalidJsonItemForPHPExceptionis actively used. Please verify whether there’s an intended scenario forInvalidJsonArrayItemForPHPException—if so, update the example in docs/UPGRADE.md to illustrate catching both; otherwise, adjust the documentation to only reference the exception that is actually thrown.src/MartinGeorgiev/Doctrine/DBAL/Types/JsonTransformer.php (2)
7-8: Good refactoring: Using a dedicated transformer class.The addition of the
PostgresJsonToPHPArrayTransformerutility class import helps to separate concerns and promote code reuse across the codebase.
35-38: Clean implementation that improves maintainability.Delegating the JSON transformation logic to a specialized class is a good architectural decision. This change centralizes the transformation logic, which makes maintenance easier and ensures consistent behavior across the codebase.
src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidJsonItemForPHPException.php (2)
1-26: Well-structured exception class following best practices.This new exception class:
- Uses clear static factory methods that provide context-specific error messages
- Extends the appropriate Doctrine exception class
- Properly handles formatting of error messages with consistent error reporting
The implementation follows a builder pattern with static factory methods, which makes error handling more concise and consistent throughout the codebase.
16-19: Clear error message for invalid Postgres JSON types.The error message is specific and clearly indicates that the failure is due to the Postgres value not being a valid JSON object. This will help developers troubleshoot issues more efficiently.
tests/MartinGeorgiev/Doctrine/DBAL/Types/TextArrayTest.php (4)
73-88: Comprehensive test coverage for backslash handling.These test cases thoroughly cover various backslash scenarios that might occur in Postgres text arrays, which is crucial for ensuring robust handling of special characters. The tests cover:
- Single backslashes at start/end
- Double backslashes at the end
- Triple backslashes in the middle
- Quadruple backslashes
This will help prevent regressions in backslash handling.
107-110: Good additional test case for standard string arrays.Adding a test case for basic string arrays ensures that simple use cases continue to work correctly while fixing more complex scenarios.
114-123: Important test case for unquoted Postgres arrays.This test directly addresses the PR objective of restoring support for unquoted string values in
text[]. It verifies that unquoted Postgres arrays are correctly transformed to PHP arrays.
125-134: Dedicated test for backslash handling.This test focuses specifically on the transformation of backslashes from Postgres to PHP, ensuring that backslashes in PostgreSQL strings are correctly preserved when converted to PHP values.
src/MartinGeorgiev/Doctrine/DBAL/Types/JsonbArray.php (3)
7-7: Good refactoring: Importing the new transformer utility.The addition of the
PostgresJsonToPHPArrayTransformerimport is consistent with the architectural changes being made throughout the codebase.
28-31: Simplified implementation with improved maintainability.Delegating the array transformation logic to a specialized utility class reduces code duplication and centralizes the transformation logic, making it easier to maintain and test.
33-39: Improved error handling for JSON array transformations.The new implementation delegates to a specialized transformer that includes proper exception handling for invalid JSON formats and types. This makes the code more robust while simplifying the implementation in this class.
src/MartinGeorgiev/Utils/PHPArrayToPostgresValueTransformer.php (3)
27-35: Robust single-dimension check.Using
\array_filter($phpArray, 'is_array')to detect any nested arrays is a straightforward and effective way to enforce single-dimensional input. This approach cleanly prevents multi-dimensional arrays from being converted erroneously.
37-44: Efficient construction of PostgreSQL array string.The combination of mapping each element through
formatValue()and then joining them with commas before wrapping with{}is an optimal and easily readable solution. It neatly enforces proper quoting and separates responsibilities between validation, mapping, and final assembly.
49-100: Comprehensive type handling informatValue().This method gracefully accounts for scalars, booleans,
null, objects, resources, and unknown types, ensuring that every case is stringified properly for PostgreSQL. The escaping logic also looks sound for quotes and backslashes. Overall, this offers robust coverage of potential input variations.src/MartinGeorgiev/Utils/PostgresJsonToPHPArrayTransformer.php (3)
21-34: Simplicity in splitting array items.After verifying an empty PostgreSQL array, the approach of removing
{}and splitting by},{is minimalistic and might be sufficient for single-dimensional arrays, but be cautious about any edge cases involving special characters or empty segments. No immediate issues stand out, though.
39-50: Correct exception handling on JSON decoding.Catching
\JsonExceptionand throwing a domain-specific exception (InvalidJsonArrayItemForPHPException) makes the error context explicit. This pattern is good for surfacing clearer semantic errors when invalid JSON is encountered.
56-64: Well-defined union return type.Returning
null|array|bool|float|int|stringafter JSON decode is flexible and documented. The approach is consistent with modern PHP patterns and helps consumers handle multiple data forms robustly.tests/MartinGeorgiev/Utils/PHPArrayToPostgresValueTransformerTest.php (4)
20-23: Clear test for basic transformations.Using a data provider (
provideValidTransformations) to check that each PHP array transforms to the correct PostgreSQL string ensures broad coverage of normal scenarios (numeric, strings, etc.). This is a solid foundation for regression prevention.
83-87: Exception coverage for non-single-dimension arrays.The test properly expects
InvalidArrayFormatExceptionto be thrown when passing multi-dimensional arrays. Validating the exception type and message is a good practice for clarity on expected failure modes.
109-121: Validation of resource handling.Confirming that a resource is transformed into
"(resource)"and verifying correctness after closing the stream is an excellent corner-case test. This ensures the code gracefully manages non-string data.
130-135: In-depth multi-dimensional array checks.The additional test for deeply nested arrays bolsters confidence in the single-dimension constraint. Throwing an exception guarantees consistent behavior and prevents unexpected PostgreSQL syntax errors.
src/MartinGeorgiev/Utils/PostgresArrayToPHPArrayTransformer.php (4)
29-44: Robust validation for array edge cases.The checks for empty input,
nullmarker, multi-dimensional syntax (},{or{{), and malformed nesting with braces demonstrate thorough handling of common pitfalls. This upfront validation clarifies error conditions.
78-93: Graceful fallback to manual parsing.Attempting JSON decoding first and then reverting to
parsePostgresArrayManually()ensures a fast path for properly quoted arrays without duplicating logic. This pattern is a smart balance of performance and flexibility in handling unquoted strings.
95-144: Detailed manual parsing logic.Manually iterating over each character, tracking quotes, escapes, and splitting on commas is thorough. It cleanly handles unquoted elements and ensures correct classification before delegating to
processPostgresValue().
216-260: Correct and careful unescape logic.The final string unescaping method accounts for multiple backslashes, escaped quotes, and trailing backslashes. This level of detail reduces the risk of subtle bugs related to nested escaping in real-world data.
tests/MartinGeorgiev/Utils/PostgresArrayToPHPArrayTransformerTest.php (1)
1-160: Comprehensive test coverage for PostgreSQL array to PHP array transformation.The test class provides thorough testing of the PostgresArrayToPHPArrayTransformer functionality, covering various scenarios including different data types, special characters, error cases, and edge conditions. The test data providers are well-structured with descriptive keys that explain the test purpose.
I particularly appreciate the tests for:
- Special character handling (quotes, backslashes)
- Unicode character support
- Numeric precision preservation
- Empty and null input handling
- Invalid format detection
This test suite effectively validates the implementation of the fix for unquoted string values stored in
text[]arrays.tests/MartinGeorgiev/Utils/PostgresJsonToPHPArrayTransformerTest.php (2)
92-117: Test for array value transformation is properly implemented.The test method
can_transform_json_array_item_to_php_arrayeffectively validates that JSON objects can be properly transformed to PHP arrays. The test cases cover both simple and nested object structures, which is good for ensuring the reliability of the transformer.
119-147: Thorough exception testing for error cases.The exception testing is well-structured, covering various error scenarios:
- Invalid JSON syntax
- Invalid JSON array items
- Non-array JSON values
These tests will help ensure robust error handling in the production code. The explicit exception message assertions are particularly valuable for providing clear error messages to users.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
tests/MartinGeorgiev/Utils/PostgresArrayToPHPArrayTransformerTest.php (2)
18-18: Incorrect PHPDoc parameter type annotation.The
@paramannotation specifiesarray<int, array<string, array|string>> $phpValue, but the actual parameter is a simple array based on the test cases inprovideValidTransformations(). The annotation should match the actual expected type.- * @param array<int, array<string, array|string>> $phpValue + * @param array<int|string, mixed> $phpValue
186-194: Strengthen assertion for escaped quotes test.The current assertion only checks that the result is a string and contains the word "quoted". This is a weak test that might pass even if the escaping logic is incorrect. Consider strengthening this test by asserting the exact expected value.
- // Just verify it's a string - self::assertIsString($result[0]); - self::assertStringContainsString('quoted', $result[0]); + // Verify the exact transformed value + self::assertSame('"quoted"', $result[0]);tests/MartinGeorgiev/Utils/PHPArrayToPostgresValueTransformerTest.php (5)
18-18: Incorrect PHPDoc parameter type annotation.The
@paramannotation specifiesarray<int, array<string, array|string>> $phpValue, but the actual parameter is a simple array based on the test cases. The annotation should match the actual expected type.- * @param array<int, array<string, array|string>> $phpValue + * @param array<int|string, mixed> $phpValue
123-125: Incorrect return type annotation for data provider.The
@returnannotation forprovideInvalidTransformations()includespostgresValuein the returned array shape, but the test method only usesphpValueandpostgresValueis not used.- * @return array<string, array{phpValue: array, postgresValue: string}> + * @return array<string, array{phpValue: array}>
146-146: Method name contains duplicate word "can".The method name
can_can_transform_object_with_to_string_method()has a duplicate "can".- public function can_can_transform_object_with_to_string_method(): void + public function can_transform_object_with_to_string_method(): void
169-179: Strengthen assertion for resource transformation.The current assertion only checks that the result string contains "resource (closed)". This is a weak test that might pass even if the formatting changes. Consider using a more specific assertion.
- self::assertStringContainsString('resource (closed)', PHPArrayToPostgresValueTransformer::transformToPostgresTextArray([$resource])); + $result = PHPArrayToPostgresValueTransformer::transformToPostgresTextArray([$resource]); + self::assertMatchesRegularExpression('/"resource \(closed\)"/', $result);
215-227: Duplicate test for multi-dimensional arrays.This test method
throws_exception_for_multi_dimensional_arrays()seems to duplicate the functionality already tested inthrows_invalid_argument_exception_when_tries_to_non_single_dimensioned_array_from_php_value(). Consider consolidating these tests to avoid duplication.The main difference appears to be the expected exception message assertion. You could add this assertion to the earlier test method and use a single data provider with more comprehensive test cases.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tests/MartinGeorgiev/Utils/PHPArrayToPostgresValueTransformerTest.php(1 hunks)tests/MartinGeorgiev/Utils/PostgresArrayToPHPArrayTransformerTest.php(1 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
tests/MartinGeorgiev/Utils/PostgresArrayToPHPArrayTransformerTest.php (5)
src/MartinGeorgiev/Utils/PostgresArrayToPHPArrayTransformer.php (2)
PostgresArrayToPHPArrayTransformer(16-261)transformPostgresArrayToPHPArray(29-93)tests/MartinGeorgiev/Doctrine/DBAL/Types/TextArrayTest.php (2)
can_transform_to_php_value(51-54)provideValidTransformations(62-112)tests/MartinGeorgiev/Doctrine/DBAL/Types/JsonbArrayTest.php (2)
can_transform_to_php_value(52-55)provideValidTransformations(60-91)src/MartinGeorgiev/Utils/PostgresJsonToPHPArrayTransformer.php (1)
transformPostgresArrayToPHPArray(21-34)tests/MartinGeorgiev/Utils/PHPArrayToPostgresValueTransformerTest.php (1)
provideValidTransformations(28-108)
tests/MartinGeorgiev/Utils/PHPArrayToPostgresValueTransformerTest.php (2)
src/MartinGeorgiev/Utils/PHPArrayToPostgresValueTransformer.php (2)
PHPArrayToPostgresValueTransformer(16-101)transformToPostgresTextArray(27-44)src/MartinGeorgiev/Doctrine/DBAL/Types/TextArray.php (1)
transformToPostgresTextArray(40-47)
⏰ Context from checks skipped due to timeout of 90000ms (20)
- GitHub Check: PHP 8.4 + Doctrine ORM 3.0 + Doctrine Lexer latest
- GitHub Check: PHP 8.4 + Doctrine ORM latest + Doctrine Lexer latest
- GitHub Check: PHP 8.4 + Doctrine ORM 2.18 + Doctrine Lexer latest
- GitHub Check: PHP 8.4 + Doctrine ORM latest + Doctrine Lexer 3.0
- GitHub Check: PHP 8.1 + Doctrine ORM 2.14 + Doctrine Lexer 1.2
- GitHub Check: PHP 8.4 + Doctrine ORM 3.0 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.4 + Doctrine ORM latest + Doctrine Lexer 2.1
- GitHub Check: PHP 8.4 + Doctrine ORM 2.14 + Doctrine Lexer latest
- GitHub Check: PHP 8.3 + Doctrine ORM 3.0 + Doctrine Lexer latest
- GitHub Check: PHP 8.3 + Doctrine ORM latest + Doctrine Lexer 3.0
- 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.3 + Doctrine ORM 3.0 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.3 + Doctrine ORM 2.14 + Doctrine Lexer latest
- GitHub Check: PHP 8.2 + Doctrine ORM latest + Doctrine Lexer latest
- GitHub Check: PHP 8.3 + Doctrine ORM 2.18 + Doctrine Lexer 2.1
- GitHub Check: PHP 8.2 + Doctrine ORM 2.18 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.1 + Doctrine ORM 3.0 + Doctrine Lexer latest
- GitHub Check: PHP 8.1 + Doctrine ORM latest + Doctrine Lexer 3.0
- GitHub Check: sloth
🔇 Additional comments (4)
tests/MartinGeorgiev/Utils/PostgresArrayToPHPArrayTransformerTest.php (3)
117-124: LGTM! Properly testing unquoted strings in PostgreSQL arrays.The test cases for unquoted strings and mixed quoted/unquoted strings are crucial for the PR objective of restoring support for unquoted string values in
text[]. This implementation correctly verifies that the transformer can handle both formats.
196-205: LGTM! Good test for numeric precision.This test properly verifies that large integers and high-precision decimals maintain their precision during transformation. This is important for applications dealing with financial data or other scenarios where precision is critical.
231-233: Comprehensive test for malformed nesting.Great test case for malformed nesting. This edge case is important to prevent invalid array structures. The test ensures that attempted nesting within a single-dimensional array context correctly throws an exception.
tests/MartinGeorgiev/Utils/PHPArrayToPostgresValueTransformerTest.php (1)
228-254: Comprehensive test cases for multi-dimensional arrays.Good set of test cases for multi-dimensional arrays covering different nesting patterns. These test cases are valuable for ensuring the transformer correctly rejects nested arrays across various structures.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
tests/MartinGeorgiev/Utils/PostgresArrayToPHPArrayTransformerTest.php (1)
176-179:⚠️ Potential issueFix the backslash escaping in PostgreSQL test case
The test case for
manual parsing with trailing backslashdoesn't conform to PostgreSQL's standard escaping convention, where backslashes must be doubled for proper escaping. This was previously flagged in a review but hasn't been fixed.Apply this diff to fix the test case:
- 'manual parsing with trailing backslash' => [ - 'phpValue' => ['backslash\\', 'another\one'], - 'postgresValue' => '{backslash\,another\one}', + 'manual parsing with trailing backslash' => [ + 'phpValue' => ['backslash\\', 'another\one'], + 'postgresValue' => '{backslash\\,another\\one}',
🧹 Nitpick comments (1)
tests/MartinGeorgiev/Utils/PHPArrayToPostgresValueTransformerTest.php (1)
158-167: Consider being more specific with assertionsThe test for objects without
__toStringuses a less specific assertion. Consider using a regex pattern match or a more precise assertion to verify the exact format.- self::assertStringContainsString('class@anonymous', PHPArrayToPostgresValueTransformer::transformToPostgresTextArray([$object])); + $result = PHPArrayToPostgresValueTransformer::transformToPostgresTextArray([$object]); + self::assertMatchesRegularExpression('/"class@anonymous.*"/', $result);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tests/MartinGeorgiev/Utils/PHPArrayToPostgresValueTransformerTest.php(1 hunks)tests/MartinGeorgiev/Utils/PostgresArrayToPHPArrayTransformerTest.php(1 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
tests/MartinGeorgiev/Utils/PostgresArrayToPHPArrayTransformerTest.php (2)
src/MartinGeorgiev/Utils/PostgresArrayToPHPArrayTransformer.php (2)
PostgresArrayToPHPArrayTransformer(16-261)transformPostgresArrayToPHPArray(29-93)src/MartinGeorgiev/Utils/PostgresJsonToPHPArrayTransformer.php (1)
transformPostgresArrayToPHPArray(21-34)
tests/MartinGeorgiev/Utils/PHPArrayToPostgresValueTransformerTest.php (2)
src/MartinGeorgiev/Utils/PHPArrayToPostgresValueTransformer.php (2)
PHPArrayToPostgresValueTransformer(16-101)transformToPostgresTextArray(27-44)src/MartinGeorgiev/Doctrine/DBAL/Types/TextArray.php (1)
transformToPostgresTextArray(40-47)
⏰ Context from checks skipped due to timeout of 90000ms (18)
- GitHub Check: PHP 8.2 + Doctrine ORM 2.18 + Doctrine Lexer latest
- GitHub Check: PHP 8.4 + Doctrine ORM 2.14 + Doctrine Lexer 2.1
- GitHub Check: PHP 8.3 + Doctrine ORM 2.14 + Doctrine Lexer latest
- GitHub Check: PHP 8.2 + Doctrine ORM latest + Doctrine Lexer 2.1
- GitHub Check: PHP 8.4 + Doctrine ORM latest + Doctrine Lexer 2.1
- GitHub Check: PHP 8.2 + Doctrine ORM 3.0 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.1 + Doctrine ORM 2.14 + Doctrine Lexer 1.2
- GitHub Check: PHP 8.4 + Doctrine ORM 3.0 + Doctrine Lexer latest
- GitHub Check: PHP 8.3 + Doctrine ORM latest + Doctrine Lexer 3.0
- GitHub Check: PHP 8.4 + Doctrine ORM latest + Doctrine Lexer 3.0
- GitHub Check: PHP 8.2 + Doctrine ORM 2.18 + Doctrine Lexer 2.1
- GitHub Check: PHP 8.2 + Doctrine ORM 2.14 + Doctrine Lexer latest
- GitHub Check: PHP 8.4 + Doctrine ORM 2.18 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.4 + Doctrine ORM 2.18 + Doctrine Lexer latest
- GitHub Check: PHP 8.1 + Doctrine ORM 2.18 + Doctrine Lexer latest
- GitHub Check: PHP 8.1 + Doctrine ORM latest + Doctrine Lexer latest
- GitHub Check: PHP 8.1 + Doctrine ORM 2.18 + Doctrine Lexer 3.0
- GitHub Check: sloth
🔇 Additional comments (10)
tests/MartinGeorgiev/Utils/PostgresArrayToPHPArrayTransformerTest.php (5)
94-108: LGTM: Comprehensive test for special characters and escapingThe test cases for strings with special characters, backslashes, and Unicode characters provide excellent coverage for various edge cases that could occur with PostgreSQL arrays.
117-138: LGTM: Multi-dimensional array validationGood test coverage for checking that multi-dimensional arrays are properly rejected with appropriate exception messages.
195-199: LGTM: Numeric precision preservationGood test for ensuring large integers and floating-point numbers maintain precision during transformation. This is important for applications dealing with financial or scientific data.
201-229: LGTM: Edge case validation for invalid formatsThe test cases for invalid PostgreSQL arrays properly cover common error scenarios like unclosed strings, invalid formats, and malformed nesting.
151-181:✅ Verification successful
Verify manual parsing behavior with automated tests
The test cases for manual parsing are important as they test the fallback mechanism when
json_decodefails. This is critical for handling unquoted strings in PostgreSQL arrays.
🏁 Script executed:
#!/bin/bash # Check how the transformer handles manual parsing in the actual implementation # Look at the source code to verify the manual parsing implementation rg -A 10 "parsePostgresArrayManually" src/Length of output: 1904
Manual Parsing Fallback Verified
The automated tests in
tests/MartinGeorgiev/Utils/PostgresArrayToPHPArrayTransformerTest.phpeffectively cover the edge cases for manual parsing (handling unquoted text, escaping scenarios, and trailing backslashes). The source code insrc/MartinGeorgiev/Utils/PostgresArrayToPHPArrayTransformer.phpconfirms that the fallback mechanism, via theparsePostgresArrayManuallymethod, is correctly invoked whenjson_decodefails. No further changes are required.tests/MartinGeorgiev/Utils/PHPArrayToPostgresValueTransformerTest.php (5)
92-102: LGTM: Comprehensive escaping of special charactersGood test coverage for handling strings with special characters and backslashes, ensuring proper escaping in the PostgreSQL array format.
146-156: LGTM: Object transformation with __toStringGood test for verifying that objects with the
__toStringmethod are properly converted to their string representation.
169-190: LGTM: Resource handling testsGood tests for both open and closed resources. This ensures the transformer correctly handles resource types which are common in PHP applications.
192-213: LGTM: Mixed type array transformationExcellent test for arrays containing mixed data types, ensuring that each type is correctly transformed in the final PostgreSQL array string.
215-254: LGTM: Comprehensive multi-dimensional array testsThe test cases for multi-dimensional arrays thoroughly cover various scenarios, including arrays with nested arrays, multiple nested arrays, and deeply nested structures.
martin-georgiev
changed the title
text[]text[]
Summary by CodeRabbit
Documentation
New Features
Refactor