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

dhairyajangir · 2025-10-19T19:14:23Z

PR: Enforce NOT NULL and Remove Default Value for `mail_attachments.created_at`

Overview

This pull request addresses an important data integrity issue in the Nextcloud Mail app: previously, attachments could be created with a created_at timestamp of 0 or NULL due to a default value on the database column and missing assignment in the application logic. This made it difficult to reliably clean up old attachments and could lead to inconsistent data.

What Has Changed

1. Database Migration

File: lib/Migration/Version5007Date20251019000000.php
Purpose:
- Alters the mail_attachments table to ensure the created_at column is always set.
- Sets the created_at column to NOT NULL and removes any default value.
- This enforces that every new attachment must have an explicit creation timestamp set by the application.

2. How the Migration Works

Checks if the mail_attachments table exists.
Checks if the created_at column exists.
Alters the column to:
- Require a value (NOT NULL)
- Remove any default value (default => null)
This ensures that the application must always provide a valid timestamp when inserting new attachments.

3. Why This Is Important

Data Integrity:
Prevents the creation of attachments with missing or invalid timestamps.
Cleanup Reliability:
Accurate created_at values are essential for reliably cleaning up old attachments and managing storage.
Prevents Future Bugs:
This change would have prevented the original bug by making it impossible to insert attachments with a default or missing timestamp.

4. How to Test

Run the migration:
- The migration will update the schema as described.
Add a new attachment:
- Confirm that the created_at field is set to the current timestamp and is never 0 or NULL.
Try inserting an attachment without a created_at value:
- The database should reject the insert, enforcing the application logic.

5. Additional Notes

This migration is safe to run multiple times; it checks for the existence of the table and column before making changes.
No existing data is deleted or modified, but any rows with a created_at of 0 or NULL should be reviewed separately if present.
This change is a prerequisite for reliable cleanup jobs and future features that depend on accurate attachment timestamps.

welcome · 2025-10-19T19:14:26Z

Thanks for opening your first pull request in this repository! ✌️

…eated_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. Signed-off-by: dhairyasquad73 <dhairya.jangir.s73@kalvium.community>

ChristophWurst

Looks good

lib/Service/Attachment/AttachmentService.php

ChristophWurst · 2025-10-20T06:19:48Z

Thanks a lot for the fix @dhairyajangir. If I understand correctly you are targeting #5901, right?

Out of curiosity, what tools did you use for the task?

ChristophWurst · 2025-10-20T06:20:50Z

The commit message says migration: …. Please change it to fix(db): …

https://www.conventionalcommits.org/en/v1.0.0/#summary

ChristophWurst · 2025-10-20T06:21:51Z

There are minor linting issues due to the wrong whitespace character used. Run composer run cs:fix to auto-fix them :)

dhairyajangir · 2025-10-20T06:31:52Z

Thanks a lot for the fix @dhairyajangir. If I understand correctly you are targeting #5901, right?

Out of curiosity, what tools did you use for the task?

Thanks @ChristophWurst,
And Yes I am targeting #5901
and i used -

Visual Studio Code for editing
PHP for implementation and Nextcloud app code
Nextcloud app framework APIs (migration layer, DI, ITimeFactory)
ISchemaWrapper (schema change in migration)

Signed-off-by: dhairyasquad73 <dhairya.jangir.s73@kalvium.community>

…-and-migration

ChristophWurst · 2025-10-20T07:10:56Z

Linter issues:


   1) lib/Service/Attachment/AttachmentService.php
      ---------- begin diff ----------
--- /home/runner/actions-runner/_work/mail/mail/lib/Service/Attachment/AttachmentService.php
+++ /home/runner/actions-runner/_work/mail/mail/lib/Service/Attachment/AttachmentService.php
@@ -22,12 +22,12 @@
 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;
 use OCP\Files\NotPermittedException;
 use Psr\Log\LoggerInterface;
-use OCP\AppFramework\Utility\ITimeFactory;
 
 class AttachmentService implements IAttachmentService {
 	/** @var LocalAttachmentMapper */

      ----------- end diff -----------

Test failure:

There was 1 failure:

1) OCA\Mail\Tests\Unit\Service\Attachment\AttachmentServiceTest::testHandleAttachmentsCloudAttachment
Expectation failed for method name is "insert" when invoked 1 time(s)
Parameter 0 for invocation OCP\AppFramework\Db\QBMapper::insert(OCA\Mail\Db\LocalAttachment Object (...)): OCP\AppFramework\Db\Entity does not match expected value.
Failed asserting that two objects are equal.
--- Expected
+++ Actual
@@ @@
         'userId' => true
         'fileName' => true
         'mimeType' => true
+        'createdAt' => true
     )
     '_fieldTypes' => Array (...)
     'userId' => 'linus'
     'fileName' => 'cat.jpg'
     'mimeType' => 'text/plain'
-    'createdAt' => null
+    'createdAt' => 123456
     'localMessageId' => null
 )

/home/runner/actions-runner/_work/mail/mail/nextcloud/apps/mail/lib/Service/Attachment/AttachmentService.php:111
/home/runner/actions-runner/_work/mail/mail/nextcloud/apps/mail/lib/Service/Attachment/AttachmentService.php:382
/home/runner/actions-runner/_work/mail/mail/nextcloud/apps/mail/lib/Service/Attachment/AttachmentService.php:254
/home/runner/actions-runner/_work/mail/mail/nextcloud/apps/mail/tests/Unit/Service/Attachment/AttachmentServiceTest.php:489

dhairyajangir · 2025-10-20T08:25:42Z

Linter issues:


   1) lib/Service/Attachment/AttachmentService.php
      ---------- begin diff ----------
--- /home/runner/actions-runner/_work/mail/mail/lib/Service/Attachment/AttachmentService.php
+++ /home/runner/actions-runner/_work/mail/mail/lib/Service/Attachment/AttachmentService.php
@@ -22,12 +22,12 @@
 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;
 use OCP\Files\NotPermittedException;
 use Psr\Log\LoggerInterface;
-use OCP\AppFramework\Utility\ITimeFactory;
 
 class AttachmentService implements IAttachmentService {
 	/** @var LocalAttachmentMapper */

      ----------- end diff -----------

Test failure:

There was 1 failure:

1) OCA\Mail\Tests\Unit\Service\Attachment\AttachmentServiceTest::testHandleAttachmentsCloudAttachment
Expectation failed for method name is "insert" when invoked 1 time(s)
Parameter 0 for invocation OCP\AppFramework\Db\QBMapper::insert(OCA\Mail\Db\LocalAttachment Object (...)): OCP\AppFramework\Db\Entity does not match expected value.
Failed asserting that two objects are equal.
--- Expected
+++ Actual
@@ @@
         'userId' => true
         'fileName' => true
         'mimeType' => true
+        'createdAt' => true
     )
     '_fieldTypes' => Array (...)
     'userId' => 'linus'
     'fileName' => 'cat.jpg'
     'mimeType' => 'text/plain'
-    'createdAt' => null
+    'createdAt' => 123456
     'localMessageId' => null
 )

/home/runner/actions-runner/_work/mail/mail/nextcloud/apps/mail/lib/Service/Attachment/AttachmentService.php:111
/home/runner/actions-runner/_work/mail/mail/nextcloud/apps/mail/lib/Service/Attachment/AttachmentService.php:382
/home/runner/actions-runner/_work/mail/mail/nextcloud/apps/mail/lib/Service/Attachment/AttachmentService.php:254
/home/runner/actions-runner/_work/mail/mail/nextcloud/apps/mail/tests/Unit/Service/Attachment/AttachmentServiceTest.php:489

I'll fix this asap!

ChristophWurst · 2025-10-20T08:57:39Z

Take your time

- Inject ITimeFactory into AttachmentService constructor - Set createdAt using timeFactory.getTime() when creating attachments - Update unit tests to mock ITimeFactory and expect createdAt value - Update integration tests to pass ITimeFactory dependency Fixes nextcloud#5901 Signed-off-by: dhairyasquad73 <dhairya.jangir.s73@kalvium.community>

lib/Migration/Version5007Date20251019000000.php

kesselb

Thank you 🙏

lib/Migration/Version5007Date20251019000000.php

kesselb · 2025-10-21T21:07:34Z

lib/Migration/Version5007Date20251019000000.php

@@ -0,0 +1,45 @@
+<?php
+
+declare(strict_types=1);


Please increase the version in appinfo/info.xml to 5.6.0-alpha.4 (otherwise the migration will not run).

lib/Migration/Version5007Date20251019000000.php

dhairyajangir · 2025-10-21T21:24:17Z

Thank you 🙏

my pleasure

Co-authored-by: Daniel <mail@danielkesselberg.de> Signed-off-by: dhairya <dhairya.jangir.s73@kalvium.community>

dhairyajangir · 2025-10-21T21:30:10Z

Open for more tasks!

…tion

Co-authored-by: Daniel <mail@danielkesselberg.de> Signed-off-by: dhairya <dhairya.jangir.s73@kalvium.community>

line 7 updated `AGPL-3.0-only` set to `AGPL-3.0-or-later` Co-authored-by: Daniel <mail@danielkesselberg.de> Signed-off-by: dhairya <dhairya.jangir.s73@kalvium.community>

…tion

Version bump to trigger migration Version5007Date20251019000000 which enforces NOT NULL and removes default on mail_attachments.created_at Signed-off-by: dhairyasquad73 <dhairya.jangir.s73@kalvium.community>

dhairyajangir requested review from ChristophWurst, GretaD and kesselb as code owners October 19, 2025 19:14

dhairyajangir force-pushed the fix/attachment-createdat-timestamp-and-migration branch from b8be40e to 804f18c Compare October 19, 2025 19:24

ChristophWurst reviewed Oct 20, 2025

View reviewed changes

lib/Service/Attachment/AttachmentService.php Outdated Show resolved Hide resolved

Fix: Use tabs instead of spaces for indentation

f75ccb4

Signed-off-by: dhairyasquad73 <dhairya.jangir.s73@kalvium.community>

ChristophWurst added bug 3. to review labels Oct 20, 2025

Merge branch 'nextcloud:main' into fix/attachment-createdat-timestamp…

b403409

…-and-migration

dhairyajangir requested a review from ChristophWurst October 20, 2025 07:00

kesselb added the hacktoberfest-accepted label Oct 21, 2025

kesselb reviewed Oct 21, 2025

View reviewed changes

lib/Migration/Version5007Date20251019000000.php Outdated Show resolved Hide resolved

lib/Migration/Version5007Date20251019000000.php Outdated Show resolved Hide resolved

lib/Migration/Version5007Date20251019000000.php Show resolved Hide resolved

kesselb reviewed Oct 21, 2025

View reviewed changes

dhairyajangir and others added 4 commits October 22, 2025 02:54

Update lib/Migration/Version5007Date20251019000000.php

0e6f1e5

Co-authored-by: Daniel <mail@danielkesselberg.de> Signed-off-by: dhairya <dhairya.jangir.s73@kalvium.community>

Update lib/Migration/Version5007Date20251019000000.php

2e0c502

Co-authored-by: Daniel <mail@danielkesselberg.de> Signed-off-by: dhairya <dhairya.jangir.s73@kalvium.community>

Update lib/Migration/Version5007Date20251019000000.php

8ef9ae9

Co-authored-by: Daniel <mail@danielkesselberg.de> Signed-off-by: dhairya <dhairya.jangir.s73@kalvium.community>

Update lib/Migration/Version5007Date20251019000000.php

11f5f97

Co-authored-by: Daniel <mail@danielkesselberg.de> Signed-off-by: dhairya <dhairya.jangir.s73@kalvium.community>

Merge branch 'main' into fix/attachment-createdat-timestamp-and-migra…

8f61520

…tion

dhairyajangir and others added 4 commits October 23, 2025 12:12

Update lib/Migration/Version5007Date20251019000000.php

f9e5510

Co-authored-by: Daniel <mail@danielkesselberg.de> Signed-off-by: dhairya <dhairya.jangir.s73@kalvium.community>

Update lib/Migration/Version5007Date20251019000000.php

cc2a93c

line 7 updated `AGPL-3.0-only` set to `AGPL-3.0-or-later` Co-authored-by: Daniel <mail@danielkesselberg.de> Signed-off-by: dhairya <dhairya.jangir.s73@kalvium.community>

Merge branch 'main' into fix/attachment-createdat-timestamp-and-migra…

4dfb510

…tion

chore: bump version to 5.6.0-alpha.4

91ade16

Version bump to trigger migration Version5007Date20251019000000 which enforces NOT NULL and removes default on mail_attachments.created_at Signed-off-by: dhairyasquad73 <dhairya.jangir.s73@kalvium.community>

kesselb changed the base branch from main to bug/5901/set-created-at-on-insert October 23, 2025 22:41

kesselb merged commit a003bbe into nextcloud:bug/5901/set-created-at-on-insert Oct 23, 2025
1 check passed

kesselb mentioned this pull request Oct 23, 2025

fix: set createdAt timestamp for local attachments #11966

Draft

		@@ -0,0 +1,45 @@
		<?php

		declare(strict_types=1);

Uh oh!

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

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

Conversation

dhairyajangir commented Oct 19, 2025 • edited by ChristophWurst Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR: Enforce NOT NULL and Remove Default Value for mail_attachments.created_at

Overview

What Has Changed

1. Database Migration

2. How the Migration Works

3. Why This Is Important

4. How to Test

5. Additional Notes

Uh oh!

welcome bot commented Oct 19, 2025

Uh oh!

ChristophWurst left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ChristophWurst commented Oct 20, 2025

Uh oh!

ChristophWurst commented Oct 20, 2025

Uh oh!

ChristophWurst commented Oct 20, 2025

Uh oh!

dhairyajangir commented Oct 20, 2025

Uh oh!

ChristophWurst commented Oct 20, 2025

Uh oh!

dhairyajangir commented Oct 20, 2025

Uh oh!

ChristophWurst commented Oct 20, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kesselb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kesselb Oct 21, 2025

Choose a reason for hiding this comment

Uh oh!

dhairyajangir Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

dhairyajangir Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

dhairyajangir commented Oct 21, 2025

Uh oh!

dhairyajangir commented Oct 21, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dhairyajangir commented Oct 19, 2025 •

edited by ChristophWurst

Loading

PR: Enforce NOT NULL and Remove Default Value for `mail_attachments.created_at`