Skip to content

added test cases for (#351) #389

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.

Sign up for GitHub

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

Closed
j4r3kb wants to merge 2 commits into from
Closed

added test cases for (#351) #389

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
289450f
added test cases for (#351)
j4r3kb May 25, 2025
ab1462b
updated postgresValue strings in test cases
j4r3kb May 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 50 additions & 4 deletions tests/Unit/MartinGeorgiev/Utils/PostgresArrayToPHPArrayTransformerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ class PostgresArrayToPHPArrayTransformerTest extends TestCase
#[Test]
public function can_transform_to_php_value(array $phpValue, string $postgresValue): void
{
self::assertEquals($phpValue, PostgresArrayToPHPArrayTransformer::transformPostgresArrayToPHPArray($postgresValue));
self::assertEquals($phpValue, PostgresArrayToPHPArrayTransformer::transformPostgresArrayToPHPArray($postgresValue), 'V3 values not equal');
self::assertEquals($phpValue, $this->transformPostgresTextArrayToPHPArray($postgresValue), 'V2 values not equal');
}

/**
Expand Down Expand Up @@ -142,9 +143,13 @@ public static function provideValidTransformations(): array
'phpValue' => ['', ''],
'postgresValue' => '{"",""}',
],
'github #351 regression: string with special characters and backslash' => [
'phpValue' => ["!@#\\$%^&*()_+=-}{[]|\":;'\\?><,./"],
'postgresValue' => '{"!@#\$%^&*()_+=-}{[]|\":;\'\?><,./"}',
'github #351 regression #1: string with special characters and backslash' => [
'phpValue' => ['⥀!@#$%^&*()_+=-}{[]|":;\'\?><,./'],
'postgresValue' => '{"⥀!@#$%^&*()_+=-}{[]|\\":;\'\\\\?><,./"}',
Copy link
Owner

@martin-georgiev martin-georgiev May 26, 2025

Choose a reason for hiding this comment

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

@j4r3kb why do you expect PHP's '\? to become '\\\\? in Postgres?

Copy link
Author

@j4r3kb j4r3kb May 26, 2025

Choose a reason for hiding this comment

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

I used Xdebug. Have a breakpoint where the value is returned from the DB, then used the option "Copy value as var export" - maybe it was a mistake on my part.
Plain copy value gives {"⥀!@#$%^&*()_+=-}{[]|\":;'\\?><,./"}. Then you have to quote it in the code to be part of the array, so it should become
'postgresValue' => '{"⥀!@#$%^&*()_+=-}{[]|\":;\'\\?><,./"}',

I think the tricky part in the input array of 'phpValue' => ['⥀!@#$%^&*()_+=-}{[]|":;\'\?><,./'], is the \' inside the string. The array value is enclosed in single quotes and contains a single quote inside that has to be escaped by \.
So, as it returns from the DB it should stay like this and only the \ before ? should be returned as \\.

Sorry If I mess something up with those return values, it's confusing with the escaping.

However you alter the code keep in mind the function I pasted here from v2. They both should have the same result right?

Copy link
Author

@j4r3kb j4r3kb May 26, 2025

Choose a reason for hiding this comment

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

I pushed a new commit with a changed postgresValue.

],
'github #351 regression #2: string with special characters, backslash and additional element' => [
'phpValue' => ['⥀!@#$%^&*()_+=-}{[]|":;\'\?><,./', 'text'],
'postgresValue' => '{"⥀!@#$%^&*()_+=-}{[]|\\":;\'\\\\?><,./",text}',
],
'backslash before backslash' => [
'phpValue' => ['a\b'],
Expand Down Expand Up @@ -254,4 +259,45 @@ public static function provideInvalidPostgresArrays(): array
],
];
}

// function from v2.10.3
private static function transformPostgresTextArrayToPHPArray(string $postgresArray): array
{
$transform = static function (string $textArrayToTransform): array {
$indicatesMultipleDimensions = \mb_strpos($textArrayToTransform, '},{') !== false
|| \mb_strpos($textArrayToTransform, '{{') === 0;
if ($indicatesMultipleDimensions) {
throw new \InvalidArgumentException('Only single-dimensioned arrays are supported');
}

$phpArray = \str_getcsv(\trim($textArrayToTransform, '{}'), escape: '\\');
foreach ($phpArray as $i => $text) {
if ($text === null) {
unset($phpArray[$i]);

break;
}

$isInteger = \is_numeric($text) && ''.(int) $text === $text;
if ($isInteger) {
$phpArray[$i] = (int) $text;

continue;
}

$isFloat = \is_numeric($text) && ''.(float) $text === $text;
if ($isFloat) {
$phpArray[$i] = (float) $text;

continue;
}

$phpArray[$i] = \stripslashes(\str_replace('\"', '"', $text));
}

return $phpArray;
};

return $transform($postgresArray);
}
Comment on lines +263 to +302
Copy link

@coderabbitai coderabbitai bot May 25, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Critical issues in the legacy transformation method implementation.

The legacy method has several problems that will cause test failures and incorrect behavior:

  1. Logic error with null handling: The break statement on line 278 will terminate processing after the first element if any element is null, leaving subsequent elements unprocessed.

  2. Inconsistent exception type: Uses \InvalidArgumentException instead of InvalidArrayFormatException used elsewhere in the codebase.

  3. Missing type conversions: Doesn't handle boolean values (true, false, t, f) or null string conversion (NULL, null) like the V3 transformer does.

  4. Unnecessary complexity: The closure wrapper adds no value and makes the code harder to read.

Apply this diff to fix the critical issues:

-    // function from v2.10.3
-    private static function transformPostgresTextArrayToPHPArray(string $postgresArray): array
-    {
-        $transform = static function (string $textArrayToTransform): array {
-            $indicatesMultipleDimensions = \mb_strpos($textArrayToTransform, '},{') !== false
-                || \mb_strpos($textArrayToTransform, '{{') === 0;
-            if ($indicatesMultipleDimensions) {
-                throw new \InvalidArgumentException('Only single-dimensioned arrays are supported');
-            }
-
-            $phpArray = \str_getcsv(\trim($textArrayToTransform, '{}'), escape: '\\');
-            foreach ($phpArray as $i => $text) {
-                if ($text === null) {
-                    unset($phpArray[$i]);
-
-                    break;
-                }
-
-                $isInteger = \is_numeric($text) && ''.(int) $text === $text;
-                if ($isInteger) {
-                    $phpArray[$i] = (int) $text;
-
-                    continue;
-                }
-
-                $isFloat = \is_numeric($text) && ''.(float) $text === $text;
-                if ($isFloat) {
-                    $phpArray[$i] = (float) $text;
-
-                    continue;
-                }
-
-                $phpArray[$i] = \stripslashes(\str_replace('\"', '"', $text));
-            }
-
-            return $phpArray;
-        };
-
-        return $transform($postgresArray);
-    }
+    // function from v2.10.3
+    private static function transformPostgresTextArrayToPHPArray(string $postgresArray): array
+    {
+        $indicatesMultipleDimensions = \mb_strpos($postgresArray, '},{') !== false
+            || \mb_strpos($postgresArray, '{{') === 0;
+        if ($indicatesMultipleDimensions) {
+            throw new InvalidArrayFormatException('Only single-dimensioned arrays are supported');
+        }
+
+        $phpArray = \str_getcsv(\trim($postgresArray, '{}'), escape: '\\');
+        foreach ($phpArray as $i => $text) {
+            // Handle null values
+            if (\in_array(\strtolower($text), ['null', ''], true)) {
+                $phpArray[$i] = null;
+                continue;
+            }
+
+            // Handle boolean values
+            if (\in_array(\strtolower($text), ['true', 't'], true)) {
+                $phpArray[$i] = true;
+                continue;
+            }
+            if (\in_array(\strtolower($text), ['false', 'f'], true)) {
+                $phpArray[$i] = false;
+                continue;
+            }
+
+            $isInteger = \is_numeric($text) && ''.(int) $text === $text;
+            if ($isInteger) {
+                $phpArray[$i] = (int) $text;
+                continue;
+            }
+
+            $isFloat = \is_numeric($text) && ''.(float) $text === $text;
+            if ($isFloat) {
+                $phpArray[$i] = (float) $text;
+                continue;
+            }
+
+            $phpArray[$i] = \stripslashes(\str_replace('\"', '"', $text));
+        }
+
+        return $phpArray;
+    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// function from v2.10.3
private static function transformPostgresTextArrayToPHPArray(string $postgresArray): array
{
$transform = static function (string $textArrayToTransform): array {
$indicatesMultipleDimensions = \mb_strpos($textArrayToTransform, '},{') !== false
|| \mb_strpos($textArrayToTransform, '{{') === 0;
if ($indicatesMultipleDimensions) {
throw new \InvalidArgumentException('Only single-dimensioned arrays are supported');
}
$phpArray = \str_getcsv(\trim($textArrayToTransform, '{}'), escape: '\\');
foreach ($phpArray as $i => $text) {
if ($text === null) {
unset($phpArray[$i]);
break;
}
$isInteger = \is_numeric($text) && ''.(int) $text === $text;
if ($isInteger) {
$phpArray[$i] = (int) $text;
continue;
}
$isFloat = \is_numeric($text) && ''.(float) $text === $text;
if ($isFloat) {
$phpArray[$i] = (float) $text;
continue;
}
$phpArray[$i] = \stripslashes(\str_replace('\"', '"', $text));
}
return $phpArray;
};
return $transform($postgresArray);
}
// function from v2.10.3
private static function transformPostgresTextArrayToPHPArray(string $postgresArray): array
{
$indicatesMultipleDimensions = \mb_strpos($postgresArray, '},{') !== false
|| \mb_strpos($postgresArray, '{{') === 0;
if ($indicatesMultipleDimensions) {
throw new InvalidArrayFormatException('Only single-dimensioned arrays are supported');
}
$phpArray = \str_getcsv(\trim($postgresArray, '{}'), escape: '\\');
foreach ($phpArray as $i => $text) {
// Handle null values
if (\in_array(\strtolower($text), ['null', ''], true)) {
$phpArray[$i] = null;
continue;
}
// Handle boolean values
if (\in_array(\strtolower($text), ['true', 't'], true)) {
$phpArray[$i] = true;
continue;
}
if (\in_array(\strtolower($text), ['false', 'f'], true)) {
$phpArray[$i] = false;
continue;
}
$isInteger = \is_numeric($text) && ''.(int) $text === $text;
if ($isInteger) {
$phpArray[$i] = (int) $text;
continue;
}
$isFloat = \is_numeric($text) && ''.(float) $text === $text;
if ($isFloat) {
$phpArray[$i] = (float) $text;
continue;
}
$phpArray[$i] = \stripslashes(\str_replace('\"', '"', $text));
}
return $phpArray;
}
🤖 Prompt for AI Agents 
In tests/Unit/MartinGeorgiev/Utils/PostgresArrayToPHPArrayTransformerTest.php
between lines 263 and 302, fix the legacy transformPostgresTextArrayToPHPArray
method by removing the break statement that prematurely stops processing on null
elements, replace the \InvalidArgumentException with InvalidArrayFormatException
for consistency, add handling to convert boolean string values ('true', 'false',
't', 'f') and null strings ('NULL', 'null') to their proper PHP types, and
eliminate the unnecessary closure wrapper to simplify the code structure.

}
Loading