[CORE][Entity] Rename createOrUpdate to 'checkExistingAndCreateOrUpdate', remove update feature from 'create' and add 'createOrUpdate' and fix users

components/Circle/Controller/SelfTagsSettings.php

@@ -45,7 +45,7 @@ class SelfTagsSettings extends Controller
                 foreach ($tags as $tag) {
                     $tag = CompTag::sanitize($tag);
 
-                    [$actor_tag, $actor_tag_existed] = ActorTag::createOrUpdate([
+                    [$actor_tag, $actor_tag_existed] = ActorTag::checkExistingAndCreateOrUpdate([
                         'tagger' => $target->getId(), // self tag means tagger = tagger in ActorTag
                         'tagged' => $target->getId(),
                         'tag'    => $tag,

components/Link/Entity/NoteToLink.php

@@ -84,16 +84,14 @@ class NoteToLink extends Entity
      * Create an instance of NoteToLink or fill in the
      * properties of $obj with the associative array $args. Doesn't
      * persist the result
-     *
-     * @param null|mixed $obj
      */
-    public static function create(array $args, $obj = null)
+    public static function create(array $args, bool $_delegated_call = false): static
     {
         $link = DB::find('link', ['id' => $args['link_id']]);
         $note = DB::find('note', ['id' => $args['note_id']]);
         Event::handle('NewLinkFromNote', [$link, $note]);
         $obj = new self();
-        return parent::create($args, $obj);
+        return parent::createOrUpdate($args, $obj);
     }
 
     public static function removeWhereNoteId(int $note_id): mixed

components/Person/Controller/PersonSettings.php

@@ -286,7 +286,7 @@ class PersonSettings extends Controller
                 $data = $form->getData();
                 unset($data['translation_domain']);
                 try {
-                    [$entity, $is_update] = UserNotificationPrefs::createOrUpdate(
+                    [$entity, $is_update] = UserNotificationPrefs::checkExistingAndCreateOrUpdate(
                         array_merge(['user_id' => $user->getId(), 'transport' => $transport_name], $data),
                         find_by_keys: ['user_id', 'transport'],
                     );

src/Core/Entity.php

@@ -23,8 +23,8 @@ declare(strict_types = 1);
 
 namespace App\Core;
 
-use App\Core\DB;
 use App\Entity\Actor;
+use App\Util\Exception\BugFoundException;
 use App\Util\Exception\NotFoundException;
 use App\Util\Formatting;
 use BadMethodCallException;
@@ -33,10 +33,11 @@ use DateTime;
 use DateTimeInterface;
 use Exception;
 use Functional as F;
-use InvalidArgumentException;
 
 /**
  * Base class to all entities, with some utilities
+ *
+ * @method int getId() // Not strictly true
  */
 abstract class Entity
 {
@@ -67,22 +68,39 @@ abstract class Entity
      * properties of $obj with the associative array $args. Doesn't
      * persist the result
      */
-    public static function create(array $args, mixed $obj = null)
+    public static function create(array $args, bool $_delegated_call = false): static
     {
+        $date  = new DateTime();
         $class = static::class;
+        $obj   = new $class();
+        foreach (['created', 'modified'] as $prop) {
+            if (property_exists($class, $prop) && !isset($args[$prop])) {
+                $args[$prop] = $date;
+            }
+        }
 
-        $date = new DateTime();
+        if (!$_delegated_call) {
-        if (!\is_null($obj)) { // Update modified
+            return static::createOrUpdate($obj, $args, _delegated_call: true);
-            if (property_exists($class, 'modified')) {
+        } else {
+            return $obj;
+        }
+    }
+
+    /**
+     * @param ?static $obj
+     */
+    public static function createOrUpdate(?self $obj, array $args, bool $_delegated_call = false): static
+    {
+        $date  = new DateTime();
+        $class = static::class;
+        if (!$_delegated_call) {
+            if (property_exists($class, 'modified') && !isset($args['modified'])) {
                 $args['modified'] = $date;
             }
-        } else {
+        }
-            $obj = new $class();
+
-            foreach (['created', 'modified'] as $prop) {
+        if (\is_null($obj)) {
-                if (property_exists($class, $prop)) {
+            $obj = static::create($args, _delegated_call: true);
-                    $args[$prop] = $date;
-                }
-            }
         }
 
         foreach ($args as $prop => $val) {
@@ -90,8 +108,7 @@ abstract class Entity
                 $set = 'set' . ucfirst(Formatting::snakeCaseToCamelCase($prop));
                 $obj->{$set}($val);
             } else {
-                Log::error($m = "Property {$class}::{$prop} doesn't exist");
+                throw new BugFoundException("Property {$class}::{$prop} doesn't exist");
-                throw new InvalidArgumentException($m);
             }
         }
 
@@ -105,12 +122,11 @@ abstract class Entity
      *
      * @return array [$obj, $is_update]
      */
-    public static function createOrUpdate(array $args, array $find_by_keys = []): array
+    public static function checkExistingAndCreateOrUpdate(array $args, array $find_by_keys = []): array
     {
-        $table   = DB::getTableForClass(static::class);
         $find_by = $find_by_keys === [] ? $args : array_intersect_key($args, array_flip($find_by_keys));
         try {
-            $obj = DB::findOneBy($table, $find_by);
+            $obj = DB::findOneBy(static::class, $find_by, return_null: false);
         } catch (NotFoundException) {
             $obj = null;
             // @codeCoverageIgnoreStart
@@ -118,8 +134,8 @@ abstract class Entity
             Log::unexpected_exception($e);
             // @codeCoverageIgnoreEnd
         }
-        $is_update = $obj !== null;
+        $is_update = !\is_null($obj);
-        return [self::create($args, $obj), $is_update];
+        return [self::createOrUpdate($obj, $args), $is_update];
     }
 
     /**

tests/Core/EntityTest.php

@@ -51,12 +51,12 @@ class EntityTest extends GNUsocialTestCase
 
     public function testCreateOrUpdate()
     {
-        [$user, $is_update] = LocalUser::createOrUpdate(['nickname' => 'taken_user']);
+        [$user, $is_update] = LocalUser::checkExistingAndCreateOrUpdate(['nickname' => 'taken_user']);
         static::assertNotNull($user);
         static::assertTrue($is_update);
-        [, $is_update] = LocalUser::createOrUpdate(['nickname' => 'taken_user', 'outgoing_email' => 'foo@bar']);
+        [, $is_update] = LocalUser::checkExistingAndCreateOrUpdate(['nickname' => 'taken_user', 'outgoing_email' => 'foo@bar']);
         static::assertFalse($is_update);
-        [$user, $is_update] = LocalUser::createOrUpdate(['nickname' => 'taken_user', 'outgoing_email' => 'foo@bar'], find_by_keys: ['nickname']);
+        [$user, $is_update] = LocalUser::checkExistingAndCreateOrUpdate(['nickname' => 'taken_user', 'outgoing_email' => 'foo@bar'], find_by_keys: ['nickname']);
         static::assertSame('foo@bar', $user->getOutgoingEmail());
         static::assertTrue($is_update);
     }