From dac94f53cdab634ed4c4869a65298ec4e9798357 Mon Sep 17 00:00:00 2001 From: Hugo Sales Date: Mon, 28 Mar 2022 03:15:39 +0100 Subject: [PATCH] [CORE][Entity] Rename createOrUpdate to 'checkExistingAndCreateOrUpdate', remove update feature from 'create' and add 'createOrUpdate' and fix users --- .../Circle/Controller/SelfTagsSettings.php | 2 +- components/Link/Entity/NoteToLink.php | 6 +- .../Person/Controller/PersonSettings.php | 2 +- src/Core/Entity.php | 56 ++++++++++++------- tests/Core/EntityTest.php | 6 +- 5 files changed, 43 insertions(+), 29 deletions(-) diff --git a/components/Circle/Controller/SelfTagsSettings.php b/components/Circle/Controller/SelfTagsSettings.php index 23b9584f49..28a64b155a 100644 --- a/components/Circle/Controller/SelfTagsSettings.php +++ b/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, diff --git a/components/Link/Entity/NoteToLink.php b/components/Link/Entity/NoteToLink.php index d55f62b1cb..5e3274aad7 100644 --- a/components/Link/Entity/NoteToLink.php +++ b/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 diff --git a/components/Person/Controller/PersonSettings.php b/components/Person/Controller/PersonSettings.php index 09592dc26e..7a4f9ee26d 100644 --- a/components/Person/Controller/PersonSettings.php +++ b/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'], ); diff --git a/src/Core/Entity.php b/src/Core/Entity.php index eaa89c3d2e..15fa447a2f 100644 --- a/src/Core/Entity.php +++ b/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 (!\is_null($obj)) { // Update modified - if (property_exists($class, 'modified')) { + if (!$_delegated_call) { + return static::createOrUpdate($obj, $args, _delegated_call: true); + } 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 (property_exists($class, $prop)) { - $args[$prop] = $date; - } - } + } + + if (\is_null($obj)) { + $obj = static::create($args, _delegated_call: true); } 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 InvalidArgumentException($m); + throw new BugFoundException("Property {$class}::{$prop} doesn't exist"); } } @@ -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; - return [self::create($args, $obj), $is_update]; + $is_update = !\is_null($obj); + return [self::createOrUpdate($obj, $args), $is_update]; } /** diff --git a/tests/Core/EntityTest.php b/tests/Core/EntityTest.php index 5f9c88a6e5..62b68b0428 100644 --- a/tests/Core/EntityTest.php +++ b/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); }