Fix: #5100, fix: #4828 - _UID fields have wrong checksum. Do not auto…

…matically create new when editing.
fisharebest · Jan 9, 2025 · 328ab05 · 328ab05
1 parent 718dc3f
commit 328ab05
Show file tree

Hide file tree

Showing 7 changed files with 83 additions and 46 deletions.
diff --git a/app/Elements/PafUid.php b/app/Elements/PafUid.php
@@ -19,29 +19,45 @@
 
 namespace Fisharebest\Webtrees\Elements;
 
+use Fisharebest\Webtrees\I18N;
 use Fisharebest\Webtrees\Registry;
 use Fisharebest\Webtrees\Tree;
 
+use function strtoupper;
+
 /**
  * _UID fields, as created by PAF and other applications
  */
 class PafUid extends AbstractElement
 {
-    protected const MAXIMUM_LENGTH = 34;
-
-    /**
-     * Create a default value for this element.
-     *
-     * @param Tree $tree
-     *
-     * @return string
-     */
-    public function default(Tree $tree): string
+    protected const MAXIMUM_LENGTH = 36;
+
+    public function canonical(string $value): string
     {
-        if ($tree->getPreference('GENERATE_UIDS') === '1') {
-            return Registry::idFactory()->pafUid();
+        $value = parent::canonical($value);
+
+        if (preg_match('/([0-9a-f]{8})-?([0-9a-f]{4})-?([0-9a-f]{4})-?([0-9a-f]{4})-?([0-9a-f]{12})/i', $value, $match) === 1) {
+            $value = strtoupper($match[1] . $match[2] . $match [3] . $match[4] . $match[5]);
+
+            return $value . Registry::idFactory()->pafUidChecksum($value);
         }
 
-        return '';
+        return Registry::idFactory()->pafUid();
+    }
+
+    public function edit(string $id, string $name, string $value, Tree $tree): string
+    {
+        return
+            '<div class="input-group mb-3">' .
+            parent::edit($id, $name, $value, $tree) .
+            '<button type="button" class="input-group-text btn btn-primary" id="create-' . e($id) . '">' .
+            I18N::translate('create') .
+            '</button>' .
+            '</div>' .
+            '<script>' .
+            'document.getElementById("create-' . e($id) . '").addEventListener("click", function(event) {' .
+            ' document.getElementById("' . e($id) . '").value="' . e(Registry::idFactory()->pafUid()) . '";' .
+            '})' .
+            '</script>';
     }
 }
diff --git a/app/Elements/Uid.php b/app/Elements/Uid.php
@@ -19,29 +19,46 @@
 
 namespace Fisharebest\Webtrees\Elements;
 
+use Fisharebest\Webtrees\I18N;
 use Fisharebest\Webtrees\Registry;
 use Fisharebest\Webtrees\Tree;
 
+use function e;
+use function preg_match;
+use function strtolower;
+
 /**
  * UID fields
  */
 class Uid extends AbstractElement
 {
     protected const MAXIMUM_LENGTH = 36;
 
-    /**
-     * Create a default value for this element.
-     *
-     * @param Tree $tree
-     *
-     * @return string
-     */
-    public function default(Tree $tree): string
+    public function canonical(string $value): string
     {
-        if ($tree->getPreference('GENERATE_UIDS') === '1') {
-            return Registry::idFactory()->uuid();
+        $value = strtolower(parent::canonical($value));
+
+        if (preg_match('/([0-9a-f]{8})-?([0-9a-f]{4})-?([0-9a-f]{4})-?([0-9a-f]{4})-?([0-9a-f]{12})/', $value, $match) === 1) {
+            return $match[1] . '-' . $match[2] . '-' . $match [3] . '-' . $match[4] . '-' . $match[5];
         }
 
-        return '';
+        return Registry::idFactory()->uuid();
+    }
+
+
+    public function edit(string $id, string $name, string $value, Tree $tree): string
+    {
+        return
+            '<div class="input-group mb-3">' .
+            parent::edit($id, $name, $value, $tree) .
+            '<button type="button" class="input-group-text btn btn-primary" id="create-' . e($id) . '">' .
+            I18N::translate('create') .
+            '</button>' .
+            '</div>' .
+            '<script>' .
+            'document.getElementById("create-' . e($id) . '").addEventListener("click", function(event) {' .
+            ' document.getElementById("' . e($id) . '").value="' . e(Registry::idFactory()->uuid()) . '";' .
+            '})' .
+            '</script>';
     }
 }
diff --git a/app/Factories/IdFactory.php b/app/Factories/IdFactory.php
@@ -25,7 +25,9 @@
 
 use function dechex;
 use function hexdec;
+use function sprintf;
 use function str_pad;
+use function str_split;
 use function strtoupper;
 use function substr;
 
@@ -42,7 +44,7 @@ class IdFactory implements IdFactoryInterface
     public function uuid(): string
     {
         try {
-            return strtolower(strtr(Uuid::uuid4()->toString(), ['-' => '']));
+            return strtolower(Uuid::uuid4()->toString());
         } catch (RandomSourceException $ex) {
             // uuid4() can fail if there is insufficient entropy in the system.
             return '';
@@ -78,23 +80,18 @@ public function pafUid(): string
     }
 
     /**
-     * @param string $uid exactly 32 hex characters
-     *
-     * @return string
+     * Based on the C implementation in "GEDCOM Unique Identifiers" by Gordon Clarke, dated 2007-06-08
      */
     public function pafUidChecksum(string $uid): string
     {
         $checksum_a = 0; // a sum of the bytes
         $checksum_b = 0; // a sum of the incremental values of $checksum_a
 
-        for ($i = 0; $i < 32; $i += 2) {
-            $checksum_a += hexdec(substr($uid, $i, 2));
-            $checksum_b += $checksum_a & 0xff;
+        foreach (str_split($uid, 2) as $byte) {
+            $checksum_a += hexdec($byte);
+            $checksum_b += $checksum_a;
         }
 
-        $digit1 = str_pad(dechex($checksum_a), 2, '0', STR_PAD_LEFT);
-        $digit2 = str_pad(dechex($checksum_b), 2, '0', STR_PAD_LEFT);
-
-        return strtoupper($digit1 . $digit2);
+        return sprintf('%02X%02X', $checksum_a & 0xff, $checksum_b & 0xff);
     }
 }
diff --git a/app/Services/GedcomImportService.php b/app/Services/GedcomImportService.php
@@ -278,10 +278,7 @@ public function importRecord(string $gedrec, Tree $tree, bool $update): void
 
         // Add a _UID
         if ($tree->getPreference('GENERATE_UIDS') === '1' && !str_contains($gedrec, "\n1 _UID ")) {
-            $element = Registry::elementFactory()->make($type . ':_UID');
-            if (!$element instanceof UnknownElement) {
-                $gedrec .= "\n1 _UID " . $element->default($tree);
-            }
+            $gedrec .= "\n1 _UID " . Registry::idFactory()->pafUid();
         }
 
         // If the user has downloaded their GEDCOM data (containing media objects) and edited it

diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
@@ -654,12 +654,6 @@ parameters:
 			count: 1
 			path: app/Factories/HeaderFactory.php
 
-		-
-			message: '#^Parameter \#1 \$num of function dechex expects int, float\|int given\.$#'
-			identifier: argument.type
-			count: 1
-			path: app/Factories/IdFactory.php
-
 		-
 			message: '#^Argument of an invalid type mixed supplied for foreach, only iterables are supported\.$#'
 			identifier: foreach.nonIterable

diff --git a/tests/app/Elements/PafUidTest.php b/tests/app/Elements/PafUidTest.php
@@ -36,4 +36,12 @@ public static function setupBeforeClass(): void
 
         self::$element = new PafUid('label');
     }
+
+    public function testCanonical(): void
+    {
+        self::assertSame(
+            'FEF44CA3CA7543ED9A05F00591315274D810',
+            self::$element->canonical('fef44CA3CA7543ED9A05F005913152740000'),
+        );
+    }
 }
diff --git a/tests/app/Elements/UidTest.php b/tests/app/Elements/UidTest.php
@@ -34,6 +34,14 @@ public static function setupBeforeClass(): void
     {
         parent::setUpBeforeClass();
 
-        self::$element = new PafUid('label');
+        self::$element = new Uid('label');
+    }
+
+    public function testCanonical(): void
+    {
+        self::assertSame(
+            'fef44ca3-ca75-43ed-9a05-f00591315274',
+            self::$element->canonical('FEF44ca3ca7543ed9a05f00591315274'),
+        );
     }
 }