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

components/Attachment/tests/Entity/AttachmentTest.php

@@ -19,15 +19,17 @@ 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\Core\GSFile;
+use App\Core\Router\Router;
 use App\Entity\Note;
 use App\Util\GNUsocialTestCase;
 use App\Util\TemporaryFile;
 use Component\Attachment\Entity\AttachmentToNote;
+use Component\Conversation\Conversation;
 use Jchook\AssertThrows\AssertThrows;
 use SplFileInfo;
 use Symfony\Component\HttpFoundation\File\File;
@@ -107,6 +109,7 @@ class AttachmentTest extends GNUsocialTestCase
 
         $actor = DB::findOneBy('actor', ['nickname' => 'taken_user']);
         DB::persist($note = Note::create(['actor_id' => $actor->getId(), 'content' => 'attachment: some content', 'content_type' => 'text/plain', 'is_local' => true]));
+        Conversation::assignLocalConversation($note, null);
         DB::persist(AttachmentToNote::create(['attachment_id' => $attachment->getId(), 'note_id' => $note->getId(), 'title' => 'A title']));
         DB::flush();
 
@@ -118,7 +121,7 @@ class AttachmentTest extends GNUsocialTestCase
         static::bootKernel();
         $attachment = DB::findBy('attachment', ['mimetype' => 'image/png'], limit: 1)[0];
         $id         = $attachment->getId();
-        static::assertSame("/attachment/{$id}/view", $attachment->getUrl());
+        static::assertSame("/object/note/42/attachment/{$id}/view", $attachment->getUrl(note: 42, type: Router::ABSOLUTE_PATH));
     }
 
     public function testMimetype()

components/Posting/Posting.php

@@ -221,13 +221,15 @@ class Posting extends Component
             Event::handle('ProcessNoteContent', [$note, $content, $content_type, $process_note_content_extra_args]);
         }
 
+        // These are note attachments now, and not just attachments, ensure these relations are ensured
         if ($processed_attachments !== []) {
             foreach ($processed_attachments as [$a, $fname]) {
+                // Most attachments should already be associated with its author, but maybe it didn't make sense
+                //for this attachment, or it's simply a repost of an attachment by a different actor
                 if (DB::count('actor_to_attachment', $args = ['attachment_id' => $a->getId(), 'actor_id' => $actor->getId()]) === 0) {
                     DB::persist(ActorToAttachment::create($args));
                 }
                 DB::persist(AttachmentToNote::create(['attachment_id' => $a->getId(), 'note_id' => $note->getId(), 'title' => $fname]));
-                $a->livesIncrementAndGet();
             }
         }
 

plugins/RepeatNote/RepeatNote.php

@@ -87,11 +87,18 @@ class RepeatNote extends NoteHandlerPlugin
             locale: \is_null($lang_id = $note->getLanguageId()) ? null : Language::getById($lang_id)->getLocale(),
             // If it's a repeat, the reply_to should be to the original, conversation ought to be the same
             reply_to: $note->getReplyTo(),
-            processed_attachments: $note->getAttachmentsWithTitle(),
+            processed_attachments: $processed_attachments = $note->getAttachmentsWithTitle(),
             flush_and_notify: false,
             rendered: $note->getRendered(),
         );
 
+        // Increment attachment lives, posting already handled actor_to_attachment relations for us,
+        // and it obviously handled the attachment_to_note
+        foreach ($processed_attachments as [$a, $fname]) {
+            // We usually don't have to increment lives manually when storing an attachment, but we aren't re-storing it
+            $a->livesIncrementAndGet();
+        }
+
         DB::persist(RepeatEntity::create([
             'note_id'   => $repeat->getId(),
             'actor_id'  => $actor_id,

src/Core/GSFile.php

@@ -32,6 +32,7 @@ use App\Util\Exception\NoSuchFileException;
 use App\Util\Exception\NotFoundException;
 use App\Util\Exception\NotStoredLocallyException;
 use App\Util\Exception\ServerException;
+use App\Util\Exception\TemporaryFileException;
 use App\Util\TemporaryFile;
 use Component\Attachment\Entity\Attachment;
 use Symfony\Component\HttpFoundation\BinaryFileResponse;
@@ -55,8 +56,10 @@ class GSFile
 {
     /**
      * Perform file validation (checks and normalization), store the given file if needed
+     * IMPORTANT: A new attachment is stored with 1 live, a known attachment has its lives incremented
      *
      * @throws DuplicateFoundException
+     * @throws TemporaryFileException
      */
     public static function storeFileAsAttachment(TemporaryFile|SymfonyFile $file, bool $check_is_supported_mimetype = true): Attachment
     {
@@ -65,8 +68,8 @@ class GSFile
         try {
             $attachment = DB::findOneBy('attachment', ['filehash' => $hash]);
             // Attachment Exists
-            // We had this attachment, but not the file, thus no filename, update meta
             if (\is_null($attachment->getFilename())) {
+                // We had this attachment, but not the file, thus no filename, update meta
                 $mimetype               = $attachment->getMimetype() ?? $file->getMimeType();
                 $width                  = $attachment->getWidth();
                 $height                 = $attachment->getHeight();
@@ -99,6 +102,8 @@ class GSFile
                     throw new FileNotAllowedException($mimetype);
                 }
             }
+            // Increment lives
+            $attachment->livesIncrementAndGet();
         } catch (NotFoundException) {
             // Create an Attachment
             // The following properly gets the mimetype with `file` or other
@@ -127,7 +132,7 @@ class GSFile
                 'size'     => $file->getSize(),
                 'width'    => $width,
                 'height'   => $height,
-                'lives'    => 0,
+                'lives'    => 1,
             ]);
             if (!$check_is_supported_mimetype || self::isMimetypeAllowed($mimetype)) {
                 $file->move(Common::config('attachments', 'dir'), $hash);