Skip to content

Commit b8be40e

Browse files
committed
Migration: Enforce NOT NULL and remove default on mail_attachments.created_at
This migration updates the mail_attachments table to require a non-null created_at timestamp for all attachments and removes any default value. This ensures that every attachment record must have an explicit creation time set by the application, preventing issues with zero or missing timestamps and enabling reliable cleanup of old data.
1 parent dd0b743 commit b8be40e

File tree

3 files changed

+73
-3
lines changed

3 files changed

+73
-3
lines changed
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-only
8+
*/
9+
10+
namespace OCA\Mail\Migration;
11+
12+
use Closure;
13+
use OCP\DB\ISchemaWrapper;
14+
use OCP\Migration\IOutput;
15+
use OCP\Migration\SimpleMigrationStep;
16+
17+
class Version5007Date20251019000000 extends SimpleMigrationStep {
18+
/**
19+
* @param IOutput $output
20+
* @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper`
21+
* @param array $options
22+
* @return null|ISchemaWrapper
23+
*/
24+
#[\Override]
25+
public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
26+
/** @var ISchemaWrapper $schema */
27+
$schema = $schemaClosure();
28+
29+
if (!$schema->hasTable('mail_attachments')) {
30+
return $schema;
31+
}
32+
33+
$attachments = $schema->getTable('mail_attachments');
34+
35+
// Ensure created_at is NOT NULL and has no default, so app must set it.
36+
if ($attachments->hasColumn('created_at')) {
37+
$attachments->changeColumn('created_at', [
38+
'notnull' => true,
39+
'default' => null,
40+
]);
41+
}
42+
43+
return $schema;
44+
}
45+
}

lib/Service/Attachment/AttachmentService.php

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
use OCP\Files\NotFoundException;
2828
use OCP\Files\NotPermittedException;
2929
use Psr\Log\LoggerInterface;
30+
use OCP\AppFramework\Utility\ITimeFactory;
3031

3132
class AttachmentService implements IAttachmentService {
3233
/** @var LocalAttachmentMapper */
@@ -52,6 +53,11 @@ class AttachmentService implements IAttachmentService {
5253
*/
5354
private $logger;
5455

56+
/**
57+
* @var ITimeFactory
58+
*/
59+
private $timeFactory;
60+
5561
/**
5662
* @param Folder $userFolder
5763
*/
@@ -60,13 +66,15 @@ public function __construct($userFolder,
6066
AttachmentStorage $storage,
6167
IMailManager $mailManager,
6268
MessageMapper $imapMessageMapper,
63-
LoggerInterface $logger) {
69+
LoggerInterface $logger,
70+
ITimeFactory $timeFactory) {
6471
$this->mapper = $mapper;
6572
$this->storage = $storage;
6673
$this->mailManager = $mailManager;
6774
$this->messageMapper = $imapMessageMapper;
6875
$this->userFolder = $userFolder;
6976
$this->logger = $logger;
77+
$this->timeFactory = $timeFactory;
7078
}
7179

7280
/**
@@ -80,6 +88,8 @@ public function addFile(string $userId, UploadedFile $file): LocalAttachment {
8088
$attachment->setUserId($userId);
8189
$attachment->setFileName($file->getFileName());
8290
$attachment->setMimeType($file->getMimeType());
91+
// set createdAt timestamp for cleanup/retention
92+
$attachment->setCreatedAt($this->timeFactory->getTime());
8393

8494
$persisted = $this->mapper->insert($attachment);
8595
try {
@@ -98,6 +108,8 @@ public function addFileFromString(string $userId, string $name, string $mime, st
98108
$attachment->setUserId($userId);
99109
$attachment->setFileName($name);
100110
$attachment->setMimeType($mime);
111+
// set createdAt timestamp for consistency with uploaded attachments
112+
$attachment->setCreatedAt($this->timeFactory->getTime());
101113

102114
$persisted = $this->mapper->insert($attachment);
103115
try {

tests/Unit/Service/Attachment/AttachmentServiceTest.php

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
use OCA\Mail\Service\Attachment\AttachmentStorage;
2525
use OCA\Mail\Service\Attachment\UploadedFile;
2626
use OCP\AppFramework\Db\DoesNotExistException;
27+
use OCP\AppFramework\Utility\ITimeFactory;
2728
use OCP\Files\Folder;
2829
use OCP\Files\NotPermittedException;
2930
use OCP\Share\IAttributes;
@@ -53,6 +54,9 @@ class AttachmentServiceTest extends TestCase {
5354
/** @var MockObject|LoggerInterface */
5455
private $logger;
5556

57+
/** @var ITimeFactory|MockObject */
58+
private $timeFactory;
59+
5660
protected function setUp(): void {
5761
parent::setUp();
5862

@@ -62,14 +66,17 @@ protected function setUp(): void {
6266
$this->messageMapper = $this->createMock(MessageMapper::class);
6367
$this->userFolder = $this->createMock(Folder::class);
6468
$this->logger = $this->createMock(LoggerInterface::class);
69+
$this->timeFactory = $this->createMock(ITimeFactory::class);
70+
$this->timeFactory->method('getTime')->willReturn(123456);
6571

6672
$this->service = new AttachmentService(
6773
$this->userFolder,
6874
$this->mapper,
6975
$this->storage,
7076
$this->mailManager,
7177
$this->messageMapper,
72-
$this->logger
78+
$this->logger,
79+
$this->timeFactory
7380
);
7481
}
7582

@@ -82,6 +89,7 @@ public function testAddFileWithUploadException() {
8289
$attachment = LocalAttachment::fromParams([
8390
'userId' => $userId,
8491
'fileName' => 'cat.jpg',
92+
'createdAt' => 123456,
8593
]);
8694
$persistedAttachment = LocalAttachment::fromParams([
8795
'id' => 123,
@@ -113,7 +121,8 @@ public function testAddFile() {
113121
->willReturn('cat.jpg');
114122
$attachment = LocalAttachment::fromParams([
115123
'userId' => $userId,
116-
'fileName' => 'cat.jpg'
124+
'fileName' => 'cat.jpg',
125+
'createdAt' => 123456
117126
]);
118127
$persistedAttachment = LocalAttachment::fromParams([
119128
'id' => 123,
@@ -138,6 +147,7 @@ public function testAddFileFromStringWithUploadException() {
138147
'userId' => $userId,
139148
'fileName' => 'cat.jpg',
140149
'mimeType' => 'image/jpg',
150+
'createdAt' => 123456,
141151
]);
142152
$persistedAttachment = LocalAttachment::fromParams([
143153
'id' => 123,
@@ -168,6 +178,7 @@ public function testAddFileFromString() {
168178
'userId' => $userId,
169179
'fileName' => 'cat.jpg',
170180
'mimeType' => 'image/jpg',
181+
'createdAt' => 123456,
171182
]);
172183
$persistedAttachment = LocalAttachment::fromParams([
173184
'id' => 123,
@@ -283,6 +294,7 @@ public function testHandleAttachmentsForwardedMessageAttachment(): void {
283294
'userId' => $userId,
284295
'fileName' => 'cat.jpg',
285296
'mimeType' => 'text/plain',
297+
'createdAt' => 123456,
286298
]);
287299
$persistedAttachment = LocalAttachment::fromParams([
288300
'id' => 123,
@@ -334,6 +346,7 @@ public function testHandleAttachmentsForwardedAttachment(): void {
334346
'userId' => $userId,
335347
'fileName' => 'cat.jpg',
336348
'mimeType' => 'text/plain',
349+
'createdAt' => 123456,
337350
]);
338351
$persistedAttachment = LocalAttachment::fromParams([
339352
'id' => 123,

0 commit comments

Comments
 (0)