[COMPONENT][Attachment][TESTS] Fix Entity/AttachmentThumbnailTest

components/Attachment/Entity/Attachment.php

@@ -224,8 +224,7 @@ class Attachment extends Entity
                     $this->setFilename(null);
                     $this->setSize(null);
                     // Important not to null neither width nor height
-                    DB::persist($this);
+                    DB::wrapInTransaction(fn () => DB::persist($this));
-                    DB::flush();
                 }
             } else {
                 // @codeCoverageIgnoreStart
@@ -340,7 +339,11 @@ class Attachment extends Entity
      */
     public function getThumbnails()
     {
-        return DB::findBy('attachment_thumbnail', ['attachment_id' => $this->id]);
+        return DB::findBy(
+            AttachmentThumbnail::class,
+            ['attachment_id' => $this->id],
+            order_by: ['size' => 'ASC', 'mimetype' => 'ASC'],
+        );
     }
 
     public function getPath()
@@ -378,7 +381,7 @@ class Attachment extends Entity
         }
     }
 
-    public function getThumbnailUrl(Note|int $note, ?string $size = null)
+    public function getThumbnailUrl(Note|int $note, ?string $size = null): string
     {
         return Router::url('note_attachment_thumbnail', ['note_id' => \is_int($note) ? $note : $note->getId(), 'attachment_id' => $this->getId(), 'size' => $size ?? Common::config('thumbnail', 'default_size')]);
     }

components/Attachment/Entity/AttachmentThumbnail.php

@@ -30,6 +30,7 @@ use App\Core\Event;
 use App\Core\GSFile;
 use App\Core\Log;
 use App\Core\Router\Router;
+use App\Entity\Note;
 use App\Util\Common;
 use App\Util\Exception\ClientException;
 use App\Util\Exception\NotFoundException;
@@ -179,7 +180,7 @@ class AttachmentThumbnail extends Entity
         if (isset($this->attachment) && !\is_null($this->attachment)) {
             return $this->attachment;
         } else {
-            return $this->attachment = DB::findOneBy('attachment', ['id' => $this->attachment_id]);
+            return $this->attachment = DB::findOneBy(Attachment::class, ['id' => $this->attachment_id]);
         }
     }
 
@@ -253,14 +254,14 @@ class AttachmentThumbnail extends Entity
         }
     }
 
-    public function getPath()
+    public function getPath(): string
     {
         return Common::config('thumbnail', 'dir') . \DIRECTORY_SEPARATOR . $this->getFilename();
     }
 
-    public function getUrl()
+    public function getUrl(Note|int $note): string
     {
-        return Router::url('attachment_thumbnail', ['id' => $this->getAttachmentId(), 'size' => self::sizeIntToStr($this->getSize())]);
+        return Router::url('note_attachment_thumbnail', ['note_id' => \is_int($note) ? $note : $note->getId(), 'attachment_id' => $this->getAttachmentId(), 'size' => self::sizeIntToStr($this->getSize())]);
     }
 
     /**
@@ -277,9 +278,10 @@ class AttachmentThumbnail extends Entity
             }
         }
         Cache::delete(self::getCacheKey($this->getAttachmentId(), $this->getSize()));
-        DB::remove($this);
         if ($flush) {
-            DB::flush();
+            DB::wrapInTransaction(fn () => DB::remove($this));
+        } else {
+            DB::remove($this);
         }
     }
 

components/Attachment/tests/Entity/AttachmentThumbnailTest.php

@@ -19,12 +19,13 @@ declare(strict_types = 1);
 // along with GNU social.  If not, see <http://www.gnu.org/licenses/>.
 // }}}
 
-namespace App\Tests\Entity;
+namespace Component\Attachment\tests\Entity;
 
 use App\Core\DB\DB;
 use App\Core\Event;
 use App\Util\Exception\NotStoredLocallyException;
 use App\Util\GNUsocialTestCase;
+use Component\Attachment\Entity\Attachment;
 use Component\Attachment\Entity\AttachmentThumbnail;
 use Functional as F;
 use Jchook\AssertThrows\AssertThrows;
@@ -42,9 +43,9 @@ class AttachmentThumbnailTest extends GNUsocialTestCase
         $file = new SplFileInfo(INSTALLDIR . '/tests/sample-uploads/attachment-lifecycle-target.jpg');
         $hash = null;
         Event::handle('HashFile', [$file->getPathname(), &$hash]);
-        $attachment = DB::findOneBy('attachment', ['filehash' => $hash]);
+        $attachment = DB::findOneBy(Attachment::class, ['filehash' => $hash]);
 
-        $thumbs = [
+        $expected = [
             AttachmentThumbnail::getOrCreate($attachment, 'small', crop: false),
             AttachmentThumbnail::getOrCreate($attachment, 'medium', crop: false),
             $thumb = AttachmentThumbnail::getOrCreate($attachment, 'big', crop: false),
@@ -54,21 +55,29 @@ class AttachmentThumbnailTest extends GNUsocialTestCase
         $thumb->setAttachment(null);
         static::assertSame($attachment, $thumb->getAttachment());
 
-        $sort      = fn ($l, $r) => [$l->getWidth(), $l->getHeight()] <=> [$r->getWidth(), $r->getHeight()];
+        $actual = $attachment->getThumbnails();
-        $at_thumbs = F\sort($attachment->getThumbnails(), $sort);
+        static::assertSame(\count($expected), \count($actual));
-        static::assertSame($thumbs, $at_thumbs);
+        foreach ($expected as $e) {
-        array_pop($thumbs);
+            $a = array_shift($actual);
-        $thumb->delete(flush: true);
+            static::assertObjectEquals($e, $a);
-        $at_thumbs = F\sort($attachment->getThumbnails(), $sort);
+        }
-        static::assertSame($thumbs, $at_thumbs);
+
+        array_pop($expected);
+        $thumb->delete();
+        $actual = $attachment->getThumbnails();
+        static::assertSame(\count($expected), \count($actual));
+        foreach ($expected as $e) {
+            $a = array_shift($actual);
+            static::assertObjectEquals($e, $a);
+        }
 
         $attachment->deleteStorage();
 
-        foreach (array_reverse($thumbs) as $t) {
+        foreach (array_reverse($attachment->getThumbnails()) as $t) {
             // Since we still have thumbnails, those will be used as the new thumbnail, even though we don't have the original
             $new = AttachmentThumbnail::getOrCreate($attachment, 'big', crop: false);
             static::assertSame([$t->getFilename(), $t->getSize()], [$new->getFilename(), $new->getSize()]);
-            $t->delete(flush: true);
+            $t->delete();
         }
 
         // Since the backed storage was deleted and we don't have any more previous thumnbs, we can't generate another thumbnail
@@ -159,10 +168,10 @@ class AttachmentThumbnailTest extends GNUsocialTestCase
     public function testGetUrl()
     {
         parent::bootKernel();
-        $attachment = DB::findBy('attachment', ['mimetype' => 'image/png'], limit: 1)[0];
+        $attachment = DB::findBy(Attachment::class, ['mimetype' => 'image/png'], limit: 1)[0];
         $thumb      = AttachmentThumbnail::getOrCreate($attachment, 'big', crop: false);
         $id         = $attachment->getId();
-        $url        = "/attachment/{$id}/thumbnail/big";
+        $expected   = "/object/note/42/attachment/{$id}/thumbnail/big";
-        static::assertSame($url, $thumb->getUrl());
+        static::assertSame($expected, $thumb->getUrl(note: 42));
     }
 }

plugins/AttachmentCollections/Entity/AttachmentCollection.php

@@ -52,7 +52,7 @@ class AttachmentCollection extends Entity
     // @codeCoverageIgnoreEnd
     // }}} Autocode
 
-    public static function schemaDef()
+    public static function schemaDef(): array
     {
         return [
             'name'   => 'attachment_collection',

plugins/AttachmentCollections/Entity/AttachmentCollectionEntry.php

@@ -63,7 +63,7 @@ class AttachmentCollectionEntry extends Entity
     // @codeCoverageIgnoreEnd
     // }}} Autocode
 
-    public static function schemaDef()
+    public static function schemaDef(): array
     {
         return [
             'name'   => 'attachment_collection_entry',

plugins/Embed/Entity/AttachmentEmbed.php

@@ -194,7 +194,7 @@ class AttachmentEmbed extends Entity
         return $attr;
     }
 
-    public static function schemaDef()
+    public static function schemaDef(): array
     {
         return [
             'name'   => 'attachment_embed',

plugins/Favourite/Entity/NoteFavourite.php

@@ -142,7 +142,7 @@ class NoteFavourite extends Entity
         return array_unique($target_ids);
     }
 
-    public static function schemaDef()
+    public static function schemaDef(): array
     {
         return [
             'name'   => 'note_favourite',

plugins/PinnedNotes/Entity/PinnedNotes.php

@@ -42,7 +42,7 @@ class PinnedNotes extends Entity
         return $this;
     }
 
-    public static function schemaDef()
+    public static function schemaDef(): array
     {
         return [
             'name'   => 'pinned_notes',

plugins/WebMonetization/Entity/Wallet.php

@@ -51,7 +51,7 @@ class Wallet extends Entity
     // @codeCoverageIgnoreEnd
     // }}} Autocode
 
-    public static function schemaDef()
+    public static function schemaDef(): array
     {
         return [
             'name'   => 'webmonetizationWallet',

plugins/WebMonetization/Entity/WebMonetization.php

@@ -90,7 +90,7 @@ class WebMonetization extends Entity
         return $target_ids;
     }
 
-    public static function schemaDef()
+    public static function schemaDef(): array
     {
         return [
             'name'   => 'webmonetization',

src/Core/Entity.php

@@ -28,6 +28,7 @@ use App\Util\Exception\NotFoundException;
 use App\Util\Formatting;
 use BadMethodCallException;
 use DateTime;
+use DateTimeInterface;
 use Exception;
 use InvalidArgumentException;
 
@@ -48,6 +49,8 @@ abstract class Entity
         throw new BadMethodCallException('Non existent method ' . static::class . "::{$name} called with arguments: " . print_r($arguments, true));
     }
 
+    abstract public static function schemaDef(): array;
+
     /**
      * Create an instance of the called class or fill in the
      * properties of $obj with the associative array $args. Doesn't
@@ -145,6 +148,32 @@ abstract class Entity
         }
     }
 
+    /**
+     * Tests that this object is equal to another one based on a custom object comparison
+     *
+     * @param self $other the value to test
+     *
+     * @return bool true if equals, false otherwise
+     */
+    public function equals(self $other): bool
+    {
+        foreach (array_keys($this::schemaDef()['fields']) as $attribute) {
+            $getter  = 'get' . Formatting::snakeCaseToPascalCase($attribute);
+            $current = $this->{$getter}();
+            $target  = $other->{$getter}();
+            if ($current instanceof DateTimeInterface) {
+                if ($current->getTimestamp() !== $target->getTimestamp()) {
+                    return false;
+                }
+            } else {
+                if ($current !== $target) {
+                    return false;
+                }
+            }
+        }
+        return true;
+    }
+
     /**
      * Who should be notified about this object?
      *

src/Entity/Actor.php

@@ -332,7 +332,7 @@ class Actor extends Entity
     {
         return Cache::getList(
             self::cacheKeys($this->getId())['self-tags'],
-            fn() => DB::findBy(ActorTag::class, ['tagger' => $this->getId(), 'tagged' => $this->getId()], order_by: ['modified' => 'DESC']),
+            fn() => DB::findBy(ActorTag::class, ['tagger' => $this->getId(), 'tagged' => $this->getId()], order_by: ['modified' => 'DESC', 'tag' => 'ASC']),
         );
     }
 

src/Util/Formatting.php

@@ -153,6 +153,11 @@ abstract class Formatting
         return implode('', F\map(preg_split('/[\b_]/', $str), F\ary('ucfirst', 1)));
     }
 
+    public static function snakeCaseToPascalCase(string $str): string
+    {
+        return ucfirst(self::snakeCaseToCamelCase($str));
+    }
+
     /**
      * Indent $in, a string or array, $level levels
      *

tests/Entity/ActorTest.php

@@ -56,10 +56,9 @@ class ActorTest extends GNUsocialTestCase
         ]));
         DB::flush();
         Cache::delete(Actor::cacheKeys($actor->getId())['self-tags']);
-        static::assertSame(
+        $actual = $actor->getSelfTags();
-            expected: [$actor_tag_foo],
+        static::assertCount(1, $actual);
-            actual: $actor->getSelfTags(),
+        static::assertObjectEquals(expected: $actor_tag_foo, actual: $actual[0]);
-        );
         // Add a second self-tag 'foo'
         $tag = CompTag::sanitize('bar');
         DB::persist($actor_tag_bar = ActorTag::create([
@@ -69,9 +68,14 @@ class ActorTest extends GNUsocialTestCase
         ]));
         DB::flush();
         Cache::delete(Actor::cacheKeys($actor->getId())['self-tags']);
-        static::assertSame(
+        $actual = $actor->getSelfTags();
-            expected: [$actor_tag_bar, $actor_tag_foo],
+        static::assertCount(2, $actual);
-            actual: $actor->getSelfTags(),
+        foreach ([$actor_tag_bar, $actor_tag_foo] as $expected) {
-        );
+            $a = array_shift($actual);
+            if ($expected->equals($a) !== true) {
+                dd($expected, $a);
+            }
+            static::assertObjectEquals($expected, $a);
+        }
     }
 }