From 28453c585f551b56ffbeec3ac29e725452369a69 Mon Sep 17 00:00:00 2001 From: Diogo Peralta Cordeiro Date: Tue, 8 Mar 2022 00:21:12 +0000 Subject: [PATCH] [COMPONENT][Attachment][TESTS] Fix Entity/AttachmentThumbnailTest --- components/Attachment/Entity/Attachment.php | 11 ++++-- .../Attachment/Entity/AttachmentThumbnail.php | 14 ++++--- .../tests}/Entity/AttachmentThumbnailTest.php | 39 ++++++++++++------- .../Entity/AttachmentCollection.php | 2 +- .../Entity/AttachmentCollectionEntry.php | 2 +- plugins/Embed/Entity/AttachmentEmbed.php | 2 +- plugins/Favourite/Entity/NoteFavourite.php | 2 +- plugins/PinnedNotes/Entity/PinnedNotes.php | 2 +- plugins/WebMonetization/Entity/Wallet.php | 2 +- .../Entity/WebMonetization.php | 2 +- src/Core/Entity.php | 29 ++++++++++++++ src/Entity/Actor.php | 2 +- src/Util/Formatting.php | 5 +++ tests/Entity/ActorTest.php | 20 ++++++---- 14 files changed, 93 insertions(+), 41 deletions(-) rename {tests => components/Attachment/tests}/Entity/AttachmentThumbnailTest.php (83%) diff --git a/components/Attachment/Entity/Attachment.php b/components/Attachment/Entity/Attachment.php index 739e1d412c..c2cb19d73b 100644 --- a/components/Attachment/Entity/Attachment.php +++ b/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::flush(); + DB::wrapInTransaction(fn () => DB::persist($this)); } } 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')]); } diff --git a/components/Attachment/Entity/AttachmentThumbnail.php b/components/Attachment/Entity/AttachmentThumbnail.php index b8edaa2511..176095b630 100644 --- a/components/Attachment/Entity/AttachmentThumbnail.php +++ b/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); } } diff --git a/tests/Entity/AttachmentThumbnailTest.php b/components/Attachment/tests/Entity/AttachmentThumbnailTest.php similarity index 83% rename from tests/Entity/AttachmentThumbnailTest.php rename to components/Attachment/tests/Entity/AttachmentThumbnailTest.php index 4f8feedfa4..6e49841a49 100644 --- a/tests/Entity/AttachmentThumbnailTest.php +++ b/components/Attachment/tests/Entity/AttachmentThumbnailTest.php @@ -19,12 +19,13 @@ declare(strict_types = 1); // along with GNU social. If not, see . // }}} -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()]; - $at_thumbs = F\sort($attachment->getThumbnails(), $sort); - static::assertSame($thumbs, $at_thumbs); - array_pop($thumbs); - $thumb->delete(flush: true); - $at_thumbs = F\sort($attachment->getThumbnails(), $sort); - static::assertSame($thumbs, $at_thumbs); + $actual = $attachment->getThumbnails(); + static::assertSame(\count($expected), \count($actual)); + foreach ($expected as $e) { + $a = array_shift($actual); + static::assertObjectEquals($e, $a); + } + + 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"; - static::assertSame($url, $thumb->getUrl()); + $expected = "/object/note/42/attachment/{$id}/thumbnail/big"; + static::assertSame($expected, $thumb->getUrl(note: 42)); } } diff --git a/plugins/AttachmentCollections/Entity/AttachmentCollection.php b/plugins/AttachmentCollections/Entity/AttachmentCollection.php index a40f30a37e..ebe804f4ff 100644 --- a/plugins/AttachmentCollections/Entity/AttachmentCollection.php +++ b/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', diff --git a/plugins/AttachmentCollections/Entity/AttachmentCollectionEntry.php b/plugins/AttachmentCollections/Entity/AttachmentCollectionEntry.php index 8497c9be21..e09048e1b5 100644 --- a/plugins/AttachmentCollections/Entity/AttachmentCollectionEntry.php +++ b/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', diff --git a/plugins/Embed/Entity/AttachmentEmbed.php b/plugins/Embed/Entity/AttachmentEmbed.php index 51e0ed48a2..de57c76a89 100644 --- a/plugins/Embed/Entity/AttachmentEmbed.php +++ b/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', diff --git a/plugins/Favourite/Entity/NoteFavourite.php b/plugins/Favourite/Entity/NoteFavourite.php index 9b08f3a164..0e8fcdf923 100644 --- a/plugins/Favourite/Entity/NoteFavourite.php +++ b/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', diff --git a/plugins/PinnedNotes/Entity/PinnedNotes.php b/plugins/PinnedNotes/Entity/PinnedNotes.php index ae121f09e1..13d23f9996 100644 --- a/plugins/PinnedNotes/Entity/PinnedNotes.php +++ b/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', diff --git a/plugins/WebMonetization/Entity/Wallet.php b/plugins/WebMonetization/Entity/Wallet.php index 765601f2ea..4ab7c2e36a 100644 --- a/plugins/WebMonetization/Entity/Wallet.php +++ b/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', diff --git a/plugins/WebMonetization/Entity/WebMonetization.php b/plugins/WebMonetization/Entity/WebMonetization.php index 27b4df9949..7d93152ecf 100644 --- a/plugins/WebMonetization/Entity/WebMonetization.php +++ b/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', diff --git a/src/Core/Entity.php b/src/Core/Entity.php index cd95be7de1..bf49d566a2 100644 --- a/src/Core/Entity.php +++ b/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? * diff --git a/src/Entity/Actor.php b/src/Entity/Actor.php index 98d69be1f3..08e654aba2 100644 --- a/src/Entity/Actor.php +++ b/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']), ); } diff --git a/src/Util/Formatting.php b/src/Util/Formatting.php index 6614e68b77..9d0b7e98ed 100644 --- a/src/Util/Formatting.php +++ b/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 * diff --git a/tests/Entity/ActorTest.php b/tests/Entity/ActorTest.php index b5e704be7f..30ab80e4fb 100644 --- a/tests/Entity/ActorTest.php +++ b/tests/Entity/ActorTest.php @@ -56,10 +56,9 @@ class ActorTest extends GNUsocialTestCase ])); DB::flush(); Cache::delete(Actor::cacheKeys($actor->getId())['self-tags']); - static::assertSame( - expected: [$actor_tag_foo], - actual: $actor->getSelfTags(), - ); + $actual = $actor->getSelfTags(); + static::assertCount(1, $actual); + 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( - expected: [$actor_tag_bar, $actor_tag_foo], - actual: $actor->getSelfTags(), - ); + $actual = $actor->getSelfTags(); + static::assertCount(2, $actual); + 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); + } } }