Skip to content

Conversation

hamza221
Copy link
Contributor

No description provided.

@hamza221
Copy link
Contributor Author

I realized this can be merged independently from #11529, rebasing

@hamza221 hamza221 force-pushed the feat/thread-attachements branch from c8079fb to 185eb96 Compare August 28, 2025 16:35
@hamza221 hamza221 changed the base branch from feat/envelope-lists to main August 28, 2025 16:35
Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works, but I'm not a big fan. It's using the distributed cache as storage. Cons:

  1. Does not work when there is no distributed cache. It will still do the expensive fetching, but the results are losts.
  2. For small installations that use APCu for local+distributed caching this will inflate cache memory quite a bit and risk that the cache is fully evicted because it's running full.

$result = array_map(static function (array $attachment) {
return $attachment['fileName'];
}, $attachments);
$this->cache->set($message->getUid(), $result);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UIDs are unique in the scope of a single mailbox. You'll run into conflicts across mailboxes, and worse, across user accounts. Use the primary key or find another unique identifier.

Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Read the code wrong. Does exactly what we discussed :shipit:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants