Merge pull request #4 from mringler/bugfix/convert_uuid_in_pk_on_update

Convert UUID_BINARY in PK during update
mringler · Nov 28, 2023 · 32161c1 · 32161c1
2 parents 2d87bfa + 6b29c6d
commit 32161c1
Show file tree

Hide file tree

Showing 5 changed files with 55 additions and 31 deletions.
diff --git a/src/Propel/Generator/Builder/Om/ObjectBuilder.php b/src/Propel/Generator/Builder/Om/ObjectBuilder.php
@@ -1723,7 +1723,7 @@ protected function addLazyLoaderBody(string &$script, Column $column): void
             if (is_resource(\$firstColumn)) {
                 \$firstColumn = stream_get_contents(\$firstColumn);
             }
-            \$this->$clo = (\$firstColumn) ? UuidConverter::binToUuid(\$firstColumn, $uuidSwapFlag) : null;";
+            \$this->$clo = UuidConverter::binToUuid(\$firstColumn, $uuidSwapFlag);";
         } else {
             $script .= "
             \$this->$clo = \$firstColumn;";
@@ -2737,7 +2737,7 @@ protected function addHydrateBody(string &$script): void
             if (is_resource(\$col)) {
                 \$col = stream_get_contents(\$col);
             }
-            \$this->$clo = (\$col) ? UuidConverter::binToUuid(\$col, $uuidSwapFlag) : null;";
+            \$this->$clo = UuidConverter::binToUuid(\$col, $uuidSwapFlag);";
                 } elseif ($col->isPhpPrimitiveType()) {
                     $script .= "
             \$this->$clo = (null !== \$col) ? (" . $col->getPhpType() . ') $col : null;';
@@ -2882,10 +2882,15 @@ protected function addBuildPkeyCriteriaBody(string &$script): void
 
         $script .= "
         \$criteria = " . $this->getQueryClassName() . '::create();';
-        foreach ($this->getTable()->getPrimaryKey() as $col) {
-            $clo = $col->getLowercasedName();
+        foreach ($this->getTable()->getPrimaryKey() as $column) {
+            $dataAccessExpression = '$this->' . $column->getLowercasedName();
+            if ($column->getType() === PropelTypes::UUID_BINARY) {
+                $uuidSwapFlag = $this->getUuidSwapFlagLiteral();
+                $dataAccessExpression = "UuidConverter::uuidToBin($dataAccessExpression, $uuidSwapFlag)";
+            }
+            $columnConstant = $this->getColumnConstant($column);
             $script .= "
-        \$criteria->add(" . $this->getColumnConstant($col) . ", \$this->$clo);";
+        \$criteria->add($columnConstant, $dataAccessExpression);";
         }
     }
 
@@ -6689,7 +6694,7 @@ protected function getAccessValueStatement(Column $column): string
         if ($column->isUuidBinaryType()) {
             $uuidSwapFlag = $this->getUuidSwapFlagLiteral();
 
-            return "(\$this->$columnName) ? UuidConverter::uuidToBin(\$this->$columnName, $uuidSwapFlag) : null";
+            return "UuidConverter::uuidToBin(\$this->$columnName, $uuidSwapFlag)";
         }
 
         return "\$this->$columnName";

diff --git a/src/Propel/Runtime/Util/UuidConverter.php b/src/Propel/Runtime/Util/UuidConverter.php
@@ -16,13 +16,16 @@ class UuidConverter
     /**
      * Transforms a UUID string to a binary string.
      *
-     * @param string $uuid
+     * @param string|null $uuid
      * @param bool $swapFlag Swap first four bytes for better indexing of version-1 UUIDs (@link https://dev.mysql.com/doc/refman/8.0/en/miscellaneous-functions.html#function_uuid-to-bin)
      *
-     * @return string
+     * @return string|null
      */
-    public static function uuidToBin(string $uuid, bool $swapFlag = true): string
+    public static function uuidToBin(?string $uuid, bool $swapFlag = true): ?string
     {
+        if (!$uuid) {
+            return null;
+        }
         $rawHex = (!$swapFlag)
             ? str_replace('-', '', $uuid)
             : preg_replace(
@@ -37,13 +40,16 @@ public static function uuidToBin(string $uuid, bool $swapFlag = true): string
     /**
      * Transforms a binary string to a UUID string.
      *
-     * @param string $bin
+     * @param string|null $bin
      * @param bool $swapFlag Assume bytes were swapped (@link https://dev.mysql.com/doc/refman/8.0/en/miscellaneous-functions.html#function_bin-to-uuid)
      *
-     * @return string
+     * @return string|null
      */
-    public static function binToUuid(string $bin, bool $swapFlag = true): string
+    public static function binToUuid(?string $bin, bool $swapFlag = true): ?string
     {
+        if (!$bin) {
+            return null;
+        }
         $rawHex = bin2hex($bin);
         $recombineFormat = $swapFlag ? '$3$4-$2-$1-$5-$6' : '$1$2-$3-$4-$5-$6';
 

diff --git a/tests/Fixtures/bookstore/schema.xml b/tests/Fixtures/bookstore/schema.xml
@@ -354,6 +354,15 @@
         </index>
     </table>
 
+
+    <table name="book_uuid_binary" phpName="BookUuidBinary">
+        <column name="id" phpName="Id" type="UUID_BINARY" primaryKey="true" required="true"/>
+        <column name="title" type="VARCHAR"/>
+        <vendor type="mysql">
+        <parameter name="Engine" value="InnoDB"/>
+        </vendor>
+    </table>
+
     <!-- Test single table inheritance with Abstract true -->
     <table name="distribution" abstract="true">
         <column name="id" type="INTEGER" primaryKey="true" autoIncrement="true"/>

diff --git a/tests/Propel/Tests/Runtime/TypeTests/UuidBinaryTypeTest.php b/tests/Propel/Tests/Runtime/TypeTests/UuidBinaryTypeTest.php
@@ -9,8 +9,10 @@
 namespace Propel\Tests\Runtime\TypeTest;
 
 use Propel\Runtime\Util\UuidConverter;
-use Propel\Tests\Bookstore\Base\Book2Query;
 use Propel\Tests\Bookstore\Book2;
+use Propel\Tests\Bookstore\Book2Query;
+use Propel\Tests\Bookstore\BookUuidBinary;
+use Propel\Tests\Bookstore\BookUuidBinaryQuery;
 use Propel\Tests\Bookstore\Map\Book2TableMap;
 use Propel\Tests\Helpers\Bookstore\BookstoreTestBase;
 
@@ -123,4 +125,21 @@ public function testModelCanUpdateUuid()
 
         $this->assertSame($updateUuid, $book->getUuidBin());
     }
+
+    /**
+     * @return void
+     */
+    public function testModelCanUpdateUuidBinaryPk()
+    {
+        BookUuidBinaryQuery::create()->deleteAll();
+        $uuid = 'b41a29db-cf78-4d43-83a9-4cd3e1e1b41a';
+        $book = new BookUuidBinary();
+        $book->setId($uuid)->setTitle('First Title')->save();
+
+        $updatedTitle = 'Second Title';
+        $book->setTitle($updatedTitle)->save();
+        $book->reload();
+
+        $this->assertSame($updatedTitle, $book->getTitle());
+    }
 }
diff --git a/tests/Propel/Tests/Runtime/Util/UuidConverterTest.php b/tests/Propel/Tests/Runtime/Util/UuidConverterTest.php
@@ -19,6 +19,7 @@ public function uuidDataProvider(): array
             // uuid, hex, hexWithSwap
             ['11112222-3333-4444-5555-666677778888', '11112222333344445555666677778888', '44443333111122225555666677778888'],
             ['aab5d5fd-70c1-11e5-a4fb-b026b977eb28', 'aab5d5fd70c111e5a4fbb026b977eb28', '11e570c1aab5d5fda4fbb026b977eb28'],
+            [null, null, null],
         ]; 
     }
 
@@ -44,9 +45,9 @@ public function testUuidToBinWithoutSwap($uuid, $hex, $hexWithSwap)
 
     /**
      */
-    public function assertBinaryEquals(string $expected, $result)
+    public function assertBinaryEquals(?string $expected, ?string $result)
     {
-        $expected = hex2bin($expected);
+        $expected = $expected ? hex2bin($expected) : $expected;
         $this->assertEquals($expected, $result);
     }
 
@@ -71,20 +72,4 @@ public function testBinToUuidWithoutSwap($uuid, $hex, $hexWithSwap)
         $result = UuidConverter::binToUuid($bin, false);
         $this->assertEquals($uuid, $result);
     }
-
-    public function testFasterUuidToBin(){
-        $this->markTestSkipped();
-        $uuid = [];
-
-        for($i = 0; $i < 100000; $i++){
-            $uuids[] = $this->guidv4();
-        }
-        $swapFlag = true;
-        $regexDuration =  $this->measure([UuidConverter::class, 'uuidToBinRegex'], $uuids, $swapFlag);
-        $regularDuration = $this->measure([UuidConverter::class, 'uuidToBin'], $uuids, $swapFlag);
-
-        echo "regular took $regularDuration, regex took $regexDuration";
-        $this->assertLessThanOrEqual($regexDuration, $regularDuration, "regular took $regularDuration, regex took $regexDuration");
-    }
-
 }