Merge pull request #11943 from dhairyajangir/fix/attachment-createdat-timestamp-and-migration

Enforce NOT NULL and Remove Default Value for mail_attachments.created_at Column

Author: kesselb

appinfo/info.xml

@@ -34,7 +34,7 @@ The rating depends on the installed text processing backend. See [the rating ove
 
 Learn more about the Nextcloud Ethical AI Rating [in our blog](https://nextcloud.com/blog/nextcloud-ethical-ai-rating/).
 	]]></description>
-	<version>5.6.0-alpha.3</version>
+	<version>5.6.0-alpha.4</version>
 	<licence>agpl</licence>
 	<author homepage="https://github.com/ChristophWurst">Christoph Wurst</author>
 	<author homepage="https://github.com/GretaD">GretaD</author>

lib/Migration/Version5007Date20251019000000.php

@@ -0,0 +1,40 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\Mail\Migration;
+
+use Closure;
+use OCP\DB\ISchemaWrapper;
+use OCP\Migration\IOutput;
+use OCP\Migration\SimpleMigrationStep;
+
+class Version5006Date20251019000000 extends SimpleMigrationStep {
+	/**
+	 * @param Closure(): ISchemaWrapper $schemaClosure
+	 */
+	#[\Override]
+	public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
+		$schema = $schemaClosure();
+
+		if (!$schema->hasTable('mail_attachments')) {
+			return $schema;
+		}
+
+		$attachments = $schema->getTable('mail_attachments');
+
+		// Ensure created_at is NOT NULL and has no default, so app must set it.
+		if ($attachments->hasColumn('created_at')) {
+			$attachments->modifyColumn('created_at', [
+				'default' => null,
+			]);
+		}
+
+		return $schema;
+	}
+}

lib/Service/Attachment/AttachmentService.php

@@ -22,6 +22,7 @@
 use OCA\Mail\Exception\UploadException;
 use OCA\Mail\IMAP\MessageMapper;
 use OCP\AppFramework\Db\DoesNotExistException;
+use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\Files\File;
 use OCP\Files\Folder;
 use OCP\Files\NotFoundException;
@@ -52,6 +53,8 @@ class AttachmentService implements IAttachmentService {
 	 */
 	private $logger;
 
+	private ITimeFactory $timeFactory;
+
 	/**
 	 * @param Folder $userFolder
 	 */
@@ -60,13 +63,15 @@ public function __construct($userFolder,
 		AttachmentStorage $storage,
 		IMailManager $mailManager,
 		MessageMapper $imapMessageMapper,
-		LoggerInterface $logger) {
+		LoggerInterface $logger,
+		ITimeFactory $timeFactory) {
 		$this->mapper = $mapper;
 		$this->storage = $storage;
 		$this->mailManager = $mailManager;
 		$this->messageMapper = $imapMessageMapper;
 		$this->userFolder = $userFolder;
 		$this->logger = $logger;
+		$this->timeFactory = $timeFactory;
 	}
 
 	/**
@@ -80,6 +85,8 @@ public function addFile(string $userId, UploadedFile $file): LocalAttachment {
 		$attachment->setUserId($userId);
 		$attachment->setFileName($file->getFileName());
 		$attachment->setMimeType($file->getMimeType());
+		// set createdAt timestamp for cleanup/retention
+		$attachment->setCreatedAt($this->timeFactory->getTime());
 
 		$persisted = $this->mapper->insert($attachment);
 		try {
@@ -98,6 +105,8 @@ public function addFileFromString(string $userId, string $name, string $mime, st
 		$attachment->setUserId($userId);
 		$attachment->setFileName($name);
 		$attachment->setMimeType($mime);
+		// set createdAt timestamp for consistency with uploaded attachments
+		$attachment->setCreatedAt($this->timeFactory->getTime());
 
 		$persisted = $this->mapper->insert($attachment);
 		try {

tests/Integration/Service/DraftServiceIntegrationTest.php

@@ -96,7 +96,8 @@ protected function setUp(): void {
 			Server::get(AttachmentStorage::class),
 			$mailManager,
 			Server::get(MessageMapper::class),
-			new NullLogger()
+			new NullLogger(),
+			Server::get(ITimeFactory::class)
 		);
 		$this->client = $this->getClient($this->account);
 		$this->mapper = Server::get(LocalMessageMapper::class);

tests/Integration/Service/OutboxServiceIntegrationTest.php

@@ -97,7 +97,8 @@ protected function setUp(): void {
 			Server::get(AttachmentStorage::class),
 			$mailManager,
 			Server::get(\OCA\Mail\IMAP\MessageMapper::class),
-			new NullLogger()
+			new NullLogger(),
+			Server::get(ITimeFactory::class)
 		);
 		$this->client = $this->getClient($this->account);
 		$this->mapper = Server::get(LocalMessageMapper::class);

tests/Unit/Service/Attachment/AttachmentServiceTest.php

@@ -24,6 +24,7 @@
 use OCA\Mail\Service\Attachment\AttachmentStorage;
 use OCA\Mail\Service\Attachment\UploadedFile;
 use OCP\AppFramework\Db\DoesNotExistException;
+use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\Files\Folder;
 use OCP\Files\NotPermittedException;
 use OCP\Share\IAttributes;
@@ -53,6 +54,9 @@ class AttachmentServiceTest extends TestCase {
 	/** @var MockObject|LoggerInterface */
 	private $logger;
 
+	/** @var ITimeFactory|MockObject */
+	private $timeFactory;
+
 	protected function setUp(): void {
 		parent::setUp();
 
@@ -62,14 +66,17 @@ protected function setUp(): void {
 		$this->messageMapper = $this->createMock(MessageMapper::class);
 		$this->userFolder = $this->createMock(Folder::class);
 		$this->logger = $this->createMock(LoggerInterface::class);
+		$this->timeFactory = $this->createMock(ITimeFactory::class);
+		$this->timeFactory->method('getTime')->willReturn(123456);
 
 		$this->service = new AttachmentService(
 			$this->userFolder,
 			$this->mapper,
 			$this->storage,
 			$this->mailManager,
 			$this->messageMapper,
-			$this->logger
+			$this->logger,
+			$this->timeFactory
 		);
 	}
 
@@ -82,6 +89,7 @@ public function testAddFileWithUploadException() {
 		$attachment = LocalAttachment::fromParams([
 			'userId' => $userId,
 			'fileName' => 'cat.jpg',
+			'createdAt' => 123456,
 		]);
 		$persistedAttachment = LocalAttachment::fromParams([
 			'id' => 123,
@@ -113,7 +121,8 @@ public function testAddFile() {
 			->willReturn('cat.jpg');
 		$attachment = LocalAttachment::fromParams([
 			'userId' => $userId,
-			'fileName' => 'cat.jpg'
+			'fileName' => 'cat.jpg',
+			'createdAt' => 123456
 		]);
 		$persistedAttachment = LocalAttachment::fromParams([
 			'id' => 123,
@@ -138,6 +147,7 @@ public function testAddFileFromStringWithUploadException() {
 			'userId' => $userId,
 			'fileName' => 'cat.jpg',
 			'mimeType' => 'image/jpg',
+			'createdAt' => 123456,
 		]);
 		$persistedAttachment = LocalAttachment::fromParams([
 			'id' => 123,
@@ -168,6 +178,7 @@ public function testAddFileFromString() {
 			'userId' => $userId,
 			'fileName' => 'cat.jpg',
 			'mimeType' => 'image/jpg',
+			'createdAt' => 123456,
 		]);
 		$persistedAttachment = LocalAttachment::fromParams([
 			'id' => 123,
@@ -283,6 +294,7 @@ public function testHandleAttachmentsForwardedMessageAttachment(): void {
 			'userId' => $userId,
 			'fileName' => 'cat.jpg',
 			'mimeType' => 'text/plain',
+			'createdAt' => 123456,
 		]);
 		$persistedAttachment = LocalAttachment::fromParams([
 			'id' => 123,
@@ -334,6 +346,7 @@ public function testHandleAttachmentsForwardedAttachment(): void {
 			'userId' => $userId,
 			'fileName' => 'cat.jpg',
 			'mimeType' => 'text/plain',
+			'createdAt' => 123456,
 		]);
 		$persistedAttachment = LocalAttachment::fromParams([
 			'id' => 123,
@@ -443,6 +456,7 @@ public function testHandleAttachmentsCloudAttachment(): void {
 			'userId' => $userId,
 			'fileName' => 'cat.jpg',
 			'mimeType' => 'text/plain',
+			'createdAt' => 123456,
 		]);
 		$persistedAttachment = LocalAttachment::fromParams([
 			'id' => 123,