ready for release, 100% CC

PhilippGrashoff · Aug 13, 2023 · 11669e0 · 11669e0
1 parent 28547fc
commit 11669e0
Show file tree

Hide file tree

Showing 7 changed files with 90 additions and 42 deletions.
diff --git a/composer.json b/composer.json
@@ -13,7 +13,7 @@
     "philippgrashoff/atkextendedtestcase": "4.*"
   },
   "require-dev": {
-    "phpunit/phpunit": "^9.5.5",
+    "phpunit/phpunit": "^9.5.25",
     "phpstan/phpstan": "1.*"
   },
   "autoload": {

diff --git a/composer.lock b/composer.lock
diff --git a/src/CreatedDateAndLastUpdatedTrait.php b/src/CreatedDateAndLastUpdatedTrait.php
@@ -3,6 +3,7 @@
 namespace atkdatamodeltraits;
 
 use Atk4\Data\Model;
+use DateTime;
 
 /**
  * @extends Model<Model>
@@ -13,7 +14,7 @@ trait CreatedDateAndLastUpdatedTrait
     /**
      * Adds created_date and created_by fields to a model
      *
-     * @param array $additionalFieldSettings
+     * @param array<string, mixed> $additionalFieldSettings
      * @return void
      */
     protected function addCreatedDateAndLastUpdateFields(array $additionalFieldSettings = []): void
@@ -49,13 +50,13 @@ function (self $model, array &$data) {
                 if ($data['created_date']) {
                     return;
                 }
-                $data['created_date'] = new \DateTime();
+                $data['created_date'] = new DateTime();
             }
         );
         $this->onHook(
             Model::HOOK_BEFORE_UPDATE,
             function (self $model, array &$data) {
-                $data['last_updated'] = new \DateTime();
+                $data['last_updated'] = new DateTime();
             }
         );
     }

diff --git a/src/CryptIdTrait.php b/src/CryptIdTrait.php
@@ -15,11 +15,8 @@ trait CryptIdTrait
 
     use UniqueFieldTrait;
 
-    /** @var string */
-    protected string $cryptIdFieldName = 'crypt_id';
-
     /** @var array<string>
-     * Chars I, l, O, 0 as they can be easily mixed up by humans
+     * Chars I, l, O, 0 are removed as they can be easily mixed up by humans
      */
     protected array $possibleChars = [
         '1',
@@ -82,32 +79,54 @@ trait CryptIdTrait
         'Z',
     ];
 
-    /**
-     * sets a cryptic Id to the fieldName passed. Only does something if the field is empty.
-     *
-     * @return void
-     * @throws Exception
-     * @throws \Atk4\Core\Exception
-     */
-    public function setCryptId(): void
+    protected function addCryptIdFieldAndHooks(string $fieldName): static
     {
-        if (!$this->get($this->cryptIdFieldName)) {
-            $this->set($this->cryptIdFieldName, $this->generateCryptId());
-            //check if another Record has the same crypt_id, if so generate a new one
-            while (!$this->isFieldUnique($this->cryptIdFieldName)) {
-                $this->set($this->cryptIdFieldName, $this->generateCryptId());
+        $this->addField(
+            $fieldName,
+            [
+                'type' => 'string',
+                'system' => true,
+                'required' => true
+            ]
+        );
+
+        $this->onHook(
+            Model::HOOK_BEFORE_INSERT,
+            function (self $entity, array &$data) use ($fieldName) {
+                if ($this->get($fieldName) === null) { //leave option to manually set crypt ID, e.g. for imports
+                    $data[$fieldName] = $entity->setCryptId($fieldName);
+                }
             }
-        } else {
-            $this->getField($this->cryptIdFieldName)->readOnly = true;
-        }
+        );
+
+
+        $this->onHook(
+            Model::HOOK_AFTER_LOAD,
+            function (self $entity) use ($fieldName) {
+                $entity->getField($fieldName)->readOnly = true;
+            }
+        );
+
+        return $this;
     }
 
     /**
+     * sets a cryptic ID to the fieldName passed. Only does something if the field is empty.
+     * Needs to return the generated crypt ID so it can be used in Model::HOOK_BEFORE_INSERT
+     *
+     * @param string $fieldName
      * @return string
+     * @throws Exception
+     * @throws \Atk4\Core\Exception
      */
-    public function getCryptId(): string
+    protected function setCryptId(string $fieldName): string
     {
-        return $this->get($this->cryptIdFieldName);
+        $this->set($fieldName, $this->generateCryptId());
+        //check if another Record has the same crypt_id, if so generate a new one
+        while (!$this->isFieldUnique($fieldName)) {
+            $this->set($fieldName, $this->generateCryptId());
+        }
+        return $this->get($fieldName);
     }
 
     /**

diff --git a/src/UniqueFieldTrait.php b/src/UniqueFieldTrait.php
@@ -31,8 +31,9 @@ trait UniqueFieldTrait
      * @return bool
      * @throws Exception
      */
-    public function isFieldUnique(string $fieldName,): bool
+    public function isFieldUnique(string $fieldName): bool
     {
+        $this->assertIsEntity();
         if (empty($this->get($fieldName))) {
             throw new Exception(
                 'The value for a unique field may not be empty. Field name: ' . $fieldName . ' in ' . __FUNCTION__
@@ -41,8 +42,10 @@ public function isFieldUnique(string $fieldName,): bool
         $checkModel = new static($this->getPersistence());
         //only load ID field to save performance
         $checkModel->setOnlyFields([$this->idField, $fieldName]);
-        $checkModel->addCondition($this->idField, '!=', $this->get($this->idField));
         $checkModel->addCondition($fieldName, '=', $this->get($fieldName));
+        if ($this->isLoaded()) {
+            $checkModel->addCondition($this->idField, '!=', $this->get($this->idField));
+        }
 
         return $checkModel->tryLoadAny() === null;
     }

diff --git a/tests/CryptIdTraitTest.php b/tests/CryptIdTraitTest.php
@@ -2,7 +2,6 @@
 
 namespace atkdatamodeltraits\tests;
 
-use Atk4\Data\Exception;
 use Atk4\Data\Model;
 use Atk4\Data\Persistence;
 use atkdatamodeltraits\CryptIdTrait;
@@ -31,19 +30,43 @@ public function testExceptionOverwriteGenerate(): void
     public function testsetCryptId(): void
     {
         $entity = (new ModelWithCryptIdTrait($this->getSqliteTestPersistence()))->createEntity();
-        $entity->setCryptId();
+        $entity->save();
         self::assertSame(
             12,
             strlen($entity->get('crypt_id'))
         );
     }
 
-
     public function testFieldSetToReadOnlyIfCryptIdNotEmpty(): void
     {
         $entity = (new ModelWithCryptIdTrait($this->getSqliteTestPersistence()))->createEntity();
-        $entity->setCryptId();
-        $entity->save();
+        $entity->save(); //save automatically reloads by default
         self::assertTrue($entity->getField('crypt_id')->readOnly);
     }
+
+    public function testNewCryptIdIsGeneratedIfGeneratedOneAlreadyExists(): void
+    {
+        $persistence = $this->getSqliteTestPersistence();
+        $entity = (new ModelWithCryptIdTrait(
+            $persistence,
+            ['generateStaticCryptIdOnFirstRun' => true]
+        ))->createEntity();
+        $entity->save();
+        self::assertSame(
+            'abcdefghijkl',
+            $entity->get('crypt_id')
+        );
+
+        //this entity will create the very same ID on the first run of setCryptId(), thus the corresponding line
+        //within setCryptId is executed
+        $entity2 = (new ModelWithCryptIdTrait(
+            $persistence,
+            ['generateStaticCryptIdOnFirstRun' => true]
+        ))->createEntity();
+        $entity2->save();
+        self::assertNotSame(
+            'abcdefghijkl',
+            $entity2->get('crypt_id')
+        );
+    }
 }
diff --git a/tests/testclasses/ModelWithCryptIdTrait.php b/tests/testclasses/ModelWithCryptIdTrait.php
@@ -11,24 +11,26 @@ class ModelWithCryptIdTrait extends Model
 {
     use CryptIdTrait;
 
+    protected bool $generateStaticCryptIdOnFirstRun = false;
+
+    protected int $runCounter = 0;
+
     public $table = 'ModelWithCryptIdTrait';
 
     protected function init(): void
     {
         parent::init();
-        $this->addField('crypt_id');
-        $this->onHook(
-            Model::HOOK_BEFORE_SAVE,
-            function (self $model, bool $isUpdate) {
-                $model->setCryptId();
-            }
-        );
+        $this->addCryptIdFieldAndHooks('crypt_id');
     }
 
     protected function generateCryptId(): string
     {
         $cryptId = '';
-        for ($i = 0; $i < 10; $i++) {
+        if ($this->generateStaticCryptIdOnFirstRun && $this->runCounter === 0) {
+            $this->runCounter++;
+            return "abcdefghijkl";
+        }
+        for ($i = 0; $i < 12; $i++) {
             $cryptId .= $this->getRandomChar();
         }